From fbe70ddf5db6fa1d90fb166f76861ad4462edc64 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2024 16:47:17 +0200 Subject: [PATCH 1/3] Use session coroutine scope instead of application coroutine scope. --- .../android/features/ftue/impl/state/DefaultFtueService.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt index 049bba4dbb..8ff01100d7 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt @@ -24,6 +24,7 @@ import io.element.android.features.ftue.api.state.FtueService import io.element.android.features.ftue.api.state.FtueState import io.element.android.features.lockscreen.api.LockScreenService import io.element.android.libraries.di.SessionScope +import io.element.android.libraries.di.annotations.SessionCoroutineScope import io.element.android.libraries.matrix.api.verification.SessionVerificationService import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus import io.element.android.libraries.permissions.api.PermissionStateProvider @@ -47,7 +48,7 @@ import kotlin.time.Duration.Companion.seconds @ContributesBinding(SessionScope::class) class DefaultFtueService @Inject constructor( private val sdkVersionProvider: BuildVersionSdkIntProvider, - coroutineScope: CoroutineScope, + @SessionCoroutineScope sessionCoroutineScope: CoroutineScope, private val analyticsService: AnalyticsService, private val permissionStateProvider: PermissionStateProvider, private val lockScreenService: LockScreenService, @@ -66,11 +67,11 @@ class DefaultFtueService @Inject constructor( init { sessionVerificationService.sessionVerifiedStatus .onEach { updateState() } - .launchIn(coroutineScope) + .launchIn(sessionCoroutineScope) analyticsService.didAskUserConsent() .onEach { updateState() } - .launchIn(coroutineScope) + .launchIn(sessionCoroutineScope) } suspend fun getNextStep(currentStep: FtueStep? = null): FtueStep? = From e48b958d5286c8529b8a55bf3c8c6c443fc6b9b9 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2024 16:48:40 +0200 Subject: [PATCH 2/3] When clearing cache, ensure that SessionPreferencesStore is removed from the cache. Fixes blank screen after clear cache. Also cleanup the codebase. --- appnav/build.gradle.kts | 1 + .../android/appnav/root/RootNavStateFlowFactory.kt | 4 ++++ .../element/android/features/ftue/impl/FtueFlowNode.kt | 10 ++++------ .../features/ftue/impl/state/DefaultFtueService.kt | 8 +++----- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/appnav/build.gradle.kts b/appnav/build.gradle.kts index e99f3e01f0..53680fd44e 100644 --- a/appnav/build.gradle.kts +++ b/appnav/build.gradle.kts @@ -42,6 +42,7 @@ dependencies { implementation(projects.libraries.architecture) implementation(projects.libraries.deeplink) implementation(projects.libraries.matrix.api) + implementation(projects.libraries.preferences.api) implementation(projects.libraries.push.api) implementation(projects.libraries.pushproviders.api) implementation(projects.libraries.designsystem) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/root/RootNavStateFlowFactory.kt b/appnav/src/main/kotlin/io/element/android/appnav/root/RootNavStateFlowFactory.kt index b801415f88..9bdd203cd5 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/root/RootNavStateFlowFactory.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/root/RootNavStateFlowFactory.kt @@ -23,6 +23,7 @@ import io.element.android.features.login.api.LoginUserStory import io.element.android.features.preferences.api.CacheService import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService import io.element.android.libraries.matrix.ui.media.ImageLoaderHolder +import io.element.android.libraries.preferences.api.store.SessionPreferencesStoreFactory import io.element.android.libraries.sessionstorage.api.LoggedInState import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine @@ -42,6 +43,7 @@ class RootNavStateFlowFactory @Inject constructor( private val matrixClientsHolder: MatrixClientsHolder, private val imageLoaderHolder: ImageLoaderHolder, private val loginUserStory: LoginUserStory, + private val sessionPreferencesStoreFactory: SessionPreferencesStoreFactory, ) { private var currentCacheIndex = 0 @@ -73,6 +75,8 @@ class RootNavStateFlowFactory @Inject constructor( matrixClientsHolder.remove(sessionId) // Ensure image loader will be recreated with the new MatrixClient imageLoaderHolder.remove(sessionId) + // Also remove cached value for SessionPreferencesStore + sessionPreferencesStoreFactory.remove(sessionId) } .toIndexFlow(initialCacheIndex) .onEach { cacheIndex -> diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt index 6720fa0274..78ee3c7e69 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/FtueFlowNode.kt @@ -45,7 +45,7 @@ import io.element.android.libraries.di.SessionScope import io.element.android.services.analytics.api.AnalyticsService import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch @@ -93,11 +93,9 @@ class FtueFlowNode @AssistedInject constructor( }) analyticsService.didAskUserConsent() - .drop(1) // We only care about consent passing from not asked to asked state - .onEach { didAskUserConsent -> - if (didAskUserConsent) { - lifecycleScope.launch { moveToNextStep() } - } + .distinctUntilChanged() + .onEach { + lifecycleScope.launch { moveToNextStep() } } .launchIn(lifecycleScope) } diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt index 8ff01100d7..24c9c7df82 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt @@ -35,6 +35,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.catch +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn @@ -70,6 +71,7 @@ class DefaultFtueService @Inject constructor( .launchIn(sessionCoroutineScope) analyticsService.didAskUserConsent() + .distinctUntilChanged() .onEach { updateState() } .launchIn(sessionCoroutineScope) } @@ -120,10 +122,7 @@ class DefaultFtueService @Inject constructor( emit(SessionVerifiedStatus.NotVerified) } .first() - // For some obscure reason we need to call this *before* we check the `readyVerifiedSessionStatus`, otherwise there's a deadlock - // It seems like a DataStore bug - val skipVerification = canSkipVerification() - return readyVerifiedSessionStatus == SessionVerifiedStatus.NotVerified && !skipVerification + return readyVerifiedSessionStatus == SessionVerifiedStatus.NotVerified && !canSkipVerification() } private suspend fun canSkipVerification(): Boolean { @@ -131,7 +130,6 @@ class DefaultFtueService @Inject constructor( } private suspend fun needsAnalyticsOptIn(): Boolean { - // We need this function to not be suspend, so we need to load the value through runBlocking return analyticsService.didAskUserConsent().first().not() } From 5ccf95018dc0167609b914063d9f9c15e6c5d552 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 8 Jul 2024 17:49:39 +0200 Subject: [PATCH 3/3] Fix test compilation --- .../android/features/ftue/impl/DefaultFtueServiceTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt index ac2d0724f1..ae2c4dec58 100644 --- a/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt +++ b/features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt @@ -251,7 +251,7 @@ class DefaultFtueServiceTest { // First version where notification permission is required sdkIntVersion: Int = Build.VERSION_CODES.TIRAMISU, ) = DefaultFtueService( - coroutineScope = coroutineScope, + sessionCoroutineScope = coroutineScope, sessionVerificationService = sessionVerificationService, sdkVersionProvider = FakeBuildVersionSdkIntProvider(sdkIntVersion), analyticsService = analyticsService,