Revert changes made to CommitOrder* - use a new TopologicalSort instead

lib/Doctrine/ORM/Internal/CommitOrder/Edge.php

@@ -8,27 +8,27 @@
 final class Edge
 {
     /**
-     * @var int
+     * @var string
      * @readonly
      */
     public $from;
 
     /**
-     * @var int
+     * @var string
      * @readonly
      */
     public $to;
 
     /**
-     * @var bool
+     * @var int
      * @readonly
      */
-    public $optional;
+    public $weight;
 
-    public function __construct(int $from, int $to, bool $optional)
+    public function __construct(string $from, string $to, int $weight)
     {
-        $this->from     = $from;
-        $this->to       = $to;
-        $this->optional = $optional;
+        $this->from   = $from;
+        $this->to     = $to;
+        $this->weight = $weight;
     }
 }

lib/Doctrine/ORM/Internal/CommitOrder/Vertex.php

@@ -4,11 +4,13 @@
 
 namespace Doctrine\ORM\Internal\CommitOrder;
 
+use Doctrine\ORM\Mapping\ClassMetadata;
+
 /** @internal */
 final class Vertex
 {
     /**
-     * @var int
+     * @var string
      * @readonly
      */
     public $hash;
@@ -20,16 +22,15 @@ final class Vertex
     public $state = VertexState::NOT_VISITED;
 
     /**
-     * @var object
+     * @var ClassMetadata
      * @readonly
      */
     public $value;
 
-    /** @var array<int, Edge> */
+    /** @var array<string, Edge> */
     public $dependencyList = [];
 
-    /** @param object $value */
-    public function __construct(int $hash, $value)
+    public function __construct(string $hash, ClassMetadata $value)
     {
         $this->hash  = $hash;
         $this->value = $value;

lib/Doctrine/ORM/Internal/CommitOrderCalculator.php

@@ -4,10 +4,10 @@
 
 namespace Doctrine\ORM\Internal;
 
-use Doctrine\ORM\Internal\CommitOrder\CycleDetectedException;
 use Doctrine\ORM\Internal\CommitOrder\Edge;
 use Doctrine\ORM\Internal\CommitOrder\Vertex;
 use Doctrine\ORM\Internal\CommitOrder\VertexState;
+use Doctrine\ORM\Mapping\ClassMetadata;
 
 use function array_reverse;
 
@@ -34,61 +34,81 @@ class CommitOrderCalculator
      *
      * Keys are provided hashes and values are the node definition objects.
      *
-     * @var array<int, Vertex>
+     * @var array<string, Vertex>
      */
     private $nodeList = [];
 
     /**
      * Volatile variable holding calculated nodes during sorting process.
      *
-     * @psalm-var array<int, object>
+     * @psalm-var list<ClassMetadata>
      */
     private $sortedNodeList = [];
 
     /**
      * Checks for node (vertex) existence in graph.
+     *
+     * @param string $hash
+     *
+     * @return bool
      */
-    public function hasNode(int $hash): bool
+    public function hasNode($hash)
     {
         return isset($this->nodeList[$hash]);
     }
 
     /**
      * Adds a new node (vertex) to the graph, assigning its hash and value.
      *
-     * @param object $node
+     * @param string        $hash
+     * @param ClassMetadata $node
+     *
+     * @return void
      */
-    public function addNode(int $hash, $node): void
+    public function addNode($hash, $node)
     {
         $this->nodeList[$hash] = new Vertex($hash, $node);
     }
 
     /**
      * Adds a new dependency (edge) to the graph using their hashes.
+     *
+     * @param string $fromHash
+     * @param string $toHash
+     * @param int    $weight
+     *
+     * @return void
      */
-    public function addDependency(int $fromHash, int $toHash, bool $optional): void
+    public function addDependency($fromHash, $toHash, $weight)
     {
         $this->nodeList[$fromHash]->dependencyList[$toHash]
-            = new Edge($fromHash, $toHash, $optional);
+            = new Edge($fromHash, $toHash, $weight);
     }
 
     /**
-     * Returns a topological sort of all nodes. When we have a dependency A->B between two nodes
-     * A and B, then A will be listed before B in the result.
+     * Return a valid order list of all current nodes.
+     * The desired topological sorting is the reverse post order of these searches.
      *
      * {@internal Highly performance-sensitive method.}
      *
-     * @psalm-return array<int, object>
+     * @psalm-return list<ClassMetadata>
      */
     public function sort()
     {
-        foreach (array_reverse($this->nodeList) as $vertex) {
-            if ($vertex->state === VertexState::NOT_VISITED) {
-                $this->visit($vertex);
+        foreach ($this->nodeList as $vertex) {
+            if ($vertex->state !== VertexState::NOT_VISITED) {
+                continue;
             }
+
+            $this->visit($vertex);
         }
 
-        return array_reverse($this->sortedNodeList, true);
+        $sortedList = $this->sortedNodeList;
+
+        $this->nodeList       = [];
+        $this->sortedNodeList = [];
+
+        return array_reverse($sortedList);
     }
 
     /**
@@ -98,45 +118,47 @@ public function sort()
      */
     private function visit(Vertex $vertex): void
     {
-        if ($vertex->state === VertexState::IN_PROGRESS) {
-            // This node is already on the current DFS stack. We've found a cycle!
-            throw new CycleDetectedException();
-        }
-
-        if ($vertex->state === VertexState::VISITED) {
-            // We've reached a node that we've already seen, including all
-            // other nodes that are reachable from here. We're done here, return.
-            return;
-        }
-
         $vertex->state = VertexState::IN_PROGRESS;
 
-        // Continue the DFS downwards the edge list
         foreach ($vertex->dependencyList as $edge) {
             $adjacentVertex = $this->nodeList[$edge->to];
 
-            try {
-                $this->visit($adjacentVertex);
-            } catch (CycleDetectedException $exception) {
-                if ($edge->optional) {
-                    // A cycle was found, and $edge is the closest edge while backtracking.
-                    // Skip this edge, continue with the next one.
-                    continue;
-                }
-
-                // We have found a cycle and cannot break it at $edge. Best we can do
-                // is to retreat from the current vertex, hoping that somewhere up the
-                // stack this can be salvaged.
-                $vertex->state = VertexState::NOT_VISITED;
-
-                throw $exception;
+            switch ($adjacentVertex->state) {
+                case VertexState::VISITED:
+                    // Do nothing, since node was already visited
+                    break;
+
+                case VertexState::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 === VertexState::NOT_VISITED) {
+                                $this->visit($adjacentEdgeVertex);
+                            }
+                        }
+
+                        $adjacentVertex->state = VertexState::VISITED;
+
+                        $this->sortedNodeList[] = $adjacentVertex->value;
+                    }
+
+                    break;
+
+                case VertexState::NOT_VISITED:
+                    $this->visit($adjacentVertex);
             }
         }
 
-        // We have traversed all edges and visited all other nodes reachable from here.
-        // So we're done with this vertex as well.
+        if ($vertex->state !== VertexState::VISITED) {
+            $vertex->state = VertexState::VISITED;
 
-        $vertex->state                       = VertexState::VISITED;
-        $this->sortedNodeList[$vertex->hash] = $vertex->value;
+            $this->sortedNodeList[] = $vertex->value;
+        }
     }
 }

lib/Doctrine/ORM/UnitOfWork.php

@@ -2551,16 +2551,9 @@ public function lock($entity, int $lockMode, $lockVersion = null): void
     /**
      * Gets the CommitOrderCalculator used by the UnitOfWork to order commits.
      *
-     * @deprecated use createCommitOrderCalculator() instead
-     *
      * @return CommitOrderCalculator
      */
     public function getCommitOrderCalculator()
-    {
-        return $this->createCommitOrderCalculator();
-    }
-
-    public function createCommitOrderCalculator(): CommitOrderCalculator
     {
         return new Internal\CommitOrderCalculator();
     }