Merge pull request #7260 from stof/regression_commit_order

Fix the handling of circular references in the commit order calculator

lib/Doctrine/ORM/Internal/CommitOrderCalculator.php

@@ -164,6 +164,17 @@ private function visit($vertex)
                 case self::IN_PROGRESS:
                     if (isset($adjacentVertex->dependencyList[$vertex->hash]) &&
                         $adjacentVertex->dependencyList[$vertex->hash]->weight < $edge->weight) {
+
+                        // If we have some non-visited dependencies in the in-progress dependency, we
+                        // need to visit them before adding the node.
+                        foreach ($adjacentVertex->dependencyList as $adjacentEdge) {
+                            $adjacentEdgeVertex = $this->nodeList[$adjacentEdge->to];
+
+                            if ($adjacentEdgeVertex->state === self::NOT_VISITED) {
+                                $this->visit($adjacentEdgeVertex);
+                            }
+                        }
+
                         $adjacentVertex->state = self::VISITED;
 
                         $this->sortedNodeList[] = $adjacentVertex->value;

tests/Doctrine/Tests/ORM/CommitOrderCalculatorTest.php

@@ -67,6 +67,39 @@ public function testCommitOrdering2()
 
         $this->assertSame($correctOrder, $sorted);
     }
+
+    public function testCommitOrdering3()
+    {
+        // this test corresponds to the GH7259Test::testPersistFileBeforeVersion functional test
+        $class1 = new ClassMetadata(NodeClass1::class);
+        $class2 = new ClassMetadata(NodeClass2::class);
+        $class3 = new ClassMetadata(NodeClass3::class);
+        $class4 = new ClassMetadata(NodeClass4::class);
+
+        $this->_calc->addNode($class1->name, $class1);
+        $this->_calc->addNode($class2->name, $class2);
+        $this->_calc->addNode($class3->name, $class3);
+        $this->_calc->addNode($class4->name, $class4);
+
+        $this->_calc->addDependency($class4->name, $class1->name, 1);
+        $this->_calc->addDependency($class1->name, $class2->name, 1);
+        $this->_calc->addDependency($class4->name, $class3->name, 1);
+        $this->_calc->addDependency($class1->name, $class4->name, 0);
+
+        $sorted = $this->_calc->sort();
+
+        // There is only multiple valid ordering for this constellation, but
+        // the class4, class1, class2 ordering is important to break the cycle
+        // on the nullable link.
+        $correctOrders = [
+            [$class4, $class1, $class2, $class3],
+            [$class4, $class1, $class3, $class2],
+            [$class4, $class3, $class1, $class2],
+        ];
+
+        // We want to perform a strict comparison of the array
+        $this->assertContains($sorted, $correctOrders, '', false, true, true);
+    }
 }
 
 class NodeClass1 {}

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

@@ -0,0 +1,165 @@
+<?php
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+final class GH7259Test extends OrmFunctionalTestCase
+{
+    protected function setUp() : void
+    {
+        parent::setUp();
+
+        $this->setUpEntitySchema([GH7259Space::class, GH7259File::class, GH7259FileVersion::class, GH7259Feed::class]);
+    }
+
+    /**
+     * @group 7259
+     */
+    public function testPersistFileBeforeVersion() : void
+    {
+        $space = new GH7259Space();
+
+        $this->_em->persist($space);
+        $this->_em->flush();
+
+        $feed = new GH7259Feed();
+        $feed->space = $space;
+
+        $file = new GH7259File();
+        $file->space = $space;
+        $fileVersion = new GH7259FileVersion();
+        $fileVersion->file = $file;
+
+        $this->_em->persist($file);
+        $this->_em->persist($fileVersion);
+        $this->_em->persist($feed);
+
+        $this->_em->flush();
+
+        self::assertNotNull($fileVersion->id);
+    }
+
+    /**
+     * @group 7259
+     */
+    public function testPersistFileAfterVersion() : void
+    {
+        $space = new GH7259Space();
+
+        $this->_em->persist($space);
+        $this->_em->flush();
+        $this->_em->clear();
+
+        $space = $this->_em->find(GH7259Space::class, $space->id);
+
+        $feed = new GH7259Feed();
+        $feed->space = $space;
+
+        $file = new GH7259File();
+        $file->space = $space;
+        $fileVersion = new GH7259FileVersion();
+        $fileVersion->file = $file;
+
+        $this->_em->persist($fileVersion);
+        $this->_em->persist($file);
+        $this->_em->persist($feed);
+
+        $this->_em->flush();
+
+        self::assertNotNull($fileVersion->id);
+    }
+}
+
+/**
+ * @Entity()
+ */
+class GH7259File
+{
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ManyToOne(targetEntity=GH7259Space::class)
+     * @JoinColumn(nullable=false)
+     *
+     * @var GH7259Space|null
+     */
+    public $space;
+}
+
+/**
+ * @Entity()
+ */
+class GH7259FileVersion
+{
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ManyToOne(targetEntity=GH7259File::class)
+     * @JoinColumn(nullable=false)
+     *
+     * @var GH7259File|null
+     */
+    public $file;
+}
+
+/**
+ * @Entity()
+ */
+class GH7259Space
+{
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ManyToOne(targetEntity=GH7259File::class)
+     * @JoinColumn(nullable=true)
+     *
+     * @var GH7259File|null
+     */
+    public $ruleFile;
+}
+
+/**
+ * @Entity()
+ */
+class GH7259Feed
+{
+    /**
+     * @Id
+     * @GeneratedValue
+     * @Column(type="integer")
+     *
+     * @var int
+     */
+    public $id;
+
+    /**
+     * @ManyToOne(targetEntity=GH7259Space::class)
+     * @JoinColumn(nullable=false)
+     *
+     * @var GH7259Space|null
+     */
+    public $space;
+}