[stable21] Fix deleting notifications with numeric user ID

lib/App.php

@@ -79,7 +79,7 @@ public function markProcessed(INotification $notification): void {
 		}
 		foreach ($deleted as $user => $notifications) {
 			foreach ($notifications as $data) {
-				$this->push->pushDeleteToDevice($user, $data['id'], $data['app']);
+				$this->push->pushDeleteToDevice((string) $user, $data['id'], $data['app']);
 			}
 		}
 		if (!$isAlreadyDeferring) {

lib/Controller/EndpointController.php

@@ -175,7 +175,7 @@ public function deleteNotification(int $id): DataResponse {
 
 		try {
 			$notification = $this->handler->getById($id, $this->getCurrentUser());
-			$deleted = $this->handler->deleteById($id, $this->getCurrentUser());
+			$deleted = $this->handler->deleteById($id, $this->getCurrentUser(), $notification);
 
 			if ($deleted) {
 				$this->push->pushDeleteToDevice($this->getCurrentUser(), $id, $notification->getApp());

lib/Handler.php

@@ -110,7 +110,7 @@ public function delete(INotification $notification): array {
 
 		foreach ($deleted as $user => $entries) {
 			foreach ($entries as $entry) {
-				$this->deleteById($entry['id'], $user, $notifications[$entry['id']]);
+				$this->deleteById($entry['id'], (string) $user, $notifications[$entry['id']]);
 			}
 		}
 

tests/Integration/app/lib/AppInfo/Application.php

@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @author Joas Schilling <[email protected]>
+ *
+ * @copyright Copyright (c) 2021 Joas Schilling <[email protected]>
+ *
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\NotificationsIntegrationTesting\AppInfo;
+
+use OCA\NotificationsIntegrationTesting\Notifier;
+use OCP\AppFramework\App;
+use OCP\AppFramework\Bootstrap\IBootContext;
+use OCP\AppFramework\Bootstrap\IBootstrap;
+use OCP\AppFramework\Bootstrap\IRegistrationContext;
+
+class Application extends App implements IBootstrap {
+	public const APP_ID = 'notificationsintegrationtesting';
+
+	public function __construct() {
+		parent::__construct(self::APP_ID);
+	}
+
+	public function register(IRegistrationContext $context): void {
+		$context->registerNotifierService(Notifier::class);
+	}
+
+	public function boot(IBootContext $context): void {
+	}
+}

tests/Integration/app/lib/Controller/EndpointController.php

@@ -44,13 +44,14 @@ public function __construct($appName, IRequest $request, IManager $manager) {
 	/**
 	 * @NoCSRFRequired
 	 *
+	 * @param string $userId
 	 * @return DataResponse
 	 */
-	public function addNotification() {
+	public function addNotification(string $userId = 'test1') {
 		$notification = $this->manager->createNotification();
 		$notification->setApp($this->request->getParam('app', 'notificationsintegrationtesting'))
 			->setDateTime(\DateTime::createFromFormat('U', $this->request->getParam('timestamp', 1449585176))) // 2015-12-08T14:32:56+00:00
-			->setUser($this->request->getParam('user', 'test1'))
+			->setUser($this->request->getParam('user', $userId))
 			->setSubject($this->request->getParam('subject', 'testing'))
 			->setLink($this->request->getParam('link', 'https://example.tld/'))
 			->setMessage($this->request->getParam('message', 'message'))

tests/Integration/features/bootstrap/FeatureContext.php

@@ -70,10 +70,8 @@ public function __construct() {
 	 * @param string $user
 	 */
 	public function hasNotifications(string $user) {
-		if ($user === 'test1') {
-			$response = $this->setTestingValue('POST', 'apps/notificationsintegrationtesting/notifications', null);
-			$this->assertStatusCode($response, 200);
-		}
+		$response = $this->setTestingValue('POST', 'apps/notificationsintegrationtesting/notifications?userId=' . $user, null);
+		$this->assertStatusCode($response, 200);
 	}
 
 	/**
@@ -83,10 +81,8 @@ public function hasNotifications(string $user) {
 	 * @param TableNode|null $formData
 	 */
 	public function receiveNotification(string $user, TableNode $formData) {
-		if ($user === 'test1') {
-			$response = $this->setTestingValue('POST', 'apps/notificationsintegrationtesting/notifications', $formData);
-			$this->assertStatusCode($response, 200);
-		}
+		$response = $this->setTestingValue('POST', 'apps/notificationsintegrationtesting/notifications?userId=' . $user, $formData);
+		$this->assertStatusCode($response, 200);
 	}
 
 	/**
@@ -165,28 +161,26 @@ protected function getArrayOfNotificationsResponded(ResponseInterface $response)
 	 * @param string $missingLast
 	 */
 	public function userNumNotifications(string $user, int $numNotifications, string $api, string $missingLast) {
-		if ($user === 'test1') {
-			$this->sendingTo('GET', '/apps/notifications/api/' . $api . '/notifications?format=json');
-			$this->assertStatusCode($this->response, 200);
-
-			$previousNotificationIds = [];
-			if ($missingLast) {
-				Assert::assertNotEmpty($this->notificationIds);
-				$previousNotificationIds = end($this->notificationIds);
-			}
+		$this->sendingTo('GET', '/apps/notifications/api/' . $api . '/notifications?format=json');
+		$this->assertStatusCode($this->response, 200);
 
-			$this->checkNumNotifications((int) $numNotifications);
+		$previousNotificationIds = [];
+		if ($missingLast) {
+			Assert::assertNotEmpty($this->notificationIds);
+			$previousNotificationIds = end($this->notificationIds);
+		}
 
-			if ($missingLast) {
-				$now = end($this->notificationIds);
-				if ($missingLast === ' missing the last one') {
-					array_unshift($now, $this->deletedNotification);
-				} else {
-					$now[] = $this->deletedNotification;
-				}
+		$this->checkNumNotifications((int) $numNotifications);
 
-				Assert::assertEquals($previousNotificationIds, $now);
+		if ($missingLast) {
+			$now = end($this->notificationIds);
+			if ($missingLast === ' missing the last one') {
+				array_unshift($now, $this->deletedNotification);
+			} else {
+				$now[] = $this->deletedNotification;
 			}
+
+			Assert::assertEquals($previousNotificationIds, $now);
 		}
 	}
 

tests/Integration/features/delete-notifications-v1.feature

@@ -45,7 +45,7 @@ Feature: delete-notifications
     Given user "test1" has notifications
     Given user "test1" has notifications
     Given user "test1" has notifications
-    Then user "test1" has 3 notifications on v2
-    And delete all notifications on v2
+    Then user "test1" has 3 notifications on v1
+    And delete all notifications on v1
     And status code is 200
-    And user "test1" has 0 notifications on v2
+    And user "test1" has 0 notifications on v1

tests/Integration/features/delete-notifications-v2.feature

@@ -1,6 +1,7 @@
 Feature: delete-notifications
   Background:
     Given user "test1" exists
+    Given user "123456" exists
     Given as user "test1"
 
   Scenario: Delete first notification
@@ -12,6 +13,16 @@ Feature: delete-notifications
     And status code is 200
     And user "test1" has 2 notifications on v2 missing the first one
 
+  Scenario: Delete first notification as numeric user
+    Given as user "123456"
+    Given user "123456" has notifications
+    Given user "123456" has notifications
+    Given user "123456" has notifications
+    Then user "123456" has 3 notifications on v2
+    And delete first notification on v2
+    And status code is 200
+    And user "123456" has 2 notifications on v2 missing the first one
+
   Scenario: Delete same notification twice
     Given user "test1" has notifications
     Given user "test1" has notifications
@@ -45,7 +56,17 @@ Feature: delete-notifications
     Given user "test1" has notifications
     Given user "test1" has notifications
     Given user "test1" has notifications
-    Then user "test1" has 3 notifications on v1
-    And delete all notifications on v1
+    Then user "test1" has 3 notifications on v2
+    And delete all notifications on v2
+    And status code is 200
+    And user "test1" has 0 notifications on v2
+
+  Scenario: Delete all notifications as numeric user
+    Given as user "123456"
+    Given user "123456" has notifications
+    Given user "123456" has notifications
+    Given user "123456" has notifications
+    Then user "123456" has 3 notifications on v2
+    And delete all notifications on v2
     And status code is 200
-    And user "test1" has 0 notifications on v1
+    And user "123456" has 0 notifications on v2