diff --git a/CHANGELOG.md b/CHANGELOG.md index 5886cb0c6d..1dc80fd9db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +### Improvements + +- Support Navigation 3 integration for multi-Activity applications, where each Activity has its own `SentryNavEffect` ([#6229](https://github.com/getsentry/sentry-java/pull/6229)) + - Limitations: Does not include support for applications where multiple `SentryNavEffect`s are meant to be simultaneously active, whether nested or as siblings (e.g., [multi-resume mode](https://developer.android.com/develop/ui/views/layout/support-multi-window-mode)) + ### Dependencies - Bump Native SDK from v0.17.2 to v0.17.3 ([#6250](https://github.com/getsentry/sentry-java/pull/6250)) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7b31df47f6..9045835feb 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -16,6 +16,7 @@ gummyBears = "0.12.0" java8Signature = "1.0" jackson = "2.18.3" jetbrainsCompose = "1.6.11" +jetbrainsLifecycle = "2.8.4" kotlin = "2.3.21" kotlin-compatible-version = "1.9" ksp = "2.3.9" @@ -139,6 +140,7 @@ jackson-bom = { module = "com.fasterxml.jackson:jackson-bom", version.ref = "jac jackson-databind = { module = "com.fasterxml.jackson.core:jackson-databind", version.ref = "jackson" } jackson-kotlin = { module = "com.fasterxml.jackson.module:jackson-module-kotlin", version.ref = "jackson" } jetbrains-annotations = { module = "org.jetbrains:annotations", version = "23.0.0" } +jetbrains-lifecycle-runtime-compose = { module = "org.jetbrains.androidx.lifecycle:lifecycle-runtime-compose", version.ref = "jetbrainsLifecycle" } kotlin-bom = { module = "org.jetbrains.kotlin:kotlin-bom", version.ref = "kotlin" } kotlin-test-junit = { module = "org.jetbrains.kotlin:kotlin-test-junit", version.ref = "kotlin" } kotlinx-coroutines = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "coroutines" } diff --git a/sentry-android-navigation3/build.gradle.kts b/sentry-android-navigation3/build.gradle.kts index da7ad9f36f..83d9a5d462 100644 --- a/sentry-android-navigation3/build.gradle.kts +++ b/sentry-android-navigation3/build.gradle.kts @@ -62,6 +62,7 @@ dependencies { implementation(projects.sentry) compileOnly(libs.androidx.compose.runtime) + compileOnly(libs.jetbrains.lifecycle.runtime.compose) testImplementation(libs.androidx.compose.ui.test.junit4) testImplementation(libs.androidx.test.core) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 37f694350f..93a1178091 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -21,6 +21,8 @@ import io.sentry.protocol.TransactionNameSource import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import java.lang.ref.WeakReference +internal const val NAVIGATION_CONTEXT_KEY: String = "Navigation" + private const val NAVIGATION_OP: String = "navigation" /** @@ -374,7 +376,6 @@ private class NavContext(private val scopes: IScopes, private val options: Sentr private companion object { private const val BACKSTACK_KEY = "Back Stack" - private const val NAVIGATION_CONTEXT_KEY = "Navigation" } fun update(scope: IScope, backStackEntries: List) { diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/NavScopes.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/NavScopes.kt new file mode 100644 index 0000000000..34a6047749 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/NavScopes.kt @@ -0,0 +1,910 @@ +package io.sentry.compose.navigation3 + +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.Lifecycle.Event.ON_DESTROY +import androidx.lifecycle.Lifecycle.Event.ON_PAUSE +import androidx.lifecycle.Lifecycle.Event.ON_RESUME +import androidx.lifecycle.Lifecycle.Event.ON_START +import androidx.lifecycle.Lifecycle.Event.ON_STOP +import androidx.lifecycle.LifecycleEventObserver +import androidx.lifecycle.LifecycleOwner +import io.sentry.Breadcrumb +import io.sentry.Hint +import io.sentry.IScope +import io.sentry.IScopes +import io.sentry.ITransaction +import io.sentry.NoOpTransaction +import io.sentry.PropagationContext +import io.sentry.Scope +import io.sentry.ScopeCallback +import io.sentry.ScopeType +import io.sentry.SentryOptions +import io.sentry.SpanStatus +import io.sentry.TransactionContext +import io.sentry.TransactionOptions + +/** + * An [IScopes] implementation that lets us coordinate updates to navigation state across multiple + * [SentryNavEffect]s. (Also supports updates managed by a single `SentryNavEffect`.) + * + * Needed because all `SentryNavEffect`s mutate a single [IScope] instance by default: + * ``` + * SentryNavEffect + * ↓ owns + * NavSession + * ├── BackStackObserver — computes nav data and nav transactions + * └── NavScopes — controls whether those results reach the real scope + * ↓ + * NavLeaseCoordinator — arbitrates ownership across all effects + * ↓ + * shared IScope + * ``` + * + * **Limitations** + * + * Multi-effect support is limited to scenarios where: + * + * - each `SentryNavEffect` is managed by a single [LifecycleOwner] (e.g., a multi-Activity app with + * one `SentryNavEffect` per Activity); + * + * - normally only one `SentryNavEffect` is active at a time; but + * + * - brief windows exist where multiple `SentryNavEffect`s may be simultaneously active (e.g., as + * the host app switches between activities). + * + * This class does ***not*** support scenarios where multiple `SentryNavEffect`s are meant to be + * simultaneously active, whether nested or as siblings (e.g., + * [multi-resume mode](https://developer.android.com/develop/ui/views/layout/support-multi-window-mode) + * across two app activities). + * + * `NavScopes` is not a general-purpose lease-protected `IScopes` implementation. Ways of accessing + * or changing an `IScope` beyond those currently used by `SentryNavEffect` are not necessarily + * protected. New operations must either stay within the current use cases or extend lease + * protection to cover them. + * + * **A fundamental tradeoff** + * + * During `LifecycleOwner` overlap, the current transaction and scope can reflect **_either_**: + * + * 1. the outgoing owner that's still visible (in `STARTED` state after pausing), or + * 2. the incoming owner that may crash before it becomes visible (in `CREATED` state) + * + * but not both. We've opted for (2) so that incoming destination populates crash context and work + * performed is associated with it†, as start-up logic is more likely to be the source of a crash or + * interesting spans than the outgoing destination's steady state. (If the incoming owner aborts + * prematurely, the nav transaction and data for the most recently resumed owner still in at least + * `STARTED` state is restored.) + * + * † Associating work with the incoming transaction means spans produced by the outgoing owner may + * appear as if they were produced by the incoming owner (or vice versa, were our policy reversed). + * A possible solution would be to implement a parent span delivery system similar to + * [LocalSentrySpan], but that's a task for another day. + * + * **Only leaseholders generate nav transactions and nav data** + * + * This class works by only allowing `NavScopes` instances holding a lease to publish their + * navigation data and transactions to the underlying `IScope`. + * + * `NavScopes` instances without a lease write to a [stagedNavData] holder, which lets them preserve + * their latest updates without overwriting another instance's published data. When an instance + * acquires the lease, it publishes its staged data to the underlying `IScope`. + * + * **Lease handoff policy** + * + * The lease is acquired preemptively by an incoming `LifecycleOwner` when it applies its first + * [back stack update][runBackStackUpdate]. + * + * The displaced owner (if any) remains eligible for restoration until it stops. Pausing releases + * the lease but preserves eligibility. If the incoming owner aborts before the displaced owner + * stops, the lease reverts to the displaced owner and its nav data and unfinished transaction are + * restored. + * + * The following diagram illustrates how handoff works: + * ``` + * Time + * │ + * ▼ + * + * SentryNavEffect A Shared state SentryNavEffect B + * ───────────────── ──────────── ───────────────── + * + * attach() Navigation lease: none + * │ + * Initial back stack update + * │ + * ├── acquire lease ────────► Navigation lease: A + * ├── create & bind txA ────► Scope transaction: txA + * ├── publish nav data ─────► Published data: A + * │ + * ON_RESUME A is eligible for restoration + * │ + * ON_PAUSE + * │ + * ├────────────────────────► Navigation lease: none + * │ txA and published data remain + * │ A remains eligible for restoration + * │ + * │ attach() + * │ │ + * │ Navigation lease: none ◄─────────────────┤ + * │ Scope transaction: txA │ + * │ Published data: A │ + * │ B registered, awaiting initial back │ + * │ stack update │ + * │ │ + * │ Initial back stack update + * │ │ + * │ suspend & unbind txA ─────┤ + * │ Navigation lease: B ◄── acquire lease ───┤ + * │ Scope transaction: txB ◄── bind txB ─────┤ + * │ Published data: B ◄── publish nav data ──┤ + * │ (rollback point) │ + * │ │ + * │ ON_RESUME + * │ │ + * │ B eligible for restoration ◄─────────────┤ + * │ │ + * ON_STOP │ + * │ │ + * ├── retain txA; timers continue │ + * ├────────────────────────► A no longer eligible for restoration │ + * │ │ + * ON_DESTROY │ + * │ │ + * ├── finish retained txA if unfinished │ + * └────────────────────────► A no longer tracked │ + * ON_PAUSE + * │ + * Navigation lease: none ◄─────────────────┤ + * Published data: B remains │ + * Existing txB may finish naturally │ + * No new transactions or publication by B │ + * │ + * ON_STOP + * │ + * B no longer eligible for restoration ◄───┤ + * │ + * ON_DESTROY + * │ + * Finish & clear txB if still bound ◄──────┤ + * Clear B's remaining screen/back stack │ + * Retain recorded breadcrumbs │ + * ▼ + * + * Alternative at rollback point: B disposes before A stops + * Finish & unbind txB ◄────── dispose B ───┤ + * Navigation lease: A │ + * Restore txA if unfinished and scope free │ + * Republish A's staged data │ + * ``` + * + * **Encapsulation policy** + * + * This class and [NavLeaseCoordinator] are meant to fully encapsulate knowledge about the + * possibility of multiple `SentryNavEffect`s and the shared mutable state issues it raises. + * Addressing those concerns is complicated – don't let that complexity escape into the rest of the + * `SentryNavEffect` system! + * + * **Usage** + * + * 1. Create a `NavScopes` instance per `SentryNavEffect`. + * + * 2. Pass the same [NavLeaseCoordinator] instance to all `NavScopes` whose effects must be + * coordinated (i.e., that target the same `IScope`). + * + * 3. Call [attach] with the lifecycle that manages the effect before the first back stack update. + * + * 4. Route back stack updates through [runBackStackUpdate]. + * + * 5. Call [dispose] when the effect leaves the composition. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. Lifecycle callbacks and method invocations should be + * confined to the same thread. + */ +internal class NavScopes( + private val delegate: IScopes, + private val coordinator: NavLeaseCoordinator = defaultNavLeaseCoordinator, +) : IScopes by delegate, LifecycleEventObserver { + + private val owner: Any = Any() + private var attachedLifecycle: Lifecycle? = null + private val stagedNavData = StagedNavData() + + /** + * Whether [runBackStackUpdate] is processing a back-stack update. + * + * This flag distinguishes a [back stack update][runBackStackUpdate] from other requests that also + * invoke [configureScope]. Needed in order to transfer lease ownership for the former but not the + * latter. + */ + private var isProcessingBackStackUpdate: Boolean = false + + /** + * Registers this [NavScopes] instance with [lifecycle]. No-ops if `lifecycle` has already been + * attached or is [destroyed][Lifecycle.Event.ON_DESTROY]. + * + * Throws if a caller tries to attach this instance to more than one lifecycle. + */ + fun attach(lifecycle: Lifecycle) { + check(attachedLifecycle == null || attachedLifecycle === lifecycle) { + "NavScopes has already been attached to a different lifecycle!" + } + + if (attachedLifecycle === lifecycle) { + return + } + if (lifecycle.currentState == Lifecycle.State.DESTROYED) { + return + } + + attachedLifecycle = lifecycle + + coordinator.registerCandidate( + owner = owner, + lifecycleState = lifecycle.currentState, + onPublishData = ::publishStagedNavData, + onClearPublishedData = stagedNavData::clearPublishedStateFrom, + ) + + lifecycle.addObserver(this) + } + + override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) { + when (event) { + ON_START -> coordinator.startCandidate(owner) + ON_RESUME -> delegate.configureScope { coordinator.resumeCandidate(owner, it) } + ON_PAUSE -> delegate.configureScope { coordinator.pauseCandidate(owner, it) } + ON_STOP -> delegate.configureScope { coordinator.stopCandidate(owner, it) } + ON_DESTROY -> dispose() + else -> Unit + } + } + + /** + * Runs [block] as a back stack update eligible to acquire the navigation lease. + * + * The observer prepares its update before invoking [configureScope], which acquires the lease + * immediately before application. Preparation failures leave the previous owner intact. + */ + fun runBackStackUpdate(block: () -> Unit) { + isProcessingBackStackUpdate = true + try { + block() + } finally { + isProcessingBackStackUpdate = false + } + } + + override fun configureScope(scopeType: ScopeType?, callback: ScopeCallback) { + val callback = ScopeCallback { scope -> + if (isProcessingBackStackUpdate) { + coordinator.onBackStackUpdate(owner, scope) + } + callback.run(LeaseProtectedNavScope(scope, owner, stagedNavData, coordinator)) + } + + if (scopeType == null) { + delegate.configureScope(callback) + } else { + delegate.configureScope(scopeType, callback) + } + } + + override fun startTransaction( + transactionContext: TransactionContext, + transactionOptions: TransactionOptions, + ): ITransaction { + if (!coordinator.hasLease(owner)) { + return NoOpTransaction.getInstance() + } + + return delegate.startTransaction(transactionContext, transactionOptions).also { transaction -> + if (!transaction.isNoOp) { + coordinator.bindTransaction(owner, transaction) + } + } + } + + override fun addBreadcrumb(breadcrumb: Breadcrumb) { + stagedNavData.addBreadcrumb(coordinator.hasLease(owner)) { + delegate.addBreadcrumb(breadcrumb) + } + } + + override fun addBreadcrumb(breadcrumb: Breadcrumb, hint: Hint?) { + stagedNavData.addBreadcrumb(coordinator.hasLease(owner)) { + delegate.addBreadcrumb(breadcrumb, hint) + } + } + + /** + * Releases the lease, clears remaining published state, and finishes this instance's transaction. + * + * Also stops observing the attached [LifecycleOwner] and cleans up memory references. + */ + fun dispose() { + attachedLifecycle?.removeObserver(this) + attachedLifecycle = null + + var callbackInvoked = false + + // Clear nav state, unbind the nav transaction, and avoid leaking the LifecycleOwner. + delegate.configureScope { scope -> + callbackInvoked = true + coordinator.unregisterCandidate(owner, scope) + } + + // If the SDK is disabled, configureScope skips its callback. Unregister without a scope only + // then; retrying a callback that failed could let its exception escape into the host app. + if (!callbackInvoked) { + coordinator.unregisterCandidate(owner, scope = null) + } + } + + private fun publishStagedNavData() { + delegate.configureScope { scope -> + if (!coordinator.hasLease(owner)) { + return@configureScope + } + stagedNavData.publishTo(scope) + } + + stagedNavData.publishPendingBreadcrumb(coordinator.hasLease(owner)) + } +} + +/** + * Holds navigation data staged for publication. + * + * In particular, two types of nav data are tracked: + * + * 1. **Nav state**: a screen name and back stack nav context + * 2. **Nav events**: a pending breadcrumb + * + * (Class also tracks propagation context so a non-leaseholder can't overwrite the current + * leaseholder's tracing state.) + */ +internal class StagedNavData { + + private val scope = Scope(SentryOptions.empty()) + + private var hasNavigationContextUpdate = false + private var hasPropagationContextUpdate = false + private var hasScreenUpdate = false + + // These may differ from the staged values after background updates. We keep the exact published + // references so cleanup preserves host replacements, including structurally equal replacements. + private var publishedNavigationContext: Any? = null + private var publishedScreen: String? = null + + /** + * The latest breadcrumb received i) before the first breadcrumb is published and ii) while not + * holding the navigation lease. + * + * This class implements a breadcrumb buffering policy that: + * + * 1. tracks the latest breadcrumb received prior to initial navigation lease acquisition; + * 2. publishes that breadcrumb upon initial lease acquisition; and + * 3. discards all breadcrumbs received after its first breadcrumb has been published when the + * navigation lease isn't held. + * + * That policy produces the following behavior: + * + * - Breadcrumbs staged before initial lease acquisition retain only the latest destination. + * Normally the first prepared update acquires the lease before emitting its breadcrumb, so it + * is published immediately. + * + * - Suppose the `SentryNavEffect` that publishes `Profile` pauses and loses the navigation lease. + * Another effect takes over and records a `Settings` back stack entry. If the paused effect + * buffers new breadcrumbs while it lacks the lease, it might later replay (an unseen) + * `Checkout` when it resumes, even though `Settings` was already reported by the active effect. + * Discarding those later breadcrumbs avoids replaying history that may be stale or out of + * order. + */ + private var pendingInitialBreadcrumb: (() -> Unit)? = null + + /** + * Whether this instance has ever published a breadcrumb. + * + * This remains true across pauses, so breadcrumbs received without the navigation lease after the + * first publication are discarded rather than replayed as stale navigation history on resume. + */ + private var hasPublishedBreadcrumb: Boolean = false + + private val screenName: String? + get() = scope.screen + + private val contexts + get() = scope.contexts + + var propagationContext: PropagationContext + get() = scope.propagationContext + set(value) { + scope.propagationContext = PropagationContext(value) + hasPropagationContextUpdate = true + } + + fun getScreen(realScope: IScope, ownsLease: Boolean): String? = + if (ownsLease) realScope.screen else screenName + + fun setScreen(screen: String?, realScope: IScope, ownsLease: Boolean) { + scope.screen = screen + hasScreenUpdate = true + + if (ownsLease) { + publishScreen(screen, realScope) + } + } + + fun setNavigationContext(value: Any?, realScope: IScope, ownsLease: Boolean) { + scope.setContexts(NAVIGATION_CONTEXT_KEY, value) + hasNavigationContextUpdate = true + + if (ownsLease) { + publishNavigationContext(value, realScope) + } + } + + fun withPropagationContext(callback: Scope.IWithPropagationContext): PropagationContext = + scope.withPropagationContext(callback).also { hasPropagationContextUpdate = true } + + /** Publishes the breadcrumb when leased, or buffers it until the first publication. */ + fun addBreadcrumb(ownsLease: Boolean, publish: () -> Unit) { + if (ownsLease) { + publish() + hasPublishedBreadcrumb = true + pendingInitialBreadcrumb = null + } else if (!hasPublishedBreadcrumb) { + // Replace older unpublished breadcrumbs so only the latest is replayed on first lease. + pendingInitialBreadcrumb = publish + } + } + + fun publishPendingBreadcrumb(ownsLease: Boolean) { + val pending = pendingInitialBreadcrumb ?: return + if (ownsLease) { + pending() + hasPublishedBreadcrumb = true + pendingInitialBreadcrumb = null + } + } + + fun publishTo(realScope: IScope) { + // A new leaseholder publishes a complete navigation state. Clearing fields it hasn't staged + // prevents the previous leaseholder's values from lingering after a handoff. + publishScreen(if (hasScreenUpdate) screenName else null, realScope) + publishNavigationContext( + if (hasNavigationContextUpdate) contexts[NAVIGATION_CONTEXT_KEY] else null, + realScope, + ) + if (hasPropagationContextUpdate) { + realScope.propagationContext = PropagationContext(scope.propagationContext) + } + } + + /** Called only for the last publishing session; releasing its lease alone does not clear data. */ + fun clearPublishedStateFrom(realScope: IScope) { + if (publishedScreen != null && realScope.screen === publishedScreen) { + realScope.screen = null + } + if ( + publishedNavigationContext != null && + realScope.contexts[NAVIGATION_CONTEXT_KEY] === publishedNavigationContext + ) { + // Use setContexts so scope observers also receive the removal. + realScope.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) + } + publishedScreen = null + publishedNavigationContext = null + } + + private fun publishScreen(screen: String?, realScope: IScope) { + publishedScreen = screen + realScope.screen = screen + } + + private fun publishNavigationContext(value: Any?, realScope: IScope) { + publishedNavigationContext = value + realScope.setContexts(NAVIGATION_CONTEXT_KEY, value) + } +} + +/** + * An [IScope] wrapper that provides access to [delegate] for nav-related updates only when the + * current [owner] holds the nav lease; otherwise all nav-related writes and relevant nav-related + * reads† are redirected to [stagedNavData]. + * + * All other access is delegated directly to [delegate]. + * + * † Note that context-related reads aren't currently redirected to [stagedNavData] because + * implementing that redirection is involved, and we don't currently need it. + */ +internal class LeaseProtectedNavScope( + private val delegate: IScope, + private val owner: Any, + private val stagedNavData: StagedNavData, + private val coordinator: NavLeaseCoordinator, +) : IScope by delegate { + + override fun getScreen(): String? = stagedNavData.getScreen(delegate, coordinator.hasLease(owner)) + + override fun setScreen(screen: String?) { + stagedNavData.setScreen(screen, delegate, coordinator.hasLease(owner)) + } + + override fun setContexts(key: String?, value: Any?) { + if (key != NAVIGATION_CONTEXT_KEY) { + delegate.setContexts(key, value) + return + } + + stagedNavData.setNavigationContext(value, delegate, coordinator.hasLease(owner)) + } + + override fun getPropagationContext(): PropagationContext = + if (coordinator.hasLease(owner)) { + delegate.propagationContext + } else { + stagedNavData.propagationContext + } + + override fun setPropagationContext(propagationContext: PropagationContext) { + stagedNavData.propagationContext = propagationContext + if (coordinator.hasLease(owner)) { + delegate.propagationContext = propagationContext + } + } + + override fun withPropagationContext(callback: Scope.IWithPropagationContext): PropagationContext = + if (coordinator.hasLease(owner)) { + delegate.withPropagationContext(callback).also { stagedNavData.propagationContext = it } + } else { + stagedNavData.withPropagationContext(callback) + } +} + +/** + * Manages a nav lease in order to determine which [NavScopes] instance can start nav transactions + * and publish nav data to a shared [IScope]. + * + * `NavScopes` instances register themselves as [candidates][Candidate] for lease ownership via + * [registerCandidate]. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. All `NavScopes` instances using this coordinator must + * confine their access to the same thread. + */ +@Suppress("TooManyFunctions") +internal class NavLeaseCoordinator { + + private val candidates = mutableListOf() + + /** Candidate whose nav transaction may still be bound to the shared scope. */ + private var activeTransactionCandidate: Candidate? = null + + /** Candidate responsible for the most recently published nav data in the shared scope. */ + private var publishedDataCandidate: Candidate? = null + + private var leaseHolder: Candidate? = null + + /** + * Counter that orders candidates by when this coordinator most recently observed them in resumed + * state. + * + * Each resume, including registration of an already-resumed owner, assigns the next value to that + * candidate's [Candidate.resumeOrder]. This lets fallback selection prefer the most recently + * resumed eligible candidate without relying on clock timestamps. Zero is reserved for candidates + * that have not been observed resumed and therefore cannot be chosen as fallbacks. + */ + private var resumeOrder: Long = 0 + + fun registerCandidate( + owner: Any, + lifecycleState: Lifecycle.State, + onPublishData: () -> Unit, + onClearPublishedData: (IScope) -> Unit = {}, + ) { + if (findCandidate(owner) != null) { + return + } + + val initialState = + when (lifecycleState) { + Lifecycle.State.INITIALIZED, + Lifecycle.State.CREATED -> Candidate.LifecycleState.INITIAL_CREATED + Lifecycle.State.STARTED -> Candidate.LifecycleState.INITIAL_STARTED + Lifecycle.State.RESUMED -> Candidate.LifecycleState.RESUMED + Lifecycle.State.DESTROYED -> return + } + + val initialResumeOrder = if (lifecycleState == Lifecycle.State.RESUMED) ++resumeOrder else 0 + candidates += + Candidate(owner, onPublishData, onClearPublishedData, initialState, initialResumeOrder) + } + + fun startCandidate(owner: Any) { + findCandidate(owner)?.start() + } + + fun resumeCandidate(owner: Any, scope: IScope) { + findCandidate(owner)?.let { + it.resume(++resumeOrder) + + if (it.hasAppliedInitialBackStackUpdate) { + transferLeaseTo(it, scope) + } + } + } + + fun pauseCandidate(owner: Any, scope: IScope) { + findCandidate(owner)?.let { + it.pause() + + if (leaseHolder === it) { + transferLeaseTo(mostRecentlyResumedCandidate(excluding = it), scope) + } + } + } + + fun stopCandidate(owner: Any, scope: IScope) { + findCandidate(owner)?.let { + it.stop() + + if (leaseHolder === it) { + transferLeaseTo(mostRecentlyResumedCandidate(), scope) + } + } + } + + fun unregisterCandidate(owner: Any, scope: IScope?) { + findCandidate(owner)?.let { + finishTransaction(it, scope) + candidates.remove(it) + + if (publishedDataCandidate === it) { + publishedDataCandidate = null + if (scope != null) { + it.onClearPublishedData(scope) + } + } + + if (leaseHolder === it) { + if (scope != null) { + transferLeaseTo(mostRecentlyResumedCandidate(), scope) + } else { + leaseHolder = null + } + } + } + } + + fun hasLease(owner: Any): Boolean = leaseHolder?.owner === owner + + /** + * Records that the registered [owner] is about to apply a back-stack update and acquires the nav + * lease if the owner's lifecycle permits it. + * + * If the lease transfers, ownership and transaction binding are updated before the candidate's + * staged nav data is published. Publication completes before this method returns, so the caller + * can then apply the back-stack update. Later calls for the same [owner] do nothing. + */ + fun onBackStackUpdate(owner: Any, scope: IScope) { + val candidate = findCandidate(owner) ?: return + if (candidate.hasAppliedInitialBackStackUpdate) { + return + } + + candidate.hasAppliedInitialBackStackUpdate = true + + if (candidate.lifecycleState.canAcquireLeaseOnFirstUpdate) { + transferLeaseTo(candidate, scope) + } + } + + fun bindTransaction(owner: Any, transaction: ITransaction) { + val candidate = leaseHolder ?: return + if (candidate.owner === owner) { + candidate.transaction = transaction + activeTransactionCandidate = candidate + } + } + + private fun findCandidate(owner: Any): Candidate? = candidates.firstOrNull { it.owner === owner } + + private fun mostRecentlyResumedCandidate(excluding: Candidate? = null): Candidate? = + candidates + .filter { + it !== excluding && + it.hasAppliedInitialBackStackUpdate && + it.lifecycleState.isAtLeastStarted && + it.resumeOrder > 0 + } + .maxByOrNull { it.resumeOrder } + + private fun transferLeaseTo(candidate: Candidate?, scope: IScope) { + if (leaseHolder === candidate) { + return + } + + if (candidate != null) { + // Unbind another owner's transaction without finishing it, so it can be restored if that + // owner returns. If this owner's retained transaction has since finished, discard it + // instead of binding a finished transaction to the shared scope. + val previous = activeTransactionCandidate + if (previous != null && previous !== candidate) { + clearTransactionFromScope(previous, scope) + activeTransactionCandidate = null + } + + val transaction = candidate.transaction + if (transaction?.isFinished == true) { + clearTransactionFromScope(candidate, scope) + candidate.transaction = null + if (activeTransactionCandidate === candidate) { + activeTransactionCandidate = null + } + } else if (transaction != null) { + // The navigation lease doesn't give this owner control over an unrelated transaction + // already bound to the shared scope. Restore the owner's retained nav transaction only if + // the scope is free or already holds that same transaction. + scope.withTransaction { current -> + if (current == null || current === transaction) { + scope.transaction = transaction + activeTransactionCandidate = candidate + } + } + } + } + + leaseHolder = candidate + if (candidate != null) { + publishedDataCandidate = candidate + candidate.onPublishData() + } + } + + private fun finishTransaction(candidate: Candidate, scope: IScope?) { + val transaction = candidate.transaction + if (transaction != null && !transaction.isFinished) { + transaction.finish(transaction.status ?: SpanStatus.OK) + } + + if (scope != null) { + clearTransactionFromScope(candidate, scope) + } + + candidate.transaction = null + + if (activeTransactionCandidate === candidate) { + activeTransactionCandidate = null + } + } + + private fun clearTransactionFromScope(candidate: Candidate, scope: IScope) { + val transaction = candidate.transaction ?: return + scope.withTransaction { current -> + if (current === transaction) { + scope.clearTransaction() + } + } + } + + /** A [LifecycleOwner] that has registered itself as a candidate for lease ownership. */ + private class Candidate( + val owner: Any, + val onPublishData: () -> Unit, + val onClearPublishedData: (IScope) -> Unit, + initialLifecycleState: LifecycleState, + initialResumeOrder: Long, + ) { + + /** Lifecycle state used to decide when a candidate may acquire the nav lease. */ + enum class LifecycleState { + + /** + * The candidate is in CREATED state but has yet to reach an observed STARTED state. + * + * "Initial" refers to the candidate's lifecycle history since being + * [registered][NavLeaseCoordinator.registerCandidate], and not necessarily to the candidate's + * lifecycle prior to registration. + */ + INITIAL_CREATED, + + /** + * The candidate is STARTED, but it has yet to reach an observed RESUMED state. + * + * "Initial" refers to the candidate's lifecycle history since being + * [registered][NavLeaseCoordinator.registerCandidate], and not necessarily to the candidate's + * lifecycle prior to registration. + */ + INITIAL_STARTED, + RESUMED, + PAUSED, + STOPPED, + + /** + * The candidate has returned to STARTED state after a being in an observed STOPPED state, but + * has not yet RESUMED. + * + * Unlike [INITIAL_STARTED], restarting does not allow a first back-stack update to take the + * nav lease from another owner. + */ + RESTARTED; + + /** + * Whether this lifecycle state permits the candidate's first back stack update since + * [registration][NavLeaseCoordinator.registerCandidate] to acquire the nav lease, including + * taking it from another owner. + * + * This checks lifecycle permission only; it does not indicate whether the candidate has + * already applied its first update. + */ + val canAcquireLeaseOnFirstUpdate: Boolean + get() = + when (this) { + INITIAL_CREATED, + INITIAL_STARTED, + RESUMED -> true + + PAUSED, + STOPPED, + RESTARTED -> false + } + + val isAtLeastStarted: Boolean + get() = + when (this) { + INITIAL_CREATED, + STOPPED -> false + + INITIAL_STARTED, + RESUMED, + PAUSED, + RESTARTED -> true + } + } + + var lifecycleState: LifecycleState = initialLifecycleState + private set + + var transaction: ITransaction? = null + + /** See [NavLeaseCoordinator.resumeOrder]. */ + var resumeOrder: Long = initialResumeOrder + private set + + var hasAppliedInitialBackStackUpdate: Boolean = false + + fun start() { + lifecycleState = + when (lifecycleState) { + LifecycleState.INITIAL_CREATED, + LifecycleState.INITIAL_STARTED -> LifecycleState.INITIAL_STARTED + + LifecycleState.STOPPED, + LifecycleState.RESTARTED -> LifecycleState.RESTARTED + // Attaching to an already-resumed lifecycle replays ON_START before ON_RESUME. + LifecycleState.RESUMED -> LifecycleState.RESUMED + LifecycleState.PAUSED -> LifecycleState.PAUSED + } + } + + fun resume(order: Long) { + resumeOrder = order + lifecycleState = LifecycleState.RESUMED + } + + fun pause() { + lifecycleState = LifecycleState.PAUSED + } + + fun stop() { + lifecycleState = LifecycleState.STOPPED + } + } +} + +internal val defaultNavLeaseCoordinator: NavLeaseCoordinator = NavLeaseCoordinator() diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/NavSession.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/NavSession.kt new file mode 100644 index 0000000000..c4ce7125fa --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/NavSession.kt @@ -0,0 +1,48 @@ +package io.sentry.compose.navigation3 + +import androidx.lifecycle.Lifecycle +import io.sentry.IScope +import io.sentry.IScopes + +/** + * Coordinates the production and publishing of navigation data for one [SentryNavEffect]. + * + * [BackStackObserver] translates back stack changes into Sentry data, while [NavScopes] controls + * whether a given `SentryNavEffect` may publish that data or whether it should defer to another + * currently active `SentryNavEffect`. + * + * **Usage** + * + * Call [attach] before the first [onBackStackChanged], and call [dispose] when observation ends. + * + * Sessions targeting the same [IScope] must share the same [NavLeaseCoordinator] instance. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. Underlying [NavScopes] lifecycle callbacks and invocations + * of this class's methods should be confined to the same thread. + */ +internal class NavSession( + scopes: IScopes, + options: SentryNavOptions, + coordinator: NavLeaseCoordinator, + entryMapper: ForwardingBackStackEntryMapper, + private val navScopes: NavScopes = NavScopes(scopes, coordinator), + private val observer: BackStackObserver = BackStackObserver(navScopes, options, entryMapper), +) { + + fun attach(lifecycle: Lifecycle) { + navScopes.attach(lifecycle) + } + + fun onBackStackChanged(backStack: List) { + navScopes.runBackStackUpdate { observer.onBackStackChanged(backStack) } + } + + fun dispose() { + // Clear only data still belonging to this session and restore any eligible previous owner. + // Observer cleanup then runs without the lease, so it cannot clear host or restored state. + navScopes.dispose() + observer.cleanup() + } +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt index 8b6269752b..eb5a2b323b 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt @@ -4,6 +4,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState +import androidx.lifecycle.compose.LocalLifecycleOwner import io.sentry.IScopes import io.sentry.ScopesAdapter import io.sentry.SentryOptions @@ -66,9 +67,12 @@ import org.jetbrains.annotations.ApiStatus * gestures. That means that spans produced by predictively rendered composables can show up under * the current destination's transaction. * - * Multiple simultaneously active `SentryNavEffect` instances writing to the same Sentry scope are - * not supported. Violating this restriction can result in interleaved breadcrumbs, clobbered screen - * names and back stacks, and transactions that interfere with one another. + * In general, host apps should ensure that only one `SentryNavEffect` is active at a time. + * `SentryNavEffect` protects against multiple instances emitting data simultaneously during + * [lifecycle owner][androidx.lifecycle.LifecycleOwner] transitions, such as when navigating between + * activities where each Activity has its own `SentryNavEffect`. But `SentryNavEffect`s associated + * with side-by-side or nested `NavDisplay`s are not supported and can result in duplicated or + * interleaved data and undefined transaction behavior. * * @param backStack The navigation backstack to observe. * @param backStackEntryMapper Maps each entry of the [backStack] to a name and optional arguments @@ -87,6 +91,7 @@ public fun SentryNavEffect( backStackEntryMapper = backStackEntryMapper, options = options, scopes = ScopesAdapter.getInstance(), + coordinator = defaultNavLeaseCoordinator, ) } @@ -94,20 +99,29 @@ public fun SentryNavEffect( internal fun SentryNavEffect( backStack: List, backStackEntryMapper: BackStackEntryMapper, - options: SentryNavOptions = SentryNavOptions(), + options: SentryNavOptions, scopes: IScopes, + coordinator: NavLeaseCoordinator, ) { val currentBackStackEntryMapper = rememberUpdatedState(backStackEntryMapper) + val lifecycle = LocalLifecycleOwner.current.lifecycle - val observer = - remember(scopes, options) { - BackStackObserver( + val session = + remember(scopes, options, lifecycle, coordinator) { + NavSession( scopes = scopes, options = options, + coordinator = coordinator, entryMapper = ForwardingBackStackEntryMapper { currentBackStackEntryMapper.value }, ) } + // Register first so the initial prepared back stack update can acquire navigation ownership. + DisposableEffect(session, lifecycle) { + session.attach(lifecycle) + onDispose { session.dispose() } + } + // Intentionally don't remember this copy. Snapshot-backed lists mutate in place, so // remember(backStack) { backStack.toList() } would cache a stale copy. (The key reference // retained by remember() and the backStack reference passed to this effect would point to the @@ -116,14 +130,10 @@ internal fun SentryNavEffect( // it with the previous one. val copy = backStack.toList() - DisposableEffect(observer, BackStackKey(copy)) { - observer.onBackStackChanged(backStack = copy) + DisposableEffect(session, BackStackKey(copy)) { + session.onBackStackChanged(backStack = copy) onDispose {} } - - DisposableEffect(observer) { - onDispose { observer.cleanup() } - } } /** diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/LeaseProtectedNavScopeTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/LeaseProtectedNavScopeTest.kt new file mode 100644 index 0000000000..dd7324909b --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/LeaseProtectedNavScopeTest.kt @@ -0,0 +1,202 @@ +package io.sentry.compose.navigation3 + +import androidx.lifecycle.Lifecycle +import com.google.common.truth.Truth.assertThat +import io.sentry.PropagationContext +import io.sentry.Scope +import io.sentry.SentryOptions +import org.junit.Test + +class LeaseProtectedNavScopeTest { + + @Test + fun `getScreen() - reads the real screen with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + leaseProtectedNavScope.screen = "Staged" + fixture.realScope.screen = "Host" + + val screen = leaseProtectedNavScope.screen + + assertThat(screen).isEqualTo("Host") + } + + @Test + fun `getScreen() - reads the staged screen without the nav lease`() { + val fixture = Fixture.create() + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + fixture.realScope.screen = "Host" + fixture.stagedNavData.setScreen("Staged", fixture.realScope, ownsLease = false) + + val screen = leaseProtectedNavScope.screen + + assertThat(screen).isEqualTo("Staged") + } + + @Test + fun `setScreen() - publishes the screen with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + + leaseProtectedNavScope.screen = "Profile" + + assertThat(fixture.realScope.screen).isEqualTo("Profile") + } + + @Test + fun `setScreen() - stages the screen without the nav lease`() { + val fixture = Fixture.create() + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + fixture.realScope.screen = "Host" + + leaseProtectedNavScope.screen = "Profile" + + assertThat(fixture.realScope.screen).isEqualTo("Host") + assertThat(leaseProtectedNavScope.screen).isEqualTo("Profile") + } + + @Test + fun `setContexts() - publishes the navigation context with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val navigationContext = mapOf("route" to "Profile") + + leaseProtectedNavScope.setContexts(NAVIGATION_CONTEXT_KEY, navigationContext) + + assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]) + .isSameInstanceAs(navigationContext) + } + + @Test + fun `setContexts() - stages the navigation context without the nav lease`() { + val fixture = Fixture.create() + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val hostContext = mapOf("route" to "Host") + val navigationContext = mapOf("route" to "Profile") + fixture.realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + + leaseProtectedNavScope.setContexts(NAVIGATION_CONTEXT_KEY, navigationContext) + + assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + fixture.stagedNavData.publishTo(fixture.realScope) + assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]) + .isSameInstanceAs(navigationContext) + } + + @Test + fun `setContexts() - delegates unrelated contexts without the nav lease`() { + val fixture = Fixture.create() + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val hostContext = mapOf("value" to "retained") + + leaseProtectedNavScope.setContexts("host", hostContext) + + assertThat(fixture.realScope.contexts["host"]).isSameInstanceAs(hostContext) + } + + @Test + fun `getPropagationContext() - reads the real context with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val realContext = fixture.realScope.propagationContext + + val context = leaseProtectedNavScope.propagationContext + + assertThat(context).isSameInstanceAs(realContext) + } + + @Test + fun `getPropagationContext() - reads the staged context without the nav lease`() { + val fixture = Fixture.create() + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val stagedContext = fixture.stagedNavData.propagationContext + + val context = leaseProtectedNavScope.propagationContext + + assertThat(context).isSameInstanceAs(stagedContext) + } + + @Test + fun `setPropagationContext() - publishes and stages the context with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val context = PropagationContext() + + leaseProtectedNavScope.propagationContext = context + + assertThat(fixture.realScope.propagationContext.traceId).isEqualTo(context.traceId) + assertThat(fixture.stagedNavData.propagationContext.traceId).isEqualTo(context.traceId) + } + + @Test + fun `setPropagationContext() - stages the context without the nav lease`() { + val fixture = Fixture.create() + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val realContext = fixture.realScope.propagationContext + val context = PropagationContext() + + leaseProtectedNavScope.propagationContext = context + + assertThat(fixture.stagedNavData.propagationContext.traceId).isEqualTo(context.traceId) + assertThat(fixture.realScope.propagationContext).isSameInstanceAs(realContext) + } + + @Test + fun `withPropagationContext() - updates the real and staged contexts with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val traceId = PropagationContext().traceId + + val updated = leaseProtectedNavScope.withPropagationContext { it.traceId = traceId } + + assertThat(updated.traceId).isEqualTo(traceId) + assertThat(fixture.realScope.propagationContext.traceId).isEqualTo(traceId) + assertThat(fixture.stagedNavData.propagationContext.traceId).isEqualTo(traceId) + } + + @Test + fun `withPropagationContext() - updates only the staged context without the nav lease`() { + val fixture = Fixture.create() + val leaseProtectedNavScope = fixture.leaseProtectedNavScope + val realContext = fixture.realScope.propagationContext + val traceId = PropagationContext().traceId + + val updated = leaseProtectedNavScope.withPropagationContext { it.traceId = traceId } + + assertThat(updated.traceId).isEqualTo(traceId) + assertThat(fixture.stagedNavData.propagationContext.traceId).isEqualTo(traceId) + assertThat(fixture.realScope.propagationContext).isSameInstanceAs(realContext) + } + + private class Fixture + private constructor( + val realScope: Scope, + val stagedNavData: StagedNavData, + val leaseProtectedNavScope: LeaseProtectedNavScope, + ) { + + companion object { + + fun create(ownsLease: Boolean = false): Fixture { + val realScope = Scope(SentryOptions.empty()) + val stagedNavData = StagedNavData() + val coordinator = NavLeaseCoordinator() + val owner = Any() + coordinator.registerCandidate( + owner, + Lifecycle.State.RESUMED, + { stagedNavData.publishTo(realScope) }, + ) + if (ownsLease) { + coordinator.onBackStackUpdate(owner, realScope) + } + + return Fixture( + realScope, + stagedNavData, + LeaseProtectedNavScope(realScope, owner, stagedNavData, coordinator), + ) + } + } + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/NavLeaseCoordinatorTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/NavLeaseCoordinatorTest.kt new file mode 100644 index 0000000000..5715b7626b --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/NavLeaseCoordinatorTest.kt @@ -0,0 +1,375 @@ +package io.sentry.compose.navigation3 + +import androidx.lifecycle.Lifecycle +import com.google.common.truth.Truth.assertThat +import io.sentry.ITransaction +import io.sentry.Scope +import io.sentry.SentryOptions +import io.sentry.SpanStatus +import org.junit.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.whenever + +class NavLeaseCoordinatorTest { + + private val owner = Any() + private val incoming = Any() + private val incumbent = Any() + private val previous = Any() + + private val transaction = mock() + private val unrelatedTransaction = mock() + + @Test + fun `a registered created owner acquires the nav lease on its first back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.published).containsExactly(owner) + } + + @Test + fun `registering the same owner twice preserves its first publication callback`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + + navLeaseCoordinator.registerCandidate( + owner, + Lifecycle.State.RESUMED, + { fixture.published += "duplicate" }, + ) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(fixture.published).containsExactly(owner) + } + + @Test + fun `registering a destroyed owner does not make it eligible for the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + + fixture.register(owner, Lifecycle.State.DESTROYED) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.published).isEmpty() + } + + @Test + fun `equal but distinct owners do not share the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val registeredOwner = String(charArrayOf('a')) + val equalOwner = String(charArrayOf('a')) + fixture.acquireLease(registeredOwner) + + val result = navLeaseCoordinator.hasLease(equalOwner) + + assertThat(result).isFalse() + assertThat(navLeaseCoordinator.hasLease(registeredOwner)).isTrue() + } + + @Test + fun `starting a created owner leaves it eligible to acquire the nav lease on its first back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + + navLeaseCoordinator.startCandidate(owner) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + } + + @Test + fun `restarting a stopped owner does not take the nav lease on its first back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + fixture.acquireLease(incumbent) + + navLeaseCoordinator.startCandidate(owner) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(incumbent)).isTrue() + assertThat(fixture.published).containsExactly(incumbent) + } + + @Test + fun `resuming an owner before its first back stack update leaves the nav lease unassigned`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.published).isEmpty() + } + + @Test + fun `resuming a stopped owner after its first back stack update transfers the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(incumbent) + fixture.register(owner, Lifecycle.State.CREATED) + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.published).containsExactly(incumbent, owner).inOrder() + } + + @Test + fun `a paused owner cannot acquire the nav lease on its first back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.RESUMED) + navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.published).isEmpty() + } + + @Test + fun `later back stack updates do not republish nav data or reclaim the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val first = Any() + val second = Any() + fixture.acquireLease(first) + fixture.acquireLease(second, state = Lifecycle.State.CREATED) + + navLeaseCoordinator.onBackStackUpdate(first, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(second)).isTrue() + assertThat(fixture.published).containsExactly(first, second).inOrder() + } + + @Test + fun `pausing the leaseholder restores the most recently resumed eligible owner`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val older = Any() + val recent = Any() + fixture.acquireLease(older) + fixture.acquireLease(recent) + fixture.acquireLease(incoming, state = Lifecycle.State.CREATED) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(recent)).isTrue() + assertThat(fixture.published.last()).isSameInstanceAs(recent) + } + + @Test + fun `pausing the leaseholder skips a resumed owner without a back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val eligible = Any() + val uninitialized = Any() + fixture.acquireLease(eligible) + fixture.register(uninitialized, Lifecycle.State.RESUMED) + fixture.acquireLease(incoming, state = Lifecycle.State.CREATED) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(eligible)).isTrue() + assertThat(navLeaseCoordinator.hasLease(uninitialized)).isFalse() + } + + @Test + fun `pausing the only eligible owner releases the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner) + + navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.published).containsExactly(owner) + } + + @Test + fun `stopping the leaseholder restores a resumed owner`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(previous) + fixture.acquireLease(owner, state = Lifecycle.State.CREATED) + + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() + assertThat(fixture.published.last()).isSameInstanceAs(previous) + } + + @Test + fun `stopping an owner excludes it from later nav lease fallback`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(previous) + navLeaseCoordinator.stopCandidate(previous, fixture.scope) + fixture.acquireLease(owner, state = Lifecycle.State.CREATED) + + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + } + + @Test + fun `a non-leaseholder cannot bind a nav transaction`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val other = Any() + fixture.acquireLease(owner) + + navLeaseCoordinator.bindTransaction(other, transaction) + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + verifyNoInteractions(transaction) + } + + @Test + fun `an unfinished nav transaction is restored when its owner regains the lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.CREATED) + + assertThat(fixture.scope.transaction).isNull() + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(transaction) + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + verify(transaction, org.mockito.kotlin.never()).finish(org.mockito.kotlin.any()) + } + + @Test + fun `restoring a nav transaction preserves an unrelated scope transaction`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.CREATED) + fixture.scope.transaction = unrelatedTransaction + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(unrelatedTransaction) + } + + @Test + fun `a finished nav transaction is discarded when its owner regains the lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.CREATED) + whenever(transaction.isFinished).thenReturn(true) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(fixture.scope.transaction).isNull() + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + } + + @Test + fun `unregistering the leaseholder finishes its transaction and clears its published nav data`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + verify(transaction).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.cleared).containsExactly(owner) + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + } + + @Test + fun `unregistering a former leaseholder preserves the current owner's published nav data`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(previous) + fixture.acquireLease(owner, state = Lifecycle.State.CREATED) + navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `unregistering the leaseholder without a scope releases the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner) + + navLeaseCoordinator.unregisterCandidate(owner, null) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `unregistering a former leaseholder finishes its retained transaction without clearing an unrelated one`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + whenever(transaction.status).thenReturn(SpanStatus.CANCELLED) + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.CREATED) + fixture.scope.transaction = unrelatedTransaction + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + verify(transaction).finish(SpanStatus.CANCELLED) + assertThat(fixture.scope.transaction).isSameInstanceAs(unrelatedTransaction) + assertThat(fixture.cleared).isEmpty() + } + + private class Fixture { + val navLeaseCoordinator = NavLeaseCoordinator() + val scope = Scope(SentryOptions()) + val published = mutableListOf() + val cleared = mutableListOf() + + fun register(owner: Any, state: Lifecycle.State) { + navLeaseCoordinator.registerCandidate( + owner, + state, + onPublishData = { published += owner }, + onClearPublishedData = { cleared += owner }, + ) + } + + fun acquireLease( + owner: Any, + state: Lifecycle.State = Lifecycle.State.RESUMED, + transaction: ITransaction? = null, + ) { + register(owner, state) + navLeaseCoordinator.onBackStackUpdate(owner, scope) + if (transaction != null) { + navLeaseCoordinator.bindTransaction(owner, transaction) + scope.transaction = transaction + } + } + + companion object { + fun create(): Fixture = Fixture() + } + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/NavSessionTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/NavSessionTest.kt new file mode 100644 index 0000000000..1761679477 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/NavSessionTest.kt @@ -0,0 +1,40 @@ +package io.sentry.compose.navigation3 + +import io.sentry.IScopes +import org.junit.Test +import org.mockito.kotlin.inOrder +import org.mockito.kotlin.mock + +class NavSessionTest { + + @Test + fun `dispose() - disposes nav scopes before cleaning up the back stack observer`() { + val fixture = Fixture.create() + val navSession = fixture.navSession + + navSession.dispose() + + inOrder(fixture.navScopes, fixture.observer) { + verify(fixture.navScopes).dispose() + verify(fixture.observer).cleanup() + } + } + + private class Fixture { + val navScopes = mock() + val observer = mock>() + val navSession = + NavSession( + scopes = mock(), + options = SentryNavOptions(), + coordinator = mock(), + entryMapper = mock>(), + navScopes = navScopes, + observer = observer, + ) + + companion object { + fun create(): Fixture = Fixture() + } + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt index 7b8a4b9946..37adad1b9e 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt @@ -3,11 +3,16 @@ package io.sentry.compose.navigation3 import android.app.Application import android.content.ComponentName import androidx.activity.ComponentActivity +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateOf import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat @@ -15,9 +20,12 @@ import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.IScope import io.sentry.IScopes +import io.sentry.ISentryClient import io.sentry.ITransaction import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.ScopeType +import io.sentry.Scopes import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.TransactionContext @@ -73,6 +81,7 @@ class SentryNavEffectTest { } val scope = Scope(options) val scopes = mock() + val coordinator = NavLeaseCoordinator() val breadcrumbs = mutableListOf() val transactions = mutableListOf() @@ -112,6 +121,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } @@ -135,6 +145,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -162,6 +173,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -189,6 +201,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -222,6 +235,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -255,6 +269,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions { maxCapturedBackStackEntries = 0 }, scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -278,6 +293,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -314,6 +330,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) val currentTop = backStack.last() @@ -348,6 +365,7 @@ class SentryNavEffectTest { backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -385,6 +403,7 @@ class SentryNavEffectTest { backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -417,6 +436,7 @@ class SentryNavEffectTest { backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -459,6 +479,7 @@ class SentryNavEffectTest { backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -496,6 +517,7 @@ class SentryNavEffectTest { options = options.value, backStackEntryMapper = defaultEntryMapper, scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -525,7 +547,9 @@ class SentryNavEffectTest { SentryNavEffect( backStack = backStack, backStackEntryMapper = defaultEntryMapper, + options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } } @@ -543,6 +567,70 @@ class SentryNavEffectTest { assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() } + @Test + fun `effect publishes initial navigation before its lifecycle owner resumes`() { + val fixture = NavTestFixture() + val owner = NavTestLifecycle() + val options = SentryNavOptions() + composeRule.setContent { + CompositionLocalProvider(LocalLifecycleOwner provides owner) { + SentryNavEffect( + backStack = listOf("Home"), + backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, + options = options, + scopes = fixture.scopes, + coordinator = fixture.coordinator, + ) + } + } + composeRule.runOnIdle { + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }).containsExactly("/Home") + owner.resume() + } + composeRule.runOnIdle { + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }).containsExactly("/Home") + } + } + + @Test + fun `replacing lifecycle owner disposes the old session and observes the new owner`() { + val fixture = NavTestFixture() + val originalOwner = NavTestLifecycle(Lifecycle.State.RESUMED) + val replacementOwner = NavTestLifecycle() + val owner = mutableStateOf(originalOwner) + val options = SentryNavOptions() + composeRule.setContent { + CompositionLocalProvider(LocalLifecycleOwner provides owner.value) { + SentryNavEffect( + backStack = listOf("Home"), + backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, + options = options, + scopes = fixture.scopes, + coordinator = fixture.coordinator, + ) + } + } + lateinit var originalTransaction: ITransaction + composeRule.runOnIdle { + originalTransaction = requireNotNull(fixture.scope.transaction) + owner.value = replacementOwner + } + composeRule.runOnIdle { + assertThat(originalTransaction.isFinished).isTrue() + assertThat(fixture.scope.transaction).isNotSameInstanceAs(originalTransaction) + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + assertThat(fixture.scope.screen).isEqualTo("/Home") + originalOwner.pause() + originalOwner.resume() + assertThat(fixture.scope.screen).isEqualTo("/Home") + replacementOwner.resume() + assertThat(fixture.scope.screen).isEqualTo("/Home") + } + } + private fun IScope.navigationBackStack(): List>? { val navigationContext = contexts[NAVIGATION_CONTEXT_KEY] as? Map<*, *> ?: return null @@ -562,3 +650,44 @@ class SentryNavEffectTest { const val BACKSTACK_KEY = "Back Stack" } } + +private class NavTestLifecycle(state: Lifecycle.State = Lifecycle.State.CREATED) : LifecycleOwner { + + override val lifecycle = + LifecycleRegistry.createUnsafe(this).apply { + if (state == Lifecycle.State.DESTROYED) currentState = Lifecycle.State.CREATED + currentState = state + } + + fun resume() { + lifecycle.currentState = Lifecycle.State.RESUMED + } + + fun pause() { + lifecycle.currentState = Lifecycle.State.STARTED + } +} + +/** Real SDK state with only the event delivery boundary stubbed. No timers or global SDK state. */ +private class NavTestFixture { + + val options = + SentryOptions().apply { + dsn = "http://key@localhost/1" + defaultScopeType = ScopeType.CURRENT + setTracesSampleRate(1.0) + idleTimeout = null + deadlineTimeout = 0 + } + + val scope = Scope(options) + val scopes = Scopes(scope, Scope(options), Scope(options), "navigation test") + + val coordinator = NavLeaseCoordinator() + + init { + val client = mock() + whenever(client.isEnabled).thenReturn(true) + scopes.bindClient(client) + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/StagedNavDataTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/StagedNavDataTest.kt new file mode 100644 index 0000000000..7d0fdc460a --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/StagedNavDataTest.kt @@ -0,0 +1,363 @@ +package io.sentry.compose.navigation3 + +import com.google.common.truth.Truth.assertThat +import io.sentry.PropagationContext +import io.sentry.Scope +import io.sentry.SentryOptions +import io.sentry.protocol.SentryId +import java.util.UUID +import org.junit.Test + +class StagedNavDataTest { + + private val realScope = Scope(SentryOptions.empty()) + private val stagedNavData = StagedNavData() + + private val realTraceId = SentryId(UUID.randomUUID()) + private val stagedTraceId = SentryId(UUID.randomUUID()) + private val updatedTraceId = SentryId(UUID.randomUUID()) + + @Test + fun `propagationContext - stages the supplied context without changing the real scope`() { + realScope.propagationContext.traceId = realTraceId + val context = PropagationContext().apply { traceId = stagedTraceId } + + stagedNavData.propagationContext = context + + assertThat(stagedNavData.propagationContext.traceId).isEqualTo(stagedTraceId) + assertThat(realScope.propagationContext.traceId).isEqualTo(realTraceId) + } + + @Test + fun `propagationContext - retains the staged context after the supplied context changes`() { + val context = PropagationContext().apply { traceId = stagedTraceId } + stagedNavData.propagationContext = context + + context.traceId = updatedTraceId + + assertThat(stagedNavData.propagationContext.traceId).isEqualTo(stagedTraceId) + } + + @Test + fun `getScreen() - reads the staged screen without the nav lease`() { + stagedNavData.setScreen("Profile", realScope, ownsLease = false) + + val screen = stagedNavData.getScreen(realScope, ownsLease = false) + + assertThat(screen).isEqualTo("Profile") + assertThat(realScope.screen).isNull() + } + + @Test + fun `getScreen() - reads the real screen with the nav lease`() { + stagedNavData.setScreen("Profile", realScope, ownsLease = false) + realScope.screen = "Host" + + val screen = stagedNavData.getScreen(realScope, ownsLease = true) + + assertThat(screen).isEqualTo("Host") + } + + @Test + fun `setScreen() - publishes the screen immediately with the nav lease`() { + stagedNavData.setScreen("Profile", realScope, ownsLease = true) + + assertThat(realScope.screen).isEqualTo("Profile") + } + + @Test + fun `setScreen() - retains the screen for later publication with the nav lease`() { + stagedNavData.setScreen("Profile", realScope, ownsLease = true) + realScope.screen = "Host" + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isEqualTo("Profile") + } + + @Test + fun `setScreen() - stages the screen without changing the real scope when the nav lease is absent`() { + realScope.screen = "Host" + + stagedNavData.setScreen("Profile", realScope, ownsLease = false) + + assertThat(stagedNavData.getScreen(realScope, ownsLease = false)).isEqualTo("Profile") + assertThat(realScope.screen).isEqualTo("Host") + } + + @Test + fun `setScreen() - retains the screen for later publication without the nav lease`() { + stagedNavData.setScreen("Profile", realScope, ownsLease = false) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isEqualTo("Profile") + } + + @Test + fun `setNavigationContext() - publishes the navigation context immediately with the nav lease`() { + val navigationContext = mapOf("route" to "Profile") + + stagedNavData.setNavigationContext(navigationContext, realScope, ownsLease = true) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) + } + + @Test + fun `setNavigationContext() - retains the navigation context for later publication with the nav lease`() { + val navigationContext = mapOf("route" to "Profile") + stagedNavData.setNavigationContext(navigationContext, realScope, ownsLease = true) + realScope.setContexts(NAVIGATION_CONTEXT_KEY, mapOf("route" to "Host")) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) + } + + @Test + fun `setNavigationContext() - leaves the real navigation context unchanged without the nav lease`() { + val navigationContext = mapOf("route" to "Profile") + val hostContext = mapOf("route" to "Host") + realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + + stagedNavData.setNavigationContext(navigationContext, realScope, ownsLease = false) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + } + + @Test + fun `setNavigationContext() - retains the navigation context for later publication without the nav lease`() { + val navigationContext = mapOf("route" to "Profile") + stagedNavData.setNavigationContext(navigationContext, realScope, ownsLease = false) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) + } + + @Test + fun `withPropagationContext() - returns the changed staged context without changing the real scope`() { + realScope.propagationContext.traceId = realTraceId + + val updated = stagedNavData.withPropagationContext { it.traceId = stagedTraceId } + + assertThat(updated.traceId).isEqualTo(stagedTraceId) + assertThat(stagedNavData.propagationContext.traceId).isEqualTo(stagedTraceId) + assertThat(realScope.propagationContext.traceId).isEqualTo(realTraceId) + } + + @Test + fun `withPropagationContext() - retains callback changes for later publication`() { + stagedNavData.withPropagationContext { it.traceId = stagedTraceId } + + stagedNavData.publishTo(realScope) + + assertThat(realScope.propagationContext.traceId).isEqualTo(stagedTraceId) + } + + @Test + fun `addBreadcrumb() - publishes immediately with the nav lease`() { + val published = mutableListOf() + + stagedNavData.addBreadcrumb(ownsLease = true) { published += "Profile" } + + assertThat(published).containsExactly("Profile") + } + + @Test + fun `addBreadcrumb() - buffers only the latest breadcrumb without the nav lease before publication`() { + val published = mutableListOf() + + stagedNavData.addBreadcrumb(ownsLease = false) { published += "Home" } + stagedNavData.addBreadcrumb(ownsLease = false) { published += "Profile" } + + assertThat(published).isEmpty() + stagedNavData.publishPendingBreadcrumb(ownsLease = true) + assertThat(published).containsExactly("Profile") + } + + @Test + fun `addBreadcrumb() - discards breadcrumbs received without the nav lease after first publication`() { + val published = mutableListOf() + stagedNavData.addBreadcrumb(ownsLease = true) { published += "Home" } + + stagedNavData.addBreadcrumb(ownsLease = false) { published += "Unseen" } + stagedNavData.publishPendingBreadcrumb(ownsLease = true) + + assertThat(published).containsExactly("Home") + } + + @Test + fun `addBreadcrumb() - replaces a pending breadcrumb when the first leased breadcrumb arrives`() { + val published = mutableListOf() + stagedNavData.addBreadcrumb(ownsLease = false) { published += "Unseen" } + + stagedNavData.addBreadcrumb(ownsLease = true) { published += "Profile" } + stagedNavData.publishPendingBreadcrumb(ownsLease = true) + + assertThat(published).containsExactly("Profile") + } + + @Test + fun `publishPendingBreadcrumb() - retains the latest pending breadcrumb without the nav lease`() { + val published = mutableListOf() + stagedNavData.addBreadcrumb(ownsLease = false) { published += "Home" } + stagedNavData.addBreadcrumb(ownsLease = false) { published += "Profile" } + + stagedNavData.publishPendingBreadcrumb(ownsLease = false) + + assertThat(published).isEmpty() + stagedNavData.publishPendingBreadcrumb(ownsLease = true) + assertThat(published).containsExactly("Profile") + } + + @Test + fun `publishPendingBreadcrumb() - publishes the latest pending breadcrumb once with the nav lease`() { + val published = mutableListOf() + stagedNavData.addBreadcrumb(ownsLease = false) { published += "Home" } + stagedNavData.addBreadcrumb(ownsLease = false) { published += "Profile" } + + stagedNavData.publishPendingBreadcrumb(ownsLease = true) + stagedNavData.publishPendingBreadcrumb(ownsLease = true) + + assertThat(published).containsExactly("Profile") + } + + @Test + fun `publishTo() - publishes the staged screen`() { + stagedNavData.setScreen("Profile", realScope, ownsLease = false) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isEqualTo("Profile") + } + + @Test + fun `publishTo() - clears the real screen when no screen was staged`() { + realScope.screen = "Previous" + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isNull() + } + + @Test + fun `publishTo() - clears the real screen when the staged screen is null`() { + realScope.screen = "Previous" + stagedNavData.setScreen(null, realScope, ownsLease = false) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isNull() + } + + @Test + fun `publishTo() - publishes the staged navigation context`() { + val navigationContext = mapOf("route" to "Profile") + stagedNavData.setNavigationContext(navigationContext, realScope, ownsLease = false) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) + } + + @Test + fun `publishTo() - clears the real navigation context when none was staged`() { + realScope.setContexts(NAVIGATION_CONTEXT_KEY, mapOf("route" to "Previous")) + realScope.setContexts("host", mapOf("value" to "retained")) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isNull() + assertThat(realScope.contexts["host"]).isEqualTo(mapOf("value" to "retained")) + } + + @Test + fun `publishTo() - clears the real navigation context when the staged context is null`() { + realScope.setContexts(NAVIGATION_CONTEXT_KEY, mapOf("route" to "Previous")) + stagedNavData.setNavigationContext(null, realScope, ownsLease = false) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isNull() + } + + @Test + fun `publishTo() - publishes the staged propagation context to the real scope`() { + realScope.propagationContext.traceId = realTraceId + val context = PropagationContext().apply { traceId = stagedTraceId } + stagedNavData.propagationContext = context + + stagedNavData.publishTo(realScope) + + assertThat(realScope.propagationContext.traceId).isEqualTo(stagedTraceId) + } + + @Test + fun `publishTo() - preserves the real propagation context when none was staged`() { + realScope.propagationContext.traceId = realTraceId + + stagedNavData.publishTo(realScope) + + assertThat(realScope.propagationContext.traceId).isEqualTo(realTraceId) + } + + @Test + fun `clearPublishedStateFrom() - clears screen and navigation context published by this holder`() { + stagedNavData.setScreen("Profile", realScope, ownsLease = true) + stagedNavData.setNavigationContext(mapOf("route" to "Profile"), realScope, ownsLease = true) + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isNull() + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isNull() + } + + @Test + fun `clearPublishedStateFrom() - clears the published screen and navigation context after newer values are staged`() { + stagedNavData.setScreen("Home", realScope, ownsLease = true) + stagedNavData.setNavigationContext(mapOf("route" to "Home"), realScope, ownsLease = true) + stagedNavData.setScreen("Profile", realScope, ownsLease = false) + stagedNavData.setNavigationContext( + mapOf("route" to "Profile"), + realScope, + ownsLease = false, + ) + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isNull() + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isNull() + } + + @Test + fun `clearPublishedStateFrom() - preserves equal but distinct host replacements`() { + val publishedScreen = String(charArrayOf('H', 'o', 'm', 'e')) + val publishedContext = mapOf("route" to "Home") + stagedNavData.setScreen(publishedScreen, realScope, ownsLease = true) + stagedNavData.setNavigationContext(publishedContext, realScope, ownsLease = true) + val hostScreen = String(charArrayOf('H', 'o', 'm', 'e')) + val hostContext = mapOf("route" to "Home") + realScope.screen = hostScreen + realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isSameInstanceAs(hostScreen) + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + } + + @Test + fun `clearPublishedStateFrom() - preserves the real screen and navigation context before publication`() { + realScope.screen = "Host" + val hostContext = mapOf("route" to "Host") + realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + stagedNavData.setScreen("Staged", realScope, ownsLease = false) + stagedNavData.setNavigationContext(mapOf("route" to "Staged"), realScope, ownsLease = false) + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isEqualTo("Host") + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + } +} diff --git a/sentry-samples/sentry-samples-android/build.gradle.kts b/sentry-samples/sentry-samples-android/build.gradle.kts index 4fee4a1b92..5c599a733c 100644 --- a/sentry-samples/sentry-samples-android/build.gradle.kts +++ b/sentry-samples/sentry-samples-android/build.gradle.kts @@ -238,6 +238,9 @@ dependencies { debugImplementation(projects.sentryAndroidDistribution) debugImplementation(libs.leakcanary) + + testImplementation(libs.google.truth) + testImplementation(libs.kotlin.test.junit) } abstract class ToggleNativeLoggingTask : Exec() { diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index 4e80ddfd86..c30820f24c 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -123,6 +123,18 @@ android:screenOrientation="portrait" android:theme="@style/AppTheme.Main" /> + + + + diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Activity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Activity.kt index 5b935a01e0..84924ab5ed 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Activity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Activity.kt @@ -75,9 +75,6 @@ class Nav3Activity : ComponentActivity() { private var routeWorkOptions by mutableStateOf(setOf(RouteWorkOption.MANUAL_CHILD_SPAN)) - private var isTransactionHistoryActive = false - private val transactionHistory = - NavigationTransactionHistory(isActive = { isTransactionHistoryActive }) private var showTransactionHistorySheet by mutableStateOf(false) private var showActivityUiLoadTransactionDelayMessage = false @@ -99,14 +96,14 @@ class Nav3Activity : ComponentActivity() { cancelCurrentActivityUiLoadTransaction() } - transactionHistory.install() + NavigationTransactionHistory.install() setContent { Nav3SampleTheme { Nav3SampleApp( performanceState = performanceState, configuration = configuration, - transactions = transactionHistory.transactions, + transactions = NavigationTransactionHistory.transactions, showActivityUiLoadTransactionDelayMessage = showActivityUiLoadTransactionDelayMessage, routeWorkOptions = routeWorkOptions, showTransactionHistorySheet = showTransactionHistorySheet, @@ -124,13 +121,7 @@ class Nav3Activity : ComponentActivity() { } } - override fun onStart() { - super.onStart() - isTransactionHistoryActive = true - } - override fun onStop() { - isTransactionHistoryActive = false performanceState.stopAutomaticWork() super.onStop() } @@ -139,7 +130,6 @@ class Nav3Activity : ComponentActivity() { if (isFinishing) { previousConfig.applyToCurrentOptions() } - transactionHistory.uninstall() super.onDestroy() } @@ -464,6 +454,17 @@ private fun Nav3SampleApp( ) } } + entry { route -> + TracedNav3Route(route, selectedScenario) { + Nav3RouteWorkEffect(route, routeWorkOptions) + MultiActivityRoute { + activity?.startActivity( + Intent(activity, Nav3ProductsActivity::class.java) + .putExtras(Bundle().apply { putNav3SampleConfiguration(configuration) }) + ) + } + } + } entry { route -> TracedNav3Route(route, selectedScenario) { Nav3RouteWorkEffect(route, routeWorkOptions) diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3MultiActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3MultiActivity.kt new file mode 100644 index 0000000000..85296018c6 --- /dev/null +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3MultiActivity.kt @@ -0,0 +1,272 @@ +package io.sentry.samples.android.navigation.nav3 + +import android.content.ClipData +import android.content.ClipboardManager +import android.content.Intent +import android.net.Uri +import android.os.Bundle +import android.util.Log +import android.widget.Toast +import androidx.activity.ComponentActivity +import androidx.activity.compose.setContent +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.safeDrawingPadding +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.AccountTree +import androidx.compose.material.icons.filled.Settings +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Scaffold +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.ExperimentalComposeUiApi +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.unit.dp +import androidx.navigation3.runtime.entryProvider +import androidx.navigation3.ui.NavDisplay +import io.sentry.compose.SentryModifier.sentryTag +import io.sentry.compose.navigation3.SentryBackStackEntry +import io.sentry.compose.navigation3.SentryNavEffect +import io.sentry.compose.navigation3.SentryNavOptions +import io.sentry.samples.android.navigation.common.NavigationSampleConfig +import io.sentry.samples.android.navigation.common.RouteWorkOption +import io.sentry.samples.android.navigation.common.hasOnlyActivityUiLoadTransactions +import io.sentry.samples.android.navigation.common.showRouteWorkDialog + +/** Keeps its navigation effect alive while checkout runs in another Activity. */ +class Nav3ProductsActivity : ComponentActivity() { + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + val configuration = (intent.extras ?: Bundle()).nav3SampleConfiguration() + NavigationTransactionHistory.install() + if (!configuration.enableActivityUiLoadTransaction) { + cancelCurrentActivityUiLoadTransaction() + } + setContent { + Nav3SampleTheme { + MultiActivityNavDisplay( + activity = this, + initialRoute = Nav3Route.ProductList, + configuration = configuration, + onFinish = { finish() }, + onCheckout = { productId -> + startActivity( + Intent(this, Nav3CheckoutActivity::class.java) + .putExtra(EXTRA_PRODUCT_ID, productId) + .putExtras(Bundle().apply { putNav3SampleConfiguration(configuration) }) + ) + }, + ) + } + } + } +} + +/** Owns the checkout back stack independently of the Products Activity. */ +class Nav3CheckoutActivity : ComponentActivity() { + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + val configuration = (intent.extras ?: Bundle()).nav3SampleConfiguration() + val productId = intent.getStringExtra(EXTRA_PRODUCT_ID) ?: "42" + NavigationTransactionHistory.install() + if (!configuration.enableActivityUiLoadTransaction) { + cancelCurrentActivityUiLoadTransaction() + } + setContent { + Nav3SampleTheme { + MultiActivityNavDisplay( + activity = this, + initialRoute = Nav3Route.Checkout(productId), + configuration = configuration, + onFinish = { finish() }, + ) + } + } + } +} + +@Composable +internal fun MultiActivityRoute(onOpenProducts: () -> Unit) { + RouteScaffold( + routeSpec = Nav3Route.MultiActivity.routeSpec(), + testTagPrefix = nav3TestTag("route_multi_activity"), + cardContent = { + RouteButton( + "Open products Activity", + onClick = onOpenProducts, + testTag = nav3TestTag("multi_activity_open_products"), + ) + }, + ) +} + +@OptIn(ExperimentalComposeUiApi::class) +@Composable +private fun MultiActivityNavDisplay( + activity: ComponentActivity, + initialRoute: Nav3Route, + configuration: NavigationSampleConfig, + onFinish: () -> Unit, + onCheckout: (String) -> Unit = {}, +) { + val backStack = rememberSaveableNav3BackStack(initialRoute) + var routeWorkOptions by remember { mutableStateOf(setOf(RouteWorkOption.MANUAL_CHILD_SPAN)) } + var showTransactionHistorySheet by remember { mutableStateOf(false) } + var showCrashConfirmation by remember { mutableStateOf(false) } + val options = + remember(configuration) { + SentryNavOptions { + enableNavigationBreadcrumbs = configuration.enableNavigationBreadcrumbs + enableNavigationTransactions = configuration.enableNavigationTransactions + captureBackStack = configuration.captureBackStack + maxCapturedBackStackEntries = configuration.maxCapturedBackStackEntries + } + } + SentryNavEffect( + backStack = backStack, + backStackEntryMapper = { route -> + SentryBackStackEntry(name = route.routeName, arguments = route.arguments) + }, + options = options, + ) + val onBack: () -> Unit = { + if (backStack.size > 1) { + backStack.removeLastOrNull() + } else { + onFinish() + } + } + Scaffold( + modifier = Modifier.fillMaxSize().safeDrawingPadding(), + topBar = { + Column(modifier = Modifier.padding(horizontal = 24.dp, vertical = 12.dp)) { + Row(modifier = Modifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically) { + Text( + if (initialRoute is Nav3Route.Checkout) "Checkout Activity" else "Products Activity", + style = MaterialTheme.typography.titleLarge, + modifier = Modifier.weight(1f), + ) + IconButton( + onClick = { showTransactionHistorySheet = true }, + modifier = + Modifier.sentryTag(nav3InteractionTag("Recent Transactions")) + .testTag(nav3TestTag("top_bar_recent_transactions")), + ) { + Icon(Icons.Filled.AccountTree, contentDescription = "Recent transactions") + } + IconButton( + onClick = { + showRouteWorkDialog(activity, routeWorkOptions) { routeWorkOptions = it } + }, + modifier = + Modifier.sentryTag(nav3InteractionTag("Route Work Settings")) + .testTag(nav3TestTag("top_bar_route_work_settings")), + ) { + Icon(Icons.Filled.Settings, contentDescription = "Route work settings") + } + } + Text("Current route: ${backStack.last().displayRoute()}") + } + }, + bottomBar = { + SentryControls( + onCaptureException = { captureSampleException("Nav3 Multi-Activity") }, + onCrashApp = { showCrashConfirmation = true }, + ) + }, + ) { padding -> + NavDisplay( + backStack = backStack, + modifier = Modifier.fillMaxSize().padding(padding), + onBack = onBack, + entryProvider = + entryProvider { + entry { route -> + TracedNav3Route(route, Nav3Scenario.MULTI_ACTIVITY) { + Nav3RouteWorkEffect(route, routeWorkOptions) + ProductListRoute(backStack) + } + } + entry { route -> + TracedNav3Route(route, Nav3Scenario.MULTI_ACTIVITY) { + Nav3RouteWorkEffect(route, routeWorkOptions) + ProductDetailRoute( + route = route, + backStack = backStack, + onCheckout = { onCheckout(route.productId) }, + showOverlays = false, + ) + } + } + entry { route -> + TracedNav3Route(route, Nav3Scenario.MULTI_ACTIVITY) { + Nav3RouteWorkEffect(route, routeWorkOptions) + CheckoutRoute(route, backStack) + } + } + entry { route -> + TracedNav3Route(route, Nav3Scenario.MULTI_ACTIVITY) { + Nav3RouteWorkEffect(route, routeWorkOptions) + ConfirmationRoute(route, backStack, rootRoute = initialRoute) + } + } + }, + ) + } + if (showCrashConfirmation) { + AlertDialog( + onDismissRequest = { showCrashConfirmation = false }, + title = { Text("Crash app?") }, + text = { Text("This will throw an uncaught exception and close the sample app.") }, + dismissButton = { + TextButton(onClick = { showCrashConfirmation = false }) { Text("Cancel") } + }, + confirmButton = { + TextButton( + onClick = { + showCrashConfirmation = false + crashSampleApp("Nav3 Multi-Activity") + } + ) { + Text("Crash") + } + }, + ) + } + if (showTransactionHistorySheet) { + NavigationTransactionHistorySheet( + sampleName = "Nav3", + transactions = NavigationTransactionHistory.transactions, + showActivityUiLoadTransactionDelayMessage = configuration.hasOnlyActivityUiLoadTransactions, + onDismissRequest = { showTransactionHistorySheet = false }, + onOpenTransaction = { url -> + activity.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(url))) + }, + onDumpTransactionUrl = { url -> + Log.i("Nav3MultiActivity", "Sentry transaction URL: $url") + Toast.makeText(activity, "Dumped transaction URL to logcat.", Toast.LENGTH_SHORT).show() + }, + onCopyTransactionUrl = { url -> + val clipboard = activity.getSystemService(ClipboardManager::class.java) + clipboard.setPrimaryClip(ClipData.newPlainText("Sentry transaction URL", url)) + Toast.makeText(activity, "Copied transaction URL to clipboard.", Toast.LENGTH_SHORT).show() + }, + ) + } +} + +private const val EXTRA_PRODUCT_ID = "io.sentry.samples.android.navigation.nav3.product_id" diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3RouteScreens.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3RouteScreens.kt index 1d4b933fdc..b47c68df3e 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3RouteScreens.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3RouteScreens.kt @@ -37,6 +37,7 @@ import androidx.compose.runtime.snapshots.SnapshotStateList import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.colorResource import androidx.compose.ui.text.font.FontWeight @@ -47,6 +48,7 @@ import io.sentry.Sentry import io.sentry.compose.SentryModifier.sentryTag import io.sentry.compose.SentryTraced import io.sentry.samples.android.R +import io.sentry.samples.android.getActivity import io.sentry.samples.android.navigation.common.RouteNames import io.sentry.samples.android.navigation.common.RouteSpec import io.sentry.samples.android.navigation.common.RouteSpecs @@ -65,7 +67,7 @@ internal fun TracedNav3Route( scenario: Nav3Scenario, content: @Composable BoxScope.() -> Unit, ) { - tagCurrentNav3Scenario(scenario) + tagCurrentNav3Scenario(scenario, LocalContext.current.getActivity().javaClass.simpleName) SentryTraced( tag = "Nav3 /${route.routeName}", enableUserInteractionTracing = false, @@ -73,8 +75,19 @@ internal fun TracedNav3Route( ) } -private fun tagCurrentNav3Scenario(scenario: Nav3Scenario) { - Sentry.getSpan()?.setTag("sample_nav3_scenario", scenario.label) +private fun tagCurrentNav3Scenario(scenario: Nav3Scenario, activityName: String) { + val sessionId = NavigationTransactionHistory.sessionId + Sentry.getSpan()?.let { span -> + span.setTag(NAV3_SCENARIO_TAG, scenario.label) + span.setTag(NAVIGATION_SAMPLE_ACTIVITY_TAG, activityName) + span.setTag(NAVIGATION_SAMPLE_SESSION_TAG, sessionId) + } + Sentry.configureScope { scope -> + scope.withTransaction { transaction -> + transaction?.setTag(NAVIGATION_SAMPLE_ACTIVITY_TAG, activityName) + transaction?.setTag(NAVIGATION_SAMPLE_SESSION_TAG, sessionId) + } + } tagCurrentNavigationSampleScenario(scenario.label) } @@ -496,6 +509,8 @@ internal fun ProductListRoute(backStack: SnapshotStateList) { internal fun ProductDetailRoute( route: Nav3Route.ProductDetail, backStack: SnapshotStateList, + onCheckout: () -> Unit = { backStack.add(Nav3Route.Checkout(route.productId)) }, + showOverlays: Boolean = true, ) { LaunchedEffect(route.productId, route.source, route.campaign) { recordSimulatedBackgroundSpan(RouteNames.PRODUCT_DETAIL) @@ -508,19 +523,21 @@ internal fun ProductDetailRoute( RouteSpecs.productDetail.displayArguments(route.arguments).forEach { (label, value) -> RouteInfo(label, value) } - RouteButton( - "Show Promo Dialog", - onClick = { backStack.add(Nav3Route.PromoDialog("detail-${route.productId}")) }, - testTag = nav3TestTag("product_detail_show_promo_dialog"), - ) - RouteButton( - "Open Share Sheet", - onClick = { backStack.add(Nav3Route.ShareSheet(route.productId)) }, - testTag = nav3TestTag("product_detail_open_share_sheet"), - ) + if (showOverlays) { + RouteButton( + "Show Promo Dialog", + onClick = { backStack.add(Nav3Route.PromoDialog("detail-${route.productId}")) }, + testTag = nav3TestTag("product_detail_show_promo_dialog"), + ) + RouteButton( + "Open Share Sheet", + onClick = { backStack.add(Nav3Route.ShareSheet(route.productId)) }, + testTag = nav3TestTag("product_detail_open_share_sheet"), + ) + } RouteButton( "Go to Checkout", - onClick = { backStack.add(Nav3Route.Checkout(route.productId)) }, + onClick = onCheckout, testTag = nav3TestTag("product_detail_go_to_checkout"), ) }, diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Routes.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Routes.kt index b9972a6cd7..8e08913990 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Routes.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3Routes.kt @@ -26,6 +26,14 @@ internal fun Nav3Route.routeSpec(): RouteSpec = "Reuses the single-stack shopping flow while simulating a power user who starts their " + "own manual transactions.", ) + Nav3Route.MultiActivity -> + RouteSpec( + routeName = Nav3Route.MultiActivity.routeName, + title = "Multi-Activity", + description = + "Tests how SentryNavEffect uses NavScopes to coordinate state updates between " + + "SentryNavEffect instances. Each Activity owns its own SentryNavEffect and NavDisplay.", + ) Nav3Route.DeepLink -> RouteSpec( routeName = Nav3Route.DeepLink.routeName, @@ -69,6 +77,7 @@ private fun Nav3Route.toSavedState(): Bundle = Nav3Route.Landing -> putString("type", "landing") Nav3Route.SingleStack -> putString("type", "single_stack") Nav3Route.Custom -> putString("type", "custom") + Nav3Route.MultiActivity -> putString("type", "multi_activity") Nav3Route.DeepLink -> putString("type", "deep_link") Nav3Route.ProductList -> putString("type", "product_list") is Nav3Route.ProductDetail -> { @@ -108,6 +117,7 @@ private fun Bundle.toNav3Route(): Nav3Route { "landing" -> Nav3Route.Landing "single_stack" -> Nav3Route.SingleStack "custom" -> Nav3Route.Custom + "multi_activity" -> Nav3Route.MultiActivity "deep_link" -> Nav3Route.DeepLink "product_list" -> Nav3Route.ProductList "product_detail" -> { @@ -148,6 +158,7 @@ internal fun SnapshotStateList.openScenario(scenario: Nav3Scenario) { Nav3Scenario.LANDING -> resetTo(Nav3Route.Landing) Nav3Scenario.SINGLE_STACK -> resetTo(Nav3Route.SingleStack) Nav3Scenario.CUSTOM -> resetTo(Nav3Route.Custom) + Nav3Scenario.MULTI_ACTIVITY -> resetTo(Nav3Route.MultiActivity) Nav3Scenario.DEEP_LINK -> resetTo(Nav3Route.DeepLink) Nav3Scenario.MULTIPANE -> resetTo(Nav3Route.Multipane) Nav3Scenario.MULTIPLE_STACKS -> resetTo(Nav3Route.Multistack) @@ -211,6 +222,10 @@ internal sealed interface Nav3Route { override val routeName: String = "Custom" } + data object MultiActivity : Nav3Route { + override val routeName: String = "MultiActivity" + } + data object DeepLink : Nav3Route { override val routeName: String = RouteNames.DEEP_LINK } @@ -282,6 +297,7 @@ internal enum class Nav3Scenario(val label: String, val showTab: Boolean = true) SINGLE_STACK("Single Stack"), DEEP_LINK("Deep Link"), CUSTOM("Custom"), + MULTI_ACTIVITY("Multi-Activity"), MULTIPANE("Multipane"), MULTIPLE_STACKS("Multistack"), PERFORMANCE("Performance"), @@ -293,6 +309,7 @@ internal val Nav3Scenario.initialRoute: Nav3Route Nav3Scenario.LANDING -> Nav3Route.Landing Nav3Scenario.SINGLE_STACK -> Nav3Route.SingleStack Nav3Scenario.CUSTOM -> Nav3Route.Custom + Nav3Scenario.MULTI_ACTIVITY -> Nav3Route.MultiActivity Nav3Scenario.DEEP_LINK -> Nav3Route.DeepLink Nav3Scenario.MULTIPANE -> Nav3Route.Multipane Nav3Scenario.MULTIPLE_STACKS -> Nav3Route.Multistack diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3SampleTracing.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3SampleTracing.kt index 4a9b77227b..e68421824b 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3SampleTracing.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3SampleTracing.kt @@ -73,7 +73,9 @@ internal const val SENTRY_FLUSH_TIMEOUT_MILLIS = 5000L private const val BACKGROUND_WORK_MILLIS = 1000L private const val NAV2_SCENARIO_TAG = "sample_nav2_scenario" -private const val NAV3_SCENARIO_TAG = "sample_nav3_scenario" +internal const val NAV3_SCENARIO_TAG = "sample_nav3_scenario" +internal const val NAVIGATION_SAMPLE_ACTIVITY_TAG = "sample_navigation_activity" +internal const val NAVIGATION_SAMPLE_SESSION_TAG = "sample_navigation_session" private const val NAVIGATION_SAMPLE_SCENARIO_TAG = "sample_navigation_scenario" private const val UNKNOWN_NAVIGATION_SCENARIO_LABEL = "Unknown" private const val ACTIVITY_UI_LOAD_OP = "ui.load" diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3SetupActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3SetupActivity.kt index db945d4519..7b37bbeaca 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3SetupActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3SetupActivity.kt @@ -35,6 +35,7 @@ class Nav3SetupActivity : AppCompatActivity() { onLaunch = { val previousOptions = currentNavigationSampleConfigSnapshot() configuration.applyToCurrentOptions() + NavigationTransactionHistory.startSession() startActivity(nav3LaunchIntent(configuration, previousOptions)) }, ) diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistory.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistory.kt index 38cd6d2039..a54f680b76 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistory.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistory.kt @@ -8,12 +8,19 @@ import io.sentry.protocol.SentrySpan import io.sentry.protocol.SentryTransaction import io.sentry.samples.android.SampleBeforeSendTransactionHook -/** State holder backing the navigation sample transaction history sheet. */ -internal class NavigationTransactionHistory(private val isActive: () -> Boolean) { +/** Process-wide state holder backing every Nav3 transaction history sheet. */ +internal object NavigationTransactionHistory { - val transactions = mutableStateListOf() + private val store = NavigationTransactionHistoryStore() + @Volatile private var sessionGeneration = 0L + + val transactions: List + get() = store.transactions + + val sessionId: String + get() = sessionGeneration.toString() - private val mainHandler = Handler(Looper.getMainLooper()) + private val mainHandler by lazy { Handler(Looper.getMainLooper()) } private val transactionListener: (SentryTransaction, String?) -> Unit = { transaction, dsn -> add(transaction, dsn) } @@ -24,34 +31,57 @@ internal class NavigationTransactionHistory(private val isActive: () -> Boolean) SampleBeforeSendTransactionHook.addListener(transactionListener) } - fun uninstall() { + fun startSession() { + install() + sessionGeneration++ clear() - SampleBeforeSendTransactionHook.removeListener(transactionListener) } fun clear() { - transactions.clear() + store.clear() } private fun add(transaction: SentryTransaction, dsn: String?) { - if (!isActive()) { + val transactionSessionId = transaction.getTag(NAVIGATION_SAMPLE_SESSION_TAG) + if (transaction.getTag(NAV3_SCENARIO_TAG) == null || transactionSessionId != sessionId) { return } val trace = transaction.toNavigationTransactionTrace(dsn) if (Looper.myLooper() == Looper.getMainLooper()) { - transactions.addMostRecent(trace) + if (transactionSessionId == sessionId) { + store.add(trace) + } } else { mainHandler.post { - if (isActive()) { - transactions.addMostRecent(trace) + if (transactionSessionId == sessionId) { + store.add(trace) } } } } } +internal class NavigationTransactionHistoryStore( + private val transactionLimit: Int = TRANSACTION_HISTORY_LIMIT +) { + + val transactions = mutableStateListOf() + + fun add(transaction: NavigationTransactionTrace) { + transactions.add(0, transaction) + while (transactions.size > transactionLimit) { + transactions.removeAt(transactions.lastIndex) + } + } + + fun clear() { + transactions.clear() + } +} + internal data class NavigationTransactionTrace( + val activity: String, val name: String, val operation: String, val eventId: String, @@ -73,7 +103,7 @@ internal data class NavigationTraceSpan( val children: List = emptyList(), ) -private fun SentryTransaction.toNavigationTransactionTrace( +internal fun SentryTransaction.toNavigationTransactionTrace( dsn: String? ): NavigationTransactionTrace { val trace = contexts.trace @@ -97,6 +127,7 @@ private fun SentryTransaction.toNavigationTransactionTrace( .sortedBy { span -> span.startOffsetMillis } return NavigationTransactionTrace( + activity = getTag(NAVIGATION_SAMPLE_ACTIVITY_TAG) ?: UNKNOWN_NAVIGATION_ACTIVITY, name = transaction ?: "", operation = trace?.operation ?: "transaction", eventId = eventId, @@ -109,15 +140,6 @@ private fun SentryTransaction.toNavigationTransactionTrace( ) } -private fun MutableList.addMostRecent( - transaction: NavigationTransactionTrace -) { - add(0, transaction) - while (size > TRANSACTION_HISTORY_LIMIT) { - removeAt(lastIndex) - } -} - private fun SentrySpan.toNavigationTraceSpan( transactionStartTimestamp: Double ): NavigationTraceSpan = @@ -172,5 +194,6 @@ private fun String.projectIdFromDsn(): String? = projectId.isNotEmpty() && projectId.all { it.isDigit() } } -private const val TRANSACTION_HISTORY_LIMIT = 10 +private const val TRANSACTION_HISTORY_LIMIT = 25 private const val SENTRY_SAMPLE_ORG_SLUG = "sentry-sdks" +private const val UNKNOWN_NAVIGATION_ACTIVITY = "Unknown" diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistorySheet.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistorySheet.kt index 0e0038083e..d0cd2c1af9 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistorySheet.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/navigation/nav3/Nav3TransactionHistorySheet.kt @@ -66,7 +66,7 @@ internal fun NavigationTransactionHistorySheet( modifier = Modifier.testTag(historyTag(sampleName, "title")), ) Text( - "Displays the last 10 finished transactions emitted by the SDK. Transactions with child work appear here after their work finishes.", + "Displays the last 25 finished Nav3 transactions emitted by the SDK. Transactions with child work appear here after their work finishes.", style = MaterialTheme.typography.bodyMedium, ) Column( @@ -133,7 +133,11 @@ private fun TransactionCard( modifier = Modifier.padding(12.dp).testTag(historyTag(sampleName, "card_${index}_content")), verticalArrangement = Arrangement.spacedBy(10.dp), ) { - Row(verticalAlignment = Alignment.CenterVertically) { + Row( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(12.dp), + verticalAlignment = Alignment.Top, + ) { Column( modifier = Modifier.weight(1f).testTag(historyTag(sampleName, "card_${index}_summary")) ) { @@ -152,12 +156,20 @@ private fun TransactionCard( modifier = Modifier.testTag(historyTag(sampleName, "card_${index}_operation")), ) } - Text( - "Tab: ${transaction.tab}", - style = MaterialTheme.typography.labelLarge, - fontFamily = FontFamily.Monospace, - modifier = Modifier.testTag(historyTag(sampleName, "card_${index}_tab")), - ) + Column(horizontalAlignment = Alignment.End) { + Text( + transaction.activity, + style = MaterialTheme.typography.labelLarge, + fontFamily = FontFamily.Monospace, + modifier = Modifier.testTag(historyTag(sampleName, "card_${index}_activity")), + ) + Text( + "Tab: ${transaction.tab}", + style = MaterialTheme.typography.labelLarge, + fontFamily = FontFamily.Monospace, + modifier = Modifier.testTag(historyTag(sampleName, "card_${index}_tab")), + ) + } } TransactionWaterfall(transaction, historyTag(sampleName, "card_${index}_waterfall")) diff --git a/sentry-samples/sentry-samples-android/src/test/java/io/sentry/samples/android/navigation/nav3/NavigationTransactionActivityMappingTest.kt b/sentry-samples/sentry-samples-android/src/test/java/io/sentry/samples/android/navigation/nav3/NavigationTransactionActivityMappingTest.kt new file mode 100644 index 0000000000..2d3f145ddd --- /dev/null +++ b/sentry-samples/sentry-samples-android/src/test/java/io/sentry/samples/android/navigation/nav3/NavigationTransactionActivityMappingTest.kt @@ -0,0 +1,40 @@ +package io.sentry.samples.android.navigation.nav3 + +import com.google.common.truth.Truth.assertThat +import io.sentry.protocol.MeasurementValue +import io.sentry.protocol.SentrySpan +import io.sentry.protocol.SentryTransaction +import io.sentry.protocol.TransactionInfo +import kotlin.test.Test + +class NavigationTransactionActivityMappingTest { + + @Test + fun `trace uses the Activity tag recorded on the transaction`() { + val transaction = + transaction().apply { + setTag(NAVIGATION_SAMPLE_ACTIVITY_TAG, "Nav3ProductsActivity") + } + + val trace = transaction.toNavigationTransactionTrace(dsn = null) + + assertThat(trace.activity).isEqualTo("Nav3ProductsActivity") + } + + @Test + fun `trace uses an unknown-activity label when the transaction has no Activity tag`() { + val trace = transaction().toNavigationTransactionTrace(dsn = null) + + assertThat(trace.activity).isEqualTo("Unknown") + } + + private fun transaction() = + SentryTransaction( + "/ProductList", + 1.0, + 2.0, + emptyList(), + emptyMap(), + TransactionInfo("route"), + ) +} diff --git a/sentry-samples/sentry-samples-android/src/test/java/io/sentry/samples/android/navigation/nav3/NavigationTransactionHistoryStoreTest.kt b/sentry-samples/sentry-samples-android/src/test/java/io/sentry/samples/android/navigation/nav3/NavigationTransactionHistoryStoreTest.kt new file mode 100644 index 0000000000..e32860ceb8 --- /dev/null +++ b/sentry-samples/sentry-samples-android/src/test/java/io/sentry/samples/android/navigation/nav3/NavigationTransactionHistoryStoreTest.kt @@ -0,0 +1,57 @@ +package io.sentry.samples.android.navigation.nav3 + +import com.google.common.truth.Truth.assertThat +import kotlin.test.Test + +class NavigationTransactionHistoryStoreTest { + + @Test + fun `records transactions from multiple activities in one history`() { + val store = NavigationTransactionHistoryStore() + + store.add(trace(activity = "Nav3ProductsActivity", name = "/ProductDetail")) + store.add(trace(activity = "Nav3CheckoutActivity", name = "/Checkout")) + + assertThat(store.transactions.map { it.activity }) + .containsExactly("Nav3CheckoutActivity", "Nav3ProductsActivity") + .inOrder() + assertThat(store.transactions.map { it.name }) + .containsExactly("/Checkout", "/ProductDetail") + .inOrder() + } + + @Test + fun `retains only the most recent transactions`() { + val store = NavigationTransactionHistoryStore(transactionLimit = 2) + + store.add(trace(activity = "FirstActivity", name = "/First")) + store.add(trace(activity = "SecondActivity", name = "/Second")) + store.add(trace(activity = "ThirdActivity", name = "/Third")) + + assertThat(store.transactions.map { it.name }).containsExactly("/Third", "/Second").inOrder() + } + + @Test + fun `clearing starts the next sample session with empty history`() { + val store = NavigationTransactionHistoryStore() + store.add(trace(activity = "Nav3ProductsActivity", name = "/ProductList")) + + store.clear() + + assertThat(store.transactions).isEmpty() + } + + private fun trace(activity: String, name: String) = + NavigationTransactionTrace( + activity = activity, + name = name, + operation = "navigation", + eventId = "event-id", + traceId = "trace-id", + status = "OK", + tab = "Multi-Activity", + durationMillis = 1.0, + sentryUrl = null, + spans = emptyList(), + ) +}