From d1e91087b9de75d6a048bb4fd8ea4ce304ca25c2 Mon Sep 17 00:00:00 2001 From: odain Date: Fri, 9 Jan 2026 08:28:01 +0100 Subject: [PATCH 01/13] =?UTF-8?q?N=C2=B08764=20-=20Halt=20setup=20if=20dat?= =?UTF-8?q?abase=20is=20not=20compatible=20with=20an=20uninstallation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/applicationinstaller.class.inc.php | 111 +++++++++++++++--- .../ModelReflectionSerializer.php | 27 ++++- setup/feature_removal/SetupAudit.php | 32 +++-- setup/wizardsteps.class.inc.php | 13 -- .../setup/feature_removal/SetupAuditTest.php | 20 +++- 5 files changed, 156 insertions(+), 47 deletions(-) diff --git a/setup/applicationinstaller.class.inc.php b/setup/applicationinstaller.class.inc.php index 377998b05..064066a1e 100644 --- a/setup/applicationinstaller.class.inc.php +++ b/setup/applicationinstaller.class.inc.php @@ -17,9 +17,13 @@ // You should have received a copy of the GNU Affero General Public License // along with iTop. If not, see +use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer; +use Combodo\iTop\Setup\FeatureRemoval\SetupAudit; + require_once(APPROOT.'setup/parameters.class.inc.php'); require_once(APPROOT.'setup/xmldataloader.class.inc.php'); require_once(APPROOT.'setup/backup.class.inc.php'); +require_once APPROOT.'setup/feature_removal/SetupAudit.php'; /** * The base class for the installation process. @@ -258,6 +262,7 @@ class ApplicationInstaller $sExtensionDir = $this->oParams->Get('extensions_dir', 'extensions'); $aMiscOptions = $this->oParams->Get('options', []); $aRemovedExtensionCodes = $this->oParams->Get('removed_extensions', []); + $sForceUninstall = $this->oParams->Get('force-uninstall', ''); $bUseSymbolicLinks = null; if ((isset($aMiscOptions['symlinks']) && $aMiscOptions['symlinks'])) { @@ -274,29 +279,36 @@ class ApplicationInstaller $aSelectedModules, $sSourceDir, $sExtensionDir, + $sForceUninstall, $bUseSymbolicLinks ); + $aResult = [ + 'status' => self::OK, + 'message' => '', + 'next-step' => 'setup-audit', + 'next-step-label' => 'Checking data consistency with the new data model', + 'percentage-completed' => 40, + ]; + break; + + case 'setup-audit': + $sForceUninstall = $this->oParams->Get('force-uninstall', ''); + $this->DoSetupAudit($sForceUninstall); $aResult = [ 'status' => self::OK, 'message' => '', 'next-step' => 'db-schema', 'next-step-label' => 'Updating database schema', - 'percentage-completed' => 40, + 'percentage-completed' => 50, ]; break; case 'db-schema': $aSelectedModules = $this->oParams->Get('selected_modules', []); - $aParamValues = $this->oParams->GetParamForConfigArray(); - $bOldAddon = $this->oParams->Get('old_addon', false); - $sUrl = $this->oParams->Get('url', ''); $this->DoUpdateDBSchema( - $aSelectedModules, - $aParamValues, - $bOldAddon, - $sUrl + $aSelectedModules ); $aResult = [ @@ -487,6 +499,7 @@ class ApplicationInstaller * @param array $aSelectedModules * @param string $sSourceDir * @param string $sExtensionDir + * @param string $sForceUninstall * @param boolean $bUseSymbolicLinks * * @return void @@ -495,8 +508,14 @@ class ApplicationInstaller * * @since 3.1.0 N°2013 added the aParamValues param */ - protected function DoCompile($aRemovedExtensionCodes, $aSelectedModules, $sSourceDir, $sExtensionDir, $bUseSymbolicLinks = null) + protected function DoCompile($aRemovedExtensionCodes, $aSelectedModules, $sSourceDir, $sExtensionDir, $sForceUninstall, $bUseSymbolicLinks = null) { + /** + * @since 3.2.0 move the ContextTag init at the very beginning of the method + * @noinspection PhpUnusedLocalVariableInspection + */ + $oContextTag = new ContextTag(ContextTag::TAG_SETUP); + SetupLog::Info("Compiling data model."); require_once(APPROOT.'setup/modulediscovery.class.inc.php'); @@ -528,7 +547,20 @@ class ApplicationInstaller if (!is_dir($sSourcePath)) { throw new Exception("Failed to find the source directory '$sSourcePath', please check the rights of the web server"); } + $bIsAlreadyInMaintenanceMode = SetupUtils::IsInMaintenanceMode(); + if ($sForceUninstall === "checked") { + //audit required + SetupLog::Info(__METHOD__, null, ['force-uninstall' => $sForceUninstall]); + if ($bIsAlreadyInMaintenanceMode) { + //required to read DM before calling SaveModelInfo + SetupUtils::ExitMaintenanceMode(); + $bIsAlreadyInMaintenanceMode = false; + } + + $this->SaveModelInfo($sEnvironment); + } + if (($sEnvironment == 'production') && !$bIsAlreadyInMaintenanceMode) { $sConfigFilePath = utils::GetConfigFilePath($sEnvironment); if (is_file($sConfigFilePath)) { @@ -620,23 +652,70 @@ class ApplicationInstaller } } + private function GetModelInfoPath(): string + { + return APPROOT.'data/beforecompilation_modelinfo.json'; + } + + private function SaveModelInfo(string $sEnvironment): void + { + $aModelInfo = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($sEnvironment); + $sModelInfoPath = $this->GetModelInfoPath(); + file_put_contents($sModelInfoPath, json_encode($aModelInfo)); + } + + private function GetPreviousModelInfo(string $sEnvironment): array + { + $sContent = file_get_contents($this->GetModelInfoPath()); + $aModelInfo = json_decode($sContent, true); + + if (false === $aModelInfo) { + throw new \Exception("Could not read (before compilation) previous model to audit data"); + } + + return $aModelInfo; + } + + protected function DoSetupAudit(string $sForceUninstall) + { + /** + * @since 3.2.0 move the ContextTag init at the very beginning of the method + * @noinspection PhpUnusedLocalVariableInspection + */ + $oContextTag = new ContextTag(ContextTag::TAG_SETUP); + + $aParamValues = $this->oParams->GetParamForConfigArray(); + $sMode = $aParamValues['mode']; + if ($sMode !== "upgrade") { + return; + } + + if ($sForceUninstall !== "checked") { + SetupLog::Info("Setup data audit disabled (force-uninstall)"); + return; + } else { + SetupLog::Info(__METHOD__, null, ['force-uninstall' => $sForceUninstall]); + } + + $sTargetEnvironment = $this->GetTargetEnv(); + $aPreviousCompilationModelInfo = $this->GetPreviousModelInfo($sTargetEnvironment); + + $oSetupAudit = new SetupAudit($sTargetEnvironment, $sTargetEnvironment); + $oSetupAudit->ComputeClasses($aPreviousCompilationModelInfo); + } + /** * @param $aSelectedModules - * @param $sModulesDir - * @param $aParamValues - * @param string $sTargetEnvironment - * @param bool $bOldAddon - * @param string $sAppRootUrl * * @throws \ConfigException * @throws \CoreException * @throws \MySQLException */ - protected function DoUpdateDBSchema($aSelectedModules, $aParamValues, $bOldAddon = false, $sAppRootUrl = '') + protected function DoUpdateDBSchema($aSelectedModules) { $sTargetEnvironment = $this->GetTargetEnv(); $sModulesDir = $this->GetTargetDir(); - + $aParamValues = $this->oParams->GetParamForConfigArray(); /** * @since 3.2.0 move the ContextTag init at the very beginning of the method * @noinspection PhpUnusedLocalVariableInspection diff --git a/setup/feature_removal/ModelReflectionSerializer.php b/setup/feature_removal/ModelReflectionSerializer.php index 5aff4dcdd..2468c963b 100644 --- a/setup/feature_removal/ModelReflectionSerializer.php +++ b/setup/feature_removal/ModelReflectionSerializer.php @@ -2,8 +2,12 @@ namespace Combodo\iTop\Setup\FeatureRemoval; +use ContextTag; use CoreException; use Exception; +use IssueLog; +use SetupLog; +use Utils; class ModelReflectionSerializer { @@ -29,27 +33,38 @@ class ModelReflectionSerializer public function GetModelFromEnvironment(string $sEnv): array { - \IssueLog::Info(__METHOD__, null, ['env' => $sEnv]); - $sPHPExec = trim(\MetaModel::GetConfig()->Get('php_path')); + IssueLog::Info(__METHOD__, null, ['env' => $sEnv]); + $sPHPExec = trim(Utils::GetConfig()->Get('php_path')); $sOutput = ""; $iRes = 0; + exec(sprintf("$sPHPExec %s/get_model_reflection.php --env='%s'", __DIR__, $sEnv), $sOutput, $iRes); if ($iRes != 0) { - \IssueLog::Error("Cannot get classes", null, ['env' => $sEnv, 'code' => $iRes, "output" => $sOutput]); + $this->LogErrorWithProperLogger("Cannot get classes", null, ['env' => $sEnv, 'code' => $iRes, "output" => $sOutput]); throw new CoreException("Cannot get classes"); } $aClasses = json_decode($sOutput[0] ?? null, true); if (false === $aClasses) { - \IssueLog::Error("Invalid JSON", null, ["output" => $sOutput]); + $this->LogErrorWithProperLogger("Invalid JSON", null, ['env' => $sEnv, "output" => $sOutput]); throw new Exception("cannot get classes"); } if (!is_array($aClasses)) { - \IssueLog::Error("not an array", null, ["classes" => $aClasses]); - throw new Exception("cannot get classes"); + $this->LogErrorWithProperLogger("not an array", null, ['env' => $sEnv, "classes" => $aClasses, "output" => $sOutput]); + throw new Exception("cannot get classes from $sEnv"); } return $aClasses; } + + //could be shared with others in log APIs ? + private function LogErrorWithProperLogger($sMessage, $sChannel = null, $aContext = []): void + { + if (ContextTag::Check(ContextTag::TAG_SETUP)) { + SetupLog::Error($sMessage, $sChannel, $aContext); + } else { + IssueLog::Error($sMessage, $sChannel, $aContext); + } + } } diff --git a/setup/feature_removal/SetupAudit.php b/setup/feature_removal/SetupAudit.php index 76831a663..f90a29c7d 100644 --- a/setup/feature_removal/SetupAudit.php +++ b/setup/feature_removal/SetupAudit.php @@ -16,21 +16,34 @@ class SetupAudit private string $sEnvBeforeExtensionRemoval; private string $sEnvAfterExtensionRemoval; - private array $aClassesBeforeRemoval; - private array $aClassesAfterRemoval; - private array $aRemovedClasses; - private array $aFinalClassesRemoved; + private bool $bClassesInitialized = false; + private array $aClassesBeforeRemoval = []; + private array $aClassesAfterRemoval = []; + private array $aRemovedClasses = []; + private array $aFinalClassesRemoved = []; public function __construct(string $sEnvBeforeExtensionRemoval, string $sEnvAfterExtensionRemoval = DryRemovalRuntimeEnvironment::DRY_REMOVAL_AUDIT_ENV) { $this->sEnvBeforeExtensionRemoval = $sEnvBeforeExtensionRemoval; $this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; + } + + public function ComputeClasses(array $aClassesBeforeRemoval = null) + { + if ($this->bClassesInitialized) { + return; + } $sCurrentEnvt = MetaModel::GetEnvironment(); - if ($sCurrentEnvt === $this->sEnvBeforeExtensionRemoval) { - $this->aClassesBeforeRemoval = MetaModel::GetClasses(); + + if (is_null($aClassesBeforeRemoval)) { + if ($sCurrentEnvt === $this->sEnvBeforeExtensionRemoval) { + $this->aClassesBeforeRemoval = MetaModel::GetClasses(); + } else { + $this->aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBeforeExtensionRemoval); + } } else { - $this->aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBeforeExtensionRemoval); + $this->aClassesBeforeRemoval = $aClassesBeforeRemoval; } if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { @@ -39,8 +52,7 @@ class SetupAudit $this->aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfterExtensionRemoval); } - $this->aRemovedClasses = []; - $this->aFinalClassesRemoved = []; + $this->bClassesInitialized = true; } /*public function SetSelectedExtensions(Config $oConfig, array $aSelectedExtensions) @@ -56,6 +68,8 @@ class SetupAudit public function GetRemovedClasses(): array { + $this->ComputeClasses(); + if (count($this->aRemovedClasses) == 0) { if (count($this->aClassesBeforeRemoval) == 0) { return $this->aRemovedClasses; diff --git a/setup/wizardsteps.class.inc.php b/setup/wizardsteps.class.inc.php index 12db0dadd..a97b680c2 100644 --- a/setup/wizardsteps.class.inc.php +++ b/setup/wizardsteps.class.inc.php @@ -50,7 +50,6 @@ require_once(APPROOT.'setup/applicationinstaller.class.inc.php'); require_once(APPROOT.'setup/parameters.class.inc.php'); require_once(APPROOT.'core/mutex.class.inc.php'); require_once(APPROOT.'setup/extensionsmap.class.inc.php'); -require_once APPROOT.'setup/feature_removal/SetupAudit.php'; /** * First step of the iTop Installation Wizard: Welcome screen, requirements @@ -2178,18 +2177,6 @@ class WizStepSummary extends WizardStep $this->bDependencyCheck = true; try { SetupUtils::AnalyzeInstallation($this->oWizard, true, $aSelectedModules); - - /*$sInstallMode = utils::ReadParam('install_mode'); - \SetupLog::Info(__METHOD__, null, ['$sInstallMode' => $sInstallMode]); - //if ($sInstallMode === "upgrade") { - $aExtensions = json_decode($this->oWizard->GetParameter('selected_extensions'), true); - $oSetupAudit = new SetupAudit([]); - - $oConfig = SetupUtils::GetConfig($this->oWizard); - $oSetupAudit->SetSelectedExtensions($oConfig, $aExtensions); - //$oSetupAudit->AuditExtensionsCleanupRules(true); - //} - */ } catch (MissingDependencyException $e) { $this->bDependencyCheck = false; $this->sDependencyIssue = $e->getHtmlDesc(); diff --git a/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php b/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php index 841c5ff3c..68707358c 100644 --- a/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php @@ -3,10 +3,12 @@ namespace Combodo\iTop\Test\UnitTest\Setup\FeatureRemoval; use Combodo\iTop\Setup\FeatureRemoval\DryRemovalRuntimeEnvironment; +use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer; use Combodo\iTop\Setup\FeatureRemoval\SetupAudit; use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase; use Combodo\iTop\Test\UnitTest\Service\UnitTestRunTimeEnvironment; use Exception; +use MetaModel; class SetupAuditTest extends ItopCustomDatamodelTestCase { @@ -52,7 +54,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase $oDryRemovalRuntimeEnvt->Prepare($this->GetTestEnvironment(), ['nominal_ext1', 'finalclass_ext2']); $oDryRemovalRuntimeEnvt->CompileFrom($this->GetTestEnvironment()); - $oSetupAudit = new SetupAudit(\MetaModel::GetEnvironment()); + $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment()); $expected = [ "Feature1Module1MyClass", @@ -67,13 +69,25 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase $this->assertEqualsCanonicalizing($expected, $oSetupAudit->GetIssues()); } + public function testGetRemovedClassesFromSetupWizard() + { + $sEnv = MetaModel::GetEnvironment(); + + $aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($sEnv); + $aClassesAfterRemoval[] = "GabuZomeu"; + + $oSetupAudit = new SetupAudit($sEnv, $sEnv); + $oSetupAudit->ComputeClasses($aClassesAfterRemoval); + $this->assertEquals(["GabuZomeu"], $oSetupAudit->GetRemovedClasses()); + } + public function testGetIssues() { $sUID = "AuditExtensionsCleanupRules_".uniqid(); $oOrg = $this->CreateOrganization($sUID); $this->createObject('FinalClassFeature1Module1MyFinalClassFromLocation', ['org_id' => $oOrg->GetKey(), 'name' => $sUID, 'name2' => uniqid()]); - $oSetupAudit = new SetupAudit(\MetaModel::GetEnvironment()); + $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment()); $aRemovedClasses = [ "Feature1Module1MyClass", "FinalClassFeature1Module1MyClass", @@ -99,7 +113,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase $this->createObject('FinalClassFeature1Module1MyFinalClassFromLocation', ['org_id' => $oOrg->GetKey(), 'name' => $sUID, 'name2' => uniqid()]); $this->createObject('FinalClassFeature2Module1MyFinalClassFromLocation', ['org_id' => $oOrg->GetKey(), 'name' => $sUID, 'name2' => uniqid()]); - $oSetupAudit = new SetupAudit(\MetaModel::GetEnvironment()); + $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment()); $aRemovedClasses = [ "Feature1Module1MyClass", "FinalClassFeature1Module1MyClass", From 43bd6217319976c076a1d287525df9864b58123b Mon Sep 17 00:00:00 2001 From: odain Date: Fri, 9 Jan 2026 11:50:33 +0100 Subject: [PATCH 02/13] =?UTF-8?q?N=C2=B07629:=20improve=20InterfaceDiscove?= =?UTF-8?q?ry::GetCandidateClasses=20robustness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sources/Service/InterfaceDiscovery/InterfaceDiscovery.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php b/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php index 67a5ca2fa..95fec0f43 100644 --- a/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php +++ b/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php @@ -178,6 +178,13 @@ class InterfaceDiscovery continue; } $aTmpClassMap = include $sAutoloadFile; + if (! is_array($aTmpClassMap)) { + //can happen when setup compilation broken in the middle + //ex: $sAutoloadFile could be empty and $aTmpClassMap is a int + $aAutoloaderErrors[] = $sAutoloadFile; + continue; + } + /** @noinspection SlowArrayOperationsInLoopInspection we are getting an associative array so the documented workarounds cannot be used */ $aClassMap = array_merge($aClassMap, $aTmpClassMap); } From 0996e8fae041020de08e30170e6becd23ddf8e06 Mon Sep 17 00:00:00 2001 From: odain Date: Fri, 9 Jan 2026 14:52:00 +0100 Subject: [PATCH 03/13] =?UTF-8?q?N=C2=B08764=20-=20stop=20setup=20and=20di?= =?UTF-8?q?splay=20data=20to=20cleanup=20message?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/applicationinstaller.class.inc.php | 31 ++++++++++++++---------- setup/feature_removal/SetupAudit.php | 24 ++++++++++++++++++ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/setup/applicationinstaller.class.inc.php b/setup/applicationinstaller.class.inc.php index 064066a1e..e989e5f41 100644 --- a/setup/applicationinstaller.class.inc.php +++ b/setup/applicationinstaller.class.inc.php @@ -262,7 +262,7 @@ class ApplicationInstaller $sExtensionDir = $this->oParams->Get('extensions_dir', 'extensions'); $aMiscOptions = $this->oParams->Get('options', []); $aRemovedExtensionCodes = $this->oParams->Get('removed_extensions', []); - $sForceUninstall = $this->oParams->Get('force-uninstall', ''); + $sDisableDataAudit = $this->oParams->Get('disable-data-audit', ''); $bUseSymbolicLinks = null; if ((isset($aMiscOptions['symlinks']) && $aMiscOptions['symlinks'])) { @@ -279,7 +279,7 @@ class ApplicationInstaller $aSelectedModules, $sSourceDir, $sExtensionDir, - $sForceUninstall, + $sDisableDataAudit, $bUseSymbolicLinks ); @@ -293,8 +293,8 @@ class ApplicationInstaller break; case 'setup-audit': - $sForceUninstall = $this->oParams->Get('force-uninstall', ''); - $this->DoSetupAudit($sForceUninstall); + $sDisableDataAudit = $this->oParams->Get('disable-data-audit', ''); + $this->DoSetupAudit($sDisableDataAudit); $aResult = [ 'status' => self::OK, 'message' => '', @@ -499,7 +499,7 @@ class ApplicationInstaller * @param array $aSelectedModules * @param string $sSourceDir * @param string $sExtensionDir - * @param string $sForceUninstall + * @param string $sDisableDataAudit * @param boolean $bUseSymbolicLinks * * @return void @@ -508,7 +508,7 @@ class ApplicationInstaller * * @since 3.1.0 N°2013 added the aParamValues param */ - protected function DoCompile($aRemovedExtensionCodes, $aSelectedModules, $sSourceDir, $sExtensionDir, $sForceUninstall, $bUseSymbolicLinks = null) + protected function DoCompile($aRemovedExtensionCodes, $aSelectedModules, $sSourceDir, $sExtensionDir, $sDisableDataAudit, $bUseSymbolicLinks = null) { /** * @since 3.2.0 move the ContextTag init at the very beginning of the method @@ -549,9 +549,9 @@ class ApplicationInstaller } $bIsAlreadyInMaintenanceMode = SetupUtils::IsInMaintenanceMode(); - if ($sForceUninstall === "checked") { + if ($sDisableDataAudit !== "checked") { //audit required - SetupLog::Info(__METHOD__, null, ['force-uninstall' => $sForceUninstall]); + SetupLog::Info(__METHOD__, null, ['disable-data-audit' => $sDisableDataAudit]); if ($bIsAlreadyInMaintenanceMode) { //required to read DM before calling SaveModelInfo SetupUtils::ExitMaintenanceMode(); @@ -670,13 +670,13 @@ class ApplicationInstaller $aModelInfo = json_decode($sContent, true); if (false === $aModelInfo) { - throw new \Exception("Could not read (before compilation) previous model to audit data"); + throw new Exception("Could not read (before compilation) previous model to audit data"); } return $aModelInfo; } - protected function DoSetupAudit(string $sForceUninstall) + protected function DoSetupAudit(string $sDisableDataAudit) { /** * @since 3.2.0 move the ContextTag init at the very beginning of the method @@ -690,11 +690,9 @@ class ApplicationInstaller return; } - if ($sForceUninstall !== "checked") { + if ($sDisableDataAudit === "checked") { SetupLog::Info("Setup data audit disabled (force-uninstall)"); return; - } else { - SetupLog::Info(__METHOD__, null, ['force-uninstall' => $sForceUninstall]); } $sTargetEnvironment = $this->GetTargetEnv(); @@ -702,6 +700,13 @@ class ApplicationInstaller $oSetupAudit = new SetupAudit($sTargetEnvironment, $sTargetEnvironment); $oSetupAudit->ComputeClasses($aPreviousCompilationModelInfo); + + try { + $oSetupAudit->GetIssues(true); + } catch (Exception $e) { + $iCount = $oSetupAudit->GetLastComputedFinalClassesRemovedCount(); + throw new Exception("$iCount elements require data adjustments or cleanup in the backoffice prior to upgrading iTop"); + } } /** diff --git a/setup/feature_removal/SetupAudit.php b/setup/feature_removal/SetupAudit.php index f90a29c7d..0b70a13bc 100644 --- a/setup/feature_removal/SetupAudit.php +++ b/setup/feature_removal/SetupAudit.php @@ -2,9 +2,12 @@ namespace Combodo\iTop\Setup\FeatureRemoval; +use ContextTag; use DBObjectSearch; use DBObjectSet; +use IssueLog; use MetaModel; +use SetupLog; require_once APPROOT.'setup/feature_removal/ModelReflectionSerializer.php'; @@ -121,14 +124,25 @@ class SetupAudit $this->aFinalClassesRemoved[$sClass] = $iCount; if ($bThrowExceptionAtFirstIssue && $iCount > 0) { //setup envt: should raise issue ASAP + $this->LogInfoWithProperLogger("Setup audit found data to cleanup", null, $this->aFinalClassesRemoved); throw new \Exception($sClass); } } } + $this->LogInfoWithProperLogger("Setup audit found data to cleanup", null, $this->aFinalClassesRemoved); return $this->aFinalClassesRemoved; } + public function GetLastComputedFinalClassesRemovedCount(): int + { + $res = 0; + foreach ($this->aFinalClassesRemoved as $sClass => $iCount) { + $res += $iCount; + } + return $res; + } + private function Count($sClass): int { $oSearch = DBObjectSearch::FromOQL("SELECT $sClass", []); @@ -137,4 +151,14 @@ class SetupAudit return $oSet->Count(); } + + //could be shared with others in log APIs ? + private function LogInfoWithProperLogger($sMessage, $sChannel = null, $aContext = []): void + { + if (ContextTag::Check(ContextTag::TAG_SETUP)) { + SetupLog::Info($sMessage, $sChannel, $aContext); + } else { + IssueLog::Info($sMessage, $sChannel, $aContext); + } + } } From 3a3f5736c0823822a2e831115e2aff8fb5bbe879 Mon Sep 17 00:00:00 2001 From: odain Date: Fri, 9 Jan 2026 15:07:59 +0100 Subject: [PATCH 04/13] =?UTF-8?q?N=C2=B08764=20-=20fix=20first=20install?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/applicationinstaller.class.inc.php | 28 +++++++++++++++--------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/setup/applicationinstaller.class.inc.php b/setup/applicationinstaller.class.inc.php index e989e5f41..24f22144f 100644 --- a/setup/applicationinstaller.class.inc.php +++ b/setup/applicationinstaller.class.inc.php @@ -549,9 +549,7 @@ class ApplicationInstaller } $bIsAlreadyInMaintenanceMode = SetupUtils::IsInMaintenanceMode(); - if ($sDisableDataAudit !== "checked") { - //audit required - SetupLog::Info(__METHOD__, null, ['disable-data-audit' => $sDisableDataAudit]); + if ($this->IsSetupDataAuditEnabled($sDisableDataAudit, $aParamValues)) { if ($bIsAlreadyInMaintenanceMode) { //required to read DM before calling SaveModelInfo SetupUtils::ExitMaintenanceMode(); @@ -685,13 +683,7 @@ class ApplicationInstaller $oContextTag = new ContextTag(ContextTag::TAG_SETUP); $aParamValues = $this->oParams->GetParamForConfigArray(); - $sMode = $aParamValues['mode']; - if ($sMode !== "upgrade") { - return; - } - - if ($sDisableDataAudit === "checked") { - SetupLog::Info("Setup data audit disabled (force-uninstall)"); + if (! $this->IsSetupDataAuditEnabled($sDisableDataAudit, $aParamValues)) { return; } @@ -709,6 +701,22 @@ class ApplicationInstaller } } + private function IsSetupDataAuditEnabled($sDisableDataAudit, array $aParamValues): bool + { + $sMode = $aParamValues['mode']; + if ($sMode !== "upgrade") { + //first install + return false; + } + + if ($sDisableDataAudit === "checked") { + SetupLog::Info("Setup data audit disabled", null, ['disable-data-audit' => $sDisableDataAudit]); + return false; + } + + return true; + } + /** * @param $aSelectedModules * From 63e473e6d0e14a47d791f6cd50fac0003d89a844 Mon Sep 17 00:00:00 2001 From: odain Date: Fri, 9 Jan 2026 23:21:30 +0100 Subject: [PATCH 05/13] =?UTF-8?q?N=C2=B08981:=20test=20ModuleDiscovery=20f?= =?UTF-8?q?iltered=20by=20removed=20extensions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/modulediscovery.class.inc.php | 2 - .../setup/ModuleDiscoveryTest.php | 102 +++++++++++++++++- 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/setup/modulediscovery.class.inc.php b/setup/modulediscovery.class.inc.php index e1c00e76b..4acd00318 100755 --- a/setup/modulediscovery.class.inc.php +++ b/setup/modulediscovery.class.inc.php @@ -239,7 +239,6 @@ class ModuleDiscovery } } } - return ModuleDependencySort::GetInstance()->GetModulesOrderedForInstallation($aFilteredModules, $bAbortOnMissingDependency); } @@ -260,7 +259,6 @@ class ModuleDiscovery if (count(self::$m_aRemovedExtensions) === 0) { return false; } - $aNonMatchingPaths = []; $sModuleFilePath = $aModuleInfo[ModuleFileReader::MODULE_FILE_PATH]; diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php index badaaafb6..5252c4b50 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php @@ -2,7 +2,9 @@ namespace Combodo\iTop\Test\UnitTest\Setup; +use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReader; use Combodo\iTop\Test\UnitTest\ItopTestCase; +use iTopExtension; use MissingDependencyException; use ModuleDiscovery; @@ -15,6 +17,12 @@ class ModuleDiscoveryTest extends ItopTestCase $this->RequireOnceItopFile('setup/modulediscovery.class.inc.php'); } + protected function tearDown(): void + { + parent::tearDown(); + ModuleDiscovery::DeclareRemovedExtensions([]); + } + public function testOrderModulesByDependencies_RealExample() { $aModules = json_decode(file_get_contents(__DIR__.'/ressources/reallife_discovered_modules.json'), true); @@ -78,6 +86,41 @@ TXT; ModuleDiscovery::OrderModulesByDependencies($aModules, true, $aChoices); } + public function testOrderModulesByDependencies_FailWhenChoosenModuleDependsOnRemovedExtensionModule() + { + $aChoices = ['id1', 'id2']; + + $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); + $this->aFileToClean[] = $sModuleFilePath; + $sModuleFilePath2 = tempnam(sys_get_temp_dir(), 'discovery_test'); + $this->aFileToClean[] = $sModuleFilePath2; + + $aModules = [ + "id1/1" => [ + 'dependencies' => [ 'id2/2'], + 'label' => 'label1', + ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath, + ], + "id2/2" => [ + 'dependencies' => [], + 'label' => 'label2', + ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath2, + ], + ]; + + $oExtension = $this->CreateExtensionWithModule('id2', '2', $sModuleFilePath2); + ModuleDiscovery::DeclareRemovedExtensions([$oExtension]); + + $sExpectedMessage = <<expectException(MissingDependencyException::class); + $this->expectExceptionMessage($sExpectedMessage); + + ModuleDiscovery::OrderModulesByDependencies($aModules, true, $aChoices); + } + public function GetModuleNameProvider() { return [ @@ -109,6 +152,63 @@ TXT; */ public function testGetModuleName($sModuleId, $expectedName, $expectedVersion) { - $this->assertEquals([$expectedName, $expectedVersion], \ModuleDiscovery::GetModuleName($sModuleId)); + $this->assertEquals([$expectedName, $expectedVersion], ModuleDiscovery::GetModuleName($sModuleId)); + } + + public function testIsModulePartOfRemovedExtension_NoRemovedExtension() + { + ModuleDiscovery::DeclareRemovedExtensions([]); + $this->assertFalse($this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModulePartOfRemovedExtension", ['module_name', '123', []])); + } + + public function testIsModulePartOfRemovedExtension_ModuleWithAnotherVersionIncludedInRemoveExtension() + { + $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); + $this->ValidateIsModulePartOfRemovedExtension('module_name', '123', $sModuleFilePath, $sModuleFilePath, false); + } + + public function testIsModulePartOfRemovedExtension_AnotherModuleWithSameVersionIncludedInRemoveExtension() + { + $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); + $this->ValidateIsModulePartOfRemovedExtension('another_module_name', '456', $sModuleFilePath, $sModuleFilePath, false); + } + + public function testIsModulePartOfRemovedExtension_SameExtensionComingFromAnotherLocation() + { + $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); + $sModuleFilePath2 = tempnam(sys_get_temp_dir(), 'discovery_test'); + $this->ValidateIsModulePartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath2, false); + } + + public function testIsModulePartOfRemovedExtension_ModuleShouldBeExcluded() + { + $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); + $this->ValidateIsModulePartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath, true); + } + + public function ValidateIsModulePartOfRemovedExtension($sModuleName, $sModuleVersion, $sModuleFilePath1, $sModuleFilePath2, $bExpected) + { + $this->aFileToClean[] = $sModuleFilePath1; + $this->aFileToClean[] = $sModuleFilePath2; + + $oExtension = $this->CreateExtensionWithModule('module_name', '456', $sModuleFilePath2); + ModuleDiscovery::DeclareRemovedExtensions([$oExtension]); + $aCurrentModuleInfo = [ + ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath1, + ]; + $this->assertEquals( + $bExpected, + $this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModulePartOfRemovedExtension", [$sModuleName, $sModuleVersion, $aCurrentModuleInfo]) + ); + } + + private function CreateExtensionWithModule(string $sModuleName, string $sVersion, bool|string $sModuleFilePath): iTopExtension + { + $oExt = new iTopExtension(); + $oExt->aModuleVersion[$sModuleName] = $sVersion; + $oExt->aModuleInfo[$sModuleName] = [ + ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath, + ]; + return $oExt; } } From c3c2135ecc1a338f2e063ea602cfe5d6caa57a6d Mon Sep 17 00:00:00 2001 From: odain Date: Mon, 12 Jan 2026 12:46:57 +0100 Subject: [PATCH 06/13] =?UTF-8?q?N=C2=B07629:=20log=20changed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sources/Service/InterfaceDiscovery/InterfaceDiscovery.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php b/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php index 95fec0f43..d21f6bcfd 100644 --- a/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php +++ b/sources/Service/InterfaceDiscovery/InterfaceDiscovery.php @@ -190,7 +190,7 @@ class InterfaceDiscovery } if (count($aAutoloaderErrors) > 0) { IssueLog::Debug( - __METHOD__." cannot load some of the autoloader files", + __METHOD__." cannot load some of the autoloader files: missing or corrupted", LogChannels::CORE, ['autoloader_errors' => $aAutoloaderErrors] ); From 693e40b9c7753eb95ccc6b1cb9b8e6160ac83420 Mon Sep 17 00:00:00 2001 From: odain Date: Mon, 12 Jan 2026 13:27:40 +0100 Subject: [PATCH 07/13] enhance test sdk: add GetTemporaryFilePath --- .../php-unit-tests/src/BaseTestCase/ItopTestCase.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index b533f77c9..86a955020 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -698,4 +698,16 @@ abstract class ItopTestCase extends KernelTestCase return $this->CallUrl($sUrl, $aPostFields, $aCurlOptions, $bXDebugEnabled); } + + /** + * Return a temporary file path. that will be cleaned up by tearDown() + * @return string: temporary file path + */ + public function GetTemporaryFilePath(string $sPrefix = "test"): string + { + + $sPath = tempnam(sys_get_temp_dir(), $sPrefix); + $this->aFileToClean[] = $sPath; + return $sPath; + } } From d647d92acfd7c68f5d39d1bf052c2724baab1808 Mon Sep 17 00:00:00 2001 From: odain Date: Mon, 12 Jan 2026 13:28:05 +0100 Subject: [PATCH 08/13] ci: test enhancement => use GetTemporaryFilePath --- .../setup/ModuleDiscoveryTest.php | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php index 5252c4b50..75a318a68 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php @@ -90,10 +90,8 @@ TXT; { $aChoices = ['id1', 'id2']; - $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); - $this->aFileToClean[] = $sModuleFilePath; - $sModuleFilePath2 = tempnam(sys_get_temp_dir(), 'discovery_test'); - $this->aFileToClean[] = $sModuleFilePath2; + $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); + $sModuleFilePath2 = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); $aModules = [ "id1/1" => [ @@ -163,38 +161,35 @@ TXT; public function testIsModulePartOfRemovedExtension_ModuleWithAnotherVersionIncludedInRemoveExtension() { - $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); - $this->ValidateIsModulePartOfRemovedExtension('module_name', '123', $sModuleFilePath, $sModuleFilePath, false); + $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); + $this->AssertModuleIsPartOfRemovedExtension('module_name', '123', $sModuleFilePath, $sModuleFilePath, false); } public function testIsModulePartOfRemovedExtension_AnotherModuleWithSameVersionIncludedInRemoveExtension() { - $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); - $this->ValidateIsModulePartOfRemovedExtension('another_module_name', '456', $sModuleFilePath, $sModuleFilePath, false); + $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); + $this->AssertModuleIsPartOfRemovedExtension('another_module_name', '456', $sModuleFilePath, $sModuleFilePath, false); } public function testIsModulePartOfRemovedExtension_SameExtensionComingFromAnotherLocation() { - $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); - $sModuleFilePath2 = tempnam(sys_get_temp_dir(), 'discovery_test'); - $this->ValidateIsModulePartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath2, false); + $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); + $sModuleFilePath2 = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); + $this->AssertModuleIsPartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath2, false); } public function testIsModulePartOfRemovedExtension_ModuleShouldBeExcluded() { $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); - $this->ValidateIsModulePartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath, true); + $this->AssertModuleIsPartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath, true); } - public function ValidateIsModulePartOfRemovedExtension($sModuleName, $sModuleVersion, $sModuleFilePath1, $sModuleFilePath2, $bExpected) + public function AssertModuleIsPartOfRemovedExtension($sModuleName, $sModuleVersion, $sModuleFilePath, $sModuleFilePathIncludedInRemovedExtension, $bExpected) { - $this->aFileToClean[] = $sModuleFilePath1; - $this->aFileToClean[] = $sModuleFilePath2; - - $oExtension = $this->CreateExtensionWithModule('module_name', '456', $sModuleFilePath2); + $oExtension = $this->CreateExtensionWithModule('module_name', '456', $sModuleFilePathIncludedInRemovedExtension); ModuleDiscovery::DeclareRemovedExtensions([$oExtension]); $aCurrentModuleInfo = [ - ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath1, + ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath, ]; $this->assertEquals( $bExpected, From 7df59427cb1c59c90e8982b8e4c733f68fd273de Mon Sep 17 00:00:00 2001 From: odain Date: Mon, 12 Jan 2026 13:29:42 +0100 Subject: [PATCH 09/13] =?UTF-8?q?N=C2=B08764=20=20halt=20setup=20wizard=20?= =?UTF-8?q?at=20data=20issue=20-=20review=20-=202=20types=20of=20SetupAudi?= =?UTF-8?q?t=20constructors=20-=20setup=20wizard=20new=20step=20management?= =?UTF-8?q?=20enhancement=20-=20change=20SetupAudit=20GetIssue=20API=20beh?= =?UTF-8?q?aviour?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/applicationinstaller.class.inc.php | 69 +++++----- setup/feature_removal/AbstractSetupAudit.php | 123 ++++++++++++++++++ setup/feature_removal/InplaceSetupAudit.php | 40 ++++++ setup/feature_removal/SetupAudit.php | 97 ++------------ .../setup/feature_removal/SetupAuditTest.php | 23 ++-- 5 files changed, 220 insertions(+), 132 deletions(-) create mode 100644 setup/feature_removal/AbstractSetupAudit.php create mode 100644 setup/feature_removal/InplaceSetupAudit.php diff --git a/setup/applicationinstaller.class.inc.php b/setup/applicationinstaller.class.inc.php index 24f22144f..5e05767bd 100644 --- a/setup/applicationinstaller.class.inc.php +++ b/setup/applicationinstaller.class.inc.php @@ -17,13 +17,13 @@ // You should have received a copy of the GNU Affero General Public License // along with iTop. If not, see +use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit; use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer; -use Combodo\iTop\Setup\FeatureRemoval\SetupAudit; require_once(APPROOT.'setup/parameters.class.inc.php'); require_once(APPROOT.'setup/xmldataloader.class.inc.php'); require_once(APPROOT.'setup/backup.class.inc.php'); -require_once APPROOT.'setup/feature_removal/SetupAudit.php'; +require_once APPROOT.'setup/feature_removal/InplaceSetupAudit.php'; /** * The base class for the installation process. @@ -262,7 +262,7 @@ class ApplicationInstaller $sExtensionDir = $this->oParams->Get('extensions_dir', 'extensions'); $aMiscOptions = $this->oParams->Get('options', []); $aRemovedExtensionCodes = $this->oParams->Get('removed_extensions', []); - $sDisableDataAudit = $this->oParams->Get('disable-data-audit', ''); + $sSkipDataAudit = $this->oParams->Get('skip-data-audit', ''); $bUseSymbolicLinks = null; if ((isset($aMiscOptions['symlinks']) && $aMiscOptions['symlinks'])) { @@ -274,27 +274,36 @@ class ApplicationInstaller } } + $aParamValues = $this->oParams->GetParamForConfigArray(); + $bIsSetupDataAuditEnabled = $this->IsSetupDataAuditEnabled($sSkipDataAudit, $aParamValues); $this->DoCompile( $aRemovedExtensionCodes, $aSelectedModules, $sSourceDir, $sExtensionDir, - $sDisableDataAudit, + $bIsSetupDataAuditEnabled, $bUseSymbolicLinks ); + if ($bIsSetupDataAuditEnabled) { + $sNextStep = 'setup-audit'; + $sNextStepLabel = 'Checking data consistency with the new data model'; + } else { + $sNextStep = 'db-schema'; + $sNextStepLabel = 'Updating database schema'; + } + $aResult = [ 'status' => self::OK, 'message' => '', - 'next-step' => 'setup-audit', - 'next-step-label' => 'Checking data consistency with the new data model', + 'next-step' => $sNextStep, + 'next-step-label' => $sNextStepLabel, 'percentage-completed' => 40, ]; break; case 'setup-audit': - $sDisableDataAudit = $this->oParams->Get('disable-data-audit', ''); - $this->DoSetupAudit($sDisableDataAudit); + $this->DoSetupAudit(); $aResult = [ 'status' => self::OK, 'message' => '', @@ -499,7 +508,7 @@ class ApplicationInstaller * @param array $aSelectedModules * @param string $sSourceDir * @param string $sExtensionDir - * @param string $sDisableDataAudit + * @param bool $bIsSetupDataAuditEnabled * @param boolean $bUseSymbolicLinks * * @return void @@ -508,7 +517,7 @@ class ApplicationInstaller * * @since 3.1.0 N°2013 added the aParamValues param */ - protected function DoCompile($aRemovedExtensionCodes, $aSelectedModules, $sSourceDir, $sExtensionDir, $sDisableDataAudit, $bUseSymbolicLinks = null) + protected function DoCompile($aRemovedExtensionCodes, $aSelectedModules, $sSourceDir, $sExtensionDir, $bIsSetupDataAuditEnabled, $bUseSymbolicLinks = null) { /** * @since 3.2.0 move the ContextTag init at the very beginning of the method @@ -549,7 +558,7 @@ class ApplicationInstaller } $bIsAlreadyInMaintenanceMode = SetupUtils::IsInMaintenanceMode(); - if ($this->IsSetupDataAuditEnabled($sDisableDataAudit, $aParamValues)) { + if ($bIsSetupDataAuditEnabled) { if ($bIsAlreadyInMaintenanceMode) { //required to read DM before calling SaveModelInfo SetupUtils::ExitMaintenanceMode(); @@ -650,21 +659,21 @@ class ApplicationInstaller } } - private function GetModelInfoPath(): string + private function GetModelInfoPath(string $sEnv): string { - return APPROOT.'data/beforecompilation_modelinfo.json'; + return APPROOT."data/beforecompilation_".$sEnv."_modelinfo.json"; } private function SaveModelInfo(string $sEnvironment): void { $aModelInfo = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($sEnvironment); - $sModelInfoPath = $this->GetModelInfoPath(); + $sModelInfoPath = $this->GetModelInfoPath($sEnvironment); file_put_contents($sModelInfoPath, json_encode($aModelInfo)); } private function GetPreviousModelInfo(string $sEnvironment): array { - $sContent = file_get_contents($this->GetModelInfoPath()); + $sContent = file_get_contents($this->GetModelInfoPath($sEnvironment)); $aModelInfo = json_decode($sContent, true); if (false === $aModelInfo) { @@ -674,7 +683,7 @@ class ApplicationInstaller return $aModelInfo; } - protected function DoSetupAudit(string $sDisableDataAudit) + protected function DoSetupAudit() { /** * @since 3.2.0 move the ContextTag init at the very beginning of the method @@ -682,35 +691,29 @@ class ApplicationInstaller */ $oContextTag = new ContextTag(ContextTag::TAG_SETUP); - $aParamValues = $this->oParams->GetParamForConfigArray(); - if (! $this->IsSetupDataAuditEnabled($sDisableDataAudit, $aParamValues)) { - return; - } - $sTargetEnvironment = $this->GetTargetEnv(); $aPreviousCompilationModelInfo = $this->GetPreviousModelInfo($sTargetEnvironment); - $oSetupAudit = new SetupAudit($sTargetEnvironment, $sTargetEnvironment); - $oSetupAudit->ComputeClasses($aPreviousCompilationModelInfo); + $oSetupAudit = new InplaceSetupAudit($aPreviousCompilationModelInfo, $sTargetEnvironment); - try { - $oSetupAudit->GetIssues(true); - } catch (Exception $e) { - $iCount = $oSetupAudit->GetLastComputedFinalClassesRemovedCount(); + $oSetupAudit->GetIssues(true); + $iCount = $oSetupAudit->GetDataToCleanupCount(); + + if ($iCount > 0) { throw new Exception("$iCount elements require data adjustments or cleanup in the backoffice prior to upgrading iTop"); } } - private function IsSetupDataAuditEnabled($sDisableDataAudit, array $aParamValues): bool + private function IsSetupDataAuditEnabled($sSkipDataAudit, array $aParamValues): bool { - $sMode = $aParamValues['mode']; - if ($sMode !== "upgrade") { - //first install + if ($sSkipDataAudit === "checked") { + SetupLog::Info("Setup data audit disabled", null, ['skip-data-audit' => $sSkipDataAudit]); return false; } - if ($sDisableDataAudit === "checked") { - SetupLog::Info("Setup data audit disabled", null, ['disable-data-audit' => $sDisableDataAudit]); + $sMode = $aParamValues['mode']; + if ($sMode !== "upgrade") { + //first install return false; } diff --git a/setup/feature_removal/AbstractSetupAudit.php b/setup/feature_removal/AbstractSetupAudit.php new file mode 100644 index 000000000..0e606620f --- /dev/null +++ b/setup/feature_removal/AbstractSetupAudit.php @@ -0,0 +1,123 @@ +ComputeClasses(); + + if (count($this->aRemovedClasses) == 0) { + if (count($this->aClassesBeforeRemoval) == 0) { + return $this->aRemovedClasses; + } + + if (count($this->aClassesAfterRemoval) == 0) { + return $this->aRemovedClasses; + } + + $aExtensionsNames = array_diff($this->aClassesBeforeRemoval, $this->aClassesAfterRemoval); + $this->aRemovedClasses = []; + $aClasses = array_values($aExtensionsNames); + sort($aClasses); + + foreach ($aClasses as $i => $sClass) { + $this->aRemovedClasses[] = $sClass; + } + } + + return $this->aRemovedClasses; + } + + /** test only: return file path that force audit error being raised + * + * @return string + */ + public static function GetErrorMessageFilePathForTestOnly(): string + { + return APPROOT."/data/".self::GETISSUE_ERROR_MSG_FILE_FORTESTONLY; + } + + public function GetIssues(bool $bStopDataCheckAtFirstIssue = false): array + { + $sErrorMessageFilePath = self::GetErrorMessageFilePathForTestOnly(); + if ($bStopDataCheckAtFirstIssue && is_file($sErrorMessageFilePath)) { + $sMsg = file_get_contents($sErrorMessageFilePath); + throw new \Exception($sMsg); + } + + $this->aFinalClassesToCleanup = []; + + foreach ($this->GetRemovedClasses() as $sClass) { + if (MetaModel::IsAbstract($sClass)) { + continue; + } + + if (!MetaModel::IsStandaloneClass($sClass)) { + $iCount = $this->Count($sClass); + $this->aFinalClassesToCleanup[$sClass] = $iCount; + if ($bStopDataCheckAtFirstIssue && $iCount > 0) { + //setup envt: should raise issue ASAP + $this->LogInfoWithProperLogger("Setup audit found data to cleanup", null, $this->aFinalClassesToCleanup); + return $this->aFinalClassesToCleanup; + } + } + } + + $this->LogInfoWithProperLogger("Setup audit found data to cleanup", null, ['data_to_cleanup' => $this->aFinalClassesToCleanup]); + return $this->aFinalClassesToCleanup; + } + + public function GetDataToCleanupCount(): int + { + $res = 0; + foreach ($this->aFinalClassesToCleanup as $sClass => $iCount) { + $res += $iCount; + } + return $res; + } + + private function Count($sClass): int + { + $oSearch = DBObjectSearch::FromOQL("SELECT $sClass", []); + $oSearch->AllowAllData(); + $oSet = new DBObjectSet($oSearch); + + return $oSet->Count(); + } + + //could be shared with others in log APIs ? + private function LogInfoWithProperLogger($sMessage, $sChannel = null, $aContext = []): void + { + if (ContextTag::Check(ContextTag::TAG_SETUP)) { + SetupLog::Info($sMessage, $sChannel, $aContext); + } else { + IssueLog::Info($sMessage, $sChannel, $aContext); + } + } +} diff --git a/setup/feature_removal/InplaceSetupAudit.php b/setup/feature_removal/InplaceSetupAudit.php new file mode 100644 index 000000000..a552d452b --- /dev/null +++ b/setup/feature_removal/InplaceSetupAudit.php @@ -0,0 +1,40 @@ +aClassesBeforeRemoval = $aClassesBeforeRemoval; + $this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; + } + + public function ComputeClasses(): void + { + if ($this->bClassesInitialized) { + return; + } + + $sCurrentEnvt = MetaModel::GetEnvironment(); + + if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { + $this->aClassesAfterRemoval = MetaModel::GetClasses(); + } else { + $this->aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfterExtensionRemoval); + } + + $this->bClassesInitialized = true; + } +} diff --git a/setup/feature_removal/SetupAudit.php b/setup/feature_removal/SetupAudit.php index 0b70a13bc..d2caabd12 100644 --- a/setup/feature_removal/SetupAudit.php +++ b/setup/feature_removal/SetupAudit.php @@ -2,16 +2,12 @@ namespace Combodo\iTop\Setup\FeatureRemoval; -use ContextTag; -use DBObjectSearch; -use DBObjectSet; -use IssueLog; use MetaModel; -use SetupLog; +require_once __DIR__.'/AbstractSetupAudit.php'; require_once APPROOT.'setup/feature_removal/ModelReflectionSerializer.php'; -class SetupAudit +class SetupAudit extends AbstractSetupAudit { //file used when present to trigger audit exception when testing specific setups public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; @@ -19,34 +15,24 @@ class SetupAudit private string $sEnvBeforeExtensionRemoval; private string $sEnvAfterExtensionRemoval; - private bool $bClassesInitialized = false; - private array $aClassesBeforeRemoval = []; - private array $aClassesAfterRemoval = []; - private array $aRemovedClasses = []; - private array $aFinalClassesRemoved = []; - - public function __construct(string $sEnvBeforeExtensionRemoval, string $sEnvAfterExtensionRemoval = DryRemovalRuntimeEnvironment::DRY_REMOVAL_AUDIT_ENV) + public function __construct(string $sEnvBeforeExtensionRemoval, string $sEnvAfterExtensionRemoval) { + parent::__construct(); $this->sEnvBeforeExtensionRemoval = $sEnvBeforeExtensionRemoval; $this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; } - public function ComputeClasses(array $aClassesBeforeRemoval = null) + public function ComputeClasses(): void { if ($this->bClassesInitialized) { return; } $sCurrentEnvt = MetaModel::GetEnvironment(); - - if (is_null($aClassesBeforeRemoval)) { - if ($sCurrentEnvt === $this->sEnvBeforeExtensionRemoval) { - $this->aClassesBeforeRemoval = MetaModel::GetClasses(); - } else { - $this->aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBeforeExtensionRemoval); - } + if ($sCurrentEnvt === $this->sEnvBeforeExtensionRemoval) { + $this->aClassesBeforeRemoval = MetaModel::GetClasses(); } else { - $this->aClassesBeforeRemoval = $aClassesBeforeRemoval; + $this->aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBeforeExtensionRemoval); } if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { @@ -94,71 +80,4 @@ class SetupAudit return $this->aRemovedClasses; } - - /** test only: return file path that force audit error being raised - * - * @return string - */ - public static function GetErrorMessageFilePathForTestOnly(): string - { - return APPROOT."/data/".self::GETISSUE_ERROR_MSG_FILE_FORTESTONLY; - } - - public function GetIssues(bool $bThrowExceptionAtFirstIssue = false): array - { - $sErrorMessageFilePath = self::GetErrorMessageFilePathForTestOnly(); - if ($bThrowExceptionAtFirstIssue && is_file($sErrorMessageFilePath)) { - $sMsg = file_get_contents($sErrorMessageFilePath); - throw new \Exception($sMsg); - } - - $this->aFinalClassesRemoved = []; - - foreach ($this->GetRemovedClasses() as $sClass) { - if (MetaModel::IsAbstract($sClass)) { - continue; - } - - if (!MetaModel::IsStandaloneClass($sClass)) { - $iCount = $this->Count($sClass); - $this->aFinalClassesRemoved[$sClass] = $iCount; - if ($bThrowExceptionAtFirstIssue && $iCount > 0) { - //setup envt: should raise issue ASAP - $this->LogInfoWithProperLogger("Setup audit found data to cleanup", null, $this->aFinalClassesRemoved); - throw new \Exception($sClass); - } - } - } - - $this->LogInfoWithProperLogger("Setup audit found data to cleanup", null, $this->aFinalClassesRemoved); - return $this->aFinalClassesRemoved; - } - - public function GetLastComputedFinalClassesRemovedCount(): int - { - $res = 0; - foreach ($this->aFinalClassesRemoved as $sClass => $iCount) { - $res += $iCount; - } - return $res; - } - - private function Count($sClass): int - { - $oSearch = DBObjectSearch::FromOQL("SELECT $sClass", []); - $oSearch->AllowAllData(); - $oSet = new DBObjectSet($oSearch); - - return $oSet->Count(); - } - - //could be shared with others in log APIs ? - private function LogInfoWithProperLogger($sMessage, $sChannel = null, $aContext = []): void - { - if (ContextTag::Check(ContextTag::TAG_SETUP)) { - SetupLog::Info($sMessage, $sChannel, $aContext); - } else { - IssueLog::Info($sMessage, $sChannel, $aContext); - } - } } diff --git a/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php b/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php index 68707358c..384f72304 100644 --- a/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php @@ -5,6 +5,7 @@ namespace Combodo\iTop\Test\UnitTest\Setup\FeatureRemoval; use Combodo\iTop\Setup\FeatureRemoval\DryRemovalRuntimeEnvironment; use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer; use Combodo\iTop\Setup\FeatureRemoval\SetupAudit; +use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit; use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase; use Combodo\iTop\Test\UnitTest\Service\UnitTestRunTimeEnvironment; use Exception; @@ -40,6 +41,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase parent::setUp(); $this->RequireOnceItopFile('/setup/feature_removal/SetupAudit.php'); + $this->RequireOnceItopFile('/setup/feature_removal/InplaceSetupAudit.php'); $this->RequireOnceItopFile('/setup/feature_removal/DryRemovalRuntimeEnvironment.php'); } @@ -54,7 +56,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase $oDryRemovalRuntimeEnvt->Prepare($this->GetTestEnvironment(), ['nominal_ext1', 'finalclass_ext2']); $oDryRemovalRuntimeEnvt->CompileFrom($this->GetTestEnvironment()); - $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment()); + $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment(), DryRemovalRuntimeEnvironment::DRY_REMOVAL_AUDIT_ENV); $expected = [ "Feature1Module1MyClass", @@ -73,11 +75,11 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase { $sEnv = MetaModel::GetEnvironment(); - $aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($sEnv); - $aClassesAfterRemoval[] = "GabuZomeu"; + $aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($sEnv); + $aClassesBeforeRemoval[] = "GabuZomeu"; - $oSetupAudit = new SetupAudit($sEnv, $sEnv); - $oSetupAudit->ComputeClasses($aClassesAfterRemoval); + $oSetupAudit = new InplaceSetupAudit($aClassesBeforeRemoval, $sEnv); + $oSetupAudit->ComputeClasses($aClassesBeforeRemoval); $this->assertEquals(["GabuZomeu"], $oSetupAudit->GetRemovedClasses()); } @@ -87,7 +89,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase $oOrg = $this->CreateOrganization($sUID); $this->createObject('FinalClassFeature1Module1MyFinalClassFromLocation', ['org_id' => $oOrg->GetKey(), 'name' => $sUID, 'name2' => uniqid()]); - $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment()); + $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment(), DryRemovalRuntimeEnvironment::DRY_REMOVAL_AUDIT_ENV); $aRemovedClasses = [ "Feature1Module1MyClass", "FinalClassFeature1Module1MyClass", @@ -113,7 +115,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase $this->createObject('FinalClassFeature1Module1MyFinalClassFromLocation', ['org_id' => $oOrg->GetKey(), 'name' => $sUID, 'name2' => uniqid()]); $this->createObject('FinalClassFeature2Module1MyFinalClassFromLocation', ['org_id' => $oOrg->GetKey(), 'name' => $sUID, 'name2' => uniqid()]); - $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment()); + $oSetupAudit = new SetupAudit(MetaModel::GetEnvironment(), DryRemovalRuntimeEnvironment::DRY_REMOVAL_AUDIT_ENV); $aRemovedClasses = [ "Feature1Module1MyClass", "FinalClassFeature1Module1MyClass", @@ -125,8 +127,9 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase //avoid setup dry computation $this->SetNonPublicProperty($oSetupAudit, 'aRemovedClasses', $aRemovedClasses); - $this->expectException(Exception::class); - $this->expectExceptionMessage('FinalClassFeature1Module1MyFinalClassFromLocation'); - $oSetupAudit->GetIssues(true); + $expected = [ + "FinalClassFeature1Module1MyFinalClassFromLocation" => 1, + ]; + $this->assertEqualsCanonicalizing($expected, $oSetupAudit->GetIssues(true)); } } From 1e15eb1161ebd298648871c07e77a25d89e86358 Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 13 Jan 2026 15:21:57 +0100 Subject: [PATCH 10/13] =?UTF-8?q?N=C2=B08764=20=20halt=20setup=20wizard=20?= =?UTF-8?q?at=20data=20issue=20-=202nd=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- setup/feature_removal/AbstractSetupAudit.php | 28 ++--------- setup/feature_removal/InplaceSetupAudit.php | 14 +++--- setup/feature_removal/SetupAudit.php | 28 +++++------ setup/modulediscovery.class.inc.php | 21 +++++--- .../src/BaseTestCase/ItopTestCase.php | 7 +-- .../setup/ModuleDiscoveryTest.php | 49 ++++++++++--------- .../setup/feature_removal/SetupAuditTest.php | 5 +- 7 files changed, 71 insertions(+), 81 deletions(-) diff --git a/setup/feature_removal/AbstractSetupAudit.php b/setup/feature_removal/AbstractSetupAudit.php index 0e606620f..abe6d142c 100644 --- a/setup/feature_removal/AbstractSetupAudit.php +++ b/setup/feature_removal/AbstractSetupAudit.php @@ -13,12 +13,9 @@ require_once APPROOT.'setup/feature_removal/ModelReflectionSerializer.php'; abstract class AbstractSetupAudit { - //file used when present to trigger audit exception when testing specific setups - public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; - protected bool $bClassesInitialized = false; - protected array $aClassesBeforeRemoval = []; - protected array $aClassesAfterRemoval = []; + protected array $aClassesBefore = []; + protected array $aClassesAfter = []; protected array $aRemovedClasses = []; protected array $aFinalClassesToCleanup = []; @@ -33,15 +30,15 @@ abstract class AbstractSetupAudit $this->ComputeClasses(); if (count($this->aRemovedClasses) == 0) { - if (count($this->aClassesBeforeRemoval) == 0) { + if (count($this->aClassesBefore) == 0) { return $this->aRemovedClasses; } - if (count($this->aClassesAfterRemoval) == 0) { + if (count($this->aClassesAfter) == 0) { return $this->aRemovedClasses; } - $aExtensionsNames = array_diff($this->aClassesBeforeRemoval, $this->aClassesAfterRemoval); + $aExtensionsNames = array_diff($this->aClassesBefore, $this->aClassesAfter); $this->aRemovedClasses = []; $aClasses = array_values($aExtensionsNames); sort($aClasses); @@ -54,23 +51,8 @@ abstract class AbstractSetupAudit return $this->aRemovedClasses; } - /** test only: return file path that force audit error being raised - * - * @return string - */ - public static function GetErrorMessageFilePathForTestOnly(): string - { - return APPROOT."/data/".self::GETISSUE_ERROR_MSG_FILE_FORTESTONLY; - } - public function GetIssues(bool $bStopDataCheckAtFirstIssue = false): array { - $sErrorMessageFilePath = self::GetErrorMessageFilePathForTestOnly(); - if ($bStopDataCheckAtFirstIssue && is_file($sErrorMessageFilePath)) { - $sMsg = file_get_contents($sErrorMessageFilePath); - throw new \Exception($sMsg); - } - $this->aFinalClassesToCleanup = []; foreach ($this->GetRemovedClasses() as $sClass) { diff --git a/setup/feature_removal/InplaceSetupAudit.php b/setup/feature_removal/InplaceSetupAudit.php index a552d452b..b49a46540 100644 --- a/setup/feature_removal/InplaceSetupAudit.php +++ b/setup/feature_removal/InplaceSetupAudit.php @@ -12,13 +12,13 @@ class InplaceSetupAudit extends AbstractSetupAudit //file used when present to trigger audit exception when testing specific setups public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; - private string $sEnvAfterExtensionRemoval; + private string $sEnvAfter; - public function __construct(array $aClassesBeforeRemoval, string $sEnvAfterExtensionRemoval) + public function __construct(array $aClassesBefore, string $sEnvAfter) { parent::__construct(); - $this->aClassesBeforeRemoval = $aClassesBeforeRemoval; - $this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; + $this->aClassesBefore = $aClassesBefore; + $this->sEnvAfter = $sEnvAfter; } public function ComputeClasses(): void @@ -29,10 +29,10 @@ class InplaceSetupAudit extends AbstractSetupAudit $sCurrentEnvt = MetaModel::GetEnvironment(); - if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { - $this->aClassesAfterRemoval = MetaModel::GetClasses(); + if ($sCurrentEnvt === $this->sEnvAfter) { + $this->aClassesAfter = MetaModel::GetClasses(); } else { - $this->aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfterExtensionRemoval); + $this->aClassesAfter = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfter); } $this->bClassesInitialized = true; diff --git a/setup/feature_removal/SetupAudit.php b/setup/feature_removal/SetupAudit.php index d2caabd12..ebad99527 100644 --- a/setup/feature_removal/SetupAudit.php +++ b/setup/feature_removal/SetupAudit.php @@ -12,14 +12,14 @@ class SetupAudit extends AbstractSetupAudit //file used when present to trigger audit exception when testing specific setups public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; - private string $sEnvBeforeExtensionRemoval; - private string $sEnvAfterExtensionRemoval; + private string $sEnvBefore; + private string $sEnvAfter; - public function __construct(string $sEnvBeforeExtensionRemoval, string $sEnvAfterExtensionRemoval) + public function __construct(string $sEnvBefore, string $sEnvAfter) { parent::__construct(); - $this->sEnvBeforeExtensionRemoval = $sEnvBeforeExtensionRemoval; - $this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; + $this->sEnvBefore = $sEnvBefore; + $this->sEnvAfter = $sEnvAfter; } public function ComputeClasses(): void @@ -29,16 +29,16 @@ class SetupAudit extends AbstractSetupAudit } $sCurrentEnvt = MetaModel::GetEnvironment(); - if ($sCurrentEnvt === $this->sEnvBeforeExtensionRemoval) { - $this->aClassesBeforeRemoval = MetaModel::GetClasses(); + if ($sCurrentEnvt === $this->sEnvBefore) { + $this->aClassesBefore = MetaModel::GetClasses(); } else { - $this->aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBeforeExtensionRemoval); + $this->aClassesBefore = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBefore); } - if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { - $this->aClassesAfterRemoval = MetaModel::GetClasses(); + if ($sCurrentEnvt === $this->sEnvAfter) { + $this->aClassesAfter = MetaModel::GetClasses(); } else { - $this->aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfterExtensionRemoval); + $this->aClassesAfter = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfter); } $this->bClassesInitialized = true; @@ -60,15 +60,15 @@ class SetupAudit extends AbstractSetupAudit $this->ComputeClasses(); if (count($this->aRemovedClasses) == 0) { - if (count($this->aClassesBeforeRemoval) == 0) { + if (count($this->aClassesBefore) == 0) { return $this->aRemovedClasses; } - if (count($this->aClassesAfterRemoval) == 0) { + if (count($this->aClassesAfter) == 0) { return $this->aRemovedClasses; } - $aExtensionsNames = array_diff($this->aClassesBeforeRemoval, $this->aClassesAfterRemoval); + $aExtensionsNames = array_diff($this->aClassesBefore, $this->aClassesAfter); $this->aRemovedClasses = []; $aClasses = array_values($aExtensionsNames); sort($aClasses); diff --git a/setup/modulediscovery.class.inc.php b/setup/modulediscovery.class.inc.php index 4acd00318..964457432 100755 --- a/setup/modulediscovery.class.inc.php +++ b/setup/modulediscovery.class.inc.php @@ -22,6 +22,7 @@ use Combodo\iTop\PhpParser\Evaluation\PhpExpressionEvaluator; use Combodo\iTop\Setup\ModuleDependency\Module; +use Combodo\iTop\Setup\ModuleDependency\ModuleDependencySort; use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReader; use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReaderException; @@ -29,8 +30,6 @@ require_once(APPROOT.'setup/modulediscovery/ModuleFileReader.php'); require_once(__DIR__.'/moduledependency/moduledependencysort.class.inc.php'); require_once(__DIR__.'/itopextension.class.inc.php'); -use Combodo\iTop\Setup\ModuleDependency\ModuleDependencySort; - class MissingDependencyException extends CoreException { /** @@ -135,7 +134,7 @@ class ModuleDiscovery list($sModuleName, $sModuleVersion) = static::GetModuleName($sId); - if (self::IsModulePartOfRemovedExtension($sModuleName, $sModuleVersion, $aArgs)) { + if (self::IsModuleInExtensionList(self::$m_aRemovedExtensions, $sModuleName, $sModuleVersion, $aArgs)) { return; } @@ -230,7 +229,7 @@ class ModuleDiscovery $oModule = new Module($sModuleId); $sModuleName = $oModule->GetModuleName(); - if (self::IsModulePartOfRemovedExtension($sModuleName, $oModule->GetVersion(), $aModuleInfo)) { + if (self::IsModuleInExtensionList(self::$m_aRemovedExtensions, $sModuleName, $oModule->GetVersion(), $aModuleInfo)) { continue; } @@ -254,16 +253,24 @@ class ModuleDiscovery self::$m_aRemovedExtensions = $aRemovedExtension; } - private static function IsModulePartOfRemovedExtension(string $sModuleName, string $sModuleVersion, array $aModuleInfo): bool + /** + * @param array<\iTopExtension> $aExtensions + * @param string $sModuleName + * @param string $sModuleVersion + * @param array $aModuleInfo + * + * @return bool + */ + private static function IsModuleInExtensionList(array $aExtensions, string $sModuleName, string $sModuleVersion, array $aModuleInfo): bool { - if (count(self::$m_aRemovedExtensions) === 0) { + if (count($aExtensions) === 0) { return false; } $aNonMatchingPaths = []; $sModuleFilePath = $aModuleInfo[ModuleFileReader::MODULE_FILE_PATH]; /** @var \iTopExtension $oExtension */ - foreach (self::$m_aRemovedExtensions as $oExtension) { + foreach ($aExtensions as $oExtension) { $sCurrentVersion = $oExtension->aModuleVersion[$sModuleName] ?? null; if (is_null($sCurrentVersion)) { continue; diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index 86a955020..f81f67073 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -701,11 +701,12 @@ abstract class ItopTestCase extends KernelTestCase /** * Return a temporary file path. that will be cleaned up by tearDown() - * @return string: temporary file path + * + * @return string: temporary file path: file prefix include phpunit test method name */ - public function GetTemporaryFilePath(string $sPrefix = "test"): string + public function GetTemporaryFilePath(): string { - + $sPrefix = $this->getName(false); $sPath = tempnam(sys_get_temp_dir(), $sPrefix); $this->aFileToClean[] = $sPath; return $sPath; diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php index 75a318a68..a3b223f29 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php @@ -90,8 +90,8 @@ TXT; { $aChoices = ['id1', 'id2']; - $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $sModuleFilePath2 = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $sModuleFilePath2 = $this->GetTemporaryFilePath(); $aModules = [ "id1/1" => [ @@ -106,7 +106,7 @@ TXT; ], ]; - $oExtension = $this->CreateExtensionWithModule('id2', '2', $sModuleFilePath2); + $oExtension = $this->GivenExtensionWithModule('id2', '2', $sModuleFilePath2); ModuleDiscovery::DeclareRemovedExtensions([$oExtension]); $sExpectedMessage = <<assertEquals([$expectedName, $expectedVersion], ModuleDiscovery::GetModuleName($sModuleId)); } - public function testIsModulePartOfRemovedExtension_NoRemovedExtension() + public function testIsModuleInExtensionList_NoRemovedExtension() { - ModuleDiscovery::DeclareRemovedExtensions([]); - $this->assertFalse($this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModulePartOfRemovedExtension", ['module_name', '123', []])); + $this->assertFalse($this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModuleInExtensionList", [[], 'module_name', '123', []])); } - public function testIsModulePartOfRemovedExtension_ModuleWithAnotherVersionIncludedInRemoveExtension() + public function testIsModuleInExtensionList_ModuleWithAnotherVersionIncludedInRemoveExtension() { - $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $this->AssertModuleIsPartOfRemovedExtension('module_name', '123', $sModuleFilePath, $sModuleFilePath, false); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)]; + $this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '123', $sModuleFilePath, false); } - public function testIsModulePartOfRemovedExtension_AnotherModuleWithSameVersionIncludedInRemoveExtension() + public function testIsModuleInExtensionList_AnotherModuleWithSameVersionIncludedInRemoveExtension() { - $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $this->AssertModuleIsPartOfRemovedExtension('another_module_name', '456', $sModuleFilePath, $sModuleFilePath, false); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)]; + $this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'another_module_name', '456', $sModuleFilePath, false); } - public function testIsModulePartOfRemovedExtension_SameExtensionComingFromAnotherLocation() + public function testIsModuleInExtensionList_SameExtensionComingFromAnotherLocation() { - $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $sModuleFilePath2 = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $this->AssertModuleIsPartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath2, false); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $sModuleFilePath2 = $this->GetTemporaryFilePath(); + $aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath2)]; + $this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '456', $sModuleFilePath, false); } - public function testIsModulePartOfRemovedExtension_ModuleShouldBeExcluded() + public function testIsModuleInExtensionList_ModuleShouldBeExcluded() { - $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); - $this->AssertModuleIsPartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath, true); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)]; + $this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '456', $sModuleFilePath, true); } - public function AssertModuleIsPartOfRemovedExtension($sModuleName, $sModuleVersion, $sModuleFilePath, $sModuleFilePathIncludedInRemovedExtension, $bExpected) + public function AssertModuleIsPartOfRemovedExtension($aExtensionList, $sModuleName, $sModuleVersion, $sModuleFilePath, $bExpected) { - $oExtension = $this->CreateExtensionWithModule('module_name', '456', $sModuleFilePathIncludedInRemovedExtension); - ModuleDiscovery::DeclareRemovedExtensions([$oExtension]); $aCurrentModuleInfo = [ ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath, ]; $this->assertEquals( $bExpected, - $this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModulePartOfRemovedExtension", [$sModuleName, $sModuleVersion, $aCurrentModuleInfo]) + $this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModuleInExtensionList", [$aExtensionList, $sModuleName, $sModuleVersion, $aCurrentModuleInfo]) ); } - private function CreateExtensionWithModule(string $sModuleName, string $sVersion, bool|string $sModuleFilePath): iTopExtension + private function GivenExtensionWithModule(string $sModuleName, string $sVersion, bool|string $sModuleFilePath): iTopExtension { $oExt = new iTopExtension(); $oExt->aModuleVersion[$sModuleName] = $sVersion; diff --git a/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php b/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php index 384f72304..4fe001494 100644 --- a/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php @@ -3,12 +3,11 @@ namespace Combodo\iTop\Test\UnitTest\Setup\FeatureRemoval; use Combodo\iTop\Setup\FeatureRemoval\DryRemovalRuntimeEnvironment; +use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit; use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer; use Combodo\iTop\Setup\FeatureRemoval\SetupAudit; -use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit; use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase; use Combodo\iTop\Test\UnitTest\Service\UnitTestRunTimeEnvironment; -use Exception; use MetaModel; class SetupAuditTest extends ItopCustomDatamodelTestCase @@ -79,7 +78,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase $aClassesBeforeRemoval[] = "GabuZomeu"; $oSetupAudit = new InplaceSetupAudit($aClassesBeforeRemoval, $sEnv); - $oSetupAudit->ComputeClasses($aClassesBeforeRemoval); + $oSetupAudit->ComputeClasses(); $this->assertEquals(["GabuZomeu"], $oSetupAudit->GetRemovedClasses()); } From 8d2e0761e0fb05986db973cc1cea3a93db1d67b2 Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 13 Jan 2026 15:52:32 +0100 Subject: [PATCH 11/13] setup: phpstan level 0 --- setup/ModuleInstallationRepository.php | 16 +++++++++++++--- setup/ajax.dataloader.php | 4 ++-- setup/email.test.php | 8 ++++---- .../ModelReflectionSerializer.php | 4 ++-- .../dependencyexpression.class.inc.php | 2 +- .../moduledependencysort.class.inc.php | 8 ++++---- setup/modulediscovery/ModuleFileReader.php | 16 ++++++++-------- setup/runtimeenv.class.inc.php | 5 ++--- setup/setuputils.class.inc.php | 2 +- setup/wizardcontroller.class.inc.php | 13 ------------- 10 files changed, 37 insertions(+), 41 deletions(-) diff --git a/setup/ModuleInstallationRepository.php b/setup/ModuleInstallationRepository.php index f98011dd5..63bd984d4 100644 --- a/setup/ModuleInstallationRepository.php +++ b/setup/ModuleInstallationRepository.php @@ -95,8 +95,17 @@ SQL; $aSelectInstall = CMDBSource::QueryToArray($sSQLQuery); } catch (MySQLException $e) { // No database or erroneous information - $this->log_error('Can not connect to the database: host: '.$oConfig->Get('db_host').', user:'.$oConfig->Get('db_user').', pwd:'.$oConfig->Get('db_pwd').', db name:'.$oConfig->Get('db_name')); - $this->log_error('Exception '.$e->getMessage()); + SetupLog::Error( + 'Can not connect to the database', + null, + [ + 'host' => $oConfig->Get('db_host'), + 'user' => $oConfig->Get('db_user'), + 'pwd:' => $oConfig->Get('db_pwd'), + 'db name' => $oConfig->Get('db_name'), + 'msg' => $e->getMessage(), + ] + ); return false; } @@ -129,7 +138,8 @@ SQL; // so assume that the datamodel version is equal to the application version $aResult['datamodel_version'] = $aResult['product_version']; } - $this->log_info("GetApplicationVersion returns: product_name: ".$aResult['product_name'].', product_version: '.$aResult['product_version']); + + SetupLog::Info(__METHOD__, null, ["product_name" => $aResult['product_name'], "product_version" => $aResult['product_version']]); return empty($aResult) ? false : $aResult; } diff --git a/setup/ajax.dataloader.php b/setup/ajax.dataloader.php index 3e9f498c1..50dae5701 100644 --- a/setup/ajax.dataloader.php +++ b/setup/ajax.dataloader.php @@ -127,7 +127,7 @@ header("Expires: Fri, 17 Jul 1970 05:00:00 GMT"); // Date in the past /** * Main program */ -$sOperation = Utils::ReadParam('operation', ''); +$sOperation = utils::ReadParam('operation', ''); try { SetupUtils::CheckSetupToken(); @@ -164,7 +164,7 @@ try { break; case 'toggle_use_symbolic_links': - $sUseSymbolicLinks = Utils::ReadParam('bUseSymbolicLinks', false); + $sUseSymbolicLinks = utils::ReadParam('bUseSymbolicLinks', false); $bUseSymbolicLinks = ($sUseSymbolicLinks === 'true'); MFCompiler::SetUseSymbolicLinksFlag($bUseSymbolicLinks); echo "toggle useSymbolicLinks flag : $bUseSymbolicLinks"; diff --git a/setup/email.test.php b/setup/email.test.php index 184a2124a..e5df5a275 100644 --- a/setup/email.test.php +++ b/setup/email.test.php @@ -34,7 +34,7 @@ require_once(APPROOT.'/application/startup.inc.php'); require_once(APPROOT.'/application/loginwebpage.class.inc.php'); LoginWebPage::DoLogin(true); // Check user rights and prompt if needed (must be admin) -$sOperation = Utils::ReadParam('operation', 'step1'); +$sOperation = utils::ReadParam('operation', 'step1'); $oP = new SetupPage('iTop email test utility'); // Although this page doesn't expose sensitive info, with it we can send multiple emails @@ -208,7 +208,7 @@ function DisplayStep2(SetupPage $oP, $sFrom, $sTo) $oP->add("

Sending an email to '".htmlentities($sTo, ENT_QUOTES, 'utf-8')."'... (From: '".htmlentities($sFrom, ENT_QUOTES, 'utf-8')."')

\n"); $oP->add("
\n"); - $oEmail = new Email(); + $oEmail = new EMail(); $oEmail->SetRecipientTO($sTo); $oEmail->SetRecipientFrom($sFrom); $oEmail->SetSubject("Test iTop"); @@ -256,8 +256,8 @@ try { case 'step2': $oP->no_cache(); - $sTo = Utils::ReadParam('to', '', false, 'raw_data'); - $sFrom = Utils::ReadParam('from', '', false, 'raw_data'); + $sTo = utils::ReadParam('to', '', false, 'raw_data'); + $sFrom = utils::ReadParam('from', '', false, 'raw_data'); DisplayStep2($oP, $sFrom, $sTo); break; diff --git a/setup/feature_removal/ModelReflectionSerializer.php b/setup/feature_removal/ModelReflectionSerializer.php index 2468c963b..ba4f8bc51 100644 --- a/setup/feature_removal/ModelReflectionSerializer.php +++ b/setup/feature_removal/ModelReflectionSerializer.php @@ -7,7 +7,7 @@ use CoreException; use Exception; use IssueLog; use SetupLog; -use Utils; +use utils; class ModelReflectionSerializer { @@ -34,7 +34,7 @@ class ModelReflectionSerializer public function GetModelFromEnvironment(string $sEnv): array { IssueLog::Info(__METHOD__, null, ['env' => $sEnv]); - $sPHPExec = trim(Utils::GetConfig()->Get('php_path')); + $sPHPExec = trim(utils::GetConfig()->Get('php_path')); $sOutput = ""; $iRes = 0; diff --git a/setup/moduledependency/dependencyexpression.class.inc.php b/setup/moduledependency/dependencyexpression.class.inc.php index e0d166c95..90537df35 100644 --- a/setup/moduledependency/dependencyexpression.class.inc.php +++ b/setup/moduledependency/dependencyexpression.class.inc.php @@ -5,7 +5,7 @@ namespace Combodo\iTop\Setup\ModuleDependency; require_once(APPROOT.'/setup/runtimeenv.class.inc.php'); use Combodo\iTop\PhpParser\Evaluation\PhpExpressionEvaluator; -use ModuleFileReaderException; +use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReaderException; use RunTimeEnvironment; /** diff --git a/setup/moduledependency/moduledependencysort.class.inc.php b/setup/moduledependency/moduledependencysort.class.inc.php index 099167c8a..65c81a056 100644 --- a/setup/moduledependency/moduledependencysort.class.inc.php +++ b/setup/moduledependency/moduledependencysort.class.inc.php @@ -19,16 +19,16 @@ class ModuleDependencySort final public static function GetInstance(): ModuleDependencySort { - if (!isset(static::$oInstance)) { - static::$oInstance = new static(); + if (!isset(self::$oInstance)) { + self::$oInstance = new ModuleDependencySort(); } - return static::$oInstance; + return self::$oInstance; } final public static function SetInstance(?ModuleDependencySort $oInstance): void { - static::$oInstance = $oInstance; + self::$oInstance = $oInstance; } /** diff --git a/setup/modulediscovery/ModuleFileReader.php b/setup/modulediscovery/ModuleFileReader.php index 048388000..b13360fa8 100644 --- a/setup/modulediscovery/ModuleFileReader.php +++ b/setup/modulediscovery/ModuleFileReader.php @@ -7,15 +7,15 @@ use CoreException; use Exception; use ParseError; use PhpParser\Error; +use PhpParser\Node\Arg; +use PhpParser\Node\Expr\Array_; +use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\ElseIf_; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\If_; use PhpParser\ParserFactory; -use PhpParser\Node\Expr\Assign; -use PhpParser\Node\Stmt\ElseIf_; -use PhpParser\Node\Expr\Array_; -use PhpParser\Node\Scalar\String_; -use PhpParser\Node\Arg; require_once __DIR__.'/ModuleFileReaderException.php'; require_once APPROOT.'sources/PhpParser/Evaluation/PhpExpressionEvaluator.php'; @@ -49,11 +49,11 @@ class ModuleFileReader final public static function GetInstance(): ModuleFileReader { - if (!isset(static::$oInstance)) { - static::$oInstance = new static(); + if (!isset(self::$oInstance)) { + self::$oInstance = new ModuleFileReader(); } - return static::$oInstance; + return self::$oInstance; } final public static function SetInstance(?ModuleFileReader $oInstance): void diff --git a/setup/runtimeenv.class.inc.php b/setup/runtimeenv.class.inc.php index 1464fd058..6147a64f7 100644 --- a/setup/runtimeenv.class.inc.php +++ b/setup/runtimeenv.class.inc.php @@ -126,9 +126,8 @@ class RunTimeEnvironment * from the given file * @param $oConfig object The configuration (volatile, not necessarily already on disk) * @param $bModelOnly boolean Whether or not to allow loading a data model with no corresponding DB - * @return none */ - public function InitDataModel($oConfig, $bModelOnly = true, $bUseCache = false) + public function InitDataModel($oConfig, $bModelOnly = true, $bUseCache = false): void { require_once APPROOT.'/setup/moduleinstallation.class.inc.php'; @@ -896,7 +895,7 @@ class RunTimeEnvironment try { call_user_func_array($aCallSpec, $aArgs); } catch (Exception $e) { - $sModuleId = isset($sModuleId) ? $sModuleId : ""; + $sModuleId = $aModuleInfo[ModuleFileReader::MODULE_INFO_ID] ?? ""; $sErrorMessage = "Module $sModuleId : error when calling module installer class $sModuleInstallerClass for $sHandlerName handler"; $aExceptionContextData = [ 'ModulelId' => $sModuleId, diff --git a/setup/setuputils.class.inc.php b/setup/setuputils.class.inc.php index e88befbe8..3706d3bca 100644 --- a/setup/setuputils.class.inc.php +++ b/setup/setuputils.class.inc.php @@ -599,7 +599,7 @@ class SetupUtils // create and test destination location // $sDestDir = dirname($sDBBackupPath); - setuputils::builddir($sDestDir); + SetupUtils::builddir($sDestDir); if (!is_dir($sDestDir)) { $aResult[] = new CheckResult(CheckResult::ERROR, "$sDestDir does not exist and could not be created."); } diff --git a/setup/wizardcontroller.class.inc.php b/setup/wizardcontroller.class.inc.php index 589c5c8a3..bd2cfa8f2 100644 --- a/setup/wizardcontroller.class.inc.php +++ b/setup/wizardcontroller.class.inc.php @@ -16,7 +16,6 @@ // // You should have received a copy of the GNU Affero General Public License // along with iTop. If not, see -use Combodo\iTop\Application\UI\Base\Component\Html\Html; use Combodo\iTop\Application\WebPage\WebPage; /** @@ -402,18 +401,6 @@ abstract class WizardStep * @return void */ abstract public function Display(WebPage $oPage); - /** - * Displays the wizard page for the current class/state - * return UIBlock - * The name of the input fields (and their id if one is supplied) MUST NOT start with "_" - * (this is reserved for the wizard's own parameters) - * @return \Combodo\iTop\Application\UI\Base\UIBlock - * @since 3.0.0 - */ - public function DisplayBlock(WebPage $oPage) - { - return new Html($this->Display($oPage)); - } /** * Processes the page's parameters and (if moving forward) returns the next step/state to be displayed From 13c18b611cf323c2e0397b135abbedfd38d6b737 Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 13 Jan 2026 16:11:34 +0100 Subject: [PATCH 12/13] setup: phpstan level 1 --- setup/ModuleInstallationRepository.php | 3 ++- setup/compiler.class.inc.php | 4 +++- setup/moduleinstaller.class.inc.php | 1 + setup/parameters.class.inc.php | 4 +++- setup/runtimeenv.class.inc.php | 10 ++++---- setup/setuputils.class.inc.php | 33 ++++++++++---------------- setup/xmldataloader.class.inc.php | 1 + 7 files changed, 28 insertions(+), 28 deletions(-) diff --git a/setup/ModuleInstallationRepository.php b/setup/ModuleInstallationRepository.php index 63bd984d4..1eeaa73fc 100644 --- a/setup/ModuleInstallationRepository.php +++ b/setup/ModuleInstallationRepository.php @@ -140,7 +140,8 @@ SQL; } SetupLog::Info(__METHOD__, null, ["product_name" => $aResult['product_name'], "product_version" => $aResult['product_version']]); - return empty($aResult) ? false : $aResult; + + return count($aResult) == 0 ? false : $aResult; } private function ComputeInstalledModules(array $aSelectInstall): array diff --git a/setup/compiler.class.inc.php b/setup/compiler.class.inc.php index 38a86d45c..edbc37214 100644 --- a/setup/compiler.class.inc.php +++ b/setup/compiler.class.inc.php @@ -21,8 +21,8 @@ use Combodo\iTop\Application\Branding; use Combodo\iTop\Application\WebPage\iTopWebPage; use Combodo\iTop\Application\WebPage\Page; -use Combodo\iTop\DesignElement; use Combodo\iTop\DesignDocument; +use Combodo\iTop\DesignElement; require_once(APPROOT.'setup/setuputils.class.inc.php'); require_once(APPROOT.'setup/modelfactory.class.inc.php'); @@ -3359,6 +3359,8 @@ EOF; $bDataXmlPrecompiledFileExists = false; clearstatcache(); + + $iDataXmlFileLastModified = 0; if (!empty($sPrecompiledFileUri)) { $sDataXmlProvidedPrecompiledFile = $sTempTargetDir.DIRECTORY_SEPARATOR.$sPrecompiledFileUri; $bDataXmlPrecompiledFileExists = file_exists($sDataXmlProvidedPrecompiledFile) ; diff --git a/setup/moduleinstaller.class.inc.php b/setup/moduleinstaller.class.inc.php index 0815fd4ba..a560141b3 100644 --- a/setup/moduleinstaller.class.inc.php +++ b/setup/moduleinstaller.class.inc.php @@ -130,6 +130,7 @@ abstract class ModuleInstallerAPI if (in_array($sTo, $aNewValues)) { $sEnumCol = $oAttDef->Get("sql"); $aFields = CMDBSource::QueryToArray("SHOW COLUMNS FROM `$sTableName` WHERE Field = '$sEnumCol'"); + $aCurrentValues = []; if (isset($aFields[0]['Type'])) { $sColType = $aFields[0]['Type']; // Note: the parsing should rely on str_getcsv (requires PHP 5.3) to cope with escaped string diff --git a/setup/parameters.class.inc.php b/setup/parameters.class.inc.php index bb22e2889..3426341c8 100644 --- a/setup/parameters.class.inc.php +++ b/setup/parameters.class.inc.php @@ -107,7 +107,9 @@ class PHPParameters extends Parameters { if ($this->aData == null) { require_once($sParametersFile); - $this->aData = $ITOP_PARAMS; // Defined in the file loaded just above + if (isset($ITOP_PARAMS)) { + $this->aData = $ITOP_PARAMS; // Defined in the file loaded just above + } } } } diff --git a/setup/runtimeenv.class.inc.php b/setup/runtimeenv.class.inc.php index 6147a64f7..7cb5ae692 100644 --- a/setup/runtimeenv.class.inc.php +++ b/setup/runtimeenv.class.inc.php @@ -347,6 +347,7 @@ class RunTimeEnvironment // $oFactory = new ModelFactory($sSourceDirFull); $aModulesToCompile = $this->GetMFModulesToCompile($sSourceEnv, $sSourceDir); + $oModule = null; foreach ($aModulesToCompile as $oModule) { if ($oModule instanceof MFDeltaModule) { // Just before loading the delta, let's save an image of the datamodel @@ -356,7 +357,7 @@ class RunTimeEnvironment $oFactory->LoadModule($oModule); } - if ($oModule instanceof MFDeltaModule) { + if (!is_null($oModule) && ($oModule instanceof MFDeltaModule)) { // A delta was loaded, let's save a second copy of the datamodel $oFactory->SaveToFile(utils::GetDataPath().'datamodel-'.$this->sTargetEnv.'-with-delta.xml'); } else { @@ -667,7 +668,8 @@ class RunTimeEnvironment $aResult['datamodel_version'] = $aResult['product_version']; } $this->log_info("GetApplicationVersion returns: product_name: ".$aResult['product_name'].', product_version: '.$aResult['product_version']); - return empty($aResult) ? false : $aResult; + + return count($aResult) == 0 ? false : $aResult; } public static function MakeDirSafe($sDir) @@ -963,7 +965,7 @@ class RunTimeEnvironment foreach ($aPreviouslyLoadedFiles as $sFileRelativePath) { $sFileName = APPROOT.$sFileRelativePath; SetupLog::Info("Loading file: $sFileName (just to get the keys mapping)"); - if (empty($sFileName) || !file_exists($sFileName)) { + if (!file_exists($sFileName)) { throw(new Exception("File $sFileName does not exist")); } @@ -975,7 +977,7 @@ class RunTimeEnvironment foreach ($aFiles as $sFileRelativePath) { $sFileName = APPROOT.$sFileRelativePath; SetupLog::Info("Loading file: $sFileName"); - if (empty($sFileName) || !file_exists($sFileName)) { + if (!file_exists($sFileName)) { throw(new Exception("File $sFileName does not exist")); } diff --git a/setup/setuputils.class.inc.php b/setup/setuputils.class.inc.php index 3706d3bca..6bce7ba82 100644 --- a/setup/setuputils.class.inc.php +++ b/setup/setuputils.class.inc.php @@ -254,32 +254,23 @@ class SetupUtils if (!utils::IsModeCLI()) { $sUploadTmpDir = self::GetUploadTmpDir(); - if (empty($sUploadTmpDir)) { - $sUploadTmpDir = '/tmp'; - $aResult[] = new CheckResult( - CheckResult::WARNING, - "Temporary directory for files upload is not defined (upload_tmp_dir), assuming that $sUploadTmpDir is used." - ); - } // check that the upload directory is indeed writable from PHP - if (!empty($sUploadTmpDir)) { - if (!file_exists($sUploadTmpDir)) { + if (!file_exists($sUploadTmpDir)) { + $aResult[] = new CheckResult( + CheckResult::ERROR, + "Temporary directory for files upload ($sUploadTmpDir) does not exist or cannot be read by PHP." + ); + } else { + if (!is_writable($sUploadTmpDir)) { $aResult[] = new CheckResult( CheckResult::ERROR, - "Temporary directory for files upload ($sUploadTmpDir) does not exist or cannot be read by PHP." + "Temporary directory for files upload ($sUploadTmpDir) is not writable." ); } else { - if (!is_writable($sUploadTmpDir)) { - $aResult[] = new CheckResult( - CheckResult::ERROR, - "Temporary directory for files upload ($sUploadTmpDir) is not writable." - ); - } else { - $aResult[] = new CheckResult( - CheckResult::TRACE, - "Info - Temporary directory for files upload ($sUploadTmpDir) is writable." - ); - } + $aResult[] = new CheckResult( + CheckResult::TRACE, + "Info - Temporary directory for files upload ($sUploadTmpDir) is writable." + ); } } } diff --git a/setup/xmldataloader.class.inc.php b/setup/xmldataloader.class.inc.php index 01085a30e..fb53e2e76 100644 --- a/setup/xmldataloader.class.inc.php +++ b/setup/xmldataloader.class.inc.php @@ -353,6 +353,7 @@ class XMLDataLoader foreach ($oObjList as $oTargetObj) { $bChanged = false; $sClass = get_class($oTargetObj); + $iExtKey = -1; foreach (MetaModel::ListAttributeDefs($sClass) as $sAttCode => $oAttDef) { if (($oAttDef->IsExternalKey()) && ($oTargetObj->Get($sAttCode) < 0)) { // Convention unresolved key = negative $sTargetClass = $oAttDef->GetTargetClass(); From 0582ae1038269cd55635386cbcddeff3335ef9d9 Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 13 Jan 2026 16:29:16 +0100 Subject: [PATCH 13/13] setup: phpstan level 2 setup: phpstan level 2 --- setup/AnalyzeInstallation.php | 3 +-- setup/ModuleInstallationRepository.php | 6 +++--- .../dependencyexpression.class.inc.php | 8 ++++---- setup/moduledependency/module.class.inc.php | 2 +- .../moduledependencysort.class.inc.php | 2 +- setup/modulediscovery.class.inc.php | 14 ++++++++------ setup/modulediscovery/ModuleFileReader.php | 12 ++++++++---- .../unattended-install/InstallationFileService.php | 4 ++-- setup/xmldataloader.class.inc.php | 2 ++ 9 files changed, 30 insertions(+), 23 deletions(-) diff --git a/setup/AnalyzeInstallation.php b/setup/AnalyzeInstallation.php index 1335c9aaf..a1947b7df 100644 --- a/setup/AnalyzeInstallation.php +++ b/setup/AnalyzeInstallation.php @@ -6,7 +6,6 @@ class AnalyzeInstallation { private static AnalyzeInstallation $oInstance; private ?array $aAvailableModules = null; - private ?array $aSelectInstall = null; protected function __construct() { @@ -23,7 +22,7 @@ class AnalyzeInstallation final public static function SetInstance(?AnalyzeInstallation $oInstance): void { - static::$oInstance = $oInstance; + self::$oInstance = $oInstance; } /** diff --git a/setup/ModuleInstallationRepository.php b/setup/ModuleInstallationRepository.php index 1eeaa73fc..149899171 100644 --- a/setup/ModuleInstallationRepository.php +++ b/setup/ModuleInstallationRepository.php @@ -19,7 +19,7 @@ class ModuleInstallationRepository final public static function SetInstance(?ModuleInstallationRepository $oInstance): void { - static::$oInstance = $oInstance; + self::$oInstance = $oInstance; } private ?array $aSelectInstall = null; @@ -205,8 +205,8 @@ SQL; $oSet->SetLimit($iOffset + 1); $iParentId = 0; - /** @var \DBObject $oModuleInstallation */ - while ($oModuleInstallation = $oSet->Fetch()) { + while (!is_null($oModuleInstallation = $oSet->Fetch())) { + /** @var \DBObject $oModuleInstallation */ if ($iOffset == 0) { $iParentId = $oModuleInstallation->Get('id'); break; diff --git a/setup/moduledependency/dependencyexpression.class.inc.php b/setup/moduledependency/dependencyexpression.class.inc.php index 90537df35..89988fb92 100644 --- a/setup/moduledependency/dependencyexpression.class.inc.php +++ b/setup/moduledependency/dependencyexpression.class.inc.php @@ -63,17 +63,17 @@ class DependencyExpression private static function GetPhpExpressionEvaluator(): PhpExpressionEvaluator { - if (!isset(static::$oPhpExpressionEvaluator)) { - static::$oPhpExpressionEvaluator = new PhpExpressionEvaluator([], RunTimeEnvironment::STATIC_CALL_AUTOSELECT_WHITELIST); + if (!isset(self::$oPhpExpressionEvaluator)) { + self::$oPhpExpressionEvaluator = new PhpExpressionEvaluator([], RunTimeEnvironment::STATIC_CALL_AUTOSELECT_WHITELIST); } - return static::$oPhpExpressionEvaluator; + return self::$oPhpExpressionEvaluator; } /** * Return module names potentially required by current dependency * - * @return array + * @return array */ public function GetRemainingModuleNamesToResolve(): array { diff --git a/setup/moduledependency/module.class.inc.php b/setup/moduledependency/module.class.inc.php index 734c081e5..133021208 100644 --- a/setup/moduledependency/module.class.inc.php +++ b/setup/moduledependency/module.class.inc.php @@ -114,7 +114,7 @@ class Module } /** - * @return array: list of unique module names + * @return array list of unique module names */ public function GetUnresolvedDependencyModuleNames(): array { diff --git a/setup/moduledependency/moduledependencysort.class.inc.php b/setup/moduledependency/moduledependencysort.class.inc.php index 65c81a056..3fa3b5234 100644 --- a/setup/moduledependency/moduledependencysort.class.inc.php +++ b/setup/moduledependency/moduledependencysort.class.inc.php @@ -168,7 +168,7 @@ class ModuleDependencySort foreach ($aCountDepsByModuleId as $sModuleId => $iInDegreeCounter) { $oModule = $aUnresolvedDependencyModules[$sModuleId]; - if ($bOneLoopAtLeast && $iInDegreeCounter > 0) { + if ($bOneLoopAtLeast && ($iInDegreeCounter > 0)) { break; } diff --git a/setup/modulediscovery.class.inc.php b/setup/modulediscovery.class.inc.php index 964457432..b3da7fdbd 100755 --- a/setup/modulediscovery.class.inc.php +++ b/setup/modulediscovery.class.inc.php @@ -95,7 +95,7 @@ class ModuleDiscovery protected static $m_aModules = []; protected static $m_aModuleVersionByName = []; - /** @var array<\iTopExtension $m_aRemovedExtensions */ + /** @var array<\iTopExtension> $m_aRemovedExtensions */ protected static $m_aRemovedExtensions = []; // All the entries below are list of file paths relative to the module directory @@ -307,11 +307,11 @@ class ModuleDiscovery private static function GetPhpExpressionEvaluator(): PhpExpressionEvaluator { - if (!isset(static::$oPhpExpressionEvaluator)) { - static::$oPhpExpressionEvaluator = new PhpExpressionEvaluator([], RunTimeEnvironment::STATIC_CALL_AUTOSELECT_WHITELIST); + if (!isset(self::$oPhpExpressionEvaluator)) { + self::$oPhpExpressionEvaluator = new PhpExpressionEvaluator([], RunTimeEnvironment::STATIC_CALL_AUTOSELECT_WHITELIST); } - return static::$oPhpExpressionEvaluator; + return self::$oPhpExpressionEvaluator; } /** @@ -360,10 +360,12 @@ class ModuleDiscovery /** * Helper function to interpret the name of a module + * * @param $sModuleId string Identifier of the module, in the form 'name/version' - * @return array(name, version) + * + * @return array of 2 elements (name, version) */ - public static function GetModuleName($sModuleId) + public static function GetModuleName($sModuleId): array { $aMatches = []; if (preg_match('!^(.*)/(.*)$!', $sModuleId, $aMatches)) { diff --git a/setup/modulediscovery/ModuleFileReader.php b/setup/modulediscovery/ModuleFileReader.php index b13360fa8..6b9064914 100644 --- a/setup/modulediscovery/ModuleFileReader.php +++ b/setup/modulediscovery/ModuleFileReader.php @@ -58,12 +58,14 @@ class ModuleFileReader final public static function SetInstance(?ModuleFileReader $oInstance): void { - static::$oInstance = $oInstance; + self::$oInstance = $oInstance; } /** * Read the information from a module file (module.xxx.php) - * @param string $sModuleFile + * + * @param string $sModuleFilePath + * * @return array * @throws ModuleFileReaderException */ @@ -109,7 +111,9 @@ class ModuleFileReader * Read the information from a module file (module.xxx.php) * Warning: this method is using eval() function to load the ModuleInstallerAPI classes. * Current method is never called at design/runtime. It is acceptable to use it during setup only. - * @param string $sModuleFile + * + * @param string $sModuleFilePath + * * @return array * @throws ModuleFileReaderException */ @@ -199,7 +203,7 @@ class ModuleFileReader /** * @param string $sModuleFilePath - * @param \PhpParser\Node\Expr\Assign $oAssignation + * @param \PhpParser\Node\Stmt\Expression $oExpression * * @return array|null * @throws ModuleFileReaderException diff --git a/setup/unattended-install/InstallationFileService.php b/setup/unattended-install/InstallationFileService.php index ad15466fa..e1d2494b0 100644 --- a/setup/unattended-install/InstallationFileService.php +++ b/setup/unattended-install/InstallationFileService.php @@ -71,12 +71,12 @@ class InstallationFileService return $this->aAfterComputationSelectedExtensions; } - public function SetItopExtensionsMap(ItopExtensionsMap $oItopExtensionsMap): void + public function SetItopExtensionsMap(iTopExtensionsMap $oItopExtensionsMap): void { $this->oItopExtensionsMap = $oItopExtensionsMap; } - public function GetItopExtensionsMap(): ItopExtensionsMap + public function GetItopExtensionsMap(): iTopExtensionsMap { if (is_null($this->oItopExtensionsMap)) { $this->oItopExtensionsMap = new iTopExtensionsMap($this->sTargetEnvironment); diff --git a/setup/xmldataloader.class.inc.php b/setup/xmldataloader.class.inc.php index fb53e2e76..7e06d0e66 100644 --- a/setup/xmldataloader.class.inc.php +++ b/setup/xmldataloader.class.inc.php @@ -230,6 +230,7 @@ class XMLDataLoader } else { $iDstObj = (int)($oSubNode); // Attempt to find the object in the list of loaded objects + /** @var \Combodo\iTop\Core\AttributeDefinition\AttributeExternalKey $oAttDef */ $iExtKey = $this->GetObjectKey($oAttDef->GetTargetClass(), $iDstObj); if ($iExtKey == 0) { $iExtKey = -$iDstObj; // Convention: Unresolved keys are stored as negative ! @@ -356,6 +357,7 @@ class XMLDataLoader $iExtKey = -1; foreach (MetaModel::ListAttributeDefs($sClass) as $sAttCode => $oAttDef) { if (($oAttDef->IsExternalKey()) && ($oTargetObj->Get($sAttCode) < 0)) { // Convention unresolved key = negative + /** @var \Combodo\iTop\Core\AttributeDefinition\AttributeExternalKey $oAttDef */ $sTargetClass = $oAttDef->GetTargetClass(); $iTempKey = $oTargetObj->Get($sAttCode);