From 3c3667fe18c3263f6e51ebeefdf5b1086e11f23a Mon Sep 17 00:00:00 2001
From: Pablo Zimdahl <pablo@nextcloud.com>
Date: Thu, 22 Aug 2024 10:22:30 +0200
Subject: [PATCH] fix(federation): always set server status to OK after
 successful runs

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>
---
 .../lib/BackgroundJob/GetSharedSecret.php     |  1 +
 .../lib/BackgroundJob/RequestSharedSecret.php |  1 +
 .../lib/SyncFederationAddressBooks.php        |  4 +++
 .../tests/SyncFederationAddressbooksTest.php  | 31 +++++++++++++++++++
 4 files changed, 37 insertions(+)

diff --git a/apps/federation/lib/BackgroundJob/GetSharedSecret.php b/apps/federation/lib/BackgroundJob/GetSharedSecret.php
index f07e0d8c2a703..1a23d58a7d100 100644
--- a/apps/federation/lib/BackgroundJob/GetSharedSecret.php
+++ b/apps/federation/lib/BackgroundJob/GetSharedSecret.php
@@ -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;
diff --git a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
index 07243f5c94d51..a1d0d2b0df06a 100644
--- a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
+++ b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
@@ -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;
diff --git a/apps/federation/lib/SyncFederationAddressBooks.php b/apps/federation/lib/SyncFederationAddressBooks.php
index 46a6a8c40a512..1b47c92db1a4c 100644
--- a/apps/federation/lib/SyncFederationAddressBooks.php
+++ b/apps/federation/lib/SyncFederationAddressBooks.php
@@ -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) {
diff --git a/apps/federation/tests/SyncFederationAddressbooksTest.php b/apps/federation/tests/SyncFederationAddressbooksTest.php
index 1ebca2f6a0f40..1b53f238bfaa6 100644
--- a/apps/federation/tests/SyncFederationAddressbooksTest.php
+++ b/apps/federation/tests/SyncFederationAddressbooksTest.php
@@ -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));
+	}
 }