fix(serializer): improve #7270 by reducing inconsistencies (#7346)

Author: soyuka

src/Serializer/ItemNormalizer.php

@@ -32,6 +32,8 @@
 /**
  * Generic item normalizer.
  *
+ * TODO: do not hardcode "id"
+ *
  * @author Kévin Dunglas <dunglas@gmail.com>
  */
 class ItemNormalizer extends AbstractItemNormalizer
@@ -59,7 +61,9 @@ public function denormalize(mixed $data, string $class, ?string $format = null,
             }
 
             if (isset($context['resource_class'])) {
-                $this->updateObjectToPopulate($data, $context);
+                if ($this->updateObjectToPopulate($data, $context)) {
+                    unset($data['id']);
+                }
             } else {
                 // See https://github.com/api-platform/core/pull/2326 to understand this message.
                 $this->logger->warning('The "resource_class" key is missing from the context.', [
@@ -68,24 +72,15 @@ public function denormalize(mixed $data, string $class, ?string $format = null,
             }
         }
 
-        // See https://github.com/api-platform/core/pull/7270 - id may be an allowed attribute due to being added in the
-        // overridden getAllowedAttributes below, in order to allow updating a nested item via ID. But in this case it
-        // may not "really" be an allowed attribute, ie we don't want to actually use it in denormalization. In this
-        // scenario it will not be present in parent::getAllowedAttributes
-        if (isset($data['id'], $context['resource_class'])) {
-            $parentAllowedAttributes = parent::getAllowedAttributes($class, $context, true);
-            if (\is_array($parentAllowedAttributes) && !\in_array('id', $parentAllowedAttributes, true)) {
-                unset($data['id']);
-            }
-        }
-
         return parent::denormalize($data, $class, $format, $context);
     }
 
-    private function updateObjectToPopulate(array $data, array &$context): void
+    private function updateObjectToPopulate(array $data, array &$context): bool
     {
         try {
             $context[self::OBJECT_TO_POPULATE] = $this->iriConverter->getResourceFromIri((string) $data['id'], $context + ['fetch_data' => true]);
+
+            return true;
         } catch (InvalidArgumentException) {
             $operation = $this->resourceMetadataCollectionFactory?->create($context['resource_class'])->getOperation();
             if (
@@ -102,6 +97,8 @@ private function updateObjectToPopulate(array $data, array &$context): void
 
             $context[self::OBJECT_TO_POPULATE] = $this->iriConverter->getResourceFromIri($iri, $context + ['fetch_data' => true]);
         }
+
+        return false;
     }
 
     private function getContextUriVariables(array $data, $operation, array $context): array
@@ -122,8 +119,9 @@ private function getContextUriVariables(array $data, $operation, array $context)
     protected function getAllowedAttributes(string|object $classOrObject, array $context, bool $attributesAsString = false): array|bool
     {
         $allowedAttributes = parent::getAllowedAttributes($classOrObject, $context, $attributesAsString);
-        if (\is_array($allowedAttributes) && ($context['api_denormalize'] ?? false)) {
-            $allowedAttributes = array_merge($allowedAttributes, ['id']);
+        // id is a special case handled above it causes issues not allowing it
+        if (\is_array($allowedAttributes) && ($context['api_denormalize'] ?? false) && !\in_array('id', $allowedAttributes, true)) {
+            $allowedAttributes[] = 'id';
         }
 
         return $allowedAttributes;

src/Serializer/Tests/ItemNormalizerTest.php

@@ -316,13 +316,14 @@ public function testDenormalizeWithWrongId(): void
         $operation = new Get(uriVariables: ['id' => new Link(identifiers: ['id'], parameterName: 'id')]);
         $obj = new Dummy();
 
-        $propertyNameCollection = new PropertyNameCollection(['name']);
+        $propertyNameCollection = new PropertyNameCollection(['id', 'name']);
         $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
         $propertyNameCollectionFactoryProphecy->create(Dummy::class, [])->willReturn($propertyNameCollection)->shouldBeCalled();
 
         $propertyMetadata = (new ApiProperty())->withReadable(true)->withWritable(true);
         $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
         $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', [])->willReturn($propertyMetadata)->shouldBeCalled();
+        $propertyMetadataFactoryProphecy->create(Dummy::class, 'id', [])->willReturn((new ApiProperty())->withIdentifier(true))->shouldBeCalled();
 
         $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
         $iriConverterProphecy->getResourceFromIri('fail', $context + ['fetch_data' => true])->willThrow(new InvalidArgumentException());