From 1e3ef6846d5ec6693cdbe807cef0d357bce66df5 Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Mon, 22 Jan 2024 16:02:41 +0100 Subject: [PATCH] =?UTF-8?q?N=C2=B03677=20-=20Fix=20AttributeImage.default?= =?UTF-8?q?=5Fimage=20URLs=20not=20up=20to=20date=20after=20app=5Froot=5Fu?= =?UTF-8?q?rl=20change=20(#526)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Many thanks to @sg-gcouronne for this contribution ! --- application/utils.inc.php | 45 ++++++++++--------- core/attributedef.class.inc.php | 14 ++++++ setup/compiler.class.inc.php | 2 +- .../unitary-tests/application/utilsTest.php | 6 +-- .../unitary-tests/core/AttributeImageTest.php | 43 ++++++++++++++++++ 5 files changed, 85 insertions(+), 25 deletions(-) create mode 100644 tests/php-unit-tests/unitary-tests/core/AttributeImageTest.php diff --git a/application/utils.inc.php b/application/utils.inc.php index 3355b9286..e67f5bff1 100644 --- a/application/utils.inc.php +++ b/application/utils.inc.php @@ -23,6 +23,7 @@ use Combodo\iTop\Application\UI\Base\Layout\UIContentBlock; use Combodo\iTop\Application\UI\Hook\iKeyboardShortcut; use Combodo\iTop\Application\WebPage\WebPage; use Combodo\iTop\Service\Module\ModuleService; +use Combodo\iTop\Test\UnitTest\Application\utilsTest; use ScssPhp\ScssPhp\Compiler; use ScssPhp\ScssPhp\OutputStyle; use ScssPhp\ScssPhp\ValueConverter; @@ -167,7 +168,11 @@ class utils private static $iNextId = 0; - private static $m_sAppRootUrl = null; + /** + * @var ?string + * @used-by GetAbsoluteUrlAppRoot + */ + private static $sAbsoluteUrlAppRootCache = null; protected static function LoadParamFile($sParamFile) { @@ -1037,28 +1042,27 @@ class utils return $bTrustProxies; } - /** - * Returns the absolute URL to the application root path - * - * @param bool $bForceTrustProxy - * - * @return string The absolute URL to the application root, without the first slash - * - * @throws \Exception - * - * @since 2.7.4 $bForceTrustProxy param added - */ + /** + * Returns the absolute URL to the application root path + * + * @param bool $bForceTrustProxy + * + * @return string The absolute URL to the application root, without the first slash + * + * @throws \Exception + * + * @since 2.7.4 $bForceTrustProxy param added + */ public static function GetAbsoluteUrlAppRoot($bForceTrustProxy = false) { - $sUrl = static::$m_sAppRootUrl; - if ($sUrl === null || $bForceTrustProxy) + if (static::$sAbsoluteUrlAppRootCache === null || $bForceTrustProxy) { - $sUrl = self::GetConfig()->Get('app_root_url'); - if ($sUrl == '') + static::$sAbsoluteUrlAppRootCache = self::GetConfig()->Get('app_root_url'); + if (static::$sAbsoluteUrlAppRootCache == '') { - $sUrl = self::GetDefaultUrlAppRoot($bForceTrustProxy); + static::$sAbsoluteUrlAppRootCache = self::GetDefaultUrlAppRoot($bForceTrustProxy); } - elseif (strpos($sUrl, SERVER_NAME_PLACEHOLDER) > -1) + elseif (strpos(static::$sAbsoluteUrlAppRootCache, SERVER_NAME_PLACEHOLDER) > -1) { if (isset($_SERVER['SERVER_NAME'])) { @@ -1069,11 +1073,10 @@ class utils // CLI mode ? $sServerName = php_uname('n'); } - $sUrl = str_replace(SERVER_NAME_PLACEHOLDER, $sServerName, $sUrl); + static::$sAbsoluteUrlAppRootCache = str_replace(SERVER_NAME_PLACEHOLDER, $sServerName, static::$sAbsoluteUrlAppRootCache); } - static::$m_sAppRootUrl = $sUrl; } - return static::$m_sAppRootUrl; + return static::$sAbsoluteUrlAppRootCache; } /** diff --git a/core/attributedef.class.inc.php b/core/attributedef.class.inc.php index 383524e0f..764dae3ab 100644 --- a/core/attributedef.class.inc.php +++ b/core/attributedef.class.inc.php @@ -8654,6 +8654,20 @@ class AttributeImage extends AttributeBlob parent::__construct($sCode, $aParams); } + public function Get($sParamName) + { + $oParamValue = parent::Get($sParamName); + + if ($sParamName === 'default_image') { + /** @noinspection NestedPositiveIfStatementsInspection */ + if (!empty($oParamValue)) { + return utils::GetAbsoluteUrlModulesRoot() . $oParamValue; + } + } + + return $oParamValue; + } + public function GetEditClass() { return "Image"; diff --git a/setup/compiler.class.inc.php b/setup/compiler.class.inc.php index b9c7cbf46..22546d72c 100644 --- a/setup/compiler.class.inc.php +++ b/setup/compiler.class.inc.php @@ -2348,7 +2348,7 @@ EOF break; case 'default_image': if (($sDefault = $oField->GetChildText('default_image')) && (strlen($sDefault) > 0)) { - $aParameters['default_image'] = "utils::GetAbsoluteUrlModulesRoot().'$sModuleRelativeDir/$sDefault'"; + $aParameters['default_image'] = "'$sModuleRelativeDir/$sDefault'"; } else { $aParameters['default_image'] = 'null'; } diff --git a/tests/php-unit-tests/unitary-tests/application/utilsTest.php b/tests/php-unit-tests/unitary-tests/application/utilsTest.php index c0a173b00..c122db8aa 100644 --- a/tests/php-unit-tests/unitary-tests/application/utilsTest.php +++ b/tests/php-unit-tests/unitary-tests/application/utilsTest.php @@ -248,6 +248,9 @@ class utilsTest extends ItopTestCase */ public function testGetAbsoluteUrlAppRootPersistency($bBehindReverseProxy,$bForceTrustProxy1 ,$sExpectedAppRootUrl1,$bForceTrustProxy2 , $sExpectedAppRootUrl2,$bForceTrustProxy3 , $sExpectedAppRootUrl3) { + // resetting static property for each test pass + $this->SetNonPublicStaticProperty(utils::class, 'sAbsoluteUrlAppRootCache', null); + utils::GetConfig()->Set('behind_reverse_proxy', $bBehindReverseProxy); utils::GetConfig()->Set('app_root_url', ''); @@ -272,9 +275,6 @@ class utilsTest extends ItopTestCase $this->assertEquals($sExpectedAppRootUrl2, utils::GetAbsoluteUrlAppRoot($bForceTrustProxy2)); $this->assertEquals($sExpectedAppRootUrl3, utils::GetAbsoluteUrlAppRoot($bForceTrustProxy3)); - - // Leave the place clean - static::SetNonPublicStaticProperty('utils', 'm_sAppRootUrl', null); } diff --git a/tests/php-unit-tests/unitary-tests/core/AttributeImageTest.php b/tests/php-unit-tests/unitary-tests/core/AttributeImageTest.php new file mode 100644 index 000000000..6a6aa5b16 --- /dev/null +++ b/tests/php-unit-tests/unitary-tests/core/AttributeImageTest.php @@ -0,0 +1,43 @@ +RequireOnceItopFile('core/attributedef.class.inc.php'); + } + + public function testGet(): void + { + $oConfig = utils::GetConfig(); + $oPersonPictureAttDef = MetaModel::GetAttributeDef(Person::class, 'picture'); + + $sAppRootUrl1 = 'http://localhost/iTop/'; + $this->SetNewAppRootUrl($oConfig, $sAppRootUrl1); + $sPersonPictureDefaultImageUrl = $oPersonPictureAttDef->Get('default_image'); + $this->assertStringStartsWith($sAppRootUrl1, $sPersonPictureDefaultImageUrl); + + $sAppRootUrl2 = 'https://demo.combodo.com/simple/'; + $this->SetNewAppRootUrl($oConfig, $sAppRootUrl2); + $oConfig->Set('app_root_url', $sAppRootUrl2); + $sPersonPictureDefaultImageUrl = $oPersonPictureAttDef->Get('default_image'); + $this->assertStringStartsWith($sAppRootUrl2, $sPersonPictureDefaultImageUrl); + } + + /** + * Note that we need to reset manually the cache in \utils::GetAbsoluteUrlAppRoot, which is called from \AttributeImage::Get + */ + private function SetNewAppRootUrl(Config $oConfig, string $sAppRootUrl):void { + $oConfig->Set('app_root_url', $sAppRootUrl); + $this->SetNonPublicStaticProperty(utils::class, 'sAbsoluteUrlAppRootCache', null); + } +}