Merge pull request #2729 from DataDog/nogorodnikov/rum-10348/remove-feature-name-argument-from-add-remove-featurecontextupdate-receiver

RUM-10348: Remove feature name argument from APIs to set/remove feature context update listener

Author: 0xnm

dd-sdk-android-core/api/apiSurface

@@ -130,9 +130,9 @@ interface com.datadog.android.api.feature.FeatureSdkCore : com.datadog.android.a
   fun updateFeatureContext(String, Boolean = true, (MutableMap<String, Any?>) -> Unit)
   fun getFeatureContext(String, Boolean = true): Map<String, Any?>
   fun setEventReceiver(String, FeatureEventReceiver)
-  fun setContextUpdateReceiver(String, FeatureContextUpdateReceiver)
-  fun removeContextUpdateReceiver(String, FeatureContextUpdateReceiver)
   fun removeEventReceiver(String)
+  fun setContextUpdateReceiver(FeatureContextUpdateReceiver)
+  fun removeContextUpdateReceiver(FeatureContextUpdateReceiver)
   fun createSingleThreadExecutorService(String): java.util.concurrent.ExecutorService
   fun createScheduledExecutorService(String): java.util.concurrent.ScheduledExecutorService
   fun setAnonymousId(java.util.UUID?)

dd-sdk-android-core/api/dd-sdk-android-core.api

@@ -402,10 +402,10 @@ public abstract interface class com/datadog/android/api/feature/FeatureSdkCore :
 	public abstract fun getFeatureContext (Ljava/lang/String;Z)Ljava/util/Map;
 	public abstract fun getInternalLogger ()Lcom/datadog/android/api/InternalLogger;
 	public abstract fun registerFeature (Lcom/datadog/android/api/feature/Feature;)V
-	public abstract fun removeContextUpdateReceiver (Ljava/lang/String;Lcom/datadog/android/api/feature/FeatureContextUpdateReceiver;)V
+	public abstract fun removeContextUpdateReceiver (Lcom/datadog/android/api/feature/FeatureContextUpdateReceiver;)V
 	public abstract fun removeEventReceiver (Ljava/lang/String;)V
 	public abstract fun setAnonymousId (Ljava/util/UUID;)V
-	public abstract fun setContextUpdateReceiver (Ljava/lang/String;Lcom/datadog/android/api/feature/FeatureContextUpdateReceiver;)V
+	public abstract fun setContextUpdateReceiver (Lcom/datadog/android/api/feature/FeatureContextUpdateReceiver;)V
 	public abstract fun setEventReceiver (Ljava/lang/String;Lcom/datadog/android/api/feature/FeatureEventReceiver;)V
 	public abstract fun updateFeatureContext (Ljava/lang/String;ZLkotlin/jvm/functions/Function1;)V
 }

dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureContextUpdateReceiver.kt

@@ -16,8 +16,8 @@ fun interface FeatureContextUpdateReceiver {
     /**
      * Called when the context for a feature is updated.
      * @param featureName the name of the feature
-     * @param event the updated context
+     * @param context the updated context
      */
     @AnyThread
-    fun onContextUpdate(featureName: String, event: Map<String, Any?>)
+    fun onContextUpdate(featureName: String, context: Map<String, Any?>)
 }

dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureSdkCore.kt

@@ -79,28 +79,26 @@ interface FeatureSdkCore : SdkCore {
     fun setEventReceiver(featureName: String, receiver: FeatureEventReceiver)
 
     /**
-     * Sets context update receiver for the given feature. Once subscribed, current context will be emitted
-     * immdediately if it exists.
+     * Removes events receive for the given feature.
      *
      * @param featureName Feature name.
-     * @param listener Listener to remove.
      */
-    fun setContextUpdateReceiver(featureName: String, listener: FeatureContextUpdateReceiver)
+    fun removeEventReceiver(featureName: String)
 
     /**
-     * Removes context update listener for the given feature.
+     * Sets feature context update listener. Once subscribed, current context will be emitted
+     * immdediately if it exists.
      *
-     * @param featureName Feature name.
      * @param listener Listener to remove.
      */
-    fun removeContextUpdateReceiver(featureName: String, listener: FeatureContextUpdateReceiver)
+    fun setContextUpdateReceiver(listener: FeatureContextUpdateReceiver)
 
     /**
-     * Removes events receive for the given feature.
+     * Removes feature context update listener.
      *
-     * @param featureName Feature name.
+     * @param listener Listener to remove.
      */
-    fun removeEventReceiver(featureName: String)
+    fun removeContextUpdateReceiver(listener: FeatureContextUpdateReceiver)
 
     /**
      * Returns a new single thread [ExecutorService], set up with backpressure and internal monitoring.

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/DatadogCore.kt

@@ -45,6 +45,7 @@ import com.datadog.android.ndk.internal.NdkCrashHandler
 import com.datadog.android.privacy.TrackingConsent
 import com.google.gson.JsonObject
 import java.io.File
+import java.util.Collections
 import java.util.Locale
 import java.util.UUID
 import java.util.concurrent.Callable
@@ -89,6 +90,10 @@ internal class DatadogCore(
 
     private var processLifecycleMonitor: ProcessLifecycleMonitor? = null
 
+    @Suppress("UnsafeThirdPartyFunctionCall") // the argument is always empty
+    internal val featureContextUpdateReceivers: MutableSet<FeatureContextUpdateReceiver> =
+        Collections.newSetFromMap(ConcurrentHashMap<FeatureContextUpdateReceiver, Boolean>())
+
     // region SdkCore
 
     /** @inheritDoc */
@@ -233,8 +238,8 @@ internal class DatadogCore(
             feature.featureContextLock.writeLock().safeTryWithLock(1, TimeUnit.SECONDS) {
                 val currentContext = feature.featureContext
                 updateCallback(currentContext)
-                features.values.forEach {
-                    it.notifyContextUpdated(featureName, currentContext)
+                featureContextUpdateReceivers.forEach {
+                    it.onContextUpdate(featureName, currentContext)
                 }
             }
         }
@@ -298,28 +303,27 @@ internal class DatadogCore(
         }
     }
 
-    @Suppress("NestedBlockDepth")
-    override fun setContextUpdateReceiver(featureName: String, listener: FeatureContextUpdateReceiver) {
-        val feature = features[featureName]
-        if (feature == null) {
+    override fun setContextUpdateReceiver(listener: FeatureContextUpdateReceiver) {
+        // the argument is always non - null, so we can suppress the warning
+        @Suppress("UnsafeThirdPartyFunctionCall")
+        if (featureContextUpdateReceivers.contains(listener)) {
             internalLogger.log(
                 InternalLogger.Level.WARN,
                 InternalLogger.Target.USER,
-                { MISSING_FEATURE_FOR_CONTEXT_UPDATE_LISTENER.format(Locale.US, featureName) }
+                { CONTEXT_UPDATE_LISTENER_ALREADY_REGISTERED.format(Locale.US, listener) }
             )
-        } else {
-            feature.setContextUpdateListener(listener)
-            features.forEach {
-                val currentContext = getFeatureContext(it.key, false)
-                if (currentContext.isNotEmpty()) {
-                    listener.onContextUpdate(it.key, currentContext)
-                }
+        }
+        features.forEach {
+            val currentContext = getFeatureContext(it.key, false)
+            if (currentContext.isNotEmpty()) {
+                listener.onContextUpdate(it.key, currentContext)
             }
         }
+        featureContextUpdateReceivers.add(listener)
     }
 
-    override fun removeContextUpdateReceiver(featureName: String, listener: FeatureContextUpdateReceiver) {
-        features[featureName]?.removeContextUpdateListener(listener)
+    override fun removeContextUpdateReceiver(listener: FeatureContextUpdateReceiver) {
+        featureContextUpdateReceivers.remove(listener)
     }
 
     /** @inheritDoc */
@@ -736,6 +740,9 @@ internal class DatadogCore(
             "No need to write last RUM view event: NDK" +
                 " crash reports feature is not enabled and API is below 30."
 
+        internal const val CONTEXT_UPDATE_LISTENER_ALREADY_REGISTERED =
+            "SDK core already has \"%s\" listener registered."
+
         internal val CONFIGURATION_TELEMETRY_DELAY_MS = TimeUnit.SECONDS.toMillis(5)
 
         // fallback for APIs below Android N, see [DefaultAppStartTimeProvider].

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/NoOpInternalSdkCore.kt

@@ -123,12 +123,10 @@ internal object NoOpInternalSdkCore : InternalSdkCore {
     override fun removeEventReceiver(featureName: String) = Unit
 
     override fun setContextUpdateReceiver(
-        featureName: String,
         listener: FeatureContextUpdateReceiver
     ) = Unit
 
     override fun removeContextUpdateReceiver(
-        featureName: String,
         listener: FeatureContextUpdateReceiver
     ) = Unit
 

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt

@@ -14,7 +14,6 @@ import com.datadog.android.api.InternalLogger
 import com.datadog.android.api.context.DatadogContext
 import com.datadog.android.api.feature.EventWriteScope
 import com.datadog.android.api.feature.Feature
-import com.datadog.android.api.feature.FeatureContextUpdateReceiver
 import com.datadog.android.api.feature.FeatureEventReceiver
 import com.datadog.android.api.feature.FeatureScope
 import com.datadog.android.api.feature.StorageBackedFeature
@@ -60,10 +59,8 @@ import com.datadog.android.internal.profiler.BenchmarkSdkUploads
 import com.datadog.android.internal.profiler.GlobalBenchmark
 import com.datadog.android.privacy.TrackingConsentProviderCallback
 import com.datadog.android.security.Encryption
-import java.util.Collections
 import java.util.Locale
 import java.util.concurrent.Callable
-import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicReference
 import java.util.concurrent.locks.ReadWriteLock
@@ -81,10 +78,6 @@ internal class SdkFeature(
     override var dataStore: DataStoreHandler = NoOpDataStoreHandler()
 
     internal val initialized = AtomicBoolean(false)
-
-    @Suppress("UnsafeThirdPartyFunctionCall") // the argument is always empty
-    internal val contextUpdateListeners =
-        Collections.newSetFromMap(ConcurrentHashMap<FeatureContextUpdateReceiver, Boolean>())
     internal val eventReceiver = AtomicReference<FeatureEventReceiver>(null)
     internal var storage: Storage = NoOpStorage()
     internal var uploader: DataUploader = NoOpDataUploader()
@@ -238,39 +231,6 @@ internal class SdkFeature(
 
     // endregion
 
-    // region Context Update Listener
-
-    internal fun notifyContextUpdated(featureName: String, context: Map<String, Any?>) {
-        contextUpdateListeners.forEach {
-            it.onContextUpdate(featureName, context)
-        }
-    }
-
-    internal fun setContextUpdateListener(listener: FeatureContextUpdateReceiver) {
-        synchronized(contextUpdateListeners) {
-            // the argument is always non - null, so we can suppress the warning
-            @Suppress("UnsafeThirdPartyFunctionCall")
-            if (contextUpdateListeners.contains(listener)) {
-                internalLogger.log(
-                    InternalLogger.Level.WARN,
-                    InternalLogger.Target.USER,
-                    { CONTEXT_UPDATE_LISTENER_ALREADY_EXISTS.format(Locale.US, wrappedFeature.name) }
-                )
-            }
-            contextUpdateListeners.add(listener)
-        }
-    }
-
-    internal fun removeContextUpdateListener(listener: FeatureContextUpdateReceiver) {
-        synchronized(contextUpdateListeners) {
-            @Suppress("UnsafeThirdPartyFunctionCall")
-            // the argument is always non - null, so we can suppress the warning
-            contextUpdateListeners.remove(listener)
-        }
-    }
-
-    // endregion
-
     // region Internal
 
     private fun createBatchCountBenchmark() {
@@ -485,8 +445,6 @@ internal class SdkFeature(
     // endregion
 
     companion object {
-        internal const val CONTEXT_UPDATE_LISTENER_ALREADY_EXISTS =
-            "Feature \"%s\" already has this listener registered."
         const val NO_EVENT_RECEIVER =
             "Feature \"%s\" has no event receiver registered, ignoring event."
         internal const val TRACK_NAME = "track"