Skip to content

Commit 257caa1

Browse files
dzinaddzinad
committed
NAVAND-777: remove expiring data only after timeout
1 parent 8a410ac commit 257caa1

File tree

12 files changed

+521
-224
lines changed

12 files changed

+521
-224
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() // dispatch 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/RouteRefresher.kt‎

Lines changed: 1 addition & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
package com.mapbox.navigation.core.routerefresh
22

3-
import com.mapbox.api.directions.v5.models.DirectionsRoute
4-
import com.mapbox.api.directions.v5.models.RouteLeg
53
import com.mapbox.navigation.base.internal.RouteRefreshRequestData
6-
import com.mapbox.navigation.base.internal.route.update
7-
import com.mapbox.navigation.base.internal.time.parseISO8601DateToLocalTimeOrNull
84
import com.mapbox.navigation.base.route.NavigationRoute
95
import com.mapbox.navigation.base.route.NavigationRouterRefreshCallback
106
import com.mapbox.navigation.base.route.NavigationRouterRefreshError
@@ -19,7 +15,6 @@ import kotlinx.coroutines.awaitAll
1915
import kotlinx.coroutines.coroutineScope
2016
import kotlinx.coroutines.suspendCancellableCoroutine
2117
import kotlinx.coroutines.withTimeoutOrNull
22-
import java.util.Date
2318
import kotlin.coroutines.resume
2419

2520
internal data class RouteRefresherResult(
@@ -33,7 +28,6 @@ internal class RouteRefresher(
3328
private val evDataHolder: EVDynamicDataHolder,
3429
private val routeDiffProvider: DirectionsRouteDiffProvider,
3530
private val routeRefresh: RouteRefresh,
36-
private val localDateProvider: () -> Date,
3731
) {
3832

3933
suspend fun refresh(
@@ -53,7 +47,7 @@ internal class RouteRefresher(
5347
} else {
5448
RouteRefresherResult(
5549
success = false,
56-
routes.map { removeExpiringDataFromRoute(it, routeProgressData.legIndex) },
50+
routes,
5751
routeProgressData
5852
)
5953
}
@@ -166,51 +160,6 @@ internal class RouteRefresher(
166160
}
167161
}
168162

169-
private fun removeExpiringDataFromRoute(
170-
route: NavigationRoute,
171-
currentLegIndex: Int,
172-
): NavigationRoute {
173-
val routeLegs = route.directionsRoute.legs()
174-
val directionsRouteBlock: DirectionsRoute.() -> DirectionsRoute = {
175-
toBuilder().legs(
176-
routeLegs?.mapIndexed { legIndex, leg ->
177-
val legHasAlreadyBeenPassed = legIndex < currentLegIndex
178-
if (legHasAlreadyBeenPassed) {
179-
leg
180-
} else {
181-
removeExpiredDataFromLeg(leg)
182-
}
183-
}
184-
).build()
185-
}
186-
return route.update(
187-
directionsRouteBlock = directionsRouteBlock,
188-
directionsResponseBlock = { this }
189-
)
190-
}
191-
192-
private fun removeExpiredDataFromLeg(leg: RouteLeg): RouteLeg {
193-
val oldAnnotation = leg.annotation()
194-
return leg.toBuilder()
195-
.annotation(
196-
oldAnnotation?.let { nonNullOldAnnotation ->
197-
nonNullOldAnnotation.toBuilder()
198-
.congestion(nonNullOldAnnotation.congestion()?.map { "unknown" })
199-
.congestionNumeric(nonNullOldAnnotation.congestionNumeric()?.map { null })
200-
.build()
201-
}
202-
)
203-
.incidents(
204-
leg.incidents()?.filter {
205-
val parsed = parseISO8601DateToLocalTimeOrNull(it.endTime())
206-
?: return@filter true
207-
val currentDate = localDateProvider()
208-
parsed > currentDate
209-
}
210-
)
211-
.build()
212-
}
213-
214163
private sealed class RouteRefreshResult {
215164
data class Success(val route: NavigationRoute) : RouteRefreshResult()
216165
data class Fail(val error: NavigationRouterRefreshError) : RouteRefreshResult()

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package com.mapbox.navigation.core.routerefresh
2+
3+
import com.mapbox.navigation.utils.internal.Time
4+
5+
internal class RouteRefresherResultProcessor(
6+
private val observersManager: RefreshObserversManager,
7+
private val expiringDataRemover: ExpiringDataRemover,
8+
private val timeProvider: Time,
9+
private val staleDataTimeoutMillis: Long,
10+
) : RouteRefresherListener {
11+
12+
private var lastRefreshTimeMillis: Long = 0
13+
14+
fun reset() {
15+
lastRefreshTimeMillis = timeProvider.millis()
16+
}
17+
18+
override fun onRoutesRefreshed(result: RouteRefresherResult) {
19+
val currentTime = timeProvider.millis()
20+
if (result.success) {
21+
lastRefreshTimeMillis = currentTime
22+
observersManager.onRoutesRefreshed(result)
23+
} else {
24+
if (currentTime >= lastRefreshTimeMillis + staleDataTimeoutMillis) {
25+
lastRefreshTimeMillis = currentTime
26+
val newRoutes = expiringDataRemover.removeExpiringDataFromRoutes(
27+
result.refreshedRoutes,
28+
result.routeProgressData.legIndex
29+
)
30+
if (result.refreshedRoutes != newRoutes) {
31+
val processedResult = RouteRefresherResult(
32+
result.success,
33+
newRoutes,
34+
result.routeProgressData
35+
)
36+
observersManager.onRoutesRefreshed(processedResult)
37+
}
38+
}
39+
}
40+
}
41+
}

0 commit comments

Comments
 (0)