From ae0e463090c0a0487d650994c203b6ab8b64ed89 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Thu, 14 Sep 2023 15:59:07 +0100 Subject: [PATCH 1/7] feat: evaluator module - Move segment rule evaluators to flag_engine.segments.evaluator module - Add missing runtime type checks --- flag_engine/segments/evaluator.py | 123 +++++++++- flag_engine/segments/models.py | 59 ----- .../unit/segments/test_segments_evaluator.py | 225 ++++++++++++++++-- tests/unit/segments/test_segments_models.py | 155 +----------- 4 files changed, 334 insertions(+), 228 deletions(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index bebd189d..2ba8f830 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -1,12 +1,24 @@ +import operator +import re import typing +from contextlib import suppress +import semver + +from flag_engine.environments.models import EnvironmentModel from flag_engine.identities.models import IdentityModel +from flag_engine.identities.traits.models import TraitModel +from flag_engine.identities.traits.types import TraitValue +from flag_engine.segments import constants +from flag_engine.segments.models import ( + SegmentConditionModel, + SegmentModel, + SegmentRuleModel, +) +from flag_engine.segments.types import ConditionOperator from flag_engine.utils.hashing import get_hashed_percentage_for_object_ids - -from ..environments.models import EnvironmentModel -from ..identities.traits.models import TraitModel -from . import constants -from .models import SegmentConditionModel, SegmentModel, SegmentRuleModel +from flag_engine.utils.semver import is_semver +from flag_engine.utils.types import get_casting_function def get_identity_segments( @@ -79,6 +91,7 @@ def _traits_match_segment_condition( identity_id: typing.Union[int, str], ) -> bool: if condition.operator == constants.PERCENTAGE_SPLIT: + assert condition.value float_value = float(condition.value) return ( get_hashed_percentage_for_object_ids([segment_id, identity_id]) @@ -95,4 +108,102 @@ def _traits_match_segment_condition( if condition.operator == constants.IS_SET: return trait is not None - return condition.matches_trait_value(trait.trait_value) if trait else False + return _matches_trait_value(condition, trait.trait_value) if trait else False + + +def _matches_trait_value( + condition: SegmentConditionModel, + trait_value: TraitValue, +) -> bool: + if match_func := MATCH_FUNCS_BY_OPERATOR.get(condition.operator): + return match_func(condition.value, trait_value) + + maybe_semver_trait_value: typing.Union[ + TraitValue, + semver.VersionInfo, + ] = trait_value + if operator_func := OPERATOR_FUNCS_BY_OPERATOR.get(condition.operator): + with suppress(TypeError, ValueError): + if isinstance(maybe_semver_trait_value, str) and is_semver(condition.value): + maybe_semver_trait_value = semver.VersionInfo.parse( + maybe_semver_trait_value, + ) + match_value = get_casting_function(maybe_semver_trait_value)( + condition.value, + ) + return operator_func( + maybe_semver_trait_value, + match_value, + ) + + return False + + +def _evaluate_not_contains( + segment_value: typing.Optional[str], + trait_value: TraitValue, +) -> bool: + return isinstance(trait_value, str) and str(segment_value) not in trait_value + + +def _evaluate_regex( + segment_value: typing.Optional[str], + trait_value: TraitValue, +) -> bool: + return ( + trait_value is not None + and re.compile(str(segment_value)).match(str(trait_value)) is not None + ) + + +def _evaluate_modulo( + segment_value: typing.Optional[str], + trait_value: TraitValue, +) -> bool: + if not isinstance(trait_value, (int, float)): + return False + + if segment_value is None: + return False + + try: + divisor_part, remainder_part = segment_value.split("|") + divisor = float(divisor_part) + remainder = float(remainder_part) + except ValueError: + return False + + return trait_value % divisor == remainder + + +def _evaluate_in(segment_value: typing.Optional[str], trait_value: TraitValue) -> bool: + if segment_value: + if isinstance(trait_value, str): + return trait_value in segment_value.split(",") + elif isinstance(trait_value, int) and not any( + trait_value is x for x in (False, True) + ): + return str(trait_value) in segment_value.split(",") + return False + + +MATCH_FUNCS_BY_OPERATOR: typing.Dict[ + ConditionOperator, typing.Callable[[typing.Optional[str], TraitValue], bool] +] = { + constants.NOT_CONTAINS: _evaluate_not_contains, + constants.REGEX: _evaluate_regex, + constants.MODULO: _evaluate_modulo, + constants.IN: _evaluate_in, +} + +OPERATOR_FUNCS_BY_OPERATOR: typing.Dict[ + ConditionOperator, typing.Callable[..., bool] +] = { + constants.EQUAL: operator.eq, + constants.GREATER_THAN: operator.gt, + constants.GREATER_THAN_INCLUSIVE: operator.ge, + constants.LESS_THAN: operator.lt, + constants.LESS_THAN_INCLUSIVE: operator.le, + constants.NOT_EQUAL: operator.ne, + constants.CONTAINS: operator.contains, +} diff --git a/flag_engine/segments/models.py b/flag_engine/segments/models.py index 96925b16..2f686147 100644 --- a/flag_engine/segments/models.py +++ b/flag_engine/segments/models.py @@ -1,15 +1,10 @@ -import re import typing -from contextlib import suppress -import semver from pydantic import BaseModel, Field from flag_engine.features.models import FeatureStateModel from flag_engine.segments import constants from flag_engine.segments.types import ConditionOperator, RuleType -from flag_engine.utils.semver import is_semver -from flag_engine.utils.types import get_casting_function class SegmentConditionModel(BaseModel): @@ -24,60 +19,6 @@ class SegmentConditionModel(BaseModel): value: typing.Optional[str] = None property_: typing.Optional[str] = None - def matches_trait_value(self, trait_value: typing.Any) -> bool: - # TODO: move this logic to the evaluator module - with suppress(ValueError): - if type(self.value) is str and is_semver(self.value): - trait_value = semver.VersionInfo.parse(trait_value) - if self.operator in self._EXCEPTION_OPERATOR_METHODS: - evaluator_function = getattr( - self, self._EXCEPTION_OPERATOR_METHODS.get(self.operator) - ) - return evaluator_function(trait_value) - - matching_function_name = { - constants.EQUAL: "__eq__", - constants.GREATER_THAN: "__gt__", - constants.GREATER_THAN_INCLUSIVE: "__ge__", - constants.LESS_THAN: "__lt__", - constants.LESS_THAN_INCLUSIVE: "__le__", - constants.NOT_EQUAL: "__ne__", - constants.CONTAINS: "__contains__", - }.get(self.operator) - matching_function = getattr( - trait_value, matching_function_name, lambda v: False - ) - to_same_type_as_trait_value = get_casting_function(trait_value) - return matching_function(to_same_type_as_trait_value(self.value)) - - return False - - def evaluate_not_contains(self, trait_value: typing.Iterable) -> bool: - return self.value not in trait_value - - def evaluate_regex(self, trait_value: str) -> bool: - return ( - trait_value is not None - and re.compile(str(self.value)).match(str(trait_value)) is not None - ) - - def evaluate_modulo(self, trait_value: typing.Union[str, int, float, bool]) -> bool: - if type(trait_value) not in (int, float): - return False - try: - divisor, remainder = self.value.split("|") - divisor = float(divisor) - remainder = float(remainder) - except ValueError: - return False - return trait_value % divisor == remainder - - def evaluate_in(self, trait_value) -> bool: - try: - return str(trait_value) in self.value.split(",") - except AttributeError: - return False - class SegmentRuleModel(BaseModel): type: RuleType diff --git a/tests/unit/segments/test_segments_evaluator.py b/tests/unit/segments/test_segments_evaluator.py index 3f3faf36..c7d94d7a 100644 --- a/tests/unit/segments/test_segments_evaluator.py +++ b/tests/unit/segments/test_segments_evaluator.py @@ -1,21 +1,23 @@ +import typing + import pytest +from pytest_lazyfixture import lazy_fixture +from pytest_mock import MockerFixture from flag_engine.identities.models import IdentityModel from flag_engine.identities.traits.models import TraitModel -from flag_engine.segments.constants import ( - ALL_RULE, - IS_NOT_SET, - IS_SET, - PERCENTAGE_SPLIT, +from flag_engine.segments import constants +from flag_engine.segments.evaluator import ( + _matches_trait_value, + evaluate_identity_in_segment, ) -from flag_engine.segments.evaluator import evaluate_identity_in_segment from flag_engine.segments.models import ( SegmentConditionModel, SegmentModel, SegmentRuleModel, ) - -from .fixtures import ( +from flag_engine.segments.types import ConditionOperator +from tests.unit.segments.fixtures import ( empty_segment, segment_conditions_and_nested_rules, segment_multiple_conditions_all, @@ -106,7 +108,11 @@ ), ), ) -def test_identity_in_segment(segment, identity_traits, expected_result): +def test_identity_in_segment( + segment: SegmentModel, + identity_traits: typing.List[TraitModel], + expected_result: bool, +) -> None: identity = IdentityModel( identifier="foo", identity_traits=identity_traits, @@ -121,13 +127,19 @@ def test_identity_in_segment(segment, identity_traits, expected_result): ((10, 1, True), (100, 50, True), (0, 1, False), (10, 20, False)), ) def test_identity_in_segment_percentage_split( - mocker, identity, segment_split_value, identity_hashed_percentage, expected_result -): + mocker: MockerFixture, + identity: IdentityModel, + segment_split_value: int, + identity_hashed_percentage: int, + expected_result: bool, +) -> None: # Given percentage_split_condition = SegmentConditionModel( - operator=PERCENTAGE_SPLIT, value=str(segment_split_value) + operator=constants.PERCENTAGE_SPLIT, value=str(segment_split_value) + ) + rule = SegmentRuleModel( + type=constants.ALL_RULE, conditions=[percentage_split_condition] ) - rule = SegmentRuleModel(type=ALL_RULE, conditions=[percentage_split_condition]) segment = SegmentModel(id=1, name="% split", rules=[rule]) mock_get_hashed_percentage = mocker.patch( @@ -145,21 +157,27 @@ def test_identity_in_segment_percentage_split( @pytest.mark.parametrize( "operator, property_, expected_result", ( - (IS_SET, pytest.lazy_fixture("segment_condition_property"), True), - (IS_NOT_SET, pytest.lazy_fixture("segment_condition_property"), False), - (IS_SET, "random_property", False), - (IS_NOT_SET, "random_property", True), + (constants.IS_SET, lazy_fixture("segment_condition_property"), True), + (constants.IS_NOT_SET, lazy_fixture("segment_condition_property"), False), + (constants.IS_SET, "random_property", False), + (constants.IS_NOT_SET, "random_property", True), ), ) def test_identity_in_segment_is_set_and_is_not_set( - mocker, identity_in_segment, operator, property_, expected_result -): + identity_in_segment: IdentityModel, + operator: ConditionOperator, + property_: str, + expected_result: bool, +) -> None: # Given segment_condition_model = SegmentConditionModel( operator=operator, property_=property_, ) - rule = SegmentRuleModel(type=ALL_RULE, conditions=[segment_condition_model]) + rule = SegmentRuleModel( + type=constants.ALL_RULE, + conditions=[segment_condition_model], + ) segment = SegmentModel(id=1, name="segment model", rules=[rule]) # When @@ -167,3 +185,170 @@ def test_identity_in_segment_is_set_and_is_not_set( # Then assert result is expected_result + + +@pytest.mark.parametrize( + "operator, trait_value, condition_value, expected_result", + ( + (constants.EQUAL, "bar", "bar", True), + (constants.EQUAL, "bar", "baz", False), + (constants.EQUAL, 1, "1", True), + (constants.EQUAL, 1, "not_an_int", False), + (constants.EQUAL, 1, "2", False), + (constants.EQUAL, True, "True", True), + (constants.EQUAL, False, "False", True), + (constants.EQUAL, False, "True", False), + (constants.EQUAL, True, "False", False), + (constants.EQUAL, 1.23, "1.23", True), + (constants.EQUAL, 1.23, "not_a_float", False), + (constants.EQUAL, 1.23, "4.56", False), + (constants.GREATER_THAN, 2, "1", True), + (constants.GREATER_THAN, 1, "1", False), + (constants.GREATER_THAN, 0, "1", False), + (constants.GREATER_THAN, 2.1, "2.0", True), + (constants.GREATER_THAN, 2.1, "2.1", False), + (constants.GREATER_THAN, 2.0, "2.1", False), + (constants.GREATER_THAN_INCLUSIVE, 2, "1", True), + (constants.GREATER_THAN_INCLUSIVE, 1, "1", True), + (constants.GREATER_THAN_INCLUSIVE, 0, "1", False), + (constants.GREATER_THAN_INCLUSIVE, 2.1, "2.0", True), + (constants.GREATER_THAN_INCLUSIVE, 2.1, "2.1", True), + (constants.GREATER_THAN_INCLUSIVE, 2.0, "2.1", False), + (constants.LESS_THAN, 1, "2", True), + (constants.LESS_THAN, 1, "1", False), + (constants.LESS_THAN, 1, "0", False), + (constants.LESS_THAN, 2.0, "2.1", True), + (constants.LESS_THAN, 2.1, "2.1", False), + (constants.LESS_THAN, 2.1, "2.0", False), + (constants.LESS_THAN_INCLUSIVE, 1, "2", True), + (constants.LESS_THAN_INCLUSIVE, 1, "1", True), + (constants.LESS_THAN_INCLUSIVE, 1, "0", False), + (constants.LESS_THAN_INCLUSIVE, 2.0, "2.1", True), + (constants.LESS_THAN_INCLUSIVE, 2.1, "2.1", True), + (constants.LESS_THAN_INCLUSIVE, 2.1, "2.0", False), + (constants.NOT_EQUAL, "bar", "baz", True), + (constants.NOT_EQUAL, "bar", "bar", False), + (constants.NOT_EQUAL, 1, "2", True), + (constants.NOT_EQUAL, 1, "1", False), + (constants.NOT_EQUAL, True, "False", True), + (constants.NOT_EQUAL, False, "True", True), + (constants.NOT_EQUAL, False, "False", False), + (constants.NOT_EQUAL, True, "True", False), + (constants.CONTAINS, "bar", "b", True), + (constants.CONTAINS, "bar", "bar", True), + (constants.CONTAINS, "bar", "baz", False), + (constants.NOT_CONTAINS, "bar", "b", False), + (constants.NOT_CONTAINS, "bar", "bar", False), + (constants.NOT_CONTAINS, "bar", "baz", True), + (constants.REGEX, "foo", r"[a-z]+", True), + (constants.REGEX, "FOO", r"[a-z]+", False), + (constants.REGEX, "1.2.3", r"\d", True), + (constants.REGEX, 1, r"\d", True), + (constants.REGEX, None, r"[a-z]", False), + (constants.IN, "foo", "", False), + (constants.IN, "foo", "foo,bar", True), + (constants.IN, "bar", "foo,bar", True), + (constants.IN, "foo", "foo", True), + (constants.IN, 1, "1,2,3,4", True), + (constants.IN, 1, "", False), + (constants.IN, 1, "1", True), + (constants.IN, 1, None, False), + ), +) +def test_segment_condition_matches_trait_value( + operator: ConditionOperator, + trait_value: typing.Union[None, int, str, float], + condition_value: typing.Optional[str], + expected_result: bool, +) -> None: + # Given + segment_condition = SegmentConditionModel( + operator=operator, + property_="foo", + value=condition_value, + ) + + # When + result = _matches_trait_value(segment_condition, trait_value) + + # Then + assert result == expected_result + + +@pytest.mark.parametrize( + "operator, trait_value, condition_value, expected_result", + [ + (constants.EQUAL, "1.0.0", "1.0.0:semver", True), + (constants.EQUAL, "not_a_semver", "1.0.0:semver", False), + (constants.EQUAL, "1.0.0", "1.0.1:semver", False), + (constants.NOT_EQUAL, "1.0.0", "1.0.0:semver", False), + (constants.NOT_EQUAL, "1.0.0", "1.0.1:semver", True), + (constants.GREATER_THAN, "1.0.1", "1.0.0:semver", True), + (constants.GREATER_THAN, "1.0.0", "1.0.0-beta:semver", True), + (constants.GREATER_THAN, "1.0.1", "1.2.0:semver", False), + (constants.GREATER_THAN, "1.0.1", "1.0.1:semver", False), + (constants.GREATER_THAN, "1.2.4", "1.2.3-pre.2+build.4:semver", True), + (constants.LESS_THAN, "1.0.0", "1.0.1:semver", True), + (constants.LESS_THAN, "1.0.0", "1.0.0:semver", False), + (constants.LESS_THAN, "1.0.1", "1.0.0:semver", False), + (constants.LESS_THAN, "1.0.0-rc.2", "1.0.0-rc.3:semver", True), + (constants.GREATER_THAN_INCLUSIVE, "1.0.1", "1.0.0:semver", True), + (constants.GREATER_THAN_INCLUSIVE, "1.0.1", "1.2.0:semver", False), + (constants.GREATER_THAN_INCLUSIVE, "1.0.1", "1.0.1:semver", True), + (constants.LESS_THAN_INCLUSIVE, "1.0.0", "1.0.1:semver", True), + (constants.LESS_THAN_INCLUSIVE, "1.0.0", "1.0.0:semver", True), + (constants.LESS_THAN_INCLUSIVE, "1.0.1", "1.0.0:semver", False), + ], +) +def test_segment_condition_matches_trait_value_for_semver( + operator: ConditionOperator, + trait_value: str, + condition_value: str, + expected_result: bool, +) -> None: + # Given + segment_condition = SegmentConditionModel( + operator=operator, + property_="version", + value=condition_value, + ) + + # When + result = _matches_trait_value(segment_condition, trait_value) + + # Then + assert result == expected_result + + +@pytest.mark.parametrize( + "trait_value, condition_value, expected_result", + [ + (1, "2|0", False), + (2, "2|0", True), + (3, "2|0", False), + (34.2, "4|3", False), + (35.0, "4|3", True), + ("dummy", "3|0", False), + ("1.0.0", "3|0", False), + (False, "1|3", False), + (1, "invalid|value", False), + (1, None, False), + ], +) +def test_segment_condition_matches_trait_value_for_modulo( + trait_value: typing.Union[int, float, str, bool], + condition_value: typing.Optional[str], + expected_result: bool, +) -> None: + # Given + segment_condition = SegmentConditionModel( + operator=constants.MODULO, + property_="version", + value=condition_value, + ) + + # When + result = _matches_trait_value(segment_condition, trait_value) + + # Then + assert result == expected_result diff --git a/tests/unit/segments/test_segments_models.py b/tests/unit/segments/test_segments_models.py index 87a14096..8e1803c7 100644 --- a/tests/unit/segments/test_segments_models.py +++ b/tests/unit/segments/test_segments_models.py @@ -1,147 +1,10 @@ +import typing + import pytest from flag_engine.segments import constants -from flag_engine.segments.models import SegmentConditionModel, SegmentRuleModel - - -@pytest.mark.parametrize( - "operator, trait_value, condition_value, expected_result", - ( - (constants.EQUAL, "bar", "bar", True), - (constants.EQUAL, "bar", "baz", False), - (constants.EQUAL, 1, "1", True), - (constants.EQUAL, 1, "not_an_int", False), - (constants.EQUAL, 1, "2", False), - (constants.EQUAL, True, "True", True), - (constants.EQUAL, False, "False", True), - (constants.EQUAL, False, "True", False), - (constants.EQUAL, True, "False", False), - (constants.EQUAL, 1.23, "1.23", True), - (constants.EQUAL, 1.23, "not_a_float", False), - (constants.EQUAL, 1.23, "4.56", False), - (constants.GREATER_THAN, 2, "1", True), - (constants.GREATER_THAN, 1, "1", False), - (constants.GREATER_THAN, 0, "1", False), - (constants.GREATER_THAN, 2.1, "2.0", True), - (constants.GREATER_THAN, 2.1, "2.1", False), - (constants.GREATER_THAN, 2.0, "2.1", False), - (constants.GREATER_THAN_INCLUSIVE, 2, "1", True), - (constants.GREATER_THAN_INCLUSIVE, 1, "1", True), - (constants.GREATER_THAN_INCLUSIVE, 0, "1", False), - (constants.GREATER_THAN_INCLUSIVE, 2.1, "2.0", True), - (constants.GREATER_THAN_INCLUSIVE, 2.1, "2.1", True), - (constants.GREATER_THAN_INCLUSIVE, 2.0, "2.1", False), - (constants.LESS_THAN, 1, "2", True), - (constants.LESS_THAN, 1, "1", False), - (constants.LESS_THAN, 1, "0", False), - (constants.LESS_THAN, 2.0, "2.1", True), - (constants.LESS_THAN, 2.1, "2.1", False), - (constants.LESS_THAN, 2.1, "2.0", False), - (constants.LESS_THAN_INCLUSIVE, 1, "2", True), - (constants.LESS_THAN_INCLUSIVE, 1, "1", True), - (constants.LESS_THAN_INCLUSIVE, 1, "0", False), - (constants.LESS_THAN_INCLUSIVE, 2.0, "2.1", True), - (constants.LESS_THAN_INCLUSIVE, 2.1, "2.1", True), - (constants.LESS_THAN_INCLUSIVE, 2.1, "2.0", False), - (constants.NOT_EQUAL, "bar", "baz", True), - (constants.NOT_EQUAL, "bar", "bar", False), - (constants.NOT_EQUAL, 1, "2", True), - (constants.NOT_EQUAL, 1, "1", False), - (constants.NOT_EQUAL, True, "False", True), - (constants.NOT_EQUAL, False, "True", True), - (constants.NOT_EQUAL, False, "False", False), - (constants.NOT_EQUAL, True, "True", False), - (constants.CONTAINS, "bar", "b", True), - (constants.CONTAINS, "bar", "bar", True), - (constants.CONTAINS, "bar", "baz", False), - (constants.NOT_CONTAINS, "bar", "b", False), - (constants.NOT_CONTAINS, "bar", "bar", False), - (constants.NOT_CONTAINS, "bar", "baz", True), - (constants.REGEX, "foo", r"[a-z]+", True), - (constants.REGEX, "FOO", r"[a-z]+", False), - (constants.REGEX, "1.2.3", r"\d", True), - (constants.REGEX, 1, r"\d", True), - (constants.REGEX, None, r"[a-z]", False), - (constants.IN, "foo", "", False), - (constants.IN, "foo", "foo,bar", True), - (constants.IN, "bar", "foo,bar", True), - (constants.IN, "foo", "foo", True), - (constants.IN, 1, "1,2,3,4", True), - (constants.IN, 1, "", False), - (constants.IN, 1, "1", True), - (constants.IN, 1, None, False), - ), -) -def test_segment_condition_matches_trait_value( - operator, trait_value, condition_value, expected_result -): - assert ( - SegmentConditionModel( - operator=operator, property_="foo", value=condition_value - ).matches_trait_value(trait_value=trait_value) - == expected_result - ) - - -@pytest.mark.parametrize( - "operator, trait_value, condition_value, expected_result", - [ - (constants.EQUAL, "1.0.0", "1.0.0:semver", True), - (constants.EQUAL, "not_a_semver", "1.0.0:semver", False), - (constants.EQUAL, "1.0.0", "1.0.1:semver", False), - (constants.NOT_EQUAL, "1.0.0", "1.0.0:semver", False), - (constants.NOT_EQUAL, "1.0.0", "1.0.1:semver", True), - (constants.GREATER_THAN, "1.0.1", "1.0.0:semver", True), - (constants.GREATER_THAN, "1.0.0", "1.0.0-beta:semver", True), - (constants.GREATER_THAN, "1.0.1", "1.2.0:semver", False), - (constants.GREATER_THAN, "1.0.1", "1.0.1:semver", False), - (constants.GREATER_THAN, "1.2.4", "1.2.3-pre.2+build.4:semver", True), - (constants.LESS_THAN, "1.0.0", "1.0.1:semver", True), - (constants.LESS_THAN, "1.0.0", "1.0.0:semver", False), - (constants.LESS_THAN, "1.0.1", "1.0.0:semver", False), - (constants.LESS_THAN, "1.0.0-rc.2", "1.0.0-rc.3:semver", True), - (constants.GREATER_THAN_INCLUSIVE, "1.0.1", "1.0.0:semver", True), - (constants.GREATER_THAN_INCLUSIVE, "1.0.1", "1.2.0:semver", False), - (constants.GREATER_THAN_INCLUSIVE, "1.0.1", "1.0.1:semver", True), - (constants.LESS_THAN_INCLUSIVE, "1.0.0", "1.0.1:semver", True), - (constants.LESS_THAN_INCLUSIVE, "1.0.0", "1.0.0:semver", True), - (constants.LESS_THAN_INCLUSIVE, "1.0.1", "1.0.0:semver", False), - ], -) -def test_segment_condition_matches_trait_value_for_semver( - operator, trait_value, condition_value, expected_result -): - assert ( - SegmentConditionModel( - operator=operator, property_="version", value=condition_value - ).matches_trait_value(trait_value=trait_value) - is expected_result - ) - - -@pytest.mark.parametrize( - "trait_value, condition_value, expected_result", - [ - (1, "2|0", False), - (2, "2|0", True), - (3, "2|0", False), - (34.2, "4|3", False), - (35.0, "4|3", True), - ("dummy", "3|0", False), - ("1.0.0", "3|0", False), - (False, "1|3", False), - (1, "invalid|value", False), - ], -) -def test_segment_condition_matches_trait_value_for_modulo( - trait_value, condition_value, expected_result -): - assert ( - SegmentConditionModel( - operator=constants.MODULO, property_="foo", value=condition_value - ).matches_trait_value(trait_value=trait_value) - is expected_result - ) +from flag_engine.segments.models import SegmentRuleModel +from flag_engine.segments.types import RuleType @pytest.mark.parametrize( @@ -154,7 +17,10 @@ def test_segment_condition_matches_trait_value_for_modulo( ([True, True], False), ), ) -def test_segment_rule_none(iterable, expected_result): +def test_segment_rule_none( + iterable: typing.List[bool], + expected_result: bool, +) -> None: assert SegmentRuleModel.none(iterable) is expected_result @@ -166,5 +32,8 @@ def test_segment_rule_none(iterable, expected_result): (constants.NONE_RULE, SegmentRuleModel.none), ), ) -def test_segment_rule_matching_function(rule_type, expected_function): +def test_segment_rule_matching_function( + rule_type: RuleType, + expected_function: typing.Callable[[typing.Iterable[object]], bool], +) -> None: assert SegmentRuleModel(type=rule_type).matching_function == expected_function From 497d42486643d8f6a6e5d6615353f6023ee88287 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 15 Sep 2023 17:54:09 +0100 Subject: [PATCH 2/7] fix: more readable code --- flag_engine/segments/evaluator.py | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 2ba8f830..0480b7ae 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -118,23 +118,14 @@ def _matches_trait_value( if match_func := MATCH_FUNCS_BY_OPERATOR.get(condition.operator): return match_func(condition.value, trait_value) - maybe_semver_trait_value: typing.Union[ - TraitValue, - semver.VersionInfo, - ] = trait_value if operator_func := OPERATOR_FUNCS_BY_OPERATOR.get(condition.operator): with suppress(TypeError, ValueError): - if isinstance(maybe_semver_trait_value, str) and is_semver(condition.value): - maybe_semver_trait_value = semver.VersionInfo.parse( - maybe_semver_trait_value, + if isinstance(trait_value, str) and is_semver(condition.value): + trait_value = semver.VersionInfo.parse( + trait_value, ) - match_value = get_casting_function(maybe_semver_trait_value)( - condition.value, - ) - return operator_func( - maybe_semver_trait_value, - match_value, - ) + match_value = get_casting_function(trait_value)(condition.value) + return operator_func(trait_value, match_value) return False From 2d549845e1ab4340e45277bcb81c1203b4d5b7ad Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 15 Sep 2023 18:18:02 +0100 Subject: [PATCH 3/7] feat: unify match function storage --- flag_engine/segments/evaluator.py | 49 ++++++++++++++++++------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 0480b7ae..69c7f3e7 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -2,6 +2,7 @@ import re import typing from contextlib import suppress +from functools import wraps import semver @@ -118,15 +119,6 @@ def _matches_trait_value( if match_func := MATCH_FUNCS_BY_OPERATOR.get(condition.operator): return match_func(condition.value, trait_value) - if operator_func := OPERATOR_FUNCS_BY_OPERATOR.get(condition.operator): - with suppress(TypeError, ValueError): - if isinstance(trait_value, str) and is_semver(condition.value): - trait_value = semver.VersionInfo.parse( - trait_value, - ) - match_value = get_casting_function(trait_value)(condition.value) - return operator_func(trait_value, match_value) - return False @@ -178,6 +170,26 @@ def _evaluate_in(segment_value: typing.Optional[str], trait_value: TraitValue) - return False +def _trait_value_typed( + func: typing.Callable[..., bool], +) -> typing.Callable[[typing.Optional[str], TraitValue], bool]: + @wraps(func) + def inner( + segment_value: typing.Optional[str], + trait_value: TraitValue, + ) -> bool: + with suppress(TypeError, ValueError): + if isinstance(trait_value, str) and is_semver(segment_value): + trait_value = semver.VersionInfo.parse( + trait_value, + ) + match_value = get_casting_function(trait_value)(segment_value) + return func(trait_value, match_value) + return False + + return inner + + MATCH_FUNCS_BY_OPERATOR: typing.Dict[ ConditionOperator, typing.Callable[[typing.Optional[str], TraitValue], bool] ] = { @@ -185,16 +197,11 @@ def _evaluate_in(segment_value: typing.Optional[str], trait_value: TraitValue) - constants.REGEX: _evaluate_regex, constants.MODULO: _evaluate_modulo, constants.IN: _evaluate_in, -} - -OPERATOR_FUNCS_BY_OPERATOR: typing.Dict[ - ConditionOperator, typing.Callable[..., bool] -] = { - constants.EQUAL: operator.eq, - constants.GREATER_THAN: operator.gt, - constants.GREATER_THAN_INCLUSIVE: operator.ge, - constants.LESS_THAN: operator.lt, - constants.LESS_THAN_INCLUSIVE: operator.le, - constants.NOT_EQUAL: operator.ne, - constants.CONTAINS: operator.contains, + constants.EQUAL: _trait_value_typed(operator.eq), + constants.GREATER_THAN: _trait_value_typed(operator.gt), + constants.GREATER_THAN_INCLUSIVE: _trait_value_typed(operator.ge), + constants.LESS_THAN: _trait_value_typed(operator.lt), + constants.LESS_THAN_INCLUSIVE: _trait_value_typed(operator.le), + constants.NOT_EQUAL: _trait_value_typed(operator.ne), + constants.CONTAINS: _trait_value_typed(operator.contains), } From 06e1cf9721ee9239e377e7db71a0cee7cad0fb8c Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 15 Sep 2023 18:25:24 +0100 Subject: [PATCH 4/7] fix: correctness --- flag_engine/segments/evaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 69c7f3e7..fce72828 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -163,7 +163,7 @@ def _evaluate_in(segment_value: typing.Optional[str], trait_value: TraitValue) - if segment_value: if isinstance(trait_value, str): return trait_value in segment_value.split(",") - elif isinstance(trait_value, int) and not any( + if isinstance(trait_value, int) and not any( trait_value is x for x in (False, True) ): return str(trait_value) in segment_value.split(",") From f96396fa66bc7c50ea3d1f2e17b403d69cdde87e Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 15 Sep 2023 18:32:44 +0100 Subject: [PATCH 5/7] feat: add segment value types in tests --- tests/unit/segments/test_segments_evaluator.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/unit/segments/test_segments_evaluator.py b/tests/unit/segments/test_segments_evaluator.py index c7d94d7a..fddd6c0e 100644 --- a/tests/unit/segments/test_segments_evaluator.py +++ b/tests/unit/segments/test_segments_evaluator.py @@ -202,30 +202,35 @@ def test_identity_in_segment_is_set_and_is_not_set( (constants.EQUAL, 1.23, "1.23", True), (constants.EQUAL, 1.23, "not_a_float", False), (constants.EQUAL, 1.23, "4.56", False), + (constants.EQUAL, 2, "not_an_number", False), (constants.GREATER_THAN, 2, "1", True), (constants.GREATER_THAN, 1, "1", False), (constants.GREATER_THAN, 0, "1", False), (constants.GREATER_THAN, 2.1, "2.0", True), (constants.GREATER_THAN, 2.1, "2.1", False), (constants.GREATER_THAN, 2.0, "2.1", False), + (constants.GREATER_THAN, 2, "not_an_number", False), (constants.GREATER_THAN_INCLUSIVE, 2, "1", True), (constants.GREATER_THAN_INCLUSIVE, 1, "1", True), (constants.GREATER_THAN_INCLUSIVE, 0, "1", False), (constants.GREATER_THAN_INCLUSIVE, 2.1, "2.0", True), (constants.GREATER_THAN_INCLUSIVE, 2.1, "2.1", True), (constants.GREATER_THAN_INCLUSIVE, 2.0, "2.1", False), + (constants.GREATER_THAN_INCLUSIVE, 2, "not_an_number", False), (constants.LESS_THAN, 1, "2", True), (constants.LESS_THAN, 1, "1", False), (constants.LESS_THAN, 1, "0", False), (constants.LESS_THAN, 2.0, "2.1", True), (constants.LESS_THAN, 2.1, "2.1", False), (constants.LESS_THAN, 2.1, "2.0", False), + (constants.LESS_THAN, 2, "not_an_number", False), (constants.LESS_THAN_INCLUSIVE, 1, "2", True), (constants.LESS_THAN_INCLUSIVE, 1, "1", True), (constants.LESS_THAN_INCLUSIVE, 1, "0", False), (constants.LESS_THAN_INCLUSIVE, 2.0, "2.1", True), (constants.LESS_THAN_INCLUSIVE, 2.1, "2.1", True), (constants.LESS_THAN_INCLUSIVE, 2.1, "2.0", False), + (constants.LESS_THAN_INCLUSIVE, 2, "not_a_number", False), (constants.NOT_EQUAL, "bar", "baz", True), (constants.NOT_EQUAL, "bar", "bar", False), (constants.NOT_EQUAL, 1, "2", True), @@ -237,6 +242,7 @@ def test_identity_in_segment_is_set_and_is_not_set( (constants.CONTAINS, "bar", "b", True), (constants.CONTAINS, "bar", "bar", True), (constants.CONTAINS, "bar", "baz", False), + (constants.CONTAINS, "bar", 1, False), (constants.NOT_CONTAINS, "bar", "b", False), (constants.NOT_CONTAINS, "bar", "bar", False), (constants.NOT_CONTAINS, "bar", "baz", True), @@ -245,6 +251,7 @@ def test_identity_in_segment_is_set_and_is_not_set( (constants.REGEX, "1.2.3", r"\d", True), (constants.REGEX, 1, r"\d", True), (constants.REGEX, None, r"[a-z]", False), + (constants.REGEX, "foo", 12, False), (constants.IN, "foo", "", False), (constants.IN, "foo", "foo,bar", True), (constants.IN, "bar", "foo,bar", True), @@ -253,12 +260,13 @@ def test_identity_in_segment_is_set_and_is_not_set( (constants.IN, 1, "", False), (constants.IN, 1, "1", True), (constants.IN, 1, None, False), + (constants.IN, 1, None, False), ), ) def test_segment_condition_matches_trait_value( operator: ConditionOperator, trait_value: typing.Union[None, int, str, float], - condition_value: typing.Optional[str], + condition_value: object, expected_result: bool, ) -> None: # Given From abe4c4705ecda234458057b1af7266059fcd70f1 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 15 Sep 2023 18:42:09 +0100 Subject: [PATCH 6/7] fix: add missing test --- .../unit/segments/test_segments_evaluator.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unit/segments/test_segments_evaluator.py b/tests/unit/segments/test_segments_evaluator.py index fddd6c0e..eb08d7c8 100644 --- a/tests/unit/segments/test_segments_evaluator.py +++ b/tests/unit/segments/test_segments_evaluator.py @@ -283,6 +283,25 @@ def test_segment_condition_matches_trait_value( assert result == expected_result +def test_segment_condition__unsupported_operator__return_false( + mocker: MockerFixture, +) -> None: + # Given + mocker.patch("flag_engine.segments.evaluator.MATCH_FUNCS_BY_OPERATOR", new={}) + segment_condition = SegmentConditionModel( + operator=constants.EQUAL, + property_="x", + value="foo", + ) + trait_value = "foo" + + # When + result = _matches_trait_value(segment_condition, trait_value) + + # Then + assert result is False + + @pytest.mark.parametrize( "operator, trait_value, condition_value, expected_result", [ From 1d2b802897458588e0287ae90848160102b9b3d0 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Mon, 18 Sep 2023 09:45:57 +0100 Subject: [PATCH 7/7] fix: remove unused attribute --- flag_engine/segments/models.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/flag_engine/segments/models.py b/flag_engine/segments/models.py index 2f686147..de950b5b 100644 --- a/flag_engine/segments/models.py +++ b/flag_engine/segments/models.py @@ -8,13 +8,6 @@ class SegmentConditionModel(BaseModel): - _EXCEPTION_OPERATOR_METHODS = { - constants.NOT_CONTAINS: "evaluate_not_contains", - constants.REGEX: "evaluate_regex", - constants.MODULO: "evaluate_modulo", - constants.IN: "evaluate_in", - } - operator: ConditionOperator value: typing.Optional[str] = None property_: typing.Optional[str] = None