Skip to content

Commit 379fa54

Browse files
icewind1991icewind1991
authored
Merge pull request #59312 from nextcloud/calender-sync-delete
Fix removed address book items not being synced between federated instances
2 parents c4cdb9b + df76c9f commit 379fa54

6 files changed

Lines changed: 163 additions & 11 deletions

File tree

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

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,13 @@ public function getCards($addressbookId) {
478478
->from($this->dbCardsTable)
479479
->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressbookId)));
480480

481+
return $this->getCardsFromQuery($query);
482+
}
483+
484+
/**
485+
* @return array[]
486+
*/
487+
private function getCardsFromQuery(IQueryBuilder $query): array {
481488
$cards = [];
482489

483490
$result = $query->executeQuery();
@@ -972,7 +979,8 @@ public function getChangesForAddressBook($addressBookId, $syncToken, $syncLevel,
972979
->from('cards')
973980
->where(
974981
$qb->expr()->eq('addressbookid', $qb->createNamedParameter($addressBookId))
975-
);
982+
)
983+
->orderBy('id');
976984
// No synctoken supplied, this is the initial sync.
977985
$qb->setMaxResults($limit);
978986
$stmt = $qb->executeQuery();
@@ -1532,4 +1540,32 @@ private function getUID(string $cardData): string {
15321540
// should already be handled, but just in case
15331541
throw new BadRequest('vCard can not be empty');
15341542
}
1543+
1544+
/**
1545+
* Mark all cards in an address book as needing to be validated
1546+
*
1547+
* This is done by setting the modified date to `null`, once a sync runs
1548+
* the mtime will be set to a non-null value. Leaving all deleted items with
1549+
* a null modified date.
1550+
*/
1551+
public function markCardsAsPending(int $addressBookId): void {
1552+
$query = $this->db->getTypedQueryBuilder();
1553+
$query->update($this->dbCardsTable)
1554+
->set('lastmodified', $query->createNamedParameter(null))
1555+
->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
1556+
->executeStatement();
1557+
}
1558+
1559+
/**
1560+
* @return array[]
1561+
*/
1562+
public function getPendingCards(int $addressBookId): array {
1563+
$query = $this->db->getQueryBuilder();
1564+
$query->select(['id', 'addressbookid', 'uri', 'lastmodified', 'etag', 'size', 'carddata', 'uid'])
1565+
->from($this->dbCardsTable)
1566+
->where($query->expr()->eq('addressbookid', $query->createNamedParameter($addressBookId)))
1567+
->andWhere($query->expr()->isNull('lastmodified'));
1568+
1569+
return $this->getCardsFromQuery($query);
1570+
}
15351571
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,4 +217,15 @@ public function syncInstance(?\Closure $progressCallback = null) {
217217
public static function getCardUri(IUser $user): string {
218218
return $user->getBackendClassName() . ':' . $user->getUID() . '.vcf';
219219
}
220+
221+
public function markCardsAsPending(int $addressBookId): void {
222+
$this->backend->markCardsAsPending($addressBookId);
223+
}
224+
225+
public function deletePendingCards(int $addressBookId): void {
226+
$cards = $this->backend->getPendingCards($addressBookId);
227+
foreach ($cards as $card) {
228+
$this->backend->deleteCard($addressBookId, $card['uri']);
229+
}
230+
}
220231
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,8 @@ public function getChanges($syncToken, $syncLevel, $limit = null) {
232232
return $changed;
233233
}
234234

235-
$added = $modified = $deleted = [];
235+
$added = $modified = [];
236+
$deleted = array_values($changed['deleted']);
236237
foreach ($changed['added'] as $uri) {
237238
try {
238239
$this->getChild($uri);

‎apps/dav/tests/unit/CardDAV/SyncServiceTest.php‎

Lines changed: 97 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
66
* SPDX-License-Identifier: AGPL-3.0-only
77
*/
8+
89
namespace OCA\DAV\Tests\unit\CardDAV;
910

1011
use GuzzleHttp\Exception\ClientException;
@@ -104,7 +105,7 @@ public function testEmptySync(): void {
104105
'system',
105106
'system',
106107
'1234567890',
107-
null,
108+
'1',
108109
'1',
109110
'principals/system/system',
110111
[]
@@ -175,7 +176,7 @@ public function testSyncWithNewElement(): void {
175176
'system',
176177
'system',
177178
'1234567890',
178-
null,
179+
'1',
179180
'1',
180181
'principals/system/system',
181182
[]
@@ -246,7 +247,7 @@ public function testSyncWithUpdatedElement(): void {
246247
'system',
247248
'system',
248249
'1234567890',
249-
null,
250+
'1',
250251
'1',
251252
'principals/system/system',
252253
[]
@@ -287,7 +288,7 @@ public function testSyncWithDeletedElement(): void {
287288
'system',
288289
'system',
289290
'1234567890',
290-
null,
291+
'1',
291292
'1',
292293
'principals/system/system',
293294
[]
@@ -296,6 +297,97 @@ public function testSyncWithDeletedElement(): void {
296297
$this->assertEquals('http://sabre.io/ns/sync/4', $token);
297298
}
298299

300+
public function testFullSyncWithOrphanElement(): void {
301+
$pendingCards = [];
302+
$this->backend->expects($this->exactly(0))
303+
->method('createCard');
304+
$this->backend->expects($this->exactly(1))
305+
->method('updateCard')
306+
->willReturnCallback(function ($id, $uri) use (&$pendingCards) {
307+
unset($pendingCards[$uri]);
308+
});
309+
$this->backend->expects($this->exactly(1))
310+
->method('markCardsAsPending')
311+
->willReturnCallback(function ($id) use (&$pendingCards) {
312+
$cards = array_values($this->backend->getCards($id));
313+
$uris = array_map(fn ($card) => $card['uri'], $cards);
314+
$pendingCards = array_combine($uris, $cards);
315+
});
316+
$this->backend->expects($this->exactly(1))
317+
->method('getPendingCards')
318+
->willReturnCallback(function ($id) use (&$pendingCards) {
319+
return array_values($pendingCards);
320+
});
321+
$this->backend->expects($this->exactly(1))
322+
->method('deleteCard');
323+
324+
$body = '<?xml version="1.0"?>
325+
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:card="urn:ietf:params:xml:ns:carddav" xmlns:oc="http://owncloud.org/ns">
326+
<d:response>
327+
<d:href>/remote.php/dav/addressbooks/system/system/system/Database:alice.vcf</d:href>
328+
<d:propstat>
329+
<d:prop>
330+
<d:getcontenttype>text/vcard; charset=utf-8</d:getcontenttype>
331+
<d:getetag>&quot;2df155fa5c2a24cd7f750353fc63f037&quot;</d:getetag>
332+
</d:prop>
333+
<d:status>HTTP/1.1 200 OK</d:status>
334+
</d:propstat>
335+
</d:response>
336+
<d:sync-token>http://sabre.io/ns/sync/3</d:sync-token>
337+
</d:multistatus>';
338+
339+
$reportResponse = new Response(new PsrResponse(
340+
207,
341+
['Content-Type' => 'application/xml; charset=utf-8', 'Content-Length' => strlen($body)],
342+
$body
343+
));
344+
345+
$this->client
346+
->method('request')
347+
->willReturn($reportResponse);
348+
349+
$vCard = 'BEGIN:VCARD
350+
VERSION:3.0
351+
PRODID:-//Sabre//Sabre VObject 4.5.4//EN
352+
UID:alice
353+
FN;X-NC-SCOPE=v2-federated:alice
354+
N;X-NC-SCOPE=v2-federated:alice;;;;
355+
X-SOCIALPROFILE;TYPE=NEXTCLOUD;X-NC-SCOPE=v2-published:https://server2.internal/index.php/u/alice
356+
CLOUD:alice@server2.internal
357+
END:VCARD';
358+
359+
$getResponse = new Response(new PsrResponse(
360+
200,
361+
['Content-Type' => 'text/vcard; charset=utf-8', 'Content-Length' => strlen($vCard)],
362+
$vCard,
363+
));
364+
365+
$this->client
366+
->method('get')
367+
->willReturn($getResponse);
368+
369+
$this->backend->method('getCards')
370+
->willReturn([
371+
['uri' => 'Database:alice.vcf'],
372+
['uri' => 'Database:bob.vcf'],
373+
]);
374+
375+
$this->service->markCardsAsPending(1);
376+
$token = $this->service->syncRemoteAddressBook(
377+
'',
378+
'system',
379+
'system',
380+
'1234567890',
381+
null,
382+
'1',
383+
'principals/system/system',
384+
[]
385+
)[0];
386+
$this->service->deletePendingCards(1);
387+
388+
$this->assertEquals('http://sabre.io/ns/sync/3', $token);
389+
}
390+
299391
public function testEnsureSystemAddressBookExists(): void {
300392
/** @var CardDavBackend&MockObject $backend */
301393
$backend = $this->createMock(CardDavBackend::class);
@@ -496,7 +588,7 @@ public function testUseAbsoluteUriReport(string $host, string $expected): void {
496588
'system',
497589
'remote.php/dav/addressbooks/system/system/system',
498590
'1234567890',
499-
null,
591+
'1',
500592
'1',
501593
'principals/system/system',
502594
[]

‎apps/federation/lib/Command/SyncFederationAddressBooks.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use Symfony\Component\Console\Command\Command;
1414
use Symfony\Component\Console\Helper\ProgressBar;
1515
use Symfony\Component\Console\Input\InputInterface;
16+
use Symfony\Component\Console\Input\InputOption;
1617
use Symfony\Component\Console\Output\OutputInterface;
1718

1819
class SyncFederationAddressBooks extends Command {
@@ -25,19 +26,21 @@ public function __construct(
2526
protected function configure() {
2627
$this
2728
->setName('federation:sync-addressbooks')
28-
->setDescription('Synchronizes addressbooks of all federated clouds');
29+
->setDescription('Synchronizes addressbooks of all federated clouds')
30+
->addOption('full', null, InputOption::VALUE_NONE, 'Perform a full sync instead of a delta sync');
2931
}
3032

3133
protected function execute(InputInterface $input, OutputInterface $output): int {
3234
$progress = new ProgressBar($output);
3335
$progress->start();
36+
$full = (bool)$input->getOption('full');
3437
$this->syncService->syncThemAll(function ($url, $ex) use ($progress, $output): void {
3538
if ($ex instanceof \Exception) {
3639
$output->writeln("Error while syncing $url : " . $ex->getMessage());
3740
} else {
3841
$progress->advance();
3942
}
40-
});
43+
}, $full);
4144

4245
$progress->finish();
4346
$output->writeln('');

‎apps/federation/lib/SyncFederationAddressBooks.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public function __construct(
2424
/**
2525
* @param \Closure $callback
2626
*/
27-
public function syncThemAll(\Closure $callback) {
27+
public function syncThemAll(\Closure $callback, bool $full = false) {
2828
$trustedServers = $this->dbHandler->getAllServer();
2929
foreach ($trustedServers as $trustedServer) {
3030
$url = $trustedServer['url'];
@@ -47,7 +47,12 @@ public function syncThemAll(\Closure $callback) {
4747
];
4848

4949
try {
50-
$syncToken = $oldSyncToken;
50+
$syncToken = $full ? null : $oldSyncToken;
51+
52+
$book = $this->syncService->ensureSystemAddressBookExists($targetPrincipal, $targetBookId, $targetBookProperties);
53+
if ($full) {
54+
$this->syncService->markCardsAsPending($book['id']);
55+
}
5156

5257
do {
5358
[$syncToken, $truncated] = $this->syncService->syncRemoteAddressBook(
@@ -62,6 +67,10 @@ public function syncThemAll(\Closure $callback) {
6267
);
6368
} while ($truncated);
6469

70+
if ($full) {
71+
$this->syncService->deletePendingCards($book['id']);
72+
}
73+
6574
if ($syncToken !== $oldSyncToken) {
6675
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $syncToken);
6776
} else {

0 commit comments

Comments
 (0)