From 50c2a819a0c5ae50571d36a3e2dec4e8267fd13b Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 27 Nov 2014 16:40:12 +0100 Subject: [PATCH] Extract interaction with config.php into SystemConfig * introduce SystemConfig to avoid DI circle (used by database connection which is itself needed by AllConfig that itself contains the methods to access the config.php which then would need the database connection - did you get it? ;)) * use DI container and use that method in legacy code paths (for easier refactoring later) * create and use getSystemConfig instead of query() in DI container --- lib/private/allconfig.php | 16 +++++++-- lib/private/db/connectionfactory.php | 20 ++++++------ lib/private/server.php | 23 ++++++++++--- lib/private/systemconfig.php | 49 ++++++++++++++++++++++++++++ lib/public/config.php | 10 ++++-- tests/lib/user/user.php | 13 ++++++-- 6 files changed, 109 insertions(+), 22 deletions(-) create mode 100644 lib/private/systemconfig.php diff --git a/lib/private/allconfig.php b/lib/private/allconfig.php index 7ebff7cf2d..295fb8b766 100644 --- a/lib/private/allconfig.php +++ b/lib/private/allconfig.php @@ -13,6 +13,16 @@ namespace OC; * Class to combine all the configuration options ownCloud offers */ class AllConfig implements \OCP\IConfig { + /** @var SystemConfig */ + private $systemConfig; + + /** + * @param SystemConfig $systemConfig + */ + function __construct(SystemConfig $systemConfig) { + $this->systemConfig = $systemConfig; + } + /** * Sets a new system wide value * @@ -20,7 +30,7 @@ class AllConfig implements \OCP\IConfig { * @param mixed $value the value that should be stored */ public function setSystemValue($key, $value) { - \OCP\Config::setSystemValue($key, $value); + $this->systemConfig->setValue($key, $value); } /** @@ -31,7 +41,7 @@ class AllConfig implements \OCP\IConfig { * @return mixed the value or $default */ public function getSystemValue($key, $default = '') { - return \OCP\Config::getSystemValue($key, $default); + return $this->systemConfig->getValue($key, $default); } /** @@ -40,7 +50,7 @@ class AllConfig implements \OCP\IConfig { * @param string $key the key of the value, under which it was saved */ public function deleteSystemValue($key) { - \OCP\Config::deleteSystemValue($key); + $this->systemConfig->deleteValue($key); } /** diff --git a/lib/private/db/connectionfactory.php b/lib/private/db/connectionfactory.php index 58043b3044..9c75baf887 100644 --- a/lib/private/db/connectionfactory.php +++ b/lib/private/db/connectionfactory.php @@ -123,23 +123,23 @@ class ConnectionFactory { /** * Create the connection parameters for the config * - * @param \OCP\IConfig $config + * @param \OC\SystemConfig $config * @return array */ public function createConnectionParams($config) { - $type = $config->getSystemValue('dbtype', 'sqlite'); + $type = $config->getValue('dbtype', 'sqlite'); $connectionParams = array( - 'user' => $config->getSystemValue('dbuser', ''), - 'password' => $config->getSystemValue('dbpassword', ''), + 'user' => $config->getValue('dbuser', ''), + 'password' => $config->getValue('dbpassword', ''), ); - $name = $config->getSystemValue('dbname', 'owncloud'); + $name = $config->getValue('dbname', 'owncloud'); if ($this->normalizeType($type) === 'sqlite3') { - $datadir = $config->getSystemValue("datadirectory", \OC::$SERVERROOT . '/data'); + $datadir = $config->getValue("datadirectory", \OC::$SERVERROOT . '/data'); $connectionParams['path'] = $datadir . '/' . $name . '.db'; } else { - $host = $config->getSystemValue('dbhost', ''); + $host = $config->getValue('dbhost', ''); if (strpos($host, ':')) { // Host variable may carry a port or socket. list($host, $portOrSocket) = explode(':', $host, 2); @@ -153,11 +153,11 @@ class ConnectionFactory { $connectionParams['dbname'] = $name; } - $connectionParams['tablePrefix'] = $config->getSystemValue('dbtableprefix', 'oc_'); - $connectionParams['sqlite.journal_mode'] = $config->getSystemValue('sqlite.journal_mode', 'WAL'); + $connectionParams['tablePrefix'] = $config->getValue('dbtableprefix', 'oc_'); + $connectionParams['sqlite.journal_mode'] = $config->getValue('sqlite.journal_mode', 'WAL'); //additional driver options, eg. for mysql ssl - $driverOptions = $config->getSystemValue('dbdriveroptions', null); + $driverOptions = $config->getValue('dbdriveroptions', null); if ($driverOptions) { $connectionParams['driverOptions'] = $driverOptions; } diff --git a/lib/private/server.php b/lib/private/server.php index a08014fa6f..5a1e955bdd 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -166,8 +166,13 @@ class Server extends SimpleContainer implements IServerContainer { $this->registerService('NavigationManager', function ($c) { return new \OC\NavigationManager(); }); - $this->registerService('AllConfig', function ($c) { - return new \OC\AllConfig(); + $this->registerService('AllConfig', function (Server $c) { + return new \OC\AllConfig( + $c->getSystemConfig() + ); + }); + $this->registerService('SystemConfig', function ($c) { + return new \OC\SystemConfig(); }); $this->registerService('AppConfig', function ($c) { return new \OC\AppConfig(\OC_DB::getConnection()); @@ -229,11 +234,12 @@ class Server extends SimpleContainer implements IServerContainer { }); $this->registerService('DatabaseConnection', function (Server $c) { $factory = new \OC\DB\ConnectionFactory(); - $type = $c->getConfig()->getSystemValue('dbtype', 'sqlite'); + $systemConfig = $c->getSystemConfig(); + $type = $systemConfig->getValue('dbtype', 'sqlite'); if (!$factory->isValidType($type)) { throw new \OC\DatabaseException('Invalid database type'); } - $connectionParams = $factory->createConnectionParams($c->getConfig()); + $connectionParams = $factory->createConnectionParams($systemConfig); $connection = $factory->getConnection($type, $connectionParams); $connection->getConfiguration()->setSQLLogger($c->getQueryLogger()); return $connection; @@ -440,6 +446,15 @@ class Server extends SimpleContainer implements IServerContainer { return $this->query('AllConfig'); } + /** + * For internal use only + * + * @return \OC\SystemConfig + */ + function getSystemConfig() { + return $this->query('SystemConfig'); + } + /** * Returns the app config manager * diff --git a/lib/private/systemconfig.php b/lib/private/systemconfig.php new file mode 100644 index 0000000000..ce6883e5ab --- /dev/null +++ b/lib/private/systemconfig.php @@ -0,0 +1,49 @@ + + * 2013 Bart Visscher + * + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + * + */ + +namespace OC; + +/** + * Class which provides access to the system config values stored in config.php + * Internal class for bootstrap only. + * fixes cyclic DI: AllConfig needs AppConfig needs Database needs AllConfig + */ +class SystemConfig { + /** + * Sets a new system wide value + * + * @param string $key the key of the value, under which will be saved + * @param mixed $value the value that should be stored + */ + public function setValue($key, $value) { + \OC_Config::setValue($key, $value); + } + + /** + * Looks up a system wide defined value + * + * @param string $key the key of the value, under which it was saved + * @param mixed $default the default value to be returned if the value isn't set + * @return mixed the value or $default + */ + public function getValue($key, $default = '') { + return \OC_Config::getValue($key, $default); + } + + /** + * Delete a system wide defined value + * + * @param string $key the key of the value, under which it was saved + */ + public function deleteValue($key) { + \OC_Config::deleteKey($key); + } +} diff --git a/lib/public/config.php b/lib/public/config.php index 65dde39cdc..57c430251b 100644 --- a/lib/public/config.php +++ b/lib/public/config.php @@ -37,6 +37,7 @@ namespace OCP; /** * This class provides functions to read and write configuration data. * configuration can be on a system, application or user level + * @deprecated use methods of \OCP\IConfig */ class Config { /** @@ -44,12 +45,13 @@ class Config { * @param string $key key * @param mixed $default = null default value * @return mixed the value or $default + * @deprecated use method getSystemValue of \OCP\IConfig * * This function gets the value from config.php. If it does not exist, * $default will be returned. */ public static function getSystemValue( $key, $default = null ) { - return \OC_Config::getValue( $key, $default ); + return \OC::$server->getConfig()->getSystemValue( $key, $default ); } /** @@ -57,13 +59,14 @@ class Config { * @param string $key key * @param mixed $value value * @return bool + * @deprecated use method setSystemValue of \OCP\IConfig * * This function sets the value and writes the config.php. If the file can * not be written, false will be returned. */ public static function setSystemValue( $key, $value ) { try { - \OC_Config::setValue( $key, $value ); + \OC::$server->getConfig()->setSystemValue( $key, $value ); } catch (\Exception $e) { return false; } @@ -73,11 +76,12 @@ class Config { /** * Deletes a value from config.php * @param string $key key + * @deprecated use method deleteSystemValue of \OCP\IConfig * * This function deletes the value from config.php. */ public static function deleteSystemValue( $key ) { - return \OC_Config::deleteKey( $key ); + \OC::$server->getConfig()->deleteSystemValue( $key ); } /** diff --git a/tests/lib/user/user.php b/tests/lib/user/user.php index 6aa7243a75..85ade9ccaf 100644 --- a/tests/lib/user/user.php +++ b/tests/lib/user/user.php @@ -228,10 +228,19 @@ class User extends \Test\TestCase { ->method('implementsActions') ->will($this->returnValue(false)); - $allConfig = new AllConfig(); + $allConfig = $this->getMockBuilder('\OC\AllConfig') + ->disableOriginalConstructor() + ->getMock(); + $allConfig->expects($this->any()) + ->method('getUserValue') + ->will($this->returnValue(true)); + $allConfig->expects($this->any()) + ->method('getSystemValue') + ->with($this->equalTo('datadirectory')) + ->will($this->returnValue('arbitrary/path')); $user = new \OC\User\User('foo', $backend, null, $allConfig); - $this->assertEquals(\OC_Config::getValue("datadirectory", \OC::$SERVERROOT . "/data") . '/foo', $user->getHome()); + $this->assertEquals('arbitrary/path/foo', $user->getHome()); } public function testCanChangePassword() {