Include ON DELETE CASCADE associations in the delete order computation

(description to be written)

Fixes #10912.

lib/Doctrine/ORM/UnitOfWork.php

@@ -1380,20 +1380,6 @@ private function computeDeleteExecutionOrder(): array
                     continue;
                 }
 
-                // For associations that implement a database-level cascade/set null operation,
-                // we do not have to follow a particular order: If the referred-to entity is
-                // deleted first, the DBMS will either delete the current $entity right away
-                // (CASCADE) or temporarily set the foreign key to NULL (SET NULL).
-                // Either way, we can skip it in the computation.
-                assert(isset($assoc['joinColumns']));
-                $joinColumns = reset($assoc['joinColumns']);
-                if (isset($joinColumns['onDelete'])) {
-                    $onDeleteOption = strtolower($joinColumns['onDelete']);
-                    if ($onDeleteOption === 'cascade' || $onDeleteOption === 'set null') {
-                        continue;
-                    }
-                }
-
                 $targetEntity = $class->getFieldValue($entity, $assoc['fieldName']);
 
                 // If the association does not refer to another entity or that entity
@@ -1403,9 +1389,53 @@ private function computeDeleteExecutionOrder(): array
                     continue;
                 }
 
+                $optionalDependency = false;
+
+                // Check if there is a database-level cascade operation that would affect
+                // $entity when $targetEntity is removed.
+                assert(isset($assoc['joinColumns']));
+                $joinColumns = reset($assoc['joinColumns']);
+                if (isset($joinColumns['onDelete'])) {
+                    $onDeleteOption = strtolower($joinColumns['onDelete']);
+                    // SET NULL: Deletion of $targetEntity would NULL out the association
+                    // at $entity. This is unproblematic: We don't care (with regard to
+                    // that particular association) whether $targetEntity is deleted first,
+                    // since the cascading operation would stop at $entity and leave it in
+                    // place. Skip this association.
+                    if ($onDeleteOption === 'set null') {
+                        continue;
+                    }
+
+                    // CASCADE deletes: Deletion of $targetEntity means that $entity
+                    // will be removed by the DBMS at the same time. So, when there
+                    // are other entities that depend upon and need to be removed
+                    // before $entity, they transitively also need to be removed
+                    // before $targetEntity. That means we need to consider this
+                    // as a dependency edge.
+                    //
+                    // There may be cases (see GH-10912) where we have circular
+                    // ON DELETE CASCADE constraints at the RDBMS level, but it
+                    // seems DBMSes can deal with this just fine. In order not to
+                    // bail out with a CircularReferenceException, treat this kind
+                    // of association as "optional" - this means cycles can be broken
+                    // when necessary. As long as there are no cycles, the order
+                    // is honored.
+                    //
+                    // I am not sure this 100% correct, considering the following case:
+                    // A has a not-nullable FK to B
+                    // B has a not-nullable FK to C
+                    // A has a FK to C with CASCADE DELETE, and vice versa
+                    // C has a FK to A with CASCADE DELETE.
+                    // This is unsolvable, since deleting A or C will immediately remove the
+                    // other one as well; but we also require to remove A _before_ B _before_ C.
+                    if ($onDeleteOption === 'cascade') {
+                        $optionalDependency = true;
+                    }
+                }
+
                 // Add dependency. The dependency direction implies that "$entity has to be removed before $targetEntity",
                 // so we can work through the topo sort result from left to right (with all edges pointing right).
-                $sort->addEdge($entity, $targetEntity, false);
+                $sort->addEdge($entity, $targetEntity, $optionalDependency);
             }
         }
 

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

@@ -0,0 +1,166 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+use function array_filter;
+use function array_values;
+use function strpos;
+
+class GH10912Test extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->setUpEntitySchema([
+            GH10912User::class,
+            GH10912Profile::class,
+            GH10912Room::class,
+        ]);
+    }
+
+    public function testIssue(): void
+    {
+        $user    = new GH10912User();
+        $profile = new GH10912Profile();
+        $room    = new GH10912Room();
+
+        $user->rooms->add($room);
+        $user->profile = $profile;
+        $profile->user = $user;
+        $room->user    = $user;
+
+        $this->_em->persist($room);
+        $this->_em->persist($user);
+        $this->_em->persist($profile);
+        $this->_em->flush();
+
+        /*
+         * This issue is about finding a special deletion order:
+         * $user and $profile cross-reference each other with ON DELETE CASCADE.
+         * So, whichever one gets deleted first, the DBMS will immediately dispose
+         * of the other one as well.
+         *
+         * $user -> $room is the unproblematic (irrelevant) inverse side of
+         * a OneToMany association.
+         *
+         * $room -> $user is a not-nullable, no DBMS-level-cascade, owning side
+         * of ManyToOne. We *must* remove the $room _before_ the $user can be
+         * deleted. And remember, $user deletion happens either when we DELETE the
+         * user (direct deletion), or when we delete the $profile (ON DELETE CASCADE
+         * propagates to the user).
+         *
+         * In the original bug report, the ordering of fields in the entities was
+         * relevant, in combination with a cascade=persist configuration.
+         *
+         * But, for the sake of clarity, let's put these features away and create
+         * the problematic sequence in UnitOfWork::$entityDeletions directly:
+         */
+        $this->_em->remove($profile);
+        $this->_em->remove($user);
+        $this->_em->remove($room);
+
+        $queryLog = $this->getQueryLog();
+        $queryLog->reset()->enable();
+
+        $this->_em->flush();
+
+        $queries = array_values(array_filter($queryLog->queries, static function ($entry) {
+            return strpos($entry['sql'], 'DELETE') === 0;
+        }));
+
+        self::assertCount(3, $queries);
+
+        // $room deletion is the first query
+        // we do not care about the order of $user vs. $profile, so do not check them.
+        self::assertSame('DELETE FROM GH10912Room WHERE id = ?', $queries[0]['sql']);
+
+        // The EntityManager is aware that all three entities have been deleted (sanity check)
+        $im = $this->_em->getUnitOfWork()->getIdentityMap();
+        self::assertEmpty($im[GH10912Profile::class]);
+        self::assertEmpty($im[GH10912User::class]);
+        self::assertEmpty($im[GH10912Room::class]);
+    }
+}
+
+/** @ORM\Entity */
+class GH10912User
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ORM\OneToMany(targetEntity=GH10912Room::class, mappedBy="user")
+     *
+     * @var Collection<int, GH10912Room>
+     */
+    public $rooms;
+
+    /**
+     * @ORM\OneToOne(targetEntity=GH10912Profile::class)
+     * @ORM\JoinColumn(onDelete="cascade")
+     *
+     * @var GH10912Profile
+     */
+    public $profile;
+
+    public function __construct()
+    {
+        $this->rooms = new ArrayCollection();
+    }
+}
+
+/** @ORM\Entity */
+class GH10912Profile
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ORM\OneToOne(targetEntity=GH10912User::class)
+     * @ORM\JoinColumn(onDelete="cascade")
+     *
+     * @var GH10912User
+     */
+    public $user;
+}
+
+/** @ORM\Entity */
+class GH10912Room
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ORM\ManyToOne(targetEntity=GH10912User::class, inversedBy="rooms")
+     * @ORM\JoinColumn(nullable=false)
+     *
+     * @var GH10912User
+     */
+    public $user;
+}