Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(activity): Improved activity exceptions #1642

Merged
merged 1 commit into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/provider.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 4 additions & 3 deletions lib/Data.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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) {
}
}

Expand Down Expand Up @@ -334,7 +335,7 @@ public function validateFilter($filterValue) {
try {
$this->activityManager->getFilterById($filterValue);
return $filterValue;
} catch (\InvalidArgumentException $e) {
} catch (FilterNotFoundException) {
return 'all';
}
}
Expand Down
7 changes: 6 additions & 1 deletion lib/GroupHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

namespace OCA\Activity;

use OCP\Activity\Exceptions\UnknownActivityException;
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\IL10N;
Expand Down Expand Up @@ -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]);
}
Expand Down
10 changes: 9 additions & 1 deletion lib/NotificationGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions lib/UserSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
namespace OCA\Activity;

use OCP\Activity\ActivitySettings;
use OCP\Activity\Exceptions\SettingNotFoundException;
use OCP\Activity\IManager;
use OCP\IConfig;

Expand Down Expand Up @@ -143,7 +144,7 @@ protected function getDefaultSetting($method, $type) {
default:
return false;
}
} catch (\InvalidArgumentException $e) {
} catch (SettingNotFoundException) {
return false;
}
}
Expand All @@ -170,7 +171,7 @@ protected function canModifySetting($method, $type) {
default:
return false;
}
} catch (\InvalidArgumentException $e) {
} catch (SettingNotFoundException) {
return false;
}
}
Expand Down
3 changes: 2 additions & 1 deletion tests/UserSettingsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())
Expand Down
Loading