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..b6d94243941 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,93 @@ 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? = + private fun normalize( + backStackEntry: T, + sanitizer: ArgumentSanitizer, + warningState: WarningState, + ): NormalizedSentryBackStackEntry { + val info = try { - extractors.invoke().getName(backStackEntry) + entryMapper.map(backStackEntry) } catch (t: Throwable) { - // Route name extractors are host app callbacks. + // Back stack entry mappers are host app callbacks. ExceptionUtils.rethrowIfFatal(t) - warningState.logNameExtractorFailureWarning(logger, t) - return UNKNOWN_ROUTE_NAME + warningState.logMapperFailureWarning(logger, t) + return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME) } - val normalizedName = name?.trim()?.takeUnless { it.isEmpty() }?.removePrefix("/") - if (normalizedName == null) { - warningState.logInvalidRouteNameWarning(logger) - return UNKNOWN_ROUTE_NAME + if (info == null) { + return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME) } - return "/$normalizedName" - } + val arguments = info.arguments?.let(sanitizer::sanitizeEntry) ?: emptyMap() + val formattedName = NormalizedSentryBackStackEntry.formatName(info.name) - /** - * 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( - backStackEntry: T, - sanitizer: ArgumentSanitizer, - ): Map { - val raw = - try { - extractors.invoke().getArguments(backStackEntry) ?: return emptyMap() - } catch (t: Throwable) { - // Route argument extractors are host app callbacks. - ExceptionUtils.rethrowIfFatal(t) - logger.log( - WARNING, - "Nav3 argumentsExtractor threw while resolving arguments. Skipping arguments.", - t, - ) - return emptyMap() - } - - return sanitizer.sanitizeEntry(raw) + return if (formattedName.isBlank()) { + warningState.logInvalidNameWarning(logger) + NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments) + } else { + 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 +96,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 +110,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 +123,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 +156,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 +170,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 +254,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 +382,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..2412afffe8b --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackEntryMapper.kt @@ -0,0 +1,173 @@ +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, 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 + * `/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 or returns + * `null`, 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 class Home(userName: String) : NavKey + * + * @Serializable + * @SerialName("ProductDetail") + * data class ProductDetail(userName: String, productId: String, tab: Tab) : NavKey + * + * ... + * + * 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..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 @@ -16,16 +16,18 @@ 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 * // 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, - * 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,55 @@ 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. + * + * 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 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 +121,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..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 @@ -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 /** @@ -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 * } * ``` @@ -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 @@ -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 new file mode 100644 index 00000000000..96b103a7157 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt @@ -0,0 +1,563 @@ +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 ?: "") + } + + 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 ?: "", 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 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") }) + + 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..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 @@ -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 ?: "") + } 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..313b21d53c2 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/ForwardingBackStackEntryMapperTest.kt @@ -0,0 +1,65 @@ +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) } + } + + 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 + 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..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 @@ -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 ?: "") + } ) 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) }