mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-10-31 16:22:49 +00:00
Fix account provisioning deep link being ignored when it cold starts the app
At cold start two writers set the root nav target of RootFlowNode: the intent path (handleIntent -> onLoginLink -> NotLoggedInFlow(params)) and the first nav state emission (NotLoggedIn -> NotLoggedInFlow(null)). safeRoot() only dedupes on strict equality, so the two targets do not cancel each other out and the last writer wins, silently dropping the login params. Keep the params coming from the intent as pending state on the node, and let the nav state observer take them into account, so that the root is derived from both the logged in state and the pending login params, and both orderings converge on the same target. The pending params are cleared when leaving the not logged in flow, so that a later sign out does not resurrect a stale provisioning link. Also log when switching to the not logged in flow, since the drop was silent. The login hint can contain an email address, so only its presence is logged. Closes #7482 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Benoit Marty
@@ -63,11 +63,14 @@ dependencies {
|
||||
testImplementation(projects.features.share.test)
|
||||
testImplementation(projects.libraries.featureflag.test)
|
||||
testImplementation(projects.libraries.matrix.test)
|
||||
testImplementation(projects.libraries.matrixmedia.test)
|
||||
testImplementation(projects.libraries.oauth.test)
|
||||
testImplementation(projects.libraries.permissions.test)
|
||||
testImplementation(projects.libraries.preferences.test)
|
||||
testImplementation(projects.libraries.push.test)
|
||||
testImplementation(projects.libraries.pushproviders.test)
|
||||
testImplementation(projects.libraries.sessionStorage.test)
|
||||
testImplementation(projects.features.announcement.test)
|
||||
testImplementation(projects.features.forward.test)
|
||||
testImplementation(projects.features.messages.test)
|
||||
testImplementation(projects.features.networkmonitor.test)
|
||||
|
||||
@@ -109,6 +109,13 @@ class RootFlowNode(
|
||||
buildContext = buildContext,
|
||||
plugins = plugins
|
||||
) {
|
||||
/**
|
||||
* Login params coming from a launch or new [Intent], waiting to be consumed by the not logged in flow.
|
||||
* Kept here so that the root nav target can be computed from both the logged in state and the pending
|
||||
* login params, whatever the order in which the intent and the first nav state emission are processed.
|
||||
*/
|
||||
private var pendingLoginParams: LoginParams? = null
|
||||
|
||||
override fun onBuilt() {
|
||||
analyticsColdStartWatcher.start()
|
||||
appCoroutineScope.launch {
|
||||
@@ -152,7 +159,7 @@ class RootFlowNode(
|
||||
}
|
||||
}
|
||||
LoggedInState.NotLoggedIn -> {
|
||||
switchToNotLoggedInFlow(null)
|
||||
switchToNotLoggedInFlow(pendingLoginParams)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -194,15 +201,18 @@ class RootFlowNode(
|
||||
}
|
||||
|
||||
private fun switchToLoggedInFlow(sessionId: SessionId, navId: Int) {
|
||||
pendingLoginParams = null
|
||||
backstack.safeRoot(NavTarget.LoggedInFlow(sessionId, navId))
|
||||
}
|
||||
|
||||
private fun switchToNotLoggedInFlow(params: LoginParams?) {
|
||||
Timber.d("switchToNotLoggedInFlow, hasLoginParams=${params != null}")
|
||||
matrixSessionCache.removeAll()
|
||||
backstack.safeRoot(NavTarget.NotLoggedInFlow(params))
|
||||
}
|
||||
|
||||
private fun switchToSignedOutFlow(sessionId: SessionId) {
|
||||
pendingLoginParams = null
|
||||
backstack.safeRoot(NavTarget.SignedOutFlow(sessionId))
|
||||
}
|
||||
|
||||
@@ -319,6 +329,7 @@ class RootFlowNode(
|
||||
}
|
||||
|
||||
override fun onDone() {
|
||||
pendingLoginParams = null
|
||||
backstack.pop()
|
||||
}
|
||||
}
|
||||
@@ -418,6 +429,7 @@ class RootFlowNode(
|
||||
Timber.w("Login link ignored, multi account is disabled")
|
||||
}
|
||||
} else {
|
||||
pendingLoginParams = params
|
||||
switchToNotLoggedInFlow(params)
|
||||
}
|
||||
} else {
|
||||
|
||||
@@ -0,0 +1,267 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
@file:OptIn(ExperimentalCoroutinesApi::class)
|
||||
|
||||
package io.element.android.appnav
|
||||
|
||||
import android.content.Intent
|
||||
import android.net.Uri
|
||||
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.ui.Modifier
|
||||
import com.bumble.appyx.core.modality.AncestryInfo
|
||||
import com.bumble.appyx.core.modality.BuildContext
|
||||
import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel
|
||||
import com.bumble.appyx.core.node.Node
|
||||
import com.bumble.appyx.core.node.ParentNode
|
||||
import com.bumble.appyx.core.node.node
|
||||
import com.bumble.appyx.navmodel.backstack.activeElement
|
||||
import com.bumble.appyx.testing.junit4.util.MainDispatcherRule
|
||||
import com.bumble.appyx.testing.unit.common.helper.parentNodeTestHelper
|
||||
import com.bumble.appyx.utils.customisations.NodeCustomisationDirectoryImpl
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.appnav.di.MatrixSessionCache
|
||||
import io.element.android.appnav.di.SyncOrchestrator
|
||||
import io.element.android.appnav.intent.IntentResolver
|
||||
import io.element.android.appnav.root.RootNavStateFlowFactory
|
||||
import io.element.android.appnav.root.RootPresenter
|
||||
import io.element.android.features.login.api.LoginEntryPoint
|
||||
import io.element.android.features.login.api.LoginParams
|
||||
import io.element.android.features.login.api.accesscontrol.AccountProviderAccessControl
|
||||
import io.element.android.features.login.test.FakeLoginIntentResolver
|
||||
import io.element.android.features.preferences.api.CacheService
|
||||
import io.element.android.features.rageshake.api.bugreport.BugReportEntryPoint
|
||||
import io.element.android.features.rageshake.test.logs.FakeAnnouncementService
|
||||
import io.element.android.features.share.test.FakeShareIntentHandler
|
||||
import io.element.android.features.signedout.api.SignedOutEntryPoint
|
||||
import io.element.android.libraries.accountselect.api.AccountSelectEntryPoint
|
||||
import io.element.android.libraries.architecture.AssistedNodeFactory
|
||||
import io.element.android.libraries.architecture.NodeFactoriesBindings
|
||||
import io.element.android.libraries.architecture.Presenter
|
||||
import io.element.android.libraries.deeplink.api.DeeplinkParser
|
||||
import io.element.android.libraries.di.DependencyInjectionGraphOwner
|
||||
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
|
||||
import io.element.android.libraries.matrix.api.MatrixClient
|
||||
import io.element.android.libraries.matrix.api.core.SessionId
|
||||
import io.element.android.libraries.matrix.test.FakeSdkMetadata
|
||||
import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService
|
||||
import io.element.android.libraries.matrix.test.permalink.FakePermalinkParser
|
||||
import io.element.android.libraries.matrix.ui.media.test.FakeImageLoaderHolder
|
||||
import io.element.android.libraries.oauth.api.OAuthAction
|
||||
import io.element.android.libraries.oauth.api.OAuthActionFlow
|
||||
import io.element.android.libraries.oauth.test.FakeOAuthIntentResolver
|
||||
import io.element.android.libraries.preferences.test.FakeSessionPreferencesStoreFactory
|
||||
import io.element.android.libraries.sessionstorage.api.SessionStore
|
||||
import io.element.android.libraries.sessionstorage.test.InMemorySessionStore
|
||||
import io.element.android.services.analytics.test.FakeAnalyticsService
|
||||
import io.element.android.services.analytics.test.watchers.FakeAnalyticsColdStartWatcher
|
||||
import io.element.android.services.apperror.test.FakeAppErrorStateService
|
||||
import io.element.android.tests.testutils.robolectric.RobolectricTest
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.flow.FlowCollector
|
||||
import kotlinx.coroutines.flow.emptyFlow
|
||||
import kotlinx.coroutines.test.TestScope
|
||||
import kotlinx.coroutines.test.runCurrent
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import kotlin.reflect.KClass
|
||||
|
||||
class RootFlowNodeTest : RobolectricTest() {
|
||||
@get:Rule
|
||||
val instantTaskExecutorRule = InstantTaskExecutorRule()
|
||||
|
||||
@get:Rule
|
||||
val mainDispatcherRule = MainDispatcherRule()
|
||||
|
||||
@Test
|
||||
fun `given no session, when a login link is handled before the nav state is observed, then the login params are kept`() = runTest {
|
||||
val loginEntryPoint = FakeLoginEntryPoint()
|
||||
val rootFlowNode = createRootFlowNode(loginEntryPoint = loginEntryPoint)
|
||||
rootFlowNode.parentNodeTestHelper()
|
||||
// The intent is handled first, this is what happens when the deep link cold starts the app.
|
||||
rootFlowNode.handleIntent(aLoginIntent())
|
||||
assertThat(rootFlowNode.backstack.activeElement).isEqualTo(RootFlowNode.NavTarget.NotLoggedInFlow(A_LOGIN_PARAMS))
|
||||
// Then the first nav state emission lands, it must not override the login params.
|
||||
runCurrent()
|
||||
assertThat(rootFlowNode.backstack.activeElement).isEqualTo(RootFlowNode.NavTarget.NotLoggedInFlow(A_LOGIN_PARAMS))
|
||||
assertThat(loginEntryPoint.params).isEqualTo(A_LOGIN_ENTRY_POINT_PARAMS)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `given no session, when a login link is handled after the nav state is observed, then the login params are applied`() = runTest {
|
||||
val loginEntryPoint = FakeLoginEntryPoint()
|
||||
val rootFlowNode = createRootFlowNode(loginEntryPoint = loginEntryPoint)
|
||||
rootFlowNode.parentNodeTestHelper()
|
||||
// The nav state is observed first, this is what happens when the app is already running.
|
||||
runCurrent()
|
||||
assertThat(rootFlowNode.backstack.activeElement).isEqualTo(RootFlowNode.NavTarget.NotLoggedInFlow(null))
|
||||
rootFlowNode.handleIntent(aLoginIntent())
|
||||
runCurrent()
|
||||
assertThat(rootFlowNode.backstack.activeElement).isEqualTo(RootFlowNode.NavTarget.NotLoggedInFlow(A_LOGIN_PARAMS))
|
||||
assertThat(loginEntryPoint.params).isEqualTo(A_LOGIN_ENTRY_POINT_PARAMS)
|
||||
}
|
||||
|
||||
private fun aLoginIntent() = Intent(Intent.ACTION_VIEW, Uri.parse(A_LOGIN_LINK))
|
||||
|
||||
private fun TestScope.createRootFlowNode(
|
||||
loginEntryPoint: LoginEntryPoint = FakeLoginEntryPoint(),
|
||||
sessionStore: SessionStore = InMemorySessionStore(),
|
||||
): RootFlowNode {
|
||||
val matrixSessionCache = MatrixSessionCache(
|
||||
authenticationService = FakeMatrixAuthenticationService(),
|
||||
syncOrchestratorFactory = FakeSyncOrchestratorFactory(),
|
||||
analyticsService = FakeAnalyticsService(),
|
||||
)
|
||||
val parentNode = FakeParentNode(
|
||||
graph = FakeNodeFactoriesBindings(
|
||||
mapOf(
|
||||
NotLoggedInFlowNode::class to AssistedNodeFactory { buildContext, plugins ->
|
||||
NotLoggedInFlowNode(
|
||||
buildContext = buildContext,
|
||||
plugins = plugins,
|
||||
loginEntryPoint = loginEntryPoint,
|
||||
imageLoaderHolder = FakeImageLoaderHolder(),
|
||||
analyticsColdStartWatcher = FakeAnalyticsColdStartWatcher(),
|
||||
)
|
||||
}
|
||||
)
|
||||
)
|
||||
)
|
||||
return RootFlowNode(
|
||||
buildContext = BuildContext(
|
||||
ancestryInfo = AncestryInfo.Child(anchor = parentNode),
|
||||
savedStateMap = null,
|
||||
customisations = NodeCustomisationDirectoryImpl(),
|
||||
),
|
||||
plugins = emptyList(),
|
||||
sessionStore = sessionStore,
|
||||
accountProviderAccessControl = FakeAccountProviderAccessControl(),
|
||||
navStateFlowFactory = RootNavStateFlowFactory(
|
||||
sessionStore = sessionStore,
|
||||
cacheService = FakeCacheService(),
|
||||
matrixSessionCache = matrixSessionCache,
|
||||
imageLoaderHolder = FakeImageLoaderHolder(),
|
||||
sessionPreferencesStoreFactory = FakeSessionPreferencesStoreFactory(),
|
||||
),
|
||||
matrixSessionCache = matrixSessionCache,
|
||||
presenter = RootPresenter(
|
||||
crashDetectionPresenter = NotUsedPresenter(),
|
||||
rageshakeDetectionPresenter = NotUsedPresenter(),
|
||||
appErrorStateService = FakeAppErrorStateService(),
|
||||
analyticsService = FakeAnalyticsService(),
|
||||
sdkMetadata = FakeSdkMetadata("sha"),
|
||||
),
|
||||
bugReportEntryPoint = FakeBugReportEntryPoint(),
|
||||
signedOutEntryPoint = FakeSignedOutEntryPoint(),
|
||||
accountSelectEntryPoint = FakeAccountSelectEntryPoint(),
|
||||
intentResolver = IntentResolver(
|
||||
deeplinkParser = DeeplinkParser { null },
|
||||
loginIntentResolver = FakeLoginIntentResolver { A_LOGIN_PARAMS },
|
||||
oAuthIntentResolver = FakeOAuthIntentResolver { null },
|
||||
permalinkParser = FakePermalinkParser(),
|
||||
shareIntentHandler = FakeShareIntentHandler(),
|
||||
),
|
||||
oAuthActionFlow = FakeOAuthActionFlow(),
|
||||
featureFlagService = FakeFeatureFlagService(),
|
||||
announcementService = FakeAnnouncementService(),
|
||||
analyticsService = FakeAnalyticsService(),
|
||||
analyticsColdStartWatcher = FakeAnalyticsColdStartWatcher(),
|
||||
appCoroutineScope = backgroundScope,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private const val A_LOGIN_LINK = "https://mobile.element.io/element/?account_provider=example.com&login_hint=mxid:@alice:example.com"
|
||||
|
||||
private val A_LOGIN_PARAMS = LoginParams(
|
||||
accountProvider = "example.com",
|
||||
loginHint = "mxid:@alice:example.com",
|
||||
)
|
||||
|
||||
private val A_LOGIN_ENTRY_POINT_PARAMS = LoginEntryPoint.Params(
|
||||
accountProvider = A_LOGIN_PARAMS.accountProvider,
|
||||
loginHint = A_LOGIN_PARAMS.loginHint,
|
||||
)
|
||||
|
||||
/**
|
||||
* The presenters are only used when the [Node] is rendered, which does not happen in these tests.
|
||||
*/
|
||||
private class NotUsedPresenter<T> : Presenter<T> {
|
||||
@Composable
|
||||
override fun present(): T = error("Not used")
|
||||
}
|
||||
|
||||
private class FakeCacheService : CacheService {
|
||||
override val clearedCacheEventFlow = emptyFlow<SessionId>()
|
||||
}
|
||||
|
||||
private class FakeSyncOrchestratorFactory : SyncOrchestrator.Factory {
|
||||
override fun create(matrixClient: MatrixClient, sessionCoroutineScope: CoroutineScope): SyncOrchestrator = error("Not used")
|
||||
}
|
||||
|
||||
private class FakeOAuthActionFlow : OAuthActionFlow {
|
||||
override fun post(oAuthAction: OAuthAction) = Unit
|
||||
override suspend fun collect(collector: FlowCollector<OAuthAction?>) = Unit
|
||||
override fun reset() = Unit
|
||||
}
|
||||
|
||||
private class FakeAccountProviderAccessControl : AccountProviderAccessControl {
|
||||
override suspend fun isAllowedToConnectToAccountProvider(accountProviderUrl: String) = true
|
||||
}
|
||||
|
||||
private class FakeBugReportEntryPoint : BugReportEntryPoint {
|
||||
override fun createNode(parentNode: Node, buildContext: BuildContext, callback: BugReportEntryPoint.Callback): Node = node(buildContext) {}
|
||||
}
|
||||
|
||||
private class FakeSignedOutEntryPoint : SignedOutEntryPoint {
|
||||
override fun createNode(parentNode: Node, buildContext: BuildContext, params: SignedOutEntryPoint.Params): Node = node(buildContext) {}
|
||||
}
|
||||
|
||||
private class FakeAccountSelectEntryPoint : AccountSelectEntryPoint {
|
||||
override fun createNode(parentNode: Node, buildContext: BuildContext, callback: AccountSelectEntryPoint.Callback): Node = node(buildContext) {}
|
||||
}
|
||||
|
||||
private class FakeLoginEntryPoint : LoginEntryPoint {
|
||||
var params: LoginEntryPoint.Params? = null
|
||||
private set
|
||||
|
||||
override fun createNode(
|
||||
parentNode: Node,
|
||||
buildContext: BuildContext,
|
||||
params: LoginEntryPoint.Params,
|
||||
callback: LoginEntryPoint.Callback,
|
||||
): Node {
|
||||
this.params = params
|
||||
return node(buildContext) {}
|
||||
}
|
||||
}
|
||||
|
||||
private class FakeNodeFactoriesBindings(
|
||||
private val nodeFactories: Map<KClass<out Node>, AssistedNodeFactory<*>>,
|
||||
) : NodeFactoriesBindings {
|
||||
override fun nodeFactories() = nodeFactories
|
||||
}
|
||||
|
||||
/**
|
||||
* [RootFlowNode] creates its children using the dependency injection graph of its ancestors, so it needs a parent
|
||||
* owning a graph able to create a [NotLoggedInFlowNode].
|
||||
*/
|
||||
private class FakeParentNode(
|
||||
override val graph: Any,
|
||||
) : ParentNode<Unit>(
|
||||
navModel = PermanentNavModel(navTargets = setOf(Unit), savedStateMap = null),
|
||||
buildContext = BuildContext.root(savedStateMap = null),
|
||||
),
|
||||
DependencyInjectionGraphOwner {
|
||||
override fun resolve(navTarget: Unit, buildContext: BuildContext): Node = node(buildContext) {}
|
||||
|
||||
@Composable
|
||||
override fun View(modifier: Modifier) = Unit
|
||||
}
|
||||
Reference in New Issue
Block a user