From a07635678981e63aae3ce28663f6283e30000314 Mon Sep 17 00:00:00 2001
From: Christoph Wurst <christoph@winzerhof-wurst.at>
Date: Wed, 6 Dec 2023 13:52:37 +0100
Subject: [PATCH] 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>
---
 apps/dav/lib/CardDAV/CardDavBackend.php |  3 ++-
 apps/dav/lib/CardDAV/SyncService.php    | 34 ++++++++++++++++++++-----
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index c1f0fe0c93c9c..54f1bb40f034b 100644
--- a/apps/dav/lib/CardDAV/CardDavBackend.php
+++ b/apps/dav/lib/CardDAV/CardDavBackend.php
@@ -389,6 +389,7 @@ public function updateAddressBook($addressBookId, \Sabre\DAV\PropPatch $propPatc
 	 * @param array $properties
 	 * @return int
 	 * @throws BadRequest
+	 * @throws Exception
 	 */
 	public function createAddressBook($principalUri, $url, array $properties) {
 		if (strlen($url) > 255) {
@@ -433,7 +434,7 @@ public function createAddressBook($principalUri, $url, array $properties) {
 					'synctoken' => $query->createParameter('synctoken'),
 				])
 				->setParameters($values)
-				->execute();
+				->executeStatement();
 
 			$addressBookId = $query->getLastInsertId();
 			return [
diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php
index 66927d35e0421..98600778e5f15 100644
--- a/apps/dav/lib/CardDAV/SyncService.php
+++ b/apps/dav/lib/CardDAV/SyncService.php
@@ -32,6 +32,7 @@
 
 use OCP\AppFramework\Db\TTransactional;
 use OCP\AppFramework\Http;
+use OCP\DB\Exception;
 use OCP\IDBConnection;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -41,6 +42,7 @@
 use Sabre\DAV\Xml\Service;
 use Sabre\HTTP\ClientHttpException;
 use Sabre\VObject\Reader;
+use Throwable;
 use function is_null;
 
 class SyncService {
@@ -116,15 +118,33 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
 	 * @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;
+		}
 	}
 
 	/**