diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index e14eeb643..e88e812a6 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -1,32 +1,13 @@ package dev.openfeature.sdk; -import lombok.Builder; -import lombok.NonNull; -import lombok.Value; -import lombok.With; +import dev.openfeature.sdk.HookContextWithoutData.HookContextWithoutDataBuilder; /** - * A data class to hold immutable context that {@link Hook} instances use. - * - * @param the type for the flag being evaluated + * A interface to hold immutable context that {@link Hook} instances use. */ -@Value -@Builder -@With -public class HookContext { - @NonNull String flagKey; - - @NonNull FlagValueType type; - - @NonNull T defaultValue; - - @NonNull EvaluationContext ctx; - - ClientMetadata clientMetadata; - Metadata providerMetadata; - +public interface HookContext { /** - * Builds a {@link HookContext} instances from request data. + * Builds a {@link HookContextWithoutData} instances from request data. * * @param key feature flag key * @param type flag value type @@ -36,21 +17,39 @@ public class HookContext { * @param defaultValue Fallback value * @param type that the flag is evaluating against * @return resulting context for hook + * @deprecated this should not be instantiated outside the SDK anymore */ - public static HookContext from( + @Deprecated + static HookContext from( String key, FlagValueType type, ClientMetadata clientMetadata, Metadata providerMetadata, EvaluationContext ctx, T defaultValue) { - return HookContext.builder() - .flagKey(key) - .type(type) - .clientMetadata(clientMetadata) - .providerMetadata(providerMetadata) - .ctx(ctx) - .defaultValue(defaultValue) - .build(); + return new HookContextWithoutData<>(key, type, defaultValue, ctx, clientMetadata, providerMetadata); + } + + /** + * Returns a builder for our default HookContext object. + */ + static HookContextWithoutDataBuilder builder() { + return HookContextWithoutData.builder(); + } + + String getFlagKey(); + + FlagValueType getType(); + + T getDefaultValue(); + + EvaluationContext getCtx(); + + ClientMetadata getClientMetadata(); + + Metadata getProviderMetadata(); + + default HookData getHookData() { + return null; } } 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..137477c11 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookContextWithData.java @@ -0,0 +1,50 @@ +package dev.openfeature.sdk; + +class HookContextWithData implements HookContext { + private final HookContext context; + private final HookData data; + + private HookContextWithData(HookContext context, HookData data) { + this.context = context; + this.data = data; + } + + public static HookContextWithData of(HookContext context, HookData data) { + return new HookContextWithData<>(context, data); + } + + @Override + public String getFlagKey() { + return context.getFlagKey(); + } + + @Override + public FlagValueType getType() { + return context.getType(); + } + + @Override + public T getDefaultValue() { + return context.getDefaultValue(); + } + + @Override + public EvaluationContext getCtx() { + return context.getCtx(); + } + + @Override + public ClientMetadata getClientMetadata() { + return context.getClientMetadata(); + } + + @Override + public Metadata getProviderMetadata() { + return context.getProviderMetadata(); + } + + @Override + public HookData getHookData() { + return data; + } +} diff --git a/src/main/java/dev/openfeature/sdk/HookContextWithoutData.java b/src/main/java/dev/openfeature/sdk/HookContextWithoutData.java new file mode 100644 index 000000000..6e5394ee1 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookContextWithoutData.java @@ -0,0 +1,48 @@ +package dev.openfeature.sdk; + +import lombok.AccessLevel; +import lombok.Builder; +import lombok.Data; +import lombok.NonNull; +import lombok.Setter; +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 +@Setter(AccessLevel.PRIVATE) +class HookContextWithoutData implements HookContext { + @NonNull String flagKey; + + @NonNull FlagValueType type; + + @NonNull T defaultValue; + + @Setter(AccessLevel.PACKAGE) + @NonNull EvaluationContext ctx; + + ClientMetadata clientMetadata; + Metadata providerMetadata; + + /** + * Builds a {@link HookContextWithoutData} instances from request data. + * + * @param key feature flag key + * @param type flag value type + * @param clientMetadata info on which client is calling + * @param providerMetadata info on the provider + * @param defaultValue Fallback value + * @param type that the flag is evaluating against + * @return resulting context for hook + */ + static HookContextWithoutData from( + String key, FlagValueType type, ClientMetadata clientMetadata, Metadata providerMetadata, T defaultValue) { + return new HookContextWithoutData<>( + key, type, defaultValue, ImmutableContext.EMPTY, clientMetadata, providerMetadata); + } +} 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..e9ebbbe58 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 = HookContextWithData.of(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 = HookContextWithData.of(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 = HookContextWithData.of(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..10c359e3e 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; + HookContextWithoutData 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 = + HookContextWithoutData.from(key, type, this.getMetadata(), provider.getMetadata(), 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..123052b7d 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,46 @@ 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() { + HookData hookData = HookData.create(); + hookData.set("test", "value"); + + HookContextWithData context = HookContextWithData.of(mock(HookContext.class), hookData); + + assertNotNull(context.getHookData()); + assertEquals("value", context.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 = HookContextWithData.of(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..67ec03d94 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 = HookContextWithoutData.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));