Skip to content

Commit f455085

Browse files
jmartinespjmartinesp
authored
Notification events resolving and rendering in batches (#4722)
- Use `NotiticationService.getNotifications()` function so we resolve the events in bulk. - Added `NotifierResolverQueue` to group the notifications to resolve based on a debounce strategy. - Batch rendering of these events as notifications.
1 parent f0c9f82 commit f455085

File tree

30 files changed

+883
-524
lines changed

30 files changed

+883
-524
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ package io.element.android.libraries.matrix.api.notification
99

1010
import io.element.android.libraries.matrix.api.core.EventId
1111
import io.element.android.libraries.matrix.api.core.RoomId
12+
import io.element.android.libraries.matrix.api.core.SessionId
1213
import io.element.android.libraries.matrix.api.core.ThreadId
1314
import io.element.android.libraries.matrix.api.core.UserId
1415
import io.element.android.libraries.matrix.api.room.RoomMembershipState
1516
import io.element.android.libraries.matrix.api.timeline.item.event.MessageType
1617

1718
data class NotificationData(
19+
val sessionId: SessionId,
1820
val eventId: EventId,
1921
val threadId: ThreadId?,
2022
val roomId: RoomId,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@ import io.element.android.libraries.matrix.api.core.EventId
1111
import io.element.android.libraries.matrix.api.core.RoomId
1212

1313
interface NotificationService {
14-
suspend fun getNotification(roomId: RoomId, eventId: EventId): Result<NotificationData?>
14+
suspend fun getNotifications(ids: Map<RoomId, List<EventId>>): Result<Map<EventId, NotificationData>>
1515
}

‎libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class RustMatrixClient(
152152
)
153153
private val notificationProcessSetup = NotificationProcessSetup.SingleProcess(innerSyncService)
154154
private val innerNotificationClient = runBlocking { innerClient.notificationClient(notificationProcessSetup) }
155-
private val notificationService = RustNotificationService(innerNotificationClient, dispatchers, clock)
155+
private val notificationService = RustNotificationService(sessionId, innerNotificationClient, dispatchers, clock)
156156
private val notificationSettingsService = RustNotificationSettingsService(innerClient, sessionCoroutineScope, dispatchers)
157157
private val encryptionService = RustEncryptionService(
158158
client = innerClient,

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ package io.element.android.libraries.matrix.impl.notification
1010
import io.element.android.libraries.core.bool.orFalse
1111
import io.element.android.libraries.matrix.api.core.EventId
1212
import io.element.android.libraries.matrix.api.core.RoomId
13+
import io.element.android.libraries.matrix.api.core.SessionId
1314
import io.element.android.libraries.matrix.api.core.UserId
1415
import io.element.android.libraries.matrix.api.notification.NotificationContent
1516
import io.element.android.libraries.matrix.api.notification.NotificationData
@@ -25,6 +26,7 @@ class NotificationMapper(
2526
private val notificationContentMapper = NotificationContentMapper()
2627

2728
fun map(
29+
sessionId: SessionId,
2830
eventId: EventId,
2931
roomId: RoomId,
3032
notificationItem: NotificationItem
@@ -35,6 +37,7 @@ class NotificationMapper(
3537
activeMembersCount = item.roomInfo.joinedMembersCount.toInt(),
3638
)
3739
NotificationData(
40+
sessionId = sessionId,
3841
eventId = eventId,
3942
// FIXME once the `NotificationItem` in the SDK returns the thread id
4043
threadId = null,

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

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,28 +10,45 @@ package io.element.android.libraries.matrix.impl.notification
1010
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
1111
import io.element.android.libraries.matrix.api.core.EventId
1212
import io.element.android.libraries.matrix.api.core.RoomId
13+
import io.element.android.libraries.matrix.api.core.SessionId
1314
import io.element.android.libraries.matrix.api.notification.NotificationData
1415
import io.element.android.libraries.matrix.api.notification.NotificationService
1516
import io.element.android.services.toolbox.api.systemclock.SystemClock
1617
import kotlinx.coroutines.withContext
1718
import org.matrix.rustcomponents.sdk.NotificationClient
18-
import org.matrix.rustcomponents.sdk.use
19+
import org.matrix.rustcomponents.sdk.NotificationItemsRequest
20+
import timber.log.Timber
1921

2022
class RustNotificationService(
23+
private val sessionId: SessionId,
2124
private val notificationClient: NotificationClient,
2225
private val dispatchers: CoroutineDispatchers,
2326
clock: SystemClock,
2427
) : NotificationService {
2528
private val notificationMapper: NotificationMapper = NotificationMapper(clock)
2629

27-
override suspend fun getNotification(
28-
roomId: RoomId,
29-
eventId: EventId,
30-
): Result<NotificationData?> = withContext(dispatchers.io) {
30+
override suspend fun getNotifications(
31+
ids: Map<RoomId, List<EventId>>
32+
): Result<Map<EventId, NotificationData>> = withContext(dispatchers.io) {
3133
runCatching {
32-
val item = notificationClient.getNotification(roomId.value, eventId.value)
33-
item?.use {
34-
notificationMapper.map(eventId, roomId, it)
34+
val requests = ids.map { (roomId, eventIds) ->
35+
NotificationItemsRequest(
36+
roomId = roomId.value,
37+
eventIds = eventIds.map { it.value }
38+
)
39+
}
40+
val items = notificationClient.getNotifications(requests)
41+
buildMap {
42+
val eventIds = requests.flatMap { it.eventIds }
43+
for (eventId in eventIds) {
44+
val item = items[eventId]
45+
if (item != null) {
46+
val roomId = RoomId(requests.find { it.eventIds.contains(eventId) }?.roomId!!)
47+
put(EventId(eventId), notificationMapper.map(sessionId, EventId(eventId), roomId, item))
48+
} else {
49+
Timber.e("Could not retrieve event for notification with $eventId")
50+
}
51+
}
3552
}
3653
}
3754
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77

88
package io.element.android.libraries.matrix.impl.fixtures.fakes
99

10-
import io.element.android.tests.testutils.simulateLongTask
1110
import org.matrix.rustcomponents.sdk.NoPointer
1211
import org.matrix.rustcomponents.sdk.NotificationClient
1312
import org.matrix.rustcomponents.sdk.NotificationItem
13+
import org.matrix.rustcomponents.sdk.NotificationItemsRequest
1414

1515
class FakeRustNotificationClient(
16-
var notificationItemResult: NotificationItem? = null
16+
var notificationItemResult: Map<String, NotificationItem> = emptyMap(),
1717
) : NotificationClient(NoPointer) {
18-
override suspend fun getNotification(roomId: String, eventId: String): NotificationItem? = simulateLongTask {
19-
notificationItemResult
18+
override suspend fun getNotifications(requests: List<NotificationItemsRequest>): Map<String, NotificationItem> {
19+
return notificationItemResult
2020
}
2121
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeRustNotificat
1515
import io.element.android.libraries.matrix.test.AN_EVENT_ID
1616
import io.element.android.libraries.matrix.test.A_MESSAGE
1717
import io.element.android.libraries.matrix.test.A_ROOM_ID
18+
import io.element.android.libraries.matrix.test.A_SESSION_ID
1819
import io.element.android.libraries.matrix.test.A_USER_ID_2
1920
import io.element.android.services.toolbox.api.systemclock.SystemClock
2021
import io.element.android.services.toolbox.test.systemclock.FakeSystemClock
@@ -28,12 +29,12 @@ class RustNotificationServiceTest {
2829
@Test
2930
fun test() = runTest {
3031
val notificationClient = FakeRustNotificationClient(
31-
notificationItemResult = aRustNotificationItem(),
32+
notificationItemResult = mapOf(AN_EVENT_ID.value to aRustNotificationItem()),
3233
)
3334
val sut = createRustNotificationService(
3435
notificationClient = notificationClient,
3536
)
36-
val result = sut.getNotification(A_ROOM_ID, AN_EVENT_ID).getOrThrow()!!
37+
val result = sut.getNotifications(mapOf(A_ROOM_ID to listOf(AN_EVENT_ID))).getOrThrow()[AN_EVENT_ID]!!
3738
assertThat(result.isEncrypted).isTrue()
3839
assertThat(result.content).isEqualTo(
3940
NotificationContent.MessageLike.RoomMessage(
@@ -51,6 +52,7 @@ class RustNotificationServiceTest {
5152
clock: SystemClock = FakeSystemClock(),
5253
) =
5354
RustNotificationService(
55+
sessionId = A_SESSION_ID,
5456
notificationClient = notificationClient,
5557
dispatchers = testCoroutineDispatchers(),
5658
clock = clock,

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,13 @@ import io.element.android.libraries.matrix.api.notification.NotificationData
1313
import io.element.android.libraries.matrix.api.notification.NotificationService
1414

1515
class FakeNotificationService : NotificationService {
16-
private var getNotificationResult: Result<NotificationData?> = Result.success(null)
16+
private var getNotificationsResult: Result<Map<EventId, NotificationData>> = Result.success(emptyMap())
1717

18-
fun givenGetNotificationResult(result: Result<NotificationData?>) {
19-
getNotificationResult = result
18+
fun givenGetNotificationsResult(result: Result<Map<EventId, NotificationData>>) {
19+
getNotificationsResult = result
2020
}
2121

22-
override suspend fun getNotification(
23-
roomId: RoomId,
24-
eventId: EventId,
25-
): Result<NotificationData?> {
26-
return getNotificationResult
22+
override suspend fun getNotifications(ids: Map<RoomId, List<EventId>>): Result<Map<EventId, NotificationData>> {
23+
return getNotificationsResult
2724
}
2825
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import io.element.android.libraries.matrix.api.notification.NotificationData
1313
import io.element.android.libraries.matrix.test.AN_EVENT_ID
1414
import io.element.android.libraries.matrix.test.A_ROOM_ID
1515
import io.element.android.libraries.matrix.test.A_ROOM_NAME
16+
import io.element.android.libraries.matrix.test.A_SESSION_ID
1617
import io.element.android.libraries.matrix.test.A_TIMESTAMP
1718
import io.element.android.libraries.matrix.test.A_USER_NAME_2
1819

@@ -27,6 +28,7 @@ fun aNotificationData(
2728
roomDisplayName: String? = A_ROOM_NAME
2829
): NotificationData {
2930
return NotificationData(
31+
sessionId = A_SESSION_ID,
3032
eventId = AN_EVENT_ID,
3133
threadId = threadId,
3234
roomId = A_ROOM_ID,

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

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ import android.content.Context
1111
import android.net.Uri
1212
import androidx.core.content.FileProvider
1313
import com.squareup.anvil.annotations.ContributesBinding
14-
import io.element.android.libraries.core.extensions.flatMap
1514
import io.element.android.libraries.core.log.logger.LoggerTag
1615
import io.element.android.libraries.di.AppScope
1716
import io.element.android.libraries.di.ApplicationContext
17+
import io.element.android.libraries.di.SingleIn
1818
import io.element.android.libraries.matrix.api.MatrixClient
1919
import io.element.android.libraries.matrix.api.MatrixClientProvider
2020
import io.element.android.libraries.matrix.api.core.EventId
@@ -61,10 +61,14 @@ private val loggerTag = LoggerTag("DefaultNotifiableEventResolver", LoggerTag.No
6161
* this pattern allow decoupling between the object responsible of displaying notifications and the matrix sdk.
6262
*/
6363
interface NotifiableEventResolver {
64-
suspend fun resolveEvent(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result<ResolvedPushEvent>
64+
suspend fun resolveEvents(
65+
sessionId: SessionId,
66+
notificationEventRequests: List<NotificationEventRequest>
67+
): Result<Map<NotificationEventRequest, Result<ResolvedPushEvent>>>
6568
}
6669

6770
@ContributesBinding(AppScope::class)
71+
@SingleIn(AppScope::class)
6872
class DefaultNotifiableEventResolver @Inject constructor(
6973
private val stringProvider: StringProvider,
7074
private val clock: SystemClock,
@@ -75,29 +79,34 @@ class DefaultNotifiableEventResolver @Inject constructor(
7579
private val callNotificationEventResolver: CallNotificationEventResolver,
7680
private val appPreferencesStore: AppPreferencesStore,
7781
) : NotifiableEventResolver {
78-
override suspend fun resolveEvent(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result<ResolvedPushEvent> {
79-
// Restore session
80-
val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return Result.failure(
81-
ResolvingException("Unable to restore session for $sessionId")
82-
)
83-
val notificationService = client.notificationService()
84-
val notificationData = notificationService.getNotification(
85-
roomId = roomId,
86-
eventId = eventId,
87-
).onFailure {
88-
Timber.tag(loggerTag.value).e(it, "Unable to resolve event: $eventId.")
82+
override suspend fun resolveEvents(
83+
sessionId: SessionId,
84+
notificationEventRequests: List<NotificationEventRequest>
85+
): Result<Map<NotificationEventRequest, Result<ResolvedPushEvent>>> {
86+
Timber.d("Queueing notifications: $notificationEventRequests")
87+
val client = matrixClientProvider.getOrRestore(sessionId).getOrElse {
88+
return Result.failure(IllegalStateException("Couldn't get or restore client for session $sessionId"))
8989
}
90+
val ids = notificationEventRequests.groupBy { it.roomId }.mapValues { (_, value) -> value.map { it.eventId } }
9091

9192
// TODO this notificationData is not always valid at the moment, sometimes the Rust SDK can't fetch the matching event
92-
return notificationData.flatMap {
93-
if (it == null) {
94-
Timber.tag(loggerTag.value).d("No notification data found for event $eventId")
95-
return@flatMap Result.failure(ResolvingException("Unable to resolve event $eventId"))
96-
} else {
97-
Timber.tag(loggerTag.value).d("Found notification item for $eventId")
98-
it.asNotifiableEvent(client, sessionId)
93+
val notifications = client.notificationService().getNotifications(ids).mapCatching { map ->
94+
map.mapValues { (_, notificationData) ->
95+
notificationData.asNotifiableEvent(client, sessionId)
9996
}
10097
}
98+
99+
return Result.success(
100+
notificationEventRequests.associate {
101+
val notificationData = notifications.getOrNull()?.get(it.eventId)
102+
if (notificationData != null) {
103+
it to notificationData
104+
} else {
105+
// TODO once the SDK can actually return what went wrong, we should return it here instead of this generic error
106+
it to Result.failure(ResolvingException("No notification data for ${it.roomId} - ${it.eventId}"))
107+
}
108+
}
109+
)
101110
}
102111

103112
private suspend fun NotificationData.asNotifiableEvent(

0 commit comments

Comments
 (0)