Params validation for API methods

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
@@ -14,6 +14,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 isinstance(input_id_key, string_types) and input_id_key:
+ 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: