Skip to content

Commit 5d50fa8

Browse files
fabpotfabpot
committed
feature symfony#52879 [PropertyAccess][Serializer] Fix "type unknown" on denormalize (seferov)
This PR was squashed before being merged into the 7.1 branch. Discussion ---------- [PropertyAccess][Serializer] Fix "type unknown" on denormalize | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | License | MIT The bug can clearly be seen on tests for `RequestPayloadValueResolver` on https://github.com/symfony/symfony/blob/6.3/src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php#L255 ```php // ... public function testValidationNotPassed() { $content = '{"price": 50, "title": ["not a string"]}'; $payload = new RequestPayload(50); $serializer = new Serializer([new ObjectNormalizer()], ['json' => new JsonEncoder()]); // removed for simplification try { $resolver->onKernelControllerArguments($event); $this->fail(sprintf('Expected "%s" to be thrown.', HttpException::class)); } catch (HttpException $e) { $validationFailedException = $e->getPrevious(); $this->assertInstanceOf(ValidationFailedException::class, $validationFailedException); $this->assertSame('This value should be of type unknown.', $validationFailedException->getViolations()[0]->getMessage()); $this->assertSame('Test', $validationFailedException->getViolations()[1]->getMessage()); } } } class RequestPayload { #[Assert\Length(min: 10, groups: ['strict'])] public string $title; public function __construct(public readonly float $price) { } } ``` Even though `title` is string, the error message shown to user is ambiguous: "This value should be of type unknown.". Instead, the message should be "This value should be of type string." Commits ------- 4a665ac [PropertyAccess][Serializer] Fix "type unknown" on denormalize
2 parents 811cce7 + 4a665ac commit 5d50fa8

File tree

6 files changed

+61
-8
lines changed

6 files changed

+61
-8
lines changed

‎src/Symfony/Component/HttpKernel/Tests/Controller/ArgumentResolver/RequestPayloadValueResolverTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ public function testValidationNotPassed()
253253
$validationFailedException = $e->getPrevious();
254254
$this->assertSame(422, $e->getStatusCode());
255255
$this->assertInstanceOf(ValidationFailedException::class, $validationFailedException);
256-
$this->assertSame('This value should be of type unknown.', $validationFailedException->getViolations()[0]->getMessage());
256+
$this->assertSame('This value should be of type string.', $validationFailedException->getViolations()[0]->getMessage());
257257
$this->assertSame('Test', $validationFailedException->getViolations()[1]->getMessage());
258258
}
259259
}
@@ -665,7 +665,7 @@ public function testRequestPayloadValidationErrorCustomStatusCode()
665665
$validationFailedException = $e->getPrevious();
666666
$this->assertSame(400, $e->getStatusCode());
667667
$this->assertInstanceOf(ValidationFailedException::class, $validationFailedException);
668-
$this->assertSame('This value should be of type unknown.', $validationFailedException->getViolations()[0]->getMessage());
668+
$this->assertSame('This value should be of type string.', $validationFailedException->getViolations()[0]->getMessage());
669669
$this->assertSame('Test', $validationFailedException->getViolations()[1]->getMessage());
670670
}
671671
}

‎src/Symfony/Component/HttpKernel/composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@
3535
"symfony/finder": "^6.4|^7.0",
3636
"symfony/http-client-contracts": "^2.5|^3",
3737
"symfony/process": "^6.4|^7.0",
38-
"symfony/property-access": "^6.4|^7.0",
38+
"symfony/property-access": "^7.1",
3939
"symfony/routing": "^6.4|^7.0",
40-
"symfony/serializer": "^6.4|^7.0",
40+
"symfony/serializer": "^7.1",
4141
"symfony/stopwatch": "^6.4|^7.0",
4242
"symfony/translation": "^6.4|^7.0",
4343
"symfony/translation-contracts": "^2.5|^3",

‎src/Symfony/Component/PropertyAccess/Exception/InvalidTypeException.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\PropertyAccess\Exception;
13+
14+
/**
15+
* Thrown when a type of given value does not match an expected type.
16+
*
17+
* @author Farhad Safarov <farhad.safarov@gmail.com>
18+
*/
19+
class InvalidTypeException extends InvalidArgumentException
20+
{
21+
public function __construct(
22+
public readonly string $expectedType,
23+
public readonly string $actualType,
24+
public readonly string $propertyPath,
25+
\Throwable $previous = null,
26+
) {
27+
parent::__construct(
28+
sprintf('Expected argument of type "%s", "%s" given at property path "%s".', $expectedType, 'NULL' === $actualType ? 'null' : $actualType, $propertyPath),
29+
previous: $previous,
30+
);
31+
}
32+
}

‎src/Symfony/Component/PropertyAccess/PropertyAccessor.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
use Symfony\Component\Cache\Adapter\ApcuAdapter;
1919
use Symfony\Component\Cache\Adapter\NullAdapter;
2020
use Symfony\Component\PropertyAccess\Exception\AccessException;
21-
use Symfony\Component\PropertyAccess\Exception\InvalidArgumentException;
21+
use Symfony\Component\PropertyAccess\Exception\InvalidTypeException;
2222
use Symfony\Component\PropertyAccess\Exception\NoSuchIndexException;
2323
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
2424
use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;
@@ -192,12 +192,12 @@ private static function throwInvalidArgumentException(string $message, array $tr
192192
if (preg_match('/^\S+::\S+\(\): Argument #\d+ \(\$\S+\) must be of type (\S+), (\S+) given/', $message, $matches)) {
193193
[, $expectedType, $actualType] = $matches;
194194

195-
throw new InvalidArgumentException(sprintf('Expected argument of type "%s", "%s" given at property path "%s".', $expectedType, 'NULL' === $actualType ? 'null' : $actualType, $propertyPath), 0, $previous);
195+
throw new InvalidTypeException($expectedType, $actualType, $propertyPath, $previous);
196196
}
197197
if (preg_match('/^Cannot assign (\S+) to property \S+::\$\S+ of type (\S+)$/', $message, $matches)) {
198198
[, $actualType, $expectedType] = $matches;
199199

200-
throw new InvalidArgumentException(sprintf('Expected argument of type "%s", "%s" given at property path "%s".', $expectedType, 'NULL' === $actualType ? 'null' : $actualType, $propertyPath), 0, $previous);
200+
throw new InvalidTypeException($expectedType, $actualType, $propertyPath, $previous);
201201
}
202202
}
203203

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Serializer\Normalizer;
1313

1414
use Symfony\Component\PropertyAccess\Exception\InvalidArgumentException as PropertyAccessInvalidArgumentException;
15+
use Symfony\Component\PropertyAccess\Exception\InvalidTypeException;
1516
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
1617
use Symfony\Component\PropertyAccess\Exception\UninitializedPropertyException;
1718
use Symfony\Component\PropertyAccess\PropertyAccess;
@@ -374,7 +375,7 @@ public function denormalize(mixed $data, string $type, string $format = null, ar
374375
$exception = NotNormalizableValueException::createForUnexpectedDataType(
375376
sprintf('Failed to denormalize attribute "%s" value for class "%s": '.$e->getMessage(), $attribute, $type),
376377
$data,
377-
['unknown'],
378+
$e instanceof InvalidTypeException ? [$e->expectedType] : ['unknown'],
378379
$context['deserialization_path'] ?? null,
379380
false,
380381
$e->getCode(),

‎src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@
1414
use PHPStan\PhpDocParser\Parser\PhpDocParser;
1515
use PHPUnit\Framework\MockObject\MockObject;
1616
use PHPUnit\Framework\TestCase;
17+
use Symfony\Component\PropertyAccess\Exception\InvalidTypeException;
1718
use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
1819
use Symfony\Component\PropertyInfo\Extractor\PhpStanExtractor;
1920
use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
2021
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
2122
use Symfony\Component\Serializer\Exception\LogicException;
23+
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
2224
use Symfony\Component\Serializer\Exception\RuntimeException;
2325
use Symfony\Component\Serializer\Exception\UnexpectedValueException;
2426
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactory;
@@ -835,6 +837,24 @@ public function testNormalizeStdClass()
835837

836838
$this->assertSame(['baz' => 'baz'], $this->normalizer->normalize($o2));
837839
}
840+
841+
public function testNotNormalizableValueInvalidType()
842+
{
843+
if (!class_exists(InvalidTypeException::class)) {
844+
$this->markTestSkipped('Skipping test as the improvements on PropertyAccess are required.');
845+
}
846+
847+
$this->expectException(NotNormalizableValueException::class);
848+
$this->expectExceptionMessage('Expected argument of type "string", "array" given at property path "initialized"');
849+
850+
try {
851+
$this->normalizer->denormalize(['initialized' => ['not a string']], TypedPropertiesObject::class, 'array');
852+
} catch (NotNormalizableValueException $e) {
853+
$this->assertSame(['string'], $e->getExpectedTypes());
854+
855+
throw $e;
856+
}
857+
}
838858
}
839859

840860
class ProxyObjectDummy extends ObjectDummy

0 commit comments

Comments
 (0)