Change how Notifiers and Apps are registered

Signed-off-by: Joas Schilling <coding@schilljs.com>

diff --git a/lib/private/Notification/Manager.php b/lib/private/Notification/Manager.php
index 876a191fb9e0918b8a48f5b2f928b9e661281a84..df54265a7adde4f3f955bf58bafef15175e7c0b6 100644 (file)
--- 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();
+               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)) {
-                               throw new \InvalidArgumentException('The given notification app does not implement the IApp interface');
+                               $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();
+               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)) {
-                               throw new \InvalidArgumentException('The given notifier does not implement the INotifier interface');
+                               $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 (file)
index 0000000..3cfe466
--- /dev/null
+++ b/lib/public/Notification/AlreadyProcessedException.php
@@ -0,0 +1,32 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019 Joas Schilling <coding@schilljs.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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 7feac76a5dce0b846a5cb812e3c913bc8c59e731..262bb121431480cedf91cea1c0feb95a7cb3d92c 100644 (file)
--- 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 44066c035b9d3b523445c68bb3e81ebc7935936e..b730b1d801552ba39bd6a457024b3ecfd46ecb29 100644 (file)
--- 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;