[NN-383] - Update only distance value in existing upcoming road objects on each status (#6922)

Author: Łukasz Paczos

changelog/unreleased/bugfixes/6922.md

@@ -0,0 +1 @@
+Fixed "global reference table overflow" in case an application accumulates and keeps links to `RouteProgress#upcomingRoadObjects`

libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/factory/RoadObjectFactory.kt

@@ -6,6 +6,7 @@ import com.mapbox.navigation.base.trip.model.roadobject.UpcomingRoadObject
 import com.mapbox.navigation.base.trip.model.roadobject.distanceinfo.RoadObjectDistanceInfo
 import com.mapbox.navigation.base.trip.model.roadobject.mapToRoadObject
 import com.mapbox.navigator.RoadObjectType
+import com.mapbox.navigator.UpcomingRouteAlert
 
 /**
  * Internal factory to build road objects
@@ -37,6 +38,27 @@ object RoadObjectFactory {
             }
     }
 
+    fun List<UpcomingRoadObject>.getUpdatedObjectsAhead(
+        upcomingRouteAlerts: List<UpcomingRouteAlert>
+    ): List<UpcomingRoadObject> {
+        val idToDistanceRemaining = upcomingRouteAlerts.associate {
+            it.roadObject.id to it.distanceToStart
+        }
+        val updateObjects = mutableListOf<UpcomingRoadObject>()
+        forEach {
+            if (it.roadObject.id in idToDistanceRemaining.keys) {
+                updateObjects.add(
+                    buildUpcomingRoadObject(
+                        roadObject = it.roadObject, // reusing the old road object reference
+                        distanceToStart = idToDistanceRemaining[it.roadObject.id],
+                        distanceInfo = null
+                    )
+                )
+            }
+        }
+        return updateObjects
+    }
+
     /**
      * Build road object from native object
      */

libnavigation-base/src/test/java/com/mapbox/navigation/base/internal/factory/RoadObjectFactoryTest.kt

@@ -2,6 +2,7 @@ package com.mapbox.navigation.base.internal.factory
 
 import com.mapbox.geojson.Geometry
 import com.mapbox.geojson.Point
+import com.mapbox.navigation.base.internal.factory.RoadObjectFactory.getUpdatedObjectsAhead
 import com.mapbox.navigation.base.internal.factory.RoadObjectFactory.toUpcomingRoadObjects
 import com.mapbox.navigation.base.trip.model.roadobject.RoadObjectType
 import com.mapbox.navigation.base.trip.model.roadobject.SDKAmenity
@@ -37,7 +38,7 @@ import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
-import java.util.*
+import java.util.Date
 
 private typealias SDKRouteAlertLocation =
     com.mapbox.navigation.base.trip.model.roadobject.location.RouteAlertLocation
@@ -321,6 +322,54 @@ class RoadObjectFactoryTest {
         }
     }
 
+    @Test
+    fun `create upcoming route objects reusing existing`() {
+        val firstUpdate: List<UpcomingRouteAlert> = listOf(
+            UpcomingRouteAlert(
+                createRoadObject(
+                    type = com.mapbox.navigator.RoadObjectType.RESTRICTED_AREA,
+                    location = matchedRoadObjectLocation(location.shape),
+                    id = "0"
+                ),
+                5.0
+            ),
+            UpcomingRouteAlert(
+                createRoadObject(
+                    type = com.mapbox.navigator.RoadObjectType.RESTRICTED_AREA,
+                    location = matchedRoadObjectLocation(location.shape),
+                    id = "1"
+                ),
+                9.0
+            ),
+        )
+        val secondUpdate: List<UpcomingRouteAlert> = listOf(
+            UpcomingRouteAlert(
+                createRoadObject(
+                    type = com.mapbox.navigator.RoadObjectType.RESTRICTED_AREA,
+                    location = matchedRoadObjectLocation(location.shape),
+                    id = "0"
+                ),
+                4.0
+            ),
+            UpcomingRouteAlert(
+                createRoadObject(
+                    type = com.mapbox.navigator.RoadObjectType.RESTRICTED_AREA,
+                    location = matchedRoadObjectLocation(location.shape),
+                    id = "1"
+                ),
+                8.0
+            ),
+        )
+        val existingUpcomingRoadObjects = firstUpdate.toUpcomingRoadObjects()
+
+        val roadObjects = existingUpcomingRoadObjects.getUpdatedObjectsAhead(secondUpdate)
+
+        assertEquals(4.0, roadObjects[0].distanceToStart)
+        assertEquals(8.0, roadObjects[1].distanceToStart)
+        assertTrue(roadObjects[0].roadObject === existingUpcomingRoadObjects[0].roadObject)
+        assertTrue(roadObjects[1].roadObject === existingUpcomingRoadObjects[1].roadObject)
+    }
+
     private fun matchedRoadObjectLocation(geometry: Geometry): MatchedRoadObjectLocation {
         return MatchedRoadObjectLocation.valueOf(object : RouteAlertLocation(1) {
             override fun getShape(): Geometry = geometry
@@ -435,6 +484,7 @@ class RoadObjectFactoryTest {
         tollCollectionInfo: com.mapbox.navigator.TollCollectionInfo? = null,
         serviceAreaInfo: com.mapbox.navigator.ServiceAreaInfo? = null,
         railwayCrossingInfo: com.mapbox.navigator.RailwayCrossingInfo? = null,
+        id: String = ID
     ): RoadObject {
         val metadata = when (type) {
             com.mapbox.navigator.RoadObjectType.INCIDENT ->
@@ -453,7 +503,7 @@ class RoadObjectFactoryTest {
         }
 
         return RoadObject(
-            ID,
+            id,
             LENGTH,
             location,
             type,

libnavigation-core/src/main/java/com/mapbox/navigation/core/navigator/NavigatorMapper.kt

@@ -9,7 +9,6 @@ import com.mapbox.api.directions.v5.models.RouteLeg
 import com.mapbox.api.directions.v5.models.VoiceInstructions
 import com.mapbox.geojson.Point
 import com.mapbox.navigation.base.ExperimentalMapboxNavigationAPI
-import com.mapbox.navigation.base.internal.factory.RoadObjectFactory.toUpcomingRoadObjects
 import com.mapbox.navigation.base.internal.factory.RouteLegProgressFactory.buildRouteLegProgressObject
 import com.mapbox.navigation.base.internal.factory.RouteProgressFactory.buildRouteProgressObject
 import com.mapbox.navigation.base.internal.factory.RouteStepProgressFactory.buildRouteStepProgressObject
@@ -18,6 +17,7 @@ import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.base.speed.model.SpeedLimit
 import com.mapbox.navigation.base.trip.model.RouteProgress
 import com.mapbox.navigation.base.trip.model.RouteProgressState
+import com.mapbox.navigation.base.trip.model.roadobject.UpcomingRoadObject
 import com.mapbox.navigation.base.utils.DecodeUtils.stepGeometryToPoints
 import com.mapbox.navigation.core.trip.session.LocationMatcherResult
 import com.mapbox.navigation.navigator.internal.TripStatus
@@ -44,13 +44,15 @@ internal fun getRouteProgressFrom(
     bannerInstructions: BannerInstructions?,
     instructionIndex: Int?,
     lastVoiceInstruction: VoiceInstructions?,
+    upcomingRoadObjects: List<UpcomingRoadObject>,
 ): RouteProgress? {
     return status.getRouteProgress(
         route,
         remainingWaypoints,
         bannerInstructions,
         instructionIndex,
         lastVoiceInstruction,
+        upcomingRoadObjects,
     )
 }
 
@@ -69,6 +71,7 @@ private fun NavigationStatus.getRouteProgress(
     bannerInstructions: BannerInstructions?,
     instructionIndex: Int?,
     lastVoiceInstruction: VoiceInstructions?,
+    upcomingRoadObjects: List<UpcomingRoadObject>,
 ): RouteProgress? {
     if (routeState == RouteState.INVALID) {
         return null
@@ -188,7 +191,7 @@ private fun NavigationStatus.getRouteProgress(
             routeProgressDurationRemaining,
             routeProgressFractionTraveled,
             remainingWaypoints,
-            upcomingRouteAlerts.toUpcomingRoadObjects(),
+            upcomingRoadObjects,
             stale,
             locatedAlternativeRouteId,
             geometryIndex,

libnavigation-core/src/main/java/com/mapbox/navigation/core/trip/session/MapboxTripSession.kt

@@ -8,6 +8,7 @@ import com.mapbox.api.directions.v5.models.VoiceInstructions
 import com.mapbox.bindgen.Expected
 import com.mapbox.navigation.base.ExperimentalMapboxNavigationAPI
 import com.mapbox.navigation.base.internal.factory.RoadFactory
+import com.mapbox.navigation.base.internal.factory.RoadObjectFactory.getUpdatedObjectsAhead
 import com.mapbox.navigation.base.internal.factory.TripNotificationStateFactory.buildTripNotificationState
 import com.mapbox.navigation.base.internal.route.refreshNativePeer
 import com.mapbox.navigation.base.route.NavigationRoute
@@ -378,13 +379,17 @@ internal class MapboxTripSession(
             }
             val remainingWaypoints = tripStatus.calculateRemainingWaypoints()
             val latestBannerInstructionsWrapper = bannerInstructionEvent.latestInstructionWrapper
+            val upcomingRoadObjects = roadObjects.getUpdatedObjectsAhead(
+                tripStatus.navigationStatus.upcomingRouteAlerts
+            )
             val routeProgress = getRouteProgressFrom(
                 tripStatus.route,
                 tripStatus.navigationStatus,
                 remainingWaypoints,
                 latestBannerInstructionsWrapper?.latestBannerInstructions,
                 latestBannerInstructionsWrapper?.latestInstructionIndex,
-                lastVoiceInstruction
+                lastVoiceInstruction,
+                upcomingRoadObjects,
             ).also {
                 if (it == null) {
                     logD(

libnavigation-core/src/test/java/com/mapbox/navigation/core/navigator/NavigatorMapperTest.kt

@@ -11,6 +11,7 @@ import com.mapbox.navigation.base.internal.factory.RouteStepProgressFactory.buil
 import com.mapbox.navigation.base.road.model.Road
 import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.base.speed.model.SpeedLimit
+import com.mapbox.navigation.base.trip.model.roadobject.UpcomingRoadObject
 import com.mapbox.navigation.core.trip.session.LocationMatcherResult
 import com.mapbox.navigation.navigator.internal.TripStatus
 import com.mapbox.navigation.testing.FileUtils
@@ -54,6 +55,7 @@ class NavigatorMapperTest {
     @Test
     fun `route progress result sanity`() {
         val bannerInstructions = navigationStatus.getCurrentBannerInstructions(route)
+        val upcomingRoadObjects = listOf<UpcomingRoadObject>(mockk())
         val expected = RouteProgressFactory.buildRouteProgressObject(
             route = route,
             bannerInstructions = bannerInstructions,
@@ -94,7 +96,7 @@ class NavigatorMapperTest {
             inTunnel = true,
             stale = true,
             remainingWaypoints = 1,
-            upcomingRoadObjects = listOf(),
+            upcomingRoadObjects = upcomingRoadObjects,
             alternativeRouteId = "alternative_id",
             currentRouteGeometryIndex = routeGeometryIndex
         )
@@ -106,6 +108,7 @@ class NavigatorMapperTest {
             bannerInstructions,
             instructionIndex = 1,
             lastVoiceInstruction = null,
+            upcomingRoadObjects,
         )
 
         assertEquals(expected, result)
@@ -443,6 +446,7 @@ class NavigatorMapperTest {
             mockk(relaxed = true),
             0,
             mockk(relaxed = true),
+            emptyList(),
         )
 
         assertNull(routeProgress)
@@ -459,6 +463,7 @@ class NavigatorMapperTest {
             mockk(relaxed = true),
             0,
             mockk(relaxed = true),
+            emptyList(),
         )
 
         assertNull(routeProgress)
@@ -473,6 +478,7 @@ class NavigatorMapperTest {
             mockk(relaxed = true),
             0,
             mockk(relaxed = true),
+            emptyList(),
         )
 
         assertNotNull(routeProgress)
@@ -488,6 +494,7 @@ class NavigatorMapperTest {
             mockk(relaxed = true),
             0,
             lastVoiceInstruction = mockk(relaxed = true),
+            emptyList(),
         )
 
         assertTrue(routeProgress!!.stale)
@@ -503,6 +510,7 @@ class NavigatorMapperTest {
             mockk(relaxed = true),
             0,
             lastVoiceInstruction = mockk(relaxed = true),
+            emptyList(),
         )
 
         assertFalse(routeProgress!!.stale)
@@ -525,6 +533,7 @@ class NavigatorMapperTest {
             mockk(relaxed = true),
             0,
             mockk(relaxed = true),
+            emptyList(),
         )
         val upcomingRouteAlerts = routeProgress!!.upcomingRoadObjects
 

libnavigation-core/src/test/java/com/mapbox/navigation/core/trip/session/MapboxTripSessionTest.kt

@@ -184,7 +184,7 @@ class MapboxTripSessionTest {
         every { routeProgress.voiceInstructions } returns null
         every { routeProgress.currentLegProgress } returns mockk(relaxed = true)
         every {
-            getRouteProgressFrom(any(), any(), any(), any(), any(), any())
+            getRouteProgressFrom(any(), any(), any(), any(), any(), any(), any())
         } returns routeProgress
         every { routeProgress.currentState } returns RouteProgressState.TRACKING
         every { routes[0].directionsResponse.uuid() } returns "uuid"
@@ -406,7 +406,7 @@ class MapboxTripSessionTest {
 
     @Test
     fun routeProgressObserverNotCalledWhenInFreeDrive() = coroutineRule.runBlockingTest {
-        every { getRouteProgressFrom(any(), any(), any(), any(), any(), any()) } returns null
+        every { getRouteProgressFrom(any(), any(), any(), any(), any(), any(), any()) } returns null
         tripSession = buildTripSession()
         tripSession.start(true)
         val observer: RouteProgressObserver = mockk(relaxUnitFun = true)
@@ -1745,7 +1745,7 @@ class MapboxTripSessionTest {
 }
 
 private fun mockNavigationRoute(
-    roadObjects: List<UpcomingRoadObject> = listOf(mockk())
+    roadObjects: List<UpcomingRoadObject> = listOf(mockk(relaxed = true))
 ): NavigationRoute = mockk(relaxed = true) {
     every { upcomingRoadObjects } returns roadObjects
 }