From c464ea381d6fc12ba2a4cb719f7a468e2b43d88a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 20 Oct 2026 16:47:30 +0200 Subject: [PATCH 1/4] collection: Sentry NavEffect lifecycle From f3025c6013088a111cba6f0254be0ffbb8b5b8a6 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 22 Oct 2026 09:49:35 +0200 Subject: [PATCH 2/4] ref(android): Add NavLeaseCoordinator (JAVA-785) (#6302) Part 1 of our Nav3 support for multi-Activity and multi-Fragment applications Introduces NavLeaseCoordinator, a lifecycle-aware lease manager that selects which SentryNavEffect instance may publish nav data to a shared IScope and create nav transactions under it. --- gradle/libs.versions.toml | 1 + sentry-android-navigation3/build.gradle.kts | 1 + .../lifecycle/NavLeaseCoordinator.kt | 465 +++++++ .../navigation3/SentryNavEffectTest.kt | 8 - .../lifecycle/NavLeaseCoordinatorTest.kt | 1068 +++++++++++++++++ 5 files changed, 1535 insertions(+), 8 deletions(-) create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e5c2f9e217..2afcc5d48b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -110,6 +110,7 @@ androidx-core-ktx = { module = "androidx.core:core-ktx", version = "1.7.0" } androidx-fragment-ktx = { module = "androidx.fragment:fragment-ktx", version = "1.3.5" } androidx-lifecycle-common-java8 = { module = "androidx.lifecycle:lifecycle-common-java8", version.ref = "androidxLifecycle" } androidx-lifecycle-process = { module = "androidx.lifecycle:lifecycle-process", version.ref = "androidxLifecycle" } +androidx-lifecycle-runtime-compose = { module = "androidx.lifecycle:lifecycle-runtime-compose", version = "2.8.3" } androidx-navigation-runtime = { module = "androidx.navigation:navigation-runtime", version.ref = "androidxNavigation" } androidx-navigation-compose = { module = "androidx.navigation:navigation-compose", version.ref = "androidxNavigation" } androidx-navigation-fragment = { module = "androidx.navigation:navigation-fragment-ktx", version.ref = "androidxNavigation" } diff --git a/sentry-android-navigation3/build.gradle.kts b/sentry-android-navigation3/build.gradle.kts index b9d02bb49d..bab8fce5c7 100644 --- a/sentry-android-navigation3/build.gradle.kts +++ b/sentry-android-navigation3/build.gradle.kts @@ -62,6 +62,7 @@ dependencies { implementation(projects.sentry) compileOnly(libs.androidx.compose.runtime) + compileOnly(libs.androidx.lifecycle.runtime.compose) testImplementation(libs.androidx.compose.ui.test.junit4) testImplementation(libs.androidx.test.core) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt new file mode 100644 index 0000000000..0068ce0d33 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinator.kt @@ -0,0 +1,465 @@ +package io.sentry.compose.navigation3.lifecycle + +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import io.sentry.IScope +import io.sentry.ITransaction +import io.sentry.SpanStatus + +/** + * 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]. + * + * Note: "Owner" below refers to the `LifecycleOwner` whose lifecycle events the candidate's + * `NavScopes` relays. + * + * **Exception semantics** + * + * Methods in this class with an `IScope` parameter can throw arbitrary exceptions†, which propagate + * to the caller. This class restores its own bookkeeping first, so the coordinator remains usable + * if the exception is caught. + * + * † 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. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. All `NavScopes` instances using this coordinator must + * confine their access to the same thread. + */ +@Suppress("TooManyFunctions") +internal class NavLeaseCoordinator { + + private val candidates = mutableListOf() + + /** Candidate responsible for the most recently published nav data in the shared scope. */ + private var publishedDataCandidate: Candidate? = null + + private var leaseHolder: Candidate? = null + + /** + * Counter that orders candidates by when this coordinator most recently observed them in resumed + * state. + * + * Each resume, including registration of an already-resumed owner, assigns the next value to that + * candidate's [Candidate.resumeOrder]. This lets fallback selection prefer the most recently + * resumed eligible candidate without relying on clock timestamps. Zero is reserved for candidates + * that have not been observed resumed and therefore cannot be chosen as fallbacks. + */ + private var resumeOrder: Long = 0 + + /** + * 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. + */ + fun registerCandidate( + owner: Any, + lifecycleState: Lifecycle.State, + onPublishData: (IScope) -> Unit, + onClearPublishedData: (IScope) -> Unit, + ) { + if (findCandidate(owner) != null) { + return + } + + if (lifecycleState == Lifecycle.State.DESTROYED) { + return + } + + 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. + * + * Coordinator references are released even if scope cleanup or lease handoff throws. See + * "Exception semantics" in the class KDoc. The remaining scope cleanup is then skipped. + */ + fun unregisterCandidate(owner: Any, scope: IScope?) { + val candidate = findCandidate(owner) ?: return + + try { + finishTransaction(candidate, scope) + candidates.remove(candidate) + + if (publishedDataCandidate === candidate) { + publishedDataCandidate = null + scope?.let { candidate.onClearPublishedData(it) } + } + + if (leaseHolder === candidate) { + handOffLease(mostRecentlyResumedCandidate(), scope) + } + } finally { + // Scope observers and transaction callbacks can throw. Never retain a departing owner + // just because cleanup failed, and preserve any successor selected during handoff. + candidates.remove(candidate) + candidate.transaction = null + if (publishedDataCandidate === candidate) { + publishedDataCandidate = null + } + if (leaseHolder === candidate) { + leaseHolder = null + } + } + } + + fun hasLease(owner: Any): Boolean = leaseHolder?.owner === owner + + /** + * Records that the registered [owner] is about to apply a back-stack update and acquires the nav + * lease if the owner's lifecycle permits it. + * + * 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. + */ + fun onBackStackUpdate(owner: Any, scope: IScope) { + val candidate = findCandidate(owner) ?: return + if (candidate.hasAppliedInitialBackStackUpdate) { + return + } + + candidate.hasAppliedInitialBackStackUpdate = true + + val holder = leaseHolder + val wouldDisplaceResumedHolder = + holder != null && + holder.state == Lifecycle.State.RESUMED && + candidate.state != Lifecycle.State.RESUMED + + if (candidate.canAcquireLeaseOnFirstUpdate && !wouldDisplaceResumedHolder) { + transferLeaseTo(candidate, scope) + } + } + + /** Whether [transaction] is a nav transaction retained for any registered owner. */ + fun isNavTransaction(transaction: ITransaction): Boolean = candidates.any { + it.transaction === transaction + } + + /** + * Records [transaction] as the nav transaction of [owner], which must hold the lease. Ignored + * otherwise, as only the leaseholder starts nav transactions. + * + * 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]. + */ + fun retainTransaction(owner: Any, transaction: ITransaction) { + val candidate = leaseHolder ?: return + if (candidate.owner !== owner) { + return + } + 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. + * + * 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. + */ + fun releaseTransaction(owner: Any, transaction: ITransaction) { + val candidate = findCandidate(owner) ?: return + if (candidate.transaction === transaction) { + candidate.transaction = null + } + } + + 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. + */ + private fun mostRecentlyResumedCandidate( + excluding: Candidate? = null, + resumedOnly: Boolean = false, + ): Candidate? = + candidates + .filter { + it !== excluding && + it.hasAppliedInitialBackStackUpdate && + it.state.isAtLeast(Lifecycle.State.STARTED) && + it.resumeOrder > 0 && + (!resumedOnly || it.state == Lifecycle.State.RESUMED) + } + .maxByOrNull { it.resumeOrder } + + /** + * Hands the current lease to [next]. Without a [scope], the lease is released instead, as [next] + * could not publish its staged data. + */ + private fun handOffLease(next: Candidate?, scope: IScope?) { + if (scope != null) { + transferLeaseTo(next, scope) + } else { + leaseHolder = null + } + } + + /** + * 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. + */ + 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) { + clearTransactionFromScope(candidate, scope) + candidate.transaction = null + } else if (transaction != null) { + // The navigation lease doesn't give this owner control over an unrelated transaction + // already bound to the shared scope. Restore the owner's retained nav transaction only if + // the scope is free or already holds that same transaction. + scope.withTransaction { current -> + if (current == null || current === transaction) { + scope.transaction = transaction + } + } + } + } + + leaseHolder = candidate + if (candidate != null) { + // 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. + val previousPublisher = publishedDataCandidate + try { + if (previousPublisher != null && previousPublisher !== candidate) { + previousPublisher.onClearPublishedData(scope) + } + publishedDataCandidate = candidate + candidate.onPublishData(scope) + } finally { + // Scope observers can throw. A candidate owns the scope's nav data from the moment it + // takes the lease, so record it as the publisher even if clearing the previous owner's + // data threw. Leaving the previous owner recorded here would strand both owners' values + // b/c the next handoff would clear data the candidate may have already replaced, and would + // never clear the candidate's own. + publishedDataCandidate = candidate + } + } + } + + /** Finishes and unbinds [candidate]'s nav transaction, then forgets it. */ + private fun finishTransaction(candidate: Candidate, scope: IScope?) { + // 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) { + val transaction = candidate.transaction + if (transaction != null && !transaction.isFinished) { + transaction.finish(transaction.status ?: SpanStatus.OK) + } + clearTransactionFromScope(candidate, scope) + } + + candidate.transaction = null + } + + private fun clearTransactionFromScope(candidate: Candidate, scope: IScope) { + val transaction = candidate.transaction ?: return + scope.withTransaction { current -> + if (current === transaction) { + scope.clearTransaction() + } + } + } + + /** A [LifecycleOwner] that has registered itself as a candidate for lease ownership. */ + private class Candidate( + val owner: Any, + val onPublishData: (IScope) -> Unit, + val onClearPublishedData: (IScope) -> Unit, + initialState: Lifecycle.State, + initialResumeOrder: Long, + ) { + + /** The candidate's lifecycle state, as last observed. */ + var state: Lifecycle.State = initialState + private set + + /** + * Whether the candidate has been observed STOPPED since + * [registration][NavLeaseCoordinator.registerCandidate]. + */ + 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]. + */ + var resumeOrder: Long = initialResumeOrder + private set + + var hasAppliedInitialBackStackUpdate: Boolean = false + + /** + * 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]). + */ + val canAcquireLeaseOnFirstUpdate: Boolean + get() = state == Lifecycle.State.RESUMED || (resumeOrder == 0L && !wasStopped) + + fun start() { + // Attaching to an already-resumed lifecycle replays ON_START before ON_RESUME, so only move + // up from CREATED. + if (!state.isAtLeast(Lifecycle.State.STARTED)) { + state = Lifecycle.State.STARTED + } + } + + fun resume(order: Long) { + resumeOrder = order + state = Lifecycle.State.RESUMED + } + + fun pause() { + state = Lifecycle.State.STARTED + } + + fun stop() { + state = Lifecycle.State.CREATED + wasStopped = true + } + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt index 7b8a4b9946..acc8445b57 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt @@ -15,7 +15,6 @@ import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.IScope import io.sentry.IScopes -import io.sentry.ITransaction import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.SentryOptions @@ -550,13 +549,6 @@ class SentryNavEffectTest { return navigationContext[BACKSTACK_KEY] as? List> } - private fun ITransaction.navigationBackStack(): List>? { - val navigationContext = contexts[NAVIGATION_CONTEXT_KEY] as? Map<*, *> ?: return null - - @Suppress("UNCHECKED_CAST") - return navigationContext[BACKSTACK_KEY] as? List> - } - private companion object { const val NAVIGATION_CONTEXT_KEY = "Navigation" const val BACKSTACK_KEY = "Back Stack" 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 new file mode 100644 index 0000000000..61e5ef2968 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavLeaseCoordinatorTest.kt @@ -0,0 +1,1068 @@ +package io.sentry.compose.navigation3.lifecycle + +import androidx.lifecycle.Lifecycle +import com.google.common.truth.Truth.assertThat +import io.sentry.IScope +import io.sentry.ITransaction +import io.sentry.Scope +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.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() + + @Test + fun `a registered created owner acquires the nav lease on its first back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.published).containsExactly(owner) + } + + @Test + fun `registering the same owner twice preserves its first publication callback`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + + navLeaseCoordinator.registerCandidate( + owner, + Lifecycle.State.RESUMED, + { fixture.published += "duplicate" }, + { fixture.cleared += "duplicate" }, + ) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(fixture.published).containsExactly(owner) + } + + @Test + fun `registering a destroyed owner does not make it eligible for the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + + fixture.register(owner, Lifecycle.State.DESTROYED) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.published).isEmpty() + } + + @Test + fun `equal but distinct owners do not share the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val registeredOwner = String(charArrayOf('a')) + val equalOwner = String(charArrayOf('a')) + fixture.acquireLease(registeredOwner) + + val result = navLeaseCoordinator.hasLease(equalOwner) + + assertThat(result).isFalse() + assertThat(navLeaseCoordinator.hasLease(registeredOwner)).isTrue() + } + + @Test + fun `starting a created owner leaves it eligible to acquire the nav lease on its first back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + + navLeaseCoordinator.startCandidate(owner) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + } + + @Test + fun `restarting a stopped owner does not take the nav lease on its first back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + fixture.acquireLease(incumbent) + + navLeaseCoordinator.startCandidate(owner) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(incumbent)).isTrue() + assertThat(fixture.published).containsExactly(incumbent) + } + + @Test + fun `resuming an owner before its first back stack update leaves the nav lease unassigned`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.CREATED) + + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.published).isEmpty() + } + + @Test + fun `resuming a stopped owner after its first back stack update transfers the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(incumbent) + fixture.register(owner, Lifecycle.State.CREATED) + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.published).containsExactly(incumbent, owner).inOrder() + } + + @Test + fun `a paused owner cannot acquire the nav lease on its first back stack update`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.register(owner, Lifecycle.State.RESUMED) + navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + + navLeaseCoordinator.onBackStackUpdate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.published).isEmpty() + } + + @Test + fun `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) + + fixture.acquireLease(owner, state = Lifecycle.State.STARTED) + + assertThat(navLeaseCoordinator.hasLease(incumbent)).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) + + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.published).containsExactly(incumbent, owner).inOrder() + } + + @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) + + fixture.acquireLease(owner, state = Lifecycle.State.CREATED) + + assertThat(navLeaseCoordinator.hasLease(owner)).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) + + fixture.acquireLease(owner, state = Lifecycle.State.CREATED) + + assertThat(navLeaseCoordinator.hasLease(owner)).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(owner) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + } + + @Test + fun `acquiring the nav lease clears the previous owner's published nav data before publishing`() { + 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" }, + ) + + navLeaseCoordinator.onBackStackUpdate(incoming, fixture.scope) + + assertThat(calls) + .containsExactly("publish previous", "clear previous", "publish incoming") + .inOrder() + } + + @Test + fun `regaining the nav lease doesn't clear the owner's own published nav data`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(previous) + fixture.acquireLease(owner) + navLeaseCoordinator.pauseCandidate(owner, null) + fixture.cleared.clear() + + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.published).containsExactly(previous, owner, owner).inOrder() + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `releasing the nav lease leaves the owner's published nav data in place`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(previous) + fixture.acquireLease(owner) + fixture.cleared.clear() + + navLeaseCoordinator.pauseCandidate(owner, null) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(fixture.cleared).isEmpty() + } + + /** + * An owner that leaves must not leave its nav data behind on the shared scope. + * + * The coordinator records which owner last published nav data. Each handoff uses that record to + * clear the previous owner's screen and back stack, then publishes the incoming owner's. If the + * clear attempt throws, the coordinator must still record the incoming owner as the one that last + * published. + * + * This test runs two handoffs over three owners. The first owner publishes; the second takes the + * lease; and then the first owner's clear attempt throws. The third then takes the lease. The + * test verifies that third handoff clears the second owner's data, and not the first's. That lets + * us ensure the second problem listed below doesn't happen. + * + * In the setup above, if the coordinator didn't record the second owner's data after the first + * owner's clear attempt threw, two things would happen when handing off to the third owner: + * + * 1. Harmless: The handoff logic would try to clear the first owner's data again. That'd remove + * nothing because the scope no longer holds the values the first owner published. + * 2. Problematic: The handoff would never clear the second owner's data. Instead, the second + * owner's data would stay on the scope, so later events would report stale nav data. + */ + @Test + fun `if a clear attempt throws during a handoff, the next handoff clears the new leaseholder's nav data`() { + 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 + fixture.acquireLease( + firstOwner, + onClearPublishedData = { + if (failClear) { + failClear = false + throw failure + } + fixture.cleared += firstOwner + }, + ) + fixture.register(secondOwner, Lifecycle.State.RESUMED) + + val thrown = + assertThrows(IllegalStateException::class.java) { + navLeaseCoordinator.onBackStackUpdate(secondOwner, fixture.scope) + } + + assertThat(thrown).isSameInstanceAs(failure) + assertThat(navLeaseCoordinator.hasLease(secondOwner)).isTrue() + + fixture.acquireLease(thirdOwner) + + assertThat(fixture.cleared).containsExactly(secondOwner) + } + + /** + * The same guarantee as + * [if a clear attempt throws during a handoff, the next handoff clears the new leaseholder's nav data], + * but for a failure while publishing instead of while clearing. + * + * This case already worked before the fix, because the coordinator records the incoming owner + * before it publishes. The test exists to catch a future change that moves the record after the + * publish. + */ + @Test + fun `if a publish attempt throws during a handoff, the next handoff clears the new leaseholder's nav data`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val failure = IllegalStateException("publish failed") + val firstOwner = previous + val secondOwner = incoming + val thirdOwner = owner + fixture.acquireLease(firstOwner) + navLeaseCoordinator.registerCandidate( + secondOwner, + Lifecycle.State.RESUMED, + onPublishData = { throw failure }, + onClearPublishedData = { fixture.cleared += secondOwner }, + ) + + val thrown = + assertThrows(IllegalStateException::class.java) { + navLeaseCoordinator.onBackStackUpdate(secondOwner, fixture.scope) + } + + assertThat(thrown).isSameInstanceAs(failure) + assertThat(navLeaseCoordinator.hasLease(secondOwner)).isTrue() + + fixture.acquireLease(thirdOwner) + + assertThat(fixture.cleared).containsExactly(firstOwner, secondOwner).inOrder() + } + + @Test + fun `unregistering an owner that threw while clearing its nav data preserves the new leaseholder's nav data`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val failure = IllegalStateException("clear failed") + var failClear = true + fixture.acquireLease( + previous, + onClearPublishedData = { + if (failClear) { + failClear = false + throw failure + } + fixture.cleared += previous + }, + ) + fixture.register(incoming, Lifecycle.State.RESUMED) + assertThrows(IllegalStateException::class.java) { + navLeaseCoordinator.onBackStackUpdate(incoming, fixture.scope) + } + + navLeaseCoordinator.unregisterCandidate(previous, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(incoming)).isTrue() + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `later back stack updates do not republish nav data or reclaim the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val first = Any() + val second = Any() + fixture.acquireLease(first) + fixture.acquireLease(second, state = Lifecycle.State.RESUMED) + + navLeaseCoordinator.onBackStackUpdate(first, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(second)).isTrue() + assertThat(fixture.published).containsExactly(first, second).inOrder() + } + + @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) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(recent)).isTrue() + assertThat(fixture.published.last()).isSameInstanceAs(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) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(incoming)).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) + + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).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) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(eligible)).isTrue() + assertThat(navLeaseCoordinator.hasLease(uninitialized)).isFalse() + } + + @Test + fun `pausing the only eligible owner keeps the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner) + + navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).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) + + navLeaseCoordinator.pauseCandidate(owner, null) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + } + + @Test + fun `resuming a paused leaseholder does not republish its nav data`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner) + navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.published).containsExactly(owner) + } + + @Test + fun `stopping the leaseholder restores a resumed owner`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(previous) + fixture.acquireLease(owner, state = Lifecycle.State.RESUMED) + + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() + assertThat(fixture.published.last()).isSameInstanceAs(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) + + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).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) + + navLeaseCoordinator.stopCandidate(owner, null) + + assertThat(navLeaseCoordinator.hasLease(owner)).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) + + navLeaseCoordinator.stopCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() + assertThat(fixture.published).containsExactly(previous, owner).inOrder() + assertThat(fixture.cleared).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) + + navLeaseCoordinator.resumeCandidate(previous, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(previous)).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) + + navLeaseCoordinator.startCandidate(owner) + + assertThat(navLeaseCoordinator.hasLease(owner)).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) + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() + assertThat(fixture.published).containsExactly(previous, owner).inOrder() + } + + @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) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).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) + + navLeaseCoordinator.resumeCandidate(owner, null) + + assertThat(navLeaseCoordinator.hasLease(incumbent)).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) + + navLeaseCoordinator.pauseCandidate(owner, null) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() + assertThat(fixture.published).containsExactly(previous, owner).inOrder() + } + + @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) + + navLeaseCoordinator.stopCandidate(owner, null) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(navLeaseCoordinator.hasLease(previous)).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) + + verifyNoInteractions(transaction) + } + + @Test + fun `a nav transaction recorded without the lease is ignored`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner) + fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + + navLeaseCoordinator.retainTransaction(owner, transaction) + navLeaseCoordinator.unregisterCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + assertThat(fixture.scope.transaction).isNull() + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + verifyNoInteractions(transaction) + } + + @Test + fun `an unfinished nav transaction is restored when its owner regains the lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + + assertThat(fixture.scope.transaction).isNull() + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(transaction) + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + verify(transaction, never()).finish(any()) + } + + @Test + fun `restoring a nav transaction preserves an unrelated scope transaction`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + fixture.scope.transaction = unrelatedTransaction + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(fixture.scope.transaction).isSameInstanceAs(unrelatedTransaction) + } + + @Test + fun `a finished nav transaction is discarded when its owner regains the lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + whenever(transaction.isFinished).thenReturn(true) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(fixture.scope.transaction).isNull() + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + } + + @Test + fun `a released unfinished nav transaction is not restored when its owner regains the lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + + navLeaseCoordinator.releaseTransaction(owner, transaction) + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.scope.transaction).isNull() + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + } + + @Test + fun `releasing a nav transaction other than the retained one keeps the retained one restorable`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + + navLeaseCoordinator.releaseTransaction(owner, unrelatedTransaction) + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(fixture.scope.transaction).isSameInstanceAs(transaction) + } + + @Test + fun `a displaced owner's retained nav transaction remains identifiable until unregistering`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isTrue() + assertThat(navLeaseCoordinator.isNavTransaction(unrelatedTransaction)).isFalse() + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + } + + @Test + fun `acquiring the nav lease binds its retained transaction before publishing nav data`() { + 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) + + navLeaseCoordinator.pauseCandidate(incoming, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(owner)).isTrue() + assertThat(transactionAtPublication).containsExactly(null, transaction).inOrder() + } + + @Test + fun `unregistering the leaseholder finishes its transaction and clears its published nav data`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + verify(transaction).finish(SpanStatus.OK) + assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.cleared).containsExactly(owner) + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + } + + @Test + fun `unregistering a former leaseholder preserves the current owner's published nav data`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(previous) + fixture.acquireLease(owner, state = Lifecycle.State.RESUMED) + navLeaseCoordinator.pauseCandidate(owner, fixture.scope) + fixture.cleared.clear() + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `unregistering the leaseholder hands the nav lease to the most recently resumed eligible owner`() { + 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) + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() + assertThat(fixture.published.last()).isSameInstanceAs(previous) + assertThat(fixture.cleared).containsExactly(older, previous, owner).inOrder() + } + + @Test + fun `unregistering the leaseholder without a scope releases the nav lease`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(previous) + fixture.acquireLease(owner) + fixture.cleared.clear() + + navLeaseCoordinator.unregisterCandidate(owner, null) + + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(navLeaseCoordinator.hasLease(previous)).isFalse() + assertThat(fixture.published).containsExactly(previous, owner).inOrder() + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `unregistering the leaseholder without a scope doesn't finish its nav transaction`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + fixture.acquireLease(owner, transaction = transaction) + + navLeaseCoordinator.unregisterCandidate(owner, null) + + verify(transaction, never()).finish(any()) + } + + @Test + fun `unregistering a former leaseholder finishes its retained transaction without clearing an unrelated one`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + whenever(transaction.status).thenReturn(SpanStatus.CANCELLED) + fixture.acquireLease(owner, transaction = transaction) + fixture.acquireLease(incoming, state = Lifecycle.State.RESUMED) + fixture.scope.transaction = unrelatedTransaction + fixture.cleared.clear() + + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + + verify(transaction).finish(SpanStatus.CANCELLED) + assertThat(fixture.scope.transaction).isSameInstanceAs(unrelatedTransaction) + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `unregistering the leaseholder forgets the owner when finishing its transaction throws`() { + 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 thrown = + assertThrows(IllegalStateException::class.java) { + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + } + + assertThat(thrown).isSameInstanceAs(failure) + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + + fixture.published.clear() + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + fixture.acquireLease(incoming) + + assertThat(fixture.published).containsExactly(incoming) + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `unregistering the leaseholder forgets the owner when unbinding its transaction throws`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val failure = IllegalStateException("unbind failed") + val failingScope = mock() + fixture.acquireLease(owner, transaction = transaction) + doThrow(failure).whenever(failingScope).withTransaction(any()) + + val thrown = + assertThrows(IllegalStateException::class.java) { + navLeaseCoordinator.unregisterCandidate(owner, failingScope) + } + + assertThat(thrown).isSameInstanceAs(failure) + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + + fixture.published.clear() + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + fixture.acquireLease(incoming) + + assertThat(fixture.published).containsExactly(incoming) + assertThat(fixture.cleared).isEmpty() + } + + @Test + fun `unregistering the leaseholder forgets the owner when clearing its published nav data throws`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val failure = IllegalStateException("clear failed") + fixture.acquireLease(owner, onClearPublishedData = { throw failure }) + + val thrown = + assertThrows(IllegalStateException::class.java) { + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + } + + assertThat(thrown).isSameInstanceAs(failure) + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + + fixture.published.clear() + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + fixture.acquireLease(incoming) + + assertThat(fixture.published).containsExactly(incoming) + assertThat(navLeaseCoordinator.hasLease(incoming)).isTrue() + } + + @Test + fun `unregistering the leaseholder forgets the owner when restoring a fallback transaction throws`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val failure = IllegalStateException("fallback restoration failed") + val failingScope = mock() + fixture.acquireLease(previous, transaction = transaction) + fixture.acquireLease(owner) + doThrow(failure).whenever(failingScope).withTransaction(any()) + + val thrown = + assertThrows(IllegalStateException::class.java) { + navLeaseCoordinator.unregisterCandidate(owner, failingScope) + } + + assertThat(thrown).isSameInstanceAs(failure) + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isTrue() + + fixture.published.clear() + fixture.cleared.clear() + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + fixture.acquireLease(incoming) + + assertThat(fixture.published).containsExactly(incoming) + assertThat(fixture.cleared).doesNotContain(owner) + } + + @Test + fun `unregistering the leaseholder preserves the fallback lease when publishing fallback nav data throws`() { + val fixture = Fixture.create() + val navLeaseCoordinator = fixture.navLeaseCoordinator + val failure = IllegalStateException("fallback publication failed") + var failPublication = false + navLeaseCoordinator.registerCandidate( + previous, + Lifecycle.State.RESUMED, + onPublishData = { + if (failPublication) { + throw failure + } + }, + onClearPublishedData = {}, + ) + navLeaseCoordinator.onBackStackUpdate(previous, fixture.scope) + fixture.acquireLease(owner, transaction = transaction) + failPublication = true + + val thrown = + assertThrows(IllegalStateException::class.java) { + navLeaseCoordinator.unregisterCandidate(owner, fixture.scope) + } + + assertThat(thrown).isSameInstanceAs(failure) + assertThat(navLeaseCoordinator.hasLease(owner)).isFalse() + assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() + assertThat(navLeaseCoordinator.isNavTransaction(transaction)).isFalse() + + fixture.published.clear() + navLeaseCoordinator.resumeCandidate(owner, fixture.scope) + + assertThat(fixture.published).isEmpty() + assertThat(navLeaseCoordinator.hasLease(previous)).isTrue() + } + + private class Fixture { + val navLeaseCoordinator = NavLeaseCoordinator() + val scope = Scope(SentryOptions()) + val published = mutableListOf() + val cleared = mutableListOf() + + fun register( + owner: Any, + state: Lifecycle.State, + onClearPublishedData: (IScope) -> Unit = { cleared += owner }, + ) { + navLeaseCoordinator.registerCandidate( + owner, + state, + onPublishData = { published += owner }, + onClearPublishedData = onClearPublishedData, + ) + } + + 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 + } + } + + companion object { + fun create(): Fixture = Fixture() + } + } +} From b67f3bf1bc2cc524c375a321e3371d012d3cc89a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 22 Oct 2026 14:02:17 +0200 Subject: [PATCH 3/4] ref(android): Add NavScopes (JAVA-785) (#6304) Part 2 of our Nav3 support for multi-Activity and multi-Fragment applications Introduces NavScopes, which lets BackStackObserver talk to a facade that knows when to swizzle between the real, shared IScope used by all SentryNavEffects vs a per-instance staging area. Effects that later gain the right to update the shared IScope can publish their staged values. --- .../compose/navigation3/lifecycle/NavScope.kt | 156 ++++++++ .../navigation3/lifecycle/NavScopes.kt | 375 ++++++++++++++++++ .../compose/navigation3/lifecycle/README.md | 12 + .../navigation3/lifecycle/StagedNavData.kt | 106 +++++ .../navigation3/lifecycle/NavScopeTest.kt | 321 +++++++++++++++ .../navigation3/lifecycle/NavScopesTest.kt | 208 ++++++++++ .../lifecycle/StagedNavDataTest.kt | 221 +++++++++++ 7 files changed, 1399 insertions(+) create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScope.kt create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavData.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavDataTest.kt 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 new file mode 100644 index 0000000000..f67c0b9b32 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScope.kt @@ -0,0 +1,156 @@ +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 new file mode 100644 index 0000000000..92fdb80b52 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopes.kt @@ -0,0 +1,375 @@ +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.SentryLevel +import io.sentry.SentryOptions +import io.sentry.TransactionContext +import io.sentry.TransactionOptions + +/** 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" + +/** + * An [INavScopes] implementation that lets us coordinate updates to navigation state across + * multiple `SentryNavEffect`s. (Also supports the trivial scenario where only a single + * `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: + * ``` + * LifecycleOwner (e.g., Activity or Fragment) + * ↓ owns + * SentryNavEffect + * ↓ + * CoordinatedBackStackObserver (wrapper) + * ├── BackStackObserver — computes nav data and nav transactions + * └── NavScopes — controls if nav data + txns reach the real scope + * ↓ + * NavLeaseCoordinator — arbitrates ownership across all observers + * ↓ + * 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** + * + * 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** + * + * 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 │ + * ``` + * + * **Usage** + * + * 1. Create a `NavScopes` instance per `SentryNavEffect`. + * + * 2. Pass the same [NavLeaseCoordinator] instance to all `NavScopes` whose effects must be + * coordinated (i.e., that target the same `IScope`). + * + * 3. Call [attach] with the lifecycle that manages the effect before the first back stack update. + * + * 4. Call [onBackStackUpdate] before applying each back stack update. + * + * 5. Call [dispose] when the effect leaves the composition. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. Lifecycle callbacks and method invocations should be + * confined to the same thread. + */ +internal class NavScopes( + private val delegate: IScopes, + private val coordinator: NavLeaseCoordinator, +) : INavScopes, LifecycleEventObserver { + + override val options: SentryOptions + get() = delegate.options + + private val owner: Any = Any() + private var attachedLifecycle: Lifecycle? = null + private val stagedNavData = StagedNavData() + + /** + * 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. + */ + 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) { + 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 + } + } + + /** + * Announces that a back stack update is about to be applied, so this instance can acquire the nav + * lease if it's eligible to. + * + * 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. + */ + fun onBackStackUpdate() { + delegate.configureScope { scope -> coordinator.onBackStackUpdate(owner, scope) } + } + + 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) + } + } + } + + /** 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. + */ + fun dispose() { + attachedLifecycle?.removeObserver(this) + attachedLifecycle = null + + // Clear nav state, unbind the nav transaction, and avoid leaking the LifecycleOwner. + withScopeOrNull { coordinator.unregisterCandidate(owner, it) } + } + + /** + * 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 + delegate.configureScope { scope -> + callbackInvoked = true + block(scope) + } + + if (!callbackInvoked) { + block(null) + } + } +} + +/** + * The subset of the [IScopes] interface used to record navigation data. + * + * 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 { + + val options: SentryOptions + + /** Runs [callback] with the current [INavScope], if the SDK provides a scope. */ + fun configureScope(callback: (INavScope) -> Unit) + + /** + * 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]). + */ + fun startTransaction( + transactionContext: TransactionContext, + transactionOptions: TransactionOptions, + ): ITransaction + + fun addBreadcrumb(breadcrumb: Breadcrumb, hint: Hint) +} 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 new file mode 100644 index 0000000000..7d697812ec --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/README.md @@ -0,0 +1,12 @@ +# `io.sentry.compose.navigation3.lifecycle` + +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. + +## 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. 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 new file mode 100644 index 0000000000..964e3694ed --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavData.kt @@ -0,0 +1,106 @@ +package io.sentry.compose.navigation3.lifecycle + +import io.sentry.IScope +import io.sentry.PropagationContext + +/** + * Holds navigation state staged for publication: a screen name and back stack nav context. + * + * 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. + * + * 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. + */ +internal class StagedNavData { + + /** The staged screen name, published or not. */ + var screen: String? = null + private set + + private var navigationContext: Any? = null + + private var hasNavigationContextUpdate = false + private var hasScreenUpdate = false + + /** Whether a propagation context rotation was requested while this instance lacked the lease. */ + private var hasDeferredPropagationContextRotation = false + + // These may differ from the staged values after background updates. We keep the exact published + // references so cleanup preserves host replacements, including structurally equal replacements. + private var publishedNavigationContext: Any? = null + private var publishedScreen: String? = null + + /** Stages [screen] for publication, replacing any previously staged screen. */ + fun stageScreen(screen: String?) { + this.screen = screen + hasScreenUpdate = true + } + + /** Stages [value] as the nav context for publication, replacing any previously staged value. */ + fun stageNavigationContext(value: Any?) { + navigationContext = value + hasNavigationContextUpdate = true + } + + /** Requests a propagation context rotation for when this instance next [publishes][publishTo]. */ + fun deferPropagationContextRotation() { + hasDeferredPropagationContextRotation = true + } + + /** + * Publishes the staged screen and nav context, and performs any deferred propagation context + * 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]. + */ + fun publishTo(realScope: IScope) { + if (hasScreenUpdate) { + publishScreen(realScope) + } + if (hasNavigationContextUpdate) { + publishNavigationContext(realScope) + } + if (hasDeferredPropagationContextRotation) { + realScope.withPropagationContext { realScope.propagationContext = PropagationContext() } + hasDeferredPropagationContextRotation = false + } + } + + /** Publishes the staged screen to [realScope]. */ + fun publishScreen(realScope: IScope) { + publishedScreen = screen + realScope.screen = screen + } + + /** Publishes the staged nav context to [realScope]. */ + fun publishNavigationContext(realScope: IScope) { + publishedNavigationContext = navigationContext + realScope.setContexts(NAVIGATION_CONTEXT_KEY, navigationContext) + } + + /** + * Clears the screen and nav context this instance published, unless something else has replaced + * them in [realScope] since. + */ + fun clearPublishedStateFrom(realScope: IScope) { + if (publishedScreen != null && realScope.screen === publishedScreen) { + realScope.screen = null + } + if ( + publishedNavigationContext != null && + realScope.contexts[NAVIGATION_CONTEXT_KEY] === publishedNavigationContext + ) { + // Use setContexts so scope observers also receive the removal. + realScope.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) + } + publishedScreen = null + publishedNavigationContext = null + } +} 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 new file mode 100644 index 0000000000..be66fb5b16 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopeTest.kt @@ -0,0 +1,321 @@ +package io.sentry.compose.navigation3.lifecycle + +import androidx.lifecycle.Lifecycle +import com.google.common.truth.Truth.assertThat +import io.sentry.ITransaction +import io.sentry.Scope +import io.sentry.ScopeObserverAdapter +import io.sentry.SentryOptions +import org.junit.Assert.assertThrows +import org.junit.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever + +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 screen = navScope.screen + + assertThat(screen).isEqualTo("Host") + } + + @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 screen = navScope.screen + + assertThat(screen).isEqualTo("Staged") + } + + @Test + fun `setScreen() - publishes the screen with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val navScope = fixture.navScope + + navScope.screen = "Profile" + + assertThat(fixture.realScope.screen).isEqualTo("Profile") + } + + @Test + fun `setScreen() - stages the screen without the nav lease`() { + val fixture = Fixture.create() + val navScope = fixture.navScope + fixture.realScope.screen = "Host" + + navScope.screen = "Profile" + + assertThat(fixture.realScope.screen).isEqualTo("Host") + assertThat(navScope.screen).isEqualTo("Profile") + } + + @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") + + navScope.setNavigationContext(navigationContext) + + assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]) + .isSameInstanceAs(navigationContext) + } + + @Test + fun `setNavigationContext() - stages the navigation context without the nav lease`() { + val fixture = Fixture.create() + val navScope = fixture.navScope + val hostContext = mapOf("route" to "Host") + val navigationContext = mapOf("route" to "Profile") + fixture.realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + + navScope.setNavigationContext(navigationContext) + + assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + fixture.stagedNavData.publishTo(fixture.realScope) + assertThat(fixture.realScope.contexts[NAVIGATION_CONTEXT_KEY]) + .isSameInstanceAs(navigationContext) + } + + @Test + fun `rotatePropagationContext() - replaces the real context with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val realContext = fixture.realScope.propagationContext + + fixture.navScope.rotatePropagationContext() + + assertThat(fixture.realScope.propagationContext).isNotSameInstanceAs(realContext) + } + + @Test + fun `rotatePropagationContext() - defers rotating the real context without the nav lease`() { + val fixture = Fixture.create() + val realContext = fixture.realScope.propagationContext + + fixture.navScope.rotatePropagationContext() + + assertThat(fixture.realScope.propagationContext).isSameInstanceAs(realContext) + fixture.stagedNavData.publishTo(fixture.realScope) + assertThat(fixture.realScope.propagationContext).isNotSameInstanceAs(realContext) + } + + @Test + fun `bindTransaction() - binds to a free scope with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val transaction = mock() + + fixture.navScope.bindTransaction(transaction) + + assertThat(fixture.realScope.transaction).isSameInstanceAs(transaction) + } + + @Test + fun `bindTransaction() - preserves an already bound transaction`() { + val fixture = Fixture.create(ownsLease = true) + val bound = mock() + fixture.realScope.transaction = bound + + fixture.navScope.bindTransaction(mock()) + + assertThat(fixture.realScope.transaction).isSameInstanceAs(bound) + } + + @Test + fun `bindTransaction() - does nothing without the nav lease`() { + val fixture = Fixture.create() + + fixture.navScope.bindTransaction(mock()) + + assertThat(fixture.realScope.transaction).isNull() + } + + @Test + fun `unbindTransaction() - unbinds the given transaction without the nav lease`() { + val fixture = Fixture.create() + val transaction = mock() + fixture.realScope.transaction = transaction + + fixture.navScope.unbindTransaction(transaction) + + assertThat(fixture.realScope.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) + + fixture.navScope.unbindTransaction(transaction) + + assertThat(fixture.coordinator.isNavTransaction(transaction)).isFalse() + } + + @Test + fun `unbindTransaction() - releases the owner's retained nav transaction without the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + val transaction = mock() + fixture.retainTransaction(transaction) + fixture.giveLeaseToAnotherOwner() + + fixture.navScope.unbindTransaction(transaction) + + assertThat(fixture.coordinator.isNavTransaction(transaction)).isFalse() + } + + @Test + fun `unbindTransaction() - releases the retained nav transaction when a scope observer throws`() { + 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() + } + } + ) + + assertThrows(ThrowingScopeObserverException::class.java) { + fixture.navScope.unbindTransaction(transaction) + } + + assertThat(fixture.coordinator.isNavTransaction(transaction)).isFalse() + } + + @Test + fun `unbindTransaction() - preserves a different bound transaction`() { + val fixture = Fixture.create(ownsLease = true) + val bound = mock() + fixture.realScope.transaction = bound + + fixture.navScope.unbindTransaction(mock()) + + assertThat(fixture.realScope.transaction).isSameInstanceAs(bound) + } + + @Test + fun `clearFinishedTransaction() - unbinds a finished transaction with the nav lease`() { + val fixture = Fixture.create(ownsLease = true) + fixture.realScope.transaction = + mock().also { whenever(it.isFinished).thenReturn(true) } + + fixture.navScope.clearFinishedTransaction() + + assertThat(fixture.realScope.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 + + fixture.navScope.clearFinishedTransaction() + + assertThat(fixture.realScope.transaction).isSameInstanceAs(finished) + } + + @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 + + val transaction = fixture.navScope.transaction + + assertThat(transaction).isNull() + } + + @Test + fun `transaction - reports a non-nav transaction without the nav lease`() { + val fixture = Fixture.create() + val hostTransaction = mock() + fixture.realScope.transaction = hostTransaction + + val transaction = fixture.navScope.transaction + + assertThat(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 + + val transaction = fixture.navScope.transaction + + assertThat(transaction).isSameInstanceAs(boundTransaction) + } + + /** 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)) + } + + /** 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)) + } + + companion object { + + fun create(ownsLease: Boolean = false): Fixture { + val realScope = Scope(SentryOptions.empty()) + val stagedNavData = StagedNavData() + val coordinator = NavLeaseCoordinator() + val owner = Any() + coordinator.registerCandidate( + owner, + Lifecycle.State.RESUMED, + stagedNavData::publishTo, + stagedNavData::clearPublishedStateFrom, + ) + if (ownsLease) { + coordinator.onBackStackUpdate(owner, realScope) + } + + 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 new file mode 100644 index 0000000000..4470072b35 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/NavScopesTest.kt @@ -0,0 +1,208 @@ +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.Breadcrumb +import io.sentry.Hint +import io.sentry.IScopes +import io.sentry.ITransaction +import io.sentry.Scope +import io.sentry.ScopeCallback +import io.sentry.SentryOptions +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +class NavScopesTest { + + @Test + fun `attach() - ignores a second, different lifecycle`() { + val fixture = Fixture() + val navScopes = fixture.navScopes() + val first = TestLifecycleOwner() + val second = TestLifecycleOwner() + navScopes.attach(first.lifecycle) + + navScopes.attach(second.lifecycle) + + assertThat(first.lifecycle.observerCount).isEqualTo(1) + assertThat(second.lifecycle.observerCount).isEqualTo(0) + } + + @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() + + incoming.dispose() + + assertThat(fixture.hasLease(outgoing)).isTrue() + } + + @Test + fun `onBackStackUpdate() - an update skipped by the scope does not count as the first update`() { + val fixture = Fixture() + val navScopes = fixture.navScopes() + val owner = TestLifecycleOwner() + navScopes.attach(owner.lifecycle) + fixture.isEnabled = false + navScopes.applyFirstBackStackUpdate() + fixture.isEnabled = true + + navScopes.applyFirstBackStackUpdate() + + assertThat(fixture.hasLease(navScopes)).isTrue() + } + + @Test + fun `addBreadcrumb() - records breadcrumbs immediately without the nav lease`() { + val fixture = Fixture() + val leaseholder = fixture.navScopes() + leaseholder.attach(TestLifecycleOwner().lifecycle) + leaseholder.applyFirstBackStackUpdate() + val other = fixture.navScopes() + other.attach(TestLifecycleOwner().lifecycle) + val breadcrumb = Breadcrumb() + val hint = Hint() + + other.addBreadcrumb(breadcrumb, hint) + + assertThat(fixture.hasLease(other)).isFalse() + verify(fixture.scopes).addBreadcrumb(breadcrumb, hint) + } + + @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() + + val transaction = + covered.startTransaction(TransactionContext("name", "op"), TransactionOptions()) + + assertThat(fixture.hasLease(covered)).isFalse() + assertThat(transaction.isNoOp).isTrue() + 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() + val incomingContext = fixture.scope.propagationContext + + covered.startTransaction(TransactionContext("name", "op"), TransactionOptions()) + assertThat(fixture.scope.propagationContext).isSameInstanceAs(incomingContext) + + 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() + val navScopes = fixture.navScopes() + navScopes.attach(TestLifecycleOwner().lifecycle) + navScopes.applyFirstBackStackUpdate() + navScopes.dispose() + + val transaction = + navScopes.startTransaction(TransactionContext("name", "op"), TransactionOptions()) + + assertThat(transaction.isNoOp).isTrue() + } + + @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 + + covered.configureScope { it.rotatePropagationContext() } + assertThat(fixture.scope.propagationContext).isSameInstanceAs(incomingContext) + + incoming.dispose() + + assertThat(fixture.hasLease(covered)).isTrue() + assertThat(fixture.scope.propagationContext).isNotSameInstanceAs(incomingContext) + } + + private fun NavScopes.applyFirstBackStackUpdate() { + onBackStackUpdate() + } + + private class TestLifecycleOwner(initialState: Lifecycle.State = Lifecycle.State.CREATED) : + LifecycleOwner { + override val lifecycle: LifecycleRegistry = + LifecycleRegistry.createUnsafe(this).apply { currentState = initialState } + } + + private class Fixture { + var isEnabled = true + val scope = Scope(SentryOptions()) + val coordinator = NavLeaseCoordinator() + val scopes = + mock().also { scopes -> + whenever(scopes.options).thenReturn(SentryOptions()) + doAnswer { + if (isEnabled) { + (it.arguments[0] as ScopeCallback).run(scope) + } + null + } + .whenever(scopes) + .configureScope(any()) + whenever(scopes.startTransaction(any(), any())) + .thenAnswer { mock() } + } + + fun navScopes(): NavScopes = NavScopes(scopes, coordinator) + + private var leaseProbeCount = 0 + + /** + * Whether [navScopes] holds the nav lease, observed through whether a screen name it sets + * reaches the real [scope]. Overwrites the scope's screen. + */ + fun hasLease(navScopes: NavScopes): Boolean { + val probe = "lease probe ${++leaseProbeCount}" + navScopes.configureScope { it.screen = probe } + return scope.screen == probe + } + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavDataTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavDataTest.kt new file mode 100644 index 0000000000..a5b74ff4a2 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/lifecycle/StagedNavDataTest.kt @@ -0,0 +1,221 @@ +package io.sentry.compose.navigation3.lifecycle + +import com.google.common.truth.Truth.assertThat +import io.sentry.Scope +import io.sentry.SentryOptions +import org.junit.Test + +class StagedNavDataTest { + + private val realScope = Scope(SentryOptions.empty()) + private val stagedNavData = StagedNavData() + + @Test + fun `stageScreen() - stages the screen without changing the real scope`() { + realScope.screen = "Host" + + stagedNavData.stageScreen("Profile") + + assertThat(stagedNavData.screen).isEqualTo("Profile") + assertThat(realScope.screen).isEqualTo("Host") + } + + @Test + fun `publishScreen() - publishes the staged screen`() { + stagedNavData.stageScreen("Profile") + + stagedNavData.publishScreen(realScope) + + assertThat(realScope.screen).isEqualTo("Profile") + } + + @Test + fun `stageNavigationContext() - leaves the real navigation context unchanged`() { + val hostContext = mapOf("route" to "Host") + realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + + stagedNavData.stageNavigationContext(mapOf("route" to "Profile")) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + } + + @Test + fun `publishNavigationContext() - publishes the staged navigation context`() { + val navigationContext = mapOf("route" to "Profile") + stagedNavData.stageNavigationContext(navigationContext) + + stagedNavData.publishNavigationContext(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) + } + + @Test + fun `publishTo() - publishes the staged screen`() { + stagedNavData.stageScreen("Profile") + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isEqualTo("Profile") + } + + @Test + fun `publishTo() - republishes an already published screen that was since replaced`() { + stagedNavData.stageScreen("Profile") + stagedNavData.publishScreen(realScope) + realScope.screen = "Host" + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isEqualTo("Profile") + } + + @Test + fun `publishTo() - leaves the real screen alone when no screen was staged`() { + realScope.screen = "Host" + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isEqualTo("Host") + } + + @Test + fun `publishTo() - clears the real screen when the staged screen is null`() { + realScope.screen = "Previous" + stagedNavData.stageScreen(null) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.screen).isNull() + } + + @Test + fun `publishTo() - publishes the staged navigation context`() { + val navigationContext = mapOf("route" to "Profile") + stagedNavData.stageNavigationContext(navigationContext) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) + } + + @Test + fun `publishTo() - republishes an already published navigation context that was since replaced`() { + val navigationContext = mapOf("route" to "Profile") + stagedNavData.stageNavigationContext(navigationContext) + stagedNavData.publishNavigationContext(realScope) + realScope.setContexts(NAVIGATION_CONTEXT_KEY, mapOf("route" to "Host")) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(navigationContext) + } + + @Test + fun `publishTo() - leaves the real navigation context alone when none was staged`() { + val hostContext = mapOf("route" to "Host") + realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + } + + @Test + fun `publishTo() - clears the real navigation context when the staged context is null`() { + realScope.setContexts(NAVIGATION_CONTEXT_KEY, mapOf("route" to "Previous")) + stagedNavData.stageNavigationContext(null) + + stagedNavData.publishTo(realScope) + + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isNull() + } + + @Test + fun `clearPublishedStateFrom() - preserves a host screen set after publication without a staged screen`() { + stagedNavData.publishTo(realScope) + realScope.screen = "Host" + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isEqualTo("Host") + } + + @Test + fun `clearPublishedStateFrom() - clears screen and navigation context published by this holder`() { + stagedNavData.stageScreen("Profile") + stagedNavData.stageNavigationContext(mapOf("route" to "Profile")) + stagedNavData.publishTo(realScope) + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isNull() + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isNull() + } + + @Test + fun `clearPublishedStateFrom() - clears the published screen and navigation context after newer values are staged`() { + stagedNavData.stageScreen("Home") + stagedNavData.stageNavigationContext(mapOf("route" to "Home")) + stagedNavData.publishTo(realScope) + stagedNavData.stageScreen("Profile") + stagedNavData.stageNavigationContext(mapOf("route" to "Profile")) + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isNull() + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isNull() + } + + @Test + fun `clearPublishedStateFrom() - preserves equal but distinct host replacements`() { + stagedNavData.stageScreen(String(charArrayOf('H', 'o', 'm', 'e'))) + stagedNavData.stageNavigationContext(mapOf("route" to "Home")) + stagedNavData.publishTo(realScope) + val hostScreen = String(charArrayOf('H', 'o', 'm', 'e')) + val hostContext = mapOf("route" to "Home") + realScope.screen = hostScreen + realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isSameInstanceAs(hostScreen) + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + } + + @Test + fun `clearPublishedStateFrom() - preserves the real screen and navigation context before publication`() { + realScope.screen = "Host" + val hostContext = mapOf("route" to "Host") + realScope.setContexts(NAVIGATION_CONTEXT_KEY, hostContext) + stagedNavData.stageScreen("Staged") + stagedNavData.stageNavigationContext(mapOf("route" to "Staged")) + + stagedNavData.clearPublishedStateFrom(realScope) + + assertThat(realScope.screen).isEqualTo("Host") + assertThat(realScope.contexts[NAVIGATION_CONTEXT_KEY]).isSameInstanceAs(hostContext) + } + + @Test + fun `publishTo() - leaves the real propagation context alone when no rotation was deferred`() { + val realContext = realScope.propagationContext + + stagedNavData.publishTo(realScope) + + assertThat(realScope.propagationContext).isSameInstanceAs(realContext) + } + + @Test + fun `publishTo() - performs a deferred propagation context rotation once`() { + stagedNavData.deferPropagationContextRotation() + stagedNavData.deferPropagationContextRotation() + val realContext = realScope.propagationContext + + stagedNavData.publishTo(realScope) + val rotatedContext = realScope.propagationContext + stagedNavData.publishTo(realScope) + + assertThat(rotatedContext).isNotSameInstanceAs(realContext) + assertThat(realScope.propagationContext).isSameInstanceAs(rotatedContext) + } +} From 723b40a1d3a69163675eea19de6d8bcb28adb551 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 22 Oct 2026 14:45:14 +0200 Subject: [PATCH 4/4] feat(android): Coordinate SentryNavEffect access to current IScope (JAVA-785) (#6305) Part 3 of our Nav3 support for multi-Activity and multi-Fragment applications Introduces CoordinatedBackStackObserver, which lets us inject the new NavScopes version of IScopes into BackStackObserver. (NavScopes is a facade that knows when to toggle between the real IScope instance shared by all SentryNavEffects vs a per-effect staging area.) --- CHANGELOG.md | 5 + sentry-android-navigation3/README.md | 2 +- .../compose/navigation3/BackStackConverter.kt | 4 +- .../compose/navigation3/BackStackObserver.kt | 118 +++---- .../CoordinatedBackStackObserver.kt | 55 ++++ .../compose/navigation3/SentryNavEffect.kt | 40 ++- .../navigation3/BackStackObserverTest.kt | 69 +++- .../CoordinatedBackStackObserverTest.kt | 174 ++++++++++ .../navigation3/SentryNavEffectTest.kt | 301 ++++++++++++++++++ 9 files changed, 675 insertions(+), 93 deletions(-) create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index acca00fc3c..ffd5de689a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +### Improvements + +- Support Navigation 3 integration for multi-Activity and multi-Fragment applications, where each Activity or Fragment has its own `SentryNavEffect` ([#6305](https://github.com/getsentry/sentry-java/pull/6305)) + - See the [Navigation for Android docs](https://docs.sentry.io/platforms/android/integrations/navigation/#supported-setups) for a detailed list of supported vs unsupported app setups. + ### Fixes - Report the number of dropped spans in client reports when a batched span envelope item is discarded, instead of a quantity of 1 (e.g. for Flutter span streaming) ([#6265](https://github.com/getsentry/sentry-java/pull/6265)) diff --git a/sentry-android-navigation3/README.md b/sentry-android-navigation3/README.md index bdfec781fc..95bbbeb006 100644 --- a/sentry-android-navigation3/README.md +++ b/sentry-android-navigation3/README.md @@ -9,4 +9,4 @@ Because Nav3 back stacks can have arbitrary key types, host apps need to provide keys and the data Sentry should display for each (see `BackStackEntryMapper`). See the [Navigation for Android docs](https://docs.sentry.io/platforms/android/integrations/navigation/) for additional details, including -installation, configuration, and limitations. +configuration, supported setups, and limitations. 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 f567f7c8c5..6100c70620 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 @@ -395,7 +395,9 @@ internal data class NormalizedSentryBackStackEntry( val name: String, /** Sanitized [SentryBackStackEntry.arguments] (i.e., bounded in size and depth). */ val arguments: Map = emptyMap(), - /** The reason why the SDK dropped host-provided arguments. */ + /** + * The reason why the SDK dropped host-provided arguments, or null if no arguments were dropped. + */ val argumentDropReason: ArgumentDropReason? = null, ) { diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 37f694350f..8e2806686e 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,13 +2,11 @@ package io.sentry.compose.navigation3 import io.sentry.Breadcrumb import io.sentry.Hint -import io.sentry.IScope -import io.sentry.IScopes import io.sentry.ITransaction -import io.sentry.PropagationContext 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 @@ -16,6 +14,9 @@ import io.sentry.compose.navigation3.BackStackConverter.RetentionPolicy import io.sentry.compose.navigation3.PreparedChange.BackStackHasNewTop import io.sentry.compose.navigation3.PreparedChange.BackStackHasSameTop import io.sentry.compose.navigation3.PreparedChange.BackStackIsEmpty +import io.sentry.compose.navigation3.lifecycle.INavScope +import io.sentry.compose.navigation3.lifecycle.INavScopes +import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion @@ -47,10 +48,11 @@ private const val NAVIGATION_OP: String = "navigation" * **Thread safety** * * This class is ***not*** thread-safe. Clients should serialize calls to [onBackStackChanged] and - * [cleanup] (e.g., via invocation from an `*Effect` or another form of thread confinement). + * [dispose] (e.g., via invocation from an `*Effect` or another form of thread confinement). */ +@Suppress("TooManyFunctions") internal class BackStackObserver( - private val scopes: IScopes, + private val scopes: INavScopes, private val options: SentryNavOptions, entryMapper: ForwardingBackStackEntryMapper, ) { @@ -110,23 +112,14 @@ internal class BackStackObserver( scopes.configureScope { scope -> applyChange(scope, change) } } - internal fun cleanup() { - previousTop = null - previousTopNormalized = null - - scopes.configureScope { scope -> - navTransaction.stop(scope) - navScreen.clear(scope) - - if (options.captureBackStack) { - // This observer owns the nav context while it's in the composition, and cleanup removes - // it to avoid leaking stale back stack data after observation stops. If the host app - // replaces one observer with another, there may be a brief gap where events lack nav - // context. Apps should keep the observer at the nav root so cleanup only runs when the - // navigation session is ending, not during normal destination changes. - navContext.clear(scope) - } - } + /** + * Clears the nav data this observer recorded. + * + * Call it when observation ends so that no stale screen, nav context, or nav transaction outlives + * it. + */ + internal fun dispose() { + scopes.configureScope { scope -> tearDown(scope) } } private fun prepareChange(backStack: List): PreparedChange { @@ -140,7 +133,7 @@ internal class BackStackObserver( } } - private fun applyChange(scope: IScope, change: PreparedChange) { + private fun applyChange(scope: INavScope, change: PreparedChange) { when (change) { is BackStackIsEmpty -> handleEmptyBackStack(scope) is BackStackHasNewTop -> handleNewTop(scope, change.previousTop, change.backStack) @@ -185,7 +178,7 @@ internal class BackStackObserver( } private fun handleNewTop( - scope: IScope, + scope: INavScope, previousTop: NormalizedSentryBackStackEntry?, currentBackStack: BackStackData, ) { @@ -212,23 +205,19 @@ internal class BackStackObserver( ) ?.let { transaction -> navContext.updateTransaction(transaction, scope, currentBackStack) } } else { - // Rotate the propagation context. - scope.withPropagationContext { scope.setPropagationContext(PropagationContext()) } + scope.rotatePropagationContext() } storeAsPreviousTop(currentBackStack.topEntry, currentBackStack.topEntryNormalized) } - private fun handleSameTop(scope: IScope, backStack: BackStackData) { + private fun handleSameTop(scope: INavScope, backStack: BackStackData) { navContext.update(scope, backStack.capturedEntriesNormalized) storeAsPreviousTop(backStack.topEntry, backStack.topEntryNormalized) } - private fun handleEmptyBackStack(scope: IScope) { - navTransaction.stop(scope) - navContext.clear(scope) - navScreen.clear(scope) - clearPreviousTop() + private fun handleEmptyBackStack(scope: INavScope) { + tearDown(scope) } private fun storeAsPreviousTop(topEntry: T, topEntryNormalized: NormalizedSentryBackStackEntry) { @@ -240,12 +229,19 @@ internal class BackStackObserver( previousTop = null previousTopNormalized = null } + + private fun tearDown(scope: INavScope) { + navTransaction.stop(scope) + navContext.clear(scope) + navScreen.clear(scope) + clearPreviousTop() + } } /** * A model for applying one back stack update. * - * Lets us separate change preparation from its application so that the [IScopes.configureScope] + * Lets us separate change preparation from its application so that the [INavScopes.configureScope] * callback in charge of application can use already-computed navigation state. Otherwise, any * exceptions thrown during state computation would be swallowed by `configureScope`'s over-broad * `catch` clause. @@ -281,7 +277,7 @@ private data class BackStackData( ) /** A helper class for managing nav transactions. */ -private class NavTransaction(private val scopes: IScopes) { +private class NavTransaction(private val scopes: INavScopes) { private companion object { private const val TRANSACTION_ORIGIN = "auto.navigation.nav3" @@ -291,11 +287,11 @@ private class NavTransaction(private val scopes: IScopes) { /** Starts an idle navigation transaction, or no-ops if another transaction is already active. */ fun start( - scope: IScope, + scope: INavScope, name: String, arguments: Map, ): ITransaction? { - clearTransactionIfFinished(scope) + scope.clearFinishedTransaction() if (scope.transaction != null) { scopes.options.logger.log( @@ -323,6 +319,7 @@ private class NavTransaction(private val scopes: IScopes) { transactionOptions, ) + // Not an error: INavScopes.startTransaction may decline to start a transaction by design. if (transaction.isNoOp) { return null } @@ -335,61 +332,39 @@ private class NavTransaction(private val scopes: IScopes) { } } - scope.withTransaction { tx -> - if (tx == null) { - scope.transaction = transaction - } - } + scope.bindTransaction(transaction) return transaction } /** Finishes and unsets the active navigation transaction, if one exists. */ - fun stop(scope: IScope) { + fun stop(scope: INavScope) { val transaction = activeNavTransaction ?: return val status = transaction.status ?: SpanStatus.OK transaction.finish(status) - - scope.withTransaction { tx -> - if (tx == transaction) { - scope.clearTransaction() - } - } - + scope.unbindTransaction(transaction) activeNavTransaction = null } - - /** Clears a stale finished transaction that's still bound to the default scope. */ - private fun clearTransactionIfFinished(scope: IScope) { - scope.withTransaction { tx -> - if (tx?.isFinished == true) { - scope.clearTransaction() - } - } - } } /** A helper class for updating [nav context][NAVIGATION_CONTEXT_KEY]. */ -private class NavContext(private val scopes: IScopes, private val options: SentryNavOptions) { +private class NavContext(private val scopes: INavScopes, private val options: SentryNavOptions) { private companion object { private const val BACKSTACK_KEY = "Back Stack" - private const val NAVIGATION_CONTEXT_KEY = "Navigation" } - fun update(scope: IScope, backStackEntries: List) { + fun update(scope: INavScope, backStackEntries: List) { if (backStackEntries.isEmpty()) { clear(scope) return } - scope.setContexts(NAVIGATION_CONTEXT_KEY, backStackEntries.toBackStackMap()) + scope.setNavigationContext(backStackEntries.toBackStackMap()) } - fun clear(scope: IScope) { - // We purposefully don't call IScope.removeContexts(), as it doesn't notify IScopeObserver and - // therefore doesn't write its updates to disk ¯\_ (ツ)_/¯. - scope.setContexts(NAVIGATION_CONTEXT_KEY, null as Any?) + fun clear(scope: INavScope) { + scope.setNavigationContext(null) } /** @@ -403,12 +378,11 @@ private class NavContext(private val scopes: IScopes, private val options: Sentr */ fun updateTransaction( transaction: ITransaction, - scope: IScope, + scope: INavScope, backStack: BackStackData, ) { if (scopes.options.isEnableScreenTracking) { - val appContext = - transaction.contexts.app ?: io.sentry.protocol.Contexts(scope.contexts).app ?: App() + val appContext = transaction.contexts.app ?: scope.app ?: App() appContext.viewNames = listOf(backStack.topEntryNormalized.name) transaction.contexts.setApp(appContext) @@ -432,12 +406,12 @@ private class NavScreen { private var lastScreenName: String? = null - fun update(scope: IScope, currentEntry: NormalizedSentryBackStackEntry) { + fun update(scope: INavScope, currentEntry: NormalizedSentryBackStackEntry) { scope.screen = currentEntry.name lastScreenName = currentEntry.name } - fun clear(scope: IScope) { + fun clear(scope: INavScope) { val screenName = lastScreenName ?: return if (scope.screen == screenName) { scope.screen = null @@ -447,7 +421,7 @@ private class NavScreen { } /** A helper class for generating nav breadcrumbs. */ -private class NavBreadcrumbs(private val scopes: IScopes) { +private class NavBreadcrumbs(private val scopes: INavScopes) { fun emit(fromEntry: NormalizedSentryBackStackEntry?, toEntry: NormalizedSentryBackStackEntry) { val breadcrumb = 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 new file mode 100644 index 0000000000..8cf9c6e1be --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserver.kt @@ -0,0 +1,55 @@ +package io.sentry.compose.navigation3 + +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleEventObserver +import io.sentry.IScope +import io.sentry.IScopes +import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator +import io.sentry.compose.navigation3.lifecycle.NavScopes + +/** + * A [BackStackObserver] whose updates to a shared mutable [IScope] are coordinated with other + * `BackStackObserver`s. + * + * Coordination is achieved via [NavScopes]. + * + * **Usage** + * + * Call [attach] before the first [onBackStackChanged], and call [dispose] when observation ends. + * + * Instances targeting the same [IScope] must be created with the same [NavLeaseCoordinator]. + * + * **Thread safety** + * + * This class is ***not*** thread-safe. Invocations should be confined to the same thread as the + * [LifecycleEventObserver] callbacks implemented by [NavScopes]. + */ +internal class CoordinatedBackStackObserver( + scopes: IScopes, + options: SentryNavOptions, + entryMapper: ForwardingBackStackEntryMapper, + coordinator: NavLeaseCoordinator, +) { + + private val navScopes = NavScopes(scopes, coordinator) + private val observer = BackStackObserver(navScopes, options, entryMapper) + + fun attach(lifecycle: Lifecycle) { + navScopes.attach(lifecycle) + } + + fun onBackStackChanged(backStack: List) { + navScopes.onBackStackUpdate() + observer.onBackStackChanged(backStack) + } + + fun dispose() { + // Call disposal on both the observer and navScopes in case their (currently overlapping) + // implementations drift over time. + observer.dispose() + navScopes.dispose() + } +} + +/** The [NavLeaseCoordinator] shared by default by all [SentryNavEffect]s. */ +internal val defaultNavLeaseCoordinator: NavLeaseCoordinator = NavLeaseCoordinator() diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt index 8b6269752b..c666122efc 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt @@ -4,9 +4,11 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState +import androidx.lifecycle.compose.LocalLifecycleOwner import io.sentry.IScopes import io.sentry.ScopesAdapter import io.sentry.SentryOptions +import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator import org.jetbrains.annotations.ApiStatus /** @@ -41,8 +43,7 @@ import org.jetbrains.annotations.ApiStatus * * **Data generated** * - * By default, the following data is produced every time the top of the provided [backStack] - * changes: + * By default, the following are produced every time the top of the provided [backStack] changes: * * - a new navigation transaction (unless another transaction is already active on the current * scope) @@ -55,20 +56,23 @@ import org.jetbrains.annotations.ApiStatus * * **Limitations** * - * `SentryNavEffect` generates all Sentry data based solely on the top entry of your back stack. In - * particular, it has no awareness of + * *Tracks the back stack, not scenes*: `SentryNavEffect` generates all Sentry data based solely on + * the top entry of your back stack. In particular, it has no awareness of * [Scenes](https://developer.android.com/guide/navigation/navigation-3/scenes). Transaction routes, * breadcrumbs, and screen names are all derived from the top entry of the back stack and are * updated as it changes. * - * `SentryNavEffect` doesn't make any special accommodations for + * *No accommodation for predictive back*: `SentryNavEffect` doesn't make any special accommodations + * for * [predictive back](https://developer.android.com/guide/navigation/custom-back/predictive-back-gesture) * gestures. That means that spans produced by predictively rendered composables can show up under * the current destination's transaction. * - * Multiple simultaneously active `SentryNavEffect` instances writing to the same Sentry scope are - * not supported. Violating this restriction can result in interleaved breadcrumbs, clobbered screen - * names and back stacks, and transactions that interfere with one another. + * *Limited support for simultaneously active `SentryNavEffect`s*: Multiple `SentryNavEffect`s are + * supported only in certain setups: roughly, one effect per Activity or Fragment, placed next to + * (and not inside) its `NavDisplay`. See the + * [Navigation for Android docs](https://docs.sentry.io/platforms/android/integrations/navigation/#supported-setups) + * for a detailed list of supported vs unsupported configurations. * * @param backStack The navigation backstack to observe. * @param backStackEntryMapper Maps each entry of the [backStack] to a name and optional arguments @@ -87,6 +91,7 @@ public fun SentryNavEffect( backStackEntryMapper = backStackEntryMapper, options = options, scopes = ScopesAdapter.getInstance(), + coordinator = defaultNavLeaseCoordinator, ) } @@ -94,20 +99,29 @@ public fun SentryNavEffect( internal fun SentryNavEffect( backStack: List, backStackEntryMapper: BackStackEntryMapper, - options: SentryNavOptions = SentryNavOptions(), + options: SentryNavOptions, scopes: IScopes, + coordinator: NavLeaseCoordinator, ) { val currentBackStackEntryMapper = rememberUpdatedState(backStackEntryMapper) + val lifecycle = LocalLifecycleOwner.current.lifecycle val observer = - remember(scopes, options) { - BackStackObserver( + remember(scopes, options, lifecycle, coordinator) { + CoordinatedBackStackObserver( scopes = scopes, options = options, entryMapper = ForwardingBackStackEntryMapper { currentBackStackEntryMapper.value }, + coordinator = coordinator, ) } + // Register first so the initial prepared back stack update can acquire navigation ownership. + DisposableEffect(observer) { + observer.attach(lifecycle) + onDispose { observer.dispose() } + } + // Intentionally don't remember this copy. Snapshot-backed lists mutate in place, so // remember(backStack) { backStack.toList() } would cache a stale copy. (The key reference // retained by remember() and the backStack reference passed to this effect would point to the @@ -120,10 +134,6 @@ internal fun SentryNavEffect( observer.onBackStackChanged(backStack = copy) onDispose {} } - - DisposableEffect(observer) { - onDispose { observer.cleanup() } - } } /** diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index 0c7cdc9f53..d331afabe6 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -1,5 +1,8 @@ package io.sentry.compose.navigation3 +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry import com.google.common.truth.Truth.assertThat import io.sentry.Breadcrumb import io.sentry.Hint @@ -16,6 +19,9 @@ import io.sentry.SentryTracer import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.compose.navigation3.ArgumentDropReason.Companion.ARGUMENT_DROP_REASON_KEY +import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY +import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator +import io.sentry.compose.navigation3.lifecycle.NavScopes import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource import org.junit.Test @@ -90,7 +96,7 @@ class BackStackObserverTest { scope.options.isEnableScreenTracking = config.enableScreenTracking return BackStackObserver( - scopes = scopes, + scopes = leaseholderNavScopes(), options = SentryNavOptions { enableNavigationBreadcrumbs = config.enableNavigationBreadcrumbs @@ -102,6 +108,23 @@ class BackStackObserverTest { ) } + /** + * Real [NavScopes] that hold the nav lease for the whole test, so [BackStackObserver] writes + * straight through to [scope] without lease coordination getting in the way. + */ + private fun leaseholderNavScopes(): NavScopes { + val owner = + object : LifecycleOwner { + override val lifecycle = + LifecycleRegistry.createUnsafe(this).apply { currentState = Lifecycle.State.RESUMED } + } + return NavScopes(scopes, NavLeaseCoordinator()).apply { + attach(owner.lifecycle) + // The sole resumed owner's first update acquires the lease, which it then keeps. + onBackStackUpdate() + } + } + private companion object { fun createOptions(logger: ILogger): SentryOptions = SentryOptions().apply { @@ -741,22 +764,60 @@ class BackStackObserverTest { } @Test - fun `cleanup clears observer owned tracked state`() { + fun `dispose finishes the active nav transaction and unbinds it from the scope`() { val fixture = Fixture() val sut = fixture.getSut() - sut.onBackStackChanged(listOf(HomeScreen())) val transaction = fixture.startedTransactions.single() - sut.cleanup() + sut.dispose() assertThat(transaction.isFinished).isTrue() assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `dispose clears the screen name and back stack context it recorded`() { + val fixture = Fixture() + val sut = fixture.getSut() + sut.onBackStackChanged(listOf(HomeScreen(), ProfileScreen("123"))) + + sut.dispose() + assertThat(fixture.scope.screen).isNull() assertThat(fixture.scope.contexts.app?.viewNames).isNull() assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() } + @Test + fun `dispose keeps a screen name that was set after the observer recorded its own`() { + val fixture = Fixture() + val sut = fixture.getSut() + sut.onBackStackChanged(listOf(HomeScreen())) + fixture.scope.screen = "/HostScreen" + + sut.dispose() + + assertThat(fixture.scope.screen).isEqualTo("/HostScreen") + } + + @Test + fun `dispose makes the next back stack update a first navigation with no from entry`() { + val fixture = Fixture() + val sut = fixture.getSut() + val home = HomeScreen() + sut.onBackStackChanged(listOf(home)) + + sut.dispose() + sut.onBackStackChanged(listOf(home)) + + assertThat(fixture.breadcrumbs).hasSize(2) + assertThat(fixture.breadcrumbs.last().data).containsExactly("to", "/HomeScreen") + assertThat(fixture.startedTransactions).hasSize(2) + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.last()) + assertThat(fixture.scope.screen).isEqualTo("/HomeScreen") + } + private fun IScope.navigationBackStack(): List>? { val navigationContext = contexts[NAVIGATION_CONTEXT_KEY] as? Map<*, *> ?: return null 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 new file mode 100644 index 0000000000..08c2003fbb --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/CoordinatedBackStackObserverTest.kt @@ -0,0 +1,174 @@ +package io.sentry.compose.navigation3 + +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.Scope +import io.sentry.ScopeCallback +import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.compose.navigation3.lifecycle.NAVIGATION_CONTEXT_KEY +import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever + +class CoordinatedBackStackObserverTest { + + @Test + fun `onBackStackChanged() - acquires the nav lease before applying the first update`() { + val fixture = Fixture() + fixture.givenPausedLeaseholder() + val sut = fixture.getSut() + + sut.onBackStackChanged(listOf("Home")) + + // An update applied before the lease is acquired is staged and gets no nav transaction. + assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) + } + + @Test + fun `a resumed owner doesn't rebind a nav transaction it retired by navigating without the nav lease`() { + val fixture = Fixture() + val homeLifecycle = fixture.resumedLifecycle() + val homeObserver = fixture.getSut(homeLifecycle) + val pickerObserver = fixture.getSut() + homeObserver.onBackStackChanged(listOf("Home")) + val homeTransaction = fixture.startedTransaction("/Home") + // A pending child span keeps the nav transaction, which waits for its children, unfinished. + homeTransaction.startChild("http.client") + homeLifecycle.currentState = Lifecycle.State.STARTED + pickerObserver.onBackStackChanged(listOf("Picker")) + homeObserver.onBackStackChanged(listOf("Home", "Result")) + check(!homeTransaction.isFinished) + + homeLifecycle.currentState = Lifecycle.State.RESUMED + + assertThat(fixture.scope.screen).isEqualTo("/Result") + // "/Result" was reached without the nav lease, so it has no nav transaction to bind. + assertThat(fixture.scope.transaction).isNull() + } + + @Test + fun `dispose() - clears the nav data it published`() { + val fixture = Fixture() + val sut = fixture.getSut() + sut.onBackStackChanged(listOf("Home")) + + sut.dispose() + + assertThat(fixture.scope.screen).isNull() + } + + @Test + fun `dispose() - without the nav lease, leaves the leaseholder's nav data and nav transaction alone`() { + val fixture = Fixture() + val homeObserver = fixture.getSut() + val settingsObserver = fixture.getSut() + homeObserver.onBackStackChanged(listOf("Home")) + // The settings observer's owner is also resumed, so it takes the nav lease from the home one. + settingsObserver.onBackStackChanged(listOf("Settings")) + val settingsTransaction = fixture.startedTransaction("/Settings") + + homeObserver.dispose() + + assertThat(fixture.scope.screen).isEqualTo("/Settings") + assertThat(fixture.scope.contexts[NAVIGATION_CONTEXT_KEY]) + .isEqualTo(mapOf("Back Stack" to listOf(mapOf("entry" to "/Settings")))) + assertThat(fixture.scope.transaction).isSameInstanceAs(settingsTransaction) + assertThat(settingsTransaction.isFinished).isFalse() + } + + @Test + fun `dispose() - after its lifecycle is destroyed, leaves the leaseholder's nav data and nav transaction alone`() { + val fixture = Fixture() + val homeLifecycle = fixture.resumedLifecycle() + val homeObserver = fixture.getSut(homeLifecycle) + val settingsObserver = fixture.getSut() + homeObserver.onBackStackChanged(listOf("Home")) + homeLifecycle.currentState = Lifecycle.State.STARTED + settingsObserver.onBackStackChanged(listOf("Settings")) + val settingsTransaction = fixture.startedTransaction("/Settings") + // Destroying the lifecycle before the composition is disposed unregisters the home observer's + // owner from the coordinator ahead of its dispose() call. + homeLifecycle.currentState = Lifecycle.State.DESTROYED + + homeObserver.dispose() + + assertThat(fixture.scope.screen).isEqualTo("/Settings") + assertThat(fixture.scope.contexts[NAVIGATION_CONTEXT_KEY]) + .isEqualTo(mapOf("Back Stack" to listOf(mapOf("entry" to "/Settings")))) + assertThat(fixture.scope.transaction).isSameInstanceAs(settingsTransaction) + assertThat(settingsTransaction.isFinished).isFalse() + } + + private class Fixture { + val scope = + Scope( + SentryOptions.empty().apply { + dsn = "http://key@localhost/proj" + tracesSampleRate = 1.0 + idleTimeout = null + deadlineTimeout = 0 + } + ) + val coordinator = NavLeaseCoordinator() + val startedTransactions = mutableListOf() + private val scopes = + mock().also { scopes -> + whenever(scopes.options).thenReturn(scope.options) + doAnswer { + (it.arguments[0] as ScopeCallback).run(scope) + null + } + .whenever(scopes) + .configureScope(any()) + doAnswer { + val transactionContext = it.arguments[0] as TransactionContext + val transactionOptions = it.arguments[1] as TransactionOptions + SentryTracer(transactionContext, scopes, transactionOptions) + .also(startedTransactions::add) + } + .whenever(scopes) + .startTransaction(any(), any()) + } + + fun startedTransaction(name: String): SentryTracer = startedTransactions.single { + it.name == name + } + + /** 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)) + } + + fun getSut(lifecycle: Lifecycle = resumedLifecycle()): CoordinatedBackStackObserver = + CoordinatedBackStackObserver( + scopes = scopes, + options = SentryNavOptions(), + entryMapper = + ForwardingBackStackEntryMapper { + BackStackEntryMapper { SentryBackStackEntry(it) } + }, + coordinator = coordinator, + ) + .apply { attach(lifecycle) } + + fun resumedLifecycle(): LifecycleRegistry = + object : LifecycleOwner { + override val lifecycle = + LifecycleRegistry.createUnsafe(this).apply { currentState = Lifecycle.State.RESUMED } + } + .lifecycle + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt index acc8445b57..32536d28cc 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt @@ -3,11 +3,16 @@ package io.sentry.compose.navigation3 import android.app.Application import android.content.ComponentName import androidx.activity.ComponentActivity +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateOf import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat @@ -15,12 +20,18 @@ import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.IScope import io.sentry.IScopes +import io.sentry.ISentryClient +import io.sentry.ITransaction +import io.sentry.PropagationContext import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.ScopeType +import io.sentry.Scopes import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.TransactionContext import io.sentry.TransactionOptions +import io.sentry.compose.navigation3.lifecycle.NavLeaseCoordinator import org.junit.Rule import org.junit.Test import org.junit.rules.TestWatcher @@ -35,6 +46,7 @@ import org.robolectric.annotation.Config @RunWith(AndroidJUnit4::class) @Config(sdk = [30]) +@Suppress("LargeClass") class SentryNavEffectTest { private val defaultEntryMapper = @@ -72,6 +84,7 @@ class SentryNavEffectTest { } val scope = Scope(options) val scopes = mock() + val coordinator = NavLeaseCoordinator() val breadcrumbs = mutableListOf() val transactions = mutableListOf() @@ -111,6 +124,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } @@ -134,6 +148,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -161,6 +176,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -188,6 +204,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -221,6 +238,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -254,6 +272,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions { maxCapturedBackStackEntries = 0 }, scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -277,6 +296,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -313,6 +333,7 @@ class SentryNavEffectTest { backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) val currentTop = backStack.last() @@ -347,6 +368,7 @@ class SentryNavEffectTest { backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -384,6 +406,7 @@ class SentryNavEffectTest { backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -416,6 +439,7 @@ class SentryNavEffectTest { backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -458,6 +482,7 @@ class SentryNavEffectTest { backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -495,6 +520,7 @@ class SentryNavEffectTest { options = options.value, backStackEntryMapper = defaultEntryMapper, scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } composeRule.waitForIdle() @@ -524,7 +550,9 @@ class SentryNavEffectTest { SentryNavEffect( backStack = backStack, backStackEntryMapper = defaultEntryMapper, + options = SentryNavOptions(), scopes = fixture.scopes, + coordinator = fixture.coordinator, ) } } @@ -542,6 +570,230 @@ class SentryNavEffectTest { assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() } + @Test + fun `effect publishes initial navigation before its lifecycle owner resumes`() { + val fixture = NavTestFixture() + val owner = NavTestLifecycle() + val options = SentryNavOptions() + composeRule.setContent { + CompositionLocalProvider(LocalLifecycleOwner provides owner) { + SentryNavEffect( + backStack = listOf("Home"), + backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, + options = options, + scopes = fixture.scopes, + coordinator = fixture.coordinator, + ) + } + } + composeRule.runOnIdle { + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }).containsExactly("/Home") + owner.resume() + } + composeRule.runOnIdle { + assertThat(fixture.scope.screen).isEqualTo("/Home") + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }).containsExactly("/Home") + } + } + + @Test + fun `effect records navigation while its lifecycle owner is paused`() { + val fixture = NavTestFixture() + val owner = NavTestLifecycle(Lifecycle.State.RESUMED) + val backStack = mutableStateListOf("Home") + val options = SentryNavOptions() + composeRule.setContent { + CompositionLocalProvider(LocalLifecycleOwner provides owner) { + SentryNavEffect( + backStack = backStack, + backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, + options = options, + scopes = fixture.scopes, + coordinator = fixture.coordinator, + ) + } + } + composeRule.runOnIdle { + owner.pause() + backStack += "Profile" + } + composeRule.runOnIdle { + assertThat(fixture.scope.transaction?.name).isEqualTo("/Profile") + assertThat(fixture.scope.screen).isEqualTo("/Profile") + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }) + .containsExactly("/Home", "/Profile") + .inOrder() + } + } + + @Test + fun `navigating without the lease records breadcrumbs, then publishes with a fresh trace on resume`() { + val fixture = NavTestFixture() + val ownerA = NavTestLifecycle(Lifecycle.State.RESUMED) + val ownerB = NavTestLifecycle() + val backStackA = mutableStateListOf("Home") + val showB = mutableStateOf(false) + val options = SentryNavOptions() + val mapper = BackStackEntryMapper { SentryBackStackEntry(it) } + composeRule.setContent { + CompositionLocalProvider(LocalLifecycleOwner provides ownerA) { + SentryNavEffect(backStackA, mapper, options, fixture.scopes, fixture.coordinator) + } + if (showB.value) { + CompositionLocalProvider(LocalLifecycleOwner provides ownerB) { + SentryNavEffect(listOf("Picker"), mapper, options, fixture.scopes, fixture.coordinator) + } + } + } + + // A starts B: A pauses, B is created and takes over, B resumes, then A stops. + composeRule.runOnIdle { + ownerA.pause() + showB.value = true + } + composeRule.runOnIdle { + ownerB.resume() + ownerA.stop() + } + // B finishes with a result: B pauses, then A restarts and navigates before it resumes. + composeRule.runOnIdle { + ownerB.pause() + ownerA.start() + backStackA += "Result" + } + lateinit var contextBeforeResume: PropagationContext + composeRule.runOnIdle { + assertThat(fixture.scope.transaction?.name).isEqualTo("/Picker") + assertThat(fixture.scope.screen).isEqualTo("/Picker") + assertThat(fixture.scope.breadcrumbs.map { it.data["to"] }) + .containsExactly("/Home", "/Picker", "/Result") + .inOrder() + contextBeforeResume = fixture.scope.propagationContext + ownerA.resume() + } + + // A gets no nav transaction for /Result: one started without the lease would have collected no + // spans of its own. It publishes its screen and gets the fresh trace a transaction would have. + composeRule.runOnIdle { + assertThat(fixture.scope.transaction).isNull() + assertThat(fixture.scope.screen).isEqualTo("/Result") + assertThat(fixture.scope.propagationContext).isNotSameInstanceAs(contextBeforeResume) + } + } + + @Test + fun `backgrounding with a translucent owner on top keeps the top owner's data published`() { + val fixture = NavTestFixture() + val beneath = NavTestLifecycle(Lifecycle.State.RESUMED) + val translucent = NavTestLifecycle() + val showTranslucent = mutableStateOf(false) + val options = SentryNavOptions() + val mapper = BackStackEntryMapper { SentryBackStackEntry(it) } + composeRule.setContent { + CompositionLocalProvider(LocalLifecycleOwner provides beneath) { + SentryNavEffect(listOf("Products"), mapper, options, fixture.scopes, fixture.coordinator) + } + if (showTranslucent.value) { + CompositionLocalProvider(LocalLifecycleOwner provides translucent) { + SentryNavEffect(listOf("Checkout"), mapper, options, fixture.scopes, fixture.coordinator) + } + } + } + + // A translucent owner opens on top: the owner beneath pauses but stays visible. + composeRule.runOnIdle { + beneath.pause() + showTranslucent.value = true + } + lateinit var checkoutTransaction: ITransaction + composeRule.runOnIdle { + translucent.resume() + checkoutTransaction = requireNotNull(fixture.scope.transaction) + assertThat(checkoutTransaction.name).isEqualTo("/Checkout") + } + + // The app goes to the background: the top owner stops before the one beneath it. + composeRule.runOnIdle { + translucent.pause() + translucent.stop() + beneath.stop() + + assertThat(fixture.scope.screen).isEqualTo("/Checkout") + assertThat(fixture.scope.navigationBackStack()?.map { it["entry"] }) + .containsExactly("/Checkout") + assertThat(fixture.scope.transaction).isSameInstanceAs(checkoutTransaction) + } + } + + @Test + fun `regaining the lease keeps a host screen name when screen tracking is off`() { + val fixture = NavTestFixture() + fixture.options.isEnableScreenTracking = false + val owner = NavTestLifecycle(Lifecycle.State.RESUMED) + val options = SentryNavOptions() + composeRule.setContent { + CompositionLocalProvider(LocalLifecycleOwner provides owner) { + SentryNavEffect( + backStack = listOf("Home"), + backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, + options = options, + scopes = fixture.scopes, + coordinator = fixture.coordinator, + ) + } + } + composeRule.runOnIdle { + fixture.scope.screen = "HostScreen" + + // E.g., the app goes to the background and comes back. + owner.pause() + owner.stop() + owner.start() + owner.resume() + + assertThat(fixture.scope.screen).isEqualTo("HostScreen") + assertThat(fixture.scope.navigationBackStack()?.map { it["entry"] }).containsExactly("/Home") + } + } + + @Test + fun `replacing lifecycle owner disposes the old session and observes the new owner`() { + val fixture = NavTestFixture() + val originalOwner = NavTestLifecycle(Lifecycle.State.RESUMED) + val replacementOwner = NavTestLifecycle() + val owner = mutableStateOf(originalOwner) + val options = SentryNavOptions() + composeRule.setContent { + CompositionLocalProvider(LocalLifecycleOwner provides owner.value) { + SentryNavEffect( + backStack = listOf("Home"), + backStackEntryMapper = BackStackEntryMapper { SentryBackStackEntry(it) }, + options = options, + scopes = fixture.scopes, + coordinator = fixture.coordinator, + ) + } + } + lateinit var originalTransaction: ITransaction + composeRule.runOnIdle { + originalTransaction = requireNotNull(fixture.scope.transaction) + owner.value = replacementOwner + } + composeRule.runOnIdle { + assertThat(originalTransaction.isFinished).isTrue() + assertThat(fixture.scope.transaction).isNotSameInstanceAs(originalTransaction) + assertThat(fixture.scope.transaction?.name).isEqualTo("/Home") + assertThat(fixture.scope.screen).isEqualTo("/Home") + originalOwner.pause() + originalOwner.resume() + assertThat(fixture.scope.screen).isEqualTo("/Home") + replacementOwner.resume() + assertThat(fixture.scope.screen).isEqualTo("/Home") + } + } + private fun IScope.navigationBackStack(): List>? { val navigationContext = contexts[NAVIGATION_CONTEXT_KEY] as? Map<*, *> ?: return null @@ -554,3 +806,52 @@ class SentryNavEffectTest { const val BACKSTACK_KEY = "Back Stack" } } + +private class NavTestLifecycle(state: Lifecycle.State = Lifecycle.State.CREATED) : LifecycleOwner { + + override val lifecycle = + LifecycleRegistry.createUnsafe(this).apply { + if (state == Lifecycle.State.DESTROYED) currentState = Lifecycle.State.CREATED + currentState = state + } + + fun resume() { + lifecycle.currentState = Lifecycle.State.RESUMED + } + + fun pause() { + lifecycle.currentState = Lifecycle.State.STARTED + } + + fun start() { + lifecycle.currentState = Lifecycle.State.STARTED + } + + fun stop() { + lifecycle.currentState = Lifecycle.State.CREATED + } +} + +/** Real SDK state with only the event delivery boundary stubbed. No timers or global SDK state. */ +private class NavTestFixture { + + val options = + SentryOptions().apply { + dsn = "http://key@localhost/1" + defaultScopeType = ScopeType.CURRENT + setTracesSampleRate(1.0) + idleTimeout = null + deadlineTimeout = 0 + } + + val scope = Scope(options) + val scopes = Scopes(scope, Scope(options), Scope(options), "navigation test") + + val coordinator = NavLeaseCoordinator() + + init { + val client = mock() + whenever(client.isEnabled).thenReturn(true) + scopes.bindClient(client) + } +}