Skip to content

Commit 8b5eef6

Browse files
Kavindu-DodanKavindu-Dodan
committed
improve targeting key handling
Signed-off-by: Kavindu Dodanduwa <[email protected]>
1 parent 911464f commit 8b5eef6

File tree

6 files changed

+74
-73
lines changed

6 files changed

+74
-73
lines changed

Diff for: ‎src/main/java/dev/openfeature/sdk/EvaluationContext.java

+3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
*/
77
@SuppressWarnings("PMD.BeanMembersShouldSerialize")
88
public interface EvaluationContext extends Structure {
9+
10+
String TARGETING_KEY = "targetingKey";
11+
912
String getTargetingKey();
1013

1114
/**

Diff for: ‎src/main/java/dev/openfeature/sdk/ImmutableContext.java

+18-22
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
package dev.openfeature.sdk;
22

3-
import java.util.HashMap;
4-
import java.util.Map;
5-
6-
import lombok.Getter;
73
import lombok.ToString;
84
import lombok.experimental.Delegate;
95

6+
import java.util.HashMap;
7+
import java.util.Map;
8+
109
/**
1110
* The EvaluationContext is a container for arbitrary contextual data
1211
* that can be used as a basis for dynamic evaluation.
@@ -17,16 +16,14 @@
1716
@SuppressWarnings("PMD.BeanMembersShouldSerialize")
1817
public final class ImmutableContext implements EvaluationContext {
1918

20-
@Getter
21-
private final String targetingKey;
2219
@Delegate
2320
private final Structure structure;
2421

2522
/**
2623
* Create an immutable context with an empty targeting_key and attributes provided.
2724
*/
2825
public ImmutableContext() {
29-
this("", new HashMap<>());
26+
this(new HashMap<>());
3027
}
3128

3229
/**
@@ -54,8 +51,18 @@ public ImmutableContext(Map<String, Value> attributes) {
5451
* @param attributes evaluation context attributes
5552
*/
5653
public ImmutableContext(String targetingKey, Map<String, Value> attributes) {
54+
if (targetingKey != null && !targetingKey.trim().isEmpty()) {
55+
attributes.put(TARGETING_KEY, new Value(targetingKey));
56+
}
5757
this.structure = new ImmutableStructure(attributes);
58-
this.targetingKey = targetingKey;
58+
}
59+
60+
/**
61+
* Retrieve targetingKey from the context.
62+
*/
63+
@Override
64+
public String getTargetingKey() {
65+
return this.getValue(TARGETING_KEY).asString();
5966
}
6067

6168
/**
@@ -67,21 +74,10 @@ public ImmutableContext(String targetingKey, Map<String, Value> attributes) {
6774
@Override
6875
public EvaluationContext merge(EvaluationContext overridingContext) {
6976
if (overridingContext == null) {
70-
return new ImmutableContext(this.targetingKey, this.asMap());
77+
return new ImmutableContext(this.asMap());
7178
}
72-
String newTargetingKey = "";
73-
if (this.getTargetingKey() != null && !this.getTargetingKey().trim().equals("")) {
74-
newTargetingKey = this.getTargetingKey();
75-
}
76-
77-
if (overridingContext.getTargetingKey() != null && !overridingContext.getTargetingKey().trim().equals("")) {
78-
newTargetingKey = overridingContext.getTargetingKey();
79-
}
80-
81-
Map<String, Value> merged = this.merge(m -> new ImmutableStructure(m),
82-
this.asMap(),
83-
overridingContext.asMap());
8479

85-
return new ImmutableContext(newTargetingKey, merged);
80+
return new ImmutableContext(
81+
this.merge(ImmutableStructure::new, this.asMap(), overridingContext.asMap()));
8682
}
8783
}

Diff for: ‎src/main/java/dev/openfeature/sdk/MutableContext.java

+43-41
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,51 @@
11
package dev.openfeature.sdk;
22

3-
import java.time.Instant;
4-
import java.util.List;
5-
import java.util.Map;
6-
73
import lombok.EqualsAndHashCode;
8-
import lombok.Getter;
9-
import lombok.Setter;
104
import lombok.ToString;
115
import lombok.experimental.Delegate;
126

7+
import java.time.Instant;
8+
import java.util.HashMap;
9+
import java.util.List;
10+
import java.util.Map;
11+
1312
/**
1413
* The EvaluationContext is a container for arbitrary contextual data
1514
* that can be used as a basis for dynamic evaluation.
16-
* The MutableContext is an EvaluationContext implementation which is not threadsafe, and whose attributes can
15+
* The MutableContext is an EvaluationContext implementation which is not threadsafe, and whose attributes can
1716
* be modified after instantiation.
1817
*/
1918
@ToString
2019
@EqualsAndHashCode
2120
@SuppressWarnings("PMD.BeanMembersShouldSerialize")
2221
public class MutableContext implements EvaluationContext {
2322

24-
@Setter() @Getter private String targetingKey;
2523
@Delegate(excludes = HideDelegateAddMethods.class) private final MutableStructure structure;
2624

2725
public MutableContext() {
28-
this.structure = new MutableStructure();
29-
this.targetingKey = "";
26+
this(new HashMap<>());
3027
}
3128

3229
public MutableContext(String targetingKey) {
33-
this();
34-
this.targetingKey = targetingKey;
30+
this(targetingKey, new HashMap<>());
3531
}
3632

3733
public MutableContext(Map<String, Value> attributes) {
38-
this.structure = new MutableStructure(attributes);
39-
this.targetingKey = "";
34+
this("", attributes);
4035
}
4136

37+
/**
38+
* Create a mutable context with given targetingKey and attributes provided. TargetingKey should be non-null
39+
* and non-empty to be accepted.
40+
*
41+
* @param targetingKey targeting key
42+
* @param attributes evaluation context attributes
43+
*/
4244
public MutableContext(String targetingKey, Map<String, Value> attributes) {
43-
this(attributes);
44-
this.targetingKey = targetingKey;
45+
if (targetingKey != null && !targetingKey.trim().isEmpty()) {
46+
attributes.put(TARGETING_KEY, new Value(targetingKey));
47+
}
48+
this.structure = new MutableStructure(attributes);
4549
}
4650

4751
// 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) {
8185
}
8286

8387
/**
84-
* Merges this EvaluationContext objects with the second overriding the this in
85-
* case of conflict.
88+
* Override or set targeting key for this mutable context. Value should be non-null and non-empty to be accepted.
89+
*/
90+
public void setTargetingKey(String targetingKey) {
91+
if (targetingKey != null && !targetingKey.trim().isEmpty()) {
92+
this.add(TARGETING_KEY, targetingKey);
93+
}
94+
}
95+
96+
97+
/**
98+
* Retrieve targetingKey from the context.
99+
*/
100+
@Override
101+
public String getTargetingKey() {
102+
return this.getValue(TARGETING_KEY).asString();
103+
}
104+
105+
/**
106+
* Merges this EvaluationContext objects with the second overriding the in case of conflict.
86107
*
87108
* @param overridingContext overriding context
88109
* @return resulting merged context
89110
*/
90111
@Override
91112
public EvaluationContext merge(EvaluationContext overridingContext) {
92113
if (overridingContext == null) {
93-
return new MutableContext(this.targetingKey, this.asMap());
94-
}
95-
96-
Map<String, Value> merged = this.merge(map -> new MutableStructure(map),
97-
this.asMap(),
98-
overridingContext.asMap());
99-
100-
String newTargetingKey = "";
101-
102-
if (this.getTargetingKey() != null && !this.getTargetingKey().trim().equals("")) {
103-
newTargetingKey = this.getTargetingKey();
104-
}
105-
106-
if (overridingContext.getTargetingKey() != null && !overridingContext.getTargetingKey().trim().equals("")) {
107-
newTargetingKey = overridingContext.getTargetingKey();
108-
}
109-
110-
EvaluationContext ec = null;
111-
if (newTargetingKey != null && !newTargetingKey.trim().equals("")) {
112-
ec = new MutableContext(newTargetingKey, merged);
113-
} else {
114-
ec = new MutableContext(merged);
114+
return new MutableContext(this.asMap());
115115
}
116116

117-
return ec;
117+
Map<String, Value> merged = this.merge(
118+
MutableStructure::new, this.asMap(), overridingContext.asMap());
119+
return new MutableContext(merged);
118120
}
119121

120122
/**

Diff for: ‎src/main/java/dev/openfeature/sdk/Structure.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,8 @@ default Object convertValue(Value value) {
113113
default <T extends Structure> Map<String, Value> merge(Function<Map<String, Value>, Structure> newStructure,
114114
Map<String, Value> base,
115115
Map<String, Value> overriding) {
116-
Map<String, Value> merged = new HashMap<>();
117116

118-
merged.putAll(base);
117+
final Map<String, Value> merged = new HashMap<>(base);
119118
for (Entry<String, Value> overridingEntry : overriding.entrySet()) {
120119
String key = overridingEntry.getKey();
121120
if (overridingEntry.getValue().isStructure() && merged.containsKey(key) && merged.get(key).isStructure()) {

Diff for: ‎src/test/java/dev/openfeature/sdk/EvalContextTest.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
package dev.openfeature.sdk;
22

3-
import static org.junit.jupiter.api.Assertions.assertEquals;
3+
import org.junit.jupiter.api.Test;
44

55
import java.time.Instant;
66
import java.time.temporal.ChronoUnit;
7-
import java.time.temporal.TemporalUnit;
87
import java.util.ArrayList;
98
import java.util.HashMap;
109
import java.util.List;
1110
import java.util.Map;
1211

13-
import io.cucumber.java.hu.Ha;
14-
import org.junit.jupiter.api.Test;
12+
import static dev.openfeature.sdk.EvaluationContext.TARGETING_KEY;
13+
import static org.junit.jupiter.api.Assertions.assertEquals;
1514

1615
public class EvalContextTest {
1716
@Specification(number="3.1.1",
@@ -184,7 +183,7 @@ public class EvalContextTest {
184183

185184
ctx2.setTargetingKey(" ");
186185
ctxMerged = ctx1.merge(ctx2);
187-
assertEquals(key1, ctxMerged.getTargetingKey());
186+
assertEquals(key2, ctxMerged.getTargetingKey());
188187
}
189188

190189
@Test void asObjectMap() {
@@ -214,6 +213,7 @@ public class EvalContextTest {
214213

215214

216215
Map<String, Object> want = new HashMap<>();
216+
want.put(TARGETING_KEY, key1);
217217
want.put("stringItem", "stringValue");
218218
want.put("boolItem", false);
219219
want.put("integerItem", 1);

Diff for: ‎src/test/java/dev/openfeature/sdk/ImmutableContextTest.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import java.util.HashMap;
77

8+
import static dev.openfeature.sdk.EvaluationContext.TARGETING_KEY;
89
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
910
import static org.junit.jupiter.api.Assertions.assertEquals;
1011
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -20,7 +21,7 @@ void shouldCreateCopyOfAttributesForImmutableContext() {
2021
attributes.put("key2", new Value("val2"));
2122
EvaluationContext ctx = new ImmutableContext("targeting key", attributes);
2223
attributes.put("key3", new Value("val3"));
23-
assertArrayEquals(new Object[]{"key1", "key2"}, ctx.keySet().toArray());
24+
assertArrayEquals(new Object[]{"key1", "key2", TARGETING_KEY}, ctx.keySet().toArray());
2425
}
2526

2627
@DisplayName("targeting key should be changed from the overriding context")
@@ -56,7 +57,7 @@ void mergeShouldReturnAllTheValuesFromTheContextWhenOverridingContextIsNull() {
5657
EvaluationContext ctx = new ImmutableContext("targeting_key", attributes);
5758
EvaluationContext merge = ctx.merge(null);
5859
assertEquals("targeting_key", merge.getTargetingKey());
59-
assertArrayEquals(new Object[]{"key1", "key2"}, merge.keySet().toArray());
60+
assertArrayEquals(new Object[]{"key1", "key2", TARGETING_KEY}, merge.keySet().toArray());
6061
}
6162

6263
@DisplayName("Merge should retain subkeys from the existing context when the overriding context has the same targeting key")
@@ -77,7 +78,7 @@ void mergeShouldRetainItsSubkeysWhenOverridingContextHasTheSameKey() {
7778
EvaluationContext overriding = new ImmutableContext("targeting_key", overridingAttributes);
7879
EvaluationContext merge = ctx.merge(overriding);
7980
assertEquals("targeting_key", merge.getTargetingKey());
80-
assertArrayEquals(new Object[]{"key1", "key2"}, merge.keySet().toArray());
81+
assertArrayEquals(new Object[]{"key1", "key2", TARGETING_KEY}, merge.keySet().toArray());
8182

8283
Value key1 = merge.getValue("key1");
8384
assertTrue(key1.isStructure());

0 commit comments

Comments
 (0)