Merge pull request #1642 from nextcloud/techdebt/noid/improved-activity-exceptions

fix(activity): Improved activity exceptions

Author: nickvergessen

docs/provider.md

@@ -37,11 +37,11 @@ The first thing the provider should do, is to check whether the `IEvent` is one
 
 ```php
 		if ($event->getApp() !== 'files' || $event->getType() !== 'favorite') {
-			throw new \InvalidArgumentException();
+			throw new \OCP\Activity\Exceptions\UnknownActivityException();
 		}
 ```
 
-Whenever a provider throws an `\InvalidArgumentException` the activity app will continue and pass the event to the next provider, so this should always be thrown when the event is unknown.
+Whenever a provider throws an `UnknownActivityException` (*Added in Nextcloud 30, before throw `\InvalidArgumentException`*), the activity app will continue and pass the event to the next provider, so this should always be thrown when the event is unknown.
 
 ### Short translation
 

lib/Data.php

@@ -28,6 +28,7 @@
 
 use Doctrine\DBAL\Platforms\MySQLPlatform;
 use OCA\Activity\Filter\AllFilter;
+use OCP\Activity\Exceptions\FilterNotFoundException;
 use OCP\Activity\IEvent;
 use OCP\Activity\IExtension;
 use OCP\Activity\IFilter;
@@ -181,7 +182,7 @@ public function get(GroupHelper $groupHelper, UserSettings $userSettings, $user,
 		$activeFilter = null;
 		try {
 			$activeFilter = $this->activityManager->getFilterById($filter);
-		} catch (\InvalidArgumentException $e) {
+		} catch (FilterNotFoundException) {
 			// Unknown filter => ignore and show all activities
 		}
 
@@ -223,7 +224,7 @@ public function get(GroupHelper $groupHelper, UserSettings $userSettings, $user,
 				$favoriteFilter = $this->activityManager->getFilterById('files_favorites');
 				/** @var \OCA\Files\Activity\Filter\Favorites $favoriteFilter */
 				$favoriteFilter->filterFavorites($query);
-			} catch (\InvalidArgumentException $e) {
+			} catch (FilterNotFoundException) {
 			}
 		}
 
@@ -334,7 +335,7 @@ public function validateFilter($filterValue) {
 				try {
 					$this->activityManager->getFilterById($filterValue);
 					return $filterValue;
-				} catch (\InvalidArgumentException $e) {
+				} catch (FilterNotFoundException) {
 					return 'all';
 				}
 		}

lib/GroupHelper.php

@@ -22,6 +22,7 @@
 
 namespace OCA\Activity;
 
+use OCP\Activity\Exceptions\UnknownActivityException;
 use OCP\Activity\IEvent;
 use OCP\Activity\IManager;
 use OCP\IL10N;
@@ -75,7 +76,11 @@ public function addEvent(int $id, IEvent $event): void {
 				} else {
 					$event = $provider->parse($language, $event);
 				}
-			} catch (\InvalidArgumentException $e) {
+			} catch (UnknownActivityException) {
+			} catch (\InvalidArgumentException) {
+				// todo 33.0.0 Log as warning
+				// todo 39.0.0 Log as error
+				$this->logger->debug(get_class($provider) . '::parse() threw \InvalidArgumentException which is deprecated. Throw \OCP\Activity\Exceptions\UnknownActivityException when the event is not known to your provider and otherwise handle all \InvalidArgumentException yourself.');
 			} catch (\Throwable $e) {
 				$this->logger->error('Error while parsing activity event', ['exception' => $e]);
 			}

lib/NotificationGenerator.php

@@ -23,13 +23,15 @@
 
 namespace OCA\Activity;
 
+use OCP\Activity\Exceptions\UnknownActivityException;
 use OCP\Activity\IEvent;
 use OCP\Activity\IManager as ActivityManager;
 use OCP\IL10N;
 use OCP\Notification\AlreadyProcessedException;
 use OCP\Notification\IManager as NotificationManager;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
+use Psr\Log\LoggerInterface;
 
 class NotificationGenerator implements INotifier {
 
@@ -38,7 +40,9 @@ public function __construct(
 		protected ActivityManager $activityManager,
 		protected NotificationManager $notificationManager,
 		protected UserSettings $userSettings,
-		protected IL10N $l10n) {
+		protected IL10N $l10n,
+		protected LoggerInterface $logger,
+	) {
 	}
 
 	public function deferNotifications(): bool {
@@ -90,7 +94,11 @@ private function populateEvent(IEvent $event, string $language) {
 		foreach ($this->activityManager->getProviders() as $provider) {
 			try {
 				$event = $provider->parse($language, $event);
+			} catch (UnknownActivityException) {
 			} catch (\InvalidArgumentException $e) {
+				// todo 33.0.0 Log as warning
+				// todo 39.0.0 Log as error
+				$this->logger->debug(get_class($provider) . '::parse() threw \InvalidArgumentException which is deprecated. Throw \OCP\Activity\Exceptions\UnknownActivityException when the event is not known to your provider and otherwise handle all \InvalidArgumentException yourself.');
 			}
 		}
 		$this->activityManager->setFormattingObject('', 0);

lib/UserSettings.php

@@ -23,6 +23,7 @@
 namespace OCA\Activity;
 
 use OCP\Activity\ActivitySettings;
+use OCP\Activity\Exceptions\SettingNotFoundException;
 use OCP\Activity\IManager;
 use OCP\IConfig;
 
@@ -143,7 +144,7 @@ protected function getDefaultSetting($method, $type) {
 				default:
 					return false;
 			}
-		} catch (\InvalidArgumentException $e) {
+		} catch (SettingNotFoundException) {
 			return false;
 		}
 	}
@@ -170,7 +171,7 @@ protected function canModifySetting($method, $type) {
 				default:
 					return false;
 			}
-		} catch (\InvalidArgumentException $e) {
+		} catch (SettingNotFoundException) {
 			return false;
 		}
 	}

tests/UserSettingsTest.php

@@ -24,6 +24,7 @@
 
 use OCA\Activity\UserSettings;
 use OCP\Activity\ActivitySettings;
+use OCP\Activity\Exceptions\SettingNotFoundException;
 use OCP\Activity\IManager;
 use OCP\IConfig;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -65,7 +66,7 @@ public function testGetDefaultSetting(string $method, string $type, $expected):
 				$this->activityManager->expects($this->once())
 					->method('getSettingById')
 					->with($type)
-					->willThrowException(new \InvalidArgumentException());
+					->willThrowException(new SettingNotFoundException($type));
 			} else {
 				$s = $this->createMock(ActivitySettings::class);
 				$s->expects($method === 'email' ? $this->once() : $this->never())