From 481b5b6425dfe41a953fefb35a1fd7ffbc09c5cd Mon Sep 17 00:00:00 2001 From: liran2000 Date: Mon, 11 Sep 2023 15:01:02 +0300 Subject: [PATCH] Problem: Provider state check is implemented per provider. Solution: Provider state check is implemented at core SDK. Signed-off-by: liran2000 --- .../java/dev/openfeature/sdk/OpenFeatureClient.java | 9 +++++++++ .../sdk/providers/memory/InMemoryProvider.java | 8 -------- .../dev/openfeature/sdk/FlagEvaluationSpecTest.java | 6 +++--- src/test/java/dev/openfeature/sdk/HookSpecTest.java | 13 ++++++++++++- .../dev/openfeature/sdk/OpenFeatureClientTest.java | 2 +- .../java/dev/openfeature/sdk/TestConstants.java | 2 +- .../sdk/providers/memory/InMemoryProviderTest.java | 4 ++-- 7 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 05d79d02..ce40ea48 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -9,6 +9,7 @@ import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.exceptions.OpenFeatureError; +import dev.openfeature.sdk.exceptions.ProviderNotReadyError; import dev.openfeature.sdk.internal.AutoCloseableLock; import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; import dev.openfeature.sdk.internal.ObjectUtils; @@ -163,6 +164,14 @@ private ProviderEvaluation createProviderEvaluation( T defaultValue, FeatureProvider provider, EvaluationContext invocationContext) { + + if (!ProviderState.READY.equals(provider.getState())) { + if (ProviderState.NOT_READY.equals(provider.getState())) { + throw new ProviderNotReadyError("provider not yet initialized"); + } + throw new GeneralError("unknown error"); + } + switch (type) { case BOOLEAN: return provider.getBooleanEvaluation(key, (Boolean) defaultValue, invocationContext); diff --git a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java index f71e9e36..47b73789 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -16,9 +16,7 @@ import dev.openfeature.sdk.Reason; import dev.openfeature.sdk.Value; import dev.openfeature.sdk.exceptions.FlagNotFoundError; -import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.exceptions.OpenFeatureError; -import dev.openfeature.sdk.exceptions.ProviderNotReadyError; import dev.openfeature.sdk.exceptions.TypeMismatchError; import lombok.Getter; import lombok.SneakyThrows; @@ -121,12 +119,6 @@ public ProviderEvaluation getObjectEvaluation(String key, Value defaultVa private ProviderEvaluation getEvaluation( String key, EvaluationContext evaluationContext, Class expectedType ) throws OpenFeatureError { - if (!ProviderState.READY.equals(state)) { - if (ProviderState.NOT_READY.equals(state)) { - throw new ProviderNotReadyError("provider not yet initialized"); - } - throw new GeneralError("unknown error"); - } Flag flag = flags.get(key); if (flag == null) { throw new FlagNotFoundError("flag " + key + "not found"); diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 35eb0769..0272c527 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -254,9 +254,9 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { assertEquals(Reason.ERROR.toString(), result.getReason()); Mockito.verify(logger).error( - ArgumentMatchers.contains("Unable to correctly evaluate flag with key"), - any(), - ArgumentMatchers.isA(FlagNotFoundError.class)); + ArgumentMatchers.contains("Unable to correctly evaluate flag with key"), + any(), + ArgumentMatchers.isA(FlagNotFoundError.class)); } @Specification(number="1.2.2", text="The client interface MUST define a metadata member or accessor, containing an immutable name field or accessor of type string, which corresponds to the name value supplied during client creation.") diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index d1daa705..e257cca0 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -190,6 +190,10 @@ void emptyApiHooks() { List evalOrder = new ArrayList<>(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new NoOpProvider() { + @Override + public ProviderState getState() { + return ProviderState.READY; + } public List getProviderHooks() { return Collections.singletonList(new BooleanHook() { @@ -387,6 +391,7 @@ public void finallyAfter(HookContext ctx, Map hints) { .thenReturn(ProviderEvaluation.builder() .value(true) .build()); + when(provider.getState()).thenReturn(ProviderState.READY); InOrder order = inOrder(hook, provider); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); @@ -491,6 +496,7 @@ public void finallyAfter(HookContext ctx, Map hints) { when(provider.getBooleanEvaluation(any(), any(), any())).thenReturn(ProviderEvaluation.builder() .value(true) .build()); + when(provider.getState()).thenReturn(ProviderState.READY); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); FeatureProviderTestUtils.setFeatureProvider(provider); @@ -551,7 +557,12 @@ public void finallyAfter(HookContext ctx, Map hints) { private Client getClient(FeatureProvider provider) { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); if (provider == null) { - FeatureProviderTestUtils.setFeatureProvider(new NoOpProvider()); + FeatureProviderTestUtils.setFeatureProvider(new NoOpProvider() { + @Override + public ProviderState getState() { + return ProviderState.READY; + } + }); } else { FeatureProviderTestUtils.setFeatureProvider(provider); } diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 9036576d..5a3cca00 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -58,7 +58,7 @@ void mergeContextTest() { .value(true).build()); when(api.getProvider()).thenReturn(mockProvider); when(api.getProvider(any())).thenReturn(mockProvider); - + when(mockProvider.getState()).thenReturn(ProviderState.READY); OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); client.setEvaluationContext(ctx); diff --git a/src/test/java/dev/openfeature/sdk/TestConstants.java b/src/test/java/dev/openfeature/sdk/TestConstants.java index e9786eb8..08e76789 100644 --- a/src/test/java/dev/openfeature/sdk/TestConstants.java +++ b/src/test/java/dev/openfeature/sdk/TestConstants.java @@ -1,5 +1,5 @@ package dev.openfeature.sdk; public class TestConstants { - public static final String BROKEN_MESSAGE = "This is borked."; + public static final String BROKEN_MESSAGE = "This is broken."; } 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 ffdc3182..5aefb61a 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -6,7 +6,7 @@ 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.OpenFeatureError; import dev.openfeature.sdk.exceptions.TypeMismatchError; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeAll; @@ -103,6 +103,6 @@ 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())); + assertThrows(OpenFeatureError.class, ()-> inMemoryProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext())); } } \ No newline at end of file