From 0f3e4255ca140c1cf8ff717b6804fe00a8d429e3 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 4 Oct 2020 20:37:35 +0200 Subject: [PATCH 1/5] Introduce Config\Cache Source indicators (File, DB, Server Env) --- src/Core/Config/Cache.php | 70 ++++++++++++++++------------ src/Core/Config/JitConfig.php | 2 +- src/Core/Config/PreloadConfig.php | 2 +- src/Util/ConfigFileLoader.php | 12 ++--- tests/src/Core/Config/CacheTest.php | 9 ++-- tests/src/Core/Config/ConfigTest.php | 6 +-- 6 files changed, 56 insertions(+), 45 deletions(-) diff --git a/src/Core/Config/Cache.php b/src/Core/Config/Cache.php index b3fe9d4e0..b78b57f57 100644 --- a/src/Core/Config/Cache.php +++ b/src/Core/Config/Cache.php @@ -30,11 +30,26 @@ use ParagonIE\HiddenString\HiddenString; */ class Cache { + /** @var int Indicates that the cache entry is set by file - Low Priority */ + const SOURCE_FILE = 0; + /** @var int Indicates that the cache entry is set by the DB config table - Middle Priority */ + const SOURCE_DB = 1; + /** @var int Indicates that the cache entry is set by a server environment variable - High Priority */ + const SOURCE_ENV = 3; + + /** @var int Default value for a config source */ + const SOURCE_DEFAULT = self::SOURCE_FILE; + /** * @var array */ private $config; + /** + * @var int[][] + */ + private $source = []; + /** * @var bool */ @@ -43,11 +58,12 @@ class Cache /** * @param array $config A initial config array * @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) + public function __construct(array $config = [], bool $hidePasswordOutput = true, $source = self::SOURCE_DEFAULT) { $this->hidePasswordOutput = $hidePasswordOutput; - $this->load($config); + $this->load($config, $source); } /** @@ -55,9 +71,9 @@ class Cache * Doesn't overwrite previously set values by default to prevent default config files to supersede DB Config. * * @param array $config - * @param bool $overwrite Force value overwrite if the config key already exists + * @param int $source Indicates the source of the config entry */ - public function load(array $config, bool $overwrite = false) + public function load(array $config, int $source = self::SOURCE_DEFAULT) { $categories = array_keys($config); @@ -68,11 +84,7 @@ class Cache foreach ($keys as $key) { $value = $config[$category][$key]; if (isset($value)) { - if ($overwrite) { - $this->set($category, $key, $value); - } else { - $this->setDefault($category, $key, $value); - } + $this->set($category, $key, $value, $source); } } } @@ -91,49 +103,45 @@ class Cache { if (isset($this->config[$cat][$key])) { return $this->config[$cat][$key]; - } elseif (!isset($key) && isset($this->config[$cat])) { + } else if (!isset($key) && isset($this->config[$cat])) { return $this->config[$cat]; } else { return null; } } - /** - * Sets a default value in the config cache. Ignores already existing keys. - * - * @param string $cat Config category - * @param string $key Config key - * @param mixed $value Default value to set - */ - private function setDefault(string $cat, string $key, $value) - { - if (!isset($this->config[$cat][$key])) { - $this->set($cat, $key, $value); - } - } - /** * 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 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) + public function set(string $cat, string $key, $value, $source = self::SOURCE_DEFAULT) { if (!isset($this->config[$cat])) { $this->config[$cat] = []; + $this->source[$cat] = []; + } + + if (isset($this->source[$cat][$key]) && + $source < $this->source[$cat][$key]) { + return false; } if ($this->hidePasswordOutput && - $key == 'password' && - is_string($value)) { + $key == 'password' && + is_string($value)) { $this->config[$cat][$key] = new HiddenString((string)$value); } else { $this->config[$cat][$key] = $value; } + + $this->source[$cat][$key] = $source; + return true; } @@ -149,8 +157,10 @@ class Cache { 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]); } return true; } else { diff --git a/src/Core/Config/JitConfig.php b/src/Core/Config/JitConfig.php index 4cf0d06f3..dbf1ea3ea 100644 --- a/src/Core/Config/JitConfig.php +++ b/src/Core/Config/JitConfig.php @@ -70,7 +70,7 @@ class JitConfig extends BaseConfig } // load the whole category out of the DB into the cache - $this->configCache->load($config, true); + $this->configCache->load($config, Cache::SOURCE_DB); } /** diff --git a/src/Core/Config/PreloadConfig.php b/src/Core/Config/PreloadConfig.php index c1181414b..168823f4d 100644 --- a/src/Core/Config/PreloadConfig.php +++ b/src/Core/Config/PreloadConfig.php @@ -69,7 +69,7 @@ class PreloadConfig extends BaseConfig $this->config_loaded = true; // load the whole category out of the DB into the cache - $this->configCache->load($config, true); + $this->configCache->load($config, Cache::SOURCE_DB); } /** diff --git a/src/Util/ConfigFileLoader.php b/src/Util/ConfigFileLoader.php index fc6685946..54c2ebe0c 100644 --- a/src/Util/ConfigFileLoader.php +++ b/src/Util/ConfigFileLoader.php @@ -104,12 +104,12 @@ class ConfigFileLoader public function setupCache(Cache $config, $raw = false) { // Load static config files first, the order is important - $config->load($this->loadStaticConfig('defaults')); - $config->load($this->loadStaticConfig('settings')); + $config->load($this->loadStaticConfig('defaults'), Cache::SOURCE_FILE); + $config->load($this->loadStaticConfig('settings'), Cache::SOURCE_FILE); // try to load the legacy config first - $config->load($this->loadLegacyConfig('htpreconfig'), true); - $config->load($this->loadLegacyConfig('htconfig'), true); + $config->load($this->loadLegacyConfig('htpreconfig'), Cache::SOURCE_FILE); + $config->load($this->loadLegacyConfig('htconfig'), Cache::SOURCE_FILE); // Now load every other config you find inside the 'config/' directory $this->loadCoreConfig($config); @@ -157,12 +157,12 @@ class ConfigFileLoader { // try to load legacy ini-files first foreach ($this->getConfigFiles(true) as $configFile) { - $config->load($this->loadINIConfigFile($configFile), true); + $config->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), true); + $config->load($this->loadConfigFile($configFile), Cache::SOURCE_FILE); } return []; diff --git a/tests/src/Core/Config/CacheTest.php b/tests/src/Core/Config/CacheTest.php index c4e59e691..02b98c7ec 100644 --- a/tests/src/Core/Config/CacheTest.php +++ b/tests/src/Core/Config/CacheTest.php @@ -83,13 +83,14 @@ class CacheTest extends MockedTest ]; $configCache = new Cache(); - $configCache->load($data); - $configCache->load($override); + $configCache->load($data, Cache::SOURCE_DB); + // doesn't override - Low Priority due Config file + $configCache->load($override, Cache::SOURCE_FILE); $this->assertConfigValues($data, $configCache); - // override the value - $configCache->load($override, true); + // override the value - High Prio due Server Env + $configCache->load($override, Cache::SOURCE_ENV); $this->assertEquals($override['system']['test'], $configCache->get('system', 'test')); $this->assertEquals($override['system']['boolTrue'], $configCache->get('system', 'boolTrue')); diff --git a/tests/src/Core/Config/ConfigTest.php b/tests/src/Core/Config/ConfigTest.php index 7dd61d451..48df2b2a4 100644 --- a/tests/src/Core/Config/ConfigTest.php +++ b/tests/src/Core/Config/ConfigTest.php @@ -350,7 +350,7 @@ abstract class ConfigTest extends MockedTest */ public function testGetWithRefresh($data) { - $this->configCache->load(['test' => ['it' => 'now']]); + $this->configCache->load(['test' => ['it' => 'now']], Cache::SOURCE_FILE); $this->testedConfig = $this->getInstance(); $this->assertInstanceOf(Cache::class, $this->testedConfig->getCache()); @@ -375,7 +375,7 @@ abstract class ConfigTest extends MockedTest */ public function testDeleteWithoutDB($data) { - $this->configCache->load(['test' => ['it' => $data]]); + $this->configCache->load(['test' => ['it' => $data]], Cache::SOURCE_FILE); $this->testedConfig = $this->getInstance(); $this->assertInstanceOf(Cache::class, $this->testedConfig->getCache()); @@ -395,7 +395,7 @@ abstract class ConfigTest extends MockedTest */ public function testDeleteWithDB() { - $this->configCache->load(['test' => ['it' => 'now', 'quarter' => 'true']]); + $this->configCache->load(['test' => ['it' => 'now', 'quarter' => 'true']], Cache::SOURCE_FILE); $this->configModel->shouldReceive('delete') ->with('test', 'it') From 3587e89482284d81213eda21dd62415698185ab7 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 6 Oct 2020 20:03:38 +0200 Subject: [PATCH 2/5] Introduce a "DatabaseException" class for fatal exceptions (used in testmode to throw an exception in case of DB errors) --- src/Database/Database.php | 36 ++++++--------------------- src/Database/DatabaseException.php | 39 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 29 deletions(-) create mode 100644 src/Database/DatabaseException.php diff --git a/src/Database/Database.php b/src/Database/Database.php index 80fd02dc0..e25323b38 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -21,10 +21,8 @@ namespace Friendica\Database; -use Exception; use Friendica\Core\Config\Cache; use Friendica\Core\System; -use Friendica\DI; use Friendica\Network\HTTPException\InternalServerErrorException; use Friendica\Util\DateTimeFormat; use Friendica\Util\Profiler; @@ -75,7 +73,6 @@ class Database $this->profiler = $profiler; $this->logger = $logger; - $this->readServerVariables($server); $this->connect(); if ($this->isConnected()) { @@ -84,30 +81,6 @@ class Database } } - private function readServerVariables(array $server) - { - // Use environment variables for mysql if they are set beforehand - if (!empty($server['MYSQL_HOST']) - && (!empty($server['MYSQL_USERNAME']) || !empty($server['MYSQL_USER'])) - && $server['MYSQL_PASSWORD'] !== false - && !empty($server['MYSQL_DATABASE'])) - { - $db_host = $server['MYSQL_HOST']; - if (!empty($server['MYSQL_PORT'])) { - $db_host .= ':' . $server['MYSQL_PORT']; - } - $this->configCache->set('database', 'hostname', $db_host); - unset($db_host); - if (!empty($server['MYSQL_USERNAME'])) { - $this->configCache->set('database', 'username', $server['MYSQL_USERNAME']); - } else { - $this->configCache->set('database', 'username', $server['MYSQL_USER']); - } - $this->configCache->set('database', 'password', (string) $server['MYSQL_PASSWORD']); - $this->configCache->set('database', 'database', $server['MYSQL_DATABASE']); - } - } - public function connect() { if (!is_null($this->connection) && $this->connected()) { @@ -124,6 +97,11 @@ class Database if (count($serverdata) > 1) { $port = trim($serverdata[1]); } + + if (!empty(trim($this->configCache->get('database', 'port')))) { + $port = trim(trim($this->configCache->get('database', 'port'))); + } + $server = trim($server); $user = trim($this->configCache->get('database', 'username')); $pass = trim($this->configCache->get('database', 'password')); @@ -658,7 +636,7 @@ class Database $errorno = $this->errorno; if ($this->testmode) { - throw new Exception(DI::l10n()->t('Database error %d "%s" at "%s"', $errorno, $error, $this->replaceParameters($sql, $args))); + throw new DatabaseException($error, $errorno, $this->replaceParameters($sql, $args)); } $this->logger->error('DB Error', [ @@ -761,7 +739,7 @@ class Database $errorno = $this->errorno; if ($this->testmode) { - throw new Exception(DI::l10n()->t('Database error %d "%s" at "%s"', $errorno, $error, $this->replaceParameters($sql, $params))); + throw new DatabaseException($error, $errorno, $this->replaceParameters($sql, $params)); } $this->logger->error('DB Error', [ diff --git a/src/Database/DatabaseException.php b/src/Database/DatabaseException.php new file mode 100644 index 000000000..8bf5d8a6c --- /dev/null +++ b/src/Database/DatabaseException.php @@ -0,0 +1,39 @@ +query = $query; + } + + /** + * {@inheritDoc} + */ + public function __toString() + { + return sprintf('Database error %d "%s" at "%s"', $this->message, $this->code, $this->query); + } +} From d39ee428f0424ee662263b23fc083008e68708da Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 6 Oct 2020 20:06:52 +0200 Subject: [PATCH 3/5] Introduce "static/env.config.php" for environment variable mapping to config cache entries - Added new database.port config value (used for MYSQL_PORT) - Removed now obsolete db environment variable functionality - Added functionality to load env variables (overwrites DB based cached) --- src/Core/Config/Cache.php | 2 ++ src/Database/Database.php | 2 +- src/Factory/ConfigFactory.php | 4 +-- src/Util/ConfigFileLoader.php | 39 +++++++++++++++++++++++++++-- static/defaults.config.php | 5 ++++ static/dependencies.config.php | 5 ++-- static/env.config.php | 31 +++++++++++++++++++++++ tests/src/Core/Config/CacheTest.php | 13 ++++++++++ 8 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 static/env.config.php diff --git a/src/Core/Config/Cache.php b/src/Core/Config/Cache.php index b78b57f57..25b25550e 100644 --- a/src/Core/Config/Cache.php +++ b/src/Core/Config/Cache.php @@ -36,6 +36,8 @@ class Cache const SOURCE_DB = 1; /** @var int Indicates that the cache entry is set by a server environment variable - High Priority */ const SOURCE_ENV = 3; + /** @var int Indicates that the cache entry is fixed and must not be changed */ + const SOURCE_FIX = 4; /** @var int Default value for a config source */ const SOURCE_DEFAULT = self::SOURCE_FILE; diff --git a/src/Database/Database.php b/src/Database/Database.php index e25323b38..727c2df93 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -66,7 +66,7 @@ class Database protected $testmode = false; private $relation = []; - public function __construct(Cache $configCache, Profiler $profiler, LoggerInterface $logger, array $server = []) + public function __construct(Cache $configCache, Profiler $profiler, LoggerInterface $logger) { // We are storing these values for being able to perform a reconnect $this->configCache = $configCache; diff --git a/src/Factory/ConfigFactory.php b/src/Factory/ConfigFactory.php index 954f89395..3110490fd 100644 --- a/src/Factory/ConfigFactory.php +++ b/src/Factory/ConfigFactory.php @@ -37,10 +37,10 @@ class ConfigFactory * * @throws Exception */ - public function createCache(ConfigFileLoader $loader) + public function createCache(ConfigFileLoader $loader, array $server = []) { $configCache = new Cache(); - $loader->setupCache($configCache); + $loader->setupCache($configCache, $server); return $configCache; } diff --git a/src/Util/ConfigFileLoader.php b/src/Util/ConfigFileLoader.php index 54c2ebe0c..60e82c04f 100644 --- a/src/Util/ConfigFileLoader.php +++ b/src/Util/ConfigFileLoader.php @@ -97,11 +97,12 @@ class ConfigFileLoader * expected local.config.php * * @param Cache $config The config cache to load to + * @param array $server The $_SERVER array * @param bool $raw Setup the raw config format * * @throws Exception */ - public function setupCache(Cache $config, $raw = false) + public function setupCache(Cache $config, array $server = [], $raw = false) { // Load static config files first, the order is important $config->load($this->loadStaticConfig('defaults'), Cache::SOURCE_FILE); @@ -114,10 +115,12 @@ class ConfigFileLoader // Now load every other config you find inside the 'config/' directory $this->loadCoreConfig($config); + $config->load($this->loadEnvConfig($server), 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'))) { // Setting at least the basepath we know - $config->set('system', 'basepath', $this->baseDir); + $config->set('system', 'basepath', $this->baseDir, Cache::SOURCE_FILE); } } @@ -192,6 +195,38 @@ class ConfigFileLoader } } + /** + * 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 Exception if the configuration file isn't readable + */ + public function loadEnvConfig(array $server) + { + $filepath = $this->baseDir . DIRECTORY_SEPARATOR . // /var/www/html/ + self::STATIC_DIR . DIRECTORY_SEPARATOR . // static/ + "env.config.php"; // env.config.php + + if (!file_exists($filepath)) { + return []; + } + + $envConfig = $this->loadConfigFile($filepath); + + $return = []; + + foreach ($envConfig as $envKey => $configStructure) { + if (isset($server[$envKey])) { + $return[$configStructure[0]][$configStructure[1]] = $server[$envKey]; + } + } + + return $return; + } + /** * Get the config files of the config-directory * diff --git a/static/defaults.config.php b/static/defaults.config.php index 11731f214..18b70d3c1 100644 --- a/static/defaults.config.php +++ b/static/defaults.config.php @@ -32,6 +32,11 @@ return [ // Can contain the port number with the syntax "hostname:port". 'hostname' => '', + // port (Integer) + // Port of the database server. + // Can be used instead of adding a port number to the hostname + 'port' => null, + // user (String) // Database user name. Please don't use "root". 'username' => '', diff --git a/static/dependencies.config.php b/static/dependencies.config.php index b1a54786a..ca9b78853 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -75,13 +75,13 @@ return [ Util\ConfigFileLoader::class => [ 'shared' => true, 'constructParams' => [ - [Dice::INSTANCE => '$basepath'], + [Dice::INSTANCE => '$basepath'] ], ], Config\Cache::class => [ 'instanceOf' => Factory\ConfigFactory::class, 'call' => [ - ['createCache', [], Dice::CHAIN_CALL], + ['createCache', [$_SERVER], Dice::CHAIN_CALL], ], ], App\Mode::class => [ @@ -105,7 +105,6 @@ return [ Database::class => [ 'constructParams' => [ [Dice::INSTANCE => \Psr\Log\NullLogger::class], - $_SERVER, ], ], /** diff --git a/static/env.config.php b/static/env.config.php new file mode 100644 index 000000000..e2b9d2b5b --- /dev/null +++ b/static/env.config.php @@ -0,0 +1,31 @@ +. + * + * Main mapping table of environment variables to correct config values + * + */ + +return [ + 'MYSQL_HOST' => ['database', 'hostname'], + 'MYSQL_USERNAME' => ['database', 'username'], + 'MYSQL_USER' => ['database', 'username'], + 'MYSQL_PORT' => ['database', 'port'], + 'MYSQL_PASSWORD' => ['database', 'password'], + 'MYSQL_DATABASE' => ['database', 'database'], +]; diff --git a/tests/src/Core/Config/CacheTest.php b/tests/src/Core/Config/CacheTest.php index 02b98c7ec..1acdd1f4f 100644 --- a/tests/src/Core/Config/CacheTest.php +++ b/tests/src/Core/Config/CacheTest.php @@ -94,6 +94,19 @@ class CacheTest extends MockedTest $this->assertEquals($override['system']['test'], $configCache->get('system', 'test')); $this->assertEquals($override['system']['boolTrue'], $configCache->get('system', 'boolTrue')); + + // Don't overwrite server ENV variables - even in load mode + $configCache->load($data, Cache::SOURCE_DB); + + $this->assertEquals($override['system']['test'], $configCache->get('system', 'test')); + $this->assertEquals($override['system']['boolTrue'], $configCache->get('system', 'boolTrue')); + + // Overwrite ENV variables with ENV variables + $configCache->load($data, Cache::SOURCE_ENV); + + $this->assertConfigValues($data, $configCache); + $this->assertNotEquals($override['system']['test'], $configCache->get('system', 'test')); + $this->assertNotEquals($override['system']['boolTrue'], $configCache->get('system', 'boolTrue')); } /** From 2a464a156f066d604aeccb4abd0ca7ce66b02ac8 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 6 Oct 2020 20:55:36 +0200 Subject: [PATCH 4/5] Update src/Database/Database.php Co-authored-by: Hypolite Petovan --- src/Database/Database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 727c2df93..c2dd3c84c 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -99,7 +99,7 @@ class Database } if (!empty(trim($this->configCache->get('database', 'port')))) { - $port = trim(trim($this->configCache->get('database', 'port'))); + $port = trim($this->configCache->get('database', 'port')); } $server = trim($server); From ccfb17151b0c251f693c6d324424a69e881bdb3d Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 6 Oct 2020 20:56:20 +0200 Subject: [PATCH 5/5] small improvements --- static/dependencies.config.php | 2 +- static/env.config.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/static/dependencies.config.php b/static/dependencies.config.php index ca9b78853..28cc58a61 100644 --- a/static/dependencies.config.php +++ b/static/dependencies.config.php @@ -75,7 +75,7 @@ return [ Util\ConfigFileLoader::class => [ 'shared' => true, 'constructParams' => [ - [Dice::INSTANCE => '$basepath'] + [Dice::INSTANCE => '$basepath'], ], ], Config\Cache::class => [ diff --git a/static/env.config.php b/static/env.config.php index e2b9d2b5b..1473445dd 100644 --- a/static/env.config.php +++ b/static/env.config.php @@ -17,7 +17,7 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . * - * Main mapping table of environment variables to correct config values + * Main mapping table of environment variables to namespaced config values * */