Params validation for API methods (#119)

Author: oakbani

optimizely/helpers/validator.py

@@ -1,4 +1,4 @@
-# Copyright 2016-2017, 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
@@ -13,6 +13,7 @@
 
 import json
 import jsonschema
+from six import string_types
 
 from optimizely.user_profile import UserProfile
 from . import constants
@@ -151,3 +152,18 @@ def is_user_profile_valid(user_profile):
       return False
 
   return True
+
+
+def is_non_empty_string(input_id_key):
+  """ Determine if provided input_id_key is a non-empty string or not.
+
+  Args:
+    input_id_key: Variable which needs to be validated.
+
+  Returns:
+    Boolean depending upon whether input is valid or not.
+  """
+  if input_id_key and isinstance(input_id_key, string_types):
+    return True
+
+  return False

optimizely/optimizely.py

@@ -179,6 +179,7 @@ def _send_impression_event(self, experiment, variation, user_id, attributes):
       self.event_dispatcher.dispatch_event(impression_event)
     except:
       self.logger.exception('Unable to dispatch impression event!')
+
     self.notification_center.send_notifications(enums.NotificationTypes.ACTIVATE,
                                                 experiment, user_id, attributes, variation, impression_event)
 
@@ -262,6 +263,14 @@ def activate(self, experiment_key, user_id, attributes=None):
       self.logger.error(enums.Errors.INVALID_DATAFILE.format('activate'))
       return None
 
+    if not validator.is_non_empty_string(experiment_key):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('experiment_key'))
+      return None
+
+    if not validator.is_non_empty_string(user_id):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id'))
+      return None
+
     variation_key = self.get_variation(experiment_key, user_id, attributes)
 
     if not variation_key:
@@ -291,6 +300,14 @@ def track(self, event_key, user_id, attributes=None, event_tags=None):
       self.logger.error(enums.Errors.INVALID_DATAFILE.format('track'))
       return
 
+    if not validator.is_non_empty_string(event_key):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('event_key'))
+      return
+
+    if not validator.is_non_empty_string(user_id):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id'))
+      return
+
     if not self._validate_user_inputs(attributes, event_tags):
       return
 
@@ -339,6 +356,14 @@ def get_variation(self, experiment_key, user_id, attributes=None):
       self.logger.error(enums.Errors.INVALID_DATAFILE.format('get_variation'))
       return None
 
+    if not validator.is_non_empty_string(experiment_key):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('experiment_key'))
+      return None
+
+    if not validator.is_non_empty_string(user_id):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id'))
+      return None
+
     experiment = self.config.get_experiment_from_key(experiment_key)
 
     if not experiment:
@@ -373,12 +398,12 @@ def is_feature_enabled(self, feature_key, user_id, attributes=None):
       self.logger.error(enums.Errors.INVALID_DATAFILE.format('is_feature_enabled'))
       return False
 
-    if feature_key is None:
-      self.logger.error(enums.Errors.NONE_FEATURE_KEY_PARAMETER)
+    if not validator.is_non_empty_string(feature_key):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('feature_key'))
       return False
 
-    if user_id is None:
-      self.logger.error(enums.Errors.NONE_USER_ID_PARAMETER)
+    if not validator.is_non_empty_string(user_id):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id'))
       return False
 
     feature = self.config.get_feature_from_key(feature_key)
@@ -417,6 +442,10 @@ def get_enabled_features(self, user_id, attributes=None):
       self.logger.error(enums.Errors.INVALID_DATAFILE.format('get_enabled_features'))
       return enabled_features
 
+    if not validator.is_non_empty_string(user_id):
+      self.logger.error(enums.Errors.INVALID_INPUT_ERROR.format('user_id'))
+      return enabled_features
+
     for feature in self.config.feature_key_map.values():
       if self.is_feature_enabled(feature.key, user_id, attributes):
         enabled_features.append(feature.key)

tests/helpers_tests/test_validator.py

@@ -1,4 +1,4 @@
-# Copyright 2016-2017, 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
@@ -130,6 +130,22 @@ def test_is_user_profile_valid__returns_false(self):
                                                       'experiment_bucket_map': {'1234': {'variation_id': '5678'},
                                                                                 '1235': {'some_key': 'some_value'}}}))
 
+  def test_is_non_empty_string(self):
+    """ Test that the method returns True only for a non-empty string. """
+
+    self.assertFalse(validator.is_non_empty_string(None))
+    self.assertFalse(validator.is_non_empty_string([]))
+    self.assertFalse(validator.is_non_empty_string({}))
+    self.assertFalse(validator.is_non_empty_string(0))
+    self.assertFalse(validator.is_non_empty_string(99))
+    self.assertFalse(validator.is_non_empty_string(1.2))
+    self.assertFalse(validator.is_non_empty_string(True))
+    self.assertFalse(validator.is_non_empty_string(False))
+    self.assertFalse(validator.is_non_empty_string(''))
+
+    self.assertTrue(validator.is_non_empty_string('0'))
+    self.assertTrue(validator.is_non_empty_string('test_user'))
+
 
 class DatafileValidationTests(base.BaseTest):
 

tests/test_optimizely.py

@@ -1143,6 +1143,30 @@ def test_track__invalid_object(self):
 
     mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "track".')
 
+  def test_track__invalid_experiment_key(self):
+    """ Test that None is returned and expected log messages are logged during track \
+    when exp_key is in invalid format. """
+
+    with mock.patch.object(self.optimizely, 'logger') as mock_client_logging, \
+         mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator:
+      self.assertIsNone(self.optimizely.track(99, 'test_user'))
+
+    mock_validator.assert_any_call(99)
+
+    mock_client_logging.error.assert_called_once_with('Provided "event_key" is in an invalid format.')
+
+  def test_track__invalid_user_id(self):
+    """ Test that None is returned and expected log messages are logged during track \
+    when user_id is in invalid format. """
+
+    with mock.patch.object(self.optimizely, 'logger') as mock_client_logging, \
+         mock.patch('optimizely.helpers.validator.is_non_empty_string', side_effect=[True, False]) as mock_validator:
+      self.assertIsNone(self.optimizely.track('test_event', 99))
+
+    mock_validator.assert_any_call(99)
+
+    mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.')
+
   def test_get_variation__invalid_object(self):
     """ Test that get_variation logs error if Optimizely object is not created correctly. """
 
@@ -1153,7 +1177,7 @@ def test_get_variation__invalid_object(self):
 
     mock_client_logging.error.assert_called_once_with('Datafile has invalid format. Failing "get_variation".')
 
-  def test_get_variation_invalid_experiment_key(self):
+  def test_get_variation_unknown_experiment_key(self):
     """ Test that get_variation retuns None when invalid experiment key is given. """
     with mock.patch.object(self.optimizely, 'logger') as mock_client_logging:
       self.optimizely.get_variation('aabbccdd', 'test_user', None)
@@ -1162,33 +1186,37 @@ def test_get_variation_invalid_experiment_key(self):
       'Experiment key "aabbccdd" is invalid. Not activating user "test_user".'
     )
 
-  def test_is_feature_enabled__returns_false_for_none_feature_key(self):
-    """ Test that is_feature_enabled returns false if the provided feature key is None. """
+  def test_is_feature_enabled__returns_false_for_invalid_feature_key(self):
+    """ Test that is_feature_enabled returns false if the provided feature key is invalid. """
 
     opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
 
-    with mock.patch.object(opt_obj, 'logger') as mock_client_logging:
+    with mock.patch.object(opt_obj, 'logger') as mock_client_logging,\
+         mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator:
       self.assertFalse(opt_obj.is_feature_enabled(None, 'test_user'))
 
-    mock_client_logging.error.assert_called_once_with(enums.Errors.NONE_FEATURE_KEY_PARAMETER)
+    mock_validator.assert_any_call(None)
+    mock_client_logging.error.assert_called_with('Provided "feature_key" is in an invalid format.')
 
-  def test_is_feature_enabled__returns_false_for_none_user_id(self):
-    """ Test that is_feature_enabled returns false if the provided user ID is None. """
+  def test_is_feature_enabled__returns_false_for_invalid_user_id(self):
+    """ Test that is_feature_enabled returns false if the provided user ID is invalid. """
 
     opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
 
-    with mock.patch.object(opt_obj, 'logger') as mock_client_logging:
-      self.assertFalse(opt_obj.is_feature_enabled('feature_key', None))
+    with mock.patch.object(opt_obj, 'logger') as mock_client_logging,\
+         mock.patch('optimizely.helpers.validator.is_non_empty_string', side_effect=[True, False]) as mock_validator:
+      self.assertFalse(opt_obj.is_feature_enabled('feature_key', 1.2))
 
-    mock_client_logging.error.assert_called_once_with(enums.Errors.NONE_USER_ID_PARAMETER)
+    mock_validator.assert_any_call(1.2)
+    mock_client_logging.error.assert_called_with('Provided "user_id" is in an invalid format.')
 
   def test_is_feature_enabled__returns_false_for_invalid_feature(self):
     """ Test that the feature is not enabled for the user if the provided feature key is invalid. """
 
     opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
 
     with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature') as mock_decision, \
-      mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event:
+            mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event:
       self.assertFalse(opt_obj.is_feature_enabled('invalid_feature', 'user1'))
 
     self.assertFalse(mock_decision.called)
@@ -1462,6 +1490,14 @@ def side_effect(*args, **kwargs):
     mock_is_feature_enabled.assert_any_call('test_feature_in_group', 'user_1', None)
     mock_is_feature_enabled.assert_any_call('test_feature_in_experiment_and_rollout', 'user_1', None)
 
+  def test_get_enabled_features_invalid_user_id(self):
+    with mock.patch.object(self.optimizely, 'logger') as mock_client_logging, \
+            mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator:
+      self.optimizely.get_enabled_features(1.2)
+
+    mock_validator.assert_any_call(1.2)
+    mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.')
+
   def test_get_enabled_features__invalid_object(self):
     """ Test that get_enabled_features returns empty list if Optimizely object is not valid. """
 
@@ -2003,6 +2039,52 @@ def test_get_variation__invalid_attributes(self):
 
     mock_client_logging.error.assert_called_once_with('Provided attributes are in an invalid format.')
 
+  def test_get_variation__invalid_experiment_key(self):
+    """ Test that None is returned and expected log messages are logged during get_variation \
+    when exp_key is in invalid format. """
+
+    with mock.patch.object(self.optimizely, 'logger') as mock_client_logging,\
+         mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator:
+      self.assertIsNone(self.optimizely.get_variation(99, 'test_user'))
+
+    mock_validator.assert_any_call(99)
+    mock_client_logging.error.assert_called_once_with('Provided "experiment_key" is in an invalid format.')
+
+  def test_get_variation__invalid_user_id(self):
+    """ Test that None is returned and expected log messages are logged during get_variation \
+    when user_id is in invalid format. """
+
+    with mock.patch.object(self.optimizely, 'logger') as mock_client_logging,\
+         mock.patch('optimizely.helpers.validator.is_non_empty_string', side_effect=[True, False]) as mock_validator:
+      self.assertIsNone(self.optimizely.get_variation('test_experiment', 99))
+
+    mock_validator.assert_any_call(99)
+    mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.')
+
+  def test_activate__invalid_experiment_key(self):
+    """ Test that None is returned and expected log messages are logged during activate \
+    when exp_key is in invalid format. """
+
+    with mock.patch.object(self.optimizely, 'logger') as mock_client_logging,\
+         mock.patch('optimizely.helpers.validator.is_non_empty_string', return_value=False) as mock_validator:
+      self.assertIsNone(self.optimizely.activate(99, 'test_user'))
+
+    mock_validator.assert_any_call(99)
+
+    mock_client_logging.error.assert_called_once_with('Provided "experiment_key" is in an invalid format.')
+
+  def test_activate__invalid_user_id(self):
+    """ Test that None is returned and expected log messages are logged during activate \
+    when user_id is in invalid format. """
+
+    with mock.patch.object(self.optimizely, 'logger') as mock_client_logging,\
+         mock.patch('optimizely.helpers.validator.is_non_empty_string', side_effect=[True, False]) as mock_validator:
+      self.assertIsNone(self.optimizely.activate('test_experiment', 99))
+
+    mock_validator.assert_any_call(99)
+
+    mock_client_logging.error.assert_called_once_with('Provided "user_id" is in an invalid format.')
+
   def test_activate__invalid_attributes(self):
     """ Test that expected log messages are logged during activate when attributes are in invalid format. """
     with mock.patch.object(self.optimizely, 'logger') as mock_client_logging: