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>

2 files changed, 26 insertions, 36 deletions

diff --git a/apps/federation/tests/Controller/SettingsControllerTest.php b/apps/federation/tests/Controller/SettingsControllerTest.php
index 856dcaa533f..c33ee6feb87 100644
--- a/apps/federation/tests/Controller/SettingsControllerTest.php
+++ b/apps/federation/tests/Controller/SettingsControllerTest.php
@@ -5,6 +5,7 @@
  * @author Björn Schießle <bjoern@schiessle.org>
  * @author Morris Jobke <hey@morrisjobke.de>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
+ * @author Carl Schwan <carl@carlschwan.eu>
  *
  * @license AGPL-3.0
  *
@@ -31,9 +32,7 @@ use OCP\IRequest;
 use Test\TestCase;
 
 class SettingsControllerTest extends TestCase {
-
-	/** @var SettingsController  */
-	private $controller;
+	private SettingsController $controller;
 
 	/** @var \PHPUnit\Framework\MockObject\MockObject | \OCP\IRequest */
 	private $request;
@@ -60,7 +59,7 @@ class SettingsControllerTest extends TestCase {
 		);
 	}
 
-	public function testAddServer() {
+	public function testAddServer(): void {
 		$this->trustedServers
 			->expects($this->once())
 			->method('isTrustedServer')
@@ -68,7 +67,7 @@ class SettingsControllerTest extends TestCase {
 			->willReturn(false);
 		$this->trustedServers
 			->expects($this->once())
-			->method('isOwnCloudServer')
+			->method('isNextcloudServer')
 			->with('url')
 			->willReturn(true);
 
@@ -83,11 +82,8 @@ class SettingsControllerTest extends TestCase {
 
 	/**
 	 * @dataProvider checkServerFails
-	 *
-	 * @param bool $isTrustedServer
-	 * @param bool $isOwnCloud
 	 */
-	public function testAddServerFail($isTrustedServer, $isOwnCloud) {
+	public function testAddServerFail(bool $isTrustedServer, bool $isNextcloud): void {
 		$this->expectException(\OCP\HintException::class);
 
 		$this->trustedServers
@@ -97,14 +93,14 @@ class SettingsControllerTest extends TestCase {
 			->willReturn($isTrustedServer);
 		$this->trustedServers
 			->expects($this->any())
-			->method('isOwnCloudServer')
+			->method('isNextcloudServer')
 			->with('url')
-			->willReturn($isOwnCloud);
+			->willReturn($isNextcloud);
 
 		$this->controller->addServer('url');
 	}
 
-	public function testRemoveServer() {
+	public function testRemoveServer(): void {
 		$this->trustedServers->expects($this->once())->method('removeServer')
 		->with('url');
 		$result = $this->controller->removeServer('url');
@@ -112,7 +108,7 @@ class SettingsControllerTest extends TestCase {
 		$this->assertSame(200, $result->getStatus());
 	}
 
-	public function testCheckServer() {
+	public function testCheckServer(): void {
 		$this->trustedServers
 			->expects($this->once())
 			->method('isTrustedServer')
@@ -120,7 +116,7 @@ class SettingsControllerTest extends TestCase {
 			->willReturn(false);
 		$this->trustedServers
 			->expects($this->once())
-			->method('isOwnCloudServer')
+			->method('isNextcloudServer')
 			->with('url')
 			->willReturn(true);
 
@@ -131,11 +127,8 @@ class SettingsControllerTest extends TestCase {
 
 	/**
 	 * @dataProvider checkServerFails
-	 *
-	 * @param bool $isTrustedServer
-	 * @param bool $isOwnCloud
 	 */
-	public function testCheckServerFail($isTrustedServer, $isOwnCloud) {
+	public function testCheckServerFail(bool $isTrustedServer, bool $isNextcloud): void {
 		$this->expectException(\OCP\HintException::class);
 
 		$this->trustedServers
@@ -145,9 +138,9 @@ class SettingsControllerTest extends TestCase {
 			->willReturn($isTrustedServer);
 		$this->trustedServers
 			->expects($this->any())
-			->method('isOwnCloudServer')
+			->method('isNextcloudServer')
 			->with('url')
-			->willReturn($isOwnCloud);
+			->willReturn($isNextcloud);
 
 		$this->assertTrue(
 			$this->invokePrivate($this->controller, 'checkServer', ['url'])
@@ -155,11 +148,9 @@ class SettingsControllerTest extends TestCase {
 	}
 
 	/**
-	 * data to simulate checkServer fails
-	 *
-	 * @return array
+	 * Data to simulate checkServer fails
 	 */
-	public function checkServerFails() {
+	public function checkServerFails(): array {
 		return [
 			[true, true],
 			[false, false]
diff --git a/apps/federation/tests/TrustedServersTest.php b/apps/federation/tests/TrustedServersTest.php
index 3dd93a445cd..0dc155f0b8d 100644
--- a/apps/federation/tests/TrustedServersTest.php
+++ b/apps/federation/tests/TrustedServersTest.php
@@ -37,7 +37,7 @@ use OCP\Http\Client\IResponse;
 use OCP\IConfig;
 use OCP\ILogger;
 use OCP\Security\ISecureRandom;
-use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use OCP\EventDispatcher\IEventDispatcher;
 use Test\TestCase;
 
 class TrustedServersTest extends TestCase {
@@ -69,7 +69,7 @@ class TrustedServersTest extends TestCase {
 	/** @var  \PHPUnit\Framework\MockObject\MockObject | IConfig */
 	private $config;
 
-	/** @var  \PHPUnit\Framework\MockObject\MockObject | EventDispatcherInterface */
+	/** @var  \PHPUnit\Framework\MockObject\MockObject | IEventDispatcher */
 	private $dispatcher;
 
 	/** @var \PHPUnit\Framework\MockObject\MockObject|ITimeFactory */
@@ -80,7 +80,7 @@ class TrustedServersTest extends TestCase {
 
 		$this->dbHandler = $this->getMockBuilder(DbHandler::class)
 			->disableOriginalConstructor()->getMock();
-		$this->dispatcher = $this->getMockBuilder(EventDispatcherInterface::class)
+		$this->dispatcher = $this->getMockBuilder(IEventDispatcher::class)
 			->disableOriginalConstructor()->getMock();
 		$this->httpClientService = $this->getMockBuilder(IClientService::class)->getMock();
 		$this->httpClient = $this->getMockBuilder(IClient::class)->getMock();
@@ -175,13 +175,12 @@ class TrustedServersTest extends TestCase {
 		$this->dbHandler->expects($this->once())->method('removeServer')->with($id);
 		$this->dbHandler->expects($this->once())->method('getServerById')->with($id)
 			->willReturn($server);
-		$this->dispatcher->expects($this->once())->method('dispatch')
+		$this->dispatcher->expects($this->once())->method('dispatchTyped')
 			->willReturnCallback(
-				function ($eventId, $event) {
-					$this->assertSame($eventId, 'OCP\Federation\TrustedServerEvent::remove');
-					$this->assertInstanceOf('Symfony\Component\EventDispatcher\GenericEvent', $event);
-					/** @var \Symfony\Component\EventDispatcher\GenericEvent $event */
-					$this->assertSame('url_hash', $event->getSubject());
+				function ($event) {
+					$this->assertTrue($event instanceof \OCP\Federated\Events\TrustedServerRemovedEvent);
+					/** @var \OCP\Federated\Events\TrustedServerRemovedEvent $event */
+					$this->assertSame('url_hash', $event->getUrlHash());
 				}
 			);
 		$this->trustedServers->removeServer($id);
@@ -221,13 +220,13 @@ class TrustedServersTest extends TestCase {
 	}
 
 	/**
-	 * @dataProvider dataTestIsOwnCloudServer
+	 * @dataProvider dataTestIsNextcloudServer
 	 *
 	 * @param int $statusCode
 	 * @param bool $isValidOwnCloudVersion
 	 * @param bool $expected
 	 */
-	public function testIsOwnCloudServer($statusCode, $isValidOwnCloudVersion, $expected) {
+	public function testIsNextcloudServer($statusCode, $isValidOwnCloudVersion, $expected) {
 		$server = 'server1';
 
 		/** @var \PHPUnit\Framework\MockObject\MockObject | TrustedServers $trustedServers */
@@ -264,7 +263,7 @@ class TrustedServersTest extends TestCase {
 		}
 
 		$this->assertSame($expected,
-			$trustedServers->isOwnCloudServer($server)
+			$trustedServers->isNextcloudServer($server)
 		);
 	}