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 <[email protected]>
nextcloud · May 7, 2024 · db80a4e · db80a4e
1 parent c139a64
commit db80a4e
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 13 deletions.
diff --git a/lib/Notification/Listener.php b/lib/Notification/Listener.php
@@ -24,10 +24,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;
 
 /**
@@ -36,10 +38,14 @@
 class Listener implements IEventListener {
 
 	protected bool $shouldSendCallNotification = false;
+	/** @var array<string, INotification> $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,
@@ -273,17 +279,52 @@ 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<string, string> $userLanguages */
+				$userLanguages = [];
+			} else {
+				$fallbackLang = $this->serverConfig->getSystemValueString('default_language', 'en');
+				/** @psalm-var array<string, string> $userLanguages */
+				$userLanguages = $this->serverConfig->getUserValueForUsers('core', 'lang', $userIds);
+			}
+		}
+
 		$this->connection->beginTransaction();
 		try {
 			foreach ($userIds as $userId) {
 				if ($actorId === $userId) {
 					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 = $translatedNotification;
+					} else {
+						$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
@@ -184,10 +184,12 @@ public function prepare(INotification $notification, string $languageCode): INot
 			throw new UnknownNotificationException('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);
@@ -204,20 +206,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) {