Skip to content

Commit f09ae33

Browse files
greghuelsgreghuels
committed
Fix evaluator and parser for FFE isolation fixtures
- DDEvaluator: return DEFAULT for invalid (non-semver) flags instead of FLAG_NOT_FOUND, add POSIX character class normalization for regex matching, use DEFAULT reason for date-gated allocations with no rules - DDEvaluatorTest: add FlagMapAdapter with per-flag error isolation matching production parser, wire up INVALID_FLAGS_HOLDER - UniversalFlagConfigParser: add validateFlag for missing split shards, track invalid flags with INVALID_FLAG error type alongside semver - OpenFeatureProviderSmokeTest: null-safe buildLoggedAllocations for malformed allocation shapes
1 parent b3b7bca commit f09ae33

4 files changed

Lines changed: 148 additions & 18 deletions

File tree

‎dd-smoke-tests/openfeature/src/test/groovy/datadog/smoketest/springboot/OpenFeatureProviderSmokeTest.groovy‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,12 @@ class OpenFeatureProviderSmokeTest extends AbstractServerSmokeTest {
206206
private static Map<String, Boolean> buildLoggedAllocations(final Map<String, Object> config) {
207207
final logged = [:]
208208
(config.flags as Map<String, Object>).each { flag, definition ->
209-
(definition.allocations ?: []).each { allocation ->
210-
logged["${flag}\u0000${allocation.key}"] = allocation.doLog == true
209+
if (definition.allocations instanceof List) {
210+
definition.allocations.each { allocation ->
211+
if (allocation instanceof Map) {
212+
logged["${flag}\u0000${allocation.key}"] = allocation.doLog == true
213+
}
214+
}
211215
}
212216
}
213217
return logged

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

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -172,13 +172,19 @@ public <T> ProviderEvaluation<T> evaluate(
172172

173173
final Flag flag = config.flags.get(key);
174174
if (flag == null) {
175-
if (config.invalidFlags != null
176-
&& "invalid_semver_comparand".equals(config.invalidFlags.get(key))) {
177-
return error(
178-
defaultValue,
179-
ErrorCode.PARSE_ERROR,
180-
"invalid configuration for flag " + key,
181-
observeFullEvaluationData);
175+
if (config.invalidFlags != null && config.invalidFlags.containsKey(key)) {
176+
if ("invalid_semver_comparand".equals(config.invalidFlags.get(key))) {
177+
return error(
178+
defaultValue,
179+
ErrorCode.PARSE_ERROR,
180+
"invalid configuration for flag " + key,
181+
observeFullEvaluationData);
182+
}
183+
return ProviderEvaluation.<T>builder()
184+
.value(defaultValue)
185+
.reason(Reason.DEFAULT.name())
186+
.flagMetadata(consentMetadata(observeFullEvaluationData))
187+
.build();
182188
}
183189
return error(defaultValue, ErrorCode.FLAG_NOT_FOUND, null, observeFullEvaluationData);
184190
}
@@ -390,10 +396,20 @@ private static boolean evaluateCondition(
390396
private static boolean matchesRegex(final Object attributeValue, final Object conditionValue) {
391397
// PatternSyntaxException is intentionally not caught here so it propagates to evaluate(),
392398
// which maps it to ErrorCode.PARSE_ERROR.
393-
final Pattern pattern = Pattern.compile(String.valueOf(conditionValue));
399+
final Pattern pattern = Pattern.compile(normalizeRegex(String.valueOf(conditionValue)));
394400
return pattern.matcher(String.valueOf(attributeValue)).find();
395401
}
396402

403+
private static String normalizeRegex(final String regex) {
404+
return regex
405+
.replace("[:alnum:]", "\\p{Alnum}")
406+
.replace("[:alpha:]", "\\p{Alpha}")
407+
.replace("[:digit:]", "\\p{Digit}")
408+
.replace("[:lower:]", "\\p{Lower}")
409+
.replace("[:upper:]", "\\p{Upper}")
410+
.replace("[:space:]", "\\p{Space}");
411+
}
412+
397413
private static boolean isOneOf(final Object attributeValue, final Object conditionValue) {
398414
if (!(conditionValue instanceof Iterable)) {
399415
return false;
@@ -553,7 +569,9 @@ private static <T> ProviderEvaluation<T> resolveVariant(
553569
.reason(
554570
!isEmpty(allocation.rules)
555571
? Reason.TARGETING_MATCH.name()
556-
: !isEmpty(split.shards) ? Reason.SPLIT.name() : Reason.STATIC.name())
572+
: allocation.startAt != null || allocation.endAt != null
573+
? Reason.DEFAULT.name()
574+
: !isEmpty(split.shards) ? Reason.SPLIT.name() : Reason.STATIC.name())
557575
.variant(variant.key)
558576
.flagMetadata(metadataBuilder.build())
559577
.build();

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

Lines changed: 86 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,16 @@ public class DDEvaluatorTest {
6868

6969
private static final String CANONICAL_FIXTURE_PATH =
7070
"dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data";
71-
private static final Moshi MOSHI = new Moshi.Builder().add(Date.class, new DateAdapter()).build();
71+
private static final Moshi MOSHI =
72+
new Moshi.Builder().add(Date.class, new DateAdapter()).add(FlagMapAdapter.FACTORY).build();
7273
private static final JsonAdapter<ServerConfiguration> CONFIG_ADAPTER =
7374
MOSHI.adapter(ServerConfiguration.class);
7475
private static final Type FIXTURE_LIST_TYPE =
7576
Types.newParameterizedType(List.class, FixtureCase.class);
7677
private static final JsonAdapter<List<FixtureCase>> FIXTURE_LIST_ADAPTER =
7778
MOSHI.adapter(FIXTURE_LIST_TYPE);
79+
private static final ThreadLocal<Map<String, String>> INVALID_FLAGS_HOLDER =
80+
ThreadLocal.withInitial(HashMap::new);
7881

7982
@Test
8083
public void testInitializeSignalsApplicationProviderActivation() throws Exception {
@@ -850,10 +853,19 @@ private static ProviderEvaluation<?> evaluate(
850853
}
851854

852855
private static ServerConfiguration loadCanonicalConfiguration() throws IOException {
853-
final ServerConfiguration config =
854-
CONFIG_ADAPTER.fromJson(read(fixtureRoot().resolve("ufc-config.json")));
855-
validateAndCacheSemverComparands(config);
856-
return config;
856+
INVALID_FLAGS_HOLDER.get().clear();
857+
try {
858+
final ServerConfiguration config =
859+
CONFIG_ADAPTER.fromJson(read(fixtureRoot().resolve("ufc-config.json")));
860+
final Map<String, String> invalidFlags = new HashMap<>(INVALID_FLAGS_HOLDER.get());
861+
if (!invalidFlags.isEmpty()) {
862+
config.invalidFlags = invalidFlags;
863+
}
864+
validateAndCacheSemverComparands(config);
865+
return config;
866+
} finally {
867+
INVALID_FLAGS_HOLDER.get().clear();
868+
}
857869
}
858870

859871
/**
@@ -865,7 +877,8 @@ private static void validateAndCacheSemverComparands(final ServerConfiguration c
865877
if (config.flags == null) {
866878
return;
867879
}
868-
final Map<String, String> invalidFlags = new HashMap<>();
880+
final Map<String, String> invalidFlags =
881+
config.invalidFlags == null ? new HashMap<>() : new HashMap<>(config.invalidFlags);
869882
final Map<String, Flag> flagsToRemove = new HashMap<>();
870883
for (final Map.Entry<String, Flag> entry : config.flags.entrySet()) {
871884
final String flagKey = entry.getKey();
@@ -1035,6 +1048,73 @@ private static final class FixtureResult {
10351048
Map<String, Object> flagMetadata = emptyMap();
10361049
}
10371050

1051+
/** Reads the flags map with per-flag failure isolation, matching the production parser. */
1052+
private static final class FlagMapAdapter extends JsonAdapter<Map<String, Flag>> {
1053+
private static final Type FLAGS_TYPE =
1054+
Types.newParameterizedType(Map.class, String.class, Flag.class);
1055+
1056+
private static final JsonAdapter.Factory FACTORY =
1057+
(type, annotations, moshi) -> {
1058+
if (!annotations.isEmpty() || !Types.equals(type, FLAGS_TYPE)) {
1059+
return null;
1060+
}
1061+
return new FlagMapAdapter(moshi.adapter(Flag.class));
1062+
};
1063+
1064+
private final JsonAdapter<Flag> flagAdapter;
1065+
1066+
private FlagMapAdapter(final JsonAdapter<Flag> flagAdapter) {
1067+
this.flagAdapter = flagAdapter;
1068+
}
1069+
1070+
@Override
1071+
public Map<String, Flag> fromJson(final JsonReader reader) throws IOException {
1072+
if (reader.peek() == JsonReader.Token.NULL) {
1073+
return reader.nextNull();
1074+
}
1075+
final Map<String, Flag> flags = new HashMap<>();
1076+
reader.beginObject();
1077+
while (reader.hasNext()) {
1078+
final String flagKey = reader.nextName();
1079+
final Object rawFlag = reader.readJsonValue();
1080+
try {
1081+
final Flag flag = flagAdapter.fromJsonValue(rawFlag);
1082+
if (flag != null) {
1083+
validateFlag(flagKey, flag);
1084+
flags.put(flagKey, flag);
1085+
}
1086+
} catch (JsonDataException | IllegalArgumentException ignored) {
1087+
INVALID_FLAGS_HOLDER.get().put(flagKey, "invalid_flag");
1088+
// A malformed flag must not prevent valid flags in the same configuration from loading.
1089+
}
1090+
}
1091+
reader.endObject();
1092+
return flags;
1093+
}
1094+
1095+
private static void validateFlag(final String flagKey, final Flag flag) {
1096+
if (flag.allocations == null) {
1097+
return;
1098+
}
1099+
for (final Allocation allocation : flag.allocations) {
1100+
if (allocation == null || allocation.splits == null) {
1101+
continue;
1102+
}
1103+
for (final Split split : allocation.splits) {
1104+
if (split != null && split.shards == null) {
1105+
throw new IllegalArgumentException(
1106+
"flag \"" + flagKey + "\" contains a split with missing shards");
1107+
}
1108+
}
1109+
}
1110+
}
1111+
1112+
@Override
1113+
public void toJson(final JsonWriter writer, final Map<String, Flag> value) {
1114+
throw new UnsupportedOperationException("Reading only adapter");
1115+
}
1116+
}
1117+
10381118
private static final class DateAdapter extends JsonAdapter<Date> {
10391119
@Override
10401120
public Date fromJson(final JsonReader reader) throws IOException {

‎products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/UniversalFlagConfigParser.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ final class UniversalFlagConfigParser implements ConfigurationDeserializer<Serve
3535

3636
private static final Logger LOGGER = LoggerFactory.getLogger(UniversalFlagConfigParser.class);
3737

38+
static final String INVALID_FLAG = "invalid_flag";
3839
static final String INVALID_SEMVER_COMPARAND = "invalid_semver_comparand";
3940

4041
/**
@@ -88,6 +89,25 @@ private static void requireEndOfDocument(final JsonReader reader) throws IOExcep
8889
reader.peek();
8990
}
9091

92+
/** Validates the required nested UFC fields and SemVer comparands for a flag. */
93+
private static void validateFlag(final String flagKey, final Flag flag) {
94+
if (flag.allocations == null) {
95+
return;
96+
}
97+
for (final Allocation allocation : flag.allocations) {
98+
if (allocation == null || allocation.splits == null) {
99+
continue;
100+
}
101+
for (final Split split : allocation.splits) {
102+
if (split != null && split.shards == null) {
103+
throw new InvalidFlagException(
104+
"flag \"" + flagKey + "\" contains a split with missing shards");
105+
}
106+
}
107+
}
108+
validateAndCacheSemverComparands(flagKey, flag);
109+
}
110+
91111
/**
92112
* Validates and caches SemVer comparands for all SEMVER_* conditions in a flag. Throws {@link
93113
* InvalidSemverComparandException} if any condition has an invalid or non-string comparand value.
@@ -150,6 +170,13 @@ private static void validateAndCacheSemverComparands(final String flagKey, final
150170
}
151171
}
152172

173+
/** Thrown when a flag has an invalid UFC shape. */
174+
static final class InvalidFlagException extends IllegalArgumentException {
175+
InvalidFlagException(final String message) {
176+
super(message);
177+
}
178+
}
179+
153180
/** Thrown when a SEMVER_* condition has an invalid or non-string comparand value. */
154181
static final class InvalidSemverComparandException extends IllegalArgumentException {
155182
InvalidSemverComparandException(final String message) {
@@ -197,10 +224,11 @@ public Map<String, Flag> fromJson(@Nonnull final JsonReader reader) throws IOExc
197224
try {
198225
final Flag flag = flagAdapter.fromJsonValue(rawFlag);
199226
if (flag != null) {
200-
validateAndCacheSemverComparands(flagKey, flag);
227+
validateFlag(flagKey, flag);
201228
flags.put(flagKey, flag);
202229
}
203230
} catch (JsonDataException | IllegalArgumentException error) {
231+
INVALID_FLAGS_HOLDER.get().put(flagKey, INVALID_FLAG);
204232
if (error instanceof InvalidSemverComparandException) {
205233
INVALID_FLAGS_HOLDER.get().put(flagKey, INVALID_SEMVER_COMPARAND);
206234
}

0 commit comments

Comments
 (0)