From ddbed22a6f9b42b173d036c344c03894d014b1d7 Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Wed, 4 Aug 2021 16:41:18 +0300 Subject: [PATCH] config internal --- .gitignore | 1 + application/Espo/Core/Portal/Utils/Config.php | 12 +-- application/Espo/Core/Utils/Config.php | 89 +++++++++++----- application/Espo/Core/Utils/Config/Access.php | 22 ++-- .../Espo/Core/Utils/Config/ConfigWriter.php | 100 ++++++++++++------ .../Utils/Config/InternalConfigHelper.php | 63 +++++++++++ .../Espo/Resources/metadata/app/config.json | 7 +- .../metadata/entityDefs/Settings.json | 3 +- application/Espo/Services/Settings.php | 2 +- .../Core/Utils/Config/ConfigWriterTest.php | 29 ++++- tests/unit/Espo/Core/Utils/ConfigTest.php | 11 +- 11 files changed, 250 insertions(+), 89 deletions(-) create mode 100644 application/Espo/Core/Utils/Config/InternalConfigHelper.php diff --git a/.gitignore b/.gitignore index 20b464b902..cfb81ca883 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ /data/preferences/* /data/.backup/* /data/config.php +/data/config-internal.php /data/tmp/* /build /node_modules diff --git a/application/Espo/Core/Portal/Utils/Config.php b/application/Espo/Core/Portal/Utils/Config.php index 3e443829b9..40f5058dfa 100644 --- a/application/Espo/Core/Portal/Utils/Config.php +++ b/application/Espo/Core/Portal/Utils/Config.php @@ -34,15 +34,15 @@ use Espo\Core\{ Utils\Config as BaseConfig, }; -use StdClass; +use stdClass; class Config extends BaseConfig { - protected $portalParamsSet = false; + private $portalParamsSet = false; - protected $portalData = []; + private $portalData = []; - protected $portalParamList = [ + private $portalParamList = [ 'companyLogoId', 'tabList', 'quickCreateList', @@ -75,9 +75,9 @@ class Config extends BaseConfig return parent::has($name); } - public function getAllData(): StdClass + public function getAllNonInternalData(): stdClass { - $data = parent::getAllData(); + $data = parent::getAllNonInternalData(); foreach ($this->portalData as $k => $v) { $data->$k = $v; diff --git a/application/Espo/Core/Utils/Config.php b/application/Espo/Core/Utils/Config.php index 23c216136b..8640c04810 100644 --- a/application/Espo/Core/Utils/Config.php +++ b/application/Espo/Core/Utils/Config.php @@ -46,6 +46,8 @@ class Config protected $configPath = 'data/config.php'; + private $internalConfigPath = 'data/config-internal.php'; + private $cacheTimestamp = 'cacheTimestamp'; protected $associativeArrayAttributeList = [ @@ -63,6 +65,8 @@ class Config private $fileManager; + private $internalParamList = []; + public function __construct(ConfigFileManager $fileManager) { $this->fileManager = $fileManager; @@ -78,6 +82,16 @@ class Config return $this->configPath; } + /** + * A path to the internal config file. + * + * @todo Move to ConfigData. + */ + public function getInternalConfigPath(): string + { + return $this->internalConfigPath; + } + /** * Get a parameter value. * @@ -87,7 +101,7 @@ class Config { $keys = explode('.', $name); - $lastBranch = $this->loadConfig(); + $lastBranch = $this->getData(); foreach ($keys as $key) { if (!is_array($lastBranch) && !is_object($lastBranch)) { @@ -117,7 +131,7 @@ class Config { $keys = explode('.', $name); - $lastBranch = $this->loadConfig(); + $lastBranch = $this->getData(); foreach ($keys as $key) { if (!is_array($lastBranch) && !is_object($lastBranch)) { @@ -148,7 +162,7 @@ class Config */ public function update() { - $this->loadConfig(true); + $this->load(); } /** @@ -255,7 +269,7 @@ class Config $this->changedData = []; $this->removeData = []; - $this->loadConfig(true); + $this->load(); } return $result; @@ -272,43 +286,68 @@ class Config return $this->fileManager->getPhpContents($this->defaultConfigPath); } - protected function loadConfig(bool $reload = false) + private function isLoaded(): bool { - if (!$reload && isset($this->data) && !empty($this->data)) { + return isset($this->data) && !empty($this->data); + } + + private function getData(bool $reload = false): array + { + if (!$reload && $this->isLoaded()) { return $this->data; } - $configPath = $this->fileManager->isFile($this->configPath) ? - $this->configPath : - $this->defaultConfigPath; - - $this->data = $this->fileManager->getPhpContents($configPath); - - $systemConfig = $this->fileManager->getPhpContents($this->systemConfigPath); - - $this->data = Util::merge($systemConfig, $this->data); - - $this->fileManager->setConfig($this); + $this->load(); return $this->data; } - /** - * Get all parameters. - */ - public function getAllData(): stdClass + private function load(): void { - return (object) $this->loadConfig(); + $configPath = $this->fileManager->isFile($this->configPath) ? + $this->configPath : + $this->defaultConfigPath; + + $data = $this->fileManager->getPhpContents($configPath); + $systemData = $this->fileManager->getPhpContents($this->systemConfigPath); + + $internalData = $this->fileManager->isFile($this->internalConfigPath) ? + $this->fileManager->getPhpContents($this->internalConfigPath) : []; + + $this->data = Util::merge($systemData, $data); + $this->data = Util::merge($this->data, $internalData); + + $this->internalParamList = array_keys($internalData); + + $this->fileManager->setConfig($this); } - /** @deprecated */ - public function getData() + /** + * Get all parameters excluding those that are set in the internal config. + */ + public function getAllNonInternalData(): stdClass { - $data = $this->loadConfig(); + $data = (object) $this->getData(); + + foreach ($this->internalParamList as $param) { + unset($data->$param); + } return $data; } + /** + * Whether a parameter is set in the internal config. + */ + public function isInternal(string $name): bool + { + if (!$this->isLoaded()) { + $this->load(); + } + + return in_array($name, $this->internalParamList); + } + /** @deprecated */ public function setData($data) { diff --git a/application/Espo/Core/Utils/Config/Access.php b/application/Espo/Core/Utils/Config/Access.php index 95a07b48b6..954a5929f2 100644 --- a/application/Espo/Core/Utils/Config/Access.php +++ b/application/Espo/Core/Utils/Config/Access.php @@ -35,18 +35,6 @@ use Espo\Core\Utils\FieldUtil; class Access { - public const DEFAULT_ACCESS_LEVEL = self::ACCESS_LEVEL_USER; - - public const ACCESS_LEVEL_GLOBAL = 'global'; - - public const ACCESS_LEVEL_USER = 'user'; - - public const ACCESS_LEVEL_ADMIN = 'admin'; - - public const ACCESS_LEVEL_SUPER_ADMIN = 'superAdmin'; - - public const ACCESS_LEVEL_SYSTEM = 'system'; - private $config; private $metadata; @@ -123,6 +111,16 @@ class Access } } + $params = $this->metadata->get(['app', 'config', 'params']) ?? []; + + foreach ($params as $name => $item) { + if (empty($item['system'])) { + continue; + } + + $itemList[] = $name; + } + return array_values($itemList); } diff --git a/application/Espo/Core/Utils/Config/ConfigWriter.php b/application/Espo/Core/Utils/Config/ConfigWriter.php index 1469a45f20..e9f3b8f71f 100644 --- a/application/Espo/Core/Utils/Config/ConfigWriter.php +++ b/application/Espo/Core/Utils/Config/ConfigWriter.php @@ -29,10 +29,8 @@ namespace Espo\Core\Utils\Config; -use Espo\Core\{ - Utils\Config, - Exceptions\Error, -}; +use Espo\Core\Utils\Config; +use Espo\Core\Exceptions\Error; use Exception; @@ -54,20 +52,24 @@ class ConfigWriter private $cacheTimestampParam = 'cacheTimestamp'; - protected $config; + private $config; - protected $fileManager; + private $fileManager; - protected $helper; + private $helper; + + private $internalConfigHelper; public function __construct( Config $config, ConfigWriterFileManager $fileManager, - ConfigWriterHelper $helper + ConfigWriterHelper $helper, + InternalConfigHelper $internalConfigHelper ) { $this->config = $config; $this->fileManager = $fileManager; $this->helper = $helper; + $this->internalConfigHelper = $internalConfigHelper; } /** @@ -112,6 +114,7 @@ class ConfigWriter } $configPath = $this->config->getConfigPath(); + $internalConfigPath = $this->config->getInternalConfigPath(); if (!$this->fileManager->isFile($configPath)) { throw new Error("Config file '{$configPath}' was not found."); @@ -119,6 +122,9 @@ class ConfigWriter $data = $this->fileManager->getPhpContents($configPath); + $dataInternal = $this->fileManager->isFile($internalConfigPath) ? + $this->fileManager->getPhpContents($internalConfigPath) : []; + if (!is_array($data)) { $data = $this->fileManager->getPhpContents($configPath); } @@ -127,39 +133,41 @@ class ConfigWriter throw new Error("Could not read config."); } + if (!is_array($dataInternal)) { + throw new Error("Could not read config-internal."); + } + + $toSaveInternal = false; + foreach ($changedData as $key => $value) { + if ($this->internalConfigHelper->isParamForInternalConfig($key)) { + $dataInternal[$key] = $value; + unset($data[$key]); + + $toSaveInternal = true; + + continue; + } + $data[$key] = $value; } foreach ($this->removeParamList as $key) { + if ($this->internalConfigHelper->isParamForInternalConfig($key)) { + unset($dataInternal[$key]); + + $toSaveInternal = true; + + continue; + } + unset($data[$key]); } - if (!is_array($data)) { - throw new Error("Invalid config data while saving."); - } + $this->saveData($configPath, $data, 'microtime'); - $data['microtime'] = $microtime = $this->helper->generateMicrotime(); - - try { - $this->fileManager->putPhpContents($configPath, $data); - } - catch (Exception $e) { - throw new Error("Could not save config."); - } - - $reloadedData = $this->fileManager->getPhpContents($configPath); - - if ( - !is_array($reloadedData) || - $microtime !== ($reloadedData['microtime'] ?? null) - ) { - try { - $this->fileManager->putPhpContentsNoRenaming($configPath, $data); - } - catch (Exception $e) { - throw new Error("Could not save config."); - } + if ($toSaveInternal) { + $this->saveData($internalConfigPath, $dataInternal, 'microtimeInternal'); } $this->changedData = []; @@ -168,6 +176,34 @@ class ConfigWriter $this->config->update(); } + private function saveData(string $path, array &$data, string $timeParam): void + { + $data[$timeParam] = $microtime = $this->helper->generateMicrotime(); + + try { + $this->fileManager->putPhpContents($path, $data); + } + catch (Exception $e) { + throw new Error("Could not save config."); + } + + $reloadedData = $this->fileManager->getPhpContents($path); + + if ( + is_array($reloadedData) && + $microtime === ($reloadedData[$timeParam] ?? null) + ) { + return; + } + + try { + $this->fileManager->putPhpContentsNoRenaming($path, $data); + } + catch (Exception $e) { + throw new Error("Could not save config."); + } + } + /** * Update the cache timestamp. * diff --git a/application/Espo/Core/Utils/Config/InternalConfigHelper.php b/application/Espo/Core/Utils/Config/InternalConfigHelper.php new file mode 100644 index 0000000000..6be6315355 --- /dev/null +++ b/application/Espo/Core/Utils/Config/InternalConfigHelper.php @@ -0,0 +1,63 @@ +config = $config; + $this->metadata = $metadata; + } + + public function isParamForInternalConfig(string $name): bool + { + if ($this->config->isInternal($name)) { + return true; + } + + if (in_array($name, $this->config->get('systemItems') ?? [])) { + return true; + } + + if ($this->metadata->get(['app', 'config', 'params', $name, 'system'])) { + return true; + } + + return false; + } +} diff --git a/application/Espo/Resources/metadata/app/config.json b/application/Espo/Resources/metadata/app/config.json index 3c1f7ca1f9..bdff46104c 100644 --- a/application/Espo/Resources/metadata/app/config.json +++ b/application/Espo/Resources/metadata/app/config.json @@ -11,5 +11,10 @@ "historyEntityList", "streamEmailNotificationsTypeList", "emailKeepParentTeamsEntityList" - ] + ], + "params": { + "awsS3Storage": { + "system": true + } + } } diff --git a/application/Espo/Resources/metadata/entityDefs/Settings.json b/application/Espo/Resources/metadata/entityDefs/Settings.json index a9967ca25a..36c84d4c51 100644 --- a/application/Espo/Resources/metadata/entityDefs/Settings.json +++ b/application/Espo/Resources/metadata/entityDefs/Settings.json @@ -634,8 +634,7 @@ "tooltip": true }, "awsS3Storage": { - "type": "jsonObject", - "onlySystem": true + "type": "jsonObject" } } } diff --git a/application/Espo/Services/Settings.php b/application/Espo/Services/Settings.php index 3a5f27ec4a..572fda583c 100644 --- a/application/Espo/Services/Settings.php +++ b/application/Espo/Services/Settings.php @@ -101,7 +101,7 @@ class Settings public function getConfigData(): stdClass { - $data = $this->config->getAllData(); + $data = $this->config->getAllNonInternalData(); $this->filterDataByAccess($data); $this->filterData($data); diff --git a/tests/unit/Espo/Core/Utils/Config/ConfigWriterTest.php b/tests/unit/Espo/Core/Utils/Config/ConfigWriterTest.php index 6378ae0c01..2f60d43dfe 100644 --- a/tests/unit/Espo/Core/Utils/Config/ConfigWriterTest.php +++ b/tests/unit/Espo/Core/Utils/Config/ConfigWriterTest.php @@ -27,13 +27,14 @@ * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. ************************************************************************/ -namespace tests\unit\Espo\Core\Utils; +namespace tests\unit\Espo\Core\Utils\Config; use Espo\Core\{ Utils\Config, Utils\Config\ConfigWriter, Utils\Config\ConfigWriterFileManager, Utils\Config\ConfigWriterHelper, + Utils\Config\InternalConfigHelper, }; class ConfigWriterTest extends \PHPUnit\Framework\TestCase @@ -46,14 +47,27 @@ class ConfigWriterTest extends \PHPUnit\Framework\TestCase $this->helper = $this->createMock(ConfigWriterHelper::class); - $this->configWriter = new ConfigWriter($this->config, $this->fileManager, $this->helper); + $this->internalConfigHelper = $this->createMock(InternalConfigHelper::class); + + $this->configWriter = new ConfigWriter( + $this->config, + $this->fileManager, + $this->helper, + $this->internalConfigHelper + ); $this->configPath = 'somepath'; + $this->internalConfigPath = 'internalSomepath'; $this->config ->expects($this->any()) ->method('getConfigPath') ->willReturn($this->configPath); + + $this->config + ->expects($this->any()) + ->method('getInternalConfigPath') + ->willReturn($this->internalConfigPath); } public function testSave1() @@ -97,10 +111,15 @@ class ConfigWriterTest extends \PHPUnit\Framework\TestCase ->method('update'); $this->fileManager - ->expects($this->once()) ->method('isFile') - ->with($this->configPath) - ->willReturn(true); + ->withConsecutive( + [$this->configPath], + [$this->internalConfigPath], + ) + ->willReturnOnConsecutiveCalls( + true, + false + ); $this->fileManager ->expects($this->once()) diff --git a/tests/unit/Espo/Core/Utils/ConfigTest.php b/tests/unit/Espo/Core/Utils/ConfigTest.php index 9c51cadbad..18dc6c5ab5 100644 --- a/tests/unit/Espo/Core/Utils/ConfigTest.php +++ b/tests/unit/Espo/Core/Utils/ConfigTest.php @@ -70,20 +70,21 @@ class ConfigTest extends \PHPUnit\Framework\TestCase public function testLoadConfig() { - $this->assertArrayHasKey('database', $this->reflection->invokeMethod('loadConfig', array())); + $this->assertArrayHasKey('database', $this->reflection->invokeMethod('getData', [])); - $this->assertArrayHasKey('dateFormat', $this->reflection->invokeMethod('loadConfig', array())); + $this->assertArrayHasKey('dateFormat', $this->reflection->invokeMethod('getData', [])); } public function testGet() { - $result = array( + $result = [ 'driver' => 'pdo_mysql', 'host' => 'localhost', 'dbname' => 'espocrm', 'user' => 'root', 'password' => '', - ); + ]; + $this->assertEquals($result, $this->config->get('database')); $result = 'pdo_mysql'; @@ -102,7 +103,7 @@ class ConfigTest extends \PHPUnit\Framework\TestCase $this->assertArrayNotHasKey('systemItems', $configDataWithoutSystem); $this->assertArrayNotHasKey('adminItems', $configDataWithoutSystem); - $configData = $this->reflection->invokeMethod('loadConfig', array()); + $configData = $this->reflection->invokeMethod('getData', []); $this->assertArrayHasKey('systemItems', $configData); $this->assertArrayHasKey('adminItems', $configData);