Wiring for mergeable evaluation contexts.

Author: justinabrahms

lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java

@@ -1,4 +1,11 @@
 package dev.openfeature.javasdk;
 
 public class EvaluationContext {
+    /**
+     * Merges two EvaluationContext objects with the second overriding the first in case of conflict.
+     */
+    public static EvaluationContext merge(EvaluationContext ctx1, EvaluationContext ctx2) {
+        // TODO(abrahms): Actually implement this when we know what the fields of EC are.
+        return ctx1;
+    }
 }

lib/src/main/java/dev/openfeature/javasdk/Hook.java

@@ -2,9 +2,13 @@
 
 import com.google.common.collect.ImmutableMap;
 
+import java.util.Optional;
+
 // TODO: interface? or abstract class?
 public abstract class Hook<T> {
-    public void before(HookContext<T> ctx, ImmutableMap<String, Object> hints) {}
+    public Optional<EvaluationContext> before(HookContext<T> ctx, ImmutableMap<String, Object> hints) {
+        return null;
+    }
     public void after(HookContext<T> ctx, FlagEvaluationDetails<T> details, ImmutableMap<String, Object> hints) {}
     public void error(HookContext<T> ctx, Exception error, ImmutableMap<String, Object> hints) {}
     public void finallyAfter(HookContext<T> ctx, ImmutableMap<String, Object> hints) {}

lib/src/main/java/dev/openfeature/javasdk/HookContext.java

@@ -3,8 +3,9 @@
 import lombok.Builder;
 import lombok.NonNull;
 import lombok.Value;
+import lombok.With;
 
-@Value @Builder
+@Value @Builder @With
 public class HookContext<T> {
     @NonNull String flagKey;
     @NonNull FlagValueType type;

lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java

@@ -11,6 +11,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 
 @SuppressWarnings("PMD.DataflowAnomalyAnalysis")
 public class OpenFeatureClient implements Client {
@@ -56,13 +57,15 @@ <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key, T defa
 
         FlagEvaluationDetails<T> details = null;
         try {
-            this.beforeHooks(hookCtx, mergedHooks, hints);
+            EvaluationContext ctxFromHook = this.beforeHooks(hookCtx, mergedHooks, hints);
+            EvaluationContext invocationContext = EvaluationContext.merge(ctxFromHook, ctx);
 
             ProviderEvaluation<T> providerEval;
             if (type == FlagValueType.BOOLEAN) {
                 // TODO: Can we guarantee that defaultValue is a boolean? If not, how to we handle that?
-                providerEval = (ProviderEvaluation<T>) provider.getBooleanEvaluation(key, (Boolean) defaultValue, ctx, options);
+                providerEval = (ProviderEvaluation<T>) provider.getBooleanEvaluation(key, (Boolean) defaultValue, invocationContext, options);
             } else {
+                // TODO: Support other flag types.
                 throw new GeneralError("Unknown flag type");
             }
 
@@ -106,13 +109,17 @@ private <T> void afterHooks(HookContext hookContext, FlagEvaluationDetails<T> de
         }
     }
 
-    private HookContext beforeHooks(HookContext hookCtx, List<Hook> hooks, ImmutableMap<String, Object> hints) {
+    private EvaluationContext beforeHooks(HookContext hookCtx, List<Hook> hooks, ImmutableMap<String, Object> hints) {
         // These traverse backwards from normal.
+        EvaluationContext ctx = hookCtx.getCtx();
         for (Hook hook : Lists.reverse(hooks)) {
-            hook.before(hookCtx, hints);
-            // TODO: Merge returned context w/ hook context object
+            Optional<EvaluationContext> newCtx = hook.before(hookCtx, hints);
+            if (newCtx.isPresent()) {
+                ctx = EvaluationContext.merge(ctx, newCtx.get());
+                hookCtx = hookCtx.withCtx(ctx);
+            }
         }
-        return hookCtx;
+        return ctx;
     }
 
     @Override

lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java

@@ -1,7 +1,6 @@
 package dev.openfeature.javasdk;
 
 import com.google.common.collect.ImmutableMap;
-import dev.openfeature.javasdk.*;
 import lombok.SneakyThrows;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Disabled;
@@ -11,6 +10,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
@@ -140,7 +140,7 @@ void emptyApiHooks() {
                 .build();
     }
 
-    @Specification(spec="hooks", number="3.2", text="The before stage MUST run before flag evaluation occurs. It accepts a hook context (required) and HookHints (optional) as parameters and returns either a HookContext or nothing.")
+    @Specification(spec="hooks", number="3.2", text="The before stage MUST run before flag evaluation occurs. It accepts a hook context (required) and HookHints (optional) as parameters and returns either a EvaluationContext or nothing.")
     @Test void before_runs_ahead_of_evaluation() {
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
         api.setProvider(new AlwaysBrokenProvider());
@@ -175,7 +175,7 @@ void emptyApiHooks() {
         verify(h, times(0)).error(any(), any(), any());
     }
 
-    @Specification(spec="hooks", number="3.4", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag evaluation. It accepts hook context (required), exception for what went wrong (required), and HookHints (optional). It has no return value.")
+    @Specification(spec="hooks", number="3.6", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag evaluation. It accepts hook context (required), exception for what went wrong (required), and HookHints (optional). It has no return value.")
     @Test void error_hook_run_during_non_finally_stage() {
         final boolean[] error_called = {false};
         Hook h = mock(Hook.class);
@@ -196,8 +196,9 @@ void emptyApiHooks() {
         api.setProvider(new NoOpProvider());
         api.registerHooks(new Hook<Boolean>() {
             @Override
-            public void before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
+            public Optional<EvaluationContext> before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
                 evalOrder.add("api before");
+                return null;
             }
 
             @Override
@@ -220,8 +221,9 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         Client c = api.getClient();
         c.registerHooks(new Hook<Boolean>() {
             @Override
-            public void before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
+            public Optional<EvaluationContext> before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
                 evalOrder.add("client before");
+                return null;
             }
 
             @Override
@@ -243,8 +245,9 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         c.getBooleanValue("key", false, null, FlagEvaluationOptions.builder()
                         .hook(new Hook<Boolean>() {
                             @Override
-                            public void before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
+                            public Optional<EvaluationContext> before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
                                 evalOrder.add("invocation before");
+                                return null;
                             }
 
                             @Override
@@ -297,8 +300,9 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         Client client = api.getClient();
         Hook<Boolean> mutatingHook = new Hook<Boolean>() {
             @Override
-            public void before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
+            public Optional<EvaluationContext> before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
                 assertTrue(hints instanceof ImmutableMap);
+                return null;
             }
 
             @Override
@@ -332,8 +336,8 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         assertTrue(feo.getHookHints().isEmpty());
     }
 
-    @Specification(spec="hooks", number="3.3", text="The after stage MUST run after flag evaluation occurs. It accepts a hook context (required), flag evaluation details (required) and HookHints (optional). It has no return value.")
-    @Specification(spec="hooks", number="3.5", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and HookHints (optional). There is no return value.")
+    @Specification(spec="hooks", number="3.5", text="The after stage MUST run after flag evaluation occurs. It accepts a hook context (required), flag evaluation details (required) and HookHints (optional). It has no return value.")
+    @Specification(spec="hooks", number="3.7", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and HookHints (optional). There is no return value.")
     @Test void flag_eval_hook_order() {
         Hook hook = mock(Hook.class);
         FeatureProvider provider = mock(FeatureProvider.class);
@@ -391,12 +395,14 @@ public void finallyAfter(HookContext<Boolean> ctx, ImmutableMap<String, Object>
         verify(hook2, times(1)).error(any(), any(), any());
     }
 
+    @Specification(spec="hooks", number="3.3", text="Any `EvaluationContext` returned from a `before` hook **MUST** be passed to subsequent `before` hooks (via `HookContext`).")
+    @Specification(spec="hooks", number="3.4", text="When `before` hooks have finished executing, any resulting `EvaluationContext` **MUST** be merged with the invocation `EvaluationContext` wherein the invocation `EvaluationContext` win any conflicts.")
     @Specification(spec="hooks", number="1.4", text="The evaluation context MUST be mutable only within the before hook.")
     @Specification(spec="hooks", number="3.1", text="Hooks MUST specify at least one stage.")
     @Test @Disabled void todo() {}
 
     @SneakyThrows
-    @Specification(spec="hooks", number="3.6", text="Condition: If finally is a reserved word in the language, finallyAfter SHOULD be used.")
+    @Specification(spec="hooks", number="3.8", text="Condition: If finally is a reserved word in the language, finallyAfter SHOULD be used.")
     @Test void doesnt_use_finally() {
         try {
             Hook.class.getMethod("finally", HookContext.class, ImmutableMap.class);