diff --git a/setup/feature_removal/AbstractSetupAudit.php b/setup/feature_removal/AbstractSetupAudit.php index 0e606620f..abe6d142c 100644 --- a/setup/feature_removal/AbstractSetupAudit.php +++ b/setup/feature_removal/AbstractSetupAudit.php @@ -13,12 +13,9 @@ require_once APPROOT.'setup/feature_removal/ModelReflectionSerializer.php'; abstract class AbstractSetupAudit { - //file used when present to trigger audit exception when testing specific setups - public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; - protected bool $bClassesInitialized = false; - protected array $aClassesBeforeRemoval = []; - protected array $aClassesAfterRemoval = []; + protected array $aClassesBefore = []; + protected array $aClassesAfter = []; protected array $aRemovedClasses = []; protected array $aFinalClassesToCleanup = []; @@ -33,15 +30,15 @@ abstract class AbstractSetupAudit $this->ComputeClasses(); if (count($this->aRemovedClasses) == 0) { - if (count($this->aClassesBeforeRemoval) == 0) { + if (count($this->aClassesBefore) == 0) { return $this->aRemovedClasses; } - if (count($this->aClassesAfterRemoval) == 0) { + if (count($this->aClassesAfter) == 0) { return $this->aRemovedClasses; } - $aExtensionsNames = array_diff($this->aClassesBeforeRemoval, $this->aClassesAfterRemoval); + $aExtensionsNames = array_diff($this->aClassesBefore, $this->aClassesAfter); $this->aRemovedClasses = []; $aClasses = array_values($aExtensionsNames); sort($aClasses); @@ -54,23 +51,8 @@ abstract class AbstractSetupAudit return $this->aRemovedClasses; } - /** test only: return file path that force audit error being raised - * - * @return string - */ - public static function GetErrorMessageFilePathForTestOnly(): string - { - return APPROOT."/data/".self::GETISSUE_ERROR_MSG_FILE_FORTESTONLY; - } - public function GetIssues(bool $bStopDataCheckAtFirstIssue = false): array { - $sErrorMessageFilePath = self::GetErrorMessageFilePathForTestOnly(); - if ($bStopDataCheckAtFirstIssue && is_file($sErrorMessageFilePath)) { - $sMsg = file_get_contents($sErrorMessageFilePath); - throw new \Exception($sMsg); - } - $this->aFinalClassesToCleanup = []; foreach ($this->GetRemovedClasses() as $sClass) { diff --git a/setup/feature_removal/InplaceSetupAudit.php b/setup/feature_removal/InplaceSetupAudit.php index a552d452b..b49a46540 100644 --- a/setup/feature_removal/InplaceSetupAudit.php +++ b/setup/feature_removal/InplaceSetupAudit.php @@ -12,13 +12,13 @@ class InplaceSetupAudit extends AbstractSetupAudit //file used when present to trigger audit exception when testing specific setups public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; - private string $sEnvAfterExtensionRemoval; + private string $sEnvAfter; - public function __construct(array $aClassesBeforeRemoval, string $sEnvAfterExtensionRemoval) + public function __construct(array $aClassesBefore, string $sEnvAfter) { parent::__construct(); - $this->aClassesBeforeRemoval = $aClassesBeforeRemoval; - $this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; + $this->aClassesBefore = $aClassesBefore; + $this->sEnvAfter = $sEnvAfter; } public function ComputeClasses(): void @@ -29,10 +29,10 @@ class InplaceSetupAudit extends AbstractSetupAudit $sCurrentEnvt = MetaModel::GetEnvironment(); - if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { - $this->aClassesAfterRemoval = MetaModel::GetClasses(); + if ($sCurrentEnvt === $this->sEnvAfter) { + $this->aClassesAfter = MetaModel::GetClasses(); } else { - $this->aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfterExtensionRemoval); + $this->aClassesAfter = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfter); } $this->bClassesInitialized = true; diff --git a/setup/feature_removal/SetupAudit.php b/setup/feature_removal/SetupAudit.php index d2caabd12..ebad99527 100644 --- a/setup/feature_removal/SetupAudit.php +++ b/setup/feature_removal/SetupAudit.php @@ -12,14 +12,14 @@ class SetupAudit extends AbstractSetupAudit //file used when present to trigger audit exception when testing specific setups public const GETISSUE_ERROR_MSG_FILE_FORTESTONLY = '.setup_audit_error_msg.txt'; - private string $sEnvBeforeExtensionRemoval; - private string $sEnvAfterExtensionRemoval; + private string $sEnvBefore; + private string $sEnvAfter; - public function __construct(string $sEnvBeforeExtensionRemoval, string $sEnvAfterExtensionRemoval) + public function __construct(string $sEnvBefore, string $sEnvAfter) { parent::__construct(); - $this->sEnvBeforeExtensionRemoval = $sEnvBeforeExtensionRemoval; - $this->sEnvAfterExtensionRemoval = $sEnvAfterExtensionRemoval; + $this->sEnvBefore = $sEnvBefore; + $this->sEnvAfter = $sEnvAfter; } public function ComputeClasses(): void @@ -29,16 +29,16 @@ class SetupAudit extends AbstractSetupAudit } $sCurrentEnvt = MetaModel::GetEnvironment(); - if ($sCurrentEnvt === $this->sEnvBeforeExtensionRemoval) { - $this->aClassesBeforeRemoval = MetaModel::GetClasses(); + if ($sCurrentEnvt === $this->sEnvBefore) { + $this->aClassesBefore = MetaModel::GetClasses(); } else { - $this->aClassesBeforeRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBeforeExtensionRemoval); + $this->aClassesBefore = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvBefore); } - if ($sCurrentEnvt === $this->sEnvAfterExtensionRemoval) { - $this->aClassesAfterRemoval = MetaModel::GetClasses(); + if ($sCurrentEnvt === $this->sEnvAfter) { + $this->aClassesAfter = MetaModel::GetClasses(); } else { - $this->aClassesAfterRemoval = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfterExtensionRemoval); + $this->aClassesAfter = ModelReflectionSerializer::GetInstance()->GetModelFromEnvironment($this->sEnvAfter); } $this->bClassesInitialized = true; @@ -60,15 +60,15 @@ class SetupAudit extends AbstractSetupAudit $this->ComputeClasses(); if (count($this->aRemovedClasses) == 0) { - if (count($this->aClassesBeforeRemoval) == 0) { + if (count($this->aClassesBefore) == 0) { return $this->aRemovedClasses; } - if (count($this->aClassesAfterRemoval) == 0) { + if (count($this->aClassesAfter) == 0) { return $this->aRemovedClasses; } - $aExtensionsNames = array_diff($this->aClassesBeforeRemoval, $this->aClassesAfterRemoval); + $aExtensionsNames = array_diff($this->aClassesBefore, $this->aClassesAfter); $this->aRemovedClasses = []; $aClasses = array_values($aExtensionsNames); sort($aClasses); diff --git a/setup/modulediscovery.class.inc.php b/setup/modulediscovery.class.inc.php index 4acd00318..964457432 100755 --- a/setup/modulediscovery.class.inc.php +++ b/setup/modulediscovery.class.inc.php @@ -22,6 +22,7 @@ use Combodo\iTop\PhpParser\Evaluation\PhpExpressionEvaluator; use Combodo\iTop\Setup\ModuleDependency\Module; +use Combodo\iTop\Setup\ModuleDependency\ModuleDependencySort; use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReader; use Combodo\iTop\Setup\ModuleDiscovery\ModuleFileReaderException; @@ -29,8 +30,6 @@ require_once(APPROOT.'setup/modulediscovery/ModuleFileReader.php'); require_once(__DIR__.'/moduledependency/moduledependencysort.class.inc.php'); require_once(__DIR__.'/itopextension.class.inc.php'); -use Combodo\iTop\Setup\ModuleDependency\ModuleDependencySort; - class MissingDependencyException extends CoreException { /** @@ -135,7 +134,7 @@ class ModuleDiscovery list($sModuleName, $sModuleVersion) = static::GetModuleName($sId); - if (self::IsModulePartOfRemovedExtension($sModuleName, $sModuleVersion, $aArgs)) { + if (self::IsModuleInExtensionList(self::$m_aRemovedExtensions, $sModuleName, $sModuleVersion, $aArgs)) { return; } @@ -230,7 +229,7 @@ class ModuleDiscovery $oModule = new Module($sModuleId); $sModuleName = $oModule->GetModuleName(); - if (self::IsModulePartOfRemovedExtension($sModuleName, $oModule->GetVersion(), $aModuleInfo)) { + if (self::IsModuleInExtensionList(self::$m_aRemovedExtensions, $sModuleName, $oModule->GetVersion(), $aModuleInfo)) { continue; } @@ -254,16 +253,24 @@ class ModuleDiscovery self::$m_aRemovedExtensions = $aRemovedExtension; } - private static function IsModulePartOfRemovedExtension(string $sModuleName, string $sModuleVersion, array $aModuleInfo): bool + /** + * @param array<\iTopExtension> $aExtensions + * @param string $sModuleName + * @param string $sModuleVersion + * @param array $aModuleInfo + * + * @return bool + */ + private static function IsModuleInExtensionList(array $aExtensions, string $sModuleName, string $sModuleVersion, array $aModuleInfo): bool { - if (count(self::$m_aRemovedExtensions) === 0) { + if (count($aExtensions) === 0) { return false; } $aNonMatchingPaths = []; $sModuleFilePath = $aModuleInfo[ModuleFileReader::MODULE_FILE_PATH]; /** @var \iTopExtension $oExtension */ - foreach (self::$m_aRemovedExtensions as $oExtension) { + foreach ($aExtensions as $oExtension) { $sCurrentVersion = $oExtension->aModuleVersion[$sModuleName] ?? null; if (is_null($sCurrentVersion)) { continue; diff --git a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php index 86a955020..f81f67073 100644 --- a/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php +++ b/tests/php-unit-tests/src/BaseTestCase/ItopTestCase.php @@ -701,11 +701,12 @@ abstract class ItopTestCase extends KernelTestCase /** * Return a temporary file path. that will be cleaned up by tearDown() - * @return string: temporary file path + * + * @return string: temporary file path: file prefix include phpunit test method name */ - public function GetTemporaryFilePath(string $sPrefix = "test"): string + public function GetTemporaryFilePath(): string { - + $sPrefix = $this->getName(false); $sPath = tempnam(sys_get_temp_dir(), $sPrefix); $this->aFileToClean[] = $sPath; return $sPath; diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php index 75a318a68..a3b223f29 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleDiscoveryTest.php @@ -90,8 +90,8 @@ TXT; { $aChoices = ['id1', 'id2']; - $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $sModuleFilePath2 = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $sModuleFilePath2 = $this->GetTemporaryFilePath(); $aModules = [ "id1/1" => [ @@ -106,7 +106,7 @@ TXT; ], ]; - $oExtension = $this->CreateExtensionWithModule('id2', '2', $sModuleFilePath2); + $oExtension = $this->GivenExtensionWithModule('id2', '2', $sModuleFilePath2); ModuleDiscovery::DeclareRemovedExtensions([$oExtension]); $sExpectedMessage = <<assertEquals([$expectedName, $expectedVersion], ModuleDiscovery::GetModuleName($sModuleId)); } - public function testIsModulePartOfRemovedExtension_NoRemovedExtension() + public function testIsModuleInExtensionList_NoRemovedExtension() { - ModuleDiscovery::DeclareRemovedExtensions([]); - $this->assertFalse($this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModulePartOfRemovedExtension", ['module_name', '123', []])); + $this->assertFalse($this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModuleInExtensionList", [[], 'module_name', '123', []])); } - public function testIsModulePartOfRemovedExtension_ModuleWithAnotherVersionIncludedInRemoveExtension() + public function testIsModuleInExtensionList_ModuleWithAnotherVersionIncludedInRemoveExtension() { - $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $this->AssertModuleIsPartOfRemovedExtension('module_name', '123', $sModuleFilePath, $sModuleFilePath, false); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)]; + $this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '123', $sModuleFilePath, false); } - public function testIsModulePartOfRemovedExtension_AnotherModuleWithSameVersionIncludedInRemoveExtension() + public function testIsModuleInExtensionList_AnotherModuleWithSameVersionIncludedInRemoveExtension() { - $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $this->AssertModuleIsPartOfRemovedExtension('another_module_name', '456', $sModuleFilePath, $sModuleFilePath, false); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)]; + $this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'another_module_name', '456', $sModuleFilePath, false); } - public function testIsModulePartOfRemovedExtension_SameExtensionComingFromAnotherLocation() + public function testIsModuleInExtensionList_SameExtensionComingFromAnotherLocation() { - $sModuleFilePath = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $sModuleFilePath2 = $this->GetTemporaryFilePath('modulediscoverytest_modulefilepath'); - $this->AssertModuleIsPartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath2, false); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $sModuleFilePath2 = $this->GetTemporaryFilePath(); + $aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath2)]; + $this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '456', $sModuleFilePath, false); } - public function testIsModulePartOfRemovedExtension_ModuleShouldBeExcluded() + public function testIsModuleInExtensionList_ModuleShouldBeExcluded() { - $sModuleFilePath = tempnam(sys_get_temp_dir(), 'discovery_test'); - $this->AssertModuleIsPartOfRemovedExtension('module_name', '456', $sModuleFilePath, $sModuleFilePath, true); + $sModuleFilePath = $this->GetTemporaryFilePath(); + $aExtensionList = [$this->GivenExtensionWithModule('module_name', '456', $sModuleFilePath)]; + $this->AssertModuleIsPartOfRemovedExtension($aExtensionList, 'module_name', '456', $sModuleFilePath, true); } - public function AssertModuleIsPartOfRemovedExtension($sModuleName, $sModuleVersion, $sModuleFilePath, $sModuleFilePathIncludedInRemovedExtension, $bExpected) + public function AssertModuleIsPartOfRemovedExtension($aExtensionList, $sModuleName, $sModuleVersion, $sModuleFilePath, $bExpected) { - $oExtension = $this->CreateExtensionWithModule('module_name', '456', $sModuleFilePathIncludedInRemovedExtension); - ModuleDiscovery::DeclareRemovedExtensions([$oExtension]); $aCurrentModuleInfo = [ ModuleFileReader::MODULE_FILE_PATH => $sModuleFilePath, ]; $this->assertEquals( $bExpected, - $this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModulePartOfRemovedExtension", [$sModuleName, $sModuleVersion, $aCurrentModuleInfo]) + $this->InvokeNonPublicStaticMethod(ModuleDiscovery::class, "IsModuleInExtensionList", [$aExtensionList, $sModuleName, $sModuleVersion, $aCurrentModuleInfo]) ); } - private function CreateExtensionWithModule(string $sModuleName, string $sVersion, bool|string $sModuleFilePath): iTopExtension + private function GivenExtensionWithModule(string $sModuleName, string $sVersion, bool|string $sModuleFilePath): iTopExtension { $oExt = new iTopExtension(); $oExt->aModuleVersion[$sModuleName] = $sVersion; diff --git a/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php b/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php index 384f72304..4fe001494 100644 --- a/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php +++ b/tests/php-unit-tests/unitary-tests/setup/feature_removal/SetupAuditTest.php @@ -3,12 +3,11 @@ namespace Combodo\iTop\Test\UnitTest\Setup\FeatureRemoval; use Combodo\iTop\Setup\FeatureRemoval\DryRemovalRuntimeEnvironment; +use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit; use Combodo\iTop\Setup\FeatureRemoval\ModelReflectionSerializer; use Combodo\iTop\Setup\FeatureRemoval\SetupAudit; -use Combodo\iTop\Setup\FeatureRemoval\InplaceSetupAudit; use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase; use Combodo\iTop\Test\UnitTest\Service\UnitTestRunTimeEnvironment; -use Exception; use MetaModel; class SetupAuditTest extends ItopCustomDatamodelTestCase @@ -79,7 +78,7 @@ class SetupAuditTest extends ItopCustomDatamodelTestCase $aClassesBeforeRemoval[] = "GabuZomeu"; $oSetupAudit = new InplaceSetupAudit($aClassesBeforeRemoval, $sEnv); - $oSetupAudit->ComputeClasses($aClassesBeforeRemoval); + $oSetupAudit->ComputeClasses(); $this->assertEquals(["GabuZomeu"], $oSetupAudit->GetRemovedClasses()); }