Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAVAND-1581 fixing reroute after destroy #7642

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.mapbox.navigation.core.reroute.NavigationRerouteController
import com.mapbox.navigation.core.reroute.RerouteController
import com.mapbox.navigation.core.reroute.RerouteState
import com.mapbox.navigation.instrumentation_tests.R
import com.mapbox.navigation.instrumentation_tests.utils.BlockResponseModifier
import com.mapbox.navigation.instrumentation_tests.utils.DelayedResponseModifier
import com.mapbox.navigation.instrumentation_tests.utils.assertions.RerouteStateTransitionAssertion
import com.mapbox.navigation.instrumentation_tests.utils.assertions.RouteProgressStateTransitionAssertion
Expand Down Expand Up @@ -70,6 +71,7 @@ import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Assert.fail
import org.junit.Before
import org.junit.Ignore
import org.junit.Rule
Expand Down Expand Up @@ -895,6 +897,58 @@ class CoreRerouteTest : BaseCoreNoCleanUpTest() {
}
}

@Test
fun reroute_during_destroy_of_navigation() = sdkTest {
val mapboxNavigation = createMapboxNavigation()
val mockRoute = RoutesProvider.dc_very_short(context)
val originLocation = mockRoute.routeWaypoints.first()
val offRouteLocationUpdate = mockLocationUpdatesRule.generateLocationUpdate {
latitude = originLocation.latitude() + 0.002
longitude = originLocation.longitude()
}

mockWebServerRule.requestHandlers.addAll(mockRoute.mockRequestHandlers)
val rerouteResponseBlock = BlockResponseModifier()
mockWebServerRule.requestHandlers.add(
MockDirectionsRequestHandler(
profile = DirectionsCriteria.PROFILE_DRIVING_TRAFFIC,
jsonResponse = readRawFileText(context, R.raw.reroute_response_dc_very_short),
expectedCoordinates = listOf(
Point.fromLngLat(
offRouteLocationUpdate.longitude,
offRouteLocationUpdate.latitude
),
mockRoute.routeWaypoints.last()
),
relaxedExpectedCoordinates = true
).apply {
jsonResponseModifier = rerouteResponseBlock
}
)

mapboxNavigation.startTripSession()
val routes = mapboxNavigation.requestRoutes(
RouteOptions.builder()
.applyDefaultNavigationOptions()
.applyLanguageAndVoiceUnitOptions(context)
.baseUrl(mockWebServerRule.baseUrl)
.coordinatesList(mockRoute.routeWaypoints).build()
).getSuccessfulResultOrThrowException().routes
mapboxNavigation.setNavigationRoutes(routes)

mockLocationReplayerRule.loopUpdateUntil(offRouteLocationUpdate) {
mapboxNavigation.routeProgressUpdates()
.filter { it.currentState == RouteProgressState.OFF_ROUTE }
.first()
}
mapboxNavigation.onDestroy()
mapboxNavigation.registerRoutesObserver {
fail("routes shouldn't be updated after destroy, but received $it")
}
rerouteResponseBlock.release()
delay(3000)
}

private fun createMapboxNavigation(customRefreshInterval: Long? = null): MapboxNavigation {
var mapboxNavigation: MapboxNavigation? = null

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.mapbox.navigation.instrumentation_tests.utils

import java.util.concurrent.CountDownLatch

class BlockResponseModifier : (String) -> String {

private val countDownLatch = CountDownLatch(1)

fun release() {
countDownLatch.countDown()
}

override fun invoke(p1: String): String {
countDownLatch.await()
return p1
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1289,8 +1289,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
runInTelemetryContext { telemetry ->
telemetry.destroy(this@MapboxNavigation)
}
threadController.cancelAllNonUICoroutines()
threadController.cancelAllUICoroutines()
threadController.destroy()
ifNonNull(reachabilityObserverId) {
ReachabilityService.removeReachabilityObserver(it)
reachabilityObserverId = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,10 @@ class ThreadController {
val parentJob = SupervisorJob(mainRootJob)
return JobControl(parentJob, CoroutineScope(parentJob + Dispatchers.Main))
}

fun destroy() {
val reason = CancellationException("thread controller is destroyed")
mainRootJob.cancel(reason)
ioRootJob.cancel(reason)
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.mapbox.navigation.utils.internal

import com.mapbox.navigation.testing.MainCoroutineRule
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CompletableJob
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
Expand All @@ -14,13 +16,19 @@ import kotlinx.coroutines.runBlocking
import org.hamcrest.CoreMatchers.`is`
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertThat
import org.junit.Assert.assertTrue
import org.junit.Assert.fail
import org.junit.Rule
import org.junit.Test
import kotlin.coroutines.suspendCoroutine

class ThreadControllerTest {

@get:Rule
val testCoroutineRule = MainCoroutineRule()

private val threadController = ThreadController()

@Test
Expand Down Expand Up @@ -132,4 +140,29 @@ class ThreadControllerTest {
mainJobController.scope.toString()
)
}

@Test
fun destroy_thread_controller() {
val handler = CompletableDeferred<Unit>()
var errorMessage: String? = null
threadController.getIOScopeAndRootJob().scope.launch {
handler.await()
errorMessage = "IO scope should be cancelled"
}
threadController.getMainScopeAndRootJob().scope.launch {
handler.await()
errorMessage = "UI scope should be cancelled"
}
threadController.destroy()
handler.complete(Unit)
assertNull(errorMessage)

threadController.getIOScopeAndRootJob().scope.launch {
errorMessage = "IO scope should be cancelled"
}
threadController.getMainScopeAndRootJob().scope.launch {
errorMessage = "UI scope should be cancelled"
}
assertNull(errorMessage)
}
}