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>

diff --git a/apps/federation/lib/BackgroundJob/GetSharedSecret.php b/apps/federation/lib/BackgroundJob/GetSharedSecret.php
index b4ad46febc2bfe52b16740eac24c10172d6511c7..dcd1f85707c82c0b325bad363a253f6fcc45e91a 100644 (file)
--- a/apps/federation/lib/BackgroundJob/GetSharedSecret.php
+++ b/apps/federation/lib/BackgroundJob/GetSharedSecret.php
@@ -112,6 +112,7 @@ class GetSharedSecret extends Job {
                // 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 f8c25168b56ce35d19e0b8d6339dcd8976dc04e0..e89e5ff79e4d6c9d532fd2a8ed1f298654564312 100644 (file)
--- a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
+++ b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
@@ -116,6 +116,7 @@ class RequestSharedSecret extends Job {
                // 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 2c598241131424036e77dd5737299f35c11f9e94..56256e06dc43af2dfa24530676a720d9a94bacb0 100644 (file)
--- a/apps/federation/lib/SyncFederationAddressBooks.php
+++ b/apps/federation/lib/SyncFederationAddressBooks.php
@@ -78,6 +78,10 @@ class SyncFederationAddressBooks {
                                        $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 782ca52322abeb5bb29fcfff31d61479df2097fd..15ad2ee035946a74e3674fe5536032b6208a6fd1 100644 (file)
--- a/apps/federation/tests/SyncFederationAddressbooksTest.php
+++ b/apps/federation/tests/SyncFederationAddressbooksTest.php
@@ -111,4 +111,35 @@ class SyncFederationAddressbooksTest extends \Test\TestCase {
                });
                $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));
+       }
 }