Merge pull request #410 from splitio/async-engine-evaluator

Updated engine evaluator class

Author: chillaq

splitio/engine/evaluator.py

@@ -1,6 +1,5 @@
 """Split evaluator module."""
 import logging
-from splitio.models.grammar.condition import ConditionType
 from splitio.models.impressions import Label
 
 
@@ -13,38 +12,30 @@
 class Evaluator(object):  # pylint: disable=too-few-public-methods
     """Split Evaluator class."""
 
-    def __init__(self, feature_flag_storage, segment_storage, splitter):
+    def __init__(self, splitter):
         """
         Construct a Evaluator instance.
 
-        :param feature_flag_storage: feature_flag storage.
-        :type feature_flag_storage: splitio.storage.SplitStorage
-
-        :param segment_storage: Segment storage.
-        :type segment_storage: splitio.storage.SegmentStorage
+        :param splitter: partition object.
+        :type splitter: splitio.engine.splitters.Splitters
         """
-        self._feature_flag_storage = feature_flag_storage
-        self._segment_storage = segment_storage
         self._splitter = splitter
 
-    def _evaluate_treatment(self, feature_flag_name, matching_key, bucketing_key, attributes, feature_flag):
+    def _evaluate_treatment(self, feature_flag, matching_key, bucketing_key, condition_matchers):
         """
         Evaluate the user submitted data against a feature and return the resulting treatment.
 
-        :param feature_flag_name: The feature flag for which to get the treatment
-        :type feature:  str
+        :param feature_flag: Split object
+        :type feature_flag: splitio.models.splits.Split|None
 
         :param matching_key: The matching_key for which to get the treatment
         :type matching_key: str
 
         :param bucketing_key: The bucketing_key for which to get the treatment
         :type bucketing_key: str
 
-        :param attributes: An optional dictionary of attributes
-        :type attributes: dict
-
-        :param feature_flag: Split object
-        :type attributes: splitio.models.splits.Split|None
+        :param condition_matchers: array of condition matchers for passed feature_flag
+        :type bucketing_key: Dict
 
         :return: The treatment for the key and feature flag
         :rtype: object
@@ -54,19 +45,19 @@ def _evaluate_treatment(self, feature_flag_name, matching_key, bucketing_key, at
         _change_number = -1
 
         if feature_flag is None:
-            _LOGGER.warning('Unknown or invalid feature: %s', feature_flag_name)
+            _LOGGER.warning('Unknown or invalid feature: %s', feature_flag.name)
             label = Label.SPLIT_NOT_FOUND
         else:
             _change_number = feature_flag.change_number
             if feature_flag.killed:
                 label = Label.KILLED
                 _treatment = feature_flag.default_treatment
             else:
-                treatment, label = self._get_treatment_for_split(
+                treatment, label = self._get_treatment_for_feature_flag(
                     feature_flag,
                     matching_key,
                     bucketing_key,
-                    attributes
+                    condition_matchers
                 )
                 if treatment is None:
                     label = Label.NO_CONDITION_MATCHED
@@ -83,61 +74,58 @@ def _evaluate_treatment(self, feature_flag_name, matching_key, bucketing_key, at
             }
         }
 
-    def evaluate_feature(self, feature_flag_name, matching_key, bucketing_key, attributes=None):
+    def evaluate_feature(self, feature_flag, matching_key, bucketing_key, condition_matchers):
         """
         Evaluate the user submitted data against a feature and return the resulting treatment.
 
-        :param feature_flag_name: The feature flag for which to get the treatment
-        :type feature:  str
+        :param feature_flag: Split object
+        :type feature_flag: splitio.models.splits.Split|None
 
         :param matching_key: The matching_key for which to get the treatment
         :type matching_key: str
 
         :param bucketing_key: The bucketing_key for which to get the treatment
         :type bucketing_key: str
 
-        :param attributes: An optional dictionary of attributes
-        :type attributes: dict
+        :param condition_matchers: array of condition matchers for passed feature_flag
+        :type bucketing_key: Dict
 
         :return: The treatment for the key and split
         :rtype: object
         """
-        # Fetching Split definition
-        feature_flag = self._feature_flag_storage.get(feature_flag_name)
-
         # Calling evaluation
-        evaluation = self._evaluate_treatment(feature_flag_name, matching_key,
-                                              bucketing_key, attributes, feature_flag)
+        evaluation = self._evaluate_treatment(feature_flag, matching_key,
+                                              bucketing_key, condition_matchers)
 
         return evaluation
 
-    def evaluate_features(self, feature_flag_names, matching_key, bucketing_key, attributes=None):
+    def evaluate_features(self, feature_flags, matching_key, bucketing_key, condition_matchers):
         """
         Evaluate the user submitted data against multiple features and return the resulting
         treatment.
 
-        :param feature_flag_names: The feature flags for which to get the treatments
-        :type feature:  list(str)
+        :param feature_flags: array of Split objects
+        :type feature_flags: [splitio.models.splits.Split|None]
 
         :param matching_key: The matching_key for which to get the treatment
         :type matching_key: str
 
         :param bucketing_key: The bucketing_key for which to get the treatment
         :type bucketing_key: str
 
-        :param attributes: An optional dictionary of attributes
-        :type attributes: dict
+        :param condition_matchers: array of condition matchers for passed feature_flag
+        :type bucketing_key: Dict
 
         :return: The treatments for the key and feature flags
         :rtype: object
         """
         return {
-            feature_flag_name: self._evaluate_treatment(feature_flag_name, matching_key,
-                                              bucketing_key, attributes, feature_flag)
-            for (feature_flag_name, feature_flag) in self._feature_flag_storage.fetch_many(feature_flag_names).items()
+            feature_flag.name: self._evaluate_treatment(feature_flag, matching_key,
+                                              bucketing_key, condition_matchers)
+            for (feature_flag) in feature_flags
         }
 
-    def _get_treatment_for_split(self, feature_flag, matching_key, bucketing_key, attributes=None):
+    def _get_treatment_for_feature_flag(self, feature_flag, matching_key, bucketing_key, condition_matchers):
         """
         Evaluate the feature considering the conditions.
 
@@ -153,43 +141,17 @@ def _get_treatment_for_split(self, feature_flag, matching_key, bucketing_key, at
         :param bucketing_key: The key for which to get the treatment
         :type key: str
 
-        :param attributes: An optional dictionary of attributes
-        :type attributes: dict
+        :param condition_matchers: array of condition matchers for passed feature_flag
+        :type bucketing_key: Dict
 
         :return: The resulting treatment and label
         :rtype: tuple
         """
         if bucketing_key is None:
             bucketing_key = matching_key
 
-        roll_out = False
-
-        context = {
-            'segment_storage': self._segment_storage,
-            'evaluator': self,
-            'bucketing_key': bucketing_key
-        }
-
-        for condition in feature_flag.conditions:
-            if (not roll_out and
-                    condition.condition_type == ConditionType.ROLLOUT):
-                if feature_flag.traffic_allocation < 100:
-                    bucket = self._splitter.get_bucket(
-                        bucketing_key,
-                        feature_flag.traffic_allocation_seed,
-                        feature_flag.algo
-                    )
-                    if bucket > feature_flag.traffic_allocation:
-                        return feature_flag.default_treatment, Label.NOT_IN_SPLIT
-                roll_out = True
-
-            condition_matches = condition.matches(
-                matching_key,
-                attributes=attributes,
-                context=context
-            )
-
-            if condition_matches:
+        for condition_matcher, condition in condition_matchers:
+            if condition_matcher:
                 return self._splitter.get_treatment(
                     bucketing_key,
                     feature_flag.seed,

tests/engine/test_evaluator.py

@@ -5,32 +5,18 @@
 from splitio.models.grammar.condition import Condition, ConditionType
 from splitio.models.impressions import Label
 from splitio.engine import evaluator, splitters
-from splitio.storage import SplitStorage, SegmentStorage
-
 
 class EvaluatorTests(object):
     """Test evaluator behavior."""
 
     def _build_evaluator_with_mocks(self, mocker):
         """Build an evaluator with mocked dependencies."""
-        split_storage_mock = mocker.Mock(spec=SplitStorage)
         splitter_mock = mocker.Mock(spec=splitters.Splitter)
-        segment_storage_mock = mocker.Mock(spec=SegmentStorage)
         logger_mock = mocker.Mock(spec=logging.Logger)
-        e = evaluator.Evaluator(split_storage_mock, segment_storage_mock, splitter_mock)
+        e = evaluator.Evaluator(splitter_mock)
         evaluator._LOGGER = logger_mock
         return e
 
-    def test_evaluate_treatment_missing_split(self, mocker):
-        """Test that a missing split logs and returns CONTROL."""
-        e = self._build_evaluator_with_mocks(mocker)
-        e._feature_flag_storage.get.return_value = None
-        result = e.evaluate_feature('feature1', 'some_key', 'some_bucketing_key', {'attr1': 1})
-        assert result['configurations'] == None
-        assert result['treatment'] == evaluator.CONTROL
-        assert result['impression']['change_number'] == -1
-        assert result['impression']['label'] == Label.SPLIT_NOT_FOUND
-
     def test_evaluate_treatment_killed_split(self, mocker):
         """Test that a killed split returns the default treatment."""
         e = self._build_evaluator_with_mocks(mocker)
@@ -39,8 +25,7 @@ def test_evaluate_treatment_killed_split(self, mocker):
         mocked_split.killed = True
         mocked_split.change_number = 123
         mocked_split.get_configurations_for.return_value = '{"some_property": 123}'
-        e._feature_flag_storage.get.return_value = mocked_split
-        result = e.evaluate_feature('feature1', 'some_key', 'some_bucketing_key', {'attr1': 1})
+        result = e.evaluate_feature(mocked_split, 'some_key', 'some_bucketing_key', mocker.Mock())
         assert result['treatment'] == 'off'
         assert result['configurations'] == '{"some_property": 123}'
         assert result['impression']['change_number'] == 123
@@ -50,15 +35,14 @@ def test_evaluate_treatment_killed_split(self, mocker):
     def test_evaluate_treatment_ok(self, mocker):
         """Test that a non-killed split returns the appropriate treatment."""
         e = self._build_evaluator_with_mocks(mocker)
-        e._get_treatment_for_split = mocker.Mock()
-        e._get_treatment_for_split.return_value = ('on', 'some_label')
+        e._get_treatment_for_feature_flag = mocker.Mock()
+        e._get_treatment_for_feature_flag.return_value = ('on', 'some_label')
         mocked_split = mocker.Mock(spec=Split)
         mocked_split.default_treatment = 'off'
         mocked_split.killed = False
         mocked_split.change_number = 123
         mocked_split.get_configurations_for.return_value = '{"some_property": 123}'
-        e._feature_flag_storage.get.return_value = mocked_split
-        result = e.evaluate_feature('feature1', 'some_key', 'some_bucketing_key', {'attr1': 1})
+        result = e.evaluate_feature(mocked_split, 'some_key', 'some_bucketing_key', mocker.Mock())
         assert result['treatment'] == 'on'
         assert result['configurations'] == '{"some_property": 123}'
         assert result['impression']['change_number'] == 123
@@ -69,15 +53,14 @@ def test_evaluate_treatment_ok(self, mocker):
     def test_evaluate_treatment_ok_no_config(self, mocker):
         """Test that a killed split returns the default treatment."""
         e = self._build_evaluator_with_mocks(mocker)
-        e._get_treatment_for_split = mocker.Mock()
-        e._get_treatment_for_split.return_value = ('on', 'some_label')
+        e._get_treatment_for_feature_flag = mocker.Mock()
+        e._get_treatment_for_feature_flag.return_value = ('on', 'some_label')
         mocked_split = mocker.Mock(spec=Split)
         mocked_split.default_treatment = 'off'
         mocked_split.killed = False
         mocked_split.change_number = 123
         mocked_split.get_configurations_for.return_value = None
-        e._feature_flag_storage.get.return_value = mocked_split
-        result = e.evaluate_feature('feature1', 'some_key', 'some_bucketing_key', {'attr1': 1})
+        result = e.evaluate_feature(mocked_split, 'some_key', 'some_bucketing_key', mocker.Mock())
         assert result['treatment'] == 'on'
         assert result['configurations'] == None
         assert result['impression']['change_number'] == 123
@@ -87,24 +70,28 @@ def test_evaluate_treatment_ok_no_config(self, mocker):
     def test_evaluate_treatments(self, mocker):
         """Test that a missing split logs and returns CONTROL."""
         e = self._build_evaluator_with_mocks(mocker)
-        e._get_treatment_for_split = mocker.Mock()
-        e._get_treatment_for_split.return_value = ('on', 'some_label')
+        e._get_treatment_for_feature_flag = mocker.Mock()
+        e._get_treatment_for_feature_flag.return_value = ('on', 'some_label')
         mocked_split = mocker.Mock(spec=Split)
         mocked_split.name = 'feature2'
         mocked_split.default_treatment = 'off'
         mocked_split.killed = False
         mocked_split.change_number = 123
         mocked_split.get_configurations_for.return_value = '{"some_property": 123}'
-        e._feature_flag_storage.fetch_many.return_value = {
-            'feature1': None,
-            'feature2': mocked_split,
-        }
-        results = e.evaluate_features(['feature1', 'feature2'], 'some_key', 'some_bucketing_key', None)
-        result = results['feature1']
+
+        mocked_split2 = mocker.Mock(spec=Split)
+        mocked_split2.name = 'feature4'
+        mocked_split2.default_treatment = 'on'
+        mocked_split2.killed = False
+        mocked_split2.change_number = 123
+        mocked_split2.get_configurations_for.return_value = None
+
+        results = e.evaluate_features([mocked_split, mocked_split2], 'some_key', 'some_bucketing_key', mocker.Mock())
+        result = results['feature4']
         assert result['configurations'] == None
-        assert result['treatment'] == evaluator.CONTROL
-        assert result['impression']['change_number'] == -1
-        assert result['impression']['label'] == Label.SPLIT_NOT_FOUND
+        assert result['treatment'] == 'on'
+        assert result['impression']['change_number'] == 123
+        assert result['impression']['label'] == 'some_label'
         result = results['feature2']
         assert result['configurations'] == '{"some_property": 123}'
         assert result['treatment'] == 'on'
@@ -115,12 +102,9 @@ def test_get_gtreatment_for_split_no_condition_matches(self, mocker):
         """Test no condition matches."""
         e = self._build_evaluator_with_mocks(mocker)
         e._splitter.get_treatment.return_value = 'on'
-        conditions_mock = mocker.PropertyMock()
-        conditions_mock.return_value = []
         mocked_split = mocker.Mock(spec=Split)
         mocked_split.killed = False
-        type(mocked_split).conditions = conditions_mock
-        treatment, label = e._get_treatment_for_split(mocked_split, 'some_key', 'some_bucketing', {'attr1': 1})
+        treatment, label = e._get_treatment_for_feature_flag(mocked_split, 'some_key', 'some_bucketing', [])
         assert treatment == None
         assert label == None
 
@@ -132,30 +116,9 @@ def test_get_gtreatment_for_split_non_rollout(self, mocker):
         mocked_condition_1.condition_type = ConditionType.WHITELIST
         mocked_condition_1.label = 'some_label'
         mocked_condition_1.matches.return_value = True
-        conditions_mock = mocker.PropertyMock()
-        conditions_mock.return_value = [mocked_condition_1]
         mocked_split = mocker.Mock(spec=Split)
         mocked_split.killed = False
-        type(mocked_split).conditions = conditions_mock
-        treatment, label = e._get_treatment_for_split(mocked_split, 'some_key', 'some_bucketing', {'attr1': 1})
+        condition_matchers = [(True, mocked_condition_1)]
+        treatment, label = e._get_treatment_for_feature_flag(mocked_split, 'some_key', 'some_bucketing', condition_matchers)
         assert treatment == 'on'
-        assert label == 'some_label'
-
-    def test_get_treatment_for_split_rollout(self, mocker):
-        """Test rollout condition returns default treatment."""
-        e = self._build_evaluator_with_mocks(mocker)
-        e._splitter.get_bucket.return_value = 60
-        mocked_condition_1 = mocker.Mock(spec=Condition)
-        mocked_condition_1.condition_type = ConditionType.ROLLOUT
-        mocked_condition_1.label = 'some_label'
-        mocked_condition_1.matches.return_value = True
-        conditions_mock = mocker.PropertyMock()
-        conditions_mock.return_value = [mocked_condition_1]
-        mocked_split = mocker.Mock(spec=Split)
-        mocked_split.traffic_allocation = 50
-        mocked_split.default_treatment = 'almost-on'
-        mocked_split.killed = False
-        type(mocked_split).conditions = conditions_mock
-        treatment, label = e._get_treatment_for_split(mocked_split, 'some_key', 'some_bucketing', {'attr1': 1})
-        assert treatment == 'almost-on'
-        assert label == Label.NOT_IN_SPLIT
+        assert label == 'some_label'