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>

2 files changed, 27 insertions, 7 deletions

diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php
index f7d0a888efd..b15ed607076 100644
--- 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 4e8c4b1355e..cc3d324faf1 100644
--- 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 {