Skip to content

Commit 16329c1

Browse files
authored
Merge pull request #63766 from nextcloud/fix/carddav/userUpdate
fix(cardDav): Only update user card on actual mapped propreties changes
2 parents 3e4bf74 + 85952bc commit 16329c1

2 files changed

Lines changed: 45 additions & 1 deletion

File tree

apps/dav/lib/CardDAV/SyncService.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,12 @@ public function updateUser(IUser $user): void {
152152
if (is_null($vCard)) {
153153
$this->backend->deleteCard($addressBookId, $cardId);
154154
} else {
155-
$this->backend->updateCard($addressBookId, $cardId, $vCard->serialize());
155+
$cardData = $vCard->serialize();
156+
// Writing an identical card would still bump the address book
157+
// sync token and make every client re-download the card
158+
if ($card['carddata'] !== $cardData) {
159+
$this->backend->updateCard($addressBookId, $cardId, $cardData);
160+
}
156161
}
157162
}
158163
}, $this->dbConnection);

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,45 @@ public function testUpdateAndDeleteUser(bool $activated, int $createCalls, int $
457457
$ss->deleteUser($user);
458458
}
459459

460+
public function testUpdateUserSkipsUnchangedCard(): void {
461+
$vCard = new VCard();
462+
$vCard->VERSION = '3.0';
463+
$vCard->UID = 'test-user';
464+
$vCard->FN = 'test-user';
465+
466+
/** @var CardDavBackend&MockObject $backend */
467+
$backend = $this->getMockBuilder(CardDavBackend::class)->disableOriginalConstructor()->getMock();
468+
$logger = $this->createMock(LoggerInterface::class);
469+
470+
$backend->expects($this->never())->method('createCard');
471+
$backend->expects($this->never())->method('updateCard');
472+
$backend->expects($this->never())->method('deleteCard');
473+
474+
$backend->method('getCard')->willReturn(['carddata' => $vCard->serialize()]);
475+
$backend->method('getAddressBooksByUri')
476+
->with('principals/system/system', 'system')
477+
->willReturn(['id' => -1]);
478+
479+
$user = $this->createMock(IUser::class);
480+
$user->method('getBackendClassName')->willReturn('unittest');
481+
$user->method('getUID')->willReturn('test-user');
482+
$user->method('isEnabled')->willReturn(true);
483+
484+
$converter = $this->createMock(Converter::class);
485+
$converter->method('createCardFromUser')->willReturn($vCard);
486+
487+
$ss = new SyncService(
488+
$this->createMock(IClientService::class),
489+
$this->createMock(IConfig::class),
490+
$backend,
491+
$this->createMock(IUserManager::class),
492+
$this->createMock(IDBConnection::class),
493+
$logger,
494+
$converter,
495+
);
496+
$ss->updateUser($user);
497+
}
498+
460499
public function testSyncInstance(): void {
461500
/** @var CardDavBackend | MockObject $backend */
462501
$backend = $this->getMockBuilder(CardDavBackend::class)->disableOriginalConstructor()->getMock();

0 commit comments

Comments
 (0)