2.4.1 Patch Build (#187)

* Cherry pick Dan's bug fix for concurrency problem

* 2.4.1 Patch release

---------

Co-authored-by: Evan Masseau <>

Author: evan-masseau

README.md

@@ -55,8 +55,8 @@ send them timely push notifications via [FCM (Firebase Cloud Messaging)](https:/
       ```kotlin
       // build.gradle.kts
       dependencies {
-          implementation("com.github.klaviyo.klaviyo-android-sdk:analytics:2.4.0")
-          implementation("com.github.klaviyo.klaviyo-android-sdk:push-fcm:2.4.0")
+          implementation("com.github.klaviyo.klaviyo-android-sdk:analytics:2.4.1")
+          implementation("com.github.klaviyo.klaviyo-android-sdk:push-fcm:2.4.1")
       }
       ```
    </details>
@@ -67,8 +67,8 @@ send them timely push notifications via [FCM (Firebase Cloud Messaging)](https:/
       ```groovy
        // build.gradle
        dependencies {
-           implementation "com.github.klaviyo.klaviyo-android-sdk:analytics:2.4.0"
-           implementation "com.github.klaviyo.klaviyo-android-sdk:push-fcm:2.4.0"
+           implementation "com.github.klaviyo.klaviyo-android-sdk:analytics:2.4.1"
+           implementation "com.github.klaviyo.klaviyo-android-sdk:push-fcm:2.4.1"
        }
       ```
    </details>

docs/index.html

@@ -1,2 +1,2 @@
 <!-- Redirect to latest version -->
-<meta HTTP-EQUIV="REFRESH" content="0; url=./2.4.0/index.html">
+<meta HTTP-EQUIV="REFRESH" content="0; url=./2.4.1/index.html">

sdk/analytics/build.gradle

@@ -19,6 +19,7 @@ android {
 dependencies {
     implementation project(":sdk:core")
     testImplementation project(":sdk:fixtures")
+    testImplementation KotlinX.coroutines.test
 }
 
 afterEvaluate {

sdk/analytics/src/main/java/com/klaviyo/analytics/networking/KlaviyoApiClient.kt

@@ -14,6 +14,7 @@ import com.klaviyo.analytics.networking.requests.ProfileApiRequest
 import com.klaviyo.analytics.networking.requests.PushTokenApiRequest
 import com.klaviyo.core.Registry
 import com.klaviyo.core.lifecycle.ActivityEvent
+import java.util.Collections
 import java.util.concurrent.ConcurrentLinkedDeque
 import org.json.JSONArray
 import org.json.JSONException
@@ -33,7 +34,9 @@ internal object KlaviyoApiClient : ApiClient {
     /**
      * List of registered API observers
      */
-    private var apiObservers = mutableListOf<ApiObserver>()
+    private val apiObservers = Collections.synchronizedList(
+        mutableListOf<ApiObserver>()
+    )
 
     /**
      * Initialize logic including lifecycle observers and reviving the queue from persistent store
@@ -136,7 +139,9 @@ internal object KlaviyoApiClient : ApiClient {
             Registry.log.verbose("${request.responseCode} $response")
         }
 
-        apiObservers.forEach { it(request) }
+        synchronized(apiObservers) {
+            apiObservers.forEach { it(request) }
+        }
     }
 
     /**

sdk/analytics/src/test/java/com/klaviyo/analytics/networking/KlaviyoApiClientTest.kt

@@ -30,6 +30,10 @@ import io.mockk.unmockkConstructor
 import io.mockk.unmockkObject
 import io.mockk.verify
 import java.net.URL
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.withContext
 import org.json.JSONObject
 import org.junit.After
 import org.junit.Assert.assertEquals
@@ -309,6 +313,26 @@ internal class KlaviyoApiClientTest : BaseTest() {
         verify { logSpy.verbose(match { it.contains("queue") }) }
     }
 
+    @Test
+    fun `Concurrent modification exception does not get thrown on observers`() = runTest {
+        val apiObserver: ApiObserver = { Thread.sleep(6) }
+        val request = mockRequest()
+
+        KlaviyoApiClient.onApiRequest(true, apiObserver)
+
+        val job = launch(Dispatchers.IO) {
+            KlaviyoApiClient.enqueueRequest(request)
+        }
+        val job2 = launch(Dispatchers.Default) {
+            withContext(Dispatchers.IO) {
+                Thread.sleep(8)
+            }
+            KlaviyoApiClient.offApiRequest(apiObserver)
+        }
+        job.start()
+        job2.start()
+    }
+
     @Test
     fun `Invokes callback and logs when request sent`() {
         every { configMock.networkFlushDepth } returns 1

sdk/core/build.gradle

@@ -2,6 +2,8 @@ import static de.fayard.refreshVersions.core.Versions.versionFor
 
 dependencies {
     testImplementation project(":sdk:fixtures")
+    // coroutine testing
+    testImplementation KotlinX.coroutines.test
 }
 description = "Core featureset of the Klaviyo SDK including SDK configuration, session tracking and analytics"
 evaluationDependsOn(":sdk")

sdk/core/src/main/java/com/klaviyo/core/lifecycle/KlaviyoLifecycleMonitor.kt

@@ -4,6 +4,7 @@ import android.app.Activity
 import android.app.Application
 import android.os.Bundle
 import com.klaviyo.core.Registry
+import java.util.Collections
 
 /**
  * Service for monitoring the application lifecycle and network connectivity
@@ -12,7 +13,9 @@ internal object KlaviyoLifecycleMonitor : LifecycleMonitor, Application.Activity
 
     private var activeActivities = 0
 
-    private var activityObservers = mutableListOf<ActivityObserver>()
+    private val activityObservers = Collections.synchronizedList(
+        mutableListOf<ActivityObserver>()
+    )
 
     init {
         onActivityEvent { Registry.log.verbose(it.type) }
@@ -26,7 +29,11 @@ internal object KlaviyoLifecycleMonitor : LifecycleMonitor, Application.Activity
         activityObservers -= observer
     }
 
-    private fun broadcastEvent(event: ActivityEvent) = activityObservers.forEach { it(event) }
+    private fun broadcastEvent(event: ActivityEvent) {
+        synchronized(activityObservers) {
+            activityObservers.forEach { it(event) }
+        }
+    }
 
     //region ActivityLifecycleCallbacks
 

sdk/core/src/main/java/com/klaviyo/core/model/SharedPreferencesDataStore.kt

@@ -3,6 +3,7 @@ package com.klaviyo.core.model
 import android.content.Context
 import android.content.SharedPreferences
 import com.klaviyo.core.Registry
+import java.util.Collections
 
 /**
  * Simple DataStore implementation using SharedPreferences for persistence
@@ -18,7 +19,9 @@ internal object SharedPreferencesDataStore : DataStore {
     /**
      * List of registered observers
      */
-    private var storeObservers = mutableListOf<StoreObserver>()
+    private val storeObservers = Collections.synchronizedList(
+        mutableListOf<StoreObserver>()
+    )
 
     init {
         onStoreChange { key, value -> Registry.log.verbose("$key=$value") }
@@ -33,7 +36,9 @@ internal object SharedPreferencesDataStore : DataStore {
     }
 
     private fun broadcastStoreChange(key: String, value: String?) {
-        storeObservers.forEach { it(key, value) }
+        synchronized(storeObservers) {
+            storeObservers.forEach { it(key, value) }
+        }
     }
 
     /**

sdk/core/src/main/java/com/klaviyo/core/networking/KlaviyoNetworkMonitor.kt

@@ -6,6 +6,7 @@ import android.net.Network
 import android.net.NetworkCapabilities
 import android.net.NetworkRequest
 import com.klaviyo.core.Registry
+import java.util.Collections
 
 /**
  * Service for monitoring the application lifecycle and network connectivity
@@ -23,7 +24,9 @@ internal object KlaviyoNetworkMonitor : NetworkMonitor {
     /**
      * List of registered network change observers
      */
-    private var networkChangeObservers = mutableListOf<NetworkObserver>()
+    private val networkChangeObservers = Collections.synchronizedList(
+        mutableListOf<NetworkObserver>()
+    )
 
     /**
      * Callback object to register with system
@@ -77,7 +80,9 @@ internal object KlaviyoNetworkMonitor : NetworkMonitor {
      */
     private fun broadcastNetworkChange() {
         val isConnected = isNetworkConnected()
-        networkChangeObservers.forEach { it(isConnected) }
+        synchronized(networkChangeObservers) {
+            networkChangeObservers.forEach { it(isConnected) }
+        }
     }
 
     /**

sdk/core/src/test/java/com/klaviyo/core/model/SharedPreferencesDataStoreTest.kt

@@ -9,6 +9,10 @@ import io.mockk.every
 import io.mockk.mockk
 import io.mockk.unmockkObject
 import io.mockk.verify
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.withContext
 import org.junit.Assert.assertEquals
 import org.junit.Test
 
@@ -71,6 +75,29 @@ internal class SharedPreferencesDataStoreTest : BaseTest() {
         verify { logSpy.verbose("$stubKey=$stubValue") }
     }
 
+    @Test
+    fun `Store observers concurrency modification test`() = runTest {
+        withPreferenceMock()
+        withWriteStringMock(stubKey, stubValue)
+        val observer: StoreObserver = { _, _ -> Thread.sleep(6) }
+
+        SharedPreferencesDataStore.onStoreChange(observer)
+
+        val job = launch(Dispatchers.IO) {
+            SharedPreferencesDataStore.clear(stubKey)
+        }
+
+        val job2 = launch(Dispatchers.Default) {
+            withContext(Dispatchers.IO) {
+                Thread.sleep(8)
+            }
+            SharedPreferencesDataStore.offStoreChange(observer)
+        }
+
+        job.start()
+        job2.start()
+    }
+
     @Test
     fun `Removing key uses Klaviyo preferences`() {
         withPreferenceMock()

sdk/core/src/test/java/com/klaviyo/core/networking/KlaviyoNetworkMonitorTest.kt

@@ -13,6 +13,10 @@ import io.mockk.mockkConstructor
 import io.mockk.slot
 import io.mockk.unmockkObject
 import io.mockk.verify
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.withContext
 import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Test
@@ -103,11 +107,11 @@ internal class KlaviyoNetworkMonitorTest : BaseTest() {
     fun `Network change observer is invoked with current network status when network changes`() {
         var expectedNetworkConnection = true
         var callCount = 0
-
-        KlaviyoNetworkMonitor.onNetworkChange {
+        val observer: NetworkObserver = {
             assert(it == expectedNetworkConnection)
             callCount++
         }
+        KlaviyoNetworkMonitor.onNetworkChange(observer)
 
         assert(netCallbackSlot.isCaptured) // attaching a listener should have initialized the network callback
 
@@ -130,6 +134,7 @@ internal class KlaviyoNetworkMonitorTest : BaseTest() {
         netCallbackSlot.captured.onLost(mockk())
 
         assertEquals(6, callCount)
+        KlaviyoNetworkMonitor.offNetworkChange(observer)
     }
 
     @Test
@@ -167,4 +172,25 @@ internal class KlaviyoNetworkMonitorTest : BaseTest() {
         netCallbackSlot.captured.onAvailable(mockk())
         assertEquals(5, callCount)
     }
+
+    @Test()
+    fun `Concurrent modification exception doesn't get thrown on concurrent observer access`() = runTest {
+        val observer: NetworkObserver = { Thread.sleep(6) }
+
+        KlaviyoNetworkMonitor.onNetworkChange(observer)
+
+        val job = launch(Dispatchers.IO) {
+            netCallbackSlot.captured.onAvailable(mockk())
+        }
+
+        val job2 = launch(Dispatchers.Default) {
+            withContext(Dispatchers.IO) {
+                Thread.sleep(5)
+            }
+            KlaviyoNetworkMonitor.offNetworkChange(observer)
+        }
+
+        job.start()
+        job2.start()
+    }
 }

versions.properties

@@ -11,9 +11,9 @@
 
 # Project versioning, run the following gradle command to update version numbers automatically:
 # ./gradlew bumpVersion --nextVersion=X.Y.Z
-version.klaviyo.versionCode=18
+version.klaviyo.versionCode=19
 
-version.klaviyo.versionName=2.4.0
+version.klaviyo.versionName=2.4.1
 
 # Android versioning