fix(flagd): improve targeting and fix fractional issue(#92) (#105)

Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>

providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py

@@ -1,29 +1,18 @@
-import time
 import typing
 
-from json_logic import builtins, jsonLogic  # type: ignore[import-untyped]
-
 from openfeature.evaluation_context import EvaluationContext
 from openfeature.exception import FlagNotFoundError, ParseError
 from openfeature.flag_evaluation import FlagResolutionDetails, Reason
 from openfeature.provider.provider import AbstractProvider
 
 from ..config import Config
-from .process.custom_ops import ends_with, fractional, sem_ver, starts_with
 from .process.file_watcher import FileWatcherFlagStore
+from .process.targeting import targeting
 
 T = typing.TypeVar("T")
 
 
 class InProcessResolver:
-    OPERATORS: typing.ClassVar[dict] = {
-        **builtins.BUILTINS,
-        "fractional": fractional,
-        "starts_with": starts_with,
-        "ends_with": ends_with,
-        "sem_ver": sem_ver,
-    }
-
     def __init__(self, config: Config, provider: AbstractProvider):
         self.config = config
         self.provider = provider
@@ -97,12 +86,8 @@ class InProcessResolver:
             variant, value = flag.default
             return FlagResolutionDetails(value, variant=variant, reason=Reason.STATIC)
 
-        json_logic_context = evaluation_context.attributes if evaluation_context else {}
-        json_logic_context["$flagd"] = {"flagKey": key, "timestamp": int(time.time())}
-        json_logic_context["targetingKey"] = (
-            evaluation_context.targeting_key if evaluation_context else None
-        )
-        variant = jsonLogic(flag.targeting, json_logic_context, self.OPERATORS)
+        variant = targeting(flag.key, flag.targeting, evaluation_context)
+
         if variant is None:
             variant, value = flag.default
             return FlagResolutionDetails(value, variant=variant, reason=Reason.DEFAULT)
@@ -110,7 +95,6 @@ class InProcessResolver:
             raise ParseError(
                 "Parsed JSONLogic targeting did not return a string or bool"
             )
-
         variant, value = flag.get_variant(variant)
         if not value:
             raise ParseError(f"Resolved variant {variant} not in variants config.")

providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/custom_ops.py

@@ -43,39 +43,40 @@ def fractional(data: dict, *args: JsonLogicArg) -> typing.Optional[str]:
 
     total_weight = 0
     fractions = []
-    for arg in args:
-        fraction = _parse_fraction(arg)
-        if fraction:
-            fractions.append(fraction)
-            total_weight += fraction.weight
+    try:
+        for arg in args:
+            fraction = _parse_fraction(arg)
+            if fraction:
+                fractions.append(fraction)
+                total_weight += fraction.weight
+
+    except ValueError:
+        logger.debug(f"Invalid {args} configuration")
+        return None
 
     range_end: float = 0
     for fraction in fractions:
         range_end += fraction.weight * 100 / total_weight
         if bucket < range_end:
             return fraction.variant
-
     return None
 
 
-def _parse_fraction(arg: JsonLogicArg) -> typing.Optional[Fraction]:
-    if not isinstance(arg, (tuple, list)) or not arg:
-        logger.error(
+def _parse_fraction(arg: JsonLogicArg) -> Fraction:
+    if not isinstance(arg, (tuple, list)) or not arg or len(arg) > 2:
+        raise ValueError(
             "Fractional variant weights must be (str, int) tuple or [str] list"
         )
-        return None
 
     if not isinstance(arg[0], str):
-        logger.error(
+        raise ValueError(
             "Fractional variant identifier (first element) isn't of type 'str'"
         )
-        return None
 
     if len(arg) >= 2 and not isinstance(arg[1], int):
-        logger.error(
+        raise ValueError(
             "Fractional variant weight value (second element) isn't of type 'int'"
         )
-        return None
 
     fraction = Fraction(variant=arg[0])
     if len(arg) >= 2:

providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/targeting.py

@@ -0,0 +1,35 @@
+import time
+import typing
+
+from json_logic import builtins, jsonLogic  # type: ignore[import-untyped]
+from json_logic.types import JsonValue  # type: ignore[import-untyped]
+
+from openfeature.evaluation_context import EvaluationContext
+
+from .custom_ops import (
+    ends_with,
+    fractional,
+    sem_ver,
+    starts_with,
+)
+
+OPERATORS = {
+    **builtins.BUILTINS,
+    "fractional": fractional,
+    "starts_with": starts_with,
+    "ends_with": ends_with,
+    "sem_ver": sem_ver,
+}
+
+
+def targeting(
+    key: str,
+    targeting: dict,
+    evaluation_context: typing.Optional[EvaluationContext] = None,
+) -> JsonValue:
+    json_logic_context = evaluation_context.attributes if evaluation_context else {}
+    json_logic_context["$flagd"] = {"flagKey": key, "timestamp": int(time.time())}
+    json_logic_context["targetingKey"] = (
+        evaluation_context.targeting_key if evaluation_context else None
+    )
+    return jsonLogic(targeting, json_logic_context, OPERATORS)

providers/openfeature-provider-flagd/tests/test_targeting.py

@@ -0,0 +1,250 @@
+import time
+import unittest
+from math import floor
+
+import pytest
+from json_logic import builtins, jsonLogic  # type: ignore[import-untyped]
+
+from openfeature.contrib.provider.flagd.resolvers.process.custom_ops import (
+    ends_with,
+    fractional,
+    sem_ver,
+    starts_with,
+)
+from openfeature.contrib.provider.flagd.resolvers.process.targeting import targeting
+from openfeature.evaluation_context import EvaluationContext
+
+OPERATORS = {
+    **builtins.BUILTINS,
+    "fractional": fractional,
+    "starts_with": starts_with,
+    "ends_with": ends_with,
+    "sem_ver": sem_ver,
+}
+
+flag_key = "flagKey"
+
+
+class BasicTests(unittest.TestCase):
+    def test_should_inject_flag_key_as_a_property(self):
+        rule = {"===": [{"var": "$flagd.flagKey"}, flag_key]}
+
+        result = targeting(flag_key, rule)
+
+        assert result
+
+    def test_should_inject_current_timestamp_as_a_property(self):
+        ts = floor(time.time() / 1000)
+
+        rule = {">=": [{"var": "$flagd.timestamp"}, ts]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_override_injected_properties_if_already_present_in_context(self):
+        rule = {"===": [{"var": "$flagd.flagKey"}, flag_key]}
+
+        ctx = {
+            "$flagd": {
+                "flagKey": "someOtherFlag",
+            },
+        }
+
+        assert targeting(flag_key, rule, EvaluationContext(attributes=ctx))
+
+
+class StringComparisonOperator(unittest.TestCase):
+    def test_should_evaluate_starts_with_calls(self):
+        rule = {"starts_with": [{"var": "email"}, "admin"]}
+        context = {"email": "admin@abc.com"}
+
+        assert targeting(flag_key, rule, EvaluationContext(attributes=context))
+
+    def test_should_evaluate_ends_with_calls(self):
+        rule = {"ends_with": [{"var": "email"}, "abc.com"]}
+        context = {"email": "admin@abc.com"}
+
+        assert targeting(flag_key, rule, EvaluationContext(attributes=context))
+
+    def test_missing_targeting(self):
+        rule = {"starts_with": [{"var": "email"}]}
+        context = {"email": "admin@abc.com"}
+
+        assert not targeting(flag_key, rule, EvaluationContext(attributes=context))
+
+    def test_non_string_variable(self):
+        rule = {"ends_with": [{"var": "number"}, "abc.com"]}
+        context = {"number": 11111}
+
+        assert not targeting(flag_key, rule, EvaluationContext(attributes=context))
+
+    def test_non_string_comparator(self):
+        rule = {"ends_with": [{"var": "email"}, 111111]}
+        context = {"email": "admin@abc.com"}
+
+        assert not targeting(flag_key, rule, EvaluationContext(attributes=context))
+
+
+@pytest.mark.skip(
+    "semvers are not working as expected, 'v' prefix is not valid within current implementation"
+)
+class SemVerOperator(unittest.TestCase):
+    def test_should_support_equal_operator(self):
+        rule = {"sem_ver": ["v1.2.3", "=", "1.2.3"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_support_neq_operator(self):
+        rule = {"sem_ver": ["v1.2.3", "!=", "1.2.4"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_support_lt_operator(self):
+        rule = {"sem_ver": ["v1.2.3", "<", "1.2.4"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_support_lte_operator(self):
+        rule = {"sem_ver": ["v1.2.3", "<=", "1.2.3"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_support_gte_operator(self):
+        rule = {"sem_ver": ["v1.2.3", ">=", "1.2.3"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_support_gt_operator(self):
+        rule = {"sem_ver": ["v1.2.4", ">", "1.2.3"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_support_major_comparison_operator(self):
+        rule = {"sem_ver": ["v1.2.3", "^", "v1.0.0"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_support_minor_comparison_operator(self):
+        rule = {"sem_ver": ["v5.0.3", "~", "v5.0.8"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_handle_unknown_operator(self):
+        rule = {"sem_ver": ["v1.0.0", "-", "v1.0.0"]}
+
+        assert targeting(flag_key, rule)
+
+    def test_should_handle_invalid_targetings(self):
+        rule = {"sem_ver": ["myVersion_1", "=", "myVersion_1"]}
+
+        assert not targeting(flag_key, rule)
+
+    def test_should_validate_targetings(self):
+        rule = {"sem_ver": ["myVersion_2", "+", "myVersion_1", "myVersion_1"]}
+
+        assert targeting(flag_key, rule)
+
+
+class FractionalOperator(unittest.TestCase):
+    def test_should_evaluate_valid_rule(self):
+        rule = {
+            "fractional": [
+                {"cat": [{"var": "$flagd.flagKey"}, {"var": "key"}]},
+                ["red", 50],
+                ["blue", 50],
+            ],
+        }
+
+        logic = targeting(
+            "flagA", rule, EvaluationContext(attributes={"key": "bucketKeyA"})
+        )
+        assert logic == "red"
+
+    def test_should_evaluate_valid_rule2(self):
+        rule = {
+            "fractional": [
+                {"cat": [{"var": "$flagd.flagKey"}, {"var": "key"}]},
+                ["red", 50],
+                ["blue", 50],
+            ],
+        }
+
+        logic = targeting(
+            "flagA", rule, EvaluationContext(attributes={"key": "bucketKeyB"})
+        )
+        assert logic == "blue"
+
+    def test_should_evaluate_valid_rule_with_targeting_key(self):
+        rule = {
+            "fractional": [
+                ["red", 50],
+                ["blue", 50],
+            ],
+        }
+
+        logic = targeting("flagA", rule, EvaluationContext(targeting_key="bucketKeyB"))
+        assert logic == "blue"
+
+    def test_should_evaluate_valid_rule_with_targeting_key_although_one_does_not_have_a_fraction(
+        self,
+    ):
+        rule = {
+            "fractional": [["red", 1], ["blue"]],
+        }
+
+        logic = targeting("flagA", rule, EvaluationContext(targeting_key="bucketKeyB"))
+        assert logic == "blue"
+
+    def test_should_return_null_if_targeting_key_is_missing(self):
+        rule = {
+            "fractional": [
+                ["red", 1],
+                ["blue", 1],
+            ],
+        }
+
+        logic = jsonLogic(rule, {}, OPERATORS)
+        assert logic is None
+
+    def test_bucket_sum_with_sum_bigger_than_100(self):
+        rule = {
+            "fractional": [
+                ["red", 55],
+                ["blue", 55],
+            ],
+        }
+
+        logic = targeting("flagA", rule, EvaluationContext(targeting_key="key"))
+        assert logic == "blue"
+
+    def test_bucket_sum_with_sum_lower_than_100(self):
+        rule = {
+            "fractional": [
+                ["red", 45],
+                ["blue", 45],
+            ],
+        }
+
+        logic = targeting("flagA", rule, EvaluationContext(targeting_key="key"))
+        assert logic == "blue"
+
+    def test_buckets_properties_to_have_variant_and_fraction(self):
+        rule = {
+            "fractional": [
+                ["red", 50],
+                [100, 50],
+            ],
+        }
+
+        logic = targeting("flagA", rule, EvaluationContext(targeting_key="key"))
+        assert logic is None
+
+    def test_buckets_properties_to_have_variant_and_fraction2(self):
+        rule = {
+            "fractional": [
+                ["red", 45, 1256],
+                ["blue", 4, 455],
+            ],
+        }
+
+        logic = targeting("flagA", rule, EvaluationContext(targeting_key="key"))
+        assert logic is None