Updating whitelisting check to precede audience check (#25)

Author: aliabbasrizvi

optimizely/helpers/experiment.py

@@ -12,3 +12,20 @@ def is_experiment_running(experiment):
   """
 
   return experiment.status in ALLOWED_EXPERIMENT_STATUS
+
+
+def is_user_in_forced_variation(forced_variations, user_id):
+  """ Determine if the user is in a forced variation.
+
+  Args:
+    forced_variations: Dict representing forced variations for the experiment.
+    user_id: User to check for in whitelist.
+
+  Returns:
+    Boolean depending on whether user is in forced variation or not.
+  """
+
+  if forced_variations and user_id in forced_variations:
+    return True
+
+  return False

optimizely/optimizely.py

@@ -80,6 +80,9 @@ def _validate_preconditions(self, experiment, user_id, attributes):
       self.logger.log(enums.LogLevels.INFO, 'Experiment "%s" is not running.' % experiment.key)
       return False
 
+    if experiment_helper.is_user_in_forced_variation(experiment.forcedVariations, user_id):
+      return True
+
     if not audience_helper.is_user_in_experiment(self.config, experiment, attributes):
       self.logger.log(
         enums.LogLevels.INFO,

tests/helpers_tests/test_experiment.py

@@ -20,3 +20,19 @@ def test_is_experiment_running__status_not_running(self):
                       '42', 'test_experiment', 'Some Status', [], [], {},[])) as mock_get_experiment:
       self.assertFalse(experiment.is_experiment_running(self.project_config.get_experiment_from_key('test_experiment')))
     mock_get_experiment.assert_called_once_with('test_experiment')
+
+  def test_is_user_in_forced_variation__no_forced_variation_dict(self):
+    """ Test that is_user_in_forced_variation returns False when experiment has no forced variations. """
+
+    self.assertFalse(experiment.is_user_in_forced_variation(None, 'test_user'))
+    self.assertFalse(experiment.is_user_in_forced_variation({}, 'test_user'))
+
+  def test_is_user_in_forced_variation__user_not_in_forced_variation(self):
+    """ Test that is_user_in_forced_variation returns False when user is not in experiment's forced variations. """
+
+    self.assertFalse(experiment.is_user_in_forced_variation({'user_1': 'control'}, 'test_user'))
+
+  def test_is_user_in_forced_variation__user_in_forced_variation(self):
+    """ Test that is_user_in_forced_variation returns True when user is in experiment's forced variations. """
+
+    self.assertTrue(experiment.is_user_in_forced_variation({'user_1': 'control'}, 'user_1'))

tests/test_optimizely.py

@@ -151,12 +151,29 @@ def test_activate__with_attributes__invalid_attributes(self):
   def test_activate__experiment_not_running(self):
     """ Test that activate returns None when experiment is not Running. """
 
-    with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True),\
+    with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                   return_value=True) as mock_whitelist_check,\
         mock.patch('optimizely.helpers.experiment.is_experiment_running',
                    return_value=False) as mock_is_experiment_running:
       self.assertIsNone(self.optimizely.activate('test_experiment', 'test_user',
                                                  attributes={'test_attribute': 'test_value'}))
     mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
+    self.assertEqual(0, mock_whitelist_check.call_count)
+    self.assertEqual(0, mock_audience_check.call_count)
+
+  def test_activate__whitelisting_overrides_audience_check(self):
+    """ Test that during activate whitelist overrides audience check if user is in the whitelist. """
+
+    with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                   return_value=True) as mock_whitelist_check,\
+        mock.patch('optimizely.helpers.experiment.is_experiment_running',
+                   return_value=True) as mock_is_experiment_running:
+      self.assertEqual('control', self.optimizely.activate('test_experiment', 'user_1'))
+    mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
+    mock_whitelist_check.assert_called_once_with({'user_1': 'control', 'user_2': 'control'}, 'user_1')
+    self.assertEqual(0, mock_audience_check.call_count)
 
   def test_activate__bucketer_returns_none(self):
     """ Test that activate returns None when user is in no variation. """
@@ -253,12 +270,35 @@ def test_track__experiment_not_running(self):
 
     with mock.patch('optimizely.helpers.experiment.is_experiment_running',
                     return_value=False) as mock_is_experiment_running,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                   return_value=True) as mock_whitelist_check,\
+        mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
         mock.patch('time.time', return_value=42),\
         mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event:
       self.optimizely.track('test_event', 'test_user')
 
     mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
     self.assertEqual(0, mock_dispatch_event.call_count)
+    self.assertEqual(0, mock_whitelist_check.call_count)
+    self.assertEqual(0, mock_audience_check.call_count)
+
+  def test_track__whitelisted_user_overrides_audience_check(self):
+    """ Test that track does not check for user in audience when user is in whitelist. """
+
+    with mock.patch('optimizely.helpers.experiment.is_experiment_running',
+                    return_value=True) as mock_is_experiment_running,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                    return_value=True) as mock_whitelist_check,\
+        mock.patch('optimizely.helpers.audience.is_user_in_experiment',
+                    return_value=False) as mock_audience_check,\
+        mock.patch('time.time', return_value=42),\
+        mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event:
+      self.optimizely.track('test_event', 'user_1')
+
+    mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
+    mock_whitelist_check.assert_called_once_with({'user_1': 'control', 'user_2': 'control'}, 'user_1')
+    self.assertEqual(1, mock_dispatch_event.call_count)
+    self.assertEqual(0, mock_audience_check.call_count)
 
   def test_get_variation__audience_match_and_experiment_running(self):
     """ Test that get variation retrieves expected variation
@@ -296,13 +336,31 @@ def test_get_variation__experiment_not_running(self):
                     return_value=self.project_config.get_variation_from_id(
                       'test_experiment', '111129'
                     )) as mock_bucket,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                   return_value=True) as mock_whitelist_check,\
+        mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
         mock.patch('optimizely.helpers.experiment.is_experiment_running',
                    return_value=False) as mock_is_experiment_running:
       self.assertIsNone(self.optimizely.get_variation('test_experiment', 'test_user',
                                                       attributes={'test_attribute': 'test_value'}))
 
     self.assertEqual(0, mock_bucket.call_count)
     mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
+    self.assertEqual(0, mock_whitelist_check.call_count)
+    self.assertEqual(0, mock_audience_check.call_count)
+
+  def test_get_variation__whitelisting_overrides_audience_check(self):
+    """ Test that in get_variation whitelist overrides audience check if user is in the whitelist. """
+
+    with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                   return_value=True) as mock_whitelist_check,\
+        mock.patch('optimizely.helpers.experiment.is_experiment_running',
+                   return_value=True) as mock_is_experiment_running:
+      self.assertEqual('control', self.optimizely.get_variation('test_experiment', 'user_1'))
+    mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
+    self.assertEqual(1, mock_whitelist_check.call_count)
+    self.assertEqual(0, mock_audience_check.call_count)
 
   def test_custom_logger(self):
     """ Test creating Optimizely object with a custom logger. """
@@ -443,7 +501,9 @@ def test_activate__with_attributes__invalid_attributes(self):
   def test_activate__experiment_not_running(self):
     """ Test that activate returns None and does not dispatch event when experiment is not Running. """
 
-    with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True),\
+    with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=True) as mock_audience_check,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                   return_value=True) as mock_whitelist_check,\
         mock.patch('optimizely.helpers.experiment.is_experiment_running',
                    return_value=False) as mock_is_experiment_running, \
         mock.patch('optimizely.bucketer.Bucketer.bucket') as mock_bucket,\
@@ -452,9 +512,24 @@ def test_activate__experiment_not_running(self):
                                                  attributes={'test_attribute': 'test_value'}))
 
     mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
+    self.assertEqual(0, mock_audience_check.call_count)
+    self.assertEqual(0, mock_whitelist_check.call_count)
     self.assertEqual(0, mock_bucket.call_count)
     self.assertEqual(0, mock_dispatch_event.call_count)
 
+  def test_activate__whitelisting_overrides_audience_check(self):
+    """ Test that during activate whitelist overrides audience check if user is in the whitelist. """
+
+    with mock.patch('optimizely.helpers.audience.is_user_in_experiment', return_value=False) as mock_audience_check,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                   return_value=True) as mock_whitelist_check,\
+        mock.patch('optimizely.helpers.experiment.is_experiment_running',
+                   return_value=True) as mock_is_experiment_running:
+      self.assertEqual('control', self.optimizely.activate('test_experiment', 'user_1'))
+    mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
+    self.assertEqual(1, mock_whitelist_check.call_count)
+    self.assertEqual(0, mock_audience_check.call_count)
+
   def test_activate__bucketer_returns_none(self):
     """ Test that activate returns None and does not dispatch event when user is in no variation. """
 
@@ -595,6 +670,24 @@ def test_track__experiment_not_running(self):
     mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
     self.assertEqual(0, mock_dispatch_event.call_count)
 
+  def test_track__whitelisted_user_overrides_audience_check(self):
+    """ Test that track does not check for user in audience when user is in whitelist. """
+
+    with mock.patch('optimizely.helpers.experiment.is_experiment_running',
+                    return_value=True) as mock_is_experiment_running,\
+        mock.patch('optimizely.helpers.experiment.is_user_in_forced_variation',
+                    return_value=True) as mock_whitelist_check,\
+        mock.patch('optimizely.helpers.audience.is_user_in_experiment',
+                    return_value=False) as mock_audience_check,\
+        mock.patch('time.time', return_value=42),\
+        mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event:
+      self.optimizely.track('test_event', 'user_1')
+
+    mock_is_experiment_running.assert_called_once_with(self.project_config.get_experiment_from_key('test_experiment'))
+    mock_whitelist_check.assert_called_once_with({'user_1': 'control', 'user_2': 'control'}, 'user_1')
+    self.assertEqual(1, mock_dispatch_event.call_count)
+    self.assertEqual(0, mock_audience_check.call_count)
+
 
 class OptimizelyWithExceptionTest(base.BaseTestV1):