diff --git a/pom.xml b/pom.xml index 0754cafbd..07c0e23a7 100644 --- a/pom.xml +++ b/pom.xml @@ -348,7 +348,7 @@ - + maven-dependency-plugin 3.8.1 diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index e88e812a6..e14eeb643 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -1,13 +1,32 @@ package dev.openfeature.sdk; -import dev.openfeature.sdk.HookContextWithoutData.HookContextWithoutDataBuilder; +import lombok.Builder; +import lombok.NonNull; +import lombok.Value; +import lombok.With; /** - * A interface to hold immutable context that {@link Hook} instances use. + * A data class to hold immutable context that {@link Hook} instances use. + * + * @param the type for the flag being evaluated */ -public interface HookContext { +@Value +@Builder +@With +public class HookContext { + @NonNull String flagKey; + + @NonNull FlagValueType type; + + @NonNull T defaultValue; + + @NonNull EvaluationContext ctx; + + ClientMetadata clientMetadata; + Metadata providerMetadata; + /** - * Builds a {@link HookContextWithoutData} instances from request data. + * Builds a {@link HookContext} instances from request data. * * @param key feature flag key * @param type flag value type @@ -17,39 +36,21 @@ public interface 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 */ - @Deprecated - static HookContext from( + public static HookContext from( String key, FlagValueType type, ClientMetadata clientMetadata, Metadata providerMetadata, EvaluationContext ctx, T defaultValue) { - 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; + return HookContext.builder() + .flagKey(key) + .type(type) + .clientMetadata(clientMetadata) + .providerMetadata(providerMetadata) + .ctx(ctx) + .defaultValue(defaultValue) + .build(); } } 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 137477c11..000000000 --- a/src/main/java/dev/openfeature/sdk/HookContextWithData.java +++ /dev/null @@ -1,50 +0,0 @@ -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 deleted file mode 100644 index df1ed6ad1..000000000 --- a/src/main/java/dev/openfeature/sdk/HookContextWithoutData.java +++ /dev/null @@ -1,55 +0,0 @@ -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); - } - - /** - * Make the builder visible for javadocs. - * - * @param flag value type - */ - public static class HookContextWithoutDataBuilder {} -} diff --git a/src/main/java/dev/openfeature/sdk/HookData.java b/src/main/java/dev/openfeature/sdk/HookData.java deleted file mode 100644 index c7c644a93..000000000 --- a/src/main/java/dev/openfeature/sdk/HookData.java +++ /dev/null @@ -1,81 +0,0 @@ -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 e9ebbbe58..73518ee8e 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.BiConsumer; +import java.util.function.Consumer; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -15,81 +15,52 @@ class HookSupport { public EvaluationContext beforeHooks( - FlagValueType flagValueType, - HookContext hookCtx, - List> hookDataPairs, - Map hints) { - return callBeforeHooks(flagValueType, hookCtx, hookDataPairs, hints); + FlagValueType flagValueType, HookContext hookCtx, List hooks, Map hints) { + return callBeforeHooks(flagValueType, hookCtx, hooks, hints); } public void afterHooks( FlagValueType flagValueType, HookContext hookContext, FlagEvaluationDetails details, - List> hookDataPairs, + List hooks, Map hints) { - executeHooksUnchecked( - flagValueType, hookDataPairs, hookContext, (hook, ctx) -> hook.after(ctx, details, hints)); + executeHooksUnchecked(flagValueType, hooks, hook -> hook.after(hookContext, details, hints)); } public void afterAllHooks( FlagValueType flagValueType, HookContext hookCtx, FlagEvaluationDetails details, - List> hookDataPairs, + List hooks, Map hints) { - executeHooks( - flagValueType, - hookDataPairs, - hookCtx, - "finally", - (hook, ctx) -> hook.finallyAfter(ctx, details, hints)); + executeHooks(flagValueType, hooks, "finally", hook -> hook.finallyAfter(hookCtx, details, hints)); } public void errorHooks( FlagValueType flagValueType, HookContext hookCtx, Exception e, - List> hookDataPairs, + List hooks, Map 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; + executeHooks(flagValueType, hooks, "error", hook -> hook.error(hookCtx, e, hints)); } 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(); - executeChecked(hook, hookData, hookContext, hookCode, hookMethod); + FlagValueType flagValueType, List hooks, String hookMethod, Consumer> hookCode) { + if (hooks != null) { + for (Hook hook : hooks) { + if (hook.supportsFlagValueType(flagValueType)) { + executeChecked(hook, hookCode, hookMethod); + } } } } // before, error, and finally hooks shouldn't throw - private void executeChecked( - Hook hook, - HookData hookData, - HookContext hookContext, - BiConsumer, HookContext> hookCode, - String hookMethod) { + private void executeChecked(Hook hook, Consumer> hookCode, String hookMethod) { try { - var hookCtxWithData = HookContextWithData.of(hookContext, hookData); - hookCode.accept(hook, hookCtxWithData); + hookCode.accept(hook); } catch (Exception exception) { log.error( "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", @@ -100,41 +71,29 @@ 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(); - var hookCtxWithData = HookContextWithData.of(hookContext, hookData); - hookCode.accept(hook, hookCtxWithData); + private void executeHooksUnchecked(FlagValueType flagValueType, List hooks, Consumer> hookCode) { + if (hooks != null) { + for (Hook hook : hooks) { + if (hook.supportsFlagValueType(flagValueType)) { + hookCode.accept(hook); + } } } } private EvaluationContext callBeforeHooks( - FlagValueType flagValueType, - HookContext hookCtx, - List> hookDataPairs, - Map hints) { + FlagValueType flagValueType, HookContext hookCtx, List hooks, Map hints) { // These traverse backwards from normal. - List> reversedHooks = new ArrayList<>(hookDataPairs); + List reversedHooks = new ArrayList<>(hooks); Collections.reverse(reversedHooks); EvaluationContext context = hookCtx.getCtx(); - - 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()); + 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()); + } } } return context; diff --git a/src/main/java/dev/openfeature/sdk/ImmutableContext.java b/src/main/java/dev/openfeature/sdk/ImmutableContext.java index e4916dfca..8560c369e 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableContext.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableContext.java @@ -20,8 +20,6 @@ @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 10c359e3e..b5522b66a 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -164,27 +164,32 @@ private FlagEvaluationDetails evaluateFlag( var hints = Collections.unmodifiableMap(flagOptions.getHookHints()); FlagEvaluationDetails details = null; - List mergedHooks; - List> hookDataPairs = null; - HookContextWithoutData hookContext = null; + List mergedHooks = null; + HookContext afterHookContext = null; try { - final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain); + 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 = - 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)); + var provider = stateManager.getProvider(); + var state = stateManager.getState(); mergedHooks = ObjectUtils.merge( provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks()); - hookDataPairs = hookSupport.getHookDataPairs(mergedHooks, type); - var mergedCtx = hookSupport.beforeHooks(type, hookContext, hookDataPairs, hints); - hookContext.setCtx(mergedCtx); + + 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); // "short circuit" if the provider is in NOT_READY or FATAL state if (ProviderState.NOT_READY.equals(state)) { @@ -202,9 +207,9 @@ private FlagEvaluationDetails evaluateFlag( var error = ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, hookContext, error, hookDataPairs, hints); + hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints); } else { - hookSupport.afterHooks(type, hookContext, details, hookDataPairs, hints); + hookSupport.afterHooks(type, afterHookContext, details, mergedHooks, hints); } } catch (Exception e) { if (details == null) { @@ -217,9 +222,9 @@ private FlagEvaluationDetails evaluateFlag( } details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, hookContext, e, hookDataPairs, hints); + hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints); } finally { - hookSupport.afterAllHooks(type, hookContext, details, hookDataPairs, hints); + hookSupport.afterAllHooks(type, afterHookContext, details, mergedHooks, hints); } return details; diff --git a/src/main/java/dev/openfeature/sdk/Pair.java b/src/main/java/dev/openfeature/sdk/Pair.java deleted file mode 100644 index bc6614093..000000000 --- a/src/main/java/dev/openfeature/sdk/Pair.java +++ /dev/null @@ -1,28 +0,0 @@ -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 123052b7d..2196b8b1f 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.*; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import org.junit.jupiter.api.Test; @@ -29,46 +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() { - 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 deleted file mode 100644 index eacbeeb78..000000000 --- a/src/test/java/dev/openfeature/sdk/HookDataTest.java +++ /dev/null @@ -1,79 +0,0 @@ -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 67ec03d94..02a8ff90c 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -9,7 +9,6 @@ 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; @@ -24,15 +23,8 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { Map attributes = new HashMap<>(); attributes.put("baseKey", new Value("baseValue")); EvaluationContext baseContext = new ImmutableContext(attributes); - FlagValueType valueType = FlagValueType.STRING; - HookContext hookContext = HookContextWithoutData.builder() - .flagKey("flagKey") - .type(valueType) - .defaultValue("defaultValue") - .ctx(baseContext) - .clientMetadata(() -> "client") - .providerMetadata(() -> "provider") - .build(); + HookContext hookContext = new HookContext<>( + "flagKey", FlagValueType.STRING, "defaultValue", baseContext, () -> "client", () -> "provider"); Hook hook1 = mockStringHook(); Hook hook2 = mockStringHook(); when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber"))); @@ -40,10 +32,7 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { HookSupport hookSupport = new HookSupport(); EvaluationContext result = hookSupport.beforeHooks( - valueType, - hookContext, - hookSupport.getHookDataPairs(Arrays.asList(hook1, hook2), valueType), - Collections.emptyMap()); + FlagValueType.STRING, hookContext, Arrays.asList(hook1, hook2), Collections.emptyMap()); assertThat(result.getValue("bla").asString()).isEqualTo("blubber"); assertThat(result.getValue("foo").asString()).isEqualTo("bar"); @@ -56,32 +45,36 @@ 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 = HookContext.builder() - .flagKey("flagKey") - .type(flagValueType) - .defaultValue(createDefaultValue(flagValueType)) - .ctx(baseContext) - .clientMetadata(() -> "client") - .providerMetadata(() -> "provider") - .build(); + HookContext hookContext = new HookContext<>( + "flagKey", + flagValueType, + createDefaultValue(flagValueType), + baseContext, + () -> "client", + () -> "provider"); - hookSupport.beforeHooks(flagValueType, hookContext, hookDataPairs, Collections.emptyMap()); + hookSupport.beforeHooks( + flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap()); hookSupport.afterHooks( flagValueType, hookContext, FlagEvaluationDetails.builder().build(), - hookDataPairs, + Collections.singletonList(genericHook), Collections.emptyMap()); hookSupport.afterAllHooks( flagValueType, hookContext, FlagEvaluationDetails.builder().build(), - hookDataPairs, + Collections.singletonList(genericHook), + Collections.emptyMap()); + hookSupport.errorHooks( + flagValueType, + hookContext, + expectedException, + Collections.singletonList(genericHook), Collections.emptyMap()); - hookSupport.errorHooks(flagValueType, hookContext, expectedException, hookDataPairs, Collections.emptyMap()); verify(genericHook).before(any(), any()); verify(genericHook).after(any(), any(), any()); @@ -89,101 +82,6 @@ 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: @@ -207,46 +105,4 @@ 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 9fe043722..5bc89d03d 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -54,30 +54,6 @@ 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));