Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedMar 16, 2023
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 OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
elevado commentedSep 22, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 in6a9f47c
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. |
xabbuh left a comment
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
xabbuh commentedSep 24, 2023
@elevado Can you rebase your changes on the |
…izer::denormalize()
nicolas-grekas commentedSep 29, 2023
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### DescriptionWith 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 reproduceExample to explain changed behavior:```json{ "data": { "item": { "id": "id-1" } }, "id": "id-2"}``````phpfinal 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"// }```### SolutionAs `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 valueCommits-------67f49d4 Fix order array sum normalizedData and nestedData
Uh oh!
There was an error while loading.Please reload this page.
In the method
AbstractObjectNormalizer::denormalize()the index of the array$normalizedDatais 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.
{"1":"John","2":"Doe","10031":"john.doe@example.com",}Array before merge:
After merge with
array_merge:After merge with
array_replace:All Serializer unittests run successfully.