composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-02-13 07:24:13 +01:00
Code Issues Packages Projects Releases Wiki Activity

N°8764 halt setup wizard at data issue - 2nd review

Browse Source
This commit is contained in:
odain
2026-01-13 15:21:57 +01:00
parent 7df59427cb
commit 1e15eb1161
7 changed files with 71 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
setup/feature_removal/AbstractSetupAudit.php
View File

@@ -13,12 +13,9 @@ require_once APPROOT.'setup/feature_removal/ModelReflectionSerializer.php';
abstract class AbstractSetupAudit 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 bool $bClassesInitialized = false;
protected array $aClassesBeforeRemoval = []; protected array $aClassesBefore = [];
protected array $aClassesAfterRemoval = []; protected array $aClassesAfter = [];
protected array $aRemovedClasses = []; protected array $aRemovedClasses = [];
protected array $aFinalClassesToCleanup = []; protected array $aFinalClassesToCleanup = [];
@@ -33,15 +30,15 @@ abstract class AbstractSetupAudit
$this->ComputeClasses(); $this->ComputeClasses();
if (count($this->aRemovedClasses) == 0) { if (count($this->aRemovedClasses) == 0) {
if (count($this->aClassesBeforeRemoval) == 0) { if (count($this->aClassesBefore) == 0) {
return $this->aRemovedClasses; return $this->aRemovedClasses;
} }
if (count($this->aClassesAfterRemoval) == 0) { if (count($this->aClassesAfter) == 0) {
return $this->aRemovedClasses; return $this->aRemovedClasses;
} }
$aExtensionsNames = array_diff($this->aClassesBeforeRemoval, $this->aClassesAfterRemoval); $aExtensionsNames = array_diff($this->aClassesBefore, $this->aClassesAfter);
$this->aRemovedClasses = []; $this->aRemovedClasses = [];
$aClasses = array_values($aExtensionsNames); $aClasses = array_values($aExtensionsNames);
sort($aClasses); sort($aClasses);
@@ -54,23 +51,8 @@ abstract class AbstractSetupAudit
return $this->aRemovedClasses; 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 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 = []; $this->aFinalClassesToCleanup = [];
foreach ($this->GetRemovedClasses() as $sClass) { foreach ($this->GetRemovedClasses() as $sClass) {

14
setup/feature_removal/InplaceSetupAudit.php
View File

@@ -12,13 +12,13 @@ class InplaceSetupAudit extends AbstractSetupAudit
//file used when present to trigger audit exception when testing specific setups //file used when present to trigger audit exception when testing specific setups
public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; 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(); parent::__construct();
$this->aClassesBeforeRemoval = $aClassesBeforeRemoval; $this->aClassesBefore = $aClassesBefore;
$this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; $this->sEnvAfter = $sEnvAfter;
} }
public function ComputeClasses(): void public function ComputeClasses(): void
@@ -29,10 +29,10 @@ class InplaceSetupAudit extends AbstractSetupAudit
$sCurrentEnvt = MetaModel::GetEnvironment(); $sCurrentEnvt = MetaModel::GetEnvironment();
if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { if ($sCurrentEnvt === $this->sEnvAfter) {
$this->aClassesAfterRemoval = MetaModel::GetClasses(); $this->aClassesAfter = MetaModel::GetClasses();
} else { } else {
$this->aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfterExtensionRemoval); $this->aClassesAfter = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfter);
} }
$this->bClassesInitialized = true; $this->bClassesInitialized = true;

28
setup/feature_removal/SetupAudit.php
View File

@@ -12,14 +12,14 @@ class SetupAudit extends AbstractSetupAudit
//file used when present to trigger audit exception when testing specific setups //file used when present to trigger audit exception when testing specific setups
public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt';
private string $sEnvBeforeExtensionRemoval; private string $sEnvBefore;
private string $sEnvAfterExtensionRemoval; private string $sEnvAfter;
public function __construct(string $sEnvBeforeExtensionRemoval, string $sEnvAfterExtensionRemoval) public function __construct(string $sEnvBefore, string $sEnvAfter)
{ {
parent::__construct(); parent::__construct();
$this->sEnvBeforeExtensionRemoval = $sEnvBeforeExtensionRemoval; $this->sEnvBefore = $sEnvBefore;
$this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; $this->sEnvAfter = $sEnvAfter;
} }
public function ComputeClasses(): void public function ComputeClasses(): void
@@ -29,16 +29,16 @@ class SetupAudit extends AbstractSetupAudit
} }
$sCurrentEnvt = MetaModel::GetEnvironment(); $sCurrentEnvt = MetaModel::GetEnvironment();
if ($sCurrentEnvt === $this->sEnvBeforeExtensionRemoval) { if ($sCurrentEnvt === $this->sEnvBefore) {
$this->aClassesBeforeRemoval = MetaModel::GetClasses(); $this->aClassesBefore = MetaModel::GetClasses();
} else { } else {
$this->aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBeforeExtensionRemoval); $this->aClassesBefore = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBefore);
} }
if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { if ($sCurrentEnvt === $this->sEnvAfter) {
$this->aClassesAfterRemoval = MetaModel::GetClasses(); $this->aClassesAfter = MetaModel::GetClasses();
} else { } else {
$this->aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfterExtensionRemoval); $this->aClassesAfter = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfter);
} }
$this->bClassesInitialized = true; $this->bClassesInitialized = true;
@@ -60,15 +60,15 @@ class SetupAudit extends AbstractSetupAudit
$this->ComputeClasses(); $this->ComputeClasses();
if (count($this->aRemovedClasses) == 0) { if (count($this->aRemovedClasses) == 0) {
if (count($this->aClassesBeforeRemoval) == 0) { if (count($this->aClassesBefore) == 0) {
return $this->aRemovedClasses; return $this->aRemovedClasses;
} }
if (count($this->aClassesAfterRemoval) == 0) { if (count($this->aClassesAfter) == 0) {
return $this->aRemovedClasses; return $this->aRemovedClasses;
} }
$aExtensionsNames = array_diff($this->aClassesBeforeRemoval, $this->aClassesAfterRemoval); $aExtensionsNames = array_diff($this->aClassesBefore, $this->aClassesAfter);
$this->aRemovedClasses = []; $this->aRemovedClasses = [];
$aClasses = array_values($aExtensionsNames); $aClasses = array_values($aExtensionsNames);
sort($aClasses); sort($aClasses);

21
setup/modulediscovery.class.inc.php
View File

@@ -22,6 +22,7 @@
use Combodo\iTop\PhpParser\Evaluation\PhpExpressionEvaluator; use Combodo\iTop\PhpParser\Evaluation\PhpExpressionEvaluator;
use Combodo\iTop\Setup\ModuleDependency\Module; use Combodo\iTop\Setup\ModuleDependency\Module;
use Combodo\iTop\Setup\ModuleDependency\ModuleDependencySort;
use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReader; use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReader;
use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReaderException; 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__.'/moduledependency/moduledependencysort.class.inc.php');
require_once(__DIR__.'/itopextension.class.inc.php'); require_once(__DIR__.'/itopextension.class.inc.php');
use Combodo\iTop\Setup\ModuleDependency\ModuleDependencySort;
class MissingDependencyException extends CoreException class MissingDependencyException extends CoreException
{ {
/** /**
@@ -135,7 +134,7 @@ class ModuleDiscovery
list($sModuleName, $sModuleVersion) = static::GetModuleName($sId); list($sModuleName, $sModuleVersion) = static::GetModuleName($sId);
if (self::IsModulePartOfRemovedExtension($sModuleName, $sModuleVersion, $aArgs)) { if (self::IsModuleInExtensionList(self::$m_aRemovedExtensions, $sModuleName, $sModuleVersion, $aArgs)) {
return; return;
} }
@@ -230,7 +229,7 @@ class ModuleDiscovery
$oModule = new Module($sModuleId); $oModule = new Module($sModuleId);
$sModuleName = $oModule->GetModuleName(); $sModuleName = $oModule->GetModuleName();
if (self::IsModulePartOfRemovedExtension($sModuleName, $oModule->GetVersion(), $aModuleInfo)) { if (self::IsModuleInExtensionList(self::$m_aRemovedExtensions, $sModuleName, $oModule->GetVersion(), $aModuleInfo)) {
continue; continue;
} }
@@ -254,16 +253,24 @@ class ModuleDiscovery
self::$m_aRemovedExtensions = $aRemovedExtension; 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; return false;
} }
$aNonMatchingPaths = []; $aNonMatchingPaths = [];
$sModuleFilePath = $aModuleInfo[ModuleFileReader::MODULE_FILE_PATH]; $sModuleFilePath = $aModuleInfo[ModuleFileReader::MODULE_FILE_PATH];
/** @var \iTopExtension $oExtension */ /** @var \iTopExtension $oExtension */
foreach (self::$m_aRemovedExtensions as $oExtension) { foreach ($aExtensions as $oExtension) {
$sCurrentVersion = $oExtension->aModuleVersion[$sModuleName] ?? null; $sCurrentVersion = $oExtension->aModuleVersion[$sModuleName] ?? null;
if (is_null($sCurrentVersion)) { if (is_null($sCurrentVersion)) {
continue; continue;

7
tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php
View File

@@ -701,11 +701,12 @@ abstract class ItopTestCase extends KernelTestCase
/** /**
* Return a temporary file path. that will be cleaned up by tearDown() * 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); $sPath = tempnam(sys_get_temp_dir(), $sPrefix);
$this->aFileToClean[] = $sPath; $this->aFileToClean[] = $sPath;
return $sPath; return $sPath;

49
tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php
View File

@@ -90,8 +90,8 @@ TXT;
{ {
$aChoices = ['id1', 'id2']; $aChoices = ['id1', 'id2'];
$sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); $sModuleFilePath = $this->GetTemporaryFilePath();
$sModuleFilePath2 = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); $sModuleFilePath2 = $this->GetTemporaryFilePath();
$aModules = [ $aModules = [
"id1/1" => [ "id1/1" => [
@@ -106,7 +106,7 @@ TXT;
], ],
]; ];
$oExtension = $this->CreateExtensionWithModule('id2', '2', $sModuleFilePath2); $oExtension = $this->GivenExtensionWithModule('id2', '2', $sModuleFilePath2);
ModuleDiscovery::DeclareRemovedExtensions([$oExtension]); ModuleDiscovery::DeclareRemovedExtensions([$oExtension]);
$sExpectedMessage = <<<TXT $sExpectedMessage = <<<TXT
@@ -153,51 +153,52 @@ TXT;
$this->assertEquals([$expectedName, $expectedVersion], ModuleDiscovery::GetModuleName($sModuleId)); $this->assertEquals([$expectedName, $expectedVersion], ModuleDiscovery::GetModuleName($sModuleId));
} }
public function testIsModulePartOfRemovedExtension_NoRemovedExtension() public function testIsModuleInExtensionList_NoRemovedExtension()
{ {
ModuleDiscovery::DeclareRemovedExtensions([]); $this->assertFalse($this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModuleInExtensionList", [[], 'module_name', '123', []]));
$this->assertFalse($this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModulePartOfRemovedExtension", ['module_name', '123', []]));
} }
public function testIsModulePartOfRemovedExtension_ModuleWithAnotherVersionIncludedInRemoveExtension() public function testIsModuleInExtensionList_ModuleWithAnotherVersionIncludedInRemoveExtension()
{ {
$sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); $sModuleFilePath = $this->GetTemporaryFilePath();
$this->AssertModuleIsPartOfRemovedExtension('module_name', '123', $sModuleFilePath, $sModuleFilePath, false); $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'); $sModuleFilePath = $this->GetTemporaryFilePath();
$this->AssertModuleIsPartOfRemovedExtension('another_module_name', '456', $sModuleFilePath, $sModuleFilePath, false); $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'); $sModuleFilePath = $this->GetTemporaryFilePath();
$sModuleFilePath2 = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); $sModuleFilePath2 = $this->GetTemporaryFilePath();
$this->AssertModuleIsPartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath2, false); $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'); $sModuleFilePath = $this->GetTemporaryFilePath();
$this->AssertModuleIsPartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath, true); $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 = [ $aCurrentModuleInfo = [
ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath, ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath,
]; ];
$this->assertEquals( $this->assertEquals(
$bExpected, $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 = new iTopExtension();
$oExt->aModuleVersion[$sModuleName] = $sVersion; $oExt->aModuleVersion[$sModuleName] = $sVersion;

5
tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php
View File

@@ -3,12 +3,11 @@
namespace Combodo\iTop\Test\UnitTest\Setup\FeatureRemoval; namespace Combodo\iTop\Test\UnitTest\Setup\FeatureRemoval;
use Combodo\iTop\Setup\FeatureRemoval\DryRemovalRuntimeEnvironment; use Combodo\iTop\Setup\FeatureRemoval\DryRemovalRuntimeEnvironment;
use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit;
use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer; use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer;
use Combodo\iTop\Setup\FeatureRemoval\SetupAudit; use Combodo\iTop\Setup\FeatureRemoval\SetupAudit;
use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit;
use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase; use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase;
use Combodo\iTop\Test\UnitTest\Service\UnitTestRunTimeEnvironment; use Combodo\iTop\Test\UnitTest\Service\UnitTestRunTimeEnvironment;
use Exception;
use MetaModel; use MetaModel;
class SetupAuditTest extends ItopCustomDatamodelTestCase class SetupAuditTest extends ItopCustomDatamodelTestCase
@@ -79,7 +78,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase
$aClassesBeforeRemoval[] = "GabuZomeu"; $aClassesBeforeRemoval[] = "GabuZomeu";
$oSetupAudit = new InplaceSetupAudit($aClassesBeforeRemoval, $sEnv); $oSetupAudit = new InplaceSetupAudit($aClassesBeforeRemoval, $sEnv);
$oSetupAudit->ComputeClasses($aClassesBeforeRemoval); $oSetupAudit->ComputeClasses();
$this->assertEquals(["GabuZomeu"], $oSetupAudit->GetRemovedClasses()); $this->assertEquals(["GabuZomeu"], $oSetupAudit->GetRemovedClasses());
} }