HookSupport improvements: rename back to old name, move code from static factory method to ctor

Signed-off-by: Alexandra Oberaigner <[email protected]>

Author: alexandraoberaigner

src/main/java/dev/openfeature/sdk/HookExecutor.java renamed to src/main/java/dev/openfeature/sdk/HookSupport.java

@@ -8,19 +8,12 @@
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
-class HookExecutor {
+class HookSupport {
     private List<Pair<Hook, HookContext>> hooks;
     private EvaluationContext evaluationContext;
     private final Map<String, Object> hints;
 
-    private HookExecutor(
-            List<Pair<Hook, HookContext>> hooks, EvaluationContext evaluationContext, Map<String, Object> hints) {
-        this.hooks = hooks;
-        this.evaluationContext = evaluationContext;
-        this.hints = hints;
-    }
-
-    public static HookExecutor create(
+    HookSupport(
             List<Hook> hooks,
             SharedHookContext sharedContext,
             EvaluationContext evaluationContext,
@@ -32,7 +25,9 @@ public static HookExecutor create(
                 hookContextPairs.add(Pair.of(hook, curContext));
             }
         }
-        return new HookExecutor(hookContextPairs, evaluationContext, hints);
+        this.hooks = hookContextPairs;
+        this.evaluationContext = evaluationContext;
+        this.hints = hints;
     }
 
     public EvaluationContext getEvaluationContext() {

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

@@ -162,7 +162,7 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
         var hints = Collections.unmodifiableMap(flagOptions.getHookHints());
 
         FlagEvaluationDetails<T> details = null;
-        HookExecutor hookExecutor = null;
+        HookSupport hookSupport = null;
 
         try {
             final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain);
@@ -177,9 +177,9 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
                     new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue);
 
             var evalContext = mergeEvaluationContext(ctx);
-            hookExecutor = HookExecutor.create(mergedHooks, sharedHookContext, evalContext, hints);
+            hookSupport = new HookSupport(mergedHooks, sharedHookContext, evalContext, hints);
 
-            hookExecutor.executeBeforeHooks();
+            hookSupport.executeBeforeHooks();
 
             // "short circuit" if the provider is in NOT_READY or FATAL state
             if (ProviderState.NOT_READY.equals(state)) {
@@ -190,16 +190,16 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
             }
 
             var providerEval = (ProviderEvaluation<T>)
-                    createProviderEvaluation(type, key, defaultValue, provider, hookExecutor.getEvaluationContext());
+                    createProviderEvaluation(type, key, defaultValue, provider, hookSupport.getEvaluationContext());
 
             details = FlagEvaluationDetails.from(providerEval, key);
             if (details.getErrorCode() != null) {
                 var error =
                         ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage());
                 enrichDetailsWithErrorDefaults(defaultValue, details);
-                hookExecutor.executeErrorHooks(error);
+                hookSupport.executeErrorHooks(error);
             } else {
-                hookExecutor.executeAfterHooks(details);
+                hookSupport.executeAfterHooks(details);
             }
         } catch (Exception e) {
             if (details == null) {
@@ -212,10 +212,10 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
             }
             details.setErrorMessage(e.getMessage());
             enrichDetailsWithErrorDefaults(defaultValue, details);
-            hookExecutor.executeErrorHooks(e);
+            hookSupport.executeErrorHooks(e);
         } finally {
-            if (hookExecutor != null) {
-                hookExecutor.executeAfterAllHooks(details);
+            if (hookSupport != null) {
+                hookSupport.executeAfterAllHooks(details);
             }
         }
 

src/test/java/dev/openfeature/sdk/HookExecutorTest.java renamed to src/test/java/dev/openfeature/sdk/HookSupportTest.java

@@ -18,7 +18,7 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.EnumSource;
 
-class HookExecutorTest implements HookFixtures {
+class HookSupportTest implements HookFixtures {
     @Test
     @DisplayName("should merge EvaluationContexts on before hooks correctly")
     void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
@@ -31,8 +31,11 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
         when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber")));
         when(hook2.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("foo", "bar")));
 
-        HookExecutor executor = HookExecutor.create(
-                Arrays.asList(hook1, hook2), getBaseHookContextForType(FlagValueType.STRING), baseContext, Collections.emptyMap());
+        HookSupport executor = new HookSupport(
+                Arrays.asList(hook1, hook2),
+                getBaseHookContextForType(FlagValueType.STRING),
+                baseContext,
+                Collections.emptyMap());
 
         executor.executeBeforeHooks();
 
@@ -49,9 +52,13 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
     void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
         Hook<?> genericHook = mockGenericHook();
 
-        HookExecutor hookExecutor = HookExecutor.create(List.of(genericHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap());
+        HookSupport hookSupport = new HookSupport(
+                List.of(genericHook),
+                getBaseHookContextForType(flagValueType),
+                ImmutableContext.EMPTY,
+                Collections.emptyMap());
 
-        callAllHooks(hookExecutor);
+        callAllHooks(hookSupport);
 
         verify(genericHook).before(any(), any());
         verify(genericHook).after(any(), any(), any());
@@ -64,18 +71,22 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
     @DisplayName("should allow hooks to store and retrieve data across stages")
     void shouldPassDataAcrossStages(FlagValueType flagValueType) {
         var testHook = new TestHookWithData();
-        HookExecutor hookExecutor = HookExecutor.create(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap());
+        HookSupport hookSupport = new HookSupport(
+                List.of(testHook),
+                getBaseHookContextForType(flagValueType),
+                ImmutableContext.EMPTY,
+                Collections.emptyMap());
 
-        hookExecutor.executeBeforeHooks();
+        hookSupport.executeBeforeHooks();
         assertHookData(testHook, "before");
 
-        hookExecutor.executeAfterHooks(FlagEvaluationDetails.builder().build());
+        hookSupport.executeAfterHooks(FlagEvaluationDetails.builder().build());
         assertHookData(testHook, "before", "after");
 
-        hookExecutor.executeAfterAllHooks(FlagEvaluationDetails.builder().build());
+        hookSupport.executeAfterAllHooks(FlagEvaluationDetails.builder().build());
         assertHookData(testHook, "before", "after", "finallyAfter");
 
-        hookExecutor.executeErrorHooks(mock(Exception.class));
+        hookSupport.executeErrorHooks(mock(Exception.class));
         assertHookData(testHook, "before", "after", "finallyAfter", "error");
     }
 
@@ -86,48 +97,49 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) {
         var testHook1 = new TestHookWithData(1);
         var testHook2 = new TestHookWithData(2);
 
-        HookExecutor hookExecutor = HookExecutor.create(List.of(testHook1, testHook2), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap());
+        HookSupport hookSupport = new HookSupport(
+                List.of(testHook1, testHook2),
+                getBaseHookContextForType(flagValueType),
+                ImmutableContext.EMPTY,
+                Collections.emptyMap());
 
-        callAllHooks(hookExecutor);
+        callAllHooks(hookSupport);
 
         assertHookData(testHook1, 1, "before", "after", "finallyAfter", "error");
         assertHookData(testHook2, 2, "before", "after", "finallyAfter", "error");
     }
 
-    private static void callAllHooks(HookExecutor hookExecutor) {
-        hookExecutor.executeBeforeHooks();
-        hookExecutor.executeAfterHooks(FlagEvaluationDetails.builder().build());
-        hookExecutor.executeAfterAllHooks(FlagEvaluationDetails.builder().build());
-        hookExecutor.executeErrorHooks(mock(Exception.class));
+    private static void callAllHooks(HookSupport hookSupport) {
+        hookSupport.executeBeforeHooks();
+        hookSupport.executeAfterHooks(FlagEvaluationDetails.builder().build());
+        hookSupport.executeAfterAllHooks(FlagEvaluationDetails.builder().build());
+        hookSupport.executeErrorHooks(mock(Exception.class));
     }
 
-    private static void assertHookData(TestHookWithData testHook, String ... expectedKeys) {
+    private static void assertHookData(TestHookWithData testHook, String... expectedKeys) {
         for (String expectedKey : expectedKeys) {
             assertThat(testHook.hookData.get(expectedKey))
                     .withFailMessage("Expected key %s not present in hook data", expectedKey)
                     .isNotNull();
         }
     }
 
-    private static void assertHookData(TestHookWithData testHook, Object expectedValue, String ... expectedKeys) {
+    private static void assertHookData(TestHookWithData testHook, Object expectedValue, String... expectedKeys) {
         for (String expectedKey : expectedKeys) {
             assertThat(testHook.hookData.get(expectedKey))
                     .withFailMessage("Expected key '%s' not present in hook data", expectedKey)
                     .isNotNull();
             assertThat(testHook.hookData.get(expectedKey))
-                    .withFailMessage("Expected key '%s' not containing expected value. Expected '%s' but found '%s'",
+                    .withFailMessage(
+                            "Expected key '%s' not containing expected value. Expected '%s' but found '%s'",
                             expectedKey, expectedValue, testHook.hookData.get(expectedKey))
                     .isEqualTo(expectedValue);
         }
     }
 
     private SharedHookContext getBaseHookContextForType(FlagValueType flagValueType) {
         return new SharedHookContext<>(
-                "flagKey",
-                flagValueType,
-                () -> "client",
-                () -> "provider",
-                createDefaultValue(flagValueType));
+                "flagKey", flagValueType, () -> "client", () -> "provider", createDefaultValue(flagValueType));
     }
 
     private Object createDefaultValue(FlagValueType flagValueType) {