N°7371 - Simplify initial approach by using common API, and revert modification of unnecessary js/utils.js API

* N°9675 - raise first exception encountered when starting metamodel

* N°9675 - enhance setup data consistency error feedback

* N°9675 - raise PHP CLI version issue as well

* N°9675 - geptile PR fixes + enhance regexp for cli php version

* cleanup

* N°9675 - remove maintenance mode before any setup compilation

* N°9675 - share php cli check in SetupUtils

* N°9675 - exit maintenance only before audit

core/metamodel.class.php

@@ -5761,36 +5761,37 @@ abstract class MetaModel
 
 		self::$m_sEnvironment = $sEnvironment;
 
-		try {
-			if (!defined('MODULESROOT')) {
-				define('MODULESROOT', APPROOT.'env-'.self::$m_sEnvironment.'/');
+		if (!defined('MODULESROOT')) {
+			define('MODULESROOT', APPROOT.'env-'.self::$m_sEnvironment.'/');
 
-				self::$m_bTraceSourceFiles = $bTraceSourceFiles;
+			self::$m_bTraceSourceFiles = $bTraceSourceFiles;
 
-				// $config can be either a filename, or a Configuration object (volatile!)
-				if ($config instanceof Config) {
-					self::LoadConfig($config, $bAllowCache);
-				} else {
-					self::LoadConfig(new Config($config), $bAllowCache);
-				}
-
-				if ($bModelOnly) {
-					return;
-				}
+			// $config can be either a filename, or a Configuration object (volatile!)
+			if ($config instanceof Config) {
+				self::LoadConfig($config, $bAllowCache);
+			} else {
+				self::LoadConfig(new Config($config), $bAllowCache);
 			}
 
-			CMDBSource::SelectDB(self::$m_sDBName);
-
-			foreach (MetaModel::EnumPlugins('ModuleHandlerApiInterface') as $oPHPClass) {
-				$oPHPClass::OnMetaModelStarted();
+			if ($bModelOnly) {
+				// Event service must be initialized after the MetaModel startup, otherwise it cannot discover classes implementing the iEventServiceSetup interface
+				EventService::InitService();
+				EventService::FireEvent(new EventData(ApplicationEvents::APPLICATION_EVENT_METAMODEL_STARTED));
+				return;
 			}
-
-			ExpressionCache::Warmup();
-		} finally {
-			// Event service must be initialized after the MetaModel startup, otherwise it cannot discover classes implementing the iEventServiceSetup interface
-			EventService::InitService();
-			EventService::FireEvent(new EventData(ApplicationEvents::APPLICATION_EVENT_METAMODEL_STARTED));
 		}
+
+		CMDBSource::SelectDB(self::$m_sDBName);
+
+		foreach (MetaModel::EnumPlugins('ModuleHandlerApiInterface') as $oPHPClass) {
+			$oPHPClass::OnMetaModelStarted();
+		}
+
+		ExpressionCache::Warmup();
+
+		// Event service must be initialized after the MetaModel startup, otherwise it cannot discover classes implementing the iEventServiceSetup interface
+		EventService::InitService();
+		EventService::FireEvent(new EventData(ApplicationEvents::APPLICATION_EVENT_METAMODEL_STARTED));
 	}
 
 	/**

datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php

@@ -129,9 +129,6 @@ class DataFeatureRemovalController extends Controller
 
 			$aRemovedExtensions = $this->aExtensionsToCheck['to_be_removed'];
 			$aHiddenInputs['removed_extensions'] = $this->ConvertIntoSetupFormat($aRemovedExtensions);
-
-			$aExtensionsNotUninstallable = $this->aExtensionsToCheck['extensions_not_uninstallable'];
-			$aHiddenInputs['extensions_not_uninstallable'] = $this->ConvertIntoSetupFormat($aExtensionsNotUninstallable);
 		}
 
 		$aRemoveExtensionCodes = array_keys($aRemovedExtensions);
@@ -429,7 +426,6 @@ class DataFeatureRemovalController extends Controller
 		$this->aExtensionsToCheck = [
 			'to_be_installed' => [],
 			'to_be_removed' => [],
-			'extensions_not_uninstallable' => [],
 		];
 		foreach ($aAvailableExtensions as $sCode => &$aExtensionData) {
 			if (!isset($aSelectedExtensionsFromUI[$sCode])) {
@@ -443,9 +439,6 @@ class DataFeatureRemovalController extends Controller
 				if (! $this->bForcedUninstallation && $aExtensionData['extra_flags']['uninstallable']) {
 					$this->bForcedUninstallation = true;
 				}
-				if (false === $aExtensionData['extra_flags']['uninstallable']) {
-					$this->aExtensionsToCheck['extensions_not_uninstallable'][] = $sCode;
-				}
 			} elseif (!$aExtensionData['installed'] && $aSelectedExtensionsFromUI[$sCode] === 'on') {
 				$aExtensionData['extra_flags']['selected'] = true;
 				$sLabel = $aAvailableExtensions[$sCode]['label'];

js/layouts/activity-panel/activity-panel.js

@@ -476,8 +476,7 @@ $(function()
 
 				let sStimulusCode = (undefined !== oData.stimulus_code) ? oData.stimulus_code : null
                 // If several entry forms filled, show a confirmation message
-                if ((GetUserPreference('activity_panel.show_multiple_entries_submit_confirmation',true) === true
-                    || GetUserPreference('activity_panel.show_multiple_entries_submit_confirmation', true) === "true")
+                if (GetUserPreferenceAsBoolean('activity_panel.show_multiple_entries_submit_confirmation',true) === true
                     && (Object.keys(await this._GetEntriesFromAllForms()).length > 1)) {
 					this._ShowEntriesSubmitConfirmation(sStimulusCode);
 				}

js/utils.js

@@ -135,14 +135,10 @@ function SetUserPreference(sPreferenceCode, sPrefValue, bPersistent) {
 	} catch (err) {
 		sPreviousValue = undefined;
 	}
+	oUserPreferences[sPreferenceCode] = sPrefValue;
 	if (bPersistent && (sPrefValue != sPreviousValue)) {
-		return $.post(GetAbsoluteUrlAppRoot()+'pages/ajax.render.php',
-			{operation: 'set_pref', code: sPreferenceCode, value: sPrefValue}, function (data) {
-			}).done(function() {
-				oUserPreferences[sPreferenceCode] = sPrefValue;
-			}); // Make it persistent
-	} else {
-		oUserPreferences[sPreferenceCode] = sPrefValue;
+		ajax_request = $.post(GetAbsoluteUrlAppRoot()+'pages/ajax.render.php',
+			{operation: 'set_pref', code: sPreferenceCode, value: sPrefValue}); // Make it persistent
 	}
 }
 

setup/feature_removal/ModelReflectionSerializer.php

@@ -2,11 +2,10 @@
 
 namespace Combodo\iTop\Setup\FeatureRemoval;
 
-use ContextTag;
 use CoreException;
-use Exception;
 use IssueLog;
 use SetupLog;
+use SetupUtils;
 use utils;
 
 class ModelReflectionSerializer
@@ -15,6 +14,7 @@ class ModelReflectionSerializer
 
 	protected function __construct()
 	{
+		SetupLog::Enable(APPROOT.'log/setup.log');
 	}
 
 	final public static function GetInstance(): ModelReflectionSerializer
@@ -31,42 +31,53 @@ class ModelReflectionSerializer
 		self::$oInstance = $oInstance;
 	}
 
+	public const ERROR_LABEL = "Data consistency check failed: %s";
+
 	public function GetModelFromEnvironment(string $sEnv): array
 	{
 		IssueLog::Debug(__METHOD__, null, ['env' => $sEnv]);
 
-		$sPHPExec = trim(utils::GetConfig()->Get('php_path'));
-		$sOutput = "";
-		$iRes = 0;
+		SetupUtils::CheckCliPhpVersionIsOk(self::ERROR_LABEL);
 
-		$sCommandLine = sprintf("$sPHPExec %s/get_model_reflection.php --env=%s", __DIR__, escapeshellarg($sEnv));
-		exec($sCommandLine, $sOutput, $iRes);
-		if ($iRes != 0) {
-			$this->LogErrorWithProperLogger("Cannot get classes", null, ['env' => $sEnv, 'code' => $iRes, "output" => $sOutput, 'cmd' => $sCommandLine]);
-			throw new CoreException("Cannot get classes from env ".$sEnv);
+		//preliminary check
+		$sEnvDir = APPROOT."env-$sEnv";
+		if (! is_dir($sEnvDir)) {
+			$sMsg = sprintf(self::ERROR_LABEL, "Missing environment ($sEnvDir)");
+			SetupLog::Error($sMsg);
+			throw new CoreException($sMsg);
 		}
 
-		$aClasses = json_decode($sOutput[0] ?? null, true);
+		$sConfigFile = APPROOT."conf/$sEnv/config-itop.php";
+		if (! is_file($sConfigFile)) {
+			$sMsg = sprintf(self::ERROR_LABEL, "Missing configuration ($sConfigFile)");
+			SetupLog::Error($sMsg);
+			throw new CoreException($sMsg);
+		}
+
+		$sPHPExec = trim(utils::GetConfig()->Get('php_path'));
+		$aOutput = null;
+		$iRes = 0;
+		$sCommandLine = sprintf("$sPHPExec %s/get_model_reflection.php --env=%s", __DIR__, escapeshellarg($sEnv));
+		exec($sCommandLine, $aOutput, $iRes);
+		if ($iRes != 0) {
+			$sError = $aOutput[0] ?? 'Invalid output when serializing model';
+			SetupLog::Error(sprintf(self::ERROR_LABEL, '(cli error) '.$sError), null, ['env' => $sEnv, 'code' => $iRes, "output" => $aOutput, 'cmd' => $sCommandLine]);
+			throw new CoreException(sprintf(self::ERROR_LABEL, $sError));
+		}
+
+		$aClasses = json_decode($aOutput[0] ?? null, true);
 		if (false === $aClasses) {
-			$this->LogErrorWithProperLogger("Invalid JSON", null, ['env' => $sEnv, "output" => $sOutput]);
-			throw new Exception("cannot get classes");
+			$sMsg = sprintf(self::ERROR_LABEL, 'Invalid JSON');
+			SetupLog::Error($sMsg, null, ['env' => $sEnv, "output" => $aOutput]);
+			throw new CoreException($sMsg);
 		}
 
 		if (!is_array($aClasses)) {
-			$this->LogErrorWithProperLogger("not an array", null, ['env' => $sEnv, "classes" => $aClasses, "output" => $sOutput]);
-			throw new Exception("cannot get classes from $sEnv");
+			$sError = $aOutput[0] ?? 'Invalid json array when serializing model';
+			SetupLog::Error(sprintf(self::ERROR_LABEL, '(JSON output not an array) '.$sError), null, ['env' => $sEnv, "classes" => $aClasses, "output" => $aOutput]);
+			throw new CoreException(sprintf(self::ERROR_LABEL, $sError));
 		}
 
 		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/get_model_reflection.php

@@ -21,8 +21,7 @@ $sConfFile = utils::GetConfigFilePath($sEnv);
 try {
 	MetaModel::Startup($sConfFile, false /* $bModelOnly */, false /* $bAllowCache */, false /* $bTraceSourceFiles */, $sEnv);
 } catch (\Throwable $e) {
-	echo $e->getMessage();
-	echo $e->getTraceAsString();
+	SetupLog::Enable(APPROOT.'log/setup.log');
 	\SetupLog::Error(
 		"Cannot read model from provided environment",
 		null,
@@ -32,7 +31,9 @@ try {
 			'stack' => $e->getTraceAsString(),
 		]
 	);
-	echo "Cannot read model from provided environment";
+
+	//keep first echo to have proper setup feedbacks
+	echo $e->getMessage();
 	exit(1);
 }
 

setup/sequencers/DataAuditSequencer.php

@@ -70,6 +70,7 @@ class DataAuditSequencer extends StepSequencer
 					$aSelectedExtensionCodes = $this->oParams->Get('selected_extensions', []);
 					$bUseSymbolicLinks = $this->oParams->Get('use_symbolic_links', null) === 'on';
 					MetaModel::ResetAllCaches($this->oRunTimeEnvironment->GetBuildEnv());
+
 					$this->oRunTimeEnvironment->DoCompile(
 						$aSelectedExtensionCodes,
 						$aRemovedExtensionCodes,
@@ -79,6 +80,7 @@ class DataAuditSequencer extends StepSequencer
 					return $this->ComputeNextStep($sStep);
 
 				case 'setup-audit':
+					$this->oRunTimeEnvironment->ExitMaintenanceMode();
 					$this->oRunTimeEnvironment->DataToCleanupAudit();
 					return $this->ComputeNextStep($sStep);
 

setup/setuputils.class.inc.php

@@ -232,6 +232,13 @@ class SetupUtils
 				$aResult[] = new CheckResult(CheckResult::WARNING, "Missing optional PHP extension: $sExtension. ".$sMessage);
 			}
 		}
+
+		try {
+			SetupUtils::CheckCliPhpVersionIsOk();
+		} catch (CoreException $e) {
+			$aResult[] = new CheckResult(CheckResult::WARNING, $e->getMessage());
+		}
+
 		// Check some ini settings here
 		if (function_exists('php_ini_loaded_file')) { // PHP >= 5.2.4
 			$sPhpIniFile = php_ini_loaded_file();
@@ -2192,6 +2199,41 @@ JS
 
 		return [$sButtonLabel, $sButtonUrl];
 	}
+
+	/**
+	* @param string $sErrorLabel: error label with 1 placeholder inside for detailed error
+	* @return void
+	* @throws \ConfigException
+	* @throws \CoreException
+	 */
+	public static function CheckCliPhpVersionIsOk(string $sErrorLabel = '%s'): void
+	{
+		$sPHPExec = trim(utils::GetConfig()->Get('php_path'));
+		$aOutput = null;
+		$iRes = 0;
+		exec("$sPHPExec --version", $aOutput, $iRes);
+		if ($iRes != 0) {
+			$sError = sprintf($sErrorLabel, "Cannot check CLI/PHP version ($sPHPExec)");
+			SetupLog::Error($sError, null, ['code' => $iRes, "output" => $aOutput, 'php_path' => $sPHPExec]);
+			throw new CoreException($sError);
+		}
+
+		SetupUtils::CheckCliPhpVersionFromOutput($sErrorLabel, PHP_MAJOR_VERSION.'.'.PHP_MINOR_VERSION, $sPHPExec, $aOutput);
+	}
+
+	private static function CheckCliPhpVersionFromOutput(string $sErrorLabel, string $sUIPhpVersion, string $sPHPExec, $aOutput): void
+	{
+		$sFoundVersion = trim($aOutput[0] ?? "");
+		if (false !== preg_match('/(\d+\.\d+)(?:\.\d+)?/', $sFoundVersion, $aMatches)) {
+			$sFoundVersion = $aMatches[1];
+		}
+
+		if ($sFoundVersion !== $sUIPhpVersion) {
+			$sError = sprintf($sErrorLabel, "Mismatch between PHP versions (CLI: $sFoundVersion/ UI: $sUIPhpVersion)");
+			SetupLog::Error($sError, null, ["output" => $aOutput, 'php_path' => $sPHPExec]);
+			throw new CoreException($sError);
+		}
+	}
 }
 
 /**

setup/wizardsteps/WizStepInstall.php

@@ -112,22 +112,6 @@ JS);
 			return;
 		}
 
-		// When the setup reach this step, it already checked whether extensions were uninstallable (during WizStepModulesChoice or combodo-data-feature-removal). We only need to log what has been done.
-		if ($this->oWizard->GetParameter('force-uninstall', false)) {
-			SetupLog::Warning("User disabled uninstallation checks");
-		}
-		$aExtensionsRemoved = json_decode($this->oWizard->GetParameter('removed_extensions'), true) ?? [];
-		$aExtensionsNotUninstallable = json_decode($this->oWizard->GetParameter('extensions_not_uninstallable'), true) ?? [];
-		$aExtensionsForceUninstalled = [];
-		foreach ($aExtensionsRemoved as $sExtensionCode => $sLabel) {
-			if (in_array($sExtensionCode, $aExtensionsNotUninstallable)) {
-				$aExtensionsForceUninstalled[] = $sExtensionCode;
-			}
-		}
-		if (count($aExtensionsForceUninstalled)) {
-			SetupLog::Warning("Extensions uninstalled forcefully : ".implode(',', $aExtensionsForceUninstalled));
-		}
-
 		$oPage->add_ready_script(<<<JS
 	$("#wiz_form").data("installation_status", "not started");
 	ExecuteStep("");

setup/wizardsteps/WizStepLandingBeforeAudit.php

@@ -31,8 +31,8 @@ class WizStepLandingBeforeAudit extends WizStepModulesChoice
 		$oWizard->SetParameter('db_tls_enabled', $this->oConfig->Get('db_tls.enabled'));
 		$oWizard->SetParameter('db_tls_ca', $this->oConfig->Get('db_tls.ca') ?? '');
 		$oWizard->SetParameter('display_choices', '');
+		$oWizard->SetParameter('extensions_not_uninstallable', '[]');
 
-		$oWizard->SaveParameter('extensions_not_uninstallable', '[]');
 		$oWizard->SaveParameter('use_symbolic_links', MFCompiler::UseSymbolicLinks());
 		$oWizard->SaveParameter('force-uninstall', false);
 		$oWizard->SaveParameter('skip_wizard', false);

setup/wizardsteps/WizStepSummary.php

@@ -111,7 +111,7 @@ class WizStepSummary extends AbstractWizStepInstall
 		$oPage->add('<div class="closed"><a class="title ibo-setup-summary-title" href="#" aria-label="Extensions to be uninstalled">Extensions to be uninstalled</a>');
 
 		$aExtensionsRemoved = json_decode($this->oWizard->GetParameter('removed_extensions'), true) ?? [];
-		$aExtensionsNotUninstallable = json_decode($this->oWizard->GetParameter('extensions_not_uninstallable'), true) ?? [];
+		$aExtensionsNotUninstallable = json_decode($this->oWizard->GetParameter('extensions_not_uninstallable')) ?? [];
 		if (count($aExtensionsRemoved) > 0) {
 			$sExtensionsRemoved = '<ul>';
 			foreach ($aExtensionsRemoved as $sExtensionCode => $sLabel) {

tests/php-unit-tests/unitary-tests/setup/SetupUtilsTest.php

@@ -2,6 +2,7 @@
 
 namespace Combodo\iTop\Test\UnitTest\Setup;
 
+use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer;
 use Combodo\iTop\Test\UnitTest\ItopTestCase;
 use SetupUtils;
 
@@ -146,4 +147,42 @@ class SetupUtilsTest extends ItopTestCase
 			);
 		}
 	}
+
+	public function testCheckCliPhpVersionIsOk()
+	{
+		$this->RequireOnceItopFile('/setup/feature_removal/ModelReflectionSerializer.php');
+		SetupUtils::CheckCliPhpVersionIsOk(ModelReflectionSerializer::ERROR_LABEL);
+		$this->assertTrue(true);
+	}
+
+	public function testCheckCliPhpVersionFromOutputFail()
+	{
+		$this->RequireOnceItopFile('/setup/feature_removal/ModelReflectionSerializer.php');
+		$sOuput = <<<OUTPUT
+PHP 7.4.33 (cli) (built: Aug 2 2024 16:22:28) ( NTS )
+OUTPUT;
+
+		$this->expectException(\CoreException::class);
+		$this->expectExceptionMessage("Data consistency check failed: Mismatch between PHP versions (CLI: 7.4/ UI: 6.6)");
+		$this->InvokeNonPublicStaticMethod(SetupUtils::class, 'CheckCliPhpVersionFromOutput', [ModelReflectionSerializer::ERROR_LABEL, '6.6', 'sPHPExec', [$sOuput]]);
+	}
+
+	public function CheckOKProvider()
+	{
+		return [
+			["7.4 7.2 7.3.33"],
+			["PHP 7.4.33 (cli) (built: Aug  2 2024 16:22:28) ( NTS )"],
+			["PHP 7.4.33 PHP 7.33.22"],
+			["version: 7.4.27 stable"],
+		];
+	}
+	/**
+	 * @dataProvider CheckOKProvider
+	 */
+	public function testCheckCliPhpVersionFromOutputOK($sOuput)
+	{
+		$this->RequireOnceItopFile('/setup/feature_removal/ModelReflectionSerializer.php');
+		$this->InvokeNonPublicStaticMethod(SetupUtils::class, 'CheckCliPhpVersionFromOutput', [ModelReflectionSerializer::ERROR_LABEL, '7.4', 'sPHPExec', [$sOuput]]);
+		$this->assertTrue(true);
+	}
 }

tests/php-unit-tests/unitary-tests/setup/feature_removal/ModelSerializationTest.php

@@ -20,10 +20,58 @@ class ModelSerializationTest extends ItopDataTestCase
 		$this->assertEqualsCanonicalizing(MetaModel::GetClasses(), $aModel);
 	}
 
-	public function testGetModelFromEnvironmentFailure()
+	public function testGetModelFromEnvironmentFailure_NoEnvt()
 	{
 		$this->expectException(\CoreException::class);
-		$this->expectExceptionMessage("Cannot get classes");
+		$sEnvDir = APPROOT."env-gabuzomeu";
+		$this->expectExceptionMessage("Data consistency check failed: Missing environment ($sEnvDir)");
 		ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment('gabuzomeu');
 	}
+
+	public function testGetModelFromEnvironmentFailure_NoConfiguration()
+	{
+		$sEnvDir = APPROOT."env-gabuzomeu";
+		$this->aFileToClean [] = $sEnvDir;
+		mkdir($sEnvDir);
+
+		$this->expectException(\CoreException::class);
+		$sConfigFile = APPROOT."conf/gabuzomeu/config-itop.php";
+		$this->expectExceptionMessage("Data consistency check failed: Missing configuration ($sConfigFile)");
+		ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment('gabuzomeu');
+	}
+
+	public function testGetModelFromEnvironmentFailure_BrokenConfiguration()
+	{
+		$sEnvDir = APPROOT."env-gabuzomeu";
+		mkdir($sEnvDir);
+		$this->aFileToClean [] = $sEnvDir;
+
+		mkdir(APPROOT."conf/gabuzomeu");
+		$this->aFileToClean [] = APPROOT."conf/gabuzomeu";
+		$sConfigFile = APPROOT."conf/gabuzomeu/config-itop.php";
+		touch($sConfigFile);
+		file_put_contents($sConfigFile, 'invalid php content...');
+
+		$this->expectException(\CoreException::class);
+		$sError = <<<ERROR
+Syntax error in configuration file: file = $sConfigFile, error = <tt>invalid php content...</tt>
+ERROR;
+
+		$this->expectExceptionMessage("Data consistency check failed: $sError");
+		ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment('gabuzomeu');
+	}
+
+	public function testGetModelFromEnvironmentFailure_ItopInMaintenanceMode()
+	{
+		touch(MAINTENANCE_MODE_FILE);
+		$this->aFileToClean [] = MAINTENANCE_MODE_FILE;
+
+		$this->expectException(\CoreException::class);
+		$sError = <<<ERROR
+This application is currently under maintenance.
+ERROR;
+
+		$this->expectExceptionMessage("Data consistency check failed: $sError");
+		ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->GetTestEnvironment());
+	}
 }

tests/php-unit-tests/unitary-tests/setup/sequencers/DataAuditSequencerTest.php

@@ -201,6 +201,7 @@ class DataAuditSequencerTest extends ItopTestCase
 		$oRunTimeEnvironment = $this->createMock(\RunTimeEnvironment::class);
 		$oRunTimeEnvironment->expects($this->once())->method('GetApplicationVersion')
 			->willReturn(['product_version' => ITOP_VERSION_FULL]);
+		$oRunTimeEnvironment->expects($this->once())->method('ExitMaintenanceMode');
 		$oRunTimeEnvironment->expects($this->once())->method('DataToCleanupAudit');
 		$oRunTimeEnvironment->expects($this->any())->method('GetFinalEnv')
 			->willReturn('production');