From d60fde96f4e50633543352b10c9497ccf51d2d5a Mon Sep 17 00:00:00 2001 From: Bedram Tamang Date: Sat, 31 Oct 2026 12:58:54 -0700 Subject: [PATCH] Fix MorphMany/MorphOne loads when the morph name equals the class name get_record_key_lookup treated a morph name equal to the class name as an unregistered model and raised ValueError, but every Model registers its class name as the default morph name, so unaliased models could never be eager or lazy loaded. Return the registry's morph name as-is, and forward nested eagers in MorphMany/MorphOne.get_related. Co-Authored-By: Claude Opus 5.5 --- docs/adr/055-morph-name-equals-class-name.md | 42 +++++++++ docs/adr/AGENTS.md | 1 + .../masoniteorm/relationships/MorphMany.py | 7 +- .../masoniteorm/relationships/MorphOne.py | 7 +- .../test_morph_name_equals_class_name.py | 93 +++++++++++++++++++ .../sqlite/relationships/test_morph_one.py | 8 +- 6 files changed, 143 insertions(+), 15 deletions(-) create mode 100644 docs/adr/055-morph-name-equals-class-name.md create mode 100644 fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_morph_name_equals_class_name.py diff --git a/docs/adr/055-morph-name-equals-class-name.md b/docs/adr/055-morph-name-equals-class-name.md new file mode 100644 index 00000000..35a671f9 --- /dev/null +++ b/docs/adr/055-morph-name-equals-class-name.md @@ -0,0 +1,42 @@ +# 055 — Morph relations accept the class name as the morph name + +- Date: 2026-10-31 +- Status: Accepted + +## Problem + +`Lexeme.with_("media")`, where `media` is a `MorphMany` on `Lexeme` and the morph name is `"Lexeme"`, raised `ValueError: Could not find the record type key for the class`. `load("media")` raised the same error. `MorphOne` behaved the same way. `MorphTo` was not affected. + +`MorphMany.get_record_key_lookup` and `MorphOne.get_record_key_lookup` asked `Registry.get_morph_name(cls)` for the parent's morph name. `get_morph_name` falls back to `cls.__name__`, so the method read "morph name equals the class name" as "model not registered" and raised. That guess is wrong. `Model.__init_subclass__` calls `Registry.register(cls)`, which stores the class name as every model's default morph name. Any model without a `Registry.morph_map` alias, and any model mapped to its own class name, always took the error branch. A model can never be missing from the registry, so the check never caught a real problem. + +`MorphMany.get_related` and `MorphOne.get_related` also dropped their `eagers` argument. Nested eager loads such as `with_("media.lexeme")` therefore did not load the inner relation, while `HasMany`/`HasOne` forward it with `with_(eagers)`. + +## Alternatives + +1. **Keep the guard, but check `Registry._reverse_map` membership.** Every `Model` subclass is in the reverse map, so the guard would never fire. This is dead code that still reads `Registry` internals. +2. **Require an explicit `morph_map` entry.** Breaks the default the registry already provides, and the class name stored in `record_type` is a normal choice (Laravel stores the class when there is no morph map). +3. **Return `Registry.get_morph_name(cls)` as-is.** Chosen. `morph_map` aliases still win because `Registry.morph_map` overrides the reverse map. + +## Decision + +- `get_record_key_lookup` on `MorphMany` and `MorphOne` returns `Registry.get_morph_name(relation.__class__)`. The class-name default is a valid morph name. +- `MorphMany.get_related` and `MorphOne.get_related` build the polymorphic query with `.with_(eagers or [])`, so nested eager loads reach the related model. + +## Consequences + +- Models without a morph-map alias work with `MorphMany`/`MorphOne` eager (`with_`) and lazy (`load`) loading. They write and read the class name in the morph type column. +- `test_get_record_key_lookup_raises_for_unknown` in `test_morph_one.py` asserted the old error for an unmapped model. It now asserts the class-name default. +- `MorphTo` and `MorphToMany` already resolved class names through `Registry.get_morph_model` and are unchanged. + +## Validation + +`tests/masoniteorm/sqlite/relationships/test_morph_name_equals_class_name.py` uses a `Lexeme` parent and a `LexemeMedia` child with no morph-map alias (morph name `"Lexeme"`). It covers: + +- the morph name lookup returns `"Lexeme"`; +- `MorphMany` eager load on a collection and on `first()`, with rows matched to the right parent; +- `MorphMany` and `MorphOne` lazy `load`; +- `MorphOne` eager load on a collection; +- `MorphTo` eager load back to `Lexeme`; +- nested eager loads `media.lexeme` and `first_medium.lexeme`. + +Without the fix, every case except `MorphTo` failed with the `ValueError` above. The full suite runs with `uv run pytest --ignore=tests/masoniteorm/postgres --cov`. diff --git a/docs/adr/AGENTS.md b/docs/adr/AGENTS.md index 86b6967c..4952cfc2 100644 --- a/docs/adr/AGENTS.md +++ b/docs/adr/AGENTS.md @@ -46,3 +46,4 @@ Read this index first, then open the records relevant to the change. Before impl | [051](051-nullable-column-casts.md) | 2026-10-31 | Nullable annotations infer the cast of their inner type | `Caster.normalize_type` only knew bare types, so `int \| None`, `Optional[int]` and `Union[int, None]` fell through to the `"str"` cast and nullable columns (e.g. a `HasMany` foreign key) were read back as strings, breaking eager-load key matching. `Caster.build` (replacing `build_casts`) removes `None` from a top-level union, also around or inside `Annotated`, maps the inner type to its usual cast and records the field as nullable. For nullable fields only, `Caster.get` turns `None` into `None` (or the `Field` default) and `Caster.set` writes `None`; bare annotations keep their behaviour. Unions of several non-`None` types get a pass-through `BaseCast`. Relationship key matching stays type-strict. | | [052](052-sqlite-savepoint-isolation.md) | 2026-10-31 | SQLite savepoints start a real transaction | pysqlite/aiosqlite defer `BEGIN` until the first DML, so a savepoint opened first in an outer transaction became a top-level transaction and `RELEASE` committed it, leaking rows past `DatabaseTransaction`/`RefreshDatabase` rollbacks. `SQliteConnection` listens to the engine `savepoint` event and runs `BEGIN` when the driver connection is not in a transaction. On the shared `StaticPool` connection, a lost `BEGIN` race (`OperationalError` while a transaction is already open) is ignored, so concurrent nested transactions still succeed as on `main`. SQLAlchemy's `isolation_level=None` + `BEGIN` on every `begin` recipe was rejected because concurrent transactions on `SQLiteConfig`'s shared `StaticPool` connection then fail with `cannot start a transaction within a transaction`. | | [053](053-validated-reads-query-and-body.md) | 2026-10-31 | `Validated` reads the query string and the body | `Validated[Model]` declared a required `Body()`, so a `GET` with filters got `422 body: Field required` and apps bound models with `Query()`, skipping `authorize`, rules and `after`. The body parameter now defaults to `None`. Without a body, `GET`/`HEAD`/`OPTIONS`/`DELETE` validate `query_input`, while `POST`/`PUT`/`PATCH` still report `"": Field required` (per embed key when embedded), so a body-less cross-site form `POST` cannot pass on the query string (CSRF). `DELETE` always needs a CORS preflight. With a body, `request_input` merges `{**query, **body}` for an object; any other body, a JSON `null` included, passes through and fails with `model_type`. `query_input` keeps the keys pydantic accepts (`validation_alias`, `AliasChoices`, single-key `AliasPath`, the name with `validate_by_name`/`populate_by_name`). `collections.abc` `Sequence`/`Set` fields (not `str`/`bytes`) get every repeated value. Declared fields can be set from the URL on write methods with a JSON body, as with Laravel's `$request->all()`: keep sensitive fields off such models. `document_query_input` (installed by `FastAPIProvider`) replaces `requestBody` with `in: query` parameters on query-method routes. Chosen over a separate `ValidatedQuery` so one `FormRequest` serves `GET` and `POST`. | +| [055](055-morph-name-equals-class-name.md) | 2026-10-31 | Morph relations accept the class name as the morph name | `MorphMany`/`MorphOne.get_record_key_lookup` raised `ValueError` whenever `Registry.get_morph_name` returned the class name, treating it as "not registered". `Model.__init_subclass__` registers every model with its class name as the default morph name, so every model without a `morph_map` alias (or mapped to its own class name) failed on `with_` and `load`. The lookup now returns `Registry.get_morph_name(cls)` as-is (`morph_map` aliases still win). `MorphMany`/`MorphOne.get_related` also forward `eagers` via `.with_(eagers or [])`, so nested loads such as `with_("media.lexeme")` work. `MorphTo`/`MorphToMany` were unaffected. | diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/MorphMany.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/MorphMany.py index d62c14b4..8c2e50aa 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/MorphMany.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/MorphMany.py @@ -68,7 +68,7 @@ def get_related(self, query, relation, eagers=None, callback=None): Returns: Model|Collection """ - self.polymorphic_builder = registry.Registry.resolve(self.fn).query() + self.polymorphic_builder = registry.Registry.resolve(self.fn).query().with_(eagers or []) if isinstance(relation, Collection): record_type = self.get_record_key_lookup(relation.first()) @@ -124,7 +124,4 @@ def morph_map(self): return registry.Registry.get_morph_map() def get_record_key_lookup(self, relation): - record_type = registry.Registry.get_morph_name(relation.__class__) - if record_type == relation.__class__.__name__: - raise ValueError(f"Could not find the record type key for the {relation} class") - return record_type + return registry.Registry.get_morph_name(relation.__class__) diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/MorphOne.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/MorphOne.py index 4b6b286c..1d976e0d 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/MorphOne.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/MorphOne.py @@ -97,7 +97,7 @@ def get_related(self, query, relation, eagers=None, callback=None): Returns: Model|Collection """ - self.polymorphic_builder = self.resolver()(self)() + self.polymorphic_builder = self.resolver()(self)().with_(eagers or []) if isinstance(relation, Collection): record_type = self.get_record_key_lookup(relation.first()) @@ -156,7 +156,4 @@ def morph_map(self): return registry.Registry.get_morph_map() def get_record_key_lookup(self, relation): - record_type = registry.Registry.get_morph_name(relation.__class__) - if record_type == relation.__class__.__name__: - raise ValueError(f"Could not find the record type key for the {relation} class") - return record_type + return registry.Registry.get_morph_name(relation.__class__) diff --git a/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_morph_name_equals_class_name.py b/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_morph_name_equals_class_name.py new file mode 100644 index 00000000..fd720d96 --- /dev/null +++ b/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_morph_name_equals_class_name.py @@ -0,0 +1,93 @@ +from fastapi_startkit.masoniteorm import Model +from fastapi_startkit.masoniteorm.collection import Collection +from fastapi_startkit.masoniteorm.models.registry import Registry +from fastapi_startkit.masoniteorm.relationships import MorphMany, MorphOne, MorphTo +from ..test_case import TestCase + + +def first_medium(self): + return LexemeMedia + + +class LexemeMedia(Model): + __table__ = "likes" + + lexeme = MorphTo("LexemeMedia", "likeable_type", "likeable_id") + + +class Lexeme(Model): + __table__ = "articles" + + media = MorphMany("LexemeMedia", morph_key="likeable_type", morph_id="likeable_id") + first_medium = MorphOne(first_medium, morph_key="likeable_type", morph_id="likeable_id") + + +class TestMorphNameEqualsClassName(TestCase): + async def asyncSetUp(self): + await super().asyncSetUp() + self.lexemes = [ + await Lexeme.create({"title": f"Lexeme {n}", "user_id": 1, "published_date": "2024-01-01 00:00:00"}) + for n in range(2) + ] + for lexeme in self.lexemes: + for _ in range(2): + await LexemeMedia.create({"likeable_type": "Lexeme", "likeable_id": lexeme.id}) + + async def test_registry_uses_class_name_as_default_morph_name(self): + self.assertEqual(Registry.get_morph_name(Lexeme), "Lexeme") + self.assertEqual(Lexeme.media.get_record_key_lookup(self.lexemes[0]), "Lexeme") + + async def test_eager_load_morph_many_on_collection(self): + lexemes = await Lexeme.with_("media").where_in("id", [lexeme.id for lexeme in self.lexemes]).get() + + self.assertEqual(len(lexemes), 2) + for lexeme in lexemes: + self.assertIsInstance(lexeme.media, Collection) + self.assertEqual(len(lexeme.media), 2) + self.assertTrue(all(medium.likeable_id == lexeme.id for medium in lexeme.media)) + + async def test_eager_load_morph_many_on_first(self): + lexeme = await Lexeme.with_("media").where("id", self.lexemes[0].id).first() + + self.assertEqual(len(lexeme.media), 2) + + async def test_lazy_load_morph_many(self): + lexeme = await Lexeme.find(self.lexemes[1].id) + await lexeme.load("media") + + self.assertEqual(len(lexeme.media), 2) + self.assertTrue(all(medium.likeable_id == lexeme.id for medium in lexeme.media)) + + async def test_eager_load_morph_one(self): + lexemes = await Lexeme.with_("first_medium").where_in("id", [lexeme.id for lexeme in self.lexemes]).get() + + for lexeme in lexemes: + self.assertIsInstance(lexeme.first_medium, LexemeMedia) + self.assertEqual(lexeme.first_medium.likeable_id, lexeme.id) + + async def test_lazy_load_morph_one(self): + lexeme = await Lexeme.find(self.lexemes[0].id) + await lexeme.load("first_medium") + + self.assertIsInstance(lexeme.first_medium, LexemeMedia) + self.assertEqual(lexeme.first_medium.likeable_id, lexeme.id) + + async def test_eager_load_morph_to(self): + media = await LexemeMedia.with_("lexeme").where("likeable_type", "Lexeme").get() + + self.assertEqual(len(media), 4) + for medium in media: + self.assertIsInstance(medium.lexeme, Lexeme) + self.assertEqual(medium.lexeme.id, medium.likeable_id) + + async def test_nested_eager_load_through_morph_one(self): + lexemes = await Lexeme.with_("first_medium.lexeme").where_in("id", [lexeme.id for lexeme in self.lexemes]).get() + + for lexeme in lexemes: + self.assertEqual(lexeme.first_medium.lexeme.id, lexeme.id) + + async def test_nested_eager_load(self): + lexeme = await Lexeme.with_("media.lexeme").where("id", self.lexemes[0].id).first() + + self.assertEqual(len(lexeme.media), 2) + self.assertTrue(all(medium.lexeme.id == lexeme.id for medium in lexeme.media)) diff --git a/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_morph_one.py b/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_morph_one.py index 53fd588c..28d7d2b8 100644 --- a/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_morph_one.py +++ b/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_morph_one.py @@ -65,15 +65,13 @@ async def test_get_record_key_lookup_returns_key(self): key = rel.get_record_key_lookup(self.article) self.assertEqual(key, "article_one") - async def test_get_record_key_lookup_raises_for_unknown(self): + async def test_get_record_key_lookup_defaults_to_class_name(self): rel = MorphOne(first_like, morph_key="likeable_type", morph_id="likeable_id") - class UnknownModel(Model): + class UnmappedModel(Model): __table__ = "articles" - unknown = UnknownModel() - with self.assertRaises(ValueError): - rel.get_record_key_lookup(unknown) + self.assertEqual(rel.get_record_key_lookup(UnmappedModel()), "UnmappedModel") async def test_apply_query_returns_single_like(self): await Like.create({"likeable_type": "article_one", "likeable_id": self.article.id})