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 <>
klaviyo · Sep 24, 2024 · c842528 · c842528
1 parent 9205720
commit c842528
Show file tree

Hide file tree

Showing 12 changed files with 119 additions and 17 deletions.
diff --git a/README.md b/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>

diff --git a/docs/index.html b/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">
diff --git a/sdk/analytics/build.gradle b/sdk/analytics/build.gradle
@@ -19,6 +19,7 @@ android {
 dependencies {
  implementation project(":sdk:core")
  testImplementation project(":sdk:fixtures")
+ testImplementation KotlinX.coroutines.test
 }
 
 afterEvaluate {

diff --git a/sdk/analytics/src/main/java/com/klaviyo/analytics/networking/KlaviyoApiClient.kt b/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) }
+ }
  }
 
  /**

diff --git a/sdk/analytics/src/test/java/com/klaviyo/analytics/networking/KlaviyoApiClientTest.kt b/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

diff --git a/sdk/core/build.gradle b/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")

diff --git a/sdk/core/src/main/java/com/klaviyo/core/lifecycle/KlaviyoLifecycleMonitor.kt b/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
 

diff --git a/sdk/core/src/main/java/com/klaviyo/core/model/SharedPreferencesDataStore.kt b/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) }
+ }
  }
 
  /**

diff --git a/sdk/core/src/main/java/com/klaviyo/core/networking/KlaviyoNetworkMonitor.kt b/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) }
+ }
  }
 
  /**

diff --git a/sdk/core/src/test/java/com/klaviyo/core/model/SharedPreferencesDataStoreTest.kt b/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()

diff --git a/sdk/core/src/test/java/com/klaviyo/core/networking/KlaviyoNetworkMonitorTest.kt b/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()
+ }
 }
diff --git a/versions.properties b/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