Update Experiment Client with LoadStoreCache and Variant (#30)

sdk/build.gradle

@@ -46,6 +46,7 @@ dependencies {
     testImplementation 'junit:junit:4.13.2'
     testImplementation 'org.json:json:20201115'
     testImplementation project(path: ':sdk')
+    testImplementation "org.mockito:mockito-core:3.7.7"
 }
 
 task docs(dependsOn: dokkaHtml) {

sdk/src/main/java/com/amplitude/experiment/DefaultExperimentClient.kt

@@ -1,7 +1,9 @@
 package com.amplitude.experiment
 
-import com.amplitude.experiment.analytics.ExposureEvent as OldExposureEvent
+import com.amplitude.experiment.storage.LoadStoreCache
 import com.amplitude.experiment.storage.Storage
+import com.amplitude.experiment.analytics.ExposureEvent as OldExposureEvent
+import com.amplitude.experiment.storage.getVariantStorage
 import com.amplitude.experiment.util.AsyncFuture
 import com.amplitude.experiment.util.Backoff
 import com.amplitude.experiment.util.BackoffConfig
@@ -35,11 +37,19 @@ internal class DefaultExperimentClient internal constructor(
     private val apiKey: String,
     private val config: ExperimentConfig,
     private val httpClient: OkHttpClient,
-    private val storage: Storage,
+    storage: Storage,
     private val executorService: ScheduledExecutorService,
 ) : ExperimentClient {
 
     private var user: ExperimentUser? = null
+    private val variants: LoadStoreCache<Variant> = getVariantStorage(
+        this.apiKey,
+        this.config.instanceName,
+        storage,
+    );
+    init {
+        this.variants.load()
+    }
 
     private val backoffLock = Any()
     private var backoff: Backoff? = null
@@ -131,6 +141,7 @@ internal class DefaultExperimentClient internal constructor(
                 }
                 return VariantAndSource(config.fallbackVariant, VariantSource.FALLBACK_CONFIG)
             }
+
             Source.INITIAL_VARIANTS -> {
                 // for source = InitialVariants, fallback order goes:
                 // 1. InitialFlags
@@ -157,7 +168,8 @@ internal class DefaultExperimentClient internal constructor(
     }
 
     override fun clear() {
-        this.storage.clear()
+        this.variants.clear()
+        this.variants.store()
     }
 
     override fun getUser(): ExperimentUser? {
@@ -274,32 +286,34 @@ internal class DefaultExperimentClient internal constructor(
         return variants
     }
 
-    private fun storeVariants(variants: Map<String, Variant>, options: FetchOptions?) = synchronized(storage) {
-        val failedFlagKeys = options?.flagKeys ?.toMutableList() ?: mutableListOf()
+    private fun storeVariants(variants: Map<String, Variant>, options: FetchOptions?) = synchronized(variants) {
+        val failedFlagKeys = options?.flagKeys?.toMutableList() ?: mutableListOf()
         if (options?.flagKeys == null) {
-            storage.clear()
+            this.variants.clear()
         }
         for (entry in variants.entries) {
-            storage.put(entry.key, entry.value)
+            this.variants.put(entry.key, entry.value)
             failedFlagKeys.remove(entry.key)
         }
         for (key in failedFlagKeys) {
-            storage.remove(key)
+            this.variants.remove(key)
         }
+
+        this.variants.store()
         Logger.d("Stored variants: $variants")
     }
 
     private fun sourceVariants(): Map<String, Variant> {
         return when (config.source) {
-            Source.LOCAL_STORAGE -> storage.getAll()
+            Source.LOCAL_STORAGE -> this.variants.getAll()
             Source.INITIAL_VARIANTS -> config.initialVariants
         }
     }
 
     private fun secondaryVariants(): Map<String, Variant> {
         return when (config.source) {
             Source.LOCAL_STORAGE -> config.initialVariants
-            Source.INITIAL_VARIANTS -> storage.getAll()
+            Source.INITIAL_VARIANTS -> this.variants.getAll()
         }
     }
 
@@ -345,7 +359,7 @@ enum class VariantSource(val type: String) {
 
     fun isFallback(): Boolean {
         return this == FALLBACK_INLINE ||
-            this == FALLBACK_CONFIG ||
-            this == SECONDARY_INITIAL_VARIANTS
+                this == FALLBACK_CONFIG ||
+                this == SECONDARY_INITIAL_VARIANTS
     }
 }

sdk/src/main/java/com/amplitude/experiment/Experiment.kt

@@ -53,7 +53,7 @@ object Experiment {
                     apiKey,
                     mergedConfig,
                     httpClient,
-                    SharedPrefsStorage(application, apiKey, instanceName),
+                    SharedPrefsStorage(application),
                     executorService,
                 )
                 instances[instanceKey] = newInstance
@@ -103,7 +103,7 @@ object Experiment {
                     apiKey,
                     configBuilder.build(),
                     httpClient,
-                    SharedPrefsStorage(application, apiKey, instanceName),
+                    SharedPrefsStorage(application),
                     executorService,
                 )
                 instances[instanceKey] = newInstance

sdk/src/main/java/com/amplitude/experiment/Variant.kt

@@ -21,7 +21,7 @@ data class Variant @JvmOverloads constructor(
      * Flag, segment, and variant metadata produced as a result of
      * evaluation for the user. Used for system purposes.
      */
-    @JvmField val metadata: Map<String, Any?> = emptyMap()
+    @JvmField val metadata: Map<String, Any?>? = null
 ) {
 
     /**

sdk/src/main/java/com/amplitude/experiment/storage/Cache.kt

@@ -1,5 +1,6 @@
 package com.amplitude.experiment.storage
 
+import com.amplitude.experiment.Variant
 import com.amplitude.experiment.util.toMap
 import org.json.JSONObject
 
@@ -56,3 +57,51 @@ internal class LoadStoreCache<V>(
         storage.put(namespace, JSONObject(values).toString())
     }
 }
+
+internal fun getVariantStorage(deploymentKey: String, instanceName: String, storage: Storage): LoadStoreCache<Variant> {
+    val truncatedDeployment = deploymentKey.takeLast(6)
+    val namespace = "amp-exp-$instanceName-$truncatedDeployment"
+    return LoadStoreCache(namespace, storage, ::transformVariantFromStorage)
+}
+
+//fun getFlagStorage(deploymentKey: String, instanceName: String, storage: Storage): LoadStoreCache<Any> {
+//    val truncatedDeployment = deploymentKey.takeLast(6)
+//    val namespace = "amp-exp-$instanceName-$truncatedDeployment-flags"
+//    return LoadStoreCache(namespace, storage)
+//}
+
+internal fun transformVariantFromStorage(storageValue: Any?): Variant {
+    return when (storageValue) {
+        is String -> {
+            // From v0 string format
+            Variant(
+                key = storageValue,
+                value = storageValue
+            )
+        }
+
+        is Map<*, *> -> {
+            // From v1 or v2 object format
+            var key = storageValue["key"] as? String
+            val value = storageValue["value"] as? String
+            val payload = storageValue["payload"]
+            var metadata = (storageValue["metadata"] as? Map<String, Any?>)?.toMutableMap()
+            var experimentKey= storageValue["expKey"] as? String
+
+            if (metadata != null && metadata["experimentKey"] != null) {
+                experimentKey = metadata["experimentKey"] as? String
+            } else if (experimentKey != null) {
+                metadata = metadata ?: HashMap()
+                metadata["experimentKey"] = experimentKey
+            }
+
+            if (key == null && value != null) {
+                key = value
+            }
+
+            Variant(key = key, value = value, payload = payload, expKey = experimentKey, metadata = metadata)
+        }
+
+        else -> Variant()
+    }
+}

sdk/src/main/java/com/amplitude/experiment/util/SessionAnalyticsProvider.kt

@@ -45,4 +45,4 @@ internal class SessionAnalyticsProvider(
         }
         analyticsProvider.unsetUserProperty(event)
     }
-}
+}

sdk/src/main/java/com/amplitude/experiment/util/Variant.kt

@@ -17,6 +17,9 @@ internal fun Variant.toJson(): String {
         if (expKey != null) {
             jsonObject.put("expKey", expKey)
         }
+        if (metadata != null) {
+            jsonObject.put("metadata", metadata)
+        }
     } catch (e: JSONException) {
         Logger.w("Error converting Variant to json string", e)
     }
@@ -53,7 +56,7 @@ internal fun JSONObject?.toVariant(): Variant? {
         }
         val metadata = when {
             has("metadata") -> getJSONObject("metadata").toMap()
-            else -> emptyMap()
+            else -> null
         }
         Variant(value, payload, expKey, key, metadata)
     } catch (e: JSONException) {

sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt

@@ -2,13 +2,14 @@ package com.amplitude.experiment
 
 import com.amplitude.experiment.analytics.ExperimentAnalyticsEvent
 import com.amplitude.experiment.analytics.ExperimentAnalyticsProvider
-import com.amplitude.experiment.storage.InMemoryStorage
+import com.amplitude.experiment.storage.Storage
 import com.amplitude.experiment.util.Logger
 import com.amplitude.experiment.util.SystemLogger
 import okhttp3.OkHttpClient
 import org.junit.Assert
 import org.junit.Test
 import java.util.concurrent.ExecutionException
+import org.mockito.Mockito
 
 private const val API_KEY = "client-DvWljIjiiuqLbyjqdvBaLFfEBrAvGuA3"
 
@@ -21,6 +22,7 @@ class ExperimentClientTest {
         Logger.implementation = SystemLogger(true)
     }
 
+    private val mockStorage = Mockito.mock(Storage::class.java)
     private val testUser = ExperimentUser(userId = "test_user")
 
     private val serverVariant = Variant(key = "on", value = "on", payload = "payload")
@@ -40,7 +42,7 @@ class ExperimentClientTest {
             initialVariants = initialVariants,
         ),
         OkHttpClient(),
-        InMemoryStorage(),
+        mockStorage,
         Experiment.executorService,
     )
 
@@ -53,7 +55,7 @@ class ExperimentClientTest {
             fetchTimeoutMillis = 1,
         ),
         OkHttpClient(),
-        InMemoryStorage(),
+        mockStorage,
         Experiment.executorService,
     )
 
@@ -65,7 +67,7 @@ class ExperimentClientTest {
             initialVariants = initialVariants,
         ),
         OkHttpClient(),
-        InMemoryStorage(),
+        mockStorage,
         Experiment.executorService,
     )
 
@@ -75,7 +77,7 @@ class ExperimentClientTest {
             debug = true,
         ),
         OkHttpClient(),
-        InMemoryStorage(),
+        mockStorage,
         Experiment.executorService,
     )
 
@@ -249,7 +251,7 @@ class ExperimentClientTest {
                 analyticsProvider = analyticsProvider,
             ),
             OkHttpClient(),
-            InMemoryStorage(),
+            mockStorage,
             Experiment.executorService,
         )
         analyticsProviderClient.fetch(testUser).get()
@@ -292,7 +294,7 @@ class ExperimentClientTest {
                 analyticsProvider = analyticsProvider,
             ),
             OkHttpClient(),
-            InMemoryStorage(),
+            mockStorage,
             Experiment.executorService,
         )
         analyticsProviderClient.fetch(testUser).get()
@@ -334,7 +336,7 @@ class ExperimentClientTest {
                 analyticsProvider = analyticsProvider,
             ),
             OkHttpClient(),
-            InMemoryStorage(),
+            mockStorage,
             Experiment.executorService,
         )
         analyticsProviderClient.fetch(testUser).get()
@@ -374,7 +376,7 @@ class ExperimentClientTest {
                 analyticsProvider = analyticsProvider,
             ),
             OkHttpClient(),
-            InMemoryStorage(),
+            mockStorage,
             Experiment.executorService,
         )
         analyticsProviderClient.fetch(testUser).get()
@@ -403,7 +405,7 @@ class ExperimentClientTest {
                 initialVariants = mapOf("flagKey" to Variant(key = "variant", expKey = "experimentKey"))
             ),
             OkHttpClient(),
-            InMemoryStorage(),
+            mockStorage,
             Experiment.executorService,
         )
         client.variant("flagKey")