Revert "Introducing feature variable accessors (#86)"

aliabbasrizvi · aliabbasrizvi · commit 0b2f019fb74d · 2017-12-18T16:57:58.000-08:00
This reverts commit db4a9a6.
diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py
@@ -15,13 +15,13 @@
 import sys
 
 from . import decision_service
-from . import entities
 from . import event_builder
 from . import exceptions
 from . import project_config
 from .error_handler import NoOpErrorHandler as noop_error_handler
 from .event_dispatcher import EventDispatcher as default_event_dispatcher
 from .helpers import enums
+from .helpers import event_tag_utils
 from .helpers import validator
 from .logger import NoOpLogger as noop_logger
 from .logger import SimpleLogger
@@ -182,64 +182,6 @@ def _send_impression_event(self, experiment, variation, user_id, attributes):
     self.notification_center.send_notifications(enums.NotificationTypes.ACTIVATE,
                                                 experiment, user_id, attributes, variation, impression_event)
 
-  def _get_feature_variable_for_type(self, feature_key, variable_key, variable_type, user_id, attributes):
-    """ Helper method to determine value for a certain variable attached to a feature flag based on type of variable.
-
-    Args:
-      feature_key: Key of the feature whose variable's value is being accessed.
-      variable_key: Key of the variable whose value is to be accessed.
-      variable_type: Type of variable which could be one of boolean/double/integer/string.
-      user_id: ID for user.
-      attributes: Dict representing user attributes.
-
-    Returns:
-      Value of the variable. None if:
-      - Feature key is invalid.
-      - Variable key is invalid.
-      - Mismatch with type of variable.
-    """
-
-    feature_flag = self.config.get_feature_from_key(feature_key)
-    if not feature_flag:
-      return None
-
-    variable = self.config.get_variable_for_feature(feature_key, variable_key)
-    if not variable:
-      return None
-
-    # Return None if type differs
-    if variable.type != variable_type:
-      self.logger.log(
-        enums.LogLevels.WARNING,
-        'Requested variable type "%s", but variable is of type "%s". '
-        'Use correct API to retrieve value. Returning None.' % (variable_type, variable.type)
-      )
-      return None
-
-    decision = self.decision_service.get_variation_for_feature(feature_flag, user_id, attributes)
-    if decision.variation:
-      variable_value = self.config.get_variable_value_for_variation(variable, decision.variation)
-      self.logger.log(
-        enums.LogLevels.INFO,
-        'Value for variable "%s" of feature flag "%s" is %s for user "%s".' % (
-          variable_key, feature_key, variable_value, user_id
-        ))
-    else:
-      variable_value = variable.defaultValue
-      self.logger.log(
-        enums.LogLevels.INFO,
-        'User "%s" is not in any variation or rollout rule. '
-        'Returning default value for variable "%s" of feature flag "%s".' % (user_id, variable_key, feature_key)
-      )
-
-    try:
-      actual_value = self.config.get_typecast_value(variable_value, variable_type)
-    except:
-      self.logger.log(enums.LogLevels.ERROR, 'Unable to cast value. Returning None.')
-      actual_value = None
-
-    return actual_value
-
   def activate(self, experiment_key, user_id, attributes=None):
     """ Buckets visitor and sends impression event to Optimizely.
 
@@ -413,82 +355,6 @@ def get_enabled_features(self, user_id, attributes=None):
 
     return enabled_features
 
-  def get_feature_variable_boolean(self, feature_key, variable_key, user_id, attributes=None):
-    """ Returns value for a certain boolean variable attached to a feature flag.
-
-    Args:
-      feature_key: Key of the feature whose variable's value is being accessed.
-      variable_key: Key of the variable whose value is to be accessed.
-      user_id: ID for user.
-      attributes: Dict representing user attributes.
-
-    Returns:
-      Boolean value of the variable. None if:
-      - Feature key is invalid.
-      - Variable key is invalid.
-      - Mismatch with type of variable.
-    """
-
-    variable_type = entities.Variable.Type.BOOLEAN
-    return self._get_feature_variable_for_type(feature_key, variable_key, variable_type, user_id, attributes)
-
-  def get_feature_variable_double(self, feature_key, variable_key, user_id, attributes=None):
-    """ Returns value for a certain double variable attached to a feature flag.
-
-    Args:
-      feature_key: Key of the feature whose variable's value is being accessed.
-      variable_key: Key of the variable whose value is to be accessed.
-      user_id: ID for user.
-      attributes: Dict representing user attributes.
-
-    Returns:
-      Double value of the variable. None if:
-      - Feature key is invalid.
-      - Variable key is invalid.
-      - Mismatch with type of variable.
-    """
-
-    variable_type = entities.Variable.Type.DOUBLE
-    return self._get_feature_variable_for_type(feature_key, variable_key, variable_type, user_id, attributes)
-
-  def get_feature_variable_integer(self, feature_key, variable_key, user_id, attributes=None):
-    """ Returns value for a certain integer variable attached to a feature flag.
-
-    Args:
-      feature_key: Key of the feature whose variable's value is being accessed.
-      variable_key: Key of the variable whose value is to be accessed.
-      user_id: ID for user.
-      attributes: Dict representing user attributes.
-
-    Returns:
-      Integer value of the variable. None if:
-      - Feature key is invalid.
-      - Variable key is invalid.
-      - Mismatch with type of variable.
-    """
-
-    variable_type = entities.Variable.Type.INTEGER
-    return self._get_feature_variable_for_type(feature_key, variable_key, variable_type, user_id, attributes)
-
-  def get_feature_variable_string(self, feature_key, variable_key, user_id, attributes=None):
-    """ Returns value for a certain string variable attached to a feature.
-
-    Args:
-      feature_key: Key of the feature whose variable's value is being accessed.
-      variable_key: Key of the variable whose value is to be accessed.
-      user_id: ID for user.
-      attributes: Dict representing user attributes.
-
-    Returns:
-      String value of the variable. None if:
-      - Feature key is invalid.
-      - Variable key is invalid.
-      - Mismatch with type of variable.
-    """
-
-    variable_type = entities.Variable.Type.STRING
-    return self._get_feature_variable_for_type(feature_key, variable_key, variable_type, user_id, attributes)
-
   def set_forced_variation(self, experiment_key, user_id, variation_key):
     """ Force a user into a variation for a given experiment.
 
diff --git a/optimizely/project_config.py b/optimizely/project_config.py
@@ -154,7 +154,7 @@ def _deserialize_audience(audience_map):
 
     return audience_map
 
-  def get_typecast_value(self, value, type):
+  def _get_typecast_value(self, value, type):
     """ Helper method to determine actual value based on type of feature variable.
 
     Args:
@@ -421,13 +421,12 @@ def get_variable_value_for_variation(self, variable, variation):
     """ Get the variable value for the given variation.
 
     Args:
-      variable: The Variable for which we are getting the value.
-      variation: The Variation for which we are getting the variable value.
+      Variable: The Variable for which we are getting the value.
+      Variation: The Variation for which we are getting the variable value.
 
     Returns:
-      The variable value or None if any of the inputs are invalid.
+      The type-casted variable value or None if any of the inputs are invalid.
     """
-
     if not variable or not variation:
       return None
 
@@ -439,13 +438,13 @@ def get_variable_value_for_variation(self, variable, variation):
     variable_usages = self.variation_variable_usage_map[variation.id]
 
     # Find usage in given variation
-    variable_usage = variable_usages.get(variable.id)
+    variable_usage = variable_usages[variable.id]
 
-    # Return default value in case there is no variable usage for the variable.
-    return variable_usage.value if variable_usage else variable.defaultValue
+    value = self._get_typecast_value(variable_usage.value, variable.type)
+    return value
 
   def get_variable_for_feature(self, feature_key, variable_key):
-    """ Get the variable with the given variable key for the given feature.
+    """ Get the variable with the given variable key for the given feature
 
     Args:
       feature_key: The key of the feature for which we are getting the variable.
diff --git a/tests/base.py b/tests/base.py
@@ -176,23 +176,10 @@ def setUp(self):
             'id': '127', 'value': 'false'
           }, {
             'id': '128', 'value': 'prod'
-          }, {
-            'id': '129', 'value': '10.01'
-          }, {
-            'id': '130', 'value': '4242'
           }]
         }, {
           'key': 'variation',
-          'id': '111129',
-          'variables': [{
-            'id': '127', 'value': 'true'
-          }, {
-            'id': '128', 'value': 'staging'
-          }, {
-            'id': '129', 'value': '10.02'
-          }, {
-            'id': '130', 'value': '4243'
-          }]
+          'id': '111129'
         }]
       }],
       'groups': [{
@@ -337,21 +324,6 @@ def setUp(self):
             'key': 'environment',
             'defaultValue': 'devel',
             'type': 'string',
-          }, {
-            'id': '129',
-            'key': 'cost',
-            'defaultValue': '10.99',
-            'type': 'double',
-          }, {
-            'id': '130',
-            'key': 'count',
-            'defaultValue': '999',
-            'type': 'integer',
-          }, {
-            'id': '131',
-            'key': 'variable_without_usage',
-            'defaultValue': '45',
-            'type': 'integer',
           }]
       }, {
         'id': '91112',
diff --git a/tests/test_config.py b/tests/test_config.py
@@ -1197,14 +1197,14 @@ def test_get_rollout_from_id__valid_rollout_id(self):
     self.assertEqual(expected_rollout, project_config.get_rollout_from_id('211111'))
 
   def test_get_variable_value_for_variation__returns_valid_value(self):
-    """ Test that the right value is returned. """
+    """ Test that the right value and type are returned. """
     opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
     project_config = opt_obj.config
 
     variation = project_config.get_variation_from_id('test_experiment', '111128')
     is_working_variable = project_config.get_variable_for_feature('test_feature_in_experiment', 'is_working')
     environment_variable = project_config.get_variable_for_feature('test_feature_in_experiment', 'environment')
-    self.assertEqual('false', project_config.get_variable_value_for_variation(is_working_variable, variation))
+    self.assertEqual(False, project_config.get_variable_value_for_variation(is_working_variable, variation))
     self.assertEqual('prod', project_config.get_variable_value_for_variation(environment_variable, variation))
 
   def test_get_variable_value_for_variation__invalid_variable(self):
@@ -1226,17 +1226,6 @@ def test_get_variable_value_for_variation__no_variables_for_variation(self):
     is_working_variable = project_config.get_variable_for_feature('test_feature_in_experiment', 'is_working')
     self.assertIsNone(project_config.get_variable_value_for_variation(is_working_variable, variation))
 
-  def test_get_variable_value_for_variation__no_usage_of_variable(self):
-    """ Test that a variable with no usage will return default value for variable. """
-
-    opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
-    project_config = opt_obj.config
-
-    variation = project_config.get_variation_from_id('test_experiment', '111128')
-    variable_without_usage_variable = project_config.get_variable_for_feature('test_feature_in_experiment',
-                                                                              'variable_without_usage')
-    self.assertEqual('45', project_config.get_variable_value_for_variation(variable_without_usage_variable, variation))
-
   def test_get_variable_for_feature__returns_valid_variable(self):
     """ Test that the feature variable is returned. """
 
diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py
