From 9cd73aa23a424e2622312b6063cfa7b616702d22 Mon Sep 17 00:00:00 2001 From: Nicklas Lundin Date: Thu, 16 Jan 2025 19:50:39 +0100 Subject: [PATCH] fix: Coroutine resume bug (#118) Signed-off-by: Nicklas Lundin --- .../sdk/events/FeatureProviderExtensions.kt | 14 +-- .../sdk/DeveloperExperienceTests.kt | 30 +++++- .../dev/openfeature/sdk/EventsHandlerTest.kt | 16 +-- .../openfeature/sdk/TestFeatureProvider.kt | 2 - .../sdk/helpers/AutoHealingProvider.kt | 101 ++++++++++++++++++ 5 files changed, 142 insertions(+), 21 deletions(-) create mode 100644 android/src/test/java/dev/openfeature/sdk/helpers/AutoHealingProvider.kt diff --git a/android/src/main/java/dev/openfeature/sdk/events/FeatureProviderExtensions.kt b/android/src/main/java/dev/openfeature/sdk/events/FeatureProviderExtensions.kt index d777b8f..db7a802 100644 --- a/android/src/main/java/dev/openfeature/sdk/events/FeatureProviderExtensions.kt +++ b/android/src/main/java/dev/openfeature/sdk/events/FeatureProviderExtensions.kt @@ -5,6 +5,7 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.take import kotlinx.coroutines.launch @@ -12,7 +13,8 @@ import kotlinx.coroutines.suspendCancellableCoroutine internal fun FeatureProvider.observeProviderReady() = observe() .onStart { - if (getProviderStatus() == OpenFeatureEvents.ProviderReady) { + val status = getProviderStatus() + if (status == OpenFeatureEvents.ProviderReady) { this.emit(OpenFeatureEvents.ProviderReady) } } @@ -30,15 +32,7 @@ suspend fun FeatureProvider.awaitReadyOrError( ) = suspendCancellableCoroutine { continuation -> val coroutineScope = CoroutineScope(dispatcher) coroutineScope.launch { - observeProviderReady() - .take(1) - .collect { - continuation.resumeWith(Result.success(Unit)) - } - } - - coroutineScope.launch { - observeProviderError() + merge(observeProviderReady(), observeProviderError()) .take(1) .collect { continuation.resumeWith(Result.success(Unit)) diff --git a/android/src/test/java/dev/openfeature/sdk/DeveloperExperienceTests.kt b/android/src/test/java/dev/openfeature/sdk/DeveloperExperienceTests.kt index dedb8fd..7d47a1a 100644 --- a/android/src/test/java/dev/openfeature/sdk/DeveloperExperienceTests.kt +++ b/android/src/test/java/dev/openfeature/sdk/DeveloperExperienceTests.kt @@ -3,11 +3,14 @@ package dev.openfeature.sdk import dev.openfeature.sdk.events.OpenFeatureEvents import dev.openfeature.sdk.exceptions.ErrorCode import dev.openfeature.sdk.helpers.AlwaysBrokenProvider +import dev.openfeature.sdk.helpers.AutoHealingProvider import dev.openfeature.sdk.helpers.DoSomethingProvider import dev.openfeature.sdk.helpers.GenericSpyHookMock import dev.openfeature.sdk.helpers.SlowProvider import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.flow.toCollection import kotlinx.coroutines.launch import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -71,7 +74,11 @@ class DeveloperExperienceTests { fun testSetProviderAndWaitReady() = runTest { val dispatcher = StandardTestDispatcher(testScheduler) CoroutineScope(dispatcher).launch { - OpenFeatureAPI.setProviderAndWait(SlowProvider(dispatcher = dispatcher), dispatcher, ImmutableContext()) + OpenFeatureAPI.setProviderAndWait( + SlowProvider(dispatcher = dispatcher), + dispatcher, + ImmutableContext() + ) } testScheduler.advanceTimeBy(1) // Make sure setProviderAndWait is called val booleanValue1 = OpenFeatureAPI.getClient().getBooleanValue("test", false) @@ -108,4 +115,25 @@ class DeveloperExperienceTests { advanceUntilIdle() Assert.assertEquals(eventCount, 1) } + + @Test + fun testProviderThatHealsWithErrorThenReady() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val healing = AutoHealingProvider(dispatcher = dispatcher, healDelay = 100) + val resultEvents = mutableListOf() + val r = async { + OpenFeatureAPI.observe().toCollection(resultEvents) + } + OpenFeatureAPI.setProviderAndWait(healing, dispatcher, ImmutableContext()) + Assert.assertEquals(2, resultEvents.size) + val errorEvent = resultEvents[0] + Assert.assertTrue(errorEvent is OpenFeatureEvents.ProviderError) + Assert.assertEquals( + "AutoHealingProvider is trying to heal", + (errorEvent as OpenFeatureEvents.ProviderError).error.message + ) + Assert.assertEquals(OpenFeatureEvents.ProviderReady, resultEvents[1]) + OpenFeatureAPI.shutdown() + r.cancel() + } } \ No newline at end of file diff --git a/android/src/test/java/dev/openfeature/sdk/EventsHandlerTest.kt b/android/src/test/java/dev/openfeature/sdk/EventsHandlerTest.kt index 190799c..f874273 100644 --- a/android/src/test/java/dev/openfeature/sdk/EventsHandlerTest.kt +++ b/android/src/test/java/dev/openfeature/sdk/EventsHandlerTest.kt @@ -22,7 +22,7 @@ class EventsHandlerTest { fun observing_event_observer_works() = runTest { val dispatcher = UnconfinedTestDispatcher(testScheduler) val eventHandler = EventHandler(dispatcher) - val provider = TestFeatureProvider(dispatcher, eventHandler) + val provider = TestFeatureProvider(eventHandler) var emitted = false val job = backgroundScope.launch(dispatcher) { @@ -41,7 +41,7 @@ class EventsHandlerTest { fun multiple_subscribers_works() = runTest { val dispatcher = UnconfinedTestDispatcher(testScheduler) val eventHandler = EventHandler(dispatcher) - val provider = TestFeatureProvider(dispatcher, eventHandler) + val provider = TestFeatureProvider(eventHandler) val numberOfSubscribers = 10 val parentJob = Job() var emitted = 0 @@ -65,7 +65,7 @@ class EventsHandlerTest { fun canceling_one_subscriber_does_not_cancel_others() = runTest { val dispatcher = UnconfinedTestDispatcher(testScheduler) val eventHandler = EventHandler(dispatcher) - val provider = TestFeatureProvider(dispatcher, eventHandler) + val provider = TestFeatureProvider(eventHandler) val numberOfSubscribers = 10 val parentJob = Job() var emitted = 0 @@ -95,7 +95,7 @@ class EventsHandlerTest { fun the_provider_status_stream_works() = runTest { val dispatcher = UnconfinedTestDispatcher(testScheduler) val eventHandler = EventHandler(dispatcher) - val provider = TestFeatureProvider(dispatcher, eventHandler) + val provider = TestFeatureProvider(eventHandler) var isProviderReady = false // observing the provider status after the provider ready event is published @@ -118,7 +118,7 @@ class EventsHandlerTest { var isProviderReady = false val dispatcher = UnconfinedTestDispatcher(testScheduler) val eventHandler = EventHandler(dispatcher) - val provider = TestFeatureProvider(dispatcher, eventHandler) + val provider = TestFeatureProvider(eventHandler) // observing the provider status after the provider ready event is published val job = backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { @@ -137,7 +137,7 @@ class EventsHandlerTest { fun the_provider_status_stream_is_replays_current_status() = runTest { val dispatcher = UnconfinedTestDispatcher(testScheduler) val eventHandler = EventHandler(dispatcher) - val provider = TestFeatureProvider(dispatcher, eventHandler) + val provider = TestFeatureProvider(eventHandler) provider.emitReady() var isProviderReady = false @@ -158,7 +158,7 @@ class EventsHandlerTest { fun the_provider_becomes_stale() = runTest { val dispatcher = UnconfinedTestDispatcher(testScheduler) val eventHandler = EventHandler(dispatcher) - val provider = TestFeatureProvider(dispatcher, eventHandler) + val provider = TestFeatureProvider(eventHandler) var isProviderStale = false val job = backgroundScope.launch(dispatcher) { @@ -179,7 +179,7 @@ class EventsHandlerTest { fun accessing_status_from_provider_works() = runTest { val dispatcher = UnconfinedTestDispatcher(testScheduler) val eventHandler = EventHandler(dispatcher) - val provider = TestFeatureProvider(dispatcher, eventHandler) + val provider = TestFeatureProvider(eventHandler) Assert.assertEquals(OpenFeatureEvents.ProviderNotReady, provider.getProviderStatus()) diff --git a/android/src/test/java/dev/openfeature/sdk/TestFeatureProvider.kt b/android/src/test/java/dev/openfeature/sdk/TestFeatureProvider.kt index ce17753..374cecc 100644 --- a/android/src/test/java/dev/openfeature/sdk/TestFeatureProvider.kt +++ b/android/src/test/java/dev/openfeature/sdk/TestFeatureProvider.kt @@ -2,10 +2,8 @@ package dev.openfeature.sdk import dev.openfeature.sdk.events.EventHandler import dev.openfeature.sdk.events.OpenFeatureEvents -import kotlinx.coroutines.CoroutineDispatcher class TestFeatureProvider( - dispatcher: CoroutineDispatcher, private val eventHandler: EventHandler ) : FeatureProvider { override val hooks: List> diff --git a/android/src/test/java/dev/openfeature/sdk/helpers/AutoHealingProvider.kt b/android/src/test/java/dev/openfeature/sdk/helpers/AutoHealingProvider.kt new file mode 100644 index 0000000..d285a6a --- /dev/null +++ b/android/src/test/java/dev/openfeature/sdk/helpers/AutoHealingProvider.kt @@ -0,0 +1,101 @@ +package dev.openfeature.sdk.helpers + +import dev.openfeature.sdk.EvaluationContext +import dev.openfeature.sdk.FeatureProvider +import dev.openfeature.sdk.Hook +import dev.openfeature.sdk.ProviderEvaluation +import dev.openfeature.sdk.ProviderMetadata +import dev.openfeature.sdk.Value +import dev.openfeature.sdk.events.EventHandler +import dev.openfeature.sdk.events.OpenFeatureEvents +import dev.openfeature.sdk.exceptions.OpenFeatureError +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestDispatcher + +class AutoHealingProvider( + val dispatcher: TestDispatcher, + val healDelay: Long = 1000L, + override val hooks: List> = emptyList() +) : FeatureProvider { + override val metadata: ProviderMetadata = object : ProviderMetadata { + override val name: String = "AutoHealingProvider" + } + private var ready = false + private var eventHandler = EventHandler(dispatcher) + override fun initialize(initialContext: EvaluationContext?) { + CoroutineScope(dispatcher).launch { + ready = false + eventHandler.publish(OpenFeatureEvents.ProviderError(OpenFeatureError.ProviderNotReadyError("AutoHealingProvider is trying to heal"))) + delay(healDelay) + ready = true + eventHandler.publish(OpenFeatureEvents.ProviderReady) + } + } + + override fun shutdown() { + // no-op + } + + override fun onContextSet( + oldContext: EvaluationContext?, + newContext: EvaluationContext + ) { + // no-op + } + + override fun getBooleanEvaluation( + key: String, + defaultValue: Boolean, + context: EvaluationContext? + ): ProviderEvaluation { + if (!ready) throw OpenFeatureError.FlagNotFoundError(key) + return ProviderEvaluation(!defaultValue) + } + + override fun getStringEvaluation( + key: String, + defaultValue: String, + context: EvaluationContext? + ): ProviderEvaluation { + if (!ready) throw OpenFeatureError.FlagNotFoundError(key) + return ProviderEvaluation(defaultValue.reversed()) + } + + override fun getIntegerEvaluation( + key: String, + defaultValue: Int, + context: EvaluationContext? + ): ProviderEvaluation { + if (!ready) throw OpenFeatureError.FlagNotFoundError(key) + return ProviderEvaluation(defaultValue * 100) + } + + override fun getDoubleEvaluation( + key: String, + defaultValue: Double, + context: EvaluationContext? + ): ProviderEvaluation { + if (!ready) throw OpenFeatureError.FlagNotFoundError(key) + return ProviderEvaluation(defaultValue * 100) + } + + override fun getObjectEvaluation( + key: String, + defaultValue: Value, + context: EvaluationContext? + ): ProviderEvaluation { + if (!ready) throw OpenFeatureError.FlagNotFoundError(key) + return ProviderEvaluation(Value.Null) + } + + override fun observe(): Flow = eventHandler.observe() + + override fun getProviderStatus(): OpenFeatureEvents = if (ready) { + OpenFeatureEvents.ProviderReady + } else { + OpenFeatureEvents.ProviderStale + } +} \ No newline at end of file