Skip to content

Commit 5a2157b

Browse files
odain-cbdodain-cbd
committed
N°8724 - refactoring for maintenability
1 parent 03e25a2 commit 5a2157b

File tree

4 files changed

+137
-97
lines changed

4 files changed

+137
-97
lines changed

‎setup/moduledependency/dependencyexpression.class.inc.php‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Combodo\iTop\Setup\ModuleDependency;
44

55
require_once(APPROOT.'/setup/runtimeenv.class.inc.php');
6+
67
use Combodo\iTop\PhpParser\Evaluation\PhpExpressionEvaluator;
78
use ModuleFileReaderException;
89
use RunTimeEnvironment;
@@ -38,7 +39,7 @@ public function __construct(string $sDependencyExpression)
3839
if (preg_match_all('/([^\(\)&| ]+)/', $sDependencyExpression, $aMatches)) {
3940
foreach ($aMatches as $aMatch) {
4041
foreach ($aMatch as $sModuleId) {
41-
if (! array_key_exists($sModuleId, $this->aParamsPerModuleId)) {
42+
if (!array_key_exists($sModuleId, $this->aParamsPerModuleId)) {
4243
// $sModuleId in the dependency string is made of a <name>/<optional_operator><version>
4344
// where the operator is < <= = > >= (by default >=)
4445
$aModuleMatches = [];
@@ -71,6 +72,7 @@ private static function GetPhpExpressionEvaluator(): PhpExpressionEvaluator
7172

7273
/**
7374
* Return module names potentially required by current dependency
75+
*
7476
* @return array
7577
*/
7678
public function GetRemainingModuleNamesToResolve(): array
@@ -85,22 +87,25 @@ public function IsResolved(): bool
8587

8688
/**
8789
* Check if dependency is resolved with current list of module versions
88-
* @param array $aModuleVersions: versions by module names dict
89-
* @param array $aSelectedModules: modules names dict
90+
*
91+
* @param array $aResolvedModuleVersions : versions by module names dict
92+
* @param array $aAllModuleNames : modules names dict
9093
*
9194
* @return void
9295
*/
93-
public function UpdateModuleResolutionState(array $aModuleVersions, array $aSelectedModules): void
96+
public function UpdateModuleResolutionState(array $aResolvedModuleVersions, array $aAllModuleNames): void
9497
{
9598
if (!$this->bValid) {
9699
return;
97100
}
98101

99102
$aReplacements = [];
103+
104+
$bDelayEvaluation = false;
100105
foreach ($this->aParamsPerModuleId as $sModuleId => list($sModuleName, $sOperator, $sExpectedVersion)) {
101-
if (array_key_exists($sModuleName, $aModuleVersions)) {
102-
// module is present, check the version
103-
$sCurrentVersion = $aModuleVersions[$sModuleName];
106+
if (array_key_exists($sModuleName, $aResolvedModuleVersions)) {
107+
// module is resolved, check the version
108+
$sCurrentVersion = $aResolvedModuleVersions[$sModuleName];
104109
if (version_compare($sCurrentVersion, $sExpectedVersion, $sOperator)) {
105110
if (array_key_exists($sModuleName, $this->aRemainingModuleNamesToResolve)) {
106111
unset($this->aRemainingModuleNamesToResolve[$sModuleName]);
@@ -112,19 +117,23 @@ public function UpdateModuleResolutionState(array $aModuleVersions, array $aSele
112117
// a function call that results in a runtime fatal error
113118
}
114119
} else {
115-
// module is not present
116-
$aReplacements[$sModuleId] = '(false)'; // Add parentheses to protect against invalid condition causing
117-
// a function call that results in a runtime fatal error
120+
// module is not resolved yet
121+
122+
if (array_key_exists($sModuleName, $aAllModuleNames)) {
123+
//Weird piece of code that covers below usecase:
124+
//module B dependency: 'moduleA || true'
125+
// if moduleA not present on disk, whole expression can be evaluated and may be resolved
126+
// if moduleA present on disk, we need to sort moduleB after moduleA. expression cannot be resolved yet
127+
$bDelayEvaluation = true;
128+
} else {
129+
$aReplacements[$sModuleId] = '(false)'; // Add parentheses to protect against invalid condition causing
130+
}
131+
118132
}
119133
}
120134

121-
foreach ($this->aRemainingModuleNamesToResolve as $sModuleName => $c) {
122-
if (array_key_exists($sModuleName, $aSelectedModules)) {
123-
// This module is actually a prerequisite
124-
if (!array_key_exists($sModuleName, $aModuleVersions)) {
125-
return;
126-
}
127-
}
135+
if ($bDelayEvaluation) {
136+
return;
128137
}
129138

130139
$bResult = false;

‎setup/moduledependency/module.class.inc.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,18 +93,18 @@ public function IsResolved(): bool
9393

9494
/**
9595
* Check if module dependencies are resolved with current list of module versions
96-
* @param array $aModuleVersions : versions by module names dict
97-
* @param array $aSelectedModules : modules names dict
96+
* @param array<string, string> $aResolvedModuleVersions : versions by module names dict
97+
* @param array<string> $aAllModuleNames : resolved modules names
9898
*
9999
* @return void
100100
*/
101-
public function UpdateModuleResolutionState(array $aModuleVersions, array $aSelectedModules): void
101+
public function UpdateModuleResolutionState(array $aResolvedModuleVersions, array $aAllModuleNames): void
102102
{
103103
$aNextDependencies = [];
104104

105105
foreach ($this->aRemainingDependenciesToResolve as $sDependencyExpression => $oModuleDependency) {
106106
/** @var DependencyExpression $oModuleDependency*/
107-
$oModuleDependency->UpdateModuleResolutionState($aModuleVersions, $aSelectedModules);
107+
$oModuleDependency->UpdateModuleResolutionState($aResolvedModuleVersions, $aAllModuleNames);
108108
if (!$oModuleDependency->IsResolved()) {
109109
$aNextDependencies[$sDependencyExpression] = $oModuleDependency;
110110
}

‎setup/moduledependency/moduledependencysort.class.inc.php‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,30 +33,32 @@ final public static function SetInstance(?ModuleDependencySort $oInstance): void
3333

3434
/**
3535
* Sort a list of modules, based on their (inter) dependencies
36+
*
3637
* @param array $aModules The list of modules to process: 'id' => $aModuleInfo
3738
* @param bool $bAbortOnMissingDependency ...
39+
*
3840
* @return array
3941
* @throws \MissingDependencyException
4042
*/
4143
public function GetModulesOrderedForInstallation($aModules, $bAbortOnMissingDependency = false)
4244
{
4345
// Filter modules to compute
4446
$aUnresolvedDependencyModules = [];
45-
$aModuleNames = [];
47+
$aAllModuleNames = [];
4648
foreach ($aModules as $sModuleId => $aModule) {
4749
$oModule = new Module($sModuleId);
4850
$sModuleName = $oModule->GetModuleName();
4951
$oModule->SetDependencies($aModule['dependencies']);
5052
$aUnresolvedDependencyModules[$sModuleId] = $oModule;
51-
$aModuleNames[$sModuleName] = true;
53+
$aAllModuleNames[$sModuleName] = true;
5254
}
5355

5456
// Make sure order is deterministic (alphabtical order)
5557
ksort($aUnresolvedDependencyModules);
5658

5759
//Attempt to resolve module dependencies
5860
$aOrderedModules = [];
59-
$aModuleVersions = [];
61+
$aResolvedModuleVersions = [];
6062
$iPreviousUnresolvedCount = -1;
6163
//loop until no dependency is resolved
6264
while ($iPreviousUnresolvedCount !== count($aUnresolvedDependencyModules)) {
@@ -67,10 +69,10 @@ public function GetModulesOrderedForInstallation($aModules, $bAbortOnMissingDepe
6769

6870
foreach ($aUnresolvedDependencyModules as $sModuleId => $oModule) {
6971
/** @var Module $oModule */
70-
$oModule->UpdateModuleResolutionState($aModuleVersions, $aModuleNames);
72+
$oModule->UpdateModuleResolutionState($aResolvedModuleVersions, $aAllModuleNames);
7173
if ($oModule->IsResolved()) {
7274
$aOrderedModules[] = $sModuleId;
73-
$aModuleVersions[$oModule->GetModuleName()] = $oModule->GetVersion();
75+
$aResolvedModuleVersions[$oModule->GetModuleName()] = $oModule->GetVersion();
7476
unset($aUnresolvedDependencyModules[$sModuleId]);
7577
}
7678
}
@@ -100,6 +102,7 @@ public function GetModulesOrderedForInstallation($aModules, $bAbortOnMissingDepe
100102
foreach ($aOrderedModules as $sId) {
101103
$aResult[$sId] = $aModules[$sId];
102104
}
105+
103106
return $aResult;
104107
}
105108

@@ -111,7 +114,7 @@ public function GetModulesOrderedForInstallation($aModules, $bAbortOnMissingDepe
111114
* - cyclic dependencies
112115
* - further versions of same module (name)
113116
*
114-
* @param array $aUnresolvedDependencyModules: dict of Module objects by moduleId key
117+
* @param array $aUnresolvedDependencyModules : dict of Module objects by moduleId key
115118
*
116119
* @return void
117120
*/
@@ -146,7 +149,7 @@ protected function SortModulesByCountOfDepencenciesDescending(array &$aUnresolve
146149

147150
uasort($aCountDepsByModuleId, function (array $aDeps1, array $aDeps2) {
148151
//compare $iInDegreeCounter
149-
$res = $aDeps1[0] - $aDeps2[0];
152+
$res = $aDeps1[0] - $aDeps2[0];
150153
if ($res != 0) {
151154
return $res;
152155
}
@@ -177,7 +180,7 @@ protected function SortModulesByCountOfDepencenciesDescending(array &$aUnresolve
177180
//when 2 versions of the same module (name) below array has been removed already
178181
if (array_key_exists($oModule->GetModuleName(), $aDependsOnModuleName)) {
179182
foreach ($aDependsOnModuleName[$oModule->GetModuleName()] as $sModuleId2) {
180-
if (! array_key_exists($sModuleId2, $aCountDepsByModuleId)) {
183+
if (!array_key_exists($sModuleId2, $aCountDepsByModuleId)) {
181184
continue;
182185
}
183186
$aDepCount = $aCountDepsByModuleId[$sModuleId2];

0 commit comments

Comments
 (0)