From b4096251babf9d48ae3679637f3b3684ad28267a Mon Sep 17 00:00:00 2001 From: Philipp Date: Thu, 5 Jan 2023 21:42:35 +0100 Subject: [PATCH 01/19] Check 'config' table as fallback for migrations --- src/Core/Update.php | 42 +++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index e5ee587dc..d440136b5 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -61,8 +61,18 @@ class Update $build = DI::config()->get('system', 'build'); if (empty($build)) { - DI::config()->set('system', 'build', DB_UPDATE_VERSION - 1); - $build = DB_UPDATE_VERSION - 1; + // legacy option - check if there's something in the Config table + if (DBStructure::existsTable('config')) { + $dbConfig = DBA::selectFirst('config', ['v'], ['cat' => 'system', 'k' => 'build']); + if (!empty($dbConfig)) { + $build = $dbConfig['v']; + } + } + + if (empty($build)) { + DI::config()->set('system', 'build', DB_UPDATE_VERSION - 1); + $build = DB_UPDATE_VERSION - 1; + } } // We don't support upgrading from very old versions anymore @@ -119,11 +129,21 @@ class Update DI::lock()->release('dbupdate', true); } - $build = DI::config()->get('system', 'build', null); + $build = DI::config()->get('system', 'build'); - if (empty($build) || ($build > DB_UPDATE_VERSION)) { - $build = DB_UPDATE_VERSION - 1; - DI::config()->set('system', 'build', $build); + if (empty($build)) { + // legacy option - check if there's something in the Config table + if (DBStructure::existsTable('config')) { + $dbConfig = DBA::selectFirst('config', ['v'], ['cat' => 'system', 'k' => 'build']); + if (!empty($dbConfig)) { + $build = $dbConfig['v']; + } + } + + if (empty($build) || ($build > DB_UPDATE_VERSION)) { + DI::config()->set('system', 'build', DB_UPDATE_VERSION - 1); + $build = DB_UPDATE_VERSION - 1; + } } if ($build != DB_UPDATE_VERSION || $force) { @@ -141,7 +161,15 @@ class Update Logger::notice('Update starting.', ['from' => $stored, 'to' => $current]); // Checks if the build changed during Lock acquiring (so no double update occurs) - $retryBuild = DI::config()->get('system', 'build', null); + $retryBuild = DI::config()->get('system', 'build'); + // legacy option - check if there's something in the Config table + if (DBStructure::existsTable('config')) { + $dbConfig = DBA::selectFirst('config', ['v'], ['cat' => 'system', 'k' => 'build']); + if (!empty($dbConfig)) { + $retryBuild = $dbConfig['v']; + } + } + if ($retryBuild !== $build) { Logger::notice('Update already done.', ['from' => $stored, 'to' => $current]); DI::lock()->release('dbupdate'); From cdd57275eb7c0b6d41d5fb1f09b3f2816b3972f1 Mon Sep 17 00:00:00 2001 From: Philipp Date: Thu, 5 Jan 2023 22:13:10 +0100 Subject: [PATCH 02/19] Some improvements - Move $_SERVER into ConfigFileManager constructor - Rename "creatConfigFileLoader" to "createConfigFileManager" - Rename variable "loader" to "manager" in all tests --- src/App.php | 2 +- src/Core/Config/Factory/Config.php | 9 +- src/Core/Config/Model/Config.php | 9 +- src/Core/Config/Util/ConfigFileManager.php | 25 +-- src/Module/Admin/Summary.php | 2 +- static/dependencies.config.php | 4 +- tests/FixtureTest.php | 2 +- tests/src/Core/Cache/DatabaseCacheTest.php | 8 +- .../Config/Cache/ConfigFileManagerTest.php | 166 +++++++++--------- tests/src/Core/Config/ConfigTest.php | 2 +- .../src/Core/Storage/DatabaseStorageTest.php | 8 +- .../Storage/Repository/StorageManagerTest.php | 22 +-- 12 files changed, 134 insertions(+), 125 deletions(-) diff --git a/src/App.php b/src/App.php index 18a696734..fc70f920d 100644 --- a/src/App.php +++ b/src/App.php @@ -359,7 +359,7 @@ class App $this->profiler->update($this->config); Core\Hook::loadHooks(); - $loader = (new Config())->createConfigFileLoader($this->getBasePath(), $_SERVER); + $loader = (new Config())->createConfigFileManager($this->getBasePath(), $_SERVER); Core\Hook::callAll('load_config', $loader); // Hooks are now working, reload the whole definitions with hook enabled diff --git a/src/Core/Config/Factory/Config.php b/src/Core/Config/Factory/Config.php index e1094fd70..75c91540c 100644 --- a/src/Core/Config/Factory/Config.php +++ b/src/Core/Config/Factory/Config.php @@ -56,7 +56,7 @@ class Config * * @return Util\ConfigFileManager */ - public function createConfigFileLoader(string $basePath, array $server = []): Util\ConfigFileManager + public function createConfigFileManager(string $basePath, array $server = []): Util\ConfigFileManager { if (!empty($server[self::CONFIG_DIR_ENV]) && is_dir($server[self::CONFIG_DIR_ENV])) { $configDir = $server[self::CONFIG_DIR_ENV]; @@ -65,19 +65,18 @@ class Config } $staticDir = $basePath . DIRECTORY_SEPARATOR . self::STATIC_DIR; - return new Util\ConfigFileManager($basePath, $configDir, $staticDir, new Util\ConfigFileTransformer()); + return new Util\ConfigFileManager($basePath, $configDir, $staticDir, $server); } /** * @param Util\ConfigFileManager $configFileManager The Config Cache manager (INI/config/.htconfig) - * @param array $server * * @return Cache */ - public function createCache(Util\ConfigFileManager $configFileManager, array $server = []): Cache + public function createCache(Util\ConfigFileManager $configFileManager): Cache { $configCache = new Cache(); - $configFileManager->setupCache($configCache, $server); + $configFileManager->setupCache($configCache); return $configCache; } diff --git a/src/Core/Config/Model/Config.php b/src/Core/Config/Model/Config.php index 593ab0407..51025c874 100644 --- a/src/Core/Config/Model/Config.php +++ b/src/Core/Config/Model/Config.php @@ -39,19 +39,14 @@ class Config implements IManageConfigValues /** @var ConfigFileManager */ protected $configFileManager; - /** @var array */ - protected $server; - /** * @param ConfigFileManager $configFileManager The configuration file manager to save back configs * @param Cache $configCache The configuration cache (based on the config-files) - * @param array $server The $_SERVER variable */ - public function __construct(ConfigFileManager $configFileManager, Cache $configCache, array $server = []) + public function __construct(ConfigFileManager $configFileManager, Cache $configCache) { $this->configFileManager = $configFileManager; $this->configCache = $configCache; - $this->server = $server; } /** @@ -87,7 +82,7 @@ class Config implements IManageConfigValues $configCache = new Cache(); try { - $this->configFileManager->setupCache($configCache, $this->server); + $this->configFileManager->setupCache($configCache); } catch (ConfigFileException $e) { throw new ConfigPersistenceException('Cannot reload config', $e); } diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index cc264ea26..6951c2428 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -69,16 +69,22 @@ class ConfigFileManager */ private $staticDir; + /** + * @var array + */ + private $server; + /** * @param string $baseDir The base * @param string $configDir * @param string $staticDir */ - public function __construct(string $baseDir, string $configDir, string $staticDir) + public function __construct(string $baseDir, string $configDir, string $staticDir, array $server = []) { $this->baseDir = $baseDir; $this->configDir = $configDir; $this->staticDir = $staticDir; + $this->server = $server; } /** @@ -88,12 +94,11 @@ class ConfigFileManager * expected local.config.php * * @param Cache $config The config cache to load to - * @param array $server The $_SERVER array * @param bool $raw Set up the raw config format * * @throws ConfigFileException */ - public function setupCache(Cache $config, array $server = [], bool $raw = false) + public function setupCache(Cache $config, bool $raw = false) { // Load static config files first, the order is important $config->load($this->loadStaticConfig('defaults'), Cache::SOURCE_STATIC); @@ -109,7 +114,7 @@ class ConfigFileManager // Now load the node.config.php file with the node specific config values (based on admin gui/console actions) $this->loadDataConfig($config); - $config->load($this->loadEnvConfig($server), Cache::SOURCE_ENV); + $config->load($this->loadEnvConfig(), Cache::SOURCE_ENV); // In case of install mode, add the found basepath (because there isn't a basepath set yet if (!$raw && empty($config->get('system', 'basepath'))) { @@ -250,13 +255,11 @@ class ConfigFileManager /** * Tries to load environment specific variables, based on the `env.config.php` mapping table * - * @param array $server The $_SERVER variable - * * @return array The config array (empty if no config was found) * * @throws ConfigFileException if the configuration file isn't readable */ - public function loadEnvConfig(array $server): array + protected function loadEnvConfig(): array { $filepath = $this->staticDir . DIRECTORY_SEPARATOR . // /var/www/html/static/ "env.config.php"; // env.config.php @@ -270,8 +273,8 @@ class ConfigFileManager $return = []; foreach ($envConfig as $envKey => $configStructure) { - if (isset($server[$envKey])) { - $return[$configStructure[0]][$configStructure[1]] = $server[$envKey]; + if (isset($this->server[$envKey])) { + $return[$configStructure[0]][$configStructure[1]] = $this->server[$envKey]; } } @@ -296,7 +299,9 @@ class ConfigFileManager $sampleEnd = self::SAMPLE_END . ($ini ? '.ini.php' : '.config.php'); foreach ($files as $filename) { - if (fnmatch($filePattern, $filename) && substr_compare($filename, $sampleEnd, -strlen($sampleEnd))) { + if (fnmatch($filePattern, $filename) && + substr_compare($filename, $sampleEnd, -strlen($sampleEnd)) && + $filename !== self::CONFIG_DATA_FILE) { $found[] = $this->configDir . '/' . $filename; } } diff --git a/src/Module/Admin/Summary.php b/src/Module/Admin/Summary.php index 6244c5c9b..038628ee1 100644 --- a/src/Module/Admin/Summary.php +++ b/src/Module/Admin/Summary.php @@ -154,7 +154,7 @@ class Summary extends BaseAdmin } // check legacy basepath settings - $configLoader = (new Config())->createConfigFileLoader($a->getBasePath(), $_SERVER); + $configLoader = (new Config())->createConfigFileManager($a->getBasePath(), $_SERVER); $configCache = new Cache(); $configLoader->setupCache($configCache); $confBasepath = $configCache->get('system', 'basepath'); diff --git a/static/dependencies.config.php b/static/dependencies.config.php index 1844001bb..6feb76989 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -79,7 +79,7 @@ return [ Config\Util\ConfigFileManager::class => [ 'instanceOf' => Config\Factory\Config::class, 'call' => [ - ['createConfigFileLoader', [ + ['createConfigFileManager', [ [Dice::INSTANCE => '$basepath'], $_SERVER, ], Dice::CHAIN_CALL], @@ -88,7 +88,7 @@ return [ Config\ValueObject\Cache::class => [ 'instanceOf' => Config\Factory\Config::class, 'call' => [ - ['createCache', [$_SERVER], Dice::CHAIN_CALL], + ['createCache', [], Dice::CHAIN_CALL], ], ], App\Mode::class => [ diff --git a/tests/FixtureTest.php b/tests/FixtureTest.php index efb928031..e0d2f2337 100644 --- a/tests/FixtureTest.php +++ b/tests/FixtureTest.php @@ -62,7 +62,7 @@ abstract class FixtureTest extends DatabaseTest ->addRules(include __DIR__ . '/../static/dependencies.config.php') ->addRule(ConfigFileManager::class, [ 'instanceOf' => Config::class, - 'call' => [['createConfigFileLoader', [$this->root->url(), $server,], + 'call' => [['createConfigFileManager', [$this->root->url(), $server,], Dice::CHAIN_CALL]]]) ->addRule(Database::class, ['instanceOf' => StaticDatabase::class, 'shared' => true]) ->addRule(IHandleSessions::class, ['instanceOf' => Memory::class, 'shared' => true, 'call' => null]) diff --git a/tests/src/Core/Cache/DatabaseCacheTest.php b/tests/src/Core/Cache/DatabaseCacheTest.php index 634e2dcf7..2a837b71f 100644 --- a/tests/src/Core/Cache/DatabaseCacheTest.php +++ b/tests/src/Core/Cache/DatabaseCacheTest.php @@ -54,11 +54,11 @@ class DatabaseCacheTest extends CacheTest $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); // load real config to avoid mocking every config-entry which is related to the Database class - $configFactory = new Config(); - $loader = (new Config())->createConfigFileLoader($this->root->url(), []); - $configCache = $configFactory->createCache($loader); + $configFactory = new Config(); + $configFileManager = (new Config())->createConfigFileManager($this->root->url(), []); + $configCache = $configFactory->createCache($configFileManager); - $dbaDefinition = (new DbaDefinition($configCache->get('system', 'basepath')))->load(); + $dbaDefinition = (new DbaDefinition($configCache->get('system', 'basepath')))->load(); $viewDefinition = (new ViewDefinition($configCache->get('system', 'basepath')))->load(); $dba = new StaticDatabase($configCache, $profiler, $dbaDefinition, $viewDefinition); diff --git a/tests/src/Core/Config/Cache/ConfigFileManagerTest.php b/tests/src/Core/Config/Cache/ConfigFileManagerTest.php index 55dff2f4a..89d757e32 100644 --- a/tests/src/Core/Config/Cache/ConfigFileManagerTest.php +++ b/tests/src/Core/Config/Cache/ConfigFileManagerTest.php @@ -51,6 +51,7 @@ class ConfigFileManagerTest extends MockedTest $this->root->url() . DIRECTORY_SEPARATOR . Config::CONFIG_DIR, $this->root->url() . DIRECTORY_SEPARATOR . Config::STATIC_DIR ); + $configCache = new Cache(); $configFileLoader->setupCache($configCache); @@ -69,15 +70,15 @@ class ConfigFileManagerTest extends MockedTest $this->delConfigFile('local.config.php'); vfsStream::newFile('local.config.php') - ->at($this->root->getChild('config')) - ->setContent('at($this->root->getChild('config')) + ->setContent('root->url(), $this->root->url() . DIRECTORY_SEPARATOR . Config::CONFIG_DIR, $this->root->url() . DIRECTORY_SEPARATOR . Config::STATIC_DIR ); - $configCache = new Cache(); + $configCache = new Cache(); $configFileLoader->setupCache($configCache); } @@ -90,23 +91,23 @@ class ConfigFileManagerTest extends MockedTest $this->delConfigFile('local.config.php'); $file = dirname(__DIR__) . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - 'datasets' . DIRECTORY_SEPARATOR . - 'config' . DIRECTORY_SEPARATOR . - 'A.config.php'; + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR . + 'A.config.php'; vfsStream::newFile('local.config.php') - ->at($this->root->getChild('config')) - ->setContent(file_get_contents($file)); + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($file)); $configFileLoader = new ConfigFileManager( $this->root->url(), $this->root->url() . DIRECTORY_SEPARATOR . Config::CONFIG_DIR, $this->root->url() . DIRECTORY_SEPARATOR . Config::STATIC_DIR ); - $configCache = new Cache(); + $configCache = new Cache(); $configFileLoader->setupCache($configCache); @@ -127,23 +128,23 @@ class ConfigFileManagerTest extends MockedTest $this->delConfigFile('local.config.php'); $file = dirname(__DIR__) . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - 'datasets' . DIRECTORY_SEPARATOR . - 'config' . DIRECTORY_SEPARATOR . - 'A.ini.php'; + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR . + 'A.ini.php'; vfsStream::newFile('local.ini.php') - ->at($this->root->getChild('config')) - ->setContent(file_get_contents($file)); + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($file)); $configFileLoader = new ConfigFileManager( $this->root->url(), $this->root->url() . DIRECTORY_SEPARATOR . Config::CONFIG_DIR, $this->root->url() . DIRECTORY_SEPARATOR . Config::STATIC_DIR ); - $configCache = new Cache(); + $configCache = new Cache(); $configFileLoader->setupCache($configCache); @@ -163,23 +164,23 @@ class ConfigFileManagerTest extends MockedTest $this->delConfigFile('local.config.php'); $file = dirname(__DIR__) . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - 'datasets' . DIRECTORY_SEPARATOR . - 'config' . DIRECTORY_SEPARATOR . - '.htconfig.php'; + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR . + '.htconfig.php'; vfsStream::newFile('.htconfig.php') - ->at($this->root) - ->setContent(file_get_contents($file)); + ->at($this->root) + ->setContent(file_get_contents($file)); $configFileLoader = new ConfigFileManager( $this->root->url(), $this->root->url() . DIRECTORY_SEPARATOR . Config::CONFIG_DIR, $this->root->url() . DIRECTORY_SEPARATOR . Config::STATIC_DIR ); - $configCache = new Cache(); + $configCache = new Cache(); $configFileLoader->setupCache($configCache); @@ -217,16 +218,16 @@ class ConfigFileManagerTest extends MockedTest vfsStream::create($structure, $this->root); $file = dirname(__DIR__) . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - 'datasets' . DIRECTORY_SEPARATOR . - 'config' . DIRECTORY_SEPARATOR . - 'A.config.php'; + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR . + 'A.config.php'; vfsStream::newFile('test.config.php') - ->at($this->root->getChild('addon')->getChild('test')->getChild('config')) - ->setContent(file_get_contents($file)); + ->at($this->root->getChild('addon')->getChild('test')->getChild('config')) + ->setContent(file_get_contents($file)); $configFileLoader = new ConfigFileManager( $this->root->url(), @@ -252,25 +253,25 @@ class ConfigFileManagerTest extends MockedTest $this->delConfigFile('local.config.php'); $fileDir = dirname(__DIR__) . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - 'datasets' . DIRECTORY_SEPARATOR . - 'config' . DIRECTORY_SEPARATOR; + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR; vfsStream::newFile('A.config.php') - ->at($this->root->getChild('config')) - ->setContent(file_get_contents($fileDir . 'A.config.php')); + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($fileDir . 'A.config.php')); vfsStream::newFile('B.config.php') - ->at($this->root->getChild('config')) - ->setContent(file_get_contents($fileDir . 'B.config.php')); + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($fileDir . 'B.config.php')); $configFileLoader = new ConfigFileManager( $this->root->url(), $this->root->url() . DIRECTORY_SEPARATOR . Config::CONFIG_DIR, $this->root->url() . DIRECTORY_SEPARATOR . Config::STATIC_DIR ); - $configCache = new Cache(); + $configCache = new Cache(); $configFileLoader->setupCache($configCache); @@ -286,25 +287,25 @@ class ConfigFileManagerTest extends MockedTest $this->delConfigFile('local.config.php'); $fileDir = dirname(__DIR__) . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR . - 'datasets' . DIRECTORY_SEPARATOR . - 'config' . DIRECTORY_SEPARATOR; + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR; vfsStream::newFile('A.ini.php') - ->at($this->root->getChild('config')) - ->setContent(file_get_contents($fileDir . 'A.ini.php')); + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($fileDir . 'A.ini.php')); vfsStream::newFile('B.ini.php') - ->at($this->root->getChild('config')) - ->setContent(file_get_contents($fileDir . 'B.ini.php')); + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($fileDir . 'B.ini.php')); $configFileLoader = new ConfigFileManager( $this->root->url(), $this->root->url() . DIRECTORY_SEPARATOR . Config::CONFIG_DIR, $this->root->url() . DIRECTORY_SEPARATOR . Config::STATIC_DIR ); - $configCache = new Cache(); + $configCache = new Cache(); $configFileLoader->setupCache($configCache); @@ -320,24 +321,25 @@ class ConfigFileManagerTest extends MockedTest $this->delConfigFile('local.config.php'); $fileDir = dirname(__DIR__) . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR . - '..' . DIRECTORY_SEPARATOR . - 'datasets' . DIRECTORY_SEPARATOR . - 'config' . DIRECTORY_SEPARATOR; + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR; vfsStream::newFile('A.ini.php') - ->at($this->root->getChild('config')) - ->setContent(file_get_contents($fileDir . 'A.ini.php')); + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($fileDir . 'A.ini.php')); vfsStream::newFile('B-sample.ini.php') - ->at($this->root->getChild('config')) - ->setContent(file_get_contents($fileDir . 'B.ini.php')); + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($fileDir . 'B.ini.php')); $configFileLoader = new ConfigFileManager( $this->root->url(), $this->root->url() . DIRECTORY_SEPARATOR . Config::CONFIG_DIR, $this->root->url() . DIRECTORY_SEPARATOR . Config::STATIC_DIR ); + $configCache = new Cache(); $configFileLoader->setupCache($configCache); @@ -353,10 +355,10 @@ class ConfigFileManagerTest extends MockedTest { $this->delConfigFile('local.config.php'); - $configFileLoader = (new Config())->createConfigFileLoader($this->root->url(), ['FRIENDICA_CONFIG_DIR' => '/a/wrong/dir/']); - $configCache = new Cache(); + $configFileManager = (new Config())->createConfigFileManager($this->root->url(), ['FRIENDICA_CONFIG_DIR' => '/a/wrong/dir/']); + $configCache = new Cache(); - $configFileLoader->setupCache($configCache); + $configFileManager->setupCache($configCache); self::assertEquals($this->root->url(), $configCache->get('system', 'basepath')); } @@ -379,10 +381,13 @@ class ConfigFileManagerTest extends MockedTest ->at($this->root->getChild('config2')) ->setContent(file_get_contents($fileDir . 'B.config.php')); - $configFileLoader = (new Config())->createConfigFileLoader($this->root->url(), ['FRIENDICA_CONFIG_DIR' => $this->root->getChild('config2')->url()]); - $configCache = new Cache(); + $configFileManager = (new Config())->createConfigFileManager($this->root->url(), + [ + 'FRIENDICA_CONFIG_DIR' => $this->root->getChild('config2')->url(), + ]); + $configCache = new Cache(); - $configFileLoader->setupCache($configCache); + $configFileManager->setupCache($configCache); self::assertEquals('newValue', $configCache->get('system', 'newKey')); } @@ -402,10 +407,13 @@ class ConfigFileManagerTest extends MockedTest ->at($this->root->getChild('config2')) ->setContent(file_get_contents($fileDir . 'B.config.php')); - $configFileLoader = (new Config())->createConfigFileLoader($this->root->url(), ['FRIENDICA_CONFIG_DIR' => $this->root->getChild('config2')->url()]); - $configCache = new Cache(); + $configFileManager = (new Config())->createConfigFileManager($this->root->url(), + [ + 'FRIENDICA_CONFIG_DIR' => $this->root->getChild('config2')->url(), + ]); + $configCache = new Cache(); - $configFileLoader->setupCache($configCache); + $configFileManager->setupCache($configCache); $specialChars = '!"§$%&/()(/&%$\'>set('config', 'test', 'it', Cache::SOURCE_DATA); $configCache->set('system', 'test_2', 2, Cache::SOURCE_DATA); $configCache->set('special_chars', 'special', $specialChars, Cache::SOURCE_DATA); - $configFileLoader->saveData($configCache); + $configFileManager->saveData($configCache); // Reload the configCache with the new values $configCache2 = new Cache(); - $configFileLoader->setupCache($configCache2); + $configFileManager->setupCache($configCache2); self::assertEquals($configCache, $configCache2); self::assertEquals([ - 'system' => [ - 'test' => 'it', + 'system' => [ + 'test' => 'it', 'test_2' => 2 ], - 'config' => [ + 'config' => [ 'test' => 'it', ], 'special_chars' => [ diff --git a/tests/src/Core/Config/ConfigTest.php b/tests/src/Core/Config/ConfigTest.php index 876a0b05b..d40af47d5 100644 --- a/tests/src/Core/Config/ConfigTest.php +++ b/tests/src/Core/Config/ConfigTest.php @@ -76,7 +76,7 @@ class ConfigTest extends MockedTest */ public function getInstance() { - $this->configFileManager->setupCache($this->configCache, []); + $this->configFileManager->setupCache($this->configCache); return new Config($this->configFileManager, $this->configCache); } diff --git a/tests/src/Core/Storage/DatabaseStorageTest.php b/tests/src/Core/Storage/DatabaseStorageTest.php index 30626a78a..7ac87245a 100644 --- a/tests/src/Core/Storage/DatabaseStorageTest.php +++ b/tests/src/Core/Storage/DatabaseStorageTest.php @@ -53,11 +53,11 @@ class DatabaseStorageTest extends StorageTest $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); // load real config to avoid mocking every config-entry which is related to the Database class - $configFactory = new Config(); - $loader = (new Config())->createConfigFileLoader($this->root->url(), []); - $configCache = $configFactory->createCache($loader); + $configFactory = new Config(); + $configFileManager = (new Config())->createConfigFileManager($this->root->url()); + $configCache = $configFactory->createCache($configFileManager); - $dbaDefinition = (new DbaDefinition($configCache->get('system', 'basepath')))->load(); + $dbaDefinition = (new DbaDefinition($configCache->get('system', 'basepath')))->load(); $viewDefinition = (new ViewDefinition($configCache->get('system', 'basepath')))->load(); $dba = new StaticDatabase($configCache, $profiler, $dbaDefinition, $viewDefinition); diff --git a/tests/src/Core/Storage/Repository/StorageManagerTest.php b/tests/src/Core/Storage/Repository/StorageManagerTest.php index db4ca8ef2..a55819ee0 100644 --- a/tests/src/Core/Storage/Repository/StorageManagerTest.php +++ b/tests/src/Core/Storage/Repository/StorageManagerTest.php @@ -52,6 +52,7 @@ use Friendica\Test\Util\SampleStorageBackend; class StorageManagerTest extends DatabaseTest { use VFSTrait; + /** @var Database */ private $dba; /** @var IManageConfigValues */ @@ -77,18 +78,19 @@ class StorageManagerTest extends DatabaseTest $profiler->shouldReceive('saveTimestamp')->withAnyArgs()->andReturn(true); // load real config to avoid mocking every config-entry which is related to the Database class - $configFactory = new Config(); - $loader = $configFactory->createConfigFileLoader($this->root->url(), []); - $configCache = $configFactory->createCache($loader); + $configFactory = new Config(); + $configFileManager = $configFactory->createConfigFileManager($this->root->url()); + $configCache = $configFactory->createCache($configFileManager); - $dbaDefinition = (new DbaDefinition($configCache->get('system', 'basepath')))->load(); + $dbaDefinition = (new DbaDefinition($configCache->get('system', 'basepath')))->load(); $viewDefinition = (new ViewDefinition($configCache->get('system', 'basepath')))->load(); $this->dba = new StaticDatabase($configCache, $profiler, $dbaDefinition, $viewDefinition); - $this->config = new \Friendica\Core\Config\Model\Config($loader, $configCache); + $this->config = new \Friendica\Core\Config\Model\Config($configFileManager, $configCache); $this->config->set('storage', 'name', 'Database'); - $this->config->set('storage', 'filesystem_path', $this->root->getChild(Type\FilesystemConfig::DEFAULT_BASE_FOLDER)->url()); + $this->config->set('storage', 'filesystem_path', $this->root->getChild(Type\FilesystemConfig::DEFAULT_BASE_FOLDER) + ->url()); $this->l10n = \Mockery::mock(L10n::class); } @@ -113,21 +115,21 @@ class StorageManagerTest extends DatabaseTest public function dataStorages() { return [ - 'empty' => [ + 'empty' => [ 'name' => '', 'valid' => false, 'interface' => ICanReadFromStorage::class, 'assert' => null, 'assertName' => '', ], - 'database' => [ + 'database' => [ 'name' => Type\Database::NAME, 'valid' => true, 'interface' => ICanWriteToStorage::class, 'assert' => Type\Database::class, 'assertName' => Type\Database::NAME, ], - 'filesystem' => [ + 'filesystem' => [ 'name' => Filesystem::NAME, 'valid' => true, 'interface' => ICanWriteToStorage::class, @@ -141,7 +143,7 @@ class StorageManagerTest extends DatabaseTest 'assert' => SystemResource::class, 'assertName' => SystemResource::NAME, ], - 'invalid' => [ + 'invalid' => [ 'name' => 'invalid', 'valid' => false, 'interface' => null, From 5aa8e8adf1af017ced555d8c33b78e71114f5cd9 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 01:02:47 +0100 Subject: [PATCH 03/19] Config fixings - Delete now really overwrites static default/setting.config.php keys - Delete now really overwrites static default/setting.config.php categories - The Update::check() routine is added to different places - Merge the given config file with the new config before writing - Remove ConfigTransaction::get() because it's no more reliable --- bin/auth_ejabberd.php | 4 + bin/daemon.php | 5 +- bin/worker.php | 3 +- index.php | 3 + src/Core/Config/Model/Config.php | 2 +- src/Core/Config/Model/ConfigTransaction.php | 40 +------ src/Core/Config/Util/ConfigFileManager.php | 104 +++++++++++------ .../Config/Util/ConfigFileTransformer.php | 10 +- src/Core/Config/ValueObject/Cache.php | 107 ++++++++---------- tests/src/Core/Config/Cache/CacheTest.php | 40 ++----- .../Config/Cache/ConfigFileManagerTest.php | 70 ++++++++++++ tests/src/Core/Config/ConfigTest.php | 2 +- .../src/Core/Config/ConfigTransactionTest.php | 12 -- 13 files changed, 221 insertions(+), 181 deletions(-) diff --git a/bin/auth_ejabberd.php b/bin/auth_ejabberd.php index ecff7218d..9c5dc0528 100755 --- a/bin/auth_ejabberd.php +++ b/bin/auth_ejabberd.php @@ -82,6 +82,10 @@ $dice = $dice->addRule(LoggerInterface::class,['constructParams' => ['auth_ejabb \Friendica\DI::init($dice); \Friendica\Core\Logger\Handler\ErrorHandler::register($dice->create(\Psr\Log\LoggerInterface::class)); + +// Check the database structure and possibly fixes it +\Friendica\Core\Update::check(\Friendica\DI::basePath(), true, \Friendica\DI::mode()); + $appMode = $dice->create(Mode::class); if ($appMode->isNormal()) { diff --git a/bin/daemon.php b/bin/daemon.php index 577e884eb..6237df49d 100755 --- a/bin/daemon.php +++ b/bin/daemon.php @@ -33,6 +33,7 @@ if (php_sapi_name() !== 'cli') { use Dice\Dice; use Friendica\App\Mode; use Friendica\Core\Logger; +use Friendica\Core\Update; use Friendica\Core\Worker; use Friendica\Database\DBA; use Friendica\DI; @@ -63,7 +64,9 @@ $dice = $dice->addRule(LoggerInterface::class,['constructParams' => ['daemon']]) DI::init($dice); \Friendica\Core\Logger\Handler\ErrorHandler::register($dice->create(\Psr\Log\LoggerInterface::class)); -$a = DI::app(); + +// Check the database structure and possibly fixes it +Update::check(DI::basePath(), true, DI::mode()); if (DI::mode()->isInstall()) { die("Friendica isn't properly installed yet.\n"); diff --git a/bin/worker.php b/bin/worker.php index 707e57972..aceeff481 100755 --- a/bin/worker.php +++ b/bin/worker.php @@ -58,12 +58,11 @@ $dice = $dice->addRule(LoggerInterface::class,['constructParams' => ['worker']]) DI::init($dice); \Friendica\Core\Logger\Handler\ErrorHandler::register($dice->create(\Psr\Log\LoggerInterface::class)); -$a = DI::app(); DI::mode()->setExecutor(Mode::WORKER); // Check the database structure and possibly fixes it -Update::check($a->getBasePath(), true, DI::mode()); +Update::check(DI::basePath(), true, DI::mode()); // Quit when in maintenance if (!DI::mode()->has(App\Mode::MAINTENANCEDISABLED)) { diff --git a/index.php b/index.php index fa6d8b087..10f04996a 100644 --- a/index.php +++ b/index.php @@ -36,6 +36,9 @@ $dice = $dice->addRule(Friendica\App\Mode::class, ['call' => [['determineRunMode \Friendica\Core\Logger\Handler\ErrorHandler::register($dice->create(\Psr\Log\LoggerInterface::class)); +// Check the database structure and possibly fixes it +\Friendica\Core\Update::check(\Friendica\DI::basePath(), true, \Friendica\DI::mode()); + $a = \Friendica\DI::app(); \Friendica\DI::mode()->setExecutor(\Friendica\App\Mode::INDEX); diff --git a/src/Core/Config/Model/Config.php b/src/Core/Config/Model/Config.php index 51025c874..c96750694 100644 --- a/src/Core/Config/Model/Config.php +++ b/src/Core/Config/Model/Config.php @@ -116,7 +116,7 @@ class Config implements IManageConfigValues /** {@inheritDoc} */ public function delete(string $cat, string $key): bool { - if ($this->configCache->delete($cat, $key)) { + if ($this->configCache->delete($cat, $key, Cache::SOURCE_DATA)) { $this->save(); return true; } else { diff --git a/src/Core/Config/Model/ConfigTransaction.php b/src/Core/Config/Model/ConfigTransaction.php index 26420b078..ec0a2b9c8 100644 --- a/src/Core/Config/Model/ConfigTransaction.php +++ b/src/Core/Config/Model/ConfigTransaction.php @@ -35,37 +35,13 @@ class ConfigTransaction implements ISetConfigValuesTransactionally protected $config; /** @var Cache */ protected $cache; - /** @var Cache */ - protected $delCache; /** @var bool field to check if something is to save */ protected $changedConfig = false; public function __construct(IManageConfigValues $config) { - $this->config = $config; - $this->cache = new Cache(); - $this->delCache = new Cache(); - } - - /** - * Get a particular user's config variable given the category name - * ($cat) and a $key from the current transaction. - * - * Isn't part of the interface because of it's rare use case - * - * @param string $cat The category of the configuration value - * @param string $key The configuration key to query - * - * @return mixed Stored value or null if it does not exist - * - * @throws ConfigPersistenceException In case the persistence layer throws errors - * - */ - public function get(string $cat, string $key) - { - return !$this->delCache->get($cat, $key) ? - ($this->cache->get($cat, $key) ?? $this->config->get($cat, $key)) : - null; + $this->config = $config; + $this->cache = clone $config->getCache(); } /** {@inheritDoc} */ @@ -81,8 +57,7 @@ class ConfigTransaction implements ISetConfigValuesTransactionally /** {@inheritDoc} */ public function delete(string $cat, string $key): ISetConfigValuesTransactionally { - $this->cache->delete($cat, $key); - $this->delCache->set($cat, $key, 'deleted'); + $this->cache->delete($cat, $key, Cache::SOURCE_DATA); $this->changedConfig = true; return $this; @@ -97,13 +72,8 @@ class ConfigTransaction implements ISetConfigValuesTransactionally } try { - $newCache = $this->config->getCache()->merge($this->cache); - $newCache = $newCache->diff($this->delCache); - $this->config->load($newCache); - - // flush current cache - $this->cache = new Cache(); - $this->delCache = new Cache(); + $this->config->load($this->cache); + $this->cache = clone $this->config->getCache(); } catch (\Exception $e) { throw new ConfigPersistenceException('Cannot save config', $e); } diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index 6951c2428..569bcdb97 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -93,33 +93,33 @@ class ConfigFileManager * First loads the default value for all the configuration keys, then the legacy configuration files, then the * expected local.config.php * - * @param Cache $config The config cache to load to - * @param bool $raw Set up the raw config format + * @param Cache $configCache The config cache to load to + * @param bool $raw Set up the raw config format * * @throws ConfigFileException */ - public function setupCache(Cache $config, bool $raw = false) + public function setupCache(Cache $configCache, bool $raw = false) { // Load static config files first, the order is important - $config->load($this->loadStaticConfig('defaults'), Cache::SOURCE_STATIC); - $config->load($this->loadStaticConfig('settings'), Cache::SOURCE_STATIC); + $configCache->load($this->loadStaticConfig('defaults'), Cache::SOURCE_STATIC); + $configCache->load($this->loadStaticConfig('settings'), Cache::SOURCE_STATIC); // try to load the legacy config first - $config->load($this->loadLegacyConfig('htpreconfig'), Cache::SOURCE_FILE); - $config->load($this->loadLegacyConfig('htconfig'), Cache::SOURCE_FILE); + $configCache->load($this->loadLegacyConfig('htpreconfig'), Cache::SOURCE_FILE); + $configCache->load($this->loadLegacyConfig('htconfig'), Cache::SOURCE_FILE); // Now load every other config you find inside the 'config/' directory - $this->loadCoreConfig($config); + $this->loadCoreConfig($configCache); // Now load the node.config.php file with the node specific config values (based on admin gui/console actions) - $this->loadDataConfig($config); + $this->loadDataConfig($configCache); - $config->load($this->loadEnvConfig(), Cache::SOURCE_ENV); + $configCache->load($this->loadEnvConfig(), Cache::SOURCE_ENV); // In case of install mode, add the found basepath (because there isn't a basepath set yet - if (!$raw && empty($config->get('system', 'basepath'))) { + if (!$raw && empty($configCache->get('system', 'basepath'))) { // Setting at least the basepath we know - $config->set('system', 'basepath', $this->baseDir, Cache::SOURCE_FILE); + $configCache->set('system', 'basepath', $this->baseDir, Cache::SOURCE_FILE); } } @@ -139,7 +139,7 @@ class ConfigFileManager if (file_exists($configName)) { return $this->loadConfigFile($configName); - } elseif (file_exists($iniName)) { + } else if (file_exists($iniName)) { return $this->loadINIConfigFile($iniName); } else { return []; @@ -149,31 +149,31 @@ class ConfigFileManager /** * Tries to load the specified core-configuration into the config cache. * - * @param Cache $config The Config cache + * @param Cache $configCache The Config cache * * @throws ConfigFileException if the configuration file isn't readable */ - private function loadCoreConfig(Cache $config) + private function loadCoreConfig(Cache $configCache) { // try to load legacy ini-files first foreach ($this->getConfigFiles(true) as $configFile) { - $config->load($this->loadINIConfigFile($configFile), Cache::SOURCE_FILE); + $configCache->load($this->loadINIConfigFile($configFile), Cache::SOURCE_FILE); } // try to load supported config at last to overwrite it foreach ($this->getConfigFiles() as $configFile) { - $config->load($this->loadConfigFile($configFile), Cache::SOURCE_FILE); + $configCache->load($this->loadConfigFile($configFile), Cache::SOURCE_FILE); } } /** * Tries to load the data config file with the overridden data * - * @param Cache $config The Config cache + * @param Cache $configCache The Config cache * * @throws ConfigFileException In case the config file isn't loadable */ - private function loadDataConfig(Cache $config) + private function loadDataConfig(Cache $configCache) { $filename = $this->configDir . '/' . self::CONFIG_DATA_FILE; @@ -194,37 +194,69 @@ class ConfigFileManager $dataArray = eval('?>' . $content); - if (!is_array($dataArray)) { - throw new ConfigFileException(sprintf('Error loading config file %s', $filename)); + if (is_array($dataArray)) { + $configCache->load($dataArray, Cache::SOURCE_DATA); } - - $config->load($dataArray, Cache::SOURCE_DATA); } } /** * Saves overridden config entries back into the data.config.phpR * - * @param Cache $config The config cache + * @param Cache $configCache The config cache * * @throws ConfigFileException In case the config file isn't writeable or the data is invalid */ - public function saveData(Cache $config) + public function saveData(Cache $configCache) { - $data = $config->getDataBySource(Cache::SOURCE_DATA); + $filename = $this->configDir . '/' . self::CONFIG_DATA_FILE; - $encodedData = ConfigFileTransformer::encode($data); - - if (!$encodedData) { - throw new ConfigFileException('config source cannot get encoded'); + if (file_exists($filename)) { + $fileExists = true; + } else { + $fileExists = false; } - $configStream = fopen($this->configDir . '/' . self::CONFIG_DATA_FILE, 'w'); + $configStream = fopen($filename, 'c+'); - if (flock($configStream, LOCK_EX)) { - fwrite($configStream, $encodedData); - fflush($configStream); - flock($configStream, LOCK_UN); + try { + if (flock($configStream, LOCK_EX)) { + + if ($fileExists) { + clearstatcache(true, $filename); + $content = fread($configStream, filesize($filename)); + rewind($configStream); + if (!$content) { + throw new ConfigFileException(sprintf('Couldn\'t read file %s', $filename)); + } + + $dataArray = eval('?>' . $content); + + if (is_array($dataArray)) { + $fileConfigCache = new Cache(); + $fileConfigCache->load($dataArray, Cache::SOURCE_DATA); + $configCache = $fileConfigCache->merge($configCache); + } + } + + $data = $configCache->getDataBySource(Cache::SOURCE_DATA); + + $encodedData = ConfigFileTransformer::encode($data); + + if (!$encodedData) { + throw new ConfigFileException('config source cannot get encoded'); + } + + clearstatcache(true, $filename); + if (!ftruncate($configStream, 0) || + !fwrite($configStream, $encodedData) || + !fflush($configStream) || + !flock($configStream, LOCK_UN)) { + throw new ConfigFileException(sprintf('Cannot modify locked file %s', $filename)); + } + } + } finally { + fclose($configStream); } } @@ -242,7 +274,7 @@ class ConfigFileManager $filepath = $this->baseDir . DIRECTORY_SEPARATOR . // /var/www/html/ Addon::DIRECTORY . DIRECTORY_SEPARATOR . // addon/ $name . DIRECTORY_SEPARATOR . // openstreetmap/ - 'config'. DIRECTORY_SEPARATOR . // config/ + 'config' . DIRECTORY_SEPARATOR . // config/ $name . ".config.php"; // openstreetmap.config.php if (file_exists($filepath)) { diff --git a/src/Core/Config/Util/ConfigFileTransformer.php b/src/Core/Config/Util/ConfigFileTransformer.php index 282714df2..4eaafe061 100644 --- a/src/Core/Config/Util/ConfigFileTransformer.php +++ b/src/Core/Config/Util/ConfigFileTransformer.php @@ -34,6 +34,12 @@ class ConfigFileTransformer $categories = array_keys($data); foreach ($categories as $category) { + + if (is_null($data[$category])) { + $dataString .= "\t'$category' => null," . PHP_EOL; + continue; + } + $dataString .= "\t'$category' => [" . PHP_EOL; if (is_array($data[$category])) { @@ -66,7 +72,9 @@ class ConfigFileTransformer { $string = str_repeat("\t", $level + 2) . "'$key' => "; - if (is_array($value)) { + if (is_null($value)) { + $string .= "null,"; + } elseif (is_array($value)) { $string .= "[" . PHP_EOL; $string .= static::extractArray($value, ++$level); $string .= str_repeat("\t", $level + 1) . '],'; diff --git a/src/Core/Config/ValueObject/Cache.php b/src/Core/Config/ValueObject/Cache.php index b5af3280c..ca76bb4ed 100644 --- a/src/Core/Config/ValueObject/Cache.php +++ b/src/Core/Config/ValueObject/Cache.php @@ -65,7 +65,7 @@ class Cache * @param bool $hidePasswordOutput True, if cache variables should take extra care of password values * @param int $source Sets a source of the initial config values */ - public function __construct(array $config = [], bool $hidePasswordOutput = true, $source = self::SOURCE_DEFAULT) + public function __construct(array $config = [], bool $hidePasswordOutput = true, int $source = self::SOURCE_DEFAULT) { $this->hidePasswordOutput = $hidePasswordOutput; $this->load($config, $source); @@ -87,11 +87,10 @@ class Cache $keys = array_keys($config[$category]); foreach ($keys as $key) { - $value = $config[$category][$key]; - if (isset($value)) { - $this->set($category, $key, $value, $source); - } + $this->set($category, $key, $config[$category][$key] ?? null, $source); } + } else { + $this->set($category, null, $config[$category], $source); } } } @@ -150,6 +149,8 @@ class Cache $data[$category][$key] = $this->config[$category][$key]; } } + } elseif (is_int($this->source[$category])) { + $data[$category] = null; } } @@ -159,40 +160,49 @@ class Cache /** * Sets a value in the config cache. Accepts raw output from the config table * - * @param string $cat Config category - * @param string $key Config key - * @param mixed $value Value to set - * @param int $source The source of the current config key + * @param string $cat Config category + * @param ?string $key Config key + * @param ?mixed $value Value to set + * @param int $source The source of the current config key * * @return bool True, if the value is set */ - public function set(string $cat, string $key, $value, int $source = self::SOURCE_DEFAULT): bool + public function set(string $cat, string $key = null, $value = null, int $source = self::SOURCE_DEFAULT): bool { - if (!isset($this->config[$cat])) { + if (!isset($this->config[$cat]) && $key !== null) { $this->config[$cat] = []; $this->source[$cat] = []; } - if (isset($this->source[$cat][$key]) && - $source < $this->source[$cat][$key]) { + if ((isset($this->source[$cat][$key]) && $source < $this->source[$cat][$key]) || + (is_int($this->source[$cat] ?? null) && $source < $this->source[$cat])) { return false; } if ($this->hidePasswordOutput && $key == 'password' && is_string($value)) { - $this->config[$cat][$key] = new HiddenString((string)$value); + $this->setCatKeyValueSource($cat, $key, new HiddenString((string)$value), $source); } else if (is_string($value)) { - $this->config[$cat][$key] = self::toConfigValue($value); + $this->setCatKeyValueSource($cat, $key, self::toConfigValue($value), $source); } else { - $this->config[$cat][$key] = $value; + $this->setCatKeyValueSource($cat, $key, $value, $source); } - $this->source[$cat][$key] = $source; - return true; } + private function setCatKeyValueSource(string $cat, string $key = null, $value = null, int $source = self::SOURCE_DEFAULT) + { + if (isset($key)) { + $this->config[$cat][$key] = $value; + $this->source[$cat][$key] = $source; + } else { + $this->config[$cat] = $value; + $this->source[$cat] = $source; + } + } + /** * Formats a DB value to a config value * - null = The db-value isn't set @@ -222,24 +232,27 @@ class Cache /** * Deletes a value from the config cache. * - * @param string $cat Config category - * @param string $key Config key + * @param string $cat Config category + * @param ?string $key Config key (if not set, the whole category will get deleted) + * @param int $source The source of the current config key * * @return bool true, if deleted */ - public function delete(string $cat, string $key): bool + public function delete(string $cat, string $key = null, int $source = self::SOURCE_DEFAULT): bool { if (isset($this->config[$cat][$key])) { - unset($this->config[$cat][$key]); - unset($this->source[$cat][$key]); - if (count($this->config[$cat]) == 0) { - unset($this->config[$cat]); - unset($this->source[$cat]); + $this->config[$cat][$key] = null; + $this->source[$cat][$key] = $source; + if (empty(array_filter($this->config[$cat], function($value) { return !is_null($value); }))) { + $this->config[$cat] = null; + $this->source[$cat] = $source; } - return true; - } else { - return false; + } elseif (isset($this->config[$cat])) { + $this->config[$cat] = null; + $this->source[$cat] = $source; } + + return true; } /** @@ -302,39 +315,9 @@ class Cache $newConfig[$category][$key] = $cache->config[$category][$key]; $newSource[$category][$key] = $cache->source[$category][$key]; } - } - } - - $newCache = new Cache(); - $newCache->config = $newConfig; - $newCache->source = $newSource; - - return $newCache; - } - - - /** - * Diffs a new Cache into the existing one and returns the diffed Cache - * - * @param Cache $cache The cache, which should get deleted for the current Cache - * - * @return Cache The diffed Cache - */ - public function diff(Cache $cache): Cache - { - $newConfig = $this->config; - $newSource = $this->source; - - $categories = array_keys($cache->config); - - foreach ($categories as $category) { - if (is_array($cache->config[$category])) { - $keys = array_keys($cache->config[$category]); - - foreach ($keys as $key) { - unset($newConfig[$category][$key]); - unset($newSource[$category][$key]); - } + } else { + $newConfig[$category] = $cache->config[$category]; + $newSource[$category] = $cache->source[$category]; } } diff --git a/tests/src/Core/Config/Cache/CacheTest.php b/tests/src/Core/Config/Cache/CacheTest.php index 024c9c7e3..6fe2fe15f 100644 --- a/tests/src/Core/Config/Cache/CacheTest.php +++ b/tests/src/Core/Config/Cache/CacheTest.php @@ -67,6 +67,8 @@ class CacheTest extends MockedTest $configCache = new Cache(); $configCache->load($data); + print_r($configCache); + self::assertConfigValues($data, $configCache); } @@ -111,9 +113,9 @@ class CacheTest extends MockedTest } /** - * Test the loadConfigArray() method with wrong/empty datasets + * Test the loadConfigArray() method with only a category */ - public function testLoadConfigArrayWrong() + public function testLoadConfigArrayWithOnlyCategory() { $configCache = new Cache(); @@ -123,9 +125,10 @@ class CacheTest extends MockedTest // wrong dataset $configCache->load(['system' => 'not_array']); - self::assertEmpty($configCache->getAll()); + self::assertEquals(['system' => 'not_array'], $configCache->getAll()); // incomplete dataset (key is integer ID of the array) + $configCache = new Cache(); $configCache->load(['system' => ['value']]); self::assertEquals('value', $configCache->get('system', 0)); } @@ -207,13 +210,16 @@ class CacheTest extends MockedTest { $configCache = new Cache($data); + $assertion = []; + foreach ($data as $cat => $values) { + $assertion[$cat] = null; foreach ($values as $key => $value) { $configCache->delete($cat, $key); } } - self::assertEmpty($configCache->getAll()); + self::assertEquals($assertion, $configCache->getAll()); } /** @@ -385,32 +391,6 @@ class CacheTest extends MockedTest self::assertEquals('added category', $mergedCache->get('new_category', 'test_23')); } - /** - * @dataProvider dataTests - */ - public function testDiff($data) - { - $configCache = new Cache(); - $configCache->load($data, Cache::SOURCE_FILE); - - $configCache->set('system', 'test_2','with_data', Cache::SOURCE_DATA); - $configCache->set('config', 'test_override','with_another_data', Cache::SOURCE_DATA); - - $newCache = new Cache(); - $newCache->set('config', 'test_override','override it again', Cache::SOURCE_DATA); - $newCache->set('system', 'test_3','new value', Cache::SOURCE_DATA); - - $mergedCache = $configCache->diff($newCache); - - print_r($mergedCache); - - self::assertEquals('with_data', $mergedCache->get('system', 'test_2')); - // existing entry was dropped - self::assertNull($mergedCache->get('config', 'test_override')); - // the newCache entry wasn't set, because we Diff - self::assertNull($mergedCache->get('system', 'test_3')); - } - public function dataTestCat() { return [ diff --git a/tests/src/Core/Config/Cache/ConfigFileManagerTest.php b/tests/src/Core/Config/Cache/ConfigFileManagerTest.php index 89d757e32..2adfc27ef 100644 --- a/tests/src/Core/Config/Cache/ConfigFileManagerTest.php +++ b/tests/src/Core/Config/Cache/ConfigFileManagerTest.php @@ -441,4 +441,74 @@ class ConfigFileManagerTest extends MockedTest 'special' => $specialChars, ]], $configCache2->getDataBySource(Cache::SOURCE_DATA)); } + + /** + * If we delete something with the Cache::delete() functionality, be sure to override the underlying source as well + */ + public function testDeleteKeyOverwrite() + { + $this->delConfigFile('node.config.php'); + + $fileDir = dirname(__DIR__) . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR; + + vfsStream::newFile('B.config.php') + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($fileDir . 'B.config.php')); + + $configFileManager = (new Config())->createConfigFileManager($this->root->url()); + $configCache = new Cache(); + + $configFileManager->setupCache($configCache); + + $configCache->delete('system', 'default_timezone', Cache::SOURCE_DATA); + + $configFileManager->saveData($configCache); + + // assert that system.default_timezone is now null, even it's set with settings.conf.php + $configCache = new Cache(); + + $configFileManager->setupCache($configCache); + + self::assertNull($configCache->get('system', 'default_timezone')); + } + + /** + * If we delete something with the Cache::delete() functionality, be sure to override the underlying source as well + */ + public function testDeleteCategoryOverwrite() + { + $this->delConfigFile('node.config.php'); + + $fileDir = dirname(__DIR__) . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + '..' . DIRECTORY_SEPARATOR . + 'datasets' . DIRECTORY_SEPARATOR . + 'config' . DIRECTORY_SEPARATOR; + + vfsStream::newFile('B.config.php') + ->at($this->root->getChild('config')) + ->setContent(file_get_contents($fileDir . 'B.config.php')); + + $configFileManager = (new Config())->createConfigFileManager($this->root->url()); + $configCache = new Cache(); + + $configFileManager->setupCache($configCache); + + $configCache->delete('system'); + + $configFileManager->saveData($configCache); + + // assert that system.default_timezone is now null, even it's set with settings.conf.php + $configCache = new Cache(); + + $configFileManager->setupCache($configCache); + + self::assertNull($configCache->get('system', 'default_timezone')); + } } diff --git a/tests/src/Core/Config/ConfigTest.php b/tests/src/Core/Config/ConfigTest.php index d40af47d5..f7d7c070d 100644 --- a/tests/src/Core/Config/ConfigTest.php +++ b/tests/src/Core/Config/ConfigTest.php @@ -364,7 +364,7 @@ class ConfigTest extends MockedTest self::assertNull($this->testedConfig->get('test', 'it')); self::assertNull($this->testedConfig->getCache()->get('test', 'it')); - self::assertEmpty($this->testedConfig->getCache()->getAll()); + self::assertEquals(['test' => null], $this->testedConfig->getCache()->getAll()); } /** diff --git a/tests/src/Core/Config/ConfigTransactionTest.php b/tests/src/Core/Config/ConfigTransactionTest.php index 454c760d4..604ed7cd6 100644 --- a/tests/src/Core/Config/ConfigTransactionTest.php +++ b/tests/src/Core/Config/ConfigTransactionTest.php @@ -54,10 +54,6 @@ class ConfigTransactionTest extends MockedTest $config->set('delete', 'keyDel', 'catDel'); $configTransaction = new ConfigTransaction($config); - self::assertEquals('value1', $configTransaction->get('config', 'key1')); - self::assertEquals('value2', $configTransaction->get('system', 'key2')); - self::assertEquals('valueDel', $configTransaction->get('system', 'keyDel')); - self::assertEquals('catDel', $configTransaction->get('delete', 'keyDel')); // the config file knows it as well immediately $tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE; self::assertEquals('value1', $tempData['config']['key1'] ?? null); @@ -77,11 +73,6 @@ class ConfigTransactionTest extends MockedTest self::assertEquals('value1', $config->get('config', 'key1')); self::assertEquals('valueDel', $config->get('system', 'keyDel')); self::assertEquals('catDel', $config->get('delete', 'keyDel')); - // but the transaction config of course knows it - self::assertEquals('value3', $configTransaction->get('transaction', 'key3')); - self::assertEquals('changedValue1', $configTransaction->get('config', 'key1')); - self::assertNull($configTransaction->get('system', 'keyDel')); - self::assertNull($configTransaction->get('delete', 'keyDel')); // The config file still doesn't know it either $tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE; self::assertEquals('value1', $tempData['config']['key1'] ?? null); @@ -97,9 +88,6 @@ class ConfigTransactionTest extends MockedTest self::assertEquals('value3', $config->get('transaction', 'key3')); self::assertNull($config->get('system', 'keyDel')); self::assertNull($config->get('delete', 'keyDel')); - self::assertEquals('value3', $configTransaction->get('transaction', 'key3')); - self::assertEquals('changedValue1', $configTransaction->get('config', 'key1')); - self::assertNull($configTransaction->get('system', 'keyDel')); $tempData = include $this->root->url() . '/config/' . ConfigFileManager::CONFIG_DATA_FILE; self::assertEquals('changedValue1', $tempData['config']['key1'] ?? null); self::assertEquals('value2', $tempData['system']['key2'] ?? null); From e14050491ae8db5814ef9e7fceeabc5f75a8b8a6 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 01:10:57 +0100 Subject: [PATCH 04/19] Config fixing - unlock/close the `node.config.php` in every circumstances --- src/Core/Config/Util/ConfigFileManager.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index 569bcdb97..e0b82e656 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -182,16 +182,18 @@ class ConfigFileManager $content = '' . $content); if (is_array($dataArray)) { @@ -250,12 +252,12 @@ class ConfigFileManager clearstatcache(true, $filename); if (!ftruncate($configStream, 0) || !fwrite($configStream, $encodedData) || - !fflush($configStream) || - !flock($configStream, LOCK_UN)) { + !fflush($configStream)) { throw new ConfigFileException(sprintf('Cannot modify locked file %s', $filename)); } } } finally { + flock($configStream, LOCK_UN); fclose($configStream); } } From 18293280b70e9c63e76792aadac7c8db4197feaa Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 01:12:54 +0100 Subject: [PATCH 05/19] Add license --- .../src/Core/Config/ConfigTransactionTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/src/Core/Config/ConfigTransactionTest.php b/tests/src/Core/Config/ConfigTransactionTest.php index 604ed7cd6..c6eb7e268 100644 --- a/tests/src/Core/Config/ConfigTransactionTest.php +++ b/tests/src/Core/Config/ConfigTransactionTest.php @@ -1,4 +1,23 @@ . + * + */ namespace Friendica\Test\src\Core\Config; From d53cb318694414961acbf7274ffe07710f5e959e Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 02:16:35 +0100 Subject: [PATCH 06/19] Update src/Core/Config/Util/ConfigFileManager.php Co-authored-by: Hypolite Petovan --- src/Core/Config/Util/ConfigFileManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index e0b82e656..e4b84d158 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -203,7 +203,7 @@ class ConfigFileManager } /** - * Saves overridden config entries back into the data.config.phpR + * Saves overridden config entries back into the data.config.php * * @param Cache $configCache The config cache * From c8b9e40b85767ff84c6da33233aa9bbb0bb2e880 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 02:54:28 +0100 Subject: [PATCH 07/19] remove print_r --- tests/src/Core/Config/Cache/CacheTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/src/Core/Config/Cache/CacheTest.php b/tests/src/Core/Config/Cache/CacheTest.php index 6fe2fe15f..dc9b62dc0 100644 --- a/tests/src/Core/Config/Cache/CacheTest.php +++ b/tests/src/Core/Config/Cache/CacheTest.php @@ -67,8 +67,6 @@ class CacheTest extends MockedTest $configCache = new Cache(); $configCache->load($data); - print_r($configCache); - self::assertConfigValues($data, $configCache); } From ce8c8202217bc0d357678d1b4155d3ded6ddb5f2 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 02:53:23 +0100 Subject: [PATCH 08/19] add description --- src/Core/Config/Util/ConfigFileManager.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index e4b84d158..eb20317cb 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -179,8 +179,15 @@ class ConfigFileManager if (file_exists($filename)) { + // The fallback empty return content $content = '". Now we're in plain HTML again and can evaluate any PHP file :-) + */ $dataArray = eval('?>' . $content); if (is_array($dataArray)) { @@ -229,7 +244,7 @@ class ConfigFileManager $content = fread($configStream, filesize($filename)); rewind($configStream); if (!$content) { - throw new ConfigFileException(sprintf('Couldn\'t read file %s', $filename)); + throw new ConfigFileException(sprintf('Cannot read file %s', $filename)); } $dataArray = eval('?>' . $content); From b3772163d821401427f4cb3996b4bff8d89337cb Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 03:06:11 +0100 Subject: [PATCH 09/19] Add doc --- src/Core/Config/Util/ConfigFileManager.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index eb20317cb..cdef2dd69 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -234,14 +234,31 @@ class ConfigFileManager $fileExists = false; } + /** + * Creates a read-write stream + * + * @see https://www.php.net/manual/en/function.fopen.php + * @note Open the file for reading and writing. If the file does not exist, it is created. + * If it exists, it is neither truncated (as opposed to 'w'), nor the call to this function fails + * (as is the case with 'x'). The file pointer is positioned on the beginning of the file. + * + */ $configStream = fopen($filename, 'c+'); try { + // We do want an exclusive lock, so we wait until every LOCK_SH (config reading) is unlocked if (flock($configStream, LOCK_EX)) { + /** + * If the file exists, read the whole file again + * Since we're currently exclusive locked, no other process can now change the config again + */ if ($fileExists) { + // When reading the config file too fast, we get a wrong filesize, "clearstatcache" prevents that clearstatcache(true, $filename); $content = fread($configStream, filesize($filename)); + // Event truncating the whole content wouldn't automatically rewind the stream, + // so we need to do it manually rewind($configStream); if (!$content) { throw new ConfigFileException(sprintf('Cannot read file %s', $filename)); @@ -249,6 +266,8 @@ class ConfigFileManager $dataArray = eval('?>' . $content); + // Merge the new content into the existing file based config cache and use it + // as the new config cache if (is_array($dataArray)) { $fileConfigCache = new Cache(); $fileConfigCache->load($dataArray, Cache::SOURCE_DATA); @@ -256,6 +275,7 @@ class ConfigFileManager } } + // Only SOURCE_DATA is wanted, the rest isn't part of the node.config.php file $data = $configCache->getDataBySource(Cache::SOURCE_DATA); $encodedData = ConfigFileTransformer::encode($data); @@ -264,6 +284,7 @@ class ConfigFileManager throw new ConfigFileException('config source cannot get encoded'); } + // Once again to avoid wrong, implicit "filesize" calls during the fwrite() or ftruncate() call clearstatcache(true, $filename); if (!ftruncate($configStream, 0) || !fwrite($configStream, $encodedData) || @@ -272,6 +293,7 @@ class ConfigFileManager } } } finally { + // unlock and close the stream for every circumstances flock($configStream, LOCK_UN); fclose($configStream); } From 9462bfa763b67cb937e2d426095ff192654bc234 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 12:42:43 +0100 Subject: [PATCH 10/19] Update src/Core/Config/Util/ConfigFileManager.php Co-authored-by: Hypolite Petovan --- src/Core/Config/Util/ConfigFileManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index cdef2dd69..c7e63a63b 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -250,7 +250,7 @@ class ConfigFileManager if (flock($configStream, LOCK_EX)) { /** - * If the file exists, read the whole file again + * If the file exists, we read the whole file again to avoid a race condition with concurrent threads that could have modified the file between the first config read of this thread and now * Since we're currently exclusive locked, no other process can now change the config again */ if ($fileExists) { From 70704ccb1911ec9aabe7921376907c03b2bf37df Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 12:42:56 +0100 Subject: [PATCH 11/19] Update src/Core/Update.php Co-authored-by: Hypolite Petovan --- src/Core/Update.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index d440136b5..03badb1b5 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -162,14 +162,6 @@ class Update // Checks if the build changed during Lock acquiring (so no double update occurs) $retryBuild = DI::config()->get('system', 'build'); - // legacy option - check if there's something in the Config table - if (DBStructure::existsTable('config')) { - $dbConfig = DBA::selectFirst('config', ['v'], ['cat' => 'system', 'k' => 'build']); - if (!empty($dbConfig)) { - $retryBuild = $dbConfig['v']; - } - } - if ($retryBuild !== $build) { Logger::notice('Update already done.', ['from' => $stored, 'to' => $current]); DI::lock()->release('dbupdate'); From 05048d4abf93169c5e0c3bc464a9b8548563d4f7 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 12:43:04 +0100 Subject: [PATCH 12/19] Update src/Core/Config/ValueObject/Cache.php Co-authored-by: Hypolite Petovan --- src/Core/Config/ValueObject/Cache.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Config/ValueObject/Cache.php b/src/Core/Config/ValueObject/Cache.php index ca76bb4ed..208251164 100644 --- a/src/Core/Config/ValueObject/Cache.php +++ b/src/Core/Config/ValueObject/Cache.php @@ -182,7 +182,7 @@ class Cache if ($this->hidePasswordOutput && $key == 'password' && is_string($value)) { - $this->setCatKeyValueSource($cat, $key, new HiddenString((string)$value), $source); + $this->setCatKeyValueSource($cat, $key, new HiddenString($value), $source); } else if (is_string($value)) { $this->setCatKeyValueSource($cat, $key, self::toConfigValue($value), $source); } else { From beb3d376b23a02c0256b58d6cd6b3a99d8d85af4 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 12:46:06 +0100 Subject: [PATCH 13/19] Apply suggestions from code review Co-authored-by: Hypolite Petovan --- src/Core/Config/Util/ConfigFileManager.php | 8 ++++---- src/Core/Config/Util/ConfigFileTransformer.php | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index c7e63a63b..3a4ebb893 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -257,13 +257,14 @@ class ConfigFileManager // When reading the config file too fast, we get a wrong filesize, "clearstatcache" prevents that clearstatcache(true, $filename); $content = fread($configStream, filesize($filename)); - // Event truncating the whole content wouldn't automatically rewind the stream, - // so we need to do it manually - rewind($configStream); if (!$content) { throw new ConfigFileException(sprintf('Cannot read file %s', $filename)); } + // Event truncating the whole content wouldn't automatically rewind the stream, + // so we need to do it manually + rewind($configStream); + $dataArray = eval('?>' . $content); // Merge the new content into the existing file based config cache and use it @@ -279,7 +280,6 @@ class ConfigFileManager $data = $configCache->getDataBySource(Cache::SOURCE_DATA); $encodedData = ConfigFileTransformer::encode($data); - if (!$encodedData) { throw new ConfigFileException('config source cannot get encoded'); } diff --git a/src/Core/Config/Util/ConfigFileTransformer.php b/src/Core/Config/Util/ConfigFileTransformer.php index 4eaafe061..ac4990df1 100644 --- a/src/Core/Config/Util/ConfigFileTransformer.php +++ b/src/Core/Config/Util/ConfigFileTransformer.php @@ -34,7 +34,6 @@ class ConfigFileTransformer $categories = array_keys($data); foreach ($categories as $category) { - if (is_null($data[$category])) { $dataString .= "\t'$category' => null," . PHP_EOL; continue; From c35fd68ec24638eae0520ed020d0f89b3f628bc0 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 12:47:00 +0100 Subject: [PATCH 14/19] Adapt doc --- src/Core/Config/Util/ConfigFileManager.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Core/Config/Util/ConfigFileManager.php b/src/Core/Config/Util/ConfigFileManager.php index 3a4ebb893..f9d3b32b6 100644 --- a/src/Core/Config/Util/ConfigFileManager.php +++ b/src/Core/Config/Util/ConfigFileManager.php @@ -203,11 +203,12 @@ class ConfigFileManager } /** - * Evaluate the fetched content + * Evaluate the content string as PHP code + * + * @see https://www.php.net/manual/en/function.eval.php * * @note - * Because `eval()` directly evaluates PHP content, we need to "close" the expected PHP content again with - * the prefixed "?>". Now we're in plain HTML again and can evaluate any PHP file :-) + * To leave the PHP mode, we have to use the appropriate PHP tags '?>' as prefix. */ $dataArray = eval('?>' . $content); From baf3225916dc44cca95aaf5a6c2d73b1a037b90a Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 12:50:14 +0100 Subject: [PATCH 15/19] Apply Update::check() suggestions --- bin/auth_ejabberd.php | 2 +- bin/daemon.php | 4 ---- bin/worker.php | 2 +- index.php | 2 +- src/App.php | 2 +- src/Core/Update.php | 2 +- 6 files changed, 5 insertions(+), 9 deletions(-) diff --git a/bin/auth_ejabberd.php b/bin/auth_ejabberd.php index 9c5dc0528..40e7d3b97 100755 --- a/bin/auth_ejabberd.php +++ b/bin/auth_ejabberd.php @@ -84,7 +84,7 @@ $dice = $dice->addRule(LoggerInterface::class,['constructParams' => ['auth_ejabb \Friendica\Core\Logger\Handler\ErrorHandler::register($dice->create(\Psr\Log\LoggerInterface::class)); // Check the database structure and possibly fixes it -\Friendica\Core\Update::check(\Friendica\DI::basePath(), true, \Friendica\DI::mode()); +\Friendica\Core\Update::check(\Friendica\DI::basePath(), true); $appMode = $dice->create(Mode::class); diff --git a/bin/daemon.php b/bin/daemon.php index 6237df49d..b970f4a75 100755 --- a/bin/daemon.php +++ b/bin/daemon.php @@ -33,7 +33,6 @@ if (php_sapi_name() !== 'cli') { use Dice\Dice; use Friendica\App\Mode; use Friendica\Core\Logger; -use Friendica\Core\Update; use Friendica\Core\Worker; use Friendica\Database\DBA; use Friendica\DI; @@ -65,9 +64,6 @@ $dice = $dice->addRule(LoggerInterface::class,['constructParams' => ['daemon']]) DI::init($dice); \Friendica\Core\Logger\Handler\ErrorHandler::register($dice->create(\Psr\Log\LoggerInterface::class)); -// Check the database structure and possibly fixes it -Update::check(DI::basePath(), true, DI::mode()); - if (DI::mode()->isInstall()) { die("Friendica isn't properly installed yet.\n"); } diff --git a/bin/worker.php b/bin/worker.php index aceeff481..de207ae98 100755 --- a/bin/worker.php +++ b/bin/worker.php @@ -62,7 +62,7 @@ DI::init($dice); DI::mode()->setExecutor(Mode::WORKER); // Check the database structure and possibly fixes it -Update::check(DI::basePath(), true, DI::mode()); +Update::check(DI::basePath(), true); // Quit when in maintenance if (!DI::mode()->has(App\Mode::MAINTENANCEDISABLED)) { diff --git a/index.php b/index.php index 10f04996a..e0f33666b 100644 --- a/index.php +++ b/index.php @@ -37,7 +37,7 @@ $dice = $dice->addRule(Friendica\App\Mode::class, ['call' => [['determineRunMode \Friendica\Core\Logger\Handler\ErrorHandler::register($dice->create(\Psr\Log\LoggerInterface::class)); // Check the database structure and possibly fixes it -\Friendica\Core\Update::check(\Friendica\DI::basePath(), true, \Friendica\DI::mode()); +\Friendica\Core\Update::check(\Friendica\DI::basePath(), true); $a = \Friendica\DI::app(); diff --git a/src/App.php b/src/App.php index fc70f920d..224a895e2 100644 --- a/src/App.php +++ b/src/App.php @@ -659,7 +659,7 @@ class App $this->baseURL->redirect('install'); } else { $this->checkURL(); - Core\Update::check($this->getBasePath(), false, $this->mode); + Core\Update::check($this->getBasePath(), false); Core\Addon::loadAddons(); Core\Hook::loadHooks(); } diff --git a/src/Core/Update.php b/src/Core/Update.php index 03badb1b5..eb207b6c2 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -47,7 +47,7 @@ class Update * @return void * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function check(string $basePath, bool $via_worker, App\Mode $mode) + public static function check(string $basePath, bool $via_worker) { if (!DBA::connected()) { return; From 081dbae7c2a206dc69b1e8d4ceb9a69d5113e05d Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 12:50:32 +0100 Subject: [PATCH 16/19] Apply Update::check() suggestions --- index.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/index.php b/index.php index e0f33666b..fa6d8b087 100644 --- a/index.php +++ b/index.php @@ -36,9 +36,6 @@ $dice = $dice->addRule(Friendica\App\Mode::class, ['call' => [['determineRunMode \Friendica\Core\Logger\Handler\ErrorHandler::register($dice->create(\Psr\Log\LoggerInterface::class)); -// Check the database structure and possibly fixes it -\Friendica\Core\Update::check(\Friendica\DI::basePath(), true); - $a = \Friendica\DI::app(); \Friendica\DI::mode()->setExecutor(\Friendica\App\Mode::INDEX); From b2e14f209be1873bbe45cce2bcfb9db3eec04afb Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 12:50:14 +0100 Subject: [PATCH 17/19] Move Update::check() into daemon loop --- bin/daemon.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/daemon.php b/bin/daemon.php index b970f4a75..7182cb8ec 100755 --- a/bin/daemon.php +++ b/bin/daemon.php @@ -33,6 +33,7 @@ if (php_sapi_name() !== 'cli') { use Dice\Dice; use Friendica\App\Mode; use Friendica\Core\Logger; +use Friendica\Core\Update; use Friendica\Core\Worker; use Friendica\Database\DBA; use Friendica\DI; @@ -192,6 +193,9 @@ $last_cron = 0; // Now running as a daemon. while (true) { + // Check the database structure and possibly fixes it + Update::check(DI::basePath(), true); + if (!$do_cron && ($last_cron + $wait_interval) < time()) { Logger::info('Forcing cron worker call.', ['pid' => $pid]); $do_cron = true; From 5b2e02889ea656707e96a04b09cd6321f5a223dc Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 6 Jan 2023 17:50:56 +0100 Subject: [PATCH 18/19] Fix Update::run() --- src/Core/Update.php | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Core/Update.php b/src/Core/Update.php index eb207b6c2..cef72f34b 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -162,10 +162,20 @@ class Update // Checks if the build changed during Lock acquiring (so no double update occurs) $retryBuild = DI::config()->get('system', 'build'); - if ($retryBuild !== $build) { - Logger::notice('Update already done.', ['from' => $stored, 'to' => $current]); - DI::lock()->release('dbupdate'); - return ''; + if ($retryBuild != $build) { + // legacy option - check if there's something in the Config table + if (DBStructure::existsTable('config')) { + $dbConfig = DBA::selectFirst('config', ['v'], ['cat' => 'system', 'k' => 'build']); + if (!empty($dbConfig)) { + $retryBuild = intval($dbConfig['v']); + } + } + + if ($retryBuild != $build) { + Logger::notice('Update already done.', ['from' => $build, 'retry' => $retryBuild, 'to' => $current]); + DI::lock()->release('dbupdate'); + return ''; + } } DI::config()->set('system', 'maintenance', 1); From 80e8f4aa34c4ccc5297e1b4c06735f0badc34a86 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 7 Jan 2023 13:43:16 +0100 Subject: [PATCH 19/19] Execute critical worker tasks, even if we're in daemon mode --- src/Core/Worker.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Worker.php b/src/Core/Worker.php index 2db9256d1..90bce0a88 100644 --- a/src/Core/Worker.php +++ b/src/Core/Worker.php @@ -1315,8 +1315,8 @@ class Worker return $added; } - // Quit on daemon mode - if (Worker\Daemon::isMode()) { + // Quit on daemon mode, except the priority is critical (like for db updates) + if (Worker\Daemon::isMode() && $priority !== self::PRIORITY_CRITICAL) { return $added; }