fix(metadata): compute isWritable during updates

Doctrine should not assume an `id` is not writable during updates.
PropertyInfo should not assume a property is not writable (for now only
during updates) but as this has a direct impact on serialization groups
maybe we should just skip readable/writable in the
PropertyInfoPropertyMetadataFactory.
SerializerPropertyMetadataFactory should run after AttributePropertyMetadataFactory or we can not override this behavior.

Mayabe that we could just simplify these and never set `writable` (or
`readable`) from Doctrine or PropertyInfo actually (and we won't need to
add the api_allow_update to the getFactoryOptions...).

fixes #7382

Author: soyuka

src/Doctrine/Odm/Metadata/Property/DoctrineMongoDbOdmPropertyMetadataFactory.php

@@ -57,6 +57,10 @@ public function create(string $resourceClass, string $property, array $options =
                     break;
                 }
 
+                if ($options['api_allow_update'] ?? false) {
+                    break;
+                }
+
                 $propertyMetadata = $propertyMetadata->withWritable(false);
 
                 break;

src/Doctrine/Orm/Metadata/Property/DoctrineOrmPropertyMetadataFactory.php

@@ -56,6 +56,10 @@ public function create(string $resourceClass, string $property, array $options =
                     break;
                 }
 
+                if ($options['api_allow_update'] ?? false) {
+                    break;
+                }
+
                 if ($doctrineClassMetadata instanceof ClassMetadata) {
                     $writable = $doctrineClassMetadata->isIdentifierNatural();
                 } else {

src/Metadata/Property/Factory/PropertyInfoPropertyMetadataFactory.php

@@ -66,7 +66,9 @@ public function create(string $resourceClass, string $property, array $options =
             $propertyMetadata = $propertyMetadata->withReadable($readable);
         }
 
-        if (null === $propertyMetadata->isWritable() && null !== $writable = $this->propertyInfo->isWritable($resourceClass, $property, $options)) {
+        // A property might not be writable and we can still want to update it,
+        // this leaves the choice to the SerializerPropertyMetadataFactory
+        if (false === ($options['api_allow_update'] ?? false) && null === $propertyMetadata->isWritable() && null !== $writable = $this->propertyInfo->isWritable($resourceClass, $property, $options)) {
             $propertyMetadata = $propertyMetadata->withWritable($writable);
         }
 

src/Serializer/AbstractItemNormalizer.php

@@ -622,7 +622,7 @@ protected function denormalizeRelation(string $attributeName, ApiProperty $prope
      */
     protected function getFactoryOptions(array $context): array
     {
-        $options = [];
+        $options = ['api_allow_update' => $context['api_allow_update'] ?? false];
         if (isset($context[self::GROUPS])) {
             /* @see https://github.com/symfony/symfony/blob/v4.2.6/src/Symfony/Component/PropertyInfo/Extractor/SerializerExtractor.php */
             $options['serializer_groups'] = (array) $context[self::GROUPS];

src/Serializer/ItemNormalizer.php

@@ -115,15 +115,4 @@ private function getContextUriVariables(array $data, $operation, array $context)
 
         return $uriVariables;
     }
-
-    protected function getAllowedAttributes(string|object $classOrObject, array $context, bool $attributesAsString = false): array|bool
-    {
-        $allowedAttributes = parent::getAllowedAttributes($classOrObject, $context, $attributesAsString);
-        // 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/Symfony/Bundle/Resources/config/metadata/property.xml

@@ -13,7 +13,7 @@
             <argument type="service" id="api_platform.metadata.property.metadata_factory.property_info.inner" />
         </service>
 
-        <service id="api_platform.metadata.property.metadata_factory.attribute" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="20" class="ApiPlatform\Metadata\Property\Factory\AttributePropertyMetadataFactory" public="false">
+        <service id="api_platform.metadata.property.metadata_factory.attribute" decorates="api_platform.metadata.property.metadata_factory" decoration-priority="35" class="ApiPlatform\Metadata\Property\Factory\AttributePropertyMetadataFactory" public="false">
             <argument type="service" id="api_platform.metadata.property.metadata_factory.attribute.inner" />
         </service>
 

tests/Fixtures/TestBundle/Entity/Camp.php

@@ -0,0 +1,53 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;
+
+use ApiPlatform\Metadata\ApiProperty;
+use ApiPlatform\Metadata\ApiResource;
+use Doctrine\ORM\Mapping as ORM;
+
+#[ApiResource(
+    denormalizationContext: [
+        'allow_extra_attributes' => false,
+    ],
+)]
+#[ORM\Entity]
+class Camp
+{
+    #[ORM\Id]
+    #[ORM\Column(type: 'integer')]
+    #[ORM\GeneratedValue]
+    #[ApiProperty(writable: false)]
+    private ?int $id = null;
+
+    #[ORM\Column(type: 'string', length: 255)]
+    private ?string $name = null;
+
+    public function getId(): ?int
+    {
+        return $this->id;
+    }
+
+    public function getName(): ?string
+    {
+        return $this->name;
+    }
+
+    public function setName(string $name): self
+    {
+        $this->name = $name;
+
+        return $this;
+    }
+}

tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php

@@ -30,7 +30,7 @@ public function __construct()
 
     #[ORM\Id]
     #[ORM\Column(type: 'symfony_uuid', unique: true)]
-    #[Groups(['Foo:Read'])]
+    #[Groups(['Foo:Read', 'Foo:Write'])]
     private Uuid $id;
 
     #[ORM\OneToOne(mappedBy: 'bar', cascade: ['persist', 'remove'])]

tests/Functional/NestedPatchTest.php

@@ -14,6 +14,7 @@
 namespace ApiPlatform\Tests\Functional;
 
 use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Camp;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue6225\Bar6225;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue6225\Foo6225;
 use ApiPlatform\Tests\RecreateSchemaTrait;
@@ -28,7 +29,7 @@ class NestedPatchTest extends ApiTestCase
 
     public static function getResources(): array
     {
-        return [Foo6225::class, Bar6225::class];
+        return [Foo6225::class, Bar6225::class, Camp::class];
     }
 
     public function testIssue6225(): void
@@ -75,4 +76,61 @@ public function testIssue6225(): void
             ],
         ], json_decode($patchResponse->getContent(), true));
     }
+
+    public function testIdNotWriteable(): void
+    {
+        if ($this->isMongoDB()) {
+            $this->markTestSkipped();
+        }
+
+        $this->recreateSchema(self::getResources());
+
+        $camp = new Camp();
+        $camp->setName('Test Camp');
+        $manager = static::getContainer()->get('doctrine')->getManager();
+        $manager->persist($camp);
+        $manager->flush();
+
+        static::createClient()->request(
+            'PATCH',
+            '/camps/'.$camp->getId(),
+            [
+                'json' => [
+                    'id' => 39,
+                ],
+                'headers' => ['Content-Type' => 'application/merge-patch+json'],
+            ]
+        );
+
+        $this->assertResponseStatusCodeSame(400);
+        $this->assertJsonContains([
+            'detail' => 'Extra attributes are not allowed ("id" is unknown).',
+        ]);
+    }
+
+    public function testIdIsNotWritable(): void
+    {
+        if ($this->isMongoDB()) {
+            $this->markTestSkipped();
+        }
+
+        $this->recreateSchema(self::getResources());
+
+        static::createClient()->request(
+            'POST',
+            '/camps',
+            [
+                'json' => [
+                    'id' => 39,
+                    'name' => 'New Camp',
+                ],
+                'headers' => ['Content-Type' => 'application/json'],
+            ]
+        );
+
+        $this->assertResponseStatusCodeSame(400);
+        $this->assertJsonContains([
+            'detail' => 'Update is not allowed for this operation.',
+        ]);
+    }
 }