From 8c3aebc376a496e5f6ac93cda8ba88b1bd1261fa Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Fri, 22 Feb 2019 23:51:13 +0100 Subject: [PATCH 1/7] Bugfixings in Config - replaced usage of "!!" with null-returns - fixed bool settings (0/1) - fixed overriding config-values - fixed basepath problems --- bin/daemon.php | 4 +- src/App.php | 6 +- src/Core/Config.php | 2 - .../Adapter/AbstractDbaConfigAdapter.php | 64 ++++++++++++++++++- src/Core/Config/Adapter/IConfigAdapter.php | 8 ++- src/Core/Config/Adapter/IPConfigAdapter.php | 6 +- src/Core/Config/Adapter/JITConfigAdapter.php | 38 ++++++----- src/Core/Config/Adapter/JITPConfigAdapter.php | 29 +++++---- .../Config/Adapter/PreloadConfigAdapter.php | 14 ++-- .../Config/Adapter/PreloadPConfigAdapter.php | 14 ++-- src/Core/Config/Cache/ConfigCache.php | 38 ++--------- src/Core/Config/Cache/ConfigCacheLoader.php | 3 - src/Core/Config/Cache/IConfigCache.php | 13 +--- src/Core/Config/Cache/IPConfigCache.php | 13 +--- src/Core/Config/Configuration.php | 11 ++-- src/Core/Config/PConfiguration.php | 9 +-- src/Core/PConfig.php | 2 - src/Database/DBA.php | 9 ++- src/Factory/DBFactory.php | 13 ++-- src/Factory/DependencyFactory.php | 8 +-- src/Model/Photo.php | 4 +- tests/include/ApiTest.php | 8 +-- tests/src/Database/DBATest.php | 8 +-- tests/src/Database/DBStructureTest.php | 8 +-- 24 files changed, 175 insertions(+), 157 deletions(-) diff --git a/bin/daemon.php b/bin/daemon.php index 047bf71be..298cfa253 100755 --- a/bin/daemon.php +++ b/bin/daemon.php @@ -144,7 +144,9 @@ if (!$foreground) { file_put_contents($pidfile, $pid); // We lose the database connection upon forking - Factory\DBFactory::init($a->getConfigCache(), $a->getProfiler(), $_SERVER); + /// @todo refactoring during https://github.com/friendica/friendica/issues/6720 + $basePath = \Friendica\Util\BasePath::create(dirname(__DIR__), $_SERVER); + Factory\DBFactory::init($basePath, $a->getConfigCache(), $a->getProfiler(), $_SERVER); } Config::set('system', 'worker_daemon_mode', true); diff --git a/src/App.php b/src/App.php index 3df285cb3..e5ac5dda3 100644 --- a/src/App.php +++ b/src/App.php @@ -205,6 +205,7 @@ class App /** * @brief App constructor. * + * @param string $basePath The basedir of the app * @param Configuration $config The Configuration * @param LoggerInterface $logger The current app logger * @param Profiler $profiler The profiler of this application @@ -212,14 +213,15 @@ class App * * @throws Exception if the Basepath is not usable */ - public function __construct(Configuration $config, LoggerInterface $logger, Profiler $profiler, $isBackend = true) + public function __construct($basePath, Configuration $config, LoggerInterface $logger, Profiler $profiler, $isBackend = true) { BaseObject::setApp($this); $this->logger = $logger; $this->config = $config; $this->profiler = $profiler; - $this->basePath = $this->config->get('system', 'basepath'); + $cfgBasePath = $this->config->get('system', 'basepath'); + $this->basePath = (isset($cfgBasePath) && $cfgBasePath !== '') ? $cfgBasePath : $basePath; if (!Core\System::isDirectoryUsable($this->basePath, false)) { throw new Exception('Basepath \'' . $this->basePath . '\' isn\'t usable.'); diff --git a/src/Core/Config.php b/src/Core/Config.php index 4bf9c5b11..f974e2fff 100644 --- a/src/Core/Config.php +++ b/src/Core/Config.php @@ -65,8 +65,6 @@ class Config * * Stores a config value ($value) in the category ($cat) under the key ($key) * - * Note: Please do not store booleans - convert to 0/1 integer values! - * * @param string $cat The category of the configuration value * @param string $key The configuration key to set * @param mixed $value The value to store diff --git a/src/Core/Config/Adapter/AbstractDbaConfigAdapter.php b/src/Core/Config/Adapter/AbstractDbaConfigAdapter.php index 770dfd2c9..38caf35ca 100644 --- a/src/Core/Config/Adapter/AbstractDbaConfigAdapter.php +++ b/src/Core/Config/Adapter/AbstractDbaConfigAdapter.php @@ -6,7 +6,11 @@ use Friendica\Database\DBA; abstract class AbstractDbaConfigAdapter { - /** @var bool */ + /** + * The connection state of the adapter + * + * @var bool + */ protected $connected = true; public function __construct() @@ -14,8 +18,66 @@ abstract class AbstractDbaConfigAdapter $this->connected = DBA::connected(); } + /** + * Checks if the adapter is currently connected + * + * @return bool + */ public function isConnected() { return $this->connected; } + + /** + * Formats a DB value to a config value + * - null = The db-value isn't set + * - bool = The db-value is either '0' or '1' + * - array = The db-value is a serialized array + * - string = The db-value is a string + * + * Keep in mind that there aren't any numeric/integer config values in the database + * + * @param null|string $value + * + * @return null|array|string + */ + protected function toConfigValue($value) + { + if (!isset($value)) { + return null; + } + + switch (true) { + // manage array value + case preg_match("|^a:[0-9]+:{.*}$|s", $value): + return unserialize($value); + + default: + return $value; + } + } + + /** + * Formats a config value to a DB value (string) + * + * @param mixed $value + * + * @return string + */ + protected function toDbValue($value) + { + // if not set, save an empty string + if (!isset($value)) { + return ''; + } + + switch (true) { + // manage arrays + case is_array($value): + return serialize($value); + + default: + return (string)$value; + } + } } diff --git a/src/Core/Config/Adapter/IConfigAdapter.php b/src/Core/Config/Adapter/IConfigAdapter.php index 21cd9a4b2..892c476e7 100644 --- a/src/Core/Config/Adapter/IConfigAdapter.php +++ b/src/Core/Config/Adapter/IConfigAdapter.php @@ -21,10 +21,12 @@ interface IConfigAdapter * Get a particular system-wide config variable given the category name * ($family) and a key. * + * Note: Boolean variables are defined as 0/1 in the database + * * @param string $cat The category of the configuration value * @param string $key The configuration key to query * - * @return mixed Stored value or "!!" if it does not exist + * @return null|mixed Stored value or null if it does not exist */ public function get($cat, $key); @@ -46,9 +48,9 @@ interface IConfigAdapter * and removes it from the database. * * @param string $cat The category of the configuration value - * @param string $key The configuration key to delete + * @param string $key The configuration key to delete * - * @return mixed + * @return bool Operation success */ public function delete($cat, $key); diff --git a/src/Core/Config/Adapter/IPConfigAdapter.php b/src/Core/Config/Adapter/IPConfigAdapter.php index 8e6c050b2..c505532c5 100644 --- a/src/Core/Config/Adapter/IPConfigAdapter.php +++ b/src/Core/Config/Adapter/IPConfigAdapter.php @@ -28,11 +28,13 @@ interface IPConfigAdapter * Get a particular user's config variable given the category name * ($family) and a key. * + * Note: Boolean variables are defined as 0/1 in the database + * * @param string $uid The user_id * @param string $cat The category of the configuration value * @param string $key The configuration key to query * - * @return mixed Stored value or "!!" if it does not exist + * @return null|mixed Stored value or null if it does not exist */ public function get($uid, $cat, $key); @@ -59,7 +61,7 @@ interface IPConfigAdapter * @param string $cat The category of the configuration value * @param string $key The configuration key to delete * - * @return bool + * @return bool Operation success */ public function delete($uid, $cat, $key); diff --git a/src/Core/Config/Adapter/JITConfigAdapter.php b/src/Core/Config/Adapter/JITConfigAdapter.php index c0d680d2c..5a4146303 100644 --- a/src/Core/Config/Adapter/JITConfigAdapter.php +++ b/src/Core/Config/Adapter/JITConfigAdapter.php @@ -34,40 +34,49 @@ class JITConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAdapte $configs = DBA::select('config', ['v', 'k'], ['cat' => $cat]); while ($config = DBA::fetch($configs)) { $key = $config['k']; - $value = $config['v']; + $value = $this->toConfigValue($config['v']); - if (isset($value) && $value !== '') { + // The value was in the db, so don't check it again (unless you have to) + $this->in_db[$cat][$key] = true; + + // just save it in case it is set + if (isset($value)) { $return[$key] = $value; - $this->in_db[$cat][$key] = true; } } DBA::close($configs); - return [$cat => $config]; + return [$cat => $return]; } /** * {@inheritdoc} + * + * @param bool $mark if true, mark the selection of the current cat/key pair */ - public function get($cat, $key) + public function get($cat, $key, $mark = true) { if (!$this->isConnected()) { - return '!!'; + return null; + } + + // The value got checked, so mark it to avoid checking it over and over again + if ($mark) { + $this->in_db[$cat][$key] = true; } $config = DBA::selectFirst('config', ['v'], ['cat' => $cat, 'k' => $key]); if (DBA::isResult($config)) { // manage array value - $value = (preg_match("|^a:[0-9]+:{.*}$|s", $config['v']) ? unserialize($config['v']) : $config['v']); + $value = $this->toConfigValue($config['v']); - if (isset($value) && $value !== '') { - $this->in_db[$cat][$key] = true; + // just return it in case it is set + if (isset($value)) { return $value; } } - $this->in_db[$cat][$key] = false; - return '!!'; + return null; } /** @@ -84,7 +93,7 @@ class JITConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAdapte // The exception are array values. $dbvalue = (!is_array($value) ? (string)$value : $value); - $stored = $this->get($cat, $key); + $stored = $this->get($cat, $key, false); if (!isset($this->in_db[$cat])) { $this->in_db[$cat] = []; @@ -93,12 +102,11 @@ class JITConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAdapte $this->in_db[$cat][$key] = false; } - if (($stored === $dbvalue) && $this->in_db[$cat][$key]) { + if (isset($stored) && ($stored === $dbvalue) && $this->in_db[$cat][$key]) { return true; } - // manage array value - $dbvalue = (is_array($value) ? serialize($value) : $dbvalue); + $dbvalue = $this->toDbValue($dbvalue); $result = DBA::update('config', ['v' => $dbvalue], ['cat' => $cat, 'k' => $key], true); diff --git a/src/Core/Config/Adapter/JITPConfigAdapter.php b/src/Core/Config/Adapter/JITPConfigAdapter.php index 4485ee5df..e320104e7 100644 --- a/src/Core/Config/Adapter/JITPConfigAdapter.php +++ b/src/Core/Config/Adapter/JITPConfigAdapter.php @@ -29,16 +29,18 @@ class JITPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfigAdap if (DBA::isResult($pconfigs)) { while ($pconfig = DBA::fetch($pconfigs)) { $key = $pconfig['k']; - $value = $pconfig['v']; + $value = $this->toConfigValue($pconfig['v']); - if (isset($value) && $value !== '') { + // The value was in the db, so don't check it again (unless you have to) + $this->in_db[$uid][$cat][$key] = true; + + if (isset($value)) { $return[$key] = $value; - $this->in_db[$uid][$cat][$key] = true; } } } else if ($cat != 'config') { // Negative caching - $return = "!!"; + $return = null; } DBA::close($pconfigs); @@ -51,22 +53,23 @@ class JITPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfigAdap public function get($uid, $cat, $key) { if (!$this->isConnected()) { - return '!!'; + return null; } + // The value was in the db, so don't check it again (unless you have to) + $this->in_db[$uid][$cat][$key] = true; + $pconfig = DBA::selectFirst('pconfig', ['v'], ['uid' => $uid, 'cat' => $cat, 'k' => $key]); if (DBA::isResult($pconfig)) { - // manage array value - $value = (preg_match("|^a:[0-9]+:{.*}$|s", $pconfig['v']) ? unserialize($pconfig['v']) : $pconfig['v']); + $value = $this->toConfigValue($pconfig['v']); - if (isset($value) && $value !== '') { - $this->in_db[$uid][$cat][$key] = true; + if (isset($value)) { return $value; } } $this->in_db[$uid][$cat][$key] = false; - return '!!'; + return null; } /** @@ -118,13 +121,11 @@ class JITPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfigAdap return false; } - if (!empty($this->in_db[$uid][$cat][$key])) { + if (isset($this->in_db[$uid][$cat][$key])) { unset($this->in_db[$uid][$cat][$key]); } - $result = DBA::delete('pconfig', ['uid' => $uid, 'cat' => $cat, 'k' => $key]); - - return $result; + return DBA::delete('pconfig', ['uid' => $uid, 'cat' => $cat, 'k' => $key]); } /** diff --git a/src/Core/Config/Adapter/PreloadConfigAdapter.php b/src/Core/Config/Adapter/PreloadConfigAdapter.php index 218648cbd..7ebd1efcf 100644 --- a/src/Core/Config/Adapter/PreloadConfigAdapter.php +++ b/src/Core/Config/Adapter/PreloadConfigAdapter.php @@ -35,8 +35,6 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd $value = $config['v']; if (isset($value) && $value !== '') { $return[$config['cat']][$config['k']] = $value; - } else { - $return[$config['cat']][$config['k']] = '!!'; } } DBA::close($configs); @@ -52,7 +50,7 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd public function get($cat, $key) { if (!$this->isConnected()) { - return '!!'; + return null; } $config = DBA::selectFirst('config', ['v'], ['cat' => $cat, 'k' => $key]); @@ -65,7 +63,7 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd } } - return '!!'; + return null; } /** @@ -89,9 +87,7 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd // manage array value $dbvalue = is_array($value) ? serialize($value) : $value; - $result = DBA::update('config', ['v' => $dbvalue], ['cat' => $cat, 'k' => $key], true); - - return $result; + return DBA::update('config', ['v' => $dbvalue], ['cat' => $cat, 'k' => $key], true); } /** @@ -103,9 +99,7 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd return false; } - $result = DBA::delete('config', ['cat' => $cat, 'k' => $key]); - - return $result; + return DBA::delete('config', ['cat' => $cat, 'k' => $key]); } /** diff --git a/src/Core/Config/Adapter/PreloadPConfigAdapter.php b/src/Core/Config/Adapter/PreloadPConfigAdapter.php index 12e25df9f..d2ba971ff 100644 --- a/src/Core/Config/Adapter/PreloadPConfigAdapter.php +++ b/src/Core/Config/Adapter/PreloadPConfigAdapter.php @@ -52,8 +52,6 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig $value = $pconfig['v']; if (isset($value) && $value !== '') { $return[$pconfig['cat']][$pconfig['k']] = $value; - } else { - $return[$pconfig['cat']][$pconfig['k']] = '!!'; } } DBA::close($pconfigs); @@ -69,7 +67,7 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig public function get($uid, $cat, $key) { if (!$this->isConnected()) { - return '!!'; + return null; } if (!$this->isLoaded($uid, $cat, $key)) { @@ -85,7 +83,7 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig return $value; } } - return '!!'; + return null; } /** @@ -112,9 +110,7 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig // manage array value $dbvalue = is_array($value) ? serialize($value) : $value; - $result = DBA::update('pconfig', ['v' => $dbvalue], ['uid' => $uid, 'cat' => $cat, 'k' => $key], true); - - return $result; + return DBA::update('pconfig', ['v' => $dbvalue], ['uid' => $uid, 'cat' => $cat, 'k' => $key], true); } /** @@ -130,9 +126,7 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig $this->load($uid, $cat); } - $result = DBA::delete('pconfig', ['uid' => $uid, 'cat' => $cat, 'k' => $key]); - - return $result; + return DBA::delete('pconfig', ['uid' => $uid, 'cat' => $cat, 'k' => $key]); } /** diff --git a/src/Core/Config/Cache/ConfigCache.php b/src/Core/Config/Cache/ConfigCache.php index 4ebcc8748..cf50b43d4 100644 --- a/src/Core/Config/Cache/ConfigCache.php +++ b/src/Core/Config/Cache/ConfigCache.php @@ -6,8 +6,6 @@ namespace Friendica\Core\Config\Cache; * The Friendica config cache for the application * Initial, all *.config.php files are loaded into this cache with the * ConfigCacheLoader ( @see ConfigCacheLoader ) - * - * Is used for further caching operations too (depending on the ConfigAdapter ) */ class ConfigCache implements IConfigCache, IPConfigCache { @@ -37,7 +35,7 @@ class ConfigCache implements IConfigCache, IPConfigCache foreach ($keys as $key) { $value = $config[$category][$key]; - if (isset($value) && $value !== '!!') { + if (isset($value)) { if ($overwrite) { $this->set($category, $key, $value); } else { @@ -56,22 +54,13 @@ class ConfigCache implements IConfigCache, IPConfigCache { if (isset($this->config[$cat][$key])) { return $this->config[$cat][$key]; - } elseif ($key == null && isset($this->config[$cat])) { + } elseif (!isset($key) && isset($this->config[$cat])) { return $this->config[$cat]; } else { - return '!!'; + return null; } } - /** - * {@inheritdoc} - */ - public function has($cat, $key = null) - { - return (isset($this->config[$cat][$key]) && $this->config[$cat][$key] !== '!!') || - ($key == null && isset($this->config[$cat]) && $this->config[$cat] !== '!!' && is_array($this->config[$cat])); - } - /** * Sets a default value in the config cache. Ignores already existing keys. * @@ -91,9 +80,6 @@ class ConfigCache implements IConfigCache, IPConfigCache */ public function set($cat, $key, $value) { - // Only arrays are serialized in database, so we have to unserialize sparingly - $value = is_string($value) && preg_match("|^a:[0-9]+:{.*}$|s", $value) ? unserialize($value) : $value; - if (!isset($this->config[$cat])) { $this->config[$cat] = []; } @@ -103,15 +89,6 @@ class ConfigCache implements IConfigCache, IPConfigCache return true; } - /** - * {@inheritdoc} - */ - public function hasP($uid, $cat, $key = null) - { - return (isset($this->config[$uid][$cat][$key]) && $this->config[$uid][$cat][$key] !== '!!') || - ($key == null && isset($this->config[$uid][$cat]) && $this->config[$uid][$cat] !== '!!' && is_array($this->config[$uid][$cat])); - } - /** * {@inheritdoc} */ @@ -142,7 +119,7 @@ class ConfigCache implements IConfigCache, IPConfigCache foreach ($keys as $key) { $value = $config[$category][$key]; - if (isset($value) && $value !== '!!') { + if (isset($value)) { $this->setP($uid, $category, $key, $value); } } @@ -157,10 +134,10 @@ class ConfigCache implements IConfigCache, IPConfigCache { if (isset($this->config[$uid][$cat][$key])) { return $this->config[$uid][$cat][$key]; - } elseif ($key == null && isset($this->config[$uid][$cat])) { + } elseif (!isset($key) && isset($this->config[$uid][$cat])) { return $this->config[$uid][$cat]; } else { - return '!!'; + return null; } } @@ -169,9 +146,6 @@ class ConfigCache implements IConfigCache, IPConfigCache */ public function setP($uid, $cat, $key, $value) { - // Only arrays are serialized in database, so we have to unserialize sparingly - $value = is_string($value) && preg_match("|^a:[0-9]+:{.*}$|s", $value) ? unserialize($value) : $value; - if (!isset($this->config[$uid]) || !is_array($this->config[$uid])) { $this->config[$uid] = []; } diff --git a/src/Core/Config/Cache/ConfigCacheLoader.php b/src/Core/Config/Cache/ConfigCacheLoader.php index b728d1082..b043bf27c 100644 --- a/src/Core/Config/Cache/ConfigCacheLoader.php +++ b/src/Core/Config/Cache/ConfigCacheLoader.php @@ -37,9 +37,6 @@ class ConfigCacheLoader */ public function loadConfigFiles(ConfigCache $config) { - // Setting at least the basepath we know - $config->set('system', 'basepath', $this->baseDir); - $config->load($this->loadCoreConfig('defaults')); $config->load($this->loadCoreConfig('settings')); diff --git a/src/Core/Config/Cache/IConfigCache.php b/src/Core/Config/Cache/IConfigCache.php index 9d948527d..499bd312d 100644 --- a/src/Core/Config/Cache/IConfigCache.php +++ b/src/Core/Config/Cache/IConfigCache.php @@ -20,9 +20,9 @@ interface IConfigCache * Gets a value from the config cache. * * @param string $cat Config category - * @param string $key Config key + * @param string $key Config key * - * @return mixed Returns the value of the Config entry or '!!' if not set + * @return null|mixed Returns the value of the Config entry or null if not set */ function get($cat, $key = null); @@ -47,15 +47,6 @@ interface IConfigCache */ function delete($cat, $key); - /** - * Checks if a value is set in the config cache. - * - * @param string $cat Config category - * @param string $key Config key - * @return bool - */ - function has($cat, $key = null); - /** * Returns the whole configuration cache * diff --git a/src/Core/Config/Cache/IPConfigCache.php b/src/Core/Config/Cache/IPConfigCache.php index 4ac21481a..30076a2a9 100644 --- a/src/Core/Config/Cache/IPConfigCache.php +++ b/src/Core/Config/Cache/IPConfigCache.php @@ -23,7 +23,7 @@ interface IPConfigCache * @param string $cat Config category * @param string $key Config key * - * @return string The value of the config entry or '!!' if not set + * @return null|string The value of the config entry or null if not set */ function getP($uid, $cat, $key = null); @@ -50,17 +50,6 @@ interface IPConfigCache */ function deleteP($uid, $cat, $key); - - /** - * Checks if a value is set in the user config cache. - * - * @param int $uid User Id - * @param string $cat Config category - * @param string $key Config key - * @return bool - */ - function hasP($uid, $cat, $key = null); - /** * Returns the whole configuration cache * diff --git a/src/Core/Config/Configuration.php b/src/Core/Config/Configuration.php index 2ac0da0ad..532ed982a 100644 --- a/src/Core/Config/Configuration.php +++ b/src/Core/Config/Configuration.php @@ -83,20 +83,19 @@ class Configuration if ($this->configAdapter->isConnected() && (!$this->configAdapter->isLoaded($cat, $key) || $refresh)) { + $dbvalue = $this->configAdapter->get($cat, $key); - if ($dbvalue !== '!!') { + if (isset($dbvalue)) { $this->configCache->set($cat, $key, $dbvalue); return $dbvalue; } } // use the config cache for return - if ($this->configCache->has($cat, $key)) { - return $this->configCache->get($cat, $key); - } else { - return $default_value; - } + $result = $this->configCache->get($cat, $key); + + return (isset($result)) ? $result : $default_value; } /** diff --git a/src/Core/Config/PConfiguration.php b/src/Core/Config/PConfiguration.php index 99b1aa146..0d3b0c178 100644 --- a/src/Core/Config/PConfiguration.php +++ b/src/Core/Config/PConfiguration.php @@ -77,18 +77,15 @@ class PConfiguration $refresh)) { $dbValue = $this->configAdapter->get($uid, $cat, $key); - if ($dbValue !== '!!') { + if (isset($dbValue)) { $this->configCache->setP($uid, $cat, $key, $dbValue); return $dbValue; } } // use the config cache for return - if ($this->configCache->hasP($uid, $cat, $key)) { - return $this->configCache->getP($uid, $cat, $key); - } else { - return $default_value; - } + $result = $this->configCache->getP($uid, $cat, $key); + return (isset($result)) ? $result : $default_value; } /** diff --git a/src/Core/PConfig.php b/src/Core/PConfig.php index f62b59f47..da4c80293 100644 --- a/src/Core/PConfig.php +++ b/src/Core/PConfig.php @@ -65,8 +65,6 @@ class PConfig /** * @brief Sets a configuration value for a user * - * @note Please do not store booleans - convert to 0/1 integer values! - * * @param string $uid The user_id * @param string $cat The category of the configuration value * @param string $key The configuration key to set diff --git a/src/Database/DBA.php b/src/Database/DBA.php index 1c17d9aca..97e5c63c8 100644 --- a/src/Database/DBA.php +++ b/src/Database/DBA.php @@ -40,6 +40,10 @@ class DBA * @var Profiler */ private static $profiler; + /** + * @var string + */ + private static $basedir; private static $server_info = ''; private static $connection; private static $driver; @@ -55,13 +59,14 @@ class DBA private static $db_name = ''; private static $db_charset = ''; - public static function connect(IConfigCache $configCache, Profiler $profiler, $serveraddr, $user, $pass, $db, $charset = null) + public static function connect($basedir, IConfigCache $configCache, Profiler $profiler, $serveraddr, $user, $pass, $db, $charset = null) { if (!is_null(self::$connection) && self::connected()) { return true; } // We are storing these values for being able to perform a reconnect + self::$basedir = $basedir; self::$configCache = $configCache; self::$profiler = $profiler; self::$db_serveraddr = $serveraddr; @@ -1034,7 +1039,7 @@ class DBA * This process must only be started once, since the value is cached. */ private static function buildRelationData() { - $definition = DBStructure::definition(self::$configCache->get('system', 'basepath')); + $definition = DBStructure::definition(self::$basedir); foreach ($definition AS $table => $structure) { foreach ($structure['fields'] AS $field => $field_struct) { diff --git a/src/Factory/DBFactory.php b/src/Factory/DBFactory.php index c1a796501..b4f0c9e3c 100644 --- a/src/Factory/DBFactory.php +++ b/src/Factory/DBFactory.php @@ -11,13 +11,16 @@ class DBFactory /** * Initialize the DBA connection * + * @param string $basePath The basepath of the application * @param Cache\IConfigCache $configCache The configuration cache - * @param Profiler $profiler The profiler - * @param array $server The $_SERVER variables + * @param Profiler $profiler The profiler + * @param array $server The $_SERVER variables * * @throws \Exception if connection went bad + * + * @todo refactor basedir during https://github.com/friendica/friendica/issues/6720 */ - public static function init(Cache\IConfigCache $configCache, Profiler $profiler, array $server) + public static function init($basePath, Cache\IConfigCache $configCache, Profiler $profiler, array $server) { if (Database\DBA::connected()) { return; @@ -48,9 +51,9 @@ class DBFactory $db_data = $server['MYSQL_DATABASE']; } - if (Database\DBA::connect($configCache, $profiler, $db_host, $db_user, $db_pass, $db_data, $charset)) { + if (Database\DBA::connect($basePath, $configCache, $profiler, $db_host, $db_user, $db_pass, $db_data, $charset)) { // Loads DB_UPDATE_VERSION constant - Database\DBStructure::definition($configCache->get('system', 'basepath'), false); + Database\DBStructure::definition($basePath, false); } unset($db_host, $db_user, $db_pass, $db_data, $charset); diff --git a/src/Factory/DependencyFactory.php b/src/Factory/DependencyFactory.php index acbf4bfaf..52178bb77 100644 --- a/src/Factory/DependencyFactory.php +++ b/src/Factory/DependencyFactory.php @@ -22,16 +22,16 @@ class DependencyFactory */ public static function setUp($channel, $directory, $isBackend = true) { - $basedir = BasePath::create($directory, $_SERVER); - $configLoader = new Cache\ConfigCacheLoader($basedir); + $basePath = BasePath::create($directory, $_SERVER); + $configLoader = new Cache\ConfigCacheLoader($basePath); $configCache = Factory\ConfigFactory::createCache($configLoader); $profiler = Factory\ProfilerFactory::create($configCache); - Factory\DBFactory::init($configCache, $profiler, $_SERVER); + Factory\DBFactory::init($basePath, $configCache, $profiler, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); // needed to call PConfig::init() Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create($channel, $config); - return new App($config, $logger, $profiler, $isBackend); + return new App($basePath, $config, $logger, $profiler, $isBackend); } } diff --git a/src/Model/Photo.php b/src/Model/Photo.php index 152e870e8..8ad7f3145 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -10,8 +10,8 @@ use Friendica\BaseObject; use Friendica\Core\Cache; use Friendica\Core\Config; use Friendica\Core\L10n; -use Friendica\Core\System; use Friendica\Core\StorageManager; +use Friendica\Core\System; use Friendica\Database\DBA; use Friendica\Database\DBStructure; use Friendica\Model\Storage\IStorage; @@ -203,7 +203,7 @@ class Photo extends BaseObject */ private static function getFields() { - $allfields = DBStructure::definition(false); + $allfields = DBStructure::definition(self::getApp()->getBasePath(), false); $fields = array_keys($allfields["photo"]["fields"]); array_splice($fields, array_search("data", $fields), 1); return $fields; diff --git a/tests/include/ApiTest.php b/tests/include/ApiTest.php index 289b3fcea..2f8becc18 100644 --- a/tests/include/ApiTest.php +++ b/tests/include/ApiTest.php @@ -36,15 +36,15 @@ class ApiTest extends DatabaseTest */ public function setUp() { - $basedir = BasePath::create(dirname(__DIR__) . '/../'); - $configLoader = new Cache\ConfigCacheLoader($basedir); + $basePath = BasePath::create(dirname(__DIR__) . '/../'); + $configLoader = new Cache\ConfigCacheLoader($basePath); $configCache = Factory\ConfigFactory::createCache($configLoader); $profiler = Factory\ProfilerFactory::create($configCache); - Factory\DBFactory::init($configCache, $profiler, $_SERVER); + Factory\DBFactory::init($basePath, $configCache, $profiler, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('test', $config); - $this->app = new App($config, $logger, $profiler, false); + $this->app = new App($basePath, $config, $logger, $profiler, false); parent::setUp(); diff --git a/tests/src/Database/DBATest.php b/tests/src/Database/DBATest.php index e8b9c68b1..8e102ec63 100644 --- a/tests/src/Database/DBATest.php +++ b/tests/src/Database/DBATest.php @@ -13,15 +13,15 @@ class DBATest extends DatabaseTest { public function setUp() { - $basedir = BasePath::create(dirname(__DIR__) . '/../../'); - $configLoader = new Cache\ConfigCacheLoader($basedir); + $basePath = BasePath::create(dirname(__DIR__) . '/../../'); + $configLoader = new Cache\ConfigCacheLoader($basePath); $configCache = Factory\ConfigFactory::createCache($configLoader); $profiler = Factory\ProfilerFactory::create($configCache); - Factory\DBFactory::init($configCache, $profiler, $_SERVER); + Factory\DBFactory::init($basePath, $configCache, $profiler, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('test', $config); - $this->app = new App($config, $logger, $profiler, false); + $this->app = new App($basePath, $config, $logger, $profiler, false); parent::setUp(); diff --git a/tests/src/Database/DBStructureTest.php b/tests/src/Database/DBStructureTest.php index 325ad4e5e..454d6e8a4 100644 --- a/tests/src/Database/DBStructureTest.php +++ b/tests/src/Database/DBStructureTest.php @@ -13,15 +13,15 @@ class DBStructureTest extends DatabaseTest { public function setUp() { - $basedir = BasePath::create(dirname(__DIR__) . '/../../'); - $configLoader = new Cache\ConfigCacheLoader($basedir); + $basePath = BasePath::create(dirname(__DIR__) . '/../../'); + $configLoader = new Cache\ConfigCacheLoader($basePath); $configCache = Factory\ConfigFactory::createCache($configLoader); $profiler = Factory\ProfilerFactory::create($configCache); - Factory\DBFactory::init($configCache, $profiler, $_SERVER); + Factory\DBFactory::init($basePath, $configCache, $profiler, $_SERVER); $config = Factory\ConfigFactory::createConfig($configCache); Factory\ConfigFactory::createPConfig($configCache); $logger = Factory\LoggerFactory::create('test', $config); - $this->app = new App($config, $logger, $profiler, false); + $this->app = new App($basePath, $config, $logger, $profiler, false); parent::setUp(); } From b47fc97b06ef1a0939f0f2d05e33b36288fa8303 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Fri, 22 Feb 2019 23:55:13 +0100 Subject: [PATCH 2/7] readded note --- src/Core/Config.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Core/Config.php b/src/Core/Config.php index f974e2fff..44556a601 100644 --- a/src/Core/Config.php +++ b/src/Core/Config.php @@ -48,6 +48,8 @@ class Config * @brief Get a particular user's config variable given the category name * ($family) and a key. * + * Note: Please do not store booleans - convert to 0/1 integer values! + * * @param string $cat The category of the configuration value * @param string $key The configuration key to query * @param mixed $default_value optional, The value to return if key is not set (default: null) From 8d56fb8fbef24496fc4a8d6cbfeeba634789a317 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Fri, 22 Feb 2019 23:57:04 +0100 Subject: [PATCH 3/7] bugfix marking of db-values --- src/Core/Config/Adapter/JITPConfigAdapter.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Core/Config/Adapter/JITPConfigAdapter.php b/src/Core/Config/Adapter/JITPConfigAdapter.php index e320104e7..5c1627eba 100644 --- a/src/Core/Config/Adapter/JITPConfigAdapter.php +++ b/src/Core/Config/Adapter/JITPConfigAdapter.php @@ -49,15 +49,19 @@ class JITPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfigAdap /** * {@inheritdoc} + * + * @param bool $mark if true, mark the selection of the current cat/key pair */ - public function get($uid, $cat, $key) + public function get($uid, $cat, $key, $mark = true) { if (!$this->isConnected()) { return null; } // The value was in the db, so don't check it again (unless you have to) - $this->in_db[$uid][$cat][$key] = true; + if ($mark) { + $this->in_db[$uid][$cat][$key] = true; + } $pconfig = DBA::selectFirst('pconfig', ['v'], ['uid' => $uid, 'cat' => $cat, 'k' => $key]); if (DBA::isResult($pconfig)) { @@ -86,7 +90,7 @@ class JITPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfigAdap // The exception are array values. $dbvalue = (!is_array($value) ? (string)$value : $value); - $stored = $this->get($uid, $cat, $key); + $stored = $this->get($uid, $cat, $key, false); if (!isset($this->in_db[$uid])) { $this->in_db[$uid] = []; From 1dee89f215f5411d451078fb9abf8ac8dea85c18 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 23 Feb 2019 00:09:57 +0100 Subject: [PATCH 4/7] fixing tests and preload config --- .../Config/Adapter/PreloadConfigAdapter.php | 4 +- .../Config/Adapter/PreloadPConfigAdapter.php | 4 +- .../Config/Cache/ConfigCacheLoaderTest.php | 13 ------- .../src/Core/Config/Cache/ConfigCacheTest.php | 37 +------------------ tests/src/Core/Config/ConfigurationTest.php | 10 ++--- tests/src/Core/Config/PConfigurationTest.php | 4 +- 6 files changed, 12 insertions(+), 60 deletions(-) diff --git a/src/Core/Config/Adapter/PreloadConfigAdapter.php b/src/Core/Config/Adapter/PreloadConfigAdapter.php index 7ebd1efcf..d9f7b132d 100644 --- a/src/Core/Config/Adapter/PreloadConfigAdapter.php +++ b/src/Core/Config/Adapter/PreloadConfigAdapter.php @@ -33,7 +33,7 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd $configs = DBA::select('config', ['cat', 'v', 'k']); while ($config = DBA::fetch($configs)) { $value = $config['v']; - if (isset($value) && $value !== '') { + if (isset($value)) { $return[$config['cat']][$config['k']] = $value; } } @@ -58,7 +58,7 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd // manage array value $value = (preg_match("|^a:[0-9]+:{.*}$|s", $config['v']) ? unserialize($config['v']) : $config['v']); - if (isset($value) && $value !== '') { + if (isset($value)) { return $value; } } diff --git a/src/Core/Config/Adapter/PreloadPConfigAdapter.php b/src/Core/Config/Adapter/PreloadPConfigAdapter.php index d2ba971ff..1af526b7c 100644 --- a/src/Core/Config/Adapter/PreloadPConfigAdapter.php +++ b/src/Core/Config/Adapter/PreloadPConfigAdapter.php @@ -50,7 +50,7 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig $pconfigs = DBA::select('pconfig', ['cat', 'v', 'k'], ['uid' => $uid]); while ($pconfig = DBA::fetch($pconfigs)) { $value = $pconfig['v']; - if (isset($value) && $value !== '') { + if (isset($value)) { $return[$pconfig['cat']][$pconfig['k']] = $value; } } @@ -79,7 +79,7 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig // manage array value $value = (preg_match("|^a:[0-9]+:{.*}$|s", $config['v']) ? unserialize($config['v']) : $config['v']); - if (isset($value) && $value !== '') { + if (isset($value)) { return $value; } } diff --git a/tests/src/Core/Config/Cache/ConfigCacheLoaderTest.php b/tests/src/Core/Config/Cache/ConfigCacheLoaderTest.php index f91f267c7..100c4ab65 100644 --- a/tests/src/Core/Config/Cache/ConfigCacheLoaderTest.php +++ b/tests/src/Core/Config/Cache/ConfigCacheLoaderTest.php @@ -19,19 +19,6 @@ class ConfigCacheLoaderTest extends MockedTest $this->setUpVfsDir(); } - /** - * Test the loadConfigFiles() method with default values - */ - public function testLoadConfigFiles() - { - $configCacheLoader = new ConfigCacheLoader($this->root->url()); - $configCache = new ConfigCache(); - - $configCacheLoader->loadConfigFiles($configCache); - - $this->assertEquals($this->root->url(), $configCache->get('system', 'basepath')); - } - /** * Test the loadConfigFiles() method with a wrong local.config.php * @expectedException \Exception diff --git a/tests/src/Core/Config/Cache/ConfigCacheTest.php b/tests/src/Core/Config/Cache/ConfigCacheTest.php index ac9fae540..582007ed8 100644 --- a/tests/src/Core/Config/Cache/ConfigCacheTest.php +++ b/tests/src/Core/Config/Cache/ConfigCacheTest.php @@ -138,22 +138,7 @@ class ConfigCacheTest extends MockedTest { $configCache = new ConfigCache(); - $this->assertEquals('!!', $configCache->get('something', 'value')); - } - - /** - * Test the has() method - */ - public function testHas() - { - $configCache = new ConfigCache(); - - $this->assertFalse($configCache->has('system', 'test')); - $this->assertFalse($configCache->has('system')); - - $configCache->set('system', 'test', 'it'); - $this->assertTrue($configCache->has('system', 'test')); - $this->assertTrue($configCache->has('system')); + $this->assertNull($configCache->get('something', 'value')); } /** @@ -171,8 +156,6 @@ class ConfigCacheTest extends MockedTest ], ]); - $this->assertTrue($configCache->has('system')); - $this->assertEquals([ 'key1' => 'value1', 'key2' => 'value2', @@ -233,8 +216,6 @@ class ConfigCacheTest extends MockedTest ], ]); - $this->assertTrue($configCache->hasP($uid,'system')); - $this->assertEquals([ 'key1' => 'value1', 'key2' => 'value2', @@ -264,20 +245,4 @@ class ConfigCacheTest extends MockedTest $this->assertEmpty($configCache->getAll()); } - - /** - * Test the hasP() method - */ - public function testHasP() - { - $configCache = new ConfigCache(); - $uid = 345; - - $this->assertFalse($configCache->hasP($uid, 'system', 'test')); - $this->assertFalse($configCache->hasP($uid, 'system')); - - $configCache->setP($uid, 'system', 'test', 'it'); - $this->assertTrue($configCache->hasP($uid, 'system', 'test')); - $this->assertTrue($configCache->hasP($uid, 'system')); - } } diff --git a/tests/src/Core/Config/ConfigurationTest.php b/tests/src/Core/Config/ConfigurationTest.php index 2e4fcd4f5..d06b2beb3 100644 --- a/tests/src/Core/Config/ConfigurationTest.php +++ b/tests/src/Core/Config/ConfigurationTest.php @@ -142,7 +142,7 @@ class ConfigurationTest extends MockedTest $this->assertNull($configuration->get('test', 'it')); /// beware that the cache returns '!!' and not null for a non existing value - $this->assertEquals('!!', $configuration->getCache()->get('test', 'it')); + $this->assertNull($configuration->getCache()->get('test', 'it')); // with default value $this->assertEquals('default', $configuration->get('test', 'it', 'default')); @@ -165,7 +165,7 @@ class ConfigurationTest extends MockedTest $configAdapter->shouldReceive('isLoaded')->with('test', 'it')->andReturn(true)->twice(); $configAdapter->shouldReceive('get')->with('test', 'it')->andReturn($data)->once(); $configAdapter->shouldReceive('isLoaded')->with('test', 'not')->andReturn(false)->once(); - $configAdapter->shouldReceive('get')->with('test', 'not')->andReturn('!!')->once(); + $configAdapter->shouldReceive('get')->with('test', 'not')->andReturn(null)->once(); $configuration = new Configuration($configCache, $configAdapter); @@ -179,7 +179,7 @@ class ConfigurationTest extends MockedTest // without refresh and wrong value and default $this->assertEquals('default', $configuration->get('test', 'not', 'default')); - $this->assertEquals('!!', $configuration->getCache()->get('test', 'not')); + $this->assertNull($configuration->getCache()->get('test', 'not')); } /** @@ -195,7 +195,7 @@ class ConfigurationTest extends MockedTest $configAdapter->shouldReceive('load')->andReturn([])->once(); $configAdapter->shouldReceive('isLoaded')->with('test', 'it')->andReturn(false)->once(); - $configAdapter->shouldReceive('get')->with('test', 'it')->andReturn('!!')->once(); + $configAdapter->shouldReceive('get')->with('test', 'it')->andReturn(null)->once(); $configAdapter->shouldReceive('isLoaded')->with('test', 'it')->andReturn(false)->once(); $configAdapter->shouldReceive('get')->with('test', 'it')->andReturn($data)->once(); @@ -234,7 +234,7 @@ class ConfigurationTest extends MockedTest $this->assertTrue($configuration->delete('test', 'it')); $this->assertNull($configuration->get('test', 'it')); - $this->assertEquals('!!', $configuration->getCache()->get('test', 'it')); + $this->assertNull($configuration->getCache()->get('test', 'it')); $this->assertEmpty($configuration->getCache()->getAll()); } diff --git a/tests/src/Core/Config/PConfigurationTest.php b/tests/src/Core/Config/PConfigurationTest.php index 025994414..68443d05b 100644 --- a/tests/src/Core/Config/PConfigurationTest.php +++ b/tests/src/Core/Config/PConfigurationTest.php @@ -145,7 +145,7 @@ class PConfigurationTest extends MockedTest $configAdapter->shouldReceive('isLoaded')->with($uid, 'test', 'it')->andReturn(true)->twice(); $configAdapter->shouldReceive('get')->with($uid, 'test', 'it')->andReturn($data)->once(); $configAdapter->shouldReceive('isLoaded')->with($uid, 'test', 'not')->andReturn(false)->once(); - $configAdapter->shouldReceive('get')->with($uid, 'test', 'not')->andReturn('!!')->once(); + $configAdapter->shouldReceive('get')->with($uid, 'test', 'not')->andReturn(null)->once(); $configuration = new PConfiguration($configCache, $configAdapter); @@ -173,7 +173,7 @@ class PConfigurationTest extends MockedTest $configAdapter->shouldReceive('isConnected')->andReturn(true)->times(3); $configAdapter->shouldReceive('isLoaded')->with($uid, 'test', 'it')->andReturn(false)->once(); - $configAdapter->shouldReceive('get')->with($uid, 'test', 'it')->andReturn('!!')->once(); + $configAdapter->shouldReceive('get')->with($uid, 'test', 'it')->andReturn(null)->once(); $configAdapter->shouldReceive('isLoaded')->with($uid, 'test', 'it')->andReturn(false)->once(); $configAdapter->shouldReceive('get')->with($uid, 'test', 'it')->andReturn($data)->once(); From 13807129aa26e61176df36f0f2d78ab9ecbe06c5 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 23 Feb 2019 00:23:41 +0100 Subject: [PATCH 5/7] fixing preload adapter --- src/Core/Config/Adapter/JITConfigAdapter.php | 10 ++++------ src/Core/Config/Adapter/JITPConfigAdapter.php | 9 ++++----- src/Core/Config/Adapter/PreloadConfigAdapter.php | 11 +++++------ src/Core/Config/Adapter/PreloadPConfigAdapter.php | 11 +++++------ 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/Core/Config/Adapter/JITConfigAdapter.php b/src/Core/Config/Adapter/JITConfigAdapter.php index 5a4146303..d125f7d40 100644 --- a/src/Core/Config/Adapter/JITConfigAdapter.php +++ b/src/Core/Config/Adapter/JITConfigAdapter.php @@ -67,7 +67,6 @@ class JITConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAdapte $config = DBA::selectFirst('config', ['v'], ['cat' => $cat, 'k' => $key]); if (DBA::isResult($config)) { - // manage array value $value = $this->toConfigValue($config['v']); // just return it in case it is set @@ -91,9 +90,8 @@ class JITConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAdapte // We store our setting values in a string variable. // So we have to do the conversion here so that the compare below works. // The exception are array values. - $dbvalue = (!is_array($value) ? (string)$value : $value); - - $stored = $this->get($cat, $key, false); + $compare_value = (!is_array($value) ? (string)$value : $value); + $stored_value = $this->get($cat, $key, false); if (!isset($this->in_db[$cat])) { $this->in_db[$cat] = []; @@ -102,11 +100,11 @@ class JITConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAdapte $this->in_db[$cat][$key] = false; } - if (isset($stored) && ($stored === $dbvalue) && $this->in_db[$cat][$key]) { + if (isset($stored_value) && ($stored_value === $compare_value) && $this->in_db[$cat][$key]) { return true; } - $dbvalue = $this->toDbValue($dbvalue); + $dbvalue = $this->toDbValue($value); $result = DBA::update('config', ['v' => $dbvalue], ['cat' => $cat, 'k' => $key], true); diff --git a/src/Core/Config/Adapter/JITPConfigAdapter.php b/src/Core/Config/Adapter/JITPConfigAdapter.php index 5c1627eba..a0c6a9547 100644 --- a/src/Core/Config/Adapter/JITPConfigAdapter.php +++ b/src/Core/Config/Adapter/JITPConfigAdapter.php @@ -88,9 +88,8 @@ class JITPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfigAdap // We store our setting values in a string variable. // So we have to do the conversion here so that the compare below works. // The exception are array values. - $dbvalue = (!is_array($value) ? (string)$value : $value); - - $stored = $this->get($uid, $cat, $key, false); + $compare_value = (!is_array($value) ? (string)$value : $value); + $stored_value = $this->get($uid, $cat, $key, false); if (!isset($this->in_db[$uid])) { $this->in_db[$uid] = []; @@ -102,12 +101,12 @@ class JITPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfigAdap $this->in_db[$uid][$cat][$key] = false; } - if (($stored === $dbvalue) && $this->in_db[$uid][$cat][$key]) { + if (isset($stored_value) && ($stored_value === $compare_value) && $this->in_db[$uid][$cat][$key]) { return true; } // manage array value - $dbvalue = (is_array($value) ? serialize($value) : $dbvalue); + $dbvalue = (is_array($value) ? serialize($value) : $value); $result = DBA::update('pconfig', ['v' => $dbvalue], ['uid' => $uid, 'cat' => $cat, 'k' => $key], true); diff --git a/src/Core/Config/Adapter/PreloadConfigAdapter.php b/src/Core/Config/Adapter/PreloadConfigAdapter.php index d9f7b132d..c691c88bc 100644 --- a/src/Core/Config/Adapter/PreloadConfigAdapter.php +++ b/src/Core/Config/Adapter/PreloadConfigAdapter.php @@ -32,7 +32,7 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd $configs = DBA::select('config', ['cat', 'v', 'k']); while ($config = DBA::fetch($configs)) { - $value = $config['v']; + $value = $this->toConfigValue($config['v']); if (isset($value)) { $return[$config['cat']][$config['k']] = $value; } @@ -55,8 +55,7 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd $config = DBA::selectFirst('config', ['v'], ['cat' => $cat, 'k' => $key]); if (DBA::isResult($config)) { - // manage array value - $value = (preg_match("|^a:[0-9]+:{.*}$|s", $config['v']) ? unserialize($config['v']) : $config['v']); + $value = $this->toConfigValue($config['v']); if (isset($value)) { return $value; @@ -79,13 +78,13 @@ class PreloadConfigAdapter extends AbstractDbaConfigAdapter implements IConfigAd // So we have to do the conversion here so that the compare below works. // The exception are array values. $compare_value = !is_array($value) ? (string)$value : $value; + $stored_value = $this->get($cat, $key); - if ($this->get($cat, $key) === $compare_value) { + if (isset($stored_value) && $stored_value === $compare_value) { return true; } - // manage array value - $dbvalue = is_array($value) ? serialize($value) : $value; + $dbvalue = $this->toDbValue($value); return DBA::update('config', ['v' => $dbvalue], ['cat' => $cat, 'k' => $key], true); } diff --git a/src/Core/Config/Adapter/PreloadPConfigAdapter.php b/src/Core/Config/Adapter/PreloadPConfigAdapter.php index 1af526b7c..838f3763d 100644 --- a/src/Core/Config/Adapter/PreloadPConfigAdapter.php +++ b/src/Core/Config/Adapter/PreloadPConfigAdapter.php @@ -49,7 +49,7 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig $pconfigs = DBA::select('pconfig', ['cat', 'v', 'k'], ['uid' => $uid]); while ($pconfig = DBA::fetch($pconfigs)) { - $value = $pconfig['v']; + $value = $this->toConfigValue($pconfig['v']); if (isset($value)) { $return[$pconfig['cat']][$pconfig['k']] = $value; } @@ -76,8 +76,7 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig $config = DBA::selectFirst('pconfig', ['v'], ['uid' => $uid, 'cat' => $cat, 'k' => $key]); if (DBA::isResult($config)) { - // manage array value - $value = (preg_match("|^a:[0-9]+:{.*}$|s", $config['v']) ? unserialize($config['v']) : $config['v']); + $value = $this->toConfigValue($config['v']); if (isset($value)) { return $value; @@ -102,13 +101,13 @@ class PreloadPConfigAdapter extends AbstractDbaConfigAdapter implements IPConfig // So we have to do the conversion here so that the compare below works. // The exception are array values. $compare_value = !is_array($value) ? (string)$value : $value; + $stored_value = $this->get($uid, $cat, $key); - if ($this->get($uid, $cat, $key) === $compare_value) { + if (isset($stored_value) && $stored_value === $compare_value) { return true; } - // manage array value - $dbvalue = is_array($value) ? serialize($value) : $value; + $dbvalue = $this->toDbValue($value); return DBA::update('pconfig', ['v' => $dbvalue], ['uid' => $uid, 'cat' => $cat, 'k' => $key], true); } From b23368b0eaded02df85d6b7d582f99b1a4ec7ce2 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 23 Feb 2019 00:51:08 +0100 Subject: [PATCH 6/7] bugfixing test --- tests/DatabaseTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/DatabaseTest.php b/tests/DatabaseTest.php index dde61e856..bb87cf36d 100644 --- a/tests/DatabaseTest.php +++ b/tests/DatabaseTest.php @@ -47,6 +47,7 @@ abstract class DatabaseTest extends MockedTest $profiler = \Mockery::mock(Profiler::class); DBA::connect( + $basedir, $config, $profiler, getenv('MYSQL_HOST'), From 40e9bbeb88961e494510a18f24d595abfb83f9f8 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 23 Feb 2019 00:53:31 +0100 Subject: [PATCH 7/7] cleanup --- src/App.php | 2 +- src/Core/Config.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/App.php b/src/App.php index e5ac5dda3..9a1536fbe 100644 --- a/src/App.php +++ b/src/App.php @@ -221,7 +221,7 @@ class App $this->config = $config; $this->profiler = $profiler; $cfgBasePath = $this->config->get('system', 'basepath'); - $this->basePath = (isset($cfgBasePath) && $cfgBasePath !== '') ? $cfgBasePath : $basePath; + $this->basePath = !empty($cfgBasePath) ? $cfgBasePath : $basePath; if (!Core\System::isDirectoryUsable($this->basePath, false)) { throw new Exception('Basepath \'' . $this->basePath . '\' isn\'t usable.'); diff --git a/src/Core/Config.php b/src/Core/Config.php index 44556a601..4bf9c5b11 100644 --- a/src/Core/Config.php +++ b/src/Core/Config.php @@ -48,8 +48,6 @@ class Config * @brief Get a particular user's config variable given the category name * ($family) and a key. * - * Note: Please do not store booleans - convert to 0/1 integer values! - * * @param string $cat The category of the configuration value * @param string $key The configuration key to query * @param mixed $default_value optional, The value to return if key is not set (default: null) @@ -67,6 +65,8 @@ class Config * * Stores a config value ($value) in the category ($cat) under the key ($key) * + * Note: Please do not store booleans - convert to 0/1 integer values! + * * @param string $cat The category of the configuration value * @param string $key The configuration key to set * @param mixed $value The value to store