Skip to content

Commit

Permalink
fix(federation): always set server status to OK after successful runs
Browse files Browse the repository at this point in the history
Previously if a server status got set to failure, it stayed that way
until an addressbook-sync found changes. Now the server status is set to
OK after each successful sync check (if that's not the case already),
regardless of addressbook changes.

This change also includes two new logging statements, which could help
next time someone debugs this.

Signed-off-by: Pablo Zimdahl <pablo@nextcloud.com>
  • Loading branch information
pabzm committed Aug 22, 2024
1 parent 4eebf47 commit 3c3667f
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 0 deletions.
1 change: 1 addition & 0 deletions apps/federation/lib/BackgroundJob/GetSharedSecret.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ protected function run($argument) {
// kill job after 30 days of trying
$deadline = $currentTime - $this->maxLifespan;
if ($created < $deadline) {
$this->logger->warning("The job to get the shared secret job is too old and gets stopped now without retention. Setting server status of '{$target}' to failure.");
$this->retainJob = false;
$this->trustedServers->setServerStatus($target, TrustedServers::STATUS_FAILURE);
return;
Expand Down
1 change: 1 addition & 0 deletions apps/federation/lib/BackgroundJob/RequestSharedSecret.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ protected function run($argument) {
// kill job after 30 days of trying
$deadline = $currentTime - $this->maxLifespan;
if ($created < $deadline) {
$this->logger->warning("The job to request the shared secret job is too old and gets stopped now without retention. Setting server status of '{$target}' to failure.");
$this->retainJob = false;
$this->trustedServers->setServerStatus($target, TrustedServers::STATUS_FAILURE);
return;
Expand Down
4 changes: 4 additions & 0 deletions apps/federation/lib/SyncFederationAddressBooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ public function syncThemAll(\Closure $callback) {
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK, $newToken);
} else {
$this->logger->debug("Sync Token for $url unchanged from previous sync");
// The server status might have been changed to a failure status in previous runs.
if ($this->dbHandler->getServerStatus($url) !== TrustedServers::STATUS_OK) {
$this->dbHandler->setServerStatus($url, TrustedServers::STATUS_OK);
}
}
} catch (\Exception $ex) {
if ($ex->getCode() === Http::STATUS_UNAUTHORIZED) {
Expand Down
31 changes: 31 additions & 0 deletions apps/federation/tests/SyncFederationAddressbooksTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,35 @@ public function testException() {
});
$this->assertEquals(2, count($this->callBacks));
}

public function testSuccessfulSyncWithoutChangesAfterFailure() {
/** @var DbHandler | MockObject $dbHandler */
$dbHandler = $this->getMockBuilder('OCA\Federation\DbHandler')
->disableOriginalConstructor()
->getMock();
$dbHandler->method('getAllServer')
->willReturn([
[
'url' => 'https://cloud.drop.box',
'url_hash' => 'sha1',
'shared_secret' => 'ilovenextcloud',
'sync_token' => '0'
]
]);
$dbHandler->method('getServerStatus')->willReturn(\OCA\Federation\TrustedServers::STATUS_FAILURE);
$dbHandler->expects($this->once())->method('setServerStatus')->
with('https://cloud.drop.box', 1);
$syncService = $this->getMockBuilder('OCA\DAV\CardDAV\SyncService')
->disableOriginalConstructor()
->getMock();
$syncService->expects($this->once())->method('syncRemoteAddressBook')
->willReturn('0');

/** @var \OCA\DAV\CardDAV\SyncService $syncService */
$s = new SyncFederationAddressBooks($dbHandler, $syncService, $this->discoveryService, $this->logger);
$s->syncThemAll(function ($url, $ex) {
$this->callBacks[] = [$url, $ex];
});
$this->assertEquals('1', count($this->callBacks));
}
}

0 comments on commit 3c3667f

Please sign in to comment.