From dc803d263ada13aa2dac72c2d799988a237a52a5 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 30 Sep 2026 10:20:19 +0200 Subject: [PATCH 1/2] feat(android): Improve SentryNavEffect by consolidating back stack route mapping (JAVA-274) Commit improves the yet-to-be-released SentryNavEffect by replacing separate route name and argument extractors with a single BackStackEntryMapper. Doing so lets us spare users from having to create three giant `when` statements mapping all nav entries in their entire app (two for us and one for Nav3's entryProvider). After this commit, users only have to create two. (Future work will allow them to create just one via a forthcoming Sentry entryProvider wrapper.) Commit also uses the term "back stack entry" rather than "back stack route" throughout to avoid developer confusion, given Google's use of "route" to mean (essentially) a navigation destination. By contrast, we need a term that refers solely to an element in the host app's back stack. (In general, Nav3 is careful to distinguish between nav destinations and back stack entries, as a destination may be composed from multiple entries in a back stack.) --- .../api/sentry-android-navigation3.api | 21 +- ...uteTranslator.kt => BackStackConverter.kt} | 253 ++++---- .../navigation3/BackStackEntryMapper.kt | 171 ++++++ .../compose/navigation3/BackStackKey.kt | 35 -- .../compose/navigation3/BackStackObserver.kt | 140 ++--- .../compose/navigation3/RouteExtractors.kt | 143 ----- .../compose/navigation3/SentryNavEffect.kt | 79 ++- .../compose/navigation3/SentryNavOptions.kt | 8 +- .../navigation3/BackStackConverterTest.kt | 555 ++++++++++++++++++ .../navigation3/BackStackObserverTest.kt | 350 +++++------ .../ForwardingBackStackEntryMapperTest.kt | 58 ++ .../navigation3/RouteExtractorsTest.kt | 81 --- .../navigation3/RouteTranslatorTest.kt | 539 ----------------- .../navigation3/SentryBackStackEntryTest.kt | 90 +++ .../navigation3/SentryNavEffectTest.kt | 112 ++-- .../navigation3/SentryNavOptionsTest.kt | 36 +- 16 files changed, 1415 insertions(+), 1256 deletions(-) rename sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/{RouteTranslator.kt => BackStackConverter.kt} (58%) create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt delete mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackKey.kt delete mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt delete mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt delete mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryBackStackEntryTest.kt diff --git a/sentry-android-navigation3/api/sentry-android-navigation3.api b/sentry-android-navigation3/api/sentry-android-navigation3.api index 12da93310ea..45f0482ed82 100644 --- a/sentry-android-navigation3/api/sentry-android-navigation3.api +++ b/sentry-android-navigation3/api/sentry-android-navigation3.api @@ -1,3 +1,7 @@ +public abstract interface class io/sentry/compose/navigation3/BackStackEntryMapper { + public abstract fun map (Ljava/lang/Object;)Lio/sentry/compose/navigation3/SentryBackStackEntry; +} + public final class io/sentry/compose/navigation3/BuildConfig { public static final field BUILD_TYPE Ljava/lang/String; public static final field DEBUG Z @@ -6,16 +10,19 @@ public final class io/sentry/compose/navigation3/BuildConfig { public fun ()V } -public abstract interface class io/sentry/compose/navigation3/RouteArgumentsExtractor { - public abstract fun extract (Ljava/lang/Object;)Ljava/util/Map; -} - -public abstract interface class io/sentry/compose/navigation3/RouteNameExtractor { - public abstract fun extract (Ljava/lang/Object;)Ljava/lang/String; +public final class io/sentry/compose/navigation3/SentryBackStackEntry { + public static final field $stable I + public fun (Ljava/lang/String;Ljava/util/Map;)V + public synthetic fun (Ljava/lang/String;Ljava/util/Map;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun equals (Ljava/lang/Object;)Z + public final fun getArguments ()Ljava/util/Map; + public final fun getName ()Ljava/lang/String; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; } public final class io/sentry/compose/navigation3/SentryNavEffectKt { - public static final fun SentryNavEffect (Ljava/util/List;Lio/sentry/compose/navigation3/RouteNameExtractor;Lio/sentry/compose/navigation3/RouteArgumentsExtractor;Lio/sentry/compose/navigation3/SentryNavOptions;Landroidx/compose/runtime/Composer;II)V + public static final fun SentryNavEffect (Ljava/util/List;Lio/sentry/compose/navigation3/BackStackEntryMapper;Lio/sentry/compose/navigation3/SentryNavOptions;Landroidx/compose/runtime/Composer;II)V } public final class io/sentry/compose/navigation3/SentryNavOptions { diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt similarity index 58% rename from sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt rename to sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt index 7007a8162f5..0785a9a8622 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt @@ -2,122 +2,88 @@ package io.sentry.compose.navigation3 import io.sentry.ILogger import io.sentry.SentryLevel.WARNING +import io.sentry.compose.navigation3.NormalizedSentryBackStackEntry.Companion.UNKNOWN_ENTRY_NAME import io.sentry.util.ExceptionUtils import java.util.IdentityHashMap -import org.jetbrains.annotations.TestOnly /** - * Translates app-defined back stack entries into input-ordered [Route]s. + * Converts the host app back stack into a list of [NormalizedSentryBackStackEntry]s. * - * **Exception handling policy** + * **Exception handling** * - * Invocations of host-provided [extractors] and sanitization of host-defined arguments are - * protected by broad `try-catch` clauses, as each may throw arbitrary exceptions. We avoid failing - * fast on the assumption that navigation telemetry is supplemental from host apps' perspective, and - * that falling back to an `/unknown` route name or losing an argument map is preferable to - * crashing. + * Invocations of the host-provided [BackStackEntryMapper] and sanitization of host-defined + * arguments are protected by broad `try-catch` clauses, as each may throw arbitrary exceptions. + * + * We avoid failing fast on the assumption that nav telemetry is supplemental, and falling back to + * an [UNKNOWN_ENTRY_NAME] or losing an argument map is preferable to crashing. * * **Threading policy** * - * This class performs work synchronously on the calling thread. Host-provided [extractors] are - * invoked on that same thread and should remain small, non-blocking, and safe for the caller's - * threading context. + * This class performs work synchronously on the calling thread. The host app's + * [BackStackEntryMapper] is invoked on that same thread and should remain small, non-blocking, and + * safe for the caller's threading context. */ -internal class RouteTranslator( - private val extractors: () -> RouteExtractors, +internal class BackStackConverter( + private val entryMapper: ForwardingBackStackEntryMapper, private val logger: ILogger, ) { - companion object { - internal const val UNKNOWN_ROUTE_NAME = "/unknown" - } - - /** Translates the provided [backStackEntries] into [Route]s and returns them in input order. */ - fun translate(backStackEntries: List, retentionPolicy: RetentionPolicy): List { + /** + * Converts the provided [backStack] into a list of [NormalizedSentryBackStackEntry]s by invoking + * the host app-provided [entryMapper] and normalizing the results. + * + * The returned list has the same order as `backStack`. + */ + fun convert( + backStack: List, + retentionPolicy: RetentionPolicy, + ): List { val warningState = WarningState() val sanitizer = ArgumentSanitizer(logger, warningState) - val routes = MutableList(backStackEntries.size) { null } + val entries = MutableList(backStack.size) { null } val indicesInPolicyOrder = when (retentionPolicy) { - RetentionPolicy.KEEP_FIRST -> backStackEntries.indices - RetentionPolicy.KEEP_LAST -> backStackEntries.indices.reversed() + RetentionPolicy.KEEP_FIRST -> backStack.indices + RetentionPolicy.KEEP_LAST -> backStack.indices.reversed() } for (index in indicesInPolicyOrder) { - val entry = backStackEntries[index] - routes[index] = - Route( - name = extractRouteName(entry, warningState), - arguments = extractRouteArguments(entry, sanitizer), - ) + val entry = backStack[index] + entries[index] = normalize(entry, sanitizer, warningState) } - return routes.requireNoNulls() + return entries.requireNoNulls() } - /** - * Returns a route name for the provided [backStackEntry], based on this translator's - * [name extractor][RouteExtractors.nameExtractor]. - * - * The returned name is normalized to always include a leading slash. E.g., both `PromoDialog` and - * `/PromoDialog` are resolved to `/PromoDialog`. (Doing so maintains parity with our Nav2 - * convention.) - */ - @TestOnly @Suppress("TooGenericExceptionCaught") - fun extractRouteName(backStackEntry: T, warningState: WarningState): String { - val name: String? = - try { - extractors.invoke().getName(backStackEntry) - } catch (t: Throwable) { - // Route name extractors are host app callbacks. - ExceptionUtils.rethrowIfFatal(t) - warningState.logNameExtractorFailureWarning(logger, t) - return UNKNOWN_ROUTE_NAME - } - - val normalizedName = name?.trim()?.takeUnless { it.isEmpty() }?.removePrefix("/") - if (normalizedName == null) { - warningState.logInvalidRouteNameWarning(logger) - return UNKNOWN_ROUTE_NAME - } - - return "/$normalizedName" - } - - /** - * Returns the arguments for the provided [backStackEntry], based on this translator's - * [arguments extractor][RouteExtractors.argumentsExtractor]. - * - * The arguments are sanitized before being returned, i.e., bounded in size and depth, and - * converted into a serializable form. - */ - @TestOnly - @Suppress("TooGenericExceptionCaught") - fun extractRouteArguments( + private fun normalize( backStackEntry: T, sanitizer: ArgumentSanitizer, - ): Map { - val raw = + warningState: WarningState, + ): NormalizedSentryBackStackEntry { + val info = try { - extractors.invoke().getArguments(backStackEntry) ?: return emptyMap() + entryMapper.map(backStackEntry) } catch (t: Throwable) { - // Route argument extractors are host app callbacks. + // Back stack entry mappers are host app callbacks. ExceptionUtils.rethrowIfFatal(t) - logger.log( - WARNING, - "Nav3 argumentsExtractor threw while resolving arguments. Skipping arguments.", - t, - ) - return emptyMap() + warningState.logMapperFailureWarning(logger, t) + return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME) } - return sanitizer.sanitizeEntry(raw) + val arguments = info.arguments?.let(sanitizer::sanitizeEntry) ?: emptyMap() + val formattedName = NormalizedSentryBackStackEntry.formatName(info.name) + if (formattedName.isBlank()) { + warningState.logInvalidNameWarning(logger) + return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments) + } + + return NormalizedSentryBackStackEntry(formattedName, arguments) } /** - * Specifies whether route info starting at the initial or final element of a back stack list + * Specifies whether entry info starting at the initial or final element of a back stack list * should be preserved if a size budget is exceeded. * * Most clients will want to select the policy that starts at the top of their back stack. @@ -125,13 +91,13 @@ internal class RouteTranslator( internal enum class RetentionPolicy { /** - * Retains route info for lower indexed elements in the back stack list if a particular info + * Retains entry info for lower indexed elements in the back stack list if a particular info * budget is reached. Retention starts at index 0 and increments until the budget is exhausted. */ KEEP_FIRST, /** - * Retains route info for higher indexed elements in the back stack list if a particular info + * Retains entry info for higher indexed elements in the back stack list if a particular info * budget is reached. Retention starts at lastIndex and decrements until the budget is * exhausted. */ @@ -139,10 +105,10 @@ internal class RouteTranslator( } /** - * Sanitizes a back stack update's argument maps into a serializable form. It bounds depth and - * total value count, and it rejects cyclic structures. + * Sanitizes a back stack entry's arguments and writes them in a serializable form. It bounds + * depth and total value count, and it rejects cyclic structures. * - * One instance is shared across every entry in a single [translate] call, so the value budget is + * One instance is shared across every entry in a single [convert] call, so the value budget is * enforced across the whole update. Once the budget is spent, the overflowing entry and every * older entry are dropped, while newer (already-processed) entries are preserved. */ @@ -152,7 +118,7 @@ internal class RouteTranslator( ) { private val activeContainers = IdentityHashMap() - private var remainingValues = MAX_ARGUMENT_VALUES + private var remainingValues = MAX_ARGUMENT_COUNT private var budgetExhausted = false /** @@ -185,7 +151,7 @@ internal class RouteTranslator( private fun sanitizeMap(value: Map<*, *>, depth: Int): Map { enter(value) try { - val sanitized = LinkedHashMap() + val sanitized = mutableMapOf() for ((key, childValue) in value) { sanitized[key.toString()] = sanitizeValue(childValue, depth + 1) } @@ -199,7 +165,7 @@ internal class RouteTranslator( enter(value) try { // The value budget bounds allocation instead of the caller-provided collection size. - val sanitized = ArrayList() + val sanitized = mutableListOf() for (childValue in value) { sanitized += sanitizeValue(childValue, depth + 1) } @@ -283,101 +249,127 @@ internal class RouteTranslator( * Max nesting depth allowed while sanitizing a single argument value for a given back stack * entry. * + * For instance, `mapOf("id" to 123)` has a depth of 1; `mapOf("items" to listOf("apple", + * "banana"))` has a depth of 2. + * * If exceeded, all arguments for that back stack entry are dropped. */ - private const val MAX_ARGUMENT_DEPTH = 20 + private const val MAX_ARGUMENT_DEPTH = 10 /** * Max number of argument values visited while sanitizing all entries in a given back stack * update. * + * For instance, `mapOf("id" to 123)` consumes 1 value; `mapOf("profile" to mapOf("id" to 123, + * "name" to "Ada"))` consumes 3 values. + * * If exceeded, the entry that overflows loses its arguments, as do older entries; newer - * entries are preserved. E.g., suppose we have the following back stack: + * entries are preserved. + * + * For instance, suppose we have the following back stack: + * * - /Checkout -> Top of the stack and processed first * - /ProductDetail -> Processed second and overflows the `MAX_ARGUMENT_VALUES` budget * - /Home * * Then /ProductDetail and /Home will have no arguments, but /Checkout will. */ - private const val MAX_ARGUMENT_VALUES = 1_000 - - private const val STRUCTURE_WARNING = - "Nav3 argument sanitization failed (possibly a cyclic or deeply nested structure). " + - "Skipping arguments." + private const val MAX_ARGUMENT_COUNT = 500 private const val BUDGET_WARNING = - "Nav3 arguments exceeded the maximum total value count for one backstack update. " + - "Skipping arguments for this and older captured entries." + "Nav3 arguments exceeded the maximum total value count for one backstack update. Skipping arguments " + + "for this and older captured entries." + + private const val STRUCTURE_WARNING = + "Nav3 argument sanitization failed (possibly a cyclic or deeply nested structure). Skipping arguments." } } /** A small state wrapper that lets us avoid spamming logs when sanitizing arguments. */ internal class WarningState { + + private var hasLoggedInvalidNameWarning = false + private var hasLoggedMapperFailureWarning = false private var hasLoggedUnsupportedValueWarning = false - private var hasLoggedInvalidRouteNameWarning = false - private var hasLoggedNameExtractorFailureWarning = false - fun logUnsupportedValueWarning(typeName: String?, logger: ILogger) { - if (hasLoggedUnsupportedValueWarning) { + fun logInvalidNameWarning(logger: ILogger) { + if (hasLoggedInvalidNameWarning) { return } logger.log( WARNING, - "Nav3 argumentsExtractor returned unsupported value of type %s while processing this back " + - "stack update. Falling back to toString(). Use String, CharSequence, Char, Number, " + - "Boolean, Enum, Map, Collection, object Array, and primitive array values for reliable " + - "results.", - typeName, + "Nav3 backStackEntryMapper returned a blank name while processing this back stack update. " + + "Using $UNKNOWN_ENTRY_NAME instead.", ) - hasLoggedUnsupportedValueWarning = true + hasLoggedInvalidNameWarning = true } - fun logInvalidRouteNameWarning(logger: ILogger) { - if (hasLoggedInvalidRouteNameWarning) { + fun logMapperFailureWarning(logger: ILogger, throwable: Throwable) { + if (hasLoggedMapperFailureWarning) { return } logger.log( WARNING, - "Nav3 nameExtractor returned a blank route name while processing this back stack update. " + - "Using /unknown instead.", + "Nav3 backStackEntryMapper threw while resolving an entry. " + + "Using $UNKNOWN_ENTRY_NAME without arguments instead.", + throwable, ) - hasLoggedInvalidRouteNameWarning = true + hasLoggedMapperFailureWarning = true } - fun logNameExtractorFailureWarning(logger: ILogger, throwable: Throwable) { - if (hasLoggedNameExtractorFailureWarning) { + fun logUnsupportedValueWarning(typeName: String?, logger: ILogger) { + if (hasLoggedUnsupportedValueWarning) { return } logger.log( WARNING, - "Nav3 nameExtractor threw while resolving a route name. Using /unknown instead.", - throwable, + "Nav3 backStackEntryMapper returned unsupported argument value of type %s while processing this back " + + "stack update. Falling back to toString(). Use String, CharSequence, Char, Number, " + + "Boolean, Enum, Map, Collection, object Array, and primitive array values for reliable " + + "results.", + typeName, ) - hasLoggedNameExtractorFailureWarning = true + hasLoggedUnsupportedValueWarning = true } } } -/** - * Summary information about a back stack entry from the host app, fit for use with Sentry data. - * - * All route names should be normalized to include a leading slash, and all arguments should be - * sanitized (i.e., bounded in size and depth, and converted into a serializable form). - */ -internal data class Route( +/** A normalized version of a [SentryBackStackEntry] produced by a [BackStackEntryMapper]. */ +internal data class NormalizedSentryBackStackEntry( + /** A [SentryBackStackEntry.name] trimmed and formatted to include a leading "/". */ val name: String, + /** Sanitized [SentryBackStackEntry.arguments] (i.e., bounded in size and depth). */ val arguments: Map = emptyMap(), ) { + companion object { + + const val UNKNOWN_ENTRY_NAME = "/unknown" + + /** + * Trims [name] and adds a "/" prefix if one isn't already present. + * + * Matches the Nav2, Dart, and Web naming patterns. + */ + fun formatName(name: String): String { + val trimmedName = name.trim() + return when { + trimmedName.isEmpty() -> "" + trimmedName.startsWith("/") -> trimmedName + else -> "/$trimmedName" + } + } + } + /** - * Returns this route in serialized form. E.g.: + * Returns this entry in serialized form. E.g.: * ``` * { - * "route": "/ProductScreen" - * "args": { + * "entry": "/ProductScreen" + * "arguments": { * "product_id": 12345 * "promo_id:": "spring-marketing-drive-2026" * } @@ -385,11 +377,12 @@ internal data class Route( * ``` */ fun serialize(): Map = buildMap { - put("route", name) + put("entry", name) if (arguments.isNotEmpty()) { - put("args", arguments) + put("arguments", arguments) } } } -internal fun List.serialize(): List> = map(Route::serialize) +internal fun List.serialize(): List> = + map(NormalizedSentryBackStackEntry::serialize) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt new file mode 100644 index 00000000000..c0a79d15a05 --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt @@ -0,0 +1,171 @@ +package io.sentry.compose.navigation3 + +import androidx.compose.runtime.snapshots.Snapshot +import org.jetbrains.annotations.ApiStatus + +/** Info about a given back stack entry, suitable for display in Sentry. */ +@ApiStatus.Experimental +@ApiStatus.Internal +public class SentryBackStackEntry( + /** + * A host-app defined name for a given back stack entry. + * + * Sentry interprets the name as a navigation destination. For instance, the following + * `SentryBackStackEntry` instances: + * ```kotlin + * SentryBackStackEntry(name = "Home") + * SentryBackStackEntry(name = "ProductDetail", arguments = mapOf("product_id" to 1234)) + * ``` + * + * will produce a breadcrumb like this: + * ```json + * { + * "from": "/Home", + * "to": "/ProductDetail", + * "to_arguments": { + * "product_id": 1234 + * } + * } + * ``` + * + * Note that the Sentry SDK normalizes `name` to include a "/" prefix. + */ + public val name: String, + /** + * Arguments from a given back stack entry, as selected by the host app. + * + * Useful for capturing any properties in the back stack key that have diagnostic value. + * + * Sentry treats these as metadata to be displayed alongside [name] in appropriate contexts. (See + * the `name` KDoc for an example.) + */ + public val arguments: Map? = null, +) { + + override fun equals(other: Any?): Boolean = + this === other || + (other is SentryBackStackEntry && name == other.name && arguments == other.arguments) + + override fun hashCode(): Int = 31 * name.hashCode() + (arguments?.hashCode() ?: 0) + + /** Omits arguments because they may contain sensitive host-app data. */ + override fun toString(): String = "SentryBackStackEntry(name=$name)" +} + +/** + * Maps a back stack entry to a displayable name and optional diagnostic arguments. + * + * To be implemented by the host app. + * + * **Privacy / PII** + * + * Values returned from [map] are ***not*** scrubbed by the Sentry SDK before being sent to Sentry. + * Only return names and arguments that are known to be safe or have been pre-scrubbed. + * + * **Performance** + * + * This mapper is invoked synchronously from [SentryNavEffect] on the same apply thread that runs + * the effect. Avoid non-performant mappings. + * + * **Choosing appropriate names** + * + * Return stable, low-cardinality names that don't depend on object identity, argument values, or + * runtime class-name preservation. E.g., `Home`, `DetailScreen`, etc. + * + * In particular, avoid `::class.simpleName`, as R8 obfuscates class names and may associate them + * with different symbols across builds. + * + * **Names fall back to "/unknown"** + * + * If [map] throws or returns a blank [name][SentryBackStackEntry.name], Sentry records the + * destination as "/unknown". Doing so signals that name extraction needs to be fixed while avoiding + * misleading gaps in navigation data. + * + * For instance, if a user navigates from `/home -> /detail -> /settings`, but the mapper for + * `/detail` throws, the back stack record will be `/home -> /unknown -> /settings` rather than + * `/home -> /settings`. + * + * **Choosing appropriate arguments** + * + * Argument values may be any of the following scalar types: + * + * - [String] + * - [CharSequence] + * - [Char] + * - [Boolean] + * - any [Number] + * - enums (via [Enum.name]) + * - `null` + * + * Or any of the following container types: + * + * - [Array]s + * - primitive arrays + * - [Map]s + * - [Collection]s + * + * Containers may be nested, but they must bottom out in supported scalar types. Cyclic or deeply + * nested containers will be skipped. + * + * **Arguments fall back to `toString()` or nothing** + * + * All non-supported argument types are stringified via `toString()`. If [map] throws, no arguments + * are recorded for that back stack entry. + * + * **Using kotlinx.serialization** + * + * If your back stack contains `@Serializable` entry types, consider giving each entry an explicit + * `@SerialName` and using its `descriptor.serialName`. For performance reasons, don't serialize + * back stack keys in their entirety as `arguments` if they might be large, deeply nested, or + * contain PII or other sensitive information. + * + * For instance: + * ```kotlin + * @Serializable + * @SerialName("Home") + * data object Home(userName: String) : NavKey + * + * @Serializable + * @SerialName("ProductDetail") + * data class ProductDetail(userName: String, productId: String, tab: Tab) : NavKey + * ``` + * ```kotlin + * val backStackItemMapper = BackStackEntryMapper { entry -> + * when (entry) { + * is Home -> SentryBackStackEntry(Home.serializer().descriptor.serialName) + * is ProductDetail -> SentryBackStackEntry( + * name = ProductDetail.serializer().descriptor.serialName, + * // Select a subset of diagnostic arguments when serialization is unsafe + * // or non-performant. + * arguments = mapOf("product_id" to entry.productId, "tab" to entry.tab) + * ) + * } + * } + * ``` + */ +@ApiStatus.Experimental +@ApiStatus.Internal +public fun interface BackStackEntryMapper { + public fun map(backStackEntry: T): SentryBackStackEntry +} + +/** + * A Compose-compatible version of [BackStackEntryMapper] that generates [SentryBackStackEntry]s by + * forwarding the request to the host app mapper in effect at the time [map] is called. + * + * Dynamically determining the current mapper lets us separate two concerns: + * + * 1. the lifetime of a consumer that tracks navigation state over time (e.g., [BackStackObserver]); + * and + * 2. the lifetime of the mapper used to generate `SentryBackStackEntry`s. + * + * Without that separation, a long-lived consumer would have to choose between holding stale mapping + * logic or recreating its own state whenever the mapper changed. + */ +internal class ForwardingBackStackEntryMapper( + private val currentMapper: () -> BackStackEntryMapper +) { + fun map(backStackEntry: T): SentryBackStackEntry = Snapshot.withoutReadObservation { + currentMapper().map(backStackEntry) + } +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackKey.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackKey.kt deleted file mode 100644 index 83ebf0b88ed..00000000000 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackKey.kt +++ /dev/null @@ -1,35 +0,0 @@ -package io.sentry.compose.navigation3 - -/** - * A key for distinguishing back stacks over time. - * - * Lets `*Effect`s restart when either the identity of a stack entry changes or the stack's entries - * are reordered. - */ -internal class BackStackKey(private val backStack: List) { - - override fun equals(other: Any?): Boolean { - // Use of identity rather than structural equality frees us from entries' equals() and - // hashCode() implementations, which are provided by the host app and may be incomplete, - // expensive, or incorrect for our purposes. - if (this === other) { - return true - } - if (other !is BackStackKey<*>) { - return false - } - if (backStack.size != other.backStack.size) { - return false - } - - return backStack.indices.all { index -> backStack[index] === other.backStack[index] } - } - - override fun hashCode(): Int { - var result = backStack.size - for (entry in backStack) { - result = 31 * result + System.identityHashCode(entry) - } - return result - } -} 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 cd1e09d14da..6978e0615ac 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 @@ -13,10 +13,10 @@ import io.sentry.SpanStatus import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.TypeCheckHint +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.RouteTranslator.RetentionPolicy import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion @@ -31,7 +31,7 @@ private const val NAVIGATION_OP: String = "navigation" * **Top of the stack == the current screen** * * This class treats top of the back stack as the current navigation destination and visible screen. - * It knows nothing about composite Scenes or multipane navigation scenarios. + * It knows nothing about composite Scenes or multi-pane destinations. * * **Entry identity determines whether the top has changed** * @@ -53,19 +53,19 @@ private const val NAVIGATION_OP: String = "navigation" internal class BackStackObserver( private val scopes: IScopes, private val options: SentryNavOptions, - extractors: () -> RouteExtractors, + entryMapper: ForwardingBackStackEntryMapper, ) { - private val routeTranslator = RouteTranslator(extractors, scopes.options.logger) + private val backStackConverter = BackStackConverter(entryMapper, scopes.options.logger) private val navTransaction = NavTransaction(scopes) - private val navContext = NavContext(scopes, options) - private val navScreen = NavScreen() private val navBreadcrumbs = NavBreadcrumbs(scopes) + private val navScreen = NavScreen() + private val navContext = NavContext(scopes, options) // Safe because the host back stack retains the current top entry strongly between updates. - private var previousTopEntry: WeakReference? = null - private var previousTopRoute: Route? = null + private var previousTop: WeakReference? = null + private var previousTopNormalized: NormalizedSentryBackStackEntry? = null private val areNavigationTransactionsEnabled: Boolean get() = scopes.options.isTracingEnabled && options.enableNavigationTransactions @@ -88,14 +88,15 @@ internal class BackStackObserver( * * By default, the following happens every time the top of the back stack changes: * + * - a new nav transaction is started and the old nav transaction, if any, is stopped * - a breadcrumb is emitted * - a screen name is recorded - * - a new nav transaction is started and the old nav transaction, if any, is stopped. * * Names and other info for all of the above are derived from the new back stack top. * - * By default, a record of the current back stack is recorded for every call, irrespective of - * whether the top changes. + * By default, a record of the current back stack is recorded every time this method is invoked, + * irrespective of whether the top of the back stack changes, as the host app may have modified + * non-top entries. * * Defaults can be configured via the [SentryNavOptions] instance passed to this class's * constructor. (Screen names can be disabled via [SentryOptions.setEnableScreenTracking].) @@ -111,8 +112,8 @@ internal class BackStackObserver( } internal fun cleanup() { - previousTopEntry = null - previousTopRoute = null + previousTop = null + previousTopNormalized = null scopes.configureScope { scope -> navTransaction.stop(scope) @@ -133,25 +134,24 @@ internal class BackStackObserver( val topEntry = backStack.lastOrNull() ?: return BackStackIsEmpty val data = backStack.extractData() - return if (topEntry === previousTopEntry?.get()) { + return if (topEntry === previousTop?.get()) { BackStackHasSameTop(data) } else { - BackStackHasNewTop(previousTopRoute, data) + BackStackHasNewTop(previousTopNormalized, data) } } private fun applyChange(scope: IScope, change: PreparedChange) { when (change) { is BackStackIsEmpty -> handleEmptyBackStack(scope) - is BackStackHasNewTop -> handleNewTop(scope, change.previousTop, change.backStack) is BackStackHasSameTop -> handleSameTop(scope, change.backStack) } } /** - * Extracts Sentry-compatible data from the receiver (i.e., a list of host app back stack entries) - * in the form of a [BackStackData]. + * Extracts Sentry-compatible data from the receiver (i.e., the host app back stack) in the form + * of a [BackStackData] instance. * * Throws if the receiver is empty. */ @@ -161,48 +161,48 @@ internal class BackStackObserver( val topEntry = this.last() val shouldCaptureBackStack = options.captureBackStack && options.maxCapturedBackStackEntries > 0 - val entriesToTranslate = + val entriesToConvert = when { shouldCaptureBackStack -> // Reverse entries so they're displayed with the newest entry on top in the Sentry UI. this.takeLast(options.maxCapturedBackStackEntries).asReversed() - // We always need to translate the top entry for use with breadcrumbs, etc., even if we're + // We always need to convert the top entry for use with breadcrumbs, etc., even if we're // not capturing the back stack. else -> listOf(topEntry) } - val routes = - routeTranslator.translate( - backStackEntries = entriesToTranslate, + val normalizedEntries = + backStackConverter.convert( + backStack = entriesToConvert, retentionPolicy = RetentionPolicy.KEEP_FIRST, ) return BackStackData( topEntry = topEntry, - topRoute = routes.first(), - capturedRoutes = if (shouldCaptureBackStack) routes else emptyList(), + topEntryNormalized = normalizedEntries.first(), + capturedEntriesNormalized = if (shouldCaptureBackStack) normalizedEntries else emptyList(), ) } private fun handleNewTop( scope: IScope, - previousTop: Route?, + previousTop: NormalizedSentryBackStackEntry?, currentBackStack: BackStackData, ) { - val currentTopRoute = currentBackStack.topRoute + val currentTop = currentBackStack.topEntryNormalized - navContext.update(scope, currentBackStack.capturedRoutes) + navContext.update(scope, currentBackStack.capturedEntriesNormalized) if (scopes.options.isEnableScreenTracking) { - navScreen.update(scope, currentTopRoute) + navScreen.update(scope, currentTop) } if (options.enableNavigationBreadcrumbs) { navBreadcrumbs.emit( - from = previousTop, - toEntry = currentBackStack.topEntry, - toRoute = currentBackStack.topRoute, + fromEntry = previousTop, + toEntry = currentBackStack.topEntryNormalized, + toRawEntry = currentBackStack.topEntry, ) } @@ -212,8 +212,8 @@ internal class BackStackObserver( navTransaction .start( scope, - currentTopRoute.name, - currentTopRoute.arguments, + currentTop.name, + currentTop.arguments, ) ?.let { transaction -> navContext.updateTransaction(transaction, scope, currentBackStack) } } else { @@ -221,12 +221,12 @@ internal class BackStackObserver( scope.withPropagationContext { scope.setPropagationContext(PropagationContext()) } } - storeAsPreviousTop(currentBackStack.topEntry, currentBackStack.topRoute) + storeAsPreviousTop(currentBackStack.topEntry, currentBackStack.topEntryNormalized) } private fun handleSameTop(scope: IScope, backStack: BackStackData) { - navContext.update(scope, backStack.capturedRoutes) - storeAsPreviousTop(backStack.topEntry, backStack.topRoute) + navContext.update(scope, backStack.capturedEntriesNormalized) + storeAsPreviousTop(backStack.topEntry, backStack.topEntryNormalized) } private fun handleEmptyBackStack(scope: IScope) { @@ -236,14 +236,14 @@ internal class BackStackObserver( clearPreviousTop() } - private fun storeAsPreviousTop(topEntry: T, topRoute: Route) { - previousTopEntry = WeakReference(topEntry) - previousTopRoute = topRoute + private fun storeAsPreviousTop(topEntry: T, topEntryNormalized: NormalizedSentryBackStackEntry) { + previousTop = WeakReference(topEntry) + previousTopNormalized = topEntryNormalized } private fun clearPreviousTop() { - previousTopEntry = null - previousTopRoute = null + previousTop = null + previousTopNormalized = null } } @@ -264,7 +264,7 @@ private sealed interface PreparedChange { * The top of the back stack has changed, and one or more entries below it may have been updated. */ data class BackStackHasNewTop( - val previousTop: Route?, + val previousTop: NormalizedSentryBackStackEntry?, val backStack: BackStackData, ) : PreparedChange @@ -275,12 +275,14 @@ private sealed interface PreparedChange { /** Info extracted from the host app's back stack in a form suitable for Sentry data. */ private data class BackStackData( val topEntry: T, - val topRoute: Route, + val topEntryNormalized: NormalizedSentryBackStackEntry, /** - * [Route]s representing the newest [SentryNavOption.maxCapturedBackStackEntries] entries from the - * host app's back stack. Possibly empty. + * [NormalizedSentryBackStackEntry]s representing the newest + * [SentryNavOption.maxCapturedBackStackEntries] entries from the host app's back stack. + * + * Possibly empty. */ - val capturedRoutes: List, + val capturedEntriesNormalized: List, ) /** A helper class for managing nav transactions. */ @@ -380,13 +382,13 @@ private class NavContext(private val scopes: IScopes, private val options: Sentr private const val NAVIGATION_CONTEXT_KEY = "navigation" } - fun update(scope: IScope, backStackRoutes: List) { - if (backStackRoutes.isEmpty()) { + fun update(scope: IScope, backStackEntries: List) { + if (backStackEntries.isEmpty()) { clear(scope) return } - scope.setContexts(NAVIGATION_CONTEXT_KEY, backStackRoutes.toBackStackMap()) + scope.setContexts(NAVIGATION_CONTEXT_KEY, backStackEntries.toBackStackMap()) } fun clear(scope: IScope) { @@ -413,17 +415,21 @@ private class NavContext(private val scopes: IScopes, private val options: Sentr val appContext = transaction.contexts.app ?: io.sentry.protocol.Contexts(scope.contexts).app ?: App() - appContext.viewNames = listOf(backStack.topRoute.name) + appContext.viewNames = listOf(backStack.topEntryNormalized.name) transaction.contexts.setApp(appContext) } - if (options.captureBackStack && backStack.capturedRoutes.isNotEmpty()) { - transaction.setContext(NAVIGATION_CONTEXT_KEY, backStack.capturedRoutes.toBackStackMap()) + if (options.captureBackStack && backStack.capturedEntriesNormalized.isNotEmpty()) { + transaction.setContext( + NAVIGATION_CONTEXT_KEY, + backStack.capturedEntriesNormalized.toBackStackMap(), + ) } } /** Builds the `{"backstack": [...]}` map bound under [NAVIGATION_CONTEXT_KEY]. */ - private fun List.toBackStackMap(): Map = mapOf(BACKSTACK_KEY to serialize()) + private fun List.toBackStackMap(): Map = + mapOf(BACKSTACK_KEY to serialize()) } /** A helper class for updating the tracked screen name. */ @@ -431,14 +437,14 @@ private class NavScreen { private var lastScreenName: String? = null - fun update(scope: IScope, currentRoute: Route) { - scope.screen = currentRoute.name - lastScreenName = currentRoute.name + fun update(scope: IScope, currentEntry: NormalizedSentryBackStackEntry) { + scope.screen = currentEntry.name + lastScreenName = currentEntry.name } fun clear(scope: IScope) { - val routeName = lastScreenName ?: return - if (scope.screen == routeName) { + val screenName = lastScreenName ?: return + if (scope.screen == screenName) { scope.screen = null } lastScreenName = null @@ -449,32 +455,32 @@ private class NavScreen { private class NavBreadcrumbs(private val scopes: IScopes) { fun emit( - from: Route?, - toEntry: T, - toRoute: Route, + fromEntry: NormalizedSentryBackStackEntry?, + toEntry: NormalizedSentryBackStackEntry, + toRawEntry: T, ) { val breadcrumb = Breadcrumb().apply { type = NAVIGATION_OP category = NAVIGATION_OP - from?.let { + fromEntry?.let { data["from"] = it.name if (it.arguments.isNotEmpty()) { data["from_arguments"] = it.arguments } } - data["to"] = toRoute.name - if (toRoute.arguments.isNotEmpty()) { - data["to_arguments"] = toRoute.arguments + data["to"] = toEntry.name + if (toEntry.arguments.isNotEmpty()) { + data["to_arguments"] = toEntry.arguments } level = INFO } val hint = Hint() - hint.set(TypeCheckHint.ANDROID_NAV3_DESTINATION, toEntry) + hint.set(TypeCheckHint.ANDROID_NAV3_DESTINATION, toRawEntry) scopes.addBreadcrumb(breadcrumb, hint) } } diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt deleted file mode 100644 index fc11e1ef238..00000000000 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt +++ /dev/null @@ -1,143 +0,0 @@ -package io.sentry.compose.navigation3 - -import androidx.compose.runtime.snapshots.Snapshot -import org.jetbrains.annotations.ApiStatus - -/** - * Extracts a human-readable route name from a back stack entry. - * - * **Privacy / PII** - * - * Values returned from [extract] are ***not*** scrubbed by the Sentry SDK before being sent to - * Sentry. Only return names that are known to be safe or have been pre-scrubbed. - * - * **Choosing appropriate route names** - * - * Implementations should return stable, low-cardinality names that don't depend on object identity, - * argument values, or runtime class-name preservation. E.g., `Home`, `DetailScreen`, etc. - * - * In particular, avoid `::class.simpleName` in release builds, as R8 obfuscates class names and may - * map them to different symbols across builds. - * - * Extractors are invoked synchronously from [SentryNavEffect] on the same apply thread that runs - * the effect. Avoid non-performant extraction logic. - * - * **Falls back to "/unknown"** - * - * If [extract] throws or returns a blank route name, Sentry records the destination as "/unknown". - * Doing so signals that name extraction needs to be fixed while avoiding misleading gaps in - * navigation data. - * - * For instance, if a user navigates from `/home -> /detail -> /settings`, but the name extractor - * for `/detail` throws, the back stack record will be `/home -> /unknown -> /settings` rather than - * `/home -> /settings`. - * - * **Using kotlinx.serialization** - * - * If your back stack contains `@Serializable` route types, consider mapping each route type to a - * stable serializer name. For instance: - * ```kotlin - * val nameExtractor = RouteNameExtractor { route -> - * when (route) { - * is HomeRoute -> HomeRoute.serializer().descriptor.serialName - * is ProfileRoute -> ProfileRoute.serializer().descriptor.serialName - * is SettingsRoute -> SettingsRoute.serializer().descriptor.serialName - * } - * } - * ``` - * - * Doing so gives each route type a stable, non-obfuscated name while leaving per-route arguments to - * [RouteArgumentsExtractor]. - */ -@ApiStatus.Experimental -@ApiStatus.Internal -public fun interface RouteNameExtractor { - public fun extract(backStackEntry: T): String -} - -/** - * Extracts diagnostic route arguments from a back stack entry as map of argument name -> argument - * values. - * - * **Privacy / PII** - * - * Values returned from [extract] are ***not*** scrubbed by the Sentry SDK before being sent to - * Sentry. Only return arguments that are known to be safe or have been pre-scrubbed. - * - * **Choosing appropriate route arguments** - * - * Return only a small subset of route data useful for diagnostics. Data should be stable enough to - * inspect in Sentry. - * - * Extractors are invoked synchronously from [SentryNavEffect] on the same apply thread that runs - * the effect. For performance reasons, implementations should avoid large structures. Cyclic or - * deeply nested containers will be skipped. (See `RouteTranslator` for more details.) - * - * **Accepted value types** - * - * Values may be any of the following scalar types: - * - * - [String] - * - [CharSequence] - * - [Char] - * - [Boolean] - * - any [Number] - * - enums (via [Enum.name]) - * - `null` - * - * Or any of the following container types: - * - * - [Array]s - * - primitive arrays - * - [Map]s - * - [Collection]s - * - * Container values may be nested, and they must bottom out in supported scalar types. - * - * **Falls back to `toString()` or nothing** - * - * All non-supported types are stringified via `toString()`. If [extract] throws, no arguments are - * recorded for the destination. - * - * **Using kotlinx.serialization** - * - * If your back stack contains `@Serializable` route types, avoid returning the entire route object - * when it may be large, nested, or privacy-sensitive. Prefer a small set of diagnostic arguments - * instead. For instance: - * ```kotlin - * val argumentsExtractor = RouteArgumentsExtractor { route -> - * when (route) { - * is HomeRoute -> emptyMap() - * is ProfileRoute -> mapOf("userId" to route.userId, "tab" to route.tab) - * is SettingsRoute -> mapOf("section" to route.section) - * } - * } - * ``` - */ -@ApiStatus.Experimental -@ApiStatus.Internal -public fun interface RouteArgumentsExtractor { - public fun extract(backStackEntry: T): Map -} - -/** - * Holds host app-defined extractors, which convert a back stack entry of type [T] into a route name - * and a map of zero or more route arguments. Extracted values are eventually grouped into [Route]s - * for display. - * - * Extractor invocations are hidden from Compose snapshot observation so they don't impact - * invalidation of the recompose scope that reads them. - */ -internal class RouteExtractors( - val nameExtractor: RouteNameExtractor, - val argumentsExtractor: RouteArgumentsExtractor?, -) { - - fun getName(backStackEntry: T): String = Snapshot.withoutReadObservation { - nameExtractor.extract(backStackEntry) - } - - fun getArguments(backStackEntry: T): Map? = Snapshot.withoutReadObservation { - argumentsExtractor?.extract(backStackEntry) - } -} 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 0dcf8e6ca80..36619d9c4d7 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 @@ -16,6 +16,7 @@ import org.jetbrains.annotations.ApiStatus * ```kotlin * @Composable * fun AppNavigation() { + * // Create your back stack like usual. * val navBackStack = rememberNavBackStack(Home) * * // Place SentryNavEffect in the same composable as your NavDisplay and call @@ -24,8 +25,9 @@ import org.jetbrains.annotations.ApiStatus * // get attributed to the appropriate nav transaction. * SentryNavEffect( * backStack = navBackStack, - * nameExtractor = { route -> route.extractName() }, - * argumentsExtractor = { route -> route.extractArgument() }, + * backStackEntryMapper = { entry -> + * SentryBackStackEntry(entry.toName(), entry.extractArguments()) + * }, * options = SentryNavOptions(), * ) * @@ -39,15 +41,14 @@ import org.jetbrains.annotations.ApiStatus * * **Data generated** * - * By default, the following data is produced for each nav destination: + * By default, the following data is produced every time the top of the provided [backStack] + * changes: * + * - a new navigation transaction * - a breadcrumb * - a screen name * - a record of the current back stack (last 10 entries) * - * A new transaction is started at each nav destination, assuming another non-nav transaction isn't - * already active. - * * You can configure the above defaults via [SentryNavOptions]. (Screen names can be disabled via * [SentryOptions.setEnableScreenTracking].) * @@ -55,57 +56,51 @@ import org.jetbrains.annotations.ApiStatus * * `SentryNavEffect` generates all Sentry data based solely on the top entry of your back stack. In * particular, it has no awareness of - * [`Scene`](https://developer.android.com/guide/navigation/navigation-3/scenes)s. Transaction - * routes, breadcrumbs, and screen names are all derived from the top entry of the back stack and - * are updated as it changes. + * [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` also doesn't make any special accommodations for + * `SentryNavEffect` doesn't make any special accommodations for * [predictive back](https://developer.android.com/guide/navigation/custom-back/predictive-back-gesture) - * gestures. That means, for instance, that spans produced by predictively rendered composables can - * show up under the current destination's transaction. + * gestures. That means that spans produced by predictively rendered composables can show up under + * the current destination's transaction. * * @param backStack The navigation backstack to observe. - * @param nameExtractor Extracts a human-readable route name from each entry of the [backStack]. - * @param argumentsExtractor Optional extractor for a map of argument name -> argument values from - * each entry of the [backStack]. If not provided, no arguments are attached. + * @param backStackEntryMapper Maps each entry of the [backStack] to a name and optional arguments + * for display in Sentry. See the [BackStackEntryMapper] KDoc for best practices. * @param options The kinds of navigation info this effect should record. */ @ApiStatus.Experimental @ApiStatus.Internal @Composable -@Suppress("FunctionNaming") public fun SentryNavEffect( backStack: List, - nameExtractor: RouteNameExtractor, - argumentsExtractor: RouteArgumentsExtractor? = null, + backStackEntryMapper: BackStackEntryMapper, options: SentryNavOptions = SentryNavOptions(), ) { SentryNavEffect( backStack = backStack, - nameExtractor = nameExtractor, - argumentsExtractor = argumentsExtractor, + backStackEntryMapper = backStackEntryMapper, options = options, scopes = ScopesAdapter.getInstance(), ) } @Composable -@Suppress("FunctionNaming") internal fun SentryNavEffect( backStack: List, - nameExtractor: RouteNameExtractor, - argumentsExtractor: RouteArgumentsExtractor? = null, + backStackEntryMapper: BackStackEntryMapper, options: SentryNavOptions = SentryNavOptions(), scopes: IScopes, ) { - val routeExtractors = rememberUpdatedState(RouteExtractors(nameExtractor, argumentsExtractor)) + val currentBackStackEntryMapper = rememberUpdatedState(backStackEntryMapper) val observer = remember(scopes, options) { BackStackObserver( scopes = scopes, options = options, - extractors = { routeExtractors.value }, + entryMapper = ForwardingBackStackEntryMapper { currentBackStackEntryMapper.value }, ) } @@ -122,3 +117,37 @@ internal fun SentryNavEffect( onDispose { observer.cleanup() } } } + +/** + * A key for distinguishing back stacks over time. + * + * Lets `*Effect`s restart when either the identity of a stack entry changes or the stack's entries + * are reordered. + */ +internal class BackStackKey(private val backStack: List) { + + override fun equals(other: Any?): Boolean { + // Use of identity rather than structural equality frees us from entries' equals() and + // hashCode() implementations, which are provided by the host app and may be incomplete, + // expensive, or incorrect for our purposes. + if (this === other) { + return true + } + if (other !is BackStackKey<*>) { + return false + } + if (backStack.size != other.backStack.size) { + return false + } + + return backStack.indices.all { index -> backStack[index] === other.backStack[index] } + } + + override fun hashCode(): Int { + var result = backStack.size + for (entry in backStack) { + result = 31 * result + System.identityHashCode(entry) + } + return result + } +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt index 628e0820dab..a9876e37d17 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt @@ -3,8 +3,8 @@ package io.sentry.compose.navigation3 import androidx.compose.runtime.Immutable import org.jetbrains.annotations.ApiStatus -// Keep the default low: every captured entry may require route-name extraction, argument -// extraction, and recursive argument sanitization when navigation changes are observed. +// Keep the default low: every captured entry may require argument extraction and recursive +// sanitization when navigation changes are observed. private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 10 /** @@ -68,8 +68,8 @@ private constructor( * most recent). Set to `0` to capture no back stack entries. * * Note: Sentry resolves and sanitizes up to [maxCapturedBackStackEntries] names + argument maps - * whenever your back stack changes. Keep name and argument extractors lightweight, and reduce - * the max captured count if extractor work is unusually expensive. + * whenever your back stack changes. Keep the back stack mapper lightweight, and reduce the max + * captured count if mapper work is expensive. */ public var maxCapturedBackStackEntries: Int = DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt new file mode 100644 index 00000000000..97cef3a7e34 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt @@ -0,0 +1,555 @@ +package io.sentry.compose.navigation3 + +import com.google.common.truth.Truth.assertThat +import io.sentry.ILogger +import io.sentry.SentryLevel.WARNING +import io.sentry.compose.navigation3.BackStackConverter.RetentionPolicy +import io.sentry.compose.navigation3.NormalizedSentryBackStackEntry.Companion.UNKNOWN_ENTRY_NAME +import java.util.AbstractCollection +import org.junit.Test +import org.mockito.kotlin.clearInvocations +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.times +import org.mockito.kotlin.verify + +class BackStackConverterTest { + + private data class HomeScreen(val id: String = "home") + + private data class ProfileScreen(val userId: String, val marketingId: String = "marketing_id") + + private data class ProductScreen(val productId: String) + + private data class SettingsScreen(val section: String) + + private enum class PrivacyMode { + PUBLIC, + PRIVATE, + } + + private val logger = mock() + + private val defaultEntryMapper = + BackStackEntryMapper { entry -> + SentryBackStackEntry(entry::class.simpleName ?: "unknown") + } + + private fun getSut( + entryMapper: BackStackEntryMapper = defaultEntryMapper + ): BackStackConverter = + BackStackConverter( + entryMapper = ForwardingBackStackEntryMapper { entryMapper }, + logger = logger, + ) + + private fun entryInfo(entry: Any, arguments: Map? = null): SentryBackStackEntry = + SentryBackStackEntry(entry::class.simpleName ?: "unknown", arguments) + + private fun BackStackConverter.convert(entry: Any): NormalizedSentryBackStackEntry = + convert(listOf(entry), RetentionPolicy.KEEP_FIRST).single() + + @Test + fun `convert returns normalized back stack entries`() { + val sut = + getSut( + entryMapper = { entry -> + entryInfo( + entry, + when (entry) { + is HomeScreen -> mapOf("renamedId" to entry.id) + is ProfileScreen -> mapOf("userId" to entry.userId) + is SettingsScreen -> emptyMap() + else -> emptyMap() + }, + ) + } + ) + + val routes = + sut.convert( + listOf( + SettingsScreen(section = "privacy"), + ProfileScreen(userId = "123", marketingId = "987"), + HomeScreen(id = "home"), + ), + RetentionPolicy.KEEP_FIRST, + ) + + assertThat(routes) + .containsExactly( + NormalizedSentryBackStackEntry(name = "/SettingsScreen"), + NormalizedSentryBackStackEntry( + name = "/ProfileScreen", + arguments = mapOf("userId" to "123"), + ), + NormalizedSentryBackStackEntry( + name = "/HomeScreen", + arguments = mapOf("renamedId" to "home"), + ), + ) + .inOrder() + } + + @Test + fun `convert preserves input order when top entry is first`() { + val sut = getSut() + + val routes = + sut.convert( + listOf(SettingsScreen("privacy"), ProfileScreen("123"), HomeScreen()), + RetentionPolicy.KEEP_FIRST, + ) + + assertThat(routes) + .containsExactly( + NormalizedSentryBackStackEntry("/SettingsScreen"), + NormalizedSentryBackStackEntry("/ProfileScreen"), + NormalizedSentryBackStackEntry("/HomeScreen"), + ) + .inOrder() + } + + @Test + fun `convert preserves input order when top entry is last`() { + val sut = getSut() + + val routes = + sut.convert( + listOf(HomeScreen(), ProfileScreen("123"), SettingsScreen("privacy")), + RetentionPolicy.KEEP_FIRST, + ) + + assertThat(routes) + .containsExactly( + NormalizedSentryBackStackEntry("/HomeScreen"), + NormalizedSentryBackStackEntry("/ProfileScreen"), + NormalizedSentryBackStackEntry("/SettingsScreen"), + ) + .inOrder() + } + + @Test + fun `convert returns an empty list if provided back stack is empty`() { + val sut = getSut() + + assertThat(sut.convert(emptyList(), RetentionPolicy.KEEP_FIRST)).isEmpty() + } + + @Test + fun `convert with KEEP_FIRST preserves arguments nearest index zero when sanitization budget is exceeded`() { + val first = SettingsScreen("privacy") + val middle = ProfileScreen("123") + val last = HomeScreen() + val sut = + getSut( + entryMapper = { key -> + entryInfo( + key, + when (key) { + is SettingsScreen -> mapOf("section" to key.section) + is ProfileScreen -> mapOf("values" to List(999) { it }) + is HomeScreen -> mapOf("home" to true) + else -> emptyMap() + }, + ) + } + ) + + val routes = sut.convert(listOf(first, middle, last), RetentionPolicy.KEEP_FIRST) + + assertThat(routes) + .containsExactly( + NormalizedSentryBackStackEntry("/SettingsScreen", mapOf("section" to "privacy")), + NormalizedSentryBackStackEntry("/ProfileScreen"), + NormalizedSentryBackStackEntry("/HomeScreen"), + ) + .inOrder() + } + + @Test + fun `convert with KEEP_LAST preserves arguments nearest lastIndex when sanitization budget is exceeded`() { + val first = HomeScreen() + val middle = ProfileScreen("123") + val last = SettingsScreen("privacy") + val sut = + getSut( + entryMapper = { key -> + entryInfo( + key, + when (key) { + is HomeScreen -> mapOf("home" to true) + is ProfileScreen -> mapOf("values" to List(999) { it }) + is SettingsScreen -> mapOf("section" to key.section) + else -> emptyMap() + }, + ) + } + ) + + val routes = sut.convert(listOf(first, middle, last), RetentionPolicy.KEEP_LAST) + + assertThat(routes) + .containsExactly( + NormalizedSentryBackStackEntry("/HomeScreen"), + NormalizedSentryBackStackEntry("/ProfileScreen"), + NormalizedSentryBackStackEntry("/SettingsScreen", mapOf("section" to "privacy")), + ) + .inOrder() + } + + @Test + fun `convert with KEEP_FIRST treats entries as distinct by position even if structurally equal`() { + val first = ProductScreen("sku-1") + val middle = ProfileScreen("123") + val last = ProductScreen("sku-1") + val sut = + getSut( + entryMapper = { entry -> + entryInfo( + entry, + when (entry) { + is ProductScreen -> mapOf("productId" to entry.productId) + is ProfileScreen -> mapOf("values" to List(999) { it }) + else -> emptyMap() + }, + ) + } + ) + + val routes = sut.convert(listOf(first, middle, last), RetentionPolicy.KEEP_FIRST) + + assertThat(routes) + .containsExactly( + NormalizedSentryBackStackEntry("/ProductScreen", mapOf("productId" to "sku-1")), + NormalizedSentryBackStackEntry("/ProfileScreen"), + NormalizedSentryBackStackEntry("/ProductScreen"), + ) + .inOrder() + } + + @Test + fun `convert with KEEP_LAST treats entries as distinct by position even if structurally equal`() { + val first = ProductScreen("sku-1") + val middle = ProfileScreen("123") + val last = ProductScreen("sku-1") + val sut = + getSut( + entryMapper = { entry -> + entryInfo( + entry, + when (entry) { + is ProductScreen -> mapOf("productId" to entry.productId) + is ProfileScreen -> mapOf("values" to List(999) { it }) + else -> emptyMap() + }, + ) + } + ) + + val routes = sut.convert(listOf(first, middle, last), RetentionPolicy.KEEP_LAST) + + assertThat(routes) + .containsExactly( + NormalizedSentryBackStackEntry("/ProductScreen"), + NormalizedSentryBackStackEntry("/ProfileScreen"), + NormalizedSentryBackStackEntry("/ProductScreen", mapOf("productId" to "sku-1")), + ) + .inOrder() + } + + @Test + fun `convert trims mapper name`() { + val sut = getSut(entryMapper = { SentryBackStackEntry(name = " /profile ") }) + + assertThat(sut.convert(ProfileScreen("123"))) + .isEqualTo(NormalizedSentryBackStackEntry(name = "/profile")) + } + + @Test + fun `convert adds leading slash to mapper name if absent`() { + val sut = getSut(entryMapper = { SentryBackStackEntry(name = "profile") }) + + assertThat(sut.convert(ProfileScreen("123"))) + .isEqualTo(NormalizedSentryBackStackEntry(name = "/profile")) + } + + @Test + fun `convert preserves leading slash in mapper name if already present`() { + val sut = getSut(entryMapper = { SentryBackStackEntry(name = "/profile") }) + + assertThat(sut.convert(ProfileScreen("123"))) + .isEqualTo(NormalizedSentryBackStackEntry(name = "/profile")) + } + + @Test + fun `convert returns unknown name if mapper name is blank`() { + val sut = getSut(entryMapper = { SentryBackStackEntry(" ", mapOf("userId" to "123")) }) + + assertThat(sut.convert(HomeScreen())) + .isEqualTo( + NormalizedSentryBackStackEntry(name = "/unknown", arguments = mapOf("userId" to "123")) + ) + verify(logger) + .log( + eq(WARNING), + eq( + "Nav3 backStackEntryMapper returned a blank name while processing this back stack update. " + + "Using /unknown instead." + ), + ) + } + + @Test + fun `convert returns unknown name without arguments if mapper throws`() { + val sut = getSut(entryMapper = { error("boom") }) + + assertThat(sut.convert(HomeScreen())) + .isEqualTo(NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)) + verify(logger) + .log( + eq(WARNING), + eq( + "Nav3 backStackEntryMapper threw while resolving an entry. Using /unknown without arguments instead." + ), + org.mockito.kotlin.any(), + ) + } + + @Test + fun `convert returns arguments in proper serializable form`() { + val text = StringBuilder("hello") + val sut = + getSut( + entryMapper = { entry -> + entryInfo( + entry, + mapOf( + "str" to "hello", + "charSequence" to text, + "char" to 'x', + "num" to 42, + "bool" to true, + "enum" to PrivacyMode.PRIVATE, + "nil" to null, + "nested" to mapOf("inner" to "value"), + "tags" to listOf("a", "b", "c"), + "array" to arrayOf("a", 1, false, PrivacyMode.PUBLIC, 'z'), + "ints" to intArrayOf(1, 2, 3), + "chars" to charArrayOf('a', 'b'), + "bytes" to byteArrayOf(4, 5), + ), + ) + } + ) + + assertThat(sut.convert(HomeScreen()).arguments) + .isEqualTo( + mapOf( + "str" to "hello", + "charSequence" to "hello", + "char" to "x", + "num" to 42, + "bool" to true, + "enum" to "PRIVATE", + "nil" to null, + "nested" to mapOf("inner" to "value"), + "tags" to listOf("a", "b", "c"), + "array" to listOf("a", 1, false, "PUBLIC", "z"), + "ints" to listOf(1, 2, 3), + "chars" to listOf("a", "b"), + "bytes" to listOf(4.toByte(), 5.toByte()), + ) + ) + } + + @Test + fun `convert sanitizes arguments with nested supported containers recursively`() { + val sut = + getSut( + entryMapper = { entry -> + entryInfo( + entry, + mapOf( + "nested" to + mapOf( + "items" to + arrayOf( + StringBuilder("x"), + listOf('y', PrivacyMode.PRIVATE), + booleanArrayOf(true, false), + charArrayOf('q'), + ) + ) + ), + ) + } + ) + + assertThat(sut.convert(HomeScreen()).arguments) + .isEqualTo( + mapOf( + "nested" to + mapOf("items" to listOf("x", listOf("y", "PRIVATE"), listOf(true, false), listOf("q"))) + ) + ) + } + + @Test + fun `convert coerces arguments with unsupported values to strings`() { + class OpaqueValue { + override fun toString(): String = "opaque-value" + } + + val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("bad" to OpaqueValue())) }) + + assertThat(sut.convert(HomeScreen()).arguments).isEqualTo(mapOf("bad" to "opaque-value")) + } + + @Test + fun `convert logs an unsupported value warning once per back stack update`() { + class OpaqueValue { + override fun toString(): String = "opaque-value" + } + + val sut = + getSut( + entryMapper = { entry -> + entryInfo( + entry, + when (entry) { + is HomeScreen -> mapOf("bad" to OpaqueValue()) + is ProfileScreen -> mapOf("alsoBad" to OpaqueValue()) + else -> emptyMap() + }, + ) + } + ) + + sut.convert(listOf(HomeScreen(), ProfileScreen("123")), RetentionPolicy.KEEP_FIRST) + + verify(logger, times(1)) + .log( + eq(WARNING), + eq( + "Nav3 backStackEntryMapper returned unsupported argument value of type %s while processing this " + + "back stack update. Falling back to toString(). Use String, CharSequence, Char, " + + "Number, Boolean, Enum, Map, Collection, object Array, and primitive array values " + + "for reliable results." + ), + eq("OpaqueValue"), + ) + } + + @Test + fun `unsupported value warning can recur with a fresh update state`() { + class OpaqueValue { + override fun toString(): String = "opaque-value" + } + + val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("bad" to OpaqueValue())) }) + + sut.convert(HomeScreen()) + clearInvocations(logger) + + sut.convert(HomeScreen()) + + verify(logger, times(1)) + .log( + eq(WARNING), + eq( + "Nav3 backStackEntryMapper returned unsupported argument value of type %s while processing this " + + "back stack update. Falling back to toString(). Use String, CharSequence, Char, " + + "Number, Boolean, Enum, Map, Collection, object Array, and primitive array values " + + "for reliable results." + ), + eq("OpaqueValue"), + ) + } + + @Test + fun `convert returns empty arguments for cyclic structures`() { + val cyclic = mutableMapOf() + cyclic["self"] = cyclic + + val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("cyclic" to cyclic)) }) + + assertThat(sut.convert(HomeScreen()).arguments).isEmpty() + } + + @Test + fun `convert returns empty arguments for deeply nested structures`() { + var nested: Any? = "value" + repeat(25) { nested = listOf(nested) } + + val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("nested" to nested)) }) + + assertThat(sut.convert(ProfileScreen("123")).arguments).isEmpty() + } + + @Test + fun `convert drops oversized arguments`() { + val sut = + getSut(entryMapper = { entry -> entryInfo(entry, mapOf("values" to List(1_001) { it })) }) + + assertThat(sut.convert(HomeScreen()).arguments).isEmpty() + } + + @Test + fun `arguments do not use caller collection size for allocation`() { + val values = + object : AbstractCollection() { + var wasSizeRead = false + + override val size: Int + get() { + wasSizeRead = true + return 2 + } + + override fun iterator(): MutableIterator = mutableListOf(1, 2).iterator() + } + val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("values" to values)) }) + + assertThat(sut.convert(HomeScreen()).arguments).isEqualTo(mapOf("values" to listOf(1, 2))) + assertThat(values.wasSizeRead).isFalse() + } + + @Test + fun `omitted arguments normalize to an empty map`() { + val sut = getSut() + + assertThat(sut.convert(HomeScreen()).arguments).isEmpty() + } + + @Test + fun `serialize returns entries in serialized form`() { + val sut = + getSut( + entryMapper = { entry -> + entryInfo( + entry, + when (entry) { + is HomeScreen -> mapOf("tab" to entry.id) + is ProfileScreen -> emptyMap() + is SettingsScreen -> mapOf("section" to entry.section) + else -> emptyMap() + }, + ) + } + ) + + val routes = + sut.convert( + listOf(SettingsScreen("privacy"), ProfileScreen("123")), + RetentionPolicy.KEEP_FIRST, + ) + + assertThat(routes.map(NormalizedSentryBackStackEntry::serialize)) + .containsExactly( + mapOf("entry" to "/SettingsScreen", "arguments" to mapOf("section" to "privacy")), + mapOf("entry" to "/ProfileScreen"), + ) + .inOrder() + } +} 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 5c510ac9fd2..b53397b5384 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 @@ -27,13 +27,13 @@ import org.mockito.kotlin.whenever class BackStackObserverTest { - private data class HomeRoute(val id: String = "home") + private data class HomeScreen(val id: String = "home") - private data class ProfileRoute(val userId: String) + private data class ProfileScreen(val userId: String) - private data class CartRoute(val productId: String) + private data class CartScreen(val productId: String) - private data class SettingsRoute(val section: String) + private data class SettingsScreen(val section: String) private data class ObserverConfig( val enableNavigationBreadcrumbs: Boolean = true, @@ -44,8 +44,10 @@ class BackStackObserverTest { ) private class Fixture { - private val defaultNameExtractor = - RouteNameExtractor { entry -> entry::class.simpleName ?: "unknown" } + private val defaultEntryMapper = + BackStackEntryMapper { entry -> + SentryBackStackEntry(entry::class.simpleName ?: "unknown") + } val logger = mock() val scope = Scope(createOptions(logger)) @@ -83,8 +85,7 @@ class BackStackObserverTest { fun getSut( config: ObserverConfig = ObserverConfig(), - nameExtractor: RouteNameExtractor = defaultNameExtractor, - argumentsExtractor: RouteArgumentsExtractor? = null, + entryMapper: BackStackEntryMapper = defaultEntryMapper, ): BackStackObserver { scope.options.isEnableScreenTracking = config.enableScreenTracking @@ -97,7 +98,7 @@ class BackStackObserverTest { captureBackStack = config.captureBackStack maxCapturedBackStackEntries = config.maxCapturedBackStackEntries }, - extractors = { RouteExtractors(nameExtractor, argumentsExtractor) }, + entryMapper = ForwardingBackStackEntryMapper { entryMapper }, ) } @@ -121,17 +122,19 @@ class BackStackObserverTest { val sut = fixture.getSut( config = ObserverConfig(enableNavigationBreadcrumbs = true), - argumentsExtractor = - RouteArgumentsExtractor { entry -> + entryMapper = { entry -> + SentryBackStackEntry( + entry::class.simpleName ?: "unknown", when (entry) { - is HomeRoute -> mapOf("tab" to entry.id) - is ProfileRoute -> mapOf("userId" to entry.userId) + is HomeScreen -> mapOf("tab" to entry.id) + is ProfileScreen -> mapOf("userId" to entry.userId) else -> emptyMap() - } - }, + }, + ) + }, ) - val home = HomeRoute() - val profile = ProfileRoute("123") + val home = HomeScreen() + val profile = ProfileScreen("123") sut.onBackStackChanged(listOf(home)) sut.onBackStackChanged(listOf(home, profile)) @@ -142,11 +145,11 @@ class BackStackObserverTest { assertThat(breadcrumb.data) .containsExactly( "from", - "/HomeRoute", + "/HomeScreen", "from_arguments", mapOf("tab" to "home"), "to", - "/ProfileRoute", + "/ProfileScreen", "to_arguments", mapOf("userId" to "123"), ) @@ -157,37 +160,37 @@ class BackStackObserverTest { @Test fun `onBackStackChanged reuses the previous top snapshot for breadcrumb from payload`() { val fixture = Fixture() - val previousProfile = ProfileRoute("123") - val replacementProfile = ProfileRoute("123") + val previousProfile = ProfileScreen("123") + val replacementProfile = ProfileScreen("123") var profileName = "profile" var profileArguments = mapOf("userId" to "123") val sut = fixture.getSut( - nameExtractor = - RouteNameExtractor { entry -> - when (entry) { - is HomeRoute -> "home" - is ProfileRoute -> profileName - is SettingsRoute -> "settings" - else -> error("unknown route: $entry") - } - }, - argumentsExtractor = - RouteArgumentsExtractor { entry -> - when (entry) { - is HomeRoute -> mapOf("tab" to entry.id) - is ProfileRoute -> profileArguments - is SettingsRoute -> mapOf("section" to entry.section) - else -> emptyMap() - } - }, + entryMapper = { entry -> + SentryBackStackEntry( + name = + when (entry) { + is HomeScreen -> "home" + is ProfileScreen -> profileName + is SettingsScreen -> "settings" + else -> error("unknown entry: $entry") + }, + arguments = + when (entry) { + is HomeScreen -> mapOf("tab" to entry.id) + is ProfileScreen -> profileArguments + is SettingsScreen -> mapOf("section" to entry.section) + else -> emptyMap() + }, + ) + } ) - sut.onBackStackChanged(listOf(HomeRoute(), previousProfile)) + sut.onBackStackChanged(listOf(HomeScreen(), previousProfile)) profileName = "mutated-profile" profileArguments = mapOf("userId" to "999") - sut.onBackStackChanged(listOf(HomeRoute(), replacementProfile, SettingsRoute("privacy"))) + sut.onBackStackChanged(listOf(HomeScreen(), replacementProfile, SettingsScreen("privacy"))) assertThat(fixture.breadcrumbs.last().data) .containsExactly( @@ -207,7 +210,7 @@ class BackStackObserverTest { val fixture = Fixture() val sut = fixture.getSut(config = ObserverConfig(enableNavigationBreadcrumbs = false)) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) assertThat(fixture.breadcrumbs).isEmpty() } @@ -217,10 +220,10 @@ class BackStackObserverTest { val fixture = Fixture() val sut = fixture.getSut(config = ObserverConfig(enableScreenTracking = true)) - sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"))) + sut.onBackStackChanged(listOf(HomeScreen(), ProfileScreen("123"))) - assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") - assertThat(fixture.scope.contexts.app?.viewNames).isEqualTo(listOf("/ProfileRoute")) + assertThat(fixture.scope.screen).isEqualTo("/ProfileScreen") + assertThat(fixture.scope.contexts.app?.viewNames).isEqualTo(listOf("/ProfileScreen")) } @Test @@ -228,7 +231,7 @@ class BackStackObserverTest { val fixture = Fixture() val sut = fixture.getSut(config = ObserverConfig(enableScreenTracking = false)) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) assertThat(fixture.scope.screen).isNull() assertThat(fixture.scope.contexts.app?.viewNames).isNull() @@ -243,10 +246,10 @@ class BackStackObserverTest { config = ObserverConfig(captureBackStack = true, maxCapturedBackStackEntries = 2) ) - sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"), SettingsRoute("privacy"))) + sut.onBackStackChanged(listOf(HomeScreen(), ProfileScreen("123"), SettingsScreen("privacy"))) assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/SettingsRoute"), mapOf("route" to "/ProfileRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/SettingsScreen"), mapOf("entry" to "/ProfileScreen"))) } @Test @@ -255,23 +258,25 @@ class BackStackObserverTest { val sut = fixture.getSut( config = ObserverConfig(captureBackStack = true), - argumentsExtractor = - RouteArgumentsExtractor { entry -> + entryMapper = { entry -> + SentryBackStackEntry( + entry::class.simpleName ?: "unknown", when (entry) { - is HomeRoute -> mapOf("values" to List(999) { it }) - is ProfileRoute -> mapOf("userId" to entry.userId) + is HomeScreen -> mapOf("values" to List(999) { it }) + is ProfileScreen -> mapOf("userId" to entry.userId) else -> emptyMap() - } - }, + }, + ) + }, ) - sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"))) + sut.onBackStackChanged(listOf(HomeScreen(), ProfileScreen("123"))) assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("route" to "/ProfileRoute", "args" to mapOf("userId" to "123")), - mapOf("route" to "/HomeRoute"), + mapOf("entry" to "/ProfileScreen", "arguments" to mapOf("userId" to "123")), + mapOf("entry" to "/HomeScreen"), ) ) assertThat(fixture.startedTransactions.single().getData("arguments")) @@ -282,21 +287,21 @@ class BackStackObserverTest { fun `onBackStackChanged emits an updated copy of the back stack even when the top entry is unchanged`() { val fixture = Fixture() val sut = fixture.getSut(config = ObserverConfig(captureBackStack = true)) - val home = HomeRoute() - val profile = ProfileRoute("123") + val home = HomeScreen() + val profile = ProfileScreen("123") sut.onBackStackChanged(listOf(home, profile)) - sut.onBackStackChanged(listOf(home, SettingsRoute("privacy"), profile)) + sut.onBackStackChanged(listOf(home, SettingsScreen("privacy"), profile)) assertThat(fixture.breadcrumbs).hasSize(1) assertThat(fixture.startedTransactions).hasSize(1) - assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") + assertThat(fixture.scope.screen).isEqualTo("/ProfileScreen") assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("route" to "/ProfileRoute"), - mapOf("route" to "/SettingsRoute"), - mapOf("route" to "/HomeRoute"), + mapOf("entry" to "/ProfileScreen"), + mapOf("entry" to "/SettingsScreen"), + mapOf("entry" to "/HomeScreen"), ) ) } @@ -305,24 +310,24 @@ class BackStackObserverTest { fun `onBackStackChanged emits new top-entry data when the top entry is replaced by an equal new instance`() { val fixture = Fixture() val sut = fixture.getSut(config = ObserverConfig(captureBackStack = true)) - val home = HomeRoute() - val firstProfile = ProfileRoute("123") - val replacementProfile = ProfileRoute("123") + val home = HomeScreen() + val firstProfile = ProfileScreen("123") + val replacementProfile = ProfileScreen("123") sut.onBackStackChanged(listOf(home, firstProfile)) sut.onBackStackChanged(listOf(home, replacementProfile)) assertThat(fixture.breadcrumbs).hasSize(2) - assertThat(fixture.breadcrumbs.last().data["from"]).isEqualTo("/ProfileRoute") - assertThat(fixture.breadcrumbs.last().data["to"]).isEqualTo("/ProfileRoute") + assertThat(fixture.breadcrumbs.last().data["from"]).isEqualTo("/ProfileScreen") + assertThat(fixture.breadcrumbs.last().data["to"]).isEqualTo("/ProfileScreen") assertThat(fixture.breadcrumbHints.last().get(TypeCheckHint.ANDROID_NAV3_DESTINATION)) .isSameInstanceAs(replacementProfile) assertThat(fixture.startedTransactions).hasSize(2) - assertThat(fixture.startedTransactions.last().name).isEqualTo("/ProfileRoute") + assertThat(fixture.startedTransactions.last().name).isEqualTo("/ProfileScreen") assertThat(fixture.startedTransactions.first().isFinished).isTrue() - assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") + assertThat(fixture.scope.screen).isEqualTo("/ProfileScreen") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/ProfileRoute"), mapOf("route" to "/HomeRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/ProfileScreen"), mapOf("entry" to "/HomeScreen"))) } @Test @@ -334,19 +339,19 @@ class BackStackObserverTest { ) fixture.scope.setContexts( "navigation", - mapOf("backstack" to listOf(mapOf("route" to "/Stale"))), + mapOf("backstack" to listOf(mapOf("entry" to "/Stale"))), ) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) // Doesn't emit a back stack... assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() // ...but continues to emit all other Sentry data. - assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/HomeRoute") + assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/HomeScreen") assertThat(fixture.startedTransactions).hasSize(1) - assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeRoute") - assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeScreen") + assertThat(fixture.scope.screen).isEqualTo("/HomeScreen") } @Test @@ -355,19 +360,19 @@ class BackStackObserverTest { val sut = fixture.getSut(config = ObserverConfig(captureBackStack = false)) fixture.scope.setContexts( "navigation", - mapOf("backstack" to listOf(mapOf("route" to "/Stale"))), + mapOf("backstack" to listOf(mapOf("entry" to "/Stale"))), ) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) // Doesn't emit a back stack... assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() // ...but continues to emit all other Sentry data. - assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/HomeRoute") + assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/HomeScreen") assertThat(fixture.startedTransactions).hasSize(1) - assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeRoute") - assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeScreen") + assertThat(fixture.scope.screen).isEqualTo("/HomeScreen") } // Like `onBackStackChanged does not emit a back stack copy when back stack capture is disabled`, @@ -375,58 +380,56 @@ class BackStackObserverTest { @Test fun `onBackStackChanged skips lower back stack resolution when back stack capture is disabled`() { val fixture = Fixture() - val home = HomeRoute() - val profile = ProfileRoute("123") - val nameCalls = mutableMapOf() - val argumentCalls = mutableMapOf() + val home = HomeScreen() + val profile = ProfileScreen("123") + val mapperCalls = mutableMapOf() val sut = fixture.getSut( config = ObserverConfig(captureBackStack = false), - nameExtractor = { entry -> - nameCalls[entry] = (nameCalls[entry] ?: 0) + 1 - entry::class.simpleName ?: "unknown" - }, - argumentsExtractor = { entry -> - argumentCalls[entry] = (argumentCalls[entry] ?: 0) + 1 - when (entry) { - is HomeRoute -> mapOf("tab" to entry.id) - is ProfileRoute -> mapOf("userId" to entry.userId) - else -> emptyMap() - } + entryMapper = { entry -> + mapperCalls[entry] = (mapperCalls[entry] ?: 0) + 1 + SentryBackStackEntry( + entry::class.simpleName ?: "unknown", + when (entry) { + is HomeScreen -> mapOf("tab" to entry.id) + is ProfileScreen -> mapOf("userId" to entry.userId) + else -> emptyMap() + }, + ) }, ) sut.onBackStackChanged(listOf(home, profile)) - assertThat(nameCalls[profile]).isEqualTo(1) - assertThat(argumentCalls[profile]).isEqualTo(1) - assertThat(nameCalls).doesNotContainKey(home) - assertThat(argumentCalls).doesNotContainKey(home) + assertThat(mapperCalls[profile]).isEqualTo(1) + assertThat(mapperCalls).doesNotContainKey(home) } @Test - fun `onBackStackChanged resolves top entry arguments once per update`() { + fun `onBackStackChanged maps each captured entry once per update`() { val fixture = Fixture() - val home = HomeRoute() - val profile = ProfileRoute("123") - val argumentCalls = mutableMapOf() + val home = HomeScreen() + val profile = ProfileScreen("123") + val mapperCalls = mutableMapOf() val sut = fixture.getSut( - argumentsExtractor = - RouteArgumentsExtractor { entry -> - argumentCalls[entry] = (argumentCalls[entry] ?: 0) + 1 + entryMapper = { entry -> + mapperCalls[entry] = (mapperCalls[entry] ?: 0) + 1 + SentryBackStackEntry( + entry::class.simpleName ?: "unknown", when (entry) { - is HomeRoute -> mapOf("tab" to entry.id) - is ProfileRoute -> mapOf("userId" to entry.userId) + is HomeScreen -> mapOf("tab" to entry.id) + is ProfileScreen -> mapOf("userId" to entry.userId) else -> emptyMap() - } - } + }, + ) + } ) sut.onBackStackChanged(listOf(home, profile)) - assertThat(argumentCalls[profile]).isEqualTo(1) - assertThat(argumentCalls[home]).isEqualTo(1) + assertThat(mapperCalls[profile]).isEqualTo(1) + assertThat(mapperCalls[home]).isEqualTo(1) } @Test @@ -435,30 +438,32 @@ class BackStackObserverTest { val sut = fixture.getSut( config = ObserverConfig(enableNavigationTransactions = true), - argumentsExtractor = - RouteArgumentsExtractor { entry -> + entryMapper = { entry -> + SentryBackStackEntry( + entry::class.simpleName ?: "unknown", when (entry) { - is ProfileRoute -> mapOf("userId" to entry.userId) + is ProfileScreen -> mapOf("userId" to entry.userId) else -> emptyMap() - } - }, + }, + ) + }, ) - sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"))) + sut.onBackStackChanged(listOf(HomeScreen(), ProfileScreen("123"))) val transaction = fixture.startedTransactions.single() - assertThat(transaction.name).isEqualTo("/ProfileRoute") + assertThat(transaction.name).isEqualTo("/ProfileScreen") assertThat(transaction.transactionNameSource).isEqualTo(TransactionNameSource.ROUTE) assertThat(transaction.operation).isEqualTo("navigation") assertThat(transaction.spanContext.origin).isEqualTo("auto.navigation.nav3") assertThat(transaction.getData("arguments")).isEqualTo(mapOf("userId" to "123")) - assertThat(transaction.contexts.app?.viewNames).isEqualTo(listOf("/ProfileRoute")) + assertThat(transaction.contexts.app?.viewNames).isEqualTo(listOf("/ProfileScreen")) assertThat(transaction.navigationBackStack()) .isEqualTo( listOf( - mapOf("route" to "/ProfileRoute", "args" to mapOf("userId" to "123")), - mapOf("route" to "/HomeRoute"), + mapOf("entry" to "/ProfileScreen", "arguments" to mapOf("userId" to "123")), + mapOf("entry" to "/HomeScreen"), ) ) assertThat(fixture.scope.transaction).isSameInstanceAs(transaction) @@ -484,7 +489,7 @@ class BackStackObserverTest { } val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) assertThat(transactionOptionsCaptor.firstValue.origin).isEqualTo("auto.navigation.nav3") } @@ -500,14 +505,14 @@ class BackStackObserverTest { fixture.scope.contexts.setApp(scopeApp) val sut = fixture.getSut(config = ObserverConfig(enableScreenTracking = true)) - sut.onBackStackChanged(listOf(HomeRoute(), ProfileRoute("123"))) + sut.onBackStackChanged(listOf(HomeScreen(), ProfileScreen("123"))) val transactionApp = fixture.startedTransactions.single().contexts.app assertThat(transactionApp).isNotNull() assertThat(transactionApp).isNotSameInstanceAs(scopeApp) assertThat(transactionApp?.appName).isEqualTo("Demo App") assertThat(transactionApp?.appIdentifier).isEqualTo("io.sentry.demo") - assertThat(transactionApp?.viewNames).isEqualTo(listOf("/ProfileRoute")) + assertThat(transactionApp?.viewNames).isEqualTo(listOf("/ProfileScreen")) } @Test @@ -516,12 +521,12 @@ class BackStackObserverTest { val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) fixture.scope.setActiveSpan(mock()) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) assertThat(fixture.startedTransactions).hasSize(1) - assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeRoute") + assertThat(fixture.startedTransactions.single().name).isEqualTo("/HomeScreen") assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) - assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + assertThat(fixture.scope.screen).isEqualTo("/HomeScreen") } @Test @@ -536,11 +541,11 @@ class BackStackObserverTest { fixture.scope.transaction = ambientTransaction val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isSameInstanceAs(ambientTransaction) - assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + assertThat(fixture.scope.screen).isEqualTo("/HomeScreen") } @Test @@ -549,7 +554,7 @@ class BackStackObserverTest { val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = false)) val originalPropagationContext = fixture.scope.propagationContext - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isNull() @@ -568,7 +573,7 @@ class BackStackObserverTest { fixture.scope.transaction = staleTransaction val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) assertThat(fixture.startedTransactions).hasSize(1) assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.startedTransactions.single()) @@ -581,11 +586,11 @@ class BackStackObserverTest { .thenReturn(NoOpTransaction.getInstance()) val sut = fixture.getSut(config = ObserverConfig(enableNavigationTransactions = true)) - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) assertThat(fixture.startedTransactions).isEmpty() assertThat(fixture.scope.transaction).isNull() - assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") + assertThat(fixture.scope.screen).isEqualTo("/HomeScreen") } @Test @@ -593,7 +598,7 @@ class BackStackObserverTest { val fixture = Fixture() val sut = fixture.getSut() - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) val transaction = fixture.startedTransactions.single() sut.onBackStackChanged(emptyList()) @@ -608,29 +613,30 @@ class BackStackObserverTest { @Test @Suppress("LongMethod") - fun `onBackStackChanged records unknown route names when destination route name can't be extracted`() { + fun `onBackStackChanged records unknown entry names when destination name can't be mapped`() { val fixture = Fixture() - val home = HomeRoute() - val profile = ProfileRoute(userId = "123") - val cart = CartRoute(productId = "987") - val settings = SettingsRoute(section = "privacy") + val home = HomeScreen() + val profile = ProfileScreen(userId = "123") + val cart = CartScreen(productId = "987") + val settings = SettingsScreen(section = "privacy") val sut = fixture.getSut( - nameExtractor = - RouteNameExtractor { entry -> + entryMapper = { entry -> + SentryBackStackEntry( when (entry) { - is HomeRoute -> "home" - is ProfileRoute -> " " - is CartRoute -> error("throwing in order to simulate a buggy name extractor") - is SettingsRoute -> "settings" - else -> error("unknown route: $entry") + is HomeScreen -> "home" + is ProfileScreen -> " " + is CartScreen -> error("throwing in order to simulate a buggy entry mapper") + is SettingsScreen -> "settings" + else -> error("unknown entry: $entry") } - } + ) + } ) // Navigate to the home screen and verify that a transaction has started and related Sentry data // have been generated (i.e., screen name, breadcrumb, and updated back stack context), as the - // host app's RouteNameExtractor returned a valid route name for the home screen entry. + // host app's BackStackEntryMapper returned a valid name for the home screen entry. sut.onBackStackChanged(listOf(home)) val transaction = fixture.startedTransactions.single() assertThat(transaction.isFinished).isFalse() @@ -638,7 +644,7 @@ class BackStackObserverTest { assertThat(fixture.scope.screen).isEqualTo("/home") assertThat(fixture.scope.contexts.app?.viewNames).isEqualTo(listOf("/home")) assertThat(fixture.breadcrumbs).hasSize(1) - assertThat(fixture.scope.navigationBackStack()).isEqualTo(listOf(mapOf("route" to "/home"))) + assertThat(fixture.scope.navigationBackStack()).isEqualTo(listOf(mapOf("entry" to "/home"))) // Navigate to the profile screen and verify the invalid route name is recorded as /unknown so // the transition history remains intact. @@ -647,19 +653,19 @@ class BackStackObserverTest { assertThat(fixture.startedTransactions).hasSize(2) val profileTransaction = fixture.startedTransactions.last() assertThat(profileTransaction.isFinished).isFalse() - assertThat(profileTransaction.name).isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(profileTransaction.name).isEqualTo(NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME) assertThat(fixture.scope.transaction).isSameInstanceAs(profileTransaction) - assertThat(fixture.scope.screen).isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(fixture.scope.screen).isEqualTo(NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME) assertThat(fixture.scope.contexts.app?.viewNames) - .isEqualTo(listOf(RouteTranslator.UNKNOWN_ROUTE_NAME)) + .isEqualTo(listOf(NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME)) assertThat(fixture.breadcrumbs).hasSize(2) assertThat(fixture.breadcrumbs.last().data) - .containsExactly("from", "/home", "to", RouteTranslator.UNKNOWN_ROUTE_NAME) + .containsExactly("from", "/home", "to", NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME) assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), - mapOf("route" to "/home"), + mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), + mapOf("entry" to "/home"), ) ) @@ -670,25 +676,25 @@ class BackStackObserverTest { assertThat(fixture.startedTransactions).hasSize(3) val cartTransaction = fixture.startedTransactions.last() assertThat(cartTransaction.isFinished).isFalse() - assertThat(cartTransaction.name).isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(cartTransaction.name).isEqualTo(NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME) assertThat(fixture.scope.transaction).isSameInstanceAs(cartTransaction) - assertThat(fixture.scope.screen).isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) + assertThat(fixture.scope.screen).isEqualTo(NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME) assertThat(fixture.scope.contexts.app?.viewNames) - .isEqualTo(listOf(RouteTranslator.UNKNOWN_ROUTE_NAME)) + .isEqualTo(listOf(NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME)) assertThat(fixture.breadcrumbs).hasSize(3) assertThat(fixture.breadcrumbs.last().data) .containsExactly( "from", - RouteTranslator.UNKNOWN_ROUTE_NAME, + NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, "to", - RouteTranslator.UNKNOWN_ROUTE_NAME, + NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, ) assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), - mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), - mapOf("route" to "/home"), + mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), + mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), + mapOf("entry" to "/home"), ) ) @@ -706,14 +712,14 @@ class BackStackObserverTest { assertThat(fixture.scope.contexts.app?.viewNames).isEqualTo(listOf("/settings")) assertThat(fixture.breadcrumbs).hasSize(4) assertThat(fixture.breadcrumbs.last().data) - .containsExactly("from", RouteTranslator.UNKNOWN_ROUTE_NAME, "to", "/settings") + .containsExactly("from", NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, "to", "/settings") assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("route" to "/settings"), - mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), - mapOf("route" to RouteTranslator.UNKNOWN_ROUTE_NAME), - mapOf("route" to "/home"), + mapOf("entry" to "/settings"), + mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), + mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), + mapOf("entry" to "/home"), ) ) } @@ -723,7 +729,7 @@ class BackStackObserverTest { val fixture = Fixture() val sut = fixture.getSut() - sut.onBackStackChanged(listOf(HomeRoute())) + sut.onBackStackChanged(listOf(HomeScreen())) val transaction = fixture.startedTransactions.single() sut.cleanup() diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt new file mode 100644 index 00000000000..5e9b0e7cb6d --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt @@ -0,0 +1,58 @@ +package io.sentry.compose.navigation3 + +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.snapshots.Snapshot +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +class ForwardingBackStackEntryMapperTest { + + private data class HomeScreen(val id: String = "home") + + private data class ProfileScreen(val userId: String) + + @Test + fun `mapper returns name and arguments when present`() { + val screen = ProfileScreen("123") + val sut = + ForwardingBackStackEntryMapper({ + BackStackEntryMapper { entry -> + SentryBackStackEntry("profile-${entry.userId}", mapOf("userId" to entry.userId)) + } + }) + + assertThat(sut.map(screen)) + .isEqualTo(SentryBackStackEntry("profile-123", mapOf("userId" to "123"))) + } + + @Test + fun `mapper returns entry without allocating arguments map when arguments are omitted`() { + val sut = ForwardingBackStackEntryMapper { + BackStackEntryMapper { SentryBackStackEntry(it.id) } + } + + assertThat(sut.map(HomeScreen())).isEqualTo(SentryBackStackEntry("home")) + assertThat(sut.map(HomeScreen()).arguments).isNull() + } + + @Test + fun `mapper hides reads from snapshot observation`() { + val name = mutableStateOf("home") + val sut = ForwardingBackStackEntryMapper { + BackStackEntryMapper { SentryBackStackEntry(name.value) } + } + + assertThat(observeReads { sut.map(HomeScreen()) }).isEqualTo(0) + } + + private fun observeReads(block: () -> Unit): Int { + var reads = 0 + val snapshot = Snapshot.takeSnapshot(readObserver = { reads++ }) + try { + snapshot.enter(block) + } finally { + snapshot.dispose() + } + return reads + } +} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt deleted file mode 100644 index fd0d7ff6e9e..00000000000 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt +++ /dev/null @@ -1,81 +0,0 @@ -package io.sentry.compose.navigation3 - -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.snapshots.Snapshot -import com.google.common.truth.Truth.assertThat -import org.junit.Test - -class RouteExtractorsTest { - - private data class HomeRoute(val id: String = "home") - - private data class ProfileRoute(val userId: String) - - private val defaultNameExtractor = RouteNameExtractor { it.id } - - @Test - fun `getArguments returns null when no arguments extractor is configured`() { - val sut = RouteExtractors(nameExtractor = defaultNameExtractor, argumentsExtractor = null) - - assertThat(sut.getArguments(HomeRoute())).isNull() - } - - @Test - fun `getName delegates to the configured extractor`() { - val route = ProfileRoute("123") - val sut = - RouteExtractors( - nameExtractor = RouteNameExtractor { entry -> "profile-${entry.userId}" }, - argumentsExtractor = null, - ) - - assertThat(sut.getName(route)).isEqualTo("profile-123") - } - - @Test - fun `getArguments delegates to the configured extractor`() { - val route = ProfileRoute("123") - val sut = - RouteExtractors( - nameExtractor = RouteNameExtractor { entry -> entry.userId }, - argumentsExtractor = RouteArgumentsExtractor { entry -> mapOf("userId" to entry.userId) }, - ) - - assertThat(sut.getArguments(route)).isEqualTo(mapOf("userId" to "123")) - } - - @Test - fun `getName hides extractor reads from snapshot observation`() { - val routeName = mutableStateOf("home") - val sut = - RouteExtractors( - nameExtractor = RouteNameExtractor { routeName.value }, - argumentsExtractor = null, - ) - - assertThat(observeReads { sut.getName(HomeRoute()) }).isEqualTo(0) - } - - @Test - fun `getArguments hides extractor reads from snapshot observation`() { - val argumentValue = mutableStateOf("123") - val sut = - RouteExtractors( - nameExtractor = defaultNameExtractor, - argumentsExtractor = RouteArgumentsExtractor { mapOf("userId" to argumentValue.value) }, - ) - - assertThat(observeReads { sut.getArguments(HomeRoute()) }).isEqualTo(0) - } - - private fun observeReads(block: () -> Unit): Int { - var reads = 0 - val snapshot = Snapshot.takeSnapshot(readObserver = { reads++ }) - try { - snapshot.enter(block) - } finally { - snapshot.dispose() - } - return reads - } -} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt deleted file mode 100644 index b6301bce181..00000000000 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt +++ /dev/null @@ -1,539 +0,0 @@ -package io.sentry.compose.navigation3 - -import com.google.common.truth.Truth.assertThat -import io.sentry.ILogger -import io.sentry.SentryLevel.WARNING -import io.sentry.compose.navigation3.RouteTranslator.ArgumentSanitizer -import io.sentry.compose.navigation3.RouteTranslator.RetentionPolicy -import io.sentry.compose.navigation3.RouteTranslator.WarningState -import java.util.AbstractCollection -import org.junit.Test -import org.mockito.kotlin.clearInvocations -import org.mockito.kotlin.eq -import org.mockito.kotlin.mock -import org.mockito.kotlin.times -import org.mockito.kotlin.verify - -class RouteTranslatorTest { - - private data class HomeRoute(val id: String = "home") - - private data class ProfileRoute(val userId: String) - - private data class ProductRoute(val productId: String) - - private data class SettingsRoute(val section: String) - - private enum class PrivacyMode { - PUBLIC, - PRIVATE, - } - - private val logger = mock() - private val defaultNameExtractor = - RouteNameExtractor { entry -> entry::class.simpleName ?: "unknown" } - - private fun getSut( - nameExtractor: RouteNameExtractor = defaultNameExtractor, - argumentsExtractor: RouteArgumentsExtractor? = null, - ): RouteTranslator = - RouteTranslator( - extractors = { RouteExtractors(nameExtractor, argumentsExtractor) }, - logger = logger, - ) - - @Test - fun `translate preserves input order`() { - val sut = getSut() - - val routes = - sut.translate( - listOf(SettingsRoute("privacy"), ProfileRoute("123"), HomeRoute()), - RetentionPolicy.KEEP_FIRST, - ) - - assertThat(routes) - .containsExactly(Route("/SettingsRoute"), Route("/ProfileRoute"), Route("/HomeRoute")) - .inOrder() - } - - @Test - fun `translate preserves input order when top entry is last`() { - val sut = getSut() - - val routes = - sut.translate( - listOf(HomeRoute(), ProfileRoute("123"), SettingsRoute("privacy")), - RetentionPolicy.KEEP_LAST, - ) - - assertThat(routes) - .containsExactly(Route("/HomeRoute"), Route("/ProfileRoute"), Route("/SettingsRoute")) - .inOrder() - } - - @Test - fun `translate returns empty routes for an empty back stack`() { - val sut = getSut() - - assertThat(sut.translate(emptyList(), RetentionPolicy.KEEP_FIRST)).isEmpty() - } - - @Test - fun `translate returns empty routes for an empty back stack when top entry is last`() { - val sut = getSut() - - assertThat(sut.translate(emptyList(), RetentionPolicy.KEEP_LAST)).isEmpty() - } - - @Test - fun `translate with KEEP_FIRST preserves arguments nearest index zero when budget is exceeded`() { - val first = SettingsRoute("privacy") - val middle = ProfileRoute("123") - val last = HomeRoute() - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { key -> - when (key) { - is SettingsRoute -> mapOf("section" to key.section) - is ProfileRoute -> mapOf("values" to List(999) { it }) - is HomeRoute -> mapOf("home" to true) - else -> emptyMap() - } - } - ) - - val routes = sut.translate(listOf(first, middle, last), RetentionPolicy.KEEP_FIRST) - - assertThat(routes) - .containsExactly( - Route("/SettingsRoute", mapOf("section" to "privacy")), - Route("/ProfileRoute"), - Route("/HomeRoute"), - ) - .inOrder() - } - - @Test - fun `translate with KEEP_LAST preserves arguments nearest lastIndex when budget is exceeded`() { - val first = HomeRoute() - val middle = ProfileRoute("123") - val last = SettingsRoute("privacy") - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { key -> - when (key) { - is HomeRoute -> mapOf("home" to true) - is ProfileRoute -> mapOf("values" to List(999) { it }) - is SettingsRoute -> mapOf("section" to key.section) - else -> emptyMap() - } - } - ) - - val routes = sut.translate(listOf(first, middle, last), RetentionPolicy.KEEP_LAST) - - assertThat(routes) - .containsExactly( - Route("/HomeRoute"), - Route("/ProfileRoute"), - Route("/SettingsRoute", mapOf("section" to "privacy")), - ) - .inOrder() - } - - @Test - fun `translate with KEEP_FIRST treats entries as distinct by position even if structurally equal`() { - val first = ProductRoute("sku-1") - val middle = ProfileRoute("123") - val last = ProductRoute("sku-1") - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { entry -> - when (entry) { - is ProductRoute -> mapOf("productId" to entry.productId) - is ProfileRoute -> mapOf("values" to List(999) { it }) - else -> emptyMap() - } - } - ) - - val routes = sut.translate(listOf(first, middle, last), RetentionPolicy.KEEP_FIRST) - - assertThat(routes) - .containsExactly( - Route("/ProductRoute", mapOf("productId" to "sku-1")), - Route("/ProfileRoute"), - Route("/ProductRoute"), - ) - .inOrder() - } - - @Test - fun `translate with KEEP_LAST treats entries as distinct by position even if structurally equal`() { - val first = ProductRoute("sku-1") - val middle = ProfileRoute("123") - val last = ProductRoute("sku-1") - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { entry -> - when (entry) { - is ProductRoute -> mapOf("productId" to entry.productId) - is ProfileRoute -> mapOf("values" to List(999) { it }) - else -> emptyMap() - } - } - ) - - val routes = sut.translate(listOf(first, middle, last), RetentionPolicy.KEEP_LAST) - - assertThat(routes) - .containsExactly( - Route("/ProductRoute"), - Route("/ProfileRoute"), - Route("/ProductRoute", mapOf("productId" to "sku-1")), - ) - .inOrder() - } - - @Test - fun `translate returns translated routes from one pass`() { - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { entry -> - when (entry) { - is HomeRoute -> mapOf("tab" to entry.id) - is ProfileRoute -> mapOf("userId" to entry.userId) - else -> emptyMap() - } - } - ) - - val routes = - sut.translate( - listOf(SettingsRoute("privacy"), ProfileRoute("123")), - RetentionPolicy.KEEP_FIRST, - ) - - assertThat(routes) - .containsExactly( - Route("/SettingsRoute"), - Route("/ProfileRoute", mapOf("userId" to "123")), - ) - .inOrder() - } - - @Test - fun `route serializes to back stack entry shape`() { - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { entry -> - when (entry) { - is HomeRoute -> mapOf("tab" to entry.id) - is ProfileRoute -> emptyMap() - is SettingsRoute -> mapOf("section" to entry.section) - else -> emptyMap() - } - } - ) - - val routes = - sut.translate( - listOf(SettingsRoute("privacy"), ProfileRoute("123")), - RetentionPolicy.KEEP_FIRST, - ) - - assertThat(routes.map(Route::serialize)) - .containsExactly( - mapOf("route" to "/SettingsRoute", "args" to mapOf("section" to "privacy")), - mapOf("route" to "/ProfileRoute"), - ) - .inOrder() - } - - @Test - fun `extractRouteName normalizes a custom name with a leading slash`() { - val sut = getSut(nameExtractor = { "profile" }) - - assertThat(sut.extractRouteName(ProfileRoute("123"), WarningState())).isEqualTo("/profile") - } - - @Test - fun `extractRouteName leaves leading slash on custom name if already present`() { - val sut = getSut(nameExtractor = { "/profile" }) - - assertThat(sut.extractRouteName(ProfileRoute("123"), WarningState())).isEqualTo("/profile") - } - - @Test - fun `extractRouteName returns the configured name extractor result`() { - val sut = getSut() - - assertThat(sut.extractRouteName(HomeRoute(), WarningState())).isEqualTo("/HomeRoute") - } - - @Test - fun `extractRouteName returns unknown when name extractor throws`() { - val sut = getSut(nameExtractor = { error("boom") }) - - assertThat(sut.extractRouteName(HomeRoute(), WarningState())) - .isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) - verify(logger) - .log( - eq(WARNING), - eq("Nav3 nameExtractor threw while resolving a route name. Using /unknown instead."), - org.mockito.kotlin.any(), - ) - } - - @Test - fun `extractRouteName returns unknown when name extractor returns blank`() { - val sut = getSut(nameExtractor = { " " }) - - assertThat(sut.extractRouteName(HomeRoute(), WarningState())) - .isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) - verify(logger) - .log( - eq(WARNING), - eq( - "Nav3 nameExtractor returned a blank route name while processing this back stack update. " + - "Using /unknown instead." - ), - ) - } - - @Test - fun `extractRouteArguments returns supported values in serializable form`() { - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { _ -> - val text = StringBuilder("hello") - mapOf( - "str" to "hello", - "charSequence" to text, - "char" to 'x', - "num" to 42, - "bool" to true, - "enum" to PrivacyMode.PRIVATE, - "nil" to null, - "nested" to mapOf("inner" to "value"), - "tags" to listOf("a", "b", "c"), - "array" to arrayOf("a", 1, false, PrivacyMode.PUBLIC, 'z'), - "ints" to intArrayOf(1, 2, 3), - "chars" to charArrayOf('a', 'b'), - "bytes" to byteArrayOf(4, 5), - ) - } - ) - - assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) - .isEqualTo( - mapOf( - "str" to "hello", - "charSequence" to "hello", - "char" to "x", - "num" to 42, - "bool" to true, - "enum" to "PRIVATE", - "nil" to null, - "nested" to mapOf("inner" to "value"), - "tags" to listOf("a", "b", "c"), - "array" to listOf("a", 1, false, "PUBLIC", "z"), - "ints" to listOf(1, 2, 3), - "chars" to listOf("a", "b"), - "bytes" to listOf(4.toByte(), 5.toByte()), - ) - ) - } - - @Test - fun `extractRouteArguments sanitizes nested supported containers recursively`() { - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { _ -> - mapOf( - "nested" to - mapOf( - "items" to - arrayOf( - StringBuilder("x"), - listOf('y', PrivacyMode.PRIVATE), - booleanArrayOf(true, false), - charArrayOf('q'), - ) - ) - ) - } - ) - - assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) - .isEqualTo( - mapOf( - "nested" to - mapOf("items" to listOf("x", listOf("y", "PRIVATE"), listOf(true, false), listOf("q"))) - ) - ) - } - - @Test - fun `extractRouteArguments coerces unsupported values to strings`() { - class OpaqueValue { - override fun toString(): String = "opaque-value" - } - - val sut = - getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("bad" to OpaqueValue()) }) - - assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) - .isEqualTo(mapOf("bad" to "opaque-value")) - } - - @Test - fun `translate logs unsupported value warning once per back stack update`() { - class OpaqueValue { - override fun toString(): String = "opaque-value" - } - - val sut = - getSut( - argumentsExtractor = - RouteArgumentsExtractor { entry -> - when (entry) { - is HomeRoute -> mapOf("bad" to OpaqueValue()) - is ProfileRoute -> mapOf("alsoBad" to OpaqueValue()) - else -> emptyMap() - } - } - ) - - sut.translate(listOf(HomeRoute(), ProfileRoute("123")), RetentionPolicy.KEEP_FIRST) - - verify(logger, times(1)) - .log( - eq(WARNING), - eq( - "Nav3 argumentsExtractor returned unsupported value of type %s while processing this " + - "back stack update. Falling back to toString(). Use String, CharSequence, Char, " + - "Number, Boolean, Enum, Map, Collection, object Array, and primitive array values " + - "for reliable results." - ), - eq("OpaqueValue"), - ) - } - - @Test - fun `unsupported value warning can recur with a fresh update state`() { - class OpaqueValue { - override fun toString(): String = "opaque-value" - } - - val sut = - getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("bad" to OpaqueValue()) }) - - sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState())) - clearInvocations(logger) - - sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState())) - - verify(logger, times(1)) - .log( - eq(WARNING), - eq( - "Nav3 argumentsExtractor returned unsupported value of type %s while processing this " + - "back stack update. Falling back to toString(). Use String, CharSequence, Char, " + - "Number, Boolean, Enum, Map, Collection, object Array, and primitive array values " + - "for reliable results." - ), - eq("OpaqueValue"), - ) - } - - @Test - fun `extractRouteArguments returns empty if no arguments extractor`() { - val sut = getSut(argumentsExtractor = null) - - assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) - .isEmpty() - } - - @Test - fun `extractRouteArguments returns empty when arguments extractor throws`() { - val sut = getSut(argumentsExtractor = { error("boom") }) - - assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) - .isEmpty() - verify(logger) - .log( - eq(WARNING), - eq("Nav3 argumentsExtractor threw while resolving arguments. Skipping arguments."), - org.mockito.kotlin.any(), - ) - } - - @Test - fun `extractRouteArguments returns empty for cyclic structures`() { - val cyclic = mutableMapOf() - cyclic["self"] = cyclic - - val sut = - getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("cyclic" to cyclic) }) - - assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) - .isEmpty() - } - - @Test - fun `extractRouteArguments returns empty for deeply nested structures`() { - var nested: Any? = "value" - repeat(25) { nested = listOf(nested) } - - val sut = - getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("nested" to nested) }) - - assertThat( - sut.extractRouteArguments(ProfileRoute("123"), ArgumentSanitizer(logger, WarningState())) - ) - .isEmpty() - } - - @Test - fun `extractRouteArguments drops oversized payloads instead of truncating them`() { - val sut = - getSut( - argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("values" to List(1_001) { it }) } - ) - - assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) - .isEmpty() - } - - @Test - fun `extractRouteArguments does not use caller collection size for allocation`() { - val values = - object : AbstractCollection() { - var wasSizeRead = false - - override val size: Int - get() { - wasSizeRead = true - return 2 - } - - override fun iterator(): MutableIterator = mutableListOf(1, 2).iterator() - } - val sut = - getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("values" to values) }) - - assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) - .isEqualTo(mapOf("values" to listOf(1, 2))) - assertThat(values.wasSizeRead).isFalse() - } -} diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryBackStackEntryTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryBackStackEntryTest.kt new file mode 100644 index 00000000000..c4bebc909e6 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryBackStackEntryTest.kt @@ -0,0 +1,90 @@ +package io.sentry.compose.navigation3 + +import com.google.common.truth.Truth.assertThat +import java.lang.reflect.Modifier +import org.junit.Test + +class SentryBackStackEntryTest { + + @Test + fun `equals follows value semantics`() { + val entry = SentryBackStackEntry("profile", mapOf("userId" to "123")) + val structurallyEqual = SentryBackStackEntry("profile", mapOf("userId" to "123")) + val structurallyDifferent = SentryBackStackEntry("settings") + + assertThat(entry).isEqualTo(entry) + assertThat(entry).isEqualTo(structurallyEqual) + assertThat(entry).isNotEqualTo(structurallyDifferent) + assertThat(entry).isNotEqualTo(null) + assertThat(entry).isNotEqualTo("profile") + } + + @Test + fun `equals includes every property`() { + val base = SentryBackStackEntry("profile", mapOf("userId" to "123")) + val instanceFields = + SentryBackStackEntry::class + .java + .declaredFields + .filterNot { Modifier.isStatic(it.modifiers) } + .map { it.name } + + assertThat(propertyMutators.keys).containsExactlyElementsIn(instanceFields) + + propertyMutators.forEach { (propertyName, mutate) -> + val changed = mutate(base) + + assertThat(changed).isNotEqualTo(base) + assertThat(propertyName).isIn(instanceFields) + } + } + + @Test + fun `equal instances share the same hash code`() { + val first = SentryBackStackEntry("profile", mapOf("userId" to "123")) + val second = SentryBackStackEntry("profile", mapOf("userId" to "123")) + + assertThat(first).isEqualTo(second) + assertThat(first.hashCode()).isEqualTo(second.hashCode()) + } + + @Test + fun `hash code includes every property`() { + val base = SentryBackStackEntry("profile", mapOf("userId" to "123")) + val instanceFields = + SentryBackStackEntry::class + .java + .declaredFields + .filterNot { Modifier.isStatic(it.modifiers) } + .map { it.name } + + assertThat(propertyMutators.keys).containsExactlyElementsIn(instanceFields) + + propertyMutators.forEach { (propertyName, mutate) -> + val changed = mutate(base) + + assertThat(changed.hashCode()).isNotEqualTo(base.hashCode()) + assertThat(propertyName).isIn(instanceFields) + } + } + + @Test + fun `toString includes the name but redacts arguments`() { + val info = SentryBackStackEntry("profile", mapOf("userId" to "123")) + + assertThat(info.toString()).isEqualTo("SentryBackStackEntry(name=profile)") + assertThat(info.toString()).doesNotContain("userId") + assertThat(info.toString()).doesNotContain("123") + } + + private companion object { + val propertyMutators = + mapOf SentryBackStackEntry>( + "name" to { entry -> SentryBackStackEntry("settings", entry.arguments) }, + "arguments" to + { entry -> + SentryBackStackEntry(entry.name, mapOf("userId" to "456")) + }, + ) + } +} 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 863d8e4ef34..51efba8f8a7 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 @@ -38,8 +38,10 @@ import org.robolectric.annotation.Config @Config(sdk = [30]) class SentryNavEffectTest { - private val defaultNameExtractor = - RouteNameExtractor { entry -> entry::class.simpleName ?: "unknown" } + private val defaultEntryMapper = + BackStackEntryMapper { entry -> + SentryBackStackEntry(entry::class.simpleName ?: "unknown") + } @get:Rule(order = 1) val addActivityToRobolectricRule = @@ -107,7 +109,7 @@ class SentryNavEffectTest { composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -119,7 +121,7 @@ class SentryNavEffectTest { assertThat(fixture.transactions.single().name).isEqualTo("/HomeRoute") assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/HomeRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/HomeRoute"))) } @Test @@ -130,7 +132,7 @@ class SentryNavEffectTest { composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -146,7 +148,7 @@ class SentryNavEffectTest { assertThat(fixture.transactions.last().name).isEqualTo("/ProfileRoute") assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/ProfileRoute"), mapOf("route" to "/HomeRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/ProfileRoute"), mapOf("entry" to "/HomeRoute"))) } @Test @@ -157,7 +159,7 @@ class SentryNavEffectTest { composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -173,7 +175,7 @@ class SentryNavEffectTest { assertThat(fixture.transactions.last().name).isEqualTo("/HomeRoute") assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/HomeRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/HomeRoute"))) } @Test @@ -184,7 +186,7 @@ class SentryNavEffectTest { composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -204,7 +206,7 @@ class SentryNavEffectTest { assertThat(fixture.transactions.last().name).isEqualTo("/HomeRoute") assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/HomeRoute"), mapOf("route" to "/ProfileRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/HomeRoute"), mapOf("entry" to "/ProfileRoute"))) } @Test @@ -217,7 +219,7 @@ class SentryNavEffectTest { composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -235,9 +237,9 @@ class SentryNavEffectTest { assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("route" to "/ProfileRoute"), - mapOf("route" to "/HomeRoute"), - mapOf("route" to "/HomeRoute"), + mapOf("entry" to "/ProfileRoute"), + mapOf("entry" to "/HomeRoute"), + mapOf("entry" to "/HomeRoute"), ) ) } @@ -250,7 +252,7 @@ class SentryNavEffectTest { composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions { maxCapturedBackStackEntries = 0 }, scopes = fixture.scopes, ) @@ -273,7 +275,7 @@ class SentryNavEffectTest { recomposeTick.intValue SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -289,7 +291,7 @@ class SentryNavEffectTest { assertThat(fixture.transactions.single().name).isEqualTo("/HomeRoute") assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/HomeRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/HomeRoute"))) } /** @@ -309,7 +311,7 @@ class SentryNavEffectTest { composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -329,18 +331,20 @@ class SentryNavEffectTest { } @Test - fun `updated name extractor is used for later navigation changes`() { + fun `updated entry mapper is used for later navigation changes`() { val fixture = Fixture() val backStack = mutableStateListOf(HomeRoute()) - val nameExtractor = - mutableStateOf>( - RouteNameExtractor { entry -> entry::class.simpleName ?: "unknown" } + val entryMapper = + mutableStateOf>( + BackStackEntryMapper { entry -> + SentryBackStackEntry(entry::class.simpleName ?: "unknown") + } ) composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = nameExtractor.value, + backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -348,8 +352,8 @@ class SentryNavEffectTest { composeRule.waitForIdle() composeRule.runOnIdle { - nameExtractor.value = RouteNameExtractor { entry -> - if (entry is ProfileRoute) "profile-updated" else "home-updated" + entryMapper.value = BackStackEntryMapper { entry -> + SentryBackStackEntry(if (entry is ProfileRoute) "profile-updated" else "home-updated") } } composeRule.waitForIdle() @@ -360,22 +364,24 @@ class SentryNavEffectTest { assertThat(fixture.transactions.last().name).isEqualTo("/profile-updated") assertThat(fixture.scope.screen).isEqualTo("/profile-updated") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/profile-updated"), mapOf("route" to "/home-updated"))) + .isEqualTo(listOf(mapOf("entry" to "/profile-updated"), mapOf("entry" to "/home-updated"))) } @Test - fun `changing the name extractor alone does not re-emit Sentry data for the current top entry`() { + fun `changing the entry mapper alone does not re-emit Sentry data for the current top entry`() { val fixture = Fixture() val backStack = mutableStateListOf(HomeRoute(), ProfileRoute("123")) - val nameExtractor = - mutableStateOf>( - RouteNameExtractor { entry -> entry::class.simpleName ?: "unknown" } + val entryMapper = + mutableStateOf>( + BackStackEntryMapper { entry -> + SentryBackStackEntry(entry::class.simpleName ?: "unknown") + } ) composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = nameExtractor.value, + backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -383,8 +389,8 @@ class SentryNavEffectTest { composeRule.waitForIdle() composeRule.runOnIdle { - nameExtractor.value = RouteNameExtractor { entry -> - if (entry is ProfileRoute) "profile-updated" else "home-updated" + entryMapper.value = BackStackEntryMapper { entry -> + SentryBackStackEntry(if (entry is ProfileRoute) "profile-updated" else "home-updated") } } composeRule.waitForIdle() @@ -395,20 +401,19 @@ class SentryNavEffectTest { assertThat(fixture.transactions.single().name).isEqualTo("/ProfileRoute") assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/ProfileRoute"), mapOf("route" to "/HomeRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/ProfileRoute"), mapOf("entry" to "/HomeRoute"))) } @Test - fun `updated arguments extractor is used for later navigation changes`() { + fun `updated entry mapper arguments are used for later navigation changes`() { val fixture = Fixture() val backStack = mutableStateListOf(HomeRoute()) - val argumentsExtractor = mutableStateOf?>(null) + val entryMapper = mutableStateOf(defaultEntryMapper) composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, - argumentsExtractor = argumentsExtractor.value, + backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -416,8 +421,11 @@ class SentryNavEffectTest { composeRule.waitForIdle() composeRule.runOnIdle { - argumentsExtractor.value = RouteArgumentsExtractor { entry -> - if (entry is ProfileRoute) mapOf("userId" to entry.userId) else emptyMap() + entryMapper.value = BackStackEntryMapper { entry -> + SentryBackStackEntry( + entry::class.simpleName ?: "unknown", + if (entry is ProfileRoute) mapOf("userId" to entry.userId) else emptyMap(), + ) } } composeRule.waitForIdle() @@ -432,23 +440,22 @@ class SentryNavEffectTest { assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("route" to "/ProfileRoute", "args" to mapOf("userId" to "123")), - mapOf("route" to "/HomeRoute"), + mapOf("entry" to "/ProfileRoute", "arguments" to mapOf("userId" to "123")), + mapOf("entry" to "/HomeRoute"), ) ) } @Test - fun `changing the arguments extractor alone does not re-emit Sentry data for the current top entry`() { + fun `changing entry mapper arguments alone does not re-emit Sentry data for current top entry`() { val fixture = Fixture() val backStack = mutableStateListOf(HomeRoute(), ProfileRoute("123")) - val argumentsExtractor = mutableStateOf?>(null) + val entryMapper = mutableStateOf(defaultEntryMapper) composeRule.setContent { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, - argumentsExtractor = argumentsExtractor.value, + backStackEntryMapper = entryMapper.value, options = SentryNavOptions(), scopes = fixture.scopes, ) @@ -456,8 +463,11 @@ class SentryNavEffectTest { composeRule.waitForIdle() composeRule.runOnIdle { - argumentsExtractor.value = RouteArgumentsExtractor { entry -> - if (entry is ProfileRoute) mapOf("userId" to entry.userId) else emptyMap() + entryMapper.value = BackStackEntryMapper { entry -> + SentryBackStackEntry( + entry::class.simpleName ?: "unknown", + if (entry is ProfileRoute) mapOf("userId" to entry.userId) else emptyMap(), + ) } } composeRule.waitForIdle() @@ -470,7 +480,7 @@ class SentryNavEffectTest { assertThat(fixture.transactions.single().getData("arguments")).isNull() assertThat(fixture.scope.screen).isEqualTo("/ProfileRoute") assertThat(fixture.scope.navigationBackStack()) - .isEqualTo(listOf(mapOf("route" to "/ProfileRoute"), mapOf("route" to "/HomeRoute"))) + .isEqualTo(listOf(mapOf("entry" to "/ProfileRoute"), mapOf("entry" to "/HomeRoute"))) } @Test @@ -483,7 +493,7 @@ class SentryNavEffectTest { SentryNavEffect( backStack = backStack, options = options.value, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, scopes = fixture.scopes, ) } @@ -513,7 +523,7 @@ class SentryNavEffectTest { if (isShown.value) { SentryNavEffect( backStack = backStack, - nameExtractor = defaultNameExtractor, + backStackEntryMapper = defaultEntryMapper, scopes = fixture.scopes, ) } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt index 742a7c0ed65..a07a1b41a69 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavOptionsTest.kt @@ -33,6 +33,39 @@ class SentryNavOptionsTest { .isEqualTo("maxCapturedBackStackEntries must be non-negative, was -1") } + @Test + fun `equals follows value semantics`() { + val options = SentryNavOptions() + val structurallyEqual = SentryNavOptions() + val structurallyDifferent = SentryNavOptions { captureBackStack = false } + + assertThat(options).isEqualTo(options) + assertThat(options).isEqualTo(structurallyEqual) + assertThat(options).isNotEqualTo(structurallyDifferent) + assertThat(options).isNotEqualTo(null) + assertThat(options).isNotEqualTo("options") + } + + @Test + fun `equals includes every property`() { + val base = SentryNavOptions() + val instanceFields = + SentryNavOptions::class + .java + .declaredFields + .filterNot { Modifier.isStatic(it.modifiers) } + .map { it.name } + + assertThat(propertyMutators.keys).containsExactlyElementsIn(instanceFields) + + propertyMutators.forEach { (propertyName, mutate) -> + val changed = mutate(base) + + assertThat(changed).isNotEqualTo(base) + assertThat(propertyName).isIn(instanceFields) + } + } + @Test fun `equal instances share the same hash code`() { val first = SentryNavOptions() @@ -43,7 +76,7 @@ class SentryNavOptionsTest { } @Test - fun `equals and hash code include every property`() { + fun `hash code includes every property`() { val base = SentryNavOptions() val instanceFields = SentryNavOptions::class @@ -57,7 +90,6 @@ class SentryNavOptionsTest { propertyMutators.forEach { (propertyName, mutate) -> val changed = mutate(base) - assertThat(changed).isNotEqualTo(base) assertThat(changed.hashCode()).isNotEqualTo(base.hashCode()) assertThat(propertyName).isIn(instanceFields) } From d877a7b0924e2be3ba296160d2b7bbec3da0f42c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 1 Oct 2026 16:15:56 +0200 Subject: [PATCH 2/2] Address Max's comments --- .../compose/navigation3/BackStackConverter.kt | 13 ++++++---- .../navigation3/BackStackEntryMapper.kt | 24 ++++++++++--------- .../compose/navigation3/SentryNavEffect.kt | 8 +++++-- .../compose/navigation3/SentryNavOptions.kt | 4 ++-- .../navigation3/BackStackConverterTest.kt | 12 ++++++++-- .../navigation3/BackStackObserverTest.kt | 2 +- .../ForwardingBackStackEntryMapperTest.kt | 11 +++++++-- .../navigation3/SentryNavEffectTest.kt | 2 +- 8 files changed, 51 insertions(+), 25 deletions(-) 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 0785a9a8622..b6d94243941 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 @@ -72,14 +72,19 @@ internal class BackStackConverter( return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME) } + if (info == null) { + return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME) + } + val arguments = info.arguments?.let(sanitizer::sanitizeEntry) ?: emptyMap() val formattedName = NormalizedSentryBackStackEntry.formatName(info.name) - if (formattedName.isBlank()) { + + return if (formattedName.isBlank()) { warningState.logInvalidNameWarning(logger) - return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments) + NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments) + } else { + NormalizedSentryBackStackEntry(formattedName, arguments) } - - return NormalizedSentryBackStackEntry(formattedName, arguments) } /** diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt index c0a79d15a05..2412afffe8b 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt @@ -77,9 +77,9 @@ public class SentryBackStackEntry( * * **Names fall back to "/unknown"** * - * If [map] throws or returns a blank [name][SentryBackStackEntry.name], Sentry records the - * destination as "/unknown". Doing so signals that name extraction needs to be fixed while avoiding - * misleading gaps in navigation data. + * If [map] throws, returns `null`, or returns a blank [name][SentryBackStackEntry.name], Sentry + * records the destination as "/unknown". Doing so signals that name extraction needs to be fixed + * while avoiding misleading gaps in navigation data. * * For instance, if a user navigates from `/home -> /detail -> /settings`, but the mapper for * `/detail` throws, the back stack record will be `/home -> /unknown -> /settings` rather than @@ -109,8 +109,8 @@ public class SentryBackStackEntry( * * **Arguments fall back to `toString()` or nothing** * - * All non-supported argument types are stringified via `toString()`. If [map] throws, no arguments - * are recorded for that back stack entry. + * All non-supported argument types are stringified via `toString()`. If [map] throws or returns + * `null`, no arguments are recorded for that back stack entry. * * **Using kotlinx.serialization** * @@ -123,14 +123,15 @@ public class SentryBackStackEntry( * ```kotlin * @Serializable * @SerialName("Home") - * data object Home(userName: String) : NavKey + * data class Home(userName: String) : NavKey * * @Serializable * @SerialName("ProductDetail") * data class ProductDetail(userName: String, productId: String, tab: Tab) : NavKey - * ``` - * ```kotlin - * val backStackItemMapper = BackStackEntryMapper { entry -> + * + * ... + * + * val backStackItemMapper = BackStackEntryMapper { entry -> * when (entry) { * is Home -> SentryBackStackEntry(Home.serializer().descriptor.serialName) * is ProductDetail -> SentryBackStackEntry( @@ -139,6 +140,7 @@ public class SentryBackStackEntry( * // or non-performant. * arguments = mapOf("product_id" to entry.productId, "tab" to entry.tab) * ) + * ... * } * } * ``` @@ -146,7 +148,7 @@ public class SentryBackStackEntry( @ApiStatus.Experimental @ApiStatus.Internal public fun interface BackStackEntryMapper { - public fun map(backStackEntry: T): SentryBackStackEntry + public fun map(backStackEntry: T): SentryBackStackEntry? } /** @@ -165,7 +167,7 @@ public fun interface BackStackEntryMapper { internal class ForwardingBackStackEntryMapper( private val currentMapper: () -> BackStackEntryMapper ) { - fun map(backStackEntry: T): SentryBackStackEntry = Snapshot.withoutReadObservation { + fun map(backStackEntry: T): SentryBackStackEntry? = Snapshot.withoutReadObservation { currentMapper().map(backStackEntry) } } 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 36619d9c4d7..cdad850ca1c 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 @@ -21,8 +21,8 @@ import org.jetbrains.annotations.ApiStatus * * // Place SentryNavEffect in the same composable as your NavDisplay and call * // the effect first. Doing so ensures the effect's lifecycle matches your - * // NavDisplay, and that any Sentry data produced by your nav destinations - * // get attributed to the appropriate nav transaction. + * // NavDisplay, and that any Sentry data produced by your initial nav + * // destination get attributed to the appropriate nav transaction. * SentryNavEffect( * backStack = navBackStack, * backStackEntryMapper = { entry -> @@ -65,6 +65,10 @@ import org.jetbrains.annotations.ApiStatus * gestures. That means that spans produced by predictively rendered composables can show up under * the current destination's transaction. * + * Multiple simultaneously active `SentryNavEffect` instances writing to the same Sentry scope are + * not supported. Violating this restriction can result in interleaved breadcrumbs, clobbered screen + * names and back stacks, and transactions that interfere with one another. + * * @param backStack The navigation backstack to observe. * @param backStackEntryMapper Maps each entry of the [backStack] to a name and optional arguments * for display in Sentry. See the [BackStackEntryMapper] KDoc for best practices. diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt index a9876e37d17..834bebd4f99 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt @@ -13,7 +13,7 @@ private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 10 * Instances are immutable; create one with the SentryNavOptions DSL: * ```kotlin * val options = SentryNavOptions { - * captureBackStack = false + * enableNavigationBreadcrumbs = false * maxCapturedBackStackEntries = 5 * } * ``` @@ -110,7 +110,7 @@ private constructor( * Creates [SentryNavOptions]. Optionally configure it via [configure]. E.g.: * ```kotlin * val options = SentryNavOptions { - * captureBackStack = false + * enableNavigationBreadcrumbs = false * maxCapturedBackStackEntries = 5 * } * ``` diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt index 97cef3a7e34..96b103a7157 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt @@ -32,7 +32,7 @@ class BackStackConverterTest { private val defaultEntryMapper = BackStackEntryMapper { entry -> - SentryBackStackEntry(entry::class.simpleName ?: "unknown") + SentryBackStackEntry(entry::class.simpleName ?: "") } private fun getSut( @@ -44,7 +44,7 @@ class BackStackConverterTest { ) private fun entryInfo(entry: Any, arguments: Map? = null): SentryBackStackEntry = - SentryBackStackEntry(entry::class.simpleName ?: "unknown", arguments) + SentryBackStackEntry(entry::class.simpleName ?: "", arguments) private fun BackStackConverter.convert(entry: Any): NormalizedSentryBackStackEntry = convert(listOf(entry), RetentionPolicy.KEEP_FIRST).single() @@ -300,6 +300,14 @@ class BackStackConverterTest { ) } + @Test + fun `convert returns unknown name without arguments if mapper returns null`() { + val sut = getSut(entryMapper = { null }) + + assertThat(sut.convert(HomeScreen())) + .isEqualTo(NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)) + } + @Test fun `convert returns unknown name without arguments if mapper throws`() { val sut = getSut(entryMapper = { error("boom") }) 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 b53397b5384..5c8d68d1710 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 @@ -46,7 +46,7 @@ class BackStackObserverTest { private class Fixture { private val defaultEntryMapper = BackStackEntryMapper { entry -> - SentryBackStackEntry(entry::class.simpleName ?: "unknown") + SentryBackStackEntry(entry::class.simpleName ?: "") } val logger = mock() diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt index 5e9b0e7cb6d..313b21d53c2 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt @@ -31,8 +31,15 @@ class ForwardingBackStackEntryMapperTest { BackStackEntryMapper { SentryBackStackEntry(it.id) } } - assertThat(sut.map(HomeScreen())).isEqualTo(SentryBackStackEntry("home")) - assertThat(sut.map(HomeScreen()).arguments).isNull() + val mapped = sut.map(HomeScreen()) + assertThat(mapped).isEqualTo(SentryBackStackEntry("home")) + assertThat(mapped?.arguments).isNull() + } + + @Test + fun `mapper forwards null results`() { + val sut = ForwardingBackStackEntryMapper { BackStackEntryMapper { null } } + assertThat(sut.map(HomeScreen())).isNull() } @Test 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 51efba8f8a7..384226f6830 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 @@ -337,7 +337,7 @@ class SentryNavEffectTest { val entryMapper = mutableStateOf>( BackStackEntryMapper { entry -> - SentryBackStackEntry(entry::class.simpleName ?: "unknown") + SentryBackStackEntry(entry::class.simpleName ?: "") } )