diff --git a/setup/modulediscovery.class.inc.php b/setup/modulediscovery.class.inc.php index 3f97421b9..fc20c954e 100644 --- a/setup/modulediscovery.class.inc.php +++ b/setup/modulediscovery.class.inc.php @@ -68,6 +68,9 @@ HTML; } } +/** + * Class that handles a module dependency + */ class ModuleDependency { private array $aPotentialPrerequisites; private array $aParamsPerModuleId; @@ -108,11 +111,22 @@ class ModuleDependency { } } - public function GetPotentialPrerequisites() : array + /** + * Return module names potentially required by current dependency + * @return array + */ + public function GetPotentialPrerequisiteModuleNames() : array { return array_keys($this->aPotentialPrerequisites); } + /** + * Check if dependency is resolved with current list of module versions + * @param array $aModuleVersions: versions by module names dict + * @param array $aSelectedModules: modules names dict + * + * @return bool + */ public function IsDependencyResolved(array $aModuleVersions, array $aSelectedModules) : bool { if ($this->bAlwaysUnresolved){ @@ -171,6 +185,9 @@ class ModuleDependency { } } +/** + * Class that handles a modules and all its dependencies + */ class Module { private string $sModuleId; private string $sModuleName; @@ -189,22 +206,36 @@ class Module { } } + /** + * @return string + */ public function GetModuleName() { return $this->sModuleName; } + /** + * @return string + */ public function GetVersion() { return $this->sVersion; } + /** + * @return string + */ public function GetModuleId() { return $this->sModuleId; } - public function SetDependencies(array $aAllDependencies) + /** + * @param array $aAllDependencies: list of dependencies (string) + * + * @return void + */ + public function SetDependencies(array $aAllDependencies): void { $this->aAllDependencies = $aAllDependencies; $this->aOngoingDependencies = []; @@ -214,6 +245,13 @@ class Module { } } + /** + * Check if module dependencies are resolved with current list of module versions + * @param array $aModuleVersions : versions by module names dict + * @param array $aSelectedModules : modules names dict + * + * @return bool + */ public function IsModuleResolved(array $aModuleVersions, array $aSelectedModules) : bool { $aNextDependencies=[]; @@ -238,12 +276,15 @@ class Module { return false; } + /** + * @return array: list of unique module names + */ public function GetUnresolvedDependencyModuleNames(): array { $aRes=[]; foreach($this->aOngoingDependencies as $sDepId => $oModuleDependency) { /** @var ModuleDependency $oModuleDependency */ - $aRes = array_merge($aRes, $oModuleDependency->GetPotentialPrerequisites()); + $aRes = array_merge($aRes, $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } return array_unique($aRes); @@ -403,6 +444,18 @@ class ModuleDiscovery return self::OrderModulesByDependencies(self::$m_aModules, $bAbortOnMissingDependency, $aModulesToLoad); } + /** + * This method is key as it sorts modules by their dependencies (topological sort). + * Modules with less dependencies are first. + * When module A depends from module B with same amount of dependencies, moduleB is first. + * This order can deal with + * - cyclic dependencies + * - further versions of same module (name) + * + * @param array $aUnresolvedDependencyModules: dict of Module objects by moduleId key + * + * @return void + */ public static function SortModulesByCountOfDepencenciesDescending(array &$aUnresolvedDependencyModules) : void { $aCountDepsByModuleId=[]; @@ -414,17 +467,18 @@ class ModuleDiscovery } foreach ($aUnresolvedDependencyModules as $sModuleId => $oModule) { - $iDepsCount = 0; + $iInDegreeCounter = 0; /** @var Module $oModule */ $aUnresolvedDependencyModuleNames = $oModule->GetUnresolvedDependencyModuleNames(); foreach ($aUnresolvedDependencyModuleNames as $sModuleName) { if (array_key_exists($sModuleName, $aDependsOnModuleName)) { $aDependsOnModuleName[$sModuleName][] = $sModuleId; - $iDepsCount++; + $iInDegreeCounter++; } } - $iDepsCountIncludingOutsideModules = count($oModule->GetUnresolvedDependencyModuleNames()); - $aCountDepsByModuleId[$sModuleId] = [$iDepsCount, $iDepsCountIncludingOutsideModules]; + //include all modules + $iInDegreeCounterIncludingOutsideModules = count($oModule->GetUnresolvedDependencyModuleNames()); + $aCountDepsByModuleId[$sModuleId] = [$iInDegreeCounter, $iInDegreeCounterIncludingOutsideModules]; } $aRes=[]; @@ -432,20 +486,21 @@ class ModuleDiscovery asort($aCountDepsByModuleId); uasort($aCountDepsByModuleId, function (array $aDeps1, array $aDeps2){ - //compare only + //compare $iInDegreeCounter $res = $aDeps1[0] - $aDeps2[0]; if ($res != 0){ return $res; } + //compare $iInDegreeCounterIncludingOutsideModules return $aDeps1[1] - $aDeps2[1]; }); $bOneLoopAtLeast=false; - foreach ($aCountDepsByModuleId as $sModuleId => $iDepsCount){ + foreach ($aCountDepsByModuleId as $sModuleId => $iInDegreeCounter){ $oModule=$aUnresolvedDependencyModules[$sModuleId]; - if ($bOneLoopAtLeast && $iDepsCount>0){ + if ($bOneLoopAtLeast && $iInDegreeCounter>0){ break; } @@ -461,8 +516,9 @@ class ModuleDiscovery continue; } $aDepCount = $aCountDepsByModuleId[$sModuleId2]; - $iDepsCount = $aDepCount[0] - 1; - $aCountDepsByModuleId[$sModuleId2] = [ $iDepsCount, $aDepCount[1]]; + $iInDegreeCounter = $aDepCount[0] - 1; + $iInDegreeCounterIncludingOutsideModules = $aDepCount[1]; + $aCountDepsByModuleId[$sModuleId2] = [$iInDegreeCounter, $iInDegreeCounterIncludingOutsideModules]; } unset($aDependsOnModuleName[$oModule->GetModuleName()]); @@ -480,6 +536,7 @@ class ModuleDiscovery * @param array $aModules The list of modules to process: 'id' => $aModuleInfo * @param bool $bAbortOnMissingDependency ... * @param array $aModulesToLoad List of modules to search for, defaults to all if omitted + * @param int $iLoopCount: used to count loop count for testing purpose (see if algo is optimized) * @return array * @throws \MissingDependencyException */ diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleDependencyTest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleDependencyTest.php index 13d668b71..02118c449 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModuleDependencyTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleDependencyTest.php @@ -3,6 +3,7 @@ namespace Combodo\iTop\Test\UnitTest\Setup; use Combodo\iTop\Test\UnitTest\ItopTestCase; +use ModuleDependency; class ModuleDependencyTest extends ItopTestCase { @@ -13,16 +14,16 @@ class ModuleDependencyTest extends ItopTestCase public function testModuleDependencyInit_Invalid() { - $oModuleDependency = new \ModuleDependency('||'); + $oModuleDependency = new ModuleDependency('||'); $this->assertEquals(true, $this->GetNonPublicProperty($oModuleDependency, 'bAlwaysUnresolved')); } public function testModuleDependencyInit() { - $oModuleDependency = new \ModuleDependency('itop-config-mgmt/2.4.0'); + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.0'); $this->assertEquals(['itop-config-mgmt/2.4.0' => [ 'itop-config-mgmt', '>=', '2.4.0']], $this->GetNonPublicProperty($oModuleDependency, 'aParamsPerModuleId')); $this->assertEquals(false, $this->GetNonPublicProperty($oModuleDependency, 'bAlwaysUnresolved')); - $this->assertEquals(['itop-config-mgmt'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-config-mgmt'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } public static function WithOperatorProvider() @@ -40,10 +41,10 @@ class ModuleDependencyTest extends ItopTestCase public function testModuleDependencyInit_WithOperator($sOperator) { $sDepId = "itop-config-mgmt/{$sOperator}2.4.0"; - $oModuleDependency = new \ModuleDependency($sDepId); + $oModuleDependency = new ModuleDependency($sDepId); $this->assertEquals([$sDepId => [ 'itop-config-mgmt', $sOperator, '2.4.0']], $this->GetNonPublicProperty($oModuleDependency, 'aParamsPerModuleId')); $this->assertEquals(false, $this->GetNonPublicProperty($oModuleDependency, 'bAlwaysUnresolved')); - $this->assertEquals(['itop-config-mgmt'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-config-mgmt'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } public static function WithOperatorOperand() @@ -62,77 +63,113 @@ class ModuleDependencyTest extends ItopTestCase public function testModuleDependencyInit_WithOperand($sOperand, $sDepId) { $sDepId = "itop-structure/3.0.0 $sOperand itop-portal/<3.2.1"; - $oModuleDependency = new \ModuleDependency($sDepId); + $oModuleDependency = new ModuleDependency($sDepId); $this->assertEquals(['itop-structure/3.0.0' => [ 'itop-structure', ">=", '3.0.0'], 'itop-portal/<3.2.1' => [ 'itop-portal', "<", '3.2.1']], $this->GetNonPublicProperty($oModuleDependency, 'aParamsPerModuleId')); $this->assertEquals(false, $this->GetNonPublicProperty($oModuleDependency, 'bAlwaysUnresolved')); - $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } public function testModuleIsDependencyResolved_SimpleCase_UnresolvedDueToMissingModule() { - $oModuleDependency = new \ModuleDependency('itop-config-mgmt/2.4.0'); + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.0'); $this->assertEquals(false, $oModuleDependency->IsDependencyResolved([], ['itop-config-mgmt' => true])); } public function testModuleIsDependencyResolved_SimpleCase_UnresolvedDueToWrongModuleVersion() { - $oModuleDependency = new \ModuleDependency('itop-config-mgmt/2.4.0'); + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.0'); $this->assertEquals(false, $oModuleDependency->IsDependencyResolved(['itop-config-mgmt' => '1.2.3'], ['itop-config-mgmt' => true])); } + public function testModuleIsDependencyResolved_SimpleCase_ResolvedDue_MinorVersion() + { + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.1'); + $this->assertEquals(true, $oModuleDependency->IsDependencyResolved(['itop-config-mgmt' => '2.4.1-1'], ['itop-config-mgmt' => true])); + } + + public function testModuleIsDependencyResolved_SimpleCase_ResolvedDue_MinorVersion2() + { + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.1-1'); + $this->assertEquals(true, $oModuleDependency->IsDependencyResolved(['itop-config-mgmt' => '2.4.1-2'], ['itop-config-mgmt' => true])); + } + + public function testModuleIsDependencyResolved_SimpleCase_ResolvedDue_MinorVersion3() + { + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.1-1'); + $this->assertEquals(true, $oModuleDependency->IsDependencyResolved(['itop-config-mgmt' => '2.4.2'], ['itop-config-mgmt' => true])); + } + + public function testModuleIsDependencyResolved_SimpleCase_UnresolvedDueToWrongModuleVersion_MinorVersion() + { + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.1'); + $this->assertEquals(false, $oModuleDependency->IsDependencyResolved(['itop-config-mgmt' => '2.4.0-1'], ['itop-config-mgmt' => true])); + } + + public function testModuleIsDependencyResolved_SimpleCase_UnresolvedDueToWrongModuleVersion_MinorVersion2() + { + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.1-1'); + $this->assertEquals(false, $oModuleDependency->IsDependencyResolved(['itop-config-mgmt' => '2.4.1'], ['itop-config-mgmt' => true])); + } + + public function testModuleIsDependencyResolved_SimpleCase_UnresolvedDueToWrongModuleVersion_MinorVersion3() + { + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.1-1'); + $this->assertEquals(false, $oModuleDependency->IsDependencyResolved(['itop-config-mgmt' => '2.4.1-0'], ['itop-config-mgmt' => true])); + } + public function testModuleIsDependencyResolved_SimpleCase_Resolved() { - $oModuleDependency = new \ModuleDependency('itop-config-mgmt/2.4.0'); - $this->assertEquals(['itop-config-mgmt'], $oModuleDependency->GetPotentialPrerequisites()); + $oModuleDependency = new ModuleDependency('itop-config-mgmt/2.4.0'); + $this->assertEquals(['itop-config-mgmt'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); $this->assertEquals(true, $oModuleDependency->IsDependencyResolved(['itop-config-mgmt' => '2.4.1'], ['itop-config-mgmt' => true])); - $this->assertEquals([], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals([], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } public function testIsDependencyResolved_AndOperand_UnresolvedDueToMissingModule() { $sDepId = "itop-structure/3.0.0 && itop-portal/3.2.1"; - $oModuleDependency = new \ModuleDependency($sDepId); + $oModuleDependency = new ModuleDependency($sDepId); $this->assertEquals(['itop-structure/3.0.0' => [ 'itop-structure', ">=", '3.0.0'], 'itop-portal/3.2.1' => [ 'itop-portal', ">=", '3.2.1']], $this->GetNonPublicProperty($oModuleDependency, 'aParamsPerModuleId')); $this->assertEquals(false, $this->GetNonPublicProperty($oModuleDependency, 'bAlwaysUnresolved')); - $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); $this->assertEquals(false, $oModuleDependency->IsDependencyResolved(['itop-structure' => '3.0.0'], ['itop-structure' => true, 'itop-portal' => true])); - $this->assertEquals(['itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } public function testIsDependencyResolved_AndOperand_UnresolvedDueToWrongModuleVersion() { $sDepId = "itop-structure/3.0.0 && itop-portal/3.2.1"; - $oModuleDependency = new \ModuleDependency($sDepId); + $oModuleDependency = new ModuleDependency($sDepId); $this->assertEquals(['itop-structure/3.0.0' => [ 'itop-structure', ">=", '3.0.0'], 'itop-portal/3.2.1' => [ 'itop-portal', ">=", '3.2.1']], $this->GetNonPublicProperty($oModuleDependency, 'aParamsPerModuleId')); $this->assertEquals(false, $this->GetNonPublicProperty($oModuleDependency, 'bAlwaysUnresolved')); - $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); $this->assertEquals(false, $oModuleDependency->IsDependencyResolved(['itop-structure' => '3.0.0', 'itop-portal' => '1.0.0'], ['itop-structure' => true, 'itop-portal' => true])); - $this->assertEquals(['itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } public function testIsDependencyResolved_AndOperand_Resolved() { $sDepId = "itop-structure/3.0.0 && itop-portal/3.2.1"; - $oModuleDependency = new \ModuleDependency($sDepId); + $oModuleDependency = new ModuleDependency($sDepId); $this->assertEquals(['itop-structure/3.0.0' => [ 'itop-structure', ">=", '3.0.0'], 'itop-portal/3.2.1' => [ 'itop-portal', ">=", '3.2.1']], $this->GetNonPublicProperty($oModuleDependency, 'aParamsPerModuleId')); $this->assertEquals(false, $this->GetNonPublicProperty($oModuleDependency, 'bAlwaysUnresolved')); - $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); $this->assertEquals(false, $oModuleDependency->IsDependencyResolved(['itop-structure' => '3.0.0'], ['itop-structure' => true])); - $this->assertEquals(['itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } public function testIsDependencyResolved_OrOperand_ResolvedDueToMissingModule() { $sDepId = "itop-structure/3.0.0 || itop-portal/3.2.1"; - $oModuleDependency = new \ModuleDependency($sDepId); + $oModuleDependency = new ModuleDependency($sDepId); $this->assertEquals(['itop-structure/3.0.0' => [ 'itop-structure', ">=", '3.0.0'], 'itop-portal/3.2.1' => [ 'itop-portal', ">=", '3.2.1']], $this->GetNonPublicProperty($oModuleDependency, 'aParamsPerModuleId')); $this->assertEquals(false, $this->GetNonPublicProperty($oModuleDependency, 'bAlwaysUnresolved')); - $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-structure', 'itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); $this->assertEquals(true, $oModuleDependency->IsDependencyResolved(['itop-structure' => '3.0.0'], ['itop-structure' => true])); - $this->assertEquals(['itop-portal'], $oModuleDependency->GetPotentialPrerequisites()); + $this->assertEquals(['itop-portal'], $oModuleDependency->GetPotentialPrerequisiteModuleNames()); } } \ No newline at end of file diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php index 80ac469e2..0916dad4b 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php @@ -164,6 +164,41 @@ MSG; $this->assertEquals(1, $iLoopCount); } + public function testOrderModulesByDependencies_ResolveOk_ModulesToLoadProvided() + { + $aModules=[ + "id1/1" => [ + 'dependencies' => [ 'id2/2'], + 'label' => 'label1', + ], + "id2/2" => [ + 'dependencies' => ['id3/3 || id3-itil/3'], + 'label' => 'label2', + ], + "id3/3" => [ + 'dependencies' => [], + 'label' => 'label3', + ], + "id3-itil/3" => [ + 'dependencies' => [], + 'label' => 'label3-itil', + ], + ]; + + foreach(["id3", "id3-itil"] as $sLastModuleNameToLoad) { + $iLoopCount = 0; + $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, ['id1', 'id2', $sLastModuleNameToLoad], $iLoopCount); + + $aExpected = [ + "$sLastModuleNameToLoad/3", + "id2/2", + "id1/1", + ]; + $this->assertEquals($aExpected, array_keys($aResult)); + $this->assertEquals(1, $iLoopCount); + } + } + public function testOrderModulesByDependencies_RealExample(){ $aModules = json_decode(file_get_contents(__DIR__ . '/ressources/module_deps.json'), true); $iLoopCount=0; @@ -216,6 +251,24 @@ MSG; array_keys($aUnresolvedDependencyModules)); } + public function testSortModulesByCountOfDepencenciesDescending_FurtherVersionsOfSameModule(){ + $aUnresolvedDependencyModules = []; + $this->AddModule($aUnresolvedDependencyModules, 'moduleA/1', []); + $this->AddModule($aUnresolvedDependencyModules, 'moduleA/2', ['moduleC/1']); + $this->AddModule($aUnresolvedDependencyModules, 'moduleB/1', ['moduleA/1']); + $this->AddModule($aUnresolvedDependencyModules, 'moduleC/1', []); + + ModuleDiscovery::SortModulesByCountOfDepencenciesDescending($aUnresolvedDependencyModules); + $this->assertEquals( + [ + 'moduleA/1', + 'moduleC/1', + 'moduleB/1', + 'moduleA/2', + ], + array_keys($aUnresolvedDependencyModules)); + } + private function AddModule(array &$aUnresolvedDependencyModules, string $sModuleId, array $aDeps){ $oModule = new \Module($sModuleId); $oModule->SetDependencies($aDeps);