diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 28275ef..96663c4 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 + 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 + 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..7e8ce1e 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1890,3 +1890,157 @@ def test_get_variation_for_feature_returns_rollout_in_experiment_bucket_range_25 mock_config_logging.debug.assert_called_with( 'Assigned bucket 4000 to user with bucketing ID "test_user".') mock_generate_bucket_value.assert_called_with("test_user211147") + + def test_get_variation__cmab_experiment__does_not_retrieve_from_ups(self): + """Test that CMAB experiments do NOT retrieve stored variations from UPS.""" + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + user_profile_service = user_profile.UserProfileService() + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) + + # Create a CMAB experiment by setting cmab property + experiment = self.project_config.get_experiment_from_key("test_experiment") + experiment.cmab = {'experiment_id': experiment.id} # Mark as CMAB experiment + + with mock.patch( + "optimizely.decision_service.DecisionService.get_whitelisted_variation", + return_value=[None, []], + ) as mock_get_whitelisted_variation, mock.patch( + "optimizely.decision_service.DecisionService.get_stored_variation", + return_value=entities.Variation("111128", "control"), + ) as mock_get_stored_variation, mock.patch( + "optimizely.helpers.audience.does_user_meet_audience_conditions", + return_value=[True, []] + ) as mock_audience_check, mock.patch( + "optimizely.decision_service.DecisionService._get_decision_for_cmab_experiment", + return_value={ + "error": False, + "result": {"variation_id": "111129", "cmab_uuid": "test-uuid"}, + "reasons": [] + } + ) as mock_cmab_decision: + result = self.decision_service.get_variation( + self.project_config, experiment, user, user_profile_tracker + ) + + # Assert that get_stored_variation was NOT called for CMAB experiment + mock_get_stored_variation.assert_not_called() + # Assert that CMAB decision was called instead + mock_cmab_decision.assert_called_once() + + def test_get_variation__cmab_experiment__does_not_save_to_ups(self): + """Test that CMAB experiments do NOT save variations to UPS.""" + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + user_profile_service = mock.Mock(spec=user_profile.UserProfileService) + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) + + # Create a CMAB experiment + experiment = self.project_config.get_experiment_from_key("test_experiment") + experiment.cmab = {'experiment_id': experiment.id} + + with mock.patch( + "optimizely.decision_service.DecisionService.get_whitelisted_variation", + return_value=[None, []], + ), mock.patch( + "optimizely.helpers.audience.does_user_meet_audience_conditions", + return_value=[True, []] + ), mock.patch( + "optimizely.decision_service.DecisionService._get_decision_for_cmab_experiment", + return_value={ + "error": False, + "result": {"variation_id": "111129", "cmab_uuid": "test-uuid"}, + "reasons": [] + } + ), mock.patch.object( + user_profile_tracker, 'update_user_profile' + ) as mock_update_profile: + result = self.decision_service.get_variation( + self.project_config, experiment, user, user_profile_tracker + ) + + # Assert that update_user_profile was NOT called for CMAB experiment + mock_update_profile.assert_not_called() + + def test_get_variation__non_cmab_experiment__retrieves_from_ups(self): + """Test that non-CMAB experiments still retrieve from UPS (regression test).""" + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + user_profile_service = user_profile.UserProfileService() + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) + + # Regular experiment (cmab is None) + experiment = self.project_config.get_experiment_from_key("test_experiment") + self.assertIsNone(experiment.cmab) # Verify it's not a CMAB experiment + + with mock.patch( + "optimizely.decision_service.DecisionService.get_whitelisted_variation", + return_value=[None, []], + ), mock.patch( + "optimizely.decision_service.DecisionService.get_stored_variation", + return_value=entities.Variation("111128", "control"), + ) as mock_get_stored_variation: + result = self.decision_service.get_variation( + self.project_config, experiment, user, user_profile_tracker + ) + + # Assert that get_stored_variation WAS called for non-CMAB experiment + mock_get_stored_variation.assert_called_once_with( + self.project_config, + experiment, + user_profile_tracker.user_profile + ) + # Verify the stored variation was returned + self.assertEqual(entities.Variation("111128", "control"), result['variation']) + + def test_get_variation__non_cmab_experiment__saves_to_ups(self): + """Test that non-CMAB experiments still save to UPS (regression test).""" + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + user_profile_service = mock.Mock(spec=user_profile.UserProfileService) + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service) + + # Regular experiment (cmab is None) + experiment = self.project_config.get_experiment_from_key("test_experiment") + self.assertIsNone(experiment.cmab) + + with mock.patch( + "optimizely.decision_service.DecisionService.get_whitelisted_variation", + return_value=[None, []], + ), mock.patch( + "optimizely.decision_service.DecisionService.get_stored_variation", + return_value=None, + ), mock.patch( + "optimizely.helpers.audience.does_user_meet_audience_conditions", + return_value=[True, []] + ), mock.patch( + "optimizely.bucketer.Bucketer.bucket", + return_value=[entities.Variation("111129", "variation"), []], + ), mock.patch.object( + user_profile_tracker, 'update_user_profile' + ) as mock_update_profile: + result = self.decision_service.get_variation( + self.project_config, experiment, user, user_profile_tracker + ) + + # Assert that update_user_profile WAS called for non-CMAB experiment + mock_update_profile.assert_called_once_with( + experiment, + entities.Variation("111129", "variation") + )