Reuse shared Polyline cache for focused shapes; drop decoded double-cache (#1908) (#1919)

A focused trip's shape was cached twice under the same shapeId with divergent
policies: TripObservationRepository's ShapeCache (Polyline, LRU 100, no TTL —
GTFS shapes are immutable) and FocusedTripRepository.shapeCache (decoded
List<GeoPoint>, LRU 32, with an unexplained 10-minute TTL on immutable data).

ensureShape() already returns the shared, deduped Polyline (coalescing the
network fetch via its own SingleFlight), so the decoded cache was a redundant
second copy (~2x memory, a second eviction policy, and the stray TTL). Drop it
and decode on demand from the shared Polyline cache; the Location -> GeoPoint
map is cheap and runs on a focus change, not the per-frame path. The fetch
concurrency throttle (shapePermits) and same-shape decode coalescing
(shapeFetches) are unchanged.

The cross-call shape memo now lives entirely in TripObservationRepository
(covered by TripObservationRepositoryTest); reframe the FocusedTripRepository
test that asserted its own memo to document the delegation.

Part of #1908 (finding 8, item 2).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: bmander

onebusaway-android/src/main/java/org/onebusaway/android/map/FocusedTripRepository.kt

@@ -120,8 +120,6 @@ class DefaultFocusedTripRepository internal constructor(
     private val shapePermits = Semaphore(MAX_CONCURRENT_SHAPE_FETCHES)
     private val routeStopCache =
         ExpiringLruCache<String, List<RouteStopGroup>>(cacheSize, cacheTtl, now)
-    private val shapeCache =
-        ExpiringLruCache<String, List<GeoPoint>>(cacheSize, cacheTtl, now)
 
     override suspend fun getGeometry(trips: Set<FocusedTrip>): FocusedTripGeometry = coroutineScope {
         val tripsWithShapes = ArrayList<FocusedTrip>()
@@ -203,15 +201,20 @@ class DefaultFocusedTripRepository internal constructor(
         null
     }
 
-    /** Shape-id cache avoids refetching one pattern merely because a later arrival has a new trip id. */
+    /**
+     * Decoded points for a shape. Retention and fetch dedup are owned by the shared [Polyline] cache
+     * inside [TripObservationRepository.ensureShape] (keyed by shapeId, no TTL — GTFS shapes are
+     * immutable), so this holds no shape cache of its own: it coalesces concurrent decodes for one
+     * shape and re-decodes the (already-cached) polyline per focus. The map is cheap and runs on a
+     * focus change, not the per-frame path; the permit caps concurrent shape fetches.
+     */
     private suspend fun fetchShape(tripId: String, shapeId: String): List<GeoPoint>? =
-        shapeCache.get(shapeId) ?: shapeFetches.run(shapeId) {
-            shapeCache.get(shapeId) ?: shapePermits.withPermit {
+        shapeFetches.run(shapeId) {
+            shapePermits.withPermit {
                 resolveOrNull("shape $shapeId") { observations.ensureShape(tripId, shapeId) }
                     ?.let { polyline ->
                         withContext(Dispatchers.Default) { polyline.points.map(Location::toGeoPoint) }
                     }
-                    ?.also { shapeCache.put(shapeId, it) }
             }
         }
 }

onebusaway-android/src/test/java/org/onebusaway/android/map/FocusedTripRepositoryTest.kt

@@ -129,7 +129,11 @@ class FocusedTripRepositoryTest {
     }
 
     @Test
-    fun `shape cache is keyed by shape rather than trip`() = runTest {
+    fun `repeat focus re-delegates shape decode to the shared polyline cache`() = runTest {
+        // FocusedTripRepository holds no shape cache of its own: retention and fetch dedup live in
+        // the shared Polyline cache inside TripObservationRepository.ensureShape (keyed by shapeId,
+        // no TTL). So a later focus on the same shape re-queries ensureShape — cheap in production,
+        // where the polyline is already cached there — rather than memoizing decoded points here.
         val observations = Observations().apply { shapes["shared"] = Polyline(emptyList()) }
         val repository = DefaultFocusedTripRepository(
             observations, RouteStops(), backgroundScope, now = { ElapsedTime(0L) }
@@ -138,7 +142,10 @@ class FocusedTripRepositoryTest {
         repository.getGeometry(setOf(FocusedTrip("older-trip", "route", "shared", null)))
         repository.getGeometry(setOf(FocusedTrip("newer-trip", "route", "shared", null)))
 
-        assertEquals(listOf("older-trip" to "shared"), observations.shapeRequests)
+        assertEquals(
+            listOf("older-trip" to "shared", "newer-trip" to "shared"),
+            observations.shapeRequests,
+        )
     }
 
     private fun stop(id: String) = ObaStopElement(id = id, lat = 47.0, lon = -122.0)