From bdfce6e43a2613fe752245e3e7e032a48b067431 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 6 Nov 2026 05:37:14 +0100 Subject: [PATCH 1/2] feat(android): Start a new Nav3 transaction when returning to a nav root A nav root that regains the nav lease (e.g., when the Activity covering it closes, the user swipes back to its ViewPager2 page, or a dialog over it is dismissed) now gets a new nav transaction for its current destination if its previous one has finished. Before, spans produced on return had no transaction to attach to until the next navigation. StagedNavTransaction keeps the destination's NavTransactionRequest after its transaction finishes or can't start, and starts a new transaction from it on the next publication. The request replaces the NavTransaction handle that recorded writes for replay, and is kept in step with back stack changes below the top via updateNavTransactionRequest. --- CHANGELOG.md | 4 + .../compose/navigation3/BackStackObserver.kt | 129 ++---- .../navigation3/lifecycle/NavScopes.kt | 65 ++- .../navigation3/lifecycle/NavTransaction.kt | 149 ------- .../lifecycle/NavTransactionRequest.kt | 103 +++++ .../compose/navigation3/lifecycle/README.md | 33 +- .../lifecycle/StagedNavTransaction.kt | 206 +++++----- .../navigation3/BackStackObserverTest.kt | 6 +- .../CoordinatedBackStackObserverTest.kt | 133 +++++- ...SentryNavEffectLifecycleIntegrationTest.kt | 110 ++++- .../navigation3/lifecycle/NavScopeTest.kt | 76 ++-- .../lifecycle/NavScopesIntegrationTest.kt | 8 +- .../navigation3/lifecycle/NavScopesTest.kt | 191 +++++++-- .../lifecycle/NavTransactionRequestTest.kt | 244 +++++++++++ .../lifecycle/NavTransactionTest.kt | 348 ---------------- .../lifecycle/StagedNavTransactionTest.kt | 385 ++++++++++++------ 16 files changed, 1224 insertions(+), 966 deletions(-) delete mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransaction.kt create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionRequest.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionRequestTest.kt delete mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index da49e486cb1..e0dd01679b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ - Quartz `SentryJobListener` can send a monitor config derived from the trigger with check-ins, so Sentry creates or updates the monitor from code. Set the `sentry-upsert-monitor-config` job or trigger data key to `"true"` to send the schedule ([#6216](https://github.com/getsentry/sentry-java/pull/6216)) - Send a monitor config derived from Spring `@Scheduled` with `@SentryCheckIn` check-ins, so Sentry creates or updates the monitor from code. Set `@SentryCheckIn(upsertMonitorConfig = true)` to send the schedule ([#6215](https://github.com/getsentry/sentry-java/pull/6215)) +### Behavioral Changes + +- Nav3: Returning to an Activity or Fragment with its own `SentryNavEffect` within the app (e.g., closing the Activity that covered it, swiping back to its ViewPager2 page, or dismissing a dialog over it) now starts a new navigation transaction for the current destination if the previous one finished, so spans produced upon return can now be consistently captured ([#XXXX](https://github.com/getsentry/sentry-java/pull/XXXX)) + ## 8.63.0 ### Features 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 0becec8b705..d3e92777ce3 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 @@ -5,23 +5,18 @@ import io.sentry.Hint import io.sentry.SentryIntegrationPackageStorage import io.sentry.SentryLevel.INFO import io.sentry.SentryOptions -import io.sentry.TransactionContext -import io.sentry.TransactionOptions import io.sentry.compose.navigation3.BackStackConverter.RetentionPolicy import io.sentry.compose.navigation3.PreparedChange.BackStackHasNewTop import io.sentry.compose.navigation3.PreparedChange.BackStackHasSameTop import io.sentry.compose.navigation3.PreparedChange.BackStackIsEmpty import io.sentry.compose.navigation3.lifecycle.INavScope import io.sentry.compose.navigation3.lifecycle.INavScopes -import io.sentry.compose.navigation3.lifecycle.INavTransaction import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY -import io.sentry.protocol.App -import io.sentry.protocol.TransactionNameSource +import io.sentry.compose.navigation3.lifecycle.NAVIGATION_OP +import io.sentry.compose.navigation3.lifecycle.NavTransactionRequest import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion import java.lang.ref.WeakReference -private const val NAVIGATION_OP: String = "navigation" - /** * Observes the back stack managed by a single [SentryNavEffect] and records Sentry state as the * back stack is updated. @@ -57,10 +52,9 @@ internal class BackStackObserver( private val backStackConverter = BackStackConverter(entryMapper, scopes.options.logger) - private val navTransactions = NavTransactions(scopes) private val navBreadcrumbs = NavBreadcrumbs(scopes) private val navScreen = NavScreen() - private val navContext = NavContext(scopes, options) + private val navContext = NavContext(options) // Safe because the host back stack retains the current top entry strongly between updates. private var previousTop: WeakReference? = null @@ -192,16 +186,10 @@ internal class BackStackObserver( navBreadcrumbs.emit(fromEntry = previousTop, toEntry = currentBackStack.topEntryNormalized) } - navTransactions.stop(scope) + scope.finishNavTransaction() if (areNavigationTransactionsEnabled) { - navTransactions - .start( - scope, - currentTop.name, - currentTop.argumentsWithMetadata(), - ) - ?.let { transaction -> navContext.updateTransaction(transaction, scope, currentBackStack) } + scope.startNavTransaction(navTransactionRequestFor(currentBackStack)) } else { scope.rotatePropagationContext() } @@ -211,6 +199,13 @@ internal class BackStackObserver( private fun handleSameTop(scope: INavScope, backStack: BackStackData) { navContext.update(scope, backStack.capturedEntriesNormalized) + + // Keeps the nav context of transactions started later for the same destination (e.g., when the + // user returns to the nav root) in step with the back stack. + if (areNavigationTransactionsEnabled) { + scope.updateNavTransactionRequest(navTransactionRequestFor(backStack)) + } + storePreviousTop(backStack) } @@ -228,8 +223,20 @@ internal class BackStackObserver( previousTopNormalized = null } + private fun navTransactionRequestFor(backStack: BackStackData): NavTransactionRequest { + val top = backStack.topEntryNormalized + + return NavTransactionRequest.forDestination( + name = top.name, + arguments = top.argumentsWithMetadata(), + contexts = navContext.forTransaction(backStack.capturedEntriesNormalized), + viewNames = if (scopes.options.isEnableScreenTracking) listOf(top.name) else null, + options = scopes.options, + ) + } + private fun tearDown(scope: INavScope) { - navTransactions.stop(scope) + scope.finishNavTransaction() navContext.clear(scope) navScreen.clear(scope) clearPreviousTop() @@ -274,58 +281,8 @@ private data class BackStackData( val capturedEntriesNormalized: List, ) -/** A helper class for managing nav transactions. */ -private class NavTransactions(private val scopes: INavScopes) { - - private companion object { - private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" - } - - /** - * Starts an idle navigation transaction, or no-ops if another transaction is already active. - * - * [Stop][stop] the current navigation transaction first. See [INavScope.startNavTransaction]. - */ - fun start( - scope: INavScope, - name: String, - arguments: Map, - ): INavTransaction? { - val transactionOptions = - TransactionOptions().also { - it.isWaitForChildren = true - it.idleTimeout = scopes.options.idleTimeout - val deadlineTimeoutMillis = scopes.options.deadlineTimeout - it.deadlineTimeout = if (deadlineTimeoutMillis <= 0) null else deadlineTimeoutMillis - it.isTrimEnd = true - it.origin = TRANSACTION_ORIGIN - } - - val transaction = - scope.startNavTransaction( - TransactionContext(name, TransactionNameSource.ROUTE, NAVIGATION_OP), - transactionOptions, - ) ?: return null - - if (arguments.isNotEmpty()) { - transaction.setData("arguments", arguments) - } - - return transaction - } - - /** - * Finishes and unsets the active navigation transaction, if one exists. - * - * See [INavScope.finishNavTransaction]. - */ - fun stop(scope: INavScope) { - scope.finishNavTransaction() - } -} - /** A helper class for updating [nav context][NAVIGATION_CONTEXT_KEY]. */ -private class NavContext(private val scopes: INavScopes, private val options: SentryNavOptions) { +private class NavContext(private val options: SentryNavOptions) { private companion object { private const val BACKSTACK_KEY = "Back Stack" @@ -345,33 +302,21 @@ private class NavContext(private val scopes: INavScopes, private val options: Se } /** - * Updates the transaction with the provided navigation info. + * The nav context to set on the nav transaction for a destination whose back stack is + * [capturedEntries], if any. * * Needed because transactions inherit base scope context on a per-key basis unless transactions - * have their own values for those keys. In our case, we need to keep fresh back stack and route - * values in the base context for purposes of crash reporting. But those values will often advance - * past what's relevant to a given transaction. This method prevents misassociation by binding - * proper values to the transaction context instead. + * have their own values for those keys. In our case, we need to keep fresh back stack values in + * the base context for purposes of crash reporting. But those values will often advance past + * what's relevant to a given transaction. Setting the destination's own back stack on its + * transaction prevents misassociation. */ - fun updateTransaction( - transaction: INavTransaction, - scope: INavScope, - backStack: BackStackData, - ) { - if (scopes.options.isEnableScreenTracking) { - val appContext = scope.app ?: App() - - appContext.viewNames = listOf(backStack.topEntryNormalized.name) - transaction.setApp(appContext) - } - - if (options.captureBackStack && backStack.capturedEntriesNormalized.isNotEmpty()) { - transaction.setContext( - NAVIGATION_CONTEXT_KEY, - backStack.capturedEntriesNormalized.toBackStackMap(), - ) + fun forTransaction(capturedEntries: List): Map = + if (options.captureBackStack && capturedEntries.isNotEmpty()) { + mapOf(NAVIGATION_CONTEXT_KEY to capturedEntries.toBackStackMap()) + } else { + emptyMap() } - } /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */ private fun List.toBackStackMap(): Map = diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt index 8c1c6d32b7e..39568fa2a29 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt @@ -8,10 +8,6 @@ import io.sentry.IScopes import io.sentry.PropagationContext import io.sentry.SentryLevel import io.sentry.SentryOptions -import io.sentry.TransactionContext -import io.sentry.TransactionOptions -import io.sentry.protocol.App -import io.sentry.protocol.Contexts /** The key under which nav context (i.e., the captured back stack) is stored in a scope. */ internal const val NAVIGATION_CONTEXT_KEY: String = "Navigation" @@ -194,15 +190,6 @@ internal class NavScopes( } } - /** - * Copies the whole of [IScope.getContexts] in order to return a copy of its [App]. - * - * [App]'s copy constructor isn't public, so an `App` can't be copied on its own. Going through - * [Contexts] deep-copies every context on the scope and discards all of it but the `App`. - */ - override val app: App? - get() = Contexts(delegate.contexts).app - private val hasLease: Boolean get() = registration?.hasLease == true @@ -221,6 +208,10 @@ internal class NavScopes( } } + override fun updateNavTransactionRequest(request: NavTransactionRequest) { + stagedNavTransaction.updateRequest(request) + } + /** * Starts and binds a nav transaction with the nav lease, or defers it until the lease is * acquired (see [StagedNavTransaction]). Callers must [finish][finishNavTransaction] the @@ -234,15 +225,12 @@ internal class NavScopes( * meantime attach to whichever transaction is bound to the scope at that time, usually the * leaseholder's. */ - override fun startNavTransaction( - transactionContext: TransactionContext, - transactionOptions: TransactionOptions, - ): INavTransaction? { - return if (hasLease) { - stagedNavTransaction.start(delegate, transactionContext, transactionOptions) + override fun startNavTransaction(request: NavTransactionRequest) { + stagedNavTransaction.stage(request) + if (hasLease) { + stagedNavTransaction.publishTo(delegate) } else { stagedNavData.deferPropagationContextRotation() - stagedNavTransaction.stage(transactionContext, transactionOptions) } } @@ -286,17 +274,6 @@ internal sealed interface INavScope { var screen: String? - /** - * A copy of the scope's app context, if any. - * - * A copy rather than the live instance, because callers mutate the result before binding it to a - * transaction, and mutating the scope's own [App] would corrupt shared scope state. - * - * Implementations may have to copy more than the [App] to produce it, so treat this as expensive: - * read it once where it's needed rather than per frame or in a loop. - */ - val app: App? - /** * Sets or, given `null`, clears the value stored under [NAVIGATION_CONTEXT_KEY]. * @@ -309,21 +286,29 @@ internal sealed interface INavScope { fun rotatePropagationContext() /** - * Starts a nav transaction and binds it to the scope, unless another unfinished transaction is - * already bound to it. Returns `null` if no transaction was started. + * Replaces the request kept from the most recent [startNavTransaction] with [request] (e.g., to + * reflect a back stack whose entries below the top changed), so that transactions started for the + * same destination later are started from it. A transaction that has already started keeps what + * it was started with. + * + * Does nothing if no request is kept, i.e., none was made since the last [finishNavTransaction]. + */ + fun updateNavTransactionRequest(request: NavTransactionRequest) + + /** + * Starts a nav transaction for [request] and binds it to the scope, unless another unfinished + * transaction is already bound to it, in which case none is started. * * (Note: Callers must first [finish][finishNavTransaction] the nav transaction most recently * requested, if any. Otherwise, with the nav lease, it blocks the new one while it's bound; * without the lease, it's replaced without being finished.) * - * The start may be deferred until the nav lease is acquired, in which case writes to the returned - * transaction are staged in the meantime, and whether another transaction blocks it is decided - * then (see [StagedNavTransaction.publishTo]). + * The start may be deferred until the nav lease is acquired, in which case whether another + * transaction blocks it is decided then (see [StagedNavTransaction.publishTo]). Either way, the + * request is kept until the next [finishNavTransaction], so that a new transaction can be started + * from it if the user returns to the nav root after this one finished. */ - fun startNavTransaction( - transactionContext: TransactionContext, - transactionOptions: TransactionOptions, - ): INavTransaction? + fun startNavTransaction(request: NavTransactionRequest) /** * Finishes the nav transaction most recently requested via [startNavTransaction] and unbinds it diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransaction.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransaction.kt deleted file mode 100644 index c8c1464687c..00000000000 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransaction.kt +++ /dev/null @@ -1,149 +0,0 @@ -package io.sentry.compose.navigation3.lifecycle - -import io.sentry.IScopes -import io.sentry.ITransaction -import io.sentry.SpanStatus -import io.sentry.TransactionContext -import io.sentry.TransactionOptions -import io.sentry.protocol.App - -/** - * An [INavTransaction] handle backed by an [ITransaction] that's either started right away or, when - * the nav lease isn't held, [started later][start]. - * - * A deferred handle records what's set on it and replays it onto the transaction once started. - * [Finishing][finish] a deferred handle before it starts discards it, so its transaction never - * starts. - * - * **Thread safety** - * - * This class is ***not*** thread-safe. Apply the thread safety policy from [NavScopes]. - */ -internal class NavTransaction -private constructor( - val name: String, - private var state: State, -) : INavTransaction { - - /** - * The backing transaction, once [started][State.Started]; `null` while [pending][State.Pending] - * or once [discarded][State.Discarded]. - */ - val transaction: ITransaction? - get() = (state as? State.Started)?.transaction - - override fun setData(key: String, value: Any) { - when (val state = state) { - is State.Pending -> state.data[key] = value - is State.Started -> state.transaction.setData(key, value) - State.Discarded -> Unit - } - } - - override fun setContext(key: String, value: Any) { - when (val state = state) { - is State.Pending -> state.contexts[key] = value - is State.Started -> state.transaction.setContext(key, value) - State.Discarded -> Unit - } - } - - override fun setApp(app: App) { - when (val state = state) { - is State.Pending -> state.app = app - is State.Started -> state.transaction.contexts.setApp(app) - State.Discarded -> Unit - } - } - - /** - * Starts a deferred handle's transaction via [scopes] and replays what was set on the handle onto - * it. - * - * Returns the started transaction, or `null` if the handle was started already or discarded, or - * [scopes] declined to start one. - */ - fun start(scopes: IScopes): ITransaction? { - val pending = state as? State.Pending ?: return null - val transaction = - scopes.startTransaction(pending.context, pending.options).takeUnless { it.isNoOp } - ?: return null - - state = State.Started(transaction) - pending.data.forEach { (key, value) -> transaction.setData(key, value) } - pending.contexts.forEach { (key, value) -> transaction.setContext(key, value) } - pending.app?.let { transaction.contexts.setApp(it) } - return transaction - } - - /** - * Finishes the backing transaction with its own status, or [OK][SpanStatus.OK] if it has none, - * and then passes it to [onFinished]. - * - * A deferred handle that hasn't started is discarded instead, so it never starts, and - * [onFinished] isn't called. - */ - fun finish(onFinished: (ITransaction) -> Unit) { - when (val state = state) { - is State.Pending -> this.state = State.Discarded - is State.Started -> { - if (!state.transaction.isFinished) { - state.transaction.finish(state.transaction.status ?: SpanStatus.OK) - } - onFinished(state.transaction) - } - State.Discarded -> Unit - } - } - - /** Lifecycle state for a [NavTransaction] handle. */ - private sealed interface State { - - /** - * Not started yet: the transaction will be started from [context] and [options], and what's set - * on the handle until then is recorded for replay. - */ - class Pending(val context: TransactionContext, val options: TransactionOptions) : State { - val data = linkedMapOf() - val contexts = linkedMapOf() - var app: App? = null - } - - /** The backing [transaction] has been started. A terminal [State]. */ - class Started(val transaction: ITransaction) : State - - /** The handle was finished before its backing transaction was started. A terminal [State]. */ - data object Discarded : State - } - - companion object { - /** A handle backed by the already-started [transaction]. */ - fun started(transaction: ITransaction): NavTransaction = - NavTransaction(transaction.name, State.Started(transaction)) - - /** A handle whose transaction is started later, via [start]. */ - fun deferred(context: TransactionContext, options: TransactionOptions): NavTransaction = - NavTransaction(context.name, State.Pending(context, options)) - } -} - -/** - * The subset of the [ITransaction] interface used to record nav data on a nav transaction. - * - * Protects against accidental misuse of what would otherwise be a partially-implemented - * `ITransaction` interface. New operations against the underlying `ITransaction` instance must be - * added here. Operations not directly delegable to `ITransaction` should ***not*** be added. - * - * Starting and finishing a nav transaction are lease-aware, so they go through [INavScope] instead. - */ -internal sealed interface INavTransaction { - - /** See [ITransaction.setData]. */ - fun setData(key: String, value: Any) - - /** See [ITransaction.setContext]. */ - fun setContext(key: String, value: Any) - - /** Sets the transaction's app context. See [ITransaction.getContexts]. */ - fun setApp(app: App) -} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionRequest.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionRequest.kt new file mode 100644 index 00000000000..343518b6542 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionRequest.kt @@ -0,0 +1,103 @@ +package io.sentry.compose.navigation3.lifecycle + +import io.sentry.IScope +import io.sentry.IScopes +import io.sentry.ITransaction +import io.sentry.SentryOptions +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.protocol.App +import io.sentry.protocol.Contexts +import io.sentry.protocol.TransactionNameSource + +/** The operation of nav transactions, and the type and category of nav breadcrumbs. */ +internal const val NAVIGATION_OP: String = "navigation" + +/** + * Everything needed to start a nav transaction for a destination. + * + * Transactions started from a request are named [name] with [TransactionNameSource.ROUTE] as their + * name source, and have the operation [NAVIGATION_OP]. + * + * A request may be reused to start a new transaction if the user returns to a nav root after its + * transaction finishes. Each [start] creates a new transaction for the same destination. + */ +internal class NavTransactionRequest( + val name: String, + /** Builds the options for each transaction started from this request. */ + private val newOptions: () -> TransactionOptions, + /** Data set on each transaction started from this request. See [ITransaction.setData]. */ + private val data: Map = emptyMap(), + /** Contexts set on each transaction started from this request. See [ITransaction.setContext]. */ + private val contexts: Map = emptyMap(), + /** + * The view names to set on the app context of each transaction started from this request, or + * `null` to leave the transaction's app context alone. The rest of the app context is copied from + * the scope when the transaction starts. + */ + private val viewNames: List? = null, +) { + + companion object { + + private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" + + /** Creates a request whose transaction options are read when each transaction starts. */ + fun forDestination( + name: String, + arguments: Map, + contexts: Map, + viewNames: List?, + options: SentryOptions, + ): NavTransactionRequest = + NavTransactionRequest( + name = name, + newOptions = { + TransactionOptions().also { + it.isWaitForChildren = true + it.idleTimeout = options.idleTimeout + val deadlineTimeoutMillis = options.deadlineTimeout + it.deadlineTimeout = if (deadlineTimeoutMillis <= 0) null else deadlineTimeoutMillis + it.isTrimEnd = true + it.origin = TRANSACTION_ORIGIN + } + }, + data = if (arguments.isNotEmpty()) mapOf("arguments" to arguments) else emptyMap(), + contexts = contexts, + viewNames = viewNames, + ) + } + + /** + * Starts a transaction via [scopes] with this request's data and contexts. Copies the app context + * from [publishingScope] when setting view names, without binding the transaction to that scope. + * + * Returns `null` if [scopes] declined to start one (i.e., returned a no-op transaction, e.g., + * because the SDK is disabled or ignores the transaction's origin). + */ + fun start(scopes: IScopes, publishingScope: IScope): ITransaction? { + val transaction = + scopes + .startTransaction( + TransactionContext(name, TransactionNameSource.ROUTE, NAVIGATION_OP), + newOptions(), + ) + .takeUnless { it.isNoOp } ?: return null + + data.forEach { (key, value) -> transaction.setData(key, value) } + contexts.forEach { (key, value) -> transaction.setContext(key, value) } + viewNames?.let { + transaction.contexts.setApp(publishingScope.appCopy().apply { this.viewNames = it }) + } + return transaction + } + + /** + * A copy of the receiver's app context, or a new one if it has none, so that setting view names + * on it leaves the scope's own app context alone. + * + * [App]'s copy constructor isn't public, so this copies all of [IScope.getContexts] and discards + * all of it but the `App`. + */ + private fun IScope.appCopy(): App = Contexts(contexts).app ?: App() +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md index d3e1a942bdf..c6aa677e7ce 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md @@ -53,7 +53,8 @@ The lease is acquired preemptively by an incoming nav root's `NavScopes` instanc A displaced nav root remains eligible for lease restoration until it stops (`ON_STOP`). It gets the lease back when it resumes (`ON_RESUME`), or sooner if the new nav root leaves or pauses while the displaced nav root is still visible. If restored, the displaced nav -root's nav data and unfinished transaction are reinstated. +root's nav data is reinstated, and so is its nav transaction if unfinished. Otherwise, the nav root gets a new nav transaction (see +[below](#returning-to-a-nav-root)). > † The exception is an incoming nav root that isn't resumed while the leaseholder is, e.g., a preloaded ViewPager2 Fragment page next to the current one. The user isn't interacting with such a @@ -138,15 +139,31 @@ Alternative at rollback point: B disposes before A stops Unbind txB ◄─────────────── dispose B ───┤ Navigation lease: A │ Republish A's staged data │ - Restore txA if unfinished and scope free │ + Restore txA if unfinished, else start a │ + new txA; bind it if the scope is free │ Finish txB if unfinished │ ``` -### Nav transactions follow navigation, not the lease +### Returning to a nav root -A nav root gets a nav transaction for a destination it navigates to, never merely for acquiring the lease. A destination reached without -the lease gets its transaction when the lease is next acquired, if it can start then; otherwise it gets none. On reacquiring the lease -without having navigated in the meantime, a nav root only gets its unfinished transaction back, if any. +A nav root is "returned to" when its candidate acquires the lease at any time other than on its first back stack update, e.g., when the +Activity covering it closes, or when the user swipes back to its ViewPager2 page. Spans produced on return (e.g., by work the nav root +resumes) need a nav transaction to attach to: -Hence the asymmetry for, e.g., ViewPager2 pages: the first time the user swipes to a preloaded page, it gets the transaction for the -destination it reached while offscreen, but swiping back to a page whose transaction has finished starts none. +- if the candidate's nav transaction is still active, that transaction is bound again, which + keeps the spans with the transaction that already holds the destination's earlier spans; and + +- if the candidate's nav transaction is finished (e.g., the transaction timed out while another + nav root held the lease, or it never started because another transaction held the scope's + transaction slot), a new nav transaction is started for the current destination. + +Either way, the transaction is bound only if the scope's transaction slot is free (see `StagedNavTransaction`). Rebinding and starting both +happen when the candidate publishes (`INavStatePublisher.publishTo`) its nav state, so the coordinator needn't tell a return from a first +acquisition: on its first back stack update, a candidate has no destination to start a transaction for yet. + +A nav root that's destroyed and recreated after losing the lease (e.g., on a configuration change) isn't returned to: it registers anew and +gets a transaction on its first back stack update. See [#6454](https://github.com/getsentry/sentry-java/issues/6454). + +Bringing the app back to the foreground isn't a return either: the leaseholder keeps the lease while the app is in the background (see +`stopCandidate`), so it starts no new nav transaction when it resumes. This matches the rest of the SDK, which starts no transaction when an +existing Activity comes back to the foreground. diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransaction.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransaction.kt index 2851a10481c..59916420c76 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransaction.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransaction.kt @@ -4,20 +4,29 @@ import io.sentry.IScope import io.sentry.IScopes import io.sentry.ITransaction import io.sentry.SentryLevel -import io.sentry.TransactionContext -import io.sentry.TransactionOptions +import io.sentry.SpanStatus /** * Holds a nav transaction staged for publication to a shared [IScope]. * - * A transaction requested while publishing isn't permitted is [staged][stage] rather than started, - * and starts when it's next [published][publishTo]. That publication is its only chance to start: - * if it can't start then, it's discarded. A started transaction that's [unpublished][unpublishFrom] - * is unbound but keeps running, so that it can be bound again if it's republished while unfinished. - * Once [finished][finish], a transaction is forgotten and never bound again. + * A [staged][stage] request starts a transaction when it's next [published][publishTo], which + * happens right away if the caller holds the nav lease. An [unpublished][unpublishFrom] transaction + * keeps running and can be rebound. [finish] forgets both the transaction and its request. * - * Starting or publishing a nav transaction never displaces an unfinished transaction bound to the - * scope by someone else (e.g., the host app). + * If a transaction ends on its own, cannot start, or is discarded, its request remains available to + * start a new transaction when the nav root returns. + * + * **Sampling** + * + * Sampled and unsampled transactions behave alike while running. Returning to a nav root reuses its + * unfinished transaction and sampling decision; starting a new transaction on return makes a new + * sampling decision. + * + * **Precedence** + * + * Navigation doesn't displace an unfinished transaction already bound to the shared `IScope`. This + * includes `ui.load`, `ui.action`, and host-app transactions. Changing their relative priority + * requires an SDK-wide policy. * * **Thread safety** * @@ -26,130 +35,112 @@ import io.sentry.TransactionOptions internal class StagedNavTransaction(private val scopes: IScopes) { /** - * The nav transaction most recently started or staged, unless it has been finished since then. + * The request most recently passed to [stage], whatever became of its transaction. Cleared when + * the nav transaction is [finished][finish] or [forgotten][forget], i.e., when the destination it + * describes is left. + * + * While there's a request but no [transaction], a transaction is started from the request when + * it's next [published][publishTo]: it's staged, or was discarded or blocked from starting. */ - private var navTransaction: NavTransaction? = null + private var request: NavTransactionRequest? = null + + /** The transaction most recently started for [request], unless it has been forgotten since. */ + private var transaction: ITransaction? = null /** - * Starts a nav transaction from [context] and [options] and binds it to [scope], unless another - * unfinished transaction is already bound to `scope`, in which case none is started, as it - * couldn't be bound. Returns `null` if no transaction was started (including when [scopes] - * returned a no-op transaction, e.g., because the SDK is disabled or ignores the transaction's - * origin), in which case the instance's current nav transaction, if any, is kept. + * Stages [request], so that the next [publishTo] starts a transaction for it. * - * **Warning**: [Finish][finish] the current nav transaction before calling this method, as this - * method replaces the current transaction without finishing it. + * Call [finish] first: staging replaces the current transaction without finishing it. */ - fun start( - scope: IScope, - context: TransactionContext, - options: TransactionOptions, - ): NavTransaction? { - if (!scope.isFreeFor(context.name, transaction = null)) { - return null - } - val transaction = - scopes.startTransaction(context, options).takeUnless { it.isNoOp } ?: return null - - // Record the transaction before binding it: binding notifies the host app's scope observers, - // which can throw, and an unrecorded transaction might never be finished. - val started = NavTransaction.started(transaction) - navTransaction = started - - bind(scope, transaction) - return started + fun stage(request: NavTransactionRequest) { + this.request = request + transaction = null } /** - * Stages a nav transaction that's started from [context] and [options] when it's - * [published][publishTo], and returns its handle, which records what's set on it in the meantime. + * Updates the saved request for a future start without changing a running transaction. * - * **Warning**: [Finish][finish] the current nav transaction before calling this method, as this - * method replaces the current transaction without finishing it. + * No-ops if no request has been [staged][stage]. */ - fun stage(context: TransactionContext, options: TransactionOptions): NavTransaction = - NavTransaction.deferred(context, options).also { navTransaction = it } + fun updateRequest(request: NavTransactionRequest) { + if (this.request == null) { + return + } + this.request = request + } /** - * Binds the nav transaction to [scope], starting it first if it's staged. A started transaction - * that has finished in the meantime is forgotten and unbound instead. - * - * If another unfinished transaction is bound to [scope], a started transaction stays unbound, so - * it can be bound the next time it's published. A staged transaction is discarded instead, as it - * is if [scopes] return a no-op transaction for it (see [start]). + * Publishes the staged transaction by binding it to [scope], or starts a transaction from the + * staged request if needed. An occupied scope slot leaves the transaction unbound and the request + * available for later publication. */ fun publishTo(scope: IScope) { - val navTransaction = navTransaction ?: return - val started = navTransaction.transaction - when { - started?.isFinished == true -> { - this.navTransaction = null - unbind(scope, started) - } - started != null -> if (scope.isFreeFor(navTransaction.name, started)) bind(scope, started) - else -> { - val startedNow = - if (scope.isFreeFor(navTransaction.name, transaction = null)) { - navTransaction.start(scopes) - } else { - null - } - if (startedNow != null) { - bind(scope, startedNow) - } else { - this.navTransaction = null - navTransaction.finish(onFinished = {}) - } + val request = request ?: return + val started = transaction + + if (started != null && !started.isFinished) { + if (scope.isFreeFor(request.name, started)) { + bind(scope, started) } + return } + + transaction = null + started?.let { unbind(scope, it) } + startAndBind(scope, request) } - /** - * Unbinds the nav transaction from [scope], if it's bound, without finishing it, so it can be - * [published][publishTo] again. - */ + /** Unbinds the transaction from [scope] without finishing it, allowing later publication. */ fun unpublishFrom(scope: IScope) { - navTransaction?.transaction?.let { unbind(scope, it) } + transaction?.let { unbind(scope, it) } } /** - * Finishes the nav transaction and unbinds it from [scope], if it's bound. A staged transaction - * is discarded instead. + * Finishes and unbinds the transaction from [scope], or discards it if staged. Uses its current + * status, or [OK][SpanStatus.OK] if unset. Forgets the request in either case. */ fun finish(scope: IScope) { - val navTransaction = navTransaction ?: return + request = null + val transaction = transaction ?: return // Forget the transaction before unbinding it: unbinding notifies the host app's scope // observers, which can throw, and a transaction left behind would be bound again later. - this.navTransaction = null - navTransaction.finish(onFinished = { unbind(scope, it) }) + this.transaction = null + if (!transaction.isFinished) { + transaction.finish(transaction.status ?: SpanStatus.OK) + } + unbind(scope, transaction) } - /** - * Forgets the nav transaction without finishing it, e.g., when there's no scope to unbind it - * from. If started, it will be left to time out. - */ + /** Forgets the transaction and request without finishing or unbinding a started transaction. */ fun forget() { - navTransaction = null + request = null + transaction = null } + /** Whether the slot is empty, holds a finished transaction, or already holds [transaction]. */ + private fun canHold(current: ITransaction?, transaction: ITransaction?): Boolean = + current == null || current.isFinished || current === transaction + /** - * Reports whether the receiver's transaction slot can hold [transaction], or a new transaction - * given `null` (see [canHold]). + * Starts and binds a transaction based on the provided [request] if [scope]'s slot is free. + * + * Returns `null` if the slot is occupied or [scopes] declines the start. */ - private fun IScope.isFreeFor(name: String, transaction: ITransaction?): Boolean { - if (canHold(current = this.transaction, transaction)) { - return true + private fun startAndBind(scope: IScope, request: NavTransactionRequest): ITransaction? { + if (!scope.isFreeFor(request.name, transaction = null)) { + return null } + val transaction = request.start(scopes, publishingScope = scope) ?: return null - scopes.options.logger.log( - SentryLevel.DEBUG, - "Nav3 transaction for route %s is blocked by another active transaction.", - name, - ) - return false + // Record the transaction before binding it: binding notifies the host app's scope observers, + // which can throw, and an unrecorded transaction might never be finished. + this.transaction = transaction + + bind(scope, transaction) + return transaction } - /** Binds [transaction] to [scope] if the scope's transaction slot can hold it (see [canHold]). */ + /** Binds [transaction] if [scope]'s slot is empty, finished, or already holds it. */ private fun bind(scope: IScope, transaction: ITransaction) { scope.withTransaction { current -> if (current !== transaction && canHold(current, transaction)) { @@ -158,13 +149,6 @@ internal class StagedNavTransaction(private val scopes: IScopes) { } } - /** - * Whether a transaction slot holding [current] can hold [transaction], or a new transaction given - * `null`: it's empty, holds a finished transaction, or already holds `transaction`. - */ - private fun canHold(current: ITransaction?, transaction: ITransaction?): Boolean = - current == null || current.isFinished || current === transaction - /** Unbinds [transaction] from [scope] if it's the bound transaction. */ private fun unbind(scope: IScope, transaction: ITransaction) { scope.withTransaction { current -> @@ -173,4 +157,20 @@ internal class StagedNavTransaction(private val scopes: IScopes) { } } } + + /** + * Checks whether this scope can hold [transaction] (or a new one if `null`), logging a blocker. + */ + private fun IScope.isFreeFor(name: String, transaction: ITransaction?): Boolean { + if (canHold(current = this.transaction, transaction)) { + return true + } + + scopes.options.logger.log( + SentryLevel.DEBUG, + "Nav3 transaction for route %s is blocked by another active transaction.", + name, + ) + return false + } } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index ef79931174f..d7d8895c608 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -63,6 +63,9 @@ class BackStackObserverTest { val breadcrumbHints = mutableListOf() val startedTransactions = mutableListOf() + /** The [NavScopes] passed to the observer created by [getSut]. */ + lateinit var navScopes: NavScopes + init { whenever(scopes.options).thenReturn(scope.options) whenever(scopes.getSpan()).thenAnswer { scope.span } @@ -95,9 +98,10 @@ class BackStackObserverTest { entryMapper: BackStackEntryMapper = defaultEntryMapper, ): BackStackObserver { scope.options.isEnableScreenTracking = config.enableScreenTracking + navScopes = leaseholderNavScopes() return BackStackObserver( - scopes = leaseholderNavScopes(), + scopes = navScopes, options = SentryNavOptions { enableNavigationBreadcrumbs = config.enableNavigationBreadcrumbs diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt index dea07db5481..6164f479bab 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt @@ -13,12 +13,14 @@ import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTracer +import io.sentry.SpanContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.compose.navigation3.lifecycle.INavStatePublisher import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator import io.sentry.compose.navigation3.lifecycle.NavRootInfo +import io.sentry.protocol.TransactionNameSource import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.doAnswer @@ -136,7 +138,7 @@ class CoordinatedBackStackObserverTest { val pageA = fixture.getSut(pageALifecycle) val pageB = fixture.getSut(pageBLifecycle) // E.g., the Activity's ui.load transaction, still running while the pager's pages load. - val hostTransaction = mock() + val hostTransaction = hostTransaction() fixture.scope.transaction = hostTransaction pageA.onBackStackChanged(listOf("PageA")) pageB.onBackStackChanged(listOf("PageB")) @@ -150,18 +152,19 @@ class CoordinatedBackStackObserverTest { } @Test - fun `a page swiped to while a host transaction is bound starts no nav transaction when swiped to again`() { + fun `pages whose nav transactions a host transaction blocked start them when swiped to after it finishes`() { val fixture = Fixture() val pageALifecycle = fixture.resumedLifecycle() val pageBLifecycle = fixture.resumedLifecycle().apply { currentState = Lifecycle.State.STARTED } val pageA = fixture.getSut(pageALifecycle) val pageB = fixture.getSut(pageBLifecycle) - val hostTransaction = mock() + val hostTransaction = hostTransaction() fixture.scope.transaction = hostTransaction pageA.onBackStackChanged(listOf("PageA")) pageB.onBackStackChanged(listOf("PageB")) pageBLifecycle.currentState = Lifecycle.State.RESUMED pageALifecycle.currentState = Lifecycle.State.STARTED + check(fixture.startedTransactions.isEmpty()) whenever(hostTransaction.isFinished).thenReturn(true) // Swipe back to page A, then to page B again. @@ -170,9 +173,12 @@ class CoordinatedBackStackObserverTest { pageBLifecycle.currentState = Lifecycle.State.RESUMED pageALifecycle.currentState = Lifecycle.State.STARTED - // As with page A, which navigated while the host transaction was bound, page B's destination - // gets no nav transaction: it couldn't start one when it first got the nav lease. - assertThat(fixture.startedTransactions).isEmpty() + // Neither page could start a nav transaction while the host transaction was bound, so each + // starts one when it's returned to. + assertThat(fixture.startedTransactions.map { it.name }) + .containsExactly("/PageA", "/PageB") + .inOrder() + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/PageB")) assertThat(fixture.scope.screen).isEqualTo("/PageB") } @@ -221,7 +227,7 @@ class CoordinatedBackStackObserverTest { } @Test - fun `swiping back to a page whose nav transaction finished starts no nav transaction`() { + fun `swiping back to a page whose nav transaction finished starts a new nav transaction for it`() { val fixture = Fixture() val pageALifecycle = fixture.resumedLifecycle() val pageBLifecycle = fixture.resumedLifecycle().apply { currentState = Lifecycle.State.STARTED } @@ -237,11 +243,118 @@ class CoordinatedBackStackObserverTest { pageALifecycle.currentState = Lifecycle.State.RESUMED pageBLifecycle.currentState = Lifecycle.State.STARTED + assertThat(fixture.startedTransactions.map { it.name }) + .containsExactly("/PageA", "/PageB", "/PageA") + .inOrder() + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.last()) + assertThat(fixture.scope.screen).isEqualTo("/PageA") + assertThat(fixture.breadcrumbs.map { it.data["to"] }) + .containsExactly("/PageA", "/PageB") + .inOrder() + } + + @Test + fun `the new nav transaction started when swiping back to a page reflects the page's nav data`() { + val fixture = Fixture() + val pageALifecycle = fixture.resumedLifecycle() + val pageBLifecycle = fixture.resumedLifecycle().apply { currentState = Lifecycle.State.STARTED } + val pageA = fixture.getSut(pageALifecycle, arguments = { mapOf("page" to it) }) + val pageB = fixture.getSut(pageBLifecycle) + pageA.onBackStackChanged(listOf("Home", "PageA")) + fixture.startedTransaction("/PageA").finish() + pageB.onBackStackChanged(listOf("PageB")) + pageBLifecycle.currentState = Lifecycle.State.RESUMED + pageALifecycle.currentState = Lifecycle.State.STARTED + + pageALifecycle.currentState = Lifecycle.State.RESUMED + pageBLifecycle.currentState = Lifecycle.State.STARTED + + val restarted = fixture.startedTransactions.last() + assertThat(restarted.name).isEqualTo("/PageA") + assertThat(restarted.transactionNameSource).isEqualTo(TransactionNameSource.ROUTE) + assertThat(restarted.operation).isEqualTo("navigation") + assertThat(restarted.spanContext.origin).isEqualTo("auto.navigation.nav3") + assertThat(restarted.getData("arguments")).isEqualTo(mapOf("page" to "PageA")) + assertThat(restarted.contexts.app?.viewNames).containsExactly("/PageA") + assertThat(restarted.contexts[NAVIGATION_CONTEXT_KEY]) + .isEqualTo( + mapOf( + "Back Stack" to + listOf( + mapOf("entry" to "/PageA", "entry_arguments" to mapOf("page" to "PageA")), + mapOf("entry" to "/Home", "entry_arguments" to mapOf("page" to "Home")), + ) + ) + ) + } + + @Test + fun `the new nav transaction started when swiping back to a page reflects its updated back stack`() { + val fixture = Fixture() + val pageALifecycle = fixture.resumedLifecycle() + val pageBLifecycle = fixture.resumedLifecycle().apply { currentState = Lifecycle.State.STARTED } + val pageA = fixture.getSut(pageALifecycle) + val pageB = fixture.getSut(pageBLifecycle) + val pageATop = "PageA" + pageA.onBackStackChanged(listOf("Home", pageATop)) + fixture.startedTransaction("/PageA").finish() + pageB.onBackStackChanged(listOf("PageB")) + pageBLifecycle.currentState = Lifecycle.State.RESUMED + pageALifecycle.currentState = Lifecycle.State.STARTED + // Changes the entries below the top while page A is offscreen. + pageA.onBackStackChanged(listOf("Settings", pageATop)) + + pageALifecycle.currentState = Lifecycle.State.RESUMED + pageBLifecycle.currentState = Lifecycle.State.STARTED + + val restarted = fixture.startedTransactions.last() + assertThat(restarted.name).isEqualTo("/PageA") + assertThat(restarted.contexts[NAVIGATION_CONTEXT_KEY]) + .isEqualTo( + mapOf("Back Stack" to listOf(mapOf("entry" to "/PageA"), mapOf("entry" to "/Settings"))) + ) + } + + @Test + fun `swiping back to a page whose back stack became empty starts no nav transaction`() { + val fixture = Fixture() + val pageALifecycle = fixture.resumedLifecycle() + val pageBLifecycle = fixture.resumedLifecycle().apply { currentState = Lifecycle.State.STARTED } + val pageA = fixture.getSut(pageALifecycle) + val pageB = fixture.getSut(pageBLifecycle) + pageA.onBackStackChanged(listOf("PageA")) + pageB.onBackStackChanged(listOf("PageB")) + pageBLifecycle.currentState = Lifecycle.State.RESUMED + pageALifecycle.currentState = Lifecycle.State.STARTED + pageA.onBackStackChanged(emptyList()) + + pageALifecycle.currentState = Lifecycle.State.RESUMED + pageBLifecycle.currentState = Lifecycle.State.STARTED + assertThat(fixture.startedTransactions.map { it.name }) .containsExactly("/PageA", "/PageB") .inOrder() assertThat(fixture.scope.transaction).isNull() - assertThat(fixture.scope.screen).isEqualTo("/PageA") + } + + @Test + fun `with nav transactions disabled, swiping back to a page starts no nav transaction`() { + val fixture = Fixture() + val options = SentryNavOptions { enableNavigationTransactions = false } + val pageALifecycle = fixture.resumedLifecycle() + val pageBLifecycle = fixture.resumedLifecycle().apply { currentState = Lifecycle.State.STARTED } + val pageA = fixture.getSut(pageALifecycle, options) + val pageB = fixture.getSut(pageBLifecycle, options) + pageA.onBackStackChanged(listOf("PageA")) + pageB.onBackStackChanged(listOf("PageB")) + pageBLifecycle.currentState = Lifecycle.State.RESUMED + pageALifecycle.currentState = Lifecycle.State.STARTED + + pageALifecycle.currentState = Lifecycle.State.RESUMED + pageBLifecycle.currentState = Lifecycle.State.STARTED + + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isNull() } @Test @@ -414,3 +527,7 @@ class CoordinatedBackStackObserverTest { override fun onUnregistered(scope: IScope?) = Unit } } + +/** A transaction the host app bound to the scope, as identified by its manual origin. */ +private fun hostTransaction(): ITransaction = + mock().also { whenever(it.spanContext).thenReturn(SpanContext("host")) } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectLifecycleIntegrationTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectLifecycleIntegrationTest.kt index d5467b10b9a..c30f296fd39 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectLifecycleIntegrationTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectLifecycleIntegrationTest.kt @@ -185,6 +185,36 @@ class SentryNavEffectLifecycleIntegrationTest { assertThat(fixture.scope.transaction).isSameInstanceAs(homeTransaction) } + @Test + fun `going back to an activity whose nav transaction finished while covered starts a new nav transaction for it`() { + val fixture = Fixture.create() + val home = Robolectric.buildActivity(HomeActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + home.pause() + val checkout = Robolectric.buildActivity(CheckoutActivity::class.java).setup() + fixture.idle() + home.stop() + fixture.idle() + homeTransaction.finish() + + checkout.pause() + home.restart() + home.resume() + fixture.idle() + checkout.stop() + checkout.destroy() + fixture.idle() + + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + assertThat(fixture.scope.transaction).isNotSameInstanceAs(homeTransaction) + assertThat(fixture.scope.transaction?.isFinished).isFalse() + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }) + .containsExactly("/Home", "/Checkout") + .inOrder() + } + // Activity recreation @Test fun `a recreated activity publishes its own nav data and transaction over the destroyed instance's`() { @@ -440,7 +470,7 @@ class SentryNavEffectLifecycleIntegrationTest { } @Test - fun `switching back to a view pager page whose nav transaction finished starts no nav transaction`() { + fun `switching back to a view pager page whose nav transaction finished starts a new nav transaction for the page`() { val fixture = Fixture.create() val activity = Robolectric.buildActivity(ViewPagerActivity::class.java).setup() fixture.idle() @@ -452,8 +482,52 @@ class SentryNavEffectLifecycleIntegrationTest { activity.get().viewPager.setCurrentItem(0, false) fixture.idle() - assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + assertThat(fixture.scope.transaction).isNotSameInstanceAs(homeTransaction) + assertThat(fixture.scope.transaction?.isFinished).isFalse() assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }) + .containsExactly("/Home", "/Checkout") + .inOrder() + } + + /** + * A transaction that isn't sampled is still started and bound, so a return while it's running + * binds it again rather than sampling a new one, just as for a sampled transaction. + */ + @Test + fun `switching back to a view pager page restores its unfinished unsampled nav transaction`() { + val fixture = Fixture.create(tracesSampleRate = 0.0) + val activity = Robolectric.buildActivity(ViewPagerActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + check(homeTransaction.isSampled == false) + activity.get().viewPager.setCurrentItem(1, false) + fixture.idle() + + activity.get().viewPager.setCurrentItem(0, false) + fixture.idle() + + assertThat(fixture.scope.transaction).isSameInstanceAs(homeTransaction) + } + + @Test + fun `switching back to a view pager page whose unsampled nav transaction finished samples a new one`() { + val fixture = Fixture.create(tracesSampleRate = 0.0) + val activity = Robolectric.buildActivity(ViewPagerActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + activity.get().viewPager.setCurrentItem(1, false) + fixture.idle() + homeTransaction.finish() + + activity.get().viewPager.setCurrentItem(0, false) + fixture.idle() + + val restarted = requireNotNull(fixture.scope.transaction) + assertThat(restarted).isNotSameInstanceAs(homeTransaction) + assertThat(restarted.name).isEqualTo("/Home") + assertThat(restarted.isSampled).isFalse() } // Dialogs @@ -539,6 +613,30 @@ class SentryNavEffectLifecycleIntegrationTest { assertThat(homeTransaction.isFinished).isFalse() } + @Test + fun `dismissing a dialog fragment starts a new nav transaction for the still-resumed activity if its own finished`() { + val fixture = Fixture.create() + val home = Robolectric.buildActivity(HomeActivity::class.java).setup() + fixture.idle() + val homeTransaction = requireNotNull(fixture.scope.transaction) + val dialog = PickerDialogFragment() + dialog.show(home.get().supportFragmentManager, "picker") + fixture.idle() + val pickerTransaction = requireNotNull(fixture.scope.transaction) + homeTransaction.finish() + + dialog.dismiss() + fixture.idle() + + assertThat(pickerTransaction.isFinished).isTrue() + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + assertThat(fixture.scope.transaction).isNotSameInstanceAs(homeTransaction) + assertThat(fixture.scope.transaction?.isFinished).isFalse() + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }) + .containsExactly("/Home", "/Picker") + .inOrder() + } + // Screens without an effect @Test @@ -758,13 +856,13 @@ class SentryNavEffectLifecycleIntegrationTest { } } - private class Fixture private constructor() { + private class Fixture private constructor(tracesSampleRate: Double) { val options = SentryOptions().apply { dsn = "http://key@localhost/1" defaultScopeType = ScopeType.CURRENT - setTracesSampleRate(1.0) + setTracesSampleRate(tracesSampleRate) isEnableScreenTracking = true idleTimeout = null deadlineTimeout = 0 @@ -820,7 +918,7 @@ class SentryNavEffectLifecycleIntegrationTest { fun require(): Fixture = checkNotNull(current) { "Fixture.create() was not called" } - fun create(): Fixture { + fun create(tracesSampleRate: Double = 1.0): Fixture { val application: Application = ApplicationProvider.getApplicationContext() val packageManager = shadowOf(application.packageManager) listOf( @@ -835,7 +933,7 @@ class SentryNavEffectLifecycleIntegrationTest { ComponentName(application.packageName, activityClass.name) ) } - return Fixture().also { current = it } + return Fixture(tracesSampleRate).also { current = it } } } } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt index ad2c7b9ae40..5f8b0ccd003 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt @@ -13,7 +13,6 @@ import io.sentry.SentryOptions import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions -import io.sentry.protocol.App import org.junit.Assert.assertThrows import org.junit.Test import org.mockito.kotlin.any @@ -76,29 +75,6 @@ class NavScopeTest { assertThat(fixture.scope.screen).isEqualTo("Profile") } - @Test - fun `getApp() - returns a copy of the scope's app context`() { - val fixture = Fixture.create() - val navScopes = fixture.navScopes - val hostApp = App().apply { appName = "Host" } - fixture.scope.contexts.setApp(hostApp) - - val app = navScopes.withNavScope { it.app } - - assertThat(app?.appName).isEqualTo("Host") - assertThat(app).isNotSameInstanceAs(hostApp) - } - - @Test - fun `getApp() - returns null when the scope has no app context`() { - val fixture = Fixture.create() - val navScopes = fixture.navScopes - - val app = navScopes.withNavScope { it.app } - - assertThat(app).isNull() - } - @Test fun `setNavigationContext() - publishes the navigation context with the nav lease`() { val fixture = Fixture.create(ownsLease = true) @@ -163,9 +139,8 @@ class NavScopeTest { val fixture = Fixture.create(ownsLease = true) val navScopes = fixture.navScopes - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } + navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(navTransaction).isNotNull() assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) } @@ -176,9 +151,8 @@ class NavScopeTest { val hostTransaction = mock() fixture.scope.transaction = hostTransaction - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } + navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(navTransaction).isNull() assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) } @@ -190,9 +164,8 @@ class NavScopeTest { fixture.scope.transaction = mock().also { whenever(it.isFinished).thenReturn(true) } - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } + navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(navTransaction).isNotNull() assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) } @@ -203,9 +176,8 @@ class NavScopeTest { val hostTransaction = mock() fixture.scope.transaction = hostTransaction - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } + navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(navTransaction).isNotNull() assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) } @@ -229,9 +201,8 @@ class NavScopeTest { val navScopes = fixture.navScopes val otherTransaction = fixture.startAnotherOwnersNavTransaction() - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } + navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(navTransaction).isNotNull() assertThat(fixture.startedTransactions).containsExactly(otherTransaction) assertThat(fixture.scope.transaction).isSameInstanceAs(otherTransaction) } @@ -243,9 +214,8 @@ class NavScopeTest { val finished = mock().also { whenever(it.isFinished).thenReturn(true) } fixture.scope.transaction = finished - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } + navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(navTransaction).isNotNull() assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isSameInstanceAs(finished) } @@ -257,9 +227,8 @@ class NavScopeTest { navScopes.withNavScope { it.startNavTransaction("/Profile") } val previous = fixture.startedTransactions.single() - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Settings") } + navScopes.withNavScope { it.startNavTransaction("/Settings") } - assertThat(navTransaction).isNull() verify(previous, never()).finish(any()) assertThat(fixture.startedTransactions).containsExactly(previous) assertThat(fixture.scope.transaction).isSameInstanceAs(previous) @@ -272,9 +241,8 @@ class NavScopeTest { navScopes.withNavScope { it.startNavTransaction("/Profile") } navScopes.withNavScope { it.finishNavTransaction() } - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Settings") } + navScopes.withNavScope { it.startNavTransaction("/Settings") } - assertThat(navTransaction).isNotNull() assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.last()) } @@ -282,13 +250,11 @@ class NavScopeTest { fun `startNavTransaction() - replaces the previous deferred transaction without the nav lease`() { val fixture = Fixture.create() val navScopes = fixture.navScopes - val previous = navScopes.withNavScope { it.startNavTransaction("/Profile") } as NavTransaction + navScopes.withNavScope { it.startNavTransaction("/Profile") } - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Settings") } + navScopes.withNavScope { it.startNavTransaction("/Settings") } fixture.acquireLease() - assertThat(navTransaction).isNotNull() - assertThat(previous.transaction).isNull() assertThat(fixture.startedTransactions.map { it.name }).containsExactly("/Settings") assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) } @@ -303,9 +269,8 @@ class NavScopeTest { fixture.scope.transaction = hostTransaction navScopes.withNavScope { it.finishNavTransaction() } - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Settings") } + navScopes.withNavScope { it.startNavTransaction("/Settings") } - assertThat(navTransaction).isNull() verify(previous).finish(SpanStatus.OK) assertThat(fixture.startedTransactions).containsExactly(previous) assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) @@ -315,15 +280,13 @@ class NavScopeTest { fun `startNavTransaction() - replaces the previous deferred transaction while a host transaction is bound`() { val fixture = Fixture.create() val navScopes = fixture.navScopes - val previous = navScopes.withNavScope { it.startNavTransaction("/Profile") } as NavTransaction + navScopes.withNavScope { it.startNavTransaction("/Profile") } fixture.scope.transaction = mock() - val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Settings") } + navScopes.withNavScope { it.startNavTransaction("/Settings") } fixture.scope.clearTransaction() fixture.acquireLease() - assertThat(navTransaction).isNotNull() - assertThat(previous.transaction).isNull() assertThat(fixture.startedTransactions.map { it.name }).containsExactly("/Settings") assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) } @@ -414,8 +377,9 @@ class NavScopeTest { return results.single() } - private fun INavScope.startNavTransaction(name: String): INavTransaction? = - startNavTransaction(TransactionContext(name, "navigation"), TransactionOptions()) + private fun INavScope.startNavTransaction(name: String) { + startNavTransaction(navTransactionRequest(name)) + } /** Thrown by a scope observer, to stand in for one the host app registered. */ private class ThrowingScopeObserverException : RuntimeException() @@ -470,7 +434,7 @@ class NavScopeTest { val other = resumedNavScopes() other.onBackStackUpdate() other.configureScope { - it.startNavTransaction(TransactionContext("/Other", "navigation"), TransactionOptions()) + it.startNavTransaction(navTransactionRequest("/Other")) } return startedTransactions.single { it.name == "/Other" } } @@ -492,3 +456,9 @@ class NavScopeTest { } } } + +private fun navTransactionRequest(name: String): NavTransactionRequest = + NavTransactionRequest( + name = name, + newOptions = ::TransactionOptions, + ) diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesIntegrationTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesIntegrationTest.kt index fe20e834ae8..fb9782877e2 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesIntegrationTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesIntegrationTest.kt @@ -10,7 +10,6 @@ import io.sentry.Scope import io.sentry.ScopeType import io.sentry.Scopes import io.sentry.SentryOptions -import io.sentry.TransactionContext import io.sentry.TransactionOptions import org.junit.Test import org.mockito.kotlin.mock @@ -37,7 +36,12 @@ class NavScopesIntegrationTest { 1.0 } incoming.configureScope { - it.startNavTransaction(TransactionContext("/Checkout", "navigation"), TransactionOptions()) + it.startNavTransaction( + NavTransactionRequest( + name = "/Checkout", + newOptions = ::TransactionOptions, + ) + ) } assertThat(contextsAtSampling).isEmpty() assertThat(fixture.scope.propagationContext).isSameInstanceAs(contextBeforeResume) diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt index a50ec5445c0..84e1d1822c9 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt @@ -15,13 +15,16 @@ import io.sentry.ScopeCallback import io.sentry.ScopeObserverAdapter import io.sentry.SentryLevel import io.sentry.SentryOptions +import io.sentry.SpanContext import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions +import io.sentry.protocol.App import io.sentry.protocol.Contexts import org.junit.Assert.assertThrows import org.junit.Test import org.mockito.kotlin.any +import org.mockito.kotlin.atLeastOnce import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.mock @@ -229,23 +232,22 @@ class NavScopesTest { navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() - val transaction = navScopes.startNavTransaction("/Home") + navScopes.startNavTransaction("/Home") val started = fixture.startedTransaction("/Home") - assertThat(transaction).isNotNull() assertThat(fixture.scope.transaction).isSameInstanceAs(started) } @Test - fun `startNavTransaction() - returns null when the delegate declines with the nav lease`() { + fun `startNavTransaction() - binds no transaction when the delegate declines with the nav lease`() { val fixture = Fixture.create(declinesTransactions = true) val navScopes = fixture.navScopes() navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() - val transaction = navScopes.startNavTransaction("/Home") + navScopes.startNavTransaction("/Home") - assertThat(transaction).isNull() + assertThat(fixture.scope.transaction).isNull() } @Test @@ -254,7 +256,7 @@ class NavScopesTest { val navScopes = fixture.navScopes() navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() - fixture.scope.transaction = mock() + fixture.scope.transaction = hostTransaction() navScopes.startNavTransaction("/Home") @@ -266,10 +268,9 @@ class NavScopesTest { val fixture = Fixture.create() val covered = fixture.coveredNavScopes() - val transaction = covered.startNavTransaction("/Result") + covered.startNavTransaction("/Result") assertThat(fixture.hasLease(covered)).isFalse() - assertThat(transaction).isNotNull() assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isNull() } @@ -337,7 +338,7 @@ class NavScopesTest { val navScopes = fixture.navScopes() navScopes.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) navScopes.applyFirstBackStackUpdate() - val hostTransaction = mock() + val hostTransaction = hostTransaction() fixture.scope.transaction = hostTransaction navScopes.finishNavTransaction() @@ -355,9 +356,8 @@ class NavScopesTest { navScopes.startNavTransaction("/Home") navScopes.finishNavTransaction() - val transaction = navScopes.startNavTransaction("/Profile") + navScopes.startNavTransaction("/Profile") - assertThat(transaction).isNotNull() assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Profile")) } @@ -402,7 +402,7 @@ class NavScopesTest { } @Test - fun `a finished nav transaction is discarded when the nav lease is regained`() { + fun `a finished nav transaction is not bound again when the nav lease is regained`() { val fixture = Fixture.create() val covered = fixture.coveredNavScopes(startedRoute = "/Home") val started = fixture.startedTransaction("/Home") @@ -411,7 +411,7 @@ class NavScopesTest { fixture.incoming.dispose() assertThat(fixture.hasLease(covered)).isTrue() - assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.transaction).isNotSameInstanceAs(started) } @Test @@ -424,6 +424,103 @@ class NavScopesTest { fixture.incoming.dispose() assertThat(fixture.hasLease(covered)).isTrue() + assertThat(fixture.startedTransactions).hasSize(1) + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `regaining the nav lease after the nav transaction finished starts a new one for its request`() { + val fixture = Fixture.create() + fixture.coveredNavScopes(startedRoute = "/Home") + val finished = fixture.startedTransaction("/Home") + whenever(finished.isFinished).thenReturn(true) + + fixture.incoming.dispose() + + val restarted = fixture.startedTransactions.last() + assertThat(fixture.startedTransactions).hasSize(2) + assertThat(restarted).isNotSameInstanceAs(finished) + assertThat(restarted.name).isEqualTo("/Home") + assertThat(fixture.scope.transaction).isSameInstanceAs(restarted) + } + + @Test + fun `regaining the nav lease after another transaction blocked the nav transaction starts one for its request`() { + val fixture = Fixture.create() + fixture.scope.transaction = hostTransaction() + fixture.coveredNavScopes(startedRoute = "/Home") + check(fixture.startedTransactions.isEmpty()) + fixture.scope.clearTransaction() + + fixture.incoming.dispose() + + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Home")) + } + + @Test + fun `regaining the nav lease again after a host transaction blocked the deferred transaction starts one`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes() + covered.startNavTransaction("/Result") + val hostTransaction = hostTransaction() + fixture.scope.transaction = hostTransaction + fixture.incoming.dispose() + check(fixture.startedTransactions.isEmpty()) + fixture.scope.clearTransaction() + val nextIncoming = fixture.navScopes() + nextIncoming.attach(NavRootInfo(fixture.lifecycle(Lifecycle.State.RESUMED), activity = null)) + nextIncoming.applyFirstBackStackUpdate() + + nextIncoming.dispose() + + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Result")) + } + + @Test + fun `regaining the nav lease with an unfinished nav transaction starts no new one`() { + val fixture = Fixture.create() + fixture.coveredNavScopes(startedRoute = "/Home") + val started = fixture.startedTransaction("/Home") + + fixture.incoming.dispose() + + assertThat(fixture.startedTransactions).containsExactly(started) + } + + @Test + fun `regaining the nav lease with a deferred transaction starts only the deferred one`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes() + covered.startNavTransaction("/Result") + + fixture.incoming.dispose() + + assertThat(fixture.startedTransactions.map { it.name }).containsExactly("/Result") + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Result")) + } + + @Test + fun `regaining the nav lease with a deferred transaction blocked by a host transaction starts none`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes() + covered.startNavTransaction("/Result") + val hostTransaction = hostTransaction() + fixture.scope.transaction = hostTransaction + + fixture.incoming.dispose() + + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + } + + @Test + fun `regaining the nav lease without ever starting a nav transaction starts none`() { + val fixture = Fixture.create() + fixture.coveredNavScopes() + + fixture.incoming.dispose() + + assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isNull() } @@ -439,17 +536,33 @@ class NavScopesTest { } @Test - fun `a deferred transaction receives the data recorded on its handle once it starts`() { + fun `a deferred transaction is started from its request when the nav lease is acquired`() { val fixture = Fixture.create() val covered = fixture.coveredNavScopes() - val transaction = covered.startNavTransaction("/Result") - transaction?.setData("arguments", "id=1") + covered.startNavTransaction( + navTransactionRequest("/Result", data = mapOf("arguments" to "id=1")) + ) fixture.incoming.dispose() verify(fixture.startedTransaction("/Result")).setData("arguments", "id=1") } + @Test + fun `a deferred transaction copies the scope's app context as it is when the nav lease is acquired`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes() + fixture.scope.contexts.setApp(App().apply { appVersion = "1.0.0" }) + covered.startNavTransaction(navTransactionRequest("/Result", viewNames = listOf("/Result"))) + fixture.scope.contexts.setApp(App().apply { appVersion = "2.0.0" }) + + fixture.incoming.dispose() + + val app = fixture.startedTransaction("/Result").contexts.app + assertThat(app?.appVersion).isEqualTo("2.0.0") + assertThat(app?.viewNames).containsExactly("/Result") + } + @Test fun `a preloaded owner's deferred transaction starts when the owner resumes`() { val fixture = Fixture.create() @@ -520,7 +633,7 @@ class NavScopesTest { val fixture = Fixture.create() val covered = fixture.coveredNavScopes() covered.startNavTransaction("/Result") - val hostTransaction = mock() + val hostTransaction = hostTransaction() fixture.scope.transaction = hostTransaction fixture.incoming.dispose() @@ -534,11 +647,12 @@ class NavScopesTest { val fixture = Fixture.create() val covered = fixture.coveredNavScopes() covered.startNavTransaction("/Result") - fixture.scope.transaction = mock() + fixture.scope.transaction = hostTransaction() fixture.incoming.dispose() - verify(fixture.logger).log(eq(SentryLevel.DEBUG), any(), eq("/Result")) + // Logged again when the restart for the same request is blocked, too. + verify(fixture.logger, atLeastOnce()).log(eq(SentryLevel.DEBUG), any(), eq("/Result")) } @Test @@ -652,12 +766,13 @@ class NavScopesTest { } /** Starts a nav transaction for [name] through the [INavScope] handed out by [NavScopes]. */ - private fun NavScopes.startNavTransaction(name: String): INavTransaction? { - var transaction: INavTransaction? = null - configureScope { - transaction = it.startNavTransaction(TransactionContext(name, "op"), TransactionOptions()) - } - return transaction + private fun NavScopes.startNavTransaction(name: String) { + startNavTransaction(navTransactionRequest(name)) + } + + /** Starts a nav transaction for [request] through the [INavScope] handed out by [NavScopes]. */ + private fun NavScopes.startNavTransaction(request: NavTransactionRequest) { + configureScope { it.startNavTransaction(request) } } /** Finishes the nav transaction through the [INavScope] handed out by [NavScopes]. */ @@ -707,8 +822,12 @@ class NavScopesTest { NoOpTransaction.getInstance() } else { val name = (it.arguments[0] as TransactionContext).name + val contexts = Contexts() mock() - .also { transaction -> whenever(transaction.name).thenReturn(name) } + .also { transaction -> + whenever(transaction.name).thenReturn(name) + whenever(transaction.contexts).thenReturn(contexts) + } .also(startedTransactions::add) } } @@ -746,9 +865,7 @@ class NavScopesTest { covered.attach(NavRootInfo(lifecycle(Lifecycle.State.RESUMED), activity = null)) covered.onBackStackUpdate() if (startedRoute != null) { - covered.configureScope { - it.startNavTransaction(TransactionContext(startedRoute, "op"), TransactionOptions()) - } + covered.configureScope { it.startNavTransaction(navTransactionRequest(startedRoute)) } } incoming = navScopes() incoming.attach(NavRootInfo(lifecycle(Lifecycle.State.RESUMED), activity = null)) @@ -778,3 +895,19 @@ class NavScopesTest { } } } + +private fun navTransactionRequest( + name: String, + data: Map = emptyMap(), + viewNames: List? = null, +): NavTransactionRequest = + NavTransactionRequest( + name = name, + newOptions = ::TransactionOptions, + data = data, + viewNames = viewNames, + ) + +/** A transaction the host app bound to the scope, as identified by its manual origin. */ +private fun hostTransaction(): ITransaction = + mock().also { whenever(it.spanContext).thenReturn(SpanContext("host")) } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionRequestTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionRequestTest.kt new file mode 100644 index 00000000000..f7155b67878 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionRequestTest.kt @@ -0,0 +1,244 @@ +package io.sentry.compose.navigation3.lifecycle + +import com.google.common.truth.Truth.assertThat +import io.sentry.IScopes +import io.sentry.ITransaction +import io.sentry.NoOpTransaction +import io.sentry.Scope +import io.sentry.SentryOptions +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.protocol.App +import io.sentry.protocol.Contexts +import io.sentry.protocol.TransactionNameSource +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +class NavTransactionRequestTest { + + private val navigationContext = mapOf("backstack" to listOf(mapOf("route" to "/Checkout"))) + + private val scope = Scope(SentryOptions()) + + @Test + fun `forDestination() - reads transaction options when each transaction starts`() { + val fixture = Fixture.create() + val options = SentryOptions().apply { idleTimeout = 100L } + val request = + NavTransactionRequest.forDestination( + name = "/Checkout", + arguments = mapOf("id" to "1"), + contexts = emptyMap(), + viewNames = null, + options = options, + ) + + options.idleTimeout = 200L + request.start(fixture.scopes, scope) + options.idleTimeout = 300L + request.start(fixture.scopes, scope) + + assertThat(fixture.startedOptions.map { it.idleTimeout }).containsExactly(200L, 300L).inOrder() + assertThat(fixture.startedOptions.map { it.origin }) + .containsExactly("auto.navigation.nav3", "auto.navigation.nav3") + verify(fixture.startedTransactions.first()).setData("arguments", mapOf("id" to "1")) + } + + @Test + fun `start() - starts a navigation transaction named by route with the request's name`() { + val fixture = Fixture.create() + val request = fixture.request() + + val transaction = request.start(fixture.scopes, scope) + + val context = fixture.startedContexts.single() + assertThat(transaction).isSameInstanceAs(fixture.startedTransactions.single()) + assertThat(context.name).isEqualTo("/Checkout") + assertThat(context.transactionNameSource).isEqualTo(TransactionNameSource.ROUTE) + assertThat(context.operation).isEqualTo("navigation") + } + + @Test + fun `start() - starts each transaction with a new transaction context`() { + val fixture = Fixture.create() + val request = fixture.request() + + request.start(fixture.scopes, scope) + request.start(fixture.scopes, scope) + + val (first, second) = fixture.startedContexts + assertThat(second).isNotSameInstanceAs(first) + assertThat(second.spanId).isNotEqualTo(first.spanId) + assertThat(second.traceId).isNotEqualTo(first.traceId) + } + + @Test + fun `start() - starts each transaction with new options from the options factory`() { + val fixture = Fixture.create() + val createdOptions = mutableListOf() + val request = fixture.request(newOptions = { TransactionOptions().also(createdOptions::add) }) + + request.start(fixture.scopes, scope) + request.start(fixture.scopes, scope) + + assertThat(createdOptions).hasSize(2) + assertThat(fixture.startedOptions).containsExactlyElementsIn(createdOptions).inOrder() + assertThat(createdOptions[1]).isNotSameInstanceAs(createdOptions[0]) + } + + @Test + fun `start() - sets the request's data on the transaction`() { + val fixture = Fixture.create() + val request = fixture.request(data = mapOf("arguments" to mapOf("id" to "1"))) + + val transaction = request.start(fixture.scopes, scope)!! + + verify(transaction).setData("arguments", mapOf("id" to "1")) + } + + @Test + fun `start() - sets the request's contexts on the transaction`() { + val fixture = Fixture.create() + val request = fixture.request(contexts = mapOf(NAVIGATION_CONTEXT_KEY to navigationContext)) + + val transaction = request.start(fixture.scopes, scope)!! + + verify(transaction).setContext(NAVIGATION_CONTEXT_KEY, navigationContext) + } + + @Test + fun `start() - sets the view names on a copy of the scope's app context`() { + val fixture = Fixture.create() + val scopeApp = + App().apply { + appName = "Sample" + appVersion = "1.2.3" + viewNames = listOf("/Previous") + } + scope.contexts.setApp(scopeApp) + val request = fixture.request(viewNames = listOf("/Checkout")) + + val transaction = request.start(fixture.scopes, scope)!! + + val transactionApp = transaction.contexts.app + assertThat(transactionApp).isNotSameInstanceAs(scopeApp) + assertThat(transactionApp?.viewNames).containsExactly("/Checkout") + assertThat(transactionApp?.appName).isEqualTo("Sample") + assertThat(transactionApp?.appVersion).isEqualTo("1.2.3") + assertThat(scope.contexts.app?.viewNames).containsExactly("/Previous") + } + + @Test + fun `start() - copies the scope's app context anew for each transaction`() { + val fixture = Fixture.create() + val request = fixture.request(viewNames = listOf("/Checkout")) + scope.contexts.setApp(App().apply { appVersion = "1.0.0" }) + request.start(fixture.scopes, scope) + scope.contexts.setApp(App().apply { appVersion = "2.0.0" }) + + val transaction = request.start(fixture.scopes, scope)!! + + assertThat(transaction.contexts.app?.appVersion).isEqualTo("2.0.0") + } + + @Test + fun `start() - sets the view names on a new app context when the scope has none`() { + val fixture = Fixture.create() + val request = fixture.request(viewNames = listOf("/Checkout")) + + val transaction = request.start(fixture.scopes, scope)!! + + assertThat(transaction.contexts.app?.viewNames).containsExactly("/Checkout") + assertThat(scope.contexts.app).isNull() + } + + @Test + fun `start() - leaves the transaction's app context alone without view names`() { + val fixture = Fixture.create() + scope.contexts.setApp(App().apply { appName = "Sample" }) + val request = fixture.request(viewNames = null) + + val transaction = request.start(fixture.scopes, scope)!! + + assertThat(transaction.contexts.app).isNull() + } + + @Test + fun `start() - returns null when the scopes start a no-op transaction`() { + val fixture = Fixture.create(declinesTransactions = true) + val request = + fixture.request( + data = mapOf("arguments" to "id=1"), + contexts = mapOf(NAVIGATION_CONTEXT_KEY to navigationContext), + viewNames = listOf("/Checkout"), + ) + + val transaction = request.start(fixture.scopes, scope) + + assertThat(transaction).isNull() + } + + @Test + fun `start() - sets nothing on a no-op transaction`() { + val fixture = Fixture.create() + val noOpTransaction = + mock().also { + whenever(it.isNoOp).thenReturn(true) + whenever(fixture.scopes.startTransaction(any(), any())).thenReturn(it) + } + val request = fixture.request(data = mapOf("arguments" to "id=1")) + + request.start(fixture.scopes, scope) + + verify(noOpTransaction, never()).setData(any(), any()) + } + + private class Fixture private constructor() { + /** Whether [scopes] declines to start transactions, returning a no-op transaction instead. */ + var declinesTransactions = false + + val startedContexts = mutableListOf() + val startedOptions = mutableListOf() + val startedTransactions = mutableListOf() + + val scopes = + mock().also { scopes -> + whenever(scopes.startTransaction(any(), any())) + .thenAnswer { + if (declinesTransactions) { + NoOpTransaction.getInstance() + } else { + startedContexts += it.arguments[0] as TransactionContext + startedOptions += it.arguments[1] as TransactionOptions + val contexts = Contexts() + mock() + .also { transaction -> whenever(transaction.contexts).thenReturn(contexts) } + .also(startedTransactions::add) + } + } + } + + fun request( + newOptions: () -> TransactionOptions = ::TransactionOptions, + data: Map = emptyMap(), + contexts: Map = emptyMap(), + viewNames: List? = null, + ): NavTransactionRequest = + NavTransactionRequest( + name = "/Checkout", + newOptions = newOptions, + data = data, + contexts = contexts, + viewNames = viewNames, + ) + + companion object { + fun create(declinesTransactions: Boolean = false): Fixture = + Fixture().also { it.declinesTransactions = declinesTransactions } + } + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionTest.kt deleted file mode 100644 index 1eca057f395..00000000000 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionTest.kt +++ /dev/null @@ -1,348 +0,0 @@ -package io.sentry.compose.navigation3.lifecycle - -import com.google.common.truth.Truth.assertThat -import io.sentry.IScopes -import io.sentry.ITransaction -import io.sentry.NoOpTransaction -import io.sentry.SpanStatus -import io.sentry.TransactionContext -import io.sentry.TransactionOptions -import io.sentry.protocol.App -import io.sentry.protocol.Contexts -import org.junit.Test -import org.mockito.kotlin.any -import org.mockito.kotlin.inOrder -import org.mockito.kotlin.mock -import org.mockito.kotlin.never -import org.mockito.kotlin.verify -import org.mockito.kotlin.verifyNoInteractions -import org.mockito.kotlin.whenever - -class NavTransactionTest { - - private val transactionContext = TransactionContext("/Checkout", "navigation") - private val transactionOptions = TransactionOptions() - - private val app = App().apply { viewNames = listOf("/Checkout") } - private val navigationContext = mapOf("Back Stack" to listOf(mapOf("entry" to "/Checkout"))) - - @Test - fun `started() - is named after the transaction`() { - val fixture = Fixture.create() - - val handle = NavTransaction.started(fixture.transaction) - - assertThat(handle.name).isEqualTo("/Checkout") - } - - @Test - fun `deferred() - is named after the transaction context`() { - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - assertThat(handle.name).isEqualTo("/Checkout") - } - - @Test - fun `transaction - is null for a deferred handle`() { - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - assertThat(handle.transaction).isNull() - } - - @Test - fun `transaction - is the backing transaction for a started handle`() { - val fixture = Fixture.create() - - val handle = NavTransaction.started(fixture.transaction) - - assertThat(handle.transaction).isSameInstanceAs(fixture.transaction) - } - - @Test - fun `transaction - is the started transaction once a deferred handle has started`() { - val fixture = Fixture.create() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - handle.start(fixture.scopes) - - assertThat(handle.transaction).isSameInstanceAs(fixture.transaction) - } - - @Test - fun `transaction - stays null when the scopes decline to start a deferred handle's transaction`() { - val fixture = Fixture.create(declinesTransactions = true) - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - handle.start(fixture.scopes) - - assertThat(handle.transaction).isNull() - } - - @Test - fun `transaction - is null once a deferred handle is finished before it starts`() { - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - handle.finish(onFinished = {}) - - assertThat(handle.transaction).isNull() - } - - @Test - fun `a started handle sets data, contexts, and the app context on the transaction`() { - val fixture = Fixture.create() - val handle = NavTransaction.started(fixture.transaction) - - handle.setData("arguments", "id=1") - handle.setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - handle.setApp(app) - - verify(fixture.transaction).setData("arguments", "id=1") - verify(fixture.transaction).setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - assertThat(fixture.transaction.contexts.app).isSameInstanceAs(app) - } - - @Test - fun `a deferred handle sets data, contexts, and the app context on the transaction once it has started`() { - val fixture = Fixture.create() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - handle.start(fixture.scopes) - - handle.setData("arguments", "id=1") - handle.setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - handle.setApp(app) - - verify(fixture.transaction).setData("arguments", "id=1") - verify(fixture.transaction).setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - assertThat(fixture.transaction.contexts.app).isSameInstanceAs(app) - } - - @Test - fun `start() - starts and returns the transaction with the deferred context and options`() { - val fixture = Fixture.create() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - val started = handle.start(fixture.scopes) - - verify(fixture.scopes).startTransaction(transactionContext, transactionOptions) - assertThat(started).isSameInstanceAs(fixture.transaction) - } - - @Test - fun `start() - replays data, contexts, and the app context set before the start`() { - val fixture = Fixture.create() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - handle.setData("arguments", "id=1") - handle.setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - handle.setApp(app) - - handle.start(fixture.scopes) - - verify(fixture.transaction).setData("arguments", "id=1") - verify(fixture.transaction).setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - assertThat(fixture.transaction.contexts.app).isSameInstanceAs(app) - } - - @Test - fun `start() - replays only the latest value set for each key`() { - val fixture = Fixture.create() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - val latestNavigationContext = mapOf("Back Stack" to listOf(mapOf("entry" to "/Receipt"))) - val latestApp = App().apply { viewNames = listOf("/Receipt") } - handle.setData("arguments", "id=1") - handle.setData("arguments", "id=2") - handle.setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - handle.setContext(NAVIGATION_CONTEXT_KEY, latestNavigationContext) - handle.setApp(app) - handle.setApp(latestApp) - - handle.start(fixture.scopes) - - verify(fixture.transaction, never()).setData("arguments", "id=1") - verify(fixture.transaction).setData("arguments", "id=2") - verify(fixture.transaction, never()).setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - verify(fixture.transaction).setContext(NAVIGATION_CONTEXT_KEY, latestNavigationContext) - assertThat(fixture.transaction.contexts.app).isSameInstanceAs(latestApp) - } - - @Test - fun `start() - returns null when the scopes decline to start a transaction`() { - val fixture = Fixture.create(declinesTransactions = true) - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - val started = handle.start(fixture.scopes) - - assertThat(started).isNull() - } - - @Test - fun `start() - starts the transaction on a retry after the scopes declined to start one`() { - val fixture = Fixture.create(declinesTransactions = true) - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - handle.setData("arguments", "id=1") - handle.start(fixture.scopes) - fixture.declinesTransactions = false - - val started = handle.start(fixture.scopes) - - assertThat(started).isSameInstanceAs(fixture.transaction) - verify(fixture.transaction).setData("arguments", "id=1") - } - - @Test - fun `start() - starts no transaction a second time`() { - val fixture = Fixture.create() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - handle.start(fixture.scopes) - - val started = handle.start(fixture.scopes) - - assertThat(started).isNull() - verify(fixture.scopes).startTransaction(any(), any()) - } - - @Test - fun `start() - starts no transaction for a started handle`() { - val fixture = Fixture.create() - val handle = NavTransaction.started(fixture.transaction) - - val started = handle.start(fixture.scopes) - - assertThat(started).isNull() - verify(fixture.scopes, never()).startTransaction(any(), any()) - } - - @Test - fun `finish() - finishes a started handle's transaction with the transaction's status`() { - val fixture = Fixture.create() - whenever(fixture.transaction.status).thenReturn(SpanStatus.CANCELLED) - val handle = NavTransaction.started(fixture.transaction) - - handle.finish(onFinished = {}) - - verify(fixture.transaction).finish(SpanStatus.CANCELLED) - } - - @Test - fun `finish() - finishes a started handle's transaction with OK when the transaction has no status`() { - val fixture = Fixture.create() - val handle = NavTransaction.started(fixture.transaction) - - handle.finish(onFinished = {}) - - verify(fixture.transaction).finish(SpanStatus.OK) - } - - @Test - fun `finish() - passes a started handle's transaction to the callback after finishing it`() { - val fixture = Fixture.create() - val onFinished = mock<(ITransaction) -> Unit>() - val handle = NavTransaction.started(fixture.transaction) - - handle.finish(onFinished) - - inOrder(fixture.transaction, onFinished) { - verify(fixture.transaction).finish(SpanStatus.OK) - verify(onFinished).invoke(fixture.transaction) - } - } - - @Test - fun `finish() - doesn't finish a started handle's transaction that has already finished`() { - val fixture = Fixture.create() - whenever(fixture.transaction.isFinished).thenReturn(true) - val handle = NavTransaction.started(fixture.transaction) - - handle.finish(onFinished = {}) - - verify(fixture.transaction, never()).finish(any()) - } - - @Test - fun `finish() - passes a started handle's already finished transaction to the callback`() { - val fixture = Fixture.create() - val onFinished = mock<(ITransaction) -> Unit>() - whenever(fixture.transaction.isFinished).thenReturn(true) - val handle = NavTransaction.started(fixture.transaction) - - handle.finish(onFinished) - - verify(onFinished).invoke(fixture.transaction) - } - - @Test - fun `finish() - finishes a deferred handle's transaction once it has started`() { - val fixture = Fixture.create() - val onFinished = mock<(ITransaction) -> Unit>() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - handle.start(fixture.scopes) - - handle.finish(onFinished) - - verify(fixture.transaction).finish(SpanStatus.OK) - verify(onFinished).invoke(fixture.transaction) - } - - @Test - fun `finish() - discards a pending handle so that it never starts a transaction`() { - val fixture = Fixture.create() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - handle.finish(onFinished = {}) - val started = handle.start(fixture.scopes) - - assertThat(started).isNull() - verify(fixture.scopes, never()).startTransaction(any(), any()) - } - - @Test - fun `finish() - does not call back for a pending handle`() { - val onFinished = mock<(ITransaction) -> Unit>() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - - handle.finish(onFinished) - - verifyNoInteractions(onFinished) - } - - @Test - fun `a discarded handle ignores later writes and finishes`() { - val fixture = Fixture.create() - val onFinished = mock<(ITransaction) -> Unit>() - val handle = NavTransaction.deferred(transactionContext, transactionOptions) - handle.finish(onFinished = {}) - - handle.setData("arguments", "id=1") - handle.setContext(NAVIGATION_CONTEXT_KEY, navigationContext) - handle.setApp(app) - handle.finish(onFinished) - val started = handle.start(fixture.scopes) - - assertThat(started).isNull() - verifyNoInteractions(fixture.transaction, onFinished) - } - - private class Fixture private constructor(val scopes: IScopes, val transaction: ITransaction) { - - /** Whether [scopes] declines to start transactions, returning a no-op transaction instead. */ - var declinesTransactions = false - - companion object { - fun create(declinesTransactions: Boolean = false): Fixture { - val contexts = Contexts() - val transaction = - mock().also { - whenever(it.name).thenReturn("/Checkout") - whenever(it.contexts).thenReturn(contexts) - } - val scopes = mock() - val fixture = Fixture(scopes, transaction) - fixture.declinesTransactions = declinesTransactions - whenever(scopes.startTransaction(any(), any())) - .thenAnswer { - if (fixture.declinesTransactions) NoOpTransaction.getInstance() else transaction - } - return fixture - } - } - } -} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransactionTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransactionTest.kt index e732db751e9..0488c7d1c17 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransactionTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransactionTest.kt @@ -9,6 +9,7 @@ import io.sentry.Scope import io.sentry.ScopeObserverAdapter import io.sentry.SentryLevel import io.sentry.SentryOptions +import io.sentry.SpanContext import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions @@ -24,232 +25,267 @@ import org.mockito.kotlin.whenever @Suppress("LargeClass") class StagedNavTransactionTest { - private val transactionContext = TransactionContext("/Profile", "navigation") - private val transactionOptions = TransactionOptions() + private val request = navTransactionRequest("/Profile") + private val updatedContexts = mapOf("Navigation" to "updated") private val finishedTransaction = mock().also { whenever(it.isFinished).thenReturn(true) } - private val hostTransaction = mock() + private val hostTransaction = transactionWithOrigin("manual") @Test - fun `start() - starts a nav transaction and binds it to a free scope`() { + fun `publishTo() - keeps the started nav transaction when binding it throws`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction + stagedNavTransaction.stage(request) + fixture.transactionObserverThrows = true - val navTransaction = - stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) - + assertThrows(ThrowingScopeObserverException::class.java) { + stagedNavTransaction.publishTo(fixture.scope) + } + fixture.transactionObserverThrows = false val started = fixture.startedTransaction("/Profile") - verify(fixture.scopes).startTransaction(transactionContext, transactionOptions) - assertThat(navTransaction?.transaction).isSameInstanceAs(started) - assertThat(fixture.scope.transaction).isSameInstanceAs(started) + stagedNavTransaction.finish(fixture.scope) + + verify(started).finish(SpanStatus.OK) } @Test - fun `start() - starts no nav transaction while another transaction is bound`() { + fun `stage() - starts no transaction`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - fixture.scope.transaction = hostTransaction - val navTransaction = - stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + stagedNavTransaction.stage(request) - assertThat(navTransaction).isNull() assertThat(fixture.startedTransactions).isEmpty() - assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + assertThat(fixture.scope.transaction).isNull() } @Test - fun `start() - logs the route when another transaction blocks the start`() { + fun `stage() - replaces the current nav transaction without finishing it`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - fixture.scope.transaction = hostTransaction + val current = fixture.start("/Home") - stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + stagedNavTransaction.stage(request) + stagedNavTransaction.finish(fixture.scope) - verify(fixture.logger).log(eq(SentryLevel.DEBUG), any(), eq("/Profile")) + verify(current, never()).finish(any()) } @Test - fun `start() - binds the nav transaction in place of a finished bound transaction`() { + fun `publishTo() - starts a staged nav transaction and binds it to a free scope`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - fixture.scope.transaction = finishedTransaction + stagedNavTransaction.stage(request) - val navTransaction = - stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + stagedNavTransaction.publishTo(fixture.scope) - val started = fixture.startedTransaction("/Profile") - assertThat(navTransaction?.transaction).isSameInstanceAs(started) - assertThat(fixture.scope.transaction).isSameInstanceAs(started) + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Profile")) } @Test - fun `start() - returns null and keeps the current nav transaction when the start is declined`() { + fun `publishTo() - starts a staged nav transaction in place of a finished bound transaction`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val current = fixture.start("/Home") - fixture.declinesTransactions = true + stagedNavTransaction.stage(request) + fixture.scope.transaction = finishedTransaction - val navTransaction = - stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + stagedNavTransaction.publishTo(fixture.scope) - assertThat(navTransaction).isNull() - assertThat(fixture.scope.transaction).isSameInstanceAs(current) - stagedNavTransaction.finish(fixture.scope) - verify(current).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Profile")) } @Test - fun `start() - replaces the current nav transaction without finishing it`() { + fun `publishTo() - doesn't start a staged nav transaction while another transaction is bound`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val current = fixture.start("/Home") - // Unbound, so that it doesn't block the start. - stagedNavTransaction.unpublishFrom(fixture.scope) + stagedNavTransaction.stage(request) + fixture.scope.transaction = hostTransaction - stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) - verify(current, never()).finish(any()) + stagedNavTransaction.publishTo(fixture.scope) - stagedNavTransaction.finish(fixture.scope) - verify(fixture.startedTransaction("/Profile")).finish(SpanStatus.OK) - verify(current, never()).finish(any()) + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) } @Test - fun `start() - keeps the started nav transaction when binding it throws`() { + fun `publishTo() - logs the route when another transaction blocks the nav transaction`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - fixture.transactionObserverThrows = true + stagedNavTransaction.stage(request) + fixture.scope.transaction = hostTransaction - assertThrows(ThrowingScopeObserverException::class.java) { - stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) - } - fixture.transactionObserverThrows = false - val started = fixture.startedTransaction("/Profile") - stagedNavTransaction.finish(fixture.scope) + stagedNavTransaction.publishTo(fixture.scope) - verify(started).finish(SpanStatus.OK) + verify(fixture.logger).log(eq(SentryLevel.DEBUG), any(), eq("/Profile")) } @Test - fun `stage() - returns a pending nav transaction without starting a transaction`() { - val fixture = Fixture.create() + fun `publishTo() - binds nothing when a staged nav transaction's start is declined`() { + val fixture = Fixture.create(declinesTransactions = true) val stagedNavTransaction = fixture.stagedNavTransaction + stagedNavTransaction.stage(request) - val navTransaction = stagedNavTransaction.stage(transactionContext, transactionOptions) + stagedNavTransaction.publishTo(fixture.scope) - assertThat(navTransaction.transaction).isNull() - assertThat(navTransaction.name).isEqualTo("/Profile") - assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isNull() } @Test - fun `stage() - replaces the current nav transaction without finishing it`() { + fun `publishTo() - starts a nav transaction for a request whose staged one was declined`() { + val fixture = Fixture.create(declinesTransactions = true) + val stagedNavTransaction = fixture.stagedNavTransaction + stagedNavTransaction.stage(request) + stagedNavTransaction.publishTo(fixture.scope) + fixture.declinesTransactions = false + + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Profile")) + } + + @Test + fun `publishTo() - binds an unpublished unfinished nav transaction again`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val current = fixture.start("/Home") + val started = fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) - stagedNavTransaction.stage(transactionContext, transactionOptions) - stagedNavTransaction.finish(fixture.scope) + stagedNavTransaction.publishTo(fixture.scope) - verify(current, never()).finish(any()) + assertThat(fixture.scope.transaction).isSameInstanceAs(started) + assertThat(fixture.startedTransactions).containsExactly(started) } @Test - fun `publishTo() - starts a staged nav transaction and binds it to a free scope`() { + fun `publishTo() - keeps the bound nav transaction bound`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val navTransaction = stagedNavTransaction.stage(transactionContext, transactionOptions) + val started = fixture.start() stagedNavTransaction.publishTo(fixture.scope) - val started = fixture.startedTransaction("/Profile") - assertThat(navTransaction.transaction).isSameInstanceAs(started) assertThat(fixture.scope.transaction).isSameInstanceAs(started) + verify(fixture.logger, never()).log(eq(SentryLevel.DEBUG), any(), any()) } @Test - fun `publishTo() - starts a staged nav transaction in place of a finished bound transaction`() { + fun `publishTo() - binds an unpublished nav transaction in place of a finished bound transaction`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - stagedNavTransaction.stage(transactionContext, transactionOptions) + val started = fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) fixture.scope.transaction = finishedTransaction stagedNavTransaction.publishTo(fixture.scope) - assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Profile")) + assertThat(fixture.scope.transaction).isSameInstanceAs(started) } @Test - fun `publishTo() - doesn't start a staged nav transaction while another transaction is bound`() { + fun `publishTo() - doesn't bind an unpublished nav transaction while another transaction is bound`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - stagedNavTransaction.stage(transactionContext, transactionOptions) + val started = fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) fixture.scope.transaction = hostTransaction stagedNavTransaction.publishTo(fixture.scope) - assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + stagedNavTransaction.finish(fixture.scope) + verify(started).finish(SpanStatus.OK) } @Test - fun `publishTo() - logs the route when another transaction blocks the nav transaction`() { + fun `publishTo() - forgets an unpublished nav transaction that has finished and keeps another transaction`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - stagedNavTransaction.stage(transactionContext, transactionOptions) + val started = fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) + whenever(started.isFinished).thenReturn(true) fixture.scope.transaction = hostTransaction stagedNavTransaction.publishTo(fixture.scope) - verify(fixture.logger).log(eq(SentryLevel.DEBUG), any(), eq("/Profile")) + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + } + + @Test + fun `publishTo() - leaves the scope untouched without a nav transaction`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.scope.transaction = finishedTransaction + + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(finishedTransaction) + } + + @Test + fun `publishTo() - starts a new nav transaction for the last request once the last one finished`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val finished = fixture.start() + whenever(finished.isFinished).thenReturn(true) + + stagedNavTransaction.publishTo(fixture.scope) + + val restarted = fixture.startedTransactions.last() + assertThat(fixture.startedTransactions).hasSize(2) + assertThat(restarted).isNotSameInstanceAs(finished) + assertThat(restarted.name).isEqualTo("/Profile") + assertThat(fixture.scope.transaction).isSameInstanceAs(restarted) } @Test - fun `publishTo() - discards a staged nav transaction blocked by another transaction`() { + fun `publishTo() - starts a nav transaction for a request whose staged one was discarded`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val navTransaction = stagedNavTransaction.stage(transactionContext, transactionOptions) + stagedNavTransaction.stage(request) fixture.scope.transaction = hostTransaction stagedNavTransaction.publishTo(fixture.scope) fixture.scope.clearTransaction() stagedNavTransaction.publishTo(fixture.scope) - assertThat(navTransaction.start(fixture.scopes)).isNull() - assertThat(fixture.startedTransactions).isEmpty() - assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Profile")) } @Test - fun `publishTo() - binds nothing when a staged nav transaction's start is declined`() { - val fixture = Fixture.create(declinesTransactions = true) + fun `publishTo() - starts a nav transaction for the most recent of several requests`() { + val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - stagedNavTransaction.stage(transactionContext, transactionOptions) + fixture.scope.transaction = hostTransaction + stagedNavTransaction.stage(navTransactionRequest("/Home")) + stagedNavTransaction.publishTo(fixture.scope) + stagedNavTransaction.stage(request) + stagedNavTransaction.publishTo(fixture.scope) + fixture.scope.clearTransaction() stagedNavTransaction.publishTo(fixture.scope) - assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Profile")) + assertThat(fixture.startedTransactions).hasSize(1) } @Test - fun `publishTo() - discards a staged nav transaction whose start was declined`() { - val fixture = Fixture.create(declinesTransactions = true) + fun `publishTo() - makes the restarted nav transaction the one that finish() finishes`() { + val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - stagedNavTransaction.stage(transactionContext, transactionOptions) + val finished = fixture.start() + whenever(finished.isFinished).thenReturn(true) stagedNavTransaction.publishTo(fixture.scope) - fixture.declinesTransactions = false + val restarted = fixture.startedTransactions.last() - stagedNavTransaction.publishTo(fixture.scope) + stagedNavTransaction.finish(fixture.scope) - assertThat(fixture.startedTransactions).isEmpty() + verify(restarted).finish(SpanStatus.OK) assertThat(fixture.scope.transaction).isNull() } @Test - fun `publishTo() - binds an unpublished unfinished nav transaction again`() { + fun `publishTo() - starts no nav transaction while the started one is unfinished`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction val started = fixture.start() @@ -257,85 +293,144 @@ class StagedNavTransactionTest { stagedNavTransaction.publishTo(fixture.scope) - assertThat(fixture.scope.transaction).isSameInstanceAs(started) assertThat(fixture.startedTransactions).containsExactly(started) } @Test - fun `publishTo() - keeps the bound nav transaction bound`() { + fun `publishTo() - starts no nav transaction without a request`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val started = fixture.start() stagedNavTransaction.publishTo(fixture.scope) - assertThat(fixture.scope.transaction).isSameInstanceAs(started) - verify(fixture.logger, never()).log(eq(SentryLevel.DEBUG), any(), any()) + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isNull() } @Test - fun `publishTo() - binds an unpublished nav transaction in place of a finished bound transaction`() { + fun `updateRequest() - restarts the nav transaction from the updated request`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val started = fixture.start() - stagedNavTransaction.unpublishFrom(fixture.scope) - fixture.scope.transaction = finishedTransaction + val finished = fixture.start() + whenever(finished.isFinished).thenReturn(true) + stagedNavTransaction.updateRequest( + navTransactionRequest("/Profile", contexts = updatedContexts) + ) stagedNavTransaction.publishTo(fixture.scope) + verify(fixture.startedTransactions.last()).setContext("Navigation", "updated") + } + + @Test + fun `updateRequest() - starts a staged nav transaction from the updated request`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + stagedNavTransaction.stage(request) + + stagedNavTransaction.updateRequest( + navTransactionRequest("/Profile", contexts = updatedContexts) + ) + stagedNavTransaction.publishTo(fixture.scope) + + val started = fixture.startedTransaction("/Profile") + verify(started).setContext("Navigation", "updated") assertThat(fixture.scope.transaction).isSameInstanceAs(started) } @Test - fun `publishTo() - doesn't bind an unpublished nav transaction while another transaction is bound`() { + fun `updateRequest() - leaves a started nav transaction alone`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction val started = fixture.start() - stagedNavTransaction.unpublishFrom(fixture.scope) - fixture.scope.transaction = hostTransaction + stagedNavTransaction.updateRequest( + navTransactionRequest("/Profile", contexts = updatedContexts) + ) stagedNavTransaction.publishTo(fixture.scope) - assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + assertThat(fixture.startedTransactions).containsExactly(started) + verify(started, never()).setContext(any(), any()) + assertThat(fixture.scope.transaction).isSameInstanceAs(started) + } + + @Test + fun `updateRequest() - keeps no request after the nav transaction is finished`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.start() stagedNavTransaction.finish(fixture.scope) - verify(started).finish(SpanStatus.OK) + + stagedNavTransaction.updateRequest( + navTransactionRequest("/Profile", contexts = updatedContexts) + ) + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.startedTransactions).hasSize(1) + assertThat(fixture.scope.transaction).isNull() } @Test - fun `publishTo() - forgets and unbinds a bound nav transaction that has finished`() { + fun `updateRequest() - keeps no request without a request`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + + stagedNavTransaction.updateRequest(request) + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.startedTransactions).isEmpty() + } + + @Test + fun `publishTo() - starts no nav transaction after the nav transaction is finished`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction val started = fixture.start() - whenever(started.isFinished).thenReturn(true) + stagedNavTransaction.finish(fixture.scope) stagedNavTransaction.publishTo(fixture.scope) + assertThat(fixture.startedTransactions).containsExactly(started) assertThat(fixture.scope.transaction).isNull() } @Test - fun `publishTo() - forgets an unpublished nav transaction that has finished and keeps another transaction`() { + fun `publishTo() - starts no nav transaction after a staged nav transaction is finished`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + stagedNavTransaction.stage(request) + stagedNavTransaction.finish(fixture.scope) + + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.startedTransactions).isEmpty() + } + + @Test + fun `publishTo() - starts no nav transaction after the nav transaction is forgotten`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction val started = fixture.start() - stagedNavTransaction.unpublishFrom(fixture.scope) whenever(started.isFinished).thenReturn(true) - fixture.scope.transaction = hostTransaction + stagedNavTransaction.forget() stagedNavTransaction.publishTo(fixture.scope) - assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + assertThat(fixture.startedTransactions).containsExactly(started) } @Test - fun `publishTo() - leaves the scope untouched without a nav transaction`() { + fun `publishTo() - starts no nav transaction while another transaction is bound`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - fixture.scope.transaction = finishedTransaction + val finished = fixture.start() + whenever(finished.isFinished).thenReturn(true) + fixture.scope.transaction = hostTransaction stagedNavTransaction.publishTo(fixture.scope) - assertThat(fixture.scope.transaction).isSameInstanceAs(finishedTransaction) + assertThat(fixture.startedTransactions).containsExactly(finished) + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) } @Test @@ -367,13 +462,13 @@ class StagedNavTransactionTest { fun `unpublishFrom() - keeps the bound transaction while the nav transaction is staged`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val navTransaction = stagedNavTransaction.stage(transactionContext, transactionOptions) + stagedNavTransaction.stage(request) fixture.scope.transaction = hostTransaction stagedNavTransaction.unpublishFrom(fixture.scope) assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) - assertThat(navTransaction.transaction).isNull() + assertThat(fixture.startedTransactions).isEmpty() } @Test @@ -388,6 +483,31 @@ class StagedNavTransactionTest { assertThat(fixture.scope.transaction).isNull() } + @Test + fun `finish() - finishes the nav transaction with its own status`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + whenever(started.status).thenReturn(SpanStatus.CANCELLED) + + stagedNavTransaction.finish(fixture.scope) + + verify(started).finish(SpanStatus.CANCELLED) + } + + @Test + fun `finish() - unbinds a nav transaction that already finished without finishing it again`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + whenever(started.isFinished).thenReturn(true) + + stagedNavTransaction.finish(fixture.scope) + + verify(started, never()).finish(any()) + assertThat(fixture.scope.transaction).isNull() + } + @Test fun `finish() - finishes an unpublished nav transaction and keeps another bound transaction`() { val fixture = Fixture.create() @@ -418,12 +538,11 @@ class StagedNavTransactionTest { fun `finish() - discards a staged nav transaction so that publishing never starts it`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - val navTransaction = stagedNavTransaction.stage(transactionContext, transactionOptions) + stagedNavTransaction.stage(request) stagedNavTransaction.finish(fixture.scope) stagedNavTransaction.publishTo(fixture.scope) - assertThat(navTransaction.start(fixture.scopes)).isNull() assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isNull() } @@ -473,7 +592,7 @@ class StagedNavTransactionTest { fun `forget() - forgets a staged nav transaction so that publishing never starts it`() { val fixture = Fixture.create() val stagedNavTransaction = fixture.stagedNavTransaction - stagedNavTransaction.stage(transactionContext, transactionOptions) + stagedNavTransaction.stage(request) stagedNavTransaction.forget() stagedNavTransaction.publishTo(fixture.scope) @@ -485,6 +604,11 @@ class StagedNavTransactionTest { /** Thrown by a scope observer, to stand in for one the host app registered. */ private class ThrowingScopeObserverException : RuntimeException() + private fun transactionWithOrigin(origin: String): ITransaction = + mock().also { + whenever(it.spanContext).thenReturn(SpanContext("op").apply { this.origin = origin }) + } + private class Fixture private constructor() { val scope = Scope(SentryOptions()) val logger = mock() @@ -538,11 +662,8 @@ class StagedNavTransactionTest { /** Starts a nav transaction for [name], binding it if the scope is free, and returns it. */ fun start(name: String = "/Profile"): ITransaction { - stagedNavTransaction.start( - scope, - TransactionContext(name, "navigation"), - TransactionOptions(), - ) + stagedNavTransaction.stage(navTransactionRequest(name)) + stagedNavTransaction.publishTo(scope) return startedTransaction(name) } @@ -552,3 +673,13 @@ class StagedNavTransactionTest { } } } + +private fun navTransactionRequest( + name: String, + contexts: Map = emptyMap(), +): NavTransactionRequest = + NavTransactionRequest( + name = name, + newOptions = ::TransactionOptions, + contexts = contexts, + ) From fb99ac6e280b33ea867452bb486412afb285942a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 6 Nov 2026 05:37:46 +0100 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0dd01679b8..ff1ad7074b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ ### Behavioral Changes -- Nav3: Returning to an Activity or Fragment with its own `SentryNavEffect` within the app (e.g., closing the Activity that covered it, swiping back to its ViewPager2 page, or dismissing a dialog over it) now starts a new navigation transaction for the current destination if the previous one finished, so spans produced upon return can now be consistently captured ([#XXXX](https://github.com/getsentry/sentry-java/pull/XXXX)) +- Nav3: Returning to an Activity or Fragment with its own `SentryNavEffect` within the app (e.g., closing the Activity that covered it, swiping back to its ViewPager2 page, or dismissing a dialog over it) now starts a new navigation transaction for the current destination if the previous one finished, so spans produced upon return can now be consistently captured ([#6482](https://github.com/getsentry/sentry-java/pull/6482)) ## 8.63.0