From 269dfe416570eb36591c615c98b93dae2f7404d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Tue, 23 Sep 2025 07:22:40 +0200 Subject: [PATCH 01/21] feat: add hook data support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/HookContext.java | 10 +- .../openfeature/sdk/HookContextDecorator.java | 23 +++ .../openfeature/sdk/HookContextWithData.java | 21 ++ .../java/dev/openfeature/sdk/HookData.java | 81 ++++++++ .../java/dev/openfeature/sdk/HookSupport.java | 105 +++++++--- .../dev/openfeature/sdk/ImmutableContext.java | 2 + .../openfeature/sdk/OpenFeatureClient.java | 43 ++-- src/main/java/dev/openfeature/sdk/Pair.java | 28 +++ .../dev/openfeature/sdk/HookContextTest.java | 50 ++++- .../dev/openfeature/sdk/HookDataTest.java | 79 ++++++++ .../dev/openfeature/sdk/HookSupportTest.java | 184 ++++++++++++++++-- .../sdk/benchmark/AllocationBenchmark.java | 24 +++ 12 files changed, 570 insertions(+), 80 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/HookContextDecorator.java create mode 100644 src/main/java/dev/openfeature/sdk/HookContextWithData.java create mode 100644 src/main/java/dev/openfeature/sdk/HookData.java create mode 100644 src/main/java/dev/openfeature/sdk/Pair.java create mode 100644 src/test/java/dev/openfeature/sdk/HookDataTest.java diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index e14eeb643..2cb58e1e1 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -1,8 +1,8 @@ package dev.openfeature.sdk; import lombok.Builder; +import lombok.Data; import lombok.NonNull; -import lombok.Value; import lombok.With; /** @@ -10,7 +10,7 @@ * * @param the type for the flag being evaluated */ -@Value +@Data @Builder @With public class HookContext { @@ -26,7 +26,7 @@ public class HookContext { Metadata providerMetadata; /** - * Builds a {@link HookContext} instances from request data. + * Builds {@link HookContext} instances from request data. * * @param key feature flag key * @param type flag value type @@ -53,4 +53,8 @@ public static HookContext from( .defaultValue(defaultValue) .build(); } + + HookData getHookData() { + return null; + } } diff --git a/src/main/java/dev/openfeature/sdk/HookContextDecorator.java b/src/main/java/dev/openfeature/sdk/HookContextDecorator.java new file mode 100644 index 000000000..e2b228a21 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookContextDecorator.java @@ -0,0 +1,23 @@ +package dev.openfeature.sdk; + +/** + * A base decorator class for {@link HookContext} that enables dynamic enhancement of its functionality. + * This class wraps an existing {@code HookContext} instance and delegates method calls to it. + * + * @param the type for the flag being evaluated + */ +class HookContextDecorator extends HookContext { + + HookContext decorated; + + protected HookContextDecorator(HookContext context) { + super(context.getFlagKey(), context.getType(), context.getDefaultValue(), context.getCtx(), + context.getClientMetadata(), context.getProviderMetadata()); + this.decorated = context; + } + + @Override + public HookData getHookData() { + return decorated.getHookData(); + } +} diff --git a/src/main/java/dev/openfeature/sdk/HookContextWithData.java b/src/main/java/dev/openfeature/sdk/HookContextWithData.java new file mode 100644 index 000000000..56cd501d4 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookContextWithData.java @@ -0,0 +1,21 @@ +package dev.openfeature.sdk; + +/** + * A concrete decorator for {@link HookContext} that adds {@link HookData} to the existing functionality. + * + * @param the type for the flag being evaluated + */ +public class HookContextWithData extends HookContextDecorator { + + private final HookData hookData; + + public HookContextWithData(HookContext context, HookData data) { + super(context); + this.hookData = data; + } + + @Override + public HookData getHookData() { + return hookData; + } +} diff --git a/src/main/java/dev/openfeature/sdk/HookData.java b/src/main/java/dev/openfeature/sdk/HookData.java new file mode 100644 index 000000000..c7c644a93 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookData.java @@ -0,0 +1,81 @@ +package dev.openfeature.sdk; + +import java.util.HashMap; +import java.util.Map; + +/** + * Hook data provides a way for hooks to maintain state across their execution stages. + * Each hook instance gets its own isolated data store that persists only for the duration + * of a single flag evaluation. + */ +public interface HookData { + + /** + * Sets a value for the given key. + * + * @param key the key to store the value under + * @param value the value to store + */ + void set(String key, Object value); + + /** + * Gets the value for the given key. + * + * @param key the key to retrieve the value for + * @return the value, or null if not found + */ + Object get(String key); + + /** + * Gets the value for the given key, cast to the specified type. + * + * @param the type to cast to + * @param key the key to retrieve the value for + * @param type the class to cast to + * @return the value cast to the specified type, or null if not found + * @throws ClassCastException if the value cannot be cast to the specified type + */ + T get(String key, Class type); + + /** + * Default implementation uses a HashMap. + */ + static HookData create() { + return new DefaultHookData(); + } + + /** + * Default implementation of HookData. + */ + class DefaultHookData implements HookData { + private Map data; + + @Override + public void set(String key, Object value) { + if (data == null) { + data = new HashMap<>(); + } + data.put(key, value); + } + + @Override + public Object get(String key) { + if (data == null) { + return null; + } + return data.get(key); + } + + @Override + public T get(String key, Class type) { + Object value = get(key); + if (value == null) { + return null; + } + if (!type.isInstance(value)) { + throw new ClassCastException("Value for key '" + key + "' is not of type " + type.getName()); + } + return type.cast(value); + } + } +} diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index 73518ee8e..cb84ba356 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -5,7 +5,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.function.Consumer; +import java.util.function.BiConsumer; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -15,52 +15,81 @@ class HookSupport { public EvaluationContext beforeHooks( - FlagValueType flagValueType, HookContext hookCtx, List hooks, Map hints) { - return callBeforeHooks(flagValueType, hookCtx, hooks, hints); + FlagValueType flagValueType, + HookContext hookCtx, + List> hookDataPairs, + Map hints) { + return callBeforeHooks(flagValueType, hookCtx, hookDataPairs, hints); } public void afterHooks( FlagValueType flagValueType, HookContext hookContext, FlagEvaluationDetails details, - List hooks, + List> hookDataPairs, Map hints) { - executeHooksUnchecked(flagValueType, hooks, hook -> hook.after(hookContext, details, hints)); + executeHooksUnchecked( + flagValueType, hookDataPairs, hookContext, (hook, ctx) -> hook.after(ctx, details, hints)); } public void afterAllHooks( FlagValueType flagValueType, HookContext hookCtx, FlagEvaluationDetails details, - List hooks, + List> hookDataPairs, Map hints) { - executeHooks(flagValueType, hooks, "finally", hook -> hook.finallyAfter(hookCtx, details, hints)); + executeHooks( + flagValueType, + hookDataPairs, + hookCtx, + "finally", + (hook, ctx) -> hook.finallyAfter(ctx, details, hints)); } public void errorHooks( FlagValueType flagValueType, HookContext hookCtx, Exception e, - List hooks, + List> hookDataPairs, Map hints) { - executeHooks(flagValueType, hooks, "error", hook -> hook.error(hookCtx, e, hints)); + executeHooks(flagValueType, hookDataPairs, hookCtx, "error", (hook, ctx) -> hook.error(ctx, e, hints)); + } + + public List> getHookDataPairs(List hooks, FlagValueType flagValueType) { + var pairs = new ArrayList>(); + for (Hook hook : hooks) { + if (hook.supportsFlagValueType(flagValueType)) { + pairs.add(Pair.of(hook, HookData.create())); + } + } + return pairs; } private void executeHooks( - FlagValueType flagValueType, List hooks, String hookMethod, Consumer> hookCode) { - if (hooks != null) { - for (Hook hook : hooks) { - if (hook.supportsFlagValueType(flagValueType)) { - executeChecked(hook, hookCode, hookMethod); - } + FlagValueType flagValueType, + List> hookDataPairs, + HookContext hookContext, + String hookMethod, + BiConsumer, HookContext> hookCode) { + if (hookDataPairs != null) { + for (Pair hookDataPair : hookDataPairs) { + Hook hook = hookDataPair.getLeft(); + HookData hookData = hookDataPair.getRight(); + executeChecked(hook, hookData, hookContext, hookCode, hookMethod); } } } // before, error, and finally hooks shouldn't throw - private void executeChecked(Hook hook, Consumer> hookCode, String hookMethod) { + private void executeChecked( + Hook hook, + HookData hookData, + HookContext hookContext, + BiConsumer, HookContext> hookCode, + String hookMethod) { try { - hookCode.accept(hook); + var hookCtxWithData = new HookContextWithData(hookContext, hookData); + hookCode.accept(hook, hookCtxWithData); } catch (Exception exception) { log.error( "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", @@ -71,29 +100,41 @@ private void executeChecked(Hook hook, Consumer> hookCode, String } // after hooks can throw in order to do validation - private void executeHooksUnchecked(FlagValueType flagValueType, List hooks, Consumer> hookCode) { - if (hooks != null) { - for (Hook hook : hooks) { - if (hook.supportsFlagValueType(flagValueType)) { - hookCode.accept(hook); - } + private void executeHooksUnchecked( + FlagValueType flagValueType, + List> hookDataPairs, + HookContext hookContext, + BiConsumer, HookContext> hookCode) { + if (hookDataPairs != null) { + for (Pair hookDataPair : hookDataPairs) { + Hook hook = hookDataPair.getLeft(); + HookData hookData = hookDataPair.getRight(); + var hookCtxWithData = new HookContextWithData(hookContext, hookData); + hookCode.accept(hook, hookCtxWithData); } } } private EvaluationContext callBeforeHooks( - FlagValueType flagValueType, HookContext hookCtx, List hooks, Map hints) { + FlagValueType flagValueType, + HookContext hookCtx, + List> hookDataPairs, + Map hints) { // These traverse backwards from normal. - List reversedHooks = new ArrayList<>(hooks); + List> reversedHooks = new ArrayList<>(hookDataPairs); Collections.reverse(reversedHooks); EvaluationContext context = hookCtx.getCtx(); - for (Hook hook : reversedHooks) { - if (hook.supportsFlagValueType(flagValueType)) { - Optional optional = - Optional.ofNullable(hook.before(hookCtx, hints)).orElse(Optional.empty()); - if (optional.isPresent()) { - context = context.merge(optional.get()); - } + + for (Pair hookDataPair : reversedHooks) { + Hook hook = hookDataPair.getLeft(); + HookData hookData = hookDataPair.getRight(); + + // Create a new context with this hook's data + HookContext contextWithHookData = new HookContextWithData(hookCtx, hookData); + Optional optional = + Optional.ofNullable(hook.before(contextWithHookData, hints)).orElse(Optional.empty()); + if (optional.isPresent()) { + context = context.merge(optional.get()); } } return context; diff --git a/src/main/java/dev/openfeature/sdk/ImmutableContext.java b/src/main/java/dev/openfeature/sdk/ImmutableContext.java index 8560c369e..e4916dfca 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableContext.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableContext.java @@ -20,6 +20,8 @@ @SuppressWarnings("PMD.BeanMembersShouldSerialize") public final class ImmutableContext implements EvaluationContext { + public static final ImmutableContext EMPTY = new ImmutableContext(); + @Delegate(excludes = DelegateExclusions.class) private final ImmutableStructure structure; diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index b5522b66a..6edc5c79b 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -164,32 +164,27 @@ private FlagEvaluationDetails evaluateFlag( var hints = Collections.unmodifiableMap(flagOptions.getHookHints()); FlagEvaluationDetails details = null; - List mergedHooks = null; - HookContext afterHookContext = null; + List mergedHooks; + List> hookDataPairs = null; + HookContext hookContext = null; try { - var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); + final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); // provider must be accessed once to maintain a consistent reference - var provider = stateManager.getProvider(); - var state = stateManager.getState(); + final var provider = stateManager.getProvider(); + final var state = stateManager.getState(); + hookContext = HookContext.from( + key, type, this.getMetadata(), provider.getMetadata(), ImmutableContext.EMPTY, defaultValue); + + // we are setting the evaluation context one after the other, so that we have a hook context in each + // possible exception case. + hookContext.setCtx(mergeEvaluationContext(ctx)); mergedHooks = ObjectUtils.merge( provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks()); - - var mergedCtx = hookSupport.beforeHooks( - type, - HookContext.from( - key, - type, - this.getMetadata(), - provider.getMetadata(), - mergeEvaluationContext(ctx), - defaultValue), - mergedHooks, - hints); - - afterHookContext = - HookContext.from(key, type, this.getMetadata(), provider.getMetadata(), mergedCtx, defaultValue); + hookDataPairs = hookSupport.getHookDataPairs(mergedHooks, type); + var mergedCtx = hookSupport.beforeHooks(type, hookContext, hookDataPairs, hints); + hookContext.setCtx(mergedCtx); // "short circuit" if the provider is in NOT_READY or FATAL state if (ProviderState.NOT_READY.equals(state)) { @@ -207,9 +202,9 @@ private FlagEvaluationDetails evaluateFlag( var error = ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints); + hookSupport.errorHooks(type, hookContext, error, hookDataPairs, hints); } else { - hookSupport.afterHooks(type, afterHookContext, details, mergedHooks, hints); + hookSupport.afterHooks(type, hookContext, details, hookDataPairs, hints); } } catch (Exception e) { if (details == null) { @@ -222,9 +217,9 @@ private FlagEvaluationDetails evaluateFlag( } details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints); + hookSupport.errorHooks(type, hookContext, e, hookDataPairs, hints); } finally { - hookSupport.afterAllHooks(type, afterHookContext, details, mergedHooks, hints); + hookSupport.afterAllHooks(type, hookContext, details, hookDataPairs, hints); } return details; diff --git a/src/main/java/dev/openfeature/sdk/Pair.java b/src/main/java/dev/openfeature/sdk/Pair.java new file mode 100644 index 000000000..bc6614093 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/Pair.java @@ -0,0 +1,28 @@ +package dev.openfeature.sdk; + +class Pair { + private final K key; + private final V value; + + private Pair(K key, V value) { + this.key = key; + this.value = value; + } + + public K getLeft() { + return key; + } + + public V getRight() { + return value; + } + + @Override + public String toString() { + return "Pair{" + "key=" + key + ", value=" + value + '}'; + } + + public static Pair of(K key, V value) { + return new Pair<>(key, value); + } +} diff --git a/src/test/java/dev/openfeature/sdk/HookContextTest.java b/src/test/java/dev/openfeature/sdk/HookContextTest.java index 2196b8b1f..cf6071ab8 100644 --- a/src/test/java/dev/openfeature/sdk/HookContextTest.java +++ b/src/test/java/dev/openfeature/sdk/HookContextTest.java @@ -1,6 +1,6 @@ package dev.openfeature.sdk; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mock; import org.junit.jupiter.api.Test; @@ -29,4 +29,52 @@ void metadata_field_is_type_metadata() { "The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters. It has no return value.") @Test void not_applicable_for_dynamic_context() {} + + @Test + void shouldCreateHookContextWithHookData() { + HookContext context = HookContext.builder() + .flagKey("test-flag") + .type(FlagValueType.STRING) + .defaultValue("default") + .ctx(new ImmutableContext()) + .build(); + HookData hookData = HookData.create(); + hookData.set("test", "value"); + + HookContextWithData contextWithData = new HookContextWithData(context, hookData); + + assertNotNull(contextWithData.getHookData()); + assertEquals("value", contextWithData.getHookData().get("test")); + } + + @Test + void shouldCreateHookContextWithoutHookData() { + HookContext context = HookContext.builder() + .flagKey("test-flag") + .type(FlagValueType.STRING) + .defaultValue("default") + .ctx(new ImmutableContext()) + .build(); + + assertNull(context.getHookData()); + } + + @Test + void shouldCreateHookContextWithHookDataUsingWith() { + HookContext originalContext = HookContext.builder() + .flagKey("test-flag") + .type(FlagValueType.STRING) + .defaultValue("default") + .ctx(new ImmutableContext()) + .build(); + + HookData hookData = HookData.create(); + hookData.set("timing", System.currentTimeMillis()); + + HookContext contextWithHookData = new HookContextWithData(originalContext, hookData); + + assertNull(originalContext.getHookData()); + assertNotNull(contextWithHookData.getHookData()); + assertNotNull(contextWithHookData.getHookData().get("timing")); + } } diff --git a/src/test/java/dev/openfeature/sdk/HookDataTest.java b/src/test/java/dev/openfeature/sdk/HookDataTest.java new file mode 100644 index 000000000..eacbeeb78 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/HookDataTest.java @@ -0,0 +1,79 @@ +package dev.openfeature.sdk; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.Test; + +class HookDataTest { + + @Test + void shouldStoreAndRetrieveValues() { + HookData hookData = HookData.create(); + + hookData.set("key1", "value1"); + hookData.set("key2", 42); + hookData.set("key3", true); + + assertEquals("value1", hookData.get("key1")); + assertEquals(42, hookData.get("key2")); + assertEquals(true, hookData.get("key3")); + } + + @Test + void shouldReturnNullForMissingKeys() { + HookData hookData = HookData.create(); + + assertNull(hookData.get("nonexistent")); + } + + @Test + void shouldSupportTypeSafeRetrieval() { + HookData hookData = HookData.create(); + + hookData.set("string", "hello"); + hookData.set("integer", 123); + hookData.set("boolean", false); + + assertEquals("hello", hookData.get("string", String.class)); + assertEquals(Integer.valueOf(123), hookData.get("integer", Integer.class)); + assertEquals(Boolean.FALSE, hookData.get("boolean", Boolean.class)); + } + + @Test + void shouldReturnNullForMissingKeysWithType() { + HookData hookData = HookData.create(); + + assertNull(hookData.get("missing", String.class)); + } + + @Test + void shouldThrowClassCastExceptionForWrongType() { + HookData hookData = HookData.create(); + + hookData.set("string", "not a number"); + + assertThrows(ClassCastException.class, () -> { + hookData.get("string", Integer.class); + }); + } + + @Test + void shouldOverwriteExistingValues() { + HookData hookData = HookData.create(); + + hookData.set("key", "original"); + assertEquals("original", hookData.get("key")); + + hookData.set("key", "updated"); + assertEquals("updated", hookData.get("key")); + } + + @Test + void shouldSupportNullValues() { + HookData hookData = HookData.create(); + + hookData.set("nullKey", null); + assertNull(hookData.get("nullKey")); + assertNull(hookData.get("nullKey", String.class)); + } +} diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index 02a8ff90c..85a25facb 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -9,6 +9,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import org.junit.jupiter.api.DisplayName; @@ -23,8 +24,15 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { Map attributes = new HashMap<>(); attributes.put("baseKey", new Value("baseValue")); EvaluationContext baseContext = new ImmutableContext(attributes); - HookContext hookContext = new HookContext<>( - "flagKey", FlagValueType.STRING, "defaultValue", baseContext, () -> "client", () -> "provider"); + FlagValueType valueType = FlagValueType.STRING; + HookContext hookContext = HookContext.builder() + .flagKey("flagKey") + .type(valueType) + .defaultValue("defaultValue") + .ctx(baseContext) + .clientMetadata(() -> "client") + .providerMetadata(() -> "provider") + .build(); Hook hook1 = mockStringHook(); Hook hook2 = mockStringHook(); when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber"))); @@ -32,7 +40,10 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { HookSupport hookSupport = new HookSupport(); EvaluationContext result = hookSupport.beforeHooks( - FlagValueType.STRING, hookContext, Arrays.asList(hook1, hook2), Collections.emptyMap()); + valueType, + hookContext, + hookSupport.getHookDataPairs(Arrays.asList(hook1, hook2), valueType), + Collections.emptyMap()); assertThat(result.getValue("bla").asString()).isEqualTo("blubber"); assertThat(result.getValue("foo").asString()).isEqualTo("bar"); @@ -45,36 +56,32 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { Hook genericHook = mockGenericHook(); HookSupport hookSupport = new HookSupport(); + var hookDataPairs = hookSupport.getHookDataPairs(Collections.singletonList(genericHook), flagValueType); EvaluationContext baseContext = new ImmutableContext(); IllegalStateException expectedException = new IllegalStateException("All fine, just a test"); - HookContext hookContext = new HookContext<>( - "flagKey", - flagValueType, - createDefaultValue(flagValueType), - baseContext, - () -> "client", - () -> "provider"); + HookContext hookContext = HookContext.builder() + .flagKey("flagKey") + .type(flagValueType) + .defaultValue(createDefaultValue(flagValueType)) + .ctx(baseContext) + .clientMetadata(() -> "client") + .providerMetadata(() -> "provider") + .build(); - hookSupport.beforeHooks( - flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap()); + hookSupport.beforeHooks(flagValueType, hookContext, hookDataPairs, Collections.emptyMap()); hookSupport.afterHooks( flagValueType, hookContext, FlagEvaluationDetails.builder().build(), - Collections.singletonList(genericHook), + hookDataPairs, Collections.emptyMap()); hookSupport.afterAllHooks( flagValueType, hookContext, FlagEvaluationDetails.builder().build(), - Collections.singletonList(genericHook), - Collections.emptyMap()); - hookSupport.errorHooks( - flagValueType, - hookContext, - expectedException, - Collections.singletonList(genericHook), + hookDataPairs, Collections.emptyMap()); + hookSupport.errorHooks(flagValueType, hookContext, expectedException, hookDataPairs, Collections.emptyMap()); verify(genericHook).before(any(), any()); verify(genericHook).after(any(), any(), any()); @@ -82,6 +89,101 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { verify(genericHook).error(any(), any(), any()); } + @ParameterizedTest + @EnumSource(value = FlagValueType.class) + @DisplayName("should allow hooks to store and retrieve data across stages") + void shouldPassDataAcrossStages(FlagValueType flagValueType) { + HookSupport hookSupport = new HookSupport(); + HookContext hookContext = getObjectHookContext(flagValueType); + + TestHookWithData testHook = new TestHookWithData("test-key", "value"); + var pairs = hookSupport.getHookDataPairs(List.of(testHook), flagValueType); + + callAllHooks(flagValueType, hookSupport, hookContext, testHook); + + assertHookData(testHook, "value"); + } + + @ParameterizedTest + @EnumSource(value = FlagValueType.class) + @DisplayName("should isolate data between different hook instances") + void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { + HookSupport hookSupport = new HookSupport(); + HookContext hookContext = getObjectHookContext(flagValueType); + + TestHookWithData testHook1 = new TestHookWithData("test-key", "value-1"); + TestHookWithData testHook2 = new TestHookWithData("test-key", "value-2"); + var pairs = hookSupport.getHookDataPairs(List.of(testHook1, testHook2), flagValueType); + + callAllHooks(flagValueType, hookSupport, hookContext, pairs); + + assertHookData(testHook1, "value-1"); + assertHookData(testHook2, "value-2"); + } + + @ParameterizedTest + @EnumSource(value = FlagValueType.class) + @DisplayName("should isolate data between the same hook instances") + void shouldIsolateDataBetweenSameHooks(FlagValueType flagValueType) { + + HookSupport hookSupport = new HookSupport(); + HookContext hookContext = getObjectHookContext(flagValueType); + + TestHookWithData testHook = new TestHookWithData("test-key", "value-1"); + + // run hooks first time + callAllHooks(flagValueType, hookSupport, hookContext, testHook); + assertHookData(testHook, "value-1"); + + // re-run with different value, will throw if HookData contains already data + testHook.value = "value-2"; + callAllHooks(flagValueType, hookSupport, hookContext, testHook); + + assertHookData(testHook, "value-2"); + } + + private HookContext getObjectHookContext(FlagValueType flagValueType) { + EvaluationContext baseContext = new ImmutableContext(); + HookContext hookContext = HookContext.builder() + .flagKey("flagKey") + .type(flagValueType) + .defaultValue(createDefaultValue(flagValueType)) + .ctx(baseContext) + .clientMetadata(() -> "client") + .providerMetadata(() -> "provider") + .build(); + return hookContext; + } + + private static void assertHookData(TestHookWithData testHook1, String expected) { + assertThat(testHook1.onBeforeValue).isEqualTo(expected); + assertThat(testHook1.onFinallyAfterValue).isEqualTo(expected); + assertThat(testHook1.onAfterValue).isEqualTo(expected); + assertThat(testHook1.onErrorValue).isEqualTo(expected); + } + + private static void callAllHooks( + FlagValueType flagValueType, + HookSupport hookSupport, + HookContext hookContext, + TestHookWithData testHook) { + var pairs = hookSupport.getHookDataPairs(List.of(testHook), flagValueType); + callAllHooks(flagValueType, hookSupport, hookContext, pairs); + } + + private static void callAllHooks( + FlagValueType flagValueType, + HookSupport hookSupport, + HookContext hookContext, + List> pairs) { + hookSupport.beforeHooks(flagValueType, hookContext, pairs, Collections.emptyMap()); + hookSupport.afterHooks( + flagValueType, hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); + hookSupport.errorHooks(flagValueType, hookContext, new Exception(), pairs, Collections.emptyMap()); + hookSupport.afterAllHooks( + flagValueType, hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); + } + private Object createDefaultValue(FlagValueType flagValueType) { switch (flagValueType) { case INTEGER: @@ -105,4 +207,46 @@ private EvaluationContext evaluationContextWithValue(String key, String value) { EvaluationContext baseContext = new ImmutableContext(attributes); return baseContext; } + + private class TestHookWithData implements Hook { + + private final String key; + Object value; + + Object onBeforeValue; + Object onAfterValue; + Object onErrorValue; + Object onFinallyAfterValue; + + TestHookWithData(String key, Object value) { + this.key = key; + this.value = value; + } + + @Override + public Optional before(HookContext ctx, Map hints) { + var storedValue = ctx.getHookData().get(key); + if (storedValue != null) { + throw new Error("Hook data isolation violated! Data is already set."); + } + ctx.getHookData().set(key, value); + onBeforeValue = ctx.getHookData().get(key); + return Optional.empty(); + } + + @Override + public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + onAfterValue = ctx.getHookData().get(key); + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + onErrorValue = ctx.getHookData().get(key); + } + + @Override + public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { + onFinallyAfterValue = ctx.getHookData().get(key); + } + } } diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index 5bc89d03d..9fe043722 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -54,6 +54,30 @@ public Optional before(HookContext ctx, Map() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); + client.addHooks(new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); + client.addHooks(new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); + client.addHooks(new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); Map invocationAttrs = new HashMap<>(); invocationAttrs.put("invoke", new Value(3)); From 3b113c1bb10ea534226f1b6535421a01e4d96d51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Tue, 23 Sep 2025 08:32:55 +0200 Subject: [PATCH 02/21] feat: gemini suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/HookSupport.java | 26 +++++++------------ .../openfeature/sdk/OpenFeatureClient.java | 10 +++---- src/main/java/dev/openfeature/sdk/Pair.java | 4 +-- .../dev/openfeature/sdk/HookSupportTest.java | 20 ++++++-------- 4 files changed, 24 insertions(+), 36 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index cb84ba356..4acbba28d 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -15,31 +15,27 @@ class HookSupport { public EvaluationContext beforeHooks( - FlagValueType flagValueType, HookContext hookCtx, List> hookDataPairs, Map hints) { - return callBeforeHooks(flagValueType, hookCtx, hookDataPairs, hints); + return callBeforeHooks(hookCtx, hookDataPairs, hints); } public void afterHooks( - FlagValueType flagValueType, HookContext hookContext, FlagEvaluationDetails details, List> hookDataPairs, Map hints) { executeHooksUnchecked( - flagValueType, hookDataPairs, hookContext, (hook, ctx) -> hook.after(ctx, details, hints)); + hookDataPairs, hookContext, (hook, ctx) -> hook.after(ctx, details, hints)); } public void afterAllHooks( - FlagValueType flagValueType, HookContext hookCtx, FlagEvaluationDetails details, List> hookDataPairs, Map hints) { executeHooks( - flagValueType, hookDataPairs, hookCtx, "finally", @@ -47,12 +43,11 @@ public void afterAllHooks( } public void errorHooks( - FlagValueType flagValueType, HookContext hookCtx, Exception e, List> hookDataPairs, Map hints) { - executeHooks(flagValueType, hookDataPairs, hookCtx, "error", (hook, ctx) -> hook.error(ctx, e, hints)); + executeHooks(hookDataPairs, hookCtx, "error", (hook, ctx) -> hook.error(ctx, e, hints)); } public List> getHookDataPairs(List hooks, FlagValueType flagValueType) { @@ -66,15 +61,14 @@ public List> getHookDataPairs(List hooks, FlagValueTy } private void executeHooks( - FlagValueType flagValueType, List> hookDataPairs, HookContext hookContext, String hookMethod, BiConsumer, HookContext> hookCode) { if (hookDataPairs != null) { for (Pair hookDataPair : hookDataPairs) { - Hook hook = hookDataPair.getLeft(); - HookData hookData = hookDataPair.getRight(); + Hook hook = hookDataPair.getKey(); + HookData hookData = hookDataPair.getValue(); executeChecked(hook, hookData, hookContext, hookCode, hookMethod); } } @@ -101,14 +95,13 @@ private void executeChecked( // after hooks can throw in order to do validation private void executeHooksUnchecked( - FlagValueType flagValueType, List> hookDataPairs, HookContext hookContext, BiConsumer, HookContext> hookCode) { if (hookDataPairs != null) { for (Pair hookDataPair : hookDataPairs) { - Hook hook = hookDataPair.getLeft(); - HookData hookData = hookDataPair.getRight(); + Hook hook = hookDataPair.getKey(); + HookData hookData = hookDataPair.getValue(); var hookCtxWithData = new HookContextWithData(hookContext, hookData); hookCode.accept(hook, hookCtxWithData); } @@ -116,7 +109,6 @@ private void executeHooksUnchecked( } private EvaluationContext callBeforeHooks( - FlagValueType flagValueType, HookContext hookCtx, List> hookDataPairs, Map hints) { @@ -126,8 +118,8 @@ private EvaluationContext callBeforeHooks( EvaluationContext context = hookCtx.getCtx(); for (Pair hookDataPair : reversedHooks) { - Hook hook = hookDataPair.getLeft(); - HookData hookData = hookDataPair.getRight(); + Hook hook = hookDataPair.getKey(); + HookData hookData = hookDataPair.getValue(); // Create a new context with this hook's data HookContext contextWithHookData = new HookContextWithData(hookCtx, hookData); diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 6edc5c79b..b35193a24 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -183,7 +183,7 @@ private FlagEvaluationDetails evaluateFlag( mergedHooks = ObjectUtils.merge( provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks()); hookDataPairs = hookSupport.getHookDataPairs(mergedHooks, type); - var mergedCtx = hookSupport.beforeHooks(type, hookContext, hookDataPairs, hints); + var mergedCtx = hookSupport.beforeHooks(hookContext, hookDataPairs, hints); hookContext.setCtx(mergedCtx); // "short circuit" if the provider is in NOT_READY or FATAL state @@ -202,9 +202,9 @@ private FlagEvaluationDetails evaluateFlag( var error = ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, hookContext, error, hookDataPairs, hints); + hookSupport.errorHooks(hookContext, error, hookDataPairs, hints); } else { - hookSupport.afterHooks(type, hookContext, details, hookDataPairs, hints); + hookSupport.afterHooks(hookContext, details, hookDataPairs, hints); } } catch (Exception e) { if (details == null) { @@ -217,9 +217,9 @@ private FlagEvaluationDetails evaluateFlag( } details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, hookContext, e, hookDataPairs, hints); + hookSupport.errorHooks(hookContext, e, hookDataPairs, hints); } finally { - hookSupport.afterAllHooks(type, hookContext, details, hookDataPairs, hints); + hookSupport.afterAllHooks(hookContext, details, hookDataPairs, hints); } return details; diff --git a/src/main/java/dev/openfeature/sdk/Pair.java b/src/main/java/dev/openfeature/sdk/Pair.java index bc6614093..e3e50974f 100644 --- a/src/main/java/dev/openfeature/sdk/Pair.java +++ b/src/main/java/dev/openfeature/sdk/Pair.java @@ -9,11 +9,11 @@ private Pair(K key, V value) { this.value = value; } - public K getLeft() { + public K getKey() { return key; } - public V getRight() { + public V getValue() { return value; } diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index 85a25facb..ec202a0bc 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -40,7 +40,6 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { HookSupport hookSupport = new HookSupport(); EvaluationContext result = hookSupport.beforeHooks( - valueType, hookContext, hookSupport.getHookDataPairs(Arrays.asList(hook1, hook2), valueType), Collections.emptyMap()); @@ -68,20 +67,18 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { .providerMetadata(() -> "provider") .build(); - hookSupport.beforeHooks(flagValueType, hookContext, hookDataPairs, Collections.emptyMap()); + hookSupport.beforeHooks(hookContext, hookDataPairs, Collections.emptyMap()); hookSupport.afterHooks( - flagValueType, hookContext, FlagEvaluationDetails.builder().build(), hookDataPairs, Collections.emptyMap()); hookSupport.afterAllHooks( - flagValueType, hookContext, FlagEvaluationDetails.builder().build(), hookDataPairs, Collections.emptyMap()); - hookSupport.errorHooks(flagValueType, hookContext, expectedException, hookDataPairs, Collections.emptyMap()); + hookSupport.errorHooks(hookContext, expectedException, hookDataPairs, Collections.emptyMap()); verify(genericHook).before(any(), any()); verify(genericHook).after(any(), any(), any()); @@ -115,7 +112,7 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { TestHookWithData testHook2 = new TestHookWithData("test-key", "value-2"); var pairs = hookSupport.getHookDataPairs(List.of(testHook1, testHook2), flagValueType); - callAllHooks(flagValueType, hookSupport, hookContext, pairs); + callAllHooks(hookSupport, hookContext, pairs); assertHookData(testHook1, "value-1"); assertHookData(testHook2, "value-2"); @@ -168,20 +165,19 @@ private static void callAllHooks( HookContext hookContext, TestHookWithData testHook) { var pairs = hookSupport.getHookDataPairs(List.of(testHook), flagValueType); - callAllHooks(flagValueType, hookSupport, hookContext, pairs); + callAllHooks(hookSupport, hookContext, pairs); } private static void callAllHooks( - FlagValueType flagValueType, HookSupport hookSupport, HookContext hookContext, List> pairs) { - hookSupport.beforeHooks(flagValueType, hookContext, pairs, Collections.emptyMap()); + hookSupport.beforeHooks(hookContext, pairs, Collections.emptyMap()); hookSupport.afterHooks( - flagValueType, hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); - hookSupport.errorHooks(flagValueType, hookContext, new Exception(), pairs, Collections.emptyMap()); + hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); + hookSupport.errorHooks(hookContext, new Exception(), pairs, Collections.emptyMap()); hookSupport.afterAllHooks( - flagValueType, hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); + hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); } private Object createDefaultValue(FlagValueType flagValueType) { From 5fb278bd86f69373f515ccbbd69d51af75210410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Tue, 23 Sep 2025 14:25:06 +0200 Subject: [PATCH 03/21] feat: hook executor impl (WIP) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/HookContext.java | 252 ++++++++++++++++-- .../openfeature/sdk/HookContextDecorator.java | 23 -- .../openfeature/sdk/HookContextWithData.java | 21 -- .../dev/openfeature/sdk/HookExecutor.java | 108 ++++++++ .../java/dev/openfeature/sdk/HookSupport.java | 134 ---------- .../openfeature/sdk/OpenFeatureClient.java | 39 ++- .../openfeature/sdk/SharedHookContext.java | 26 ++ .../dev/openfeature/sdk/HookSupportTest.java | 20 +- 8 files changed, 394 insertions(+), 229 deletions(-) delete mode 100644 src/main/java/dev/openfeature/sdk/HookContextDecorator.java delete mode 100644 src/main/java/dev/openfeature/sdk/HookContextWithData.java create mode 100644 src/main/java/dev/openfeature/sdk/HookExecutor.java delete mode 100644 src/main/java/dev/openfeature/sdk/HookSupport.java create mode 100644 src/main/java/dev/openfeature/sdk/SharedHookContext.java diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index 2cb58e1e1..ab94657b5 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -1,29 +1,41 @@ package dev.openfeature.sdk; -import lombok.Builder; -import lombok.Data; import lombok.NonNull; -import lombok.With; /** * A data class to hold immutable context that {@link Hook} instances use. * * @param the type for the flag being evaluated */ -@Data -@Builder -@With -public class HookContext { - @NonNull String flagKey; +public final class HookContext { + @NonNull + private final String flagKey; - @NonNull FlagValueType type; + @NonNull + private final FlagValueType type; - @NonNull T defaultValue; + @NonNull + private final T defaultValue; - @NonNull EvaluationContext ctx; + @NonNull + private EvaluationContext ctx; - ClientMetadata clientMetadata; - Metadata providerMetadata; + private final ClientMetadata clientMetadata; + private final Metadata providerMetadata; + + private final HookData hookData; + + HookContext(@NonNull String flagKey, @NonNull FlagValueType type, @NonNull T defaultValue, + @NonNull EvaluationContext ctx, ClientMetadata clientMetadata, Metadata providerMetadata, + HookData hookData) { + this.flagKey = flagKey; + this.type = type; + this.defaultValue = defaultValue; + this.ctx = ctx; + this.clientMetadata = clientMetadata; + this.providerMetadata = providerMetadata; + this.hookData = hookData; + } /** * Builds {@link HookContext} instances from request data. @@ -51,10 +63,220 @@ public static HookContext from( .providerMetadata(providerMetadata) .ctx(ctx) .defaultValue(defaultValue) + .hookData(null) .build(); } - HookData getHookData() { - return null; + public static HookContextBuilder builder() {return new HookContextBuilder();} + + public @NonNull String getFlagKey() { + return this.flagKey; + } + + public @NonNull FlagValueType getType() { + return this.type; + } + + public @NonNull T getDefaultValue() { + return this.defaultValue; + } + + public @NonNull EvaluationContext getCtx() { + return this.ctx; + } + + public ClientMetadata getClientMetadata() { + return this.clientMetadata; + } + + public Metadata getProviderMetadata() { + return this.providerMetadata; + } + + public HookData getHookData() { + return this.hookData; + } + + @Override + public boolean equals(final Object o) { + if (o == this) { + return true; + } + if (!(o instanceof HookContext)) { + return false; + } + final HookContext other = (HookContext) o; + final Object this$flagKey = this.getFlagKey(); + final Object other$flagKey = other.getFlagKey(); + if (this$flagKey == null ? other$flagKey != null : !this$flagKey.equals(other$flagKey)) { + return false; + } + final Object this$type = this.getType(); + final Object other$type = other.getType(); + if (this$type == null ? other$type != null : !this$type.equals(other$type)) { + return false; + } + final Object this$defaultValue = this.getDefaultValue(); + final Object other$defaultValue = other.getDefaultValue(); + if (this$defaultValue == null ? other$defaultValue != null : !this$defaultValue.equals(other$defaultValue)) { + return false; + } + final Object this$ctx = this.getCtx(); + final Object other$ctx = other.getCtx(); + if (this$ctx == null ? other$ctx != null : !this$ctx.equals(other$ctx)) { + return false; + } + final Object this$clientMetadata = this.getClientMetadata(); + final Object other$clientMetadata = other.getClientMetadata(); + if (this$clientMetadata == null + ? other$clientMetadata != null + : !this$clientMetadata.equals(other$clientMetadata)) { + return false; + } + final Object this$providerMetadata = this.getProviderMetadata(); + final Object other$providerMetadata = other.getProviderMetadata(); + if (this$providerMetadata == null + ? other$providerMetadata != null + : !this$providerMetadata.equals(other$providerMetadata)) { + return false; + } + final Object this$hookData = this.getHookData(); + final Object other$hookData = other.getHookData(); + if (this$hookData == null ? other$hookData != null : !this$hookData.equals(other$hookData)) { + return false; + } + return true; + } + + @Override + public int hashCode() { + final int PRIME = 59; + int result = 1; + final Object $flagKey = this.getFlagKey(); + result = result * PRIME + ($flagKey == null ? 43 : $flagKey.hashCode()); + final Object $type = this.getType(); + result = result * PRIME + ($type == null ? 43 : $type.hashCode()); + final Object $defaultValue = this.getDefaultValue(); + result = result * PRIME + ($defaultValue == null ? 43 : $defaultValue.hashCode()); + final Object $ctx = this.getCtx(); + result = result * PRIME + ($ctx == null ? 43 : $ctx.hashCode()); + final Object $clientMetadata = this.getClientMetadata(); + result = result * PRIME + ($clientMetadata == null ? 43 : $clientMetadata.hashCode()); + final Object $providerMetadata = this.getProviderMetadata(); + result = result * PRIME + ($providerMetadata == null ? 43 : $providerMetadata.hashCode()); + final Object $hookData = this.getHookData(); + result = result * PRIME + ($hookData == null ? 43 : $hookData.hashCode()); + return result; + } + + @Override + public String toString() { + return "HookContext(flagKey=" + this.getFlagKey() + ", type=" + this.getType() + ", defaultValue=" + + this.getDefaultValue() + ", ctx=" + this.getCtx() + ", clientMetadata=" + this.getClientMetadata() + + ", providerMetadata=" + this.getProviderMetadata() + ", hookData=" + this.getHookData() + ")"; + } + + void setCtx(@NonNull EvaluationContext ctx) { + this.ctx = ctx; + } + + public HookContext withFlagKey(@NonNull String flagKey) { + return this.flagKey == flagKey ? this + : new HookContext(flagKey, this.type, this.defaultValue, this.ctx, this.clientMetadata, + this.providerMetadata, this.hookData); + } + + public HookContext withType(@NonNull FlagValueType type) { + return this.type == type ? this + : new HookContext(this.flagKey, type, this.defaultValue, this.ctx, this.clientMetadata, + this.providerMetadata, this.hookData); + } + + public HookContext withDefaultValue(@NonNull T defaultValue) { + return this.defaultValue == defaultValue ? this + : new HookContext(this.flagKey, this.type, defaultValue, this.ctx, this.clientMetadata, + this.providerMetadata, this.hookData); + } + + public HookContext withCtx(@NonNull EvaluationContext ctx) { + return this.ctx == ctx ? this + : new HookContext(this.flagKey, this.type, this.defaultValue, ctx, this.clientMetadata, + this.providerMetadata, this.hookData); + } + + public HookContext withClientMetadata(ClientMetadata clientMetadata) { + return this.clientMetadata == clientMetadata ? this + : new HookContext(this.flagKey, this.type, this.defaultValue, this.ctx, clientMetadata, + this.providerMetadata, this.hookData); + } + + public HookContext withProviderMetadata(Metadata providerMetadata) { + return this.providerMetadata == providerMetadata ? this + : new HookContext(this.flagKey, this.type, this.defaultValue, this.ctx, this.clientMetadata, + providerMetadata, this.hookData); + } + + public HookContext withHookData(HookData hookData) { + return this.hookData == hookData ? this + : new HookContext(this.flagKey, this.type, this.defaultValue, this.ctx, this.clientMetadata, + this.providerMetadata, hookData); + } + + public static class HookContextBuilder { + private @NonNull String flagKey; + private @NonNull FlagValueType type; + private @NonNull T defaultValue; + private @NonNull EvaluationContext ctx; + private ClientMetadata clientMetadata; + private Metadata providerMetadata; + private HookData hookData; + + HookContextBuilder() {} + + public HookContextBuilder flagKey(@NonNull String flagKey) { + this.flagKey = flagKey; + return this; + } + + public HookContextBuilder type(@NonNull FlagValueType type) { + this.type = type; + return this; + } + + public HookContextBuilder defaultValue(@NonNull T defaultValue) { + this.defaultValue = defaultValue; + return this; + } + + public HookContextBuilder ctx(@NonNull EvaluationContext ctx) { + this.ctx = ctx; + return this; + } + + public HookContextBuilder clientMetadata(ClientMetadata clientMetadata) { + this.clientMetadata = clientMetadata; + return this; + } + + public HookContextBuilder providerMetadata(Metadata providerMetadata) { + this.providerMetadata = providerMetadata; + return this; + } + + public HookContextBuilder hookData(HookData hookData) { + this.hookData = hookData; + return this; + } + + public HookContext build() { + return new HookContext(this.flagKey, this.type, this.defaultValue, this.ctx, this.clientMetadata, + this.providerMetadata, this.hookData); + } + + public String toString() { + return "HookContext.HookContextBuilder(flagKey=" + this.flagKey + ", type=" + this.type + ", defaultValue=" + + this.defaultValue + ", ctx=" + this.ctx + ", clientMetadata=" + this.clientMetadata + + ", providerMetadata=" + this.providerMetadata + ", hookData=" + this.hookData + ")"; + } } } diff --git a/src/main/java/dev/openfeature/sdk/HookContextDecorator.java b/src/main/java/dev/openfeature/sdk/HookContextDecorator.java deleted file mode 100644 index e2b228a21..000000000 --- a/src/main/java/dev/openfeature/sdk/HookContextDecorator.java +++ /dev/null @@ -1,23 +0,0 @@ -package dev.openfeature.sdk; - -/** - * A base decorator class for {@link HookContext} that enables dynamic enhancement of its functionality. - * This class wraps an existing {@code HookContext} instance and delegates method calls to it. - * - * @param the type for the flag being evaluated - */ -class HookContextDecorator extends HookContext { - - HookContext decorated; - - protected HookContextDecorator(HookContext context) { - super(context.getFlagKey(), context.getType(), context.getDefaultValue(), context.getCtx(), - context.getClientMetadata(), context.getProviderMetadata()); - this.decorated = context; - } - - @Override - public HookData getHookData() { - return decorated.getHookData(); - } -} diff --git a/src/main/java/dev/openfeature/sdk/HookContextWithData.java b/src/main/java/dev/openfeature/sdk/HookContextWithData.java deleted file mode 100644 index 56cd501d4..000000000 --- a/src/main/java/dev/openfeature/sdk/HookContextWithData.java +++ /dev/null @@ -1,21 +0,0 @@ -package dev.openfeature.sdk; - -/** - * A concrete decorator for {@link HookContext} that adds {@link HookData} to the existing functionality. - * - * @param the type for the flag being evaluated - */ -public class HookContextWithData extends HookContextDecorator { - - private final HookData hookData; - - public HookContextWithData(HookContext context, HookData data) { - super(context); - this.hookData = data; - } - - @Override - public HookData getHookData() { - return hookData; - } -} diff --git a/src/main/java/dev/openfeature/sdk/HookExecutor.java b/src/main/java/dev/openfeature/sdk/HookExecutor.java new file mode 100644 index 000000000..4de19f884 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookExecutor.java @@ -0,0 +1,108 @@ +package dev.openfeature.sdk; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +class HookExecutor { + + private List> hooks; + private EvaluationContext evaluationContext; + private final Map hints; + + private HookExecutor( + List> hooks, EvaluationContext evaluationContext, Map hints) { + this.hooks = hooks; + this.evaluationContext = evaluationContext; + this.hints = hints; + } + + public static HookExecutor create( + List hooks, + SharedHookContext sharedContext, + EvaluationContext evaluationContext, + Map hints) { + List> hookContextPairs = new ArrayList<>(); + for (Hook hook : hooks) { + if (hook.supportsFlagValueType(sharedContext.getType())) { + HookContext curContext = sharedContext.hookContextFor(evaluationContext, HookData.create()); + hookContextPairs.add(Pair.of(hook, curContext)); + } + } + return new HookExecutor(hookContextPairs, evaluationContext, hints); + } + + public EvaluationContext getEvaluationContext() { + return evaluationContext; + } + + private void setEvaluationContext(EvaluationContext evaluationContext) { + this.evaluationContext = evaluationContext; + for (Pair hookContextPair : hooks) { + hookContextPair.getValue().setCtx(evaluationContext); + } + } + + public void executeBeforeHooks() { + // These traverse backwards from normal. + List> reversedHooks = new ArrayList<>(hooks); + Collections.reverse(reversedHooks); + + for (Pair hookContextPair : reversedHooks) { + var hook = hookContextPair.getKey(); + var hookContext = hookContextPair.getValue(); + + Optional returnedEvalContext = + Optional.ofNullable(hook.before(hookContext, hints)).orElse(Optional.empty()); + if (returnedEvalContext.isPresent()) { + // update shared evaluation context for all hooks + setEvaluationContext(evaluationContext.merge(returnedEvalContext.get())); + } + } + } + + public void executeErrorHooks(Exception error) { + for (Pair hookContextPair : hooks) { + var hook = hookContextPair.getKey(); + var hookContext = hookContextPair.getValue(); + try { + hook.error(hookContext, error, hints); + } catch (Exception e) { + log.error( + "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", + "error", + hook.getClass(), + e); + } + } + } + + // after hooks can throw in order to do validation + public void executeAfterHooks(FlagEvaluationDetails details) { + for (Pair hookContextPair : hooks) { + var hook = hookContextPair.getKey(); + var hookContext = hookContextPair.getValue(); + hook.after(hookContext, details, hints); + } + } + + public void executeAfterAllHooks(FlagEvaluationDetails details) { + for (Pair hookContextPair : hooks) { + var hook = hookContextPair.getKey(); + var hookContext = hookContextPair.getValue(); + try { + hook.finallyAfter(hookContext, details, hints); + } catch (Exception e) { + log.error( + "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", + "finally", + hook.getClass(), + e); + } + } + } +} diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java deleted file mode 100644 index 4acbba28d..000000000 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ /dev/null @@ -1,134 +0,0 @@ -package dev.openfeature.sdk; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.function.BiConsumer; -import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; - -@Slf4j -@RequiredArgsConstructor -@SuppressWarnings({"unchecked", "rawtypes"}) -class HookSupport { - - public EvaluationContext beforeHooks( - HookContext hookCtx, - List> hookDataPairs, - Map hints) { - return callBeforeHooks(hookCtx, hookDataPairs, hints); - } - - public void afterHooks( - HookContext hookContext, - FlagEvaluationDetails details, - List> hookDataPairs, - Map hints) { - executeHooksUnchecked( - hookDataPairs, hookContext, (hook, ctx) -> hook.after(ctx, details, hints)); - } - - public void afterAllHooks( - HookContext hookCtx, - FlagEvaluationDetails details, - List> hookDataPairs, - Map hints) { - executeHooks( - hookDataPairs, - hookCtx, - "finally", - (hook, ctx) -> hook.finallyAfter(ctx, details, hints)); - } - - public void errorHooks( - HookContext hookCtx, - Exception e, - List> hookDataPairs, - Map hints) { - executeHooks(hookDataPairs, hookCtx, "error", (hook, ctx) -> hook.error(ctx, e, hints)); - } - - public List> getHookDataPairs(List hooks, FlagValueType flagValueType) { - var pairs = new ArrayList>(); - for (Hook hook : hooks) { - if (hook.supportsFlagValueType(flagValueType)) { - pairs.add(Pair.of(hook, HookData.create())); - } - } - return pairs; - } - - private void executeHooks( - List> hookDataPairs, - HookContext hookContext, - String hookMethod, - BiConsumer, HookContext> hookCode) { - if (hookDataPairs != null) { - for (Pair hookDataPair : hookDataPairs) { - Hook hook = hookDataPair.getKey(); - HookData hookData = hookDataPair.getValue(); - executeChecked(hook, hookData, hookContext, hookCode, hookMethod); - } - } - } - - // before, error, and finally hooks shouldn't throw - private void executeChecked( - Hook hook, - HookData hookData, - HookContext hookContext, - BiConsumer, HookContext> hookCode, - String hookMethod) { - try { - var hookCtxWithData = new HookContextWithData(hookContext, hookData); - hookCode.accept(hook, hookCtxWithData); - } catch (Exception exception) { - log.error( - "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", - hookMethod, - hook.getClass(), - exception); - } - } - - // after hooks can throw in order to do validation - private void executeHooksUnchecked( - List> hookDataPairs, - HookContext hookContext, - BiConsumer, HookContext> hookCode) { - if (hookDataPairs != null) { - for (Pair hookDataPair : hookDataPairs) { - Hook hook = hookDataPair.getKey(); - HookData hookData = hookDataPair.getValue(); - var hookCtxWithData = new HookContextWithData(hookContext, hookData); - hookCode.accept(hook, hookCtxWithData); - } - } - } - - private EvaluationContext callBeforeHooks( - HookContext hookCtx, - List> hookDataPairs, - Map hints) { - // These traverse backwards from normal. - List> reversedHooks = new ArrayList<>(hookDataPairs); - Collections.reverse(reversedHooks); - EvaluationContext context = hookCtx.getCtx(); - - for (Pair hookDataPair : reversedHooks) { - Hook hook = hookDataPair.getKey(); - HookData hookData = hookDataPair.getValue(); - - // Create a new context with this hook's data - HookContext contextWithHookData = new HookContextWithData(hookCtx, hookData); - Optional optional = - Optional.ofNullable(hook.before(contextWithHookData, hints)).orElse(Optional.empty()); - if (optional.isPresent()) { - context = context.merge(optional.get()); - } - } - return context; - } -} diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index b35193a24..faea6b5d3 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -48,7 +48,6 @@ public class OpenFeatureClient implements Client { private final String version; private final ConcurrentLinkedQueue clientHooks; - private final HookSupport hookSupport; private final AtomicReference evaluationContext = new AtomicReference<>(); /** @@ -68,7 +67,6 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String ve this.domain = domain; this.version = version; this.clientHooks = new ConcurrentLinkedQueue<>(); - this.hookSupport = new HookSupport(); } /** @@ -164,27 +162,24 @@ private FlagEvaluationDetails evaluateFlag( var hints = Collections.unmodifiableMap(flagOptions.getHookHints()); FlagEvaluationDetails details = null; - List mergedHooks; - List> hookDataPairs = null; - HookContext hookContext = null; + HookExecutor hookExecutor = null; try { final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); // provider must be accessed once to maintain a consistent reference final var provider = stateManager.getProvider(); final var state = stateManager.getState(); - hookContext = HookContext.from( - key, type, this.getMetadata(), provider.getMetadata(), ImmutableContext.EMPTY, defaultValue); - // we are setting the evaluation context one after the other, so that we have a hook context in each - // possible exception case. - hookContext.setCtx(mergeEvaluationContext(ctx)); - - mergedHooks = ObjectUtils.merge( + var mergedHooks = ObjectUtils.merge( provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks()); - hookDataPairs = hookSupport.getHookDataPairs(mergedHooks, type); - var mergedCtx = hookSupport.beforeHooks(hookContext, hookDataPairs, hints); - hookContext.setCtx(mergedCtx); + + var sharedHookContext = + new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue); + + var evalContext = mergeEvaluationContext(ctx); + hookExecutor = HookExecutor.create(mergedHooks, sharedHookContext, evalContext, hints); + + hookExecutor.executeBeforeHooks(); // "short circuit" if the provider is in NOT_READY or FATAL state if (ProviderState.NOT_READY.equals(state)) { @@ -194,17 +189,17 @@ private FlagEvaluationDetails evaluateFlag( throw new FatalError("Provider is in an irrecoverable error state"); } - var providerEval = - (ProviderEvaluation) createProviderEvaluation(type, key, defaultValue, provider, mergedCtx); + var providerEval = (ProviderEvaluation) + createProviderEvaluation(type, key, defaultValue, provider, hookExecutor.getEvaluationContext()); details = FlagEvaluationDetails.from(providerEval, key); if (details.getErrorCode() != null) { var error = ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(hookContext, error, hookDataPairs, hints); + hookExecutor.executeErrorHooks(error); } else { - hookSupport.afterHooks(hookContext, details, hookDataPairs, hints); + hookExecutor.executeAfterHooks(details); } } catch (Exception e) { if (details == null) { @@ -217,9 +212,11 @@ private FlagEvaluationDetails evaluateFlag( } details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(hookContext, e, hookDataPairs, hints); + hookExecutor.executeErrorHooks(e); } finally { - hookSupport.afterAllHooks(hookContext, details, hookDataPairs, hints); + if (hookExecutor != null) { + hookExecutor.executeAfterAllHooks(details); + } } return details; diff --git a/src/main/java/dev/openfeature/sdk/SharedHookContext.java b/src/main/java/dev/openfeature/sdk/SharedHookContext.java new file mode 100644 index 000000000..d5d59e62f --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/SharedHookContext.java @@ -0,0 +1,26 @@ +package dev.openfeature.sdk; + +import lombok.Getter; + +@Getter +class SharedHookContext { + + private final String key; + private final FlagValueType type; + private final ClientMetadata clientMetadata; + private final Metadata providerMetadata; + private final T defaultValue; + + public SharedHookContext( + String key, FlagValueType type, ClientMetadata clientMetadata, Metadata providerMetadata, T defaultValue) { + this.key = key; + this.type = type; + this.clientMetadata = clientMetadata; + this.providerMetadata = providerMetadata; + this.defaultValue = defaultValue; + } + + public HookContext hookContextFor(EvaluationContext evaluationContext, HookData hookData) { + return new HookContext<>(this, evaluationContext, hookData); + } +} diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index ec202a0bc..87a271af1 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -69,15 +69,9 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { hookSupport.beforeHooks(hookContext, hookDataPairs, Collections.emptyMap()); hookSupport.afterHooks( - hookContext, - FlagEvaluationDetails.builder().build(), - hookDataPairs, - Collections.emptyMap()); + hookContext, FlagEvaluationDetails.builder().build(), hookDataPairs, Collections.emptyMap()); hookSupport.afterAllHooks( - hookContext, - FlagEvaluationDetails.builder().build(), - hookDataPairs, - Collections.emptyMap()); + hookContext, FlagEvaluationDetails.builder().build(), hookDataPairs, Collections.emptyMap()); hookSupport.errorHooks(hookContext, expectedException, hookDataPairs, Collections.emptyMap()); verify(genericHook).before(any(), any()); @@ -169,15 +163,11 @@ private static void callAllHooks( } private static void callAllHooks( - HookSupport hookSupport, - HookContext hookContext, - List> pairs) { + HookSupport hookSupport, HookContext hookContext, List> pairs) { hookSupport.beforeHooks(hookContext, pairs, Collections.emptyMap()); - hookSupport.afterHooks( - hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); + hookSupport.afterHooks(hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); hookSupport.errorHooks(hookContext, new Exception(), pairs, Collections.emptyMap()); - hookSupport.afterAllHooks( - hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); + hookSupport.afterAllHooks(hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); } private Object createDefaultValue(FlagValueType flagValueType) { From ceedffd3182e876d98a6e18f89bb04beb83b7277 Mon Sep 17 00:00:00 2001 From: Guido Breitenhuber Date: Tue, 23 Sep 2025 15:54:49 +0200 Subject: [PATCH 04/21] Use shared hook context Signed-off-by: Guido Breitenhuber --- .../java/dev/openfeature/sdk/HookContext.java | 317 +++++++++++------- .../java/dev/openfeature/sdk/HookData.java | 15 + .../openfeature/sdk/SharedHookContext.java | 29 +- 3 files changed, 244 insertions(+), 117 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index ab94657b5..770b65038 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -1,5 +1,6 @@ package dev.openfeature.sdk; +import java.util.Objects; import lombok.NonNull; /** @@ -8,35 +9,43 @@ * @param the type for the flag being evaluated */ public final class HookContext { - @NonNull - private final String flagKey; - - @NonNull - private final FlagValueType type; - - @NonNull - private final T defaultValue; - - @NonNull + private final SharedHookContext sharedContext; private EvaluationContext ctx; - - private final ClientMetadata clientMetadata; - private final Metadata providerMetadata; - private final HookData hookData; - HookContext(@NonNull String flagKey, @NonNull FlagValueType type, @NonNull T defaultValue, - @NonNull EvaluationContext ctx, ClientMetadata clientMetadata, Metadata providerMetadata, + HookContext( + @NonNull SharedHookContext sharedContext, + @NonNull EvaluationContext evaluationContext, HookData hookData) { - this.flagKey = flagKey; - this.type = type; - this.defaultValue = defaultValue; - this.ctx = ctx; - this.clientMetadata = clientMetadata; - this.providerMetadata = providerMetadata; + this.sharedContext = sharedContext; + ctx = evaluationContext; this.hookData = hookData; } + /** + * Obsolete constructor. + * This constructor is retained for binary compatibility but is no longer part of the public API. + * + * @param flagKey feature flag key + * @param type flag value type + * @param clientMetadata info on which client is calling + * @param providerMetadata info on the provider + * @param ctx Evaluation Context for the request + * @param defaultValue Fallback value + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated + HookContext( + @NonNull String flagKey, + @NonNull FlagValueType type, + @NonNull T defaultValue, + @NonNull EvaluationContext ctx, + ClientMetadata clientMetadata, + Metadata providerMetadata, + HookData hookData) { + this(new SharedHookContext<>(flagKey, type, clientMetadata, providerMetadata, defaultValue), ctx, hookData); + } + /** * Builds {@link HookContext} instances from request data. * @@ -48,7 +57,9 @@ public final class HookContext { * @param defaultValue Fallback value * @param type that the flag is evaluating against * @return resulting context for hook + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Deprecated public static HookContext from( String key, FlagValueType type, @@ -67,18 +78,28 @@ public static HookContext from( .build(); } - public static HookContextBuilder builder() {return new HookContextBuilder();} + /** + * Creates a new builder for {@link HookContext}. + * + * @param the type for the flag being evaluated + * @return a new builder + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated + public static HookContextBuilder builder() { + return new HookContextBuilder(); + } public @NonNull String getFlagKey() { - return this.flagKey; + return sharedContext.getFlagKey(); } public @NonNull FlagValueType getType() { - return this.type; + return sharedContext.getType(); } public @NonNull T getDefaultValue() { - return this.defaultValue; + return sharedContext.getDefaultValue(); } public @NonNull EvaluationContext getCtx() { @@ -86,11 +107,11 @@ public static HookContext from( } public ClientMetadata getClientMetadata() { - return this.clientMetadata; + return sharedContext.getClientMetadata(); } public Metadata getProviderMetadata() { - return this.providerMetadata; + return sharedContext.getProviderMetadata(); } public HookData getHookData() { @@ -98,75 +119,19 @@ public HookData getHookData() { } @Override - public boolean equals(final Object o) { - if (o == this) { - return true; - } - if (!(o instanceof HookContext)) { - return false; - } - final HookContext other = (HookContext) o; - final Object this$flagKey = this.getFlagKey(); - final Object other$flagKey = other.getFlagKey(); - if (this$flagKey == null ? other$flagKey != null : !this$flagKey.equals(other$flagKey)) { - return false; - } - final Object this$type = this.getType(); - final Object other$type = other.getType(); - if (this$type == null ? other$type != null : !this$type.equals(other$type)) { - return false; - } - final Object this$defaultValue = this.getDefaultValue(); - final Object other$defaultValue = other.getDefaultValue(); - if (this$defaultValue == null ? other$defaultValue != null : !this$defaultValue.equals(other$defaultValue)) { - return false; - } - final Object this$ctx = this.getCtx(); - final Object other$ctx = other.getCtx(); - if (this$ctx == null ? other$ctx != null : !this$ctx.equals(other$ctx)) { - return false; - } - final Object this$clientMetadata = this.getClientMetadata(); - final Object other$clientMetadata = other.getClientMetadata(); - if (this$clientMetadata == null - ? other$clientMetadata != null - : !this$clientMetadata.equals(other$clientMetadata)) { - return false; - } - final Object this$providerMetadata = this.getProviderMetadata(); - final Object other$providerMetadata = other.getProviderMetadata(); - if (this$providerMetadata == null - ? other$providerMetadata != null - : !this$providerMetadata.equals(other$providerMetadata)) { - return false; - } - final Object this$hookData = this.getHookData(); - final Object other$hookData = other.getHookData(); - if (this$hookData == null ? other$hookData != null : !this$hookData.equals(other$hookData)) { + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { return false; } - return true; + HookContext that = (HookContext) o; + return Objects.equals(ctx, that.ctx) + && Objects.equals(hookData, that.hookData) + && Objects.equals(sharedContext, that.sharedContext); } @Override public int hashCode() { - final int PRIME = 59; - int result = 1; - final Object $flagKey = this.getFlagKey(); - result = result * PRIME + ($flagKey == null ? 43 : $flagKey.hashCode()); - final Object $type = this.getType(); - result = result * PRIME + ($type == null ? 43 : $type.hashCode()); - final Object $defaultValue = this.getDefaultValue(); - result = result * PRIME + ($defaultValue == null ? 43 : $defaultValue.hashCode()); - final Object $ctx = this.getCtx(); - result = result * PRIME + ($ctx == null ? 43 : $ctx.hashCode()); - final Object $clientMetadata = this.getClientMetadata(); - result = result * PRIME + ($clientMetadata == null ? 43 : $clientMetadata.hashCode()); - final Object $providerMetadata = this.getProviderMetadata(); - result = result * PRIME + ($providerMetadata == null ? 43 : $providerMetadata.hashCode()); - final Object $hookData = this.getHookData(); - result = result * PRIME + ($hookData == null ? 43 : $hookData.hashCode()); - return result; + return Objects.hash(ctx, hookData, sharedContext); } @Override @@ -180,48 +145,160 @@ void setCtx(@NonNull EvaluationContext ctx) { this.ctx = ctx; } + /** + * Returns a new HookContext with the provided flagKey if it is different from the current one. + * + * @param flagKey new flag key + * @return new HookContext with updated flagKey or the same instance if unchanged + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated public HookContext withFlagKey(@NonNull String flagKey) { - return this.flagKey == flagKey ? this - : new HookContext(flagKey, this.type, this.defaultValue, this.ctx, this.clientMetadata, - this.providerMetadata, this.hookData); + return Objects.equals(this.getFlagKey(), flagKey) + ? this + : new HookContext( + flagKey, + this.getType(), + this.getDefaultValue(), + this.getCtx(), + this.getClientMetadata(), + this.getProviderMetadata(), + this.hookData); } + /** + * Returns a new HookContext with the provided type if it is different from the current one. + * + * @param type new flag value type + * @return new HookContext with updated type or the same instance if unchanged + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated public HookContext withType(@NonNull FlagValueType type) { - return this.type == type ? this - : new HookContext(this.flagKey, type, this.defaultValue, this.ctx, this.clientMetadata, - this.providerMetadata, this.hookData); + return this.getType() == type + ? this + : new HookContext( + this.getFlagKey(), + type, + this.getDefaultValue(), + this.getCtx(), + this.getClientMetadata(), + this.getProviderMetadata(), + this.hookData); } + /** + * Returns a new HookContext with the provided defaultValue if it is different from the current one. + * + * @param defaultValue new default value + * @return new HookContext with updated defaultValue or the same instance if unchanged + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated public HookContext withDefaultValue(@NonNull T defaultValue) { - return this.defaultValue == defaultValue ? this - : new HookContext(this.flagKey, this.type, defaultValue, this.ctx, this.clientMetadata, - this.providerMetadata, this.hookData); + return this.getDefaultValue() == defaultValue + ? this + : new HookContext( + this.getFlagKey(), + this.getType(), + defaultValue, + this.getCtx(), + this.getClientMetadata(), + this.getProviderMetadata(), + this.hookData); } + /** + * Returns a new HookContext with the provided ctx if it is different from the current one. + * + * @param ctx new evaluation context + * @return new HookContext with updated ctx or the same instance if unchanged + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated public HookContext withCtx(@NonNull EvaluationContext ctx) { - return this.ctx == ctx ? this - : new HookContext(this.flagKey, this.type, this.defaultValue, ctx, this.clientMetadata, - this.providerMetadata, this.hookData); + return this.ctx == ctx + ? this + : new HookContext( + this.getFlagKey(), + this.getType(), + this.getDefaultValue(), + ctx, + this.getClientMetadata(), + this.getProviderMetadata(), + this.hookData); } + /** + * Returns a new HookContext with the provided clientMetadata if it is different from the current one. + * + * @param clientMetadata new client metadata + * @return new HookContext with updated clientMetadata or the same instance if unchanged + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated public HookContext withClientMetadata(ClientMetadata clientMetadata) { - return this.clientMetadata == clientMetadata ? this - : new HookContext(this.flagKey, this.type, this.defaultValue, this.ctx, clientMetadata, - this.providerMetadata, this.hookData); + return this.getClientMetadata() == clientMetadata + ? this + : new HookContext( + this.getFlagKey(), + this.getType(), + this.getDefaultValue(), + this.getCtx(), + clientMetadata, + this.getProviderMetadata(), + this.hookData); } + /** + * Returns a new HookContext with the provided providerMetadata if it is different from the current one. + * + * @param providerMetadata new provider metadata + * @return new HookContext with updated providerMetadata or the same instance if unchanged + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated public HookContext withProviderMetadata(Metadata providerMetadata) { - return this.providerMetadata == providerMetadata ? this - : new HookContext(this.flagKey, this.type, this.defaultValue, this.ctx, this.clientMetadata, - providerMetadata, this.hookData); + return this.getProviderMetadata() == providerMetadata + ? this + : new HookContext( + this.getFlagKey(), + this.getType(), + this.getDefaultValue(), + this.getCtx(), + this.getClientMetadata(), + providerMetadata, + this.hookData); } + /** + * Returns a new HookContext with the provided hookData if it is different from the current one. + * + * @param hookData new hook data + * @return new HookContext with updated hookData or the same instance if unchanged + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated public HookContext withHookData(HookData hookData) { - return this.hookData == hookData ? this - : new HookContext(this.flagKey, this.type, this.defaultValue, this.ctx, this.clientMetadata, - this.providerMetadata, hookData); + return this.hookData == hookData + ? this + : new HookContext( + this.getFlagKey(), + this.getType(), + this.getDefaultValue(), + this.getCtx(), + this.getClientMetadata(), + this.getProviderMetadata(), + hookData); } + /** + * Builder for HookContext. + * + * @param The flag type. + * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. + */ + @Deprecated public static class HookContextBuilder { private @NonNull String flagKey; private @NonNull FlagValueType type; @@ -268,11 +345,23 @@ public HookContextBuilder hookData(HookData hookData) { return this; } + /** + * Builds the HookContext instance. + * + * @return a new HookContext + */ public HookContext build() { - return new HookContext(this.flagKey, this.type, this.defaultValue, this.ctx, this.clientMetadata, - this.providerMetadata, this.hookData); + return new HookContext( + this.flagKey, + this.type, + this.defaultValue, + this.ctx, + this.clientMetadata, + this.providerMetadata, + this.hookData); } + @Override public String toString() { return "HookContext.HookContextBuilder(flagKey=" + this.flagKey + ", type=" + this.type + ", defaultValue=" + this.defaultValue + ", ctx=" + this.ctx + ", clientMetadata=" + this.clientMetadata diff --git a/src/main/java/dev/openfeature/sdk/HookData.java b/src/main/java/dev/openfeature/sdk/HookData.java index c7c644a93..8b1ee2979 100644 --- a/src/main/java/dev/openfeature/sdk/HookData.java +++ b/src/main/java/dev/openfeature/sdk/HookData.java @@ -2,6 +2,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Objects; /** * Hook data provides a way for hooks to maintain state across their execution stages. @@ -77,5 +78,19 @@ public T get(String key, Class type) { } return type.cast(value); } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + DefaultHookData that = (DefaultHookData) o; + return Objects.equals(data, that.data); + } + + @Override + public int hashCode() { + return Objects.hashCode(data); + } } } diff --git a/src/main/java/dev/openfeature/sdk/SharedHookContext.java b/src/main/java/dev/openfeature/sdk/SharedHookContext.java index d5d59e62f..7e7804fc9 100644 --- a/src/main/java/dev/openfeature/sdk/SharedHookContext.java +++ b/src/main/java/dev/openfeature/sdk/SharedHookContext.java @@ -1,19 +1,24 @@ package dev.openfeature.sdk; +import java.util.Objects; import lombok.Getter; @Getter class SharedHookContext { - private final String key; + private final String flagKey; private final FlagValueType type; private final ClientMetadata clientMetadata; private final Metadata providerMetadata; private final T defaultValue; public SharedHookContext( - String key, FlagValueType type, ClientMetadata clientMetadata, Metadata providerMetadata, T defaultValue) { - this.key = key; + String flagKey, + FlagValueType type, + ClientMetadata clientMetadata, + Metadata providerMetadata, + T defaultValue) { + this.flagKey = flagKey; this.type = type; this.clientMetadata = clientMetadata; this.providerMetadata = providerMetadata; @@ -23,4 +28,22 @@ public SharedHookContext( public HookContext hookContextFor(EvaluationContext evaluationContext, HookData hookData) { return new HookContext<>(this, evaluationContext, hookData); } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + SharedHookContext that = (SharedHookContext) o; + return Objects.equals(flagKey, that.flagKey) + && type == that.type + && Objects.equals(clientMetadata, that.clientMetadata) + && Objects.equals(providerMetadata, that.providerMetadata) + && Objects.equals(defaultValue, that.defaultValue); + } + + @Override + public int hashCode() { + return Objects.hash(flagKey, type, clientMetadata, providerMetadata, defaultValue); + } } From 2843298b0e7cf0486b620aba307b883820c784c7 Mon Sep 17 00:00:00 2001 From: Guido Breitenhuber Date: Tue, 23 Sep 2025 15:57:41 +0200 Subject: [PATCH 05/21] Split HookData interface and implementation Signed-off-by: Guido Breitenhuber --- .../dev/openfeature/sdk/DefaultHookData.java | 54 ++++++++++++++++ .../java/dev/openfeature/sdk/HookData.java | 61 ------------------- .../dev/openfeature/sdk/HookExecutor.java | 3 +- 3 files changed, 55 insertions(+), 63 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/DefaultHookData.java diff --git a/src/main/java/dev/openfeature/sdk/DefaultHookData.java b/src/main/java/dev/openfeature/sdk/DefaultHookData.java new file mode 100644 index 000000000..31d982c87 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/DefaultHookData.java @@ -0,0 +1,54 @@ +package dev.openfeature.sdk; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +/** + * Default implementation of HookData. + */ +public class DefaultHookData implements HookData { + private Map data; + + @Override + public void set(String key, Object value) { + if (data == null) { + data = new HashMap<>(); + } + data.put(key, value); + } + + @Override + public Object get(String key) { + if (data == null) { + return null; + } + return data.get(key); + } + + @Override + public T get(String key, Class type) { + Object value = get(key); + if (value == null) { + return null; + } + if (!type.isInstance(value)) { + throw new ClassCastException("Value for key '" + key + "' is not of type " + type.getName()); + } + return type.cast(value); + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + dev.openfeature.sdk.DefaultHookData that = (dev.openfeature.sdk.DefaultHookData) o; + return Objects.equals(data, that.data); + } + + @Override + public int hashCode() { + return Objects.hashCode(data); + } +} diff --git a/src/main/java/dev/openfeature/sdk/HookData.java b/src/main/java/dev/openfeature/sdk/HookData.java index 8b1ee2979..bd2c5dba9 100644 --- a/src/main/java/dev/openfeature/sdk/HookData.java +++ b/src/main/java/dev/openfeature/sdk/HookData.java @@ -1,16 +1,11 @@ package dev.openfeature.sdk; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; - /** * Hook data provides a way for hooks to maintain state across their execution stages. * Each hook instance gets its own isolated data store that persists only for the duration * of a single flag evaluation. */ public interface HookData { - /** * Sets a value for the given key. * @@ -37,60 +32,4 @@ public interface HookData { * @throws ClassCastException if the value cannot be cast to the specified type */ T get(String key, Class type); - - /** - * Default implementation uses a HashMap. - */ - static HookData create() { - return new DefaultHookData(); - } - - /** - * Default implementation of HookData. - */ - class DefaultHookData implements HookData { - private Map data; - - @Override - public void set(String key, Object value) { - if (data == null) { - data = new HashMap<>(); - } - data.put(key, value); - } - - @Override - public Object get(String key) { - if (data == null) { - return null; - } - return data.get(key); - } - - @Override - public T get(String key, Class type) { - Object value = get(key); - if (value == null) { - return null; - } - if (!type.isInstance(value)) { - throw new ClassCastException("Value for key '" + key + "' is not of type " + type.getName()); - } - return type.cast(value); - } - - @Override - public boolean equals(Object o) { - if (o == null || getClass() != o.getClass()) { - return false; - } - DefaultHookData that = (DefaultHookData) o; - return Objects.equals(data, that.data); - } - - @Override - public int hashCode() { - return Objects.hashCode(data); - } - } } diff --git a/src/main/java/dev/openfeature/sdk/HookExecutor.java b/src/main/java/dev/openfeature/sdk/HookExecutor.java index 4de19f884..34275dc2d 100644 --- a/src/main/java/dev/openfeature/sdk/HookExecutor.java +++ b/src/main/java/dev/openfeature/sdk/HookExecutor.java @@ -9,7 +9,6 @@ @Slf4j class HookExecutor { - private List> hooks; private EvaluationContext evaluationContext; private final Map hints; @@ -29,7 +28,7 @@ public static HookExecutor create( List> hookContextPairs = new ArrayList<>(); for (Hook hook : hooks) { if (hook.supportsFlagValueType(sharedContext.getType())) { - HookContext curContext = sharedContext.hookContextFor(evaluationContext, HookData.create()); + HookContext curContext = sharedContext.hookContextFor(evaluationContext, new DefaultHookData()); hookContextPairs.add(Pair.of(hook, curContext)); } } From 235abba082f8f923c73a7524e78e326e191fc248 Mon Sep 17 00:00:00 2001 From: Guido Breitenhuber Date: Tue, 23 Sep 2025 17:50:43 +0200 Subject: [PATCH 06/21] Atopted tests Signed-off-by: Guido Breitenhuber --- ...DataTest.java => DefaultHookDataTest.java} | 16 +- .../dev/openfeature/sdk/HookContextTest.java | 48 ---- .../dev/openfeature/sdk/HookExecutorTest.java | 265 ++++++++++++++++++ .../dev/openfeature/sdk/HookSupportTest.java | 238 ---------------- 4 files changed, 273 insertions(+), 294 deletions(-) rename src/test/java/dev/openfeature/sdk/{HookDataTest.java => DefaultHookDataTest.java} (83%) create mode 100644 src/test/java/dev/openfeature/sdk/HookExecutorTest.java delete mode 100644 src/test/java/dev/openfeature/sdk/HookSupportTest.java diff --git a/src/test/java/dev/openfeature/sdk/HookDataTest.java b/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java similarity index 83% rename from src/test/java/dev/openfeature/sdk/HookDataTest.java rename to src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java index eacbeeb78..0d207fd86 100644 --- a/src/test/java/dev/openfeature/sdk/HookDataTest.java +++ b/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java @@ -4,11 +4,11 @@ import org.junit.jupiter.api.Test; -class HookDataTest { +class DefaultHookDataTest { @Test void shouldStoreAndRetrieveValues() { - HookData hookData = HookData.create(); + var hookData = new DefaultHookData(); hookData.set("key1", "value1"); hookData.set("key2", 42); @@ -21,14 +21,14 @@ void shouldStoreAndRetrieveValues() { @Test void shouldReturnNullForMissingKeys() { - HookData hookData = HookData.create(); + var hookData = new DefaultHookData(); assertNull(hookData.get("nonexistent")); } @Test void shouldSupportTypeSafeRetrieval() { - HookData hookData = HookData.create(); + var hookData = new DefaultHookData(); hookData.set("string", "hello"); hookData.set("integer", 123); @@ -41,14 +41,14 @@ void shouldSupportTypeSafeRetrieval() { @Test void shouldReturnNullForMissingKeysWithType() { - HookData hookData = HookData.create(); + var hookData = new DefaultHookData(); assertNull(hookData.get("missing", String.class)); } @Test void shouldThrowClassCastExceptionForWrongType() { - HookData hookData = HookData.create(); + var hookData = new DefaultHookData(); hookData.set("string", "not a number"); @@ -59,7 +59,7 @@ void shouldThrowClassCastExceptionForWrongType() { @Test void shouldOverwriteExistingValues() { - HookData hookData = HookData.create(); + var hookData = new DefaultHookData(); hookData.set("key", "original"); assertEquals("original", hookData.get("key")); @@ -70,7 +70,7 @@ void shouldOverwriteExistingValues() { @Test void shouldSupportNullValues() { - HookData hookData = HookData.create(); + var hookData = new DefaultHookData(); hookData.set("nullKey", null); assertNull(hookData.get("nullKey")); diff --git a/src/test/java/dev/openfeature/sdk/HookContextTest.java b/src/test/java/dev/openfeature/sdk/HookContextTest.java index cf6071ab8..a37ade9d5 100644 --- a/src/test/java/dev/openfeature/sdk/HookContextTest.java +++ b/src/test/java/dev/openfeature/sdk/HookContextTest.java @@ -29,52 +29,4 @@ void metadata_field_is_type_metadata() { "The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters. It has no return value.") @Test void not_applicable_for_dynamic_context() {} - - @Test - void shouldCreateHookContextWithHookData() { - HookContext context = HookContext.builder() - .flagKey("test-flag") - .type(FlagValueType.STRING) - .defaultValue("default") - .ctx(new ImmutableContext()) - .build(); - HookData hookData = HookData.create(); - hookData.set("test", "value"); - - HookContextWithData contextWithData = new HookContextWithData(context, hookData); - - assertNotNull(contextWithData.getHookData()); - assertEquals("value", contextWithData.getHookData().get("test")); - } - - @Test - void shouldCreateHookContextWithoutHookData() { - HookContext context = HookContext.builder() - .flagKey("test-flag") - .type(FlagValueType.STRING) - .defaultValue("default") - .ctx(new ImmutableContext()) - .build(); - - assertNull(context.getHookData()); - } - - @Test - void shouldCreateHookContextWithHookDataUsingWith() { - HookContext originalContext = HookContext.builder() - .flagKey("test-flag") - .type(FlagValueType.STRING) - .defaultValue("default") - .ctx(new ImmutableContext()) - .build(); - - HookData hookData = HookData.create(); - hookData.set("timing", System.currentTimeMillis()); - - HookContext contextWithHookData = new HookContextWithData(originalContext, hookData); - - assertNull(originalContext.getHookData()); - assertNotNull(contextWithHookData.getHookData()); - assertNotNull(contextWithHookData.getHookData().get("timing")); - } } diff --git a/src/test/java/dev/openfeature/sdk/HookExecutorTest.java b/src/test/java/dev/openfeature/sdk/HookExecutorTest.java new file mode 100644 index 000000000..13e424055 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/HookExecutorTest.java @@ -0,0 +1,265 @@ +package dev.openfeature.sdk; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +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; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +class HookExecutorTest implements HookFixtures { + @Test + @DisplayName("should merge EvaluationContexts on before hooks correctly") + void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { + Map attributes = new HashMap<>(); + attributes.put("baseKey", new Value("baseValue")); + EvaluationContext baseContext = new ImmutableContext(attributes); + + Hook hook1 = mockStringHook(); + Hook hook2 = mockStringHook(); + 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()); + + executor.executeBeforeHooks(); + + EvaluationContext result = executor.getEvaluationContext(); + + assertThat(result.getValue("bla").asString()).isEqualTo("blubber"); + assertThat(result.getValue("foo").asString()).isEqualTo("bar"); + assertThat(result.getValue("baseKey").asString()).isEqualTo("baseValue"); + } + + @ParameterizedTest + @EnumSource(value = FlagValueType.class) + @DisplayName("should always call generic hook") + void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { + Hook genericHook = mockGenericHook(); + + HookExecutor hookExecutor = HookExecutor.create(List.of(genericHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); + + callAllHooks(hookExecutor); + + verify(genericHook).before(any(), any()); + verify(genericHook).after(any(), any(), any()); + verify(genericHook).finallyAfter(any(), any(), any()); + verify(genericHook).error(any(), any(), any()); + } + + @ParameterizedTest + @EnumSource(value = FlagValueType.class) + @DisplayName("should allow hooks to store and retrieve data across stages") + void shouldPassDataAcrossStages(FlagValueType flagValueType) { + var testHook = new HookDataHook(); + HookExecutor hookExecutor = HookExecutor.create(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); + + hookExecutor.executeBeforeHooks(); + assertHookData(testHook, "before"); + + hookExecutor.executeAfterHooks(FlagEvaluationDetails.builder().build()); + assertHookData(testHook, "before", "after"); + + hookExecutor.executeAfterAllHooks(FlagEvaluationDetails.builder().build()); + assertHookData(testHook, "before", "after", "finallyAfter"); + + hookExecutor.executeErrorHooks(mock(Exception.class)); + assertHookData(testHook, "before", "after", "finallyAfter", "error"); + } + + @ParameterizedTest + @EnumSource(value = FlagValueType.class) + @DisplayName("should isolate data between different hook instances") + void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { + var testHook1 = new HookDataHook(1); + var testHook2 = new HookDataHook(2); + + HookExecutor hookExecutor = HookExecutor.create(List.of(testHook1, testHook2), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); + + callAllHooks(hookExecutor); + + assertHookData(testHook1, 1, "before", "after", "finallyAfter", "error"); + assertHookData(testHook2, 2, "before", "after", "finallyAfter", "error"); + } + + @ParameterizedTest + @EnumSource(value = FlagValueType.class) + @DisplayName("should isolate data between the same hook executions") + void shouldIsolateDataBetweenSameHookExecutions(FlagValueType flagValueType) { + TestHookWithData testHook = new TestHookWithData("test-key", "value-1"); + + HookExecutor hookExecutor1 = HookExecutor.create(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); + HookExecutor hookExecutor2 = HookExecutor.create(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); + + // run hooks first time + callAllHooks(hookExecutor1); + assertHookData(testHook, "value-1"); + + // re-run with different value, will throw if HookData contains already data + testHook.value = "value-2"; + callAllHooks(hookExecutor2); + assertHookData(testHook, "value-2"); + } + + 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 assertHookData(TestHookWithData testHook1, String expected) { + assertThat(testHook1.onBeforeValue).isEqualTo(expected); + assertThat(testHook1.onFinallyAfterValue).isEqualTo(expected); + assertThat(testHook1.onAfterValue).isEqualTo(expected); + assertThat(testHook1.onErrorValue).isEqualTo(expected); + } + + private static void assertHookData(HookDataHook 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(HookDataHook 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'", + expectedKey, expectedValue, testHook.hookData.get(expectedKey)) + .isEqualTo(expectedValue); + } + } + + private SharedHookContext getBaseHookContextForType(FlagValueType flagValueType) { + return new SharedHookContext<>( + "flagKey", + flagValueType, + () -> "client", + () -> "provider", + createDefaultValue(flagValueType)); + } + + private Object createDefaultValue(FlagValueType flagValueType) { + switch (flagValueType) { + case INTEGER: + return 1; + case BOOLEAN: + return true; + case STRING: + return "defaultValue"; + case OBJECT: + return "object"; + case DOUBLE: + return "double"; + default: + throw new IllegalArgumentException(); + } + } + + private EvaluationContext evaluationContextWithValue(String key, String value) { + Map attributes = new HashMap<>(); + attributes.put(key, new Value(value)); + return new ImmutableContext(attributes); + } + + private static class HookDataHook implements Hook { + private final Object value; + HookData hookData = null; + + public HookDataHook(Object value) { + this.value = value; + } + + public HookDataHook() { + this("test"); + } + + @Override + public Optional before(HookContext ctx, Map hints) { + ctx.getHookData().set("before", value); + hookData = ctx.getHookData(); + return Optional.empty(); + } + + @Override + public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + ctx.getHookData().set("after", value); + hookData = ctx.getHookData(); + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + ctx.getHookData().set("error", value); + hookData = ctx.getHookData(); + } + + @Override + public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { + ctx.getHookData().set("finallyAfter", value); + hookData = ctx.getHookData(); + } + } + + private class TestHookWithData implements Hook { + + private final String key; + Object value; + + Object onBeforeValue; + Object onAfterValue; + Object onErrorValue; + Object onFinallyAfterValue; + + TestHookWithData(Object value) { + this("test", value); + } + + TestHookWithData(String key, Object value) { + this.key = key; + this.value = value; + } + + @Override + public Optional before(HookContext ctx, Map hints) { + var storedValue = ctx.getHookData().get(key); + if (storedValue != null) { + throw new Error("Hook data isolation violated! Data is already set."); + } + ctx.getHookData().set(key, value); + onBeforeValue = ctx.getHookData().get(key); + return Optional.empty(); + } + + @Override + public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + onAfterValue = ctx.getHookData().get(key); + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + onErrorValue = ctx.getHookData().get(key); + } + + @Override + public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { + onFinallyAfterValue = ctx.getHookData().get(key); + } + } +} diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java deleted file mode 100644 index 87a271af1..000000000 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ /dev/null @@ -1,238 +0,0 @@ -package dev.openfeature.sdk; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.verify; -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; -import java.util.Map; -import java.util.Optional; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EnumSource; - -class HookSupportTest implements HookFixtures { - @Test - @DisplayName("should merge EvaluationContexts on before hooks correctly") - void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { - Map attributes = new HashMap<>(); - attributes.put("baseKey", new Value("baseValue")); - EvaluationContext baseContext = new ImmutableContext(attributes); - FlagValueType valueType = FlagValueType.STRING; - HookContext hookContext = HookContext.builder() - .flagKey("flagKey") - .type(valueType) - .defaultValue("defaultValue") - .ctx(baseContext) - .clientMetadata(() -> "client") - .providerMetadata(() -> "provider") - .build(); - Hook hook1 = mockStringHook(); - Hook hook2 = mockStringHook(); - when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber"))); - when(hook2.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("foo", "bar"))); - HookSupport hookSupport = new HookSupport(); - - EvaluationContext result = hookSupport.beforeHooks( - hookContext, - hookSupport.getHookDataPairs(Arrays.asList(hook1, hook2), valueType), - Collections.emptyMap()); - - assertThat(result.getValue("bla").asString()).isEqualTo("blubber"); - assertThat(result.getValue("foo").asString()).isEqualTo("bar"); - assertThat(result.getValue("baseKey").asString()).isEqualTo("baseValue"); - } - - @ParameterizedTest - @EnumSource(value = FlagValueType.class) - @DisplayName("should always call generic hook") - void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { - Hook genericHook = mockGenericHook(); - HookSupport hookSupport = new HookSupport(); - var hookDataPairs = hookSupport.getHookDataPairs(Collections.singletonList(genericHook), flagValueType); - EvaluationContext baseContext = new ImmutableContext(); - IllegalStateException expectedException = new IllegalStateException("All fine, just a test"); - HookContext hookContext = HookContext.builder() - .flagKey("flagKey") - .type(flagValueType) - .defaultValue(createDefaultValue(flagValueType)) - .ctx(baseContext) - .clientMetadata(() -> "client") - .providerMetadata(() -> "provider") - .build(); - - hookSupport.beforeHooks(hookContext, hookDataPairs, Collections.emptyMap()); - hookSupport.afterHooks( - hookContext, FlagEvaluationDetails.builder().build(), hookDataPairs, Collections.emptyMap()); - hookSupport.afterAllHooks( - hookContext, FlagEvaluationDetails.builder().build(), hookDataPairs, Collections.emptyMap()); - hookSupport.errorHooks(hookContext, expectedException, hookDataPairs, Collections.emptyMap()); - - verify(genericHook).before(any(), any()); - verify(genericHook).after(any(), any(), any()); - verify(genericHook).finallyAfter(any(), any(), any()); - verify(genericHook).error(any(), any(), any()); - } - - @ParameterizedTest - @EnumSource(value = FlagValueType.class) - @DisplayName("should allow hooks to store and retrieve data across stages") - void shouldPassDataAcrossStages(FlagValueType flagValueType) { - HookSupport hookSupport = new HookSupport(); - HookContext hookContext = getObjectHookContext(flagValueType); - - TestHookWithData testHook = new TestHookWithData("test-key", "value"); - var pairs = hookSupport.getHookDataPairs(List.of(testHook), flagValueType); - - callAllHooks(flagValueType, hookSupport, hookContext, testHook); - - assertHookData(testHook, "value"); - } - - @ParameterizedTest - @EnumSource(value = FlagValueType.class) - @DisplayName("should isolate data between different hook instances") - void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { - HookSupport hookSupport = new HookSupport(); - HookContext hookContext = getObjectHookContext(flagValueType); - - TestHookWithData testHook1 = new TestHookWithData("test-key", "value-1"); - TestHookWithData testHook2 = new TestHookWithData("test-key", "value-2"); - var pairs = hookSupport.getHookDataPairs(List.of(testHook1, testHook2), flagValueType); - - callAllHooks(hookSupport, hookContext, pairs); - - assertHookData(testHook1, "value-1"); - assertHookData(testHook2, "value-2"); - } - - @ParameterizedTest - @EnumSource(value = FlagValueType.class) - @DisplayName("should isolate data between the same hook instances") - void shouldIsolateDataBetweenSameHooks(FlagValueType flagValueType) { - - HookSupport hookSupport = new HookSupport(); - HookContext hookContext = getObjectHookContext(flagValueType); - - TestHookWithData testHook = new TestHookWithData("test-key", "value-1"); - - // run hooks first time - callAllHooks(flagValueType, hookSupport, hookContext, testHook); - assertHookData(testHook, "value-1"); - - // re-run with different value, will throw if HookData contains already data - testHook.value = "value-2"; - callAllHooks(flagValueType, hookSupport, hookContext, testHook); - - assertHookData(testHook, "value-2"); - } - - private HookContext getObjectHookContext(FlagValueType flagValueType) { - EvaluationContext baseContext = new ImmutableContext(); - HookContext hookContext = HookContext.builder() - .flagKey("flagKey") - .type(flagValueType) - .defaultValue(createDefaultValue(flagValueType)) - .ctx(baseContext) - .clientMetadata(() -> "client") - .providerMetadata(() -> "provider") - .build(); - return hookContext; - } - - private static void assertHookData(TestHookWithData testHook1, String expected) { - assertThat(testHook1.onBeforeValue).isEqualTo(expected); - assertThat(testHook1.onFinallyAfterValue).isEqualTo(expected); - assertThat(testHook1.onAfterValue).isEqualTo(expected); - assertThat(testHook1.onErrorValue).isEqualTo(expected); - } - - private static void callAllHooks( - FlagValueType flagValueType, - HookSupport hookSupport, - HookContext hookContext, - TestHookWithData testHook) { - var pairs = hookSupport.getHookDataPairs(List.of(testHook), flagValueType); - callAllHooks(hookSupport, hookContext, pairs); - } - - private static void callAllHooks( - HookSupport hookSupport, HookContext hookContext, List> pairs) { - hookSupport.beforeHooks(hookContext, pairs, Collections.emptyMap()); - hookSupport.afterHooks(hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); - hookSupport.errorHooks(hookContext, new Exception(), pairs, Collections.emptyMap()); - hookSupport.afterAllHooks(hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap()); - } - - private Object createDefaultValue(FlagValueType flagValueType) { - switch (flagValueType) { - case INTEGER: - return 1; - case BOOLEAN: - return true; - case STRING: - return "defaultValue"; - case OBJECT: - return "object"; - case DOUBLE: - return "double"; - default: - throw new IllegalArgumentException(); - } - } - - private EvaluationContext evaluationContextWithValue(String key, String value) { - Map attributes = new HashMap<>(); - attributes.put(key, new Value(value)); - EvaluationContext baseContext = new ImmutableContext(attributes); - return baseContext; - } - - private class TestHookWithData implements Hook { - - private final String key; - Object value; - - Object onBeforeValue; - Object onAfterValue; - Object onErrorValue; - Object onFinallyAfterValue; - - TestHookWithData(String key, Object value) { - this.key = key; - this.value = value; - } - - @Override - public Optional before(HookContext ctx, Map hints) { - var storedValue = ctx.getHookData().get(key); - if (storedValue != null) { - throw new Error("Hook data isolation violated! Data is already set."); - } - ctx.getHookData().set(key, value); - onBeforeValue = ctx.getHookData().get(key); - return Optional.empty(); - } - - @Override - public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { - onAfterValue = ctx.getHookData().get(key); - } - - @Override - public void error(HookContext ctx, Exception error, Map hints) { - onErrorValue = ctx.getHookData().get(key); - } - - @Override - public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { - onFinallyAfterValue = ctx.getHookData().get(key); - } - } -} From e0935f35e5b4fd77b799b8ae7d39f65e02dc8c71 Mon Sep 17 00:00:00 2001 From: Guido Breitenhuber Date: Tue, 23 Sep 2025 18:55:17 +0200 Subject: [PATCH 07/21] Remove obsolete test Signed-off-by: Guido Breitenhuber --- .../dev/openfeature/sdk/HookExecutorTest.java | 88 ++----------------- 1 file changed, 8 insertions(+), 80 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/HookExecutorTest.java b/src/test/java/dev/openfeature/sdk/HookExecutorTest.java index 13e424055..6179d8d09 100644 --- a/src/test/java/dev/openfeature/sdk/HookExecutorTest.java +++ b/src/test/java/dev/openfeature/sdk/HookExecutorTest.java @@ -63,7 +63,7 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { @EnumSource(value = FlagValueType.class) @DisplayName("should allow hooks to store and retrieve data across stages") void shouldPassDataAcrossStages(FlagValueType flagValueType) { - var testHook = new HookDataHook(); + var testHook = new TestHookWithData(); HookExecutor hookExecutor = HookExecutor.create(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); hookExecutor.executeBeforeHooks(); @@ -83,8 +83,8 @@ void shouldPassDataAcrossStages(FlagValueType flagValueType) { @EnumSource(value = FlagValueType.class) @DisplayName("should isolate data between different hook instances") void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { - var testHook1 = new HookDataHook(1); - var testHook2 = new HookDataHook(2); + var testHook1 = new TestHookWithData(1); + var testHook2 = new TestHookWithData(2); HookExecutor hookExecutor = HookExecutor.create(List.of(testHook1, testHook2), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); @@ -94,25 +94,6 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { assertHookData(testHook2, 2, "before", "after", "finallyAfter", "error"); } - @ParameterizedTest - @EnumSource(value = FlagValueType.class) - @DisplayName("should isolate data between the same hook executions") - void shouldIsolateDataBetweenSameHookExecutions(FlagValueType flagValueType) { - TestHookWithData testHook = new TestHookWithData("test-key", "value-1"); - - HookExecutor hookExecutor1 = HookExecutor.create(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); - HookExecutor hookExecutor2 = HookExecutor.create(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); - - // run hooks first time - callAllHooks(hookExecutor1); - assertHookData(testHook, "value-1"); - - // re-run with different value, will throw if HookData contains already data - testHook.value = "value-2"; - callAllHooks(hookExecutor2); - assertHookData(testHook, "value-2"); - } - private static void callAllHooks(HookExecutor hookExecutor) { hookExecutor.executeBeforeHooks(); hookExecutor.executeAfterHooks(FlagEvaluationDetails.builder().build()); @@ -120,14 +101,7 @@ private static void callAllHooks(HookExecutor hookExecutor) { hookExecutor.executeErrorHooks(mock(Exception.class)); } - private static void assertHookData(TestHookWithData testHook1, String expected) { - assertThat(testHook1.onBeforeValue).isEqualTo(expected); - assertThat(testHook1.onFinallyAfterValue).isEqualTo(expected); - assertThat(testHook1.onAfterValue).isEqualTo(expected); - assertThat(testHook1.onErrorValue).isEqualTo(expected); - } - - private static void assertHookData(HookDataHook 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) @@ -135,7 +109,7 @@ private static void assertHookData(HookDataHook testHook, String ... expectedKey } } - private static void assertHookData(HookDataHook 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) @@ -179,15 +153,15 @@ private EvaluationContext evaluationContextWithValue(String key, String value) { return new ImmutableContext(attributes); } - private static class HookDataHook implements Hook { + private static class TestHookWithData implements Hook { private final Object value; HookData hookData = null; - public HookDataHook(Object value) { + public TestHookWithData(Object value) { this.value = value; } - public HookDataHook() { + public TestHookWithData() { this("test"); } @@ -216,50 +190,4 @@ public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hin hookData = ctx.getHookData(); } } - - private class TestHookWithData implements Hook { - - private final String key; - Object value; - - Object onBeforeValue; - Object onAfterValue; - Object onErrorValue; - Object onFinallyAfterValue; - - TestHookWithData(Object value) { - this("test", value); - } - - TestHookWithData(String key, Object value) { - this.key = key; - this.value = value; - } - - @Override - public Optional before(HookContext ctx, Map hints) { - var storedValue = ctx.getHookData().get(key); - if (storedValue != null) { - throw new Error("Hook data isolation violated! Data is already set."); - } - ctx.getHookData().set(key, value); - onBeforeValue = ctx.getHookData().get(key); - return Optional.empty(); - } - - @Override - public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { - onAfterValue = ctx.getHookData().get(key); - } - - @Override - public void error(HookContext ctx, Exception error, Map hints) { - onErrorValue = ctx.getHookData().get(key); - } - - @Override - public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { - onFinallyAfterValue = ctx.getHookData().get(key); - } - } } From dbfcab60177463c0335cf1e980b6051ac3ced1ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Wed, 24 Sep 2025 16:44:57 +0200 Subject: [PATCH 08/21] HookSupport improvements: rename back to old name, move code from static factory method to ctor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../{HookExecutor.java => HookSupport.java} | 15 ++--- .../openfeature/sdk/OpenFeatureClient.java | 18 +++--- ...ExecutorTest.java => HookSupportTest.java} | 62 +++++++++++-------- 3 files changed, 51 insertions(+), 44 deletions(-) rename src/main/java/dev/openfeature/sdk/{HookExecutor.java => HookSupport.java} (92%) rename src/test/java/dev/openfeature/sdk/{HookExecutorTest.java => HookSupportTest.java} (75%) diff --git a/src/main/java/dev/openfeature/sdk/HookExecutor.java b/src/main/java/dev/openfeature/sdk/HookSupport.java similarity index 92% rename from src/main/java/dev/openfeature/sdk/HookExecutor.java rename to src/main/java/dev/openfeature/sdk/HookSupport.java index 34275dc2d..142cf6d9f 100644 --- a/src/main/java/dev/openfeature/sdk/HookExecutor.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -8,19 +8,12 @@ import lombok.extern.slf4j.Slf4j; @Slf4j -class HookExecutor { +class HookSupport { private List> hooks; private EvaluationContext evaluationContext; private final Map hints; - private HookExecutor( - List> hooks, EvaluationContext evaluationContext, Map hints) { - this.hooks = hooks; - this.evaluationContext = evaluationContext; - this.hints = hints; - } - - public static HookExecutor create( + HookSupport( List 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() { diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index faea6b5d3..ac198028f 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -162,7 +162,7 @@ private FlagEvaluationDetails evaluateFlag( var hints = Collections.unmodifiableMap(flagOptions.getHookHints()); FlagEvaluationDetails details = null; - HookExecutor hookExecutor = null; + HookSupport hookSupport = null; try { final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); @@ -177,9 +177,9 @@ private FlagEvaluationDetails 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 FlagEvaluationDetails evaluateFlag( } var providerEval = (ProviderEvaluation) - 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 FlagEvaluationDetails 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); } } diff --git a/src/test/java/dev/openfeature/sdk/HookExecutorTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java similarity index 75% rename from src/test/java/dev/openfeature/sdk/HookExecutorTest.java rename to src/test/java/dev/openfeature/sdk/HookSupportTest.java index 6179d8d09..b0cd9ea70 100644 --- a/src/test/java/dev/openfeature/sdk/HookExecutorTest.java +++ b/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,22 +97,26 @@ 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) @@ -109,13 +124,14 @@ private static void assertHookData(TestHookWithData testHook, String ... expecte } } - 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); } @@ -123,11 +139,7 @@ private static void assertHookData(TestHookWithData testHook, Object expectedVal 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) { From a3ed93ee2e151978506f42078fa19f6c8e4c9c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Wed, 24 Sep 2025 16:50:25 +0200 Subject: [PATCH 09/21] PR suggestion: use concrete hooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/ObjectHook.java | 15 +++++++++++++++ .../sdk/benchmark/AllocationBenchmark.java | 18 +++++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/ObjectHook.java diff --git a/src/main/java/dev/openfeature/sdk/ObjectHook.java b/src/main/java/dev/openfeature/sdk/ObjectHook.java new file mode 100644 index 000000000..ad3af6444 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/ObjectHook.java @@ -0,0 +1,15 @@ +package dev.openfeature.sdk; + +/** + * An extension point which can run around flag resolution. They are intended to be used as a way to add custom logic + * to the lifecycle of flag evaluation. + * + * @see Hook + */ +public interface ObjectHook extends Hook { + + @Override + default boolean supportsFlagValueType(FlagValueType flagValueType) { + return FlagValueType.OBJECT == flagValueType; + } +} diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index 9fe043722..d6a03efd6 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -6,14 +6,18 @@ import static dev.openfeature.sdk.testutils.TestFlagsUtils.OBJECT_FLAG_KEY; import static dev.openfeature.sdk.testutils.TestFlagsUtils.STRING_FLAG_KEY; +import dev.openfeature.sdk.BooleanHook; import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.DoubleHook; import dev.openfeature.sdk.EvaluationContext; -import dev.openfeature.sdk.Hook; import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.ImmutableContext; import dev.openfeature.sdk.ImmutableStructure; +import dev.openfeature.sdk.IntegerHook; import dev.openfeature.sdk.NoOpProvider; +import dev.openfeature.sdk.ObjectHook; import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.StringHook; import dev.openfeature.sdk.Value; import java.util.HashMap; import java.util.Map; @@ -25,7 +29,7 @@ /** * Runs a large volume of flag evaluations on a VM with 1G memory and GC - * completely disabled so we can take a heap-dump. + * completely disabled, so we can take a heap-dump. */ public class AllocationBenchmark { @@ -48,31 +52,31 @@ public void run() { Map clientAttrs = new HashMap<>(); clientAttrs.put("client", new Value(2)); client.setEvaluationContext(new ImmutableContext(clientAttrs)); - client.addHooks(new Hook() { + client.addHooks(new ObjectHook() { @Override public Optional before(HookContext ctx, Map hints) { return Optional.ofNullable(new ImmutableContext()); } }); - client.addHooks(new Hook() { + client.addHooks(new StringHook() { @Override public Optional before(HookContext ctx, Map hints) { return Optional.ofNullable(new ImmutableContext()); } }); - client.addHooks(new Hook() { + client.addHooks(new BooleanHook() { @Override public Optional before(HookContext ctx, Map hints) { return Optional.ofNullable(new ImmutableContext()); } }); - client.addHooks(new Hook() { + client.addHooks(new IntegerHook() { @Override public Optional before(HookContext ctx, Map hints) { return Optional.ofNullable(new ImmutableContext()); } }); - client.addHooks(new Hook() { + client.addHooks(new DoubleHook() { @Override public Optional before(HookContext ctx, Map hints) { return Optional.ofNullable(new ImmutableContext()); From 3ea98945870a1775399fa7c6dd6f7d77875835b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Wed, 24 Sep 2025 16:56:07 +0200 Subject: [PATCH 10/21] PR suggestions: DefaultHookData access modifier, no star imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- src/main/java/dev/openfeature/sdk/DefaultHookData.java | 2 +- src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/DefaultHookData.java b/src/main/java/dev/openfeature/sdk/DefaultHookData.java index 31d982c87..6ee80d342 100644 --- a/src/main/java/dev/openfeature/sdk/DefaultHookData.java +++ b/src/main/java/dev/openfeature/sdk/DefaultHookData.java @@ -7,7 +7,7 @@ /** * Default implementation of HookData. */ -public class DefaultHookData implements HookData { +class DefaultHookData implements HookData { private Map data; @Override diff --git a/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java b/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java index 0d207fd86..f8fa4d81c 100644 --- a/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java +++ b/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java @@ -1,9 +1,12 @@ package dev.openfeature.sdk; -import static org.junit.jupiter.api.Assertions.*; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + class DefaultHookDataTest { @Test From 27f3e2a44a9359fd4e0d1b38a0abd97fb7e569dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Thu, 25 Sep 2025 14:25:31 +0200 Subject: [PATCH 11/21] feat: separate hook support data from logic, PR suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/HookContext.java | 11 ++-- .../java/dev/openfeature/sdk/HookSupport.java | 61 +++++-------------- .../dev/openfeature/sdk/HookSupportData.java | 45 ++++++++++++++ .../openfeature/sdk/OpenFeatureClient.java | 25 +++++--- .../dev/openfeature/sdk/HookSupportTest.java | 45 ++++++++------ 5 files changed, 111 insertions(+), 76 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/HookSupportData.java diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index 770b65038..86f414b24 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -1,5 +1,6 @@ package dev.openfeature.sdk; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Objects; import lombok.NonNull; @@ -114,6 +115,7 @@ public Metadata getProviderMetadata() { return sharedContext.getProviderMetadata(); } + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "Intentional exposure of hookData") public HookData getHookData() { return this.hookData; } @@ -300,10 +302,10 @@ public HookContext withHookData(HookData hookData) { */ @Deprecated public static class HookContextBuilder { - private @NonNull String flagKey; - private @NonNull FlagValueType type; - private @NonNull T defaultValue; - private @NonNull EvaluationContext ctx; + private String flagKey; + private FlagValueType type; + private T defaultValue; + private EvaluationContext ctx; private ClientMetadata clientMetadata; private Metadata providerMetadata; private HookData hookData; @@ -340,6 +342,7 @@ public HookContextBuilder providerMetadata(Metadata providerMetadata) { return this; } + @SuppressFBWarnings(value = "EI_EXPOSE_REP2", justification = "Intentional exposure of hookData") public HookContextBuilder hookData(HookData hookData) { this.hookData = hookData; return this; diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index 142cf6d9f..1d37e34ab 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -3,68 +3,37 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Optional; import lombok.extern.slf4j.Slf4j; @Slf4j class HookSupport { - private List> hooks; - private EvaluationContext evaluationContext; - private final Map hints; - HookSupport( - List hooks, - SharedHookContext sharedContext, - EvaluationContext evaluationContext, - Map hints) { - List> hookContextPairs = new ArrayList<>(); - for (Hook hook : hooks) { - if (hook.supportsFlagValueType(sharedContext.getType())) { - HookContext curContext = sharedContext.hookContextFor(evaluationContext, new DefaultHookData()); - hookContextPairs.add(Pair.of(hook, curContext)); - } - } - this.hooks = hookContextPairs; - this.evaluationContext = evaluationContext; - this.hints = hints; - } - - public EvaluationContext getEvaluationContext() { - return evaluationContext; - } - - private void setEvaluationContext(EvaluationContext evaluationContext) { - this.evaluationContext = evaluationContext; - for (Pair hookContextPair : hooks) { - hookContextPair.getValue().setCtx(evaluationContext); - } - } - - public void executeBeforeHooks() { + public void executeBeforeHooks(HookSupportData data) { // These traverse backwards from normal. - List> reversedHooks = new ArrayList<>(hooks); + List> reversedHooks = new ArrayList<>(data.getHooks()); Collections.reverse(reversedHooks); for (Pair hookContextPair : reversedHooks) { var hook = hookContextPair.getKey(); var hookContext = hookContextPair.getValue(); - Optional returnedEvalContext = - Optional.ofNullable(hook.before(hookContext, hints)).orElse(Optional.empty()); + Optional returnedEvalContext = Optional.ofNullable( + hook.before(hookContext, data.getHints())) + .orElse(Optional.empty()); if (returnedEvalContext.isPresent()) { // update shared evaluation context for all hooks - setEvaluationContext(evaluationContext.merge(returnedEvalContext.get())); + data.setEvaluationContext(data.getEvaluationContext().merge(returnedEvalContext.get())); } } } - public void executeErrorHooks(Exception error) { - for (Pair hookContextPair : hooks) { + public void executeErrorHooks(HookSupportData data, Exception error) { + for (Pair hookContextPair : data.getHooks()) { var hook = hookContextPair.getKey(); var hookContext = hookContextPair.getValue(); try { - hook.error(hookContext, error, hints); + hook.error(hookContext, error, data.getHints()); } catch (Exception e) { log.error( "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", @@ -76,20 +45,20 @@ public void executeErrorHooks(Exception error) { } // after hooks can throw in order to do validation - public void executeAfterHooks(FlagEvaluationDetails details) { - for (Pair hookContextPair : hooks) { + public void executeAfterHooks(HookSupportData data, FlagEvaluationDetails details) { + for (Pair hookContextPair : data.getHooks()) { var hook = hookContextPair.getKey(); var hookContext = hookContextPair.getValue(); - hook.after(hookContext, details, hints); + hook.after(hookContext, details, data.getHints()); } } - public void executeAfterAllHooks(FlagEvaluationDetails details) { - for (Pair hookContextPair : hooks) { + public void executeAfterAllHooks(HookSupportData data, FlagEvaluationDetails details) { + for (Pair hookContextPair : data.getHooks()) { var hook = hookContextPair.getKey(); var hookContext = hookContextPair.getValue(); try { - hook.finallyAfter(hookContext, details, hints); + hook.finallyAfter(hookContext, details, data.getHints()); } catch (Exception e) { log.error( "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", diff --git a/src/main/java/dev/openfeature/sdk/HookSupportData.java b/src/main/java/dev/openfeature/sdk/HookSupportData.java new file mode 100644 index 000000000..9949d63e8 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookSupportData.java @@ -0,0 +1,45 @@ +package dev.openfeature.sdk; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import lombok.Getter; + +/** + * Encapsulates data for hook execution per flag evaluation. + */ +@Getter +class HookSupportData { + + private List> hooks; + private EvaluationContext evaluationContext; + private Map hints; + private boolean isInitialized = false; + + HookSupportData() {} + + void initialize( + List hooks, + SharedHookContext sharedContext, + EvaluationContext evaluationContext, + Map hints) { + List> hookContextPairs = new ArrayList<>(); + for (Hook hook : hooks) { + if (hook.supportsFlagValueType(sharedContext.getType())) { + HookContext curContext = sharedContext.hookContextFor(evaluationContext, new DefaultHookData()); + hookContextPairs.add(Pair.of(hook, curContext)); + } + } + this.hooks = hookContextPairs; + this.evaluationContext = evaluationContext; + this.hints = hints; + isInitialized = true; + } + + public void setEvaluationContext(EvaluationContext evaluationContext) { + this.evaluationContext = evaluationContext; + for (Pair hookContextPair : hooks) { + hookContextPair.getValue().setCtx(evaluationContext); + } + } +} diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index ac198028f..4f789e8b1 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -50,6 +50,8 @@ public class OpenFeatureClient implements Client { private final ConcurrentLinkedQueue clientHooks; private final AtomicReference evaluationContext = new AtomicReference<>(); + private final HookSupport hookSupport; + /** * Deprecated public constructor. Use OpenFeature.API.getClient() instead. * @@ -66,6 +68,7 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String ve this.openfeatureApi = openFeatureAPI; this.domain = domain; this.version = version; + this.hookSupport = new HookSupport(); this.clientHooks = new ConcurrentLinkedQueue<>(); } @@ -162,7 +165,7 @@ private FlagEvaluationDetails evaluateFlag( var hints = Collections.unmodifiableMap(flagOptions.getHookHints()); FlagEvaluationDetails details = null; - HookSupport hookSupport = null; + HookSupportData hookSupportData = new HookSupportData(); try { final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); @@ -176,10 +179,12 @@ private FlagEvaluationDetails evaluateFlag( var sharedHookContext = new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue); + hookSupportData.initialize(mergedHooks, sharedHookContext, ctx, hints); + var evalContext = mergeEvaluationContext(ctx); - hookSupport = new HookSupport(mergedHooks, sharedHookContext, evalContext, hints); + hookSupportData.setEvaluationContext(evalContext); - hookSupport.executeBeforeHooks(); + hookSupport.executeBeforeHooks(hookSupportData); // "short circuit" if the provider is in NOT_READY or FATAL state if (ProviderState.NOT_READY.equals(state)) { @@ -190,16 +195,16 @@ private FlagEvaluationDetails evaluateFlag( } var providerEval = (ProviderEvaluation) - createProviderEvaluation(type, key, defaultValue, provider, hookSupport.getEvaluationContext()); + createProviderEvaluation(type, key, defaultValue, provider, hookSupportData.getEvaluationContext()); details = FlagEvaluationDetails.from(providerEval, key); if (details.getErrorCode() != null) { var error = ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.executeErrorHooks(error); + hookSupport.executeErrorHooks(hookSupportData, error); } else { - hookSupport.executeAfterHooks(details); + hookSupport.executeAfterHooks(hookSupportData, details); } } catch (Exception e) { if (details == null) { @@ -212,10 +217,12 @@ private FlagEvaluationDetails evaluateFlag( } details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.executeErrorHooks(e); + if (hookSupportData.isInitialized()) { + hookSupport.executeErrorHooks(hookSupportData, e); + } } finally { - if (hookSupport != null) { - hookSupport.executeAfterAllHooks(details); + if (hookSupportData.isInitialized()) { + hookSupport.executeAfterAllHooks(hookSupportData, details); } } diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index b0cd9ea70..5fa8b6cc2 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -19,6 +19,9 @@ import org.junit.jupiter.params.provider.EnumSource; class HookSupportTest implements HookFixtures { + + private static final HookSupport hookSupport = new HookSupport(); + @Test @DisplayName("should merge EvaluationContexts on before hooks correctly") void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { @@ -31,15 +34,16 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber"))); when(hook2.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("foo", "bar"))); - HookSupport executor = new HookSupport( + var hookSupportData = new HookSupportData(); + hookSupportData.initialize( Arrays.asList(hook1, hook2), getBaseHookContextForType(FlagValueType.STRING), baseContext, Collections.emptyMap()); - executor.executeBeforeHooks(); + hookSupport.executeBeforeHooks(hookSupportData); - EvaluationContext result = executor.getEvaluationContext(); + EvaluationContext result = hookSupportData.getEvaluationContext(); assertThat(result.getValue("bla").asString()).isEqualTo("blubber"); assertThat(result.getValue("foo").asString()).isEqualTo("bar"); @@ -52,13 +56,14 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { Hook genericHook = mockGenericHook(); - HookSupport hookSupport = new HookSupport( + var hookSupportData = new HookSupportData(); + hookSupportData.initialize( List.of(genericHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); - callAllHooks(hookSupport); + callAllHooks(hookSupportData); verify(genericHook).before(any(), any()); verify(genericHook).after(any(), any(), any()); @@ -71,22 +76,25 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { @DisplayName("should allow hooks to store and retrieve data across stages") void shouldPassDataAcrossStages(FlagValueType flagValueType) { var testHook = new TestHookWithData(); - HookSupport hookSupport = new HookSupport( + var hookSupportData = new HookSupportData(); + hookSupportData.initialize( List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); - hookSupport.executeBeforeHooks(); + hookSupport.executeBeforeHooks(hookSupportData); assertHookData(testHook, "before"); - hookSupport.executeAfterHooks(FlagEvaluationDetails.builder().build()); + hookSupport.executeAfterHooks( + hookSupportData, FlagEvaluationDetails.builder().build()); assertHookData(testHook, "before", "after"); - hookSupport.executeAfterAllHooks(FlagEvaluationDetails.builder().build()); + hookSupport.executeAfterAllHooks( + hookSupportData, FlagEvaluationDetails.builder().build()); assertHookData(testHook, "before", "after", "finallyAfter"); - hookSupport.executeErrorHooks(mock(Exception.class)); + hookSupport.executeErrorHooks(hookSupportData, mock(Exception.class)); assertHookData(testHook, "before", "after", "finallyAfter", "error"); } @@ -97,23 +105,26 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { var testHook1 = new TestHookWithData(1); var testHook2 = new TestHookWithData(2); - HookSupport hookSupport = new HookSupport( + var hookSupportData = new HookSupportData(); + hookSupportData.initialize( List.of(testHook1, testHook2), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY, Collections.emptyMap()); - callAllHooks(hookSupport); + callAllHooks(hookSupportData); assertHookData(testHook1, 1, "before", "after", "finallyAfter", "error"); assertHookData(testHook2, 2, "before", "after", "finallyAfter", "error"); } - 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 callAllHooks(HookSupportData hookSupportData) { + hookSupport.executeBeforeHooks(hookSupportData); + hookSupport.executeAfterHooks( + hookSupportData, FlagEvaluationDetails.builder().build()); + hookSupport.executeAfterAllHooks( + hookSupportData, FlagEvaluationDetails.builder().build()); + hookSupport.executeErrorHooks(hookSupportData, mock(Exception.class)); } private static void assertHookData(TestHookWithData testHook, String... expectedKeys) { From 4ab14bfd1c06f33d93c33189bb7f38c1fe472391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Thu, 25 Sep 2025 14:26:55 +0200 Subject: [PATCH 12/21] Update DefaultHookDataTest.java spotless MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java b/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java index f8fa4d81c..ac50988ea 100644 --- a/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java +++ b/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java @@ -1,12 +1,11 @@ package dev.openfeature.sdk; - -import org.junit.jupiter.api.Test; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import org.junit.jupiter.api.Test; + class DefaultHookDataTest { @Test From 1d8f758985d8faff506c626889b688a14ffa296a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Thu, 25 Sep 2025 16:02:01 +0200 Subject: [PATCH 13/21] fix tests, spotless apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- src/main/java/dev/openfeature/sdk/HookContext.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index 86f414b24..7f9b5da9b 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -14,10 +14,7 @@ public final class HookContext { private EvaluationContext ctx; private final HookData hookData; - HookContext( - @NonNull SharedHookContext sharedContext, - @NonNull EvaluationContext evaluationContext, - HookData hookData) { + HookContext(@NonNull SharedHookContext sharedContext, EvaluationContext evaluationContext, HookData hookData) { this.sharedContext = sharedContext; ctx = evaluationContext; this.hookData = hookData; From 8a9738f54eb589d08e40878d025ae7541cabcd00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Thu, 25 Sep 2025 18:26:43 +0200 Subject: [PATCH 14/21] exclude lombok generated functions from codecov MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/DefaultHookData.java | 3 +++ src/main/java/dev/openfeature/sdk/HookContext.java | 14 ++++++++++++++ src/main/java/dev/openfeature/sdk/Pair.java | 3 +++ .../dev/openfeature/sdk/SharedHookContext.java | 3 +++ 4 files changed, 23 insertions(+) diff --git a/src/main/java/dev/openfeature/sdk/DefaultHookData.java b/src/main/java/dev/openfeature/sdk/DefaultHookData.java index 6ee80d342..e24c9a0d3 100644 --- a/src/main/java/dev/openfeature/sdk/DefaultHookData.java +++ b/src/main/java/dev/openfeature/sdk/DefaultHookData.java @@ -3,6 +3,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import lombok.Generated; /** * Default implementation of HookData. @@ -38,6 +39,7 @@ public T get(String key, Class type) { return type.cast(value); } + @Generated @Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) { @@ -47,6 +49,7 @@ public boolean equals(Object o) { return Objects.equals(data, that.data); } + @Generated @Override public int hashCode() { return Objects.hashCode(data); diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index 7f9b5da9b..d8ebc8f84 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -2,6 +2,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Objects; +import lombok.Generated; import lombok.NonNull; /** @@ -92,6 +93,7 @@ public static HookContextBuilder builder() { return sharedContext.getFlagKey(); } + @Generated public @NonNull FlagValueType getType() { return sharedContext.getType(); } @@ -117,6 +119,7 @@ public HookData getHookData() { return this.hookData; } + @Generated @Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) { @@ -128,11 +131,13 @@ public boolean equals(Object o) { && Objects.equals(sharedContext, that.sharedContext); } + @Generated @Override public int hashCode() { return Objects.hash(ctx, hookData, sharedContext); } + @Generated @Override public String toString() { return "HookContext(flagKey=" + this.getFlagKey() + ", type=" + this.getType() + ", defaultValue=" @@ -151,6 +156,7 @@ void setCtx(@NonNull EvaluationContext ctx) { * @return new HookContext with updated flagKey or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Generated @Deprecated public HookContext withFlagKey(@NonNull String flagKey) { return Objects.equals(this.getFlagKey(), flagKey) @@ -172,6 +178,7 @@ public HookContext withFlagKey(@NonNull String flagKey) { * @return new HookContext with updated type or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Generated @Deprecated public HookContext withType(@NonNull FlagValueType type) { return this.getType() == type @@ -193,6 +200,7 @@ public HookContext withType(@NonNull FlagValueType type) { * @return new HookContext with updated defaultValue or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Generated @Deprecated public HookContext withDefaultValue(@NonNull T defaultValue) { return this.getDefaultValue() == defaultValue @@ -214,6 +222,7 @@ public HookContext withDefaultValue(@NonNull T defaultValue) { * @return new HookContext with updated ctx or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Generated @Deprecated public HookContext withCtx(@NonNull EvaluationContext ctx) { return this.ctx == ctx @@ -235,6 +244,7 @@ public HookContext withCtx(@NonNull EvaluationContext ctx) { * @return new HookContext with updated clientMetadata or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Generated @Deprecated public HookContext withClientMetadata(ClientMetadata clientMetadata) { return this.getClientMetadata() == clientMetadata @@ -256,6 +266,7 @@ public HookContext withClientMetadata(ClientMetadata clientMetadata) { * @return new HookContext with updated providerMetadata or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Generated @Deprecated public HookContext withProviderMetadata(Metadata providerMetadata) { return this.getProviderMetadata() == providerMetadata @@ -277,6 +288,7 @@ public HookContext withProviderMetadata(Metadata providerMetadata) { * @return new HookContext with updated hookData or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Generated @Deprecated public HookContext withHookData(HookData hookData) { return this.hookData == hookData @@ -297,6 +309,7 @@ public HookContext withHookData(HookData hookData) { * @param The flag type. * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ + @Generated @Deprecated public static class HookContextBuilder { private String flagKey; @@ -361,6 +374,7 @@ public HookContext build() { this.hookData); } + @Generated @Override public String toString() { return "HookContext.HookContextBuilder(flagKey=" + this.flagKey + ", type=" + this.type + ", defaultValue=" diff --git a/src/main/java/dev/openfeature/sdk/Pair.java b/src/main/java/dev/openfeature/sdk/Pair.java index e3e50974f..68e6de235 100644 --- a/src/main/java/dev/openfeature/sdk/Pair.java +++ b/src/main/java/dev/openfeature/sdk/Pair.java @@ -1,5 +1,7 @@ package dev.openfeature.sdk; +import lombok.Generated; + class Pair { private final K key; private final V value; @@ -17,6 +19,7 @@ public V getValue() { return value; } + @Generated @Override public String toString() { return "Pair{" + "key=" + key + ", value=" + value + '}'; diff --git a/src/main/java/dev/openfeature/sdk/SharedHookContext.java b/src/main/java/dev/openfeature/sdk/SharedHookContext.java index 7e7804fc9..d12531a29 100644 --- a/src/main/java/dev/openfeature/sdk/SharedHookContext.java +++ b/src/main/java/dev/openfeature/sdk/SharedHookContext.java @@ -1,6 +1,7 @@ package dev.openfeature.sdk; import java.util.Objects; +import lombok.Generated; import lombok.Getter; @Getter @@ -29,6 +30,7 @@ public HookContext hookContextFor(EvaluationContext evaluationContext, HookDa return new HookContext<>(this, evaluationContext, hookData); } + @Generated @Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) { @@ -42,6 +44,7 @@ public boolean equals(Object o) { && Objects.equals(defaultValue, that.defaultValue); } + @Generated @Override public int hashCode() { return Objects.hash(flagKey, type, clientMetadata, providerMetadata, defaultValue); From b6cd0e242cdc88502c71ba6f253f819169749c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Mon, 29 Sep 2025 16:45:28 +0200 Subject: [PATCH 15/21] replace init function with setters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../dev/openfeature/sdk/HookSupportData.java | 28 +++++++++---------- .../openfeature/sdk/OpenFeatureClient.java | 13 +++++---- .../dev/openfeature/sdk/HookSupportTest.java | 28 +++++-------------- 3 files changed, 27 insertions(+), 42 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookSupportData.java b/src/main/java/dev/openfeature/sdk/HookSupportData.java index 9949d63e8..8b7edb167 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupportData.java +++ b/src/main/java/dev/openfeature/sdk/HookSupportData.java @@ -4,6 +4,7 @@ import java.util.List; import java.util.Map; import lombok.Getter; +import lombok.Setter; /** * Encapsulates data for hook execution per flag evaluation. @@ -13,16 +14,22 @@ class HookSupportData { private List> hooks; private EvaluationContext evaluationContext; + + @Setter private Map hints; - private boolean isInitialized = false; HookSupportData() {} - void initialize( - List hooks, - SharedHookContext sharedContext, - EvaluationContext evaluationContext, - Map hints) { + public void setEvaluationContext(EvaluationContext evaluationContext) { + this.evaluationContext = evaluationContext; + if (hooks != null) { + for (Pair hookContextPair : hooks) { + hookContextPair.getValue().setCtx(evaluationContext); + } + } + } + + public void setHooks(List hooks, SharedHookContext sharedContext, EvaluationContext evaluationContext) { List> hookContextPairs = new ArrayList<>(); for (Hook hook : hooks) { if (hook.supportsFlagValueType(sharedContext.getType())) { @@ -32,14 +39,5 @@ void initialize( } this.hooks = hookContextPairs; this.evaluationContext = evaluationContext; - this.hints = hints; - isInitialized = true; - } - - public void setEvaluationContext(EvaluationContext evaluationContext) { - this.evaluationContext = evaluationContext; - for (Pair hookContextPair : hooks) { - hookContextPair.getValue().setCtx(evaluationContext); - } } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 4f789e8b1..f45f78e76 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -160,12 +160,13 @@ public EvaluationContext getEvaluationContext() { + "Instead, we return an evaluation result with the appropriate error code.") private FlagEvaluationDetails evaluateFlag( FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { + FlagEvaluationDetails details = null; + HookSupportData hookSupportData = new HookSupportData(); + var flagOptions = ObjectUtils.defaultIfNull( options, () -> FlagEvaluationOptions.builder().build()); var hints = Collections.unmodifiableMap(flagOptions.getHookHints()); - - FlagEvaluationDetails details = null; - HookSupportData hookSupportData = new HookSupportData(); + hookSupportData.setHints(hints); try { final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); @@ -179,7 +180,7 @@ private FlagEvaluationDetails evaluateFlag( var sharedHookContext = new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue); - hookSupportData.initialize(mergedHooks, sharedHookContext, ctx, hints); + hookSupportData.setHooks(mergedHooks, sharedHookContext, ctx); var evalContext = mergeEvaluationContext(ctx); hookSupportData.setEvaluationContext(evalContext); @@ -217,11 +218,11 @@ private FlagEvaluationDetails evaluateFlag( } details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - if (hookSupportData.isInitialized()) { + if (hookSupportData.getHooks() != null && hookSupportData.getHints() != null) { hookSupport.executeErrorHooks(hookSupportData, e); } } finally { - if (hookSupportData.isInitialized()) { + if (hookSupportData.getHooks() != null && hookSupportData.getHints() != null) { hookSupport.executeAfterAllHooks(hookSupportData, details); } } diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index 5fa8b6cc2..c8c2128a5 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -8,7 +8,6 @@ import dev.openfeature.sdk.fixtures.HookFixtures; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -35,11 +34,8 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { when(hook2.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("foo", "bar"))); var hookSupportData = new HookSupportData(); - hookSupportData.initialize( - Arrays.asList(hook1, hook2), - getBaseHookContextForType(FlagValueType.STRING), - baseContext, - Collections.emptyMap()); + hookSupportData.setHooks( + Arrays.asList(hook1, hook2), getBaseHookContextForType(FlagValueType.STRING), baseContext); hookSupport.executeBeforeHooks(hookSupportData); @@ -57,11 +53,8 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { Hook genericHook = mockGenericHook(); var hookSupportData = new HookSupportData(); - hookSupportData.initialize( - List.of(genericHook), - getBaseHookContextForType(flagValueType), - ImmutableContext.EMPTY, - Collections.emptyMap()); + hookSupportData.setHooks( + List.of(genericHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY); callAllHooks(hookSupportData); @@ -77,11 +70,7 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { void shouldPassDataAcrossStages(FlagValueType flagValueType) { var testHook = new TestHookWithData(); var hookSupportData = new HookSupportData(); - hookSupportData.initialize( - List.of(testHook), - getBaseHookContextForType(flagValueType), - ImmutableContext.EMPTY, - Collections.emptyMap()); + hookSupportData.setHooks(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY); hookSupport.executeBeforeHooks(hookSupportData); assertHookData(testHook, "before"); @@ -106,11 +95,8 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { var testHook2 = new TestHookWithData(2); var hookSupportData = new HookSupportData(); - hookSupportData.initialize( - List.of(testHook1, testHook2), - getBaseHookContextForType(flagValueType), - ImmutableContext.EMPTY, - Collections.emptyMap()); + hookSupportData.setHooks( + List.of(testHook1, testHook2), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY); callAllHooks(hookSupportData); From 1a7e5af0bfb48dbde4630e7b530674d8eadfda19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Tue, 30 Sep 2025 08:20:16 +0200 Subject: [PATCH 16/21] pr suggestion: replace Generated annotation with more descriptive ExcludeFromGeneratedCoverageReport, replace delomboked functions with lombok annotations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../dev/openfeature/sdk/DefaultHookData.java | 18 ------ .../java/dev/openfeature/sdk/HookContext.java | 64 ++++++------------- src/main/java/dev/openfeature/sdk/Pair.java | 9 +-- .../openfeature/sdk/SharedHookContext.java | 24 +------ 4 files changed, 26 insertions(+), 89 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/DefaultHookData.java b/src/main/java/dev/openfeature/sdk/DefaultHookData.java index e24c9a0d3..c34b21496 100644 --- a/src/main/java/dev/openfeature/sdk/DefaultHookData.java +++ b/src/main/java/dev/openfeature/sdk/DefaultHookData.java @@ -2,8 +2,6 @@ import java.util.HashMap; import java.util.Map; -import java.util.Objects; -import lombok.Generated; /** * Default implementation of HookData. @@ -38,20 +36,4 @@ public T get(String key, Class type) { } return type.cast(value); } - - @Generated - @Override - public boolean equals(Object o) { - if (o == null || getClass() != o.getClass()) { - return false; - } - dev.openfeature.sdk.DefaultHookData that = (dev.openfeature.sdk.DefaultHookData) o; - return Objects.equals(data, that.data); - } - - @Generated - @Override - public int hashCode() { - return Objects.hashCode(data); - } } diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index d8ebc8f84..896d96c9d 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -1,15 +1,20 @@ package dev.openfeature.sdk; +import dev.openfeature.sdk.internal.ExcludeFromGeneratedCoverageReport; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Objects; +import lombok.EqualsAndHashCode; import lombok.Generated; import lombok.NonNull; +import lombok.ToString; /** * A data class to hold immutable context that {@link Hook} instances use. * * @param the type for the flag being evaluated */ +@EqualsAndHashCode +@ToString public final class HookContext { private final SharedHookContext sharedContext; private EvaluationContext ctx; @@ -93,7 +98,6 @@ public static HookContextBuilder builder() { return sharedContext.getFlagKey(); } - @Generated public @NonNull FlagValueType getType() { return sharedContext.getType(); } @@ -119,32 +123,6 @@ public HookData getHookData() { return this.hookData; } - @Generated - @Override - public boolean equals(Object o) { - if (o == null || getClass() != o.getClass()) { - return false; - } - HookContext that = (HookContext) o; - return Objects.equals(ctx, that.ctx) - && Objects.equals(hookData, that.hookData) - && Objects.equals(sharedContext, that.sharedContext); - } - - @Generated - @Override - public int hashCode() { - return Objects.hash(ctx, hookData, sharedContext); - } - - @Generated - @Override - public String toString() { - return "HookContext(flagKey=" + this.getFlagKey() + ", type=" + this.getType() + ", defaultValue=" - + this.getDefaultValue() + ", ctx=" + this.getCtx() + ", clientMetadata=" + this.getClientMetadata() - + ", providerMetadata=" + this.getProviderMetadata() + ", hookData=" + this.getHookData() + ")"; - } - void setCtx(@NonNull EvaluationContext ctx) { this.ctx = ctx; } @@ -156,7 +134,7 @@ void setCtx(@NonNull EvaluationContext ctx) { * @return new HookContext with updated flagKey or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ - @Generated + @ExcludeFromGeneratedCoverageReport @Deprecated public HookContext withFlagKey(@NonNull String flagKey) { return Objects.equals(this.getFlagKey(), flagKey) @@ -178,7 +156,7 @@ public HookContext withFlagKey(@NonNull String flagKey) { * @return new HookContext with updated type or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ - @Generated + @ExcludeFromGeneratedCoverageReport @Deprecated public HookContext withType(@NonNull FlagValueType type) { return this.getType() == type @@ -200,7 +178,7 @@ public HookContext withType(@NonNull FlagValueType type) { * @return new HookContext with updated defaultValue or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ - @Generated + @ExcludeFromGeneratedCoverageReport @Deprecated public HookContext withDefaultValue(@NonNull T defaultValue) { return this.getDefaultValue() == defaultValue @@ -222,7 +200,7 @@ public HookContext withDefaultValue(@NonNull T defaultValue) { * @return new HookContext with updated ctx or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ - @Generated + @ExcludeFromGeneratedCoverageReport @Deprecated public HookContext withCtx(@NonNull EvaluationContext ctx) { return this.ctx == ctx @@ -244,7 +222,7 @@ public HookContext withCtx(@NonNull EvaluationContext ctx) { * @return new HookContext with updated clientMetadata or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ - @Generated + @ExcludeFromGeneratedCoverageReport @Deprecated public HookContext withClientMetadata(ClientMetadata clientMetadata) { return this.getClientMetadata() == clientMetadata @@ -266,7 +244,7 @@ public HookContext withClientMetadata(ClientMetadata clientMetadata) { * @return new HookContext with updated providerMetadata or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ - @Generated + @ExcludeFromGeneratedCoverageReport @Deprecated public HookContext withProviderMetadata(Metadata providerMetadata) { return this.getProviderMetadata() == providerMetadata @@ -288,7 +266,7 @@ public HookContext withProviderMetadata(Metadata providerMetadata) { * @return new HookContext with updated hookData or the same instance if unchanged * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ - @Generated + @ExcludeFromGeneratedCoverageReport @Deprecated public HookContext withHookData(HookData hookData) { return this.hookData == hookData @@ -309,8 +287,8 @@ public HookContext withHookData(HookData hookData) { * @param The flag type. * @deprecated HookContext is initialized by the SDK and passed to hooks. Users should not create new instances. */ - @Generated @Deprecated + @ToString public static class HookContextBuilder { private String flagKey; private FlagValueType type; @@ -322,37 +300,44 @@ public static class HookContextBuilder { HookContextBuilder() {} + @ExcludeFromGeneratedCoverageReport public HookContextBuilder flagKey(@NonNull String flagKey) { this.flagKey = flagKey; return this; } + @ExcludeFromGeneratedCoverageReport public HookContextBuilder type(@NonNull FlagValueType type) { this.type = type; return this; } + @ExcludeFromGeneratedCoverageReport public HookContextBuilder defaultValue(@NonNull T defaultValue) { this.defaultValue = defaultValue; return this; } + @ExcludeFromGeneratedCoverageReport public HookContextBuilder ctx(@NonNull EvaluationContext ctx) { this.ctx = ctx; return this; } + @ExcludeFromGeneratedCoverageReport public HookContextBuilder clientMetadata(ClientMetadata clientMetadata) { this.clientMetadata = clientMetadata; return this; } + @ExcludeFromGeneratedCoverageReport public HookContextBuilder providerMetadata(Metadata providerMetadata) { this.providerMetadata = providerMetadata; return this; } @SuppressFBWarnings(value = "EI_EXPOSE_REP2", justification = "Intentional exposure of hookData") + @ExcludeFromGeneratedCoverageReport public HookContextBuilder hookData(HookData hookData) { this.hookData = hookData; return this; @@ -363,6 +348,7 @@ public HookContextBuilder hookData(HookData hookData) { * * @return a new HookContext */ + @ExcludeFromGeneratedCoverageReport public HookContext build() { return new HookContext( this.flagKey, @@ -373,13 +359,5 @@ public HookContext build() { this.providerMetadata, this.hookData); } - - @Generated - @Override - public String toString() { - return "HookContext.HookContextBuilder(flagKey=" + this.flagKey + ", type=" + this.type + ", defaultValue=" - + this.defaultValue + ", ctx=" + this.ctx + ", clientMetadata=" + this.clientMetadata - + ", providerMetadata=" + this.providerMetadata + ", hookData=" + this.hookData + ")"; - } } } diff --git a/src/main/java/dev/openfeature/sdk/Pair.java b/src/main/java/dev/openfeature/sdk/Pair.java index 68e6de235..90c51f0f7 100644 --- a/src/main/java/dev/openfeature/sdk/Pair.java +++ b/src/main/java/dev/openfeature/sdk/Pair.java @@ -1,7 +1,8 @@ package dev.openfeature.sdk; -import lombok.Generated; +import lombok.ToString; +@ToString class Pair { private final K key; private final V value; @@ -19,12 +20,6 @@ public V getValue() { return value; } - @Generated - @Override - public String toString() { - return "Pair{" + "key=" + key + ", value=" + value + '}'; - } - public static Pair of(K key, V value) { return new Pair<>(key, value); } diff --git a/src/main/java/dev/openfeature/sdk/SharedHookContext.java b/src/main/java/dev/openfeature/sdk/SharedHookContext.java index d12531a29..694072d13 100644 --- a/src/main/java/dev/openfeature/sdk/SharedHookContext.java +++ b/src/main/java/dev/openfeature/sdk/SharedHookContext.java @@ -1,10 +1,12 @@ package dev.openfeature.sdk; +import dev.openfeature.sdk.internal.ExcludeFromGeneratedCoverageReport; import java.util.Objects; -import lombok.Generated; +import lombok.EqualsAndHashCode; import lombok.Getter; @Getter +@EqualsAndHashCode class SharedHookContext { private final String flagKey; @@ -29,24 +31,4 @@ public SharedHookContext( public HookContext hookContextFor(EvaluationContext evaluationContext, HookData hookData) { return new HookContext<>(this, evaluationContext, hookData); } - - @Generated - @Override - public boolean equals(Object o) { - if (o == null || getClass() != o.getClass()) { - return false; - } - SharedHookContext that = (SharedHookContext) o; - return Objects.equals(flagKey, that.flagKey) - && type == that.type - && Objects.equals(clientMetadata, that.clientMetadata) - && Objects.equals(providerMetadata, that.providerMetadata) - && Objects.equals(defaultValue, that.defaultValue); - } - - @Generated - @Override - public int hashCode() { - return Objects.hash(flagKey, type, clientMetadata, providerMetadata, defaultValue); - } } From 7fe0675858f633c77df4f54ff1bb89da5acbd8ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Tue, 30 Sep 2025 14:20:50 +0200 Subject: [PATCH 17/21] PR suggestion: make HookSupportData a real POJO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/HookContext.java | 1 - .../java/dev/openfeature/sdk/HookSupport.java | 41 ++++++++++++++++++- .../dev/openfeature/sdk/HookSupportData.java | 31 ++------------ .../openfeature/sdk/OpenFeatureClient.java | 7 ++-- .../openfeature/sdk/SharedHookContext.java | 2 - .../dev/openfeature/sdk/HookSupportTest.java | 25 +++++++---- 6 files changed, 63 insertions(+), 44 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index 896d96c9d..8d4d2e13a 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -4,7 +4,6 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Objects; import lombok.EqualsAndHashCode; -import lombok.Generated; import lombok.NonNull; import lombok.ToString; diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index 1d37e34ab..b8bda7deb 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -9,6 +9,45 @@ @Slf4j class HookSupport { + /** + * Updates the evaluation context in the given data object's eval context and each hooks eval context. + * + * @param hookSupportData the data object to modify + * @param evaluationContext the new context to set + */ + public void updateEvaluationContext(HookSupportData hookSupportData, EvaluationContext evaluationContext) { + hookSupportData.evaluationContext = evaluationContext; + if (hookSupportData.hooks != null) { + for (Pair hookContextPair : hookSupportData.hooks) { + hookContextPair.getValue().setCtx(evaluationContext); + } + } + } + + /** + * Sets the {@link Hook}-{@link HookContext}-{@link Pair} list in the given data object. + * + * @param hookSupportData the data object to modify + * @param hooks the new hooks to set + * @param sharedContext the shared context across all hooks from which each hook's {@link HookContext} is + * created + * @param evaluationContext the evaluation context which is + */ + public void setHookSupportDataHooks( + HookSupportData hookSupportData, + List hooks, + SharedHookContext sharedContext, + EvaluationContext evaluationContext) { + List> hookContextPairs = new ArrayList<>(); + for (Hook hook : hooks) { + if (hook.supportsFlagValueType(sharedContext.getType())) { + HookContext curContext = sharedContext.hookContextFor(evaluationContext, new DefaultHookData()); + hookContextPairs.add(Pair.of(hook, curContext)); + } + } + hookSupportData.hooks = hookContextPairs; + } + public void executeBeforeHooks(HookSupportData data) { // These traverse backwards from normal. List> reversedHooks = new ArrayList<>(data.getHooks()); @@ -23,7 +62,7 @@ public void executeBeforeHooks(HookSupportData data) { .orElse(Optional.empty()); if (returnedEvalContext.isPresent()) { // update shared evaluation context for all hooks - data.setEvaluationContext(data.getEvaluationContext().merge(returnedEvalContext.get())); + updateEvaluationContext(data, data.getEvaluationContext().merge(returnedEvalContext.get())); } } } diff --git a/src/main/java/dev/openfeature/sdk/HookSupportData.java b/src/main/java/dev/openfeature/sdk/HookSupportData.java index 8b7edb167..2d3346ba1 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupportData.java +++ b/src/main/java/dev/openfeature/sdk/HookSupportData.java @@ -1,10 +1,8 @@ package dev.openfeature.sdk; -import java.util.ArrayList; import java.util.List; import java.util.Map; import lombok.Getter; -import lombok.Setter; /** * Encapsulates data for hook execution per flag evaluation. @@ -12,32 +10,9 @@ @Getter class HookSupportData { - private List> hooks; - private EvaluationContext evaluationContext; - - @Setter - private Map hints; + List> hooks; + EvaluationContext evaluationContext; + Map hints; HookSupportData() {} - - public void setEvaluationContext(EvaluationContext evaluationContext) { - this.evaluationContext = evaluationContext; - if (hooks != null) { - for (Pair hookContextPair : hooks) { - hookContextPair.getValue().setCtx(evaluationContext); - } - } - } - - public void setHooks(List hooks, SharedHookContext sharedContext, EvaluationContext evaluationContext) { - List> hookContextPairs = new ArrayList<>(); - for (Hook hook : hooks) { - if (hook.supportsFlagValueType(sharedContext.getType())) { - HookContext curContext = sharedContext.hookContextFor(evaluationContext, new DefaultHookData()); - hookContextPairs.add(Pair.of(hook, curContext)); - } - } - this.hooks = hookContextPairs; - this.evaluationContext = evaluationContext; - } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index f45f78e76..67aa17eb3 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -165,8 +165,7 @@ private FlagEvaluationDetails evaluateFlag( var flagOptions = ObjectUtils.defaultIfNull( options, () -> FlagEvaluationOptions.builder().build()); - var hints = Collections.unmodifiableMap(flagOptions.getHookHints()); - hookSupportData.setHints(hints); + hookSupportData.hints = Collections.unmodifiableMap(flagOptions.getHookHints()); try { final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); @@ -180,10 +179,10 @@ private FlagEvaluationDetails evaluateFlag( var sharedHookContext = new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue); - hookSupportData.setHooks(mergedHooks, sharedHookContext, ctx); + hookSupport.setHookSupportDataHooks(hookSupportData, mergedHooks, sharedHookContext, ctx); var evalContext = mergeEvaluationContext(ctx); - hookSupportData.setEvaluationContext(evalContext); + hookSupport.updateEvaluationContext(hookSupportData, evalContext); hookSupport.executeBeforeHooks(hookSupportData); diff --git a/src/main/java/dev/openfeature/sdk/SharedHookContext.java b/src/main/java/dev/openfeature/sdk/SharedHookContext.java index 694072d13..8faab37b2 100644 --- a/src/main/java/dev/openfeature/sdk/SharedHookContext.java +++ b/src/main/java/dev/openfeature/sdk/SharedHookContext.java @@ -1,7 +1,5 @@ package dev.openfeature.sdk; -import dev.openfeature.sdk.internal.ExcludeFromGeneratedCoverageReport; -import java.util.Objects; import lombok.EqualsAndHashCode; import lombok.Getter; diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index c8c2128a5..de48dabac 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -26,16 +26,18 @@ class HookSupportTest implements HookFixtures { void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { Map attributes = new HashMap<>(); attributes.put("baseKey", new Value("baseValue")); - EvaluationContext baseContext = new ImmutableContext(attributes); + EvaluationContext baseEvalContext = new ImmutableContext(attributes); Hook hook1 = mockStringHook(); Hook hook2 = mockStringHook(); when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber"))); when(hook2.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("foo", "bar"))); + var sharedContext = getBaseHookContextForType(FlagValueType.STRING); var hookSupportData = new HookSupportData(); - hookSupportData.setHooks( - Arrays.asList(hook1, hook2), getBaseHookContextForType(FlagValueType.STRING), baseContext); + hookSupport.setHookSupportDataHooks( + hookSupportData, Arrays.asList(hook1, hook2), sharedContext, baseEvalContext); + hookSupport.updateEvaluationContext(hookSupportData, baseEvalContext); hookSupport.executeBeforeHooks(hookSupportData); @@ -53,8 +55,11 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { Hook genericHook = mockGenericHook(); var hookSupportData = new HookSupportData(); - hookSupportData.setHooks( - List.of(genericHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY); + hookSupport.setHookSupportDataHooks( + hookSupportData, + List.of(genericHook), + getBaseHookContextForType(flagValueType), + ImmutableContext.EMPTY); callAllHooks(hookSupportData); @@ -70,7 +75,8 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { void shouldPassDataAcrossStages(FlagValueType flagValueType) { var testHook = new TestHookWithData(); var hookSupportData = new HookSupportData(); - hookSupportData.setHooks(List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY); + hookSupport.setHookSupportDataHooks( + hookSupportData, List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY); hookSupport.executeBeforeHooks(hookSupportData); assertHookData(testHook, "before"); @@ -95,8 +101,11 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { var testHook2 = new TestHookWithData(2); var hookSupportData = new HookSupportData(); - hookSupportData.setHooks( - List.of(testHook1, testHook2), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY); + hookSupport.setHookSupportDataHooks( + hookSupportData, + List.of(testHook1, testHook2), + getBaseHookContextForType(flagValueType), + ImmutableContext.EMPTY); callAllHooks(hookSupportData); From 92b6dc4e01d8fc29f64fd85f0ebb9d98809a1051 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Tue, 30 Sep 2025 15:13:01 +0200 Subject: [PATCH 18/21] gemini suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- src/main/java/dev/openfeature/sdk/OpenFeatureClient.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 67aa17eb3..a30bc4940 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -217,11 +217,11 @@ private FlagEvaluationDetails evaluateFlag( } details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - if (hookSupportData.getHooks() != null && hookSupportData.getHints() != null) { + if (hookSupportData.getHooks() != null) { hookSupport.executeErrorHooks(hookSupportData, e); } } finally { - if (hookSupportData.getHooks() != null && hookSupportData.getHints() != null) { + if (hookSupportData.getHooks() != null) { hookSupport.executeAfterAllHooks(hookSupportData, details); } } From 0e897f284e82cd7025e9d7ded478135009db1610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Tue, 30 Sep 2025 17:19:57 +0200 Subject: [PATCH 19/21] PR suggestion: call hooks as early as possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/HookSupport.java | 63 ++++++++++++------- .../openfeature/sdk/OpenFeatureClient.java | 5 +- .../dev/openfeature/sdk/HookSupportTest.java | 21 +++---- 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index b8bda7deb..e9d01c382 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -6,46 +6,63 @@ import java.util.Optional; import lombok.extern.slf4j.Slf4j; +/** + * Helper class to run hooks. Initialize {@link HookSupportData} by calling setHooks, setHookContexts + * & updateEvaluationContext in this exact order. + */ @Slf4j class HookSupport { /** - * Updates the evaluation context in the given data object's eval context and each hooks eval context. + * 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 evaluationContext the new context to set + * @param hooks the hooks to set + * @param type the flag value type to filter unsupported hooks */ - public void updateEvaluationContext(HookSupportData hookSupportData, EvaluationContext evaluationContext) { - hookSupportData.evaluationContext = evaluationContext; - if (hookSupportData.hooks != null) { - for (Pair hookContextPair : hookSupportData.hooks) { - hookContextPair.getValue().setCtx(evaluationContext); + public void setHooks(HookSupportData hookSupportData, List hooks, FlagValueType type) { + List> hookContextPairs = new ArrayList<>(); + for (Hook hook : hooks) { + if (hook.supportsFlagValueType(type)) { + hookContextPairs.add(Pair.of(hook, null)); } } + hookSupportData.hooks = hookContextPairs; } /** - * Sets the {@link Hook}-{@link HookContext}-{@link Pair} list in the given data object. + * 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 hooks the new hooks to set - * @param sharedContext the shared context across all hooks from which each hook's {@link HookContext} is - * created - * @param evaluationContext the evaluation context which is + * @param sharedContext the shared context from which the new {@link HookContext} is created */ - public void setHookSupportDataHooks( - HookSupportData hookSupportData, - List hooks, - SharedHookContext sharedContext, - EvaluationContext evaluationContext) { - List> hookContextPairs = new ArrayList<>(); - for (Hook hook : hooks) { - if (hook.supportsFlagValueType(sharedContext.getType())) { - HookContext curContext = sharedContext.hookContextFor(evaluationContext, new DefaultHookData()); - hookContextPairs.add(Pair.of(hook, curContext)); + public void setHookContexts(HookSupportData hookSupportData, SharedHookContext sharedContext) { + for (int i = 0; i < hookSupportData.hooks.size(); i++) { + Pair hookContextPair = hookSupportData.hooks.get(i); + HookContext curHookContext = sharedContext.hookContextFor(null, new DefaultHookData()); + Pair updatedPair = Pair.of(hookContextPair.getKey(), curHookContext); + hookSupportData.hooks.set(i, updatedPair); + } + } + + /** + * Updates the evaluation context in the given data object's eval context and each hooks eval context. + * + * @param hookSupportData the data object to modify + * @param evaluationContext the new context to set + */ + public void updateEvaluationContext(HookSupportData hookSupportData, EvaluationContext evaluationContext) { + hookSupportData.evaluationContext = evaluationContext; + if (hookSupportData.hooks != null) { + for (Pair hookContextPair : hookSupportData.hooks) { + var curHookContext = hookContextPair.getValue(); + if (curHookContext != null) { + curHookContext.setCtx(evaluationContext); + } } } - hookSupportData.hooks = hookContextPairs; } public void executeBeforeHooks(HookSupportData data) { diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index a30bc4940..614bc1e34 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -173,13 +173,14 @@ private FlagEvaluationDetails evaluateFlag( final var provider = stateManager.getProvider(); 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); var sharedHookContext = new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue); - - hookSupport.setHookSupportDataHooks(hookSupportData, mergedHooks, sharedHookContext, ctx); + hookSupport.setHookContexts(hookSupportData, sharedHookContext); var evalContext = mergeEvaluationContext(ctx); hookSupport.updateEvaluationContext(hookSupportData, evalContext); diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index de48dabac..cc1b5ea62 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -35,8 +35,8 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { var sharedContext = getBaseHookContextForType(FlagValueType.STRING); var hookSupportData = new HookSupportData(); - hookSupport.setHookSupportDataHooks( - hookSupportData, Arrays.asList(hook1, hook2), sharedContext, baseEvalContext); + hookSupport.setHooks(hookSupportData, Arrays.asList(hook1, hook2), FlagValueType.STRING); + hookSupport.setHookContexts(hookSupportData, sharedContext); hookSupport.updateEvaluationContext(hookSupportData, baseEvalContext); hookSupport.executeBeforeHooks(hookSupportData); @@ -55,11 +55,7 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { Hook genericHook = mockGenericHook(); var hookSupportData = new HookSupportData(); - hookSupport.setHookSupportDataHooks( - hookSupportData, - List.of(genericHook), - getBaseHookContextForType(flagValueType), - ImmutableContext.EMPTY); + hookSupport.setHooks(hookSupportData, List.of(genericHook), flagValueType); callAllHooks(hookSupportData); @@ -75,8 +71,8 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { void shouldPassDataAcrossStages(FlagValueType flagValueType) { var testHook = new TestHookWithData(); var hookSupportData = new HookSupportData(); - hookSupport.setHookSupportDataHooks( - hookSupportData, List.of(testHook), getBaseHookContextForType(flagValueType), ImmutableContext.EMPTY); + hookSupport.setHooks(hookSupportData, List.of(testHook), flagValueType); + hookSupport.setHookContexts(hookSupportData, getBaseHookContextForType(flagValueType)); hookSupport.executeBeforeHooks(hookSupportData); assertHookData(testHook, "before"); @@ -101,11 +97,8 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { var testHook2 = new TestHookWithData(2); var hookSupportData = new HookSupportData(); - hookSupport.setHookSupportDataHooks( - hookSupportData, - List.of(testHook1, testHook2), - getBaseHookContextForType(flagValueType), - ImmutableContext.EMPTY); + hookSupport.setHooks(hookSupportData, List.of(testHook1, testHook2), flagValueType); + hookSupport.setHookContexts(hookSupportData, getBaseHookContextForType(flagValueType)); callAllHooks(hookSupportData); From 8b6aba9d2f96230c2476cbc6f2253d0d6ff50a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Thu, 2 Oct 2025 12:21:20 +0200 Subject: [PATCH 20/21] PR suggestions: integration test hook data usage in client, set pair value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../java/dev/openfeature/sdk/HookSupport.java | 3 +- src/main/java/dev/openfeature/sdk/Pair.java | 5 ++- .../dev/openfeature/sdk/HookSupportTest.java | 38 ----------------- .../sdk/OpenFeatureClientTest.java | 31 ++++++++++++++ .../dev/openfeature/sdk/TestHookWithData.java | 42 +++++++++++++++++++ 5 files changed, 78 insertions(+), 41 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/TestHookWithData.java diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index e9d01c382..c7a7630da 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -42,8 +42,7 @@ public void setHookContexts(HookSupportData hookSupportData, SharedHookContext s for (int i = 0; i < hookSupportData.hooks.size(); i++) { Pair hookContextPair = hookSupportData.hooks.get(i); HookContext curHookContext = sharedContext.hookContextFor(null, new DefaultHookData()); - Pair updatedPair = Pair.of(hookContextPair.getKey(), curHookContext); - hookSupportData.hooks.set(i, updatedPair); + hookContextPair.setValue(curHookContext); } } diff --git a/src/main/java/dev/openfeature/sdk/Pair.java b/src/main/java/dev/openfeature/sdk/Pair.java index 90c51f0f7..765be9f2d 100644 --- a/src/main/java/dev/openfeature/sdk/Pair.java +++ b/src/main/java/dev/openfeature/sdk/Pair.java @@ -1,11 +1,14 @@ package dev.openfeature.sdk; +import lombok.Setter; import lombok.ToString; @ToString class Pair { private final K key; - private final V value; + + @Setter + private V value; private Pair(K key, V value) { this.key = key; diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index cc1b5ea62..b1bb70ba1 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -163,42 +163,4 @@ private EvaluationContext evaluationContextWithValue(String key, String value) { attributes.put(key, new Value(value)); return new ImmutableContext(attributes); } - - private static class TestHookWithData implements Hook { - private final Object value; - HookData hookData = null; - - public TestHookWithData(Object value) { - this.value = value; - } - - public TestHookWithData() { - this("test"); - } - - @Override - public Optional before(HookContext ctx, Map hints) { - ctx.getHookData().set("before", value); - hookData = ctx.getHookData(); - return Optional.empty(); - } - - @Override - public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { - ctx.getHookData().set("after", value); - hookData = ctx.getHookData(); - } - - @Override - public void error(HookContext ctx, Exception error, Map hints) { - ctx.getHookData().set("error", value); - hookData = ctx.getHookData(); - } - - @Override - public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { - ctx.getHookData().set("finallyAfter", value); - hookData = ctx.getHookData(); - } - } } diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 97a1417a1..4511b94e1 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -16,6 +16,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mockito; import org.simplify4u.slf4jmock.LoggerMock; import org.slf4j.Logger; @@ -104,4 +106,33 @@ void shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState() { assertThat(details.getErrorCode()).isEqualTo(ErrorCode.PROVIDER_NOT_READY); } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + @DisplayName("Should support usage of HookData with/without error") + void shouldSupportUsageOfHookData(boolean isError) { + FeatureProvider provider = new TestEventsProvider(5000); + OpenFeatureAPI api = new OpenFeatureAPI(); + if (isError) { + api.setProvider("shouldSupportUsageOfHookData", provider); + } else { + api.setProviderAndWait("shouldSupportUsageOfHookData", provider); + } + + var testHook = new TestHookWithData("test-data"); + api.addHooks(testHook); + + Client client = api.getClient("shouldSupportUsageOfHookData"); + client.getBooleanDetails("key", true); + + assertThat(testHook.hookData.get("before")).isEqualTo("test-data"); + assertThat(testHook.hookData.get("finallyAfter")).isEqualTo("test-data"); + if (isError) { + assertThat(testHook.hookData.get("after")).isEqualTo(null); + assertThat(testHook.hookData.get("error")).isEqualTo("test-data"); + } else { + assertThat(testHook.hookData.get("after")).isEqualTo("test-data"); + assertThat(testHook.hookData.get("error")).isEqualTo(null); + } + } } diff --git a/src/test/java/dev/openfeature/sdk/TestHookWithData.java b/src/test/java/dev/openfeature/sdk/TestHookWithData.java new file mode 100644 index 000000000..dc415fa16 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/TestHookWithData.java @@ -0,0 +1,42 @@ +package dev.openfeature.sdk; + +import java.util.Map; +import java.util.Optional; + +class TestHookWithData implements Hook { + private final Object value; + HookData hookData = null; + + public TestHookWithData(Object value) { + this.value = value; + } + + public TestHookWithData() { + this("test"); + } + + @Override + public Optional before(HookContext ctx, Map hints) { + ctx.getHookData().set("before", value); + hookData = ctx.getHookData(); + return Optional.empty(); + } + + @Override + public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + ctx.getHookData().set("after", value); + hookData = ctx.getHookData(); + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + ctx.getHookData().set("error", value); + hookData = ctx.getHookData(); + } + + @Override + public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { + ctx.getHookData().set("finallyAfter", value); + hookData = ctx.getHookData(); + } +} From 3417827ecd03e997c6276f6325faaaee1151382e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandra=C2=A0Oberaigner?= Date: Thu, 2 Oct 2025 12:22:14 +0200 Subject: [PATCH 21/21] add hook data spec test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- .../dev/openfeature/sdk/DefaultHookData.java | 2 +- .../dev/openfeature/sdk/HookSpecTest.java | 25 +++++++++++++++++++ .../sdk/OpenFeatureClientTest.java | 7 +++--- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/DefaultHookData.java b/src/main/java/dev/openfeature/sdk/DefaultHookData.java index c34b21496..d0efe49d0 100644 --- a/src/main/java/dev/openfeature/sdk/DefaultHookData.java +++ b/src/main/java/dev/openfeature/sdk/DefaultHookData.java @@ -6,7 +6,7 @@ /** * Default implementation of HookData. */ -class DefaultHookData implements HookData { +public class DefaultHookData implements HookData { private Map data; @Override diff --git a/src/test/java/dev/openfeature/sdk/HookSpecTest.java b/src/test/java/dev/openfeature/sdk/HookSpecTest.java index 3a953d18a..56d88dfba 100644 --- a/src/test/java/dev/openfeature/sdk/HookSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSpecTest.java @@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; @@ -801,4 +802,28 @@ void doesnt_use_finally() { Hook.class.getMethod("finallyAfter", HookContext.class, FlagEvaluationDetails.class, Map.class)) .doesNotThrowAnyException(); } + + @Specification( + number = "4.6.1", + text = "hook data MUST be a structure supporting the definition of arbitrary " + + "properties, with keys of type string, and values of any type.") + @Test + void hook_data_structure() { + // Arrange + HookData hookData = new DefaultHookData(); + + // Act - Add arbitrary properties to hook data + hookData.set("stringKey", "StringValue"); // String value + hookData.set("intKey", 42); // Integer value + hookData.set("doubleKey", 3.14); // Double value + hookData.set("objectKey", new Object()); // Object value + hookData.set("nullKey", null); // Null value + + // Assert - Retrieve and validate the properties + assertEquals("StringValue", hookData.get("stringKey")); + assertEquals(42, hookData.get("intKey")); + assertEquals(3.14, hookData.get("doubleKey")); + assertNotNull(hookData.get("objectKey")); + assertNull(hookData.get("nullKey")); + } } diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 4511b94e1..88ebfaf9d 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -111,13 +111,14 @@ void shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState() { @ValueSource(booleans = {true, false}) @DisplayName("Should support usage of HookData with/without error") void shouldSupportUsageOfHookData(boolean isError) { - FeatureProvider provider = new TestEventsProvider(5000); OpenFeatureAPI api = new OpenFeatureAPI(); + FeatureProvider provider; if (isError) { - api.setProvider("shouldSupportUsageOfHookData", provider); + provider = new AlwaysBrokenWithExceptionProvider(); } else { - api.setProviderAndWait("shouldSupportUsageOfHookData", provider); + provider = new DoSomethingProvider(); } + api.setProviderAndWait("shouldSupportUsageOfHookData", provider); var testHook = new TestHookWithData("test-data"); api.addHooks(testHook);