Skip to content

Commit 0bb68ab

Browse files
ChristophWurstskjnldsv
authored andcommitted
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 <[email protected]>
1 parent 452e4be commit 0bb68ab

File tree

2 files changed

+27
-7
lines changed

2 files changed

+27
-7
lines changed

apps/dav/lib/CardDAV/CardDavBackend.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ public function updateAddressBook($addressBookId, \Sabre\DAV\PropPatch $propPatc
351351
* @param array $properties
352352
* @return int
353353
* @throws BadRequest
354+
* @throws Exception
354355
*/
355356
public function createAddressBook($principalUri, $url, array $properties) {
356357
if (strlen($url) > 255) {

apps/dav/lib/CardDAV/SyncService.php

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use OCP\AppFramework\Db\TTransactional;
1212
use OCP\AppFramework\Http;
13+
use OCP\DB\Exception;
1314
use OCP\Http\Client\IClient;
1415
use OCP\Http\Client\IClientService;
1516
use OCP\IConfig;
@@ -89,15 +90,33 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
8990
* @throws \Sabre\DAV\Exception\BadRequest
9091
*/
9192
public function ensureSystemAddressBookExists(string $principal, string $uri, array $properties): ?array {
92-
return $this->atomic(function () use ($principal, $uri, $properties) {
93-
$book = $this->backend->getAddressBooksByUri($principal, $uri);
94-
if (!is_null($book)) {
95-
return $book;
93+
try {
94+
return $this->atomic(function () use ($principal, $uri, $properties) {
95+
$book = $this->backend->getAddressBooksByUri($principal, $uri);
96+
if (!is_null($book)) {
97+
return $book;
98+
}
99+
$this->backend->createAddressBook($principal, $uri, $properties);
100+
101+
return $this->backend->getAddressBooksByUri($principal, $uri);
102+
}, $this->dbConnection);
103+
} catch (Exception $e) {
104+
// READ COMMITTED doesn't prevent a nonrepeatable read above, so
105+
// two processes might create an address book here. Ignore our
106+
// failure and continue loading the entry written by the other process
107+
if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
108+
throw $e;
96109
}
97-
$this->backend->createAddressBook($principal, $uri, $properties);
98110

99-
return $this->backend->getAddressBooksByUri($principal, $uri);
100-
}, $this->dbConnection);
111+
// If this fails we might have hit a replication node that does not
112+
// have the row written in the other process.
113+
// TODO: find an elegant way to handle this
114+
$ab = $this->backend->getAddressBooksByUri($principal, $uri);
115+
if ($ab === null) {
116+
throw new Exception('Could not create system address book', $e->getCode(), $e);
117+
}
118+
return $ab;
119+
}
101120
}
102121

103122
private function prepareUri(string $host, string $path): string {

0 commit comments

Comments
 (0)