From 206a4786c549452b9037034f1830d676646a7087 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 6 Oct 2021 09:53:14 +0200 Subject: [PATCH 1/2] Fix deleting notifications with numeric user ID Signed-off-by: Joas Schilling --- lib/App.php | 2 +- lib/Controller/EndpointController.php | 2 +- lib/Handler.php | 2 +- .../app/lib/Controller/EndpointController.php | 5 +- .../features/bootstrap/FeatureContext.php | 46 ++++++++----------- .../features/delete-notifications-v1.feature | 6 +-- .../features/delete-notifications-v2.feature | 27 +++++++++-- 7 files changed, 53 insertions(+), 37 deletions(-) diff --git a/lib/App.php b/lib/App.php index 8b0555e23..0485eeb55 100644 --- a/lib/App.php +++ b/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) { diff --git a/lib/Controller/EndpointController.php b/lib/Controller/EndpointController.php index df56b4b01..6dc273f58 100644 --- a/lib/Controller/EndpointController.php +++ b/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()); diff --git a/lib/Handler.php b/lib/Handler.php index 71b460616..8d3e8a2cb 100644 --- a/lib/Handler.php +++ b/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']]); } } diff --git a/tests/Integration/app/lib/Controller/EndpointController.php b/tests/Integration/app/lib/Controller/EndpointController.php index 11a6cfa25..c637d7e23 100644 --- a/tests/Integration/app/lib/Controller/EndpointController.php +++ b/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')) diff --git a/tests/Integration/features/bootstrap/FeatureContext.php b/tests/Integration/features/bootstrap/FeatureContext.php index f88429fea..51b66cb8c 100644 --- a/tests/Integration/features/bootstrap/FeatureContext.php +++ b/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); } } diff --git a/tests/Integration/features/delete-notifications-v1.feature b/tests/Integration/features/delete-notifications-v1.feature index 37adaf75e..6b81b1437 100644 --- a/tests/Integration/features/delete-notifications-v1.feature +++ b/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 diff --git a/tests/Integration/features/delete-notifications-v2.feature b/tests/Integration/features/delete-notifications-v2.feature index c3b7b5808..649a3386c 100644 --- a/tests/Integration/features/delete-notifications-v2.feature +++ b/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 From 33609b4d27a421758a0c10f8234bcfc7e56ccd0c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 6 Oct 2021 10:29:48 +0200 Subject: [PATCH 2/2] Replace app.php Signed-off-by: Joas Schilling --- tests/Integration/app/appinfo/app.php | 22 --------- .../app/lib/AppInfo/Application.php | 46 +++++++++++++++++++ 2 files changed, 46 insertions(+), 22 deletions(-) delete mode 100644 tests/Integration/app/appinfo/app.php create mode 100644 tests/Integration/app/lib/AppInfo/Application.php diff --git a/tests/Integration/app/appinfo/app.php b/tests/Integration/app/appinfo/app.php deleted file mode 100644 index 4d376dfc4..000000000 --- a/tests/Integration/app/appinfo/app.php +++ /dev/null @@ -1,22 +0,0 @@ - - * - * @copyright Copyright (c) 2016, ownCloud, Inc. - * @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 - * - */ - -\OC::$server->getNotificationManager()->registerNotifierService(\OCA\NotificationsIntegrationTesting\Notifier::class); diff --git a/tests/Integration/app/lib/AppInfo/Application.php b/tests/Integration/app/lib/AppInfo/Application.php new file mode 100644 index 000000000..26f0c7b25 --- /dev/null +++ b/tests/Integration/app/lib/AppInfo/Application.php @@ -0,0 +1,46 @@ + + * + * @copyright Copyright (c) 2021 Joas Schilling + * + * @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 + * + */ + +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 { + } +}