From ea92bded987b6c662d3f525806b42c8b19908c07 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 30 Mar 2026 17:20:01 +0200 Subject: [PATCH 1/6] fix: send deleted addressbook items in caldav sync Signed-off-by: Robin Appelman --- apps/dav/lib/CardDAV/SystemAddressbook.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/CardDAV/SystemAddressbook.php b/apps/dav/lib/CardDAV/SystemAddressbook.php index 912a2f1dcee05..636089dbab084 100644 --- a/apps/dav/lib/CardDAV/SystemAddressbook.php +++ b/apps/dav/lib/CardDAV/SystemAddressbook.php @@ -232,7 +232,8 @@ public function getChanges($syncToken, $syncLevel, $limit = null) { return $changed; } - $added = $modified = $deleted = []; + $added = $modified = []; + $deleted = array_values($changed['deleted']); foreach ($changed['added'] as $uri) { try { $this->getChild($uri); From be81390ee3cdb8129bda45dcc25d6cb1eb3572ff Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 30 Mar 2026 17:32:14 +0200 Subject: [PATCH 2/6] feat: add option to perform a full addressbook sync instead of a delta sync Signed-off-by: Robin Appelman --- apps/dav/lib/CardDAV/SyncService.php | 10 ++++++++++ .../lib/Command/SyncFederationAddressBooks.php | 7 +++++-- apps/federation/lib/SyncFederationAddressBooks.php | 4 ++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php index e6da3ed5923d1..279ef34669be6 100644 --- a/apps/dav/lib/CardDAV/SyncService.php +++ b/apps/dav/lib/CardDAV/SyncService.php @@ -66,6 +66,7 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add throw $ex; } + $received = []; // 3. apply changes // TODO: use multi-get for download foreach ($response['response'] as $resource => $status) { @@ -85,6 +86,15 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add } } + // when doing a full sync, remove any items in the local address book that aren't in the remote one + if (!$syncToken) { + $existingCards = $this->backend->getCards($addressBookId); + $removedCards = array_filter($existingCards, fn (array $card) => !in_array($card['uri'], $received)); + foreach ($removedCards as $removedCard) { + $this->backend->deleteCard($addressBookId, $removedCard['uri']); + } + } + return [ $response['token'], $response['truncated'], diff --git a/apps/federation/lib/Command/SyncFederationAddressBooks.php b/apps/federation/lib/Command/SyncFederationAddressBooks.php index 36cb99473f761..0a2de8143dd7e 100644 --- a/apps/federation/lib/Command/SyncFederationAddressBooks.php +++ b/apps/federation/lib/Command/SyncFederationAddressBooks.php @@ -11,6 +11,7 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class SyncFederationAddressBooks extends Command { @@ -23,19 +24,21 @@ public function __construct( protected function configure() { $this ->setName('federation:sync-addressbooks') - ->setDescription('Synchronizes addressbooks of all federated clouds'); + ->setDescription('Synchronizes addressbooks of all federated clouds') + ->addOption('full', null, InputOption::VALUE_NONE, 'Perform a full sync instead of a delta sync'); } protected function execute(InputInterface $input, OutputInterface $output): int { $progress = new ProgressBar($output); $progress->start(); + $full = (bool)$input->getOption('full'); $this->syncService->syncThemAll(function ($url, $ex) use ($progress, $output): void { if ($ex instanceof \Exception) { $output->writeln("Error while syncing $url : " . $ex->getMessage()); } else { $progress->advance(); } - }); + }, $full); $progress->finish(); $output->writeln(''); diff --git a/apps/federation/lib/SyncFederationAddressBooks.php b/apps/federation/lib/SyncFederationAddressBooks.php index d11f92b76efa8..e78a98835e005 100644 --- a/apps/federation/lib/SyncFederationAddressBooks.php +++ b/apps/federation/lib/SyncFederationAddressBooks.php @@ -28,7 +28,7 @@ public function __construct( /** * @param \Closure $callback */ - public function syncThemAll(\Closure $callback) { + public function syncThemAll(\Closure $callback, bool $full = false) { $trustedServers = $this->dbHandler->getAllServer(); foreach ($trustedServers as $trustedServer) { $url = $trustedServer['url']; @@ -59,7 +59,7 @@ public function syncThemAll(\Closure $callback) { $cardDavUser, $addressBookUrl, $sharedSecret, - $syncToken, + $full ? null : $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties From af0069c00e402bed58df4780b40302b7d5fa3908 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 30 Mar 2026 18:12:40 +0200 Subject: [PATCH 3/6] test: adjust tests to full address book sync Signed-off-by: Robin Appelman --- .../tests/unit/CardDAV/SyncServiceTest.php | 83 +++++++++++++++++-- 1 file changed, 78 insertions(+), 5 deletions(-) diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php index 499ba57e0428c..63cd6afbe0b4d 100644 --- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php @@ -100,7 +100,7 @@ public function testEmptySync(): void { 'system', 'system', '1234567890', - null, + '1', '1', 'principals/system/system', [] @@ -171,7 +171,7 @@ public function testSyncWithNewElement(): void { 'system', 'system', '1234567890', - null, + '1', '1', 'principals/system/system', [] @@ -242,7 +242,7 @@ public function testSyncWithUpdatedElement(): void { 'system', 'system', '1234567890', - null, + '1', '1', 'principals/system/system', [] @@ -283,7 +283,7 @@ public function testSyncWithDeletedElement(): void { 'system', 'system', '1234567890', - null, + '1', '1', 'principals/system/system', [] @@ -292,6 +292,79 @@ public function testSyncWithDeletedElement(): void { $this->assertEquals('http://sabre.io/ns/sync/4', $token); } + public function testFullSyncWithOrphanElement(): void { + $this->backend->expects($this->exactly(0)) + ->method('createCard'); + $this->backend->expects($this->exactly(1)) + ->method('updateCard'); + $this->backend->expects($this->exactly(1)) + ->method('deleteCard'); + + $body = ' + + + /remote.php/dav/addressbooks/system/system/system/Database:alice.vcf + + + text/vcard; charset=utf-8 + "2df155fa5c2a24cd7f750353fc63f037" + + HTTP/1.1 200 OK + + + http://sabre.io/ns/sync/3 +'; + + $reportResponse = new Response(new PsrResponse( + 207, + ['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)], + $body + )); + + $this->client + ->method('request') + ->willReturn($reportResponse); + + $vCard = 'BEGIN:VCARD +VERSION:3.0 +PRODID:-//Sabre//Sabre VObject 4.5.4//EN +UID:alice +FN;X-NC-SCOPE=v2-federated:alice +N;X-NC-SCOPE=v2-federated:alice;;;; +X-SOCIALPROFILE;TYPE=NEXTCLOUD;X-NC-SCOPE=v2-published:https://server2.internal/index.php/u/alice +CLOUD:alice@server2.internal +END:VCARD'; + + $getResponse = new Response(new PsrResponse( + 200, + ['Content-Type' => 'text/vcard; charset=utf-8', 'Content-Length' => strlen($vCard)], + $vCard, + )); + + $this->client + ->method('get') + ->willReturn($getResponse); + + $this->backend->method('getCards') + ->willReturn([ + ['uri' => 'Database:alice.vcf'], + ['uri' => 'Database:bob.vcf'], + ]); + + $token = $this->service->syncRemoteAddressBook( + '', + 'system', + 'system', + '1234567890', + null, + '1', + 'principals/system/system', + [] + )[0]; + + $this->assertEquals('http://sabre.io/ns/sync/3', $token); + } + public function testEnsureSystemAddressBookExists(): void { /** @var CardDavBackend | \PHPUnit\Framework\MockObject\MockObject $backend */ $backend = $this->getMockBuilder(CardDavBackend::class)->disableOriginalConstructor()->getMock(); @@ -468,7 +541,7 @@ public function testUseAbsoluteUriReport(string $host, string $expected): void { 'system', 'remote.php/dav/addressbooks/system/system/system', '1234567890', - null, + '1', '1', 'principals/system/system', [] From e87645a78aa96d8747a0191339da06db1936b569 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 7 Apr 2026 17:59:35 +0200 Subject: [PATCH 4/6] fix: fix full addressbook sync with truncated results Signed-off-by: Robin Appelman --- apps/dav/lib/CardDAV/CardDavBackend.php | 35 +++++++++++++++++++ apps/dav/lib/CardDAV/SyncService.php | 21 +++++------ .../lib/SyncFederationAddressBooks.php | 13 +++++-- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php index 0faef4bc7b8c0..db4656fcd4bd7 100644 --- a/apps/dav/lib/CardDAV/CardDavBackend.php +++ b/apps/dav/lib/CardDAV/CardDavBackend.php @@ -479,6 +479,13 @@ public function getCards($addressbookId) { ->from($this->dbCardsTable) ->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressbookId))); + return $this->getCardsFromQuery($query); + } + + /** + * @return array[] + */ + private function getCardsFromQuery(IQueryBuilder $query): array { $cards = []; $result = $query->executeQuery(); @@ -1532,4 +1539,32 @@ private function getUID(string $cardData): string { // should already be handled, but just in case throw new BadRequest('vCard can not be empty'); } + + /** + * Mark all cards in an address book as needing to be validated + * + * This is done by setting the modified date to `null`, once a sync runs + * the mtime will be set to a non-null value. Leaving all deleted items with + * a null modified date. + */ + public function markCardsAsPending(int $addressBookId): void { + $query = $this->db->getQueryBuilder(); + $query->update($this->dbCardsTable) + ->set('lastmodified', $query->createNamedParameter(null)) + ->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId))) + ->executeStatement(); + } + + /** + * @return array[] + */ + public function getPendingCards(int $addressBookId): array { + $query = $this->db->getQueryBuilder(); + $query->select(['id', 'addressbookid', 'uri', 'lastmodified', 'etag', 'size', 'carddata', 'uid']) + ->from($this->dbCardsTable) + ->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId))) + ->andWhere($query->expr()->isNull('lastmodified')); + + return $this->getCardsFromQuery($query); + } } diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php index 279ef34669be6..2384eb5569100 100644 --- a/apps/dav/lib/CardDAV/SyncService.php +++ b/apps/dav/lib/CardDAV/SyncService.php @@ -66,7 +66,6 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add throw $ex; } - $received = []; // 3. apply changes // TODO: use multi-get for download foreach ($response['response'] as $resource => $status) { @@ -86,15 +85,6 @@ public function syncRemoteAddressBook(string $url, string $userName, string $add } } - // when doing a full sync, remove any items in the local address book that aren't in the remote one - if (!$syncToken) { - $existingCards = $this->backend->getCards($addressBookId); - $removedCards = array_filter($existingCards, fn (array $card) => !in_array($card['uri'], $received)); - foreach ($removedCards as $removedCard) { - $this->backend->deleteCard($addressBookId, $removedCard['uri']); - } - } - return [ $response['token'], $response['truncated'], @@ -365,4 +355,15 @@ public function syncInstance(?\Closure $progressCallback = null) { public static function getCardUri(IUser $user): string { return $user->getBackendClassName() . ':' . $user->getUID() . '.vcf'; } + + public function markCardsAsPending(int $addressBookId): void { + $this->backend->markCardsAsPending($addressBookId); + } + + public function deletePendingCards(int $addressBookId): void { + $cards = $this->backend->getPendingCards($addressBookId); + foreach ($cards as $card) { + $this->backend->deleteCard($addressBookId, $card['uri']); + } + } } diff --git a/apps/federation/lib/SyncFederationAddressBooks.php b/apps/federation/lib/SyncFederationAddressBooks.php index e78a98835e005..b3272597110d7 100644 --- a/apps/federation/lib/SyncFederationAddressBooks.php +++ b/apps/federation/lib/SyncFederationAddressBooks.php @@ -51,7 +51,12 @@ public function syncThemAll(\Closure $callback, bool $full = false) { ]; try { - $syncToken = $oldSyncToken; + $syncToken = $full ? null : $oldSyncToken; + + $book = $this->syncService->ensureSystemAddressBookExists($targetPrincipal, $targetBookId, $targetBookProperties); + if ($full) { + $this->syncService->markCardsAsPending($book['id']); + } do { [$syncToken, $truncated] = $this->syncService->syncRemoteAddressBook( @@ -59,13 +64,17 @@ public function syncThemAll(\Closure $callback, bool $full = false) { $cardDavUser, $addressBookUrl, $sharedSecret, - $full ? null : $syncToken, + $syncToken, $targetBookId, $targetPrincipal, $targetBookProperties ); } while ($truncated); + if ($full) { + $this->syncService->deletePendingCards($book['id']); + } + if ($syncToken !== $oldSyncToken) { $this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $syncToken); } else { From e164139c571532e311a441981da34587dc7b049e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 7 Apr 2026 17:59:52 +0200 Subject: [PATCH 5/6] test: update tests to new full sync Signed-off-by: Robin Appelman --- .../tests/unit/CardDAV/SyncServiceTest.php | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php index 63cd6afbe0b4d..0aa8cf3d814e6 100644 --- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php @@ -5,6 +5,7 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace OCA\DAV\Tests\unit\CardDAV; use GuzzleHttp\Exception\ClientException; @@ -293,10 +294,26 @@ public function testSyncWithDeletedElement(): void { } public function testFullSyncWithOrphanElement(): void { + $pendingCards = []; $this->backend->expects($this->exactly(0)) ->method('createCard'); $this->backend->expects($this->exactly(1)) - ->method('updateCard'); + ->method('updateCard') + ->willReturnCallback(function ($id, $uri) use (&$pendingCards) { + unset($pendingCards[$uri]); + }); + $this->backend->expects($this->exactly(1)) + ->method('markCardsAsPending') + ->willReturnCallback(function ($id) use (&$pendingCards) { + $cards = array_values($this->backend->getCards($id)); + $uris = array_map(fn ($card) => $card['uri'], $cards); + $pendingCards = array_combine($uris, $cards); + }); + $this->backend->expects($this->exactly(1)) + ->method('getPendingCards') + ->willReturnCallback(function ($id) use (&$pendingCards) { + return array_values($pendingCards); + }); $this->backend->expects($this->exactly(1)) ->method('deleteCard'); @@ -351,6 +368,7 @@ public function testFullSyncWithOrphanElement(): void { ['uri' => 'Database:bob.vcf'], ]); + $this->service->markCardsAsPending(1); $token = $this->service->syncRemoteAddressBook( '', 'system', @@ -361,6 +379,7 @@ public function testFullSyncWithOrphanElement(): void { 'principals/system/system', [] )[0]; + $this->service->deletePendingCards(1); $this->assertEquals('http://sabre.io/ns/sync/3', $token); } From 146ebef5678bd62619aa11d4c4250bc4aeae33b9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 23 Apr 2026 17:03:48 +0200 Subject: [PATCH 6/6] fix: fix initial addressbook sync skipping items due to ordering Signed-off-by: Robin Appelman --- apps/dav/lib/CardDAV/CardDavBackend.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php index db4656fcd4bd7..ff2184d5a215c 100644 --- a/apps/dav/lib/CardDAV/CardDavBackend.php +++ b/apps/dav/lib/CardDAV/CardDavBackend.php @@ -980,7 +980,8 @@ public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel, ->from('cards') ->where( $qb->expr()->eq('addressbookid', $qb->createNamedParameter($addressBookId)) - ); + ) + ->orderBy('id'); // No synctoken supplied, this is the initial sync. $qb->setMaxResults($limit); $stmt = $qb->executeQuery();