feat: Cache hooks per Flag Evaluation Type

src/main/java/dev/openfeature/sdk/FeatureProvider.java

@@ -15,6 +15,23 @@
         return new ArrayList<>();
     }
 
+    /**
+     * Returns all hooks that support the given flag value type.
+     *
+     * @param flagType the flag value type to support
+     * @return a list of hooks that support the given flag value type
+     */
+    default List<Hook> getProviderHooks(FlagValueType flagType) {
+        var allHooks = getProviderHooks();
+        var filteredHooks = new ArrayList<Hook>(allHooks.size());
+        for (Hook hook : allHooks) {
+            if (hook.supportsFlagValueType(flagType)) {
+                filteredHooks.add(hook);
+            }
+        }
+        return filteredHooks;
+    }
+
     ProviderEvaluation<Boolean> getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx);
 
     ProviderEvaluation<String> getStringEvaluation(String key, String defaultValue, EvaluationContext ctx);

src/main/java/dev/openfeature/sdk/FlagEvaluationOptions.java

@@ -1,5 +1,7 @@
 package dev.openfeature.sdk;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -15,4 +17,18 @@
 
     @Builder.Default
     Map<String, Object> hookHints = new HashMap<>();
+
+    List<Hook> getHooks(FlagValueType flagValueType) {
+        if (hooks == null || hooks.isEmpty()) {
+            return Collections.emptyList();
+        }
+
+        var result = new ArrayList<Hook>(hooks.size());
+        for (var hook : hooks) {
+            if (hook.supportsFlagValueType(flagValueType)) {
+                result.add(hook);
+            }
+        }
+        return result;
+    }
 }

src/main/java/dev/openfeature/sdk/HookSupport.java

@@ -1,9 +1,12 @@
 package dev.openfeature.sdk;
 
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import lombok.extern.slf4j.Slf4j;
 
 /**
@@ -17,26 +20,59 @@
      * Sets the {@link Hook}-{@link HookContext}-{@link Pair} list in the given data object with {@link HookContext}
      * set to null. Filters hooks by supported {@link FlagValueType}.
      *
-     * @param hookSupportData   the data object to modify
-     * @param hooks             the hooks to set
-     * @param type              the flag value type to filter unsupported hooks
+     * @param hookSupportData  the data object to modify
+     * @param providerHooks    the hooks filtered for the proper flag value type from the respective layer
+     * @param flagOptionsHooks the hooks filtered for the proper flag value type from the respective layer
+     * @param clientHooks      the hooks filtered for the proper flag value type from the respective layer
+     * @param apiHooks         the hooks filtered for the proper flag value type from the respective layer
      */
-    public void setHooks(HookSupportData hookSupportData, List<Hook> hooks, FlagValueType type) {
-        List<Pair<Hook, HookContext>> hookContextPairs = new ArrayList<>();
-        for (Hook hook : hooks) {
-            if (hook.supportsFlagValueType(type)) {
-                hookContextPairs.add(Pair.of(hook, null));
-            }
+    public void setHooks(
+            HookSupportData hookSupportData,
+            List<Hook> providerHooks,
+            List<Hook> flagOptionsHooks,
+            ConcurrentLinkedQueue<Hook> clientHooks,
+            ConcurrentLinkedQueue<Hook> apiHooks) {
+        var lengthEstimate = 0;
+
+        if (providerHooks != null) {
+            lengthEstimate += providerHooks.size();
+        }
+        if (flagOptionsHooks != null) {
+            lengthEstimate += flagOptionsHooks.size();
+        }
+        if (clientHooks != null) {
+            lengthEstimate += clientHooks.size();
+        }
+        if (apiHooks != null) {
+            lengthEstimate += apiHooks.size();
         }
+
+        ArrayList<Pair<Hook, HookContext>> hookContextPairs = new ArrayList<>(lengthEstimate);
+
+        addAll(hookContextPairs, providerHooks);
+        addAll(hookContextPairs, flagOptionsHooks);
+        addAll(hookContextPairs, clientHooks);
+        addAll(hookContextPairs, apiHooks);
+
         hookSupportData.hooks = hookContextPairs;
     }
 
+    private void addAll(List<Pair<Hook, HookContext>> accumulator, Collection<Hook> toAdd) {
+        if (toAdd == null || toAdd.isEmpty()) {
+            return;
+        }
+
+        for (Hook hook : toAdd) {
+            accumulator.add(Pair.of(hook, null));
+        }
+    }
+
     /**
      * Creates & sets a {@link HookContext} for every {@link Hook}-{@link HookContext}-{@link Pair}
      * in the given data object with a new {@link HookData} instance.
      *
-     * @param hookSupportData   the data object to modify
-     * @param sharedContext     the shared context from which the new {@link HookContext} is created
+     * @param hookSupportData the data object to modify
+     * @param sharedContext   the shared context from which the new {@link HookContext} is created
      */
     public void setHookContexts(HookSupportData hookSupportData, SharedHookContext sharedContext) {
         for (int i = 0; i < hookSupportData.hooks.size(); i++) {
@@ -66,10 +102,9 @@
 
     public void executeBeforeHooks(HookSupportData data) {
         // These traverse backwards from normal.
-        List<Pair<Hook, HookContext>> reversedHooks = new ArrayList<>(data.getHooks());
-        Collections.reverse(reversedHooks);
-
-        for (Pair<Hook, HookContext> hookContextPair : reversedHooks) {
+        var hooks = data.getHooks();
+        for (int i = hooks.size() - 1; i >= 0; i--) {
+            var hookContextPair = hooks.get(i);
             var hook = hookContextPair.getKey();
             var hookContext = hookContextPair.getValue();
 
@@ -123,4 +158,26 @@
             }
         }
     }
+
+    static void addHooks(Map<FlagValueType, ConcurrentLinkedQueue<Hook>> hookMap, Hook... hooksToAdd) {
+        var types = FlagValueType.values();
+        for (int i = 0; i < hooksToAdd.length; i++) {
+            var current = hooksToAdd[i];
+            for (int j = 0; j < types.length; j++) {
+                var type = types[j];
+                if (current.supportsFlagValueType(type)) {
+                    hookMap.get(type).add(current);
+                }
+            }
+        }
+    }
+
+    static ArrayList<Hook> getAllUniqueHooks(Map<FlagValueType, ConcurrentLinkedQueue<Hook>> hookMap) {
+        // Hooks can be duplicated if they support multiple FlagValueTypes
+        var allHooks = new HashSet<Hook>();
+        for (var queue : hookMap.values()) {
+            allHooks.addAll(queue);
+        }
+        return new ArrayList<>(allHooks);
+    }
 }

src/main/java/dev/openfeature/sdk/HookSupportData.java

@@ -1,6 +1,6 @@
 package dev.openfeature.sdk;
 
-import java.util.List;
+import java.util.ArrayList;
 import java.util.Map;
 import lombok.Getter;
 
@@ -10,7 +10,7 @@
 @Getter
 class HookSupportData {
 
-    List<Pair<Hook, HookContext>> hooks;
+    ArrayList<Pair<Hook, HookContext>> hooks;
     EvaluationContext evaluationContext;
     Map<String, Object> hints;
 

src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java

@@ -3,12 +3,10 @@
 import dev.openfeature.sdk.exceptions.OpenFeatureError;
 import dev.openfeature.sdk.internal.AutoCloseableLock;
 import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
@@ -24,14 +22,18 @@
 public class OpenFeatureAPI implements EventBus<OpenFeatureAPI> {
     // package-private multi-read/single-write lock
     static AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock();
-    private final ConcurrentLinkedQueue<Hook> apiHooks;
+    private final ConcurrentHashMap<FlagValueType, ConcurrentLinkedQueue<Hook>> apiHooks;
     private ProviderRepository providerRepository;
     private EventSupport eventSupport;
     private final AtomicReference<EvaluationContext> evaluationContext = new AtomicReference<>();
     private TransactionContextPropagator transactionContextPropagator;
 
     protected OpenFeatureAPI() {
-        apiHooks = new ConcurrentLinkedQueue<>();
+        var values = FlagValueType.values();
+        apiHooks = new ConcurrentHashMap<>(values.length);
+        for (FlagValueType value : values) {
+            apiHooks.put(value, new ConcurrentLinkedQueue<>());
+        }
         providerRepository = new ProviderRepository(this);
         eventSupport = new EventSupport();
         transactionContextPropagator = new NoOpTransactionContextPropagator();
@@ -304,7 +306,7 @@
      * @param hooks The hook to add.
      */
     public void addHooks(Hook... hooks) {
-        this.apiHooks.addAll(Arrays.asList(hooks));
+        HookSupport.addHooks(apiHooks, hooks);
     }
 
     /**
@@ -313,16 +315,16 @@
      * @return A list of {@link Hook}s.
      */
     public List<Hook> getHooks() {
-        return new ArrayList<>(this.apiHooks);
+        return HookSupport.getAllUniqueHooks(apiHooks);
     }
 
     /**
-     * Returns a reference to the collection of {@link Hook}s.
+     * Fetch the hooks associated to this client, that support the given FlagValueType.
      *
-     * @return The collection of {@link Hook}s.
+     * @return A list of {@link Hook}s.
      */
-    Collection<Hook> getMutableHooks() {
-        return this.apiHooks;
+    ConcurrentLinkedQueue<Hook> getHooks(FlagValueType type) {
+        return apiHooks.get(type);
     }
 
     /**

src/main/java/dev/openfeature/sdk/OpenFeatureClient.java

@@ -7,13 +7,12 @@
 import dev.openfeature.sdk.exceptions.ProviderNotReadyError;
 import dev.openfeature.sdk.internal.ObjectUtils;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
@@ -47,7 +46,7 @@ public class OpenFeatureClient implements Client {
     @Getter
     private final String version;
 
-    private final ConcurrentLinkedQueue<Hook> clientHooks;
+    private final ConcurrentHashMap<FlagValueType, ConcurrentLinkedQueue<Hook>> clientHooks;
     private final AtomicReference<EvaluationContext> evaluationContext = new AtomicReference<>();
 
     private final HookSupport hookSupport;
@@ -69,7 +68,11 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String ve
         this.domain = domain;
         this.version = version;
         this.hookSupport = new HookSupport();
-        this.clientHooks = new ConcurrentLinkedQueue<>();
+        var values = FlagValueType.values();
+        this.clientHooks = new ConcurrentHashMap<>(values.length);
+        for (FlagValueType value : values) {
+            this.clientHooks.put(value, new ConcurrentLinkedQueue<>());
+        }
     }
 
     /**
@@ -125,7 +128,7 @@ public void track(String trackingEventName, EvaluationContext context, TrackingE
      */
     @Override
     public OpenFeatureClient addHooks(Hook... hooks) {
-        this.clientHooks.addAll(Arrays.asList(hooks));
+        HookSupport.addHooks(clientHooks, hooks);
         return this;
     }
 
@@ -134,7 +137,7 @@ public OpenFeatureClient addHooks(Hook... hooks) {
      */
     @Override
     public List<Hook> getHooks() {
-        return new ArrayList<>(this.clientHooks);
+        return HookSupport.getAllUniqueHooks(clientHooks);
     }
 
     /**
@@ -174,9 +177,12 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
             final var state = stateManager.getState();
 
             // Hooks are initialized as early as possible to enable the execution of error stages
-            var mergedHooks = ObjectUtils.merge(
-                    provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks());
-            hookSupport.setHooks(hookSupportData, mergedHooks, type);
+            hookSupport.setHooks(
+                    hookSupportData,
+                    provider.getProviderHooks(type),
+                    flagOptions.getHooks(type),
+                    clientHooks.get(type),
+                    openfeatureApi.getHooks(type));
 
             var sharedHookContext =
                     new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue);

src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java

@@ -3,6 +3,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -14,6 +15,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.atomic.AtomicBoolean;
 import lombok.SneakyThrows;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -101,9 +103,10 @@ void brokenProvider() {
     void providerLockedPerTransaction() {
 
         final String defaultValue = "string-value";
-        final OpenFeatureAPI api = new OpenFeatureAPI();
-        var provider1 = TestProvider.builder().initsToReady();
-        var provider2 = TestProvider.builder().initsToReady();
+        final OpenFeatureAPI testApi = new OpenFeatureAPI();
+        final var provider1 = TestProvider.builder().initsToReady();
+        final var provider2 = TestProvider.builder().initsToReady();
+        final var wasHookCalled = new AtomicBoolean(false);
 
         class MutatingHook implements Hook {
 
@@ -112,24 +115,27 @@ class MutatingHook implements Hook {
             // change the provider during a before hook - this should not impact the evaluation in progress
             public Optional before(HookContext ctx, Map hints) {
 
-                api.setProviderAndWait(provider2);
-
+                testApi.setProviderAndWait(provider2);
+                wasHookCalled.set(true);
                 return Optional.empty();
             }
         }
 
-        final Client client = api.getClient();
-        api.setProviderAndWait(provider1);
-        api.addHooks(new MutatingHook());
+        final Client client = testApi.getClient();
+        testApi.setProviderAndWait(provider1);
+        testApi.addHooks(new MutatingHook());
 
         // if provider is changed during an evaluation transaction it should proceed with the original provider
         client.getStringValue("val", defaultValue);
         assertEquals(1, provider1.getFlagEvaluations().size());
+        assertEquals(0, provider2.getFlagEvaluations().size());
+        assertTrue(wasHookCalled.get());
 
-        api.clearHooks();
+        testApi.clearHooks();
 
         // subsequent evaluations should now use new provider set by hook
         client.getStringValue("val", defaultValue);
+        assertEquals(1, provider1.getFlagEvaluations().size());
         assertEquals(1, provider2.getFlagEvaluations().size());
     }