Summer cleanup of the federation app

- Use IEventDispatcher instead of deprecated symfony dispatcher
- Use LoggerInterface where possible
- Use php 7.4 properties
- Add type hinting where possible
- Move federation hooks to a seperate listener

Signed-off-by: Carl Schwan <carl@carlschwan.eu>

Author: CarlSchwan

apps/dav/composer/composer/autoload_classmap.php

@@ -246,6 +246,7 @@
     'OCA\\DAV\\Listener\\CardListener' => $baseDir . '/../lib/Listener/CardListener.php',
     'OCA\\DAV\\Listener\\ClearPhotoCacheListener' => $baseDir . '/../lib/Listener/ClearPhotoCacheListener.php',
     'OCA\\DAV\\Listener\\SubscriptionListener' => $baseDir . '/../lib/Listener/SubscriptionListener.php',
+    'OCA\\DAV\\Listener\\TrustedServerRemovedListener' => $baseDir . '/../lib/Listener/TrustedServerRemovedListener.php',
     'OCA\\DAV\\Migration\\BuildCalendarSearchIndex' => $baseDir . '/../lib/Migration/BuildCalendarSearchIndex.php',
     'OCA\\DAV\\Migration\\BuildCalendarSearchIndexBackgroundJob' => $baseDir . '/../lib/Migration/BuildCalendarSearchIndexBackgroundJob.php',
     'OCA\\DAV\\Migration\\BuildSocialSearchIndex' => $baseDir . '/../lib/Migration/BuildSocialSearchIndex.php',

apps/dav/composer/composer/autoload_static.php

@@ -261,6 +261,7 @@ class ComposerStaticInitDAV
         'OCA\\DAV\\Listener\\CardListener' => __DIR__ . '/..' . '/../lib/Listener/CardListener.php',
         'OCA\\DAV\\Listener\\ClearPhotoCacheListener' => __DIR__ . '/..' . '/../lib/Listener/ClearPhotoCacheListener.php',
         'OCA\\DAV\\Listener\\SubscriptionListener' => __DIR__ . '/..' . '/../lib/Listener/SubscriptionListener.php',
+        'OCA\\DAV\\Listener\\TrustedServerRemovedListener' => __DIR__ . '/..' . '/../lib/Listener/TrustedServerRemovedListener.php',
         'OCA\\DAV\\Migration\\BuildCalendarSearchIndex' => __DIR__ . '/..' . '/../lib/Migration/BuildCalendarSearchIndex.php',
         'OCA\\DAV\\Migration\\BuildCalendarSearchIndexBackgroundJob' => __DIR__ . '/..' . '/../lib/Migration/BuildCalendarSearchIndexBackgroundJob.php',
         'OCA\\DAV\\Migration\\BuildSocialSearchIndex' => __DIR__ . '/..' . '/../lib/Migration/BuildSocialSearchIndex.php',

apps/dav/lib/AppInfo/Application.php

@@ -71,6 +71,7 @@
 use OCA\DAV\Events\CardUpdatedEvent;
 use OCA\DAV\Events\SubscriptionCreatedEvent;
 use OCA\DAV\Events\SubscriptionDeletedEvent;
+use OCP\Federation\Events\TrustedServerRemovedEvent;
 use OCA\DAV\HookManager;
 use OCA\DAV\Listener\ActivityUpdaterListener;
 use OCA\DAV\Listener\AddressbookListener;
@@ -83,6 +84,7 @@
 use OCA\DAV\Listener\CardListener;
 use OCA\DAV\Listener\ClearPhotoCacheListener;
 use OCA\DAV\Listener\SubscriptionListener;
+use OCA\DAV\Listener\TrustedServerRemovedListener;
 use OCA\DAV\Search\ContactsSearchProvider;
 use OCA\DAV\Search\EventsSearchProvider;
 use OCA\DAV\Search\TasksSearchProvider;
@@ -182,6 +184,7 @@ public function register(IRegistrationContext $context): void {
 		$context->registerEventListener(CardUpdatedEvent::class, BirthdayListener::class);
 		$context->registerEventListener(CardDeletedEvent::class, ClearPhotoCacheListener::class);
 		$context->registerEventListener(CardUpdatedEvent::class, ClearPhotoCacheListener::class);
+		$context->registerEventListener(TrustedServerRemovedEvent::class, TrustedServerRemovedListener::class);
 
 		$context->registerNotifierService(Notifier::class);
 
@@ -235,18 +238,6 @@ public function registerHooks(HookManager $hm,
 			// Here we should recalculate if reminders should be sent to new or old sharees
 		});
 
-		$dispatcher->addListener('OCP\Federation\TrustedServerEvent::remove',
-			function (GenericEvent $event) {
-				/** @var CardDavBackend $cardDavBackend */
-				$cardDavBackend = \OC::$server->query(CardDavBackend::class);
-				$addressBookUri = $event->getSubject();
-				$addressBook = $cardDavBackend->getAddressBooksByUri('principals/system/system', $addressBookUri);
-				if (!is_null($addressBook)) {
-					$cardDavBackend->deleteAddressBook($addressBook['id']);
-				}
-			}
-		);
-
 		$eventHandler = function () use ($container, $serverContainer): void {
 			try {
 				/** @var UpdateCalendarResourcesRoomsBackgroundJob $job */

apps/dav/lib/CardDAV/SyncService.php

@@ -73,20 +73,13 @@ public function __construct(CardDavBackend $backend,
 	}
 
 	/**
-	 * @param string $url
-	 * @param string $userName
-	 * @param string $addressBookUrl
-	 * @param string $sharedSecret
-	 * @param string $syncToken
-	 * @param int $targetBookId
-	 * @param string $targetPrincipal
 	 * @param array $targetProperties
 	 * @return string
 	 * @throws \Exception
 	 */
-	public function syncRemoteAddressBook($url, $userName, $addressBookUrl, $sharedSecret, $syncToken, $targetBookId, $targetPrincipal, $targetProperties) {
+	public function syncRemoteAddressBook(string $url, string $userName, string $addressBookUrl, string $sharedSecret, string $syncToken, string $targetBookId, string $targetPrincipal, array $targetProperties) {
 		// 1. create addressbook
-		$book = $this->ensureSystemAddressBookExists($targetPrincipal, (string)$targetBookId, $targetProperties);
+		$book = $this->ensureSystemAddressBookExists($targetPrincipal, $targetBookId, $targetProperties);
 		$addressBookId = $book['id'];
 
 		// 2. query changes

apps/dav/lib/Listener/TrustedServerRemovedListener.php

@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright 2022 Carl Schwan <carl@carlschwan.eu>
+ *
+ * @author Carl Schwan <carl@carlschwan.eu>
+ *
+ * @license AGPL-3.0-or-later
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OCA\DAV\Listener;
+
+use OCA\DAV\CardDAV\CardDavBackend;
+use OCP\EventDispatcher\Event;
+use OCP\EventDispatcher\IEventListener;
+use OCP\Federation\Events\TrustedServerRemovedEvent;
+
+class TrustedServerRemovedListener implements IEventListener {
+	private CardDavBackend $cardDavBackend;
+
+	public function __construct(CardDavBackend $cardDavBackend) {
+		$this->cardDavBackend = $cardDavBackend;
+	}
+
+	public function handle(Event $event): void {
+		if (!$event instanceof TrustedServerRemovedEvent) {
+			return;
+		}
+		$addressBookUri = $event->getUrlHash();
+		$addressBook = $this->cardDavBackend->getAddressBooksByUri('principals/system/system', $addressBookUri);
+		if (!is_null($addressBook)) {
+			$this->cardDavBackend->deleteAddressBook($addressBook['id']);
+		}
+	}
+}

apps/federation/lib/BackgroundJob/GetSharedSecret.php

@@ -42,59 +42,34 @@
 use OCP\ILogger;
 use OCP\IURLGenerator;
 use OCP\OCS\IDiscoveryService;
+use Psr\Log\LoggerInterface;
 
 /**
  * Class GetSharedSecret
  *
- * request shared secret from remote Nextcloud
+ * Request shared secret from remote Nextcloud
  *
  * @package OCA\Federation\Backgroundjob
  */
 class GetSharedSecret extends Job {
+	private IClient $httpClient;
+	private IJobList $jobList;
+	private IURLGenerator $urlGenerator;
+	private TrustedServers $trustedServers;
+	private IDiscoveryService $ocsDiscoveryService;
+	private LoggerInterface $logger;
+	protected bool $retainJob = false;
+	private string $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret';
+
+	/** 30 day = 2592000sec */
+	private int $maxLifespan = 2592000;
 
-	/** @var IClient */
-	private $httpClient;
-
-	/** @var IJobList */
-	private $jobList;
-
-	/** @var IURLGenerator */
-	private $urlGenerator;
-
-	/** @var TrustedServers  */
-	private $trustedServers;
-
-	/** @var IDiscoveryService  */
-	private $ocsDiscoveryService;
-
-	/** @var ILogger */
-	private $logger;
-
-	/** @var bool */
-	protected $retainJob = false;
-
-	private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/shared-secret';
-
-	/** @var  int  30 day = 2592000sec */
-	private $maxLifespan = 2592000;
-
-	/**
-	 * RequestSharedSecret constructor.
-	 *
-	 * @param IClientService $httpClientService
-	 * @param IURLGenerator $urlGenerator
-	 * @param IJobList $jobList
-	 * @param TrustedServers $trustedServers
-	 * @param ILogger $logger
-	 * @param IDiscoveryService $ocsDiscoveryService
-	 * @param ITimeFactory $timeFactory
-	 */
 	public function __construct(
 		IClientService $httpClientService,
 		IURLGenerator $urlGenerator,
 		IJobList $jobList,
 		TrustedServers $trustedServers,
-		ILogger $logger,
+		LoggerInterface $logger,
 		IDiscoveryService $ocsDiscoveryService,
 		ITimeFactory $timeFactory
 	) {
@@ -128,7 +103,7 @@ public function execute(IJobList $jobList, ILogger $logger = null) {
 	}
 
 	/**
-	 * call execute() method of parent
+	 * Call execute() method of parent
 	 *
 	 * @param IJobList $jobList
 	 * @param ILogger $logger
@@ -185,14 +160,16 @@ protected function run($argument) {
 			}
 		} catch (RequestException $e) {
 			$status = -1; // There is no status code if we could not connect
-			$this->logger->logException($e, [
-				'message' => 'Could not connect to ' . $target,
-				'level' => ILogger::INFO,
+			$this->logger->info('Could not connect to ' . $target, [
+				'exception' => $e,
 				'app' => 'federation',
 			]);
 		} catch (\Throwable $e) {
 			$status = Http::STATUS_INTERNAL_SERVER_ERROR;
-			$this->logger->logException($e, ['app' => 'federation']);
+			$this->logger->error($e->getMessage(), [
+				'app' => 'federation',
+				'exception' => $e,
+			]);
 		}
 
 		// if we received a unexpected response we try again later
@@ -226,7 +203,7 @@ protected function run($argument) {
 	 *
 	 * @param array $argument
 	 */
-	protected function reAddJob(array $argument) {
+	protected function reAddJob(array $argument): void {
 		$url = $argument['url'];
 		$created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime();
 		$token = $argument['token'];

apps/federation/lib/Command/SyncFederationAddressBooks.php

@@ -28,16 +28,12 @@
 use Symfony\Component\Console\Helper\ProgressBar;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
+use OCA\Federation\SyncFederationAddressBooks as SyncService;
 
 class SyncFederationAddressBooks extends Command {
+	private SyncService $syncService;
 
-	/** @var \OCA\Federation\SyncFederationAddressBooks */
-	private $syncService;
-
-	/**
-	 * @param \OCA\Federation\SyncFederationAddressBooks $syncService
-	 */
-	public function __construct(\OCA\Federation\SyncFederationAddressBooks $syncService) {
+	public function __construct(SyncService $syncService) {
 		parent::__construct();
 
 		$this->syncService = $syncService;
@@ -49,11 +45,6 @@ protected function configure() {
 			->setDescription('Synchronizes addressbooks of all federated clouds');
 	}
 
-	/**
-	 * @param InputInterface $input
-	 * @param OutputInterface $output
-	 * @return int
-	 */
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$progress = new ProgressBar($output);
 		$progress->start();