fix(activity): Improved activity exceptions

Signed-off-by: Joas Schilling <coding@schilljs.com>
nextcloud · Apr 17, 2024 · 815da7c · 815da7c
1 parent 2beb577
commit 815da7c
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 8 deletions.
diff --git a/lib/Data.php b/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';
 				}
 		}

diff --git a/lib/GroupHelper.php b/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]);
 			}

diff --git a/lib/NotificationGenerator.php b/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);

diff --git a/lib/UserSettings.php b/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;
 		}
 	}

diff --git a/tests/UserSettingsTest.php b/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())