diff --git a/eppo_client/client.py b/eppo_client/client.py index 94dda31..8e0c65c 100644 --- a/eppo_client/client.py +++ b/eppo_client/client.py @@ -74,12 +74,15 @@ def get_numeric_assignment( default: float, subject_attributes: Optional[SubjectAttributes] = None, ) -> float: - return self.get_assignment_variation( - subject_key, - flag_key, - default, - subject_attributes, - VariationType.NUMERIC, + # convert to float in case we get an int + return float( + self.get_assignment_variation( + subject_key, + flag_key, + default, + subject_attributes, + VariationType.NUMERIC, + ) ) def get_boolean_assignment( @@ -182,6 +185,15 @@ def get_assignment_detail( result = self.__evaluator.evaluate_flag(flag, subject_key, subject_attributes) + if result.variation and not check_value_type_match( + expected_variation_type, result.variation.value + ): + logger.error( + "[Eppo SDK] Variation value does not have the correct type for the flag: " + f"{flag_key} and variation key {result.variation.key}" + ) + return None + assignment_event = { **(result.extra_logging if result else {}), "allocation": result.allocation_key if result else None, @@ -227,3 +239,20 @@ def check_type_match( expected_type: Optional[VariationType], actual_type: VariationType ): return expected_type is None or actual_type == expected_type + + +def check_value_type_match( + expected_type: Optional[VariationType], value: ValueType +) -> bool: + if expected_type is None: + return True + if expected_type in [VariationType.JSON, VariationType.STRING]: + return isinstance(value, str) + if expected_type == VariationType.INTEGER: + return isinstance(value, int) + if expected_type == VariationType.NUMERIC: + # we can convert int to float + return isinstance(value, float) or isinstance(value, int) + if expected_type == VariationType.BOOLEAN: + return isinstance(value, bool) + return False diff --git a/eppo_client/eval.py b/eppo_client/eval.py index 0bd9976..d8350bd 100644 --- a/eppo_client/eval.py +++ b/eppo_client/eval.py @@ -43,12 +43,8 @@ def evaluate_flag( if allocation.end_at and now > allocation.end_at: continue - # Skip allocations when none of the rules match - # So we look for (rule 1) OR (rule 2) OR (rule 3) etc. - # If there are no rules, then we always match - if not allocation.rules or any( - matches_rule(rule, {"id": subject_key, **subject_attributes}) - for rule in allocation.rules + if matches_rules( + allocation.rules, {"id": subject_key, **subject_attributes} ): for split in allocation.splits: # Split needs to match all shards @@ -86,6 +82,13 @@ def hash_key(salt: str, subject_key: str) -> str: return f"{salt}-{subject_key}" +def matches_rules(rules, subject_attributes): + # Skip allocations when none of the rules match + # So we look for (rule 1) OR (rule 2) OR (rule 3) etc. + # If there are no rules, then we always match + return not rules or any(matches_rule(rule, subject_attributes) for rule in rules) + + def none_result( flag_key: str, variation_type: VariationType, diff --git a/eppo_client/rules.py b/eppo_client/rules.py index fb2f9f0..bc7c969 100644 --- a/eppo_client/rules.py +++ b/eppo_client/rules.py @@ -17,6 +17,7 @@ class OperatorType(Enum): LT = "LT" ONE_OF = "ONE_OF" NOT_ONE_OF = "NOT_ONE_OF" + IS_NULL = "IS_NULL" class Condition(SdkBaseModel): @@ -40,6 +41,11 @@ def evaluate_condition( condition: Condition, subject_attributes: SubjectAttributes ) -> bool: subject_value = subject_attributes.get(condition.attribute, None) + if condition.operator == OperatorType.IS_NULL: + if condition.value: + return subject_value is None + return subject_value is not None + if subject_value is not None: if condition.operator == OperatorType.MATCHES: return isinstance(condition.value, str) and bool( diff --git a/test/client_test.py b/test/client_test.py index fa39f90..e9aeae0 100644 --- a/test/client_test.py +++ b/test/client_test.py @@ -5,7 +5,7 @@ import httpretty # type: ignore import pytest from eppo_client.assignment_logger import AssignmentLogger -from eppo_client.client import EppoClient, check_type_match +from eppo_client.client import EppoClient, check_type_match, check_value_type_match from eppo_client.config import Config from eppo_client.models import ( Allocation, @@ -270,3 +270,16 @@ def test_get_numeric_assignment_on_bool_feature_flag_should_return_none(test_cas def test_check_type_match(): assert check_type_match(VariationType.STRING, VariationType.STRING) assert check_type_match(None, VariationType.STRING) + + +def test_check_value_type_match(): + assert check_value_type_match(VariationType.STRING, "hello") + assert check_value_type_match(VariationType.INTEGER, 1) + assert check_value_type_match(VariationType.NUMERIC, 1.0) + assert check_value_type_match(VariationType.NUMERIC, 1) + assert check_value_type_match(VariationType.BOOLEAN, True) + assert check_value_type_match(VariationType.JSON, '{"hello": "world"}') + + assert not check_type_match(VariationType.STRING, 1) + assert not check_type_match(VariationType.INTEGER, 1.0) + assert not check_type_match(VariationType.BOOLEAN, "true") diff --git a/test/eval_test.py b/test/eval_test.py index 0c0d6e0..7eb3475 100644 --- a/test/eval_test.py +++ b/test/eval_test.py @@ -9,7 +9,13 @@ Split, Shard, ) -from eppo_client.eval import Evaluator, FlagEvaluation, is_in_shard_range, hash_key +from eppo_client.eval import ( + Evaluator, + FlagEvaluation, + is_in_shard_range, + hash_key, + matches_rules, +) from eppo_client.rules import Condition, OperatorType, Rule from eppo_client.sharders import DeterministicSharder, MD5Sharder @@ -424,6 +430,46 @@ def test_eval_after_alloc(mocker): assert result.variation is None +def test_matches_rules_empty(): + rules = [] + subject_attributes = {"size": 10} + assert matches_rules(rules, subject_attributes) + + +def test_matches_rules_with_conditions(): + rules = [ + Rule( + conditions=[ + Condition(attribute="size", operator=OperatorType.IS_NULL, value=True) + ] + ), + Rule( + conditions=[ + Condition( + attribute="country", operator=OperatorType.ONE_OF, value=["UK"] + ) + ] + ), + ] + subject_attributes_1 = {"size": None, "country": "US"} + subject_attributes_2 = {"size": 10, "country": "UK"} + subject_attributes_3 = {"size": 5, "country": "US"} + subject_attributes_4 = {"country": "US"} + + assert ( + matches_rules(rules, subject_attributes_1) is True + ), f"{subject_attributes_1} should match first rule" + assert ( + matches_rules(rules, subject_attributes_2) is True + ), f"{subject_attributes_2} should match second rule" + assert ( + matches_rules(rules, subject_attributes_3) is False + ), f"{subject_attributes_3} should not match any rule" + assert ( + matches_rules(rules, subject_attributes_4) is True + ), f"{subject_attributes_4} should match first rule" + + def test_seed(): assert hash_key("salt", "subject") == "salt-subject" diff --git a/test/rules_test.py b/test/rules_test.py index 23d90c8..4c67079 100644 --- a/test/rules_test.py +++ b/test/rules_test.py @@ -255,3 +255,21 @@ def test_one_of_operator_with_number(): assert not evaluate_condition(not_one_of_condition, {"number": 10}) assert evaluate_condition(not_one_of_condition, {"number": "11"}) assert evaluate_condition(not_one_of_condition, {"number": 11}) + + +def test_is_null_operator(): + is_null_condition = Condition( + operator=OperatorType.IS_NULL, value=True, attribute="size" + ) + assert evaluate_condition(is_null_condition, {"size": None}) + assert not evaluate_condition(is_null_condition, {"size": 10}) + assert evaluate_condition(is_null_condition, {}) + + +def test_is_not_null_operator(): + is_not_null_condition = Condition( + operator=OperatorType.IS_NULL, value=False, attribute="size" + ) + assert not evaluate_condition(is_not_null_condition, {"size": None}) + assert evaluate_condition(is_not_null_condition, {"size": 10}) + assert not evaluate_condition(is_not_null_condition, {})