diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 28275ef..7555762 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -457,7 +457,8 @@ def get_variation( } # Check to see if user has a decision available for the given experiment - if user_profile_tracker is not None and not ignore_user_profile: + # CMAB experiments should not use UPS for sticky bucketing as it contradicts their dynamic nature + if user_profile_tracker is not None and not ignore_user_profile and not experiment.cmab: variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile()) if variation: message = f'Returning previously activated variation ID "{variation}" of experiment ' \ @@ -529,7 +530,8 @@ def get_variation( self.logger.info(message) decide_reasons.append(message) # Store this new decision and return the variation for the user - if user_profile_tracker is not None and not ignore_user_profile: + # CMAB experiments should not use UPS for sticky bucketing as it contradicts their dynamic nature + if user_profile_tracker is not None and not ignore_user_profile and not experiment.cmab: try: user_profile_tracker.update_user_profile(experiment, variation) except: diff --git a/tests/test_decision_service.py b/tests/test_decision_service.py index dbcb743..544d3b7 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1074,6 +1074,211 @@ def test_get_variation_cmab_experiment_with_whitelisted_variation(self): mock_bucket.assert_not_called() mock_cmab_decision.assert_not_called() + def test_get_variation_cmab_experiment_does_not_use_stored_variation(self): + """Test that CMAB experiments do not use stored variations from UserProfileService.""" + + # Create a user context + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + + # Create a CMAB experiment + cmab_experiment = entities.Experiment( + '111150', + 'cmab_experiment', + 'Running', + '111150', + [], # No audience IDs + {}, + [ + entities.Variation('111151', 'variation_1'), + entities.Variation('111152', 'variation_2') + ], + [ + {'entityId': '111151', 'endOfRange': 5000}, + {'entityId': '111152', 'endOfRange': 10000} + ], + cmab={'trafficAllocation': 5000} + ) + + # Create a user profile with a stored variation + stored_variation = entities.Variation('111152', 'variation_2') + user_profile_tracker = user_profile.UserProfileTracker( + 'test_user', + user_profile.UserProfileService(), + None + ) + # Simulate a stored variation in user profile + user_profile_tracker.get_user_profile().save_variation_for_experiment('111150', '111152') + + with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \ + mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \ + mock.patch.object(self.decision_service, 'get_forced_variation', return_value=[None, []]), \ + mock.patch.object(self.decision_service, 'get_whitelisted_variation', return_value=[None, []]), \ + mock.patch.object(self.decision_service, 'get_stored_variation', + return_value=stored_variation) as mock_get_stored, \ + mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id', + return_value=['$', []]), \ + mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \ + mock.patch.object(self.project_config, 'get_variation_from_id', + return_value=entities.Variation('111151', 'variation_1')): + + # Configure CMAB service to return a different variation + mock_cmab_service.get_decision.return_value = ( + { + 'variation_id': '111151', + 'cmab_uuid': 'test-cmab-uuid-123' + }, + [] + ) + + # Call get_variation with the CMAB experiment + variation_result = self.decision_service.get_variation( + self.project_config, + cmab_experiment, + user, + user_profile_tracker + ) + variation = variation_result['variation'] + cmab_uuid = variation_result['cmab_uuid'] + + # Verify get_stored_variation was NOT called because experiment.cmab is True + mock_get_stored.assert_not_called() + + # Verify we got the CMAB decision, not the stored variation + self.assertEqual(entities.Variation('111151', 'variation_1'), variation) + self.assertEqual('test-cmab-uuid-123', cmab_uuid) + + def test_get_variation_cmab_experiment_does_not_save_to_user_profile(self): + """Test that CMAB experiments do not save variations to UserProfileService.""" + + # Create a user context + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + + # Create a CMAB experiment + cmab_experiment = entities.Experiment( + '111150', + 'cmab_experiment', + 'Running', + '111150', + [], # No audience IDs + {}, + [ + entities.Variation('111151', 'variation_1'), + entities.Variation('111152', 'variation_2') + ], + [ + {'entityId': '111151', 'endOfRange': 5000}, + {'entityId': '111152', 'endOfRange': 10000} + ], + cmab={'trafficAllocation': 5000} + ) + + # Create a user profile tracker with mock service + mock_ups = mock.Mock(spec=user_profile.UserProfileService) + user_profile_tracker = user_profile.UserProfileTracker( + 'test_user', + mock_ups, + None + ) + + with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \ + mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \ + mock.patch.object(self.decision_service, 'get_forced_variation', return_value=[None, []]), \ + mock.patch.object(self.decision_service, 'get_whitelisted_variation', return_value=[None, []]), \ + mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id', + return_value=['$', []]), \ + mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \ + mock.patch.object(self.project_config, 'get_variation_from_id', + return_value=entities.Variation('111151', 'variation_1')), \ + mock.patch.object(user_profile_tracker, 'update_user_profile') as mock_update_profile: + + # Configure CMAB service to return a decision + mock_cmab_service.get_decision.return_value = ( + { + 'variation_id': '111151', + 'cmab_uuid': 'test-cmab-uuid-123' + }, + [] + ) + + # Call get_variation with the CMAB experiment + variation_result = self.decision_service.get_variation( + self.project_config, + cmab_experiment, + user, + user_profile_tracker + ) + variation = variation_result['variation'] + + # Verify update_user_profile was NOT called because experiment.cmab is True + mock_update_profile.assert_not_called() + + # Verify we got the variation + self.assertEqual(entities.Variation('111151', 'variation_1'), variation) + + def test_get_variation_non_cmab_experiment_still_uses_user_profile(self): + """Test that non-CMAB experiments still use UserProfileService as expected.""" + + # Create a user context + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + + # Create a regular (non-CMAB) experiment + regular_experiment = entities.Experiment( + '111127', + 'test_experiment', + 'Running', + '111182', + ['11154'], + {}, + [entities.Variation('111128', 'control'), entities.Variation('111129', 'variation')], + {'111128': 4000, '111129': 8000}, + None # No cmab property + ) + + # Create a user profile with a stored variation + stored_variation = entities.Variation('111129', 'variation') + user_profile_tracker = user_profile.UserProfileTracker( + 'test_user', + user_profile.UserProfileService(), + None + ) + user_profile_tracker.get_user_profile().save_variation_for_experiment('111127', '111129') + + with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \ + mock.patch.object(self.decision_service, 'get_forced_variation', return_value=[None, []]), \ + mock.patch.object(self.decision_service, 'get_whitelisted_variation', return_value=[None, []]), \ + mock.patch.object(self.decision_service, 'get_stored_variation', + return_value=stored_variation) as mock_get_stored: + + # Call get_variation with the regular experiment + variation_result = self.decision_service.get_variation( + self.project_config, + regular_experiment, + user, + user_profile_tracker + ) + variation = variation_result['variation'] + + # Verify get_stored_variation WAS called because experiment.cmab is None + mock_get_stored.assert_called_once() + + # Verify we got the stored variation + self.assertEqual(stored_variation, variation) + class FeatureFlagDecisionTests(base.BaseTest): def setUp(self):