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})