Repository navigation
Add Laravel-style fluent validation module - #326
Conversation
Adds fastapi_startkit.validation: Field()/Email() builders used as Annotated metadata on FormRequest models, sync rules run after pydantic validation with full model data (cross-field rules work regardless of declaration order, empty values skip non-implicit rules, first failure per field stops), AsyncRule (unique/exists via the async QueryBuilder) run in an awaited phase through the Validated[Model] dependency with path params as context, and a MessageBag rendered as the Laravel 422 shape by the existing ValidationExceptionHandler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-tmgbedu
left a comment
There was a problem hiding this comment.
Verdict: changes requested (one blocker, five should-fix items; the rest is solid)
Blocker
-
form_request.py:125:Validatedpasses raw string path params into the async rules.request.path_paramsholds unvalidated strings, so the update example in ADR 042,unique("users", ignore=lambda data: data["user_id"]), binds'1'against an integerid. I checked this against a real Postgres (asyncpg):context={"user_id": 1}passescontext={"user_id": "1"}(whatValidatedpasses) fails withDBAPIError ... DataError: invalid input for query argument $2: '1' ('str' object cannot be interpreted as an integer), which is a 500
SQLite hides this through type affinity, and the FastAPI test uses a fake
AsyncRulethat comparesstr(...), so CI cannot catch it. A related problem: FastAPI runs sub-dependencies before it validates the endpoint's own path params. SoPUT /users/abc(withuser_id: int) queries the database with'abc'and returns422 {"email": ["The email has already been taken."]}on SQLite (a 500 on Postgres) instead of theuser_idint error. Possible fixes: have the dependency take the route's validated path values instead ofrequest.path_params, or at least change the ADR/example to coerce (int(data["user_id"])). Either way, add a real-uniqueroute test with an intignore.
Should fix
form_request.py:124: the dependency's body parameter is namedpayload. When the endpoint has a second body param, FastAPI embeds the body under"payload", not under the endpoint's parameter name. Withasync def two(data: Validated[Item], note: Annotated[str, Body()]), sending{"data": {...}, "note": ...}givesloc: ["body", "payload"]"Field required", and only{"payload": {...}}works. OpenAPI showspayloadtoo. Document this or find a way to keep the user's name.message_bag.py:11/exceptions.py:225: a backward-compat trap. The oldValidationException(errors: dict)was typically called as{"field": "msg"}. NowValidationException({"email": "Taken."}).errors == {"email": ["T","a","k","e","n","."]}. Accept astrvalue as a single message.field.py:160: rules on a nested plainBaseModelinside aFormRequestare silently skipped.coerced_valuesis already set by the outer model, soInner.code: Annotated[str, Field().required().min(5)]accepts"ab". The ADR says plain-BaseModeluse raisesTypeErrorso that rules never silently fail to run. This case gets around that guarantee. Either raise here too or document nested rules as unsupported (the ADR only lists them as a follow-up).rules.py:241:in_({"a","b"})on a list or dict value raisesTypeError: unhashable type(a 500). CatchTypeErrorand returnFalse.- Missing tests: a real-DB
uniquewith a path-paramignorethroughValidated(see 1), an invalid path param combined with an async rule, and a multi-body-param endpoint.
Nits / notes
- OpenAPI:
requiredfields are advertised asanyOf [string, null](they stay inrequired, which is good, but generated clients will type them as nullable). required_if("is_business", True)renders "...when is business is True." (Laravel renderstrue). Consider lowercasing bools in:value.numeric().max(10)on astrfield measures length ("25"passes). Laravel treats it as a number whennumericis present. Document this or match Laravel.- Non-mapping input also runs rules on the defaults, which adds a spurious
name"required" error next to pydantic's "valid dictionary" error. - Combined errors are ordered with pydantic type errors first, then rule errors, not in declaration order. Harmless.
Review focus results
- Rules / messages / bail / empty-skip /
msg()/ callable messages: correct in all my tests (whitespace counts as empty forrequired;{braces}in messages are kept literally). - Cross-field rules:
same_as,confirmedandrequired_ifwork regardless of declaration order. - Async phase: properly awaited with no nested loops. Values are bound as parameters, and table/column names come from the developer. When the sync phase fails, async rules are skipped (documented in the ADR and confirmed over HTTP).
- FastAPI: the OpenAPI body schema and
requiredlist are kept. JSON, Inertia (303) and plain-form (303) negotiation work for both phases. A plaindata: Modelruns the sync phase only (a duplicate email passes, as documented). - Breaking change:
exceptions.ValidationExceptionhas no users insrc/,example/,application/or the docs, so repurposing it is safe apart from item 3. The addedmessagekey on 422s is additive (only exact-body assertions break). Fine for a minor release with a changelog note. - Typing: basedpyright reports 0 errors on the module and on a user-style model and endpoint (
Validated[Model]resolves to the model type). - Repo rules: the ADR and index entry are present, there are no new explanatory comments, and the module has 100% coverage.
Test results
uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3555 passed, 10 skipped, total coverage 92%,validation/*100%ruff check,ruff format --check,basedpyright src/fastapi_startkit/validation: all clean
QA (uvicorn + SQLite, real HTTP)
valid register 200 {"ok":true,"username":"john"}
duplicate email 422 {"message":"The email has already been taken.","errors":{"email":["The email has already been taken."]}}
empty {} 422 {"message":"The email field is required. (and 3 more errors)","errors":{"email":["The email field is required."],"username":["The username field is required."],"password":["The password field is required."],"password_confirmation":["The password confirmation field is required."]}}
username "ab" 422 {"message":"The username field must be at least 3 characters.", ...}
username 11 chars 422 {"message":"The username field must not be greater than 10 characters.", ...}
short pw + mismatch 422 {"message":"The password field must be at least 8 characters. (and 1 more error)","errors":{"password":[...],"password_confirmation":["The password confirmation field must match password."]}}
is_business w/o tax_id 422 {"message":"The tax id field is required when is business is True.", ...}
is_business + tax_id 200
type errors + rule errors 422 {"message":"Input should be a valid string (and 4 more errors)","errors":{"username":["Input should be a valid string"],"is_business":["Input should be a valid boolean, unable to interpret input"],"email":[...],"password":[...],"password_confirmation":[...]}}
sync fail + dup email 422 only username error (async skipped)
PUT /users/1 own email 200
PUT /users/1 bob's email 422 {"message":"The email has already been taken.", ...}
PUT /users/abc 422 email taken <- should be the user_id int error (blocker 1)
Inertia sync / async fail 303 -> Referer
| from fastapi import Request | ||
|
|
||
| async def dependency(request: Request, payload: Any) -> Any: | ||
| return await payload.validate_async(request.path_params) |
There was a problem hiding this comment.
Blocker: request.path_params holds raw strings. On Postgres, the ADR's unique("users", ignore=lambda data: data["user_id"]) fails with asyncpg DataError: invalid input for query argument $2: '1' ('str' object cannot be interpreted as an integer), which is a 500. This dependency also runs before FastAPI validates the endpoint's user_id: int, so PUT /users/abc queries the DB with 'abc' and returns an email error instead of the path-param error.
There was a problem hiding this comment.
Fixed in 0f81e67. Validated now reads the matched APIRoute from request.scope['route'] and converts the raw path values with FastAPI's own request_params_to_args over every declared path param (get_flat_dependant(route.dependant).path_params). So data['user_id'] is the typed int. If any declared path param is invalid, the dependency returns the model without running the async phase. FastAPI then reports its normal path 422 and no query runs. Regression tests: a real unique(..., ignore=lambda d: d['account_id']) through Validated on SQLite asserts the bound query parameter is int and that /accounts/abc never reaches the DB (tests/validation/test_validated_database.py). The same route runs on Postgres/asyncpg in tests/masoniteorm/postgres/test_validated_unique.py. It reproduced your DataError on the old code and passes now. ADR 042 documents this.
| def validated_dependency(model: type[FormRequest]) -> Any: | ||
| from fastapi import Request | ||
|
|
||
| async def dependency(request: Request, payload: Any) -> Any: |
There was a problem hiding this comment.
If the endpoint has another body param, FastAPI embeds this body under "payload" (not the endpoint's data): {"data": {...}} gives loc ["body","payload"] Field required.
There was a problem hiding this comment.
Addressed in 0f81e67. FastAPI builds a dependency's signature without the name of the endpoint param that declares it, so the endpoint's data name can't be reused. The body key is now the snake-cased model name (UpdateUser -> update_user) instead of a fixed payload. That keeps it predictable and lets two different Validated models on one endpoint coexist. A single body param is still not embedded. Test: test_validated_body_is_embedded_under_the_model_name_with_other_body_params (request body plus OpenAPI keys). ADR 042 documents the key and the escape hatch: data: Model + await data.validate_async(context).
| def __init__(self, messages: Mapping[str, Iterable[str]] | None = None) -> None: | ||
| self.messages: dict[str, list[str]] = {} | ||
| for field, field_messages in (messages or {}).items(): | ||
| for message in field_messages: |
There was a problem hiding this comment.
ValidationException({"email": "Taken."}) (the old call shape) gives {"email": ["T","a","k","e","n","."]}. Treat a str as one message.
There was a problem hiding this comment.
Fixed in 0f81e67. MessageBag treats a str value as one message, so ValidationException({'email': 'Taken.'}).errors == {'email': ['Taken.']}. Covered by test_string_messages_are_single_messages.
| return schema | ||
|
|
||
| @staticmethod | ||
| def require_form_request(value: Any) -> Any: |
There was a problem hiding this comment.
A nested plain BaseModel with Field rules inside a FormRequest passes this check, because the outer model set the ContextVar, so its rules silently never run (min(5) accepts "ab"). That contradicts the ADR's no-silent-skip guarantee.
There was a problem hiding this comment.
Fixed in 0f81e67. FormRequest.__pydantic_init_subclass__ raises TypeError at class definition when a field's annotation (direct, list[...], X | None, other generic args) contains a model with Field rules. Rules can no longer silently skip. Nested dotted-key rules stay a follow-up (ADR 042 updated). Tests: test_rules_on_nested_models_raise_at_class_definition, test_nested_models_without_rules_are_allowed.
| self.values = values | ||
|
|
||
| def passes(self, attribute: str, value: Any, data: Mapping[str, Any]) -> bool: | ||
| return value in self.values |
There was a problem hiding this comment.
An unhashable value (list/dict) with set values raises TypeError, which is a 500. Catch it and return False.
There was a problem hiding this comment.
Fixed in 0f81e67. In.contains catches TypeError: in_ fails validation and not_in passes for unhashable values. Covered in the rule matrix and in test_in_rule_rejects_unhashable_values (a 422 message, not a 500).
| self.expected = expected | ||
|
|
||
| def replacements(self) -> Mapping[str, Any]: | ||
| return {"other": humanize(self.other), "value": self.expected} |
There was a problem hiding this comment.
Nit: renders is True / is False. Laravel renders true/false.
There was a problem hiding this comment.
Fixed in 0f81e67. Booleans in :value render lowercase: '... when is business is true.' Covered by test_required_if_renders_booleans_in_lowercase.
… rules - Validated converts declared path params with FastAPI's own param validation before the async phase; invalid path params skip it so FastAPI returns its path 422 and no query runs. - Dependency body key is the snake-cased model name. - MessageBag treats a string value as one message. - Field rules on nested models raise TypeError at class definition. - in_/not_in treat unhashable values as not in the collection. - Required fields keep a plain JSON schema type, booleans render in lowercase, size rules compare numeric values with numeric/integer, and non-mapping input reports only the type error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude-tmgbedu
left a comment
There was a problem hiding this comment.
Verdict: changes requested. One new blocker. Everything else from the previous review is fixed and verified, over real HTTP on both SQLite and Postgres.
Blocker
-
form_request.py:148:get_flat_dependantno longer exists in current FastAPI, so everyValidatedrequest returns 500. The package declaresfastapi[standard]>=0.124.4with no upper bound. A fresh install therefore resolves to FastAPI 0.162.0, where the import raises:ImportError: cannot import name 'get_flat_dependant' from 'fastapi.dependencies.utils'Because the import is lazy inside the dependency, this happens at request time and gives a 500 on every
Validatedendpoint. I bisected it: the function is present in 0.124.4 through 0.140.0 and gone in 0.140.13, 0.141.x, 0.145, 0.150, 0.155, 0.160 and 0.162.uv run --with fastapi==0.162.0 pytest tests/validationgives 8 failed (everyValidatedHTTP test). CI only exercises the locked 0.124.4, so it is green.Suggested fix (I verified it on both 0.124.4 and 0.162.0): drop
get_flat_dependantand collect the path params from the publicDependantdataclass fields, which exist in every supported version:def declared_path_params(dependant: Dependant) -> list[ModelField]: return [*dependant.path_params, *(p for sub in dependant.dependencies for p in declared_path_params(sub))]
request_params_to_args(fields, received)is also internal, but it still has the same signature and behaviour in 0.162.0. With this walker, my probe app gives identical results on both versions: typed ints for router-prefix params, for params declared only independencies=[Depends(...)]and forPath(gt=0)constraints, plus the path 422 with no async phase when any of them is invalid.
Please also add a guard so this cannot regress silently, for example a CI job (or a nox/uv step) that runstests/validationagainst the latest FastAPI. Alternatively, cap the upper bound until the code is verified on a newer version.
Previous items: verified
- Typed path context (old blocker): fixed on 0.124.4.
PUT /users/abcreturns FastAPI's path 422 with zero queries (checked with a query logger), andignorebinds anint. The PR'stests/masoniteorm/postgres/test_validated_unique.pypasses against a throwawaypostgres:16. Also checked:APIRouter(prefix="/teams/{team_id}")gives both params as ints, and/teams/x/...returns the path 422 without the async phase.- The
fastapi_startkit.fastapi.Routerwrapper works. - A route-level
dependencies=[Depends(dep(tenant_id: int))]givestenant_idas an int, and an invalid value returns 422. Path(gt=0)with0returns 422 without the async phase.- A mounted sub-app (
app.mount("/orgs/{org_id}", inner)) gives the inner route's params typed, and the outer mount param stays a rawstr. This is expected, since the inner app doesn't declare it. - An undeclared path param stays a raw
str, and no matchedAPIRoutefalls back to raw params.
- Embed key: acceptable. A single-body endpoint still takes the plain body (
{"email": ...}gives 200, and the OpenAPI$refpoints straight atUpdateUser). With a second body param, the body is embedded underupdate_user, and{"data": ...}gives a clearloc ["body","update_user"] Field required. FastAPI gives a dependency no access to the declaring param's name, so a stable, model-derived key is the most predictable option, and the ADR documents the escape hatch. Two edge cases are worth guarding or documenting (non-blocking):- A model class named
Request(keyrequest) raisesValueError: duplicate parameter name: 'request'at route definition, because it collides with the dependency's ownrequestparam. Rename that param to something unlikely, or raise a clear error. - An endpoint body param literally named
update_usersilently shares the key withValidated[UpdateUser], and the request fails with a confusing combined 422.
- A model class named
- MessageBag
str:ValidationException({"email": "Taken."}).errors == {"email": ["Taken."]}. Verified. - Nested rules: a
TypeErroris raised at class definition, including throughdict[str, list[Inner]]. Verified. in_unhashable: gives a 422 rule message, not a 500. Verified.- Nits: all verified. The plain
{"type": "string"}schema is shown, booleans render lowercase (...is true.),numeric().max(10)rejects"25", and non-mapping input gives only themodel_typeerror.
Notes (non-blocking)
typed_path_paramsrecomputes the flattened dependant on every request. It is cheap, but the field list could be cached per route (for example in aWeakKeyDictionarykeyed byroute).
Test results (0f81e67, FastAPI 0.124.4 from the lockfile)
uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3570 passed, 10 skipped, coverage 92.41%ruff check,ruff format --checkandbasedpyright src/fastapi_startkit/validation: cleantests/masoniteorm/postgres/test_validated_unique.pyon postgres:16: 2 passedtests/validationon FastAPI 0.162.0: 8 failed (blocker 1)
QA: uvicorn over real HTTP, SQLite and Postgres 16 (asyncpg) gave identical results
valid register 200 {"ok":true,"username":"john"}
duplicate email 422 {"message":"The email has already been taken.","errors":{"email":["The email has already been taken."]}}
empty {} 422 {"message":"The username field is required. (and 3 more errors)", ...}
is_business w/o tax_id 422 {"message":"The tax id field is required when is business is true.", ...}
sync fail + dup email 422 only username min error; queries=[]
PUT /users/1 own email 200 {"id":1,"email":"jane@example.com"} queries=[[... "id" != ?, ["jane@example.com", 1]]]
PUT /users/1 bob's email 422 {"message":"The email has already been taken.","errors":{"email":["The email has already been taken."]}}
PUT /users/abc 422 {"message":"Input should be a valid integer, unable to parse string as an integer","errors":{"user_id":[...]}} queries=[]
PUT /users/1 "nope" 422 {"message":"The email field must be a valid email address.", ...} queries=[]
PUT /users/1 email=["a"] 422 {"message":"Input should be a valid string", ...} queries=[]
get_flat_dependant was removed in FastAPI 0.140, so every Validated request raised ImportError on a fresh install. Walk the public Dependant tree instead, cache the result per route, rename the dependency's request parameter so a model named Request does not collide, and run the validation and FastAPI tests against the latest FastAPI in CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Blocker fixed in 55625cb.
Results:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
claude-tmgbedu
left a comment
There was a problem hiding this comment.
Verdict: APPROVE (posted as COMMENT because this is an own-PR). Re-review of ffe64582 after the FastAPI-compat fix.
What I checked
get_flat_dependantis gone.declared_path_paramswalks the publicDependant.path_paramsand.dependenciesrecursively. On the locked FastAPI 0.124.4,tests/validation+tests/fastapigive 294 passed. In a separate venv withuv pip install --upgrade fastapi(0.162.0), the same suites also give 294 passed.- Per-route cache (
id(route)+weakref.finalize).APIRoute.__hash__ is None(it inherits Starlette's__eq__), soWeakKeyDictionarycannot be used and an id key is needed. The finalizer runs when the route is deallocated, before its memory (and so its id) can be reused, so a stale entry cannot be served to a new route. I probed this outside the PR's tests:- Recreated apps: 50 apps built and GC'd in a loop. Each request saw its own typed
uid, and the cache never held more than 1 entry, so there is no leak. - Mounted sub-app with
Path(gt=0):5gives typed context.0gives a 422 and the async phase never runs. dependency_overrides: an overridden sub-dependency that declares the path param works (200, typed context).
- Recreated apps: 50 apps built and GC'd in a loop. Each request saw its own typed
__validated_request. Name mangling only applies inside class bodies, and this is a nested function at module level that builds aninspect.Signatureand looks the value up by string key, so it is not mangled. FastAPI injects the request by annotation (Request), not by name. The new test shows aFormRequestnamedRequestworks.- CI. In run 40851309238 (head
ffe64582), every Python job (3.12, 3.13, 3.14, 3.15) logs- fastapi==0.124.4 / + fastapi==0.162.0, printsFastAPI 0.162.0, and then shows294 passed. Because--no-syncis used, the upgrade is not reverted. env="testing"removal. All threemake_applicationcalls run inside the test (in the lazy_applicationsetup), wherePYTEST_CURRENT_TESTis set, soEnvironment.resolve_environmentreturnstesting. I confirmedApplication(...).env == "testing"andis_testing()without the argument.- ADR 042 and
docs/adr/AGENTS.md. Both are updated with the rationale for droppingget_flat_dependant, the cache design, the__validated_requestnaming, and the CI step. No explanatory comments or docstrings invalidation/. The full suite (excluding postgres) gives 3572 passed with 92% total coverage, andvalidation/*is at 100%. Ruff, Basedpyright and codecov/patch are green.
Non-blocking notes
declared_path_paramscan return the same param twice when a router prefix param is also declared on the endpoint. This is harmless becauserequest_params_to_argswrites the same value again.- With
dependency_overrides, path-param types come from the original route'sDependant, not from the override. In practice they match, and this is fine as is.
Not merging.
Per-module CI steps do not scale; drop the step and the ADR 042 claim that CI runs the validation tests against the latest FastAPI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # docs/adr/AGENTS.md
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
validate_input runs authorize, prepare_for_validation, a type-only pydantic parse, every rule in written order (awaiting async rules and closures in place) and after hooks, then raises one ValidationException. Validated takes the raw body so type and rule errors share one 422, and validate_async is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…k/validation-async # Conflicts: # docs/adr/AGENTS.md
|
QA (task #3714): bug with multiple When an endpoint takes two
Repro: class Billing(FormRequest):
name: Annotated[str, Field().required().min(3)]
class Shipping(FormRequest):
name: Annotated[str, Field().required().min(5)]
@app.fastapi.post("/checkout")
async def checkout(billing: Validated[Billing], shipping: Validated[Shipping]): ...
The cause is in Everything else passed in a scratch FastAPI app on sqlite: combined type + rule + unique/exists + sync/async closure 422, async authorize → 403 (before type parsing), async prepare/after, rule order, Note: the 422 body is now |
Each Validated dependency records its errors in the request scope, prefixed with its embed key when the body is embedded, and the route's last Validated dependency raises them together. ValidationExceptionHandler merges recorded errors into RequestValidationError responses. ADR 048 now documents the {message, errors} 422 body as a superset of main's.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Adds
fastapi_startkit.validation, a Laravel-style fluent validation module (ADR 042).Failing requests return 422 with
{"message": "<first> (and N more errors)", "errors": {"field": ["..."]}}.required,nullable,string,integer,numeric,boolean,min,max,between,password,email,url,regex,in_,not_in,same_as,confirmed,required_if,unique,exists, plusadd(rule)andmsg(message). Custom rules subclassRule(sync) orAsyncRule.min/max/betweenchange wording for strings, numbers and lists). A custom message can be a string with:attribute/:min/:max/:other/:valueplaceholders, or a callable(field, value, data). The builder takes it asmsg=.FormRequest's wrapmodel_validatorafter pydantic has validated every field, with the whole model's data. Cross-field rules therefore see fields declared after them. Empty values skip non-implicit rules, the first failure stops the field (bail), and fields with type errors run no rules. Type errors and rule errors come back together in oneValidationError.unique/existsand customAsyncRules run inawait model.validate_async(context)using the asyncQueryBuilder(a table name goes throughDB.table, aModelclass throughModel.query()).Validated[Model]runs this phase after the body has parsed and passes the route's path parameters as context, sounique("users", ignore=lambda d: d["user_id"])works on update routes. Outside HTTP, useawait Model.validate_input(data).exceptions.ValidationExceptionnow subclassesHttpException(422) and carries aMessageBag.FastAPIProvidersends it to the sameValidationExceptionHandlerasRequestValidationError, so both phases get the same JSON/Inertia/session handling. For JSON clients the only change is a newmessagekey next toerrors. FastAPI's default handler, and apps that don't useFastAPIProvider, are not changed.Follow-ups (not in this PR): docs page, nested/list rules, combining with the form-encoded
RequestModel, more rules (date, array, sometimes, ...).Test plan
uv run pytest --ignore=tests/masoniteorm/postgres --ignore=tests/masoniteorm/mysql --cov: 3515 passed, total coverage 92.25%,fastapi_startkit/validationat 100%tests/validation/: each rule passing and failing, default/str/callable messages, bail and empty-value skipping, cross-field rules in any declaration order, unique/exists against SQLite through the async ORM (including ignore), and the 422 shape through a real FastAPIProvider appruff check,ruff format --checkbasedpyright: no new errors (the one remaining error is the existing unresolved optionallangchainimport)🤖 Generated with Claude Code