bug #49525 [Serializer] Fix serialized path for non-scalar values (boenner)

nicolas-grekas · nicolas-grekas · commit 4f2aaf9b79b3 · 2023-03-31T11:05:48.000+02:00
This PR was merged into the 6.2 branch. Discussion ---------- [Serializer] Fix serialized path for non-scalar values | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #49494 | License | MIT | Doc PR | no This relates to #49494 and #49225. When non-scalar values are normalized, they are normalized twice in the `normalize()` function: ```php if (null !== $attributeValue && !\is_scalar($attributeValue)) { $stack[$attribute] = $attributeValue; } $data = $this->updateData($data, $attribute, $attributeValue, $class, $format, $attributeContext, $attributesMetadata, $classMetadata); ``` and a bit later: ```php foreach ($stack as $attribute => $attributeValue) { ... $data = $this->updateData($data, $attribute, $this->serializer->normalize($attributeValue, $format, $childContext), $class, $format, $attributeContext, $attributesMetadata, $classMetadata); } ``` For non-scalar values with a `SerializedPath` annotation this leads to an exception because the serializer is trying to re-populate the path. Running `updateData()` only once fixes this, but breaks a couple of tests across the components as it changes the order of elements in the serialized string (non-scalar values will be pushed to the end). Other than the string comparisons, nothing seems to break. This was also an issue while reviewing the PR for the `SerializedPath` annotation (#43534 (comment)) and got reverted because of the potential BC break. I'm not sure what benefit normalizing twice brings, so I added the test from `@HonzaMatosik`, changed the behavior in the normalizer and fixed the broken tests. If that's not the preferred solution here, I'd be ok with just eleminating the "The element you are trying to set is already populated" exception in the `SerializedPath` and allow overwriting values. Commits ------- d82ec41 [Serializer] Fix serializedpath for non scalar types
diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
@@ -198,14 +198,15 @@ public function normalize(mixed $object, string $format = null, array $context =
 
             $attributeValue = $this->applyCallbacks($attributeValue, $object, $attribute, $format, $attributeContext);
 
-            if (null !== $attributeValue && !\is_scalar($attributeValue)) {
-                $stack[$attribute] = $attributeValue;
-            }
-
-            $data = $this->updateData($data, $attribute, $attributeValue, $class, $format, $attributeContext, $attributesMetadata, $classMetadata);
+            $stack[$attribute] = $attributeValue;
         }
 
         foreach ($stack as $attribute => $attributeValue) {
+            if (null === $attributeValue || \is_scalar($attributeValue)) {
+                $data = $this->updateData($data, $attribute, $attributeValue, $class, $format, $context, $attributesMetadata, $classMetadata);
+                continue;
+            }
+
             if (!$this->serializer instanceof NormalizerInterface) {
                 throw new LogicException(sprintf('Cannot normalize attribute "%s" because the injected serializer is not a normalizer.', $attribute));
             }
diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php
@@ -542,6 +542,21 @@ public function testDenormalizeUsesContextAttributeForPropertiesInConstructorWit
 
         $this->assertSame($obj->propertyWithSerializedName->format('Y-m-d'), $obj->propertyWithoutSerializedName->format('Y-m-d'));
     }
+
+    public function testNormalizeUsesContextAttributeForPropertiesInConstructorWithSerializedPath()
+    {
+        $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
+
+        $extractor = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);
+        $normalizer = new ObjectNormalizer($classMetadataFactory, new MetadataAwareNameConverter($classMetadataFactory), null, $extractor);
+        $serializer = new Serializer([new DateTimeNormalizer([DateTimeNormalizer::FORMAT_KEY => 'd-m-Y']), $normalizer]);
+
+        $obj = new ObjectDummyWithContextAttributeAndSerializedPath(new \DateTimeImmutable('22-02-2023'));
+
+        $data = $serializer->normalize($obj);
+
+        $this->assertSame(['property' => ['with_path' => '22-02-2023']], $data);
+    }
 }
 
 class AbstractObjectNormalizerDummy extends AbstractObjectNormalizer
@@ -643,6 +658,16 @@ class DuplicateKeyNestedDummy
     public $notquux;
 }
 
+class ObjectDummyWithContextAttributeAndSerializedPath
+{
+    public function __construct(
+        #[Context([DateTimeNormalizer::FORMAT_KEY => 'm-d-Y'])]
+        #[SerializedPath('[property][with_path]')]
+        public \DateTimeImmutable $propertyWithPath,
+    ) {
+    }
+}
+
 class AbstractObjectNormalizerWithMetadata extends AbstractObjectNormalizer
 {
     public function __construct()
