From fc7bdafbcbf95c0cf168801c2a836e96de18d45e Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 6 Jul 2023 18:08:29 +0200 Subject: [PATCH] Nodes: rework RootFlowNode with cache service --- .../io/element/android/appnav/RootFlowNode.kt | 88 +++++++++++-------- features/preferences/api/build.gradle.kts | 1 + .../features/preferences/api/CacheService.kt | 8 +- .../preferences/impl/DefaultCacheService.kt | 13 ++- .../impl/tasks/ClearCacheUseCase.kt | 3 +- .../libraries/architecture/ParentNodeExt.kt | 23 +++++ 6 files changed, 87 insertions(+), 49 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt index 733485fcef..96379e5792 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt @@ -31,7 +31,6 @@ import com.bumble.appyx.core.node.node import com.bumble.appyx.core.plugin.Plugin import com.bumble.appyx.core.plugin.plugins import com.bumble.appyx.navmodel.backstack.BackStack -import com.bumble.appyx.navmodel.backstack.operation.newRoot import com.bumble.appyx.navmodel.backstack.operation.pop import com.bumble.appyx.navmodel.backstack.operation.push import dagger.assisted.Assisted @@ -49,19 +48,21 @@ import io.element.android.features.rageshake.api.bugreport.BugReportEntryPoint import io.element.android.libraries.architecture.BackstackNode import io.element.android.libraries.architecture.animation.rememberDefaultTransitionHandler import io.element.android.libraries.architecture.createNode +import io.element.android.libraries.architecture.waitForChildAttached import io.element.android.libraries.deeplink.DeeplinkData import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService import io.element.android.libraries.matrix.api.core.SessionId -import io.element.android.libraries.matrix.api.core.UserId +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.onStart import kotlinx.parcelize.Parcelize import timber.log.Timber +import java.util.UUID @ContributesNode(AppScope::class) class RootFlowNode @AssistedInject constructor( @@ -90,21 +91,15 @@ class RootFlowNode @AssistedInject constructor( } private fun observeLoggedInState() { - authenticationService.isLoggedIn() - .distinctUntilChanged() - .combine( - cacheService.cacheIndex().onEach { - Timber.v("cacheIndex=$it") - matrixClientsHolder.removeAll() - } - ) { isLoggedIn, cacheIdx -> isLoggedIn to cacheIdx } - .onEach { pair -> - val isLoggedIn = pair.first - val cacheIndex = pair.second - Timber.v("isLoggedIn=$isLoggedIn, cacheIndex=$cacheIndex") + combine( + cacheService.onClearedCacheEventFlow(), + authenticationService.isLoggedIn(), + ) { _, isLoggedIn -> isLoggedIn } + .onEach { isLoggedIn -> + Timber.v("isLoggedIn=$isLoggedIn") if (isLoggedIn) { tryToRestoreLatestSession( - onSuccess = { switchToLoggedInFlow(it, cacheIndex) }, + onSuccess = { switchToLoggedInFlow(it) }, onFailure = { switchToNotLoggedInFlow() } ) } else { @@ -114,8 +109,8 @@ class RootFlowNode @AssistedInject constructor( .launchIn(lifecycleScope) } - private fun switchToLoggedInFlow(sessionId: SessionId, cacheIndex: Int) { - backstack.safeRoot(NavTarget.LoggedInFlow(sessionId, cacheIndex)) + private fun switchToLoggedInFlow(sessionId: SessionId) { + backstack.safeRoot(NavTarget.LoggedInFlow(sessionId)) } private fun switchToNotLoggedInFlow() { @@ -123,30 +118,40 @@ class RootFlowNode @AssistedInject constructor( backstack.safeRoot(NavTarget.NotLoggedInFlow) } - private suspend fun tryToRestoreLatestSession( - onSuccess: (UserId) -> Unit = {}, - onFailure: () -> Unit = {} + private suspend fun restoreSessionIfNeeded( + sessionId: SessionId, + onFailure: () -> Unit = {}, + onSuccess: (SessionId) -> Unit = {}, ) { - val latestKnownUserId = authenticationService.getLatestSessionId() - if (latestKnownUserId == null) { - onFailure() + // If the session is already known it'll be restored by the node hierarchy + if (matrixClientsHolder.knowSession(sessionId)) { + Timber.v("Session $sessionId already alive, no need to restore.") return } - if (matrixClientsHolder.knowSession(latestKnownUserId)) { - onSuccess(latestKnownUserId) - return - } - authenticationService.restoreSession(UserId(latestKnownUserId.value)) + authenticationService.restoreSession(sessionId) .onSuccess { matrixClient -> matrixClientsHolder.add(matrixClient) - onSuccess(matrixClient.sessionId) + Timber.v("Succeed to restore session $sessionId") + onSuccess(sessionId) } .onFailure { - Timber.v("Failed to restore session...") + Timber.v("Failed to restore session $sessionId") onFailure() } } + private suspend fun tryToRestoreLatestSession( + onSuccess: (SessionId) -> Unit = {}, + onFailure: () -> Unit = {} + ) { + val latestSessionId = authenticationService.getLatestSessionId() + if (latestSessionId == null) { + onFailure() + return + } + restoreSessionIfNeeded(latestSessionId, onFailure, onSuccess) + } + private fun onOpenBugReport() { backstack.push(NavTarget.BugReport) } @@ -175,7 +180,10 @@ class RootFlowNode @AssistedInject constructor( object NotLoggedInFlow : NavTarget @Parcelize - data class LoggedInFlow(val sessionId: SessionId, val cacheIndex: Int) : NavTarget + data class LoggedInFlow( + val sessionId: SessionId, + val navId: UUID = UUID.randomUUID(), + ) : NavTarget @Parcelize object BugReport : NavTarget @@ -186,7 +194,6 @@ class RootFlowNode @AssistedInject constructor( is NavTarget.LoggedInFlow -> { val matrixClient = matrixClientsHolder.getOrNull(navTarget.sessionId) ?: return splashNode(buildContext).also { Timber.w("Couldn't find any session, go through SplashScreen") - backstack.newRoot(NavTarget.SplashScreen) } val inputs = LoggedInFlowNode.Inputs(matrixClient) val callback = object : LoggedInFlowNode.Callback { @@ -247,9 +254,16 @@ class RootFlowNode @AssistedInject constructor( } private suspend fun attachSession(sessionId: SessionId): LoggedInFlowNode { - val cacheIndex = cacheService.cacheIndex().first() - return attachChild { - backstack.newRoot(NavTarget.LoggedInFlow(sessionId, cacheIndex)) + //TODO handle multi-session + return waitForChildAttached { navTarget -> + navTarget is NavTarget.LoggedInFlow && navTarget.sessionId == sessionId } } + + private fun CacheService.onClearedCacheEventFlow(): Flow { + return clearedCacheEventFlow + .onEach { sessionId -> matrixClientsHolder.remove(sessionId) } + .map { } + .onStart { emit((Unit)) } + } } diff --git a/features/preferences/api/build.gradle.kts b/features/preferences/api/build.gradle.kts index bf0cb4a51f..c20fe9aabb 100644 --- a/features/preferences/api/build.gradle.kts +++ b/features/preferences/api/build.gradle.kts @@ -23,4 +23,5 @@ android { dependencies { implementation(projects.libraries.architecture) + api(projects.libraries.matrix.api) } diff --git a/features/preferences/api/src/main/kotlin/io/element/android/features/preferences/api/CacheService.kt b/features/preferences/api/src/main/kotlin/io/element/android/features/preferences/api/CacheService.kt index 0bc9285853..191535a3a0 100644 --- a/features/preferences/api/src/main/kotlin/io/element/android/features/preferences/api/CacheService.kt +++ b/features/preferences/api/src/main/kotlin/io/element/android/features/preferences/api/CacheService.kt @@ -16,13 +16,13 @@ package io.element.android.features.preferences.api +import io.element.android.libraries.matrix.api.core.SessionId import kotlinx.coroutines.flow.Flow interface CacheService { /** - * Returns a flow of the current cache index, can let the app to know when the - * cache has been cleared, for instance to restart the app. - * Will be a flow of Int, starting from 0, and incrementing each time the cache is cleared. + * A flow of [SessionId], can let the app to know when the + * cache has been cleared for a given session, for instance to restart the app. */ - fun cacheIndex(): Flow + val clearedCacheEventFlow: Flow } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/DefaultCacheService.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/DefaultCacheService.kt index 7675ec3dd6..2ffe480518 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/DefaultCacheService.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/DefaultCacheService.kt @@ -20,20 +20,19 @@ import com.squareup.anvil.annotations.ContributesBinding import io.element.android.features.preferences.api.CacheService import io.element.android.libraries.di.AppScope import io.element.android.libraries.di.SingleIn +import io.element.android.libraries.matrix.api.core.SessionId import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.MutableSharedFlow import javax.inject.Inject @SingleIn(AppScope::class) @ContributesBinding(AppScope::class) class DefaultCacheService @Inject constructor() : CacheService { - private val cacheIndexState = MutableStateFlow(0) - override fun cacheIndex(): Flow { - return cacheIndexState - } + private val _clearedCacheEventFlow = MutableSharedFlow(0) + override val clearedCacheEventFlow: Flow = _clearedCacheEventFlow - fun incrementCacheIndex() { - cacheIndexState.value++ + suspend fun onClearedCache(sessionId: SessionId) { + _clearedCacheEventFlow.emit(sessionId) } } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt index f7b0d01130..0ef0e02558 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/tasks/ClearCacheUseCase.kt @@ -27,6 +27,7 @@ import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.di.ApplicationContext import io.element.android.libraries.di.SessionScope import io.element.android.libraries.matrix.api.MatrixClient +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.withContext import okhttp3.OkHttpClient import javax.inject.Inject @@ -57,6 +58,6 @@ class DefaultClearCacheUseCase @Inject constructor( // Clear app cache context.cacheDir.deleteRecursively() // Ensure the app is restarted - defaultCacheIndexProvider.incrementCacheIndex() + defaultCacheIndexProvider.onClearedCache(matrixClient.sessionId) } } diff --git a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt index b8284ff7b9..c03ff64f6c 100644 --- a/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt +++ b/libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/ParentNodeExt.kt @@ -16,12 +16,35 @@ package io.element.android.libraries.architecture +import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.children.nodeOrNull import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode +import kotlinx.coroutines.launch +import kotlinx.coroutines.suspendCancellableCoroutine +import kotlin.coroutines.resume fun ParentNode.childNode(navTarget: NavTarget): Node? { val childMap = children.value val key = childMap.keys.find { it.navTarget == navTarget } return childMap[key]?.nodeOrNull } + +suspend inline fun ParentNode.waitForChildAttached(crossinline predicate: (NavTarget) -> Boolean): N = + suspendCancellableCoroutine { continuation -> + lifecycleScope.launch { + children.collect { childMap -> + val expectedChildNode = childMap.entries + .map { it.key.navTarget } + .lastOrNull(predicate) + ?.let { + childNode(it) as? N + } + if (expectedChildNode != null && !continuation.isCompleted) { + continuation.resume(expectedChildNode) + } + } + }.invokeOnCompletion { + continuation.cancel() + } + }