[Serializer] Add Support of recursive denormalization on object_to_populate #30607
Conversation
e8f290f to
39af1d0
Compare
|
Nice feature, does this works on collection ? (don't think so seeing the code, and wondering if it's doable) |
be46749 to
7971b08
Compare
|
See #21669 (comment) I don't think those issues are addressed. |
7971b08 to
585e86b
Compare
|
@joelwurtz i test it tomorrow |
|
@joelwurtz you have right, an object collection in the parent has no populated object, the problem is to have a reference for linked the object passed in the 'object_to_populate' and the object received by decoding. Either we consider this PR for objects containing other objects, or we have a solution to bind collection objects to the input received
|
|
I don't think this PR should solve that problem, futhermore, some users would have different behavior:
|
src/Symfony/Component/Serializer/Normalizer/ObjectToPopulateTrait.php
Outdated
Show resolved
Hide resolved
|
@jewome62 You need to rebase the PR as we don't merge PRs with merge commits. |
I fix that, I have use the confilct resolver into from github |
1ebbadb to
82d8bb2
Compare
|
Failure of unit tests is not related to this PR |
82d8bb2 to
5b72386
Compare
|
Thank you @jewome62. |
…on object_to_populate (jewome62) This PR was squashed before being merged into the 4.3-dev branch (closes #30607). Discussion ---------- [Serializer] Add Support of recursive denormalization on object_to_populate | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | Pending | Fixed tickets | #21669 | License | MIT | Doc PR | Pending Currently the deserialization re-create new sub-object with object_to_populate. This option permit to make object_to_populate recursive. Commits ------- 5b72386 [Serializer] Add Support of recursive denormalization on object_to_populate
| @@ -38,6 +39,7 @@ abstract class AbstractObjectNormalizer extends AbstractNormalizer | |||
| const SKIP_NULL_VALUES = 'skip_null_values'; | |||
| const MAX_DEPTH_HANDLER = 'max_depth_handler'; | |||
| const EXCLUDE_FROM_CACHE_KEY = 'exclude_from_cache_key'; | |||
| const DEEP_OBJECT_TO_POPULATE = 'deep_object_to_populate'; | |||
There was a problem hiding this comment.
in #30888 i am adding phpdoc to these configuration constants to explain what they do and what values they accept.
There was a problem hiding this comment.
as this is already merged, i will rebase #30888 and document it there. and extract the test for this into a trait like we do the other tests there.
| @@ -274,6 +276,13 @@ public function denormalize($data, $class, $format = null, array $context = []) | |||
| continue; | |||
| } | |||
|
|
|||
| if ($context[self::DEEP_OBJECT_TO_POPULATE] ?? $this->defaultContext[self::DEEP_OBJECT_TO_POPULATE] ?? false) { | |||
| try { | |||
| $context[self::OBJECT_TO_POPULATE] = $this->getAttributeValue($object, $attribute, $format, $context); | |||
There was a problem hiding this comment.
this overwrites the context. i think this means that if there are more loops, you might have a previous object to populate in the context. i think we should unset OBJECT_TO_POPULATE if it does not find anything and when the DEEP_OBJECT_TO_POPULATE flag is not true.
There was a problem hiding this comment.
we found where OBJECT_TO_POPULATE is unset. so this is fine as is.
i will try to do a test and fix to make OBJECT_TO_POPULATE more robust, as i think there are edge cases that would lead to weird behaviour.
|
documentation PR: symfony/symfony-docs#11344 |
This PR was merged into the master branch. Discussion ---------- [serializer] document DEEP_OBJECT_TO_POPULATE Document the new feature added in symfony/symfony#30607 Commits ------- 7ea06fa document DEEP_OBJECT_TO_POPULATE
…populate (dbu) This PR was merged into the 3.4 branch. Discussion ---------- [serializer] prevent mixup in normalizer of the object to populate EUFOSSA | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - OBJECT_TO_POPULATE is meant to specify the top level object. The implementation left the option in the context and it would be used whenever we have the first element that matches the class. #30607 (to master) introduces the feature to also keep the instances of attributes to deeply populate an existing object tree. In both cases, we do not want the mix up to happen with what the current OBJECT_TO_POPULATE is. Commits ------- fdb668e prevent mixup of the object to populate
Currently the deserialization re-create new sub-object with object_to_populate.
This option permit to make object_to_populate recursive.