Notification events resolving and rendering in batches

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationData.kt

@@ -9,12 +9,14 @@ package io.element.android.libraries.matrix.api.notification
 
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.api.core.ThreadId
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.room.RoomMembershipState
 import io.element.android.libraries.matrix.api.timeline.item.event.MessageType
 
 data class NotificationData(
+    val sessionId: SessionId,
     val eventId: EventId,
     val threadId: ThreadId?,
     val roomId: RoomId,

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt

@@ -9,7 +9,8 @@ package io.element.android.libraries.matrix.api.notification
 
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.core.SessionId
 
 interface NotificationService {
-    suspend fun getNotification(roomId: RoomId, eventId: EventId): Result<NotificationData?>
+    suspend fun getNotifications(sessionId: SessionId, ids: Map<RoomId, List<EventId>>): Result<Map<EventId, NotificationData>>
 }

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/NotificationMapper.kt

@@ -10,6 +10,7 @@ package io.element.android.libraries.matrix.impl.notification
 import io.element.android.libraries.core.bool.orFalse
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.notification.NotificationContent
 import io.element.android.libraries.matrix.api.notification.NotificationData
@@ -25,6 +26,7 @@ class NotificationMapper(
     private val notificationContentMapper = NotificationContentMapper()
 
     fun map(
+        sessionId: SessionId,
         eventId: EventId,
         roomId: RoomId,
         notificationItem: NotificationItem
@@ -35,6 +37,7 @@ class NotificationMapper(
                 activeMembersCount = item.roomInfo.joinedMembersCount.toInt(),
             )
             NotificationData(
+                sessionId = sessionId,
                 eventId = eventId,
                 // FIXME once the `NotificationItem` in the SDK returns the thread id
                 threadId = null,

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt

@@ -10,12 +10,14 @@
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.api.notification.NotificationData
 import io.element.android.libraries.matrix.api.notification.NotificationService
 import io.element.android.services.toolbox.api.systemclock.SystemClock
 import kotlinx.coroutines.withContext
 import org.matrix.rustcomponents.sdk.NotificationClient
-import org.matrix.rustcomponents.sdk.use
+import org.matrix.rustcomponents.sdk.NotificationItemsRequest
+import timber.log.Timber
 
 class RustNotificationService(
     private val notificationClient: NotificationClient,
@@ -24,14 +26,29 @@
 ) : NotificationService {
     private val notificationMapper: NotificationMapper = NotificationMapper(clock)
 
-    override suspend fun getNotification(
-        roomId: RoomId,
-        eventId: EventId,
-    ): Result<NotificationData?> = withContext(dispatchers.io) {
+    override suspend fun getNotifications(
+        sessionId: SessionId,
+        ids: Map<RoomId, List<EventId>>
+    ): Result<Map<EventId, NotificationData>> = withContext(dispatchers.io) {
         runCatching {
-            val item = notificationClient.getNotification(roomId.value, eventId.value)
-            item?.use {
-                notificationMapper.map(eventId, roomId, it)
+            val requests = ids.map { (roomId, eventIds) ->
+                NotificationItemsRequest(
+                    roomId = roomId.value,
+                    eventIds = eventIds.map { it.value }
+                )
+            }
+            val items = notificationClient.getNotifications(requests)
+            buildMap {
+                val eventIds = requests.flatMap { it.eventIds }
+                for (eventId in eventIds) {
+                    val item = items[eventId]
+                    if (item != null) {
+                        val roomId = RoomId(requests.find { it.eventIds.contains(eventId) }?.roomId!!)
+                        put(EventId(eventId), notificationMapper.map(sessionId, EventId(eventId), roomId, item))
+                    } else {
+                        Timber.e("Could not retrieve event for notification with $eventId")
+                    }
+                }
             }
         }
     }

libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeRustNotificationClient.kt

@@ -7,15 +7,15 @@
 
 package io.element.android.libraries.matrix.impl.fixtures.fakes
 
-import io.element.android.tests.testutils.simulateLongTask
 import org.matrix.rustcomponents.sdk.NoPointer
 import org.matrix.rustcomponents.sdk.NotificationClient
 import org.matrix.rustcomponents.sdk.NotificationItem
+import org.matrix.rustcomponents.sdk.NotificationItemsRequest
 
 class FakeRustNotificationClient(
-    var notificationItemResult: NotificationItem? = null
+    var notificationItemResult: Map<String, NotificationItem> = emptyMap(),
 ) : NotificationClient(NoPointer) {
-    override suspend fun getNotification(roomId: String, eventId: String): NotificationItem? = simulateLongTask {
-        notificationItemResult
+    override suspend fun getNotifications(requests: List<NotificationItemsRequest>): Map<String, NotificationItem> {
+        return notificationItemResult
     }
 }

libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt

@@ -15,6 +15,7 @@ import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeRustNotificat
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
 import io.element.android.libraries.matrix.test.A_MESSAGE
 import io.element.android.libraries.matrix.test.A_ROOM_ID
+import io.element.android.libraries.matrix.test.A_SESSION_ID
 import io.element.android.libraries.matrix.test.A_USER_ID_2
 import io.element.android.services.toolbox.api.systemclock.SystemClock
 import io.element.android.services.toolbox.test.systemclock.FakeSystemClock
@@ -28,12 +29,12 @@ class RustNotificationServiceTest {
     @Test
     fun test() = runTest {
         val notificationClient = FakeRustNotificationClient(
-            notificationItemResult = aRustNotificationItem(),
+            notificationItemResult = mapOf(AN_EVENT_ID.value to aRustNotificationItem()),
         )
         val sut = createRustNotificationService(
             notificationClient = notificationClient,
         )
-        val result = sut.getNotification(A_ROOM_ID, AN_EVENT_ID).getOrThrow()!!
+        val result = sut.getNotifications(A_SESSION_ID, mapOf(A_ROOM_ID to listOf(AN_EVENT_ID))).getOrThrow()[AN_EVENT_ID]!!
         assertThat(result.isEncrypted).isTrue()
         assertThat(result.content).isEqualTo(
             NotificationContent.MessageLike.RoomMessage(

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt

@@ -9,20 +9,18 @@ package io.element.android.libraries.matrix.test.notification
 
 import io.element.android.libraries.matrix.api.core.EventId
 import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.api.notification.NotificationData
 import io.element.android.libraries.matrix.api.notification.NotificationService
 
 class FakeNotificationService : NotificationService {
-    private var getNotificationResult: Result<NotificationData?> = Result.success(null)
+    private var getNotificationsResult: Result<Map<EventId, NotificationData>> = Result.success(emptyMap())
 
-    fun givenGetNotificationResult(result: Result<NotificationData?>) {
-        getNotificationResult = result
+    fun givenGetNotificationsResult(result: Result<Map<EventId, NotificationData>>) {
+        getNotificationsResult = result
     }
 
-    override suspend fun getNotification(
-        roomId: RoomId,
-        eventId: EventId,
-    ): Result<NotificationData?> {
-        return getNotificationResult
+    override suspend fun getNotifications(sessionId: SessionId, ids: Map<RoomId, List<EventId>>): Result<Map<EventId, NotificationData>> {
+        return getNotificationsResult
     }
 }

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/NotificationData.kt

@@ -13,6 +13,7 @@ import io.element.android.libraries.matrix.api.notification.NotificationData
 import io.element.android.libraries.matrix.test.AN_EVENT_ID
 import io.element.android.libraries.matrix.test.A_ROOM_ID
 import io.element.android.libraries.matrix.test.A_ROOM_NAME
+import io.element.android.libraries.matrix.test.A_SESSION_ID
 import io.element.android.libraries.matrix.test.A_TIMESTAMP
 import io.element.android.libraries.matrix.test.A_USER_NAME_2
 
@@ -27,6 +28,7 @@ fun aNotificationData(
     roomDisplayName: String? = A_ROOM_NAME
 ): NotificationData {
     return NotificationData(
+        sessionId = A_SESSION_ID,
         eventId = AN_EVENT_ID,
         threadId = threadId,
         roomId = A_ROOM_ID,

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt

@@ -11,10 +11,10 @@ import android.content.Context
 import android.net.Uri
 import androidx.core.content.FileProvider
 import com.squareup.anvil.annotations.ContributesBinding
-import io.element.android.libraries.core.extensions.flatMap
 import io.element.android.libraries.core.log.logger.LoggerTag
 import io.element.android.libraries.di.AppScope
 import io.element.android.libraries.di.ApplicationContext
+import io.element.android.libraries.di.SingleIn
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.MatrixClientProvider
 import io.element.android.libraries.matrix.api.core.EventId
@@ -61,10 +61,14 @@ private val loggerTag = LoggerTag("DefaultNotifiableEventResolver", LoggerTag.No
  * this pattern allow decoupling between the object responsible of displaying notifications and the matrix sdk.
  */
 interface NotifiableEventResolver {
-    suspend fun resolveEvent(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result<ResolvedPushEvent>
+    suspend fun resolveEvents(
+        sessionId: SessionId,
+        notificationEventRequests: List<NotificationEventRequest>
+    ): Result<Map<NotificationEventRequest, Result<ResolvedPushEvent>>>
 }
 
 @ContributesBinding(AppScope::class)
+@SingleIn(AppScope::class)
 class DefaultNotifiableEventResolver @Inject constructor(
     private val stringProvider: StringProvider,
     private val clock: SystemClock,
@@ -75,29 +79,34 @@ class DefaultNotifiableEventResolver @Inject constructor(
     private val callNotificationEventResolver: CallNotificationEventResolver,
     private val appPreferencesStore: AppPreferencesStore,
 ) : NotifiableEventResolver {
-    override suspend fun resolveEvent(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result<ResolvedPushEvent> {
-        // Restore session
-        val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return Result.failure(
-            ResolvingException("Unable to restore session for $sessionId")
-        )
-        val notificationService = client.notificationService()
-        val notificationData = notificationService.getNotification(
-            roomId = roomId,
-            eventId = eventId,
-        ).onFailure {
-            Timber.tag(loggerTag.value).e(it, "Unable to resolve event: $eventId.")
+    override suspend fun resolveEvents(
+        sessionId: SessionId,
+        notificationEventRequests: List<NotificationEventRequest>
+    ): Result<Map<NotificationEventRequest, Result<ResolvedPushEvent>>> {
+        Timber.d("Queueing notifications: $notificationEventRequests")
+        val client = matrixClientProvider.getOrRestore(sessionId).getOrElse {
+            return Result.failure(IllegalStateException("Couldn't get or restore client for session $sessionId"))
         }
+        val ids = notificationEventRequests.groupBy { it.roomId }.mapValues { (_, value) -> value.map { it.eventId } }
 
         // TODO this notificationData is not always valid at the moment, sometimes the Rust SDK can't fetch the matching event
-        return notificationData.flatMap {
-            if (it == null) {
-                Timber.tag(loggerTag.value).d("No notification data found for event $eventId")
-                return@flatMap Result.failure(ResolvingException("Unable to resolve event $eventId"))
-            } else {
-                Timber.tag(loggerTag.value).d("Found notification item for $eventId")
-                it.asNotifiableEvent(client, sessionId)
+        val notifications = client.notificationService().getNotifications(sessionId, ids).mapCatching { map ->
+            map.mapValues { (_, notificationData) ->
+                notificationData.asNotifiableEvent(client, sessionId)
             }
         }
+
+        return Result.success(
+            notificationEventRequests.associate {
+                val notificationData = notifications.getOrNull()?.get(it.eventId)
+                if (notificationData != null) {
+                    it to notificationData
+                } else {
+                    // TODO once the SDK can actually return what went wrong, we should return it here instead of this generic error
+                    it to Result.failure(ResolvingException("No notification data for ${it.roomId} - ${it.eventId}"))
+                }
+            }
+        )
     }
 
     private suspend fun NotificationData.asNotifiableEvent(

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt

@@ -113,6 +113,11 @@
         renderEvents(listOf(notifiableEvent))
     }
 
+    suspend fun onNotifiableEventsReceived(notifiableEvents: List<NotifiableEvent>) {
+        val eventsToNotify = notifiableEvents.filter { !it.shouldIgnoreEventInRoom(appNavigationStateService.appNavigationState.value) }
+        renderEvents(eventsToNotify)
+    }
+
     /**
      * Clear all known message events for a [sessionId].
      */

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt

@@ -30,8 +30,9 @@ class DefaultOnMissedCallNotificationHandler @Inject constructor(
         // Resolve the event and add a notification for it, at this point it should no longer be a ringing one
         val notificationData = matrixClientProvider.getOrRestore(sessionId).getOrNull()
             ?.notificationService()
-            ?.getNotification(roomId, eventId)
+            ?.getNotifications(sessionId, mapOf(roomId to listOf(eventId)))
             ?.getOrNull()
+            ?.get(eventId)
             ?: return
 
         val notifiableEvent = callNotificationEventResolver.resolveEvent(

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt

@@ -159,7 +159,7 @@
             roomIsDm = room.isDm(),
             outGoingMessage = true,
         )
-        onNotifiableEventReceived.onNotifiableEventReceived(notifiableMessageEvent)
+        onNotifiableEventReceived.onNotifiableEventsReceived(listOf(notifiableMessageEvent))
 
         if (threadId != null && replyToEventId != null) {
             room.liveTimeline.replyMessage(
@@ -177,9 +177,11 @@
             )
         }.onFailure {
             Timber.e(it, "Failed to send smart reply message")
-            onNotifiableEventReceived.onNotifiableEventReceived(
-                notifiableMessageEvent.copy(
-                    outGoingMessageFailed = true
+            onNotifiableEventReceived.onNotifiableEventsReceived(
+                listOf(
+                    notifiableMessageEvent.copy(
+                        outGoingMessageFailed = true
+                    )
                 )
             )
         }

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDisplayer.kt

@@ -37,6 +37,7 @@ class DefaultNotificationDisplayer @Inject constructor(
             return false
         }
         notificationManager.notify(tag, id, notification)
+        Timber.d("Notifying with tag: $tag, id: $id")
         return true
     }