diff --git a/setup/module/ItopCoreModuleDependency.class.inc.php b/setup/module/ItopCoreModuleDependency.class.inc.php index 079b5aa6c..abcf1181a 100644 --- a/setup/module/ItopCoreModuleDependency.class.inc.php +++ b/setup/module/ItopCoreModuleDependency.class.inc.php @@ -1,9 +1,13 @@ sDepString); - $bOk = @eval('$bResult = '.$sBooleanExpr.'; return true;'); - if ($bOk == false) - { - SetupLog::Warning("Eval of '$sBooleanExpr' returned false"); + try{ + $bResult = self::GetPhpExpressionEvaluator()->ParseAndEvaluateBooleanExpression($sBooleanExpr); + } catch(ModuleFileReaderException $e){ + //logged already echo "Failed to parse the boolean Expression = '$sBooleanExpr'
"; } return $bResult; diff --git a/setup/module/iTopCoreModule.class.inc.php b/setup/module/iTopCoreModule.class.inc.php index 1557670bf..60e23d70c 100644 --- a/setup/module/iTopCoreModule.class.inc.php +++ b/setup/module/iTopCoreModule.class.inc.php @@ -85,12 +85,7 @@ class iTopCoreModule { $this->aOngoingDependencies=$aNextDependencies; - if ($bDependenciesSolved) - { - return true; - } - - return false; + return $bDependenciesSolved; } /** diff --git a/setup/module/iTopCoreModuleDependencySort.class.inc.php b/setup/module/iTopCoreModuleDependencySort.class.inc.php index 2da91bf13..2ac7cce1e 100644 --- a/setup/module/iTopCoreModuleDependencySort.class.inc.php +++ b/setup/module/iTopCoreModuleDependencySort.class.inc.php @@ -35,7 +35,7 @@ class iTopCoreModuleDependencySort { * * @return void */ - public function SortModulesByCountOfDepencenciesDescending(array &$aUnresolvedDependencyModules) : void + public static function SortModulesByCountOfDepencenciesDescending(array &$aUnresolvedDependencyModules) : void { $aCountDepsByModuleId=[]; $aDependsOnModuleName=[]; @@ -125,7 +125,7 @@ class iTopCoreModuleDependencySort { * @return array * @throws \MissingDependencyException */ - public function OrderModulesByDependencies($aModules, $bAbortOnMissingDependency = false, $aModulesToLoad = null, ?int &$iLoopCount=0) + public static function OrderModulesByDependencies($aModules, $bAbortOnMissingDependency = false, $aModulesToLoad = null, ?int &$iLoopCount=0) { $iLoopCount=0; diff --git a/setup/modulediscovery.class.inc.php b/setup/modulediscovery.class.inc.php index 0757c9d5a..66318b862 100755 --- a/setup/modulediscovery.class.inc.php +++ b/setup/modulediscovery.class.inc.php @@ -97,6 +97,7 @@ class ModuleDiscovery protected static $m_sModulePath = null; private static PhpExpressionEvaluator $oPhpExpressionEvaluator; + private static mixed $bNewFeedback = false; protected static function SetModulePath($sModulePath) { @@ -186,7 +187,7 @@ class ModuleDiscovery $sDir = dirname($sFilePath); $aDirs = [ $sDir => self::$m_sModulePath, - $sDir.'/dictionaries' => self::$m_sModulePath.'/dictionaries' + $sDir.'/dictionaries' => self::$m_sModulePath.'/dictionaries', ]; foreach ($aDirs as $sRootDir => $sPath) { @@ -221,6 +222,9 @@ class ModuleDiscovery return self::OrderModulesByDependencies(self::$m_aModules, $bAbortOnMissingDependency, $aModulesToLoad); } + public static function UseNewUiFeedback($bNewFeedback){ + self::$bNewFeedback=$bNewFeedback; + } /** * Arrange an list of modules, based on their (inter) dependencies * @param array $aModules The list of modules to process: 'id' => $aModuleInfo @@ -246,7 +250,7 @@ class ModuleDiscovery ksort($aDependencies); $aOrderedModules = []; $iLoopCount = 1; - while(($iLoopCount < count($aModules)) && (count($aDependencies) > 0) ) + while(($iLoopCount < count($aModules)+1) && (count($aDependencies) > 0) ) { foreach($aDependencies as $sId => $aRemainingDeps) { @@ -268,6 +272,10 @@ class ModuleDiscovery } if ($bAbortOnMissingDependency && count($aDependencies) > 0) { + if (self::$bNewFeedback){ + iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, $bAbortOnMissingDependency, $aModulesToLoad); + } + $aModulesInfo = []; $aModuleDeps = []; foreach($aDependencies as $sId => $aDeps) diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php index 6884c2320..2c83e9aa6 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php @@ -13,7 +13,21 @@ class ModuleDiscoveryTest extends ItopDataTestCase $this->RequireOnceItopFile('setup/modulediscovery.class.inc.php'); } - public function testOrderModulesByDependencies_CheckMissingDependenciesAreCorrectlyOrderedInTheException() + public function tearDown() : void { + ModuleDiscovery::UseNewUiFeedback(false); + } + public static function OrderModulesByDependenciesProvider(){ + return [ + 'legacy computation' => [0], + 'hybrid computation: order is legacy/ error is new usr friendly one' => [1], + 'new computation' => [2], + ]; + } + + /** + * @dataProvider OrderModulesByDependenciesProvider + */ + public function testOrderModulesByDependencies_CheckMissingDependenciesAreCorrectlyOrderedInTheException(int $mode) { $aModules=[ "id1/123" => [ @@ -27,19 +41,40 @@ class ModuleDiscoveryTest extends ItopDataTestCase ]; $iLoopCount=0; try{ - ModuleDiscovery::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + if ($mode===2){ + iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + } else { + if ($mode===1){ + ModuleDiscovery::UseNewUiFeedback(true); + } + ModuleDiscovery::OrderModulesByDependencies($aModules, true, null); + } } catch(\MissingDependencyException $e){ - $sExpectedMessage = <<assertEquals($sExpectedMessage, $e->getMessage()); - $this->assertEquals(1, $iLoopCount); + if ($mode===2) { + $this->assertEquals(1, $iLoopCount); + } } } - public function testOrderModulesByDependencies_ValidateExceptionWithSomeDependenciesResolved() + /** + * @dataProvider OrderModulesByDependenciesProvider + */ + public function testOrderModulesByDependencies_ValidateExceptionWithSomeDependenciesResolved(int $mode) { $aModules=[ "id1/123" => [ @@ -57,19 +92,39 @@ MSG; ]; $iLoopCount=0; try{ - ModuleDiscovery::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + if ($mode===2){ + iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + } else { + if ($mode===1){ + ModuleDiscovery::UseNewUiFeedback(true); + } + ModuleDiscovery::OrderModulesByDependencies($aModules, true, null); + } } catch(\MissingDependencyException $e){ - $sExpectedMessage = <<assertEquals($sExpectedMessage, $e->getMessage()); - $this->assertEquals(2, $iLoopCount); + if ($mode===2) { + $this->assertEquals(2, $iLoopCount); + } } } - public function testOrderModulesByDependencies_KeepGoingEvenWithFailure_WithSomeDependenciesResolved() + /** + * @dataProvider OrderModulesByDependenciesProvider + */ + public function testOrderModulesByDependencies_KeepGoingEvenWithFailure_WithSomeDependenciesResolved(int $mode) { $aModules=[ "id1/123" => [ @@ -86,16 +141,28 @@ MSG; ], ]; $iLoopCount=0; - $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, false, null, $iLoopCount); + if ($mode===2){ + $aResult = iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, false, null, $iLoopCount); + } else { + if ($mode===1){ + ModuleDiscovery::UseNewUiFeedback(true); + } + $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, false, null); + } $aExpected = [ - 'id2/456' + 'id2/456', ]; $this->assertEquals($aExpected, array_keys($aResult)); - $this->assertEquals(2, $iLoopCount); + if ($mode===2) { + $this->assertEquals(2, $iLoopCount); + } } - public function testOrderModulesByDependencies_UnResolveWithCircularDependency() + /** + * @dataProvider OrderModulesByDependenciesProvider + */ + public function testOrderModulesByDependencies_UnResolveWithCircularDependency(int $mode) { $aModules=[ "id1/1" => [ @@ -118,21 +185,43 @@ MSG; $iLoopCount=0; try{ - ModuleDiscovery::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + if ($mode===2){ + iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + } else { + if ($mode===1){ + ModuleDiscovery::UseNewUiFeedback(true); + } + ModuleDiscovery::OrderModulesByDependencies($aModules, true, null); + } } catch(\MissingDependencyException $e){ - $sExpectedMessage = <<assertEquals($sExpectedMessage, $e->getMessage()); - $this->assertEquals(1, $iLoopCount); + if ($mode===2) { + $this->assertEquals(1, $iLoopCount); + } } } - public function testOrderModulesByDependencies_ResolveOk() + /** + * @dataProvider OrderModulesByDependenciesProvider + */ + public function testOrderModulesByDependencies_ResolveOk(int $mode) { $aModules=[ "id0/1" => [ @@ -157,7 +246,14 @@ MSG; ], ]; $iLoopCount=0; - $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + if ($mode===2){ + $aResult = iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + } else { + if ($mode===1){ + ModuleDiscovery::UseNewUiFeedback(true); + } + $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, null); + } $aExpected = [ "id4/4", @@ -167,10 +263,15 @@ MSG; "id0/1", ]; $this->assertEquals($aExpected, array_keys($aResult)); - $this->assertEquals(1, $iLoopCount); + if ($mode===2) { + $this->assertEquals(1, $iLoopCount); + } } - public function testOrderModulesByDependencies_ResolveNoDependendenciesOrderByAlphabeticalOrder() + /** + * @dataProvider OrderModulesByDependenciesProvider + */ + public function testOrderModulesByDependencies_ResolveNoDependendenciesOrderByAlphabeticalOrder(int $mode) { $aModules=[ "id2/2" => [ @@ -195,7 +296,14 @@ MSG; ], ]; $iLoopCount=0; - $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + if ($mode===2){ + $aResult = iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + } else { + if ($mode===1){ + ModuleDiscovery::UseNewUiFeedback(true); + } + $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, null); + } $aExpected = [ "id0/1", @@ -205,10 +313,15 @@ MSG; "id4/4", ]; $this->assertEquals($aExpected, array_keys($aResult)); - $this->assertEquals(1, $iLoopCount); + if ($mode===2) { + $this->assertEquals(1, $iLoopCount); + } } - public function testOrderModulesByDependencies_ResolveOk_ModulesToLoadProvided() + /** + * @dataProvider OrderModulesByDependenciesProvider + */ + public function testOrderModulesByDependencies_ResolveOk_ModulesToLoadProvided(int $mode) { $aModules=[ "id1/1" => [ @@ -231,7 +344,14 @@ MSG; foreach(["id3", "id3-itil"] as $sLastModuleNameToLoad) { $iLoopCount = 0; - $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, ['id1', 'id2', $sLastModuleNameToLoad], $iLoopCount); + if ($mode===2){ + $aResult = iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, true, ['id1', 'id2', $sLastModuleNameToLoad], $iLoopCount); + } else { + if ($mode===1){ + ModuleDiscovery::UseNewUiFeedback(true); + } + $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, ['id1', 'id2', $sLastModuleNameToLoad]); + } $aExpected = [ "$sLastModuleNameToLoad/3", @@ -239,14 +359,24 @@ MSG; "id1/1", ]; $this->assertEquals($aExpected, array_keys($aResult)); - $this->assertEquals(1, $iLoopCount); + if ($mode===2) { + $this->assertEquals(1, $iLoopCount); + } } } - public function testOrderModulesByDependencies_RealExample(){ + /*public function testOrderModulesByDependencies_RealExample(){ + $aModules = json_decode(file_get_contents(__DIR__ . '/ressources/module_deps.json'), true); + $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, null); + + $aExpected = json_decode(file_get_contents(__DIR__ . '/ressources/expected_ordered_module_ids.json'), true); + $this->assertEquals($aExpected, array_keys($aResult)); + }*/ + + public function testOrderModulesByDependenciesNewwComputation_RealExample(){ $aModules = json_decode(file_get_contents(__DIR__ . '/ressources/module_deps.json'), true); $iLoopCount=0; - $aResult = ModuleDiscovery::OrderModulesByDependencies($aModules, true, null, $iLoopCount); + $aResult = iTopCoreModuleDependencySort::OrderModulesByDependencies($aModules, true, null, $iLoopCount); $aExpected = json_decode(file_get_contents(__DIR__ . '/ressources/expected_ordered_module_ids.json'), true); $this->assertEquals($aExpected, array_keys($aResult));