From 89e2f63c9cc245ff541e04ed5004e4e6b6588511 Mon Sep 17 00:00:00 2001 From: Jeroeny Date: Wed, 11 Oct 2023 14:49:06 +0200 Subject: [PATCH 1/2] Add support for union collection value types in `ArrayDenormalizer` --- src/Symfony/Component/Serializer/CHANGELOG.md | 4 + .../Normalizer/AbstractObjectNormalizer.php | 54 +++----- .../Normalizer/ArrayDenormalizer.php | 46 ++++++- .../Normalizer/ArrayDenormalizerTest.php | 128 ++++++++++++++---- .../Serializer/Tests/SerializerTest.php | 2 +- 5 files changed, 168 insertions(+), 66 deletions(-) diff --git a/src/Symfony/Component/Serializer/CHANGELOG.md b/src/Symfony/Component/Serializer/CHANGELOG.md index 3118834d80175..882c072404a93 100644 --- a/src/Symfony/Component/Serializer/CHANGELOG.md +++ b/src/Symfony/Component/Serializer/CHANGELOG.md @@ -1,6 +1,10 @@ CHANGELOG ========= +7.2 +--- +* Add support for union collection value types in `ArrayDenormalizer` + 7.1 --- diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php index d1f565cea151f..a2fd65fd9ed47 100644 --- a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php @@ -444,11 +444,9 @@ private function validateAndDenormalizeLegacy(array $types, string $currentClass return null; } - $collectionValueType = $type->isCollection() ? $type->getCollectionValueTypes()[0] ?? null : null; - // Fix a collection that contains the only one element // This is special to xml format only - if ('xml' === $format && null !== $collectionValueType && (!\is_array($data) || !\is_int(key($data)))) { + if ('xml' === $format && \count($type->getCollectionValueTypes()) > 0 && (!\is_array($data) || !\is_int(key($data)))) { $data = [$data]; } @@ -464,7 +462,7 @@ private function validateAndDenormalizeLegacy(array $types, string $currentClass $builtinType = $type->getBuiltinType(); if (\is_string($data) && (XmlEncoder::FORMAT === $format || CsvEncoder::FORMAT === $format)) { if ('' === $data) { - if (LegacyType::BUILTIN_TYPE_ARRAY === $builtinType) { + if (LegacyLegacyType::BUILTIN_TYPE_ARRAY === $builtinType) { return []; } @@ -508,41 +506,33 @@ private function validateAndDenormalizeLegacy(array $types, string $currentClass } } - if (null !== $collectionValueType && LegacyType::BUILTIN_TYPE_OBJECT === $collectionValueType->getBuiltinType()) { - $builtinType = LegacyType::BUILTIN_TYPE_OBJECT; - $class = $collectionValueType->getClassName().'[]'; - - if (\count($collectionKeyType = $type->getCollectionKeyTypes()) > 0) { - $context['key_type'] = \count($collectionKeyType) > 1 ? $collectionKeyType : $collectionKeyType[0]; - } - - $context['value_type'] = $collectionValueType; - } elseif ($type->isCollection() && \count($collectionValueType = $type->getCollectionValueTypes()) > 0 && LegacyType::BUILTIN_TYPE_ARRAY === $collectionValueType[0]->getBuiltinType()) { - // get inner type for any nested array - [$innerType] = $collectionValueType; + $builtinType = $type->getBuiltinType(); + $class = $type->getClassName(); - // note that it will break for any other builtinType - $dimensions = '[]'; - while (\count($innerType->getCollectionValueTypes()) > 0 && LegacyType::BUILTIN_TYPE_ARRAY === $innerType->getBuiltinType()) { - $dimensions .= '[]'; + $innerType = $type; + if ($type->isCollection() && \count($type->getCollectionValueTypes()) > 0) { + while (1 === \count($innerType->getCollectionValueTypes()) && LegacyType::BUILTIN_TYPE_ARRAY === $innerType->getCollectionValueTypes()[0]->getBuiltinType()) { [$innerType] = $innerType->getCollectionValueTypes(); } - if (null !== $innerType->getClassName()) { - // the builtinType is the inner one and the class is the class followed by []...[] - $builtinType = $innerType->getBuiltinType(); - $class = $innerType->getClassName().$dimensions; - } else { - // default fallback (keep it as array) - $builtinType = $type->getBuiltinType(); - $class = $type->getClassName(); + $dimensions = ''; + $arrayType = $type; + do { + $dimensions .= '[]'; + [$arrayType] = $arrayType->getCollectionValueTypes(); + } while (\count($arrayType->getCollectionValueTypes()) > 0 && LegacyType::BUILTIN_TYPE_ARRAY === $arrayType->getBuiltinType()); + + if (\count($innerType->getCollectionValueTypes()) > 1 || \in_array($innerType->getCollectionValueTypes()[0]->getBuiltinType(), [LegacyType::BUILTIN_TYPE_OBJECT, LegacyType::BUILTIN_TYPE_ARRAY], true)) { + $builtinType = LegacyType::BUILTIN_TYPE_OBJECT; + $class = $arrayType->getClassName().$dimensions; + $context['value_type'] = $type; + $expectedTypes['array<'.implode('|', array_map(fn (Type $t) => $t->getClassName() ?? $t->getBuiltinType(), $innerType->getCollectionValueTypes())).'>'] = true; } - } else { - $builtinType = $type->getBuiltinType(); - $class = $type->getClassName(); } - $expectedTypes[LegacyType::BUILTIN_TYPE_OBJECT === $builtinType && $class ? $class : $builtinType] = true; + if (!str_ends_with($class, '[]')) { + $expectedTypes[LegacyType::BUILTIN_TYPE_OBJECT === $builtinType && $class ? $class : $builtinType] = true; + } if (LegacyType::BUILTIN_TYPE_OBJECT === $builtinType && null !== $class) { if (!$this->serializer instanceof DenormalizerInterface) { diff --git a/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php b/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php index 1bd6c54b374ce..6b472c26afa58 100644 --- a/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php @@ -13,7 +13,9 @@ use Symfony\Component\PropertyInfo\Type as LegacyType; use Symfony\Component\Serializer\Exception\BadMethodCallException; +use Symfony\Component\Serializer\Exception\ExtraAttributesException; use Symfony\Component\Serializer\Exception\InvalidArgumentException; +use Symfony\Component\Serializer\Exception\MissingConstructorArgumentsException; use Symfony\Component\Serializer\Exception\NotNormalizableValueException; use Symfony\Component\TypeInfo\Type; use Symfony\Component\TypeInfo\Type\UnionType; @@ -48,14 +50,19 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a throw new BadMethodCallException('Please set a denormalizer before calling denormalize()!'); } if (!\is_array($data)) { - throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('Data expected to be "%s", "%s" given.', $type, get_debug_type($data)), $data, ['array'], $context['deserialization_path'] ?? null); + $valueType = $context['value_type'] ?? null; + $expected = $valueType ? 'array<'.implode('|', array_map(fn (Type $type) => $type->getClassName() ?? $type->getBuiltinType(), $valueType->getCollectionValueTypes())).'>' : $type; + + throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('Data expected to be "%s", "%s" given.', $expected, get_debug_type($data)), $data, ['array'], $context['deserialization_path'] ?? null); } if (!str_ends_with($type, '[]')) { throw new InvalidArgumentException('Unsupported class: '.$type); } $type = substr($type, 0, -2); + $valueType = $context['value_type'] ?? null; + # todo $typeIdentifiers = []; if (null !== $keyType = ($context['key_type'] ?? null)) { if ($keyType instanceof Type) { @@ -65,13 +72,46 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a } } + if ($valueType instanceof Type && \count($keyTypes = $valueType->getCollectionKeyTypes()) > 0) { + $builtinTypes = array_map(static fn (Type $keyType) => $keyType->getBuiltinType(), $keyTypes); + } else { + $builtinTypes = array_map(static fn (LegacyType $keyType) => $keyType->getBuiltinType(), \is_array($keyType = $context['key_type'] ?? []) ? $keyType : [$keyType]); + } + foreach ($data as $key => $value) { $subContext = $context; $subContext['deserialization_path'] = ($context['deserialization_path'] ?? false) ? sprintf('%s[%s]', $context['deserialization_path'], $key) : "[$key]"; $this->validateKeyType($typeIdentifiers, $key, $subContext['deserialization_path']); - $data[$key] = $this->denormalizer->denormalize($value, $type, $format, $subContext); + if ($valueType instanceof Type) { + foreach ($valueType->getCollectionValueTypes() as $subtype) { + try { + $subContext['value_type'] = $subtype; + + if ($subtype->isNullable() && null === $value) { + $data[$key] = null; + + continue 2; + } + + if (Type::BUILTIN_TYPE_ARRAY === $subtype->getBuiltinType()) { + $class = $type; + } else { + $class = $subtype->getClassName() ?? $subtype->getBuiltinType(); + } + + $data[$key] = $this->denormalizer->denormalize($value, $class, $format, $subContext); + + continue 2; + } catch (NotNormalizableValueException|InvalidArgumentException|ExtraAttributesException|MissingConstructorArgumentsException $e) { + } + } + + throw $e; + } else { + $data[$key] = $this->denormalizer->denormalize($value, $type, $format, $subContext); + } } return $data; @@ -90,7 +130,7 @@ public function supportsDenormalization(mixed $data, string $type, ?string $form /** * @param list $typeIdentifiers */ - private function validateKeyType(array $typeIdentifiers, mixed $key, string $path): void + private function validateKeyType(array $builtinTypes, mixed $key, string $path): void { if (!$typeIdentifiers) { return; diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/ArrayDenormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/ArrayDenormalizerTest.php index be4a30d866c4e..d07858ecc6e16 100644 --- a/src/Symfony/Component/Serializer/Tests/Normalizer/ArrayDenormalizerTest.php +++ b/src/Symfony/Component/Serializer/Tests/Normalizer/ArrayDenormalizerTest.php @@ -13,6 +13,8 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Component\PropertyInfo\Type; +use Symfony\Component\Serializer\Exception\NotNormalizableValueException; use Symfony\Component\Serializer\Normalizer\ArrayDenormalizer; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; @@ -28,38 +30,26 @@ protected function setUp(): void $this->denormalizer->setDenormalizer($this->serializer); } - public function testDenormalize() + /** + * @dataProvider getTestArrays + */ + public function testDenormalize(array $input, array $expected, string $type, string $format, array $context = []) { - $series = [ - [[['foo' => 'one', 'bar' => 'two']], new ArrayDummy('one', 'two')], - [[['foo' => 'three', 'bar' => 'four']], new ArrayDummy('three', 'four')], - ]; - - $this->serializer->expects($this->exactly(2)) + $this->serializer->expects($this->atLeastOnce()) ->method('denormalize') - ->willReturnCallback(function ($data) use (&$series) { - [$expectedArgs, $return] = array_shift($series); - $this->assertSame($expectedArgs, [$data]); - - return $return; - }) - ; - - $result = $this->denormalizer->denormalize( - [ - ['foo' => 'one', 'bar' => 'two'], - ['foo' => 'three', 'bar' => 'four'], - ], - __NAMESPACE__.'\ArrayDummy[]' - ); - - $this->assertEquals( - [ - new ArrayDummy('one', 'two'), - new ArrayDummy('three', 'four'), - ], - $result - ); + ->willReturnCallback(function ($data, $type, $format, $context) use ($input) { + $key = (int) trim($context['deserialization_path'], '[]'); + $expected = $input[$key]; + $this->assertSame($expected, $data); + + try { + return class_exists($type) ? new $type(...$data) : $data; + } catch (\Throwable $e) { + throw new NotNormalizableValueException($e->getMessage(), $e->getCode(), $e); + } + }); + + $this->assertEquals($expected, $this->denormalizer->denormalize($input, $type, $format, $context)); } public function testSupportsValidArray() @@ -108,6 +98,74 @@ public function testSupportsNoArray() ) ); } + + public static function getTestArrays(): array + { + return [ + 'array' => [ + [ + ['foo' => 'one', 'bar' => 'two'], + ['foo' => 'three', 'bar' => 'four'], + ], + [ + new ArrayDummy('one', 'two'), + new ArrayDummy('three', 'four'), + ], + __NAMESPACE__.'\ArrayDummy[]', + 'json', + ], + + 'array' => [ + [ + ['foo' => 'one', 'bar' => 'two'], + ['baz' => 'three'], + null, + ], + [ + new ArrayDummy('one', 'two'), + new UnionDummy('three'), + null, + ], + 'mixed[]', + 'json', + [ + 'value_type' => new Type( + Type::BUILTIN_TYPE_ARRAY, + collection: true, + collectionValueType: [ + new Type(Type::BUILTIN_TYPE_OBJECT, true, ArrayDummy::class), + new Type(Type::BUILTIN_TYPE_OBJECT, class: UnionDummy::class), + ] + ), + ], + ], + + 'array' => [ + [ + ['foo' => 'one', 'bar' => 'two'], + ['foo' => 'three', 'bar' => 'four'], + 'string', + ], + [ + new ArrayDummy('one', 'two'), + new ArrayDummy('three', 'four'), + 'string', + ], + 'mixed[]', + 'json', + [ + 'value_type' => new Type( + Type::BUILTIN_TYPE_ARRAY, + collection: true, + collectionValueType: [ + new Type(Type::BUILTIN_TYPE_OBJECT, class: ArrayDummy::class), + new Type(Type::BUILTIN_TYPE_STRING), + ] + ), + ], + ], + ]; + } } class ArrayDummy @@ -121,3 +179,13 @@ public function __construct($foo, $bar) $this->bar = $bar; } } + +class UnionDummy +{ + public $baz; + + public function __construct($baz) + { + $this->baz = $baz; + } +} diff --git a/src/Symfony/Component/Serializer/Tests/SerializerTest.php b/src/Symfony/Component/Serializer/Tests/SerializerTest.php index e6c0cd7205ee0..b39d24cceb0f7 100644 --- a/src/Symfony/Component/Serializer/Tests/SerializerTest.php +++ b/src/Symfony/Component/Serializer/Tests/SerializerTest.php @@ -1141,7 +1141,7 @@ public function testCollectDenormalizationErrors(?ClassMetadataFactory $classMet 'expectedTypes' => ['array'], 'path' => 'anotherCollection', 'useMessageForUser' => false, - 'message' => 'Data expected to be "Symfony\Component\Serializer\Tests\Fixtures\Php74Full[]", "null" given.', + 'message' => 'Data expected to be "array", "null" given.', ], ]; From 514bcc2abacef4b1edc5800d52c7d02027fbf304 Mon Sep 17 00:00:00 2001 From: Jeroeny Date: Mon, 6 May 2024 13:46:54 +0200 Subject: [PATCH 2/2] Rebase onto 7.1 --- .../Normalizer/AbstractObjectNormalizer.php | 2 +- .../Normalizer/ArrayDenormalizer.php | 32 ++++++------------- .../Serializer/Tests/SerializerTest.php | 6 ++-- 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php index a2fd65fd9ed47..076f61599c22b 100644 --- a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php @@ -462,7 +462,7 @@ private function validateAndDenormalizeLegacy(array $types, string $currentClass $builtinType = $type->getBuiltinType(); if (\is_string($data) && (XmlEncoder::FORMAT === $format || CsvEncoder::FORMAT === $format)) { if ('' === $data) { - if (LegacyLegacyType::BUILTIN_TYPE_ARRAY === $builtinType) { + if (LegacyType::BUILTIN_TYPE_ARRAY === $builtinType) { return []; } diff --git a/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php b/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php index 6b472c26afa58..e4149b559a8e3 100644 --- a/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php @@ -17,8 +17,6 @@ use Symfony\Component\Serializer\Exception\InvalidArgumentException; use Symfony\Component\Serializer\Exception\MissingConstructorArgumentsException; use Symfony\Component\Serializer\Exception\NotNormalizableValueException; -use Symfony\Component\TypeInfo\Type; -use Symfony\Component\TypeInfo\Type\UnionType; /** * Denormalizes arrays of objects. @@ -51,7 +49,7 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a } if (!\is_array($data)) { $valueType = $context['value_type'] ?? null; - $expected = $valueType ? 'array<'.implode('|', array_map(fn (Type $type) => $type->getClassName() ?? $type->getBuiltinType(), $valueType->getCollectionValueTypes())).'>' : $type; + $expected = $valueType ? 'array<'.implode('|', array_map(fn (LegacyType $type) => $type->getClassName() ?? $type->getBuiltinType(), $valueType->getCollectionValueTypes())).'>' : $type; throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('Data expected to be "%s", "%s" given.', $expected, get_debug_type($data)), $data, ['array'], $context['deserialization_path'] ?? null); } @@ -62,18 +60,8 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a $type = substr($type, 0, -2); $valueType = $context['value_type'] ?? null; - # todo - $typeIdentifiers = []; - if (null !== $keyType = ($context['key_type'] ?? null)) { - if ($keyType instanceof Type) { - $typeIdentifiers = array_map(fn (Type $t): string => $t->getBaseType()->getTypeIdentifier()->value, $keyType instanceof UnionType ? $keyType->getTypes() : [$keyType]); - } else { - $typeIdentifiers = array_map(fn (LegacyType $t): string => $t->getBuiltinType(), \is_array($keyType) ? $keyType : [$keyType]); - } - } - - if ($valueType instanceof Type && \count($keyTypes = $valueType->getCollectionKeyTypes()) > 0) { - $builtinTypes = array_map(static fn (Type $keyType) => $keyType->getBuiltinType(), $keyTypes); + if ($valueType instanceof LegacyType && \count($keyTypes = $valueType->getCollectionKeyTypes()) > 0) { + $builtinTypes = array_map(static fn (LegacyType $keyType) => $keyType->getBuiltinType(), $keyTypes); } else { $builtinTypes = array_map(static fn (LegacyType $keyType) => $keyType->getBuiltinType(), \is_array($keyType = $context['key_type'] ?? []) ? $keyType : [$keyType]); } @@ -82,9 +70,9 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a $subContext = $context; $subContext['deserialization_path'] = ($context['deserialization_path'] ?? false) ? sprintf('%s[%s]', $context['deserialization_path'], $key) : "[$key]"; - $this->validateKeyType($typeIdentifiers, $key, $subContext['deserialization_path']); + $this->validateKeyType($builtinTypes, $key, $subContext['deserialization_path']); - if ($valueType instanceof Type) { + if ($valueType instanceof LegacyType) { foreach ($valueType->getCollectionValueTypes() as $subtype) { try { $subContext['value_type'] = $subtype; @@ -95,7 +83,7 @@ public function denormalize(mixed $data, string $type, ?string $format = null, a continue 2; } - if (Type::BUILTIN_TYPE_ARRAY === $subtype->getBuiltinType()) { + if (LegacyType::BUILTIN_TYPE_ARRAY === $subtype->getBuiltinType()) { $class = $type; } else { $class = $subtype->getClassName() ?? $subtype->getBuiltinType(); @@ -128,20 +116,20 @@ public function supportsDenormalization(mixed $data, string $type, ?string $form } /** - * @param list $typeIdentifiers + * @param list $builtinTypes */ private function validateKeyType(array $builtinTypes, mixed $key, string $path): void { - if (!$typeIdentifiers) { + if (!$builtinTypes) { return; } - foreach ($typeIdentifiers as $typeIdentifier) { + foreach ($builtinTypes as $typeIdentifier) { if (('is_'.$typeIdentifier)($key)) { return; } } - throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('The type of the key "%s" must be "%s" ("%s" given).', $key, implode('", "', $typeIdentifiers), get_debug_type($key)), $key, $typeIdentifiers, $path, true); + throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('The type of the key "%s" must be "%s" ("%s" given).', $key, implode('", "', $builtinTypes), get_debug_type($key)), $key, $builtinTypes, $path, true); } } diff --git a/src/Symfony/Component/Serializer/Tests/SerializerTest.php b/src/Symfony/Component/Serializer/Tests/SerializerTest.php index b39d24cceb0f7..526871ba5e354 100644 --- a/src/Symfony/Component/Serializer/Tests/SerializerTest.php +++ b/src/Symfony/Component/Serializer/Tests/SerializerTest.php @@ -1214,7 +1214,7 @@ public function testCollectDenormalizationErrors2(?ClassMetadataFactory $classMe 'useMessageForUser' => false, 'message' => 'The type of the "string" attribute for class "Symfony\\Component\\Serializer\\Tests\\Fixtures\\Php74Full" must be one of "string" ("null" given).', ], - ]; + ]; $this->assertSame($expected, $exceptionsAsArray); } @@ -1464,8 +1464,8 @@ public function testCollectDenormalizationErrorsWithWrongPropertyWithoutConstruc try { $serializer->deserialize('{"get": "POST"}', DummyObjectWithEnumProperty::class, 'json', [ - DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true, - ]); + DenormalizerInterface::COLLECT_DENORMALIZATION_ERRORS => true, + ]); } catch (\Throwable $e) { $this->assertInstanceOf(PartialDenormalizationException::class, $e); }