Skip to content

Commit b7fb6fc

Browse files
leoromanovskydevflow.devflow-routing-intake
andauthored
fix(openfeature): allow null targeting key for static and rule-only flags (#10990)
fix(openfeature): defer null targeting key check to shard evaluation point - Remove early null targeting key guard at evaluate() entry point - Add deferred null check inside shard else-branch before matchesShard call - Static and rule-only flags can now evaluate with null targeting key - TARGETING_KEY_MISSING only returned when shard computation needs it test(openfeature): add 3 null targeting key test cases for DDEvaluatorTest - Update existing test: null TK on static flag now expects success (was incorrectly expecting TARGETING_KEY_MISSING error) - Add test: null TK on sharded flag expects TARGETING_KEY_MISSING error - Add test: null TK on rule-only flag (country attribute) expects success - Add createCountryRuleFlag() helper: rule matching on 'country' attribute with no shards, validates rule evaluation works without targeting key style: fix spotless formatting in DDEvaluatorTest fix: remove accidentally staged ffe-system-test-data submodule pointer Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent c55fb32 commit b7fb6fc

2 files changed

Lines changed: 45 additions & 4 deletions

File tree

products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,6 @@ public <T> ProviderEvaluation<T> evaluate(
8787
return error(defaultValue, ErrorCode.INVALID_CONTEXT);
8888
}
8989

90-
if (context.getTargetingKey() == null) {
91-
return error(defaultValue, ErrorCode.TARGETING_KEY_MISSING);
92-
}
93-
9490
final Flag flag = config.flags.get(key);
9591
if (flag == null) {
9692
return error(defaultValue, ErrorCode.FLAG_NOT_FOUND);
@@ -127,6 +123,9 @@ public <T> ProviderEvaluation<T> evaluate(
127123
return resolveVariant(
128124
target, key, defaultValue, flag, split.variationKey, allocation, context);
129125
} else {
126+
if (targetingKey == null) {
127+
return error(defaultValue, ErrorCode.TARGETING_KEY_MISSING);
128+
}
130129
// To match a split, subject must match ALL underlying shards
131130
boolean allShardsMatch = true;
132131
for (final Shard shard : split.shards) {

products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,23 @@ public void testFlattening(
210210

211211
private static List<TestCase<?>> evaluateTestCases() {
212212
return Arrays.asList(
213+
// OF spec 3.1.1: targeting key is optional; static flags must evaluate successfully without
214+
// it
213215
new TestCase<>("default")
214216
.flag("simple-string")
217+
// no .targetingKey() -- null by default
218+
.result(new Result<>("test-value").reason(TARGETING_MATCH.name()).variant("on")),
219+
// Null targeting key on sharded flag must return TARGETING_KEY_MISSING
220+
new TestCase<>("default")
221+
.flag("shard-flag")
222+
// no .targetingKey() -- null
215223
.result(new Result<>("default").reason(ERROR.name()).errorCode(TARGETING_KEY_MISSING)),
224+
// Null targeting key on rule-only flag (matching on non-id attribute) must succeed
225+
new TestCase<>("default")
226+
.flag("country-rule-flag")
227+
// no .targetingKey() -- null
228+
.context("country", "US")
229+
.result(new Result<>("us-value").reason(TARGETING_MATCH.name()).variant("us")),
216230
// OF.7: Empty string is a valid targeting key - evaluation should proceed as normal
217231
new TestCase<>("default")
218232
.flag("simple-string")
@@ -536,6 +550,7 @@ private ServerConfiguration createTestConfiguration() {
536550
flags.put("not-matches-false-flag", createNotMatchesFalseFlag());
537551
flags.put("not-one-of-false-flag", createNotOneOfFalseFlag());
538552
flags.put("null-context-values-flag", createNullContextValuesFlag());
553+
flags.put("country-rule-flag", createCountryRuleFlag());
539554
return new ServerConfiguration(null, null, null, flags);
540555
}
541556

@@ -1194,6 +1209,33 @@ private Flag createNullContextValuesFlag() {
11941209
"null-context-values-flag", true, ValueType.STRING, variants, singletonList(allocation));
11951210
}
11961211

1212+
private Flag createCountryRuleFlag() {
1213+
final Map<String, Variant> variants = new HashMap<>();
1214+
variants.put("us", new Variant("us", "us-value"));
1215+
variants.put("global", new Variant("global", "global-value"));
1216+
1217+
// Rule: country ONE_OF ["US"] -> us (no shards, so null targeting key is fine)
1218+
final List<String> usCountries = singletonList("US");
1219+
final List<ConditionConfiguration> usConditions =
1220+
singletonList(new ConditionConfiguration(ConditionOperator.ONE_OF, "country", usCountries));
1221+
final List<Rule> usRules = singletonList(new Rule(usConditions));
1222+
final List<Split> usSplits = singletonList(new Split(emptyList(), "us", null));
1223+
final Allocation usAllocation =
1224+
new Allocation("us-alloc", usRules, null, null, usSplits, false);
1225+
1226+
// Fallback allocation (no rules, no shards)
1227+
final List<Split> globalSplits = singletonList(new Split(emptyList(), "global", null));
1228+
final Allocation globalAllocation =
1229+
new Allocation("global-alloc", null, null, null, globalSplits, false);
1230+
1231+
return new Flag(
1232+
"country-rule-flag",
1233+
true,
1234+
ValueType.STRING,
1235+
variants,
1236+
asList(usAllocation, globalAllocation));
1237+
}
1238+
11971239
private static Map<String, Object> mapOf(final Object... props) {
11981240
final Map<String, Object> result = new HashMap<>(props.length << 1);
11991241
int index = 0;

0 commit comments

Comments
 (0)