Skip to content

Commit 3c0a44a

Browse files
dzinaddzinad
committed
NAVAND-777: remove expiring data only after timeout
1 parent afda02e commit 3c0a44a

File tree

14 files changed

+538
-226
lines changed

14 files changed

+538
-226
lines changed

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

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import kotlinx.coroutines.flow.filter
3636
import kotlinx.coroutines.flow.first
3737
import kotlinx.coroutines.flow.take
3838
import kotlinx.coroutines.flow.toList
39+
import org.junit.After
3940
import org.junit.Assert.assertEquals
4041
import org.junit.Before
4142
import org.junit.Rule
@@ -51,6 +52,7 @@ class RouteRefreshOnDemandTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::
5152
@get:Rule
5253
val mockLocationReplayerRule = MockLocationReplayerRule(mockLocationUpdatesRule)
5354

55+
private lateinit var failedRefreshHandlerWrapper: FailByRequestMockRequestHandler
5456
private lateinit var refreshHandler: MockDirectionsRefreshHandler
5557
private lateinit var mapboxNavigation: MapboxNavigation
5658
private val twoCoordinates = listOf(
@@ -74,6 +76,11 @@ class RouteRefreshOnDemandTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::
7476
)
7577
}
7678

79+
@After
80+
fun tearDown() {
81+
failedRefreshHandlerWrapper.failResponse = false
82+
}
83+
7784
@Test(timeout = 10_000)
7885
fun route_refresh_on_demand_executes_before_refresh_interval() = sdkTest {
7986
val routeRefreshOptions = RouteRefreshOptions.Builder()
@@ -149,6 +156,55 @@ class RouteRefreshOnDemandTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::
149156
.toList()
150157
}
151158

159+
@Test
160+
fun failed_route_refresh_on_demand_does_not_notify_refresh_observer_before_timeout() = sdkTest {
161+
val refreshes = mutableListOf<RoutesUpdatedResult>()
162+
val routeRefreshOptions = RouteRefreshOptions.Builder()
163+
.intervalMillis(TimeUnit.MINUTES.toMillis(1))
164+
.build()
165+
failedRefreshHandlerWrapper.failResponse = true
166+
createMapboxNavigation(routeRefreshOptions)
167+
val routeOptions = generateRouteOptions(twoCoordinates)
168+
val requestedRoutes = mapboxNavigation.requestRoutes(routeOptions)
169+
.getSuccessfulResultOrThrowException()
170+
.routes
171+
mapboxNavigation.startTripSession()
172+
mapboxNavigation.registerRoutesObserver {
173+
if (it.reason == RoutesExtra.ROUTES_UPDATE_REASON_REFRESH) {
174+
refreshes.add(it)
175+
}
176+
}
177+
stayOnInitialPosition()
178+
mapboxNavigation.setNavigationRoutesAndWaitForUpdate(requestedRoutes)
179+
180+
mapboxNavigation.refreshRoutesImmediately()
181+
delay(2000)
182+
assertEquals(0, refreshes.size)
183+
}
184+
185+
@Test
186+
fun failed_route_refresh_on_demand_notifies_refresh_observer_after_timeout() = sdkTest {
187+
val routeRefreshOptions = createRouteRefreshOptionsWithInvalidInterval(3000)
188+
failedRefreshHandlerWrapper.failResponse = true
189+
createMapboxNavigation(routeRefreshOptions)
190+
val routeOptions = generateRouteOptions(twoCoordinates)
191+
val requestedRoutes = mapboxNavigation.requestRoutes(routeOptions)
192+
.getSuccessfulResultOrThrowException()
193+
.routes
194+
mapboxNavigation.startTripSession()
195+
stayOnInitialPosition()
196+
mapboxNavigation.setNavigationRoutesAndWaitForUpdate(requestedRoutes)
197+
198+
delay(7000) // 2 failed planned attempts + accuracy
199+
mapboxNavigation.refreshRoutesImmediately() // fail and postpone next planned attempt
200+
delay(2000)
201+
mapboxNavigation.refreshRoutesImmediately() // should distance new routes with REFRESH reason
202+
203+
mapboxNavigation.routesUpdates()
204+
.filter { it.reason == RoutesExtra.ROUTES_UPDATE_REASON_REFRESH }
205+
.first()
206+
}
207+
152208
private fun createMapboxNavigation(routeRefreshOptions: RouteRefreshOptions) {
153209
mapboxNavigation = MapboxNavigationProvider.create(
154210
NavigationOptions.Builder(activity)
@@ -204,7 +260,8 @@ class RouteRefreshOnDemandTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::
204260
readRawFileText(activity, refreshResponse),
205261
acceptedGeometryIndex
206262
)
207-
mockWebServerRule.requestHandlers.add(FailByRequestMockRequestHandler(refreshHandler))
263+
failedRefreshHandlerWrapper = FailByRequestMockRequestHandler(refreshHandler)
264+
mockWebServerRule.requestHandlers.add(failedRefreshHandlerWrapper)
208265
mockWebServerRule.requestHandlers.add(MockRoutingTileEndpointErrorRequestHandler())
209266
}
210267

@@ -213,4 +270,17 @@ class RouteRefreshOnDemandTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::
213270
?.annotation()
214271
?.duration()
215272
?.sum()!!
273+
274+
private fun createRouteRefreshOptionsWithInvalidInterval(
275+
intervalMillis: Long
276+
): RouteRefreshOptions {
277+
val routeRefreshOptions = RouteRefreshOptions.Builder()
278+
.intervalMillis(TimeUnit.SECONDS.toMillis(30))
279+
.build()
280+
RouteRefreshOptions::class.java.getDeclaredField("intervalMillis").apply {
281+
isAccessible = true
282+
set(routeRefreshOptions, intervalMillis)
283+
}
284+
return routeRefreshOptions
285+
}
216286
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
352352
mapboxNavigation.setNavigationRoutesAndWaitForUpdate(requestedRoutes)
353353
mapboxNavigation.refreshRoutesImmediately()
354354

355-
waitForRefresh()
355+
delay(2500) // execute request
356356

357357
assertEquals(
358358
listOf(
@@ -461,7 +461,7 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
461461
mapboxNavigation.refreshRoutesImmediately()
462462
delay(5000)
463463

464-
waitForRefreshes(2) // immediate + planned
464+
waitForRefresh()
465465

466466
assertEquals(
467467
listOf(
@@ -533,7 +533,7 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
533533

534534
mapboxNavigation.refreshRoutesImmediately()
535535

536-
waitForRefreshes(2) // one from immediate and the next planned
536+
waitForRefresh()
537537

538538
assertEquals(
539539
listOf(

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package com.mapbox.navigation.core.routerefresh
2+
3+
import com.mapbox.api.directions.v5.models.DirectionsRoute
4+
import com.mapbox.api.directions.v5.models.RouteLeg
5+
import com.mapbox.navigation.base.internal.route.update
6+
import com.mapbox.navigation.base.internal.time.parseISO8601DateToLocalTimeOrNull
7+
import com.mapbox.navigation.base.route.NavigationRoute
8+
import java.util.Date
9+
10+
internal class ExpiringDataRemover(
11+
private val localDateProvider: () -> Date,
12+
) {
13+
14+
fun removeExpiringDataFromRoutes(
15+
routes: List<NavigationRoute>,
16+
currentLegIndex: Int,
17+
): List<NavigationRoute> {
18+
return routes.map {
19+
removeExpiringDataFromRoute(it, currentLegIndex)
20+
}
21+
}
22+
23+
private fun removeExpiringDataFromRoute(
24+
route: NavigationRoute,
25+
currentLegIndex: Int,
26+
): NavigationRoute {
27+
val routeLegs = route.directionsRoute.legs()
28+
val directionsRouteBlock: DirectionsRoute.() -> DirectionsRoute = {
29+
toBuilder().legs(
30+
routeLegs?.mapIndexed { legIndex, leg ->
31+
val legHasAlreadyBeenPassed = legIndex < currentLegIndex
32+
if (legHasAlreadyBeenPassed) {
33+
leg
34+
} else {
35+
removeExpiredDataFromLeg(leg)
36+
}
37+
}
38+
).build()
39+
}
40+
return route.update(
41+
directionsRouteBlock = directionsRouteBlock,
42+
directionsResponseBlock = { this }
43+
)
44+
}
45+
46+
private fun removeExpiredDataFromLeg(leg: RouteLeg): RouteLeg {
47+
val oldAnnotation = leg.annotation()
48+
return leg.toBuilder()
49+
.annotation(
50+
oldAnnotation?.let { nonNullOldAnnotation ->
51+
nonNullOldAnnotation.toBuilder()
52+
.congestion(nonNullOldAnnotation.congestion()?.map { "unknown" })
53+
.congestionNumeric(nonNullOldAnnotation.congestionNumeric()?.map { null })
54+
.build()
55+
}
56+
)
57+
.incidents(
58+
leg.incidents()?.filter {
59+
val parsed = parseISO8601DateToLocalTimeOrNull(it.endTime())
60+
?: return@filter true
61+
val currentDate = localDateProvider()
62+
parsed > currentDate
63+
}
64+
)
65+
.build()
66+
}
67+
}

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
2929
stateHolder,
3030
listener,
3131
CancellableHandler(scope),
32-
RetryRouteRefreshStrategy(maxRetryCount = 2)
32+
RetryRouteRefreshStrategy(maxRetryCount = MAX_RETRY_COUNT)
3333
)
3434

3535
private var paused = false
@@ -132,14 +132,16 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
132132
scheduleUpdateRetry(routes, shouldNotifyOnStart = false)
133133
} else {
134134
stateHolder.onFailure(null)
135-
if (routeRefresherResult.refreshedRoutes != routes) {
136-
listener.onRoutesRefreshed(routeRefresherResult)
137-
} else {
138-
scheduleNewUpdate(routes)
139-
}
135+
listener.onRoutesRefreshed(routeRefresherResult)
136+
scheduleNewUpdate(routes)
140137
}
141138
}
142139
}
143140
}
144141
}
142+
143+
companion object {
144+
145+
const val MAX_RETRY_COUNT = 2
146+
}
145147
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ internal fun interface RouteRefreshObserver {
77
fun onRoutesRefreshed(routeInfo: RefreshedRouteInfo)
88
}
99

10-
internal class RefreshObserversManager : RouteRefresherListener {
10+
internal class RefreshObserversManager {
1111

1212
private val refreshObservers = CopyOnWriteArraySet<RouteRefreshObserver>()
1313

@@ -23,7 +23,7 @@ internal class RefreshObserversManager : RouteRefresherListener {
2323
refreshObservers.clear()
2424
}
2525

26-
override fun onRoutesRefreshed(result: RouteRefresherResult) {
26+
fun onRoutesRefreshed(result: RouteRefresherResult) {
2727
refreshObservers.forEach { observer ->
2828
observer.onRoutesRefreshed(result.toRefreshedRoutesInfo())
2929
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ internal class RouteRefreshController(
99
private val immediateRouteRefreshController: ImmediateRouteRefreshController,
1010
private val stateHolder: RouteRefreshStateHolder,
1111
private val refreshObserversManager: RefreshObserversManager,
12+
private val routeRefresherResultProcessor: RouteRefresherResultProcessor,
1213
) {
1314

1415
fun registerRouteRefreshObserver(observer: RouteRefreshObserver) {
@@ -35,6 +36,7 @@ internal class RouteRefreshController(
3536
}
3637

3738
fun requestPlannedRouteRefresh(routes: List<NavigationRoute>) {
39+
routeRefresherResultProcessor.reset()
3840
plannedRouteRefreshController.startRoutesRefreshing(routes)
3941
}
4042

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import com.mapbox.navigation.base.route.RouteRefreshOptions
55
import com.mapbox.navigation.core.RouteProgressDataProvider
66
import com.mapbox.navigation.core.directions.session.DirectionsSession
77
import com.mapbox.navigation.utils.internal.ThreadController
8+
import com.mapbox.navigation.utils.internal.Time
89
import java.util.Date
910

1011
@OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
@@ -23,7 +24,6 @@ internal object RouteRefreshControllerProvider {
2324
evDataHolder,
2425
DirectionsRouteDiffProvider(),
2526
directionsSession,
26-
{ Date() },
2727
)
2828
val routeRefresherExecutor = RouteRefresherExecutor(
2929
routeRefresher,
@@ -32,25 +32,32 @@ internal object RouteRefreshControllerProvider {
3232
)
3333
val stateHolder = RouteRefreshStateHolder()
3434
val refreshObserversManager = RefreshObserversManager()
35+
val routeRefresherResultProcessor = RouteRefresherResultProcessor(
36+
refreshObserversManager,
37+
ExpiringDataRemover { Date() },
38+
Time.SystemImpl,
39+
routeRefreshOptions.intervalMillis * (PlannedRouteRefreshController.MAX_RETRY_COUNT + 1)
40+
)
3541

3642
val plannedRouteRefreshController = PlannedRouteRefreshController(
3743
routeRefresherExecutor,
3844
routeRefreshOptions,
3945
stateHolder,
4046
scope,
41-
refreshObserversManager
47+
routeRefresherResultProcessor
4248
)
4349
val immediateRouteRefreshController = ImmediateRouteRefreshController(
4450
routeRefresherExecutor,
4551
plannedRouteRefreshController,
4652
stateHolder,
47-
refreshObserversManager
53+
routeRefresherResultProcessor
4854
)
4955
return RouteRefreshController(
5056
plannedRouteRefreshController,
5157
immediateRouteRefreshController,
5258
stateHolder,
53-
refreshObserversManager
59+
refreshObserversManager,
60+
routeRefresherResultProcessor
5461
)
5562
}
5663
}

0 commit comments

Comments
 (0)