From 39bc303f129f7bfd0f8a88f16e05c87b2419bbbf Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 13 Dec 2023 10:22:32 +0100 Subject: [PATCH] fix(notifications): Preparse call notifications for improved performance Reducing the roundtrips in the notification providers by only parsing each language once Signed-off-by: Joas Schilling --- lib/Notification/Listener.php | 43 +++++++++++++++++++++++++++++++++-- lib/Notification/Notifier.php | 31 ++++++++++++++++--------- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/lib/Notification/Listener.php b/lib/Notification/Listener.php index f622dd865ca..0231d612e48 100644 --- a/lib/Notification/Listener.php +++ b/lib/Notification/Listener.php @@ -41,10 +41,12 @@ use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; use OCP\EventDispatcher\IEventListener; +use OCP\IConfig; use OCP\IDBConnection; use OCP\IUser; use OCP\IUserSession; use OCP\Notification\IManager; +use OCP\Notification\INotification; use Psr\Log\LoggerInterface; /** @@ -53,10 +55,14 @@ class Listener implements IEventListener { protected bool $shouldSendCallNotification = false; + /** @var array $preparedCallNotifications Map of language => parsed notification in that language */ + protected array $preparedCallNotifications = []; public function __construct( + protected IConfig $serverConfig, protected IDBConnection $connection, protected IManager $notificationManager, + protected Notifier $notificationProvider, protected ParticipantService $participantsService, protected IEventDispatcher $dispatcher, protected IUserSession $userSession, @@ -290,7 +296,24 @@ protected function sendCallNotifications(Room $room): void { return; } + $this->preparedCallNotifications = []; $userIds = $this->participantsService->getParticipantUserIdsForCallNotifications($room); + // Room name depends on the notification user for one-to-one, + // so we avoid pre-parsing it there. Also, it comes with some base load, + // so we only do it for "big enough" calls. + $preparseNotificationForPush = count($userIds) > 10; + if ($preparseNotificationForPush) { + $fallbackLang = $this->serverConfig->getSystemValue('force_language', null); + if (is_string($fallbackLang)) { + /** @psalm-var array $userLanguages */ + $userLanguages = []; + } else { + $fallbackLang = $this->serverConfig->getSystemValueString('default_language', 'en'); + /** @psalm-var array $userLanguages */ + $userLanguages = $this->serverConfig->getUserValueForUsers('core', 'lang', $userIds); + } + } + $this->connection->beginTransaction(); try { foreach ($userIds as $userId) { @@ -298,9 +321,25 @@ protected function sendCallNotifications(Room $room): void { continue; } + if ($preparseNotificationForPush) { + // Get the settings for this particular user, then check if we have notifications to email them + $languageCode = $userLanguages[$userId] ?? $fallbackLang; + + if (!isset($this->preparedCallNotifications[$languageCode])) { + $translatedNotification = clone $notification; + + $this->notificationManager->setPreparingPushNotification(true); + $this->preparedCallNotifications[$languageCode] = $this->notificationProvider->prepare($translatedNotification, $languageCode); + $this->notificationManager->setPreparingPushNotification(false); + } + $userNotification = $this->preparedCallNotifications[$languageCode]; + } else { + $userNotification = $notification; + } + try { - $notification->setUser($userId); - $this->notificationManager->notify($notification); + $userNotification->setUser($userId); + $this->notificationManager->notify($userNotification); } catch (\InvalidArgumentException $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); } diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 683212f5bd2..b7a7f893b8b 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -197,10 +197,12 @@ public function prepare(INotification $notification, string $languageCode): INot throw new \InvalidArgumentException('Incorrect app'); } - $userId = $notification->getUser(); - $user = $this->userManager->get($userId); - if (!$user instanceof IUser || $this->config->isDisabledForUser($user)) { - throw new AlreadyProcessedException(); + if (!($this->notificationManager->isPreparingPushNotification() && $notification->getSubject() === 'call')) { + $userId = $notification->getUser(); + $user = $this->userManager->get($userId); + if (!$user instanceof IUser || $this->config->isDisabledForUser($user)) { + throw new AlreadyProcessedException(); + } } $l = $this->lFactory->get(Application::APP_ID, $languageCode); @@ -217,20 +219,27 @@ public function prepare(INotification $notification, string $languageCode): INot return $this->parseCertificateExpiration($notification, $l); } - try { - $room = $this->getRoom($notification->getObjectId(), $userId); - } catch (RoomNotFoundException $e) { - // Room does not exist - throw new AlreadyProcessedException(); - } - if ($this->notificationManager->isPreparingPushNotification() && $notification->getSubject() === 'call') { + try { + $room = $this->manager->getRoomByToken($notification->getObjectId()); + } catch (RoomNotFoundException) { + // Room does not exist + throw new AlreadyProcessedException(); + } + // Skip the participant check when we generate push notifications // we just looped over the participants to create the notification, // they can not be removed between these 2 steps, but we can save // n queries. $participant = null; } else { + try { + $room = $this->getRoom($notification->getObjectId(), $userId); + } catch (RoomNotFoundException $e) { + // Room does not exist + throw new AlreadyProcessedException(); + } + try { $participant = $this->getParticipant($room, $userId); } catch (ParticipantNotFoundException $e) {