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

6 files changed, 109 insertions, 22 deletions

diff --git a/lib/private/allconfig.php b/lib/private/allconfig.php
index 7ebff7cf2db..295fb8b7668 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 58043b30440..9c75baf887d 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 a08014fa6fa..5a1e955bdd2 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;
@@ -441,6 +447,15 @@ class Server extends SimpleContainer implements IServerContainer {
 	}
 
 	/**
+	 * For internal use only
+	 *
+	 * @return \OC\SystemConfig
+	 */
+	function getSystemConfig() {
+		return $this->query('SystemConfig');
+	}
+
+	/**
 	 * Returns the app config manager
 	 *
 	 * @return \OCP\IAppConfig
diff --git a/lib/private/systemconfig.php b/lib/private/systemconfig.php
new file mode 100644
index 00000000000..ce6883e5ab3
--- /dev/null
+++ b/lib/private/systemconfig.php
@@ -0,0 +1,49 @@
+<?php
+/**
+ * Copyright (c) 2014 Morris Jobke <hey@morrisjobke.de>
+ *               2013 Bart Visscher <bartv@thisnet.nl>
+ *
+ * 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 65dde39cdce..57c430251b5 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 6aa7243a75a..85ade9ccaf1 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() {