[Serializer] Fix denormalize constructor arguments

symfony · nicolas-grekas · Nov 20, 2023 · Nov 14, 2023 · Nov 20, 2023 · 8f7c7aef8f5e87b0cd565f8046e70df8a5fb79ed
commit 8f7c7aef8f5e87b0cd565f8046e70df8a5fb79ed
@@ -350,6 +350,7 @@ protected function instantiateObject(array &$data, string $class, array &$contex
            $constructorParameters = $constructor->getParameters();
            $missingConstructorArguments = [];
            $params = [];
+            $unsetKeys = [];
            foreach ($constructorParameters as $constructorParameter) {
                $paramName = $constructorParameter->name;
                $key = $this->nameConverter ? $this->nameConverter->normalize($paramName, $class, $format, $context) : $paramName;
@@ -368,18 +369,17 @@ protected function instantiateObject(array &$data, string $class, array &$contex
                        }

                        $params = array_merge($params, $variadicParameters);
-                        unset($data[$key]);
+                        $unsetKeys[] = $key;
                    }
                } elseif ($allowed && !$ignored && (isset($data[$key]) || \array_key_exists($key, $data))) {
                    $parameterData = $data[$key];
                    if (null === $parameterData && $constructorParameter->allowsNull()) {
                        $params[] = null;
-                        // Don't run set for a parameter passed to the constructor
-                        unset($data[$key]);
+                        $unsetKeys[] = $key;
+
                        continue;
                    }

-                    // Don't run set for a parameter passed to the constructor
                    try {
                        $params[] = $this->denormalizeParameter($reflectionClass, $constructorParameter, $paramName, $parameterData, $context, $format);
                    } catch (NotNormalizableValueException $exception) {
@@ -390,7 +390,8 @@ protected function instantiateObject(array &$data, string $class, array &$contex
                        $context['not_normalizable_value_exceptions'][] = $exception;
                        $params[] = $parameterData;
                    }
-                    unset($data[$key]);
+
+                    $unsetKeys[] = $key;
                } elseif (\array_key_exists($key, $context[static::DEFAULT_CONSTRUCTOR_ARGUMENTS][$class] ?? [])) {
                    $params[] = $context[static::DEFAULT_CONSTRUCTOR_ARGUMENTS][$class][$key];
                } elseif (\array_key_exists($key, $this->defaultContext[self::DEFAULT_CONSTRUCTOR_ARGUMENTS][$class] ?? [])) {
@@ -421,11 +422,25 @@ protected function instantiateObject(array &$data, string $class, array &$contex
            }

            if (!$constructor->isConstructor()) {
-                return $constructor->invokeArgs(null, $params);
+                $instance = $constructor->invokeArgs(null, $params);
+
+                // do not set a parameter that has been set in the constructor
+                foreach ($unsetKeys as $key) {
+                    unset($data[$key]);
+                }
+
+                return $instance;
            }

            try {
-                return $reflectionClass->newInstanceArgs($params);
+                $instance = $reflectionClass->newInstanceArgs($params);
+
+                // do not set a parameter that has been set in the constructor
+                foreach ($unsetKeys as $key) {
+                    unset($data[$key]);
+                }
+
+                return $instance;
            } catch (\TypeError $e) {
                if (!isset($context['not_normalizable_value_exceptions'])) {
                    throw $e;

@@ -228,8 +228,20 @@ public function denormalize($data, string $type, string $format = null, array $c
            $context['not_normalizable_value_exceptions'] = [];
            $errors = &$context['not_normalizable_value_exceptions'];
            $denormalized = $normalizer->denormalize($data, $type, $format, $context);
+
            if ($errors) {
-                throw new PartialDenormalizationException($denormalized, $errors);
+                // merge errors so that one path has only one error
+                $uniqueErrors = [];
+                foreach ($errors as $error) {
+                    if (null === $error->getPath()) {
+                        $uniqueErrors[] = $error;
+                        continue;
+                    }
+
+                    $uniqueErrors[$error->getPath()] = $uniqueErrors[$error->getPath()] ?? $error;
+                }
+
+                throw new PartialDenormalizationException($denormalized, array_values($uniqueErrors));
            }

            return $denormalized;

@@ -46,7 +46,7 @@ public function __construct($constructorArgument)

 final class Php74FullWithTypedConstructor
 {
-    public function __construct(float $something)
+    public function __construct(float $something, bool $somethingElse)
    {
    }
 }

@@ -58,23 +58,23 @@ public function testMetadataAwareNameConvertorWithNotSerializedConstructorParame
    public function testConstructorWithMissingData()
    {
        $data = [
-            'foo' => 10,
+            'bar' => 10,
        ];

        $normalizer = $this->getDenormalizerForConstructArguments();
        try {
            $normalizer->denormalize($data, ConstructorArgumentsObject::class);
            self::fail(sprintf('Failed asserting that exception of type "%s" is thrown.', MissingConstructorArgumentsException::class));
        } catch (MissingConstructorArgumentsException $e) {
-            self::assertSame(sprintf('Cannot create an instance of "%s" from serialized data because its constructor requires the following parameters to be present : "$bar", "$baz".', ConstructorArgumentsObject::class), $e->getMessage());
-            self::assertSame(['bar', 'baz'], $e->getMissingConstructorArguments());
+            self::assertSame(sprintf('Cannot create an instance of "%s" from serialized data because its constructor requires the following parameters to be present : "$foo", "$baz".', ConstructorArgumentsObject::class), $e->getMessage());
+            self::assertSame(['foo', 'baz'], $e->getMissingConstructorArguments());
        }
    }

    public function testExceptionsAreCollectedForConstructorWithMissingData()
    {
        $data = [
-            'foo' => 10,
+            'bar' => 10,
        ];

        $exceptions = [];
@@ -85,7 +85,7 @@ public function testExceptionsAreCollectedForConstructorWithMissingData()
        ]);

        self::assertCount(2, $exceptions);
-        self::assertSame('Failed to create object because the class misses the "bar" property.', $exceptions[0]->getMessage());
+        self::assertSame('Failed to create object because the class misses the "foo" property.', $exceptions[0]->getMessage());
        self::assertSame('Failed to create object because the class misses the "baz" property.', $exceptions[1]->getMessage());
    }
 }
@@ -868,7 +868,8 @@ public function testCollectDenormalizationErrors(?ClassMetadataFactory $classMet
            ],
            "php74FullWithConstructor": {},
            "php74FullWithTypedConstructor": {
-                "something": "not a float"
+                "something": "not a float",
+                "somethingElse": "not a bool"
            },
            "dummyMessage": {
            },
@@ -1032,6 +1033,24 @@ public function testCollectDenormalizationErrors(?ClassMetadataFactory $classMet
                'useMessageForUser' => false,
                'message' => 'The type of the "something" attribute for class "Symfony\Component\Serializer\Tests\Fixtures\Php74FullWithTypedConstructor" must be one of "float" ("string" given).',
            ],
+            [
+                'currentType' => 'string',
+                'expectedTypes' => [
+                    'float',
+                ],
+                'path' => 'php74FullWithTypedConstructor.something',
+                'useMessageForUser' => false,
+                'message' => 'The type of the "something" attribute for class "Symfony\Component\Serializer\Tests\Fixtures\Php74FullWithTypedConstructor" must be one of "float" ("string" given).',
+            ],
+            [
+                'currentType' => 'string',
+                'expectedTypes' => [
+                    'bool',
+                ],
+                'path' => 'php74FullWithTypedConstructor.somethingElse',
+                'useMessageForUser' => false,
+                'message' => 'The type of the "somethingElse" attribute for class "Symfony\Component\Serializer\Tests\Fixtures\Php74FullWithTypedConstructor" must be one of "bool" ("string" given).',
+            ],
            $classMetadataFactory ?
                [
                    'currentType' => 'null',