From ff6bb1d3dd2d4ef1d568c591b1ebaded102b5bdd Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 16 Jan 2026 09:16:23 -0500 Subject: [PATCH 1/7] 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 --- .../dev/openfeature/sdk/MutableContext.java | 9 ++++---- .../openfeature/sdk/MutableContextTest.java | 22 ++++++++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/MutableContext.java b/src/main/java/dev/openfeature/sdk/MutableContext.java index 2ad68c98a..5e684a8b3 100644 --- a/src/main/java/dev/openfeature/sdk/MutableContext.java +++ b/src/main/java/dev/openfeature/sdk/MutableContext.java @@ -36,14 +36,14 @@ public MutableContext(Map attributes) { /** * Create a mutable context with given targetingKey and attributes provided. TargetingKey should be non-null - * and non-empty to be accepted. + * to be accepted. Empty string is a valid targeting key value. * * @param targetingKey targeting key * @param attributes evaluation context attributes */ public MutableContext(String targetingKey, Map attributes) { this.structure = new MutableStructure(new HashMap<>(attributes)); - if (targetingKey != null && !targetingKey.trim().isEmpty()) { + if (targetingKey != null) { this.structure.attributes.put(TARGETING_KEY, new Value(targetingKey)); } } @@ -85,10 +85,11 @@ public MutableContext add(String key, List value) { } /** - * Override or set targeting key for this mutable context. Value should be non-null and non-empty to be accepted. + * Override or set targeting key for this mutable context. Value should be non-null to be accepted. + * Empty string is a valid targeting key value. */ public MutableContext setTargetingKey(String targetingKey) { - if (targetingKey != null && !targetingKey.trim().isEmpty()) { + if (targetingKey != null) { this.add(TARGETING_KEY, targetingKey); } return this; diff --git a/src/test/java/dev/openfeature/sdk/MutableContextTest.java b/src/test/java/dev/openfeature/sdk/MutableContextTest.java index b81cbccb1..54c7e5bab 100644 --- a/src/test/java/dev/openfeature/sdk/MutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/MutableContextTest.java @@ -40,16 +40,17 @@ void shouldChangeTargetingKeyFromOverridingContext() { assertEquals("overriding_key", merge.getTargetingKey()); } - @DisplayName("targeting key should not changed from the overriding context if missing") + @DisplayName("targeting key should be changed from the overriding context even if empty string") @Test - void shouldRetainTargetingKeyWhenOverridingContextTargetingKeyValueIsEmpty() { + void shouldOverrideTargetingKeyWhenOverridingContextTargetingKeyIsEmptyString() { HashMap attributes = new HashMap<>(); attributes.put("key1", new Value("val1")); attributes.put("key2", new Value("val2")); EvaluationContext ctx = new MutableContext("targeting_key", attributes); EvaluationContext overriding = new MutableContext(""); EvaluationContext merge = ctx.merge(overriding); - assertEquals("targeting_key", merge.getTargetingKey()); + // Empty string is a valid targeting key and should override + assertEquals("", merge.getTargetingKey()); } @DisplayName("missing targeting key should return null") @@ -59,6 +60,21 @@ void missingTargetingKeyShould() { assertEquals(null, ctx.getTargetingKey()); } + @DisplayName("empty string is a valid targeting key via constructor") + @Test + void emptyStringIsValidTargetingKeyViaConstructor() { + EvaluationContext ctx = new MutableContext(""); + assertEquals("", ctx.getTargetingKey()); + } + + @DisplayName("empty string is a valid targeting key via setter") + @Test + void emptyStringIsValidTargetingKeyViaSetter() { + MutableContext ctx = new MutableContext(); + ctx.setTargetingKey(""); + assertEquals("", ctx.getTargetingKey()); + } + @DisplayName("Merge should retain all the attributes from the existing context when overriding context is null") @Test void mergeShouldReturnAllTheValuesFromTheContextWhenOverridingContextIsNull() { From aeae09af200262aa16cf32b7bae7410a58b84bcb Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 16 Jan 2026 09:25:12 -0500 Subject: [PATCH 2/7] Also update ImmutableContext and add whitespace tests - Update ImmutableContext to allow empty string targeting keys, matching the MutableContext behavior for consistency - Add tests for whitespace-only targeting keys to explicitly document this is now permitted - Update merge behavior tests to reflect that empty string targeting keys now override existing values Signed-off-by: Leo Romanovsky --- .../dev/openfeature/sdk/ImmutableContext.java | 3 ++- .../openfeature/sdk/ImmutableContextTest.java | 21 ++++++++++++++++--- .../openfeature/sdk/MutableContextTest.java | 15 +++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/ImmutableContext.java b/src/main/java/dev/openfeature/sdk/ImmutableContext.java index accebe6fc..150f9c171 100644 --- a/src/main/java/dev/openfeature/sdk/ImmutableContext.java +++ b/src/main/java/dev/openfeature/sdk/ImmutableContext.java @@ -55,12 +55,13 @@ public ImmutableContext(Map attributes) { /** * Create an immutable context with given targetingKey and attributes provided. + * Empty string is a valid targeting key value. * * @param targetingKey targeting key * @param attributes evaluation context attributes */ public ImmutableContext(String targetingKey, Map attributes) { - if (targetingKey != null && !targetingKey.isBlank()) { + if (targetingKey != null) { this.structure = new ImmutableStructure(targetingKey, attributes); } else { this.structure = new ImmutableStructure(attributes); diff --git a/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java b/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java index 2852be536..7267b3080 100644 --- a/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java @@ -54,16 +54,17 @@ void shouldChangeTargetingKeyFromOverridingContext() { assertEquals("overriding_key", merge.getTargetingKey()); } - @DisplayName("targeting key should not be changed from the overriding context if missing") + @DisplayName("targeting key should be changed from the overriding context even if empty string") @Test - void shouldRetainTargetingKeyWhenOverridingContextTargetingKeyValueIsEmpty() { + void shouldOverrideTargetingKeyWhenOverridingContextTargetingKeyIsEmptyString() { HashMap attributes = new HashMap<>(); attributes.put("key1", new Value("val1")); attributes.put("key2", new Value("val2")); EvaluationContext ctx = new ImmutableContext("targeting_key", attributes); EvaluationContext overriding = new ImmutableContext(""); EvaluationContext merge = ctx.merge(overriding); - assertEquals("targeting_key", merge.getTargetingKey()); + // Empty string is a valid targeting key and should override + assertEquals("", merge.getTargetingKey()); } @DisplayName("missing targeting key should return null") @@ -73,6 +74,20 @@ void missingTargetingKeyShould() { assertNull(ctx.getTargetingKey()); } + @DisplayName("empty string is a valid targeting key") + @Test + void emptyStringIsValidTargetingKey() { + EvaluationContext ctx = new ImmutableContext(""); + assertEquals("", ctx.getTargetingKey()); + } + + @DisplayName("whitespace-only string is a valid targeting key") + @Test + void whitespaceOnlyStringIsValidTargetingKey() { + EvaluationContext ctx = new ImmutableContext(" "); + assertEquals(" ", ctx.getTargetingKey()); + } + @DisplayName("Merge should retain all the attributes from the existing context when overriding context is null") @Test void mergeShouldReturnAllTheValuesFromTheContextWhenOverridingContextIsNull() { diff --git a/src/test/java/dev/openfeature/sdk/MutableContextTest.java b/src/test/java/dev/openfeature/sdk/MutableContextTest.java index 54c7e5bab..38305d0ef 100644 --- a/src/test/java/dev/openfeature/sdk/MutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/MutableContextTest.java @@ -75,6 +75,21 @@ void emptyStringIsValidTargetingKeyViaSetter() { assertEquals("", ctx.getTargetingKey()); } + @DisplayName("whitespace-only string is a valid targeting key via constructor") + @Test + void whitespaceOnlyStringIsValidTargetingKeyViaConstructor() { + EvaluationContext ctx = new MutableContext(" "); + assertEquals(" ", ctx.getTargetingKey()); + } + + @DisplayName("whitespace-only string is a valid targeting key via setter") + @Test + void whitespaceOnlyStringIsValidTargetingKeyViaSetter() { + MutableContext ctx = new MutableContext(); + ctx.setTargetingKey(" "); + assertEquals(" ", ctx.getTargetingKey()); + } + @DisplayName("Merge should retain all the attributes from the existing context when overriding context is null") @Test void mergeShouldReturnAllTheValuesFromTheContextWhenOverridingContextIsNull() { From 6ba08e0219710ef5eced17c958d4c15317c9e031 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 16 Jan 2026 09:25:57 -0500 Subject: [PATCH 3/7] Combine empty and whitespace setter tests per review feedback Signed-off-by: Leo Romanovsky --- .../dev/openfeature/sdk/MutableContextTest.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/MutableContextTest.java b/src/test/java/dev/openfeature/sdk/MutableContextTest.java index 38305d0ef..3f788ddb0 100644 --- a/src/test/java/dev/openfeature/sdk/MutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/MutableContextTest.java @@ -67,12 +67,16 @@ void emptyStringIsValidTargetingKeyViaConstructor() { assertEquals("", ctx.getTargetingKey()); } - @DisplayName("empty string is a valid targeting key via setter") + @DisplayName("empty and whitespace-only strings are valid targeting keys via setter") @Test - void emptyStringIsValidTargetingKeyViaSetter() { + void emptyAndWhitespaceAreValidTargetingKeysViaSetter() { MutableContext ctx = new MutableContext(); + ctx.setTargetingKey(""); assertEquals("", ctx.getTargetingKey()); + + ctx.setTargetingKey(" "); + assertEquals(" ", ctx.getTargetingKey()); } @DisplayName("whitespace-only string is a valid targeting key via constructor") @@ -82,14 +86,6 @@ void whitespaceOnlyStringIsValidTargetingKeyViaConstructor() { assertEquals(" ", ctx.getTargetingKey()); } - @DisplayName("whitespace-only string is a valid targeting key via setter") - @Test - void whitespaceOnlyStringIsValidTargetingKeyViaSetter() { - MutableContext ctx = new MutableContext(); - ctx.setTargetingKey(" "); - assertEquals(" ", ctx.getTargetingKey()); - } - @DisplayName("Merge should retain all the attributes from the existing context when overriding context is null") @Test void mergeShouldReturnAllTheValuesFromTheContextWhenOverridingContextIsNull() { From 4f528e0b7b5b30587435da914a98d9d59aa7d132 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 16 Jan 2026 09:29:54 -0500 Subject: [PATCH 4/7] Fix EvalContextTest merge tests for whitespace targeting keys Signed-off-by: Leo Romanovsky --- src/test/java/dev/openfeature/sdk/EvalContextTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/java/dev/openfeature/sdk/EvalContextTest.java b/src/test/java/dev/openfeature/sdk/EvalContextTest.java index 0f910b00e..a4f9fcd20 100644 --- a/src/test/java/dev/openfeature/sdk/EvalContextTest.java +++ b/src/test/java/dev/openfeature/sdk/EvalContextTest.java @@ -173,9 +173,10 @@ void Immutable_context_merge_targeting_key() { ctxMerged = ctx1.merge(ctx2); assertEquals(key2, ctxMerged.getTargetingKey()); + // Whitespace is now a valid targeting key and should override ctx2 = new ImmutableContext(" ", new HashMap<>()); ctxMerged = ctx1.merge(ctx2); - assertEquals(key1, ctxMerged.getTargetingKey()); + assertEquals(" ", ctxMerged.getTargetingKey()); } @Test @@ -200,9 +201,10 @@ void merge_targeting_key() { ctxMerged = ctx1.merge(ctx2); assertEquals(key2, ctxMerged.getTargetingKey()); + // Whitespace is now a valid targeting key and should override ctx2.setTargetingKey(" "); ctxMerged = ctx1.merge(ctx2); - assertEquals(key2, ctxMerged.getTargetingKey()); + assertEquals(" ", ctxMerged.getTargetingKey()); } @Test From afb2f31bba2bef1ecefc612314f4ffaa939533d6 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 16 Jan 2026 10:12:22 -0500 Subject: [PATCH 5/7] Add test for setTargetingKey(null) to improve coverage Signed-off-by: Leo Romanovsky --- .../java/dev/openfeature/sdk/MutableContextTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/java/dev/openfeature/sdk/MutableContextTest.java b/src/test/java/dev/openfeature/sdk/MutableContextTest.java index 3f788ddb0..0cd0e3267 100644 --- a/src/test/java/dev/openfeature/sdk/MutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/MutableContextTest.java @@ -60,6 +60,15 @@ void missingTargetingKeyShould() { assertEquals(null, ctx.getTargetingKey()); } + @DisplayName("setTargetingKey with null should not set targeting key") + @Test + void setTargetingKeyWithNullShouldNotSet() { + MutableContext ctx = new MutableContext("original"); + ctx.setTargetingKey(null); + // null should not override the existing targeting key + assertEquals("original", ctx.getTargetingKey()); + } + @DisplayName("empty string is a valid targeting key via constructor") @Test void emptyStringIsValidTargetingKeyViaConstructor() { From 80bd62189735373fb5c0fd3c7413a14cb1251a6e Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 19 Jan 2026 10:48:53 -0500 Subject: [PATCH 6/7] Add nullability test for ImmutableContext constructor Per review feedback, add test to verify that passing null to the ImmutableContext constructor results in no targeting key being set. Signed-off-by: Leo Romanovsky --- .../java/dev/openfeature/sdk/ImmutableContextTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java b/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java index 7267b3080..cf8cdd22c 100644 --- a/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java @@ -74,6 +74,13 @@ void missingTargetingKeyShould() { assertNull(ctx.getTargetingKey()); } + @DisplayName("null targeting key in constructor should result in no targeting key") + @Test + void nullTargetingKeyInConstructorShouldResultInNoTargetingKey() { + EvaluationContext ctx = new ImmutableContext(null); + assertNull(ctx.getTargetingKey()); + } + @DisplayName("empty string is a valid targeting key") @Test void emptyStringIsValidTargetingKey() { From 0ae6db98c4f381c0f2f762911b6e734859cedba9 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Mon, 19 Jan 2026 11:12:41 -0500 Subject: [PATCH 7/7] Fix ambiguous constructor call in ImmutableContext test Cast null to String to disambiguate between ImmutableContext(String) and ImmutableContext(Map) constructors. Signed-off-by: Leo Romanovsky --- src/test/java/dev/openfeature/sdk/ImmutableContextTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java b/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java index cf8cdd22c..7d8210be4 100644 --- a/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java +++ b/src/test/java/dev/openfeature/sdk/ImmutableContextTest.java @@ -77,7 +77,7 @@ void missingTargetingKeyShould() { @DisplayName("null targeting key in constructor should result in no targeting key") @Test void nullTargetingKeyInConstructorShouldResultInNoTargetingKey() { - EvaluationContext ctx = new ImmutableContext(null); + EvaluationContext ctx = new ImmutableContext((String) null); assertNull(ctx.getTargetingKey()); }