NAVAND-1472: use a descriptive sealed class instead of Expected

mapbox · Sep 19, 2023 · 02614f5 · 02614f5
1 parent 0bea71e
commit 02614f5
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 59 deletions.
diff --git a/.../src/main/java/com/mapbox/navigation/core/routerefresh/ImmediateRouteRefreshController.kt b/.../src/main/java/com/mapbox/navigation/core/routerefresh/ImmediateRouteRefreshController.kt
@@ -1,6 +1,5 @@
 package com.mapbox.navigation.core.routerefresh
 
-import com.mapbox.bindgen.Expected
 import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
 import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.utils.internal.logW
@@ -22,7 +21,7 @@ internal class ImmediateRouteRefreshController(
     @Throws(IllegalArgumentException::class)
     fun requestRoutesRefresh(
         routes: List<NavigationRoute>,
-        callback: (Expected<String, RoutesRefresherResult>) -> Unit
+        callback: (RoutesRefresherExecutorResult) -> Unit
     ) {
         if (routes.isEmpty()) {
             throw IllegalArgumentException("Routes to refresh should not be empty")
@@ -39,18 +38,24 @@ internal class ImmediateRouteRefreshController(
             }
 
             callback(result)
-            result.fold(
-                { logW("Route refresh on-demand error: $it", RouteRefreshLog.LOG_CATEGORY) },
-                {
-                    attemptListener.onRoutesRefreshAttemptFinished(it)
-                    if (it.anySuccess()) {
+            when (result) {
+                is RoutesRefresherExecutorResult.ReplacedByNewer -> {
+                    logW(
+                        "Route refresh on-demand error: " +
+                            "request is skipped as a newer one is available",
+                        RouteRefreshLog.LOG_CATEGORY
+                    )
+                }
+                is RoutesRefresherExecutorResult.Finished -> {
+                    attemptListener.onRoutesRefreshAttemptFinished(result.value)
+                    if (result.value.anySuccess()) {
                         stateHolder.onSuccess()
                     } else {
                         stateHolder.onFailure(null)
                     }
-                    listener.onRoutesRefreshed(it)
+                    listener.onRoutesRefreshed(result.value)
                 }
-            )
+            }
         }
     }
 

diff --git a/...re/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt b/...re/src/main/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshController.kt
@@ -141,28 +141,40 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
                 }
             }
         )
-        routeRefresherResult.fold(
-            { logW("Planned route refresh error: $it", RouteRefreshLog.LOG_CATEGORY) },
-            {
-                attemptListener.onRoutesRefreshAttemptFinished(it)
-                if (it.anySuccess()) {
+        when (routeRefresherResult) {
+            is RoutesRefresherExecutorResult.ReplacedByNewer -> {
+                logW(
+                    "Planned route refresh error: " +
+                        "request is skipped as a newer one is available",
+                    RouteRefreshLog.LOG_CATEGORY
+                )
+            }
+            is RoutesRefresherExecutorResult.Finished -> {
+                attemptListener.onRoutesRefreshAttemptFinished(routeRefresherResult.value)
+                if (routeRefresherResult.value.anySuccess()) {
                     stateHolder.onSuccess()
-                    listener.onRoutesRefreshed(it)
-                    val refreshedRoutes = listOf(it.primaryRouteRefresherResult.route) +
-                        it.alternativesRouteRefresherResults.map { it.route }
+                    listener.onRoutesRefreshed(routeRefresherResult.value)
+                    val refreshedRoutes = listOf(
+                        routeRefresherResult.value.primaryRouteRefresherResult.route
+                    ) + routeRefresherResult.value.alternativesRouteRefresherResults.map {
+                        it.route
+                    }
                     routesToRefresh = refreshedRoutes
                     scheduleNewUpdate(refreshedRoutes, false)
                 } else {
-                    if (it.anyRequestFailed() && retryStrategy.shouldRetry()) {
+                    if (
+                        routeRefresherResult.value.anyRequestFailed() &&
+                        retryStrategy.shouldRetry()
+                    ) {
                         scheduleUpdateRetry(routes, shouldNotifyOnStart = false)
                     } else {
                         stateHolder.onFailure(null)
-                        listener.onRoutesRefreshed(it)
+                        listener.onRoutesRefreshed(routeRefresherResult.value)
                         scheduleNewUpdate(routes, false)
                     }
                 }
             }
-        )
+        }
     }
 
     private fun recreateScope() {

diff --git a/...tion-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefreshController.kt b/...tion-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefreshController.kt
@@ -63,7 +63,7 @@ class RouteRefreshController internal constructor(
         }
         plannedRouteRefreshController.pause()
         immediateRouteRefreshController.requestRoutesRefresh(routes) {
-            if (it.isValue) {
+            if (it is RoutesRefresherExecutorResult.Finished) {
                 plannedRouteRefreshController.resume()
             }
         }

diff --git a/...tion-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefresherExecutor.kt b/...tion-core/src/main/java/com/mapbox/navigation/core/routerefresh/RouteRefresherExecutor.kt
@@ -1,17 +1,22 @@
 package com.mapbox.navigation.core.routerefresh
 
-import com.mapbox.bindgen.Expected
-import com.mapbox.bindgen.ExpectedFactory
 import com.mapbox.navigation.base.route.NavigationRoute
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlin.coroutines.resume
 
+internal sealed class RoutesRefresherExecutorResult {
+
+    internal data class Finished(val value: RoutesRefresherResult) : RoutesRefresherExecutorResult()
+
+    internal object ReplacedByNewer : RoutesRefresherExecutorResult()
+}
+
 private data class QueuedRequest(
     val routes: List<NavigationRoute>,
     val startCallback: () -> Unit,
-    val finishCallback: (Expected<String, RoutesRefresherResult>) -> Unit,
+    val finishCallback: (RoutesRefresherExecutorResult) -> Unit,
 )
 
 internal class RouteRefresherExecutor(
@@ -26,7 +31,7 @@ internal class RouteRefresherExecutor(
     suspend fun executeRoutesRefresh(
         routes: List<NavigationRoute>,
         startCallback: () -> Unit,
-    ): Expected<String, RoutesRefresherResult> = suspendCancellableCoroutine { cont ->
+    ): RoutesRefresherExecutorResult = suspendCancellableCoroutine { cont ->
         cont.invokeOnCancellation {
             currentRequest = null
             queuedRequest = null
@@ -39,10 +44,10 @@ internal class RouteRefresherExecutor(
     private fun executeRoutesRefresh(
         routes: List<NavigationRoute>,
         startCallback: () -> Unit,
-        finishCallback: (Expected<String, RoutesRefresherResult>) -> Unit,
+        finishCallback: (RoutesRefresherExecutorResult) -> Unit,
     ) {
         queuedRequest?.finishCallback?.invoke(
-            ExpectedFactory.createError("Skipping request as a newer one is queued.")
+            RoutesRefresherExecutorResult.ReplacedByNewer
         )
         queuedRequest = QueuedRequest(routes, startCallback, finishCallback)
         runQueue()
@@ -60,7 +65,7 @@ internal class RouteRefresherExecutor(
                     currentRequest = null
                 }
                 runQueue()
-                localCurrentRequest.finishCallback(ExpectedFactory.createValue(result))
+                localCurrentRequest.finishCallback(RoutesRefresherExecutorResult.Finished(result))
             }
         }
     }

diff --git a/.../test/java/com/mapbox/navigation/core/routerefresh/ImmediateRouteRefreshControllerTest.kt b/.../test/java/com/mapbox/navigation/core/routerefresh/ImmediateRouteRefreshControllerTest.kt
@@ -1,7 +1,5 @@
 package com.mapbox.navigation.core.routerefresh
 
-import com.mapbox.bindgen.Expected
-import com.mapbox.bindgen.ExpectedFactory
 import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
 import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.testing.LoggingFrontendTestRule
@@ -34,7 +32,7 @@ class ImmediateRouteRefreshControllerTest {
     private val attemptListener = mockk<RoutesRefreshAttemptListener>(relaxed = true)
     private val listener = mockk<RouteRefresherListener>(relaxed = true)
     private val clientCallback =
-        mockk<(Expected<String, RoutesRefresherResult>) -> Unit>(relaxed = true)
+        mockk<(RoutesRefresherExecutorResult) -> Unit>(relaxed = true)
     private val routes = listOf<NavigationRoute>(mockk())
 
     private val sut = ImmediateRouteRefreshController(
@@ -74,14 +72,18 @@ class ImmediateRouteRefreshControllerTest {
         val result = mockk<RoutesRefresherResult> { every { anySuccess() } returns true }
         coEvery {
             routeRefresherExecutor.executeRoutesRefresh(any(), any())
-        } returns ExpectedFactory.createValue(result)
+        } returns RoutesRefresherExecutorResult.Finished(result)
 
         sut.requestRoutesRefresh(routes, clientCallback)
 
         verify(exactly = 1) { attemptListener.onRoutesRefreshAttemptFinished(result) }
         verify(exactly = 1) { stateHolder.onSuccess() }
         verify(exactly = 1) { listener.onRoutesRefreshed(result) }
-        verify(exactly = 1) { clientCallback(match { it.value == result }) }
+        verify(exactly = 1) {
+            clientCallback(
+                match { (it as RoutesRefresherExecutorResult.Finished).value == result }
+            )
+        }
         verify(exactly = 0) { stateHolder.onCancel() }
     }
 
@@ -93,21 +95,22 @@ class ImmediateRouteRefreshControllerTest {
         }
         coEvery {
             routeRefresherExecutor.executeRoutesRefresh(any(), any())
-        } returns ExpectedFactory.createValue(result)
+        } returns RoutesRefresherExecutorResult.Finished(result)
 
         sut.requestRoutesRefresh(routes, clientCallback)
 
         verify(exactly = 1) { attemptListener.onRoutesRefreshAttemptFinished(result) }
         verify(exactly = 1) { stateHolder.onFailure(null) }
-        verify(exactly = 1) { clientCallback(match { it.value == result }) }
+        verify(exactly = 1) {
+            clientCallback(match { (it as RoutesRefresherExecutorResult.Finished).value == result })
+        }
         verify(exactly = 1) { listener.onRoutesRefreshed(result) }
         verify(exactly = 0) { stateHolder.onCancel() }
     }
 
     @Test
     fun routesRefreshFinishedWithError() = coroutineRule.runBlockingTest {
-        val error: Expected<String, RoutesRefresherResult> =
-            ExpectedFactory.createError("Some error")
+        val error = RoutesRefresherExecutorResult.ReplacedByNewer
         coEvery {
             routeRefresherExecutor.executeRoutesRefresh(any(), any())
         } returns error
@@ -124,7 +127,7 @@ class ImmediateRouteRefreshControllerTest {
         verify(exactly = 1) { clientCallback.invoke(error) }
         verify(exactly = 1) {
             logger.logW(
-                "Route refresh on-demand error: Some error",
+                "Route refresh on-demand error: request is skipped as a newer one is available",
                 "RouteRefreshController"
             )
         }
@@ -169,7 +172,7 @@ class ImmediateRouteRefreshControllerTest {
 
         coEvery {
             routeRefresherExecutor.executeRoutesRefresh(any(), any())
-        } returns ExpectedFactory.createError("some error")
+        } returns RoutesRefresherExecutorResult.ReplacedByNewer
         sut.requestRoutesRefresh(routes, clientCallback)
 
         coVerify(exactly = 1) {

diff --git a/...rc/test/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshControllerTest.kt b/...rc/test/java/com/mapbox/navigation/core/routerefresh/PlannedRouteRefreshControllerTest.kt
@@ -1,7 +1,5 @@
 package com.mapbox.navigation.core.routerefresh
 
-import com.mapbox.bindgen.Expected
-import com.mapbox.bindgen.ExpectedFactory
 import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
 import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.core.internal.utils.CoroutineUtils
@@ -529,7 +527,7 @@ class PlannedRouteRefreshControllerTest {
 
         sut.startRoutesRefreshing(routes)
         clearMocks(retryStrategy, answers = false)
-        finishRequest(ExpectedFactory.createError("Some error"))
+        finishRequest(RoutesRefresherExecutorResult.ReplacedByNewer)
 
         verify(exactly = 0) {
             attemptListener.onRoutesRefreshAttemptFinished(any())
@@ -544,7 +542,10 @@ class PlannedRouteRefreshControllerTest {
             executor.executeRoutesRefresh(any(), any())
         }
         verify(exactly = 1) {
-            logger.logW("Planned route refresh error: Some error", "RouteRefreshController")
+            logger.logW(
+                "Planned route refresh error: request is skipped as a newer one is available",
+                "RouteRefreshController"
+            )
         }
     }
 
@@ -556,10 +557,10 @@ class PlannedRouteRefreshControllerTest {
     }
 
     private suspend fun finishRequest(result: RoutesRefresherResult) {
-        finishRequest(ExpectedFactory.createValue(result))
+        finishRequest(RoutesRefresherExecutorResult.Finished(result))
     }
 
-    private suspend fun finishRequest(result: Expected<String, RoutesRefresherResult>) {
+    private suspend fun finishRequest(result: RoutesRefresherExecutorResult) {
         coEvery {
             executor.executeRoutesRefresh(any(), any())
         } returns result

diff --git a/...-core/src/test/java/com/mapbox/navigation/core/routerefresh/RouteRefreshControllerTest.kt b/...-core/src/test/java/com/mapbox/navigation/core/routerefresh/RouteRefreshControllerTest.kt
@@ -1,7 +1,5 @@
 package com.mapbox.navigation.core.routerefresh
 
-import com.mapbox.bindgen.Expected
-import com.mapbox.bindgen.ExpectedFactory
 import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
 import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.core.RoutesInvalidatedObserver
@@ -174,7 +172,7 @@ class RouteRefreshControllerTest {
     @Test
     fun requestImmediateRouteRefreshWithNonEmptySuccess() {
         val routes = listOf<NavigationRoute>(mockk())
-        val callback = slot<(Expected<String, RoutesRefresherResult>) -> Unit>()
+        val callback = slot<(RoutesRefresherExecutorResult) -> Unit>()
         every { plannedRouteRefreshController.routesToRefresh } returns routes
 
         sut.requestImmediateRouteRefresh()
@@ -183,7 +181,7 @@ class RouteRefreshControllerTest {
             immediateRouteRefreshController.requestRoutesRefresh(routes, capture(callback))
         }
         callback.captured(
-            ExpectedFactory.createValue(
+            RoutesRefresherExecutorResult.Finished(
                 mockk { every { anySuccess() } returns true }
             )
         )
@@ -195,7 +193,7 @@ class RouteRefreshControllerTest {
     @Test
     fun requestImmediateRouteRefreshWithNonEmptyFailure() {
         val routes = listOf<NavigationRoute>(mockk())
-        val callback = slot<(Expected<String, RoutesRefresherResult>) -> Unit>()
+        val callback = slot<(RoutesRefresherExecutorResult) -> Unit>()
         every { plannedRouteRefreshController.routesToRefresh } returns routes
 
         sut.requestImmediateRouteRefresh()
@@ -204,7 +202,7 @@ class RouteRefreshControllerTest {
             immediateRouteRefreshController.requestRoutesRefresh(routes, capture(callback))
         }
         callback.captured(
-            ExpectedFactory.createValue(
+            RoutesRefresherExecutorResult.Finished(
                 mockk {
                     every { anySuccess() } returns false
                 }
@@ -218,15 +216,15 @@ class RouteRefreshControllerTest {
     @Test
     fun requestImmediateRouteRefreshWithNonEmptyError() {
         val routes = listOf<NavigationRoute>(mockk())
-        val callback = slot<(Expected<String, RoutesRefresherResult>) -> Unit>()
+        val callback = slot<(RoutesRefresherExecutorResult) -> Unit>()
         every { plannedRouteRefreshController.routesToRefresh } returns routes
 
         sut.requestImmediateRouteRefresh()
 
         verify(exactly = 1) {
             immediateRouteRefreshController.requestRoutesRefresh(routes, capture(callback))
         }
-        callback.captured(ExpectedFactory.createError("error"))
+        callback.captured(RoutesRefresherExecutorResult.ReplacedByNewer)
         verify(exactly = 0) {
             plannedRouteRefreshController.resume()
         }