diff --git a/apps/comments/lib/AppInfo/Application.php b/apps/comments/lib/AppInfo/Application.php index b44c1c519c..916345e4a5 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 2132f05ef8..e8a7ade820 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; @@ -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]; diff --git a/apps/comments/tests/Unit/Controller/NotificationsTest.php b/apps/comments/tests/Unit/Controller/NotificationsTest.php index c5209b9f9d..9df81f01b0 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 d6f83262f3..e31b227bce 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 6eceed4491..d008c4c312 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 f941987365..6b273faca8 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 03491f20af..02a46d65bb 100644 --- a/apps/federatedfilesharing/lib/Notifier.php +++ b/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(); diff --git a/apps/twofactor_backupcodes/lib/AppInfo/Application.php b/apps/twofactor_backupcodes/lib/AppInfo/Application.php index 041af03706..735ee091bd 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 e4e2bcebb6..ab83d8ee2d 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 14512bae83..6a1bf71946 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 4e3a30f225..44fe91c63d 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; @@ -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(); } } diff --git a/apps/updatenotification/tests/Notification/NotifierTest.php b/apps/updatenotification/tests/Notification/NotifierTest.php index b1ddf7b478..f97ff776c9 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 5afd928301..c6e1208e6e 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 34625a3bef..2c89743fa7 100644 --- a/apps/user_ldap/lib/Notification/Notifier.php +++ b/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(); diff --git a/core/Application.php b/core/Application.php index 9655a8e1a4..7949cfd663 100644 --- a/core/Application.php +++ b/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) { diff --git a/core/Notification/RemoveLinkSharesNotifier.php b/core/Notification/RemoveLinkSharesNotifier.php index b77846c847..b2e73aee23 100644 --- a/core/Notification/RemoveLinkSharesNotifier.php +++ b/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'); } - } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1fa9f6243f..d8d9227cb0 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/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', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 58c1b031f9..043846e04d 100644 --- a/lib/composer/composer/autoload_static.php +++ b/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', diff --git a/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php b/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php index ffddddff1d..57213a8a19 100644 --- a/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php +++ b/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(), ]); diff --git a/lib/private/Authentication/Notifications/Notifier.php b/lib/private/Authentication/Notifications/Notifier.php index 0aafc115b2..f76bb077a3 100644 --- a/lib/private/Authentication/Notifications/Notifier.php +++ b/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'); + } } diff --git a/lib/private/Notification/Action.php b/lib/private/Notification/Action.php index 8dfeecb98c..f6d6a33358 100644 --- a/lib/private/Notification/Action.php +++ b/lib/private/Notification/Action.php @@ -1,4 +1,5 @@ label = ''; $this->labelParsed = ''; @@ -62,8 +60,8 @@ class Action implements IAction { * @throws \InvalidArgumentException if the label is invalid * @since 8.2.0 */ - public function setLabel($label) { - if (!is_string($label) || $label === '' || isset($label[32])) { + public function setLabel(string $label): IAction { + if ($label === '' || isset($label[32])) { throw new \InvalidArgumentException('The given label is invalid'); } $this->label = $label; @@ -74,7 +72,7 @@ class Action implements IAction { * @return string * @since 8.2.0 */ - public function getLabel() { + public function getLabel(): string { return $this->label; } @@ -84,8 +82,8 @@ class Action implements IAction { * @throws \InvalidArgumentException if the label is invalid * @since 8.2.0 */ - public function setParsedLabel($label) { - if (!is_string($label) || $label === '') { + public function setParsedLabel(string $label): IAction { + if ($label === '') { throw new \InvalidArgumentException('The given parsed label is invalid'); } $this->labelParsed = $label; @@ -96,21 +94,16 @@ class Action implements IAction { * @return string * @since 8.2.0 */ - public function getParsedLabel() { + public function getParsedLabel(): string { return $this->labelParsed; } /** * @param $primary bool * @return $this - * @throws \InvalidArgumentException if $primary is invalid * @since 9.0.0 */ - public function setPrimary($primary) { - if (!is_bool($primary)) { - throw new \InvalidArgumentException('The given primary option is invalid'); - } - + public function setPrimary(bool $primary): IAction { $this->primary = $primary; return $this; } @@ -119,7 +112,7 @@ class Action implements IAction { * @return bool * @since 9.0.0 */ - public function isPrimary() { + public function isPrimary(): bool { return $this->primary; } @@ -130,11 +123,17 @@ class Action implements IAction { * @throws \InvalidArgumentException if the link is invalid * @since 8.2.0 */ - public function setLink($link, $requestType) { - if (!is_string($link) || $link === '' || isset($link[256])) { + public function setLink(string $link, string $requestType): IAction { + if ($link === '' || isset($link[256])) { throw new \InvalidArgumentException('The given link is invalid'); } - if (!in_array($requestType, ['GET', 'POST', 'PUT', 'DELETE'], true)) { + if (!in_array($requestType, [ + self::TYPE_GET, + self::TYPE_POST, + self::TYPE_PUT, + self::TYPE_DELETE, + self::TYPE_WEB, + ], true)) { throw new \InvalidArgumentException('The given request type is invalid'); } $this->link = $link; @@ -146,7 +145,7 @@ class Action implements IAction { * @return string * @since 8.2.0 */ - public function getLink() { + public function getLink(): string { return $this->link; } @@ -154,21 +153,21 @@ class Action implements IAction { * @return string * @since 8.2.0 */ - public function getRequestType() { + public function getRequestType(): string { return $this->requestType; } /** * @return bool */ - public function isValid() { + public function isValid(): bool { return $this->label !== '' && $this->link !== ''; } /** * @return bool */ - public function isValidParsed() { + public function isValidParsed(): bool { return $this->labelParsed !== '' && $this->link !== ''; } } diff --git a/lib/private/Notification/Manager.php b/lib/private/Notification/Manager.php index 4c3f7a2453..fae0a66491 100644 --- a/lib/private/Notification/Manager.php +++ b/lib/private/Notification/Manager.php @@ -1,4 +1,5 @@ validator = $validator; + $this->logger = $logger; $this->apps = []; $this->notifiers = []; - $this->notifiersInfo = []; - $this->appsClosures = []; - $this->notifiersClosures = []; - $this->notifiersInfoClosures = []; + $this->appClasses = []; + $this->notifierClasses = []; $this->preparingPushNotification = false; } - /** - * @param \Closure $service The service must implement IApp, otherwise a + * @param string $appClass The service must implement IApp, otherwise a * \InvalidArgumentException is thrown later - * @since 8.2.0 + * @since 17.0.0 */ - public function registerApp(\Closure $service) { - $this->appsClosures[] = $service; - $this->apps = []; + public function registerApp(string $appClass): void { + $this->appClasses[] = $appClass; } /** @@ -87,78 +78,93 @@ class Manager implements IManager { * \InvalidArgumentException is thrown later * @param \Closure $info An array with the keys 'id' and 'name' containing * the app id and the app name + * @deprecated 17.0.0 use registerNotifierService instead. * @since 8.2.0 - Parameter $info was added in 9.0.0 */ public function registerNotifier(\Closure $service, \Closure $info) { - $this->notifiersClosures[] = $service; - $this->notifiersInfoClosures[] = $info; - $this->notifiers = []; - $this->notifiersInfo = []; + $infoData = $info(); + $this->logger->logException(new \InvalidArgumentException( + 'Notifier ' . $infoData['name'] . ' (id: ' . $infoData['id'] . ') is not considered because it is using the old way to register.' + )); + } + + /** + * @param string $notifierService The service must implement INotifier, otherwise a + * \InvalidArgumentException is thrown later + * @since 17.0.0 + */ + public function registerNotifierService(string $notifierService): void { + $this->notifierClasses[] = $notifierService; } /** * @return IApp[] */ protected function getApps(): array { - if (!empty($this->apps)) { + if (empty($this->appClasses)) { return $this->apps; } - $this->apps = []; - foreach ($this->appsClosures as $closure) { - $app = $closure(); - if (!($app instanceof IApp)) { - throw new \InvalidArgumentException('The given notification app does not implement the IApp interface'); + foreach ($this->appClasses as $appClass) { + try { + $app = \OC::$server->query($appClass); + } catch (QueryException $e) { + $this->logger->logException($e, [ + 'message' => 'Failed to load notification app class: ' . $appClass, + 'app' => 'notifications', + ]); + continue; } + + if (!($app instanceof IApp)) { + $this->logger->error('Notification app class ' . $appClass . ' is not implementing ' . IApp::class, [ + 'app' => 'notifications', + ]); + continue; + } + $this->apps[] = $app; } + $this->appClasses = []; + return $this->apps; } /** * @return INotifier[] */ - protected function getNotifiers(): array { - if (!empty($this->notifiers)) { + public function getNotifiers(): array { + if (empty($this->notifierClasses)) { return $this->notifiers; } - $this->notifiers = []; - foreach ($this->notifiersClosures as $closure) { - $notifier = $closure(); - if (!($notifier instanceof INotifier)) { - throw new \InvalidArgumentException('The given notifier does not implement the INotifier interface'); + foreach ($this->notifierClasses as $notifierClass) { + try { + $notifier = \OC::$server->query($notifierClass); + } catch (QueryException $e) { + $this->logger->logException($e, [ + 'message' => 'Failed to load notification notifier class: ' . $notifierClass, + 'app' => 'notifications', + ]); + continue; } + + if (!($notifier instanceof INotifier)) { + $this->logger->error('Notification notifier class ' . $notifierClass . ' is not implementing ' . INotifier::class, [ + 'app' => 'notifications', + ]); + continue; + } + $this->notifiers[] = $notifier; } + $this->notifierClasses = []; + return $this->notifiers; } - /** - * @return array[] - */ - public function listNotifiers(): array { - if (!empty($this->notifiersInfo)) { - return $this->notifiersInfo; - } - - $this->notifiersInfo = []; - foreach ($this->notifiersInfoClosures as $closure) { - $notifier = $closure(); - if (!\is_array($notifier) || \count($notifier) !== 2 || !isset($notifier['id'], $notifier['name'])) { - throw new \InvalidArgumentException('The given notifier information is invalid'); - } - if (isset($this->notifiersInfo[$notifier['id']])) { - throw new \InvalidArgumentException('The given notifier ID ' . $notifier['id'] . ' is already in use'); - } - $this->notifiersInfo[$notifier['id']] = $notifier['name']; - } - - return $this->notifiersInfo; - } - /** * @return INotification * @since 8.2.0 @@ -172,14 +178,14 @@ class Manager implements IManager { * @since 8.2.0 */ public function hasNotifiers(): bool { - return !empty($this->notifiersClosures); + return !empty($this->notifiers) || !empty($this->notifierClasses); } /** * @param bool $preparingPushNotification * @since 14.0.0 */ - public function setPreparingPushNotification($preparingPushNotification) { + public function setPreparingPushNotification(bool $preparingPushNotification): void { $this->preparingPushNotification = $preparingPushNotification; } @@ -196,7 +202,7 @@ class Manager implements IManager { * @throws \InvalidArgumentException When the notification is not valid * @since 8.2.0 */ - public function notify(INotification $notification) { + public function notify(INotification $notification): void { if (!$notification->isValid()) { throw new \InvalidArgumentException('The given notification is invalid'); } @@ -211,14 +217,35 @@ class Manager implements IManager { } } + /** + * 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 'core'; + } + /** * @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 8.2.0 */ - public function prepare(INotification $notification, $languageCode): INotification { + public function prepare(INotification $notification, string $languageCode): INotification { $notifiers = $this->getNotifiers(); foreach ($notifiers as $notifier) { @@ -226,6 +253,9 @@ class Manager implements IManager { $notification = $notifier->prepare($notification, $languageCode); } catch (\InvalidArgumentException $e) { continue; + } catch (AlreadyProcessedException $e) { + $this->markProcessed($notification); + throw new \InvalidArgumentException('The given notification has been processed'); } if (!($notification instanceof INotification) || !$notification->isValidParsed()) { @@ -243,7 +273,7 @@ class Manager implements IManager { /** * @param INotification $notification */ - public function markProcessed(INotification $notification) { + public function markProcessed(INotification $notification): void { $apps = $this->getApps(); foreach ($apps as $app) { diff --git a/lib/private/Notification/Notification.php b/lib/private/Notification/Notification.php index 47f415d15d..e64c059db2 100644 --- a/lib/private/Notification/Notification.php +++ b/lib/private/Notification/Notification.php @@ -1,5 +1,5 @@ richValidator = $richValidator; $this->app = ''; @@ -134,8 +129,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the app id is invalid * @since 8.2.0 */ - public function setApp(string $app) { - if (trim($app) === '' || isset($app[32])) { + public function setApp(string $app): INotification { + if ($app === '' || isset($app[32])) { throw new \InvalidArgumentException('The given app name is invalid'); } $this->app = $app; @@ -146,7 +141,7 @@ class Notification implements INotification { * @return string * @since 8.2.0 */ - public function getApp() { + public function getApp(): string { return $this->app; } @@ -156,8 +151,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the user id is invalid * @since 8.2.0 */ - public function setUser(string $user) { - if (trim($user) === '' || isset($user[64])) { + public function setUser(string $user): INotification { + if ($user === '' || isset($user[64])) { throw new \InvalidArgumentException('The given user id is invalid'); } $this->user = $user; @@ -168,7 +163,7 @@ class Notification implements INotification { * @return string * @since 8.2.0 */ - public function getUser() { + public function getUser(): string { return $this->user; } @@ -178,7 +173,7 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the $dateTime is invalid * @since 9.0.0 */ - public function setDateTime(\DateTime $dateTime) { + public function setDateTime(\DateTime $dateTime): INotification { if ($dateTime->getTimestamp() === 0) { throw new \InvalidArgumentException('The given date time is invalid'); } @@ -190,7 +185,7 @@ class Notification implements INotification { * @return \DateTime * @since 9.0.0 */ - public function getDateTime() { + public function getDateTime(): \DateTime { return $this->dateTime; } @@ -201,13 +196,13 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the object type or id is invalid * @since 8.2.0 - 9.0.0: Type of $id changed to string */ - public function setObject(string $type, $id) { - if (trim($type) === '' || isset($type[64])) { + public function setObject(string $type, string $id): INotification { + if ($type === '' || isset($type[64])) { throw new \InvalidArgumentException('The given object type is invalid'); } $this->objectType = $type; - if (!is_int($id) && (!is_string($id) || $id === '' || isset($id[64]))) { + if ($id === '' || isset($id[64])) { throw new \InvalidArgumentException('The given object id is invalid'); } $this->objectId = (string) $id; @@ -218,7 +213,7 @@ class Notification implements INotification { * @return string * @since 8.2.0 */ - public function getObjectType() { + public function getObjectType(): string { return $this->objectType; } @@ -226,7 +221,7 @@ class Notification implements INotification { * @return string * @since 8.2.0 - 9.0.0: Return type changed to string */ - public function getObjectId() { + public function getObjectId(): string { return $this->objectId; } @@ -237,8 +232,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the subject or parameters are invalid * @since 8.2.0 */ - public function setSubject(string $subject, array $parameters = []) { - if (trim($subject) === '' || isset($subject[64])) { + public function setSubject(string $subject, array $parameters = []): INotification { + if ($subject === '' || isset($subject[64])) { throw new \InvalidArgumentException('The given subject is invalid'); } @@ -252,15 +247,15 @@ class Notification implements INotification { * @return string * @since 8.2.0 */ - public function getSubject() { + public function getSubject(): string { return $this->subject; } /** - * @return string[] + * @return array * @since 8.2.0 */ - public function getSubjectParameters() { + public function getSubjectParameters(): array { return $this->subjectParameters; } @@ -270,8 +265,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the subject is invalid * @since 8.2.0 */ - public function setParsedSubject(string $subject) { - if (trim($subject) === '') { + public function setParsedSubject(string $subject): INotification { + if ($subject === '') { throw new \InvalidArgumentException('The given parsed subject is invalid'); } $this->subjectParsed = $subject; @@ -282,7 +277,7 @@ class Notification implements INotification { * @return string * @since 8.2.0 */ - public function getParsedSubject() { + public function getParsedSubject(): string { return $this->subjectParsed; } @@ -293,8 +288,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the subject or parameters are invalid * @since 11.0.0 */ - public function setRichSubject(string $subject, array $parameters = []) { - if (trim($subject) === '') { + public function setRichSubject(string $subject, array $parameters = []): INotification { + if ($subject === '') { throw new \InvalidArgumentException('The given parsed subject is invalid'); } @@ -308,7 +303,7 @@ class Notification implements INotification { * @return string * @since 11.0.0 */ - public function getRichSubject() { + public function getRichSubject(): string { return $this->subjectRich; } @@ -316,7 +311,7 @@ class Notification implements INotification { * @return array[] * @since 11.0.0 */ - public function getRichSubjectParameters() { + public function getRichSubjectParameters(): array { return $this->subjectRichParameters; } @@ -327,8 +322,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the message or parameters are invalid * @since 8.2.0 */ - public function setMessage(string $message, array $parameters = []) { - if (trim($message) === '' || isset($message[64])) { + public function setMessage(string $message, array $parameters = []): INotification { + if ($message === '' || isset($message[64])) { throw new \InvalidArgumentException('The given message is invalid'); } @@ -342,15 +337,15 @@ class Notification implements INotification { * @return string * @since 8.2.0 */ - public function getMessage() { + public function getMessage(): string { return $this->message; } /** - * @return string[] + * @return array * @since 8.2.0 */ - public function getMessageParameters() { + public function getMessageParameters(): array { return $this->messageParameters; } @@ -360,8 +355,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the message is invalid * @since 8.2.0 */ - public function setParsedMessage(string $message) { - if (trim($message) === '') { + public function setParsedMessage(string $message): INotification { + if ($message === '') { throw new \InvalidArgumentException('The given parsed message is invalid'); } $this->messageParsed = $message; @@ -372,7 +367,7 @@ class Notification implements INotification { * @return string * @since 8.2.0 */ - public function getParsedMessage() { + public function getParsedMessage(): string { return $this->messageParsed; } @@ -383,8 +378,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the message or parameters are invalid * @since 11.0.0 */ - public function setRichMessage(string $message, array $parameters = []) { - if (trim($message) === '') { + public function setRichMessage(string $message, array $parameters = []): INotification { + if ($message === '') { throw new \InvalidArgumentException('The given parsed message is invalid'); } @@ -398,7 +393,7 @@ class Notification implements INotification { * @return string * @since 11.0.0 */ - public function getRichMessage() { + public function getRichMessage(): string { return $this->messageRich; } @@ -406,7 +401,7 @@ class Notification implements INotification { * @return array[] * @since 11.0.0 */ - public function getRichMessageParameters() { + public function getRichMessageParameters(): array { return $this->messageRichParameters; } @@ -416,8 +411,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the link is invalid * @since 8.2.0 */ - public function setLink(string $link) { - if (trim($link) === '' || isset($link[4000])) { + public function setLink(string $link): INotification { + if ($link === '' || isset($link[4000])) { throw new \InvalidArgumentException('The given link is invalid'); } $this->link = $link; @@ -428,7 +423,7 @@ class Notification implements INotification { * @return string * @since 8.2.0 */ - public function getLink() { + public function getLink(): string { return $this->link; } @@ -438,8 +433,8 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the icon is invalid * @since 11.0.0 */ - public function setIcon(string $icon) { - if (trim($icon) === '' || isset($icon[4000])) { + public function setIcon(string $icon): INotification { + if ($icon === '' || isset($icon[4000])) { throw new \InvalidArgumentException('The given icon is invalid'); } $this->icon = $icon; @@ -450,7 +445,7 @@ class Notification implements INotification { * @return string * @since 11.0.0 */ - public function getIcon() { + public function getIcon(): string { return $this->icon; } @@ -458,7 +453,7 @@ class Notification implements INotification { * @return IAction * @since 8.2.0 */ - public function createAction() { + public function createAction(): IAction { return new Action(); } @@ -468,7 +463,7 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the action is invalid * @since 8.2.0 */ - public function addAction(IAction $action) { + public function addAction(IAction $action): INotification { if (!$action->isValid()) { throw new \InvalidArgumentException('The given action is invalid'); } @@ -489,7 +484,7 @@ class Notification implements INotification { * @return IAction[] * @since 8.2.0 */ - public function getActions() { + public function getActions(): array { return $this->actions; } @@ -499,7 +494,7 @@ class Notification implements INotification { * @throws \InvalidArgumentException if the action is invalid * @since 8.2.0 */ - public function addParsedAction(IAction $action) { + public function addParsedAction(IAction $action): INotification { if (!$action->isValidParsed()) { throw new \InvalidArgumentException('The given parsed action is invalid'); } @@ -524,7 +519,7 @@ class Notification implements INotification { * @return IAction[] * @since 8.2.0 */ - public function getParsedActions() { + public function getParsedActions(): array { return $this->actionsParsed; } @@ -532,7 +527,7 @@ class Notification implements INotification { * @return bool * @since 8.2.0 */ - public function isValid() { + public function isValid(): bool { return $this->isValidCommon() && @@ -544,7 +539,7 @@ class Notification implements INotification { * @return bool * @since 8.2.0 */ - public function isValidParsed() { + public function isValidParsed(): bool { if ($this->getRichSubject() !== '' || !empty($this->getRichSubjectParameters())) { try { $this->richValidator->validate($this->getRichSubject(), $this->getRichSubjectParameters()); @@ -568,10 +563,7 @@ class Notification implements INotification { ; } - /** - * @return bool - */ - protected function isValidCommon() { + protected function isValidCommon(): bool { return $this->getApp() !== '' && diff --git a/lib/private/Server.php b/lib/private/Server.php index ccdb31c3e8..64180f1cd7 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -900,7 +900,8 @@ class Server extends ServerContainer implements IServerContainer { }); $this->registerService(\OCP\Notification\IManager::class, function (Server $c) { return new Manager( - $c->query(IValidator::class) + $c->query(IValidator::class), + $c->getLogger() ); }); $this->registerAlias('NotificationManager', \OCP\Notification\IManager::class); diff --git a/lib/public/Notification/AlreadyProcessedException.php b/lib/public/Notification/AlreadyProcessedException.php new file mode 100644 index 0000000000..0ccac25f0f --- /dev/null +++ b/lib/public/Notification/AlreadyProcessedException.php @@ -0,0 +1,38 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace OCP\Notification; + + +/** + * @since 17.0.0 + */ +class AlreadyProcessedException extends \RuntimeException { + + /** + * @since 17.0.0 + */ + public function __construct() { + parent::__construct('Notification is processed already'); + } + +} diff --git a/lib/public/Notification/IAction.php b/lib/public/Notification/IAction.php index 6f2b78e3a8..718a61b2f3 100644 --- a/lib/public/Notification/IAction.php +++ b/lib/public/Notification/IAction.php @@ -1,4 +1,5 @@ App Name + * @param string $notifierService The service must implement INotifier, otherwise a + * \InvalidArgumentException is thrown later + * @since 17.0.0 + */ + public function registerNotifierService(string $notifierService): void; + + /** + * @return INotifier[] * @since 9.0.0 */ - public function listNotifiers(); + public function getNotifiers(): array; /** * @return INotification * @since 9.0.0 */ - public function createNotification(); + public function createNotification(): INotification; /** * @return bool * @since 9.0.0 */ - public function hasNotifiers(); + public function hasNotifiers(): bool; /** * @param bool $preparingPushNotification * @since 14.0.0 */ - public function setPreparingPushNotification($preparingPushNotification); + public function setPreparingPushNotification(bool $preparingPushNotification): void; /** * @return bool * @since 14.0.0 */ - public function isPreparingPushNotification(); + public function isPreparingPushNotification(): bool; } diff --git a/lib/public/Notification/INotification.php b/lib/public/Notification/INotification.php index f7400e4778..7261409e35 100644 --- a/lib/public/Notification/INotification.php +++ b/lib/public/Notification/INotification.php @@ -1,5 +1,5 @@ method('getId')->willReturn(123); $notification->expects($this->once()) ->method('setObject') - ->with('token', 123) + ->with('token', '123') ->willReturnSelf(); $token->method('getName')->willReturn('Token 1'); $notification->expects($this->once()) @@ -132,7 +132,7 @@ class RemoteWipeNotificationListenerTests extends TestCase { $token->method('getId')->willReturn(123); $notification->expects($this->once()) ->method('setObject') - ->with('token', 123) + ->with('token', '123') ->willReturnSelf(); $token->method('getName')->willReturn('Token 1'); $notification->expects($this->once()) diff --git a/tests/lib/Notification/ActionTest.php b/tests/lib/Notification/ActionTest.php index 74c995280c..9160ea7a7c 100644 --- a/tests/lib/Notification/ActionTest.php +++ b/tests/lib/Notification/ActionTest.php @@ -55,14 +55,8 @@ class ActionTest extends TestCase { public function dataSetLabelInvalid() { return [ - [true], - [false], - [0], - [1], [''], [str_repeat('a', 33)], - [[]], - [[str_repeat('a', 33)]], ]; } @@ -96,13 +90,7 @@ class ActionTest extends TestCase { public function dataSetParsedLabelInvalid() { return [ - [true], - [false], - [0], - [1], [''], - [[]], - [[str_repeat('a', 33)]], ]; } @@ -140,23 +128,11 @@ class ActionTest extends TestCase { public function dataSetLinkInvalid() { return [ // Invalid link - [true, 'GET'], - [false, 'GET'], - [0, 'GET'], - [1, 'GET'], ['', 'GET'], [str_repeat('a', 257), 'GET'], - [[], 'GET'], - [[str_repeat('a', 257)], 'GET'], // Invalid type ['url', 'notGET'], - ['url', true], - ['url', false], - ['url', 0], - ['url', 1], - ['url', []], - ['url', ['GET']], ]; } @@ -188,27 +164,6 @@ class ActionTest extends TestCase { $this->assertSame($primary, $this->action->isPrimary()); } - public function dataSetPrimaryInvalid() { - return [ - [0], - [1], - [''], - [str_repeat('a', 257)], - [[]], - [[str_repeat('a', 257)]], - ]; - } - - /** - * @dataProvider dataSetPrimaryInvalid - * @param mixed $primary - * - * @expectedException \InvalidArgumentException - */ - public function testSetPrimaryInvalid($primary) { - $this->action->setPrimary($primary); - } - public function testIsValid() { $this->assertFalse($this->action->isValid()); $this->assertFalse($this->action->isValidParsed()); diff --git a/tests/lib/Notification/DummyApp.php b/tests/lib/Notification/DummyApp.php new file mode 100644 index 0000000000..680fb44d00 --- /dev/null +++ b/tests/lib/Notification/DummyApp.php @@ -0,0 +1,56 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace Test\Notification; + + +use OCP\Notification\IApp; +use OCP\Notification\INotification; + +class DummyApp implements IApp { + + /** + * @param INotification $notification + * @throws \InvalidArgumentException When the notification is not valid + * @since 9.0.0 + */ + public function notify(INotification $notification): void { + // TODO: Implement notify() method. + } + + /** + * @param INotification $notification + * @since 9.0.0 + */ + public function markProcessed(INotification $notification): void { + // TODO: Implement markProcessed() method. + } + + /** + * @param INotification $notification + * @return int + * @since 9.0.0 + */ + public function getCount(INotification $notification): int { + // TODO: Implement getCount() method. + } +} diff --git a/tests/lib/Notification/DummyNotifier.php b/tests/lib/Notification/DummyNotifier.php new file mode 100644 index 0000000000..849fb3c275 --- /dev/null +++ b/tests/lib/Notification/DummyNotifier.php @@ -0,0 +1,63 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * 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 + * along with this program. If not, see . + * + */ + +namespace Test\Notification; + + +use OCP\Notification\AlreadyProcessedException; +use OCP\Notification\INotification; +use OCP\Notification\INotifier; + +class DummyNotifier implements INotifier { + + /** + * Identifier of the notifier, only use [a-z0-9_] + * + * @return string + * @since 17.0.0 + */ + public function getID(): string { + // TODO: Implement getID() method. + } + + /** + * Human readable name describing the notifier + * + * @return string + * @since 17.0.0 + */ + public function getName(): string { + // TODO: Implement getName() method. + } + + /** + * @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, string $languageCode): INotification { + // TODO: Implement prepare() method. + } +} diff --git a/tests/lib/Notification/ManagerTest.php b/tests/lib/Notification/ManagerTest.php index cb6504b67e..259ac0beef 100644 --- a/tests/lib/Notification/ManagerTest.php +++ b/tests/lib/Notification/ManagerTest.php @@ -22,162 +22,72 @@ namespace Test\Notification; use OC\Notification\Manager; +use OCP\ILogger; use OCP\Notification\IApp; use OCP\Notification\IManager; use OCP\Notification\INotification; use OCP\Notification\INotifier; use OCP\RichObjectStrings\IValidator; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class ManagerTest extends TestCase { /** @var IManager */ protected $manager; + /** @var IValidator|MockObject */ + protected $validator; + /** @var ILogger|MockObject */ + protected $logger; + public function setUp() { parent::setUp(); - $validator = $this->createMock(IValidator::class); - $this->manager = new Manager($validator); + $this->validator = $this->createMock(IValidator::class); + $this->logger = $this->createMock(ILogger::class); + $this->manager = new Manager($this->validator, $this->logger); } public function testRegisterApp() { - $app = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() - ->getMock(); - $closure = function() use ($app) { - return $app; - }; + $this->assertEquals([], self::invokePrivate($this->manager, 'getApps')); - $this->assertEquals([], $this->invokePrivate($this->manager, 'getApps')); + $this->manager->registerApp(DummyApp::class); - $this->manager->registerApp($closure); + $this->assertCount(1, self::invokePrivate($this->manager, 'getApps')); + $this->assertCount(1, self::invokePrivate($this->manager, 'getApps')); - $this->assertEquals([$app], $this->invokePrivate($this->manager, 'getApps')); - $this->assertEquals([$app], $this->invokePrivate($this->manager, 'getApps')); + $this->manager->registerApp(DummyApp::class); - $this->manager->registerApp($closure); - - $this->assertEquals([$app, $app], $this->invokePrivate($this->manager, 'getApps')); + $this->assertCount(2, self::invokePrivate($this->manager, 'getApps')); } - /** - * @expectedException \InvalidArgumentException - */ public function testRegisterAppInvalid() { - $notifier = $this->getMockBuilder(INotifier::class) - ->disableOriginalConstructor() - ->getMock(); + $this->manager->registerApp(DummyNotifier::class); - $closure = function() use ($notifier) { - return $notifier; - }; - - $this->manager->registerApp($closure); - - $this->invokePrivate($this->manager, 'getApps'); + $this->logger->expects($this->once()) + ->method('error'); + self::invokePrivate($this->manager, 'getApps'); } public function testRegisterNotifier() { - $notifier = $this->getMockBuilder(INotifier::class) - ->disableOriginalConstructor() - ->getMock(); + $this->assertEquals([], self::invokePrivate($this->manager, 'getNotifiers')); - $closure = function() use ($notifier) { - return $notifier; - }; + $this->manager->registerNotifierService(DummyNotifier::class); - $this->assertEquals([], $this->invokePrivate($this->manager, 'getNotifiers')); - $this->assertEquals([], $this->invokePrivate($this->manager, 'listNotifiers')); + $this->assertCount(1, self::invokePrivate($this->manager, 'getNotifiers')); + $this->assertCount(1, self::invokePrivate($this->manager, 'getNotifiers')); - $this->manager->registerNotifier($closure, function() { - return ['id' => 'test1', 'name' => 'Test One']; - }); + $this->manager->registerNotifierService(DummyNotifier::class); - $this->assertEquals([$notifier], $this->invokePrivate($this->manager, 'getNotifiers')); - $this->assertEquals(['test1' => 'Test One'], $this->invokePrivate($this->manager, 'listNotifiers')); - $this->assertEquals([$notifier], $this->invokePrivate($this->manager, 'getNotifiers')); - $this->assertEquals(['test1' => 'Test One'], $this->invokePrivate($this->manager, 'listNotifiers')); - - $this->manager->registerNotifier($closure, function() { - return ['id' => 'test2', 'name' => 'Test Two']; - }); - - $this->assertEquals([$notifier, $notifier], $this->invokePrivate($this->manager, 'getNotifiers')); - $this->assertEquals(['test1' => 'Test One', 'test2' => 'Test Two'], $this->invokePrivate($this->manager, 'listNotifiers')); + $this->assertCount(2, self::invokePrivate($this->manager, 'getNotifiers')); } - /** - * @expectedException \InvalidArgumentException - */ public function testRegisterNotifierInvalid() { - $app = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() - ->getMock(); + $this->manager->registerNotifierService(DummyApp::class); - $closure = function() use ($app) { - return $app; - }; - - $this->manager->registerNotifier($closure, function() { - return ['id' => 'test1', 'name' => 'Test One']; - }); - - $this->invokePrivate($this->manager, 'getNotifiers'); - } - - public function dataRegisterNotifierInfoInvalid() { - return [ - [null], - ['No array'], - [['id' => 'test1', 'name' => 'Test One', 'size' => 'Invalid']], - [['no-id' => 'test1', 'name' => 'Test One']], - [['id' => 'test1', 'no-name' => 'Test One']], - ]; - } - - /** - * @dataProvider dataRegisterNotifierInfoInvalid - * @expectedException \InvalidArgumentException - * @param mixed $data - */ - public function testRegisterNotifierInfoInvalid($data) { - $app = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() - ->getMock(); - - $closure = function() use ($app) { - return $app; - }; - - $this->manager->registerNotifier($closure, function() use ($data) { - return $data; - }); - - $this->manager->listNotifiers(); - } - - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage The given notifier ID test1 is already in use - */ - public function testRegisterNotifierInfoDuplicate() { - $app = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() - ->getMock(); - - $closure = function() use ($app) { - return $app; - }; - - $this->manager->registerNotifier($closure, function() { - return ['id' => 'test1', 'name' => 'Test One']; - }); - - $this->manager->registerNotifier($closure, function() { - return ['id' => 'test1', 'name' => 'Test One']; - }); - - $this->manager->listNotifiers(); + $this->logger->expects($this->once()) + ->method('error'); + self::invokePrivate($this->manager, 'getNotifiers'); } public function testCreateNotification() { @@ -194,30 +104,19 @@ class ManagerTest extends TestCase { ->method('isValid') ->willReturn(true); - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $app */ - $app = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() + $manager = $this->getMockBuilder(Manager::class) + ->setConstructorArgs([ + $this->validator, + $this->logger, + ]) + ->setMethods(['getApps']) ->getMock(); - $app->expects($this->once()) - ->method('notify') - ->with($notification); - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $app2 */ - $app2 = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() - ->getMock(); - $app2->expects($this->once()) - ->method('notify') - ->with($notification); + $manager->expects($this->once()) + ->method('getApps') + ->willReturn([]); - $this->manager->registerApp(function() use ($app) { - return $app; - }); - $this->manager->registerApp(function() use ($app2) { - return $app2; - }); - - $this->manager->notify($notification); + $manager->notify($notification); } /** @@ -232,127 +131,18 @@ class ManagerTest extends TestCase { ->method('isValid') ->willReturn(false); - $this->manager->notify($notification); - } - - public function testPrepare() { - /** @var \OCP\Notification\INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ - $notification = $this->getMockBuilder(INotification::class) - ->disableOriginalConstructor() + $manager = $this->getMockBuilder(Manager::class) + ->setConstructorArgs([ + $this->validator, + $this->logger, + ]) + ->setMethods(['getApps']) ->getMock(); - $notification->expects($this->once()) - ->method('isValidParsed') - ->willReturn(true); - /** @var \OCP\Notification\INotification|\PHPUnit_Framework_MockObject_MockObject $notification2 */ - $notification2 = $this->getMockBuilder(INotification::class) - ->disableOriginalConstructor() - ->getMock(); - $notification2->expects($this->exactly(2)) - ->method('isValidParsed') - ->willReturn(true); - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $notifier */ - $notifier = $this->getMockBuilder(INotifier::class) - ->disableOriginalConstructor() - ->getMock(); - $notifier->expects($this->once()) - ->method('prepare') - ->with($notification, 'en') - ->willReturnArgument(0); + $manager->expects($this->never()) + ->method('getApps'); - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $notifier2 */ - $notifier2 = $this->getMockBuilder(INotifier::class) - ->disableOriginalConstructor() - ->getMock(); - $notifier2->expects($this->once()) - ->method('prepare') - ->with($notification, 'en') - ->willReturn($notification2); - - $this->manager->registerNotifier(function() use ($notifier) { - return $notifier; - }, function() { - return ['id' => 'test1', 'name' => 'Test One']; - }); - $this->manager->registerNotifier(function() use ($notifier2) { - return $notifier2; - }, function() { - return ['id' => 'test2', 'name' => 'Test Two']; - }); - - $this->assertEquals($notification2, $this->manager->prepare($notification, 'en')); - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testPrepareInvalid() { - /** @var \OCP\Notification\INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ - $notification = $this->getMockBuilder(INotification::class) - ->disableOriginalConstructor() - ->getMock(); - $notification->expects($this->once()) - ->method('isValidParsed') - ->willReturn(false); - - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $notifier */ - $notifier = $this->getMockBuilder(INotifier::class) - ->disableOriginalConstructor() - ->getMock(); - $notifier->expects($this->once()) - ->method('prepare') - ->with($notification, 'de') - ->willReturnArgument(0); - - $this->manager->registerNotifier(function() use ($notifier) { - return $notifier; - }, function() { - return ['id' => 'test1', 'name' => 'Test One']; - }); - - $this->manager->prepare($notification, 'de'); - } - - public function testPrepareNotifierThrows() { - /** @var \OCP\Notification\INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ - $notification = $this->getMockBuilder(INotification::class) - ->disableOriginalConstructor() - ->getMock(); - $notification->expects($this->once()) - ->method('isValidParsed') - ->willReturn(true); - - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $notifier */ - $notifier = $this->getMockBuilder(INotifier::class) - ->disableOriginalConstructor() - ->getMock(); - $notifier->expects($this->once()) - ->method('prepare') - ->with($notification, 'de') - ->willThrowException(new \InvalidArgumentException); - - $this->manager->registerNotifier(function() use ($notifier) { - return $notifier; - }, function() { - return ['id' => 'test1', 'name' => 'Test One']; - }); - - $this->assertEquals($notification, $this->manager->prepare($notification, 'de')); - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testPrepareNoNotifier() { - /** @var \OCP\Notification\INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ - $notification = $this->getMockBuilder(INotification::class) - ->disableOriginalConstructor() - ->getMock(); - $notification->expects($this->once()) - ->method('isValidParsed') - ->willReturn(false); - - $this->manager->prepare($notification, 'en'); + $manager->notify($notification); } public function testMarkProcessed() { @@ -361,30 +151,19 @@ class ManagerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $app */ - $app = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() + $manager = $this->getMockBuilder(Manager::class) + ->setConstructorArgs([ + $this->validator, + $this->logger, + ]) + ->setMethods(['getApps']) ->getMock(); - $app->expects($this->once()) - ->method('markProcessed') - ->with($notification); - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $app2 */ - $app2 = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() - ->getMock(); - $app2->expects($this->once()) - ->method('markProcessed') - ->with($notification); + $manager->expects($this->once()) + ->method('getApps') + ->willReturn([]); - $this->manager->registerApp(function() use ($app) { - return $app; - }); - $this->manager->registerApp(function() use ($app2) { - return $app2; - }); - - $this->manager->markProcessed($notification); + $manager->markProcessed($notification); } public function testGetCount() { @@ -393,31 +172,18 @@ class ManagerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $app */ - $app = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() + $manager = $this->getMockBuilder(Manager::class) + ->setConstructorArgs([ + $this->validator, + $this->logger, + ]) + ->setMethods(['getApps']) ->getMock(); - $app->expects($this->once()) - ->method('getCount') - ->with($notification) - ->willReturn(21); - /** @var \OCP\Notification\IApp|\PHPUnit_Framework_MockObject_MockObject $app2 */ - $app2 = $this->getMockBuilder(IApp::class) - ->disableOriginalConstructor() - ->getMock(); - $app2->expects($this->once()) - ->method('getCount') - ->with($notification) - ->willReturn(42); + $manager->expects($this->once()) + ->method('getApps') + ->willReturn([]); - $this->manager->registerApp(function() use ($app) { - return $app; - }); - $this->manager->registerApp(function() use ($app2) { - return $app2; - }); - - $this->assertSame(63, $this->manager->getCount($notification)); + $manager->getCount($notification); } } diff --git a/tests/lib/Notification/NotificationTest.php b/tests/lib/Notification/NotificationTest.php index 7517be715e..d72c9f62ae 100644 --- a/tests/lib/Notification/NotificationTest.php +++ b/tests/lib/Notification/NotificationTest.php @@ -63,29 +63,6 @@ class NotificationTest extends TestCase { return $dataSets; } - protected function dataInvalidStringType() { - return [ - [true], - [false], - [16412], - [[]], - [null], - ]; - } - - protected function dataInvalidInt() { - return [ - [true], - [false], - [''], - ['a'], - [str_repeat('a', 256)], - [[]], - [['a']], - [[str_repeat('a', 256)]], - ]; - } - public function dataSetApp() { return $this->dataValidString(32); } @@ -104,10 +81,6 @@ class NotificationTest extends TestCase { return $this->dataInvalidString(32); } - public function dataSetAppInvalidType() { - return $this->dataInvalidStringType(); - } - /** * @dataProvider dataSetAppInvalid * @param mixed $app @@ -118,16 +91,6 @@ class NotificationTest extends TestCase { $this->notification->setApp($app); } - /** - * @dataProvider dataSetAppInvalidType - * @param mixed $app - * - * @expectedException \TypeError - */ - public function testSetAppInvalidType($app) { - $this->notification->setApp($app); - } - public function dataSetUser() { return $this->dataValidString(64); @@ -147,10 +110,6 @@ class NotificationTest extends TestCase { return $this->dataInvalidString(64); } - public function dataSetUserInvalidType() { - return $this->dataInvalidStringType(); - } - /** * @dataProvider dataSetUserInvalid * @param mixed $user @@ -161,16 +120,6 @@ class NotificationTest extends TestCase { $this->notification->setUser($user); } - /** - * @dataProvider dataSetUserInvalidType - * @param mixed $user - * - * @expectedException \TypeError - */ - public function testSetUserInvalidType($user) { - $this->notification->setUser($user); - } - public function dataSetDateTime() { $past = new \DateTime(); $past->sub(new \DateInterval('P1Y')); @@ -216,48 +165,32 @@ class NotificationTest extends TestCase { public function dataSetObject() { return [ - ['a', '21', '21'], - [str_repeat('a', 64), 42, '42'], + ['a', '21'], + [str_repeat('a', 64), '42'], ]; } /** * @dataProvider dataSetObject * @param string $type - * @param int|string $id - * @param string $exptectedId + * @param string $id */ - public function testSetObject($type, $id, $exptectedId) { + public function testSetObject($type, $id) { $this->assertSame('', $this->notification->getObjectType()); $this->assertSame('', $this->notification->getObjectId()); $this->assertSame($this->notification, $this->notification->setObject($type, $id)); $this->assertSame($type, $this->notification->getObjectType()); - $this->assertSame($exptectedId, $this->notification->getObjectId()); + $this->assertSame($id, $this->notification->getObjectId()); } public function dataSetObjectTypeInvalid() { return $this->dataInvalidString(64); } - /** - * @dataProvider dataSetObjectTypeInvalid - * @param mixed $type - * - * @expectedException \InvalidArgumentException - * @expectedMessage 'The given object type is invalid' - */ - public function testSetObjectTypeInvalid($type) { - $this->notification->setObject($type, 1337); - } - public function dataSetObjectIdInvalid() { return [ - [true], - [false], [''], [str_repeat('a', 64 + 1)], - [[]], - [[str_repeat('a', 64 + 1)]], ]; }