Skip to content

feat(state): constraint-aware 422 for denormalization errors#8211

Open
soyuka wants to merge 1 commit into
api-platform:mainfrom
soyuka:feat/denormalization-422-issue-7981
Open

feat(state): constraint-aware 422 for denormalization errors#8211
soyuka wants to merge 1 commit into
api-platform:mainfrom
soyuka:feat/denormalization-422-issue-7981

Conversation

@soyuka
Copy link
Copy Markdown
Member

@soyuka soyuka commented May 29, 2026

Summary

Closes #7981. Translates PHP denormalization type errors (NotNormalizableValueException) into 422 ValidationException responses only when the target property has a matching validation constraint. Otherwise rethrows the original exception → honest 400.

RFC 9110 §15.5.21: "syntax of the request content is correct, thus 400 is inappropriate" — JSON parses, type-mismatch is 422 territory. This brings API Platform in line with FastAPI/Laravel/DRF behavior for properties the user explicitly opted into validation on, without silently inventing constraints elsewhere.

What changed

New service ApiPlatform\Validator\DenormalizationViolationFactory resolves the target property's constraints via Symfony's validator metadata factory and emits a typed ConstraintViolation per the rule table:

Exception current type Matching constraint Emitted violation
null NotBlank NotBlank::IS_BLANK_ERROR + constraint message
null NotNull NotNull::IS_NULL_ERROR + constraint message
any wrong type Type Type::INVALID_TYPE_ERROR + constraint message
any wrong type any other constraint generic Type violation @ 422
any wrong type no constraint null → caller rethrows → 400

DeserializeProvider accepts the factory as a new nullable 5th constructor argument (BC). The single-error catch consults the factory and rethrows on null match; the collect-mode catch falls back to the existing generic Type violation when the factory returns null, preserving prior collect-mode semantics.

The isBackedEnumException() special case (introduced in 4.3.6 / 44bb18d as a BC-breaking workaround for #8183) is removed. The new rule subsumes it: enum properties with Assert\NotNull, Assert\Type(EnumClass), or Assert\Choice still produce 422.

BC impact

Scenario Before After
'abc'float, no constraint 400 400 (unchanged)
'abc'float, w/ Assert\Type('numeric') 400 422 ✓ new
null → required, w/ Assert\NotBlank 400 type err 422 NotBlankcloses #7981
enum, no constraint, no auto_mapping 422 (4.3.6) 400 ⚠ regression
enum, no constraint, w/ auto_mapping enabled 422 (4.3.6) 422 ✓
enum, w/ Assert\NotNull / Assert\Type / Assert\Choice 422 422 ✓
collect mode, mix of constrained + unconstrained props 422 generic 422 specific where constraint exists, generic elsewhere

The enum regression is the only one. Mitigation:

  • Add an explicit Assert\Type(EnumClass) or Assert\Choice constraint on the enum property
  • Or enable Symfony's framework.validation.auto_mapping.pathsPropertyInfoLoader auto-adds Type(EnumClass) constraints

Justified because the 4.3.6 commit itself was a silent BC break (400 → 422 unconditionally for enum properties); the constraint-aware rule is more principled and respects explicit user opt-in.

Laravel

Intentionally untouched in this PR. Laravel uses Illuminate\Validation rules, not Symfony validator metadata, so the Symfony-validator-based factory cannot apply. A LaravelDenormalizationViolationFactory reading Eloquent/request rules is tracked as a follow-up.

Tests

  • src/Validator/Tests/DenormalizationViolationFactoryTest.php — 9 unit tests covering each rule row, group filtering, unknown class/property, and nested-path rejection (top-level only in v1)
  • tests/Functional/DenormalizationValidationTest.php — 5 functional tests covering null+NotBlank, null+NotNull, wrong-type+Type, no-constraint→400, and collect-mode mix
  • Existing tests/Functional/EnumDenormalizationValidationTest.php (introduced by Validation groups are skipped when deserializing invalid PHP enums (BackedEnumNormalizer throws before validation) #8183) passes unchanged — NotNull on the enum prop triggers the "any wrong type + any other constraint" rule → 422

Out of scope (follow-ups)

  • Nested path resolution (address.street, items[0].brand) — v1 returns null for nested paths, caller rethrows
  • Laravel parity via Illuminate\Validation rules
  • docs/symfony/validation.md update + CHANGELOG.md entry
  • Auto-constraints from PHP type-hints inside API Platform (defer; rely on Symfony's PropertyInfoLoader)
  • GraphQL deserialization error path

Test plan

  • vendor/bin/phpunit tests/Functional/DenormalizationValidationTest.php — 5/5 green
  • vendor/bin/phpunit tests/Functional/EnumDenormalizationValidationTest.php — 2/2 green
  • cd src/State && ./vendor/bin/phpunit Tests/Provider/DeserializeProviderTest.php — 13/13 green
  • cd src/Validator && ./vendor/bin/phpunit Tests/DenormalizationViolationFactoryTest.php — 9/9 green
  • vendor/bin/phpunit tests/Functional/ --filter "Validation |Denormalization|BackedEnum" — 122/124 green (2 unrelated McpTest failures from vendor/mcp/sdk signature mismatch — not touched by this PR)

Refs #8183

@soyuka soyuka force-pushed the feat/denormalization-422-issue-7981 branch from afe00da to eca83f8 Compare May 29, 2026 14:01
{
public const IS_BLANK_ERROR = 'c1051bb4-d103-4f74-8988-acbcafc7fdc3';
public const IS_NULL_ERROR = 'ad32d13f-c3d4-423b-909a-857b961eb720';
public const INVALID_TYPE_ERROR = 'ba785a8c-82cb-4283-967c-3cf342181b40';
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why uuids? we could have something more explicit especially if hard-coded

return null;
}

if (str_contains($path, '.') || str_contains($path, '[')) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not liking this we have ways to do this with the property accessor no?

$hasNullable = \in_array('nullable', $rules, true);

if ($isNull) {
if ($hasNullable && !array_intersect(self::REQUIRED_RULES, $rules) && !array_intersect(self::PRESENT_RULES, $rules)) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better use keys to get the proper item of the array, array_intersect is slow

return null;
}

if (array_intersect(self::REQUIRED_RULES, $rules)) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repeats array_intersect... same below

private readonly SerializerInterface $serializer,
private readonly SerializerContextBuilderInterface $serializerContextBuilder,
private ?TranslatorInterface $translator = null,
private readonly ?DenormalizationErrorHandlerInterface $errorHandler = null,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should cover bc layer, probably we should add a default instance or what


// v1: top-level paths only. Nested paths (e.g. "address.street") return null
// so the caller's fallback (collect mode) or rethrow (single mode) kicks in.
if (str_contains($path, '.') || str_contains($path, '[')) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo we can handle this using property accessor

}
}

return array_values($constraints);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the optimization but I dislike the spl_object_hash can we use another key? maybe property.$contraint::class?

}

return null;
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we merge this function with the one above to reduce complexity?

null,
(string) Type::INVALID_TYPE_ERROR,
);
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quite some duplicated logic with function above

$normalized = [];
foreach ($expectedTypes ?? [] as $expectedType) {
if (\is_string($expectedType) && (class_exists($expectedType) || interface_exists($expectedType))) {
$normalized[] = (new \ReflectionClass($expectedType))->getShortName();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need reflection here?

Translates PHP denormalization type errors (NotNormalizableValueException)
into a 422 validation exception when the target property has a matching
validation contract; rethrows for an honest 400 otherwise. Closes api-platform#7981.

Architecture:

- api-platform/state owns DenormalizationErrorHandlerInterface (no symfony/
  validator dep). DeserializeProvider becomes a thin delegate: catches the
  denormalization exception, hands it to the handler, rethrows on miss.
- api-platform/validator owns the Symfony impl (DenormalizationErrorHandler)
  which reads Symfony Validator metadata and throws ApiPlatform\Validator\
  Exception\ValidationException.
- api-platform/laravel owns ApiPlatform\Laravel\State\DenormalizationError
  Handler which reads Operation::getRules() and throws Laravel's native
  ApiPlatform\Laravel\ApiResource\ValidationError directly. The Laravel
  package does NOT depend on symfony/validator nor api-platform/validator.

Symfony rule table:
- null + NotBlank             → IS_BLANK_ERROR + constraint message
- null + NotNull              → IS_NULL_ERROR  + constraint message
- wrong type + Type           → INVALID_TYPE_ERROR + constraint message
- wrong type + any constraint → generic Type @ 422
- no constraint               → no match → rethrow → 400

Laravel rule table:
- null + required/filled                → IS_BLANK_ERROR
- null + present                        → IS_NULL_ERROR
- wrong type + string/integer/int/      → INVALID_TYPE_ERROR
  numeric/boolean/bool/array/date/json
- wrong type + any other rule           → INVALID_TYPE_ERROR
- null + nullable (no required/present) → no match → rethrow
- no rule                               → no match → rethrow → 400

Replaces the BackedEnum-only special case (commit 44bb18d) with a general
constraint-aware rule. Enums with NotNull/Type/Choice still produce 422;
enums with zero constraints regress to 400 — opt in via auto_mapping or
add an explicit Assert\Type(EnumClass).

In collect mode (collect_denormalization_errors=true), unconstrained errors
still emit a generic Type/INVALID_TYPE_ERROR entry so the response surface
stays consistent with prior behavior.

FormRequest-class rules and pure-callable rule sets are intentionally
skipped in the Laravel impl (v1): a FormRequest-based contract typically
runs in the validation phase against the raw request.

Tests:
- src/Validator/Tests/DenormalizationErrorHandlerTest.php — 10 unit tests
  covering each row of the matching table + group filtering + collect mode
- src/State/Tests/Provider/DeserializeProviderTest.php — handler-delegation
  assertions (drops the 3 tests that depended on Validator\Exception\
  ValidationException; that coverage moved to the Validator handler test)
- tests/Functional/DenormalizationValidationTest.php — 5 Symfony functional
  tests for null+NotBlank, null+NotNull, wrong+Type, no-constraint→400,
  collect mode mix
- src/Laravel/Tests/DenormalizationValidationTest.php — 3 Laravel functional
  tests including typed-DTO+rule→422 with ValidationError shape verified

Composer:
- api-platform/state: drops api-platform/validator dev-dep (cycle fix)
- api-platform/validator: requires api-platform/state
- api-platform/laravel: no new dependencies

Closes api-platform#7981
Refs api-platform#8183
@soyuka soyuka force-pushed the feat/denormalization-422-issue-7981 branch from eca83f8 to cb98da4 Compare May 29, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Request must be validated, but not Object after deserialization

1 participant