Remove dependency on AppNavigationStateService from DefaultGetCurrentPushProvider

This commit is contained in:
Benoit Marty
2025-10-23 15:03:04 +02:00
parent 0b9a4a6302
commit c53dabce16
15 changed files with 66 additions and 70 deletions

View File

@@ -134,7 +134,7 @@ class LoggedInPresenter(
private suspend fun ensurePusherIsRegistered(pusherRegistrationState: MutableState<AsyncData<Unit>>) {
Timber.tag(pusherTag.value).d("Ensure pusher is registered")
val currentPushProvider = pushService.getCurrentPushProvider()
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()

View File

@@ -487,7 +487,7 @@ class LoggedInPresenterTest {
Result.success(Unit)
},
selectPushProviderLambda: (SessionId, PushProvider) -> Unit = { _, _ -> lambdaError() },
currentPushProvider: () -> PushProvider? = { null },
currentPushProvider: (SessionId) -> PushProvider? = { null },
setIgnoreRegistrationErrorLambda: (SessionId, Boolean) -> Unit = { _, _ -> lambdaError() },
): PushService {
return FakePushService(

View File

@@ -94,7 +94,7 @@ class NotificationSettingsPresenter(
var refreshPushProvider by remember { mutableIntStateOf(0) }
LaunchedEffect(refreshPushProvider) {
val p = pushService.getCurrentPushProvider()
val p = pushService.getCurrentPushProvider(matrixClient.sessionId)
val distributor = p?.getCurrentDistributor(matrixClient.sessionId)
currentDistributor = if (distributor != null) {
AsyncData.Success(distributor)

View File

@@ -7,6 +7,8 @@
package io.element.android.libraries.push.api
import io.element.android.libraries.matrix.api.core.SessionId
interface GetCurrentPushProvider {
suspend fun getCurrentPushProvider(): String?
suspend fun getCurrentPushProvider(sessionId: SessionId): String?
}

View File

@@ -18,7 +18,7 @@ interface PushService {
/**
* Return the current push provider, or null if none.
*/
suspend fun getCurrentPushProvider(): PushProvider?
suspend fun getCurrentPushProvider(sessionId: SessionId): PushProvider?
/**
* Return the list of push providers, available at compile time, sorted by index.
@@ -51,7 +51,7 @@ interface PushService {
/**
* Return false in case of early error.
*/
suspend fun testPush(): Boolean
suspend fun testPush(sessionId: SessionId): Boolean
/**
* Get a flow of total number of received Push.

View File

@@ -9,23 +9,15 @@ package io.element.android.libraries.push.impl
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.push.api.GetCurrentPushProvider
import io.element.android.libraries.pushstore.api.UserPushStoreFactory
import io.element.android.services.appnavstate.api.AppNavigationStateService
import io.element.android.services.appnavstate.api.currentSessionId
@ContributesBinding(AppScope::class)
class DefaultGetCurrentPushProvider(
private val pushStoreFactory: UserPushStoreFactory,
private val appNavigationStateService: AppNavigationStateService,
) : GetCurrentPushProvider {
override suspend fun getCurrentPushProvider(): String? {
return appNavigationStateService
.appNavigationState
.value
.navigationState
.currentSessionId()
?.let { pushStoreFactory.getOrCreate(it) }
?.getPushProviderName()
override suspend fun getCurrentPushProvider(sessionId: SessionId): String? {
return pushStoreFactory.getOrCreate(sessionId).getPushProviderName()
}
}

View File

@@ -44,8 +44,8 @@ class DefaultPushService(
observeSessions()
}
override suspend fun getCurrentPushProvider(): PushProvider? {
val currentPushProvider = getCurrentPushProvider.getCurrentPushProvider()
override suspend fun getCurrentPushProvider(sessionId: SessionId): PushProvider? {
val currentPushProvider = getCurrentPushProvider.getCurrentPushProvider(sessionId)
return pushProviders.find { it.name == currentPushProvider }
}
@@ -97,8 +97,8 @@ class DefaultPushService(
userPushStoreFactory.getOrCreate(sessionId).setIgnoreRegistrationError(ignore)
}
override suspend fun testPush(): Boolean {
val pushProvider = getCurrentPushProvider() ?: return false
override suspend fun testPush(sessionId: SessionId): Boolean {
val pushProvider = getCurrentPushProvider(sessionId) ?: return false
val config = pushProvider.getCurrentUserPushConfig() ?: return false
testPush.execute(config)
return true

View File

@@ -37,7 +37,7 @@ class CurrentPushProviderTest(
override suspend fun run(coroutineScope: CoroutineScope) {
delegate.start()
val pushProvider = pushService.getCurrentPushProvider()
val pushProvider = pushService.getCurrentPushProvider(sessionId)
if (pushProvider == null) {
delegate.updateState(
description = stringProvider.getString(R.string.troubleshoot_notifications_test_current_push_provider_failure),

View File

@@ -7,9 +7,10 @@
package io.element.android.libraries.push.impl.troubleshoot
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesIntoSet
import dev.zacsweers.metro.Inject
import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.push.api.PushService
import io.element.android.libraries.push.api.gateway.PushGatewayFailure
import io.element.android.libraries.push.impl.R
@@ -28,9 +29,10 @@ import kotlinx.coroutines.withTimeout
import timber.log.Timber
import kotlin.time.Duration.Companion.seconds
@ContributesIntoSet(AppScope::class)
@ContributesIntoSet(SessionScope::class)
@Inject
class PushLoopbackTest(
private val sessionId: SessionId,
private val pushService: PushService,
private val diagnosticPushHandler: DiagnosticPushHandler,
private val clock: SystemClock,
@@ -52,9 +54,9 @@ class PushLoopbackTest(
completable.complete(clock.epochMillis() - startTime)
}
val testPushResult = try {
pushService.testPush()
pushService.testPush(sessionId)
} catch (pusherRejected: PushGatewayFailure.PusherRejected) {
val hasQuickFix = pushService.getCurrentPushProvider()?.canRotateToken() == true
val hasQuickFix = pushService.getCurrentPushProvider(sessionId)?.canRotateToken() == true
delegate.updateState(
description = stringProvider.getString(R.string.troubleshoot_notifications_test_push_loop_back_failure_1),
status = NotificationTroubleshootTestState.Status.Failure(hasQuickFix = hasQuickFix)
@@ -105,7 +107,7 @@ class PushLoopbackTest(
navigator: NotificationTroubleshootNavigator,
) {
delegate.start()
pushService.getCurrentPushProvider()?.rotateToken()
pushService.getCurrentPushProvider(sessionId)?.rotateToken()
run(coroutineScope)
}

View File

@@ -47,7 +47,7 @@ class DefaultPushServiceTest {
@Test
fun `test push no push provider`() = runTest {
val defaultPushService = createDefaultPushService()
assertThat(defaultPushService.testPush()).isFalse()
assertThat(defaultPushService.testPush(A_SESSION_ID)).isFalse()
}
@Test
@@ -57,7 +57,7 @@ class DefaultPushServiceTest {
pushProviders = setOf(aPushProvider),
getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = aPushProvider.name),
)
assertThat(defaultPushService.testPush()).isFalse()
assertThat(defaultPushService.testPush(A_SESSION_ID)).isFalse()
}
@Test
@@ -72,7 +72,7 @@ class DefaultPushServiceTest {
getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = aPushProvider.name),
testPush = FakeTestPush(executeResult = testPushResult),
)
assertThat(defaultPushService.testPush()).isTrue()
assertThat(defaultPushService.testPush(A_SESSION_ID)).isTrue()
testPushResult.assertions()
.isCalledOnce()
.with(value(aConfig))
@@ -81,7 +81,7 @@ class DefaultPushServiceTest {
@Test
fun `getCurrentPushProvider null`() = runTest {
val defaultPushService = createDefaultPushService()
val result = defaultPushService.getCurrentPushProvider()
val result = defaultPushService.getCurrentPushProvider(A_SESSION_ID)
assertThat(result).isNull()
}
@@ -92,7 +92,7 @@ class DefaultPushServiceTest {
pushProviders = setOf(aPushProvider),
getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = aPushProvider.name),
)
val result = defaultPushService.getCurrentPushProvider()
val result = defaultPushService.getCurrentPushProvider(A_SESSION_ID)
assertThat(result).isEqualTo(aPushProvider)
}

View File

@@ -8,14 +8,19 @@
package io.element.android.libraries.push.impl.troubleshoot
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.test.AN_EXCEPTION
import io.element.android.libraries.matrix.test.A_FAILURE_REASON
import io.element.android.libraries.matrix.test.A_SESSION_ID
import io.element.android.libraries.push.api.PushService
import io.element.android.libraries.push.api.gateway.PushGatewayFailure
import io.element.android.libraries.push.test.FakePushService
import io.element.android.libraries.pushproviders.test.FakePushProvider
import io.element.android.libraries.troubleshoot.api.test.NotificationTroubleshootTestState
import io.element.android.libraries.troubleshoot.test.FakeNotificationTroubleshootNavigator
import io.element.android.libraries.troubleshoot.test.runAndTestState
import io.element.android.services.toolbox.api.strings.StringProvider
import io.element.android.services.toolbox.api.systemclock.SystemClock
import io.element.android.services.toolbox.test.strings.FakeStringProvider
import io.element.android.services.toolbox.test.systemclock.FakeSystemClock
import io.element.android.tests.testutils.lambda.lambdaRecorder
@@ -25,13 +30,7 @@ import org.junit.Test
class PushLoopbackTestTest {
@Test
fun `test PushLoopbackTest timeout - push is not received`() = runTest {
val diagnosticPushHandler = DiagnosticPushHandler()
val sut = PushLoopbackTest(
pushService = FakePushService(),
diagnosticPushHandler = diagnosticPushHandler,
clock = FakeSystemClock(),
stringProvider = FakeStringProvider(),
)
val sut = createPushLoopbackTest()
sut.runAndTestState {
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Idle(true))
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.InProgress)
@@ -42,16 +41,12 @@ class PushLoopbackTestTest {
@Test
fun `test PushLoopbackTest PusherRejected error`() = runTest {
val diagnosticPushHandler = DiagnosticPushHandler()
val sut = PushLoopbackTest(
val sut = createPushLoopbackTest(
pushService = FakePushService(
testPushBlock = {
throw PushGatewayFailure.PusherRejected()
}
),
diagnosticPushHandler = diagnosticPushHandler,
clock = FakeSystemClock(),
stringProvider = FakeStringProvider(),
)
sut.runAndTestState {
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Idle(true))
@@ -65,9 +60,8 @@ class PushLoopbackTestTest {
@Test
fun `test PushLoopbackTest PusherRejected error with quick fix`() = runTest {
val diagnosticPushHandler = DiagnosticPushHandler()
val rotateTokenLambda = lambdaRecorder<Result<Unit>> { Result.success(Unit) }
val sut = PushLoopbackTest(
val sut = createPushLoopbackTest(
pushService = FakePushService(
testPushBlock = {
throw PushGatewayFailure.PusherRejected()
@@ -79,9 +73,6 @@ class PushLoopbackTestTest {
)
}
),
diagnosticPushHandler = diagnosticPushHandler,
clock = FakeSystemClock(),
stringProvider = FakeStringProvider(),
)
sut.runAndTestState {
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Idle(true))
@@ -97,14 +88,10 @@ class PushLoopbackTestTest {
@Test
fun `test PushLoopbackTest setup error`() = runTest {
val diagnosticPushHandler = DiagnosticPushHandler()
val sut = PushLoopbackTest(
val sut = createPushLoopbackTest(
pushService = FakePushService(
testPushBlock = { false }
),
diagnosticPushHandler = diagnosticPushHandler,
clock = FakeSystemClock(),
stringProvider = FakeStringProvider(),
)
sut.runAndTestState {
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Idle(true))
@@ -116,16 +103,12 @@ class PushLoopbackTestTest {
@Test
fun `test PushLoopbackTest other error`() = runTest {
val diagnosticPushHandler = DiagnosticPushHandler()
val sut = PushLoopbackTest(
val sut = createPushLoopbackTest(
pushService = FakePushService(
testPushBlock = {
throw AN_EXCEPTION
}
),
diagnosticPushHandler = diagnosticPushHandler,
clock = FakeSystemClock(),
stringProvider = FakeStringProvider(),
)
sut.runAndTestState {
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Idle(true))
@@ -139,14 +122,12 @@ class PushLoopbackTestTest {
@Test
fun `test PushLoopbackTest push is received`() = runTest {
val diagnosticPushHandler = DiagnosticPushHandler()
val sut = PushLoopbackTest(
val sut = createPushLoopbackTest(
pushService = FakePushService(testPushBlock = {
diagnosticPushHandler.handlePush()
true
}),
diagnosticPushHandler = diagnosticPushHandler,
clock = FakeSystemClock(),
stringProvider = FakeStringProvider(),
)
sut.runAndTestState {
assertThat(awaitItem().status).isEqualTo(NotificationTroubleshootTestState.Status.Idle(true))
@@ -156,3 +137,17 @@ class PushLoopbackTestTest {
}
}
}
private fun createPushLoopbackTest(
sessionId: SessionId = A_SESSION_ID,
pushService: PushService = FakePushService(),
diagnosticPushHandler: DiagnosticPushHandler = DiagnosticPushHandler(),
clock: SystemClock = FakeSystemClock(),
stringProvider: StringProvider = FakeStringProvider(),
) = PushLoopbackTest(
sessionId = sessionId,
pushService = pushService,
diagnosticPushHandler = diagnosticPushHandler,
clock = clock,
stringProvider = stringProvider
)

View File

@@ -7,10 +7,11 @@
package io.element.android.libraries.push.test
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.push.api.GetCurrentPushProvider
class FakeGetCurrentPushProvider(
private val currentPushProvider: String?
) : GetCurrentPushProvider {
override suspend fun getCurrentPushProvider(): String? = currentPushProvider
override suspend fun getCurrentPushProvider(sessionId: SessionId): String? = currentPushProvider
}

View File

@@ -19,19 +19,19 @@ import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
class FakePushService(
private val testPushBlock: suspend () -> Boolean = { true },
private val testPushBlock: suspend (SessionId) -> Boolean = { true },
private val availablePushProviders: List<PushProvider> = emptyList(),
private val registerWithLambda: suspend (MatrixClient, PushProvider, Distributor) -> Result<Unit> = { _, _, _ ->
Result.success(Unit)
},
private val currentPushProvider: () -> PushProvider? = { availablePushProviders.firstOrNull() },
private val currentPushProvider: (SessionId) -> PushProvider? = { availablePushProviders.firstOrNull() },
private val selectPushProviderLambda: suspend (SessionId, PushProvider) -> Unit = { _, _ -> lambdaError() },
private val setIgnoreRegistrationErrorLambda: (SessionId, Boolean) -> Unit = { _, _ -> lambdaError() },
private val resetPushHistoryResult: () -> Unit = { lambdaError() },
private val resetBatteryOptimizationStateResult: () -> Unit = { lambdaError() },
) : PushService {
override suspend fun getCurrentPushProvider(): PushProvider? {
return registeredPushProvider ?: currentPushProvider()
override suspend fun getCurrentPushProvider(sessionId: SessionId): PushProvider? {
return registeredPushProvider ?: currentPushProvider(sessionId)
}
override fun getAvailablePushProviders(): List<PushProvider> {
@@ -68,8 +68,8 @@ class FakePushService(
setIgnoreRegistrationErrorLambda(sessionId, ignore)
}
override suspend fun testPush(): Boolean = simulateLongTask {
testPushBlock()
override suspend fun testPush(sessionId: SessionId): Boolean = simulateLongTask {
testPushBlock(sessionId)
}
private val pushHistoryItemsFlow = MutableStateFlow<List<PushHistoryItem>>(emptyList())

View File

@@ -10,6 +10,7 @@ package io.element.android.libraries.troubleshoot.impl
import dev.zacsweers.metro.Inject
import im.vector.app.features.analytics.plan.NotificationTroubleshoot
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.push.api.GetCurrentPushProvider
import io.element.android.libraries.troubleshoot.api.test.NotificationTroubleshootNavigator
import io.element.android.libraries.troubleshoot.api.test.NotificationTroubleshootTest
@@ -25,6 +26,7 @@ import kotlinx.coroutines.flow.onEach
@Inject
class TroubleshootTestSuite(
private val sessionId: SessionId,
private val notificationTroubleshootTests: Set<@JvmSuppressWildcards NotificationTroubleshootTest>,
private val getCurrentPushProvider: GetCurrentPushProvider,
private val analyticsService: AnalyticsService,
@@ -41,7 +43,7 @@ class TroubleshootTestSuite(
suspend fun start(coroutineScope: CoroutineScope) {
val testFilterData = TestFilterData(
currentPushProviderName = getCurrentPushProvider.getCurrentPushProvider()
currentPushProviderName = getCurrentPushProvider.getCurrentPushProvider(sessionId)
)
tests = notificationTroubleshootTests
.filter { it.isRelevant(testFilterData) }

View File

@@ -9,6 +9,7 @@ package io.element.android.libraries.troubleshoot.impl
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.test.A_SESSION_ID
import io.element.android.libraries.push.test.FakeGetCurrentPushProvider
import io.element.android.libraries.troubleshoot.api.test.NotificationTroubleshootNavigator
import io.element.android.libraries.troubleshoot.api.test.NotificationTroubleshootTest
@@ -170,6 +171,7 @@ private fun createTroubleshootTestSuite(
currentPushProvider: String? = null,
): TroubleshootTestSuite {
return TroubleshootTestSuite(
sessionId = A_SESSION_ID,
notificationTroubleshootTests = tests,
getCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider),
analyticsService = FakeAnalyticsService(),