[stable18] Delete unknown devices

.gitignore

@@ -57,6 +57,7 @@ nbproject
 /tests/Unit/coverage*
 /tests/Unit/clover.xml
 /tests/Unit/js/node_modules
+/tests/Unit/.phpunit.result.cache
 
 /tests/Integration/output
 /tests/Integration/vendor

lib/Push.php

@@ -92,7 +92,7 @@ public function pushToDevice(int $id, INotification $notification): void {
 		$userKey = $this->keyManager->getKey($user);
 
 		$isTalkNotification = \in_array($notification->getApp(), ['spreed', 'talk'], true);
-		$talkApps = array_filter($devices, function($device) {
+		$talkApps = array_filter($devices, function ($device) {
 			return $device['apptype'] === 'talk';
 		});
 		$hasTalkApps = !empty($talkApps);
@@ -131,43 +131,7 @@ public function pushToDevice(int $id, INotification $notification): void {
 			}
 		}
 
-		if (empty($pushNotifications)) {
-			return;
-		}
-
-		$client = $this->clientService->newClient();
-		foreach ($pushNotifications as $proxyServer => $notifications) {
-			try {
-				$response = $client->post($proxyServer . '/notifications', [
-					'body' => [
-						'notifications' => $notifications,
-					],
-				]);
-			} catch (\Exception $e) {
-				$this->log->logException($e, [
-					'app' => 'notifications',
-					'level' => $e->getCode() === Http::STATUS_BAD_REQUEST ? ILogger::INFO : ILogger::WARN,
-				]);
-				continue;
-			}
-
-			$status = $response->getStatusCode();
-			if ($status !== Http::STATUS_OK && $status !== Http::STATUS_SERVICE_UNAVAILABLE) {
-				$body = $response->getBody();
-				$this->log->error('Could not send notification to push server [{url}]: {error}',[
-					'error' => \is_string($body) ? $body : 'no reason given',
-					'url' => $proxyServer,
-					'app' => 'notifications',
-				]);
-			} else if ($status === Http::STATUS_SERVICE_UNAVAILABLE && $this->config->getSystemValue('debug', false)) {
-				$body = $response->getBody();
-				$this->log->debug('Could not send notification to push server [{url}]: {error}',[
-					'error' => \is_string($body) ? $body : 'no reason given',
-					'url' => $proxyServer,
-					'app' => 'notifications',
-				]);
-			}
-		}
+		$this->sendNotificationsToProxies($pushNotifications);
 	}
 
 	public function pushDeleteToDevice(string $userId, int $notificationId): void {
@@ -201,6 +165,10 @@ public function pushDeleteToDevice(string $userId, int $notificationId): void {
 			}
 		}
 
+		$this->sendNotificationsToProxies($pushNotifications);
+	}
+
+	protected function sendNotificationsToProxies(array $pushNotifications): void {
 		if (empty($pushNotifications)) {
 			return;
 		}
@@ -222,21 +190,31 @@ public function pushDeleteToDevice(string $userId, int $notificationId): void {
 			}
 
 			$status = $response->getStatusCode();
-			if ($status !== Http::STATUS_OK && $status !== Http::STATUS_SERVICE_UNAVAILABLE) {
+			if ($status === Http::STATUS_SERVICE_UNAVAILABLE && $this->config->getSystemValue('debug', false)) {
 				$body = $response->getBody();
-				$this->log->error('Could not send notification to push server [{url}]: {error}',[
+				$this->log->debug('Could not send notification to push server [{url}]: {error}',[
 					'error' => \is_string($body) ? $body : 'no reason given',
 					'url' => $proxyServer,
 					'app' => 'notifications',
 				]);
-			} else if ($status === Http::STATUS_SERVICE_UNAVAILABLE && $this->config->getSystemValue('debug', false)) {
-				$body = $response->getBody();
-				$this->log->debug('Could not send notification to push server [{url}]: {error}',[
-					'error' => \is_string($body) ? $body : 'no reason given',
+				continue;
+			}
+
+			$body = $response->getBody();
+			$bodyData = json_decode($body, true);
+			if ($status !== Http::STATUS_OK) {
+				$this->log->error('Could not send notification to push server [{url}]: {error}',[
+					'error' => \is_string($body) && $bodyData === null ? $body : 'no reason given',
 					'url' => $proxyServer,
 					'app' => 'notifications',
 				]);
 			}
+
+			if (is_array($bodyData) && !empty($bodyData['unknown']) && is_array($bodyData['unknown'])) {
+				foreach ($bodyData['unknown'] as $unknownDevice) {
+					$this->deletePushTokenByDeviceIdentifier($unknownDevice);
+				}
+			}
 		}
 	}
 
@@ -388,4 +366,16 @@ protected function deletePushToken(int $tokenId): bool {
 
 		return $query->execute() !== 0;
 	}
+
+	/**
+	 * @param string $deviceIdentifier
+	 * @return bool
+	 */
+	protected function deletePushTokenByDeviceIdentifier(string $deviceIdentifier): bool {
+		$query = $this->db->getQueryBuilder();
+		$query->delete('notifications_pushtokens')
+			->where($query->expr()->eq('deviceidentifier', $query->createNamedParameter($deviceIdentifier)));
+
+		return $query->execute() !== 0;
+	}
 }

tests/Unit/AppInfo/RoutesTest.php

@@ -33,10 +33,10 @@
 class RoutesTest extends TestCase {
 	public function testRoutes() {
 		$routes = include __DIR__ . '/../../../appinfo/routes.php';
-		$this->assertInternalType('array', $routes);
+		$this->assertIsArray($routes);
 		$this->assertCount(1, $routes);
 		$this->assertArrayHasKey('ocs', $routes);
-		$this->assertInternalType('array', $routes['ocs']);
+		$this->assertIsArray($routes['ocs']);
 		$this->assertCount(7, $routes['ocs']);
 	}
 }

tests/Unit/Controller/EndpointControllerTest.php

@@ -440,15 +440,15 @@ public function testDeleteAllNotifications($_, $username) {
 
 	public function dataNotificationToArray() {
 		return [
-			['v1', 42, 'app1', 'user1', 1234, 'type1', 42, 'subject1', '', [], 'message1', 'richMessage 1', ['richMessage param'], 'link1', 'icon1', [], []],
-			['v1', 1337, 'app2', 'user2', 1337, 'type2', 21, 'subject2', 'richSubject 2', ['richSubject param'], 'message2', '', [], 'link2', 'icon2', [
+			['v1', 42, 'app1', 'user1', 1234, 'type1', '42', 'subject1', '', [], 'message1', 'richMessage 1', ['richMessage param'], 'link1', 'icon1', [], []],
+			['v1', 1337, 'app2', 'user2', 1337, 'type2', '21', 'subject2', 'richSubject 2', ['richSubject param'], 'message2', '', [], 'link2', 'icon2', [
 				$this->getMockBuilder(IAction::class)
 					->getMock(),
 				$this->getMockBuilder(IAction::class)
 					->getMock(),
 			], [['action'], ['action']]],
-			['v2', 42, 'app1', 'user1', 1234, 'type1', 42, 'subject1', '', [], 'message1', 'richMessage 1', ['richMessage param'], 'link1', 'icon1', [], []],
-			['v2', 1337, 'app2', 'user2', 1337, 'type2', 21, 'subject2', 'richSubject 2', ['richSubject param'], 'message2', '', [], 'link2', 'icon2', [
+			['v2', 42, 'app1', 'user1', 1234, 'type1', '42', 'subject1', '', [], 'message1', 'richMessage 1', ['richMessage param'], 'link1', 'icon1', [], []],
+			['v2', 1337, 'app2', 'user2', 1337, 'type2', '21', 'subject2', 'richSubject 2', ['richSubject param'], 'message2', '', [], 'link2', 'icon2', [
 				$this->getMockBuilder(IAction::class)
 					->getMock(),
 				$this->getMockBuilder(IAction::class)

tests/Unit/Notifier/NotifierTest.php

@@ -61,10 +61,6 @@ protected function setUp(): void {
 		);
 	}
 
-	/**
-	 * @expectedException \InvalidArgumentException
-	 * @expectedExceptionMessage Unknown app
-	 */
 	public function testPrepareWrongApp() {
 		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
 		$notification = $this->createMock(INotification::class);
@@ -75,13 +71,11 @@ public function testPrepareWrongApp() {
 		$notification->expects($this->never())
 			->method('getSubject');
 
+		$this->expectException(\InvalidArgumentException::class);
+		$this->expectExceptionMessage('Unknown app');
 		$this->notifier->prepare($notification, 'en');
 	}
 
-	/**
-	 * @expectedException \InvalidArgumentException
-	 * @expectedExceptionMessage Unknown subject
-	 */
 	public function testPrepareWrongSubject() {
 		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
 		$notification = $this->createMock(INotification::class);
@@ -93,6 +87,8 @@ public function testPrepareWrongSubject() {
 			->method('getSubject')
 			->willReturn('wrong subject');
 
+		$this->expectException(\InvalidArgumentException::class);
+		$this->expectExceptionMessage('Unknown subject');
 		$this->notifier->prepare($notification, 'en');
 	}
 

tests/Unit/PushTest.php

@@ -468,7 +468,7 @@ public function testPushToDeviceSending($isDebug) {
 		$response2->expects($this->once())
 			->method('getStatusCode')
 			->willReturn(Http::STATUS_SERVICE_UNAVAILABLE);
-		$response2->expects($isDebug ? $this->once() : $this->never())
+		$response2->expects($this->once())
 			->method('getBody')
 			->willReturn('Maintenance');
 		$client->expects($this->at(2))

tests/Unit/phpunit.xml

@@ -1,6 +1,5 @@
 <?xml version="1.0" encoding="utf-8" ?>
 <phpunit bootstrap="bootstrap.php"
-		 strict="true"
 		 verbose="true"
 		 timeoutForSmallTests="900"
 		 timeoutForMediumTests="900"