diff --git a/datamodels/2.x/itop-config-mgmt/data/data.sample.networkdevicetype.xml b/datamodels/2.x/itop-config-mgmt/data/data.sample.networkdevicetype.xml deleted file mode 100644 index bf2008bfc2..0000000000 --- a/datamodels/2.x/itop-config-mgmt/data/data.sample.networkdevicetype.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - Router - - - Switch - - diff --git a/datamodels/2.x/itop-config-mgmt/module.itop-config-mgmt.php b/datamodels/2.x/itop-config-mgmt/module.itop-config-mgmt.php index 440e6e8da7..88b15db821 100755 --- a/datamodels/2.x/itop-config-mgmt/module.itop-config-mgmt.php +++ b/datamodels/2.x/itop-config-mgmt/module.itop-config-mgmt.php @@ -25,14 +25,13 @@ SetupWebPage::AddModule( 'main.itop-config-mgmt.php', ], 'data.struct' => [ - 'data/en_us.data.itop-brand.xml', - 'data/en_us.data.itop-networkdevicetype.xml', - 'data/en_us.data.itop-osfamily.xml', - 'data/en_us.data.itop-osversion.xml', + 'data/data.itop-brand.xml', + 'data/data.itop-networkdevicetype.xml', + 'data/data.itop-osfamily.xml', + 'data/data.itop-osversion.xml', ], 'data.sample' => [ 'data/data.sample.model.xml', - 'data/data.sample.networkdevicetype.xml', 'data/data.sample.servers.xml', 'data/data.sample.nw-devices.xml', 'data/data.sample.software.xml', diff --git a/datamodels/2.x/itop-structure/data/data.sample.contacttype.xml b/datamodels/2.x/itop-structure/data/data.sample.contacttype.xml deleted file mode 100644 index 5d3f7a52b5..0000000000 --- a/datamodels/2.x/itop-structure/data/data.sample.contacttype.xml +++ /dev/null @@ -1,21 +0,0 @@ - - - - Customer manager - - - Helpdesk - - - Manager - - - Support Agent - - - Support level1 - - - Team leader - - diff --git a/datamodels/2.x/itop-structure/module.itop-structure.php b/datamodels/2.x/itop-structure/module.itop-structure.php index cea9e5c7b3..7e1a389e91 100644 --- a/datamodels/2.x/itop-structure/module.itop-structure.php +++ b/datamodels/2.x/itop-structure/module.itop-structure.php @@ -32,7 +32,6 @@ SetupWebPage::AddModule( 'data/data.sample.persons.xml', 'data/data.sample.teams.xml', 'data/data.sample.contactteam.xml', - 'data/data.sample.contacttype.xml', 'data/data.sample.auditdomain.xml', 'data/data.sample.auditcategory.xml', 'data/data.sample.auditcategory-auditdomain.xml', diff --git a/datamodels/2.x/itop-tickets/module.itop-tickets.php b/datamodels/2.x/itop-tickets/module.itop-tickets.php index 973309835e..8b407ca25e 100755 --- a/datamodels/2.x/itop-tickets/module.itop-tickets.php +++ b/datamodels/2.x/itop-tickets/module.itop-tickets.php @@ -61,6 +61,6 @@ class TicketsInstaller extends ModuleInstallerAPI } } // Load localized structural data: predefined query phrases for notifications - static::LoadLocalizedDataOnCrossingVersion($oConfiguration, $sPreviousVersion, $sCurrentVersion, '3.0.0', __DIR__."/data/data.itop-tickets.en_us.xml"); + static::LoadLocalizedDataOnCrossingVersion($oConfiguration, $sPreviousVersion, $sCurrentVersion, '3.0.0', utils::GetAbsoluteModulePath('itop-tickets')."/data/data.itop-tickets.en_us.xml"); } } diff --git a/setup/moduleinstaller.class.inc.php b/setup/moduleinstaller.class.inc.php index d62caa6552..62d5f0907a 100644 --- a/setup/moduleinstaller.class.inc.php +++ b/setup/moduleinstaller.class.inc.php @@ -326,36 +326,11 @@ abstract class ModuleInstallerAPI public static function LoadLocalizedDataOnCrossingVersion(Config $oConfiguration, ?string $sPreviousVersion, ?string $sCurrentVersion, string $sFirstLoadingVersion, string $sDefaultFileName): void { self::AssertLoadLocalizedDataParametersAreValid($sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion); - - // The loading is done only if - // - it's a first install of the module - // - or it's an upgrade of that module (PreviousVersion is less than the CurrentVersion), which means that we are really upgrading (and not reinstalling the same version or downgrading), and - // - either the FirstLoadingVersion is between the PreviousVersion and the CurrentVersion - // - or the FirstLoadingVersion is empty, forcing the loading on all upgrades, - if (($sPreviousVersion === '') || - (version_compare($sPreviousVersion, $sCurrentVersion, '<') - && version_compare($sPreviousVersion, $sFirstLoadingVersion, '<') - && version_compare($sFirstLoadingVersion, $sCurrentVersion, '<='))) { - + if (self::IsVersionCrossed($sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion)) { $sWishedLanguage = $oConfiguration->GetDefaultLanguage(); self::LoadLocalizedData($sWishedLanguage, $sDefaultFileName); } } - /** - * Helper which will be removed as the standard knows how to load localized data on first install, but this is kept for backward compatibility. - * @param \Config $oConfiguration - * @param string $sPreviousVersion The previous version of the module (empty string in case of first install) - * @param string $sFilePattern The pattern of the file to load, with {{language_code}} as placeholder for the language code (e.g. 'data.sample.{{language_code}}.xml') - * - * @return void - */ - public static function LoadLocalizedDataOnNewInstall(Config $oConfiguration, ?string $sPreviousVersion, string $sFilePattern): void - { - if (utils::IsNullOrEmptyString($sPreviousVersion)) { - $sWishedLanguage = $oConfiguration->GetDefaultLanguage(); - self::LoadLocalizedData($sWishedLanguage, $sFilePattern); - } - } /** * Helper to load a localized data file based on the default language of the application. @@ -381,27 +356,6 @@ abstract class ModuleInstallerAPI $oDataLoader->EndSession(); } - /** - * @throws \CoreUnexpectedValue - */ - private static function AssertLoadLocalizedDataParametersAreValid(?string $sPreviousVersion, ?string $sCurrentVersion, string $sFirstLoadingVersion): void - { - if (($sPreviousVersion !== '') && !self::IsValidLocalizedDataVersion($sPreviousVersion)) { - throw new CoreUnexpectedValue("LoadLocalizedData expects sPreviousVersion to be empty or match x.y[.z][-name], got '{$sPreviousVersion}'"); - } - if (!self::IsValidLocalizedDataVersion($sCurrentVersion)) { - throw new CoreUnexpectedValue("LoadLocalizedData expects sCurrentVersion to match x.y[.z][-name], got '{$sCurrentVersion}'"); - } - if (($sFirstLoadingVersion !== '') && !self::IsValidLocalizedDataVersion($sFirstLoadingVersion)) { - throw new CoreUnexpectedValue("LoadLocalizedData expects sFirstLoadingVersion to match x.y[.z][-name], got '{$sFirstLoadingVersion}'"); - } - } - - private static function IsValidLocalizedDataVersion(string $sVersion): bool - { - return (preg_match('/^\d+\.\d+(?:\.\d+)?(?:-[A-Za-z0-9]+)?$/', $sVersion) === 1); - } - /** * Helper to get the localized file name for a given language code, based on the original file name which must end by .en_us.xml * @param string $sLanguage The language code to use for localization (e.g. 'FR FR' or 'fr_fr') @@ -431,4 +385,40 @@ abstract class ModuleInstallerAPI } return $sFileName; } + + /** + * @throws \CoreUnexpectedValue + */ + private static function AssertLoadLocalizedDataParametersAreValid(?string $sPreviousVersion, ?string $sCurrentVersion, string $sFirstLoadingVersion): void + { + if (($sPreviousVersion !== '') && !self::IsValidLocalizedDataVersion($sPreviousVersion)) { + throw new CoreUnexpectedValue("LoadLocalizedData expects sPreviousVersion to be empty or match x.y[.z][-name], got '{$sPreviousVersion}'"); + } + if (!self::IsValidLocalizedDataVersion($sCurrentVersion)) { + throw new CoreUnexpectedValue("LoadLocalizedData expects sCurrentVersion to match x.y[.z][-name], got '{$sCurrentVersion}'"); + } + if (($sFirstLoadingVersion !== '') && !self::IsValidLocalizedDataVersion($sFirstLoadingVersion)) { + throw new CoreUnexpectedValue("LoadLocalizedData expects sFirstLoadingVersion to match x.y[.z][-name], got '{$sFirstLoadingVersion}'"); + } + } + + private static function IsValidLocalizedDataVersion(string $sVersion): bool + { + return (preg_match('/^\d+\.\d+(?:\.\d+)?(?:-[A-Za-z0-9]+)?$/', $sVersion) === 1); + } + + /** + * @param string|null $sPreviousVersion + * @param string|null $sCurrentVersion + * @param string $sFirstLoadingVersion + * + * @return bool + */ + private static function IsVersionCrossed(?string $sPreviousVersion, ?string $sCurrentVersion, string $sFirstLoadingVersion): bool + { + return ($sPreviousVersion === '') || + (version_compare($sPreviousVersion, $sCurrentVersion, '<') + && version_compare($sPreviousVersion, $sFirstLoadingVersion, '<') + && version_compare($sFirstLoadingVersion, $sCurrentVersion, '<=')); + } } diff --git a/setup/runtimeenv.class.inc.php b/setup/runtimeenv.class.inc.php index 71e3c42314..31fde0810e 100644 --- a/setup/runtimeenv.class.inc.php +++ b/setup/runtimeenv.class.inc.php @@ -1264,7 +1264,7 @@ class RunTimeEnvironment if (!file_exists($sFileName)) { throw(new Exception("File $sFileName does not exist")); } - $sFileName = ModuleInstallerAPI::GetLocalizedFileName($sFileName, $sDefaultLanguage); + $sFileName = ModuleInstallerAPI::GetLocalizedFileName($sDefaultLanguage, $sFileName); $oDataLoader->LoadFile($sFileName); $sResult = sprintf("loading of %s done.", basename($sFileName)); SetupLog::Info($sResult); diff --git a/tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php b/tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php index da890b36c9..284e3e0c90 100644 --- a/tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php +++ b/tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php @@ -334,53 +334,41 @@ SQL } /** - * @covers \ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion - * @dataProvider LoadLocalizedData_VersionConditionNotMetProvider + * @covers \ModuleInstallerAPI::IsVersionCrossed + * @dataProvider IsVersionCrossedProvider */ - public function testLoadLocalizedData_DoesNotLoadWhenVersionConditionIsNotMet(string $sPreviousVersion, string $sCurrentVersion, string $sFirstLoadingVersion): void + public function testIsVersionCrossed_ReturnsExpectedValue(string $sPreviousVersion, string $sCurrentVersion, string $sFirstLoadingVersion, bool $bExpected): void { - // Given - [$oConfig, $sOrgName, $sPattern] = $this->GivenLocalizedDataTestContext('XML_Load_NoLoad_', 'en_us', ['en_us']); - // When version gate conditions are not met - ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, $sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion, $sPattern); - // Then no data loaded - $this->AssertOrganizationCountByName($sOrgName, 'en_us', 0); + $bIsVersionCrossed = $this->InvokeNonPublicStaticMethod(ModuleInstallerAPI::class, 'IsVersionCrossed', [$sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion]); + + $this->assertSame($bExpected, $bIsVersionCrossed); } - public function LoadLocalizedData_VersionConditionNotMetProvider(): array + public function IsVersionCrossedProvider(): array { return [ - 'Equal versions (reinstall)' => ['3.1.0', '3.1.0', '3.0.0'], - 'Downgrade attempt' => ['3.2.0', '3.1.0', '3.0.0'], - 'Upgrade but first loading version already passed' => ['3.1.0', '3.2.0', '3.0.0'], - 'Upgrade with boundary equality on first loading version' => ['3.0.0', '3.1.0', '3.0.0'], - 'Upgrade but first loading version empty' => ['3.1.0', '3.2.0', ''], - + 'First install always crosses' => ['', '3.2.0', '3.0.0', true], + 'Upgrade crosses first loading version' => ['3.0.0', '3.2.0', '3.1.0', true], + 'Upgrade but first loading version already passed' => ['3.1.0', '3.2.0', '3.0.0', false], + 'Reinstall same version does not cross' => ['3.1.0', '3.1.0', '3.0.0', false], + 'Downgrade does not cross' => ['3.2.0', '3.1.0', '3.0.0', false], + 'First install with suffixed current version' => ['', '3.2-dev', '3.0.0', true], + 'Upgrade with suffixed current version crosses' => ['1.0.3-2', '1.2.4-1', '1.1.0', true], + 'Upgrade with suffixed current version below threshold does not cross' => ['1.0.0', '1.2.0-beta', '1.2.0', false], ]; } /** * @covers \ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion - * @dataProvider LoadLocalizedData_ValidVersionFormatsProvider */ - public function testLoadLocalizedData_AcceptsSupportedVersionFormats(string $sCurrentVersion, string $sFirstLoadingVersion): void + public function testLoadLocalizedData_LoadsWhenVersionCrossingIsTrue(): void { - [$oConfig, $sOrgName, $sPattern] = $this->GivenLocalizedDataTestContext('XML_Load_ValidVersion_', 'en_us', ['en_us']); + [$oConfig, $sOrgName, $sPattern] = $this->GivenLocalizedDataTestContext('XML_Load_VersionCrossingTrue_', 'en_us', ['en_us']); - ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '', $sCurrentVersion, $sFirstLoadingVersion, $sPattern); + ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '3.0.0', '3.2.0', '3.1.0', $sPattern); $this->AssertOrganizationCountByName($sOrgName, 'en_us', 1); } - - public function LoadLocalizedData_ValidVersionFormatsProvider(): array - { - return [ - 'Current version with suffix' => ['3.2-dev', '3.0.0'], - 'Current version x.y.z' => ['10.12.140-Tagada34', '1.0'], - 'Current version x.y.z-suffix' => ['2.3.3-beta', '2.3.3-alpha'], - 'Current version x.y.z-1' => ['1.2.4-1', '1.0.3-2'], - ]; - } // Test when a file is loaded twice because of the version conditions, it doesn't create duplicates (idempotent loading) public function testLoadLocalizedData_IdempotentLoading(): void { @@ -388,8 +376,8 @@ SQL [$oConfig, $sOrgName, $sPattern] = $this->GivenLocalizedDataTestContext('XML_Load_Idempotent_', 'en_us', ['en_us']); // When LoadLocalizedData is called twice with conditions that would load the file both times - ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '', '3.1.0', '3.0.0', $sPattern); - ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '3.1.0', '3.2.0', '', $sPattern); + ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '', '3.0.1', '3.0.0', $sPattern); + ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '3.0.1', '3.1.0', '3.0.2', $sPattern); // Then no duplicate data loaded $this->AssertOrganizationCountByName($sOrgName, 'en_us', 1); @@ -420,30 +408,23 @@ SQL 'previous' => 'v3.2', 'current' => '3.2.0', 'first' => '3.0.0', - 'pattern' => $sTmpDir.DIRECTORY_SEPARATOR.'data.{{language_code}}.xml', + 'pattern' => $sTmpDir.DIRECTORY_SEPARATOR.'data.en_us.xml', 'message' => 'sPreviousVersion', ], 'Invalid current version format' => [ 'previous' => '', 'current' => '3', 'first' => '3.0.0', - 'pattern' => $sTmpDir.DIRECTORY_SEPARATOR.'data.{{language_code}}.xml', + 'pattern' => $sTmpDir.DIRECTORY_SEPARATOR.'data.en_us.xml', 'message' => 'sCurrentVersion', ], 'Invalid first loading version format' => [ 'previous' => '', 'current' => '3.2.0', 'first' => '3.0.0-beta.1', - 'pattern' => $sTmpDir.DIRECTORY_SEPARATOR.'data.{{language_code}}.xml', + 'pattern' => $sTmpDir.DIRECTORY_SEPARATOR.'data.en_us.xml', 'message' => 'sFirstLoadingVersion', ], - 'Missing strict placeholder' => [ - 'previous' => '', - 'current' => '3.2.0', - 'first' => '3.0.0', - 'pattern' => $sTmpDir.DIRECTORY_SEPARATOR.'data.{{LANGUAGE_CODE}}.xml', - 'message' => "{{language_code}}", - ], ]; } @@ -462,7 +443,7 @@ SQL $sTmpDir = static::CreateTmpdir(); $this->aFileToClean[] = $sTmpDir; - $sPattern = $sTmpDir.DIRECTORY_SEPARATOR.'data.{{language_code}}.xml'; + $sPattern = $sTmpDir.DIRECTORY_SEPARATOR.'data.en_us.xml'; foreach ($aAvailableLanguages as $sAvailableLanguage) { $this->GivenLocalizedDataFile($sTmpDir, $sAvailableLanguage, $sOrgName);