diff --git a/lib/internal/Magento/Framework/Data/Graph.php b/lib/internal/Magento/Framework/Data/Graph.php index fb680bb6870d0..850ffa1a93fb3 100644 --- a/lib/internal/Magento/Framework/Data/Graph.php +++ b/lib/internal/Magento/Framework/Data/Graph.php @@ -76,6 +76,26 @@ public function addRelation($fromNode, $toNode) return $this; } + /** + * Unset a relation between nodes + * + * @param string|int $fromNode + * @param string|int $toNode + * @return $this + * @throws \InvalidArgumentException + */ + public function removeRelation($fromNode, $toNode) + { + if ($fromNode == $toNode) { + throw new \InvalidArgumentException("Graph node '{$fromNode}' is linked to itself."); + } + $this->_assertNode($fromNode, true); + $this->_assertNode($toNode, true); + unset($this->_from[$fromNode][$toNode]); + unset($this->_to[$toNode][$fromNode]); + return $this; + } + /** * Export relations between nodes. Can return inverse relations * @@ -181,6 +201,75 @@ public function dfs($fromNode, $toNode, $mode = self::DIRECTIONAL) return $this->_dfs($fromNode, $toNode, $this->getRelations($mode)); } + /** + * Perform a topological sort on the graph. This algorithm is not possible if the graph contains a cycle. + * + * @return array + * @throws \Exception + */ + public function topoSort() + { + $sortedElements = []; + $nodeSet = $this->findStartNodes(); + + while (!empty($nodeSet)) { + $cur = array_pop($nodeSet); + $sortedElements[] = $cur; + if (!isset($this->_from[$cur])) { + continue; + } + foreach ($this->_from[$cur] as $parent) { + $this->removeRelation($cur, $parent); + if (empty($this->_to[$parent])) { + $nodeSet[] = $parent; + } + } + } + + if ($this->hasEdges()) { + $cycle = $this->findCycle(); + $message = "Dependency cycle detected: " . implode(" -> ", $cycle); + throw new \Exception($message); + } + + return array_reverse($sortedElements); + } + + /** + * Determines whether the graph has edges. + * + * Returns true if the graph has edges. Returns false otherwise. + * + * @return bool + */ + public function hasEdges() + { + foreach ($this->_from as $from) { + if (count($from) > 0) { + return true; + } + } + + foreach ($this->_to as $to) { + if (count($to) > 0) { + return true; + } + } + + return false; + } + + /** + * Finds all graph nodes with no incoming edge + * + * @return array + */ + public function findStartNodes() + { + $incomingEdgeNodes = array_keys($this->_to); + return array_diff($this->_nodes, $incomingEdgeNodes); + } + /** * Recursive sub-routine of dfs() * diff --git a/lib/internal/Magento/Framework/Module/ModuleList/Loader.php b/lib/internal/Magento/Framework/Module/ModuleList/Loader.php index bdfb77762b41c..c05351e659346 100644 --- a/lib/internal/Magento/Framework/Module/ModuleList/Loader.php +++ b/lib/internal/Magento/Framework/Module/ModuleList/Loader.php @@ -126,62 +126,30 @@ private function getModuleConfigs() * * @param array $origList * @return array - * @SuppressWarnings(PHPMD.UnusedLocalVariable) */ private function sortBySequence($origList) { - ksort($origList); - $expanded = []; - foreach ($origList as $moduleName => $value) { - $expanded[] = [ - 'name' => $moduleName, - 'sequence' => $this->expandSequence($origList, $moduleName), - ]; - } + $nodes = []; + $relations = []; - // Use "bubble sorting" because usort does not check each pair of elements and in this case it is important - $total = count($expanded); - for ($i = 0; $i < $total - 1; $i++) { - for ($j = $i; $j < $total; $j++) { - if (in_array($expanded[$j]['name'], $expanded[$i]['sequence'])) { - $temp = $expanded[$i]; - $expanded[$i] = $expanded[$j]; - $expanded[$j] = $temp; - } + // collect graph nodes and edges + foreach ($origList as $module) { + $nodes[] = $module['name']; + foreach ($module['sequence'] as $dependency) { + $relations[] = [$module['name'], $dependency]; } } + // construct a graph and perform topological sort on it + $moduleGraph = new \Magento\Framework\Data\Graph($nodes, $relations); + $sortedResult = $moduleGraph->topoSort(); + + // re-attach version information to the linear ordering $result = []; - foreach ($expanded as $pair) { - $result[$pair['name']] = $origList[$pair['name']]; + foreach ($sortedResult as $item) { + $result[$item] = $origList[$item]; } return $result; } - - /** - * Accumulate information about all transitive "sequence" references - * - * @param array $list - * @param string $name - * @param array $accumulated - * @return array - * @throws \Exception - */ - private function expandSequence($list, $name, $accumulated = []) - { - $accumulated[] = $name; - $result = $list[$name]['sequence']; - foreach ($result as $relatedName) { - if (in_array($relatedName, $accumulated)) { - throw new \Exception("Circular sequence reference from '{$name}' to '{$relatedName}'."); - } - if (!isset($list[$relatedName])) { - continue; - } - $relatedResult = $this->expandSequence($list, $relatedName, $accumulated); - $result = array_unique(array_merge($result, $relatedResult)); - } - return $result; - } } diff --git a/lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php b/lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php index fe613450fd485..34bb23dccdb32 100644 --- a/lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php +++ b/lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php @@ -143,7 +143,7 @@ public function testLoadExclude() /** * @expectedException \Exception - * @expectedExceptionMessage Circular sequence reference from 'b' to 'a' + * @expectedExceptionMessage Dependency cycle detected: a -> b -> a */ public function testLoadCircular() {