Merge branch 'feature/8981-prepare' into feature/uninstallation

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;
 	}
 
 	/**

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;
@@ -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,8 +138,10 @@ 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']);
-		return empty($aResult) ? false : $aResult;
+
+		SetupLog::Info(__METHOD__, null, ["product_name" => $aResult['product_name'], "product_version" => $aResult['product_version']]);
+
+		return count($aResult) == 0 ? false : $aResult;
 	}
 
 	private function ComputeInstalledModules(array $aSelectInstall): array
@@ -194,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;

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";

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 <http://www.gnu.org/licenses/>
 
+use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit;
+use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer;
+
 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/InplaceSetupAudit.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', []);
+					$sSkipDataAudit = $this->oParams->Get('skip-data-audit', '');
 
 					$bUseSymbolicLinks = null;
 					if ((isset($aMiscOptions['symlinks']) && $aMiscOptions['symlinks'])) {
@@ -269,34 +274,50 @@ class ApplicationInstaller
 						}
 					}
 
+					$aParamValues = $this->oParams->GetParamForConfigArray();
+					$bIsSetupDataAuditEnabled = $this->IsSetupDataAuditEnabled($sSkipDataAudit, $aParamValues);
 					$this->DoCompile(
 						$aRemovedExtensionCodes,
 						$aSelectedModules,
 						$sSourceDir,
 						$sExtensionDir,
+						$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' => $sNextStep,
+						'next-step-label' => $sNextStepLabel,
+						'percentage-completed' => 40,
+					];
+					break;
+
+				case 'setup-audit':
+					$this->DoSetupAudit();
 					$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 +508,7 @@ class ApplicationInstaller
 	 * @param array $aSelectedModules
 	 * @param string $sSourceDir
 	 * @param string $sExtensionDir
+	 * @param bool $bIsSetupDataAuditEnabled
 	 * @param boolean $bUseSymbolicLinks
 	 *
 	 * @return void
@@ -495,8 +517,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, $bIsSetupDataAuditEnabled, $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 +556,18 @@ 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 ($bIsSetupDataAuditEnabled) {
+			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 +659,79 @@ class ApplicationInstaller
 		}
 	}
 
+	private function GetModelInfoPath(string $sEnv): string
+	{
+		return APPROOT."data/beforecompilation_".$sEnv."_modelinfo.json";
+	}
+
+	private function SaveModelInfo(string $sEnvironment): void
+	{
+		$aModelInfo = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($sEnvironment);
+		$sModelInfoPath = $this->GetModelInfoPath($sEnvironment);
+		file_put_contents($sModelInfoPath, json_encode($aModelInfo));
+	}
+
+	private function GetPreviousModelInfo(string $sEnvironment): array
+	{
+		$sContent = file_get_contents($this->GetModelInfoPath($sEnvironment));
+		$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()
+	{
+		/**
+		 * @since 3.2.0 move the ContextTag init at the very beginning of the method
+		 * @noinspection PhpUnusedLocalVariableInspection
+		 */
+		$oContextTag = new ContextTag(ContextTag::TAG_SETUP);
+
+		$sTargetEnvironment = $this->GetTargetEnv();
+		$aPreviousCompilationModelInfo = $this->GetPreviousModelInfo($sTargetEnvironment);
+
+		$oSetupAudit = new InplaceSetupAudit($aPreviousCompilationModelInfo, $sTargetEnvironment);
+
+		$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($sSkipDataAudit, array $aParamValues): bool
+	{
+		if ($sSkipDataAudit === "checked") {
+			SetupLog::Info("Setup data audit disabled", null, ['skip-data-audit' => $sSkipDataAudit]);
+			return false;
+		}
+
+		$sMode = $aParamValues['mode'];
+		if ($sMode !== "upgrade") {
+			//first install
+			return false;
+		}
+
+		return true;
+	}
+
 	/**
 	 * @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

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) ;

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("<p>Sending an email to '".htmlentities($sTo, ENT_QUOTES, 'utf-8')."'... (From: '".htmlentities($sFrom, ENT_QUOTES, 'utf-8')."')</p>\n");
 	$oP->add("<form method=\"post\">\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;
 

setup/feature_removal/AbstractSetupAudit.php

@@ -0,0 +1,105 @@
+<?php
+
+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';
+
+abstract class AbstractSetupAudit
+{
+	protected bool $bClassesInitialized = false;
+	protected array $aClassesBefore = [];
+	protected array $aClassesAfter = [];
+	protected array $aRemovedClasses = [];
+	protected array $aFinalClassesToCleanup = [];
+
+	public function __construct()
+	{
+	}
+
+	abstract public function ComputeClasses(): void;
+
+	public function GetRemovedClasses(): array
+	{
+		$this->ComputeClasses();
+
+		if (count($this->aRemovedClasses) == 0) {
+			if (count($this->aClassesBefore) == 0) {
+				return $this->aRemovedClasses;
+			}
+
+			if (count($this->aClassesAfter) == 0) {
+				return $this->aRemovedClasses;
+			}
+
+			$aExtensionsNames = array_diff($this->aClassesBefore, $this->aClassesAfter);
+			$this->aRemovedClasses = [];
+			$aClasses = array_values($aExtensionsNames);
+			sort($aClasses);
+
+			foreach ($aClasses as $i => $sClass) {
+				$this->aRemovedClasses[] = $sClass;
+			}
+		}
+
+		return $this->aRemovedClasses;
+	}
+
+	public function GetIssues(bool $bStopDataCheckAtFirstIssue = false): array
+	{
+		$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);
+		}
+	}
+}

setup/feature_removal/InplaceSetupAudit.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace Combodo\iTop\Setup\FeatureRemoval;
+
+use MetaModel;
+
+require_once __DIR__.'/AbstractSetupAudit.php';
+require_once APPROOT.'setup/feature_removal/ModelReflectionSerializer.php';
+
+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 $sEnvAfter;
+
+	public function __construct(array $aClassesBefore, string $sEnvAfter)
+	{
+		parent::__construct();
+		$this->aClassesBefore = $aClassesBefore;
+		$this->sEnvAfter = $sEnvAfter;
+	}
+
+	public function ComputeClasses(): void
+	{
+		if ($this->bClassesInitialized) {
+			return;
+		}
+
+		$sCurrentEnvt = MetaModel::GetEnvironment();
+
+		if ($sCurrentEnvt === $this->sEnvAfter) {
+			$this->aClassesAfter = MetaModel::GetClasses();
+		} else {
+			$this->aClassesAfter = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfter);
+		}
+
+		$this->bClassesInitialized = true;
+	}
+}

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);
+		}
+	}
 }

setup/feature_removal/SetupAudit.php

@@ -2,45 +2,46 @@
 
 namespace Combodo\iTop\Setup\FeatureRemoval;
 
-use DBObjectSearch;
-use DBObjectSet;
 use MetaModel;
 
+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';
 
-	private string $sEnvBeforeExtensionRemoval;
-	private string $sEnvAfterExtensionRemoval;
+	private string $sEnvBefore;
+	private string $sEnvAfter;
 
-	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 $sEnvBefore, string $sEnvAfter)
 	{
-		$this->sEnvBeforeExtensionRemoval = $sEnvBeforeExtensionRemoval;
-		$this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval;
+		parent::__construct();
+		$this->sEnvBefore = $sEnvBefore;
+		$this->sEnvAfter = $sEnvAfter;
+	}
+
+	public function ComputeClasses(): void
+	{
+		if ($this->bClassesInitialized) {
+			return;
+		}
 
 		$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->aRemovedClasses = [];
-		$this->aFinalClassesRemoved = [];
+		$this->bClassesInitialized = true;
 	}
 
 	/*public function SetSelectedExtensions(Config $oConfig, array $aSelectedExtensions)
@@ -56,16 +57,18 @@ class SetupAudit
 
 	public function GetRemovedClasses(): array
 	{
+		$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);
@@ -77,50 +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
-					throw new \Exception($sClass);
-				}
-			}
-		}
-
-		return $this->aFinalClassesRemoved;
-	}
-
-	private function Count($sClass): int
-	{
-		$oSearch = DBObjectSearch::FromOQL("SELECT $sClass", []);
-		$oSearch->AllowAllData();
-		$oSet = new DBObjectSet($oSearch);
-
-		return $oSet->Count();
-	}
 }

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;
 
 /**
@@ -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<string>
 	 */
 	public function GetRemainingModuleNamesToResolve(): array
 	{

setup/moduledependency/module.class.inc.php

@@ -114,7 +114,7 @@ class Module
 	}
 
 	/**
-	 * @return array: list of unique module names
+	 * @return array<string> list of unique module names
 	 */
 	public function GetUnresolvedDependencyModuleNames(): array
 	{

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;
 	}
 
 	/**
@@ -168,7 +168,7 @@ class ModuleDependencySort
 			foreach ($aCountDepsByModuleId as $sModuleId => $iInDegreeCounter) {
 				$oModule = $aUnresolvedDependencyModules[$sModuleId];
 
-				if ($bOneLoopAtLeast && $iInDegreeCounter > 0) {
+				if ($bOneLoopAtLeast && ($iInDegreeCounter > 0)) {
 					break;
 				}
 

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
 {
 	/**
@@ -96,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
@@ -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;
 				}
 
@@ -239,7 +238,6 @@ class ModuleDiscovery
 				}
 			}
 		}
-
 		return ModuleDependencySort::GetInstance()->GetModulesOrderedForInstallation($aFilteredModules, $bAbortOnMissingDependency);
 	}
 
@@ -255,17 +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;
@@ -302,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;
 	}
 
 	/**
@@ -355,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)) {

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,21 +49,23 @@ 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
 	{
-		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

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

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
+			}
 		}
 	}
 }

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';
 
@@ -348,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
@@ -357,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 {
@@ -668,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)
@@ -896,7 +897,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,
@@ -964,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"));
 			}
 
@@ -976,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"));
 			}
 

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."
+					);
 				}
 			}
 		}
@@ -599,7 +590,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.");
 		}

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);

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 <http://www.gnu.org/licenses/>
-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

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();

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 !
@@ -353,8 +354,10 @@ 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
+						/** @var \Combodo\iTop\Core\AttributeDefinition\AttributeExternalKey $oAttDef */
 						$sTargetClass = $oAttDef->GetTargetClass();
 						$iTempKey = $oTargetObj->Get($sAttCode);
 

sources/Service/InterfaceDiscovery/InterfaceDiscovery.php

@@ -178,12 +178,19 @@ 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);
 			}
 			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]
 				);

tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php

@@ -698,4 +698,17 @@ 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: file prefix include phpunit test method name
+	 */
+	public function GetTemporaryFilePath(): string
+	{
+		$sPrefix = $this->getName(false);
+		$sPath = tempnam(sys_get_temp_dir(), $sPrefix);
+		$this->aFileToClean[] = $sPath;
+		return $sPath;
+	}
 }

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,39 @@ TXT;
 		ModuleDiscovery::OrderModulesByDependencies($aModules, true, $aChoices);
 	}
 
+	public function testOrderModulesByDependencies_FailWhenChoosenModuleDependsOnRemovedExtensionModule()
+	{
+		$aChoices = ['id1', 'id2'];
+
+		$sModuleFilePath = $this->GetTemporaryFilePath();
+		$sModuleFilePath2 = $this->GetTemporaryFilePath();
+
+		$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->GivenExtensionWithModule('id2', '2', $sModuleFilePath2);
+		ModuleDiscovery::DeclareRemovedExtensions([$oExtension]);
+
+		$sExpectedMessage = <<<TXT
+The following modules have unmet dependencies:
+label1 (id: id1/1) depends on: ❌ id2/2
+TXT;
+		$this->expectException(MissingDependencyException::class);
+		$this->expectExceptionMessage($sExpectedMessage);
+
+		ModuleDiscovery::OrderModulesByDependencies($aModules, true, $aChoices);
+	}
+
 	public function GetModuleNameProvider()
 	{
 		return [
@@ -109,6 +150,61 @@ TXT;
 	 */
 	public function testGetModuleName($sModuleId, $expectedName, $expectedVersion)
 	{
-		$this->assertEquals([$expectedName, $expectedVersion], \ModuleDiscovery::GetModuleName($sModuleId));
+		$this->assertEquals([$expectedName, $expectedVersion], ModuleDiscovery::GetModuleName($sModuleId));
+	}
+
+	public function testIsModuleInExtensionList_NoRemovedExtension()
+	{
+		$this->assertFalse($this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModuleInExtensionList", [[], 'module_name', '123', []]));
+	}
+
+	public function testIsModuleInExtensionList_ModuleWithAnotherVersionIncludedInRemoveExtension()
+	{
+		$sModuleFilePath = $this->GetTemporaryFilePath();
+		$aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)];
+		$this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '123', $sModuleFilePath, false);
+	}
+
+	public function testIsModuleInExtensionList_AnotherModuleWithSameVersionIncludedInRemoveExtension()
+	{
+		$sModuleFilePath = $this->GetTemporaryFilePath();
+		$aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)];
+		$this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'another_module_name', '456', $sModuleFilePath, false);
+	}
+
+	public function testIsModuleInExtensionList_SameExtensionComingFromAnotherLocation()
+	{
+		$sModuleFilePath = $this->GetTemporaryFilePath();
+		$sModuleFilePath2 = $this->GetTemporaryFilePath();
+		$aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath2)];
+		$this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '456', $sModuleFilePath, false);
+	}
+
+	public function testIsModuleInExtensionList_ModuleShouldBeExcluded()
+	{
+		$sModuleFilePath = $this->GetTemporaryFilePath();
+		$aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)];
+		$this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '456', $sModuleFilePath, true);
+	}
+
+	public function AssertModuleIsPartOfRemovedExtension($aExtensionList, $sModuleName, $sModuleVersion, $sModuleFilePath, $bExpected)
+	{
+		$aCurrentModuleInfo = [
+			ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath,
+		];
+		$this->assertEquals(
+			$bExpected,
+			$this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModuleInExtensionList", [$aExtensionList, $sModuleName, $sModuleVersion, $aCurrentModuleInfo])
+		);
+	}
+
+	private function GivenExtensionWithModule(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;
 	}
 }

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\InplaceSetupAudit;
+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
 {
@@ -38,6 +40,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');
 	}
 
@@ -52,7 +55,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",
@@ -67,13 +70,25 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase
 		$this->assertEqualsCanonicalizing($expected, $oSetupAudit->GetIssues());
 	}
 
+	public function testGetRemovedClassesFromSetupWizard()
+	{
+		$sEnv = MetaModel::GetEnvironment();
+
+		$aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($sEnv);
+		$aClassesBeforeRemoval[] = "GabuZomeu";
+
+		$oSetupAudit = new InplaceSetupAudit($aClassesBeforeRemoval, $sEnv);
+		$oSetupAudit->ComputeClasses();
+		$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(), DryRemovalRuntimeEnvironment::DRY_REMOVAL_AUDIT_ENV);
 		$aRemovedClasses = [
 			"Feature1Module1MyClass",
 			"FinalClassFeature1Module1MyClass",
@@ -99,7 +114,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",
@@ -111,8 +126,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));
 	}
 }