add phpdoc + add more tests

2026-04-23 02:28:44 +02:00 · 2025-03-11 17:12:31 +01:00
parent 460a3e356e
commit bb16108f04
3 changed files with 183 additions and 36 deletions
--- 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
 */
--- 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());
 	}
 }
--- 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);