From 4efb313eff2f238d0bb3f1d941b6ab132d20d559 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 25 Sep 2026 14:59:19 -0400 Subject: [PATCH 1/3] fixup: use SDK hook impl Signed-off-by: Todd Baert --- .../sdk/MultiProviderHookExecutor.java | 95 ++++ .../multiprovider/HookExecutionContext.java | 15 + .../sdk/multiprovider/MultiProvider.java | 137 ++++- .../sdk/MultiProviderHookExecutorTest.java | 472 ++++++++++++++++++ .../multiprovider/MultiProviderHooksTest.java | 270 ++++++++++ 5 files changed, 980 insertions(+), 9 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/MultiProviderHookExecutor.java create mode 100644 src/main/java/dev/openfeature/sdk/multiprovider/HookExecutionContext.java create mode 100644 src/test/java/dev/openfeature/sdk/MultiProviderHookExecutorTest.java create mode 100644 src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java diff --git a/src/main/java/dev/openfeature/sdk/MultiProviderHookExecutor.java b/src/main/java/dev/openfeature/sdk/MultiProviderHookExecutor.java new file mode 100644 index 000000000..bf5b4da2f --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/MultiProviderHookExecutor.java @@ -0,0 +1,95 @@ +package dev.openfeature.sdk; + +import dev.openfeature.sdk.exceptions.ExceptionUtils; +import dev.openfeature.sdk.exceptions.OpenFeatureError; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.function.BiFunction; + +/** + * Runs a single provider's own hooks around one evaluation, delegating to {@link HookSupport} so a + * child provider's hooks behave the same as when the provider is used standalone. + * + *

Internal. Public only so {@link dev.openfeature.sdk.multiprovider.MultiProvider} can + * reach it across packages; {@link HookSupport} and its collaborators are package-private. Not part + * of the public API; may change or be removed without notice. + * + * @hidden + */ +public final class MultiProviderHookExecutor { + + private final HookSupport hookSupport = new HookSupport(); + + /** + * Runs {@code provider}'s hooks around the evaluation performed by {@code providerFunction}. + */ + public ProviderEvaluation execute( + FeatureProvider provider, + String key, + T defaultValue, + FlagValueType type, + EvaluationContext context, + ClientMetadata clientMetadata, + Map hints, + BiFunction> providerFunction) { + + List providerHooks = provider.getProviderHooks(); + if (providerHooks == null || providerHooks.isEmpty()) { + return providerFunction.apply(provider, context); + } + + HookSupportData data = new HookSupportData(); + data.hints = hints == null ? Collections.emptyMap() : hints; + LayeredEvaluationContext layeredContext = new LayeredEvaluationContext(null, null, null, context); + data.evaluationContext = layeredContext; + + hookSupport.setHooks( + data, providerHooks, Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), type); + if (data.getHooks().isEmpty()) { + // no hooks support this type + return providerFunction.apply(provider, context); + } + + SharedHookContext sharedHookContext = + new SharedHookContext<>(key, type, clientMetadata, provider.getMetadata(), defaultValue); + hookSupport.setHookContexts(data, sharedHookContext, layeredContext); + + FlagEvaluationDetails details = null; + try { + hookSupport.executeBeforeHooks(data); + ProviderEvaluation providerEvaluation = providerFunction.apply(provider, data.getEvaluationContext()); + details = FlagEvaluationDetails.from(providerEvaluation, key); + if (details.getErrorCode() != null) { + Exception error = + ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage()); + enrichDetailsWithErrorDefaults(defaultValue, details); + hookSupport.executeErrorHooks(data, error); + } else { + hookSupport.executeAfterHooks(data, details); + } + return providerEvaluation; + } catch (Exception e) { + if (details == null) { + details = FlagEvaluationDetails.builder().flagKey(key).build(); + } + if (e instanceof OpenFeatureError) { + details.setErrorCode(((OpenFeatureError) e).getErrorCode()); + } else { + details.setErrorCode(ErrorCode.GENERAL); + } + details.setErrorMessage(e.getMessage()); + enrichDetailsWithErrorDefaults(defaultValue, details); + hookSupport.executeErrorHooks(data, e); + throw e; + } finally { + // details is always set by now: from the evaluation on success, or the catch on failure + hookSupport.executeAfterAllHooks(data, details); + } + } + + private static void enrichDetailsWithErrorDefaults(T defaultValue, FlagEvaluationDetails details) { + details.setValue(defaultValue); + details.setReason(Reason.ERROR.toString()); + } +} diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/HookExecutionContext.java b/src/main/java/dev/openfeature/sdk/multiprovider/HookExecutionContext.java new file mode 100644 index 000000000..5b0d501d9 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/multiprovider/HookExecutionContext.java @@ -0,0 +1,15 @@ +package dev.openfeature.sdk.multiprovider; + +import dev.openfeature.sdk.ClientMetadata; +import java.util.Map; + +/** Captures hook lifecycle context (client metadata and hints) for per-provider hook execution. */ +final class HookExecutionContext { + final ClientMetadata clientMetadata; + final Map hints; + + HookExecutionContext(ClientMetadata clientMetadata, Map hints) { + this.clientMetadata = clientMetadata; + this.hints = hints; + } +} diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java index 00611ed78..e190ba885 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java @@ -1,9 +1,15 @@ package dev.openfeature.sdk.multiprovider; +import dev.openfeature.sdk.ClientMetadata; import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.EventProvider; import dev.openfeature.sdk.FeatureProvider; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.FlagValueType; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; import dev.openfeature.sdk.Metadata; +import dev.openfeature.sdk.MultiProviderHookExecutor; import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.Value; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -15,10 +21,12 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.function.BiFunction; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -28,6 +36,9 @@ *

This provider delegates flag evaluations to multiple underlying providers using a configurable * {@link Strategy}. It also exposes combined metadata containing the original metadata of each * underlying provider. + * + *

Hooks registered on the child providers are executed around each child evaluation, so a child + * provider's own hooks observe the evaluation it takes part in. */ @Slf4j public class MultiProvider extends EventProvider { @@ -40,6 +51,12 @@ public class MultiProvider extends EventProvider { private final Map providers; private final Strategy strategy; + + // side-channel for hook-context not passed to resolvers; assumes before/resolve share a thread + private final ThreadLocal localHookExecutionContext = new ThreadLocal<>(); + private final ClientMetadata hookClientMetadata = MultiProvider::getNAME; + private final MultiProviderHookExecutor hookExecutor = new MultiProviderHookExecutor(); + private MultiProviderMetadata metadata; /** @@ -63,6 +80,37 @@ public MultiProvider(List providers, Strategy strategy) { this.strategy = Objects.requireNonNull(strategy, "strategy must not be null"); } + @SuppressWarnings({"rawtypes", "unchecked"}) + private final List providerHooks = List.of(new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + localHookExecutionContext.set(new HookExecutionContext(ctx.getClientMetadata(), snapshotHints(hints))); + return Optional.empty(); + } + + @Override + public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { + localHookExecutionContext.remove(); + } + }); + + /** + * Provider-level hooks for this MultiProvider. Includes a {@code before} hook that captures the + * {@link ClientMetadata} and hints from the SDK lifecycle for use during per-child hook execution. + */ + @Override + public List getProviderHooks() { + return providerHooks; + } + + // defensive copy: mutable hookHints may be read by parallel strategies; plain copy allows nulls + private static Map snapshotHints(Map hints) { + if (hints == null || hints.isEmpty()) { + return Collections.emptyMap(); + } + return Collections.unmodifiableMap(new HashMap<>(hints)); + } + protected static Map buildProviders(List providers) { Map providersMap = new LinkedHashMap<>(providers.size()); for (FeatureProvider provider : providers) { @@ -119,13 +167,11 @@ public void initialize(EvaluationContext evaluationContext, String domain) throw List> results = executorService.invokeAll(tasks); for (Future result : results) { - // This will re-throw any exception from the provider's initialize method, - // wrapped in an ExecutionException. + // re-throws any provider initialize exception, wrapped in an ExecutionException result.get(); } } catch (Exception e) { - // If initialization fails for any provider, attempt to shut down via the - // standard shutdown path to avoid a partial/limbo state. + // on any provider init failure, shut down via the standard path to avoid a limbo state try { shutdown(); } catch (Exception shutdownEx) { @@ -148,28 +194,101 @@ public Metadata getMetadata() { @Override public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { return strategy.evaluate( - providers, key, defaultValue, ctx, p -> p.getBooleanEvaluation(key, defaultValue, ctx)); + providers, + key, + defaultValue, + ctx, + provider -> evaluateChild( + provider, + key, + defaultValue, + ctx, + FlagValueType.BOOLEAN, + (p, evaluationContext) -> p.getBooleanEvaluation(key, defaultValue, evaluationContext))); } @Override public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { - return strategy.evaluate(providers, key, defaultValue, ctx, p -> p.getStringEvaluation(key, defaultValue, ctx)); + return strategy.evaluate( + providers, + key, + defaultValue, + ctx, + provider -> evaluateChild( + provider, + key, + defaultValue, + ctx, + FlagValueType.STRING, + (p, evaluationContext) -> p.getStringEvaluation(key, defaultValue, evaluationContext))); } @Override public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { return strategy.evaluate( - providers, key, defaultValue, ctx, p -> p.getIntegerEvaluation(key, defaultValue, ctx)); + providers, + key, + defaultValue, + ctx, + provider -> evaluateChild( + provider, + key, + defaultValue, + ctx, + FlagValueType.INTEGER, + (p, evaluationContext) -> p.getIntegerEvaluation(key, defaultValue, evaluationContext))); } @Override public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { - return strategy.evaluate(providers, key, defaultValue, ctx, p -> p.getDoubleEvaluation(key, defaultValue, ctx)); + return strategy.evaluate( + providers, + key, + defaultValue, + ctx, + provider -> evaluateChild( + provider, + key, + defaultValue, + ctx, + FlagValueType.DOUBLE, + (p, evaluationContext) -> p.getDoubleEvaluation(key, defaultValue, evaluationContext))); } @Override public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { - return strategy.evaluate(providers, key, defaultValue, ctx, p -> p.getObjectEvaluation(key, defaultValue, ctx)); + return strategy.evaluate( + providers, + key, + defaultValue, + ctx, + provider -> evaluateChild( + provider, + key, + defaultValue, + ctx, + FlagValueType.OBJECT, + (p, evaluationContext) -> p.getObjectEvaluation(key, defaultValue, evaluationContext))); + } + + // runs the child's own hooks around its evaluation, using captured client metadata and hints + // (or this provider's metadata and no hints when evaluated outside a client lifecycle) + private ProviderEvaluation evaluateChild( + FeatureProvider provider, + String key, + T defaultValue, + EvaluationContext ctx, + FlagValueType type, + BiFunction> providerFunction) { + HookExecutionContext hookExecutionContext = currentHookExecutionContext(); + ClientMetadata clientMetadata = + hookExecutionContext != null ? hookExecutionContext.clientMetadata : hookClientMetadata; + Map hints = hookExecutionContext != null ? hookExecutionContext.hints : Collections.emptyMap(); + return hookExecutor.execute(provider, key, defaultValue, type, ctx, clientMetadata, hints, providerFunction); + } + + private HookExecutionContext currentHookExecutionContext() { + return localHookExecutionContext.get(); } @Override diff --git a/src/test/java/dev/openfeature/sdk/MultiProviderHookExecutorTest.java b/src/test/java/dev/openfeature/sdk/MultiProviderHookExecutorTest.java new file mode 100644 index 000000000..fcbc2fc3f --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/MultiProviderHookExecutorTest.java @@ -0,0 +1,472 @@ +package dev.openfeature.sdk; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +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.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import dev.openfeature.sdk.exceptions.TypeMismatchError; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.Test; + +class MultiProviderHookExecutorTest { + + private final MultiProviderHookExecutor executor = new MultiProviderHookExecutor(); + private final ClientMetadata clientMetadata = () -> "test"; + + @Test + void shortCircuitsDirectlyWhenProviderHasNoHooks() { + AtomicBoolean called = new AtomicBoolean(false); + ProviderEvaluation result = executor.execute( + stubProvider("p", Collections.emptyList()), + "flag", + "default", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> { + called.set(true); + return ProviderEvaluation.builder().value("direct").build(); + }); + + assertTrue(called.get()); + assertEquals("direct", result.getValue()); + } + + @Test + void shortCircuitsWhenProviderReturnsNullHookList() { + AtomicBoolean called = new AtomicBoolean(false); + ProviderEvaluation result = executor.execute( + stubProvider("p", null), + "flag", + "default", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> { + called.set(true); + return ProviderEvaluation.builder().value("direct").build(); + }); + + assertTrue(called.get()); + assertEquals("direct", result.getValue()); + } + + @Test + void runsHooksInSameOrderAsStandaloneEvaluation() { + // SDK order (see HookSupport): before reversed, after/finally in registration order + List calls = new ArrayList<>(); + Hook first = orderRecordingHook(calls, "first"); + Hook second = orderRecordingHook(calls, "second"); + + executor.execute( + stubProvider("p", List.of(first, second)), + "flag", + "default", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> ProviderEvaluation.builder().value("ok").build()); + + assertEquals( + List.of( + "before:second", + "before:first", + "after:first", + "after:second", + "finally:first", + "finally:second"), + calls); + } + + @Test + void shortCircuitsWhenNoHooksSupportTheFlagType() { + AtomicBoolean called = new AtomicBoolean(false); + Hook boolOnlyHook = new Hook() { + @Override + public boolean supportsFlagValueType(FlagValueType type) { + return type == FlagValueType.BOOLEAN; + } + }; + ProviderEvaluation result = executor.execute( + stubProvider("p", List.of(boolOnlyHook)), + "flag", + "default", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> { + called.set(true); + return ProviderEvaluation.builder().value("direct").build(); + }); + + assertTrue(called.get()); + assertEquals("direct", result.getValue()); + } + + @Test + @SuppressWarnings("rawtypes") + void toleratesNullReturnedFromBeforeHook() { + Hook nullBeforeHook = new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return null; + } + }; + ProviderEvaluation result = executor.execute( + stubProvider("p", List.of(nullBeforeHook)), + "flag", + "default", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> ProviderEvaluation.builder().value("ok").build()); + + assertEquals("ok", result.getValue()); + } + + @Test + void swallowsExceptionThrownFromErrorHook() { + AtomicBoolean errorHookCalled = new AtomicBoolean(false); + Hook throwingErrorHook = new Hook() { + @Override + public void error(HookContext ctx, Exception error, Map hints) { + errorHookCalled.set(true); + throw new RuntimeException("error hook exploded"); + } + }; + RuntimeException providerEx = new RuntimeException("provider failed"); + + RuntimeException thrown = assertThrows( + RuntimeException.class, + () -> executor.execute( + stubProvider("p", List.of(throwingErrorHook)), + "flag", + "default", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> { + throw providerEx; + })); + + assertTrue(errorHookCalled.get(), "error() hook should have been called"); + assertEquals(providerEx, thrown, "original provider exception must propagate"); + } + + @Test + void swallowsExceptionThrownFromFinallyAfterHook() { + Hook throwingFinallyHook = new Hook() { + @Override + public void finallyAfter( + HookContext ctx, FlagEvaluationDetails details, Map hints) { + throw new RuntimeException("finallyAfter exploded"); + } + }; + + assertDoesNotThrow(() -> executor.execute( + stubProvider("p", List.of(throwingFinallyHook)), + "flag", + "default", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> ProviderEvaluation.builder().value("ok").build())); + } + + @Test + void finallyAfterReceivesSyntheticDetailsWhenBeforeThrows() { + AtomicReference> captured = new AtomicReference<>(); + Hook hook = new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + throw new RuntimeException("before failed"); + } + + @Override + public void finallyAfter( + HookContext ctx, FlagEvaluationDetails details, Map hints) { + captured.set(details); + } + }; + + assertThrows( + RuntimeException.class, + () -> executor.execute( + stubProvider("p", List.of(hook)), + "flag", + "fallback", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> + ProviderEvaluation.builder().value("ok").build())); + + assertNotNull(captured.get(), "finallyAfter must be called even when before() throws"); + assertEquals("flag", captured.get().getFlagKey()); + assertEquals("fallback", captured.get().getValue()); + } + + @Test + void passesDefaultValueThroughToHookContext() { + AtomicReference capturedDefault = new AtomicReference<>(); + Hook hook = new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + capturedDefault.set(ctx.getDefaultValue()); + return Optional.empty(); + } + }; + + executor.execute( + stubProvider("p", List.of(hook)), + "flag", + "the-default", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> ProviderEvaluation.builder().value("ok").build()); + + assertEquals("the-default", capturedDefault.get()); + } + + @Test + void runsErrorStageWhenProviderReturnsAnErrorCodedEvaluation() { + AtomicReference capturedError = new AtomicReference<>(); + AtomicReference> capturedDetails = new AtomicReference<>(); + AtomicBoolean afterCalled = new AtomicBoolean(false); + Hook hook = new Hook() { + @Override + public void after( + HookContext ctx, FlagEvaluationDetails details, Map hints) { + afterCalled.set(true); + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + capturedError.set(error); + } + + @Override + public void finallyAfter( + HookContext ctx, FlagEvaluationDetails details, Map hints) { + capturedDetails.set(details); + } + }; + + ProviderEvaluation result = executor.execute( + stubProvider("p", List.of(hook)), + "flag", + "fallback", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> ProviderEvaluation.builder() + .errorCode(ErrorCode.FLAG_NOT_FOUND) + .errorMessage("nope") + .build()); + + assertEquals(ErrorCode.FLAG_NOT_FOUND, result.getErrorCode()); + assertFalse(afterCalled.get(), "after() must not run for an error-coded evaluation"); + assertInstanceOf(FlagNotFoundError.class, capturedError.get()); + assertEquals("fallback", capturedDetails.get().getValue()); + assertEquals(Reason.ERROR.toString(), capturedDetails.get().getReason()); + } + + @Test + void usesErrorCodeFromOpenFeatureErrorThrownByProvider() { + AtomicReference> capturedDetails = new AtomicReference<>(); + Hook hook = new Hook() { + @Override + public void finallyAfter( + HookContext ctx, FlagEvaluationDetails details, Map hints) { + capturedDetails.set(details); + } + }; + + assertThrows( + TypeMismatchError.class, + () -> executor.execute( + stubProvider("p", List.of(hook)), + "flag", + "fallback", + FlagValueType.STRING, + null, + clientMetadata, + null, + (p, ctx) -> { + throw new TypeMismatchError("wrong type"); + })); + + assertEquals(ErrorCode.TYPE_MISMATCH, capturedDetails.get().getErrorCode()); + assertEquals("wrong type", capturedDetails.get().getErrorMessage()); + assertEquals("fallback", capturedDetails.get().getValue()); + } + + @Test + void passesClientMetadataAndHintsToHooks() { + AtomicReference capturedClientName = new AtomicReference<>(); + AtomicReference> capturedHints = new AtomicReference<>(); + Hook hook = new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + capturedClientName.set(ctx.getClientMetadata().getName()); + capturedHints.set(hints); + return Optional.empty(); + } + }; + + executor.execute( + stubProvider("p", List.of(hook)), + "flag", + "default", + FlagValueType.STRING, + null, + () -> "my-client", + Map.of("hint", "value"), + (p, ctx) -> ProviderEvaluation.builder().value("ok").build()); + + assertEquals("my-client", capturedClientName.get()); + assertEquals("value", capturedHints.get().get("hint")); + } + + @Test + void mergesContextReturnedFromBeforeHookAndPreservesTargetingKey() { + AtomicReference capturedProviderContext = new AtomicReference<>(); + Hook hook = new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + return Optional.of(new ImmutableContext(Map.of("added", new Value("yes")))); + } + }; + + executor.execute( + stubProvider("p", List.of(hook)), + "flag", + "default", + FlagValueType.STRING, + new ImmutableContext("user-1", Map.of("original", new Value("kept"))), + clientMetadata, + null, + (p, ctx) -> { + capturedProviderContext.set(ctx); + return ProviderEvaluation.builder().value("ok").build(); + }); + + EvaluationContext providerContext = capturedProviderContext.get(); + assertNotNull(providerContext); + assertEquals("user-1", providerContext.getTargetingKey()); + assertEquals("kept", providerContext.getValue("original").asString()); + assertEquals("yes", providerContext.getValue("added").asString()); + } + + @Test + void passesContextWithoutTargetingKey() { + AtomicReference capturedProviderContext = new AtomicReference<>(); + Hook hook = new Hook() {}; + + executor.execute( + stubProvider("p", List.of(hook)), + "flag", + "default", + FlagValueType.STRING, + new ImmutableContext(Map.of("original", new Value("kept"))), + clientMetadata, + null, + (p, ctx) -> { + capturedProviderContext.set(ctx); + return ProviderEvaluation.builder().value("ok").build(); + }); + + assertNull(capturedProviderContext.get().getTargetingKey()); + assertEquals("kept", capturedProviderContext.get().getValue("original").asString()); + } + + private static Hook orderRecordingHook(List calls, String name) { + return new Hook() { + @Override + public Optional before(HookContext ctx, Map hints) { + calls.add("before:" + name); + return Optional.empty(); + } + + @Override + public void after( + HookContext ctx, FlagEvaluationDetails details, Map hints) { + calls.add("after:" + name); + } + + @Override + public void finallyAfter( + HookContext ctx, FlagEvaluationDetails details, Map hints) { + calls.add("finally:" + name); + } + }; + } + + @SuppressWarnings("rawtypes") + private static FeatureProvider stubProvider(String name, List hooks) { + return new EventProvider() { + @Override + public Metadata getMetadata() { + return () -> name; + } + + @Override + public List getProviderHooks() { + return hooks; + } + + @Override + public ProviderEvaluation getBooleanEvaluation( + String key, Boolean defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getStringEvaluation( + String key, String defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getIntegerEvaluation( + String key, Integer defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getDoubleEvaluation( + String key, Double defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getObjectEvaluation( + String key, Value defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + }; + } +} diff --git a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java new file mode 100644 index 000000000..1cc815609 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java @@ -0,0 +1,270 @@ +package dev.openfeature.sdk.multiprovider; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; + +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.ErrorCode; +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.EventProvider; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.FlagEvaluationOptions; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; +import dev.openfeature.sdk.ImmutableContext; +import dev.openfeature.sdk.Metadata; +import dev.openfeature.sdk.MutableContext; +import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.Reason; +import dev.openfeature.sdk.Value; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.Test; + +class MultiProviderHooksTest { + + @Test + void shouldExecuteProviderHooksAndKeepPerProviderContextIsolation() throws Exception { + RecordingHook firstHook = new RecordingHook("provider1"); + RecordingHook secondHook = new RecordingHook("provider2"); + + HookedStringProvider provider1 = new HookedStringProvider( + "provider1", + List.of(firstHook), + ProviderEvaluation.builder() + .errorCode(dev.openfeature.sdk.ErrorCode.GENERAL) + .errorMessage("failed") + .build()); + HookedStringProvider provider2 = new HookedStringProvider( + "provider2", + List.of(secondHook), + ProviderEvaluation.builder().value("ok").build()); + + MultiProvider multiProvider = new MultiProvider(List.of(provider1, provider2), new FirstSuccessfulStrategy()); + multiProvider.initialize(null); + + ProviderEvaluation evaluation = multiProvider.getStringEvaluation("flag", "default", null); + + assertEquals("ok", evaluation.getValue()); + + assertEquals(1, firstHook.beforeCount.get()); + assertEquals(0, firstHook.afterCount.get()); + assertEquals(1, firstHook.errorCount.get()); + assertEquals(1, firstHook.finallyCount.get()); + + assertEquals(1, secondHook.beforeCount.get()); + assertEquals(1, secondHook.afterCount.get()); + assertEquals(0, secondHook.errorCount.get()); + assertEquals(1, secondHook.finallyCount.get()); + + assertEquals( + "provider1", + provider1.lastEvaluationContext.getValue("hookOwner").asString()); + assertNull(provider1.lastEvaluationContext.getValue("provider2Marker")); + assertNotNull(firstHook.lastFinallyDetails); + assertEquals(ErrorCode.GENERAL, firstHook.lastFinallyDetails.getErrorCode()); + assertEquals(Reason.ERROR.toString(), firstHook.lastFinallyDetails.getReason()); + assertEquals("default", firstHook.lastFinallyDetails.getValue()); + assertEquals("failed", firstHook.lastFinallyDetails.getErrorMessage()); + + assertEquals( + "provider2", + provider2.lastEvaluationContext.getValue("hookOwner").asString()); + assertNull(provider2.lastEvaluationContext.getValue("provider1Marker")); + } + + @Test + void shouldPassHookHintsAndClientMetadataAndEnrichThrownProviderErrors() throws Exception { + RecordingHook firstHook = new RecordingHook("provider1"); + RecordingHook secondHook = new RecordingHook("provider2"); + + HookedStringProvider provider1 = + new HookedStringProvider("provider1", List.of(firstHook), new RuntimeException("boom")); + HookedStringProvider provider2 = new HookedStringProvider( + "provider2", + List.of(secondHook), + ProviderEvaluation.builder().value("ok").build()); + + MultiProvider multiProvider = new MultiProvider(List.of(provider1, provider2), new FirstSuccessfulStrategy()); + + OpenFeatureAPI api = OpenFeatureAPI.createIsolated(); + try { + api.setProviderAndWait("multiProviderHooks", multiProvider); + Client client = api.getClient("multiProviderHooks"); + + var evaluation = client.getStringDetails( + "flag", + "default", + new ImmutableContext(), + FlagEvaluationOptions.builder() + .hookHints(Map.of("hintKey", "hintValue")) + .build()); + + assertEquals("ok", evaluation.getValue()); + + assertEquals("hintValue", firstHook.lastHints.get("hintKey")); + assertEquals("hintValue", secondHook.lastHints.get("hintKey")); + assertEquals("multiProviderHooks", firstHook.lastClientDomain); + assertEquals("multiProviderHooks", secondHook.lastClientDomain); + + assertNotNull(firstHook.lastFinallyDetails); + assertEquals(ErrorCode.GENERAL, firstHook.lastFinallyDetails.getErrorCode()); + assertEquals(Reason.ERROR.toString(), firstHook.lastFinallyDetails.getReason()); + assertEquals("default", firstHook.lastFinallyDetails.getValue()); + assertEquals("boom", firstHook.lastFinallyDetails.getErrorMessage()); + } finally { + api.shutdown(); + } + } + + @Test + void beforeHookAndResolveRunOnSameThread() { + // locks the assumption behind MultiProvider#localHookExecutionContext + RecordingHook hook = new RecordingHook("provider1"); + HookedStringProvider provider = new HookedStringProvider( + "provider1", + List.of(hook), + ProviderEvaluation.builder().value("ok").build()); + MultiProvider multiProvider = new MultiProvider(List.of(provider)); + + OpenFeatureAPI api = OpenFeatureAPI.createIsolated(); + try { + api.setProviderAndWait("threadAssumption", multiProvider); + assertEquals("ok", api.getClient("threadAssumption").getStringValue("flag", "default")); + + assertNotNull(hook.beforeThread); + assertSame(hook.beforeThread, provider.lastEvaluationThread); + } finally { + api.shutdown(); + } + } + + static class RecordingHook implements Hook { + private final String providerName; + private final AtomicInteger beforeCount = new AtomicInteger(); + private final AtomicInteger afterCount = new AtomicInteger(); + private final AtomicInteger errorCount = new AtomicInteger(); + private final AtomicInteger finallyCount = new AtomicInteger(); + private Map lastHints = Map.of(); + private String lastClientDomain; + private FlagEvaluationDetails lastFinallyDetails; + private Thread beforeThread; + + RecordingHook(String providerName) { + this.providerName = providerName; + } + + @Override + public Optional before(HookContext ctx, Map hints) { + beforeCount.incrementAndGet(); + beforeThread = Thread.currentThread(); + ctx.getHookData().set("provider", providerName); + lastHints = hints; + lastClientDomain = ctx.getClientMetadata().getDomain(); + return Optional.of( + new MutableContext().add("hookOwner", providerName).add(providerName + "Marker", providerName)); + } + + @Override + public void after( + HookContext ctx, + dev.openfeature.sdk.FlagEvaluationDetails details, + Map hints) { + afterCount.incrementAndGet(); + assertEquals(providerName, ctx.getHookData().get("provider")); + lastHints = hints; + lastClientDomain = ctx.getClientMetadata().getDomain(); + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + errorCount.incrementAndGet(); + assertEquals(providerName, ctx.getHookData().get("provider")); + lastHints = hints; + lastClientDomain = ctx.getClientMetadata().getDomain(); + } + + @Override + public void finallyAfter( + HookContext ctx, + dev.openfeature.sdk.FlagEvaluationDetails details, + Map hints) { + finallyCount.incrementAndGet(); + assertEquals(providerName, ctx.getHookData().get("provider")); + lastHints = hints; + lastClientDomain = ctx.getClientMetadata().getDomain(); + lastFinallyDetails = details; + } + } + + static class HookedStringProvider extends EventProvider { + private final String name; + private final List> hooks; + private final ProviderEvaluation evaluation; + private final RuntimeException evaluationException; + private EvaluationContext lastEvaluationContext; + private Thread lastEvaluationThread; + + HookedStringProvider(String name, List> hooks, ProviderEvaluation evaluation) { + this.name = name; + this.hooks = hooks; + this.evaluation = evaluation; + this.evaluationException = null; + } + + HookedStringProvider(String name, List> hooks, RuntimeException evaluationException) { + this.name = name; + this.hooks = hooks; + this.evaluation = null; + this.evaluationException = evaluationException; + } + + @Override + public Metadata getMetadata() { + return () -> name; + } + + @Override + @SuppressWarnings("rawtypes") + public List getProviderHooks() { + return List.copyOf(hooks); + } + + @Override + public ProviderEvaluation getBooleanEvaluation( + String key, Boolean defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { + lastEvaluationContext = ctx == null ? new MutableContext() : ctx; + lastEvaluationThread = Thread.currentThread(); + if (evaluationException != null) { + throw evaluationException; + } + return evaluation; + } + + @Override + public ProviderEvaluation getIntegerEvaluation( + String key, Integer defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + + @Override + public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { + return ProviderEvaluation.builder().value(defaultValue).build(); + } + } +} From 62d60cc8b16a46825a269a0161950dbef3b5d432 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 25 Sep 2026 15:02:03 -0400 Subject: [PATCH 2/3] fixup: comment Signed-off-by: Todd Baert --- .../openfeature/sdk/multiprovider/MultiProviderHooksTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java index 1cc815609..e2e8cb8be 100644 --- a/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java +++ b/src/test/java/dev/openfeature/sdk/multiprovider/MultiProviderHooksTest.java @@ -29,7 +29,7 @@ class MultiProviderHooksTest { @Test - void shouldExecuteProviderHooksAndKeepPerProviderContextIsolation() throws Exception { + void shouldRunChildHooksForwardTheirContextToEvaluationAndIsolatePerProvider() throws Exception { RecordingHook firstHook = new RecordingHook("provider1"); RecordingHook secondHook = new RecordingHook("provider2"); @@ -62,6 +62,7 @@ void shouldExecuteProviderHooksAndKeepPerProviderContextIsolation() throws Excep assertEquals(0, secondHook.errorCount.get()); assertEquals(1, secondHook.finallyCount.get()); + // a child hook's before-enriched context must reach that child's evaluation assertEquals( "provider1", provider1.lastEvaluationContext.getValue("hookOwner").asString()); From 94ddcb9b16faf3c972ada962243bcf23b54981ec Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 25 Sep 2026 15:14:33 -0400 Subject: [PATCH 3/3] pr feedback Signed-off-by: Todd Baert --- .../sdk/multiprovider/MultiProvider.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java index e190ba885..8335f234a 100644 --- a/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java +++ b/src/main/java/dev/openfeature/sdk/multiprovider/MultiProvider.java @@ -193,12 +193,14 @@ public Metadata getMetadata() { @Override public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { + HookExecutionContext hookCtx = currentHookExecutionContext(); return strategy.evaluate( providers, key, defaultValue, ctx, provider -> evaluateChild( + hookCtx, provider, key, defaultValue, @@ -209,12 +211,14 @@ public ProviderEvaluation getBooleanEvaluation(String key, Boolean defa @Override public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { + HookExecutionContext hookCtx = currentHookExecutionContext(); return strategy.evaluate( providers, key, defaultValue, ctx, provider -> evaluateChild( + hookCtx, provider, key, defaultValue, @@ -225,12 +229,14 @@ public ProviderEvaluation getStringEvaluation(String key, String default @Override public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { + HookExecutionContext hookCtx = currentHookExecutionContext(); return strategy.evaluate( providers, key, defaultValue, ctx, provider -> evaluateChild( + hookCtx, provider, key, defaultValue, @@ -241,12 +247,14 @@ public ProviderEvaluation getIntegerEvaluation(String key, Integer defa @Override public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { + HookExecutionContext hookCtx = currentHookExecutionContext(); return strategy.evaluate( providers, key, defaultValue, ctx, provider -> evaluateChild( + hookCtx, provider, key, defaultValue, @@ -257,12 +265,14 @@ public ProviderEvaluation getDoubleEvaluation(String key, Double default @Override public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { + HookExecutionContext hookCtx = currentHookExecutionContext(); return strategy.evaluate( providers, key, defaultValue, ctx, provider -> evaluateChild( + hookCtx, provider, key, defaultValue, @@ -271,16 +281,15 @@ public ProviderEvaluation getObjectEvaluation(String key, Value defaultVa (p, evaluationContext) -> p.getObjectEvaluation(key, defaultValue, evaluationContext))); } - // runs the child's own hooks around its evaluation, using captured client metadata and hints - // (or this provider's metadata and no hints when evaluated outside a client lifecycle) + // runs the child's own hooks; caller-thread snapshot private ProviderEvaluation evaluateChild( + HookExecutionContext hookExecutionContext, FeatureProvider provider, String key, T defaultValue, EvaluationContext ctx, FlagValueType type, BiFunction> providerFunction) { - HookExecutionContext hookExecutionContext = currentHookExecutionContext(); ClientMetadata clientMetadata = hookExecutionContext != null ? hookExecutionContext.clientMetadata : hookClientMetadata; Map hints = hookExecutionContext != null ? hookExecutionContext.hints : Collections.emptyMap();