fix: updated context not passed to all hooks

Signed-off-by: Todd Baert <[email protected]>

src/main/java/dev/openfeature/sdk/HookContext.java

@@ -4,6 +4,7 @@
 import lombok.NonNull;
 import lombok.Value;
 import lombok.With;
+import lombok.experimental.NonFinal;
 
 /**
  * A data class to hold immutable context that {@link Hook} instances use.
@@ -15,10 +16,19 @@ public class HookContext<T> {
  @NonNull String flagKey;
  @NonNull FlagValueType type;
  @NonNull T defaultValue;
- @NonNull EvaluationContext ctx;
+ @NonNull @NonFinal EvaluationContext ctx;
  ClientMetadata clientMetadata;
  Metadata providerMetadata;
 
+ /**
+ * Internal means of updating the evaluation context in this hook context.
+ * Not for public use.
+ * @param ctx context to set
+ */
+ void setEvaluationCtx(EvaluationContext ctx) {
+ this.ctx = ctx;
+ }
+
  /**
  * Builds a {@link HookContext} instances from request data.
  * @param key feature flag key

src/main/java/dev/openfeature/sdk/ImmutableContext.java

@@ -17,7 +17,7 @@
 public final class ImmutableContext implements EvaluationContext {
 
  @Delegate
- private final Structure structure;
+ private Structure structure;
 
  /**
  * Create an immutable context with an empty targeting_key and attributes provided.
@@ -78,7 +78,7 @@ public String getTargetingKey() {
  @Override
  public EvaluationContext merge(EvaluationContext overridingContext) {
  if (overridingContext == null) {
- return new ImmutableContext(this.asMap());
+ return this;
  }
 
  return new ImmutableContext(

src/main/java/dev/openfeature/sdk/OpenFeatureClient.java

@@ -116,11 +116,10 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
  openfeatureApi.getHooks());
 
  hookCtx = HookContext.from(key, type, this.getMetadata(),
- provider.getMetadata(), ctx, defaultValue);
+ provider.getMetadata(), mergeEvaluationContext(ctx), defaultValue);
 
- EvaluationContext ctxFromHook = hookSupport.beforeHooks(type, hookCtx, mergedHooks, hints);
-
- EvaluationContext mergedCtx = mergeEvaluationContext(ctxFromHook, ctx);
+ EvaluationContext mergedCtx = hookSupport.beforeHooks(type, hookCtx, mergedHooks, hints);
+ hookCtx.setEvaluationCtx(mergedCtx); 
 
  ProviderEvaluation<T> providerEval = (ProviderEvaluation<T>) createProviderEvaluation(type, key,
  defaultValue, provider, mergedCtx);
@@ -153,15 +152,13 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
  }
 
  /**
- * Merge hook and invocation contexts with API, transaction and client contexts.
+ * Merge invocation contexts with API, transaction and client contexts.
+ * Does not merge before context.
  *
- * @param hookContext hook context
  * @param invocationContext invocation context
  * @return merged evaluation context
  */
- private EvaluationContext mergeEvaluationContext(
- EvaluationContext hookContext,
- EvaluationContext invocationContext) {
+ private EvaluationContext mergeEvaluationContext(EvaluationContext invocationContext) {
  final EvaluationContext apiContext = openfeatureApi.getEvaluationContext() != null
  ? openfeatureApi.getEvaluationContext()
  : new ImmutableContext();
@@ -172,7 +169,7 @@ private EvaluationContext mergeEvaluationContext(
  ? openfeatureApi.getTransactionContext()
  : new ImmutableContext();
 
- return apiContext.merge(transactionContext.merge(clientContext.merge(invocationContext.merge(hookContext))));
+ return apiContext.merge(transactionContext.merge(clientContext.merge(invocationContext)));
  }
 
  private <T> ProviderEvaluation<?> createProviderEvaluation(

src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java

@@ -2,7 +2,6 @@
 
 import static dev.openfeature.sdk.DoSomethingProvider.DEFAULT_METADATA;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -20,6 +19,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 import org.awaitility.Awaitility;
 import org.junit.jupiter.api.AfterEach;
@@ -336,11 +336,25 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
  FeatureProviderTestUtils.setFeatureProvider(provider);
  TransactionContextPropagator transactionContextPropagator = new ThreadLocalTransactionContextPropagator();
  api.setTransactionContextPropagator(transactionContextPropagator);
+ Hook<Boolean> hook = spy(new Hook<Boolean>() {
+ @Override
+ public Optional<EvaluationContext> before(HookContext<Boolean> ctx, Map<String, Object> hints) {
+ Map<String, Value> attrs = ctx.getCtx().asMap();
+ attrs.put("before", new Value("5"));
+ attrs.put("common7", new Value("5"));
+ return Optional.ofNullable(new ImmutableContext(attrs));
+ }
+ @Override
+ public void after(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
+ Hook.super.after(ctx, details, hints);
+ }
+ });
 
  Map<String, Value> apiAttributes = new HashMap<>();
  apiAttributes.put("common1", new Value("1"));
  apiAttributes.put("common2", new Value("1"));
  apiAttributes.put("common3", new Value("1"));
+ apiAttributes.put("common7", new Value("1"));
  apiAttributes.put("api", new Value("1"));
  EvaluationContext apiCtx = new ImmutableContext(apiAttributes);
 
@@ -377,21 +391,54 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
  invocationAttributes.put("invocation", new Value("4"));
  EvaluationContext invocationCtx = new ImmutableContext(invocationAttributes);
 
- c.getBooleanValue("key", false, invocationCtx);
-
- // assert the connect overrides
+ c.getBooleanValue("key", false, invocationCtx, FlagEvaluationOptions.builder().hook(hook).build());
+
+ // assert the connect overrides in before hook
+ verify(hook).before(argThat((arg) -> {
+ EvaluationContext evaluationContext = arg.getCtx();
+ return evaluationContext.getValue("api").asString().equals("1") &&
+ evaluationContext.getValue("transaction").asString().equals("2") &&
+ evaluationContext.getValue("client").asString().equals("3") &&
+ evaluationContext.getValue("invocation").asString().equals("4") &&
+ evaluationContext.getValue("common1").asString().equals("2") &&
+ evaluationContext.getValue("common2").asString().equals("3") &&
+ evaluationContext.getValue("common3").asString().equals("4") &&
+ evaluationContext.getValue("common4").asString().equals("3") &&
+ evaluationContext.getValue("common5").asString().equals("4") &&
+ evaluationContext.getValue("common6").asString().equals("4");
+ }), any());
+
+ // assert the connect overrides in evaluation
  verify(provider).getBooleanEvaluation(any(), any(), argThat((arg) -> {
  return arg.getValue("api").asString().equals("1") &&
  arg.getValue("transaction").asString().equals("2") &&
  arg.getValue("client").asString().equals("3") &&
  arg.getValue("invocation").asString().equals("4") &&
+ arg.getValue("before").asString().equals("5") &&
  arg.getValue("common1").asString().equals("2") &&
  arg.getValue("common2").asString().equals("3") &&
  arg.getValue("common3").asString().equals("4") &&
  arg.getValue("common4").asString().equals("3") &&
  arg.getValue("common5").asString().equals("4") &&
- arg.getValue("common6").asString().equals("4");
+ arg.getValue("common6").asString().equals("4") &&
+ arg.getValue("common7").asString().equals("5");
  }));
+
+ // assert the connect overrides in after hook
+ verify(hook).after(argThat((arg) -> {
+ EvaluationContext evaluationContext = arg.getCtx();
+ return evaluationContext.getValue("api").asString().equals("1") &&
+ evaluationContext.getValue("transaction").asString().equals("2") &&
+ evaluationContext.getValue("client").asString().equals("3") &&
+ evaluationContext.getValue("invocation").asString().equals("4") &&
+ evaluationContext.getValue("before").asString().equals("5") &&
+ evaluationContext.getValue("common1").asString().equals("2") &&
+ evaluationContext.getValue("common2").asString().equals("3") &&
+ evaluationContext.getValue("common3").asString().equals("4") &&
+ evaluationContext.getValue("common4").asString().equals("3") &&
+ evaluationContext.getValue("common5").asString().equals("4") &&
+ evaluationContext.getValue("common6").asString().equals("4");
+ }), any(), any());
  }
 
  @Specification(number="3.3.1.1", text="The API SHOULD have a method for setting a transaction context propagator.")

src/test/java/dev/openfeature/sdk/HookSpecTest.java

@@ -484,7 +484,8 @@ public void finallyAfter(HookContext<Boolean> ctx, Map<String, Object> hints) {
  @Specification(number = "4.1.4", text = "The evaluation context MUST be mutable only within the before hook.")
  @Specification(number = "4.3.4", text = "Any `evaluation context` returned from a `before` hook MUST be passed to subsequent `before` hooks (via `HookContext`).")
  @Test void beforeContextUpdated() {
- EvaluationContext ctx = new ImmutableContext();
+ String targetingKey = "test-key";
+ EvaluationContext ctx = new ImmutableContext(targetingKey);
  Hook hook = mockBooleanHook();
  when(hook.before(any(), any())).thenReturn(Optional.of(ctx));
  Hook hook2 = mockBooleanHook();
@@ -503,7 +504,7 @@ public void finallyAfter(HookContext<Boolean> ctx, Map<String, Object> hints) {
  order.verify(hook2).before(captor.capture(), any());
 
  HookContext hc = captor.getValue();
- assertEquals(hc.getCtx(), ctx);
+ assertEquals(hc.getCtx().getTargetingKey(), targetingKey);
 
  }