From 8b2b6167f037de2665c156234cfd30cf3ed1a40c Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 14 Jun 2023 16:46:39 -0400 Subject: [PATCH 01/18] feat: events Signed-off-by: Todd Baert --- pom.xml | 18 - src/main/java/dev/openfeature/sdk/Client.java | 2 +- .../dev/openfeature/sdk/EventDetails.java | 28 + .../dev/openfeature/sdk/EventHandling.java | 64 ++ .../dev/openfeature/sdk/EventProvider.java | 83 +++ .../dev/openfeature/sdk/EventSupport.java | 112 +++ .../dev/openfeature/sdk/FeatureProvider.java | 34 +- .../dev/openfeature/sdk/NoOpProvider.java | 6 + .../dev/openfeature/sdk/OpenFeatureAPI.java | 194 +++++- .../openfeature/sdk/OpenFeatureClient.java | 76 ++- .../dev/openfeature/sdk/ProviderEvent.java | 8 + .../openfeature/sdk/ProviderEventDetails.java | 18 + .../openfeature/sdk/ProviderRepository.java | 136 ++-- .../dev/openfeature/sdk/ProviderState.java | 8 + .../openfeature/sdk/internal/ObjectUtils.java | 2 +- .../openfeature/sdk/internal/TriConsumer.java | 30 + .../java/dev/openfeature/sdk/EventsTest.java | 644 ++++++++++++++++++ .../sdk/FlagEvaluationSpecTest.java | 10 +- .../sdk/InitializeBehaviorSpecTest.java | 24 +- .../java/dev/openfeature/sdk/LockingTest.java | 128 +++- .../sdk/ProviderRepositoryTest.java | 281 +++----- .../dev/openfeature/sdk/ProviderSpecTest.java | 2 +- .../sdk/ShutdownBehaviorSpecTest.java | 2 +- .../sdk/fixtures/ProviderFixture.java | 14 +- 24 files changed, 1576 insertions(+), 348 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/EventDetails.java create mode 100644 src/main/java/dev/openfeature/sdk/EventHandling.java create mode 100644 src/main/java/dev/openfeature/sdk/EventProvider.java create mode 100644 src/main/java/dev/openfeature/sdk/EventSupport.java create mode 100644 src/main/java/dev/openfeature/sdk/ProviderEvent.java create mode 100644 src/main/java/dev/openfeature/sdk/ProviderEventDetails.java create mode 100644 src/main/java/dev/openfeature/sdk/ProviderState.java create mode 100644 src/main/java/dev/openfeature/sdk/internal/TriConsumer.java create mode 100644 src/test/java/dev/openfeature/sdk/EventsTest.java diff --git a/pom.xml b/pom.xml index e30d67407..af8807b81 100644 --- a/pom.xml +++ b/pom.xml @@ -178,21 +178,6 @@ - - org.codehaus.mojo - build-helper-maven-plugin - 3.4.0 - - - validate - get-cpu-count - - cpu-count - - - - - org.cyclonedx cyclonedx-maven-plugin @@ -261,9 +246,6 @@ ${surefireArgLine} - - ${cpu.count} - false ${testExclusions} diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java index a4ccf26f9..c5f3d110e 100644 --- a/src/main/java/dev/openfeature/sdk/Client.java +++ b/src/main/java/dev/openfeature/sdk/Client.java @@ -5,7 +5,7 @@ /** * Interface used to resolve flags of varying types. */ -public interface Client extends Features { +public interface Client extends Features, EventHandling { Metadata getMetadata(); /** diff --git a/src/main/java/dev/openfeature/sdk/EventDetails.java b/src/main/java/dev/openfeature/sdk/EventDetails.java new file mode 100644 index 000000000..3f6db159f --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/EventDetails.java @@ -0,0 +1,28 @@ +package dev.openfeature.sdk; + +import edu.umd.cs.findbugs.annotations.Nullable; +import lombok.Data; +import lombok.experimental.SuperBuilder; + +/** + * The details of a particular event. + */ +@Data @SuperBuilder(toBuilder = true) +public class EventDetails extends ProviderEventDetails { + private String clientName; + + static EventDetails fromProviderEventDetails(ProviderEventDetails providerEventDetails) { + return EventDetails.fromProviderEventDetails(providerEventDetails, null); + } + + static EventDetails fromProviderEventDetails( + ProviderEventDetails providerEventDetails, + @Nullable String clientName) { + return EventDetails.builder() + .clientName(clientName) + .flagsChanged(providerEventDetails.getFlagsChanged()) + .eventMetadata(providerEventDetails.getEventMetadata()) + .message(providerEventDetails.getMessage()) + .build(); + } +} diff --git a/src/main/java/dev/openfeature/sdk/EventHandling.java b/src/main/java/dev/openfeature/sdk/EventHandling.java new file mode 100644 index 000000000..31a622708 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/EventHandling.java @@ -0,0 +1,64 @@ +package dev.openfeature.sdk; + +import java.util.function.Consumer; + +/** + * Interface for attaching event handlers. + */ +public interface EventHandling { + + /** + * Add a handler for the {@link ProviderEvent#PROVIDER_READY} event. + * Shorthand for {@link #on(ProviderEvent, Consumer)} + * + * @param handler behavior to add with this event + * @return this + */ + T onProviderReady(Consumer handler); + + /** + * Add a handler for the {@link ProviderEvent#PROVIDER_CONFIGURATION_CHANGED} event. + * Shorthand for {@link #on(ProviderEvent, Consumer)} + * + * @param handler behavior to add with this event + * @return this + */ + T onProviderConfigurationChanged(Consumer handler); + + /** + * Add a handler for the {@link ProviderEvent#PROVIDER_STALE} event. + * Shorthand for {@link #on(ProviderEvent, Consumer)} + * + * @param handler behavior to add with this event + * @return this + */ + T onProviderError(Consumer handler); + + /** + * Add a handler for the {@link ProviderEvent#PROVIDER_ERROR} event. + * Shorthand for {@link #on(ProviderEvent, Consumer)} + * + * @param handler behavior to add with this event + * @return this + */ + T onProviderStale(Consumer handler); + + /** + * Add a handler for the specified {@link ProviderEvent}. + * + * @param event event type + * @param handler behavior to add with this event + * @return this + */ + T on(ProviderEvent event, Consumer handler); + + /** + * Remove the previously attached handler by reference. + * If the handler doesn't exists, no-op. + * + * @param event event type + * @param handler to be removed + * @return this + */ + T removeHandler(ProviderEvent event, Consumer handler); +} diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java new file mode 100644 index 000000000..3235019a3 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -0,0 +1,83 @@ +package dev.openfeature.sdk; + +import org.apache.logging.log4j.util.TriConsumer; + +/** + * Abstract EventProvider. Providers must extend this class to support events. + * Emit events with {@link #emit(ProviderEvent, ProviderEventDetails)}. Please + * note that the SDK will automatically emit + * {@link ProviderEvent#PROVIDER_READY } or + * {@link ProviderEvent#PROVIDER_ERROR } accordingly when + * {@link FeatureProvider#initialize(EvaluationContext)} completes successfully + * or with error, so these events need not be emitted manually during + * initialization. + * + * @see FeatureProvider + */ +public abstract class EventProvider implements FeatureProvider { + + private TriConsumer onEmit = null; + + void detach() { + this.onEmit = null; + } + + void attach(TriConsumer onEmit) { + if (this.onEmit == null) { + this.onEmit = onEmit; + } + } + + /** + * Emit the specified {@link dev.openfeature.sdk.ProviderEvent}. + * + * @param event The event type + * @param details The details of the event + */ + public void emit(ProviderEvent event, ProviderEventDetails details) { + if (this.onEmit != null) { + this.onEmit.accept(this, event, details); + } + } + + /** + * Emit a {@link dev.openfeature.sdk.ProviderEvent#PROVIDER_READY} event. + * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} + * + * @param details The details of the event + */ + public void emitProviderReady(ProviderEventDetails details) { + emit(ProviderEvent.PROVIDER_READY, details); + } + + /** + * Emit a + * {@link dev.openfeature.sdk.ProviderEvent#PROVIDER_CONFIGURATION_CHANGED} + * event. Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} + * + * @param details The details of the event + */ + public void emitProviderConfigurationChanged(ProviderEventDetails details) { + emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); + } + + /** + * Emit a {@link dev.openfeature.sdk.ProviderEvent#PROVIDER_STALE} event. + * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} + * + * @param details The details of the event + */ + public void emitProviderStale(ProviderEventDetails details) { + emit(ProviderEvent.PROVIDER_STALE, details); + } + + /** + * Emit a {@link dev.openfeature.sdk.ProviderEvent#PROVIDER_ERROR} event. + * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} + * + * @param details The details of the event + */ + public void emitProviderError(ProviderEventDetails details) { + emit(ProviderEvent.PROVIDER_ERROR, details); + } +} diff --git a/src/main/java/dev/openfeature/sdk/EventSupport.java b/src/main/java/dev/openfeature/sdk/EventSupport.java new file mode 100644 index 000000000..8658df2cc --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/EventSupport.java @@ -0,0 +1,112 @@ +package dev.openfeature.sdk; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.function.Consumer; + +import edu.umd.cs.findbugs.annotations.Nullable; +import lombok.extern.slf4j.Slf4j; + +/** + * Util class for storing and running handlers. + */ +@Slf4j +class EventSupport { + + private static final String defaultClientUuid = UUID.randomUUID().toString(); + private static final ExecutorService taskExecutor = Executors.newCachedThreadPool(); + private final Map handlerStores = new ConcurrentHashMap<>(); + private final HandlerStore globalHanderStore = new HandlerStore(); + + public void runClientHandlers(@Nullable String clientName, ProviderEvent event, EventDetails eventDetails) { + clientName = Optional.ofNullable(clientName) + .orElse(defaultClientUuid); + + Optional.ofNullable(handlerStores.get(clientName)) + .filter(store -> Optional.of(store).isPresent()) + .map(store -> store.handlerMap.get(event)) + .ifPresent(handlers -> handlers + .forEach(handler -> runHandler(handler, eventDetails))); + } + + public void runGlobalHandlers(ProviderEvent event, EventDetails eventDetails) { + globalHanderStore.handlerMap.get(event) + .forEach(handler -> { + runHandler(handler, eventDetails); + }); + } + + public void addClientHandler(@Nullable String clientName, ProviderEvent event, Consumer handler) { + final String name = Optional.ofNullable(clientName) + .orElse(defaultClientUuid); + + HandlerStore store = Optional.ofNullable(this.handlerStores.get(name)) + .orElseGet(() -> { + HandlerStore newStore = new HandlerStore(); + this.handlerStores.put(name, newStore); + return newStore; + }); + if (this.handlerStores.get(name) == null) { + // TODO: test for not adding tons of handler stores + // Note we create spaces for client handlers lazily, not with every named client. + this.handlerStores.put(name, new HandlerStore()); + } + store.addHandler(event, handler); + } + + public void removeClientHandler(String clientName, ProviderEvent event, Consumer handler) { + clientName = Optional.ofNullable(clientName) + .orElse(defaultClientUuid); + this.handlerStores.get(clientName).removeHandler(event, handler); + } + + public void addGlobalHandler(ProviderEvent event, Consumer handler) { + this.globalHanderStore.addHandler(event, handler); + } + + public void removeGlobalHandler(ProviderEvent event, Consumer handler) { + this.globalHanderStore.removeHandler(event, handler); + } + + public Set getAllClientNames() { + return this.handlerStores.keySet(); + } + + private void runHandler(Consumer handler, EventDetails eventDetails) { + taskExecutor.submit(() -> { + try { + handler.accept(eventDetails); + } catch (Exception e) { + log.error("Exception in event handler {}", handler, e); + } + }); + } + + class HandlerStore { + + private final Map>> handlerMap; + + { + handlerMap = new ConcurrentHashMap>>(); + handlerMap.put(ProviderEvent.PROVIDER_READY, new ArrayList<>()); + handlerMap.put(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, new ArrayList<>()); + handlerMap.put(ProviderEvent.PROVIDER_ERROR, new ArrayList<>()); + handlerMap.put(ProviderEvent.PROVIDER_STALE, new ArrayList<>()); + } + + void addHandler(ProviderEvent event, Consumer handler) { + handlerMap.get(event).add(handler); + } + + void removeHandler(ProviderEvent event, Consumer handler) { + handlerMap.get(event).remove(handler); + } + } +} diff --git a/src/main/java/dev/openfeature/sdk/FeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProvider.java index 7df56a5f0..933166fac 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProvider.java @@ -4,7 +4,9 @@ import java.util.List; /** - * The interface implemented by upstream flag providers to resolve flags for their service. + * The interface implemented by upstream flag providers to resolve flags for + * their service. If you want to support realtime events with your provider, you + * should extend {@link EventProvider} */ public interface FeatureProvider { Metadata getMetadata(); @@ -24,22 +26,28 @@ default List getProviderHooks() { ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx); /** - * This method is called before a provider is used to evaluate flags. Providers can overwrite this method, - * if they have special initialization needed prior being called for flag evaluation. + * This method is called before a provider is used to evaluate flags. Providers + * can overwrite this method, + * if they have special initialization needed prior being called for flag + * evaluation. *

- * It is ok, if the method is expensive as it is executed in the background. All runtime exceptions will be + * It is ok, if the method is expensive as it is executed in the background. All + * runtime exceptions will be * caught and logged. *

*/ - default void initialize() { + default void initialize(EvaluationContext evaluationContext) throws Exception { // Intentionally left blank } /** - * This method is called when a new provider is about to be used to evaluate flags, or the SDK is shut down. - * Providers can overwrite this method, if they have special shutdown actions needed. + * This method is called when a new provider is about to be used to evaluate + * flags, or the SDK is shut down. + * Providers can overwrite this method, if they have special shutdown actions + * needed. *

- * It is ok, if the method is expensive as it is executed in the background. All runtime exceptions will be + * It is ok, if the method is expensive as it is executed in the background. All + * runtime exceptions will be * caught and logged. *

*/ @@ -47,4 +55,14 @@ default void shutdown() { // Intentionally left blank } + /** + * Returns a representation of the current readiness of the provider. + * Providers which do not implement this method are assumed to be ready immediately. + * + * @return ProviderState + */ + default ProviderState getState() { + return ProviderState.READY; + } + } diff --git a/src/main/java/dev/openfeature/sdk/NoOpProvider.java b/src/main/java/dev/openfeature/sdk/NoOpProvider.java index c2e841a53..d3d9ca21b 100644 --- a/src/main/java/dev/openfeature/sdk/NoOpProvider.java +++ b/src/main/java/dev/openfeature/sdk/NoOpProvider.java @@ -10,6 +10,12 @@ public class NoOpProvider implements FeatureProvider { @Getter private final String name = "No-op Provider"; + // The Noop provider is ALWAYS NOT_READY, otherwise READY handlers would run immediately when attached. + @Override + public ProviderState getState() { + return ProviderState.NOT_READY; + } + @Override public Metadata getMetadata() { return new Metadata() { diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 2e921a746..8eb0f6b74 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -1,31 +1,33 @@ package dev.openfeature.sdk; -import dev.openfeature.sdk.internal.AutoCloseableLock; -import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; -import lombok.extern.slf4j.Slf4j; - -import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Set; +import java.util.function.Consumer; + +import javax.annotation.Nullable; + +import dev.openfeature.sdk.internal.AutoCloseableLock; +import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; +import lombok.extern.slf4j.Slf4j; /** - * A global singleton which holds base configuration for the OpenFeature library. + * A global singleton which holds base configuration for the OpenFeature + * library. * Configuration here will be shared across all {@link Client}s. */ @Slf4j -public class OpenFeatureAPI { +public class OpenFeatureAPI implements EventHandling { // package-private multi-read/single-write lock - static AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock(); - static AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock(); - + static AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock(); + private EvaluationContext evaluationContext; private final List apiHooks; - private ProviderRepository providerRepository = new ProviderRepository(); - private EvaluationContext evaluationContext; + private final EventSupport eventSupport = new EventSupport(); protected OpenFeatureAPI() { - this.apiHooks = new ArrayList<>(); + apiHooks = new ArrayList<>(); } private static class SingletonHolder { @@ -49,23 +51,35 @@ public Metadata getProviderMetadata(String clientName) { return getProvider(clientName).getMetadata(); } + /** + * {@inheritDoc} + */ public Client getClient() { return getClient(null, null); } + /** + * {@inheritDoc} + */ public Client getClient(@Nullable String name) { return getClient(name, null); } + /** + * {@inheritDoc} + */ public Client getClient(@Nullable String name, @Nullable String version) { - return new OpenFeatureClient(this, name, version); + return new OpenFeatureClient(this, + () -> this.providerRepository.getProvider(name).getState(), + name, + version); } /** * {@inheritDoc} */ public void setEvaluationContext(EvaluationContext evaluationContext) { - try (AutoCloseableLock __ = contextLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { this.evaluationContext = evaluationContext; } } @@ -74,7 +88,7 @@ public void setEvaluationContext(EvaluationContext evaluationContext) { * {@inheritDoc} */ public EvaluationContext getEvaluationContext() { - try (AutoCloseableLock __ = contextLock.readLockAutoCloseable()) { + try (AutoCloseableLock __ = lock.readLockAutoCloseable()) { return this.evaluationContext; } } @@ -83,7 +97,14 @@ public EvaluationContext getEvaluationContext() { * Set the default provider. */ public void setProvider(FeatureProvider provider) { - providerRepository.setProvider(provider); + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { + providerRepository.setProvider( + provider, + (p) -> attachEventProvider(p), + (p) -> emitReady(p), + (p) -> detachEventProvider(p), + (p, message) -> emitError(p, message)); + } } /** @@ -93,7 +114,37 @@ public void setProvider(FeatureProvider provider) { * @param provider The provider to set. */ public void setProvider(String clientName, FeatureProvider provider) { - providerRepository.setProvider(clientName, provider); + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { + providerRepository.setProvider(clientName, + provider, + (p) -> attachEventProvider(p), + (p) -> emitReady(p), + (p) -> detachEventProvider(p), + (p, message) -> emitError(p, message)); + } + } + + private void attachEventProvider(FeatureProvider provider) { + if (provider instanceof EventProvider) { + ((EventProvider)provider).attach((p, event, details) -> { + runHandlersForProvider(p, event, details); + }); + } + } + + private void emitReady(FeatureProvider provider) { + runHandlersForProvider(provider, ProviderEvent.PROVIDER_READY, ProviderEventDetails.builder().build()); + } + + private void detachEventProvider(FeatureProvider provider) { + if (provider instanceof EventProvider) { + ((EventProvider)provider).detach(); + } + } + + private void emitError(FeatureProvider provider, String message) { + runHandlersForProvider(provider, ProviderEvent.PROVIDER_ERROR, + ProviderEventDetails.builder().message(message).build()); } /** @@ -117,7 +168,7 @@ public FeatureProvider getProvider(String name) { * {@inheritDoc} */ public void addHooks(Hook... hooks) { - try (AutoCloseableLock __ = hooksLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { this.apiHooks.addAll(Arrays.asList(hooks)); } } @@ -126,7 +177,7 @@ public void addHooks(Hook... hooks) { * {@inheritDoc} */ public List getHooks() { - try (AutoCloseableLock __ = hooksLock.readLockAutoCloseable()) { + try (AutoCloseableLock __ = lock.readLockAutoCloseable()) { return this.apiHooks; } } @@ -135,19 +186,118 @@ public List getHooks() { * {@inheritDoc} */ public void clearHooks() { - try (AutoCloseableLock __ = hooksLock.writeLockAutoCloseable()) { + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { this.apiHooks.clear(); } } public void shutdown() { providerRepository.shutdown(); + // TODO: shutdown events } /** - * This method is only here for testing as otherwise all tests after the API shutdown test would fail. + * {@inheritDoc} + */ + @Override + public OpenFeatureAPI onProviderReady(Consumer handler) { + return this.on(ProviderEvent.PROVIDER_READY, handler); + } + + /** + * {@inheritDoc} + */ + @Override + public OpenFeatureAPI onProviderConfigurationChanged(Consumer handler) { + return this.on(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, handler); + } + + /** + * {@inheritDoc} + */ + @Override + public OpenFeatureAPI onProviderStale(Consumer handler) { + return this.on(ProviderEvent.PROVIDER_STALE, handler); + } + + /** + * {@inheritDoc} + */ + @Override + public OpenFeatureAPI onProviderError(Consumer handler) { + return this.on(ProviderEvent.PROVIDER_ERROR, handler); + } + + /** + * {@inheritDoc} + */ + @Override + public OpenFeatureAPI on(ProviderEvent event, Consumer handler) { + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { + this.eventSupport.addGlobalHandler(event, handler); + return this; + } + } + + /** + * {@inheritDoc} + */ + @Override + public OpenFeatureAPI removeHandler(ProviderEvent event, Consumer handler) { + this.eventSupport.removeGlobalHandler(event, handler); + return this; + } + + void removeHandler(String clientName, ProviderEvent event, Consumer handler) { + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { + eventSupport.removeClientHandler(clientName, event, handler); + } + } + + void addHandler(String clientName, ProviderEvent event, Consumer handler) { + try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { + eventSupport.addClientHandler(clientName, event, handler); + } + } + + /** + * This method is only here for testing as otherwise all tests after the API + * shutdown test would fail. */ final void resetProviderRepository() { providerRepository = new ProviderRepository(); } + + /** + * 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 + */ + private void runHandlersForProvider(FeatureProvider provider, ProviderEvent event, ProviderEventDetails details) { + try (AutoCloseableLock __ = lock.readLockAutoCloseable()) { + + List clientNamesForProvider = providerRepository + .getClientNamesForProvider(provider); + + // run the global handlers + eventSupport.runGlobalHandlers(event, EventDetails.fromProviderEventDetails(details)); + + // run the handlers associated with named clients for this provider + clientNamesForProvider.forEach(name -> { + eventSupport.runClientHandlers(name, event, EventDetails.fromProviderEventDetails(details, name)); + }); + + if (providerRepository.isDefaultProvider(provider)) { + // run handlers for clients that have no bound providers (since this is the default) + Set allClientNames = eventSupport.getAllClientNames(); + Set boundClientNames = providerRepository.getAllBoundClientNames(); + allClientNames.removeAll(boundClientNames); + allClientNames.forEach(name -> { + eventSupport.runClientHandlers(name, event, EventDetails.fromProviderEventDetails(details, name)); + }); + } + } + } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 44febf77b..bfeffaaf9 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -5,6 +5,8 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.function.Consumer; +import java.util.function.Supplier; import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.exceptions.OpenFeatureError; @@ -28,24 +30,43 @@ public class OpenFeatureClient implements Client { private final String version; private final List clientHooks; private final HookSupport hookSupport; + // private final EventEmitter emitter = new EventEmitter(); AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock(); AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock(); private EvaluationContext evaluationContext; + private Supplier providerState; /** - * Client for evaluating the flag. There may be multiples of these floating - * around. + * Deprecated constructor. Use OpenFeature.API.getClient() instead. * * @param openFeatureAPI Backing global singleton * @param name Name of the client (used by observability tools). * @param version Version of the client (used by observability tools). + * @deprecated Do not use this constructor it wil be removed. + * Clients created using it will not run event handlers. + * Use the OpenFeatureAPI's getClient factory method instead. */ + @Deprecated() public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String name, String version) { this.openfeatureApi = openFeatureAPI; this.name = name; this.version = version; this.clientHooks = new ArrayList<>(); this.hookSupport = new HookSupport(); + log.warn( + "You've directly constructed a OpenFeatureClient. Use OpenFeature.API.getClient() instead."); + } + + OpenFeatureClient(OpenFeatureAPI openFeatureAPI, + final Supplier providerState, + String name, + String version) { + this.openfeatureApi = openFeatureAPI; + this.providerState = providerState; + this.name = name; + this.version = version; + this.clientHooks = new ArrayList<>(); + this.hookSupport = new HookSupport(); } /** @@ -95,7 +116,6 @@ private FlagEvaluationDetails evaluateFlag(FlagValueType type, String key Map hints = Collections.unmodifiableMap(flagOptions.getHookHints()); ctx = ObjectUtils.defaultIfNull(ctx, () -> new ImmutableContext()); - FlagEvaluationDetails details = null; List mergedHooks = null; HookContext hookCtx = null; @@ -341,4 +361,54 @@ public FlagEvaluationDetails getObjectDetails(String key, Value defaultVa public Metadata getMetadata() { return () -> name; } + + /** + * {@inheritDoc} + */ + @Override + public Client onProviderReady(Consumer handler) { + return on(ProviderEvent.PROVIDER_READY, handler); + } + + /** + * {@inheritDoc} + */ + @Override + public Client onProviderConfigurationChanged(Consumer handler) { + return on(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, handler); + } + + /** + * {@inheritDoc} + */ + @Override + public Client onProviderError(Consumer handler) { + return on(ProviderEvent.PROVIDER_ERROR, handler); + } + + /** + * {@inheritDoc} + */ + @Override + public Client onProviderStale(Consumer handler) { + return on(ProviderEvent.PROVIDER_STALE, handler); + } + + /** + * {@inheritDoc} + */ + @Override + public Client on(ProviderEvent event, Consumer handler) { + OpenFeatureAPI.getInstance().addHandler(name, event, handler); + return this; + } + + /** + * {@inheritDoc} + */ + @Override + public Client removeHandler(ProviderEvent event, Consumer handler) { + OpenFeatureAPI.getInstance().removeHandler(name, event, handler); + return this; + } } diff --git a/src/main/java/dev/openfeature/sdk/ProviderEvent.java b/src/main/java/dev/openfeature/sdk/ProviderEvent.java new file mode 100644 index 000000000..dcefd606a --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/ProviderEvent.java @@ -0,0 +1,8 @@ +package dev.openfeature.sdk; + +/** + * Provider event types. + */ +public enum ProviderEvent { + PROVIDER_READY, PROVIDER_CONFIGURATION_CHANGED, PROVIDER_ERROR, PROVIDER_STALE; +} diff --git a/src/main/java/dev/openfeature/sdk/ProviderEventDetails.java b/src/main/java/dev/openfeature/sdk/ProviderEventDetails.java new file mode 100644 index 000000000..149c92a7e --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/ProviderEventDetails.java @@ -0,0 +1,18 @@ +package dev.openfeature.sdk; + +import java.util.List; + +import javax.annotation.Nullable; + +import lombok.Data; +import lombok.experimental.SuperBuilder; + +/** + * The details of a particular event. + */ +@Data @SuperBuilder(toBuilder = true) +public class ProviderEventDetails { + @Nullable private List flagsChanged; + @Nullable private String message; + @Nullable private ImmutableMetadata eventMetadata; +} diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 5a360eb63..6bbb29c98 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -1,24 +1,28 @@ package dev.openfeature.sdk; -import lombok.extern.slf4j.Slf4j; - +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiConsumer; import java.util.function.Consumer; +import java.util.stream.Collectors; import java.util.stream.Stream; +import javax.annotation.Nullable; + +import lombok.extern.slf4j.Slf4j; + @Slf4j class ProviderRepository { private final Map providers = new ConcurrentHashMap<>(); - private final ExecutorService taskExecutor = Executors.newCachedThreadPool(); - private final Map initializingNamedProviders = new ConcurrentHashMap<>(); private final AtomicReference defaultProvider = new AtomicReference<>(new NoOpProvider()); - private FeatureProvider initializingDefaultProvider; + private final ExecutorService taskExecutor = Executors.newCachedThreadPool(); /** * Return the default provider. @@ -37,14 +41,32 @@ public FeatureProvider getProvider(String name) { return Optional.ofNullable(name).map(this.providers::get).orElse(this.defaultProvider.get()); } + public List getClientNamesForProvider(FeatureProvider provider) { + return providers.entrySet().stream() + .filter(entry -> entry.getValue().equals(provider)) + .map(entry -> entry.getKey()).collect(Collectors.toList()); + } + + public Set getAllBoundClientNames() { + return providers.keySet(); + } + + public boolean isDefaultProvider(FeatureProvider provider) { + return this.getProvider().equals(provider); + } + /** * Set the default provider. */ - public void setProvider(FeatureProvider provider) { + public void setProvider(FeatureProvider provider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError) { if (provider == null) { throw new IllegalArgumentException("Provider cannot be null"); } - initializeProvider(provider); + initializeProvider(null, provider, afterSet, afterInit, afterShutdown, afterError); } /** @@ -53,76 +75,53 @@ public void setProvider(FeatureProvider provider) { * @param clientName The name of the client. * @param provider The provider to set. */ - public void setProvider(String clientName, FeatureProvider provider) { + public void setProvider(String clientName, + FeatureProvider provider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError) { if (provider == null) { throw new IllegalArgumentException("Provider cannot be null"); } if (clientName == null) { throw new IllegalArgumentException("clientName cannot be null"); } - initializeProvider(clientName, provider); - } - - private void initializeProvider(FeatureProvider provider) { - initializingDefaultProvider = provider; - initializeProvider(provider, this::updateDefaultProviderAfterInitialization); - } - - private void initializeProvider(String clientName, FeatureProvider provider) { - initializingNamedProviders.put(clientName, provider); - initializeProvider(provider, newProvider -> updateProviderAfterInit(clientName, newProvider)); - } - - private void initializeProvider(FeatureProvider provider, Consumer afterInitialization) { + initializeProvider(clientName, provider, afterSet, afterInit, afterShutdown, afterError); + } + + private void initializeProvider(@Nullable String clientName, + FeatureProvider newProvider, + Consumer afterSet, + Consumer afterInit, + Consumer afterShutdown, + BiConsumer afterError) { + // provider is set immediately, on this thread + FeatureProvider oldProvider = clientName != null + ? this.providers.put(clientName, newProvider) + : this.defaultProvider.getAndSet(newProvider); + afterSet.accept(newProvider); taskExecutor.submit(() -> { + // initialization happens in a different thread try { - if (!isProviderRegistered(provider)) { - provider.initialize(); + if (ProviderState.NOT_READY.equals(newProvider.getState())) { + newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); } - afterInitialization.accept(provider); + emitReadyAndShutdownOld(clientName, newProvider, oldProvider, afterInit, afterShutdown); } catch (Exception e) { - log.error("Exception when initializing feature provider {}", provider.getClass().getName(), e); + log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e); + afterError.accept(newProvider, e.getMessage()); } }); } - private void updateProviderAfterInit(String clientName, FeatureProvider newProvider) { - Optional - .ofNullable(initializingNamedProviders.get(clientName)) - .filter(initializingProvider -> initializingProvider.equals(newProvider)) - .ifPresent(provider -> updateNamedProviderAfterInitialization(clientName, provider)); - } - - private void updateDefaultProviderAfterInitialization(FeatureProvider initializedProvider) { - Optional - .ofNullable(this.initializingDefaultProvider) - .filter(initializingProvider -> initializingProvider.equals(initializedProvider)) - .ifPresent(this::replaceDefaultProvider); - } - - private void replaceDefaultProvider(FeatureProvider provider) { - FeatureProvider oldProvider = this.defaultProvider.getAndSet(provider); - if (isOldProviderNotBoundByName(oldProvider)) { - shutdownProvider(oldProvider); - } - } - - private boolean isOldProviderNotBoundByName(FeatureProvider oldProvider) { - return !this.providers.containsValue(oldProvider); - } - - private void updateNamedProviderAfterInitialization(String clientName, FeatureProvider initializedProvider) { - Optional - .ofNullable(this.initializingNamedProviders.get(clientName)) - .filter(initializingProvider -> initializingProvider.equals(initializedProvider)) - .ifPresent(provider -> replaceNamedProviderAndShutdownOldOne(clientName, provider)); - } - - private void replaceNamedProviderAndShutdownOldOne(String clientName, FeatureProvider provider) { - FeatureProvider oldProvider = this.providers.put(clientName, provider); - this.initializingNamedProviders.remove(clientName, provider); + private void emitReadyAndShutdownOld(@Nullable String clientName, FeatureProvider newProvider, + FeatureProvider oldProvider, Consumer afterInit, + Consumer afterShutdown) { + afterInit.accept(newProvider); if (!isProviderRegistered(oldProvider)) { shutdownProvider(oldProvider); + afterShutdown.accept(oldProvider); } } @@ -133,6 +132,7 @@ private boolean isProviderRegistered(FeatureProvider oldProvider) { private void shutdownProvider(FeatureProvider provider) { taskExecutor.submit(() -> { try { + // detachProviderEvents(provider); provider.shutdown(); } catch (Exception e) { log.error("Exception when shutting down feature provider {}", provider.getClass().getName(), e); @@ -141,7 +141,8 @@ private void shutdownProvider(FeatureProvider provider) { } /** - * Shutdowns this repository which includes shutting down all FeatureProviders that are registered, + * Shuts down this repository which includes shutting down all FeatureProviders + * that are registered, * including the default feature provider. */ public void shutdown() { @@ -149,7 +150,16 @@ public void shutdown() { .concat(Stream.of(this.defaultProvider.get()), this.providers.values().stream()) .distinct() .forEach(this::shutdownProvider); - setProvider(new NoOpProvider()); + setProvider(new NoOpProvider(), + (FeatureProvider fp) -> { + }, + (FeatureProvider fp) -> { + }, + (FeatureProvider fp) -> { + }, + (FeatureProvider fp, + String message) -> { + }); this.providers.clear(); taskExecutor.shutdown(); } diff --git a/src/main/java/dev/openfeature/sdk/ProviderState.java b/src/main/java/dev/openfeature/sdk/ProviderState.java new file mode 100644 index 000000000..6685f8fe9 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/ProviderState.java @@ -0,0 +1,8 @@ +package dev.openfeature.sdk; + +/** + * Indicates the state of the provider. + */ +public enum ProviderState { + READY, NOT_READY, ERROR; +} diff --git a/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java b/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java index ff16422e0..08b81aa86 100644 --- a/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java +++ b/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java @@ -4,6 +4,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -69,5 +70,4 @@ public static List merge(List... sources) { .flatMap(Collection::stream) .collect(Collectors.toList()); } - } diff --git a/src/main/java/dev/openfeature/sdk/internal/TriConsumer.java b/src/main/java/dev/openfeature/sdk/internal/TriConsumer.java new file mode 100644 index 000000000..b7824cb91 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/internal/TriConsumer.java @@ -0,0 +1,30 @@ +package dev.openfeature.sdk.internal; + +import java.util.Objects; + +/** + * Like {@link java.util.function.BiConsumer} but with 3 params. + * + * @see java.util.function.BiConsumer + */ +@FunctionalInterface +interface TriConsumer { + + /** + * Performs this operation on the given arguments. + * + * @param t the first input argument + * @param u the second input argument + * @param v the third input argument + */ + void accept(T t, U u, V v); + + default TriConsumer andThen(TriConsumer after) { + Objects.requireNonNull(after); + + return (t, u, v) -> { + accept(t, u, v); + after.accept(t, u, v); + }; + } +} \ 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 new file mode 100644 index 000000000..6993e9d36 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -0,0 +1,644 @@ +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 org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentMatcher; + +import io.cucumber.java.AfterAll; + +class EventsTest { + + private static final int TIMEOUT = 1000; + + @AfterAll + public static void resetDefaultProvider() { + OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); + } + + @Nested + class ApiEvents { + + @Nested + class NamedProvider { + + @Nested + class Initialization { + + @Test + @DisplayName("should fire initial READY event when provider init succeeds") + @Specification(number = "5.3.1", text = "If the provider's initialize function terminates normally," + + " PROVIDER_READY handlers MUST run.") + void apiInitReady() { + final Consumer handler = mock(Consumer.class); + final String name = "apiInitReady"; + + TestEventsProvider provider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().onProviderReady(handler); + OpenFeatureAPI.getInstance().setProvider(name, provider); + verify(handler, timeout(TIMEOUT)) + .accept(any()); + } + + @Test + @DisplayName("should fire initial ERROR event when provider init errors") + @Specification(number = "5.3.2", text = "If the provider's initialize function terminates abnormally," + + " PROVIDER_ERROR handlers MUST run.") + void apiInitError() { + final Consumer handler = mock(Consumer.class); + final String name = "apiInitError"; + final String errMessage = "oh no!"; + + TestEventsProvider provider = new TestEventsProvider(true, errMessage); + OpenFeatureAPI.getInstance().onProviderError(handler); + OpenFeatureAPI.getInstance().setProvider(name, provider); + verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { + return details.getMessage().equals(errMessage); + })); + } + } + + @Nested + class ProviderEvents { + + @Test + @DisplayName("should propagate events") + @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, " + + + "the associated client and API event handlers MUST run.") + void apiShouldPropagateEvents() { + final Consumer handler = mock(Consumer.class); + final String name = "apiShouldPropagateEvents"; + + TestEventsProvider provider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(name, provider); + OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler); + + provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, EventDetails.builder().build()); + verify(handler, timeout(TIMEOUT)).accept(any()); + } + + @Test + @DisplayName("should support all event types") + @Specification(number = "5.1.1", text = "The provider MAY define a mechanism for signaling the occurrence " + + "of one of a set of events, including PROVIDER_READY, PROVIDER_ERROR, " + + "PROVIDER_CONFIGURATION_CHANGED and PROVIDER_STALE, with a provider event details payload.") + @Specification(number = "5.2.2", text = "The API MUST provide a function for associating handler functions" + + + " with a particular provider event type.") + void apiShouldSupportAllEventTypes() throws Exception { + final String name = "apiShouldSupportAllEventTypes"; + final Consumer handler1 = mock(Consumer.class); + final Consumer handler2 = mock(Consumer.class); + final Consumer handler3 = mock(Consumer.class); + final Consumer handler4 = mock(Consumer.class); + + TestEventsProvider provider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(name, provider); + + OpenFeatureAPI.getInstance().onProviderReady(handler1); + OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler2); + OpenFeatureAPI.getInstance().onProviderStale(handler3); + OpenFeatureAPI.getInstance().onProviderError(handler4); + + Arrays.asList(ProviderEvent.values()).stream().forEach(eventType -> { + provider.mockEvent(eventType, ProviderEventDetails.builder().build()); + }); + + verify(handler1, timeout(TIMEOUT).atLeastOnce()).accept(any()); + verify(handler2, timeout(TIMEOUT).atLeastOnce()).accept(any()); + verify(handler3, timeout(TIMEOUT).atLeastOnce()).accept(any()); + verify(handler4, timeout(TIMEOUT).atLeastOnce()).accept(any()); + } + } + } + } + + @Nested + class ClientEvents { + + @Nested + class DefaultProvider { + + @Nested + class ProviderEvents { + + @Test + @DisplayName("should propagate events for default provider and anonymous client") + @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, the associated client and API event handlers MUST run.") + void shouldPropagateDefaultAndAnon() { + final Consumer handler = mock(Consumer.class); + + TestEventsProvider provider = new TestEventsProvider(); + // set provider before getting a client + OpenFeatureAPI.getInstance().setProvider(provider); + Client client = OpenFeatureAPI.getInstance().getClient(); + client.onProviderStale(handler); + + provider.mockEvent(ProviderEvent.PROVIDER_STALE, EventDetails.builder().build()); + verify(handler, timeout(TIMEOUT)).accept(any()); + } + + @Test + @DisplayName("should propagate events for default provider and named client") + @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, the associated client and API event handlers MUST run.") + void shouldPropagateDefaultAndNamed() { + final Consumer handler = mock(Consumer.class); + final String name = "shouldPropagateDefaultAndNamed"; + + TestEventsProvider provider = new TestEventsProvider(); + // set provider before getting a client + OpenFeatureAPI.getInstance().setProvider(provider); + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderStale(handler); + + provider.mockEvent(ProviderEvent.PROVIDER_STALE, EventDetails.builder().build()); + verify(handler, timeout(TIMEOUT)).accept(any()); + } + } + } + } + + @Nested + class NamedProvider { + + @Nested + class Initialization { + @Test + @DisplayName("should fire initial READY event when provider init succeeds after client retrieved") + @Specification(number = "5.3.1", text = "If the provider's initialize function terminates normally, PROVIDER_READY handlers MUST run.") + void initReadyProviderBefore() throws InterruptedException { + final Consumer handler = mock(Consumer.class); + final String name = "initReadyProviderBefore"; + + TestEventsProvider provider = new TestEventsProvider(); + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderReady(handler); + // set provider after getting a client + OpenFeatureAPI.getInstance().setProvider(name, provider); + verify(handler, timeout(TIMEOUT).atLeastOnce()) + .accept(argThat(details -> details.getClientName().equals(name))); + } + + @Test + @DisplayName("should fire initial READY event when provider init succeeds before client retrieved") + @Specification(number = "5.3.1", text = "If the provider's initialize function terminates normally, PROVIDER_READY handlers MUST run.") + void initReadyProviderAfter() { + final Consumer handler = mock(Consumer.class); + final String name = "initReadyProviderAfter"; + + TestEventsProvider provider = new TestEventsProvider(); + // set provider before getting a client + OpenFeatureAPI.getInstance().setProvider(name, provider); + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderReady(handler); + verify(handler, timeout(TIMEOUT).atLeastOnce()) + .accept(argThat(details -> details.getClientName().equals(name))); + } + + @Test + @DisplayName("should fire initial ERROR event when provider init errors after client retrieved") + @Specification(number = "5.3.2", text = "If the provider's initialize function terminates abnormally, PROVIDER_ERROR handlers MUST run.") + void initErrorProviderAfter() { + final Consumer handler = mock(Consumer.class); + final String name = "initErrorProviderAfter"; + final String errMessage = "oh no!"; + + TestEventsProvider provider = new TestEventsProvider(true, errMessage); + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderError(handler); + // set provider after getting a client + OpenFeatureAPI.getInstance().setProvider(name, provider); + verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { + return details.getClientName().equals(name) + && details.getMessage().equals(errMessage); + })); + } + + @Test + @DisplayName("should fire initial ERROR event when provider init errors before client retrieved") + @Specification(number = "5.3.2", text = "If the provider's initialize function terminates abnormally, PROVIDER_ERROR handlers MUST run.") + void initErrorProviderBefore() { + final Consumer handler = mock(Consumer.class); + final String name = "initErrorProviderBefore"; + final String errMessage = "oh no!"; + + TestEventsProvider provider = new TestEventsProvider(true, errMessage); + // set provider after getting a client + OpenFeatureAPI.getInstance().setProvider(name, provider); + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderError(handler); + verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { + return details.getClientName().equals(name) + && details.getMessage().equals(errMessage); + })); + } + } + + @Nested + class ProviderEvents { + + @Test + @DisplayName("should propagate events when provider set before client retrieved") + @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, the associated client and API event handlers MUST run.") + void shouldPropagateBefore() { + final Consumer handler = mock(Consumer.class); + final String name = "shouldPropagateBefore"; + + TestEventsProvider provider = new TestEventsProvider(); + // set provider before getting a client + OpenFeatureAPI.getInstance().setProvider(name, provider); + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderConfigurationChanged(handler); + + provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, EventDetails.builder().build()); + verify(handler, timeout(TIMEOUT)).accept(argThat(details -> details.getClientName().equals(name))); + } + + @Test + @DisplayName("should propagate events when provider set after client retrieved") + @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, the associated client and API event handlers MUST run.") + void shouldPropagateAfter() { + + final Consumer handler = mock(Consumer.class); + final String name = "shouldPropagateAfter"; + + TestEventsProvider provider = new TestEventsProvider(); + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderConfigurationChanged(handler); + // set provider after getting a client + OpenFeatureAPI.getInstance().setProvider(name, provider); + + provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, EventDetails.builder().build()); + verify(handler, timeout(TIMEOUT)).accept(argThat(details -> details.getClientName().equals(name))); + } + + @Test + @DisplayName("should support all event types") + @Specification(number = "5.1.1", text = "The provider MAY define a mechanism for signaling the occurrence " + + "of one of a set of events, including PROVIDER_READY, PROVIDER_ERROR, " + + "PROVIDER_CONFIGURATION_CHANGED and PROVIDER_STALE, with a provider event details payload.") + @Specification(number = "5.2.1", text = "The client MUST provide a function for associating handler functions" + + + " with a particular provider event type.") + void shouldSupportAllEventTypes() throws Exception { + final String name = "shouldSupportAllEventTypes"; + final Consumer handler1 = mock(Consumer.class); + final Consumer handler2 = mock(Consumer.class); + final Consumer handler3 = mock(Consumer.class); + final Consumer handler4 = mock(Consumer.class); + + TestEventsProvider provider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(name, provider); + Client client = OpenFeatureAPI.getInstance().getClient(name); + + client.onProviderReady(handler1); + client.onProviderConfigurationChanged(handler2); + client.onProviderStale(handler3); + client.onProviderError(handler4); + + Arrays.asList(ProviderEvent.values()).stream().forEach(eventType -> { + provider.mockEvent(eventType, ProviderEventDetails.builder().build()); + }); + ArgumentMatcher nameMatches = (EventDetails details) -> details.getClientName() + .equals(name); + verify(handler1, timeout(TIMEOUT).atLeastOnce()).accept(argThat(nameMatches)); + verify(handler2, timeout(TIMEOUT).atLeastOnce()).accept(argThat(nameMatches)); + verify(handler3, timeout(TIMEOUT).atLeastOnce()).accept(argThat(nameMatches)); + verify(handler4, timeout(TIMEOUT).atLeastOnce()).accept(argThat(nameMatches)); + } + } + } + + @Test + @DisplayName("shutdown provider should not run handlers") + void shouldNotRunHandlers() throws Exception { + final Consumer handler1 = mock(Consumer.class); + final Consumer handler2 = mock(Consumer.class); + final String name = "shouldNotRunHandlers"; + + TestEventsProvider provider1 = new TestEventsProvider(); + TestEventsProvider provider2 = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(name, provider1); + Client client = OpenFeatureAPI.getInstance().getClient(name); + + // attached handlers + OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler1); + client.onProviderConfigurationChanged(handler2); + + OpenFeatureAPI.getInstance().setProvider(name, provider2); + + // wait for the new provider to be ready and make sure things are cleaned up. + await().until(() -> provider1.isShutDown()); + + // fire old event + provider1.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, EventDetails.builder().build()); + + // a bit of waiting here, but we want to make sure these are indeed never + // called. + verify(handler1, after(TIMEOUT).never()).accept(any()); + verify(handler2, never()).accept(any()); + } + + @Test + @DisplayName("other client handlers should not run") + @Specification(number = "5.1.3", text = "When a provider signals the occurrence of a particular event, " + + "event handlers on clients which are not associated with that provider MUST NOT run.") + void otherClientHandlersShouldNotRun() throws Exception { + final String name1 = "otherClientHandlersShouldNotRun1"; + final String name2 = "otherClientHandlersShouldNotRun2"; + final Consumer handlerToRun = mock(Consumer.class); + final Consumer handlerNotToRun = mock(Consumer.class); + + TestEventsProvider provider1 = new TestEventsProvider(); + TestEventsProvider provider2 = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(name1, provider1); + OpenFeatureAPI.getInstance().setProvider(name2, provider2); + + Client client1 = OpenFeatureAPI.getInstance().getClient(name1); + Client client2 = OpenFeatureAPI.getInstance().getClient(name2); + + client1.onProviderConfigurationChanged(handlerToRun); + client2.onProviderConfigurationChanged(handlerNotToRun); + + provider1.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); + + verify(handlerToRun, timeout(TIMEOUT)).accept(any()); + verify(handlerNotToRun, never()).accept(any()); + } + + @Test + @DisplayName("bound named client handlers should not run with default") + @Specification(number = "5.1.3", text = "When a provider signals the occurrence of a particular event, " + + "event handlers on clients which are not associated with that provider MUST NOT run.") + void boundShouldNotRunWithDefault() throws Exception { + final String name = "boundShouldNotRunWithDefault"; + final Consumer handlerNotToRun = mock(Consumer.class); + + TestEventsProvider namedProvider = new TestEventsProvider(); + TestEventsProvider defaultProvider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(defaultProvider); + + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderConfigurationChanged(handlerNotToRun); + OpenFeatureAPI.getInstance().setProvider(name, namedProvider); + + // await the new provider to make sure the old one is shut down + await().until(() -> namedProvider.getState().equals(ProviderState.READY)); + + // fire event on default provider + defaultProvider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); + + verify(handlerNotToRun, after(TIMEOUT).never()).accept(any()); + OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); + } + + @Test + @DisplayName("unbound named client handlers should run with default") + @Specification(number = "5.1.3", text = "When a provider signals the occurrence of a particular event, " + + "event handlers on clients which are not associated with that provider MUST NOT run.") + void unboundShouldRunWithDefault() throws Exception { + final String name = "unboundShouldRunWithDefault"; + final Consumer handlerToRun = mock(Consumer.class); + + TestEventsProvider defaultProvider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(defaultProvider); + + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderConfigurationChanged(handlerToRun); + + // await the new provider to make sure the old one is shut down + await().until(() -> defaultProvider.getState().equals(ProviderState.READY)); + + // fire event on default provider + defaultProvider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); + + verify(handlerToRun, timeout(TIMEOUT)).accept(any()); + OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); + } + + @Test + @DisplayName("subsequent handlers run if earlier throws") + @Specification(number = "5.2.5", text = "If a handler function terminates abnormally, other handler functions MUST run.") + void handlersRunIfOneThrows() throws Exception { + final String name = "handlersRunIfOneThrows"; + final Consumer errorHandler = mock(Consumer.class); + doThrow(new NullPointerException()).when(errorHandler).accept(any()); + final Consumer nextHandler = mock(Consumer.class); + final Consumer lastHandler = mock(Consumer.class); + + TestEventsProvider provider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(name, provider); + + Client client1 = OpenFeatureAPI.getInstance().getClient(name); + + client1.onProviderConfigurationChanged(errorHandler); + client1.onProviderConfigurationChanged(nextHandler); + client1.onProviderConfigurationChanged(lastHandler); + + provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); + verify(errorHandler, timeout(TIMEOUT)).accept(any()); + verify(nextHandler, timeout(TIMEOUT)).accept(any()); + verify(lastHandler, timeout(TIMEOUT)).accept(any()); + } + + @Test + @DisplayName("should have all properties") + @Specification(number = "5.2.4", text = "The handler function MUST accept a event details parameter.") + @Specification(number = "5.2.3", text = "The event details MUST contain the client name associated with the event.") + void shouldHaveAllProperties() throws Exception { + final Consumer handler1 = mock(Consumer.class); + final Consumer handler2 = mock(Consumer.class); + final String name = "shouldHaveAllProperties"; + + TestEventsProvider provider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(name, provider); + Client client = OpenFeatureAPI.getInstance().getClient(name); + + // attached handlers + OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler1); + client.onProviderConfigurationChanged(handler2); + + List flagsChanged = Arrays.asList("flag"); + ImmutableMetadata metadata = ImmutableMetadata.builder().addInteger("int", 1).build(); + String message = "a message"; + ProviderEventDetails details = ProviderEventDetails.builder() + .eventMetadata(metadata) + .flagsChanged(flagsChanged) + .message(message) + .build(); + + provider.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); + + // both global and client handler should have all the fields. + verify(handler1, timeout(TIMEOUT)) + .accept(argThat((EventDetails eventDetails) -> { + return metadata.equals(eventDetails.getEventMetadata()) + // TODO: issue for client name in events + && flagsChanged.equals(eventDetails.getFlagsChanged()) + && message.equals(eventDetails.getMessage()); + })); + verify(handler2, timeout(TIMEOUT)) + .accept(argThat((EventDetails eventDetails) -> { + return metadata.equals(eventDetails.getEventMetadata()) + && flagsChanged.equals(eventDetails.getFlagsChanged()) + && message.equals(eventDetails.getMessage()) + && name.equals(eventDetails.getClientName()); + })); + } + + @Test + @DisplayName("must persist across changes") + @Specification(number = "5.2.6", text = "Event handlers MUST persist across provider changes.") + void mustPersistAcrossChanges() throws Exception { + final String name = "mustPersistAcrossChanges"; + final Consumer handler = mock(Consumer.class); + + TestEventsProvider provider1 = new TestEventsProvider(); + TestEventsProvider provider2 = new TestEventsProvider(); + + OpenFeatureAPI.getInstance().setProvider(name, provider1); + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderConfigurationChanged(handler); + + provider1.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); + ArgumentMatcher nameMatches = (EventDetails details) -> details.getClientName().equals(name); + + 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)); + + // verify that with the new provider under the same name, the handler is called + // again. + provider2.mockEvent(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, ProviderEventDetails.builder().build()); + verify(handler, timeout(TIMEOUT).times(2)).accept(argThat(nameMatches)); + } + + @Nested + class HandlerRemoval { + @Test + @DisplayName("should not run removed events") + void removedEventsShouldNotRun() { + final String name = "removedEventsShouldNotRun"; + final Consumer handler1 = mock(Consumer.class); + final Consumer handler2 = mock(Consumer.class); + + TestEventsProvider provider = new TestEventsProvider(); + OpenFeatureAPI.getInstance().setProvider(name, provider); + Client client = OpenFeatureAPI.getInstance().getClient(name); + + // attached handlers + OpenFeatureAPI.getInstance().onProviderStale(handler1); + client.onProviderConfigurationChanged(handler2); + + OpenFeatureAPI.getInstance().removeHandler(ProviderEvent.PROVIDER_STALE, handler1); + client.removeHandler(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, handler2); + + // 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()); + } + } + + @Specification(number = "5.1.4", text = "PROVIDER_ERROR events SHOULD populate the provider event details's error message field.") + @Test + void thisIsAProviderRequirement() { + } + + class TestEventsProvider extends EventProvider implements FeatureProvider { + + private boolean initError = false; + private String initErrorMessage; + private ProviderState state = ProviderState.NOT_READY; + private boolean shutDown = false; + + @Override + public ProviderState getState() { + return this.state; + } + + TestEventsProvider() { + } + + TestEventsProvider(boolean initError, String initErrorMessage) { + this.initError = initError; + this.initErrorMessage = initErrorMessage; + } + + public void mockEvent(ProviderEvent event, ProviderEventDetails details) { + emit(event, details); + } + + public boolean isShutDown() { + return this.shutDown; + } + + @Override + public void shutdown() { + this.shutDown = true; + } + + @Override + public void initialize(EvaluationContext evaluationContext) throws Exception { + if (this.initError) { + this.state = ProviderState.ERROR; + throw new Exception(initErrorMessage); + } + this.state = ProviderState.READY; + } + + @Override + public Metadata getMetadata() { + throw new UnsupportedOperationException("Unimplemented method 'getMetadata'"); + } + + @Override + public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getBooleanEvaluation'"); + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getStringEvaluation'"); + } + + @Override + public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getIntegerEvaluation'"); + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getDoubleEvaluation'"); + } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getObjectEvaluation'"); + } + }; +} diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 57f0c0454..eb41fd950 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -62,20 +62,20 @@ void getApiInstance() { assertSame(OpenFeatureAPI.getInstance(), OpenFeatureAPI.getInstance()); } - @Specification(number="1.1.2", text="The API MUST provide a function to set the global provider singleton, which accepts an API-conformant provider implementation.") + @Specification(number="1.1.2.1", text="The API MUST define a provider mutator, a function to set the default provider, which accepts an API-conformant provider implementation.") @Test void provider() { FeatureProvider mockProvider = mock(FeatureProvider.class); FeatureProviderTestUtils.setFeatureProvider(mockProvider); assertThat(api.getProvider()).isEqualTo(mockProvider); } - @Specification(number="1.1.4", text="The API MUST provide a function for retrieving the metadata field of the configured provider.") + @Specification(number="1.1.5", text="The API MUST provide a function for retrieving the metadata field of the configured provider.") @Test void provider_metadata() { FeatureProviderTestUtils.setFeatureProvider(new DoSomethingProvider()); assertThat(api.getProviderMetadata().getName()).isEqualTo(DoSomethingProvider.name); } - @Specification(number="1.1.3", text="The API MUST provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") + @Specification(number="1.1.4", text="The API MUST provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") @Test void hook_addition() { Hook h1 = mock(Hook.class); Hook h2 = mock(Hook.class); @@ -89,7 +89,7 @@ void getApiInstance() { assertEquals(h2, api.getHooks().get(1)); } - @Specification(number="1.1.5", text="The API MUST provide a function for creating a client which accepts the following options: - name (optional): A logical string identifier for the client.") + @Specification(number="1.1.6", text="The API MUST provide a function for creating a client which accepts the following options: - name (optional): A logical string identifier for the client.") @Test void namedClient() { assertThatCode(() -> api.getClient("Sir Calls-a-lot")).doesNotThrowAnyException(); // TODO: Doesn't say that you can *get* the client name.. which seems useful? @@ -286,7 +286,7 @@ void getApiInstance() { @Specification(number="1.3.3", text="The client SHOULD guarantee the returned value of any typed flag evaluation method is of the expected type. If the value returned by the underlying provider implementation does not match the expected type, it's to be considered abnormal execution, and the supplied default value should be returned.") @Test void type_system_prevents_this() {} - @Specification(number="1.1.6", text="The client creation function MUST NOT throw, or otherwise abnormally terminate.") + @Specification(number="1.1.7", text="The client creation function MUST NOT throw, or otherwise abnormally terminate.") @Test void constructor_does_not_throw() {} @Specification(number="1.4.11", text="The client SHOULD provide asynchronous or non-blocking mechanisms for flag evaluation.") diff --git a/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java b/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java index 7061719fa..0ab5e3719 100644 --- a/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/InitializeBehaviorSpecTest.java @@ -21,12 +21,13 @@ class DefaultProvider { @Test @DisplayName("must call initialize function of the newly registered provider before using it for " + "flag evaluation") - void mustCallInitializeFunctionOfTheNewlyRegisteredProviderBeforeUsingItForFlagEvaluation() { + void mustCallInitializeFunctionOfTheNewlyRegisteredProviderBeforeUsingItForFlagEvaluation() throws Exception { FeatureProvider featureProvider = mock(FeatureProvider.class); + doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); OpenFeatureAPI.getInstance().setProvider(featureProvider); - verify(featureProvider, timeout(1000)).initialize(); + verify(featureProvider, timeout(1000)).initialize(any()); } @Specification(number = "1.4.9", text = "Methods, functions, or operations on the client MUST NOT throw " @@ -35,14 +36,15 @@ void mustCallInitializeFunctionOfTheNewlyRegisteredProviderBeforeUsingItForFlagE + "the purposes for configuration or setup.") @Test @DisplayName("should catch exception thrown by the provider on initialization") - void shouldCatchExceptionThrownByTheProviderOnInitialization() { + void shouldCatchExceptionThrownByTheProviderOnInitialization() throws Exception { FeatureProvider featureProvider = mock(FeatureProvider.class); - doThrow(TestException.class).when(featureProvider).initialize(); + doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); + doThrow(TestException.class).when(featureProvider).initialize(any()); assertThatCode(() -> OpenFeatureAPI.getInstance().setProvider(featureProvider)) .doesNotThrowAnyException(); - verify(featureProvider, timeout(1000)).initialize(); + verify(featureProvider, timeout(1000)).initialize(any()); } } @@ -54,12 +56,13 @@ class ProviderForNamedClient { @Test @DisplayName("must call initialize function of the newly registered named provider before using it " + "for flag evaluation") - void mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItForFlagEvaluation() { + void mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItForFlagEvaluation() throws Exception { FeatureProvider featureProvider = mock(FeatureProvider.class); + doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); OpenFeatureAPI.getInstance().setProvider("clientName", featureProvider); - verify(featureProvider, timeout(1000)).initialize(); + verify(featureProvider, timeout(1000)).initialize(any()); } @Specification(number = "1.4.9", text = "Methods, functions, or operations on the client MUST NOT throw " @@ -68,14 +71,15 @@ void mustCallInitializeFunctionOfTheNewlyRegisteredNamedProviderBeforeUsingItFor + "the purposes for configuration or setup.") @Test @DisplayName("should catch exception thrown by the named client provider on initialization") - void shouldCatchExceptionThrownByTheNamedClientProviderOnInitialization() { + void shouldCatchExceptionThrownByTheNamedClientProviderOnInitialization() throws Exception { FeatureProvider featureProvider = mock(FeatureProvider.class); - doThrow(TestException.class).when(featureProvider).initialize(); + doReturn(ProviderState.NOT_READY).when(featureProvider).getState(); + doThrow(TestException.class).when(featureProvider).initialize(any()); assertThatCode(() -> OpenFeatureAPI.getInstance().setProvider("clientName", featureProvider)) .doesNotThrowAnyException(); - verify(featureProvider, timeout(1000)).initialize(); + verify(featureProvider, timeout(1000)).initialize(any()); } } } diff --git a/src/test/java/dev/openfeature/sdk/LockingTest.java b/src/test/java/dev/openfeature/sdk/LockingTest.java index 3d8d90c8f..d9601e85b 100644 --- a/src/test/java/dev/openfeature/sdk/LockingTest.java +++ b/src/test/java/dev/openfeature/sdk/LockingTest.java @@ -6,19 +6,20 @@ import static org.mockito.Mockito.when; import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.Consumer; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; class LockingTest { - + private static OpenFeatureAPI api; private OpenFeatureClient client; - private AutoCloseableReentrantReadWriteLock apiContextLock; - private AutoCloseableReentrantReadWriteLock apiHooksLock; + private AutoCloseableReentrantReadWriteLock apiLock; private AutoCloseableReentrantReadWriteLock clientContextLock; private AutoCloseableReentrantReadWriteLock clientHooksLock; @@ -31,10 +32,8 @@ static void beforeAll() { void beforeEach() { client = (OpenFeatureClient) api.getClient(); - apiContextLock = setupLock(apiContextLock, mockInnerReadLock(), mockInnerWriteLock()); - apiHooksLock = setupLock(apiHooksLock, mockInnerReadLock(), mockInnerWriteLock()); - OpenFeatureAPI.contextLock = apiContextLock; - OpenFeatureAPI.hooksLock = apiHooksLock; + apiLock = setupLock(apiLock, mockInnerReadLock(), mockInnerWriteLock()); + OpenFeatureAPI.lock = apiLock; clientContextLock = setupLock(clientContextLock, mockInnerReadLock(), mockInnerWriteLock()); clientHooksLock = setupLock(clientHooksLock, mockInnerReadLock(), mockInnerWriteLock()); @@ -42,6 +41,101 @@ void beforeEach() { client.hooksLock = clientHooksLock; } + @Nested + class EventsLocking { + + @Nested + class Api { + + @Test + void onShouldWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.on(ProviderEvent.PROVIDER_READY, handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + + @Test + void onProviderReadyShouldWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.onProviderReady(handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + + @Test + void onProviderConfigurationChangedShouldWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.onProviderConfigurationChanged(handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + + @Test + void onProviderStaleShouldWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.onProviderStale(handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + + @Test + void onProviderErrorShouldWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.onProviderError(handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + } + + @Nested + class Client { + + // Note that the API lock is used for adding client handlers, they are all added (indirectly) on the API object. + + @Test + void onShouldApiWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + client.on(ProviderEvent.PROVIDER_READY, handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + + @Test + void onProviderReadyShouldApiWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.onProviderReady(handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + + @Test + void onProviderConfigurationChangedProviderReadyShouldApiWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.onProviderConfigurationChanged(handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + + @Test + void onProviderStaleProviderReadyShouldApiWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.onProviderStale(handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + + @Test + void onProviderErrorProviderReadyShouldApiWriteLockAndUnlock() { + Consumer handler = mock(Consumer.class); + api.onProviderError(handler); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); + } + } + } + + @Test void addHooksShouldWriteLockAndUnlock() { client.addHooks(new Hook() { @@ -51,8 +145,8 @@ void addHooksShouldWriteLockAndUnlock() { api.addHooks(new Hook() { }); - verify(apiHooksLock.writeLock()).lock(); - verify(apiHooksLock.writeLock()).unlock(); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); } @Test @@ -62,8 +156,8 @@ void getHooksShouldReadLockAndUnlock() { verify(clientHooksLock.readLock()).unlock(); api.getHooks(); - verify(apiHooksLock.readLock()).lock(); - verify(apiHooksLock.readLock()).unlock(); + verify(apiLock.readLock()).lock(); + verify(apiLock.readLock()).unlock(); } @Test @@ -73,8 +167,8 @@ void setContextShouldWriteLockAndUnlock() { verify(clientContextLock.writeLock()).unlock(); api.setEvaluationContext(new ImmutableContext()); - verify(apiContextLock.writeLock()).lock(); - verify(apiContextLock.writeLock()).unlock(); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); } @Test @@ -84,16 +178,16 @@ void getContextShouldReadLockAndUnlock() { verify(clientContextLock.readLock()).unlock(); api.getEvaluationContext(); - verify(apiContextLock.readLock()).lock(); - verify(apiContextLock.readLock()).unlock(); + verify(apiLock.readLock()).lock(); + verify(apiLock.readLock()).unlock(); } @Test void clearHooksShouldWriteLockAndUnlock() { api.clearHooks(); - verify(apiHooksLock.writeLock()).lock(); - verify(apiHooksLock.writeLock()).unlock(); + verify(apiLock.writeLock()).lock(); + verify(apiLock.writeLock()).unlock(); } private static ReentrantReadWriteLock.ReadLock mockInnerReadLock() { diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index 00c7949e6..4c1faa310 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -1,35 +1,38 @@ package dev.openfeature.sdk; -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 static dev.openfeature.sdk.fixtures.ProviderFixture.createMockedProvider; +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.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; import java.time.Duration; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.function.BiConsumer; +import java.util.function.Consumer; import java.util.function.Function; -import static dev.openfeature.sdk.fixtures.ProviderFixture.*; -import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doBlock; -import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doDelayResponse; -import static java.util.concurrent.TimeUnit.SECONDS; -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.*; +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.testutils.exception.TestException; class ProviderRepositoryTest { private static final String CLIENT_NAME = "client name"; private static final String ANOTHER_CLIENT_NAME = "another client name"; private static final String FEATURE_KEY = "some key"; + private static final int TIMEOUT = 5000; private final ExecutorService executorService = Executors.newCachedThreadPool(); @@ -49,7 +52,8 @@ class DefaultProvider { @Test @DisplayName("should reject null as default provider") void shouldRejectNullAsDefaultProvider() { - assertThatCode(() -> providerRepository.setProvider(null)).isInstanceOf(IllegalArgumentException.class); + assertThatCode(() -> providerRepository.setProvider(null, mockAfterSet(), mockAfterInit(), + mockAfterShutdown(), mockAfterError())).isInstanceOf(IllegalArgumentException.class); } @Test @@ -60,78 +64,35 @@ void shouldHaveNoOpProviderSetAsDefaultOnInitialization() { @Test @DisplayName("should immediately return when calling the provider mutator") - void shouldImmediatelyReturnWhenCallingTheProviderMutator() { + void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { FeatureProvider featureProvider = createMockedProvider(); - doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(); + doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(new ImmutableContext()); await() .alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) .atMost(Duration.ofSeconds(1)) .until(() -> { - providerRepository.setProvider(featureProvider); - verify(featureProvider, timeout(100)).initialize(); + providerRepository.setProvider(featureProvider, mockAfterSet(), mockAfterInit(), + mockAfterShutdown(), mockAfterError()); + verify(featureProvider, timeout(TIMEOUT)).initialize(any()); return true; }); - verify(featureProvider).initialize(); - } - - @Test - @DisplayName("should not return set provider if initialize has not yet been finished executing") - void shouldNotReturnSetProviderIfItsInitializeMethodHasNotYetBeenFinishedExecuting() { - CountDownLatch latch = new CountDownLatch(1); - FeatureProvider newProvider = createMockedProvider(); - doBlock(latch).when(newProvider).initialize(); - FeatureProvider oldProvider = providerRepository.getProvider(); - - providerRepository.setProvider(newProvider); - - FeatureProvider providerWhileInitialization = providerRepository.getProvider(); - latch.countDown(); - - assertThat(providerWhileInitialization).isEqualTo(oldProvider); - await() - .pollDelay(Duration.ofMillis(1)) - .atMost(Duration.ofSeconds(1)) - .untilAsserted(() -> assertThat(providerRepository.getProvider()).isEqualTo(newProvider)); - verify(newProvider, timeout(100)).initialize(); - } - - @SneakyThrows - @Test - @DisplayName("should discard provider still initializing if a newer has finished before") - void shouldDiscardProviderStillInitializingIfANewerHasFinishedBefore() { - CountDownLatch latch = new CountDownLatch(1); - CountDownLatch testBlockingLatch = new CountDownLatch(1); - FeatureProvider blockedProvider = createBlockedProvider(latch, testBlockingLatch::countDown); - FeatureProvider fastProvider = createUnblockingProvider(latch); - - providerRepository.setProvider(blockedProvider); - providerRepository.setProvider(fastProvider); - - assertThat(testBlockingLatch.await(2, SECONDS)) - .as("blocking provider initialization not completed within 2 seconds") - .isTrue(); - - await() - .pollDelay(Duration.ofMillis(1)) - .atMost(Duration.ofSeconds(1)) - .untilAsserted(() -> assertThat(providerRepository.getProvider()).isEqualTo(fastProvider)); - - verify(blockedProvider, timeout(100)).initialize(); - verify(fastProvider, timeout(100)).initialize(); + verify(featureProvider).initialize(any()); } @Test @DisplayName("should avoid additional initialization call if provider has been initialized already") - void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() { + void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() throws Exception { FeatureProvider provider = createMockedProvider(); setFeatureProvider(CLIENT_NAME, provider); + doReturn(ProviderState.READY).when(provider).getState(); + setFeatureProvider(provider); - verify(provider).initialize(); + verify(provider).initialize(any()); } } @@ -141,90 +102,49 @@ class NamedProvider { @Test @DisplayName("should reject null as named provider") void shouldRejectNullAsNamedProvider() { - assertThatCode(() -> providerRepository.setProvider(CLIENT_NAME, null)).isInstanceOf(IllegalArgumentException.class); + assertThatCode(() -> providerRepository.setProvider(CLIENT_NAME, null, mockAfterSet(), mockAfterInit(), + mockAfterShutdown(), mockAfterError())) + .isInstanceOf(IllegalArgumentException.class); } @Test @DisplayName("should reject null as client name") void shouldRejectNullAsDefaultProvider() { NoOpProvider provider = new NoOpProvider(); - assertThatCode(() -> providerRepository.setProvider(null, provider)).isInstanceOf(IllegalArgumentException.class); + assertThatCode(() -> providerRepository.setProvider(null, provider, mockAfterSet(), mockAfterInit(), + mockAfterShutdown(), mockAfterError())) + .isInstanceOf(IllegalArgumentException.class); } @Test @DisplayName("should immediately return when calling the named client provider mutator") - void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() { + void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() throws Exception { FeatureProvider featureProvider = createMockedProvider(); - doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(); + doDelayResponse(Duration.ofSeconds(10)).when(featureProvider).initialize(any()); await() .alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) .atMost(Duration.ofSeconds(1)) .until(() -> { - providerRepository.setProvider("named client", featureProvider); - verify(featureProvider, timeout(1000)).initialize(); + providerRepository.setProvider("named client", featureProvider, mockAfterSet(), + mockAfterInit(), mockAfterShutdown(), mockAfterError()); + verify(featureProvider, timeout(TIMEOUT)).initialize(any()); return true; }); } - @Test - @DisplayName("should not return set provider if it's initialization has not yet been finished executing") - void shouldNotReturnSetProviderIfItsInitializeMethodHasNotYetBeenFinishedExecuting() { - CountDownLatch latch = new CountDownLatch(1); - FeatureProvider newProvider = createMockedProvider(); - doBlock(latch).when(newProvider).initialize(); - FeatureProvider oldProvider = createMockedProvider(); - setFeatureProvider(CLIENT_NAME, oldProvider); - - providerRepository.setProvider(CLIENT_NAME, newProvider); - FeatureProvider providerWhileInitialization = getNamedProvider(); - latch.countDown(); - - assertThat(providerWhileInitialization).isEqualTo(oldProvider); - await() - .pollDelay(Duration.ofMillis(1)) - .atMost(Duration.ofSeconds(1)) - .untilAsserted(() -> assertThat(getNamedProvider()).isEqualTo(newProvider)); - verify(newProvider, timeout(100)).initialize(); - } - - @SneakyThrows - @Test - @DisplayName("should discard provider still initializing if a newer has finished before") - void shouldDiscardProviderStillInitializingIfANewerHasFinishedBefore() { - String clientName = "clientName"; - CountDownLatch latch = new CountDownLatch(1); - CountDownLatch testBlockingLatch = new CountDownLatch(1); - FeatureProvider blockedProvider = createBlockedProvider(latch, testBlockingLatch::countDown); - FeatureProvider unblockingProvider = createUnblockingProvider(latch); - - providerRepository.setProvider(clientName, blockedProvider); - providerRepository.setProvider(clientName, unblockingProvider); - - assertThat(testBlockingLatch.await(2, SECONDS)) - .as("blocking provider initialization not completed within 2 seconds") - .isTrue(); - - await() - .pollDelay(Duration.ofMillis(1)) - .atMost(Duration.ofSeconds(1)) - .untilAsserted(() -> assertThat(providerRepository.getProvider(clientName)) - .isEqualTo(unblockingProvider)); - - verify(blockedProvider, timeout(100)).initialize(); - verify(unblockingProvider, timeout(100)).initialize(); - } - @Test @DisplayName("should avoid additional initialization call if provider has been initialized already") - void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() { + void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() throws Exception { FeatureProvider provider = createMockedProvider(); setFeatureProvider(provider); + doReturn(ProviderState.READY).when(provider).getState(); + setFeatureProvider(CLIENT_NAME, provider); - verify(provider).initialize(); + verify(provider).initialize(any()); } } } @@ -237,43 +157,22 @@ class DefaultProvider { @Test @DisplayName("should immediately return when calling the provider mutator") - void shouldImmediatelyReturnWhenCallingTheProviderMutator() { + void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { FeatureProvider newProvider = createMockedProvider(); - doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(); + doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any()); await() .alias("wait for provider mutator to return") .pollDelay(Duration.ofMillis(1)) .atMost(Duration.ofSeconds(1)) .until(() -> { - providerRepository.setProvider(newProvider); - verify(newProvider, timeout(100)).initialize(); + providerRepository.setProvider(newProvider, mockAfterSet(), mockAfterInit(), + mockAfterShutdown(), mockAfterError()); + verify(newProvider, timeout(TIMEOUT)).initialize(any()); return true; }); - verify(newProvider).initialize(); - } - - @Test - @DisplayName("should use old provider if replacing one has not yet been finished initializing") - void shouldUseOldProviderIfReplacingOneHasNotYetBeenFinishedInitializing() { - CountDownLatch latch = new CountDownLatch(1); - FeatureProvider newProvider = createMockedProvider(); - doBlock(latch).when(newProvider).initialize(); - FeatureProvider oldProvider = createMockedProvider(); - - setFeatureProvider(oldProvider); - providerRepository.setProvider(newProvider); - - providerRepository.getProvider().getBooleanEvaluation("some key", true, null); - latch.countDown(); - - await() - .atMost(Duration.ofSeconds(1)) - .pollDelay(Duration.ofMillis(1)) - .untilAsserted(() -> assertThat(getProvider()).isEqualTo(newProvider)); - verify(oldProvider, timeout(100)).getBooleanEvaluation(any(), any(), any()); - verify(newProvider, never()).getBooleanEvaluation(any(), any(), any()); + verify(newProvider).initialize(any()); } @Test @@ -295,12 +194,13 @@ class NamedProvider { @Test @DisplayName("should immediately return when calling the provider mutator") - void shouldImmediatelyReturnWhenCallingTheProviderMutator() { + void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { FeatureProvider newProvider = createMockedProvider(); - doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(); + doDelayResponse(Duration.ofSeconds(10)).when(newProvider).initialize(any()); Future providerMutation = executorService - .submit(() -> providerRepository.setProvider(CLIENT_NAME, newProvider)); + .submit(() -> providerRepository.setProvider(CLIENT_NAME, newProvider, mockAfterSet(), + mockAfterInit(), mockAfterShutdown(), mockAfterError())); await() .alias("wait for provider mutator to return") @@ -309,34 +209,13 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() { .until(providerMutation::isDone); } - @Test - @DisplayName("should use old provider if replacement one has not yet been finished initializing") - void shouldUseOldProviderIfReplacementHasNotYetBeenFinishedInitializing() { - CountDownLatch latch = new CountDownLatch(1); - FeatureProvider newProvider = createMockedProvider(); - doBlock(latch).when(newProvider).initialize(); - FeatureProvider oldProvider = createMockedProvider(); - - setFeatureProvider(CLIENT_NAME, oldProvider); - providerRepository.setProvider(CLIENT_NAME, newProvider); - - providerRepository.getProvider(CLIENT_NAME).getBooleanEvaluation(FEATURE_KEY, true, null); - latch.countDown(); - - await() - .pollDelay(Duration.ofMillis(1)) - .atMost(Duration.ofSeconds(1)) - .untilAsserted(() -> assertThat(getNamedProvider()).isEqualTo(newProvider)); - verify(oldProvider, timeout(100)).getBooleanEvaluation(eq(FEATURE_KEY), any(), any()); - verify(newProvider, never()).getBooleanEvaluation(any(), any(), any()); - } - @Test @DisplayName("should not call shutdown if replaced provider is bound to multiple names") - void shouldNotCallShutdownIfReplacedProviderIsBoundToMultipleNames() { + void shouldNotCallShutdownIfReplacedProviderIsBoundToMultipleNames() throws InterruptedException { FeatureProvider oldProvider = createMockedProvider(); FeatureProvider newProvider = createMockedProvider(); setFeatureProvider(CLIENT_NAME, oldProvider); + setFeatureProvider(ANOTHER_CLIENT_NAME, oldProvider); setFeatureProvider(CLIENT_NAME, newProvider); @@ -385,7 +264,7 @@ void shouldShutdownAllFeatureProvidersOnShutdown() { await() .pollDelay(Duration.ofMillis(1)) - .atMost(Duration.ofSeconds(1)) + .atMost(Duration.ofSeconds(TIMEOUT)) .untilAsserted(() -> { assertThat(providerRepository.getProvider()).isInstanceOf(NoOpProvider.class); assertThat(providerRepository.getProvider(CLIENT_NAME)).isInstanceOf(NoOpProvider.class); @@ -395,21 +274,15 @@ void shouldShutdownAllFeatureProvidersOnShutdown() { verify(featureProvider2).shutdown(); } - private FeatureProvider getProvider() { - return providerRepository.getProvider(); - } - - private FeatureProvider getNamedProvider() { - return providerRepository.getProvider(CLIENT_NAME); - } - private void setFeatureProvider(FeatureProvider provider) { - providerRepository.setProvider(provider); + providerRepository.setProvider(provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), + mockAfterError()); waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider); } private void setFeatureProvider(String namedProvider, FeatureProvider provider) { - providerRepository.setProvider(namedProvider, provider); + providerRepository.setProvider(namedProvider, provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), + mockAfterError()); waitForSettingProviderHasBeenCompleted(repository -> repository.getProvider(namedProvider), provider); } @@ -418,8 +291,30 @@ private void waitForSettingProviderHasBeenCompleted( FeatureProvider provider) { await() .pollDelay(Duration.ofMillis(1)) - .atMost(Duration.ofSeconds(1)) - .until(() -> extractor.apply(providerRepository) == provider); + .atMost(Duration.ofSeconds(5)) + .until(() -> { + return extractor.apply(providerRepository) == provider; + }); + } + + private Consumer mockAfterSet() { + return fp -> { + }; + } + + private Consumer mockAfterInit() { + return fp -> { + }; + } + + private Consumer mockAfterShutdown() { + return fp -> { + }; + } + + private BiConsumer mockAfterError() { + return (fp, message) -> { + }; } } diff --git a/src/test/java/dev/openfeature/sdk/ProviderSpecTest.java b/src/test/java/dev/openfeature/sdk/ProviderSpecTest.java index 31a6a5e8d..f5e5e6a42 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderSpecTest.java @@ -18,7 +18,7 @@ void name_accessor() { @Specification(number = "2.2.2.1", text = "The feature provider interface MUST define methods for typed " + "flag resolution, including boolean, numeric, string, and structure.") @Specification(number = "2.2.3", text = "In cases of normal execution, the `provider` MUST populate the `resolution details` structure's `value` field with the resolved flag value.") - @Specification(number = "2.2.1", text = "The `feature provider` interface MUST define methods to resolve flag values, with parameters `flag key` (string, required), `default value` (boolean | number | string | structure, required) + and `evaluation context` (optional), which returns a `resolution details` structure.") + @Specification(number = "2.2.1", text = "The `feature provider` interface MUST define methods to resolve flag values, with parameters `flag key` (string, required), `default value` (boolean | number | string | structure, required) and `evaluation context` (optional), which returns a `resolution details` structure.") @Specification(number = "2.2.8.1", text = "The `resolution details` structure SHOULD accept a generic argument (or use an equivalent language feature) which indicates the type of the wrapped `value` field.") @Test void flag_value_set() { diff --git a/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java b/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java index d191c8c42..e62c82e31 100644 --- a/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java @@ -89,7 +89,7 @@ void shouldCatchExceptionThrownByTheNamedClientProviderOnShutdown() { @Nested class General { - @Specification(number = "1.6.1", text = "The API MUST define a shutdown function which, when called, must call the respective shutdown function on the active provider.") + @Specification(number = "1.6.1", text = "The API MUST define a mechanism to propagate a shutdown request to active providers.") @Test @DisplayName("must shutdown all providers on shutting down api") void mustShutdownAllProvidersOnShuttingDownApi() { diff --git a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java index f0b786422..ce14457bf 100644 --- a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java +++ b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java @@ -1,6 +1,8 @@ package dev.openfeature.sdk.fixtures; import dev.openfeature.sdk.FeatureProvider; +import dev.openfeature.sdk.ImmutableContext; +import dev.openfeature.sdk.ProviderState; import lombok.experimental.UtilityClass; import org.mockito.stubbing.Answer; @@ -13,12 +15,14 @@ public class ProviderFixture { public static FeatureProvider createMockedProvider() { - return mock(FeatureProvider.class); + FeatureProvider provider = mock(FeatureProvider.class); + doReturn(ProviderState.NOT_READY).when(provider).getState(); + return provider; } - public static FeatureProvider createBlockedProvider(CountDownLatch latch, Runnable onAnswer) { + public static FeatureProvider createBlockedProvider(CountDownLatch latch, Runnable onAnswer) throws Exception { FeatureProvider provider = createMockedProvider(); - doBlock(latch, createAnswerExecutingCode(onAnswer)).when(provider).initialize(); + doBlock(latch, createAnswerExecutingCode(onAnswer)).when(provider).initialize(new ImmutableContext()); doReturn("blockedProvider").when(provider).toString(); return provider; } @@ -30,12 +34,12 @@ private static Answer createAnswerExecutingCode(Runnable onAnswer) { }; } - public static FeatureProvider createUnblockingProvider(CountDownLatch latch) { + public static FeatureProvider createUnblockingProvider(CountDownLatch latch) throws Exception { FeatureProvider provider = createMockedProvider(); doAnswer(invocation -> { latch.countDown(); return null; - }).when(provider).initialize(); + }).when(provider).initialize(new ImmutableContext()); doReturn("unblockingProvider").when(provider).toString(); return provider; } From 384b96e6b75a31318c21c6186950f6adad4d5153 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 22 Jun 2023 15:41:57 -0400 Subject: [PATCH 02/18] fixup: fix bad import, fix test Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/EventProvider.java | 2 +- .../java/dev/openfeature/sdk/internal/TriConsumer.java | 10 +++++++++- src/test/java/dev/openfeature/sdk/EventsTest.java | 6 ++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 3235019a3..6fbc41d8f 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -1,6 +1,6 @@ package dev.openfeature.sdk; -import org.apache.logging.log4j.util.TriConsumer; +import dev.openfeature.sdk.internal.TriConsumer; /** * Abstract EventProvider. Providers must extend this class to support events. diff --git a/src/main/java/dev/openfeature/sdk/internal/TriConsumer.java b/src/main/java/dev/openfeature/sdk/internal/TriConsumer.java index b7824cb91..723f4aeb4 100644 --- a/src/main/java/dev/openfeature/sdk/internal/TriConsumer.java +++ b/src/main/java/dev/openfeature/sdk/internal/TriConsumer.java @@ -8,7 +8,7 @@ * @see java.util.function.BiConsumer */ @FunctionalInterface -interface TriConsumer { +public interface TriConsumer { /** * Performs this operation on the given arguments. @@ -19,6 +19,14 @@ interface TriConsumer { */ void accept(T t, U u, V v); + /** + * Returns a composed {@code TriConsumer} that performs an additional operation. + * + * @param after the operation to perform after this operation + * @return a composed {@code TriConsumer} that performs in sequence this + * operation followed by the {@code after} operation + * @throws NullPointerException if {@code after} is null + */ default TriConsumer andThen(TriConsumer after) { Objects.requireNonNull(after); diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 6993e9d36..6245a9364 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -23,7 +23,7 @@ class EventsTest { - private static final int TIMEOUT = 1000; + private static final int TIMEOUT = 100; @AfterAll public static void resetDefaultProvider() { @@ -67,7 +67,7 @@ void apiInitError() { OpenFeatureAPI.getInstance().onProviderError(handler); OpenFeatureAPI.getInstance().setProvider(name, provider); verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { - return details.getMessage().equals(errMessage); + return errMessage.equals(details.getMessage()); })); } } @@ -599,6 +599,8 @@ public void shutdown() { @Override public void initialize(EvaluationContext evaluationContext) throws Exception { + // wait half the TIMEOUT, otherwise some init/errors can be fired before we add handlers + Thread.sleep(TIMEOUT / 2); if (this.initError) { this.state = ProviderState.ERROR; throw new Exception(initErrorMessage); From 72fb07767f1ab1f826b661a4e74a3eb245a72d25 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 22 Jun 2023 15:56:48 -0400 Subject: [PATCH 03/18] fixup: add more coverage of internal Signed-off-by: Todd Baert --- .../sdk/internal/TriConsumerTest.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java diff --git a/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java b/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java new file mode 100644 index 000000000..332a27d87 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java @@ -0,0 +1,37 @@ +package dev.openfeature.sdk.internal; + +import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.*; + +import static dev.openfeature.sdk.internal.ObjectUtils.defaultIfNull; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; + +class TriConsumerTest { + + @Test + @DisplayName("should run accept") + void shouldRunAccept() { + AtomicInteger result = new AtomicInteger(0); + TriConsumer triConsumer = (num1, num2, num3) -> { + result.set(result.get() + num1 + num2 + num3); + }; + triConsumer.accept(1, 2, 3); + assertEquals(6, result.get()); + } + + @Test + @DisplayName("should run after accept") + void shouldRunAfterAccept() { + AtomicInteger result = new AtomicInteger(0); + TriConsumer triConsumer = (num1, num2, num3) -> { + result.set(result.get() + num1 + num2 + num3); + }; + TriConsumer composed = triConsumer.andThen(triConsumer); + composed.accept(1, 2, 3); + assertEquals(12, result.get()); + } +} \ No newline at end of file From a8eea105db5d1f0ed49d2871f327746392ba9416 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 22 Jun 2023 15:57:29 -0400 Subject: [PATCH 04/18] fixup: remove unused imports Signed-off-by: Todd Baert --- .../dev/openfeature/sdk/internal/TriConsumerTest.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java b/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java index 332a27d87..0c85a7cc5 100644 --- a/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java +++ b/src/test/java/dev/openfeature/sdk/internal/TriConsumerTest.java @@ -1,14 +1,11 @@ package dev.openfeature.sdk.internal; -import java.util.*; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; +import static org.junit.jupiter.api.Assertions.assertEquals; -import org.junit.jupiter.api.*; +import java.util.concurrent.atomic.AtomicInteger; -import static dev.openfeature.sdk.internal.ObjectUtils.defaultIfNull; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; class TriConsumerTest { From 8d78df71240fe075b0c32a9ffc4da7bc32e8c642 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 22 Jun 2023 16:02:05 -0400 Subject: [PATCH 05/18] fixup: unsed imports Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/ProviderRepository.java | 4 ++-- src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java | 1 - src/test/java/dev/openfeature/sdk/EventsTest.java | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 6bbb29c98..3ce1cafdd 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -107,7 +107,7 @@ private void initializeProvider(@Nullable String clientName, if (ProviderState.NOT_READY.equals(newProvider.getState())) { newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); } - emitReadyAndShutdownOld(clientName, newProvider, oldProvider, afterInit, afterShutdown); + emitReadyAndShutdownOld(newProvider, oldProvider, afterInit, afterShutdown); } catch (Exception e) { log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e); afterError.accept(newProvider, e.getMessage()); @@ -115,7 +115,7 @@ private void initializeProvider(@Nullable String clientName, }); } - private void emitReadyAndShutdownOld(@Nullable String clientName, FeatureProvider newProvider, + private void emitReadyAndShutdownOld(FeatureProvider newProvider, FeatureProvider oldProvider, Consumer afterInit, Consumer afterShutdown) { afterInit.accept(newProvider); diff --git a/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java b/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java index 08b81aa86..34caadaea 100644 --- a/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java +++ b/src/main/java/dev/openfeature/sdk/internal/ObjectUtils.java @@ -4,7 +4,6 @@ import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 6245a9364..a268edbec 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -23,7 +23,7 @@ class EventsTest { - private static final int TIMEOUT = 100; + private static final int TIMEOUT = 200; @AfterAll public static void resetDefaultProvider() { From f527a130f0b36a98c4e057162a01e9849bf920a9 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 22 Jun 2023 16:06:03 -0400 Subject: [PATCH 06/18] fixup: make inner static Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/EventSupport.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfeature/sdk/EventSupport.java b/src/main/java/dev/openfeature/sdk/EventSupport.java index 8658df2cc..b41b97aa5 100644 --- a/src/main/java/dev/openfeature/sdk/EventSupport.java +++ b/src/main/java/dev/openfeature/sdk/EventSupport.java @@ -89,7 +89,7 @@ private void runHandler(Consumer handler, EventDetails eventDetail }); } - class HandlerStore { + static class HandlerStore { private final Map>> handlerMap; From 934e136ec0df38b30b464f201021da94fd6d678a Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Thu, 22 Jun 2023 18:08:36 -0400 Subject: [PATCH 07/18] fixup: more tests, run ready immediately Signed-off-by: Todd Baert --- .../dev/openfeature/sdk/EventSupport.java | 2 +- .../dev/openfeature/sdk/OpenFeatureAPI.java | 6 +- .../openfeature/sdk/OpenFeatureClient.java | 22 +-- .../openfeature/sdk/ProviderRepository.java | 8 +- .../openfeature/sdk/EventProviderTest.java | 39 +++++ .../java/dev/openfeature/sdk/EventsTest.java | 154 ++++++------------ .../sdk/testutils/TestEventsProvider.java | 98 +++++++++++ 7 files changed, 200 insertions(+), 129 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/EventProviderTest.java create mode 100644 src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java diff --git a/src/main/java/dev/openfeature/sdk/EventSupport.java b/src/main/java/dev/openfeature/sdk/EventSupport.java index b41b97aa5..c0b84d82a 100644 --- a/src/main/java/dev/openfeature/sdk/EventSupport.java +++ b/src/main/java/dev/openfeature/sdk/EventSupport.java @@ -79,7 +79,7 @@ public Set getAllClientNames() { return this.handlerStores.keySet(); } - private void runHandler(Consumer handler, EventDetails eventDetails) { + public void runHandler(Consumer handler, EventDetails eventDetails) { taskExecutor.submit(() -> { try { handler.accept(eventDetails); diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 8eb0f6b74..3f347aa0a 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -70,7 +70,6 @@ public Client getClient(@Nullable String name) { */ public Client getClient(@Nullable String name, @Nullable String version) { return new OpenFeatureClient(this, - () -> this.providerRepository.getProvider(name).getState(), name, version); } @@ -256,6 +255,11 @@ void removeHandler(String clientName, ProviderEvent event, Consumer handler) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { + // if the provider is READY, run immediately + if (event.equals(ProviderEvent.PROVIDER_READY) + && ProviderState.READY.equals(this.providerRepository.getProvider(clientName).getState())) { + eventSupport.runHandler(handler, EventDetails.builder().clientName(clientName).build()); + } eventSupport.addClientHandler(clientName, event, handler); } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index bfeffaaf9..3516f4e7e 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -6,7 +6,6 @@ import java.util.List; import java.util.Map; import java.util.function.Consumer; -import java.util.function.Supplier; import dev.openfeature.sdk.exceptions.GeneralError; import dev.openfeature.sdk.exceptions.OpenFeatureError; @@ -34,39 +33,24 @@ public class OpenFeatureClient implements Client { AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock(); AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock(); private EvaluationContext evaluationContext; - private Supplier providerState; /** - * Deprecated constructor. Use OpenFeature.API.getClient() instead. + * Deprecated public constructor. Use OpenFeature.API.getClient() instead. * * @param openFeatureAPI Backing global singleton * @param name Name of the client (used by observability tools). * @param version Version of the client (used by observability tools). - * @deprecated Do not use this constructor it wil be removed. + * @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. */ - @Deprecated() + @Deprecated() // TODO: eventually we will make this non-public public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String name, String version) { this.openfeatureApi = openFeatureAPI; this.name = name; this.version = version; this.clientHooks = new ArrayList<>(); this.hookSupport = new HookSupport(); - log.warn( - "You've directly constructed a OpenFeatureClient. Use OpenFeature.API.getClient() instead."); - } - - OpenFeatureClient(OpenFeatureAPI openFeatureAPI, - final Supplier providerState, - String name, - String version) { - this.openfeatureApi = openFeatureAPI; - this.providerState = providerState; - this.name = name; - this.version = version; - this.clientHooks = new ArrayList<>(); - this.hookSupport = new HookSupport(); } /** diff --git a/src/main/java/dev/openfeature/sdk/ProviderRepository.java b/src/main/java/dev/openfeature/sdk/ProviderRepository.java index 3ce1cafdd..0ff3b70be 100644 --- a/src/main/java/dev/openfeature/sdk/ProviderRepository.java +++ b/src/main/java/dev/openfeature/sdk/ProviderRepository.java @@ -106,8 +106,9 @@ private void initializeProvider(@Nullable String clientName, try { if (ProviderState.NOT_READY.equals(newProvider.getState())) { newProvider.initialize(OpenFeatureAPI.getInstance().getEvaluationContext()); + afterInit.accept(newProvider); } - emitReadyAndShutdownOld(newProvider, oldProvider, afterInit, afterShutdown); + shutDownOld(oldProvider, afterShutdown); } catch (Exception e) { log.error("Exception when initializing feature provider {}", newProvider.getClass().getName(), e); afterError.accept(newProvider, e.getMessage()); @@ -115,10 +116,7 @@ private void initializeProvider(@Nullable String clientName, }); } - private void emitReadyAndShutdownOld(FeatureProvider newProvider, - FeatureProvider oldProvider, Consumer afterInit, - Consumer afterShutdown) { - afterInit.accept(newProvider); + private void shutDownOld(FeatureProvider oldProvider,Consumer afterShutdown) { if (!isProviderRegistered(oldProvider)) { shutdownProvider(oldProvider); afterShutdown.accept(oldProvider); diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java new file mode 100644 index 000000000..be599e616 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -0,0 +1,39 @@ +package dev.openfeature.sdk; + +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import dev.openfeature.sdk.internal.TriConsumer; +import dev.openfeature.sdk.testutils.TestEventsProvider; + +class EventProviderTest { + + @Nested + @DisplayName("if attached") + class IfAttached { + + @Test + @DisplayName("should run onEmit") + void shouldRunOnEmit() { + TriConsumer onEmit = mock(TriConsumer.class); + EventProvider provider = new TestEventsProvider(ProviderState.READY); + provider.attach(onEmit); + EventDetails details = EventDetails.builder().build(); + provider.emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); + provider.emitProviderReady(details); + provider.emitProviderConfigurationChanged(details); + provider.emitProviderStale(details); + provider.emitProviderError(details); + verify(onEmit, times(1)).accept(provider, ProviderEvent.PROVIDER_READY, details); + verify(onEmit, times(2)).accept(provider, ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); + verify(onEmit, times(1)).accept(provider, ProviderEvent.PROVIDER_STALE, details); + verify(onEmit, times(1)).accept(provider, ProviderEvent.PROVIDER_ERROR, details); + } + } +} \ 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 a268edbec..62b45f859 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -19,11 +19,13 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentMatcher; +import dev.openfeature.sdk.testutils.TestEventsProvider; import io.cucumber.java.AfterAll; class EventsTest { private static final int TIMEOUT = 200; + private static final int INIT_DELAY = TIMEOUT / 2; @AfterAll public static void resetDefaultProvider() { @@ -34,9 +36,11 @@ public static void resetDefaultProvider() { class ApiEvents { @Nested + @DisplayName("named provider") class NamedProvider { @Nested + @DisplayName("initialization") class Initialization { @Test @@ -47,10 +51,10 @@ void apiInitReady() { final Consumer handler = mock(Consumer.class); final String name = "apiInitReady"; - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().onProviderReady(handler); OpenFeatureAPI.getInstance().setProvider(name, provider); - verify(handler, timeout(TIMEOUT)) + verify(handler, timeout(TIMEOUT).atLeastOnce()) .accept(any()); } @@ -73,6 +77,7 @@ void apiInitError() { } @Nested + @DisplayName("provider events") class ProviderEvents { @Test @@ -84,7 +89,7 @@ void apiShouldPropagateEvents() { final Consumer handler = mock(Consumer.class); final String name = "apiShouldPropagateEvents"; - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); OpenFeatureAPI.getInstance().onProviderConfigurationChanged(handler); @@ -107,7 +112,7 @@ void apiShouldSupportAllEventTypes() throws Exception { final Consumer handler3 = mock(Consumer.class); final Consumer handler4 = mock(Consumer.class); - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); OpenFeatureAPI.getInstance().onProviderReady(handler1); @@ -129,12 +134,15 @@ void apiShouldSupportAllEventTypes() throws Exception { } @Nested + @DisplayName("client events") class ClientEvents { @Nested + @DisplayName("default provider") class DefaultProvider { @Nested + @DisplayName("provider events") class ProviderEvents { @Test @@ -143,7 +151,7 @@ class ProviderEvents { void shouldPropagateDefaultAndAnon() { final Consumer handler = mock(Consumer.class); - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client OpenFeatureAPI.getInstance().setProvider(provider); Client client = OpenFeatureAPI.getInstance().getClient(); @@ -160,7 +168,7 @@ void shouldPropagateDefaultAndNamed() { final Consumer handler = mock(Consumer.class); final String name = "shouldPropagateDefaultAndNamed"; - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client OpenFeatureAPI.getInstance().setProvider(provider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -174,9 +182,11 @@ void shouldPropagateDefaultAndNamed() { } @Nested + @DisplayName("named provider") class NamedProvider { @Nested + @DisplayName("initialization") class Initialization { @Test @DisplayName("should fire initial READY event when provider init succeeds after client retrieved") @@ -185,7 +195,7 @@ void initReadyProviderBefore() throws InterruptedException { final Consumer handler = mock(Consumer.class); final String name = "initReadyProviderBefore"; - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderReady(handler); // set provider after getting a client @@ -201,7 +211,7 @@ void initReadyProviderAfter() { final Consumer handler = mock(Consumer.class); final String name = "initReadyProviderAfter"; - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client OpenFeatureAPI.getInstance().setProvider(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -250,6 +260,7 @@ void initErrorProviderBefore() { } @Nested + @DisplayName("provider events") class ProviderEvents { @Test @@ -259,7 +270,7 @@ void shouldPropagateBefore() { final Consumer handler = mock(Consumer.class); final String name = "shouldPropagateBefore"; - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client OpenFeatureAPI.getInstance().setProvider(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -277,7 +288,7 @@ void shouldPropagateAfter() { final Consumer handler = mock(Consumer.class); final String name = "shouldPropagateAfter"; - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderConfigurationChanged(handler); // set provider after getting a client @@ -302,7 +313,7 @@ void shouldSupportAllEventTypes() throws Exception { final Consumer handler3 = mock(Consumer.class); final Consumer handler4 = mock(Consumer.class); - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -331,8 +342,8 @@ void shouldNotRunHandlers() throws Exception { final Consumer handler2 = mock(Consumer.class); final String name = "shouldNotRunHandlers"; - TestEventsProvider provider1 = new TestEventsProvider(); - TestEventsProvider provider2 = new TestEventsProvider(); + TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); + TestEventsProvider provider2 = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider1); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -364,8 +375,8 @@ void otherClientHandlersShouldNotRun() throws Exception { final Consumer handlerToRun = mock(Consumer.class); final Consumer handlerNotToRun = mock(Consumer.class); - TestEventsProvider provider1 = new TestEventsProvider(); - TestEventsProvider provider2 = new TestEventsProvider(); + TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); + TestEventsProvider provider2 = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name1, provider1); OpenFeatureAPI.getInstance().setProvider(name2, provider2); @@ -389,8 +400,8 @@ void boundShouldNotRunWithDefault() throws Exception { final String name = "boundShouldNotRunWithDefault"; final Consumer handlerNotToRun = mock(Consumer.class); - TestEventsProvider namedProvider = new TestEventsProvider(); - TestEventsProvider defaultProvider = new TestEventsProvider(); + TestEventsProvider namedProvider = new TestEventsProvider(INIT_DELAY); + TestEventsProvider defaultProvider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(defaultProvider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -415,7 +426,7 @@ void unboundShouldRunWithDefault() throws Exception { final String name = "unboundShouldRunWithDefault"; final Consumer handlerToRun = mock(Consumer.class); - TestEventsProvider defaultProvider = new TestEventsProvider(); + TestEventsProvider defaultProvider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(defaultProvider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -441,7 +452,7 @@ void handlersRunIfOneThrows() throws Exception { final Consumer nextHandler = mock(Consumer.class); final Consumer lastHandler = mock(Consumer.class); - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); Client client1 = OpenFeatureAPI.getInstance().getClient(name); @@ -465,7 +476,7 @@ void shouldHaveAllProperties() throws Exception { final Consumer handler2 = mock(Consumer.class); final String name = "shouldHaveAllProperties"; - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -501,6 +512,23 @@ void shouldHaveAllProperties() throws Exception { })); } + @Test + @DisplayName("if the provider is ready handlers must run immediately") + @Specification(number = "5.3.3", text = "PROVIDER_READY handlers attached after the provider is already in a ready state MUST run immediately.") + void readyMustRunImmediately() throws Exception { + final String name = "readyMustRunImmediately"; + final Consumer handler = mock(Consumer.class); + + // provider which is already ready + TestEventsProvider provider = new TestEventsProvider(ProviderState.READY); + OpenFeatureAPI.getInstance().setProvider(name, provider); + + // should run even thought handler was added after ready + Client client = OpenFeatureAPI.getInstance().getClient(name); + client.onProviderReady(handler); + verify(handler, timeout(TIMEOUT)).accept(any()); + } + @Test @DisplayName("must persist across changes") @Specification(number = "5.2.6", text = "Event handlers MUST persist across provider changes.") @@ -508,8 +536,8 @@ void mustPersistAcrossChanges() throws Exception { final String name = "mustPersistAcrossChanges"; final Consumer handler = mock(Consumer.class); - TestEventsProvider provider1 = new TestEventsProvider(); - TestEventsProvider provider2 = new TestEventsProvider(); + TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); + TestEventsProvider provider2 = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider1); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -539,7 +567,7 @@ void removedEventsShouldNotRun() { final Consumer handler1 = mock(Consumer.class); final Consumer handler2 = mock(Consumer.class); - TestEventsProvider provider = new TestEventsProvider(); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); @@ -563,84 +591,4 @@ void removedEventsShouldNotRun() { @Test void thisIsAProviderRequirement() { } - - class TestEventsProvider extends EventProvider implements FeatureProvider { - - private boolean initError = false; - private String initErrorMessage; - private ProviderState state = ProviderState.NOT_READY; - private boolean shutDown = false; - - @Override - public ProviderState getState() { - return this.state; - } - - TestEventsProvider() { - } - - TestEventsProvider(boolean initError, String initErrorMessage) { - this.initError = initError; - this.initErrorMessage = initErrorMessage; - } - - public void mockEvent(ProviderEvent event, ProviderEventDetails details) { - emit(event, details); - } - - public boolean isShutDown() { - return this.shutDown; - } - - @Override - public void shutdown() { - this.shutDown = true; - } - - @Override - public void initialize(EvaluationContext evaluationContext) throws Exception { - // wait half the TIMEOUT, otherwise some init/errors can be fired before we add handlers - Thread.sleep(TIMEOUT / 2); - if (this.initError) { - this.state = ProviderState.ERROR; - throw new Exception(initErrorMessage); - } - this.state = ProviderState.READY; - } - - @Override - public Metadata getMetadata() { - throw new UnsupportedOperationException("Unimplemented method 'getMetadata'"); - } - - @Override - public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getBooleanEvaluation'"); - } - - @Override - public ProviderEvaluation getStringEvaluation(String key, String defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getStringEvaluation'"); - } - - @Override - public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getIntegerEvaluation'"); - } - - @Override - public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getDoubleEvaluation'"); - } - - @Override - public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, - EvaluationContext ctx) { - throw new UnsupportedOperationException("Unimplemented method 'getObjectEvaluation'"); - } - }; } diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java new file mode 100644 index 000000000..3c56f0551 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -0,0 +1,98 @@ +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; + +public class TestEventsProvider extends EventProvider { + + private boolean initError = false; + private String initErrorMessage; + private ProviderState state = ProviderState.NOT_READY; + private boolean shutDown = false; + private int initTimeout = 0; + + @Override + public ProviderState getState() { + return this.state; + } + + public TestEventsProvider(int initTimeout) { + this.initTimeout = initTimeout; + } + + public TestEventsProvider(boolean initError, String initErrorMessage) { + this.initError = initError; + this.initErrorMessage = initErrorMessage; + } + + public TestEventsProvider(ProviderState initialState) { + this.state = initialState; + } + + public void mockEvent(ProviderEvent event, ProviderEventDetails details) { + emit(event, details); + } + + public boolean isShutDown() { + return this.shutDown; + } + + @Override + public void shutdown() { + this.shutDown = true; + } + + @Override + public void initialize(EvaluationContext evaluationContext) throws Exception { + if (ProviderState.NOT_READY.equals(state)) { + // wait half the TIMEOUT, otherwise some init/errors can be fired before we add handlers + Thread.sleep(initTimeout); + if (this.initError) { + this.state = ProviderState.ERROR; + throw new Exception(initErrorMessage); + } + this.state = ProviderState.READY; + } + } + + @Override + public Metadata getMetadata() { + throw new UnsupportedOperationException("Unimplemented method 'getMetadata'"); + } + + @Override + public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getBooleanEvaluation'"); + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getStringEvaluation'"); + } + + @Override + public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getIntegerEvaluation'"); + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getDoubleEvaluation'"); + } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, + EvaluationContext ctx) { + throw new UnsupportedOperationException("Unimplemented method 'getObjectEvaluation'"); + } +}; \ No newline at end of file From 92b8886be00e31fad29dd8ef46e7847526d30829 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 23 Jun 2023 12:55:47 -0400 Subject: [PATCH 08/18] fixup: improve reliability of error tests Signed-off-by: Todd Baert --- src/test/java/dev/openfeature/sdk/EventsTest.java | 6 +++--- .../dev/openfeature/sdk/testutils/TestEventsProvider.java | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 62b45f859..640d9ec80 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -67,7 +67,7 @@ void apiInitError() { final String name = "apiInitError"; final String errMessage = "oh no!"; - TestEventsProvider provider = new TestEventsProvider(true, errMessage); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY, true, errMessage); OpenFeatureAPI.getInstance().onProviderError(handler); OpenFeatureAPI.getInstance().setProvider(name, provider); verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { @@ -228,7 +228,7 @@ void initErrorProviderAfter() { final String name = "initErrorProviderAfter"; final String errMessage = "oh no!"; - TestEventsProvider provider = new TestEventsProvider(true, errMessage); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY, true, errMessage); Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderError(handler); // set provider after getting a client @@ -247,7 +247,7 @@ void initErrorProviderBefore() { final String name = "initErrorProviderBefore"; final String errMessage = "oh no!"; - TestEventsProvider provider = new TestEventsProvider(true, errMessage); + TestEventsProvider provider = new TestEventsProvider(INIT_DELAY, true, errMessage); // set provider after getting a client OpenFeatureAPI.getInstance().setProvider(name, provider); Client client = OpenFeatureAPI.getInstance().getClient(name); diff --git a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java index 3c56f0551..3fcb58886 100644 --- a/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java +++ b/src/test/java/dev/openfeature/sdk/testutils/TestEventsProvider.java @@ -26,7 +26,8 @@ public TestEventsProvider(int initTimeout) { this.initTimeout = initTimeout; } - public TestEventsProvider(boolean initError, String initErrorMessage) { + public TestEventsProvider(int initTimeout, boolean initError, String initErrorMessage) { + this.initTimeout = initTimeout; this.initError = initError; this.initErrorMessage = initErrorMessage; } From cb7ede278f2acbd293bc321632d940a4d91f5bb8 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 23 Jun 2023 13:43:10 -0400 Subject: [PATCH 09/18] Update src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java Co-authored-by: Giovanni Liva Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 3f347aa0a..4e6156ec4 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -256,7 +256,7 @@ void removeHandler(String clientName, ProviderEvent event, Consumer handler) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { // if the provider is READY, run immediately - if (event.equals(ProviderEvent.PROVIDER_READY) + if (ProviderEvent.PROVIDER_READY.equals(event) && ProviderState.READY.equals(this.providerRepository.getProvider(clientName).getState())) { eventSupport.runHandler(handler, EventDetails.builder().clientName(clientName).build()); } From 5491769d7e3d79bcc7cf86cfc52f4ae853c7cfee Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 23 Jun 2023 13:57:12 -0400 Subject: [PATCH 10/18] fixup: review feedback, add comments Signed-off-by: Todd Baert --- .../java/dev/openfeature/sdk/EventSupport.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventSupport.java b/src/main/java/dev/openfeature/sdk/EventSupport.java index c0b84d82a..f096539df 100644 --- a/src/main/java/dev/openfeature/sdk/EventSupport.java +++ b/src/main/java/dev/openfeature/sdk/EventSupport.java @@ -20,15 +20,18 @@ @Slf4j class EventSupport { + // we use a v4 uuid as a "placeholder" for anonymous clients, since + // ConcurrentHashMap doesn't support nulls private static final String defaultClientUuid = UUID.randomUUID().toString(); private static final ExecutorService taskExecutor = Executors.newCachedThreadPool(); private final Map handlerStores = new ConcurrentHashMap<>(); - private final HandlerStore globalHanderStore = new HandlerStore(); + private final HandlerStore globalHandlerStore = new HandlerStore(); public void runClientHandlers(@Nullable String clientName, ProviderEvent event, EventDetails eventDetails) { clientName = Optional.ofNullable(clientName) .orElse(defaultClientUuid); + // run handlers if they exist Optional.ofNullable(handlerStores.get(clientName)) .filter(store -> Optional.of(store).isPresent()) .map(store -> store.handlerMap.get(event)) @@ -37,7 +40,7 @@ public void runClientHandlers(@Nullable String clientName, ProviderEvent event, } public void runGlobalHandlers(ProviderEvent event, EventDetails eventDetails) { - globalHanderStore.handlerMap.get(event) + globalHandlerStore.handlerMap.get(event) .forEach(handler -> { runHandler(handler, eventDetails); }); @@ -47,17 +50,13 @@ public void addClientHandler(@Nullable String clientName, ProviderEvent event, C final String name = Optional.ofNullable(clientName) .orElse(defaultClientUuid); + // lazily create and cache a HandlerStore if it doesn't exist HandlerStore store = Optional.ofNullable(this.handlerStores.get(name)) .orElseGet(() -> { HandlerStore newStore = new HandlerStore(); this.handlerStores.put(name, newStore); return newStore; }); - if (this.handlerStores.get(name) == null) { - // TODO: test for not adding tons of handler stores - // Note we create spaces for client handlers lazily, not with every named client. - this.handlerStores.put(name, new HandlerStore()); - } store.addHandler(event, handler); } @@ -68,11 +67,11 @@ public void removeClientHandler(String clientName, ProviderEvent event, Consumer } public void addGlobalHandler(ProviderEvent event, Consumer handler) { - this.globalHanderStore.addHandler(event, handler); + this.globalHandlerStore.addHandler(event, handler); } public void removeGlobalHandler(ProviderEvent event, Consumer handler) { - this.globalHanderStore.removeHandler(event, handler); + this.globalHandlerStore.removeHandler(event, handler); } public Set getAllClientNames() { From 208e58b0572b97186dd90fab2c056a9979f501ac Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 26 Jun 2023 09:27:34 -0400 Subject: [PATCH 11/18] fixup: add provider repo tests, fitest warnings Signed-off-by: Todd Baert --- .../java/dev/openfeature/sdk/EventsTest.java | 81 ++++++++++--------- .../sdk/ProviderRepositoryTest.java | 74 +++++++++++++---- .../sdk/fixtures/ProviderFixture.java | 31 +++++-- 3 files changed, 127 insertions(+), 59 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java index 640d9ec80..70f81657e 100644 --- a/src/test/java/dev/openfeature/sdk/EventsTest.java +++ b/src/test/java/dev/openfeature/sdk/EventsTest.java @@ -48,7 +48,7 @@ class Initialization { @Specification(number = "5.3.1", text = "If the provider's initialize function terminates normally," + " PROVIDER_READY handlers MUST run.") void apiInitReady() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = (Consumer)mockHandler(); final String name = "apiInitReady"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -63,7 +63,7 @@ void apiInitReady() { @Specification(number = "5.3.2", text = "If the provider's initialize function terminates abnormally," + " PROVIDER_ERROR handlers MUST run.") void apiInitError() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "apiInitError"; final String errMessage = "oh no!"; @@ -86,7 +86,7 @@ class ProviderEvents { + "the associated client and API event handlers MUST run.") void apiShouldPropagateEvents() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "apiShouldPropagateEvents"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -107,10 +107,10 @@ void apiShouldPropagateEvents() { " with a particular provider event type.") void apiShouldSupportAllEventTypes() throws Exception { final String name = "apiShouldSupportAllEventTypes"; - final Consumer handler1 = mock(Consumer.class); - final Consumer handler2 = mock(Consumer.class); - final Consumer handler3 = mock(Consumer.class); - final Consumer handler4 = mock(Consumer.class); + final Consumer handler1 = mockHandler(); + final Consumer handler2 = mockHandler(); + final Consumer handler3 = mockHandler(); + final Consumer handler4 = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); @@ -149,7 +149,7 @@ class ProviderEvents { @DisplayName("should propagate events for default provider and anonymous client") @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, the associated client and API event handlers MUST run.") void shouldPropagateDefaultAndAnon() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); // set provider before getting a client @@ -165,7 +165,7 @@ void shouldPropagateDefaultAndAnon() { @DisplayName("should propagate events for default provider and named client") @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, the associated client and API event handlers MUST run.") void shouldPropagateDefaultAndNamed() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "shouldPropagateDefaultAndNamed"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -192,7 +192,7 @@ class Initialization { @DisplayName("should fire initial READY event when provider init succeeds after client retrieved") @Specification(number = "5.3.1", text = "If the provider's initialize function terminates normally, PROVIDER_READY handlers MUST run.") void initReadyProviderBefore() throws InterruptedException { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "initReadyProviderBefore"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -208,7 +208,7 @@ void initReadyProviderBefore() throws InterruptedException { @DisplayName("should fire initial READY event when provider init succeeds before client retrieved") @Specification(number = "5.3.1", text = "If the provider's initialize function terminates normally, PROVIDER_READY handlers MUST run.") void initReadyProviderAfter() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "initReadyProviderAfter"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -224,7 +224,7 @@ void initReadyProviderAfter() { @DisplayName("should fire initial ERROR event when provider init errors after client retrieved") @Specification(number = "5.3.2", text = "If the provider's initialize function terminates abnormally, PROVIDER_ERROR handlers MUST run.") void initErrorProviderAfter() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "initErrorProviderAfter"; final String errMessage = "oh no!"; @@ -234,8 +234,8 @@ void initErrorProviderAfter() { // set provider after getting a client OpenFeatureAPI.getInstance().setProvider(name, provider); verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { - return details.getClientName().equals(name) - && details.getMessage().equals(errMessage); + return name.equals(details.getClientName()) + && errMessage.equals(details.getMessage()); })); } @@ -243,7 +243,7 @@ void initErrorProviderAfter() { @DisplayName("should fire initial ERROR event when provider init errors before client retrieved") @Specification(number = "5.3.2", text = "If the provider's initialize function terminates abnormally, PROVIDER_ERROR handlers MUST run.") void initErrorProviderBefore() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "initErrorProviderBefore"; final String errMessage = "oh no!"; @@ -253,8 +253,8 @@ void initErrorProviderBefore() { Client client = OpenFeatureAPI.getInstance().getClient(name); client.onProviderError(handler); verify(handler, timeout(TIMEOUT)).accept(argThat(details -> { - return details.getClientName().equals(name) - && details.getMessage().equals(errMessage); + return name.equals(details.getClientName()) + && errMessage.equals(details.getMessage()); })); } } @@ -267,7 +267,7 @@ class ProviderEvents { @DisplayName("should propagate events when provider set before client retrieved") @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, the associated client and API event handlers MUST run.") void shouldPropagateBefore() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "shouldPropagateBefore"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -285,7 +285,7 @@ void shouldPropagateBefore() { @Specification(number = "5.1.2", text = "When a provider signals the occurrence of a particular event, the associated client and API event handlers MUST run.") void shouldPropagateAfter() { - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); final String name = "shouldPropagateAfter"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -308,10 +308,10 @@ void shouldPropagateAfter() { " with a particular provider event type.") void shouldSupportAllEventTypes() throws Exception { final String name = "shouldSupportAllEventTypes"; - final Consumer handler1 = mock(Consumer.class); - final Consumer handler2 = mock(Consumer.class); - final Consumer handler3 = mock(Consumer.class); - final Consumer handler4 = mock(Consumer.class); + final Consumer handler1 = mockHandler(); + final Consumer handler2 = mockHandler(); + final Consumer handler3 = mockHandler(); + final Consumer handler4 = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); @@ -338,8 +338,8 @@ void shouldSupportAllEventTypes() throws Exception { @Test @DisplayName("shutdown provider should not run handlers") void shouldNotRunHandlers() throws Exception { - final Consumer handler1 = mock(Consumer.class); - final Consumer handler2 = mock(Consumer.class); + final Consumer handler1 = mockHandler(); + final Consumer handler2 = mockHandler(); final String name = "shouldNotRunHandlers"; TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); @@ -372,8 +372,8 @@ void shouldNotRunHandlers() throws Exception { void otherClientHandlersShouldNotRun() throws Exception { final String name1 = "otherClientHandlersShouldNotRun1"; final String name2 = "otherClientHandlersShouldNotRun2"; - final Consumer handlerToRun = mock(Consumer.class); - final Consumer handlerNotToRun = mock(Consumer.class); + final Consumer handlerToRun = mockHandler(); + final Consumer handlerNotToRun = mockHandler(); TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); TestEventsProvider provider2 = new TestEventsProvider(INIT_DELAY); @@ -398,7 +398,7 @@ void otherClientHandlersShouldNotRun() throws Exception { "event handlers on clients which are not associated with that provider MUST NOT run.") void boundShouldNotRunWithDefault() throws Exception { final String name = "boundShouldNotRunWithDefault"; - final Consumer handlerNotToRun = mock(Consumer.class); + final Consumer handlerNotToRun = mockHandler(); TestEventsProvider namedProvider = new TestEventsProvider(INIT_DELAY); TestEventsProvider defaultProvider = new TestEventsProvider(INIT_DELAY); @@ -424,7 +424,7 @@ void boundShouldNotRunWithDefault() throws Exception { "event handlers on clients which are not associated with that provider MUST NOT run.") void unboundShouldRunWithDefault() throws Exception { final String name = "unboundShouldRunWithDefault"; - final Consumer handlerToRun = mock(Consumer.class); + final Consumer handlerToRun = mockHandler(); TestEventsProvider defaultProvider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(defaultProvider); @@ -447,10 +447,10 @@ void unboundShouldRunWithDefault() throws Exception { @Specification(number = "5.2.5", text = "If a handler function terminates abnormally, other handler functions MUST run.") void handlersRunIfOneThrows() throws Exception { final String name = "handlersRunIfOneThrows"; - final Consumer errorHandler = mock(Consumer.class); + final Consumer errorHandler = mockHandler(); doThrow(new NullPointerException()).when(errorHandler).accept(any()); - final Consumer nextHandler = mock(Consumer.class); - final Consumer lastHandler = mock(Consumer.class); + final Consumer nextHandler = mockHandler(); + final Consumer lastHandler = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); @@ -472,8 +472,8 @@ void handlersRunIfOneThrows() throws Exception { @Specification(number = "5.2.4", text = "The handler function MUST accept a event details parameter.") @Specification(number = "5.2.3", text = "The event details MUST contain the client name associated with the event.") void shouldHaveAllProperties() throws Exception { - final Consumer handler1 = mock(Consumer.class); - final Consumer handler2 = mock(Consumer.class); + final Consumer handler1 = mockHandler(); + final Consumer handler2 = mockHandler(); final String name = "shouldHaveAllProperties"; TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); @@ -517,7 +517,7 @@ void shouldHaveAllProperties() throws Exception { @Specification(number = "5.3.3", text = "PROVIDER_READY handlers attached after the provider is already in a ready state MUST run immediately.") void readyMustRunImmediately() throws Exception { final String name = "readyMustRunImmediately"; - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); // provider which is already ready TestEventsProvider provider = new TestEventsProvider(ProviderState.READY); @@ -534,7 +534,7 @@ void readyMustRunImmediately() throws Exception { @Specification(number = "5.2.6", text = "Event handlers MUST persist across provider changes.") void mustPersistAcrossChanges() throws Exception { final String name = "mustPersistAcrossChanges"; - final Consumer handler = mock(Consumer.class); + final Consumer handler = mockHandler(); TestEventsProvider provider1 = new TestEventsProvider(INIT_DELAY); TestEventsProvider provider2 = new TestEventsProvider(INIT_DELAY); @@ -564,8 +564,8 @@ class HandlerRemoval { @DisplayName("should not run removed events") void removedEventsShouldNotRun() { final String name = "removedEventsShouldNotRun"; - final Consumer handler1 = mock(Consumer.class); - final Consumer handler2 = mock(Consumer.class); + final Consumer handler1 = mockHandler(); + final Consumer handler2 = mockHandler(); TestEventsProvider provider = new TestEventsProvider(INIT_DELAY); OpenFeatureAPI.getInstance().setProvider(name, provider); @@ -591,4 +591,9 @@ void removedEventsShouldNotRun() { @Test void thisIsAProviderRequirement() { } + + @SuppressWarnings("unchecked") + private static Consumer mockHandler() { + return mock(Consumer.class); + } } diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index 4c1faa310..644112316 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -1,13 +1,16 @@ 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.Mockito.doReturn; +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; @@ -31,7 +34,6 @@ class ProviderRepositoryTest { private static final String CLIENT_NAME = "client name"; private static final String ANOTHER_CLIENT_NAME = "another client name"; - private static final String FEATURE_KEY = "some key"; private static final int TIMEOUT = 5000; private final ExecutorService executorService = Executors.newCachedThreadPool(); @@ -85,14 +87,10 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { @Test @DisplayName("should avoid additional initialization call if provider has been initialized already") void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() throws Exception { - FeatureProvider provider = createMockedProvider(); - setFeatureProvider(CLIENT_NAME, provider); - - doReturn(ProviderState.READY).when(provider).getState(); - + FeatureProvider provider = createMockedReadyProvider(); setFeatureProvider(provider); - - verify(provider).initialize(any()); + + verify(provider, never()).initialize(any()); } } @@ -137,14 +135,10 @@ void shouldImmediatelyReturnWhenCallingTheNamedClientProviderMutator() throws Ex @Test @DisplayName("should avoid additional initialization call if provider has been initialized already") void shouldAvoidAdditionalInitializationCallIfProviderHasBeenInitializedAlready() throws Exception { - FeatureProvider provider = createMockedProvider(); - setFeatureProvider(provider); - - doReturn(ProviderState.READY).when(provider).getState(); - + FeatureProvider provider = createMockedReadyProvider(); setFeatureProvider(CLIENT_NAME, provider); - verify(provider).initialize(any()); + verify(provider, never()).initialize(any()); } } } @@ -248,6 +242,47 @@ void shouldNotThrowExceptionIfProviderThrowsOneOnShutdown() { verify(provider).shutdown(); } } + + @Nested + class LifecyleLambdas { + @Test + @DisplayName("should run afterSet, afterInit, afterShutdown on successful set/init") + @SuppressWarnings("unchecked") + void shouldRunLambdasOnSuccessful() { + Consumer afterSet = mock(Consumer.class); + 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).accept(featureProvider1); + verify(afterInit).accept(featureProvider1); + verify(afterShutdown).accept(oldProvider); + verify(afterError, never()).accept(any(), any()); + } + + @Test + @DisplayName("should run afterSet, afterError on unsuccessful set/init") + @SuppressWarnings("unchecked") + void shouldRunLambdasOnError() throws Exception { + Consumer afterSet = mock(Consumer.class); + 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).accept(errorFeatureProvider); + verify(afterInit, never()).accept(any());; + verify(afterError, timeout(TIMEOUT)).accept(eq(errorFeatureProvider), any()); + } + } } @Test @@ -280,6 +315,15 @@ private void setFeatureProvider(FeatureProvider provider) { waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider); } + + private void setFeatureProvider(FeatureProvider provider, Consumer afterSet, + Consumer afterInit, Consumer afterShutdown, + BiConsumer afterError) { + providerRepository.setProvider(provider, afterSet, afterInit, afterShutdown, + afterError); + waitForSettingProviderHasBeenCompleted(ProviderRepository::getProvider, provider); + } + private void setFeatureProvider(String namedProvider, FeatureProvider provider) { providerRepository.setProvider(namedProvider, provider, mockAfterSet(), mockAfterInit(), mockAfterShutdown(), mockAfterError()); diff --git a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java index ce14457bf..c00b8ff27 100644 --- a/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java +++ b/src/test/java/dev/openfeature/sdk/fixtures/ProviderFixture.java @@ -1,15 +1,21 @@ package dev.openfeature.sdk.fixtures; +import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doBlock; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; + +import java.io.FileNotFoundException; +import java.util.concurrent.CountDownLatch; + +import org.mockito.stubbing.Answer; + import dev.openfeature.sdk.FeatureProvider; import dev.openfeature.sdk.ImmutableContext; import dev.openfeature.sdk.ProviderState; import lombok.experimental.UtilityClass; -import org.mockito.stubbing.Answer; - -import java.util.concurrent.CountDownLatch; - -import static dev.openfeature.sdk.testutils.stubbing.ConditionStubber.doBlock; -import static org.mockito.Mockito.*; @UtilityClass public class ProviderFixture { @@ -20,6 +26,19 @@ public static FeatureProvider createMockedProvider() { return provider; } + public static FeatureProvider createMockedReadyProvider() { + FeatureProvider provider = mock(FeatureProvider.class); + doReturn(ProviderState.READY).when(provider).getState(); + return provider; + } + + public static FeatureProvider createMockedErrorProvider() throws Exception { + FeatureProvider provider = mock(FeatureProvider.class); + doReturn(ProviderState.NOT_READY).when(provider).getState(); + doThrow(FileNotFoundException.class).when(provider).initialize(any()); + return provider; + } + public static FeatureProvider createBlockedProvider(CountDownLatch latch, Runnable onAnswer) throws Exception { FeatureProvider provider = createMockedProvider(); doBlock(latch, createAnswerExecutingCode(onAnswer)).when(provider).initialize(new ImmutableContext()); From 5eee2c23375ff7529ac7bfcb1ab2c2607471e753 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 27 Jun 2023 14:41:20 -0400 Subject: [PATCH 12/18] Update src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java Co-authored-by: Kavindu Dodanduwa Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 4e6156ec4..e38bb4bec 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -116,10 +116,10 @@ public void setProvider(String clientName, FeatureProvider provider) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { providerRepository.setProvider(clientName, provider, - (p) -> attachEventProvider(p), - (p) -> emitReady(p), - (p) -> detachEventProvider(p), - (p, message) -> emitError(p, message)); + this::attachEventProvider, + this::emitReady, + this::detachEventProvider, + this::emitError) } } From e232c0048f7db36735b1f04ae15cf18911ff4d95 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 27 Jun 2023 14:57:04 -0400 Subject: [PATCH 13/18] fixup: shorten javadoc links, shutdown tasks, use methods refs Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/EventProvider.java | 10 +++++----- src/main/java/dev/openfeature/sdk/EventSupport.java | 4 ++++ src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java | 9 +++++---- .../dev/openfeature/sdk/ShutdownBehaviorSpecTest.java | 2 +- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index 6fbc41d8f..b285c49f7 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -29,7 +29,7 @@ void attach(TriConsumer onEm } /** - * Emit the specified {@link dev.openfeature.sdk.ProviderEvent}. + * Emit the specified {@link ProviderEvent}. * * @param event The event type * @param details The details of the event @@ -41,7 +41,7 @@ public void emit(ProviderEvent event, ProviderEventDetails details) { } /** - * Emit a {@link dev.openfeature.sdk.ProviderEvent#PROVIDER_READY} event. + * Emit a {@link ProviderEvent#PROVIDER_READY} event. * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} * * @param details The details of the event @@ -52,7 +52,7 @@ public void emitProviderReady(ProviderEventDetails details) { /** * Emit a - * {@link dev.openfeature.sdk.ProviderEvent#PROVIDER_CONFIGURATION_CHANGED} + * {@link ProviderEvent#PROVIDER_CONFIGURATION_CHANGED} * event. Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} * * @param details The details of the event @@ -62,7 +62,7 @@ public void emitProviderConfigurationChanged(ProviderEventDetails details) { } /** - * Emit a {@link dev.openfeature.sdk.ProviderEvent#PROVIDER_STALE} event. + * Emit a {@link ProviderEvent#PROVIDER_STALE} event. * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} * * @param details The details of the event @@ -72,7 +72,7 @@ public void emitProviderStale(ProviderEventDetails details) { } /** - * Emit a {@link dev.openfeature.sdk.ProviderEvent#PROVIDER_ERROR} event. + * Emit a {@link ProviderEvent#PROVIDER_ERROR} event. * Shorthand for {@link #emit(ProviderEvent, ProviderEventDetails)} * * @param details The details of the event diff --git a/src/main/java/dev/openfeature/sdk/EventSupport.java b/src/main/java/dev/openfeature/sdk/EventSupport.java index f096539df..f2f8f2960 100644 --- a/src/main/java/dev/openfeature/sdk/EventSupport.java +++ b/src/main/java/dev/openfeature/sdk/EventSupport.java @@ -88,6 +88,10 @@ public void runHandler(Consumer handler, EventDetails eventDetails }); } + public void shutdown() { + taskExecutor.shutdown(); + } + static class HandlerStore { private final Map>> handlerMap; diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index e38bb4bec..1bd2cff59 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -24,7 +24,7 @@ public class OpenFeatureAPI implements EventHandling { private EvaluationContext evaluationContext; private final List apiHooks; private ProviderRepository providerRepository = new ProviderRepository(); - private final EventSupport eventSupport = new EventSupport(); + private EventSupport eventSupport = new EventSupport(); protected OpenFeatureAPI() { apiHooks = new ArrayList<>(); @@ -119,7 +119,7 @@ public void setProvider(String clientName, FeatureProvider provider) { this::attachEventProvider, this::emitReady, this::detachEventProvider, - this::emitError) + this::emitError); } } @@ -192,7 +192,7 @@ public void clearHooks() { public void shutdown() { providerRepository.shutdown(); - // TODO: shutdown events + eventSupport.shutdown(); } /** @@ -268,8 +268,9 @@ void addHandler(String clientName, ProviderEvent event, Consumer h * This method is only here for testing as otherwise all tests after the API * shutdown test would fail. */ - final void resetProviderRepository() { + final void reset() { providerRepository = new ProviderRepository(); + eventSupport = new EventSupport(); } /** diff --git a/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java b/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java index e62c82e31..e470819f7 100644 --- a/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/ShutdownBehaviorSpecTest.java @@ -110,7 +110,7 @@ void mustShutdownAllProvidersOnShuttingDownApi() { verify(namedProvider).shutdown(); }); - api.resetProviderRepository(); + api.reset(); } } } From 9a628ae1a48efce4bf84362530eafac89faccd89 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 10 Jul 2023 10:01:40 -0400 Subject: [PATCH 14/18] fixup: flaky test Signed-off-by: Todd Baert --- src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index 644112316..d1ffccbc9 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -305,8 +305,8 @@ void shouldShutdownAllFeatureProvidersOnShutdown() { assertThat(providerRepository.getProvider(CLIENT_NAME)).isInstanceOf(NoOpProvider.class); assertThat(providerRepository.getProvider(ANOTHER_CLIENT_NAME)).isInstanceOf(NoOpProvider.class); }); - verify(featureProvider1).shutdown(); - verify(featureProvider2).shutdown(); + verify(featureProvider1, timeout(TIMEOUT)).shutdown(); + verify(featureProvider2, timeout(TIMEOUT)).shutdown(); } private void setFeatureProvider(FeatureProvider provider) { From 9e23fb86ea46484cff6f52a4f2f4e02765f52dad Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 11 Jul 2023 10:54:12 -0400 Subject: [PATCH 15/18] Update src/main/java/dev/openfeature/sdk/OpenFeatureClient.java Co-authored-by: Justin Abrahms Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/OpenFeatureClient.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 3516f4e7e..05d79d02d 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -29,7 +29,6 @@ public class OpenFeatureClient implements Client { private final String version; private final List clientHooks; private final HookSupport hookSupport; - // private final EventEmitter emitter = new EventEmitter(); AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock(); AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock(); private EvaluationContext evaluationContext; From 5568e8757f1d1ceb6bc99bead7d7a8ce394ad49c Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 11 Jul 2023 12:40:54 -0400 Subject: [PATCH 16/18] fixup: feedback from justin Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/Client.java | 2 +- .../sdk/{EventHandling.java => EventBus.java} | 2 +- .../dev/openfeature/sdk/EventProvider.java | 17 ++- .../dev/openfeature/sdk/EventSupport.java | 59 ++++++++ .../dev/openfeature/sdk/OpenFeatureAPI.java | 2 +- .../openfeature/sdk/EventProviderTest.java | 137 +++++++++++++++--- 6 files changed, 188 insertions(+), 31 deletions(-) rename src/main/java/dev/openfeature/sdk/{EventHandling.java => EventBus.java} (98%) diff --git a/src/main/java/dev/openfeature/sdk/Client.java b/src/main/java/dev/openfeature/sdk/Client.java index c5f3d110e..ebca0b131 100644 --- a/src/main/java/dev/openfeature/sdk/Client.java +++ b/src/main/java/dev/openfeature/sdk/Client.java @@ -5,7 +5,7 @@ /** * Interface used to resolve flags of varying types. */ -public interface Client extends Features, EventHandling { +public interface Client extends Features, EventBus { Metadata getMetadata(); /** diff --git a/src/main/java/dev/openfeature/sdk/EventHandling.java b/src/main/java/dev/openfeature/sdk/EventBus.java similarity index 98% rename from src/main/java/dev/openfeature/sdk/EventHandling.java rename to src/main/java/dev/openfeature/sdk/EventBus.java index 31a622708..d635e9bac 100644 --- a/src/main/java/dev/openfeature/sdk/EventHandling.java +++ b/src/main/java/dev/openfeature/sdk/EventBus.java @@ -5,7 +5,7 @@ /** * Interface for attaching event handlers. */ -public interface EventHandling { +public interface EventBus { /** * Add a handler for the {@link ProviderEvent#PROVIDER_READY} event. diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index b285c49f7..b979e1546 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -18,16 +18,23 @@ public abstract class EventProvider implements FeatureProvider { private TriConsumer onEmit = null; - void detach() { - this.onEmit = null; - } - + // package private method to "attach" this EventProvider to an SDK, which allows + // events to propagate from this provider void attach(TriConsumer onEmit) { - if (this.onEmit == null) { + if (this.onEmit != null && this.onEmit != onEmit) { + // if we are trying to attach this provider to a different onEmit, something has gone wrong + throw new IllegalStateException("Provider " + this.getMetadata().getName() + " is already attached."); + } else { this.onEmit = onEmit; } } + // package private method to "detach" this EventProvider from an SDK, stopping + // event propagation + void detach() { + this.onEmit = null; + } + /** * Emit the specified {@link ProviderEvent}. * diff --git a/src/main/java/dev/openfeature/sdk/EventSupport.java b/src/main/java/dev/openfeature/sdk/EventSupport.java index f2f8f2960..6558f9694 100644 --- a/src/main/java/dev/openfeature/sdk/EventSupport.java +++ b/src/main/java/dev/openfeature/sdk/EventSupport.java @@ -27,6 +27,14 @@ class EventSupport { private final Map handlerStores = new ConcurrentHashMap<>(); private final HandlerStore globalHandlerStore = new HandlerStore(); + /** + * Run all the event handlers associated with this client name. + * If the client name is null, handlers attached to unnamed clients will run. + * + * @param clientName the client name to run event handlers for, or null + * @param event the event type + * @param eventDetails the event details + */ public void runClientHandlers(@Nullable String clientName, ProviderEvent event, EventDetails eventDetails) { clientName = Optional.ofNullable(clientName) .orElse(defaultClientUuid); @@ -39,6 +47,12 @@ public void runClientHandlers(@Nullable String clientName, ProviderEvent event, .forEach(handler -> runHandler(handler, eventDetails))); } + /** + * Run all the API (global) event handlers. + * + * @param event the event type + * @param eventDetails the event details + */ public void runGlobalHandlers(ProviderEvent event, EventDetails eventDetails) { globalHandlerStore.handlerMap.get(event) .forEach(handler -> { @@ -46,6 +60,14 @@ public void runGlobalHandlers(ProviderEvent event, EventDetails eventDetails) { }); } + /** + * Add a handler for the specified client name, or all unnamed clients. + * + * @param clientName the client name to add handlers for, or else the unnamed + * client + * @param event the event type + * @param handler the handler function to run + */ public void addClientHandler(@Nullable String clientName, ProviderEvent event, Consumer handler) { final String name = Optional.ofNullable(clientName) .orElse(defaultClientUuid); @@ -60,24 +82,55 @@ public void addClientHandler(@Nullable String clientName, ProviderEvent event, C store.addHandler(event, handler); } + /** + * Remove a client event handler for the specified event type. + * + * @param clientName the name of the client handler to remove, or null to remove + * from unnamed clients + * @param event the event type + * @param handler the handler ref to be removed + */ public void removeClientHandler(String clientName, ProviderEvent event, Consumer handler) { clientName = Optional.ofNullable(clientName) .orElse(defaultClientUuid); this.handlerStores.get(clientName).removeHandler(event, handler); } + /** + * Add a global event handler of the specified event type. + * + * @param event the event type + * @param handler the handler to be added + */ public void addGlobalHandler(ProviderEvent event, Consumer handler) { this.globalHandlerStore.addHandler(event, handler); } + /** + * Remove a global event handler for the specified event type. + * + * @param event the event type + * @param handler the handler ref to be removed + */ public void removeGlobalHandler(ProviderEvent event, Consumer handler) { this.globalHandlerStore.removeHandler(event, handler); } + /** + * Get all client names for which we have event handlers registered. + * + * @return set of client names + */ public Set getAllClientNames() { return this.handlerStores.keySet(); } + /** + * Run the passed handler on the taskExecutor. + * + * @param handler the handler to run + * @param eventDetails the event details + */ public void runHandler(Consumer handler, EventDetails eventDetails) { taskExecutor.submit(() -> { try { @@ -88,10 +141,16 @@ public void runHandler(Consumer handler, EventDetails eventDetails }); } + /** + * Stop the event handler task executor. + */ public void shutdown() { taskExecutor.shutdown(); } + // Handler store maintains a set of handlers for each event type. + // Each client in the SDK gets it's own handler store, which is lazily + // instantiated when a handler is added to that client. static class HandlerStore { private final Map>> handlerMap; diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 1bd2cff59..42ff4708c 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -18,7 +18,7 @@ * Configuration here will be shared across all {@link Client}s. */ @Slf4j -public class OpenFeatureAPI implements EventHandling { +public class OpenFeatureAPI implements EventBus { // package-private multi-read/single-write lock static AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock(); private EvaluationContext evaluationContext; diff --git a/src/test/java/dev/openfeature/sdk/EventProviderTest.java b/src/test/java/dev/openfeature/sdk/EventProviderTest.java index be599e616..cb73b5292 100644 --- a/src/test/java/dev/openfeature/sdk/EventProviderTest.java +++ b/src/test/java/dev/openfeature/sdk/EventProviderTest.java @@ -1,39 +1,130 @@ package dev.openfeature.sdk; -import static org.mockito.ArgumentMatchers.argThat; +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 org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import dev.openfeature.sdk.internal.TriConsumer; -import dev.openfeature.sdk.testutils.TestEventsProvider; class EventProviderTest { - @Nested - @DisplayName("if attached") - class IfAttached { - - @Test - @DisplayName("should run onEmit") - void shouldRunOnEmit() { - TriConsumer onEmit = mock(TriConsumer.class); - EventProvider provider = new TestEventsProvider(ProviderState.READY); - provider.attach(onEmit); - EventDetails details = EventDetails.builder().build(); - provider.emit(ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); - provider.emitProviderReady(details); - provider.emitProviderConfigurationChanged(details); - provider.emitProviderStale(details); - provider.emitProviderError(details); - verify(onEmit, times(1)).accept(provider, ProviderEvent.PROVIDER_READY, details); - verify(onEmit, times(2)).accept(provider, ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); - verify(onEmit, times(1)).accept(provider, ProviderEvent.PROVIDER_STALE, details); - verify(onEmit, times(1)).accept(provider, ProviderEvent.PROVIDER_ERROR, details); + @Test + @DisplayName("should run attached onEmit with emitters") + void emitsEventsWhenAttached() { + TestEventProvider eventProvider = new TestEventProvider(); + TriConsumer onEmit = mockOnEmit(); + eventProvider.attach(onEmit); + + ProviderEventDetails details = ProviderEventDetails.builder().build(); + eventProvider.emit(ProviderEvent.PROVIDER_READY, details); + eventProvider.emitProviderReady(details); + eventProvider.emitProviderConfigurationChanged(details); + eventProvider.emitProviderStale(details); + eventProvider.emitProviderError(details); + + verify(onEmit, times(2)).accept(eventProvider, ProviderEvent.PROVIDER_READY, details); + verify(onEmit, times(1)).accept(eventProvider, ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, details); + verify(onEmit, times(1)).accept(eventProvider, ProviderEvent.PROVIDER_STALE, details); + verify(onEmit, times(1)).accept(eventProvider, ProviderEvent.PROVIDER_ERROR, details); + } + + @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(); + + ProviderEventDetails details = ProviderEventDetails.builder().build(); + eventProvider.emit(ProviderEvent.PROVIDER_READY, details); + eventProvider.emitProviderReady(details); + eventProvider.emitProviderConfigurationChanged(details); + eventProvider.emitProviderStale(details); + eventProvider.emitProviderError(details); + + // should not be called + verify(onEmit, never()).accept(any(), any(), any()); + } + + @Test + @DisplayName("should throw if second different onEmit attached") + void throwsWhenOnEmitDifferent() { + TestEventProvider eventProvider = new TestEventProvider(); + TriConsumer onEmit1 = mockOnEmit(); + TriConsumer onEmit2 = mockOnEmit(); + eventProvider.attach(onEmit1); + assertThrows(IllegalStateException.class, () -> eventProvider.attach(onEmit2)); + } + + + @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 + } + + + class TestEventProvider extends EventProvider { + + @Override + public Metadata getMetadata() { + return new Metadata() { + @Override + public String getName() { + return "TestEventProvider"; + } + }; + } + + @Override + public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, + EvaluationContext ctx) { + // TODO Auto-generated method stub + throw new UnsupportedOperationException("Unimplemented method 'getBooleanEvaluation'"); + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, + EvaluationContext ctx) { + // TODO Auto-generated method stub + throw new UnsupportedOperationException("Unimplemented method 'getStringEvaluation'"); + } + + @Override + public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, + EvaluationContext ctx) { + // TODO Auto-generated method stub + throw new UnsupportedOperationException("Unimplemented method 'getIntegerEvaluation'"); + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, + EvaluationContext ctx) { + // TODO Auto-generated method stub + throw new UnsupportedOperationException("Unimplemented method 'getDoubleEvaluation'"); } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, + EvaluationContext ctx) { + // TODO Auto-generated method stub + throw new UnsupportedOperationException("Unimplemented method 'getObjectEvaluation'"); + } + } + + @SuppressWarnings("unchecked") + private TriConsumer mockOnEmit() { + return (TriConsumer)mock(TriConsumer.class); } } \ No newline at end of file From 447327779bcb3205be93f0fb8c50d3eceff938bc Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Tue, 11 Jul 2023 12:45:48 -0400 Subject: [PATCH 17/18] fixup: improve javadoc Signed-off-by: Todd Baert --- .../java/dev/openfeature/sdk/EventProvider.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/EventProvider.java b/src/main/java/dev/openfeature/sdk/EventProvider.java index b979e1546..de12b0777 100644 --- a/src/main/java/dev/openfeature/sdk/EventProvider.java +++ b/src/main/java/dev/openfeature/sdk/EventProvider.java @@ -18,8 +18,12 @@ public abstract class EventProvider implements FeatureProvider { private TriConsumer onEmit = null; - // package private method to "attach" this EventProvider to an SDK, which allows - // events to propagate from this provider + /** + * "Attach" this EventProvider to an SDK, which allows events to propagate from this provider. + * No-op if the same onEmit is already attached. + * + * @param onEmit the function to run when a provider emits events. + */ void attach(TriConsumer onEmit) { if (this.onEmit != null && this.onEmit != onEmit) { // if we are trying to attach this provider to a different onEmit, something has gone wrong @@ -29,8 +33,9 @@ void attach(TriConsumer onEm } } - // package private method to "detach" this EventProvider from an SDK, stopping - // event propagation + /** + * "Detach" this EventProvider from an SDK, stopping propagation of all events. + */ void detach() { this.onEmit = null; } From 7eebd0114c24a5711458bec092b5d1a9e4892fe5 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 12 Jul 2023 09:55:04 -0400 Subject: [PATCH 18/18] fixup: test race condition fixes Signed-off-by: Todd Baert --- .../openfeature/sdk/ProviderRepositoryTest.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java index d1ffccbc9..5b6dac1b5 100644 --- a/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java +++ b/src/test/java/dev/openfeature/sdk/ProviderRepositoryTest.java @@ -81,7 +81,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { return true; }); - verify(featureProvider).initialize(any()); + verify(featureProvider, timeout(TIMEOUT)).initialize(any()); } @Test @@ -166,7 +166,7 @@ void shouldImmediatelyReturnWhenCallingTheProviderMutator() throws Exception { return true; }); - verify(newProvider).initialize(any()); + verify(newProvider, timeout(TIMEOUT)).initialize(any()); } @Test @@ -239,7 +239,7 @@ void shouldNotThrowExceptionIfProviderThrowsOneOnShutdown() { assertThatCode(() -> setFeatureProvider(new NoOpProvider())).doesNotThrowAnyException(); - verify(provider).shutdown(); + verify(provider, timeout(TIMEOUT)).shutdown(); } } @@ -260,9 +260,9 @@ void shouldRunLambdasOnSuccessful() { setFeatureProvider(featureProvider1, afterSet, afterInit, afterShutdown, afterError); setFeatureProvider(featureProvider2); - verify(afterSet).accept(featureProvider1); - verify(afterInit).accept(featureProvider1); - verify(afterShutdown).accept(oldProvider); + verify(afterSet, timeout(TIMEOUT)).accept(featureProvider1); + verify(afterInit, timeout(TIMEOUT)).accept(featureProvider1); + verify(afterShutdown, timeout(TIMEOUT)).accept(oldProvider); verify(afterError, never()).accept(any(), any()); } @@ -278,7 +278,7 @@ void shouldRunLambdasOnError() throws Exception { FeatureProvider errorFeatureProvider = createMockedErrorProvider(); setFeatureProvider(errorFeatureProvider, afterSet, afterInit, afterShutdown, afterError); - verify(afterSet).accept(errorFeatureProvider); + verify(afterSet, timeout(TIMEOUT)).accept(errorFeatureProvider); verify(afterInit, never()).accept(any());; verify(afterError, timeout(TIMEOUT)).accept(eq(errorFeatureProvider), any()); }