From 618d0b36aa35bbc194b7ec85d1e038eab00295ba Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 4 Feb 2024 13:43:40 +0100 Subject: [PATCH 01/18] wip: override config from env vars --- .../Mage/Core/Helper/EnvironmentLoader.php | 75 +++++++++++++++++++ app/code/core/Mage/Core/Model/App.php | 1 + app/code/core/Mage/Core/Model/Config.php | 16 ++++ 3 files changed, 92 insertions(+) create mode 100644 app/code/core/Mage/Core/Helper/EnvironmentLoader.php diff --git a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentLoader.php new file mode 100644 index 00000000000..865ace1e69d --- /dev/null +++ b/app/code/core/Mage/Core/Helper/EnvironmentLoader.php @@ -0,0 +1,75 @@ + $value) { + if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { + continue; + } + $configKey = str_replace(static::ENV_STARTS_WITH, '', $configKey); + list($_, $scope) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + switch ($scope) { + case static::CONFIG_KEY_DEFAULT: + list($_, $scope, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + $path = implode('/', [$section, $group, $field]); + $path = strtolower($path); + $scope = strtolower($scope); + + $xmlConfig->setNode($scope . '/' . $path, $value); + break; + case static::CONFIG_KEY_WEBSITES: + list($_, $_, $websiteCode, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + $path = implode('/', [$section, $group, $field]); + $path = strtolower($path); + $websiteCode = strtolower($websiteCode); + + $nodePath = sprintf('websites/%s/%s', $websiteCode, $path); + $xmlConfig->setNode($nodePath, $value); + break; + case static::CONFIG_KEY_STORES: + list($_, $_, $storeCode, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + $path = implode('/', [$section, $group, $field]); + $path = strtolower($path); + $storeCode = strtolower($storeCode); + $nodePath = sprintf('stores/%s/%s', $storeCode, $path); + $xmlConfig->setNode($nodePath, $value); + break; + } + } + } +} diff --git a/app/code/core/Mage/Core/Model/App.php b/app/code/core/Mage/Core/Model/App.php index a44360011fd..9989bd21744 100644 --- a/app/code/core/Mage/Core/Model/App.php +++ b/app/code/core/Mage/Core/Model/App.php @@ -442,6 +442,7 @@ protected function _initModules() Varien_Profiler::stop('mage::app::init::apply_db_schema_updates'); } $this->_config->loadDb(); + $this->_config->loadEnv(); $this->_config->saveCache(); } } finally { diff --git a/app/code/core/Mage/Core/Model/Config.php b/app/code/core/Mage/Core/Model/Config.php index b4498800c31..ef311fb3c80 100644 --- a/app/code/core/Mage/Core/Model/Config.php +++ b/app/code/core/Mage/Core/Model/Config.php @@ -320,6 +320,7 @@ public function init($options = []) $this->_useCache = false; $this->loadModules(); $this->loadDb(); + $this->loadEnv(); $this->saveCache(); } } finally { @@ -422,6 +423,21 @@ public function loadDb() return $this; } + /** + * Load environment variables and override config + * + * @return self + */ + public function loadEnv(): Mage_Core_Model_Config + { + if ($this->_isLocalConfigLoaded && Mage::isInstalled()) { + Varien_Profiler::start('config/load-env'); + Mage::helper('core/environmentLoader')->overrideEnvironment($this); + Varien_Profiler::stop('config/load-env'); + } + return $this; + } + /** * Reinitialize configuration * From 1824a0d32c7009ddc69aec1b055bee0362396512 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 4 Feb 2024 14:03:01 +0100 Subject: [PATCH 02/18] docs and phpcs changes --- .../Mage/Core/Helper/EnvironmentLoader.php | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentLoader.php index 865ace1e69d..641f1bab826 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentLoader.php @@ -21,15 +21,32 @@ */ class Mage_Core_Helper_EnvironmentLoader extends Mage_Core_Helper_Abstract { - const ENV_STARTS_WITH = 'OPENMAGE_CONFIG'; - const ENV_KEY_SEPARATOR = '__'; - const CONFIG_KEY_DEFAULT = 'DEFAULT'; - const CONFIG_KEY_WEBSITES = 'WEBSITES'; - const CONFIG_KEY_STORES = 'STORES'; + protected const ENV_STARTS_WITH = 'OPENMAGE_CONFIG'; + protected const ENV_KEY_SEPARATOR = '__'; + protected const CONFIG_KEY_DEFAULT = 'DEFAULT'; + protected const CONFIG_KEY_WEBSITES = 'WEBSITES'; + protected const CONFIG_KEY_STORES = 'STORES'; /** * Load configuration values from ENV variables into xml config object * + * Environment variables work on this schema: + * + * self::ENV_STARTS_WITH . self::ENV_KEY_SEPARATOR (OPENMAGE_CONFIG__) + * ^ Prefix (required) + * __ + * ^ Where scope is DEFAULT, WEBSITES__ or STORES__ + * + * ^ Where GROUP, SECTION and FIELD are separated by self::ENV_KEY_SEPARATOR + * + * Each example will override the 'general/store_information/name' value. + * Override from the default configuration: + * @example OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME=default + * Override the website 'base' configuration: + * @example OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME=website + * Override the store 'german' configuration: + * @example OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME=store_german + * * @param Mage_Core_Model_Config $xmlConfig * @return void */ @@ -45,7 +62,7 @@ public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) list($_, $scope) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); switch ($scope) { case static::CONFIG_KEY_DEFAULT: - list($_, $scope, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + list($_, $_, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); $path = implode('/', [$section, $group, $field]); $path = strtolower($path); $scope = strtolower($scope); From 1e1d30ff063b1b848ba3b00c9f233c5e7ac73b00 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 11 Feb 2024 23:52:50 +0100 Subject: [PATCH 03/18] feat: refactor config environment loader --- ...Loader.php => EnvironmentConfigLoader.php} | 62 ++++++++++++------- app/code/core/Mage/Core/Model/Config.php | 4 +- 2 files changed, 42 insertions(+), 24 deletions(-) rename app/code/core/Mage/Core/Helper/{EnvironmentLoader.php => EnvironmentConfigLoader.php} (63%) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php similarity index 63% rename from app/code/core/Mage/Core/Helper/EnvironmentLoader.php rename to app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 641f1bab826..47ba2303f9f 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -19,8 +19,9 @@ * @category Mage * @package Mage_Core */ -class Mage_Core_Helper_EnvironmentLoader extends Mage_Core_Helper_Abstract +class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract { + protected $_moduleName = 'Mage_Core'; protected const ENV_STARTS_WITH = 'OPENMAGE_CONFIG'; protected const ENV_KEY_SEPARATOR = '__'; protected const CONFIG_KEY_DEFAULT = 'DEFAULT'; @@ -52,41 +53,56 @@ class Mage_Core_Helper_EnvironmentLoader extends Mage_Core_Helper_Abstract */ public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) { - // override from env $env = getenv(); + foreach ($env as $configKey => $value) { if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { continue; } - $configKey = str_replace(static::ENV_STARTS_WITH, '', $configKey); - list($_, $scope) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + + $configKeyParts = array_filter(explode(static::ENV_KEY_SEPARATOR, str_replace(static::ENV_STARTS_WITH, '', $configKey)), 'trim'); + list($_, $scope) = $configKeyParts; + switch ($scope) { case static::CONFIG_KEY_DEFAULT: - list($_, $_, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); - $path = implode('/', [$section, $group, $field]); - $path = strtolower($path); - $scope = strtolower($scope); - - $xmlConfig->setNode($scope . '/' . $path, $value); + list($_, $_, $section, $group, $field) = $configKeyParts; + $path = $this->buildPath($section, $group, $field); + $xmlConfig->setNode($this->buildNodePath($scope, $path), $value); break; - case static::CONFIG_KEY_WEBSITES: - list($_, $_, $websiteCode, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); - $path = implode('/', [$section, $group, $field]); - $path = strtolower($path); - $websiteCode = strtolower($websiteCode); - $nodePath = sprintf('websites/%s/%s', $websiteCode, $path); - $xmlConfig->setNode($nodePath, $value); - break; + case static::CONFIG_KEY_WEBSITES: case static::CONFIG_KEY_STORES: - list($_, $_, $storeCode, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); - $path = implode('/', [$section, $group, $field]); - $path = strtolower($path); - $storeCode = strtolower($storeCode); - $nodePath = sprintf('stores/%s/%s', $storeCode, $path); + list($_, $_, $code, $section, $group, $field) = $configKeyParts; + $path = $this->buildPath($section, $group, $field); + $nodePath = sprintf('%s/%s/%s', strtolower($scope), strtolower($code), $path); $xmlConfig->setNode($nodePath, $value); break; } } } + + /** + * Build configuration path. + * + * @param string $section + * @param string $group + * @param string $field + * @return string + */ + public function buildPath($section, $group, $field): string + { + return strtolower(implode('/', [$section, $group, $field])); + } + + /** + * Build configuration node path. + * + * @param string $scope + * @param string $path + * @return string + */ + public function buildNodePath($scope, $path): string + { + return strtolower($scope) . '/' . $path; + } } diff --git a/app/code/core/Mage/Core/Model/Config.php b/app/code/core/Mage/Core/Model/Config.php index ef311fb3c80..e5a6fb86b6d 100644 --- a/app/code/core/Mage/Core/Model/Config.php +++ b/app/code/core/Mage/Core/Model/Config.php @@ -432,7 +432,9 @@ public function loadEnv(): Mage_Core_Model_Config { if ($this->_isLocalConfigLoaded && Mage::isInstalled()) { Varien_Profiler::start('config/load-env'); - Mage::helper('core/environmentLoader')->overrideEnvironment($this); + /** @var Mage_Core_Helper_EnvironmentConfigLoader $environmentConfigLoaderHelper */ + $environmentConfigLoaderHelper = Mage::helper('core/environmentConfigLoader'); + $environmentConfigLoaderHelper->overrideEnvironment($this); Varien_Profiler::stop('config/load-env'); } return $this; From b6c75edcf47bee722dadcfbe323139c373bc21e3 Mon Sep 17 00:00:00 2001 From: Flyingmana Date: Sun, 18 Feb 2024 21:11:48 +0100 Subject: [PATCH 04/18] add unittest and some changes for mocking and making the logic work --- .../Core/Helper/EnvironmentConfigLoader.php | 28 +++++++++++- .../Helper/EnvironmentConfigLoaderTest.php | 44 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php diff --git a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 47ba2303f9f..91d93acba0f 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -28,6 +28,8 @@ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract protected const CONFIG_KEY_WEBSITES = 'WEBSITES'; protected const CONFIG_KEY_STORES = 'STORES'; + protected array $envStore = []; + /** * Load configuration values from ENV variables into xml config object * @@ -53,14 +55,20 @@ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract */ public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) { - $env = getenv(); + $env = $this->getEnv(); foreach ($env as $configKey => $value) { if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { continue; } - $configKeyParts = array_filter(explode(static::ENV_KEY_SEPARATOR, str_replace(static::ENV_STARTS_WITH, '', $configKey)), 'trim'); + $configKeyParts = array_filter( + explode( + static::ENV_KEY_SEPARATOR, + $configKey + ), + 'trim' + ); list($_, $scope) = $configKeyParts; switch ($scope) { @@ -81,6 +89,22 @@ public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) } } + /** + * @internal method mostly for mocking + */ + public function setEnvStore(array $envStorage): void + { + $this->envStore = $envStorage; + } + + public function getEnv(): array + { + if (empty($this->envStore)) { + $this->envStore = getenv(); + } + return $this->envStore; + } + /** * Build configuration path. * diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php new file mode 100644 index 00000000000..7a62644497a --- /dev/null +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -0,0 +1,44 @@ +') + ); + + $helper->setEnvStore([ + $envName => $envValue + ]); + $helper->overrideEnvironment($config); + + $this->assertEquals( + $expectedValue, + $config->getNode($expectedPath) + ); + } +} From c6eabc328ad846dfa949af31868be57903b54af9 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 18 Feb 2024 23:07:15 +0100 Subject: [PATCH 05/18] tests: finish some more tests w/ website and store data overrides --- .../Helper/EnvironmentConfigLoaderTest.php | 155 +++++++++++++++--- 1 file changed, 132 insertions(+), 23 deletions(-) diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php index 7a62644497a..203c62f7f80 100644 --- a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -4,41 +4,150 @@ namespace OpenMage\Tests\Unit\Core\Helper; use PHPUnit\Framework\TestCase; +use Mage_Core_Helper_EnvironmentConfigLoader; +use Mage_Core_Model_Config; -class Security extends TestCase +class EnvironmentConfigLoaderTest extends TestCase { + protected const ENV_CONFIG_DEFAULT_PATH = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME'; + protected const ENV_CONFIG_DEFAULT_VALUE = 'default_new_value'; + protected const ENV_CONFIG_WEBSITE_PATH = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME'; + protected const ENV_CONFIG_WEBSITE_VALUE = 'website_new_value'; + protected const ENV_CONFIG_STORE_PATH = 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME'; + protected const ENV_CONFIG_STORE_VALUE = 'store_german_new_value'; - public function simpleEnvValueDataProvider(): array + public function setup(): void + { + \Mage::setRoot(''); + } + + public function testBuildPath() + { + $environmentConfigLoaderHelper = new Mage_Core_Helper_EnvironmentConfigLoader(); + $path = $environmentConfigLoaderHelper->buildPath('GENERAL', 'STORE_INFORMATION', 'NAME'); + $this->assertEquals('general/store_information/name', $path); + } + + public function testBuildNodePath() + { + $loader = new Mage_Core_Helper_EnvironmentConfigLoader(); + $nodePath = $loader->buildNodePath('DEFAULT', 'general/store_information/name'); + $this->assertEquals('default/general/store_information/name', $nodePath); + } + + /** + * @dataProvider envOverridesDataProvider + * + */ + public function testEnvOverrides(array $config) + { + error_reporting(0); + $xmlStruct = $this->getTestXml(); + + $xmlDefault = new Mage_Core_Model_Config($xmlStruct); + $xmlDefault = $xmlDefault->loadModulesConfiguration('config.xml', $xmlDefault); + $xml = new Mage_Core_Model_Config($xmlStruct); + $xml = $xml->loadModulesConfiguration('config.xml', $xml); + + $this->assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name')); + $this->assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name')); + $this->assertEquals('test_store', (string)$xml->getNode('stores/german/general/store_information/name')); + + // act + $loader = new Mage_Core_Helper_EnvironmentConfigLoader(); + $loader->setEnvStore([ + $config['path'] => $config['value'] + ]); + $loader->overrideEnvironment($xml); + + switch ($config['case']) { + case 'DEFAULT': + $defaultValue = $xmlDefault->getNode('default/general/store_information/name'); + $valueAfterOverride = $xml->getNode('default/general/store_information/name'); + break; + case 'STORE': + $defaultValue = $xmlDefault->getNode('stores/german/general/store_information/name'); + $valueAfterOverride = $xml->getNode('general/store_information/name'); + break; + case 'WEBSITE': + $defaultValue = $xmlDefault->getNode('default/general/store_information/name'); + $valueAfterOverride = $xml->getNode('website/base/store_information/name'); + break; + } + + // assert + $this->assertNotEquals((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.'); + } + + public function envOverridesDataProvider(): array { return [ [ - 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME', - 'exampleValue', - 'stores/german/general/store_information/name', - 'exampleValue' + 'Case DEFAULT with ' . static::ENV_CONFIG_DEFAULT_PATH . ' overrides.' => [ + 'case' => 'DEFAULT', + 'path' => static::ENV_CONFIG_DEFAULT_PATH, + 'value' => static::ENV_CONFIG_DEFAULT_VALUE + ] ], + [ + 'Case STORE with ' . static::ENV_CONFIG_STORE_PATH . ' overrides.' => [ + 'case' => 'STORE', + 'path' => static::ENV_CONFIG_STORE_PATH, + 'value' => static::ENV_CONFIG_STORE_VALUE + ] + ], + [ + 'Case WEBSITE with ' . static::ENV_CONFIG_WEBSITE_PATH . ' overrides.' => [ + 'case' => 'WEBSITE', + 'path' => static::ENV_CONFIG_WEBSITE_PATH, + 'value' => static::ENV_CONFIG_WEBSITE_VALUE + ] + ] ]; } /** - * @dataProvider simpleEnvValueDataProvider + * @return string */ - public function testSimpleEnvToConfig($envName, $envValue, $expectedPath, $expectedValue):void + public function getTestXml(): string { - \Mage::setRoot(''); - $helper = new \Mage_Core_Helper_EnvironmentConfigLoader(); - $config = new \Mage_Core_Model_Config( - new \Varien_Simplexml_Element('') - ); - - $helper->setEnvStore([ - $envName => $envValue - ]); - $helper->overrideEnvironment($config); - - $this->assertEquals( - $expectedValue, - $config->getNode($expectedPath) - ); + return << + + + + true + core + + + + + + + test_default + + + + + + + + + test_website + + + + + + + + + test_store + + + + + +XML; } } From 50bdaf8221b6749ae4433361c3de8ff269bec968 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 4 Feb 2024 13:43:40 +0100 Subject: [PATCH 06/18] wip: override config from env vars --- .../Mage/Core/Helper/EnvironmentLoader.php | 75 +++++++++++++++++++ app/code/core/Mage/Core/Model/App.php | 1 + app/code/core/Mage/Core/Model/Config.php | 16 ++++ 3 files changed, 92 insertions(+) create mode 100644 app/code/core/Mage/Core/Helper/EnvironmentLoader.php diff --git a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentLoader.php new file mode 100644 index 00000000000..865ace1e69d --- /dev/null +++ b/app/code/core/Mage/Core/Helper/EnvironmentLoader.php @@ -0,0 +1,75 @@ + $value) { + if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { + continue; + } + $configKey = str_replace(static::ENV_STARTS_WITH, '', $configKey); + list($_, $scope) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + switch ($scope) { + case static::CONFIG_KEY_DEFAULT: + list($_, $scope, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + $path = implode('/', [$section, $group, $field]); + $path = strtolower($path); + $scope = strtolower($scope); + + $xmlConfig->setNode($scope . '/' . $path, $value); + break; + case static::CONFIG_KEY_WEBSITES: + list($_, $_, $websiteCode, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + $path = implode('/', [$section, $group, $field]); + $path = strtolower($path); + $websiteCode = strtolower($websiteCode); + + $nodePath = sprintf('websites/%s/%s', $websiteCode, $path); + $xmlConfig->setNode($nodePath, $value); + break; + case static::CONFIG_KEY_STORES: + list($_, $_, $storeCode, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + $path = implode('/', [$section, $group, $field]); + $path = strtolower($path); + $storeCode = strtolower($storeCode); + $nodePath = sprintf('stores/%s/%s', $storeCode, $path); + $xmlConfig->setNode($nodePath, $value); + break; + } + } + } +} diff --git a/app/code/core/Mage/Core/Model/App.php b/app/code/core/Mage/Core/Model/App.php index a44360011fd..9989bd21744 100644 --- a/app/code/core/Mage/Core/Model/App.php +++ b/app/code/core/Mage/Core/Model/App.php @@ -442,6 +442,7 @@ protected function _initModules() Varien_Profiler::stop('mage::app::init::apply_db_schema_updates'); } $this->_config->loadDb(); + $this->_config->loadEnv(); $this->_config->saveCache(); } } finally { diff --git a/app/code/core/Mage/Core/Model/Config.php b/app/code/core/Mage/Core/Model/Config.php index b4498800c31..ef311fb3c80 100644 --- a/app/code/core/Mage/Core/Model/Config.php +++ b/app/code/core/Mage/Core/Model/Config.php @@ -320,6 +320,7 @@ public function init($options = []) $this->_useCache = false; $this->loadModules(); $this->loadDb(); + $this->loadEnv(); $this->saveCache(); } } finally { @@ -422,6 +423,21 @@ public function loadDb() return $this; } + /** + * Load environment variables and override config + * + * @return self + */ + public function loadEnv(): Mage_Core_Model_Config + { + if ($this->_isLocalConfigLoaded && Mage::isInstalled()) { + Varien_Profiler::start('config/load-env'); + Mage::helper('core/environmentLoader')->overrideEnvironment($this); + Varien_Profiler::stop('config/load-env'); + } + return $this; + } + /** * Reinitialize configuration * From 67b82e7c8152909ac6daf5bfe06ede66f8b43a80 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 4 Feb 2024 14:03:01 +0100 Subject: [PATCH 07/18] docs and phpcs changes --- .../Mage/Core/Helper/EnvironmentLoader.php | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentLoader.php index 865ace1e69d..641f1bab826 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentLoader.php @@ -21,15 +21,32 @@ */ class Mage_Core_Helper_EnvironmentLoader extends Mage_Core_Helper_Abstract { - const ENV_STARTS_WITH = 'OPENMAGE_CONFIG'; - const ENV_KEY_SEPARATOR = '__'; - const CONFIG_KEY_DEFAULT = 'DEFAULT'; - const CONFIG_KEY_WEBSITES = 'WEBSITES'; - const CONFIG_KEY_STORES = 'STORES'; + protected const ENV_STARTS_WITH = 'OPENMAGE_CONFIG'; + protected const ENV_KEY_SEPARATOR = '__'; + protected const CONFIG_KEY_DEFAULT = 'DEFAULT'; + protected const CONFIG_KEY_WEBSITES = 'WEBSITES'; + protected const CONFIG_KEY_STORES = 'STORES'; /** * Load configuration values from ENV variables into xml config object * + * Environment variables work on this schema: + * + * self::ENV_STARTS_WITH . self::ENV_KEY_SEPARATOR (OPENMAGE_CONFIG__) + * ^ Prefix (required) + * __ + * ^ Where scope is DEFAULT, WEBSITES__ or STORES__ + * + * ^ Where GROUP, SECTION and FIELD are separated by self::ENV_KEY_SEPARATOR + * + * Each example will override the 'general/store_information/name' value. + * Override from the default configuration: + * @example OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME=default + * Override the website 'base' configuration: + * @example OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME=website + * Override the store 'german' configuration: + * @example OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME=store_german + * * @param Mage_Core_Model_Config $xmlConfig * @return void */ @@ -45,7 +62,7 @@ public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) list($_, $scope) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); switch ($scope) { case static::CONFIG_KEY_DEFAULT: - list($_, $scope, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + list($_, $_, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); $path = implode('/', [$section, $group, $field]); $path = strtolower($path); $scope = strtolower($scope); From fe1521630ad4cf19dbac3a8c06d9f7626478a390 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 11 Feb 2024 23:52:50 +0100 Subject: [PATCH 08/18] feat: refactor config environment loader --- ...Loader.php => EnvironmentConfigLoader.php} | 62 ++++++++++++------- app/code/core/Mage/Core/Model/Config.php | 4 +- 2 files changed, 42 insertions(+), 24 deletions(-) rename app/code/core/Mage/Core/Helper/{EnvironmentLoader.php => EnvironmentConfigLoader.php} (63%) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php similarity index 63% rename from app/code/core/Mage/Core/Helper/EnvironmentLoader.php rename to app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 641f1bab826..47ba2303f9f 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -19,8 +19,9 @@ * @category Mage * @package Mage_Core */ -class Mage_Core_Helper_EnvironmentLoader extends Mage_Core_Helper_Abstract +class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract { + protected $_moduleName = 'Mage_Core'; protected const ENV_STARTS_WITH = 'OPENMAGE_CONFIG'; protected const ENV_KEY_SEPARATOR = '__'; protected const CONFIG_KEY_DEFAULT = 'DEFAULT'; @@ -52,41 +53,56 @@ class Mage_Core_Helper_EnvironmentLoader extends Mage_Core_Helper_Abstract */ public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) { - // override from env $env = getenv(); + foreach ($env as $configKey => $value) { if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { continue; } - $configKey = str_replace(static::ENV_STARTS_WITH, '', $configKey); - list($_, $scope) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); + + $configKeyParts = array_filter(explode(static::ENV_KEY_SEPARATOR, str_replace(static::ENV_STARTS_WITH, '', $configKey)), 'trim'); + list($_, $scope) = $configKeyParts; + switch ($scope) { case static::CONFIG_KEY_DEFAULT: - list($_, $_, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); - $path = implode('/', [$section, $group, $field]); - $path = strtolower($path); - $scope = strtolower($scope); - - $xmlConfig->setNode($scope . '/' . $path, $value); + list($_, $_, $section, $group, $field) = $configKeyParts; + $path = $this->buildPath($section, $group, $field); + $xmlConfig->setNode($this->buildNodePath($scope, $path), $value); break; - case static::CONFIG_KEY_WEBSITES: - list($_, $_, $websiteCode, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); - $path = implode('/', [$section, $group, $field]); - $path = strtolower($path); - $websiteCode = strtolower($websiteCode); - $nodePath = sprintf('websites/%s/%s', $websiteCode, $path); - $xmlConfig->setNode($nodePath, $value); - break; + case static::CONFIG_KEY_WEBSITES: case static::CONFIG_KEY_STORES: - list($_, $_, $storeCode, $section, $group, $field) = array_filter(explode(static::ENV_KEY_SEPARATOR, $configKey), 'trim'); - $path = implode('/', [$section, $group, $field]); - $path = strtolower($path); - $storeCode = strtolower($storeCode); - $nodePath = sprintf('stores/%s/%s', $storeCode, $path); + list($_, $_, $code, $section, $group, $field) = $configKeyParts; + $path = $this->buildPath($section, $group, $field); + $nodePath = sprintf('%s/%s/%s', strtolower($scope), strtolower($code), $path); $xmlConfig->setNode($nodePath, $value); break; } } } + + /** + * Build configuration path. + * + * @param string $section + * @param string $group + * @param string $field + * @return string + */ + public function buildPath($section, $group, $field): string + { + return strtolower(implode('/', [$section, $group, $field])); + } + + /** + * Build configuration node path. + * + * @param string $scope + * @param string $path + * @return string + */ + public function buildNodePath($scope, $path): string + { + return strtolower($scope) . '/' . $path; + } } diff --git a/app/code/core/Mage/Core/Model/Config.php b/app/code/core/Mage/Core/Model/Config.php index ef311fb3c80..e5a6fb86b6d 100644 --- a/app/code/core/Mage/Core/Model/Config.php +++ b/app/code/core/Mage/Core/Model/Config.php @@ -432,7 +432,9 @@ public function loadEnv(): Mage_Core_Model_Config { if ($this->_isLocalConfigLoaded && Mage::isInstalled()) { Varien_Profiler::start('config/load-env'); - Mage::helper('core/environmentLoader')->overrideEnvironment($this); + /** @var Mage_Core_Helper_EnvironmentConfigLoader $environmentConfigLoaderHelper */ + $environmentConfigLoaderHelper = Mage::helper('core/environmentConfigLoader'); + $environmentConfigLoaderHelper->overrideEnvironment($this); Varien_Profiler::stop('config/load-env'); } return $this; From 2da4951089a1c927f0801cc4a4d73e44304f2dd2 Mon Sep 17 00:00:00 2001 From: Flyingmana Date: Sun, 18 Feb 2024 21:11:48 +0100 Subject: [PATCH 09/18] add unittest and some changes for mocking and making the logic work --- .../Core/Helper/EnvironmentConfigLoader.php | 28 +++++++++++- .../Helper/EnvironmentConfigLoaderTest.php | 44 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php diff --git a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 47ba2303f9f..91d93acba0f 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -28,6 +28,8 @@ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract protected const CONFIG_KEY_WEBSITES = 'WEBSITES'; protected const CONFIG_KEY_STORES = 'STORES'; + protected array $envStore = []; + /** * Load configuration values from ENV variables into xml config object * @@ -53,14 +55,20 @@ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract */ public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) { - $env = getenv(); + $env = $this->getEnv(); foreach ($env as $configKey => $value) { if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { continue; } - $configKeyParts = array_filter(explode(static::ENV_KEY_SEPARATOR, str_replace(static::ENV_STARTS_WITH, '', $configKey)), 'trim'); + $configKeyParts = array_filter( + explode( + static::ENV_KEY_SEPARATOR, + $configKey + ), + 'trim' + ); list($_, $scope) = $configKeyParts; switch ($scope) { @@ -81,6 +89,22 @@ public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) } } + /** + * @internal method mostly for mocking + */ + public function setEnvStore(array $envStorage): void + { + $this->envStore = $envStorage; + } + + public function getEnv(): array + { + if (empty($this->envStore)) { + $this->envStore = getenv(); + } + return $this->envStore; + } + /** * Build configuration path. * diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php new file mode 100644 index 00000000000..7a62644497a --- /dev/null +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -0,0 +1,44 @@ +') + ); + + $helper->setEnvStore([ + $envName => $envValue + ]); + $helper->overrideEnvironment($config); + + $this->assertEquals( + $expectedValue, + $config->getNode($expectedPath) + ); + } +} From 3eda6410a65768de455ac20fd281d7a4d3448f21 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 18 Feb 2024 23:07:15 +0100 Subject: [PATCH 10/18] tests: finish some more tests w/ website and store data overrides --- .../Helper/EnvironmentConfigLoaderTest.php | 155 +++++++++++++++--- 1 file changed, 132 insertions(+), 23 deletions(-) diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php index 7a62644497a..203c62f7f80 100644 --- a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -4,41 +4,150 @@ namespace OpenMage\Tests\Unit\Core\Helper; use PHPUnit\Framework\TestCase; +use Mage_Core_Helper_EnvironmentConfigLoader; +use Mage_Core_Model_Config; -class Security extends TestCase +class EnvironmentConfigLoaderTest extends TestCase { + protected const ENV_CONFIG_DEFAULT_PATH = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME'; + protected const ENV_CONFIG_DEFAULT_VALUE = 'default_new_value'; + protected const ENV_CONFIG_WEBSITE_PATH = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME'; + protected const ENV_CONFIG_WEBSITE_VALUE = 'website_new_value'; + protected const ENV_CONFIG_STORE_PATH = 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME'; + protected const ENV_CONFIG_STORE_VALUE = 'store_german_new_value'; - public function simpleEnvValueDataProvider(): array + public function setup(): void + { + \Mage::setRoot(''); + } + + public function testBuildPath() + { + $environmentConfigLoaderHelper = new Mage_Core_Helper_EnvironmentConfigLoader(); + $path = $environmentConfigLoaderHelper->buildPath('GENERAL', 'STORE_INFORMATION', 'NAME'); + $this->assertEquals('general/store_information/name', $path); + } + + public function testBuildNodePath() + { + $loader = new Mage_Core_Helper_EnvironmentConfigLoader(); + $nodePath = $loader->buildNodePath('DEFAULT', 'general/store_information/name'); + $this->assertEquals('default/general/store_information/name', $nodePath); + } + + /** + * @dataProvider envOverridesDataProvider + * + */ + public function testEnvOverrides(array $config) + { + error_reporting(0); + $xmlStruct = $this->getTestXml(); + + $xmlDefault = new Mage_Core_Model_Config($xmlStruct); + $xmlDefault = $xmlDefault->loadModulesConfiguration('config.xml', $xmlDefault); + $xml = new Mage_Core_Model_Config($xmlStruct); + $xml = $xml->loadModulesConfiguration('config.xml', $xml); + + $this->assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name')); + $this->assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name')); + $this->assertEquals('test_store', (string)$xml->getNode('stores/german/general/store_information/name')); + + // act + $loader = new Mage_Core_Helper_EnvironmentConfigLoader(); + $loader->setEnvStore([ + $config['path'] => $config['value'] + ]); + $loader->overrideEnvironment($xml); + + switch ($config['case']) { + case 'DEFAULT': + $defaultValue = $xmlDefault->getNode('default/general/store_information/name'); + $valueAfterOverride = $xml->getNode('default/general/store_information/name'); + break; + case 'STORE': + $defaultValue = $xmlDefault->getNode('stores/german/general/store_information/name'); + $valueAfterOverride = $xml->getNode('general/store_information/name'); + break; + case 'WEBSITE': + $defaultValue = $xmlDefault->getNode('default/general/store_information/name'); + $valueAfterOverride = $xml->getNode('website/base/store_information/name'); + break; + } + + // assert + $this->assertNotEquals((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.'); + } + + public function envOverridesDataProvider(): array { return [ [ - 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME', - 'exampleValue', - 'stores/german/general/store_information/name', - 'exampleValue' + 'Case DEFAULT with ' . static::ENV_CONFIG_DEFAULT_PATH . ' overrides.' => [ + 'case' => 'DEFAULT', + 'path' => static::ENV_CONFIG_DEFAULT_PATH, + 'value' => static::ENV_CONFIG_DEFAULT_VALUE + ] ], + [ + 'Case STORE with ' . static::ENV_CONFIG_STORE_PATH . ' overrides.' => [ + 'case' => 'STORE', + 'path' => static::ENV_CONFIG_STORE_PATH, + 'value' => static::ENV_CONFIG_STORE_VALUE + ] + ], + [ + 'Case WEBSITE with ' . static::ENV_CONFIG_WEBSITE_PATH . ' overrides.' => [ + 'case' => 'WEBSITE', + 'path' => static::ENV_CONFIG_WEBSITE_PATH, + 'value' => static::ENV_CONFIG_WEBSITE_VALUE + ] + ] ]; } /** - * @dataProvider simpleEnvValueDataProvider + * @return string */ - public function testSimpleEnvToConfig($envName, $envValue, $expectedPath, $expectedValue):void + public function getTestXml(): string { - \Mage::setRoot(''); - $helper = new \Mage_Core_Helper_EnvironmentConfigLoader(); - $config = new \Mage_Core_Model_Config( - new \Varien_Simplexml_Element('') - ); - - $helper->setEnvStore([ - $envName => $envValue - ]); - $helper->overrideEnvironment($config); - - $this->assertEquals( - $expectedValue, - $config->getNode($expectedPath) - ); + return << + + + + true + core + + + + + + + test_default + + + + + + + + + test_website + + + + + + + + + test_store + + + + + +XML; } } From b3da428b68040bf88ac6fa16987c04bcde65f917 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sat, 2 Mar 2024 14:02:13 +0100 Subject: [PATCH 11/18] fix: PHP8 test error when constructing Mage_Core_Model_Config The constructor of Mage_Core_Model_Config inherits from Varien_Object and its first argument is written to $this->data. So when the "constructor" of Mage_Core_Model_Config kicks in, it wanted to write an array to a already populated $this->_data variable, which yields errors since PHP 7.1. --- .../core/Mage/Core/Helper/EnvironmentConfigLoader.php | 4 ++-- .../Mage/Core/Helper/EnvironmentConfigLoaderTest.php | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 91d93acba0f..331a1e90fff 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -50,10 +50,10 @@ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract * Override the store 'german' configuration: * @example OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME=store_german * - * @param Mage_Core_Model_Config $xmlConfig + * @param Varien_Simplexml_Config $xmlConfig * @return void */ - public function overrideEnvironment(Mage_Core_Model_Config $xmlConfig) + public function overrideEnvironment(Varien_Simplexml_Config $xmlConfig) { $env = $this->getEnv(); diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php index 203c62f7f80..da9546e682e 100644 --- a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -41,13 +41,12 @@ public function testBuildNodePath() */ public function testEnvOverrides(array $config) { - error_reporting(0); $xmlStruct = $this->getTestXml(); - $xmlDefault = new Mage_Core_Model_Config($xmlStruct); - $xmlDefault = $xmlDefault->loadModulesConfiguration('config.xml', $xmlDefault); - $xml = new Mage_Core_Model_Config($xmlStruct); - $xml = $xml->loadModulesConfiguration('config.xml', $xml); + $xmlDefault = new \Varien_Simplexml_Config(); + $xmlDefault->loadString($xmlStruct); + $xml = new \Varien_Simplexml_Config(); + $xml->loadString($xmlStruct); $this->assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name')); $this->assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name')); From e3d9424e26205656e7386b532017a2cb4afe8cc7 Mon Sep 17 00:00:00 2001 From: Fabrizio Balliano Date: Sat, 2 Mar 2024 23:20:53 +0000 Subject: [PATCH 12/18] Updated copyright --- app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 331a1e90fff..9490b3f27b0 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -8,8 +8,7 @@ * * @category Mage * @package Mage_Core - * @copyright Copyright (c) 2006-2020 Magento, Inc. (https://www.magento.com) - * @copyright Copyright (c) 2016-present The OpenMage Contributors (https://www.openmage.org) + * @copyright Copyright (c) 2024 The OpenMage Contributors (https://www.openmage.org) * @license https://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0) */ From 36e55c7fb803989e9c5d6ab8f254997e3d400fdf Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 3 Mar 2024 10:13:26 +0100 Subject: [PATCH 13/18] fix: config overrides should not work with invalid config key --- .../Core/Helper/EnvironmentConfigLoader.php | 19 ++- .../Helper/EnvironmentConfigLoaderTest.php | 122 +++++++++++++----- 2 files changed, 110 insertions(+), 31 deletions(-) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 9490b3f27b0..14bb7be4c7c 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -57,7 +57,7 @@ public function overrideEnvironment(Varien_Simplexml_Config $xmlConfig) $env = $this->getEnv(); foreach ($env as $configKey => $value) { - if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { + if (!$this->isConfigKeyValid($configKey)) { continue; } @@ -88,6 +88,23 @@ public function overrideEnvironment(Varien_Simplexml_Config $xmlConfig) } } + public function isConfigKeyValid(string $configKey): bool + { + if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { + return false; + } + + $sectionGroupFieldRegexp = '([A-Z_]*)'; + $regexp = '/' . static::ENV_STARTS_WITH . static::ENV_KEY_SEPARATOR . '(WEBSITES' . static::ENV_KEY_SEPARATOR + . '[A-Z_]+|DEFAULT|STORES' . static::ENV_KEY_SEPARATOR . '[A-Z_]+)' + . static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp + . static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp + . static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp . '/'; + // /OPENMAGE_CONFIG__(WEBSITES__[A-Z_]+|DEFAULT|STORES__[A-Z_]+)__([A-Z_]*)__([A-Z_]+)__([A-Z_]+)/ + + return preg_match($regexp, $configKey); + } + /** * @internal method mostly for mocking */ diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php index da9546e682e..0d23c829c14 100644 --- a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -9,13 +9,6 @@ class EnvironmentConfigLoaderTest extends TestCase { - protected const ENV_CONFIG_DEFAULT_PATH = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME'; - protected const ENV_CONFIG_DEFAULT_VALUE = 'default_new_value'; - protected const ENV_CONFIG_WEBSITE_PATH = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME'; - protected const ENV_CONFIG_WEBSITE_VALUE = 'website_new_value'; - protected const ENV_CONFIG_STORE_PATH = 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME'; - protected const ENV_CONFIG_STORE_VALUE = 'store_german_new_value'; - public function setup(): void { \Mage::setRoot(''); @@ -36,10 +29,10 @@ public function testBuildNodePath() } /** - * @dataProvider envOverridesDataProvider - * + * @dataProvider env_overrides_correct_config_keys + * @test */ - public function testEnvOverrides(array $config) + public function env_overrides_for_valid_config_keys(array $config) { $xmlStruct = $this->getTestXml(); @@ -66,11 +59,11 @@ public function testEnvOverrides(array $config) break; case 'STORE': $defaultValue = $xmlDefault->getNode('stores/german/general/store_information/name'); - $valueAfterOverride = $xml->getNode('general/store_information/name'); + $valueAfterOverride = $xml->getNode('stores/german/general/store_information/name'); break; case 'WEBSITE': - $defaultValue = $xmlDefault->getNode('default/general/store_information/name'); - $valueAfterOverride = $xml->getNode('website/base/store_information/name'); + $defaultValue = $xmlDefault->getNode('websites/base/general/store_information/name'); + $valueAfterOverride = $xml->getNode('websites/base/general/store_information/name'); break; } @@ -78,28 +71,104 @@ public function testEnvOverrides(array $config) $this->assertNotEquals((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.'); } - public function envOverridesDataProvider(): array + public function env_overrides_correct_config_keys(): array + { + $defaultPath = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME'; + $websitePath = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME'; + $storePath = 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME'; + return [ + [ + 'Case DEFAULT with ' . $defaultPath . ' overrides.' => [ + 'case' => 'DEFAULT', + 'path' => $defaultPath, + 'value' => 'default_new_value' + ] + ], + [ + 'Case STORE with ' . $storePath . ' overrides.' => [ + 'case' => 'STORE', + 'path' => $storePath, + 'value' => 'store_new_value' + ] + ], + [ + 'Case WEBSITE with ' . $websitePath . ' overrides.' => [ + 'case' => 'WEBSITE', + 'path' => $websitePath, + 'value' => 'website_new_value' + ] + ] + ]; + } + + /** + * @dataProvider env_does_not_override_on_wrong_config_keys + * @test + */ + public function env_does_not_override_for_valid_config_keys(array $config) + { + $xmlStruct = $this->getTestXml(); + + $xmlDefault = new \Varien_Simplexml_Config(); + $xmlDefault->loadString($xmlStruct); + $xml = new \Varien_Simplexml_Config(); + $xml->loadString($xmlStruct); + + $defaultValue = 'test_default'; + $this->assertEquals($defaultValue, (string)$xml->getNode('default/general/store_information/name')); + $defaultWebsiteValue = 'test_website'; + $this->assertEquals($defaultWebsiteValue, (string)$xml->getNode('websites/base/general/store_information/name')); + $defaultStoreValue = 'test_store'; + $this->assertEquals($defaultStoreValue, (string)$xml->getNode('stores/german/general/store_information/name')); + + // act + $loader = new Mage_Core_Helper_EnvironmentConfigLoader(); + $loader->setEnvStore([ + $config['path'] => $config['value'] + ]); + $loader->overrideEnvironment($xml); + + switch ($config['case']) { + case 'DEFAULT': + $valueAfterCheck = $xml->getNode('default/general/store_information/name'); + break; + case 'STORE': + $valueAfterCheck = $xml->getNode('stores/german/general/store_information/name'); + break; + case 'WEBSITE': + $valueAfterCheck = $xml->getNode('websites/base/general/store_information/name'); + break; + } + + // assert + $this->assertTrue(!str_contains('value_will_not_be_changed', (string)$valueAfterCheck), 'Default value was wrongfully overridden.'); + } + + public function env_does_not_override_on_wrong_config_keys(): array { + $defaultPath = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__ST'; + $websitePath = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__ST'; + $storePath = 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__ST'; return [ [ - 'Case DEFAULT with ' . static::ENV_CONFIG_DEFAULT_PATH . ' overrides.' => [ + 'Case DEFAULT with ' . $defaultPath . ' will not override.' => [ 'case' => 'DEFAULT', - 'path' => static::ENV_CONFIG_DEFAULT_PATH, - 'value' => static::ENV_CONFIG_DEFAULT_VALUE + 'path' => $defaultPath, + 'value' => 'default_value_will_not_be_changed' ] ], [ - 'Case STORE with ' . static::ENV_CONFIG_STORE_PATH . ' overrides.' => [ + 'Case STORE with ' . $storePath . ' will not override.' => [ 'case' => 'STORE', - 'path' => static::ENV_CONFIG_STORE_PATH, - 'value' => static::ENV_CONFIG_STORE_VALUE + 'path' => $storePath, + 'value' => 'store_value_will_not_be_changed' ] ], [ - 'Case WEBSITE with ' . static::ENV_CONFIG_WEBSITE_PATH . ' overrides.' => [ + 'Case WEBSITE with ' . $websitePath . ' will not override.' => [ 'case' => 'WEBSITE', - 'path' => static::ENV_CONFIG_WEBSITE_PATH, - 'value' => static::ENV_CONFIG_WEBSITE_VALUE + 'path' => $websitePath, + 'value' => 'website_value_will_not_be_changed' ] ] ]; @@ -113,13 +182,6 @@ public function getTestXml(): string return << - - - true - core - - - From 7b5da9b4af0bcc97ca1e73ce2d49e8fc4d03efc6 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 17 Mar 2024 15:59:35 +0100 Subject: [PATCH 14/18] feat: add allow list for regexp; alter tests now includes "-" and "_" as default allowed values next to A-Z (regexp) --- .../Core/Helper/EnvironmentConfigLoader.php | 33 +++-- .../Helper/EnvironmentConfigLoaderTest.php | 129 +++++++++++++----- 2 files changed, 119 insertions(+), 43 deletions(-) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 14bb7be4c7c..901febc5946 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -26,6 +26,10 @@ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract protected const CONFIG_KEY_DEFAULT = 'DEFAULT'; protected const CONFIG_KEY_WEBSITES = 'WEBSITES'; protected const CONFIG_KEY_STORES = 'STORES'; + /** + * To be used as regex condition + */ + protected const ALLOWED_CHARS = ['A-Z', '-', '_']; protected array $envStore = []; @@ -61,14 +65,7 @@ public function overrideEnvironment(Varien_Simplexml_Config $xmlConfig) continue; } - $configKeyParts = array_filter( - explode( - static::ENV_KEY_SEPARATOR, - $configKey - ), - 'trim' - ); - list($_, $scope) = $configKeyParts; + list($configKeyParts, $scope) = $this->getConfigKey($configKey); switch ($scope) { case static::CONFIG_KEY_DEFAULT: @@ -88,19 +85,33 @@ public function overrideEnvironment(Varien_Simplexml_Config $xmlConfig) } } + public function getConfigKey(string $configKey): array + { + $configKeyParts = array_filter( + explode( + static::ENV_KEY_SEPARATOR, + $configKey + ), + 'trim' + ); + list($_, $scope) = $configKeyParts; + return array($configKeyParts, $scope); + } + public function isConfigKeyValid(string $configKey): bool { if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { return false; } - $sectionGroupFieldRegexp = '([A-Z_]*)'; + $sectionGroupFieldRegexp = sprintf('([%s]*)', implode('', static::ALLOWED_CHARS)); + $allowedChars = sprintf('[%s]', implode('', static::ALLOWED_CHARS)); $regexp = '/' . static::ENV_STARTS_WITH . static::ENV_KEY_SEPARATOR . '(WEBSITES' . static::ENV_KEY_SEPARATOR - . '[A-Z_]+|DEFAULT|STORES' . static::ENV_KEY_SEPARATOR . '[A-Z_]+)' + . $allowedChars . '+|DEFAULT|STORES' . static::ENV_KEY_SEPARATOR . $allowedChars . '+)' . static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp . static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp . static::ENV_KEY_SEPARATOR . $sectionGroupFieldRegexp . '/'; - // /OPENMAGE_CONFIG__(WEBSITES__[A-Z_]+|DEFAULT|STORES__[A-Z_]+)__([A-Z_]*)__([A-Z_]+)__([A-Z_]+)/ + // /OPENMAGE_CONFIG__(WEBSITES__[A-Z-_]+|DEFAULT|STORES__[A-Z-_]+)__([A-Z-_]*)__([A-Z-_]*)__([A-Z-_]*)/ return preg_match($regexp, $configKey); } diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php index 0d23c829c14..0e9516a8bdc 100644 --- a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -28,6 +28,16 @@ public function testBuildNodePath() $this->assertEquals('default/general/store_information/name', $nodePath); } + public function test_xml_has_test_strings() + { + $xmlStruct = $this->getTestXml(); + $xml = new \Varien_Simplexml_Config(); + $xml->loadString($xmlStruct); + $this->assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name')); + $this->assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name')); + $this->assertEquals('test_store', (string)$xml->getNode('stores/german/general/store_information/name')); + } + /** * @dataProvider env_overrides_correct_config_keys * @test @@ -41,31 +51,16 @@ public function env_overrides_for_valid_config_keys(array $config) $xml = new \Varien_Simplexml_Config(); $xml->loadString($xmlStruct); - $this->assertEquals('test_default', (string)$xml->getNode('default/general/store_information/name')); - $this->assertEquals('test_website', (string)$xml->getNode('websites/base/general/store_information/name')); - $this->assertEquals('test_store', (string)$xml->getNode('stores/german/general/store_information/name')); - // act $loader = new Mage_Core_Helper_EnvironmentConfigLoader(); $loader->setEnvStore([ - $config['path'] => $config['value'] + $config['env_path'] => $config['value'] ]); $loader->overrideEnvironment($xml); - switch ($config['case']) { - case 'DEFAULT': - $defaultValue = $xmlDefault->getNode('default/general/store_information/name'); - $valueAfterOverride = $xml->getNode('default/general/store_information/name'); - break; - case 'STORE': - $defaultValue = $xmlDefault->getNode('stores/german/general/store_information/name'); - $valueAfterOverride = $xml->getNode('stores/german/general/store_information/name'); - break; - case 'WEBSITE': - $defaultValue = $xmlDefault->getNode('websites/base/general/store_information/name'); - $valueAfterOverride = $xml->getNode('websites/base/general/store_information/name'); - break; - } + $configPath = $config['xml_path']; + $defaultValue = $xmlDefault->getNode($configPath); + $valueAfterOverride = $xml->getNode($configPath); // assert $this->assertNotEquals((string)$defaultValue, (string)$valueAfterOverride, 'Default value was not overridden.'); @@ -74,28 +69,70 @@ public function env_overrides_for_valid_config_keys(array $config) public function env_overrides_correct_config_keys(): array { $defaultPath = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME'; + $websitePath = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME'; + $websiteWithDashPath = 'OPENMAGE_CONFIG__WEBSITES__BASE-AT__GENERAL__STORE_INFORMATION__NAME'; + $websiteWithUnderscorePath = 'OPENMAGE_CONFIG__WEBSITES__BASE_CH__GENERAL__STORE_INFORMATION__NAME'; + + $storeWithDashPath = 'OPENMAGE_CONFIG__STORES__GERMAN-AT__GENERAL__STORE_INFORMATION__NAME'; + $storeWithUnderscorePath = 'OPENMAGE_CONFIG__STORES__GERMAN_CH__GENERAL__STORE_INFORMATION__NAME'; $storePath = 'OPENMAGE_CONFIG__STORES__GERMAN__GENERAL__STORE_INFORMATION__NAME'; + return [ [ - 'Case DEFAULT with ' . $defaultPath . ' overrides.' => [ - 'case' => 'DEFAULT', - 'path' => $defaultPath, - 'value' => 'default_new_value' + 'Case DEFAULT overrides.' => [ + 'case' => 'DEFAULT', + 'xml_path' => 'default/general/store_information/name', + 'env_path' => $defaultPath, + 'value' => 'default_new_value' ] ], [ - 'Case STORE with ' . $storePath . ' overrides.' => [ - 'case' => 'STORE', - 'path' => $storePath, - 'value' => 'store_new_value' + 'Case STORE overrides.' => [ + 'case' => 'STORE', + 'xml_path' => 'stores/german/general/store_information/name', + 'env_path' => $storePath, + 'value' => 'store_new_value' ] ], [ - 'Case WEBSITE with ' . $websitePath . ' overrides.' => [ - 'case' => 'WEBSITE', - 'path' => $websitePath, - 'value' => 'website_new_value' + 'Case STORE overrides.' => [ + 'case' => 'STORE', + 'xml_path' => 'stores/german-at/general/store_information/name', + 'env_path' => $storeWithDashPath, + 'value' => 'store_new_value' + ] + ], + [ + 'Case STORE overrides.' => [ + 'case' => 'STORE', + 'xml_path' => 'stores/german_ch/general/store_information/name', + 'env_path' => $storeWithUnderscorePath, + 'value' => 'store_new_value' + ] + ], + [ + 'Case WEBSITE overrides.' => [ + 'case' => 'WEBSITE', + 'xml_path' => 'websites/base/general/store_information/name', + 'env_path' => $websitePath, + 'value' => 'website_new_value' + ] + ], + [ + 'Case WEBSITE overrides.' => [ + 'case' => 'WEBSITE', + 'xml_path' => 'websites/base_ch/general/store_information/name', + 'env_path' => $websiteWithUnderscorePath, + 'value' => 'website_new_value' + ] + ], + [ + 'Case WEBSITE overrides.' => [ + 'case' => 'WEBSITE', + 'xml_path' => 'websites/base-at/general/store_information/name', + 'env_path' => $websiteWithDashPath, + 'value' => 'website_new_value' ] ] ]; @@ -105,7 +142,7 @@ public function env_overrides_correct_config_keys(): array * @dataProvider env_does_not_override_on_wrong_config_keys * @test */ - public function env_does_not_override_for_valid_config_keys(array $config) + public function env_does_not_override_for_invalid_config_keys(array $config) { $xmlStruct = $this->getTestXml(); @@ -198,6 +235,20 @@ public function getTestXml(): string + + + + test_website + + + + + + + test_website + + + @@ -207,6 +258,20 @@ public function getTestXml(): string + + + + test_store + + + + + + + test_store + + + XML; From fd285433d1af231c9cf609a8796234e9c73fea05 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Sun, 17 Mar 2024 16:36:12 +0100 Subject: [PATCH 15/18] fix: add tests for dash and underscore for group/section or field this only tests group, but the pattern is the same --- .../Helper/EnvironmentConfigLoaderTest.php | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php index 0e9516a8bdc..0067b2a5308 100644 --- a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -69,6 +69,8 @@ public function env_overrides_for_valid_config_keys(array $config) public function env_overrides_correct_config_keys(): array { $defaultPath = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__STORE_INFORMATION__NAME'; + $defaultPathWithDash = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__FOO-BAR__NAME'; + $defaultPathWithUnderscore = 'OPENMAGE_CONFIG__DEFAULT__GENERAL__FOO_BAR__NAME'; $websitePath = 'OPENMAGE_CONFIG__WEBSITES__BASE__GENERAL__STORE_INFORMATION__NAME'; $websiteWithDashPath = 'OPENMAGE_CONFIG__WEBSITES__BASE-AT__GENERAL__STORE_INFORMATION__NAME'; @@ -87,6 +89,22 @@ public function env_overrides_correct_config_keys(): array 'value' => 'default_new_value' ] ], + [ + 'Case DEFAULT overrides.' => [ + 'case' => 'DEFAULT', + 'xml_path' => 'default/general/foo-bar/name', + 'env_path' => $defaultPathWithDash, + 'value' => 'baz' + ] + ], + [ + 'Case DEFAULT overrides.' => [ + 'case' => 'DEFAULT', + 'xml_path' => 'default/general/foo_bar/name', + 'env_path' => $defaultPathWithUnderscore, + 'value' => 'baz' + ] + ], [ 'Case STORE overrides.' => [ 'case' => 'STORE', @@ -224,6 +242,12 @@ public function getTestXml(): string test_default + + test_default + + + test_default + From 0fa2a840201b4d4c5af8a0d5e6e9470f2421cc4e Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Fri, 26 Apr 2024 13:35:38 +0200 Subject: [PATCH 16/18] chore: move property below constants --- app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 901febc5946..8002c1a0bb0 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -20,7 +20,6 @@ */ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract { - protected $_moduleName = 'Mage_Core'; protected const ENV_STARTS_WITH = 'OPENMAGE_CONFIG'; protected const ENV_KEY_SEPARATOR = '__'; protected const CONFIG_KEY_DEFAULT = 'DEFAULT'; @@ -30,7 +29,7 @@ class Mage_Core_Helper_EnvironmentConfigLoader extends Mage_Core_Helper_Abstract * To be used as regex condition */ protected const ALLOWED_CHARS = ['A-Z', '-', '_']; - + protected $_moduleName = 'Mage_Core'; protected array $envStore = []; /** From 3a28479b429c82145aa9e9fa7aa884b89f223d50 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Fri, 26 Apr 2024 13:37:56 +0200 Subject: [PATCH 17/18] chore: change method visibility from public to protected; move accordingly and add methods parameter type to declaration --- .../Core/Helper/EnvironmentConfigLoader.php | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php index 8002c1a0bb0..61abc39e84e 100644 --- a/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php +++ b/app/code/core/Mage/Core/Helper/EnvironmentConfigLoader.php @@ -84,7 +84,23 @@ public function overrideEnvironment(Varien_Simplexml_Config $xmlConfig) } } - public function getConfigKey(string $configKey): array + /** + * @internal method mostly for mocking + */ + public function setEnvStore(array $envStorage): void + { + $this->envStore = $envStorage; + } + + public function getEnv(): array + { + if (empty($this->envStore)) { + $this->envStore = getenv(); + } + return $this->envStore; + } + + protected function getConfigKey(string $configKey): array { $configKeyParts = array_filter( explode( @@ -97,7 +113,7 @@ public function getConfigKey(string $configKey): array return array($configKeyParts, $scope); } - public function isConfigKeyValid(string $configKey): bool + protected function isConfigKeyValid(string $configKey): bool { if (!str_starts_with($configKey, static::ENV_STARTS_WITH)) { return false; @@ -115,22 +131,6 @@ public function isConfigKeyValid(string $configKey): bool return preg_match($regexp, $configKey); } - /** - * @internal method mostly for mocking - */ - public function setEnvStore(array $envStorage): void - { - $this->envStore = $envStorage; - } - - public function getEnv(): array - { - if (empty($this->envStore)) { - $this->envStore = getenv(); - } - return $this->envStore; - } - /** * Build configuration path. * @@ -139,7 +139,7 @@ public function getEnv(): array * @param string $field * @return string */ - public function buildPath($section, $group, $field): string + protected function buildPath(string $section, string $group, string $field): string { return strtolower(implode('/', [$section, $group, $field])); } @@ -151,7 +151,7 @@ public function buildPath($section, $group, $field): string * @param string $path * @return string */ - public function buildNodePath($scope, $path): string + protected function buildNodePath(string $scope, string $path): string { return strtolower($scope) . '/' . $path; } From b487236e3532cdd423741cb135fd060fb1c1c795 Mon Sep 17 00:00:00 2001 From: Pascal Querner Date: Fri, 26 Apr 2024 13:48:19 +0200 Subject: [PATCH 18/18] feat: make tests compatible to visibility changes on helper class --- .../Helper/EnvironmentConfigLoaderTest.php | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php index 0067b2a5308..ed7911d29ba 100644 --- a/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php +++ b/dev/tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php @@ -7,6 +7,18 @@ use Mage_Core_Helper_EnvironmentConfigLoader; use Mage_Core_Model_Config; +class TestEnvLoaderHelper extends Mage_Core_Helper_EnvironmentConfigLoader { + public function exposedBuildPath(string $section, string $group, string $field): string + { + return $this->buildPath($section, $group, $field); + } + + public function exposedBuildNodePath(string $scope, string $path): string + { + return $this->buildNodePath($scope, $path); + } +} + class EnvironmentConfigLoaderTest extends TestCase { public function setup(): void @@ -16,15 +28,15 @@ public function setup(): void public function testBuildPath() { - $environmentConfigLoaderHelper = new Mage_Core_Helper_EnvironmentConfigLoader(); - $path = $environmentConfigLoaderHelper->buildPath('GENERAL', 'STORE_INFORMATION', 'NAME'); + $environmentConfigLoaderHelper = new TestEnvLoaderHelper(); + $path = $environmentConfigLoaderHelper->exposedBuildPath('GENERAL', 'STORE_INFORMATION', 'NAME'); $this->assertEquals('general/store_information/name', $path); } public function testBuildNodePath() { - $loader = new Mage_Core_Helper_EnvironmentConfigLoader(); - $nodePath = $loader->buildNodePath('DEFAULT', 'general/store_information/name'); + $environmentConfigLoaderHelper = new TestEnvLoaderHelper(); + $nodePath = $environmentConfigLoaderHelper->exposedBuildNodePath('DEFAULT', 'general/store_information/name'); $this->assertEquals('default/general/store_information/name', $nodePath); }