fix(carddav): Handle race for SAB creation better

* Accept non repeatable read and INSERT conflict by another read
* Handle replication edge case

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index f7d0a888efd79d35292e45a22b816edb2de44881..b15ed60707685cd7fc41616d6909be49f96ff727 100644 (file)
--- a/apps/dav/lib/CardDAV/CardDavBackend.php
+++ b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -351,6 +351,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
         * @param array $properties
         * @return int
         * @throws BadRequest
+        * @throws Exception
         */
        public function createAddressBook($principalUri, $url, array $properties) {
                if (strlen($url) > 255) {

diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php
index 4e8c4b1355e9a887a54e013902ee1c83b1e889d7..cc3d324faf1ffc95ac50ea82347f11038dd4a29d 100644 (file)
--- a/apps/dav/lib/CardDAV/SyncService.php
+++ b/apps/dav/lib/CardDAV/SyncService.php
@@ -10,6 +10,7 @@ namespace OCA\DAV\CardDAV;
 
 use OCP\AppFramework\Db\TTransactional;
 use OCP\AppFramework\Http;
+use OCP\DB\Exception;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\IConfig;
@@ -89,15 +90,33 @@ class SyncService {
         * @throws \Sabre\DAV\Exception\BadRequest
         */
        public function ensureSystemAddressBookExists(string $principal, string $uri, array $properties): ?array {
-               return $this->atomic(function () use ($principal, $uri, $properties) {
-                       $book = $this->backend->getAddressBooksByUri($principal, $uri);
-                       if (!is_null($book)) {
-                               return $book;
+               try {
+                       return $this->atomic(function () use ($principal, $uri, $properties) {
+                               $book = $this->backend->getAddressBooksByUri($principal, $uri);
+                               if (!is_null($book)) {
+                                       return $book;
+                               }
+                               $this->backend->createAddressBook($principal, $uri, $properties);
+
+                               return $this->backend->getAddressBooksByUri($principal, $uri);
+                       }, $this->dbConnection);
+               } catch (Exception $e) {
+                       // READ COMMITTED doesn't prevent a nonrepeatable read above, so
+                       // two processes might create an address book here. Ignore our
+                       // failure and continue loading the entry written by the other process
+                       if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
+                               throw $e;
                        }
-                       $this->backend->createAddressBook($principal, $uri, $properties);
 
-                       return $this->backend->getAddressBooksByUri($principal, $uri);
-               }, $this->dbConnection);
+                       // If this fails we might have hit a replication node that does not
+                       // have the row written in the other process.
+                       // TODO: find an elegant way to handle this
+                       $ab = $this->backend->getAddressBooksByUri($principal, $uri);
+                       if ($ab === null) {
+                               throw new Exception('Could not create system address book', $e->getCode(), $e);
+                       }
+                       return $ab;
+               }
        }
 
        private function prepareUri(string $host, string $path): string {