feat!: fractional bucketing improvements (#1740)

Signed-off-by: christian.lutnik <christian.lutnik@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>

Author: chrfwow

providers/flagd/schemas

@@ -28,7 +28,17 @@
 @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("file")
-@ExcludeTags({"unixsocket", "targetURI", "reconnect", "customCert", "events", "contextEnrichment", "deprecated"})
+@ExcludeTags({
+    "unixsocket",
+    "targetURI",
+    "reconnect",
+    "customCert",
+    "events",
+    "contextEnrichment",
+    "fractional-v1",
+    "deprecated",
+    "operator-errors"
+})
 @Testcontainers
 public class RunFileTest {
 

providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java

@@ -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", "deprecated"})
+@ExcludeTags({"unixsocket", "fractional-v1", "deprecated", "operator-errors"})
 @Testcontainers
 public class RunInProcessTest {
 

providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java

@@ -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({"rpc"})
-@ExcludeTags({"unixsocket", "deprecated"})
+@ExcludeTags({"unixsocket", "fractional-v1", "deprecated", "operator-errors"})
 @Testcontainers
 public class RunRpcTest {
 

providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java

@@ -17,14 +17,16 @@
 @Slf4j
 class Fractional implements PreEvaluatedArgumentsExpression {
 
+    static final int MAX_WEIGHT = Integer.MAX_VALUE;
+
     @Override
     public String key() {
         return "fractional";
     }
 
     @Override
     public Object evaluate(List arguments, Object data, String jsonPath) throws JsonLogicEvaluationException {
-        if (arguments.size() < 2) {
+        if (arguments.size() < 1) {
             return null;
         }
 
@@ -39,7 +41,6 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json
         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);
         } else {
@@ -54,7 +55,7 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json
         }
 
         final List<FractionProperty> propertyList = new ArrayList<>();
-        int totalWeight = 0;
+        long totalWeight = 0;
 
         try {
             for (Object dist : distributions) {
@@ -67,34 +68,54 @@ public Object evaluate(List arguments, Object data, String jsonPath) throws Json
             return null;
         }
 
+        if (totalWeight > MAX_WEIGHT) {
+            log.debug("Total weight {} exceeds maximum allowed value {}", totalWeight, MAX_WEIGHT);
+            return null;
+        }
+
+        if (totalWeight == 0) {
+            log.debug("Total weight is 0, no valid distribution possible");
+            return null;
+        }
+
         // find distribution
-        return distributeValue(bucketBy, propertyList, totalWeight, jsonPath);
+        return distributeValue(bucketBy, propertyList, (int) totalWeight, jsonPath);
     }
 
-    private static String distributeValue(
-            final String hashKey, final List<FractionProperty> propertyList, int totalWeight, String jsonPath)
+    private static Object distributeValue(
+            final String hashKey,
+            final List<FractionProperty> 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;
+        return distributeValueFromHash(mmrHash, propertyList, totalWeight, jsonPath);
+    }
 
-        float bucketSum = 0;
+    static Object distributeValueFromHash(
+            final int hash, final List<FractionProperty> propertyList, final int totalWeight, final String jsonPath)
+            throws JsonLogicEvaluationException {
+        long longHash = Integer.toUnsignedLong(hash);
+        int bucket = (int) ((longHash * totalWeight) >>> 32);
+
+        int bucketSum = 0;
         for (FractionProperty p : propertyList) {
-            bucketSum += p.getPercentage(totalWeight);
+            bucketSum += p.weight;
 
             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 {
-        private final String variant;
+    static class FractionProperty {
+        private final Object variant;
         private final int weight;
 
         protected final void finalize() {
@@ -112,29 +133,38 @@ protected final void finalize() {
                 throw new JsonLogicException("Fraction property needs at least one element", jsonPath);
             }
 
-            // first must be a string
-            if (!(array.get(0) instanceof String)) {
+            // variant must be a primitive (string, number, boolean) or null;
+            // nested JSONLogic expressions are pre-evaluated to these types
+            Object first = array.get(0);
+            if (first instanceof String || first instanceof Number || first instanceof Boolean || first == null) {
+                variant = first;
+            } else {
                 throw new JsonLogicException(
-                        "First element of the fraction property is not a string variant", jsonPath);
+                        "First element of the fraction property must resolve to a string, number, boolean, or null",
+                        jsonPath);
             }
 
-            variant = (String) array.get(0);
             if (array.size() >= 2) {
-                // second element must be a number
+                // weight must be a number
                 if (!(array.get(1) instanceof Number)) {
                     throw new JsonLogicException("Second element of the fraction property is not a number", jsonPath);
                 }
-                weight = ((Number) array.get(1)).intValue();
+                Number rawWeight = (Number) array.get(1);
+
+                // weights must be integers
+                double weightDouble = rawWeight.doubleValue();
+                if (Double.isInfinite(weightDouble)
+                        || Double.isNaN(weightDouble)
+                        || weightDouble != Math.floor(weightDouble)) {
+                    throw new JsonLogicException("Weights must be integers", jsonPath);
+                }
+
+                // negative weights can be the result of rollout calculations,
+                // so we clamp to 0 rather than throwing an error
+                weight = Math.max(0, (int) weightDouble);
             } else {
                 weight = 1;
             }
         }
-
-        float getPercentage(int totalWeight) {
-            if (weight == 0) {
-                return 0;
-            }
-            return (float) (weight * 100) / totalWeight;
-        }
     }
 }

providers/flagd/test-harness

@@ -35,7 +35,7 @@
           "type": "string"
         },
         {
-          "description": "When returned from rules, strings are used to as keys to retrieve the associated value from the \"variants\" object. Be sure that the returned string is present as a key in the variants!.",
+          "description": "When returned from rules, the behavior of arrays is not defined.",
           "type": "array"
         }
       ]
@@ -461,18 +461,26 @@
       "maxItems": 2,
       "items": [
         {
-          "description": "If this bucket is randomly selected, this string is used to as a key to retrieve the associated value from the \"variants\" object.",
-          "type": "string"
+          "description": "If this bucket is randomly selected, this JSONLogic will be evaluated, and the result will be used as the variant key to return from the variants map.",
+          "$ref": "#/definitions/args"
         },
         {
-          "description": "Weighted distribution for this variant key.",
-          "type": "number"
+          "description": "Weighted distribution for this variant key. Must be a non-negative integer. Can be a JSONLogic expression that evaluates to a number (e.g. for time-based progressive rollouts); computed negative weights are clamped to 0 at evaluation time. The total weight sum across all variants must not exceed 2,147,483,647.",
+          "oneOf": [
+            {
+              "type": "integer",
+              "minimum": 0
+            },
+            {
+              "$ref": "#/definitions/anyRule"
+            }
+          ]
         }
       ]
     },
     "fractionalOp": {
       "type": "array",
-      "minItems": 3,
+      "minItems": 1,
       "$comment": "there seems to be a bug here, where ajv gives a warning (not an error) because maxItems doesn't equal the number of entries in items, though this is valid in this case",
       "items": [
         {
@@ -492,7 +500,7 @@
     },
     "fractionalShorthandOp": {
       "type": "array",
-      "minItems": 2,
+      "minItems": 1,
       "items": {
         "$ref": "#/definitions/fractionalWeightArg"
       }

tools/flagd-api-testkit/test-harness

@@ -3,13 +3,15 @@
 import dev.openfeature.contrib.tools.flagd.api.Evaluator;
 import dev.openfeature.contrib.tools.flagd.api.testkit.AbstractEvaluatorTest;
 import dev.openfeature.contrib.tools.flagd.core.FlagdCore;
+import org.junit.platform.suite.api.ExcludeTags;
 
 /**
  * Compliance test suite for {@link FlagdCore} — the reference implementation of the
  * flagd-api-testkit. Extends {@link AbstractEvaluatorTest} which provides all runner
  * configuration. Registered as an {@link dev.openfeature.contrib.tools.flagd.api.testkit.EvaluatorFactory}
  * via {@code META-INF/services}.
  */
+@ExcludeTags({"fractional-v1"})
 public class FlagdCoreEvaluatorTest extends AbstractEvaluatorTest {
 
     @Override