diff --git a/CLAUDE.md b/CLAUDE.md index c40d3e56e2..b811a101c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -368,10 +368,10 @@ The message is a lambda, so build strings inline without guarding on build type logcat { "Plain info-level message" } // defaults to INFO logcat(LogPriority.DEBUG) { "GraphQL ${operation.name()} START" } // explicit priority logcat(LogPriority.ERROR, throwable) { "Failed to load X: ${throwable.message}" } -logcat(LogPriority.INFO, tag = DEEP_LINK_STACK_DEBUG_TAG) { "…" } // greppable custom tag +logcat(LogPriority.INFO, tag = SOMETHING_DEBUG_TAG) { "…" } // greppable custom tag ``` -Tag is optional — for a one-off log, just omit it (the caller's class name is used). When you want a greppable trace that spans several call sites or files, a shared `const val SOMETHING_DEBUG_TAG = "…"` passed as `tag` everywhere is a handy tool (see `DEEP_LINK_STACK_DEBUG_TAG` usage across `MainActivity`/`BackstackController`). It's a convenience, not a requirement; don't introduce a const for a single isolated log. +Tag is optional — for a one-off log, just omit it (the caller's class name is used). When you want a greppable trace that spans several call sites or files, a shared `const val SOMETHING_DEBUG_TAG = "…"` passed as `tag` everywhere is a handy tool. It's a convenience, not a requirement; don't introduce a const for a single isolated log. There is also an Apollo overload, `logcat(priority, operationError: ApolloOperationError, tag, message)`, which auto-downgrades to at most `WARN` for unauthenticated errors. Use it when logging a failed `safeExecute`/`safeFlow` result. diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt b/app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt index 5933b9d267..dc3f856247 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/MainActivity.kt @@ -121,9 +121,16 @@ class MainActivity : AppCompatActivity() { * top of [onCreate]; the controller, session reconciler and merged ViewModel factory are read off it. */ private val navRetainedViewModel: NavRetainedViewModel by lazy { + // Seed isOwnTask with isTaskRoot at construction so the controller never starts from a guessed + // default; it is refreshed authoritatively on every onResume (see refreshIsOwnTask). Captured as a + // value here because the initializer lambda's receiver is not this Activity. Only used on the + // genuine first creation — a config change reuses the retained instance and skips the initializer. + val isOwnTaskAtCreation = isTaskRoot ViewModelProvider( this, - viewModelFactory { initializer { NavRetainedViewModel((application as HedvigApplication).appGraph) } }, + viewModelFactory { + initializer { NavRetainedViewModel((application as HedvigApplication).appGraph, isOwnTaskAtCreation) } + }, )[NavRetainedViewModel::class.java] } @@ -197,6 +204,16 @@ class MainActivity : AppCompatActivity() { } } } + val launchedFromHistory = intent.flags and Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY != 0 + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "MainActivity.onCreate launch context: " + + "isColdStart(savedInstanceState==null)=${savedInstanceState == null} | " + + "isTaskRoot=$isTaskRoot | " + + "intent.action=${intent.action} | " + + "intent.data=${intent.data} | " + + "launchedFromHistory=$launchedFromHistory | " + + "intent.flags=0x${Integer.toHexString(intent.flags)}" + } if (savedInstanceState == null) { handleDeepLinkIntent(intent) } @@ -267,8 +284,24 @@ class MainActivity : AppCompatActivity() { * in onCreate — there is no shared, process-wide controller that a backgrounded Activity could * steal, which is why no resume/top-resume re-attachment is needed. */ + override fun onResume() { + super.onResume() + refreshIsOwnTask() + } + + /** + * Pushes the current [isTaskRoot] into the controller. Unlike the one-time hooks in + * [attachBackstackTaskHooks], this is a *value* that can change over the Activity's life (e.g. an + * Activity below us finishes, or [isTaskRoot] reads more reliably once resumed than at onCreate), so + * it is refreshed on every onResume to keep the snapshot-backed value honest — otherwise the + * lone-deep-link chrome can latch a stale "not own task" reading. See [BackstackController.isOwnTask]. + */ + private fun refreshIsOwnTask() { + backstackController.isOwnTask = isTaskRoot + } + private fun attachBackstackTaskHooks() { - backstackController.isOwnTask = { isTaskRoot } + refreshIsOwnTask() backstackController.escapeToOwnTask = { parentStack -> NavigationStateBridge.escapeToOwnTask(this@MainActivity, parentStack, serializersModules) } @@ -278,10 +311,18 @@ class MainActivity : AppCompatActivity() { private fun handleDeepLinkIntent(intent: Intent) { if (intent.action != Intent.ACTION_VIEW) return val uri = intent.data?.toString() ?: return + val launchedFromHistory = intent.flags and Intent.FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY != 0 + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "handleDeepLinkIntent forwarding ACTION_VIEW uri=$uri | launchedFromHistory=$launchedFromHistory " + + "(launchedFromHistory=true => STALE re-delivered deep link, NOT a fresh user action)" + } deepLinkChannel.trySend(uri) } } +/** Shared logcat tag for the cold-start/login deep-link-stack investigation. Grep this to follow the flow. */ +internal const val DEEP_LINK_STACK_DEBUG_TAG = "DeepLinkStackDebug" + /** * Applies the theme in two ways: * 1. Uses UiModeManager to persist the last theme selected so that on new launches the splash screen matches the theme diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/NavigationStateBridge.kt b/app/app/src/main/kotlin/com/hedvig/android/app/NavigationStateBridge.kt index dc85033309..b3872e7b01 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/NavigationStateBridge.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/NavigationStateBridge.kt @@ -8,10 +8,13 @@ import androidx.savedstate.serialization.decodeFromSavedState import androidx.savedstate.serialization.encodeToSavedState import com.hedvig.android.app.navigation.BackstackController import com.hedvig.android.feature.login.navigation.LoginKey +import com.hedvig.android.logger.LogPriority +import com.hedvig.android.logger.logcat import com.hedvig.android.navigation.common.HedvigNavKey import com.hedvig.android.navigation.common.StashedSession import com.hedvig.android.navigation.common.TopLevelTab import com.hedvig.android.navigation.compose.merge +import kotlin.time.Duration.Companion.minutes import kotlinx.serialization.Polymorphic import kotlinx.serialization.PolymorphicSerializer import kotlinx.serialization.Serializable @@ -33,6 +36,14 @@ internal object NavigationStateBridge { private const val EXTRA_RESTORE_STACK = "com.hedvig.android.app.RESTORE_STACK" private const val NAV_STATE_REGISTRY_KEY = "com.hedvig.android.app.NAV_STATE" + /** + * How long a [BackstackController.pendingDeepLink] may survive a process death and still be landed at + * the next login. Covers a real "killed mid-login" window (switching to BankID, OTP, etc., is seconds + * to a few minutes) while discarding a link stashed long ago, which would otherwise bleed into an + * unrelated later login and strand the member on a lone deep-link stack. See [restoreAndPersist]. + */ + private val MAX_PENDING_DEEP_LINK_AGE = 30.minutes + private val handoffSerializer = ListSerializer(PolymorphicSerializer(HedvigNavKey::class)) /** @@ -63,18 +74,49 @@ internal object NavigationStateBridge { null } if (!handoff.isNullOrEmpty()) { + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "NavigationStateBridge.restoreAndPersist: escape-to-own-task handoff present, reseeding with $handoff" + } backstackController.reseed(handoff) } else { - savedStateRegistry.consumeRestoredStateForKey(NAV_STATE_REGISTRY_KEY) + val snapshot = savedStateRegistry.consumeRestoredStateForKey(NAV_STATE_REGISTRY_KEY) ?.let { decodeFromSavedState(NavStateSnapshot.serializer(), it, savedStateConfiguration) } - ?.let { snapshot -> - backstackController.restoreFromSavedState( - entries = snapshot.entries, - parkedRuns = snapshot.parkedRuns, - pendingDeepLink = snapshot.pendingDeepLink, - stashedSession = snapshot.stashedSession, - ) + if (snapshot != null) { + // Discard a pending deep link that is too old to still belong to an in-flight login, so it + // can't bleed into an unrelated later login as a lone deep-link stack. + val now = System.currentTimeMillis() + val stashedAt = snapshot.pendingDeepLinkStashedAtEpochMs + val ageMs = if (stashedAt != null) now - stashedAt else null + val pendingStillValid = snapshot.pendingDeepLink != null && + isPendingDeepLinkStashTimeFresh(stashedAt, now) + val restoredPending = snapshot.pendingDeepLink.takeIf { pendingStillValid } + val restoredPendingStashedAt = stashedAt.takeIf { pendingStillValid } + if (snapshot.pendingDeepLink != null && !pendingStillValid) { + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "NavigationStateBridge.restoreAndPersist: DROPPING stale restored " + + "pendingDeepLink=${snapshot.pendingDeepLink} (ageMs=$ageMs, max=$MAX_PENDING_DEEP_LINK_AGE) " + + "— it would otherwise have landed as a lone deep-link stack at the next login" + } + } + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "NavigationStateBridge.restoreAndPersist: restoring snapshot from SavedStateRegistry " + + "(process-death restore): entries=${snapshot.entries} | " + + "pendingDeepLink=$restoredPending (raw=${snapshot.pendingDeepLink}, ageMs=$ageMs) | " + + "stashedSession.member=${snapshot.stashedSession?.memberId} | " + + "parkedRuns=${snapshot.parkedRuns.keys}" + } + backstackController.restoreFromSavedState( + entries = snapshot.entries, + parkedRuns = snapshot.parkedRuns, + pendingDeepLink = restoredPending, + pendingDeepLinkStashedAtEpochMs = restoredPendingStashedAt, + stashedSession = snapshot.stashedSession, + ) + } else { + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "NavigationStateBridge.restoreAndPersist: no saved snapshot to restore (isColdStart=$isColdStart)" } + } backstackController.seedIfEmpty(listOf(LoginKey)) } @@ -87,6 +129,7 @@ internal object NavigationStateBridge { entries = backstackController.entries.toList(), parkedRuns = backstackController.parkedRuns.toMap(), pendingDeepLink = backstackController.pendingDeepLink, + pendingDeepLinkStashedAtEpochMs = backstackController.pendingDeepLinkStashedAtEpochMs, stashedSession = backstackController.stashedSession, ), savedStateConfiguration, @@ -124,12 +167,28 @@ internal object NavigationStateBridge { private fun json(serializersModules: Set): Json = Json { serializersModule = serializersModules.merge() } + + /** + * Whether a pending deep link stashed at [stashedAtEpochMs] is recent enough (as of [nowEpochMs]) to + * still be landed after a process-death restore. `null` (no timestamp — e.g. a pre-upgrade snapshot) + * and a negative age (clock moved backwards) both count as not-fresh, so the link is dropped rather + * than risk landing a stale one. Pure and clock-injected so it is unit-testable without Android. + */ + internal fun isPendingDeepLinkStashTimeFresh(stashedAtEpochMs: Long?, nowEpochMs: Long): Boolean { + if (stashedAtEpochMs == null) return false + val ageMs = nowEpochMs - stashedAtEpochMs + return ageMs in 0..MAX_PENDING_DEEP_LINK_AGE.inWholeMilliseconds + } } /** * The full hoisted navigation state, serialized into the Activity's SavedStateRegistry so the - * in-memory [BackstackController] singleton can be re-hydrated after process death. Mirrors the four + * in-memory [BackstackController] singleton can be re-hydrated after process death. Mirrors the * holders the controller owns. + * + * [pendingDeepLinkStashedAtEpochMs] defaults to `null` so snapshots written before this field existed + * still decode; a missing timestamp means an age can't be computed, so such a restored pending link is + * treated as stale and dropped (see [NavigationStateBridge.restoreAndPersist]). */ @Serializable private data class NavStateSnapshot( @@ -137,4 +196,5 @@ private data class NavStateSnapshot( val parkedRuns: Map>, val pendingDeepLink: (@Polymorphic HedvigNavKey)?, val stashedSession: StashedSession?, + val pendingDeepLinkStashedAtEpochMs: Long? = null, ) diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/BackstackController.kt b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/BackstackController.kt index e1c904655b..7f10a66966 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/BackstackController.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/BackstackController.kt @@ -3,13 +3,17 @@ package com.hedvig.android.app.navigation import androidx.compose.runtime.MutableState import androidx.compose.runtime.Stable import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshots.Snapshot import androidx.compose.runtime.snapshots.SnapshotStateList import androidx.compose.runtime.snapshots.SnapshotStateMap +import com.hedvig.android.app.DEEP_LINK_STACK_DEBUG_TAG import com.hedvig.android.app.ui.startDestination import com.hedvig.android.feature.home.home.navigation.HomeKey import com.hedvig.android.feature.login.navigation.LoginKey +import com.hedvig.android.logger.LogPriority +import com.hedvig.android.logger.logcat import com.hedvig.android.navigation.common.DeliberateLogoutOrigin import com.hedvig.android.navigation.common.HedvigNavKey import com.hedvig.android.navigation.common.StashedSession @@ -39,18 +43,10 @@ internal class BackstackController( internal val parkedRuns: SnapshotStateMap>, pendingDeepLinkState: MutableState, stashedSessionState: MutableState, - /** - * Whether this activity is the root of its own task. `false` means we were launched into the - * caller's task by an external deep link, so an Up press must escape into our own task rather than - * rebuilding the ancestry in place (which would leave our screens hosted under the foreign app). - * - * Attached by `MainActivity` in `onCreate` (see `attachBackstackTaskHooks`), not at construction: the - * controller is owned by this Activity's retained `ViewModel` and is 1:1 with the Activity, so there - * is no shared, process-wide controller a backgrounded Activity could steal — the hook is wired once - * at creation, with no resume re-attachment needed. Defaults to `true` so unit tests and any - * pre-attach use stay fully in-process. - */ - var isOwnTask: () -> Boolean = { true }, + // Defaulted (and placed after the required states) so the many positional test constructions need no + // change; the real construction in NavRetainedViewModel passes it explicitly by name. + pendingDeepLinkStashedAtState: MutableState = mutableStateOf(null), + initialIsOwnTask: Boolean = true, /** * Re-roots the app in its own task seeded with the given stack (see [navigateUp]). The Activity * owns the mechanics (relaunch with NEW_TASK|CLEAR_TASK + finish); the controller only supplies @@ -65,12 +61,49 @@ internal class BackstackController( */ var finishApp: () -> Unit = {}, ) : Backstack { + /** + * Whether this Activity is the root of its own task. `false` means we were launched into the + * caller's task by an external deep link, so an Up press must escape into our own task rather than + * rebuilding the ancestry in place (which would leave our screens hosted under the foreign app). + * + * Snapshot-backed on purpose, rather than a live `() -> isTaskRoot` lambda: [loneDeepLinkChrome] + * derives the whole back-arrow-vs-nav-bar decision partly from this, so it must (a) be observable so + * Compose recomposes the chrome when it changes, and (b) never sample a stale value. `MainActivity` + * pushes `isTaskRoot` here in `onCreate` (see `attachBackstackTaskHooks`) AND on every `onResume` — + * onResume because the value can settle or change after creation (an Activity below us finishing, or + * an early-launch read returning a not-yet-settled value). Defaults to `true` so unit tests and any + * pre-attach use stay fully in-process. + */ + var isOwnTask: Boolean by mutableStateOf(initialIsOwnTask) + /** * A deep link resolved while logged out, held until [setLoggedIn] consumes it (so it can land - * alone). Persisted across rotation / process death (e.g. mid-OTP) — see the class KDoc. + * alone). Persisted across rotation / process death (e.g. mid-OTP) — see the class KDoc. Always set + * via [stashPendingDeepLink] so its companion timestamp [pendingDeepLinkStashedAtEpochMs] stays in + * sync; cleared (with the timestamp) when consumed by [setLoggedIn] or wiped by [reseed]. */ internal var pendingDeepLink: HedvigNavKey? by pendingDeepLinkState + /** + * Wall-clock time ([System.currentTimeMillis]) when [pendingDeepLink] was last stashed, or `null` + * when there is none. Persisted alongside [pendingDeepLink] so a process-death restore can discard a + * link that is too old to still belong to the in-flight login (see `NavigationStateBridge`), which + * stops a long-abandoned pending link from bleeding into an unrelated later login. + */ + internal var pendingDeepLinkStashedAtEpochMs: Long? by pendingDeepLinkStashedAtState + + /** Stashes [key] as the [pendingDeepLink], stamping [pendingDeepLinkStashedAtEpochMs] with now. */ + private fun stashPendingDeepLink(key: HedvigNavKey) { + pendingDeepLink = key + pendingDeepLinkStashedAtEpochMs = System.currentTimeMillis() + } + + /** Clears [pendingDeepLink] and its companion timestamp together. */ + private fun clearPendingDeepLink() { + pendingDeepLink = null + pendingDeepLinkStashedAtEpochMs = null + } + /** * The previous logged-in session, held between logout and the next login. Excluded from * [allLiveContentKeys] on purpose — see [StashedSession]. Persisted across process death. @@ -111,6 +144,9 @@ internal class BackstackController( */ private val owningTabByContentKey = mutableMapOf() + /** Last value logged by [loneDeepLinkChrome], so the (frequently-read) getter only logs on change. */ + private var lastLoggedChrome: LoneDeepLinkChrome? = null + fun owningTopLevelTabForContentKey(contentKey: Any?): TopLevelTab? { if (contentKey == null) return null var tab: TopLevelTab? = null @@ -139,13 +175,24 @@ internal class BackstackController( val loneDeepLinkChrome: LoneDeepLinkChrome get() { val first = entries.firstOrNull() - val insideRunsModel = isOwnTask() && (first is HomeKey || first is LoginKey) - if (insideRunsModel) return LoneDeepLinkChrome.ShowSuite - return if (entries.lastOrNull()?.topLevelTabOrNull() != null) { - LoneDeepLinkChrome.ShowUpBar - } else { - LoneDeepLinkChrome.ShowNothing + val ownTask = isOwnTask + val insideRunsModel = ownTask && (first is HomeKey || first is LoginKey) + val result = when { + insideRunsModel -> LoneDeepLinkChrome.ShowSuite + entries.lastOrNull()?.topLevelTabOrNull() != null -> LoneDeepLinkChrome.ShowUpBar + else -> LoneDeepLinkChrome.ShowNothing + } + if (result != lastLoggedChrome) { + lastLoggedChrome = result + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "BackstackController.loneDeepLinkChrome -> $result | isOwnTask=$ownTask | " + + "first=${first?.let { it::class.simpleName }} | " + + "last=${entries.lastOrNull()?.let { it::class.simpleName }} | " + + "pendingDeepLink=$pendingDeepLink | entries=${entries.toList()} " + + "(ShowUpBar with isOwnTask=false on a normal Home stack => NO deep link needed)" + } } + return result } /** @@ -181,7 +228,11 @@ internal class BackstackController( */ fun navigateToInAppLink(key: HedvigNavKey) { if (!isLoggedIn) { - pendingDeepLink = key + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "BackstackController.navigateToInAppLink while LOGGED OUT -> stashing pendingDeepLink=$key " + + "(will land ALONE after login)" + } + stashPendingDeepLink(key) return } Snapshot.withMutableSnapshot { @@ -199,9 +250,16 @@ internal class BackstackController( */ fun navigateToExternalDeepLink(key: HedvigNavKey) { if (!isLoggedIn) { - pendingDeepLink = key + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "BackstackController.navigateToExternalDeepLink while LOGGED OUT -> stashing pendingDeepLink=$key " + + "(will land ALONE after login)" + } + stashPendingDeepLink(key) return } + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "BackstackController.navigateToExternalDeepLink while LOGGED IN -> reseed to lone [$key]" + } reseed(listOf(key)) } @@ -222,11 +280,11 @@ internal class BackstackController( val parentStack = synthetic.dropLast(1) when { parentStack.isNotEmpty() -> { - if (isOwnTask()) entries.replaceWith(parentStack) else escapeToOwnTask(parentStack) + if (isOwnTask) entries.replaceWith(parentStack) else escapeToOwnTask(parentStack) return true } - !isOwnTask() -> { + !isOwnTask -> { escapeToOwnTask(synthetic) return true } @@ -265,21 +323,32 @@ internal class BackstackController( fun setLoggedIn(memberId: String?) { Snapshot.withMutableSnapshot { val pending = pendingDeepLink - pendingDeepLink = null + clearPendingDeepLink() val stash = stashedSession?.takeIf { memberId != null && it.memberId == memberId } stashedSession = null parkedRuns.clear() when { pending != null -> { + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "BackstackController.setLoggedIn: landing pendingDeepLink ALONE -> entries=[$pending] " + + "(memberId=$memberId). THIS is the lone/deep-link stack (back arrow, no nav bar)." + } entries.replaceWith(listOf(pending)) } stash != null -> { + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "BackstackController.setLoggedIn: restoring stashed session (memberId=$memberId) -> " + + "entries=${stash.entries}" + } parkedRuns.putAll(stash.parkedRuns) entries.replaceWith(stash.entries) } else -> { + logcat(priority = LogPriority.VERBOSE, tag = DEEP_LINK_STACK_DEBUG_TAG) { + "BackstackController.setLoggedIn: fresh Home root (memberId=$memberId)" + } entries.replaceWith(listOf(HomeKey)) } } @@ -328,7 +397,7 @@ internal class BackstackController( Snapshot.withMutableSnapshot { entries.replaceWith(stack) parkedRuns.clear() - pendingDeepLink = null + clearPendingDeepLink() stashedSession = null } } @@ -342,6 +411,7 @@ internal class BackstackController( entries: List, parkedRuns: Map>, pendingDeepLink: HedvigNavKey?, + pendingDeepLinkStashedAtEpochMs: Long?, stashedSession: StashedSession?, ) { if (this.entries.isNotEmpty()) return @@ -349,6 +419,7 @@ internal class BackstackController( this.entries.addAll(entries) this.parkedRuns.putAll(parkedRuns) this.pendingDeepLink = pendingDeepLink + this.pendingDeepLinkStashedAtEpochMs = pendingDeepLinkStashedAtEpochMs this.stashedSession = stashedSession } } diff --git a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/NavRetainedViewModel.kt b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/NavRetainedViewModel.kt index 8a97bfd36a..16fd9fe8c7 100644 --- a/app/app/src/main/kotlin/com/hedvig/android/app/navigation/NavRetainedViewModel.kt +++ b/app/app/src/main/kotlin/com/hedvig/android/app/navigation/NavRetainedViewModel.kt @@ -20,13 +20,18 @@ import dev.zacsweers.metrox.viewmodel.MetroViewModelFactory * spins up a per-Activity [ActivityRetainedGraph] seeded with that controller. Everything that must * talk to this Activity's stack — the [sessionReconciler] and every back-stack ViewModel resolved by * [viewModelFactory] — comes from that extension, so two `MainActivity` instances never share state. + * + * [isOwnTask] seeds [BackstackController.isOwnTask] with the Activity's `isTaskRoot` at first creation + * (so it never starts from a guessed default); `MainActivity` refreshes it authoritatively on resume. */ -internal class NavRetainedViewModel(appGraph: AppGraph) : ViewModel() { +internal class NavRetainedViewModel(appGraph: AppGraph, isOwnTask: Boolean) : ViewModel() { val backstackController: BackstackController = BackstackController( entries = mutableStateListOf(), parkedRuns = mutableStateMapOf(), pendingDeepLinkState = mutableStateOf(null), + pendingDeepLinkStashedAtState = mutableStateOf(null), stashedSessionState = mutableStateOf(null), + initialIsOwnTask = isOwnTask, ) private val activityGraph: ActivityRetainedGraph = diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/NavigationStateBridgeTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/NavigationStateBridgeTest.kt new file mode 100644 index 0000000000..31d3a46e9c --- /dev/null +++ b/app/app/src/test/kotlin/com/hedvig/android/app/NavigationStateBridgeTest.kt @@ -0,0 +1,50 @@ +package com.hedvig.android.app + +import assertk.assertThat +import assertk.assertions.isFalse +import assertk.assertions.isTrue +import org.junit.Test + +internal class NavigationStateBridgeTest { + private val thirtyMinutesMs = 30L * 60L * 1000L + + @Test + fun `a missing stash timestamp is treated as not fresh`() { + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(null, nowEpochMs = 1_000_000L)) + .isFalse() + } + + @Test + fun `a just-stashed pending link is fresh`() { + val now = 1_000_000L + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(now, nowEpochMs = now)).isTrue() + } + + @Test + fun `a link stashed just under the limit is fresh`() { + val now = 5_000_000L + val stashedAt = now - (thirtyMinutesMs - 1) + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(stashedAt, nowEpochMs = now)).isTrue() + } + + @Test + fun `a link stashed exactly at the limit is still fresh`() { + val now = 5_000_000L + val stashedAt = now - thirtyMinutesMs + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(stashedAt, nowEpochMs = now)).isTrue() + } + + @Test + fun `a link stashed past the limit is stale`() { + val now = 5_000_000L + val stashedAt = now - (thirtyMinutesMs + 1) + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(stashedAt, nowEpochMs = now)).isFalse() + } + + @Test + fun `a negative age from a backwards clock is treated as not fresh`() { + val now = 5_000_000L + val stashedAt = now + 60_000L // stashed "in the future" relative to now + assertThat(NavigationStateBridge.isPendingDeepLinkStashTimeFresh(stashedAt, nowEpochMs = now)).isFalse() + } +} diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BackstackControllerTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BackstackControllerTest.kt index 363de7ab55..8c748dbb51 100644 --- a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BackstackControllerTest.kt +++ b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/BackstackControllerTest.kt @@ -9,6 +9,8 @@ import assertk.assertions.containsExactlyInAnyOrder import assertk.assertions.isEmpty import assertk.assertions.isEqualTo import assertk.assertions.isFalse +import assertk.assertions.isNotNull +import assertk.assertions.isNull import assertk.assertions.isTrue import com.hedvig.android.feature.help.center.navigation.HelpCenterKey import com.hedvig.android.feature.home.home.navigation.HomeKey @@ -16,13 +18,18 @@ import com.hedvig.android.feature.insurances.navigation.InsurancesKey import com.hedvig.android.feature.login.navigation.LoginKey import com.hedvig.android.feature.payments.navigation.PaymentsKey import com.hedvig.android.feature.profile.navigation.ProfileKey +import com.hedvig.android.logger.TestLogcatLoggingRule import com.hedvig.android.navigation.common.HedvigNavKey import com.hedvig.android.navigation.common.TopLevelTab import com.hedvig.android.navigation.compose.LoneDeepLinkChrome import com.hedvig.android.navigation.compose.popUpTo +import org.junit.Rule import org.junit.Test internal class BackstackControllerTest { + @get:Rule + val testLogcatLogger = TestLogcatLoggingRule() + private fun controllerWith(vararg keys: HedvigNavKey) = BackstackController( mutableStateListOf(*keys), mutableStateMapOf(), @@ -272,7 +279,7 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { false }, + initialIsOwnTask = false, escapeToOwnTask = { escaped = it }, ) assertThat(controller.navigateUp()).isTrue() @@ -289,7 +296,7 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { true }, + initialIsOwnTask = true, escapeToOwnTask = { escaped = it }, ) assertThat(controller.navigateUp()).isTrue() @@ -404,6 +411,41 @@ internal class BackstackControllerTest { assertThat(controller.pendingDeepLink).isEqualTo(HelpCenterKey) } + @Test + fun `stashing a pending deep link records a timestamp, and consuming it at login clears both`() { + val controller = controllerWith(LoginKey) + controller.navigateToExternalDeepLink(HelpCenterKey) + assertThat(controller.pendingDeepLink).isEqualTo(HelpCenterKey) + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isNotNull() + controller.setLoggedIn("mem-1") + assertThat(controller.pendingDeepLink).isNull() + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isNull() + } + + @Test + fun `reseed clears the pending deep link and its timestamp`() { + val controller = controllerWith(LoginKey) + controller.navigateToInAppLink(HelpCenterKey) + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isNotNull() + controller.reseed(listOf(HomeKey)) + assertThat(controller.pendingDeepLink).isNull() + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isNull() + } + + @Test + fun `restoreFromSavedState round-trips the pending deep link timestamp`() { + val controller = controllerWith() // empty so the restore applies (live state would otherwise win) + controller.restoreFromSavedState( + entries = listOf(LoginKey), + parkedRuns = emptyMap(), + pendingDeepLink = HelpCenterKey, + pendingDeepLinkStashedAtEpochMs = 123_456L, + stashedSession = null, + ) + assertThat(controller.pendingDeepLink).isEqualTo(HelpCenterKey) + assertThat(controller.pendingDeepLinkStashedAtEpochMs).isEqualTo(123_456L) + } + @Test fun `loneDeepLinkChrome is ShowUpBar for a lone tab root`() { assertThat(controllerWith(InsurancesKey).loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowUpBar) @@ -445,7 +487,7 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { false }, + initialIsOwnTask = false, ) assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowUpBar) } @@ -457,11 +499,24 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { true }, + initialIsOwnTask = true, ) assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowSuite) } + @Test + fun `toggling isOwnTask flips a lone Home between suite and up bar`() { + // Guards the no-deep-link path: a normal lone Home renders as a deep-link stack (up bar) whenever + // isTaskRoot is false, and must self-correct once isOwnTask is refreshed back to true (the onResume + // fix). Also proves isOwnTask is read live, not frozen at construction. + val controller = controllerWith(HomeKey) + assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowSuite) + controller.isOwnTask = false + assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowUpBar) + controller.isOwnTask = true + assertThat(controller.loneDeepLinkChrome).isEqualTo(LoneDeepLinkChrome.ShowSuite) + } + @Test fun `navigateUp from a lone deep link drops the leaf and exposes the rebuilt ancestry`() { val controller = controllerWith(InsurancesKey) @@ -478,7 +533,7 @@ internal class BackstackControllerTest { mutableStateMapOf(), mutableStateOf(null), // pendingDeepLink mutableStateOf(null), // stashedSession - isOwnTask = { false }, + initialIsOwnTask = false, escapeToOwnTask = { escaped = it }, ) assertThat(controller.navigateUp()).isTrue() diff --git a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/DeepLinkNavigationTest.kt b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/DeepLinkNavigationTest.kt index c1f8e35e69..a1fe80faf4 100644 --- a/app/app/src/test/kotlin/com/hedvig/android/app/navigation/DeepLinkNavigationTest.kt +++ b/app/app/src/test/kotlin/com/hedvig/android/app/navigation/DeepLinkNavigationTest.kt @@ -8,7 +8,9 @@ import assertk.assertions.containsExactly import com.hedvig.android.feature.help.center.navigation.HelpCenterKey import com.hedvig.android.feature.home.home.navigation.HomeKey import com.hedvig.android.feature.insurances.navigation.InsurancesKey +import com.hedvig.android.logger.TestLogcatLoggingRule import com.hedvig.android.navigation.common.HedvigNavKey +import org.junit.Rule import org.junit.Test /** @@ -18,6 +20,9 @@ import org.junit.Test * to a key already on the stack must therefore never blind-append. */ internal class DeepLinkNavigationTest { + @get:Rule + val testLogcatLogger = TestLogcatLoggingRule() + private fun controllerWith(vararg keys: HedvigNavKey) = BackstackController( mutableStateListOf(*keys), mutableStateMapOf(),