Skip to content

Commit 6e9d48b

Browse files
hamza221hamza221
authored
Merge pull request #50092 from nextcloud/feat/sync-truncation
feat(cardav): support result truncation for addressbook federation
2 parents b514d75 + 13f25c9 commit 6e9d48b

File tree

12 files changed

+166
-51
lines changed

12 files changed

+166
-51
lines changed

‎apps/dav/appinfo/v1/carddav.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
Server::get(IUserManager::class),
6464
Server::get(IEventDispatcher::class),
6565
Server::get(\OCA\DAV\CardDAV\Sharing\Backend::class),
66+
Server::get(IConfig::class),
6667
);
6768

6869
$debugging = Server::get(IConfig::class)->getSystemValue('debug', false);

‎apps/dav/lib/CardDAV/AddressBook.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
namespace OCA\DAV\CardDAV;
99

1010
use OCA\DAV\DAV\Sharing\IShareable;
11-
use OCA\DAV\Exception\UnsupportedLimitOnInitialSyncException;
1211
use OCP\DB\Exception;
1312
use OCP\IL10N;
1413
use OCP\Server;
@@ -234,9 +233,6 @@ private function canWrite(): bool {
234233
}
235234

236235
public function getChanges($syncToken, $syncLevel, $limit = null) {
237-
if (!$syncToken && $limit) {
238-
throw new UnsupportedLimitOnInitialSyncException();
239-
}
240236

241237
return parent::getChanges($syncToken, $syncLevel, $limit);
242238
}

‎apps/dav/lib/CardDAV/CardDavBackend.php‎

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use OCP\DB\Exception;
2424
use OCP\DB\QueryBuilder\IQueryBuilder;
2525
use OCP\EventDispatcher\IEventDispatcher;
26+
use OCP\IConfig;
2627
use OCP\IDBConnection;
2728
use OCP\IUserManager;
2829
use PDO;
@@ -59,6 +60,7 @@ public function __construct(
5960
private IUserManager $userManager,
6061
private IEventDispatcher $dispatcher,
6162
private Sharing\Backend $sharingBackend,
63+
private IConfig $config,
6264
) {
6365
}
6466

@@ -851,6 +853,8 @@ public function deleteCard($addressBookId, $cardUri) {
851853
* @return array
852854
*/
853855
public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel, $limit = null) {
856+
$maxLimit = $this->config->getSystemValueInt('carddav_sync_request_truncation', 2500);
857+
$limit = ($limit === null) ? $maxLimit : min($limit, $maxLimit);
854858
// Current synctoken
855859
return $this->atomic(function () use ($addressBookId, $syncToken, $syncLevel, $limit) {
856860
$qb = $this->db->getQueryBuilder();
@@ -873,10 +877,35 @@ public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel,
873877
'modified' => [],
874878
'deleted' => [],
875879
];
876-
877-
if ($syncToken) {
880+
if (str_starts_with($syncToken, 'init_')) {
881+
$syncValues = explode('_', $syncToken);
882+
$lastID = $syncValues[1];
883+
$initialSyncToken = $syncValues[2];
878884
$qb = $this->db->getQueryBuilder();
879-
$qb->select('uri', 'operation')
885+
$qb->select('id', 'uri')
886+
->from('cards')
887+
->where(
888+
$qb->expr()->andX(
889+
$qb->expr()->eq('addressbookid', $qb->createNamedParameter($addressBookId)),
890+
$qb->expr()->gt('id', $qb->createNamedParameter($lastID)))
891+
)->orderBy('id')
892+
->setMaxResults($limit);
893+
$stmt = $qb->executeQuery();
894+
$values = $stmt->fetchAll(\PDO::FETCH_ASSOC);
895+
$stmt->closeCursor();
896+
if (count($values) === 0) {
897+
$result['syncToken'] = $initialSyncToken;
898+
$result['result_truncated'] = false;
899+
$result['added'] = [];
900+
} else {
901+
$lastID = $values[array_key_last($values)]['id'];
902+
$result['added'] = array_column($values, 'uri');
903+
$result['syncToken'] = count($result['added']) >= $limit ? "init_{$lastID}_$initialSyncToken" : $initialSyncToken ;
904+
$result['result_truncated'] = count($result['added']) >= $limit;
905+
}
906+
} elseif ($syncToken) {
907+
$qb = $this->db->getQueryBuilder();
908+
$qb->select('uri', 'operation', 'synctoken')
880909
->from('addressbookchanges')
881910
->where(
882911
$qb->expr()->andX(
@@ -886,22 +915,31 @@ public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel,
886915
)
887916
)->orderBy('synctoken');
888917

889-
if (is_int($limit) && $limit > 0) {
918+
if ($limit > 0) {
890919
$qb->setMaxResults($limit);
891920
}
892921

893922
// Fetching all changes
894923
$stmt = $qb->executeQuery();
924+
$rowCount = $stmt->rowCount();
895925

896926
$changes = [];
927+
$highestSyncToken = 0;
897928

898929
// This loop ensures that any duplicates are overwritten, only the
899930
// last change on a node is relevant.
900931
while ($row = $stmt->fetch(\PDO::FETCH_ASSOC)) {
901932
$changes[$row['uri']] = $row['operation'];
933+
$highestSyncToken = $row['synctoken'];
902934
}
935+
903936
$stmt->closeCursor();
904937

938+
// No changes found, use current token
939+
if (empty($changes)) {
940+
$result['syncToken'] = $currentToken;
941+
}
942+
905943
foreach ($changes as $uri => $operation) {
906944
switch ($operation) {
907945
case 1:
@@ -915,16 +953,43 @@ public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel,
915953
break;
916954
}
917955
}
956+
957+
/*
958+
* The synctoken in oc_addressbooks is always the highest synctoken in oc_addressbookchanges for a given addressbook plus one (see addChange).
959+
*
960+
* For truncated results, it is expected that we return the highest token from the response, so the client can continue from the latest change.
961+
*
962+
* For non-truncated results, it is expected to return the currentToken. If we return the highest token, as with truncated results, the client will always think it is one change behind.
963+
*
964+
* Therefore, we differentiate between truncated and non-truncated results when returning the synctoken.
965+
*/
966+
if ($rowCount === $limit && $highestSyncToken < $currentToken) {
967+
$result['syncToken'] = $highestSyncToken;
968+
$result['result_truncated'] = true;
969+
}
918970
} else {
919971
$qb = $this->db->getQueryBuilder();
920-
$qb->select('uri')
972+
$qb->select('id', 'uri')
921973
->from('cards')
922974
->where(
923975
$qb->expr()->eq('addressbookid', $qb->createNamedParameter($addressBookId))
924976
);
925977
// No synctoken supplied, this is the initial sync.
978+
$qb->setMaxResults($limit);
926979
$stmt = $qb->executeQuery();
927-
$result['added'] = $stmt->fetchAll(\PDO::FETCH_COLUMN);
980+
$values = $stmt->fetchAll(\PDO::FETCH_ASSOC);
981+
if (empty($values)) {
982+
$result['added'] = [];
983+
return $result;
984+
}
985+
$lastID = $values[array_key_last($values)]['id'];
986+
if (count($values) >= $limit) {
987+
$result['syncToken'] = 'init_' . $lastID . '_' . $currentToken;
988+
$result['result_truncated'] = true;
989+
}
990+
991+
$result['added'] = array_column($values, 'uri');
992+
928993
$stmt->closeCursor();
929994
}
930995
return $result;

‎apps/dav/lib/CardDAV/SyncService.php‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use Sabre\DAV\Xml\Response\MultiStatus;
2323
use Sabre\DAV\Xml\Service;
2424
use Sabre\VObject\Reader;
25+
use Sabre\Xml\ParseException;
2526
use function is_null;
2627

2728
class SyncService {
@@ -43,9 +44,10 @@ public function __construct(
4344
}
4445

4546
/**
47+
* @psalm-return list{0: ?string, 1: boolean}
4648
* @throws \Exception
4749
*/
48-
public function syncRemoteAddressBook(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken, string $targetBookHash, string $targetPrincipal, array $targetProperties): string {
50+
public function syncRemoteAddressBook(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken, string $targetBookHash, string $targetPrincipal, array $targetProperties): array {
4951
// 1. create addressbook
5052
$book = $this->ensureSystemAddressBookExists($targetPrincipal, $targetBookHash, $targetProperties);
5153
$addressBookId = $book['id'];
@@ -83,7 +85,10 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
8385
}
8486
}
8587

86-
return $response['token'];
88+
return [
89+
$response['token'],
90+
$response['truncated'],
91+
];
8792
}
8893

8994
/**
@@ -127,7 +132,7 @@ public function ensureLocalSystemAddressBookExists(): ?array {
127132

128133
private function prepareUri(string $host, string $path): string {
129134
/*
130-
* The trailing slash is important for merging the uris together.
135+
* The trailing slash is important for merging the uris.
131136
*
132137
* $host is stored in oc_trusted_servers.url and usually without a trailing slash.
133138
*
@@ -158,7 +163,9 @@ private function prepareUri(string $host, string $path): string {
158163
}
159164

160165
/**
166+
* @return array{response: array<string, array<array-key, mixed>>, token: ?string, truncated: bool}
161167
* @throws ClientExceptionInterface
168+
* @throws ParseException
162169
*/
163170
protected function requestSyncReport(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken): array {
164171
$client = $this->clientService->newClient();
@@ -181,7 +188,7 @@ protected function requestSyncReport(string $url, string $userName, string $addr
181188
$body = $response->getBody();
182189
assert(is_string($body));
183190

184-
return $this->parseMultiStatus($body);
191+
return $this->parseMultiStatus($body, $addressBookUrl);
185192
}
186193

187194
protected function download(string $url, string $userName, string $sharedSecret, string $resourcePath): string {
@@ -219,22 +226,50 @@ private function buildSyncCollectionRequestBody(?string $syncToken): string {
219226
}
220227

221228
/**
222-
* @param string $body
223-
* @return array
224-
* @throws \Sabre\Xml\ParseException
229+
* @return array{response: array<string, array<array-key, mixed>>, token: ?string, truncated: bool}
230+
* @throws ParseException
225231
*/
226-
private function parseMultiStatus($body) {
227-
$xml = new Service();
228-
232+
private function parseMultiStatus(string $body, string $addressBookUrl): array {
229233
/** @var MultiStatus $multiStatus */
230-
$multiStatus = $xml->expect('{DAV:}multistatus', $body);
234+
$multiStatus = (new Service())->expect('{DAV:}multistatus', $body);
231235

232236
$result = [];
237+
$truncated = false;
238+
233239
foreach ($multiStatus->getResponses() as $response) {
234-
$result[$response->getHref()] = $response->getResponseProperties();
240+
$href = $response->getHref();
241+
if ($response->getHttpStatus() === '507' && $this->isResponseForRequestUri($href, $addressBookUrl)) {
242+
$truncated = true;
243+
} else {
244+
$result[$response->getHref()] = $response->getResponseProperties();
245+
}
235246
}
236247

237-
return ['response' => $result, 'token' => $multiStatus->getSyncToken()];
248+
return ['response' => $result, 'token' => $multiStatus->getSyncToken(), 'truncated' => $truncated];
249+
}
250+
251+
/**
252+
* Determines whether the provided response URI corresponds to the given request URI.
253+
*/
254+
private function isResponseForRequestUri(string $responseUri, string $requestUri): bool {
255+
/*
256+
* Example response uri:
257+
*
258+
* /remote.php/dav/addressbooks/system/system/system/
259+
* /cloud/remote.php/dav/addressbooks/system/system/system/ (when installed in a subdirectory)
260+
*
261+
* Example request uri:
262+
*
263+
* remote.php/dav/addressbooks/system/system/system
264+
*
265+
* References:
266+
* https://github.com/nextcloud/3rdparty/blob/e0a509739b13820f0a62ff9cad5d0fede00e76ee/sabre/dav/lib/DAV/Sync/Plugin.php#L172-L174
267+
* https://github.com/nextcloud/server/blob/b40acb34a39592070d8455eb91c5364c07928c50/apps/federation/lib/SyncFederationAddressBooks.php#L41
268+
*/
269+
return str_ends_with(
270+
rtrim($responseUri, '/'),
271+
rtrim($requestUri, '/')
272+
);
238273
}
239274

240275
/**

‎apps/dav/lib/CardDAV/SystemAddressbook.php‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
*/
99
namespace OCA\DAV\CardDAV;
1010

11-
use OCA\DAV\Exception\UnsupportedLimitOnInitialSyncException;
1211
use OCA\Federation\TrustedServers;
1312
use OCP\Accounts\IAccountManager;
1413
use OCP\IConfig;
@@ -212,14 +211,7 @@ public function getChild($name): Card {
212211
}
213212
return new Card($this->carddavBackend, $this->addressBookInfo, $obj);
214213
}
215-
216-
/**
217-
* @throws UnsupportedLimitOnInitialSyncException
218-
*/
219214
public function getChanges($syncToken, $syncLevel, $limit = null) {
220-
if (!$syncToken && $limit) {
221-
throw new UnsupportedLimitOnInitialSyncException();
222-
}
223215

224216
if (!$this->carddavBackend instanceof SyncSupport) {
225217
return null;

‎apps/dav/lib/RootCollection.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ public function __construct() {
132132
);
133133

134134
$contactsSharingBackend = Server::get(\OCA\DAV\CardDAV\Sharing\Backend::class);
135+
$config = Server::get(IConfig::class);
135136

136137
$pluginManager = new PluginManager(\OC::$server, Server::get(IAppManager::class));
137138
$usersCardDavBackend = new CardDavBackend(
@@ -140,6 +141,7 @@ public function __construct() {
140141
$userManager,
141142
$dispatcher,
142143
$contactsSharingBackend,
144+
$config
143145
);
144146
$usersAddressBookRoot = new AddressBookRoot($userPrincipalBackend, $usersCardDavBackend, $pluginManager, $userSession->getUser(), $groupManager, 'principals/users');
145147
$usersAddressBookRoot->disableListing = $disableListing;
@@ -150,6 +152,7 @@ public function __construct() {
150152
$userManager,
151153
$dispatcher,
152154
$contactsSharingBackend,
155+
$config
153156
);
154157
$systemAddressBookRoot = new AddressBookRoot(new SystemPrincipalBackend(), $systemCardDavBackend, $pluginManager, $userSession->getUser(), $groupManager, 'principals/system');
155158
$systemAddressBookRoot->disableListing = $disableListing;

0 commit comments

Comments
 (0)