From b595dacd26fd523d8950b75fa3ea9d284fc2f933 Mon Sep 17 00:00:00 2001 From: soyuka Date: Mon, 4 Dec 2023 18:12:08 +0100 Subject: [PATCH 1/2] fix(doctrine): get reference with identifier value --- features/main/standard_put.feature | 45 ++++++++++++ .../Common/State/PersistProcessor.php | 54 +++++++-------- .../TestBundle/Entity/UidIdentified.php | 68 +++++++++++++++++++ 3 files changed, 139 insertions(+), 28 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Entity/UidIdentified.php diff --git a/features/main/standard_put.feature b/features/main/standard_put.feature index cf1febce0ae..8a95f9f5930 100644 --- a/features/main/standard_put.feature +++ b/features/main/standard_put.feature @@ -47,3 +47,48 @@ Feature: Spec-compliant PUT support "bar": "" } """ + + @createSchema + @!mongodb + Scenario: Create a new resource identified by an uid + When I add "Content-Type" header equal to "application/ld+json" + And I send a "PUT" request to "/uid_identifieds/fbcf5910-d915-4f7d-ba39-6b2957c57335" with body: + """ + { + "name": "test" + } + """ + Then the response status code should be 201 + And the response should be in JSON + And the JSON should be equal to: + """ + { + "@context": "/contexts/UidIdentified", + "@id": "/uid_identifieds/fbcf5910-d915-4f7d-ba39-6b2957c57335", + "@type": "UidIdentified", + "id": "fbcf5910-d915-4f7d-ba39-6b2957c57335", + "name": "test" + } + """ + + @!mongodb + Scenario: Replace an existing resource + When I add "Content-Type" header equal to "application/ld+json" + And I send a "PUT" request to "/uid_identifieds/fbcf5910-d915-4f7d-ba39-6b2957c57335" with body: + """ + { + "name": "bar" + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the JSON should be equal to: + """ + { + "@context": "/contexts/UidIdentified", + "@id": "/uid_identifieds/fbcf5910-d915-4f7d-ba39-6b2957c57335", + "@type": "UidIdentified", + "id": "fbcf5910-d915-4f7d-ba39-6b2957c57335", + "name": "bar" + } + """ diff --git a/src/Doctrine/Common/State/PersistProcessor.php b/src/Doctrine/Common/State/PersistProcessor.php index 677dfe6148a..76016309bdc 100644 --- a/src/Doctrine/Common/State/PersistProcessor.php +++ b/src/Doctrine/Common/State/PersistProcessor.php @@ -17,8 +17,6 @@ use ApiPlatform\Metadata\Operation; use ApiPlatform\Metadata\Util\ClassInfoTrait; use ApiPlatform\State\ProcessorInterface; -use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; -use Doctrine\ORM\Mapping\ClassMetadataInfo; use Doctrine\Persistence\ManagerRegistry; use Doctrine\Persistence\ObjectManager as DoctrineObjectManager; @@ -50,48 +48,48 @@ public function process(mixed $data, Operation $operation, array $uriVariables = // PUT: reset the existing object managed by Doctrine and merge data sent by the user in it // This custom logic is needed because EntityManager::merge() has been deprecated and UPSERT isn't supported: // https://github.com/doctrine/orm/issues/8461#issuecomment-1250233555 - if ($operation instanceof HttpOperation && HttpOperation::METHOD_PUT === $operation->getMethod() && ($operation->getExtraProperties()['standard_put'] ?? false)) { + if ($operation instanceof HttpOperation && 'PUT' === $operation->getMethod() && ($operation->getExtraProperties()['standard_put'] ?? false)) { \assert(method_exists($manager, 'getReference')); - // TODO: the call to getReference is most likely to fail with complex identifiers $newData = $data; - if (isset($context['previous_data'])) { - $newData = 1 === \count($uriVariables) ? $manager->getReference($class, current($uriVariables)) : clone $context['previous_data']; - } - $identifiers = array_reverse($uriVariables); $links = $this->getLinks($class, $operation, $context); $reflectionProperties = $this->getReflectionProperties($data); - if (!isset($context['previous_data'])) { - foreach (array_reverse($links) as $link) { - if ($link->getExpandedValue() || !$link->getFromClass()) { + // TODO: the call to getReference is most likely to fail with complex identifiers + if ($previousData = $context['previous_data']) { + $classMetadata = $manager->getClassMetadata($class); + $identifiers = $classMetadata->getIdentifierValues($previousData); + $newData = 1 === \count($identifiers) ? $manager->getReference($class, current($identifiers)) : clone $previousData; + + foreach ($reflectionProperties as $propertyName => $reflectionProperty) { + // // Don't override the property if it's part of the subresource system + if (isset($identifiers[$propertyName]) || isset($uriVariables[$propertyName])) { continue; } - $identifierProperties = $link->getIdentifiers(); - $hasCompositeIdentifiers = 1 < \count($identifierProperties); + // Skip URI variables as sometime an uri variable is not the doctrine identifier + foreach ($links as $link) { + if (\in_array($propertyName, $link->getIdentifiers(), true)) { + continue 2; + } + } - foreach ($identifierProperties as $identifierProperty) { - $reflectionProperty = $reflectionProperties[$identifierProperty]; - $reflectionProperty->setValue($newData, $this->getIdentifierValue($identifiers, $hasCompositeIdentifiers ? $identifierProperty : null)); + if (($newValue = $reflectionProperty->getValue($data)) !== $reflectionProperty->getValue($newData)) { + $reflectionProperty->setValue($newData, $newValue); } } } else { - foreach ($reflectionProperties as $propertyName => $reflectionProperty) { - // Don't override the property if it's part of the subresource system - if (isset($uriVariables[$propertyName])) { + foreach (array_reverse($links) as $link) { + if ($link->getExpandedValue() || !$link->getFromClass()) { continue; } - foreach ($links as $link) { - $identifierProperties = $link->getIdentifiers(); - if (\in_array($propertyName, $identifierProperties, true)) { - continue; - } + $identifierProperties = $link->getIdentifiers(); + $hasCompositeIdentifiers = 1 < \count($identifierProperties); - if (($newValue = $reflectionProperty->getValue($data)) !== $reflectionProperty->getValue($newData)) { - $reflectionProperty->setValue($newData, $newValue); - } + foreach ($identifierProperties as $identifierProperty) { + $reflectionProperty = $reflectionProperties[$identifierProperty]; + $reflectionProperty->setValue($newData, $this->getIdentifierValue($identifiers, $hasCompositeIdentifiers ? $identifierProperty : null)); } } } @@ -115,7 +113,7 @@ public function process(mixed $data, Operation $operation, array $uriVariables = private function isDeferredExplicit(DoctrineObjectManager $manager, $data): bool { $classMetadata = $manager->getClassMetadata($this->getObjectClass($data)); - if (($classMetadata instanceof ClassMetadataInfo || $classMetadata instanceof ClassMetadata) && method_exists($classMetadata, 'isChangeTrackingDeferredExplicit')) { + if ($classMetadata && method_exists($classMetadata, 'isChangeTrackingDeferredExplicit')) { // @phpstan-ignore-line metadata can be null return $classMetadata->isChangeTrackingDeferredExplicit(); } diff --git a/tests/Fixtures/TestBundle/Entity/UidIdentified.php b/tests/Fixtures/TestBundle/Entity/UidIdentified.php new file mode 100644 index 00000000000..a3d45c509b1 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/UidIdentified.php @@ -0,0 +1,68 @@ + + * + * 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\Put; +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\Serializer\Attribute\Ignore; +use Symfony\Component\Serializer\Annotation\Ignore as LegacyIgnore; +use Symfony\Component\Serializer\Attribute\SerializedName; +use Symfony\Component\Serializer\Annotation\SerializedName as LegacySerializedName; +use Symfony\Component\Uid\Uuid; +use Symfony\Component\Validator\Constraints as Assert; + +#[Put( + allowCreate: true, + extraProperties: [ + 'standard_put' => true, + ] +)] +#[ORM\Entity] +class UidIdentified +{ + /** + * The entity ID. + */ + #[ORM\Id] + #[ORM\Column(type: 'integer')] + #[ORM\GeneratedValue] + #[ApiProperty(identifier: false)] + #[Ignore] + #[LegacyIgnore] + private ?int $id = null; + + #[ORM\Column(type: 'symfony_uuid', unique: true, nullable: false)] + #[ApiProperty(identifier: true)] + #[SerializedName('id')] + #[LegacySerializedName('id')] + private ?Uuid $uuid = null; + + /** + * A nice person. + */ + #[ORM\Column] + #[Assert\NotBlank] + public string $name = ''; + + public function getId(): ?int + { + return $this->id; + } + + public function getUuid(): ?Uuid + { + return $this->uuid; + } +} From 64d1578b2c3dd102bc54822fd2aa7dc8223ca9f7 Mon Sep 17 00:00:00 2001 From: soyuka Date: Tue, 19 Dec 2023 10:46:01 +0100 Subject: [PATCH 2/2] cs --- tests/Fixtures/TestBundle/Entity/UidIdentified.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Fixtures/TestBundle/Entity/UidIdentified.php b/tests/Fixtures/TestBundle/Entity/UidIdentified.php index a3d45c509b1..abcc9d94ec7 100644 --- a/tests/Fixtures/TestBundle/Entity/UidIdentified.php +++ b/tests/Fixtures/TestBundle/Entity/UidIdentified.php @@ -16,10 +16,10 @@ use ApiPlatform\Metadata\ApiProperty; use ApiPlatform\Metadata\Put; use Doctrine\ORM\Mapping as ORM; -use Symfony\Component\Serializer\Attribute\Ignore; use Symfony\Component\Serializer\Annotation\Ignore as LegacyIgnore; -use Symfony\Component\Serializer\Attribute\SerializedName; use Symfony\Component\Serializer\Annotation\SerializedName as LegacySerializedName; +use Symfony\Component\Serializer\Attribute\Ignore; +use Symfony\Component\Serializer\Attribute\SerializedName; use Symfony\Component\Uid\Uuid; use Symfony\Component\Validator\Constraints as Assert;