fix(carddav): return correct sync token for non-truncated requests

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

Author: kesselb

apps/dav/lib/CardDAV/CardDavBackend.php

@@ -921,18 +921,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;
@@ -951,6 +953,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')

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

@@ -108,7 +108,7 @@ public function testEmptySync(): void {
 			'1',
 			'principals/system/system',
 			[]
-		)['token'];
+		)[0];
 
 		$this->assertEquals('http://sabre.io/ns/sync/1', $token);
 	}
@@ -179,7 +179,7 @@ public function testSyncWithNewElement(): void {
 			'1',
 			'principals/system/system',
 			[]
-		)['token'];
+		)[0];
 
 		$this->assertEquals('http://sabre.io/ns/sync/2', $token);
 	}
@@ -250,7 +250,7 @@ public function testSyncWithUpdatedElement(): void {
 			'1',
 			'principals/system/system',
 			[]
-		)['token'];
+		)[0];
 
 		$this->assertEquals('http://sabre.io/ns/sync/3', $token);
 	}
@@ -291,7 +291,7 @@ public function testSyncWithDeletedElement(): void {
 			'1',
 			'principals/system/system',
 			[]
-		)['token'];
+		)[0];
 
 		$this->assertEquals('http://sabre.io/ns/sync/4', $token);
 	}
@@ -422,7 +422,7 @@ public function testDeleteAddressbookWhenAccessRevoked(): void {
 			'1',
 			'principals/system/system',
 			[]
-		)['token'];
+		);
 	}
 
 	#[\PHPUnit\Framework\Attributes\DataProvider('providerUseAbsoluteUriReport')]
@@ -462,7 +462,7 @@ public function testUseAbsoluteUriReport(string $host, string $expected): void {
 			'1',
 			'principals/system/system',
 			[]
-		)['token'];
+		);
 	}
 
 	public static function providerUseAbsoluteUriReport(): array {

apps/federation/tests/SyncFederationAddressbooksTest.php

@@ -45,7 +45,7 @@ public function testSync(): void {
 			->with('https://cloud.example.org', 1, '1');
 		$syncService = $this->createMock(SyncService::class);
 		$syncService->expects($this->once())->method('syncRemoteAddressBook')
-			->willReturn('1');
+			->willReturn(['1', false]);
 
 		/** @var SyncService $syncService */
 		$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger);
@@ -96,7 +96,7 @@ public function testSuccessfulSyncWithoutChangesAfterFailure(): void {
 			->with('https://cloud.example.org', 1);
 		$syncService = $this->createMock(SyncService::class);
 		$syncService->expects($this->once())->method('syncRemoteAddressBook')
-			->willReturn('0');
+			->willReturn(['0', false]);
 
 		/** @var SyncService $syncService */
 		$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger);

tests/lib/Avatar/UserAvatarTest.php

@@ -27,7 +27,7 @@ class UserAvatarTest extends \Test\TestCase {
 	private SimpleFolder&MockObject $folder;
 	private IConfig&MockObject $config;
 	private User&MockObject $user;
-	
+
 	protected function setUp(): void {
 		parent::setUp();