diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index dbcdacc27f0..49bcf78c28b 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -56,6 +56,8 @@ $cardDavBackend = new CardDavBackend( \OC::$server->getUserManager(), \OC::$server->get(IEventDispatcher::class), \OC::$server->get(\OCA\DAV\CardDAV\Sharing\Backend::class), + \OC::$server->get(OCP\IConfig::class), + ); $debugging = \OC::$server->getConfig()->getSystemValue('debug', false); diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php index eaaa569be1b..7701ea6edbd 100644 --- a/apps/dav/lib/CardDAV/CardDavBackend.php +++ b/apps/dav/lib/CardDAV/CardDavBackend.php @@ -920,18 +920,20 @@ class CardDavBackend implements BackendInterface, SyncSupport { // 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; @@ -950,6 +952,20 @@ class CardDavBackend implements BackendInterface, SyncSupport { 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') diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php index dcb12d15bb7..46a32b1a0d8 100644 --- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php +++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php @@ -56,6 +56,9 @@ class CardDavBackendTest extends TestCase { /** @var IGroupManager|\PHPUnit\Framework\MockObject\MockObject */ private $groupManager; + /** @var IConfig|MockObject */ + private $config; + /** @var IEventDispatcher|MockObject */ private $dispatcher; private Backend $sharingBackend; @@ -239,7 +242,7 @@ class CardDavBackendTest extends TestCase { public function testCardOperations(): void { /** @var CardDavBackend | \PHPUnit\Framework\MockObject\MockObject $backend */ $backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config]) ->onlyMethods(['updateProperties', 'purgeProperties'])->getMock(); // create a new address book @@ -294,7 +297,7 @@ class CardDavBackendTest extends TestCase { public function testMultiCard(): void { $this->backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config]) ->setMethods(['updateProperties'])->getMock(); // create a new address book @@ -347,7 +350,7 @@ class CardDavBackendTest extends TestCase { public function testMultipleUIDOnDifferentAddressbooks(): void { $this->backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config]) ->onlyMethods(['updateProperties'])->getMock(); // create 2 new address books @@ -369,7 +372,7 @@ class CardDavBackendTest extends TestCase { public function testMultipleUIDDenied(): void { $this->backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config]) ->setMethods(['updateProperties'])->getMock(); // create a new address book @@ -390,7 +393,7 @@ class CardDavBackendTest extends TestCase { public function testNoValidUID(): void { $this->backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config]) ->setMethods(['updateProperties'])->getMock(); // create a new address book @@ -447,7 +450,7 @@ class CardDavBackendTest extends TestCase { public function testSyncSupport(): void { $this->backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher, $this->sharingBackend, $this->config]) ->setMethods(['updateProperties'])->getMock(); // create a new address book diff --git a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php index fc27a654c00..499ba57e042 100644 --- a/apps/dav/tests/unit/CardDAV/SyncServiceTest.php +++ b/apps/dav/tests/unit/CardDAV/SyncServiceTest.php @@ -104,7 +104,7 @@ class SyncServiceTest extends TestCase { '1', 'principals/system/system', [] - )['token']; + )[0]; $this->assertEquals('http://sabre.io/ns/sync/1', $token); } @@ -175,7 +175,7 @@ END:VCARD'; '1', 'principals/system/system', [] - )['token']; + )[0]; $this->assertEquals('http://sabre.io/ns/sync/2', $token); } @@ -246,7 +246,7 @@ END:VCARD'; '1', 'principals/system/system', [] - )['token']; + )[0]; $this->assertEquals('http://sabre.io/ns/sync/3', $token); } @@ -287,7 +287,7 @@ END:VCARD'; '1', 'principals/system/system', [] - )['token']; + )[0]; $this->assertEquals('http://sabre.io/ns/sync/4', $token); } @@ -430,7 +430,7 @@ END:VCARD'; '1', 'principals/system/system', [] - )['token']; + ); } /** @@ -472,7 +472,7 @@ END:VCARD'; '1', 'principals/system/system', [] - )['token']; + ); } public function providerUseAbsoluteUriReport(): array { diff --git a/apps/federation/tests/SyncFederationAddressbooksTest.php b/apps/federation/tests/SyncFederationAddressbooksTest.php index 39274a11ade..2c069881eca 100644 --- a/apps/federation/tests/SyncFederationAddressbooksTest.php +++ b/apps/federation/tests/SyncFederationAddressbooksTest.php @@ -55,7 +55,7 @@ class SyncFederationAddressbooksTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $syncService->expects($this->once())->method('syncRemoteAddressBook') - ->willReturn('1'); + ->willReturn(['1', false]); /** @var SyncService $syncService */ $s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger); @@ -114,7 +114,7 @@ class SyncFederationAddressbooksTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); $syncService->expects($this->once())->method('syncRemoteAddressBook') - ->willReturn('0'); + ->willReturn(['0', false]); /** @var SyncService $syncService */ $s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger); diff --git a/config/config.sample.php b/config/config.sample.php index 6ab227410d8..06aa5033bb8 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -346,6 +346,12 @@ $CONFIG = [ */ 'carddav_sync_request_timeout' => 30, +/** + * The limit applied to the synchronization report request, e.g. federated system address books (as run by `occ federation:sync-addressbooks`). + */ +'carddav_sync_request_truncation' => 2500, + + /** * `true` enabled a relaxed session timeout, where the session timeout would no longer be * handled by Nextcloud but by either the PHP garbage collection or the expiration of