Skip to content
Draft
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 @@ -36,13 +36,27 @@ 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);
Object[] remaining = Arrays.copyOfRange(source, 1, source.length);

// json-logic pre-evaluation flattens a single-entry fractional
// e.g. [["single",1]] becomes ["single", 1]; detect and re-wrap
if (remaining.length > 0 && !(remaining[0] instanceof List)) {
List<Object> wrapped = new ArrayList<>();
wrapped.add(arg1);
for (Object r : remaining) {
wrapped.add(r);
}
distributions = new ArrayList<>();
distributions.add(wrapped);
} else {
distributions = Arrays.asList(remaining);
}
Comment on lines 44 to +59
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The current implementation performs multiple array copies and manual list building, which can be inefficient as this code is executed during every flag evaluation. Using List.subList and List.addAll is more efficient and idiomatic for this purpose.

            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 = Arrays.asList((Object) wrapped);
            } else {
                distributions = remaining;
            }

} else {
// fallback to targeting key if present
if (properties.getTargetingKey() == null) {
Expand All @@ -51,7 +65,7 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json
}

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
// 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,35 @@ public void invalidNumberOfArgs() throws JsonLogicEvaluationException {
// given
final StringComp operator = new StringComp(StringComp.Type.STARTS_WITH);

// when
// when - too many args
Object result = operator.evaluate(Arrays.asList("123", "12", "1"), new Object(), "jsonPath");

// then
assertThat(result).isNull();
}

@Test
public void tooFewArgs() throws JsonLogicEvaluationException {
// given
final StringComp startsWith = new StringComp(StringComp.Type.STARTS_WITH);
final StringComp endsWith = new StringComp(StringComp.Type.ENDS_WITH);

// when/then - single arg returns null
assertThat(startsWith.evaluate(Arrays.asList("abc"), new Object(), "jsonPath"))
.isNull();
assertThat(endsWith.evaluate(Arrays.asList("xyz"), new Object(), "jsonPath"))
.isNull();
}

@Test
public void endsWithNonStringInput() throws JsonLogicEvaluationException {
// given
final StringComp operator = new StringComp(StringComp.Type.ENDS_WITH);

// when - non-string first arg
Object result = operator.evaluate(Arrays.asList(123, "abc"), new Object(), "jsonPath");

// then
assertThat(result).isNull();
}
}
Loading