fix(notifications): Improved notification exceptions

lib/composer/composer/LICENSE

@@ -1,3 +1,4 @@
+
 Copyright (c) Nils Adermann, Jordi Boggiano
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
@@ -17,3 +18,4 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 THE SOFTWARE.
+

lib/composer/composer/autoload_classmap.php

@@ -567,6 +567,10 @@
     'OCP\\Notification\\IManager' => $baseDir . '/lib/public/Notification/IManager.php',
     'OCP\\Notification\\INotification' => $baseDir . '/lib/public/Notification/INotification.php',
     'OCP\\Notification\\INotifier' => $baseDir . '/lib/public/Notification/INotifier.php',
+    'OCP\\Notification\\IncompleteNotificationException' => $baseDir . '/lib/public/Notification/IncompleteNotificationException.php',
+    'OCP\\Notification\\IncompleteParsedNotificationException' => $baseDir . '/lib/public/Notification/IncompleteParsedNotificationException.php',
+    'OCP\\Notification\\InvalidValueException' => $baseDir . '/lib/public/Notification/InvalidValueException.php',
+    'OCP\\Notification\\UnknownNotificationException' => $baseDir . '/lib/public/Notification/UnknownNotificationException.php',
     'OCP\\OCM\\Events\\ResourceTypeRegisterEvent' => $baseDir . '/lib/public/OCM/Events/ResourceTypeRegisterEvent.php',
     'OCP\\OCM\\Exceptions\\OCMArgumentException' => $baseDir . '/lib/public/OCM/Exceptions/OCMArgumentException.php',
     'OCP\\OCM\\Exceptions\\OCMProviderException' => $baseDir . '/lib/public/OCM/Exceptions/OCMProviderException.php',

lib/composer/composer/autoload_static.php

@@ -600,6 +600,10 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OCP\\Notification\\IManager' => __DIR__ . '/../../..' . '/lib/public/Notification/IManager.php',
         'OCP\\Notification\\INotification' => __DIR__ . '/../../..' . '/lib/public/Notification/INotification.php',
         'OCP\\Notification\\INotifier' => __DIR__ . '/../../..' . '/lib/public/Notification/INotifier.php',
+        'OCP\\Notification\\IncompleteNotificationException' => __DIR__ . '/../../..' . '/lib/public/Notification/IncompleteNotificationException.php',
+        'OCP\\Notification\\IncompleteParsedNotificationException' => __DIR__ . '/../../..' . '/lib/public/Notification/IncompleteParsedNotificationException.php',
+        'OCP\\Notification\\InvalidValueException' => __DIR__ . '/../../..' . '/lib/public/Notification/InvalidValueException.php',
+        'OCP\\Notification\\UnknownNotificationException' => __DIR__ . '/../../..' . '/lib/public/Notification/UnknownNotificationException.php',
         'OCP\\OCM\\Events\\ResourceTypeRegisterEvent' => __DIR__ . '/../../..' . '/lib/public/OCM/Events/ResourceTypeRegisterEvent.php',
         'OCP\\OCM\\Exceptions\\OCMArgumentException' => __DIR__ . '/../../..' . '/lib/public/OCM/Exceptions/OCMArgumentException.php',
         'OCP\\OCM\\Exceptions\\OCMProviderException' => __DIR__ . '/../../..' . '/lib/public/OCM/Exceptions/OCMProviderException.php',

lib/private/Notification/Action.php

@@ -25,100 +25,72 @@
 namespace OC\Notification;
 
 use OCP\Notification\IAction;
+use OCP\Notification\InvalidValueException;
 
 class Action implements IAction {
-	protected string $label;
-
-	protected string $labelParsed;
-
-	protected string $link;
-
-	protected string $requestType;
-
-	protected string $icon;
-
-	protected bool $primary;
-
-	public function __construct() {
-		$this->label = '';
-		$this->labelParsed = '';
-		$this->link = '';
-		$this->requestType = '';
-		$this->primary = false;
-	}
+	protected string $label = '';
+	protected string $labelParsed = '';
+	protected string $link = '';
+	protected string $requestType = '';
+	protected bool $primary = false;
 
 	/**
-	 * @param string $label
-	 * @return $this
-	 * @throws \InvalidArgumentException if the label is invalid
-	 * @since 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function setLabel(string $label): IAction {
 		if ($label === '' || isset($label[32])) {
-			throw new \InvalidArgumentException('The given label is invalid');
+			throw new InvalidValueException('label');
 		}
 		$this->label = $label;
 		return $this;
 	}
 
 	/**
-	 * @return string
-	 * @since 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function getLabel(): string {
 		return $this->label;
 	}
 
 	/**
-	 * @param string $label
-	 * @return $this
-	 * @throws \InvalidArgumentException if the label is invalid
-	 * @since 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function setParsedLabel(string $label): IAction {
 		if ($label === '') {
-			throw new \InvalidArgumentException('The given parsed label is invalid');
+			throw new InvalidValueException('parsedLabel');
 		}
 		$this->labelParsed = $label;
 		return $this;
 	}
 
 	/**
-	 * @return string
-	 * @since 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function getParsedLabel(): string {
 		return $this->labelParsed;
 	}
 
 	/**
-	 * @param $primary bool
-	 * @return $this
-	 * @since 9.0.0
+	 * {@inheritDoc}
 	 */
 	public function setPrimary(bool $primary): IAction {
 		$this->primary = $primary;
 		return $this;
 	}
 
 	/**
-	 * @return bool
-	 * @since 9.0.0
+	 * {@inheritDoc}
 	 */
 	public function isPrimary(): bool {
 		return $this->primary;
 	}
 
 	/**
-	 * @param string $link
-	 * @param string $requestType
-	 * @return $this
-	 * @throws \InvalidArgumentException if the link is invalid
-	 * @since 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function setLink(string $link, string $requestType): IAction {
 		if ($link === '' || isset($link[256])) {
-			throw new \InvalidArgumentException('The given link is invalid');
+			throw new InvalidValueException('link');
 		}
 		if (!in_array($requestType, [
 			self::TYPE_GET,
@@ -127,38 +99,36 @@ public function setLink(string $link, string $requestType): IAction {
 			self::TYPE_DELETE,
 			self::TYPE_WEB,
 		], true)) {
-			throw new \InvalidArgumentException('The given request type is invalid');
+			throw new InvalidValueException('requestType');
 		}
 		$this->link = $link;
 		$this->requestType = $requestType;
 		return $this;
 	}
 
 	/**
-	 * @return string
-	 * @since 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function getLink(): string {
 		return $this->link;
 	}
 
 	/**
-	 * @return string
-	 * @since 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function getRequestType(): string {
 		return $this->requestType;
 	}
 
 	/**
-	 * @return bool
+	 * {@inheritDoc}
 	 */
 	public function isValid(): bool {
 		return $this->label !== '' && $this->link !== '';
 	}
 
 	/**
-	 * @return bool
+	 * {@inheritDoc}
 	 */
 	public function isValidParsed(): bool {
 		return $this->labelParsed !== '' && $this->link !== '';

lib/private/Notification/Manager.php

@@ -35,8 +35,11 @@
 use OCP\Notification\IDeferrableApp;
 use OCP\Notification\IDismissableNotifier;
 use OCP\Notification\IManager;
+use OCP\Notification\IncompleteNotificationException;
+use OCP\Notification\IncompleteParsedNotificationException;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
+use OCP\Notification\UnknownNotificationException;
 use OCP\RichObjectStrings\IValidator;
 use OCP\Support\Subscription\IRegistry;
 use Psr\Container\ContainerExceptionInterface;
@@ -300,21 +303,23 @@ public function isFairUseOfFreePushService(): bool {
 	}
 
 	/**
-	 * @param INotification $notification
-	 * @throws \InvalidArgumentException When the notification is not valid
-	 * @since 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function notify(INotification $notification): void {
 		if (!$notification->isValid()) {
-			throw new \InvalidArgumentException('The given notification is invalid');
+			throw new IncompleteNotificationException('The given notification is invalid');
 		}
 
 		$apps = $this->getApps();
 
 		foreach ($apps as $app) {
 			try {
 				$app->notify($notification);
+			} catch (IncompleteNotificationException) {
 			} catch (\InvalidArgumentException $e) {
+				// todo 33.0.0 Log as warning
+				// todo 39.0.0 Log as error
+				$this->logger->debug(get_class($app) . '::notify() threw \InvalidArgumentException which is deprecated. Throw \OCP\Notification\IncompleteNotificationException when the notification is incomplete for your app and otherwise handle all \InvalidArgumentException yourself.');
 			}
 		}
 	}
@@ -340,34 +345,52 @@ 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 8.2.0
+	 * {@inheritDoc}
 	 */
 	public function prepare(INotification $notification, string $languageCode): INotification {
 		$notifiers = $this->getNotifiers();
 
 		foreach ($notifiers as $notifier) {
 			try {
 				$notification = $notifier->prepare($notification, $languageCode);
-			} catch (\InvalidArgumentException $e) {
-				continue;
 			} catch (AlreadyProcessedException $e) {
 				$this->markProcessed($notification);
-				throw new \InvalidArgumentException('The given notification has been processed');
+				throw $e;
+			} catch (UnknownNotificationException) {
+				continue;
+			} catch (\InvalidArgumentException $e) {
+				// todo 33.0.0 Log as warning
+				// todo 39.0.0 Log as error
+				$this->logger->debug(get_class($notifier) . '::prepare() threw \InvalidArgumentException which is deprecated. Throw \OCP\Notification\UnknownNotificationException when the notification is not known to your notifier and otherwise handle all \InvalidArgumentException yourself.');
+				continue;
 			}
 
 			if (!$notification->isValidParsed()) {
-				throw new \InvalidArgumentException('The given notification has not been handled');
+				$this->logger->info('Notification was claimed to be parsed, but was not fully parsed by ' . get_class($notifier) . ' [app: ' . $notification->getApp() . ', subject: ' . $notification->getSubject() . ']');
+				throw new IncompleteParsedNotificationException();
 			}
 		}
 
 		if (!$notification->isValidParsed()) {
 			$this->logger->info('Notification was not parsed by any notifier [app: ' . $notification->getApp() . ', subject: ' . $notification->getSubject() . ']');
-			throw new \InvalidArgumentException('The given notification has not been handled');
+			throw new IncompleteParsedNotificationException();
+		}
+
+		$link = $notification->getLink();
+		if ($link !== '' && !str_starts_with($link, 'http://') && !str_starts_with($link, 'https://')) {
+			$this->logger->warning('Link of notification is not an absolute URL and does not work in mobile and desktop clients [app: ' . $notification->getApp() . ', subject: ' . $notification->getSubject() . ']');
+		}
+
+		$icon = $notification->getIcon();
+		if ($icon !== '' && !str_starts_with($icon, 'http://') && !str_starts_with($icon, 'https://')) {
+			$this->logger->warning('Icon of notification is not an absolute URL and does not work in mobile and desktop clients [app: ' . $notification->getApp() . ', subject: ' . $notification->getSubject() . ']');
+		}
+
+		foreach ($notification->getParsedActions() as $action) {
+			$link = $action->getLink();
+			if ($link !== '' && !str_starts_with($link, 'http://') && !str_starts_with($link, 'https://')) {
+				$this->logger->warning('Link of action is not an absolute URL and does not work in mobile and desktop clients [app: ' . $notification->getApp() . ', subject: ' . $notification->getSubject() . ']');
+			}
 		}
 
 		return $notification;
@@ -399,14 +422,22 @@ public function getCount(INotification $notification): int {
 		return $count;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
 	public function dismissNotification(INotification $notification): void {
 		$notifiers = $this->getNotifiers();
 
 		foreach ($notifiers as $notifier) {
 			if ($notifier instanceof IDismissableNotifier) {
 				try {
 					$notifier->dismissNotification($notification);
+				} catch (UnknownNotificationException) {
+					continue;
 				} catch (\InvalidArgumentException $e) {
+					// todo 33.0.0 Log as warning
+					// todo 39.0.0 Log as error
+					$this->logger->debug(get_class($notifier) . '::dismissNotification() threw \InvalidArgumentException which is deprecated. Throw \OCP\Notification\UnknownNotificationException when the notification is not known to your notifier and otherwise handle all \InvalidArgumentException yourself.');
 					continue;
 				}
 			}