Revisit MapboxMap plugin related extension functions to not use singl…

…etons (#75)

* Move MapboxMap extension functions to sdk-base

* Move extension functions back to plugins

* Add tests

plugin-animation/src/main/java/com/mapbox/maps/plugin/animation/CameraAnimationsPluginImpl.kt

@@ -12,7 +12,6 @@ import com.mapbox.maps.EdgeInsets
 import com.mapbox.maps.ScreenCoordinate
 import com.mapbox.maps.plugin.animation.animator.*
 import com.mapbox.maps.plugin.delegates.*
-import java.lang.ref.WeakReference
 import java.util.concurrent.CopyOnWriteArraySet
 import kotlin.properties.Delegates
 
@@ -126,13 +125,6 @@ internal class CameraAnimationsPluginImpl : CameraAnimationsPlugin {
     cameraAnimationsFactory = CameraAnimatorsFactory(mapDelegateProvider)
   }
 
-  /**
-   * Called when the plugin is first added to the map.
-   */
-  override fun initialize() {
-    pluginRegistry[mapTransformDelegate] = WeakReference(this)
-  }
-
   /**
    * Called when the map is destroyed. Should be used to cleanup plugin resources for that map.
    * Cancel all running animations and cleanup all resources (registered animations, listeners).
@@ -148,9 +140,6 @@ internal class CameraAnimationsPluginImpl : CameraAnimationsPlugin {
     paddingListeners.clear()
     lifecycleListener.clear()
     animators.clear()
-    pluginRegistry.remove(mapTransformDelegate).also { weakRef ->
-      weakRef?.clear()
-    }
   }
 
   private fun performMapJump(cameraOptions: CameraOptions) {
@@ -817,8 +806,6 @@ internal class CameraAnimationsPluginImpl : CameraAnimationsPlugin {
    */
   companion object {
     private const val TAG = "Mbgl-CameraManager"
-    internal var pluginRegistry: HashMap<MapTransformDelegate, WeakReference<CameraAnimationsPluginImpl>> =
-      HashMap()
   }
 }
 
@@ -832,78 +819,77 @@ fun MapPluginProviderDelegate.getCameraAnimationsPlugin(): CameraAnimationsPlugi
 }
 
 /**
- * Extension easeTo() for [MapTransformDelegate]
+ * Extension easeTo() for [MapPluginExtensionsDelegate]
  * Ease the map camera to a given camera options and animation options.
  *
  * @param cameraOptions The camera options to ease to
  * @param animationOptions Transition options (animation duration, listeners etc)
  */
-fun MapTransformDelegate.easeTo(
+fun MapPluginExtensionsDelegate.easeTo(
   cameraOptions: CameraOptions,
   animationOptions: MapAnimationOptions? = null
-) = CameraAnimationsPluginImpl.pluginRegistry[this]?.get()?.easeTo(cameraOptions, animationOptions)
+) = cameraAnimationsPlugin { easeTo(cameraOptions, animationOptions) }
 
 /**
- * Extension flyTo() function for [MapTransformDelegate]
+ * Extension flyTo() function for [MapPluginExtensionsDelegate]
  * Fly the map camera to a given camera options.
  *
  * @param cameraOptions The camera options to fly to
  * @param animationOptions Transition options (animation duration, listeners etc)
  */
-fun MapTransformDelegate.flyTo(
+fun MapPluginExtensionsDelegate.flyTo(
   cameraOptions: CameraOptions,
   animationOptions: MapAnimationOptions? = null
-) = CameraAnimationsPluginImpl.pluginRegistry[this]?.get()?.flyTo(cameraOptions, animationOptions)
+) = cameraAnimationsPlugin { flyTo(cameraOptions, animationOptions) }
 
 /**
- * Extension pitchBy() function for [MapTransformDelegate]
+ * Extension pitchBy() function for [MapPluginExtensionsDelegate]
  * Pitch the map by with optional animation.
  *
  * @param pitch The amount to pitch by
  * @param animationOptions Transition options (animation duration, listeners etc)
  */
-fun MapTransformDelegate.pitchBy(
+fun MapPluginExtensionsDelegate.pitchBy(
   pitch: Double,
   animationOptions: MapAnimationOptions? = null
-) = CameraAnimationsPluginImpl.pluginRegistry[this]?.get()?.pitchBy(pitch, animationOptions)
+) = cameraAnimationsPlugin { pitchBy(pitch, animationOptions) }
 
 /**
- * Extension scaleBy() function for [MapTransformDelegate]
+ * Extension scaleBy() function for [MapPluginExtensionsDelegate]
  * Scale the map by with optional animation.
  *
  * @param amount The amount to scale by
  * @param screenCoordinate The optional focal point to scale on
  * @param animationOptions Transition options (animation duration, listeners etc)
  */
-fun MapTransformDelegate.scaleBy(
+fun MapPluginExtensionsDelegate.scaleBy(
   amount: Double,
   screenCoordinate: ScreenCoordinate?,
   animationOptions: MapAnimationOptions? = null
-) = CameraAnimationsPluginImpl.pluginRegistry[this]?.get()
-  ?.scaleBy(amount, screenCoordinate, animationOptions)
+) = cameraAnimationsPlugin { scaleBy(amount, screenCoordinate, animationOptions) }
 
 /**
- * Extension moveBy() function for [MapTransformDelegate]
+ * Extension moveBy() function for [MapPluginExtensionsDelegate]
  * Move the map by a given screen coordinate with optional animation.
  *
  * @param screenCoordinate The screen coordinate distance to move by
  * @param animationOptions Transition options (animation duration, listeners etc)
  */
-fun MapTransformDelegate.moveBy(
+fun MapPluginExtensionsDelegate.moveBy(
   screenCoordinate: ScreenCoordinate,
   animationOptions: MapAnimationOptions? = null
-) = CameraAnimationsPluginImpl.pluginRegistry[this]?.get()?.moveBy(screenCoordinate, animationOptions)
+) = cameraAnimationsPlugin { moveBy(screenCoordinate, animationOptions) }
 
 /**
- * Extension rotateBy() function for [MapTransformDelegate]
+ * Extension rotateBy() function for [MapPluginExtensionsDelegate]
  * Rotate the map by with optional animation.
  *
  * @param first The first pointer to rotate on
  * @param second The second pointer to rotate on
  * @param animationOptions Transition options (animation duration, listeners etc)
  */
-fun MapTransformDelegate.rotateBy(
+fun MapPluginExtensionsDelegate.rotateBy(
   first: ScreenCoordinate,
   second: ScreenCoordinate,
   animationOptions: MapAnimationOptions? = null
-) = CameraAnimationsPluginImpl.pluginRegistry[this]?.get()?.rotateBy(first, second, animationOptions)
+) = cameraAnimationsPlugin { rotateBy(first, second, animationOptions) }

plugin-gestures/src/main/java/com/mapbox/maps/plugin/gestures/GesturesPluginImpl.kt

@@ -23,11 +23,9 @@ import com.mapbox.maps.plugin.animation.MapAnimationOptions
 import com.mapbox.maps.plugin.animation.MapAnimationOptions.Companion.mapAnimationOptions
 import com.mapbox.maps.plugin.animation.MapAnimationOwnerRegistry
 import com.mapbox.maps.plugin.delegates.*
-import com.mapbox.maps.plugin.gestures.GesturesPluginImpl.Companion.gesturesPlugin
 import com.mapbox.maps.plugin.gestures.generated.GesturesAttributeParser
 import com.mapbox.maps.plugin.gestures.generated.GesturesSettings
 import com.mapbox.maps.plugin.gestures.generated.GesturesSettingsBase
-import java.lang.ref.WeakReference
 import java.util.*
 import java.util.concurrent.CopyOnWriteArrayList
 import kotlin.math.*
@@ -1460,7 +1458,7 @@ class GesturesPluginImpl : GesturesPlugin, GesturesSettingsBase {
   /**
    * Remove a callback that is invoked when the map is scaled.
    */
-  fun removeOnScaleListener(listener: OnScaleListener) {
+  override fun removeOnScaleListener(listener: OnScaleListener) {
     onScaleListenerList.remove(listener)
   }
 
@@ -1537,8 +1535,6 @@ class GesturesPluginImpl : GesturesPlugin, GesturesSettingsBase {
    */
   override fun cleanup() {
     protectedCameraAnimatorOwnerList.clear()
-    gesturesPluginWeakRef?.clear()
-    gesturesPluginWeakRef = null
   }
 
   /**
@@ -1587,7 +1583,6 @@ class GesturesPluginImpl : GesturesPlugin, GesturesSettingsBase {
    * Called when the plugin is first added to the map.
    */
   override fun initialize() {
-    gesturesPluginWeakRef = WeakReference(this)
     initializeGesturesManager(gesturesManager, true)
     initializeGestureListeners(context, true)
   }
@@ -1600,9 +1595,6 @@ class GesturesPluginImpl : GesturesPlugin, GesturesSettingsBase {
       duration = 0
       owner = MapAnimationOwnerRegistry.GESTURES
     }
-    private var gesturesPluginWeakRef: WeakReference<GesturesPluginImpl>? = null
-    internal val gesturesPlugin: GesturesPluginImpl?
-      get() = gesturesPluginWeakRef?.get()
   }
 }
 
@@ -1618,124 +1610,126 @@ fun MapPluginProviderDelegate.getGesturesPlugin(): GesturesPlugin {
 /**
  * Add a callback that is invoked when the map is clicked.
  */
-fun MapListenerDelegate.addOnMapClickListener(onMapClickListener: OnMapClickListener) {
-  gesturesPlugin?.addOnMapClickListener(onMapClickListener)
+fun MapPluginExtensionsDelegate.addOnMapClickListener(onMapClickListener: OnMapClickListener) {
+  gesturesPlugin { addOnMapClickListener(onMapClickListener) }
 }
 
 /**
  * Remove a callback that is invoked when the map is clicked.
  */
-fun MapListenerDelegate.removeOnMapClickListener(onMapClickListener: OnMapClickListener) {
-  gesturesPlugin?.removeOnMapClickListener(onMapClickListener)
+fun MapPluginExtensionsDelegate.removeOnMapClickListener(onMapClickListener: OnMapClickListener) {
+  gesturesPlugin { removeOnMapClickListener(onMapClickListener) }
 }
 
 /**
  * Add a callback that is invoked when the map is long clicked.
  */
-fun MapListenerDelegate.addOnMapLongClickListener(onMapLongClickListener: OnMapLongClickListener) {
-  gesturesPlugin?.addOnMapLongClickListener(onMapLongClickListener)
+fun MapPluginExtensionsDelegate.addOnMapLongClickListener(onMapLongClickListener: OnMapLongClickListener) {
+  gesturesPlugin { addOnMapLongClickListener(onMapLongClickListener) }
 }
 
 /**
  * Remove a callback that is invoked when the map is long clicked.
  */
-fun MapListenerDelegate.removeOnMapLongClickListener(onMapLongClickListener: OnMapLongClickListener) {
-  gesturesPlugin?.removeOnMapLongClickListener(onMapLongClickListener)
+fun MapPluginExtensionsDelegate.removeOnMapLongClickListener(onMapLongClickListener: OnMapLongClickListener) {
+  gesturesPlugin { removeOnMapLongClickListener(onMapLongClickListener) }
 }
 
 /**
  * Add a callback that is invoked when the map is has received a fling gesture.
  */
-fun MapListenerDelegate.addOnFlingListener(onFlingListener: OnFlingListener) {
-  gesturesPlugin?.addOnFlingListener(onFlingListener)
+fun MapPluginExtensionsDelegate.addOnFlingListener(onFlingListener: OnFlingListener) {
+  gesturesPlugin { addOnFlingListener(onFlingListener) }
 }
 
 /**
  * Remove a callback that is invoked when the map is has received a fling gesture.
  */
-fun MapListenerDelegate.removeOnFlingListener(onFlingListener: OnFlingListener) {
-  gesturesPlugin?.removeOnFlingListener(onFlingListener)
+fun MapPluginExtensionsDelegate.removeOnFlingListener(onFlingListener: OnFlingListener) {
+  gesturesPlugin { removeOnFlingListener(onFlingListener) }
 }
 
 /**
  * Add a callback that is invoked when the map is moved.
  */
-fun MapListenerDelegate.addOnMoveListener(listener: OnMoveListener) {
-  gesturesPlugin?.addOnMoveListener(listener)
+fun MapPluginExtensionsDelegate.addOnMoveListener(listener: OnMoveListener) {
+  gesturesPlugin { addOnMoveListener(listener) }
 }
 
 /**
  * Remove a callback that is invoked when the map is moved.
  */
-fun MapListenerDelegate.removeOnMoveListener(listener: OnMoveListener) {
-  gesturesPlugin?.removeOnMoveListener(listener)
+fun MapPluginExtensionsDelegate.removeOnMoveListener(listener: OnMoveListener) {
+  gesturesPlugin { removeOnMoveListener(listener) }
 }
 
 /**
  * Add a callback that is invoked when the map is rotated.
  */
-fun MapListenerDelegate.addOnRotateListener(listener: OnRotateListener) {
-  gesturesPlugin?.addOnRotateListener(listener)
+fun MapPluginExtensionsDelegate.addOnRotateListener(listener: OnRotateListener) {
+  gesturesPlugin { addOnRotateListener(listener) }
 }
 
 /**
  * Remove a callback that is invoked when the map is rotated.
  */
-fun MapListenerDelegate.removeOnRotateListener(listener: OnRotateListener) {
-  gesturesPlugin?.removeOnRotateListener(listener)
+fun MapPluginExtensionsDelegate.removeOnRotateListener(listener: OnRotateListener) {
+  gesturesPlugin { removeOnRotateListener(listener) }
 }
 
 /**
  * Add a callback that is invoked when the map is scaled.
  */
-fun MapListenerDelegate.addOnScaleListener(listener: OnScaleListener) {
-  gesturesPlugin?.addOnScaleListener(listener)
+fun MapPluginExtensionsDelegate.addOnScaleListener(listener: OnScaleListener) {
+  gesturesPlugin { addOnScaleListener(listener) }
 }
 
 /**
  * Remove a callback that is invoked when the map is scaled.
  */
-fun MapListenerDelegate.removeOnScaleListener(listener: OnScaleListener) {
-  gesturesPlugin?.removeOnScaleListener(listener)
+fun MapPluginExtensionsDelegate.removeOnScaleListener(listener: OnScaleListener) {
+  gesturesPlugin { removeOnScaleListener(listener) }
 }
 
 /**
  * Add a callback that is invoked when the map is shoved.
  */
-fun MapListenerDelegate.addOnShoveListener(listener: OnShoveListener) {
-  gesturesPlugin?.addOnShoveListener(listener)
+fun MapPluginExtensionsDelegate.addOnShoveListener(listener: OnShoveListener) {
+  gesturesPlugin { addOnShoveListener(listener) }
 }
 
 /**
  * Remove a callback that is invoked when the map is shoved.
  */
-fun MapListenerDelegate.removeOnShoveListener(listener: OnShoveListener) {
-  gesturesPlugin?.removeOnShoveListener(listener)
+fun MapPluginExtensionsDelegate.removeOnShoveListener(listener: OnShoveListener) {
+  gesturesPlugin { removeOnShoveListener(listener) }
 }
 
 /**
  * Get the current configured AndroidGesturesManager.
  */
-fun MapListenerDelegate.getGesturesManager(): AndroidGesturesManager? {
-  return gesturesPlugin?.getGesturesManager()
+fun MapPluginExtensionsDelegate.getGesturesManager(): AndroidGesturesManager? {
+  return gesturesPlugin { getGesturesManager() } as AndroidGesturesManager?
 }
 
 /**
  * Set the AndroidGesturesManager instance.
  */
-fun MapListenerDelegate.setGesturesManager(
+fun MapPluginExtensionsDelegate.setGesturesManager(
   androidGesturesManager: AndroidGesturesManager,
   attachDefaultListeners: Boolean,
   setDefaultMutuallyExclusives: Boolean
 ) {
-  gesturesPlugin?.setGesturesManager(
-    androidGesturesManager,
-    attachDefaultListeners,
-    setDefaultMutuallyExclusives
-  )
+  gesturesPlugin {
+    setGesturesManager(
+      androidGesturesManager,
+      attachDefaultListeners,
+      setDefaultMutuallyExclusives
+    )
+  }
 }
 
 /**
  * The gesture configuration object.
  */
-fun MapListenerDelegate.getGesturesSettings() = gesturesPlugin?.getSettings()
+fun MapPluginExtensionsDelegate.getGesturesSettings() = gesturesPlugin { getSettings() } as GesturesSettings?

sdk-base/src/main/java/com/mapbox/maps/plugin/delegates/MapPluginExtensionsDelegate.kt

@@ -0,0 +1,23 @@
+package com.mapbox.maps.plugin.delegates
+
+import com.mapbox.maps.plugin.animation.CameraAnimationsPlugin
+import com.mapbox.maps.plugin.gestures.GesturesPlugin
+
+/**
+ * Definition of the map plugin extensions delegate.
+ * Makes possible to call plugin functions directly from MapboxMap.
+ */
+interface MapPluginExtensionsDelegate {
+
+  /**
+   * Call extension function on [CameraAnimationsPlugin].
+   * In most cases should not be called directly.
+   */
+  fun cameraAnimationsPlugin(function: (CameraAnimationsPlugin.() -> Any?)): Any?
+
+  /**
+   * Call extension function on [GesturesPlugin].
+   * In most cases should not be called directly.
+   */
+  fun gesturesPlugin(function: (GesturesPlugin.() -> Any?)): Any?
+}

sdk-base/src/main/java/com/mapbox/maps/plugin/gestures/GesturesPlugin.kt

@@ -133,6 +133,11 @@ interface GesturesPlugin : MapPlugin, ContextBinder, MapSizePlugin, GesturesSett
    */
   fun addOnScaleListener(onScaleListener: OnScaleListener)
 
+  /**
+   * Remove a callback that is invoked when the map is scaled.
+   */
+  fun removeOnScaleListener(listener: OnScaleListener)
+
   /**
    * Add a shove gesture listener tha is invoked when a map is shoved
    *