fix: Variant payload parsing format (#46)

amplitude · Nov 14, 2023 · 7f75de9 · 7f75de9
1 parent b604a1f
commit 7f75de9
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 26 deletions.
diff --git a/sdk/src/main/java/com/amplitude/experiment/DefaultExperimentClient.kt b/sdk/src/main/java/com/amplitude/experiment/DefaultExperimentClient.kt
@@ -394,7 +394,7 @@ internal class DefaultExperimentClient internal constructor(
         return variants
     }
 
-    private fun storeVariants(variants: Map<String, Variant>, options: FetchOptions?) = synchronized(variants) {
+    private fun storeVariants(variants: Map<String, Variant>, options: FetchOptions?) {
         val failedFlagKeys = options?.flagKeys?.toMutableList() ?: mutableListOf()
         synchronized(this.variants) {
             if (options?.flagKeys == null) {
@@ -671,8 +671,6 @@ enum class VariantSource(val type: String) {
     }
 
     fun isFallback(): Boolean {
-        return this == FALLBACK_INLINE ||
-            this == FALLBACK_CONFIG ||
-            this == SECONDARY_INITIAL_VARIANTS
+        return this == FALLBACK_INLINE || this == FALLBACK_CONFIG || this == SECONDARY_INITIAL_VARIANTS
     }
 }
diff --git a/sdk/src/main/java/com/amplitude/experiment/util/Variant.kt b/sdk/src/main/java/com/amplitude/experiment/util/Variant.kt
@@ -57,14 +57,8 @@ internal fun JSONObject?.toVariant(): Variant? {
 
         val payload = when {
             has("payload") -> {
-                val payload = get("payload")
-                if (payload is JSONObject) {
-                    payload.toMap()
-                } else {
-                    payload
-                }
+                get("payload")
             }
-
             else -> null
         }
 
@@ -102,7 +96,15 @@ internal fun EvaluationVariant.convertToVariant(): Variant {
         else -> null
     }
     val payload = when {
-        this.payload != null -> this.payload
+        this.payload != null -> {
+            if (this.payload is Map<*, *>) {
+                this.payload.toJSONObject()
+            } else if (this.payload is Collection<*>) {
+                this.payload.toJSONArray()
+            } else {
+                this.payload
+            }
+        }
         else -> null
     }
     val metadata = when {

diff --git a/sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt b/sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt
@@ -14,6 +14,8 @@ import io.mockk.spyk
 import io.mockk.verify
 import okhttp3.HttpUrl.Companion.toHttpUrl
 import okhttp3.OkHttpClient
+import org.json.JSONArray
+import org.json.JSONObject
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
@@ -1120,4 +1122,34 @@ class ExperimentClientTest {
         spyClient.start(null).get()
         verify(exactly = 0) { spyClient.fetchInternal(any(), any(), any(), any()) }
     }
+
+    @Test
+    fun `LocalEvaluation - test payload format`() {
+        val user = ExperimentUser(userId = "test_user")
+        val exposureTrackingProvider = mockk<TestExposureTrackingProvider>()
+        every { exposureTrackingProvider.track(any()) } just Runs
+        val client = DefaultExperimentClient(
+            API_KEY,
+            ExperimentConfig(
+                debug = true,
+                exposureTrackingProvider = exposureTrackingProvider,
+                fetchOnStart = false,
+                source = Source.LOCAL_STORAGE,
+            ),
+            OkHttpClient(),
+            mockStorage,
+            Experiment.executorService,
+        )
+        client.start(user).get()
+        var variant = client.variant("sdk-payload-ci-test")
+        val obj = JSONObject().put("key1", "val1").put("key2", "val2")
+        val array = JSONArray().put(JSONObject().put("key1", "obj1")).put(JSONObject().put("key2", "obj2"))
+        Assert.assertEquals(obj::class, variant.payload!!::class)
+        Assert.assertEquals(obj.toString(), variant.payload.toString())
+        // set null user to get array variant
+        client.start(ExperimentUser()).get()
+        variant = client.variant("sdk-payload-ci-test")
+        Assert.assertEquals(array::class, variant.payload!!::class)
+        Assert.assertEquals(array.toString(), variant.payload.toString())
+    }
 }
diff --git a/sdk/src/test/java/com/amplitude/experiment/StorageTest.kt b/sdk/src/test/java/com/amplitude/experiment/StorageTest.kt
@@ -25,8 +25,8 @@ class StorageTest {
             )
         ).toString()
         Assert.assertEquals(
-            Variant(key = "on", value = "on", payload = mapOf("k" to "v")),
-            decodeVariantFromStorage(storedVariant)
+            Variant(key = "on", value = "on", payload = JSONObject(mapOf("k" to "v"))).toString(),
+            decodeVariantFromStorage(storedVariant).toString()
         )
     }
 
@@ -43,11 +43,11 @@ class StorageTest {
             Variant(
                 key = "on",
                 value = "on",
-                payload = mapOf("k" to "v"),
+                payload = JSONObject(mapOf("k" to "v")),
                 expKey = "exp-1",
                 metadata = mapOf("experimentKey" to "exp-1")
-            ),
-            decodeVariantFromStorage(storedVariant)
+            ).toString(),
+            decodeVariantFromStorage(storedVariant).toString()
         )
     }
 
@@ -78,9 +78,9 @@ class StorageTest {
             Variant(
                 key = "treatment",
                 value = "on",
-                payload = mapOf("k" to "v")
-            ),
-            decodeVariantFromStorage(storedVariant)
+                payload = JSONObject(mapOf("k" to "v"))
+            ).toString(),
+            decodeVariantFromStorage(storedVariant).toString()
         )
     }
 
@@ -98,11 +98,11 @@ class StorageTest {
             Variant(
                 key = "treatment",
                 value = "on",
-                payload = mapOf("k" to "v"),
+                payload = JSONObject(mapOf("k" to "v")),
                 expKey = "exp-1",
                 metadata = mapOf("experimentKey" to "exp-1")
-            ),
-            decodeVariantFromStorage(storedVariant)
+            ).toString(),
+            decodeVariantFromStorage(storedVariant).toString()
         )
     }
 
@@ -120,11 +120,11 @@ class StorageTest {
             Variant(
                 key = "treatment",
                 value = "on",
-                payload = mapOf("k" to "v"),
+                payload = JSONObject(mapOf("k" to "v")),
                 expKey = "exp-1",
                 metadata = mapOf("experimentKey" to "exp-1")
-            ),
-            decodeVariantFromStorage(storedVariant)
+            ).toString(),
+            decodeVariantFromStorage(storedVariant).toString()
         )
     }
 }