From 8c6a7ccac5a5b4c69162169c581ffb0e7c16bbf6 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 11:41:11 +0100 Subject: [PATCH 01/20] Use the method `setLargeIcon(Bitmap?)` instead of `setLargeIcon(Icon?)` because it may scale the Bitmap on versions before API 27. Starting in API 27, the framework does this automatically. --- .../push/impl/notifications/factories/NotificationCreator.kt | 5 +---- 1 file changed, 1 insertion(+), 4 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 a69ce5e673..8324bd8bba 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 @@ -11,7 +11,6 @@ package io.element.android.libraries.push.impl.notifications.factories import android.app.Notification import android.content.Context import android.graphics.Bitmap -import android.graphics.drawable.Icon import androidx.annotation.ColorInt import androidx.core.app.NotificationCompat import androidx.core.app.NotificationCompat.MessagingStyle @@ -215,9 +214,7 @@ class DefaultNotificationCreator( if (openIntent != null) { setContentIntent(openIntent) } - if (largeIcon != null) { - setLargeIcon(Icon.createWithBitmap(largeIcon)) - } + setLargeIcon(largeIcon) setDeleteIntent(pendingIntentFactory.createDismissRoomPendingIntent(roomInfo.sessionId, roomInfo.roomId)) // If any of the events are of rtc notification type it means a missed call, set the category to the right value From 3a3ab4e69662cd4fb476e1ef962b4d1886206780 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 11:59:58 +0100 Subject: [PATCH 02/20] Cleanup NotificationCreator. --- .../factories/NotificationCreator.kt | 89 +++++++------------ 1 file changed, 31 insertions(+), 58 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 8324bd8bba..70d01cbf24 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 @@ -142,14 +142,21 @@ class DefaultNotificationCreator( } else { notificationChannels.getChannelIdForMessage(noisy = roomInfo.shouldBing) } + // A category allows groups of notifications to be ranked and filtered – per user or system settings. + // For example, alarm notifications should display before promo notifications, or message from known contact + // that can be displayed in not disturb mode if white listed (the later will need compat28.x) + // If any of the events are of rtc notification type it means a missed call, set the category to the right value + val category = if (containsMissedCall) { + NotificationCompat.CATEGORY_MISSED_CALL + } else { + NotificationCompat.CATEGORY_MESSAGE + } val builder = if (existingNotification != null) { NotificationCompat.Builder(context, existingNotification) + // Clear existing actions + .clearActions() } else { NotificationCompat.Builder(context, channelId) - // A category allows groups of notifications to be ranked and filtered – per user or system settings. - // For example, alarm notifications should display before promo notifications, or message from known contact - // that can be displayed in not disturb mode if white listed (the later will need compat28.x) - .setCategory(NotificationCompat.CATEGORY_MESSAGE) // ID of the corresponding shortcut, for conversation features under API 30+ // Must match those created in the ShortcutInfoCompat.Builder() // for the notification to appear as a "Conversation": @@ -165,7 +172,6 @@ class DefaultNotificationCreator( // Remove notification after opening it or using an action .setAutoCancel(true) } - val messagingStyle = existingNotification?.let { MessagingStyle.extractMessagingStyleFromNotification(it) } ?: createMessagingStyleFromCurrentUser( @@ -175,52 +181,35 @@ class DefaultNotificationCreator( isThread = threadId != null, roomIsGroup = !roomInfo.isDm, ) - messagingStyle.addMessagesFromEvents(events, imageLoader) - return builder + .setCategory(category) .setNumber(events.size) .setOnlyAlertOnce(roomInfo.isUpdated) .setWhen(lastMessageTimestamp) // MESSAGING_STYLE sets title and content for API 16 and above devices. .setStyle(messagingStyle) .configureWith(notificationAccountParams) - // Sets priority for 25 and below. For 26 and above, 'priority' is deprecated for - // 'importance' which is set in the NotificationChannel. The integers representing - // 'priority' are different from 'importance', so make sure you don't mix them. + // Mark room/thread as read + .addAction(markAsReadActionFactory.create(roomInfo, threadId)) + .setContentIntent(openIntent) + .setLargeIcon(largeIcon) + .setDeleteIntent(pendingIntentFactory.createDismissRoomPendingIntent(roomInfo.sessionId, roomInfo.roomId)) .apply { + // Sets priority for 25 and below. For 26 and above, 'priority' is deprecated for + // 'importance' which is set in the NotificationChannel. The integers representing + // 'priority' are different from 'importance', so make sure you don't mix them. if (roomInfo.shouldBing) { - // Compat priority = NotificationCompat.PRIORITY_DEFAULT - /* - vectorPreferences.getNotificationRingTone()?.let { - setSound(it) - } - */ setLights(notificationAccountParams.color, 500, 500) } else { priority = NotificationCompat.PRIORITY_LOW } - // Clear existing actions since we might be updating an existing notification - clearActions() - // Add actions and notification intents - // Mark room/thread as read - addAction(markAsReadActionFactory.create(roomInfo, threadId)) // Quick reply if (!roomInfo.hasSmartReplyError) { val latestEventId = events.lastOrNull()?.eventId addAction(quickReplyActionFactory.create(roomInfo, latestEventId, threadId)) } - if (openIntent != null) { - setContentIntent(openIntent) - } - setLargeIcon(largeIcon) - setDeleteIntent(pendingIntentFactory.createDismissRoomPendingIntent(roomInfo.sessionId, roomInfo.roomId)) - - // If any of the events are of rtc notification type it means a missed call, set the category to the right value - if (events.any { it.type == EventType.RTC_NOTIFICATION }) { - setCategory(NotificationCompat.CATEGORY_MISSED_CALL) - } } .setTicker(tickerText) .build() @@ -237,32 +226,26 @@ class DefaultNotificationCreator( .setContentText(inviteNotifiableEvent.description.annotateForDebug(6)) .setGroupAlertBehavior(NotificationCompat.GROUP_ALERT_ALL) .configureWith(notificationAccountParams) + .addAction(rejectInvitationActionFactory.create(inviteNotifiableEvent)) + .addAction(acceptInvitationActionFactory.create(inviteNotifiableEvent)) + // Build the pending intent for when the notification is clicked + .setContentIntent(pendingIntentFactory.createOpenRoomPendingIntent(inviteNotifiableEvent.sessionId, inviteNotifiableEvent.roomId, null)) .apply { - addAction(rejectInvitationActionFactory.create(inviteNotifiableEvent)) - addAction(acceptInvitationActionFactory.create(inviteNotifiableEvent)) - // Build the pending intent for when the notification is clicked - setContentIntent(pendingIntentFactory.createOpenRoomPendingIntent(inviteNotifiableEvent.sessionId, inviteNotifiableEvent.roomId, null)) - if (inviteNotifiableEvent.noisy) { // Compat priority = NotificationCompat.PRIORITY_DEFAULT - /* - vectorPreferences.getNotificationRingTone()?.let { - setSound(it) - } - */ setLights(notificationAccountParams.color, 500, 500) } else { priority = NotificationCompat.PRIORITY_LOW } - setDeleteIntent( - pendingIntentFactory.createDismissInvitePendingIntent( - inviteNotifiableEvent.sessionId, - inviteNotifiableEvent.roomId, - ) - ) - setAutoCancel(true) } + .setDeleteIntent( + pendingIntentFactory.createDismissInvitePendingIntent( + inviteNotifiableEvent.sessionId, + inviteNotifiableEvent.roomId, + ) + ) + .setAutoCancel(true) .build() } @@ -283,11 +266,6 @@ class DefaultNotificationCreator( if (simpleNotifiableEvent.noisy) { // Compat priority = NotificationCompat.PRIORITY_DEFAULT - /* - vectorPreferences.getNotificationRingTone()?.let { - setSound(it) - } - */ setLights(notificationAccountParams.color, 500, 500) } else { priority = NotificationCompat.PRIORITY_LOW @@ -346,11 +324,6 @@ class DefaultNotificationCreator( if (noisy) { // Compat priority = NotificationCompat.PRIORITY_DEFAULT - /* - vectorPreferences.getNotificationRingTone()?.let { - setSound(it) - } - */ setLights(notificationAccountParams.color, 500, 500) } else { // compat From e706e5fa0e2e2a9afde30f25693edca0d5ef9228 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 12:26:35 +0100 Subject: [PATCH 03/20] Unregister the pusher when the topic is deleted (unregistered) --- .../UnifiedPushRemovedGatewayHandler.kt | 53 +++++++++++++++++++ .../UnregisterUnifiedPushUseCase.kt | 25 ++++++--- .../VectorUnifiedPushMessagingReceiver.kt | 17 ++---- 3 files changed, 77 insertions(+), 18 deletions(-) create mode 100644 libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt new file mode 100644 index 0000000000..7f494c3f52 --- /dev/null +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2025 Element Creations 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.pushproviders.unifiedpush + +import dev.zacsweers.metro.AppScope +import dev.zacsweers.metro.ContributesBinding +import io.element.android.libraries.core.extensions.flatMap +import io.element.android.libraries.core.log.logger.LoggerTag +import io.element.android.libraries.matrix.api.MatrixClientProvider +import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret +import timber.log.Timber + +private val loggerTag = LoggerTag("UnifiedPushRemovedGatewayHandler", LoggerTag.PushLoggerTag) + +/** + * Handle new endpoint received from UnifiedPush. Will update the session matching the client secret. + */ +interface UnifiedPushRemovedGatewayHandler { + suspend fun handle(clientSecret: String): Result +} + +@ContributesBinding(AppScope::class) +class DefaultUnifiedPushRemovedGatewayHandler( + private val unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase, + private val pushClientSecret: PushClientSecret, + private val matrixClientProvider: MatrixClientProvider, +) : UnifiedPushRemovedGatewayHandler { + override suspend fun handle(clientSecret: String): Result { + // Unregister the pusher for the session with this client secret, if is it using UnifiedPush. + val userId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure( + IllegalStateException("Unable to retrieve session") + ).also { + Timber.tag(loggerTag.value).w("Unable to retrieve session") + } + return matrixClientProvider + .getOrRestore(userId) + .flatMap { client -> + unregisterUnifiedPushUseCase.unregister( + matrixClient = client, + clientSecret = clientSecret, + unregisterUnifiedPush = false, + ) + } + .onFailure { + Timber.tag(loggerTag.value).w(it, "Unable to unregister pusher") + } + } +} diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt index 4340344b14..3157f1492f 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt @@ -21,12 +21,19 @@ interface UnregisterUnifiedPushUseCase { /** * Unregister the app from the homeserver, then from UnifiedPush. */ - suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result + suspend fun unregister( + matrixClient: MatrixClient, + clientSecret: String, + unregisterUnifiedPush: Boolean = true, + ): Result /** * Cleanup any remaining data for the given client secret and unregister the app from UnifiedPush. */ - fun cleanup(clientSecret: String) + fun cleanup( + clientSecret: String, + unregisterUnifiedPush: Boolean = true, + ) } @ContributesBinding(AppScope::class) @@ -35,7 +42,11 @@ class DefaultUnregisterUnifiedPushUseCase( private val unifiedPushStore: UnifiedPushStore, private val pusherSubscriber: PusherSubscriber, ) : UnregisterUnifiedPushUseCase { - override suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result { + override suspend fun unregister( + matrixClient: MatrixClient, + clientSecret: String, + unregisterUnifiedPush: Boolean, + ): Result { val endpoint = unifiedPushStore.getEndpoint(clientSecret) val gateway = unifiedPushStore.getPushGateway(clientSecret) if (endpoint == null || gateway == null) { @@ -46,13 +57,15 @@ class DefaultUnregisterUnifiedPushUseCase( } return pusherSubscriber.unregisterPusher(matrixClient, endpoint, gateway) .onSuccess { - cleanup(clientSecret) + cleanup(clientSecret, unregisterUnifiedPush) } } - override fun cleanup(clientSecret: String) { + override fun cleanup(clientSecret: String, unregisterUnifiedPush: Boolean) { unifiedPushStore.storeUpEndpoint(clientSecret, null) unifiedPushStore.storePushGateway(clientSecret, null) - UnifiedPush.unregister(context, clientSecret) + if (unregisterUnifiedPush) { + UnifiedPush.unregister(context, clientSecret) + } } } diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt index 8180ac436b..a7484585fe 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt @@ -35,7 +35,9 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() { @Inject lateinit var unifiedPushGatewayResolver: UnifiedPushGatewayResolver @Inject lateinit var unifiedPushGatewayUrlResolver: UnifiedPushGatewayUrlResolver @Inject lateinit var newGatewayHandler: UnifiedPushNewGatewayHandler + @Inject lateinit var removedGatewayHandler: UnifiedPushRemovedGatewayHandler @Inject lateinit var endpointRegistrationHandler: EndpointRegistrationHandler + @AppCoroutineScope @Inject lateinit var coroutineScope: CoroutineScope @@ -116,18 +118,9 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() { * Called when this application is unregistered from receiving push messages. */ override fun onUnregistered(context: Context, instance: String) { - Timber.tag(loggerTag.value).w("UnifiedPush: Unregistered") - /* - val mode = BackgroundSyncMode.FDROID_BACKGROUND_SYNC_MODE_FOR_REALTIME - pushDataStore.setFdroidSyncBackgroundMode(mode) - guardServiceStarter.start() - runBlocking { - try { - pushersManager.unregisterPusher(unifiedPushHelper.getEndpointOrToken().orEmpty()) - } catch (e: Exception) { - Timber.tag(loggerTag.value).d("Probably unregistering a non existing pusher") - } + Timber.tag(loggerTag.value).w("onUnregistered $instance") + coroutineScope.launch { + removedGatewayHandler.handle(instance) } - */ } } From 6b7e8f72f57051808724e4014d45c74d4acc5846 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 12:26:46 +0100 Subject: [PATCH 04/20] Improve logs --- .../unifiedpush/UnifiedPushNewGatewayHandler.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushNewGatewayHandler.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushNewGatewayHandler.kt index 98e11c8bb7..d7de923a26 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushNewGatewayHandler.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushNewGatewayHandler.kt @@ -39,7 +39,7 @@ class DefaultUnifiedPushNewGatewayHandler( val userId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure( IllegalStateException("Unable to retrieve session") ).also { - Timber.w("Unable to retrieve session") + Timber.tag(loggerTag.value).w("Unable to retrieve session") } val userDataStore = userPushStoreFactory.getOrCreate(userId) return if (userDataStore.getPushProviderName() == UnifiedPushConfig.NAME) { @@ -48,6 +48,9 @@ class DefaultUnifiedPushNewGatewayHandler( .flatMap { client -> pusherSubscriber.registerPusher(client, endpoint, pushGateway) } + .onFailure { + Timber.tag(loggerTag.value).w(it, "Unable to register pusher") + } } else { Timber.tag(loggerTag.value).d("This session is not using UnifiedPush pusher") Result.failure( From f9c0b9e8bb528a514e38e6671d985c056c41fe5a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 12:34:40 +0100 Subject: [PATCH 05/20] UnifiedPush: emit error when registration fails. Note that I did not manage to have the method `onRegistrationFailed` invoked. If the network is not available for instance, unregistering the previous pusher will fail first. --- .../android/libraries/push/api/PushService.kt | 6 +++ .../libraries/push/impl/DefaultPushService.kt | 7 +++ .../notifications/NotificationDisplayer.kt | 10 ++++ .../factories/NotificationCreator.kt | 26 ++++++++++ .../ServiceUnregisteredHandler.kt | 47 +++++++++++++++++++ .../push/impl/DefaultPushServiceTest.kt | 4 ++ .../fake/FakeNotificationCreator.kt | 6 +++ .../fake/FakeNotificationDisplayer.kt | 5 ++ .../FakeServiceUnregisteredHandler.kt | 19 ++++++++ .../libraries/push/test/FakePushService.kt | 6 +++ .../unifiedpush/build.gradle.kts | 1 + .../UnifiedPushRemovedGatewayHandler.kt | 35 ++++++++++++-- .../VectorUnifiedPushMessagingReceiver.kt | 14 +++--- .../FakeUnregisterUnifiedPushUseCase.kt | 19 +++++--- .../unifiedpush/UnifiedPushProviderTest.kt | 12 ++--- .../VectorUnifiedPushMessagingReceiverTest.kt | 14 +++++- 16 files changed, 208 insertions(+), 23 deletions(-) create mode 100644 libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unregistration/ServiceUnregisteredHandler.kt create mode 100644 libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unregistration/FakeServiceUnregisteredHandler.kt diff --git a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt index 553bee1e17..2096a04656 100644 --- a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt +++ b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt @@ -10,6 +10,7 @@ package io.element.android.libraries.push.api import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.SessionId +import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.push.api.history.PushHistoryItem import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider @@ -73,4 +74,9 @@ interface PushService { * Reset the battery optimization state. */ suspend fun resetBatteryOptimizationState() + + /** + * Notify the user that the service is un-registered. + */ + suspend fun onServiceUnregistered(userId: UserId) } diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt index 1ea30a7766..5c77438293 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt @@ -14,12 +14,14 @@ import dev.zacsweers.metro.SingleIn import dev.zacsweers.metro.binding import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.SessionId +import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.push.api.GetCurrentPushProvider import io.element.android.libraries.push.api.PushService import io.element.android.libraries.push.api.history.PushHistoryItem import io.element.android.libraries.push.impl.push.MutableBatteryOptimizationStore import io.element.android.libraries.push.impl.store.PushDataStore import io.element.android.libraries.push.impl.test.TestPush +import io.element.android.libraries.push.impl.unregistration.ServiceUnregisteredHandler import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider import io.element.android.libraries.pushstore.api.UserPushStoreFactory @@ -40,6 +42,7 @@ class DefaultPushService( private val pushClientSecretStore: PushClientSecretStore, private val pushDataStore: PushDataStore, private val mutableBatteryOptimizationStore: MutableBatteryOptimizationStore, + private val serviceUnregisteredHandler: ServiceUnregisteredHandler, ) : PushService, SessionListener { init { observeSessions() @@ -141,4 +144,8 @@ class DefaultPushService( override suspend fun resetBatteryOptimizationState() { mutableBatteryOptimizationStore.reset() } + + override suspend fun onServiceUnregistered(userId: UserId) { + serviceUnregisteredHandler.handle(userId) + } } 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 3e9c917cfd..d46bffce73 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 @@ -24,6 +24,7 @@ interface NotificationDisplayer { fun cancelNotification(tag: String?, id: Int) fun displayDiagnosticNotification(notification: Notification): Boolean fun dismissDiagnosticNotification() + fun displayUnregistrationNotification(notification: Notification): Boolean } @ContributesBinding(AppScope::class) @@ -60,6 +61,14 @@ class DefaultNotificationDisplayer( ) } + override fun displayUnregistrationNotification(notification: Notification): Boolean { + return showNotification( + tag = TAG_DIAGNOSTIC, + id = NOTIFICATION_ID_UNREGISTRATION, + notification = notification, + ) + } + companion object { private const val TAG_DIAGNOSTIC = "DIAGNOSTIC" @@ -67,5 +76,6 @@ class DefaultNotificationDisplayer( * IDs for notifications * ========================================================================================== */ private const val NOTIFICATION_ID_DIAGNOSTIC = 888 + private const val NOTIFICATION_ID_UNREGISTRATION = 889 } } 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 70d01cbf24..98a1228297 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 @@ -91,6 +91,10 @@ interface NotificationCreator { @ColorInt color: Int, ): Notification + fun createUnregistrationNotification( + notificationAccountParams: NotificationAccountParams, + ): Notification + companion object { /** * Creates a tag for a message notification given its [roomId] and optional [threadId]. @@ -352,6 +356,28 @@ class DefaultNotificationCreator( .build() } + override fun createUnregistrationNotification( + notificationAccountParams: NotificationAccountParams, + ): Notification { + val userId = notificationAccountParams.user.userId + val text = if (notificationAccountParams.showSessionId) { + // TODO i18n + "$userId will not receive notifications anymore." + } else { + // TODO i18n + "You will not receive notifications anymore." + } + return NotificationCompat.Builder(context, notificationChannels.getChannelIdForTest()) + .setContentTitle(stringProvider.getString(CommonStrings.dialog_title_warning)) + .setContentText(text) + .configureWith(notificationAccountParams) + .setPriority(NotificationCompat.PRIORITY_MAX) + .setCategory(NotificationCompat.CATEGORY_ERROR) + .setAutoCancel(true) + .setContentIntent(pendingIntentFactory.createOpenSessionPendingIntent(userId)) + .build() + } + private suspend fun MessagingStyle.addMessagesFromEvents( events: List, imageLoader: ImageLoader, diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unregistration/ServiceUnregisteredHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unregistration/ServiceUnregisteredHandler.kt new file mode 100644 index 0000000000..cb42ceaf5d --- /dev/null +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unregistration/ServiceUnregisteredHandler.kt @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2025 Element Creations 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.unregistration + +import androidx.compose.ui.graphics.toArgb +import dev.zacsweers.metro.AppScope +import dev.zacsweers.metro.ContributesBinding +import io.element.android.appconfig.NotificationConfig +import io.element.android.features.enterprise.api.EnterpriseService +import io.element.android.libraries.matrix.api.core.UserId +import io.element.android.libraries.matrix.api.user.MatrixUser +import io.element.android.libraries.push.impl.notifications.NotificationDisplayer +import io.element.android.libraries.push.impl.notifications.factories.NotificationAccountParams +import io.element.android.libraries.push.impl.notifications.factories.NotificationCreator +import io.element.android.libraries.sessionstorage.api.SessionStore +import kotlinx.coroutines.flow.first + +interface ServiceUnregisteredHandler { + suspend fun handle(userId: UserId) +} + +@ContributesBinding(AppScope::class) +class DefaultServiceUnregisteredHandler( + private val enterpriseService: EnterpriseService, + private val notificationCreator: NotificationCreator, + private val notificationDisplayer: NotificationDisplayer, + private val sessionStore: SessionStore, +) : ServiceUnregisteredHandler { + override suspend fun handle(userId: UserId) { + val color = enterpriseService.brandColorsFlow(userId).first()?.toArgb() + ?: NotificationConfig.NOTIFICATION_ACCENT_COLOR + val hasMultipleAccounts = sessionStore.numberOfSessions() > 1 + val notification = notificationCreator.createUnregistrationNotification( + NotificationAccountParams( + user = MatrixUser(userId), + color = color, + showSessionId = hasMultipleAccounts, + ) + ) + notificationDisplayer.displayDiagnosticNotification(notification) + } +} diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt index d862e1cf39..7ddbc93fb1 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt @@ -25,6 +25,8 @@ import io.element.android.libraries.push.impl.store.InMemoryPushDataStore import io.element.android.libraries.push.impl.store.PushDataStore import io.element.android.libraries.push.impl.test.FakeTestPush import io.element.android.libraries.push.impl.test.TestPush +import io.element.android.libraries.push.impl.unregistration.FakeServiceUnregisteredHandler +import io.element.android.libraries.push.impl.unregistration.ServiceUnregisteredHandler import io.element.android.libraries.push.test.FakeGetCurrentPushProvider import io.element.android.libraries.pushproviders.api.Config import io.element.android.libraries.pushproviders.api.Distributor @@ -346,6 +348,7 @@ class DefaultPushServiceTest { pushClientSecretStore: PushClientSecretStore = InMemoryPushClientSecretStore(), pushDataStore: PushDataStore = InMemoryPushDataStore(), mutableBatteryOptimizationStore: MutableBatteryOptimizationStore = FakeMutableBatteryOptimizationStore(), + serviceUnregisteredHandler: ServiceUnregisteredHandler = FakeServiceUnregisteredHandler(), ): DefaultPushService { return DefaultPushService( testPush = testPush, @@ -356,6 +359,7 @@ class DefaultPushServiceTest { pushClientSecretStore = pushClientSecretStore, pushDataStore = pushDataStore, mutableBatteryOptimizationStore = mutableBatteryOptimizationStore, + serviceUnregisteredHandler = serviceUnregisteredHandler, ) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationCreator.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationCreator.kt index 76302d772e..d5e4ad9695 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationCreator.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationCreator.kt @@ -41,6 +41,8 @@ class FakeNotificationCreator( > = lambdaRecorder { _, _, _, _, _ -> A_NOTIFICATION }, var createDiagnosticNotificationResult: LambdaOneParamRecorder = lambdaRecorder { _ -> A_NOTIFICATION }, + val createUnregistrationNotificationResult: LambdaOneParamRecorder = + lambdaRecorder { _ -> A_NOTIFICATION }, ) : NotificationCreator { override suspend fun createMessagesListNotification( notificationAccountParams: NotificationAccountParams, @@ -93,4 +95,8 @@ class FakeNotificationCreator( ): Notification { return createDiagnosticNotificationResult(color) } + + override fun createUnregistrationNotification(notificationAccountParams: NotificationAccountParams): Notification { + return createUnregistrationNotificationResult(notificationAccountParams) + } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationDisplayer.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationDisplayer.kt index 9ab32cc13e..fd4af70a72 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationDisplayer.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fake/FakeNotificationDisplayer.kt @@ -24,6 +24,7 @@ class FakeNotificationDisplayer( var cancelNotificationResult: LambdaTwoParamsRecorder = lambdaRecorder { _, _ -> }, var displayDiagnosticNotificationResult: LambdaOneParamRecorder = lambdaRecorder { _ -> true }, var dismissDiagnosticNotificationResult: LambdaNoParamRecorder = lambdaRecorder { -> }, + var displayUnregistrationNotificationResult: LambdaOneParamRecorder = lambdaRecorder { _ -> true }, ) : NotificationDisplayer { override fun showNotification(tag: String?, id: Int, notification: Notification): Boolean { return showNotificationResult(tag, id, notification) @@ -41,6 +42,10 @@ class FakeNotificationDisplayer( return dismissDiagnosticNotificationResult() } + override fun displayUnregistrationNotification(notification: Notification): Boolean { + return displayUnregistrationNotificationResult(notification) + } + fun verifySummaryCancelled(times: Int = 1) { cancelNotificationResult.assertions().isCalledExactly(times).withSequence( listOf(value(null), value(NotificationIdProvider.getSummaryNotificationId(A_SESSION_ID))) diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unregistration/FakeServiceUnregisteredHandler.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unregistration/FakeServiceUnregisteredHandler.kt new file mode 100644 index 0000000000..0ae4190973 --- /dev/null +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unregistration/FakeServiceUnregisteredHandler.kt @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2025 Element Creations 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.unregistration + +import io.element.android.libraries.matrix.api.core.UserId +import io.element.android.tests.testutils.lambda.lambdaError + +class FakeServiceUnregisteredHandler( + private val handleResult: (UserId) -> Unit = { lambdaError() }, +) : ServiceUnregisteredHandler { + override suspend fun handle(userId: UserId) { + handleResult(userId) + } +} diff --git a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt b/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt index d0198fba91..ee99ef1a20 100644 --- a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt +++ b/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt @@ -10,6 +10,7 @@ package io.element.android.libraries.push.test import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.SessionId +import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.push.api.PushService import io.element.android.libraries.push.api.history.PushHistoryItem import io.element.android.libraries.pushproviders.api.Distributor @@ -30,6 +31,7 @@ class FakePushService( private val setIgnoreRegistrationErrorLambda: (SessionId, Boolean) -> Unit = { _, _ -> lambdaError() }, private val resetPushHistoryResult: () -> Unit = { lambdaError() }, private val resetBatteryOptimizationStateResult: () -> Unit = { lambdaError() }, + private val onServiceUnregisteredResult: (UserId) -> Unit = { lambdaError() }, ) : PushService { override suspend fun getCurrentPushProvider(sessionId: SessionId): PushProvider? { return registeredPushProvider ?: currentPushProvider(sessionId) @@ -98,4 +100,8 @@ class FakePushService( override suspend fun resetBatteryOptimizationState() { resetBatteryOptimizationStateResult() } + + override suspend fun onServiceUnregistered(userId: UserId) { + onServiceUnregisteredResult(userId) + } } diff --git a/libraries/pushproviders/unifiedpush/build.gradle.kts b/libraries/pushproviders/unifiedpush/build.gradle.kts index 6369399f28..1b24bc4b1f 100644 --- a/libraries/pushproviders/unifiedpush/build.gradle.kts +++ b/libraries/pushproviders/unifiedpush/build.gradle.kts @@ -24,6 +24,7 @@ dependencies { implementation(projects.libraries.androidutils) implementation(projects.libraries.core) implementation(projects.libraries.matrix.api) + implementation(projects.libraries.push.api) implementation(projects.libraries.uiStrings) api(projects.libraries.troubleshoot.api) diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt index 7f494c3f52..fa75fd101e 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt @@ -12,6 +12,7 @@ import dev.zacsweers.metro.ContributesBinding import io.element.android.libraries.core.extensions.flatMap import io.element.android.libraries.core.log.logger.LoggerTag import io.element.android.libraries.matrix.api.MatrixClientProvider +import io.element.android.libraries.push.api.PushService import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret import timber.log.Timber @@ -20,7 +21,7 @@ private val loggerTag = LoggerTag("UnifiedPushRemovedGatewayHandler", LoggerTag. /** * Handle new endpoint received from UnifiedPush. Will update the session matching the client secret. */ -interface UnifiedPushRemovedGatewayHandler { +fun interface UnifiedPushRemovedGatewayHandler { suspend fun handle(clientSecret: String): Result } @@ -29,9 +30,10 @@ class DefaultUnifiedPushRemovedGatewayHandler( private val unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase, private val pushClientSecret: PushClientSecret, private val matrixClientProvider: MatrixClientProvider, + private val pushService: PushService, ) : UnifiedPushRemovedGatewayHandler { override suspend fun handle(clientSecret: String): Result { - // Unregister the pusher for the session with this client secret, if is it using UnifiedPush. + // Unregister the pusher for the session with this client secret. val userId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure( IllegalStateException("Unable to retrieve session") ).also { @@ -39,15 +41,42 @@ class DefaultUnifiedPushRemovedGatewayHandler( } return matrixClientProvider .getOrRestore(userId) + .onFailure { + Timber.tag(loggerTag.value).w(it, "Fails to restore client") + } .flatMap { client -> unregisterUnifiedPushUseCase.unregister( matrixClient = client, clientSecret = clientSecret, unregisterUnifiedPush = false, ) + .onFailure { + Timber.tag(loggerTag.value).w(it, "Unable to unregister pusher") + } + .flatMap { + val pushProvider = pushService.getCurrentPushProvider(userId) + val distributor = pushProvider?.getCurrentDistributor(userId) + // Attempt to register again + if (pushProvider != null && distributor != null) { + pushService.registerWith( + client, + pushProvider, + distributor, + ) + .onFailure { + Timber.tag(loggerTag.value).w(it, "Unable to register with current data") + } + } else { + Result.failure(IllegalStateException("Unable to register again")) + } + } + .onFailure { + // Let the user know + pushService.onServiceUnregistered(userId) + } } .onFailure { - Timber.tag(loggerTag.value).w(it, "Unable to unregister pusher") + Timber.tag(loggerTag.value).w(it, "Issue during pusher unregistration / re registration") } } } diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt index a7484585fe..05f6969fc5 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiver.kt @@ -106,12 +106,14 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() { */ override fun onRegistrationFailed(context: Context, reason: FailedReason, instance: String) { Timber.tag(loggerTag.value).e("onRegistrationFailed for $instance, reason: $reason") - /* - Toast.makeText(context, "Push service registration failed", Toast.LENGTH_SHORT).show() - val mode = BackgroundSyncMode.FDROID_BACKGROUND_SYNC_MODE_FOR_REALTIME - pushDataStore.setFdroidSyncBackgroundMode(mode) - guardServiceStarter.start() - */ + coroutineScope.launch { + endpointRegistrationHandler.registrationDone( + RegistrationResult( + clientSecret = instance, + result = Result.failure(Exception("Registration failed. Reason: $reason")), + ) + ) + } } /** diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt index cda28554db..182bb5f823 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt @@ -12,14 +12,21 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.tests.testutils.lambda.lambdaError class FakeUnregisterUnifiedPushUseCase( - private val unregisterLambda: (MatrixClient, String) -> Result = { _, _ -> lambdaError() }, - private val cleanupLambda: (String) -> Unit = { lambdaError() }, + private val unregisterLambda: (MatrixClient, String, Boolean) -> Result = { _, _, _ -> lambdaError() }, + private val cleanupLambda: (String, Boolean) -> Unit = { _, _ -> lambdaError() }, ) : UnregisterUnifiedPushUseCase { - override suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result { - return unregisterLambda(matrixClient, clientSecret) + override suspend fun unregister( + matrixClient: MatrixClient, + clientSecret: String, + unregisterUnifiedPush: Boolean, + ): Result { + return unregisterLambda(matrixClient, clientSecret, unregisterUnifiedPush) } - override fun cleanup(clientSecret: String) { - cleanupLambda(clientSecret) + override fun cleanup( + clientSecret: String, + unregisterUnifiedPush: Boolean, + ) { + cleanupLambda(clientSecret, unregisterUnifiedPush) } } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt index 10ed18118b..e22c1c3bf4 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt @@ -118,7 +118,7 @@ class UnifiedPushProviderTest { fun `unregister ok`() = runTest { val matrixClient = FakeMatrixClient() val getSecretForUserResultLambda = lambdaRecorder { A_SECRET } - val unregisterLambda = lambdaRecorder> { _, _ -> Result.success(Unit) } + val unregisterLambda = lambdaRecorder> { _, _, _ -> Result.success(Unit) } val unifiedPushProvider = createUnifiedPushProvider( pushClientSecret = FakePushClientSecret( getSecretForUserResult = getSecretForUserResultLambda, @@ -134,14 +134,14 @@ class UnifiedPushProviderTest { .with(value(A_SESSION_ID)) unregisterLambda.assertions() .isCalledOnce() - .with(value(matrixClient), value(A_SECRET)) + .with(value(matrixClient), value(A_SECRET), value(true)) } @Test fun `unregister ko`() = runTest { val matrixClient = FakeMatrixClient() val getSecretForUserResultLambda = lambdaRecorder { A_SECRET } - val unregisterLambda = lambdaRecorder> { _, _ -> Result.failure(AN_EXCEPTION) } + val unregisterLambda = lambdaRecorder> { _, _, _ -> Result.failure(AN_EXCEPTION) } val unifiedPushProvider = createUnifiedPushProvider( pushClientSecret = FakePushClientSecret( getSecretForUserResult = getSecretForUserResultLambda, @@ -157,7 +157,7 @@ class UnifiedPushProviderTest { .with(value(A_SESSION_ID)) unregisterLambda.assertions() .isCalledOnce() - .with(value(matrixClient), value(A_SECRET)) + .with(value(matrixClient), value(A_SECRET), value(true)) } @Test @@ -230,7 +230,7 @@ class UnifiedPushProviderTest { @Test fun `onSessionDeleted should do the cleanup`() = runTest { - val cleanupLambda = lambdaRecorder { } + val cleanupLambda = lambdaRecorder { _, _ -> } val unifiedPushProvider = createUnifiedPushProvider( pushClientSecret = FakePushClientSecret( getSecretForUserResult = { A_SECRET } @@ -240,7 +240,7 @@ class UnifiedPushProviderTest { ), ) unifiedPushProvider.onSessionDeleted(A_SESSION_ID) - cleanupLambda.assertions().isCalledOnce().with(value(A_SECRET)) + cleanupLambda.assertions().isCalledOnce().with(value(A_SECRET), value(true)) } private fun createUnifiedPushProvider( diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt index 6cf82c1fc0..f10f6430f0 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/VectorUnifiedPushMessagingReceiverTest.kt @@ -23,6 +23,7 @@ import io.element.android.libraries.pushproviders.api.PushData import io.element.android.libraries.pushproviders.api.PushHandler import io.element.android.libraries.pushproviders.unifiedpush.registration.EndpointRegistrationHandler import io.element.android.libraries.pushproviders.unifiedpush.registration.RegistrationResult +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 @@ -51,10 +52,17 @@ class VectorUnifiedPushMessagingReceiverTest { } @Test - fun `onUnregistered does nothing`() = runTest { + fun `onUnregistered invokes the removedGatewayHandler`() = runTest { val context = InstrumentationRegistry.getInstrumentation().context - val vectorUnifiedPushMessagingReceiver = createVectorUnifiedPushMessagingReceiver() + val handleResult = lambdaRecorder> { + Result.success(Unit) + } + val vectorUnifiedPushMessagingReceiver = createVectorUnifiedPushMessagingReceiver( + removedGatewayHandler = UnifiedPushRemovedGatewayHandler { handleResult(it) }, + ) vectorUnifiedPushMessagingReceiver.onUnregistered(context, A_SECRET) + advanceUntilIdle() + handleResult.assertions().isCalledOnce().with(value(A_SECRET)) } @Test @@ -199,6 +207,7 @@ class VectorUnifiedPushMessagingReceiverTest { unifiedPushGatewayUrlResolver: UnifiedPushGatewayUrlResolver = FakeUnifiedPushGatewayUrlResolver(), unifiedPushNewGatewayHandler: UnifiedPushNewGatewayHandler = FakeUnifiedPushNewGatewayHandler(), endpointRegistrationHandler: EndpointRegistrationHandler = EndpointRegistrationHandler(), + removedGatewayHandler: UnifiedPushRemovedGatewayHandler = UnifiedPushRemovedGatewayHandler { lambdaError() }, ): VectorUnifiedPushMessagingReceiver { return VectorUnifiedPushMessagingReceiver().apply { this.pushParser = unifiedPushParser @@ -208,6 +217,7 @@ class VectorUnifiedPushMessagingReceiverTest { this.unifiedPushGatewayResolver = unifiedPushGatewayResolver this.unifiedPushGatewayUrlResolver = unifiedPushGatewayUrlResolver this.newGatewayHandler = unifiedPushNewGatewayHandler + this.removedGatewayHandler = removedGatewayHandler this.endpointRegistrationHandler = endpointRegistrationHandler this.coroutineScope = this@createVectorUnifiedPushMessagingReceiver } From 71e8df262826e9dfff2c8c366061c67e906a967f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 14:58:44 +0100 Subject: [PATCH 06/20] Rename val --- .../call/impl/notifications/RingingCallNotificationCreator.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/notifications/RingingCallNotificationCreator.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/notifications/RingingCallNotificationCreator.kt index d1575c7a97..88ddddf0cf 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/notifications/RingingCallNotificationCreator.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/notifications/RingingCallNotificationCreator.kt @@ -72,7 +72,7 @@ class RingingCallNotificationCreator( ): Notification? { val matrixClient = matrixClientProvider.getOrRestore(sessionId).getOrNull() ?: return null val imageLoader = imageLoaderHolder.get(matrixClient) - val largeIcon = notificationBitmapLoader.getUserIcon( + val userIcon = notificationBitmapLoader.getUserIcon( avatarData = AvatarData( id = roomId.value, name = roomName, @@ -84,7 +84,7 @@ class RingingCallNotificationCreator( val caller = Person.Builder() .setName(senderDisplayName) - .setIcon(largeIcon) + .setIcon(userIcon) .setImportant(true) .build() From 0dcd63c7f9fec1a1d0bb50de5d2dd51130e3efe6 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 15:13:05 +0100 Subject: [PATCH 07/20] Add unit test on DefaultServiceUnregisteredHandler --- .../ServiceUnregisteredHandler.kt | 2 +- .../DefaultServiceUnregisteredHandlerTest.kt | 118 ++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unregistration/DefaultServiceUnregisteredHandlerTest.kt diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unregistration/ServiceUnregisteredHandler.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unregistration/ServiceUnregisteredHandler.kt index cb42ceaf5d..29052b42ca 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unregistration/ServiceUnregisteredHandler.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unregistration/ServiceUnregisteredHandler.kt @@ -42,6 +42,6 @@ class DefaultServiceUnregisteredHandler( showSessionId = hasMultipleAccounts, ) ) - notificationDisplayer.displayDiagnosticNotification(notification) + notificationDisplayer.displayUnregistrationNotification(notification) } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unregistration/DefaultServiceUnregisteredHandlerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unregistration/DefaultServiceUnregisteredHandlerTest.kt new file mode 100644 index 0000000000..bda82a3c3e --- /dev/null +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unregistration/DefaultServiceUnregisteredHandlerTest.kt @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2025 Element Creations 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.unregistration + +import android.app.Notification +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.toArgb +import io.element.android.appconfig.NotificationConfig +import io.element.android.features.enterprise.api.EnterpriseService +import io.element.android.features.enterprise.test.FakeEnterpriseService +import io.element.android.libraries.matrix.api.user.MatrixUser +import io.element.android.libraries.matrix.test.A_SESSION_ID +import io.element.android.libraries.matrix.test.A_SESSION_ID_2 +import io.element.android.libraries.push.impl.notifications.NotificationDisplayer +import io.element.android.libraries.push.impl.notifications.factories.NotificationAccountParams +import io.element.android.libraries.push.impl.notifications.factories.NotificationCreator +import io.element.android.libraries.push.impl.notifications.fake.FakeNotificationCreator +import io.element.android.libraries.push.impl.notifications.fake.FakeNotificationDisplayer +import io.element.android.libraries.push.impl.notifications.fixtures.A_NOTIFICATION +import io.element.android.libraries.sessionstorage.api.SessionStore +import io.element.android.libraries.sessionstorage.test.InMemorySessionStore +import io.element.android.libraries.sessionstorage.test.aSessionData +import io.element.android.tests.testutils.lambda.lambdaRecorder +import io.element.android.tests.testutils.lambda.value +import kotlinx.coroutines.test.runTest +import org.junit.Test + +class DefaultServiceUnregisteredHandlerTest { + @Test + fun `handle will create a notification and render it`() = runTest { + val notification = A_NOTIFICATION + val createUnregistrationNotificationResult = lambdaRecorder { notification } + val displayUnregistrationNotificationResult = lambdaRecorder { true } + val sut = createDefaultServiceUnregisteredHandler( + notificationCreator = FakeNotificationCreator( + createUnregistrationNotificationResult = createUnregistrationNotificationResult, + ), + notificationDisplayer = FakeNotificationDisplayer( + displayUnregistrationNotificationResult = displayUnregistrationNotificationResult, + ) + ) + sut.handle(A_SESSION_ID) + createUnregistrationNotificationResult.assertions().isCalledOnce().with( + value( + NotificationAccountParams( + MatrixUser( + userId = A_SESSION_ID, + displayName = null, + avatarUrl = null, + ), + color = NotificationConfig.NOTIFICATION_ACCENT_COLOR, + showSessionId = false, + ) + ) + ) + displayUnregistrationNotificationResult.assertions().isCalledOnce().with( + value(notification) + ) + } + + @Test + fun `handle will create a notification and render it - custom color and multi accounts`() = runTest { + val notification = A_NOTIFICATION + val createUnregistrationNotificationResult = lambdaRecorder { notification } + val displayUnregistrationNotificationResult = lambdaRecorder { true } + val sut = createDefaultServiceUnregisteredHandler( + enterpriseService = FakeEnterpriseService( + initialBrandColor = Color.Red, + ), + notificationCreator = FakeNotificationCreator( + createUnregistrationNotificationResult = createUnregistrationNotificationResult, + ), + notificationDisplayer = FakeNotificationDisplayer( + displayUnregistrationNotificationResult = displayUnregistrationNotificationResult, + ), + sessionStore = InMemorySessionStore( + initialList = listOf( + aSessionData(sessionId = A_SESSION_ID.value), + aSessionData(sessionId = A_SESSION_ID_2.value), + ) + ) + ) + sut.handle(A_SESSION_ID) + createUnregistrationNotificationResult.assertions().isCalledOnce().with( + value( + NotificationAccountParams( + MatrixUser( + userId = A_SESSION_ID, + displayName = null, + avatarUrl = null, + ), + color = Color.Red.toArgb(), + showSessionId = true, + ) + ) + ) + displayUnregistrationNotificationResult.assertions().isCalledOnce().with( + value(notification) + ) + } + + private fun createDefaultServiceUnregisteredHandler( + enterpriseService: EnterpriseService = FakeEnterpriseService(), + notificationCreator: NotificationCreator = FakeNotificationCreator(), + notificationDisplayer: NotificationDisplayer = FakeNotificationDisplayer(), + sessionStore: SessionStore = InMemorySessionStore(), + ) = DefaultServiceUnregisteredHandler( + enterpriseService = enterpriseService, + notificationCreator = notificationCreator, + notificationDisplayer = notificationDisplayer, + sessionStore = sessionStore, + ) +} From c453d2adacf35d7480c435f270da2204f2e3f74f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 15:34:33 +0100 Subject: [PATCH 08/20] Add unit test on DefaultUnifiedPushRemovedGatewayHandler --- ...ultUnifiedPushRemovedGatewayHandlerTest.kt | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt new file mode 100644 index 0000000000..04c2bf0323 --- /dev/null +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt @@ -0,0 +1,202 @@ +/* + * Copyright (c) 2025 Element Creations 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.pushproviders.unifiedpush + +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.MatrixClientProvider +import io.element.android.libraries.matrix.api.core.UserId +import io.element.android.libraries.matrix.test.AN_EXCEPTION +import io.element.android.libraries.matrix.test.A_SECRET +import io.element.android.libraries.matrix.test.A_SESSION_ID +import io.element.android.libraries.matrix.test.FakeMatrixClient +import io.element.android.libraries.matrix.test.FakeMatrixClientProvider +import io.element.android.libraries.push.api.PushService +import io.element.android.libraries.push.test.FakePushService +import io.element.android.libraries.pushproviders.api.Distributor +import io.element.android.libraries.pushproviders.test.FakePushProvider +import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret +import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.FakePushClientSecret +import io.element.android.tests.testutils.lambda.any +import io.element.android.tests.testutils.lambda.lambdaRecorder +import io.element.android.tests.testutils.lambda.value +import kotlinx.coroutines.test.runTest +import org.junit.Test + +class DefaultUnifiedPushRemovedGatewayHandlerTest { + @Test + fun `handle returns error if the secret is unknown`() = runTest { + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { null }, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isFailure).isTrue() + } + + @Test + fun `handle returns error if cannot restore the client`() = runTest { + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_SESSION_ID }, + ), + matrixClientProvider = FakeMatrixClientProvider( + getClient = { Result.failure(AN_EXCEPTION) }, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isFailure).isTrue() + } + + @Test + fun `handle returns error if cannot unregister the pusher, and user is notified`() = runTest { + val onServiceUnregisteredResult = lambdaRecorder { } + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_SESSION_ID }, + ), + matrixClientProvider = FakeMatrixClientProvider( + getClient = { Result.success(FakeMatrixClient()) }, + ), + unregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( + unregisterLambda = { _, _, _ -> Result.failure(AN_EXCEPTION) }, + ), + pushService = FakePushService( + onServiceUnregisteredResult = onServiceUnregisteredResult, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isFailure).isTrue() + onServiceUnregisteredResult.assertions().isCalledOnce().with(value(A_SESSION_ID)) + } + + @Test + fun `handle returns error if cannot get current push provider, and user is notified`() = runTest { + val onServiceUnregisteredResult = lambdaRecorder { } + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_SESSION_ID }, + ), + matrixClientProvider = FakeMatrixClientProvider( + getClient = { Result.success(FakeMatrixClient()) }, + ), + unregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( + unregisterLambda = { _, _, _ -> Result.success(Unit) }, + ), + pushService = FakePushService( + currentPushProvider = { null }, + onServiceUnregisteredResult = onServiceUnregisteredResult, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isFailure).isTrue() + onServiceUnregisteredResult.assertions().isCalledOnce().with(value(A_SESSION_ID)) + } + + @Test + fun `handle returns error if cannot get current distributor, and user is notified`() = runTest { + val onServiceUnregisteredResult = lambdaRecorder { } + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_SESSION_ID }, + ), + matrixClientProvider = FakeMatrixClientProvider( + getClient = { Result.success(FakeMatrixClient()) }, + ), + unregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( + unregisterLambda = { _, _, _ -> Result.success(Unit) }, + ), + pushService = FakePushService( + currentPushProvider = { + FakePushProvider( + currentDistributor = { null }, + ) + }, + onServiceUnregisteredResult = onServiceUnregisteredResult, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isFailure).isTrue() + onServiceUnregisteredResult.assertions().isCalledOnce().with(value(A_SESSION_ID)) + } + + @Test + fun `handle returns error if cannot register again, and user is notified`() = runTest { + val onServiceUnregisteredResult = lambdaRecorder { } + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_SESSION_ID }, + ), + matrixClientProvider = FakeMatrixClientProvider( + getClient = { Result.success(FakeMatrixClient()) }, + ), + unregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( + unregisterLambda = { _, _, _ -> Result.success(Unit) }, + ), + pushService = FakePushService( + currentPushProvider = { + FakePushProvider( + currentDistributor = { Distributor("aValue", "aName") }, + ) + }, + registerWithLambda = { _, _, _ -> Result.failure(AN_EXCEPTION) }, + onServiceUnregisteredResult = onServiceUnregisteredResult, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isFailure).isTrue() + onServiceUnregisteredResult.assertions().isCalledOnce().with(value(A_SESSION_ID)) + } + + @Test + fun `handle returns success if can register again, and user is not notified`() = runTest { + val onServiceUnregisteredResult = lambdaRecorder { } + val unregisterLambda = lambdaRecorder> { _, _, _ -> Result.success(Unit) } + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_SESSION_ID }, + ), + matrixClientProvider = FakeMatrixClientProvider( + getClient = { Result.success(FakeMatrixClient()) }, + ), + unregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( + unregisterLambda = unregisterLambda, + ), + pushService = FakePushService( + currentPushProvider = { + FakePushProvider( + currentDistributor = { Distributor("aValue", "aName") }, + ) + }, + registerWithLambda = { _, _, _ -> Result.success(Unit) }, + onServiceUnregisteredResult = onServiceUnregisteredResult, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isSuccess).isTrue() + unregisterLambda.assertions().isCalledOnce().with( + any(), + value(A_SECRET), + value(false), + ) + onServiceUnregisteredResult.assertions().isNeverCalled() + } + + private fun createDefaultUnifiedPushRemovedGatewayHandler( + unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase(), + pushClientSecret: PushClientSecret = FakePushClientSecret(), + matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider(), + pushService: PushService = FakePushService(), + ) = DefaultUnifiedPushRemovedGatewayHandler( + unregisterUnifiedPushUseCase = unregisterUnifiedPushUseCase, + pushClientSecret = pushClientSecret, + matrixClientProvider = matrixClientProvider, + pushService = pushService, + ) +} From c7d468947319d39b236ca806d41b7bf7fedb9497 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 15:39:20 +0100 Subject: [PATCH 09/20] Add missing test. --- .../factories/DefaultNotificationCreatorTest.kt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/factories/DefaultNotificationCreatorTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/factories/DefaultNotificationCreatorTest.kt index cec3631ea3..0504ae433b 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/factories/DefaultNotificationCreatorTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/factories/DefaultNotificationCreatorTest.kt @@ -63,6 +63,21 @@ class DefaultNotificationCreatorTest { ) } + @Test + fun `test createUnregistrationNotification`() { + val sut = createNotificationCreator() + val matrixUser = aMatrixUser() + val result = sut.createUnregistrationNotification( + notificationAccountParams = aNotificationAccountParams( + user = matrixUser, + ), + ) + result.commonAssertions( + expectedGroup = matrixUser.userId.value, + expectedCategory = NotificationCompat.CATEGORY_ERROR, + ) + } + @Test fun `test createFallbackNotification`() { val sut = createNotificationCreator() From d3339872ff19a2291bd56fbd9da75db7a979ca05 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 17:46:49 +0100 Subject: [PATCH 10/20] Ensure that disabling (resp. enabling) notification unregisters (resp. registers) the pusher --- .../appnav/loggedin/LoggedInPresenter.kt | 67 +---- .../appnav/loggedin/LoggedInStateProvider.kt | 1 + .../android/appnav/loggedin/LoggedInView.kt | 1 + .../appnav/loggedin/LoggedInPresenterTest.kt | 270 ++--------------- .../NotificationSettingsPresenter.kt | 10 +- .../NotificationSettingsPresenterTest.kt | 85 +++--- .../android/libraries/push/api/PushService.kt | 9 + .../push/api}/PusherRegistrationFailure.kt | 2 +- .../libraries/push/impl/DefaultPushService.kt | 57 ++++ .../push/impl/DefaultPushServiceTest.kt | 279 ++++++++++++++++++ .../libraries/push/test/FakePushService.kt | 5 + 11 files changed, 436 insertions(+), 350 deletions(-) rename {appnav/src/main/kotlin/io/element/android/appnav/loggedin => libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api}/PusherRegistrationFailure.kt (95%) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt index 9a2ed21755..184766e323 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInPresenter.kt @@ -36,7 +36,7 @@ import io.element.android.libraries.matrix.api.sync.SyncService import io.element.android.libraries.matrix.api.verification.SessionVerificationService import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.push.api.PushService -import io.element.android.libraries.pushproviders.api.RegistrationFailure +import io.element.android.libraries.push.api.PusherRegistrationFailure import io.element.android.services.analytics.api.AnalyticsService import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.combine @@ -71,7 +71,17 @@ class LoggedInPresenter( when (sessionVerifiedStatus) { SessionVerifiedStatus.Unknown -> Unit SessionVerifiedStatus.Verified -> { - ensurePusherIsRegistered(pusherRegistrationState) + Timber.tag(pusherTag.value).d("Ensure pusher is registered") + pushService.ensurePusherIsRegistered(matrixClient).fold( + onSuccess = { + Timber.tag(pusherTag.value).d("Pusher registered") + pusherRegistrationState.value = AsyncData.Success(Unit) + }, + onFailure = { + Timber.tag(pusherTag.value).e(it, "Failed to register pusher") + pusherRegistrationState.value = AsyncData.Failure(it) + }, + ) } SessionVerifiedStatus.NotVerified -> { pusherRegistrationState.value = AsyncData.Failure(PusherRegistrationFailure.AccountNotVerified()) @@ -133,59 +143,6 @@ class LoggedInPresenter( currentSlidingSyncVersion == SlidingSyncVersion.Proxy } - private suspend fun ensurePusherIsRegistered(pusherRegistrationState: MutableState>) { - Timber.tag(pusherTag.value).d("Ensure pusher is registered") - val currentPushProvider = pushService.getCurrentPushProvider(matrixClient.sessionId) - val result = if (currentPushProvider == null) { - Timber.tag(pusherTag.value).d("Register with the first available push provider with at least one distributor") - val pushProvider = pushService.getAvailablePushProviders() - .firstOrNull { it.getDistributors().isNotEmpty() } - // Else fallback to the first available push provider (the list should never be empty) - ?: pushService.getAvailablePushProviders().firstOrNull() - ?: return Unit - .also { Timber.tag(pusherTag.value).w("No push providers available") } - .also { pusherRegistrationState.value = AsyncData.Failure(PusherRegistrationFailure.NoProvidersAvailable()) } - val distributor = pushProvider.getDistributors().firstOrNull() - ?: return Unit - .also { Timber.tag(pusherTag.value).w("No distributors available") } - .also { - // In this case, consider the push provider is chosen. - pushService.selectPushProvider(matrixClient.sessionId, pushProvider) - } - .also { pusherRegistrationState.value = AsyncData.Failure(PusherRegistrationFailure.NoDistributorsAvailable()) } - pushService.registerWith(matrixClient, pushProvider, distributor) - } else { - val currentPushDistributor = currentPushProvider.getCurrentDistributor(matrixClient.sessionId) - if (currentPushDistributor == null) { - Timber.tag(pusherTag.value).d("Register with the first available distributor") - val distributor = currentPushProvider.getDistributors().firstOrNull() - ?: return Unit - .also { Timber.tag(pusherTag.value).w("No distributors available") } - .also { pusherRegistrationState.value = AsyncData.Failure(PusherRegistrationFailure.NoDistributorsAvailable()) } - pushService.registerWith(matrixClient, currentPushProvider, distributor) - } else { - Timber.tag(pusherTag.value).d("Re-register with the current distributor") - pushService.registerWith(matrixClient, currentPushProvider, currentPushDistributor) - } - } - result.fold( - onSuccess = { - Timber.tag(pusherTag.value).d("Pusher registered") - pusherRegistrationState.value = AsyncData.Success(Unit) - }, - onFailure = { - Timber.tag(pusherTag.value).e(it, "Failed to register pusher") - if (it is RegistrationFailure) { - pusherRegistrationState.value = AsyncData.Failure( - PusherRegistrationFailure.RegistrationFailure(it.clientException, it.isRegisteringAgain) - ) - } else { - pusherRegistrationState.value = AsyncData.Failure(it) - } - } - ) - } - private fun reportCryptoStatusToAnalytics(verificationState: SessionVerifiedStatus, recoveryState: RecoveryState) { // Update first the user property, to store the current status for that posthog user val userVerificationState = verificationState.toAnalyticsUserPropertyValue() diff --git a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInStateProvider.kt b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInStateProvider.kt index 9f92a19506..b2f5407519 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInStateProvider.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInStateProvider.kt @@ -10,6 +10,7 @@ package io.element.android.appnav.loggedin import androidx.compose.ui.tooling.preview.PreviewParameterProvider import io.element.android.libraries.architecture.AsyncData +import io.element.android.libraries.push.api.PusherRegistrationFailure open class LoggedInStateProvider : PreviewParameterProvider { override val values: Sequence diff --git a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInView.kt b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInView.kt index bb908ac0fd..62d8de8c29 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInView.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/loggedin/LoggedInView.kt @@ -25,6 +25,7 @@ import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight import io.element.android.libraries.designsystem.utils.OnLifecycleEvent import io.element.android.libraries.matrix.api.exception.isNetworkError +import io.element.android.libraries.push.api.PusherRegistrationFailure import io.element.android.libraries.ui.strings.CommonStrings @Composable diff --git a/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt index aca9545ae7..849dfa85b2 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/loggedin/LoggedInPresenterTest.kt @@ -34,6 +34,7 @@ import io.element.android.libraries.matrix.test.roomlist.FakeRoomListService import io.element.android.libraries.matrix.test.sync.FakeSyncService import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService import io.element.android.libraries.push.api.PushService +import io.element.android.libraries.push.api.PusherRegistrationFailure import io.element.android.libraries.push.test.FakePushService import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider @@ -42,7 +43,6 @@ import io.element.android.services.analytics.api.AnalyticsService import io.element.android.services.analytics.test.FakeAnalyticsService import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.consumeItemsUntilPredicate -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.value @@ -115,7 +115,9 @@ class LoggedInPresenterTest { encryptionService = encryptionService, ), syncService = FakeSyncService(initialSyncState = SyncState.Running), - pushService = FakePushService(), + pushService = FakePushService( + ensurePusherIsRegisteredResult = { Result.success(Unit) }, + ), sessionVerificationService = verificationService, analyticsService = analyticsService, encryptionService = encryptionService, @@ -139,10 +141,10 @@ class LoggedInPresenterTest { @Test fun `present - ensure default pusher is not registered if session is not verified`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> + val lambda = lambdaRecorder> { Result.success(Unit) } - val pushService = createFakePushService(registerWithLambda = lambda) + val pushService = createFakePushService(ensurePusherIsRegisteredResult = lambda) val verificationService = FakeSessionVerificationService( initialSessionVerifiedStatus = SessionVerifiedStatus.NotVerified ) @@ -153,21 +155,18 @@ class LoggedInPresenterTest { val finalState = awaitFirstItem() assertThat(finalState.pusherRegistrationState.errorOrNull()) .isInstanceOf(PusherRegistrationFailure.AccountNotVerified::class.java) - lambda.assertions() - .isNeverCalled() + lambda.assertions().isNeverCalled() } } @Test fun `present - ensure default pusher is registered with default provider`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> - Result.success(Unit) - } + val lambda = lambdaRecorder> { Result.success(Unit) } val sessionVerificationService = FakeSessionVerificationService( initialSessionVerifiedStatus = SessionVerifiedStatus.Verified ) val pushService = createFakePushService( - registerWithLambda = lambda, + ensurePusherIsRegisteredResult = lambda, ) createLoggedInPresenter( pushService = pushService, @@ -180,27 +179,17 @@ class LoggedInPresenterTest { assertThat(finalState.pusherRegistrationState.isSuccess()).isTrue() lambda.assertions() .isCalledOnce() - .with( - // MatrixClient - any(), - // PushProvider with highest priority (lower index) - value(pushService.getAvailablePushProviders()[0]), - // First distributor - value(pushService.getAvailablePushProviders()[0].getDistributors()[0]), - ) } } @Test fun `present - ensure default pusher is registered with default provider - fail to register`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> - Result.failure(AN_EXCEPTION) - } + val lambda = lambdaRecorder> { Result.failure(AN_EXCEPTION) } val sessionVerificationService = FakeSessionVerificationService( initialSessionVerifiedStatus = SessionVerifiedStatus.Verified ) val pushService = createFakePushService( - registerWithLambda = lambda, + ensurePusherIsRegisteredResult = lambda, ) createLoggedInPresenter( pushService = pushService, @@ -213,83 +202,24 @@ class LoggedInPresenterTest { assertThat(finalState.pusherRegistrationState.isFailure()).isTrue() lambda.assertions() .isCalledOnce() - .with( - // MatrixClient - any(), - // PushProvider with highest priority (lower index) - value(pushService.getAvailablePushProviders()[0]), - // First distributor - value(pushService.getAvailablePushProviders()[0].getDistributors()[0]), - ) - } - } - - @Test - fun `present - ensure current provider is registered with current distributor`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> - Result.success(Unit) - } - val sessionVerificationService = FakeSessionVerificationService( - initialSessionVerifiedStatus = SessionVerifiedStatus.Verified - ) - val distributor = Distributor("aDistributorValue1", "aDistributorName1") - val pushProvider = FakePushProvider( - index = 0, - name = "aFakePushProvider0", - distributors = listOf( - Distributor("aDistributorValue0", "aDistributorName0"), - distributor, - ), - currentDistributor = { distributor }, - ) - val pushService = createFakePushService( - pushProvider1 = pushProvider, - currentPushProvider = { pushProvider }, - registerWithLambda = lambda, - ) - createLoggedInPresenter( - pushService = pushService, - sessionVerificationService = sessionVerificationService, - matrixClient = FakeMatrixClient( - accountManagementUrlResult = { Result.success(null) }, - ), - ).test { - val finalState = awaitFirstItem() - assertThat(finalState.pusherRegistrationState.isSuccess()).isTrue() - lambda.assertions() - .isCalledOnce() - .with( - // MatrixClient - any(), - // Current push provider - value(pushProvider), - // Current distributor - value(distributor), - ) + // Reset the error and do not show again + finalState.eventSink(LoggedInEvents.CloseErrorDialog(doNotShowAgain = false)) + val lastState = awaitItem() + assertThat(lastState.pusherRegistrationState.isUninitialized()).isTrue() + assertThat(lastState.ignoreRegistrationError).isFalse() } } @Test - fun `present - if current push provider does not have current distributor, the first one is used`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> - Result.success(Unit) - } + fun `present - ensure default pusher is registered with default provider - fail to register - do not show again`() = runTest { + val lambda = lambdaRecorder> { Result.failure(AN_EXCEPTION) } + val setIgnoreRegistrationErrorLambda = lambdaRecorder { _, _ -> } val sessionVerificationService = FakeSessionVerificationService( initialSessionVerifiedStatus = SessionVerifiedStatus.Verified ) - val pushProvider = FakePushProvider( - index = 0, - name = "aFakePushProvider0", - distributors = listOf( - Distributor("aDistributorValue0", "aDistributorName0"), - Distributor("aDistributorValue1", "aDistributorName1"), - ), - currentDistributor = { null }, - ) val pushService = createFakePushService( - pushProvider0 = pushProvider, - currentPushProvider = { pushProvider }, - registerWithLambda = lambda, + ensurePusherIsRegisteredResult = lambda, + setIgnoreRegistrationErrorLambda = setIgnoreRegistrationErrorLambda, ) createLoggedInPresenter( pushService = pushService, @@ -299,72 +229,9 @@ class LoggedInPresenterTest { ), ).test { val finalState = awaitFirstItem() - assertThat(finalState.pusherRegistrationState.isSuccess()).isTrue() + assertThat(finalState.pusherRegistrationState.isFailure()).isTrue() lambda.assertions() .isCalledOnce() - .with( - // MatrixClient - any(), - // PushProvider with highest priority (lower index) - value(pushService.getAvailablePushProviders()[0]), - // First distributor - value(pushService.getAvailablePushProviders()[0].getDistributors()[0]), - ) - } - } - - @Test - fun `present - if current push provider does not have distributors, nothing happen`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> - Result.success(Unit) - } - val sessionVerificationService = FakeSessionVerificationService( - initialSessionVerifiedStatus = SessionVerifiedStatus.Verified - ) - val pushProvider = FakePushProvider( - index = 0, - name = "aFakePushProvider0", - distributors = emptyList(), - ) - val pushService = createFakePushService( - pushProvider0 = pushProvider, - currentPushProvider = { pushProvider }, - registerWithLambda = lambda, - ) - createLoggedInPresenter( - pushService = pushService, - sessionVerificationService = sessionVerificationService, - ).test { - val finalState = awaitFirstItem() - assertThat(finalState.pusherRegistrationState.errorOrNull()) - .isInstanceOf(PusherRegistrationFailure.NoDistributorsAvailable::class.java) - lambda.assertions() - .isNeverCalled() - } - } - - @Test - fun `present - case no push provider available provider`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> - Result.success(Unit) - } - val sessionVerificationService = FakeSessionVerificationService(SessionVerifiedStatus.Verified) - val setIgnoreRegistrationErrorLambda = lambdaRecorder { _, _ -> } - val pushService = createFakePushService( - pushProvider0 = null, - pushProvider1 = null, - registerWithLambda = lambda, - setIgnoreRegistrationErrorLambda = setIgnoreRegistrationErrorLambda, - ) - createLoggedInPresenter( - pushService = pushService, - sessionVerificationService = sessionVerificationService, - ).test { - val finalState = awaitFirstItem() - assertThat(finalState.pusherRegistrationState.errorOrNull()) - .isInstanceOf(PusherRegistrationFailure.NoProvidersAvailable::class.java) - lambda.assertions() - .isNeverCalled() // Reset the error and do not show again finalState.eventSink(LoggedInEvents.CloseErrorDialog(doNotShowAgain = true)) skipItems(1) @@ -382,95 +249,6 @@ class LoggedInPresenterTest { } } - @Test - fun `present - case one push provider but no distributor available`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> - Result.success(Unit) - } - val selectPushProviderLambda = lambdaRecorder { _, _ -> } - val sessionVerificationService = FakeSessionVerificationService( - initialSessionVerifiedStatus = SessionVerifiedStatus.Verified - ) - val pushProvider = FakePushProvider( - index = 0, - name = "aFakePushProvider", - distributors = emptyList(), - ) - val pushService = createFakePushService( - pushProvider0 = pushProvider, - pushProvider1 = null, - registerWithLambda = lambda, - selectPushProviderLambda = selectPushProviderLambda, - ) - createLoggedInPresenter( - pushService = pushService, - sessionVerificationService = sessionVerificationService, - ).test { - val finalState = awaitFirstItem() - assertThat(finalState.pusherRegistrationState.errorOrNull()) - .isInstanceOf(PusherRegistrationFailure.NoDistributorsAvailable::class.java) - lambda.assertions() - .isNeverCalled() - selectPushProviderLambda.assertions() - .isCalledOnce() - .with( - // SessionId - value(A_SESSION_ID), - // PushProvider - value(pushProvider), - ) - // Reset the error - finalState.eventSink(LoggedInEvents.CloseErrorDialog(doNotShowAgain = false)) - val lastState = awaitItem() - assertThat(lastState.pusherRegistrationState.isUninitialized()).isTrue() - } - } - - @Test - fun `present - case two push providers but first one does not have distributor - second one will be used`() = runTest { - val lambda = lambdaRecorder> { _, _, _ -> - Result.success(Unit) - } - val sessionVerificationService = FakeSessionVerificationService( - initialSessionVerifiedStatus = SessionVerifiedStatus.Verified - ) - val pushProvider0 = FakePushProvider( - index = 0, - name = "aFakePushProvider0", - distributors = emptyList(), - ) - val distributor = Distributor("aDistributorValue1", "aDistributorName1") - val pushProvider1 = FakePushProvider( - index = 1, - name = "aFakePushProvider1", - distributors = listOf(distributor), - ) - val pushService = createFakePushService( - pushProvider0 = pushProvider0, - pushProvider1 = pushProvider1, - registerWithLambda = lambda, - ) - createLoggedInPresenter( - pushService = pushService, - sessionVerificationService = sessionVerificationService, - matrixClient = FakeMatrixClient( - accountManagementUrlResult = { Result.success(null) }, - ), - ).test { - val finalState = awaitFirstItem() - assertThat(finalState.pusherRegistrationState.isSuccess()).isTrue() - lambda.assertions().isCalledOnce() - .with( - // MatrixClient - any(), - // PushProvider with the distributor - value(pushProvider1), - // First distributor of second push provider - value(distributor), - ) - } - } - private fun createFakePushService( pushProvider0: PushProvider? = FakePushProvider( index = 0, @@ -484,7 +262,7 @@ class LoggedInPresenterTest { distributors = listOf(Distributor("aDistributorValue1", "aDistributorName1")), currentDistributor = { null }, ), - registerWithLambda: (MatrixClient, PushProvider, Distributor) -> Result = { _, _, _ -> + ensurePusherIsRegisteredResult: () -> Result = { Result.success(Unit) }, selectPushProviderLambda: (SessionId, PushProvider) -> Unit = { _, _ -> lambdaError() }, @@ -493,7 +271,7 @@ class LoggedInPresenterTest { ): PushService { return FakePushService( availablePushProviders = listOfNotNull(pushProvider0, pushProvider1), - registerWithLambda = registerWithLambda, + ensurePusherIsRegisteredResult = ensurePusherIsRegisteredResult, currentPushProvider = currentPushProvider, selectPushProviderLambda = selectPushProviderLambda, setIgnoreRegistrationErrorLambda = setIgnoreRegistrationErrorLambda, diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt index 24c569cc4b..9d9e80b3f3 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenter.kt @@ -25,6 +25,7 @@ import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.runUpdatingStateNoSuccess import io.element.android.libraries.core.extensions.runCatchingExceptions +import io.element.android.libraries.di.annotations.SessionCoroutineScope import io.element.android.libraries.fullscreenintent.api.FullScreenIntentPermissionsState import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService @@ -51,6 +52,8 @@ class NotificationSettingsPresenter( private val pushService: PushService, private val systemNotificationsEnabledProvider: SystemNotificationsEnabledProvider, private val fullScreenIntentPermissionsPresenter: Presenter, + @SessionCoroutineScope + private val sessionCoroutineScope: CoroutineScope, ) : Presenter { @Composable override fun present(): NotificationSettingsState { @@ -141,7 +144,7 @@ class NotificationSettingsPresenter( is NotificationSettingsEvents.SetInviteForMeNotificationsEnabled -> { localCoroutineScope.setInviteForMeNotificationsEnabled(event.enabled, changeNotificationSettingAction) } - is NotificationSettingsEvents.SetNotificationsEnabled -> localCoroutineScope.setNotificationsEnabled(userPushStore, event.enabled) + is NotificationSettingsEvents.SetNotificationsEnabled -> sessionCoroutineScope.setNotificationsEnabled(userPushStore, event.enabled) NotificationSettingsEvents.ClearConfigurationMismatchError -> { matrixSettings.value = NotificationSettingsState.MatrixSettings.Invalid(fixFailed = false) } @@ -262,5 +265,10 @@ class NotificationSettingsPresenter( private fun CoroutineScope.setNotificationsEnabled(userPushStore: UserPushStore, enabled: Boolean) = launch { userPushStore.setNotificationEnabledForDevice(enabled) + if (enabled) { + pushService.ensurePusherIsRegistered(matrixClient) + } else { + pushService.getCurrentPushProvider(matrixClient.sessionId)?.unregister(matrixClient) + } } } diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt index 27cca104b1..5821d141cc 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt @@ -8,9 +8,6 @@ package io.element.android.features.preferences.impl.notifications -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.fullscreenintent.api.FullScreenIntentPermissionsState @@ -28,6 +25,9 @@ import io.element.android.libraries.pushproviders.test.FakePushProvider import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStoreFactory import io.element.android.tests.testutils.awaitLastSequentialItem import io.element.android.tests.testutils.consumeItemsUntilPredicate +import io.element.android.tests.testutils.lambda.lambdaRecorder +import io.element.android.tests.testutils.test +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.junit.Test import kotlin.time.Duration.Companion.milliseconds @@ -36,9 +36,7 @@ class NotificationSettingsPresenterTest { @Test fun `present - ensures initial state is correct`() = runTest { val presenter = createNotificationSettingsPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() assertThat(initialState.appSettings.appNotificationsEnabled).isFalse() assertThat(initialState.appSettings.systemNotificationsEnabled).isTrue() @@ -62,9 +60,7 @@ class NotificationSettingsPresenterTest { fun `present - default group notification mode changed`() = runTest { val notificationSettingsService = FakeNotificationSettingsService() val presenter = createNotificationSettingsPresenter(notificationSettingsService) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = true, isOneToOne = false, mode = RoomNotificationMode.ALL_MESSAGES) notificationSettingsService.setDefaultRoomNotificationMode(isEncrypted = false, isOneToOne = false, mode = RoomNotificationMode.ALL_MESSAGES) val updatedState = consumeItemsUntilPredicate { @@ -80,9 +76,7 @@ class NotificationSettingsPresenterTest { fun `present - notification settings mismatched`() = runTest { val notificationSettingsService = FakeNotificationSettingsService() val presenter = createNotificationSettingsPresenter(notificationSettingsService) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { notificationSettingsService.setDefaultRoomNotificationMode( isEncrypted = true, isOneToOne = false, @@ -110,9 +104,7 @@ class NotificationSettingsPresenterTest { initialOneToOneDefaultMode = RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY ) val presenter = createNotificationSettingsPresenter(notificationSettingsService) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(NotificationSettingsEvents.FixConfigurationMismatch) val fixedState = consumeItemsUntilPredicate(timeout = 2000.milliseconds) { @@ -125,10 +117,19 @@ class NotificationSettingsPresenterTest { @Test fun `present - set notifications enabled`() = runTest { - val presenter = createNotificationSettingsPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val unregisterWithResult = lambdaRecorder> { Result.success(Unit) } + val ensurePusherIsRegisteredResult = lambdaRecorder> { Result.success(Unit) } + val presenter = createNotificationSettingsPresenter( + pushService = FakePushService( + currentPushProvider = { + FakePushProvider( + unregisterWithResult = unregisterWithResult, + ) + }, + ensurePusherIsRegisteredResult = ensurePusherIsRegisteredResult, + ) + ) + presenter.test { val loadedState = consumeItemsUntilPredicate { it.matrixSettings is NotificationSettingsState.MatrixSettings.Valid }.last() @@ -138,16 +139,21 @@ class NotificationSettingsPresenterTest { !it.appSettings.appNotificationsEnabled }.last() assertThat(updatedState.appSettings.appNotificationsEnabled).isFalse() - cancelAndIgnoreRemainingEvents() + unregisterWithResult.assertions().isCalledOnce() + // Enable notification again + loadedState.eventSink(NotificationSettingsEvents.SetNotificationsEnabled(true)) + val updatedState2 = consumeItemsUntilPredicate { + it.appSettings.appNotificationsEnabled + }.last() + assertThat(updatedState2.appSettings.appNotificationsEnabled).isTrue() + ensurePusherIsRegisteredResult.assertions().isCalledOnce() } } @Test fun `present - set call notifications enabled`() = runTest { val presenter = createNotificationSettingsPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val loadedState = consumeItemsUntilPredicate { (it.matrixSettings as? NotificationSettingsState.MatrixSettings.Valid)?.callNotificationsEnabled == false }.last() @@ -166,9 +172,7 @@ class NotificationSettingsPresenterTest { @Test fun `present - set invite for me notifications enabled`() = runTest { val presenter = createNotificationSettingsPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val loadedState = consumeItemsUntilPredicate { (it.matrixSettings as? NotificationSettingsState.MatrixSettings.Valid)?.inviteForMeNotificationsEnabled == false }.last() @@ -187,9 +191,7 @@ class NotificationSettingsPresenterTest { @Test fun `present - set atRoom notifications enabled`() = runTest { val presenter = createNotificationSettingsPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val loadedState = consumeItemsUntilPredicate { (it.matrixSettings as? NotificationSettingsState.MatrixSettings.Valid)?.atRoomNotificationsEnabled == false }.last() @@ -210,9 +212,7 @@ class NotificationSettingsPresenterTest { val notificationSettingsService = FakeNotificationSettingsService() val presenter = createNotificationSettingsPresenter(notificationSettingsService) notificationSettingsService.givenSetAtRoomError(AN_EXCEPTION) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val loadedState = consumeItemsUntilPredicate { (it.matrixSettings as? NotificationSettingsState.MatrixSettings.Valid)?.atRoomNotificationsEnabled == false }.last() @@ -237,9 +237,7 @@ class NotificationSettingsPresenterTest { val presenter = createNotificationSettingsPresenter( pushService = createFakePushService(), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitLastSequentialItem() assertThat(initialState.currentPushDistributor).isEqualTo(AsyncData.Success(Distributor(value = "aDistributorValue0", name = "aDistributorName0"))) assertThat(initialState.availablePushDistributors).containsExactly( @@ -271,9 +269,7 @@ class NotificationSettingsPresenterTest { val presenter = createNotificationSettingsPresenter( pushService = createFakePushService(), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitLastSequentialItem() assertThat(initialState.currentPushDistributor).isEqualTo(AsyncData.Success(Distributor(value = "aDistributorValue0", name = "aDistributorName0"))) assertThat(initialState.availablePushDistributors).containsExactly( @@ -298,9 +294,7 @@ class NotificationSettingsPresenterTest { pushService = createFakePushService(), fullScreenIntentPermissionsStateLambda = fullScreenIntentPermissionsStateLambda, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitLastSequentialItem() assertThat(initialState.fullScreenIntentPermissionsState.permissionGranted).isFalse() @@ -324,11 +318,7 @@ class NotificationSettingsPresenterTest { }, ), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { - val initialState = awaitLastSequentialItem() - initialState.eventSink.invoke(NotificationSettingsEvents.ChangePushProvider) + presenter.test { val withDialog = awaitItem() assertThat(withDialog.showChangePushProviderDialog).isTrue() withDialog.eventSink(NotificationSettingsEvents.SetPushProvider(1)) @@ -361,7 +351,7 @@ class NotificationSettingsPresenterTest { ) } - private fun createNotificationSettingsPresenter( + private fun TestScope.createNotificationSettingsPresenter( notificationSettingsService: FakeNotificationSettingsService = FakeNotificationSettingsService(), pushService: PushService = FakePushService(), fullScreenIntentPermissionsStateLambda: () -> FullScreenIntentPermissionsState = { aFullScreenIntentPermissionsState() }, @@ -374,6 +364,7 @@ class NotificationSettingsPresenterTest { pushService = pushService, systemNotificationsEnabledProvider = FakeSystemNotificationsEnabledProvider(), fullScreenIntentPermissionsPresenter = { fullScreenIntentPermissionsStateLambda() }, + sessionCoroutineScope = backgroundScope, ) } } diff --git a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt index 2096a04656..a43ee36403 100644 --- a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt +++ b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PushService.kt @@ -38,6 +38,15 @@ interface PushService { distributor: Distributor, ): Result + /** + * Ensure that the pusher with the current push provider and distributor is registered. + * If there is no current config, the default push provider with the default distributor will be used. + * Error can be [PusherRegistrationFailure]. + */ + suspend fun ensurePusherIsRegistered( + matrixClient: MatrixClient, + ): Result + /** * Store the given push provider as the current one, but do not register. * To be used when there is no distributor available. diff --git a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/PusherRegistrationFailure.kt b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PusherRegistrationFailure.kt similarity index 95% rename from appnav/src/main/kotlin/io/element/android/appnav/loggedin/PusherRegistrationFailure.kt rename to libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PusherRegistrationFailure.kt index 4b386cd46a..b8ae677aab 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/loggedin/PusherRegistrationFailure.kt +++ b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/PusherRegistrationFailure.kt @@ -6,7 +6,7 @@ * Please see LICENSE files in the repository root for full details. */ -package io.element.android.appnav.loggedin +package io.element.android.libraries.push.api import io.element.android.libraries.matrix.api.exception.ClientException diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt index 5c77438293..3f3a8d05b4 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt @@ -15,8 +15,10 @@ import dev.zacsweers.metro.binding import io.element.android.libraries.matrix.api.MatrixClient 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.verification.SessionVerifiedStatus import io.element.android.libraries.push.api.GetCurrentPushProvider import io.element.android.libraries.push.api.PushService +import io.element.android.libraries.push.api.PusherRegistrationFailure import io.element.android.libraries.push.api.history.PushHistoryItem import io.element.android.libraries.push.impl.push.MutableBatteryOptimizationStore import io.element.android.libraries.push.impl.store.PushDataStore @@ -24,11 +26,13 @@ import io.element.android.libraries.push.impl.test.TestPush import io.element.android.libraries.push.impl.unregistration.ServiceUnregisteredHandler import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider +import io.element.android.libraries.pushproviders.api.RegistrationFailure import io.element.android.libraries.pushstore.api.UserPushStoreFactory import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretStore import io.element.android.libraries.sessionstorage.api.observer.SessionListener import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first import timber.log.Timber @ContributesBinding(AppScope::class, binding = binding()) @@ -84,6 +88,59 @@ class DefaultPushService( return pushProvider.registerWith(matrixClient, distributor) } + override suspend fun ensurePusherIsRegistered(matrixClient: MatrixClient): Result { + val verificationStatus = matrixClient.sessionVerificationService.sessionVerifiedStatus.first() + if (verificationStatus != SessionVerifiedStatus.Verified) { + return Result.failure(PusherRegistrationFailure.AccountNotVerified()) + .also { Timber.w("Account is not verified") } + } + Timber.d("Ensure pusher is registered") + val currentPushProvider = getCurrentPushProvider(matrixClient.sessionId) + val result = if (currentPushProvider == null) { + Timber.d("Register with the first available push provider with at least one distributor") + val pushProvider = getAvailablePushProviders() + .firstOrNull { it.getDistributors().isNotEmpty() } + // Else fallback to the first available push provider (the list should never be empty) + ?: getAvailablePushProviders().firstOrNull() + ?: return Result.failure(PusherRegistrationFailure.NoProvidersAvailable()) + .also { Timber.w("No push providers available") } + val distributor = pushProvider.getDistributors().firstOrNull() + ?: return Result.failure(PusherRegistrationFailure.NoDistributorsAvailable()) + .also { Timber.w("No distributors available") } + .also { + // In this case, consider the push provider is chosen. + selectPushProvider(matrixClient.sessionId, pushProvider) + } + registerWith(matrixClient, pushProvider, distributor) + } else { + val currentPushDistributor = currentPushProvider.getCurrentDistributor(matrixClient.sessionId) + if (currentPushDistributor == null) { + Timber.d("Register with the first available distributor") + val distributor = currentPushProvider.getDistributors().firstOrNull() + ?: return Result.failure(PusherRegistrationFailure.NoDistributorsAvailable()) + .also { Timber.w("No distributors available") } + registerWith(matrixClient, currentPushProvider, distributor) + } else { + Timber.d("Re-register with the current distributor") + registerWith(matrixClient, currentPushProvider, currentPushDistributor) + } + } + return result.fold( + onSuccess = { + Timber.d("Pusher registered") + Result.success(Unit) + }, + onFailure = { + Timber.e(it, "Failed to register pusher") + if (it is RegistrationFailure) { + Result.failure(PusherRegistrationFailure.RegistrationFailure(it.clientException, it.isRegisteringAgain)) + } else { + Result.failure(it) + } + } + ) + } + override suspend fun selectPushProvider( sessionId: SessionId, pushProvider: PushProvider, diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt index 7ddbc93fb1..f48b7ee612 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt @@ -12,12 +12,15 @@ import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.SessionId +import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.AN_EXCEPTION 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.FakeMatrixClient +import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService import io.element.android.libraries.push.api.GetCurrentPushProvider +import io.element.android.libraries.push.api.PusherRegistrationFailure import io.element.android.libraries.push.api.history.PushHistoryItem import io.element.android.libraries.push.impl.push.FakeMutableBatteryOptimizationStore import io.element.android.libraries.push.impl.push.MutableBatteryOptimizationStore @@ -40,6 +43,7 @@ import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushSto import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.InMemoryPushClientSecretStore import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver +import io.element.android.tests.testutils.lambda.any import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import kotlinx.coroutines.flow.first @@ -339,6 +343,281 @@ class DefaultPushServiceTest { } } + @Test + fun `ensurePusher - error when account is not verified`() = runTest { + val sessionVerificationService = FakeSessionVerificationService( + initialSessionVerifiedStatus = SessionVerifiedStatus.NotVerified + ) + val pushService = createDefaultPushService() + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.exceptionOrNull()!!).isInstanceOf(PusherRegistrationFailure.AccountNotVerified::class.java) + } + + @Test + fun `ensurePusher - case two push providers but first one does not have distributor - second one will be used`() = runTest { + val lambda = lambdaRecorder> { _, _ -> + Result.success(Unit) + } + val sessionVerificationService = FakeSessionVerificationService( + initialSessionVerifiedStatus = SessionVerifiedStatus.Verified + ) + val pushProvider0 = FakePushProvider( + index = 0, + name = "aFakePushProvider0", + distributors = emptyList(), + ) + val distributor = Distributor("aDistributorValue1", "aDistributorName1") + val pushProvider1 = FakePushProvider( + index = 1, + name = "aFakePushProvider1", + distributors = listOf(distributor), + registerWithResult = lambda, + ) + val pushService = createDefaultPushService( + pushProviders = setOf( + pushProvider0, + pushProvider1, + ), + ) + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.isSuccess).isTrue() + lambda.assertions().isCalledOnce() + .with( + // MatrixClient + any(), + // First distributor of second push provider + value(distributor), + ) + } + + @Test + fun `ensurePusher - case one push provider but no distributor available`() = runTest { + val lambda = lambdaRecorder> { _, _ -> + Result.success(Unit) + } + val sessionVerificationService = FakeSessionVerificationService( + initialSessionVerifiedStatus = SessionVerifiedStatus.Verified + ) + val pushProvider = FakePushProvider( + index = 0, + name = "aFakePushProvider", + distributors = emptyList(), + registerWithResult = lambda, + ) + val pushService = createDefaultPushService( + pushProviders = setOf(pushProvider), + ) + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.exceptionOrNull()).isInstanceOf(PusherRegistrationFailure.NoDistributorsAvailable::class.java) + lambda.assertions().isNeverCalled() + } + + @Test + fun `ensurePusher - ensure default pusher is registered with default provider`() = runTest { + val lambda = lambdaRecorder> { _, _ -> + Result.success(Unit) + } + val sessionVerificationService = FakeSessionVerificationService( + initialSessionVerifiedStatus = SessionVerifiedStatus.Verified + ) + val pushService = createDefaultPushService( + pushProviders = setOf( + FakePushProvider( + index = 0, + name = "aFakePushProvider", + distributors = listOf(Distributor("aDistributorValue0", "aDistributorName0")), + registerWithResult = lambda, + ) + ), + ) + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.isSuccess).isTrue() + lambda.assertions() + .isCalledOnce() + .with( + // MatrixClient + any(), + // First distributor + value(pushService.getAvailablePushProviders()[0].getDistributors()[0]), + ) + } + + @Test + fun `ensurePusher - ensure default pusher is registered with default provider - fail to register`() = runTest { + val lambda = lambdaRecorder> { _, _ -> + Result.failure(AN_EXCEPTION) + } + val sessionVerificationService = FakeSessionVerificationService( + initialSessionVerifiedStatus = SessionVerifiedStatus.Verified + ) + val pushService = createDefaultPushService( + pushProviders = setOf( + FakePushProvider( + index = 0, + name = "aFakePushProvider", + distributors = listOf(Distributor("aDistributorValue0", "aDistributorName0")), + registerWithResult = lambda, + ) + ), + ) + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.isFailure).isTrue() + lambda.assertions() + .isCalledOnce() + .with( + // MatrixClient + any(), + // First distributor + value(pushService.getAvailablePushProviders()[0].getDistributors()[0]), + ) + } + + @Test + fun `ensurePusher - if current push provider does not have distributors, nothing happen`() = runTest { + val lambda = lambdaRecorder> { _, _ -> + Result.success(Unit) + } + val sessionVerificationService = FakeSessionVerificationService( + initialSessionVerifiedStatus = SessionVerifiedStatus.Verified + ) + val pushProvider = FakePushProvider( + index = 0, + name = "aFakePushProvider0", + distributors = emptyList(), + registerWithResult = lambda, + ) + val pushService = createDefaultPushService( + pushProviders = setOf(pushProvider), + getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = pushProvider.name), + ) + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.exceptionOrNull()) + .isInstanceOf(PusherRegistrationFailure.NoDistributorsAvailable::class.java) + lambda.assertions() + .isNeverCalled() + } + + @Test + fun `ensurePusher - ensure current provider is registered with current distributor`() = runTest { + val lambda = lambdaRecorder> { _, _ -> + Result.success(Unit) + } + val sessionVerificationService = FakeSessionVerificationService( + initialSessionVerifiedStatus = SessionVerifiedStatus.Verified + ) + val distributor = Distributor("aDistributorValue1", "aDistributorName1") + val pushProvider = FakePushProvider( + index = 0, + name = "aFakePushProvider0", + distributors = listOf( + Distributor("aDistributorValue0", "aDistributorName0"), + distributor, + ), + currentDistributor = { distributor }, + registerWithResult = lambda, + ) + val pushService = createDefaultPushService( + pushProviders = setOf(pushProvider), + getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = pushProvider.name), + ) + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.isSuccess).isTrue() + lambda.assertions() + .isCalledOnce() + .with( + // MatrixClient + any(), + // Current distributor + value(distributor), + ) + } + + @Test + fun `ensurePusher - case no push provider available provider`() = runTest { + val lambda = lambdaRecorder> { _, _ -> + Result.success(Unit) + } + val sessionVerificationService = FakeSessionVerificationService(SessionVerifiedStatus.Verified) + val pushService = createDefaultPushService( + pushProviders = emptySet(), + ) + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.exceptionOrNull()) + .isInstanceOf(PusherRegistrationFailure.NoProvidersAvailable::class.java) + lambda.assertions() + .isNeverCalled() + } + + @Test + fun `ensurePusher - if current push provider does not have current distributor, the first one is used`() = runTest { + val lambda = lambdaRecorder> { _, _ -> + Result.success(Unit) + } + val sessionVerificationService = FakeSessionVerificationService( + initialSessionVerifiedStatus = SessionVerifiedStatus.Verified + ) + val pushProvider = FakePushProvider( + index = 0, + name = "aFakePushProvider0", + distributors = listOf( + Distributor("aDistributorValue0", "aDistributorName0"), + Distributor("aDistributorValue1", "aDistributorName1"), + ), + currentDistributor = { null }, + registerWithResult = lambda, + ) + val pushService = createDefaultPushService( + pushProviders = setOf(pushProvider), + getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = pushProvider.name), + ) + val result = pushService.ensurePusherIsRegistered( + FakeMatrixClient( + sessionVerificationService = sessionVerificationService, + ) + ) + assertThat(result.isSuccess).isTrue() + lambda.assertions() + .isCalledOnce() + .with( + // MatrixClient + any(), + // First distributor + value(pushService.getAvailablePushProviders()[0].getDistributors()[0]), + ) + } + private fun createDefaultPushService( testPush: TestPush = FakeTestPush(), userPushStoreFactory: UserPushStoreFactory = FakeUserPushStoreFactory(), diff --git a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt b/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt index ee99ef1a20..d39c5eb274 100644 --- a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt +++ b/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt @@ -32,6 +32,7 @@ class FakePushService( private val resetPushHistoryResult: () -> Unit = { lambdaError() }, private val resetBatteryOptimizationStateResult: () -> Unit = { lambdaError() }, private val onServiceUnregisteredResult: (UserId) -> Unit = { lambdaError() }, + private val ensurePusherIsRegisteredResult: () -> Result = { lambdaError() }, ) : PushService { override suspend fun getCurrentPushProvider(sessionId: SessionId): PushProvider? { return registeredPushProvider ?: currentPushProvider(sessionId) @@ -56,6 +57,10 @@ class FakePushService( } } + override suspend fun ensurePusherIsRegistered(matrixClient: MatrixClient): Result { + return ensurePusherIsRegisteredResult() + } + override suspend fun selectPushProvider(sessionId: SessionId, pushProvider: PushProvider) { selectPushProviderLambda(sessionId, pushProvider) } From 358731def3d3489f2fbdcb1ebf535a9e1669398d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 18:06:27 +0100 Subject: [PATCH 11/20] Update comment --- .../unifiedpush/UnifiedPushRemovedGatewayHandler.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt index fa75fd101e..f8ddb8385a 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt @@ -19,7 +19,7 @@ import timber.log.Timber private val loggerTag = LoggerTag("UnifiedPushRemovedGatewayHandler", LoggerTag.PushLoggerTag) /** - * Handle new endpoint received from UnifiedPush. Will update the session matching the client secret. + * Handle endpoint removal received from UnifiedPush. Will try to register again. */ fun interface UnifiedPushRemovedGatewayHandler { suspend fun handle(clientSecret: String): Result From 9498e6f7fb0c531dfe7ec0d85d830dcb7ecc6f88 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 18:11:39 +0100 Subject: [PATCH 12/20] Restore deleted code --- .../impl/notifications/NotificationSettingsPresenterTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt index 5821d141cc..33eddc3926 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt @@ -319,6 +319,8 @@ class NotificationSettingsPresenterTest { ), ) presenter.test { + val initialState = awaitLastSequentialItem() + initialState.eventSink.invoke(NotificationSettingsEvents.ChangePushProvider) val withDialog = awaitItem() assertThat(withDialog.showChangePushProviderDialog).isTrue() withDialog.eventSink(NotificationSettingsEvents.SetPushProvider(1)) From f1e12c1c1cb284fa8c47c72e8c0c6f5928b6a1ce Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 13 Nov 2025 23:12:57 +0100 Subject: [PATCH 13/20] Do not attempt to restore the pusher after 2 removal in a short time. --- .../NotificationSettingsPresenterTest.kt | 2 +- .../libraries/push/test/FakePushService.kt | 2 +- .../UnifiedPushRemovedGatewayHandler.kt | 45 +++++++-- ...ultUnifiedPushRemovedGatewayHandlerTest.kt | 98 ++++++++++++++++++- 4 files changed, 136 insertions(+), 11 deletions(-) diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt index 33eddc3926..9b36c477a4 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/notifications/NotificationSettingsPresenterTest.kt @@ -333,7 +333,7 @@ class NotificationSettingsPresenterTest { } private fun createFakePushService( - registerWithLambda: suspend (MatrixClient, PushProvider, Distributor) -> Result = { _, _, _ -> + registerWithLambda: (MatrixClient, PushProvider, Distributor) -> Result = { _, _, _ -> Result.success(Unit) } ): PushService { diff --git a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt b/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt index d39c5eb274..604ff88728 100644 --- a/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt +++ b/libraries/push/test/src/main/kotlin/io/element/android/libraries/push/test/FakePushService.kt @@ -23,7 +23,7 @@ import kotlinx.coroutines.flow.MutableStateFlow class FakePushService( private val testPushBlock: suspend (SessionId) -> Boolean = { true }, private val availablePushProviders: List = emptyList(), - private val registerWithLambda: suspend (MatrixClient, PushProvider, Distributor) -> Result = { _, _, _ -> + private val registerWithLambda: (MatrixClient, PushProvider, Distributor) -> Result = { _, _, _ -> Result.success(Unit) }, private val currentPushProvider: (SessionId) -> PushProvider? = { availablePushProviders.firstOrNull() }, diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt index f8ddb8385a..2b30209245 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt @@ -9,11 +9,16 @@ package io.element.android.libraries.pushproviders.unifiedpush import dev.zacsweers.metro.AppScope import dev.zacsweers.metro.ContributesBinding +import dev.zacsweers.metro.Inject +import dev.zacsweers.metro.SingleIn +import io.element.android.libraries.androidutils.throttler.FirstThrottler import io.element.android.libraries.core.extensions.flatMap import io.element.android.libraries.core.log.logger.LoggerTag +import io.element.android.libraries.di.annotations.AppCoroutineScope import io.element.android.libraries.matrix.api.MatrixClientProvider import io.element.android.libraries.push.api.PushService import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret +import kotlinx.coroutines.CoroutineScope import timber.log.Timber private val loggerTag = LoggerTag("UnifiedPushRemovedGatewayHandler", LoggerTag.PushLoggerTag) @@ -25,12 +30,29 @@ fun interface UnifiedPushRemovedGatewayHandler { suspend fun handle(clientSecret: String): Result } +@Inject +@SingleIn(AppScope::class) +class UnifiedPushRemovedGatewayThrottler( + @AppCoroutineScope + private val appCoroutineScope: CoroutineScope, +) { + private val firstThrottler = FirstThrottler( + minimumInterval = 60_000, + coroutineScope = appCoroutineScope, + ) + + fun canRegisterAgain(): Boolean { + return firstThrottler.canHandle() + } +} + @ContributesBinding(AppScope::class) class DefaultUnifiedPushRemovedGatewayHandler( private val unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase, private val pushClientSecret: PushClientSecret, private val matrixClientProvider: MatrixClientProvider, private val pushService: PushService, + private val unifiedPushRemovedGatewayThrottler: UnifiedPushRemovedGatewayThrottler, ) : UnifiedPushRemovedGatewayHandler { override suspend fun handle(clientSecret: String): Result { // Unregister the pusher for the session with this client secret. @@ -58,14 +80,21 @@ class DefaultUnifiedPushRemovedGatewayHandler( val distributor = pushProvider?.getCurrentDistributor(userId) // Attempt to register again if (pushProvider != null && distributor != null) { - pushService.registerWith( - client, - pushProvider, - distributor, - ) - .onFailure { - Timber.tag(loggerTag.value).w(it, "Unable to register with current data") - } + if (unifiedPushRemovedGatewayThrottler.canRegisterAgain()) { + pushService.registerWith( + client, + pushProvider, + distributor, + ) + .onFailure { + Timber.tag(loggerTag.value).w(it, "Unable to register with current data") + } + } else { + // Let the user know + Timber.tag(loggerTag.value).w("Second removal in less than 1 minute, do not register again") + pushService.onServiceUnregistered(userId) + Result.success(Unit) + } } else { Result.failure(IllegalStateException("Unable to register again")) } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt index 04c2bf0323..452013e18d 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt @@ -19,14 +19,19 @@ import io.element.android.libraries.matrix.test.FakeMatrixClientProvider import io.element.android.libraries.push.api.PushService import io.element.android.libraries.push.test.FakePushService import io.element.android.libraries.pushproviders.api.Distributor +import io.element.android.libraries.pushproviders.api.PushProvider import io.element.android.libraries.pushproviders.test.FakePushProvider import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.FakePushClientSecret import io.element.android.tests.testutils.lambda.any 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 kotlin.time.Duration.Companion.seconds class DefaultUnifiedPushRemovedGatewayHandlerTest { @Test @@ -188,7 +193,95 @@ class DefaultUnifiedPushRemovedGatewayHandlerTest { onServiceUnregisteredResult.assertions().isNeverCalled() } - private fun createDefaultUnifiedPushRemovedGatewayHandler( + @Test + fun `handle returns success if can register again, but after 2 removals user is notified`() = runTest { + val onServiceUnregisteredResult = lambdaRecorder { } + val unregisterLambda = lambdaRecorder> { _, _, _ -> Result.success(Unit) } + val registerWithLambda = lambdaRecorder> { _, _, _ -> Result.success(Unit) } + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_SESSION_ID }, + ), + matrixClientProvider = FakeMatrixClientProvider( + getClient = { Result.success(FakeMatrixClient()) }, + ), + unregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( + unregisterLambda = unregisterLambda, + ), + pushService = FakePushService( + currentPushProvider = { + FakePushProvider( + currentDistributor = { Distributor("aValue", "aName") }, + ) + }, + registerWithLambda = registerWithLambda, + onServiceUnregisteredResult = onServiceUnregisteredResult, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isSuccess).isTrue() + unregisterLambda.assertions().isCalledOnce().with( + any(), + value(A_SECRET), + value(false), + ) + registerWithLambda.assertions().isCalledOnce() + onServiceUnregisteredResult.assertions().isNeverCalled() + // Second attempt in less than 1 minute + val result2 = sut.handle(A_SECRET) + assertThat(result2.isSuccess).isTrue() + unregisterLambda.assertions().isCalledExactly(2) + // Registration is not called twice + registerWithLambda.assertions().isCalledOnce() + onServiceUnregisteredResult.assertions().isCalledOnce().with(value(A_SESSION_ID)) + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun `handle returns success if can register again, but after 2 distant removals user is not notified`() = runTest { + val onServiceUnregisteredResult = lambdaRecorder { } + val unregisterLambda = lambdaRecorder> { _, _, _ -> Result.success(Unit) } + val registerWithLambda = lambdaRecorder> { _, _, _ -> Result.success(Unit) } + val sut = createDefaultUnifiedPushRemovedGatewayHandler( + pushClientSecret = FakePushClientSecret( + getUserIdFromSecretResult = { A_SESSION_ID }, + ), + matrixClientProvider = FakeMatrixClientProvider( + getClient = { Result.success(FakeMatrixClient()) }, + ), + unregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( + unregisterLambda = unregisterLambda, + ), + pushService = FakePushService( + currentPushProvider = { + FakePushProvider( + currentDistributor = { Distributor("aValue", "aName") }, + ) + }, + registerWithLambda = registerWithLambda, + onServiceUnregisteredResult = onServiceUnregisteredResult, + ), + ) + val result = sut.handle(A_SECRET) + assertThat(result.isSuccess).isTrue() + unregisterLambda.assertions().isCalledOnce().with( + any(), + value(A_SECRET), + value(false), + ) + registerWithLambda.assertions().isCalledOnce() + onServiceUnregisteredResult.assertions().isNeverCalled() + // Second attempt in more than 1 minute + advanceTimeBy(61.seconds) + val result2 = sut.handle(A_SECRET) + assertThat(result2.isSuccess).isTrue() + unregisterLambda.assertions().isCalledExactly(2) + // Registration is not called twice + registerWithLambda.assertions().isCalledExactly(2) + onServiceUnregisteredResult.assertions().isNeverCalled() + } + + private fun TestScope.createDefaultUnifiedPushRemovedGatewayHandler( unregisterUnifiedPushUseCase: UnregisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase(), pushClientSecret: PushClientSecret = FakePushClientSecret(), matrixClientProvider: MatrixClientProvider = FakeMatrixClientProvider(), @@ -198,5 +291,8 @@ class DefaultUnifiedPushRemovedGatewayHandlerTest { pushClientSecret = pushClientSecret, matrixClientProvider = matrixClientProvider, pushService = pushService, + unifiedPushRemovedGatewayThrottler = UnifiedPushRemovedGatewayThrottler( + appCoroutineScope = backgroundScope, + ), ) } From 1c8b637858942eb659e4c2438d3e9faa0671b119 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Nov 2025 09:24:41 +0100 Subject: [PATCH 14/20] No need to use apply setContentText accept null parameter. --- .../impl/notifications/RingingCallNotificationCreator.kt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/notifications/RingingCallNotificationCreator.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/notifications/RingingCallNotificationCreator.kt index 88ddddf0cf..e7d270ef8e 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/notifications/RingingCallNotificationCreator.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/notifications/RingingCallNotificationCreator.kt @@ -133,12 +133,8 @@ class RingingCallNotificationCreator( .setWhen(timestamp) .setOngoing(true) .setShowWhen(false) - .apply { - if (textContent != null) { - setContentText(textContent) - // Else the content text is set by the style (will be "Incoming call") - } - } + // If textContent is null, the content text is set by the style (will be "Incoming call") + .setContentText(textContent) .setSound(Settings.System.DEFAULT_RINGTONE_URI, AudioManager.STREAM_RING) .setTimeoutAfter(ElementCallConfig.RINGING_CALL_DURATION_SECONDS.seconds.inWholeMilliseconds) .setContentIntent(answerIntent) From b64a46738e3840fce9e3139ca60b9ab67176154d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Nov 2025 09:33:40 +0100 Subject: [PATCH 15/20] Small cleanup --- .../features/call/impl/services/CallForegroundService.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/services/CallForegroundService.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/services/CallForegroundService.kt index 1e5b338549..89c9fdb52c 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/services/CallForegroundService.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/services/CallForegroundService.kt @@ -22,7 +22,6 @@ import androidx.core.app.NotificationManagerCompat import androidx.core.app.PendingIntentCompat import androidx.core.app.ServiceCompat import androidx.core.content.ContextCompat -import androidx.core.graphics.drawable.IconCompat import io.element.android.features.call.impl.R import io.element.android.features.call.impl.ui.ElementCallActivity import io.element.android.libraries.core.extensions.runCatchingExceptions @@ -69,7 +68,7 @@ class CallForegroundService : Service() { val callActivityIntent = Intent(this, ElementCallActivity::class.java) val pendingIntent = PendingIntentCompat.getActivity(this, 0, callActivityIntent, 0, false) val notification = NotificationCompat.Builder(this, foregroundServiceChannel.id) - .setSmallIcon(IconCompat.createWithResource(this, CommonDrawables.ic_notification)) + .setSmallIcon(CommonDrawables.ic_notification) .setContentTitle(getString(R.string.call_foreground_service_title_android)) .setContentText(getString(R.string.call_foreground_service_message_android)) .setContentIntent(pendingIntent) From 6f13feed130557f645fa19fc28e79bf332dc0ab2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Nov 2025 09:38:31 +0100 Subject: [PATCH 16/20] No need for an id, can use ordinal. --- .../push/api/notifications/NotificationIdProvider.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/notifications/NotificationIdProvider.kt b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/notifications/NotificationIdProvider.kt index ab0dfb0425..ff7119b647 100644 --- a/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/notifications/NotificationIdProvider.kt +++ b/libraries/push/api/src/main/kotlin/io/element/android/libraries/push/api/notifications/NotificationIdProvider.kt @@ -33,7 +33,7 @@ object NotificationIdProvider { } fun getForegroundServiceNotificationId(type: ForegroundServiceType): Int { - return type.id * 10 + FOREGROUND_SERVICE_NOTIFICATION_ID + return type.ordinal * 10 + FOREGROUND_SERVICE_NOTIFICATION_ID } private fun getOffset(sessionId: SessionId): Int { @@ -50,7 +50,7 @@ object NotificationIdProvider { private const val FOREGROUND_SERVICE_NOTIFICATION_ID = 4 } -enum class ForegroundServiceType(val id: Int) { - INCOMING_CALL(1), - ONGOING_CALL(2), +enum class ForegroundServiceType { + INCOMING_CALL, + ONGOING_CALL, } From 8a53be3b3320552d9a49b88fa692a6200fb7f7b9 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Nov 2025 09:39:55 +0100 Subject: [PATCH 17/20] Fix warning --- .../android/features/call/impl/utils/ActiveCallManager.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/ActiveCallManager.kt b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/ActiveCallManager.kt index 70c2c38e00..91fcc2593e 100644 --- a/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/ActiveCallManager.kt +++ b/features/call/impl/src/main/kotlin/io/element/android/features/call/impl/utils/ActiveCallManager.kt @@ -100,7 +100,7 @@ class DefaultActiveCallManager( private val imageLoaderHolder: ImageLoaderHolder, private val systemClock: SystemClock, ) : ActiveCallManager { - private val tag = "DefaultActiveCallManager" + private val tag = "ActiveCallManager" private var timedOutCallJob: Job? = null @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) From dbb9bb13387db260ee31047bbc447bcf6e63f0d2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Nov 2025 14:23:29 +0100 Subject: [PATCH 18/20] Update KDoc --- .../pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt index 3157f1492f..2e0d6ea413 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt @@ -19,7 +19,7 @@ import timber.log.Timber interface UnregisterUnifiedPushUseCase { /** - * Unregister the app from the homeserver, then from UnifiedPush. + * Unregister the app from the homeserver, then from UnifiedPush if [unregisterUnifiedPush] is true. */ suspend fun unregister( matrixClient: MatrixClient, From 1d7d00e4bb554cb794f11fba85a439e2dfd15836 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Nov 2025 15:10:24 +0100 Subject: [PATCH 19/20] Improve code readability and documentation. --- .../UnifiedPushRemovedGatewayHandler.kt | 91 +++++++++++-------- ...ultUnifiedPushRemovedGatewayHandlerTest.kt | 2 +- 2 files changed, 54 insertions(+), 39 deletions(-) diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt index 2b30209245..7baaaab1bd 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushRemovedGatewayHandler.kt @@ -15,6 +15,7 @@ import io.element.android.libraries.androidutils.throttler.FirstThrottler import io.element.android.libraries.core.extensions.flatMap import io.element.android.libraries.core.log.logger.LoggerTag import io.element.android.libraries.di.annotations.AppCoroutineScope +import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.MatrixClientProvider import io.element.android.libraries.push.api.PushService import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret @@ -54,58 +55,72 @@ class DefaultUnifiedPushRemovedGatewayHandler( private val pushService: PushService, private val unifiedPushRemovedGatewayThrottler: UnifiedPushRemovedGatewayThrottler, ) : UnifiedPushRemovedGatewayHandler { + /** + * The application has been informed by the UnifiedPush distributor that the topic has been deleted. + * So this code aim to unregister the pusher from the homeserver, register a new topic on the + * UnifiedPush application then register a new pusher to the homeserver. + * No registration will happen if the topic deletion has already occurred in the last minute. + */ override suspend fun handle(clientSecret: String): Result { - // Unregister the pusher for the session with this client secret. - val userId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure( + val sessionId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure( IllegalStateException("Unable to retrieve session") ).also { Timber.tag(loggerTag.value).w("Unable to retrieve session") } return matrixClientProvider - .getOrRestore(userId) + .getOrRestore(sessionId) .onFailure { + // Silently ignore this error (do not invoke onServiceUnregistered) Timber.tag(loggerTag.value).w(it, "Fails to restore client") } .flatMap { client -> - unregisterUnifiedPushUseCase.unregister( - matrixClient = client, - clientSecret = clientSecret, - unregisterUnifiedPush = false, - ) - .onFailure { - Timber.tag(loggerTag.value).w(it, "Unable to unregister pusher") - } - .flatMap { - val pushProvider = pushService.getCurrentPushProvider(userId) - val distributor = pushProvider?.getCurrentDistributor(userId) - // Attempt to register again - if (pushProvider != null && distributor != null) { - if (unifiedPushRemovedGatewayThrottler.canRegisterAgain()) { - pushService.registerWith( - client, - pushProvider, - distributor, - ) - .onFailure { - Timber.tag(loggerTag.value).w(it, "Unable to register with current data") - } - } else { - // Let the user know - Timber.tag(loggerTag.value).w("Second removal in less than 1 minute, do not register again") - pushService.onServiceUnregistered(userId) - Result.success(Unit) - } - } else { - Result.failure(IllegalStateException("Unable to register again")) - } - } + client.rotateRegistration(clientSecret = clientSecret) .onFailure { + Timber.tag(loggerTag.value).w(it, "Issue during pusher unregistration / re registration") // Let the user know - pushService.onServiceUnregistered(userId) + pushService.onServiceUnregistered(sessionId) } } - .onFailure { - Timber.tag(loggerTag.value).w(it, "Issue during pusher unregistration / re registration") + } + + /** + * Unregister the pusher for the session. Then register again if possible. + */ + private suspend fun MatrixClient.rotateRegistration(clientSecret: String): Result { + val unregisterResult = unregisterUnifiedPushUseCase.unregister( + matrixClient = this, + clientSecret = clientSecret, + unregisterUnifiedPush = false, + ).onFailure { + Timber.tag(loggerTag.value).w(it, "Unable to unregister pusher") + } + return unregisterResult.flatMap { + registerAgain() + } + } + + /** + * Attempt to register again, if possible i.e. the current configuration is known and the + * deletion of data in the UnifiedPush application has not already occurred in the last minute. + */ + private suspend fun MatrixClient.registerAgain(): Result { + return if (unifiedPushRemovedGatewayThrottler.canRegisterAgain()) { + val pushProvider = pushService.getCurrentPushProvider(sessionId) + val distributor = pushProvider?.getCurrentDistributor(sessionId) + if (pushProvider != null && distributor != null) { + pushService.registerWith( + matrixClient = this, + pushProvider = pushProvider, + distributor = distributor, + ).onFailure { + Timber.tag(loggerTag.value).w(it, "Unable to register with current data") + } + } else { + Result.failure(IllegalStateException("Unable to register again")) } + } else { + Timber.tag(loggerTag.value).w("Second removal in less than 1 minute, do not register again") + Result.failure(IllegalStateException("Too many requests to register again")) + } } } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt index 452013e18d..2fcaa3934b 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnifiedPushRemovedGatewayHandlerTest.kt @@ -229,7 +229,7 @@ class DefaultUnifiedPushRemovedGatewayHandlerTest { onServiceUnregisteredResult.assertions().isNeverCalled() // Second attempt in less than 1 minute val result2 = sut.handle(A_SECRET) - assertThat(result2.isSuccess).isTrue() + assertThat(result2.isFailure).isTrue() unregisterLambda.assertions().isCalledExactly(2) // Registration is not called twice registerWithLambda.assertions().isCalledOnce() From 017a259f1c69821189b6dd763189cb7365f432b4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 14 Nov 2025 15:25:04 +0100 Subject: [PATCH 20/20] Update notification content --- .../notifications/factories/NotificationCreator.kt | 13 +++++-------- .../impl/src/main/res/values-cs/translations.xml | 1 + .../impl/src/main/res/values-da/translations.xml | 1 + .../impl/src/main/res/values-de/translations.xml | 1 + .../impl/src/main/res/values-et/translations.xml | 1 + .../impl/src/main/res/values-fr/translations.xml | 1 + .../impl/src/main/res/values-sk/translations.xml | 1 + .../src/main/res/values-zh-rTW/translations.xml | 1 + .../push/impl/src/main/res/values/localazy.xml | 2 ++ .../src/main/res/values-cs/translations.xml | 1 - .../src/main/res/values-da/translations.xml | 1 - .../src/main/res/values-de/translations.xml | 1 - .../src/main/res/values-et/translations.xml | 1 - .../src/main/res/values-fr/translations.xml | 1 - .../src/main/res/values-sk/translations.xml | 1 - .../src/main/res/values-zh-rTW/translations.xml | 1 - .../ui-strings/src/main/res/values/localazy.xml | 1 - tools/localazy/config.json | 1 + 18 files changed, 15 insertions(+), 16 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 98a1228297..ff969c2b84 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 @@ -360,14 +360,11 @@ class DefaultNotificationCreator( notificationAccountParams: NotificationAccountParams, ): Notification { val userId = notificationAccountParams.user.userId - val text = if (notificationAccountParams.showSessionId) { - // TODO i18n - "$userId will not receive notifications anymore." - } else { - // TODO i18n - "You will not receive notifications anymore." - } + val text = stringProvider.getString(R.string.notification_error_unified_push_unregistered_android) return NotificationCompat.Builder(context, notificationChannels.getChannelIdForTest()) + .setSubText(userId.value) + // The text is long and can be truncated so use BigTextStyle. + .setStyle(NotificationCompat.BigTextStyle().bigText(text)) .setContentTitle(stringProvider.getString(CommonStrings.dialog_title_warning)) .setContentText(text) .configureWith(notificationAccountParams) @@ -479,7 +476,7 @@ class DefaultNotificationCreator( .build() ).also { it.conversationTitle = if (isThread) { - stringProvider.getString(CommonStrings.notification_thread_in_room, roomName) + stringProvider.getString(R.string.notification_thread_in_room, roomName) } else { roomName } diff --git a/libraries/push/impl/src/main/res/values-cs/translations.xml b/libraries/push/impl/src/main/res/values-cs/translations.xml index b93ae49dd7..8cc5d79095 100644 --- a/libraries/push/impl/src/main/res/values-cs/translations.xml +++ b/libraries/push/impl/src/main/res/values-cs/translations.xml @@ -42,6 +42,7 @@ "Já" "%1$s zmínil(a) nebo odpověděl(a)" "Prohlížíte si oznámení! Klikněte na mě!" + "Vlákno v %1$s" "%1$s: %2$s" "%1$s: %2$s %3$s" diff --git a/libraries/push/impl/src/main/res/values-da/translations.xml b/libraries/push/impl/src/main/res/values-da/translations.xml index 255605953b..a5b23fc7ba 100644 --- a/libraries/push/impl/src/main/res/values-da/translations.xml +++ b/libraries/push/impl/src/main/res/values-da/translations.xml @@ -38,6 +38,7 @@ "Mig" "%1$s nævnt eller besvaret" "Du ser notifikationen! Klik på mig!" + "Tråd i %1$s" "%1$s: %2$s" "%1$s: %2$s %3$s" diff --git a/libraries/push/impl/src/main/res/values-de/translations.xml b/libraries/push/impl/src/main/res/values-de/translations.xml index dff6371773..1a9e2f21c4 100644 --- a/libraries/push/impl/src/main/res/values-de/translations.xml +++ b/libraries/push/impl/src/main/res/values-de/translations.xml @@ -38,6 +38,7 @@ "Ich" "%1$s hat Dich erwähnt oder geantwortet" "Du siehst dir die Benachrichtigung an! Klicke hier!" + "Thread in %1$s" "%1$s: %2$s" "%1$s: %2$s %3$s" diff --git a/libraries/push/impl/src/main/res/values-et/translations.xml b/libraries/push/impl/src/main/res/values-et/translations.xml index 45d8e3c8c8..7f012722e7 100644 --- a/libraries/push/impl/src/main/res/values-et/translations.xml +++ b/libraries/push/impl/src/main/res/values-et/translations.xml @@ -38,6 +38,7 @@ "Mina" "%1$s mainis või vastas" "See ongi teavitus! Klõpsi mind!" + "Jutulõng „%1$s“ jututoas" "%1$s: %2$s" "%1$s: %2$s %3$s" diff --git a/libraries/push/impl/src/main/res/values-fr/translations.xml b/libraries/push/impl/src/main/res/values-fr/translations.xml index 5324597f8b..9464297d8a 100644 --- a/libraries/push/impl/src/main/res/values-fr/translations.xml +++ b/libraries/push/impl/src/main/res/values-fr/translations.xml @@ -38,6 +38,7 @@ "Moi" "%1$s mentionné ou en réponse" "Vous êtes en train de voir la notification ! Cliquez-moi !" + "Discussion dans %1$s" "%1$s : %2$s" "%1$s : %2$s %3$s" diff --git a/libraries/push/impl/src/main/res/values-sk/translations.xml b/libraries/push/impl/src/main/res/values-sk/translations.xml index 5dd3b8c0f2..a4deefba78 100644 --- a/libraries/push/impl/src/main/res/values-sk/translations.xml +++ b/libraries/push/impl/src/main/res/values-sk/translations.xml @@ -42,6 +42,7 @@ "Ja" "%1$s spomenul/a alebo odpovedal/a" "Prezeráte si oznámenie! Kliknite na mňa!" + "Vlákno v %1$s" "%1$s: %2$s" "%1$s: %2$s %3$s" diff --git a/libraries/push/impl/src/main/res/values-zh-rTW/translations.xml b/libraries/push/impl/src/main/res/values-zh-rTW/translations.xml index eded8c4c88..9e167a2e92 100644 --- a/libraries/push/impl/src/main/res/values-zh-rTW/translations.xml +++ b/libraries/push/impl/src/main/res/values-zh-rTW/translations.xml @@ -34,6 +34,7 @@ "我" "%1$s 提及或回覆" "您正在查看通知！點我！" + "在 %1$s 的討論串" "%1$s：%2$s" "%1$s：%2$s %3$s" diff --git a/libraries/push/impl/src/main/res/values/localazy.xml b/libraries/push/impl/src/main/res/values/localazy.xml index 45a3235ad5..0764851647 100644 --- a/libraries/push/impl/src/main/res/values/localazy.xml +++ b/libraries/push/impl/src/main/res/values/localazy.xml @@ -13,6 +13,7 @@ "%d notification" "%d notifications" + "The UnifiedPush notification distributor couldn\'t be registered, so you will not receive notifications anymore. Please check the notifications settings of the app and the status of the push distributor." "You have new messages." "📹 Incoming call" "** Failed to send - please open room" @@ -38,6 +39,7 @@ "Me" "%1$s mentioned or replied" "You are viewing the notification! Click me!" + "Thread in %1$s" "%1$s: %2$s" "%1$s: %2$s %3$s" diff --git a/libraries/ui-strings/src/main/res/values-cs/translations.xml b/libraries/ui-strings/src/main/res/values-cs/translations.xml index 0dc85069ef..ccc293a691 100644 --- a/libraries/ui-strings/src/main/res/values-cs/translations.xml +++ b/libraries/ui-strings/src/main/res/values-cs/translations.xml @@ -427,7 +427,6 @@ Opravdu chcete pokračovat?" "🔐️ Připojte se ke mně na %1$s" "Ahoj, ozvi se mi na %1$s: %2$s" "%1$s Android" - "Vlákno v %1$s" "Zatřeste zařízením pro nahlášení chyby" "Snímek obrazovky" "%1$s: %2$s" diff --git a/libraries/ui-strings/src/main/res/values-da/translations.xml b/libraries/ui-strings/src/main/res/values-da/translations.xml index b3d2aac151..f8491d6f26 100644 --- a/libraries/ui-strings/src/main/res/values-da/translations.xml +++ b/libraries/ui-strings/src/main/res/values-da/translations.xml @@ -420,7 +420,6 @@ Er du sikker på, at du vil fortsætte?" "🔐️ Kom med mig til %1$s" "Hej, lad os snakkes på %1$s: %2$s" "%1$s Android" - "Tråd i %1$s" "Ryst enheden i frustration for at anmelde en fejl" "Skærmbillede" "%1$s: %2$s" diff --git a/libraries/ui-strings/src/main/res/values-de/translations.xml b/libraries/ui-strings/src/main/res/values-de/translations.xml index e8ac460307..27e75b687c 100644 --- a/libraries/ui-strings/src/main/res/values-de/translations.xml +++ b/libraries/ui-strings/src/main/res/values-de/translations.xml @@ -420,7 +420,6 @@ Möchtest du wirklich fortfahren?" "🔐️ Begleite mich auf %1$s" "Hey, sprich mit mir auf %1$s: %2$s" "%1$s Android" - "Thread in %1$s" "Heftiges Schütteln um Fehler zu melden" "Bildschirmfoto" "%1$s: %2$s" diff --git a/libraries/ui-strings/src/main/res/values-et/translations.xml b/libraries/ui-strings/src/main/res/values-et/translations.xml index 2f3bd58e04..2732cb60de 100644 --- a/libraries/ui-strings/src/main/res/values-et/translations.xml +++ b/libraries/ui-strings/src/main/res/values-et/translations.xml @@ -421,7 +421,6 @@ Kas sa oled kindel, et soovid jätkata?" "🔐️ Liitu minuga rakenduses %1$s" "Hei, suhtle minuga %1$s võrgus: %2$s" "%1$s Android" - "Jutulõng „%1$s“ jututoas" "Veast teatamiseks raputa nutiseadet ägedalt" "Ekraanitõmmis" "%1$s: %2$s" diff --git a/libraries/ui-strings/src/main/res/values-fr/translations.xml b/libraries/ui-strings/src/main/res/values-fr/translations.xml index ec88a9e79f..ebc611015a 100644 --- a/libraries/ui-strings/src/main/res/values-fr/translations.xml +++ b/libraries/ui-strings/src/main/res/values-fr/translations.xml @@ -421,7 +421,6 @@ Raison : %1$s." "🔐️ Rejoignez-moi sur %1$s" "Salut, parle-moi sur %1$s : %2$s" "%1$s Android" - "Discussion dans %1$s" "Rageshake pour signaler un problème" "Capture d’écran" "%1$s: %2$s" diff --git a/libraries/ui-strings/src/main/res/values-sk/translations.xml b/libraries/ui-strings/src/main/res/values-sk/translations.xml index 9a2601019c..a94c5657cf 100644 --- a/libraries/ui-strings/src/main/res/values-sk/translations.xml +++ b/libraries/ui-strings/src/main/res/values-sk/translations.xml @@ -429,7 +429,6 @@ Naozaj chcete pokračovať?" "🔐️ Pripojte sa ku mne na %1$s" "Ahoj, porozprávajte sa so mnou na %1$s: %2$s" "%1$s Android" - "Vlákno v %1$s" "Zúrivo potriasť pre nahlásenie chyby" "Snímka obrazovky" "%1$s: %2$s" diff --git a/libraries/ui-strings/src/main/res/values-zh-rTW/translations.xml b/libraries/ui-strings/src/main/res/values-zh-rTW/translations.xml index 2838cf9669..0a8f32a694 100644 --- a/libraries/ui-strings/src/main/res/values-zh-rTW/translations.xml +++ b/libraries/ui-strings/src/main/res/values-zh-rTW/translations.xml @@ -412,7 +412,6 @@ "🔐️ 在 %1$s 上加入我" "嘿，來 %1$s 和我聊天：%2$s" "%1$s Android" - "在 %1$s 的討論串" "憤怒搖晃以回報臭蟲" "螢幕截圖" "%1$s：%2$s" diff --git a/libraries/ui-strings/src/main/res/values/localazy.xml b/libraries/ui-strings/src/main/res/values/localazy.xml index 7bf9e8192c..efe530f885 100644 --- a/libraries/ui-strings/src/main/res/values/localazy.xml +++ b/libraries/ui-strings/src/main/res/values/localazy.xml @@ -421,7 +421,6 @@ Are you sure you want to continue?" "🔐️ Join me on %1$s" "Hey, talk to me on %1$s: %2$s" "%1$s Android" - "Thread in %1$s" "Rageshake to report bug" "Screenshot" "%1$s: %2$s" diff --git a/tools/localazy/config.json b/tools/localazy/config.json index 39d63484a9..405451bb34 100644 --- a/tools/localazy/config.json +++ b/tools/localazy/config.json @@ -129,6 +129,7 @@ "includeRegex" : [ "push_.*", "notification_.*", + "notification\\..*", "troubleshoot_notifications\\.test_blocked_users\\..*", "troubleshoot_notifications_test_current_push_provider.*", "troubleshoot_notifications_test_detect_push_provider.*",