feat: implement spec 0.7.0 changes (#655)

* feat: implement spec 0.7.0 changes * run any event handler immediately if the provider is in the associated state, not just ready * add providerName to event details * add STALE provider state * update/add associated tests * also fixed spec/test associations mismatches from previous changes Signed-off-by: Todd Baert <[email protected]> Co-authored-by: Thiyagu GK <[email protected]>
open-feature · Oct 23, 2023 · fe5a20f · fe5a20f
1 parent fb4d369
commit fe5a20f
Show file tree

Hide file tree

Showing 16 changed files with 254 additions and 91 deletions.
diff --git a/README.md b/README.md
@@ -13,8 +13,8 @@
 <!-- x-hide-in-docs-end -->
 <!-- The 'github-badges' class is used in the docs -->
 <p align="center" class="github-badges">
-  <a href="https://github.com/open-feature/spec/tree/v0.6.0">
-    <img alt="Specification" src="https://img.shields.io/static/v1?label=specification&message=v0.6.0&color=yellow&style=for-the-badge" />
+  <a href="https://github.com/open-feature/spec/tree/v0.7.0">
+    <img alt="Specification" src="https://img.shields.io/static/v1?label=specification&message=v0.7.0&color=yellow&style=for-the-badge" />
   </a>
   <!-- x-release-please-start-version -->
 

diff --git a/src/main/java/dev/openfeature/sdk/EventDetails.java b/src/main/java/dev/openfeature/sdk/EventDetails.java
@@ -7,19 +7,23 @@
 /**
  * The details of a particular event.
  */
-@Data @SuperBuilder(toBuilder = true)
+@Data
+@SuperBuilder(toBuilder = true)
 public class EventDetails extends ProviderEventDetails {
     private String clientName;
+    private String providerName;
 
-    static EventDetails fromProviderEventDetails(ProviderEventDetails providerEventDetails) {
-        return EventDetails.fromProviderEventDetails(providerEventDetails, null);
+    static EventDetails fromProviderEventDetails(ProviderEventDetails providerEventDetails, String providerName) {
+        return EventDetails.fromProviderEventDetails(providerEventDetails, providerName, null);
     }
 
     static EventDetails fromProviderEventDetails(
             ProviderEventDetails providerEventDetails,
+            @Nullable String providerName,
             @Nullable String clientName) {
         return EventDetails.builder()
                 .clientName(clientName)
+                .providerName(providerName)
                 .flagsChanged(providerEventDetails.getFlagsChanged())
                 .eventMetadata(providerEventDetails.getEventMetadata())
                 .message(providerEventDetails.getMessage())

diff --git a/src/main/java/dev/openfeature/sdk/FlagEvaluationDetails.java b/src/main/java/dev/openfeature/sdk/FlagEvaluationDetails.java
@@ -1,5 +1,7 @@
 package dev.openfeature.sdk;
 
+import java.util.Optional;
+
 import javax.annotation.Nullable;
 
 import lombok.AllArgsConstructor;
@@ -8,7 +10,8 @@
 import lombok.NoArgsConstructor;
 
 /**
- * Contains information about how the provider resolved a flag, including the resolved value.
+ * Contains information about how the provider resolved a flag, including the
+ * resolved value.
  *
  * @param <T> the type of the flag being evaluated.
  */
@@ -20,11 +23,15 @@ public class FlagEvaluationDetails<T> implements BaseEvaluation<T> {
 
     private String flagKey;
     private T value;
-    @Nullable private String variant;
-    @Nullable private String reason;
+    @Nullable
+    private String variant;
+    @Nullable
+    private String reason;
     private ErrorCode errorCode;
-    @Nullable private String errorMessage;
-    @Builder.Default private ImmutableMetadata flagMetadata = ImmutableMetadata.builder().build();
+    @Nullable
+    private String errorMessage;
+    @Builder.Default
+    private ImmutableMetadata flagMetadata = ImmutableMetadata.builder().build();
 
     /**
      * Generate detail payload from the provider response.
@@ -42,7 +49,8 @@ public static <T> FlagEvaluationDetails<T> from(ProviderEvaluation<T> providerEv
                 .reason(providerEval.getReason())
                 .errorMessage(providerEval.getErrorMessage())
                 .errorCode(providerEval.getErrorCode())
-                .flagMetadata(providerEval.getFlagMetadata())
+                .flagMetadata(
+                        Optional.ofNullable(providerEval.getFlagMetadata()).orElse(ImmutableMetadata.builder().build()))
                 .build();
     }
 }
diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
@@ -3,6 +3,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
 
@@ -100,12 +101,12 @@ public EvaluationContext getEvaluationContext() {
     public void setProvider(FeatureProvider provider) {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.setProvider(
-                provider,
+                    provider,
                     this::attachEventProvider,
                     this::emitReady,
                     this::detachEventProvider,
                     this::emitError,
-                false);
+                    false);
         }
     }
 
@@ -118,12 +119,12 @@ public void setProvider(FeatureProvider provider) {
     public void setProvider(String clientName, FeatureProvider provider) {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.setProvider(clientName,
-                provider,
-                this::attachEventProvider,
-                this::emitReady,
-                this::detachEventProvider,
-                this::emitError,
-                false);
+                    provider,
+                    this::attachEventProvider,
+                    this::emitReady,
+                    this::detachEventProvider,
+                    this::emitError,
+                    false);
         }
     }
 
@@ -133,12 +134,12 @@ public void setProvider(String clientName, FeatureProvider provider) {
     public void setProviderAndWait(FeatureProvider provider) {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.setProvider(
-                provider,
-                this::attachEventProvider,
-                this::emitReady,
-                this::detachEventProvider,
-                this::emitError,
-                true);
+                    provider,
+                    this::attachEventProvider,
+                    this::emitReady,
+                    this::detachEventProvider,
+                    this::emitError,
+                    true);
         }
     }
 
@@ -151,18 +152,18 @@ public void setProviderAndWait(FeatureProvider provider) {
     public void setProviderAndWait(String clientName, FeatureProvider provider) {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.setProvider(clientName,
-                provider,
-                this::attachEventProvider,
-                this::emitReady,
-                this::detachEventProvider,
-                this::emitError,
-                true);
+                    provider,
+                    this::attachEventProvider,
+                    this::emitReady,
+                    this::detachEventProvider,
+                    this::emitError,
+                    true);
         }
     }
 
     private void attachEventProvider(FeatureProvider provider) {
         if (provider instanceof EventProvider) {
-            ((EventProvider)provider).attach((p, event, details) -> {
+            ((EventProvider) provider).attach((p, event, details) -> {
                 runHandlersForProvider(p, event, details);
             });
         }
@@ -174,7 +175,7 @@ private void emitReady(FeatureProvider provider) {
 
     private void detachEventProvider(FeatureProvider provider) {
         if (provider instanceof EventProvider) {
-            ((EventProvider)provider).detach();
+            ((EventProvider) provider).detach();
         }
     }
 
@@ -229,9 +230,10 @@ public void clearHooks() {
 
     /**
      * Shut down and reset the current status of OpenFeature API.
-     * This call cleans up all active providers and attempts to shut down internal event handling mechanisms.
+     * This call cleans up all active providers and attempts to shut down internal
+     * event handling mechanisms.
      * Once shut down is complete, API is reset and ready to use again.
-     * */
+     */
     public void shutdown() {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
             providerRepository.shutdown();
@@ -302,9 +304,9 @@ void removeHandler(String clientName, ProviderEvent event, Consumer<EventDetails
 
     void addHandler(String clientName, ProviderEvent event, Consumer<EventDetails> handler) {
         try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) {
-            // if the provider is READY, run immediately
-            if (ProviderEvent.PROVIDER_READY.equals(event)
-                    && ProviderState.READY.equals(this.providerRepository.getProvider(clientName).getState())) {
+            // if the provider is in the state associated with event, run immediately
+            if (Optional.ofNullable(this.providerRepository.getProvider(clientName).getState())
+                    .orElse(ProviderState.READY).matchesEvent(event)) {
                 eventSupport.runHandler(handler, EventDetails.builder().clientName(clientName).build());
             }
             eventSupport.addClientHandler(clientName, event, handler);
@@ -315,30 +317,36 @@ void addHandler(String clientName, ProviderEvent event, Consumer<EventDetails> h
      * 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
+     * @param event    the event type
+     * @param details  the event details
      */
     private void runHandlersForProvider(FeatureProvider provider, ProviderEvent event, ProviderEventDetails details) {
         try (AutoCloseableLock __ = lock.readLockAutoCloseable()) {
-    
+
             List<String> clientNamesForProvider = providerRepository
-                .getClientNamesForProvider(provider);
-
+                    .getClientNamesForProvider(provider);
+
+            final String providerName = Optional.ofNullable(provider.getMetadata())
+                    .map(metadata -> metadata.getName())
+                    .orElse(null);
+
             // run the global handlers
-            eventSupport.runGlobalHandlers(event, EventDetails.fromProviderEventDetails(details));
+            eventSupport.runGlobalHandlers(event, EventDetails.fromProviderEventDetails(details, providerName));
 
             // run the handlers associated with named clients for this provider
-            clientNamesForProvider.forEach(name -> {   
-                eventSupport.runClientHandlers(name, event, EventDetails.fromProviderEventDetails(details, name));
+            clientNamesForProvider.forEach(name -> {
+                eventSupport.runClientHandlers(name, event,
+                        EventDetails.fromProviderEventDetails(details, providerName, name));
             });
-    
+
             if (providerRepository.isDefaultProvider(provider)) {
                 // run handlers for clients that have no bound providers (since this is the default)
                 Set<String> allClientNames = eventSupport.getAllClientNames();
                 Set<String> boundClientNames = providerRepository.getAllBoundClientNames();
                 allClientNames.removeAll(boundClientNames);
                 allClientNames.forEach(name -> {
-                    eventSupport.runClientHandlers(name, event, EventDetails.fromProviderEventDetails(details, name));
+                    eventSupport.runClientHandlers(name, event,
+                            EventDetails.fromProviderEventDetails(details, providerName, name));
                 });
             }
         }

diff --git a/src/main/java/dev/openfeature/sdk/ProviderState.java b/src/main/java/dev/openfeature/sdk/ProviderState.java
@@ -4,5 +4,17 @@
  * Indicates the state of the provider.
  */
 public enum ProviderState {
-    READY, NOT_READY, ERROR;
+    READY, NOT_READY, ERROR, STALE;
+
+    /**
+     * Returns true if the passed ProviderEvent maps to this ProviderState.
+     * 
+     * @param event event to compare
+     * @return boolean if matches.
+     */
+    boolean matchesEvent(ProviderEvent event) {
+        return this == READY && event == ProviderEvent.PROVIDER_READY
+                || this == STALE && event == ProviderEvent.PROVIDER_STALE
+                || this == ERROR && event == ProviderEvent.PROVIDER_ERROR;
+    }
 }
diff --git a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java
@@ -4,10 +4,19 @@ class DoSomethingProvider implements FeatureProvider {
 
     static final String name = "Something";
     // Flag evaluation metadata
-    static final ImmutableMetadata flagMetadata = ImmutableMetadata.builder().build();
+    static final ImmutableMetadata DEFAULT_METADATA = ImmutableMetadata.builder().build();
+    private ImmutableMetadata flagMetadata;
 
     private EvaluationContext savedContext;
 
+    public DoSomethingProvider() {
+        this.flagMetadata = DEFAULT_METADATA;
+    }
+
+    public DoSomethingProvider(ImmutableMetadata flagMetadata) {
+        this.flagMetadata = flagMetadata;
+    }
+
     EvaluationContext getMergedContext() {
         return savedContext;
     }

diff --git a/src/test/java/dev/openfeature/sdk/EventsTest.java b/src/test/java/dev/openfeature/sdk/EventsTest.java
@@ -24,7 +24,7 @@
 
 class EventsTest {
 
-    private static final int TIMEOUT = 200;
+    private static final int TIMEOUT = 300;
     private static final int INIT_DELAY = TIMEOUT / 2;
 
     @AfterAll
@@ -470,7 +470,7 @@ void handlersRunIfOneThrows() throws Exception {
     @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.")
+    @Specification(number = "5.2.3", text = "The `event details` MUST contain the `provider name` associated with the event.")
     void shouldHaveAllProperties() throws Exception {
         final Consumer<EventDetails> handler1 = mockHandler();
         final Consumer<EventDetails> handler2 = mockHandler();
@@ -514,9 +514,9 @@ 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";
+    @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.")
+    void matchingReadyEventsMustRunImmediately() throws Exception {
+        final String name = "matchingEventsMustRunImmediately";
         final Consumer<EventDetails> handler = mockHandler();
 
         // provider which is already ready
@@ -529,6 +529,40 @@ void readyMustRunImmediately() throws Exception {
         verify(handler, timeout(TIMEOUT)).accept(any());
     }
 
+    @Test
+    @DisplayName("if the provider is ready handlers must run immediately")
+    @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.")
+    void matchingStaleEventsMustRunImmediately() throws Exception {
+        final String name = "matchingEventsMustRunImmediately";
+        final Consumer<EventDetails> handler = mockHandler();
+
+        // provider which is already stale
+        TestEventsProvider provider = new TestEventsProvider(ProviderState.STALE);
+        OpenFeatureAPI.getInstance().setProvider(name, provider);
+
+        // should run even thought handler was added after stale
+        Client client = OpenFeatureAPI.getInstance().getClient(name);
+        client.onProviderStale(handler);
+        verify(handler, timeout(TIMEOUT)).accept(any());
+    }
+
+    @Test
+    @DisplayName("if the provider is ready handlers must run immediately")
+    @Specification(number = "5.3.3", text = "Handlers attached after the provider is already in the associated state, MUST run immediately.")
+    void matchingErrorEventsMustRunImmediately() throws Exception {
+        final String name = "matchingEventsMustRunImmediately";
+        final Consumer<EventDetails> handler = mockHandler();
+
+        // provider which is already in error
+        TestEventsProvider provider = new TestEventsProvider(ProviderState.ERROR);
+        OpenFeatureAPI.getInstance().setProvider(name, provider);
+
+        // should run even thought handler was added after error
+        Client client = OpenFeatureAPI.getInstance().getClient(name);
+        client.onProviderError(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.")
@@ -560,6 +594,7 @@ void mustPersistAcrossChanges() throws Exception {
 
     @Nested
     class HandlerRemoval {
+        @Specification(number="5.2.7", text="The API and client MUST provide a function allowing the removal of event handlers.")
         @Test
         @DisplayName("should not run removed events")
         void removedEventsShouldNotRun() {