Skip to content

Feat/sync truncation3 #53976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kesselb merged 2 commits into from
Jul 17, 2025
Merged

Feat/sync truncation3 #53976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0ffc60
feat(carddav): handle truncated non-initial requests
kesselb Jun 17, 2025
1989027
fix(carddav): return correct sync token for non-truncated requests
kesselb Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions apps/dav/lib/CardDAV/CardDavBackend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,18 +920,20 @@ public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel,

// Fetching all changes
$stmt = $qb->executeQuery();
$rowCount = $stmt->rowCount();

$changes = [];
$highestSyncToken = 0;

// This loop ensures that any duplicates are overwritten, only the
// last change on a node is relevant.
while ($row = $stmt->fetch(\PDO::FETCH_ASSOC)) {
$changes[$row['uri']] = $row['operation'];
// get the last synctoken, needed in case a limit was set
$result['syncToken'] = $row['synctoken'];
$highestSyncToken = $row['synctoken'];
}

$stmt->closeCursor();

// No changes found, use current token
if (empty($changes)) {
$result['syncToken'] = $currentToken;
Expand All @@ -950,6 +952,20 @@ public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel,
break;
}
}

/*
* The synctoken in oc_addressbooks is always the highest synctoken in oc_addressbookchanges for a given addressbook plus one (see addChange).
*
* For truncated results, it is expected that we return the highest token from the response, so the client can continue from the latest change.
*
* 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.
*
* Therefore, we differentiate between truncated and non-truncated results when returning the synctoken.
*/
if ($rowCount === $limit && $highestSyncToken < $currentToken) {
$result['syncToken'] = $highestSyncToken;
$result['result_truncated'] = true;
}
} else {
$qb = $this->db->getQueryBuilder();
$qb->select('id', 'uri')
Expand Down
61 changes: 48 additions & 13 deletions apps/dav/lib/CardDAV/SyncService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Sabre\DAV\Xml\Response\MultiStatus;
use Sabre\DAV\Xml\Service;
use Sabre\VObject\Reader;
use Sabre\Xml\ParseException;
use function is_null;

class SyncService {
Expand All @@ -43,9 +44,10 @@ public function __construct(
}

/**
* @psalm-return list{0: ?string, 1: boolean}
* @throws \Exception
*/
public function syncRemoteAddressBook(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken, string $targetBookHash, string $targetPrincipal, array $targetProperties): string {
public function syncRemoteAddressBook(string $url, string $userName, string $addressBookUrl, string $sharedSecret, ?string $syncToken, string $targetBookHash, string $targetPrincipal, array $targetProperties): array {
// 1. create addressbook
$book = $this->ensureSystemAddressBookExists($targetPrincipal, $targetBookHash, $targetProperties);
$addressBookId = $book['id'];
Expand Down Expand Up @@ -83,7 +85,10 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add
}
}

return $response['token'];
return [
$response['token'],
$response['truncated'],
];
}

/**
Expand Down Expand Up @@ -127,7 +132,7 @@ public function ensureLocalSystemAddressBookExists(): ?array {

private function prepareUri(string $host, string $path): string {
/*
* The trailing slash is important for merging the uris together.
* The trailing slash is important for merging the uris.
*
* $host is stored in oc_trusted_servers.url and usually without a trailing slash.
*
Expand Down Expand Up @@ -158,7 +163,9 @@ private function prepareUri(string $host, string $path): string {
}

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

return $this->parseMultiStatus($body);
return $this->parseMultiStatus($body, $addressBookUrl);
}

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

/**
* @param string $body
* @return array
* @throws \Sabre\Xml\ParseException
* @return array{response: array<string, array<array-key, mixed>>, token: ?string, truncated: bool}
* @throws ParseException
*/
private function parseMultiStatus($body) {
$xml = new Service();

private function parseMultiStatus(string $body, string $addressBookUrl): array {
/** @var MultiStatus $multiStatus */
$multiStatus = $xml->expect('{DAV:}multistatus', $body);
$multiStatus = (new Service())->expect('{DAV:}multistatus', $body);

$result = [];
$truncated = false;

foreach ($multiStatus->getResponses() as $response) {
$result[$response->getHref()] = $response->getResponseProperties();
$href = $response->getHref();
if ($response->getHttpStatus() === '507' && $this->isResponseForRequestUri($href, $addressBookUrl)) {
$truncated = true;
} else {
$result[$response->getHref()] = $response->getResponseProperties();
}
}

return ['response' => $result, 'token' => $multiStatus->getSyncToken()];
return ['response' => $result, 'token' => $multiStatus->getSyncToken(), 'truncated' => $truncated];
}

/**
* Determines whether the provided response URI corresponds to the given request URI.
*/
private function isResponseForRequestUri(string $responseUri, string $requestUri): bool {
/*
* Example response uri:
*
* /remote.php/dav/addressbooks/system/system/system/
* /cloud/remote.php/dav/addressbooks/system/system/system/ (when installed in a subdirectory)
*
* Example request uri:
*
* remote.php/dav/addressbooks/system/system/system
*
* References:
* https://github.com/nextcloud/3rdparty/blob/e0a509739b13820f0a62ff9cad5d0fede00e76ee/sabre/dav/lib/DAV/Sync/Plugin.php#L172-L174
* https://github.com/nextcloud/server/blob/b40acb34a39592070d8455eb91c5364c07928c50/apps/federation/lib/SyncFederationAddressBooks.php#L41
*/
return str_ends_with(
rtrim($responseUri, '/'),
rtrim($requestUri, '/')
);
}

/**
Expand Down
29 changes: 19 additions & 10 deletions apps/federation/lib/SyncFederationAddressBooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function syncThemAll(\Closure $callback) {
$url = $trustedServer['url'];
$callback($url, null);
$sharedSecret = $trustedServer['shared_secret'];
$syncToken = $trustedServer['sync_token'];
$oldSyncToken = $trustedServer['sync_token'];

$endPoints = $this->ocsDiscoveryService->discover($url, 'FEDERATED_SHARING');
$cardDavUser = $endPoints['carddav-user'] ?? 'system';
Expand All @@ -49,16 +49,25 @@ public function syncThemAll(\Closure $callback) {
$targetBookProperties = [
'{DAV:}displayname' => $url
];

try {
$newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
if ($newToken !== $syncToken) {
// Finish truncated initial sync.
if (strpos($newToken, 'init') !== false) {
do {
$newToken = $this->syncService->syncRemoteAddressBook($url, $cardDavUser, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties);
} while (str_contains($newToken, 'init_'));
}
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $newToken);
$syncToken = $oldSyncToken;

do {
[$syncToken, $truncated] = $this->syncService->syncRemoteAddressBook(
$url,
$cardDavUser,
$addressBookUrl,
$sharedSecret,
$syncToken,
$targetBookId,
$targetPrincipal,
$targetBookProperties
);
} while ($truncated);

if ($syncToken !== $oldSyncToken) {
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $syncToken);
} else {
$this->logger->debug("Sync Token for $url unchanged from previous sync");
// The server status might have been changed to a failure status in previous runs.
Expand Down
Loading