fix: session id not set before plugin setup (#189)

* fix: ensure sessionId is set before Plugin.setup is called
amplitude · Mar 27, 2024 · 42e4fef · 42e4fef
1 parent 1428953
commit 42e4fef
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 8 deletions.
diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt
@@ -15,6 +15,7 @@ import com.amplitude.core.platform.plugins.AmplitudeDestination
 import com.amplitude.core.platform.plugins.GetAmpliExtrasPlugin
 import com.amplitude.core.utilities.FileStorage
 import com.amplitude.id.IdentityConfiguration
+import kotlinx.coroutines.Deferred
 import kotlinx.coroutines.launch
 
 open class Amplitude(
@@ -51,10 +52,19 @@ open class Amplitude(
         )
     }
 
+    override fun build(): Deferred<Boolean> {
+        val deferred = super.build()
+
+        // Start session before adding plugins
+        (timeline as Timeline).initSession()
+
+        return deferred
+    }
+
     override suspend fun buildInternal(identityConfiguration: IdentityConfiguration) {
         // Migrations
         ApiKeyStorageMigration(this).execute()
-        if ((this.configuration as Configuration).migrateLegacyData) {
+        if ((configuration as Configuration).migrateLegacyData) {
             RemnantDataMigration(this).execute()
         }
 
@@ -84,8 +94,7 @@ open class Amplitude(
             }
         }
 
-        val androidTimeline = timeline as Timeline
-        androidTimeline.start()
+        (timeline as Timeline).start()
     }
 
     /**

diff --git a/android/src/main/java/com/amplitude/android/Timeline.kt b/android/src/main/java/com/amplitude/android/Timeline.kt
@@ -26,13 +26,15 @@ class Timeline : Timeline() {
     internal var sessionId: Long = Session.EMPTY_SESSION_ID
         get() = if (session == null) Session.EMPTY_SESSION_ID else session.sessionId
 
-    internal suspend fun start() {
+    internal fun initSession() {
         this.session = Session(
             amplitude.configuration as Configuration,
             amplitude.storage,
             amplitude.store
         )
+    }
 
+    internal suspend fun start() {
         val sessionEvents = session.startNewSessionIfNeeded(
             SystemTime.getCurrentTimeMillis(),
             amplitude.configuration.sessionId
@@ -49,9 +51,11 @@ class Timeline : Timeline() {
             }
         }
 
-        runBlocking {
-            sessionEvents?.forEach {
-                processImmediately(it)
+        if (!amplitude.configuration.optOut) {
+            runBlocking {
+                sessionEvents?.forEach {
+                    processImmediately(it)
+                }
             }
         }
     }

diff --git a/android/src/test/java/com/amplitude/android/AmplitudeTest.kt b/android/src/test/java/com/amplitude/android/AmplitudeTest.kt
@@ -10,6 +10,7 @@ import com.amplitude.android.utils.mockSystemTime
 import com.amplitude.common.android.AndroidContextProvider
 import com.amplitude.core.StorageProvider
 import com.amplitude.core.events.BaseEvent
+import com.amplitude.core.platform.DestinationPlugin
 import com.amplitude.core.platform.EventPlugin
 import com.amplitude.core.platform.Plugin
 import com.amplitude.core.utilities.ConsoleLoggerProvider
@@ -231,6 +232,38 @@ class AmplitudeTest {
         }
     }
 
+    @Test
+    fun amplitude_should_set_sessionId_before_plugin_setup() = runTest {
+        class SessionIdPlugin() : DestinationPlugin() {
+            override val type: Plugin.Type = Plugin.Type.Destination
+
+            override lateinit var amplitude: com.amplitude.core.Amplitude
+
+            override fun setup(amplitude: com.amplitude.core.Amplitude) {
+                super.setup(amplitude)
+
+                this.amplitude = amplitude
+
+                val sessionId = (amplitude as Amplitude).sessionId
+
+                Assertions.assertNotNull(sessionId)
+            }
+        }
+
+        // set session Id in the config
+        val config = createConfiguration()
+        // isolate storage from other tests
+        config.instanceName = "session-id-for-plugin-setup"
+        val amp = Amplitude(config)
+        amp.add(SessionIdPlugin())
+
+        setDispatcher(testScheduler)
+
+        if (amp?.isBuilt!!.await()) {
+            Assertions.assertNotNull(amp?.sessionId)
+        }
+    }
+
     @Test
     fun amplitude_should_set_deviceId_from_configuration() = runTest {
         val testDeviceId = "test device id"

diff --git a/core/src/main/java/com/amplitude/core/Amplitude.kt b/core/src/main/java/com/amplitude/core/Amplitude.kt
@@ -100,9 +100,10 @@ open class Amplitude internal constructor(
     protected open fun build(): Deferred<Boolean> {
         val amplitude = this
 
+        storage = configuration.storageProvider.getStorage(amplitude)
+
         val built =
             amplitudeScope.async(amplitudeDispatcher, CoroutineStart.LAZY) {
-                storage = configuration.storageProvider.getStorage(amplitude)
                 identifyInterceptStorage =
                     configuration.identifyInterceptStorageProvider.getStorage(
                         amplitude,