From e2724c7e812e63dec2d08a0f69a55b21e1ec1ab0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Tue, 6 May 2025 14:30:23 +0200 Subject: [PATCH 01/12] Notification batching PoC --- .../api/notification/NotificationService.kt | 1 + .../notification/RustNotificationService.kt | 31 +++++++ .../DefaultNotifiableEventResolver.kt | 82 ++++++++++++++++--- 3 files changed, 104 insertions(+), 10 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt index f229a46677a..84a0c19ab23 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt @@ -12,4 +12,5 @@ import io.element.android.libraries.matrix.api.core.RoomId interface NotificationService { suspend fun getNotification(roomId: RoomId, eventId: EventId): Result + suspend fun getNotifications(ids: Map>): Result> } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt index dfc891ace85..bebec0dbc83 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt @@ -15,7 +15,10 @@ 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.NotificationItemRequest +import org.matrix.rustcomponents.sdk.NotificationResult import org.matrix.rustcomponents.sdk.use +import timber.log.Timber class RustNotificationService( private val notificationClient: NotificationClient, @@ -36,6 +39,34 @@ class RustNotificationService( } } + override suspend fun getNotifications( + ids: Map> + ): Result> = withContext(dispatchers.io) { + runCatching { + val requests = ids.map { (roomId, eventIds) -> + NotificationItemRequest( + roomId = roomId.value, + eventIds = eventIds.map { it.value } + ) + } + val items = notificationClient.getNotifications(requests) + buildMap { + for ((id, item) in items) { + item.use { + val eventId = EventId(id) + when (it) { + is NotificationResult.Ok -> { + val roomId = RoomId(requests.find { it.eventIds.contains(eventId.value) }?.roomId!!) + put(eventId, notificationMapper.map(eventId, roomId, it.v1)) + } + is NotificationResult.Err -> Timber.e(it.v1, "Could not retrieve event for notification with $eventId") + } + } + } + } + } + } + fun close() { notificationClient.close() } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt index 982daec73e3..617eeb5d526 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt @@ -15,6 +15,7 @@ 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 @@ -48,9 +49,18 @@ import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEv import io.element.android.libraries.ui.strings.CommonStrings import io.element.android.services.toolbox.api.strings.StringProvider import io.element.android.services.toolbox.api.systemclock.SystemClock +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeout import timber.log.Timber import javax.inject.Inject +import kotlin.time.Duration.Companion.seconds private val loggerTag = LoggerTag("DefaultNotifiableEventResolver", LoggerTag.NotificationLoggerTag) @@ -65,6 +75,7 @@ interface NotifiableEventResolver { } @ContributesBinding(AppScope::class) +@SingleIn(AppScope::class) class DefaultNotifiableEventResolver @Inject constructor( private val stringProvider: StringProvider, private val clock: SystemClock, @@ -74,18 +85,19 @@ class DefaultNotifiableEventResolver @Inject constructor( private val permalinkParser: PermalinkParser, private val callNotificationEventResolver: CallNotificationEventResolver, private val appPreferencesStore: AppPreferencesStore, + private val appCoroutineScope: CoroutineScope, ) : NotifiableEventResolver { + + private val resolver = NotificationResolver( + matrixClientProvider = matrixClientProvider, + coroutineScope = appCoroutineScope, + ) + override suspend fun resolveEvent(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result { - // 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.") + Timber.d("Queueing notification for $sessionId: $roomId, $eventId") + resolver.queue(NotificationEventRequest(sessionId, eventId, roomId)) + val notificationData = runCatching { + withTimeout(30.seconds) { resolver.results.map { it[eventId] }.first() } } // TODO this notificationData is not always valid at the moment, sometimes the Rust SDK can't fetch the matching event @@ -95,6 +107,7 @@ class DefaultNotifiableEventResolver @Inject constructor( return@flatMap Result.failure(ResolvingException("Unable to resolve event $eventId")) } else { Timber.tag(loggerTag.value).d("Found notification item for $eventId") + val client = matrixClientProvider.getOrRestore(sessionId).getOrThrow() it.asNotifiableEvent(client, sessionId) } } @@ -400,3 +413,52 @@ internal fun buildNotifiableMessageEvent( type = type, hasMentionOrReply = hasMentionOrReply, ) + +class NotificationResolver( + private val matrixClientProvider: MatrixClientProvider, + private val coroutineScope: CoroutineScope, +) { + private val requests = Channel(capacity = 100) + + val results = MutableSharedFlow>() + + init { + coroutineScope.launch { + while (coroutineScope.isActive) { + delay(250) + val ids = buildList { + while (!requests.isEmpty) { + add(requests.receive()) + } + }.groupBy { it.sessionId } + + val sessionIds = ids.keys + for (sessionId in sessionIds) { + val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() + if (client != null) { + val notificationService = client.notificationService() + val requestsByRoom = ids[sessionId].orEmpty().groupBy { it.roomId }.mapValues { it.value.map { it.eventId } } + Timber.d("Fetching notifications for $sessionId: $requestsByRoom. Pending requests: ${!requests.isEmpty}") + + coroutineScope.launch { + val results = notificationService.getNotifications(requestsByRoom).getOrNull().orEmpty() + if (results.isNotEmpty()) { + this@NotificationResolver.results.emit(results) + } + } + } + } + } + } + } + + suspend fun queue(request: NotificationEventRequest) { + requests.send(request) + } +} + +data class NotificationEventRequest( + val sessionId: SessionId, + val eventId: EventId, + val roomId: RoomId, +) From 8778a1be7b70735dae4d11f33b10e8229ebb628a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 14 May 2025 16:43:58 +0200 Subject: [PATCH 02/12] Try batching display and removal of notifications too (sync on notification is disabled for the time being). --- .../api/notification/NotificationData.kt | 2 + .../api/notification/NotificationService.kt | 5 +- .../impl/notification/NotificationMapper.kt | 3 + .../notification/RustNotificationService.kt | 33 +++-- .../DefaultNotifiableEventResolver.kt | 97 +++----------- .../DefaultNotificationDrawerManager.kt | 5 + .../DefaultOnMissedCallNotificationHandler.kt | 2 +- .../notifications/NotificationDisplayer.kt | 1 + .../NotificationResolverQueue.kt | 82 ++++++++++++ .../factories/NotificationCreator.kt | 10 +- .../notifications/model/ResolvedPushEvent.kt | 18 ++- .../push/impl/push/DefaultPushHandler.kt | 126 +++++++++++------- .../impl/push/OnNotifiableEventReceived.kt | 11 +- .../push/impl/push/OnRedactedEventReceived.kt | 53 ++++++++ 14 files changed, 297 insertions(+), 151 deletions(-) create mode 100644 libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationData.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationData.kt index 5ac236ad800..338193ed44b 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationData.kt +++ b/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, diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt index 84a0c19ab23..93409b2d5e1 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt @@ -9,8 +9,9 @@ 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 - suspend fun getNotifications(ids: Map>): Result> + suspend fun getNotification(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result + suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/NotificationMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/NotificationMapper.kt index 7fdb4a9fdc0..9130df5c7a0 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/NotificationMapper.kt +++ b/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, diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt index bebec0dbc83..c6614b27a61 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt @@ -10,13 +10,13 @@ package io.element.android.libraries.matrix.impl.notification 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.NotificationItemRequest -import org.matrix.rustcomponents.sdk.NotificationResult +import org.matrix.rustcomponents.sdk.NotificationItemsRequest import org.matrix.rustcomponents.sdk.use import timber.log.Timber @@ -28,39 +28,44 @@ class RustNotificationService( private val notificationMapper: NotificationMapper = NotificationMapper(clock) override suspend fun getNotification( + sessionId: SessionId, roomId: RoomId, eventId: EventId, ): Result = withContext(dispatchers.io) { runCatching { val item = notificationClient.getNotification(roomId.value, eventId.value) item?.use { - notificationMapper.map(eventId, roomId, it) + notificationMapper.map( + sessionId = sessionId, + eventId = eventId, + roomId = roomId, + notificationItem = it + ) } } } override suspend fun getNotifications( + sessionId: SessionId, ids: Map> ): Result> = withContext(dispatchers.io) { runCatching { val requests = ids.map { (roomId, eventIds) -> - NotificationItemRequest( + NotificationItemsRequest( roomId = roomId.value, eventIds = eventIds.map { it.value } ) } val items = notificationClient.getNotifications(requests) buildMap { - for ((id, item) in items) { - item.use { - val eventId = EventId(id) - when (it) { - is NotificationResult.Ok -> { - val roomId = RoomId(requests.find { it.eventIds.contains(eventId.value) }?.roomId!!) - put(eventId, notificationMapper.map(eventId, roomId, it.v1)) - } - is NotificationResult.Err -> Timber.e(it.v1, "Could not retrieve event for notification with $eventId") - } + 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") } } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt index 617eeb5d526..64c9c0e2d71 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt @@ -11,7 +11,6 @@ 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 @@ -49,18 +48,9 @@ import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEv import io.element.android.libraries.ui.strings.CommonStrings import io.element.android.services.toolbox.api.strings.StringProvider import io.element.android.services.toolbox.api.systemclock.SystemClock -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.map -import kotlinx.coroutines.isActive -import kotlinx.coroutines.launch -import kotlinx.coroutines.withTimeout import timber.log.Timber import javax.inject.Inject -import kotlin.time.Duration.Companion.seconds private val loggerTag = LoggerTag("DefaultNotifiableEventResolver", LoggerTag.NotificationLoggerTag) @@ -71,7 +61,10 @@ 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 + suspend fun resolveEvents( + sessionId: SessionId, + notificationEventRequests: List + ): Result> } @ContributesBinding(AppScope::class) @@ -85,30 +78,21 @@ class DefaultNotifiableEventResolver @Inject constructor( private val permalinkParser: PermalinkParser, private val callNotificationEventResolver: CallNotificationEventResolver, private val appPreferencesStore: AppPreferencesStore, - private val appCoroutineScope: CoroutineScope, ) : NotifiableEventResolver { - - private val resolver = NotificationResolver( - matrixClientProvider = matrixClientProvider, - coroutineScope = appCoroutineScope, - ) - - override suspend fun resolveEvent(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result { - Timber.d("Queueing notification for $sessionId: $roomId, $eventId") - resolver.queue(NotificationEventRequest(sessionId, eventId, roomId)) - val notificationData = runCatching { - withTimeout(30.seconds) { resolver.results.map { it[eventId] }.first() } - } + override suspend fun resolveEvents( + sessionId: SessionId, + notificationEventRequests: List + ): Result> { + Timber.d("Queueing notifications: $notificationEventRequests") + // TODO fix throw + val client = matrixClientProvider.getOrRestore(sessionId).getOrThrow() + val ids = notificationEventRequests.groupBy { it.roomId }.mapValues { (_, value) -> value.map { it.eventId } } + val notifications = client.notificationService().getNotifications(sessionId, ids) // 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") - val client = matrixClientProvider.getOrRestore(sessionId).getOrThrow() - it.asNotifiableEvent(client, sessionId) + return notifications.mapCatching { map -> + map.mapValues { (_, notificationData) -> + notificationData?.asNotifiableEvent(client, sessionId)?.getOrNull() } } } @@ -413,52 +397,3 @@ internal fun buildNotifiableMessageEvent( type = type, hasMentionOrReply = hasMentionOrReply, ) - -class NotificationResolver( - private val matrixClientProvider: MatrixClientProvider, - private val coroutineScope: CoroutineScope, -) { - private val requests = Channel(capacity = 100) - - val results = MutableSharedFlow>() - - init { - coroutineScope.launch { - while (coroutineScope.isActive) { - delay(250) - val ids = buildList { - while (!requests.isEmpty) { - add(requests.receive()) - } - }.groupBy { it.sessionId } - - val sessionIds = ids.keys - for (sessionId in sessionIds) { - val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() - if (client != null) { - val notificationService = client.notificationService() - val requestsByRoom = ids[sessionId].orEmpty().groupBy { it.roomId }.mapValues { it.value.map { it.eventId } } - Timber.d("Fetching notifications for $sessionId: $requestsByRoom. Pending requests: ${!requests.isEmpty}") - - coroutineScope.launch { - val results = notificationService.getNotifications(requestsByRoom).getOrNull().orEmpty() - if (results.isNotEmpty()) { - this@NotificationResolver.results.emit(results) - } - } - } - } - } - } - } - - suspend fun queue(request: NotificationEventRequest) { - requests.send(request) - } -} - -data class NotificationEventRequest( - val sessionId: SessionId, - val eventId: EventId, - val roomId: RoomId, -) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt index 93e4bd85e5d..f485ba29542 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt @@ -113,6 +113,11 @@ class DefaultNotificationDrawerManager @Inject constructor( renderEvents(listOf(notifiableEvent)) } + suspend fun onNotifiableEventsReceived(notifiableEvents: List) { + val eventsToNotify = notifiableEvents.filter { !it.shouldIgnoreEventInRoom(appNavigationStateService.appNavigationState.value) } + renderEvents(eventsToNotify) + } + /** * Clear all known message events for a [sessionId]. */ diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt index 03a9c717d86..0b5f7680bc5 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt @@ -30,7 +30,7 @@ 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) + ?.getNotification(sessionId, roomId, eventId) ?.getOrNull() ?: return diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDisplayer.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDisplayer.kt index 09bfbb51ba4..88e6021cc43 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDisplayer.kt +++ b/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 } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt new file mode 100644 index 00000000000..27b926bc574 --- /dev/null +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt @@ -0,0 +1,82 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.libraries.push.impl.notifications + +import io.element.android.libraries.di.AppScope +import io.element.android.libraries.di.SingleIn +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.push.impl.notifications.model.ResolvedPushEvent +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.SharedFlow +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import timber.log.Timber +import java.time.LocalTime +import javax.inject.Inject +import kotlin.time.Duration.Companion.milliseconds + +@OptIn(ExperimentalCoroutinesApi::class) +@SingleIn(AppScope::class) +class NotificationResolverQueue @Inject constructor( + private val notifiableEventResolver: NotifiableEventResolver, + private val coroutineScope: CoroutineScope, +) { + companion object { + private const val BATCH_WINDOW_MS = 2000L + } + private val requestQueue = Channel(capacity = 100) + + val results: SharedFlow, List>> = MutableSharedFlow() + + init { + coroutineScope.launch { + while (coroutineScope.isActive) { + // Wait for a batch of requests to be received in a specified time window + delay(BATCH_WINDOW_MS.milliseconds) + + val groupedRequestsById = buildList { + while (!requestQueue.isEmpty) { + requestQueue.receiveCatching().getOrNull()?.let(this::add) + } + }.groupBy { it.sessionId } + + val sessionIds = groupedRequestsById.keys + for (sessionId in sessionIds) { + val requests = groupedRequestsById[sessionId].orEmpty() + Timber.d("Fetching notifications for $sessionId: $requests. Pending requests: ${!requestQueue.isEmpty}") + + launch { + // No need for a Mutex since the SDK already has one internally + val notifications = notifiableEventResolver.resolveEvents(sessionId, requests).getOrNull().orEmpty() + if (notifications.isNotEmpty()) { + (results as MutableSharedFlow).emit(requests to notifications.values.filterNotNull()) + } + } + } + } + } + } + + suspend fun enqueue(request: NotificationEventRequest) { + requestQueue.send(request) + } +} + +data class NotificationEventRequest( + val sessionId: SessionId, + val eventId: EventId, + val roomId: RoomId, + val providerInfo: String, + val timestamp: LocalTime = LocalTime.now(), +) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt index d462ebb5f6e..47797bbcf10 100755 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt @@ -14,7 +14,9 @@ import android.graphics.Canvas import androidx.annotation.DrawableRes import androidx.core.app.NotificationCompat import androidx.core.app.NotificationCompat.MessagingStyle +import androidx.core.app.NotificationManagerCompat import androidx.core.app.Person +import androidx.core.content.getSystemService import androidx.core.content.res.ResourcesCompat import coil3.ImageLoader import com.squareup.anvil.annotations.ContributesBinding @@ -41,6 +43,7 @@ import io.element.android.libraries.push.impl.notifications.model.InviteNotifiab import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent import io.element.android.libraries.push.impl.notifications.model.SimpleNotifiableEvent import io.element.android.services.toolbox.api.strings.StringProvider +import timber.log.Timber import javax.inject.Inject interface NotificationCreator { @@ -178,7 +181,11 @@ class DefaultNotificationCreator @Inject constructor( // 'importance' which is set in the NotificationChannel. The integers representing // 'priority' are different from 'importance', so make sure you don't mix them. .apply { - if (roomInfo.shouldBing) { + val notificationManager = NotificationManagerCompat.from(context) + val previousNotification = notificationManager.activeNotifications.find { it.tag == roomInfo.roomId.value } + val elapsed = System.currentTimeMillis() - (previousNotification?.postTime ?: 0L) + Timber.d("Creating noisy notification for room ${roomInfo.roomId.value} with elapsed time $elapsed") + if (roomInfo.shouldBing && elapsed > 1000L) { // Compat priority = NotificationCompat.PRIORITY_DEFAULT /* @@ -188,6 +195,7 @@ class DefaultNotificationCreator @Inject constructor( */ setLights(accentColor, 500, 500) } else { + Timber.d("Creating low priority notification") priority = NotificationCompat.PRIORITY_LOW } // Clear existing actions since we might be updating an existing notification diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/ResolvedPushEvent.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/ResolvedPushEvent.kt index 48e2af6e980..2363e38e1d7 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/ResolvedPushEvent.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/ResolvedPushEvent.kt @@ -12,12 +12,22 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId sealed interface ResolvedPushEvent { - data class Event(val notifiableEvent: NotifiableEvent) : ResolvedPushEvent + val sessionId: SessionId + val roomId: RoomId + val eventId: EventId + + data class Event(val notifiableEvent: NotifiableEvent) : ResolvedPushEvent { + override val sessionId: SessionId = notifiableEvent.sessionId + override val roomId: RoomId = notifiableEvent.roomId + override val eventId: EventId = notifiableEvent.eventId + } data class Redaction( - val sessionId: SessionId, - val roomId: RoomId, + override val sessionId: SessionId, + override val roomId: RoomId, val redactedEventId: EventId, val reason: String?, - ) : ResolvedPushEvent + ) : ResolvedPushEvent { + override val eventId: EventId = redactedEventId + } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt index f6b6ace8664..f15041e960c 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt @@ -13,6 +13,7 @@ import io.element.android.features.call.api.ElementCallEntryPoint import io.element.android.libraries.core.log.logger.LoggerTag import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.di.AppScope +import io.element.android.libraries.di.SingleIn import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService import io.element.android.libraries.push.impl.history.PushHistoryService import io.element.android.libraries.push.impl.history.onDiagnosticPush @@ -21,7 +22,10 @@ import io.element.android.libraries.push.impl.history.onSuccess import io.element.android.libraries.push.impl.history.onUnableToResolveEvent import io.element.android.libraries.push.impl.history.onUnableToRetrieveSession import io.element.android.libraries.push.impl.notifications.NotifiableEventResolver +import io.element.android.libraries.push.impl.notifications.NotificationEventRequest +import io.element.android.libraries.push.impl.notifications.NotificationResolverQueue import io.element.android.libraries.push.impl.notifications.channels.NotificationChannels +import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent import io.element.android.libraries.push.impl.notifications.model.NotifiableRingingCallEvent import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEvent import io.element.android.libraries.push.impl.test.DefaultTestPush @@ -30,17 +34,21 @@ import io.element.android.libraries.pushproviders.api.PushData import io.element.android.libraries.pushproviders.api.PushHandler import io.element.android.libraries.pushstore.api.UserPushStoreFactory import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.launch import timber.log.Timber import javax.inject.Inject private val loggerTag = LoggerTag("PushHandler", LoggerTag.PushLoggerTag) +@SingleIn(AppScope::class) @ContributesBinding(AppScope::class) class DefaultPushHandler @Inject constructor( private val onNotifiableEventReceived: OnNotifiableEventReceived, private val onRedactedEventReceived: OnRedactedEventReceived, - private val notifiableEventResolver: NotifiableEventResolver, private val incrementPushDataStore: IncrementPushDataStore, private val userPushStoreFactory: UserPushStoreFactory, private val pushClientSecret: PushClientSecret, @@ -50,7 +58,68 @@ class DefaultPushHandler @Inject constructor( private val elementCallEntryPoint: ElementCallEntryPoint, private val notificationChannels: NotificationChannels, private val pushHistoryService: PushHistoryService, + private val resolverQueue: NotificationResolverQueue, + private val appCoroutineScope: CoroutineScope, ) : PushHandler { + + init { + resolverQueue.results + .map { (requests, resolvedEvents) -> + for (request in requests) { + if (resolvedEvents.any { it.sessionId == request.sessionId && it.roomId == it.roomId && it.eventId == request.eventId }) { + pushHistoryService.onSuccess( + providerInfo = request.providerInfo, + eventId = request.eventId, + roomId = request.roomId, + sessionId = request.sessionId, + comment = "Push handled successfully", + ) + } else { + pushHistoryService.onUnableToResolveEvent( + providerInfo = request.providerInfo, + eventId = request.eventId, + roomId = request.roomId, + sessionId = request.sessionId, + reason = "Push not handled", + ) + } + } + + val events = mutableListOf() + val redactions = mutableListOf() + for (event in resolvedEvents) { + val userPushStore = userPushStoreFactory.getOrCreate(event.sessionId) + val areNotificationsEnabled = userPushStore.getNotificationEnabledForDevice().first() + // If notifications are disabled for this session and device, we don't want to show the notification + if (!areNotificationsEnabled) continue + + when (event) { + is ResolvedPushEvent.Event -> { + events.add(event.notifiableEvent) + } + is ResolvedPushEvent.Redaction -> { + redactions.add(event) + } + } + } + + if (redactions.isNotEmpty()) { + onRedactedEventReceived.onRedactedEventsReceived(redactions) + } + + val (ringingCallEvents, nonRingingCallEvents) = events.partition { it is NotifiableRingingCallEvent } + for (ringingCallEvent in ringingCallEvents) { + Timber.tag(loggerTag.value).d("Ringing call event: $ringingCallEvent") + handleRingingCallEvent(ringingCallEvent as NotifiableRingingCallEvent) + } + + if (nonRingingCallEvents.isNotEmpty()) { + onNotifiableEventReceived.onNotifiableEventsReceived(nonRingingCallEvents) + } + } + .launchIn(appCoroutineScope) + } + /** * Called when message is received. * @@ -119,52 +188,17 @@ class DefaultPushHandler @Inject constructor( ) return } - notifiableEventResolver.resolveEvent(userId, pushData.roomId, pushData.eventId).fold( - onSuccess = { resolvedPushEvent -> - pushHistoryService.onSuccess( - providerInfo = providerInfo, - eventId = pushData.eventId, - roomId = pushData.roomId, - sessionId = userId, - comment = resolvedPushEvent.javaClass.simpleName, - ) - when (resolvedPushEvent) { - is ResolvedPushEvent.Event -> { - when (val notifiableEvent = resolvedPushEvent.notifiableEvent) { - is NotifiableRingingCallEvent -> { - Timber.tag(loggerTag.value).d("Notifiable event ${pushData.eventId} is ringing call: $notifiableEvent") - onNotifiableEventReceived.onNotifiableEventReceived(notifiableEvent) - handleRingingCallEvent(notifiableEvent) - } - else -> { - Timber.tag(loggerTag.value).d("Notifiable event ${pushData.eventId} is normal event: $notifiableEvent") - val userPushStore = userPushStoreFactory.getOrCreate(userId) - val areNotificationsEnabled = userPushStore.getNotificationEnabledForDevice().first() - if (areNotificationsEnabled) { - onNotifiableEventReceived.onNotifiableEventReceived(notifiableEvent) - } else { - Timber.tag(loggerTag.value).i("Notification are disabled for this device, ignore push.") - } - } - } - } - is ResolvedPushEvent.Redaction -> { - onRedactedEventReceived.onRedactedEventReceived(resolvedPushEvent) - } - } - }, - onFailure = { failure -> - Timber.tag(loggerTag.value).w(failure, "Unable to get a notification data") - pushHistoryService.onUnableToResolveEvent( - providerInfo = providerInfo, - eventId = pushData.eventId, - roomId = pushData.roomId, - sessionId = userId, - reason = failure.message ?: failure.javaClass.simpleName, - ) - } - ) + appCoroutineScope.launch { + val notificationEventRequest = NotificationEventRequest( + sessionId = userId, + eventId = pushData.eventId, + roomId = pushData.roomId, + providerInfo = providerInfo, + ) + Timber.d("Queueing notification: $notificationEventRequest") + resolverQueue.enqueue(notificationEventRequest) + } } catch (e: Exception) { Timber.tag(loggerTag.value).e(e, "## handleInternal() failed") } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt index 7269dbcff77..c38b87c3344 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt @@ -18,20 +18,27 @@ import javax.inject.Inject interface OnNotifiableEventReceived { fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) + fun onNotifiableEventsReceived(notifiableEvents: List) } @ContributesBinding(AppScope::class) class DefaultOnNotifiableEventReceived @Inject constructor( private val defaultNotificationDrawerManager: DefaultNotificationDrawerManager, private val coroutineScope: CoroutineScope, - private val syncOnNotifiableEvent: SyncOnNotifiableEvent, +// private val syncOnNotifiableEvent: SyncOnNotifiableEvent, ) : OnNotifiableEventReceived { override fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) { coroutineScope.launch { - launch { syncOnNotifiableEvent(notifiableEvent) } +// launch { syncOnNotifiableEvent(notifiableEvent) } if (notifiableEvent !is NotifiableRingingCallEvent) { defaultNotificationDrawerManager.onNotifiableEventReceived(notifiableEvent) } } } + + override fun onNotifiableEventsReceived(notifiableEvents: List) { + coroutineScope.launch { + defaultNotificationDrawerManager.onNotifiableEventsReceived((notifiableEvents.filter { it !is NotifiableRingingCallEvent })) + } + } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnRedactedEventReceived.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnRedactedEventReceived.kt index bf452bd3b7b..f53348b0ee4 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnRedactedEventReceived.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnRedactedEventReceived.kt @@ -30,6 +30,7 @@ import javax.inject.Inject interface OnRedactedEventReceived { fun onRedactedEventReceived(redaction: ResolvedPushEvent.Redaction) + fun onRedactedEventsReceived(redactions: List) } @ContributesBinding(AppScope::class) @@ -85,4 +86,56 @@ class DefaultOnRedactedEventReceived @Inject constructor( } } } + + override fun onRedactedEventsReceived(redactions: List) { + coroutineScope.launch { + val redactionsBySessionIdAndRoom = redactions.groupBy { redaction -> + redaction.sessionId to redaction.roomId + } + for ((keys, roomRedactions) in redactionsBySessionIdAndRoom) { + val (sessionId, roomId) = keys + val notifications = activeNotificationsProvider.getMessageNotificationsForRoom( + sessionId, + roomId, + ) + if (notifications.isEmpty()) { + Timber.d("No notifications found for redacted event") + } + notifications.forEach { statusBarNotification -> + val notification = statusBarNotification.notification + val messagingStyle = MessagingStyle.extractMessagingStyleFromNotification(notification) + if (messagingStyle == null) { + Timber.w("Unable to retrieve messaging style from notification") + return@forEach + } + val messageToRedactIndex = messagingStyle.messages.indexOfFirst { message -> + roomRedactions.any { it.redactedEventId.value == message.extras.getString(DefaultNotificationCreator.MESSAGE_EVENT_ID) } + } + if (messageToRedactIndex == -1) { + Timber.d("Unable to find the message to remove from notification") + return@forEach + } + val oldMessage = messagingStyle.messages[messageToRedactIndex] + val content = buildSpannedString { + inSpans(StyleSpan(Typeface.ITALIC)) { + append(stringProvider.getString(CommonStrings.common_message_removed)) + } + } + val newMessage = MessagingStyle.Message( + content, + oldMessage.timestamp, + oldMessage.person + ) + messagingStyle.messages[messageToRedactIndex] = newMessage + notificationDisplayer.showNotificationMessage( + statusBarNotification.tag, + statusBarNotification.id, + NotificationCompat.Builder(context, notification) + .setStyle(messagingStyle) + .build() + ) + } + } + } + } } From 77d63f91073d5ca7420a58b12c2b2ce79627a4a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 14 May 2025 18:03:45 +0200 Subject: [PATCH 03/12] Try restoring `SyncOnNotifiableEvent` behaviour --- .../push/impl/push/DefaultPushHandler.kt | 1 - .../impl/push/OnNotifiableEventReceived.kt | 3 +- .../push/impl/push/SyncOnNotifiableEvent.kt | 32 +++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt index f15041e960c..2b26bfa9d7a 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt @@ -21,7 +21,6 @@ import io.element.android.libraries.push.impl.history.onInvalidPushReceived import io.element.android.libraries.push.impl.history.onSuccess import io.element.android.libraries.push.impl.history.onUnableToResolveEvent import io.element.android.libraries.push.impl.history.onUnableToRetrieveSession -import io.element.android.libraries.push.impl.notifications.NotifiableEventResolver import io.element.android.libraries.push.impl.notifications.NotificationEventRequest import io.element.android.libraries.push.impl.notifications.NotificationResolverQueue import io.element.android.libraries.push.impl.notifications.channels.NotificationChannels diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt index c38b87c3344..7c2e8f08656 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt @@ -25,7 +25,7 @@ interface OnNotifiableEventReceived { class DefaultOnNotifiableEventReceived @Inject constructor( private val defaultNotificationDrawerManager: DefaultNotificationDrawerManager, private val coroutineScope: CoroutineScope, -// private val syncOnNotifiableEvent: SyncOnNotifiableEvent, + private val syncOnNotifiableEvent: SyncOnNotifiableEvent, ) : OnNotifiableEventReceived { override fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) { coroutineScope.launch { @@ -38,6 +38,7 @@ class DefaultOnNotifiableEventReceived @Inject constructor( override fun onNotifiableEventsReceived(notifiableEvents: List) { coroutineScope.launch { + launch { syncOnNotifiableEvent.batch(notifiableEvents) } defaultNotificationDrawerManager.onNotifiableEventsReceived((notifiableEvents.filter { it !is NotifiableRingingCallEvent })) } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt index 4c5eee260b7..c41e99e6233 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt @@ -8,6 +8,7 @@ package io.element.android.libraries.push.impl.push import io.element.android.libraries.core.coroutine.CoroutineDispatchers +import io.element.android.libraries.core.coroutine.parallelMap import io.element.android.libraries.featureflag.api.FeatureFlagService import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.MatrixClientProvider @@ -31,6 +32,37 @@ class SyncOnNotifiableEvent @Inject constructor( private val appForegroundStateService: AppForegroundStateService, private val dispatchers: CoroutineDispatchers, ) { + suspend fun batch(notifiableEvents: List) = withContext(dispatchers.io) { + if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncOnPush)) { + return@withContext + } + + try { + val eventsBySession = notifiableEvents.groupBy { it.sessionId } + + appForegroundStateService.updateIsSyncingNotificationEvent(true) + + for ((sessionId, events) in eventsBySession) { + val client = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: continue + val eventsByRoomId = events.groupBy { it.roomId } + + client.roomListService.subscribeToVisibleRooms(eventsByRoomId.keys.toList()) + + if (!appForegroundStateService.isInForeground.value) { + for ((roomId, eventsInRoom) in eventsByRoomId) { + client.getJoinedRoom(roomId)?.use { room -> + eventsInRoom.parallelMap { event -> + room.waitsUntilEventIsKnown(event.eventId, timeout = 10.seconds) + } + } + } + } + } + } finally { + appForegroundStateService.updateIsSyncingNotificationEvent(false) + } + } + suspend operator fun invoke(notifiableEvent: NotifiableEvent) = withContext(dispatchers.io) { val isRingingCallEvent = notifiableEvent is NotifiableRingingCallEvent if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncOnPush) && !isRingingCallEvent) { From 51673056c4510aebc9e50455395a45ba714cb586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 15 May 2025 09:36:31 +0200 Subject: [PATCH 04/12] Reduce batch window time that was incresed for testing --- .../push/impl/notifications/NotificationResolverQueue.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt index 27b926bc574..bf2031c3c23 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt @@ -33,7 +33,7 @@ class NotificationResolverQueue @Inject constructor( private val coroutineScope: CoroutineScope, ) { companion object { - private const val BATCH_WINDOW_MS = 2000L + private const val BATCH_WINDOW_MS = 250L } private val requestQueue = Channel(capacity = 100) @@ -75,8 +75,8 @@ class NotificationResolverQueue @Inject constructor( data class NotificationEventRequest( val sessionId: SessionId, - val eventId: EventId, val roomId: RoomId, + val eventId: EventId, val providerInfo: String, val timestamp: LocalTime = LocalTime.now(), ) From ddc046cb7243f3e9344ecac1f14acc11fe223368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 15 May 2025 09:36:47 +0200 Subject: [PATCH 05/12] Make tests build again (they need refactor and fixes) --- .../RustNotificationServiceTest.kt | 3 +- .../notification/FakeNotificationService.kt | 11 ++++ .../test/notification/NotificationData.kt | 2 + .../push/impl/push/DefaultPushHandler.kt | 2 +- .../DefaultNotifiableEventResolverTest.kt | 58 +++++++++---------- .../FakeNotifiableEventResolver.kt | 10 +++- .../push/impl/push/DefaultPushHandlerTest.kt | 7 ++- .../push/FakeOnNotifiableEventReceived.kt | 6 ++ .../impl/push/FakeOnRedactedEventReceived.kt | 6 ++ 9 files changed, 70 insertions(+), 35 deletions(-) diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt index 71623337cd9..54b02ecc5e8 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt +++ b/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 @@ -33,7 +34,7 @@ class RustNotificationServiceTest { val sut = createRustNotificationService( notificationClient = notificationClient, ) - val result = sut.getNotification(A_ROOM_ID, AN_EVENT_ID).getOrThrow()!! + val result = sut.getNotification(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID).getOrThrow()!! assertThat(result.isEncrypted).isTrue() assertThat(result.content).isEqualTo( NotificationContent.MessageLike.RoomMessage( diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt index 38a8c79e175..c7f2be3c752 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt @@ -9,20 +9,31 @@ 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 = Result.success(null) + private var getNotificationsResult: Result> = Result.success(emptyMap()) fun givenGetNotificationResult(result: Result) { getNotificationResult = result } + fun givenGetNotificationsResult(result: Result>) { + getNotificationsResult = result + } + override suspend fun getNotification( + sessionId: SessionId, roomId: RoomId, eventId: EventId, ): Result { return getNotificationResult } + + override suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> { + return getNotificationsResult + } } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/NotificationData.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/NotificationData.kt index 85473d9367f..2bfd54954b7 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/NotificationData.kt +++ b/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, diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt index 2b26bfa9d7a..dc90f987de0 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt @@ -191,8 +191,8 @@ class DefaultPushHandler @Inject constructor( appCoroutineScope.launch { val notificationEventRequest = NotificationEventRequest( sessionId = userId, - eventId = pushData.eventId, roomId = pushData.roomId, + eventId = pushData.eventId, providerInfo = providerInfo, ) Timber.d("Queueing notification: $notificationEventRequest") diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt index cf9244166d6..a931fcf7f88 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt @@ -67,7 +67,7 @@ class DefaultNotifiableEventResolverTest { @Test fun `resolve event no session`() = runTest { val sut = createDefaultNotifiableEventResolver(notificationService = null) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.isFailure).isTrue() } @@ -76,7 +76,7 @@ class DefaultNotifiableEventResolverTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.failure(AN_EXCEPTION) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.isFailure).isTrue() } @@ -85,7 +85,7 @@ class DefaultNotifiableEventResolverTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success(null) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.isFailure).isTrue() } @@ -101,7 +101,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) @@ -122,7 +122,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world", hasMentionOrReply = true) ) @@ -147,7 +147,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) @@ -172,7 +172,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) @@ -191,7 +191,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Audio") ) @@ -210,7 +210,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Video") ) @@ -229,7 +229,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Voice message") ) @@ -248,7 +248,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Image") ) @@ -267,7 +267,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Sticker") ) @@ -286,7 +286,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "File") ) @@ -305,7 +305,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Location") ) @@ -324,7 +324,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Notice") ) @@ -343,7 +343,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "* Bob is happy") ) @@ -362,7 +362,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Poll: A question") ) @@ -382,7 +382,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.getOrNull()).isNull() } @@ -398,7 +398,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( InviteNotifiableEvent( sessionId = A_SESSION_ID, @@ -432,7 +432,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( InviteNotifiableEvent( sessionId = A_SESSION_ID, @@ -467,7 +467,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( InviteNotifiableEvent( sessionId = A_SESSION_ID, @@ -502,7 +502,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( InviteNotifiableEvent( sessionId = A_SESSION_ID, @@ -536,7 +536,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.getOrNull()).isNull() } @@ -549,7 +549,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( FallbackNotifiableEvent( sessionId = A_SESSION_ID, @@ -575,7 +575,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) val expectedResult = ResolvedPushEvent.Event( NotifiableMessageEvent( sessionId = A_SESSION_ID, @@ -639,7 +639,7 @@ class DefaultNotifiableEventResolverTest { ) ) callNotificationEventResolver.resolveEventLambda = { _, _, _ -> Result.success(expectedResult.notifiableEvent) } - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.getOrNull()).isEqualTo(expectedResult) } @@ -661,7 +661,7 @@ class DefaultNotifiableEventResolverTest { redactedEventId = AN_EVENT_ID_2, reason = A_REDACTION_REASON, ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.getOrNull()).isEqualTo(expectedResult) } @@ -677,7 +677,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.isFailure).isTrue() } @@ -725,7 +725,7 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID) + val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) assertThat(result.isFailure).isTrue() } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt index 3e98f8f9e85..01485353a9e 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt @@ -16,7 +16,13 @@ import io.element.android.tests.testutils.lambda.lambdaError class FakeNotifiableEventResolver( private val notifiableEventResult: (SessionId, RoomId, EventId) -> Result = { _, _, _ -> lambdaError() } ) : NotifiableEventResolver { - override suspend fun resolveEvent(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result { - return notifiableEventResult(sessionId, roomId, eventId) + override suspend fun resolveEvents(sessionId: SessionId, notificationEventRequests: List): Result> { + return notificationEventRequests.associate { + val eventId = it.eventId + val roomId = it.roomId + eventId to notifiableEventResult(sessionId, roomId, eventId).getOrNull() + }.let { resolvedEvents -> + Result.success(resolvedEvents) + } } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt index 77b71c0c696..e53b2f6068c 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt @@ -31,6 +31,7 @@ import io.element.android.libraries.matrix.test.core.aBuildMeta import io.element.android.libraries.push.impl.history.FakePushHistoryService import io.element.android.libraries.push.impl.history.PushHistoryService import io.element.android.libraries.push.impl.notifications.FakeNotifiableEventResolver +import io.element.android.libraries.push.impl.notifications.NotificationResolverQueue import io.element.android.libraries.push.impl.notifications.ResolvingException import io.element.android.libraries.push.impl.notifications.channels.FakeNotificationChannels import io.element.android.libraries.push.impl.notifications.fixtures.aNotifiableCallEvent @@ -50,6 +51,7 @@ import io.element.android.tests.testutils.lambda.lambdaError import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.junit.Test import java.time.Instant @@ -493,7 +495,7 @@ class DefaultPushHandlerTest { .isCalledOnce() } - private fun createDefaultPushHandler( + private fun TestScope.createDefaultPushHandler( onNotifiableEventReceived: (NotifiableEvent) -> Unit = { lambdaError() }, onRedactedEventReceived: (ResolvedPushEvent.Redaction) -> Unit = { lambdaError() }, notifiableEventResult: (SessionId, RoomId, EventId) -> Result = { _, _, _ -> lambdaError() }, @@ -510,7 +512,6 @@ class DefaultPushHandlerTest { return DefaultPushHandler( onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventReceived), onRedactedEventReceived = FakeOnRedactedEventReceived(onRedactedEventReceived), - notifiableEventResolver = FakeNotifiableEventResolver(notifiableEventResult), incrementPushDataStore = object : IncrementPushDataStore { override suspend fun incrementPushCounter() { incrementPushCounterResult() @@ -524,6 +525,8 @@ class DefaultPushHandlerTest { elementCallEntryPoint = elementCallEntryPoint, notificationChannels = notificationChannels, pushHistoryService = pushHistoryService, + resolverQueue = NotificationResolverQueue(notifiableEventResolver = FakeNotifiableEventResolver(notifiableEventResult), backgroundScope), + appCoroutineScope = backgroundScope, ) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnNotifiableEventReceived.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnNotifiableEventReceived.kt index 0de4094809a..206eeb25444 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnNotifiableEventReceived.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnNotifiableEventReceived.kt @@ -16,4 +16,10 @@ class FakeOnNotifiableEventReceived( override fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) { onNotifiableEventReceivedResult(notifiableEvent) } + + override fun onNotifiableEventsReceived(notifiableEvents: List) { + for (event in notifiableEvents) { + onNotifiableEventReceived(event) + } + } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnRedactedEventReceived.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnRedactedEventReceived.kt index 96e76f6da0a..262aeee1242 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnRedactedEventReceived.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnRedactedEventReceived.kt @@ -16,4 +16,10 @@ class FakeOnRedactedEventReceived( override fun onRedactedEventReceived(redaction: ResolvedPushEvent.Redaction) { onRedactedEventReceivedResult(redaction) } + + override fun onRedactedEventsReceived(redactions: List) { + for (redaction in redactions) { + onRedactedEventReceived(redaction) + } + } } From 735a88e4bc5b844df81e1cde092aed4d73ad0fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 15 May 2025 12:52:32 +0200 Subject: [PATCH 06/12] Fix tests, remove duplicates of methods added for lists of notifications --- .../api/notification/NotificationService.kt | 1 - .../notification/RustNotificationService.kt | 19 - .../fakes/FakeRustNotificationClient.kt | 8 +- .../RustNotificationServiceTest.kt | 4 +- .../notification/FakeNotificationService.kt | 13 - .../DefaultNotifiableEventResolver.kt | 5 +- .../DefaultOnMissedCallNotificationHandler.kt | 3 +- .../NotificationBroadcastReceiverHandler.kt | 10 +- .../NotificationResolverQueue.kt | 6 +- .../factories/NotificationCreator.kt | 1 - .../push/impl/push/DefaultPushHandler.kt | 5 +- .../impl/push/OnNotifiableEventReceived.kt | 14 +- .../push/impl/push/OnRedactedEventReceived.kt | 47 -- .../push/impl/push/SyncOnNotifiableEvent.kt | 42 +- .../DefaultNotifiableEventResolverTest.kt | 439 ++++++++++-------- ...aultOnMissedCallNotificationHandlerTest.kt | 4 +- .../FakeNotifiableEventResolver.kt | 16 +- ...otificationBroadcastReceiverHandlerTest.kt | 12 +- .../DefaultOnRedactedEventReceivedTest.kt | 4 +- .../push/impl/push/DefaultPushHandlerTest.kt | 148 +++--- .../push/FakeOnNotifiableEventReceived.kt | 10 +- .../impl/push/FakeOnRedactedEventReceived.kt | 10 +- .../impl/push/SyncOnNotifiableEventTest.kt | 30 +- 23 files changed, 382 insertions(+), 469 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt index 93409b2d5e1..99e6693110a 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt @@ -12,6 +12,5 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId interface NotificationService { - suspend fun getNotification(sessionId: SessionId, roomId: RoomId, eventId: EventId): Result suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt index c6614b27a61..1bd7285fc3c 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt @@ -17,7 +17,6 @@ import io.element.android.services.toolbox.api.systemclock.SystemClock import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.NotificationClient import org.matrix.rustcomponents.sdk.NotificationItemsRequest -import org.matrix.rustcomponents.sdk.use import timber.log.Timber class RustNotificationService( @@ -27,24 +26,6 @@ class RustNotificationService( ) : NotificationService { private val notificationMapper: NotificationMapper = NotificationMapper(clock) - override suspend fun getNotification( - sessionId: SessionId, - roomId: RoomId, - eventId: EventId, - ): Result = withContext(dispatchers.io) { - runCatching { - val item = notificationClient.getNotification(roomId.value, eventId.value) - item?.use { - notificationMapper.map( - sessionId = sessionId, - eventId = eventId, - roomId = roomId, - notificationItem = it - ) - } - } - } - override suspend fun getNotifications( sessionId: SessionId, ids: Map> diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeRustNotificationClient.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeRustNotificationClient.kt index 1e607351b91..802f20c509c 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeRustNotificationClient.kt +++ b/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 = emptyMap(), ) : NotificationClient(NoPointer) { - override suspend fun getNotification(roomId: String, eventId: String): NotificationItem? = simulateLongTask { - notificationItemResult + override suspend fun getNotifications(requests: List): Map { + return notificationItemResult } } diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt index 54b02ecc5e8..88d0234af55 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt @@ -29,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_SESSION_ID, 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( diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt index c7f2be3c752..3940fb3c490 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt @@ -14,25 +14,12 @@ 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 = Result.success(null) private var getNotificationsResult: Result> = Result.success(emptyMap()) - fun givenGetNotificationResult(result: Result) { - getNotificationResult = result - } - fun givenGetNotificationsResult(result: Result>) { getNotificationsResult = result } - override suspend fun getNotification( - sessionId: SessionId, - roomId: RoomId, - eventId: EventId, - ): Result { - return getNotificationResult - } - override suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> { return getNotificationsResult } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt index 64c9c0e2d71..2430d4bfed1 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt @@ -84,8 +84,9 @@ class DefaultNotifiableEventResolver @Inject constructor( notificationEventRequests: List ): Result> { Timber.d("Queueing notifications: $notificationEventRequests") - // TODO fix throw - val client = matrixClientProvider.getOrRestore(sessionId).getOrThrow() + 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 } } val notifications = client.notificationService().getNotifications(sessionId, ids) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt index 0b5f7680bc5..ca9fd29533d 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt +++ b/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(sessionId, roomId, eventId) + ?.getNotifications(sessionId, mapOf(roomId to listOf(eventId))) ?.getOrNull() + ?.get(eventId) ?: return val notifiableEvent = callNotificationEventResolver.resolveEvent( diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt index 9c899c9dfd0..1544efb5179 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt @@ -159,7 +159,7 @@ class NotificationBroadcastReceiverHandler @Inject constructor( roomIsDm = room.isDm(), outGoingMessage = true, ) - onNotifiableEventReceived.onNotifiableEventReceived(notifiableMessageEvent) + onNotifiableEventReceived.onNotifiableEventsReceived(listOf(notifiableMessageEvent)) if (threadId != null && replyToEventId != null) { room.liveTimeline.replyMessage( @@ -177,9 +177,11 @@ class NotificationBroadcastReceiverHandler @Inject constructor( ) }.onFailure { Timber.e(it, "Failed to send smart reply message") - onNotifiableEventReceived.onNotifiableEventReceived( - notifiableMessageEvent.copy( - outGoingMessageFailed = true + onNotifiableEventReceived.onNotifiableEventsReceived( + listOf( + notifiableMessageEvent.copy( + outGoingMessageFailed = true + ) ) ) } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt index bf2031c3c23..2caf43a98c1 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt @@ -22,7 +22,6 @@ import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import timber.log.Timber -import java.time.LocalTime import javax.inject.Inject import kotlin.time.Duration.Companion.milliseconds @@ -59,9 +58,7 @@ class NotificationResolverQueue @Inject constructor( launch { // No need for a Mutex since the SDK already has one internally val notifications = notifiableEventResolver.resolveEvents(sessionId, requests).getOrNull().orEmpty() - if (notifications.isNotEmpty()) { - (results as MutableSharedFlow).emit(requests to notifications.values.filterNotNull()) - } + (results as MutableSharedFlow).emit(requests to notifications.values.filterNotNull()) } } } @@ -78,5 +75,4 @@ data class NotificationEventRequest( val roomId: RoomId, val eventId: EventId, val providerInfo: String, - val timestamp: LocalTime = LocalTime.now(), ) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt index 47797bbcf10..1e4fb9afae6 100755 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt @@ -16,7 +16,6 @@ import androidx.core.app.NotificationCompat import androidx.core.app.NotificationCompat.MessagingStyle import androidx.core.app.NotificationManagerCompat import androidx.core.app.Person -import androidx.core.content.getSystemService import androidx.core.content.res.ResourcesCompat import coil3.ImageLoader import com.squareup.anvil.annotations.ContributesBinding diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt index dc90f987de0..d06a37cdc18 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt @@ -60,7 +60,6 @@ class DefaultPushHandler @Inject constructor( private val resolverQueue: NotificationResolverQueue, private val appCoroutineScope: CoroutineScope, ) : PushHandler { - init { resolverQueue.results .map { (requests, resolvedEvents) -> @@ -90,7 +89,9 @@ class DefaultPushHandler @Inject constructor( val userPushStore = userPushStoreFactory.getOrCreate(event.sessionId) val areNotificationsEnabled = userPushStore.getNotificationEnabledForDevice().first() // If notifications are disabled for this session and device, we don't want to show the notification - if (!areNotificationsEnabled) continue + // But if it's a ringing call, we want to show it anyway + val isRingingCall = (event as? ResolvedPushEvent.Event)?.notifiableEvent is NotifiableRingingCallEvent + if (!areNotificationsEnabled && !isRingingCall) continue when (event) { is ResolvedPushEvent.Event -> { diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt index 7c2e8f08656..14cb3d8ac64 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt @@ -17,7 +17,6 @@ import kotlinx.coroutines.launch import javax.inject.Inject interface OnNotifiableEventReceived { - fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) fun onNotifiableEventsReceived(notifiableEvents: List) } @@ -27,19 +26,10 @@ class DefaultOnNotifiableEventReceived @Inject constructor( private val coroutineScope: CoroutineScope, private val syncOnNotifiableEvent: SyncOnNotifiableEvent, ) : OnNotifiableEventReceived { - override fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) { - coroutineScope.launch { -// launch { syncOnNotifiableEvent(notifiableEvent) } - if (notifiableEvent !is NotifiableRingingCallEvent) { - defaultNotificationDrawerManager.onNotifiableEventReceived(notifiableEvent) - } - } - } - override fun onNotifiableEventsReceived(notifiableEvents: List) { coroutineScope.launch { - launch { syncOnNotifiableEvent.batch(notifiableEvents) } - defaultNotificationDrawerManager.onNotifiableEventsReceived((notifiableEvents.filter { it !is NotifiableRingingCallEvent })) + launch { syncOnNotifiableEvent(notifiableEvents) } + defaultNotificationDrawerManager.onNotifiableEventsReceived(notifiableEvents.filter { it !is NotifiableRingingCallEvent }) } } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnRedactedEventReceived.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnRedactedEventReceived.kt index f53348b0ee4..8cf77b18988 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnRedactedEventReceived.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnRedactedEventReceived.kt @@ -29,7 +29,6 @@ import timber.log.Timber import javax.inject.Inject interface OnRedactedEventReceived { - fun onRedactedEventReceived(redaction: ResolvedPushEvent.Redaction) fun onRedactedEventsReceived(redactions: List) } @@ -41,52 +40,6 @@ class DefaultOnRedactedEventReceived @Inject constructor( @ApplicationContext private val context: Context, private val stringProvider: StringProvider, ) : OnRedactedEventReceived { - override fun onRedactedEventReceived(redaction: ResolvedPushEvent.Redaction) { - coroutineScope.launch { - val notifications = activeNotificationsProvider.getMessageNotificationsForRoom( - redaction.sessionId, - redaction.roomId, - ) - if (notifications.isEmpty()) { - Timber.d("No notifications found for redacted event") - } - notifications.forEach { statusBarNotification -> - val notification = statusBarNotification.notification - val messagingStyle = MessagingStyle.extractMessagingStyleFromNotification(notification) - if (messagingStyle == null) { - Timber.w("Unable to retrieve messaging style from notification") - return@forEach - } - val messageToRedactIndex = messagingStyle.messages.indexOfFirst { message -> - message.extras.getString(DefaultNotificationCreator.MESSAGE_EVENT_ID) == redaction.redactedEventId.value - } - if (messageToRedactIndex == -1) { - Timber.d("Unable to find the message to remove from notification") - return@forEach - } - val oldMessage = messagingStyle.messages[messageToRedactIndex] - val content = buildSpannedString { - inSpans(StyleSpan(Typeface.ITALIC)) { - append(stringProvider.getString(CommonStrings.common_message_removed)) - } - } - val newMessage = MessagingStyle.Message( - content, - oldMessage.timestamp, - oldMessage.person - ) - messagingStyle.messages[messageToRedactIndex] = newMessage - notificationDisplayer.showNotificationMessage( - statusBarNotification.tag, - statusBarNotification.id, - NotificationCompat.Builder(context, notification) - .setStyle(messagingStyle) - .build() - ) - } - } - } - override fun onRedactedEventsReceived(redactions: List) { coroutineScope.launch { val redactionsBySessionIdAndRoom = redactions.groupBy { redaction -> diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt index c41e99e6233..3c1f1a0f0ab 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt @@ -13,11 +13,9 @@ import io.element.android.libraries.featureflag.api.FeatureFlagService import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.MatrixClientProvider import io.element.android.libraries.matrix.api.core.EventId -import io.element.android.libraries.matrix.api.room.BaseRoom import io.element.android.libraries.matrix.api.room.JoinedRoom import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent -import io.element.android.libraries.push.impl.notifications.model.NotifiableRingingCallEvent import io.element.android.services.appnavstate.api.AppForegroundStateService import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext @@ -32,7 +30,7 @@ class SyncOnNotifiableEvent @Inject constructor( private val appForegroundStateService: AppForegroundStateService, private val dispatchers: CoroutineDispatchers, ) { - suspend fun batch(notifiableEvents: List) = withContext(dispatchers.io) { + suspend operator fun invoke(notifiableEvents: List) = withContext(dispatchers.io) { if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncOnPush)) { return@withContext } @@ -63,44 +61,6 @@ class SyncOnNotifiableEvent @Inject constructor( } } - suspend operator fun invoke(notifiableEvent: NotifiableEvent) = withContext(dispatchers.io) { - val isRingingCallEvent = notifiableEvent is NotifiableRingingCallEvent - if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncOnPush) && !isRingingCallEvent) { - return@withContext - } - val client = matrixClientProvider.getOrRestore(notifiableEvent.sessionId).getOrNull() ?: return@withContext - - client.getJoinedRoom(notifiableEvent.roomId)?.use { room -> - room.subscribeToSync() - - // If the app is in foreground, sync is already running, so we just add the subscription above. - if (!appForegroundStateService.isInForeground.value) { - if (isRingingCallEvent) { - room.waitsUntilUserIsInTheCall(timeout = 60.seconds) - } else { - try { - appForegroundStateService.updateIsSyncingNotificationEvent(true) - room.waitsUntilEventIsKnown(eventId = notifiableEvent.eventId, timeout = 10.seconds) - } finally { - appForegroundStateService.updateIsSyncingNotificationEvent(false) - } - } - } - } - } - - /** - * User can be in the call if they answer using another session. - * If the user does not join the call, the timeout will be reached. - */ - private suspend fun BaseRoom.waitsUntilUserIsInTheCall(timeout: Duration) { - withTimeoutOrNull(timeout) { - roomInfoFlow.first { - sessionId in it.activeRoomCallParticipants - } - } - } - private suspend fun JoinedRoom.waitsUntilEventIsKnown(eventId: EventId, timeout: Duration) { withTimeoutOrNull(timeout) { liveTimeline.timelineItems.first { timelineItems -> diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt index a931fcf7f88..986b6a081aa 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt @@ -9,6 +9,7 @@ package io.element.android.libraries.push.impl.notifications import android.content.Context import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.media.MediaSource import io.element.android.libraries.matrix.api.notification.CallNotifyType import io.element.android.libraries.matrix.api.notification.NotificationContent @@ -83,21 +84,23 @@ class DefaultNotifiableEventResolverTest { @Test fun `resolve event null`() = runTest { val sut = createDefaultNotifiableEventResolver( - notificationResult = Result.success(null) + notificationResult = Result.success(emptyMap()) ) val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.isFailure).isTrue() + assertThat(result.getOrNull()?.isEmpty()).isTrue() } @Test fun `resolve event message text`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = TextMessageType(body = "Hello world", formatted = null) - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = TextMessageType(body = "Hello world", formatted = null) + ), + ) ) ) ) @@ -105,7 +108,7 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test @@ -113,12 +116,14 @@ class DefaultNotifiableEventResolverTest { fun `resolve event message with mention`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = TextMessageType(body = "Hello world", formatted = null) - ), - hasMention = true, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = TextMessageType(body = "Hello world", formatted = null) + ), + hasMention = true, + ) ) ) ) @@ -126,24 +131,26 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world", hasMentionOrReply = true) ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve HTML formatted event message text takes plain text version`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = TextMessageType( - body = "Hello world!", - formatted = FormattedBody( - body = "Hello world", - format = MessageFormat.HTML, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = TextMessageType( + body = "Hello world!", + formatted = FormattedBody( + body = "Hello world", + format = MessageFormat.HTML, + ) ) - ) - ), + ), + ) ) ) ) @@ -151,24 +158,26 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve incorrectly formatted event message text uses fallback`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = TextMessageType( - body = "Hello world", - formatted = FormattedBody( - body = "???Hello world!???", - format = MessageFormat.UNKNOWN, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = TextMessageType( + body = "Hello world", + formatted = FormattedBody( + body = "???Hello world!???", + format = MessageFormat.UNKNOWN, + ) ) - ) - ), + ), + ) ) ) ) @@ -176,18 +185,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message audio`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = AudioMessageType("Audio", null, null, MediaSource("url"), null) - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = AudioMessageType("Audio", null, null, MediaSource("url"), null) + ), + ) ) ) ) @@ -195,18 +206,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Audio") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message video`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = VideoMessageType("Video", null, null, MediaSource("url"), null) - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = VideoMessageType("Video", null, null, MediaSource("url"), null) + ), + ) ) ) ) @@ -214,18 +227,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Video") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message voice`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = VoiceMessageType("Voice", null, null, MediaSource("url"), null, null) - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = VoiceMessageType("Voice", null, null, MediaSource("url"), null, null) + ), + ) ) ) ) @@ -233,18 +248,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Voice message") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message image`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = ImageMessageType("Image", null, null, MediaSource("url"), null), - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = ImageMessageType("Image", null, null, MediaSource("url"), null), + ), + ) ) ) ) @@ -252,18 +269,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Image") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message sticker`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = StickerMessageType("Sticker", null, null, MediaSource("url"), null), - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = StickerMessageType("Sticker", null, null, MediaSource("url"), null), + ), + ) ) ) ) @@ -271,18 +290,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Sticker") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message file`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = FileMessageType("File", null, null, MediaSource("url"), null), - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = FileMessageType("File", null, null, MediaSource("url"), null), + ), + ) ) ) ) @@ -290,18 +311,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "File") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message location`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = LocationMessageType("Location", "geo:1,2", null), - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = LocationMessageType("Location", "geo:1,2", null), + ), + ) ) ) ) @@ -309,18 +332,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Location") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message notice`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = NoticeMessageType("Notice", null), - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = NoticeMessageType("Notice", null), + ), + ) ) ) ) @@ -328,18 +353,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Notice") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve event message emote`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomMessage( - senderId = A_USER_ID_2, - messageType = EmoteMessageType("is happy", null), - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomMessage( + senderId = A_USER_ID_2, + messageType = EmoteMessageType("is happy", null), + ), + ) ) ) ) @@ -347,18 +374,20 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "* Bob is happy") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve poll`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.Poll( - senderId = A_USER_ID_2, - question = "A question" - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.Poll( + senderId = A_USER_ID_2, + question = "A question" + ), + ) ) ) ) @@ -366,35 +395,39 @@ class DefaultNotifiableEventResolverTest { val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Poll: A question") ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve RoomMemberContent invite room`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.StateEvent.RoomMemberContent( - userId = A_USER_ID_2, - membershipState = RoomMembershipState.INVITE - ), - isDirect = false, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.StateEvent.RoomMemberContent( + userId = A_USER_ID_2, + membershipState = RoomMembershipState.INVITE + ), + isDirect = false, + ) ) ) ) val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()).isNull() + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isNull() } @Test fun `resolve invite room`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.Invite( - senderId = A_USER_ID_2, - ), - isDirect = false, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.Invite( + senderId = A_USER_ID_2, + ), + isDirect = false, + ) ) ) ) @@ -417,18 +450,20 @@ class DefaultNotifiableEventResolverTest { isUpdated = false, ) ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve invite direct`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.Invite( - senderId = A_USER_ID_2, - ), - isDirect = true, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.Invite( + senderId = A_USER_ID_2, + ), + isDirect = true, + ) ) ) ) @@ -451,19 +486,21 @@ class DefaultNotifiableEventResolverTest { isUpdated = false, ) ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve invite direct, no display name`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.Invite( - senderId = A_USER_ID_2, - ), - isDirect = true, - senderDisplayName = null, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.Invite( + senderId = A_USER_ID_2, + ), + isDirect = true, + senderDisplayName = null, + ) ) ) ) @@ -486,19 +523,21 @@ class DefaultNotifiableEventResolverTest { isUpdated = false, ) ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve invite direct, ambiguous display name`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.Invite( - senderId = A_USER_ID_2, - ), - isDirect = false, - senderIsNameAmbiguous = true, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.Invite( + senderId = A_USER_ID_2, + ), + isDirect = false, + senderIsNameAmbiguous = true, + ) ) ) ) @@ -521,32 +560,32 @@ class DefaultNotifiableEventResolverTest { isUpdated = false, ) ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve RoomMemberContent other`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.StateEvent.RoomMemberContent( - userId = A_USER_ID_2, - membershipState = RoomMembershipState.JOIN + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.StateEvent.RoomMemberContent( + userId = A_USER_ID_2, + membershipState = RoomMembershipState.JOIN + ) ) ) ) ) val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()).isNull() + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isNull() } @Test fun `resolve RoomEncrypted`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomEncrypted - ) + mapOf(AN_EVENT_ID to aNotificationData(content = NotificationContent.MessageLike.RoomEncrypted)) ) ) val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) @@ -563,15 +602,17 @@ class DefaultNotifiableEventResolverTest { timestamp = A_FAKE_TIMESTAMP, ) ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve CallInvite`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.CallInvite(A_USER_ID_2), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.CallInvite(A_USER_ID_2), + ) ) ) ) @@ -601,7 +642,7 @@ class DefaultNotifiableEventResolverTest { isUpdated = false ) ) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test @@ -609,11 +650,13 @@ class DefaultNotifiableEventResolverTest { val callNotificationEventResolver = FakeCallNotificationEventResolver() val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.CallNotify( - A_USER_ID_2, - CallNotifyType.NOTIFY - ), + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.CallNotify( + A_USER_ID_2, + CallNotifyType.NOTIFY + ), + ) ) ), callNotificationEventResolver = callNotificationEventResolver, @@ -640,17 +683,19 @@ class DefaultNotifiableEventResolverTest { ) callNotificationEventResolver.resolveEventLambda = { _, _, _ -> Result.success(expectedResult.notifiableEvent) } val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve RoomRedaction`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomRedaction( - AN_EVENT_ID_2, - A_REDACTION_REASON, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomRedaction( + AN_EVENT_ID_2, + A_REDACTION_REASON, + ) ) ) ) @@ -662,81 +707,81 @@ class DefaultNotifiableEventResolverTest { reason = A_REDACTION_REASON, ) val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()).isEqualTo(expectedResult) + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) } @Test fun `resolve RoomRedaction with null redactedEventId should return null`() = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = NotificationContent.MessageLike.RoomRedaction( - null, - A_REDACTION_REASON, + mapOf( + AN_EVENT_ID to aNotificationData( + content = NotificationContent.MessageLike.RoomRedaction( + null, + A_REDACTION_REASON, + ) ) ) ) ) val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.isFailure).isTrue() + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isNull() } @Test fun `resolve null cases`() { - testFailure(NotificationContent.MessageLike.CallAnswer) - testFailure(NotificationContent.MessageLike.CallHangup) - testFailure(NotificationContent.MessageLike.CallCandidates) - testFailure(NotificationContent.MessageLike.KeyVerificationReady) - testFailure(NotificationContent.MessageLike.KeyVerificationStart) - testFailure(NotificationContent.MessageLike.KeyVerificationCancel) - testFailure(NotificationContent.MessageLike.KeyVerificationAccept) - testFailure(NotificationContent.MessageLike.KeyVerificationKey) - testFailure(NotificationContent.MessageLike.KeyVerificationMac) - testFailure(NotificationContent.MessageLike.KeyVerificationDone) - testFailure(NotificationContent.MessageLike.ReactionContent(relatedEventId = AN_EVENT_ID_2.value)) - testFailure(NotificationContent.MessageLike.Sticker) - testFailure(NotificationContent.StateEvent.PolicyRuleRoom) - testFailure(NotificationContent.StateEvent.PolicyRuleServer) - testFailure(NotificationContent.StateEvent.PolicyRuleUser) - testFailure(NotificationContent.StateEvent.RoomAliases) - testFailure(NotificationContent.StateEvent.RoomAvatar) - testFailure(NotificationContent.StateEvent.RoomCanonicalAlias) - testFailure(NotificationContent.StateEvent.RoomCreate) - testFailure(NotificationContent.StateEvent.RoomEncryption) - testFailure(NotificationContent.StateEvent.RoomGuestAccess) - testFailure(NotificationContent.StateEvent.RoomHistoryVisibility) - testFailure(NotificationContent.StateEvent.RoomJoinRules) - testFailure(NotificationContent.StateEvent.RoomName) - testFailure(NotificationContent.StateEvent.RoomPinnedEvents) - testFailure(NotificationContent.StateEvent.RoomPowerLevels) - testFailure(NotificationContent.StateEvent.RoomServerAcl) - testFailure(NotificationContent.StateEvent.RoomThirdPartyInvite) - testFailure(NotificationContent.StateEvent.RoomTombstone) - testFailure(NotificationContent.StateEvent.RoomTopic("")) - testFailure(NotificationContent.StateEvent.SpaceChild) - testFailure(NotificationContent.StateEvent.SpaceParent) + testNoResults(NotificationContent.MessageLike.CallAnswer) + testNoResults(NotificationContent.MessageLike.CallHangup) + testNoResults(NotificationContent.MessageLike.CallCandidates) + testNoResults(NotificationContent.MessageLike.KeyVerificationReady) + testNoResults(NotificationContent.MessageLike.KeyVerificationStart) + testNoResults(NotificationContent.MessageLike.KeyVerificationCancel) + testNoResults(NotificationContent.MessageLike.KeyVerificationAccept) + testNoResults(NotificationContent.MessageLike.KeyVerificationKey) + testNoResults(NotificationContent.MessageLike.KeyVerificationMac) + testNoResults(NotificationContent.MessageLike.KeyVerificationDone) + testNoResults(NotificationContent.MessageLike.ReactionContent(relatedEventId = AN_EVENT_ID_2.value)) + testNoResults(NotificationContent.MessageLike.Sticker) + testNoResults(NotificationContent.StateEvent.PolicyRuleRoom) + testNoResults(NotificationContent.StateEvent.PolicyRuleServer) + testNoResults(NotificationContent.StateEvent.PolicyRuleUser) + testNoResults(NotificationContent.StateEvent.RoomAliases) + testNoResults(NotificationContent.StateEvent.RoomAvatar) + testNoResults(NotificationContent.StateEvent.RoomCanonicalAlias) + testNoResults(NotificationContent.StateEvent.RoomCreate) + testNoResults(NotificationContent.StateEvent.RoomEncryption) + testNoResults(NotificationContent.StateEvent.RoomGuestAccess) + testNoResults(NotificationContent.StateEvent.RoomHistoryVisibility) + testNoResults(NotificationContent.StateEvent.RoomJoinRules) + testNoResults(NotificationContent.StateEvent.RoomName) + testNoResults(NotificationContent.StateEvent.RoomPinnedEvents) + testNoResults(NotificationContent.StateEvent.RoomPowerLevels) + testNoResults(NotificationContent.StateEvent.RoomServerAcl) + testNoResults(NotificationContent.StateEvent.RoomThirdPartyInvite) + testNoResults(NotificationContent.StateEvent.RoomTombstone) + testNoResults(NotificationContent.StateEvent.RoomTopic("")) + testNoResults(NotificationContent.StateEvent.SpaceChild) + testNoResults(NotificationContent.StateEvent.SpaceParent) } - private fun testFailure(content: NotificationContent) = runTest { + private fun testNoResults(content: NotificationContent) = runTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.success( - aNotificationData( - content = content - ) + mapOf(AN_EVENT_ID to aNotificationData(content = content)) ) ) val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.isFailure).isTrue() + assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isNull() } private fun createDefaultNotifiableEventResolver( notificationService: FakeNotificationService? = FakeNotificationService(), - notificationResult: Result = Result.success(null), + notificationResult: Result> = Result.success(emptyMap()), appPreferencesStore: AppPreferencesStore = InMemoryAppPreferencesStore(), callNotificationEventResolver: FakeCallNotificationEventResolver = FakeCallNotificationEventResolver(), ): DefaultNotifiableEventResolver { val context = RuntimeEnvironment.getApplication() as Context - notificationService?.givenGetNotificationResult(notificationResult) + notificationService?.givenGetNotificationsResult(notificationResult) val matrixClientProvider = FakeMatrixClientProvider(getClient = { if (notificationService == null) { Result.failure(IllegalStateException("Client not found")) diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandlerTest.kt index 6a4cd52430c..1dc21a7ebee 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandlerTest.kt @@ -42,7 +42,9 @@ class DefaultOnMissedCallNotificationHandlerTest { // Create a fake matrix client provider that returns a fake matrix client with a fake notification service that returns a valid notification data val matrixClientProvider = FakeMatrixClientProvider(getClient = { val notificationService = FakeNotificationService().apply { - givenGetNotificationResult(Result.success(aNotificationData(senderDisplayName = A_USER_NAME, senderIsNameAmbiguous = false))) + givenGetNotificationsResult( + Result.success(mapOf(AN_EVENT_ID to aNotificationData(senderDisplayName = A_USER_NAME, senderIsNameAmbiguous = false))) + ) } Result.success(FakeMatrixClient(notificationService = notificationService)) }) diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt index 01485353a9e..ff56a79cf4d 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt @@ -8,21 +8,17 @@ package io.element.android.libraries.push.impl.notifications 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.push.impl.notifications.model.ResolvedPushEvent import io.element.android.tests.testutils.lambda.lambdaError class FakeNotifiableEventResolver( - private val notifiableEventResult: (SessionId, RoomId, EventId) -> Result = { _, _, _ -> lambdaError() } + private val resolveEventsResult: (SessionId, List) -> Result> = { _, _ -> lambdaError() } ) : NotifiableEventResolver { - override suspend fun resolveEvents(sessionId: SessionId, notificationEventRequests: List): Result> { - return notificationEventRequests.associate { - val eventId = it.eventId - val roomId = it.roomId - eventId to notifiableEventResult(sessionId, roomId, eventId).getOrNull() - }.let { resolvedEvents -> - Result.success(resolvedEvents) - } + override suspend fun resolveEvents( + sessionId: SessionId, + notificationEventRequests: List + ): Result> { + return resolveEventsResult(sessionId, notificationEventRequests) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt index dfa440bcf05..4d781152ef4 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandlerTest.kt @@ -353,8 +353,8 @@ class NotificationBroadcastReceiverHandlerTest { ) ) } - val onNotifiableEventReceivedResult = lambdaRecorder { _ -> } - val onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventReceivedResult = onNotifiableEventReceivedResult) + val onNotifiableEventsReceivedResult = lambdaRecorder, Unit> { _ -> } + val onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventsReceivedResult = onNotifiableEventsReceivedResult) val sut = createNotificationBroadcastReceiverHandler( joinedRoom = joinedRoom, onNotifiableEventReceived = onNotifiableEventReceived, @@ -370,7 +370,7 @@ class NotificationBroadcastReceiverHandlerTest { sendMessage.assertions() .isCalledOnce() .with(value(A_MESSAGE), value(null), value(emptyList())) - onNotifiableEventReceivedResult.assertions() + onNotifiableEventsReceivedResult.assertions() .isCalledOnce() replyMessage.assertions() .isNeverCalled() @@ -420,8 +420,8 @@ class NotificationBroadcastReceiverHandlerTest { ) ) } - val onNotifiableEventReceivedResult = lambdaRecorder { _ -> } - val onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventReceivedResult = onNotifiableEventReceivedResult) + val onNotifiableEventsReceivedResult = lambdaRecorder, Unit> { _ -> } + val onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventsReceivedResult = onNotifiableEventsReceivedResult) val sut = createNotificationBroadcastReceiverHandler( joinedRoom = joinedRoom, onNotifiableEventReceived = onNotifiableEventReceived, @@ -438,7 +438,7 @@ class NotificationBroadcastReceiverHandlerTest { runCurrent() sendMessage.assertions() .isNeverCalled() - onNotifiableEventReceivedResult.assertions() + onNotifiableEventsReceivedResult.assertions() .isCalledOnce() replyMessage.assertions() .isCalledOnce() diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultOnRedactedEventReceivedTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultOnRedactedEventReceivedTest.kt index 2c8583bb614..1b08f7b14c4 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultOnRedactedEventReceivedTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultOnRedactedEventReceivedTest.kt @@ -34,7 +34,7 @@ class DefaultOnRedactedEventReceivedTest { val sut = createDefaultOnRedactedEventReceived( getMessageNotificationsForRoomResult = { _, _ -> emptyList() } ) - sut.onRedactedEventReceived(ResolvedPushEvent.Redaction(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, null)) + sut.onRedactedEventsReceived(listOf(ResolvedPushEvent.Redaction(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, null))) } @Test @@ -48,7 +48,7 @@ class DefaultOnRedactedEventReceivedTest { ) } ) - sut.onRedactedEventReceived(ResolvedPushEvent.Redaction(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, null)) + sut.onRedactedEventsReceived(listOf(ResolvedPushEvent.Redaction(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, null))) } private fun TestScope.createDefaultOnRedactedEventReceived( diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt index e53b2f6068c..60d058fce53 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt @@ -31,6 +31,7 @@ import io.element.android.libraries.matrix.test.core.aBuildMeta import io.element.android.libraries.push.impl.history.FakePushHistoryService import io.element.android.libraries.push.impl.history.PushHistoryService import io.element.android.libraries.push.impl.notifications.FakeNotifiableEventResolver +import io.element.android.libraries.push.impl.notifications.NotificationEventRequest import io.element.android.libraries.push.impl.notifications.NotificationResolverQueue import io.element.android.libraries.push.impl.notifications.ResolvingException import io.element.android.libraries.push.impl.notifications.channels.FakeNotificationChannels @@ -52,9 +53,11 @@ import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runTest import org.junit.Test import java.time.Instant +import kotlin.time.Duration.Companion.milliseconds private const val A_PUSHER_INFO = "info" @@ -82,10 +85,10 @@ class DefaultPushHandlerTest { fun `when classical PushData is received, the notification drawer is informed`() = runTest { val aNotifiableMessageEvent = aNotifiableMessageEvent() val notifiableEventResult = - lambdaRecorder> { _, _, _ -> - Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)) + lambdaRecorder, Result>> { _, _, -> + Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent))) } - val onNotifiableEventReceived = lambdaRecorder {} + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} val onPushReceivedResult = lambdaRecorder { _, _, _, _, _, _, _ -> } val pushHistoryService = FakePushHistoryService( @@ -98,8 +101,8 @@ class DefaultPushHandlerTest { clientSecret = A_SECRET, ) val defaultPushHandler = createDefaultPushHandler( - onNotifiableEventReceived = onNotifiableEventReceived, - notifiableEventResult = notifiableEventResult, + onNotifiableEventsReceived = onNotifiableEventsReceived, + notifiableEventsResult = notifiableEventResult, pushClientSecret = FakePushClientSecret( getUserIdFromSecretResult = { A_USER_ID } ), @@ -107,14 +110,17 @@ class DefaultPushHandlerTest { pushHistoryService = pushHistoryService, ) defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + + advanceTimeBy(300.milliseconds) + incrementPushCounterResult.assertions() .isCalledOnce() notifiableEventResult.assertions() .isCalledOnce() - .with(value(A_USER_ID), value(A_ROOM_ID), value(AN_EVENT_ID)) - onNotifiableEventReceived.assertions() + .with(value(A_USER_ID), any()) + onNotifiableEventsReceived.assertions() .isCalledOnce() - .with(value(aNotifiableMessageEvent)) + .with(value(listOf(aNotifiableMessageEvent))) onPushReceivedResult.assertions() .isCalledOnce() } @@ -124,10 +130,10 @@ class DefaultPushHandlerTest { runTest { val aNotifiableMessageEvent = aNotifiableMessageEvent() val notifiableEventResult = - lambdaRecorder> { _, _, _ -> - Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)) + lambdaRecorder, Result>> { _, _ -> + Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent))) } - val onNotifiableEventReceived = lambdaRecorder {} + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} val aPushData = PushData( eventId = AN_EVENT_ID, @@ -140,8 +146,8 @@ class DefaultPushHandlerTest { onPushReceivedResult = onPushReceivedResult, ) val defaultPushHandler = createDefaultPushHandler( - onNotifiableEventReceived = onNotifiableEventReceived, - notifiableEventResult = notifiableEventResult, + onNotifiableEventsReceived = onNotifiableEventsReceived, + notifiableEventsResult = notifiableEventResult, pushClientSecret = FakePushClientSecret( getUserIdFromSecretResult = { A_USER_ID } ), @@ -152,11 +158,14 @@ class DefaultPushHandlerTest { pushHistoryService = pushHistoryService, ) defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + + advanceTimeBy(300.milliseconds) + incrementPushCounterResult.assertions() .isCalledOnce() notifiableEventResult.assertions() .isCalledOnce() - onNotifiableEventReceived.assertions() + onNotifiableEventsReceived.assertions() .isNeverCalled() onPushReceivedResult.assertions() .isCalledOnce() @@ -167,10 +176,10 @@ class DefaultPushHandlerTest { runTest { val aNotifiableMessageEvent = aNotifiableMessageEvent() val notifiableEventResult = - lambdaRecorder> { _, _, _ -> - Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)) + lambdaRecorder, Result>> { _, _ -> + Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent))) } - val onNotifiableEventReceived = lambdaRecorder {} + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} val aPushData = PushData( eventId = AN_EVENT_ID, @@ -183,8 +192,8 @@ class DefaultPushHandlerTest { onPushReceivedResult = onPushReceivedResult, ) val defaultPushHandler = createDefaultPushHandler( - onNotifiableEventReceived = onNotifiableEventReceived, - notifiableEventResult = notifiableEventResult, + onNotifiableEventsReceived = onNotifiableEventsReceived, + notifiableEventsResult = notifiableEventResult, pushClientSecret = FakePushClientSecret( getUserIdFromSecretResult = { null } ), @@ -195,14 +204,17 @@ class DefaultPushHandlerTest { pushHistoryService = pushHistoryService, ) defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + + advanceTimeBy(300.milliseconds) + incrementPushCounterResult.assertions() .isCalledOnce() notifiableEventResult.assertions() .isCalledOnce() - .with(value(A_USER_ID), value(A_ROOM_ID), value(AN_EVENT_ID)) - onNotifiableEventReceived.assertions() + .with(value(A_USER_ID), any()) + onNotifiableEventsReceived.assertions() .isCalledOnce() - .with(value(aNotifiableMessageEvent)) + .with(value(listOf(aNotifiableMessageEvent))) onPushReceivedResult.assertions() .isCalledOnce() } @@ -212,10 +224,10 @@ class DefaultPushHandlerTest { runTest { val aNotifiableMessageEvent = aNotifiableMessageEvent() val notifiableEventResult = - lambdaRecorder> { _, _, _ -> - Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)) + lambdaRecorder, Result>> { _, _ -> + Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent))) } - val onNotifiableEventReceived = lambdaRecorder {} + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} val aPushData = PushData( eventId = AN_EVENT_ID, @@ -228,8 +240,8 @@ class DefaultPushHandlerTest { onPushReceivedResult = onPushReceivedResult, ) val defaultPushHandler = createDefaultPushHandler( - onNotifiableEventReceived = onNotifiableEventReceived, - notifiableEventResult = notifiableEventResult, + onNotifiableEventsReceived = onNotifiableEventsReceived, + notifiableEventsResult = notifiableEventResult, pushClientSecret = FakePushClientSecret( getUserIdFromSecretResult = { null } ), @@ -244,7 +256,7 @@ class DefaultPushHandlerTest { .isCalledOnce() notifiableEventResult.assertions() .isNeverCalled() - onNotifiableEventReceived.assertions() + onNotifiableEventsReceived.assertions() .isNeverCalled() onPushReceivedResult.assertions() .isCalledOnce() @@ -254,10 +266,10 @@ class DefaultPushHandlerTest { fun `when classical PushData is received, but not able to resolve the event, nothing happen`() = runTest { val notifiableEventResult = - lambdaRecorder> { _, _, _ -> + lambdaRecorder, Result>> { _, _ -> Result.failure(ResolvingException("Unable to resolve")) } - val onNotifiableEventReceived = lambdaRecorder {} + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} val aPushData = PushData( eventId = AN_EVENT_ID, @@ -270,8 +282,8 @@ class DefaultPushHandlerTest { onPushReceivedResult = onPushReceivedResult, ) val defaultPushHandler = createDefaultPushHandler( - onNotifiableEventReceived = onNotifiableEventReceived, - notifiableEventResult = notifiableEventResult, + onNotifiableEventsReceived = onNotifiableEventsReceived, + notifiableEventsResult = notifiableEventResult, buildMeta = aBuildMeta( // Also test `lowPrivacyLoggingEnabled = false` here lowPrivacyLoggingEnabled = false @@ -283,12 +295,15 @@ class DefaultPushHandlerTest { pushHistoryService = pushHistoryService, ) defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + + advanceTimeBy(300.milliseconds) + incrementPushCounterResult.assertions() .isCalledOnce() notifiableEventResult.assertions() .isCalledOnce() - .with(value(A_USER_ID), value(A_ROOM_ID), value(AN_EVENT_ID)) - onNotifiableEventReceived.assertions() + .with(value(A_USER_ID), any()) + onNotifiableEventsReceived.assertions() .isNeverCalled() onPushReceivedResult.assertions() .isCalledOnce() @@ -315,28 +330,35 @@ class DefaultPushHandlerTest { Unit, > { _, _, _, _, _, _, _, _ -> } val elementCallEntryPoint = FakeElementCallEntryPoint(handleIncomingCallResult = handleIncomingCallLambda) - val onNotifiableEventReceived = lambdaRecorder {} + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val onPushReceivedResult = lambdaRecorder { _, _, _, _, _, _, _ -> } val pushHistoryService = FakePushHistoryService( onPushReceivedResult = onPushReceivedResult, ) val defaultPushHandler = createDefaultPushHandler( elementCallEntryPoint = elementCallEntryPoint, - notifiableEventResult = { _, _, _ -> + notifiableEventsResult = { _, _ -> Result.success( - ResolvedPushEvent.Event(aNotifiableCallEvent(callNotifyType = CallNotifyType.RING, timestamp = Instant.now().toEpochMilli())) + mapOf( + AN_EVENT_ID to ResolvedPushEvent.Event( + aNotifiableCallEvent(callNotifyType = CallNotifyType.RING, timestamp = Instant.now().toEpochMilli()) + ) + ) ) }, incrementPushCounterResult = {}, pushClientSecret = FakePushClientSecret( getUserIdFromSecretResult = { A_USER_ID } ), - onNotifiableEventReceived = onNotifiableEventReceived, + onNotifiableEventsReceived = onNotifiableEventsReceived, pushHistoryService = pushHistoryService, ) defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + + advanceTimeBy(300.milliseconds) + handleIncomingCallLambda.assertions().isCalledOnce() - onNotifiableEventReceived.assertions().isCalledOnce() + onNotifiableEventsReceived.assertions().isNeverCalled() onPushReceivedResult.assertions().isCalledOnce() } @@ -348,7 +370,7 @@ class DefaultPushHandlerTest { unread = 0, clientSecret = A_SECRET, ) - val onNotifiableEventReceived = lambdaRecorder {} + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val handleIncomingCallLambda = lambdaRecorder< CallType.RoomCall, EventId, @@ -367,9 +389,9 @@ class DefaultPushHandlerTest { ) val defaultPushHandler = createDefaultPushHandler( elementCallEntryPoint = elementCallEntryPoint, - onNotifiableEventReceived = onNotifiableEventReceived, - notifiableEventResult = { _, _, _ -> - Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent(type = EventType.CALL_NOTIFY))) + onNotifiableEventsReceived = onNotifiableEventsReceived, + notifiableEventsResult = { _, _ -> + Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent(type = EventType.CALL_NOTIFY)))) }, incrementPushCounterResult = {}, pushClientSecret = FakePushClientSecret( @@ -379,8 +401,10 @@ class DefaultPushHandlerTest { ) defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + advanceTimeBy(300.milliseconds) + handleIncomingCallLambda.assertions().isNeverCalled() - onNotifiableEventReceived.assertions().isCalledOnce() + onNotifiableEventsReceived.assertions().isCalledOnce() onPushReceivedResult.assertions().isCalledOnce() } @@ -392,7 +416,7 @@ class DefaultPushHandlerTest { unread = 0, clientSecret = A_SECRET, ) - val onNotifiableEventReceived = lambdaRecorder {} + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val handleIncomingCallLambda = lambdaRecorder< CallType.RoomCall, EventId, @@ -411,9 +435,9 @@ class DefaultPushHandlerTest { ) val defaultPushHandler = createDefaultPushHandler( elementCallEntryPoint = elementCallEntryPoint, - onNotifiableEventReceived = onNotifiableEventReceived, - notifiableEventResult = { _, _, _ -> - Result.success(ResolvedPushEvent.Event(aNotifiableCallEvent())) + onNotifiableEventsReceived = onNotifiableEventsReceived, + notifiableEventsResult = { _, _ -> + Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableCallEvent()))) }, incrementPushCounterResult = {}, userPushStore = FakeUserPushStore().apply { @@ -425,8 +449,11 @@ class DefaultPushHandlerTest { pushHistoryService = pushHistoryService, ) defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + + advanceTimeBy(300.milliseconds) + handleIncomingCallLambda.assertions().isCalledOnce() - onNotifiableEventReceived.assertions().isCalledOnce() + onNotifiableEventsReceived.assertions().isNeverCalled() onPushReceivedResult.assertions().isCalledOnce() } @@ -444,26 +471,29 @@ class DefaultPushHandlerTest { redactedEventId = AN_EVENT_ID_2, reason = null ) - val onRedactedEventReceived = lambdaRecorder { } + val onRedactedEventReceived = lambdaRecorder, Unit> { } val incrementPushCounterResult = lambdaRecorder {} val onPushReceivedResult = lambdaRecorder { _, _, _, _, _, _, _ -> } val pushHistoryService = FakePushHistoryService( onPushReceivedResult = onPushReceivedResult, ) val defaultPushHandler = createDefaultPushHandler( - onRedactedEventReceived = onRedactedEventReceived, + onRedactedEventsReceived = onRedactedEventReceived, incrementPushCounterResult = incrementPushCounterResult, - notifiableEventResult = { _, _, _ -> Result.success(aRedaction) }, + notifiableEventsResult = { _, _ -> Result.success(mapOf(AN_EVENT_ID to aRedaction)) }, pushClientSecret = FakePushClientSecret( getUserIdFromSecretResult = { A_USER_ID } ), pushHistoryService = pushHistoryService, ) defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + + advanceTimeBy(300.milliseconds) + incrementPushCounterResult.assertions() .isCalledOnce() onRedactedEventReceived.assertions().isCalledOnce() - .with(value(aRedaction)) + .with(value(listOf(aRedaction))) onPushReceivedResult.assertions() .isCalledOnce() } @@ -496,9 +526,9 @@ class DefaultPushHandlerTest { } private fun TestScope.createDefaultPushHandler( - onNotifiableEventReceived: (NotifiableEvent) -> Unit = { lambdaError() }, - onRedactedEventReceived: (ResolvedPushEvent.Redaction) -> Unit = { lambdaError() }, - notifiableEventResult: (SessionId, RoomId, EventId) -> Result = { _, _, _ -> lambdaError() }, + onNotifiableEventsReceived: (List) -> Unit = { lambdaError() }, + onRedactedEventsReceived: (List) -> Unit = { lambdaError() }, + notifiableEventsResult: (SessionId, List) -> Result> = { _, _, -> lambdaError() }, incrementPushCounterResult: () -> Unit = { lambdaError() }, userPushStore: UserPushStore = FakeUserPushStore(), pushClientSecret: PushClientSecret = FakePushClientSecret(), @@ -510,8 +540,8 @@ class DefaultPushHandlerTest { pushHistoryService: PushHistoryService = FakePushHistoryService(), ): DefaultPushHandler { return DefaultPushHandler( - onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventReceived), - onRedactedEventReceived = FakeOnRedactedEventReceived(onRedactedEventReceived), + onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventsReceived), + onRedactedEventReceived = FakeOnRedactedEventReceived(onRedactedEventsReceived), incrementPushDataStore = object : IncrementPushDataStore { override suspend fun incrementPushCounter() { incrementPushCounterResult() @@ -525,7 +555,7 @@ class DefaultPushHandlerTest { elementCallEntryPoint = elementCallEntryPoint, notificationChannels = notificationChannels, pushHistoryService = pushHistoryService, - resolverQueue = NotificationResolverQueue(notifiableEventResolver = FakeNotifiableEventResolver(notifiableEventResult), backgroundScope), + resolverQueue = NotificationResolverQueue(notifiableEventResolver = FakeNotifiableEventResolver(notifiableEventsResult), backgroundScope), appCoroutineScope = backgroundScope, ) } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnNotifiableEventReceived.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnNotifiableEventReceived.kt index 206eeb25444..055b1f322e5 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnNotifiableEventReceived.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnNotifiableEventReceived.kt @@ -11,15 +11,9 @@ import io.element.android.libraries.push.impl.notifications.model.NotifiableEven import io.element.android.tests.testutils.lambda.lambdaError class FakeOnNotifiableEventReceived( - private val onNotifiableEventReceivedResult: (NotifiableEvent) -> Unit = { lambdaError() }, + private val onNotifiableEventsReceivedResult: (List) -> Unit = { lambdaError() }, ) : OnNotifiableEventReceived { - override fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) { - onNotifiableEventReceivedResult(notifiableEvent) - } - override fun onNotifiableEventsReceived(notifiableEvents: List) { - for (event in notifiableEvents) { - onNotifiableEventReceived(event) - } + onNotifiableEventsReceivedResult(notifiableEvents) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnRedactedEventReceived.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnRedactedEventReceived.kt index 262aeee1242..b5a3731830e 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnRedactedEventReceived.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/FakeOnRedactedEventReceived.kt @@ -11,15 +11,9 @@ import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEv import io.element.android.tests.testutils.lambda.lambdaError class FakeOnRedactedEventReceived( - private val onRedactedEventReceivedResult: (ResolvedPushEvent.Redaction) -> Unit = { lambdaError() }, + private val onRedactedEventsReceivedResult: (List) -> Unit = { lambdaError() }, ) : OnRedactedEventReceived { - override fun onRedactedEventReceived(redaction: ResolvedPushEvent.Redaction) { - onRedactedEventReceivedResult(redaction) - } - override fun onRedactedEventsReceived(redactions: List) { - for (redaction in redactions) { - onRedactedEventReceived(redaction) - } + onRedactedEventsReceivedResult(redactions) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt index 8b3907be0b4..cb0c233503d 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEventTest.kt @@ -75,7 +75,7 @@ class SyncOnNotifiableEventTest { fun `when feature flag is disabled, nothing happens`() = runTest { val sut = createSyncOnNotifiableEvent(client = client, isSyncOnPushEnabled = false) - sut(notifiableEvent) + sut(listOf(notifiableEvent)) assert(startSyncLambda).isNeverCalled() assert(stopSyncLambda).isNeverCalled() @@ -96,7 +96,7 @@ class SyncOnNotifiableEventTest { unlocked.set(true) room.givenRoomInfo(aRoomInfo(hasRoomCall = true)) } - sut(incomingCallNotifiableEvent) + sut(listOf(incomingCallNotifiableEvent)) // The process was completed before the timeout assertThat(unlocked.get()).isTrue() @@ -116,30 +116,12 @@ class SyncOnNotifiableEventTest { unlocked.set(true) room.givenRoomInfo(aRoomInfo(hasRoomCall = true)) } - sut(incomingCallNotifiableEvent) + sut(listOf(incomingCallNotifiableEvent)) // Didn't unlock before the timeout assertThat(unlocked.get()).isFalse() } - @Test - fun `when feature flag is enabled and app is in foreground, sync is not started`() = runTest { - val appForegroundStateService = FakeAppForegroundStateService( - initialForegroundValue = true, - ) - val sut = createSyncOnNotifiableEvent(client = client, appForegroundStateService = appForegroundStateService, isSyncOnPushEnabled = true) - - appForegroundStateService.isSyncingNotificationEvent.test { - sut(notifiableEvent) - sut(incomingCallNotifiableEvent) - - // It's initially false - assertThat(awaitItem()).isFalse() - // It never becomes true - ensureAllEventsConsumed() - } - } - @Test fun `when feature flag is enabled and app is in background, sync is started and stopped`() = runTest { val appForegroundStateService = FakeAppForegroundStateService( @@ -153,7 +135,7 @@ class SyncOnNotifiableEventTest { appForegroundStateService.isSyncingNotificationEvent.test { syncService.emitSyncState(SyncState.Running) - sut(notifiableEvent) + sut(listOf(notifiableEvent)) // It's initially false assertThat(awaitItem()).isFalse() @@ -174,8 +156,8 @@ class SyncOnNotifiableEventTest { val sut = createSyncOnNotifiableEvent(client = client, appForegroundStateService = appForegroundStateService, isSyncOnPushEnabled = true) appForegroundStateService.isSyncingNotificationEvent.test { - launch { sut(notifiableEvent) } - launch { sut(notifiableEvent) } + launch { sut(listOf(notifiableEvent)) } + launch { sut(listOf(notifiableEvent)) } launch { delay(1) timelineItems.emit( From 6b517e169dafaff4e9d79112c91f51b724b9f947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 16 May 2025 08:46:22 +0200 Subject: [PATCH 07/12] Try improving return type for `NotifiableEventResolver.resolveEvents` --- .../api/notification/NotificationService.kt | 2 +- .../notification/RustNotificationService.kt | 2 +- .../notification/FakeNotificationService.kt | 6 +- .../DefaultNotifiableEventResolver.kt | 20 ++- .../NotificationResolverQueue.kt | 4 +- .../push/impl/push/DefaultPushHandler.kt | 8 +- .../DefaultNotifiableEventResolverTest.kt | 152 ++++++++++-------- .../FakeNotifiableEventResolver.kt | 6 +- .../push/impl/push/DefaultPushHandlerTest.kt | 43 +++-- 9 files changed, 147 insertions(+), 96 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt index 99e6693110a..5902ff33277 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt @@ -12,5 +12,5 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId interface NotificationService { - suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> + suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt index 1bd7285fc3c..1d642c21e2f 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt @@ -29,7 +29,7 @@ class RustNotificationService( override suspend fun getNotifications( sessionId: SessionId, ids: Map> - ): Result> = withContext(dispatchers.io) { + ): Result> = withContext(dispatchers.io) { runCatching { val requests = ids.map { (roomId, eventIds) -> NotificationItemsRequest( diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt index 3940fb3c490..5ddf6d16edd 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt @@ -14,13 +14,13 @@ import io.element.android.libraries.matrix.api.notification.NotificationData import io.element.android.libraries.matrix.api.notification.NotificationService class FakeNotificationService : NotificationService { - private var getNotificationsResult: Result> = Result.success(emptyMap()) + private var getNotificationsResult: Result> = Result.success(emptyMap()) - fun givenGetNotificationsResult(result: Result>) { + fun givenGetNotificationsResult(result: Result>) { getNotificationsResult = result } - override suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> { + override suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> { return getNotificationsResult } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt index 2430d4bfed1..347fe9ed7d2 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt @@ -64,7 +64,7 @@ interface NotifiableEventResolver { suspend fun resolveEvents( sessionId: SessionId, notificationEventRequests: List - ): Result> + ): Result>> } @ContributesBinding(AppScope::class) @@ -82,20 +82,30 @@ class DefaultNotifiableEventResolver @Inject constructor( override suspend fun resolveEvents( sessionId: SessionId, notificationEventRequests: List - ): Result> { + ): Result>> { 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 } } - val notifications = client.notificationService().getNotifications(sessionId, ids) // TODO this notificationData is not always valid at the moment, sometimes the Rust SDK can't fetch the matching event - return notifications.mapCatching { map -> + val notifications = client.notificationService().getNotifications(sessionId, ids).mapCatching { map -> map.mapValues { (_, notificationData) -> - notificationData?.asNotifiableEvent(client, sessionId)?.getOrNull() + notificationData.asNotifiableEvent(client, sessionId) } } + + return Result.success( + notificationEventRequests.associate { + val notificationData = notifications.getOrNull()?.get(it.eventId) + if (notificationData != null) { + it to notificationData + } else { + it to Result.failure(ResolvingException("No notification data for ${it.roomId} - ${it.eventId}")) + } + } + ) } private suspend fun NotificationData.asNotifiableEvent( diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt index 2caf43a98c1..a1e356be143 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt @@ -36,7 +36,7 @@ class NotificationResolverQueue @Inject constructor( } private val requestQueue = Channel(capacity = 100) - val results: SharedFlow, List>> = MutableSharedFlow() + val results: SharedFlow, Map>>> = MutableSharedFlow() init { coroutineScope.launch { @@ -58,7 +58,7 @@ class NotificationResolverQueue @Inject constructor( launch { // No need for a Mutex since the SDK already has one internally val notifications = notifiableEventResolver.resolveEvents(sessionId, requests).getOrNull().orEmpty() - (results as MutableSharedFlow).emit(requests to notifications.values.filterNotNull()) + (results as MutableSharedFlow).emit(requests to notifications) } } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt index d06a37cdc18..48fdd6022d6 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt @@ -64,7 +64,8 @@ class DefaultPushHandler @Inject constructor( resolverQueue.results .map { (requests, resolvedEvents) -> for (request in requests) { - if (resolvedEvents.any { it.sessionId == request.sessionId && it.roomId == it.roomId && it.eventId == request.eventId }) { + val result = resolvedEvents[request] + if (result?.isSuccess == true) { pushHistoryService.onSuccess( providerInfo = request.providerInfo, eventId = request.eventId, @@ -85,7 +86,10 @@ class DefaultPushHandler @Inject constructor( val events = mutableListOf() val redactions = mutableListOf() - for (event in resolvedEvents) { + + @Suppress("LoopWithTooManyJumpStatements") + for (result in resolvedEvents.values) { + val event = result.getOrNull() ?: continue val userPushStore = userPushStoreFactory.getOrCreate(event.sessionId) val areNotificationsEnabled = userPushStore.getNotificationEnabledForDevice().first() // If notifications are disabled for this session and device, we don't want to show the notification diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt index 986b6a081aa..2ba00156c7c 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt @@ -77,17 +77,9 @@ class DefaultNotifiableEventResolverTest { val sut = createDefaultNotifiableEventResolver( notificationResult = Result.failure(AN_EXCEPTION) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.isFailure).isTrue() - } - - @Test - fun `resolve event null`() = runTest { - val sut = createDefaultNotifiableEventResolver( - notificationResult = Result.success(emptyMap()) - ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()?.isEmpty()).isTrue() + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) + assertThat(result.getEvent(request)?.isFailure).isTrue() } @Test @@ -104,11 +96,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -127,11 +120,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world", hasMentionOrReply = true) ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -154,11 +148,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -181,11 +176,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Hello world") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -202,11 +198,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Audio") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -223,11 +220,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Video") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -244,11 +242,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Voice message") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -265,11 +264,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Image") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -286,11 +286,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Sticker") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -307,11 +308,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "File") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -328,11 +330,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Location") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -349,11 +352,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Notice") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -370,11 +374,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "* Bob is happy") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -391,11 +396,12 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( aNotifiableMessageEvent(body = "Poll: A question") ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -413,8 +419,9 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isNull() + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) + assertThat(result.getEvent(request)?.getOrNull()).isNull() } @Test @@ -431,7 +438,8 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( InviteNotifiableEvent( sessionId = A_SESSION_ID, @@ -450,7 +458,7 @@ class DefaultNotifiableEventResolverTest { isUpdated = false, ) ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -467,7 +475,8 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( InviteNotifiableEvent( sessionId = A_SESSION_ID, @@ -486,7 +495,7 @@ class DefaultNotifiableEventResolverTest { isUpdated = false, ) ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -504,7 +513,8 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( InviteNotifiableEvent( sessionId = A_SESSION_ID, @@ -523,7 +533,7 @@ class DefaultNotifiableEventResolverTest { isUpdated = false, ) ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -541,7 +551,8 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( InviteNotifiableEvent( sessionId = A_SESSION_ID, @@ -560,7 +571,7 @@ class DefaultNotifiableEventResolverTest { isUpdated = false, ) ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -577,8 +588,9 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isNull() + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) + assertThat(result.getEvent(request)?.getOrNull()).isNull() } @Test @@ -588,7 +600,8 @@ class DefaultNotifiableEventResolverTest { mapOf(AN_EVENT_ID to aNotificationData(content = NotificationContent.MessageLike.RoomEncrypted)) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( FallbackNotifiableEvent( sessionId = A_SESSION_ID, @@ -602,7 +615,7 @@ class DefaultNotifiableEventResolverTest { timestamp = A_FAKE_TIMESTAMP, ) ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -616,7 +629,8 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) val expectedResult = ResolvedPushEvent.Event( NotifiableMessageEvent( sessionId = A_SESSION_ID, @@ -642,7 +656,7 @@ class DefaultNotifiableEventResolverTest { isUpdated = false ) ) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -682,8 +696,9 @@ class DefaultNotifiableEventResolverTest { ) ) callNotificationEventResolver.resolveEventLambda = { _, _, _ -> Result.success(expectedResult.notifiableEvent) } - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -706,8 +721,9 @@ class DefaultNotifiableEventResolverTest { redactedEventId = AN_EVENT_ID_2, reason = A_REDACTION_REASON, ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isEqualTo(expectedResult) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) + assertThat(result.getEvent(request)).isEqualTo(Result.success(expectedResult)) } @Test @@ -724,8 +740,9 @@ class DefaultNotifiableEventResolverTest { ) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isNull() + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) + assertThat(result.getEvent(request)?.getOrNull()).isNull() } @Test @@ -770,13 +787,20 @@ class DefaultNotifiableEventResolverTest { mapOf(AN_EVENT_ID to aNotificationData(content = content)) ) ) - val result = sut.resolveEvents(A_SESSION_ID, listOf(NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase"))) - assertThat(result.getOrNull()?.get(AN_EVENT_ID)).isNull() + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, "firebase") + val result = sut.resolveEvents(A_SESSION_ID, listOf(request)) + assertThat(result.getEvent(request)?.getOrNull()).isNull() + } + + private fun Result>>.getEvent( + request: NotificationEventRequest + ): Result? { + return getOrNull()?.get(request) } private fun createDefaultNotifiableEventResolver( notificationService: FakeNotificationService? = FakeNotificationService(), - notificationResult: Result> = Result.success(emptyMap()), + notificationResult: Result> = Result.success(emptyMap()), appPreferencesStore: AppPreferencesStore = InMemoryAppPreferencesStore(), callNotificationEventResolver: FakeCallNotificationEventResolver = FakeCallNotificationEventResolver(), ): DefaultNotifiableEventResolver { diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt index ff56a79cf4d..d38bc098a01 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/FakeNotifiableEventResolver.kt @@ -7,18 +7,18 @@ package io.element.android.libraries.push.impl.notifications -import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEvent import io.element.android.tests.testutils.lambda.lambdaError class FakeNotifiableEventResolver( - private val resolveEventsResult: (SessionId, List) -> Result> = { _, _ -> lambdaError() } + private val resolveEventsResult: (SessionId, List) -> Result>> = + { _, _ -> lambdaError() } ) : NotifiableEventResolver { override suspend fun resolveEvents( sessionId: SessionId, notificationEventRequests: List - ): Result> { + ): Result>> { return resolveEventsResult(sessionId, notificationEventRequests) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt index 60d058fce53..3dd26f0b0c3 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt @@ -85,8 +85,9 @@ class DefaultPushHandlerTest { fun `when classical PushData is received, the notification drawer is informed`() = runTest { val aNotifiableMessageEvent = aNotifiableMessageEvent() val notifiableEventResult = - lambdaRecorder, Result>> { _, _, -> - Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent))) + lambdaRecorder, Result>>> { _, _, -> + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) + Result.success(mapOf(request to Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)))) } val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} @@ -130,8 +131,9 @@ class DefaultPushHandlerTest { runTest { val aNotifiableMessageEvent = aNotifiableMessageEvent() val notifiableEventResult = - lambdaRecorder, Result>> { _, _ -> - Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent))) + lambdaRecorder, Result>>> { _, _ -> + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) + Result.success(mapOf(request to Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)))) } val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} @@ -176,8 +178,9 @@ class DefaultPushHandlerTest { runTest { val aNotifiableMessageEvent = aNotifiableMessageEvent() val notifiableEventResult = - lambdaRecorder, Result>> { _, _ -> - Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent))) + lambdaRecorder, Result>>> { _, _ -> + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) + Result.success(mapOf(request to Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)))) } val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} @@ -224,8 +227,9 @@ class DefaultPushHandlerTest { runTest { val aNotifiableMessageEvent = aNotifiableMessageEvent() val notifiableEventResult = - lambdaRecorder, Result>> { _, _ -> - Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent))) + lambdaRecorder, Result>>> { _, _ -> + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) + Result.success(mapOf(request to Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)))) } val onNotifiableEventsReceived = lambdaRecorder, Unit> {} val incrementPushCounterResult = lambdaRecorder {} @@ -266,7 +270,7 @@ class DefaultPushHandlerTest { fun `when classical PushData is received, but not able to resolve the event, nothing happen`() = runTest { val notifiableEventResult = - lambdaRecorder, Result>> { _, _ -> + lambdaRecorder, Result>>> { _, _ -> Result.failure(ResolvingException("Unable to resolve")) } val onNotifiableEventsReceived = lambdaRecorder, Unit> {} @@ -338,10 +342,13 @@ class DefaultPushHandlerTest { val defaultPushHandler = createDefaultPushHandler( elementCallEntryPoint = elementCallEntryPoint, notifiableEventsResult = { _, _ -> + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) Result.success( mapOf( - AN_EVENT_ID to ResolvedPushEvent.Event( - aNotifiableCallEvent(callNotifyType = CallNotifyType.RING, timestamp = Instant.now().toEpochMilli()) + request to Result.success( + ResolvedPushEvent.Event( + aNotifiableCallEvent(callNotifyType = CallNotifyType.RING, timestamp = Instant.now().toEpochMilli()) + ) ) ) ) @@ -391,7 +398,8 @@ class DefaultPushHandlerTest { elementCallEntryPoint = elementCallEntryPoint, onNotifiableEventsReceived = onNotifiableEventsReceived, notifiableEventsResult = { _, _ -> - Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableMessageEvent(type = EventType.CALL_NOTIFY)))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) + Result.success(mapOf(request to Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent(type = EventType.CALL_NOTIFY))))) }, incrementPushCounterResult = {}, pushClientSecret = FakePushClientSecret( @@ -437,7 +445,8 @@ class DefaultPushHandlerTest { elementCallEntryPoint = elementCallEntryPoint, onNotifiableEventsReceived = onNotifiableEventsReceived, notifiableEventsResult = { _, _ -> - Result.success(mapOf(AN_EVENT_ID to ResolvedPushEvent.Event(aNotifiableCallEvent()))) + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) + Result.success(mapOf(request to Result.success(ResolvedPushEvent.Event(aNotifiableCallEvent())))) }, incrementPushCounterResult = {}, userPushStore = FakeUserPushStore().apply { @@ -480,7 +489,10 @@ class DefaultPushHandlerTest { val defaultPushHandler = createDefaultPushHandler( onRedactedEventsReceived = onRedactedEventReceived, incrementPushCounterResult = incrementPushCounterResult, - notifiableEventsResult = { _, _ -> Result.success(mapOf(AN_EVENT_ID to aRedaction)) }, + notifiableEventsResult = { _, _ -> + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) + Result.success(mapOf(request to Result.success(aRedaction))) + }, pushClientSecret = FakePushClientSecret( getUserIdFromSecretResult = { A_USER_ID } ), @@ -528,7 +540,8 @@ class DefaultPushHandlerTest { private fun TestScope.createDefaultPushHandler( onNotifiableEventsReceived: (List) -> Unit = { lambdaError() }, onRedactedEventsReceived: (List) -> Unit = { lambdaError() }, - notifiableEventsResult: (SessionId, List) -> Result> = { _, _, -> lambdaError() }, + notifiableEventsResult: (SessionId, List) -> Result>> = + { _, _, -> lambdaError() }, incrementPushCounterResult: () -> Unit = { lambdaError() }, userPushStore: UserPushStore = FakeUserPushStore(), pushClientSecret: PushClientSecret = FakePushClientSecret(), From e5735c40ad280d400a790e0b8f147373fd6bfb43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 16 May 2025 08:55:29 +0200 Subject: [PATCH 08/12] Add some docs --- .../DefaultNotifiableEventResolver.kt | 1 + .../notifications/NotificationResolverQueue.kt | 14 ++++++++++++++ .../libraries/push/impl/push/DefaultPushHandler.kt | 12 ++++++++++++ 3 files changed, 27 insertions(+) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt index 347fe9ed7d2..35dda2a80cc 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt @@ -102,6 +102,7 @@ class DefaultNotifiableEventResolver @Inject constructor( 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}")) } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt index a1e356be143..9862ff7c64b 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt @@ -25,6 +25,10 @@ import timber.log.Timber import javax.inject.Inject import kotlin.time.Duration.Companion.milliseconds +/** + * This class is responsible for periodically batching notification requests and resolving them in a single call, + * so that we can avoid having to resolve each notification individually in the SDK. + */ @OptIn(ExperimentalCoroutinesApi::class) @SingleIn(AppScope::class) class NotificationResolverQueue @Inject constructor( @@ -36,6 +40,10 @@ class NotificationResolverQueue @Inject constructor( } private val requestQueue = Channel(capacity = 100) + /** + * A flow that emits pairs of a list of notification event requests and a map of the resolved events. + * The map contains the original request as the key and the resolved event as the value. + */ val results: SharedFlow, Map>>> = MutableSharedFlow() init { @@ -65,6 +73,12 @@ class NotificationResolverQueue @Inject constructor( } } + /** + * Enqueues a notification event request to be resolved. + * The request will be processed in batches, so it may not be resolved immediately. + * + * @param request The notification event request to enqueue. + */ suspend fun enqueue(request: NotificationEventRequest) { requestQueue.send(request) } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt index 48fdd6022d6..19b060d36b9 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandler.kt @@ -61,9 +61,17 @@ class DefaultPushHandler @Inject constructor( private val appCoroutineScope: CoroutineScope, ) : PushHandler { init { + processPushEventResults() + } + + /** + * Process the push notification event results emitted by the [resolverQueue]. + */ + private fun processPushEventResults() { resolverQueue.results .map { (requests, resolvedEvents) -> for (request in requests) { + // Log the result of the push notification event val result = resolvedEvents[request] if (result?.isSuccess == true) { pushHistoryService.onSuccess( @@ -97,6 +105,7 @@ class DefaultPushHandler @Inject constructor( val isRingingCall = (event as? ResolvedPushEvent.Event)?.notifiableEvent is NotifiableRingingCallEvent if (!areNotificationsEnabled && !isRingingCall) continue + // We categorise each result into either a NotifiableEvent or a Redaction when (event) { is ResolvedPushEvent.Event -> { events.add(event.notifiableEvent) @@ -107,16 +116,19 @@ class DefaultPushHandler @Inject constructor( } } + // Process redactions of messages if (redactions.isNotEmpty()) { onRedactedEventReceived.onRedactedEventsReceived(redactions) } + // Find and process ringing call notifications separately val (ringingCallEvents, nonRingingCallEvents) = events.partition { it is NotifiableRingingCallEvent } for (ringingCallEvent in ringingCallEvents) { Timber.tag(loggerTag.value).d("Ringing call event: $ringingCallEvent") handleRingingCallEvent(ringingCallEvent as NotifiableRingingCallEvent) } + // Finally, process other notifications (messages, invites, generic notifications, etc.) if (nonRingingCallEvents.isNotEmpty()) { onNotifiableEventReceived.onNotifiableEventsReceived(nonRingingCallEvents) } From ad25233aa58eaee192305a077e85b81ace40d706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 16 May 2025 09:37:52 +0200 Subject: [PATCH 09/12] Add test for `DefaultPushHandler` when receiving several notifications inside the same time window --- .../push/impl/push/DefaultPushHandlerTest.kt | 54 +++++++++++++++++++ .../testutils/lambda/ParameterMatcher.kt | 12 +++++ 2 files changed, 66 insertions(+) diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt index 3dd26f0b0c3..933c830525b 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/push/DefaultPushHandlerTest.kt @@ -50,6 +50,7 @@ import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.Fa import io.element.android.tests.testutils.lambda.any import io.element.android.tests.testutils.lambda.lambdaError import io.element.android.tests.testutils.lambda.lambdaRecorder +import io.element.android.tests.testutils.lambda.matching import io.element.android.tests.testutils.lambda.value import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestScope @@ -537,6 +538,59 @@ class DefaultPushHandlerTest { .isCalledOnce() } + @Test + fun `when receiving several push notifications at the same time, those are batched before being processed`() = runTest { + val aNotifiableMessageEvent = aNotifiableMessageEvent() + val notifiableEventResult = + lambdaRecorder, Result>>> { _, _, -> + val request = NotificationEventRequest(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID, A_PUSHER_INFO) + Result.success(mapOf(request to Result.success(ResolvedPushEvent.Event(aNotifiableMessageEvent)))) + } + val onNotifiableEventsReceived = lambdaRecorder, Unit> {} + val incrementPushCounterResult = lambdaRecorder {} + val onPushReceivedResult = lambdaRecorder { _, _, _, _, _, _, _ -> } + val pushHistoryService = FakePushHistoryService( + onPushReceivedResult = onPushReceivedResult, + ) + val aPushData = PushData( + eventId = AN_EVENT_ID, + roomId = A_ROOM_ID, + unread = 0, + clientSecret = A_SECRET, + ) + val anotherPushData = PushData( + eventId = AN_EVENT_ID_2, + roomId = A_ROOM_ID, + unread = 0, + clientSecret = A_SECRET, + ) + val defaultPushHandler = createDefaultPushHandler( + onNotifiableEventsReceived = onNotifiableEventsReceived, + notifiableEventsResult = notifiableEventResult, + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_USER_ID } + ), + incrementPushCounterResult = incrementPushCounterResult, + pushHistoryService = pushHistoryService, + ) + defaultPushHandler.handle(aPushData, A_PUSHER_INFO) + defaultPushHandler.handle(anotherPushData, A_PUSHER_INFO) + + advanceTimeBy(300.milliseconds) + + incrementPushCounterResult.assertions() + .isCalledExactly(2) + notifiableEventResult.assertions() + .isCalledOnce() + .with(value(A_USER_ID), matching> { requests -> + requests.size == 2 && requests.first().eventId == AN_EVENT_ID && requests.last().eventId == AN_EVENT_ID_2 + }) + onNotifiableEventsReceived.assertions() + .isCalledOnce() + onPushReceivedResult.assertions() + .isCalledExactly(2) + } + private fun TestScope.createDefaultPushHandler( onNotifiableEventsReceived: (List) -> Unit = { lambdaError() }, onRedactedEventsReceived: (List) -> Unit = { lambdaError() }, diff --git a/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/lambda/ParameterMatcher.kt b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/lambda/ParameterMatcher.kt index 8a692f67db8..2b16b099bf0 100644 --- a/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/lambda/ParameterMatcher.kt +++ b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/lambda/ParameterMatcher.kt @@ -24,6 +24,18 @@ fun value(expectedValue: T) = object : ParameterMatcher { override fun toString(): String = "value($expectedValue)" } +/** + * A matcher that matches a value based on a condition. + * Can be used to assert that a lambda has been called with a value that satisfies a specific condition. + */ +fun matching(check: (T) -> Boolean) = object : ParameterMatcher { + override fun match(param: Any?): Boolean { + @Suppress("UNCHECKED_CAST") + return (param as? T)?.let { check(it) } ?: false + } + override fun toString(): String = "matching(condition)" +} + /** * A matcher that matches any value. * Can be used when we don't care about the value of a parameter. From d9bb5b74172126359570a769ea7745062ecc4966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 16 May 2025 17:55:29 +0200 Subject: [PATCH 10/12] Revert leftovers of experiment on notification priority --- .../impl/notifications/factories/NotificationCreator.kt | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt index 1e4fb9afae6..d462ebb5f6e 100755 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt @@ -14,7 +14,6 @@ import android.graphics.Canvas import androidx.annotation.DrawableRes import androidx.core.app.NotificationCompat import androidx.core.app.NotificationCompat.MessagingStyle -import androidx.core.app.NotificationManagerCompat import androidx.core.app.Person import androidx.core.content.res.ResourcesCompat import coil3.ImageLoader @@ -42,7 +41,6 @@ import io.element.android.libraries.push.impl.notifications.model.InviteNotifiab import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent import io.element.android.libraries.push.impl.notifications.model.SimpleNotifiableEvent import io.element.android.services.toolbox.api.strings.StringProvider -import timber.log.Timber import javax.inject.Inject interface NotificationCreator { @@ -180,11 +178,7 @@ class DefaultNotificationCreator @Inject constructor( // 'importance' which is set in the NotificationChannel. The integers representing // 'priority' are different from 'importance', so make sure you don't mix them. .apply { - val notificationManager = NotificationManagerCompat.from(context) - val previousNotification = notificationManager.activeNotifications.find { it.tag == roomInfo.roomId.value } - val elapsed = System.currentTimeMillis() - (previousNotification?.postTime ?: 0L) - Timber.d("Creating noisy notification for room ${roomInfo.roomId.value} with elapsed time $elapsed") - if (roomInfo.shouldBing && elapsed > 1000L) { + if (roomInfo.shouldBing) { // Compat priority = NotificationCompat.PRIORITY_DEFAULT /* @@ -194,7 +188,6 @@ class DefaultNotificationCreator @Inject constructor( */ setLights(accentColor, 500, 500) } else { - Timber.d("Creating low priority notification") priority = NotificationCompat.PRIORITY_LOW } // Clear existing actions since we might be updating an existing notification From b5ece84b52dc9730af6f5a1041e9717761766c39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Mon, 19 May 2025 12:29:03 +0200 Subject: [PATCH 11/12] Move `sessionId` parameter from `RustNotificationService.getNotifications` to its constructor --- .../libraries/matrix/api/notification/NotificationService.kt | 3 +-- .../element/android/libraries/matrix/impl/RustMatrixClient.kt | 2 +- .../matrix/impl/notification/RustNotificationService.kt | 2 +- .../matrix/impl/notification/RustNotificationServiceTest.kt | 3 ++- .../matrix/test/notification/FakeNotificationService.kt | 3 +-- .../push/impl/notifications/DefaultNotifiableEventResolver.kt | 2 +- .../notifications/DefaultOnMissedCallNotificationHandler.kt | 2 +- 7 files changed, 8 insertions(+), 9 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt index 5902ff33277..1e1c8b7fb77 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt @@ -9,8 +9,7 @@ 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 getNotifications(sessionId: SessionId, ids: Map>): Result> + suspend fun getNotifications(ids: Map>): Result> } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index 9dba6fc5894..5ef878df5ac 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt @@ -152,7 +152,7 @@ class RustMatrixClient( ) private val notificationProcessSetup = NotificationProcessSetup.SingleProcess(innerSyncService) private val innerNotificationClient = runBlocking { innerClient.notificationClient(notificationProcessSetup) } - private val notificationService = RustNotificationService(innerNotificationClient, dispatchers, clock) + private val notificationService = RustNotificationService(sessionId, innerNotificationClient, dispatchers, clock) private val notificationSettingsService = RustNotificationSettingsService(innerClient, sessionCoroutineScope, dispatchers) private val encryptionService = RustEncryptionService( client = innerClient, diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt index 1d642c21e2f..7cf432bad41 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt @@ -20,6 +20,7 @@ import org.matrix.rustcomponents.sdk.NotificationItemsRequest import timber.log.Timber class RustNotificationService( + private val sessionId: SessionId, private val notificationClient: NotificationClient, private val dispatchers: CoroutineDispatchers, clock: SystemClock, @@ -27,7 +28,6 @@ class RustNotificationService( private val notificationMapper: NotificationMapper = NotificationMapper(clock) override suspend fun getNotifications( - sessionId: SessionId, ids: Map> ): Result> = withContext(dispatchers.io) { runCatching { diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt index 88d0234af55..c8d40d7332a 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationServiceTest.kt @@ -34,7 +34,7 @@ class RustNotificationServiceTest { val sut = createRustNotificationService( notificationClient = notificationClient, ) - val result = sut.getNotifications(A_SESSION_ID, mapOf(A_ROOM_ID to listOf(AN_EVENT_ID))).getOrThrow()[AN_EVENT_ID]!! + val result = sut.getNotifications(mapOf(A_ROOM_ID to listOf(AN_EVENT_ID))).getOrThrow()[AN_EVENT_ID]!! assertThat(result.isEncrypted).isTrue() assertThat(result.content).isEqualTo( NotificationContent.MessageLike.RoomMessage( @@ -52,6 +52,7 @@ class RustNotificationServiceTest { clock: SystemClock = FakeSystemClock(), ) = RustNotificationService( + sessionId = A_SESSION_ID, notificationClient = notificationClient, dispatchers = testCoroutineDispatchers(), clock = clock, diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt index 5ddf6d16edd..4a9671f6773 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt @@ -9,7 +9,6 @@ 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 @@ -20,7 +19,7 @@ class FakeNotificationService : NotificationService { getNotificationsResult = result } - override suspend fun getNotifications(sessionId: SessionId, ids: Map>): Result> { + override suspend fun getNotifications(ids: Map>): Result> { return getNotificationsResult } } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt index 35dda2a80cc..6ee03ed4185 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt @@ -90,7 +90,7 @@ class DefaultNotifiableEventResolver @Inject constructor( 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 - val notifications = client.notificationService().getNotifications(sessionId, ids).mapCatching { map -> + val notifications = client.notificationService().getNotifications(ids).mapCatching { map -> map.mapValues { (_, notificationData) -> notificationData.asNotifiableEvent(client, sessionId) } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt index ca9fd29533d..1efe57ce065 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultOnMissedCallNotificationHandler.kt @@ -30,7 +30,7 @@ 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() - ?.getNotifications(sessionId, mapOf(roomId to listOf(eventId))) + ?.getNotifications(mapOf(roomId to listOf(eventId))) ?.getOrNull() ?.get(eventId) ?: return From bd27b50e75a8889ccde22fcbfa8e6cda400b2b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Mon, 19 May 2025 12:47:38 +0200 Subject: [PATCH 12/12] Try using a debounce strategy instead of a time window one --- .../NotificationResolverQueue.kt | 67 +++++++++++-------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt index 9862ff7c64b..fccd4df0274 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationResolverQueue.kt @@ -15,6 +15,7 @@ import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.push.impl.notifications.model.ResolvedPushEvent import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableSharedFlow @@ -40,39 +41,14 @@ class NotificationResolverQueue @Inject constructor( } private val requestQueue = Channel(capacity = 100) + private var currentProcessingJob: Job? = null + /** * A flow that emits pairs of a list of notification event requests and a map of the resolved events. * The map contains the original request as the key and the resolved event as the value. */ val results: SharedFlow, Map>>> = MutableSharedFlow() - init { - coroutineScope.launch { - while (coroutineScope.isActive) { - // Wait for a batch of requests to be received in a specified time window - delay(BATCH_WINDOW_MS.milliseconds) - - val groupedRequestsById = buildList { - while (!requestQueue.isEmpty) { - requestQueue.receiveCatching().getOrNull()?.let(this::add) - } - }.groupBy { it.sessionId } - - val sessionIds = groupedRequestsById.keys - for (sessionId in sessionIds) { - val requests = groupedRequestsById[sessionId].orEmpty() - Timber.d("Fetching notifications for $sessionId: $requests. Pending requests: ${!requestQueue.isEmpty}") - - launch { - // No need for a Mutex since the SDK already has one internally - val notifications = notifiableEventResolver.resolveEvents(sessionId, requests).getOrNull().orEmpty() - (results as MutableSharedFlow).emit(requests to notifications) - } - } - } - } - } - /** * Enqueues a notification event request to be resolved. * The request will be processed in batches, so it may not be resolved immediately. @@ -80,7 +56,44 @@ class NotificationResolverQueue @Inject constructor( * @param request The notification event request to enqueue. */ suspend fun enqueue(request: NotificationEventRequest) { + // Cancel previous processing job if it exists, acting as a debounce operation + Timber.d("Cancelling job: $currentProcessingJob") + currentProcessingJob?.cancel() + + // Enqueue the request and start a delayed processing job requestQueue.send(request) + currentProcessingJob = processQueue() + Timber.d("Starting processing job for request: $request") + } + + private fun processQueue() = coroutineScope.launch { + if (!isActive) return@launch + + delay(BATCH_WINDOW_MS.milliseconds) + + if (!isActive) return@launch + + // If this job is still active (so this is the latest job), we launch a separate one that won't be cancelled when enqueueing new items + // to process the existing queued items. + coroutineScope.launch { + val groupedRequestsById = buildList { + while (!requestQueue.isEmpty) { + requestQueue.receiveCatching().getOrNull()?.let(this::add) + } + }.groupBy { it.sessionId } + + val sessionIds = groupedRequestsById.keys + for (sessionId in sessionIds) { + val requests = groupedRequestsById[sessionId].orEmpty() + Timber.d("Fetching notifications for $sessionId: $requests. Pending requests: ${!requestQueue.isEmpty}") + // Resolving the events in parallel should improve performance since each session id will query a different Client + launch { + // No need for a Mutex since the SDK already has one internally + val notifications = notifiableEventResolver.resolveEvents(sessionId, requests).getOrNull().orEmpty() + (results as MutableSharedFlow).emit(requests to notifications) + } + } + } } }