Merge pull request #803 from vector-im/feature/fga/app_nav_node_fixes

Feature/fga/app nav node fixes
2023-07-07 10:26:59 +02:00
parent df8f447e23 b2232fdd37
commit b70fbb24a6
8 changed files with 111 additions and 77 deletions
--- a/appnav/src/main/kotlin/io/element/android/appnav/BackstackExt.kt
+++ b/appnav/src/main/kotlin/io/element/android/appnav/BackstackExt.kt
@@ -18,6 +18,7 @@ package io.element.android.appnav

 import com.bumble.appyx.navmodel.backstack.BackStack
 import com.bumble.appyx.navmodel.backstack.operation.NewRoot
+import com.bumble.appyx.navmodel.backstack.operation.Remove

 /**
 * Don't process NewRoot if the nav target already exists in the stack.
@@ -29,3 +30,14 @@ fun <T : Any> BackStack<T>.safeRoot(element: T) {
    if (containsRoot) return
    accept(NewRoot(element))
 }
+
+/**
+ * Remove the last element on the backstack equals to the given one.
+ */
+fun <T : Any> BackStack<T>.removeLast(element: T) {
+    val lastExpectedNavElement = elements.value.lastOrNull {
+        it.key.navTarget == element
+    } ?: return
+    accept(Remove(lastExpectedNavElement.key))
+}
+
--- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt
+++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt
@@ -56,7 +56,6 @@ import io.element.android.libraries.architecture.animation.rememberDefaultTransi
 import io.element.android.libraries.architecture.bindings
 import io.element.android.libraries.architecture.createNode
 import io.element.android.libraries.architecture.inputs
-import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator
 import io.element.android.libraries.designsystem.theme.components.Text
 import io.element.android.libraries.designsystem.utils.SnackbarDispatcher
 import io.element.android.libraries.di.AppScope
@@ -92,7 +91,7 @@ class LoggedInFlowNode @AssistedInject constructor(
    snackbarDispatcher: SnackbarDispatcher,
 ) : BackstackNode<LoggedInFlowNode.NavTarget>(
    backstack = BackStack(
-        initialElement = NavTarget.SplashScreen,
+        initialElement = NavTarget.RoomList,
        savedStateMap = buildContext.savedStateMap,
    ),
    buildContext = buildContext,
@@ -104,22 +103,14 @@ class LoggedInFlowNode @AssistedInject constructor(
            .distinctUntilChanged()
            .onEach { isConsentAsked ->
                if (isConsentAsked) {
-                    switchToRoomList()
+                    backstack.removeLast(NavTarget.AnalyticsOptIn)
                } else {
-                    switchToAnalytics()
+                    backstack.push(NavTarget.AnalyticsOptIn)
                }
            }
            .launchIn(lifecycleScope)
    }

-    private fun switchToRoomList() {
-        backstack.safeRoot(NavTarget.RoomList)
-    }
-
-    private fun switchToAnalytics() {
-        backstack.safeRoot(NavTarget.AnalyticsSettings)
-    }
-
    interface Callback : Plugin {
        fun onOpenBugReport() = Unit
    }
@@ -194,9 +185,6 @@ class LoggedInFlowNode @AssistedInject constructor(
    }

    sealed interface NavTarget : Parcelable {
-        @Parcelize
-        object SplashScreen : NavTarget
-
        @Parcelize
        object Permanent : NavTarget

@@ -222,12 +210,11 @@ class LoggedInFlowNode @AssistedInject constructor(
        object InviteList : NavTarget

        @Parcelize
-        object AnalyticsSettings : NavTarget
+        object AnalyticsOptIn : NavTarget
    }

    override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
        return when (navTarget) {
-            NavTarget.SplashScreen -> splashNode(buildContext)
            NavTarget.Permanent -> {
                createNode<LoggedInNode>(buildContext)
            }
@@ -330,7 +317,7 @@ class LoggedInFlowNode @AssistedInject constructor(
                    .callback(callback)
                    .build()
            }
-            NavTarget.AnalyticsSettings -> {
+            NavTarget.AnalyticsOptIn -> {
                analyticsOptInEntryPoint.createNode(this, buildContext)
            }
        }
@@ -349,12 +336,6 @@ class LoggedInFlowNode @AssistedInject constructor(
        }
    }

-    private fun splashNode(buildContext: BuildContext) = node(buildContext) {
-        Box(modifier = it.fillMaxSize(), contentAlignment = Alignment.Center) {
-            CircularProgressIndicator()
-        }
-    }
-
    @Composable
    override fun View(modifier: Modifier) {
        Box(modifier = modifier) {
--- 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
@@ -50,20 +49,23 @@ 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.distinctUntilChanged
+
 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(
@@ -93,20 +95,15 @@ class RootFlowNode @AssistedInject constructor(
    }

    private fun observeLoggedInState() {
-        isUserLoggedInFlow()
-            .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(),
+            isUserLoggedInFlow(),
+        ) { _, isLoggedIn -> isLoggedIn }
+            .onEach { isLoggedIn ->
+                Timber.v("isLoggedIn=$isLoggedIn")
                if (isLoggedIn) {
                    tryToRestoreLatestSession(
-                        onSuccess = { switchToLoggedInFlow(it, cacheIndex) },
+                        onSuccess = { switchToLoggedInFlow(it) },
                        onFailure = { switchToNotLoggedInFlow() }
                    )
                } else {
@@ -116,6 +113,11 @@ class RootFlowNode @AssistedInject constructor(
            .launchIn(lifecycleScope)
    }

+
+    private fun switchToLoggedInFlow(sessionId: SessionId) {
+        backstack.safeRoot(NavTarget.LoggedInFlow(sessionId))
+    }
+
    private fun isUserLoggedInFlow(): Flow<Boolean> {
        return combine(
            authenticationService.isLoggedIn(),
@@ -126,37 +128,43 @@ class RootFlowNode @AssistedInject constructor(
            .distinctUntilChanged()
    }

-    private fun switchToLoggedInFlow(sessionId: SessionId, cacheIndex: Int) {
-        backstack.safeRoot(NavTarget.LoggedInFlow(sessionId, cacheIndex))
-    }
-
    private fun switchToNotLoggedInFlow() {
        matrixClientsHolder.removeAll()
        backstack.safeRoot(NavTarget.NotLoggedInFlow)
    }

+    private suspend fun restoreSessionIfNeeded(
+        sessionId: SessionId,
+        onFailure: () -> Unit = {},
+        onSuccess: (SessionId) -> Unit = {},
+    ) {
+        // 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
+        }
+        authenticationService.restoreSession(sessionId)
+            .onSuccess { matrixClient ->
+                matrixClientsHolder.add(matrixClient)
+                Timber.v("Succeed to restore session $sessionId")
+                onSuccess(sessionId)
+            }
+            .onFailure {
+                Timber.v("Failed to restore session $sessionId")
+                onFailure()
+            }
+    }
+
    private suspend fun tryToRestoreLatestSession(
-        onSuccess: (UserId) -> Unit = {},
+        onSuccess: (SessionId) -> Unit = {},
        onFailure: () -> Unit = {}
    ) {
-        val latestKnownUserId = authenticationService.getLatestSessionId()
-        if (latestKnownUserId == null) {
+        val latestSessionId = authenticationService.getLatestSessionId()
+        if (latestSessionId == null) {
            onFailure()
            return
        }
-        if (matrixClientsHolder.knowSession(latestKnownUserId)) {
-            onSuccess(latestKnownUserId)
-            return
-        }
-        authenticationService.restoreSession(UserId(latestKnownUserId.value))
-            .onSuccess { matrixClient ->
-                matrixClientsHolder.add(matrixClient)
-                onSuccess(matrixClient.sessionId)
-            }
-            .onFailure {
-                Timber.v("Failed to restore session...")
-                onFailure()
-            }
+        restoreSessionIfNeeded(latestSessionId, onFailure, onSuccess)
    }

    private fun onOpenBugReport() {
@@ -187,7 +195,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
@@ -198,7 +209,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 {
@@ -259,9 +269,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<Unit> {
+        return clearedCacheEventFlow
+            .onEach { sessionId -> matrixClientsHolder.remove(sessionId) }
+            .map { }
+            .onStart { emit((Unit)) }
+    }
 }
--- 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)
 }
--- 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<Int>
+    val clearedCacheEventFlow: Flow<SessionId>
 }
--- 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<Int> {
-        return cacheIndexState
-    }
+    private val _clearedCacheEventFlow = MutableSharedFlow<SessionId>(0)
+    override val clearedCacheEventFlow: Flow<SessionId> = _clearedCacheEventFlow

-    fun incrementCacheIndex() {
-        cacheIndexState.value++
+    suspend fun onClearedCache(sessionId: SessionId) {
+        _clearedCacheEventFlow.emit(sessionId)
    }
 }
--- 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)
    }
 }
--- 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 <NavTarget : Any> ParentNode<NavTarget>.childNode(navTarget: NavTarget): Node? {
    val childMap = children.value
    val key = childMap.keys.find { it.navTarget == navTarget }
    return childMap[key]?.nodeOrNull
 }
+
+suspend inline fun <reified N : Node, NavTarget : Any> ParentNode<NavTarget>.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()
+        }
+    }