From 9690b3153a0909c04437d0730302045c82b63bb7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Apr 2019 14:45:33 +0200 Subject: [PATCH] Change how Notifiers and Apps are registered Signed-off-by: Joas Schilling --- lib/private/Notification/Manager.php | 143 ++++++++++-------- .../AlreadyProcessedException.php | 32 ++++ lib/public/Notification/IManager.php | 18 +-- lib/public/Notification/INotifier.php | 18 +++ 4 files changed, 141 insertions(+), 70 deletions(-) create mode 100644 lib/public/Notification/AlreadyProcessedException.php diff --git a/lib/private/Notification/Manager.php b/lib/private/Notification/Manager.php index 876a191fb9e..df54265a7ad 100644 --- a/lib/private/Notification/Manager.php +++ b/lib/private/Notification/Manager.php @@ -26,6 +26,9 @@ declare(strict_types=1); namespace OC\Notification; +use OCP\AppFramework\QueryException; +use OCP\ILogger; +use OCP\Notification\AlreadyProcessedException; use OCP\Notification\IApp; use OCP\Notification\IManager; use OCP\Notification\INotification; @@ -35,6 +38,8 @@ use OCP\RichObjectStrings\IValidator; class Manager implements IManager { /** @var IValidator */ protected $validator; + /** @var ILogger */ + protected $logger; /** @var IApp[] */ protected $apps; @@ -45,11 +50,11 @@ class Manager implements IManager { /** @var array[] */ protected $notifiersInfo; - /** @var \Closure[] */ - protected $appsClosures; + /** @var string[] */ + protected $appClasses; - /** @var \Closure[] */ - protected $notifiersClosures; + /** @var string[] */ + protected $notifierClasses; /** @var \Closure[] */ protected $notifiersInfoClosures; @@ -57,55 +62,60 @@ class Manager implements IManager { /** @var bool */ protected $preparingPushNotification; - public function __construct(IValidator $validator) { + public function __construct(IValidator $validator, + ILogger $logger) { $this->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 9.0.0 */ - public function registerApp(\Closure $service) { - $this->appsClosures[] = $service; - $this->apps = []; + public function registerApp(string $appClass): void { + $this->appClasses[] = $appClass; } /** - * @param \Closure $service The service must implement INotifier, otherwise a + * @param string $notifierClass The service must implement INotifier, otherwise a * \InvalidArgumentException is thrown later - * @param \Closure $info An array with the keys 'id' and 'name' containing - * the app id and the app name - * @since 8.2.0 - Parameter $info was added in 9.0.0 + * @since 17.0.0 */ - public function registerNotifier(\Closure $service, \Closure $info) { - $this->notifiersClosures[] = $service; - $this->notifiersInfoClosures[] = $info; - $this->notifiers = []; - $this->notifiersInfo = []; + public function registerNotifier(string $notifierClass): void { + $this->notifierClasses[] = $notifierClass; } /** * @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; } @@ -115,46 +125,35 @@ class Manager implements IManager { /** * @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; } 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 @@ -207,11 +206,32 @@ 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, string $languageCode): INotification { @@ -222,6 +242,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()) { diff --git a/lib/public/Notification/AlreadyProcessedException.php b/lib/public/Notification/AlreadyProcessedException.php new file mode 100644 index 00000000000..3cfe4667d6b --- /dev/null +++ b/lib/public/Notification/AlreadyProcessedException.php @@ -0,0 +1,32 @@ + + * + * @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; + + +class AlreadyProcessedException extends \RuntimeException { + + public function __construct() { + parent::__construct('Notification is processed already'); + } + +} diff --git a/lib/public/Notification/IManager.php b/lib/public/Notification/IManager.php index 7feac76a5dc..262bb121431 100644 --- a/lib/public/Notification/IManager.php +++ b/lib/public/Notification/IManager.php @@ -31,26 +31,24 @@ namespace OCP\Notification; */ interface IManager extends IApp, INotifier { /** - * @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 9.0.0 + * @since 17.0.0 */ - public function registerApp(\Closure $service); + public function registerApp(string $appClass): void; /** - * @param \Closure $service The service must implement INotifier, otherwise a + * @param string $notifierClass The service must implement INotifier, otherwise a * \InvalidArgumentException is thrown later - * @param \Closure $info An array with the keys 'id' and 'name' containing - * the app id and the app name - * @since 9.0.0 + * @since 17.0.0 */ - public function registerNotifier(\Closure $service, \Closure $info); + public function registerNotifier(string $notifierClass): void; /** - * @return array App ID => App Name + * @return INotifier[] * @since 9.0.0 */ - public function listNotifiers(): array; + public function getNotifiers(): array; /** * @return INotification diff --git a/lib/public/Notification/INotifier.php b/lib/public/Notification/INotifier.php index 44066c035b9..b730b1d8015 100644 --- a/lib/public/Notification/INotifier.php +++ b/lib/public/Notification/INotifier.php @@ -30,11 +30,29 @@ namespace OCP\Notification; * @since 9.0.0 */ interface INotifier { + + /** + * Identifier of the notifier, only use [a-z0-9_] + * + * @return string + * @since 17.0.0 + */ + public function getID(): string; + + /** + * Human readable name describing the notifier + * + * @return string + * @since 17.0.0 + */ + public function getName(): string; + /** * @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;