-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Serializer] Fix reindex normalizedData array in AbstractObjectNormalizer::denormalize() #49700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hey! Thanks for your PR. You are targeting branch "6.3" but it seems your PR description refers to branch "6.2". Cheers! Carsonbot |
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
Outdated
Show resolved
Hide resolved
We spoked about this pull request. As discussed, I tested whether this error already existed in version 6.0, however it is only from version 6.2 onwards. In AbstractObjectNormalizer.php, the block from line 311 to 324 has been added. However the cause of the error is the array_merge() in line 324. symfony/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php Lines 311 to 324 in 6a9f47c
Therefore, an update from 5.4 LTS to 6.4 LTS will definitely result in a breaking change. Can you please check and release this pull request as a second reviewer so that this fix goes into 6.4. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://3v4l.org/CUbsP shows the difference
@elevado Can you rebase your changes on the |
…izer::denormalize()
Thank you @elevado. |
This PR was merged into the 6.3 branch. Discussion ---------- Fix order array sum normalizedData and nestedData | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #51823 | License | MIT ### Description With the update of Serializer to 6.3.5, some deserialization of array to objects does behave differently (changed order of priority of configuration via attribute `#[SerializedPath]` vs. property name, when there is a key on root level with the same name as the private property. Related to #49700. ### How to reproduce Example to explain changed behavior: ```json { "data": { "item": { "id": "id-1" } }, "id": "id-2" } ``` ```php final class SomeEvent { #[SerializedPath('[data][item][id]')] public string $id; } ``` Before 6.3.5, the value of the id was `id-1`, with the change of #49700, the value of the id becomes `id-2`. #49700 changes `array_merge` with `array + array`. It seems that the problem stated above is related to the fact that array_merge does overwrite keys differently than array + array: ```php $a = ['key' => 'value-a']; $b = ['key' => 'value-b']; var_dump(array_merge($a, $b)); // Results in: // array(1) { // ["key"]=> // string(7) "value-b" // } var_dump($a + $b); // Results in: // array(1) { // ["key"]=> // string(7) "value-a" // } ``` ### Solution As `array_merge` does behave slightly differently that array + array, the solution could be to switch array order to: ```diff - $normalizedData = $normalizedData + $nestedData; + $normalizedData = $nestedData + $normalizedData; ``` This would result in the same, while keeping the fix (#49700) for the numeric key value Commits ------- 67f49d4 Fix order array sum normalizedData and nestedData
In the method
AbstractObjectNormalizer::denormalize()
the index of the array$normalizedData
is reindexed after anarray_merge
.This error occurs when a JSON is deserialised and when the SerializedName is numeric. This results in an incorrect mapping to the properties.
Array before merge:
After merge with
array_merge
:After merge with
array_replace
:All Serializer unittests run successfully.