From 8850a9a92fbf2efc17c7f8aad63ee411c7593cfd Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Mon, 22 Apr 2024 14:04:00 +0200 Subject: [PATCH 01/13] listen for changes in the provider for the context changes --- .../java/com/spotify/confidence/Confidence.kt | 25 +++--- .../spotify/confidence/ConfidenceContext.kt | 1 - .../confidence/ConfidenceFeatureProvider.kt | 85 +++++++++---------- 3 files changed, 57 insertions(+), 54 deletions(-) diff --git a/Provider/src/main/java/com/spotify/confidence/Confidence.kt b/Provider/src/main/java/com/spotify/confidence/Confidence.kt index 990356ea..10203b46 100644 --- a/Provider/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Provider/src/main/java/com/spotify/confidence/Confidence.kt @@ -10,6 +10,8 @@ import com.spotify.confidence.client.FlagApplierClientImpl import com.spotify.confidence.client.SdkMetadata import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import okhttp3.OkHttpClient class Confidence internal constructor( @@ -23,7 +25,8 @@ class Confidence internal constructor( private val region: ConfidenceRegion = ConfidenceRegion.GLOBAL ) : Contextual, EventSender { private val removedKeys = mutableListOf() - private var contextMap: MutableMap = mutableMapOf() + private var contextMap = MutableStateFlow(mapOf()) + internal val contextChanges: Flow> = contextMap private val flagApplier = FlagApplierWithRetries( client = flagApplierClient, @@ -48,26 +51,28 @@ class Confidence internal constructor( } override fun putContext(key: String, value: ConfidenceValue) { - contextMap[key] = value + val map = contextMap.value.toMutableMap() + map[key] = value + contextMap.value = map } override fun putContext(context: Map) { - contextMap += context - } - - override fun setContext(context: Map) { - contextMap = context.toMutableMap() + val map = contextMap.value.toMutableMap() + map += context + contextMap.value = map } override fun removeContext(key: String) { + val map = contextMap.value.toMutableMap() + map.remove(key) + contextMap.value = map removedKeys.add(key) - contextMap.remove(key) } override fun getContext(): Map = this.parent?.let { - it.getContext().filterKeys { key -> !removedKeys.contains(key) } + contextMap - } ?: contextMap + it.getContext().filterKeys { key -> !removedKeys.contains(key) } + contextMap.value + } ?: contextMap.value override fun withContext(context: Map): Confidence = Confidence( clientSecret, diff --git a/Provider/src/main/java/com/spotify/confidence/ConfidenceContext.kt b/Provider/src/main/java/com/spotify/confidence/ConfidenceContext.kt index d1fbe3dd..950a0239 100644 --- a/Provider/src/main/java/com/spotify/confidence/ConfidenceContext.kt +++ b/Provider/src/main/java/com/spotify/confidence/ConfidenceContext.kt @@ -10,7 +10,6 @@ interface Contextual : ConfidenceContextProvider { fun withContext(context: Map): Contextual fun putContext(context: Map) - fun setContext(context: Map) fun putContext(key: String, value: ConfidenceValue) fun removeContext(key: String) } \ No newline at end of file diff --git a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt index 8a2802ce..9dd299e4 100644 --- a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt @@ -21,6 +21,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancelChildren import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.launch const val SDK_ID = "SDK_ID_KOTLIN_PROVIDER" @@ -49,52 +50,57 @@ class ConfidenceFeatureProvider private constructor( } } + private fun startListeningForContext() { + coroutineScope.launch { + confidence.contextChanges + .drop(1) + .collect { + resolve(InitialisationStrategy.FetchAndActivate) + } + } + } + override fun initialize(initialContext: EvaluationContext?) { initialContext?.let { - internalInitialize( - initialContext, - initialisationStrategy - ) + // refresh cache with the last stored data + storage.read().let(providerCache::refresh) + if (initialisationStrategy == InitialisationStrategy.ActivateAndFetchAsync) { + eventHandler.publish(OpenFeatureEvents.ProviderReady) + } + + coroutineScope.launch(networkExceptionHandler) { + confidence.putContext(OPEN_FEATURE_CONTEXT_KEY, initialContext.toConfidenceContext()) + resolve(initialisationStrategy) + startListeningForContext() + } } } - private fun internalInitialize( - initialContext: EvaluationContext, - strategy: InitialisationStrategy - ) { - // refresh cache with the last stored data - storage.read().let(providerCache::refresh) - if (strategy == InitialisationStrategy.ActivateAndFetchAsync) { - eventHandler.publish(OpenFeatureEvents.ProviderReady) - } + private suspend fun resolve(strategy: InitialisationStrategy) { + try { + val resolveResponse = confidence.resolve(listOf()) + if (resolveResponse is Result.Success) { + // we store the flag anyways except when the response was not modified + if (resolveResponse.data != FlagResolution.EMPTY) { + storage.store(resolveResponse.data) + } - coroutineScope.launch(networkExceptionHandler) { - confidence.putContext(OPEN_FEATURE_CONTEXT_KEY, initialContext.toConfidenceContext()) - try { - val resolveResponse = confidence.resolve(listOf()) - if (resolveResponse is Result.Success) { - // we store the flag anyways except when the response was not modified - if (resolveResponse.data != FlagResolution.EMPTY) { - storage.store(resolveResponse.data) + when (strategy) { + InitialisationStrategy.FetchAndActivate -> { + // refresh the cache from the stored data + providerCache.refresh(resolveResponse.data) + eventHandler.publish(OpenFeatureEvents.ProviderReady) } - when (strategy) { - InitialisationStrategy.FetchAndActivate -> { - // refresh the cache from the stored data - providerCache.refresh(resolveResponse.data) - eventHandler.publish(OpenFeatureEvents.ProviderReady) - } - - InitialisationStrategy.ActivateAndFetchAsync -> { - // do nothing - } + InitialisationStrategy.ActivateAndFetchAsync -> { + // do nothing } - } else { - eventHandler.publish(OpenFeatureEvents.ProviderReady) } - } catch (e: ParseError) { - throw OpenFeatureError.ParseError(e.message) + } else { + eventHandler.publish(OpenFeatureEvents.ProviderReady) } + } catch (e: ParseError) { + throw OpenFeatureError.ParseError(e.message) } } @@ -106,14 +112,7 @@ class ConfidenceFeatureProvider private constructor( oldContext: EvaluationContext?, newContext: EvaluationContext ) { - if (newContext != oldContext) { - // on the new context we want to fetch new values and update - // the storage & cache right away which is why we pass `InitialisationStrategy.FetchAndActivate` - internalInitialize( - newContext, - InitialisationStrategy.FetchAndActivate - ) - } + confidence.putContext(OPEN_FEATURE_CONTEXT_KEY, newContext.toConfidenceContext()) } override fun observe(): Flow = eventHandler.observe() From 14752e4a39488551e3e23246804122191748644e Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Mon, 22 Apr 2024 14:05:39 +0200 Subject: [PATCH 02/13] fixup! listen for changes in the provider for the context changes --- .../java/com/spotify/confidence/ConfidenceFeatureProvider.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt index 9dd299e4..9709e3d8 100644 --- a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt @@ -21,6 +21,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancelChildren import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.drop import kotlinx.coroutines.launch @@ -54,6 +55,7 @@ class ConfidenceFeatureProvider private constructor( coroutineScope.launch { confidence.contextChanges .drop(1) + .distinctUntilChanged() .collect { resolve(InitialisationStrategy.FetchAndActivate) } From cf8f712a141862f8d54f1c70593625b65770d754 Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Mon, 22 Apr 2024 14:07:47 +0200 Subject: [PATCH 03/13] move distinct value and drop the initial value to the confidenc --- .../src/main/java/com/spotify/confidence/Confidence.kt | 7 +++++++ .../com/spotify/confidence/ConfidenceFeatureProvider.kt | 4 ---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Provider/src/main/java/com/spotify/confidence/Confidence.kt b/Provider/src/main/java/com/spotify/confidence/Confidence.kt index 10203b46..a4f1258c 100644 --- a/Provider/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Provider/src/main/java/com/spotify/confidence/Confidence.kt @@ -12,6 +12,8 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.drop import okhttp3.OkHttpClient class Confidence internal constructor( @@ -26,7 +28,12 @@ class Confidence internal constructor( ) : Contextual, EventSender { private val removedKeys = mutableListOf() private var contextMap = MutableStateFlow(mapOf()) + + // only return changes not the initial value + // only return distinct value internal val contextChanges: Flow> = contextMap + .drop(1) + .distinctUntilChanged() private val flagApplier = FlagApplierWithRetries( client = flagApplierClient, diff --git a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt index 9709e3d8..9aeaabc7 100644 --- a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt @@ -21,8 +21,6 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancelChildren import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.drop import kotlinx.coroutines.launch const val SDK_ID = "SDK_ID_KOTLIN_PROVIDER" @@ -54,8 +52,6 @@ class ConfidenceFeatureProvider private constructor( private fun startListeningForContext() { coroutineScope.launch { confidence.contextChanges - .drop(1) - .distinctUntilChanged() .collect { resolve(InitialisationStrategy.FetchAndActivate) } From 31f2f8cba1fea3329d3ca6ed34f2c1eecfff89d7 Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Mon, 22 Apr 2024 14:16:20 +0200 Subject: [PATCH 04/13] resolve again on change context --- .../com/spotify/confidence/ConfidenceFeatureProviderTests.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt b/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt index f5744ffc..2937e804 100644 --- a/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt +++ b/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt @@ -587,6 +587,7 @@ internal class ConfidenceFeatureProviderTests { ) assertEquals(newContextEval.reason, Reason.STALE.name) assertEquals(newContextEval.value, "red") + verify(flagResolverClient, times(2)).resolve(any(), any()) } @Test From 623a3ef5a67d7319b2de4b0935b56ddedfa4ba82 Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Wed, 24 Apr 2024 17:24:24 +0200 Subject: [PATCH 05/13] diff context and add of context without of key --- .../confidence/ConfidenceFeatureProvider.kt | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt index 9aeaabc7..dabacfda 100644 --- a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt @@ -67,7 +67,10 @@ class ConfidenceFeatureProvider private constructor( } coroutineScope.launch(networkExceptionHandler) { - confidence.putContext(OPEN_FEATURE_CONTEXT_KEY, initialContext.toConfidenceContext()) + val context = initialContext.toConfidenceContext() + for (entry in context.map) { + confidence.putContext(entry.key, entry.value) + } resolve(initialisationStrategy) startListeningForContext() } @@ -110,7 +113,16 @@ class ConfidenceFeatureProvider private constructor( oldContext: EvaluationContext?, newContext: EvaluationContext ) { - confidence.putContext(OPEN_FEATURE_CONTEXT_KEY, newContext.toConfidenceContext()) + val context = newContext.toConfidenceContext() + for (entry in context.map) { + confidence.putContext(entry.key, entry.value) + } + + oldContext?.let { old -> + old.asMap().keys.minus(newContext.asMap().keys).forEach { + confidence.removeContext(it) + } + } } override fun observe(): Flow = eventHandler.observe() From b445658db9247d6a7452c167d41ecd446622a8d4 Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Thu, 25 Apr 2024 10:34:51 +0200 Subject: [PATCH 06/13] add test, have removed keys transactional --- .../java/com/spotify/confidence/Confidence.kt | 12 ++++++- .../confidence/ConfidenceFeatureProvider.kt | 15 ++------ .../ConfidenceFeatureProviderTests.kt | 34 +++++++++++++++++++ 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/Provider/src/main/java/com/spotify/confidence/Confidence.kt b/Provider/src/main/java/com/spotify/confidence/Confidence.kt index d25db9b7..76de4adb 100644 --- a/Provider/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Provider/src/main/java/com/spotify/confidence/Confidence.kt @@ -69,11 +69,21 @@ class Confidence internal constructor( contextMap.value = map } + internal fun putContext(context: Map, removedKeys: List) { + val map = contextMap.value.toMutableMap() + map += context + for (key in removedKeys) { + map.remove(key) + } + this.removedKeys.addAll(removedKeys) + contextMap.value = map + } + override fun removeContext(key: String) { val map = contextMap.value.toMutableMap() map.remove(key) - contextMap.value = map removedKeys.add(key) + contextMap.value = map } override fun getContext(): Map = diff --git a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt index dabacfda..c64655da 100644 --- a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt @@ -68,9 +68,7 @@ class ConfidenceFeatureProvider private constructor( coroutineScope.launch(networkExceptionHandler) { val context = initialContext.toConfidenceContext() - for (entry in context.map) { - confidence.putContext(entry.key, entry.value) - } + confidence.putContext(context.map) resolve(initialisationStrategy) startListeningForContext() } @@ -114,15 +112,8 @@ class ConfidenceFeatureProvider private constructor( newContext: EvaluationContext ) { val context = newContext.toConfidenceContext() - for (entry in context.map) { - confidence.putContext(entry.key, entry.value) - } - - oldContext?.let { old -> - old.asMap().keys.minus(newContext.asMap().keys).forEach { - confidence.removeContext(it) - } - } + val removedKeys = oldContext?.asMap()?.keys?.minus(newContext.asMap().keys) ?: emptySet() + confidence.putContext(context.map, removedKeys.toList()) } override fun observe(): Flow = eventHandler.observe() diff --git a/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt b/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt index 2937e804..4d42d9ad 100644 --- a/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt +++ b/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt @@ -543,6 +543,40 @@ internal class ConfidenceFeatureProviderTests { assertNull(evalString2.errorCode) } + @Test + fun confidenceContextRemovedWorks() = runTest { + val testDispatcher = UnconfinedTestDispatcher(testScheduler) + val mockConfidence = getConfidence(testDispatcher) + val eventHandler = EventHandler(testDispatcher) + val confidenceFeatureProvider = ConfidenceFeatureProvider.create( + context = mockContext, + eventHandler = eventHandler, + confidence = mockConfidence, + dispatcher = testDispatcher + ) + val evaluationContext = ImmutableContext("foo", mapOf("hello" to Value.String("world"))) + val context = evaluationContext.toConfidenceContext().map + whenever(flagResolverClient.resolve(eq(listOf()), eq(context))).thenReturn( + Result.Success( + FlagResolution( + context, + resolvedFlags.list, + "token1" + ) + ) + ) + + confidenceFeatureProvider.initialize(evaluationContext) + advanceUntilIdle() + assertEquals(mockConfidence.getContext(), context) + verify(flagResolverClient, times(1)).resolve(any(), eq(context)) + val newContext = ImmutableContext("foo").toConfidenceContext().map + confidenceFeatureProvider.onContextSet(evaluationContext, ImmutableContext("foo")) + advanceUntilIdle() + assertEquals(mockConfidence.getContext(), newContext) + verify(flagResolverClient, times(1)).resolve(any(), eq(newContext)) + } + @Test fun testStaleValueReturnValueAndStaleReason() = runTest { val testDispatcher = UnconfinedTestDispatcher(testScheduler) From 6b2c5cd8eb05a764e3887f3620f3b8f08119c6d9 Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Thu, 25 Apr 2024 10:36:00 +0200 Subject: [PATCH 07/13] fixup! add test, have removed keys transactional --- .../confidence/ConfidenceFeatureProviderTests.kt | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt b/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt index 4d42d9ad..1125315e 100644 --- a/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt +++ b/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt @@ -556,16 +556,6 @@ internal class ConfidenceFeatureProviderTests { ) val evaluationContext = ImmutableContext("foo", mapOf("hello" to Value.String("world"))) val context = evaluationContext.toConfidenceContext().map - whenever(flagResolverClient.resolve(eq(listOf()), eq(context))).thenReturn( - Result.Success( - FlagResolution( - context, - resolvedFlags.list, - "token1" - ) - ) - ) - confidenceFeatureProvider.initialize(evaluationContext) advanceUntilIdle() assertEquals(mockConfidence.getContext(), context) From 591dbe3166b16ba54f85220fd72712b43de3c12a Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Thu, 25 Apr 2024 11:09:44 +0200 Subject: [PATCH 08/13] remove flattening of context --- .../main/java/com/spotify/confidence/Confidence.kt | 14 +------------- .../confidence/ConfidenceFeatureProvider.kt | 2 +- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/Provider/src/main/java/com/spotify/confidence/Confidence.kt b/Provider/src/main/java/com/spotify/confidence/Confidence.kt index 76de4adb..f0c33c8c 100644 --- a/Provider/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Provider/src/main/java/com/spotify/confidence/Confidence.kt @@ -50,7 +50,7 @@ class Confidence internal constructor( } internal suspend fun resolve(flags: List): Result { - return flagResolver.resolve(flags, getContext().openFeatureFlatten()) + return flagResolver.resolve(flags, getContext()) } internal fun apply(flagName: String, resolveToken: String) { @@ -112,18 +112,6 @@ class Confidence internal constructor( } } -internal fun Map.openFeatureFlatten(): Map { - val context = this.toMutableMap() - val openFeatureContext = context[OPEN_FEATURE_CONTEXT_KEY]?.let { it as ConfidenceValue.Struct } - openFeatureContext?.let { - context += it.map - } - context.remove(OPEN_FEATURE_CONTEXT_KEY) - return context -} - -internal const val OPEN_FEATURE_CONTEXT_KEY = "open_feature" - object ConfidenceFactory { fun create( context: Context, diff --git a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt index c64655da..e9a09259 100644 --- a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt @@ -177,7 +177,7 @@ class ConfidenceFeatureProvider private constructor( return providerCache.get().getEvaluation( key, defaultValue, - confidence.getContext().openFeatureFlatten() + confidence.getContext() ) { flagName, resolveToken -> // this lambda will be invoked inside the evaluation process // and only if the resolve reason is not targeting key error. From 601ca90c0e3d22b5cae69fa01bd3b4d22accdefa Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Thu, 25 Apr 2024 17:12:43 +0200 Subject: [PATCH 09/13] fixup! remove flattening of context --- Provider/src/main/java/com/spotify/confidence/Confidence.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Provider/src/main/java/com/spotify/confidence/Confidence.kt b/Provider/src/main/java/com/spotify/confidence/Confidence.kt index f0c33c8c..5a395eca 100644 --- a/Provider/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Provider/src/main/java/com/spotify/confidence/Confidence.kt @@ -57,18 +57,21 @@ class Confidence internal constructor( flagApplier.apply(flagName, resolveToken) } + @Synchronized override fun putContext(key: String, value: ConfidenceValue) { val map = contextMap.value.toMutableMap() map[key] = value contextMap.value = map } + @Synchronized override fun putContext(context: Map) { val map = contextMap.value.toMutableMap() map += context contextMap.value = map } + @Synchronized internal fun putContext(context: Map, removedKeys: List) { val map = contextMap.value.toMutableMap() map += context @@ -79,6 +82,7 @@ class Confidence internal constructor( contextMap.value = map } + @Synchronized override fun removeContext(key: String) { val map = contextMap.value.toMutableMap() map.remove(key) From 0d9e268c59cedaf7305a1805c313183e578a6b43 Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Fri, 26 Apr 2024 11:17:16 +0200 Subject: [PATCH 10/13] cancel the previous resolve and serialize the resolve operations --- .../confidence/ConfidenceFeatureProvider.kt | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt index e9a09259..0c873be1 100644 --- a/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt +++ b/Provider/src/main/java/com/spotify/confidence/ConfidenceFeatureProvider.kt @@ -18,9 +18,12 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancelChildren import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.launch const val SDK_ID = "SDK_ID_KOTLIN_PROVIDER" @@ -49,12 +52,12 @@ class ConfidenceFeatureProvider private constructor( } } + @OptIn(ExperimentalCoroutinesApi::class) private fun startListeningForContext() { coroutineScope.launch { confidence.contextChanges - .collect { - resolve(InitialisationStrategy.FetchAndActivate) - } + .suspendingSwitchMap { resolve(InitialisationStrategy.FetchAndActivate) } + .collect {} } } @@ -227,6 +230,13 @@ class ConfidenceFeatureProvider private constructor( } } +@OptIn(ExperimentalCoroutinesApi::class) +private fun Flow.suspendingSwitchMap(function: suspend () -> U): Flow = flatMapLatest { + flow { + emit(function()) + } +} + internal fun Value.toConfidenceValue(): ConfidenceValue = when (this) { is Value.Structure -> ConfidenceValue.Struct(structure.mapValues { it.value.toConfidenceValue() }) is Value.Boolean -> ConfidenceValue.Boolean(this.boolean) From 0581caa7b0e8fbfd7fa40f40107da07891b9319e Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Fri, 26 Apr 2024 13:47:43 +0200 Subject: [PATCH 11/13] add test for cancelling previous network request --- .../ConfidenceFeatureProviderTests.kt | 57 ++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt b/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt index 1125315e..49865d61 100644 --- a/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt +++ b/Provider/src/test/java/com/spotify/confidence/ConfidenceFeatureProviderTests.kt @@ -30,6 +30,7 @@ import dev.openfeature.sdk.exceptions.OpenFeatureError import junit.framework.TestCase.assertEquals import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -113,11 +114,11 @@ internal class ConfidenceFeatureProviderTests { whenever(mockContext.filesDir).thenReturn(Files.createTempDirectory("tmpTests").toFile()) } - private fun getConfidence(dispatcher: CoroutineDispatcher): Confidence = Confidence( + private fun getConfidence(dispatcher: CoroutineDispatcher, flagResolver: FlagResolver? = null): Confidence = Confidence( clientSecret = "", dispatcher = dispatcher, eventSenderEngine = mock(), - flagResolver = flagResolverClient, + flagResolver = flagResolver ?: flagResolverClient, flagApplierClient = flagApplierClient, diskStorage = FileDiskStorage.create(mockContext), region = ConfidenceRegion.EUROPE @@ -567,6 +568,58 @@ internal class ConfidenceFeatureProviderTests { verify(flagResolverClient, times(1)).resolve(any(), eq(newContext)) } + @Test + fun testWithSlowResolvesWeCancelTheFirstResolveOnNewContextChangesOfConfidence() = runTest { + val testDispatcher = UnconfinedTestDispatcher(testScheduler) + val flagResolver = object : FlagResolver { + var callCount = 0 + var returnCount = 0 + var latestCalledContext = mapOf() + override suspend fun resolve( + flags: List, + context: Map + ): Result { + latestCalledContext = context + if (callCount++ == 0) { + delay(2000) + } + returnCount++ + return Result.Success( + FlagResolution( + context, + resolvedFlags.list, + "token1" + ) + ) + } + } + val mockConfidence = getConfidence(testDispatcher, flagResolver) + val eventHandler = EventHandler(testDispatcher) + + val confidenceFeatureProvider = ConfidenceFeatureProvider.create( + context = mockContext, + eventHandler = eventHandler, + confidence = mockConfidence, + dispatcher = testDispatcher + ) + val evaluationContext = ImmutableContext("foo", mapOf("hello" to Value.String("world"))) + val context = evaluationContext.toConfidenceContext().map + confidenceFeatureProvider.initialize(evaluationContext) + advanceUntilIdle() + // reset the fake flag resolver to count only changes + flagResolver.callCount = 0 + flagResolver.returnCount = 0 + assertEquals(mockConfidence.getContext(), context) + assertEquals(context, flagResolver.latestCalledContext) + confidenceFeatureProvider.onContextSet(evaluationContext, ImmutableContext("foo")) + val newContext2 = ImmutableContext("foo2").toConfidenceContext().map + confidenceFeatureProvider.onContextSet(evaluationContext, ImmutableContext("foo2")) + advanceUntilIdle() + assertEquals(mockConfidence.getContext(), newContext2) + assertEquals(newContext2, flagResolver.latestCalledContext) + assertEquals(1, flagResolver.returnCount) + } + @Test fun testStaleValueReturnValueAndStaleReason() = runTest { val testDispatcher = UnconfinedTestDispatcher(testScheduler) From 8226a8572e81960de5e2aca76c92a06259d04077 Mon Sep 17 00:00:00 2001 From: Fabrizio Demaria Date: Fri, 26 Apr 2024 15:55:24 +0200 Subject: [PATCH 12/13] Make variable val --- Provider/src/main/java/com/spotify/confidence/Confidence.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Provider/src/main/java/com/spotify/confidence/Confidence.kt b/Provider/src/main/java/com/spotify/confidence/Confidence.kt index 5a395eca..bb09a903 100644 --- a/Provider/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Provider/src/main/java/com/spotify/confidence/Confidence.kt @@ -27,7 +27,7 @@ class Confidence internal constructor( private val region: ConfidenceRegion = ConfidenceRegion.GLOBAL ) : Contextual, EventSender { private val removedKeys = mutableListOf() - private var contextMap = MutableStateFlow(mapOf()) + private val contextMap = MutableStateFlow(mapOf()) // only return changes not the initial value // only return distinct value @@ -154,4 +154,4 @@ object ConfidenceFactory { flagApplierClient = flagApplierClient ) } -} \ No newline at end of file +} From e559b48c33e8ad6b331c83f61fa512f21b3d8c1d Mon Sep 17 00:00:00 2001 From: vahid torkaman Date: Fri, 26 Apr 2024 15:58:43 +0200 Subject: [PATCH 13/13] fixup! Make variable val --- Provider/src/main/java/com/spotify/confidence/Confidence.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Provider/src/main/java/com/spotify/confidence/Confidence.kt b/Provider/src/main/java/com/spotify/confidence/Confidence.kt index bb09a903..a9cbadad 100644 --- a/Provider/src/main/java/com/spotify/confidence/Confidence.kt +++ b/Provider/src/main/java/com/spotify/confidence/Confidence.kt @@ -154,4 +154,4 @@ object ConfidenceFactory { flagApplierClient = flagApplierClient ) } -} +} \ No newline at end of file