cache hooks

Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com>
open-feature · chrfwow · Sep 15, 2025 · Sep 15, 2025 · Oct 15, 2025 · Oct 15, 2025
commit b5a443f04e07b4b6efd37a894f5cf5cb61c65c20
@@ -15,6 +15,12 @@ default List<Hook> getProviderHooks() {
         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());

@@ -2,7 +2,6 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentLinkedQueue;
@@ -19,22 +18,34 @@ class HookSupport {
      * 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> providerHooks,
             List<Hook> flagOptionsHooks,
             ConcurrentLinkedQueue<Hook> clientHooks,
-            ConcurrentLinkedQueue<Hook> apiHooks
-    ) {
-        var lengthEstimate = providerHooks.size()
-                + flagOptionsHooks.size()
-                + clientHooks.size()
-                + apiHooks.size();
-        List<Pair<Hook, HookContext>> hookContextPairs = new ArrayList<>(lengthEstimate);
+            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);
@@ -45,7 +56,7 @@ public void setHooks(
     }
 
     private void addAll(List<Pair<Hook, HookContext>> accumulator, Collection<Hook> toAdd) {
-        if(toAdd.isEmpty()){
+        if (toAdd == null || toAdd.isEmpty()) {
             return;
         }
 
@@ -89,10 +100,9 @@ public void updateEvaluationContext(HookSupportData hookSupportData, EvaluationC
 
     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();
 

@@ -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;
 

@@ -4,8 +4,7 @@
 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.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -314,7 +313,7 @@ public void addHooks(Hook... hooks) {
             var current = hooks[i];
             for (int j = 0; j < types.length; j++) {
                 var type = types[j];
-                if(current.supportsFlagValueType(type)) {
+                if (current.supportsFlagValueType(type)) {
                     this.apiHooks.get(type).add(current);
                 }
             }
@@ -327,15 +326,16 @@ public void addHooks(Hook... hooks) {
      * @return A list of {@link Hook}s.
      */
     public List<Hook> getHooks() {
-        var allHooks = new ArrayList<Hook>();
+        // Hooks can be duplicated if they support multiple FlagValueTypes
+        var allHooks = new HashSet<Hook>();
         for (var queue : this.apiHooks.values()) {
             allHooks.addAll(queue);
         }
-        return allHooks;
+        return new ArrayList<>(allHooks);
     }
 
     /**
-     * Fetch the hooks associated to this client.
+     * Fetch the hooks associated to this client, that support the given FlagValueType.
      *
      * @return A list of {@link Hook}s.
      */

@@ -8,9 +8,9 @@
 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.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -31,11 +31,11 @@
  */
 @Slf4j
 @SuppressWarnings({
-        "PMD.DataflowAnomalyAnalysis",
-        "PMD.BeanMembersShouldSerialize",
-        "PMD.UnusedLocalVariable",
-        "unchecked",
-        "rawtypes"
+    "PMD.DataflowAnomalyAnalysis",
+    "PMD.BeanMembersShouldSerialize",
+    "PMD.UnusedLocalVariable",
+    "unchecked",
+    "rawtypes"
 })
 @Deprecated() // TODO: eventually we will make this non-public. See issue #872
 public class OpenFeatureClient implements Client {
@@ -148,11 +148,11 @@ public OpenFeatureClient addHooks(Hook... hooks) {
      */
     @Override
     public List<Hook> getHooks() {
-        var allHooks = new ArrayList<Hook>();
+        var allHooks = new HashSet<Hook>();
         for (var queue : this.clientHooks.values()) {
             allHooks.addAll(queue);
         }
-        return allHooks;
+        return new ArrayList<>(allHooks);
     }
 
     /**
@@ -197,8 +197,7 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
                     provider.getProviderHooks(type),
                     flagOptions.getHooks(type),
                     clientHooks.get(type),
-                    openfeatureApi.getHooks(type)
-            );
+                    openfeatureApi.getHooks(type));
 
             var sharedHookContext =
                     new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue);

@@ -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());
     }
 

@@ -14,6 +14,7 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import dev.openfeature.sdk.e2e.Flag;
 import dev.openfeature.sdk.exceptions.GeneralError;
@@ -143,14 +144,16 @@ void provider_metadata() {
     void hook_addition() {
         Hook h1 = mock(Hook.class);
         Hook h2 = mock(Hook.class);
+        when(h1.supportsFlagValueType(any())).thenReturn(true);
+        when(h2.supportsFlagValueType(any())).thenReturn(true);
         api.addHooks(h1);
 
         assertEquals(1, api.getHooks().size());
         assertEquals(h1, api.getHooks().get(0));
 
         api.addHooks(h2);
         assertEquals(2, api.getHooks().size());
-        assertEquals(h2, api.getHooks().get(1));
+        assertTrue(api.getHooks().contains(h2));
     }
 
     @Specification(
@@ -175,6 +178,8 @@ void hookRegistration() {
         Client c = _client();
         Hook m1 = mock(Hook.class);
         Hook m2 = mock(Hook.class);
+        when(m1.supportsFlagValueType(any())).thenReturn(true);
+        when(m2.supportsFlagValueType(any())).thenReturn(true);
         c.addHooks(m1);
         c.addHooks(m2);
         List<Hook> hooks = c.getHooks();

@@ -7,7 +7,6 @@
 import static org.mockito.Mockito.when;
 
 import dev.openfeature.sdk.fixtures.HookFixtures;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -37,7 +36,12 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
 
         var sharedContext = getBaseHookContextForType(FlagValueType.STRING);
         var hookSupportData = new HookSupportData();
-        hookSupport.setHooks(hookSupportData, List.of(hook1, hook2), Collections.emptyList(), new ConcurrentLinkedQueue<>(), new ConcurrentLinkedQueue<>());
+        hookSupport.setHooks(
+                hookSupportData,
+                List.of(hook1, hook2),
+                Collections.emptyList(),
+                new ConcurrentLinkedQueue<>(),
+                new ConcurrentLinkedQueue<>());
         hookSupport.setHookContexts(hookSupportData, sharedContext);
         hookSupport.updateEvaluationContext(hookSupportData, baseEvalContext);
 
@@ -50,14 +54,18 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
         assertThat(result.getValue("baseKey").asString()).isEqualTo("baseValue");
     }
 
-  /*  @ParameterizedTest
-    @EnumSource(value = FlagValueType.class)
+    @Test
     @DisplayName("should always call generic hook")
-    void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
+    void shouldAlwaysCallGenericHook() {
         Hook<?> genericHook = mockGenericHook();
 
         var hookSupportData = new HookSupportData();
-        hookSupport.setHooks(hookSupportData, List.of(genericHook), flagValueType);
+        hookSupport.setHooks(
+                hookSupportData,
+                List.of(genericHook),
+                Collections.emptyList(),
+                new ConcurrentLinkedQueue<>(),
+                new ConcurrentLinkedQueue<>());
 
         callAllHooks(hookSupportData);
 
@@ -73,7 +81,12 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
     void shouldPassDataAcrossStages(FlagValueType flagValueType) {
         var testHook = new TestHookWithData();
         var hookSupportData = new HookSupportData();
-        hookSupport.setHooks(hookSupportData, List.of(testHook), flagValueType);
+        hookSupport.setHooks(
+                hookSupportData,
+                List.of(testHook),
+                Collections.emptyList(),
+                new ConcurrentLinkedQueue<>(),
+                new ConcurrentLinkedQueue<>());
         hookSupport.setHookContexts(hookSupportData, getBaseHookContextForType(flagValueType));
 
         hookSupport.executeBeforeHooks(hookSupportData);
@@ -99,14 +112,19 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) {
         var testHook2 = new TestHookWithData(2);
 
         var hookSupportData = new HookSupportData();
-        hookSupport.setHooks(hookSupportData, List.of(testHook1, testHook2), flagValueType);
+        hookSupport.setHooks(
+                hookSupportData,
+                List.of(testHook1, testHook2),
+                Collections.emptyList(),
+                new ConcurrentLinkedQueue<>(),
+                new ConcurrentLinkedQueue<>());
         hookSupport.setHookContexts(hookSupportData, getBaseHookContextForType(flagValueType));
 
         callAllHooks(hookSupportData);
 
         assertHookData(testHook1, 1, "before", "after", "finallyAfter", "error");
         assertHookData(testHook2, 2, "before", "after", "finallyAfter", "error");
-    }*/
+    }
 
     private static void callAllHooks(HookSupportData hookSupportData) {
         hookSupport.executeBeforeHooks(hookSupportData);