8000 bug #45838 [Serializer] Fix denormalizing union types (T-bond) · symfony/symfony@62344fd · GitHub
[go: up one dir, main page]
Skip to content

Commit 62344fd

Browse files
nicolas-grekasnicolas-grekas
committed
bug #45838 [Serializer] Fix denormalizing union types (T-bond)
This PR was merged into the 4.4 branch. Discussion ---------- [Serializer] Fix denormalizing union types | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #45818 | License | MIT | Doc PR | - <!-- Replace this notice by a short README for your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against the latest branch. - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> `@nicolas`-grekas Replaces: #45824 as I accidentally registerred a lot of people to the notifier list when changing target branch. Note about difference between docblock vs PHP 8 union types: With PHP 8 union types, the resolved `$types` array was always (when I checked) in the same order, no matter the type definition. So these two things: ```public null|boolean|DateTime``` and ```public DateTime|boolean|null``` would **always** generate an array, where the **first** item was `DateTime`. With PHP docblock the `$tpyes` array order depends of the order of the documentation. So the `* `@var` \DateTime|bool|null` would resolve to an array with `DateTime` as **first** item, but the `* `@var` null|bool|\DateTime` would resolve to the `DateTime` being the **last** element. Commits ------- 98acd62 [Serializer] Fix denormalizing union types
2 parents 002165c + 98acd62 commit 62344fd

File tree

2 files changed

+86
-20
lines changed

2 files changed

+86
-20
lines changed

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

Lines changed: 32 additions & 20 deletions
8000
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ private function validateAndDenormalize(string $currentClass, string $attribute,
404404
}
405405

406406
$expectedTypes = [];
407+
$isUnionType = \count($types) > 1;
407408
foreach ($types as $type) {
408409
if (null === $data && $type->isNullable()) {
409410
return null;
@@ -455,29 +456,40 @@ private function validateAndDenormalize(string $currentClass, string $attribute,
455456

456457
$expectedTypes[Type::BUILTIN_TYPE_OBJECT === $builtinType && $class ? $class : $builtinType] = true;
457458

458-
if (Type::BUILTIN_TYPE_OBJECT === $builtinType) {
459-
if (!$this->serializer instanceof DenormalizerInterface) {
460-
throw new LogicException(sprintf('Cannot denormalize attribute "%s" for class "%s" because injected serializer is not a denormalizer.', $attribute, $class));
459+
// This try-catch should cover all NotNormalizableValueException (and all return branches after the first
460+
// exception) so we could try denormalizing all types of an union type. If the target type is not an union
461+
// type, we will just re-throw the catched exception.
462+
// In the case of no denormalization succeeds with an union type, it will fall back to the default exception
463+
// with the acceptable types list.
464+
try {
465+
if (Type::BUILTIN_TYPE_OBJECT === $builtinType) {
466+
if (!$this->serializer instanceof DenormalizerInterface) {
467+
throw new LogicException(sprintf('Cannot denormalize attribute "%s" for class "%s" because injected serializer is not a denormalizer.', $attribute, $class));
468+
}
469+
470+
$childContext = $this->createChildContext($context, $attribute, $format);
471+
if ($this->serializer->supportsDenormalization($data, $class, $format, $childContext)) {
472+
return $this->serializer->denormalize($data, $class, $format, $childContext);
473+
}
461474
}
462475

463-
$childContext = $this->createChildContext($context, $attribute, $format);
464-
if ($this->serializer->supportsDenormalization($data, $class, $format, $childContext)) {
465-
return $this->serializer->denormalize($data, $class, $format, $childContext);
476+
// JSON only has a Number type corresponding to both int and float PHP types.
477+
// PHP's json_encode, JavaScript's JSON.stringify, Go's json.Marshal as well as most other JSON encoders convert
478+
// floating-point numbers like 12.0 to 12 (the decimal part is dropped when possible).
479+
// PHP's json_decode automatically converts Numbers without a decimal part to integers.
480+
// To circumvent this behavior, integers are converted to floats when denormalizing JSON based formats and when
481+
// a float is expected.
482+
if (Type::BUILTIN_TYPE_FLOAT === $builtinType && \is_int($data) && null !== $format && str_contains($format, JsonEncoder::FORMAT)) {
483+
return (float) $data;
466484
}
467-
}
468-
469-
// JSON only has a Number type corresponding to both int and float PHP types.
470-
// PHP's json_encode, JavaScript's JSON.stringify, Go's json.Marshal as well as most other JSON encoders convert
471-
// floating-point numbers like 12.0 to 12 (the decimal part is dropped when possible).
472-
// PHP's json_decode automatically converts Numbers without a decimal part to integers.
473-
// To circumvent this behavior, integers are converted to floats when denormalizing JSON based formats and when
474-
// a float is expected.
475-
if (Type::BUILTIN_TYPE_FLOAT === $builtinType && \is_int($data) && null !== $format && str_contains($format, JsonEncoder::FORMAT)) {
476-
return (float) $data;
477-
}
478485

479-
if (('is_'.$builtinType)($data)) {
480-
return $data;
486+
if (('is_'.$builtinType)($data)) {
487+
return $data;
488+
}
489+
} catch (NotNormalizableValueException $e) {
490+
if (!$isUnionType) {
491+
throw $e;
492+
}
481493
}
482494
}
483495

@@ -639,7 +651,7 @@ private function getCacheKey(?string $format, array $context)
639651
'ignored' => $this->ignoredAttributes,
640652
'camelized' => $this->camelizedAttributes,
641653
]));
642-
} catch (\Exception $exception) {
654+
} catch (\Exception $e) {
643655
// The context cannot be serialized, skip the cache
644656
return false;
645657
}

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use PHPUnit\Framework\TestCase;
1616
use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
1717
use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
18+
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
1819
use Symfony\Component\Serializer\Encoder\DecoderInterface;
1920
use Symfony\Component\Serializer\Encoder\EncoderInterface;
2021
use Symfony\Component\Serializer\Encoder\JsonEncoder;
@@ -33,6 +34,7 @@
3334
use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;
3435
use Symfony\Component\Serializer\Normalizer\ArrayDenormalizer;
3536
use Symfony\Component\Serializer\Normalizer\CustomNormalizer;
37+
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;
3638
use Symfony\Component\Serializer\Normalizer\DenormalizerAwareInterface;
3739
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
3840
use Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer;
@@ -538,6 +540,38 @@ public function testNormalizePreserveEmptyArrayObject()
538540
$this->assertEquals('{"foo":{},"bar":["notempty"],"baz":{"nested":{}}}', $serializer->serialize($object, 'json', [AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS => true]));
539541
}
540542

543+
public function testUnionTypeDeserializable()
544+
{
545+
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLo F438 ader(new AnnotationReader()));
546+
$extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);
547+
$serializer = new Serializer(
548+
[
549+
new DateTimeNormalizer(),
550+
new ObjectNormalizer($classMetadataFactory, null, null, $extractor, new ClassDiscriminatorFromClassMetadata($classMetadataFactory)),
551+
],
552+
['json' => new JsonEncoder()]
553+
);
554+
555+
$actual = $serializer->deserialize('{ "changed": null }', DummyUnionType::class, 'json', [
556+
DateTimeNormalizer::FORMAT_KEY => \DateTime::ISO8601,
557+
]);
558+
559+
$this->assertEquals((new DummyUnionType())->setChanged(null), $actual, 'Union type denormalization first case failed.');
560+
561+
$actual = $serializer->deserialize('{ "changed": "2022-03-22T16:15:05+0000" }', DummyUnionType::class, 'json', [
562+
DateTimeNormalizer::FORMAT_KEY => \DateTime::ISO8601,
563+
]);
564+
565+
$expectedDateTime = \DateTime::createFromFormat(\DateTime::ISO8601, '2022-03-22T16:15:05+0000');
566+
$this->assertEquals((new DummyUnionType())->setChanged($expectedDateTime), $actual, 'Union type denormalization second case failed.');
567+
568+
$actual = $serializer->deserialize('{ "changed": false }', DummyUnionType::class, 'json', [
569+
DateTimeNormalizer::FORMAT_KEY => \DateTime::ISO8601,
570+
]);
571+
572+
$this->assertEquals(new DummyUnionType(), $actual, 'Union type denormalization third case failed.');
573+
}
574+
541575
private function serializerWithClassDiscriminator()
542576
{
543577
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
@@ -610,6 +644,26 @@ public function __construct($value)
610644
}
611645
}
612646

647+
class DummyUnionType
648+
{
649+
/**
650+
* @var \DateTime|bool|null
651+
*/
652+
public $changed = false;
653+
654+
/**
655+
* @param \DateTime|bool|null
656+
*
657+
* @return $this
658+
*/
659+
public function setChanged($changed): self
660+
{
661+
$this->changed = $changed;
662+
663+
return $this;
664+
}
665+
}
666+
613667
interface NormalizerAwareNormalizer extends NormalizerInterface, NormalizerAwareInterface
614668
{
615669
}

0 commit comments

Comments
 (0)
0