feat: Change fractional custom op from percentage-based to relative weighting. open-feature#828

Signed-off-by: Simon Schrottner <[email protected]>

Author: aepfli

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<FractionProperty> 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<FractionProperty> propertyList)
+    private static String distributeValue(final String hashKey, final List<FractionProperty> 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<FractionP
     @SuppressWarnings({"checkstyle:NoFinalizer"})
     private static class FractionProperty {
         private final String variant;
-        private final int percentage;
+        private final int weight;
 
         protected final void finalize() {
             // DO NOT REMOVE, spotbugs: CT_CONSTRUCTOR_THROW
@@ -108,23 +103,32 @@ protected final void finalize() {
 
             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
             if (!(array.get(0) instanceof String)) {
                 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;
+        }
     }
 }

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<Object> rule = new ArrayList<>();
+
+        final List<Object> bucket1 = new ArrayList<>();
+        bucket1.add("blue");
+        bucket1.add(50);
+
+        final List<Object> bucket2 = new ArrayList<>();
+        bucket2.add("green");
+        bucket2.add(70);
+
+        rule.add(bucket1);
+        rule.add(bucket2);
+
+        Map<String, String> data = new HashMap<>();
+        data.put(FLAG_KEY, "headerColor");
+        data.put(TARGET_KEY, "[email protected]");
+
+        // 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);
     }
 
-}
+}