From 296ab1dfa80d346bb109d1f253d9602d415dbefc Mon Sep 17 00:00:00 2001 From: Callum Browne Date: Tue, 1 Jul 2025 17:48:49 +0100 Subject: [PATCH 1/6] fix(serializer): allow "ID" field even when allow_extra_attributes=false --- src/Serializer/ItemNormalizer.php | 10 +++ .../TestBundle/Entity/Issue6225/Bar6225.php | 75 +++++++++++++++++++ .../TestBundle/Entity/Issue6225/Foo6225.php | 74 ++++++++++++++++++ tests/Functional/NestedPatchTest.php | 60 +++++++++++++++ 4 files changed, 219 insertions(+) create mode 100644 tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php create mode 100644 tests/Fixtures/TestBundle/Entity/Issue6225/Foo6225.php create mode 100644 tests/Functional/NestedPatchTest.php diff --git a/src/Serializer/ItemNormalizer.php b/src/Serializer/ItemNormalizer.php index c426584aa8d..821fd710a65 100644 --- a/src/Serializer/ItemNormalizer.php +++ b/src/Serializer/ItemNormalizer.php @@ -107,4 +107,14 @@ 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); + if (\is_array($allowedAttributes) && ($context['api_denormalize'] ?? false)) { + $allowedAttributes = array_merge($allowedAttributes, ['id']); + } + + return $allowedAttributes; + } } diff --git a/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php b/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php new file mode 100644 index 00000000000..3e9f17b979a --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php @@ -0,0 +1,75 @@ + + * + * 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\Issue6225; + +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Link; +use ApiPlatform\Metadata\Post; +use ApiPlatform\Metadata\Put; +use Doctrine\ORM\Mapping as ORM; +use Symfony\Bridge\Doctrine\Types\UuidType; +use Symfony\Component\Serializer\Annotation\Groups; +use Symfony\Component\Uid\Uuid; + +#[ORM\Entity] +#[ORM\Table(name: 'bar6225')] +#[ApiResource] +class Bar6225 +{ + public function __construct() + { + $this->id = Uuid::v7(); + } + + #[ORM\Id] + #[ORM\Column(type: 'symfony_uuid', unique: true)] + #[Groups(['Foo:Read'])] + private Uuid $id; + + #[ORM\OneToOne(mappedBy: 'bar', cascade: ['persist', 'remove'])] + private Foo6225 $foo; + + #[ORM\Column(length: 255)] + #[Groups(['Foo:Write', 'Foo:Read'])] + private string $someProperty; + + public function getId(): Uuid + { + return $this->id; + } + + public function getFoo(): Foo6225 + { + return $this->foo; + } + + public function setFoo(Foo6225 $foo): static { + $this->foo = $foo; + + return $this; + } + + public function getSomeProperty(): string + { + return $this->someProperty; + } + + public function setSomeProperty(string $someProperty): static { + $this->someProperty = $someProperty; + + return $this; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/Issue6225/Foo6225.php b/tests/Fixtures/TestBundle/Entity/Issue6225/Foo6225.php new file mode 100644 index 00000000000..0ffde4db1e1 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Issue6225/Foo6225.php @@ -0,0 +1,74 @@ + + * + * 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\Issue6225; + +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Patch; +use ApiPlatform\Metadata\Post; +use ApiPlatform\Metadata\Put; +use Doctrine\ORM\Mapping as ORM; +use Symfony\Bridge\Doctrine\Types\UuidType; +use Symfony\Component\Serializer\Annotation\Groups; +use Symfony\Component\Uid\Uuid; + +#[ORM\Entity()] +#[ORM\Table(name: 'foo6225')] +#[ApiResource( + operations: [ + new Post(), + new Patch(), + ], + normalizationContext: [ + 'groups' => ['Foo:Read'], + ], + denormalizationContext: [ + 'allow_extra_attributes' => false, + 'groups' => ['Foo:Write'], + ], +)] +class Foo6225 +{ + public function __construct() + { + $this->id = Uuid::v7(); + } + + #[ORM\Id] + #[ORM\Column(type: 'symfony_uuid', unique: true)] + #[Groups(['Foo:Read'])] + private Uuid $id; + + #[ORM\OneToOne(inversedBy: 'foo', cascade: ['persist', 'remove'])] + #[ORM\JoinColumn(nullable: false)] + #[Groups(['Foo:Write', 'Foo:Read'])] + private Bar6225 $bar; + + public function getId(): Uuid + { + return $this->id; + } + + public function getBar(): Bar6225 + { + return $this->bar; + } + + public function setBar(Bar6225 $bar): static + { + $this->bar = $bar; + + return $this; + } +} diff --git a/tests/Functional/NestedPatchTest.php b/tests/Functional/NestedPatchTest.php new file mode 100644 index 00000000000..32a4c9ae7c2 --- /dev/null +++ b/tests/Functional/NestedPatchTest.php @@ -0,0 +1,60 @@ +recreateSchema(self::getResources()); + + $response = self::createClient()->request('POST', '/foo6225s', [ + 'json' => [ + 'bar' => [ + 'someProperty' => 'abc' + ] + ], + 'headers' => [ + 'accept' => 'application/json' + ] + ]); + static::assertResponseIsSuccessful(); + $responseContent = json_decode($response->getContent(), true); + $createdFooId = $responseContent['id']; + $createdBarId = $responseContent['bar']['id']; + + $patchResponse = self::createClient()->request('PATCH', '/foo6225s/'.$createdFooId, [ + 'json' => [ + 'bar' => [ + 'id' => $createdBarId, + 'someProperty' => 'def' + ] + ], + 'headers' => [ + 'accept' => 'application/json', + 'content-type' => 'application/merge-patch+json' + ] + ]); + static::assertResponseIsSuccessful(); + static::assertEquals([ + 'id' => $createdFooId, + 'bar' => [ + 'id' => $createdBarId, + 'someProperty' => 'def' + ] + ], json_decode($patchResponse->getContent(), true)); + } +} From 30ec232dec4ba3e995c27e8f734befbc8b03b3ff Mon Sep 17 00:00:00 2001 From: Callum Browne Date: Tue, 1 Jul 2025 18:08:37 +0100 Subject: [PATCH 2/6] Make property nullable --- tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php b/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php index 3e9f17b979a..51239be5b05 100644 --- a/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php +++ b/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php @@ -40,7 +40,7 @@ public function __construct() private Uuid $id; #[ORM\OneToOne(mappedBy: 'bar', cascade: ['persist', 'remove'])] - private Foo6225 $foo; + private ?Foo6225 $foo; #[ORM\Column(length: 255)] #[Groups(['Foo:Write', 'Foo:Read'])] From 71e6be6a162bed21c61a4e6c79b6c268c0184c4a Mon Sep 17 00:00:00 2001 From: Callum Browne Date: Tue, 1 Jul 2025 18:10:20 +0100 Subject: [PATCH 3/6] Use protected static ?bool $alwaysBootKernel = false; --- tests/Functional/NestedPatchTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Functional/NestedPatchTest.php b/tests/Functional/NestedPatchTest.php index 32a4c9ae7c2..848ba66cf76 100644 --- a/tests/Functional/NestedPatchTest.php +++ b/tests/Functional/NestedPatchTest.php @@ -13,6 +13,8 @@ class NestedPatchTest extends ApiTestCase use RecreateSchemaTrait; use SetupClassResourcesTrait; + protected static ?bool $alwaysBootKernel = false; + public static function getResources(): array { return [Foo6225::class, Bar6225::class]; } From 2dde9dd4f06fa4cccf2fa86ea3aaa24d9c05ef68 Mon Sep 17 00:00:00 2001 From: Callum Browne Date: Sun, 6 Jul 2025 00:12:41 +0100 Subject: [PATCH 4/6] Unset ID before calling parent denormalizer --- src/Serializer/ItemNormalizer.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Serializer/ItemNormalizer.php b/src/Serializer/ItemNormalizer.php index 821fd710a65..c1513ab03c7 100644 --- a/src/Serializer/ItemNormalizer.php +++ b/src/Serializer/ItemNormalizer.php @@ -68,6 +68,17 @@ 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)) { + unset($data['id']); + } + } + return parent::denormalize($data, $class, $format, $context); } From de3ebb57d082295cb4f974a3a077cca7db764dba Mon Sep 17 00:00:00 2001 From: Callum Browne Date: Sun, 6 Jul 2025 00:20:11 +0100 Subject: [PATCH 5/6] Skip test in mongodb environment --- tests/Functional/NestedPatchTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Functional/NestedPatchTest.php b/tests/Functional/NestedPatchTest.php index 848ba66cf76..3de20e3e309 100644 --- a/tests/Functional/NestedPatchTest.php +++ b/tests/Functional/NestedPatchTest.php @@ -21,6 +21,10 @@ public static function getResources(): array { public function testIssue6225(): void { + if ($this->isMongoDB()) { + $this->markTestSkipped(); + } + $this->recreateSchema(self::getResources()); $response = self::createClient()->request('POST', '/foo6225s', [ From 5a0d047ee5553dbc2d213bd9596930b6944250b6 Mon Sep 17 00:00:00 2001 From: Callum Browne Date: Sun, 6 Jul 2025 00:25:26 +0100 Subject: [PATCH 6/6] php-cs-fixer fix --- src/Serializer/ItemNormalizer.php | 2 +- .../TestBundle/Entity/Issue6225/Bar6225.php | 12 +++---- .../TestBundle/Entity/Issue6225/Foo6225.php | 3 -- tests/Functional/NestedPatchTest.php | 34 +++++++++++++------ 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/Serializer/ItemNormalizer.php b/src/Serializer/ItemNormalizer.php index c1513ab03c7..1cc8452a985 100644 --- a/src/Serializer/ItemNormalizer.php +++ b/src/Serializer/ItemNormalizer.php @@ -74,7 +74,7 @@ public function denormalize(mixed $data, string $class, ?string $format = null, // 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)) { + if (\is_array($parentAllowedAttributes) && !\in_array('id', $parentAllowedAttributes, true)) { unset($data['id']); } } diff --git a/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php b/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php index 51239be5b05..6a5b06df56e 100644 --- a/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php +++ b/tests/Fixtures/TestBundle/Entity/Issue6225/Bar6225.php @@ -14,13 +14,7 @@ namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue6225; use ApiPlatform\Metadata\ApiResource; -use ApiPlatform\Metadata\Get; -use ApiPlatform\Metadata\GetCollection; -use ApiPlatform\Metadata\Link; -use ApiPlatform\Metadata\Post; -use ApiPlatform\Metadata\Put; use Doctrine\ORM\Mapping as ORM; -use Symfony\Bridge\Doctrine\Types\UuidType; use Symfony\Component\Serializer\Annotation\Groups; use Symfony\Component\Uid\Uuid; @@ -56,7 +50,8 @@ public function getFoo(): Foo6225 return $this->foo; } - public function setFoo(Foo6225 $foo): static { + public function setFoo(Foo6225 $foo): static + { $this->foo = $foo; return $this; @@ -67,7 +62,8 @@ public function getSomeProperty(): string return $this->someProperty; } - public function setSomeProperty(string $someProperty): static { + public function setSomeProperty(string $someProperty): static + { $this->someProperty = $someProperty; return $this; diff --git a/tests/Fixtures/TestBundle/Entity/Issue6225/Foo6225.php b/tests/Fixtures/TestBundle/Entity/Issue6225/Foo6225.php index 0ffde4db1e1..7f625901db2 100644 --- a/tests/Fixtures/TestBundle/Entity/Issue6225/Foo6225.php +++ b/tests/Fixtures/TestBundle/Entity/Issue6225/Foo6225.php @@ -14,12 +14,9 @@ namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue6225; use ApiPlatform\Metadata\ApiResource; -use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\Patch; use ApiPlatform\Metadata\Post; -use ApiPlatform\Metadata\Put; use Doctrine\ORM\Mapping as ORM; -use Symfony\Bridge\Doctrine\Types\UuidType; use Symfony\Component\Serializer\Annotation\Groups; use Symfony\Component\Uid\Uuid; diff --git a/tests/Functional/NestedPatchTest.php b/tests/Functional/NestedPatchTest.php index 3de20e3e309..52fc1c756c1 100644 --- a/tests/Functional/NestedPatchTest.php +++ b/tests/Functional/NestedPatchTest.php @@ -1,5 +1,16 @@ + * + * 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\Functional; use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; @@ -15,7 +26,8 @@ class NestedPatchTest extends ApiTestCase protected static ?bool $alwaysBootKernel = false; - public static function getResources(): array { + public static function getResources(): array + { return [Foo6225::class, Bar6225::class]; } @@ -30,12 +42,12 @@ public function testIssue6225(): void $response = self::createClient()->request('POST', '/foo6225s', [ 'json' => [ 'bar' => [ - 'someProperty' => 'abc' - ] + 'someProperty' => 'abc', + ], ], 'headers' => [ - 'accept' => 'application/json' - ] + 'accept' => 'application/json', + ], ]); static::assertResponseIsSuccessful(); $responseContent = json_decode($response->getContent(), true); @@ -46,21 +58,21 @@ public function testIssue6225(): void 'json' => [ 'bar' => [ 'id' => $createdBarId, - 'someProperty' => 'def' - ] + 'someProperty' => 'def', + ], ], 'headers' => [ 'accept' => 'application/json', - 'content-type' => 'application/merge-patch+json' - ] + 'content-type' => 'application/merge-patch+json', + ], ]); static::assertResponseIsSuccessful(); static::assertEquals([ 'id' => $createdFooId, 'bar' => [ 'id' => $createdBarId, - 'someProperty' => 'def' - ] + 'someProperty' => 'def', + ], ], json_decode($patchResponse->getContent(), true)); } }