bug #50193 [Serializer] Fix SerializedPath not working with constru…

…ctor arguments (HypeMC) This PR was merged into the 6.2 branch. Discussion ---------- [Serializer] Fix `SerializedPath` not working with constructor arguments | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Currently the `#[SerializedPath]` attribute doesn't work with constructor arguments: ```php class Test { public function __construct( #[SerializedPath('[foo][bar]')] public string $bar, ) { } } $serializer->denormalize(['foo' => ['bar' => 'something']], Test::class); ``` ``` In AbstractNormalizer.php line 384: Cannot create an instance of "Test" from serialized data because its constructor requires parameter "bar" to be present. ``` Commits ------- 240c03130b [Serializer] Fix SerializedPath not working with constructor arguments
symfony · May 19, 2023 · 7777df2 · 7777df2
2 parents b56bab1 + ff4111b
commit 7777df2
Show file tree

Hide file tree

Showing 10 changed files with 298 additions and 35 deletions.
diff --git a/Normalizer/AbstractObjectNormalizer.php b/Normalizer/AbstractObjectNormalizer.php
@@ -128,8 +128,8 @@ public function __construct(ClassMetadataFactoryInterface $classMetadataFactory
 
         $this->propertyTypeExtractor = $propertyTypeExtractor;
 
-        if (null === $classDiscriminatorResolver && null !== $classMetadataFactory) {
-            $classDiscriminatorResolver = new ClassDiscriminatorFromClassMetadata($classMetadataFactory);
+        if ($classMetadataFactory) {
+            $classDiscriminatorResolver ??= new ClassDiscriminatorFromClassMetadata($classMetadataFactory);
         }
         $this->classDiscriminatorResolver = $classDiscriminatorResolver;
         $this->objectClassResolver = $objectClassResolver;
@@ -217,7 +217,7 @@ public function normalize(mixed $object, string $format = null, array $context =
         }
 
         $preserveEmptyObjects = $context[self::PRESERVE_EMPTY_OBJECTS] ?? $this->defaultContext[self::PRESERVE_EMPTY_OBJECTS] ?? false;
-        if ($preserveEmptyObjects && !\count($data)) {
+        if ($preserveEmptyObjects && !$data) {
             return new \ArrayObject();
         }
 
@@ -226,19 +226,8 @@ public function normalize(mixed $object, string $format = null, array $context =
 
     protected function instantiateObject(array &$data, string $class, array &$context, \ReflectionClass $reflectionClass, array|bool $allowedAttributes, string $format = null)
     {
-        if ($this->classDiscriminatorResolver && $mapping = $this->classDiscriminatorResolver->getMappingForClass($class)) {
-            if (!isset($data[$mapping->getTypeProperty()])) {
-                throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('Type property "%s" not found for the abstract object "%s".', $mapping->getTypeProperty(), $class), null, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), false);
-            }
-
-            $type = $data[$mapping->getTypeProperty()];
-            if (null === ($mappedClass = $mapping->getClassForType($type))) {
-                throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('The type "%s" is not a valid value.', $type), $type, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), true);
-            }
-
-            if ($mappedClass !== $class) {
-                return $this->instantiateObject($data, $mappedClass, $context, new \ReflectionClass($mappedClass), $allowedAttributes, $format);
-            }
+        if ($class !== $mappedClass = $this->getMappedClass($data, $class, $context)) {
+            return $this->instantiateObject($data, $mappedClass, $context, new \ReflectionClass($mappedClass), $allowedAttributes, $format);
         }
 
         return parent::instantiateObject($data, $class, $context, $reflectionClass, $allowedAttributes, $format);
@@ -270,7 +259,7 @@ protected function getAttributes(object $object, ?string $format, array $context
 
         $attributes = $this->extractAttributes($object, $format, $context);
 
-        if ($this->classDiscriminatorResolver && $mapping = $this->classDiscriminatorResolver->getMappingForMappedObject($object)) {
+        if ($mapping = $this->classDiscriminatorResolver?->getMappingForMappedObject($object)) {
             array_unshift($attributes, $mapping->getTypeProperty());
         }
 
@@ -319,11 +308,9 @@ public function denormalize(mixed $data, string $type, string $format = null, ar
         $normalizedData = $this->prepareForDenormalization($data);
         $extraAttributes = [];
 
-        $reflectionClass = new \ReflectionClass($type);
-        $object = $this->instantiateObject($normalizedData, $type, $context, $reflectionClass, $allowedAttributes, $format);
-        $resolvedClass = $this->objectClassResolver ? ($this->objectClassResolver)($object) : $object::class;
+        $mappedClass = $this->getMappedClass($normalizedData, $type, $context);
 
-        $nestedAttributes = $this->getNestedAttributes($resolvedClass);
+        $nestedAttributes = $this->getNestedAttributes($mappedClass);
         $nestedData = [];
         $propertyAccessor = PropertyAccess::createPropertyAccessor();
         foreach ($nestedAttributes as $property => $serializedPath) {
@@ -336,6 +323,9 @@ public function denormalize(mixed $data, string $type, string $format = null, ar
 
         $normalizedData = array_merge($normalizedData, $nestedData);
 
+        $object = $this->instantiateObject($normalizedData, $mappedClass, $context, new \ReflectionClass($mappedClass), $allowedAttributes, $format);
+        $resolvedClass = $this->objectClassResolver ? ($this->objectClassResolver)($object) : $object::class;
+
         foreach ($normalizedData as $attribute => $value) {
             if ($this->nameConverter) {
                 $notConverted = $attribute;
@@ -675,11 +665,8 @@ private function updateData(array $data, string $attribute, mixed $attributeValu
      */
     private function isMaxDepthReached(array $attributesMetadata, string $class, string $attribute, array &$context): bool
     {
-        $enableMaxDepth = $context[self::ENABLE_MAX_DEPTH] ?? $this->defaultContext[self::ENABLE_MAX_DEPTH] ?? false;
-        if (
-            !$enableMaxDepth ||
-            !isset($attributesMetadata[$attribute]) ||
-            null === $maxDepth = $attributesMetadata[$attribute]->getMaxDepth()
+        if (!($enableMaxDepth = $context[self::ENABLE_MAX_DEPTH] ?? $this->defaultContext[self::ENABLE_MAX_DEPTH] ?? false)
+            || null === $maxDepth = $attributesMetadata[$attribute]?->getMaxDepth()
         ) {
             return false;
         }
@@ -755,7 +742,7 @@ private function isUninitializedValueError(\Error $e): bool
      */
     private function getNestedAttributes(string $class): array
     {
-        if (!$this->classMetadataFactory || !$this->classMetadataFactory->hasMetadataFor($class)) {
+        if (!$this->classMetadataFactory?->hasMetadataFor($class)) {
             return [];
         }
 
@@ -781,15 +768,30 @@ private function getNestedAttributes(string $class): array
     private function removeNestedValue(array $path, array $data): array
     {
         $element = array_shift($path);
-        if ([] === $path) {
+        if (!$path || !$data[$element] = $this->removeNestedValue($path, $data[$element])) {
             unset($data[$element]);
-        } else {
-            $data[$element] = $this->removeNestedValue($path, $data[$element]);
-            if ([] === $data[$element]) {
-                unset($data[$element]);
-            }
         }
 
         return $data;
     }
+
+    /**
+     * @return class-string
+     */
+    private function getMappedClass(array $data, string $class, array $context): string
+    {
+        if (!$mapping = $this->classDiscriminatorResolver?->getMappingForClass($class)) {
+            return $class;
+        }
+
+        if (null === $type = $data[$mapping->getTypeProperty()] ?? null) {
+            throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('Type property "%s" not found for the abstract object "%s".', $mapping->getTypeProperty(), $class), null, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), false);
+        }
+
+        if (null === $mappedClass = $mapping->getClassForType($type)) {
+            throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('The type "%s" is not a valid value.', $type), $type, ['string'], isset($context['deserialization_path']) ? $context['deserialization_path'].'.'.$mapping->getTypeProperty() : $mapping->getTypeProperty(), true);
+        }
+
+        return $mappedClass;
+    }
 }
diff --git a/Tests/Fixtures/Annotations/SerializedPathInConstructorDummy.php b/Tests/Fixtures/Annotations/SerializedPathInConstructorDummy.php
@@ -0,0 +1,25 @@
+<?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\Annotations;
+
+use Symfony\Component\Serializer\Annotation\SerializedPath;
+
+class SerializedPathInConstructorDummy
+{
+    public function __construct(
+        /**
+         * @SerializedPath("[one][two]")
+         */
+        public $three,
+    ) {
+    }
+}
diff --git a/Tests/Fixtures/Attributes/SerializedPathInConstructorDummy.php b/Tests/Fixtures/Attributes/SerializedPathInConstructorDummy.php
@@ -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\Attributes;
+
+use Symfony\Component\Serializer\Annotation\SerializedPath;
+
+class SerializedPathInConstructorDummy
+{
+    public function __construct(
+        #[SerializedPath('[one][two]')] public $three,
+    ) {
+    }
+}
diff --git a/Tests/Fixtures/serialization.xml b/Tests/Fixtures/serialization.xml
@@ -30,6 +30,10 @@
         <attribute name="seven" serialized-path="[three][four]" />
     </class>
 
+    <class name="Symfony\Component\Serializer\Tests\Fixtures\Annotations\SerializedPathInConstructorDummy">
+        <attribute name="three" serialized-path="[one][two]" />
+    </class>
+
     <class name="Symfony\Component\Serializer\Tests\Fixtures\Annotations\AbstractDummy">
         <discriminator-map type-property="type">
             <mapping type="first" class="Symfony\Component\Serializer\Tests\Fixtures\Annotations\AbstractDummyFirstChild" />

diff --git a/Tests/Fixtures/serialization.yml b/Tests/Fixtures/serialization.yml
@@ -22,6 +22,10 @@
       serialized_path: '[one][two]'
     seven:
       serialized_path: '[three][four]'
+'Symfony\Component\Serializer\Tests\Fixtures\Annotations\SerializedPathInConstructorDummy':
+  attributes:
+    three:
+      serialized_path: '[one][two]'
 'Symfony\Component\Serializer\Tests\Fixtures\Annotations\AbstractDummy':
   discriminator_map:
     type_property: type

diff --git a/Tests/Mapping/Factory/ClassMetadataFactoryCompilerTest.php b/Tests/Mapping/Factory/ClassMetadataFactoryCompilerTest.php
@@ -19,6 +19,7 @@
 use Symfony\Component\Serializer\Tests\Fixtures\Annotations\MaxDepthDummy;
 use Symfony\Component\Serializer\Tests\Fixtures\Annotations\SerializedNameDummy;
 use Symfony\Component\Serializer\Tests\Fixtures\Annotations\SerializedPathDummy;
+use Symfony\Component\Serializer\Tests\Fixtures\Annotations\SerializedPathInConstructorDummy;
 use Symfony\Component\Serializer\Tests\Fixtures\Dummy;
 
 final class ClassMetadataFactoryCompilerTest extends TestCase
@@ -46,18 +47,20 @@ public function testItDumpMetadata()
         $maxDepthDummyMetadata = $classMetatadataFactory->getMetadataFor(MaxDepthDummy::class);
         $serializedNameDummyMetadata = $classMetatadataFactory->getMetadataFor(SerializedNameDummy::class);
         $serializedPathDummyMetadata = $classMetatadataFactory->getMetadataFor(SerializedPathDummy::class);
+        $serializedPathInConstructorDummyMetadata = $classMetatadataFactory->getMetadataFor(SerializedPathInConstructorDummy::class);
 
         $code = (new ClassMetadataFactoryCompiler())->compile([
             $dummyMetadata,
             $maxDepthDummyMetadata,
             $serializedNameDummyMetadata,
             $serializedPathDummyMetadata,
+            $serializedPathInConstructorDummyMetadata,
         ]);
 
         file_put_contents($this->dumpPath, $code);
         $compiledMetadata = require $this->dumpPath;
 
-        $this->assertCount(4, $compiledMetadata);
+        $this->assertCount(5, $compiledMetadata);
 
         $this->assertArrayHasKey(Dummy::class, $compiledMetadata);
         $this->assertEquals([
@@ -99,5 +102,13 @@ public function testItDumpMetadata()
             ],
             null,
         ], $compiledMetadata[SerializedPathDummy::class]);
+
+        $this->assertArrayHasKey(SerializedPathInConstructorDummy::class, $compiledMetadata);
+        $this->assertEquals([
+            [
+                'three' => [[], null, null, '[one][two]'],
+            ],
+            null,
+        ], $compiledMetadata[SerializedPathInConstructorDummy::class]);
     }
 }
diff --git a/Tests/Mapping/Loader/AnnotationLoaderTestCase.php b/Tests/Mapping/Loader/AnnotationLoaderTestCase.php
@@ -103,6 +103,15 @@ public function testLoadSerializedPath()
         $this->assertEquals(new PropertyPath('[three][four]'), $attributesMetadata['seven']->getSerializedPath());
     }
 
+    public function testLoadSerializedPathInConstructor()
+    {
+        $classMetadata = new ClassMetadata($this->getNamespace().'\SerializedPathInConstructorDummy');
+        $this->loader->loadClassMetadata($classMetadata);
+
+        $attributesMetadata = $classMetadata->getAttributesMetadata();
+        $this->assertEquals(new PropertyPath('[one][two]'), $attributesMetadata['three']->getSerializedPath());
+    }
+
     public function testLoadClassMetadataAndMerge()
     {
         $classMetadata = new ClassMetadata($this->getNamespace().'\GroupDummy');

diff --git a/Tests/Mapping/Loader/XmlFileLoaderTest.php b/Tests/Mapping/Loader/XmlFileLoaderTest.php
@@ -94,6 +94,15 @@ public function testSerializedPath()
         $this->assertEquals('[three][four]', $attributesMetadata['seven']->getSerializedPath());
     }
 
+    public function testSerializedPathInConstructor()
+    {
+        $classMetadata = new ClassMetadata('Symfony\Component\Serializer\Tests\Fixtures\Annotations\SerializedPathInConstructorDummy');
+        $this->loader->loadClassMetadata($classMetadata);
+
+        $attributesMetadata = $classMetadata->getAttributesMetadata();
+        $this->assertEquals('[one][two]', $attributesMetadata['three']->getSerializedPath());
+    }
+
     public function testLoadDiscriminatorMap()
     {
         $classMetadata = new ClassMetadata(AbstractDummy::class);

diff --git a/Tests/Mapping/Loader/YamlFileLoaderTest.php b/Tests/Mapping/Loader/YamlFileLoaderTest.php
@@ -108,6 +108,15 @@ public function testSerializedPath()
         $this->assertEquals(new PropertyPath('[three][four]'), $attributesMetadata['seven']->getSerializedPath());
     }
 
+    public function testSerializedPathInConstructor()
+    {
+        $classMetadata = new ClassMetadata('Symfony\Component\Serializer\Tests\Fixtures\Annotations\SerializedPathInConstructorDummy');
+        $this->loader->loadClassMetadata($classMetadata);
+
+        $attributesMetadata = $classMetadata->getAttributesMetadata();
+        $this->assertEquals(new PropertyPath('[one][two]'), $attributesMetadata['three']->getSerializedPath());
+    }
+
     public function testLoadDiscriminatorMap()
     {
         $classMetadata = new ClassMetadata(AbstractDummy::class);