From f2e909734fc1711e520dc8b6c3b1b91b6f8b078e Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 9 Sep 2024 15:51:37 +0200 Subject: [PATCH 01/23] Make provider interface "stateless", SDK maintains provider state Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/Client.java | 6 + .../java/dev/openfeature/sdk/ErrorCode.java | 2 +- .../dev/openfeature/sdk/EventProvider.java | 60 +++++++-- .../dev/openfeature/sdk/OpenFeatureAPI.java | 30 ++--- .../openfeature/sdk/OpenFeatureClient.java | 68 ++++++----- .../dev/openfeature/sdk/ProviderAccessor.java | 6 + .../dev/openfeature/sdk/ProviderState.java | 2 +- .../sdk/exceptions/FatalError.java | 12 ++ .../providers/memory/InMemoryProvider.java | 75 ++++-------- .../sdk/DeveloperExperienceTest.java | 42 +++---- .../openfeature/sdk/EventProviderTest.java | 115 ++++++++++++++---- .../java/dev/openfeature/sdk/EventsTest.java | 46 +++---- .../sdk/FlagEvaluationSpecTest.java | 61 +++++----- .../dev/openfeature/sdk/HookSpecTest.java | 50 +++----- .../openfeature/sdk/OpenFeatureAPITest.java | 19 ++- .../sdk/OpenFeatureClientTest.java | 41 ++++--- .../sdk/testutils/TestEventsProvider.java | 73 ++++++----- 17 files changed, 407 insertions(+), 301 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/ProviderAccessor.java create mode 100644 src/main/java/dev/openfeature/sdk/exceptions/FatalError.java diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java index 2184cdcbb..5c87345f5 100644 --- a/src/main/java/dev/openfeature/sdk/Client.java +++ b/src/main/java/dev/openfeature/sdk/Client.java @@ -33,4 +33,10 @@ public interface Client extends Features, EventBus { * @return A list of {@link Hook}s. */ List getHooks(); + + /** + * Returns the current state of the associated provider + * @return the provider state + */ + ProviderState getProviderState(); } diff --git a/src/main/java/dev/openfeature/sdk/ErrorCode.java b/src/main/java/dev/openfeature/sdk/ErrorCode.java index 6b387e27d..aadfecf59 100644 --- a/src/main/java/dev/openfeature/sdk/ErrorCode.java +++ b/src/main/java/dev/openfeature/sdk/ErrorCode.java @@ -2,5 +2,5 @@ @SuppressWarnings("checkstyle:MissingJavadocType") public enum ErrorCode { - PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, TARGETING_KEY_MISSING, INVALID_CONTEXT, GENERAL + PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, TARGETING_KEY_MISSING, INVALID_CONTEXT, GENERAL, PROVIDER_FATAL } diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 4e8c022b4..b6f8afcb6 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -1,5 +1,7 @@ package dev.openfeature.sdk; +import dev.openfeature.sdk.exceptions.GeneralError; +import dev.openfeature.sdk.exceptions.OpenFeatureError; import dev.openfeature.sdk.internal.TriConsumer; /** @@ -16,21 +18,56 @@ */ public abstract class EventProvider implements FeatureProvider { + private ProviderState providerState = ProviderState.NOT_READY; + /** * {@inheritDoc} */ @Override - public abstract ProviderState getState(); + public final ProviderState getState() { + return providerState; + } + + @Override + public final void initialize(EvaluationContext evaluationContext) throws Exception { + try { + doInitialization(evaluationContext); + providerState = ProviderState.READY; + } catch (OpenFeatureError openFeatureError) { + if (ErrorCode.PROVIDER_FATAL.equals(openFeatureError.getErrorCode())) { + providerState = ProviderState.FATAL; + } else { + providerState = ProviderState.ERROR; + } + throw openFeatureError; + } catch (Exception e) { + providerState = ProviderState.ERROR; + throw new GeneralError(e); + } + } + + protected void doInitialization(EvaluationContext evaluationContext) throws Exception { + // Intentionally left blank, to be implemented by inheritors + } + + @Override + public final void shutdown() { + providerState = ProviderState.NOT_READY; + doShutdown(); + } + + protected void doShutdown() { + // Intentionally left blank, to be implemented by inheritors + } private TriConsumer onEmit = null; /** * "Attach" this EventProvider to an SDK, which allows events to propagate from this provider. - * No-op if the same onEmit is already attached. + * No-op if the same onEmit is already attached. * * @param onEmit the function to run when a provider emits events. * @throws IllegalStateException if attempted to bind a new emitter for already bound provider - * */ void attach(TriConsumer onEmit) { if (this.onEmit != null && this.onEmit != onEmit) { @@ -50,11 +87,18 @@ void detach() { /** * Emit the specified {@link ProviderEvent}. - * + * * @param event The event type * @param details The details of the event */ public void emit(ProviderEvent event, ProviderEventDetails details) { + if (ProviderEvent.PROVIDER_ERROR.equals(event)) { + providerState = ProviderState.ERROR; + } else if (ProviderEvent.PROVIDER_STALE.equals(event)) { + providerState = ProviderState.STALE; + } else if (ProviderEvent.PROVIDER_READY.equals(event)) { + providerState = ProviderState.READY; + } if (this.onEmit != null) { this.onEmit.accept(this, event, details); } @@ -63,7 +107,7 @@ public void emit(ProviderEvent event, ProviderEventDetails details) { /** * Emit a {@link ProviderEvent#PROVIDER_READY} event. * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} - * + * * @param details The details of the event */ public void emitProviderReady(ProviderEventDetails details) { @@ -74,7 +118,7 @@ public void emitProviderReady(ProviderEventDetails details) { * Emit a * {@link ProviderEvent#PROVIDER_CONFIGURATION_CHANGED} * event. Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} - * + * * @param details The details of the event */ public void emitProviderConfigurationChanged(ProviderEventDetails details) { @@ -84,7 +128,7 @@ public void emitProviderConfigurationChanged(ProviderEventDetails details) { /** * Emit a {@link ProviderEvent#PROVIDER_STALE} event. * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} - * + * * @param details The details of the event */ public void emitProviderStale(ProviderEventDetails details) { @@ -94,7 +138,7 @@ public void emitProviderStale(ProviderEventDetails details) { /** * Emit a {@link ProviderEvent#PROVIDER_ERROR} event. * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} - * + * * @param details The details of the event */ public void emitProviderError(ProviderEventDetails details) { diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index f4d20caf2..5131d1328 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -5,11 +5,7 @@ import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; import lombok.extern.slf4j.Slf4j; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.function.Consumer; /** @@ -86,7 +82,7 @@ public Client getClient() { * Multiple clients can be used to segment feature flag configuration. * If there is already a provider bound to this domain, this provider will be used. * Otherwise, the default provider is used until a provider is assigned to that domain. - * + * * @param domain an identifier which logically binds clients with providers * @return a new client instance */ @@ -100,15 +96,18 @@ public Client getClient(String domain) { * Multiple clients can be used to segment feature flag configuration. * If there is already a provider bound to this domain, this provider will be used. * Otherwise, the default provider is used until a provider is assigned to that domain. - * - * @param domain a identifier which logically binds clients with providers + * + * @param domain a identifier which logically binds clients with providers * @param version a version identifier * @return a new client instance */ public Client getClient(String domain, String version) { - return new OpenFeatureClient(this, + return new OpenFeatureClient( + () -> providerRepository.getProvider(domain), + this, domain, - version); + version + ); } /** @@ -193,8 +192,8 @@ public void setProvider(FeatureProvider provider) { /** * Add a provider for a domain. * - * @param domain The domain to bind the provider to. - * @param provider The provider to set. + * @param domain The domain to bind the provider to. + * @param provider The provider to set. */ public void setProvider(String domain, FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { @@ -226,8 +225,8 @@ public void setProviderAndWait(FeatureProvider provider) throws OpenFeatureError /** * Add a provider for a domain and wait for initialization to finish. * - * @param domain The domain to bind the provider to. - * @param provider The provider to set. + * @param domain The domain to bind the provider to. + * @param provider The provider to set. */ public void setProviderAndWait(String domain, FeatureProvider provider) throws OpenFeatureError { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { @@ -300,6 +299,7 @@ public void addHooks(Hook... hooks) { /** * Fetch the hooks associated to this client. + * * @return A list of {@link Hook}s. */ public List getHooks() { @@ -404,7 +404,7 @@ void addHandler(String domain, ProviderEvent event, Consumer handl /** * Runs the handlers associated with a particular provider. - * + * * @param provider the provider from where this event originated * @param event the event type * @param details the event details diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index d8004e5de..800c70b6e 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -1,35 +1,30 @@ package dev.openfeature.sdk; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.function.Consumer; - -import dev.openfeature.sdk.exceptions.GeneralError; -import dev.openfeature.sdk.exceptions.OpenFeatureError; +import dev.openfeature.sdk.exceptions.*; import dev.openfeature.sdk.internal.AutoCloseableLock; import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; -import dev.openfeature.sdk.exceptions.ExceptionUtils; import dev.openfeature.sdk.internal.ObjectUtils; import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import java.util.*; +import java.util.function.Consumer; + /** * OpenFeature Client implementation. * You should not instantiate this or reference this class. * Use the dev.openfeature.sdk.Client interface instead. + * * @see Client - * * @deprecated // TODO: eventually we will make this non-public. See issue #872 */ @Slf4j -@SuppressWarnings({ "PMD.DataflowAnomalyAnalysis", "PMD.BeanMembersShouldSerialize", - "PMD.UnusedLocalVariable", "unchecked", "rawtypes" }) +@SuppressWarnings({"PMD.DataflowAnomalyAnalysis", "PMD.BeanMembersShouldSerialize", + "PMD.UnusedLocalVariable", "unchecked", "rawtypes"}) @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public class OpenFeatureClient implements Client { + private final ProviderAccessor providerAccessor; private final OpenFeatureAPI openfeatureApi; @Getter private final String domain; @@ -48,11 +43,17 @@ public class OpenFeatureClient implements Client { * @param domain An identifier which logically binds clients with providers (used by observability tools). * @param version Version of the client (used by observability tools). * @deprecated Do not use this constructor. It's for internal use only. - * Clients created using it will not run event handlers. - * Use the OpenFeatureAPI's getClient factory method instead. + * Clients created using it will not run event handlers. + * Use the OpenFeatureAPI's getClient factory method instead. */ @Deprecated() // TODO: eventually we will make this non-public. See issue #872 - public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String version) { + public OpenFeatureClient( + ProviderAccessor providerAccessor, + OpenFeatureAPI openFeatureAPI, + String domain, + String version + ) { + this.providerAccessor = providerAccessor; this.openfeatureApi = openFeatureAPI; this.domain = domain; this.version = version; @@ -60,6 +61,11 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String ve this.hookSupport = new HookSupport(); } + @Override + public ProviderState getProviderState() { + return providerAccessor.getProvider().getState(); + } + /** * {@inheritDoc} */ @@ -103,7 +109,7 @@ public EvaluationContext getEvaluationContext() { } private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key, T defaultValue, - EvaluationContext ctx, FlagEvaluationOptions options) { + EvaluationContext ctx, FlagEvaluationOptions options) { FlagEvaluationOptions flagOptions = ObjectUtils.defaultIfNull(options, () -> FlagEvaluationOptions.builder().build()); Map hints = Collections.unmodifiableMap(flagOptions.getHookHints()); @@ -117,6 +123,12 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key try { // openfeatureApi.getProvider() must be called once to maintain a consistent reference provider = openfeatureApi.getProvider(this.domain); + if (ProviderState.NOT_READY.equals(provider.getState())) { + throw new ProviderNotReadyError("provider not yet initialized"); + } + if (ProviderState.FATAL.equals(provider.getState())) { + throw new FatalError("provider is in an irrecoverable error state"); + } mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getHooks()); @@ -211,7 +223,7 @@ public Boolean getBooleanValue(String key, Boolean defaultValue, EvaluationConte @Override public Boolean getBooleanValue(String key, Boolean defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return getBooleanDetails(key, defaultValue, ctx, options).getValue(); } @@ -227,7 +239,7 @@ public FlagEvaluationDetails getBooleanDetails(String key, Boolean defa @Override public FlagEvaluationDetails getBooleanDetails(String key, Boolean defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return this.evaluateFlag(FlagValueType.BOOLEAN, key, defaultValue, ctx, options); } @@ -243,7 +255,7 @@ public String getStringValue(String key, String defaultValue, EvaluationContext @Override public String getStringValue(String key, String defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return getStringDetails(key, defaultValue, ctx, options).getValue(); } @@ -259,7 +271,7 @@ public FlagEvaluationDetails getStringDetails(String key, String default @Override public FlagEvaluationDetails getStringDetails(String key, String defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return this.evaluateFlag(FlagValueType.STRING, key, defaultValue, ctx, options); } @@ -275,7 +287,7 @@ public Integer getIntegerValue(String key, Integer defaultValue, EvaluationConte @Override public Integer getIntegerValue(String key, Integer defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return getIntegerDetails(key, defaultValue, ctx, options).getValue(); } @@ -291,7 +303,7 @@ public FlagEvaluationDetails getIntegerDetails(String key, Integer defa @Override public FlagEvaluationDetails getIntegerDetails(String key, Integer defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return this.evaluateFlag(FlagValueType.INTEGER, key, defaultValue, ctx, options); } @@ -307,7 +319,7 @@ public Double getDoubleValue(String key, Double defaultValue, EvaluationContext @Override public Double getDoubleValue(String key, Double defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return this.evaluateFlag(FlagValueType.DOUBLE, key, defaultValue, ctx, options).getValue(); } @@ -323,7 +335,7 @@ public FlagEvaluationDetails getDoubleDetails(String key, Double default @Override public FlagEvaluationDetails getDoubleDetails(String key, Double defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return this.evaluateFlag(FlagValueType.DOUBLE, key, defaultValue, ctx, options); } @@ -339,7 +351,7 @@ public Value getObjectValue(String key, Value defaultValue, EvaluationContext ct @Override public Value getObjectValue(String key, Value defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return getObjectDetails(key, defaultValue, ctx, options).getValue(); } @@ -350,13 +362,13 @@ public FlagEvaluationDetails getObjectDetails(String key, Value defaultVa @Override public FlagEvaluationDetails getObjectDetails(String key, Value defaultValue, - EvaluationContext ctx) { + EvaluationContext ctx) { return getObjectDetails(key, defaultValue, ctx, FlagEvaluationOptions.builder().build()); } @Override public FlagEvaluationDetails getObjectDetails(String key, Value defaultValue, EvaluationContext ctx, - FlagEvaluationOptions options) { + FlagEvaluationOptions options) { return this.evaluateFlag(FlagValueType.OBJECT, key, defaultValue, ctx, options); } diff --git a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java new file mode 100644 index 000000000..a2044d4f7 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java @@ -0,0 +1,6 @@ +package dev.openfeature.sdk; + +@FunctionalInterface +public interface ProviderAccessor { + FeatureProvider getProvider(); +} diff --git a/src/main/java/dev/openfeature/sdk/ProviderState.java b/src/main/java/dev/openfeature/sdk/ProviderState.java index a66d4e944..b924b4d22 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderState.java +++ b/src/main/java/dev/openfeature/sdk/ProviderState.java @@ -4,7 +4,7 @@ * Indicates the state of the provider. */ public enum ProviderState { - READY, NOT_READY, ERROR, STALE; + READY, NOT_READY, ERROR, STALE, FATAL; /** * Returns true if the passed ProviderEvent maps to this ProviderState. diff --git a/src/main/java/dev/openfeature/sdk/exceptions/FatalError.java b/src/main/java/dev/openfeature/sdk/exceptions/FatalError.java new file mode 100644 index 000000000..ad8c69b06 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/exceptions/FatalError.java @@ -0,0 +1,12 @@ +package dev.openfeature.sdk.exceptions; + +import dev.openfeature.sdk.ErrorCode; +import lombok.Getter; +import lombok.experimental.StandardException; + +@StandardException +public class FatalError extends OpenFeatureError { + private static final long serialVersionUID = 1L; + @Getter + private final ErrorCode errorCode = ErrorCode.PROVIDER_FATAL; +} \ No newline at end of file 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 5b6d28702..c0f3ce061 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -1,27 +1,12 @@ package dev.openfeature.sdk.providers.memory; -import dev.openfeature.sdk.EvaluationContext; -import dev.openfeature.sdk.EventProvider; -import dev.openfeature.sdk.Metadata; -import dev.openfeature.sdk.ProviderEvaluation; -import dev.openfeature.sdk.ProviderEventDetails; -import dev.openfeature.sdk.ProviderState; -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 dev.openfeature.sdk.*; +import dev.openfeature.sdk.exceptions.*; import lombok.Getter; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; /** @@ -35,9 +20,6 @@ public class InMemoryProvider extends EventProvider { private final Map> flags; - @Getter - private ProviderState state = ProviderState.NOT_READY; - @Override public Metadata getMetadata() { return () -> NAME; @@ -47,22 +29,11 @@ public InMemoryProvider(Map> flags) { this.flags = new ConcurrentHashMap<>(flags); } - /** - * Initializes the provider. - * @param evaluationContext evaluation context - * @throws Exception on error - */ - @Override - public void initialize(EvaluationContext evaluationContext) throws Exception { - super.initialize(evaluationContext); - state = ProviderState.READY; - log.debug("finished initializing provider, state: {}", state); - } - /** * Updates the provider flags configuration. * For existing flags, the new configurations replace the old one. * For new flags, they are added to the configuration. + * * @param newFlags the new flag configurations */ public void updateFlags(Map> newFlags) { @@ -70,9 +41,9 @@ public void updateFlags(Map> newFlags) { this.flags.putAll(newFlags); ProviderEventDetails details = ProviderEventDetails.builder() - .flagsChanged(new ArrayList<>(flagsChanged)) - .message("flags changed") - .build(); + .flagsChanged(new ArrayList<>(flagsChanged)) + .message("flags changed") + .build(); emitProviderConfigurationChanged(details); } @@ -80,55 +51,59 @@ public void updateFlags(Map> newFlags) { * Updates a single provider flag configuration. * For existing flag, the new configuration replaces the old one. * For new flag, they are added to the configuration. + * * @param newFlag the flag to update */ public void updateFlag(String flagKey, Flag newFlag) { this.flags.put(flagKey, newFlag); ProviderEventDetails details = ProviderEventDetails.builder() - .flagsChanged(Collections.singletonList(flagKey)) - .message("flag added/updated") - .build(); + .flagsChanged(Collections.singletonList(flagKey)) + .message("flag added/updated") + .build(); emitProviderConfigurationChanged(details); } @Override public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, - EvaluationContext evaluationContext) { + EvaluationContext evaluationContext) { return getEvaluation(key, evaluationContext, Boolean.class); } @Override public ProviderEvaluation getStringEvaluation(String key, String defaultValue, - EvaluationContext evaluationContext) { + EvaluationContext evaluationContext) { return getEvaluation(key, evaluationContext, String.class); } @Override public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, - EvaluationContext evaluationContext) { + EvaluationContext evaluationContext) { return getEvaluation(key, evaluationContext, Integer.class); } @Override public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, - EvaluationContext evaluationContext) { + EvaluationContext evaluationContext) { return getEvaluation(key, evaluationContext, Double.class); } @SneakyThrows @Override public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, - EvaluationContext evaluationContext) { + EvaluationContext evaluationContext) { return getEvaluation(key, evaluationContext, Value.class); } private ProviderEvaluation getEvaluation( String key, EvaluationContext evaluationContext, Class expectedType ) throws OpenFeatureError { - if (!ProviderState.READY.equals(state)) { - if (ProviderState.NOT_READY.equals(state)) { + if (!ProviderState.READY.equals(getState())) { + if (ProviderState.NOT_READY.equals(getState())) { throw new ProviderNotReadyError("provider not yet initialized"); } + if (ProviderState.FATAL.equals(getState())) { + throw new FatalError("provider in fatal error state"); + } throw new GeneralError("unknown error"); } Flag flag = flags.get(key); @@ -144,10 +119,10 @@ private ProviderEvaluation getEvaluation( value = (T) flag.getVariants().get(flag.getDefaultVariant()); } return ProviderEvaluation.builder() - .value(value) - .variant(flag.getDefaultVariant()) - .reason(Reason.STATIC.toString()) - .build(); + .value(value) + .variant(flag.getDefaultVariant()) + .reason(Reason.STATIC.toString()) + .build(); } } diff --git a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java index b5e5bedf3..8c3c0ae98 100644 --- a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java +++ b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java @@ -1,38 +1,34 @@ package dev.openfeature.sdk; +import dev.openfeature.sdk.fixtures.HookFixtures; +import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; +import dev.openfeature.sdk.testutils.TestEventsProvider; +import org.junit.jupiter.api.Test; + +import java.util.*; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; - -import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; -import org.junit.jupiter.api.Test; - -import dev.openfeature.sdk.fixtures.HookFixtures; - class DeveloperExperienceTest implements HookFixtures { transient String flagKey = "mykey"; - @Test void simpleBooleanFlag() { + @Test void simpleBooleanFlag() throws Exception { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); + api.setProvider(TestEventsProvider.initialized()); Client client = api.getClient(); Boolean retval = client.getBooleanValue(flagKey, false); assertFalse(retval); } - @Test void clientHooks() { + @Test void clientHooks() throws Exception { Hook exampleHook = mockBooleanHook(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); + api.setProvider(TestEventsProvider.initialized()); Client client = api.getClient(); client.addHooks(exampleHook); Boolean retval = client.getBooleanValue(flagKey, false); @@ -40,12 +36,12 @@ class DeveloperExperienceTest implements HookFixtures { assertFalse(retval); } - @Test void evalHooks() { + @Test void evalHooks() throws Exception { Hook clientHook = mockBooleanHook(); Hook evalHook = mockBooleanHook(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); + api.setProvider(TestEventsProvider.initialized()); Client client = api.getClient(); client.addHooks(clientHook); Boolean retval = client.getBooleanValue(flagKey, false, null, @@ -59,10 +55,10 @@ class DeveloperExperienceTest implements HookFixtures { * As an application author, you probably know special things about your users. You can communicate these to the * provider via {@link MutableContext} */ - @Test void providingContext() { + @Test void providingContext() throws Exception { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); + api.setProvider(TestEventsProvider.initialized()); Client client = api.getClient(); Map attributes = new HashMap<>(); List values = Arrays.asList(new Value(2), new Value(4)); @@ -94,8 +90,12 @@ class MutatingHook implements Hook { @Override // change the provider during a before hook - this should not impact the evaluation in progress - public Optional before(HookContext ctx, Map hints) { - FeatureProviderTestUtils.setFeatureProvider(new NoOpProvider()); + public Optional before(HookContext ctx, Map hints) { + try { + FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.initialized()); + } catch (Exception e) { + throw new RuntimeException(e); + } return Optional.empty(); } } diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index a5be8589c..89e95ca67 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -1,23 +1,32 @@ package dev.openfeature.sdk; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; - +import dev.openfeature.sdk.exceptions.FatalError; +import dev.openfeature.sdk.exceptions.GeneralError; +import dev.openfeature.sdk.internal.TriConsumer; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import dev.openfeature.sdk.internal.TriConsumer; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; class EventProviderTest { + private TestEventProvider eventProvider; + + @BeforeEach + void setup() throws Exception { + eventProvider = new TestEventProvider(); + eventProvider.initialize(null); + } + @Test @DisplayName("should run attached onEmit with emitters") void emitsEventsWhenAttached() { - TestEventProvider eventProvider = new TestEventProvider(); TriConsumer onEmit = mockOnEmit(); eventProvider.attach(onEmit); @@ -37,8 +46,6 @@ void emitsEventsWhenAttached() { @Test @DisplayName("should do nothing with emitters if no onEmit attached") void doesNotEmitsEventsWhenNotAttached() { - TestEventProvider eventProvider = new TestEventProvider(); - // don't attach this emitter TriConsumer onEmit = mockOnEmit(); @@ -56,7 +63,6 @@ void doesNotEmitsEventsWhenNotAttached() { @Test @DisplayName("should throw if second different onEmit attached") void throwsWhenOnEmitDifferent() { - TestEventProvider eventProvider = new TestEventProvider(); TriConsumer onEmit1 = mockOnEmit(); TriConsumer onEmit2 = mockOnEmit(); eventProvider.attach(onEmit1); @@ -67,15 +73,75 @@ void throwsWhenOnEmitDifferent() { @Test @DisplayName("should not throw if second same onEmit attached") void doesNotThrowWhenOnEmitSame() { - TestEventProvider eventProvider = new TestEventProvider(); TriConsumer onEmit1 = mockOnEmit(); TriConsumer onEmit2 = onEmit1; eventProvider.attach(onEmit1); eventProvider.attach(onEmit2); // should not throw, same instance. noop } + @Test + @DisplayName("the state is NOT_READY before initialize method is called") + void stateNotReadyBeforeCallingInit() { + AtomicBoolean doInitializationCalled = new AtomicBoolean(); + EventProvider provider = new TestEventProvider() { + @Override + protected void doInitialization(EvaluationContext evaluationContext) { + doInitializationCalled.set(true); + } + }; + + assertThat(provider.getState()).isEqualTo(ProviderState.NOT_READY); + assertThat(doInitializationCalled).isFalse(); + } + + @Test + @DisplayName("sets the state to READY after running the initialize method") + void setsStateToReadyAfterInit() throws Exception { + AtomicBoolean doInitializationCalled = new AtomicBoolean(); + EventProvider provider = new TestEventProvider() { + @Override + protected void doInitialization(EvaluationContext evaluationContext) { + doInitializationCalled.set(true); + } + }; + provider.initialize(null); + assertThat(provider.getState()).isEqualTo(ProviderState.READY); + assertThat(doInitializationCalled).isTrue(); + } + + @Test + @DisplayName("sets the state to ERROR when the doInitialization method throws an error") + void setsStateToErrorWhenFailingInit() { + AtomicBoolean doInitializationCalled = new AtomicBoolean(); + EventProvider provider = new TestEventProvider() { + @Override + protected void doInitialization(EvaluationContext evaluationContext) { + doInitializationCalled.set(true); + throw new RuntimeException(); + } + }; + assertThrows(GeneralError.class, () -> provider.initialize(null)); + assertThat(provider.getState()).isEqualTo(ProviderState.ERROR); + assertThat(doInitializationCalled).isTrue(); + } + + @Test + @DisplayName("sets the state to PROVIDER_FATAL when the doInitialization method throws a fatal error") + void setsStateToFatalWhenFailingInit() { + AtomicBoolean doInitializationCalled = new AtomicBoolean(); + EventProvider provider = new TestEventProvider() { + @Override + protected void doInitialization(EvaluationContext evaluationContext) { + doInitializationCalled.set(true); + throw new FatalError(); + } + }; + assertThrows(FatalError.class, () -> provider.initialize(null)); + assertThat(provider.getState()).isEqualTo(ProviderState.FATAL); + assertThat(doInitializationCalled).isTrue(); + } - class TestEventProvider extends EventProvider { + static class TestEventProvider extends EventProvider { private static final String NAME = "TestEventProvider"; @@ -86,47 +152,42 @@ public Metadata getMetadata() { @Override public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, - EvaluationContext ctx) { - // TODO Auto-generated method stub + EvaluationContext ctx) { throw new UnsupportedOperationException("Unimplemented method 'getBooleanEvaluation'"); } @Override public ProviderEvaluation getStringEvaluation(String key, String defaultValue, - EvaluationContext ctx) { - // TODO Auto-generated method stub + EvaluationContext ctx) { throw new UnsupportedOperationException("Unimplemented method 'getStringEvaluation'"); } @Override public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, - EvaluationContext ctx) { - // TODO Auto-generated method stub + EvaluationContext ctx) { throw new UnsupportedOperationException("Unimplemented method 'getIntegerEvaluation'"); } @Override public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, - EvaluationContext ctx) { - // TODO Auto-generated method stub + EvaluationContext ctx) { throw new UnsupportedOperationException("Unimplemented method 'getDoubleEvaluation'"); } @Override public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, - EvaluationContext ctx) { - // TODO Auto-generated method stub + EvaluationContext ctx) { throw new UnsupportedOperationException("Unimplemented method 'getObjectEvaluation'"); } @Override - public ProviderState getState() { - return ProviderState.READY; + public void attach(TriConsumer onEmit) { + super.attach(onEmit); } } @SuppressWarnings("unchecked") private TriConsumer mockOnEmit() { - return (TriConsumer)mock(TriConsumer.class); + return (TriConsumer) mock(TriConsumer.class); } } \ No newline at end of file diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 6b70617f6..64a40549c 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -1,26 +1,21 @@ package dev.openfeature.sdk; -import static org.awaitility.Awaitility.await; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.after; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.timeout; -import static org.mockito.Mockito.verify; - -import java.util.Arrays; -import java.util.List; -import java.util.function.Consumer; - +import dev.openfeature.sdk.testutils.TestEventsProvider; +import io.cucumber.java.AfterAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.mockito.ArgumentMatcher; -import dev.openfeature.sdk.testutils.TestEventsProvider; -import io.cucumber.java.AfterAll; +import java.util.Arrays; +import java.util.List; +import java.util.function.Consumer; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.*; class EventsTest { @@ -515,12 +510,12 @@ void shouldHaveAllProperties() { @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingReadyEventsMustRunImmediately() { + void matchingReadyEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already ready - TestEventsProvider provider = new TestEventsProvider(ProviderState.READY); + TestEventsProvider provider = new TestEventsProvider(); OpenFeatureAPI.getInstance().setProvider(name, provider); // should run even thought handler was added after ready @@ -532,12 +527,14 @@ void matchingReadyEventsMustRunImmediately() { @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingStaleEventsMustRunImmediately() { + void matchingStaleEventsMustRunImmediately() throws Exception { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already stale - TestEventsProvider provider = new TestEventsProvider(ProviderState.STALE); + TestEventsProvider provider =TestEventsProvider.initialized(); + provider.emitProviderStale(null); + assertThat(provider.getState()).isEqualTo(ProviderState.STALE); OpenFeatureAPI.getInstance().setProvider(name, provider); // should run even thought handler was added after stale @@ -549,12 +546,14 @@ void matchingStaleEventsMustRunImmediately() { @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingErrorEventsMustRunImmediately() { + void matchingErrorEventsMustRunImmediately() throws Exception { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already in error - TestEventsProvider provider = new TestEventsProvider(ProviderState.ERROR); + TestEventsProvider provider = TestEventsProvider.initialized(); + provider.emitProviderError(null); + assertThat(provider.getState()).isEqualTo(ProviderState.ERROR); OpenFeatureAPI.getInstance().setProvider(name, provider); // should run even thought handler was added after error @@ -597,12 +596,13 @@ class HandlerRemoval { @Specification(number="5.2.7", text="The API and client MUST provide a function allowing the removal of event handlers.") @Test @DisplayName("should not run removed events") - void removedEventsShouldNotRun() { + void removedEventsShouldNotRun() throws Exception { final String name = "removedEventsShouldNotRun"; final Consumer handler1 = mockHandler(); final Consumer handler2 = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); + provider.initialize(null); OpenFeatureAPI.getInstance().setProvider(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index b4978cb4b..00df6bc28 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -1,28 +1,11 @@ package dev.openfeature.sdk; -import static dev.openfeature.sdk.DoSomethingProvider.DEFAULT_METADATA; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; - -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; - -import org.awaitility.Awaitility; +import dev.openfeature.sdk.exceptions.GeneralError; +import dev.openfeature.sdk.fixtures.HookFixtures; +import dev.openfeature.sdk.providers.memory.InMemoryProvider; +import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; +import dev.openfeature.sdk.testutils.TestEventsProvider; +import lombok.SneakyThrows; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -30,12 +13,17 @@ import org.simplify4u.slf4jmock.LoggerMock; import org.slf4j.Logger; -import dev.openfeature.sdk.exceptions.GeneralError; -import dev.openfeature.sdk.fixtures.HookFixtures; -import dev.openfeature.sdk.providers.memory.InMemoryProvider; -import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; -import dev.openfeature.sdk.testutils.TestEventsProvider; -import lombok.SneakyThrows; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static dev.openfeature.sdk.DoSomethingProvider.DEFAULT_METADATA; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.*; class FlagEvaluationSpecTest implements HookFixtures { @@ -47,6 +35,13 @@ private Client _client() { return api.getClient(); } + private Client _initializedClient() throws Exception { + TestEventsProvider provider = new TestEventsProvider(); + provider.initialize(null); + FeatureProviderTestUtils.setFeatureProvider(provider); + return api.getClient(); + } + @BeforeEach void getApiInstance() { api = OpenFeatureAPI.getInstance(); @@ -105,8 +100,8 @@ void getApiInstance() { @Test void shouldReturnNotReadyIfNotInitialized() { FeatureProvider provider = new InMemoryProvider(new HashMap<>()) { @Override - public void initialize(EvaluationContext evaluationContext) throws Exception { - Awaitility.await().wait(3000); + protected void doInitialization(EvaluationContext evaluationContext) throws Exception { + Thread.sleep(10000); } }; String providerName = "shouldReturnNotReadyIfNotInitialized"; @@ -240,8 +235,8 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { } @Specification(number="1.5.1", text="The evaluation options structure's hooks field denotes an ordered collection of hooks that the client MUST execute for the respective flag evaluation, in addition to those already configured.") - @Test void hooks() { - Client c = _client(); + @Test void hooks() throws Exception { + Client c = _initializedClient(); Hook clientHook = mockBooleanHook(); Hook invocationHook = mockBooleanHook(); c.addHooks(clientHook); diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index 2554457ab..f988e822b 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -3,34 +3,20 @@ import dev.openfeature.sdk.exceptions.FlagNotFoundError; import dev.openfeature.sdk.fixtures.HookFixtures; import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; +import dev.openfeature.sdk.testutils.TestEventsProvider; import lombok.SneakyThrows; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.fail; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; class HookSpecTest implements HookFixtures { @AfterEach @@ -225,7 +211,7 @@ void emptyApiHooks() { @Test void hook_eval_order() { List evalOrder = new ArrayList<>(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider() { + api.setProvider(new TestEventsProvider() { public List getProviderHooks() { return Collections.singletonList(new BooleanHook() { @@ -354,12 +340,12 @@ public void finallyAfter(HookContext ctx, Map hints) { } @Specification(number="4.4.6", text="If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") - @Test void error_stops_after() { + @Test void error_stops_after() throws Exception { Hook h = mockBooleanHook(); doThrow(RuntimeException.class).when(h).after(any(), any(), any()); Hook h2 = mockBooleanHook(); - Client c = getClient(null); + Client c = getClient(TestEventsProvider.initialized()); c.getBooleanDetails("key", false, null, FlagEvaluationOptions.builder() .hook(h) @@ -373,7 +359,7 @@ public void finallyAfter(HookContext ctx, Map hints) { @Specification(number="4.5.2", text="hook hints MUST be passed to each hook.") @Specification(number="4.2.2.1", text="Condition: Hook hints MUST be immutable.") @Specification(number="4.5.3", text="The hook MUST NOT alter the hook hints structure.") - @Test void hook_hints() { + @Test void hook_hints() throws Exception { String hintKey = "My hint key"; Client client = getClient(null); Hook mutatingHook = new BooleanHook() { @@ -439,10 +425,10 @@ public void finallyAfter(HookContext ctx, Map hints) { @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") @Specification(number="4.4.7", text="If an error occurs in the before hooks, the default value MUST be returned.") - @Test void error_hooks__before() { + @Test void error_hooks__before() throws Exception { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); - Client client = getClient(null); + Client client = getClient(TestEventsProvider.initialized()); Boolean value = client.getBooleanValue("key", false, new ImmutableContext(), FlagEvaluationOptions.builder().hook(hook).build()); verify(hook, times(1)).before(any(), any()); @@ -451,17 +437,17 @@ public void finallyAfter(HookContext ctx, Map hints) { } @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") - @Test void error_hooks__after() { + @Test void error_hooks__after() throws Exception { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).after(any(), any(), any()); - Client client = getClient(null); + Client client = getClient(TestEventsProvider.initialized()); client.getBooleanValue("key", false, new ImmutableContext(), FlagEvaluationOptions.builder().hook(hook).build()); verify(hook, times(1)).after(any(), any(), any()); verify(hook, times(1)).error(any(), any(), any()); } - @Test void multi_hooks_early_out__before() { + @Test void multi_hooks_early_out__before() throws Exception { Hook hook = mockBooleanHook(); Hook hook2 = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); @@ -483,7 +469,7 @@ public void finallyAfter(HookContext ctx, Map hints) { @Specification(number = "4.1.4", text = "The evaluation context MUST be mutable only within the before hook.") @Specification(number = "4.3.4", text = "Any `evaluation context` returned from a `before` hook MUST be passed to subsequent `before` hooks (via `HookContext`).") - @Test void beforeContextUpdated() { + @Test void beforeContextUpdated() throws Exception { String targetingKey = "test-key"; EvaluationContext ctx = new ImmutableContext(targetingKey); Hook hook = mockBooleanHook(); @@ -546,7 +532,7 @@ public void finallyAfter(HookContext ctx, Map hints) { } @Specification(number="4.4.3", text="If a finally hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining finally hooks.") - @Test void first_finally_broken() { + @Test void first_finally_broken() throws Exception { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); doThrow(RuntimeException.class).when(hook).finallyAfter(any(), any()); @@ -566,7 +552,7 @@ public void finallyAfter(HookContext ctx, Map hints) { } @Specification(number="4.4.4", text="If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.") - @Test void first_error_broken() { + @Test void first_error_broken() throws Exception { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); doThrow(RuntimeException.class).when(hook).error(any(), any(), any()); @@ -585,10 +571,10 @@ public void finallyAfter(HookContext ctx, Map hints) { order.verify(hook).error(any(), any(), any()); } - private Client getClient(FeatureProvider provider) { + private Client getClient(FeatureProvider provider) throws Exception { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); if (provider == null) { - FeatureProviderTestUtils.setFeatureProvider(new NoOpProvider()); + FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.initialized()); } else { FeatureProviderTestUtils.setFeatureProvider(provider); } diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java index 74298f72f..8fd863ce7 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java @@ -1,17 +1,16 @@ package dev.openfeature.sdk; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.junit.jupiter.api.Assertions.assertEquals; +import dev.openfeature.sdk.providers.memory.InMemoryProvider; +import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import java.util.Collections; import java.util.HashMap; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import dev.openfeature.sdk.providers.memory.InMemoryProvider; -import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.junit.jupiter.api.Assertions.assertEquals; class OpenFeatureAPITest { @@ -33,7 +32,7 @@ void namedProviderTest() { .isEqualTo(api.getProviderMetadata("namedProviderTest").getName()); } - @Specification(number="1.1.3", text="The API MUST provide a function to bind a given provider to one or more clients using a domain. If the domain already has a bound provider, it is overwritten with the new mapping.") + @Specification(number = "1.1.3", text = "The API MUST provide a function to bind a given provider to one or more clients using a domain. If the domain already has a bound provider, it is overwritten with the new mapping.") @Test void namedProviderOverwrittenTest() { String domain = "namedProviderOverwrittenTest"; @@ -82,7 +81,7 @@ void settingTransactionalContextPropagatorToNullErrors() { @Test void setEvaluationContextShouldAllowChaining() { - OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); EvaluationContext ctx = new ImmutableContext("targeting key", new HashMap<>()); OpenFeatureClient result = client.setEvaluationContext(ctx); assertEquals(client, result); diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index d6340a844..609c122f6 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -1,34 +1,37 @@ package dev.openfeature.sdk; -import java.util.*; - import dev.openfeature.sdk.fixtures.HookFixtures; - -import org.junit.jupiter.api.*; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; import org.mockito.Mockito; import org.simplify4u.slf4jmock.LoggerMock; import org.slf4j.Logger; +import java.util.Arrays; +import java.util.HashMap; + import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; class OpenFeatureClientTest implements HookFixtures { private Logger logger; - @BeforeEach void set_logger() { + @BeforeEach + void set_logger() { logger = Mockito.mock(Logger.class); LoggerMock.setMock(OpenFeatureClient.class, logger); } - @AfterEach void reset_logs() { + @AfterEach + void reset_logs() { LoggerMock.setMock(OpenFeatureClient.class, logger); } + @Test @DisplayName("should not throw exception if hook has different type argument than hookContext") void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() { @@ -36,7 +39,7 @@ void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() { when(api.getProvider(any())).thenReturn(new DoSomethingProvider()); when(api.getHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook())); - OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); FlagEvaluationDetails actual = client.getBooleanDetails("feature key", Boolean.FALSE); @@ -59,14 +62,14 @@ void mergeContextTest() { FeatureProvider mockProvider = mock(FeatureProvider.class); // this makes it so that true is returned only if the targeting key set at the client level is honored when(mockProvider.getBooleanEvaluation( - eq(flag), eq(defaultValue), argThat( - context -> context.getTargetingKey().equals(targetingKey)))).thenReturn(ProviderEvaluation.builder() - .value(true).build()); + eq(flag), eq(defaultValue), argThat( + context -> context.getTargetingKey().equals(targetingKey)))).thenReturn(ProviderEvaluation.builder() + .value(true).build()); when(api.getProvider()).thenReturn(mockProvider); when(api.getProvider(any())).thenReturn(mockProvider); - OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); client.setEvaluationContext(ctx); FlagEvaluationDetails result = client.getBooleanDetails(flag, defaultValue); @@ -78,23 +81,23 @@ void mergeContextTest() { @DisplayName("addHooks should allow chaining by returning the same client instance") void addHooksShouldAllowChaining() { OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); Hook hook1 = Mockito.mock(Hook.class); Hook hook2 = Mockito.mock(Hook.class); OpenFeatureClient result = client.addHooks(hook1, hook2); - assertEquals(client, result); + assertEquals(client, result); } @Test @DisplayName("setEvaluationContext should allow chaining by returning the same client instance") void setEvaluationContextShouldAllowChaining() { OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); EvaluationContext ctx = new ImmutableContext("targeting key", new HashMap<>()); OpenFeatureClient result = client.setEvaluationContext(ctx); assertEquals(client, result); } - + } diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java index 0b19f82a5..5334cee04 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -1,28 +1,19 @@ package dev.openfeature.sdk.testutils; -import dev.openfeature.sdk.EvaluationContext; -import dev.openfeature.sdk.EventProvider; -import dev.openfeature.sdk.Metadata; -import dev.openfeature.sdk.ProviderEvaluation; -import dev.openfeature.sdk.ProviderEvent; -import dev.openfeature.sdk.ProviderEventDetails; -import dev.openfeature.sdk.ProviderState; -import dev.openfeature.sdk.Value; +import dev.openfeature.sdk.*; import dev.openfeature.sdk.exceptions.GeneralError; public class TestEventsProvider extends EventProvider { + public static final String PASSED_IN_DEFAULT = "Passed in default"; private boolean initError = false; private String initErrorMessage; - private ProviderState state = ProviderState.NOT_READY; private boolean shutDown = false; private int initTimeoutMs = 0; private String name = "test"; private Metadata metadata = () -> name; - @Override - public ProviderState getState() { - return this.state; + public TestEventsProvider() { } public TestEventsProvider(int initTimeoutMs) { @@ -35,8 +26,10 @@ public TestEventsProvider(int initTimeoutMs, boolean initError, String initError this.initErrorMessage = initErrorMessage; } - public TestEventsProvider(ProviderState initialState) { - this.state = initialState; + public static TestEventsProvider initialized() throws Exception { + TestEventsProvider provider = new TestEventsProvider(); + provider.initialize(null); + return provider; } public void mockEvent(ProviderEvent event, ProviderEventDetails details) { @@ -48,20 +41,18 @@ public boolean isShutDown() { } @Override - public void shutdown() { + protected void doShutdown() { this.shutDown = true; } @Override - public void initialize(EvaluationContext evaluationContext) throws Exception { - if (ProviderState.NOT_READY.equals(state)) { + protected void doInitialization(EvaluationContext evaluationContext) throws Exception { + if (ProviderState.NOT_READY.equals(getState())) { // wait half the TIMEOUT, otherwise some init/errors can be fired before we add handlers Thread.sleep(initTimeoutMs); if (this.initError) { - this.state = ProviderState.ERROR; throw new GeneralError(initErrorMessage); } - this.state = ProviderState.READY; } } @@ -71,32 +62,48 @@ public Metadata getMetadata() { } @Override - public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getBooleanEvaluation'"); + public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder() + .value(defaultValue) + .variant(PASSED_IN_DEFAULT) + .reason(Reason.DEFAULT.toString()) + .build(); } @Override - public ProviderEvaluation getStringEvaluation(String key, String defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getStringEvaluation'"); + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder() + .value(defaultValue) + .variant(PASSED_IN_DEFAULT) + .reason(Reason.DEFAULT.toString()) + .build(); } @Override - public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getIntegerEvaluation'"); + public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder() + .value(defaultValue) + .variant(PASSED_IN_DEFAULT) + .reason(Reason.DEFAULT.toString()) + .build(); } @Override - public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getDoubleEvaluation'"); + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder() + .value(defaultValue) + .variant(PASSED_IN_DEFAULT) + .reason(Reason.DEFAULT.toString()) + .build(); } @Override public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getObjectEvaluation'"); + EvaluationContext invocationContext) { + return ProviderEvaluation.builder() + .value(defaultValue) + .variant(PASSED_IN_DEFAULT) + .reason(Reason.DEFAULT.toString()) + .build(); } }; \ No newline at end of file From d2b15fefdb7de45a9eb7566bc2b226042479b13b Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 9 Sep 2024 16:35:59 +0200 Subject: [PATCH 02/23] Add tests and fix bug Signed-off-by: christian.lutnik --- .../openfeature/sdk/OpenFeatureClient.java | 2 +- .../sdk/OpenFeatureClientTest.java | 84 ++++++++++++++++++- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 800c70b6e..4792c98fc 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -122,7 +122,7 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key try { // openfeatureApi.getProvider() must be called once to maintain a consistent reference - provider = openfeatureApi.getProvider(this.domain); + provider = providerAccessor.getProvider(); if (ProviderState.NOT_READY.equals(provider.getState())) { throw new ProviderNotReadyError("provider not yet initialized"); } diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 609c122f6..8d0e0234b 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -11,6 +11,7 @@ import java.util.Arrays; import java.util.HashMap; +import java.util.concurrent.atomic.AtomicBoolean; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -35,11 +36,12 @@ void reset_logs() { @Test @DisplayName("should not throw exception if hook has different type argument than hookContext") void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() { + DoSomethingProvider provider = new DoSomethingProvider(); OpenFeatureAPI api = mock(OpenFeatureAPI.class); - when(api.getProvider(any())).thenReturn(new DoSomethingProvider()); + when(api.getProvider(any())).thenReturn(provider); when(api.getHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook())); - OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> provider, api, "name", "version"); FlagEvaluationDetails actual = client.getBooleanDetails("feature key", Boolean.FALSE); @@ -69,7 +71,7 @@ void mergeContextTest() { when(api.getProvider(any())).thenReturn(mockProvider); - OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> mockProvider, api, "name", "version"); client.setEvaluationContext(ctx); FlagEvaluationDetails result = client.getBooleanDetails(flag, defaultValue); @@ -100,4 +102,80 @@ void setEvaluationContextShouldAllowChaining() { assertEquals(client, result); } + @Test + @DisplayName("Should not call evaluation methods when the provider has state FATAL") + void shouldNotCallEvaluationMethodsWhenProviderIsInFatalErrorState() { + MockProvider mockProvider = new MockProvider(ProviderState.FATAL); + OpenFeatureAPI api = mock(OpenFeatureAPI.class); + OpenFeatureClient client = new OpenFeatureClient(() -> mockProvider, api, "name", "version"); + FlagEvaluationDetails details = client.getBooleanDetails("key", true); + + assertThat(mockProvider.isEvaluationCalled()).isFalse(); + assertThat(details.getErrorCode()).isEqualTo(ErrorCode.PROVIDER_FATAL); + } + + @Test + @DisplayName("Should not call evaluation methods when the provider has state NOT_READY") + void shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState() { + MockProvider mockProvider = new MockProvider(ProviderState.NOT_READY); + OpenFeatureAPI api = mock(OpenFeatureAPI.class); + OpenFeatureClient client = new OpenFeatureClient(() -> mockProvider, api, "name", "version"); + FlagEvaluationDetails details = client.getBooleanDetails("key", true); + + assertThat(mockProvider.isEvaluationCalled()).isFalse(); + assertThat(details.getErrorCode()).isEqualTo(ErrorCode.PROVIDER_NOT_READY); + } + + private static class MockProvider implements FeatureProvider { + private final AtomicBoolean evaluationCalled = new AtomicBoolean(); + private final ProviderState providerState; + + public MockProvider(ProviderState providerState) { + this.providerState = providerState; + } + + public boolean isEvaluationCalled() { + return evaluationCalled.get(); + } + + @Override + public ProviderState getState() { + return providerState; + } + + @Override + public Metadata getMetadata() { + return null; + } + + @Override + public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { + evaluationCalled.set(true); + return null; + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { + evaluationCalled.set(true); + return null; + } + + @Override + public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { + evaluationCalled.set(true); + return null; + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { + evaluationCalled.set(true); + return null; + } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { + evaluationCalled.set(true); + return null; + } + } } From b993ee21bae9eba5938c7b9034624eba911a846d Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Tue, 10 Sep 2024 07:59:26 +0200 Subject: [PATCH 03/23] Fix checkstyle errors Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/Client.java | 2 +- src/main/java/dev/openfeature/sdk/ErrorCode.java | 3 ++- src/main/java/dev/openfeature/sdk/OpenFeatureClient.java | 8 ++++---- src/main/java/dev/openfeature/sdk/ProviderAccessor.java | 3 +++ .../java/dev/openfeature/sdk/exceptions/FatalError.java | 1 + 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java index 5c87345f5..7b41b9b07 100644 --- a/src/main/java/dev/openfeature/sdk/Client.java +++ b/src/main/java/dev/openfeature/sdk/Client.java @@ -35,7 +35,7 @@ public interface Client extends Features, EventBus { List getHooks(); /** - * Returns the current state of the associated provider + * Returns the current state of the associated provider. * @return the provider state */ ProviderState getProviderState(); diff --git a/src/main/java/dev/openfeature/sdk/ErrorCode.java b/src/main/java/dev/openfeature/sdk/ErrorCode.java index aadfecf59..00451bdfb 100644 --- a/src/main/java/dev/openfeature/sdk/ErrorCode.java +++ b/src/main/java/dev/openfeature/sdk/ErrorCode.java @@ -2,5 +2,6 @@ @SuppressWarnings("checkstyle:MissingJavadocType") public enum ErrorCode { - PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, TARGETING_KEY_MISSING, INVALID_CONTEXT, GENERAL, PROVIDER_FATAL + PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, TARGETING_KEY_MISSING, INVALID_CONTEXT, GENERAL, + PROVIDER_FATAL } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 4792c98fc..6e2c7e7b7 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -19,8 +19,8 @@ * @deprecated // TODO: eventually we will make this non-public. See issue #872 */ @Slf4j -@SuppressWarnings({"PMD.DataflowAnomalyAnalysis", "PMD.BeanMembersShouldSerialize", - "PMD.UnusedLocalVariable", "unchecked", "rawtypes"}) +@SuppressWarnings({"PMD.DataflowAnomalyAnalysis", "PMD.BeanMembersShouldSerialize", "PMD.UnusedLocalVariable", + "unchecked", "rawtypes"}) @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public class OpenFeatureClient implements Client { @@ -43,8 +43,8 @@ public class OpenFeatureClient implements Client { * @param domain An identifier which logically binds clients with providers (used by observability tools). * @param version Version of the client (used by observability tools). * @deprecated Do not use this constructor. It's for internal use only. - * Clients created using it will not run event handlers. - * Use the OpenFeatureAPI's getClient factory method instead. + * Clients created using it will not run event handlers. + * Use the OpenFeatureAPI's getClient factory method instead. */ @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public OpenFeatureClient( diff --git a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java index a2044d4f7..cf1fbec67 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java +++ b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java @@ -1,5 +1,8 @@ package dev.openfeature.sdk; +/** + * Provides access to the future provider for the domain of the client. + */ @FunctionalInterface public interface ProviderAccessor { FeatureProvider getProvider(); diff --git a/src/main/java/dev/openfeature/sdk/exceptions/FatalError.java b/src/main/java/dev/openfeature/sdk/exceptions/FatalError.java index ad8c69b06..d50d1a42c 100644 --- a/src/main/java/dev/openfeature/sdk/exceptions/FatalError.java +++ b/src/main/java/dev/openfeature/sdk/exceptions/FatalError.java @@ -4,6 +4,7 @@ import lombok.Getter; import lombok.experimental.StandardException; +@SuppressWarnings("checkstyle:MissingJavadocType") @StandardException public class FatalError extends OpenFeatureError { private static final long serialVersionUID = 1L; From 625a40452b838c4bffe85feaed031ff46e5cd248 Mon Sep 17 00:00:00 2001 From: chrfwow Date: Tue, 10 Sep 2024 08:17:59 +0200 Subject: [PATCH 04/23] add test Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/OpenFeatureAPITest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java index 8fd863ce7..83b3ba195 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java @@ -2,6 +2,7 @@ import dev.openfeature.sdk.providers.memory.InMemoryProvider; import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; +import dev.openfeature.sdk.testutils.TestEventsProvider; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -86,4 +87,18 @@ void setEvaluationContextShouldAllowChaining() { OpenFeatureClient result = client.setEvaluationContext(ctx); assertEquals(client, result); } + + @Test + void getStateReturnsTheStateOfTheAppropriateProvider() throws Exception { + String domain = "namedProviderOverwrittenTest"; + FeatureProvider provider1 = new NoOpProvider(); + FeatureProvider provider2 = new TestEventsProvider(); + FeatureProviderTestUtils.setFeatureProvider(domain, provider1); + FeatureProviderTestUtils.setFeatureProvider(domain, provider2); + + provider2.initialize(null); + + assertThat(OpenFeatureAPI.getInstance().getClient(domain).getProviderState()) + .isEqualTo(ProviderState.READY); + } } From 98c620f14a81a0fda47207eb9d912826a33ec446 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Tue, 10 Sep 2024 08:28:49 +0200 Subject: [PATCH 05/23] add test Signed-off-by: christian.lutnik --- .../memory/InMemoryProviderTest.java | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) 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 cb7770d48..101f64477 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -1,12 +1,9 @@ package dev.openfeature.sdk.providers.memory; import com.google.common.collect.ImmutableMap; -import dev.openfeature.sdk.Client; -import dev.openfeature.sdk.EventDetails; -import dev.openfeature.sdk.ImmutableContext; -import dev.openfeature.sdk.OpenFeatureAPI; -import dev.openfeature.sdk.Value; +import dev.openfeature.sdk.*; import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.exceptions.ProviderNotReadyError; import dev.openfeature.sdk.exceptions.TypeMismatchError; import lombok.SneakyThrows; @@ -19,16 +16,11 @@ import static dev.openfeature.sdk.Structure.mapToStructure; import static dev.openfeature.sdk.testutils.TestFlagsUtils.buildFlags; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.*; class InMemoryProviderTest { @@ -41,15 +33,16 @@ class InMemoryProviderTest { void beforeEach() { Map> flags = buildFlags(); provider = spy(new InMemoryProvider(flags)); - OpenFeatureAPI.getInstance().onProviderConfigurationChanged(eventDetails -> {}); + OpenFeatureAPI.getInstance().onProviderConfigurationChanged(eventDetails -> { + }); OpenFeatureAPI.getInstance().setProviderAndWait(provider); client = OpenFeatureAPI.getInstance().getClient(); provider.updateFlags(flags); provider.updateFlag("addedFlag", Flag.builder() - .variant("on", true) - .variant("off", false) - .defaultVariant("on") - .build()); + .variant("on", true) + .variant("off", false) + .defaultVariant("on") + .build()); } @SneakyThrows @@ -81,9 +74,9 @@ void getDoubleEvaluation() { @Test void getObjectEvaluation() { Value expectedObject = new Value(mapToStructure(ImmutableMap.of( - "showImages", new Value(true), - "title", new Value("Check out these pics!"), - "imagesPerPage", new Value(100) + "showImages", new Value(true), + "title", new Value("Check out these pics!"), + "imagesPerPage", new Value(100) ))); assertEquals(expectedObject, client.getObjectValue("object-flag", new Value(true))); } @@ -108,7 +101,7 @@ 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(ProviderNotReadyError.class, () -> inMemoryProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext())); } @SuppressWarnings("unchecked") @@ -125,4 +118,15 @@ void emitChangedFlagsOnlyIfThereAreChangedFlags() { await().untilAsserted(() -> verify(handler, times(1)) .accept(argThat(details -> details.getFlagsChanged().size() == buildFlags().size()))); } + + @SneakyThrows + @Test + void shouldThrowGeneralErrorAfterEmittingErrorEvent() { + InMemoryProvider inMemoryProvider = new InMemoryProvider(new HashMap<>()); + inMemoryProvider.initialize(null); + inMemoryProvider.emitProviderError(ProviderEventDetails.builder().build()); + + // ErrorCode.GENERAL should be returned when evaluated via the client + assertThrows(GeneralError.class, () -> inMemoryProvider.getBooleanEvaluation("fail_in_error", false, new ImmutableContext())); + } } \ No newline at end of file From d0a202739172f3e7755110ec9ac4b79d6b77e80b Mon Sep 17 00:00:00 2001 From: chrfwow Date: Wed, 11 Sep 2024 09:06:38 +0200 Subject: [PATCH 06/23] implement feedback from codereview Signed-off-by: christian.lutnik --- .../sdk/DeveloperExperienceTest.java | 39 ++-- .../openfeature/sdk/EventProviderTest.java | 50 ++++- .../java/dev/openfeature/sdk/EventsTest.java | 20 +- .../sdk/FlagEvaluationSpecTest.java | 9 +- .../dev/openfeature/sdk/HookSpecTest.java | 195 ++++++++++-------- .../sdk/ProviderRepositoryTest.java | 60 +++--- .../sdk/testutils/TestEventsProvider.java | 6 +- 7 files changed, 229 insertions(+), 150 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java index 8c3c0ae98..dc4c735e8 100644 --- a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java +++ b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java @@ -3,6 +3,7 @@ import dev.openfeature.sdk.fixtures.HookFixtures; import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; import dev.openfeature.sdk.testutils.TestEventsProvider; +import lombok.SneakyThrows; import org.junit.jupiter.api.Test; import java.util.*; @@ -16,19 +17,21 @@ class DeveloperExperienceTest implements HookFixtures { transient String flagKey = "mykey"; - @Test void simpleBooleanFlag() throws Exception { + @Test + void simpleBooleanFlag() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(TestEventsProvider.initialized()); + api.setProviderAndWait(new TestEventsProvider()); Client client = api.getClient(); Boolean retval = client.getBooleanValue(flagKey, false); assertFalse(retval); } - @Test void clientHooks() throws Exception { + @Test + void clientHooks() { Hook exampleHook = mockBooleanHook(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(TestEventsProvider.initialized()); + api.setProviderAndWait(new TestEventsProvider()); Client client = api.getClient(); client.addHooks(exampleHook); Boolean retval = client.getBooleanValue(flagKey, false); @@ -36,12 +39,13 @@ class DeveloperExperienceTest implements HookFixtures { assertFalse(retval); } - @Test void evalHooks() throws Exception { + @Test + void evalHooks() { Hook clientHook = mockBooleanHook(); Hook evalHook = mockBooleanHook(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(TestEventsProvider.initialized()); + api.setProviderAndWait(new TestEventsProvider()); Client client = api.getClient(); client.addHooks(clientHook); Boolean retval = client.getBooleanValue(flagKey, false, null, @@ -55,10 +59,11 @@ class DeveloperExperienceTest implements HookFixtures { * As an application author, you probably know special things about your users. You can communicate these to the * provider via {@link MutableContext} */ - @Test void providingContext() throws Exception { + @Test + void providingContext() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(TestEventsProvider.initialized()); + api.setProviderAndWait(new TestEventsProvider()); Client client = api.getClient(); Map attributes = new HashMap<>(); List values = Arrays.asList(new Value(2), new Value(4)); @@ -72,7 +77,8 @@ class DeveloperExperienceTest implements HookFixtures { assertFalse(retval); } - @Test void brokenProvider() { + @Test + void brokenProvider() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); FeatureProviderTestUtils.setFeatureProvider(new AlwaysBrokenProvider()); Client client = api.getClient(); @@ -89,17 +95,16 @@ void providerLockedPerTransaction() { class MutatingHook implements Hook { @Override + @SneakyThrows // change the provider during a before hook - this should not impact the evaluation in progress - public Optional before(HookContext ctx, Map hints) { - try { - FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.initialized()); - } catch (Exception e) { - throw new RuntimeException(e); - } + public Optional before(HookContext ctx, Map hints) { + + FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.newInitializedTestEventsProvider()); + return Optional.empty(); } } - + final String defaultValue = "string-value"; final OpenFeatureAPI api = OpenFeatureAPI.getInstance(); final Client client = api.getClient(); @@ -111,7 +116,7 @@ public Optional before(HookContext ctx, Map hints) { assertEquals(new StringBuilder(defaultValue).reverse().toString(), doSomethingValue); api.clearHooks(); - + // subsequent evaluations should now use new provider set by hook String noOpValue = client.getStringValue("val", defaultValue); assertEquals(noOpValue, defaultValue); diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index 89e95ca67..37f28816a 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -3,6 +3,7 @@ import dev.openfeature.sdk.exceptions.FatalError; import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.internal.TriConsumer; +import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -19,7 +20,8 @@ class EventProviderTest { private TestEventProvider eventProvider; @BeforeEach - void setup() throws Exception { + @SneakyThrows + void setup() { eventProvider = new TestEventProvider(); eventProvider.initialize(null); } @@ -95,8 +97,9 @@ protected void doInitialization(EvaluationContext evaluationContext) { } @Test + @SneakyThrows @DisplayName("sets the state to READY after running the initialize method") - void setsStateToReadyAfterInit() throws Exception { + void setsStateToReadyAfterInit() { AtomicBoolean doInitializationCalled = new AtomicBoolean(); EventProvider provider = new TestEventProvider() { @Override @@ -141,6 +144,49 @@ protected void doInitialization(EvaluationContext evaluationContext) { assertThat(doInitializationCalled).isTrue(); } + @Test + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @DisplayName("sets the state to ERROR when an error event is emitted") + void setsStateToErrorWhenErrorEventIsEmitted() { + EventProvider provider = new TestEventProvider() { + @Override + protected void doInitialization(EvaluationContext evaluationContext) { + } + }; + assertThat(provider.getState()).isNotEqualTo(ProviderState.ERROR); + provider.emitProviderError(ProviderEventDetails.builder().build()); + assertThat(provider.getState()).isEqualTo(ProviderState.ERROR); + } + + @Test + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @DisplayName("sets the state to STALE when a stale event is emitted") + void setsStateToStaleWhenStaleEventIsEmitted() { + EventProvider provider = new TestEventProvider() { + @Override + protected void doInitialization(EvaluationContext evaluationContext) { + } + }; + assertThat(provider.getState()).isNotEqualTo(ProviderState.STALE); + provider.emitProviderStale(ProviderEventDetails.builder().build()); + assertThat(provider.getState()).isEqualTo(ProviderState.STALE); + } + + @Test + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @DisplayName("sets the state to READY when a ready event is emitted") + void setsStateToReadyWhenReadyEventIsEmitted() { + EventProvider provider = new TestEventProvider() { + @Override + protected void doInitialization(EvaluationContext evaluationContext) { + } + }; + provider.emitProviderStale(ProviderEventDetails.builder().build()); + assertThat(provider.getState()).isNotEqualTo(ProviderState.READY); + provider.emitProviderReady(ProviderEventDetails.builder().build()); + assertThat(provider.getState()).isEqualTo(ProviderState.READY); + } + static class TestEventProvider extends EventProvider { private static final String NAME = "TestEventProvider"; diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 64a40549c..3e6cfb1b3 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -2,6 +2,7 @@ import dev.openfeature.sdk.testutils.TestEventsProvider; import io.cucumber.java.AfterAll; +import lombok.SneakyThrows; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -332,7 +333,7 @@ void shouldSupportAllEventTypes() { @Test @DisplayName("shutdown provider should not run handlers") - void shouldNotRunHandlers() { + void shouldNotRunHandlers() { final Consumer handler1 = mockHandler(); final Consumer handler2 = mockHandler(); final String name = "shouldNotRunHandlers"; @@ -510,13 +511,13 @@ void shouldHaveAllProperties() { @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingReadyEventsMustRunImmediately() { + void matchingReadyEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already ready TestEventsProvider provider = new TestEventsProvider(); - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); // should run even thought handler was added after ready Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -527,12 +528,12 @@ void matchingReadyEventsMustRunImmediately() { @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingStaleEventsMustRunImmediately() throws Exception { + void matchingStaleEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already stale - TestEventsProvider provider =TestEventsProvider.initialized(); + TestEventsProvider provider = TestEventsProvider.newInitializedTestEventsProvider(); provider.emitProviderStale(null); assertThat(provider.getState()).isEqualTo(ProviderState.STALE); OpenFeatureAPI.getInstance().setProvider(name, provider); @@ -546,12 +547,12 @@ void matchingStaleEventsMustRunImmediately() throws Exception { @Test @DisplayName("if the provider is ready handlers must run immediately") @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.") - void matchingErrorEventsMustRunImmediately() throws Exception { + void matchingErrorEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); // provider which is already in error - TestEventsProvider provider = TestEventsProvider.initialized(); + TestEventsProvider provider = TestEventsProvider.newInitializedTestEventsProvider(); provider.emitProviderError(null); assertThat(provider.getState()).isEqualTo(ProviderState.ERROR); OpenFeatureAPI.getInstance().setProvider(name, provider); @@ -593,10 +594,11 @@ void mustPersistAcrossChanges() { @Nested class HandlerRemoval { - @Specification(number="5.2.7", text="The API and client MUST provide a function allowing the removal of event handlers.") + @Specification(number = "5.2.7", text = "The API and client MUST provide a function allowing the removal of event handlers.") @Test @DisplayName("should not run removed events") - void removedEventsShouldNotRun() throws Exception { + @SneakyThrows + void removedEventsShouldNotRun() { final String name = "removedEventsShouldNotRun"; final Consumer handler1 = mockHandler(); final Consumer handler2 = mockHandler(); diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 00df6bc28..fe2a8e9d1 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -6,6 +6,7 @@ import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; 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; @@ -35,7 +36,8 @@ private Client _client() { return api.getClient(); } - private Client _initializedClient() throws Exception { + @SneakyThrows + private Client _initializedClient() { TestEventsProvider provider = new TestEventsProvider(); provider.initialize(null); FeatureProviderTestUtils.setFeatureProvider(provider); @@ -101,7 +103,7 @@ void getApiInstance() { FeatureProvider provider = new InMemoryProvider(new HashMap<>()) { @Override protected void doInitialization(EvaluationContext evaluationContext) throws Exception { - Thread.sleep(10000); + Awaitility.await().wait(3000); } }; String providerName = "shouldReturnNotReadyIfNotInitialized"; @@ -235,7 +237,8 @@ protected void doInitialization(EvaluationContext evaluationContext) throws Exce } @Specification(number="1.5.1", text="The evaluation options structure's hooks field denotes an ordered collection of hooks that the client MUST execute for the respective flag evaluation, in addition to those already configured.") - @Test void hooks() throws Exception { + @SneakyThrows + @Test void hooks() { Client c = _initializedClient(); Hook clientHook = mockBooleanHook(); Hook invocationHook = mockBooleanHook(); diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index f988e822b..2b29acedb 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -25,8 +25,9 @@ void emptyApiHooks() { OpenFeatureAPI.getInstance().clearHooks(); } - @Specification(number="4.1.3", text="The flag key, flag type, and default value properties MUST be immutable. If the language does not support immutability, the hook MUST NOT modify these properties.") - @Test void immutableValues() { + @Specification(number = "4.1.3", text = "The flag key, flag type, and default value properties MUST be immutable. If the language does not support immutability, the hook MUST NOT modify these properties.") + @Test + void immutableValues() { try { HookContext.class.getMethod("setFlagKey"); fail("Shouldn't be able to find this method"); @@ -49,8 +50,9 @@ void emptyApiHooks() { } } - @Specification(number="4.1.1", text="Hook context MUST provide: the flag key, flag value type, evaluation context, and the default value.") - @Test void nullish_properties_on_hookcontext() { + @Specification(number = "4.1.1", text = "Hook context MUST provide: the flag key, flag value type, evaluation context, and the default value.") + @Test + void nullish_properties_on_hookcontext() { // missing ctx try { HookContext.builder() @@ -113,8 +115,9 @@ void emptyApiHooks() { } - @Specification(number="4.1.2", text="The hook context SHOULD provide: access to the client metadata and the provider metadata fields.") - @Test void optional_properties() { + @Specification(number = "4.1.2", text = "The hook context SHOULD provide: access to the client metadata and the provider metadata fields.") + @Test + void optional_properties() { // don't specify HookContext.builder() .flagKey("key") @@ -142,8 +145,9 @@ void emptyApiHooks() { .build(); } - @Specification(number="4.3.2.1", text="The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters and returns either an evaluation context or nothing.") - @Test void before_runs_ahead_of_evaluation() { + @Specification(number = "4.3.2.1", text = "The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters and returns either an evaluation context or nothing.") + @Test + void before_runs_ahead_of_evaluation() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new AlwaysBrokenProvider()); Client client = api.getClient(); @@ -155,13 +159,15 @@ void emptyApiHooks() { verify(evalHook, times(1)).before(any(), any()); } - @Test void feo_has_hook_list() { + @Test + void feo_has_hook_list() { FlagEvaluationOptions feo = FlagEvaluationOptions.builder() .build(); assertNotNull(feo.getHooks()); } - @Test void error_hook_run_during_non_finally_stage() { + @Test + void error_hook_run_during_non_finally_stage() { final boolean[] error_called = {false}; Hook h = mockBooleanHook(); doThrow(RuntimeException.class).when(h).finallyAfter(any(), any()); @@ -170,7 +176,8 @@ void emptyApiHooks() { } - @Test void error_hook_must_run_if_resolution_details_returns_an_error_code() { + @Test + void error_hook_must_run_if_resolution_details_returns_an_error_code() { String errorMessage = "not found..."; @@ -195,7 +202,7 @@ void emptyApiHooks() { verify(hook, times(1)).before(any(), any()); verify(hook, times(1)).error(any(), captor.capture(), any()); verify(hook, times(1)).finallyAfter(any(), any()); - verify(hook, never()).after(any(),any(), any()); + verify(hook, never()).after(any(), any(), any()); Exception exception = captor.getValue(); assertEquals(errorMessage, exception.getMessage()); @@ -203,12 +210,13 @@ void emptyApiHooks() { } - @Specification(number="4.3.6", text="The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.") - @Specification(number="4.3.7", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value.") - @Specification(number="4.3.8", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and hook hints (optional). There is no return value.") - @Specification(number="4.4.1", text="The API, Client, Provider, and invocation MUST have a method for registering hooks.") - @Specification(number="4.4.2", text="Hooks MUST be evaluated in the following order: - before: API, Client, Invocation, Provider - after: Provider, Invocation, Client, API - error (if applicable): Provider, Invocation, Client, API - finally: Provider, Invocation, Client, API") - @Test void hook_eval_order() { + @Specification(number = "4.3.6", text = "The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.") + @Specification(number = "4.3.7", text = "The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value.") + @Specification(number = "4.3.8", text = "The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and hook hints (optional). There is no return value.") + @Specification(number = "4.4.1", text = "The API, Client, Provider, and invocation MUST have a method for registering hooks.") + @Specification(number = "4.4.2", text = "Hooks MUST be evaluated in the following order: - before: API, Client, Invocation, Provider - after: Provider, Invocation, Client, API - error (if applicable): Provider, Invocation, Client, API - finally: Provider, Invocation, Client, API") + @Test + void hook_eval_order() { List evalOrder = new ArrayList<>(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new TestEventsProvider() { @@ -288,41 +296,42 @@ public void finallyAfter(HookContext ctx, Map hints) { }); c.getBooleanValue("key", false, null, FlagEvaluationOptions - .builder() - .hook(new BooleanHook() { - @Override - public Optional before(HookContext ctx, Map hints) { - evalOrder.add("invocation before"); - return null; - } - - @Override - public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { - evalOrder.add("invocation after"); - } - - @Override - public void error(HookContext ctx, Exception error, Map hints) { - evalOrder.add("invocation error"); - } - - @Override - public void finallyAfter(HookContext ctx, Map hints) { - evalOrder.add("invocation finally"); - } - }) - .build()); + .builder() + .hook(new BooleanHook() { + @Override + public Optional before(HookContext ctx, Map hints) { + evalOrder.add("invocation before"); + return null; + } + + @Override + public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + evalOrder.add("invocation after"); + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + evalOrder.add("invocation error"); + } + + @Override + public void finallyAfter(HookContext ctx, Map hints) { + evalOrder.add("invocation finally"); + } + }) + .build()); List expectedOrder = Arrays.asList( - "api before", "client before", "invocation before", "provider before", - "provider after", "invocation after", "client after", "api after", - "provider error", "invocation error", "client error", "api error", - "provider finally", "invocation finally", "client finally", "api finally"); + "api before", "client before", "invocation before", "provider before", + "provider after", "invocation after", "client after", "api after", + "provider error", "invocation error", "client error", "api error", + "provider finally", "invocation finally", "client finally", "api finally"); assertEquals(expectedOrder, evalOrder); } - @Specification(number="4.4.6", text="If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") - @Test void error_stops_before() { + @Specification(number = "4.4.6", text = "If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") + @Test + void error_stops_before() { Hook h = mockBooleanHook(); doThrow(RuntimeException.class).when(h).before(any(), any()); Hook h2 = mockBooleanHook(); @@ -339,13 +348,15 @@ public void finallyAfter(HookContext ctx, Map hints) { verify(h2, times(0)).before(any(), any()); } - @Specification(number="4.4.6", text="If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") - @Test void error_stops_after() throws Exception { + @Specification(number = "4.4.6", text = "If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") + @SneakyThrows + @Test + void error_stops_after() { Hook h = mockBooleanHook(); doThrow(RuntimeException.class).when(h).after(any(), any(), any()); Hook h2 = mockBooleanHook(); - Client c = getClient(TestEventsProvider.initialized()); + Client c = getClient(TestEventsProvider.newInitializedTestEventsProvider()); c.getBooleanDetails("key", false, null, FlagEvaluationOptions.builder() .hook(h) @@ -355,11 +366,13 @@ public void finallyAfter(HookContext ctx, Map hints) { verify(h2, times(0)).after(any(), any(), any()); } - @Specification(number="4.2.1", text="hook hints MUST be a structure supports definition of arbitrary properties, with keys of type string, and values of type boolean | string | number | datetime | structure..") - @Specification(number="4.5.2", text="hook hints MUST be passed to each hook.") - @Specification(number="4.2.2.1", text="Condition: Hook hints MUST be immutable.") - @Specification(number="4.5.3", text="The hook MUST NOT alter the hook hints structure.") - @Test void hook_hints() throws Exception { + @Specification(number = "4.2.1", text = "hook hints MUST be a structure supports definition of arbitrary properties, with keys of type string, and values of type boolean | string | number | datetime | structure..") + @Specification(number = "4.5.2", text = "hook hints MUST be passed to each hook.") + @Specification(number = "4.2.2.1", text = "Condition: Hook hints MUST be immutable.") + @Specification(number = "4.5.3", text = "The hook MUST NOT alter the hook hints structure.") + @SneakyThrows + @Test + void hook_hints() { String hintKey = "My hint key"; Client client = getClient(null); Hook mutatingHook = new BooleanHook() { @@ -395,14 +408,16 @@ public void finallyAfter(HookContext ctx, Map hints) { .build()); } - @Specification(number="4.5.1", text="Flag evaluation options MAY contain hook hints, a map of data to be provided to hook invocations.") - @Test void missing_hook_hints() { + @Specification(number = "4.5.1", text = "Flag evaluation options MAY contain hook hints, a map of data to be provided to hook invocations.") + @Test + void missing_hook_hints() { FlagEvaluationOptions feo = FlagEvaluationOptions.builder().build(); assertNotNull(feo.getHookHints()); assertTrue(feo.getHookHints().isEmpty()); } - @Test void flag_eval_hook_order() { + @Test + void flag_eval_hook_order() { Hook hook = mockBooleanHook(); FeatureProvider provider = mock(FeatureProvider.class); when(provider.getBooleanEvaluation(any(), any(), any())) @@ -423,12 +438,13 @@ public void finallyAfter(HookContext ctx, Map hints) { order.verify(hook).finallyAfter(any(), any()); } - @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") - @Specification(number="4.4.7", text="If an error occurs in the before hooks, the default value MUST be returned.") - @Test void error_hooks__before() throws Exception { + @Specification(number = "4.4.5", text = "If an error occurs in the before or after hooks, the error hooks MUST be invoked.") + @Specification(number = "4.4.7", text = "If an error occurs in the before hooks, the default value MUST be returned.") + @Test + void error_hooks__before() { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); - Client client = getClient(TestEventsProvider.initialized()); + Client client = getClient(TestEventsProvider.newInitializedTestEventsProvider()); Boolean value = client.getBooleanValue("key", false, new ImmutableContext(), FlagEvaluationOptions.builder().hook(hook).build()); verify(hook, times(1)).before(any(), any()); @@ -436,18 +452,20 @@ public void finallyAfter(HookContext ctx, Map hints) { assertEquals(false, value, "Falls through to the default."); } - @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") - @Test void error_hooks__after() throws Exception { + @Specification(number = "4.4.5", text = "If an error occurs in the before or after hooks, the error hooks MUST be invoked.") + @Test + void error_hooks__after() { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).after(any(), any(), any()); - Client client = getClient(TestEventsProvider.initialized()); + Client client = getClient(TestEventsProvider.newInitializedTestEventsProvider()); client.getBooleanValue("key", false, new ImmutableContext(), FlagEvaluationOptions.builder().hook(hook).build()); verify(hook, times(1)).after(any(), any(), any()); verify(hook, times(1)).error(any(), any(), any()); } - @Test void multi_hooks_early_out__before() throws Exception { + @Test + void multi_hooks_early_out__before() { Hook hook = mockBooleanHook(); Hook hook2 = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); @@ -469,7 +487,8 @@ public void finallyAfter(HookContext ctx, Map hints) { @Specification(number = "4.1.4", text = "The evaluation context MUST be mutable only within the before hook.") @Specification(number = "4.3.4", text = "Any `evaluation context` returned from a `before` hook MUST be passed to subsequent `before` hooks (via `HookContext`).") - @Test void beforeContextUpdated() throws Exception { + @Test + void beforeContextUpdated() { String targetingKey = "test-key"; EvaluationContext ctx = new ImmutableContext(targetingKey); Hook hook = mockBooleanHook(); @@ -494,15 +513,16 @@ public void finallyAfter(HookContext ctx, Map hints) { } - @Specification(number="4.3.5", text="When before hooks have finished executing, any resulting evaluation context MUST be merged with the existing evaluation context.") - @Test void mergeHappensCorrectly() { - Map attributes= new HashMap<>(); + @Specification(number = "4.3.5", text = "When before hooks have finished executing, any resulting evaluation context MUST be merged with the existing evaluation context.") + @Test + void mergeHappensCorrectly() { + Map attributes = new HashMap<>(); attributes.put("test", new Value("works")); attributes.put("another", new Value("exists")); EvaluationContext hookCtx = new ImmutableContext(attributes); - Map attributes1= new HashMap<>(); + Map attributes1 = new HashMap<>(); attributes1.put("something", new Value("here")); attributes1.put("test", new Value("broken")); EvaluationContext invocationCtx = new ImmutableContext(attributes1); @@ -531,8 +551,9 @@ public void finallyAfter(HookContext ctx, Map hints) { assertEquals("here", ec.getValue("something").asString()); } - @Specification(number="4.4.3", text="If a finally hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining finally hooks.") - @Test void first_finally_broken() throws Exception { + @Specification(number = "4.4.3", text = "If a finally hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining finally hooks.") + @Test + void first_finally_broken() { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); doThrow(RuntimeException.class).when(hook).finallyAfter(any(), any()); @@ -551,8 +572,9 @@ public void finallyAfter(HookContext ctx, Map hints) { order.verify(hook).finallyAfter(any(), any()); } - @Specification(number="4.4.4", text="If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.") - @Test void first_error_broken() throws Exception { + @Specification(number = "4.4.4", text = "If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.") + @Test + void first_error_broken() { Hook hook = mockBooleanHook(); doThrow(RuntimeException.class).when(hook).before(any(), any()); doThrow(RuntimeException.class).when(hook).error(any(), any(), any()); @@ -571,28 +593,31 @@ public void finallyAfter(HookContext ctx, Map hints) { order.verify(hook).error(any(), any(), any()); } - private Client getClient(FeatureProvider provider) throws Exception { + private Client getClient(FeatureProvider provider) { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); if (provider == null) { - FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.initialized()); + FeatureProviderTestUtils.setFeatureProvider(TestEventsProvider.newInitializedTestEventsProvider()); } else { FeatureProviderTestUtils.setFeatureProvider(provider); } return api.getClient(); } - @Specification(number="4.3.1", text="Hooks MUST specify at least one stage.") - @Test void default_methods_so_impossible() {} + @Specification(number = "4.3.1", text = "Hooks MUST specify at least one stage.") + @Test + void default_methods_so_impossible() { + } - @Specification(number="4.3.9.1", text="Instead of finally, finallyAfter SHOULD be used.") + @Specification(number = "4.3.9.1", text = "Instead of finally, finallyAfter SHOULD be used.") @SneakyThrows - @Test void doesnt_use_finally() { + @Test + void doesnt_use_finally() { assertThatCode(() -> Hook.class.getMethod("finally", HookContext.class, Map.class)) - .as("Not possible. Finally is a reserved word.") - .isInstanceOf(NoSuchMethodException.class); + .as("Not possible. Finally is a reserved word.") + .isInstanceOf(NoSuchMethodException.class); assertThatCode(() -> Hook.class.getMethod("finallyAfter", HookContext.class, Map.class)) - .doesNotThrowAnyException(); + .doesNotThrowAnyException(); } } diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index c827c91fb..ff522ecf9 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -1,19 +1,12 @@ package dev.openfeature.sdk; -import static dev.openfeature.sdk.fixtures.ProviderFixture.createMockedErrorProvider; -import static dev.openfeature.sdk.fixtures.ProviderFixture.createMockedProvider; -import static dev.openfeature.sdk.fixtures.ProviderFixture.createMockedReadyProvider; -import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doDelayResponse; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.awaitility.Awaitility.await; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.timeout; -import static org.mockito.Mockito.verify; +import dev.openfeature.sdk.exceptions.OpenFeatureError; +import dev.openfeature.sdk.testutils.exception.TestException; +import lombok.SneakyThrows; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; import java.time.Duration; import java.util.concurrent.ExecutorService; @@ -23,13 +16,14 @@ import java.util.function.Consumer; import java.util.function.Function; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Nested; -import org.junit.jupiter.api.Test; - -import dev.openfeature.sdk.exceptions.OpenFeatureError; -import dev.openfeature.sdk.testutils.exception.TestException; +import static dev.openfeature.sdk.fixtures.ProviderFixture.*; +import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doDelayResponse; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.awaitility.Awaitility.await; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.*; class ProviderRepositoryTest { @@ -85,12 +79,13 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { verify(featureProvider, timeout(TIMEOUT)).initialize(any()); } + @SneakyThrows @Test - @DisplayName("should avoid additional initialization call if provider has been initialized already") - void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() throws Exception { + @DisplayName("should avoid additional initialization call if provider has been newInitializedTestEventsProvider already") + void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() { FeatureProvider provider = createMockedReadyProvider(); setFeatureProvider(provider); - + verify(provider, never()).initialize(any()); } } @@ -134,7 +129,7 @@ void shouldImmediatelyReturnWhenCallingTheDomainProviderMutator() throws Excepti } @Test - @DisplayName("should avoid additional initialization call if provider has been initialized already") + @DisplayName("should avoid additional initialization call if provider has been newInitializedTestEventsProvider already") void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() throws Exception { FeatureProvider provider = createMockedReadyProvider(); setFeatureProvider(DOMAIN_NAME, provider); @@ -254,11 +249,11 @@ void shouldRunLambdasOnSuccessful() { Consumer afterInit = mock(Consumer.class); Consumer afterShutdown = mock(Consumer.class); BiConsumer afterError = mock(BiConsumer.class); - + FeatureProvider oldProvider = providerRepository.getProvider(); FeatureProvider featureProvider1 = createMockedProvider(); FeatureProvider featureProvider2 = createMockedProvider(); - + setFeatureProvider(featureProvider1, afterSet, afterInit, afterShutdown, afterError); setFeatureProvider(featureProvider2); verify(afterSet, timeout(TIMEOUT)).accept(featureProvider1); @@ -275,12 +270,13 @@ void shouldRunLambdasOnError() throws Exception { Consumer afterInit = mock(Consumer.class); Consumer afterShutdown = mock(Consumer.class); BiConsumer afterError = mock(BiConsumer.class); - + FeatureProvider errorFeatureProvider = createMockedErrorProvider(); - + setFeatureProvider(errorFeatureProvider, afterSet, afterInit, afterShutdown, afterError); verify(afterSet, timeout(TIMEOUT)).accept(errorFeatureProvider); - verify(afterInit, never()).accept(any());; + verify(afterInit, never()).accept(any()); + ; verify(afterError, timeout(TIMEOUT)).accept(eq(errorFeatureProvider), any()); } } @@ -309,8 +305,8 @@ private void setFeatureProvider(FeatureProvider provider) { private void setFeatureProvider(FeatureProvider provider, Consumer afterSet, - Consumer afterInit, Consumer afterShutdown, - BiConsumer afterError) { + Consumer afterInit, Consumer afterShutdown, + BiConsumer afterError) { providerRepository.setProvider(provider, afterSet, afterInit, afterShutdown, afterError, false); waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider); diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java index 5334cee04..a337ae61a 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -2,6 +2,7 @@ import dev.openfeature.sdk.*; import dev.openfeature.sdk.exceptions.GeneralError; +import lombok.SneakyThrows; public class TestEventsProvider extends EventProvider { public static final String PASSED_IN_DEFAULT = "Passed in default"; @@ -26,7 +27,8 @@ public TestEventsProvider(int initTimeoutMs, boolean initError, String initError this.initErrorMessage = initErrorMessage; } - public static TestEventsProvider initialized() throws Exception { + @SneakyThrows + public static TestEventsProvider newInitializedTestEventsProvider() { TestEventsProvider provider = new TestEventsProvider(); provider.initialize(null); return provider; @@ -106,4 +108,4 @@ public ProviderEvaluation getObjectEvaluation(String key, Value defaultVa .reason(Reason.DEFAULT.toString()) .build(); } -}; \ No newline at end of file +} From 11a0ccb36af721bbb7c987ce285932132be860e7 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Thu, 12 Sep 2024 08:42:06 +0200 Subject: [PATCH 07/23] add wrapper around EventProvider, to manage and hide the provider state Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/EventProvider.java | 66 ++----- .../sdk/EventProviderListener.java | 6 + .../dev/openfeature/sdk/FeatureProvider.java | 3 + .../sdk/FeatureProviderWrapper.java | 118 ++++++++++++ .../dev/openfeature/sdk/NoOpProvider.java | 13 +- .../dev/openfeature/sdk/OpenFeatureAPI.java | 18 ++ .../openfeature/sdk/ProviderRepository.java | 104 +++++----- .../providers/memory/InMemoryProvider.java | 48 +++-- .../openfeature/sdk/EventProviderTest.java | 106 ----------- .../java/dev/openfeature/sdk/EventsTest.java | 21 ++- .../sdk/FeatureProviderWrapperTest.java | 177 ++++++++++++++++++ .../sdk/FlagEvaluationSpecTest.java | 14 +- .../sdk/ProviderRepositoryTest.java | 24 +-- .../memory/InMemoryProviderTest.java | 11 -- .../testutils/FeatureProviderTestUtils.java | 2 +- .../sdk/testutils/TestEventsProvider.java | 14 +- 16 files changed, 472 insertions(+), 273 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/EventProviderListener.java create mode 100644 src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java create mode 100644 src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index b6f8afcb6..fbfa8d751 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -1,9 +1,9 @@ package dev.openfeature.sdk; -import dev.openfeature.sdk.exceptions.GeneralError; -import dev.openfeature.sdk.exceptions.OpenFeatureError; import dev.openfeature.sdk.internal.TriConsumer; +import javax.annotation.Nullable; + /** * Abstract EventProvider. Providers must extend this class to support events. * Emit events with {@link #emit(ProviderEvent, ProviderEventDetails)}. Please @@ -17,47 +17,10 @@ * @see FeatureProvider */ public abstract class EventProvider implements FeatureProvider { + private @Nullable EventProviderListener eventProviderListener; - private ProviderState providerState = ProviderState.NOT_READY; - - /** - * {@inheritDoc} - */ - @Override - public final ProviderState getState() { - return providerState; - } - - @Override - public final void initialize(EvaluationContext evaluationContext) throws Exception { - try { - doInitialization(evaluationContext); - providerState = ProviderState.READY; - } catch (OpenFeatureError openFeatureError) { - if (ErrorCode.PROVIDER_FATAL.equals(openFeatureError.getErrorCode())) { - providerState = ProviderState.FATAL; - } else { - providerState = ProviderState.ERROR; - } - throw openFeatureError; - } catch (Exception e) { - providerState = ProviderState.ERROR; - throw new GeneralError(e); - } - } - - protected void doInitialization(EvaluationContext evaluationContext) throws Exception { - // Intentionally left blank, to be implemented by inheritors - } - - @Override - public final void shutdown() { - providerState = ProviderState.NOT_READY; - doShutdown(); - } - - protected void doShutdown() { - // Intentionally left blank, to be implemented by inheritors + void setEventProviderListener(@Nullable EventProviderListener eventProviderListener) { + this.eventProviderListener = eventProviderListener; } private TriConsumer onEmit = null; @@ -92,12 +55,8 @@ void detach() { * @param details The details of the event */ public void emit(ProviderEvent event, ProviderEventDetails details) { - if (ProviderEvent.PROVIDER_ERROR.equals(event)) { - providerState = ProviderState.ERROR; - } else if (ProviderEvent.PROVIDER_STALE.equals(event)) { - providerState = ProviderState.STALE; - } else if (ProviderEvent.PROVIDER_READY.equals(event)) { - providerState = ProviderState.READY; + if (eventProviderListener != null) { + eventProviderListener.onEmit(event, details); } if (this.onEmit != null) { this.onEmit.accept(this, event, details); @@ -144,4 +103,15 @@ public void emitProviderStale(ProviderEventDetails details) { public void emitProviderError(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_ERROR, details); } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (obj instanceof FeatureProviderWrapper) { + return this == ((FeatureProviderWrapper) obj).getDelegate(); + } + return this == obj; + } } diff --git a/src/main/java/dev/openfeature/sdk/EventProviderListener.java b/src/main/java/dev/openfeature/sdk/EventProviderListener.java new file mode 100644 index 000000000..c1f839aab --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/EventProviderListener.java @@ -0,0 +1,6 @@ +package dev.openfeature.sdk; + +@FunctionalInterface +interface EventProviderListener { + void onEmit(ProviderEvent event, ProviderEventDetails details); +} diff --git a/src/main/java/dev/openfeature/sdk/FeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProvider.java index e1e06d0ab..7319d6f09 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProvider.java @@ -56,6 +56,8 @@ default void shutdown() { } /** + * @deprecated State is handled by the SDK internally. + *

* Returns a representation of the current readiness of the provider. * If the provider needs to be initialized, it should return {@link ProviderState#NOT_READY}. * If the provider is in an error state, it should return {@link ProviderState#ERROR}. @@ -65,6 +67,7 @@ default void shutdown() { * * @return ProviderState */ + @Deprecated default ProviderState getState() { return ProviderState.READY; } diff --git a/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java b/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java new file mode 100644 index 000000000..36b37c71d --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java @@ -0,0 +1,118 @@ +package dev.openfeature.sdk; + +import dev.openfeature.sdk.exceptions.OpenFeatureError; + +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; + +public class FeatureProviderWrapper implements FeatureProvider, EventProviderListener { + private final FeatureProvider delegate; + private final AtomicBoolean isInitialized = new AtomicBoolean(); + private ProviderState state = ProviderState.NOT_READY; + + public FeatureProviderWrapper(FeatureProvider delegate) { + this.delegate = delegate; + if (delegate instanceof EventProvider) { + ((EventProvider) delegate).setEventProviderListener(this); + } + } + + @Override + public Metadata getMetadata() { + return delegate.getMetadata(); + } + + @Override + public List getProviderHooks() { + return delegate.getProviderHooks(); + } + + @Override + public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { + return delegate.getBooleanEvaluation(key, defaultValue, ctx); + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { + return delegate.getStringEvaluation(key, defaultValue, ctx); + } + + @Override + public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { + return delegate.getIntegerEvaluation(key, defaultValue, ctx); + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { + return delegate.getDoubleEvaluation(key, defaultValue, ctx); + } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { + return delegate.getObjectEvaluation(key, defaultValue, ctx); + } + + @Override + public void initialize(EvaluationContext evaluationContext) throws Exception { + if (isInitialized.getAndSet(true)) { + return; + } + try { + delegate.initialize(evaluationContext); + state = ProviderState.READY; + } catch (OpenFeatureError openFeatureError) { + if (ErrorCode.PROVIDER_FATAL.equals(openFeatureError.getErrorCode())) { + state = ProviderState.FATAL; + } else { + state = ProviderState.ERROR; + } + isInitialized.set(false); + throw openFeatureError; + } catch (Exception e) { + state = ProviderState.ERROR; + isInitialized.set(false); + throw e; + } + } + + @Override + public void shutdown() { + delegate.shutdown(); + state = ProviderState.NOT_READY; + isInitialized.set(false); + } + + @Override + public ProviderState getState() { + return state; + } + + @Override + public int hashCode() { + return delegate.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj instanceof FeatureProviderWrapper) { + return delegate.equals(((FeatureProviderWrapper) obj).delegate); + } + return delegate.equals(obj); + } + + @Override + public void onEmit(ProviderEvent event, ProviderEventDetails details) { + if (ProviderEvent.PROVIDER_ERROR.equals(event)) { + state = ProviderState.ERROR; + } else if (ProviderEvent.PROVIDER_STALE.equals(event)) { + state = ProviderState.STALE; + } else if (ProviderEvent.PROVIDER_READY.equals(event)) { + state = ProviderState.READY; + } + } + + FeatureProvider getDelegate(){ + return delegate; + } +} diff --git a/src/main/java/dev/openfeature/sdk/NoOpProvider.java b/src/main/java/dev/openfeature/sdk/NoOpProvider.java index ef8cf1f83..a8f6e7ac2 100644 --- a/src/main/java/dev/openfeature/sdk/NoOpProvider.java +++ b/src/main/java/dev/openfeature/sdk/NoOpProvider.java @@ -59,11 +59,22 @@ public ProviderEvaluation getDoubleEvaluation(String key, Double default @Override public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, - EvaluationContext invocationContext) { + EvaluationContext invocationContext) { return ProviderEvaluation.builder() .value(defaultValue) .variant(PASSED_IN_DEFAULT) .reason(Reason.DEFAULT.toString()) .build(); } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (obj instanceof FeatureProviderWrapper) { + return ((FeatureProviderWrapper) obj).getDelegate() == this; + } + return obj == this; + } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 5131d1328..b354607af 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -275,6 +275,13 @@ public FeatureProvider getProvider() { return providerRepository.getProvider(); } + /** + * Return the state of the default provider. + */ + public ProviderState getProviderState() { + return providerRepository.getProviderState(); + } + /** * Fetch a provider for a domain. If not found, return the default. * @@ -285,6 +292,17 @@ public FeatureProvider getProvider(String domain) { return providerRepository.getProvider(domain); } + /** + * Get the state of the provider for a domain. If no provider with the domain is found, returns the state of the + * default provider. + * + * @param domain The domain to look for. + * @return A named {@link FeatureProvider} + */ + public ProviderState getProviderState(String domain) { + return providerRepository.getProviderState(domain); + } + /** * Adds hooks for globally, used for all evaluations. * Hooks are run in the order they're added in the before stage. They are run in reverse order for all other stages. diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 47b9ecf58..cf2563add 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -1,5 +1,9 @@ package dev.openfeature.sdk; +import dev.openfeature.sdk.exceptions.GeneralError; +import dev.openfeature.sdk.exceptions.OpenFeatureError; +import lombok.extern.slf4j.Slf4j; + import java.util.List; import java.util.Map; import java.util.Optional; @@ -13,15 +17,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import dev.openfeature.sdk.exceptions.GeneralError; -import dev.openfeature.sdk.exceptions.OpenFeatureError; -import lombok.extern.slf4j.Slf4j; - @Slf4j class ProviderRepository { - private final Map providers = new ConcurrentHashMap<>(); - private final AtomicReference defaultProvider = new AtomicReference<>(new NoOpProvider()); + private final Map providers = new ConcurrentHashMap<>(); + private final AtomicReference defaultProvider = new AtomicReference<>(new FeatureProviderWrapper(new NoOpProvider())); private final ExecutorService taskExecutor = Executors.newCachedThreadPool(runnable -> { final Thread thread = new Thread(runnable); thread.setDaemon(true); @@ -45,10 +45,18 @@ public FeatureProvider getProvider(String domain) { return Optional.ofNullable(domain).map(this.providers::get).orElse(this.defaultProvider.get()); } + public ProviderState getProviderState() { + return defaultProvider.get().getState(); + } + + public ProviderState getProviderState(String domain) { + return Optional.ofNullable(domain).map(this.providers::get).orElse(this.defaultProvider.get()).getState(); + } + public List getDomainsForProvider(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 getAllBoundDomains() { @@ -63,11 +71,11 @@ public boolean isDefaultProvider(FeatureProvider provider) { * Set the default provider. */ public void setProvider(FeatureProvider provider, - Consumer afterSet, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError, - boolean waitForInit) { + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError, + boolean waitForInit) { if (provider == null) { throw new IllegalArgumentException("Provider cannot be null"); } @@ -83,12 +91,12 @@ public void setProvider(FeatureProvider provider, * Otherwise, initialization happens in the background. */ public void setProvider(String domain, - FeatureProvider provider, - Consumer afterSet, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError, - boolean waitForInit) { + FeatureProvider provider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError, + boolean waitForInit) { if (provider == null) { throw new IllegalArgumentException("Provider cannot be null"); } @@ -99,12 +107,14 @@ public void setProvider(String domain, } private void prepareAndInitializeProvider(String domain, - FeatureProvider newProvider, - Consumer afterSet, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError, - boolean waitForInit) { + FeatureProvider newProvider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError, + boolean waitForInit) { + + FeatureProviderWrapper newProviderWrapper = new FeatureProviderWrapper(newProvider); if (!isProviderRegistered(newProvider)) { // only run afterSet if new provider is not already attached @@ -112,49 +122,50 @@ private void prepareAndInitializeProvider(String domain, } // provider is set immediately, on this thread - FeatureProvider oldProvider = domain != null - ? this.providers.put(domain, newProvider) - : this.defaultProvider.getAndSet(newProvider); + FeatureProviderWrapper oldProvider = domain != null + ? this.providers.put(domain, newProviderWrapper) + : this.defaultProvider.getAndSet(newProviderWrapper); if (waitForInit) { - initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider); + initializeProvider(newProviderWrapper, afterInit, afterShutdown, afterError, oldProvider); } else { taskExecutor.submit(() -> { // initialization happens in a different thread if we're not waiting it - initializeProvider(newProvider, afterInit, afterShutdown, afterError, oldProvider); + initializeProvider(newProviderWrapper, afterInit, afterShutdown, afterError, oldProvider); }); } } - private void initializeProvider(FeatureProvider newProvider, - Consumer afterInit, - Consumer afterShutdown, - BiConsumer afterError, - FeatureProvider oldProvider) { + private void initializeProvider(FeatureProviderWrapper newProvider, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError, + FeatureProviderWrapper oldProvider) { try { if (ProviderState.NOT_READY.equals(newProvider.getState())) { newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); - afterInit.accept(newProvider); + afterInit.accept(newProvider.getDelegate()); } shutDownOld(oldProvider, afterShutdown); } catch (OpenFeatureError e) { - log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e); - afterError.accept(newProvider, e); + log.error("Exception when initializing feature provider {}", newProvider.getDelegate().getClass().getName(), e); + afterError.accept(newProvider.getDelegate(), e); } catch (Exception e) { - log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e); - afterError.accept(newProvider, new GeneralError(e)); + log.error("Exception when initializing feature provider {}", newProvider.getDelegate().getClass().getName(), e); + afterError.accept(newProvider.getDelegate(), new GeneralError(e)); } } - private void shutDownOld(FeatureProvider oldProvider, Consumer afterShutdown) { - if (!isProviderRegistered(oldProvider)) { + private void shutDownOld(FeatureProviderWrapper oldProvider, Consumer afterShutdown) { + if (oldProvider != null && !isProviderRegistered(oldProvider)) { shutdownProvider(oldProvider); - afterShutdown.accept(oldProvider); + afterShutdown.accept(oldProvider.getDelegate()); } } /** * Helper to check if provider is already known (registered). + * * @param provider provider to check for registration * @return boolean true if already registered, false otherwise */ @@ -163,6 +174,13 @@ private boolean isProviderRegistered(FeatureProvider provider) { && (this.providers.containsValue(provider) || this.defaultProvider.get().equals(provider)); } + private void shutdownProvider(FeatureProviderWrapper provider) { + if (provider == null) { + return; + } + shutdownProvider(provider.getDelegate()); + } + private void shutdownProvider(FeatureProvider provider) { taskExecutor.submit(() -> { try { 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 c0f3ce061..2bea3e4ef 100644 --- a/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java +++ b/src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java @@ -6,7 +6,11 @@ import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; /** @@ -20,6 +24,9 @@ public class InMemoryProvider extends EventProvider { private final Map> flags; + @Getter + private ProviderState state = ProviderState.NOT_READY; + @Override public Metadata getMetadata() { return () -> NAME; @@ -29,6 +36,18 @@ public InMemoryProvider(Map> flags) { this.flags = new ConcurrentHashMap<>(flags); } + /** + * Initializes the provider. + * @param evaluationContext evaluation context + * @throws Exception on error + */ + @Override + public void initialize(EvaluationContext evaluationContext) throws Exception { + super.initialize(evaluationContext); + state = ProviderState.READY; + log.debug("finished initializing provider, state: {}", state); + } + /** * Updates the provider flags configuration. * For existing flags, the new configurations replace the old one. @@ -41,9 +60,9 @@ public void updateFlags(Map> newFlags) { this.flags.putAll(newFlags); ProviderEventDetails details = ProviderEventDetails.builder() - .flagsChanged(new ArrayList<>(flagsChanged)) - .message("flags changed") - .build(); + .flagsChanged(new ArrayList<>(flagsChanged)) + .message("flags changed") + .build(); emitProviderConfigurationChanged(details); } @@ -57,9 +76,9 @@ public void updateFlags(Map> newFlags) { public void updateFlag(String flagKey, Flag newFlag) { this.flags.put(flagKey, newFlag); ProviderEventDetails details = ProviderEventDetails.builder() - .flagsChanged(Collections.singletonList(flagKey)) - .message("flag added/updated") - .build(); + .flagsChanged(Collections.singletonList(flagKey)) + .message("flag added/updated") + .build(); emitProviderConfigurationChanged(details); } @@ -97,11 +116,11 @@ public ProviderEvaluation getObjectEvaluation(String key, Value defaultVa private ProviderEvaluation getEvaluation( String key, EvaluationContext evaluationContext, Class expectedType ) throws OpenFeatureError { - if (!ProviderState.READY.equals(getState())) { - if (ProviderState.NOT_READY.equals(getState())) { + if (!ProviderState.READY.equals(state)) { + if (ProviderState.NOT_READY.equals(state)) { throw new ProviderNotReadyError("provider not yet initialized"); } - if (ProviderState.FATAL.equals(getState())) { + if (ProviderState.FATAL.equals(state)) { throw new FatalError("provider in fatal error state"); } throw new GeneralError("unknown error"); @@ -119,10 +138,9 @@ private ProviderEvaluation getEvaluation( value = (T) flag.getVariants().get(flag.getDefaultVariant()); } return ProviderEvaluation.builder() - .value(value) - .variant(flag.getDefaultVariant()) - .reason(Reason.STATIC.toString()) - .build(); + .value(value) + .variant(flag.getDefaultVariant()) + .reason(Reason.STATIC.toString()) + .build(); } - } diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index 37f28816a..acf2ce6b3 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -81,112 +81,6 @@ void doesNotThrowWhenOnEmitSame() { eventProvider.attach(onEmit2); // should not throw, same instance. noop } - @Test - @DisplayName("the state is NOT_READY before initialize method is called") - void stateNotReadyBeforeCallingInit() { - AtomicBoolean doInitializationCalled = new AtomicBoolean(); - EventProvider provider = new TestEventProvider() { - @Override - protected void doInitialization(EvaluationContext evaluationContext) { - doInitializationCalled.set(true); - } - }; - - assertThat(provider.getState()).isEqualTo(ProviderState.NOT_READY); - assertThat(doInitializationCalled).isFalse(); - } - - @Test - @SneakyThrows - @DisplayName("sets the state to READY after running the initialize method") - void setsStateToReadyAfterInit() { - AtomicBoolean doInitializationCalled = new AtomicBoolean(); - EventProvider provider = new TestEventProvider() { - @Override - protected void doInitialization(EvaluationContext evaluationContext) { - doInitializationCalled.set(true); - } - }; - provider.initialize(null); - assertThat(provider.getState()).isEqualTo(ProviderState.READY); - assertThat(doInitializationCalled).isTrue(); - } - - @Test - @DisplayName("sets the state to ERROR when the doInitialization method throws an error") - void setsStateToErrorWhenFailingInit() { - AtomicBoolean doInitializationCalled = new AtomicBoolean(); - EventProvider provider = new TestEventProvider() { - @Override - protected void doInitialization(EvaluationContext evaluationContext) { - doInitializationCalled.set(true); - throw new RuntimeException(); - } - }; - assertThrows(GeneralError.class, () -> provider.initialize(null)); - assertThat(provider.getState()).isEqualTo(ProviderState.ERROR); - assertThat(doInitializationCalled).isTrue(); - } - - @Test - @DisplayName("sets the state to PROVIDER_FATAL when the doInitialization method throws a fatal error") - void setsStateToFatalWhenFailingInit() { - AtomicBoolean doInitializationCalled = new AtomicBoolean(); - EventProvider provider = new TestEventProvider() { - @Override - protected void doInitialization(EvaluationContext evaluationContext) { - doInitializationCalled.set(true); - throw new FatalError(); - } - }; - assertThrows(FatalError.class, () -> provider.initialize(null)); - assertThat(provider.getState()).isEqualTo(ProviderState.FATAL); - assertThat(doInitializationCalled).isTrue(); - } - - @Test - @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") - @DisplayName("sets the state to ERROR when an error event is emitted") - void setsStateToErrorWhenErrorEventIsEmitted() { - EventProvider provider = new TestEventProvider() { - @Override - protected void doInitialization(EvaluationContext evaluationContext) { - } - }; - assertThat(provider.getState()).isNotEqualTo(ProviderState.ERROR); - provider.emitProviderError(ProviderEventDetails.builder().build()); - assertThat(provider.getState()).isEqualTo(ProviderState.ERROR); - } - - @Test - @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") - @DisplayName("sets the state to STALE when a stale event is emitted") - void setsStateToStaleWhenStaleEventIsEmitted() { - EventProvider provider = new TestEventProvider() { - @Override - protected void doInitialization(EvaluationContext evaluationContext) { - } - }; - assertThat(provider.getState()).isNotEqualTo(ProviderState.STALE); - provider.emitProviderStale(ProviderEventDetails.builder().build()); - assertThat(provider.getState()).isEqualTo(ProviderState.STALE); - } - - @Test - @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") - @DisplayName("sets the state to READY when a ready event is emitted") - void setsStateToReadyWhenReadyEventIsEmitted() { - EventProvider provider = new TestEventProvider() { - @Override - protected void doInitialization(EvaluationContext evaluationContext) { - } - }; - provider.emitProviderStale(ProviderEventDetails.builder().build()); - assertThat(provider.getState()).isNotEqualTo(ProviderState.READY); - provider.emitProviderReady(ProviderEventDetails.builder().build()); - assertThat(provider.getState()).isEqualTo(ProviderState.READY); - } - static class TestEventProvider extends EventProvider { private static final String NAME = "TestEventProvider"; diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 3e6cfb1b3..da50d16b5 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -531,15 +531,16 @@ void matchingReadyEventsMustRunImmediately() { void matchingStaleEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); // provider which is already stale TestEventsProvider provider = TestEventsProvider.newInitializedTestEventsProvider(); - provider.emitProviderStale(null); - assertThat(provider.getState()).isEqualTo(ProviderState.STALE); - OpenFeatureAPI.getInstance().setProvider(name, provider); + api.setProviderAndWait(name, provider); + provider.emitProviderStale(ProviderEventDetails.builder().build()); + assertThat(api.getProviderState(name)).isEqualTo(ProviderState.STALE); // should run even thought handler was added after stale - Client client = OpenFeatureAPI.getInstance().getClient(name); + Client client = api.getClient(name); client.onProviderStale(handler); verify(handler, timeout(TIMEOUT)).accept(any()); } @@ -550,15 +551,17 @@ void matchingStaleEventsMustRunImmediately() { void matchingErrorEventsMustRunImmediately() { final String name = "matchingEventsMustRunImmediately"; final Consumer handler = mockHandler(); + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); // provider which is already in error - TestEventsProvider provider = TestEventsProvider.newInitializedTestEventsProvider(); - provider.emitProviderError(null); - assertThat(provider.getState()).isEqualTo(ProviderState.ERROR); - OpenFeatureAPI.getInstance().setProvider(name, provider); + TestEventsProvider provider = new TestEventsProvider(); + api.setProvider(name, provider); + provider.emitProviderError(ProviderEventDetails.builder().build()); + assertThat(api.getProviderState(name)).isEqualTo(ProviderState.ERROR); + // should run even thought handler was added after error - Client client = OpenFeatureAPI.getInstance().getClient(name); + Client client = api.getClient(name); client.onProviderError(handler); verify(handler, timeout(TIMEOUT)).accept(any()); } diff --git a/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java b/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java new file mode 100644 index 000000000..f68a7f48a --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java @@ -0,0 +1,177 @@ +package dev.openfeature.sdk; + +import dev.openfeature.sdk.exceptions.FatalError; +import dev.openfeature.sdk.exceptions.GeneralError; +import lombok.SneakyThrows; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javax.annotation.Nullable; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class FeatureProviderWrapperTest { + + private FeatureProviderWrapper wrapper; + private TestDelegate testDelegate; + + @BeforeEach + public void setUp() { + testDelegate = new TestDelegate(); + wrapper = new FeatureProviderWrapper(testDelegate); + } + + @Test + void wrapperShouldEqualItsDelegate() { + assertThat(wrapper).isEqualTo(testDelegate); + } + + @Test + void wrapperShouldHaveSameHashCodeAsDelegate() { + assertThat(wrapper.hashCode()).isEqualTo(testDelegate.hashCode()); + } + + @SneakyThrows + @Test + void shouldOnlyCallInitOnce() { + wrapper.initialize(null); + wrapper.initialize(null); + assertThat(testDelegate.initCalled.get()).isOne(); + } + + @SneakyThrows + @Test + void shouldCallInitTwiceWhenShutDownInTheMeantime() { + wrapper.initialize(null); + wrapper.shutdown(); + wrapper.initialize(null); + assertThat(testDelegate.initCalled.get()).isEqualTo(2); + assertThat(testDelegate.shutdownCalled.get()).isOne(); + } + + @Test + void shouldSetStateToNotReadyAfterConstruction() { + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + } + + @SneakyThrows + @Test + @Specification(number = "1.7.3", text = "The client's provider status accessor MUST indicate READY if the initialize function of the associated provider terminates normally.") + void shouldSetStateToReadyAfterInit() { + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + wrapper.initialize(null); + assertThat(wrapper.getState()).isEqualTo(ProviderState.READY); + } + + @SneakyThrows + @Test + void shouldSetStateToNotReadyAfterShutdown() { + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + wrapper.initialize(null); + assertThat(wrapper.getState()).isEqualTo(ProviderState.READY); + wrapper.shutdown(); + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + } + + @Specification(number = "1.7.4", text = "The client's provider status accessor MUST indicate ERROR if the initialize function of the associated provider terminates abnormally.") + @Test + void shouldSetStateToErrorAfterErrorOnInit() { + testDelegate.throwOnInit = new Exception(); + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + assertThrows(Exception.class, () -> wrapper.initialize(null)); + assertThat(wrapper.getState()).isEqualTo(ProviderState.ERROR); + } + + @Specification(number = "1.7.4", text = "The client's provider status accessor MUST indicate ERROR if the initialize function of the associated provider terminates abnormally.") + @Test + void shouldSetStateToErrorAfterOpenFeatureErrorOnInit() { + testDelegate.throwOnInit = new GeneralError(); + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + assertThrows(GeneralError.class, () -> wrapper.initialize(null)); + assertThat(wrapper.getState()).isEqualTo(ProviderState.ERROR); + } + + @Specification(number = "1.7.5", text = "The client's provider status accessor MUST indicate FATAL if the initialize function of the associated provider terminates abnormally and indicates error code PROVIDER_FATAL.") + @Test + void shouldSetStateToErrorAfterFatalErrorOnInit() { + testDelegate.throwOnInit = new FatalError(); + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + assertThrows(FatalError.class, () -> wrapper.initialize(null)); + assertThat(wrapper.getState()).isEqualTo(ProviderState.FATAL); + } + + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @Test + void shouldSetTheStateToReadyWhenAReadyEventIsEmitted() { + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + wrapper.onEmit(ProviderEvent.PROVIDER_READY, null); + assertThat(wrapper.getState()).isEqualTo(ProviderState.READY); + } + + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @Test + void shouldSetTheStateToStaleWhenAStaleEventIsEmitted() { + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + wrapper.onEmit(ProviderEvent.PROVIDER_STALE, null); + assertThat(wrapper.getState()).isEqualTo(ProviderState.STALE); + } + + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @Test + void shouldSetTheStateToErrorWhenAnErrorEventIsEmitted() { + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + wrapper.onEmit(ProviderEvent.PROVIDER_ERROR, null); + assertThat(wrapper.getState()).isEqualTo(ProviderState.ERROR); + } + + static class TestDelegate extends EventProvider { + private final AtomicInteger initCalled = new AtomicInteger(); + private final AtomicInteger shutdownCalled = new AtomicInteger(); + private @Nullable Exception throwOnInit; + + @Override + public Metadata getMetadata() { + return null; + } + + @Override + public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { + return null; + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { + return null; + } + + @Override + public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { + return null; + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { + return null; + } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { + return null; + } + + @Override + public void initialize(EvaluationContext evaluationContext) throws Exception { + initCalled.incrementAndGet(); + if (throwOnInit != null) { + throw throwOnInit; + } + } + + @Override + public void shutdown() { + shutdownCalled.incrementAndGet(); + } + } +} \ No newline at end of file diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index fe2a8e9d1..037b6f539 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -6,7 +6,6 @@ import dev.openfeature.sdk.testutils.FeatureProviderTestUtils; 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; @@ -79,12 +78,12 @@ void getApiInstance() { @Test void providerAndWait() { FeatureProvider provider = new TestEventsProvider(500); OpenFeatureAPI.getInstance().setProviderAndWait(provider); - assertThat(api.getProvider().getState()).isEqualTo(ProviderState.READY); + assertThat(api.getProviderState()).isEqualTo(ProviderState.READY); provider = new TestEventsProvider(500); String providerName = "providerAndWait"; OpenFeatureAPI.getInstance().setProviderAndWait(providerName, provider); - assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.READY); + assertThat(api.getProviderState(providerName)).isEqualTo(ProviderState.READY); } @SneakyThrows @@ -100,15 +99,10 @@ void getApiInstance() { @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 - protected void doInitialization(EvaluationContext evaluationContext) throws Exception { - Awaitility.await().wait(3000); - } - }; + FeatureProvider provider = new InMemoryProvider(new HashMap<>()) ; String providerName = "shouldReturnNotReadyIfNotInitialized"; OpenFeatureAPI.getInstance().setProvider(providerName, provider); - assertThat(api.getProvider(providerName).getState()).isEqualTo(ProviderState.NOT_READY); + assertThat(api.getProviderState(providerName)).isEqualTo(ProviderState.NOT_READY); Client client = OpenFeatureAPI.getInstance().getClient(providerName); assertEquals(ErrorCode.PROVIDER_NOT_READY, client.getBooleanDetails("return_error_when_not_initialized", false).getErrorCode()); } diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index ff522ecf9..289a503ab 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -56,7 +56,8 @@ void shouldRejectNullAsDefaultProvider() { @Test @DisplayName("should have NoOpProvider set as default on initialization") void shouldHaveNoOpProviderSetAsDefaultOnInitialization() { - assertThat(providerRepository.getProvider()).isInstanceOf(NoOpProvider.class); + assertThat(((FeatureProviderWrapper)providerRepository.getProvider()).getDelegate()) + .isInstanceOf(NoOpProvider.class); } @Test @@ -78,16 +79,6 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { verify(featureProvider, timeout(TIMEOUT)).initialize(any()); } - - @SneakyThrows - @Test - @DisplayName("should avoid additional initialization call if provider has been newInitializedTestEventsProvider already") - void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() { - FeatureProvider provider = createMockedReadyProvider(); - setFeatureProvider(provider); - - verify(provider, never()).initialize(any()); - } } @Nested @@ -127,15 +118,6 @@ void shouldImmediatelyReturnWhenCallingTheDomainProviderMutator() throws Excepti return true; }); } - - @Test - @DisplayName("should avoid additional initialization call if provider has been newInitializedTestEventsProvider already") - void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() throws Exception { - FeatureProvider provider = createMockedReadyProvider(); - setFeatureProvider(DOMAIN_NAME, provider); - - verify(provider, never()).initialize(any()); - } } } @@ -325,7 +307,7 @@ private void waitForSettingProviderHasBeenCompleted( .pollDelay(Duration.ofMillis(1)) .atMost(Duration.ofSeconds(5)) .until(() -> { - return extractor.apply(providerRepository) == provider; + return extractor.apply(providerRepository).equals(provider); }); } 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 101f64477..715181b02 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -118,15 +118,4 @@ void emitChangedFlagsOnlyIfThereAreChangedFlags() { await().untilAsserted(() -> verify(handler, times(1)) .accept(argThat(details -> details.getFlagsChanged().size() == buildFlags().size()))); } - - @SneakyThrows - @Test - void shouldThrowGeneralErrorAfterEmittingErrorEvent() { - InMemoryProvider inMemoryProvider = new InMemoryProvider(new HashMap<>()); - inMemoryProvider.initialize(null); - inMemoryProvider.emitProviderError(ProviderEventDetails.builder().build()); - - // ErrorCode.GENERAL should be returned when evaluated via the client - assertThrows(GeneralError.class, () -> inMemoryProvider.getBooleanEvaluation("fail_in_error", false, new ImmutableContext())); - } } \ No newline at end of file diff --git a/src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java b/src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java index 41ffbe18f..12fb71b1b 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java +++ b/src/test/java/dev/openfeature/sdk/testutils/FeatureProviderTestUtils.java @@ -21,7 +21,7 @@ private static void waitForProviderInitializationComplete(Function extractor.apply(OpenFeatureAPI.getInstance()) == provider); + .until(() -> extractor.apply(OpenFeatureAPI.getInstance()).equals(provider)); } public static void setFeatureProvider(String domain, FeatureProvider provider) { diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java index a337ae61a..41e7bff4c 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -43,18 +43,16 @@ public boolean isShutDown() { } @Override - protected void doShutdown() { + public void shutdown() { this.shutDown = true; } @Override - protected void doInitialization(EvaluationContext evaluationContext) throws Exception { - if (ProviderState.NOT_READY.equals(getState())) { - // wait half the TIMEOUT, otherwise some init/errors can be fired before we add handlers - Thread.sleep(initTimeoutMs); - if (this.initError) { - throw new GeneralError(initErrorMessage); - } + public void initialize(EvaluationContext evaluationContext) throws Exception { + // wait half the TIMEOUT, otherwise some init/errors can be fired before we add handlers + Thread.sleep(initTimeoutMs); + if (this.initError) { + throw new GeneralError(initErrorMessage); } } From 56e66a5d5efad42b57ec9c959c03a8734cafa44c Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Thu, 12 Sep 2024 08:46:37 +0200 Subject: [PATCH 08/23] ignore sonar rule Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/FeatureProviderWrapperTest.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java b/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java index f68a7f48a..c71fc5a50 100644 --- a/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java +++ b/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java @@ -23,14 +23,21 @@ public void setUp() { wrapper = new FeatureProviderWrapper(testDelegate); } + @SuppressWarnings("java:S5845") @Test void wrapperShouldEqualItsDelegate() { assertThat(wrapper).isEqualTo(testDelegate); } + @SuppressWarnings("java:S5845") + @Test + void delegateShouldEqualItsWrapper() { + assertThat(testDelegate).isEqualTo(wrapper); + } + @Test void wrapperShouldHaveSameHashCodeAsDelegate() { - assertThat(wrapper.hashCode()).isEqualTo(testDelegate.hashCode()); + assertThat(wrapper).hasSameHashCodeAs(testDelegate); } @SneakyThrows From 724596319a3b3b81adb29a224dfb6db0c0e63518 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 12 Sep 2024 15:56:43 -0400 Subject: [PATCH 09/23] fixup: flakey tests Signed-off-by: Todd Baert --- .../openfeature/sdk/AlwaysBrokenProvider.java | 6 +-- .../java/dev/openfeature/sdk/EventsTest.java | 53 +++++++++---------- .../sdk/FlagEvaluationSpecTest.java | 2 +- .../dev/openfeature/sdk/HookSpecTest.java | 6 +-- 4 files changed, 33 insertions(+), 34 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java b/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java index c10976232..841d738e5 100644 --- a/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java +++ b/src/test/java/dev/openfeature/sdk/AlwaysBrokenProvider.java @@ -4,11 +4,11 @@ public class AlwaysBrokenProvider implements FeatureProvider { + private final String name = "always broken"; + @Override public Metadata getMetadata() { - return () -> { - throw new FlagNotFoundError(TestConstants.BROKEN_MESSAGE); - }; + return () -> name; } @Override diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index da50d16b5..8ccf584ea 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -25,7 +25,7 @@ class EventsTest { @AfterAll public static void resetDefaultProvider() { - OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); + OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); } @Nested @@ -49,7 +49,7 @@ void apiInitReady() { TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().onProviderReady(handler); - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); verify(handler, timeout(TIMEOUT).atLeastOnce()) .accept(any()); } @@ -86,7 +86,7 @@ void apiShouldPropagateEvents() { final String name = "apiShouldPropagateEvents"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler); provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, EventDetails.builder().build()); @@ -109,7 +109,7 @@ void apiShouldSupportAllEventTypes() { final Consumer handler4 = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); OpenFeatureAPI.getInstance().onProviderReady(handler1); OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler2); @@ -149,7 +149,7 @@ void shouldPropagateDefaultAndAnon() { TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client - OpenFeatureAPI.getInstance().setProvider(provider); + OpenFeatureAPI.getInstance().setProviderAndWait(provider); Client client = OpenFeatureAPI.getInstance().getClient(); client.onProviderStale(handler); @@ -166,7 +166,7 @@ void shouldPropagateDefaultAndNamed() { TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client - OpenFeatureAPI.getInstance().setProvider(provider); + OpenFeatureAPI.getInstance().setProviderAndWait(provider); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderStale(handler); @@ -195,7 +195,7 @@ void initReadyProviderBefore() { Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderReady(handler); // set provider after getting a client - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); verify(handler, timeout(TIMEOUT).atLeastOnce()) .accept(argThat(details -> details.getDomain().equals(name))); } @@ -209,7 +209,7 @@ void initReadyProviderAfter() { TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderReady(handler); verify(handler, timeout(TIMEOUT).atLeastOnce()) @@ -268,7 +268,7 @@ void shouldPropagateBefore() { TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderConfigurationChanged(handler); @@ -288,7 +288,7 @@ void shouldPropagateAfter() { Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderConfigurationChanged(handler); // set provider after getting a client - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, EventDetails.builder().build()); verify(handler, timeout(TIMEOUT)).accept(argThat(details -> details.getDomain().equals(name))); @@ -310,7 +310,7 @@ void shouldSupportAllEventTypes() { final Consumer handler4 = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderReady(handler1); @@ -340,14 +340,14 @@ void shouldNotRunHandlers() { TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); TestEventsProvider provider2 = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(name, provider1); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider1); Client client = OpenFeatureAPI.getInstance().getClient(name); // attached handlers OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler1); client.onProviderConfigurationChanged(handler2); - OpenFeatureAPI.getInstance().setProvider(name, provider2); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider2); // wait for the new provider to be ready and make sure things are cleaned up. await().until(() -> provider1.isShutDown()); @@ -373,8 +373,8 @@ void otherClientHandlersShouldNotRun() { TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); TestEventsProvider provider2 = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(name1, provider1); - OpenFeatureAPI.getInstance().setProvider(name2, provider2); + OpenFeatureAPI.getInstance().setProviderAndWait(name1, provider1); + OpenFeatureAPI.getInstance().setProviderAndWait(name2, provider2); Client client1 = OpenFeatureAPI.getInstance().getClient(name1); Client client2 = OpenFeatureAPI.getInstance().getClient(name2); @@ -398,11 +398,11 @@ void boundShouldNotRunWithDefault() { TestEventsProvider namedProvider = new TestEventsProvider(INIT_DELAY); TestEventsProvider defaultProvider = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(defaultProvider); + OpenFeatureAPI.getInstance().setProviderAndWait(defaultProvider); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderConfigurationChanged(handlerNotToRun); - OpenFeatureAPI.getInstance().setProvider(name, namedProvider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, namedProvider); // await the new provider to make sure the old one is shut down await().until(() -> namedProvider.getState().equals(ProviderState.READY)); @@ -411,7 +411,7 @@ void boundShouldNotRunWithDefault() { defaultProvider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); verify(handlerNotToRun, after(TIMEOUT).never()).accept(any()); - OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); + OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); } @Test @@ -423,7 +423,7 @@ void unboundShouldRunWithDefault() { final Consumer handlerToRun = mockHandler(); TestEventsProvider defaultProvider = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(defaultProvider); + OpenFeatureAPI.getInstance().setProviderAndWait(defaultProvider); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderConfigurationChanged(handlerToRun); @@ -435,7 +435,7 @@ void unboundShouldRunWithDefault() { defaultProvider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); verify(handlerToRun, timeout(TIMEOUT)).accept(any()); - OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); + OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); } @Test @@ -449,7 +449,7 @@ void handlersRunIfOneThrows() { final Consumer lastHandler = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); Client client1 = OpenFeatureAPI.getInstance().getClient(name); @@ -473,7 +473,7 @@ void shouldHaveAllProperties() { final String name = "shouldHaveAllProperties"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); // attached handlers @@ -555,7 +555,7 @@ void matchingErrorEventsMustRunImmediately() { // provider which is already in error TestEventsProvider provider = new TestEventsProvider(); - api.setProvider(name, provider); + api.setProviderAndWait(name, provider); provider.emitProviderError(ProviderEventDetails.builder().build()); assertThat(api.getProviderState(name)).isEqualTo(ProviderState.ERROR); @@ -576,7 +576,7 @@ void mustPersistAcrossChanges() { TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); TestEventsProvider provider2 = new TestEventsProvider(INIT_DELAY); - OpenFeatureAPI.getInstance().setProvider(name, provider1); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider1); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderConfigurationChanged(handler); @@ -586,8 +586,7 @@ void mustPersistAcrossChanges() { verify(handler, timeout(TIMEOUT).times(1)).accept(argThat(nameMatches)); // wait for the new provider to be ready. - OpenFeatureAPI.getInstance().setProvider(name, provider2); - await().until(() -> provider2.getState().equals(ProviderState.READY)); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider2); // verify that with the new provider under the same name, the handler is called // again. @@ -608,7 +607,7 @@ void removedEventsShouldNotRun() { TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); provider.initialize(null); - OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); // attached handlers diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 037b6f539..972104e3c 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -99,7 +99,7 @@ void getApiInstance() { @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<>()) ; + FeatureProvider provider = new TestEventsProvider(100); String providerName = "shouldReturnNotReadyIfNotInitialized"; OpenFeatureAPI.getInstance().setProvider(providerName, provider); assertThat(api.getProviderState(providerName)).isEqualTo(ProviderState.NOT_READY); diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index 2b29acedb..e2c2825d6 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -337,15 +337,15 @@ void error_stops_before() { Hook h2 = mockBooleanHook(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new AlwaysBrokenProvider()); + api.setProviderAndWait(new AlwaysBrokenProvider()); Client c = api.getClient(); c.getBooleanDetails("key", false, null, FlagEvaluationOptions.builder() .hook(h2) .hook(h) .build()); - verify(h, times(1)).before(any(), any()); - verify(h2, times(0)).before(any(), any()); + verify(h, times(1)).before(any(), any()); + verify(h2, times(0)).before(any(), any()); } @Specification(number = "4.4.6", text = "If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") From b90f393c38b0c06d3e3df70bc8006cc2491870cc Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Fri, 13 Sep 2024 08:14:03 +0200 Subject: [PATCH 10/23] use lombok delegate, add tests and fix codestyle Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/EventProvider.java | 5 +- .../dev/openfeature/sdk/FeatureProvider.java | 7 +-- .../sdk/FeatureProviderWrapper.java | 55 ++++++------------- .../dev/openfeature/sdk/OpenFeatureAPI.java | 14 ++--- .../openfeature/sdk/ProviderRepository.java | 16 +++++- .../sdk/DeveloperExperienceTest.java | 51 +++++++++++++++++ 6 files changed, 92 insertions(+), 56 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index fbfa8d751..7dfd2ff45 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -2,7 +2,6 @@ import dev.openfeature.sdk.internal.TriConsumer; -import javax.annotation.Nullable; /** * Abstract EventProvider. Providers must extend this class to support events. @@ -17,9 +16,9 @@ * @see FeatureProvider */ public abstract class EventProvider implements FeatureProvider { - private @Nullable EventProviderListener eventProviderListener; + private EventProviderListener eventProviderListener; - void setEventProviderListener(@Nullable EventProviderListener eventProviderListener) { + void setEventProviderListener(EventProviderListener eventProviderListener) { this.eventProviderListener = eventProviderListener; } diff --git a/src/main/java/dev/openfeature/sdk/FeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProvider.java index 7319d6f09..f73b6cdfa 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProvider.java @@ -56,16 +56,15 @@ default void shutdown() { } /** - * @deprecated State is handled by the SDK internally. - *

* Returns a representation of the current readiness of the provider. * If the provider needs to be initialized, it should return {@link ProviderState#NOT_READY}. * If the provider is in an error state, it should return {@link ProviderState#ERROR}. * If the provider is functioning normally, it should return {@link ProviderState#READY}. - * + * *

Providers which do not implement this method are assumed to be ready immediately.

- * + * * @return ProviderState + * @deprecated The state is handled by the SDK internally. Query the state from the {@link Client} instead. */ @Deprecated default ProviderState getState() { diff --git a/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java b/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java index 36b37c71d..a36521693 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java @@ -1,11 +1,21 @@ package dev.openfeature.sdk; import dev.openfeature.sdk.exceptions.OpenFeatureError; +import lombok.experimental.Delegate; -import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; -public class FeatureProviderWrapper implements FeatureProvider, EventProviderListener { +class FeatureProviderWrapper implements FeatureProvider, EventProviderListener { + + private interface ExcludeFromDelegate { + void initialize(EvaluationContext evaluationContext) throws Exception; + + void shutdown(); + + ProviderState getState(); + } + + @Delegate(excludes = ExcludeFromDelegate.class) private final FeatureProvider delegate; private final AtomicBoolean isInitialized = new AtomicBoolean(); private ProviderState state = ProviderState.NOT_READY; @@ -17,41 +27,6 @@ public FeatureProviderWrapper(FeatureProvider delegate) { } } - @Override - public Metadata getMetadata() { - return delegate.getMetadata(); - } - - @Override - public List getProviderHooks() { - return delegate.getProviderHooks(); - } - - @Override - public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { - return delegate.getBooleanEvaluation(key, defaultValue, ctx); - } - - @Override - public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { - return delegate.getStringEvaluation(key, defaultValue, ctx); - } - - @Override - public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { - return delegate.getIntegerEvaluation(key, defaultValue, ctx); - } - - @Override - public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { - return delegate.getDoubleEvaluation(key, defaultValue, ctx); - } - - @Override - public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { - return delegate.getObjectEvaluation(key, defaultValue, ctx); - } - @Override public void initialize(EvaluationContext evaluationContext) throws Exception { if (isInitialized.getAndSet(true)) { @@ -94,7 +69,9 @@ public int hashCode() { @Override public boolean equals(Object obj) { - if (this == obj) return true; + if (this == obj) { + return true; + } if (obj instanceof FeatureProviderWrapper) { return delegate.equals(((FeatureProviderWrapper) obj).delegate); } @@ -112,7 +89,7 @@ public void onEmit(ProviderEvent event, ProviderEventDetails details) { } } - FeatureProvider getDelegate(){ + FeatureProvider getDelegate() { return delegate; } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index b354607af..726a833f6 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -275,13 +275,6 @@ public FeatureProvider getProvider() { return providerRepository.getProvider(); } - /** - * Return the state of the default provider. - */ - public ProviderState getProviderState() { - return providerRepository.getProviderState(); - } - /** * Fetch a provider for a domain. If not found, return the default. * @@ -292,6 +285,13 @@ public FeatureProvider getProvider(String domain) { return providerRepository.getProvider(domain); } + /** + * Return the state of the default provider. + */ + public ProviderState getProviderState() { + return providerRepository.getProviderState(); + } + /** * Get the state of the provider for a domain. If no provider with the domain is found, returns the state of the * default provider. diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index cf2563add..cd8343c5e 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -21,7 +21,9 @@ class ProviderRepository { private final Map providers = new ConcurrentHashMap<>(); - private final AtomicReference defaultProvider = new AtomicReference<>(new FeatureProviderWrapper(new NoOpProvider())); + private final AtomicReference defaultProvider = new AtomicReference<>( + new FeatureProviderWrapper(new NoOpProvider()) + ); private final ExecutorService taskExecutor = Executors.newCachedThreadPool(runnable -> { final Thread thread = new Thread(runnable); thread.setDaemon(true); @@ -148,10 +150,18 @@ private void initializeProvider(FeatureProviderWrapper newProvider, } shutDownOld(oldProvider, afterShutdown); } catch (OpenFeatureError e) { - log.error("Exception when initializing feature provider {}", newProvider.getDelegate().getClass().getName(), e); + log.error( + "Exception when initializing feature provider {}", + newProvider.getDelegate().getClass().getName(), + e + ); afterError.accept(newProvider.getDelegate(), e); } catch (Exception e) { - log.error("Exception when initializing feature provider {}", newProvider.getDelegate().getClass().getName(), e); + log.error( + "Exception when initializing feature provider {}", + newProvider.getDelegate().getClass().getName(), + e + ); afterError.accept(newProvider.getDelegate(), new GeneralError(e)); } } diff --git a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java index dc4c735e8..4502699b1 100644 --- a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java +++ b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java @@ -8,6 +8,7 @@ import java.util.*; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.mockito.ArgumentMatchers.any; @@ -121,4 +122,54 @@ public Optional before(HookContext ctx, Map hints) { String noOpValue = client.getStringValue("val", defaultValue); assertEquals(noOpValue, defaultValue); } + + @Test + void setProviderAndWaitShouldPutTheProviderInReadyState() { + String domain = "domain"; + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProviderAndWait(domain, new TestEventsProvider()); + Client client = api.getClient(domain); + assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); + } + + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @Test + void shouldPutTheProviderInStateErrorAfterEmittingErrorEvent() { + String domain = "domain"; + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + TestEventsProvider provider = new TestEventsProvider(); + api.setProviderAndWait(domain, provider); + Client client = api.getClient(domain); + assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); + provider.emitProviderError(ProviderEventDetails.builder().build()); + assertThat(client.getProviderState()).isEqualTo(ProviderState.ERROR); + } + + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @Test + void shouldPutTheProviderInStateStaleAfterEmittingStaleEvent() { + String domain = "domain"; + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + TestEventsProvider provider = new TestEventsProvider(); + api.setProviderAndWait(domain, provider); + Client client = api.getClient(domain); + assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); + provider.emitProviderStale(ProviderEventDetails.builder().build()); + assertThat(client.getProviderState()).isEqualTo(ProviderState.STALE); + } + + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @Test + void shouldPutTheProviderInStateReadyAfterEmittingReadyEvent() { + String domain = "domain"; + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + TestEventsProvider provider = new TestEventsProvider(); + api.setProviderAndWait(domain, provider); + Client client = api.getClient(domain); + assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); + provider.emitProviderStale(ProviderEventDetails.builder().build()); + assertThat(client.getProviderState()).isEqualTo(ProviderState.STALE); + provider.emitProviderReady(ProviderEventDetails.builder().build()); + assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); + } } From 7d89d52d0a800d4590b9bd397a801739780bf689 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Fri, 13 Sep 2024 12:47:18 +0200 Subject: [PATCH 11/23] rename wrapper class, add public facing methods Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/EventProvider.java | 4 +-- .../dev/openfeature/sdk/NoOpProvider.java | 4 +-- .../dev/openfeature/sdk/OpenFeatureAPI.java | 4 +++ .../openfeature/sdk/ProviderRepository.java | 36 ++++++++++++++----- ...pper.java => StatefulFeatureProvider.java} | 8 ++--- .../sdk/ProviderRepositoryTest.java | 3 +- ....java => StatefulFeatureProviderTest.java} | 6 ++-- 7 files changed, 43 insertions(+), 22 deletions(-) rename src/main/java/dev/openfeature/sdk/{FeatureProviderWrapper.java => StatefulFeatureProvider.java} (90%) rename src/test/java/dev/openfeature/sdk/{FeatureProviderWrapperTest.java => StatefulFeatureProviderTest.java} (98%) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 7dfd2ff45..7e980f48a 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -108,8 +108,8 @@ public boolean equals(Object obj) { if (obj == null) { return false; } - if (obj instanceof FeatureProviderWrapper) { - return this == ((FeatureProviderWrapper) obj).getDelegate(); + if (obj instanceof StatefulFeatureProvider) { + return this == ((StatefulFeatureProvider) obj).getDelegate(); } return this == obj; } diff --git a/src/main/java/dev/openfeature/sdk/NoOpProvider.java b/src/main/java/dev/openfeature/sdk/NoOpProvider.java index a8f6e7ac2..dd5806f30 100644 --- a/src/main/java/dev/openfeature/sdk/NoOpProvider.java +++ b/src/main/java/dev/openfeature/sdk/NoOpProvider.java @@ -72,8 +72,8 @@ public boolean equals(Object obj) { if (obj == null) { return false; } - if (obj instanceof FeatureProviderWrapper) { - return ((FeatureProviderWrapper) obj).getDelegate() == this; + if (obj instanceof StatefulFeatureProvider) { + return ((StatefulFeatureProvider) obj).getDelegate() == this; } return obj == this; } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 726a833f6..0e54a614f 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -303,6 +303,10 @@ public ProviderState getProviderState(String domain) { return providerRepository.getProviderState(domain); } + public ProviderState getProviderState(FeatureProvider provider) { + return providerRepository.getProviderState(provider); + } + /** * Adds hooks for globally, used for all evaluations. * Hooks are run in the order they're added in the before stage. They are run in reverse order for all other stages. diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index cd8343c5e..b83dd56bb 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -20,9 +20,9 @@ @Slf4j class ProviderRepository { - private final Map providers = new ConcurrentHashMap<>(); - private final AtomicReference defaultProvider = new AtomicReference<>( - new FeatureProviderWrapper(new NoOpProvider()) + private final Map providers = new ConcurrentHashMap<>(); + private final AtomicReference defaultProvider = new AtomicReference<>( + new StatefulFeatureProvider(new NoOpProvider()) ); private final ExecutorService taskExecutor = Executors.newCachedThreadPool(runnable -> { final Thread thread = new Thread(runnable); @@ -51,6 +51,24 @@ public ProviderState getProviderState() { return defaultProvider.get().getState(); } + public ProviderState getProviderState(FeatureProvider featureProvider) { + if (featureProvider instanceof StatefulFeatureProvider) { + return ((StatefulFeatureProvider) featureProvider).getState(); + } + + StatefulFeatureProvider defaultProvider = this.defaultProvider.get(); + if (defaultProvider.equals(featureProvider)) { + return defaultProvider.getState(); + } + + for (StatefulFeatureProvider wrapper : providers.values()) { + if (wrapper.equals(featureProvider)) { + return wrapper.getState(); + } + } + return null; + } + public ProviderState getProviderState(String domain) { return Optional.ofNullable(domain).map(this.providers::get).orElse(this.defaultProvider.get()).getState(); } @@ -116,7 +134,7 @@ private void prepareAndInitializeProvider(String domain, BiConsumer afterError, boolean waitForInit) { - FeatureProviderWrapper newProviderWrapper = new FeatureProviderWrapper(newProvider); + StatefulFeatureProvider newProviderWrapper = new StatefulFeatureProvider(newProvider); if (!isProviderRegistered(newProvider)) { // only run afterSet if new provider is not already attached @@ -124,7 +142,7 @@ private void prepareAndInitializeProvider(String domain, } // provider is set immediately, on this thread - FeatureProviderWrapper oldProvider = domain != null + StatefulFeatureProvider oldProvider = domain != null ? this.providers.put(domain, newProviderWrapper) : this.defaultProvider.getAndSet(newProviderWrapper); @@ -138,11 +156,11 @@ private void prepareAndInitializeProvider(String domain, } } - private void initializeProvider(FeatureProviderWrapper newProvider, + private void initializeProvider(StatefulFeatureProvider newProvider, Consumer afterInit, Consumer afterShutdown, BiConsumer afterError, - FeatureProviderWrapper oldProvider) { + StatefulFeatureProvider oldProvider) { try { if (ProviderState.NOT_READY.equals(newProvider.getState())) { newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); @@ -166,7 +184,7 @@ private void initializeProvider(FeatureProviderWrapper newProvider, } } - private void shutDownOld(FeatureProviderWrapper oldProvider, Consumer afterShutdown) { + private void shutDownOld(StatefulFeatureProvider oldProvider, Consumer afterShutdown) { if (oldProvider != null && !isProviderRegistered(oldProvider)) { shutdownProvider(oldProvider); afterShutdown.accept(oldProvider.getDelegate()); @@ -184,7 +202,7 @@ private boolean isProviderRegistered(FeatureProvider provider) { && (this.providers.containsValue(provider) || this.defaultProvider.get().equals(provider)); } - private void shutdownProvider(FeatureProviderWrapper provider) { + private void shutdownProvider(StatefulFeatureProvider provider) { if (provider == null) { return; } diff --git a/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java b/src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java similarity index 90% rename from src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java rename to src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java index a36521693..922fe0614 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProviderWrapper.java +++ b/src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java @@ -5,7 +5,7 @@ import java.util.concurrent.atomic.AtomicBoolean; -class FeatureProviderWrapper implements FeatureProvider, EventProviderListener { +class StatefulFeatureProvider implements FeatureProvider, EventProviderListener { private interface ExcludeFromDelegate { void initialize(EvaluationContext evaluationContext) throws Exception; @@ -20,7 +20,7 @@ private interface ExcludeFromDelegate { private final AtomicBoolean isInitialized = new AtomicBoolean(); private ProviderState state = ProviderState.NOT_READY; - public FeatureProviderWrapper(FeatureProvider delegate) { + public StatefulFeatureProvider(FeatureProvider delegate) { this.delegate = delegate; if (delegate instanceof EventProvider) { ((EventProvider) delegate).setEventProviderListener(this); @@ -72,8 +72,8 @@ public boolean equals(Object obj) { if (this == obj) { return true; } - if (obj instanceof FeatureProviderWrapper) { - return delegate.equals(((FeatureProviderWrapper) obj).delegate); + if (obj instanceof StatefulFeatureProvider) { + return delegate.equals(((StatefulFeatureProvider) obj).delegate); } return delegate.equals(obj); } diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index 289a503ab..5f6cc8242 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -2,7 +2,6 @@ import dev.openfeature.sdk.exceptions.OpenFeatureError; import dev.openfeature.sdk.testutils.exception.TestException; -import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -56,7 +55,7 @@ void shouldRejectNullAsDefaultProvider() { @Test @DisplayName("should have NoOpProvider set as default on initialization") void shouldHaveNoOpProviderSetAsDefaultOnInitialization() { - assertThat(((FeatureProviderWrapper)providerRepository.getProvider()).getDelegate()) + assertThat(((StatefulFeatureProvider)providerRepository.getProvider()).getDelegate()) .isInstanceOf(NoOpProvider.class); } diff --git a/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java b/src/test/java/dev/openfeature/sdk/StatefulFeatureProviderTest.java similarity index 98% rename from src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java rename to src/test/java/dev/openfeature/sdk/StatefulFeatureProviderTest.java index c71fc5a50..3dc812cb3 100644 --- a/src/test/java/dev/openfeature/sdk/FeatureProviderWrapperTest.java +++ b/src/test/java/dev/openfeature/sdk/StatefulFeatureProviderTest.java @@ -12,15 +12,15 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; -class FeatureProviderWrapperTest { +class StatefulFeatureProviderTest { - private FeatureProviderWrapper wrapper; + private StatefulFeatureProvider wrapper; private TestDelegate testDelegate; @BeforeEach public void setUp() { testDelegate = new TestDelegate(); - wrapper = new FeatureProviderWrapper(testDelegate); + wrapper = new StatefulFeatureProvider(testDelegate); } @SuppressWarnings("java:S5845") From e3c1d3027d7d96145cafc40899dd74119f05bfe1 Mon Sep 17 00:00:00 2001 From: chrfwow Date: Mon, 16 Sep 2024 07:49:04 +0200 Subject: [PATCH 12/23] reduce visibility of emit methods, add error code to ProviderEventDetails Signed-off-by: christian.lutnik --- .../java/dev/openfeature/sdk/EventProvider.java | 10 +++++----- .../openfeature/sdk/ProviderEventDetails.java | 1 + .../openfeature/sdk/StatefulFeatureProvider.java | 6 +++++- .../sdk/StatefulFeatureProviderTest.java | 16 +++++++++++++++- .../providers/memory/InMemoryProviderTest.java | 6 ------ 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 7e980f48a..0cd8e81ac 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -53,7 +53,7 @@ void detach() { * @param event The event type * @param details The details of the event */ - public void emit(ProviderEvent event, ProviderEventDetails details) { + protected void emit(ProviderEvent event, ProviderEventDetails details) { if (eventProviderListener != null) { eventProviderListener.onEmit(event, details); } @@ -68,7 +68,7 @@ public void emit(ProviderEvent event, ProviderEventDetails details) { * * @param details The details of the event */ - public void emitProviderReady(ProviderEventDetails details) { + protected void emitProviderReady(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_READY, details); } @@ -79,7 +79,7 @@ public void emitProviderReady(ProviderEventDetails details) { * * @param details The details of the event */ - public void emitProviderConfigurationChanged(ProviderEventDetails details) { + protected void emitProviderConfigurationChanged(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); } @@ -89,7 +89,7 @@ public void emitProviderConfigurationChanged(ProviderEventDetails details) { * * @param details The details of the event */ - public void emitProviderStale(ProviderEventDetails details) { + protected void emitProviderStale(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_STALE, details); } @@ -99,7 +99,7 @@ public void emitProviderStale(ProviderEventDetails details) { * * @param details The details of the event */ - public void emitProviderError(ProviderEventDetails details) { + protected void emitProviderError(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_ERROR, details); } diff --git a/src/main/java/dev/openfeature/sdk/ProviderEventDetails.java b/src/main/java/dev/openfeature/sdk/ProviderEventDetails.java index d28da9e50..d927e4291 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderEventDetails.java +++ b/src/main/java/dev/openfeature/sdk/ProviderEventDetails.java @@ -13,4 +13,5 @@ public class ProviderEventDetails { private List flagsChanged; private String message; private ImmutableMetadata eventMetadata; + private ErrorCode errorCode; } diff --git a/src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java b/src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java index 922fe0614..214ba408c 100644 --- a/src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java @@ -81,7 +81,11 @@ public boolean equals(Object obj) { @Override public void onEmit(ProviderEvent event, ProviderEventDetails details) { if (ProviderEvent.PROVIDER_ERROR.equals(event)) { - state = ProviderState.ERROR; + if (details != null && details.getErrorCode() == ErrorCode.PROVIDER_FATAL) { + state = ProviderState.FATAL; + } else { + state = ProviderState.ERROR; + } } else if (ProviderEvent.PROVIDER_STALE.equals(event)) { state = ProviderState.STALE; } else if (ProviderEvent.PROVIDER_READY.equals(event)) { diff --git a/src/test/java/dev/openfeature/sdk/StatefulFeatureProviderTest.java b/src/test/java/dev/openfeature/sdk/StatefulFeatureProviderTest.java index 3dc812cb3..4df564ef8 100644 --- a/src/test/java/dev/openfeature/sdk/StatefulFeatureProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/StatefulFeatureProviderTest.java @@ -129,10 +129,24 @@ void shouldSetTheStateToStaleWhenAStaleEventIsEmitted() { @Test void shouldSetTheStateToErrorWhenAnErrorEventIsEmitted() { assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); - wrapper.onEmit(ProviderEvent.PROVIDER_ERROR, null); + wrapper.onEmit( + ProviderEvent.PROVIDER_ERROR, + ProviderEventDetails.builder().errorCode(ErrorCode.GENERAL).build() + ); assertThat(wrapper.getState()).isEqualTo(ProviderState.ERROR); } + @Specification(number = "5.3.5", text = "If the provider emits an event, the value of the client's provider status MUST be updated accordingly.") + @Test + void shouldSetTheStateToFatalWhenAFatalErrorEventIsEmitted() { + assertThat(wrapper.getState()).isEqualTo(ProviderState.NOT_READY); + wrapper.onEmit( + ProviderEvent.PROVIDER_ERROR, + ProviderEventDetails.builder().errorCode(ErrorCode.PROVIDER_FATAL).build() + ); + assertThat(wrapper.getState()).isEqualTo(ProviderState.FATAL); + } + static class TestDelegate extends EventProvider { private final AtomicInteger initCalled = new AtomicInteger(); private final AtomicInteger shutdownCalled = new AtomicInteger(); 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 715181b02..55ddc07cd 100644 --- a/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/providers/memory/InMemoryProviderTest.java @@ -45,12 +45,6 @@ void beforeEach() { .build()); } - @SneakyThrows - @Test - void eventsTest() { - verify(provider, times(2)).emitProviderConfigurationChanged(any()); - } - @Test void getBooleanEvaluation() { assertTrue(client.getBooleanValue("boolean-flag", false)); From 4bc6f664d2efc3db90aa2184f33a66f31b596b74 Mon Sep 17 00:00:00 2001 From: chrfwow Date: Mon, 16 Sep 2024 13:06:31 +0200 Subject: [PATCH 13/23] return provider delegate Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/OpenFeatureAPI.java | 14 +++- .../openfeature/sdk/OpenFeatureClient.java | 7 +- .../dev/openfeature/sdk/ProviderAccessor.java | 2 +- .../openfeature/sdk/ProviderRepository.java | 10 ++- .../openfeature/sdk/OpenFeatureAPITest.java | 12 +++- .../sdk/OpenFeatureClientTest.java | 72 +++++++++++++++++-- .../sdk/ProviderRepositoryTest.java | 3 +- 7 files changed, 103 insertions(+), 17 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 0e54a614f..394343bfc 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -103,7 +103,17 @@ public Client getClient(String domain) { */ public Client getClient(String domain, String version) { return new OpenFeatureClient( - () -> providerRepository.getProvider(domain), + new ProviderAccessor() { + @Override + public FeatureProvider getProvider() { + return providerRepository.getProvider(domain); + } + + @Override + public ProviderState getProviderState() { + return providerRepository.getProviderState(domain); + } + }, this, domain, version @@ -416,7 +426,7 @@ void removeHandler(String domain, ProviderEvent event, Consumer ha void addHandler(String domain, ProviderEvent event, Consumer handler) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { // if the provider is in the state associated with event, run immediately - if (Optional.ofNullable(this.providerRepository.getProvider(domain).getState()) + if (Optional.ofNullable(this.providerRepository.getProviderState(domain)) .orElse(ProviderState.READY).matchesEvent(event)) { eventSupport.runHandler(handler, EventDetails.builder().domain(domain).build()); } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 6e2c7e7b7..ee57af106 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -63,7 +63,7 @@ public OpenFeatureClient( @Override public ProviderState getProviderState() { - return providerAccessor.getProvider().getState(); + return providerAccessor.getProviderState(); } /** @@ -123,10 +123,11 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key try { // openfeatureApi.getProvider() must be called once to maintain a consistent reference provider = providerAccessor.getProvider(); - if (ProviderState.NOT_READY.equals(provider.getState())) { + ProviderState state = providerAccessor.getProviderState(); + if (ProviderState.NOT_READY.equals(state)) { throw new ProviderNotReadyError("provider not yet initialized"); } - if (ProviderState.FATAL.equals(provider.getState())) { + if (ProviderState.FATAL.equals(state)) { throw new FatalError("provider is in an irrecoverable error state"); } diff --git a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java index cf1fbec67..efec96284 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java +++ b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java @@ -3,7 +3,7 @@ /** * Provides access to the future provider for the domain of the client. */ -@FunctionalInterface public interface ProviderAccessor { FeatureProvider getProvider(); + ProviderState getProviderState(); } diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index b83dd56bb..327322a52 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -34,7 +34,7 @@ class ProviderRepository { * Return the default provider. */ public FeatureProvider getProvider() { - return defaultProvider.get(); + return defaultProvider.get().getDelegate(); } /** @@ -44,7 +44,13 @@ public FeatureProvider getProvider() { * @return A named {@link FeatureProvider} */ public FeatureProvider getProvider(String domain) { - return Optional.ofNullable(domain).map(this.providers::get).orElse(this.defaultProvider.get()); + if (domain == null) return defaultProvider.get().getDelegate(); + StatefulFeatureProvider fromMap = this.providers.get(domain); + if (fromMap == null) { + return this.defaultProvider.get().getDelegate(); + } else { + return fromMap.getDelegate(); + } } public ProviderState getProviderState() { diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java index 83b3ba195..796d43346 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java @@ -82,7 +82,17 @@ void settingTransactionalContextPropagatorToNullErrors() { @Test void setEvaluationContextShouldAllowChaining() { - OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { + @Override + public FeatureProvider getProvider() { + return null; + } + + @Override + public ProviderState getProviderState() { + return ProviderState.READY; + } + }, api, "name", "version"); EvaluationContext ctx = new ImmutableContext("targeting key", new HashMap<>()); OpenFeatureClient result = client.setEvaluationContext(ctx); assertEquals(client, result); diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 8d0e0234b..6ea4c7cf9 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -41,7 +41,17 @@ void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() { when(api.getProvider(any())).thenReturn(provider); when(api.getHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook())); - OpenFeatureClient client = new OpenFeatureClient(() -> provider, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { + @Override + public FeatureProvider getProvider() { + return provider; + } + + @Override + public ProviderState getProviderState() { + return ProviderState.READY; + } + }, api, "name", "version"); FlagEvaluationDetails actual = client.getBooleanDetails("feature key", Boolean.FALSE); @@ -71,7 +81,17 @@ void mergeContextTest() { when(api.getProvider(any())).thenReturn(mockProvider); - OpenFeatureClient client = new OpenFeatureClient(() -> mockProvider, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { + @Override + public FeatureProvider getProvider() { + return mockProvider; + } + + @Override + public ProviderState getProviderState() { + return ProviderState.READY; + } + }, api, "name", "version"); client.setEvaluationContext(ctx); FlagEvaluationDetails result = client.getBooleanDetails(flag, defaultValue); @@ -83,7 +103,17 @@ void mergeContextTest() { @DisplayName("addHooks should allow chaining by returning the same client instance") void addHooksShouldAllowChaining() { OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { + @Override + public FeatureProvider getProvider() { + return null; + } + + @Override + public ProviderState getProviderState() { + return ProviderState.READY; + } + }, api, "name", "version"); Hook hook1 = Mockito.mock(Hook.class); Hook hook2 = Mockito.mock(Hook.class); @@ -95,7 +125,17 @@ void addHooksShouldAllowChaining() { @DisplayName("setEvaluationContext should allow chaining by returning the same client instance") void setEvaluationContextShouldAllowChaining() { OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { + @Override + public FeatureProvider getProvider() { + return null; + } + + @Override + public ProviderState getProviderState() { + return ProviderState.READY; + } + }, api, "name", "version"); EvaluationContext ctx = new ImmutableContext("targeting key", new HashMap<>()); OpenFeatureClient result = client.setEvaluationContext(ctx); @@ -107,7 +147,17 @@ void setEvaluationContextShouldAllowChaining() { void shouldNotCallEvaluationMethodsWhenProviderIsInFatalErrorState() { MockProvider mockProvider = new MockProvider(ProviderState.FATAL); OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(() -> mockProvider, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { + @Override + public FeatureProvider getProvider() { + return mockProvider; + } + + @Override + public ProviderState getProviderState() { + return ProviderState.FATAL; + } + }, api, "name", "version"); FlagEvaluationDetails details = client.getBooleanDetails("key", true); assertThat(mockProvider.isEvaluationCalled()).isFalse(); @@ -119,7 +169,17 @@ void shouldNotCallEvaluationMethodsWhenProviderIsInFatalErrorState() { void shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState() { MockProvider mockProvider = new MockProvider(ProviderState.NOT_READY); OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(() -> mockProvider, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { + @Override + public FeatureProvider getProvider() { + return mockProvider; + } + + @Override + public ProviderState getProviderState() { + return ProviderState.NOT_READY; + } + }, api, "name", "version"); FlagEvaluationDetails details = client.getBooleanDetails("key", true); assertThat(mockProvider.isEvaluationCalled()).isFalse(); diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index 5f6cc8242..26a04d533 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -55,8 +55,7 @@ void shouldRejectNullAsDefaultProvider() { @Test @DisplayName("should have NoOpProvider set as default on initialization") void shouldHaveNoOpProviderSetAsDefaultOnInitialization() { - assertThat(((StatefulFeatureProvider)providerRepository.getProvider()).getDelegate()) - .isInstanceOf(NoOpProvider.class); + assertThat(providerRepository.getProvider()).isInstanceOf(NoOpProvider.class); } @Test From 8bd726d56884ca83b12c3478a55b195807622ad9 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 16 Sep 2024 16:08:48 +0200 Subject: [PATCH 14/23] fix checkstyle errors Signed-off-by: christian.lutnik --- src/main/java/dev/openfeature/sdk/ProviderAccessor.java | 1 + src/main/java/dev/openfeature/sdk/ProviderRepository.java | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java index efec96284..15f62bdfb 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java +++ b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java @@ -5,5 +5,6 @@ */ public interface ProviderAccessor { FeatureProvider getProvider(); + ProviderState getProviderState(); } diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 327322a52..02813541b 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -44,7 +44,9 @@ public FeatureProvider getProvider() { * @return A named {@link FeatureProvider} */ public FeatureProvider getProvider(String domain) { - if (domain == null) return defaultProvider.get().getDelegate(); + if (domain == null) { + return defaultProvider.get().getDelegate(); + } StatefulFeatureProvider fromMap = this.providers.get(domain); if (fromMap == null) { return this.defaultProvider.get().getDelegate(); From 37326d98fa667d906296b40814d31d4e53b52ff7 Mon Sep 17 00:00:00 2001 From: chrfwow Date: Wed, 18 Sep 2024 09:07:50 +0200 Subject: [PATCH 15/23] make FeatureProviderStateManager a true wrapper, without implementing FeatureProvider Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/EventProvider.java | 4 +- ....java => FeatureProviderStateManager.java} | 35 ++--- .../dev/openfeature/sdk/NoOpProvider.java | 11 -- .../dev/openfeature/sdk/OpenFeatureAPI.java | 18 +-- .../openfeature/sdk/OpenFeatureClient.java | 7 +- .../dev/openfeature/sdk/ProviderAccessor.java | 7 +- .../openfeature/sdk/ProviderRepository.java | 132 +++++++++++------- ...a => FeatureProviderStateManagerTest.java} | 23 +-- .../openfeature/sdk/OpenFeatureAPITest.java | 12 +- .../sdk/OpenFeatureClientTest.java | 98 ++++--------- 10 files changed, 136 insertions(+), 211 deletions(-) rename src/main/java/dev/openfeature/sdk/{StatefulFeatureProvider.java => FeatureProviderStateManager.java} (77%) rename src/test/java/dev/openfeature/sdk/{StatefulFeatureProviderTest.java => FeatureProviderStateManagerTest.java} (92%) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 0cd8e81ac..1fef9d027 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -108,8 +108,8 @@ public boolean equals(Object obj) { if (obj == null) { return false; } - if (obj instanceof StatefulFeatureProvider) { - return this == ((StatefulFeatureProvider) obj).getDelegate(); + if (obj instanceof FeatureProviderStateManager) { + return this == ((FeatureProviderStateManager) obj).getProvider(); } return this == obj; } diff --git a/src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java similarity index 77% rename from src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java rename to src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java index 214ba408c..8814d5146 100644 --- a/src/main/java/dev/openfeature/sdk/StatefulFeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java @@ -1,11 +1,12 @@ package dev.openfeature.sdk; import dev.openfeature.sdk.exceptions.OpenFeatureError; +import lombok.Getter; import lombok.experimental.Delegate; import java.util.concurrent.atomic.AtomicBoolean; -class StatefulFeatureProvider implements FeatureProvider, EventProviderListener { +class FeatureProviderStateManager implements EventProviderListener { private interface ExcludeFromDelegate { void initialize(EvaluationContext evaluationContext) throws Exception; @@ -18,16 +19,16 @@ private interface ExcludeFromDelegate { @Delegate(excludes = ExcludeFromDelegate.class) private final FeatureProvider delegate; private final AtomicBoolean isInitialized = new AtomicBoolean(); + @Getter private ProviderState state = ProviderState.NOT_READY; - public StatefulFeatureProvider(FeatureProvider delegate) { + public FeatureProviderStateManager(FeatureProvider delegate) { this.delegate = delegate; if (delegate instanceof EventProvider) { ((EventProvider) delegate).setEventProviderListener(this); } } - @Override public void initialize(EvaluationContext evaluationContext) throws Exception { if (isInitialized.getAndSet(true)) { return; @@ -50,34 +51,12 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { } } - @Override public void shutdown() { delegate.shutdown(); state = ProviderState.NOT_READY; isInitialized.set(false); } - @Override - public ProviderState getState() { - return state; - } - - @Override - public int hashCode() { - return delegate.hashCode(); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj instanceof StatefulFeatureProvider) { - return delegate.equals(((StatefulFeatureProvider) obj).delegate); - } - return delegate.equals(obj); - } - @Override public void onEmit(ProviderEvent event, ProviderEventDetails details) { if (ProviderEvent.PROVIDER_ERROR.equals(event)) { @@ -93,7 +72,11 @@ public void onEmit(ProviderEvent event, ProviderEventDetails details) { } } - FeatureProvider getDelegate() { + FeatureProvider getProvider() { return delegate; } + + public boolean hasSameProvider(FeatureProvider featureProvider){ + return this.delegate.equals(featureProvider); + } } diff --git a/src/main/java/dev/openfeature/sdk/NoOpProvider.java b/src/main/java/dev/openfeature/sdk/NoOpProvider.java index dd5806f30..2ad59c8bf 100644 --- a/src/main/java/dev/openfeature/sdk/NoOpProvider.java +++ b/src/main/java/dev/openfeature/sdk/NoOpProvider.java @@ -66,15 +66,4 @@ public ProviderEvaluation getObjectEvaluation(String key, Value defaultVa .reason(Reason.DEFAULT.toString()) .build(); } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (obj instanceof StatefulFeatureProvider) { - return ((StatefulFeatureProvider) obj).getDelegate() == this; - } - return obj == this; - } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 394343bfc..a06029d60 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -65,7 +65,7 @@ public Metadata getProviderMetadata(String domain) { } /** - * A factory function for creating new, OpenFeature clients. + * A factory function for creating new, OpenFeature client. * Clients can contain their own state (e.g. logger, hook, context). * Multiple clients can be used to segment feature flag configuration. * All un-named or unbound clients use the default provider. @@ -77,7 +77,7 @@ public Client getClient() { } /** - * A factory function for creating new domainless OpenFeature clients. + * A factory function for creating new domainless OpenFeature client. * Clients can contain their own state (e.g. logger, hook, context). * Multiple clients can be used to segment feature flag configuration. * If there is already a provider bound to this domain, this provider will be used. @@ -91,7 +91,7 @@ public Client getClient(String domain) { } /** - * A factory function for creating new domainless OpenFeature clients. + * A factory function for creating new domainless OpenFeature client. * Clients can contain their own state (e.g. logger, hook, context). * Multiple clients can be used to segment feature flag configuration. * If there is already a provider bound to this domain, this provider will be used. @@ -103,17 +103,7 @@ public Client getClient(String domain) { */ public Client getClient(String domain, String version) { return new OpenFeatureClient( - new ProviderAccessor() { - @Override - public FeatureProvider getProvider() { - return providerRepository.getProvider(domain); - } - - @Override - public ProviderState getProviderState() { - return providerRepository.getProviderState(domain); - } - }, + () -> providerRepository.getFeatureProviderStateManager(domain), this, domain, version diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index ee57af106..7f4604cba 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -63,7 +63,7 @@ public OpenFeatureClient( @Override public ProviderState getProviderState() { - return providerAccessor.getProviderState(); + return providerAccessor.getProviderStateManager().getState(); } /** @@ -122,8 +122,9 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key try { // openfeatureApi.getProvider() must be called once to maintain a consistent reference - provider = providerAccessor.getProvider(); - ProviderState state = providerAccessor.getProviderState(); + FeatureProviderStateManager stateManager = providerAccessor.getProviderStateManager(); + provider = stateManager.getProvider(); + ProviderState state = stateManager.getState(); if (ProviderState.NOT_READY.equals(state)) { throw new ProviderNotReadyError("provider not yet initialized"); } diff --git a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java index 15f62bdfb..0f1a4836d 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java +++ b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java @@ -3,8 +3,7 @@ /** * Provides access to the future provider for the domain of the client. */ -public interface ProviderAccessor { - FeatureProvider getProvider(); - - ProviderState getProviderState(); +@FunctionalInterface +interface ProviderAccessor { + FeatureProviderStateManager getProviderStateManager(); } diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 02813541b..6e9f2a1f8 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -20,21 +20,38 @@ @Slf4j class ProviderRepository { - private final Map providers = new ConcurrentHashMap<>(); - private final AtomicReference defaultProvider = new AtomicReference<>( - new StatefulFeatureProvider(new NoOpProvider()) + private final Map stateManagers = new ConcurrentHashMap<>(); + private final AtomicReference defaultStateManger = new AtomicReference<>( + new FeatureProviderStateManager(new NoOpProvider()) ); private final ExecutorService taskExecutor = Executors.newCachedThreadPool(runnable -> { final Thread thread = new Thread(runnable); thread.setDaemon(true); return thread; }); + private final Object registerStateManagerLock = new Object(); + + FeatureProviderStateManager getFeatureProviderStateManager() { + return defaultStateManger.get(); + } + + FeatureProviderStateManager getFeatureProviderStateManager(String domain) { + if (domain == null) { + return defaultStateManger.get(); + } + FeatureProviderStateManager fromMap = this.stateManagers.get(domain); + if (fromMap == null) { + return this.defaultStateManger.get(); + } else { + return fromMap; + } + } /** * Return the default provider. */ public FeatureProvider getProvider() { - return defaultProvider.get().getDelegate(); + return defaultStateManger.get().getProvider(); } /** @@ -44,33 +61,25 @@ public FeatureProvider getProvider() { * @return A named {@link FeatureProvider} */ public FeatureProvider getProvider(String domain) { - if (domain == null) { - return defaultProvider.get().getDelegate(); - } - StatefulFeatureProvider fromMap = this.providers.get(domain); - if (fromMap == null) { - return this.defaultProvider.get().getDelegate(); - } else { - return fromMap.getDelegate(); - } + return getFeatureProviderStateManager(domain).getProvider(); } public ProviderState getProviderState() { - return defaultProvider.get().getState(); + return getFeatureProviderStateManager().getState(); } public ProviderState getProviderState(FeatureProvider featureProvider) { - if (featureProvider instanceof StatefulFeatureProvider) { - return ((StatefulFeatureProvider) featureProvider).getState(); + if (featureProvider instanceof FeatureProviderStateManager) { + return ((FeatureProviderStateManager) featureProvider).getState(); } - StatefulFeatureProvider defaultProvider = this.defaultProvider.get(); - if (defaultProvider.equals(featureProvider)) { + FeatureProviderStateManager defaultProvider = this.defaultStateManger.get(); + if (defaultProvider.hasSameProvider(featureProvider)) { return defaultProvider.getState(); } - for (StatefulFeatureProvider wrapper : providers.values()) { - if (wrapper.equals(featureProvider)) { + for (FeatureProviderStateManager wrapper : stateManagers.values()) { + if (wrapper.hasSameProvider(featureProvider)) { return wrapper.getState(); } } @@ -78,17 +87,17 @@ public ProviderState getProviderState(FeatureProvider featureProvider) { } public ProviderState getProviderState(String domain) { - return Optional.ofNullable(domain).map(this.providers::get).orElse(this.defaultProvider.get()).getState(); + return Optional.ofNullable(domain).map(this.stateManagers::get).orElse(this.defaultStateManger.get()).getState(); } public List getDomainsForProvider(FeatureProvider provider) { - return providers.entrySet().stream() - .filter(entry -> entry.getValue().equals(provider)) - .map(entry -> entry.getKey()).collect(Collectors.toList()); + return stateManagers.entrySet().stream() + .filter(entry -> entry.getValue().hasSameProvider(provider)) + .map(Map.Entry::getKey).collect(Collectors.toList()); } public Set getAllBoundDomains() { - return providers.keySet(); + return stateManagers.keySet(); } public boolean isDefaultProvider(FeatureProvider provider) { @@ -141,61 +150,78 @@ private void prepareAndInitializeProvider(String domain, Consumer afterShutdown, BiConsumer afterError, boolean waitForInit) { + final FeatureProviderStateManager newStateManager; + final FeatureProviderStateManager oldStateManager; - StatefulFeatureProvider newProviderWrapper = new StatefulFeatureProvider(newProvider); + synchronized (registerStateManagerLock) { + FeatureProviderStateManager existing = getExistingStateManagerForProvider(newProvider); + if (existing == null) { + newStateManager = new FeatureProviderStateManager(newProvider); + // only run afterSet if new provider is not already attached + afterSet.accept(newProvider); + } else { + newStateManager = existing; + } - if (!isProviderRegistered(newProvider)) { - // only run afterSet if new provider is not already attached - afterSet.accept(newProvider); + // provider is set immediately, on this thread + oldStateManager = domain != null + ? this.stateManagers.put(domain, newStateManager) + : this.defaultStateManger.getAndSet(newStateManager); } - // provider is set immediately, on this thread - StatefulFeatureProvider oldProvider = domain != null - ? this.providers.put(domain, newProviderWrapper) - : this.defaultProvider.getAndSet(newProviderWrapper); - if (waitForInit) { - initializeProvider(newProviderWrapper, afterInit, afterShutdown, afterError, oldProvider); + initializeProvider(newStateManager, afterInit, afterShutdown, afterError, oldStateManager); } else { taskExecutor.submit(() -> { - // initialization happens in a different thread if we're not waiting it - initializeProvider(newProviderWrapper, afterInit, afterShutdown, afterError, oldProvider); + // initialization happens in a different thread if we're not waiting for it + initializeProvider(newStateManager, afterInit, afterShutdown, afterError, oldStateManager); }); } } - private void initializeProvider(StatefulFeatureProvider newProvider, + private FeatureProviderStateManager getExistingStateManagerForProvider(FeatureProvider provider) { + for (FeatureProviderStateManager stateManager : stateManagers.values()) { + if (stateManager.hasSameProvider(provider)) return stateManager; + } + FeatureProviderStateManager defaultFeatureProviderStateManager = defaultStateManger.get(); + if (defaultFeatureProviderStateManager.hasSameProvider(provider)) { + return defaultFeatureProviderStateManager; + } + return null; + } + + private void initializeProvider(FeatureProviderStateManager newProvider, Consumer afterInit, Consumer afterShutdown, BiConsumer afterError, - StatefulFeatureProvider oldProvider) { + FeatureProviderStateManager oldProvider) { try { if (ProviderState.NOT_READY.equals(newProvider.getState())) { newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); - afterInit.accept(newProvider.getDelegate()); + afterInit.accept(newProvider.getProvider()); } shutDownOld(oldProvider, afterShutdown); } catch (OpenFeatureError e) { log.error( "Exception when initializing feature provider {}", - newProvider.getDelegate().getClass().getName(), + newProvider.getProvider().getClass().getName(), e ); - afterError.accept(newProvider.getDelegate(), e); + afterError.accept(newProvider.getProvider(), e); } catch (Exception e) { log.error( "Exception when initializing feature provider {}", - newProvider.getDelegate().getClass().getName(), + newProvider.getProvider().getClass().getName(), e ); - afterError.accept(newProvider.getDelegate(), new GeneralError(e)); + afterError.accept(newProvider.getProvider(), new GeneralError(e)); } } - private void shutDownOld(StatefulFeatureProvider oldProvider, Consumer afterShutdown) { - if (oldProvider != null && !isProviderRegistered(oldProvider)) { + private void shutDownOld(FeatureProviderStateManager oldProvider, Consumer afterShutdown) { + if (oldProvider != null && !isStateManagerRegistered(oldProvider)) { shutdownProvider(oldProvider); - afterShutdown.accept(oldProvider.getDelegate()); + afterShutdown.accept(oldProvider.getProvider()); } } @@ -205,16 +231,16 @@ private void shutDownOld(StatefulFeatureProvider oldProvider, Consumer null, api, "name", "version"); EvaluationContext ctx = new ImmutableContext("targeting key", new HashMap<>()); OpenFeatureClient result = client.setEvaluationContext(ctx); assertEquals(client, result); diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 6ea4c7cf9..d7bf70870 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -1,6 +1,7 @@ package dev.openfeature.sdk; import dev.openfeature.sdk.fixtures.HookFixtures; +import lombok.SneakyThrows; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -41,17 +42,8 @@ void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() { when(api.getProvider(any())).thenReturn(provider); when(api.getHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook())); - OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { - @Override - public FeatureProvider getProvider() { - return provider; - } - - @Override - public ProviderState getProviderState() { - return ProviderState.READY; - } - }, api, "name", "version"); + MockProviderRepository mockProviderRepository = new MockProviderRepository(provider, true); + OpenFeatureClient client = new OpenFeatureClient(mockProviderRepository, api, "name", "version"); FlagEvaluationDetails actual = client.getBooleanDetails("feature key", Boolean.FALSE); @@ -80,18 +72,8 @@ void mergeContextTest() { when(api.getProvider()).thenReturn(mockProvider); when(api.getProvider(any())).thenReturn(mockProvider); - - OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { - @Override - public FeatureProvider getProvider() { - return mockProvider; - } - - @Override - public ProviderState getProviderState() { - return ProviderState.READY; - } - }, api, "name", "version"); + MockProviderRepository mockProviderRepository = new MockProviderRepository(mockProvider, true); + OpenFeatureClient client = new OpenFeatureClient(mockProviderRepository, api, "name", "version"); client.setEvaluationContext(ctx); FlagEvaluationDetails result = client.getBooleanDetails(flag, defaultValue); @@ -103,17 +85,7 @@ public ProviderState getProviderState() { @DisplayName("addHooks should allow chaining by returning the same client instance") void addHooksShouldAllowChaining() { OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { - @Override - public FeatureProvider getProvider() { - return null; - } - - @Override - public ProviderState getProviderState() { - return ProviderState.READY; - } - }, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); Hook hook1 = Mockito.mock(Hook.class); Hook hook2 = Mockito.mock(Hook.class); @@ -125,17 +97,7 @@ public ProviderState getProviderState() { @DisplayName("setEvaluationContext should allow chaining by returning the same client instance") void setEvaluationContextShouldAllowChaining() { OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { - @Override - public FeatureProvider getProvider() { - return null; - } - - @Override - public ProviderState getProviderState() { - return ProviderState.READY; - } - }, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); EvaluationContext ctx = new ImmutableContext("targeting key", new HashMap<>()); OpenFeatureClient result = client.setEvaluationContext(ctx); @@ -147,17 +109,12 @@ public ProviderState getProviderState() { void shouldNotCallEvaluationMethodsWhenProviderIsInFatalErrorState() { MockProvider mockProvider = new MockProvider(ProviderState.FATAL); OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { - @Override - public FeatureProvider getProvider() { - return mockProvider; - } - - @Override - public ProviderState getProviderState() { - return ProviderState.FATAL; - } - }, api, "name", "version"); + MockProviderRepository mockProviderRepository = new MockProviderRepository(mockProvider, true); + OpenFeatureClient client = new OpenFeatureClient(mockProviderRepository, api, "name", "version"); + mockProviderRepository.featureProviderStateManager.onEmit( + ProviderEvent.PROVIDER_ERROR, + ProviderEventDetails.builder().errorCode(ErrorCode.PROVIDER_FATAL).build() + ); FlagEvaluationDetails details = client.getBooleanDetails("key", true); assertThat(mockProvider.isEvaluationCalled()).isFalse(); @@ -169,23 +126,30 @@ public ProviderState getProviderState() { void shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState() { MockProvider mockProvider = new MockProvider(ProviderState.NOT_READY); OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(new ProviderAccessor() { - @Override - public FeatureProvider getProvider() { - return mockProvider; - } - - @Override - public ProviderState getProviderState() { - return ProviderState.NOT_READY; - } - }, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(new MockProviderRepository(mockProvider, false), api, "name", "version"); FlagEvaluationDetails details = client.getBooleanDetails("key", true); assertThat(mockProvider.isEvaluationCalled()).isFalse(); assertThat(details.getErrorCode()).isEqualTo(ErrorCode.PROVIDER_NOT_READY); } + private static class MockProviderRepository implements ProviderAccessor { + private final FeatureProviderStateManager featureProviderStateManager; + + @SneakyThrows + public MockProviderRepository(FeatureProvider featureProvider, boolean init) { + this.featureProviderStateManager = new FeatureProviderStateManager(featureProvider); + if (init) { + this.featureProviderStateManager.initialize(null); + } + } + + @Override + public FeatureProviderStateManager getProviderStateManager() { + return featureProviderStateManager; + } + } + private static class MockProvider implements FeatureProvider { private final AtomicBoolean evaluationCalled = new AtomicBoolean(); private final ProviderState providerState; From b0d66a5253d8a5500b19d76c23e23f2ee5a74ae7 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 18 Sep 2024 09:09:18 +0200 Subject: [PATCH 16/23] make FeatureProviderStateManager a true wrapper, without implementing FeatureProvider, fix codestyle Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/FeatureProviderStateManager.java | 2 +- src/main/java/dev/openfeature/sdk/ProviderRepository.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java index 8814d5146..9a94ca18c 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java @@ -76,7 +76,7 @@ FeatureProvider getProvider() { return delegate; } - public boolean hasSameProvider(FeatureProvider featureProvider){ + public boolean hasSameProvider(FeatureProvider featureProvider) { return this.delegate.equals(featureProvider); } } diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 6e9f2a1f8..c3808bbcd 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -87,7 +87,10 @@ public ProviderState getProviderState(FeatureProvider featureProvider) { } public ProviderState getProviderState(String domain) { - return Optional.ofNullable(domain).map(this.stateManagers::get).orElse(this.defaultStateManger.get()).getState(); + return Optional.ofNullable(domain) + .map(this.stateManagers::get) + .orElse(this.defaultStateManger.get()) + .getState(); } public List getDomainsForProvider(FeatureProvider provider) { From 124be3644c768d43c229dbae9eca9971518e355d Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 18 Sep 2024 12:31:54 -0400 Subject: [PATCH 17/23] fixup: pmd fix and remove equals Signed-off-by: Todd Baert --- .../dev/openfeature/sdk/EventProvider.java | 11 ----------- .../openfeature/sdk/ProviderRepository.java | 18 ++++++++++-------- .../java/dev/openfeature/sdk/EventsTest.java | 3 +-- .../java/dev/openfeature/sdk/HookSpecTest.java | 2 +- 4 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 1fef9d027..5d0192b7d 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -102,15 +102,4 @@ protected void emitProviderStale(ProviderEventDetails details) { protected void emitProviderError(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_ERROR, details); } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (obj instanceof FeatureProviderStateManager) { - return this == ((FeatureProviderStateManager) obj).getProvider(); - } - return this == obj; - } } diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index c3808bbcd..d4d620eb5 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -184,7 +184,9 @@ private void prepareAndInitializeProvider(String domain, private FeatureProviderStateManager getExistingStateManagerForProvider(FeatureProvider provider) { for (FeatureProviderStateManager stateManager : stateManagers.values()) { - if (stateManager.hasSameProvider(provider)) return stateManager; + if (stateManager.hasSameProvider(provider)) { + return stateManager; + } } FeatureProviderStateManager defaultFeatureProviderStateManager = defaultStateManger.get(); if (defaultFeatureProviderStateManager.hasSameProvider(provider)) { @@ -231,19 +233,19 @@ private void shutDownOld(FeatureProviderStateManager oldProvider, Consumer handler2 = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); - provider.initialize(null); OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -619,7 +618,7 @@ void removedEventsShouldNotRun() { // emit event provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); - + // both global and client handlers should not run. verify(handler1, after(TIMEOUT).never()).accept(any()); verify(handler2, never()).accept(any()); diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index e2c2825d6..34cc1d495 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -149,7 +149,7 @@ void optional_properties() { @Test void before_runs_ahead_of_evaluation() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new AlwaysBrokenProvider()); + api.setProviderAndWait(new AlwaysBrokenProvider()); Client client = api.getClient(); Hook evalHook = mockBooleanHook(); From 8ca0d2d3dc785eb4012e255201a581e43c117f6d Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 18 Sep 2024 15:47:14 -0400 Subject: [PATCH 18/23] fixup: revert am change on emit Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/EventProvider.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 5d0192b7d..84893bece 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -53,7 +53,7 @@ void detach() { * @param event The event type * @param details The details of the event */ - protected void emit(ProviderEvent event, ProviderEventDetails details) { + public void emit(ProviderEvent event, ProviderEventDetails details) { if (eventProviderListener != null) { eventProviderListener.onEmit(event, details); } @@ -68,7 +68,7 @@ protected void emit(ProviderEvent event, ProviderEventDetails details) { * * @param details The details of the event */ - protected void emitProviderReady(ProviderEventDetails details) { + public void emitProviderReady(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_READY, details); } @@ -79,7 +79,7 @@ protected void emitProviderReady(ProviderEventDetails details) { * * @param details The details of the event */ - protected void emitProviderConfigurationChanged(ProviderEventDetails details) { + public void emitProviderConfigurationChanged(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); } @@ -89,7 +89,7 @@ protected void emitProviderConfigurationChanged(ProviderEventDetails details) { * * @param details The details of the event */ - protected void emitProviderStale(ProviderEventDetails details) { + public void emitProviderStale(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_STALE, details); } @@ -99,7 +99,7 @@ protected void emitProviderStale(ProviderEventDetails details) { * * @param details The details of the event */ - protected void emitProviderError(ProviderEventDetails details) { + public void emitProviderError(ProviderEventDetails details) { emit(ProviderEvent.PROVIDER_ERROR, details); } } From 53dae87c0977d0fe1f5eeaedc8a831239e5b52f7 Mon Sep 17 00:00:00 2001 From: chrfwow Date: Thu, 19 Sep 2024 09:13:45 +0200 Subject: [PATCH 19/23] minor refactorings, update readme Signed-off-by: christian.lutnik --- README.md | 6 ++- .../openfeature/sdk/ProviderRepository.java | 44 +++++++++---------- .../java/dev/openfeature/sdk/EventsTest.java | 1 - 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index a7e984624..21044925c 100644 --- a/README.md +++ b/README.md @@ -318,8 +318,9 @@ public class MyProvider implements FeatureProvider { } @Override + @Deprecated public ProviderState getState() { - // optionally indicate your provider's state (assumed to be READY if not implemented) + // the state of a FeatureProvider is managed by the SDK } @Override @@ -369,8 +370,9 @@ class MyEventProvider extends EventProvider { } @Override + @Deprecated public ProviderState getState() { - // indicate your provider's state (required for EventProviders) + // the state of an EventProvider is managed by the SDK } @Override diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index c3808bbcd..07e1e954d 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -193,57 +193,57 @@ private FeatureProviderStateManager getExistingStateManagerForProvider(FeaturePr return null; } - private void initializeProvider(FeatureProviderStateManager newProvider, + private void initializeProvider(FeatureProviderStateManager newManager, Consumer afterInit, Consumer afterShutdown, BiConsumer afterError, - FeatureProviderStateManager oldProvider) { + FeatureProviderStateManager oldManager) { try { - if (ProviderState.NOT_READY.equals(newProvider.getState())) { - newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); - afterInit.accept(newProvider.getProvider()); + if (ProviderState.NOT_READY.equals(newManager.getState())) { + newManager.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); + afterInit.accept(newManager.getProvider()); } - shutDownOld(oldProvider, afterShutdown); + shutDownOld(oldManager, afterShutdown); } catch (OpenFeatureError e) { log.error( "Exception when initializing feature provider {}", - newProvider.getProvider().getClass().getName(), + newManager.getProvider().getClass().getName(), e ); - afterError.accept(newProvider.getProvider(), e); + afterError.accept(newManager.getProvider(), e); } catch (Exception e) { log.error( "Exception when initializing feature provider {}", - newProvider.getProvider().getClass().getName(), + newManager.getProvider().getClass().getName(), e ); - afterError.accept(newProvider.getProvider(), new GeneralError(e)); + afterError.accept(newManager.getProvider(), new GeneralError(e)); } } - private void shutDownOld(FeatureProviderStateManager oldProvider, Consumer afterShutdown) { - if (oldProvider != null && !isStateManagerRegistered(oldProvider)) { - shutdownProvider(oldProvider); - afterShutdown.accept(oldProvider.getProvider()); + private void shutDownOld(FeatureProviderStateManager oldManager, Consumer afterShutdown) { + if (oldManager != null && !isStateManagerRegistered(oldManager)) { + shutdownProvider(oldManager); + afterShutdown.accept(oldManager.getProvider()); } } /** - * Helper to check if provider is already known (registered). + * Helper to check if manager is already known (registered). * - * @param provider provider to check for registration + * @param manager manager to check for registration * @return boolean true if already registered, false otherwise */ - private boolean isStateManagerRegistered(FeatureProviderStateManager provider) { - return provider != null - && (this.stateManagers.containsValue(provider) || this.defaultStateManger.get().equals(provider)); + private boolean isStateManagerRegistered(FeatureProviderStateManager manager) { + return manager != null + && (this.stateManagers.containsValue(manager) || this.defaultStateManger.get().equals(manager)); } - private void shutdownProvider(FeatureProviderStateManager provider) { - if (provider == null) { + private void shutdownProvider(FeatureProviderStateManager manager) { + if (manager == null) { return; } - shutdownProvider(provider.getProvider()); + shutdownProvider(manager.getProvider()); } private void shutdownProvider(FeatureProvider provider) { diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 8ccf584ea..73b82fabb 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -606,7 +606,6 @@ void removedEventsShouldNotRun() { final Consumer handler2 = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); - provider.initialize(null); OpenFeatureAPI.getInstance().setProviderAndWait(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); From 975d3ad14a62007a8dec4826cd74392e6d4e82da Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Thu, 19 Sep 2024 09:19:59 +0200 Subject: [PATCH 20/23] update readme Signed-off-by: christian.lutnik --- README.md | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 21044925c..0b38f058f 100644 --- a/README.md +++ b/README.md @@ -317,12 +317,6 @@ public class MyProvider implements FeatureProvider { return () -> "My Provider"; } - @Override - @Deprecated - public ProviderState getState() { - // the state of a FeatureProvider is managed by the SDK - } - @Override public void initialize(EvaluationContext evaluationContext) throws Exception { // start up your provider @@ -369,12 +363,6 @@ class MyEventProvider extends EventProvider { return () -> "My Event Provider"; } - @Override - @Deprecated - public ProviderState getState() { - // the state of an EventProvider is managed by the SDK - } - @Override public void initialize(EvaluationContext evaluationContext) throws Exception { // emit events when flags are changed in a hypothetical REST API @@ -393,6 +381,13 @@ class MyEventProvider extends EventProvider { } ``` +Providers no longer need to manage their own state, this is done by the SDK itself. If desired, the state of a provider +can be queried through the client that uses the provider. + +```java +OpenFeatureAPI.getInstance().getClient().getProviderState(); +``` + > Built a new provider? [Let us know](https://github.com/open-feature/openfeature.dev/issues/new?assignees=&labels=provider&projects=&template=document-provider.yaml&title=%5BProvider%5D%3A+) so we can add it to the docs! ### Develop a hook From ca8fc16f315f4ec036f9ee16b5c32ab803c13200 Mon Sep 17 00:00:00 2001 From: chrfwow Date: Fri, 20 Sep 2024 12:53:26 +0200 Subject: [PATCH 21/23] remove unused delegate, update comments Signed-off-by: christian.lutnik --- .../openfeature/sdk/FeatureProviderStateManager.java | 11 ----------- .../java/dev/openfeature/sdk/OpenFeatureClient.java | 2 +- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java index 9a94ca18c..973d46997 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProviderStateManager.java @@ -2,21 +2,10 @@ import dev.openfeature.sdk.exceptions.OpenFeatureError; import lombok.Getter; -import lombok.experimental.Delegate; import java.util.concurrent.atomic.AtomicBoolean; class FeatureProviderStateManager implements EventProviderListener { - - private interface ExcludeFromDelegate { - void initialize(EvaluationContext evaluationContext) throws Exception; - - void shutdown(); - - ProviderState getState(); - } - - @Delegate(excludes = ExcludeFromDelegate.class) private final FeatureProvider delegate; private final AtomicBoolean isInitialized = new AtomicBoolean(); @Getter diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index f0a16439e..06e083464 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -121,7 +121,7 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key FeatureProvider provider; try { - // openfeatureApi.getProvider() must be called once to maintain a consistent reference + // providerAccessor.getProviderStateManager() must be called once to maintain a consistent reference FeatureProviderStateManager stateManager = providerAccessor.getProviderStateManager(); provider = stateManager.getProvider(); ProviderState state = stateManager.getState(); From 45e9ba2e0b33ee84b0449003daff3f03cb820a45 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 23 Sep 2024 11:54:58 -0400 Subject: [PATCH 22/23] fixup: feedback from guido Signed-off-by: Todd Baert --- .../dev/openfeature/sdk/OpenFeatureAPI.java | 28 +---- .../openfeature/sdk/OpenFeatureClient.java | 9 +- .../dev/openfeature/sdk/ProviderAccessor.java | 9 -- .../java/dev/openfeature/sdk/EventsTest.java | 8 +- .../sdk/FlagEvaluationSpecTest.java | 7 +- .../openfeature/sdk/OpenFeatureAPITest.java | 2 +- .../sdk/OpenFeatureClientTest.java | 105 +++++------------- .../sdk/testutils/TestEventsProvider.java | 12 ++ 8 files changed, 58 insertions(+), 122 deletions(-) delete mode 100644 src/main/java/dev/openfeature/sdk/ProviderAccessor.java diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index a06029d60..d087b5b2c 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -103,7 +103,6 @@ public Client getClient(String domain) { */ public Client getClient(String domain, String version) { return new OpenFeatureClient( - () -> providerRepository.getFeatureProviderStateManager(domain), this, domain, version @@ -285,28 +284,6 @@ public FeatureProvider getProvider(String domain) { return providerRepository.getProvider(domain); } - /** - * Return the state of the default provider. - */ - public ProviderState getProviderState() { - return providerRepository.getProviderState(); - } - - /** - * Get the state of the provider for a domain. If no provider with the domain is found, returns the state of the - * default provider. - * - * @param domain The domain to look for. - * @return A named {@link FeatureProvider} - */ - public ProviderState getProviderState(String domain) { - return providerRepository.getProviderState(domain); - } - - public ProviderState getProviderState(FeatureProvider provider) { - return providerRepository.getProviderState(provider); - } - /** * Adds hooks for globally, used for all evaluations. * Hooks are run in the order they're added in the before stage. They are run in reverse order for all other stages. @@ -424,6 +401,11 @@ void addHandler(String domain, ProviderEvent event, Consumer handl } } + FeatureProviderStateManager getFeatureProviderStateManager(String domain) { + return providerRepository.getFeatureProviderStateManager(domain); + } + + /** * Runs the handlers associated with a particular provider. * diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 06e083464..2162f4130 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -24,7 +24,6 @@ @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public class OpenFeatureClient implements Client { - private final ProviderAccessor providerAccessor; private final OpenFeatureAPI openfeatureApi; @Getter private final String domain; @@ -48,12 +47,10 @@ public class OpenFeatureClient implements Client { */ @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public OpenFeatureClient( - ProviderAccessor providerAccessor, OpenFeatureAPI openFeatureAPI, String domain, String version ) { - this.providerAccessor = providerAccessor; this.openfeatureApi = openFeatureAPI; this.domain = domain; this.version = version; @@ -63,7 +60,7 @@ public OpenFeatureClient( @Override public ProviderState getProviderState() { - return providerAccessor.getProviderStateManager().getState(); + return openfeatureApi.getFeatureProviderStateManager(domain).getState(); } /** @@ -121,8 +118,8 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key FeatureProvider provider; try { - // providerAccessor.getProviderStateManager() must be called once to maintain a consistent reference - FeatureProviderStateManager stateManager = providerAccessor.getProviderStateManager(); + FeatureProviderStateManager stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); + // provider must be accessed once to maintain a consistent reference provider = stateManager.getProvider(); ProviderState state = stateManager.getState(); if (ProviderState.NOT_READY.equals(state)) { diff --git a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java b/src/main/java/dev/openfeature/sdk/ProviderAccessor.java deleted file mode 100644 index 0f1a4836d..000000000 --- a/src/main/java/dev/openfeature/sdk/ProviderAccessor.java +++ /dev/null @@ -1,9 +0,0 @@ -package dev.openfeature.sdk; - -/** - * Provides access to the future provider for the domain of the client. - */ -@FunctionalInterface -interface ProviderAccessor { - FeatureProviderStateManager getProviderStateManager(); -} diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index bab48ee58..41bcf86c4 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -535,12 +535,12 @@ void matchingStaleEventsMustRunImmediately() { // provider which is already stale TestEventsProvider provider = TestEventsProvider.newInitializedTestEventsProvider(); + Client client = api.getClient(name); api.setProviderAndWait(name, provider); provider.emitProviderStale(ProviderEventDetails.builder().build()); - assertThat(api.getProviderState(name)).isEqualTo(ProviderState.STALE); + assertThat(client.getProviderState()).isEqualTo(ProviderState.STALE); // should run even thought handler was added after stale - Client client = api.getClient(name); client.onProviderStale(handler); verify(handler, timeout(TIMEOUT)).accept(any()); } @@ -555,13 +555,13 @@ void matchingErrorEventsMustRunImmediately() { // provider which is already in error TestEventsProvider provider = new TestEventsProvider(); + Client client = api.getClient(name); api.setProviderAndWait(name, provider); provider.emitProviderError(ProviderEventDetails.builder().build()); - assertThat(api.getProviderState(name)).isEqualTo(ProviderState.ERROR); + assertThat(client.getProviderState()).isEqualTo(ProviderState.ERROR); // should run even thought handler was added after error - Client client = api.getClient(name); client.onProviderError(handler); verify(handler, timeout(TIMEOUT)).accept(any()); } diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 4dd28f719..a2316a59c 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -78,12 +78,14 @@ void getApiInstance() { @Test void providerAndWait() { FeatureProvider provider = new TestEventsProvider(500); OpenFeatureAPI.getInstance().setProviderAndWait(provider); - assertThat(api.getProviderState()).isEqualTo(ProviderState.READY); + Client client = api.getClient(); + assertThat(client.getProviderState()).isEqualTo(ProviderState.READY); provider = new TestEventsProvider(500); String providerName = "providerAndWait"; OpenFeatureAPI.getInstance().setProviderAndWait(providerName, provider); - assertThat(api.getProviderState(providerName)).isEqualTo(ProviderState.READY); + Client client2 = api.getClient(providerName); + assertThat(client2.getProviderState()).isEqualTo(ProviderState.READY); } @SneakyThrows @@ -102,7 +104,6 @@ void getApiInstance() { FeatureProvider provider = new TestEventsProvider(100); String providerName = "shouldReturnNotReadyIfNotInitialized"; OpenFeatureAPI.getInstance().setProvider(providerName, provider); - assertThat(api.getProviderState(providerName)).isEqualTo(ProviderState.NOT_READY); Client client = OpenFeatureAPI.getInstance().getClient(providerName); FlagEvaluationDetails details = client.getBooleanDetails("return_error_when_not_initialized", false); assertEquals(ErrorCode.PROVIDER_NOT_READY, details.getErrorCode()); diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java index 83b3ba195..23c758e9f 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java @@ -82,7 +82,7 @@ void settingTransactionalContextPropagatorToNullErrors() { @Test void setEvaluationContextShouldAllowChaining() { - OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); EvaluationContext ctx = new ImmutableContext("targeting key", new HashMap<>()); OpenFeatureClient result = client.setEvaluationContext(ctx); assertEquals(client, result); diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index d7bf70870..82199accf 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -1,7 +1,16 @@ package dev.openfeature.sdk; -import dev.openfeature.sdk.fixtures.HookFixtures; -import lombok.SneakyThrows; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; + +import java.util.HashMap; +import java.util.concurrent.atomic.AtomicBoolean; + import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -10,14 +19,9 @@ import org.simplify4u.slf4jmock.LoggerMock; import org.slf4j.Logger; -import java.util.Arrays; -import java.util.HashMap; -import java.util.concurrent.atomic.AtomicBoolean; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; +import dev.openfeature.sdk.exceptions.FatalError; +import dev.openfeature.sdk.fixtures.HookFixtures; +import dev.openfeature.sdk.testutils.TestEventsProvider; class OpenFeatureClientTest implements HookFixtures { @@ -37,14 +41,10 @@ void reset_logs() { @Test @DisplayName("should not throw exception if hook has different type argument than hookContext") void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() { - DoSomethingProvider provider = new DoSomethingProvider(); - OpenFeatureAPI api = mock(OpenFeatureAPI.class); - when(api.getProvider(any())).thenReturn(provider); - when(api.getHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook())); - - MockProviderRepository mockProviderRepository = new MockProviderRepository(provider, true); - OpenFeatureClient client = new OpenFeatureClient(mockProviderRepository, api, "name", "version"); - + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProvider("shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext", new DoSomethingProvider()); + Client client = api.getClient("shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext"); + client.addHooks(mockBooleanHook(), mockStringHook()); FlagEvaluationDetails actual = client.getBooleanDetails("feature key", Boolean.FALSE); assertThat(actual.getValue()).isTrue(); @@ -56,36 +56,11 @@ void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() { Mockito.verify(logger, never()).error(anyString(), any(), any()); } - @Test - void mergeContextTest() { - String flag = "feature key"; - boolean defaultValue = false; - String targetingKey = "targeting key"; - EvaluationContext ctx = new ImmutableContext(targetingKey, new HashMap<>()); - OpenFeatureAPI api = mock(OpenFeatureAPI.class); - FeatureProvider mockProvider = mock(FeatureProvider.class); - // this makes it so that true is returned only if the targeting key set at the client level is honored - when(mockProvider.getBooleanEvaluation( - eq(flag), eq(defaultValue), argThat( - context -> context.getTargetingKey().equals(targetingKey)))).thenReturn(ProviderEvaluation.builder() - .value(true).build()); - when(api.getProvider()).thenReturn(mockProvider); - when(api.getProvider(any())).thenReturn(mockProvider); - - MockProviderRepository mockProviderRepository = new MockProviderRepository(mockProvider, true); - OpenFeatureClient client = new OpenFeatureClient(mockProviderRepository, api, "name", "version"); - client.setEvaluationContext(ctx); - - FlagEvaluationDetails result = client.getBooleanDetails(flag, defaultValue); - - assertThat(result.getValue()).isTrue(); - } - @Test @DisplayName("addHooks should allow chaining by returning the same client instance") void addHooksShouldAllowChaining() { OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); Hook hook1 = Mockito.mock(Hook.class); Hook hook2 = Mockito.mock(Hook.class); @@ -97,7 +72,7 @@ void addHooksShouldAllowChaining() { @DisplayName("setEvaluationContext should allow chaining by returning the same client instance") void setEvaluationContextShouldAllowChaining() { OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(() -> null, api, "name", "version"); + OpenFeatureClient client = new OpenFeatureClient(api, "name", "version"); EvaluationContext ctx = new ImmutableContext("targeting key", new HashMap<>()); OpenFeatureClient result = client.setEvaluationContext(ctx); @@ -107,49 +82,27 @@ void setEvaluationContextShouldAllowChaining() { @Test @DisplayName("Should not call evaluation methods when the provider has state FATAL") void shouldNotCallEvaluationMethodsWhenProviderIsInFatalErrorState() { - MockProvider mockProvider = new MockProvider(ProviderState.FATAL); - OpenFeatureAPI api = mock(OpenFeatureAPI.class); - MockProviderRepository mockProviderRepository = new MockProviderRepository(mockProvider, true); - OpenFeatureClient client = new OpenFeatureClient(mockProviderRepository, api, "name", "version"); - mockProviderRepository.featureProviderStateManager.onEmit( - ProviderEvent.PROVIDER_ERROR, - ProviderEventDetails.builder().errorCode(ErrorCode.PROVIDER_FATAL).build() - ); - FlagEvaluationDetails details = client.getBooleanDetails("key", true); + FeatureProvider provider = new TestEventsProvider(100, true, "fake fatal", true); + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + Client client = api.getClient("shouldNotCallEvaluationMethodsWhenProviderIsInFatalErrorState"); - assertThat(mockProvider.isEvaluationCalled()).isFalse(); + assertThrows(FatalError.class, () -> api.setProviderAndWait("shouldNotCallEvaluationMethodsWhenProviderIsInFatalErrorState", provider)); + FlagEvaluationDetails details = client.getBooleanDetails("key", true); assertThat(details.getErrorCode()).isEqualTo(ErrorCode.PROVIDER_FATAL); } @Test @DisplayName("Should not call evaluation methods when the provider has state NOT_READY") void shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState() { - MockProvider mockProvider = new MockProvider(ProviderState.NOT_READY); - OpenFeatureAPI api = mock(OpenFeatureAPI.class); - OpenFeatureClient client = new OpenFeatureClient(new MockProviderRepository(mockProvider, false), api, "name", "version"); + FeatureProvider provider = new TestEventsProvider(5000); + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProvider("shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState", provider); + Client client = api.getClient("shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState"); FlagEvaluationDetails details = client.getBooleanDetails("key", true); - assertThat(mockProvider.isEvaluationCalled()).isFalse(); assertThat(details.getErrorCode()).isEqualTo(ErrorCode.PROVIDER_NOT_READY); } - private static class MockProviderRepository implements ProviderAccessor { - private final FeatureProviderStateManager featureProviderStateManager; - - @SneakyThrows - public MockProviderRepository(FeatureProvider featureProvider, boolean init) { - this.featureProviderStateManager = new FeatureProviderStateManager(featureProvider); - if (init) { - this.featureProviderStateManager.initialize(null); - } - } - - @Override - public FeatureProviderStateManager getProviderStateManager() { - return featureProviderStateManager; - } - } - private static class MockProvider implements FeatureProvider { private final AtomicBoolean evaluationCalled = new AtomicBoolean(); private final ProviderState providerState; diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java index 41e7bff4c..1944fce22 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -1,6 +1,7 @@ package dev.openfeature.sdk.testutils; import dev.openfeature.sdk.*; +import dev.openfeature.sdk.exceptions.FatalError; import dev.openfeature.sdk.exceptions.GeneralError; import lombok.SneakyThrows; @@ -13,6 +14,7 @@ public class TestEventsProvider extends EventProvider { private int initTimeoutMs = 0; private String name = "test"; private Metadata metadata = () -> name; + private boolean isFatalInitError = false; public TestEventsProvider() { } @@ -27,6 +29,13 @@ public TestEventsProvider(int initTimeoutMs, boolean initError, String initError this.initErrorMessage = initErrorMessage; } + public TestEventsProvider(int initTimeoutMs, boolean initError, String initErrorMessage, boolean fatal) { + this.initTimeoutMs = initTimeoutMs; + this.initError = initError; + this.initErrorMessage = initErrorMessage; + this.isFatalInitError = fatal; + } + @SneakyThrows public static TestEventsProvider newInitializedTestEventsProvider() { TestEventsProvider provider = new TestEventsProvider(); @@ -52,6 +61,9 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { // wait half the TIMEOUT, otherwise some init/errors can be fired before we add handlers Thread.sleep(initTimeoutMs); if (this.initError) { + if (this.isFatalInitError) { + throw new FatalError(initErrorMessage); + } throw new GeneralError(initErrorMessage); } } From 58167324fb457eed1fa59ac581fe7f9af8401b50 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 23 Sep 2024 12:33:03 -0400 Subject: [PATCH 23/23] fixup: flaky test and spacing Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java | 1 - src/test/java/dev/openfeature/sdk/HookSpecTest.java | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index d087b5b2c..ad528568a 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -405,7 +405,6 @@ FeatureProviderStateManager getFeatureProviderStateManager(String domain) { return providerRepository.getFeatureProviderStateManager(domain); } - /** * Runs the handlers associated with a particular provider. * diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index 34cc1d495..d0d759fa1 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -219,7 +219,7 @@ void error_hook_must_run_if_resolution_details_returns_an_error_code() { void hook_eval_order() { List evalOrder = new ArrayList<>(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new TestEventsProvider() { + api.setProvider("evalOrder", new TestEventsProvider() { public List getProviderHooks() { return Collections.singletonList(new BooleanHook() { @@ -271,7 +271,7 @@ public void finallyAfter(HookContext ctx, Map hints) { } }); - Client c = api.getClient(); + Client c = api.getClient("evalOrder"); c.addHooks(new BooleanHook() { @Override public Optional before(HookContext ctx, Map hints) {