Skip to content

[Serializer] Fix denormalizing nested arrays as object values #30258

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

Closed
wants to merge 1 commit into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,6 @@ protected function instantiateObject(array &$data, $class, array &$context, \Ref
*
* @param object $object
* @param string|null $format
* @param array $context
*
* @return string[]
*/
Expand Down Expand Up @@ -397,23 +396,7 @@ private function validateAndDenormalize(string $currentClass, string $attribute,
return null;
}

if ($type->isCollection() && null !== ($collectionValueType = $type->getCollectionValueType()) && Type::BUILTIN_TYPE_OBJECT === $collectionValueType->getBuiltinType()) {
$builtinType = Type::BUILTIN_TYPE_OBJECT;
$class = $collectionValueType->getClassName().'[]';

// Fix a collection that contains the only one element
// This is special to xml format only
if ('xml' === $format && !\is_int(key($data))) {
$data = [$data];
}

if (null !== $collectionKeyType = $type->getCollectionKeyType()) {
$context['key_type'] = $collectionKeyType;
}
} else {
$builtinType = $type->getBuiltinType();
$class = $type->getClassName();
}
[$builtinType, $class] = $this->flattenDenormalizationType($type, $context);

$expectedTypes[Type::BUILTIN_TYPE_OBJECT === $builtinType && $class ? $class : $builtinType] = true;

Expand Down Expand Up @@ -450,6 +433,29 @@ private function validateAndDenormalize(string $currentClass, string $attribute,
throw new NotNormalizableValueException(sprintf('The type of the "%s" attribute for class "%s" must be one of "%s" ("%s" given).', $attribute, $currentClass, implode('", "', array_keys($expectedTypes)), \gettype($data)));
}

private function flattenDenormalizationType(Type $type, array &$context): array
{
if (!$type->isCollection() || !($collectionValueType = $type->getCollectionValueType()) || (Type::BUILTIN_TYPE_OBJECT !== $collectionValueType->getBuiltinType() && Type::BUILTIN_TYPE_ARRAY !== $collectionValueType->getBuiltinType())) {
return [$type->getBuiltinType(), $type->getClassName()];
}

if ($collectionValueType->isCollection()) {
[$builtinType, $class] = $this->flattenDenormalizationType($collectionValueType, $context);
} else {
[$builtinType, $class] = [$collectionValueType->getBuiltinType(), $collectionValueType->getClassName()];
}

if (Type::BUILTIN_TYPE_OBJECT === $builtinType) {
$class = $class.'[]';

if (null !== $collectionKeyType = $type->getCollectionKeyType()) {
$context['key_types'][] = $collectionKeyType;
}
}

return [$builtinType, $class];
}

/**
* @internal
*/
Expand Down Expand Up @@ -558,8 +564,6 @@ private function isMaxDepthReached(array $attributesMetadata, string $class, str
*
* {@inheritdoc}
*
* @param string|null $format
*
* @internal
*/
protected function createChildContext(array $parentContext, $attribute/*, ?string $format */)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Serializer\Normalizer;

use Symfony\Component\Serializer\Encoder\XmlEncoder;
use Symfony\Component\Serializer\Exception\BadMethodCallException;
use Symfony\Component\Serializer\Exception\InvalidArgumentException;
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
Expand Down Expand Up @@ -51,7 +52,18 @@ public function denormalize($data, $type, $format = null, array $context = [])
$serializer = $this->serializer;
$type = substr($type, 0, -2);

$builtinType = isset($context['key_type']) ? $context['key_type']->getBuiltinType() : null;
if (isset($context['key_types']) && \count($context['key_types'])) {
$builtinType = array_pop($context['key_types'])->getBuiltinType();
} else {
$builtinType = isset($context['key_type']) ? $context['key_type']->getBuiltinType() : null;
}

// Fix a collection that contains the only one element
// This is special to xml format only
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fix was in ObjectNormalizer, I think it makes more sense here, and now with the recursive types its harder to fit into ObjectNormalizer.

if (XmlEncoder::FORMAT === $format && (null === $builtinType ? !\is_int(key($data)) : !('\is_'.$builtinType)(key($data)))) {
$data = [$data];
}

foreach ($data as $key => $value) {
if (null !== $builtinType && !('is_'.$builtinType)($key)) {
throw new NotNormalizableValueException(sprintf('The type of the key "%s" must be "%s" ("%s" given).', $key, $builtinType, \gettype($key)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface;
use Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader;
use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;
use Symfony\Component\Serializer\Normalizer\ArrayDenormalizer;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
use Symfony\Component\Serializer\Serializer;
Expand Down Expand Up @@ -85,6 +86,31 @@ public function testDenormalizeWithExtraAttributesAndNoGroupsWithMetadataFactory
);
}

public function testDenormalizeDeepCollection()
{
$denormalizer = $this->getDenormalizerForDummyDeepCollection();

$dummyCollection = $denormalizer->denormalize(
[
'children' => [
[
[
'bar' => 'first',
],
],
],
],
DummyDeepCollection::class
);

$this->assertInstanceOf(DummyDeepCollection::class, $dummyCollection);
$this->assertIsArray($dummyCollection->children);
$this->assertCount(1, $dummyCollection->children);
$this->assertIsArray($dummyCollection->children[0]);
$this->assertCount(1, $dummyCollection->children[0]);
$this->assertInstanceOf(DummyChild::class, $dummyCollection->children[0][0]);
}

public function testDenormalizeCollectionDecodedFromXmlWithOneChild()
{
$denormalizer = $this->getDenormalizerForDummyCollection();
Expand Down Expand Up @@ -146,7 +172,41 @@ private function getDenormalizerForDummyCollection()
));

$denormalizer = new AbstractObjectNormalizerCollectionDummy(null, null, $extractor);
$arrayDenormalizer = new ArrayDenormalizerDummy();
$arrayDenormalizer = new ArrayDenormalizer();
$serializer = new SerializerCollectionDummy([$arrayDenormalizer, $denormalizer]);
$arrayDenormalizer->setSerializer($serializer);
$denormalizer->setSerializer($serializer);

return $denormalizer;
}

private function getDenormalizerForDummyDeepCollection()
{
$extractor = $this->getMockBuilder(PhpDocExtractor::class)->getMock();
$extractor->method('getTypes')
->will($this->onConsecutiveCalls(
[
new Type(
'array',
false,
null,
true,
new Type('int'),
new Type(
'array',
false,
null,
true,
new Type('int'),
new Type('object', false, DummyChild::class)
)
),
],
null
));

$denormalizer = new AbstractObjectNormalizerCollectionDummy(null, null, $extractor);
$arrayDenormalizer = new ArrayDenormalizer();
$serializer = new SerializerCollectionDummy([$arrayDenormalizer, $denormalizer]);
$arrayDenormalizer->setSerializer($serializer);
$denormalizer->setSerializer($serializer);
Expand Down Expand Up @@ -264,6 +324,12 @@ class DummyCollection
public $children;
}

class DummyDeepCollection
{
/** @var DummyChild[][] */
public $children;
}

class DummyChild
{
public $bar;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@

use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Symfony\Component\PropertyInfo\Type;
use Symfony\Component\Serializer\Normalizer\ArrayDenormalizer;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Serializer;
use Symfony\Component\Serializer\SerializerInterface;

class ArrayDenormalizerTest extends TestCase
Expand Down Expand Up @@ -64,6 +67,108 @@ public function testDenormalize()
);
}

public function testDenormalizeNested()
{
$m = $this->getMockBuilder(DenormalizerInterface::class)
->getMock();

$denormalizer = new ArrayDenormalizer();
$serializer = new Serializer([$denormalizer, $m]);
$denormalizer->setSerializer($serializer);

$m->method('denormalize')
->with(['foo' => 'one', 'bar' => 'two'], __NAMESPACE__.'\ArrayDummy')
->willReturn(new ArrayDummy('one', 'two'));

$m->method('supportsDenormalization')
->with($this->anything(), __NAMESPACE__.'\ArrayDummy')
->willReturn(true);

$result = $denormalizer->denormalize(
[
[
['foo' => 'one', 'bar' => 'two'],
],
],
__NAMESPACE__.'\ArrayDummy[][]',
null,
['key_types' => [new Type('int'), new Type('int')]]
);

$this->assertEquals(
[[
new ArrayDummy('one', 'two'),
]],
$result
);
}

public function testDenormalizeCheckKeyType()
{
$this->expectException(\Symfony\Component\Serializer\Exception\NotNormalizableValueException::class);

$this->denormalizer->denormalize(
[
['foo' => 'one', 'bar' => 'two'],
],
__NAMESPACE__.'\ArrayDummy[]',
null,
['key_type' => new Type('string')]
);
}

public function testDenormalizeCheckKeyTypes()
{
$this->expectException(\Symfony\Component\Serializer\Exception\NotNormalizableValueException::class);

$this->denormalizer->denormalize(
[
['foo' => 'one', 'bar' => 'two'],
],
__NAMESPACE__.'\ArrayDummy[]',
null,
['key_types' => [new Type('string')]]
);
}

public function testDenormalizeNestedCheckKeyTypes()
{
$m = $this->getMockBuilder(DenormalizerInterface::class)
->getMock();

$denormalizer = new ArrayDenormalizer();
$serializer = new Serializer([$denormalizer, $m]);
$denormalizer->setSerializer($serializer);

$m->method('denormalize')
->with(['foo' => 'one', 'bar' => 'two'], __NAMESPACE__.'\ArrayDummy')
->willReturn(new ArrayDummy('one', 'two'));

$m->method('supportsDenormalization')
->with($this->anything(), __NAMESPACE__.'\ArrayDummy')
->willReturn(true);

$result = $denormalizer->denormalize(
[
'top' => [
['foo' => 'one', 'bar' => 'two'],
],
],
__NAMESPACE__.'\ArrayDummy[][]',
null,
['key_types' => [new Type('string')], 'key_type' => new Type('int')]
);

$this->assertEquals(
[
'top' => [
new ArrayDummy('one', 'two'),
],
],
$result
);
}

public function testSupportsValidArray()
{
$this->serializer->expects($this->once())
Expand Down