Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps")
@ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory")
@IncludeTags("in-process")
@ExcludeTags({"unixsocket", "fractional-v1", "deprecated", "operator-errors"})
@ExcludeTags({"unixsocket", "fractional-v1", "deprecated"})
@Testcontainers
public class RunInProcessTest {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,16 @@
@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps")
@ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory")
@IncludeTags({"rpc"})
@ExcludeTags({"unixsocket", "fractional-v1", "deprecated", "operator-errors"})
@ExcludeTags({
"unixsocket",
"fractional-v1",
"deprecated",
"operator-errors",
"semver-edge-cases",
"evaluator-refs-whitespace",
"non-existent-evaluator-ref",
"fractional-single-entry"
})
@Testcontainers
public class RunRpcTest {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,28 @@ public ContextSteps(State state) {
public void a_context_containing_a_key_with_type_and_with_value(String key, String type, String value)
throws ClassNotFoundException, InstantiationException {
Map<String, Value> map = state.context.asMap();
map.put(key, new Value(value));
Value typedValue;
switch (type) {
case "Integer":
long longVal = Long.parseLong(value);
if (longVal >= Integer.MIN_VALUE && longVal <= Integer.MAX_VALUE) {
typedValue = new Value((int) longVal);
} else {
// value exceeds int range; store as string to preserve precision
typedValue = new Value(value);
}
break;
case "Float":
typedValue = new Value(Double.parseDouble(value));
break;
case "Boolean":
typedValue = new Value(Boolean.parseBoolean(value));
break;
default:
typedValue = new Value(value);
break;
}
map.put(key, typedValue);
state.context = new MutableContext(state.context.getTargetingKey(), map);
}

Expand Down
2 changes: 1 addition & 1 deletion providers/flagd/test-harness
2 changes: 1 addition & 1 deletion tools/flagd-api-testkit/test-harness
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;

Expand All @@ -28,7 +27,6 @@ public class FlagParser {
private static final String FLAG_KEY = "flags";
private static final String METADATA_KEY = "metadata";
private static final String EVALUATOR_KEY = "$evaluators";
private static final String REPLACER_FORMAT = "\"\\$ref\":(\\s)*\"%s\"";
private static final ObjectMapper MAPPER = new ObjectMapper();
private static JsonSchema SCHEMA_VALIDATOR;

Expand Down Expand Up @@ -116,32 +114,23 @@ private static Map<String, Object> parseMetadata(TreeNode metadataNode) throws J

private static String transposeEvaluators(final String configuration) throws IOException {
try (JsonParser parser = MAPPER.createParser(configuration)) {
final Map<String, Pattern> patternMap = new HashMap<>();
final TreeNode treeNode = parser.readValueAsTree();
final TreeNode evaluators = treeNode.get(EVALUATOR_KEY);

if (evaluators == null || evaluators.size() == 0) {
return configuration;
}

String replacedConfigurations = configuration;
// round-trip to normalize whitespace so we can use plain string matching
String replacedConfigurations = MAPPER.writeValueAsString(MAPPER.readTree(configuration));
final Iterator<String> evalFields = evaluators.fieldNames();

while (evalFields.hasNext()) {
final String evalName = evalFields.next();
// first replace outmost brackets
final String evaluator = evaluators.get(evalName).toString();
final String replacer = evaluator.substring(1, evaluator.length() - 1);
final String refPattern = "{\"$ref\":\"" + evalName + "\"}";

final String replacePattern = String.format(REPLACER_FORMAT, evalName);

// then derive pattern
final Pattern regReplace =
patternMap.computeIfAbsent(replacePattern, s -> Pattern.compile(replacePattern));

// finally replace all references
replacedConfigurations =
regReplace.matcher(replacedConfigurations).replaceAll(replacer);
replacedConfigurations = replacedConfigurations.replace(refPattern, evaluator);
}

return replacedConfigurations;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import io.github.jamsesso.jsonlogic.evaluator.expressions.PreEvaluatedArgumentsExpression;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -25,6 +24,7 @@ public String key() {
}

@Override
@SuppressWarnings("unchecked") // json-logic-java's PreEvaluatedArgumentsExpression uses raw List
public Object evaluate(List arguments, Object data, String jsonPath) throws JsonLogicEvaluationException {
if (arguments.size() < 1) {
return null;
Expand All @@ -36,13 +36,23 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json
Object arg1 = arguments.get(0);

final String bucketBy;
final Object[] distributions;
final List<Object> distributions;

if (arg1 instanceof String) {
// first arg is a String, use for bucketing
bucketBy = (String) arg1;
Object[] source = arguments.toArray();
distributions = Arrays.copyOfRange(source, 1, source.length);
List<Object> remaining = arguments.subList(1, arguments.size());

// json-logic pre-evaluation flattens a single-entry fractional
// e.g. [["single",1]] becomes ["single", 1]; detect and re-wrap
if (!remaining.isEmpty() && !(remaining.get(0) instanceof List)) {
List<Object> wrapped = new ArrayList<>(remaining.size() + 1);
wrapped.add(arg1);
wrapped.addAll(remaining);
distributions = List.of(wrapped);
} else {
distributions = remaining;
}
} else {
// fallback to targeting key if present
if (properties.getTargetingKey() == null) {
Expand All @@ -51,7 +61,7 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to check for flattening here for single-entry fractionals? Or does that not happen here because the pre-evaluation doesn't "get confused" by 2 different list entry types?

bucketBy = properties.getFlagKey() + properties.getTargetingKey();
distributions = arguments.toArray();
distributions = arguments;
}

final List<FractionProperty> propertyList = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,25 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json
return null;
}

for (int i = 0; i < 3; i++) {
if (!(arguments.get(i) instanceof String)) {
log.debug("Invalid argument type. Require Strings");
return null;
}
// arg 1 and arg 3 must be strings or numbers (coerced to string)
// arg 2 must be a string (operator)
final String arg1Str = coerceToString(arguments.get(0));
final String arg3Str = coerceToString(arguments.get(2));

if (arg1Str == null || arg3Str == null) {
log.debug("Arguments 1 and 3 must be strings or numbers");
return null;
}

if (!(arguments.get(1) instanceof String)) {
log.debug("Argument 2 (operator) must be a string");
return null;
}

// arg 1 should be a SemVer
final Semver arg1Parsed;

if ((arg1Parsed = Semver.parse((String) arguments.get(0))) == null) {
if ((arg1Parsed = normalizeVersion(arg1Str)) == null) {
log.debug("Argument one is not a valid SemVer");
return null;
}
Expand All @@ -75,14 +83,58 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json
// arg 3 should be a SemVer
final Semver arg3Parsed;

if ((arg3Parsed = Semver.parse((String) arguments.get(2))) == null) {
if ((arg3Parsed = normalizeVersion(arg3Str)) == null) {
log.debug("Argument three is not a valid SemVer");
return null;
}

return compare(arg2Parsed, arg1Parsed, arg3Parsed, jsonPath);
}

/**
* Coerce a value to a string representation suitable for semver parsing.
*/
private static String coerceToString(Object value) {
if (value instanceof String) {
return (String) value;
}
if (value instanceof Number) {
Number num = (Number) value;
double dub = num.doubleValue();
if (dub == Math.floor(dub) && !Double.isInfinite(dub)) {
return String.valueOf(num.longValue());
}
return String.valueOf(dub);
}
return null;
}

/**
* Parse a semver string, handling v-prefix (case-insensitive) and partial versions.
*/
private static Semver normalizeVersion(String version) {
// strip v/V prefix
String stripped = version;
if (stripped.startsWith("v") || stripped.startsWith("V")) {
stripped = stripped.substring(1);
}

// try strict parse first
Semver result = Semver.parse(stripped);
if (result != null) {
return result;
}

// fall back to coerce for partial versions (fewer than 2 dots)
// do not coerce strings that have too many parts (e.g. "2.0.0.0")
long dotCount = stripped.chars().filter(c -> c == '.').count();
if (dotCount < 2) {
return Semver.coerce(stripped);
}

return null;
}

private static boolean compare(final String operator, final Semver arg1, final Semver arg2, final String jsonPath)
throws JsonLogicEvaluationException {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* configuration. Registered as an {@link dev.openfeature.contrib.tools.flagd.api.testkit.EvaluatorFactory}
* via {@code META-INF/services}.
*/
@ExcludeTags({"fractional-v1"})
@ExcludeTags({"fractional-v1", "evaluator-refs-whitespace", "non-existent-evaluator-ref"})
public class FlagdCoreEvaluatorTest extends AbstractEvaluatorTest {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static dev.openfeature.contrib.tools.flagd.core.targeting.Operator.FLAG_KEY;
import static dev.openfeature.contrib.tools.flagd.core.targeting.Operator.TARGET_KEY;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Named.named;
import static org.junit.jupiter.params.provider.Arguments.arguments;

Expand All @@ -19,6 +20,7 @@
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.converter.ArgumentConversionException;
import org.junit.jupiter.params.converter.ConvertWith;
Expand Down Expand Up @@ -80,4 +82,39 @@ static class TestData {
@JsonProperty("rule")
List<Object> rule;
}

@Test
void missingBucketKeyReturnsNull() throws JsonLogicEvaluationException {
// no targeting key in data; bucket key var resolves to null
Fractional fractional = new Fractional();

Map<String, Object> data = new HashMap<>();
Map<String, String> flagdProperties = new HashMap<>();
flagdProperties.put(FLAG_KEY, "flagA");
data.put(FLAGD_PROPS_KEY, flagdProperties);
// no TARGET_KEY set

List<Object> rule = List.of(
// bucket key is a null var result (simulated by being a non-string, non-list)
List.of("one", 50), List.of("two", 50));

// targeting key is null, so fractional falls back to flagKey + targetingKey
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit-pick, do you mean "bucketing key is null"?

// but targetingKey is null, so it should return null
assertNull(fractional.evaluate(rule, data, "path"));
}

@Test
void zeroWeightsReturnsNull() throws JsonLogicEvaluationException {
Fractional fractional = new Fractional();

Map<String, Object> data = new HashMap<>();
data.put(TARGET_KEY, "user");
Map<String, String> flagdProperties = new HashMap<>();
flagdProperties.put(FLAG_KEY, "flagA");
data.put(FLAGD_PROPS_KEY, flagdProperties);

List<Object> rule = List.of(List.of("one", 0), List.of("two", 0));

assertNull(fractional.evaluate(rule, data, "path"));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package dev.openfeature.contrib.tools.flagd.core.targeting;

import static org.assertj.core.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

Expand Down Expand Up @@ -45,10 +46,22 @@ void testValidCases(List<String> args) throws JsonLogicEvaluationException {

static Stream<Arguments> invalidInputs() {
return Stream.of(
Arguments.of(Arrays.asList("1.2.3", "=", 1.2)),
Arguments.of(Arrays.asList("1.2", "=", "1.2.3")),
// invalid operator
Arguments.of(Arrays.asList("1.2.3", "*", "1.2.3")),
Arguments.of(Arrays.asList("1.2.3", "=", "1.2")));
// wrong argument count (too few)
Arguments.of(Arrays.asList("1.0.0", "=")),
// wrong argument count (too many)
Arguments.of(Arrays.asList("1.0.0", "=", "1.0.0", "extra")));
}

static Stream<Arguments> coercedInputs() {
return Stream.of(
// numeric third arg coerced to semver
Arguments.of(Arrays.asList("1.2.3", "=", 1.2), false),
// partial version coerced
Arguments.of(Arrays.asList("1.2", "=", "1.2.3"), false),
Arguments.of(Arrays.asList("1.2.3", "=", "1.2"), false),
Arguments.of(Arrays.asList("1.2.0", "=", "1.2"), true));
}

@ParameterizedTest
Expand All @@ -60,4 +73,17 @@ void testInvalidCases(List args) throws JsonLogicEvaluationException {
// then
assertNull(semVer.evaluate(args, new Object(), "jsonPath"));
}

@ParameterizedTest
@MethodSource("coercedInputs")
void testCoercedCases(List args, boolean expected) throws JsonLogicEvaluationException {
// given
final SemVer semVer = new SemVer();

// when
Object result = semVer.evaluate(args, new Object(), "jsonPath");

// then
assertEquals(expected, result);
}
}
Loading
Loading