diff --git a/CHANGELOG.md b/CHANGELOG.md index da49e486cb1..ff1ad7074b6 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 ([#6482](https://github.com/getsentry/sentry-java/pull/6482)) + ## 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, + )