Skip to content

Commit

Permalink
Update Variant class and use new /sdk/v2/vardata API (#28)
Browse files Browse the repository at this point in the history
  • Loading branch information
tyiuhc authored Sep 14, 2023
1 parent db317fd commit c4c27b4
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ jobs:
key: ${{ runner.os }}-gradle-caches-${{ hashFiles('**/*.gradle', '**/*.gradle.kts') }}

- name: Test
run: ./gradlew test --info
run: ./gradlew test --info --stacktrace
5 changes: 5 additions & 0 deletions analytics-connector/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ android {
proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'
}
}

compileOptions {
sourceCompatibility 1.8
targetCompatibility 1.8
}
}

dependencies {
Expand Down
5 changes: 5 additions & 0 deletions sdk/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ android {
proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'
}
}

compileOptions {
sourceCompatibility 1.8
targetCompatibility 1.8
}
}

dependencies {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ internal class DefaultExperimentClient internal constructor(
val exposedUser = getUserMergedWithProvider()
val event = OldExposureEvent(exposedUser, key, variant, source)
// Track the exposure event if an analytics provider is set
if (source.isFallback() || variant.value == null) {
if (source.isFallback() || variant.key == null) {
userSessionExposureTracker?.track(Exposure(key, null, variant.expKey), exposedUser)
analyticsProvider?.unsetUserProperty(event)
} else {
userSessionExposureTracker?.track(Exposure(key, variant.value, variant.expKey), exposedUser)
userSessionExposureTracker?.track(Exposure(key, variant.key, variant.expKey), exposedUser)
analyticsProvider?.setUserProperty(event)
analyticsProvider?.track(event)
}
Expand Down Expand Up @@ -208,7 +208,7 @@ internal class DefaultExperimentClient internal constructor(
.toByteString()
.base64Url()
val url = serverUrl.newBuilder()
.addPathSegments("sdk/vardata")
.addPathSegments("sdk/v2/vardata")
.build()
val builder = Request.Builder()
.get()
Expand Down
17 changes: 13 additions & 4 deletions sdk/src/main/java/com/amplitude/experiment/Variant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,19 @@ data class Variant @JvmOverloads constructor(
* The experiment key. Used to distinguish two experiments associated with the same flag.
*/
@JvmField val expKey: String? = null,
/**
* The key of the variant.
*/
@JvmField val key: String? = null,
/**
* 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()
) {

/**
* Useful for comparing a variant's value to a string in java.
* Useful for comparing a variant's key to a string in java.
*
* ```
* variant.is("on");
Expand All @@ -25,10 +34,10 @@ data class Variant @JvmOverloads constructor(
* is equivalent to
*
* ```
* "on".equals(variant.value);
* "on".equals(variant.key);
* ```
*
* @param value The value to compare with the value of this variant.
* @param value The value to compare with the key of this variant.
*/
fun `is`(value: String): Boolean = this.value == value
fun `is`(value: String): Boolean = this.key == value
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ class ExposureEvent(
override val name: String = "[Experiment] Exposure"
override val properties: Map<String, String?> = mapOf(
"key" to key,
"variant" to variant.value,
"variant" to variant.key,
"source" to source.toString(),
)
override val userProperties: Map<String, Any?> = mapOf(
"[Experiment] $key" to variant.value
"[Experiment] $key" to variant.key
)
override val userProperty: String = "[Experiment] $key"
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ internal class SessionAnalyticsProvider(
private val unsetProperties = mutableSetOf<String>()

override fun track(event: ExperimentAnalyticsEvent) {
val variant = event.variant.value ?: return
val variant = event.variant.key ?: return
synchronized(lock) {
if (setProperties[event.key] == variant) {
return
Expand All @@ -25,7 +25,7 @@ internal class SessionAnalyticsProvider(
}

override fun setUserProperty(event: ExperimentAnalyticsEvent) {
val variant = event.variant.value ?: return
val variant = event.variant.key ?: return
synchronized(lock) {
if (setProperties[event.key] == variant) {
return@setUserProperty
Expand Down
18 changes: 14 additions & 4 deletions sdk/src/main/java/com/amplitude/experiment/util/Variant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import org.json.JSONObject
internal fun Variant.toJson(): String {
val jsonObject = JSONObject()
try {
jsonObject.put("value", value)
jsonObject.put("key", key)
if (value != null) {
jsonObject.put("value", value)
}
if (payload != null) {
jsonObject.put("payload", payload)
}
Expand All @@ -32,11 +35,14 @@ internal fun JSONObject?.toVariant(): Variant? {
return if (this == null) {
return null
} else try {
val value = when {
has("value") -> getString("value")
val key = when {
has("key") -> getString("key")
else -> return null
}
val value = when {
has("value") -> getString("value")
else -> null
}
val payload = when {
has("payload") -> get("payload")
else -> null
Expand All @@ -45,7 +51,11 @@ internal fun JSONObject?.toVariant(): Variant? {
has("expKey") -> getString("expKey")
else -> null
}
Variant(value, payload, expKey)
val metadata = when {
has("metadata") -> getJSONObject("metadata").toMap()
else -> emptyMap()
}
Variant(value, payload, expKey, key, metadata)
} catch (e: JSONException) {
Logger.w("Error parsing Variant from json string $this")
return null
Expand Down
24 changes: 13 additions & 11 deletions sdk/src/test/java/com/amplitude/experiment/ExperimentClientTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ class ExperimentClientTest {

private val testUser = ExperimentUser(userId = "test_user")

private val serverVariant = Variant("on", "payload")
private val fallbackVariant = Variant("fallback", "payload")
private val initialVariant = Variant("initial")
private val serverVariant = Variant(key = "on", value = "on", payload = "payload")
private val fallbackVariant = Variant(key = "fallback", payload = "payload")
private val initialVariant = Variant(key = "initial")

private val initialVariants = mapOf(
INITIAL_KEY to initialVariant,
KEY to Variant("off"),
KEY to Variant(key = "off"),
)

private val client = DefaultExperimentClient(
Expand Down Expand Up @@ -94,7 +94,7 @@ class ExperimentClientTest {
} catch (e: ExecutionException) {
// Timeout is expected
val variant = timeoutClient.variant(KEY)
Assert.assertEquals("off", variant.value)
Assert.assertEquals("off", variant.key)
return
}
Assert.fail("expected timeout exception")
Expand All @@ -107,7 +107,7 @@ class ExperimentClientTest {
} catch (e: ExecutionException) {
// Timeout is expected
val offVariant = timeoutClient.variant(KEY)
Assert.assertEquals("off", offVariant.value)
Assert.assertEquals("off", offVariant.key)
// Wait for retry to succeed and check updated variant
Thread.sleep(1000)
val variant = timeoutClient.variant(KEY)
Expand Down Expand Up @@ -161,7 +161,7 @@ class ExperimentClientTest {
fun `clear the flag config in storage`() {
generalClient.fetch(testUser).get()
val variant = generalClient.variant("sdk-ci-test")
Assert.assertEquals(Variant("on", "payload"), variant)
Assert.assertEquals(Variant(key = "on", value = "on", payload = "payload"), variant)
generalClient.clear()
val clearedVariants = generalClient.all()
Assert.assertEquals(0, clearedVariants.entries.size)
Expand All @@ -174,7 +174,7 @@ class ExperimentClientTest {
initialVariantSourceClient.fetch(testUser).get()
variant = initialVariantSourceClient.variant(KEY)
Assert.assertNotNull(variant)
Assert.assertEquals("off", variant.value)
Assert.assertEquals("off", variant.key)
Assert.assertNull(variant.payload)
}

Expand Down Expand Up @@ -221,7 +221,7 @@ class ExperimentClientTest {
Assert.assertEquals(
mapOf(
"key" to KEY,
"variant" to serverVariant.value,
"variant" to serverVariant.key,
"source" to VariantSource.LOCAL_STORAGE.toString()
),
event.properties
Expand Down Expand Up @@ -320,6 +320,7 @@ class ExperimentClientTest {
override fun setUserProperty(event: ExperimentAnalyticsEvent) {
Assert.fail("analytics provider set() should not be called.")
}

override fun unsetUserProperty(event: ExperimentAnalyticsEvent) {
Assert.assertEquals("[Experiment] $INITIAL_KEY", event.userProperty)
}
Expand Down Expand Up @@ -349,7 +350,7 @@ class ExperimentClientTest {
override fun track(event: ExperimentAnalyticsEvent) {
Assert.assertEquals(
event.userProperties,
mapOf("[Experiment] $KEY" to serverVariant.value)
mapOf("[Experiment] $KEY" to serverVariant.key)
)
didExposureGetTracked = true
}
Expand All @@ -361,6 +362,7 @@ class ExperimentClientTest {
)
didUserPropertyGetSet = true
}

override fun unsetUserProperty(event: ExperimentAnalyticsEvent) {
Assert.fail("analytics provider unset() should not be called")
}
Expand Down Expand Up @@ -398,7 +400,7 @@ class ExperimentClientTest {
debug = true,
exposureTrackingProvider = exposureTrackingProvider,
source = Source.INITIAL_VARIANTS,
initialVariants = mapOf("flagKey" to Variant("variant", null, "experimentKey"))
initialVariants = mapOf("flagKey" to Variant(key = "variant", expKey = "experimentKey"))
),
OkHttpClient(),
InMemoryStorage(),
Expand Down
17 changes: 5 additions & 12 deletions sdk/src/test/java/com/amplitude/experiment/VariantTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,24 @@ class VariantTest {
@Test
fun `json object to variant`() {
val jsonObject = JSONObject()
jsonObject.put("key", "key")
jsonObject.put("value", "value")
jsonObject.put("payload", "payload")
jsonObject.put("expKey", "expKey")
val variant = jsonObject.toVariant()
Assert.assertNotNull(variant)
Assert.assertEquals("value", variant!!.value)
Assert.assertEquals("key", variant!!.key)
Assert.assertEquals("value", variant.value)
Assert.assertEquals("payload", variant.payload)
Assert.assertEquals("expKey", variant.expKey)
}

@Test
fun `json object to variant deprecated field`() {
val jsonObject = JSONObject()
jsonObject.put("key", "value")
val variant = jsonObject.toVariant()
Assert.assertNotNull(variant)
Assert.assertEquals("value", variant!!.value)
Assert.assertNull(variant.payload)
}

@Test
fun `variant to json object`() {
run {
val variant = Variant("value", null, "expKey")
val variant = Variant("value", null, "expKey", "key")
val jsonObject = JSONObject()
jsonObject.put("key", "key")
jsonObject.put("value", "value")
jsonObject.put("expKey", "expKey")
Assert.assertEquals(jsonObject.toString(), variant.toJson())
Expand Down

0 comments on commit c4c27b4

Please sign in to comment.