Fix crash due to missing access token; delete CredentialsManager.kt (#…

…365) * Fix token not found error * Delete CredentialsManager.kt * Make ResourceOptionsManager.kt as a data class * Fix the MapViewCustomizationActivity and update docs. * Update doc. * Update doc * Check token in an appropriate time * Fix review comments * Update sdk/src/main/java/com/mapbox/maps/MapInitOptions.kt Co-authored-by: Peng Liu <peng.liu@mapbox.com> * Add test cases * Remove redundant codes. * Update sdk/src/test/java/com/mapbox/maps/ResourceAttributeParserTest.kt Co-authored-by: Peng Liu <peng.liu@mapbox.com> * Update codes Co-authored-by: Peng Liu <peng.liu@mapbox.com>
mapbox · May 28, 2021 · 7c46fbc · 7c46fbc
1 parent 936ccd3
commit 7c46fbc
Show file tree

Hide file tree

Showing 12 changed files with 199 additions and 165 deletions.
diff --git a/app/src/main/java/com/mapbox/maps/testapp/examples/MapViewCustomizationActivity.kt b/app/src/main/java/com/mapbox/maps/testapp/examples/MapViewCustomizationActivity.kt
@@ -21,17 +21,17 @@ class MapViewCustomizationActivity : AppCompatActivity() {
 
   override fun onCreate(savedInstanceState: Bundle?) {
     super.onCreate(savedInstanceState)
-    // Create  a custom CredentialsManager with a token and set it to CredentialsManager.shared, so that all MapViews created with default config will apply this token.
-    CredentialsManager.default.setAccessToken(getString(R.string.mapbox_access_token))
 
-    // Create a custom ResourceOptionsManager with customised tile store and tile size, so that all MapViews created with default config will apply these settings.
-    ResourceOptionsManager.getDefault(this).update {
+    // Set the application-scoped ResourceOptionsManager with customised token, tile store and tile size
+    // so that all MapViews created with default config will apply these settings.
+    ResourceOptionsManager.getDefault(this, getString(R.string.mapbox_access_token)).update {
       tileStore(TileStore.getInstance())
-      tileStoreUsageMode(TileStoreUsageMode.READ_AND_UPDATE)
+      tileStoreUsageMode(TileStoreUsageMode.READ_ONLY)
       cacheSize(75_000L)
     }
 
     setContentView(R.layout.activity_map_view_customization)
+
     // all options provided in xml file - so we just load style
     mapView.getMapboxMap().loadStyleUri(Style.DARK)
     configureMapViewFromCode()
@@ -107,8 +107,7 @@ class MapViewCustomizationActivity : AppCompatActivity() {
     super.onDestroy()
     mapView.onDestroy()
     customMapView.onDestroy()
-    // Restore the default resource settings, otherwise it will affect other activities
-    ResourceOptionsManager.getDefault(this).resourceOptions =
-      ResourceOptions.Builder().applyDefaultParams(this).build()
+    // Restore the default application-scoped resource settings, otherwise it will affect other activities
+    ResourceOptionsManager.destroyDefault()
   }
 }
diff --git a/sdk/src/main/java/com/mapbox/maps/CredentialsManager.kt b/sdk/src/main/java/com/mapbox/maps/CredentialsManager.kt
diff --git a/sdk/src/main/java/com/mapbox/maps/MapInitOptions.kt b/sdk/src/main/java/com/mapbox/maps/MapInitOptions.kt
@@ -38,7 +38,7 @@ data class MapInitOptions constructor(
    */
   companion object {
     /**
-     * Get a default [ResourceOptions] with token from default [CredentialsManager]
+     * Get a default [ResourceOptions] instance.
      * @property context the context of the application.
      */
     fun getDefaultResourceOptions(context: Context): ResourceOptions =
@@ -91,10 +91,14 @@ fun MapOptions.Builder.applyDefaultParams(context: Context): MapOptions.Builder
 }
 
 /**
- * Get a default [ResourceOptions.Builder] with token from default [CredentialsManager]
- * @property context the context of the application.
+ * Get a default [ResourceOptions.Builder] with Mapbox pre-defined options, and
+ * with the access token taken from the Android resources(if available).
+ *
+ * @param context the context of the application.
  */
-fun ResourceOptions.Builder.applyDefaultParams(context: Context): ResourceOptions.Builder = also {
+fun ResourceOptions.Builder.applyDefaultParams(
+  context: Context
+): ResourceOptions.Builder = also {
   // make sure that directory `/mapbox/maps` exists
   val databaseDirectoryPath = "${context.filesDir.absolutePath}/$DATABASE_PATH"
   val databaseDirectory = File(databaseDirectoryPath)
@@ -104,7 +108,13 @@ fun ResourceOptions.Builder.applyDefaultParams(context: Context): ResourceOption
     }
   }
 
-  accessToken(CredentialsManager.default.getAccessToken(context))
+  // check in the resources
+  val tokenResId = ResourceOptionsManager.getTokenResId(context)
+  // Apply the token from resources.
+  if (tokenResId != 0) {
+    accessToken(context.getString(tokenResId))
+  }
+
   cachePath("$databaseDirectoryPath/$DATABASE_NAME")
   cacheSize(DEFAULT_CACHE_SIZE) // 50 mb
 }
diff --git a/sdk/src/main/java/com/mapbox/maps/MapView.kt b/sdk/src/main/java/com/mapbox/maps/MapView.kt
@@ -110,8 +110,7 @@ open class MapView : FrameLayout, MapPluginProviderDelegate, MapControllable {
       val resourceOptions =
         ResourcesAttributeParser.parseResourcesOptions(
           context,
-          typedArray,
-          CredentialsManager.default
+          typedArray
         )
       val mapOptions =
         MapAttributeParser.parseMapOptions(typedArray, context.resources.displayMetrics.density)

diff --git a/sdk/src/main/java/com/mapbox/maps/ResourceOptionsManager.kt b/sdk/src/main/java/com/mapbox/maps/ResourceOptionsManager.kt
@@ -3,13 +3,13 @@ package com.mapbox.maps
 import android.content.Context
 
 /**
- * Convenience class that holds the default ResourceOptions.
- * `ResourceOptionsManager` could be created per `MapView` instance if given view should use some specific resource options.
- * Most common use-case when all `MapView`s should use same resource options could be handled by using `ResourceOptionsManager.getDefault(context: Context)` static object.
+ * Convenience class that manages a global `ResourceOptions`
+ * It's possible to create `ResourceOptionsManager` instances as you need them,
+ * however it's convenient to use the default object (`default`).
  *
  * @property resourceOptions the initial resource options.
  */
-class ResourceOptionsManager(
+data class ResourceOptionsManager(
   var resourceOptions: ResourceOptions
 ) {
 
@@ -24,20 +24,56 @@ class ResourceOptionsManager(
    * Static variables and methods.
    */
   companion object {
-    private lateinit var default: ResourceOptionsManager
+    private var default: ResourceOptionsManager? = null
+    internal fun getTokenResId(context: Context): Int = context.resources.getIdentifier(
+      MAPBOX_ACCESS_TOKEN_RESOURCE_NAME,
+      "string",
+      context.packageName
+    )
+
+    /**
+     * Convenience function to remove the default instance. Calling `getDefault` again will re-create the default instance.
+     */
+    fun destroyDefault() {
+      default = null
+    }
 
     /**
      * The default shared instance with default resource options.
+     *
+     * @param context the application context
+     * @defaultToken the default token will be set to the default [ResourceOptionsManager] instance.
+     * If set to null, will reuse the previously set one or search for an access token in the application resources for the first time of init.
+     * It will throw [MapboxConfigurationException] if there's no token found.
      */
     @Synchronized
-    fun getDefault(context: Context): ResourceOptionsManager {
-      if (!this::default.isInitialized) {
-        default = ResourceOptionsManager(
-          ResourceOptions.Builder().applyDefaultParams(context)
-            .build()
-        )
+    fun getDefault(context: Context, defaultToken: String? = null): ResourceOptionsManager {
+      // Apply the user setting token as the default token.
+      defaultToken?.let { token ->
+        default?.let {
+          // Reuse the config from previous default and only update the token
+          it.update { accessToken(token) }
+          return it
+        }
+        // Build a new ResourceOptionsManager with the provided token.
+        val builder = ResourceOptions.Builder().applyDefaultParams(context).accessToken(token)
+        return ResourceOptionsManager(builder.build()).also { default = it }
       }
-      return default
+
+      // Have a default instance created before.
+      default?.let {
+        return it
+      }
+
+      // No defaultToken provided, search in the resources to init default instance.
+      getTokenResId(context).also {
+        // Throw exception as no token could be found as default token.
+        if (it == 0) throw MapboxConfigurationException()
+      }
+
+      // Build a new ResourceOptions with token from the resources.
+      val builder = ResourceOptions.Builder().applyDefaultParams(context)
+      return ResourceOptionsManager(builder.build()).also { default = it }
     }
   }
 }
diff --git a/sdk/src/main/java/com/mapbox/maps/ResourcesAttributeParser.kt b/sdk/src/main/java/com/mapbox/maps/ResourcesAttributeParser.kt
@@ -16,17 +16,11 @@ internal object ResourcesAttributeParser {
    */
   fun parseResourcesOptions(
     context: Context,
-    typedArray: TypedArray,
-    credentialsManager: CredentialsManager
+    typedArray: TypedArray
   ): ResourceOptions {
-    val builder = ResourceOptionsManager.getDefault(context).resourceOptions.toBuilder()
 
-    if (typedArray.hasValue(R.styleable.mapbox_MapView_mapbox_resourcesAccessToken)) {
-      builder.accessToken(
-        typedArray.getString(R.styleable.mapbox_MapView_mapbox_resourcesAccessToken)
-          ?: credentialsManager.getAccessToken(context)
-      )
-    }
+    val token = typedArray.getString(R.styleable.mapbox_MapView_mapbox_resourcesAccessToken)
+    val builder = ResourceOptionsManager.getDefault(context, token).resourceOptions.toBuilder()
 
     if (typedArray.hasValue(R.styleable.mapbox_MapView_mapbox_resourcesBaseUrl)) {
       builder.baseURL(typedArray.getString(R.styleable.mapbox_MapView_mapbox_resourcesBaseUrl))

diff --git a/sdk/src/test/java/com/mapbox/maps/CredentialsManagerTest.kt b/sdk/src/test/java/com/mapbox/maps/CredentialsManagerTest.kt
diff --git a/sdk/src/test/java/com/mapbox/maps/MapControllerTest.kt b/sdk/src/test/java/com/mapbox/maps/MapControllerTest.kt
@@ -1,5 +1,6 @@
 package com.mapbox.maps
 
+import android.content.Context
 import android.graphics.Bitmap
 import android.view.MotionEvent
 import com.mapbox.common.ShadowLogger
@@ -16,6 +17,7 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
+import java.io.File
 
 @RunWith(RobolectricTestRunner::class)
 @Config(shadows = [ShadowLogger::class])
@@ -41,13 +43,23 @@ class MapControllerTest {
 
   private lateinit var mapController: MapController
 
+  private val context: Context = mockk(relaxUnitFun = true)
+
   @Before
   fun setUp() {
     mockkStatic(MapboxMapStaticInitializer::class)
     every { MapboxMapStaticInitializer.loadMapboxMapNativeLib() } just Runs
-
-    CredentialsManager.default.setAccessToken("foobar")
-
+    every { context.getString(-1) } returns "token"
+    every {
+      context.resources.getIdentifier(
+        "mapbox_access_token",
+        "string",
+        "com.mapbox.maps"
+      )
+    } returns -1
+    every { context.packageName } returns "com.mapbox.maps"
+    every { context.filesDir } returns File("foobar")
+    ResourceOptionsManager.getDefault(context).update { accessToken("foobar") }
     mapView = mockk(relaxUnitFun = true)
     val token = "pk.123"
     val resourceOptions = mockk<ResourceOptions>()

diff --git a/sdk/src/test/java/com/mapbox/maps/MapInitOptionsTest.kt b/sdk/src/test/java/com/mapbox/maps/MapInitOptionsTest.kt
@@ -25,7 +25,14 @@ class MapInitOptionsTest {
 
   @Before
   fun setUp() {
-    CredentialsManager.default.setAccessToken("token")
+    every { context.getString(-1) } returns "token"
+    every {
+      context.resources.getIdentifier(
+        "mapbox_access_token",
+        "string",
+        "com.mapbox.maps"
+      )
+    } returns -1
     every { context.packageName } returns "com.mapbox.maps"
     every { context.filesDir } returns File("foobar")
     every { context.resources.displayMetrics } returns displayMetrics
@@ -34,9 +41,7 @@ class MapInitOptionsTest {
 
   @After
   fun cleanUp() {
-    ResourceOptionsManager.getDefault(context).resourceOptions =
-      ResourceOptions.Builder().applyDefaultParams(context)
-        .build()
+    ResourceOptionsManager.destroyDefault()
   }
 
   @Test
@@ -45,15 +50,16 @@ class MapInitOptionsTest {
     assertEquals(true, MapInitOptions(context, textureView = true).textureView)
   }
 
+  @Test
   fun setInvalidToken() {
-    CredentialsManager.default.setAccessToken("")
-
+    ResourceOptionsManager.getDefault(context).update { accessToken("") }
     val mapboxMapOptions = MapInitOptions(context)
     assertEquals("", mapboxMapOptions.resourceOptions.accessToken)
   }
 
   @Test
   fun setValidToken() {
+    ResourceOptionsManager.getDefault(context).update { accessToken("token") }
     val mapboxMapOptions = MapInitOptions(context)
     assertEquals("token", mapboxMapOptions.resourceOptions.accessToken)
   }

diff --git a/sdk/src/test/java/com/mapbox/maps/MapViewTest.kt b/sdk/src/test/java/com/mapbox/maps/MapViewTest.kt
@@ -54,7 +54,7 @@ class MapViewTest {
     mockkObject(CameraAttributeParser)
     every { context.resources } returns resources
     every { resources.displayMetrics } returns displayMetrics
-    every { ResourcesAttributeParser.parseResourcesOptions(any(), any(), any()) } returns mockk()
+    every { ResourcesAttributeParser.parseResourcesOptions(any(), any()) } returns mockk()
     every { MapAttributeParser.parseMapOptions(any(), any()) } returns mockk()
     every { CameraAttributeParser.parseCameraOptions(any()) } returns mockk()
     every { typedArray.getInt(R.styleable.mapbox_MapView_mapbox_mapSurface, 0) } returns 1