fix(set_forced_variation): Treats empty variation key as invalid and does not reset forced variation. (#149)

Author: rashidsp

optimizely/project_config.py

@@ -15,6 +15,7 @@
 
 from .helpers import condition as condition_helper
 from .helpers import enums
+from .helpers import validator
 from . import entities
 from . import exceptions
 
@@ -499,7 +500,7 @@ def set_forced_variation(self, experiment_key, user_id, variation_key):
  return False
 
  experiment_id = experiment.id
- if not variation_key:
+ if variation_key is None:
  if user_id in self.forced_variation_map:
  experiment_to_variation_map = self.forced_variation_map.get(user_id)
  if experiment_id in experiment_to_variation_map:
@@ -517,6 +518,10 @@ def set_forced_variation(self, experiment_key, user_id, variation_key):
  self.logger.debug('Nothing to remove. User "%s" does not exist in the forced variation map.' % user_id)
  return True
 
+ if not validator.is_non_empty_string(variation_key):
+ self.logger.debug('Variation key is invalid.')
+ return False
+
  forced_variation = self.get_variation_from_key(experiment_key, variation_key)
  if not forced_variation:
  # The invalid variation key will be logged inside this call.

tests/test_config.py

@@ -1055,9 +1055,12 @@ def test_set_forced_variation__invalid_variation_key(self):
 
  self.assertFalse(self.project_config.set_forced_variation('test_experiment', 'test_user',
  'variation_not_in_datafile'))
- self.assertTrue(self.project_config.set_forced_variation('test_experiment', 'test_user', ''))
  self.assertTrue(self.project_config.set_forced_variation('test_experiment', 'test_user', None))
 
+ with mock.patch.object(self.project_config, 'logger') as mock_config_logging:
+ self.assertIs(self.project_config.set_forced_variation('test_experiment', 'test_user', ''), False)
+ mock_config_logging.debug.assert_called_once_with('Variation key is invalid.')
+
  def test_set_forced_variation__multiple_sets(self):
  """ Test multiple sets of experiments for one and multiple users work """