diff --git a/tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java b/tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java index f36ebfffe..234c8a11e 100644 --- a/tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java +++ b/tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java @@ -3,6 +3,8 @@ import io.github.jamsesso.jsonlogic.JsonLogicException; import io.github.jamsesso.jsonlogic.evaluator.JsonLogicEvaluationException; import io.github.jamsesso.jsonlogic.evaluator.expressions.PreEvaluatedArgumentsExpression; +import java.math.BigDecimal; +import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; @@ -33,13 +35,19 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json // check optional string target in first arg Object arg1 = arguments.get(0); - final String bucketBy; + final byte[] bucketBy; final Object[] distributions; if (arg1 instanceof String) { - // first arg is a String, use for bucketing - bucketBy = (String) arg1; - + bucketBy = ((String) arg1).getBytes(StandardCharsets.UTF_8); + Object[] source = arguments.toArray(); + distributions = Arrays.copyOfRange(source, 1, source.length); + } else if (arg1 instanceof Number) { + bucketBy = numberToByteArray((Number) arg1); + Object[] source = arguments.toArray(); + distributions = Arrays.copyOfRange(source, 1, source.length); + } else if (arg1 instanceof Boolean) { + bucketBy = new byte[] {(byte) (((boolean) arg1) ? 1 : 0)}; Object[] source = arguments.toArray(); distributions = Arrays.copyOfRange(source, 1, source.length); } else { @@ -49,7 +57,7 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json return null; } - bucketBy = properties.getFlagKey() + properties.getTargetingKey(); + bucketBy = (properties.getFlagKey() + properties.getTargetingKey()).getBytes(StandardCharsets.UTF_8); distributions = arguments.toArray(); } @@ -71,29 +79,78 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json return distributeValue(bucketBy, propertyList, totalWeight, jsonPath); } + private byte[] numberToByteArray(Number number) { + if (number instanceof Integer) { + return new byte[] { + (byte) ((int) number >> 24), + (byte) ((int) number >> 16), + (byte) ((int) number >> 8), + (byte) ((int) number) + }; + } else if (number instanceof Double) { + return numberToByteArray(Double.doubleToLongBits((Double) number)); + } else if (number instanceof Long) { + return new byte[] { + (byte) ((long) number >> 56), + (byte) ((long) number >> 48), + (byte) ((long) number >> 40), + (byte) ((long) number >> 32), + (byte) ((long) number >> 24), + (byte) ((long) number >> 16), + (byte) ((long) number >> 8), + (byte) ((long) number) + }; + } else if (number instanceof BigInteger) { + return ((BigInteger) number).toByteArray(); + } else if (number instanceof Byte) { + return new byte[] {(byte) number}; + } else if (number instanceof Short) { + return new byte[] {(byte) ((short) number >> 8), (byte) ((short) number)}; + } else if (number instanceof Float) { + return numberToByteArray(Float.floatToIntBits((Float) number)); + } else if (number instanceof BigDecimal) { + return numberToByteArray(Double.doubleToLongBits(number.doubleValue())); + } else { + throw new IllegalArgumentException("Unsupported number type: " + number.getClass()); + } + } + private static String distributeValue( - final String hashKey, final List propertyList, int totalWeight, String jsonPath) + final byte[] hashKey, + final List propertyList, + final int totalWeight, + final String jsonPath) + throws JsonLogicEvaluationException { + int mmrHash = MurmurHash3.hash32x86(hashKey, 0, hashKey.length, 0); + return distributeValueFromHash(mmrHash, propertyList, totalWeight, jsonPath); + } + + static String distributeValueFromHash( + final int hash, final List propertyList, final int totalWeight, final String jsonPath) throws JsonLogicEvaluationException { - byte[] bytes = hashKey.getBytes(StandardCharsets.UTF_8); - int mmrHash = MurmurHash3.hash32x86(bytes, 0, bytes.length, 0); - float bucket = Math.abs(mmrHash) * 1.0f / Integer.MAX_VALUE * 100; + long longHash = Math.abs((long) hash); + if (hash < 0) { + // preserve the MSB (sign) of the hash, which would get lost in a typecast and in Math.abs + longHash = longHash | (1L << 31); + } + int bucket = Math.abs((int) ((longHash * totalWeight) >> 32)); - float bucketSum = 0; + int bucketSum = 0; for (FractionProperty p : propertyList) { - bucketSum += p.getPercentage(totalWeight); + bucketSum += p.weight; - if (bucket < bucketSum) { + if (bucket <= bucketSum) { return p.getVariant(); } } // this shall not be reached - throw new JsonLogicEvaluationException("Unable to find a correct bucket", jsonPath); + throw new JsonLogicEvaluationException("Unable to find a correct bucket for hash " + hash, jsonPath); } @Getter @SuppressWarnings({"checkstyle:NoFinalizer"}) - private static class FractionProperty { + static class FractionProperty { private final String variant; private final int weight; @@ -129,12 +186,5 @@ protected final void finalize() { weight = 1; } } - - float getPercentage(int totalWeight) { - if (weight == 0) { - return 0; - } - return (float) (weight * 100) / totalWeight; - } } } diff --git a/tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/FractionalTest.java b/tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/FractionalTest.java index 897be5219..cb5404f9e 100644 --- a/tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/FractionalTest.java +++ b/tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/FractionalTest.java @@ -1,27 +1,36 @@ package dev.openfeature.contrib.tools.flagd.core.targeting; import static dev.openfeature.contrib.tools.flagd.core.targeting.Operator.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Named.named; import static org.junit.jupiter.params.provider.Arguments.arguments; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; +import io.github.jamsesso.jsonlogic.JsonLogicException; import io.github.jamsesso.jsonlogic.evaluator.JsonLogicEvaluationException; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; 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; import org.junit.jupiter.params.converter.TypedArgumentConverter; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; class FractionalTest { @@ -47,6 +56,60 @@ void validate_emptyJson_targetingReturned(@ConvertWith(FileContentConverter.clas assertEquals(testData.result, evaluate); } + @ParameterizedTest + @ValueSource(ints = {0, 1, -1, Integer.MAX_VALUE, Integer.MAX_VALUE - 1, Integer.MIN_VALUE, Integer.MIN_VALUE + 1}) + void edgeCasesDoNotThrow(int hash) throws JsonLogicException { + int totalWeight = 8; + int buckets = 4; + List bucketsList = new ArrayList<>(buckets); + for (int i = 0; i < buckets; i++) { + bucketsList.add(new Fractional.FractionProperty(List.of("bucket" + i, totalWeight / buckets), "")); + } + + AtomicReference result = new AtomicReference<>(); + assertDoesNotThrow(() -> result.set(Fractional.distributeValueFromHash(hash, bucketsList, totalWeight, ""))); + + assertNotNull(result.get()); + assertTrue(result.get().startsWith("bucket")); + } + + @Test + void statistics() throws JsonLogicException { + int totalWeight = Integer.MAX_VALUE; + int buckets = 16; + int[] hits = new int[buckets]; + List bucketsList = new ArrayList<>(buckets); + int weight = totalWeight / buckets; + for (int i = 0; i < buckets - 1; i++) { + bucketsList.add(new Fractional.FractionProperty(List.of("" + i, weight), "")); + } + bucketsList.add( + new Fractional.FractionProperty(List.of("" + (buckets - 1), totalWeight - weight * (buckets - 1)), "")); + + for (long i = Integer.MIN_VALUE; i <= Integer.MAX_VALUE; i += 127) { + String bucketStr = Fractional.distributeValueFromHash((int) i, bucketsList, totalWeight, ""); + int bucket = Integer.parseInt(bucketStr); + hits[bucket]++; + } + + int min = Integer.MAX_VALUE; + int max = Integer.MIN_VALUE; + for (int i = 0; i < hits.length; i++) { + int current = hits[i]; + if (current < min) { + min = current; + } + if (current > max) { + max = current; + } + } + + int delta = max - min; + assertTrue( + delta < 3, + "Delta should be less than 3, but was " + delta + ". Distributions: " + Arrays.toString(hits)); + } + public static Stream allFilesInDir() throws IOException { return Files.list(Paths.get("src", "test", "resources", "fractional")) .map(path -> arguments(named(path.getFileName().toString(), path))); diff --git a/tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/OperatorTest.java b/tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/OperatorTest.java index cbd28a9e9..0408bf083 100644 --- a/tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/OperatorTest.java +++ b/tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/OperatorTest.java @@ -75,7 +75,7 @@ void testFlagPropertiesConstructor() { } @Test - void fractionalTestA() throws TargetingRuleException { + void fractionalTestB() throws TargetingRuleException { // given // fractional rule with email as expression key @@ -112,11 +112,11 @@ void fractionalTestA() throws TargetingRuleException { Object evalVariant = OPERATOR.apply("headerColor", targetingRule, new ImmutableContext(ctxData)); // then - assertEquals("yellow", evalVariant); + assertEquals("blue", evalVariant); } @Test - void fractionalTestB() throws TargetingRuleException { + void fractionalTestA() throws TargetingRuleException { // given // fractional rule with email as expression key @@ -153,7 +153,7 @@ void fractionalTestB() throws TargetingRuleException { Object evalVariant = OPERATOR.apply("headerColor", targetingRule, new ImmutableContext(ctxData)); // then - assertEquals("blue", evalVariant); + assertEquals("red", evalVariant); } @Test diff --git a/tools/flagd-core/src/test/resources/fractional/boolean.json b/tools/flagd-core/src/test/resources/fractional/boolean.json new file mode 100644 index 000000000..36182886f --- /dev/null +++ b/tools/flagd-core/src/test/resources/fractional/boolean.json @@ -0,0 +1,14 @@ +{ + "rule": [ + true, + [ + "blue", + 50 + ], + [ + "green", + 70 + ] + ], + "result": "green" +} diff --git a/tools/flagd-core/src/test/resources/fractional/largeDouble.json b/tools/flagd-core/src/test/resources/fractional/largeDouble.json new file mode 100644 index 000000000..b47cb6385 --- /dev/null +++ b/tools/flagd-core/src/test/resources/fractional/largeDouble.json @@ -0,0 +1,14 @@ +{ + "rule": [ + 9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999.9, + [ + "blue", + 50 + ], + [ + "green", + 70 + ] + ], + "result": "green" +} diff --git a/tools/flagd-core/src/test/resources/fractional/largeInt.json b/tools/flagd-core/src/test/resources/fractional/largeInt.json new file mode 100644 index 000000000..4bb815044 --- /dev/null +++ b/tools/flagd-core/src/test/resources/fractional/largeInt.json @@ -0,0 +1,14 @@ +{ + "rule": [ + 9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999, + [ + "blue", + 50 + ], + [ + "green", + 70 + ] + ], + "result": "blue" +} diff --git a/tools/flagd-core/src/test/resources/fractional/selfContainedFractionalB.json b/tools/flagd-core/src/test/resources/fractional/selfContainedFractionalB.json index 2beb7e5be..e632f17d9 100644 --- a/tools/flagd-core/src/test/resources/fractional/selfContainedFractionalB.json +++ b/tools/flagd-core/src/test/resources/fractional/selfContainedFractionalB.json @@ -10,5 +10,5 @@ 50 ] ], - "result": "blue" + "result": "red" } diff --git a/tools/flagd-core/src/test/resources/fractional/string.json b/tools/flagd-core/src/test/resources/fractional/string.json new file mode 100644 index 000000000..8c55b68c1 --- /dev/null +++ b/tools/flagd-core/src/test/resources/fractional/string.json @@ -0,0 +1,14 @@ +{ + "rule": [ + "some string", + [ + "blue", + 50 + ], + [ + "green", + 70 + ] + ], + "result": "blue" +}