diff --git a/src/Sorter/TopologicalSorter.php b/src/Sorter/TopologicalSorter.php index a8c1ef81..29b4313f 100644 --- a/src/Sorter/TopologicalSorter.php +++ b/src/Sorter/TopologicalSorter.php @@ -8,8 +8,9 @@ use Doctrine\ORM\Mapping\ClassMetadata; use RuntimeException; -use function get_class; +use function count; use function sprintf; +use function uasort; /** * TopologicalSorter is an ordering algorithm for directed graphs (DG) and/or @@ -101,6 +102,9 @@ public function addDependency($fromHash, $toHash) */ public function sort() { + uasort($this->nodeList, static function (Vertex $a, Vertex $b) { + return count($a->dependencyList) > count($b->dependencyList) ? 1 : -1; + }); foreach ($this->nodeList as $definition) { if ($definition->state !== Vertex::NOT_VISITED) { continue; @@ -132,15 +136,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) { @@ -167,6 +163,18 @@ 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) { + continue; + } + + $this->visit($nestedChildDefinition); + } + break; case Vertex::NOT_VISITED: $this->visit($childDefinition); @@ -177,4 +185,17 @@ private function visit(Vertex $definition) $this->sortedNodeList[] = $definition->value; } + + private 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\'s not listed to be loaded.', + $definition->value->getName(), + $dependency, + )); + } + + return $this->nodeList[$dependency]; + } } diff --git a/tests/Common/DataFixtures/Sorter/TopologicalSorterTest.php b/tests/Common/DataFixtures/Sorter/TopologicalSorterTest.php index 1a1f8c23..506ace29 100644 --- a/tests/Common/DataFixtures/Sorter/TopologicalSorterTest.php +++ b/tests/Common/DataFixtures/Sorter/TopologicalSorterTest.php @@ -10,6 +10,13 @@ use Doctrine\Tests\Common\DataFixtures\BaseTestCase; use RuntimeException; +use function array_map; +use function array_search; +use function array_splice; +use function array_unshift; +use function count; +use function implode; + /** * TopologicalSorter tests. * @@ -76,28 +83,79 @@ public function testSuccessSortMultiDependency(): void self::assertSame($correctList, $sortedList); } - public function testSortCyclicDependency(): void + public static function cyclicNodePermutationsDataProvider(): iterable { - $sorter = new TopologicalSorter(); + $list[] = new ClassMetadata('1'); + $list[] = new ClassMetadata('2'); + $list[] = new ClassMetadata('3'); + $list[] = new ClassMetadata('4'); + $list[] = new ClassMetadata('5'); - $node1 = new ClassMetadata('1'); - $node2 = new ClassMetadata('2'); - $node3 = new ClassMetadata('3'); + foreach (self::permute($list) as $list) { + $label = 'Node order: ' . implode(',', self::nodeNames($list)); - $sorter->addNode('1', $node1); - $sorter->addNode('2', $node2); - $sorter->addNode('3', $node3); + yield $label => [$list]; + } + } - $sorter->addDependency('1', '2'); - $sorter->addDependency('2', '3'); - $sorter->addDependency('3', '1'); + /** @param ClassMetadata[] $list */ + private static function nodeNames(array $list): array + { + return array_map(static function (ClassMetadata $node) { + return $node->getName(); + }, $list); + } - $sortedList = $sorter->sort(); - $correctList = [$node3, $node2, $node1]; + private static function permute(array $items, array $permutation = [], array &$result = []): array + { + if (empty($items)) { + $result[] = $permutation; + } else { + for ($i = count($items) - 1; $i >= 0; --$i) { + $newItems = $items; + $newPermutation = $permutation; + [$item] = array_splice($newItems, $i, 1); + array_unshift($newPermutation, $item); + self::permute($newItems, $newPermutation, $result); + } + } + + return $result; + } - self::assertSame($correctList, $sortedList); + /** @dataProvider cyclicNodePermutationsDataProvider */ + public function testSortCyclicDependency(array $nodes): void + { + $sorter = new TopologicalSorter(); - $sorter->sort(); + foreach ($nodes as $node) { + $sorter->addNode($node->getName(), $node); + } + + $sorter->addDependency('1', '2'); + $sorter->addDependency('2', '3'); + $sorter->addDependency('3', '4'); + $sorter->addDependency('4', '2'); + $sorter->addDependency('2', '5'); + + $sortedList = self::nodeNames($sorter->sort()); + $node1Position = array_search('1', $sortedList); + $node2Position = array_search('2', $sortedList); + $node3Position = array_search('3', $sortedList); + $node4Position = array_search('4', $sortedList); + $node5Position = array_search('5', $sortedList); + + self::assertTrue($node5Position < $node2Position, '5 should come before 2'); + self::assertTrue($node5Position < $node3Position, '5 should come before 3'); + self::assertTrue($node5Position < $node4Position, '5 should come before 4'); + self::assertTrue($node1Position > $node5Position, '1 should come after 2'); + self::assertTrue($node1Position > $node3Position, '1 should come after 3'); + self::assertTrue($node1Position > $node4Position, '1 should come after 4'); + + // these a cyclic and sort order is based on the order of the nodes + self::assertContains('2', $sortedList); + self::assertContains('3', $sortedList); + self::assertContains('4', $sortedList); } public function testFailureSortCyclicDependency(): void