Improve code readability and documentation.

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<Unit> {
-        // Unregister the pusher for the session with this client secret.
+        val sessionId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure<Unit>(
-        val userId = pushClientSecret.getUserIdFromSecret(clientSecret) ?: return Result.failure<Unit>(
             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(
+                client.rotateRegistration(clientSecret = clientSecret)
-                    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"))
-                        }
-                    }
                     .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<Unit> {
+        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<Unit> {
+        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"))
+        }
     }
 }

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()