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

@@ -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<object>
+     */
+    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.

psalm-baseline.xml

@@ -3452,7 +3452,7 @@
       <code>$class</code>
       <code>$collectionToDelete</code>
       <code>$collectionToUpdate</code>
-      <code>$commitOrder[$i]</code>
+      <code>$commitOrderForDeletions[$i]</code>
     </ArgumentTypeCoercion>
     <DocblockTypeContradiction occurrences="4">
       <code>! is_object($object)</code>

tests/Doctrine/Tests/ORM/Functional/Ticket/GH9376Test.php

@@ -0,0 +1,204 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\ORM\Mapping\Column;
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\GeneratedValue;
+use Doctrine\ORM\Mapping\Id;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+class GH9376Test extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->_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;
+    }
+}