Skip to content

Commit 8f7c7ae

Browse files
mtarldmtarld
committed
[Serializer] Fix denormalize constructor arguments
1 parent 0a30c9b commit 8f7c7ae

File tree

5 files changed

+61
-15
lines changed

5 files changed

+61
-15
lines changed

‎src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php

+22-7
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ protected function instantiateObject(array &$data, string $class, array &$contex
350350
$constructorParameters = $constructor->getParameters();
351351
$missingConstructorArguments = [];
352352
$params = [];
353+
$unsetKeys = [];
353354
foreach ($constructorParameters as $constructorParameter) {
354355
$paramName = $constructorParameter->name;
355356
$key = $this->nameConverter ? $this->nameConverter->normalize($paramName, $class, $format, $context) : $paramName;
@@ -368,18 +369,17 @@ protected function instantiateObject(array &$data, string $class, array &$contex
368369
}
369370

370371
$params = array_merge($params, $variadicParameters);
371-
unset($data[$key]);
372+
$unsetKeys[] = $key;
372373
}
373374
} elseif ($allowed && !$ignored && (isset($data[$key]) || \array_key_exists($key, $data))) {
374375
$parameterData = $data[$key];
375376
if (null === $parameterData && $constructorParameter->allowsNull()) {
376377
$params[] = null;
377-
// Don't run set for a parameter passed to the constructor
378-
unset($data[$key]);
378+
$unsetKeys[] = $key;
379+
379380
continue;
380381
}
381382

382-
// Don't run set for a parameter passed to the constructor
383383
try {
384384
$params[] = $this->denormalizeParameter($reflectionClass, $constructorParameter, $paramName, $parameterData, $context, $format);
385385
} catch (NotNormalizableValueException $exception) {
@@ -390,7 +390,8 @@ protected function instantiateObject(array &$data, string $class, array &$contex
390390
$context['not_normalizable_value_exceptions'][] = $exception;
391391
$params[] = $parameterData;
392392
}
393-
unset($data[$key]);
393+
394+
$unsetKeys[] = $key;
394395
} elseif (\array_key_exists($key, $context[static::DEFAULT_CONSTRUCTOR_ARGUMENTS][$class] ?? [])) {
395396
$params[] = $context[static::DEFAULT_CONSTRUCTOR_ARGUMENTS][$class][$key];
396397
} 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
421422
}
422423

423424
if (!$constructor->isConstructor()) {
424-
return $constructor->invokeArgs(null, $params);
425+
$instance = $constructor->invokeArgs(null, $params);
426+
427+
// do not set a parameter that has been set in the constructor
428+
foreach ($unsetKeys as $key) {
429+
unset($data[$key]);
430+
}
431+
432+
return $instance;
425433
}
426434

427435
try {
428-
return $reflectionClass->newInstanceArgs($params);
436+
$instance = $reflectionClass->newInstanceArgs($params);
437+
438+
// do not set a parameter that has been set in the constructor
439+
foreach ($unsetKeys as $key) {
440+
unset($data[$key]);
441+
}
442+
443+
return $instance;
429444
} catch (\TypeError $e) {
430445
if (!isset($context['not_normalizable_value_exceptions'])) {
431446
throw $e;

‎src/Symfony/Component/Serializer/Serializer.php

+13-1
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,20 @@ public function denormalize($data, string $type, string $format = null, array $c
228228
$context['not_normalizable_value_exceptions'] = [];
229229
$errors = &$context['not_normalizable_value_exceptions'];
230230
$denormalized = $normalizer->denormalize($data, $type, $format, $context);
231+
231232
if ($errors) {
232-
throw new PartialDenormalizationException($denormalized, $errors);
233+
// merge errors so that one path has only one error
234+
$uniqueErrors = [];
235+
foreach ($errors as $error) {
236+
if (null === $error->getPath()) {
237+
$uniqueErrors[] = $error;
238+
continue;
239+
}
240+
241+
$uniqueErrors[$error->getPath()] = $uniqueErrors[$error->getPath()] ?? $error;
242+
}
243+
244+
throw new PartialDenormalizationException($denormalized, array_values($uniqueErrors));
233245
}
234246

235247
return $denormalized;

‎src/Symfony/Component/Serializer/Tests/Fixtures/Php74Full.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public function __construct($constructorArgument)
4646

4747
final class Php74FullWithTypedConstructor
4848
{
49-
public function __construct(float $something)
49+
public function __construct(float $something, bool $somethingElse)
5050
{
5151
}
5252
}

‎src/Symfony/Component/Serializer/Tests/Normalizer/Features/ConstructorArgumentsTestTrait.php

+5-5
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,23 @@ public function testMetadataAwareNameConvertorWithNotSerializedConstructorParame
5858
public function testConstructorWithMissingData()
5959
{
6060
$data = [
61-
'foo' => 10,
61+
'bar' => 10,
6262
];
6363

6464
$normalizer = $this->getDenormalizerForConstructArguments();
6565
try {
6666
$normalizer->denormalize($data, ConstructorArgumentsObject::class);
6767
self::fail(sprintf('Failed asserting that exception of type "%s" is thrown.', MissingConstructorArgumentsException::class));
6868
} catch (MissingConstructorArgumentsException $e) {
69-
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());
70-
self::assertSame(['bar', 'baz'], $e->getMissingConstructorArguments());
69+
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());
70+
self::assertSame(['foo', 'baz'], $e->getMissingConstructorArguments());
7171
}
7272
}
7373

7474
public function testExceptionsAreCollectedForConstructorWithMissingData()
7575
{
7676
$data = [
77-
'foo' => 10,
77+
'bar' => 10,
7878
];
7979

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

8787
self::assertCount(2, $exceptions);
88-
self::assertSame('Failed to create object because the class misses the "bar" property.', $exceptions[0]->getMessage());
88+
self::assertSame('Failed to create object because the class misses the "foo" property.', $exceptions[0]->getMessage());
8989
self::assertSame('Failed to create object because the class misses the "baz" property.', $exceptions[1]->getMessage());
9090
}
9191
}

‎src/Symfony/Component/Serializer/Tests/SerializerTest.php

+20-1
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,8 @@ public function testCollectDenormalizationErrors(?ClassMetadataFactory $classMet
868868
],
869869
"php74FullWithConstructor": {},
870870
"php74FullWithTypedConstructor": {
871-
"something": "not a float"
871+
"something": "not a float",
872+
"somethingElse": "not a bool"
872873
},
873874
"dummyMessage": {
874875
},
@@ -1032,6 +1033,24 @@ public function testCollectDenormalizationErrors(?ClassMetadataFactory $classMet
10321033
'useMessageForUser' => false,
10331034
'message' => 'The type of the "something" attribute for class "Symfony\Component\Serializer\Tests\Fixtures\Php74FullWithTypedConstructor" must be one of "float" ("string" given).',
10341035
],
1036+
[
1037+
'currentType' => 'string',
1038+
'expectedTypes' => [
1039+
'float',
1040+
],
1041+
'path' => 'php74FullWithTypedConstructor.something',
1042+
'useMessageForUser' => false,
1043+
'message' => 'The type of the "something" attribute for class "Symfony\Component\Serializer\Tests\Fixtures\Php74FullWithTypedConstructor" must be one of "float" ("string" given).',
1044+
],
1045+
[
1046+
'currentType' => 'string',
1047+
'expectedTypes' => [
1048+
'bool',
1049+
],
1050+
'path' => 'php74FullWithTypedConstructor.somethingElse',
1051+
'useMessageForUser' => false,
1052+
'message' => 'The type of the "somethingElse" attribute for class "Symfony\Component\Serializer\Tests\Fixtures\Php74FullWithTypedConstructor" must be one of "bool" ("string" given).',
1053+
],
10351054
$classMetadataFactory ?
10361055
[
10371056
'currentType' => 'null',

0 commit comments

Comments
 (0)