From 229c4b1c32ec81d875444ade7ce9a3f1e3e40dfb Mon Sep 17 00:00:00 2001 From: liran2000 Date: Wed, 16 Aug 2023 18:40:37 +0300 Subject: [PATCH 1/4] feat: spec 1.1.8 - setProviderAndWait The API SHOULD provide functions to set a provider and wait for the initialize function to return or throw Signed-off-by: liran2000 --- .../dev/openfeature/sdk/OpenFeatureAPI.java | 41 ++++++- .../openfeature/sdk/ProviderRepository.java | 108 ++++++++++++++---- .../sdk/FlagEvaluationSpecTest.java | 30 +++++ .../openfeature/sdk/e2e/StepDefinitions.java | 6 +- .../memory/InMemoryProviderTest.java | 6 +- 5 files changed, 151 insertions(+), 40 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 94047b8cf..6566021e9 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -100,11 +100,11 @@ public EvaluationContext getEvaluationContext() { public void setProvider(FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { providerRepository.setProvider( - provider, - (p) -> attachEventProvider(p), - (p) -> emitReady(p), - (p) -> detachEventProvider(p), - (p, message) -> emitError(p, message)); + provider, + (p) -> attachEventProvider(p), + (p) -> emitReady(p), + (p) -> detachEventProvider(p), + (p, message) -> emitError(p, message)); } } @@ -125,6 +125,37 @@ public void setProvider(String clientName, FeatureProvider provider) { } } + /** + * Set the default provider and wait for initialization to finish. + */ + public void setProviderAndWait(FeatureProvider provider) { + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { + providerRepository.setProviderAndWait( + provider, + (p) -> attachEventProvider(p), + (p) -> emitReady(p), + (p) -> detachEventProvider(p), + (p, message) -> emitError(p, message)); + } + } + + /** + * Add a provider for a named client and wait for initialization to finish. + * + * @param clientName The name of the client. + * @param provider The provider to set. + */ + public void setProviderAndWait(String clientName, FeatureProvider provider) { + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { + providerRepository.setProviderAndWait(clientName, + provider, + this::attachEventProvider, + this::emitReady, + this::detachEventProvider, + this::emitError); + } + } + private void attachEventProvider(FeatureProvider provider) { if (provider instanceof EventProvider) { ((EventProvider)provider).attach((p, event, details) -> { diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 0ff3b70be..c9264a9b3 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -43,8 +43,8 @@ public FeatureProvider getProvider(String name) { public List getClientNamesForProvider(FeatureProvider provider) { return providers.entrySet().stream() - .filter(entry -> entry.getValue().equals(provider)) - .map(entry -> entry.getKey()).collect(Collectors.toList()); + .filter(entry -> entry.getValue().equals(provider)) + .map(entry -> entry.getKey()).collect(Collectors.toList()); } public Set getAllBoundClientNames() { @@ -55,6 +55,18 @@ public boolean isDefaultProvider(FeatureProvider provider) { return this.getProvider().equals(provider); } + private void setProvider(FeatureProvider provider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError, + boolean waitForInit) { + if (provider == null) { + throw new IllegalArgumentException("Provider cannot be null"); + } + initializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit); + } + /** * Set the default provider. */ @@ -63,10 +75,23 @@ public void setProvider(FeatureProvider provider, Consumer afterInit, Consumer afterShutdown, BiConsumer afterError) { + setProvider(provider, afterSet, afterInit, afterShutdown, afterError, false); + } + + private void setProvider(String clientName, + FeatureProvider provider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError, + boolean waitForInit) { if (provider == null) { throw new IllegalArgumentException("Provider cannot be null"); } - initializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError); + if (clientName == null) { + throw new IllegalArgumentException("clientName cannot be null"); + } + initializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit); } /** @@ -81,13 +106,33 @@ public void setProvider(String clientName, Consumer afterInit, Consumer afterShutdown, BiConsumer afterError) { - if (provider == null) { - throw new IllegalArgumentException("Provider cannot be null"); - } - if (clientName == null) { - throw new IllegalArgumentException("clientName cannot be null"); - } - initializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError); + setProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, false); + } + + /** + * Set the default provider and wait for initialization to finish. + */ + public void setProviderAndWait(FeatureProvider provider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError) { + setProvider(provider, afterSet, afterInit, afterShutdown, afterError, true); + } + + /** + * Add a provider for a named client and wait for initialization to finish. + * + * @param clientName The name of the client. + * @param provider The provider to set. + */ + public void setProviderAndWait(String clientName, + FeatureProvider provider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError) { + setProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, true); } private void initializeProvider(@Nullable String clientName, @@ -95,25 +140,38 @@ private void initializeProvider(@Nullable String clientName, Consumer afterSet, Consumer afterInit, Consumer afterShutdown, - BiConsumer afterError) { + BiConsumer afterError, + boolean waitForInit) { // provider is set immediately, on this thread FeatureProvider oldProvider = clientName != null - ? this.providers.put(clientName, newProvider) - : this.defaultProvider.getAndSet(newProvider); + ? this.providers.put(clientName, newProvider) + : this.defaultProvider.getAndSet(newProvider); afterSet.accept(newProvider); - taskExecutor.submit(() -> { - // initialization happens in a different thread - try { - if (ProviderState.NOT_READY.equals(newProvider.getState())) { - newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); - afterInit.accept(newProvider); - } - shutDownOld(oldProvider, afterShutdown); - } catch (Exception e) { - log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e); - afterError.accept(newProvider, e.getMessage()); + if (waitForInit) { + initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider); + } else { + taskExecutor.submit(() -> { + // initialization happens in a different thread + initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider); + }); + } + } + + private void initializeProvider(FeatureProvider newProvider, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError, + FeatureProvider oldProvider) { + try { + if (ProviderState.NOT_READY.equals(newProvider.getState())) { + newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); + afterInit.accept(newProvider); } - }); + shutDownOld(oldProvider, afterShutdown); + } catch (Exception e) { + log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e); + afterError.accept(newProvider, e.getMessage()); + } } private void shutDownOld(FeatureProvider oldProvider,Consumer afterShutdown) { diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index eb41fd950..68dc0b021 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -17,6 +17,9 @@ import java.util.List; import java.util.Map; +import dev.openfeature.sdk.providers.memory.InMemoryProvider; +import lombok.SneakyThrows; +import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -69,6 +72,33 @@ void getApiInstance() { assertThat(api.getProvider()).isEqualTo(mockProvider); } + @SneakyThrows + @Specification(number="1.1.8", text="The API SHOULD provide functions to set a provider and wait for the initialize function to return or throw.") + @Test + void providerAndWait() { + FeatureProvider provider = buildLongInitProvider(); + OpenFeatureAPI.getInstance().setProviderAndWait(provider); + assertThat(api.getProvider().getState()).isEqualTo(ProviderState.READY); + + provider = buildLongInitProvider(); + String providerName = "providerAndWait"; + OpenFeatureAPI.getInstance().setProviderAndWait(providerName, provider); + assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY); + } + + private static FeatureProvider buildLongInitProvider() { + FeatureProvider provider = new InMemoryProvider(new HashMap<>()) { + @Override + public void initialize(EvaluationContext evaluationContext) throws Exception { + super.initialize(evaluationContext); + + // intentionally cause slow initialize to verify setProviderAndWait waits for it. + Awaitility.await().wait(500); + } + }; + return provider; + } + @Specification(number="1.1.5", text="The API MUST provide a function for retrieving the metadata field of the configured provider.") @Test void provider_metadata() { FeatureProviderTestUtils.setFeatureProvider(new DoSomethingProvider()); diff --git a/src/test/java/dev/openfeature/sdk/e2e/StepDefinitions.java b/src/test/java/dev/openfeature/sdk/e2e/StepDefinitions.java index 650fa242b..38ae8a4d8 100644 --- a/src/test/java/dev/openfeature/sdk/e2e/StepDefinitions.java +++ b/src/test/java/dev/openfeature/sdk/e2e/StepDefinitions.java @@ -56,11 +56,7 @@ public class StepDefinitions { public static void setup() { Map> flags = buildFlags(); InMemoryProvider provider = new InMemoryProvider(flags); - OpenFeatureAPI.getInstance().setProvider(provider); - - // TODO: setProvider with wait for init, pending https://github.com/open-feature/ofep/pull/80 - Thread.sleep(500); - + OpenFeatureAPI.getInstance().setProviderAndWait(provider); client = OpenFeatureAPI.getInstance().getClient(); } diff --git a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java index f05a6b79f..ac932e838 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -31,11 +31,7 @@ static void beforeAll() { Map> flags = buildFlags(); provider = spy(new InMemoryProvider(flags)); OpenFeatureAPI.getInstance().onProviderConfigurationChanged(eventDetails -> {}); - OpenFeatureAPI.getInstance().setProvider(provider); - - // TODO: setProvider with wait for init, pending https://github.com/open-feature/ofep/pull/80 - Thread.sleep(500); - + OpenFeatureAPI.getInstance().setProviderAndWait(provider); client = OpenFeatureAPI.getInstance().getClient(); provider.updateFlags(flags); provider.updateFlag("addedFlag", Flag.builder() From 74509e6376c0e4fd212162f26e92854c0e2565eb Mon Sep 17 00:00:00 2001 From: liran2000 Date: Thu, 17 Aug 2023 10:38:38 +0300 Subject: [PATCH 2/4] remove method overloading from package private class Signed-off-by: liran2000 --- .../dev/openfeature/sdk/OpenFeatureAPI.java | 24 +++--- .../openfeature/sdk/ProviderRepository.java | 84 +++++-------------- .../sdk/FlagEvaluationSpecTest.java | 20 +---- .../sdk/ProviderRepositoryTest.java | 20 ++--- .../sdk/testutils/TestEventsProvider.java | 12 +-- 5 files changed, 55 insertions(+), 105 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 6566021e9..fa8b7f516 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -104,7 +104,8 @@ public void setProvider(FeatureProvider provider) { (p) -> attachEventProvider(p), (p) -> emitReady(p), (p) -> detachEventProvider(p), - (p, message) -> emitError(p, message)); + (p, message) -> emitError(p, message), + false); } } @@ -117,11 +118,12 @@ public void setProvider(FeatureProvider provider) { public void setProvider(String clientName, FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { providerRepository.setProvider(clientName, - provider, - this::attachEventProvider, - this::emitReady, - this::detachEventProvider, - this::emitError); + provider, + this::attachEventProvider, + this::emitReady, + this::detachEventProvider, + this::emitError, + false); } } @@ -130,12 +132,13 @@ public void setProvider(String clientName, FeatureProvider provider) { */ public void setProviderAndWait(FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { - providerRepository.setProviderAndWait( + providerRepository.setProvider( provider, (p) -> attachEventProvider(p), (p) -> emitReady(p), (p) -> detachEventProvider(p), - (p, message) -> emitError(p, message)); + (p, message) -> emitError(p, message), + true); } } @@ -147,12 +150,13 @@ public void setProviderAndWait(FeatureProvider provider) { */ public void setProviderAndWait(String clientName, FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { - providerRepository.setProviderAndWait(clientName, + providerRepository.setProvider(clientName, provider, this::attachEventProvider, this::emitReady, this::detachEventProvider, - this::emitError); + this::emitError, + true); } } diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index c9264a9b3..b7d570498 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -55,7 +55,10 @@ public boolean isDefaultProvider(FeatureProvider provider) { return this.getProvider().equals(provider); } - private void setProvider(FeatureProvider provider, + /** + * Set the default provider. + */ + public void setProvider(FeatureProvider provider, Consumer afterSet, Consumer afterInit, Consumer afterShutdown, @@ -64,21 +67,18 @@ private void setProvider(FeatureProvider provider, if (provider == null) { throw new IllegalArgumentException("Provider cannot be null"); } - initializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit); + prepareAndInitializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit); } /** - * Set the default provider. + * Add a provider for a named client. + * + * @param clientName The name of the client. + * @param provider The provider to set. + * @param waitForInit When true, wait for initialization to finish, then returns. + * Otherwise, initialization happens in the background. */ - public void setProvider(FeatureProvider provider, - Consumer afterSet, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError) { - setProvider(provider, afterSet, afterInit, afterShutdown, afterError, false); - } - - private void setProvider(String clientName, + public void setProvider(String clientName, FeatureProvider provider, Consumer afterSet, Consumer afterInit, @@ -91,57 +91,17 @@ private void setProvider(String clientName, if (clientName == null) { throw new IllegalArgumentException("clientName cannot be null"); } - initializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit); + prepareAndInitializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, waitForInit); } - /** - * Add a provider for a named client. - * - * @param clientName The name of the client. - * @param provider The provider to set. - */ - public void setProvider(String clientName, - FeatureProvider provider, - Consumer afterSet, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError) { - setProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, false); - } + private void prepareAndInitializeProvider(@Nullable String clientName, + FeatureProvider newProvider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError, + boolean waitForInit) { - /** - * Set the default provider and wait for initialization to finish. - */ - public void setProviderAndWait(FeatureProvider provider, - Consumer afterSet, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError) { - setProvider(provider, afterSet, afterInit, afterShutdown, afterError, true); - } - - /** - * Add a provider for a named client and wait for initialization to finish. - * - * @param clientName The name of the client. - * @param provider The provider to set. - */ - public void setProviderAndWait(String clientName, - FeatureProvider provider, - Consumer afterSet, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError) { - setProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError, true); - } - - private void initializeProvider(@Nullable String clientName, - FeatureProvider newProvider, - Consumer afterSet, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError, - boolean waitForInit) { // provider is set immediately, on this thread FeatureProvider oldProvider = clientName != null ? this.providers.put(clientName, newProvider) @@ -151,7 +111,7 @@ private void initializeProvider(@Nullable String clientName, initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider); } else { taskExecutor.submit(() -> { - // initialization happens in a different thread + // initialization happens in a different thread if we're not waiting it initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider); }); } @@ -215,7 +175,7 @@ public void shutdown() { }, (FeatureProvider fp, String message) -> { - }); + }, false); this.providers.clear(); taskExecutor.shutdown(); } diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 68dc0b021..a7e17b7e6 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -17,9 +17,8 @@ import java.util.List; import java.util.Map; -import dev.openfeature.sdk.providers.memory.InMemoryProvider; +import dev.openfeature.sdk.testutils.TestEventsProvider; import lombok.SneakyThrows; -import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -76,29 +75,16 @@ void getApiInstance() { @Specification(number="1.1.8", text="The API SHOULD provide functions to set a provider and wait for the initialize function to return or throw.") @Test void providerAndWait() { - FeatureProvider provider = buildLongInitProvider(); + FeatureProvider provider = new TestEventsProvider(500); OpenFeatureAPI.getInstance().setProviderAndWait(provider); assertThat(api.getProvider().getState()).isEqualTo(ProviderState.READY); - provider = buildLongInitProvider(); + provider = new TestEventsProvider(500); String providerName = "providerAndWait"; OpenFeatureAPI.getInstance().setProviderAndWait(providerName, provider); assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY); } - private static FeatureProvider buildLongInitProvider() { - FeatureProvider provider = new InMemoryProvider(new HashMap<>()) { - @Override - public void initialize(EvaluationContext evaluationContext) throws Exception { - super.initialize(evaluationContext); - - // intentionally cause slow initialize to verify setProviderAndWait waits for it. - Awaitility.await().wait(500); - } - }; - return provider; - } - @Specification(number="1.1.5", text="The API MUST provide a function for retrieving the metadata field of the configured provider.") @Test void provider_metadata() { FeatureProviderTestUtils.setFeatureProvider(new DoSomethingProvider()); diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index 5b6dac1b5..0d4ae5d6a 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -55,7 +55,7 @@ class DefaultProvider { @DisplayName("should reject null as default provider") void shouldRejectNullAsDefaultProvider() { assertThatCode(() -> providerRepository.setProvider(null, mockAfterSet(), mockAfterInit(), - mockAfterShutdown(), mockAfterError())).isInstanceOf(IllegalArgumentException.class); + mockAfterShutdown(), mockAfterError(), false)).isInstanceOf(IllegalArgumentException.class); } @Test @@ -76,7 +76,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { .atMost(Duration.ofSeconds(1)) .until(() -> { providerRepository.setProvider(featureProvider, mockAfterSet(), mockAfterInit(), - mockAfterShutdown(), mockAfterError()); + mockAfterShutdown(), mockAfterError(), false); verify(featureProvider, timeout(TIMEOUT)).initialize(any()); return true; }); @@ -101,7 +101,7 @@ class NamedProvider { @DisplayName("should reject null as named provider") void shouldRejectNullAsNamedProvider() { assertThatCode(() -> providerRepository.setProvider(CLIENT_NAME, null, mockAfterSet(), mockAfterInit(), - mockAfterShutdown(), mockAfterError())) + mockAfterShutdown(), mockAfterError(), false)) .isInstanceOf(IllegalArgumentException.class); } @@ -110,7 +110,7 @@ void shouldRejectNullAsNamedProvider() { void shouldRejectNullAsDefaultProvider() { NoOpProvider provider = new NoOpProvider(); assertThatCode(() -> providerRepository.setProvider(null, provider, mockAfterSet(), mockAfterInit(), - mockAfterShutdown(), mockAfterError())) + mockAfterShutdown(), mockAfterError(), false)) .isInstanceOf(IllegalArgumentException.class); } @@ -126,7 +126,7 @@ void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() throws Ex .atMost(Duration.ofSeconds(1)) .until(() -> { providerRepository.setProvider("named client", featureProvider, mockAfterSet(), - mockAfterInit(), mockAfterShutdown(), mockAfterError()); + mockAfterInit(), mockAfterShutdown(), mockAfterError(), false); verify(featureProvider, timeout(TIMEOUT)).initialize(any()); return true; }); @@ -161,7 +161,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { .atMost(Duration.ofSeconds(1)) .until(() -> { providerRepository.setProvider(newProvider, mockAfterSet(), mockAfterInit(), - mockAfterShutdown(), mockAfterError()); + mockAfterShutdown(), mockAfterError(), false); verify(newProvider, timeout(TIMEOUT)).initialize(any()); return true; }); @@ -194,7 +194,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { Future providerMutation = executorService .submit(() -> providerRepository.setProvider(CLIENT_NAME, newProvider, mockAfterSet(), - mockAfterInit(), mockAfterShutdown(), mockAfterError())); + mockAfterInit(), mockAfterShutdown(), mockAfterError(), false)); await() .alias("wait for provider mutator to return") @@ -311,7 +311,7 @@ void shouldShutdownAllFeatureProvidersOnShutdown() { private void setFeatureProvider(FeatureProvider provider) { providerRepository.setProvider(provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), - mockAfterError()); + mockAfterError(), false); waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider); } @@ -320,13 +320,13 @@ private void setFeatureProvider(FeatureProvider provider, Consumer afterInit, Consumer afterShutdown, BiConsumer afterError) { providerRepository.setProvider(provider, afterSet, afterInit, afterShutdown, - afterError); + afterError, false); waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider); } private void setFeatureProvider(String namedProvider, FeatureProvider provider) { providerRepository.setProvider(namedProvider, provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), - mockAfterError()); + mockAfterError(), false); waitForSettingProviderHasBeenCompleted(repository -> repository.getProvider(namedProvider), provider); } diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java index 3fcb58886..25650bf61 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -15,19 +15,19 @@ public class TestEventsProvider extends EventProvider { private String initErrorMessage; private ProviderState state = ProviderState.NOT_READY; private boolean shutDown = false; - private int initTimeout = 0; + private int initTimeoutMs = 0; @Override public ProviderState getState() { return this.state; } - public TestEventsProvider(int initTimeout) { - this.initTimeout = initTimeout; + public TestEventsProvider(int initTimeoutMs) { + this.initTimeoutMs = initTimeoutMs; } - public TestEventsProvider(int initTimeout, boolean initError, String initErrorMessage) { - this.initTimeout = initTimeout; + public TestEventsProvider(int initTimeoutMs, boolean initError, String initErrorMessage) { + this.initTimeoutMs = initTimeoutMs; this.initError = initError; this.initErrorMessage = initErrorMessage; } @@ -53,7 +53,7 @@ public void shutdown() { public void initialize(EvaluationContext evaluationContext) throws Exception { if (ProviderState.NOT_READY.equals(state)) { // wait half the TIMEOUT, otherwise some init/errors can be fired before we add handlers - Thread.sleep(initTimeout); + Thread.sleep(initTimeoutMs); if (this.initError) { this.state = ProviderState.ERROR; throw new Exception(initErrorMessage); From e0006213937168099d292a659ba9fc91717f277e Mon Sep 17 00:00:00 2001 From: liran2000 Date: Thu, 17 Aug 2023 12:04:52 +0300 Subject: [PATCH 3/4] add test case for spec 2.4.5 Signed-off-by: liran2000 --- .../sdk/FlagEvaluationSpecTest.java | 21 +++++++++++++++++-- .../memory/InMemoryProviderTest.java | 12 ++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index a7e17b7e6..35eb0769c 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -17,8 +17,10 @@ import java.util.List; import java.util.Map; +import dev.openfeature.sdk.providers.memory.InMemoryProvider; import dev.openfeature.sdk.testutils.TestEventsProvider; import lombok.SneakyThrows; +import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -73,8 +75,7 @@ void getApiInstance() { @SneakyThrows @Specification(number="1.1.8", text="The API SHOULD provide functions to set a provider and wait for the initialize function to return or throw.") - @Test - void providerAndWait() { + @Test void providerAndWait() { FeatureProvider provider = new TestEventsProvider(500); OpenFeatureAPI.getInstance().setProviderAndWait(provider); assertThat(api.getProvider().getState()).isEqualTo(ProviderState.READY); @@ -85,6 +86,21 @@ void providerAndWait() { assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY); } + @Specification(number="2.4.5", text="The provider SHOULD indicate an error if flag resolution is attempted before the provider is ready.") + @Test void shouldReturnNotReadyIfNotInitialized() { + FeatureProvider provider = new InMemoryProvider(new HashMap<>()) { + @Override + public void initialize(EvaluationContext evaluationContext) throws Exception { + Awaitility.await().wait(3000); + } + }; + String providerName = "shouldReturnNotReadyIfNotInitialized"; + OpenFeatureAPI.getInstance().setProvider(providerName, provider); + assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.NOT_READY); + Client client = OpenFeatureAPI.getInstance().getClient(providerName); + assertEquals(ErrorCode.PROVIDER_NOT_READY, client.getBooleanDetails("return_error_when_not_initialized", false).getErrorCode()); + } + @Specification(number="1.1.5", text="The API MUST provide a function for retrieving the metadata field of the configured provider.") @Test void provider_metadata() { FeatureProviderTestUtils.setFeatureProvider(new DoSomethingProvider()); @@ -307,4 +323,5 @@ void providerAndWait() { @Specification(number="1.4.11", text="The client SHOULD provide asynchronous or non-blocking mechanisms for flag evaluation.") @Test void one_thread_per_request_model() {} + } diff --git a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java index 4a9a94080..ffdc31822 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -6,12 +6,13 @@ import dev.openfeature.sdk.OpenFeatureAPI; import dev.openfeature.sdk.Value; import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import dev.openfeature.sdk.exceptions.ProviderNotReadyError; import dev.openfeature.sdk.exceptions.TypeMismatchError; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.omg.CORBA.DynAnyPackage.TypeMismatch; +import java.util.HashMap; import java.util.Map; import static dev.openfeature.sdk.Structure.mapToStructure; @@ -95,4 +96,13 @@ void typeMismatch() { provider.getBooleanEvaluation("string-flag", false, new ImmutableContext()); }); } + + @SneakyThrows + @Test + void shouldThrowIfNotInitialized() { + InMemoryProvider inMemoryProvider = new InMemoryProvider(new HashMap<>()); + + // ErrorCode.PROVIDER_NOT_READY should be returned when evaluated via the client + assertThrows(ProviderNotReadyError.class, ()-> inMemoryProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext())); + } } \ No newline at end of file From 8b54e5e9afba0f101e84244b18cb56c606913ace Mon Sep 17 00:00:00 2001 From: liran2000 Date: Thu, 17 Aug 2023 16:51:48 +0300 Subject: [PATCH 4/4] minor updates Signed-off-by: liran2000 --- .../java/dev/openfeature/sdk/OpenFeatureAPI.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index fa8b7f516..47c093886 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -101,10 +101,10 @@ public void setProvider(FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { providerRepository.setProvider( provider, - (p) -> attachEventProvider(p), - (p) -> emitReady(p), - (p) -> detachEventProvider(p), - (p, message) -> emitError(p, message), + this::attachEventProvider, + this::emitReady, + this::detachEventProvider, + this::emitError, false); } } @@ -134,10 +134,10 @@ public void setProviderAndWait(FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { providerRepository.setProvider( provider, - (p) -> attachEventProvider(p), - (p) -> emitReady(p), - (p) -> detachEventProvider(p), - (p, message) -> emitError(p, message), + this::attachEventProvider, + this::emitReady, + this::detachEventProvider, + this::emitError, true); } }