Repository: open-feature/java-sdk
Pull request #70 (2022-09-13 21:56:32)

fix: isList check in Value checks type of list
A list of any type would be able to be created due to the new object constructor. This makes it so that the isList check only returns true if the list contains objects of type Value (or is empty).

codecov-commenter: # [Codecov](https://codecov.io/gh/open-feature/java-sdk/pull/70?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) Report
> Merging [#70](https://codecov.io/gh/open-feature/java-sdk/pull/70?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) (3915b69) into [main](https://codecov.io/gh/open-feature/java-sdk/commit/00af2f88f84951f685fa279cfe3fad5f413cedce?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) (00af2f8) will **increase** coverage by `0.05%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 3915b69 differs from pull request most recent head d13a310. Consider uploading reports for the commit d13a310 to get more accurate results

```diff
@@             Coverage Diff              @@
##               main      #70      +/-   ##
============================================
+ Coverage     91.27%   91.32%   +0.05%     
- Complexity      159      162       +3     
============================================
  Files            19       19              
  Lines           344      346       +2     
  Branches         14       16       +2     
============================================
+ Hits            314      316       +2     
  Misses           21       21              
  Partials          9        9              
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `91.32% <100.00%> (+0.05%)` | :arrow_up: |

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/70?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature) | Coverage Δ | |
|---|---|---|
| [src/main/java/dev/openfeature/javasdk/Value.java](https://codecov.io/gh/open-feature/java-sdk/pull/70/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature#diff-c3JjL21haW4vamF2YS9kZXYvb3BlbmZlYXR1cmUvamF2YXNkay9WYWx1ZS5qYXZh) | `88.40% <100.00%> (+0.34%)` | :arrow_up: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-feature)

rgrassian-split: @justinabrahms @toddbaert please check out this PR, as I was updating the Split Provider I noticed it would be possible to set a list of any time due to the new constructor. This change should make it so that only lists containing Values are allowed to be created through that constructor

Patch:
diff --git a/src/main/java/dev/openfeature/javasdk/Value.java b/src/main/java/dev/openfeature/javasdk/Value.java
index 859348a..abadd60 100644
--- a/src/main/java/dev/openfeature/javasdk/Value.java
+++ b/src/main/java/dev/openfeature/javasdk/Value.java
@@ -125,7 +125,9 @@ public class Value {
      * @return boolean
      */
     public boolean isList() {
-        return this.innerObject instanceof List;
+        return this.innerObject instanceof List
+            && (((List) this.innerObject).isEmpty()
+            || ((List) this.innerObject).get(0) instanceof Value);
     }
 
     /** 


Test patch:
diff --git a/src/test/java/dev/openfeature/javasdk/ValueTest.java b/src/test/java/dev/openfeature/javasdk/ValueTest.java
index 57cd6aa..1221032 100644
--- a/src/test/java/dev/openfeature/javasdk/ValueTest.java
+++ b/src/test/java/dev/openfeature/javasdk/ValueTest.java
@@ -110,4 +110,28 @@ public class ValueTest {
         assertTrue(value.isList());
         assertEquals(ITEM_VALUE, value.asList().get(0).asString());
     }
+
+    @Test public void listMustBeOfValues() {
+        String item = "item";
+        List<String> list = new ArrayList<>();
+        list.add(item);
+        try {
+            new Value((Object) list);
+            fail("Should fail due to creation of list of non-values.");
+        } catch (InstantiationException e) {
+            assertEquals("Invalid value type: class java.util.ArrayList", e.getMessage());
+        }
+    }
+
+    @Test public void emptyListAllowed() {
+        List<String> list = new ArrayList<>();
+        try {
+            Value value = new Value((Object) list);
+            assertTrue(value.isList());
+            List<Value> values = value.asList();
+            assertTrue(values.isEmpty());
+        } catch (Exception e) {
+            fail("Unexpected exception occurred.", e);
+        }
+    }
 }