Support for eval context & merging relevant thereof

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

@@ -1,33 +1,21 @@
 package dev.openfeature.javasdk;
 
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
+import lombok.Setter;
+import lombok.ToString;
 
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.HashMap;
 import java.util.Map;
 
+@ToString @EqualsAndHashCode
 public class EvaluationContext {
-    @Getter private final String targetingKey;
+    @Setter @Getter private String targetingKey;
     private final Map<String, Integer> integerAttributes;
     private final Map<String, String> stringAttributes;
 
-
-    private enum KNOWN_KEYS {
-        EMAIL,
-        FIRST_NAME,
-        LAST_NAME,
-        NAME,
-        IP,
-        TZ,
-        LOCALE,
-        COUNTRY_CODE,
-        ENVIRONMENT,
-        APPLICATION,
-        VERSION,
-        TIMESTAMP,
-    }
-
     EvaluationContext() {
         this.targetingKey = "";
         this.integerAttributes = new HashMap<>();
@@ -62,6 +50,8 @@ public class EvaluationContext {
         this.stringAttributes.put(key, value.format(DateTimeFormatter.ISO_ZONED_DATE_TIME));
     }
 
+    // TODO: addStructure or similar.
+
     public ZonedDateTime getDatetimeAttribute(String key) {
         String attr = this.stringAttributes.get(key);
         if (attr == null) {
@@ -70,107 +60,34 @@ public class EvaluationContext {
         return ZonedDateTime.parse(attr, DateTimeFormatter.ISO_ZONED_DATE_TIME);
     }
 
-    public String getEmail() {
-        return this.stringAttributes.get(KNOWN_KEYS.EMAIL.toString());
-    }
-
-    public String getFirstName() {
-        return this.stringAttributes.get(KNOWN_KEYS.FIRST_NAME.toString());
-    }
-
-    public String getLastName() {
-        return this.stringAttributes.get(KNOWN_KEYS.LAST_NAME.toString());
-    }
-
-    public String getName() {
-        return this.stringAttributes.get(KNOWN_KEYS.NAME.toString());
-    }
-
-    public String getIp() {
-        return this.stringAttributes.get(KNOWN_KEYS.IP.toString());
-    }
-
-    public String getTz() {
-        return this.stringAttributes.get(KNOWN_KEYS.TZ.toString());
-    }
-
-    public String getLocale() {
-        return this.stringAttributes.get(KNOWN_KEYS.LOCALE.toString());
-    }
-
-    public String getCountryCode() {
-        return this.stringAttributes.get(KNOWN_KEYS.COUNTRY_CODE.toString());
-    }
-
-    public String getEnvironment() {
-        return this.stringAttributes.get(KNOWN_KEYS.ENVIRONMENT.toString());
-    }
-
-    public String getApplication() {
-        return this.stringAttributes.get(KNOWN_KEYS.APPLICATION.toString());
-    }
-
-    public String getVersion() {
-        return this.stringAttributes.get(KNOWN_KEYS.VERSION.toString());
-    }
-
-    public ZonedDateTime getTimestamp() {
-        return getDatetimeAttribute(KNOWN_KEYS.TIMESTAMP.toString());
-    }
-
-    public void setEmail(String email) {
-        this.stringAttributes.put(KNOWN_KEYS.EMAIL.toString(), email);
-    }
-
-    public void setFirstName(String firstname) {
-        this.stringAttributes.put(KNOWN_KEYS.FIRST_NAME.toString(), firstname);
-    }
-
-    public void setLastName(String lastname) {
-        this.stringAttributes.put(KNOWN_KEYS.LAST_NAME.toString(), lastname);
-    }
-
-    public void setName(String name) {
-        this.stringAttributes.put(KNOWN_KEYS.NAME.toString(), name);
-    }
-
-    public void setIp(String ip) {
-        this.stringAttributes.put(KNOWN_KEYS.IP.toString(), ip);
-    }
-
-    public void setTz(String tz) {
-        this.stringAttributes.put(KNOWN_KEYS.TZ.toString(), tz);
-    }
-
-    public void setLocale(String locale) {
-        this.stringAttributes.put(KNOWN_KEYS.LOCALE.toString(), locale);
-    }
-
-    public void setCountryCode(String countryCode) {
-        this.stringAttributes.put(KNOWN_KEYS.COUNTRY_CODE.toString(), countryCode);
-    }
-
-    public void setEnvironment(String environment) {
-        this.stringAttributes.put(KNOWN_KEYS.ENVIRONMENT.toString(), environment);
-    }
-
-    public void setApplication(String application) {
-        this.stringAttributes.put(KNOWN_KEYS.APPLICATION.toString(), application);
-    }
-
-    public void setVersion(String version) {
-        this.stringAttributes.put(KNOWN_KEYS.VERSION.toString(), version);
-    }
-
-    public void setTimestamp(ZonedDateTime timestamp) {
-        addDatetimeAttribute(KNOWN_KEYS.TIMESTAMP.toString(), timestamp);
-    }
-
     /**
      * 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.
+        EvaluationContext ec = new EvaluationContext();
-        return ctx1;
+        for (Map.Entry<String, Integer> e : ctx1.integerAttributes.entrySet()) {
+            ec.addIntegerAttribute(e.getKey(), e.getValue());
+        }
+
+        for (Map.Entry<String, Integer> e : ctx2.integerAttributes.entrySet()) {
+            ec.addIntegerAttribute(e.getKey(), e.getValue());
+        }
+
+        for (Map.Entry<String, String> e : ctx1.stringAttributes.entrySet()) {
+            ec.addStringAttribute(e.getKey(), e.getValue());
+        }
+
+        for (Map.Entry<String, String> e : ctx2.stringAttributes.entrySet()) {
+            ec.addStringAttribute(e.getKey(), e.getValue());
+        }
+        if (ctx1.getTargetingKey() != null) {
+            ec.setTargetingKey(ctx1.getTargetingKey());
+        }
+
+        if (ctx2.getTargetingKey() != null) {
+            ec.setTargetingKey(ctx2.getTargetingKey());
+        }
+
+        return ec;
     }
 }

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

@@ -7,7 +7,7 @@ import java.util.Optional;
 // TODO: interface? or abstract class?
 public abstract class Hook<T> {
     public Optional<EvaluationContext> before(HookContext<T> ctx, ImmutableMap<String, Object> hints) {
-        return null;
+        return Optional.empty();
     }
     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) {}

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

@@ -4,14 +4,13 @@ import lombok.Builder;
 import lombok.NonNull;
 import lombok.Value;
 import lombok.With;
-import javax.annotation.Nullable;
 
 @Value @Builder @With
 public class HookContext<T> {
     @NonNull String flagKey;
     @NonNull FlagValueType type;
     @NonNull T defaultValue;
-    @Nullable EvaluationContext ctx;
+    @NonNull EvaluationContext ctx;
     Client client;
     FeatureProvider provider;
 

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

@@ -36,11 +36,14 @@ public class OpenFeatureClient implements Client {
     <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) {
         FeatureProvider provider = this.openfeatureApi.getProvider();
         ImmutableMap<String, Object> hints = options.getHookHints();
+        if (ctx == null) {
+            ctx = new EvaluationContext();
+        }
 
         // merge of: API.context, client.context, invocation.context
 
         // TODO: Context transformation?
-        HookContext hookCtx = HookContext.from(key, type, this, null, defaultValue);
+        HookContext hookCtx = HookContext.from(key, type, this, ctx, defaultValue);
 
         List<Hook> mergedHooks;
         if (options != null && options.getHooks() != null) {
@@ -112,7 +115,7 @@ public class OpenFeatureClient implements Client {
         EvaluationContext ctx = hookCtx.getCtx();
         for (Hook hook : Lists.reverse(hooks)) {
             Optional<EvaluationContext> newCtx = hook.before(hookCtx, hints);
-            if (newCtx.isPresent()) {
+            if (newCtx != null && newCtx.isPresent()) {
                 ctx = EvaluationContext.merge(ctx, newCtx.get());
                 hookCtx = hookCtx.withCtx(ctx);
             }
@@ -137,7 +140,7 @@ public class OpenFeatureClient implements Client {
 
     @Override
     public FlagEvaluationDetails<Boolean> getBooleanDetails(String key, Boolean defaultValue) {
-        return getBooleanDetails(key, defaultValue, null);
+        return getBooleanDetails(key, defaultValue, new EvaluationContext());
     }
 
     @Override
@@ -197,7 +200,7 @@ public class OpenFeatureClient implements Client {
 
     @Override
     public FlagEvaluationDetails<Integer> getIntegerDetails(String key, Integer defaultValue) {
-        return getIntegerDetails(key, defaultValue, null);
+        return getIntegerDetails(key, defaultValue, new EvaluationContext());
     }
 
     @Override

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

@@ -6,21 +6,20 @@ import org.junit.jupiter.api.Test;
 import java.time.ZonedDateTime;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
 
 public class EvalContextTests {
-    @Specification(spec="flag evaluation", number="3.1",
+    @Specification(spec="Evaluation Context", number="3.1",
-            text="The `evaluation context` structure MUST define a required `targeting key` " +
+            text="The `evaluation context` structure **MUST** define an optional `targeting key` field of " +
-                    "field of type string, identifying the subject of the flag evaluation.")
+                    "type string, identifying the subject of the flag evaluation.")
-    @Disabled("https://github.com/open-feature/spec/pull/60/files#r872827439")
     @Test void requires_targeting_key() {
         EvaluationContext ec = new EvaluationContext();
+        ec.setTargetingKey("targeting-key");
         assertEquals("targeting-key", ec.getTargetingKey());
     }
 
-    @Specification(spec="flag evaluation", number="3.3", text="The evaluation context MUST support the inclusion " +
+    @Specification(spec="Evaluation Context", number="3.2", text="The evaluation context MUST support the inclusion of " +
-            "of custom fields, having keys of type `string`, and values of " +
+            "custom fields, having keys of type `string`, and " +
-            "type `boolean | string | number | datetime`.")
+            "values of type `boolean | string | number | datetime | structure`.")
     @Test void eval_context() {
         EvaluationContext ec = new EvaluationContext();
 
@@ -38,63 +37,9 @@ public class EvalContextTests {
         assertEquals(dt, ec.getDatetimeAttribute("dt"));
     }
 
-
+    @Specification(spec="Evaluation Context", number="3.2", text="The evaluation context MUST support the inclusion of " +
-    @Specification(spec="flag evaluation", number="3.2", text="The evaluation context MUST define the " +
+            "custom fields, having keys of type `string`, and " +
-            "following optional fields: `email` (string), `first name` (string), `last name`(string), " +
+            "values of type `boolean | string | number | datetime | structure`.")
-            "`name`(string), `ip`(string), `tz`(string), `locale`(string), `country code` (string), " +
+    @Disabled("Structure support")
-            "`timestamp`(date), `environment`(string), `application`(string), and `version`(string).")
+    @Test void eval_context__structure() {}
-    @Test void mandated_fields() {
-        EvaluationContext ec = new EvaluationContext();
-
-        assertNull(ec.getEmail());
-        ec.setEmail("Test");
-        assertEquals("Test", ec.getEmail());
-
-        assertNull(ec.getFirstName());
-        ec.setFirstName("Test");
-        assertEquals("Test", ec.getFirstName());
-
-        assertNull(ec.getLastName());
-        ec.setLastName("Test");
-        assertEquals("Test", ec.getLastName());
-
-        assertNull(ec.getName());
-        ec.setName("Test");
-        assertEquals("Test", ec.getName());
-
-        assertNull(ec.getIp());
-        ec.setIp("Test");
-        assertEquals("Test", ec.getIp());
-
-        assertNull(ec.getTz());
-        ec.setTz("Test");
-        assertEquals("Test", ec.getTz());
-
-        assertNull(ec.getLocale());
-        ec.setLocale("Test");
-        assertEquals("Test", ec.getLocale());
-
-        assertNull(ec.getCountryCode());
-        ec.setCountryCode("Test");
-        assertEquals("Test", ec.getCountryCode());
-
-        assertNull(ec.getTimestamp());
-        ZonedDateTime dt = ZonedDateTime.now();
-        ec.setTimestamp(dt);
-        assertEquals(dt, ec.getTimestamp());
-
-        assertNull(ec.getEnvironment());
-        ec.setEnvironment("Test");
-        assertEquals("Test", ec.getEnvironment());
-
-
-        assertNull(ec.getApplication());
-        ec.setApplication("Test");
-        assertEquals("Test", ec.getApplication());
-
-
-        assertNull(ec.getVersion());
-        ec.setVersion("Test");
-        assertEquals("Test", ec.getVersion());
-    }
 }

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

@@ -1,6 +1,5 @@
 package dev.openfeature.javasdk;
 
-import dev.openfeature.javasdk.*;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
@@ -132,6 +131,7 @@ public class FlagEvaluationSpecTests {
             "unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.")
     @Disabled
     @Test void detail_flags() {
+        // TODO: Add tests re: detail functions.
         throw new NotImplementedException();
     }
 

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

@@ -5,6 +5,7 @@ import lombok.SneakyThrows;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 
 import java.util.ArrayList;
@@ -396,7 +397,66 @@ public class HookSpecTests {
     }
 
     @Specification(spec="hooks", number="3.3", text="Any `EvaluationContext` returned from a `before` hook **MUST** be passed to subsequent `before` hooks (via `HookContext`).")
+    @Test void beforeContextUpdated() {
+        EvaluationContext ctx = new EvaluationContext();
+        Hook hook = mock(Hook.class);
+        when(hook.before(any(), any())).thenReturn(Optional.of(ctx));
+        Hook hook2 = mock(Hook.class);
+        when(hook.before(any(), any())).thenReturn(Optional.empty());
+        InOrder order = inOrder(hook, hook2);
+
+
+
+        OpenFeatureAPI api = OpenFeatureAPI.getInstance();
+        api.setProvider(new NoOpProvider());
+        Client client = api.getClient();
+        client.getBooleanValue("key", false, new EvaluationContext(),
+                FlagEvaluationOptions.builder()
+                        .hook(hook2)
+                        .hook(hook)
+                        .build());
+
+        order.verify(hook).before(any(), any());
+        ArgumentCaptor<HookContext> captor = ArgumentCaptor.forClass(HookContext.class);
+        order.verify(hook2).before(captor.capture(), any());
+
+        HookContext hc = captor.getValue();
+        assertEquals(hc.getCtx(), ctx);
+
+    }
     @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.")
+    @Test void mergeHappensCorrectly() {
+        EvaluationContext hookCtx = new EvaluationContext();
+        hookCtx.addStringAttribute("test", "broken");
+        hookCtx.addStringAttribute("another", "exists");
+
+        EvaluationContext invocationCtx = new EvaluationContext();
+        invocationCtx.addStringAttribute("test", "works");
+
+        Hook hook = mock(Hook.class);
+        when(hook.before(any(), any())).thenReturn(Optional.of(hookCtx));
+
+        FeatureProvider provider = mock(FeatureProvider.class);
+        when(provider.getBooleanEvaluation(any(),any(),any(),any())).thenReturn(ProviderEvaluation.<Boolean>builder()
+                        .value(true)
+                .build());
+
+        OpenFeatureAPI api = OpenFeatureAPI.getInstance();
+        api.setProvider(provider);
+        Client client = api.getClient();
+        client.getBooleanValue("key", false, invocationCtx,
+                FlagEvaluationOptions.builder()
+                        .hook(hook)
+                        .build());
+
+        ArgumentCaptor<EvaluationContext> captor = ArgumentCaptor.forClass(EvaluationContext.class);
+        verify(provider).getBooleanEvaluation(any(), any(), captor.capture(), any());
+        EvaluationContext ec = captor.getValue();
+        assertEquals("works", ec.getStringAttribute("test"));
+        assertEquals("exists", ec.getStringAttribute("another"));
+    }
+
+
     @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() {}