diff --git a/changelog/unreleased/bugfixes/7581.md b/changelog/unreleased/bugfixes/7581.md new file mode 100644 index 00000000000..44d2a22ea7f --- /dev/null +++ b/changelog/unreleased/bugfixes/7581.md @@ -0,0 +1 @@ +- Fixed an issue where route refreshes might not have been paused after invoking `RouteRefreshController#pauseRouteRefreshes`. \ No newline at end of file diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt index 48468199ed2..104ed174b16 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt +++ b/libnavigation-core/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? = null private set fun startRoutesRefreshing(routes: List) { - 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/libnavigation-core/src/test/java/com/mapbox/navigation/core/routerefresh/RouteRefreshPauseResumeIntegrationTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/routerefresh/RouteRefreshPauseResumeIntegrationTest.kt index ed337ee3c76..14e87d65e14 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/routerefresh/RouteRefreshPauseResumeIntegrationTest.kt +++ b/libnavigation-core/src/test/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) } }