From ae853e45b31dc5b4e7860408d0c556eced742081 Mon Sep 17 00:00:00 2001 From: Bedram Tamang Date: Sat, 31 Oct 2026 02:29:22 -0700 Subject: [PATCH 1/2] Address post-merge review of has-one-of-many - Ignore NULL columns in the of-many subquery so latest/oldest follow MAX/MIN semantics on every database - of_many() defaults to the related primary key and orders by it once - Type the aggregate as Literal["max", "min"] - Reject @relationship methods returning an existing descriptor or a builder, and HasMany relations without a resolvable foreign key - Rename relationships/relationship.py to RelationshipMethod.py while keeping the public relationship imports - Cover NULLs, custom primary keys, subclasses, self-referencing tables and MySQL/PostgreSQL SQL - Mark ADR 050 accepted and document the MySQL 1093 and naming caveats Co-Authored-By: Claude Opus 5.5 --- docs/adr/050-has-one-of-many.md | 45 +++-- docs/adr/AGENTS.md | 2 +- .../masoniteorm/relationships/HasMany.py | 4 +- .../masoniteorm/relationships/HasOneOfMany.py | 28 +-- ...{relationship.py => RelationshipMethod.py} | 5 + .../masoniteorm/relationships/__init__.py | 2 +- .../builder/test_mysql_has_one_of_many_sql.py | 30 ++++ .../test_postgres_has_one_of_many_sql.py | 40 +++++ .../relationships/test_has_one_of_many.py | 161 +++++++++++++++++- 9 files changed, 286 insertions(+), 31 deletions(-) rename fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/{relationship.py => RelationshipMethod.py} (76%) create mode 100644 fastapi_startkit/tests/masoniteorm/mysql/builder/test_mysql_has_one_of_many_sql.py create mode 100644 fastapi_startkit/tests/masoniteorm/postgres/grammar/test_postgres_has_one_of_many_sql.py diff --git a/docs/adr/050-has-one-of-many.md b/docs/adr/050-has-one-of-many.md index 133d705f..1d72a441 100644 --- a/docs/adr/050-has-one-of-many.md +++ b/docs/adr/050-has-one-of-many.md @@ -1,7 +1,7 @@ -# 050 — Has-one-of-many derived from a HasMany (POC) +# 050 — Has-one-of-many derived from a HasMany - Date: 2026-10-31 -- Status: Proposed (proof of concept, draft PR) +- Status: Accepted (merged in #331; post-merge review fixes in a follow-up PR). The open questions below are pending a user decision. ## Problem @@ -20,22 +20,45 @@ The ORM had no way to express this. `HasOne` eager-loads every child with `where 1. **Pick in Python** — load every child and keep the max per parent. Rejected: loads N rows to keep one and breaks `where_has`. 2. **Window function** — `id IN (SELECT id FROM (SELECT id, ROW_NUMBER() OVER (PARTITION BY fk ORDER BY col DESC, id DESC) rn FROM t) WHERE rn = 1)`. Works on SQLite ≥ 3.25, PostgreSQL and MySQL 8, but the builder has no derived-table source, so it needs hand-written SQL in `where_raw`, and without pushing the parent filter inside the window it ranks the whole table. 3. **Laravel's join to an aggregate subquery** (`JOIN (SELECT MAX(col), fk … GROUP BY fk)`, with a second aggregate on the pk for ties). Needs `join_sub` (PR #330) and two nested aggregates for the tie-break. -4. **Correlated subquery on the primary key** — `t.id = (SELECT o.id FROM t AS o WHERE o.fk = t.fk ORDER BY o.col DESC, o.id DESC LIMIT 1)`. Chosen. +4. **Correlated subquery on the primary key** — `t.id = (SELECT o.id FROM t AS o WHERE o.fk = t.fk AND o.col IS NOT NULL ORDER BY o.col DESC, o.id DESC LIMIT 1)`. Chosen. ## Decision -- `HasMany.latest_of_many(column="created_at")`, `oldest_of_many(column="created_at")` and `of_many(column="id", aggregate="max"|"min")` return a new `HasOneOfMany`, a `HasOne` subclass that reuses the parent's related model and keys. Any other aggregate raises `ValueError`. -- `HasOneOfMany` overrides only `get_builder` and `get_exists_builder`. Both add `table.pk = (correlated subquery)`. The subquery orders by the column, then by the primary key in the same direction, so ties are deterministic (`max` → latest pk wins, `min` → earliest pk wins). The inherited `HasOne` eager (`where_in`), lazy (`load`), `where_has` and `register_related` code paths then work unchanged and always see at most one row per parent. -- `relationship` (in `relationships/relationship.py`) is a decorator that returns a `RelationshipMethod` descriptor. On first access from an owner class it calls the decorated function with the **class** as its argument, so `self.statuses` is the class-level `HasMany` descriptor, not a loaded collection. The resulting relation is named after the method and cached per owner class. Instance access goes through the relation's normal `access` (loaded value or `LazyLoadingViolation`). `@relationship @classmethod` is also accepted. +- `HasMany.latest_of_many(column="created_at")`, `oldest_of_many(column="created_at")` and `of_many(column=None, aggregate="max"|"min")` return a new `HasOneOfMany`, a `HasOne` subclass that reuses the parent's related model and keys. `aggregate` is typed `Literal["max", "min"]`, and any other value raises `ValueError` at runtime. `column=None` means the related model's `__primary_key__`, so models with a custom primary key work. +- `HasOneOfMany` overrides only `get_builder` and `get_exists_builder`. Both add `table.pk = (correlated subquery)`. The subquery orders by the column and then by the primary key in the same direction, so ties are deterministic (`max` → latest pk wins, `min` → earliest pk wins). When the column is the primary key, it is ordered once. The inherited `HasOne` eager (`where_in`), lazy (`load`), `where_has` and `register_related` code paths then work unchanged and always see at most one row per parent. +- NULLs: the subquery adds `column IS NOT NULL`, which gives `MAX`/`MIN` semantics on every database (NULLs are ignored), instead of each database's own NULL sort position. Without it, PostgreSQL's `DESC` and SQLite/MySQL's `ASC` would put NULL rows first. A parent whose children all have a NULL column gets `None`. +- `HasOneOfMany` runs `HasOne.__init__`, so `_model_name` and `fn` are set the usual way, and `related_model()` resolves through the parent relation. It raises `TypeError` when the parent `HasMany` has neither an explicit `foreign_key` nor an attribute name to derive one from, instead of silently using `None_id`. +- `relationship` (in `relationships/RelationshipMethod.py`, still exported as `fastapi_startkit.masoniteorm.relationship` and `fastapi_startkit.masoniteorm.relationships.relationship`) is a decorator that returns a `RelationshipMethod` descriptor. The module was renamed from `relationship.py` because the exported function shadowed it and its name was close to `models/relationship.py`. On first access from an owner class, the decorator calls the decorated function with the **class** as its argument, so `self.statuses` is the class-level `HasMany` descriptor, not a loaded collection. The result must be a new, unnamed `BaseRelationship`. Returning an existing descriptor (`return self.statuses`) or a builder raises `TypeError`, because naming it would rename the shared descriptor. The relation is named after the method and cached per owner class, so subclasses resolve their own. Instance access goes through the relation's normal `access` (loaded value or `LazyLoadingViolation`). Both the plain-function form and `@relationship @classmethod` are supported. - Typing: `relationship(fn: Callable[[Any], R]) -> R`, so the method's return annotation decides the type. `User.due_date` is `HasOne[Status]` and `user.due_date` is `Status | None`. -## Known limitations / open questions +## Caveats -- basedpyright reports `self.statuses.latest_of_many()` as an error, because instance access types `statuses` as `Collection[Status]`. The `@relationship @classmethod def due_date(cls)` form type-checks cleanly. Choosing one canonical form is still open. +- MySQL rejects writes through the relation, e.g. `User.due_date.where(...).update()` / `.delete()`, with error 1093 (the target table appears in a subquery). Reads are fine. Write through the `HasMany` or the related model instead. +- A relationship method named `latest` or `oldest` overrides `Model.latest()`/`Model.oldest()` (basedpyright `reportIncompatibleMethodOverride`). Pick another name. +- `where_column` emits unquoted identifiers (pre-existing behaviour), and now every of-many query uses it, so mixed-case table names on PostgreSQL would break. +- `@classmethod @relationship` (reversed order) is not supported. +- Performance relies on an index on `(fk, column, pk)`. + +## Open questions (pending user decision) + +- Canonical form: basedpyright reports `self.statuses.latest_of_many()` as an error, because instance access types `statuses` as `Collection[Status]`. The `@relationship @classmethod def due_date(cls)` form type-checks cleanly. Both forms are kept until the user chooses. +- Default column: `latest_of_many`/`oldest_of_many` keep `created_at`, matching `Model.latest()`/`oldest()`. Laravel defaults to the key. - Eager callbacks (`with_({"due_date": cb})`) and `where_has` callbacks filter the selected row ("the latest status, if it is done"), not the candidate set. Laravel's `ofMany(..., closure)` inner constraints are not implemented. -- Global scopes on the related model run on the correlated subquery's builder too. Scopes that hard-code the table name instead of using the builder's alias are not covered by this POC. -- Performance relies on an index on `(fk, column, pk)`. Only SQLite is tested. ## Validation -`tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py` covers: latest, oldest and `of_many` max/min with pk tie-breaks, parents without children (`None`), eager loading in one query (counted through `Connection.select`), lazy `load` on a model and on a collection, `where_has` with and without a callback, the classmethod form, unchanged `HasMany`, and rejection of unknown aggregates. The full suite (excluding Postgres) and basedpyright on `masoniteorm/relationships` pass. +`tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py` covers: +- latest, oldest and `of_many` max/min with pk tie-breaks +- parents without children (`None`) +- eager loading in one query (counted through `Connection.select`) +- lazy `load` on a model and on a collection +- `where_has` with and without a callback +- the classmethod form and an unchanged `HasMany` +- rejection of unknown aggregates, existing descriptors, builders and unresolvable foreign keys +- `related_model()` +- NULL columns ignored for latest and oldest +- the `of_many()` default on a custom primary key, ordered once +- a subclass of the owner +- a self-referencing table, eager and `where_has` + +SQL snapshots are in `tests/masoniteorm/mysql/builder/test_mysql_has_one_of_many_sql.py` and `tests/masoniteorm/postgres/grammar/test_postgres_has_one_of_many_sql.py`. Neither needs a live server. The full suite (excluding Postgres), basedpyright and ruff all pass. diff --git a/docs/adr/AGENTS.md b/docs/adr/AGENTS.md index 984f929d..fabca106 100644 --- a/docs/adr/AGENTS.md +++ b/docs/adr/AGENTS.md @@ -42,4 +42,4 @@ Read this index first, then open the records relevant to the change. Before impl | [042](042-fluent-validation-rules.md) | 2026-10-29 | Fluent validation rules with an async phase and a Laravel 422 shape | New `fastapi_startkit.validation`: `Field()`/`Email()` builders used as `Annotated` metadata on a `FormRequest` model (`required`, `nullable`, `string`, `integer`, `numeric`, `boolean`, `min`, `max`, `between`, `email`, `url`, `regex`, `in_`, `not_in`, `same_as`, `confirmed`, `required_if`, `password`, `unique`, `exists`, `add`, `msg`). A wildcard `field_validator` only records coerced values and the wrap `model_validator` fills missing `required` keys with `None`; it then runs sync rules per field in order with all values (so cross-field rules see later fields), skipping empty values for non-implicit rules and stopping at the first failure. `AsyncRule`s (`unique`/`exists` via the async `QueryBuilder`) run in an awaited second phase, `validate_async(context)`, triggered by the `Validated[Model]` FastAPI dependency with the route's typed path params as context (`ignore=lambda data: data["user_id"]`); an invalid path param skips the async phase so FastAPI returns its path 422 before any query; path params are collected by walking the public `Dependant` tree (no `get_flat_dependant`, removed in FastAPI 0.140) and cached per route. The dependency body key is the snake-cased model name (visible only when the endpoint has other body params). Rules on nested models raise `TypeError` at class definition. The existing unused `ValidationException` becomes a 422 `HttpException` carrying a `MessageBag`; `FastAPIProvider`'s existing `ValidationExceptionHandler` renders both phases as `{"message", "errors"}` (only the `message` key is new). | | [047](047-ai-bedrock-provider.md) | 2026-10-29 | AWS Bedrock provider for the AI module | Adds `provider="bedrock"` via LangChain's `bedrock_converse` (`ChatBedrockConverse`), the same `init_chat_model` path as other providers. `BedrockConfig` reads `AWS_REGION`/`AWS_DEFAULT_REGION`, IAM keys, session token, `AWS_PROFILE`, `AWS_BEARER_TOKEN_BEDROCK` and `AWS_BEDROCK_MODEL`. `Lab.get_client_options()` maps them to ChatBedrockConverse arguments and drops empty values so boto3's default credential chain applies; other providers return `{}`. The `bedrock` extra is `fastapi-startkit[ai]` plus `langchain-aws`, because `langchain-aws` does not pull in `langchain`. `langchain-aws` is imported only by `init_chat_model` when a Bedrock model is built. | | [048](048-async-first-validation.md) | 2026-10-30 | Async-first validation: one awaited rule pass, async hooks and closures | Supersedes ADR 042's two-phase design (sync rules in pydantic, `AsyncRule`s in `validate_async`). `await Model.validate_input(data, context=None, request=None)` is the only validation entry point: a per-call `Validator` runs `authorize` (false → 403 `AuthorizationException`), `prepare_for_validation`, a pydantic type-only parse (type errors added to the bag), then every field's rules in declaration and written order, awaiting a rule's result only when it is awaitable (sync rules unchanged, `unique`/`exists` in place via the async `QueryBuilder`), then `after`, and raises one `ValidationException`. Hooks may be sync or async classmethods; `Field().using(callback)` adds sync/async closure rules; `unique(ignore=...)` accepts async callables; `stop_on_first_failure` stops after the first error. `model_validate`/plain body params only parse types. `Validated[Model]` takes the raw body (`RawInput` keeps the OpenAPI `$ref` via `json_schema_input_schema`) and awaits `validate_input` with the typed path params, so type and rule errors share one 422. Several `Validated` params on one route are all validated: each records its errors in the request scope (prefixed with its embed key, e.g. `shipping.name`, when the body is embedded) and the route's last `Validated` dependency raises them together; `ValidationExceptionHandler` merges recorded errors into a `RequestValidationError` too. With `FastAPIProvider`, every 422 (also plain `RequestValidationError`) is `{"message", "errors"}`, a superset of `main`'s `{"errors"}`. `validate_async` is removed. | -| [050](050-has-one-of-many.md) | 2026-10-31 | Has-one-of-many derived from a HasMany (POC) | `HasMany.latest_of_many(col="created_at")`, `oldest_of_many(col)` and `of_many(col, "max"\|"min")` return a `HasOneOfMany` (`HasOne` subclass reusing the parent keys) whose `get_builder`/`get_exists_builder` add `pk = (SELECT pk … WHERE fk = outer.fk ORDER BY col, pk LIMIT 1)`, so eager `with_`, lazy `load` and `where_has` pick one row per parent in SQL with a pk tie-break, portable without window functions or `join_sub`. A new `@relationship` decorator calls the method with the owner class (so `self.statuses` is the descriptor), names and caches the relation per class, and types it by the return annotation; `@relationship @classmethod` is the basedpyright-clean form. | +| [050](050-has-one-of-many.md) | 2026-10-31 | Has-one-of-many derived from a HasMany | `HasMany.latest_of_many(col="created_at")`, `oldest_of_many(col)` and `of_many(col=None → related pk, Literal["max","min"])` return a `HasOneOfMany` (`HasOne` subclass reusing the parent keys; `TypeError` if the fk cannot be resolved). Its `get_builder`/`get_exists_builder` add `pk = (SELECT pk … WHERE fk = outer.fk AND col IS NOT NULL ORDER BY col, pk LIMIT 1)`, so eager `with_`, lazy `load` and `where_has` pick one row per parent in SQL with a pk tie-break, NULLs ignored like `MAX`/`MIN`, and no window functions or `join_sub`. `@relationship` (module `RelationshipMethod.py`) calls the method with the owner class (so `self.statuses` is the descriptor), requires a new unnamed relation (`TypeError` otherwise), names and caches it per class and types it by the return annotation; `@relationship @classmethod` is the basedpyright-clean form. MySQL error 1093 blocks writes through the relation. Canonical form, default column and inner constraints are open. | diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasMany.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasMany.py index 71bdb6e0..ac77c4cc 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasMany.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasMany.py @@ -1,5 +1,5 @@ from .BaseRelationship import BaseRelationship, ManyResult, TModel -from .HasOneOfMany import HasOneOfMany +from .HasOneOfMany import HasOneOfMany, OfManyAggregate from ..collection import Collection @@ -32,7 +32,7 @@ def latest_of_many(self, column: str = "created_at") -> HasOneOfMany[TModel]: def oldest_of_many(self, column: str = "created_at") -> HasOneOfMany[TModel]: return self.of_many(column, "min") - def of_many(self, column: str = "id", aggregate: str = "max") -> HasOneOfMany[TModel]: + def of_many(self, column: str | None = None, aggregate: OfManyAggregate = "max") -> HasOneOfMany[TModel]: return HasOneOfMany(self, column, aggregate) def register_related(self, key, model, collection): diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasOneOfMany.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasOneOfMany.py index 766f0103..20da1af9 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasOneOfMany.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasOneOfMany.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Literal from .BaseRelationship import TModel from .HasOne import HasOne @@ -6,19 +6,21 @@ if TYPE_CHECKING: from .HasMany import HasMany -AGGREGATE_DIRECTIONS = {"max": "desc", "min": "asc"} +type OfManyAggregate = Literal["max", "min"] + +AGGREGATE_DIRECTIONS: dict[str, str] = {"max": "desc", "min": "asc"} class HasOneOfMany(HasOne[TModel]): - def __init__(self, relation: "HasMany[TModel]", column: str, aggregate: str): + def __init__(self, relation: "HasMany[TModel]", column: str | None, aggregate: OfManyAggregate): if aggregate not in AGGREGATE_DIRECTIONS: raise ValueError(f"of_many() aggregate must be one of {', '.join(AGGREGATE_DIRECTIONS)}, got {aggregate!r}") + if relation.foreign_key is None and relation.attribute is None: + raise TypeError("of_many() needs a HasMany declared on a model or given an explicit foreign_key") relation.set_keys(None, relation.attribute) + super().__init__(foreign_key=relation.foreign_key, local_key=relation.local_key) self.relation = relation - self.fn = self.related_model - self.local_key = relation.local_key - self.foreign_key = relation.foreign_key self.column = column self.direction = AGGREGATE_DIRECTIONS[aggregate] self.attribute = None @@ -44,11 +46,9 @@ def one_of_many(self, table): builder = self.relation.get_builder() alias = f"{builder.get_table_name()}_of_many" key = builder._model.__primary_key__ - return ( - builder.table(builder.get_table_name(), alias) - .select(f"{alias}.{key}") - .where_column(f"{alias}.{self.foreign_key}", f"{table}.{self.foreign_key}") - .order_by(f"{alias}.{self.column}", self.direction) - .order_by(f"{alias}.{key}", self.direction) - .limit(1) - ) + column = self.column or key + query = builder.table(builder.get_table_name(), alias).select(f"{alias}.{key}") + query.where_column(f"{alias}.{self.foreign_key}", f"{table}.{self.foreign_key}") + if column != key: + query.where_not_null(f"{alias}.{column}").order_by(f"{alias}.{column}", self.direction) + return query.order_by(f"{alias}.{key}", self.direction).limit(1) diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/relationship.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/RelationshipMethod.py similarity index 76% rename from fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/relationship.py rename to fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/RelationshipMethod.py index a0d6c7be..a63d64e7 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/relationship.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/RelationshipMethod.py @@ -18,6 +18,11 @@ def __set_name__(self, owner, name): def relation(self, owner: type) -> BaseRelationship: if owner not in self.relations: relation = self.method(owner) + if not isinstance(relation, BaseRelationship) or vars(relation).get("attribute") is not None: + raise TypeError( + f"@relationship {owner.__name__}.{self.attribute} must return a new relationship, " + f"got {type(relation).__name__}; derive one such as self.statuses.latest_of_many()" + ) relation.__set_name__(owner, self.attribute) self.relations[owner] = relation return self.relations[owner] diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/__init__.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/__init__.py index 594f0a97..a99dc46f 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/__init__.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/__init__.py @@ -10,7 +10,7 @@ from .MorphOne import MorphOne from .MorphTo import MorphTo from .MorphToMany import MorphToMany -from .relationship import relationship +from .RelationshipMethod import relationship __all__ = [ "BaseRelationship", diff --git a/fastapi_startkit/tests/masoniteorm/mysql/builder/test_mysql_has_one_of_many_sql.py b/fastapi_startkit/tests/masoniteorm/mysql/builder/test_mysql_has_one_of_many_sql.py new file mode 100644 index 00000000..464ecf0a --- /dev/null +++ b/fastapi_startkit/tests/masoniteorm/mysql/builder/test_mysql_has_one_of_many_sql.py @@ -0,0 +1,30 @@ +from fastapi_startkit.masoniteorm import HasMany, HasOne, Model, relationship + +from ..test_case import TestCase + + +class MySQLOfManyStatus(Model): + __table__ = "statuses" + + +class MySQLOfManyUser(Model): + __table__ = "users" + + statuses = HasMany["MySQLOfManyStatus"](local_key="id", foreign_key="user_id") + + @relationship + def latest_status(self) -> HasOne["MySQLOfManyStatus"]: + return self.statuses.latest_of_many() + + +class TestMySQLHasOneOfManySql(TestCase): + async def test_eager_query_picks_one_row_per_parent(self): + sql = MySQLOfManyUser.latest_status.where_in("statuses.user_id", [1, 2]).to_sql() + + self.assertEqual( + sql, + "SELECT * FROM `statuses` WHERE `statuses`.`id` = (SELECT `statuses_of_many`.`id` FROM `statuses` AS " + "`statuses_of_many` WHERE statuses_of_many.user_id = statuses.user_id AND " + "`statuses_of_many`.`created_at` IS NOT NULL ORDER BY `statuses_of_many`.`created_at` DESC, " + "`statuses_of_many`.`id` DESC LIMIT 1) AND `statuses`.`user_id` IN ('1','2')", + ) diff --git a/fastapi_startkit/tests/masoniteorm/postgres/grammar/test_postgres_has_one_of_many_sql.py b/fastapi_startkit/tests/masoniteorm/postgres/grammar/test_postgres_has_one_of_many_sql.py new file mode 100644 index 00000000..c116ca1b --- /dev/null +++ b/fastapi_startkit/tests/masoniteorm/postgres/grammar/test_postgres_has_one_of_many_sql.py @@ -0,0 +1,40 @@ +from unittest import IsolatedAsyncioTestCase + +from fastapi_startkit.masoniteorm import HasMany, HasOne, Model, relationship + +from ..fixtures.db import DB + + +class PostgresOfManyStatus(Model): + __table__ = "statuses" + + +class PostgresOfManyUser(Model): + __table__ = "users" + + statuses = HasMany["PostgresOfManyStatus"](local_key="id", foreign_key="user_id") + + @relationship + def first_status(self) -> HasOne["PostgresOfManyStatus"]: + return self.statuses.oldest_of_many() + + +class TestPostgresHasOneOfManySql(IsolatedAsyncioTestCase): + async def asyncSetUp(self): + self.previous_db_manager = Model.db_manager + Model.db_manager = DB + + async def asyncTearDown(self): + Model.db_manager = self.previous_db_manager + + async def test_where_has_query_picks_the_oldest_non_null_row(self): + sql = PostgresOfManyUser.where_has("first_status", lambda q: q.where("name", "done")).to_sql() + + self.assertEqual( + sql, + 'SELECT * FROM "users" WHERE EXISTS (SELECT * FROM "statuses" WHERE "statuses"."id" = ' + '(SELECT "statuses_of_many"."id" FROM "statuses" AS "statuses_of_many" WHERE ' + 'statuses_of_many.user_id = statuses.user_id AND "statuses_of_many"."created_at" IS NOT NULL ' + 'ORDER BY "statuses_of_many"."created_at" ASC, "statuses_of_many"."id" ASC LIMIT 1) AND ' + 'statuses.user_id = users.id AND "statuses"."name" = \'done\')', + ) diff --git a/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py b/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py index 8b1e40d7..8d3be57b 100644 --- a/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py +++ b/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py @@ -52,7 +52,7 @@ async def asyncSetUp(self): table.integer("user_id") table.string("name") table.integer("priority") - table.timestamp("created_at") + table.timestamp("created_at", nullable=True) self.alice = await OfManyUser.create({"name": "Alice"}) self.bob = await OfManyUser.create({"name": "Bob"}) @@ -144,4 +144,161 @@ async def test_has_many_is_unchanged(self): async def test_unknown_aggregate_is_rejected(self): with pytest.raises(ValueError, match="aggregate"): - OfManyUser.statuses.of_many("priority", "avg") + OfManyUser.statuses.of_many("priority", "avg") # pyright: ignore[reportArgumentType] + + async def test_related_model_resolves_through_the_parent_relation(self): + self.assertIs(OfManyUser.due_date.related_model(), OfManyStatus) + + async def test_returning_an_existing_relation_is_rejected_and_leaves_it_intact(self): + class AliasedUser(Model): + __table__ = "of_many_users" + __timestamps__ = False + + statuses = HasMany["OfManyStatus"](local_key="id", foreign_key="user_id") + + @relationship + def items(self) -> HasMany["OfManyStatus"]: + return self.statuses + + @relationship + def done_statuses(self) -> HasMany["OfManyStatus"]: + return self.statuses.where("name", "done") + + with pytest.raises(TypeError, match="must return a new relationship"): + AliasedUser.items + with pytest.raises(TypeError, match="must return a new relationship"): + AliasedUser.done_statuses + + user = await AliasedUser.with_("statuses").find(self.alice.id) + self.assertEqual(len(user.statuses), 3) + + async def test_unnamed_has_many_without_foreign_key_is_rejected(self): + with pytest.raises(TypeError, match="explicit foreign_key"): + HasMany["OfManyStatus"]().latest_of_many() + + +class OfManyEvent(Model): + __table__ = "of_many_events" + __timestamps__ = False + + +class OfManyTicket(Model): + __table__ = "of_many_tickets" + __primary_key__ = "ticket_id" + __timestamps__ = False + + +class OfManyAccount(Model): + __table__ = "of_many_accounts" + __timestamps__ = False + + events = HasMany["OfManyEvent"](local_key="id", foreign_key="account_id") + tickets = HasMany["OfManyTicket"](local_key="id", foreign_key="account_id") + + @relationship + def latest_event(self) -> HasOne["OfManyEvent"]: + return self.events.latest_of_many() + + @relationship + def oldest_event(self) -> HasOne["OfManyEvent"]: + return self.events.oldest_of_many() + + @relationship + def newest_ticket(self) -> HasOne["OfManyTicket"]: + return self.tickets.of_many() + + @relationship + def first_ticket(self) -> HasOne["OfManyTicket"]: + return self.tickets.of_many(aggregate="min") + + +class OfManyPremiumAccount(OfManyAccount): + pass + + +class OfManyNode(Model): + __table__ = "of_many_nodes" + __timestamps__ = False + + children = HasMany["OfManyNode"](local_key="id", foreign_key="parent_id") + + @relationship + def newest_child(self) -> HasOne["OfManyNode"]: + return self.children.of_many() + + +class TestHasOneOfManyEdgeCases(TestCase): + async def asyncSetUp(self): + await super().asyncSetUp() + async with await self.schema.create_table_if_not_exists("of_many_accounts") as table: + table.id() + table.string("name") + async with await self.schema.create_table_if_not_exists("of_many_events") as table: + table.id() + table.integer("account_id") + table.string("name") + table.timestamp("created_at", nullable=True) + async with await self.schema.create_table_if_not_exists("of_many_tickets") as table: + table.increments("ticket_id") + table.integer("account_id") + table.string("name") + async with await self.schema.create_table_if_not_exists("of_many_nodes") as table: + table.id() + table.string("name") + table.integer("parent_id").nullable() + + self.acme = await OfManyAccount.create({"name": "Acme"}) + self.blank = await OfManyAccount.create({"name": "Blank"}) + events = [("dated-old", "2026-01-01 00:00:00"), ("null-ts", None), ("dated-new", "2026-02-01 00:00:00")] + for name, created_at in events: + await OfManyEvent.create({"account_id": self.acme.id, "name": name, "created_at": created_at}) + await OfManyEvent.create({"account_id": self.blank.id, "name": "only-null", "created_at": None}) + for name in ["ticket-1", "ticket-2"]: + await OfManyTicket.create({"account_id": self.acme.id, "name": name}) + + root = await OfManyNode.create({"name": "root"}) + await OfManyNode.create({"name": "child-1", "parent_id": root.id}) + await OfManyNode.create({"name": "child-2", "parent_id": root.id}) + + async def names_by_parent(self, model, relation): + parents = await model.with_(relation).order_by("id").get() + return {parent.name: getattr(getattr(parent, relation), "name", None) for parent in parents} + + async def test_null_columns_are_ignored_like_max_and_min(self): + self.assertEqual( + await self.names_by_parent(OfManyAccount, "latest_event"), {"Acme": "dated-new", "Blank": None} + ) + self.assertEqual( + await self.names_by_parent(OfManyAccount, "oldest_event"), {"Acme": "dated-old", "Blank": None} + ) + + async def test_of_many_defaults_to_the_related_primary_key(self): + self.assertEqual( + await self.names_by_parent(OfManyAccount, "newest_ticket"), {"Acme": "ticket-2", "Blank": None} + ) + self.assertEqual(await self.names_by_parent(OfManyAccount, "first_ticket"), {"Acme": "ticket-1", "Blank": None}) + + async def test_primary_key_column_is_ordered_once_without_a_null_filter(self): + sql = OfManyAccount.newest_ticket.to_sql() + + self.assertIn('ORDER BY "of_many_tickets_of_many"."ticket_id" DESC LIMIT 1', sql) + self.assertEqual(sql.count("ORDER BY"), 1) + self.assertNotIn("IS NOT NULL", sql) + + async def test_subclass_of_the_owner_resolves_its_own_relation(self): + self.assertEqual( + await self.names_by_parent(OfManyPremiumAccount, "latest_event"), {"Acme": "dated-new", "Blank": None} + ) + self.assertIsNot(OfManyPremiumAccount.latest_event, OfManyAccount.latest_event) + + async def test_self_referencing_relation_eager_and_where_has(self): + nodes = await OfManyNode.with_("newest_child").order_by("id").get() + with_child_two = await OfManyNode.where_has("newest_child", lambda q: q.where("name", "child-2")).get() + with_child_one = await OfManyNode.where_has("newest_child", lambda q: q.where("name", "child-1")).get() + + self.assertEqual( + {node.name: getattr(node.newest_child, "name", None) for node in nodes}, + {"root": "child-2", "child-1": None, "child-2": None}, + ) + self.assertEqual(with_child_two.pluck("name").all(), ["root"]) + self.assertTrue(with_child_one.is_empty()) From fcf0ff0e03097542c359cfa2f1f8d66cb7324d44 Mon Sep 17 00:00:00 2001 From: Bedram Tamang Date: Sat, 31 Oct 2026 02:37:42 -0700 Subject: [PATCH 2/2] Keep the v0.71.0 relationship import path and restore scope caveat - Re-export relationship and RelationshipMethod from relationships/relationship.py so the old module path imports again; the package imports through it so its attribute stays the function - Restore the ADR caveat about global scopes that hard-code the table name inside the correlated subquery - Chain where_column in one_of_many Co-Authored-By: Claude Opus 5.5 --- docs/adr/050-has-one-of-many.md | 5 +++-- docs/adr/AGENTS.md | 2 +- .../masoniteorm/relationships/HasOneOfMany.py | 7 +++++-- .../masoniteorm/relationships/__init__.py | 2 +- .../masoniteorm/relationships/relationship.py | 3 +++ .../sqlite/relationships/test_has_one_of_many.py | 9 +++++++++ 6 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/relationship.py diff --git a/docs/adr/050-has-one-of-many.md b/docs/adr/050-has-one-of-many.md index 1d72a441..97373f3c 100644 --- a/docs/adr/050-has-one-of-many.md +++ b/docs/adr/050-has-one-of-many.md @@ -28,7 +28,7 @@ The ORM had no way to express this. `HasOne` eager-loads every child with `where - `HasOneOfMany` overrides only `get_builder` and `get_exists_builder`. Both add `table.pk = (correlated subquery)`. The subquery orders by the column and then by the primary key in the same direction, so ties are deterministic (`max` → latest pk wins, `min` → earliest pk wins). When the column is the primary key, it is ordered once. The inherited `HasOne` eager (`where_in`), lazy (`load`), `where_has` and `register_related` code paths then work unchanged and always see at most one row per parent. - NULLs: the subquery adds `column IS NOT NULL`, which gives `MAX`/`MIN` semantics on every database (NULLs are ignored), instead of each database's own NULL sort position. Without it, PostgreSQL's `DESC` and SQLite/MySQL's `ASC` would put NULL rows first. A parent whose children all have a NULL column gets `None`. - `HasOneOfMany` runs `HasOne.__init__`, so `_model_name` and `fn` are set the usual way, and `related_model()` resolves through the parent relation. It raises `TypeError` when the parent `HasMany` has neither an explicit `foreign_key` nor an attribute name to derive one from, instead of silently using `None_id`. -- `relationship` (in `relationships/RelationshipMethod.py`, still exported as `fastapi_startkit.masoniteorm.relationship` and `fastapi_startkit.masoniteorm.relationships.relationship`) is a decorator that returns a `RelationshipMethod` descriptor. The module was renamed from `relationship.py` because the exported function shadowed it and its name was close to `models/relationship.py`. On first access from an owner class, the decorator calls the decorated function with the **class** as its argument, so `self.statuses` is the class-level `HasMany` descriptor, not a loaded collection. The result must be a new, unnamed `BaseRelationship`. Returning an existing descriptor (`return self.statuses`) or a builder raises `TypeError`, because naming it would rename the shared descriptor. The relation is named after the method and cached per owner class, so subclasses resolve their own. Instance access goes through the relation's normal `access` (loaded value or `LazyLoadingViolation`). Both the plain-function form and `@relationship @classmethod` are supported. +- `relationship` (in `relationships/RelationshipMethod.py`, still exported as `fastapi_startkit.masoniteorm.relationship` and `fastapi_startkit.masoniteorm.relationships.relationship`) is a decorator that returns a `RelationshipMethod` descriptor. The implementation moved out of `relationship.py` because the exported function shadowed that module and its name was close to `models/relationship.py`. `relationships/relationship.py` stays as a re-export so the v0.71.0 import path `fastapi_startkit.masoniteorm.relationships.relationship` keeps working, and the package imports through it so the package attribute stays bound to the function. On first access from an owner class, the decorator calls the decorated function with the **class** as its argument, so `self.statuses` is the class-level `HasMany` descriptor, not a loaded collection. The result must be a new, unnamed `BaseRelationship`. Returning an existing descriptor (`return self.statuses`) or a builder raises `TypeError`, because naming it would rename the shared descriptor. The relation is named after the method and cached per owner class, so subclasses resolve their own. Instance access goes through the relation's normal `access` (loaded value or `LazyLoadingViolation`). Both the plain-function form and `@relationship @classmethod` are supported. - Typing: `relationship(fn: Callable[[Any], R]) -> R`, so the method's return annotation decides the type. `User.due_date` is `HasOne[Status]` and `user.due_date` is `Status | None`. ## Caveats @@ -36,6 +36,7 @@ The ORM had no way to express this. `HasOne` eager-loads every child with `where - MySQL rejects writes through the relation, e.g. `User.due_date.where(...).update()` / `.delete()`, with error 1093 (the target table appears in a subquery). Reads are fine. Write through the `HasMany` or the related model instead. - A relationship method named `latest` or `oldest` overrides `Model.latest()`/`Model.oldest()` (basedpyright `reportIncompatibleMethodOverride`). Pick another name. - `where_column` emits unquoted identifiers (pre-existing behaviour), and now every of-many query uses it, so mixed-case table names on PostgreSQL would break. +- Global scopes on the related model also run on the correlated subquery's builder. Scopes that hard-code the table name instead of using the builder's alias are not handled. - `@classmethod @relationship` (reversed order) is not supported. - Performance relies on an index on `(fk, column, pk)`. @@ -55,7 +56,7 @@ The ORM had no way to express this. `HasOne` eager-loads every child with `where - `where_has` with and without a callback - the classmethod form and an unchanged `HasMany` - rejection of unknown aggregates, existing descriptors, builders and unresolvable foreign keys -- `related_model()` +- `related_model()` and the v0.71.0 import path - NULL columns ignored for latest and oldest - the `of_many()` default on a custom primary key, ordered once - a subclass of the owner diff --git a/docs/adr/AGENTS.md b/docs/adr/AGENTS.md index fabca106..e11e5a90 100644 --- a/docs/adr/AGENTS.md +++ b/docs/adr/AGENTS.md @@ -42,4 +42,4 @@ Read this index first, then open the records relevant to the change. Before impl | [042](042-fluent-validation-rules.md) | 2026-10-29 | Fluent validation rules with an async phase and a Laravel 422 shape | New `fastapi_startkit.validation`: `Field()`/`Email()` builders used as `Annotated` metadata on a `FormRequest` model (`required`, `nullable`, `string`, `integer`, `numeric`, `boolean`, `min`, `max`, `between`, `email`, `url`, `regex`, `in_`, `not_in`, `same_as`, `confirmed`, `required_if`, `password`, `unique`, `exists`, `add`, `msg`). A wildcard `field_validator` only records coerced values and the wrap `model_validator` fills missing `required` keys with `None`; it then runs sync rules per field in order with all values (so cross-field rules see later fields), skipping empty values for non-implicit rules and stopping at the first failure. `AsyncRule`s (`unique`/`exists` via the async `QueryBuilder`) run in an awaited second phase, `validate_async(context)`, triggered by the `Validated[Model]` FastAPI dependency with the route's typed path params as context (`ignore=lambda data: data["user_id"]`); an invalid path param skips the async phase so FastAPI returns its path 422 before any query; path params are collected by walking the public `Dependant` tree (no `get_flat_dependant`, removed in FastAPI 0.140) and cached per route. The dependency body key is the snake-cased model name (visible only when the endpoint has other body params). Rules on nested models raise `TypeError` at class definition. The existing unused `ValidationException` becomes a 422 `HttpException` carrying a `MessageBag`; `FastAPIProvider`'s existing `ValidationExceptionHandler` renders both phases as `{"message", "errors"}` (only the `message` key is new). | | [047](047-ai-bedrock-provider.md) | 2026-10-29 | AWS Bedrock provider for the AI module | Adds `provider="bedrock"` via LangChain's `bedrock_converse` (`ChatBedrockConverse`), the same `init_chat_model` path as other providers. `BedrockConfig` reads `AWS_REGION`/`AWS_DEFAULT_REGION`, IAM keys, session token, `AWS_PROFILE`, `AWS_BEARER_TOKEN_BEDROCK` and `AWS_BEDROCK_MODEL`. `Lab.get_client_options()` maps them to ChatBedrockConverse arguments and drops empty values so boto3's default credential chain applies; other providers return `{}`. The `bedrock` extra is `fastapi-startkit[ai]` plus `langchain-aws`, because `langchain-aws` does not pull in `langchain`. `langchain-aws` is imported only by `init_chat_model` when a Bedrock model is built. | | [048](048-async-first-validation.md) | 2026-10-30 | Async-first validation: one awaited rule pass, async hooks and closures | Supersedes ADR 042's two-phase design (sync rules in pydantic, `AsyncRule`s in `validate_async`). `await Model.validate_input(data, context=None, request=None)` is the only validation entry point: a per-call `Validator` runs `authorize` (false → 403 `AuthorizationException`), `prepare_for_validation`, a pydantic type-only parse (type errors added to the bag), then every field's rules in declaration and written order, awaiting a rule's result only when it is awaitable (sync rules unchanged, `unique`/`exists` in place via the async `QueryBuilder`), then `after`, and raises one `ValidationException`. Hooks may be sync or async classmethods; `Field().using(callback)` adds sync/async closure rules; `unique(ignore=...)` accepts async callables; `stop_on_first_failure` stops after the first error. `model_validate`/plain body params only parse types. `Validated[Model]` takes the raw body (`RawInput` keeps the OpenAPI `$ref` via `json_schema_input_schema`) and awaits `validate_input` with the typed path params, so type and rule errors share one 422. Several `Validated` params on one route are all validated: each records its errors in the request scope (prefixed with its embed key, e.g. `shipping.name`, when the body is embedded) and the route's last `Validated` dependency raises them together; `ValidationExceptionHandler` merges recorded errors into a `RequestValidationError` too. With `FastAPIProvider`, every 422 (also plain `RequestValidationError`) is `{"message", "errors"}`, a superset of `main`'s `{"errors"}`. `validate_async` is removed. | -| [050](050-has-one-of-many.md) | 2026-10-31 | Has-one-of-many derived from a HasMany | `HasMany.latest_of_many(col="created_at")`, `oldest_of_many(col)` and `of_many(col=None → related pk, Literal["max","min"])` return a `HasOneOfMany` (`HasOne` subclass reusing the parent keys; `TypeError` if the fk cannot be resolved). Its `get_builder`/`get_exists_builder` add `pk = (SELECT pk … WHERE fk = outer.fk AND col IS NOT NULL ORDER BY col, pk LIMIT 1)`, so eager `with_`, lazy `load` and `where_has` pick one row per parent in SQL with a pk tie-break, NULLs ignored like `MAX`/`MIN`, and no window functions or `join_sub`. `@relationship` (module `RelationshipMethod.py`) calls the method with the owner class (so `self.statuses` is the descriptor), requires a new unnamed relation (`TypeError` otherwise), names and caches it per class and types it by the return annotation; `@relationship @classmethod` is the basedpyright-clean form. MySQL error 1093 blocks writes through the relation. Canonical form, default column and inner constraints are open. | +| [050](050-has-one-of-many.md) | 2026-10-31 | Has-one-of-many derived from a HasMany | `HasMany.latest_of_many(col="created_at")`, `oldest_of_many(col)` and `of_many(col=None → related pk, Literal["max","min"])` return a `HasOneOfMany` (`HasOne` subclass reusing the parent keys; `TypeError` if the fk cannot be resolved). Its `get_builder`/`get_exists_builder` add `pk = (SELECT pk … WHERE fk = outer.fk AND col IS NOT NULL ORDER BY col, pk LIMIT 1)`, so eager `with_`, lazy `load` and `where_has` pick one row per parent in SQL with a pk tie-break, NULLs ignored like `MAX`/`MIN`, and no window functions or `join_sub`. `@relationship` (module `RelationshipMethod.py`, with `relationship.py` kept as a v0.71.0 re-export) calls the method with the owner class (so `self.statuses` is the descriptor), requires a new unnamed relation (`TypeError` otherwise), names and caches it per class and types it by the return annotation; `@relationship @classmethod` is the basedpyright-clean form. MySQL error 1093 blocks writes through the relation. Canonical form, default column and inner constraints are open. | diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasOneOfMany.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasOneOfMany.py index 20da1af9..da9b5231 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasOneOfMany.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/HasOneOfMany.py @@ -47,8 +47,11 @@ def one_of_many(self, table): alias = f"{builder.get_table_name()}_of_many" key = builder._model.__primary_key__ column = self.column or key - query = builder.table(builder.get_table_name(), alias).select(f"{alias}.{key}") - query.where_column(f"{alias}.{self.foreign_key}", f"{table}.{self.foreign_key}") + query = ( + builder.table(builder.get_table_name(), alias) + .select(f"{alias}.{key}") + .where_column(f"{alias}.{self.foreign_key}", f"{table}.{self.foreign_key}") + ) if column != key: query.where_not_null(f"{alias}.{column}").order_by(f"{alias}.{column}", self.direction) return query.order_by(f"{alias}.{key}", self.direction).limit(1) diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/__init__.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/__init__.py index a99dc46f..594f0a97 100644 --- a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/__init__.py +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/__init__.py @@ -10,7 +10,7 @@ from .MorphOne import MorphOne from .MorphTo import MorphTo from .MorphToMany import MorphToMany -from .RelationshipMethod import relationship +from .relationship import relationship __all__ = [ "BaseRelationship", diff --git a/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/relationship.py b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/relationship.py new file mode 100644 index 00000000..d1559688 --- /dev/null +++ b/fastapi_startkit/src/fastapi_startkit/masoniteorm/relationships/relationship.py @@ -0,0 +1,3 @@ +from .RelationshipMethod import RelationshipMethod, relationship + +__all__ = ["RelationshipMethod", "relationship"] diff --git a/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py b/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py index 8d3be57b..3aab4ccf 100644 --- a/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py +++ b/fastapi_startkit/tests/masoniteorm/sqlite/relationships/test_has_one_of_many.py @@ -172,6 +172,15 @@ def done_statuses(self) -> HasMany["OfManyStatus"]: user = await AliasedUser.with_("statuses").find(self.alice.id) self.assertEqual(len(user.statuses), 3) + async def test_previous_module_path_still_imports_the_decorator(self): + from fastapi_startkit.masoniteorm import relationships + from fastapi_startkit.masoniteorm.relationships.relationship import RelationshipMethod + from fastapi_startkit.masoniteorm.relationships.relationship import relationship as legacy + + self.assertIs(legacy, relationship) + self.assertIs(relationships.relationship, relationship) + self.assertIsInstance(vars(OfManyUser)["due_date"], RelationshipMethod) + async def test_unnamed_has_many_without_foreign_key_is_rejected(self): with pytest.raises(TypeError, match="explicit foreign_key"): HasMany["OfManyStatus"]().latest_of_many()