From df49024471836cfcb59a8f0531209c659f377f4b Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 15 Sep 2025 10:58:02 +0200 Subject: [PATCH 01/11] add bench Signed-off-by: christian.lutnik --- .../sdk/benchmark/AllocationBenchmark.java | 56 ++++++++++++++++++- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index 5bc89d03d..eea91632e 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -32,9 +32,9 @@ public class AllocationBenchmark { // 10K iterations works well with Xmx1024m (we don't want to run out of memory) private static final int ITERATIONS = 10000; - @Benchmark - @BenchmarkMode(Mode.SingleShotTime) - @Fork(jvmArgsAppend = {"-Xmx1024m", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseEpsilonGC"}) + //@Benchmark + //@BenchmarkMode(Mode.SingleShotTime) + //@Fork(jvmArgsAppend = {"-Xmx1024m", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseEpsilonGC"}) public void run() { OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); @@ -67,4 +67,54 @@ public Optional before(HookContext ctx, Map globalAttrs = new HashMap<>(); + globalAttrs.put("global", new Value(1)); + EvaluationContext globalContext = new ImmutableContext(globalAttrs); + OpenFeatureAPI.getInstance().setEvaluationContext(globalContext); + + Client client = OpenFeatureAPI.getInstance().getClient(); + + Map clientAttrs = new HashMap<>(); + clientAttrs.put("client", new Value(2)); + client.setEvaluationContext(new ImmutableContext(clientAttrs)); + + Map transactionAttr = new HashMap<>(); + transactionAttr.put("trans", new Value(4)); + + Map transactionAttr2 = new HashMap<>(); + transactionAttr2.put("trans2", new Value(5)); + + Map invocationAttrs = new HashMap<>(); + invocationAttrs.put("invoke", new Value(3)); + EvaluationContext invocationContext = new ImmutableContext(invocationAttrs); + + for (int i = 0; i < 100; i++) { + OpenFeatureAPI.getInstance().setTransactionContext(new ImmutableContext(transactionAttr)); + + for (int j = 0; j < 10; j++) { + client.getBooleanValue(BOOLEAN_FLAG_KEY, false); + client.getStringValue(STRING_FLAG_KEY, "default"); + client.getIntegerValue(INT_FLAG_KEY, 0); + client.getDoubleValue(FLOAT_FLAG_KEY, 0.0); + client.getObjectDetails(OBJECT_FLAG_KEY, new Value(new ImmutableStructure()), invocationContext); + } + + OpenFeatureAPI.getInstance().setTransactionContext(new ImmutableContext(transactionAttr2)); + + for (int j = 0; j < 10; j++) { + client.getBooleanValue(BOOLEAN_FLAG_KEY, false); + client.getStringValue(STRING_FLAG_KEY, "default"); + client.getIntegerValue(INT_FLAG_KEY, 0); + client.getDoubleValue(FLOAT_FLAG_KEY, 0.0); + client.getObjectDetails(OBJECT_FLAG_KEY, new Value(new ImmutableStructure()), invocationContext); + } + } + } } From ea3990dc9abe711cfa44efec7312b29dc818b850 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 15 Sep 2025 11:23:33 +0200 Subject: [PATCH 02/11] fix bench Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/benchmark/AllocationBenchmark.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index eea91632e..31f817118 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -14,6 +14,7 @@ import dev.openfeature.sdk.ImmutableStructure; import dev.openfeature.sdk.NoOpProvider; import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.ThreadLocalTransactionContextPropagator; import dev.openfeature.sdk.Value; import java.util.HashMap; import java.util.Map; @@ -74,6 +75,8 @@ public Optional before(HookContext ctx, Map globalAttrs = new HashMap<>(); globalAttrs.put("global", new Value(1)); EvaluationContext globalContext = new ImmutableContext(globalAttrs); From 9d49b91e464f64ece7d74b95c907620845314c2e Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 15 Oct 2025 15:29:28 +0200 Subject: [PATCH 03/11] merge master Signed-off-by: christian.lutnik --- .../openfeature/sdk/benchmark/AllocationBenchmark.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index d7668b25d..e1a6f489c 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import org.junit.jupiter.api.Test; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; @@ -37,9 +38,9 @@ public class AllocationBenchmark { // 10K iterations works well with Xmx1024m (we don't want to run out of memory) private static final int ITERATIONS = 10000; - //@Benchmark - //@BenchmarkMode(Mode.SingleShotTime) - //@Fork(jvmArgsAppend = {"-Xmx1024m", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseEpsilonGC"}) + // @Benchmark + // @BenchmarkMode(Mode.SingleShotTime) + // @Fork(jvmArgsAppend = {"-Xmx1024m", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseEpsilonGC"}) public void run() { OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); @@ -100,6 +101,7 @@ public Optional before(HookContext ctx, Map Date: Wed, 12 Nov 2025 10:26:22 +0100 Subject: [PATCH 04/11] improve benchmark Signed-off-by: christian.lutnik --- .../sdk/benchmark/AllocationBenchmark.java | 74 ++++++------------- .../benchmark/AllocationBenchmarkState.java | 50 +++++++++++++ 2 files changed, 74 insertions(+), 50 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index e1a6f489c..9fdfc5629 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -18,16 +18,15 @@ import dev.openfeature.sdk.ObjectHook; import dev.openfeature.sdk.OpenFeatureAPI; import dev.openfeature.sdk.StringHook; -import dev.openfeature.sdk.ThreadLocalTransactionContextPropagator; import dev.openfeature.sdk.Value; import java.util.HashMap; import java.util.Map; import java.util.Optional; -import org.junit.jupiter.api.Test; import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; -import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.infra.Blackhole; /** * Runs a large volume of flag evaluations on a VM with 1G memory and GC @@ -99,55 +98,30 @@ public Optional before(HookContext ctx, Map globalAttrs = new HashMap<>(); - globalAttrs.put("global", new Value(1)); - EvaluationContext globalContext = new ImmutableContext(globalAttrs); - OpenFeatureAPI.getInstance().setEvaluationContext(globalContext); - - Client client = OpenFeatureAPI.getInstance().getClient(); - - Map clientAttrs = new HashMap<>(); - clientAttrs.put("client", new Value(2)); - client.setEvaluationContext(new ImmutableContext(clientAttrs)); - - Map transactionAttr = new HashMap<>(); - transactionAttr.put("trans", new Value(4)); - - Map transactionAttr2 = new HashMap<>(); - transactionAttr2.put("trans2", new Value(5)); - - Map invocationAttrs = new HashMap<>(); - invocationAttrs.put("invoke", new Value(3)); - EvaluationContext invocationContext = new ImmutableContext(invocationAttrs); - - for (int i = 0; i < 100; i++) { - OpenFeatureAPI.getInstance().setTransactionContext(new ImmutableContext(transactionAttr)); - - for (int j = 0; j < 10; j++) { - client.getBooleanValue(BOOLEAN_FLAG_KEY, false); - client.getStringValue(STRING_FLAG_KEY, "default"); - client.getIntegerValue(INT_FLAG_KEY, 0); - client.getDoubleValue(FLOAT_FLAG_KEY, 0.0); - client.getObjectDetails(OBJECT_FLAG_KEY, new Value(new ImmutableStructure()), invocationContext); - } + //@Test + public void context(Blackhole blackhole, AllocationBenchmarkState state) { + OpenFeatureAPI.getInstance().setTransactionContext(new ImmutableContext(state.transactionAttr)); + + for (int j = 0; j < 2; j++) { + blackhole.consume(state.client.getBooleanValue(BOOLEAN_FLAG_KEY, false)); + blackhole.consume(state.client.getStringValue(STRING_FLAG_KEY, "default")); + blackhole.consume(state.client.getIntegerValue(INT_FLAG_KEY, 0, state.invocationContext)); + blackhole.consume(state.client.getDoubleValue(FLOAT_FLAG_KEY, 0.0)); + blackhole.consume(state.client.getObjectDetails(OBJECT_FLAG_KEY, new Value(new ImmutableStructure()), + state.invocationContext)); + } - OpenFeatureAPI.getInstance().setTransactionContext(new ImmutableContext(transactionAttr2)); + OpenFeatureAPI.getInstance().setTransactionContext(new ImmutableContext(state.transactionAttr2)); - for (int j = 0; j < 10; j++) { - client.getBooleanValue(BOOLEAN_FLAG_KEY, false); - client.getStringValue(STRING_FLAG_KEY, "default"); - client.getIntegerValue(INT_FLAG_KEY, 0); - client.getDoubleValue(FLOAT_FLAG_KEY, 0.0); - client.getObjectDetails(OBJECT_FLAG_KEY, new Value(new ImmutableStructure()), invocationContext); - } + for (int j = 0; j < 2; j++) { + blackhole.consume(state.client.getBooleanValue(BOOLEAN_FLAG_KEY, false)); + blackhole.consume(state.client.getStringValue(STRING_FLAG_KEY, "default")); + blackhole.consume(state.client.getIntegerValue(INT_FLAG_KEY, 0, state.invocationContext)); + blackhole.consume(state.client.getDoubleValue(FLOAT_FLAG_KEY, 0.0)); + blackhole.consume(state.client.getObjectDetails(OBJECT_FLAG_KEY, new Value(new ImmutableStructure()), + state.invocationContext)); } } } diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java new file mode 100644 index 000000000..56f62fce1 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java @@ -0,0 +1,50 @@ +package dev.openfeature.sdk.benchmark; + +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.ImmutableContext; +import dev.openfeature.sdk.NoOpProvider; +import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.ThreadLocalTransactionContextPropagator; +import dev.openfeature.sdk.Value; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import java.util.HashMap; +import java.util.Map; + +@State(Scope.Benchmark) +public class AllocationBenchmarkState { + public final Client client; + public final Map transactionAttr; + public final Map transactionAttr2; + public final EvaluationContext invocationContext; + + public AllocationBenchmarkState(){ + long start = System.currentTimeMillis(); + OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); + OpenFeatureAPI.getInstance().setTransactionContextPropagator(new ThreadLocalTransactionContextPropagator()); + long end = System.currentTimeMillis(); + System.out.println("Setup time: " + (end - start) + "ms"); + Map globalAttrs = new HashMap<>(); + globalAttrs.put("global", new Value(1)); + EvaluationContext globalContext = new ImmutableContext(globalAttrs); + OpenFeatureAPI.getInstance().setEvaluationContext(globalContext); + + client = OpenFeatureAPI.getInstance().getClient(); + + Map clientAttrs = new HashMap<>(); + clientAttrs.put("client", new Value(2)); + client.setEvaluationContext(new ImmutableContext(clientAttrs)); + + transactionAttr = new HashMap<>(); + transactionAttr.put("trans", new Value(4)); + + transactionAttr2 = new HashMap<>(); + transactionAttr2.put("trans2", new Value(5)); + + Map invocationAttrs = new HashMap<>(); + invocationAttrs.put("invoke", new Value(3)); + invocationContext = new ImmutableContext(invocationAttrs); + + } +} From 2e9af46afa8f44916f2acdac03c30a6fb38ddcdb Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Wed, 12 Nov 2025 11:12:32 +0100 Subject: [PATCH 05/11] improve benchmark Signed-off-by: christian.lutnik --- .../sdk/benchmark/AllocationBenchmark.java | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index 9fdfc5629..437292fa8 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -22,6 +22,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import org.junit.jupiter.api.Test; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Fork; import org.openjdk.jmh.annotations.Scope; @@ -99,8 +100,7 @@ public Optional before(HookContext ctx, Map Date: Wed, 19 Nov 2025 11:41:06 +0100 Subject: [PATCH 06/11] add str hook for ctx Signed-off-by: christian.lutnik --- .../sdk/benchmark/AllocationBenchmarkState.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java index 56f62fce1..f923af2ae 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmarkState.java @@ -2,15 +2,18 @@ import dev.openfeature.sdk.Client; import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.ImmutableContext; import dev.openfeature.sdk.NoOpProvider; import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.StringHook; import dev.openfeature.sdk.ThreadLocalTransactionContextPropagator; import dev.openfeature.sdk.Value; import org.openjdk.jmh.annotations.Scope; import org.openjdk.jmh.annotations.State; import java.util.HashMap; import java.util.Map; +import java.util.Optional; @State(Scope.Benchmark) public class AllocationBenchmarkState { @@ -19,7 +22,7 @@ public class AllocationBenchmarkState { public final Map transactionAttr2; public final EvaluationContext invocationContext; - public AllocationBenchmarkState(){ + public AllocationBenchmarkState() { long start = System.currentTimeMillis(); OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); OpenFeatureAPI.getInstance().setTransactionContextPropagator(new ThreadLocalTransactionContextPropagator()); @@ -46,5 +49,15 @@ public AllocationBenchmarkState(){ invocationAttrs.put("invoke", new Value(3)); invocationContext = new ImmutableContext(invocationAttrs); + Map hookAttrs = new HashMap<>(); + hookAttrs.put("hook", new Value(30)); + Optional hookCtx = Optional.of(new ImmutableContext(hookAttrs)); + + client.addHooks(new StringHook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return hookCtx; + } + }); } } From 8a0770e0dcd5f59fbab1b5fae66c04840ba1cace Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Thu, 20 Nov 2025 14:56:51 +0100 Subject: [PATCH 07/11] cache hooks Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/FeatureProvider.java | 11 +++++ .../sdk/FlagEvaluationOptions.java | 16 +++++++ .../java/dev/openfeature/sdk/HookSupport.java | 47 ++++++++++++++----- .../dev/openfeature/sdk/OpenFeatureAPI.java | 34 ++++++++++---- .../openfeature/sdk/OpenFeatureClient.java | 46 +++++++++++++----- .../dev/openfeature/sdk/HookSupportTest.java | 8 ++-- 6 files changed, 127 insertions(+), 35 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/FeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProvider.java index 22819ef10..c241a11a2 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProvider.java @@ -15,6 +15,17 @@ default List getProviderHooks() { return new ArrayList<>(); } + default List getProviderHooks(FlagValueType flagType) { + var allHooks = getProviderHooks(); + var filteredHooks = new ArrayList(allHooks.size()); + for (Hook hook : allHooks) { + if (hook.supportsFlagValueType(flagType)) { + filteredHooks.add(hook); + } + } + return filteredHooks; + } + ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx); ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx); diff --git a/src/main/java/dev/openfeature/sdk/FlagEvaluationOptions.java b/src/main/java/dev/openfeature/sdk/FlagEvaluationOptions.java index 01ecb9b2e..c612d880c 100644 --- a/src/main/java/dev/openfeature/sdk/FlagEvaluationOptions.java +++ b/src/main/java/dev/openfeature/sdk/FlagEvaluationOptions.java @@ -1,5 +1,7 @@ package dev.openfeature.sdk; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -15,4 +17,18 @@ public class FlagEvaluationOptions { @Builder.Default Map hookHints = new HashMap<>(); + + List getHooks(FlagValueType flagValueType) { + if (hooks == null || hooks.isEmpty()) { + return Collections.emptyList(); + } + + var result = new ArrayList(hooks.size()); + for (var hook : hooks) { + if (hook.supportsFlagValueType(flagValueType)) { + result.add(hook); + } + } + return result; + } } diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index c7a7630da..11d8a2b0f 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -1,9 +1,11 @@ package dev.openfeature.sdk; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.concurrent.ConcurrentLinkedQueue; import lombok.extern.slf4j.Slf4j; /** @@ -17,26 +19,47 @@ class HookSupport { * 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 + * @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)); - } - } + public void setHooks( + HookSupportData hookSupportData, + List providerHooks, + List flagOptionsHooks, + ConcurrentLinkedQueue clientHooks, + ConcurrentLinkedQueue apiHooks + ) { + var lengthEstimate = providerHooks.size() + + flagOptionsHooks.size() + + clientHooks.size() + + apiHooks.size(); + List> hookContextPairs = new ArrayList<>(lengthEstimate); + + addAll(hookContextPairs, providerHooks); + addAll(hookContextPairs, flagOptionsHooks); + addAll(hookContextPairs, clientHooks); + addAll(hookContextPairs, apiHooks); + hookSupportData.hooks = hookContextPairs; } + private void addAll(List> accumulator, Collection toAdd) { + if(toAdd.isEmpty()){ + return; + } + + for (Hook hook : toAdd) { + accumulator.add(Pair.of(hook, null)); + } + } + /** * 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 + * @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++) { diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 6d0d8feb4..c14ded7e4 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -9,6 +9,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -24,14 +25,18 @@ public class OpenFeatureAPI implements EventBus { // package-private multi-read/single-write lock static AutoCloseableReentrantReadWriteLock lock = new AutoCloseableReentrantReadWriteLock(); - private final ConcurrentLinkedQueue apiHooks; + private final ConcurrentHashMap> apiHooks; private ProviderRepository providerRepository; private EventSupport eventSupport; private final AtomicReference evaluationContext = new AtomicReference<>(); private TransactionContextPropagator transactionContextPropagator; protected OpenFeatureAPI() { - apiHooks = new ConcurrentLinkedQueue<>(); + var values = FlagValueType.values(); + apiHooks = new ConcurrentHashMap<>(values.length); + for (FlagValueType value : values) { + apiHooks.put(value, new ConcurrentLinkedQueue<>()); + } providerRepository = new ProviderRepository(this); eventSupport = new EventSupport(); transactionContextPropagator = new NoOpTransactionContextPropagator(); @@ -304,7 +309,16 @@ public FeatureProvider getProvider(String domain) { * @param hooks The hook to add. */ public void addHooks(Hook... hooks) { - this.apiHooks.addAll(Arrays.asList(hooks)); + var types = FlagValueType.values(); + for (int i = 0; i < hooks.length; i++) { + var current = hooks[i]; + for (int j = 0; j < types.length; j++) { + var type = types[j]; + if(current.supportsFlagValueType(type)) { + this.apiHooks.get(type).add(current); + } + } + } } /** @@ -313,16 +327,20 @@ public void addHooks(Hook... hooks) { * @return A list of {@link Hook}s. */ public List getHooks() { - return new ArrayList<>(this.apiHooks); + var allHooks = new ArrayList(); + for (var queue : this.apiHooks.values()) { + allHooks.addAll(queue); + } + return allHooks; } /** - * Returns a reference to the collection of {@link Hook}s. + * Fetch the hooks associated to this client. * - * @return The collection of {@link Hook}s. + * @return A list of {@link Hook}s. */ - Collection getMutableHooks() { - return this.apiHooks; + ConcurrentLinkedQueue getHooks(FlagValueType type) { + return apiHooks.get(type); } /** diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index 614bc1e34..aa733d419 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -14,6 +14,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -30,11 +31,11 @@ */ @Slf4j @SuppressWarnings({ - "PMD.DataflowAnomalyAnalysis", - "PMD.BeanMembersShouldSerialize", - "PMD.UnusedLocalVariable", - "unchecked", - "rawtypes" + "PMD.DataflowAnomalyAnalysis", + "PMD.BeanMembersShouldSerialize", + "PMD.UnusedLocalVariable", + "unchecked", + "rawtypes" }) @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public class OpenFeatureClient implements Client { @@ -47,7 +48,7 @@ public class OpenFeatureClient implements Client { @Getter private final String version; - private final ConcurrentLinkedQueue clientHooks; + private final ConcurrentHashMap> clientHooks; private final AtomicReference evaluationContext = new AtomicReference<>(); private final HookSupport hookSupport; @@ -69,7 +70,11 @@ public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String domain, String ve this.domain = domain; this.version = version; this.hookSupport = new HookSupport(); - this.clientHooks = new ConcurrentLinkedQueue<>(); + var values = FlagValueType.values(); + this.clientHooks = new ConcurrentHashMap<>(values.length); + for (FlagValueType value : values) { + this.clientHooks.put(value, new ConcurrentLinkedQueue<>()); + } } /** @@ -125,7 +130,16 @@ public void track(String trackingEventName, EvaluationContext context, TrackingE */ @Override public OpenFeatureClient addHooks(Hook... hooks) { - this.clientHooks.addAll(Arrays.asList(hooks)); + var types = FlagValueType.values(); + for (int i = 0; i < hooks.length; i++) { + var current = hooks[i]; + for (int j = 0; j < types.length; j++) { + var type = types[j]; + if (current.supportsFlagValueType(type)) { + this.clientHooks.get(type).add(current); + } + } + } return this; } @@ -134,7 +148,11 @@ public OpenFeatureClient addHooks(Hook... hooks) { */ @Override public List getHooks() { - return new ArrayList<>(this.clientHooks); + var allHooks = new ArrayList(); + for (var queue : this.clientHooks.values()) { + allHooks.addAll(queue); + } + return allHooks; } /** @@ -174,9 +192,13 @@ private FlagEvaluationDetails evaluateFlag( final var state = stateManager.getState(); // Hooks are initialized as early as possible to enable the execution of error stages - var mergedHooks = ObjectUtils.merge( - provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks()); - hookSupport.setHooks(hookSupportData, mergedHooks, type); + hookSupport.setHooks( + hookSupportData, + provider.getProviderHooks(type), + flagOptions.getHooks(type), + clientHooks.get(type), + openfeatureApi.getHooks(type) + ); var sharedHookContext = new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue); diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index b1bb70ba1..d4a3bff97 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -8,10 +8,12 @@ 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 java.util.concurrent.ConcurrentLinkedQueue; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -35,7 +37,7 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { var sharedContext = getBaseHookContextForType(FlagValueType.STRING); var hookSupportData = new HookSupportData(); - hookSupport.setHooks(hookSupportData, Arrays.asList(hook1, hook2), FlagValueType.STRING); + hookSupport.setHooks(hookSupportData, List.of(hook1, hook2), Collections.emptyList(), new ConcurrentLinkedQueue<>(), new ConcurrentLinkedQueue<>()); hookSupport.setHookContexts(hookSupportData, sharedContext); hookSupport.updateEvaluationContext(hookSupportData, baseEvalContext); @@ -48,7 +50,7 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { assertThat(result.getValue("baseKey").asString()).isEqualTo("baseValue"); } - @ParameterizedTest + /* @ParameterizedTest @EnumSource(value = FlagValueType.class) @DisplayName("should always call generic hook") void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { @@ -104,7 +106,7 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { assertHookData(testHook1, 1, "before", "after", "finallyAfter", "error"); assertHookData(testHook2, 2, "before", "after", "finallyAfter", "error"); - } + }*/ private static void callAllHooks(HookSupportData hookSupportData) { hookSupport.executeBeforeHooks(hookSupportData); From b5a443f04e07b4b6efd37a894f5cf5cb61c65c20 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Thu, 20 Nov 2025 17:36:11 +0100 Subject: [PATCH 08/11] cache hooks Signed-off-by: christian.lutnik --- .../dev/openfeature/sdk/FeatureProvider.java | 6 + .../java/dev/openfeature/sdk/HookSupport.java | 42 +++-- .../dev/openfeature/sdk/HookSupportData.java | 4 +- .../dev/openfeature/sdk/OpenFeatureAPI.java | 12 +- .../openfeature/sdk/OpenFeatureClient.java | 19 +-- .../sdk/DeveloperExperienceTest.java | 24 ++- .../sdk/FlagEvaluationSpecTest.java | 7 +- .../dev/openfeature/sdk/HookSupportTest.java | 36 +++- .../sdk/OpenFeatureClientTest.java | 154 +++++++++++++++++- .../dev/openfeature/sdk/TypedTestHook.java | 43 +++++ .../sdk/benchmark/AllocationBenchmark.java | 21 +-- .../benchmark/AllocationBenchmarkState.java | 4 +- 12 files changed, 303 insertions(+), 69 deletions(-) create mode 100644 src/test/java/dev/openfeature/sdk/TypedTestHook.java diff --git a/src/main/java/dev/openfeature/sdk/FeatureProvider.java b/src/main/java/dev/openfeature/sdk/FeatureProvider.java index c241a11a2..694d5e14c 100644 --- a/src/main/java/dev/openfeature/sdk/FeatureProvider.java +++ b/src/main/java/dev/openfeature/sdk/FeatureProvider.java @@ -15,6 +15,12 @@ default List getProviderHooks() { return new ArrayList<>(); } + /** + * Returns all hooks that support the given flag value type. + * + * @param flagType the flag value type to support + * @return a list of hooks that support the given flag value type + */ default List getProviderHooks(FlagValueType flagType) { var allHooks = getProviderHooks(); var filteredHooks = new ArrayList(allHooks.size()); diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index 11d8a2b0f..c3b813add 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -2,7 +2,6 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.concurrent.ConcurrentLinkedQueue; @@ -19,22 +18,34 @@ class HookSupport { * 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 + * @param hookSupportData the data object to modify + * @param providerHooks the hooks filtered for the proper flag value type from the respective layer + * @param flagOptionsHooks the hooks filtered for the proper flag value type from the respective layer + * @param clientHooks the hooks filtered for the proper flag value type from the respective layer + * @param apiHooks the hooks filtered for the proper flag value type from the respective layer */ public void setHooks( HookSupportData hookSupportData, List providerHooks, List flagOptionsHooks, ConcurrentLinkedQueue clientHooks, - ConcurrentLinkedQueue apiHooks - ) { - var lengthEstimate = providerHooks.size() - + flagOptionsHooks.size() - + clientHooks.size() - + apiHooks.size(); - List> hookContextPairs = new ArrayList<>(lengthEstimate); + ConcurrentLinkedQueue apiHooks) { + var lengthEstimate = 0; + + if (providerHooks != null) { + lengthEstimate += providerHooks.size(); + } + if (flagOptionsHooks != null) { + lengthEstimate += flagOptionsHooks.size(); + } + if (clientHooks != null) { + lengthEstimate += clientHooks.size(); + } + if (apiHooks != null) { + lengthEstimate += apiHooks.size(); + } + + ArrayList> hookContextPairs = new ArrayList<>(lengthEstimate); addAll(hookContextPairs, providerHooks); addAll(hookContextPairs, flagOptionsHooks); @@ -45,7 +56,7 @@ public void setHooks( } private void addAll(List> accumulator, Collection toAdd) { - if(toAdd.isEmpty()){ + if (toAdd == null || toAdd.isEmpty()) { return; } @@ -89,10 +100,9 @@ public void updateEvaluationContext(HookSupportData hookSupportData, EvaluationC public void executeBeforeHooks(HookSupportData data) { // These traverse backwards from normal. - List> reversedHooks = new ArrayList<>(data.getHooks()); - Collections.reverse(reversedHooks); - - for (Pair hookContextPair : reversedHooks) { + var hooks = data.getHooks(); + for (int i = hooks.size() - 1; i >= 0; i--) { + var hookContextPair = hooks.get(i); var hook = hookContextPair.getKey(); var hookContext = hookContextPair.getValue(); diff --git a/src/main/java/dev/openfeature/sdk/HookSupportData.java b/src/main/java/dev/openfeature/sdk/HookSupportData.java index 2d3346ba1..78c494325 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupportData.java +++ b/src/main/java/dev/openfeature/sdk/HookSupportData.java @@ -1,6 +1,6 @@ package dev.openfeature.sdk; -import java.util.List; +import java.util.ArrayList; import java.util.Map; import lombok.Getter; @@ -10,7 +10,7 @@ @Getter class HookSupportData { - List> hooks; + ArrayList> hooks; EvaluationContext evaluationContext; Map hints; diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index c14ded7e4..a7e6a855d 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -4,8 +4,7 @@ import dev.openfeature.sdk.internal.AutoCloseableLock; import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -314,7 +313,7 @@ public void addHooks(Hook... hooks) { var current = hooks[i]; for (int j = 0; j < types.length; j++) { var type = types[j]; - if(current.supportsFlagValueType(type)) { + if (current.supportsFlagValueType(type)) { this.apiHooks.get(type).add(current); } } @@ -327,15 +326,16 @@ public void addHooks(Hook... hooks) { * @return A list of {@link Hook}s. */ public List getHooks() { - var allHooks = new ArrayList(); + // Hooks can be duplicated if they support multiple FlagValueTypes + var allHooks = new HashSet(); for (var queue : this.apiHooks.values()) { allHooks.addAll(queue); } - return allHooks; + return new ArrayList<>(allHooks); } /** - * Fetch the hooks associated to this client. + * Fetch the hooks associated to this client, that support the given FlagValueType. * * @return A list of {@link Hook}s. */ diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index aa733d419..ff209065d 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -8,9 +8,9 @@ import dev.openfeature.sdk.internal.ObjectUtils; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -31,11 +31,11 @@ */ @Slf4j @SuppressWarnings({ - "PMD.DataflowAnomalyAnalysis", - "PMD.BeanMembersShouldSerialize", - "PMD.UnusedLocalVariable", - "unchecked", - "rawtypes" + "PMD.DataflowAnomalyAnalysis", + "PMD.BeanMembersShouldSerialize", + "PMD.UnusedLocalVariable", + "unchecked", + "rawtypes" }) @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public class OpenFeatureClient implements Client { @@ -148,11 +148,11 @@ public OpenFeatureClient addHooks(Hook... hooks) { */ @Override public List getHooks() { - var allHooks = new ArrayList(); + var allHooks = new HashSet(); for (var queue : this.clientHooks.values()) { allHooks.addAll(queue); } - return allHooks; + return new ArrayList<>(allHooks); } /** @@ -197,8 +197,7 @@ private FlagEvaluationDetails evaluateFlag( provider.getProviderHooks(type), flagOptions.getHooks(type), clientHooks.get(type), - openfeatureApi.getHooks(type) - ); + openfeatureApi.getHooks(type)); var sharedHookContext = new SharedHookContext(key, type, this.getMetadata(), provider.getMetadata(), defaultValue); diff --git a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java index 19108bde5..03e851566 100644 --- a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java +++ b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -14,6 +15,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import lombok.SneakyThrows; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -101,9 +103,10 @@ void brokenProvider() { void providerLockedPerTransaction() { final String defaultValue = "string-value"; - final OpenFeatureAPI api = new OpenFeatureAPI(); - var provider1 = TestProvider.builder().initsToReady(); - var provider2 = TestProvider.builder().initsToReady(); + final OpenFeatureAPI testApi = new OpenFeatureAPI(); + final var provider1 = TestProvider.builder().initsToReady(); + final var provider2 = TestProvider.builder().initsToReady(); + final var wasHookCalled = new AtomicBoolean(false); class MutatingHook implements Hook { @@ -112,24 +115,27 @@ class MutatingHook implements Hook { // change the provider during a before hook - this should not impact the evaluation in progress public Optional before(HookContext ctx, Map hints) { - api.setProviderAndWait(provider2); - + testApi.setProviderAndWait(provider2); + wasHookCalled.set(true); return Optional.empty(); } } - final Client client = api.getClient(); - api.setProviderAndWait(provider1); - api.addHooks(new MutatingHook()); + final Client client = testApi.getClient(); + testApi.setProviderAndWait(provider1); + testApi.addHooks(new MutatingHook()); // if provider is changed during an evaluation transaction it should proceed with the original provider client.getStringValue("val", defaultValue); assertEquals(1, provider1.getFlagEvaluations().size()); + assertEquals(0, provider2.getFlagEvaluations().size()); + assertTrue(wasHookCalled.get()); - api.clearHooks(); + testApi.clearHooks(); // subsequent evaluations should now use new provider set by hook client.getStringValue("val", defaultValue); + assertEquals(1, provider1.getFlagEvaluations().size()); assertEquals(1, provider2.getFlagEvaluations().size()); } diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index 82aa4e3cc..67dded945 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -14,6 +14,7 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import dev.openfeature.sdk.e2e.Flag; import dev.openfeature.sdk.exceptions.GeneralError; @@ -143,6 +144,8 @@ void provider_metadata() { void hook_addition() { Hook h1 = mock(Hook.class); Hook h2 = mock(Hook.class); + when(h1.supportsFlagValueType(any())).thenReturn(true); + when(h2.supportsFlagValueType(any())).thenReturn(true); api.addHooks(h1); assertEquals(1, api.getHooks().size()); @@ -150,7 +153,7 @@ void hook_addition() { api.addHooks(h2); assertEquals(2, api.getHooks().size()); - assertEquals(h2, api.getHooks().get(1)); + assertTrue(api.getHooks().contains(h2)); } @Specification( @@ -175,6 +178,8 @@ void hookRegistration() { Client c = _client(); Hook m1 = mock(Hook.class); Hook m2 = mock(Hook.class); + when(m1.supportsFlagValueType(any())).thenReturn(true); + when(m2.supportsFlagValueType(any())).thenReturn(true); c.addHooks(m1); c.addHooks(m2); List hooks = c.getHooks(); diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index d4a3bff97..506c400e7 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -7,7 +7,6 @@ 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; @@ -37,7 +36,12 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { var sharedContext = getBaseHookContextForType(FlagValueType.STRING); var hookSupportData = new HookSupportData(); - hookSupport.setHooks(hookSupportData, List.of(hook1, hook2), Collections.emptyList(), new ConcurrentLinkedQueue<>(), new ConcurrentLinkedQueue<>()); + hookSupport.setHooks( + hookSupportData, + List.of(hook1, hook2), + Collections.emptyList(), + new ConcurrentLinkedQueue<>(), + new ConcurrentLinkedQueue<>()); hookSupport.setHookContexts(hookSupportData, sharedContext); hookSupport.updateEvaluationContext(hookSupportData, baseEvalContext); @@ -50,14 +54,18 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { assertThat(result.getValue("baseKey").asString()).isEqualTo("baseValue"); } - /* @ParameterizedTest - @EnumSource(value = FlagValueType.class) + @Test @DisplayName("should always call generic hook") - void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { + void shouldAlwaysCallGenericHook() { Hook genericHook = mockGenericHook(); var hookSupportData = new HookSupportData(); - hookSupport.setHooks(hookSupportData, List.of(genericHook), flagValueType); + hookSupport.setHooks( + hookSupportData, + List.of(genericHook), + Collections.emptyList(), + new ConcurrentLinkedQueue<>(), + new ConcurrentLinkedQueue<>()); callAllHooks(hookSupportData); @@ -73,7 +81,12 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { void shouldPassDataAcrossStages(FlagValueType flagValueType) { var testHook = new TestHookWithData(); var hookSupportData = new HookSupportData(); - hookSupport.setHooks(hookSupportData, List.of(testHook), flagValueType); + hookSupport.setHooks( + hookSupportData, + List.of(testHook), + Collections.emptyList(), + new ConcurrentLinkedQueue<>(), + new ConcurrentLinkedQueue<>()); hookSupport.setHookContexts(hookSupportData, getBaseHookContextForType(flagValueType)); hookSupport.executeBeforeHooks(hookSupportData); @@ -99,14 +112,19 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) { var testHook2 = new TestHookWithData(2); var hookSupportData = new HookSupportData(); - hookSupport.setHooks(hookSupportData, List.of(testHook1, testHook2), flagValueType); + hookSupport.setHooks( + hookSupportData, + List.of(testHook1, testHook2), + Collections.emptyList(), + new ConcurrentLinkedQueue<>(), + new ConcurrentLinkedQueue<>()); 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); diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java index 91509bd45..c379ff3fa 100644 --- a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java +++ b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -12,11 +13,13 @@ import dev.openfeature.sdk.fixtures.HookFixtures; import dev.openfeature.sdk.testutils.testProvider.TestProvider; import java.util.HashMap; +import java.util.List; import org.junit.jupiter.api.AfterEach; 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.EnumSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mockito; import org.simplify4u.slf4jmock.LoggerMock; @@ -128,11 +131,158 @@ void shouldSupportUsageOfHookData(boolean isError) { 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("after")).isNull(); assertThat(testHook.hookData.get("error")).isEqualTo("test-data"); } else { assertThat(testHook.hookData.get("after")).isEqualTo("test-data"); - assertThat(testHook.hookData.get("error")).isEqualTo(null); + assertThat(testHook.hookData.get("error")).isNull(); + } + } + + @ParameterizedTest + @EnumSource(FlagValueType.class) + @DisplayName("Should call hooks that support the flag value type") + void shouldExecuteAppropriateHooks(FlagValueType flagValueType) { + var allTypes = FlagValueType.values(); + var apiHooks = new TypedTestHook[allTypes.length]; + var clientHooks = new TypedTestHook[allTypes.length]; + var providerHooks = new TypedTestHook[allTypes.length]; + var evaluationHooks = new TypedTestHook[allTypes.length]; + for (int i = 0; i < allTypes.length; i++) { + apiHooks[i] = new TypedTestHook(allTypes[i]); + clientHooks[i] = new TypedTestHook(allTypes[i]); + providerHooks[i] = new TypedTestHook(allTypes[i]); + evaluationHooks[i] = new TypedTestHook(allTypes[i]); + } + var allHooks = new TypedTestHook[][] {apiHooks, clientHooks, providerHooks, evaluationHooks}; + + OpenFeatureAPI api = new OpenFeatureAPI(); + var provider = TestProvider.builder() + .withHooks(providerHooks) + .allowUnknownFlags() + .initsToReady(); + api.setProviderAndWait(provider); + + Client client = api.getClient(); + + api.addHooks(apiHooks); + client.addHooks(clientHooks); + + var options = + FlagEvaluationOptions.builder().hooks(List.of(evaluationHooks)).build(); + + if (flagValueType == FlagValueType.BOOLEAN) { + client.getBooleanDetails("key", true, ImmutableContext.EMPTY, options); + } else if (flagValueType == FlagValueType.STRING) { + client.getStringDetails("key", "default", ImmutableContext.EMPTY, options); + } else if (flagValueType == FlagValueType.INTEGER) { + client.getIntegerDetails("key", 42, ImmutableContext.EMPTY, options); + } else if (flagValueType == FlagValueType.DOUBLE) { + client.getDoubleValue("key", 3.14, ImmutableContext.EMPTY, options); + } else if (flagValueType == FlagValueType.OBJECT) { + client.getObjectDetails("key", new Value(1), ImmutableContext.EMPTY, options); + } + + for (TypedTestHook[] level : allHooks) { + for (TypedTestHook hook : level) { + assertEquals( + flagValueType == hook.flagValueType, + hook.beforeCalled.get(), + () -> hook.flagValueType + + " hook called? " + + hook.beforeCalled.get() + + ", should have been called? " + + (flagValueType == hook.flagValueType)); + assertEquals( + flagValueType == hook.flagValueType, + hook.afterCalled.get(), + () -> hook.flagValueType + + " hook called? " + + hook.afterCalled.get() + + ", should have been called? " + + (flagValueType == hook.flagValueType)); + assertEquals( + flagValueType == hook.flagValueType, + hook.finallyAfterCalled.get(), + () -> hook.flagValueType + + " hook called? " + + hook.finallyAfterCalled.get() + + ", should have been called? " + + (flagValueType == hook.flagValueType)); + assertFalse(hook.errorCalled.get()); + } + } + } + + @ParameterizedTest + @EnumSource(FlagValueType.class) + @DisplayName("Should call hooks that support the flag value type in error scenarios") + void shouldExecuteAppropriateErrorHooks(FlagValueType flagValueType) { + var allTypes = FlagValueType.values(); + var apiHooks = new TypedTestHook[allTypes.length]; + var clientHooks = new TypedTestHook[allTypes.length]; + var providerHooks = new TypedTestHook[allTypes.length]; + var evaluationHooks = new TypedTestHook[allTypes.length]; + for (int i = 0; i < allTypes.length; i++) { + apiHooks[i] = new TypedTestHook(allTypes[i]); + clientHooks[i] = new TypedTestHook(allTypes[i]); + providerHooks[i] = new TypedTestHook(allTypes[i]); + evaluationHooks[i] = new TypedTestHook(allTypes[i]); + } + var allHooks = new TypedTestHook[][] {apiHooks, clientHooks, providerHooks, evaluationHooks}; + + OpenFeatureAPI api = new OpenFeatureAPI(); + var provider = TestProvider.builder().withHooks(providerHooks).initsToReady(); + api.setProviderAndWait(provider); + + Client client = api.getClient(); + + api.addHooks(apiHooks); + client.addHooks(clientHooks); + + var options = + FlagEvaluationOptions.builder().hooks(List.of(evaluationHooks)).build(); + + if (flagValueType == FlagValueType.BOOLEAN) { + client.getBooleanDetails("key", true, ImmutableContext.EMPTY, options); + } else if (flagValueType == FlagValueType.STRING) { + client.getStringDetails("key", "default", ImmutableContext.EMPTY, options); + } else if (flagValueType == FlagValueType.INTEGER) { + client.getIntegerDetails("key", 42, ImmutableContext.EMPTY, options); + } else if (flagValueType == FlagValueType.DOUBLE) { + client.getDoubleValue("key", 3.14, ImmutableContext.EMPTY, options); + } else if (flagValueType == FlagValueType.OBJECT) { + client.getObjectDetails("key", new Value(1), ImmutableContext.EMPTY, options); + } + + for (TypedTestHook[] level : allHooks) { + for (TypedTestHook hook : level) { + assertEquals( + flagValueType == hook.flagValueType, + hook.beforeCalled.get(), + () -> hook.flagValueType + + " hook called? " + + hook.beforeCalled.get() + + ", should have been called? " + + (flagValueType == hook.flagValueType)); + assertEquals( + flagValueType == hook.flagValueType, + hook.errorCalled.get(), + () -> hook.flagValueType + + " hook called? " + + hook.errorCalled.get() + + ", should have been called? " + + (flagValueType == hook.flagValueType)); + assertEquals( + flagValueType == hook.flagValueType, + hook.finallyAfterCalled.get(), + () -> hook.flagValueType + + " hook called? " + + hook.finallyAfterCalled.get() + + ", should have been called? " + + (flagValueType == hook.flagValueType)); + assertFalse(hook.afterCalled.get()); + } } } } diff --git a/src/test/java/dev/openfeature/sdk/TypedTestHook.java b/src/test/java/dev/openfeature/sdk/TypedTestHook.java new file mode 100644 index 000000000..96ffed637 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/TypedTestHook.java @@ -0,0 +1,43 @@ +package dev.openfeature.sdk; + +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; + +public class TypedTestHook implements Hook { + public final FlagValueType flagValueType; + public final AtomicBoolean beforeCalled = new AtomicBoolean(false); + public final AtomicBoolean afterCalled = new AtomicBoolean(false); + public final AtomicBoolean errorCalled = new AtomicBoolean(false); + public final AtomicBoolean finallyAfterCalled = new AtomicBoolean(false); + + public TypedTestHook(FlagValueType flagValueType) { + this.flagValueType = flagValueType; + } + + @Override + public boolean supportsFlagValueType(FlagValueType flagValueType) { + return this.flagValueType == flagValueType; + } + + @Override + public Optional before(HookContext ctx, Map hints) { + beforeCalled.set(true); + return Optional.empty(); + } + + @Override + public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + afterCalled.set(true); + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + errorCalled.set(true); + } + + @Override + public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { + finallyAfterCalled.set(true); + } +} diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index 437292fa8..1b9a9f535 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -22,11 +22,8 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; -import org.junit.jupiter.api.Test; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.Fork; -import org.openjdk.jmh.annotations.Scope; -import org.openjdk.jmh.annotations.State; import org.openjdk.jmh.infra.Blackhole; /** @@ -99,7 +96,7 @@ public Optional before(HookContext ctx, Map Date: Mon, 24 Nov 2025 09:51:08 +0100 Subject: [PATCH 09/11] extract common code Signed-off-by: christian.lutnik --- .../java/dev/openfeature/sdk/HookSupport.java | 24 +++++++++++++++ .../dev/openfeature/sdk/OpenFeatureAPI.java | 20 ++----------- .../openfeature/sdk/OpenFeatureClient.java | 29 +++++-------------- 3 files changed, 33 insertions(+), 40 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index c3b813add..7052664cb 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -2,7 +2,9 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentLinkedQueue; import lombok.extern.slf4j.Slf4j; @@ -156,4 +158,26 @@ public void executeAfterAllHooks(HookSupportData data, FlagEvaluationDetails } } } + + static void addHooks(Map> hookMap, Hook... hooksToAdd) { + var types = FlagValueType.values(); + for (int i = 0; i < hooksToAdd.length; i++) { + var current = hooksToAdd[i]; + for (int j = 0; j < types.length; j++) { + var type = types[j]; + if (current.supportsFlagValueType(type)) { + hookMap.get(type).add(current); + } + } + } + } + + static ArrayList getAllUniqueHooks(Map> hookMap) { + // Hooks can be duplicated if they support multiple FlagValueTypes + var allHooks = new HashSet(); + for (var queue : hookMap.values()) { + allHooks.addAll(queue); + } + return new ArrayList<>(allHooks); + } } diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index a7e6a855d..14ec034be 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -3,8 +3,6 @@ import dev.openfeature.sdk.exceptions.OpenFeatureError; import dev.openfeature.sdk.internal.AutoCloseableLock; import dev.openfeature.sdk.internal.AutoCloseableReentrantReadWriteLock; -import java.util.ArrayList; -import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -308,16 +306,7 @@ public FeatureProvider getProvider(String domain) { * @param hooks The hook to add. */ public void addHooks(Hook... hooks) { - var types = FlagValueType.values(); - for (int i = 0; i < hooks.length; i++) { - var current = hooks[i]; - for (int j = 0; j < types.length; j++) { - var type = types[j]; - if (current.supportsFlagValueType(type)) { - this.apiHooks.get(type).add(current); - } - } - } + HookSupport.addHooks(apiHooks, hooks); } /** @@ -326,12 +315,7 @@ public void addHooks(Hook... hooks) { * @return A list of {@link Hook}s. */ public List getHooks() { - // Hooks can be duplicated if they support multiple FlagValueTypes - var allHooks = new HashSet(); - for (var queue : this.apiHooks.values()) { - allHooks.addAll(queue); - } - return new ArrayList<>(allHooks); + return HookSupport.getAllUniqueHooks(apiHooks); } /** diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index ff209065d..b621ac47b 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -7,10 +7,8 @@ import dev.openfeature.sdk.exceptions.ProviderNotReadyError; import dev.openfeature.sdk.internal.ObjectUtils; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -31,11 +29,11 @@ */ @Slf4j @SuppressWarnings({ - "PMD.DataflowAnomalyAnalysis", - "PMD.BeanMembersShouldSerialize", - "PMD.UnusedLocalVariable", - "unchecked", - "rawtypes" + "PMD.DataflowAnomalyAnalysis", + "PMD.BeanMembersShouldSerialize", + "PMD.UnusedLocalVariable", + "unchecked", + "rawtypes" }) @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public class OpenFeatureClient implements Client { @@ -130,16 +128,7 @@ public void track(String trackingEventName, EvaluationContext context, TrackingE */ @Override public OpenFeatureClient addHooks(Hook... hooks) { - var types = FlagValueType.values(); - for (int i = 0; i < hooks.length; i++) { - var current = hooks[i]; - for (int j = 0; j < types.length; j++) { - var type = types[j]; - if (current.supportsFlagValueType(type)) { - this.clientHooks.get(type).add(current); - } - } - } + HookSupport.addHooks(clientHooks, hooks); return this; } @@ -148,11 +137,7 @@ public OpenFeatureClient addHooks(Hook... hooks) { */ @Override public List getHooks() { - var allHooks = new HashSet(); - for (var queue : this.clientHooks.values()) { - allHooks.addAll(queue); - } - return new ArrayList<>(allHooks); + return HookSupport.getAllUniqueHooks(clientHooks); } /** From 8f9da44e201a3910ec4540b76b131a77bff27b16 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Mon, 24 Nov 2025 09:51:44 +0100 Subject: [PATCH 10/11] spotless Signed-off-by: christian.lutnik --- .../java/dev/openfeature/sdk/OpenFeatureClient.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java index b621ac47b..fbb608c64 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java @@ -29,11 +29,11 @@ */ @Slf4j @SuppressWarnings({ - "PMD.DataflowAnomalyAnalysis", - "PMD.BeanMembersShouldSerialize", - "PMD.UnusedLocalVariable", - "unchecked", - "rawtypes" + "PMD.DataflowAnomalyAnalysis", + "PMD.BeanMembersShouldSerialize", + "PMD.UnusedLocalVariable", + "unchecked", + "rawtypes" }) @Deprecated() // TODO: eventually we will make this non-public. See issue #872 public class OpenFeatureClient implements Client { From 59530f5470f87f8facf47989f27f266fd7d24221 Mon Sep 17 00:00:00 2001 From: "christian.lutnik" Date: Tue, 25 Nov 2025 17:14:14 +0100 Subject: [PATCH 11/11] revert comments Signed-off-by: christian.lutnik --- .../openfeature/sdk/benchmark/AllocationBenchmark.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java index 1b9a9f535..c41492577 100644 --- a/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java +++ b/src/test/java/dev/openfeature/sdk/benchmark/AllocationBenchmark.java @@ -23,7 +23,9 @@ import java.util.Map; import java.util.Optional; import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Mode; import org.openjdk.jmh.infra.Blackhole; /** @@ -35,9 +37,9 @@ public class AllocationBenchmark { // 10K iterations works well with Xmx1024m (we don't want to run out of memory) private static final int ITERATIONS = 10000; - // @Benchmark - // @BenchmarkMode(Mode.SingleShotTime) - // @Fork(jvmArgsAppend = {"-Xmx1024m", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseEpsilonGC"}) + @Benchmark + @BenchmarkMode(Mode.SingleShotTime) + @Fork(jvmArgsAppend = {"-Xmx1024m", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseEpsilonGC"}) public void run() { OpenFeatureAPI.getInstance().setProviderAndWait(new NoOpProvider()); @@ -96,7 +98,7 @@ public Optional before(HookContext ctx, Map