Skip to content

[Serializer] Add Support of recursive denormalization on object_to_populate #30607

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

Merged
merged 1 commit into from
Apr 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Symfony/Component/Serializer/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ CHANGELOG

* added the list of constraint violations' parameters in `ConstraintViolationListNormalizer`
* added support for serializing `DateTimeZone` objects
* added a `deep_object_to_populate` context option to recursive denormalize on `object_to_populate` object.

4.2.0
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Serializer\Normalizer;

use Symfony\Component\PropertyAccess\Exception\InvalidArgumentException;
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
use Symfony\Component\PropertyInfo\PropertyTypeExtractorInterface;
use Symfony\Component\PropertyInfo\Type;
use Symfony\Component\Serializer\Encoder\JsonEncoder;
Expand All @@ -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';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in #30888 i am adding phpdoc to these configuration constants to explain what they do and what values they accept.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


private $propertyTypeExtractor;
private $typesCache = [];
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=> #30977

} catch (NoSuchPropertyException $e) {
}
}

$value = $this->validateAndDenormalize($class, $attribute, $value, $format, $context);
try {
$this->setAttributeValue($object, $attribute, $value, $format, $context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ trait ObjectToPopulateTrait
*/
protected function extractObjectToPopulate($class, array $context, $key = null)
{
$key = $key ?: 'object_to_populate';
$key = $key ?? AbstractNormalizer::OBJECT_TO_POPULATE;

if (isset($context[$key]) && \is_object($context[$key]) && $context[$key] instanceof $class) {
return $context[$key];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Serializer\Tests\Fixtures;

/**
* @author Jérôme Desjardin <jewome62@gmail.com>
*/
class DeepObjectPopulateChildDummy
{
public $foo;

public $bar;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Serializer\Tests\Fixtures;

/**
* @author Jérôme Desjardin <jewome62@gmail.com>
*/
class DeepObjectPopulateParentDummy
{
/**
* @var DeepObjectPopulateChildDummy|null
*/
private $child;

public function setChild(?DeepObjectPopulateChildDummy $child)
{
$this->child = $child;
}

public function getChild(): ?DeepObjectPopulateChildDummy
{
return $this->child;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
use Symfony\Component\Serializer\SerializerAwareInterface;
use Symfony\Component\Serializer\SerializerInterface;
use Symfony\Component\Serializer\Tests\Fixtures\DeepObjectPopulateChildDummy;
use Symfony\Component\Serializer\Tests\Fixtures\DeepObjectPopulateParentDummy;

class AbstractObjectNormalizerTest extends TestCase
{
Expand Down Expand Up @@ -171,6 +173,48 @@ public function testSkipNullValues()
$result = $normalizer->normalize($dummy, null, [AbstractObjectNormalizer::SKIP_NULL_VALUES => true]);
$this->assertSame(['bar' => 'present'], $result);
}

public function testDeepObjectToPopulate()
{
$child = new DeepObjectPopulateChildDummy();
$child->bar = 'bar-old';
$child->foo = 'foo-old';

$parent = new DeepObjectPopulateParentDummy();
$parent->setChild($child);

$context = [
AbstractObjectNormalizer::OBJECT_TO_POPULATE => $parent,
AbstractObjectNormalizer::DEEP_OBJECT_TO_POPULATE => true,
];

$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
$normalizer = new ObjectNormalizer($classMetadataFactory, null, null, new PhpDocExtractor());

$newChild = new DeepObjectPopulateChildDummy();
$newChild->bar = 'bar-new';
$newChild->foo = 'foo-old';

$serializer = $this->getMockBuilder(__NAMESPACE__.'\ObjectSerializerDenormalizer')->getMock();
$serializer
->method('supportsDenormalization')
->with($this->arrayHasKey('bar'),
$this->equalTo(DeepObjectPopulateChildDummy::class),
$this->isNull(),
$this->contains($child))
->willReturn(true);
$serializer->method('denormalize')->willReturn($newChild);

$normalizer->setSerializer($serializer);
$normalizer->denormalize([
'child' => [
'bar' => 'bar-new',
],
], 'Symfony\Component\Serializer\Tests\Fixtures\DeepObjectPopulateParentDummy', null, $context);

$this->assertSame('bar-new', $parent->getChild()->bar);
$this->assertSame('foo-old', $parent->getChild()->foo);
}
}

class AbstractObjectNormalizerDummy extends AbstractObjectNormalizer
Expand Down Expand Up @@ -348,3 +392,7 @@ public function setSerializer(SerializerInterface $serializer)
$this->serializer = $serializer;
}
}

abstract class ObjectSerializerDenormalizer implements SerializerInterface, DenormalizerInterface
{
}