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..d0efe49d0 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/DefaultHookData.java @@ -0,0 +1,39 @@ +package dev.openfeature.sdk; + +import java.util.HashMap; +import java.util.Map; + +/** + * 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); + } +} diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index e14eeb643..8d4d2e13a 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -1,32 +1,56 @@ package dev.openfeature.sdk; -import lombok.Builder; +import dev.openfeature.sdk.internal.ExcludeFromGeneratedCoverageReport; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.util.Objects; +import lombok.EqualsAndHashCode; import lombok.NonNull; -import lombok.Value; -import lombok.With; +import lombok.ToString; /** * A data class to hold immutable context that {@link Hook} instances use. * * @param the type for the flag being evaluated */ -@Value -@Builder -@With -public class HookContext { - @NonNull String flagKey; +@EqualsAndHashCode +@ToString +public final class HookContext { + private final SharedHookContext sharedContext; + private EvaluationContext ctx; + private final HookData hookData; - @NonNull FlagValueType type; - - @NonNull T defaultValue; - - @NonNull EvaluationContext ctx; + HookContext(@NonNull SharedHookContext sharedContext, EvaluationContext evaluationContext, HookData hookData) { + this.sharedContext = sharedContext; + ctx = evaluationContext; + this.hookData = hookData; + } - ClientMetadata clientMetadata; - Metadata providerMetadata; + /** + * 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 a {@link HookContext} instances from request data. + * Builds {@link HookContext} instances from request data. * * @param key feature flag key * @param type flag value type @@ -36,7 +60,9 @@ public 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, @@ -51,6 +77,286 @@ public static HookContext from( .providerMetadata(providerMetadata) .ctx(ctx) .defaultValue(defaultValue) + .hookData(null) .build(); } + + /** + * 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 sharedContext.getFlagKey(); + } + + public @NonNull FlagValueType getType() { + return sharedContext.getType(); + } + + public @NonNull T getDefaultValue() { + return sharedContext.getDefaultValue(); + } + + public @NonNull EvaluationContext getCtx() { + return this.ctx; + } + + public ClientMetadata getClientMetadata() { + return sharedContext.getClientMetadata(); + } + + public Metadata getProviderMetadata() { + return sharedContext.getProviderMetadata(); + } + + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "Intentional exposure of hookData") + public HookData getHookData() { + return this.hookData; + } + + 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. + */ + @ExcludeFromGeneratedCoverageReport + @Deprecated + public HookContext withFlagKey(@NonNull String flagKey) { + 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. + */ + @ExcludeFromGeneratedCoverageReport + @Deprecated + public HookContext withType(@NonNull FlagValueType type) { + 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. + */ + @ExcludeFromGeneratedCoverageReport + @Deprecated + public HookContext withDefaultValue(@NonNull T defaultValue) { + 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. + */ + @ExcludeFromGeneratedCoverageReport + @Deprecated + public HookContext withCtx(@NonNull EvaluationContext ctx) { + 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. + */ + @ExcludeFromGeneratedCoverageReport + @Deprecated + public HookContext withClientMetadata(ClientMetadata clientMetadata) { + 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. + */ + @ExcludeFromGeneratedCoverageReport + @Deprecated + public HookContext withProviderMetadata(Metadata providerMetadata) { + 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. + */ + @ExcludeFromGeneratedCoverageReport + @Deprecated + public HookContext withHookData(HookData 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 + @ToString + public static class HookContextBuilder { + private String flagKey; + private FlagValueType type; + private T defaultValue; + private EvaluationContext ctx; + private ClientMetadata clientMetadata; + private Metadata providerMetadata; + private HookData hookData; + + 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; + } + + /** + * Builds the HookContext instance. + * + * @return a new HookContext + */ + @ExcludeFromGeneratedCoverageReport + public HookContext build() { + return new HookContext( + this.flagKey, + this.type, + this.defaultValue, + this.ctx, + this.clientMetadata, + this.providerMetadata, + this.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..bd2c5dba9 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookData.java @@ -0,0 +1,35 @@ +package dev.openfeature.sdk; + +/** + * 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); +} diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index 73518ee8e..c7a7630da 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -3,99 +3,124 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Optional; -import java.util.function.Consumer; -import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +/** + * Helper class to run hooks. Initialize {@link HookSupportData} by calling setHooks, setHookContexts + * & updateEvaluationContext in this exact order. + */ @Slf4j -@RequiredArgsConstructor -@SuppressWarnings({"unchecked", "rawtypes"}) class HookSupport { - public EvaluationContext beforeHooks( - FlagValueType flagValueType, HookContext hookCtx, List hooks, Map hints) { - return callBeforeHooks(flagValueType, hookCtx, hooks, hints); + /** + * Sets the {@link Hook}-{@link HookContext}-{@link Pair} list in the given data object with {@link HookContext} + * set to null. Filters hooks by supported {@link FlagValueType}. + * + * @param hookSupportData the data object to modify + * @param hooks the hooks to set + * @param type the flag value type to filter unsupported hooks + */ + 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; } - public void afterHooks( - FlagValueType flagValueType, - HookContext hookContext, - FlagEvaluationDetails details, - List hooks, - Map hints) { - executeHooksUnchecked(flagValueType, hooks, hook -> hook.after(hookContext, details, hints)); + /** + * 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 sharedContext the shared context from which the new {@link HookContext} is created + */ + 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()); + hookContextPair.setValue(curHookContext); + } } - public void afterAllHooks( - FlagValueType flagValueType, - HookContext hookCtx, - FlagEvaluationDetails details, - List hooks, - Map hints) { - executeHooks(flagValueType, hooks, "finally", hook -> hook.finallyAfter(hookCtx, details, hints)); + /** + * 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); + } + } + } } - public void errorHooks( - FlagValueType flagValueType, - HookContext hookCtx, - Exception e, - List hooks, - Map hints) { - executeHooks(flagValueType, hooks, "error", hook -> hook.error(hookCtx, e, hints)); - } + public void executeBeforeHooks(HookSupportData data) { + // These traverse backwards from normal. + List> reversedHooks = new ArrayList<>(data.getHooks()); + Collections.reverse(reversedHooks); - 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); - } + for (Pair hookContextPair : reversedHooks) { + var hook = hookContextPair.getKey(); + var hookContext = hookContextPair.getValue(); + + Optional returnedEvalContext = Optional.ofNullable( + hook.before(hookContext, data.getHints())) + .orElse(Optional.empty()); + if (returnedEvalContext.isPresent()) { + // update shared evaluation context for all hooks + updateEvaluationContext(data, data.getEvaluationContext().merge(returnedEvalContext.get())); } } } - // before, error, and finally hooks shouldn't throw - private void executeChecked(Hook hook, Consumer> hookCode, String hookMethod) { - try { - hookCode.accept(hook); - } catch (Exception exception) { - log.error( - "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", - hookMethod, - hook.getClass(), - exception); + 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, data.getHints()); + } 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 - private void executeHooksUnchecked(FlagValueType flagValueType, List hooks, Consumer> hookCode) { - if (hooks != null) { - for (Hook hook : hooks) { - if (hook.supportsFlagValueType(flagValueType)) { - hookCode.accept(hook); - } - } + public void executeAfterHooks(HookSupportData data, FlagEvaluationDetails details) { + for (Pair hookContextPair : data.getHooks()) { + var hook = hookContextPair.getKey(); + var hookContext = hookContextPair.getValue(); + hook.after(hookContext, details, data.getHints()); } } - private EvaluationContext callBeforeHooks( - FlagValueType flagValueType, HookContext hookCtx, List hooks, Map hints) { - // These traverse backwards from normal. - List reversedHooks = new ArrayList<>(hooks); - 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()); - } + 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, data.getHints()); + } catch (Exception e) { + log.error( + "Unhandled exception when running {} hook {} (only 'after' hooks should throw)", + "finally", + hook.getClass(), + e); } } - return context; } } 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..2d3346ba1 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookSupportData.java @@ -0,0 +1,18 @@ +package dev.openfeature.sdk; + +import java.util.List; +import java.util.Map; +import lombok.Getter; + +/** + * Encapsulates data for hook execution per flag evaluation. + */ +@Getter +class HookSupportData { + + List> hooks; + EvaluationContext evaluationContext; + Map hints; + + HookSupportData() {} +} 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/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/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index b5522b66a..614bc1e34 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -48,9 +48,10 @@ public class OpenFeatureClient implements Client { private final String version; private final ConcurrentLinkedQueue clientHooks; - private final HookSupport hookSupport; private final AtomicReference evaluationContext = new AtomicReference<>(); + private final HookSupport hookSupport; + /** * Deprecated public constructor. Use OpenFeature.API.getClient() instead. * @@ -67,8 +68,8 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String ve this.openfeatureApi = openFeatureAPI; this.domain = domain; this.version = version; - this.clientHooks = new ConcurrentLinkedQueue<>(); this.hookSupport = new HookSupport(); + this.clientHooks = new ConcurrentLinkedQueue<>(); } /** @@ -159,37 +160,32 @@ 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; - List mergedHooks = null; - HookContext afterHookContext = null; + hookSupportData.hints = Collections.unmodifiableMap(flagOptions.getHookHints()); 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(); - mergedHooks = ObjectUtils.merge( + // 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.setHookContexts(hookSupportData, sharedHookContext); - 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); + var evalContext = mergeEvaluationContext(ctx); + hookSupport.updateEvaluationContext(hookSupportData, evalContext); + + hookSupport.executeBeforeHooks(hookSupportData); // "short circuit" if the provider is in NOT_READY or FATAL state if (ProviderState.NOT_READY.equals(state)) { @@ -199,17 +195,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, hookSupportData.getEvaluationContext()); details = FlagEvaluationDetails.from(providerEval, key); if (details.getErrorCode() != null) { var error = ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, afterHookContext, error, mergedHooks, hints); + hookSupport.executeErrorHooks(hookSupportData, error); } else { - hookSupport.afterHooks(type, afterHookContext, details, mergedHooks, hints); + hookSupport.executeAfterHooks(hookSupportData, details); } } catch (Exception e) { if (details == null) { @@ -222,9 +218,13 @@ private FlagEvaluationDetails evaluateFlag( } details.setErrorMessage(e.getMessage()); enrichDetailsWithErrorDefaults(defaultValue, details); - hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints); + if (hookSupportData.getHooks() != null) { + hookSupport.executeErrorHooks(hookSupportData, e); + } } finally { - hookSupport.afterAllHooks(type, afterHookContext, details, mergedHooks, hints); + if (hookSupportData.getHooks() != null) { + hookSupport.executeAfterAllHooks(hookSupportData, details); + } } 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..765be9f2d --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/Pair.java @@ -0,0 +1,29 @@ +package dev.openfeature.sdk; + +import lombok.Setter; +import lombok.ToString; + +@ToString +class Pair { + private final K key; + + @Setter + private V value; + + private Pair(K key, V value) { + this.key = key; + this.value = value; + } + + public K getKey() { + return key; + } + + public V getValue() { + return 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 new file mode 100644 index 000000000..8faab37b2 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/SharedHookContext.java @@ -0,0 +1,32 @@ +package dev.openfeature.sdk; + +import lombok.EqualsAndHashCode; +import lombok.Getter; + +@Getter +@EqualsAndHashCode +class SharedHookContext { + + private final String flagKey; + private final FlagValueType type; + private final ClientMetadata clientMetadata; + private final Metadata providerMetadata; + private final T defaultValue; + + public SharedHookContext( + String flagKey, + FlagValueType type, + ClientMetadata clientMetadata, + Metadata providerMetadata, + T defaultValue) { + this.flagKey = flagKey; + 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/DefaultHookDataTest.java b/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java new file mode 100644 index 000000000..ac50988ea --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/DefaultHookDataTest.java @@ -0,0 +1,81 @@ +package dev.openfeature.sdk; + +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 + void shouldStoreAndRetrieveValues() { + var hookData = new DefaultHookData(); + + 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() { + var hookData = new DefaultHookData(); + + assertNull(hookData.get("nonexistent")); + } + + @Test + void shouldSupportTypeSafeRetrieval() { + var hookData = new DefaultHookData(); + + 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() { + var hookData = new DefaultHookData(); + + assertNull(hookData.get("missing", String.class)); + } + + @Test + void shouldThrowClassCastExceptionForWrongType() { + var hookData = new DefaultHookData(); + + hookData.set("string", "not a number"); + + assertThrows(ClassCastException.class, () -> { + hookData.get("string", Integer.class); + }); + } + + @Test + void shouldOverwriteExistingValues() { + var hookData = new DefaultHookData(); + + hookData.set("key", "original"); + assertEquals("original", hookData.get("key")); + + hookData.set("key", "updated"); + assertEquals("updated", hookData.get("key")); + } + + @Test + void shouldSupportNullValues() { + var hookData = new DefaultHookData(); + + hookData.set("nullKey", null); + assertNull(hookData.get("nullKey")); + assertNull(hookData.get("nullKey", String.class)); + } +} diff --git a/src/test/java/dev/openfeature/sdk/HookContextTest.java b/src/test/java/dev/openfeature/sdk/HookContextTest.java index 2196b8b1f..a37ade9d5 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; 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/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index 02a8ff90c..b1bb70ba1 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -2,13 +2,14 @@ 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; @@ -17,22 +18,30 @@ 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() { 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"); + 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"))); - HookSupport hookSupport = new HookSupport(); - EvaluationContext result = hookSupport.beforeHooks( - FlagValueType.STRING, hookContext, Arrays.asList(hook1, hook2), Collections.emptyMap()); + var sharedContext = getBaseHookContextForType(FlagValueType.STRING); + var hookSupportData = new HookSupportData(); + hookSupport.setHooks(hookSupportData, Arrays.asList(hook1, hook2), FlagValueType.STRING); + hookSupport.setHookContexts(hookSupportData, sharedContext); + hookSupport.updateEvaluationContext(hookSupportData, baseEvalContext); + + hookSupport.executeBeforeHooks(hookSupportData); + + EvaluationContext result = hookSupportData.getEvaluationContext(); assertThat(result.getValue("bla").asString()).isEqualTo("blubber"); assertThat(result.getValue("foo").asString()).isEqualTo("bar"); @@ -44,37 +53,11 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { @DisplayName("should always call generic hook") void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { Hook genericHook = mockGenericHook(); - HookSupport hookSupport = new HookSupport(); - EvaluationContext baseContext = new ImmutableContext(); - IllegalStateException expectedException = new IllegalStateException("All fine, just a test"); - HookContext hookContext = new HookContext<>( - "flagKey", - flagValueType, - createDefaultValue(flagValueType), - baseContext, - () -> "client", - () -> "provider"); - - hookSupport.beforeHooks( - flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap()); - hookSupport.afterHooks( - flagValueType, - hookContext, - FlagEvaluationDetails.builder().build(), - Collections.singletonList(genericHook), - Collections.emptyMap()); - hookSupport.afterAllHooks( - flagValueType, - hookContext, - FlagEvaluationDetails.builder().build(), - Collections.singletonList(genericHook), - Collections.emptyMap()); - hookSupport.errorHooks( - flagValueType, - hookContext, - expectedException, - Collections.singletonList(genericHook), - Collections.emptyMap()); + + var hookSupportData = new HookSupportData(); + hookSupport.setHooks(hookSupportData, List.of(genericHook), flagValueType); + + callAllHooks(hookSupportData); verify(genericHook).before(any(), any()); verify(genericHook).after(any(), any(), any()); @@ -82,6 +65,82 @@ 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) { + var testHook = new TestHookWithData(); + var hookSupportData = new HookSupportData(); + hookSupport.setHooks(hookSupportData, List.of(testHook), flagValueType); + hookSupport.setHookContexts(hookSupportData, getBaseHookContextForType(flagValueType)); + + hookSupport.executeBeforeHooks(hookSupportData); + assertHookData(testHook, "before"); + + hookSupport.executeAfterHooks( + hookSupportData, FlagEvaluationDetails.builder().build()); + assertHookData(testHook, "before", "after"); + + hookSupport.executeAfterAllHooks( + hookSupportData, FlagEvaluationDetails.builder().build()); + assertHookData(testHook, "before", "after", "finallyAfter"); + + hookSupport.executeErrorHooks(hookSupportData, 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 TestHookWithData(1); + var testHook2 = new TestHookWithData(2); + + var hookSupportData = new HookSupportData(); + hookSupport.setHooks(hookSupportData, List.of(testHook1, testHook2), flagValueType); + hookSupport.setHookContexts(hookSupportData, getBaseHookContextForType(flagValueType)); + + callAllHooks(hookSupportData); + + assertHookData(testHook1, 1, "before", "after", "finallyAfter", "error"); + assertHookData(testHook2, 2, "before", "after", "finallyAfter", "error"); + } + + private static void callAllHooks(HookSupportData hookSupportData) { + hookSupport.executeBeforeHooks(hookSupportData); + 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) { + for (String expectedKey : expectedKeys) { + assertThat(testHook.hookData.get(expectedKey)) + .withFailMessage("Expected key %s not present in hook data", expectedKey) + .isNotNull(); + } + } + + private static void assertHookData(TestHookWithData testHook, Object expectedValue, String... expectedKeys) { + 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: @@ -102,7 +161,6 @@ private Object createDefaultValue(FlagValueType flagValueType) { private EvaluationContext evaluationContextWithValue(String key, String value) { Map attributes = new HashMap<>(); attributes.put(key, new Value(value)); - EvaluationContext baseContext = new ImmutableContext(attributes); - return baseContext; + return new ImmutableContext(attributes); } } diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 97a1417a1..88ebfaf9d 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,34 @@ 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) { + OpenFeatureAPI api = new OpenFeatureAPI(); + FeatureProvider provider; + if (isError) { + provider = new AlwaysBrokenWithExceptionProvider(); + } else { + provider = new DoSomethingProvider(); + } + 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(); + } +} diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index 5bc89d03d..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,12 +52,36 @@ 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 StringHook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); + client.addHooks(new BooleanHook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); + client.addHooks(new IntegerHook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); + client.addHooks(new DoubleHook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.ofNullable(new ImmutableContext()); + } + }); Map invocationAttrs = new HashMap<>(); invocationAttrs.put("invoke", new Value(3));