NAVAND-1158: ignore old alternatives

Author: dzinad

changelog/unreleased/bugfixes/dd.md

@@ -0,0 +1 @@
+- Fixed a rare race condition where the alternative routes might have not been in sync with the current primary route.  

libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt

@@ -1044,33 +1044,48 @@ class MapboxNavigation @VisibleForTesting internal constructor(
         restartRouteRefreshScope()
         threadController.getMainScopeAndRootJob().scope.launch(Dispatchers.Main.immediate) {
             routeUpdateMutex.withLock {
-                historyRecordingStateHandler.setRoutes(routes)
                 val routesSetResult: Expected<RoutesSetError, RoutesSetSuccess>
-                when (val processedRoutes = setRoutesToTripSession(routes, setRoutesInfo)) {
-                    is NativeSetRouteValue -> {
-                        val directionsSessionRoutes = Utils.createDirectionsSessionRoutes(
-                            routes,
-                            processedRoutes,
-                            setRoutesInfo
+                if (
+                    setRoutesInfo is SetRoutes.Alternatives &&
+                    routes.first().id != directionsSession.routes.firstOrNull()?.id
+                ) {
+                    routesSetResult = ExpectedFactory.createError(
+                        RoutesSetError(
+                            "Alternatives ${routes.drop(1).map { it.id }} " +
+                                "are outdated. Primary route has changed " +
+                                "from ${routes.first().id} " +
+                                "to ${directionsSession.routes.firstOrNull()?.id}"
                         )
-                        directionsSession.setRoutes(directionsSessionRoutes)
-                        routesSetResult = ExpectedFactory.createValue(
-                            RoutesSetSuccess(
-                                directionsSessionRoutes.ignoredRoutes.associate {
-                                    it.navigationRoute.id to RoutesSetError("invalid alternative")
-                                }
+                    )
+                } else {
+                    historyRecordingStateHandler.setRoutes(routes)
+                    when (val processedRoutes = setRoutesToTripSession(routes, setRoutesInfo)) {
+                        is NativeSetRouteValue -> {
+                            val directionsSessionRoutes = Utils.createDirectionsSessionRoutes(
+                                routes,
+                                processedRoutes,
+                                setRoutesInfo
                             )
-                        )
-                    }
-                    is NativeSetRouteError -> {
-                        logE(
-                            "Routes with IDs ${routes.map { it.id }} " +
-                                "will be ignored as they are not valid"
-                        )
-                        routesSetResult = ExpectedFactory.createError(
-                            RoutesSetError(processedRoutes.error)
-                        )
-                        historyRecordingStateHandler.lastSetRoutesFailed()
+                            directionsSession.setRoutes(directionsSessionRoutes)
+                            routesSetResult = ExpectedFactory.createValue(
+                                RoutesSetSuccess(
+                                    directionsSessionRoutes.ignoredRoutes.associate {
+                                        it.navigationRoute.id to
+                                            RoutesSetError("invalid alternative")
+                                    }
+                                )
+                            )
+                        }
+                        is NativeSetRouteError -> {
+                            logE(
+                                "Routes with IDs ${routes.map { it.id }} " +
+                                    "will be ignored as they are not valid"
+                            )
+                            routesSetResult = ExpectedFactory.createError(
+                                RoutesSetError(processedRoutes.error)
+                            )
+                            historyRecordingStateHandler.lastSetRoutesFailed()
+                        }
                     }
                 }
                 callback?.onRoutesSet(routesSetResult)

libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt

@@ -1746,6 +1746,78 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
         }
     }
 
+    @Test
+    fun `setNavigationRoutes alternative for current primary route`() = coroutineRule.runBlockingTest {
+        createMapboxNavigation()
+        val route1 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id1"
+        }
+        val route2 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id2"
+        }
+        val route3 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id3"
+        }
+        every { directionsSession.routes } returns listOf(route1, route2)
+
+        mapboxNavigation.setNavigationRoutes(listOf(route1, route3))
+
+        coVerify(exactly = 1) {
+            tripSession.setRoutes(any(), ofType(SetRoutes.Alternatives::class))
+        }
+        verify(exactly = 1) {
+            directionsSession.setRoutes(any())
+        }
+    }
+
+    @Test
+    fun `setNavigationRoutes new routes`() = coroutineRule.runBlockingTest {
+        createMapboxNavigation()
+        val route1 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id1"
+        }
+        val route2 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id2"
+        }
+        val route3 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id3"
+        }
+        every { directionsSession.routes } returns listOf(route1, route2)
+
+        mapboxNavigation.setNavigationRoutes(listOf(route3, route2))
+
+        coVerify(exactly = 1) {
+            tripSession.setRoutes(any(), ofType(SetRoutes.NewRoutes::class))
+        }
+        verify(exactly = 1) {
+            directionsSession.setRoutes(any())
+        }
+    }
+
+    @Test
+    fun `setNavigationRoutes alternative for outdated primary route`() = coroutineRule.runBlockingTest {
+        createMapboxNavigation()
+        val route1 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id1"
+        }
+        val route2 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id2"
+        }
+        val route3 = mockk<NavigationRoute>(relaxed = true) {
+            every { id } returns "id3"
+        }
+        every { directionsSession.routes } returnsMany listOf(listOf(route1), listOf(route2))
+
+        mapboxNavigation.setNavigationRoutes(listOf(route1, route3))
+
+        coVerify(exactly = 0) {
+            tripSession.setRoutes(any(), any())
+        }
+        verify(exactly = 0) {
+            directionsSession.setRoutes(any())
+        }
+    }
+
     @Test
     fun `refreshed route is set to trip session and directions session`() =
         coroutineRule.runBlockingTest {