diff --git a/Confidence/api/Confidence.api b/Confidence/api/Confidence.api index 9b4f73a6..ddf1a43e 100644 --- a/Confidence/api/Confidence.api +++ b/Confidence/api/Confidence.api @@ -13,9 +13,13 @@ public final class com/spotify/confidence/Confidence : com/spotify/confidence/Co public fun putContext (Ljava/lang/String;Lcom/spotify/confidence/ConfidenceValue;)V public fun putContext (Ljava/util/Map;)V public final fun putContext (Ljava/util/Map;Ljava/util/List;)V + public final fun putContextAndWait (Ljava/util/Map;Ljava/util/List;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; + public static synthetic fun putContextAndWait$default (Lcom/spotify/confidence/Confidence;Ljava/util/Map;Ljava/util/List;JLkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object; public final fun putContextLocal (Ljava/util/Map;)V public fun removeContext (Ljava/lang/String;)V public fun removeContext (Ljava/util/Collection;)V + public final fun removeContextAndWait (Ljava/util/Collection;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; + public static synthetic fun removeContextAndWait$default (Lcom/spotify/confidence/Confidence;Ljava/util/Collection;JLkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object; public fun stop ()V public fun track (Lcom/spotify/confidence/Producer;)V public fun track (Ljava/lang/String;Ljava/util/Map;)V diff --git a/Confidence/src/main/java/com/spotify/confidence/Confidence.kt b/Confidence/src/main/java/com/spotify/confidence/Confidence.kt index 232adcc8..92b2847c 100644 --- a/Confidence/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Confidence/src/main/java/com/spotify/confidence/Confidence.kt @@ -43,7 +43,8 @@ class Confidence internal constructor( private val parent: ConfidenceContextProvider? = null, private val region: ConfidenceRegion = ConfidenceRegion.GLOBAL, private val debugLogger: DebugLogger?, - internal val telemetry: Telemetry = Telemetry(SDK_ID, Telemetry.Library.CONFIDENCE, SDK_VERSION) + internal val telemetry: Telemetry = Telemetry(SDK_ID, Telemetry.Library.CONFIDENCE, SDK_VERSION), + private val reconciliationTimeoutMillis: Long = 10000 ) : Contextual, EventSender { private val removedKeys = mutableListOf() private val contextMap = MutableStateFlow(initialContext) @@ -67,7 +68,7 @@ class Confidence internal constructor( } } - suspend fun awaitReconciliation(timeoutMillis: Long = 5000) { + suspend fun awaitReconciliation(timeoutMillis: Long = reconciliationTimeoutMillis) { if (timeoutMillis <= 0) error("timeoutMillis need to be larger than 0") debugLogger?.logMessage("reconciliation started") yield() // will make sure that we respect other coroutine scopes triggered before this @@ -144,20 +145,14 @@ class Confidence internal constructor( @Synchronized override fun putContext(key: String, value: ConfidenceValue) { - val map = contextMap.value.toMutableMap() - map[key] = value - contextMap.value = map + updateContext(mapOf(key to value), emptyList(), "PutContext") triggerNewFlagFetch() - debugLogger?.logContext("PutContext", contextMap.value) } @Synchronized override fun putContext(context: Map) { - val map = contextMap.value.toMutableMap() - map += context - contextMap.value = map + updateContext(context, emptyList(), "PutContext") triggerNewFlagFetch() - debugLogger?.logContext("PutContext", contextMap.value) } /** @@ -167,11 +162,8 @@ class Confidence internal constructor( */ @Synchronized fun putContextLocal(context: Map) { - val map = contextMap.value.toMutableMap() - map += context - contextMap.value = map + updateContext(context, emptyList(), "putContextLocal") // No triggering of new flag fetch - debugLogger?.logContext("putContextLocal", contextMap.value) } /** @@ -186,6 +178,16 @@ class Confidence internal constructor( */ @Synchronized fun putContext(context: Map, removedKeys: List) { + updateContext(context, removedKeys, "PutContext") + triggerNewFlagFetch() + } + + @Synchronized + private fun updateContext( + context: Map, + removedKeys: Collection, + logAction: String + ) { val map = contextMap.value.toMutableMap() map += context for (key in removedKeys) { @@ -193,8 +195,60 @@ class Confidence internal constructor( } this.removedKeys.addAll(removedKeys) contextMap.value = map - triggerNewFlagFetch() - debugLogger?.logContext("PutContext", contextMap.value) + debugLogger?.logContext(logAction, contextMap.value) + } + + /** + * Mutates context, waits for flags to be fetched for the updated context, and activates them. + * + * Returns [Result.Failure] when reconciliation fails or times out. Any previously activated cached + * flags remain available for stale evaluation. + */ + suspend fun putContextAndWait( + context: Map, + removedKeys: List = emptyList(), + timeoutMillis: Long = reconciliationTimeoutMillis + ): Result = updateContextAndWait(context, removedKeys, "PutContext", timeoutMillis) + + /** + * Removes context keys, waits for flags to be fetched for the updated context, and activates them. + * + * Returns [Result.Failure] when reconciliation fails or times out. Any previously activated cached + * flags remain available for stale evaluation. + */ + suspend fun removeContextAndWait( + keys: Collection, + timeoutMillis: Long = reconciliationTimeoutMillis + ): Result = updateContextAndWait(emptyMap(), keys, "RemoveContext", timeoutMillis) + + private suspend fun updateContextAndWait( + context: Map, + removedKeys: Collection, + logAction: String, + timeoutMillis: Long + ): Result = kotlinx.coroutines.withContext(dispatcher) { + if (timeoutMillis <= 0) error("timeoutMillis need to be larger than 0") + currentFetchJob?.cancel().also { + currentFetchJob = null + } + updateContext(context, removedKeys, logAction) + val fetchResult = try { + withTimeout(timeoutMillis) { + fetchAndStore(failOnStaleResponse = true) + } + } catch (e: TimeoutCancellationException) { + debugLogger?.logMessage("timed out after $timeoutMillis") + Result.Failure(e) + } + try { + activate() + fetchResult + } catch (e: Exception) { + when (fetchResult) { + is Result.Success -> Result.Failure(e) + is Result.Failure -> fetchResult + } + } } private fun triggerNewFlagFetch() { @@ -213,14 +267,8 @@ class Confidence internal constructor( @Synchronized override fun removeContext(keys: Collection) { - val map = contextMap.value.toMutableMap() - for (key in keys) { - map.remove(key) - } - removedKeys.addAll(keys) - contextMap.value = map + updateContext(emptyMap(), keys, "RemoveContext") triggerNewFlagFetch() - debugLogger?.logContext("RemoveContext", contextMap.value) } override fun getContext(): Map = @@ -234,18 +282,19 @@ class Confidence internal constructor( } override fun withContext(context: Map): EventSender = Confidence( - clientSecret, - dispatcher, - eventSenderEngine, - diskStorage, - flagResolver, - cache, - mapOf(), - flagApplierClient, - this, - region, - debugLogger, - telemetry + clientSecret = clientSecret, + dispatcher = dispatcher, + eventSenderEngine = eventSenderEngine, + diskStorage = diskStorage, + flagResolver = flagResolver, + cache = cache, + initialContext = mapOf(), + flagApplierClient = flagApplierClient, + parent = this, + region = region, + debugLogger = debugLogger, + telemetry = telemetry, + reconciliationTimeoutMillis = reconciliationTimeoutMillis ).also { it.putContext(context) } @@ -276,29 +325,49 @@ class Confidence internal constructor( } } - private fun fetch(): Job = coroutineScope.launch(networkExceptionHandler) { + private suspend fun fetchAndStore(failOnStaleResponse: Boolean = false): Result { try { - val resolveResponse = resolve(listOf()) - if (resolveResponse is Result.Success) { - // we store the flag anyways except when the response was not modified - if (resolveResponse.data != FlagResolution.EMPTY) { - // Discard stale responses: if context changed during the - // in-flight request, the response is for an outdated context - if (resolveResponse.data.context == getContext()) { - diskStorage.store(resolveResponse.data) - } else { - debugLogger?.logMessage( - "Discarding stale resolve response: " + - "context changed during in-flight request", - isWarning = true - ) + return when (val resolveResponse = resolve(listOf())) { + is Result.Success -> { + val staleResponse = resolveResponse.data != FlagResolution.EMPTY && + resolveResponse.data.context != getContext() + when { + resolveResponse.data == FlagResolution.EMPTY -> Result.Success(Unit) + staleResponse -> { + val message = "Discarding stale resolve response: " + + "context changed during in-flight request" + debugLogger?.logMessage(message, isWarning = true) + if (failOnStaleResponse) { + Result.Failure(IllegalStateException(message)) + } else { + Result.Success(Unit) + } + } + else -> { + diskStorage.store(resolveResponse.data) + Result.Success(Unit) + } } } + is Result.Failure -> resolveResponse } } catch (e: ParseError) { - throw ParseError(e.message) + return Result.Failure(e) } catch (e: HttpError) { + return Result.Failure(e) + } catch (e: java.util.concurrent.CancellationException) { throw e + } catch (e: Exception) { + return Result.Failure(e) + } + } + + private fun fetch(): Job = coroutineScope.launch(networkExceptionHandler) { + when (val result = fetchAndStore()) { + is Result.Success -> Unit + is Result.Failure -> { + throw result.error + } } } @@ -568,7 +637,8 @@ object ConfidenceFactory { diskStorage = FileDiskStorage.create(context), flagApplierClient = flagApplierClient, debugLogger = debugLogger, - telemetry = telemetry + telemetry = telemetry, + reconciliationTimeoutMillis = timeoutMillis ) } } diff --git a/Confidence/src/test/java/com/spotify/confidence/ActivateAndFetchAsyncRaceConditionTest.kt b/Confidence/src/test/java/com/spotify/confidence/ActivateAndFetchAsyncRaceConditionTest.kt index aff28824..d618d1a2 100644 --- a/Confidence/src/test/java/com/spotify/confidence/ActivateAndFetchAsyncRaceConditionTest.kt +++ b/Confidence/src/test/java/com/spotify/confidence/ActivateAndFetchAsyncRaceConditionTest.kt @@ -253,4 +253,147 @@ internal class ActivateAndFetchAsyncRaceConditionTest { storedResolution ) } + + @Test + fun testPutContextAndWaitReturnsSuccessAndActivatesFetchedFlags() = runTest { + val testDispatcher = UnconfinedTestDispatcher(testScheduler) + val context = mapOf("targeting_key" to ConfidenceValue.String("user-new")) + + val flagResolver = object : FlagResolver { + override suspend fun resolve( + flags: List, + context: Map + ): Result { + return Result.Success( + FlagResolution( + context, + listOf( + ResolvedFlag( + "test-flag", + "flags/test-flag/variants/variant-1", + mutableMapOf("mystring" to ConfidenceValue.String("value-new")), + ResolveReason.RESOLVE_REASON_MATCH, + shouldApply = true + ) + ), + "token1" + ) + ) + } + } + + val confidence = getConfidence( + testDispatcher, + flagResolver = flagResolver + ) + whenever(flagApplierClient.apply(any(), any())).thenReturn(Result.Success(Unit)) + + val result = confidence.putContextAndWait(context) + + TestCase.assertTrue(result is Result.Success) + val eval = confidence.getFlag("test-flag.mystring", "default") + TestCase.assertEquals("value-new", eval.value) + TestCase.assertEquals(ResolveReason.RESOLVE_REASON_MATCH, eval.reason) + } + + @Test + fun testPutContextAndWaitReturnsFailureAndKeepsStaleCacheWhenFetchFails() = runTest { + val testDispatcher = UnconfinedTestDispatcher(testScheduler) + val context1 = mapOf("targeting_key" to ConfidenceValue.String("user-old")) + val context2 = mapOf("targeting_key" to ConfidenceValue.String("user-new")) + + val flagResolver = object : FlagResolver { + override suspend fun resolve( + flags: List, + context: Map + ): Result { + if (context["targeting_key"] == ConfidenceValue.String("user-new")) { + return Result.Failure(IllegalStateException("fetch failed")) + } + return Result.Success( + FlagResolution( + context, + listOf( + ResolvedFlag( + "test-flag", + "flags/test-flag/variants/variant-1", + mutableMapOf("mystring" to ConfidenceValue.String("value-old")), + ResolveReason.RESOLVE_REASON_MATCH, + shouldApply = true + ) + ), + "token1" + ) + ) + } + } + + val confidence = getConfidence( + testDispatcher, + flagResolver = flagResolver, + initialContext = context1 + ) + whenever(flagApplierClient.apply(any(), any())).thenReturn(Result.Success(Unit)) + + confidence.fetchAndActivate() + + val result = confidence.putContextAndWait(context2) + + TestCase.assertTrue(result is Result.Failure) + val eval = confidence.getFlag("test-flag.mystring", "default") + TestCase.assertEquals("value-old", eval.value) + TestCase.assertEquals(ResolveReason.RESOLVE_REASON_STALE, eval.reason) + } + + @Test + fun testRemoveContextAndWaitRemovesKeysBeforeFetch() = runTest { + val testDispatcher = UnconfinedTestDispatcher(testScheduler) + val initialContext = mapOf( + "targeting_key" to ConfidenceValue.String("user-1"), + "plan" to ConfidenceValue.String("free") + ) + + val flagResolver = object : FlagResolver { + override suspend fun resolve( + flags: List, + context: Map + ): Result { + val resolvedValue = if (context.containsKey("plan")) { + "plan-present" + } else { + "plan-removed" + } + return Result.Success( + FlagResolution( + context, + listOf( + ResolvedFlag( + "test-flag", + "flags/test-flag/variants/variant-1", + mutableMapOf("mystring" to ConfidenceValue.String(resolvedValue)), + ResolveReason.RESOLVE_REASON_MATCH, + shouldApply = true + ) + ), + "token1" + ) + ) + } + } + + val confidence = getConfidence( + testDispatcher, + flagResolver = flagResolver, + initialContext = initialContext + ) + whenever(flagApplierClient.apply(any(), any())).thenReturn(Result.Success(Unit)) + + val result = confidence.removeContextAndWait(listOf("plan")) + + TestCase.assertTrue(result is Result.Success) + TestCase.assertFalse(confidence.getContext().containsKey("plan")) + val eval = confidence.getFlag("test-flag.mystring", "default") + TestCase.assertEquals("plan-removed", eval.value) + TestCase.assertEquals(ResolveReason.RESOLVE_REASON_MATCH, eval.reason) + } } diff --git a/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt index 5f4095c5..66eb15c3 100644 --- a/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/openfeature/ConfidenceFeatureProvider.kt @@ -18,7 +18,12 @@ import dev.openfeature.kotlin.sdk.ProviderMetadata import dev.openfeature.kotlin.sdk.Reason import dev.openfeature.kotlin.sdk.TrackingEventDetails import dev.openfeature.kotlin.sdk.Value +import dev.openfeature.kotlin.sdk.events.OpenFeatureProviderEvents import dev.openfeature.kotlin.sdk.exceptions.OpenFeatureError +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow import java.util.Date import kotlin.time.ExperimentalTime import kotlin.time.Instant @@ -35,20 +40,32 @@ class ConfidenceFeatureProvider private constructor( private val initialisationStrategy: InitialisationStrategy, private val confidence: Confidence ) : FeatureProvider { + private val providerEvents = MutableSharedFlow(replay = 1) override suspend fun initialize(initialContext: EvaluationContext?) { - initialContext?.toConfidenceContext()?.let { - confidence.putContextLocal(it.map) - } - - when (initialisationStrategy) { - InitialisationStrategy.ActivateAndFetchAsync -> { - confidence.activate() - confidence.asyncFetch() + try { + initialContext?.toConfidenceContext()?.let { + confidence.putContextLocal(it.map) } - InitialisationStrategy.FetchAndActivate -> { - confidence.fetchAndActivate() + + when (initialisationStrategy) { + InitialisationStrategy.ActivateAndFetchAsync -> { + confidence.activate() + confidence.asyncFetch() + } + InitialisationStrategy.FetchAndActivate -> { + confidence.fetchAndActivate() + } } + providerEvents.emit(OpenFeatureProviderEvents.ProviderReady()) + } catch (e: OpenFeatureError) { + providerEvents.emit(e.toProviderErrorEvent()) + throw e + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + providerEvents.emit(e.toProviderErrorEvent()) + throw e } } @@ -56,14 +73,33 @@ class ConfidenceFeatureProvider private constructor( confidence.stop() } + override fun observe(): Flow = providerEvents.asSharedFlow() + override suspend fun onContextSet( oldContext: EvaluationContext?, newContext: EvaluationContext ) { - val context = newContext.toConfidenceContext() - val removedKeys = oldContext?.asMap()?.keys?.minus(newContext.asMap().keys) ?: emptySet() - confidence.putContext(context.map, removedKeys.toList()) - confidence.awaitReconciliation() + try { + val context = newContext.toConfidenceContext() + val removedKeys = oldContext?.asMap()?.keys?.minus(newContext.asMap().keys) ?: emptySet() + when (val result = confidence.putContextAndWait(context.map, removedKeys.toList())) { + is com.spotify.confidence.Result.Success -> { + // This should be ContextChanged once the Kotlin SDK exposes that event. + providerEvents.emit(OpenFeatureProviderEvents.ProviderReady()) + } + is com.spotify.confidence.Result.Failure -> { + providerEvents.emit(result.error.toProviderStaleEvent()) + } + } + } catch (e: OpenFeatureError) { + providerEvents.emit(e.toProviderErrorEvent()) + throw e + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + providerEvents.emit(e.toProviderErrorEvent()) + throw e + } } override fun getBooleanEvaluation( @@ -230,6 +266,29 @@ private fun ErrorCode?.toOFErrorCode() = when (this) { else -> dev.openfeature.kotlin.sdk.exceptions.ErrorCode.PROVIDER_NOT_READY } +private fun OpenFeatureError.toProviderErrorEvent(): OpenFeatureProviderEvents.ProviderError { + return OpenFeatureProviderEvents.ProviderError( + eventDetails = OpenFeatureProviderEvents.EventDetails( + message = message, + errorCode = errorCode() + ), + error = this + ) +} + +private fun Exception.toProviderErrorEvent(): OpenFeatureProviderEvents.ProviderError { + val error = OpenFeatureError.GeneralError(message ?: "Unknown error") + return error.toProviderErrorEvent() +} + +private fun Throwable.toProviderStaleEvent(): OpenFeatureProviderEvents.ProviderStale { + return OpenFeatureProviderEvents.ProviderStale( + eventDetails = OpenFeatureProviderEvents.EventDetails( + message = message + ) + ) +} + sealed interface InitialisationStrategy { object FetchAndActivate : InitialisationStrategy object ActivateAndFetchAsync : InitialisationStrategy diff --git a/Provider/src/test/java/com/spotify/confidence/openfeature/ConfidenceFeatureProviderTrackTest.kt b/Provider/src/test/java/com/spotify/confidence/openfeature/ConfidenceFeatureProviderTrackTest.kt index 1bb671b3..94c68e03 100644 --- a/Provider/src/test/java/com/spotify/confidence/openfeature/ConfidenceFeatureProviderTrackTest.kt +++ b/Provider/src/test/java/com/spotify/confidence/openfeature/ConfidenceFeatureProviderTrackTest.kt @@ -6,13 +6,21 @@ import dev.openfeature.kotlin.sdk.ImmutableContext import dev.openfeature.kotlin.sdk.ImmutableStructure import dev.openfeature.kotlin.sdk.TrackingEventDetails import dev.openfeature.kotlin.sdk.Value +import dev.openfeature.kotlin.sdk.events.OpenFeatureProviderEvents +import dev.openfeature.kotlin.sdk.exceptions.ErrorCode +import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.slot import io.mockk.verify +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue +import org.junit.Assert.fail import org.junit.Test +import com.spotify.confidence.Result as ConfidenceResult class ConfidenceFeatureProviderTrackTest { @Test @@ -24,6 +32,99 @@ class ConfidenceFeatureProviderTrackTest { verify(exactly = 1) { confidence.stop() } } + @Test + fun initializeEmitsProviderReady() = runTest { + val confidence = mockk(relaxed = true) + val provider = ConfidenceFeatureProvider.create(confidence) + + provider.initialize( + ImmutableContext( + targetingKey = "user-1", + attributes = mapOf("country" to Value.String("SE")) + ) + ) + + assertTrue(provider.observe().first() is OpenFeatureProviderEvents.ProviderReady) + verify { + confidence.putContextLocal( + match { + it["targeting_key"] == ConfidenceValue.String("user-1") && + it["country"] == ConfidenceValue.String("SE") + } + ) + } + coVerify { confidence.fetchAndActivate() } + } + + @Test + fun initializeEmitsProviderErrorBeforeThrowing() = runTest { + val confidence = mockk(relaxed = true) + val error = IllegalStateException("boom") + coEvery { confidence.fetchAndActivate() } throws error + val provider = ConfidenceFeatureProvider.create(confidence) + + try { + provider.initialize(null) + fail("Expected initialization to throw") + } catch (e: IllegalStateException) { + assertEquals(error, e) + } + + val event = provider.observe().first() + assertTrue(event is OpenFeatureProviderEvents.ProviderError) + event as OpenFeatureProviderEvents.ProviderError + assertEquals("boom", event.eventDetails!!.message) + assertEquals(ErrorCode.GENERAL, event.eventDetails!!.errorCode) + } + + @Test + fun onContextSetEmitsProviderReadyAfterReconciliation() = runTest { + val confidence = mockk(relaxed = true) + coEvery { confidence.putContextAndWait(any(), any()) } returns ConfidenceResult.Success(Unit) + val provider = ConfidenceFeatureProvider.create(confidence) + + provider.onContextSet( + oldContext = ImmutableContext(attributes = mapOf("plan" to Value.String("free"))), + newContext = ImmutableContext( + targetingKey = "user-1", + attributes = mapOf("country" to Value.String("SE")) + ) + ) + + assertTrue(provider.observe().first() is OpenFeatureProviderEvents.ProviderReady) + coVerify { + confidence.putContextAndWait( + match { + it["targeting_key"] == ConfidenceValue.String("user-1") && + it["country"] == ConfidenceValue.String("SE") + }, + listOf("plan") + ) + } + } + + @Test + fun onContextSetEmitsProviderStaleWhenReconciliationFails() = runTest { + val confidence = mockk(relaxed = true) + coEvery { + confidence.putContextAndWait(any(), any()) + } returns ConfidenceResult.Failure(IllegalStateException("fetch failed")) + val provider = ConfidenceFeatureProvider.create(confidence) + + provider.onContextSet( + oldContext = ImmutableContext(attributes = mapOf("plan" to Value.String("free"))), + newContext = ImmutableContext( + targetingKey = "user-1", + attributes = mapOf("country" to Value.String("SE")) + ) + ) + + val event = provider.observe().first() + assertTrue(event is OpenFeatureProviderEvents.ProviderStale) + event as OpenFeatureProviderEvents.ProviderStale + assertEquals("fetch failed", event.eventDetails!!.message) + } + @Test fun trackForwardsMergedContextAndMappedData() { val confidence = mockk(relaxed = true) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 81296a94..fadbea76 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -15,7 +15,7 @@ okHttp = "4.12.0" kotlinxSerialization = "1.6.0" # Provider -openFeatureSDK = "0.6.2" +openFeatureSDK = "0.8.0" # Sample app activityCompose = "1.3.1"