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

Notifications overhaul

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() {

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;
 
@@ -66,13 +67,35 @@ class Notifier implements INotifier {
 		$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 @@ 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];
 

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')

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)

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';

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();
 

apps/federatedfilesharing/lib/Notifier.php

@@ -59,13 +59,33 @@ class Notifier implements INotifier {
 		$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 @@ 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) {

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();
 		}
 	}
-
 }

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);
 	}
 }

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;
@@ -79,14 +80,35 @@ class Notifier implements INotifier {
 		$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 @@ 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();
 		}
 	}
 

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);
 		}
 	}
 }

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');

apps/user_ldap/lib/Notification/Notifier.php

@@ -42,13 +42,33 @@ class Notifier implements INotifier {
 		$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();

core/Application.php

@@ -65,24 +65,8 @@ class Application extends App {
 		$eventDispatcher = $server->query(IEventDispatcher::class);
 
 		$notificationManager = $server->getNotificationManager();
-		$notificationManager->registerNotifier(function () use ($server) {
-			return new RemoveLinkSharesNotifier(
-				$server->getL10NFactory()
-			);
-		}, function () {
-			return [
-				'id' => 'core',
-				'name' => 'core',
-			];
-		});
-		$notificationManager->registerNotifier(function () use ($server) {
-			return $server->query(AuthenticationNotifier::class);
-		}, function () {
-			return [
-				'id' => 'auth',
-				'name' => 'authentication notifier',
-			];
-		});
+		$notificationManager->registerNotifierService(RemoveLinkSharesNotifier::class);
+		$notificationManager->registerNotifierService(AuthenticationNotifier::class);
 
 		$eventDispatcher->addListener(IDBConnection::CHECK_MISSING_INDEXES_EVENT,
 			function (GenericEvent $event) use ($container) {

core/Notification/RemoveLinkSharesNotifier.php

@@ -36,7 +36,27 @@ class RemoveLinkSharesNotifier implements INotifier {
 		$this->l10nFactory = $factory;
 	}
 
-	public function prepare(INotification $notification, $languageCode): INotification {
+	/**
+	 * Identifier of the notifier, only use [a-z0-9_]
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getID(): string {
+		return 'core';
+	}
+
+	/**
+	 * Human readable name describing the notifier
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getName(): string {
+		return $this->l10nFactory->get('core')->t('Nextcloud Server');
+	}
+
+	public function prepare(INotification $notification, string $languageCode): INotification {
 		if($notification->getApp() !== 'core') {
 			throw new \InvalidArgumentException();
 		}
@@ -51,5 +71,4 @@ class RemoveLinkSharesNotifier implements INotifier {
 
 		throw new \InvalidArgumentException('Invalid subject');
 	}
-
 }

lib/composer/composer/autoload_classmap.php

@@ -347,6 +347,7 @@ return array(
     'OCP\\Migration\\IOutput' => $baseDir . '/lib/public/Migration/IOutput.php',
     'OCP\\Migration\\IRepairStep' => $baseDir . '/lib/public/Migration/IRepairStep.php',
     'OCP\\Migration\\SimpleMigrationStep' => $baseDir . '/lib/public/Migration/SimpleMigrationStep.php',
+    'OCP\\Notification\\AlreadyProcessedException' => $baseDir . '/lib/public/Notification/AlreadyProcessedException.php',
     'OCP\\Notification\\IAction' => $baseDir . '/lib/public/Notification/IAction.php',
     'OCP\\Notification\\IApp' => $baseDir . '/lib/public/Notification/IApp.php',
     'OCP\\Notification\\IManager' => $baseDir . '/lib/public/Notification/IManager.php',

lib/composer/composer/autoload_static.php

@@ -381,6 +381,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\Migration\\IOutput' => __DIR__ . '/../../..' . '/lib/public/Migration/IOutput.php',
         'OCP\\Migration\\IRepairStep' => __DIR__ . '/../../..' . '/lib/public/Migration/IRepairStep.php',
         'OCP\\Migration\\SimpleMigrationStep' => __DIR__ . '/../../..' . '/lib/public/Migration/SimpleMigrationStep.php',
+        'OCP\\Notification\\AlreadyProcessedException' => __DIR__ . '/../../..' . '/lib/public/Notification/AlreadyProcessedException.php',
         'OCP\\Notification\\IAction' => __DIR__ . '/../../..' . '/lib/public/Notification/IAction.php',
         'OCP\\Notification\\IApp' => __DIR__ . '/../../..' . '/lib/public/Notification/IApp.php',
         'OCP\\Notification\\IManager' => __DIR__ . '/../../..' . '/lib/public/Notification/IManager.php',

lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php

@@ -61,7 +61,7 @@ class RemoteWipeNotificationsListener implements IEventListener {
 		$notification->setApp('auth')
 			->setUser($token->getUID())
 			->setDateTime($this->timeFactory->getDateTime())
-			->setObject('token', $token->getId())
+			->setObject('token', (string) $token->getId())
 			->setSubject($event, [
 				'name' => $token->getName(),
 			]);

lib/private/Authentication/Notifications/Notifier.php

@@ -42,7 +42,7 @@ class Notifier implements INotifier {
 	/**
 	 * @inheritDoc
 	 */
-	public function prepare(INotification $notification, $languageCode) {
+	public function prepare(INotification $notification, string $languageCode): INotification {
 		if ($notification->getApp() !== 'auth') {
 			// Not my app => throw
 			throw new InvalidArgumentException();
@@ -74,4 +74,23 @@ class Notifier implements INotifier {
 		}
 	}
 
+	/**
+	 * Identifier of the notifier, only use [a-z0-9_]
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getID(): string {
+		return 'auth';
+	}
+
+	/**
+	 * Human readable name describing the notifier
+	 *
+	 * @return string
+	 * @since 17.0.0
+	 */
+	public function getName(): string {
+		return $this->factory->get('lib')->t('Authentication');
+	}
 }