refact(ConditionEvaluator): Simplify condition evaluation

optimizely/entities.py

@@ -27,11 +27,10 @@ def __init__(self, id, key, **kwargs):
 
 class Audience(BaseEntity):
 
-  def __init__(self, id, name, conditions, conditionStructure=None, conditionList=None, **kwargs):
+  def __init__(self, id, name, conditions, conditionList=None, **kwargs):
     self.id = id
     self.name = name
     self.conditions = conditions
-    self.conditionStructure = conditionStructure
     self.conditionList = conditionList
 
 

optimizely/helpers/audience.py

@@ -1,4 +1,4 @@
-# Copyright 2016, Optimizely
+# Copyright 2016, 2018, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -14,20 +14,6 @@
 from . import condition as condition_helper
 
 
-def is_match(audience, attributes):
-  """ Given audience information and user attributes determine if user meets the conditions.
-
-  Args:
-    audience: Dict representing the audience.
-    attributes: Dict representing user attributes which will be used in determining if the audience conditions are met.
-
-  Return:
-    Boolean representing if user satisfies audience conditions or not.
-  """
-  condition_evaluator = condition_helper.ConditionEvaluator(audience.conditionList, attributes)
-  return condition_evaluator.evaluate(audience.conditionStructure)
-
-
 def is_user_in_experiment(config, experiment, attributes):
   """ Determine for given experiment if user satisfies the audiences for the experiment.
 
@@ -48,11 +34,13 @@ def is_user_in_experiment(config, experiment, attributes):
   if not attributes:
     return False
 
+  condition_evaluator = condition_helper.ConditionEvaluator(attributes)
+
   # Return True if conditions for any one audience are met
   for audience_id in experiment.audienceIds:
     audience = config.get_audience(audience_id)
 
-    if is_match(audience, attributes):
+    if condition_evaluator.evaluate(audience.conditionList) is True:
       return True
 
   return False

optimizely/helpers/condition.py

@@ -1,4 +1,4 @@
-# Copyright 2016, Optimizely
+# Copyright 2016,2018, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -30,21 +30,20 @@ class ConditionalOperatorTypes(object):
 class ConditionEvaluator(object):
   """ Class encapsulating methods to be used in audience condition evaluation. """
 
-  def __init__(self, condition_data, attributes):
-    self.condition_data = condition_data
+  def __init__(self, attributes):
     self.attributes = attributes
 
   def evaluator(self, condition):
     """ Method to compare single audience condition against provided user data i.e. attributes.
 
     Args:
-      condition: Integer representing the index of condition_data that needs to be used for comparison.
+      condition: Dict representing audience condition name, value, type etc.
 
     Returns:
       Boolean indicating the result of comparing the condition value against the user attributes.
     """
 
-    return self.attributes.get(self.condition_data[condition][0]) == self.condition_data[condition][1]
+    return self.attributes.get(condition['name']) == condition['value']
 
   def and_evaluator(self, conditions):
     """ Evaluates a list of conditions as if the evaluator had been applied
@@ -124,64 +123,17 @@ def evaluate(self, conditions):
 
 
 class ConditionDecoder(object):
-  """ Class which provides an object_hook method for decoding dict
-  objects into a list when given a condition_decoder. """
+  """ Class encapsulating methods to be used in audience condition decoding. """
 
-  def __init__(self, condition_decoder):
-    self.condition_list = []
-    self.index = -1
-    self.decoder = condition_decoder
-
-  def object_hook(self, object_dict):
-    """ Hook which when passed into a json.JSONDecoder will replace each dict
-    in a json string with its index and convert the dict to an object as defined
-    by the passed in condition_decoder. The newly created condition object is
-    appended to the conditions_list.
+  @staticmethod
+  def deserialize_audience_conditions(conditions_string):
+    """ Deserializes the conditions property into a list of structures and conditions.
 
     Args:
-      object_dict: Dict representing an object.
+      conditions_string: String defining valid and/or conditions.
 
     Returns:
-      An index which will be used as the placeholder in the condition_structure
+      list of conditions.
     """
-    instance = self.decoder(object_dict)
-    self.condition_list.append(instance)
-    self.index += 1
-    return self.index
-
-
-def _audience_condition_deserializer(obj_dict):
-  """ Deserializer defining how dict objects need to be decoded for audience conditions.
-
-  Args:
-    obj_dict: Dict representing one audience condition.
-
-  Returns:
-    List consisting of condition key and corresponding value.
-  """
-  return [obj_dict.get('name'), obj_dict.get('value')]
-
-
-def loads(conditions_string):
-  """ Deserializes the conditions property into its corresponding
-  components: the condition_structure and the condition_list.
-
-  Args:
-    conditions_string: String defining valid and/or conditions.
-
-  Returns:
-    A tuple of (condition_structure, condition_list).
-    condition_structure: nested list of operators and placeholders for operands.
-    condition_list: list of conditions whose index correspond to the values of the placeholders.
-  """
-  decoder = ConditionDecoder(_audience_condition_deserializer)
-
-  # Create a custom JSONDecoder using the ConditionDecoder's object_hook method
-  # to create the condition_structure as well as populate the condition_list
-  json_decoder = json.JSONDecoder(object_hook=decoder.object_hook)
-
-  # Perform the decoding
-  condition_structure = json_decoder.decode(conditions_string)
-  condition_list = decoder.condition_list
 
-  return (condition_structure, condition_list)
+    return json.loads(conditions_string)

optimizely/project_config.py

@@ -133,19 +133,18 @@ def _generate_key_map(entity_list, key, entity_class):
 
   @staticmethod
   def _deserialize_audience(audience_map):
-    """ Helper method to de-serialize and populate audience map with the condition list and structure.
+    """ Helper method to de-serialize and populate audience map with the condition list.
 
     Args:
       audience_map: Dict mapping audience ID to audience object.
 
     Returns:
-      Dict additionally consisting of condition list and structure on every audience object.
+      Dict additionally consisting of condition list.
     """
 
     for audience in audience_map.values():
-      condition_structure, condition_list = condition_helper.loads(audience.conditions)
+      condition_list = condition_helper.ConditionDecoder.deserialize_audience_conditions(audience.conditions)
       audience.__dict__.update({
-        'conditionStructure': condition_structure,
         'conditionList': condition_list
       })
 

tests/helpers_tests/test_audience.py

@@ -20,26 +20,30 @@
 class AudienceTest(base.BaseTest):
 
   def test_is_match__audience_condition_matches(self):
-    """ Test that is_match returns True when audience conditions are met. """
+    """ Test that is_user_in_experiment returns True when audience conditions are met. """
 
     user_attributes = {
       'test_attribute': 'test_value_1',
       'browser_type': 'firefox',
       'location': 'San Francisco'
     }
 
-    self.assertTrue(audience.is_match(self.optimizely.config.get_audience('11154'), user_attributes))
+    self.assertTrue(
+      audience.is_user_in_experiment(
+        self.project_config, self.project_config.get_experiment_from_key('test_experiment'), user_attributes))
 
-  def test_is_match__audience_condition_does_not_match(self):
-    """ Test that is_match returns False when audience conditions are not met. """
+  def test_is_user_in_experiment__audience_condition_does_not_match(self):
+    """ Test that is_user_in_experiment returns False when audience conditions are not met. """
 
     user_attributes = {
       'test_attribute': 'wrong_test_value',
       'browser_type': 'chrome',
       'location': 'San Francisco'
     }
 
-    self.assertFalse(audience.is_match(self.optimizely.config.get_audience('11154'), user_attributes))
+    self.assertFalse(
+      audience.is_user_in_experiment(
+        self.project_config, self.project_config.get_experiment_from_key('test_experiment'), user_attributes))
 
   def test_is_user_in_experiment__no_audience(self):
     """ Test that is_user_in_experiment returns True when experiment is using no audience. """
@@ -54,7 +58,8 @@ def test_is_user_in_experiment__no_audience(self):
     self.assertTrue(audience.is_user_in_experiment(self.project_config, experiment, user_attributes))
 
   def test_is_user_in_experiment__no_attributes(self):
-    """ Test that is_user_in_experiment returns True when experiment is using no audience. """
+    """ Test that is_user_in_experiment returns False when attributes are empty
+    and experiment has an audience. """
 
     self.assertFalse(audience.is_user_in_experiment(
       self.project_config, self.project_config.get_experiment_from_key('test_experiment'), None)
@@ -65,31 +70,31 @@ def test_is_user_in_experiment__no_attributes(self):
     )
 
   def test_is_user_in_experiment__audience_conditions_are_met(self):
-    """ Test that is_user_in_experiment returns True when audience conditions are met. """
+    """ Test that is_user_in_experiment returns True when Condition Evaluator returns True."""
 
     user_attributes = {
       'test_attribute': 'test_value_1',
       'browser_type': 'firefox',
       'location': 'San Francisco'
     }
 
-    with mock.patch('optimizely.helpers.audience.is_match', return_value=True) as mock_is_match:
+    with mock.patch('optimizely.helpers.condition.ConditionEvaluator.evaluate', return_value=True) as mock_evaluate:
       self.assertTrue(audience.is_user_in_experiment(self.project_config,
                                                      self.project_config.get_experiment_from_key('test_experiment'),
                                                      user_attributes))
-    mock_is_match.assert_called_once_with(self.optimizely.config.get_audience('11154'), user_attributes)
+    mock_evaluate.assert_called_once_with(self.optimizely.config.get_audience('11154').conditionList)
 
   def test_is_user_in_experiment__audience_conditions_not_met(self):
-    """ Test that is_user_in_experiment returns False when audience conditions are not met. """
+    """ Test that is_user_in_experiment returns False when Condition Evaluator returns False. """
 
     user_attributes = {
       'test_attribute': 'wrong_test_value',
       'browser_type': 'chrome',
       'location': 'San Francisco'
     }
 
-    with mock.patch('optimizely.helpers.audience.is_match', return_value=False) as mock_is_match:
+    with mock.patch('optimizely.helpers.condition.ConditionEvaluator.evaluate', return_value=False) as mock_evaluate:
       self.assertFalse(audience.is_user_in_experiment(self.project_config,
                                                       self.project_config.get_experiment_from_key('test_experiment'),
                                                       user_attributes))
-    mock_is_match.assert_called_once_with(self.optimizely.config.get_audience('11154'), user_attributes)
+    mock_evaluate.assert_called_once_with(self.optimizely.config.get_audience('11154').conditionList)

tests/helpers_tests/test_condition.py

@@ -22,50 +22,51 @@ class ConditionEvaluatorTests(base.BaseTest):
 
   def setUp(self):
     base.BaseTest.setUp(self)
-    self.condition_structure, self.condition_list = condition_helper.loads(
+    self.condition_list = condition_helper.ConditionDecoder.deserialize_audience_conditions(
       self.config_dict['audiences'][0]['conditions']
     )
+
     attributes = {
       'test_attribute': 'test_value_1',
       'browser_type': 'firefox',
       'location': 'San Francisco'
     }
-    self.condition_evaluator = condition_helper.ConditionEvaluator(self.condition_list, attributes)
+    self.condition_evaluator = condition_helper.ConditionEvaluator(attributes)
 
   def test_evaluator__returns_true(self):
     """ Test that evaluator correctly returns True when there is an exact match.
     Also test that evaluator works for falsy values. """
 
     # string attribute value
-    condition_list = [['test_attribute', '']]
-    condition_evaluator = condition_helper.ConditionEvaluator(condition_list, {'test_attribute': ''})
-    self.assertTrue(self.condition_evaluator.evaluator(0))
+    condition_list = {'type': 'custom_attribute', 'name': 'test_attribute', 'value': ''}
+    condition_evaluator = condition_helper.ConditionEvaluator({'test_attribute': ''})
+    self.assertTrue(condition_evaluator.evaluator(condition_list))
 
     # boolean attribute value
-    condition_list = [['boolean_key', False]]
-    condition_evaluator = condition_helper.ConditionEvaluator(condition_list, {'boolean_key': False})
-    self.assertTrue(condition_evaluator.evaluator(0))
+    condition_list = {'type': 'custom_attribute', 'name': 'boolean_key', 'value': False}
+    condition_evaluator = condition_helper.ConditionEvaluator({'boolean_key': False})
+    self.assertTrue(condition_evaluator.evaluator(condition_list))
 
     # integer attribute value
-    condition_list = [['integer_key', 0]]
-    condition_evaluator = condition_helper.ConditionEvaluator(condition_list, {'integer_key': 0})
-    self.assertTrue(condition_evaluator.evaluator(0))
+    condition_list = {'type': 'custom_attribute', 'name': 'integer_key', 'value': 0}
+    condition_evaluator = condition_helper.ConditionEvaluator({'integer_key': 0})
+    self.assertTrue(condition_evaluator.evaluator(condition_list))
 
     # double attribute value
-    condition_list = [['double_key', 0.0]]
-    condition_evaluator = condition_helper.ConditionEvaluator(condition_list, {'double_key': 0.0})
-    self.assertTrue(condition_evaluator.evaluator(0))
+    condition_list = {'type': 'custom_attribute', 'name': 'double_key', 'value': 0.0}
+    condition_evaluator = condition_helper.ConditionEvaluator({'double_key': 0.0})
+    self.assertTrue(condition_evaluator.evaluator(condition_list))
 
   def test_evaluator__returns_false(self):
     """ Test that evaluator correctly returns False when there is no match. """
-
+    condition_list = {'type': 'custom_attribute', 'name': 'browser_type', 'value': 'firefox'}
     attributes = {
       'browser_type': 'chrome',
       'location': 'San Francisco'
     }
-    self.condition_evaluator = condition_helper.ConditionEvaluator(self.condition_list, attributes)
+    condition_evaluator = condition_helper.ConditionEvaluator(attributes)
 
-    self.assertFalse(self.condition_evaluator.evaluator(0))
+    self.assertFalse(condition_evaluator.evaluator(condition_list))
 
   def test_and_evaluator__returns_true(self):
     """ Test that and_evaluator returns True when all conditions evaluate to True. """
@@ -121,23 +122,25 @@ def test_not_evaluator__returns_false_more_than_one_condition(self):
   def test_evaluate__returns_true(self):
     """ Test that evaluate returns True when conditions evaluate to True. """
 
-    self.assertTrue(self.condition_evaluator.evaluate(self.condition_structure))
+    self.assertTrue(self.condition_evaluator.evaluate(self.condition_list))
 
   def test_evaluate__returns_false(self):
     """ Test that evaluate returns False when conditions evaluate to False. """
 
-    condition_structure = ['and', ['or', ['not', 0]]]
+    condition_structure = {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_x"}
     self.assertFalse(self.condition_evaluator.evaluate(condition_structure))
 
 
 class ConditionDecoderTests(base.BaseTest):
 
-  def test_loads(self):
-    """ Test that loads correctly sets condition structure and list. """
+  def test_deserialize_audience_conditions(self):
+    """ Test that deserialize_audience_conditions correctly sets condition list. """
 
-    condition_structure, condition_list = condition_helper.loads(
+    condition_list = condition_helper.ConditionDecoder.deserialize_audience_conditions(
       self.config_dict['audiences'][0]['conditions']
     )
 
-    self.assertEqual(['and', ['or', ['or', 0]]], condition_structure)
-    self.assertEqual([['test_attribute', 'test_value_1']], condition_list)
+    self.assertEqual(
+      ['and', ['or', ['or', {"name": "test_attribute", "type": "custom_attribute", "value": "test_value_1"}]]],
+      condition_list
+    )