N°8724 - refactoring for maintenability

This commit is contained in:
odain
2025-11-27 15:47:25 +01:00
parent 03e25a226e
commit 5a2157ba21
4 changed files with 133 additions and 93 deletions

View File

@@ -3,6 +3,7 @@
namespace Combodo\iTop\Setup\ModuleDependency;
require_once(APPROOT.'/setup/runtimeenv.class.inc.php');
use Combodo\iTop\PhpParser\Evaluation\PhpExpressionEvaluator;
use ModuleFileReaderException;
use RunTimeEnvironment;
@@ -71,6 +72,7 @@ class DependencyExpression
/**
* Return module names potentially required by current dependency
*
* @return array
*/
public function GetRemainingModuleNamesToResolve(): array
@@ -85,22 +87,25 @@ class DependencyExpression
/**
* 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
*
* @param array $aResolvedModuleVersions : versions by module names dict
* @param array $aAllModuleNames : modules names dict
*
* @return void
*/
public function UpdateModuleResolutionState(array $aModuleVersions, array $aSelectedModules): void
public function UpdateModuleResolutionState(array $aResolvedModuleVersions, array $aAllModuleNames): void
{
if (!$this->bValid) {
return;
}
$aReplacements = [];
$bDelayEvaluation = false;
foreach ($this->aParamsPerModuleId as $sModuleId => list($sModuleName, $sOperator, $sExpectedVersion)) {
if (array_key_exists($sModuleName, $aModuleVersions)) {
// module is present, check the version
$sCurrentVersion = $aModuleVersions[$sModuleName];
if (array_key_exists($sModuleName, $aResolvedModuleVersions)) {
// module is resolved, check the version
$sCurrentVersion = $aResolvedModuleVersions[$sModuleName];
if (version_compare($sCurrentVersion, $sExpectedVersion, $sOperator)) {
if (array_key_exists($sModuleName, $this->aRemainingModuleNamesToResolve)) {
unset($this->aRemainingModuleNamesToResolve[$sModuleName]);
@@ -112,20 +117,24 @@ class DependencyExpression
// a function call that results in a runtime fatal error
}
} else {
// module is not present
// module is not resolved yet
if (array_key_exists($sModuleName, $aAllModuleNames)) {
//Weird piece of code that covers below usecase:
//module B dependency: 'moduleA || true'
// if moduleA not present on disk, whole expression can be evaluated and may be resolved
// if moduleA present on disk, we need to sort moduleB after moduleA. expression cannot be resolved yet
$bDelayEvaluation = true;
} else {
$aReplacements[$sModuleId] = '(false)'; // Add parentheses to protect against invalid condition causing
// a function call that results in a runtime fatal error
}
}
}
foreach ($this->aRemainingModuleNamesToResolve as $sModuleName => $c) {
if (array_key_exists($sModuleName, $aSelectedModules)) {
// This module is actually a prerequisite
if (!array_key_exists($sModuleName, $aModuleVersions)) {
if ($bDelayEvaluation) {
return;
}
}
}
$bResult = false;
$sBooleanExpr = str_replace(array_keys($aReplacements), array_values($aReplacements), $this->sDependencyExpression);

View File

@@ -93,18 +93,18 @@ 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
* @param array<string, string> $aResolvedModuleVersions : versions by module names dict
* @param array<string> $aAllModuleNames : resolved modules names
*
* @return void
*/
public function UpdateModuleResolutionState(array $aModuleVersions, array $aSelectedModules): void
public function UpdateModuleResolutionState(array $aResolvedModuleVersions, array $aAllModuleNames): void
{
$aNextDependencies = [];
foreach ($this->aRemainingDependenciesToResolve as $sDependencyExpression => $oModuleDependency) {
/** @var DependencyExpression $oModuleDependency*/
$oModuleDependency->UpdateModuleResolutionState($aModuleVersions, $aSelectedModules);
$oModuleDependency->UpdateModuleResolutionState($aResolvedModuleVersions, $aAllModuleNames);
if (!$oModuleDependency->IsResolved()) {
$aNextDependencies[$sDependencyExpression] = $oModuleDependency;
}

View File

@@ -33,8 +33,10 @@ class ModuleDependencySort
/**
* Sort a list of modules, based on their (inter) dependencies
*
* @param array $aModules The list of modules to process: 'id' => $aModuleInfo
* @param bool $bAbortOnMissingDependency ...
*
* @return array
* @throws \MissingDependencyException
*/
@@ -42,13 +44,13 @@ class ModuleDependencySort
{
// Filter modules to compute
$aUnresolvedDependencyModules = [];
$aModuleNames = [];
$aAllModuleNames = [];
foreach ($aModules as $sModuleId => $aModule) {
$oModule = new Module($sModuleId);
$sModuleName = $oModule->GetModuleName();
$oModule->SetDependencies($aModule['dependencies']);
$aUnresolvedDependencyModules[$sModuleId] = $oModule;
$aModuleNames[$sModuleName] = true;
$aAllModuleNames[$sModuleName] = true;
}
// Make sure order is deterministic (alphabtical order)
@@ -56,7 +58,7 @@ class ModuleDependencySort
//Attempt to resolve module dependencies
$aOrderedModules = [];
$aModuleVersions = [];
$aResolvedModuleVersions = [];
$iPreviousUnresolvedCount = -1;
//loop until no dependency is resolved
while ($iPreviousUnresolvedCount !== count($aUnresolvedDependencyModules)) {
@@ -67,10 +69,10 @@ class ModuleDependencySort
foreach ($aUnresolvedDependencyModules as $sModuleId => $oModule) {
/** @var Module $oModule */
$oModule->UpdateModuleResolutionState($aModuleVersions, $aModuleNames);
$oModule->UpdateModuleResolutionState($aResolvedModuleVersions, $aAllModuleNames);
if ($oModule->IsResolved()) {
$aOrderedModules[] = $sModuleId;
$aModuleVersions[$oModule->GetModuleName()] = $oModule->GetVersion();
$aResolvedModuleVersions[$oModule->GetModuleName()] = $oModule->GetVersion();
unset($aUnresolvedDependencyModules[$sModuleId]);
}
}
@@ -100,6 +102,7 @@ class ModuleDependencySort
foreach ($aOrderedModules as $sId) {
$aResult[$sId] = $aModules[$sId];
}
return $aResult;
}

View File

@@ -59,9 +59,10 @@ class DependencyExpressionTest extends ItopTestCase
$this->assertEquals(['itop-config-mgmt'], $oModuleDependency->GetRemainingModuleNamesToResolve());
}
public static function WithOperatorOperandProvider()
public static function WithVariousOperatorProvider()
{
$aInternalStructure = ['itop-structure/3.0.0' => ['itop-structure', ">=", '3.0.0'], 'itop-portal/<3.2.1' => ['itop-portal', "<", '3.2.1']];
return [
'&&' => [
'sDepId' => 'itop-structure/3.0.0 && itop-portal/<3.2.1',
@@ -83,7 +84,7 @@ class DependencyExpressionTest extends ItopTestCase
}
/**
* @dataProvider WithOperatorOperandProvider
* @dataProvider WithVariousOperatorProvider
*/
public function testModuleDependencyInit_WithOperand($sDepId, $sExpected)
{
@@ -99,27 +100,27 @@ class DependencyExpressionTest extends ItopTestCase
return [
'unresolved with major version' => [
'expr' => 'itop-config-mgmt/2.4.0',
'module_versions' => ['itop-config-mgmt' => '1.2.3'],
'resolved_module_versions' => ['itop-config-mgmt' => '1.2.3'],
'expected_is_resolved' => false,
],
'unresolved with minor version' => [
'expr' => 'itop-config-mgmt/2.4.1',
'module_versions' => ['itop-config-mgmt' => '2.4.0-1'],
'resolved_module_versions' => ['itop-config-mgmt' => '2.4.0-1'],
'expected_is_resolved' => false,
],
'resolution OK with major version' => [
'expr' => 'itop-config-mgmt/2.4.0',
'module_versions' => ['itop-config-mgmt' => '2.4.2'],
'resolved_module_versions' => ['itop-config-mgmt' => '2.4.2'],
'expected_is_resolved' => true,
],
'resolution OK with minor version' => [
'expr' => 'itop-config-mgmt/2.4.0',
'module_versions' => ['itop-config-mgmt' => '2.4.0-1'],
'resolved_module_versions' => ['itop-config-mgmt' => '2.4.0-1'],
'expected_is_resolved' => true,
],
'unproper use of api' => [
'expr' => 'itop-config-mgmt/2.4.0',
'module_versions' => [],
'resolved_module_versions' => [],
'expected_is_resolved' => false,
],
];
@@ -140,37 +141,64 @@ class DependencyExpressionTest extends ItopTestCase
public static function ComplexDependencyExpressionIsResolvedProvider()
{
$aAllModules = ['itop-structure' => true, 'itop-portal' => true];
return [
'and + unresolved due to missing itop-portal' => [
'expr' => 'itop-structure/3.0.0 && itop-portal/3.2.1',
'module_versions' => ['itop-structure' => '3.0.0'],
'resolved_module_versions' => ['itop-structure' => '3.0.0'],
'all_modules' => $aAllModules,
'expected_is_resolved' => false,
'remaining_module_names' => ['itop-portal'],
],
'and + unresolved due to unsifficient itop-portal version' => [
'expr' => 'itop-structure/3.0.0 && itop-portal/3.2.1',
'module_versions' => ['itop-structure' => '3.0.0', 'itop-portal' => '1.0.0'],
'resolved_module_versions' => ['itop-structure' => '3.0.0', 'itop-portal' => '1.0.0'],
'all_modules' => $aAllModules,
'expected_is_resolved' => false,
'remaining_module_names' => ['itop-portal'],
],
'and + resolved' => [
'expr' => 'itop-structure/3.0.0 && itop-portal/3.2.1',
'module_versions' => ['itop-structure' => '3.0.0', 'itop-portal' => '3.3.3'],
'resolved_module_versions' => ['itop-structure' => '3.0.0', 'itop-portal' => '3.3.3'],
'all_modules' => $aAllModules,
'expected_is_resolved' => true,
'remaining_module_names' => [],
],
'or + resolved' => [
'or||true (step1) + dependency expression evaluation is delayed for sorting purpose' => [
'expr' => 'itop-structure/3.0.0||true',
'resolved_module_versions' => [],
'all_modules' => $aAllModules,
'expected_is_resolved' => false,
'remaining_module_names' => ['itop-structure'],
],
'or||true (step2) + expression is evaluated because itop-structure has been resolved' => [
'expr' => 'itop-structure/3.0.0||true',
'resolved_module_versions' => ['itop-structure' => '3.0.0'],
'all_modules' => $aAllModules,
'expected_is_resolved' => true,
'remaining_module_names' => [],
],
'or||true + resolved DIRECTLY as itop-structure is not on disk (all_modules)' => [
'expr' => 'itop-structure/3.0.0||true',
'resolved_module_versions' => [],
'all_modules' => [],
'expected_is_resolved' => true,
'remaining_module_names' => ['itop-structure'],
],
'or + unresolved because dependency trick used to sort as well' => [
'expr' => 'itop-structure/3.0.0 || itop-portal/3.2.1',
'module_versions' => ['itop-structure' => '3.0.0'],
'resolved_module_versions' => ['itop-structure' => '3.0.0'],
'all_modules' => $aAllModules,
'expected_is_resolved' => false,
'remaining_module_names' => ['itop-portal'],
],
'or + resolved with less prerequisites' => [
'expr' => 'itop-structure/3.0.0 || itop-portal/3.2.1',
'module_versions' => ['itop-structure' => '3.0.0'],
'1 can be used as a boolean' => [
'expr' => 'true||1',
'resolved_module_versions' => [],
'all_modules' => [],
'expected_is_resolved' => true,
'remaining_module_names' => ['itop-portal'],
'prerequisites' => ['itop-structure' => true],
'remaining_module_names' => [],
],
];
}
@@ -178,11 +206,11 @@ class DependencyExpressionTest extends ItopTestCase
/**
* @dataProvider ComplexDependencyExpressionIsResolvedProvider
*/
public function testComplexDependencyExpressionIsResolved($sExpression, $aModuleVersions, $bExpectedResolved, $aRemainingModuleNames, $aPrerequisites = ['itop-structure' => true, 'itop-portal' => true])
public function testComplexDependencyExpressionIsResolved($sExpression, $aModuleVersions, $aAllModules, $bExpectedResolved, $aRemainingModuleNames)
{
$oModuleDependency = new DependencyExpression($sExpression);
$oModuleDependency->UpdateModuleResolutionState($aModuleVersions, $aPrerequisites);
$oModuleDependency->UpdateModuleResolutionState($aModuleVersions, $aAllModules);
$this->assertEquals($aRemainingModuleNames, $oModuleDependency->GetRemainingModuleNamesToResolve());
$this->assertEquals($bExpectedResolved, $oModuleDependency->IsResolved());
}