fix: Do not throw null reference exception accessing a missing item. #300

kinyoklion · 2023-02-16T02:40:09Z

This PR

I noticed when implementing a provider that accessing a missing item from an ImmutableContext throws an exception instead of returning null. I do not believe this to be the desired behavior.

How to test

I think the added unit test should cover it.

Signed-off-by: Ryan Lamb <[email protected]>

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

codecov · 2023-02-16T02:44:02Z

Codecov Report

Merging #300 (2a8a549) into main (7459eaa) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               main     #300   +/-   ##
=========================================
  Coverage     91.80%   91.80%           
- Complexity      212      213    +1     
=========================================
  Files            23       23           
  Lines           488      488           
  Branches         30       31    +1     
=========================================
  Hits            448      448           
  Misses           24       24           
  Partials         16       16

Flag	Coverage Δ
unittests	`91.80% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...n/java/dev/openfeature/sdk/ImmutableStructure.java	`76.19% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

toddbaert · 2023-02-16T19:03:51Z

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

@@ -50,7 +50,7 @@ public Set<String> keySet() {
    @Override
    public Value getValue(String key) {
        Value value = this.attributes.get(key);
-        return value.clone();
+        return value != null ? value.clone() : null;


Thanks for finding/fixing/testing this. I don't think we have a similar issue in MutableStructure (the older implementation) because we don't clone the retrieved value.