Merge pull request #15040 from nextcloud/feature/13980/push-for-delet…

…ed-notifications

Notifications overhaul

apps/comments/lib/AppInfo/Application.php

@@ -76,15 +76,7 @@ protected function registerDavEntity(EventDispatcherInterface $dispatcher) {
 	}
 
 	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() {

apps/comments/lib/Notification/Notifier.php

@@ -32,6 +32,7 @@
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
+use OCP\Notification\AlreadyProcessedException;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
 
@@ -66,13 +67,35 @@ public function __construct(
 		$this->userManager = $userManager;
 	}
 
+	/**
+	 * 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 @@ public function prepare(INotification $notification, $languageCode) {
 				$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];
 

apps/comments/tests/Unit/Controller/NotificationsTest.php

@@ -117,6 +117,9 @@ public function testViewSuccess() {
 		$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 @@ public function testViewNoFile() {
 		$comment->expects($this->any())
 			->method('getObjectType')
 			->willReturn('files');
+		$comment->expects($this->any())
+			->method('getId')
+			->willReturn('1234');
 
 		$this->commentsManager->expects($this->any())
 			->method('get')

apps/comments/tests/Unit/Notification/ListenerTest.php

@@ -91,6 +91,9 @@ public function testEvaluate($eventType, $notificationMethod) {
 				[ '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 @@ public function testEvaluateUserDoesNotExist() {
 		$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)

apps/comments/tests/Unit/Notification/NotifierTest.php

@@ -195,6 +195,9 @@ public function testPrepareSuccess() {
 			->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 @@ public function testPrepareNotFiles() {
 	}
 
 	/**
-	 * @expectedException \InvalidArgumentException
+	 * @expectedException \OCP\Notification\AlreadyProcessedException
 	 */
 	public function testPrepareUnresolvableFileID() {
 		$displayName = 'Huraga';

apps/federatedfilesharing/appinfo/app.php

@@ -29,15 +29,7 @@
 $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();
 

apps/federatedfilesharing/lib/Notifier.php

@@ -59,13 +59,33 @@ public function __construct(IFactory $factory, IManager $contactsManager, IURLGe
 		$this->cloudIdManager = $cloudIdManager;
 	}
 
+	/**
+	 * 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();

apps/twofactor_backupcodes/lib/AppInfo/Application.php

@@ -74,15 +74,7 @@ public function registerNotification() {
 		$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) {

apps/twofactor_backupcodes/lib/Notifications/Notifier.php

@@ -42,7 +42,27 @@ public function __construct(IFactory $factory, IURLGenerator $url) {
 		$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 @@ public function prepare(INotification $notification, $languageCode) {
 				throw new \InvalidArgumentException();
 		}
 	}
-
 }

apps/updatenotification/lib/AppInfo/Application.php

@@ -71,14 +71,6 @@ public function register() {
 
 	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);
 	}
 }

apps/updatenotification/lib/Notification/Notifier.php

@@ -31,6 +31,7 @@
 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;
@@ -79,14 +80,35 @@ public function __construct(IURLGenerator $url, IConfig $config, IManager $notif
 		$this->appVersions = $this->getAppVersions();
 	}
 
+	/**
+	 * 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 @@ public function prepare(INotification $notification, $languageCode): INotificati
 	 *
 	 * @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();
 		}
 	}
 

apps/updatenotification/tests/Notification/NotifierTest.php

@@ -30,6 +30,7 @@
 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 @@ public function testUpdateAlreadyInstalledCheck(string $versionNotification, str
 			->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);
 		}
 	}
 }

apps/user_ldap/appinfo/app.php

@@ -42,17 +42,7 @@
 	$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');

apps/user_ldap/lib/Notification/Notifier.php

@@ -42,13 +42,33 @@ public function __construct(\OCP\L10N\IFactory $l10nFactory) {
 		$this->l10nFactory = $l10nFactory;
 	}
 
+	/**
+	 * 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();