Skip to content

Commit

Permalink
Merge pull request #12156 from nextcloud/techdebt/noid/update-activit…
Browse files Browse the repository at this point in the history
…y-exception-handling

fix(techdebt): Update exception handling in activity app
  • Loading branch information
nickvergessen authored Apr 20, 2024
2 parents d1239c4 + 1712329 commit b723998
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 17 deletions.
6 changes: 3 additions & 3 deletions lib/Activity/Listener.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ protected function generateCallActivity(Room $room, bool $endForEveryone = false
try {
$event->setAffectedUser($userId);
$this->activityManager->publish($event);
} catch (\BadMethodCallException|\InvalidArgumentException $e) {
} catch (\Throwable $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
}
}
Expand Down Expand Up @@ -219,7 +219,7 @@ protected function generateInvitationActivity(Room $room, array $attendees): voi
'user' => $actor->getUID(),
'room' => $room->getId(),
]);
} catch (\InvalidArgumentException $e) {
} catch (\Throwable $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
return;
}
Expand Down Expand Up @@ -254,7 +254,7 @@ protected function generateInvitationActivity(Room $room, array $attendees): voi
])
->setAffectedUser($attendee->getActorId());
$this->activityManager->publish($event);
} catch (\BadMethodCallException|\InvalidArgumentException $e) {
} catch (\Throwable $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
}
}
Expand Down
8 changes: 4 additions & 4 deletions lib/Activity/Provider/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use OCA\Talk\Manager;
use OCA\Talk\Room;
use OCA\Talk\Service\AvatarService;
use OCP\Activity\Exceptions\UnknownActivityException;
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\Activity\IProvider;
Expand All @@ -52,17 +53,17 @@ public function __construct(
/**
* @param IEvent $event
* @return IEvent
* @throws \InvalidArgumentException
* @throws UnknownActivityException
*/
public function preParse(IEvent $event): IEvent {
if ($event->getApp() !== 'spreed') {
throw new \InvalidArgumentException('Wrong app');
throw new UnknownActivityException('app');
}

$uid = $event->getAffectedUser();
$user = $this->userManager->get($uid);
if (!$user instanceof IUser || $this->config->isDisabledForUser($user)) {
throw new \InvalidArgumentException('User can not user Talk');
throw new UnknownActivityException('User can not use Talk');
}

if ($this->activityManager->getRequirePNG()) {
Expand All @@ -78,7 +79,6 @@ public function preParse(IEvent $event): IEvent {
* @param IEvent $event
* @param string $subject
* @param array $parameters
* @throws \InvalidArgumentException
*/
protected function setSubjects(IEvent $event, string $subject, array $parameters): void {
$placeholders = $replacements = [];
Expand Down
7 changes: 4 additions & 3 deletions lib/Activity/Provider/Call.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

namespace OCA\Talk\Activity\Provider;

use OCP\Activity\Exceptions\UnknownActivityException;
use OCP\Activity\IEvent;
use OCP\IL10N;

Expand All @@ -32,7 +33,7 @@ class Call extends Base {
* @param IEvent $event
* @param IEvent|null $previousEvent
* @return IEvent
* @throws \InvalidArgumentException
* @throws UnknownActivityException
* @since 11.0.0
*/
public function parse($language, IEvent $event, ?IEvent $previousEvent = null): IEvent {
Expand All @@ -54,7 +55,7 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null):
// $result['params']['call'] = $roomParameter;
$this->setSubjects($event, $result['subject'], $result['params']);
} else {
throw new \InvalidArgumentException('Wrong subject');
throw new UnknownActivityException('subject');
}

return $event;
Expand All @@ -80,7 +81,7 @@ protected function parseCall(IEvent $event, IL10N $l): array {

$currentUser = array_search($this->activityManager->getCurrentUserId(), $parameters['users'], true);
if ($currentUser === false) {
throw new \InvalidArgumentException('Unknown case');
throw new UnknownActivityException('Unknown case');
}
unset($parameters['users'][$currentUser]);
sort($parameters['users']);
Expand Down
5 changes: 3 additions & 2 deletions lib/Activity/Provider/Invitation.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
namespace OCA\Talk\Activity\Provider;

use OCA\Talk\Exceptions\RoomNotFoundException;
use OCP\Activity\Exceptions\UnknownActivityException;
use OCP\Activity\IEvent;

class Invitation extends Base {
Expand All @@ -32,7 +33,7 @@ class Invitation extends Base {
* @param IEvent $event
* @param IEvent|null $previousEvent
* @return IEvent
* @throws \InvalidArgumentException
* @throws UnknownActivityException
* @since 11.0.0
*/
public function parse($language, IEvent $event, ?IEvent $previousEvent = null): IEvent {
Expand All @@ -54,7 +55,7 @@ public function parse($language, IEvent $event, ?IEvent $previousEvent = null):
'call' => $roomParameter,
]);
} else {
throw new \InvalidArgumentException('Wrong subject');
throw new UnknownActivityException('subject');
}

return $event;
Expand Down
8 changes: 4 additions & 4 deletions tests/php/Activity/Provider/BaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use OCA\Talk\Manager;
use OCA\Talk\Room;
use OCA\Talk\Service\AvatarService;
use OCP\Activity\Exceptions\UnknownActivityException;
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\IURLGenerator;
Expand Down Expand Up @@ -65,9 +66,8 @@ public function setUp(): void {

/**
* @param string[] $methods
* @return Base|MockObject
*/
protected function getProvider(array $methods = []) {
protected function getProvider(array $methods = []): Base&MockObject {
$methods[] = 'parse';
return $this->getMockBuilder(Base::class)
->setConstructorArgs([
Expand Down Expand Up @@ -106,7 +106,7 @@ public function testPreParse(string $appId, bool $hasUser, bool $disabledForUser
->willReturn($appId);

if ($willThrowException) {
$this->expectException(\InvalidArgumentException::class);
$this->expectException(UnknownActivityException::class);
}
$event->expects($this->exactly($willThrowException ? 0 : 1))
->method('setIcon')
Expand Down Expand Up @@ -137,7 +137,7 @@ public function testPreParseThrows(): void {
->method('getApp')
->willReturn('activity');
$provider = $this->getProvider();
$this->expectException(\InvalidArgumentException::class);
$this->expectException(UnknownActivityException::class);
static::invokePrivate($provider, 'preParse', [$event]);
}

Expand Down
3 changes: 2 additions & 1 deletion tests/php/Activity/Provider/InvitationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
use OCA\Talk\Manager;
use OCA\Talk\Room;
use OCA\Talk\Service\AvatarService;
use OCP\Activity\Exceptions\UnknownActivityException;
use OCP\Activity\IEvent;
use OCP\Activity\IManager;
use OCP\IL10N;
Expand Down Expand Up @@ -119,7 +120,7 @@ public function testParseThrowsWrongSubject(): void {
->willReturn(false);

$provider = $this->getProvider();
$this->expectException(\InvalidArgumentException::class);
$this->expectException(UnknownActivityException::class);
$provider->parse('en', $event);
}

Expand Down

0 comments on commit b723998

Please sign in to comment.