Repository: open-feature/java-sdk
Pull request #41 (2022-08-02 21:12:16)

iterators to get all integer, string, boolean, or structure map entries
Signed-off-by: Robert Grassian <robert.grassian@split.io>

codecov-commenter: # [Codecov](https://codecov.io/gh/open-feature/java-sdk/pull/41?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) Report
> Merging [#41](https://codecov.io/gh/open-feature/java-sdk/pull/41?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) (af0d026) into [main](https://codecov.io/gh/open-feature/java-sdk/commit/19119067fa9fb95c9a4ce76ca72b0a7006e94052?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) (1911906) will **decrease** coverage by `1.55%`.
> The diff coverage is `0.00%`.

```diff
@@             Coverage Diff              @@
##               main      #41      +/-   ##
============================================
- Coverage     92.73%   91.17%   -1.56%     
  Complexity       94       94              
============================================
  Files            20       20              
  Lines           234      238       +4     
  Branches          6        6              
============================================
  Hits            217      217              
- Misses           12       16       +4     
  Partials          5        5              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `91.17% <0.00%> (-1.56%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/open-feature/java-sdk/pull/41?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | Coverage Δ | |
|---|---|---|
| [...ava/dev/openfeature/javasdk/EvaluationContext.java](https://codecov.io/gh/open-feature/java-sdk/pull/41/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#diff-c3JjL21haW4vamF2YS9kZXYvb3BlbmZlYXR1cmUvamF2YXNkay9FdmFsdWF0aW9uQ29udGV4dC5qYXZh) | `80.85% <0.00%> (-7.53%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature)

beeme1mr: Relates to https://github.com/open-feature/spec/pull/120
thomaspoignant: For the sake of simplicity will it be easier to have a method that returns everything at one rather than having to call 4 iterators to fetch all the data?

In the other SDKs you have access to everything with a single call:
- https://github.com/open-feature/golang-sdk/blob/32d6d647ae8ba8eed0c049aff70b48d6d00a35af/pkg/openfeature/evaluation_context.go#L7
- https://github.com/open-feature/python-sdk/blob/6b05df774e9e2a167b38db884d01c6ffd59acc86/open_feature/evaluation_context/evaluation_context.py#L4
- https://github.com/open-feature/node-sdk/blob/6a0dfb1f9e61149ef9cffba924751b640dd18f54/src/types.ts#L13

This question is more general in the java SDK it-self.
Should we store all the keys in a single `Map` or should we have multiple `Map` as it is right now?


rgrassian-split: > For the sake of simplicity will it be easier to have a method that returns everything at one rather than having to call 4 iterators to fetch all the data?
> 
> In the other SDKs you have access to everything with a single call:
> 
> * https://github.com/open-feature/golang-sdk/blob/32d6d647ae8ba8eed0c049aff70b48d6d00a35af/pkg/openfeature/evaluation_context.go#L7
> * https://github.com/open-feature/python-sdk/blob/6b05df774e9e2a167b38db884d01c6ffd59acc86/open_feature/evaluation_context/evaluation_context.py#L4
> * https://github.com/open-feature/node-sdk/blob/6a0dfb1f9e61149ef9cffba924751b640dd18f54/src/types.ts#L13
> 
> This question is more general in the java SDK it-self. Should we store all the keys in a single `Map` or should we have multiple `Map` as it is right now?

I honestly agree and think we could store everything in one map, that also gets rid of the key collision issue you had pointed out. We could keep everything in a map from string -> string, serializing each value, and maybe do something like the map from key -> class I mentioned above when deserializing. I will leave this up to @justinabrahms though
rgrassian-split: @justinabrahms @thomaspoignant thoughts on making these return copies of the maps instead of iterator over entries? Would probably be way easier to work with

Patch:
diff --git a/src/main/java/dev/openfeature/javasdk/EvaluationContext.java b/src/main/java/dev/openfeature/javasdk/EvaluationContext.java
index f8ad671..336feac 100644
--- a/src/main/java/dev/openfeature/javasdk/EvaluationContext.java
+++ b/src/main/java/dev/openfeature/javasdk/EvaluationContext.java
@@ -39,6 +39,10 @@ public class EvaluationContext {
         return objMapper.readValue(val, klass);
     }
 
+    public Map<String, String> getStructureAttributes() {
+        return new HashMap<>(jsonAttributes);
+    }
+
     public void addStringAttribute(String key, String value) {
         stringAttributes.put(key, value);
     }
@@ -47,6 +51,10 @@ public class EvaluationContext {
         return stringAttributes.get(key);
     }
 
+    public Map<String, String> getStringAttributes() {
+        return new HashMap<>(stringAttributes);
+    }
+
     public void addIntegerAttribute(String key, Integer value) {
         integerAttributes.put(key, value);
     }
@@ -55,6 +63,10 @@ public class EvaluationContext {
         return integerAttributes.get(key);
     }
 
+    public Map<String, Integer> getIntegerAttributes() {
+        return new HashMap<>(integerAttributes);
+    }
+
     public Boolean getBooleanAttribute(String key) {
         return booleanAttributes.get(key);
     }
@@ -63,6 +75,10 @@ public class EvaluationContext {
         booleanAttributes.put(key, b);
     }
 
+    public Map<String, Boolean> getBooleanAttributes() {
+        return new HashMap<>(booleanAttributes);
+    }
+
     public void addDatetimeAttribute(String key, ZonedDateTime value) {
         this.stringAttributes.put(key, value.format(DateTimeFormatter.ISO_ZONED_DATE_TIME));
     }


Test patch:
diff --git a/src/test/java/dev/openfeature/javasdk/EvalContextTest.java b/src/test/java/dev/openfeature/javasdk/EvalContextTest.java
index 4efcc3c..85bc4e2 100644
--- a/src/test/java/dev/openfeature/javasdk/EvalContextTest.java
+++ b/src/test/java/dev/openfeature/javasdk/EvalContextTest.java
@@ -1,13 +1,19 @@
 package dev.openfeature.javasdk;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.jupiter.api.Test;
 
 import java.time.ZonedDateTime;
+import java.util.Map;
 
 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.junit.jupiter.api.Assertions.fail;
 
 public class EvalContextTest {
-    @Specification(number="3.1",
+    @Specification(number="3.1.1",
             text="The `evaluation context` structure **MUST** define an optional `targeting key` field of " +
                     "type string, identifying the subject of the flag evaluation.")
     @Test void requires_targeting_key() {
@@ -16,7 +22,7 @@ public class EvalContextTest {
         assertEquals("targeting-key", ec.getTargetingKey());
     }
 
-    @Specification(number="3.2", text="The evaluation context MUST support the inclusion of " +
+    @Specification(number="3.1.2", text="The evaluation context MUST support the inclusion of " +
             "custom fields, having keys of type `string`, and " +
             "values of type `boolean | string | number | datetime | structure`.")
     @Test void eval_context() {
@@ -36,7 +42,7 @@ public class EvalContextTest {
         assertEquals(dt, ec.getDatetimeAttribute("dt"));
     }
 
-    @Specification(number="3.2", text="The evaluation context MUST support the inclusion of " +
+    @Specification(number="3.1.2", text="The evaluation context MUST support the inclusion of " +
             "custom fields, having keys of type `string`, and " +
             "values of type `boolean | string | number | datetime | structure`.")
     @Test void eval_context__structure() {
@@ -59,4 +65,48 @@ public class EvalContextTest {
         assertEquals(2, nodeFromString.value);
         assertEquals(4, nodeFromString.left.value);
     }
+
+    @Specification(number="3.1.3", text="The evaluation context MUST support fetching the custom fields by key and " +
+            "also fetching all of the keys and values.")
+    @Test void fetch_all() {
+        EvaluationContext ec = new EvaluationContext();
+
+        ec.addStringAttribute("str", "test");
+        ec.addStringAttribute("str2", "test2");
+
+        ec.addBooleanAttribute("bool", true);
+        ec.addBooleanAttribute("bool2", false);
+
+        ec.addIntegerAttribute("int", 4);
+        ec.addIntegerAttribute("int2", 2);
+
+        ZonedDateTime dt = ZonedDateTime.now();
+        ec.addDatetimeAttribute("dt", dt);
+
+        Node<Integer> n1 = new Node<>();
+        n1.value = 4;
+        Node<Integer> n2 = new Node<>();
+        n2.value = 2;
+        n2.left = n1;
+        ec.addStructureAttribute("obj", n2);
+
+        Map<String, String> foundStr = ec.getStringAttributes();
+        assertEquals(ec.getStringAttribute("str"), foundStr.get("str"));
+        assertEquals(ec.getStringAttribute("str2"), foundStr.get("str2"));
+
+        Map<String, Boolean> foundBool = ec.getBooleanAttributes();
+        assertEquals(ec.getBooleanAttribute("bool"), foundBool.get("bool"));
+        assertEquals(ec.getBooleanAttribute("bool2"), foundBool.get("bool2"));
+
+        Map<String, Integer> foundInt = ec.getIntegerAttributes();
+        assertEquals(ec.getIntegerAttribute("int"), foundInt.get("int"));
+        assertEquals(ec.getIntegerAttribute("int2"), foundInt.get("int2"));
+
+        Map<String, String> foundObj = ec.getStructureAttributes();
+        try {
+            assertEquals(ec.getStructureAttribute("obj", Node.class), new ObjectMapper().readValue(foundObj.get("obj"), Node.class));
+        } catch (JsonProcessingException e) {
+            fail("Unexpected exception occurred: ", e);
+        }
+    }
 }
diff --git a/src/test/java/dev/openfeature/javasdk/HookSpecTest.java b/src/test/java/dev/openfeature/javasdk/HookSpecTest.java
index 9d38f03..8f0e8fe 100644
--- a/src/test/java/dev/openfeature/javasdk/HookSpecTest.java
+++ b/src/test/java/dev/openfeature/javasdk/HookSpecTest.java
@@ -411,7 +411,7 @@ public class HookSpecTest implements HookFixtures {
 
 
         Client client = getClient(null);
-        client.getBooleanValue("key", false, new EvaluationContext(),
+        client.getBooleanValue("key", false, ctx,
                 FlagEvaluationOptions.builder()
                         .hook(hook2)
                         .hook(hook)