Merge pull request #10985 from owncloud/db-cleanup

Cleanup database handling

core/command/maintenance/repair.php

@@ -35,10 +35,6 @@ class Repair extends Command {
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
-		// TODO: inject DB connection/factory when possible
-		$connection = \OC_DB::getConnection();
-		$connection->disableQueryStatementCaching();
-
 		$maintenanceMode = $this->config->getValue('maintenance', false);
 		$this->config->setValue('maintenance', true);
 

lib/private/appframework/db/db.php

@@ -30,27 +30,41 @@ use \OCP\IDb;
  * Small Facade for being able to inject the database connection for tests
  */
 class Db implements IDb {
+	/**
+	 * @var \OCP\IDBConnection
+	 */
+	protected $connection;
 
+	/**
+	 * @param \OCP\IDBConnection $connection
+	 */
+	public function __construct($connection) {
+		$this->connection = $connection;
+	}
 
 	/**
 	 * Used to abstract the owncloud database access away
+	 *
 	 * @param string $sql the sql query with ? placeholder for params
 	 * @param int $limit the maximum number of rows
 	 * @param int $offset from which row we want to start
 	 * @return \OC_DB_StatementWrapper prepared SQL query
 	 */
-    public function prepareQuery($sql, $limit=null, $offset=null){
+	public function prepareQuery($sql, $limit = null, $offset = null) {
-        return \OCP\DB::prepare($sql, $limit, $offset);
+		$isManipulation = \OC_DB::isManipulation($sql);
+		$statement = $this->connection->prepare($sql, $limit, $offset);
+		return new \OC_DB_StatementWrapper($statement, $isManipulation);
 	}
 
 
 	/**
 	 * Used to get the id of the just inserted element
+	 *
 	 * @param string $tableName the name of the table where we inserted the item
 	 * @return int the id of the inserted element
 	 */
-    public function getInsertId($tableName){
+	public function getInsertId($tableName) {
-        return \OCP\DB::insertid($tableName);
+		return $this->connection->lastInsertId($tableName);
 	}
 
 

lib/private/db.php

@@ -41,87 +41,12 @@ class DatabaseException extends Exception {
  * Doctrine with some adaptions.
  */
 class OC_DB {
-	/**
-	 * @var \OC\DB\Connection $connection
-	 */
-	static private $connection; //the preferred connection to use, only Doctrine
 
 	/**
-	 * connects to the database
+	 * @return \OCP\IDBConnection
-	 * @return boolean|null true if connection can be established or false on error
-	 *
-	 * Connects to the database as specified in config.php
-	 */
-	public static function connect() {
-		if(self::$connection) {
-			return true;
-		}
-
-		$type = OC_Config::getValue('dbtype', 'sqlite');
-		$factory = new \OC\DB\ConnectionFactory();
-		if (!$factory->isValidType($type)) {
-			return false;
-		}
-
-		$connectionParams = array(
-			'user' => OC_Config::getValue('dbuser', ''),
-			'password' => OC_Config::getValue('dbpassword', ''),
-		);
-		$name = OC_Config::getValue('dbname', 'owncloud');
-
-		if ($factory->normalizeType($type) === 'sqlite3') {
-			$datadir = OC_Config::getValue("datadirectory", OC::$SERVERROOT.'/data');
-			$connectionParams['path'] = $datadir.'/'.$name.'.db';
-		} else {
-			$host = OC_Config::getValue('dbhost', '');
-			if (strpos($host, ':')) {
-				// Host variable may carry a port or socket.
-				list($host, $portOrSocket) = explode(':', $host, 2);
-				if (ctype_digit($portOrSocket)) {
-					$connectionParams['port'] = $portOrSocket;
-				} else {
-					$connectionParams['unix_socket'] = $portOrSocket;
-				}
-			}
-			$connectionParams['host'] = $host;
-			$connectionParams['dbname'] = $name;
-		}
-
-		$connectionParams['tablePrefix'] = OC_Config::getValue('dbtableprefix', 'oc_');
-
-		try {
-			self::$connection = $factory->getConnection($type, $connectionParams);
-			self::$connection->getConfiguration()->setSQLLogger(\OC::$server->getQueryLogger());
-		} catch(\Doctrine\DBAL\DBALException $e) {
-			OC_Log::write('core', $e->getMessage(), OC_Log::FATAL);
-			OC_User::setUserId(null);
-
-			// send http status 503
-			header('HTTP/1.1 503 Service Temporarily Unavailable');
-			header('Status: 503 Service Temporarily Unavailable');
-			OC_Template::printErrorPage('Failed to connect to database');
-			die();
-		}
-
-		return true;
-	}
-
-	/**
-	 * The existing database connection is closed and connected again
-	 */
-	public static function reconnect() {
-		if(self::$connection) {
-			self::$connection->close();
-			self::$connection->connect();
-		}
-	}
-
-	/**
-	 * @return \OC\DB\Connection
 	 */
 	static public function getConnection() {
-		self::connect();
+		return \OC::$server->getDatabaseConnection();
-		return self::$connection;
 	}
 
 	/**
@@ -131,7 +56,7 @@ class OC_DB {
 	 */
 	private static function getMDB2SchemaManager()
 	{
-		return new \OC\DB\MDB2SchemaManager(self::getConnection());
+		return new \OC\DB\MDB2SchemaManager(\OC::$server->getDatabaseConnection());
 	}
 
 	/**
@@ -146,7 +71,7 @@ class OC_DB {
 	 * SQL query via Doctrine prepare(), needs to be execute()'d!
 	 */
 	static public function prepare( $query , $limit = null, $offset = null, $isManipulation = null) {
-		self::connect();
+		$connection = \OC::$server->getDatabaseConnection();
 
 		if ($isManipulation === null) {
 			//try to guess, so we return the number of rows on manipulations
@@ -155,7 +80,7 @@ class OC_DB {
 
 		// return the result
 		try {
-			$result = self::$connection->prepare($query, $limit, $offset);
+			$result =$connection->prepare($query, $limit, $offset);
 		} catch (\Doctrine\DBAL\DBALException $e) {
 			throw new \DatabaseException($e->getMessage(), $query);
 		}
@@ -252,8 +177,7 @@ class OC_DB {
 	 * cause trouble!
 	 */
 	public static function insertid($table=null) {
-		self::connect();
+		return \OC::$server->getDatabaseConnection()->lastInsertId($table);
-		return self::$connection->lastInsertId($table);
 	}
 
 	/**
@@ -263,24 +187,21 @@ class OC_DB {
 	 * @return boolean number of updated rows
 	 */
 	public static function insertIfNotExist($table, $input) {
-		self::connect();
+		return \OC::$server->getDatabaseConnection()->insertIfNotExist($table, $input);
-		return self::$connection->insertIfNotExist($table, $input);
 	}
 
 	/**
 	 * Start a transaction
 	 */
 	public static function beginTransaction() {
-		self::connect();
+		return \OC::$server->getDatabaseConnection()->beginTransaction();
-		self::$connection->beginTransaction();
 	}
 
 	/**
 	 * Commit the database changes done during a transaction that is in progress
 	 */
 	public static function commit() {
-		self::connect();
+		return \OC::$server->getDatabaseConnection()->commit();
-		self::$connection->commit();
 	}
 
 	/**
@@ -348,17 +269,17 @@ class OC_DB {
 	 * @param string $tableName the table to drop
 	 */
 	public static function dropTable($tableName) {
-
+		$connection = \OC::$server->getDatabaseConnection();
 		$tableName = OC_Config::getValue('dbtableprefix', 'oc_' ) . trim($tableName);
 
-		self::$connection->beginTransaction();
+		$connection->beginTransaction();
 
-		$platform = self::$connection->getDatabasePlatform();
+		$platform = $connection->getDatabasePlatform();
 		$sql = $platform->getDropTableSQL($platform->quoteIdentifier($tableName));
 
-		self::$connection->query($sql);
+		$connection->executeQuery($sql);
 
-		self::$connection->commit();
+		$connection->commit();
 	}
 
 	/**
@@ -398,8 +319,8 @@ class OC_DB {
 	}
 
 	public static function getErrorCode($error) {
-		$code = self::$connection->errorCode();
+		$connection = \OC::$server->getDatabaseConnection();
-		return $code;
+		return $connection->errorCode();
 	}
 	/**
 	 * returns the error code and message as a string for logging
@@ -408,22 +329,8 @@ class OC_DB {
 	 * @return string
 	 */
 	public static function getErrorMessage($error) {
-		if (self::$connection) {
+		$connection = \OC::$server->getDatabaseConnection();
-			return self::$connection->getError();
+		return $connection->getError();
-		}
-		return '';
-	}
-
-	/**
-	 * @param bool $enabled
-	 */
-	static public function enableCaching($enabled) {
-		self::connect();
-		if ($enabled) {
-			self::$connection->enableQueryStatementCaching();
-		} else {
-			self::$connection->disableQueryStatementCaching();
-		}
 	}
 
 	/**

lib/private/db/connection.php

@@ -11,8 +11,9 @@ use Doctrine\DBAL\Driver;
 use Doctrine\DBAL\Configuration;
 use Doctrine\DBAL\Cache\QueryCacheProfile;
 use Doctrine\Common\EventManager;
+use OCP\IDBConnection;
 
-class Connection extends \Doctrine\DBAL\Connection {
+class Connection extends \Doctrine\DBAL\Connection implements IDBConnection {
 	/**
 	 * @var string $tablePrefix
 	 */
@@ -23,13 +24,6 @@ class Connection extends \Doctrine\DBAL\Connection {
 	 */
 	protected $adapter;
 
-	/**
-	 * @var \Doctrine\DBAL\Driver\Statement[] $preparedQueries
-	 */
-	protected $preparedQueries = array();
-
-	protected $cachingQueryStatementEnabled = true;
-
 	/**
 	 * Initializes a new instance of the Connection class.
 	 *
@@ -69,9 +63,6 @@ class Connection extends \Doctrine\DBAL\Connection {
 			$platform = $this->getDatabasePlatform();
 			$statement = $platform->modifyLimitQuery($statement, $limit, $offset);
 		} else {
-			if (isset($this->preparedQueries[$statement]) && $this->cachingQueryStatementEnabled) {
-				return $this->preparedQueries[$statement];
-			}
 			$origStatement = $statement;
 		}
 		$statement = $this->replaceTablePrefix($statement);
@@ -80,11 +71,7 @@ class Connection extends \Doctrine\DBAL\Connection {
 		if(\OC_Config::getValue( 'log_query', false)) {
 			\OC_Log::write('core', 'DB prepare : '.$statement, \OC_Log::DEBUG);
 		}
-		$result = parent::prepare($statement);
+		return parent::prepare($statement);
-		if (is_null($limit) && $this->cachingQueryStatementEnabled) {
-			$this->preparedQueries[$origStatement] = $result;
-		}
-		return $result;
 	}
 
 	/**
@@ -185,13 +172,4 @@ class Connection extends \Doctrine\DBAL\Connection {
 	protected function replaceTablePrefix($statement) {
 		return str_replace( '*PREFIX*', $this->tablePrefix, $statement );
 	}
-
-	public function enableQueryStatementCaching() {
-		$this->cachingQueryStatementEnabled = true;
-	}
-
-	public function disableQueryStatementCaching() {
-		$this->cachingQueryStatementEnabled = false;
-		$this->preparedQueries = array();
-	}
 }

lib/private/db/connectionfactory.php

@@ -98,19 +98,6 @@ class ConnectionFactory {
 			new \Doctrine\DBAL\Configuration(),
 			$eventManager
 		);
-		switch ($normalizedType) {
-			case 'sqlite3':
-				// Sqlite doesn't handle query caching and schema changes
-				// TODO: find a better way to handle this
-				/** @var $connection \OC\DB\Connection */
-				$connection->disableQueryStatementCaching();
-				break;
-			case 'oci':
-				// oracle seems to have issues with cached statements which have been closed
-				/** @var $connection \OC\DB\Connection */
-				$connection->disableQueryStatementCaching();
-				break;
-		}
 		return $connection;
 	}
 
@@ -131,4 +118,41 @@ class ConnectionFactory {
 		$normalizedType = $this->normalizeType($type);
 		return isset($this->defaultConnectionParams[$normalizedType]);
 	}
+
+	/**
+	 * Create the connection parameters for the config
+	 *
+	 * @param \OCP\IConfig $config
+	 * @return array
+	 */
+	public function createConnectionParams($config) {
+		$type = $config->getSystemValue('dbtype', 'sqlite');
+
+		$connectionParams = array(
+			'user' => $config->getSystemValue('dbuser', ''),
+			'password' => $config->getSystemValue('dbpassword', ''),
+		);
+		$name = $config->getSystemValue('dbname', 'owncloud');
+
+		if ($this->normalizeType($type) === 'sqlite3') {
+			$datadir = $config->getSystemValue("datadirectory", \OC::$SERVERROOT . '/data');
+			$connectionParams['path'] = $datadir . '/' . $name . '.db';
+		} else {
+			$host = $config->getSystemValue('dbhost', '');
+			if (strpos($host, ':')) {
+				// Host variable may carry a port or socket.
+				list($host, $portOrSocket) = explode(':', $host, 2);
+				if (ctype_digit($portOrSocket)) {
+					$connectionParams['port'] = $portOrSocket;
+				} else {
+					$connectionParams['unix_socket'] = $portOrSocket;
+				}
+			}
+			$connectionParams['host'] = $host;
+			$connectionParams['dbname'] = $name;
+		}
+
+		$connectionParams['tablePrefix'] = $config->getSystemValue('dbtableprefix', 'oc_');
+		return $connectionParams;
+	}
 }

lib/private/db/connectionwrapper.php

@@ -1,99 +0,0 @@
-<?php
-/**
- * Copyright (c) 2013 Thomas Müller <deepdiver@owncloud.com>
- * This file is licensed under the Affero General Public License version 3 or
- * later.
- * See the COPYING-README file.
- */
-
-namespace OC\DB;
-
-
-class ConnectionWrapper implements \OCP\IDBConnection {
-
-	private $connection;
-
-	public function __construct(Connection $conn) {
-		$this->connection = $conn;
-	}
-
-	/**
-	 * Used to the owncloud database access away
-	 * @param string $sql the sql query with ? placeholder for params
-	 * @param int $limit the maximum number of rows
-	 * @param int $offset from which row we want to start
-	 * @return \Doctrine\DBAL\Driver\Statement The prepared statement.
-	 */
-	public function prepare($sql, $limit = null, $offset = null)
-	{
-		return $this->connection->prepare($sql, $limit, $offset);
-	}
-
-	/**
-	 * Used to get the id of the just inserted element
-	 * @param string $table the name of the table where we inserted the item
-	 * @return string the id of the inserted element
-	 */
-	public function lastInsertId($table = null)
-	{
-		return $this->connection->lastInsertId($table);
-	}
-
-	/**
-	 * Insert a row if a matching row doesn't exists.
-	 * @param string $table The table name (will replace *PREFIX*) to perform the replace on.
-	 * @param array $input
-	 *
-	 * The input array if in the form:
-	 *
-	 * array ( 'id' => array ( 'value' => 6,
-	 *    'key' => true
-	 *    ),
-	 *    'name' => array ('value' => 'Stoyan'),
-	 *    'family' => array ('value' => 'Stefanov'),
-	 *    'birth_date' => array ('value' => '1975-06-20')
-	 *    );
-	 * @return bool
-	 *
-	 */
-	public function insertIfNotExist($table, $input)
-	{
-		return $this->connection->insertIfNotExist($table, $input);
-	}
-
-	/**
-	 * Start a transaction
-	 * @return bool TRUE on success or FALSE on failure
-	 */
-	public function beginTransaction()
-	{
-		return $this->connection->beginTransaction();
-	}
-
-	/**
-	 * Commit the database changes done during a transaction that is in progress
-	 * @return bool TRUE on success or FALSE on failure
-	 */
-	public function commit()
-	{
-		return $this->connection->commit();
-	}
-
-	/**
-	 * Rollback the database changes done during a transaction that is in progress
-	 * @return bool TRUE on success or FALSE on failure
-	 */
-	public function rollBack()
-	{
-		return $this->connection->rollBack();
-	}
-
-	/**
-	 * Gets the error code and message as a string for logging
-	 * @return string
-	 */
-	public function getError()
-	{
-		return $this->connection->getError();
-	}
-}

lib/private/db/mdb2schemamanager.php

@@ -21,7 +21,7 @@ class MDB2SchemaManager {
 	protected $conn;
 
 	/**
-	 * @param \OC\DB\Connection $conn
+	 * @param \OCP\IDBConnection $conn
 	 */
 	public function __construct($conn) {
 		$this->conn = $conn;
@@ -155,7 +155,8 @@ class MDB2SchemaManager {
 		$this->conn->commit();
 
 		if ($this->conn->getDatabasePlatform() instanceof SqlitePlatform) {
-			\OC_DB::reconnect();
+			$this->conn->close();
+			$this->conn->connect();
 		}
 		return true;
 	}

lib/private/preferences.php

@@ -36,7 +36,7 @@
 
 namespace OC;
 
-use \OC\DB\Connection;
+use OCP\IDBConnection;
 
 
 /**
@@ -61,9 +61,9 @@ class Preferences {
 	protected $cache = array();
 
 	/**
-	 * @param \OC\DB\Connection $conn
+	 * @param \OCP\IDBConnection $conn
 	 */
-	public function __construct(Connection $conn) {
+	public function __construct(IDBConnection $conn) {
 		$this->conn = $conn;
 	}
 

lib/private/server.php

@@ -217,8 +217,25 @@ class Server extends SimpleContainer implements IServerContainer {
 		$this->registerService('Crypto', function ($c) {
 			return new Crypto(\OC::$server->getConfig(), \OC::$server->getSecureRandom());
 		});
+		$this->registerService('DatabaseConnection', function ($c) {
+			/**
+			 * @var Server $c
+			 */
+			$factory = new \OC\DB\ConnectionFactory();
+			$type = $c->getConfig()->getSystemValue('dbtype', 'sqlite');
+			if (!$factory->isValidType($type)) {
+				throw new \DatabaseException('Invalid database type');
+			}
+			$connectionParams = $factory->createConnectionParams($c->getConfig());
+			$connection = $factory->getConnection($type, $connectionParams);
+			$connection->getConfiguration()->setSQLLogger($c->getQueryLogger());
+			return $connection;
+		});
 		$this->registerService('Db', function ($c) {
-			return new Db();
+			/**
+			 * @var Server $c
+			 */
+			return new Db($c->getDatabaseConnection());
 		});
 		$this->registerService('HTTPHelper', function (SimpleContainer $c) {
 			$config = $c->query('AllConfig');
@@ -469,7 +486,7 @@ class Server extends SimpleContainer implements IServerContainer {
 	 * @return \OCP\IDBConnection
 	 */
 	function getDatabaseConnection() {
-		return new ConnectionWrapper(\OC_DB::getConnection());
+		return $this->query('DatabaseConnection');
 	}
 
 	/**

lib/private/updater.php

@@ -129,7 +129,6 @@ class Updater extends BasicEmitter {
 	 * @return bool true if the operation succeeded, false otherwise
 	 */
 	public function upgrade() {
-		\OC_DB::enableCaching(false);
 		\OC_Config::setValue('maintenance', true);
 
 		$installedVersion = \OC_Config::getValue('version', '0.0.0');

lib/public/idbconnection.php

@@ -43,6 +43,32 @@ interface IDBConnection {
 	 */
 	public function prepare($sql, $limit=null, $offset=null);
 
+	/**
+	 * Executes an, optionally parameterized, SQL query.
+	 *
+	 * If the query is parameterized, a prepared statement is used.
+	 * If an SQLLogger is configured, the execution is logged.
+	 *
+	 * @param string $query The SQL query to execute.
+	 * @param string[] $params The parameters to bind to the query, if any.
+	 * @param array $types The types the previous parameters are in.
+	 * @return \Doctrine\DBAL\Driver\Statement The executed statement.
+	 */
+	public function executeQuery($query, array $params = array(), $types = array());
+
+	/**
+	 * Executes an SQL INSERT/UPDATE/DELETE query with the given parameters
+	 * and returns the number of affected rows.
+	 *
+	 * This method supports PDO binding types as well as DBAL mapping types.
+	 *
+	 * @param string $query The SQL query.
+	 * @param array $params The query parameters.
+	 * @param array $types The parameter types.
+	 * @return integer The number of affected rows.
+	 */
+	public function executeUpdate($query, array $params = array(), array $types = array());
+
 	/**
 	 * Used to get the id of the just inserted element
 	 * @param string $table the name of the table where we inserted the item
@@ -71,19 +97,16 @@ interface IDBConnection {
 
 	/**
 	 * Start a transaction
-	 * @return bool TRUE on success or FALSE on failure
 	 */
 	public function beginTransaction();
 
 	/**
 	 * Commit the database changes done during a transaction that is in progress
-	 * @return bool TRUE on success or FALSE on failure
 	 */
 	public function commit();
 
 	/**
 	 * Rollback the database changes done during a transaction that is in progress
-	 * @return bool TRUE on success or FALSE on failure
 	 */
 	public function rollBack();
 
@@ -92,4 +115,47 @@ interface IDBConnection {
 	 * @return string
 	 */
 	public function getError();
+
+	/**
+	 * Fetch the SQLSTATE associated with the last database operation.
+	 *
+	 * @return integer The last error code.
+	 */
+	public function errorCode();
+
+	/**
+	 * Fetch extended error information associated with the last database operation.
+	 *
+	 * @return array The last error information.
+	 */
+	public function errorInfo();
+
+	/**
+	 * Establishes the connection with the database.
+	 *
+	 * @return bool
+	 */
+	public function connect();
+
+	/**
+	 * Close the database connection
+	 */
+	public function close();
+
+	/**
+	 * Quotes a given input parameter.
+	 *
+	 * @param mixed $input Parameter to be quoted.
+	 * @param int $type Type of the parameter.
+	 * @return string The quoted parameter.
+	 */
+	public function quote($input, $type = \PDO::PARAM_STR);
+
+	/**
+	 * Gets the DatabasePlatform instance that provides all the metadata about
+	 * the platform this driver connects to.
+	 *
+	 * @return \Doctrine\DBAL\Platforms\AbstractPlatform The database platform.
+	 */
+	public function getDatabasePlatform();
 }

tests/lib/tags.php

@@ -34,7 +34,7 @@ class Test_Tags extends PHPUnit_Framework_TestCase {
 		$this->objectType = uniqid('type_');
 		OC_User::createUser($this->user, 'pass');
 		OC_User::setUserId($this->user);
-		$this->tagMapper = new OC\Tagging\TagMapper(new OC\AppFramework\Db\Db());
+		$this->tagMapper = new OC\Tagging\TagMapper(\OC::$server->getDb());
 		$this->tagMgr = new OC\TagManager($this->tagMapper, $this->user);
 
 	}