From 0b25b31897176976f534bdf99ce8491ab35a4a0f Mon Sep 17 00:00:00 2001 From: sroyal-statsig <76536058+sroyal-statsig@users.noreply.github.com> Date: Thu, 7 Mar 2024 10:20:58 -0800 Subject: [PATCH] Add Evaluation Callback Option (#195) * Add Evaluation Callback Option * fix lint * fix more lint * One more lint fix * fix tests * update test * update * update * update --- .../java/com/statsig/androidsdk/BaseConfig.kt | 14 +++ .../com/statsig/androidsdk/DynamicConfig.kt | 10 +- .../com/statsig/androidsdk/FeatureGate.kt | 42 +++++++ .../statsig/androidsdk/InitializeResponse.kt | 21 ---- src/main/java/com/statsig/androidsdk/Layer.kt | 10 +- .../com/statsig/androidsdk/StatsigClient.kt | 45 ++++--- .../com/statsig/androidsdk/StatsigLogger.kt | 12 +- .../com/statsig/androidsdk/StatsigOptions.kt | 2 + src/main/java/com/statsig/androidsdk/Store.kt | 4 +- .../androidsdk/EvaluationCallbackTest.kt | 112 ++++++++++++++++++ .../statsig/androidsdk/StatsigCacheTest.kt | 2 +- .../java/com/statsig/androidsdk/StoreTest.kt | 4 +- 12 files changed, 209 insertions(+), 69 deletions(-) create mode 100644 src/main/java/com/statsig/androidsdk/BaseConfig.kt create mode 100644 src/main/java/com/statsig/androidsdk/FeatureGate.kt create mode 100644 src/test/java/com/statsig/androidsdk/EvaluationCallbackTest.kt diff --git a/src/main/java/com/statsig/androidsdk/BaseConfig.kt b/src/main/java/com/statsig/androidsdk/BaseConfig.kt new file mode 100644 index 0000000..5aaab01 --- /dev/null +++ b/src/main/java/com/statsig/androidsdk/BaseConfig.kt @@ -0,0 +1,14 @@ +package com.statsig.androidsdk + +open class BaseConfig( + private val name: String, + private val details: EvaluationDetails, +) { + open fun getName(): String { + return this.name + } + + open fun getEvaluationDetails(): EvaluationDetails { + return this.details + } +} diff --git a/src/main/java/com/statsig/androidsdk/DynamicConfig.kt b/src/main/java/com/statsig/androidsdk/DynamicConfig.kt index 3b058f5..22b6c46 100644 --- a/src/main/java/com/statsig/androidsdk/DynamicConfig.kt +++ b/src/main/java/com/statsig/androidsdk/DynamicConfig.kt @@ -14,7 +14,7 @@ class DynamicConfig( private val isExperimentActive: Boolean = false, private val isDeviceBased: Boolean = false, private val allocatedExperimentName: String? = null, -) { +) : BaseConfig(name, details) { internal constructor( configName: String, apiDynamicConfig: APIDynamicConfig, @@ -175,10 +175,6 @@ class DynamicConfig( return this.jsonValue } - fun getName(): String { - return this.name - } - fun getIsUserInExperiment(): Boolean { return this.isUserInExperiment } @@ -187,10 +183,6 @@ class DynamicConfig( return this.isExperimentActive } - fun getEvaluationDetails(): EvaluationDetails { - return this.details - } - fun getRuleID(): String { return this.rule } diff --git a/src/main/java/com/statsig/androidsdk/FeatureGate.kt b/src/main/java/com/statsig/androidsdk/FeatureGate.kt new file mode 100644 index 0000000..79d841b --- /dev/null +++ b/src/main/java/com/statsig/androidsdk/FeatureGate.kt @@ -0,0 +1,42 @@ +package com.statsig.androidsdk + +/** + * A helper class for interfacing with Feature Gate defined in the Statsig console + */ +class FeatureGate( + private val name: String, + private val details: EvaluationDetails, + private val value: Boolean, + private val rule: String = "", + private val groupName: String? = null, + private val secondaryExposures: Array> = arrayOf(), +) : BaseConfig(name, details) { + internal constructor( + gateName: String, + apiFeatureGate: APIFeatureGate, + evalDetails: EvaluationDetails, + ) : this( + gateName, + evalDetails, + apiFeatureGate.value, + apiFeatureGate.ruleID, + apiFeatureGate.groupName, + apiFeatureGate.secondaryExposures, + ) + + fun getValue(): Boolean { + return this.value + } + + fun getRuleID(): String { + return this.rule + } + + fun getGroupName(): String? { + return this.groupName + } + + fun getSecondaryExposures(): Array> { + return this.secondaryExposures + } +} diff --git a/src/main/java/com/statsig/androidsdk/InitializeResponse.kt b/src/main/java/com/statsig/androidsdk/InitializeResponse.kt index fe1ceff..dac8046 100644 --- a/src/main/java/com/statsig/androidsdk/InitializeResponse.kt +++ b/src/main/java/com/statsig/androidsdk/InitializeResponse.kt @@ -48,24 +48,3 @@ internal data class APIDynamicConfig( @SerializedName("allocated_experiment_name") val allocatedExperimentName: String? = null, @SerializedName("explicit_parameters") val explicitParameters: Array = arrayOf(), ) - -internal data class FeatureGate( - val name: String, - val details: EvaluationDetails, - val value: Boolean = false, - val ruleID: String = "", - val groupName: String? = null, - val secondaryExposures: Array> = arrayOf(), -) { - constructor( - apiFeatureGate: APIFeatureGate, - evalDetails: EvaluationDetails, - ) : this( - apiFeatureGate.name, - evalDetails, - apiFeatureGate.value, - apiFeatureGate.ruleID, - apiFeatureGate.groupName, - apiFeatureGate.secondaryExposures, - ) -} diff --git a/src/main/java/com/statsig/androidsdk/Layer.kt b/src/main/java/com/statsig/androidsdk/Layer.kt index 35c0030..e4ccaa4 100644 --- a/src/main/java/com/statsig/androidsdk/Layer.kt +++ b/src/main/java/com/statsig/androidsdk/Layer.kt @@ -17,7 +17,7 @@ class Layer internal constructor( private val isDeviceBased: Boolean = false, private val allocatedExperimentName: String? = null, private val explicitParameters: Set? = null, -) { +) : BaseConfig(name, details) { internal constructor( client: StatsigClient?, layerName: String, @@ -142,10 +142,6 @@ class Layer internal constructor( } } - fun getName(): String { - return this.name - } - fun getIsUserInExperiment(): Boolean { return this.isUserInExperiment } @@ -154,10 +150,6 @@ class Layer internal constructor( return this.isExperimentActive } - fun getEvaluationDetails(): EvaluationDetails { - return this.details - } - fun getRuleID(): String { return this.rule } diff --git a/src/main/java/com/statsig/androidsdk/StatsigClient.kt b/src/main/java/com/statsig/androidsdk/StatsigClient.kt index 066d20e..5c467d3 100644 --- a/src/main/java/com/statsig/androidsdk/StatsigClient.kt +++ b/src/main/java/com/statsig/androidsdk/StatsigClient.kt @@ -155,9 +155,11 @@ class StatsigClient() : LifecycleEventListener { var result = false errorBoundary.capture({ val gate = store.checkGate(gateName) - result = gate.value + options.evaluationCallback?.invoke(gate) + result = gate.getValue() logExposure(gateName, gate) }, tag = functionName, configName = gateName) + options.evaluationCallback?.invoke(FeatureGate(gateName, EvaluationDetails(EvaluationReason.Uninitialized), false, "")) return result } @@ -175,8 +177,10 @@ class StatsigClient() : LifecycleEventListener { errorBoundary.capture({ this.logger.addNonExposedCheck(gateName) val gate = store.checkGate(gateName) - result = gate.value + options.evaluationCallback?.invoke(gate) + result = gate.getValue() }, tag = functionName, configName = gateName) + options.evaluationCallback?.invoke(FeatureGate(gateName, EvaluationDetails(EvaluationReason.Uninitialized), false, "")) return result } @@ -190,12 +194,13 @@ class StatsigClient() : LifecycleEventListener { fun getConfig(configName: String): DynamicConfig { val functionName = "getConfig" enforceInitialized(functionName) - var result: DynamicConfig? = null + var result: DynamicConfig = DynamicConfig.getUninitialized(configName) errorBoundary.capture({ result = store.getConfig(configName) - logExposure(configName, result!!) + logExposure(configName, result) }, tag = functionName, configName = configName) - return result ?: DynamicConfig.getUninitialized(configName) + options.evaluationCallback?.invoke(result) + return result } /** @@ -208,12 +213,13 @@ class StatsigClient() : LifecycleEventListener { fun getConfigWithExposureLoggingDisabled(configName: String): DynamicConfig { val functionName = "getConfigWithExposureLoggingDisabled" enforceInitialized(functionName) - var result: DynamicConfig? = null + var result: DynamicConfig = DynamicConfig.getUninitialized(configName) errorBoundary.capture({ this.logger.addNonExposedCheck(configName) result = store.getConfig(configName) }, tag = functionName, configName = configName) - return result ?: DynamicConfig.getUninitialized(configName) + options.evaluationCallback?.invoke(result) + return result } /** @@ -227,14 +233,14 @@ class StatsigClient() : LifecycleEventListener { fun getExperiment(experimentName: String, keepDeviceValue: Boolean = false): DynamicConfig { val functionName = "getExperiment" enforceInitialized(functionName) - var res: DynamicConfig? = null + var res: DynamicConfig = DynamicConfig.getUninitialized(experimentName) errorBoundary.capture({ res = store.getExperiment(experimentName, keepDeviceValue) updateStickyValues() - logExposure(experimentName, res!!) + logExposure(experimentName, res) }, tag = functionName, configName = experimentName) - - return res ?: DynamicConfig.getUninitialized(experimentName) + options.evaluationCallback?.invoke(res) + return res } /** @@ -251,13 +257,14 @@ class StatsigClient() : LifecycleEventListener { ): DynamicConfig { val functionName = "getExperimentWithExposureLoggingDisabled" enforceInitialized(functionName) - var exp: DynamicConfig? = null + var exp: DynamicConfig = DynamicConfig.getUninitialized(experimentName) errorBoundary.capture({ this.logger.addNonExposedCheck(experimentName) exp = store.getExperiment(experimentName, keepDeviceValue) updateStickyValues() }, configName = experimentName, tag = functionName) - return exp ?: DynamicConfig.getUninitialized(experimentName) + options.evaluationCallback?.invoke(exp) + return exp } /** @@ -271,13 +278,13 @@ class StatsigClient() : LifecycleEventListener { fun getLayer(layerName: String, keepDeviceValue: Boolean = false): Layer { val functionName = "getLayer" enforceInitialized(functionName) - var layer: Layer? = null + var layer: Layer = Layer.getUninitialized(layerName) errorBoundary.capture({ layer = store.getLayer(this, layerName, keepDeviceValue) updateStickyValues() }, tag = functionName, configName = layerName) - - return layer ?: Layer.getUninitialized(layerName) + options.evaluationCallback?.invoke(layer) + return layer } /** @@ -294,14 +301,14 @@ class StatsigClient() : LifecycleEventListener { ): Layer { val functionName = "getLayerWithExposureLoggingDisabled" enforceInitialized(functionName) - var layer: Layer? = null + var layer: Layer = Layer.getUninitialized(layerName) errorBoundary.capture({ this.logger.addNonExposedCheck(layerName) layer = store.getLayer(null, layerName, keepDeviceValue) updateStickyValues() }, tag = functionName, configName = layerName) - - return layer ?: Layer.getUninitialized(layerName) + options.evaluationCallback?.invoke(layer) + return layer } /** diff --git a/src/main/java/com/statsig/androidsdk/StatsigLogger.kt b/src/main/java/com/statsig/androidsdk/StatsigLogger.kt index 9a537f9..4c52439 100644 --- a/src/main/java/com/statsig/androidsdk/StatsigLogger.kt +++ b/src/main/java/com/statsig/androidsdk/StatsigLogger.kt @@ -79,7 +79,7 @@ internal class StatsigLogger( } fun logExposure(name: String, gate: FeatureGate, user: StatsigUser, isManual: Boolean) { - val dedupeKey = name + gate.value + gate.ruleID + gate.details.reason.toString() + val dedupeKey = name + gate.getValue() + gate.getRuleID() + gate.getEvaluationDetails().reason.toString() if (!shouldLogExposure(dedupeKey)) { return } @@ -90,15 +90,15 @@ internal class StatsigLogger( val metadata = mutableMapOf( "gate" to name, - "gateValue" to gate.value.toString(), - "ruleID" to gate.ruleID, - "reason" to gate.details.reason.toString(), - "time" to gate.details.time.toString(), + "gateValue" to gate.getValue().toString(), + "ruleID" to gate.getRuleID(), + "reason" to gate.getEvaluationDetails().reason.toString(), + "time" to gate.getEvaluationDetails().time.toString(), ) addManualFlag(metadata, isManual) event.metadata = metadata - event.secondaryExposures = gate.secondaryExposures + event.secondaryExposures = gate.getSecondaryExposures() log(event) } } diff --git a/src/main/java/com/statsig/androidsdk/StatsigOptions.kt b/src/main/java/com/statsig/androidsdk/StatsigOptions.kt index 64ae8da..1808e3d 100644 --- a/src/main/java/com/statsig/androidsdk/StatsigOptions.kt +++ b/src/main/java/com/statsig/androidsdk/StatsigOptions.kt @@ -83,6 +83,8 @@ class StatsigOptions( * Note: This requires special authorization from Statsig */ @SerializedName("disableHashing") var disableHashing: Boolean? = false, + + var evaluationCallback: ((BaseConfig) -> Unit)? = null, ) { private var environment: MutableMap? = null diff --git a/src/main/java/com/statsig/androidsdk/Store.kt b/src/main/java/com/statsig/androidsdk/Store.kt index ef38b7a..81b3658 100644 --- a/src/main/java/com/statsig/androidsdk/Store.kt +++ b/src/main/java/com/statsig/androidsdk/Store.kt @@ -185,8 +185,8 @@ internal class Store(private val statsigScope: CoroutineScope, private val share val hashName = Hashing.getHashedString(gateName, currentCache.values.hashUsed) val gate = currentCache.values.featureGates?.get(hashName) - ?: return FeatureGate(gateName, getEvaluationDetails(false)) - return FeatureGate(gate, getEvaluationDetails(true)) + ?: return FeatureGate(gateName, getEvaluationDetails(false), false) + return FeatureGate(gateName, gate, getEvaluationDetails(true)) } fun getConfig(configName: String): DynamicConfig { diff --git a/src/test/java/com/statsig/androidsdk/EvaluationCallbackTest.kt b/src/test/java/com/statsig/androidsdk/EvaluationCallbackTest.kt new file mode 100644 index 0000000..4c3f440 --- /dev/null +++ b/src/test/java/com/statsig/androidsdk/EvaluationCallbackTest.kt @@ -0,0 +1,112 @@ +package com.statsig.androidsdk + +import android.app.Application +import io.mockk.coEvery +import io.mockk.mockk +import io.mockk.unmockkAll +import org.junit.After +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test + +class EvaluationCallbackTest { + + private lateinit var app: Application + private var flushedLogs: String = "" + private var initUser: StatsigUser? = null + private var client: StatsigClient = StatsigClient() + private lateinit var network: StatsigNetwork + private lateinit var testSharedPrefs: TestSharedPreferences + private var checkedGate = "" + private var checkedConfig = "" + private var checkedLayer = "" + + @Before + internal fun setup() { + TestUtil.mockDispatchers() + + app = mockk() + testSharedPrefs = TestUtil.stubAppFunctions(app) + + TestUtil.mockStatsigUtil() + + network = TestUtil.mockNetwork(captureUser = { user -> + initUser = user + }) + + coEvery { + network.apiPostLogs(any(), any(), any()) + } answers { + flushedLogs = secondArg() + } + } + + @After + internal fun tearDown() { + unmockkAll() + } + + @Test + fun testInitialize() { + val user = StatsigUser("123") + val now = System.currentTimeMillis() + user.customIDs = mapOf("random_id" to "abcde") + + fun evalCallback(config: BaseConfig) { + if (config is FeatureGate) { + checkedGate = config.getName() + } else if (config is DynamicConfig) { + checkedConfig = config.getName() + } else if (config is Layer) { + checkedLayer = config.getName() + } + } + + val evalCallback: (BaseConfig) -> Unit = ::evalCallback + + TestUtil.startStatsigAndWait(app, user, StatsigOptions(overrideStableID = "custom_stable_id", evaluationCallback = evalCallback), network = network) + client = Statsig.client + + assertTrue(client.checkGate("always_on")) + assertEquals("always_on", checkedGate) + assertTrue(client.checkGateWithExposureLoggingDisabled("always_on_v2")) + assertEquals("always_on_v2", checkedGate) + assertFalse(client.checkGateWithExposureLoggingDisabled("a_different_gate")) + assertEquals("a_different_gate", checkedGate) + assertFalse(client.checkGate("always_off")) + assertEquals("always_off", checkedGate) + assertFalse(client.checkGate("not_a_valid_gate_name")) + assertEquals("not_a_valid_gate_name", checkedGate) + + client.getConfig("test_config") + assertEquals("test_config", checkedConfig) + + client.getConfigWithExposureLoggingDisabled("a_different_config") + assertEquals("a_different_config", checkedConfig) + + client.getConfig("not_a_valid_config") + assertEquals("not_a_valid_config", checkedConfig) + + client.getExperiment("exp") + assertEquals("exp", checkedConfig) + + client.getExperimentWithExposureLoggingDisabled("exp_other") + assertEquals("exp_other", checkedConfig) + + client.getLayer("layer") + assertEquals("layer", checkedLayer) + + client.getLayerWithExposureLoggingDisabled("layer_other") + assertEquals("layer_other", checkedLayer) + + // check a few previously checked gate and config; they should not result in exposure logs due to deduping logic + client.checkGate("always_on") + assertEquals("always_on", checkedGate) + client.getConfig("test_config") + assertEquals("test_config", checkedConfig) + client.getExperiment("exp") + assertEquals("exp", checkedConfig) + + client.shutdown() + } +} diff --git a/src/test/java/com/statsig/androidsdk/StatsigCacheTest.kt b/src/test/java/com/statsig/androidsdk/StatsigCacheTest.kt index 0fb35f6..93b9fd2 100644 --- a/src/test/java/com/statsig/androidsdk/StatsigCacheTest.kt +++ b/src/test/java/com/statsig/androidsdk/StatsigCacheTest.kt @@ -55,7 +55,7 @@ class StatsigCacheTest { TestUtil.startStatsigAndDontWait(app, user, StatsigOptions()) client = Statsig.client assertTrue(client.isInitialized()) - assertEquals(EvaluationReason.Cache, client.getStore().checkGate("always_on").details.reason) + assertEquals(EvaluationReason.Cache, client.getStore().checkGate("always_on").getEvaluationDetails().reason) assertTrue(client.checkGate("always_on")) runBlocking { diff --git a/src/test/java/com/statsig/androidsdk/StoreTest.kt b/src/test/java/com/statsig/androidsdk/StoreTest.kt index ef32611..ed27b85 100644 --- a/src/test/java/com/statsig/androidsdk/StoreTest.kt +++ b/src/test/java/com/statsig/androidsdk/StoreTest.kt @@ -113,10 +113,10 @@ class StoreTest { store.save(getInitValue("v0", inExperiment = false, active = true), userDloomb) store.loadAndResetForUser(userJkw) - assertEquals(true, store.checkGate("gate").value) + assertEquals(true, store.checkGate("gate").getValue()) store.loadAndResetForUser(userDloomb) - assertEquals(false, store.checkGate("gate").value) + assertEquals(false, store.checkGate("gate").getValue()) } @Test