From 230e93f5756dca187f77c8df3cf69e3d7e3d05ff Mon Sep 17 00:00:00 2001 From: Morris Jobke <hey@morrisjobke.de> Date: Fri, 9 Nov 2018 10:35:12 +0100 Subject: Catch UniqueConstraintViolationException inside insertIfNotExist This is the most common case for the usage of this method. See also https://github.com/nextcloud/server/issues/12369 and the linked tickets. Signed-off-by: Morris Jobke <hey@morrisjobke.de> --- lib/private/DB/Adapter.php | 16 ++++++++++++++-- lib/private/DB/AdapterSqlite.php | 16 ++++++++++++++-- lib/private/DB/Connection.php | 4 +++- lib/public/IDBConnection.php | 4 +++- 4 files changed, 34 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/private/DB/Adapter.php b/lib/private/DB/Adapter.php index e88e3cf09c6..8f062030604 100644 --- a/lib/private/DB/Adapter.php +++ b/lib/private/DB/Adapter.php @@ -27,6 +27,8 @@ namespace OC\DB; +use Doctrine\DBAL\Exception\UniqueConstraintViolationException; + /** * This handles the way we use to write queries, into something that can be * handled by the database abstraction layer. @@ -79,7 +81,9 @@ class Adapter { } /** - * Insert a row if the matching row does not exists. + * Insert a row if the matching row does not exists. To accomplish proper race condition avoidance + * it is needed that there is also a unique constraint on the values. Then this method will + * catch the exception and return 0. * * @param string $table The table name (will replace *PREFIX* with the actual prefix) * @param array $input data that should be inserted into the table (column name => value) @@ -111,6 +115,14 @@ class Adapter { $query = substr($query, 0, -5); $query .= ' HAVING COUNT(*) = 0'; - return $this->conn->executeUpdate($query, $inserts); + try { + return $this->conn->executeUpdate($query, $inserts); + } catch (UniqueConstraintViolationException $e) { + // if this is thrown then a concurrent insert happened between the insert and the sub-select in the insert, that should have avoided it + // it's fine to ignore this then + // + // more discussions about this can be found at https://github.com/nextcloud/server/pull/12315 + return 0; + } } } diff --git a/lib/private/DB/AdapterSqlite.php b/lib/private/DB/AdapterSqlite.php index dbeec9ded43..24650829aa3 100644 --- a/lib/private/DB/AdapterSqlite.php +++ b/lib/private/DB/AdapterSqlite.php @@ -27,6 +27,8 @@ namespace OC\DB; +use Doctrine\DBAL\Exception\UniqueConstraintViolationException; + class AdapterSqlite extends Adapter { /** @@ -50,7 +52,9 @@ class AdapterSqlite extends Adapter { } /** - * Insert a row if the matching row does not exists. + * Insert a row if the matching row does not exists. To accomplish proper race condition avoidance + * it is needed that there is also a unique constraint on the values. Then this method will + * catch the exception and return 0. * * @param string $table The table name (will replace *PREFIX* with the actual prefix) * @param array $input data that should be inserted into the table (column name => value) @@ -82,6 +86,14 @@ class AdapterSqlite extends Adapter { $query = substr($query, 0, -5); $query .= ')'; - return $this->conn->executeUpdate($query, $inserts); + try { + return $this->conn->executeUpdate($query, $inserts); + } catch (UniqueConstraintViolationException $e) { + // if this is thrown then a concurrent insert happened between the insert and the sub-select in the insert, that should have avoided it + // it's fine to ignore this then + // + // more discussions about this can be found at https://github.com/nextcloud/server/pull/12315 + return 0; + } } } diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php index 8bb959ffbc1..6a64925711a 100644 --- a/lib/private/DB/Connection.php +++ b/lib/private/DB/Connection.php @@ -240,7 +240,9 @@ class Connection extends ReconnectWrapper implements IDBConnection { } /** - * Insert a row if the matching row does not exists. + * Insert a row if the matching row does not exists. To accomplish proper race condition avoidance + * it is needed that there is also a unique constraint on the values. Then this method will + * catch the exception and return 0. * * @param string $table The table name (will replace *PREFIX* with the actual prefix) * @param array $input data that should be inserted into the table (column name => value) diff --git a/lib/public/IDBConnection.php b/lib/public/IDBConnection.php index 4e450eae736..204d7bc99ed 100644 --- a/lib/public/IDBConnection.php +++ b/lib/public/IDBConnection.php @@ -104,7 +104,9 @@ interface IDBConnection { public function lastInsertId($table = null); /** - * Insert a row if the matching row does not exists. + * Insert a row if the matching row does not exists. To accomplish proper race condition avoidance + * it is needed that there is also a unique constraint on the values. Then this method will + * catch the exception and return 0. * * @param string $table The table name (will replace *PREFIX* with the actual prefix) * @param array $input data that should be inserted into the table (column name => value) -- cgit v1.2.3