NAVAND-1564: do not resume route refreshes after routes are changed

mapbox · Oct 23, 2023 · 3abcabf · 3abcabf
1 parent a26ec08
commit 3abcabf
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 16 deletions.
diff --git a/changelog/unreleased/bugfixes/dd.md b/changelog/unreleased/bugfixes/dd.md
@@ -0,0 +1 @@
+- Fixed an issue where route refreshes might not have been paused after invoking `RouteRefreshController#pauseRouteRefreshes`.
diff --git a/...re/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt b/...re/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt
@@ -43,13 +43,16 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
         RetryRouteRefreshStrategy(maxAttemptsCount = MAX_RETRY_COUNT)
     )
 
-    private var plannedRefreshScope = CoroutineUtils.createChildScope(parentScope)
-    private var paused = false
+    // null if refreshes are paused
+    private var plannedRefreshScope: CoroutineScope? = CoroutineUtils.createChildScope(parentScope)
     var routesToRefresh: List<NavigationRoute>? = null
         private set
 
     fun startRoutesRefreshing(routes: List<NavigationRoute>) {
-        recreateScope()
+        if (plannedRefreshScope != null) {
+            plannedRefreshScope?.cancel()
+            plannedRefreshScope = CoroutineUtils.createChildScope(parentScope)
+        }
         if (routes.isEmpty()) {
             routesToRefresh = null
             logI("Routes are empty, nothing to refresh", RouteRefreshLog.LOG_CATEGORY)
@@ -82,15 +85,15 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
     }
 
     fun pause() {
-        if (!paused) {
-            paused = true
-            recreateScope()
-        }
+        logI("Pausing refreshes", RouteRefreshLog.LOG_CATEGORY)
+        plannedRefreshScope?.cancel()
+        plannedRefreshScope = null
     }
 
     fun resume() {
-        if (paused) {
-            paused = false
+        logI("Resuming refreshes", RouteRefreshLog.LOG_CATEGORY)
+        if (plannedRefreshScope == null) {
+            plannedRefreshScope = CoroutineUtils.createChildScope(parentScope)
             routesToRefresh?.let {
                 if (retryStrategy.shouldRetry()) {
                     scheduleUpdateRetry(it, shouldNotifyOnStart = true)
@@ -114,7 +117,7 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
     }
 
     private fun postAttempt(shouldResume: Boolean, attemptBlock: suspend () -> Unit) {
-        plannedRefreshScope.launch {
+        plannedRefreshScope?.launch {
             try {
                 if (shouldResume) {
                     delayer.resumeDelay()
@@ -177,11 +180,6 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
         }
     }
 
-    private fun recreateScope() {
-        plannedRefreshScope.cancel()
-        plannedRefreshScope = CoroutineUtils.createChildScope(parentScope)
-    }
-
     companion object {
 
         const val MAX_RETRY_COUNT = 2

diff --git a/...st/java/com/mapbox/navigation/core/routerefresh/RouteRefreshPauseResumeIntegrationTest.kt b/...st/java/com/mapbox/navigation/core/routerefresh/RouteRefreshPauseResumeIntegrationTest.kt
@@ -36,6 +36,27 @@ internal class RouteRefreshPauseResumeIntegrationTest : RouteRefreshIntegrationT
         routeRefreshController.destroy()
     }
 
+    @Test
+    fun pause_refresh_while_request_is_in_progress() = runBlocking {
+        val routes = setUpRoutes("route_response_single_route_refresh.json", responseDelay = 2000)
+        routeRefreshController = createRefreshController(refreshInterval = 60_000)
+        routeRefreshController.registerRouteRefreshObserver(refreshObserver)
+        routeRefreshController.onRoutesChanged(
+            RoutesUpdatedResult(routes, emptyList(), RoutesExtra.ROUTES_UPDATE_REASON_NEW)
+        )
+        testDispatcher.advanceTimeBy(61_000)
+
+        routeRefreshController.pauseRouteRefreshes()
+
+        testDispatcher.advanceTimeBy(1_000)
+
+        assertEquals(0, refreshObserver.refreshes.size)
+
+        testDispatcher.advanceTimeBy(62_000)
+
+        assertEquals(0, refreshObserver.refreshes.size)
+    }
+
     @Test
     fun request_new_routes_planned_refresh_when_refresh_is_paused() = runBlocking {
         val routes = setUpRoutes("route_response_single_route_refresh.json")
@@ -62,6 +83,6 @@ internal class RouteRefreshPauseResumeIntegrationTest : RouteRefreshIntegrationT
         assertEquals(1, refreshObserver.refreshes.size)
 
         testDispatcher.advanceTimeBy(30_000)
-        assertEquals(2, refreshObserver.refreshes.size)
+        assertEquals(1, refreshObserver.refreshes.size)
     }
 }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		- Fixed an issue where route refreshes might not have been paused after invoking `RouteRefreshController#pauseRouteRefreshes`.