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 """