From 2d301aae722fca748cc8393658f86fdcd16e0707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josef=20=C5=A0upka?= Date: Tue, 8 Feb 2022 10:53:09 +0100 Subject: [PATCH] Compute particular commit order for deletions This fixes a bug which arises in some cases when trying to delete two entities which have a circular reference through other entities. UnifOfWork computed the order which was not correct for deletions and it failed with ForeignKeyConstraintViolationException. To fix that, this adds a new particular function getCommitOrderForDeletions() to compute the right order. This function is similar to getCommitOrder(), but it operates only with entities set to be deleted. This also adds test for this issue. --- lib/Doctrine/ORM/UnitOfWork.php | 48 ++++- psalm-baseline.xml | 2 +- .../ORM/Functional/Ticket/GH9376Test.php | 204 ++++++++++++++++++ 3 files changed, 251 insertions(+), 3 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/GH9376Test.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index cd5d727167a..1b98a7eb49c 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -444,8 +444,9 @@ public function commit($entity = null) // Entity deletions come last and need to be in reverse commit order if ($this->entityDeletions) { - for ($count = count($commitOrder), $i = $count - 1; $i >= 0 && $this->entityDeletions; --$i) { - $this->executeDeletions($commitOrder[$i]); + $commitOrderForDeletions = $this->getCommitOrderForDeletions(); + for ($count = count($commitOrderForDeletions), $i = $count - 1; $i >= 0 && $this->entityDeletions; --$i) { + $this->executeDeletions($commitOrderForDeletions[$i]); } } @@ -1330,6 +1331,49 @@ private function getCommitOrder(): array return $calc->sort(); } + /** + * Gets the commit order for deletions. + * + * @return list + */ + private function getCommitOrderForDeletions(): array + { + $calc = $this->getCommitOrderCalculator(); + + $newNodes = []; + + foreach ($this->entityDeletions as $entity) { + $class = $this->em->getClassMetadata(get_class($entity)); + + if ($calc->hasNode($class->name)) { + continue; + } + + $calc->addNode($class->name, $class); + + $newNodes[] = $class; + } + + // Calculate dependencies for new nodes + while ($class = array_pop($newNodes)) { + foreach ($class->associationMappings as $assoc) { + if (! ($assoc['isOwningSide'] && $assoc['type'] & ClassMetadata::TO_ONE)) { + continue; + } + + $targetClass = $this->em->getClassMetadata($assoc['targetEntity']); + + $joinColumns = reset($assoc['joinColumns']); + + if ($calc->hasNode($targetClass->name)) { + $calc->addDependency($targetClass->name, $class->name, (int) empty($joinColumns['nullable'])); + } + } + } + + return $calc->sort(); + } + /** * Schedules an entity for insertion into the database. * If the entity already has an identifier, it will be added to the identity map. diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 73538651193..b77d123ff17 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -3452,7 +3452,7 @@ $class $collectionToDelete $collectionToUpdate - $commitOrder[$i] + $commitOrderForDeletions[$i] ! is_object($object) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH9376Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH9376Test.php new file mode 100644 index 00000000000..0adaf976875 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH9376Test.php @@ -0,0 +1,204 @@ +_schemaTool->createSchema([ + $this->_em->getClassMetadata(GH9376GiftVariant::class), + $this->_em->getClassMetadata(GH9376OrderGiftVariant::class), + $this->_em->getClassMetadata(GH9376Order::class), + $this->_em->getClassMetadata(GH9376ProductPartner::class), + $this->_em->getClassMetadata(GH9376Product::class), + $this->_em->getClassMetadata(GH9376Gift::class), + ]); + } + + protected function tearDown(): void + { + $this->_schemaTool->dropSchema([ + $this->_em->getClassMetadata(GH9376GiftVariant::class), + $this->_em->getClassMetadata(GH9376OrderGiftVariant::class), + $this->_em->getClassMetadata(GH9376Order::class), + $this->_em->getClassMetadata(GH9376ProductPartner::class), + $this->_em->getClassMetadata(GH9376Product::class), + $this->_em->getClassMetadata(GH9376Gift::class), + ]); + + parent::tearDown(); + } + + public function testIssueRemove(): void + { + $product = new GH9376Product(); + $gift = new GH9376Gift($product); + $giftVariant = new GH9376GiftVariant($gift); + + $this->_em->persist($product); + $this->_em->persist($gift); + $this->_em->persist($giftVariant); + $this->_em->flush(); + $this->_em->clear(); + + $persistedGiftVariant = $this->_em->find(GH9376GiftVariant::class, 1); + $this->_em->remove($persistedGiftVariant); + + $persistedGift = $this->_em->find(GH9376Gift::class, 1); + $this->_em->remove($persistedGift); + + $this->_em->flush(); + $this->_em->clear(); + + self::assertEmpty($this->_em->getRepository(GH9376Gift::class)->findAll()); + self::assertEmpty($this->_em->getRepository(GH9376GiftVariant::class)->findAll()); + } +} + +/** + * @Entity + */ +class GH9376GiftVariant +{ + /** + * @var int + * @Id + * @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @ORM\ManyToOne(targetEntity=GH9376Gift::class) + * @ORM\JoinColumn(nullable=false) + * + * @var GH9376Gift + */ + public $gift; + + public function __construct(GH9376Gift $gift) + { + $this->gift = $gift; + } +} + +/** + * @Entity + */ +class GH9376OrderGiftVariant +{ + /** + * @var int + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @ORM\ManyToOne(targetEntity=GH9376GiftVariant::class) + * @ORM\JoinColumn(nullable=true) + * + * @var GH9376GiftVariant|null + */ + public $giftVariant; +} + +/** + * @Entity + */ +class GH9376Order +{ + /** + * @var int + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @ORM\OneToOne(targetEntity=GH9376OrderGiftVariant::class, cascade={"persist"}) + * + * @var GH9376OrderGiftVariant|null + */ + public $orderGiftVariant; +} + +/** + * @Entity + */ +class GH9376ProductPartner +{ + /** + * @var int + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @ORM\OneToOne(targetEntity=GH9376Order::class) + * @ORM\JoinColumn(nullable=true) + * + * @var GH9376Order|null + */ + public $order = null; +} + +/** + * @Entity + */ +class GH9376Product +{ + /** + * @var int + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @ORM\OneToOne(targetEntity=GH9376ProductPartner::class) + * @ORM\JoinColumn(nullable=true) + * + * @var GH9376ProductPartner|null + */ + public $productPartner = null; +} + +/** + * @Entity + */ +class GH9376Gift +{ + /** + * @var int + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @ORM\ManyToOne(targetEntity=GH9376Product::class) + * @ORM\JoinColumn(nullable=false) + * + * @var GH9376Product + */ + public $product; + + public function __construct(GH9376Product $product) + { + $this->product = $product; + } +}