NAVAND-777: remove expiring data only after timeout

Author: dzinad

instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/RouteRefreshStateTest.kt

@@ -352,7 +352,7 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
         mapboxNavigation.setNavigationRoutesAndWaitForUpdate(requestedRoutes)
         mapboxNavigation.refreshRoutesImmediately()
 
-        waitForRefresh()
+        delay(2500) // execute request
 
         assertEquals(
             listOf(
@@ -461,7 +461,7 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
         mapboxNavigation.refreshRoutesImmediately()
         delay(5000)
 
-        waitForRefreshes(2) // immediate + planned
+        waitForRefresh()
 
         assertEquals(
             listOf(
@@ -533,7 +533,7 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
 
         mapboxNavigation.refreshRoutesImmediately()
 
-        waitForRefreshes(2) // one from immediate and the next planned
+        waitForRefresh()
 
         assertEquals(
             listOf(

libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/ExpiringDataRemover.kt

@@ -0,0 +1,67 @@
+package com.mapbox.navigation.core.routerefresh
+
+import com.mapbox.api.directions.v5.models.DirectionsRoute
+import com.mapbox.api.directions.v5.models.RouteLeg
+import com.mapbox.navigation.base.internal.route.update
+import com.mapbox.navigation.base.internal.time.parseISO8601DateToLocalTimeOrNull
+import com.mapbox.navigation.base.route.NavigationRoute
+import java.util.Date
+
+internal class ExpiringDataRemover(
+    private val localDateProvider: () -> Date,
+) {
+
+    fun removeExpiringDataFromRoutes(
+        routes: List<NavigationRoute>,
+        currentLegIndex: Int,
+    ): List<NavigationRoute> {
+        return routes.map {
+            removeExpiringDataFromRoute(it, currentLegIndex)
+        }
+    }
+
+    private fun removeExpiringDataFromRoute(
+        route: NavigationRoute,
+        currentLegIndex: Int,
+    ): NavigationRoute {
+        val routeLegs = route.directionsRoute.legs()
+        val directionsRouteBlock: DirectionsRoute.() -> DirectionsRoute = {
+            toBuilder().legs(
+                routeLegs?.mapIndexed { legIndex, leg ->
+                    val legHasAlreadyBeenPassed = legIndex < currentLegIndex
+                    if (legHasAlreadyBeenPassed) {
+                        leg
+                    } else {
+                        removeExpiredDataFromLeg(leg)
+                    }
+                }
+            ).build()
+        }
+        return route.update(
+            directionsRouteBlock = directionsRouteBlock,
+            directionsResponseBlock = { this }
+        )
+    }
+
+    private fun removeExpiredDataFromLeg(leg: RouteLeg): RouteLeg {
+        val oldAnnotation = leg.annotation()
+        return leg.toBuilder()
+            .annotation(
+                oldAnnotation?.let { nonNullOldAnnotation ->
+                    nonNullOldAnnotation.toBuilder()
+                        .congestion(nonNullOldAnnotation.congestion()?.map { "unknown" })
+                        .congestionNumeric(nonNullOldAnnotation.congestionNumeric()?.map { null })
+                        .build()
+                }
+            )
+            .incidents(
+                leg.incidents()?.filter {
+                    val parsed = parseISO8601DateToLocalTimeOrNull(it.endTime())
+                        ?: return@filter true
+                    val currentDate = localDateProvider()
+                    parsed > currentDate
+                }
+            )
+            .build()
+    }
+}

libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt

@@ -29,7 +29,7 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
         stateHolder,
         listener,
         CancellableHandler(scope),
-        RetryRouteRefreshStrategy(maxRetryCount = 2)
+        RetryRouteRefreshStrategy(maxRetryCount = MAX_RETRY_COUNT)
     )
 
     private var paused = false
@@ -142,4 +142,9 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
             }
         }
     }
+
+    companion object {
+
+        const val MAX_RETRY_COUNT = 2
+    }
 }

libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/RefreshObserversManager.kt

@@ -7,7 +7,7 @@ internal fun interface RouteRefreshObserver {
     fun onRoutesRefreshed(routeInfo: RefreshedRouteInfo)
 }
 
-internal class RefreshObserversManager : RouteRefresherListener {
+internal class RefreshObserversManager {
 
     private val refreshObservers = CopyOnWriteArraySet<RouteRefreshObserver>()
 
@@ -23,7 +23,7 @@ internal class RefreshObserversManager : RouteRefresherListener {
         refreshObservers.clear()
     }
 
-    override fun onRoutesRefreshed(result: RouteRefresherResult) {
+    fun onRoutesRefreshed(result: RouteRefresherResult) {
         refreshObservers.forEach { observer ->
             observer.onRoutesRefreshed(result.toRefreshedRoutesInfo())
         }

libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefreshController.kt

@@ -9,6 +9,7 @@ internal class RouteRefreshController(
     private val immediateRouteRefreshController: ImmediateRouteRefreshController,
     private val stateHolder: RouteRefreshStateHolder,
     private val refreshObserversManager: RefreshObserversManager,
+    private val routeRefresherResultProcessor: RouteRefresherResultProcessor,
 ) {
 
     fun registerRouteRefreshObserver(observer: RouteRefreshObserver) {
@@ -35,6 +36,7 @@ internal class RouteRefreshController(
     }
 
     fun requestPlannedRouteRefresh(routes: List<NavigationRoute>) {
+        routeRefresherResultProcessor.reset()
         plannedRouteRefreshController.startRoutesRefreshing(routes)
     }
 

libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefreshControllerProvider.kt

@@ -5,6 +5,7 @@ import com.mapbox.navigation.base.route.RouteRefreshOptions
 import com.mapbox.navigation.core.RouteProgressDataProvider
 import com.mapbox.navigation.core.directions.session.DirectionsSession
 import com.mapbox.navigation.utils.internal.ThreadController
+import com.mapbox.navigation.utils.internal.Time
 import java.util.Date
 
 @OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
@@ -23,7 +24,6 @@ internal object RouteRefreshControllerProvider {
             evDataHolder,
             DirectionsRouteDiffProvider(),
             directionsSession,
-            { Date() },
         )
         val routeRefresherExecutor = RouteRefresherExecutor(
             routeRefresher,
@@ -32,25 +32,32 @@ internal object RouteRefreshControllerProvider {
         )
         val stateHolder = RouteRefreshStateHolder()
         val refreshObserversManager = RefreshObserversManager()
+        val routeRefresherResultProcessor = RouteRefresherResultProcessor(
+            refreshObserversManager,
+            ExpiringDataRemover { Date() },
+            Time.SystemImpl,
+            routeRefreshOptions.intervalMillis * (PlannedRouteRefreshController.MAX_RETRY_COUNT + 1)
+        )
 
         val plannedRouteRefreshController = PlannedRouteRefreshController(
             routeRefresherExecutor,
             routeRefreshOptions,
             stateHolder,
             scope,
-            refreshObserversManager
+            routeRefresherResultProcessor
         )
         val immediateRouteRefreshController = ImmediateRouteRefreshController(
             routeRefresherExecutor,
             plannedRouteRefreshController,
             stateHolder,
-            refreshObserversManager
+            routeRefresherResultProcessor
         )
         return RouteRefreshController(
             plannedRouteRefreshController,
             immediateRouteRefreshController,
             stateHolder,
-            refreshObserversManager
+            refreshObserversManager,
+            routeRefresherResultProcessor
         )
     }
 }

libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefresher.kt

@@ -1,10 +1,6 @@
 package com.mapbox.navigation.core.routerefresh
 
-import com.mapbox.api.directions.v5.models.DirectionsRoute
-import com.mapbox.api.directions.v5.models.RouteLeg
 import com.mapbox.navigation.base.internal.RouteRefreshRequestData
-import com.mapbox.navigation.base.internal.route.update
-import com.mapbox.navigation.base.internal.time.parseISO8601DateToLocalTimeOrNull
 import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.base.route.NavigationRouterRefreshCallback
 import com.mapbox.navigation.base.route.NavigationRouterRefreshError
@@ -18,7 +14,6 @@ import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlinx.coroutines.withTimeoutOrNull
-import java.util.Date
 import kotlin.coroutines.resume
 
 internal data class RouteRefresherResult(
@@ -32,7 +27,6 @@ internal class RouteRefresher(
     private val evDataHolder: EVDataHolder,
     private val routeDiffProvider: DirectionsRouteDiffProvider,
     private val routeRefresh: RouteRefresh,
-    private val localDateProvider: () -> Date,
 ) {
 
     suspend fun refresh(
@@ -52,7 +46,7 @@ internal class RouteRefresher(
         } else {
             RouteRefresherResult(
                 success = false,
-                routes.map { removeExpiringDataFromRoute(it, routeProgressData.legIndex) },
+                routes,
                 routeProgressData
             )
         }
@@ -165,51 +159,6 @@ internal class RouteRefresher(
         }
     }
 
-    private fun removeExpiringDataFromRoute(
-        route: NavigationRoute,
-        currentLegIndex: Int,
-    ): NavigationRoute {
-        val routeLegs = route.directionsRoute.legs()
-        val directionsRouteBlock: DirectionsRoute.() -> DirectionsRoute = {
-            toBuilder().legs(
-                routeLegs?.mapIndexed { legIndex, leg ->
-                    val legHasAlreadyBeenPassed = legIndex < currentLegIndex
-                    if (legHasAlreadyBeenPassed) {
-                        leg
-                    } else {
-                        removeExpiredDataFromLeg(leg)
-                    }
-                }
-            ).build()
-        }
-        return route.update(
-            directionsRouteBlock = directionsRouteBlock,
-            directionsResponseBlock = { this }
-        )
-    }
-
-    private fun removeExpiredDataFromLeg(leg: RouteLeg): RouteLeg {
-        val oldAnnotation = leg.annotation()
-        return leg.toBuilder()
-            .annotation(
-                oldAnnotation?.let { nonNullOldAnnotation ->
-                    nonNullOldAnnotation.toBuilder()
-                        .congestion(nonNullOldAnnotation.congestion()?.map { "unknown" })
-                        .congestionNumeric(nonNullOldAnnotation.congestionNumeric()?.map { null })
-                        .build()
-                }
-            )
-            .incidents(
-                leg.incidents()?.filter {
-                    val parsed = parseISO8601DateToLocalTimeOrNull(it.endTime())
-                        ?: return@filter true
-                    val currentDate = localDateProvider()
-                    parsed > currentDate
-                }
-            )
-            .build()
-    }
-
     private sealed class RouteRefreshResult {
         data class Success(val route: NavigationRoute) : RouteRefreshResult()
         data class Fail(val error: NavigationRouterRefreshError) : RouteRefreshResult()

libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefresherResultProcessor.kt

@@ -0,0 +1,41 @@
+package com.mapbox.navigation.core.routerefresh
+
+import com.mapbox.navigation.utils.internal.Time
+
+internal class RouteRefresherResultProcessor(
+    private val observersManager: RefreshObserversManager,
+    private val expiringDataRemover: ExpiringDataRemover,
+    private val timeProvider: Time,
+    private val staleDataTimeoutMillis: Long,
+) : RouteRefresherListener {
+
+    private var lastRefreshTimeMillis: Long = 0
+
+    fun reset() {
+        lastRefreshTimeMillis = timeProvider.millis()
+    }
+
+    override fun onRoutesRefreshed(result: RouteRefresherResult) {
+        val currentTime = timeProvider.millis()
+        if (result.success) {
+            lastRefreshTimeMillis = currentTime
+            observersManager.onRoutesRefreshed(result)
+        } else {
+            if (currentTime >= lastRefreshTimeMillis + staleDataTimeoutMillis) {
+                lastRefreshTimeMillis = currentTime
+                val newRoutes = expiringDataRemover.removeExpiringDataFromRoutes(
+                    result.refreshedRoutes,
+                    result.routeProgressData.legIndex
+                )
+                if (result.refreshedRoutes != newRoutes) {
+                    val processedResult = RouteRefresherResult(
+                        result.success,
+                        newRoutes,
+                        result.routeProgressData
+                    )
+                    observersManager.onRoutesRefreshed(processedResult)
+                }
+            }
+        }
+    }
+}