Remove location permission from starter options

Author: kmadsen

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

@@ -17,8 +17,10 @@ import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.collect
+import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
+import kotlin.jvm.Throws
 
 /**
  * This makes it simpler to start a MapboxNavigation trip session. It can be used to enable replay
@@ -33,6 +35,7 @@ import kotlinx.coroutines.launch
 class MapboxTripStarter internal constructor() : MapboxNavigationObserver {
 
     private val optionsFlow = MutableStateFlow(MapboxTripStarterOptions.Builder().build())
+    private val stateFlow = MutableStateFlow(MapboxTripStarterState())
     private var replayRouteTripSession: ReplayRouteSession? = null
     private var mapboxNavigation: MapboxNavigation? = null
 
@@ -53,12 +56,12 @@ class MapboxTripStarter internal constructor() : MapboxNavigationObserver {
         // Initialize the options to be aware of the location permissions
         val context = mapboxNavigation.navigationOptions.applicationContext
         val granted = PermissionsManager.areLocationPermissionsGranted(context)
-        optionsFlow.update { it.toBuilder().isLocationPermissionGranted(granted).build() }
+        stateFlow.update { it.copy(isLocationPermissionGranted = granted) }
 
         // Observe any changes to the options
         coroutineScope.launch {
-            optionsFlow.collect { options ->
-                onStarterOptionsChanged(mapboxNavigation, options)
+            combine(optionsFlow, stateFlow, ::Pair).collect {
+                onStarterOptionsChanged(mapboxNavigation, it.first, it.second)
             }
         }
     }
@@ -85,33 +88,37 @@ class MapboxTripStarter internal constructor() : MapboxNavigationObserver {
         this.optionsFlow.value = options
     }
 
-    private fun onStarterOptionsChanged(
-        mapboxNavigation: MapboxNavigation,
-        options: MapboxTripStarterOptions
-    ) {
-        checkOptions(options)
-        if (options.tripType == MAPBOX_TRIP_STARTER_REPLAY_ROUTE) {
-            onReplayTripEnabled(mapboxNavigation)
-        } else if (options.tripType == MAPBOX_TRIP_STARTER_FOLLOW_DEVICE &&
-            options.isLocationPermissionGranted
-        ) {
-            onTripSessionEnabled(mapboxNavigation)
-        } else {
-            onTripDisabled(mapboxNavigation)
-        }
+    /**
+     * Get the location permission state. This may not reflect the actual permissions. Used for
+     * verifying the tests and will likely be removed.
+     */
+    @VisibleForTesting
+    internal fun getLocationPermissionGranted(): Boolean =
+        stateFlow.value.isLocationPermissionGranted
+
+    /**
+     * Set the location permission state. [MAPBOX_TRIP_STARTER_FOLLOW_DEVICE] will not work unless
+     * location permissions have been granted. If the location permissions are not granted and
+     * this is set to true, an error will be thrown.
+     */
+    @Throws(IllegalStateException::class)
+    fun setLocationPermissionGranted(granted: Boolean) {
+        val nextState = stateFlow.value.copy(isLocationPermissionGranted = granted)
+        checkState(nextState)
+        stateFlow.value = nextState
     }
 
     /**
      * Throws an error if location permissions are set to true but the location permissions are
      * not actually granted.
      */
-    private fun checkOptions(options: MapboxTripStarterOptions) {
+    private fun checkState(state: MapboxTripStarterState) {
         val mapboxNavigation = this.mapboxNavigation
         checkNotNull(mapboxNavigation) {
             "MapboxTripStarter cannot be used while MapboxNavigation is detached."
         }
         val context = mapboxNavigation.navigationOptions.applicationContext
-        val granted = options.isLocationPermissionGranted
+        val granted = state.isLocationPermissionGranted
         if (granted && !PermissionsManager.areLocationPermissionsGranted(context)) {
             error(
                 "updateLocationPermissions can only be set to true when location permissions" +
@@ -120,6 +127,22 @@ class MapboxTripStarter internal constructor() : MapboxNavigationObserver {
         }
     }
 
+    private fun onStarterOptionsChanged(
+        mapboxNavigation: MapboxNavigation,
+        options: MapboxTripStarterOptions,
+        state: MapboxTripStarterState,
+    ) {
+        if (options.tripType == MAPBOX_TRIP_STARTER_REPLAY_ROUTE) {
+            onReplayTripEnabled(mapboxNavigation)
+        } else if (options.tripType == MAPBOX_TRIP_STARTER_FOLLOW_DEVICE &&
+            state.isLocationPermissionGranted
+        ) {
+            onTripSessionEnabled(mapboxNavigation)
+        } else {
+            onTripDisabled(mapboxNavigation)
+        }
+    }
+
     private fun onReplayTripEnabled(mapboxNavigation: MapboxNavigation) {
         if (mapboxNavigation.getTripSessionState() != TripSessionState.STARTED ||
             !mapboxNavigation.isReplayEnabled()

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

@@ -7,32 +7,62 @@ import com.mapbox.navigation.core.replay.route.ReplayRouteSessionOptions
  * Defines options for the automatic trip starter [MapboxTripStarter].
  *
  * @param tripType Specify the type of trip to start.
- * @param isLocationPermissionGranted True if location permissions are granted.
  * @param replayRouteSessionOptions options to use when isReplayRouteEnabled is true
  */
 @ExperimentalPreviewMapboxNavigationAPI
 class MapboxTripStarterOptions private constructor(
     @MapboxTripStarterExtra.Type
     val tripType: String,
-    val isLocationPermissionGranted: Boolean,
     val replayRouteSessionOptions: ReplayRouteSessionOptions,
 ) {
     /**
      * @return builder matching the one used to create this instance
      */
     fun toBuilder(): Builder = Builder()
         .tripType(tripType)
-        .isLocationPermissionGranted(isLocationPermissionGranted)
         .replayRouteSessionOptions(replayRouteSessionOptions)
 
+    /**
+     * Regenerate whenever a change is made
+     */
+    override fun toString(): String {
+        return "MapboxTripStarterOptions(" +
+            "tripType='$tripType', " +
+            "replayRouteSessionOptions=$replayRouteSessionOptions" +
+            ")"
+    }
+
+    /**
+     * Regenerate whenever a change is made
+     */
+    override fun equals(other: Any?): Boolean {
+        if (this === other) return true
+        if (javaClass != other?.javaClass) return false
+
+        other as MapboxTripStarterOptions
+
+        if (tripType != other.tripType) return false
+        if (replayRouteSessionOptions != other.replayRouteSessionOptions) return false
+
+        return true
+    }
+
+    /**
+     * Regenerate whenever a change is made
+     */
+    override fun hashCode(): Int {
+        var result = tripType.hashCode()
+        result = 31 * result + replayRouteSessionOptions.hashCode()
+        return result
+    }
+
     /**
      * Build your [MapboxTripStarterOptions].
      */
     class Builder {
 
         @MapboxTripStarterExtra.Type
         private var tripType: String = MapboxTripStarterExtra.MAPBOX_TRIP_STARTER_FOLLOW_DEVICE
-        private var isLocationPermissionGranted = false
         private var replayRouteSessionOptions: ReplayRouteSessionOptions? = null
 
         /**
@@ -42,13 +72,6 @@ class MapboxTripStarterOptions private constructor(
             this.tripType = tripType
         }
 
-        /**
-         * True if location permissions are granted.
-         */
-        fun isLocationPermissionGranted(isLocationPermissionGranted: Boolean) = apply {
-            this.isLocationPermissionGranted = isLocationPermissionGranted
-        }
-
         /**
          * True if navigation routes will be simulated.
          */
@@ -62,7 +85,6 @@ class MapboxTripStarterOptions private constructor(
         fun build(): MapboxTripStarterOptions {
             return MapboxTripStarterOptions(
                 tripType = tripType,
-                isLocationPermissionGranted = isLocationPermissionGranted,
                 replayRouteSessionOptions = replayRouteSessionOptions
                     ?: ReplayRouteSessionOptions.Builder().build()
             )

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

@@ -0,0 +1,11 @@
+package com.mapbox.navigation.core.trip
+
+/**
+ * State used by [MapboxTripStarter]. This class is internal because location permissions is the
+ * only property and it may be removed in favor of better location permission core classes.
+ *
+ * @param isLocationPermissionGranted True if location permissions are granted.
+ */
+internal data class MapboxTripStarterState(
+    val isLocationPermissionGranted: Boolean = false
+)

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

@@ -0,0 +1,19 @@
+package com.mapbox.navigation.core.trip
+
+import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI
+import com.mapbox.navigation.testing.BuilderTest
+import io.mockk.mockk
+
+@OptIn(ExperimentalPreviewMapboxNavigationAPI::class)
+class MapboxTripStarterOptionsTest :
+    BuilderTest<MapboxTripStarterOptions, MapboxTripStarterOptions.Builder>() {
+    override fun getImplementationClass() = MapboxTripStarterOptions::class
+
+    override fun getFilledUpBuilder() = MapboxTripStarterOptions.Builder()
+        .tripType("Not a real type")
+        .replayRouteSessionOptions(mockk())
+
+    override fun trigger() {
+        // only used to trigger JUnit4 to run this class if all test cases come from the parent
+    }
+}

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

@@ -39,11 +39,11 @@ class MapboxTripStarterTest {
     fun `onAttached will update location permission options`() {
         every { PermissionsManager.areLocationPermissionsGranted(any()) } returns true
 
-        val initialOptions = sut.getOptions()
+        val initialState = sut.getLocationPermissionGranted()
         sut.onAttached(mockk(relaxed = true))
 
-        assertFalse(initialOptions.isLocationPermissionGranted)
-        assertTrue(sut.getOptions().isLocationPermissionGranted)
+        assertFalse(initialState)
+        assertTrue(sut.getLocationPermissionGranted())
     }
 
     @Test
@@ -73,7 +73,7 @@ class MapboxTripStarterTest {
         val mapboxNavigation = mockMapboxNavigation()
         sut.onAttached(mapboxNavigation)
         every { PermissionsManager.areLocationPermissionsGranted(any()) } returns true
-        sut.update { it.isLocationPermissionGranted(true) }
+        sut.setLocationPermissionGranted(true)
 
         verify(exactly = 1) { mapboxNavigation.startTripSession() }
     }
@@ -84,7 +84,7 @@ class MapboxTripStarterTest {
 
         val mapboxNavigation = mockMapboxNavigation()
         sut.onAttached(mapboxNavigation)
-        sut.update { it.isLocationPermissionGranted(true) }
+        sut.setLocationPermissionGranted(true)
 
         // Clean up coroutines early so that the error is caught by the tests.
         assertThrows(Exception::class.java) {
@@ -112,7 +112,7 @@ class MapboxTripStarterTest {
         sut.onAttached(mapboxNavigation)
         sut.update { it.tripType(MapboxTripStarterExtra.MAPBOX_TRIP_STARTER_REPLAY_ROUTE) }
         every { PermissionsManager.areLocationPermissionsGranted(any()) } returns true
-        sut.update { it.isLocationPermissionGranted(true) }
+        sut.setLocationPermissionGranted(true)
 
         verify(exactly = 1) { mapboxNavigation.startReplayTripSession() }
     }

libnavui-app/src/main/java/com/mapbox/navigation/ui/app/internal/controller/TripSessionStarterStateController.kt

@@ -35,9 +35,7 @@ class TripSessionStarterStateController(store: Store) : StateController() {
     ) {
         when (action) {
             is TripSessionStarterAction.OnLocationPermission -> {
-                tripStarter.update {
-                    it.isLocationPermissionGranted(action.granted)
-                }
+                tripStarter.setLocationPermissionGranted(action.granted)
             }
             TripSessionStarterAction.EnableReplayTripSession -> {
                 tripStarter.update {

libnavui-app/src/test/java/com/mapbox/navigation/ui/app/internal/controller/TripSessionStarterStateControllerTest.kt

@@ -6,11 +6,13 @@ import com.mapbox.navigation.core.trip.MapboxTripStarterExtra.MAPBOX_TRIP_STARTE
 import com.mapbox.navigation.core.trip.MapboxTripStarterExtra.MAPBOX_TRIP_STARTER_REPLAY_ROUTE
 import com.mapbox.navigation.core.trip.MapboxTripStarterOptions
 import com.mapbox.navigation.testing.MainCoroutineRule
+import com.mapbox.navigation.ui.app.internal.Store
 import com.mapbox.navigation.ui.app.internal.tripsession.TripSessionStarterAction
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkObject
 import io.mockk.unmockkAll
+import io.mockk.verify
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import org.junit.After
 import org.junit.Assert.assertEquals
@@ -27,6 +29,7 @@ class TripSessionStarterStateControllerTest {
 
     private lateinit var mapboxTripStarter: MapboxTripStarter
     private lateinit var sut: TripSessionStarterStateController
+    private val store: Store = mockk(relaxed = true)
     private val updateSlot = mutableListOf<MapboxTripStarterOptions>()
 
     @Before
@@ -37,19 +40,24 @@ class TripSessionStarterStateControllerTest {
 
         mockkObject(MapboxTripStarter.Companion)
         every { MapboxTripStarter.getRegisteredInstance() } returns mapboxTripStarter
-        sut = TripSessionStarterStateController()
+        sut = TripSessionStarterStateController(store)
     }
 
     @After
     fun teardown() {
         unmockkAll()
     }
 
+    @Test
+    fun `constructor will register to the store`() {
+        verify(exactly = 1) { store.register(sut) }
+    }
+
     @Test
     fun `on OnLocationPermission action should update TripSessionStarterState`() {
         sut.process(mockk(), TripSessionStarterAction.OnLocationPermission(true))
 
-        assertTrue(updateSlot[0].isLocationPermissionGranted)
+        verify(exactly = 1) { mapboxTripStarter.setLocationPermissionGranted(true) }
     }
 
     @Test