diff --git a/lib/FilesHooks.php b/lib/FilesHooks.php index 84d7ca8ee..57dd8f9cd 100644 --- a/lib/FilesHooks.php +++ b/lib/FilesHooks.php @@ -799,8 +799,8 @@ protected function shareWithTeam(string $shareWith, Node $fileSource, string $fi return; } + $this->teamManager->startSuperSession(); try { - $this->teamManager->startSuperSession(); $team = $this->teamManager->getCircle($shareWith); $members = $team->getInheritedMembers(); $members = array_filter($members, fn ($member) => $member->getUserType() === Member::TYPE_USER); @@ -809,6 +809,8 @@ protected function shareWithTeam(string $shareWith, Node $fileSource, string $fi $this->logger->debug('Fetching team members for share activity failed', ['exception' => $e]); // error in teams app - setting users list to empty $userIds = []; + } finally { + $this->teamManager->stopSession(); } // Activity for user performing the share diff --git a/lib/NotificationGenerator.php b/lib/NotificationGenerator.php index 59836ea07..6dd8fd1e9 100644 --- a/lib/NotificationGenerator.php +++ b/lib/NotificationGenerator.php @@ -124,8 +124,11 @@ public function prepare(INotification $notification, string $languageCode): INot throw new AlreadyProcessedException(); } $this->activityManager->setCurrentUserId($notification->getUser()); - $event = $this->populateEvent($event, $languageCode); - $this->activityManager->setCurrentUserId(null); + try { + $event = $this->populateEvent($event, $languageCode); + } finally { + $this->activityManager->setCurrentUserId(null); + } try { return $this->getDisplayNotificationForEvent($event, $event->getObjectId()); diff --git a/tests/FilesHooksTest.php b/tests/FilesHooksTest.php index 6939974bc..b992d3a0e 100644 --- a/tests/FilesHooksTest.php +++ b/tests/FilesHooksTest.php @@ -1039,6 +1039,31 @@ public static function dataAddNotificationsForUser(): array { ]; } + public function testAddNotificationsForUserSkipsInvalidEvent(): void { + $this->urlGenerator->method('linkToRouteAbsolute') + ->willReturn('routeToFilesIndex'); + + $event = $this->createMock(IEvent::class); + $event->method('setApp')->willReturnSelf(); + $event->method('setType')->willReturnSelf(); + $event->method('setAffectedUser')->willReturnSelf(); + $event->method('setTimestamp')->willReturnSelf(); + $event->method('setSubject') + ->willThrowException(new \InvalidArgumentException('invalid subject')); + + $this->activityManager->expects($this->once()) + ->method('generateEvent') + ->willReturn($event); + + // A half-built event must never reach the stream or the mail queue + $this->data->expects($this->never()) + ->method('send'); + $this->data->expects($this->never()) + ->method('storeMail'); + + self::invokePrivate($this->filesHooks, 'addNotificationsForUser', ['user1', 'subject', [], 42, '/file.txt', true, 3600, true, 'shared']); + } + #[DataProvider('dataAddNotificationsForUser')] public function testAddNotificationsForUser(string $user, string $subject, array $parameter, int $fileId, string $path, string $urlPath, bool $isFile, bool $notification, bool $email, string $type, string $app, bool $sentEmail): void { $this->urlGenerator->expects($this->once()) diff --git a/tests/NotificationGeneratorTest.php b/tests/NotificationGeneratorTest.php new file mode 100644 index 000000000..98e329909 --- /dev/null +++ b/tests/NotificationGeneratorTest.php @@ -0,0 +1,75 @@ +data = $this->createMock(Data::class); + $this->activityManager = $this->createMock(ActivityManager::class); + + $this->generator = new NotificationGenerator( + $this->data, + $this->activityManager, + $this->createMock(NotificationManager::class), + $this->createMock(UserSettings::class), + $this->createMock(IL10N::class), + $this->createMock(LoggerInterface::class), + ); + } + + public function testCurrentUserIsResetWhenAProviderThrows(): void { + $notification = $this->createMock(INotification::class); + $notification->method('getObjectType')->willReturn('activity_notification'); + $notification->method('getObjectId')->willReturn('42'); + $notification->method('getUser')->willReturn('affected'); + + $event = $this->createMock(IEvent::class); + $event->method('getAffectedUser')->willReturn('affected'); + $this->data->method('getById')->willReturn($event); + + $provider = $this->createMock(IProvider::class); + $provider->method('parse') + ->willThrowException(new \RuntimeException('provider exploded')); + $this->activityManager->method('getProviders') + ->willReturn([$provider]); + + $seenUsers = []; + $this->activityManager->method('setCurrentUserId') + ->willReturnCallback(static function (?string $uid) use (&$seenUsers): void { + $seenUsers[] = $uid; + }); + + try { + $this->generator->prepare($notification, 'en'); + $this->fail('Expected the provider exception to propagate'); + } catch (\RuntimeException) { + } + + // The identity must not survive an exploding provider + $this->assertSame(['affected', null], $seenUsers); + } +} diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index 2f435f189..25d3a3f89 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -23,6 +23,7 @@ + teamManager]]> teamManager]]> teamManager]]>