[Serializer] Documenting the new SKIP_UNINITIALIZED_VALUES option #15823
Conversation
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
fabpot
added a commit
to symfony/symfony
that referenced
this pull request
Oct 16, 2021
…(ivannemets-sravniru) This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Serializer] #36594 attributes cache breaks normalization | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #36594 | License | MIT | Doc PR | symfony/symfony-docs#15823 The bug itself is explained in the following (simplified) example: ```php class Dummy { public array $requiredData; // supposed to be set for any object public array $optionalData; // can be not initialized (and if so - ignored by serializer) } $object1 = new Dummy(); $object1->requiredData = ['username' => 'foo']; $json1 = $serializer->serialize($object1, 'json'); // {"requiredData": {"username": "foo"}} // at this point object normalizer has already cached attributes for Dummy::class and context, // now it contains array ['requiredData'] - optionalData has been ignored as it's unitialized // then, while the script is still running, we have another object of the same class with optionalData set $object2 = new Dummy(); $object2->requiredData = ['username' => 'bar']; $object2->optionalData = ['email' => 'bar@test.com']; $json2 = $serializer->serialize($object2, 'json'); // expected: {"requiredData": {"username": "bar"}, "optionalData": {"email": "bar@test.com"}} // actual: {"requiredData": {"username": "bar"}} // here normalizer has no clue about optionalData attribute since it reuses attributes cached // in \Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::$attributesCache // while normalizing the first object ``` Though this PR created for 5.4 branch, it actually fixes a bug reproducible in 5.3. The reason why I use 5.4 for this fix is that 5.4 introduces a [new feature](symfony/symfony-docs#15823) related to the same problem. If this PR gets approved and merged, I can potentially implement the same fix for 5.3, but `SKIP_UNINITIALIZED_VALUES` will cause some merge conflicts in the future.. As of v 5.3 symfony ignores uninitialized properties by default in `ObjectNormalizer::extractAttributes` and `PropertyNormalizer::extractAttributes` (implemented in #38900 and #34791) but this approach is wrong - **`extractAttributes` method MUST return the same attributes list for any instance of the same class**, otherwise cached attributes do not match actual attributes list for different instances having different set of initialized/uninitialized properties So, this PR does a few things: 1. removes ignoring attributes from all built-in implementations of `\Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::extractAttributes` so that even uninitialized attributes will be cached by normalizer. Instead, we ignore uninitialized attributes when trying to access them while calling `\Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::getAttributeValue` 2. sets `true` as the default value for `SKIP_UNINITIALIZED_VALUES` context setting as I believe this is the expected default behavior for any built-in object normalizer 3. makes `SKIP_UNINITIALIZED_VALUES` compatible with not just `ObjectNormalizer`, but with two other built-in normalizers `PropertyNormalizer` and `GetSetMethodNormalizer` as well Commits ------- 55818c3 [Serializer] #36594 attributes cache breaks normalization
|
Georgi, congrats on your first Symfony Docs contribution 🎉 This was an amazing first-time contribution, and I'm really sorry it took us so long to merge it. |
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.
Documents changes introduced here: symfony/symfony#41615
Original issue: symfony/symfony#40578
Docs issue: #15785