composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-06-04 15:12:18 +02:00
Code Issues Packages Projects Releases Wiki Activity

N°9553 - Refactor methods to be clearer on what it does

Browse Source
This commit is contained in:
v-dumas
2026-06-02 17:36:01 +02:00
parent b68df9ec07
commit 03fd49f688
7 changed files with 70 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
datamodels/2.x/itop-config-mgmt/module.itop-config-mgmt.php
View File

@@ -26,7 +26,6 @@ SetupWebPage::AddModule(
],
'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',
],
@@ -102,6 +101,8 @@ if (!class_exists('ConfigMgmtInstaller')) {
*/
public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousVersion, $sCurrentVersion)
{
// load localized data for NetworkDeviceType
static::LoadLocalizedDataOnCrossingVersion($oConfiguration, $sPreviousVersion, $sCurrentVersion,'3.3.0',__DIR__."/data/{{language_code}}.data.itop-networkdevicetype.xml" );
}
}
}

8
datamodels/2.x/itop-service-mgmt-provider/module.itop-service-mgmt-provider.php
View File

@@ -88,13 +88,7 @@ if (!class_exists('ServiceMgmtProviderInstaller')) {
public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousVersion, $sCurrentVersion)
{
// Load localized structural data: contract types
static::LoadLocalizedData(
$oConfiguration,
$sPreviousVersion,
$sCurrentVersion,
'3.3.0',
__DIR__."/data/{{language_code}}.data.itop-contracttype.xml"
);
static::LoadLocalizedDataOnNewInstall($oConfiguration, $sPreviousVersion, __DIR__."/data/{{language_code}}.data.itop-contracttype.xml");
}
}
}

8
datamodels/2.x/itop-service-mgmt/module.itop-service-mgmt.php
View File

@@ -85,13 +85,7 @@ if (!class_exists('ServiceMgmtInstaller')) {
public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousVersion, $sCurrentVersion)
{
// Load localized structural data: contact types and document types
static::LoadLocalizedData(
$oConfiguration,
$sPreviousVersion,
$sCurrentVersion,
'3.3.0',
__DIR__."/data/{{language_code}}.data.itop-contracttype.xml"
);
static::LoadLocalizedDataOnNewInstall($oConfiguration, $sPreviousVersion, __DIR__."/data/{{language_code}}.data.itop-contracttype.xml");
}
}
}

16
datamodels/2.x/itop-structure/module.itop-structure.php
View File

@@ -100,20 +100,8 @@ if (!class_exists('StructureInstaller')) {
public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousVersion, $sCurrentVersion)
{
// Load localized structural data: contact types and document types
static::LoadLocalizedData(
$oConfiguration,
$sPreviousVersion,
$sCurrentVersion,
'3.3.0',
__DIR__."/data/{{language_code}}.data.itop-contacttype.xml"
);
static::LoadLocalizedData(
$oConfiguration,
$sPreviousVersion,
$sCurrentVersion,
'3.3.0',
__DIR__."/data/{{language_code}}.data.itop-documenttype.xml"
);
static::LoadLocalizedDataOnNewInstall($oConfiguration, $sPreviousVersion, __DIR__."/data/{{language_code}}.data.itop-contacttype.xml");
static::LoadLocalizedDataOnNewInstall($oConfiguration, $sPreviousVersion, __DIR__."/data/{{language_code}}.data.itop-documenttype.xml");
// Default language will be used for actions
// Note: There is a issue when upgrading, default language cannot be retrieved from the passed configuration, we have to read it from the disk

2
datamodels/2.x/itop-tickets/module.itop-tickets.php
View File

@@ -61,6 +61,6 @@ class TicketsInstaller extends ModuleInstallerAPI
}
}
// Load localized structural data: predefined query phrases for notifications
static::LoadLocalizedData($oConfiguration, $sPreviousVersion, $sCurrentVersion, '3.0.0', __DIR__."/data/{{language_code}}.data.itop-tickets.xml");
static::LoadLocalizedDataOnCrossingVersion($oConfiguration, $sPreviousVersion, $sCurrentVersion, '3.0.0', __DIR__."/data/{{language_code}}.data.itop-tickets.xml");
}
}

83
setup/moduleinstaller.class.inc.php
View File

@@ -321,9 +321,9 @@ abstract class ModuleInstallerAPI
* @throws \CoreException
* @throws \CoreUnexpectedValue
*/
public static function LoadLocalizedData(Config $oConfiguration, ?string $sPreviousVersion, ?string $sCurrentVersion, string $sFirstLoadingVersion, string $sFilePattern): void
public static function LoadLocalizedDataOnCrossingVersion(Config $oConfiguration, ?string $sPreviousVersion, ?string $sCurrentVersion, string $sFirstLoadingVersion, string $sFilePattern): void
{
self::AssertLoadLocalizedDataParametersAreValid($sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion, $sFilePattern);
self::AssertLoadLocalizedDataParametersAreValid($sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion);
// The loading is done only if
// - it's a first install of the module
@@ -331,47 +331,41 @@ abstract class ModuleInstallerAPI
// - 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, '<') &&
(($sFirstLoadingVersion === '') || version_compare($sPreviousVersion, $sFirstLoadingVersion, '<')))) {
(version_compare($sPreviousVersion, $sCurrentVersion, '<')
&& version_compare($sPreviousVersion, $sFirstLoadingVersion, '<')
&& version_compare($sFirstLoadingVersion, $sCurrentVersion, '<='))) {
$sDefaultLanguage = $oConfiguration->GetDefaultLanguage();
$sFileName = self::GetLocalizedFileName($sDefaultLanguage, $sFilePattern);
self::XMLFileLoad($sFileName);
self::LoadLocalizedData($oConfiguration, $sFilePattern);
}
}
/**
* @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)) {
self::LoadLocalizedData($oConfiguration, $sFilePattern);
}
}
/**
* @throws \CoreUnexpectedValue
*/
private static function AssertLoadLocalizedDataParametersAreValid(?string $sPreviousVersion, ?string $sCurrentVersion, string $sFirstLoadingVersion, string $sFilePattern): 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}'");
}
if (substr_count($sFilePattern, '{{language_code}}') !== 1) {
throw new CoreUnexpectedValue("LoadLocalizedData expects $sFilePattern to contain the exact placeholder '{{language_code}}' exactly once");
}
}
private static function IsValidLocalizedDataVersion(string $sVersion): bool
{
return (preg_match('/^\d+\.\d+(?:\.\d+)?(?:-[A-Za-z0-9]+)?$/', $sVersion) === 1);
}
/**
* @param array|string $sFileName
* @param \Config $oConfiguration
* @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
* @throws \Exception
*/
public static function XMLFileLoad(string $sFileName): void
protected static function LoadLocalizedData(Config $oConfiguration, string $sFilePattern): void
{
if (substr_count($sFilePattern, '{{language_code}}') !== 1) {
throw new CoreUnexpectedValue("LoadLocalizedData expects $sFilePattern to contain the exact placeholder '{{language_code}}' exactly once");
}
$sDefaultLanguage = $oConfiguration->GetDefaultLanguage();
$sFileName = self::GetLocalizedFileName($sDefaultLanguage, $sFilePattern);
if (!file_exists($sFileName)) {
throw new Exception("File $sFileName not found");
}
@@ -384,13 +378,34 @@ 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);
}
/**
* @param string $sLanguage The language code to use for localization (e.g. 'EN US')
* @param string $sFilePattern The full path+name of the file to localize, with {{language_code}} as placeholder for the language code (e.g. 'data.sample.{{language_code}}.xml')
*
* @return string The localized file name if found, or an empty string if not found
*/
public static function GetLocalizedFileName($sLanguage, string $sFilePattern): string
private static function GetLocalizedFileName($sLanguage, string $sFilePattern): string
{
$sLang = str_replace(' ', '_', strtolower($sLanguage));
$sFileName = str_replace('{{language_code}}', $sLang, $sFilePattern);

39
tests/php-unit-tests/unitary-tests/setup/ModuleInstallerAPITest.php
View File

@@ -285,7 +285,7 @@ SQL
}
/**
* @covers \ModuleInstallerAPI::LoadLocalizedData
* @covers \ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion
* @dataProvider LoadLocalizedData_RequiredLanguageProvider
*/
public function testLoadLocalizedData_LoadRequiredLanguageOnFirstInstall(string $sRequiredLanguage, array $aAvailableLanguages, array $aExpectedCountByLanguage): void
@@ -294,7 +294,7 @@ SQL
[$oConfig, $sOrgName, $sPattern] = $this->GivenLocalizedDataTestContext('XML_Load_RequiredLanguage_', $sRequiredLanguage, $aAvailableLanguages);
// When no previous version, and current version higher than the first loading version
ModuleInstallerAPI::LoadLocalizedData($oConfig, '', '3.3.0', '3.0.0', $sPattern);
ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '', '3.3.0', '3.0.0', $sPattern);
// Then data loaded
foreach ($aExpectedCountByLanguage as $sLanguage => $iExpectedCount) {
@@ -334,7 +334,7 @@ SQL
}
/**
* @covers \ModuleInstallerAPI::LoadLocalizedData
* @covers \ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion
* @dataProvider LoadLocalizedData_VersionConditionNotMetProvider
*/
public function testLoadLocalizedData_DoesNotLoadWhenVersionConditionIsNotMet(string $sPreviousVersion, string $sCurrentVersion, string $sFirstLoadingVersion): void
@@ -342,7 +342,7 @@ SQL
// Given
[$oConfig, $sOrgName, $sPattern] = $this->GivenLocalizedDataTestContext('XML_Load_NoLoad_', 'en_us', ['en_us']);
// When version gate conditions are not met
ModuleInstallerAPI::LoadLocalizedData($oConfig, $sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion, $sPattern);
ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, $sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion, $sPattern);
// Then no data loaded
$this->AssertOrganizationCountByName($sOrgName, 'en_us', 0);
}
@@ -354,31 +354,20 @@ SQL
'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', ''],
];
}
/**
* @covers \ModuleInstallerAPI::LoadLocalizedData
*/
public function testLoadLocalizedData_AlwaysLoadWhenFistLoadingVersionEmpty(): void
{
// Given
[$oConfig, $sOrgName, $sPattern] = $this->GivenLocalizedDataTestContext('XML_Load_NoLoad_', 'en_us', ['en_us']);
// When a previous version that is lower than the first loading version, but higher or equal to the current version
ModuleInstallerAPI::LoadLocalizedData($oConfig, '3.0.0', '3.1.0', '', $sPattern);
// Then no data loaded
$this->AssertOrganizationCountByName($sOrgName, 'en_us', 1);
}
/**
* @covers \ModuleInstallerAPI::LoadLocalizedData
* @covers \ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion
* @dataProvider LoadLocalizedData_ValidVersionFormatsProvider
*/
public function testLoadLocalizedData_AcceptsSupportedVersionFormats(string $sCurrentVersion, string $sFirstLoadingVersion): void
{
[$oConfig, $sOrgName, $sPattern] = $this->GivenLocalizedDataTestContext('XML_Load_ValidVersion_', 'en_us', ['en_us']);
ModuleInstallerAPI::LoadLocalizedData($oConfig, '', $sCurrentVersion, $sFirstLoadingVersion, $sPattern);
ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '', $sCurrentVersion, $sFirstLoadingVersion, $sPattern);
$this->AssertOrganizationCountByName($sOrgName, 'en_us', 1);
}
@@ -387,8 +376,8 @@ SQL
{
return [
'Current version with suffix' => ['3.2-dev', '3.0.0'],
'Current version x.y.z' => ['1.2.4', '1.0'],
'Current version x.y.z-suffix' => ['2.3.3-beta', '2.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'],
];
}
@@ -399,15 +388,15 @@ 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::LoadLocalizedData($oConfig, '', '3.1.0', '3.0.0', $sPattern);
ModuleInstallerAPI::LoadLocalizedData($oConfig, '3.1.0', '3.2.0', '', $sPattern);
ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '', '3.1.0', '3.0.0', $sPattern);
ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, '3.1.0', '3.2.0', '', $sPattern);
// Then no duplicate data loaded
$this->AssertOrganizationCountByName($sOrgName, 'en_us', 1);
}
/**
* @covers \ModuleInstallerAPI::LoadLocalizedData
* @covers \ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion
* @dataProvider LoadLocalizedData_InvalidParametersProvider
*/
public function testLoadLocalizedData_ThrowsOnInvalidParameters(string $sPreviousVersion, string $sCurrentVersion, string $sFirstLoadingVersion, string $sPattern, string $sExpectedMessage): void
@@ -418,7 +407,7 @@ SQL
$this->expectException(\CoreUnexpectedValue::class);
$this->expectExceptionMessage($sExpectedMessage);
ModuleInstallerAPI::LoadLocalizedData($oConfig, $sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion, $sPattern);
ModuleInstallerAPI::LoadLocalizedDataOnCrossingVersion($oConfig, $sPreviousVersion, $sCurrentVersion, $sFirstLoadingVersion, $sPattern);
}
public function LoadLocalizedData_InvalidParametersProvider(): array