Fix order of sorting for cyclic dependencies

where multiple dependencies are assumed to always go after the cyclic one
doctrine · Jun 20, 2024 · 7c7546a · 7c7546a
1 parent bf6c619
commit 7c7546a
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 11 deletions.
diff --git a/src/Sorter/TopologicalSorter.php b/src/Sorter/TopologicalSorter.php
@@ -132,15 +132,7 @@ private function visit(Vertex $definition)
         $definition->state = Vertex::IN_PROGRESS;
 
         foreach ($definition->dependencyList as $dependency) {
-            if (! isset($this->nodeList[$dependency])) {
-                throw new RuntimeException(sprintf(
-                    'Fixture "%s" has a dependency of fixture "%s", but it not listed to be loaded.',
-                    get_class($definition->value),
-                    $dependency,
-                ));
-            }
-
-            $childDefinition = $this->nodeList[$dependency];
+            $childDefinition = $this->getDefinition($dependency, $definition);
 
             // allow self referencing classes
             if ($definition === $childDefinition) {
@@ -166,6 +158,15 @@ private function visit(Vertex $definition)
                             ),
                         );
                     }
+                    // first do the rest of the unvisited children of the "in_progress" child before we continue
+                    // with the current one, to be sure that there are no other dependencies that need
+                    // to be cleared before this ($definition) node
+                    foreach ($childDefinition->dependencyList as $childDependency) {
+                        $nestedChildDefinition = $this->getDefinition($childDependency, $childDefinition);
+                        if ($nestedChildDefinition->state === Vertex::NOT_VISITED) {
+                            $this->visit($nestedChildDefinition);
+                        }
+                    }
 
                     break;
                 case Vertex::NOT_VISITED:
@@ -177,4 +178,17 @@ private function visit(Vertex $definition)
 
         $this->sortedNodeList[] = $definition->value;
     }
+
+    public function getDefinition(string $dependency, Vertex $definition): Vertex
+    {
+        if (! isset($this->nodeList[$dependency])) {
+            throw new RuntimeException(sprintf(
+                'Fixture "%s" has a dependency of fixture "%s", but it not listed to be loaded.',
+                get_class($definition->value),
+                $dependency,
+            ));
+        }
+
+        return $this->nodeList[$dependency];
+    }
 }
diff --git a/tests/Common/DataFixtures/Sorter/TopologicalSorterTest.php b/tests/Common/DataFixtures/Sorter/TopologicalSorterTest.php
@@ -83,17 +83,53 @@ public function testSortCyclicDependency(): void
         $node1 = new ClassMetadata('1');
         $node2 = new ClassMetadata('2');
         $node3 = new ClassMetadata('3');
+        $node4 = new ClassMetadata('4');
+        $node5 = new ClassMetadata('5');
 
         $sorter->addNode('1', $node1);
         $sorter->addNode('2', $node2);
         $sorter->addNode('3', $node3);
+        $sorter->addNode('4', $node4);
+        $sorter->addNode('5', $node5);
 
         $sorter->addDependency('1', '2');
+        $sorter->addDependency('1', '4');
         $sorter->addDependency('2', '3');
-        $sorter->addDependency('3', '1');
+        $sorter->addDependency('5', '1');
+        $sorter->addDependency('3', '4');
+
+        $sortedList  = $sorter->sort();
+        $correctList = [$node4, $node3, $node2, $node1, $node5];
+
+        self::assertSame($correctList, $sortedList);
+
+        $sorter->sort();
+    }
+
+    public function testSortCyclicDependencyWhenNodesAreAddedInAnotherOrder(): void
+    {
+        $sorter = new TopologicalSorter();
+
+        $node1 = new ClassMetadata('1');
+        $node2 = new ClassMetadata('2');
+        $node3 = new ClassMetadata('3');
+        $node4 = new ClassMetadata('4');
+        $node5 = new ClassMetadata('5');
+
+        $sorter->addNode('5', $node5);
+        $sorter->addNode('4', $node4);
+        $sorter->addNode('3', $node3);
+        $sorter->addNode('2', $node2);
+        $sorter->addNode('1', $node1);
+
+        $sorter->addDependency('1', '2');
+        $sorter->addDependency('1', '4');
+        $sorter->addDependency('2', '3');
+        $sorter->addDependency('5', '1');
+        $sorter->addDependency('3', '4');
 
         $sortedList  = $sorter->sort();
-        $correctList = [$node3, $node2, $node1];
+        $correctList = [$node4, $node3, $node2, $node1, $node5];
 
         self::assertSame($correctList, $sortedList);