Do not attempt to restore the pusher after 2 removal in a short time.

This commit is contained in:
Benoit Marty
2025-11-13 23:12:57 +01:00
parent f5ca009887
commit ced5af18d0
4 changed files with 136 additions and 11 deletions

View File

@@ -333,7 +333,7 @@ class NotificationSettingsPresenterTest {
}
private fun createFakePushService(
registerWithLambda: suspend (MatrixClient, PushProvider, Distributor) -> Result<Unit> = { _, _, _ ->
registerWithLambda: (MatrixClient, PushProvider, Distributor) -> Result<Unit> = { _, _, _ ->
Result.success(Unit)
}
): PushService {

View File

@@ -23,7 +23,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
class FakePushService(
private val testPushBlock: suspend (SessionId) -> Boolean = { true },
private val availablePushProviders: List<PushProvider> = emptyList(),
private val registerWithLambda: suspend (MatrixClient, PushProvider, Distributor) -> Result<Unit> = { _, _, _ ->
private val registerWithLambda: (MatrixClient, PushProvider, Distributor) -> Result<Unit> = { _, _, _ ->
Result.success(Unit)
},
private val currentPushProvider: (SessionId) -> PushProvider? = { availablePushProviders.firstOrNull() },

View File

@@ -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<Unit>
}
@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<Unit> {
// 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"))
}

View File

@@ -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<UserId, Unit> { }
val unregisterLambda = lambdaRecorder<MatrixClient, String, Boolean, Result<Unit>> { _, _, _ -> Result.success(Unit) }
val registerWithLambda = lambdaRecorder<MatrixClient, PushProvider, Distributor, Result<Unit>> { _, _, _ -> 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<UserId, Unit> { }
val unregisterLambda = lambdaRecorder<MatrixClient, String, Boolean, Result<Unit>> { _, _, _ -> Result.success(Unit) }
val registerWithLambda = lambdaRecorder<MatrixClient, PushProvider, Distributor, Result<Unit>> { _, _, _ -> 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,
),
)
}