fix(serializer): preserve denormalization errors for nullable object properties#8393
Open
bendavies wants to merge 2 commits into
Open
fix(serializer): preserve denormalization errors for nullable object properties#8393bendavies wants to merge 2 commits into
bendavies wants to merge 2 commits into
Conversation
When a wrong-typed value is sent for a nullable backed enum property (e.g.
`?Status`), collectDenormalizationErrors produced the violation message
"This value should be of type Status|null." — the internal PHP type, which
appears nowhere in the OpenAPI schema and names none of the accepted values.
AbstractItemNormalizer delegates nullable backed enums to BackedEnumNormalizer,
which throws an actionable exception (accepted scalars + hint). The post-loop
block then rewrapped it, replacing its expectedTypes with `(string) $types`
("Status|null") and dropping the user-facing hint flag.
Preserve the hint flag when the failure comes from an object/enum normalizer,
and in DeserializeProvider render a BackedEnum expected type as its JSON-visible
backing scalar ("string"/"int") instead of the enum FQCN. The enum class stays
in expectedTypes so the 400->422 promotion (api-platform#8183) keeps working.
Now `{"status": true}` on a `?Status` property yields
"This value should be of type string|null." plus the enum hint.
Closes api-platform#8388
…properties A value of the wrong type for a nullable object property (?Uuid, ?DateTimeImmutable, ...) produced "This value should be of type Uuid|null." with no hint, while the non-nullable version of the same property produced the schema accurate "This value should be of type string." plus a hint. Nullability alone flipped the result: a nullable type is a composite, so the dedicated normalizer's exception was stashed, and once every union member failed the error was rebuilt from the PHP type union. The rebuild kept the message, but the visible violation is built by DeserializeProvider from getExpectedTypes(), and those were replaced with the stringified union members. Re-throw the stashed exception when the union contains an object member that is neither a resource class nor an enum, extending its expected types with "null" when the property accepts null. The label comes from whatever the dedicated normalizer reports as its accepted input types: a wrong value for a nullable Ulid property now renders "This value should be of type string|null." plus the UidNormalizer hint, matching the 4.1.x output extended with "null". Enums keep flowing through the rebuilt exception and are mapped to their backing type downstream; scalar unions and resource relations are untouched.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8392.
Built on top of #8389, whose commit is included in this branch; only the last commit is new here. I will rebase once #8389 merges. Related to #8388 (the backed enum flavor of the same regression, fixed by #8389); this PR covers the remaining nullable object types.
Description
When a value of the wrong type is sent for a nullable object typed property (
?Uuid,?Ulid,?DateTimeImmutable,?DateInterval,?UuidInterface,?\BcMath\Number, ...), the 422 violation collected withcollectDenormalizationErrors: truereports the property's PHP type union as the expected type, with no hint:{ "propertyPath": "ulid", "message": "This value should be of type Ulid|null." }Ulid|nullis the internal PHP type: it appears nowhere in the JSON Schema / OpenAPI output (which documentsstringplus aformat), it cannot be sent over JSON, and it does not tell the consumer what to send.The trigger is nullability alone. The non-nullable version of the same property yields the correct error from the dedicated normalizer (
This value should be of type string.plus a hint), and 4.1.x produced the correct message for nullable properties too; this is a regression from the TypeInfo migration (#6979).Root cause
In
AbstractItemNormalizer::createAndValidateAttributeValue(), a nullable property type is a composite (NullableType), so theNotNormalizableValueExceptionthrown by the dedicated normalizer (UidNormalizer,DateTimeNormalizer,DateIntervalNormalizer, API Platform's RamseyUuidDenormalizer, ...) is stashed while the remaining union members are tried. Once they all fail, the error is rebuilt withexpectedTypes = array_map(strval(...), $types), the stringified PHP union members.DeserializeProvider::createViolationFromException()renders the violation message fromgetExpectedTypes()and only surfaces the exception message as thehint, so the normalizer's accurate expected types (e.g.["string"]) are lost, and the rebuild also passesfalseforuseMessageForUser, so the hint disappears as well. Fixing only the message string would change nothing visible.For a non-nullable property the union has a single member and the same exception is re-thrown unchanged, which is why nullability alone flips the result.
Change
When the union contains an object member that is neither a resource class nor an enum, re-throw the stashed normalizer exception with its expected types, message, hint and
canUseMessageForUser()intact, extending the expected types withnullwhen the property also accepts null:?Ulid/?DateTimeImmutableThis value should be of type Ulid|null.(no hint)This value should be of type string|null.plus the normalizer hint?UuidInterface(ramsey/uuid)This value should be of type UuidInterface|null.(no hint)This value should be of type uuid|null.plus hintInvalid UUID string: yThe label comes from whatever the dedicated normalizer reports as its accepted input types, matching the 4.1.x output extended with
null. (For Ramsey UUIDs that label isuuid, a preexisting choice ofUuidDenormalizeralso visible on non-nullable properties; aligning it with the schema typestringwould be a separate change.)Enum members are deliberately excluded from the re-throw: their violations keep flowing through the rebuilt exception that #8389 renders via the enum backing type, and #8389's
EnumDenormalizationValidationTestpasses on top of this change. Pure scalar unions (?string,?int, ...) and resource relations are untouched.Tests
?DateTimeImmutableparity tests asserting the surfaced exception keeps the normalizer's expected types (plusnullfor the nullable case), message, hint flag and path.ValidationTestnow assertsThis value should be of type uuid|null.plus theInvalid UUID string: yhint on the native TypeInfo path; the legacy property-info path keepsThis value should be of type uuid..