From 0e5f21f5b233e6655aa0d8476a30d607db89e7c7 Mon Sep 17 00:00:00 2001 From: Dylan van der Hout Date: Wed, 2 Sep 2026 16:22:56 +0200 Subject: [PATCH] fix(doctrine): do not replace new relations with assigned ids by references PersistProcessor::handleLazyObjectRelations() swapped any unmanaged related object holding a non-null identifier with a Doctrine reference, assuming the row already exists. With application-assigned identifiers (e.g. UUIDs), a freshly mapped nested entity was therefore never cascade-persisted, causing a foreign key violation on flush. Consult the unit of work state (STATE_NEW) before creating a reference so that new objects are left to cascade persist, while objects with database-generated ids or existing rows are still replaced by references. Fixes api-platform/core#8438 --- .../Common/State/PersistProcessor.php | 31 +++++ .../Tests/State/PersistProcessorTest.php | 131 ++++++++++++++++++ .../Issue8438TaskRuleConditionDto.php | 43 ++++++ .../Issue8438/Issue8438TaskRuleDto.php | 47 +++++++ .../Entity/Issue8438/Issue8438TaskRule.php | 42 ++++++ .../Issue8438/Issue8438TaskRuleCondition.php | 39 ++++++ tests/Functional/Doctrine/StateOptionTest.php | 71 +++++++++- 7 files changed, 403 insertions(+), 1 deletion(-) create mode 100644 tests/Fixtures/TestBundle/ApiResource/Issue8438/Issue8438TaskRuleConditionDto.php create mode 100644 tests/Fixtures/TestBundle/ApiResource/Issue8438/Issue8438TaskRuleDto.php create mode 100644 tests/Fixtures/TestBundle/Entity/Issue8438/Issue8438TaskRule.php create mode 100644 tests/Fixtures/TestBundle/Entity/Issue8438/Issue8438TaskRuleCondition.php diff --git a/src/Doctrine/Common/State/PersistProcessor.php b/src/Doctrine/Common/State/PersistProcessor.php index 85b331ddb0c..d1ad748c39a 100644 --- a/src/Doctrine/Common/State/PersistProcessor.php +++ b/src/Doctrine/Common/State/PersistProcessor.php @@ -17,6 +17,8 @@ use ApiPlatform\Metadata\Operation; use ApiPlatform\Metadata\Util\ClassInfoTrait; use ApiPlatform\State\ProcessorInterface; +use Doctrine\ODM\MongoDB\UnitOfWork as OdmUnitOfWork; +use Doctrine\ORM\UnitOfWork as OrmUnitOfWork; use Doctrine\Persistence\ManagerRegistry; use Doctrine\Persistence\ObjectManager as DoctrineObjectManager; @@ -221,9 +223,38 @@ private function handleLazyObjectRelations(object $data, DoctrineObjectManager $ continue; } + // Do not get reference for new objects with application-assigned identifiers (e.g. UUID generated in the constructor): + // the object does not exist in the database yet and must be handled by cascade persist + if ($this->isNewObject($relManager, $value)) { + continue; + } + \assert(method_exists($relManager, 'getReference')); $reflectionProperty->setValue($data, $relManager->getReference($relClass, $identifiers)); } } + + /** + * Checks if an object holding identifiers is new (not yet in the database) according to the unit of work. + * Objects with database-generated identifiers are never considered new here since Doctrine assumes they exist. + */ + private function isNewObject(DoctrineObjectManager $manager, object $object): bool + { + if (!method_exists($manager, 'getUnitOfWork')) { + return false; + } + + $unitOfWork = $manager->getUnitOfWork(); + + if (method_exists($unitOfWork, 'getEntityState')) { + return OrmUnitOfWork::STATE_NEW === $unitOfWork->getEntityState($object); + } + + if (method_exists($unitOfWork, 'getDocumentState')) { + return OdmUnitOfWork::STATE_NEW === $unitOfWork->getDocumentState($object); + } + + return false; + } } diff --git a/src/Doctrine/Common/Tests/State/PersistProcessorTest.php b/src/Doctrine/Common/Tests/State/PersistProcessorTest.php index 59da208971e..33390172b41 100644 --- a/src/Doctrine/Common/Tests/State/PersistProcessorTest.php +++ b/src/Doctrine/Common/Tests/State/PersistProcessorTest.php @@ -22,9 +22,13 @@ use ApiPlatform\Metadata\Put; use ApiPlatform\State\ProcessorInterface; use Doctrine\ODM\MongoDB\Mapping\ClassMetadata; +use Doctrine\ODM\MongoDB\UnitOfWork as ODMUnitOfWork; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadata as ORMClassMetadata; +use Doctrine\ORM\UnitOfWork as ORMUnitOfWork; use Doctrine\Persistence\ManagerRegistry; +use Doctrine\Persistence\Mapping\ClassMetadata as PersistenceClassMetadata; +use Doctrine\Persistence\Mapping\RuntimeReflectionService; use Doctrine\Persistence\ObjectManager; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -145,6 +149,60 @@ public function testHandleLazyObjectRelationsSkipsUninitializedProperties(): voi $this->assertSame($dummy, $result); } + public function testHandleLazyObjectRelationsKeepsNewOrmRelationWithAssignedIdentifier(): void + { + $relation = new PersistProcessorTestRelationStub(); + $data = new PersistProcessorTestObjectWithRelationStub($relation); + + $unitOfWork = $this->createMock(ORMUnitOfWork::class); + $unitOfWork->expects($this->once())->method('getEntityState')->with($relation)->willReturn(ORMUnitOfWork::STATE_NEW); + + $relationManager = $this->createMock(EntityManagerInterface::class); + $relationManager->method('contains')->with($relation)->willReturn(false); + $relationManager->method('getClassMetadata')->with(PersistProcessorTestRelationStub::class)->willReturn($this->createOrmRelationMetadata()); + $relationManager->method('getUnitOfWork')->willReturn($unitOfWork); + $relationManager->expects($this->never())->method('getReference'); + + $this->processObjectWithRelation($data, $relationManager); + + $this->assertSame($relation, $data->relation); + } + + public function testHandleLazyObjectRelationsKeepsNewOdmRelationWithAssignedIdentifier(): void + { + $relation = new PersistProcessorTestRelationStub(); + $data = new PersistProcessorTestObjectWithRelationStub($relation); + + $unitOfWork = $this->createMock(PersistProcessorTestDocumentUnitOfWorkStub::class); + $unitOfWork->expects($this->once())->method('getDocumentState')->with($relation)->willReturn(ODMUnitOfWork::STATE_NEW); + + $relationManager = $this->createMockForIntersectionOfInterfaces([ObjectManager::class, PersistProcessorTestDocumentManagerStub::class]); + $relationManager->method('contains')->with($relation)->willReturn(false); + $relationManager->method('getClassMetadata')->with(PersistProcessorTestRelationStub::class)->willReturn($this->createRelationMetadata()); + $relationManager->method('getUnitOfWork')->willReturn($unitOfWork); + $relationManager->expects($this->never())->method('getReference'); + + $this->processObjectWithRelation($data, $relationManager); + + $this->assertSame($relation, $data->relation); + } + + public function testHandleLazyObjectRelationsReplacesRelationWhenManagerHasNoUnitOfWork(): void + { + $relation = new PersistProcessorTestRelationStub(); + $reference = new PersistProcessorTestRelationStub(); + $data = new PersistProcessorTestObjectWithRelationStub($relation); + + $relationManager = $this->createMockForIntersectionOfInterfaces([ObjectManager::class, PersistProcessorTestReferenceManagerStub::class]); + $relationManager->method('contains')->with($relation)->willReturn(false); + $relationManager->method('getClassMetadata')->with(PersistProcessorTestRelationStub::class)->willReturn($this->createRelationMetadata()); + $relationManager->expects($this->once())->method('getReference')->with(PersistProcessorTestRelationStub::class, ['id' => 'relation-id'])->willReturn($reference); + + $this->processObjectWithRelation($data, $relationManager); + + $this->assertSame($reference, $data->relation); + } + public function testPersistPutCreateResolvesParentLinkViaToProperty(): void { $device = new PersistProcessorTestDeviceStub(); @@ -191,6 +249,41 @@ public function testPersistPutCreateResolvesParentLinkViaToProperty(): void $this->assertSame($userReference, $device->user); $this->assertSame('device-uuid', $device->id); } + + private function createRelationMetadata(): PersistenceClassMetadata + { + $metadata = $this->createStub(PersistenceClassMetadata::class); + $metadata->method('getIdentifierValues')->willReturn(['id' => 'relation-id']); + + return $metadata; + } + + private function createOrmRelationMetadata(): ORMClassMetadata + { + $metadata = new ORMClassMetadata(PersistProcessorTestRelationStub::class); + $metadata->mapField(['fieldName' => 'id', 'type' => 'string', 'id' => true]); + $metadata->wakeupReflection(new RuntimeReflectionService()); + + return $metadata; + } + + private function processObjectWithRelation(PersistProcessorTestObjectWithRelationStub $data, ObjectManager $relationManager): void + { + $dataManager = $this->createMock(ObjectManager::class); + $dataManager->method('getClassMetadata')->with(PersistProcessorTestObjectWithRelationStub::class)->willReturn($this->createStub(PersistenceClassMetadata::class)); + $dataManager->method('contains')->with($data)->willReturn(false); + $dataManager->expects($this->once())->method('persist')->with($data); + $dataManager->expects($this->once())->method('flush'); + $dataManager->expects($this->once())->method('refresh')->with($data); + + $managerRegistry = $this->createStub(ManagerRegistry::class); + $managerRegistry->method('getManagerForClass')->willReturnMap([ + [PersistProcessorTestObjectWithRelationStub::class, $dataManager], + [PersistProcessorTestRelationStub::class, $relationManager], + ]); + + (new PersistProcessor($managerRegistry))->process($data, new Post(map: true)); + } } /** @internal */ @@ -205,3 +298,41 @@ class PersistProcessorTestDeviceStub public ?string $id = null; public ?PersistProcessorTestUserStub $user = null; } + +/** @internal */ +class PersistProcessorTestObjectWithRelationStub +{ + public ?string $id = null; + + public function __construct(public PersistProcessorTestRelationStub $relation) + { + } +} + +/** @internal */ +class PersistProcessorTestRelationStub +{ + public string $id = 'relation-id'; +} + +/** @internal */ +interface PersistProcessorTestReferenceManagerStub +{ + /** + * @param class-string $className + * @param array $identifier + */ + public function getReference(string $className, array $identifier): object; +} + +/** @internal */ +interface PersistProcessorTestDocumentManagerStub extends PersistProcessorTestReferenceManagerStub +{ + public function getUnitOfWork(): PersistProcessorTestDocumentUnitOfWorkStub; +} + +/** @internal */ +interface PersistProcessorTestDocumentUnitOfWorkStub +{ + public function getDocumentState(object $object): int; +} diff --git a/tests/Fixtures/TestBundle/ApiResource/Issue8438/Issue8438TaskRuleConditionDto.php b/tests/Fixtures/TestBundle/ApiResource/Issue8438/Issue8438TaskRuleConditionDto.php new file mode 100644 index 00000000000..414d7115a9f --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/Issue8438/Issue8438TaskRuleConditionDto.php @@ -0,0 +1,43 @@ + + * + * 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\ApiResource\Issue8438; + +use ApiPlatform\Doctrine\Orm\State\Options; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Post; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue8438\Issue8438TaskRuleCondition; +use Symfony\Component\ObjectMapper\Attribute\Map; +use Symfony\Component\Uid\Uuid; + +#[ApiResource( + operations: [ + new Get(), + new Post(), + ], + shortName: 'Issue8438TaskRuleCondition', + stateOptions: new Options(entityClass: Issue8438TaskRuleCondition::class) +)] +#[Map(target: Issue8438TaskRuleCondition::class)] +class Issue8438TaskRuleConditionDto +{ + public Uuid $id; + public string $completeWhen = ''; + + public function __construct() + { + // Application-assigned identifier, generated before the entity is persisted + $this->id = Uuid::v7(); + } +} diff --git a/tests/Fixtures/TestBundle/ApiResource/Issue8438/Issue8438TaskRuleDto.php b/tests/Fixtures/TestBundle/ApiResource/Issue8438/Issue8438TaskRuleDto.php new file mode 100644 index 00000000000..f4542c2bd92 --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/Issue8438/Issue8438TaskRuleDto.php @@ -0,0 +1,47 @@ + + * + * 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\ApiResource\Issue8438; + +use ApiPlatform\Doctrine\Orm\State\Options; +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Post; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue8438\Issue8438TaskRule; +use Symfony\Component\ObjectMapper\Attribute\Map; +use Symfony\Component\Uid\Uuid; + +#[ApiResource( + operations: [ + new Get(), + new Post(), + ], + shortName: 'Issue8438TaskRule', + stateOptions: new Options(entityClass: Issue8438TaskRule::class) +)] +#[Map(target: Issue8438TaskRule::class)] +class Issue8438TaskRuleDto +{ + public Uuid $id; + public string $name = ''; + + #[ApiProperty(readableLink: true, writableLink: true)] + public ?Issue8438TaskRuleConditionDto $condition = null; + + public function __construct() + { + // Application-assigned identifier, generated before the entity is persisted + $this->id = Uuid::v7(); + } +} diff --git a/tests/Fixtures/TestBundle/Entity/Issue8438/Issue8438TaskRule.php b/tests/Fixtures/TestBundle/Entity/Issue8438/Issue8438TaskRule.php new file mode 100644 index 00000000000..42b9781961b --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Issue8438/Issue8438TaskRule.php @@ -0,0 +1,42 @@ + + * + * 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\Issue8438; + +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue8438\Issue8438TaskRuleDto; +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\ObjectMapper\Attribute\Map; +use Symfony\Component\Uid\Uuid; + +/** + * Owning side with an application-assigned identifier and cascade persist on the relation. + */ +#[ORM\Entity] +#[Map(target: Issue8438TaskRuleDto::class)] +class Issue8438TaskRule +{ + #[ORM\Id] + #[ORM\Column(type: 'symfony_uuid', unique: true)] + public Uuid $id; + + public function __construct( + #[ORM\Column(type: 'string', length: 255)] + public string $name = '', + #[ORM\ManyToOne(targetEntity: Issue8438TaskRuleCondition::class, cascade: ['persist'])] + #[ORM\JoinColumn(nullable: true)] + public ?Issue8438TaskRuleCondition $condition = null, + ?Uuid $id = null, + ) { + $this->id = $id ?? Uuid::v7(); + } +} diff --git a/tests/Fixtures/TestBundle/Entity/Issue8438/Issue8438TaskRuleCondition.php b/tests/Fixtures/TestBundle/Entity/Issue8438/Issue8438TaskRuleCondition.php new file mode 100644 index 00000000000..3edeaa6763e --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Issue8438/Issue8438TaskRuleCondition.php @@ -0,0 +1,39 @@ + + * + * 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\Issue8438; + +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue8438\Issue8438TaskRuleConditionDto; +use Doctrine\ORM\Mapping as ORM; +use Symfony\Component\ObjectMapper\Attribute\Map; +use Symfony\Component\Uid\Uuid; + +/** + * Related entity with an application-assigned identifier (no DB-generated id). + */ +#[ORM\Entity] +#[Map(target: Issue8438TaskRuleConditionDto::class)] +class Issue8438TaskRuleCondition +{ + #[ORM\Id] + #[ORM\Column(type: 'symfony_uuid', unique: true)] + public Uuid $id; + + public function __construct( + #[ORM\Column(type: 'string', length: 255)] + public string $completeWhen = '', + ?Uuid $id = null, + ) { + $this->id = $id ?? Uuid::v7(); + } +} diff --git a/tests/Functional/Doctrine/StateOptionTest.php b/tests/Functional/Doctrine/StateOptionTest.php index b77694aa74c..1c68953a1fa 100644 --- a/tests/Functional/Doctrine/StateOptionTest.php +++ b/tests/Functional/Doctrine/StateOptionTest.php @@ -17,9 +17,13 @@ use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue6039\UserApi; use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue7689\Issue7689CategoryDto; use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue7689\Issue7689ProductDto; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue8438\Issue8438TaskRuleConditionDto; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue8438\Issue8438TaskRuleDto; use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue6039\Issue6039EntityUser; use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue7689\Issue7689Category; use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue7689\Issue7689Product; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue8438\Issue8438TaskRule; +use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue8438\Issue8438TaskRuleCondition; use ApiPlatform\Tests\RecreateSchemaTrait; use ApiPlatform\Tests\SetupClassResourcesTrait; use Symfony\Component\ObjectMapper\Metadata\ReverseClassObjectMapperMetadataFactory; @@ -36,7 +40,7 @@ final class StateOptionTest extends ApiTestCase */ public static function getResources(): array { - return [UserApi::class, Issue7689ProductDto::class, Issue7689CategoryDto::class]; + return [UserApi::class, Issue7689ProductDto::class, Issue7689CategoryDto::class, Issue8438TaskRuleDto::class, Issue8438TaskRuleConditionDto::class]; } public function testDtoWithEntityClassOptionCollection(): void @@ -91,4 +95,69 @@ public function testPostWithEntityClassOption(): void $this->assertNotNull($product->category); $this->assertEquals(1, $product->category->getId()); } + + /** + * @see https://github.com/api-platform/core/issues/8438 + */ + public function testPostWithEntityClassOptionCascadePersistsNewRelationWithAssignedIdentifier(): void + { + if ($this->isMongoDB()) { + $this->markTestSkipped('MongoDB not tested.'); + } + + if (!class_exists(ReverseClassObjectMapperMetadataFactory::class)) { + $this->markTestSkipped('This test requires symfony/object-mapper >= 8.1'); + } + + $this->recreateSchema([Issue8438TaskRule::class, Issue8438TaskRuleCondition::class]); + $manager = static::getContainer()->get('doctrine')->getManager(); + + static::createClient()->request('POST', '/issue8438_task_rules', ['json' => [ + 'name' => 'rule', + 'condition' => ['completeWhen' => 'done'], + ]]); + $this->assertResponseStatusCodeSame(201); + $this->assertJsonContains(['name' => 'rule', 'condition' => ['completeWhen' => 'done']]); + + $this->assertCount(1, $manager->getRepository(Issue8438TaskRuleCondition::class)->findAll()); + $rule = $manager->getRepository(Issue8438TaskRule::class)->findOneBy(['name' => 'rule']); + $this->assertNotNull($rule); + $this->assertNotNull($rule->condition); + $this->assertSame('done', $rule->condition->completeWhen); + } + + /** + * @see https://github.com/api-platform/core/issues/8438 + */ + public function testPostWithEntityClassOptionReferencesExistingRelationWithAssignedIdentifier(): void + { + if ($this->isMongoDB()) { + $this->markTestSkipped('MongoDB not tested.'); + } + + if (!class_exists(ReverseClassObjectMapperMetadataFactory::class)) { + $this->markTestSkipped('This test requires symfony/object-mapper >= 8.1'); + } + + $this->recreateSchema([Issue8438TaskRule::class, Issue8438TaskRuleCondition::class]); + $manager = static::getContainer()->get('doctrine')->getManager(); + + $condition = new Issue8438TaskRuleCondition('existing'); + $manager->persist($condition); + $manager->flush(); + $manager->clear(); + + static::createClient()->request('POST', '/issue8438_task_rules', ['json' => [ + 'name' => 'rule', + 'condition' => '/issue8438_task_rule_conditions/'.$condition->id, + ]]); + $this->assertResponseStatusCodeSame(201); + + $this->assertCount(1, $manager->getRepository(Issue8438TaskRuleCondition::class)->findAll()); + $rule = $manager->getRepository(Issue8438TaskRule::class)->findOneBy(['name' => 'rule']); + $this->assertNotNull($rule); + $this->assertNotNull($rule->condition); + $this->assertTrue($condition->id->equals($rule->condition->id)); + $this->assertSame('existing', $rule->condition->completeWhen); + } }