From d83934b99ef8bf68cb22473b365bd8fc92e3ccb1 Mon Sep 17 00:00:00 2001 From: SebastianKrupinski Date: Mon, 26 Oct 2026 12:38:20 -0400 Subject: [PATCH] fix(federation): persist address book sync token after each page Signed-off-by: SebastianKrupinski --- apps/federation/lib/DbHandler.php | 12 ++++ .../lib/SyncFederationAddressBooks.php | 4 ++ apps/federation/tests/DbHandlerTest.php | 11 +++ .../tests/SyncFederationAddressbooksTest.php | 69 +++++++++++++++++++ 4 files changed, 96 insertions(+) diff --git a/apps/federation/lib/DbHandler.php b/apps/federation/lib/DbHandler.php index 13cee661ba289..314c8b6df627b 100644 --- a/apps/federation/lib/DbHandler.php +++ b/apps/federation/lib/DbHandler.php @@ -210,6 +210,18 @@ public function setServerStatus(string $url, int $status, ?string $token = null) $query->executeStatement(); } + /** + * Set server sync token without changing the server status + */ + public function setServerSyncToken(string $url, string $token): void { + $hash = $this->hash($url); + $query = $this->connection->getQueryBuilder(); + $query->update($this->dbTable) + ->set('sync_token', $query->createNamedParameter($token)) + ->where($query->expr()->eq('url_hash', $query->createNamedParameter($hash))); + $query->executeStatement(); + } + /** * Get server status */ diff --git a/apps/federation/lib/SyncFederationAddressBooks.php b/apps/federation/lib/SyncFederationAddressBooks.php index f61cde8e10d3a..308bdf4356e78 100644 --- a/apps/federation/lib/SyncFederationAddressBooks.php +++ b/apps/federation/lib/SyncFederationAddressBooks.php @@ -66,6 +66,10 @@ public function syncThemAll(\Closure $callback, bool $full = false) { $targetPrincipal, $targetBookProperties ); + if ($truncated && $syncToken !== null) { + // Keep the progress of completed pages in case a later page fails + $this->dbHandler->setServerSyncToken($url, $syncToken); + } } while ($truncated); if ($full) { diff --git a/apps/federation/tests/DbHandlerTest.php b/apps/federation/tests/DbHandlerTest.php index 149841367f979..4e56e5086f88f 100644 --- a/apps/federation/tests/DbHandlerTest.php +++ b/apps/federation/tests/DbHandlerTest.php @@ -215,6 +215,17 @@ public function testSetServerStatus(): void { $this->assertSame(TrustedServers::STATUS_OK, (int)$result[0]['status']); } + public function testSetServerSyncToken(): void { + $this->dbHandler->addServer('server1'); + $this->dbHandler->setServerStatus('http://server1', TrustedServers::STATUS_FAILURE); + + $this->dbHandler->setServerSyncToken('http://server1', 'token1234567890'); + + $servers = $this->dbHandler->getAllServer(); + $this->assertSame('token1234567890', $servers[0]['sync_token']); + $this->assertSame(TrustedServers::STATUS_FAILURE, (int)$servers[0]['status']); + } + public function testGetServerStatus(): void { $this->dbHandler->addServer('server1'); $this->dbHandler->setServerStatus('http://server1', TrustedServers::STATUS_OK); diff --git a/apps/federation/tests/SyncFederationAddressbooksTest.php b/apps/federation/tests/SyncFederationAddressbooksTest.php index 1dacbb288ba8e..76fe1858cd997 100644 --- a/apps/federation/tests/SyncFederationAddressbooksTest.php +++ b/apps/federation/tests/SyncFederationAddressbooksTest.php @@ -80,6 +80,75 @@ public function testException(): void { $this->assertCount(2, $this->callBacks); } + public function testTruncatedSyncPersistsTokenAfterEachPage(): void { + /** @var DbHandler&MockObject $dbHandler */ + $dbHandler = $this->createMock(DbHandler::class); + $dbHandler->method('getAllServer') + ->willReturn([ + [ + 'url' => 'https://cloud.example.org', + 'url_hash' => 'sha1', + 'shared_secret' => 'ilovenextcloud', + 'sync_token' => null + ] + ]); + $tokenCalls = []; + $dbHandler->expects($this->exactly(2))->method('setServerSyncToken') + ->willReturnCallback(function (string $url, string $token) use (&$tokenCalls): void { + $tokenCalls[] = [$url, $token]; + }); + $dbHandler->expects($this->once())->method('setServerStatus') + ->with('https://cloud.example.org', TrustedServers::STATUS_OK, '5'); + $syncService = $this->createMock(SyncService::class); + $syncService->expects($this->exactly(3))->method('syncRemoteAddressBook') + ->willReturnOnConsecutiveCalls(['init_1_5', true], ['init_2_5', true], ['5', false]); + + /** @var SyncService $syncService */ + $s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger); + $s->syncThemAll(function ($url, $ex): void { + $this->callBacks[] = [$url, $ex]; + }); + $this->assertCount(1, $this->callBacks); + $this->assertSame([ + ['https://cloud.example.org', 'init_1_5'], + ['https://cloud.example.org', 'init_2_5'], + ], $tokenCalls); + } + + public function testFailedPageKeepsTokenOfCompletedPages(): void { + /** @var DbHandler&MockObject $dbHandler */ + $dbHandler = $this->createMock(DbHandler::class); + $dbHandler->method('getAllServer') + ->willReturn([ + [ + 'url' => 'https://cloud.example.org', + 'url_hash' => 'sha1', + 'shared_secret' => 'ilovenextcloud', + 'sync_token' => null + ] + ]); + $dbHandler->expects($this->once())->method('setServerSyncToken') + ->with('https://cloud.example.org', 'init_1_5'); + $dbHandler->expects($this->once())->method('setServerStatus') + ->with('https://cloud.example.org', TrustedServers::STATUS_FAILURE); + $page = 0; + $syncService = $this->createMock(SyncService::class); + $syncService->expects($this->exactly(2))->method('syncRemoteAddressBook') + ->willReturnCallback(function () use (&$page) { + if ($page++ === 0) { + return ['init_1_5', true]; + } + throw new \Exception('something did not work out'); + }); + + /** @var SyncService $syncService */ + $s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger); + $s->syncThemAll(function ($url, $ex): void { + $this->callBacks[] = [$url, $ex]; + }); + $this->assertCount(2, $this->callBacks); + } + public function testSuccessfulSyncWithoutChangesAfterFailure(): void { /** @var DbHandler&MockObject $dbHandler */ $dbHandler = $this->createMock(DbHandler::class);