From 8f8e93f339145d7e877056488dd77949b1146d13 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 19 Jun 2024 13:44:34 +0200 Subject: [PATCH] feat: Change fractional custom op from percentage-based to relative weighting. #828 Signed-off-by: Simon Schrottner --- .../process/targeting/Fractional.java | 46 ++++++++-------- .../process/targeting/FractionalTest.java | 52 +++++++++++++++++-- 2 files changed, 72 insertions(+), 26 deletions(-) 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..75b08974e 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,32 @@ 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) + 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 +90,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 +112,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; + } } + double getPercentage(int totalWeight) { + if (weight == 0) { + return 0; + } + return 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..fd758f967 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 @@ -151,7 +151,7 @@ void targetingBackedFractional() throws JsonLogicEvaluationException { @Test - void invalidRuleSumNot100() throws JsonLogicEvaluationException { + void invalidRuleSumGreater100() throws JsonLogicEvaluationException { // given Fractional fractional = new Fractional(); @@ -189,7 +189,49 @@ void invalidRuleSumNot100() throws JsonLogicEvaluationException { Object evaluate = fractional.evaluate(rule, data); // then - assertNull(evaluate); + assertEquals("blue", evaluate); + } + + @Test + void invalidRuleSumlower100() throws JsonLogicEvaluationException { + // given + Fractional fractional = new Fractional(); + + /* Rule + * [ + * [ + * "blue", + * 50 + * ], + * [ + * "green", + * 30 + * ] + * ] + * */ + + 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 + assertEquals("blue", evaluate); } @Test @@ -227,7 +269,7 @@ void notEnoughBuckets() throws JsonLogicEvaluationException { @Test - void invalidRule() throws JsonLogicEvaluationException { + void prefillingRuleWithPlaceHolderValue() throws JsonLogicEvaluationException { // given Fractional fractional = new Fractional(); @@ -263,7 +305,7 @@ void invalidRule() throws JsonLogicEvaluationException { Object evaluate = fractional.evaluate(rule, data); // then - assertNull(evaluate); + assertEquals("blue", evaluate); } -} \ No newline at end of file +}