From c82fd9560ea2419b6bd4080963b9084c55d70a0f Mon Sep 17 00:00:00 2001 From: Dzina Date: Mon, 23 Oct 2023 19:18:54 +0300 Subject: [PATCH 1/2] NAVAND-1564: do not resume route refreshes after routes are changed --- changelog/unreleased/bugfixes/dd.md | 1 + .../PlannedRouteRefreshController.kt | 28 +++++++++---------- .../RouteRefreshPauseResumeIntegrationTest.kt | 23 ++++++++++++++- 3 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 changelog/unreleased/bugfixes/dd.md diff --git a/changelog/unreleased/bugfixes/dd.md b/changelog/unreleased/bugfixes/dd.md new file mode 100644 index 00000000000..44d2a22ea7f --- /dev/null +++ b/changelog/unreleased/bugfixes/dd.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) } } From b02ed9d1925a39be77902e1ee1c5caffc157d95e Mon Sep 17 00:00:00 2001 From: runner Date: Wed, 25 Oct 2023 11:23:59 +0000 Subject: [PATCH 2/2] Rename changelog files --- changelog/unreleased/bugfixes/{dd.md => 7581.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/unreleased/bugfixes/{dd.md => 7581.md} (100%) diff --git a/changelog/unreleased/bugfixes/dd.md b/changelog/unreleased/bugfixes/7581.md similarity index 100% rename from changelog/unreleased/bugfixes/dd.md rename to changelog/unreleased/bugfixes/7581.md