Merge pull request #15040 from nextcloud/feature/13980/push-for-deleted-notifications

Notifications overhaul

14 files changed, 136 insertions, 68 deletions

diff --git a/apps/comments/lib/AppInfo/Application.php b/apps/comments/lib/AppInfo/Application.php
index b44c1c519c8..916345e4a5f 100644
--- a/apps/comments/lib/AppInfo/Application.php
+++ b/apps/comments/lib/AppInfo/Application.php
@@ -76,15 +76,7 @@ class Application extends App {
 	}
 
 	protected function registerNotifier() {
-		$this->getContainer()->getServer()->getNotificationManager()->registerNotifier(
-			function() {
-				return $this->getContainer()->query(Notifier::class);
-			},
-			function () {
-				$l = $this->getContainer()->getServer()->getL10NFactory()->get('comments');
-				return ['id' => 'comments', 'name' => $l->t('Comments')];
-			}
-		);
+		$this->getContainer()->getServer()->getNotificationManager()->registerNotifierService(Notifier::class);
 	}
 
 	protected function registerCommentsEventHandler() {
diff --git a/apps/comments/lib/Notification/Notifier.php b/apps/comments/lib/Notification/Notifier.php
index 2132f05ef88..e8a7ade820d 100644
--- a/apps/comments/lib/Notification/Notifier.php
+++ b/apps/comments/lib/Notification/Notifier.php
@@ -32,6 +32,7 @@ use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
+use OCP\Notification\AlreadyProcessedException;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
 
@@ -67,12 +68,34 @@ class Notifier implements INotifier {
 	}
 
 	/**
+	 * Identifier of the notifier, only use [a-z0-9_]
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getID(): string {
+		return 'comments';
+	}
+
+	/**
+	 * Human readable name describing the notifier
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getName(): string {
+		return $this->l10nFactory->get('comments')->t('Comments');
+	}
+
+	/**
 	 * @param INotification $notification
 	 * @param string $languageCode The code of the language that should be used to prepare the notification
 	 * @return INotification
 	 * @throws \InvalidArgumentException When the notification was not prepared by a notifier
+	 * @throws AlreadyProcessedException When the notification is not needed anymore and should be deleted
+	 * @since 9.0.0
 	 */
-	public function prepare(INotification $notification, $languageCode) {
+	public function prepare(INotification $notification, string $languageCode): INotification {
 		if($notification->getApp() !== 'comments') {
 			throw new \InvalidArgumentException();
 		}
@@ -101,7 +124,7 @@ class Notifier implements INotifier {
 				$userFolder = $this->rootFolder->getUserFolder($notification->getUser());
 				$nodes = $userFolder->getById((int)$parameters[1]);
 				if(empty($nodes)) {
-					throw new \InvalidArgumentException('Cannot resolve file ID to node instance');
+					throw new AlreadyProcessedException();
 				}
 				$node = $nodes[0];
 
diff --git a/apps/comments/tests/Unit/Controller/NotificationsTest.php b/apps/comments/tests/Unit/Controller/NotificationsTest.php
index c5209b9f9d5..9df81f01b02 100644
--- a/apps/comments/tests/Unit/Controller/NotificationsTest.php
+++ b/apps/comments/tests/Unit/Controller/NotificationsTest.php
@@ -117,6 +117,9 @@ class NotificationsTest extends TestCase {
 		$comment->expects($this->any())
 			->method('getObjectType')
 			->willReturn('files');
+		$comment->expects($this->any())
+			->method('getId')
+			->willReturn('1234');
 
 		$this->commentsManager->expects($this->any())
 			->method('get')
@@ -192,6 +195,9 @@ class NotificationsTest extends TestCase {
 		$comment->expects($this->any())
 			->method('getObjectType')
 			->willReturn('files');
+		$comment->expects($this->any())
+			->method('getId')
+			->willReturn('1234');
 
 		$this->commentsManager->expects($this->any())
 			->method('get')
diff --git a/apps/comments/tests/Unit/Notification/ListenerTest.php b/apps/comments/tests/Unit/Notification/ListenerTest.php
index d6f83262f30..e31b227bce8 100644
--- a/apps/comments/tests/Unit/Notification/ListenerTest.php
+++ b/apps/comments/tests/Unit/Notification/ListenerTest.php
@@ -91,6 +91,9 @@ class ListenerTest extends TestCase {
 				[ 'type' => 'user', 'id' => '23452-4333-54353-2342'],
 				[ 'type' => 'user', 'id' => 'yolo'],
 			]);
+		$comment->expects($this->atLeastOnce())
+			->method('getId')
+			->willReturn('1234');
 
 		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder(CommentsEvent::class)
@@ -186,6 +189,9 @@ class ListenerTest extends TestCase {
 		$comment->expects($this->once())
 			->method('getMentions')
 			->willReturn([[ 'type' => 'user', 'id' => 'foobar']]);
+		$comment->expects($this->atLeastOnce())
+			->method('getId')
+			->willReturn('1234');
 
 		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder(CommentsEvent::class)
diff --git a/apps/comments/tests/Unit/Notification/NotifierTest.php b/apps/comments/tests/Unit/Notification/NotifierTest.php
index 6eceed44919..d008c4c3121 100644
--- a/apps/comments/tests/Unit/Notification/NotifierTest.php
+++ b/apps/comments/tests/Unit/Notification/NotifierTest.php
@@ -195,6 +195,9 @@ class NotifierTest extends TestCase {
 			->expects($this->any())
 			->method('getMentions')
 			->willReturn([['type' => 'user', 'id' => 'you']]);
+		$this->comment->expects($this->atLeastOnce())
+			->method('getId')
+			->willReturn('1234');
 
 		$this->commentsManager
 			->expects($this->once())
@@ -539,7 +542,7 @@ class NotifierTest extends TestCase {
 	}
 
 	/**
-	 * @expectedException \InvalidArgumentException
+	 * @expectedException \OCP\Notification\AlreadyProcessedException
 	 */
 	public function testPrepareUnresolvableFileID() {
 		$displayName = 'Huraga';
diff --git a/apps/federatedfilesharing/appinfo/app.php b/apps/federatedfilesharing/appinfo/app.php
index f941987365f..6b273faca80 100644
--- a/apps/federatedfilesharing/appinfo/app.php
+++ b/apps/federatedfilesharing/appinfo/app.php
@@ -29,15 +29,7 @@ $app = new \OCA\FederatedFileSharing\AppInfo\Application();
 $eventDispatcher = \OC::$server->getEventDispatcher();
 
 $manager = \OC::$server->getNotificationManager();
-$manager->registerNotifier(function() {
-	return \OC::$server->query(Notifier::class);
-}, function() {
-	$l = \OC::$server->getL10N('files_sharing');
-	return [
-		'id' => 'files_sharing',
-		'name' => $l->t('Federated sharing'),
-	];
-});
+$manager->registerNotifierService(Notifier::class);
 
 $federatedShareProvider = $app->getFederatedShareProvider();
 
diff --git a/apps/federatedfilesharing/lib/Notifier.php b/apps/federatedfilesharing/lib/Notifier.php
index 03491f20af6..02a46d65bba 100644
--- a/apps/federatedfilesharing/lib/Notifier.php
+++ b/apps/federatedfilesharing/lib/Notifier.php
@@ -60,12 +60,32 @@ class Notifier implements INotifier {
 	}
 
 	/**
+	 * Identifier of the notifier, only use [a-z0-9_]
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getID(): string {
+		return 'federatedfilesharing';
+	}
+
+	/**
+	 * Human readable name describing the notifier
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getName(): string {
+		return $this->factory->get('federatedfilesharing')->t('Federated sharing');
+	}
+
+	/**
 	 * @param INotification $notification
 	 * @param string $languageCode The code of the language that should be used to prepare the notification
 	 * @return INotification
 	 * @throws \InvalidArgumentException
 	 */
-	public function prepare(INotification $notification, $languageCode) {
+	public function prepare(INotification $notification, string $languageCode): INotification {
 		if ($notification->getApp() !== 'files_sharing') {
 			// Not my app => throw
 			throw new \InvalidArgumentException();
diff --git a/apps/twofactor_backupcodes/lib/AppInfo/Application.php b/apps/twofactor_backupcodes/lib/AppInfo/Application.php
index 041af037067..735ee091bd1 100644
--- a/apps/twofactor_backupcodes/lib/AppInfo/Application.php
+++ b/apps/twofactor_backupcodes/lib/AppInfo/Application.php
@@ -74,15 +74,7 @@ class Application extends App {
 		$container = $this->getContainer();
 		/** @var IManager $manager */
 		$manager = $container->query(IManager::class);
-		$manager->registerNotifier(
-			function() use ($container) {
-				return $container->query(Notifier::class);
-			},
-			function () use ($container) {
-				$l = $container->query(IL10N::class);
-				return ['id' => 'twofactor_backupcodes', 'name' => $l->t('Second-factor backup codes')];
-			}
-		);
+		$manager->registerNotifierService(Notifier::class);
 	}
 
 	public function deleteUser($params) {
diff --git a/apps/twofactor_backupcodes/lib/Notifications/Notifier.php b/apps/twofactor_backupcodes/lib/Notifications/Notifier.php
index e4e2bcebb68..ab83d8ee2de 100644
--- a/apps/twofactor_backupcodes/lib/Notifications/Notifier.php
+++ b/apps/twofactor_backupcodes/lib/Notifications/Notifier.php
@@ -42,7 +42,27 @@ class Notifier implements INotifier {
 		$this->url = $url;
 	}
 
-	public function prepare(INotification $notification, $languageCode) {
+	/**
+	 * Identifier of the notifier, only use [a-z0-9_]
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getID(): string {
+		return 'twofactor_backupcodes';
+	}
+
+	/**
+	 * Human readable name describing the notifier
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getName(): string {
+		return $this->factory->get('twofactor_backupcodes')->t('Second-factor backup codes');
+	}
+
+	public function prepare(INotification $notification, string $languageCode): INotification {
 		if ($notification->getApp() !== 'twofactor_backupcodes') {
 			// Not my app => throw
 			throw new \InvalidArgumentException();
@@ -70,5 +90,4 @@ class Notifier implements INotifier {
 				throw new \InvalidArgumentException();
 		}
 	}
-
 }
diff --git a/apps/updatenotification/lib/AppInfo/Application.php b/apps/updatenotification/lib/AppInfo/Application.php
index 14512bae838..6a1bf719469 100644
--- a/apps/updatenotification/lib/AppInfo/Application.php
+++ b/apps/updatenotification/lib/AppInfo/Application.php
@@ -71,14 +71,6 @@ class Application extends App {
 
 	public function registerNotifier() {
 		$notificationsManager = $this->getContainer()->getServer()->getNotificationManager();
-		$notificationsManager->registerNotifier(function() {
-			return  $this->getContainer()->query(Notifier::class);
-		}, function() {
-			$l = $this->getContainer()->getServer()->getL10N('updatenotification');
-			return [
-				'id' => 'updatenotification',
-				'name' => $l->t('Update notifications'),
-			];
-		});
+		$notificationsManager->registerNotifierService(Notifier::class);
 	}
 }
diff --git a/apps/updatenotification/lib/Notification/Notifier.php b/apps/updatenotification/lib/Notification/Notifier.php
index 4e3a30f225e..44fe91c63d5 100644
--- a/apps/updatenotification/lib/Notification/Notifier.php
+++ b/apps/updatenotification/lib/Notification/Notifier.php
@@ -31,6 +31,7 @@ use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
+use OCP\Notification\AlreadyProcessedException;
 use OCP\Notification\IManager;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
@@ -80,13 +81,34 @@ class Notifier implements INotifier {
 	}
 
 	/**
+	 * Identifier of the notifier, only use [a-z0-9_]
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getID(): string {
+		return 'updatenotification';
+	}
+
+	/**
+	 * Human readable name describing the notifier
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getName(): string {
+		return $this->l10NFactory->get('updatenotification')->t('Update notifications');
+	}
+
+	/**
 	 * @param INotification $notification
 	 * @param string $languageCode The code of the language that should be used to prepare the notification
 	 * @return INotification
 	 * @throws \InvalidArgumentException When the notification was not prepared by a notifier
+	 * @throws AlreadyProcessedException When the notification is not needed anymore and should be deleted
 	 * @since 9.0.0
 	 */
-	public function prepare(INotification $notification, $languageCode): INotification {
+	public function prepare(INotification $notification, string $languageCode): INotification {
 		if ($notification->getApp() !== 'updatenotification') {
 			throw new \InvalidArgumentException('Unknown app id');
 		}
@@ -142,12 +164,11 @@ class Notifier implements INotifier {
 	 *
 	 * @param INotification $notification
 	 * @param string $installedVersion
-	 * @throws \InvalidArgumentException When the update is already installed
+	 * @throws AlreadyProcessedException When the update is already installed
 	 */
 	protected function updateAlreadyInstalledCheck(INotification $notification, $installedVersion) {
 		if (version_compare($notification->getObjectId(), $installedVersion, '<=')) {
-			$this->notificationManager->markProcessed($notification);
-			throw new \InvalidArgumentException('Update already installed');
+			throw new AlreadyProcessedException();
 		}
 	}
 
diff --git a/apps/updatenotification/tests/Notification/NotifierTest.php b/apps/updatenotification/tests/Notification/NotifierTest.php
index b1ddf7b478d..f97ff776c93 100644
--- a/apps/updatenotification/tests/Notification/NotifierTest.php
+++ b/apps/updatenotification/tests/Notification/NotifierTest.php
@@ -30,6 +30,7 @@ use OCP\IGroupManager;
 use OCP\IURLGenerator;
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
+use OCP\Notification\AlreadyProcessedException;
 use OCP\Notification\IManager;
 use OCP\Notification\INotification;
 use Test\TestCase;
@@ -112,21 +113,12 @@ class NotifierTest extends TestCase {
 			->method('getObjectId')
 			->willReturn($versionNotification);
 
-		if ($exception) {
-			$this->notificationManager->expects($this->once())
-				->method('markProcessed')
-				->with($notification);
-		} else {
-			$this->notificationManager->expects($this->never())
-				->method('markProcessed');
-		}
-
 		try {
 			self::invokePrivate($notifier, 'updateAlreadyInstalledCheck', [$notification, $versionInstalled]);
 			$this->assertFalse($exception);
 		} catch (\Exception $e) {
 			$this->assertTrue($exception);
-			$this->assertInstanceOf('InvalidArgumentException', $e);
+			$this->assertInstanceOf(AlreadyProcessedException::class, $e);
 		}
 	}
 }
diff --git a/apps/user_ldap/appinfo/app.php b/apps/user_ldap/appinfo/app.php
index 5afd928301a..c6e1208e6ea 100644
--- a/apps/user_ldap/appinfo/app.php
+++ b/apps/user_ldap/appinfo/app.php
@@ -42,17 +42,7 @@ if(count($configPrefixes) > 0) {
 	$ldapWrapper = new OCA\User_LDAP\LDAP();
 	$ocConfig = \OC::$server->getConfig();
 	$notificationManager = \OC::$server->getNotificationManager();
-	$notificationManager->registerNotifier(function() {
-		return new \OCA\User_LDAP\Notification\Notifier(
-			\OC::$server->getL10NFactory()
-		);
-	}, function() {
-		$l = \OC::$server->getL10N('user_ldap');
-		return [
-			'id' => 'user_ldap',
-			'name' => $l->t('LDAP user and group backend'),
-		];
-	});
+	$notificationManager->registerNotifierService(\OCA\User_LDAP\Notification\Notifier::class);
 	$userSession = \OC::$server->getUserSession();
 
 	$userPluginManager = \OC::$server->query('LDAPUserPluginManager');
diff --git a/apps/user_ldap/lib/Notification/Notifier.php b/apps/user_ldap/lib/Notification/Notifier.php
index 34625a3bef4..2c89743fa72 100644
--- a/apps/user_ldap/lib/Notification/Notifier.php
+++ b/apps/user_ldap/lib/Notification/Notifier.php
@@ -43,12 +43,32 @@ class Notifier implements INotifier {
 	}
 
 	/**
+	 * Identifier of the notifier, only use [a-z0-9_]
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getID(): string {
+		return 'user_ldap';
+	}
+
+	/**
+	 * Human readable name describing the notifier
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getName(): string {
+		return $this->l10nFactory->get('user_ldap')->t('LDAP User backend');
+	}
+
+	/**
 	 * @param INotification $notification
 	 * @param string $languageCode The code of the language that should be used to prepare the notification
 	 * @return INotification
 	 * @throws \InvalidArgumentException When the notification was not prepared by a notifier
 	 */
-	public function prepare(INotification $notification, $languageCode) {
+	public function prepare(INotification $notification, string $languageCode): INotification {
 		if ($notification->getApp() !== 'user_ldap') {
 			// Not my app => throw
 			throw new \InvalidArgumentException();