Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@korshaknn
korshaknn committed Sep 14, 2021
1 parent 1900612 commit 9b6d055
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 48 deletions.
123 changes: 75 additions & 48 deletions libnavigation-router/src/main/java/com/mapbox/navigation/route/internal/RouterWrapper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.mapbox.navigation.route.internal.util.httpUrl
import com.mapbox.navigation.route.internal.util.redactQueryParam
import com.mapbox.navigation.route.onboard.OfflineRoute
import com.mapbox.navigation.utils.internal.ThreadController
import com.mapbox.navigation.utils.internal.logI
import com.mapbox.navigation.utils.internal.logW
import com.mapbox.navigator.RouteRefreshOptions
import com.mapbox.navigator.RouterInterface
Expand Down Expand Up @@ -50,28 +51,11 @@ class RouterWrapper(

return router.getRoute(offlineRouteUrl) { result, origin ->
val urlWithoutToken = URL(offlineRouteUrl.redactQueryParam(ACCESS_TOKEN_QUERY_PARAM))

mainJobControl.scope.launch {
if (result.isValue) {
val routes = parseDirectionsResponse(result.value!!, routeOptions)
if (routes.isNullOrEmpty()) {
callback.onFailure(
listOf(
RouterFailure(
urlWithoutToken,
origin.mapToSdkRouteOrigin(),
ROUTES_LIST_EMPTY
)
),
routeOptions
)
} else {
callback.onRoutesReady(routes, origin.mapToSdkRouteOrigin())
}
} else {
result.error!!.run {
if (error == REQUEST_CANCELLED) {
logW(
result.fold(
{
mainJobControl.scope.launch {
if (it.error == REQUEST_CANCELLED) {
logI(
TAG,
Message(
"""
Expand All @@ -87,8 +71,8 @@ class RouterWrapper(
RouterFailure(
url = urlWithoutToken,
routerOrigin = origin.mapToSdkRouteOrigin(),
message = error,
code = code
message = it.error,
code = it.code
)
)

Expand All @@ -105,8 +89,27 @@ class RouterWrapper(
callback.onFailure(failureReasons, routeOptions)
}
}
},
{
mainJobControl.scope.launch {
val routes = parseDirectionsResponse(result.value!!, routeOptions)
if (routes.isNullOrEmpty()) {
callback.onFailure(
listOf(
RouterFailure(
urlWithoutToken,
origin.mapToSdkRouteOrigin(),
ROUTES_LIST_EMPTY
)
),
routeOptions
)
} else {
callback.onRoutesReady(routes, origin.mapToSdkRouteOrigin())
}
}
}
}
)
}
}

Expand All @@ -115,36 +118,46 @@ class RouterWrapper(
legIndex: Int,
callback: RouteRefreshCallback
): Long {
val routeOptions = route.routeOptions()!!
val routeOptions = route.routeOptions()
val requestUuid = route.requestUuid()
val routeIndex = route.routeIndex()?.toIntOrNull()
if (routeOptions == null || requestUuid == null || routeIndex == null) {
val errorMessage =
"""
Route refresh failed because of a null param:
routeOptions = $routeOptions
requestUuid = $requestUuid
routeIndex = $routeIndex
""".trimIndent()

logW(TAG, Message(errorMessage))

callback.onError(
RouteRefreshError("Route refresh failed", Exception(errorMessage))
)

return REQUEST_FAILURE
}

val userAndProfile = "${routeOptions.user()}/${routeOptions.profile()}"
val refreshOptions = RouteRefreshOptions(
route.requestUuid() ?: "",
route.routeIndex()?.toIntOrNull() ?: 0,
requestUuid,
routeIndex,
legIndex,
RoutingProfile(userAndProfile),
)

return router.getRouteRefresh(refreshOptions, route.toJson()) { result, _ ->
mainJobControl.scope.launch {
if (result.isValue) {
val refreshedRoute =
withContext(ThreadController.IODispatcher) {
DirectionsRoute.fromJson(
result.value!!,
routeOptions,
route.requestUuid()
)
}
callback.onRefresh(refreshedRoute)
} else {
result.error!!.run {
result.fold(
{
mainJobControl.scope.launch {
val errorMessage =
"""
Route refresh failed.
error = $error
legIndex = $legIndex
code = $code
requestId = $requestId
Route refresh failed.
error = ${it.error}
legIndex = $legIndex
code = ${it.code}
requestId = ${it.requestId}
""".trimIndent()

logW(TAG, Message(errorMessage))
Expand All @@ -153,8 +166,21 @@ class RouterWrapper(
RouteRefreshError("Route refresh failed", Exception(errorMessage))
)
}
},
{
mainJobControl.scope.launch {
val refreshedRoute =
withContext(ThreadController.IODispatcher) {
DirectionsRoute.fromJson(
result.value!!,
routeOptions,
route.requestUuid()
)
}
callback.onRefresh(refreshedRoute)
}
}
}
)
}
}

Expand Down Expand Up @@ -192,9 +218,10 @@ class RouterWrapper(
}

private companion object {
private val TAG = Tag("RouterWrapper")
private val TAG = Tag("MbxRouterWrapper")
private const val UUID = "uuid"
private const val REQUEST_CANCELLED = "Cancelled"
private const val ROUTES_LIST_EMPTY = "routes list is empty"
private const val REQUEST_FAILURE = -1L
}
}
97 changes: 97 additions & 0 deletions ...avigation-router/src/test/java/com/mapbox/navigation/route/internal/RouterWrapperTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ class RouterWrapperTests {
every { isError } returns false
every { value } returns SUCCESS_RESPONSE
every { error } returns null

val valueSlot = slot<Expected.Transformer<String, Unit>>()
every { fold(any(), capture(valueSlot)) } answers {
valueSlot.captured.invoke(this@mockk.value!!)
}
}
private val routerResultFailure: Expected<RouterError, String> = mockk {
every { isValue } returns false
Expand All @@ -88,6 +93,11 @@ class RouterWrapperTests {
FAILURE_CODE,
REQUEST_ID
)

val errorSlot = slot<Expected.Transformer<RouterError, Unit>>()
every { fold(capture(errorSlot), any()) } answers {
errorSlot.captured.invoke(this@mockk.error!!)
}
}
private val routerResultCancelled: Expected<RouterError, String> = mockk {
every { isValue } returns false
Expand All @@ -98,18 +108,33 @@ class RouterWrapperTests {
FAILURE_CODE,
REQUEST_ID
)

val errorSlot = slot<Expected.Transformer<RouterError, Unit>>()
every { fold(capture(errorSlot), any()) } answers {
errorSlot.captured.invoke(this@mockk.error!!)
}
}
private val routerResultSuccessEmptyRoutes: Expected<RouterError, String> = mockk {
every { isValue } returns true
every { isError } returns false
every { value } returns SUCCESS_RESPONSE_EMPTY_ROUTES
every { error } returns null

val valueSlot = slot<Expected.Transformer<String, Unit>>()
every { fold(any(), capture(valueSlot)) } answers {
valueSlot.captured.invoke(this@mockk.value!!)
}
}
private val routerRefreshSuccess: Expected<RouterError, String> = mockk {
every { isValue } returns true
every { isError } returns false
every { value } returns REFRESHED_ROUTE
every { error } returns null

val valueSlot = slot<Expected.Transformer<String, Unit>>()
every { fold(any(), capture(valueSlot)) } answers {
valueSlot.captured.invoke(this@mockk.value!!)
}
}
private val nativeOriginOnline: RouterOrigin = RouterOrigin.ONLINE
private val nativeOriginOnboard: RouterOrigin = RouterOrigin.ONBOARD
Expand Down Expand Up @@ -327,6 +352,78 @@ class RouterWrapperTests {
verify(exactly = 1) { router.cancelRequest(refreshIdTwo) }
}

@Test
fun `route refresh fails with null routeOptions`() {
val route: DirectionsRoute = mockk(relaxed = true)
every { route.requestUuid() } returns UUID
every { route.routeIndex() } returns "1"
every { route.routeOptions() } returns null

routerWrapper.getRouteRefresh(route, 0, routerRefreshCallback)

val expectedErrorMessage =
"""
Route refresh failed because of a null param:
routeOptions = null
requestUuid = $UUID
routeIndex = 1
""".trimIndent()

val errorSlot = slot<RouteRefreshError>()
verify(exactly = 1) { routerRefreshCallback.onError(capture(errorSlot)) }
verify(exactly = 0) { router.getRouteRefresh(any(), any(), any()) }
assertEquals("Route refresh failed", errorSlot.captured.message)
assertEquals(expectedErrorMessage, errorSlot.captured.throwable?.message)
}

@Test
fun `route refresh fails with null requestUuid`() {
val route: DirectionsRoute = mockk(relaxed = true)
every { route.requestUuid() } returns null
every { route.routeIndex() } returns "1"
every { route.routeOptions() } returns routerOptions

routerWrapper.getRouteRefresh(route, 0, routerRefreshCallback)

val expectedErrorMessage =
"""
Route refresh failed because of a null param:
routeOptions = $routerOptions
requestUuid = null
routeIndex = 1
""".trimIndent()

val errorSlot = slot<RouteRefreshError>()
verify(exactly = 1) { routerRefreshCallback.onError(capture(errorSlot)) }
verify(exactly = 0) { router.getRouteRefresh(any(), any(), any()) }
assertEquals("Route refresh failed", errorSlot.captured.message)
assertEquals(expectedErrorMessage, errorSlot.captured.throwable?.message)
}

@Test
fun `route refresh fails with null routeIndex`() {
val route: DirectionsRoute = mockk(relaxed = true)
every { route.requestUuid() } returns UUID
every { route.routeIndex() } returns null
every { route.routeOptions() } returns routerOptions

routerWrapper.getRouteRefresh(route, 0, routerRefreshCallback)

val expectedErrorMessage =
"""
Route refresh failed because of a null param:
routeOptions = $routerOptions
requestUuid = $UUID
routeIndex = null
""".trimIndent()

val errorSlot = slot<RouteRefreshError>()
verify(exactly = 1) { routerRefreshCallback.onError(capture(errorSlot)) }
verify(exactly = 0) { router.getRouteRefresh(any(), any(), any()) }
assertEquals("Route refresh failed", errorSlot.captured.message)
assertEquals(expectedErrorMessage, errorSlot.captured.throwable?.message)
}

@Ignore("Remove after https://github.com/mapbox/mapbox-navigation-native/issues/4016")
@Test
fun `route refresh set right params`() {
Expand Down

0 comments on commit 9b6d055

Please sign in to comment.