fix: remove userId from the configurationAndroid class (#448)

* fix: remove userId from ConfigurationAndroid and ConfigurationAndroidBuilder

UserId should not be part of the sdk configuration. It can only be modified by the identify event.

* fix: remove the code to update the userId in configuration object

* refactor: use `androidStorage` to fetch the userId

* test: fix failing test case in FillDefaultsPluginTest

Use real storage object so that userId could be fetched

* refactor: directly set the userId in storage

Analytics.setUserId extension function was required for two purpose: First is to store the userId in storage and second is to update the userId configuration. Now we no longer require the latter part we can simplify the logic.

* chore: remove setting of userId from v2Cache

As we are no longer updating the userId in configuration object, we don't need to basically fetch the userId from the storage and again store it in storage.

* refactor: create an extension function for empty string and use that

android/src/main/java/com/rudderstack/android/ConfigurationAndroid.kt

@@ -33,7 +33,6 @@ interface ConfigurationAndroid : Configuration {
      *
      * @property application
      * @property anonymousId
-     * @property userId
      * @property trackLifecycleEvents
      * @property recordScreenViews
      * @property isPeriodicFlushEnabled
@@ -62,7 +61,6 @@ interface ConfigurationAndroid : Configuration {
      */
     val application: Application
     val anonymousId: String?
-    val userId: String?
     val trackLifecycleEvents: Boolean
     val recordScreenViews: Boolean
     val isPeriodicFlushEnabled: Boolean
@@ -83,7 +81,6 @@ interface ConfigurationAndroid : Configuration {
             application: Application,
             jsonAdapter: JsonAdapter,
             anonymousId: String? = null,
-            userId: String? = null,
             options: RudderOption = RudderOption(),
             flushQueueSize: Int = Defaults.DEFAULT_FLUSH_QUEUE_SIZE,
             maxFlushInterval: Long = Defaults.DEFAULT_MAX_FLUSH_INTERVAL,
@@ -112,7 +109,6 @@ interface ConfigurationAndroid : Configuration {
             application,
             jsonAdapter,
             anonymousId,
-            userId,
             options,
             flushQueueSize,
             maxFlushInterval,
@@ -143,7 +139,6 @@ interface ConfigurationAndroid : Configuration {
             application: Application,
             jsonAdapter: JsonAdapter,
             anonymousId: String? = null,
-            userId: String? = null,
             options: RudderOption = RudderOption(),
             flushQueueSize: Int = Defaults.DEFAULT_FLUSH_QUEUE_SIZE,
             maxFlushInterval: Long = Defaults.DEFAULT_MAX_FLUSH_INTERVAL,
@@ -175,7 +170,6 @@ interface ConfigurationAndroid : Configuration {
         ): ConfigurationAndroid = object : ConfigurationAndroid {
             override val application: Application = application
             override val anonymousId: String? = anonymousId
-            override val userId: String? = userId
             override val trackLifecycleEvents: Boolean = trackLifecycleEvents
             override val recordScreenViews: Boolean = recordScreenViews
             override val isPeriodicFlushEnabled: Boolean = isPeriodicFlushEnabled
@@ -210,7 +204,6 @@ interface ConfigurationAndroid : Configuration {
             configuration: Configuration,
             application: Application,
             anonymousId: String = AndroidUtils.generateAnonymousId(Defaults.COLLECT_DEVICE_ID, application),
-            userId: String? = null,
             trackLifecycleEvents: Boolean = Defaults.TRACK_LIFECYCLE_EVENTS,
             recordScreenViews: Boolean = Defaults.RECORD_SCREEN_VIEWS,
             isPeriodicFlushEnabled: Boolean = Defaults.IS_PERIODIC_FLUSH_ENABLED,
@@ -229,7 +222,6 @@ interface ConfigurationAndroid : Configuration {
                 application,
                 configuration.jsonAdapter,
                 anonymousId,
-                userId,
                 configuration.options,
                 configuration.flushQueueSize,
                 configuration.maxFlushInterval,
@@ -302,7 +294,6 @@ interface ConfigurationAndroid : Configuration {
         networkExecutor: ExecutorService = this.networkExecutor,
         base64Generator: Base64Generator = this.base64Generator,
         anonymousId: String? = this.anonymousId,
-        userId: String? = this.userId,
         advertisingId: String? = this.advertisingId,
         autoCollectAdvertId: Boolean = this.autoCollectAdvertId,
         deviceToken: String? = this.deviceToken,
@@ -313,7 +304,6 @@ interface ConfigurationAndroid : Configuration {
             application,
             jsonAdapter,
             anonymousId,
-            userId,
             options,
             flushQueueSize,
             maxFlushInterval,

android/src/main/java/com/rudderstack/android/compat/ConfigurationAndroidBuilder.java

@@ -30,7 +30,6 @@
 public class ConfigurationAndroidBuilder extends ConfigurationBuilder {
     private final Application application ;
     private String anonymousId;
-    private String userId = null;
     private Boolean trackLifecycleEvents = ConfigurationAndroid.Defaults.TRACK_LIFECYCLE_EVENTS;
     private Boolean recordScreenViews  = ConfigurationAndroid.Defaults.RECORD_SCREEN_VIEWS;
     private Boolean isPeriodicFlushEnabled  = ConfigurationAndroid.Defaults.IS_PERIODIC_FLUSH_ENABLED;
@@ -54,10 +53,6 @@ public ConfigurationBuilder withAnonymousId(String anonymousId) {
         this.anonymousId = anonymousId;
         return this;
     }
-    public ConfigurationBuilder withUserId(String userId) {
-        this.userId = userId;
-        return this;
-    }
     public ConfigurationBuilder withTrackLifecycleEvents(Boolean trackLifecycleEvents) {
         this.trackLifecycleEvents = trackLifecycleEvents;
         return this;
@@ -118,7 +113,6 @@ public ConfigurationAndroid build() {
         return ConfigurationAndroid.Companion.invoke(super.build(),
                 application,
                 anonymousId,
-                userId,
                 trackLifecycleEvents,
                 recordScreenViews,
                 isPeriodicFlushEnabled,

android/src/main/java/com/rudderstack/android/internal/infrastructure/ReinstatePlugin.kt

@@ -22,7 +22,6 @@ import com.rudderstack.android.utilities.currentConfigurationAndroid
 import com.rudderstack.android.utilities.initializeSessionManagement
 import com.rudderstack.android.utilities.processNewContext
 import com.rudderstack.android.utilities.setAnonymousId
-import com.rudderstack.android.utilities.setUserId
 import com.rudderstack.core.Analytics
 import com.rudderstack.core.DataUploadService
 import com.rudderstack.core.InfrastructurePlugin
@@ -84,7 +83,6 @@ internal class ReinstatePlugin : InfrastructurePlugin {
     }
 
     private fun reinstateV2FromCache() {
-        val userId = _analytics?.androidStorage?.userId
         val anonId = _analytics?.androidStorage?.anonymousId
             ?: _analytics?.currentConfigurationAndroid?.let {
                 AndroidUtils.generateAnonymousId(
@@ -96,9 +94,6 @@ internal class ReinstatePlugin : InfrastructurePlugin {
         context?.let {
             _analytics?.processNewContext(context)
         }
-        userId?.let {
-            _analytics?.setUserId(it)
-        }
         if (anonId != null)
             _analytics?.setAnonymousId(anonId)
         _analytics?.initializeSessionManagement(
@@ -146,7 +141,7 @@ internal class ReinstatePlugin : InfrastructurePlugin {
         val traits = androidStorage.v1Traits
         val userId = traits?.get("userId") as? String ?: traits?.get("id") as? String
         if (userId.isNullOrEmpty() || !this.androidStorage.userId.isNullOrEmpty()) return
-        _analytics?.setUserId(userId)
+        androidStorage.setUserId(userId)
     }
 
     private fun Analytics.migrateAnonymousIdFromV1() {

android/src/main/java/com/rudderstack/android/internal/plugins/ExtractStatePlugin.kt

@@ -14,10 +14,11 @@
 
 package com.rudderstack.android.internal.plugins
 
+import com.rudderstack.android.utilities.androidStorage
 import com.rudderstack.android.utilities.contextState
 import com.rudderstack.android.utilities.currentConfigurationAndroid
+import com.rudderstack.android.utilities.empty
 import com.rudderstack.android.utilities.processNewContext
-import com.rudderstack.android.utilities.setUserId
 import com.rudderstack.core.Analytics
 import com.rudderstack.core.Plugin
 import com.rudderstack.core.optAdd
@@ -65,9 +66,7 @@ internal class ExtractStatePlugin : Plugin {
             val newUserId = getUserId(message)
 
             _analytics?.rudderLogger?.debug(log = "New user id detected: $newUserId")
-            val prevId = _analytics?.currentConfigurationAndroid?.let {
-                it.userId ?: it.anonymousId
-            } ?: ""
+            val prevId = _analytics?.androidStorage?.userId ?: _analytics?.currentConfigurationAndroid?.anonymousId ?: String.empty()
             // in case of identify, the stored traits (if any) are replaced by the ones provided
             // if user id is different. else traits are added to it
             val msg = when (message) {
@@ -95,7 +94,7 @@ internal class ExtractStatePlugin : Plugin {
             }?:message
             msg.also {
                 newUserId?.let { id ->
-                    _analytics?.setUserId(id)
+                    _analytics?.androidStorage?.setUserId(id)
                 }
             }
             return chain.proceed(msg)

android/src/main/java/com/rudderstack/android/internal/plugins/FillDefaultsPlugin.kt

@@ -14,6 +14,7 @@
 
 package com.rudderstack.android.internal.plugins
 
+import com.rudderstack.android.utilities.androidStorage
 import com.rudderstack.android.utilities.contextState
 import com.rudderstack.android.utilities.currentConfigurationAndroid
 import com.rudderstack.core.Analytics
@@ -51,7 +52,7 @@ internal class FillDefaultsPlugin : Plugin {
     @Throws(MissingPropertiesException::class)
     private inline fun <reified T : Message> T.withDefaults(): T {
         val anonId = this.anonymousId ?: _analytics?.currentConfigurationAndroid?.anonymousId
-        val userId = this.userId ?: _analytics?.currentConfigurationAndroid?.userId
+        val userId = this.userId ?: _analytics?.androidStorage?.userId
         if (anonId == null && userId == null) {
             val ex = MissingPropertiesException("Either Anonymous Id or User Id must be present");
             _analytics?.currentConfigurationAndroid?.rudderLogger?.error(
@@ -64,7 +65,7 @@ internal class FillDefaultsPlugin : Plugin {
         val newContext =
                 // in case of alias we purposefully remove traits from context
                 _analytics?.contextState?.value?.let {
-                    if (this is AliasMessage && this.userId != _analytics?.currentConfigurationAndroid?.userId) it.updateWith(
+                    if (this is AliasMessage && this.userId != _analytics?.androidStorage?.userId) it.updateWith(
                         traits = mapOf()
                     ) else it
                 } selectiveReplace context.let {

android/src/main/java/com/rudderstack/android/utilities/AnalyticsUtil.kt

@@ -85,21 +85,6 @@ fun Analytics.setAnonymousId(anonymousId: String) {
     processNewContext(newContext)
 }
 
-/**
- * Setting the [ConfigurationAndroid.userId] explicitly.
- *
- * @param userId String to be used as userId
- */
-fun Analytics.setUserId(userId: String) {
-    androidStorage.setUserId(userId)
-    applyConfiguration {
-        if (this is ConfigurationAndroid) copy(
-            userId = userId
-        )
-        else this
-    }
-}
-
 private val infrastructurePlugins = arrayOf(
     ReinstatePlugin(),
     AnonymousIdHeaderPlugin(),
@@ -157,4 +142,6 @@ private fun Analytics.attachSavedContextIfAvailable() {
     androidStorage.context?.let {
         processNewContext(it)
     }
-}
+}
+
+fun String.Companion.empty(): String = ""

android/src/test/java/com/rudderstack/android/plugins/FillDefaultsPluginTest.kt

@@ -20,13 +20,16 @@ import com.rudderstack.android.ConfigurationAndroid
 import com.rudderstack.android.utils.TestExecutor
 import com.rudderstack.android.internal.plugins.FillDefaultsPlugin
 import com.rudderstack.android.internal.states.ContextState
+import com.rudderstack.android.storage.AndroidStorage
+import com.rudderstack.android.storage.AndroidStorageImpl
 import com.rudderstack.core.Analytics
 import com.rudderstack.core.RudderLogger
 import com.rudderstack.core.RudderUtils
 import com.rudderstack.core.holder.associateState
 import com.rudderstack.core.holder.retrieveState
+import com.rudderstack.gsonrudderadapter.GsonAdapter
 import com.rudderstack.models.*
-import com.vagabond.testcommon.Verification
+import com.rudderstack.rudderjsonadapter.JsonAdapter
 import com.vagabond.testcommon.assertArgument
 import com.vagabond.testcommon.generateTestAnalytics
 import com.vagabond.testcommon.testPlugin
@@ -36,7 +39,6 @@ import org.junit.After
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.mockito.kotlin.mock
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
 
@@ -46,34 +48,36 @@ import org.robolectric.annotation.Config
     @Config(manifest = Config.NONE, sdk = [Build.VERSION_CODES.P])
 class FillDefaultsPluginTest {
 
-    //    private val commonContext = mapOf(
-//        "some_context1" to "some_value",
-//        "some_context2" to "some_value_2"
-//    )
     private lateinit var analytics: Analytics
     lateinit var mockConfig: ConfigurationAndroid
-
-    private val fillDefaultsPlugin = FillDefaultsPlugin(
-    )
+    private val fillDefaultsPlugin = FillDefaultsPlugin()
+    private val jsonAdapter: JsonAdapter = GsonAdapter()
+    private lateinit var storage: AndroidStorage
 
     @Before
     fun setup() {
+        storage = AndroidStorageImpl(
+            getApplicationContext(),
+            false,
+            writeKey = "test_writeKey",
+            storageExecutor = TestExecutor(),
+        )
         mockConfig = ConfigurationAndroid(
             application = getApplicationContext(),
-            mock(),
+            jsonAdapter,
             anonymousId = "anon_id",
-            userId = "user_id",
             shouldVerifySdk = false,
             analyticsExecutor = TestExecutor(),
             logLevel = RudderLogger.LogLevel.DEBUG,
         )
-        analytics = generateTestAnalytics(mockConfig)
+        analytics = generateTestAnalytics(mockConfig, storage = storage)
         analytics.associateState(ContextState())
         fillDefaultsPlugin.setup(analytics)
         fillDefaultsPlugin.updateConfiguration(mockConfig)
     }
     @After
     fun destroy() {
+        analytics.storage.clearStorage()
         analytics.shutdown()
     }
 
@@ -104,21 +108,18 @@ class FillDefaultsPluginTest {
             ), customContextMap = null
         )
 
-//        val chain = CentralPluginChain(message, listOf(fillDefaultsPlugin))
         analytics.testPlugin(fillDefaultsPlugin)
         analytics.track(message)
         analytics.assertArgument { input, output ->
             //check for expected values
             assertThat(output?.anonymousId, allOf(notNullValue(), `is`("anon_id")))
-            assertThat(output?.userId, allOf(notNullValue(), `is`("user_id")))
             //message context to override
             assertThat(
                 output?.context?.traits, allOf(
                     notNullValue(),
                     aMapWithSize(2),
-                    hasEntry("age", 31),
+                    hasEntry("age", 31.0),
                     hasEntry("office", "Rudderstack"),
-//            hasEntry("name", "some_name"),
                 )
             )
             assertThat(
@@ -137,6 +138,5 @@ class FillDefaultsPluginTest {
 
             )
         }
-
     }
 }