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 bf2b1ed5422..47e26fb9207 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 @@ -51,13 +51,16 @@ class DefaultPushService @Inject constructor( pushProvider: PushProvider, distributor: Distributor, ): Result { + Timber.d("Registering with ${pushProvider.name}/${distributor.name}}") val userPushStore = userPushStoreFactory.getOrCreate(matrixClient.sessionId) val currentPushProviderName = userPushStore.getPushProviderName() val currentPushProvider = pushProviders.find { it.name == currentPushProviderName } val currentDistributorValue = currentPushProvider?.getCurrentDistributor(matrixClient)?.value if (currentPushProviderName != pushProvider.name || currentDistributorValue != distributor.value) { // Unregister previous one if any - currentPushProvider?.unregister(matrixClient) + currentPushProvider + ?.also { Timber.d("Unregistering previous push provider $currentPushProviderName/$currentDistributorValue") } + ?.unregister(matrixClient) ?.onFailure { Timber.w(it, "Failed to unregister previous push provider") return Result.failure(it) 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 b88d70b1572..0228f8f74ba 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 @@ -64,14 +64,13 @@ class FirebasePushProvider @Inject constructor( override suspend fun getCurrentDistributor(matrixClient: MatrixClient) = firebaseDistributor 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 { + val pushKey = firebaseStore.getFcmToken() + return if (pushKey == null) { Timber.tag(loggerTag.value).w("Unable to unregister pusher, Firebase token is not known.") + Result.success(Unit) + } else { + 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/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt index 3c826827673..880be2f053f 100644 --- a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt +++ b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProviderTest.kt @@ -134,17 +134,14 @@ class FirebasePushProviderTest { } @Test - fun `unregister ko no token`() = runTest { + fun `unregister no token - in this case, the error is ignored`() = runTest { val firebasePushProvider = createFirebasePushProvider( firebaseStore = InMemoryFirebaseStore( token = null ), - pusherSubscriber = FakePusherSubscriber( - unregisterPusherResult = { _, _, _ -> Result.success(Unit) } - ) ) val result = firebasePushProvider.unregister(FakeMatrixClient()) - assertThat(result.isFailure).isTrue() + assertThat(result.isSuccess).isTrue() } @Test 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 65d924740de..769f6507d5b 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 @@ -23,6 +23,7 @@ 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 interface UnregisterUnifiedPushUseCase { @@ -39,7 +40,11 @@ class DefaultUnregisterUnifiedPushUseCase @Inject constructor( val endpoint = unifiedPushStore.getEndpoint(clientSecret) val gateway = unifiedPushStore.getPushGateway(clientSecret) if (endpoint == null || gateway == null) { - return Result.failure(IllegalStateException("No endpoint or gateway found for client secret")) + Timber.w("No endpoint or gateway found for client secret") + // Ensure we don't have any remaining data, but ignore this error + unifiedPushStore.storeUpEndpoint(clientSecret, null) + unifiedPushStore.storePushGateway(clientSecret, null) + return Result.success(Unit) } return pusherSubscriber.unregisterPusher(matrixClient, endpoint, gateway) .onSuccess { diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt index 1d04a07651f..dfa03707cc0 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt @@ -64,29 +64,49 @@ class DefaultUnregisterUnifiedPushUseCaseTest { } @Test - fun `test un registration error - no endpoint`() = runTest { + fun `test un registration error - no endpoint - will not unregister but return success`() = runTest { val matrixClient = FakeMatrixClient() + val storeUpEndpointResult = lambdaRecorder { _: String, _: String? -> } + val storePushGatewayResult = lambdaRecorder { _: String, _: String? -> } val useCase = createDefaultUnregisterUnifiedPushUseCase( unifiedPushStore = FakeUnifiedPushStore( getEndpointResult = { null }, getPushGatewayResult = { "aGateway" }, + storeUpEndpointResult = storeUpEndpointResult, + storePushGatewayResult = storePushGatewayResult, ), ) val result = useCase.execute(matrixClient, A_SECRET) - assertThat(result.isFailure).isTrue() + assertThat(result.isSuccess).isTrue() + storeUpEndpointResult.assertions() + .isCalledOnce() + .with(value(A_SECRET), value(null)) + storePushGatewayResult.assertions() + .isCalledOnce() + .with(value(A_SECRET), value(null)) } @Test - fun `test un registration error - no gateway`() = runTest { + fun `test un registration error - no gateway - will not unregister but return success`() = runTest { val matrixClient = FakeMatrixClient() + val storeUpEndpointResult = lambdaRecorder { _: String, _: String? -> } + val storePushGatewayResult = lambdaRecorder { _: String, _: String? -> } val useCase = createDefaultUnregisterUnifiedPushUseCase( unifiedPushStore = FakeUnifiedPushStore( getEndpointResult = { "aEndpoint" }, getPushGatewayResult = { null }, + storeUpEndpointResult = storeUpEndpointResult, + storePushGatewayResult = storePushGatewayResult, ), ) val result = useCase.execute(matrixClient, A_SECRET) - assertThat(result.isFailure).isTrue() + assertThat(result.isSuccess).isTrue() + storeUpEndpointResult.assertions() + .isCalledOnce() + .with(value(A_SECRET), value(null)) + storePushGatewayResult.assertions() + .isCalledOnce() + .with(value(A_SECRET), value(null)) } @Test