From 4679ff726a7090c2b9b342e5168bcda805c1331c 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 b4ad46febc2bf..dcd1f85707c82 100644
--- a/apps/federation/lib/BackgroundJob/GetSharedSecret.php
+++ b/apps/federation/lib/BackgroundJob/GetSharedSecret.php
@@ -112,6 +112,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 f8c25168b56ce..e89e5ff79e4d6 100644
--- a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
+++ b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
@@ -116,6 +116,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 2c59824113142..56256e06dc43a 100644
--- a/apps/federation/lib/SyncFederationAddressBooks.php
+++ b/apps/federation/lib/SyncFederationAddressBooks.php
@@ -78,6 +78,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 782ca52322abe..15ad2ee035946 100644
--- a/apps/federation/tests/SyncFederationAddressbooksTest.php
+++ b/apps/federation/tests/SyncFederationAddressbooksTest.php
@@ -111,4 +111,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));
+	}
 }