Skip to content

Commit ff6bb1d

Browse files
committed
Allow empty string as a valid targeting key
The OpenFeature spec defines targeting key as "a string" with no restriction on empty strings. The TARGETING_KEY_MISSING error code is for when a key is "not provided" (null), not when it's an empty string. Previously, MutableContext rejected empty strings by checking `!targetingKey.trim().isEmpty()`, treating them the same as null. This made it impossible to distinguish between "not provided" and "explicitly set to empty string". Changes: - Remove isEmpty check from constructor and setTargetingKey() - Empty string is now stored and returned by getTargetingKey() - Null still results in no targeting key being set - Updated tests to reflect new behavior - Added explicit tests for empty string targeting key Signed-off-by: Leo Romanovsky <leo.romanovsky@datadoghq.com>
1 parent 8426a9c commit ff6bb1d

2 files changed

Lines changed: 24 additions & 7 deletions

File tree

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,14 @@ public MutableContext(Map<String, Value> attributes) {
3636

3737
/**
3838
* Create a mutable context with given targetingKey and attributes provided. TargetingKey should be non-null
39-
* and non-empty to be accepted.
39+
* to be accepted. Empty string is a valid targeting key value.
4040
*
4141
* @param targetingKey targeting key
4242
* @param attributes evaluation context attributes
4343
*/
4444
public MutableContext(String targetingKey, Map<String, Value> attributes) {
4545
this.structure = new MutableStructure(new HashMap<>(attributes));
46-
if (targetingKey != null && !targetingKey.trim().isEmpty()) {
46+
if (targetingKey != null) {
4747
this.structure.attributes.put(TARGETING_KEY, new Value(targetingKey));
4848
}
4949
}
@@ -85,10 +85,11 @@ public MutableContext add(String key, List<Value> value) {
8585
}
8686

8787
/**
88-
* Override or set targeting key for this mutable context. Value should be non-null and non-empty to be accepted.
88+
* Override or set targeting key for this mutable context. Value should be non-null to be accepted.
89+
* Empty string is a valid targeting key value.
8990
*/
9091
public MutableContext setTargetingKey(String targetingKey) {
91-
if (targetingKey != null && !targetingKey.trim().isEmpty()) {
92+
if (targetingKey != null) {
9293
this.add(TARGETING_KEY, targetingKey);
9394
}
9495
return this;

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,17 @@ void shouldChangeTargetingKeyFromOverridingContext() {
4040
assertEquals("overriding_key", merge.getTargetingKey());
4141
}
4242

43-
@DisplayName("targeting key should not changed from the overriding context if missing")
43+
@DisplayName("targeting key should be changed from the overriding context even if empty string")
4444
@Test
45-
void shouldRetainTargetingKeyWhenOverridingContextTargetingKeyValueIsEmpty() {
45+
void shouldOverrideTargetingKeyWhenOverridingContextTargetingKeyIsEmptyString() {
4646
HashMap<String, Value> attributes = new HashMap<>();
4747
attributes.put("key1", new Value("val1"));
4848
attributes.put("key2", new Value("val2"));
4949
EvaluationContext ctx = new MutableContext("targeting_key", attributes);
5050
EvaluationContext overriding = new MutableContext("");
5151
EvaluationContext merge = ctx.merge(overriding);
52-
assertEquals("targeting_key", merge.getTargetingKey());
52+
// Empty string is a valid targeting key and should override
53+
assertEquals("", merge.getTargetingKey());
5354
}
5455

5556
@DisplayName("missing targeting key should return null")
@@ -59,6 +60,21 @@ void missingTargetingKeyShould() {
5960
assertEquals(null, ctx.getTargetingKey());
6061
}
6162

63+
@DisplayName("empty string is a valid targeting key via constructor")
64+
@Test
65+
void emptyStringIsValidTargetingKeyViaConstructor() {
66+
EvaluationContext ctx = new MutableContext("");
67+
assertEquals("", ctx.getTargetingKey());
68+
}
69+
70+
@DisplayName("empty string is a valid targeting key via setter")
71+
@Test
72+
void emptyStringIsValidTargetingKeyViaSetter() {
73+
MutableContext ctx = new MutableContext();
74+
ctx.setTargetingKey("");
75+
assertEquals("", ctx.getTargetingKey());
76+
}
77+
6278
@DisplayName("Merge should retain all the attributes from the existing context when overriding context is null")
6379
@Test
6480
void mergeShouldReturnAllTheValuesFromTheContextWhenOverridingContextIsNull() {

0 commit comments

Comments
 (0)