fix: improve targetingKey handling in the context (#805)

improve targeting key handling

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>

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

@@ -6,6 +6,9 @@
  */
 @SuppressWarnings("PMD.BeanMembersShouldSerialize")
 public interface EvaluationContext extends Structure {
+
+    String TARGETING_KEY = "targetingKey";
+
     String getTargetingKey();
 
     /**

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

@@ -1,12 +1,11 @@
 package dev.openfeature.sdk;
 
-import java.util.HashMap;
-import java.util.Map;
-
-import lombok.Getter;
 import lombok.ToString;
 import lombok.experimental.Delegate;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * The EvaluationContext is a container for arbitrary contextual data
  * that can be used as a basis for dynamic evaluation.
@@ -17,16 +16,14 @@
 @SuppressWarnings("PMD.BeanMembersShouldSerialize")
 public final class ImmutableContext implements EvaluationContext {
 
-    @Getter
-    private final String targetingKey;
     @Delegate
     private final Structure structure;
 
     /**
      * Create an immutable context with an empty targeting_key and attributes provided.
      */
     public ImmutableContext() {
-        this("", new HashMap<>());
+        this(new HashMap<>());
     }
 
     /**
@@ -54,8 +51,18 @@ public ImmutableContext(Map<String, Value> attributes) {
      * @param attributes   evaluation context attributes
      */
     public ImmutableContext(String targetingKey, Map<String, Value> attributes) {
+        if (targetingKey != null && !targetingKey.trim().isEmpty()) {
+            attributes.put(TARGETING_KEY, new Value(targetingKey));
+        }
         this.structure = new ImmutableStructure(attributes);
-        this.targetingKey = targetingKey;
+    }
+
+    /**
+     * Retrieve targetingKey from the context.
+     */
+    @Override
+    public String getTargetingKey() {
+        return this.getValue(TARGETING_KEY).asString();
     }
 
     /**
@@ -67,21 +74,10 @@ public ImmutableContext(String targetingKey, Map<String, Value> attributes) {
     @Override
     public EvaluationContext merge(EvaluationContext overridingContext) {
         if (overridingContext == null) {
-            return new ImmutableContext(this.targetingKey, this.asMap());
+            return new ImmutableContext(this.asMap());
         }
-        String newTargetingKey = "";
-        if (this.getTargetingKey() != null && !this.getTargetingKey().trim().equals("")) {
-            newTargetingKey = this.getTargetingKey();
-        }
-
-        if (overridingContext.getTargetingKey() != null && !overridingContext.getTargetingKey().trim().equals("")) {
-            newTargetingKey = overridingContext.getTargetingKey();
-        }
-
-        Map<String, Value> merged = this.merge(m -> new ImmutableStructure(m),
-                                               this.asMap(),
-                                               overridingContext.asMap());
 
-        return new ImmutableContext(newTargetingKey, merged);
+        return new ImmutableContext(
+                this.merge(ImmutableStructure::new, this.asMap(), overridingContext.asMap()));
     }
 }

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

@@ -1,47 +1,51 @@
 package dev.openfeature.sdk;
 
-import java.time.Instant;
-import java.util.List;
-import java.util.Map;
-
 import lombok.EqualsAndHashCode;
-import lombok.Getter;
-import lombok.Setter;
 import lombok.ToString;
 import lombok.experimental.Delegate;
 
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 /**
  * The EvaluationContext is a container for arbitrary contextual data
  * that can be used as a basis for dynamic evaluation.
- * The MutableContext is an EvaluationContext implementation which is not threadsafe, and whose attributes can 
+ * The MutableContext is an EvaluationContext implementation which is not threadsafe, and whose attributes can
  * be modified after instantiation.
  */
 @ToString
 @EqualsAndHashCode
 @SuppressWarnings("PMD.BeanMembersShouldSerialize")
 public class MutableContext implements EvaluationContext {
 
-    @Setter() @Getter private String targetingKey;
     @Delegate(excludes = HideDelegateAddMethods.class) private final MutableStructure structure;
 
     public MutableContext() {
-        this.structure = new MutableStructure();
-        this.targetingKey = "";
+        this(new HashMap<>());
     }
 
     public MutableContext(String targetingKey) {
-        this();
-        this.targetingKey = targetingKey;
+        this(targetingKey, new HashMap<>());
     }
 
     public MutableContext(Map<String, Value> attributes) {
-        this.structure = new MutableStructure(attributes);
-        this.targetingKey = "";
+        this("", attributes);
     }
 
+    /**
+     * Create a mutable context with given targetingKey and attributes provided. TargetingKey should be non-null
+     * and non-empty to be accepted.
+     *
+     * @param targetingKey targeting key
+     * @param attributes   evaluation context attributes
+     */
     public MutableContext(String targetingKey, Map<String, Value> attributes) {
-        this(attributes);
-        this.targetingKey = targetingKey;
+        if (targetingKey != null && !targetingKey.trim().isEmpty()) {
+            attributes.put(TARGETING_KEY, new Value(targetingKey));
+        }
+        this.structure = new MutableStructure(attributes);
     }
 
     // override @Delegate methods so that we can use "add" methods and still return MutableContext, not Structure
@@ -81,40 +85,38 @@ public MutableContext add(String key, List<Value> value) {
     }
 
     /**
-     * Merges this EvaluationContext objects with the second overriding the this in
-     * case of conflict.
+     * Override or set targeting key for this mutable context. Value should be non-null and non-empty to be accepted.
+     */
+    public void setTargetingKey(String targetingKey) {
+        if (targetingKey != null && !targetingKey.trim().isEmpty()) {
+            this.add(TARGETING_KEY, targetingKey);
+        }
+    }
+
+
+    /**
+     * Retrieve targetingKey from the context.
+     */
+    @Override
+    public String getTargetingKey() {
+        return this.getValue(TARGETING_KEY).asString();
+    }
+
+    /**
+     * Merges this EvaluationContext objects with the second overriding the in case of conflict.
      *
      * @param overridingContext overriding context
      * @return resulting merged context
      */
     @Override
     public EvaluationContext merge(EvaluationContext overridingContext) {
         if (overridingContext == null) {
-            return new MutableContext(this.targetingKey, this.asMap());
-        }
-
-        Map<String, Value> merged = this.merge(map -> new MutableStructure(map),
-                                               this.asMap(),
-                                               overridingContext.asMap());
-
-        String newTargetingKey = "";
-
-        if (this.getTargetingKey() != null && !this.getTargetingKey().trim().equals("")) {
-            newTargetingKey = this.getTargetingKey();
-        }
-
-        if (overridingContext.getTargetingKey() != null && !overridingContext.getTargetingKey().trim().equals("")) {
-            newTargetingKey = overridingContext.getTargetingKey();
-        }
-
-        EvaluationContext ec = null;
-        if (newTargetingKey != null && !newTargetingKey.trim().equals("")) {
-            ec = new MutableContext(newTargetingKey, merged);
-        } else {
-            ec = new MutableContext(merged);
+            return new MutableContext(this.asMap());
         }
 
-        return ec;
+        Map<String, Value> merged = this.merge(
+                MutableStructure::new, this.asMap(), overridingContext.asMap());
+        return new MutableContext(merged);
     }
 
     /**

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

@@ -113,9 +113,8 @@ default Object convertValue(Value value) {
     default <T extends Structure> Map<String, Value> merge(Function<Map<String, Value>, Structure> newStructure,
                                                            Map<String, Value> base,
                                                            Map<String, Value> overriding) {
-        Map<String, Value> merged = new HashMap<>();
 
-        merged.putAll(base);
+        final Map<String, Value> merged = new HashMap<>(base);
         for (Entry<String, Value> overridingEntry : overriding.entrySet()) {
             String key = overridingEntry.getKey();
             if (overridingEntry.getValue().isStructure() && merged.containsKey(key) && merged.get(key).isStructure()) {

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

@@ -1,17 +1,16 @@
 package dev.openfeature.sdk;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import org.junit.jupiter.api.Test;
 
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
-import java.time.temporal.TemporalUnit;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import io.cucumber.java.hu.Ha;
-import org.junit.jupiter.api.Test;
+import static dev.openfeature.sdk.EvaluationContext.TARGETING_KEY;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 public class EvalContextTest {
     @Specification(number="3.1.1",
@@ -184,7 +183,7 @@ public class EvalContextTest {
 
         ctx2.setTargetingKey("  ");
         ctxMerged = ctx1.merge(ctx2);
-        assertEquals(key1, ctxMerged.getTargetingKey());
+        assertEquals(key2, ctxMerged.getTargetingKey());
     }
 
     @Test void asObjectMap() {
@@ -214,6 +213,7 @@ public class EvalContextTest {
 
 
         Map<String, Object> want = new HashMap<>();
+        want.put(TARGETING_KEY, key1);
         want.put("stringItem", "stringValue");
         want.put("boolItem", false);
         want.put("integerItem", 1);

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

@@ -5,6 +5,7 @@
 
 import java.util.HashMap;
 
+import static dev.openfeature.sdk.EvaluationContext.TARGETING_KEY;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -20,7 +21,7 @@ void shouldCreateCopyOfAttributesForImmutableContext() {
         attributes.put("key2", new Value("val2"));
         EvaluationContext ctx = new ImmutableContext("targeting key", attributes);
         attributes.put("key3", new Value("val3"));
-        assertArrayEquals(new Object[]{"key1", "key2"}, ctx.keySet().toArray());
+        assertArrayEquals(new Object[]{"key1", "key2", TARGETING_KEY}, ctx.keySet().toArray());
     }
 
     @DisplayName("targeting key should be changed from the overriding context")
@@ -56,7 +57,7 @@ void mergeShouldReturnAllTheValuesFromTheContextWhenOverridingContextIsNull() {
         EvaluationContext ctx = new ImmutableContext("targeting_key", attributes);
         EvaluationContext merge = ctx.merge(null);
         assertEquals("targeting_key", merge.getTargetingKey());
-        assertArrayEquals(new Object[]{"key1", "key2"}, merge.keySet().toArray());
+        assertArrayEquals(new Object[]{"key1", "key2", TARGETING_KEY}, merge.keySet().toArray());
     }
 
     @DisplayName("Merge should retain subkeys from the existing context when the overriding context has the same targeting key")
@@ -77,7 +78,7 @@ void mergeShouldRetainItsSubkeysWhenOverridingContextHasTheSameKey() {
         EvaluationContext overriding = new ImmutableContext("targeting_key", overridingAttributes);
         EvaluationContext merge = ctx.merge(overriding);
         assertEquals("targeting_key", merge.getTargetingKey());
-        assertArrayEquals(new Object[]{"key1", "key2"}, merge.keySet().toArray());
+        assertArrayEquals(new Object[]{"key1", "key2", TARGETING_KEY}, merge.keySet().toArray());
 
         Value key1 = merge.getValue("key1");
         assertTrue(key1.isStructure());