Merge pull request #14734 from owncloud/unit-test-insertIfNotExist

[sqlite] Use an atomic implementation on sqlite for insertIfNotExist() + use it in the file cache ...

3 files changed, 35 insertions, 51 deletions

diff --git a/lib/private/db/adaptersqlite.php b/lib/private/db/adaptersqlite.php
index c5dfa85aaac..df4a804feb1 100644
--- a/lib/private/db/adaptersqlite.php
+++ b/lib/private/db/adaptersqlite.php
@@ -19,11 +19,13 @@ class AdapterSqlite extends Adapter {
 	}
 
 	public function insertIfNotExist($table, $input) {
-		// NOTE: For SQLite we have to use this clumsy approach
-		// otherwise all fieldnames used must have a unique key.
-		$query = 'SELECT COUNT(*) FROM `' . $table . '` WHERE ';
-		$inserts = array();
-		foreach ($input as $key => $value) {
+		$fieldList = '`' . implode('`,`', array_keys($input)) . '`';
+		$query = "INSERT INTO `$table` ($fieldList) SELECT "
+			. str_repeat('?,', count($input)-1).'? '
+			. " WHERE NOT EXISTS (SELECT 1 FROM `$table` WHERE ";
+
+		$inserts = array_values($input);
+		foreach($input as $key => $value) {
 			$query .= '`' . $key . '`';
 			if (is_null($value)) {
 				$query .= ' IS NULL AND ';
@@ -33,34 +35,10 @@ class AdapterSqlite extends Adapter {
 			}
 		}
 		$query = substr($query, 0, strlen($query) - 5);
+		$query .= ')';
 
 		try {
-			$stmt = $this->conn->prepare($query);
-			$result = $stmt->execute($inserts);
-		} catch(\Doctrine\DBAL\DBALException $e) {
-			$entry = 'DB Error: "'.$e->getMessage() . '"<br />';
-			$entry .= 'Offending command was: ' . $query . '<br />';
-			\OC_Log::write('core', $entry, \OC_Log::FATAL);
-			$l = \OC::$server->getL10N('lib');
-			throw new \OC\HintException(
-				$l->t('Database Error'),
-				$l->t('Please contact your system administrator.'),
-				0,
-				$e
-			);
-		}
-
-		if ($stmt->fetchColumn() === '0') {
-			$query = 'INSERT INTO `' . $table . '` (`'
-				. implode('`,`', array_keys($input)) . '`) VALUES('
-				. str_repeat('?,', count($input)-1).'? ' . ')';
-		} else {
-			return 0; //no rows updated
-		}
-
-		try {
-			$statement = $this->conn->prepare($query);
-			$result = $statement->execute(array_values($input));
+			return $this->conn->executeUpdate($query, $inserts);
 		} catch(\Doctrine\DBAL\DBALException $e) {
 			$entry = 'DB Error: "'.$e->getMessage() . '"<br />';
 			$entry .= 'Offending command was: ' . $query.'<br />';
@@ -73,7 +51,5 @@ class AdapterSqlite extends Adapter {
 				$e
 			);
 		}
-
-		return $result;
 	}
 }
diff --git a/lib/private/db/connection.php b/lib/private/db/connection.php
index 53935c1e1ed..6ba29fc2ccf 100644
--- a/lib/private/db/connection.php
+++ b/lib/private/db/connection.php
@@ -79,8 +79,6 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection {
 		if (!is_null($limit)) {
 			$platform = $this->getDatabasePlatform();
 			$statement = $platform->modifyLimitQuery($statement, $limit, $offset);
-		} else {
-			$origStatement = $statement;
 		}
 		$statement = $this->replaceTablePrefix($statement);
 		$statement = $this->adapter->fixupStatement($statement);
@@ -92,17 +90,19 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection {
 	}
 
 	/**
-	 * Executes an, optionally parameterized, SQL query.
+	 * Executes an, optionally parametrized, SQL query.
 	 *
-	 * If the query is parameterized, a prepared statement is used.
+	 * If the query is parametrized, 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.
-	 * @param QueryCacheProfile $qcp
+	 * @param string                                      $query  The SQL query to execute.
+	 * @param array                                       $params The parameters to bind to the query, if any.
+	 * @param array                                       $types  The types the previous parameters are in.
+	 * @param \Doctrine\DBAL\Cache\QueryCacheProfile|null $qcp    The query cache profile, optional.
+	 *
 	 * @return \Doctrine\DBAL\Driver\Statement The executed statement.
-	 * @internal PERF: Directly prepares a driver statement, not a wrapper.
+	 *
+	 * @throws \Doctrine\DBAL\DBALException
 	 */
 	public function executeQuery($query, array $params = array(), $types = array(), QueryCacheProfile $qcp = null)
 	{
@@ -117,11 +117,13 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection {
 	 *
 	 * 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.
+	 * @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.
-	 * @internal PERF: Directly prepares a driver statement, not a wrapper.
+	 *
+	 * @throws \Doctrine\DBAL\DBALException
 	 */
 	public function executeUpdate($query, array $params = array(), array $types = array())
 	{
diff --git a/lib/private/files/cache/cache.php b/lib/private/files/cache/cache.php
index 1f30382e101..62c32ce6593 100644
--- a/lib/private/files/cache/cache.php
+++ b/lib/private/files/cache/cache.php
@@ -243,13 +243,19 @@ class Cache {
 			list($queryParts, $params) = $this->buildParts($data);
 			$queryParts[] = '`storage`';
 			$params[] = $this->getNumericStorageId();
-			$valuesPlaceholder = array_fill(0, count($queryParts), '?');
 
-			$sql = 'INSERT INTO `*PREFIX*filecache` (' . implode(', ', $queryParts) . ')'
-				. ' VALUES (' . implode(', ', $valuesPlaceholder) . ')';
-			\OC_DB::executeAudited($sql, $params);
+			$params = array_map(function($item) {
+				return trim($item, "`");
+			}, $params);
+			$queryParts = array_map(function($item) {
+				return trim($item, "`");
+			}, $queryParts);
+			$values = array_combine($queryParts, $params);
+			if (\OC::$server->getDatabaseConnection()->insertIfNotExist('*PREFIX*filecache', $values)) {
+				return (int)\OC_DB::insertid('*PREFIX*filecache');
+			}
 
-			return (int)\OC_DB::insertid('*PREFIX*filecache');
+			return $this->getId($file);
 		}
 	}