From 4bd01b6f4f3ae3d0c7a41b2f1903b00d14f3afa4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 6 May 2024 23:06:25 +0200 Subject: [PATCH] Improve API, to avoid ignoring errors --- .../appnav/loggedin/LoggedInPresenter.kt | 6 ++- .../android/libraries/push/api/PushService.kt | 6 ++- .../libraries/push/impl/DefaultPushService.kt | 18 +++++-- .../libraries/push/impl/PushersManager.kt | 44 +++++++++++------ .../libraries/push/test/FakePushService.kt | 7 ++- .../pushproviders/api/PushProvider.kt | 4 +- .../pushproviders/api/PusherSubscriber.kt | 4 +- .../firebase/FirebaseNewTokenHandler.kt | 15 ++++-- .../firebase/FirebasePushProvider.kt | 20 +++++--- .../pushproviders/test/FakePushProvider.kt | 8 ++-- .../unifiedpush/RegisterUnifiedPushUseCase.kt | 47 ++----------------- .../UnifiedPushNewGatewayHandler.kt | 20 +++++--- .../unifiedpush/UnifiedPushProvider.kt | 12 +++-- .../UnregisterUnifiedPushUseCase.kt | 20 ++++---- .../VectorUnifiedPushMessagingReceiver.kt | 7 ++- .../libraries/pushstore/api/UserPushStore.kt | 2 +- .../pushstore/impl/UserPushStoreDataStore.kt | 8 +++- .../test/userpushstore/FakeUserPushStore.kt | 2 +- 18 files changed, 143 insertions(+), 107 deletions(-) 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 838780edc8..6362695772 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,6 +36,7 @@ import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatu import io.element.android.libraries.push.api.PushService import io.element.android.services.analytics.api.AnalyticsService import kotlinx.coroutines.flow.map +import timber.log.Timber import javax.inject.Inject class LoggedInPresenter @Inject constructor( @@ -56,7 +57,7 @@ class LoggedInPresenter @Inject constructor( if (isVerified) { // Ensure pusher is registered val currentPushProvider = pushService.getCurrentPushProvider() - if (currentPushProvider == null) { + val result = if (currentPushProvider == null) { // Register with the first available push provider val pushProvider = pushService.getAvailablePushProviders().firstOrNull() ?: return@LaunchedEffect val distributor = pushProvider.getDistributors().firstOrNull() ?: return@LaunchedEffect @@ -72,6 +73,9 @@ class LoggedInPresenter @Inject constructor( pushService.registerWith(matrixClient, currentPushProvider, currentPushDistributor) } } + result.onFailure { + Timber.e(it, "Failed to register pusher") + } } } 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 fdb1d939ae..607213953d 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 @@ -40,7 +40,11 @@ interface PushService { * * The method has effect only if the [PushProvider] is different than the current one. */ - suspend fun registerWith(matrixClient: MatrixClient, pushProvider: PushProvider, distributor: Distributor) + suspend fun registerWith( + matrixClient: MatrixClient, + pushProvider: PushProvider, + distributor: Distributor, + ): Result /** * Return false in case of early error. 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 fe4d24882b..6d0f21b5d9 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 @@ -25,6 +25,7 @@ import io.element.android.libraries.push.impl.notifications.DefaultNotificationD import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider import io.element.android.libraries.pushstore.api.UserPushStoreFactory +import timber.log.Timber import javax.inject.Inject @ContributesBinding(AppScope::class) @@ -53,17 +54,26 @@ class DefaultPushService @Inject constructor( /** * Get current push provider, compare with provided one, then unregister and register if different, and store change. */ - override suspend fun registerWith(matrixClient: MatrixClient, pushProvider: PushProvider, distributor: Distributor) { + override suspend fun registerWith( + matrixClient: MatrixClient, + pushProvider: PushProvider, + distributor: Distributor, + ): Result { val userPushStore = userPushStoreFactory.getOrCreate(matrixClient.sessionId) val currentPushProviderName = userPushStore.getPushProviderName() val currentDistributorValue = pushProvider.getCurrentDistributor(matrixClient)?.value if (currentPushProviderName != pushProvider.name || currentDistributorValue != distributor.value) { // Unregister previous one if any pushProviders.find { it.name == currentPushProviderName }?.unregister(matrixClient) + ?.onFailure { + Timber.w(it, "Failed to unregister previous push provider") + } } - pushProvider.registerWith(matrixClient, distributor) - // Store new value - userPushStore.setPushProviderName(pushProvider.name) + return pushProvider.registerWith(matrixClient, distributor) + .onSuccess { + // Store new value + userPushStore.setPushProviderName(pushProvider.name) + } } override suspend fun testPush(): Boolean { diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/PushersManager.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/PushersManager.kt index ce60476961..7dba9f5678 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/PushersManager.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/PushersManager.kt @@ -63,22 +63,26 @@ class PushersManager @Inject constructor( /** * Register a pusher to the server if not done yet. */ - override suspend fun registerPusher(matrixClient: MatrixClient, pushKey: String, gateway: String) { + override suspend fun registerPusher( + matrixClient: MatrixClient, + pushKey: String, + gateway: String, + ): Result { val userDataStore = userPushStoreFactory.getOrCreate(matrixClient.sessionId) if (userDataStore.getCurrentRegisteredPushKey() == pushKey) { Timber.tag(loggerTag.value) .d("Unnecessary to register again the same pusher, but do it in case the pusher has been removed from the server") } - matrixClient.pushersService().setHttpPusher( - createHttpPusher(pushKey, gateway, matrixClient.sessionId) - ).fold( - { + return matrixClient.pushersService() + .setHttpPusher( + createHttpPusher(pushKey, gateway, matrixClient.sessionId) + ) + .onSuccess { userDataStore.setCurrentRegisteredPushKey(pushKey) - }, - { throwable -> + } + .onFailure { throwable -> Timber.tag(loggerTag.value).e(throwable, "Unable to register the pusher") } - ) } private suspend fun createHttpPusher( @@ -107,13 +111,25 @@ class PushersManager @Inject constructor( return "{\"cs\":\"$secretForUser\"}" } - override suspend fun unregisterPusher(matrixClient: MatrixClient, pushKey: String, gateway: String) { - matrixClient.pushersService().unsetHttpPusher( - unsetHttpPusherData = UnsetHttpPusherData( - pushKey = pushKey, - appId = PushConfig.PUSHER_APP_ID + override suspend fun unregisterPusher( + matrixClient: MatrixClient, + pushKey: String, + gateway: String, + ): Result { + val userDataStore = userPushStoreFactory.getOrCreate(matrixClient.sessionId) + return matrixClient.pushersService() + .unsetHttpPusher( + unsetHttpPusherData = UnsetHttpPusherData( + pushKey = pushKey, + appId = PushConfig.PUSHER_APP_ID + ) ) - ) + .onSuccess { + userDataStore.setCurrentRegisteredPushKey(null) + } + .onFailure { throwable -> + Timber.tag(loggerTag.value).e(throwable, "Unable to unregister the pusher") + } } companion object { 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 969815ec66..1f8dbc5e2f 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,7 +32,12 @@ class FakePushService( return emptyList() } - override suspend fun registerWith(matrixClient: MatrixClient, pushProvider: PushProvider, distributor: Distributor) { + override suspend fun registerWith( + matrixClient: MatrixClient, + pushProvider: PushProvider, + distributor: Distributor, + ): Result { + return Result.success(Unit) } override suspend fun testPush(): Boolean = simulateLongTask { diff --git a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt index a1c99df3fb..d111dc139f 100644 --- a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt +++ b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt @@ -42,7 +42,7 @@ interface PushProvider { /** * Register the pusher to the homeserver. */ - suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor) + suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor): Result /** * Return the current distributor, or null if none. @@ -52,7 +52,7 @@ interface PushProvider { /** * Unregister the pusher. */ - suspend fun unregister(matrixClient: MatrixClient) + suspend fun unregister(matrixClient: MatrixClient): Result suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? } diff --git a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PusherSubscriber.kt b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PusherSubscriber.kt index 2529e4bb96..d38f5dec1e 100644 --- a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PusherSubscriber.kt +++ b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PusherSubscriber.kt @@ -19,6 +19,6 @@ package io.element.android.libraries.pushproviders.api import io.element.android.libraries.matrix.api.MatrixClient interface PusherSubscriber { - suspend fun registerPusher(matrixClient: MatrixClient, pushKey: String, gateway: String) - suspend fun unregisterPusher(matrixClient: MatrixClient, pushKey: String, gateway: String) + suspend fun registerPusher(matrixClient: MatrixClient, pushKey: String, gateway: String): Result + suspend fun unregisterPusher(matrixClient: MatrixClient, pushKey: String, gateway: String): Result } diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseNewTokenHandler.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseNewTokenHandler.kt index 20d0de4ebf..1261df0911 100644 --- a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseNewTokenHandler.kt +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebaseNewTokenHandler.kt @@ -16,6 +16,7 @@ package io.element.android.libraries.pushproviders.firebase +import io.element.android.libraries.core.extensions.flatMap import io.element.android.libraries.core.log.logger.LoggerTag import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService import io.element.android.libraries.matrix.api.core.SessionId @@ -46,9 +47,17 @@ class FirebaseNewTokenHandler @Inject constructor( .forEach { userId -> val userDataStore = userPushStoreFactory.getOrCreate(userId) if (userDataStore.getPushProviderName() == FirebaseConfig.NAME) { - matrixAuthenticationService.restoreSession(userId).getOrNull()?.use { client -> - pusherSubscriber.registerPusher(client, firebaseToken, FirebaseConfig.PUSHER_HTTP_URL) - } + matrixAuthenticationService + .restoreSession(userId) + .onFailure { + Timber.tag(loggerTag.value).e(it, "Failed to restore session $userId") + } + .flatMap { client -> + pusherSubscriber.registerPusher(client, firebaseToken, FirebaseConfig.PUSHER_HTTP_URL) + } + .onFailure { + Timber.tag(loggerTag.value).e(it, "Failed to register pusher for session $userId") + } } else { Timber.tag(loggerTag.value).d("This session is not using Firebase pusher") } diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt index c53b2815fe..f804ec4227 100644 --- a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt @@ -46,20 +46,28 @@ class FirebasePushProvider @Inject constructor( return listOf(firebaseDistributor) } - override suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor) { - val pushKey = firebaseStore.getFcmToken() ?: return Unit.also { + override suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor): Result { + val pushKey = firebaseStore.getFcmToken() ?: return Result.failure( + IllegalStateException( + "Unable to register pusher, Firebase token is not known." + ) + ).also { Timber.tag(loggerTag.value).w("Unable to register pusher, Firebase token is not known.") } - pusherSubscriber.registerPusher(matrixClient, pushKey, FirebaseConfig.PUSHER_HTTP_URL) + return pusherSubscriber.registerPusher(matrixClient, pushKey, FirebaseConfig.PUSHER_HTTP_URL) } override suspend fun getCurrentDistributor(matrixClient: MatrixClient) = firebaseDistributor - override suspend fun unregister(matrixClient: MatrixClient) { - val pushKey = firebaseStore.getFcmToken() ?: return Unit.also { + override suspend fun unregister(matrixClient: MatrixClient): Result { + val pushKey = firebaseStore.getFcmToken() ?: return Result.failure( + IllegalStateException( + "Unable to unregister pusher, Firebase token is not known." + ) + ).also { Timber.tag(loggerTag.value).w("Unable to unregister pusher, Firebase token is not known.") } - pusherSubscriber.unregisterPusher(matrixClient, pushKey, FirebaseConfig.PUSHER_HTTP_URL) + return pusherSubscriber.unregisterPusher(matrixClient, pushKey, FirebaseConfig.PUSHER_HTTP_URL) } override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { diff --git a/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt b/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt index 8d8b94ec19..74627bcf41 100644 --- a/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt +++ b/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt @@ -31,12 +31,12 @@ class FakePushProvider( override fun getDistributors(): List = distributors - override suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor) { - // No-op + override suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor): Result { + return Result.success(Unit) } - override suspend fun unregister(matrixClient: MatrixClient) { - // No-op + override suspend fun unregister(matrixClient: MatrixClient): Result { + return Result.success(Unit) } override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/RegisterUnifiedPushUseCase.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/RegisterUnifiedPushUseCase.kt index 8dd71118b3..d24f1291d8 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/RegisterUnifiedPushUseCase.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/RegisterUnifiedPushUseCase.kt @@ -18,55 +18,18 @@ package io.element.android.libraries.pushproviders.unifiedpush import android.content.Context import io.element.android.libraries.di.ApplicationContext -import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.pushproviders.api.Distributor -import io.element.android.libraries.pushproviders.api.PusherSubscriber import org.unifiedpush.android.connector.UnifiedPush import javax.inject.Inject class RegisterUnifiedPushUseCase @Inject constructor( @ApplicationContext private val context: Context, - private val pusherSubscriber: PusherSubscriber, - private val unifiedPushStore: UnifiedPushStore, ) { - sealed interface RegisterUnifiedPushResult { - data object Success : RegisterUnifiedPushResult - data object NeedToAskUserForDistributor : RegisterUnifiedPushResult - data object Error : RegisterUnifiedPushResult - } - - suspend fun execute(matrixClient: MatrixClient, distributor: Distributor, clientSecret: String): RegisterUnifiedPushResult { - val distributorValue = distributor.value - if (distributorValue.isNotEmpty()) { - saveAndRegisterApp(distributorValue, clientSecret) - val endpoint = unifiedPushStore.getEndpoint(clientSecret) ?: return RegisterUnifiedPushResult.Error - val gateway = unifiedPushStore.getPushGateway(clientSecret) ?: return RegisterUnifiedPushResult.Error - pusherSubscriber.registerPusher(matrixClient, endpoint, gateway) - return RegisterUnifiedPushResult.Success - } - - // TODO Below should never happen? - if (UnifiedPush.getDistributor(context).isNotEmpty()) { - registerApp(clientSecret) - return RegisterUnifiedPushResult.Success - } - - val distributors = UnifiedPush.getDistributors(context) - - return if (distributors.size == 1) { - saveAndRegisterApp(distributors.first(), clientSecret) - RegisterUnifiedPushResult.Success - } else { - RegisterUnifiedPushResult.NeedToAskUserForDistributor - } - } - - private fun saveAndRegisterApp(distributor: String, clientSecret: String) { - UnifiedPush.saveDistributor(context, distributor) - registerApp(clientSecret) - } - - private fun registerApp(clientSecret: String) { + fun execute(distributor: Distributor, clientSecret: String): Result { + UnifiedPush.saveDistributor(context, distributor.value) + // This will trigger the callback + // VectorUnifiedPushMessagingReceiver.onNewEndpoint UnifiedPush.registerApp(context = context, instance = clientSecret) + return Result.success(Unit) } } 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 4ee637a3ab..dad99960a7 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 @@ -16,6 +16,7 @@ package io.element.android.libraries.pushproviders.unifiedpush +import io.element.android.libraries.core.extensions.flatMap import io.element.android.libraries.core.log.logger.LoggerTag import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService import io.element.android.libraries.pushproviders.api.PusherSubscriber @@ -35,18 +36,25 @@ class UnifiedPushNewGatewayHandler @Inject constructor( private val pushClientSecret: PushClientSecret, private val matrixAuthenticationService: MatrixAuthenticationService, ) { - suspend fun handle(endpoint: String, pushGateway: String, clientSecret: String) { + suspend fun handle(endpoint: String, pushGateway: String, clientSecret: String): Result { // Register the pusher for the session with this client secret, if is it using UnifiedPush. - val userId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Unit.also { + val userId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure( + IllegalStateException("Unable to retrieve session") + ).also { Timber.w("Unable to retrieve session") } val userDataStore = userPushStoreFactory.getOrCreate(userId) - if (userDataStore.getPushProviderName() == UnifiedPushConfig.NAME) { - matrixAuthenticationService.restoreSession(userId).getOrNull()?.use { client -> - pusherSubscriber.registerPusher(client, endpoint, pushGateway) - } + return if (userDataStore.getPushProviderName() == UnifiedPushConfig.NAME) { + matrixAuthenticationService + .restoreSession(userId) + .flatMap { client -> + pusherSubscriber.registerPusher(client, endpoint, pushGateway) + } } else { Timber.tag(loggerTag.value).d("This session is not using UnifiedPush pusher") + Result.failure( + IllegalStateException("This session is not using UnifiedPush pusher") + ) } } } diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt index cd16481ec3..4530a4667f 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt @@ -58,10 +58,12 @@ class UnifiedPushProvider @Inject constructor( return unifiedPushDistributorProvider.getDistributors() } - override suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor) { + override suspend fun registerWith(matrixClient: MatrixClient, distributor: Distributor): Result { val clientSecret = pushClientSecret.getSecretForUser(matrixClient.sessionId) - registerUnifiedPushUseCase.execute(matrixClient, distributor, clientSecret) - unifiedPushStore.setDistributorValue(matrixClient.sessionId, distributor.value) + return registerUnifiedPushUseCase.execute(distributor, clientSecret) + .onSuccess { + unifiedPushStore.setDistributorValue(matrixClient.sessionId, distributor.value) + } } override suspend fun getCurrentDistributor(matrixClient: MatrixClient): Distributor? { @@ -69,9 +71,9 @@ class UnifiedPushProvider @Inject constructor( return getDistributors().find { it.value == distributorValue } } - override suspend fun unregister(matrixClient: MatrixClient) { + override suspend fun unregister(matrixClient: MatrixClient): Result { val clientSecret = pushClientSecret.getSecretForUser(matrixClient.sessionId) - unRegisterUnifiedPushUseCase.execute(matrixClient, clientSecret) + return unRegisterUnifiedPushUseCase.execute(matrixClient, clientSecret) } override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { 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 ba389d0ae6..b2dd33c252 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,7 +21,6 @@ import io.element.android.libraries.di.ApplicationContext import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.pushproviders.api.PusherSubscriber import org.unifiedpush.android.connector.UnifiedPush -import timber.log.Timber import javax.inject.Inject class UnregisterUnifiedPushUseCase @Inject constructor( @@ -29,18 +28,17 @@ class UnregisterUnifiedPushUseCase @Inject constructor( private val unifiedPushStore: UnifiedPushStore, private val pusherSubscriber: PusherSubscriber, ) { - suspend fun execute(matrixClient: MatrixClient, clientSecret: String) { + suspend fun execute(matrixClient: MatrixClient, clientSecret: String): Result { val endpoint = unifiedPushStore.getEndpoint(clientSecret) val gateway = unifiedPushStore.getPushGateway(clientSecret) - if (endpoint != null && gateway != null) { - try { - pusherSubscriber.unregisterPusher(matrixClient, endpoint, gateway) - } catch (e: Exception) { - Timber.d(e, "Probably unregistering a non existing pusher") - } + if (endpoint == null || gateway == null) { + return Result.failure(IllegalStateException("No endpoint or gateway found for client secret")) } - unifiedPushStore.storeUpEndpoint(null, clientSecret) - unifiedPushStore.storePushGateway(null, clientSecret) - UnifiedPush.unregisterApp(context) + return pusherSubscriber.unregisterPusher(matrixClient, endpoint, gateway) + .onSuccess { + unifiedPushStore.storeUpEndpoint(null, clientSecret) + unifiedPushStore.storePushGateway(null, clientSecret) + UnifiedPush.unregisterApp(context) + } } } 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 e28d67ecf7..c87d915fdb 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 @@ -73,12 +73,17 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() { // If the endpoint has changed // or the gateway has changed if (unifiedPushStore.getEndpoint(instance) != endpoint) { - unifiedPushStore.storeUpEndpoint(endpoint, instance) coroutineScope.launch { val gateway = unifiedPushGatewayResolver.getGateway(endpoint) unifiedPushStore.storePushGateway(gateway, instance) gateway?.let { pushGateway -> newGatewayHandler.handle(endpoint, pushGateway, instance) + .onFailure { + Timber.tag(loggerTag.value).e("Failed to handle new gateway") + } + .onSuccess { + unifiedPushStore.storeUpEndpoint(endpoint, instance) + } } } } else { diff --git a/libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/UserPushStore.kt b/libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/UserPushStore.kt index fcfd6475c3..d24cc5ff5e 100644 --- a/libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/UserPushStore.kt +++ b/libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/UserPushStore.kt @@ -24,7 +24,7 @@ interface UserPushStore { suspend fun getPushProviderName(): String? suspend fun setPushProviderName(value: String) suspend fun getCurrentRegisteredPushKey(): String? - suspend fun setCurrentRegisteredPushKey(value: String) + suspend fun setCurrentRegisteredPushKey(value: String?) fun getNotificationEnabledForDevice(): Flow suspend fun setNotificationEnabledForDevice(enabled: Boolean) diff --git a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/UserPushStoreDataStore.kt b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/UserPushStoreDataStore.kt index f7a159f6c3..cfcc9e3da4 100644 --- a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/UserPushStoreDataStore.kt +++ b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/UserPushStoreDataStore.kt @@ -76,9 +76,13 @@ class UserPushStoreDataStore( return context.dataStore.data.first()[currentPushKey] } - override suspend fun setCurrentRegisteredPushKey(value: String) { + override suspend fun setCurrentRegisteredPushKey(value: String?) { context.dataStore.edit { - it[currentPushKey] = value + if (value == null) { + it.remove(currentPushKey) + } else { + it[currentPushKey] = value + } } } diff --git a/libraries/pushstore/test/src/main/kotlin/com/element/android/libraries/pushstore/test/userpushstore/FakeUserPushStore.kt b/libraries/pushstore/test/src/main/kotlin/com/element/android/libraries/pushstore/test/userpushstore/FakeUserPushStore.kt index 2afbf3210e..3428fb5d3e 100644 --- a/libraries/pushstore/test/src/main/kotlin/com/element/android/libraries/pushstore/test/userpushstore/FakeUserPushStore.kt +++ b/libraries/pushstore/test/src/main/kotlin/com/element/android/libraries/pushstore/test/userpushstore/FakeUserPushStore.kt @@ -36,7 +36,7 @@ class FakeUserPushStore : UserPushStore { return currentRegisteredPushKey } - override suspend fun setCurrentRegisteredPushKey(value: String) { + override suspend fun setCurrentRegisteredPushKey(value: String?) { currentRegisteredPushKey = value }