PR fixes

mapbox · Aug 15, 2022 · d7bf2f9 · d7bf2f9
1 parent f585696
commit d7bf2f9
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 30 deletions.
diff --git a/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/CameraAnimationsExt.kt b/plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/CameraAnimationsExt.kt
@@ -27,7 +27,7 @@ private val emptyCancelable = Cancelable { }
  * @param cameraOptions The camera options to ease to
  * @param animationOptions Transition options (animation duration, listeners etc)
  *
- * @return [Cancelable] animator set object or empty cancelable object if associated map object was garbage collected.
+ * @return [Cancelable] animator set object or empty cancelable object if associated `MapView` was destroyed.
  */
 @JvmOverloads
 fun MapPluginExtensionsDelegate.easeTo(
@@ -45,7 +45,7 @@ fun MapPluginExtensionsDelegate.easeTo(
  * @param cameraOptions The camera options to fly to
  * @param animationOptions Transition options (animation duration, listeners etc)
  *
- * @return [Cancelable] animator set object or empty cancelable object if associated map object was garbage collected.
+ * @return [Cancelable] animator set object or empty cancelable object if associated `MapView` was destroyed.
  */
 @JvmOverloads
 fun MapPluginExtensionsDelegate.flyTo(
@@ -63,7 +63,7 @@ fun MapPluginExtensionsDelegate.flyTo(
  * @param pitch The amount to pitch by
  * @param animationOptions Transition options (animation duration, listeners etc)
  *
- * @return [Cancelable] animator set object or empty cancelable object if associated map object was garbage collected.
+ * @return [Cancelable] animator set object or empty cancelable object if associated `MapView` was destroyed.
  */
 @JvmOverloads
 fun MapPluginExtensionsDelegate.pitchBy(
@@ -82,7 +82,7 @@ fun MapPluginExtensionsDelegate.pitchBy(
  * @param screenCoordinate The optional focal point to scale on
  * @param animationOptions Transition options (animation duration, listeners etc)
  *
- * @return [Cancelable] animator set object or empty cancelable object if associated map object was garbage collected.
+ * @return [Cancelable] animator set object or empty cancelable object if associated `MapView` was destroyed.
  */
 @JvmOverloads
 fun MapPluginExtensionsDelegate.scaleBy(
@@ -101,7 +101,7 @@ fun MapPluginExtensionsDelegate.scaleBy(
  * @param screenCoordinate The screen coordinate distance to move by
  * @param animationOptions Transition options (animation duration, listeners etc)
  *
- * @return [Cancelable] animator set object or empty cancelable object if associated map object was garbage collected.
+ * @return [Cancelable] animator set object or empty cancelable object if associated `MapView` was destroyed.
  */
 @JvmOverloads
 fun MapPluginExtensionsDelegate.moveBy(
@@ -120,7 +120,7 @@ fun MapPluginExtensionsDelegate.moveBy(
  * @param second The second pointer to rotate on
  * @param animationOptions Transition options (animation duration, listeners etc)
  *
- * @return [Cancelable] animator set object or empty cancelable object if associated map object was garbage collected.
+ * @return [Cancelable] animator set object or empty cancelable object if associated `MapView` was destroyed.
  */
 @JvmOverloads
 fun MapPluginExtensionsDelegate.rotateBy(

diff --git a/sdk/src/main/java/com/mapbox/maps/MapController.kt b/sdk/src/main/java/com/mapbox/maps/MapController.kt
@@ -169,8 +169,6 @@ internal class MapController : MapPluginProviderDelegate, MapControllable {
       return
     }
     lifecycleState = LifecycleState.STATE_DESTROYED
-    mapboxMap.setCameraAnimationPlugin(null)
-    mapboxMap.setGesturesAnimationPlugin(null)
     pluginRegistry.onDestroy()
     nativeObserver.onDestroy()
     renderer.onDestroy()

diff --git a/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt b/sdk/src/main/java/com/mapbox/maps/MapboxMap.kt
@@ -1697,8 +1697,8 @@ class MapboxMap :
     return nativeMap.getDragCameraOptions(fromPoint, toPoint)
   }
 
-  internal fun setCameraAnimationPlugin(cameraAnimationsPlugin: CameraAnimationsPlugin?) {
-    if (cameraAnimationsPlugin == null || cameraAnimationsPlugin is CameraAnimationsPluginImpl) {
+  internal fun setCameraAnimationPlugin(cameraAnimationsPlugin: CameraAnimationsPlugin) {
+    if (cameraAnimationsPlugin is CameraAnimationsPluginImpl) {
       this.cameraAnimationsPlugin = cameraAnimationsPlugin
     } else {
       logW(TAG, "MapboxMap camera extension functions could work only with Mapbox developed plugin!")
@@ -1721,8 +1721,8 @@ class MapboxMap :
     return null
   }
 
-  internal fun setGesturesAnimationPlugin(gesturesPlugin: GesturesPlugin?) {
-    if (gesturesPlugin == null || gesturesPlugin is GesturesPluginImpl) {
+  internal fun setGesturesAnimationPlugin(gesturesPlugin: GesturesPlugin) {
+    if (gesturesPlugin is GesturesPluginImpl) {
       this.gesturesPlugin = gesturesPlugin
     } else {
       logW(TAG, "MapboxMap gestures extension functions could work only with Mapbox developed plugin!")
@@ -1746,6 +1746,8 @@ class MapboxMap :
   }
 
   internal fun onDestroy() {
+    cameraAnimationsPlugin = null
+    gesturesPlugin = null
     observers.forEach {
       nativeMap.unsubscribe(it)
     }

diff --git a/sdk/src/test/java/com/mapbox/maps/MapControllerTest.kt b/sdk/src/test/java/com/mapbox/maps/MapControllerTest.kt
@@ -165,14 +165,10 @@ class MapControllerTest {
     every { mockPluginRegistry.onDestroy() } just Runs
     every { mockNativeObserver.onDestroy() } just Runs
     every { mockRenderer.onDestroy() } just Runs
-    every { mockMapboxMap.setCameraAnimationPlugin(any()) } just Runs
-    every { mockMapboxMap.setGesturesAnimationPlugin(any()) } just Runs
 
     testMapController.onDestroy()
 
     verifySequence {
-      mockMapboxMap.setCameraAnimationPlugin(null)
-      mockMapboxMap.setGesturesAnimationPlugin(null)
       mockPluginRegistry.onDestroy()
       mockNativeObserver.onDestroy()
       mockRenderer.onDestroy()

diff --git a/sdk/src/test/java/com/mapbox/maps/MapboxMapTest.kt b/sdk/src/test/java/com/mapbox/maps/MapboxMapTest.kt
@@ -55,6 +55,15 @@ class MapboxMapTest {
     assertFalse(mapboxMap.isValid())
   }
 
+  @Test
+  fun onDestroyWithPlugins() {
+    mapboxMap.cameraAnimationsPlugin = mockk<CameraAnimationsPluginImpl>()
+    mapboxMap.gesturesPlugin = mockk<GesturesPluginImpl>()
+    mapboxMap.onDestroy()
+    assertTrue(mapboxMap.cameraAnimationsPlugin == null)
+    assertTrue(mapboxMap.gesturesPlugin == null)
+  }
+
   @Test
   fun loadStyleUri() {
     Shadows.shadowOf(Looper.getMainLooper()).pause()
@@ -976,13 +985,6 @@ class MapboxMapTest {
     unmockkStatic("com.mapbox.maps.MapboxLogger")
   }
 
-  @Test
-  fun setCameraAnimationsPluginNull() {
-    mapboxMap.setCameraAnimationPlugin(mockk<CameraAnimationsPluginImpl>())
-    mapboxMap.setCameraAnimationPlugin(null)
-    assertNull(mapboxMap.cameraAnimationsPlugin)
-  }
-
   @Test
   fun setGesturesPluginInvalid() {
     mockkStatic("com.mapbox.maps.MapboxLogger")
@@ -992,13 +994,6 @@ class MapboxMapTest {
     unmockkStatic("com.mapbox.maps.MapboxLogger")
   }
 
-  @Test
-  fun setGesturesPluginNull() {
-    mapboxMap.setGesturesAnimationPlugin(mockk<GesturesPluginImpl>())
-    mapboxMap.setGesturesAnimationPlugin(null)
-    assertNull(mapboxMap.gesturesPlugin)
-  }
-
   @Test
   fun cameraAnimationsPluginValid() {
     val animations = mockk<CameraAnimationsPlugin>(relaxed = true)