diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt index 6100c70620..4bd0bd24e5 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt @@ -450,7 +450,7 @@ internal data class NormalizedSentryBackStackEntry( internal fun List.serialize(): List> = map(NormalizedSentryBackStackEntry::serialize) -/** Why host-provided arguments were unavailable in emitted navigation data. */ +/** Why host-provided arguments were unavailable in emitted nav state. */ internal enum class ArgumentDropReason( val serializedValue: String, val exhaustsUpdateBudget: Boolean = false, diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt index 7455cfda5f..d26336d106 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt @@ -78,7 +78,7 @@ public class SentryBackStackEntry( * * If [map] throws, returns `null`, or returns a blank [name][SentryBackStackEntry.name], Sentry * records the destination as "/unknown". Doing so signals that name extraction needs to be fixed - * while avoiding misleading gaps in navigation data. + * while avoiding misleading gaps in nav state. * * For instance, if a user navigates from `/home -> /detail -> /settings`, but the mapper for * `/detail` throws, the back stack record will be `/home -> /unknown -> /settings` rather than 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 8e2806686e..2c4fd1fb14 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 @@ -2,12 +2,9 @@ package io.sentry.compose.navigation3 import io.sentry.Breadcrumb import io.sentry.Hint -import io.sentry.ITransaction import io.sentry.SentryIntegrationPackageStorage -import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.INFO import io.sentry.SentryOptions -import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.compose.navigation3.BackStackConverter.RetentionPolicy @@ -16,6 +13,7 @@ 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 @@ -59,7 +57,7 @@ internal class BackStackObserver( private val backStackConverter = BackStackConverter(entryMapper, scopes.options.logger) - private val navTransaction = NavTransaction(scopes) + private val navTransactions = NavTransactions(scopes) private val navBreadcrumbs = NavBreadcrumbs(scopes) private val navScreen = NavScreen() private val navContext = NavContext(scopes, options) @@ -83,7 +81,7 @@ internal class BackStackObserver( } /** - * Updates Sentry nav data based on the provided [backStack]. + * Updates Sentry nav state based on the provided [backStack]. * * **Data generated** * @@ -113,7 +111,7 @@ internal class BackStackObserver( } /** - * Clears the nav data this observer recorded. + * Clears the nav state this observer recorded. * * Call it when observation ends so that no stale screen, nav context, or nav transaction outlives * it. @@ -194,10 +192,10 @@ internal class BackStackObserver( navBreadcrumbs.emit(fromEntry = previousTop, toEntry = currentBackStack.topEntryNormalized) } - navTransaction.stop(scope) + navTransactions.stop(scope) if (areNavigationTransactionsEnabled) { - navTransaction + navTransactions .start( scope, currentTop.name, @@ -231,7 +229,7 @@ internal class BackStackObserver( } private fun tearDown(scope: INavScope) { - navTransaction.stop(scope) + navTransactions.stop(scope) navContext.clear(scope) navScreen.clear(scope) clearPreviousTop() @@ -277,32 +275,22 @@ private data class BackStackData( ) /** A helper class for managing nav transactions. */ -private class NavTransaction(private val scopes: INavScopes) { +private class NavTransactions(private val scopes: INavScopes) { private companion object { private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" } - private var activeNavTransaction: ITransaction? = null - - /** Starts an idle navigation transaction, or no-ops if another transaction is already active. */ + /** + * 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, - ): ITransaction? { - scope.clearFinishedTransaction() - - if (scope.transaction != null) { - scopes.options.logger.log( - DEBUG, - "Nav3 transaction for route %s won't be created because another transaction is active.", - name, - ) - - return null - } - + ): INavTransaction? { val transactionOptions = TransactionOptions().also { it.isWaitForChildren = true @@ -314,36 +302,25 @@ private class NavTransaction(private val scopes: INavScopes) { } val transaction = - scopes.startTransaction( + scope.startNavTransaction( TransactionContext(name, TransactionNameSource.ROUTE, NAVIGATION_OP), transactionOptions, - ) + ) ?: return null - // Not an error: INavScopes.startTransaction may decline to start a transaction by design. - if (transaction.isNoOp) { - return null + if (arguments.isNotEmpty()) { + transaction.setData("arguments", arguments) } - activeNavTransaction = transaction - - transaction.apply { - if (arguments.isNotEmpty()) { - setData("arguments", arguments) - } - } - - scope.bindTransaction(transaction) - return transaction } - /** Finishes and unsets the active navigation transaction, if one exists. */ + /** + * Finishes and unsets the active navigation transaction, if one exists. + * + * See [INavScope.finishNavTransaction]. + */ fun stop(scope: INavScope) { - val transaction = activeNavTransaction ?: return - val status = transaction.status ?: SpanStatus.OK - transaction.finish(status) - scope.unbindTransaction(transaction) - activeNavTransaction = null + scope.finishNavTransaction() } } @@ -377,15 +354,15 @@ private class NavContext(private val scopes: INavScopes, private val options: Se * proper values to the transaction context instead. */ fun updateTransaction( - transaction: ITransaction, + transaction: INavTransaction, scope: INavScope, backStack: BackStackData, ) { if (scopes.options.isEnableScreenTracking) { - val appContext = transaction.contexts.app ?: scope.app ?: App() + val appContext = scope.app ?: App() appContext.viewNames = listOf(backStack.topEntryNormalized.name) - transaction.contexts.setApp(appContext) + transaction.setApp(appContext) } if (options.captureBackStack && backStack.capturedEntriesNormalized.isNotEmpty()) { diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt index 8cf9c6e1be..a34a214dec 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt @@ -22,7 +22,8 @@ import io.sentry.compose.navigation3.lifecycle.NavScopes * **Thread safety** * * This class is ***not*** thread-safe. Invocations should be confined to the same thread as the - * [LifecycleEventObserver] callbacks implemented by [NavScopes]. + * [LifecycleEventObserver] callbacks through which the [NavLeaseCoordinator] observes the attached + * lifecycle. */ internal class CoordinatedBackStackObserver( scopes: IScopes, diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt index 87b0f8f75b..9f8a0f87fd 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt @@ -1,30 +1,149 @@ package io.sentry.compose.navigation3.lifecycle import androidx.lifecycle.Lifecycle +import androidx.lifecycle.Lifecycle.Event.ON_DESTROY +import androidx.lifecycle.Lifecycle.Event.ON_PAUSE +import androidx.lifecycle.Lifecycle.Event.ON_RESUME +import androidx.lifecycle.Lifecycle.Event.ON_START +import androidx.lifecycle.Lifecycle.Event.ON_STOP +import androidx.lifecycle.LifecycleEventObserver import androidx.lifecycle.LifecycleOwner import io.sentry.IScope -import io.sentry.ITransaction -import io.sentry.SpanStatus +import io.sentry.IScopes /** * Manages a lease in order to determine which [NavScopes] instance can bind nav transactions and * publish nav data to a shared [IScope]. * - * `NavScopes` instances register as [candidates][Candidate] for lease ownership via - * [registerCandidate]. + * `NavScopes` instances register as [candidates][Candidate] for lease ownership via [register]. + * Each candidate observes its owner's lifecycle until it's disposed. * - * Note: "Owner" below refers to the `LifecycleOwner` whose lifecycle events the candidate's - * `NavScopes` relays. + * Note: "Owner" below refers to the `LifecycleOwner` whose lifecycle the candidate observes. * - * **Exception semantics** + * **A fundamental tradeoff** + * + * During `LifecycleOwner` overlap, the nav transaction and scoped nav data can reflect + * **_either_**: + * + * 1. the outgoing owner that's still visible, or + * 2. the incoming owner that may crash before it becomes visible, + * + * but not both. This class opts for (2) so that the incoming destination populates crash context + * and any work performed is associated with it† – the idea being that start-up logic is more likely + * to cause a crash or produce interesting spans than an outgoing destination's steady state. + * + * † Associating work with the incoming transaction means spans produced by the outgoing owner may + * appear as if they were produced by the incoming owner (or vice versa, were our policy reversed). + * A possible solution would be to implement a parent span delivery system similar to + * `LocalSentrySpan`, but that's a task for another day. + * + * **Lease handoff policy** + * + * The lease is acquired preemptively by an incoming owner's `NavScopes` instance when it applies + * its first [back stack update][onBackStackUpdate].† + * + * A displaced owner remains eligible for lease restoration until it + * [stops][Lifecycle.Event.ON_STOP]. It gets the lease back when it + * [resumes][Lifecycle.Event.ON_RESUME], or sooner if the new owner leaves or pauses while the + * displaced owner is still visible. If restored, the displaced owner's nav data and unfinished + * transaction are reinstated. + * + * † There are some exceptions, e.g., the leaseholder is resumed and the incoming owner isn't (e.g., + * a preloaded ViewPager2 Fragment page next to the current one). Such an owner acquires the lease + * when it resumes instead. + * + * The following diagram illustrates how handoff works: + * ``` + * Time + * │ + * ▼ + * + * NavScopes A + Lifecycle A Shared state NavScopes B + Lifecycle B + * ───────────────────────── ──────────── ───────────────────────── + * + * attach() Navigation lease: none + * │ + * Initial back stack update + * │ + * ├── acquire lease ────────► Navigation lease: A + * ├── create & bind txA ────► Scope transaction: txA + * ├── publish nav data ─────► Published data: A + * │ + * ON_RESUME A is eligible for restoration + * │ + * ON_PAUSE + * │ + * ├────────────────────────► Navigation lease: A (no other resumed owner) + * │ A remains eligible for restoration + * │ + * │ attach() + * │ │ + * │ Navigation lease: A ◄────────────────────┤ + * │ Scope transaction: txA │ + * │ Published data: A │ + * │ B registered, awaiting initial back │ + * │ stack update │ + * │ │ + * │ Initial back stack update + * │ │ + * │ Suspend & unbind txA ◄───────────────────┤ + * │ Navigation lease: B │ + * │ Published data: B │ + * │ Scope transaction: txB │ + * │ (rollback point) │ + * │ │ + * │ ON_RESUME + * │ │ + * │ B eligible for restoration ◄─────────────┤ + * │ │ + * ON_STOP │ + * │ │ + * ├── retain txA; timers continue │ + * ├────────────────────────► A no longer eligible for restoration │ + * │ │ + * ON_DESTROY │ + * │ │ + * ├── finish retained txA if unfinished │ + * └────────────────────────► A no longer tracked │ + * ON_PAUSE + * │ + * Navigation lease: B (no other resumed ◄─┤ + * owner) │ + * │ + * ON_STOP + * │ + * Navigation lease: B (no other resumed ◄─┤ + * owner); B keeps recording navigation │ + * B no longer eligible for restoration │ + * │ + * ON_DESTROY + * │ + * Finish txB if unfinished; ◄──────────────┤ + * unbind if still bound │ + * Clear B's remaining screen/back stack │ + * Retain recorded nav events │ + * Navigation lease: none │ + * ▼ + * + * 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 │ + * Finish txB if unfinished │ + * ``` + * + * **Exception policy** * * Methods in this class with an `IScope` parameter can throw arbitrary exceptions†, which propagate - * to the caller. Each method updates its own bookkeeping before calling into the scope, so the - * coordinator remains usable if the exception is caught. + * to the caller. Each method updates its own bookkeeping before calling into the scope or a + * candidate's [LeasedNavState], so the coordinator remains usable if the exception is caught. + * Candidates call these methods from [configureScope][IScopes.configureScope] callbacks, whose + * error handling catches the exceptions. * * † E.g., `IScope` can throw arbitrary exceptions when scope mutators notify registered observers – - * including the host app – in an unguarded loop, or when `finish()` is called on a retained nav - * transaction, which can trigger the host app's event processors. + * including the host app – in an unguarded loop, or when starting a nav transaction triggers the + * host app's callbacks. * * **Thread safety** * @@ -36,195 +155,47 @@ internal class NavLeaseCoordinator { private val candidates = mutableListOf() - /** Candidate responsible for the most recently published nav data in the shared scope. */ - private var publishedDataCandidate: Candidate? = null + /** Candidate whose nav state was most recently published to the shared scope. */ + private var publisher: Candidate? = null private var leaseHolder: Candidate? = null /** * Counter that orders candidates by when this coordinator most recently observed them in resumed * state. - * - * Each resume, including registration of an already-resumed owner, assigns the next value to that - * candidate's [Candidate.resumeOrder]. This lets fallback selection prefer the most recently - * resumed eligible candidate without relying on clock timestamps. Zero is reserved for candidates - * that have not been observed resumed and therefore cannot be chosen as fallbacks. */ private var resumeOrder: Long = 0 /** - * Registers [owner] as a candidate for the nav lease. No-ops if `owner` instance is already - * registered or [lifecycleState] is [destroyed][Lifecycle.State.DESTROYED]. - * - * [onPublishData] publishes `owner`'s staged nav data to the given scope when `owner` acquires - * the lease. [onClearPublishedData] clears `owner`'s published nav data from the given scope when - * its data is the most recently published and either another owner acquires the lease or `owner` - * is unregistered. + * Registers a candidate for the nav lease that observes [lifecycle] until it's + * [disposed][NavLeaseRegistration.dispose] or `lifecycle` is destroyed. Returns `null` if + * `lifecycle` is already [destroyed][Lifecycle.State.DESTROYED]. */ - fun registerCandidate( - owner: Any, - lifecycleState: Lifecycle.State, - onPublishData: (IScope) -> Unit, - onClearPublishedData: (IScope) -> Unit, - ) { - if (findCandidate(owner) != null) { - return - } - + fun register( + lifecycle: Lifecycle, + scopes: IScopes, + navState: LeasedNavState, + ): NavLeaseRegistration? { + val lifecycleState = lifecycle.currentState if (lifecycleState == Lifecycle.State.DESTROYED) { - return + return null } val initialResumeOrder = if (lifecycleState == Lifecycle.State.RESUMED) ++resumeOrder else 0 - candidates += - Candidate(owner, onPublishData, onClearPublishedData, lifecycleState, initialResumeOrder) - } - - /** Records that [owner] started. */ - fun startCandidate(owner: Any) { - findCandidate(owner)?.start() - } - - /** - * Records that [owner] resumed. - * - * Restores the lease to `owner` if it already applied a back stack update and [scope] is - * non-null. - * - * A `null` scope (e.g., the SDK is disabled) still records the lifecycle transition so the - * candidate remains eligible for fallback once a scope becomes available. - * - * Can throw arbitrary exceptions. See "Exception semantics" in the class KDoc. - */ - fun resumeCandidate(owner: Any, scope: IScope?) { - findCandidate(owner)?.let { - it.resume(++resumeOrder) - - if (scope != null && it.hasAppliedInitialBackStackUpdate) { - transferLeaseTo(it, scope) - } - } - } - - /** - * Records that [owner] paused. - * - * Hands the lease to another owner only if that owner is resumed (e.g., a Fragment whose dialog - * is being dismissed); otherwise keeps it. A paused leaseholder may still be the topmost visible - * owner (e.g., beneath a translucent window or in picture-in-picture mode), and any other paused - * owner is beneath it. A displaced owner gets the lease back when it resumes. - * - * A `null` scope (e.g., the SDK is disabled) still records the lifecycle transition so the - * candidate remains eligible for fallback once a scope becomes available. - * - * Can throw arbitrary exceptions. See "Exception semantics" in the class KDoc. - */ - fun pauseCandidate(owner: Any, scope: IScope?) { - findCandidate(owner)?.let { - it.pause() - - if (leaseHolder === it) { - mostRecentlyResumedCandidate(excluding = it, resumedOnly = true)?.let { next -> - handOffLease(next, scope) - } - } - } - } - - /** - * Records that [owner] stopped. - * - * As with pausing, hands the lease to another owner only if that owner is resumed; otherwise the - * stopped owner keeps it. Keeping it lets a lone owner go on recording navigation that happens - * while it isn't resumed (e.g., while the app is in the background, or after it restarts but - * before it resumes), including starting nav transactions. - * - * A paused owner doesn't get the lease: when the leaseholder stops before it, the app is - * typically going to the background (e.g., a translucent leaseholder stops before the owner - * beneath it), and that owner isn't what the user last saw. A paused owner that comes back to the - * foreground gets the lease when it resumes. - * - * A `null` scope (e.g., the SDK is disabled) still records the lifecycle transition so the - * candidate remains eligible for fallback once a scope becomes available. - * - * Can throw arbitrary exceptions. See "Exception semantics" in the class KDoc. - */ - fun stopCandidate(owner: Any, scope: IScope?) { - findCandidate(owner)?.let { - it.stop() - - if (leaseHolder === it) { - mostRecentlyResumedCandidate(resumedOnly = true)?.let { next -> handOffLease(next, scope) } - } - } - } - - /** - * Stops tracking [owner] (e.g., because it was destroyed or its effect left the composition). - * No-ops if `owner` instance isn't registered. - * - * Finishes and unbinds `owner`'s nav transaction, and clears its published nav data if that data - * is the most recently published. If `owner` holds the lease, hands it to the eligible candidate - * most recently observed resumed, even if that candidate is now only started or paused, or - * releases it if there's none. - * - * A `null` scope (e.g., the SDK is disabled) still removes the candidate, but its transaction is - * left to time out, its published data is left in place, and the lease is released rather than - * handed off. - * - * Can throw arbitrary exceptions. See "Exception semantics" in the class KDoc. Coordinator - * references to `owner` are released before any scope call, so they're released even if scope - * cleanup or lease handoff throws. The remaining scope cleanup is then skipped. - */ - fun unregisterCandidate(owner: Any, scope: IScope?) { - val candidate = findCandidate(owner) ?: return - val transaction = candidate.transaction - val wasPublisher = publishedDataCandidate === candidate - val wasLeaseHolder = leaseHolder === candidate - - candidates.remove(candidate) - candidate.transaction = null - if (wasPublisher) { - publishedDataCandidate = null - } - if (wasLeaseHolder) { - leaseHolder = null - } - - // No need to finish the transaction if we don't have a scope (e.g., the SDK is disabled), as - // the transaction can't be sent and it will time out anyway. Not finishing it has the added - // advantage of keeping SDK work inside `configureScope`'s error handling. - if (scope == null) { - return - } + val candidate = Candidate(lifecycle, scopes, navState, lifecycleState, initialResumeOrder) + candidates += candidate - finishTransaction(transaction, scope) - if (wasPublisher) { - candidate.onClearPublishedData(scope) - } - if (wasLeaseHolder) { - transferLeaseTo(mostRecentlyResumedCandidate(), scope) - } + lifecycle.addObserver(candidate) + return candidate } - fun hasLease(owner: Any): Boolean = leaseHolder?.owner === owner - /** - * Records that the registered [owner] is about to apply a back-stack update and acquires the nav - * lease if the owner's lifecycle permits it. + * Records that [candidate] is about to apply a back-stack update and acquires the nav lease if + * the owner's lifecycle permits it. * - * An owner that isn't resumed doesn't take the lease from a resumed leaseholder, as it isn't the - * one the user is interacting with (e.g., a preloaded ViewPager2 page next to the current one). - * Instead, it acquires the lease when it resumes. - * - * If the lease transfers, ownership and transaction binding are updated before the candidate's - * staged nav data is published. Publication completes before this method returns, so the caller - * can then apply the back-stack update. Later calls for the same [owner] do nothing. - * - * Can throw arbitrary exceptions. See "Exception semantics" in the class KDoc. + * Later calls for the same [candidate] do nothing. */ - fun onBackStackUpdate(owner: Any, scope: IScope) { - val candidate = findCandidate(owner) ?: return + private fun onBackStackUpdate(candidate: Candidate, scope: IScope) { if (candidate.hasAppliedInitialBackStackUpdate) { return } @@ -232,6 +203,10 @@ internal class NavLeaseCoordinator { candidate.hasAppliedInitialBackStackUpdate = true val holder = leaseHolder + + // An owner that isn't resumed doesn't take the lease from a resumed leaseholder, as it isn't + // the one the user is interacting with (e.g., a preloaded ViewPager2 page next to the current + // page). Instead, it acquires the lease when it resumes. val wouldDisplaceResumedHolder = holder != null && holder.state == Lifecycle.State.RESUMED && @@ -242,49 +217,66 @@ internal class NavLeaseCoordinator { } } - /** Whether [transaction] is a nav transaction retained for any registered owner. */ - fun isNavTransaction(transaction: ITransaction): Boolean = candidates.any { - it.transaction === transaction + /** + * Records that [candidate] resumed. + * + * Restores the lease to `candidate` if it already applied a back stack update and [scope] is + * non-null. + */ + private fun resumeCandidate(candidate: Candidate, scope: IScope?) { + // A null scope (e.g., the SDK is disabled) still records the lifecycle transition so that the + // candidate remains eligible for fallback once the scope becomes available. + candidate.resume(++resumeOrder) + + if (scope != null && candidate.hasAppliedInitialBackStackUpdate) { + transferLeaseTo(candidate, scope) + } } /** - * Records [transaction] as the nav transaction of [owner], which must hold the lease. Ignored - * otherwise, as only the leaseholder starts nav transactions. + * Records that [candidate] paused. * - * If [owner] loses the lease, the transaction is unbound but retained: it's rebound if [owner] - * regains the lease while it's still unfinished and the scope is free, and finished when [owner] - * is [unregistered][unregisterCandidate]. It's forgotten once [owner] retires it; see - * [releaseTransaction]. + * Hands the lease to another owner only if that owner is resumed (e.g., a Fragment whose dialog + * is being dismissed); otherwise keeps it. */ - fun retainTransaction(owner: Any, transaction: ITransaction) { - val candidate = leaseHolder ?: return - if (candidate.owner !== owner) { - return + private fun pauseCandidate(candidate: Candidate, scope: IScope?) { + candidate.pause() + + // A paused leaseholder may still be the topmost visible owner (e.g., beneath a translucent + // window or picture-in-picture mode), and any other paused owner is beneath it. A displaced + // owner will acquire the lease again when it resumes. + if (leaseHolder === candidate) { + mostRecentlyResumedCandidate(excluding = candidate, resumedOnly = true)?.let { next -> + handOffLease(next, scope) + } } - candidate.transaction = transaction } /** - * Forgets [transaction] as the nav transaction of [owner], if it's the one retained. Called when - * [owner] retires the transaction (e.g., it navigated away), so it's never rebound later. + * Records that [candidate] stopped. * - * Retiring is distinct from losing the lease: a transaction that's still waiting for child spans - * isn't [finished][ITransaction.isFinished] yet, so without this, regaining the lease would - * rebind a transaction whose destination [owner] has already left. + * As with pausing, hands the lease to another owner only if that owner is resumed; otherwise the + * stopped owner keeps it. Keeping it lets a lone owner go on recording navigation that happens + * while it isn't resumed (e.g., while the app is in the background, or after it restarts but + * before it resumes), including starting nav transactions. */ - fun releaseTransaction(owner: Any, transaction: ITransaction) { - val candidate = findCandidate(owner) ?: return - if (candidate.transaction === transaction) { - candidate.transaction = null + private fun stopCandidate(candidate: Candidate, scope: IScope?) { + candidate.stop() + + // A paused owner doesn't get the lease. When the leaseholder stops before it, the app is + // typically going to the background (e.g., a translucent leaseholder stops before the owner + // beneath it), and that owner isn't what the user last saw. A paused owner that comes back to + // the foreground will acquire the lease when it resumes. + if (leaseHolder === candidate) { + mostRecentlyResumedCandidate(resumedOnly = true)?.let { next -> handOffLease(next, scope) } } } - private fun findCandidate(owner: Any): Candidate? = candidates.firstOrNull { it.owner === owner } - /** - * The eligible candidate most recently observed resumed, other than [excluding]. With - * [resumedOnly], only candidates that are resumed right now are considered; otherwise, started - * and paused ones are too. + * The eligible candidate most recently observed to be in a resumed state, other than [excluding]. + * + * If [resumedOnly] is set to `true`, only candidates that are resumed right now are considered; + * if `false`, started and paused candidates are also considered. */ private fun mostRecentlyResumedCandidate( excluding: Candidate? = null, @@ -312,94 +304,74 @@ internal class NavLeaseCoordinator { } } - /** - * Hands the lease to [candidate], or releases it given `null`. - * - * Maintains the invariant that, while a lease is held, the leaseholder is also the candidate - * recorded as having published the shared scope's nav data. The invariant holds even when a - * publish or clear callback throws part way through the handoff. See "Exception semantics" in the - * class KDoc. - */ + /** Hands the lease to [candidate], or releases it given `null`. */ private fun transferLeaseTo(candidate: Candidate?, scope: IScope) { if (leaseHolder === candidate) { return } - if (candidate != null) { - // Unbind another owner's transaction without finishing it, so it can be restored if that - // owner returns. If this owner's retained transaction has since finished, discard it - // instead of binding a finished transaction to the shared scope. - scope.withTransaction { current -> - if (current != null && candidates.any { it !== candidate && it.transaction === current }) { - scope.clearTransaction() - } - } - - val transaction = candidate.transaction - if (transaction?.isFinished == true) { - candidate.transaction = null - clearTransactionFromScope(transaction, scope) - } else if (transaction != null) { - // The navigation lease doesn't give this owner control over an unrelated transaction - // already bound to the shared scope. Restore the owner's retained nav transaction only if - // the scope is free or already holds that same transaction. - scope.withTransaction { current -> - if (current == null || current === transaction) { - scope.transaction = transaction - } - } - } - } - - val previousPublisher = publishedDataCandidate + val previousPublisher = publisher leaseHolder = candidate if (candidate != null) { // Record the candidate as the publisher before calling into the scope: it owns the scope's - // nav data from the moment it takes the lease. If clearing the previous owner's data then - // throws, the next handoff still clears the candidate's data rather than never clearing it. - // - // Clear the previous publisher's nav data before publishing the candidate's, so the - // previous owner's values don't linger where the candidate has nothing staged to replace - // them. - publishedDataCandidate = candidate + // nav state from the moment it takes the lease. If unpublishing the previous owner's state + // then throws, the next handoff still unpublishes the candidate's state rather than never + // unpublishing it. + publisher = candidate if (previousPublisher != null && previousPublisher !== candidate) { - previousPublisher.onClearPublishedData(scope) + previousPublisher.navState.unpublishFrom(scope) } - candidate.onPublishData(scope) + candidate.navState.publishTo(scope) } } /** - * Finishes [transaction] if it's unfinished, and unbinds it from [scope]. + * Stops tracking [candidate] (e.g., because it was destroyed). * - * Callers must forget the transaction first. + * Unpublishes `candidate`'s nav state from [scope] if it's the most recently published. If + * `candidate` holds the lease, hands it to the eligible candidate most recently observed resumed, + * even if that candidate is now only started or paused, or releases it if there's none. */ - private fun finishTransaction(transaction: ITransaction?, scope: IScope) { - if (transaction == null) { - return + private fun unregisterCandidate(candidate: Candidate, scope: IScope?) { + val wasPublisher = publisher === candidate + val wasLeaseHolder = leaseHolder === candidate + + // Remove candidate before any scope calls to ensure it's released even if cleanup or handoff + // throws. + candidates.remove(candidate) + + if (wasPublisher) { + publisher = null } - if (!transaction.isFinished) { - transaction.finish(transaction.status ?: SpanStatus.OK) + if (wasLeaseHolder) { + leaseHolder = null } - clearTransactionFromScope(transaction, scope) - } - private fun clearTransactionFromScope(transaction: ITransaction, scope: IScope) { - scope.withTransaction { current -> - if (current === transaction) { - scope.clearTransaction() - } + // A null scope (e.g., the SDK is disabled) still removes the candidate, but its published nav + // state is left in place and the lease is released rather than handed off. + if (scope == null) { + return + } + + if (wasPublisher) { + candidate.navState.unpublishFrom(scope) + } + if (wasLeaseHolder) { + transferLeaseTo(mostRecentlyResumedCandidate(), scope) } } - /** A [LifecycleOwner] that has registered itself as a candidate for lease ownership. */ - private class Candidate( - val owner: Any, - val onPublishData: (IScope) -> Unit, - val onClearPublishedData: (IScope) -> Unit, + /** + * A [LifecycleOwner] registered as a candidate for lease ownership. Relays its owner's lifecycle + * events to the coordinator until it's [disposed][dispose]. + */ + private inner class Candidate( + private val lifecycle: Lifecycle, + private val scopes: IScopes, + val navState: LeasedNavState, initialState: Lifecycle.State, initialResumeOrder: Long, - ) { + ) : NavLeaseRegistration, LifecycleEventObserver { /** The candidate's lifecycle state, as last observed. */ var state: Lifecycle.State = initialState @@ -407,15 +379,13 @@ internal class NavLeaseCoordinator { /** * Whether the candidate has been observed STOPPED since - * [registration][NavLeaseCoordinator.registerCandidate]. + * [registration][NavLeaseCoordinator.register]. */ private var wasStopped: Boolean = false - var transaction: ITransaction? = null - /** * See [NavLeaseCoordinator.resumeOrder]. Non-zero once the candidate has been observed RESUMED - * since [registration][NavLeaseCoordinator.registerCandidate]. + * since [registration][NavLeaseCoordinator.register]. */ var resumeOrder: Long = initialResumeOrder private set @@ -424,14 +394,9 @@ internal class NavLeaseCoordinator { /** * Whether the candidate's lifecycle permits its first back stack update since - * [registration][NavLeaseCoordinator.registerCandidate] to acquire the nav lease, including - * taking it from another owner that isn't resumed: it's resumed, or it has neither resumed nor - * stopped since registration (i.e., it's still coming up for the first time). A paused or - * restarted candidate is returning to the screen rather than arriving on it. - * - * This checks the candidate's own lifecycle only; it does not indicate whether the candidate - * has already applied its first update, nor account for a resumed leaseholder (see - * [NavLeaseCoordinator.onBackStackUpdate]). + * [registration][NavLeaseCoordinator.register] to acquire the nav lease: it's resumed, or it + * has neither resumed nor stopped since registration (i.e., it's still coming up for the first + * time). */ val canAcquireLeaseOnFirstUpdate: Boolean get() = state == Lifecycle.State.RESUMED || (resumeOrder == 0L && !wasStopped) @@ -457,5 +422,112 @@ internal class NavLeaseCoordinator { state = Lifecycle.State.CREATED wasStopped = true } + + private var isDisposed = false + + override val hasLease: Boolean + get() = leaseHolder === this + + override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) { + when (event) { + ON_START -> start() + ON_RESUME -> withScopeOrNull { resumeCandidate(this, it) } + ON_PAUSE -> withScopeOrNull { pauseCandidate(this, it) } + ON_STOP -> withScopeOrNull { stopCandidate(this, it) } + ON_DESTROY -> dispose() + else -> Unit + } + } + + override fun onBackStackUpdate() { + if (isDisposed) { + return + } + scopes.configureScope { scope -> this@NavLeaseCoordinator.onBackStackUpdate(this, scope) } + } + + override fun dispose() { + if (isDisposed) { + return + } + isDisposed = true + lifecycle.removeObserver(this) + + withScopeOrNull { scope -> + try { + unregisterCandidate(this, scope) + } finally { + navState.onUnregistered(scope) + } + } + } + + /** + * Runs [block] with the real scope, or with `null` if [configureScope][IScopes.configureScope] + * skips its callback (e.g., the SDK is disabled), so coordinator bookkeeping still happens. + * + * A callback that ran but threw is not retried, as retrying could let its exception escape into + * the host app. + */ + private fun withScopeOrNull(block: (IScope?) -> Unit) { + var callbackInvoked = false + scopes.configureScope { scope -> + callbackInvoked = true + block(scope) + } + + if (!callbackInvoked) { + block(null) + } + } } } + +/** + * A candidate's registration with a [NavLeaseCoordinator], starting from + * [NavLeaseCoordinator.register] and continuing until it's [disposed][dispose] or its lifecycle is + * destroyed. + */ +internal interface NavLeaseRegistration { + + /** Whether the candidate holds the nav lease. */ + val hasLease: Boolean + + /** + * Announces that the candidate is about to apply a back stack update, so it can acquire the nav + * lease if it's eligible to. Does nothing once the candidate is unregistered. + */ + fun onBackStackUpdate() + + /** + * Stops observing the candidate's lifecycle and unregisters it: releases the lease, unpublishes + * the candidate's nav state, and then calls [LeasedNavState.onUnregistered], even if + * unregistering throws. Does nothing once the candidate is unregistered. + */ + fun dispose() +} + +/** + * The nav state a [NavLeaseCoordinator] candidate publishes to the shared [IScope] while it holds + * the nav lease. + * + * Nav transactions and nav data each occupy a single, mutually-exclusive slot in the `IScope`, so + * only one candidate's nav state can be published at a time. Unpublishing nav state removes it from + * the scope but keeps it staged, so the candidate can publish it again if it regains the lease. + */ +internal interface LeasedNavState { + + /** Publishes staged nav state to [scope]. */ + fun publishTo(scope: IScope) + + /** Removes published nav state from [scope], keeping it staged so it can be republished. */ + fun unpublishFrom(scope: IScope) + + /** + * Called when the lease candidate is unregistered, after its nav state has been unpublished. + * + * The provided [scope] is the scope the candidate was unregistered against, or `null` if there + * was none (e.g., the SDK is disabled). + */ + fun onUnregistered(scope: IScope?) +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScope.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScope.kt deleted file mode 100644 index f67c0b9b32..0000000000 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScope.kt +++ /dev/null @@ -1,156 +0,0 @@ -package io.sentry.compose.navigation3.lifecycle - -import io.sentry.IScope -import io.sentry.ITransaction -import io.sentry.PropagationContext -import io.sentry.protocol.App -import io.sentry.protocol.Contexts - -/** - * An [INavScope] implementation that writes to and reads from [delegate] only when [owner] holds - * the nav lease; otherwise it falls back to [stagedNavData]. - */ -internal class NavScope( - private val delegate: IScope, - private val stagedNavData: StagedNavData, - private val coordinator: NavLeaseCoordinator, - private val owner: Any, -) : INavScope { - - private val hasLease: Boolean - get() = coordinator.hasLease(owner) - - override var screen: String? - get() = if (hasLease) delegate.screen else stagedNavData.screen - set(value) { - stagedNavData.stageScreen(value) - if (hasLease) { - stagedNavData.publishScreen(delegate) - } - } - - override fun setNavigationContext(value: Any?) { - stagedNavData.stageNavigationContext(value) - if (hasLease) { - stagedNavData.publishNavigationContext(delegate) - } - } - - /** - * 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`. - * - * Called once per navigation that changes the top of the back stack, so the cost is bounded. - * Don't add call sites without reading [INavScope.app] first. - */ - override val app: App? - get() = Contexts(delegate.contexts).app - - /** - * The transaction bound to the scope. Without the lease, another owner's nav transaction isn't - * reported, so [owner]'s attempt to start a nav transaction reaches [NavScopes.startTransaction], - * which declines it and defers the propagation context rotation it implies. Non-nav transactions - * (e.g., the host's) are always reported, so they still block nav transactions as usual. - */ - override val transaction: ITransaction? - get() = - delegate.transaction?.takeUnless { current -> - !hasLease && coordinator.isNavTransaction(current) - } - - override fun bindTransaction(transaction: ITransaction) { - if (!hasLease) { - return - } - delegate.withTransaction { current -> - if (current == null) { - delegate.transaction = transaction - } - } - } - - override fun unbindTransaction(transaction: ITransaction) { - // Coordinator is called before clearTransaction() because the latter notifies the host app's - // scope observer(s), which could throw. Clearing the transaction first would cause the - // coordinator to retain a stale transaction. - coordinator.releaseTransaction(owner, transaction) - delegate.withTransaction { current -> - if (current === transaction) { - delegate.clearTransaction() - } - } - } - - override fun clearFinishedTransaction() { - if (!hasLease) { - return - } - delegate.withTransaction { current -> - if (current?.isFinished == true) { - delegate.clearTransaction() - } - } - } - - override fun rotatePropagationContext() { - if (hasLease) { - delegate.withPropagationContext { delegate.propagationContext = PropagationContext() } - } else { - stagedNavData.deferPropagationContextRotation() - } - } -} - -/** - * The subset of the [IScope] interface used to record navigation data. - * - * Protects against accidental misuse of what would otherwise be a partially-implemented `IScope` - * interface. New operations against the underlying `IScope` instance must be added here. Operations - * not directly delegable to `IScope` should ***not*** be added. - */ -internal 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. See [NavScope.app]. - */ - val app: App? - - /** - * The transaction bound to the scope, if any, as far as starting a nav transaction is concerned: - * see [NavScope.transaction]. - */ - val transaction: ITransaction? - - /** Binds [transaction] to the scope if no transaction is bound to it. */ - fun bindTransaction(transaction: ITransaction) - - /** - * Unbinds [transaction] from the scope if it's the bound transaction, and ensures it won't be - * bound to the scope again. - */ - fun unbindTransaction(transaction: ITransaction) - - /** Unbinds the scope's transaction if it has finished. */ - fun clearFinishedTransaction() - - /** - * Sets or, given `null`, clears the value stored under [NAVIGATION_CONTEXT_KEY]. - * - * Implementations should clear via [IScope.setContexts] rather than [IScope.removeContexts], as - * the latter doesn't notify scope observers and therefore doesn't persist the removal to disk. - */ - fun setNavigationContext(value: Any?) - - /** Replaces the scope's propagation context with a new one. */ - fun rotatePropagationContext() -} 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 92fdb80b52..323656c8df 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 @@ -1,23 +1,17 @@ package io.sentry.compose.navigation3.lifecycle import androidx.lifecycle.Lifecycle -import androidx.lifecycle.Lifecycle.Event.ON_DESTROY -import androidx.lifecycle.Lifecycle.Event.ON_PAUSE -import androidx.lifecycle.Lifecycle.Event.ON_RESUME -import androidx.lifecycle.Lifecycle.Event.ON_START -import androidx.lifecycle.Lifecycle.Event.ON_STOP -import androidx.lifecycle.LifecycleEventObserver -import androidx.lifecycle.LifecycleOwner import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.IScope import io.sentry.IScopes -import io.sentry.ITransaction -import io.sentry.NoOpTransaction +import io.sentry.PropagationContext import io.sentry.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" @@ -28,14 +22,14 @@ internal const val NAVIGATION_CONTEXT_KEY: String = "Navigation" * `SentryNavEffect` exists.) * * Needed because all `SentryNavEffect`s mutate a single [IScope] instance by default and will - * clobber or interleave one another's updates unless managed properly: + * clobber or interleave one another's updates unless properly managed: * ``` * LifecycleOwner (e.g., Activity or Fragment) * ↓ owns * SentryNavEffect * ↓ * CoordinatedBackStackObserver (wrapper) - * ├── BackStackObserver — computes nav data and nav transactions + * ├── BackStackObserver — computes nav data and requests nav transactions * └── NavScopes — controls if nav data + txns reach the real scope * ↓ * NavLeaseCoordinator — arbitrates ownership across all observers @@ -43,151 +37,18 @@ internal const val NAVIGATION_CONTEXT_KEY: String = "Navigation" * shared IScope * ``` * - * **Limitations** - * - * Multi-effect support is restricted to the scenarios documented in the - * [Navigation for Android docs](https://docs.sentry.io/platforms/android/integrations/navigation/#supported-setups). - * - * This class implements the policy that underlies those scenarios. It assumes that, at any moment, - * the user is interacting with at most one registered [LifecycleOwner] in the host app, and that - * the `LifecycleOwner` being interacted with is the one that was most recently resumed. - * - * Concretely, a host app can safely use more than one `SentryNavEffect` only if: - * - * 1. each `SentryNavEffect` is called in the same composable as the `NavDisplay` it observes; - * 2. no `SentryNavEffect` is called from inside a `NavDisplay`, e.g., from a nav entry, scene, or - * entry decorator†; - * 3. each Activity or Fragment calls at most one `SentryNavEffect` from its own content (sibling - * Fragments may each have their own effect, but not alongside one in the Activity hosting them); - * and - * 4. the user can interact with only one of those Activities or Fragments at a time. - * - * † Meant to ensure each effect's `LocalLifecycleOwner` is an Activity or a Fragment and not a - * `NavDisplay`-provided owner. `NavDisplay` provides its own `LocalLifecycleOwner` to the entries - * and scenes it renders. That owner drops to STARTED state during transitions and while overlays - * are shown. An effect called inside a `NavDisplay` would therefore (wrongly) gain and lose the nav - * lease this class manages as navigation animates, rather than as its Activity or Fragment comes - * and goes. - * - * **A fundamental tradeoff** - * - * During `LifecycleOwner` overlap, the nav transaction and scoped nav data can reflect - * **_either_**: - * - * 1. the outgoing owner that's still visible, or - * 2. the incoming owner that may crash before it becomes visible, - * - * but not both. This class opts for (2) so that the incoming destination populates crash context - * and any work performed is associated with it† – the idea being that start-up logic is more likely - * to cause a crash or produce interesting spans than an outgoing destination's steady state. - * - * † Associating work with the incoming transaction means spans produced by the outgoing owner may - * appear as if they were produced by the incoming owner (or vice versa, were our policy reversed). - * A possible solution would be to implement a parent span delivery system similar to - * `LocalSentrySpan`, but that's a task for another day. - * - * **Only leaseholders publish nav state or start nav transactions** + * **Only leaseholders start nav transactions or publish nav data** * * This class works by only allowing `NavScopes` instances holding a lease to start nav transactions - * and publish their navigation data to the shared `IScope`. - * - * `NavScopes` instances without a lease write to their own copy of [stagedNavData], which lets them - * preserve their latest updates without overwriting another instance's published data. When an - * instance acquires the lease, it publishes its staged data to the shared `IScope`. - * - * **Lease handoff policy** + * and publish their navigation data to the shared `IScope`. (The lease policy is implemented by + * [NavLeaseCoordinator].) * - * The lease is acquired preemptively by an incoming owner's `NavScopes` instance when it applies - * its first [back stack update][onBackStackUpdate].† - * - * A displaced owner 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 owner leaves or pauses while the - * displaced owner is still visible. If restored, the displaced owner's nav data and unfinished - * transaction are reinstated. - * - * † There are some exceptions, e.g., the leaseholder is resumed and the incoming owner isn't (e.g., - * a preloaded ViewPager2 Fragment page next to the current one). Such an owner acquires the lease - * when it resumes instead. - * - * The following diagram illustrates how handoff works: - * ``` - * Time - * │ - * ▼ - * - * SentryNavEffect A Shared state SentryNavEffect B - * ───────────────── ──────────── ───────────────── - * - * attach() Navigation lease: none - * │ - * Initial back stack update - * │ - * ├── acquire lease ────────► Navigation lease: A - * ├── create & bind txA ────► Scope transaction: txA - * ├── publish nav data ─────► Published data: A - * │ - * ON_RESUME A is eligible for restoration - * │ - * ON_PAUSE - * │ - * ├────────────────────────► Navigation lease: A (no other resumed owner) - * │ A remains eligible for restoration - * │ - * │ attach() - * │ │ - * │ Navigation lease: A ◄────────────────────┤ - * │ Scope transaction: txA │ - * │ Published data: A │ - * │ B registered, awaiting initial back │ - * │ stack update │ - * │ │ - * │ Initial back stack update - * │ │ - * │ Suspend & unbind txA ◄───────────────────┤ - * │ Navigation lease: B │ - * │ Published data: B │ - * │ Scope transaction: txB │ - * │ (rollback point) │ - * │ │ - * │ ON_RESUME - * │ │ - * │ B eligible for restoration ◄─────────────┤ - * │ │ - * ON_STOP │ - * │ │ - * ├── retain txA; timers continue │ - * ├────────────────────────► A no longer eligible for restoration │ - * │ │ - * ON_DESTROY │ - * │ │ - * ├── finish retained txA if unfinished │ - * └────────────────────────► A no longer tracked │ - * ON_PAUSE - * │ - * Navigation lease: B (no other resumed ◄─┤ - * owner) │ - * │ - * ON_STOP - * │ - * Navigation lease: B (no other resumed ◄─┤ - * owner); B keeps recording navigation │ - * B no longer eligible for restoration │ - * │ - * ON_DESTROY - * │ - * Finish txB if unfinished; ◄──────────────┤ - * unbind if still bound │ - * Clear B's remaining screen/back stack │ - * Retain recorded breadcrumbs │ - * Navigation lease: none │ - * ▼ - * - * Alternative at rollback point: B disposes before A stops - * Finish & unbind txB ◄────── dispose B ───┤ - * Navigation lease: A │ - * Restore txA if unfinished and scope free │ - * Republish A's staged data │ - * ``` + * `NavScopes` instances without a lease write to their own copy of [stagedNavData], and keep their + * unfinished nav transaction in [stagedNavTransaction], which lets them preserve their latest + * updates without overwriting another instance's transaction or published data. When an instance + * acquires the lease, it writes its staged data to the shared `IScope` and binds its unfinished + * transaction again (subject to any publishing conditions, e.g., the transaction stays unbound if + * another transaction is already bound to the shared `IScope`). * * **Usage** * @@ -210,166 +71,251 @@ internal const val NAVIGATION_CONTEXT_KEY: String = "Navigation" internal class NavScopes( private val delegate: IScopes, private val coordinator: NavLeaseCoordinator, -) : INavScopes, LifecycleEventObserver { +) : INavScopes { override val options: SentryOptions get() = delegate.options - private val owner: Any = Any() private var attachedLifecycle: Lifecycle? = null + + /** This instance's registration with the [coordinator], while it's attached. */ + private var registration: NavLeaseRegistration? = null + + private val stagedNavTransaction = StagedNavTransaction(delegate) private val stagedNavData = StagedNavData() + private val navStatePublisher = NavStatePublisher() + /** * Registers this [NavScopes] instance with [lifecycle]. No-ops if `lifecycle` has already been * attached or is [destroyed][Lifecycle.Event.ON_DESTROY]. * - * Attaching to a second, different lifecycle is unsupported; it's logged and ignored. + * Attaching to a second, different lifecycle no-ops. */ fun attach(lifecycle: Lifecycle) { val currentLifecycle = attachedLifecycle - if (currentLifecycle != null && currentLifecycle !== lifecycle) { - delegate.options.logger.log( - SentryLevel.WARNING, - "NavScopes is already attached to a different lifecycle; ignoring the new lifecycle.", - ) - return - } - if (currentLifecycle === lifecycle) { + if (currentLifecycle != null) { + if (currentLifecycle !== lifecycle) { + delegate.options.logger.log( + SentryLevel.WARNING, + "NavScopes is already attached to a different lifecycle; ignoring the new lifecycle.", + ) + } + + // No-op if already attached to a lifecycle instance (whether the same or a different one). return } + if (lifecycle.currentState == Lifecycle.State.DESTROYED) { return } attachedLifecycle = lifecycle - - coordinator.registerCandidate( - owner = owner, - lifecycleState = lifecycle.currentState, - onPublishData = stagedNavData::publishTo, - onClearPublishedData = stagedNavData::clearPublishedStateFrom, - ) - - lifecycle.addObserver(this) - } - - override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) { - when (event) { - ON_START -> coordinator.startCandidate(owner) - ON_RESUME -> withScopeOrNull { coordinator.resumeCandidate(owner, it) } - ON_PAUSE -> withScopeOrNull { coordinator.pauseCandidate(owner, it) } - ON_STOP -> withScopeOrNull { coordinator.stopCandidate(owner, it) } - ON_DESTROY -> dispose() - else -> Unit - } + registration = coordinator.register(lifecycle, delegate, navStatePublisher) } /** * Announces that a back stack update is about to be applied, so this instance can acquire the nav - * lease if it's eligible to. + * lease if it's eligible. * - * Must be called before each update. If [configureScope][IScopes.configureScope] skips its - * callback (e.g., the SDK is disabled), the update itself can't be applied either, so nothing is - * recorded and the next update applied is treated as this instance's first. + * Must be called before each update. */ fun onBackStackUpdate() { - delegate.configureScope { scope -> coordinator.onBackStackUpdate(owner, scope) } + registration?.onBackStackUpdate() } override fun configureScope(callback: (INavScope) -> Unit) { - delegate.configureScope { scope -> - callback(NavScope(scope, stagedNavData, coordinator, owner)) - } - } - - /** - * Starts a nav transaction, provided this instance holds the nav lease. - * - * Without the lease, returns a no-op transaction and defers the propagation context rotation a - * new transaction would have implied until the lease is acquired. A transaction started without - * the lease couldn't be bound to the scope, so its destination's spans would attach to the - * leaseholder's bound transaction instead, and the empty transaction would be dropped. - * - * The [coordinator] retains the leaseholder's transaction, so it can be restored if this instance - * loses the lease and regains it while the transaction is still unfinished. - */ - override fun startTransaction( - transactionContext: TransactionContext, - transactionOptions: TransactionOptions, - ): ITransaction { - if (!coordinator.hasLease(owner)) { - stagedNavData.deferPropagationContextRotation() - return NoOpTransaction.getInstance() - } - - return delegate.startTransaction(transactionContext, transactionOptions).also { transaction -> - if (!transaction.isNoOp) { - coordinator.retainTransaction(owner, transaction) - } - } + delegate.configureScope { scope -> callback(NavScope(scope)) } } - /** Records [breadcrumb] immediately; breadcrumbs only append, so they aren't lease-protected. */ override fun addBreadcrumb(breadcrumb: Breadcrumb, hint: Hint) { delegate.addBreadcrumb(breadcrumb, hint) } /** - * Releases the lease, clears remaining published state, and finishes this instance's transaction. - * - * Also stops observing the attached [LifecycleOwner] and cleans up memory references. + * Releases the lease (if held), unpublishes this instance's nav state, and finishes its nav + * transaction. */ fun dispose() { - attachedLifecycle?.removeObserver(this) + registration?.dispose() + stagedNavTransaction.forget() attachedLifecycle = null + registration = null + } - // Clear nav state, unbind the nav transaction, and avoid leaking the LifecycleOwner. - withScopeOrNull { coordinator.unregisterCandidate(owner, it) } + /** + * Publishes this instance's [stagedNavTransaction] and [stagedNavData] to the shared [IScope] + * while it holds the nav lease, as directed by the [coordinator]. + */ + private inner class NavStatePublisher : LeasedNavState { + + override fun publishTo(scope: IScope) { + stagedNavData.publishTo(scope) + stagedNavTransaction.publishTo(scope) + } + + override fun unpublishFrom(scope: IScope) { + stagedNavTransaction.unpublishFrom(scope) + stagedNavData.clearPublishedStateFrom(scope) + } + + override fun onUnregistered(scope: IScope?) { + // Unregistering on destroy detaches this instance, as dispose() does. + attachedLifecycle = null + registration = null + + if (scope != null) { + stagedNavTransaction.finish(scope) + } else { + // Without a scope (e.g., the SDK is disabled), the transaction can't be sent and will time + // out anyway. Not finishing it immediately lets us ensure the error-handling policy from + // IScopes.configureScope covers all SDK work. + stagedNavTransaction.forget() + } + } } /** - * Runs [block] with the real scope, or with `null` if [configureScope][IScopes.configureScope] - * skips its callback (e.g., the SDK is disabled), so coordinator bookkeeping still happens. - * - * A callback that ran but threw is not retried, as retrying could let its exception escape into - * the host app. + * An [INavScope] that maintains a [stagedNavTransaction] and [stagedNavData], and only writes to + * or reads from the [delegate IScope][IScope] when this instance holds the nav lease. */ - private fun withScopeOrNull(block: (IScope?) -> Unit) { - var callbackInvoked = false - delegate.configureScope { scope -> - callbackInvoked = true - block(scope) + private inner class NavScope(private val delegate: IScope) : INavScope { + + override var screen: String? + get() = if (hasLease) delegate.screen else stagedNavData.screen + set(value) { + stagedNavData.stageScreen(value) + if (hasLease) { + stagedNavData.publishScreen(delegate) + } + } + + /** + * 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 + + override fun setNavigationContext(value: Any?) { + stagedNavData.stageNavigationContext(value) + if (hasLease) { + stagedNavData.publishNavigationContext(delegate) + } + } + + override fun rotatePropagationContext() { + if (hasLease) { + delegate.withPropagationContext { delegate.propagationContext = PropagationContext() } + } else { + stagedNavData.deferPropagationContextRotation() + } + } + + /** + * Starts and binds a nav transaction with the nav lease (see [StagedNavTransaction]). Callers + * must [finish][finishNavTransaction] the previous nav transaction first. + * + * Without the lease, starts none and defers the propagation context rotation a new transaction + * would have implied until the lease is acquired. A transaction started without the lease + * couldn't be bound to the scope, so its destination's spans would attach to the leaseholder's + * bound transaction instead, and the empty transaction would be dropped. + */ + override fun startNavTransaction( + transactionContext: TransactionContext, + transactionOptions: TransactionOptions, + ): INavTransaction? { + return if (hasLease) { + stagedNavTransaction.start(delegate, transactionContext, transactionOptions) + } else { + stagedNavData.deferPropagationContextRotation() + null + } } - if (!callbackInvoked) { - block(null) + override fun finishNavTransaction() { + stagedNavTransaction.finish(delegate) } } } /** - * The subset of the [IScopes] interface used to record navigation data. + * The subset of the [IScopes] interface used to record nav state. * * Protects against accidental misuse of what would otherwise be a partially-implemented `IScopes` * interface. New operations against the underlying `IScopes` instance must be added here. * Operations not directly delegable to `IScopes` should ***not*** be added. */ -internal interface INavScopes { +internal sealed interface INavScopes { val options: SentryOptions - /** Runs [callback] with the current [INavScope], if the SDK provides a scope. */ + /** + * Runs [callback] with the current [INavScope]. + * + * No-ops if the SDK doesn't provide a scope (e.g., the SDK is disabled). + */ fun configureScope(callback: (INavScope) -> Unit) + fun addBreadcrumb(breadcrumb: Breadcrumb, hint: Hint) +} + +/** + * The scope operations used to record nav data and nav transactions. + * + * Protects against accidental misuse of what would otherwise be a partially-implemented `IScope` + * interface. New operations against the underlying `IScope` instance must be added here. Operations + * not directly delegable to `IScope` should ***not*** be added. The exception is the nav + * transaction lifecycle ([startNavTransaction] and [finishNavTransaction]), which depends on the + * nav lease and so has no `IScope` equivalent. + */ +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]. + * + * Implementations should clear via [IScope.setContexts] rather than [IScope.removeContexts], as + * the latter doesn't notify scope observers and therefore doesn't persist the removal to disk. + */ + fun setNavigationContext(value: Any?) + + /** Replaces the scope's propagation context with a new one. */ + fun rotatePropagationContext() + /** - * Starts a transaction, or returns a no-op transaction if none can be started (e.g., tracing is - * disabled, or the implementation declines to start one; see [NavScopes.startTransaction]). + * Starts a nav transaction and binds it to the scope, unless another unfinished transaction is + * already bound to it or the nav lease isn't held. Returns `null` if no transaction was started. + * + * (Note: Callers must first [finish][finishNavTransaction] the nav transaction most recently + * started, if any. Otherwise, it blocks the new one while it's bound.) */ - fun startTransaction( + fun startNavTransaction( transactionContext: TransactionContext, transactionOptions: TransactionOptions, - ): ITransaction + ): INavTransaction? - fun addBreadcrumb(breadcrumb: Breadcrumb, hint: Hint) + /** + * Finishes the nav transaction most recently started via [startNavTransaction] and unbinds it + * from the shared [IScope]. + */ + fun finishNavTransaction() } 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 new file mode 100644 index 0000000000..cfbfd05f8f --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransaction.kt @@ -0,0 +1,71 @@ +package io.sentry.compose.navigation3.lifecycle + +import io.sentry.ITransaction +import io.sentry.SpanStatus +import io.sentry.protocol.App + +/** + * An [INavTransaction] handle backed by a started [ITransaction]. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. Apply the thread safety policy from [NavScopes]. + */ +internal class NavTransaction +private constructor( + /** The backing transaction. */ + val transaction: ITransaction +) : INavTransaction { + + val name: String + get() = transaction.name + + override fun setData(key: String, value: Any) { + transaction.setData(key, value) + } + + override fun setContext(key: String, value: Any) { + transaction.setContext(key, value) + } + + override fun setApp(app: App) { + transaction.contexts.setApp(app) + } + + /** + * Finishes the backing transaction with its own status, or [OK][SpanStatus.OK] if it has none, + * and then passes it to [onFinished]. + */ + fun finish(onFinished: (ITransaction) -> Unit) { + if (!transaction.isFinished) { + transaction.finish(transaction.status ?: SpanStatus.OK) + } + onFinished(transaction) + } + + companion object { + /** A handle backed by the already-started [transaction]. */ + fun started(transaction: ITransaction): NavTransaction = NavTransaction(transaction) + } +} + +/** + * 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/README.md b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md index 7d697812ec..e003af9906 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 @@ -3,10 +3,32 @@ Package contains code for coordinating multiple `SentryNavEffect`s and their access to a shared `IScope`, so that overlapping effects (e.g., during Activity or Fragment transitions) don't clobber one another's screen names, nav context, or nav transactions. -See the `NavScopes` KDoc for the lease policy that decides which effect may write to the scope. +See the `NavLeaseCoordinator` KDoc for the lease policy that decides which effect may write to the scope. ## Encapsulation policy This package is meant to encapsulate knowledge about the possibility of multiple `SentryNavEffect`s and the shared mutable state issues it raises. Addressing those concerns is complicated. Confining them here makes them easier to reason about and easier to swap for alternative logic should we someday need to. + +## Limitations + +Support for multiple `SentryNavEffect`s is restricted to the scenarios documented in the +[Navigation for Android docs](https://docs.sentry.io/platforms/android/integrations/navigation/#supported-setups). + +`NavLeaseCoordinator` implements the policy that underlies those scenarios. It assumes that, at any moment, the user is interacting with at +most one registered `LifecycleOwner` in the host app, and that the `LifecycleOwner` being interacted with is the one that was most recently +resumed. + +Concretely, a host app can safely use more than one `SentryNavEffect` only if: + +1. each `SentryNavEffect` is called in the same composable as the `NavDisplay` it observes; +2. no `SentryNavEffect` is called from inside a `NavDisplay`, e.g., from a nav entry, scene, or entry decorator†; +3. each Activity or Fragment calls at most one `SentryNavEffect` from its own content (sibling Fragments may each have their own effect, but + not alongside one in the Activity hosting them); and +4. the user can interact with only one of those Activities or Fragments at a time. + +† Meant to ensure each effect's `LocalLifecycleOwner` is an Activity or a Fragment and not a `NavDisplay`-provided owner. `NavDisplay` +provides its own `LocalLifecycleOwner` to the entries and scenes it renders. That owner drops to STARTED state during transitions and while +overlays are shown. An effect called inside a `NavDisplay` would therefore (wrongly) gain and lose the nav lease `NavLeaseCoordinator` +manages as navigation animates, rather than as its Activity or Fragment comes and goes. diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavData.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavData.kt index a32cc74d50..affa1caff6 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavData.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavData.kt @@ -4,17 +4,22 @@ import io.sentry.IScope import io.sentry.PropagationContext /** - * Holds navigation state staged for publication. + * Holds nav data (i.e., the screen name and nav context) staged for publication to a shared + * [IScope]. * - * Only state that occupies a single slot in the shared [IScope] is staged. Nav events (i.e., - * breadcrumbs) only append to the scope, so they can't overwrite another instance's data and are - * recorded immediately, irrespective of lease ownership. + * Only state that occupies a single slot in the shared `IScope` is staged. Nav events (i.e., + * breadcrumbs) are excluded as they only append to the scope and are recorded immediately, + * irrespective of lease ownership. * * Propagation context isn't staged either, since an earlier propagation context mustn't overwrite * the scope's current one. Instead, a rotation requested without the lease, or implied by a nav * transaction that couldn't be started without it, is [deferred][deferPropagationContextRotation] * and performed once when the lease is acquired. As a result, any destination reached in the * meantime still gets a fresh trace. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. Apply the thread safety policy from [NavScopes]. */ internal class StagedNavData { @@ -57,8 +62,7 @@ internal class StagedNavData { * rotation. * * Only staged values are published, so values the host app or other integrations set (e.g., a - * screen name while screen tracking is off) survive. Values published by the previous leaseholder - * are cleared separately, via its [clearPublishedStateFrom]. + * screen name while screen tracking is off) survive. */ fun publishTo(realScope: IScope) { if (hasScreenUpdate) { 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 new file mode 100644 index 0000000000..793aad3914 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransaction.kt @@ -0,0 +1,143 @@ +package io.sentry.compose.navigation3.lifecycle + +import io.sentry.IScope +import io.sentry.IScopes +import io.sentry.ITransaction +import io.sentry.SentryLevel +import io.sentry.TransactionContext +import io.sentry.TransactionOptions + +/** + * Holds a nav transaction staged for publication to a shared [IScope]. + * + * 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. + * + * Starting or publishing a nav transaction never displaces an unfinished transaction bound to the + * scope by someone else (e.g., the host app). + * + * **Thread safety** + * + * This class is ***not*** thread-safe. Apply the thread safety policy from [NavScopes]. + */ +internal class StagedNavTransaction(private val scopes: IScopes) { + + /** The nav transaction most recently started, unless it has been finished since then. */ + private var navTransaction: NavTransaction? = 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. + * + * **Warning**: [Finish][finish] the current nav transaction before calling this method, as this + * method 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 + } + + /** + * Binds the nav transaction to [scope]. A transaction that has finished in the meantime is + * forgotten and unbound instead. + * + * If another unfinished transaction is bound to [scope], the transaction stays unbound, so it can + * be bound the next time it's published. + */ + fun publishTo(scope: IScope) { + val navTransaction = navTransaction ?: return + val started = navTransaction.transaction + if (started.isFinished) { + this.navTransaction = null + unbind(scope, started) + } else if (scope.isFreeFor(navTransaction.name, started)) { + bind(scope, started) + } + } + + /** + * Unbinds the nav transaction from [scope], if it's bound, without finishing it, so it can be + * [published][publishTo] again. + */ + fun unpublishFrom(scope: IScope) { + navTransaction?.let { unbind(scope, it.transaction) } + } + + /** Finishes the nav transaction and unbinds it from [scope], if it's bound. */ + fun finish(scope: IScope) { + val navTransaction = navTransaction ?: 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) }) + } + + /** + * Forgets the nav transaction without finishing it, e.g., when there's no scope to unbind it + * from. It will be left to time out. + */ + fun forget() { + navTransaction = null + } + + /** + * Reports whether the receiver's transaction slot can hold [transaction], or a new transaction + * given `null` (see [canHold]). + */ + 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 + } + + /** Binds [transaction] to [scope] if the scope's transaction slot can hold it (see [canHold]). */ + private fun bind(scope: IScope, transaction: ITransaction) { + scope.withTransaction { current -> + if (current !== transaction && canHold(current, transaction)) { + scope.transaction = transaction + } + } + } + + /** + * 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 -> + if (current === transaction) { + scope.clearTransaction() + } + } + } +} 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 08c2003fbb..c742d3dd5b 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 @@ -4,6 +4,7 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LifecycleRegistry import com.google.common.truth.Truth.assertThat +import io.sentry.IScope import io.sentry.IScopes import io.sentry.Scope import io.sentry.ScopeCallback @@ -11,6 +12,7 @@ import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.TransactionContext import io.sentry.TransactionOptions +import io.sentry.compose.navigation3.lifecycle.LeasedNavState import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator import org.junit.Test @@ -145,11 +147,11 @@ class CoordinatedBackStackObserverTest { /** Registers another owner that takes the nav lease and then pauses, keeping the lease. */ fun givenPausedLeaseholder() { - val other = Any() - coordinator.registerCandidate(other, Lifecycle.State.RESUMED, {}, {}) - coordinator.onBackStackUpdate(other, scope) - coordinator.pauseCandidate(other, scope) - check(coordinator.hasLease(other)) + val otherLifecycle = resumedLifecycle() + val other = checkNotNull(coordinator.register(otherLifecycle, scopes, NoOpLeasedNavState)) + other.onBackStackUpdate() + otherLifecycle.currentState = Lifecycle.State.STARTED + check(other.hasLease) } fun getSut(lifecycle: Lifecycle = resumedLifecycle()): CoordinatedBackStackObserver = @@ -164,11 +166,27 @@ class CoordinatedBackStackObserverTest { ) .apply { attach(lifecycle) } + /** + * Owners of the lifecycles created by [resumedLifecycle]. `LifecycleRegistry` holds its owner + * weakly and throws once it's collected, so the fixture keeps them alive for the test. + */ + private val lifecycleOwners = mutableListOf() + fun resumedLifecycle(): LifecycleRegistry = object : LifecycleOwner { override val lifecycle = LifecycleRegistry.createUnsafe(this).apply { currentState = Lifecycle.State.RESUMED } } + .also(lifecycleOwners::add) .lifecycle } + + /** Nav state for a competing owner that has nothing to publish. */ + private object NoOpLeasedNavState : LeasedNavState { + override fun publishTo(scope: IScope) = Unit + + override fun unpublishFrom(scope: IScope) = Unit + + override fun onUnregistered(scope: IScope?) = Unit + } } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt index 44986848b5..9a521312f6 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt @@ -1,868 +1,931 @@ package io.sentry.compose.navigation3.lifecycle import androidx.lifecycle.Lifecycle +import androidx.lifecycle.Lifecycle.State.CREATED +import androidx.lifecycle.Lifecycle.State.DESTROYED +import androidx.lifecycle.Lifecycle.State.RESUMED +import androidx.lifecycle.Lifecycle.State.STARTED +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry import com.google.common.truth.Truth.assertThat import io.sentry.IScope -import io.sentry.ITransaction +import io.sentry.IScopes import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions -import io.sentry.SpanStatus import org.junit.Assert.assertThrows import org.junit.Test import org.mockito.kotlin.any -import org.mockito.kotlin.doThrow +import org.mockito.kotlin.doAnswer 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 @Suppress("LargeClass") class NavLeaseCoordinatorTest { - private val owner = Any() - private val incoming = Any() - private val incumbent = Any() - private val previous = Any() - - private val transaction = mock() - private val unrelatedTransaction = mock() + // region registration @Test - fun `a registered created owner acquires the nav lease on its first back stack update`() { + fun `register() - returns no registration for a destroyed lifecycle and doesn't observe it`() { val fixture = Fixture.create() val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.register(owner, Lifecycle.State.CREATED) + val owner = TestOwner(DESTROYED) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + val registration = + navLeaseCoordinator.register(owner.lifecycle, fixture.scopes, fixture.navState("owner")) - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(owner) + assertThat(registration).isNull() + assertThat(owner.lifecycle.observerCount).isEqualTo(0) } @Test - fun `registering the same owner twice preserves its first publication callback`() { + fun `separate registrations for the same lifecycle don't share the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.register(owner, Lifecycle.State.CREATED) + val owner = TestOwner(RESUMED) + val first = fixture.acquireLease("first", owner) - navLeaseCoordinator.registerCandidate( - owner, - Lifecycle.State.RESUMED, - { fixture.published += "duplicate" }, - { fixture.cleared += "duplicate" }, - ) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + val second = fixture.acquireLease("second", owner) - assertThat(fixture.published).containsExactly(owner) + assertThat(first.hasLease).isFalse() + assertThat(second.hasLease).isTrue() } - @Test - fun `registering a destroyed owner does not make it eligible for the nav lease`() { - val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - - fixture.register(owner, Lifecycle.State.DESTROYED) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + // endregion - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() - assertThat(fixture.published).isEmpty() - } + // region acquiring the nav lease on the first back stack update @Test - fun `equal but distinct owners do not share the nav lease`() { + fun `a created owner acquires the nav lease on its first back stack update`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val registeredOwner = String(charArrayOf('a')) - val equalOwner = String(charArrayOf('a')) - fixture.acquireLease(registeredOwner) + val registration = fixture.register("owner", TestOwner(CREATED)) - val result = navLeaseCoordinator.hasLease(equalOwner) + registration.onBackStackUpdate() - assertThat(result).isFalse() - assertThat(navLeaseCoordinator.hasLease(registeredOwner)).isTrue() + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") } @Test fun `starting a created owner leaves it eligible to acquire the nav lease on its first back stack update`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.register(owner, Lifecycle.State.CREATED) + val owner = TestOwner(CREATED) + val registration = fixture.register("owner", owner) - navLeaseCoordinator.startCandidate(owner) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + owner.lifecycle.currentState = STARTED + registration.onBackStackUpdate() - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(registration.hasLease).isTrue() } @Test fun `restarting a stopped owner does not take the nav lease on its first back stack update`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.register(owner, Lifecycle.State.CREATED) - navLeaseCoordinator.stopCandidate(owner, fixture.scope) - fixture.acquireLease(incumbent) + val owner = TestOwner(STARTED) + val registration = fixture.register("owner", owner) + owner.lifecycle.currentState = CREATED + val incumbentOwner = TestOwner(RESUMED) + val incumbent = fixture.acquireLease("incumbent", incumbentOwner) + // The incumbent keeps the lease, but no longer blocks a first update from taking it. + incumbentOwner.lifecycle.currentState = STARTED - navLeaseCoordinator.startCandidate(owner) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + owner.lifecycle.currentState = STARTED + registration.onBackStackUpdate() - assertThat(navLeaseCoordinator.hasLease(incumbent)).isTrue() - assertThat(fixture.published).containsExactly(incumbent) + assertThat(incumbent.hasLease).isTrue() + assertThat(fixture.published).containsExactly("incumbent") } @Test fun `resuming an owner before its first back stack update leaves the nav lease unassigned`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.register(owner, Lifecycle.State.CREATED) + val owner = TestOwner(CREATED) + val registration = fixture.register("owner", owner) - navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + owner.lifecycle.currentState = RESUMED - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(registration.hasLease).isFalse() assertThat(fixture.published).isEmpty() } @Test fun `resuming a stopped owner after its first back stack update transfers the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(incumbent) - fixture.register(owner, Lifecycle.State.CREATED) - navLeaseCoordinator.stopCandidate(owner, fixture.scope) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + fixture.acquireLease("incumbent", TestOwner(RESUMED)) + val owner = TestOwner(STARTED) + val registration = fixture.register("owner", owner) + owner.lifecycle.currentState = CREATED + registration.onBackStackUpdate() - navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + owner.lifecycle.currentState = RESUMED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(incumbent, owner).inOrder() + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("incumbent", "owner").inOrder() } @Test fun `a paused owner cannot acquire the nav lease on its first back stack update`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.register(owner, Lifecycle.State.RESUMED) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + val owner = TestOwner(RESUMED) + val registration = fixture.register("owner", owner) + owner.lifecycle.currentState = STARTED - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + registration.onBackStackUpdate() - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(registration.hasLease).isFalse() assertThat(fixture.published).isEmpty() } @Test fun `an owner that isn't resumed doesn't take the nav lease from a resumed leaseholder`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(incumbent) + val incumbent = fixture.acquireLease("incumbent", TestOwner(RESUMED)) - fixture.acquireLease(owner, state = Lifecycle.State.STARTED) + fixture.acquireLease("owner", TestOwner(STARTED)) - assertThat(navLeaseCoordinator.hasLease(incumbent)).isTrue() - assertThat(fixture.published).containsExactly(incumbent) + assertThat(incumbent.hasLease).isTrue() + assertThat(fixture.published).containsExactly("incumbent") } @Test fun `an owner kept from the nav lease by a resumed leaseholder acquires it when it resumes`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(incumbent) - fixture.acquireLease(owner, state = Lifecycle.State.STARTED) + fixture.acquireLease("incumbent", TestOwner(RESUMED)) + val owner = TestOwner(STARTED) + val registration = fixture.acquireLease("owner", owner) + + owner.lifecycle.currentState = RESUMED + + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("incumbent", "owner").inOrder() + } + + /** + * `NavScopes` relies on this: it starts a nav transaction deferred without the lease when the + * lease is acquired, which must not coincide with a back stack update that starts a transaction + * of its own. + */ + @Test + fun `an owner kept from the nav lease on its first back stack update doesn't take it on a later one`() { + val fixture = Fixture.create() + val incumbentOwner = TestOwner(RESUMED) + val incumbent = fixture.acquireLease("incumbent", incumbentOwner) + val registration = fixture.acquireLease("owner", TestOwner(STARTED)) + // The incumbent keeps the lease, but no longer blocks a first update from taking it. + incumbentOwner.lifecycle.currentState = STARTED - navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + registration.onBackStackUpdate() - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(incumbent, owner).inOrder() + assertThat(incumbent.hasLease).isTrue() + assertThat(fixture.published).containsExactly("incumbent") } @Test fun `an owner that isn't resumed takes the nav lease from a paused leaseholder`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(incumbent) - navLeaseCoordinator.pauseCandidate(incumbent, fixture.scope) + val incumbentOwner = TestOwner(RESUMED) + fixture.acquireLease("incumbent", incumbentOwner) + incumbentOwner.lifecycle.currentState = STARTED - fixture.acquireLease(owner, state = Lifecycle.State.CREATED) + val registration = fixture.acquireLease("owner", TestOwner(CREATED)) - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(registration.hasLease).isTrue() } @Test fun `an owner that isn't resumed takes the nav lease from a stopped leaseholder`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(incumbent) - navLeaseCoordinator.pauseCandidate(incumbent, fixture.scope) - navLeaseCoordinator.stopCandidate(incumbent, fixture.scope) + val incumbentOwner = TestOwner(RESUMED) + fixture.acquireLease("incumbent", incumbentOwner) + incumbentOwner.lifecycle.currentState = CREATED - fixture.acquireLease(owner, state = Lifecycle.State.CREATED) + val registration = fixture.acquireLease("owner", TestOwner(CREATED)) - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(incumbent, owner).inOrder() + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("incumbent", "owner").inOrder() } @Test fun `a resumed owner takes the nav lease from a resumed leaseholder`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(incumbent) + fixture.acquireLease("incumbent", TestOwner(RESUMED)) - fixture.acquireLease(owner) + val registration = fixture.acquireLease("owner", TestOwner(RESUMED)) - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(registration.hasLease).isTrue() } @Test - fun `acquiring the nav lease clears the previous owner's published nav data before publishing`() { + fun `acquiring the nav lease unpublishes the previous publisher's nav state before publishing its own`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator val calls = mutableListOf() - navLeaseCoordinator.registerCandidate( - previous, - Lifecycle.State.RESUMED, - onPublishData = { calls += "publish previous" }, - onClearPublishedData = { calls += "clear previous" }, - ) - navLeaseCoordinator.onBackStackUpdate(previous, fixture.scope) - navLeaseCoordinator.registerCandidate( - incoming, - Lifecycle.State.RESUMED, - onPublishData = { calls += "publish incoming" }, - onClearPublishedData = { calls += "clear incoming" }, + fixture.acquireLease( + "previous", + TestOwner(RESUMED), + navState = + fixture.navState( + "previous", + onPublishTo = { calls += "publish previous" }, + onUnpublishFrom = { calls += "unpublish previous" }, + ), ) + val incoming = + fixture.register( + "incoming", + TestOwner(RESUMED), + fixture.navState( + "incoming", + onPublishTo = { calls += "publish incoming" }, + onUnpublishFrom = { calls += "unpublish incoming" }, + ), + ) - navLeaseCoordinator.onBackStackUpdate(incoming, fixture.scope) + incoming.onBackStackUpdate() assertThat(calls) - .containsExactly("publish previous", "clear previous", "publish incoming") + .containsExactly("publish previous", "unpublish previous", "publish incoming") .inOrder() } @Test - fun `regaining the nav lease doesn't clear the owner's own published nav data`() { + fun `regaining the nav lease doesn't unpublish the owner's own published nav state`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, null) - fixture.cleared.clear() + fixture.acquireLease("previous", TestOwner(RESUMED)) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + fixture.withoutScope { owner.lifecycle.currentState = STARTED } + fixture.unpublished.clear() - navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + owner.lifecycle.currentState = RESUMED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(previous, owner, owner).inOrder() - assertThat(fixture.cleared).isEmpty() + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("previous", "owner", "owner").inOrder() + assertThat(fixture.unpublished).isEmpty() } @Test - fun `releasing the nav lease leaves the owner's published nav data in place`() { + fun `releasing the nav lease leaves the owner's published nav state in place`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - fixture.acquireLease(owner) - fixture.cleared.clear() + fixture.acquireLease("previous", TestOwner(RESUMED)) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + fixture.unpublished.clear() - navLeaseCoordinator.pauseCandidate(owner, null) + fixture.withoutScope { owner.lifecycle.currentState = STARTED } - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() - assertThat(fixture.cleared).isEmpty() + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).isEmpty() } /** - * The coordinator records the incoming owner as the publisher before calling into the scope, so a - * clear callback that throws mid-handoff doesn't strand the incoming owner's nav data: the next - * handoff still clears it. + * The coordinator records the incoming owner as the publisher before calling into the scope, so + * unpublishing that throws mid-handoff doesn't strand the incoming owner's nav state: the next + * handoff still unpublishes it. */ @Test - fun `if a clear attempt throws during a handoff, the next handoff clears the new leaseholder's nav data`() { + fun `if unpublishing throws during a handoff, the next handoff unpublishes the new leaseholder's nav state`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val failure = IllegalStateException("clear failed") - var failClear = true - val firstOwner = previous - val secondOwner = incoming - val thirdOwner = owner + val failure = IllegalStateException("unpublish failed") fixture.acquireLease( - firstOwner, - onClearPublishedData = { - if (failClear) { - failClear = false - throw failure - } - fixture.cleared += firstOwner - }, + "first", + TestOwner(RESUMED), + navState = fixture.navState("first", onUnpublishFrom = { throw failure }), ) - fixture.register(secondOwner, Lifecycle.State.RESUMED) + val second = fixture.register("second", TestOwner(RESUMED)) - val thrown = - assertThrows(IllegalStateException::class.java) { - navLeaseCoordinator.onBackStackUpdate(secondOwner, fixture.scope) - } + val thrown = assertThrows(IllegalStateException::class.java) { second.onBackStackUpdate() } assertThat(thrown).isSameInstanceAs(failure) - assertThat(navLeaseCoordinator.hasLease(secondOwner)).isTrue() + assertThat(second.hasLease).isTrue() - fixture.acquireLease(thirdOwner) + fixture.acquireLease("third", TestOwner(RESUMED)) - assertThat(fixture.cleared).containsExactly(secondOwner) + assertThat(fixture.unpublished).containsExactly("second") } @Test - fun `later back stack updates do not republish nav data or reclaim the nav lease`() { + fun `later back stack updates do not republish nav state or reclaim the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val first = Any() - val second = Any() - fixture.acquireLease(first) - fixture.acquireLease(second, state = Lifecycle.State.RESUMED) + val first = fixture.acquireLease("first", TestOwner(RESUMED)) + val second = fixture.acquireLease("second", TestOwner(RESUMED)) + + first.onBackStackUpdate() + + assertThat(second.hasLease).isTrue() + assertThat(fixture.published).containsExactly("first", "second").inOrder() + } + + @Test + fun `a back stack update skipped by configureScope doesn't acquire the nav lease`() { + val fixture = Fixture.create() + val registration = fixture.register("owner", TestOwner(CREATED)) + + fixture.withoutScope { registration.onBackStackUpdate() } + + assertThat(registration.hasLease).isFalse() + assertThat(fixture.published).isEmpty() + } + + @Test + fun `a back stack update skipped by configureScope doesn't count as the first back stack update`() { + val fixture = Fixture.create() + val registration = fixture.register("owner", TestOwner(CREATED)) + fixture.withoutScope { registration.onBackStackUpdate() } - navLeaseCoordinator.onBackStackUpdate(first, fixture.scope) + registration.onBackStackUpdate() - assertThat(navLeaseCoordinator.hasLease(second)).isTrue() - assertThat(fixture.published).containsExactly(first, second).inOrder() + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") } + // endregion + + // region pausing + @Test fun `pausing the leaseholder hands the nav lease to the most recently resumed owner that's still resumed`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val older = Any() - val recent = Any() - fixture.acquireLease(older) - fixture.acquireLease(recent) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + fixture.acquireLease("older", TestOwner(RESUMED)) + val recent = fixture.acquireLease("recent", TestOwner(RESUMED)) + val incomingOwner = TestOwner(RESUMED) + fixture.acquireLease("incoming", incomingOwner) - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + incomingOwner.lifecycle.currentState = STARTED - assertThat(navLeaseCoordinator.hasLease(recent)).isTrue() - assertThat(fixture.published.last()).isSameInstanceAs(recent) + assertThat(recent.hasLease).isTrue() + assertThat(fixture.published.last()).isEqualTo("recent") } @Test fun `pausing the leaseholder keeps the nav lease when other eligible owners are only paused`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) - fixture.acquireLease(incoming) + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = STARTED + val incomingOwner = TestOwner(RESUMED) + val incoming = fixture.acquireLease("incoming", incomingOwner) - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + incomingOwner.lifecycle.currentState = STARTED - assertThat(navLeaseCoordinator.hasLease(incoming)).isTrue() - assertThat(fixture.published).containsExactly(owner, incoming).inOrder() + assertThat(incoming.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner", "incoming").inOrder() } @Test fun `a displaced paused owner regains the nav lease when it resumes`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) - fixture.acquireLease(incoming) - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = STARTED + val incomingOwner = TestOwner(RESUMED) + fixture.acquireLease("incoming", incomingOwner) + incomingOwner.lifecycle.currentState = STARTED - navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + owner.lifecycle.currentState = RESUMED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(owner, incoming, owner).inOrder() + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner", "incoming", "owner").inOrder() } @Test fun `pausing the leaseholder skips a resumed owner without a back stack update`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val eligible = Any() - val uninitialized = Any() - fixture.acquireLease(eligible) - fixture.register(uninitialized, Lifecycle.State.RESUMED) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + val eligible = fixture.acquireLease("eligible", TestOwner(RESUMED)) + val uninitialized = fixture.register("uninitialized", TestOwner(RESUMED)) + val incomingOwner = TestOwner(RESUMED) + fixture.acquireLease("incoming", incomingOwner) - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + incomingOwner.lifecycle.currentState = STARTED - assertThat(navLeaseCoordinator.hasLease(eligible)).isTrue() - assertThat(navLeaseCoordinator.hasLease(uninitialized)).isFalse() + assertThat(eligible.hasLease).isTrue() + assertThat(uninitialized.hasLease).isFalse() } @Test fun `pausing the only eligible owner keeps the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + owner.lifecycle.currentState = STARTED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(owner) + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") } @Test fun `pausing the only eligible owner without a scope keeps the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) - navLeaseCoordinator.pauseCandidate(owner, null) + fixture.withoutScope { owner.lifecycle.currentState = STARTED } - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(registration.hasLease).isTrue() } @Test - fun `resuming a paused leaseholder does not republish its nav data`() { + fun `resuming a paused leaseholder does not republish its nav state`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = STARTED - navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + owner.lifecycle.currentState = RESUMED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(owner) + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") } + // endregion + + // region stopping + + /** + * A real lifecycle pauses before it stops, so the leaseholder here is an owner that acquired the + * lease without ever resuming, and the other owner resumes without a scope, which records the + * resume without restoring its lease. + */ @Test fun `stopping the leaseholder restores a resumed owner`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - fixture.acquireLease(owner, state = Lifecycle.State.RESUMED) + val previousOwner = TestOwner(RESUMED) + val previous = fixture.acquireLease("previous", previousOwner) + previousOwner.lifecycle.currentState = STARTED + val owner = TestOwner(STARTED) + fixture.acquireLease("owner", owner) + fixture.withoutScope { previousOwner.lifecycle.currentState = RESUMED } - navLeaseCoordinator.stopCandidate(owner, fixture.scope) + owner.lifecycle.currentState = CREATED - assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() - assertThat(fixture.published.last()).isSameInstanceAs(previous) + assertThat(previous.hasLease).isTrue() + assertThat(fixture.published.last()).isEqualTo("previous") } @Test fun `stopping the only eligible owner keeps the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = STARTED - navLeaseCoordinator.stopCandidate(owner, fixture.scope) + owner.lifecycle.currentState = CREATED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(owner) + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") } @Test fun `stopping the only eligible owner without a scope keeps the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, null) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + fixture.withoutScope { owner.lifecycle.currentState = STARTED } - navLeaseCoordinator.stopCandidate(owner, null) + fixture.withoutScope { owner.lifecycle.currentState = CREATED } - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(registration.hasLease).isTrue() } @Test fun `stopping the leaseholder doesn't hand the nav lease to a paused owner`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - navLeaseCoordinator.pauseCandidate(previous, fixture.scope) - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + val previousOwner = TestOwner(RESUMED) + val previous = fixture.acquireLease("previous", previousOwner) + previousOwner.lifecycle.currentState = STARTED + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = STARTED - navLeaseCoordinator.stopCandidate(owner, fixture.scope) + owner.lifecycle.currentState = CREATED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() - assertThat(fixture.published).containsExactly(previous, owner).inOrder() - assertThat(fixture.cleared).containsExactly(previous) + assertThat(registration.hasLease).isTrue() + assertThat(previous.hasLease).isFalse() + assertThat(fixture.published).containsExactly("previous", "owner").inOrder() + assertThat(fixture.unpublished).containsExactly("previous") } @Test fun `a paused owner passed over when the leaseholder stopped gets the nav lease when it resumes`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - navLeaseCoordinator.pauseCandidate(previous, fixture.scope) - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) - navLeaseCoordinator.stopCandidate(owner, fixture.scope) + val previousOwner = TestOwner(RESUMED) + val previous = fixture.acquireLease("previous", previousOwner) + previousOwner.lifecycle.currentState = STARTED + val owner = TestOwner(RESUMED) + fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = CREATED - navLeaseCoordinator.resumeCandidate(previous, fixture.scope) + previousOwner.lifecycle.currentState = RESUMED - assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() - assertThat(fixture.published).containsExactly(previous, owner, previous).inOrder() + assertThat(previous.hasLease).isTrue() + assertThat(fixture.published).containsExactly("previous", "owner", "previous").inOrder() } @Test fun `restarting a stopped leaseholder keeps the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) - navLeaseCoordinator.stopCandidate(owner, fixture.scope) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = CREATED - navLeaseCoordinator.startCandidate(owner) + owner.lifecycle.currentState = STARTED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.published).containsExactly(owner) + assertThat(registration.hasLease).isTrue() + assertThat(fixture.published).containsExactly("owner") } @Test fun `stopping an owner excludes it from later nav lease fallback`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - fixture.acquireLease(owner) - navLeaseCoordinator.pauseCandidate(previous, fixture.scope) - navLeaseCoordinator.stopCandidate(previous, fixture.scope) + val previousOwner = TestOwner(RESUMED) + val previous = fixture.acquireLease("previous", previousOwner) + val registration = fixture.acquireLease("owner", TestOwner(RESUMED)) + previousOwner.lifecycle.currentState = CREATED - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + registration.dispose() - assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() - assertThat(fixture.published).containsExactly(previous, owner).inOrder() + assertThat(previous.hasLease).isFalse() + assertThat(fixture.published).containsExactly("previous", "owner").inOrder() } + // endregion + + // region lifecycle events without a scope + @Test fun `resuming without a scope keeps the owner eligible for nav lease fallback`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.register(owner, Lifecycle.State.CREATED) - navLeaseCoordinator.resumeCandidate(owner, null) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) - fixture.acquireLease(incoming, state = Lifecycle.State.CREATED) + val owner = TestOwner(CREATED) + val registration = fixture.acquireLease("owner", owner) + val incomingOwner = TestOwner(RESUMED) + fixture.acquireLease("incoming", incomingOwner) + fixture.withoutScope { owner.lifecycle.currentState = RESUMED } - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + incomingOwner.lifecycle.currentState = STARTED - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(registration.hasLease).isTrue() } @Test fun `resuming without a scope does not transfer the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(incumbent) - fixture.register(owner, Lifecycle.State.CREATED) - navLeaseCoordinator.stopCandidate(owner, fixture.scope) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + val incumbent = fixture.acquireLease("incumbent", TestOwner(RESUMED)) + val owner = TestOwner(STARTED) + val registration = fixture.register("owner", owner) + owner.lifecycle.currentState = CREATED + registration.onBackStackUpdate() - navLeaseCoordinator.resumeCandidate(owner, null) + fixture.withoutScope { owner.lifecycle.currentState = RESUMED } - assertThat(navLeaseCoordinator.hasLease(incumbent)).isTrue() - assertThat(fixture.published).containsExactly(incumbent) + assertThat(incumbent.hasLease).isTrue() + assertThat(fixture.published).containsExactly("incumbent") } @Test fun `pausing the leaseholder without a scope releases the nav lease without publishing a fallback`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - fixture.acquireLease(owner, state = Lifecycle.State.RESUMED) + val previous = fixture.acquireLease("previous", TestOwner(RESUMED)) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) - navLeaseCoordinator.pauseCandidate(owner, null) + fixture.withoutScope { owner.lifecycle.currentState = STARTED } - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() - assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() - assertThat(fixture.published).containsExactly(previous, owner).inOrder() + assertThat(registration.hasLease).isFalse() + assertThat(previous.hasLease).isFalse() + assertThat(fixture.published).containsExactly("previous", "owner").inOrder() } + /** + * A real lifecycle pauses before it stops, so the leaseholder here is an owner that acquired the + * lease without ever resuming, and the other owner resumes without a scope, which records the + * resume without restoring its lease. + */ @Test fun `stopping the leaseholder without a scope releases the nav lease without publishing a fallback`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - fixture.acquireLease(owner, state = Lifecycle.State.RESUMED) + val previousOwner = TestOwner(RESUMED) + val previous = fixture.acquireLease("previous", previousOwner) + previousOwner.lifecycle.currentState = STARTED + val owner = TestOwner(STARTED) + val registration = fixture.acquireLease("owner", owner) + fixture.withoutScope { previousOwner.lifecycle.currentState = RESUMED } - navLeaseCoordinator.stopCandidate(owner, null) + fixture.withoutScope { owner.lifecycle.currentState = CREATED } - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() - assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() - assertThat(fixture.published).containsExactly(previous, owner).inOrder() + assertThat(registration.hasLease).isFalse() + assertThat(previous.hasLease).isFalse() + assertThat(fixture.published).containsExactly("previous", "owner").inOrder() } - @Test - fun `an unregistered owner's nav transaction is not retained`() { - val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val other = Any() - fixture.acquireLease(owner) - - navLeaseCoordinator.retainTransaction(other, transaction) - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + // endregion - verifyNoInteractions(transaction) - } + // region unregistering @Test - fun `a nav transaction recorded without the lease is ignored`() { + fun `disposing the leaseholder unpublishes its published nav state and releases the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + val registration = fixture.acquireLease("owner", TestOwner(RESUMED)) - navLeaseCoordinator.retainTransaction(owner, transaction) - navLeaseCoordinator.unregisterCandidate(incoming, fixture.scope) + registration.dispose() - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() - assertThat(fixture.scope.transaction).isNull() - - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) - - verifyNoInteractions(transaction) + assertThat(fixture.unpublished).containsExactly("owner") + assertThat(registration.hasLease).isFalse() } @Test - fun `an unfinished nav transaction is restored when its owner regains the lease`() { + fun `destroying an owner's lifecycle unregisters its candidate`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner, transaction = transaction) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) - assertThat(fixture.scope.transaction).isNull() - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + owner.lifecycle.currentState = DESTROYED - assertThat(fixture.scope.transaction).isSameInstanceAs(transaction) - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - verify(transaction, never()).finish(any()) + assertThat(registration.hasLease).isFalse() + assertThat(fixture.unpublished).containsExactly("owner") + assertThat(fixture.unregistered).containsExactly("owner" to fixture.scope) } @Test - fun `restoring a nav transaction preserves an unrelated scope transaction`() { + fun `disposing a former leaseholder preserves the current owner's published nav state`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner, transaction = transaction) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) - fixture.scope.transaction = unrelatedTransaction + val previous = fixture.acquireLease("previous", TestOwner(RESUMED)) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + owner.lifecycle.currentState = STARTED + fixture.unpublished.clear() - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + registration.dispose() - assertThat(fixture.scope.transaction).isSameInstanceAs(unrelatedTransaction) + assertThat(previous.hasLease).isTrue() + assertThat(fixture.unpublished).isEmpty() } @Test - fun `a finished nav transaction is discarded when its owner regains the lease`() { + fun `disposing an owner whose nav state is still published without the nav lease unpublishes it`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner, transaction = transaction) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) - whenever(transaction.isFinished).thenReturn(true) + val previous = fixture.acquireLease("previous", TestOwner(RESUMED)) + val owner = TestOwner(RESUMED) + val registration = fixture.acquireLease("owner", owner) + fixture.withoutScope { owner.lifecycle.currentState = STARTED } + fixture.published.clear() + fixture.unpublished.clear() - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + registration.dispose() - assertThat(fixture.scope.transaction).isNull() - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + assertThat(fixture.unpublished).containsExactly("owner") + assertThat(fixture.published).isEmpty() + assertThat(previous.hasLease).isFalse() } @Test - fun `a released unfinished nav transaction is not restored when its owner regains the lease`() { + fun `disposing the leaseholder hands the nav lease to the most recently resumed eligible owner`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner, transaction = transaction) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + val olderOwner = TestOwner(RESUMED) + fixture.acquireLease("older", olderOwner) + olderOwner.lifecycle.currentState = STARTED + val previousOwner = TestOwner(RESUMED) + val previous = fixture.acquireLease("previous", previousOwner) + previousOwner.lifecycle.currentState = STARTED + val registration = fixture.acquireLease("owner", TestOwner(RESUMED)) - navLeaseCoordinator.releaseTransaction(owner, transaction) - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + registration.dispose() - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.scope.transaction).isNull() - assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + assertThat(previous.hasLease).isTrue() + assertThat(fixture.published.last()).isEqualTo("previous") + assertThat(fixture.unpublished).containsExactly("older", "previous", "owner").inOrder() } @Test - fun `releasing a nav transaction other than the retained one keeps the retained one restorable`() { + fun `disposing the leaseholder without a scope releases the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner, transaction = transaction) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + val previous = fixture.acquireLease("previous", TestOwner(RESUMED)) + val registration = fixture.acquireLease("owner", TestOwner(RESUMED)) + fixture.unpublished.clear() - navLeaseCoordinator.releaseTransaction(owner, unrelatedTransaction) - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + fixture.withoutScope { registration.dispose() } - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(fixture.scope.transaction).isSameInstanceAs(transaction) + assertThat(registration.hasLease).isFalse() + assertThat(previous.hasLease).isFalse() + assertThat(fixture.published).containsExactly("previous", "owner").inOrder() + assertThat(fixture.unpublished).isEmpty() } @Test - fun `a displaced owner's retained nav transaction remains identifiable until unregistering`() { + fun `disposing the leaseholder forgets the owner when unpublishing its nav state throws`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner, transaction = transaction) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + val failure = IllegalStateException("unpublish failed") + val owner = TestOwner(RESUMED) + val registration = + fixture.acquireLease( + "owner", + owner, + navState = fixture.navState("owner", onUnpublishFrom = { throw failure }), + ) - assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isTrue() - assertThat(navLeaseCoordinator.isNavTransaction(unrelatedTransaction)).isFalse() + val thrown = assertThrows(IllegalStateException::class.java) { registration.dispose() } - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + assertThat(thrown).isSameInstanceAs(failure) + assertThat(registration.hasLease).isFalse() + + fixture.published.clear() + owner.lifecycle.currentState = STARTED + owner.lifecycle.currentState = RESUMED + fixture.acquireLease("incoming", TestOwner(RESUMED)) - assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + assertThat(fixture.published).containsExactly("incoming") + assertThat(fixture.unpublished).isEmpty() } @Test - fun `acquiring the nav lease binds its retained transaction before publishing nav data`() { + fun `disposing an owner before its first back stack update keeps it from acquiring the nav lease`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val transactionAtPublication = mutableListOf() - navLeaseCoordinator.registerCandidate( - owner, - Lifecycle.State.RESUMED, - onPublishData = { transactionAtPublication += it.transaction }, - onClearPublishedData = {}, - ) - navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) - navLeaseCoordinator.retainTransaction(owner, transaction) - fixture.scope.transaction = transaction - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + val registration = fixture.register("owner", TestOwner(RESUMED)) + registration.dispose() - navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + registration.onBackStackUpdate() - assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() - assertThat(transactionAtPublication).containsExactly(null, transaction).inOrder() + assertThat(registration.hasLease).isFalse() + assertThat(fixture.published).isEmpty() } @Test - fun `unregistering the leaseholder finishes its transaction and clears its published nav data`() { + fun `dispose() - notifies the nav state that it was unregistered after unpublishing it`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner, transaction = transaction) + val calls = mutableListOf() + val registration = + fixture.acquireLease( + "owner", + TestOwner(RESUMED), + navState = + fixture.navState( + "owner", + onUnpublishFrom = { calls += "unpublish" }, + onUnregistered = { calls += "unregistered" }, + ), + ) - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + registration.dispose() - verify(transaction).finish(SpanStatus.OK) - assertThat(fixture.scope.transaction).isNull() - assertThat(fixture.cleared).containsExactly(owner) - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(calls).containsExactly("unpublish", "unregistered").inOrder() } @Test - fun `unregistering a former leaseholder preserves the current owner's published nav data`() { + fun `dispose() - passes the scope to the nav state's onUnregistered`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - fixture.acquireLease(owner, state = Lifecycle.State.RESUMED) - navLeaseCoordinator.pauseCandidate(owner, fixture.scope) - fixture.cleared.clear() + val registration = fixture.register("owner", TestOwner(CREATED)) - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + registration.dispose() - assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() - assertThat(fixture.cleared).isEmpty() + assertThat(fixture.unregistered).containsExactly("owner" to fixture.scope) } @Test - fun `unregistering the leaseholder hands the nav lease to the most recently resumed eligible owner`() { + fun `dispose() - passes no scope to the nav state's onUnregistered when configureScope skips it`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val older = Any() - fixture.acquireLease(older) - navLeaseCoordinator.pauseCandidate(older, fixture.scope) - fixture.acquireLease(previous) - navLeaseCoordinator.pauseCandidate(previous, fixture.scope) - fixture.acquireLease(owner) + val registration = fixture.register("owner", TestOwner(CREATED)) - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + fixture.withoutScope { registration.dispose() } - assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() - assertThat(fixture.published.last()).isSameInstanceAs(previous) - assertThat(fixture.cleared).containsExactly(older, previous, owner).inOrder() + assertThat(fixture.unregistered).containsExactly("owner" to null) } @Test - fun `unregistering the leaseholder without a scope releases the nav lease`() { + fun `dispose() - notifies the nav state once with the scope when unpublishing it throws`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(previous) - fixture.acquireLease(owner) - fixture.cleared.clear() + val failure = IllegalStateException("unpublish failed") + val registration = + fixture.acquireLease( + "owner", + TestOwner(RESUMED), + navState = fixture.navState("owner", onUnpublishFrom = { throw failure }), + ) - navLeaseCoordinator.unregisterCandidate(owner, null) + val thrown = assertThrows(IllegalStateException::class.java) { registration.dispose() } - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() - assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() - assertThat(fixture.published).containsExactly(previous, owner).inOrder() - assertThat(fixture.cleared).isEmpty() + assertThat(thrown).isSameInstanceAs(failure) + assertThat(fixture.unregistered).containsExactly("owner" to fixture.scope) } @Test - fun `unregistering the leaseholder without a scope doesn't finish its nav transaction`() { + fun `dispose() - notifies the nav state only once when called again`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - fixture.acquireLease(owner, transaction = transaction) + val registration = fixture.register("owner", TestOwner(CREATED)) + registration.dispose() - navLeaseCoordinator.unregisterCandidate(owner, null) + registration.dispose() - verify(transaction, never()).finish(any()) + assertThat(fixture.unregistered).hasSize(1) } @Test - fun `unregistering a former leaseholder finishes its retained transaction without clearing an unrelated one`() { + fun `dispose() - notifies the nav state only once when its lifecycle is destroyed afterward`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - whenever(transaction.status).thenReturn(SpanStatus.CANCELLED) - fixture.acquireLease(owner, transaction = transaction) - fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) - fixture.scope.transaction = unrelatedTransaction - fixture.cleared.clear() + val owner = TestOwner(RESUMED) + val registration = fixture.register("owner", owner) + registration.dispose() - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + owner.lifecycle.currentState = DESTROYED - verify(transaction).finish(SpanStatus.CANCELLED) - assertThat(fixture.scope.transaction).isSameInstanceAs(unrelatedTransaction) - assertThat(fixture.cleared).isEmpty() + assertThat(fixture.unregistered).hasSize(1) } @Test - fun `unregistering the leaseholder forgets the owner when finishing its transaction throws`() { + fun `dispose() - stops observing the lifecycle`() { val fixture = Fixture.create() - val navLeaseCoordinator = fixture.navLeaseCoordinator - val failure = IllegalStateException("finish failed") - fixture.acquireLease(owner, transaction = transaction) - doThrow(failure).whenever(transaction).finish(SpanStatus.OK) + val owner = TestOwner(RESUMED) + val registration = fixture.register("owner", owner) - val thrown = - assertThrows(IllegalStateException::class.java) { - navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) - } + registration.dispose() - assertThat(thrown).isSameInstanceAs(failure) - assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() - assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + assertThat(owner.lifecycle.observerCount).isEqualTo(0) + } - fixture.published.clear() - navLeaseCoordinator.resumeCandidate(owner, fixture.scope) - fixture.acquireLease(incoming) + // endregion - assertThat(fixture.published).containsExactly(incoming) - assertThat(fixture.cleared).isEmpty() + /** + * A [LifecycleOwner] whose lifecycle starts in [initialState] and can be moved through states by + * setting [LifecycleRegistry.currentState], which dispatches the events in between. + */ + private class TestOwner(initialState: Lifecycle.State) : LifecycleOwner { + override val lifecycle: LifecycleRegistry = + LifecycleRegistry.createUnsafe(this).apply { + // A lifecycle must be created before it can be destroyed. + currentState = CREATED + currentState = initialState + } } - private class Fixture { + private class Fixture private constructor() { val navLeaseCoordinator = NavLeaseCoordinator() val scope = Scope(SentryOptions()) - val published = mutableListOf() - val cleared = mutableListOf() + /** Whether [scopes] runs `configureScope` callbacks; when false, it skips them. */ + private var providesScope = true + + val scopes = + mock().also { scopes -> + doAnswer { + if (providesScope) { + (it.arguments[0] as ScopeCallback).run(scope) + } + null + } + .whenever(scopes) + .configureScope(any()) + } + + /** Names of the nav states published, in publication order. */ + val published = mutableListOf() + + /** Names of the nav states unpublished, in the order they were unpublished. */ + val unpublished = mutableListOf() + + /** Names of the nav states notified of unregistration, with the scope they were given. */ + val unregistered = mutableListOf>() + + /** + * `LifecycleRegistry` holds its owner weakly and throws once it's collected, so the fixture + * keeps registered owners alive for the test. + */ + private val owners = mutableListOf() + + /** + * Nav state named [name] that, by default, records its publication in [published], its + * unpublishing in [unpublished], and its unregistration in [unregistered]. + */ + fun navState( + name: String, + onPublishTo: (IScope) -> Unit = { published += name }, + onUnpublishFrom: (IScope) -> Unit = { unpublished += name }, + onUnregistered: (IScope?) -> Unit = { unregistered += name to it }, + ): LeasedNavState = FakeLeasedNavState(onPublishTo, onUnpublishFrom, onUnregistered) + + /** Registers a candidate for [owner]'s lifecycle with the nav state named [name]. */ fun register( - owner: Any, - state: Lifecycle.State, - onClearPublishedData: (IScope) -> Unit = { cleared += owner }, - ) { - navLeaseCoordinator.registerCandidate( - owner, - state, - onPublishData = { published += owner }, - onClearPublishedData = onClearPublishedData, - ) + name: String, + owner: TestOwner, + navState: LeasedNavState = navState(name), + ): NavLeaseRegistration { + owners += owner + return checkNotNull(navLeaseCoordinator.register(owner.lifecycle, scopes, navState)) } + /** + * Registers a candidate for [owner]'s lifecycle and applies its first back stack update, which + * acquires the nav lease if the owner's lifecycle permits it. + */ fun acquireLease( - owner: Any, - state: Lifecycle.State = Lifecycle.State.RESUMED, - transaction: ITransaction? = null, - onClearPublishedData: (IScope) -> Unit = { cleared += owner }, - ) { - register(owner, state, onClearPublishedData) - navLeaseCoordinator.onBackStackUpdate(owner, scope) - if (transaction != null) { - navLeaseCoordinator.retainTransaction(owner, transaction) - scope.transaction = transaction + name: String, + owner: TestOwner, + navState: LeasedNavState = navState(name), + ): NavLeaseRegistration = register(name, owner, navState).also { it.onBackStackUpdate() } + + /** Runs [block] while `configureScope` skips its callbacks, e.g., as if the SDK is disabled. */ + fun withoutScope(block: () -> Unit) { + providesScope = false + try { + block() + } finally { + providesScope = true } } @@ -870,4 +933,16 @@ class NavLeaseCoordinatorTest { fun create(): Fixture = Fixture() } } + + private class FakeLeasedNavState( + private val onPublishTo: (IScope) -> Unit, + private val onUnpublishFrom: (IScope) -> Unit, + private val onUnregistered: (IScope?) -> Unit, + ) : LeasedNavState { + override fun publishTo(scope: IScope) = onPublishTo(scope) + + override fun unpublishFrom(scope: IScope) = onUnpublishFrom(scope) + + override fun onUnregistered(scope: IScope?) = onUnregistered.invoke(scope) + } } 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 be66fb5b16..a079c91a54 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 @@ -1,26 +1,42 @@ package io.sentry.compose.navigation3.lifecycle import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry import com.google.common.truth.Truth.assertThat +import io.sentry.IScopes import io.sentry.ITransaction import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.ScopeObserverAdapter 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 +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +/** + * Tests for the [INavScope] that [NavScopes.configureScope] hands out, which is only reachable + * through it. + */ class NavScopeTest { @Test fun `getScreen() - reads the real screen with the nav lease`() { val fixture = Fixture.create(ownsLease = true) - val navScope = fixture.navScope - navScope.screen = "Staged" - fixture.realScope.screen = "Host" + val navScopes = fixture.navScopes + navScopes.withNavScope { it.screen = "Staged" } + fixture.scope.screen = "Host" - val screen = navScope.screen + val screen = navScopes.withNavScope { it.screen } assertThat(screen).isEqualTo("Host") } @@ -28,11 +44,11 @@ class NavScopeTest { @Test fun `getScreen() - reads the staged screen without the nav lease`() { val fixture = Fixture.create() - val navScope = fixture.navScope - fixture.realScope.screen = "Host" - fixture.stagedNavData.stageScreen("Staged") + val navScopes = fixture.navScopes + fixture.scope.screen = "Host" + navScopes.withNavScope { it.screen = "Staged" } - val screen = navScope.screen + val screen = navScopes.withNavScope { it.screen } assertThat(screen).isEqualTo("Staged") } @@ -40,282 +56,388 @@ class NavScopeTest { @Test fun `setScreen() - publishes the screen with the nav lease`() { val fixture = Fixture.create(ownsLease = true) - val navScope = fixture.navScope + val navScopes = fixture.navScopes - navScope.screen = "Profile" + navScopes.withNavScope { it.screen = "Profile" } - assertThat(fixture.realScope.screen).isEqualTo("Profile") + assertThat(fixture.scope.screen).isEqualTo("Profile") } @Test - fun `setScreen() - stages the screen without the nav lease`() { + fun `setScreen() - stages the screen without the nav lease until the lease is acquired`() { val fixture = Fixture.create() - val navScope = fixture.navScope - fixture.realScope.screen = "Host" + val navScopes = fixture.navScopes + fixture.scope.screen = "Host" - navScope.screen = "Profile" + navScopes.withNavScope { it.screen = "Profile" } + assertThat(fixture.scope.screen).isEqualTo("Host") - assertThat(fixture.realScope.screen).isEqualTo("Host") - assertThat(navScope.screen).isEqualTo("Profile") + fixture.acquireLease() + 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) - val navScope = fixture.navScope - val navigationContext = mapOf("route" to "Profile") + val navScopes = fixture.navScopes + + navScopes.withNavScope { it.setNavigationContext(navigationContext) } + + assertThat(fixture.scope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) + } + + @Test + fun `setNavigationContext() - clears the published navigation context given null with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val navScopes = fixture.navScopes + navScopes.withNavScope { it.setNavigationContext(navigationContext) } - navScope.setNavigationContext(navigationContext) + navScopes.withNavScope { it.setNavigationContext(null) } - assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]) - .isSameInstanceAs(navigationContext) + assertThat(fixture.scope.contexts[NAVIGATION_CONTEXT_KEY]).isNull() } @Test - fun `setNavigationContext() - stages the navigation context without the nav lease`() { + fun `setNavigationContext() - stages the navigation context without the nav lease until the lease is acquired`() { val fixture = Fixture.create() - val navScope = fixture.navScope + val navScopes = fixture.navScopes val hostContext = mapOf("route" to "Host") - val navigationContext = mapOf("route" to "Profile") - fixture.realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + fixture.scope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) - navScope.setNavigationContext(navigationContext) + navScopes.withNavScope { it.setNavigationContext(navigationContext) } + assertThat(fixture.scope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) - assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) - fixture.stagedNavData.publishTo(fixture.realScope) - assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]) - .isSameInstanceAs(navigationContext) + fixture.acquireLease() + assertThat(fixture.scope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) } @Test - fun `rotatePropagationContext() - replaces the real context with the nav lease`() { + fun `rotatePropagationContext() - replaces the real propagation context with the nav lease`() { val fixture = Fixture.create(ownsLease = true) - val realContext = fixture.realScope.propagationContext + val navScopes = fixture.navScopes + val realContext = fixture.scope.propagationContext - fixture.navScope.rotatePropagationContext() + navScopes.withNavScope { it.rotatePropagationContext() } - assertThat(fixture.realScope.propagationContext).isNotSameInstanceAs(realContext) + assertThat(fixture.scope.propagationContext).isNotSameInstanceAs(realContext) } @Test - fun `rotatePropagationContext() - defers rotating the real context without the nav lease`() { + fun `rotatePropagationContext() - defers the rotation without the nav lease until the lease is acquired`() { val fixture = Fixture.create() - val realContext = fixture.realScope.propagationContext + val navScopes = fixture.navScopes + val realContext = fixture.scope.propagationContext - fixture.navScope.rotatePropagationContext() + navScopes.withNavScope { it.rotatePropagationContext() } + assertThat(fixture.scope.propagationContext).isSameInstanceAs(realContext) - assertThat(fixture.realScope.propagationContext).isSameInstanceAs(realContext) - fixture.stagedNavData.publishTo(fixture.realScope) - assertThat(fixture.realScope.propagationContext).isNotSameInstanceAs(realContext) + fixture.acquireLease() + assertThat(fixture.scope.propagationContext).isNotSameInstanceAs(realContext) } @Test - fun `bindTransaction() - binds to a free scope with the nav lease`() { + fun `startNavTransaction() - binds the nav transaction to a free scope with the nav lease`() { val fixture = Fixture.create(ownsLease = true) - val transaction = mock() + val navScopes = fixture.navScopes - fixture.navScope.bindTransaction(transaction) + val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(fixture.realScope.transaction).isSameInstanceAs(transaction) + assertThat(navTransaction).isNotNull() + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) } @Test - fun `bindTransaction() - preserves an already bound transaction`() { + fun `startNavTransaction() - starts none and keeps a bound host transaction with the nav lease`() { val fixture = Fixture.create(ownsLease = true) - val bound = mock() - fixture.realScope.transaction = bound + val navScopes = fixture.navScopes + val hostTransaction = mock() + fixture.scope.transaction = hostTransaction - fixture.navScope.bindTransaction(mock()) + val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(fixture.realScope.transaction).isSameInstanceAs(bound) + assertThat(navTransaction).isNull() + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) } @Test - fun `bindTransaction() - does nothing without the nav lease`() { + fun `startNavTransaction() - replaces a finished bound transaction with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val navScopes = fixture.navScopes + fixture.scope.transaction = + mock().also { whenever(it.isFinished).thenReturn(true) } + + val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } + + assertThat(navTransaction).isNotNull() + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) + } + + @Test + fun `startNavTransaction() - starts none without the nav lease`() { val fixture = Fixture.create() + val navScopes = fixture.navScopes - fixture.navScope.bindTransaction(mock()) + val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(fixture.realScope.transaction).isNull() + assertThat(navTransaction).isNull() + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isNull() } @Test - fun `unbindTransaction() - unbinds the given transaction without the nav lease`() { + fun `startNavTransaction() - starts none without the nav lease, even once the lease is acquired`() { val fixture = Fixture.create() - val transaction = mock() - fixture.realScope.transaction = transaction + val navScopes = fixture.navScopes + navScopes.withNavScope { it.startNavTransaction("/Profile") } - fixture.navScope.unbindTransaction(transaction) + fixture.acquireLease() - assertThat(fixture.realScope.transaction).isNull() + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isNull() } @Test - fun `unbindTransaction() - releases the owner's retained nav transaction with the nav lease`() { - val fixture = Fixture.create(ownsLease = true) - val transaction = mock() - fixture.retainTransaction(transaction) + fun `startNavTransaction() - starts none and keeps another owner's bound nav transaction without the nav lease`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes + val otherTransaction = fixture.startAnotherOwnersNavTransaction() - fixture.navScope.unbindTransaction(transaction) + val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Profile") } - assertThat(fixture.coordinator.isNavTransaction(transaction)).isFalse() + assertThat(navTransaction).isNull() + assertThat(fixture.startedTransactions).containsExactly(otherTransaction) + assertThat(fixture.scope.transaction).isSameInstanceAs(otherTransaction) } @Test - fun `unbindTransaction() - releases the owner's retained nav transaction without the nav lease`() { + fun `startNavTransaction() - starts none while its unfinished previous nav transaction is bound`() { val fixture = Fixture.create(ownsLease = true) - val transaction = mock() - fixture.retainTransaction(transaction) - fixture.giveLeaseToAnotherOwner() + val navScopes = fixture.navScopes + navScopes.withNavScope { it.startNavTransaction("/Profile") } + val previous = fixture.startedTransactions.single() - fixture.navScope.unbindTransaction(transaction) + val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Settings") } - assertThat(fixture.coordinator.isNavTransaction(transaction)).isFalse() + assertThat(navTransaction).isNull() + verify(previous, never()).finish(any()) + assertThat(fixture.startedTransactions).containsExactly(previous) + assertThat(fixture.scope.transaction).isSameInstanceAs(previous) } @Test - fun `unbindTransaction() - releases the retained nav transaction when a scope observer throws`() { + fun `startNavTransaction() - binds the new nav transaction once the previous one is finished with the nav lease`() { val fixture = Fixture.create(ownsLease = true) - val transaction = mock() - fixture.retainTransaction(transaction) - fixture.realScope.transaction = transaction - // Registered last, so that only clearTransaction() reaches it. - fixture.realScope.options.addScopeObserver( - object : ScopeObserverAdapter() { - override fun setTransaction(transaction: String?) { - throw ThrowingScopeObserverException() - } - } - ) + val navScopes = fixture.navScopes + navScopes.withNavScope { it.startNavTransaction("/Profile") } + navScopes.withNavScope { it.finishNavTransaction() } - assertThrows(ThrowingScopeObserverException::class.java) { - fixture.navScope.unbindTransaction(transaction) - } + val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Settings") } - assertThat(fixture.coordinator.isNavTransaction(transaction)).isFalse() + assertThat(navTransaction).isNotNull() + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.last()) } @Test - fun `unbindTransaction() - preserves a different bound transaction`() { + fun `startNavTransaction() - starts none while a host transaction is bound after the previous one finishes`() { val fixture = Fixture.create(ownsLease = true) - val bound = mock() - fixture.realScope.transaction = bound + val navScopes = fixture.navScopes + navScopes.withNavScope { it.startNavTransaction("/Profile") } + val previous = fixture.startedTransactions.single() + val hostTransaction = mock() + fixture.scope.transaction = hostTransaction + navScopes.withNavScope { it.finishNavTransaction() } - fixture.navScope.unbindTransaction(mock()) + val navTransaction = navScopes.withNavScope { it.startNavTransaction("/Settings") } - assertThat(fixture.realScope.transaction).isSameInstanceAs(bound) + assertThat(navTransaction).isNull() + verify(previous).finish(SpanStatus.OK) + assertThat(fixture.startedTransactions).containsExactly(previous) + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) } @Test - fun `clearFinishedTransaction() - unbinds a finished transaction with the nav lease`() { + fun `finishNavTransaction() - finishes and unbinds the nav transaction`() { val fixture = Fixture.create(ownsLease = true) - fixture.realScope.transaction = - mock().also { whenever(it.isFinished).thenReturn(true) } + val navScopes = fixture.navScopes + navScopes.withNavScope { it.startNavTransaction("/Profile") } + val started = fixture.startedTransactions.single() - fixture.navScope.clearFinishedTransaction() + navScopes.withNavScope { it.finishNavTransaction() } - assertThat(fixture.realScope.transaction).isNull() + verify(started).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isNull() } @Test - fun `clearFinishedTransaction() - does nothing without the nav lease`() { - val fixture = Fixture.create() - val finished = mock().also { whenever(it.isFinished).thenReturn(true) } - fixture.realScope.transaction = finished + fun `finishNavTransaction() - finishes it without the nav lease and keeps the leaseholder's bound`() { + val fixture = Fixture.create(ownsLease = true) + val navScopes = fixture.navScopes + navScopes.withNavScope { it.startNavTransaction("/Profile") } + val started = fixture.startedTransactions.single() + val otherTransaction = fixture.startAnotherOwnersNavTransaction() - fixture.navScope.clearFinishedTransaction() + navScopes.withNavScope { it.finishNavTransaction() } - assertThat(fixture.realScope.transaction).isSameInstanceAs(finished) + verify(started).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isSameInstanceAs(otherTransaction) } @Test - fun `transaction - hides another owner's nav transaction without the nav lease`() { - val fixture = Fixture.create() - val other = Any() - val otherTransaction = mock() - fixture.coordinator.registerCandidate(other, Lifecycle.State.RESUMED, {}, {}) - fixture.coordinator.onBackStackUpdate(other, fixture.realScope) - fixture.coordinator.retainTransaction(other, otherTransaction) - fixture.realScope.transaction = otherTransaction + fun `finishNavTransaction() - forgets the nav transaction if unbinding throws`() { + val fixture = Fixture.create(ownsLease = true) + val navScopes = fixture.navScopes + navScopes.withNavScope { it.startNavTransaction("/Profile") } + val started = fixture.startedTransactions.single() + // Registered after the start, so that only unbinding the transaction reaches it. + fixture.scope.options.addScopeObserver( + object : ScopeObserverAdapter() { + override fun setTransaction(transaction: String?) { + throw ThrowingScopeObserverException() + } + } + ) - val transaction = fixture.navScope.transaction + assertThrows(ThrowingScopeObserverException::class.java) { + navScopes.withNavScope { it.finishNavTransaction() } + } + navScopes.withNavScope { it.finishNavTransaction() } - assertThat(transaction).isNull() + // Finished once: the second call has no nav transaction left to finish. + verify(started).finish(SpanStatus.OK) } @Test - fun `transaction - reports a non-nav transaction without the nav lease`() { - val fixture = Fixture.create() + fun `finishNavTransaction() - keeps a different bound transaction`() { + val fixture = Fixture.create(ownsLease = true) + val navScopes = fixture.navScopes + navScopes.withNavScope { it.startNavTransaction("/Profile") } val hostTransaction = mock() - fixture.realScope.transaction = hostTransaction + fixture.scope.transaction = hostTransaction - val transaction = fixture.navScope.transaction + navScopes.withNavScope { it.finishNavTransaction() } - assertThat(transaction).isSameInstanceAs(hostTransaction) + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) } @Test - fun `transaction - reports the bound transaction with the nav lease`() { - val fixture = Fixture.create(ownsLease = true) - val boundTransaction = mock() - fixture.realScope.transaction = boundTransaction + fun `finishNavTransaction() - keeps the bound transaction when the nav transaction has not started`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes + val otherTransaction = fixture.startAnotherOwnersNavTransaction() + navScopes.withNavScope { it.startNavTransaction("/Profile") } + + navScopes.withNavScope { it.finishNavTransaction() } + + assertThat(fixture.scope.transaction).isSameInstanceAs(otherTransaction) + verify(otherTransaction, never()).finish(any()) + } - val transaction = fixture.navScope.transaction + private val navigationContext = mapOf("route" to "Profile") - assertThat(transaction).isSameInstanceAs(boundTransaction) + /** Runs [block] with the [INavScope] handed out by [NavScopes.configureScope]. */ + private fun NavScopes.withNavScope(block: (INavScope) -> T): T { + val results = mutableListOf() + configureScope { results += block(it) } + return results.single() } + private fun INavScope.startNavTransaction(name: String): INavTransaction? = + startNavTransaction(TransactionContext(name, "navigation"), TransactionOptions()) + /** Thrown by a scope observer, to stand in for one the host app registered. */ private class ThrowingScopeObserverException : RuntimeException() - private class Fixture - private constructor( - val realScope: Scope, - val stagedNavData: StagedNavData, - val coordinator: NavLeaseCoordinator, - val navScope: NavScope, - private val owner: Any, - ) { - - /** Retains [transaction] as the owner's nav transaction; the owner must hold the nav lease. */ - fun retainTransaction(transaction: ITransaction) { - coordinator.retainTransaction(owner, transaction) - check(coordinator.isNavTransaction(transaction)) + private class TestLifecycleOwner : LifecycleOwner { + override val lifecycle: LifecycleRegistry = + LifecycleRegistry.createUnsafe(this).apply { currentState = Lifecycle.State.RESUMED } + } + + private class Fixture private constructor() { + val scope = Scope(SentryOptions()) + val coordinator = NavLeaseCoordinator() + val startedTransactions = mutableListOf() + + /** + * `LifecycleRegistry` holds its owner weakly and throws once it's collected, so the fixture + * keeps the owners it creates alive for the test. + */ + private val lifecycleOwners = mutableListOf() + + private val scopes = + mock().also { scopes -> + whenever(scopes.options).thenReturn(SentryOptions()) + doAnswer { + (it.arguments[0] as ScopeCallback).run(scope) + null + } + .whenever(scopes) + .configureScope(any()) + whenever(scopes.startTransaction(any(), any())) + .thenAnswer { + val name = (it.arguments[0] as TransactionContext).name + mock() + .also { transaction -> whenever(transaction.name).thenReturn(name) } + .also(startedTransactions::add) + } + } + + /** Attached to a resumed owner, but without the nav lease until [acquireLease]. */ + val navScopes = resumedNavScopes() + + /** Lets [navScopes] acquire the nav lease by applying its first back stack update. */ + fun acquireLease() { + navScopes.onBackStackUpdate() } - /** Lets another resumed owner take the nav lease from the owner. */ - fun giveLeaseToAnotherOwner() { - val other = Any() - coordinator.registerCandidate(other, Lifecycle.State.RESUMED, {}, {}) - coordinator.onBackStackUpdate(other, realScope) - check(!coordinator.hasLease(owner)) + /** + * Lets another resumed owner acquire the nav lease and start a nav transaction, which is + * returned. + */ + fun startAnotherOwnersNavTransaction(): ITransaction { + val other = resumedNavScopes() + other.onBackStackUpdate() + other.configureScope { + it.startNavTransaction(TransactionContext("/Other", "navigation"), TransactionOptions()) + } + return startedTransactions.single { it.name == "/Other" } } - companion object { + private fun resumedNavScopes(): NavScopes = + NavScopes(scopes, coordinator).also { + it.attach(TestLifecycleOwner().also(lifecycleOwners::add).lifecycle) + } - fun create(ownsLease: Boolean = false): Fixture { - val realScope = Scope(SentryOptions.empty()) - val stagedNavData = StagedNavData() - val coordinator = NavLeaseCoordinator() - val owner = Any() - coordinator.registerCandidate( - owner, - Lifecycle.State.RESUMED, - stagedNavData::publishTo, - stagedNavData::clearPublishedStateFrom, - ) - if (ownsLease) { - coordinator.onBackStackUpdate(owner, realScope) + companion object { + fun create(ownsLease: Boolean = false): Fixture = + Fixture().also { + if (ownsLease) { + it.acquireLease() + } } - - return Fixture( - realScope, - stagedNavData, - coordinator, - NavScope(realScope, stagedNavData, coordinator, owner), - owner, - ) - } } } } 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 4470072b35..3226cbe162 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 @@ -6,25 +6,58 @@ import androidx.lifecycle.LifecycleRegistry import com.google.common.truth.Truth.assertThat import io.sentry.Breadcrumb import io.sentry.Hint +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.ITransaction +import io.sentry.NoOpTransaction import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.ScopeObserverAdapter +import io.sentry.SentryLevel import io.sentry.SentryOptions +import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions +import io.sentry.protocol.Contexts +import org.junit.Assert.assertThrows import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever class NavScopesTest { + @Test + fun `attach() - lets the first back stack update acquire the nav lease`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle()) + + navScopes.applyFirstBackStackUpdate() + + assertThat(fixture.hasLease(navScopes)).isTrue() + } + + @Test + fun `attach() - does nothing for the lifecycle that's already attached`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + val lifecycle = fixture.lifecycle() + navScopes.attach(lifecycle) + + navScopes.attach(lifecycle) + + assertThat(lifecycle.observerCount).isEqualTo(1) + } + @Test fun `attach() - ignores a second, different lifecycle`() { - val fixture = Fixture() + val fixture = Fixture.create() val navScopes = fixture.navScopes() val first = TestLifecycleOwner() val second = TestLifecycleOwner() @@ -37,45 +70,79 @@ class NavScopesTest { } @Test - fun `lifecycle callbacks are recorded while scope callbacks are skipped`() { - val fixture = Fixture() - val outgoing = fixture.navScopes() - val outgoingOwner = TestLifecycleOwner() - outgoing.attach(outgoingOwner.lifecycle) - fixture.isEnabled = false - outgoingOwner.lifecycle.currentState = Lifecycle.State.RESUMED - fixture.isEnabled = true - outgoing.applyFirstBackStackUpdate() - - val incoming = fixture.navScopes() - val incomingOwner = TestLifecycleOwner(Lifecycle.State.RESUMED) - incoming.attach(incomingOwner.lifecycle) - incoming.applyFirstBackStackUpdate() - assertThat(fixture.hasLease(incoming)).isTrue() + fun `attach() - ignores a destroyed lifecycle`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + val lifecycle = fixture.lifecycle(Lifecycle.State.DESTROYED) - incoming.dispose() + navScopes.attach(lifecycle) + navScopes.applyFirstBackStackUpdate() - assertThat(fixture.hasLease(outgoing)).isTrue() + assertThat(fixture.hasLease(navScopes)).isFalse() } @Test - fun `onBackStackUpdate() - an update skipped by the scope does not count as the first update`() { - val fixture = Fixture() + fun `attach() - accepts a new lifecycle after the attached one is destroyed`() { + val fixture = Fixture.create() val navScopes = fixture.navScopes() - val owner = TestLifecycleOwner() - navScopes.attach(owner.lifecycle) - fixture.isEnabled = false + val first = fixture.lifecycle(Lifecycle.State.RESUMED) + navScopes.attach(first) navScopes.applyFirstBackStackUpdate() - fixture.isEnabled = true + first.currentState = Lifecycle.State.DESTROYED + val second = fixture.lifecycle(Lifecycle.State.RESUMED) + navScopes.attach(second) navScopes.applyFirstBackStackUpdate() + assertThat(second.observerCount).isEqualTo(1) assertThat(fixture.hasLease(navScopes)).isTrue() } + @Test + fun `onBackStackUpdate() - doesn't acquire the nav lease before attach`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + + navScopes.applyFirstBackStackUpdate() + + assertThat(fixture.hasLease(navScopes)).isFalse() + } + + @Test + fun `destroying the attached lifecycle finishes the nav transaction`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + val lifecycle = fixture.lifecycle(Lifecycle.State.RESUMED) + navScopes.attach(lifecycle) + navScopes.applyFirstBackStackUpdate() + navScopes.startNavTransaction("/Home") + val started = fixture.startedTransaction("/Home") + + lifecycle.currentState = Lifecycle.State.DESTROYED + + verify(started).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `destroying the attached lifecycle without a scope doesn't finish the nav transaction`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + val lifecycle = fixture.lifecycle(Lifecycle.State.RESUMED) + navScopes.attach(lifecycle) + navScopes.applyFirstBackStackUpdate() + navScopes.startNavTransaction("/Home") + val started = fixture.startedTransaction("/Home") + fixture.isEnabled = false + + lifecycle.currentState = Lifecycle.State.DESTROYED + + verify(started, never()).finish(any()) + } + @Test fun `addBreadcrumb() - records breadcrumbs immediately without the nav lease`() { - val fixture = Fixture() + val fixture = Fixture.create() val leaseholder = fixture.navScopes() leaseholder.attach(TestLifecycleOwner().lifecycle) leaseholder.applyFirstBackStackUpdate() @@ -91,75 +158,273 @@ class NavScopesTest { } @Test - fun `startTransaction() - returns a no-op transaction without the nav lease`() { - val fixture = Fixture() - val covered = fixture.navScopes() - covered.attach(TestLifecycleOwner(Lifecycle.State.RESUMED).lifecycle) - covered.applyFirstBackStackUpdate() - val incoming = fixture.navScopes() - incoming.attach(TestLifecycleOwner(Lifecycle.State.RESUMED).lifecycle) - incoming.applyFirstBackStackUpdate() + fun `dispose() - finishes a displaced owner's nav transaction and keeps the leaseholder's bound`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes(startedRoute = "/Home") + fixture.incoming.startNavTransaction("/Incoming") + val coveredTransaction = fixture.startedTransaction("/Home") + val incomingTransaction = fixture.startedTransaction("/Incoming") + + covered.dispose() + + verify(coveredTransaction).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isSameInstanceAs(incomingTransaction) + } + + @Test + fun `dispose() - finishes the nav transaction when handing off the nav lease throws`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes() + covered.configureScope { it.screen = "/Result" } + fixture.incoming.startNavTransaction("/Incoming") + val incomingTransaction = fixture.startedTransaction("/Incoming") + val failure = IllegalStateException("observer failure") + fixture.scope.options.addScopeObserver( + object : ScopeObserverAdapter() { + override fun setContexts(contexts: Contexts) { + throw failure + } + } + ) + + val thrown = assertThrows(IllegalStateException::class.java) { fixture.incoming.dispose() } + + assertThat(thrown).isSameInstanceAs(failure) + verify(incomingTransaction).finish(SpanStatus.OK) + } + + @Test + fun `dispose() - doesn't finish the nav transaction without a scope`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.applyFirstBackStackUpdate() + navScopes.startNavTransaction("/Home") + val started = fixture.startedTransaction("/Home") + fixture.isEnabled = false + + navScopes.dispose() + + verify(started, never()).finish(any()) + } + + @Test + fun `startNavTransaction() - starts and binds a nav transaction with the nav lease`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.applyFirstBackStackUpdate() + + val transaction = 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`() { + val fixture = Fixture.create(declinesTransactions = true) + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.applyFirstBackStackUpdate() + + val transaction = navScopes.startNavTransaction("/Home") + + assertThat(transaction).isNull() + } + + @Test + fun `startNavTransaction() - logs the route when another transaction blocks the start`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.applyFirstBackStackUpdate() + fixture.scope.transaction = mock() + + navScopes.startNavTransaction("/Home") + + verify(fixture.logger).log(eq(SentryLevel.DEBUG), any(), eq("/Home")) + } + + @Test + fun `startNavTransaction() - starts no nav transaction without the nav lease`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes() - val transaction = - covered.startTransaction(TransactionContext("name", "op"), TransactionOptions()) + val transaction = covered.startNavTransaction("/Result") assertThat(fixture.hasLease(covered)).isFalse() - assertThat(transaction.isNoOp).isTrue() + assertThat(transaction).isNull() + assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isNull() } @Test - fun `startTransaction() - declining without the nav lease defers a propagation context rotation`() { - val fixture = Fixture() - val covered = fixture.navScopes() - covered.attach(TestLifecycleOwner(Lifecycle.State.RESUMED).lifecycle) - covered.applyFirstBackStackUpdate() - val incoming = fixture.navScopes() - incoming.attach(TestLifecycleOwner(Lifecycle.State.RESUMED).lifecycle) - incoming.applyFirstBackStackUpdate() + fun `startNavTransaction() - defers the propagation context rotation without the nav lease`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes() val incomingContext = fixture.scope.propagationContext - covered.startTransaction(TransactionContext("name", "op"), TransactionOptions()) + covered.startNavTransaction("/Result") assertThat(fixture.scope.propagationContext).isSameInstanceAs(incomingContext) - incoming.dispose() + fixture.incoming.dispose() assertThat(fixture.hasLease(covered)).isTrue() assertThat(fixture.scope.propagationContext).isNotSameInstanceAs(incomingContext) } @Test - fun `startTransaction() - returns a no-op transaction after dispose`() { - val fixture = Fixture() + fun `startNavTransaction() - starts no nav transaction after dispose`() { + val fixture = Fixture.create() val navScopes = fixture.navScopes() - navScopes.attach(TestLifecycleOwner().lifecycle) + navScopes.attach(fixture.lifecycle()) navScopes.applyFirstBackStackUpdate() navScopes.dispose() - val transaction = - navScopes.startTransaction(TransactionContext("name", "op"), TransactionOptions()) + navScopes.startNavTransaction("/Home") - assertThat(transaction.isNoOp).isTrue() + assertThat(fixture.startedTransactions).isEmpty() } @Test - fun `rotatePropagationContext() - a rotation requested without the nav lease happens once the lease is acquired`() { - val fixture = Fixture() - val covered = fixture.navScopes() - covered.attach(TestLifecycleOwner(Lifecycle.State.RESUMED).lifecycle) - covered.applyFirstBackStackUpdate() - val incoming = fixture.navScopes() - incoming.attach(TestLifecycleOwner(Lifecycle.State.RESUMED).lifecycle) - incoming.applyFirstBackStackUpdate() - val incomingContext = fixture.scope.propagationContext + fun `finishNavTransaction() - finishes the started nav transaction`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.applyFirstBackStackUpdate() + navScopes.startNavTransaction("/Home") + val started = fixture.startedTransaction("/Home") - covered.configureScope { it.rotatePropagationContext() } - assertThat(fixture.scope.propagationContext).isSameInstanceAs(incomingContext) + navScopes.finishNavTransaction() - incoming.dispose() + verify(started).finish(SpanStatus.OK) + } + + @Test + fun `finishNavTransaction() - finishes the started nav transaction only once`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.applyFirstBackStackUpdate() + navScopes.startNavTransaction("/Home") + val started = fixture.startedTransaction("/Home") + navScopes.finishNavTransaction() + + navScopes.finishNavTransaction() + + verify(started, times(1)).finish(any()) + } + + @Test + fun `finishNavTransaction() - does nothing without a nav transaction`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.applyFirstBackStackUpdate() + val hostTransaction = mock() + fixture.scope.transaction = hostTransaction + + navScopes.finishNavTransaction() + + verify(hostTransaction, never()).finish(any()) + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + } + + @Test + fun `a nav transaction starts and is bound after the previous one is finished`() { + val fixture = Fixture.create() + val navScopes = fixture.navScopes() + navScopes.attach(fixture.lifecycle(Lifecycle.State.RESUMED)) + navScopes.applyFirstBackStackUpdate() + navScopes.startNavTransaction("/Home") + navScopes.finishNavTransaction() + + val transaction = navScopes.startNavTransaction("/Profile") + + assertThat(transaction).isNotNull() + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Profile")) + } + + @Test + fun `losing the nav lease unbinds the nav transaction without finishing it`() { + val fixture = Fixture.create() + + fixture.coveredNavScopes(startedRoute = "/Home") + + val started = fixture.startedTransaction("/Home") + assertThat(fixture.scope.transaction).isNull() + verify(started, never()).finish(any()) + } + + @Test + fun `regaining the nav lease rebinds the unfinished nav transaction`() { + val fixture = Fixture.create() + fixture.coveredNavScopes(startedRoute = "/Home") + + fixture.incoming.dispose() + + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransaction("/Home")) + } + + @Test + fun `regaining the nav lease republishes the nav data before rebinding the nav transaction`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes(startedRoute = "/Home") + covered.configureScope { it.screen = "Home" } + val screensAtBinding = mutableListOf() + fixture.scope.options.addScopeObserver( + object : ScopeObserverAdapter() { + override fun setTransaction(transaction: String?) { + if (transaction == "/Home") screensAtBinding += fixture.scope.screen + } + } + ) + + fixture.incoming.dispose() + + assertThat(screensAtBinding).containsExactly("Home") + } + + @Test + fun `a finished nav transaction is discarded when the nav lease is regained`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes(startedRoute = "/Home") + val started = fixture.startedTransaction("/Home") + whenever(started.isFinished).thenReturn(true) + + fixture.incoming.dispose() assertThat(fixture.hasLease(covered)).isTrue() - assertThat(fixture.scope.propagationContext).isNotSameInstanceAs(incomingContext) + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `a nav transaction finished without the nav lease is not bound when the nav lease is regained`() { + val fixture = Fixture.create() + val covered = fixture.coveredNavScopes(startedRoute = "/Home") + // The mock keeps reporting itself unfinished, like a transaction waiting for its child spans. + covered.finishNavTransaction() + + fixture.incoming.dispose() + + assertThat(fixture.hasLease(covered)).isTrue() + assertThat(fixture.scope.transaction).isNull() + } + + /** 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 + } + + /** Finishes the nav transaction through the [INavScope] handed out by [NavScopes]. */ + private fun NavScopes.finishNavTransaction() { + configureScope { it.finishNavTransaction() } } private fun NavScopes.applyFirstBackStackUpdate() { @@ -169,16 +434,27 @@ class NavScopesTest { private class TestLifecycleOwner(initialState: Lifecycle.State = Lifecycle.State.CREATED) : LifecycleOwner { override val lifecycle: LifecycleRegistry = - LifecycleRegistry.createUnsafe(this).apply { currentState = initialState } + LifecycleRegistry.createUnsafe(this).apply { + // A lifecycle must be created before it can be destroyed. + currentState = Lifecycle.State.CREATED + currentState = initialState + } } - private class Fixture { + private class Fixture private constructor() { var isEnabled = true val scope = Scope(SentryOptions()) val coordinator = NavLeaseCoordinator() + val logger = mock() val scopes = mock().also { scopes -> - whenever(scopes.options).thenReturn(SentryOptions()) + whenever(scopes.options) + .thenReturn( + SentryOptions().apply { + setLogger(this@Fixture.logger) + isDebug = true + } + ) doAnswer { if (isEnabled) { (it.arguments[0] as ScopeCallback).run(scope) @@ -188,11 +464,61 @@ class NavScopesTest { .whenever(scopes) .configureScope(any()) whenever(scopes.startTransaction(any(), any())) - .thenAnswer { mock() } + .thenAnswer { + if (declinesTransactions) { + NoOpTransaction.getInstance() + } else { + val name = (it.arguments[0] as TransactionContext).name + mock() + .also { transaction -> whenever(transaction.name).thenReturn(name) } + .also(startedTransactions::add) + } + } } + /** Whether [scopes] declines to start transactions, returning a no-op transaction instead. */ + var declinesTransactions = false + val startedTransactions = mutableListOf() + + /** The resumed leaseholder that covers the [NavScopes] returned by [coveredNavScopes]. */ + lateinit var incoming: NavScopes + + /** + * `LifecycleRegistry` holds its owner weakly and throws once it's collected, so the fixture + * keeps the owners created by [lifecycle] alive for the test. + */ + private val lifecycleOwners = mutableListOf() + fun navScopes(): NavScopes = NavScopes(scopes, coordinator) + fun lifecycle(state: Lifecycle.State = Lifecycle.State.CREATED): LifecycleRegistry = + TestLifecycleOwner(state).also(lifecycleOwners::add).lifecycle + + fun startedTransaction(name: String): ITransaction = startedTransactions.single { + it.name == name + } + + /** + * Returns a resumed [NavScopes] that applied its first back stack update, started a nav + * transaction for [startedRoute] if given, and then lost the nav lease to the resumed + * [incoming] one. It regains the lease when [incoming] is disposed. + */ + fun coveredNavScopes(startedRoute: String? = null): NavScopes { + val covered = navScopes() + covered.attach(lifecycle(Lifecycle.State.RESUMED)) + covered.onBackStackUpdate() + if (startedRoute != null) { + covered.configureScope { + it.startNavTransaction(TransactionContext(startedRoute, "op"), TransactionOptions()) + } + } + incoming = navScopes() + incoming.attach(lifecycle(Lifecycle.State.RESUMED)) + incoming.onBackStackUpdate() + check(!hasLease(covered)) + return covered + } + private var leaseProbeCount = 0 /** @@ -204,5 +530,13 @@ class NavScopesTest { navScopes.configureScope { it.screen = probe } return scope.screen == probe } + + companion object { + /** + * [declinesTransactions] can also be changed later, e.g., once a transaction was deferred. + */ + 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 new file mode 100644 index 0000000000..2977db6e5b --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavTransactionTest.kt @@ -0,0 +1,125 @@ +package io.sentry.compose.navigation3.lifecycle + +import com.google.common.truth.Truth.assertThat +import io.sentry.ITransaction +import io.sentry.SpanStatus +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.whenever + +class NavTransactionTest { + + 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 `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 `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 `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) + } + + private class Fixture private constructor(val transaction: ITransaction) { + + companion object { + fun create(): Fixture { + val contexts = Contexts() + val transaction = + mock().also { + whenever(it.name).thenReturn("/Checkout") + whenever(it.contexts).thenReturn(contexts) + } + return Fixture(transaction) + } + } + } +} 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 new file mode 100644 index 0000000000..8827eba438 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavTransactionTest.kt @@ -0,0 +1,398 @@ +package io.sentry.compose.navigation3.lifecycle + +import com.google.common.truth.Truth.assertThat +import io.sentry.ILogger +import io.sentry.IScopes +import io.sentry.ITransaction +import io.sentry.NoOpTransaction +import io.sentry.Scope +import io.sentry.ScopeObserverAdapter +import io.sentry.SentryLevel +import io.sentry.SentryOptions +import io.sentry.SpanStatus +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import org.junit.Assert.assertThrows +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +@Suppress("LargeClass") +class StagedNavTransactionTest { + + private val transactionContext = TransactionContext("/Profile", "navigation") + private val transactionOptions = TransactionOptions() + + private val finishedTransaction = + mock().also { whenever(it.isFinished).thenReturn(true) } + private val hostTransaction = mock() + + @Test + fun `start() - starts a nav transaction and binds it to a free scope`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + + val navTransaction = + stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + + val started = fixture.startedTransaction("/Profile") + verify(fixture.scopes).startTransaction(transactionContext, transactionOptions) + assertThat(navTransaction?.transaction).isSameInstanceAs(started) + assertThat(fixture.scope.transaction).isSameInstanceAs(started) + } + + @Test + fun `start() - starts no nav transaction while another transaction is bound`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.scope.transaction = hostTransaction + + val navTransaction = + stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + + assertThat(navTransaction).isNull() + assertThat(fixture.startedTransactions).isEmpty() + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + } + + @Test + fun `start() - logs the route when another transaction blocks the start`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.scope.transaction = hostTransaction + + stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + + verify(fixture.logger).log(eq(SentryLevel.DEBUG), any(), eq("/Profile")) + } + + @Test + fun `start() - binds the nav transaction in place of a finished bound transaction`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.scope.transaction = finishedTransaction + + val navTransaction = + stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + + val started = fixture.startedTransaction("/Profile") + assertThat(navTransaction?.transaction).isSameInstanceAs(started) + assertThat(fixture.scope.transaction).isSameInstanceAs(started) + } + + @Test + fun `start() - returns null and keeps the current nav transaction when the start is declined`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val current = fixture.start("/Home") + fixture.declinesTransactions = true + + val navTransaction = + stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + + assertThat(navTransaction).isNull() + assertThat(fixture.scope.transaction).isSameInstanceAs(current) + stagedNavTransaction.finish(fixture.scope) + verify(current).finish(SpanStatus.OK) + } + + @Test + fun `start() - replaces the current nav transaction without finishing it`() { + 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.start(fixture.scope, transactionContext, transactionOptions) + verify(current, never()).finish(any()) + + stagedNavTransaction.finish(fixture.scope) + verify(fixture.startedTransaction("/Profile")).finish(SpanStatus.OK) + verify(current, never()).finish(any()) + } + + @Test + fun `start() - keeps the started nav transaction when binding it throws`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.transactionObserverThrows = true + + assertThrows(ThrowingScopeObserverException::class.java) { + stagedNavTransaction.start(fixture.scope, transactionContext, transactionOptions) + } + fixture.transactionObserverThrows = false + val started = fixture.startedTransaction("/Profile") + stagedNavTransaction.finish(fixture.scope) + + verify(started).finish(SpanStatus.OK) + } + + @Test + fun `publishTo() - binds an unpublished unfinished nav transaction again`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) + + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(started) + assertThat(fixture.startedTransactions).containsExactly(started) + } + + @Test + fun `publishTo() - keeps the bound nav transaction bound`() { + 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()) + } + + @Test + fun `publishTo() - binds an unpublished nav transaction in place of a finished bound transaction`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) + fixture.scope.transaction = finishedTransaction + + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(started) + } + + @Test + fun `publishTo() - doesn't bind an unpublished nav transaction while another transaction is bound`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) + fixture.scope.transaction = hostTransaction + + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + stagedNavTransaction.finish(fixture.scope) + verify(started).finish(SpanStatus.OK) + } + + @Test + fun `publishTo() - forgets and unbinds a bound nav transaction that has finished`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + whenever(started.isFinished).thenReturn(true) + + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `publishTo() - forgets an unpublished nav transaction that has finished and keeps another transaction`() { + 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.publishTo(fixture.scope) + + 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 `unpublishFrom() - unbinds the started nav transaction without finishing it`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + + stagedNavTransaction.unpublishFrom(fixture.scope) + + assertThat(fixture.scope.transaction).isNull() + verify(started, never()).finish(any()) + } + + @Test + fun `unpublishFrom() - keeps another bound transaction`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) + fixture.scope.transaction = hostTransaction + + stagedNavTransaction.unpublishFrom(fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + } + + @Test + fun `finish() - finishes the started nav transaction and unbinds it`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + + stagedNavTransaction.finish(fixture.scope) + + verify(started).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `finish() - finishes an unpublished nav transaction and keeps another bound transaction`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + stagedNavTransaction.unpublishFrom(fixture.scope) + fixture.scope.transaction = hostTransaction + + stagedNavTransaction.finish(fixture.scope) + + verify(started).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + } + + @Test + fun `finish() - forgets the nav transaction so that publishing never binds it again`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.start() + stagedNavTransaction.finish(fixture.scope) + + stagedNavTransaction.publishTo(fixture.scope) + + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `finish() - forgets the nav transaction when unbinding it throws`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + fixture.transactionObserverThrows = true + + assertThrows(ThrowingScopeObserverException::class.java) { + stagedNavTransaction.finish(fixture.scope) + } + stagedNavTransaction.finish(fixture.scope) + + // Finished once: the second call has no nav transaction left to finish or unbind. + verify(started).finish(SpanStatus.OK) + } + + @Test + fun `finish() - leaves the scope untouched without a nav transaction`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + fixture.scope.transaction = hostTransaction + + stagedNavTransaction.finish(fixture.scope) + + verify(hostTransaction, never()).finish(any()) + assertThat(fixture.scope.transaction).isSameInstanceAs(hostTransaction) + } + + @Test + fun `forget() - forgets the started nav transaction without finishing or unbinding it`() { + val fixture = Fixture.create() + val stagedNavTransaction = fixture.stagedNavTransaction + val started = fixture.start() + + stagedNavTransaction.forget() + stagedNavTransaction.finish(fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(started) + verify(started, never()).finish(any()) + } + + /** Thrown by a scope observer, to stand in for one the host app registered. */ + private class ThrowingScopeObserverException : RuntimeException() + + private class Fixture private constructor() { + val scope = Scope(SentryOptions()) + val logger = mock() + + /** Whether [scopes] declines to start transactions, returning a no-op transaction instead. */ + var declinesTransactions = false + val startedTransactions = mutableListOf() + + /** Whether a scope observer throws when the scope's transaction is bound or unbound. */ + var transactionObserverThrows = false + + val scopes = + mock().also { scopes -> + whenever(scopes.options) + .thenReturn( + SentryOptions().apply { + setLogger(this@Fixture.logger) + isDebug = true + } + ) + whenever(scopes.startTransaction(any(), any())) + .thenAnswer { + if (declinesTransactions) { + NoOpTransaction.getInstance() + } else { + val name = (it.arguments[0] as TransactionContext).name + mock() + .also { transaction -> whenever(transaction.name).thenReturn(name) } + .also(startedTransactions::add) + } + } + } + + val stagedNavTransaction = StagedNavTransaction(scopes) + + init { + scope.options.addScopeObserver( + object : ScopeObserverAdapter() { + override fun setTransaction(transaction: String?) { + if (transactionObserverThrows) { + throw ThrowingScopeObserverException() + } + } + } + ) + } + + fun startedTransaction(name: String): ITransaction = startedTransactions.single { + it.name == name + } + + /** 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(), + ) + return startedTransaction(name) + } + + companion object { + fun create(declinesTransactions: Boolean = false): Fixture = + Fixture().also { it.declinesTransactions = declinesTransactions } + } + } +}