From 01c0ad0caf4847d992b70363e32d462e7ce3be73 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Sat, 12 Sep 2026 07:00:33 +0200 Subject: [PATCH 1/5] feat(android-nav3): Model navigation routes Introduce the route translation layer for Navigation 3 so later stages can reason about route names and sanitized arguments in one place. At this stage the module still exposes no meaningful public API surface. --- gradle/libs.versions.toml | 1 + .../api/sentry-android-navigation3.api | 0 sentry-android-navigation3/build.gradle.kts | 12 + .../compose/navigation3/RouteExtractors.kt | 137 ++++++ .../compose/navigation3/RouteTranslator.kt | 356 +++++++++++++++ .../compose/navigation3/RouteResolversTest.kt | 83 ++++ .../navigation3/RouteTranslatorTest.kt | 419 ++++++++++++++++++ 7 files changed, 1008 insertions(+) create mode 100644 sentry-android-navigation3/api/sentry-android-navigation3.api create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt create mode 100644 sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteResolversTest.kt create mode 100644 sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index fbe9ef0177..ad2e5624b9 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -90,6 +90,7 @@ androidx-activity-compose = { module = "androidx.activity:activity-compose", ver androidx-compose-foundation = { module = "androidx.compose.foundation:foundation", version.ref = "androidxCompose" } androidx-compose-foundation-layout = { module = "androidx.compose.foundation:foundation-layout", version.ref = "androidxCompose" } androidx-compose-material3 = { module = "androidx.compose.material3:material3", version = "1.4.0" } +androidx-compose-runtime = { module = "androidx.compose.runtime:runtime", version.ref = "androidxCompose" } androidx-compose-material-icons-core = { module = "androidx.compose.material:material-icons-core", version="1.7.8" } androidx-compose-material-icons-extended = { module = "androidx.compose.material:material-icons-extended", version="1.7.8" } androidx-compose-ui = { module = "androidx.compose.ui:ui", version.ref = "androidxCompose" } diff --git a/sentry-android-navigation3/api/sentry-android-navigation3.api b/sentry-android-navigation3/api/sentry-android-navigation3.api new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sentry-android-navigation3/build.gradle.kts b/sentry-android-navigation3/build.gradle.kts index f967385a78..3e35211b22 100644 --- a/sentry-android-navigation3/build.gradle.kts +++ b/sentry-android-navigation3/build.gradle.kts @@ -47,6 +47,18 @@ android { kotlin { explicitApi() } +dependencies { + implementation(projects.sentry) + + compileOnly(libs.androidx.compose.runtime) + + testImplementation(libs.androidx.compose.runtime) + testImplementation(libs.google.truth) + testImplementation(libs.kotlin.test.junit) + testImplementation(libs.mockito.inline) + testImplementation(libs.mockito.kotlin) +} + tasks.withType().configureEach { // Target version of the generated JVM bytecode. It is used for type resolution. jvmTarget = JavaVersion.VERSION_1_8.toString() 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 new file mode 100644 index 0000000000..deac06f2ea --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteExtractors.kt @@ -0,0 +1,137 @@ +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 stable 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. + * + * **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, you may want to 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 prevents route names from being obfuscated while leaving per-route arguments to + * [RouteArgumentsExtractor]. + */ +@ApiStatus.Experimental +internal fun interface RouteNameExtractor { + 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 performant route arguments** + * + * Return only a small subset of route data useful for diagnostics. Data should be stable enough to + * inspect in Sentry. + * + * 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** + * + * Even if your back stack contains `@Serializable` route types, consider mapping each route type to + * a small set of diagnostic arguments to avoid the cost of serializing and returning the entire + * route object. 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 +internal fun interface RouteArgumentsExtractor { + 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 RouteResolvers( + 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/RouteTranslator.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt new file mode 100644 index 0000000000..32845bec8e --- /dev/null +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/RouteTranslator.kt @@ -0,0 +1,356 @@ +package io.sentry.compose.navigation3 + +import io.sentry.ILogger +import io.sentry.SentryLevel.WARNING +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. */ +internal class RouteTranslator( + private val resolvers: () -> RouteResolvers, + 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. + * + * Callers should provide entries newest first so the shared argument budget preserves data for + * the destinations most relevant to the current navigation state. + */ + fun translate(backStackEntries: List): List { + val warningState = WarningState() + val sanitizer = ArgumentSanitizer(logger, warningState) + + return backStackEntries.map { entry -> resolveRoute(entry, warningState, sanitizer) } + } + + private fun resolveRoute( + backStackEntry: T, + warningState: WarningState, + sanitizer: ArgumentSanitizer, + ): Route = + Route( + name = resolveRouteName(backStackEntry, warningState), + arguments = resolveArguments(backStackEntry, sanitizer), + ) + + /** + * Returns a route name for the provided [backStackEntry], based on this translator's + * [name extractor][RouteResolvers.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 resolveRouteName(backStackEntry: T, warningState: WarningState): String { + val name: String? = + try { + resolvers.invoke().getName(backStackEntry) + } catch (t: Throwable) { + // Route name extractors are host app callbacks. + ExceptionUtils.rethrowIfFatal(t) + warningState.logNameExtractorFailureWarning(logger, t) + return UNKNOWN_ROUTE_NAME + } + + val normalizedName = name?.trim()?.takeUnless { it.isEmpty() }?.removePrefix("/") + if (normalizedName == null) { + warningState.logInvalidRouteNameWarning(logger) + return UNKNOWN_ROUTE_NAME + } + + return "/$normalizedName" + } + + /** + * Returns the arguments for the provided [backStackEntry], based on this translator's + * [arguments extractor][RouteResolvers.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 resolveArguments( + backStackEntry: T, + sanitizer: ArgumentSanitizer, + ): Map { + val raw = + try { + resolvers.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) + } + + /** + * Sanitizes a back stack update's argument maps into 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 + * 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. + */ + internal class ArgumentSanitizer( + private val logger: ILogger, + private val warningState: WarningState, + ) { + + private val activeContainers = IdentityHashMap() + private var remainingValues = MAX_ARGUMENT_VALUES + private var budgetExhausted = false + + /** + * Sanitizes one entry's arguments, or returns an empty map to drop them, either because the + * structure is cyclic or too deeply nested (this entry only), or because the shared per-update + * value budget is spent (this entry and every older one). + */ + @Suppress("TooGenericExceptionCaught") + fun sanitizeEntry(raw: Map): Map { + if (budgetExhausted) { + return emptyMap() + } + + return try { + sanitizeMap(raw, depth = 0) + } catch (drop: DropSubtree) { + if (drop.exhaustsBudget) { + budgetExhausted = true + } + logger.log(WARNING, drop.warning) + emptyMap() + } catch (t: Throwable) { + // Extracted maps may invoke host app code while iterating or stringifying values. + ExceptionUtils.rethrowIfFatal(t) + logger.log(WARNING, STRUCTURE_WARNING, t) + emptyMap() + } + } + + private fun sanitizeMap(value: Map<*, *>, depth: Int): Map { + enter(value) + try { + val sanitized = LinkedHashMap() + for ((key, childValue) in value) { + sanitized[key.toString()] = sanitizeValue(childValue, depth + 1) + } + return sanitized + } finally { + exit(value) + } + } + + private fun sanitizeCollection(value: Collection<*>, depth: Int): List { + enter(value) + try { + // The value budget bounds allocation instead of the caller-provided collection size. + val sanitized = ArrayList() + for (childValue in value) { + sanitized += sanitizeValue(childValue, depth + 1) + } + return sanitized + } finally { + exit(value) + } + } + + private fun sanitizeValue(value: Any?, depth: Int): Any? { + visit(depth) + val collection = value?.asSanitizableCollectionOrNull() + + return when { + value == null || value is String || value is Number || value is Boolean -> value + value is CharSequence || value is Char -> value.toString() + value is Enum<*> -> value.name + value is Map<*, *> -> sanitizeMap(value, depth) + collection != null -> sanitizeCollection(collection, depth) + else -> { + warningState.logUnsupportedValueWarning(value::class.simpleName, logger) + value.toString() + } + } + } + + private fun Any.asSanitizableCollectionOrNull(): Collection<*>? = + when (this) { + is Collection<*> -> this + is Array<*> -> asList() + is BooleanArray -> asList() + is ByteArray -> asList() + is ShortArray -> asList() + is IntArray -> asList() + is LongArray -> asList() + is FloatArray -> asList() + is DoubleArray -> asList() + is CharArray -> asList() + else -> null + } + + /** + * Records a visit to one value, enforcing the per-entry depth cap and the shared per-update + * value budget. Throws [DropSubtree] to abort the current subtree when either is exceeded. + */ + private fun visit(depth: Int) { + if (depth > MAX_ARGUMENT_DEPTH) { + throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false) + } + if (--remainingValues < 0) { + throw DropSubtree(BUDGET_WARNING, exhaustsBudget = true) + } + } + + private fun enter(container: Any) { + if (activeContainers.put(container, Unit) != null) { + throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false) + } + } + + private fun exit(container: Any) { + activeContainers.remove(container) + } + + /** + * Control-flow signal to abort sanitization of the current subtree. Internal to + * [ArgumentSanitizer]. + * + * [exhaustsBudget] distinguishes an entry-local drop (cycle or over-deep structure) from an + * update-wide one (the shared value budget is spent). Overrides [fillInStackTrace] to skip + * stack-trace capture. + */ + private class DropSubtree(val warning: String, val exhaustsBudget: Boolean) : + RuntimeException() { + override fun fillInStackTrace(): Throwable = this + } + + private companion object { + + /** + * Max nesting depth allowed while sanitizing a single argument value for a given back stack + * entry. + * + * If exceeded, all arguments for that back stack entry are dropped. + */ + private const val MAX_ARGUMENT_DEPTH = 20 + + /** + * Max number of argument values visited while sanitizing all entries in a given back stack + * update. + * + * 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: + * - /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 BUDGET_WARNING = + "Nav3 arguments exceeded the maximum total value count for one backstack update. " + + "Skipping arguments for this and older captured entries." + } + } + + /** A small state wrapper that lets us avoid spamming logs when sanitizing arguments. */ + internal class WarningState { + private var hasLoggedUnsupportedValueWarning = false + private var hasLoggedInvalidRouteNameWarning = false + private var hasLoggedNameExtractorFailureWarning = false + + fun logUnsupportedValueWarning(typeName: String?, logger: ILogger) { + if (hasLoggedUnsupportedValueWarning) { + 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, + ) + hasLoggedUnsupportedValueWarning = true + } + + fun logInvalidRouteNameWarning(logger: ILogger) { + if (hasLoggedInvalidRouteNameWarning) { + return + } + + logger.log( + WARNING, + "Nav3 nameExtractor returned a blank route name while processing this back stack update. " + + "Using /unknown instead.", + ) + hasLoggedInvalidRouteNameWarning = true + } + + fun logNameExtractorFailureWarning(logger: ILogger, throwable: Throwable) { + if (hasLoggedNameExtractorFailureWarning) { + return + } + + logger.log( + WARNING, + "Nav3 nameExtractor threw while resolving a route name. Using /unknown instead.", + throwable, + ) + hasLoggedNameExtractorFailureWarning = 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( + val name: String, + val arguments: Map = emptyMap(), +) { + + /** + * Returns this route in serialized form. E.g.: + * ``` + * { + * "route": "/ProductScreen" + * "args": { + * "product_id": 12345 + * "promo_id:": "spring-marketing-drive-2026" + * } + * } + * ``` + */ + fun serialize(): Map = buildMap { + put("route", name) + if (arguments.isNotEmpty()) { + put("args", arguments) + } + } +} + +internal fun List.serialize(): List> = map(Route::serialize) diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteResolversTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteResolversTest.kt new file mode 100644 index 0000000000..595fb31dde --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteResolversTest.kt @@ -0,0 +1,83 @@ +package io.sentry.compose.navigation3 + +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.snapshots.Snapshot +import com.google.common.truth.Truth.assertThat +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class RouteResolversTest { + + 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 = RouteResolvers(nameExtractor = defaultNameExtractor, argumentsExtractor = null) + + assertNull(sut.getArguments(HomeRoute())) + } + + @Test + fun `getName delegates to the configured extractor`() { + val route = ProfileRoute("123") + val sut = + RouteResolvers( + nameExtractor = RouteNameExtractor { entry -> "profile-${entry.userId}" }, + argumentsExtractor = null, + ) + + assertEquals("profile-123", sut.getName(route)) + } + + @Test + fun `getArguments delegates to the configured extractor`() { + val route = ProfileRoute("123") + val sut = + RouteResolvers( + 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 = + RouteResolvers( + nameExtractor = RouteNameExtractor { routeName.value }, + argumentsExtractor = null, + ) + + assertEquals(0, observeReads { sut.getName(HomeRoute()) }) + } + + @Test + fun `getArguments hides extractor reads from snapshot observation`() { + val argumentValue = mutableStateOf("123") + val sut = + RouteResolvers( + nameExtractor = defaultNameExtractor, + argumentsExtractor = RouteArgumentsExtractor { mapOf("userId" to argumentValue.value) }, + ) + + assertEquals(0, observeReads { sut.getArguments(HomeRoute()) }) + } + + 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 new file mode 100644 index 0000000000..a85e68ff07 --- /dev/null +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteTranslatorTest.kt @@ -0,0 +1,419 @@ +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.WarningState +import java.util.AbstractCollection +import kotlin.test.Test +import kotlin.test.assertEquals +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 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( + resolvers = { RouteResolvers(nameExtractor, argumentsExtractor) }, + logger = logger, + ) + + @Test + fun `translate preserves input order`() { + val sut = getSut() + + val routes = sut.translate(listOf(SettingsRoute("privacy"), ProfileRoute("123"), HomeRoute())) + + assertThat(routes) + .containsExactly(Route("/SettingsRoute"), Route("/ProfileRoute"), Route("/HomeRoute")) + .inOrder() + } + + @Test + fun `translate returns empty routes for an empty back stack`() { + val sut = getSut() + + assertThat(sut.translate(emptyList())).isEmpty() + } + + @Test + fun `translate preserves newer entry arguments when the shared budget overflows`() { + val newest = SettingsRoute("privacy") + val middle = ProfileRoute("123") + val oldest = HomeRoute() + 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(newest, middle, oldest)) + + assertThat(routes).hasSize(3) + assertThat(routes[0]).isEqualTo(Route("/SettingsRoute", mapOf("section" to "privacy"))) + assertThat(routes[1].name).isEqualTo("/ProfileRoute") + assertThat(routes[1].arguments).isEmpty() + assertThat(routes[2].name).isEqualTo("/HomeRoute") + assertThat(routes[2].arguments).isEmpty() + } + + @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"))) + + 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"))) + + assertThat(routes.map(Route::serialize)) + .containsExactly( + mapOf("route" to "/SettingsRoute", "args" to mapOf("section" to "privacy")), + mapOf("route" to "/ProfileRoute"), + ) + .inOrder() + } + + @Test + fun `resolveRouteName normalizes a custom name with a leading slash`() { + val sut = getSut(nameExtractor = { "profile" }) + + assertEquals("/profile", sut.resolveRouteName(ProfileRoute("123"), WarningState())) + } + + @Test + fun `resolveRouteName leaves leading slash on custom name if already present`() { + val sut = getSut(nameExtractor = { "/profile" }) + + assertEquals("/profile", sut.resolveRouteName(ProfileRoute("123"), WarningState())) + } + + @Test + fun `resolveRouteName returns the configured name extractor result`() { + val sut = getSut() + + assertEquals("/HomeRoute", sut.resolveRouteName(HomeRoute(), WarningState())) + } + + @Test + fun `resolveRouteName returns unknown when name extractor throws`() { + val sut = getSut(nameExtractor = { error("boom") }) + + assertEquals( + RouteTranslator.UNKNOWN_ROUTE_NAME, + sut.resolveRouteName(HomeRoute(), WarningState()), + ) + verify(logger) + .log( + eq(WARNING), + eq("Nav3 nameExtractor threw while resolving a route name. Using /unknown instead."), + org.mockito.kotlin.any(), + ) + } + + @Test + fun `resolveRouteName returns unknown when name extractor returns blank`() { + val sut = getSut(nameExtractor = { " " }) + + assertEquals( + RouteTranslator.UNKNOWN_ROUTE_NAME, + sut.resolveRouteName(HomeRoute(), WarningState()), + ) + verify(logger) + .log( + eq(WARNING), + eq( + "Nav3 nameExtractor returned a blank route name while processing this back stack update. " + + "Using /unknown instead." + ), + ) + } + + @Test + fun `resolveArguments 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.resolveArguments(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 `resolveArguments 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.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + .isEqualTo( + mapOf( + "nested" to + mapOf("items" to listOf("x", listOf("y", "PRIVATE"), listOf(true, false), listOf("q"))) + ) + ) + } + + @Test + fun `resolveArguments coerces unsupported values to strings`() { + class OpaqueValue { + override fun toString(): String = "opaque-value" + } + + val sut = + getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("bad" to OpaqueValue()) }) + + assertThat(sut.resolveArguments(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"))) + + 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.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState())) + clearInvocations(logger) + + sut.resolveArguments(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 `resolveArguments returns empty if no arguments extractor`() { + val sut = getSut(argumentsExtractor = null) + + assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + .isEmpty() + } + + @Test + fun `resolveArguments returns empty when arguments extractor throws`() { + val sut = getSut(argumentsExtractor = { error("boom") }) + + assertThat(sut.resolveArguments(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 `resolveArguments returns empty for cyclic structures`() { + val cyclic = mutableMapOf() + cyclic["self"] = cyclic + + val sut = + getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("cyclic" to cyclic) }) + + assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + .isEmpty() + } + + @Test + fun `resolveArguments 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.resolveArguments(ProfileRoute("123"), ArgumentSanitizer(logger, WarningState()))) + .isEmpty() + } + + @Test + fun `resolveArguments drops oversized payloads instead of truncating them`() { + val sut = + getSut( + argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("values" to List(1_001) { it }) } + ) + + assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + .isEmpty() + } + + @Test + fun `resolveArguments 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.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + .isEqualTo(mapOf("values" to listOf(1, 2))) + assertThat(values.wasSizeRead).isFalse() + } +} From 12f5e01e95b881a7f1d307183acbcbf7a4acb2cf Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 24 Sep 2026 10:37:12 +0200 Subject: [PATCH 2/5] Rename RouteResolvers -> RouteExtractors --- .../compose/navigation3/RouteExtractors.kt | 2 +- .../compose/navigation3/RouteTranslator.kt | 31 ++++----- ...esolversTest.kt => RouteExtractorsTest.kt} | 12 ++-- .../navigation3/RouteTranslatorTest.kt | 64 ++++++++++--------- 4 files changed, 53 insertions(+), 56 deletions(-) rename sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/{RouteResolversTest.kt => RouteExtractorsTest.kt} (89%) 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 index deac06f2ea..92dd05407f 100644 --- 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 @@ -122,7 +122,7 @@ internal fun interface RouteArgumentsExtractor { * Extractor invocations are hidden from Compose snapshot observation so they don't impact * invalidation of the recompose scope that reads them. */ -internal class RouteResolvers( +internal class RouteExtractors( val nameExtractor: RouteNameExtractor, val argumentsExtractor: RouteArgumentsExtractor?, ) { 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/RouteTranslator.kt index 32845bec8e..6faa865407 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/RouteTranslator.kt @@ -8,7 +8,7 @@ import org.jetbrains.annotations.TestOnly /** Translates app-defined back stack entries into input-ordered [Route]s. */ internal class RouteTranslator( - private val resolvers: () -> RouteResolvers, + private val extractors: () -> RouteExtractors, private val logger: ILogger, ) { @@ -26,22 +26,17 @@ internal class RouteTranslator( val warningState = WarningState() val sanitizer = ArgumentSanitizer(logger, warningState) - return backStackEntries.map { entry -> resolveRoute(entry, warningState, sanitizer) } + return backStackEntries.map { entry -> + Route( + name = extractRouteName(entry, warningState), + arguments = extractRouteArguments(entry, sanitizer), + ) + } } - private fun resolveRoute( - backStackEntry: T, - warningState: WarningState, - sanitizer: ArgumentSanitizer, - ): Route = - Route( - name = resolveRouteName(backStackEntry, warningState), - arguments = resolveArguments(backStackEntry, sanitizer), - ) - /** * Returns a route name for the provided [backStackEntry], based on this translator's - * [name extractor][RouteResolvers.nameExtractor]. + * [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 @@ -49,10 +44,10 @@ internal class RouteTranslator( */ @TestOnly @Suppress("TooGenericExceptionCaught") - fun resolveRouteName(backStackEntry: T, warningState: WarningState): String { + fun extractRouteName(backStackEntry: T, warningState: WarningState): String { val name: String? = try { - resolvers.invoke().getName(backStackEntry) + extractors.invoke().getName(backStackEntry) } catch (t: Throwable) { // Route name extractors are host app callbacks. ExceptionUtils.rethrowIfFatal(t) @@ -71,20 +66,20 @@ internal class RouteTranslator( /** * Returns the arguments for the provided [backStackEntry], based on this translator's - * [arguments extractor][RouteResolvers.argumentsExtractor]. + * [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 resolveArguments( + fun extractRouteArguments( backStackEntry: T, sanitizer: ArgumentSanitizer, ): Map { val raw = try { - resolvers.invoke().getArguments(backStackEntry) ?: return emptyMap() + extractors.invoke().getArguments(backStackEntry) ?: return emptyMap() } catch (t: Throwable) { // Route argument extractors are host app callbacks. ExceptionUtils.rethrowIfFatal(t) diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteResolversTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt similarity index 89% rename from sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteResolversTest.kt rename to sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt index 595fb31dde..05e7855642 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteResolversTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/RouteExtractorsTest.kt @@ -7,7 +7,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNull -class RouteResolversTest { +class RouteExtractorsTest { private data class HomeRoute(val id: String = "home") @@ -17,7 +17,7 @@ class RouteResolversTest { @Test fun `getArguments returns null when no arguments extractor is configured`() { - val sut = RouteResolvers(nameExtractor = defaultNameExtractor, argumentsExtractor = null) + val sut = RouteExtractors(nameExtractor = defaultNameExtractor, argumentsExtractor = null) assertNull(sut.getArguments(HomeRoute())) } @@ -26,7 +26,7 @@ class RouteResolversTest { fun `getName delegates to the configured extractor`() { val route = ProfileRoute("123") val sut = - RouteResolvers( + RouteExtractors( nameExtractor = RouteNameExtractor { entry -> "profile-${entry.userId}" }, argumentsExtractor = null, ) @@ -38,7 +38,7 @@ class RouteResolversTest { fun `getArguments delegates to the configured extractor`() { val route = ProfileRoute("123") val sut = - RouteResolvers( + RouteExtractors( nameExtractor = RouteNameExtractor { entry -> entry.userId }, argumentsExtractor = RouteArgumentsExtractor { entry -> mapOf("userId" to entry.userId) }, ) @@ -50,7 +50,7 @@ class RouteResolversTest { fun `getName hides extractor reads from snapshot observation`() { val routeName = mutableStateOf("home") val sut = - RouteResolvers( + RouteExtractors( nameExtractor = RouteNameExtractor { routeName.value }, argumentsExtractor = null, ) @@ -62,7 +62,7 @@ class RouteResolversTest { fun `getArguments hides extractor reads from snapshot observation`() { val argumentValue = mutableStateOf("123") val sut = - RouteResolvers( + RouteExtractors( nameExtractor = defaultNameExtractor, argumentsExtractor = RouteArgumentsExtractor { mapOf("userId" to argumentValue.value) }, ) 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 index a85e68ff07..29be14c405 100644 --- 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 @@ -36,7 +36,7 @@ class RouteTranslatorTest { argumentsExtractor: RouteArgumentsExtractor? = null, ): RouteTranslator = RouteTranslator( - resolvers = { RouteResolvers(nameExtractor, argumentsExtractor) }, + extractors = { RouteExtractors(nameExtractor, argumentsExtractor) }, logger = logger, ) @@ -136,33 +136,33 @@ class RouteTranslatorTest { } @Test - fun `resolveRouteName normalizes a custom name with a leading slash`() { + fun `extractRouteName normalizes a custom name with a leading slash`() { val sut = getSut(nameExtractor = { "profile" }) - assertEquals("/profile", sut.resolveRouteName(ProfileRoute("123"), WarningState())) + assertEquals("/profile", sut.extractRouteName(ProfileRoute("123"), WarningState())) } @Test - fun `resolveRouteName leaves leading slash on custom name if already present`() { + fun `extractRouteName leaves leading slash on custom name if already present`() { val sut = getSut(nameExtractor = { "/profile" }) - assertEquals("/profile", sut.resolveRouteName(ProfileRoute("123"), WarningState())) + assertEquals("/profile", sut.extractRouteName(ProfileRoute("123"), WarningState())) } @Test - fun `resolveRouteName returns the configured name extractor result`() { + fun `extractRouteName returns the configured name extractor result`() { val sut = getSut() - assertEquals("/HomeRoute", sut.resolveRouteName(HomeRoute(), WarningState())) + assertEquals("/HomeRoute", sut.extractRouteName(HomeRoute(), WarningState())) } @Test - fun `resolveRouteName returns unknown when name extractor throws`() { + fun `extractRouteName returns unknown when name extractor throws`() { val sut = getSut(nameExtractor = { error("boom") }) assertEquals( RouteTranslator.UNKNOWN_ROUTE_NAME, - sut.resolveRouteName(HomeRoute(), WarningState()), + sut.extractRouteName(HomeRoute(), WarningState()), ) verify(logger) .log( @@ -173,12 +173,12 @@ class RouteTranslatorTest { } @Test - fun `resolveRouteName returns unknown when name extractor returns blank`() { + fun `extractRouteName returns unknown when name extractor returns blank`() { val sut = getSut(nameExtractor = { " " }) assertEquals( RouteTranslator.UNKNOWN_ROUTE_NAME, - sut.resolveRouteName(HomeRoute(), WarningState()), + sut.extractRouteName(HomeRoute(), WarningState()), ) verify(logger) .log( @@ -191,7 +191,7 @@ class RouteTranslatorTest { } @Test - fun `resolveArguments returns supported values in serializable form`() { + fun `extractRouteArguments returns supported values in serializable form`() { val sut = getSut( argumentsExtractor = @@ -215,7 +215,7 @@ class RouteTranslatorTest { } ) - assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) .isEqualTo( mapOf( "str" to "hello", @@ -236,7 +236,7 @@ class RouteTranslatorTest { } @Test - fun `resolveArguments sanitizes nested supported containers recursively`() { + fun `extractRouteArguments sanitizes nested supported containers recursively`() { val sut = getSut( argumentsExtractor = @@ -256,7 +256,7 @@ class RouteTranslatorTest { } ) - assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) .isEqualTo( mapOf( "nested" to @@ -266,7 +266,7 @@ class RouteTranslatorTest { } @Test - fun `resolveArguments coerces unsupported values to strings`() { + fun `extractRouteArguments coerces unsupported values to strings`() { class OpaqueValue { override fun toString(): String = "opaque-value" } @@ -274,7 +274,7 @@ class RouteTranslatorTest { val sut = getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("bad" to OpaqueValue()) }) - assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) .isEqualTo(mapOf("bad" to "opaque-value")) } @@ -320,10 +320,10 @@ class RouteTranslatorTest { val sut = getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("bad" to OpaqueValue()) }) - sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState())) + sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState())) clearInvocations(logger) - sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState())) + sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState())) verify(logger, times(1)) .log( @@ -339,18 +339,18 @@ class RouteTranslatorTest { } @Test - fun `resolveArguments returns empty if no arguments extractor`() { + fun `extractRouteArguments returns empty if no arguments extractor`() { val sut = getSut(argumentsExtractor = null) - assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) .isEmpty() } @Test - fun `resolveArguments returns empty when arguments extractor throws`() { + fun `extractRouteArguments returns empty when arguments extractor throws`() { val sut = getSut(argumentsExtractor = { error("boom") }) - assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) .isEmpty() verify(logger) .log( @@ -361,42 +361,44 @@ class RouteTranslatorTest { } @Test - fun `resolveArguments returns empty for cyclic structures`() { + fun `extractRouteArguments returns empty for cyclic structures`() { val cyclic = mutableMapOf() cyclic["self"] = cyclic val sut = getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("cyclic" to cyclic) }) - assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) .isEmpty() } @Test - fun `resolveArguments returns empty for deeply nested structures`() { + 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.resolveArguments(ProfileRoute("123"), ArgumentSanitizer(logger, WarningState()))) + assertThat( + sut.extractRouteArguments(ProfileRoute("123"), ArgumentSanitizer(logger, WarningState())) + ) .isEmpty() } @Test - fun `resolveArguments drops oversized payloads instead of truncating them`() { + fun `extractRouteArguments drops oversized payloads instead of truncating them`() { val sut = getSut( argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("values" to List(1_001) { it }) } ) - assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) .isEmpty() } @Test - fun `resolveArguments does not use caller collection size for allocation`() { + fun `extractRouteArguments does not use caller collection size for allocation`() { val values = object : AbstractCollection() { var wasSizeRead = false @@ -412,7 +414,7 @@ class RouteTranslatorTest { val sut = getSut(argumentsExtractor = RouteArgumentsExtractor { _ -> mapOf("values" to values) }) - assertThat(sut.resolveArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) + assertThat(sut.extractRouteArguments(HomeRoute(), ArgumentSanitizer(logger, WarningState()))) .isEqualTo(mapOf("values" to listOf(1, 2))) assertThat(values.wasSizeRead).isFalse() } From 7c321ff29185cb8d6673d9c8b866974045407d66 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 24 Sep 2026 10:49:16 +0200 Subject: [PATCH 3/5] test(android-nav3): Use Truth in route tests Convert the new Navigation 3 route test assertions from kotlin.test to Google Truth to match the repository's preferred unit-test assertion style. Co-Authored-By: OpenAI GPT-5.4 --- .../navigation3/RouteExtractorsTest.kt | 10 ++++------ .../navigation3/RouteTranslatorTest.kt | 19 +++++++------------ 2 files changed, 11 insertions(+), 18 deletions(-) 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 index 05e7855642..f97e5241ac 100644 --- 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 @@ -4,8 +4,6 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.snapshots.Snapshot import com.google.common.truth.Truth.assertThat import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertNull class RouteExtractorsTest { @@ -19,7 +17,7 @@ class RouteExtractorsTest { fun `getArguments returns null when no arguments extractor is configured`() { val sut = RouteExtractors(nameExtractor = defaultNameExtractor, argumentsExtractor = null) - assertNull(sut.getArguments(HomeRoute())) + assertThat(sut.getArguments(HomeRoute())).isNull() } @Test @@ -31,7 +29,7 @@ class RouteExtractorsTest { argumentsExtractor = null, ) - assertEquals("profile-123", sut.getName(route)) + assertThat(sut.getName(route)).isEqualTo("profile-123") } @Test @@ -55,7 +53,7 @@ class RouteExtractorsTest { argumentsExtractor = null, ) - assertEquals(0, observeReads { sut.getName(HomeRoute()) }) + assertThat(observeReads { sut.getName(HomeRoute()) }).isEqualTo(0) } @Test @@ -67,7 +65,7 @@ class RouteExtractorsTest { argumentsExtractor = RouteArgumentsExtractor { mapOf("userId" to argumentValue.value) }, ) - assertEquals(0, observeReads { sut.getArguments(HomeRoute()) }) + assertThat(observeReads { sut.getArguments(HomeRoute()) }).isEqualTo(0) } private fun observeReads(block: () -> Unit): Int { 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 index 29be14c405..f8daf3412b 100644 --- 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 @@ -7,7 +7,6 @@ import io.sentry.compose.navigation3.RouteTranslator.ArgumentSanitizer import io.sentry.compose.navigation3.RouteTranslator.WarningState import java.util.AbstractCollection import kotlin.test.Test -import kotlin.test.assertEquals import org.mockito.kotlin.clearInvocations import org.mockito.kotlin.eq import org.mockito.kotlin.mock @@ -139,31 +138,29 @@ class RouteTranslatorTest { fun `extractRouteName normalizes a custom name with a leading slash`() { val sut = getSut(nameExtractor = { "profile" }) - assertEquals("/profile", sut.extractRouteName(ProfileRoute("123"), WarningState())) + 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" }) - assertEquals("/profile", sut.extractRouteName(ProfileRoute("123"), WarningState())) + assertThat(sut.extractRouteName(ProfileRoute("123"), WarningState())).isEqualTo("/profile") } @Test fun `extractRouteName returns the configured name extractor result`() { val sut = getSut() - assertEquals("/HomeRoute", sut.extractRouteName(HomeRoute(), WarningState())) + assertThat(sut.extractRouteName(HomeRoute(), WarningState())).isEqualTo("/HomeRoute") } @Test fun `extractRouteName returns unknown when name extractor throws`() { val sut = getSut(nameExtractor = { error("boom") }) - assertEquals( - RouteTranslator.UNKNOWN_ROUTE_NAME, - sut.extractRouteName(HomeRoute(), WarningState()), - ) + assertThat(sut.extractRouteName(HomeRoute(), WarningState())) + .isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) verify(logger) .log( eq(WARNING), @@ -176,10 +173,8 @@ class RouteTranslatorTest { fun `extractRouteName returns unknown when name extractor returns blank`() { val sut = getSut(nameExtractor = { " " }) - assertEquals( - RouteTranslator.UNKNOWN_ROUTE_NAME, - sut.extractRouteName(HomeRoute(), WarningState()), - ) + assertThat(sut.extractRouteName(HomeRoute(), WarningState())) + .isEqualTo(RouteTranslator.UNKNOWN_ROUTE_NAME) verify(logger) .log( eq(WARNING), From 16726da8a9c9ecff11a0b8df4c44f383779c01c8 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 24 Sep 2026 10:53:42 +0200 Subject: [PATCH 4/5] feat(android-nav3): Add explicit retention policy Add a RetentionPolicy enum to RouteTranslator.translate so callers can declare whether route info for the lower or higher values in the provided list should be retained when an extraction budget is reached. dThis removes the implicit newest-first coupling while preserving the existing shared argument budget behavior for the top of the stack. --- .../compose/navigation3/RouteTranslator.kt | 51 ++++++++-- .../navigation3/RouteTranslatorTest.kt | 95 ++++++++++++++++--- 2 files changed, 122 insertions(+), 24 deletions(-) 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/RouteTranslator.kt index 6faa865407..9cc7f26418 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/RouteTranslator.kt @@ -6,7 +6,15 @@ 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. */ +/** + * Translates app-defined back stack entries into input-ordered [Route]s. + * + * **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. + */ internal class RouteTranslator( private val extractors: () -> RouteExtractors, private val logger: ILogger, @@ -16,22 +24,25 @@ internal class RouteTranslator( internal const val UNKNOWN_ROUTE_NAME = "/unknown" } - /** - * Translates the provided [backStackEntries] into [Route]s and returns them in input order. - * - * Callers should provide entries newest first so the shared argument budget preserves data for - * the destinations most relevant to the current navigation state. - */ - fun translate(backStackEntries: List): List { + /** Translates the provided [backStackEntries] into [Route]s and returns them in input order. */ + fun translate(backStackEntries: List, policy: RetentionPolicy): List { val warningState = WarningState() val sanitizer = ArgumentSanitizer(logger, warningState) - return backStackEntries.map { entry -> + val entriesInPolicyOrder = + when (policy) { + RetentionPolicy.KEEP_FIRST -> backStackEntries + RetentionPolicy.KEEP_LAST -> backStackEntries.asReversed() + } + + val translatedByEntry = entriesInPolicyOrder.associateWith { entry -> Route( name = extractRouteName(entry, warningState), arguments = extractRouteArguments(entry, sanitizer), ) } + + return backStackEntries.map { entry -> translatedByEntry.getValue(entry) } } /** @@ -94,6 +105,28 @@ internal class RouteTranslator( return sanitizer.sanitizeEntry(raw) } + /** + * Specifies whether route 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. + */ + internal enum class RetentionPolicy { + + /** + * Retains route 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 + * budget is reached. Retention starts at lastIndex and decrements until the budget is + * exhausted. + */ + KEEP_LAST, + } + /** * Sanitizes a back stack update's argument maps into a serializable form. It bounds depth and * total value count, and it rejects cyclic structures. 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 index f8daf3412b..55836d9e62 100644 --- 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 @@ -4,6 +4,7 @@ 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 kotlin.test.Test @@ -43,25 +44,80 @@ class RouteTranslatorTest { fun `translate preserves input order`() { val sut = getSut() - val routes = sut.translate(listOf(SettingsRoute("privacy"), ProfileRoute("123"), HomeRoute())) + 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())).isEmpty() + 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 preserves newer entry arguments when the shared budget overflows`() { - val newest = SettingsRoute("privacy") + fun `translate with KEEP_FIRST preserves arguments nearest index zero when budget is exceeded`() { + val first = SettingsRoute("privacy") val middle = ProfileRoute("123") - val oldest = HomeRoute() + 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 = @@ -75,14 +131,15 @@ class RouteTranslatorTest { } ) - val routes = sut.translate(listOf(newest, middle, oldest)) + val routes = sut.translate(listOf(first, middle, last), RetentionPolicy.KEEP_LAST) - assertThat(routes).hasSize(3) - assertThat(routes[0]).isEqualTo(Route("/SettingsRoute", mapOf("section" to "privacy"))) - assertThat(routes[1].name).isEqualTo("/ProfileRoute") - assertThat(routes[1].arguments).isEmpty() - assertThat(routes[2].name).isEqualTo("/HomeRoute") - assertThat(routes[2].arguments).isEmpty() + assertThat(routes) + .containsExactly( + Route("/HomeRoute"), + Route("/ProfileRoute"), + Route("/SettingsRoute", mapOf("section" to "privacy")), + ) + .inOrder() } @Test @@ -99,7 +156,11 @@ class RouteTranslatorTest { } ) - val routes = sut.translate(listOf(SettingsRoute("privacy"), ProfileRoute("123"))) + val routes = + sut.translate( + listOf(SettingsRoute("privacy"), ProfileRoute("123")), + RetentionPolicy.KEEP_FIRST, + ) assertThat(routes) .containsExactly( @@ -124,7 +185,11 @@ class RouteTranslatorTest { } ) - val routes = sut.translate(listOf(SettingsRoute("privacy"), ProfileRoute("123"))) + val routes = + sut.translate( + listOf(SettingsRoute("privacy"), ProfileRoute("123")), + RetentionPolicy.KEEP_FIRST, + ) assertThat(routes.map(Route::serialize)) .containsExactly( @@ -291,7 +356,7 @@ class RouteTranslatorTest { } ) - sut.translate(listOf(HomeRoute(), ProfileRoute("123"))) + sut.translate(listOf(HomeRoute(), ProfileRoute("123")), RetentionPolicy.KEEP_FIRST) verify(logger, times(1)) .log( From 3cead789503862778790a327067a8f93a3f06a8a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 24 Sep 2026 15:38:21 +0200 Subject: [PATCH 5/5] Avoid associateWith b/c it can conflate keys --- .../compose/navigation3/RouteTranslator.kt | 21 ++++--- .../navigation3/RouteTranslatorTest.kt | 58 +++++++++++++++++++ 2 files changed, 70 insertions(+), 9 deletions(-) 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/RouteTranslator.kt index 9cc7f26418..d687a7d890 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/RouteTranslator.kt @@ -29,20 +29,23 @@ internal class RouteTranslator( val warningState = WarningState() val sanitizer = ArgumentSanitizer(logger, warningState) - val entriesInPolicyOrder = + val routes = MutableList(backStackEntries.size) { null } + val indicesInPolicyOrder = when (policy) { - RetentionPolicy.KEEP_FIRST -> backStackEntries - RetentionPolicy.KEEP_LAST -> backStackEntries.asReversed() + RetentionPolicy.KEEP_FIRST -> backStackEntries.indices + RetentionPolicy.KEEP_LAST -> backStackEntries.indices.reversed() } - val translatedByEntry = entriesInPolicyOrder.associateWith { entry -> - Route( - name = extractRouteName(entry, warningState), - arguments = extractRouteArguments(entry, sanitizer), - ) + for (index in indicesInPolicyOrder) { + val entry = backStackEntries[index] + routes[index] = + Route( + name = extractRouteName(entry, warningState), + arguments = extractRouteArguments(entry, sanitizer), + ) } - return backStackEntries.map { entry -> translatedByEntry.getValue(entry) } + return routes.requireNoNulls() } /** 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 index 55836d9e62..ca2f8db318 100644 --- 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 @@ -20,6 +20,8 @@ class RouteTranslatorTest { 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 { @@ -142,6 +144,62 @@ class RouteTranslatorTest { .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 =