From 825c9f3a0e37b4c37adc63b56ef6c56f4ece1c11 Mon Sep 17 00:00:00 2001 From: odain Date: Wed, 1 Jul 2026 15:53:41 +0200 Subject: [PATCH] =?UTF-8?q?N=C2=B09675=20-=20enhance=20setup=20data=20cons?= =?UTF-8?q?istency=20error=20feedback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../ModelReflectionSerializer.php | 51 ++++++++++++------ .../feature_removal/get_model_reflection.php | 7 +-- .../ModelSerializationTest.php | 53 ++++++++++++++++++- 3 files changed, 89 insertions(+), 22 deletions(-) diff --git a/setup/feature_removal/ModelReflectionSerializer.php b/setup/feature_removal/ModelReflectionSerializer.php index 7e858a9096..3570e36721 100644 --- a/setup/feature_removal/ModelReflectionSerializer.php +++ b/setup/feature_removal/ModelReflectionSerializer.php @@ -36,37 +36,54 @@ class ModelReflectionSerializer IssueLog::Debug(__METHOD__, null, ['env' => $sEnv]); $sPHPExec = trim(utils::GetConfig()->Get('php_path')); - $sOutput = ""; + $aOutput = null; $iRes = 0; - $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); + $sErrorLabel = "Data consistency check failed: %s"; + + //preliminary check + $sEnvDir = APPROOT."env-$sEnv"; + if (! is_dir($sEnvDir)) { + $sMsg = sprintf($sErrorLabel, "Missing environment ($sEnvDir)"); + $this->LogSetupError($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($sErrorLabel, "Missing configuration ($sConfigFile)"); + $this->LogSetupError($sMsg); + throw new CoreException($sMsg); + } + + $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'; + $this->LogSetupError(sprintf($sErrorLabel, '(cli error) '.$sError), null, ['env' => $sEnv, 'code' => $iRes, "output" => $aOutput, 'cmd' => $sCommandLine]); + throw new CoreException(sprintf($sErrorLabel, $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($sErrorLabel, 'Invalid JSON'); + $this->LogSetupError($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'; + $this->LogSetupError(sprintf($sErrorLabel, '(JSON output not an array) '.$sError), null, ['env' => $sEnv, "classes" => $aClasses, "output" => $aOutput]); + throw new CoreException(sprintf($sErrorLabel, $sError)); } return $aClasses; } //could be shared with others in log APIs ? - private function LogErrorWithProperLogger($sMessage, $sChannel = null, $aContext = []): void + private function LogSetupError($sMessage, $sChannel = null, $aContext = []): void { - if (ContextTag::Check(ContextTag::TAG_SETUP)) { - SetupLog::Error($sMessage, $sChannel, $aContext); - } else { - IssueLog::Error($sMessage, $sChannel, $aContext); - } + SetupLog::Enable(APPROOT.'log/setup.log'); + SetupLog::Error($sMessage, $sChannel, $aContext); } } diff --git a/setup/feature_removal/get_model_reflection.php b/setup/feature_removal/get_model_reflection.php index 92f99e9067..459be319ad 100644 --- a/setup/feature_removal/get_model_reflection.php +++ b/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); } diff --git a/tests/php-unit-tests/unitary-tests/setup/feature_removal/ModelSerializationTest.php b/tests/php-unit-tests/unitary-tests/setup/feature_removal/ModelSerializationTest.php index b671ac9840..cfa57dccc6 100644 --- a/tests/php-unit-tests/unitary-tests/setup/feature_removal/ModelSerializationTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/feature_removal/ModelSerializationTest.php @@ -4,6 +4,7 @@ namespace Combodo\iTop\Test\UnitTest\Setup\FeatureRemoval; use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; +use ContextTag; use MetaModel; class ModelSerializationTest extends ItopDataTestCase @@ -20,10 +21,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 = <<invalid php content... +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 = <<expectExceptionMessage("Data consistency check failed: $sError"); + ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->GetTestEnvironment()); + } }