From e35e7859baf19004499074fe0f9db96fc37cbacd Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 3 Feb 2026 06:02:11 +0100 Subject: [PATCH] review: ModuleDiscovery:GetModulesOrderedByDependencies replacing deprecated GetAvailableModules method --- core/config.class.inc.php | 2 +- setup/extensionsmap.class.inc.php | 2 +- setup/modelfactory.class.inc.php | 2 +- setup/modulediscovery.class.inc.php | 44 ++-- .../AnalyzeInstallation.php | 2 +- .../InstallationChoicesService.php | 190 ------------------ setup/setuputils.class.inc.php | 2 +- 7 files changed, 21 insertions(+), 223 deletions(-) delete mode 100644 setup/moduleinstallation/InstallationChoicesService.php diff --git a/core/config.class.inc.php b/core/config.class.inc.php index 970b7e3b7..be7a95663 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -2766,7 +2766,7 @@ class Config $oEmptyConfig = new Config('dummy_file', false); // Do NOT load any config file, just set the default values $aAddOns = $oEmptyConfig->GetAddOns(); - $aModules = ModuleDiscovery::GetAvailableModules([APPROOT.$sModulesDir]); + $aModules = ModuleDiscovery::GetModulesOrderedByDependencies([APPROOT.$sModulesDir]); foreach ($aModules as $sModuleId => $aModuleInfo) { list($sModuleName, $sModuleVersion) = ModuleDiscovery::GetModuleName($sModuleId); if (is_null($aSelectedModules) || in_array($sModuleName, $aSelectedModules)) { diff --git a/setup/extensionsmap.class.inc.php b/setup/extensionsmap.class.inc.php index 34f480b84..b846061ca 100644 --- a/setup/extensionsmap.class.inc.php +++ b/setup/extensionsmap.class.inc.php @@ -329,7 +329,7 @@ class iTopExtensionsMap $aSearchDirs = array_merge($aSearchDirs, $this->aScannedDirs); try { - ModuleDiscovery::GetAvailableModules($aSearchDirs, true); + ModuleDiscovery::GetModulesOrderedByDependencies($aSearchDirs, true); } catch (MissingDependencyException $e) { // Some modules have missing dependencies // Let's check what is the impact at the "extensions" level diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php index 85bd28e88..98a6c38fc 100644 --- a/setup/modelfactory.class.inc.php +++ b/setup/modelfactory.class.inc.php @@ -1801,7 +1801,7 @@ EOF */ public function FindModules() { - $aAvailableModules = ModuleDiscovery::GetAvailableModules($this->aRootDirs); + $aAvailableModules = ModuleDiscovery::GetModulesOrderedByDependencies($this->aRootDirs); $aResult = []; foreach ($aAvailableModules as $sId => $aModule) { $oModule = new MFModule($sId, $aModule['root_dir'], $aModule['label'], isset($aModule['auto_select'])); diff --git a/setup/modulediscovery.class.inc.php b/setup/modulediscovery.class.inc.php index a6ce46c52..0a6ea4f19 100755 --- a/setup/modulediscovery.class.inc.php +++ b/setup/modulediscovery.class.inc.php @@ -196,21 +196,6 @@ class ModuleDiscovery } } - /** - * Get the list of "discovered" modules, ordered based on their (inter) dependencies - * - * @param bool $bAbortOnMissingDependency ... - * @param array $aModulesToLoad List of modules to search for, defaults to all if omitted - * - * @return array - * @throws \MissingDependencyException - */ - protected static function GetModules($bAbortOnMissingDependency = false, $aModulesToLoad = null) - { - // Order the modules to take into account their inter-dependencies - return self::OrderModulesByDependencies(self::$m_aModules, $bAbortOnMissingDependency, $aModulesToLoad); - } - /** * Arrange an list of modules, based on their (inter) dependencies * @param array $aModules The list of modules to process: 'id' => $aModuleInfo @@ -254,7 +239,7 @@ class ModuleDiscovery self::$m_aRemovedExtensions = $aRemovedExtension; } - public static function Init($aSearchDirs): void + private static function Init($aSearchDirs): void { if (self::$m_aSearchDirs != $aSearchDirs) { self::ResetCache(); @@ -277,10 +262,7 @@ class ModuleDiscovery } /** - * Search (on the disk) for all defined iTop modules, load them and returns the list (as an array) - * of the possible iTop modules to install - * Compute dependency as well (compatibilities & order) - * + * Return all modules found on disk ordered by dependencies. Skipping modules coming from extensions declared as removed (@see ModuleDiscovery::DeclareRemovedExtensions) * @param $aSearchDirs array of directories to search (absolute paths) * @param bool $bAbortOnMissingDependency ... * @param array $aModulesToLoad List of modules to search for, defaults to all if omitted @@ -288,17 +270,23 @@ class ModuleDiscovery * @return array A big array moduleID => ModuleData * @throws \Exception */ - public static function GetAvailableModules($aSearchDirs, $bAbortOnMissingDependency = false, $aModulesToLoad = null) + public static function GetModulesOrderedByDependencies($aSearchDirs, $bAbortOnMissingDependency = false, $aModulesToLoad = null) { self::Init($aSearchDirs); - return self::GetModules($bAbortOnMissingDependency, $aModulesToLoad); + return self::OrderModulesByDependencies(self::$m_aModules, $bAbortOnMissingDependency, $aModulesToLoad); } /** - * Search (on the disk) for all defined iTop modules, load them and returns the list (as an array) - * of the possible iTop modules to install - * No dependency consideration here. + * @deprecated use \ModuleDiscovery::GetModulesOrderedByDependencies instead + */ + public static function GetAvailableModules($aSearchDirs, $bAbortOnMissingDependency = false, $aModulesToLoad = null) + { + return ModuleDiscovery::GetModulesOrderedByDependencies($aSearchDirs, $bAbortOnMissingDependency, $aModulesToLoad); + } + + /** + * Return all modules found on disk (without any dependency consideration). Skipping modules coming from extensions declared as removed (@see ModuleDiscovery::DeclareRemovedExtensions) * * @param $aSearchDirs array of directories to search (absolute paths) * @@ -309,7 +297,7 @@ class ModuleDiscovery { self::Init($aSearchDirs); - $aModulesNonRemoved = []; + $aNonRemovedModules = []; foreach (self::$m_aModules as $sModuleId => $aModuleInfo) { $oModule = new Module($sModuleId); $sModuleName = $oModule->GetModuleName(); @@ -318,10 +306,10 @@ class ModuleDiscovery continue; } - $aModulesNonRemoved[$sModuleId] = $aModuleInfo; + $aNonRemovedModules[$sModuleId] = $aModuleInfo; } - return $aModulesNonRemoved; + return $aNonRemovedModules; } public static function ResetCache() diff --git a/setup/moduleinstallation/AnalyzeInstallation.php b/setup/moduleinstallation/AnalyzeInstallation.php index a1947b7df..6a30e6edf 100644 --- a/setup/moduleinstallation/AnalyzeInstallation.php +++ b/setup/moduleinstallation/AnalyzeInstallation.php @@ -73,7 +73,7 @@ class AnalyzeInstallation //test only $aAvailableModules = $this->aAvailableModules; } else { - $aAvailableModules = ModuleDiscovery::GetAvailableModules($aDirs, $bAbortOnMissingDependency, $aModulesToLoad); + $aAvailableModules = ModuleDiscovery::GetModulesOrderedByDependencies($aDirs, $bAbortOnMissingDependency, $aModulesToLoad); } foreach ($aAvailableModules as $sModuleId => $aModuleInfo) { diff --git a/setup/moduleinstallation/InstallationChoicesService.php b/setup/moduleinstallation/InstallationChoicesService.php deleted file mode 100644 index 68eabbcba..000000000 --- a/setup/moduleinstallation/InstallationChoicesService.php +++ /dev/null @@ -1,190 +0,0 @@ -Get('steps', []); - if (!is_array($aSteps)) { - return []; - } - } else { - $aSteps = $aInstallationChoices; - } - - $aInstalledModuleNames = $this->FindInstalledPackageModules($aPackageModules, $aInstallationChoices, $aSteps, $bInstallationFileProvided); - - $aInstalledModules = []; - foreach (array_keys($aPackageModules) as $sModuleId) { - list($sModuleName) = ModuleDiscovery::GetModuleName($sModuleId); - if (in_array($sModuleName, $aInstalledModuleNames)) { - $aInstalledModules[] = $sModuleId; - } - } - - return $aInstalledModules; - } - - private function FindInstalledPackageModules(array $aPackageModules, array $aInstallationChoices, array $aInstallationDescription, bool $bInstallationFileProvided): array - { - $aInstalledModules = []; - - $this->ProcessDefaultModules($aPackageModules, $aInstalledModules); - - if ($bInstallationFileProvided) { - $this->ProcessInstallationChoices(array_keys($aInstallationChoices), $aInstallationDescription, $aInstalledModules); - } else { - $aInstalledModules = array_merge($aInstalledModules, $aInstallationChoices); - } - - $this->ProcessAutoSelectModules($aPackageModules, $aInstalledModules); - - return array_keys($aInstalledModules); - } - - private function ProcessDefaultModules(array $aPackageModules, array &$aInstalledModules): void - { - foreach ($aPackageModules as $sModuleId => $aModule) { - if (($sModuleId != ROOT_MODULE)) { - if (isset($aModule['auto_select'])) { - continue; - } - - if (($aModule['category'] == 'authentication') || (!$aModule['visible'])) { - list($sModuleName) = ModuleDiscovery::GetModuleName($sModuleId); - $aInstalledModules[$sModuleName] = true; - } - } - } - } - - private function ProcessAutoSelectModules(array $aPackageModules, array &$aInstalledModules): void - { - foreach ($aPackageModules as $sModuleId => $aModule) { - if (($sModuleId !== ROOT_MODULE) && isset($aModule['auto_select'])) { - try { - SetupInfo::SetSelectedModules($aInstalledModules); - $bSelected = DependencyExpression::GetPhpExpressionEvaluator()->ParseAndEvaluateBooleanExpression($aModule['auto_select']); - if ($bSelected) { - list($sModuleName) = ModuleDiscovery::GetModuleName($sModuleId); - $aInstalledModules[$sModuleName] = true; - } - } catch (Exception $e) { - IssueLog::Error('Error evaluating module auto-select', null, [ - 'module' => $sModuleId, - 'error' => $e->getMessage(), - 'evaluated code' => $aModule['auto_select'], - 'stacktrace' => $e->getTraceAsString(), - ]); - } - } - } - } - - private function ProcessInstallationChoices(array $aInstallationChoices, array $aInstallationDescription, array &$aInstalledModules): void - { - foreach ($aInstallationDescription as $aStepInfo) { - $aOptions = $aStepInfo['options'] ?? null; - if (is_array($aOptions)) { - foreach ($aOptions as $aChoiceInfo) { - $this->ProcessSelectedChoice($aInstallationChoices, $aChoiceInfo, $aInstalledModules); - } - } - $aOptions = $aStepInfo['alternatives'] ?? null; - if (is_array($aOptions)) { - foreach ($aOptions as $aChoiceInfo) { - $this->ProcessSelectedChoice($aInstallationChoices, $aChoiceInfo, $aInstalledModules); - } - } - } - } - - private function ProcessSelectedChoice(array $aInstallationChoices, array $aChoiceInfo, array &$aInstalledModules) - { - if (!is_array($aChoiceInfo)) { - return; - } - - $sMandatory = $aChoiceInfo['mandatory'] ?? 'false'; - - $aCurrentModules = $aChoiceInfo['modules'] ?? []; - $sExtensionCode = $aChoiceInfo['extension_code']; - - $bSelected = ($sMandatory === 'true') || in_array($sExtensionCode, $aInstallationChoices); - - if (!$bSelected) { - return; - } - - foreach ($aCurrentModules as $sModuleId) { - $aInstalledModules[$sModuleId] = true; - } - - $aAlternatives = $aChoiceInfo['alternatives'] ?? null; - if (is_array($aAlternatives)) { - foreach ($aAlternatives as $aSubChoiceInfo) { - $this->ProcessSelectedChoice($aInstallationChoices, $aSubChoiceInfo, $aInstalledModules); - } - } - - $aSubOptionsChoiceInfo = $aChoiceInfo['sub_options'] ?? null; - if (is_array($aSubOptionsChoiceInfo)) { - $aSubOptions = $aSubOptionsChoiceInfo['options'] ?? null; - if (is_array($aSubOptions)) { - foreach ($aSubOptions as $aSubChoiceInfo) { - $this->ProcessSelectedChoice($aInstallationChoices, $aSubChoiceInfo, $aInstalledModules); - } - } - $aSubAlternatives = $aSubOptionsChoiceInfo['alternatives'] ?? null; - if (is_array($aSubAlternatives)) { - foreach ($aSubAlternatives as $aSubChoiceInfo) { - $this->ProcessSelectedChoice($aInstallationChoices, $aSubChoiceInfo, $aInstalledModules); - } - } - } - } - -} diff --git a/setup/setuputils.class.inc.php b/setup/setuputils.class.inc.php index 6bce7ba82..6378f4179 100644 --- a/setup/setuputils.class.inc.php +++ b/setup/setuputils.class.inc.php @@ -509,7 +509,7 @@ class SetupUtils } require_once(APPROOT.'setup/modulediscovery.class.inc.php'); try { - ModuleDiscovery::GetAvailableModules($aDirsToScan, true, $aSelectedModules); + ModuleDiscovery::GetModulesOrderedByDependencies($aDirsToScan, true, $aSelectedModules); } catch (Exception $e) { $aResult[] = new CheckResult(CheckResult::ERROR, $e->getMessage()); }