diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/targeting/Fractional.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/targeting/Fractional.java index 9f2d1859f..33693b62f 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/targeting/Fractional.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/targeting/Fractional.java @@ -50,37 +50,35 @@ public Object evaluate(List arguments, Object data) throws JsonLogicEvaluationEx } final List propertyList = new ArrayList<>(); + int totalWeight = 0; - double distribution = 0; try { for (Object dist : distibutions) { FractionProperty fractionProperty = new FractionProperty(dist); propertyList.add(fractionProperty); - distribution += fractionProperty.getPercentage(); + totalWeight += fractionProperty.getWeight(); } } catch (JsonLogicException e) { log.debug("Error parsing fractional targeting rule", e); return null; } - if (distribution != 100) { - log.debug("Fractional properties do not sum to 100"); - return null; - } - // find distribution - return distributeValue(bucketBy, propertyList); + return distributeValue(bucketBy, propertyList, totalWeight); } - private static String distributeValue(final String hashKey, final List propertyList) - throws JsonLogicEvaluationException { + private static String distributeValue( + final String hashKey, + final List propertyList, + int totalWeight + ) throws JsonLogicEvaluationException { byte[] bytes = hashKey.getBytes(StandardCharsets.UTF_8); int mmrHash = MurmurHash3.hash32x86(bytes, 0, bytes.length, 0); - int bucket = (int) ((Math.abs(mmrHash) * 1.0f / Integer.MAX_VALUE) * 100); + float bucket = (Math.abs(mmrHash) * 1.0f / Integer.MAX_VALUE) * 100; - int bucketSum = 0; + float bucketSum = 0; for (FractionProperty p : propertyList) { - bucketSum += p.getPercentage(); + bucketSum += p.getPercentage(totalWeight); if (bucket < bucketSum) { return p.getVariant(); @@ -95,7 +93,7 @@ private static String distributeValue(final String hashKey, final List array = (List) from; - if (array.size() != 2) { - throw new JsonLogicException("Fraction property does not have two elements"); + if (array.isEmpty()) { + throw new JsonLogicException("Fraction property needs at least one element"); } // first must be a string @@ -117,14 +115,23 @@ protected final void finalize() { throw new JsonLogicException("First element of the fraction property is not a string variant"); } - // second element must be a number - if (!(array.get(1) instanceof Number)) { - throw new JsonLogicException("Second element of the fraction property is not a number"); - } - variant = (String) array.get(0); - percentage = ((Number) array.get(1)).intValue(); + if (array.size() >= 2) { + // second element must be a number + if (!(array.get(1) instanceof Number)) { + throw new JsonLogicException("Second element of the fraction property is not a number"); + } + weight = ((Number) array.get(1)).intValue(); + } else { + weight = 1; + } } + float getPercentage(int totalWeight) { + if (weight == 0) { + return 0; + } + return (float) (weight * 100) / totalWeight; + } } } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/targeting/FractionalTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/targeting/FractionalTest.java index 9ef877d41..9f1002fe2 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/targeting/FractionalTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/targeting/FractionalTest.java @@ -1,269 +1,81 @@ package dev.openfeature.contrib.providers.flagd.resolver.process.targeting; -import static dev.openfeature.contrib.providers.flagd.resolver.process.targeting.Operator.FLAGD_PROPS_KEY; -import static dev.openfeature.contrib.providers.flagd.resolver.process.targeting.Operator.FLAG_KEY; -import static dev.openfeature.contrib.providers.flagd.resolver.process.targeting.Operator.TARGET_KEY; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; - -import java.util.ArrayList; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.github.jamsesso.jsonlogic.evaluator.JsonLogicEvaluationException; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.converter.ArgumentConversionException; +import org.junit.jupiter.params.converter.ConvertWith; +import org.junit.jupiter.params.converter.TypedArgumentConverter; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; -import org.junit.jupiter.api.Test; - -import io.github.jamsesso.jsonlogic.evaluator.JsonLogicEvaluationException; +import static dev.openfeature.contrib.providers.flagd.resolver.process.targeting.Operator.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Named.named; +import static org.junit.jupiter.params.provider.Arguments.arguments; class FractionalTest { - @Test - void selfContainedFractionalA() throws JsonLogicEvaluationException { + @ParameterizedTest + @MethodSource("allFilesInDir") + void validate_emptyJson_targetingReturned(@ConvertWith(FileContentConverter.class) TestData testData) + throws JsonLogicEvaluationException { // given Fractional fractional = new Fractional(); - /* Rule - * [ - * "flagAbucketKeyA", // this is resolved value of an expression - * [ - * "red", - * 50 - * ], - * [ - * "blue", - * 50 - * ] - * ] - * */ - - final List rule = new ArrayList<>(); - rule.add("flagAbucketKeyA"); - - final List bucket1 = new ArrayList<>(); - bucket1.add("red"); - bucket1.add(50); - - final List bucket2 = new ArrayList<>(); - bucket2.add("green"); - bucket2.add(50); - - rule.add(bucket1); - rule.add(bucket2); - - Map flagdProperties = new HashMap<>(); - flagdProperties.put(FLAG_KEY, "flagA"); Map data = new HashMap<>(); - data.put(FLAGD_PROPS_KEY, flagdProperties); - - // when - Object evaluate = fractional.evaluate(rule, data); - - // then - assertEquals("red", evaluate); - } - - @Test - void selfContainedFractionalB() throws JsonLogicEvaluationException { - // given - Fractional fractional = new Fractional(); - - /* Rule - * [ - * "flagAbucketKeyB", // this is resolved value of an expression - * [ - * "red", - * 50 - * ], - * [ - * "blue", - * 50 - * ] - * ] - * */ - - final List rule = new ArrayList<>(); - rule.add("flagAbucketKeyB"); - - final List bucket1 = new ArrayList<>(); - bucket1.add("red"); - bucket1.add(50); - - final List bucket2 = new ArrayList<>(); - bucket2.add("green"); - bucket2.add(50); - - rule.add(bucket1); - rule.add(bucket2); + data.put(FLAG_KEY, "headerColor"); + data.put(TARGET_KEY, "foo@foo.com"); Map flagdProperties = new HashMap<>(); flagdProperties.put(FLAG_KEY, "flagA"); - Map data = new HashMap<>(); data.put(FLAGD_PROPS_KEY, flagdProperties); // when - Object evaluate = fractional.evaluate(rule, data); - - // then - assertEquals("green", evaluate); - } - - @Test - void targetingBackedFractional() throws JsonLogicEvaluationException { - // given - Fractional fractional = new Fractional(); - - /* Rule - * [ - * [ - * "blue", - * 50 - * ], - * [ - * "green", - * 50 - * ] - * ] - * */ - - final List rule = new ArrayList<>(); - - final List bucket1 = new ArrayList<>(); - bucket1.add("blue"); - bucket1.add(50); - - final List bucket2 = new ArrayList<>(); - bucket2.add("green"); - bucket2.add(50); - - rule.add(bucket1); - rule.add(bucket2); - - Map data = new HashMap<>(); - data.put(FLAG_KEY, "headerColor"); - data.put(TARGET_KEY, "foo@foo.com"); - - // when - Object evaluate = fractional.evaluate(rule, data); + Object evaluate = fractional.evaluate(testData.rule, data); // then - assertEquals("blue", evaluate); + assertEquals(testData.result, evaluate); } - - @Test - void invalidRuleSumNot100() throws JsonLogicEvaluationException { - // given - Fractional fractional = new Fractional(); - - /* Rule - * [ - * [ - * "blue", - * 50 - * ], - * [ - * "green", - * 70 - * ] - * ] - * */ - - final List rule = new ArrayList<>(); - - final List bucket1 = new ArrayList<>(); - bucket1.add("blue"); - bucket1.add(50); - - final List bucket2 = new ArrayList<>(); - bucket2.add("green"); - bucket2.add(70); - - rule.add(bucket1); - rule.add(bucket2); - - Map data = new HashMap<>(); - data.put(FLAG_KEY, "headerColor"); - data.put(TARGET_KEY, "foo@foo.com"); - - // when - Object evaluate = fractional.evaluate(rule, data); - - // then - assertNull(evaluate); + public static Stream allFilesInDir() throws IOException { + return Files.list(Paths.get("src", "test", "resources", "fractional")) + .map(path -> arguments(named(path.getFileName().toString(), path))); } - @Test - void notEnoughBuckets() throws JsonLogicEvaluationException { - // given - Fractional fractional = new Fractional(); - - /* Rule - * [ - * [ - * "blue", - * 100 - * ] - * ] - * */ - - final List rule = new ArrayList<>(); - - final List bucket1 = new ArrayList<>(); - bucket1.add("blue"); - bucket1.add(50); - - rule.add(bucket1); - - Map data = new HashMap<>(); - data.put(FLAG_KEY, "headerColor"); - data.put(TARGET_KEY, "foo@foo.com"); - - // when - Object evaluate = fractional.evaluate(rule, data); - - // then - assertNull(evaluate); + static class FileContentConverter extends TypedArgumentConverter { + protected FileContentConverter() { + super(Path.class, TestData.class); + } + + @Override + protected TestData convert(Path path) throws ArgumentConversionException { + try { + Stream lines = Files.lines(path); + String data = lines.collect(Collectors.joining("\n")); + lines.close(); + ObjectMapper mapper = new ObjectMapper(); + return mapper.readValue(data, TestData.class); + } catch (IOException e) { + throw new RuntimeException(e); + } + } } - - @Test - void invalidRule() throws JsonLogicEvaluationException { - // given - Fractional fractional = new Fractional(); - - /* Rule - * [ - * [ - * "blue", - * 50 - * ], - * [ - * "green" - * ] - * ] - * */ - - final List rule = new ArrayList<>(); - - final List bucket1 = new ArrayList<>(); - bucket1.add("blue"); - bucket1.add(50); - - final List bucket2 = new ArrayList<>(); - bucket2.add("green"); - - rule.add(bucket1); - rule.add(bucket2); - - Map data = new HashMap<>(); - data.put(FLAG_KEY, "headerColor"); - data.put(TARGET_KEY, "foo@foo.com"); - - // when - Object evaluate = fractional.evaluate(rule, data); - - // then - assertNull(evaluate); + static class TestData { + @JsonProperty("result") + Object result; + @JsonProperty("rule") + List rule; } - -} \ No newline at end of file +} diff --git a/providers/flagd/src/test/resources/fractional/1-1-1-1.json b/providers/flagd/src/test/resources/fractional/1-1-1-1.json new file mode 100644 index 000000000..74f0a1b67 --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/1-1-1-1.json @@ -0,0 +1,21 @@ +{ + "rule": [ + [ + "blue", + 1 + ], + [ + "green", + 1 + ], + [ + "red", + 1 + ], + [ + "yellow", + 1 + ] + ], + "result": "blue" +} diff --git a/providers/flagd/src/test/resources/fractional/25-25-25-25.json b/providers/flagd/src/test/resources/fractional/25-25-25-25.json new file mode 100644 index 000000000..a849ba602 --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/25-25-25-25.json @@ -0,0 +1,21 @@ +{ + "rule": [ + [ + "blue", + 25 + ], + [ + "green", + 25 + ], + [ + "red", + 25 + ], + [ + "yellow", + 25 + ] + ], + "result": "blue" +} diff --git a/providers/flagd/src/test/resources/fractional/50-50.json b/providers/flagd/src/test/resources/fractional/50-50.json new file mode 100644 index 000000000..5f158006c --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/50-50.json @@ -0,0 +1,13 @@ +{ + "rule": [ + [ + "blue", + 50 + ], + [ + "green", + 50 + ] + ], + "result": "blue" +} diff --git a/providers/flagd/src/test/resources/fractional/notEnoughBuckets.json b/providers/flagd/src/test/resources/fractional/notEnoughBuckets.json new file mode 100644 index 000000000..b29de7a83 --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/notEnoughBuckets.json @@ -0,0 +1,9 @@ +{ + "rule": [ + [ + "blue", + 50 + ] + ], + "result": null +} diff --git a/providers/flagd/src/test/resources/fractional/selfContainedFractionalA.json b/providers/flagd/src/test/resources/fractional/selfContainedFractionalA.json new file mode 100644 index 000000000..88868c6f3 --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/selfContainedFractionalA.json @@ -0,0 +1,14 @@ +{ + "rule": [ + "flagAbucketKeyA", + [ + "red", + 50 + ], + [ + "blue", + 50 + ] + ], + "result": "red" +} diff --git a/providers/flagd/src/test/resources/fractional/selfContainedFractionalB.json b/providers/flagd/src/test/resources/fractional/selfContainedFractionalB.json new file mode 100644 index 000000000..2beb7e5be --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/selfContainedFractionalB.json @@ -0,0 +1,14 @@ +{ + "rule": [ + "flagAbucketKeyB", + [ + "red", + 50 + ], + [ + "blue", + 50 + ] + ], + "result": "blue" +} diff --git a/providers/flagd/src/test/resources/fractional/sum-greater-100.json b/providers/flagd/src/test/resources/fractional/sum-greater-100.json new file mode 100644 index 000000000..806dae532 --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/sum-greater-100.json @@ -0,0 +1,13 @@ +{ + "rule": [ + [ + "blue", + 50 + ], + [ + "green", + 70 + ] + ], + "result": "blue" +} diff --git a/providers/flagd/src/test/resources/fractional/sum-lower-100.json.json b/providers/flagd/src/test/resources/fractional/sum-lower-100.json.json new file mode 100644 index 000000000..06bdb8f14 --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/sum-lower-100.json.json @@ -0,0 +1,13 @@ +{ + "rule": [ + [ + "blue", + 50 + ], + [ + "green", + 30 + ] + ], + "result": "blue" +} diff --git a/providers/flagd/src/test/resources/fractional/template.json b/providers/flagd/src/test/resources/fractional/template.json new file mode 100644 index 000000000..8258e04de --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/template.json @@ -0,0 +1,5 @@ +{ + "rule": [ + ], + "result": null +} diff --git a/providers/flagd/src/test/resources/fractional/weighting-not-set.json b/providers/flagd/src/test/resources/fractional/weighting-not-set.json new file mode 100644 index 000000000..9e2278791 --- /dev/null +++ b/providers/flagd/src/test/resources/fractional/weighting-not-set.json @@ -0,0 +1,12 @@ +{ + "rule": [ + [ + "blue", + 50 + ], + [ + "green" + ] + ], + "result": "blue" +}