diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index 28275ef..68b75f7 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -529,7 +529,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: + # Exclude CMAB experiments from UserProfileService as they require dynamic decisions + 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..92232e3 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1890,3 +1890,217 @@ 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_update_user_profile(self): + """Test that CMAB experiments do NOT update user profile service.""" + + # Create a user context + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + + # Create user profile tracker with mock service + mock_user_profile_service = mock.Mock(spec=user_profile.UserProfileService) + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, mock_user_profile_service) + + # 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} + ) + + 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.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: + + # Configure CMAB service to return a decision + mock_cmab_service.get_decision.return_value = ( + { + 'variation_id': '111151', + 'cmab_uuid': 'test-cmab-uuid-123' + }, + [] # reasons list + ) + + # Call get_variation with the CMAB experiment and user profile tracker + variation_result = self.decision_service.get_variation( + self.project_config, + cmab_experiment, + user, + user_profile_tracker + ) + + variation = variation_result['variation'] + + # Verify the variation was returned + self.assertEqual(entities.Variation('111151', 'variation_1'), variation) + + # CRITICAL: Verify that update_user_profile was NOT called for CMAB experiment + # This is the key assertion - CMAB should not update UPS + mock_update.assert_not_called() + + def test_get_variation_non_cmab_experiment_updates_user_profile(self): + """Test that non-CMAB experiments still update user profile service (regression test).""" + + # Create a user context + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + + # Create user profile tracker + mock_user_profile_service = mock.Mock(spec=user_profile.UserProfileService) + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, mock_user_profile_service) + + # Get a regular (non-CMAB) experiment + experiment = self.project_config.get_experiment_from_key("test_experiment") + + # Ensure it's not a CMAB experiment + self.assertIsNone(experiment.cmab) + + 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=None), \ + mock.patch.object(self.decision_service.bucketer, 'bucket', + return_value=[entities.Variation('111129', 'variation'), []]): + + # Call get_variation with a non-CMAB experiment + variation_result = self.decision_service.get_variation( + self.project_config, + experiment, + user, + user_profile_tracker + ) + + variation = variation_result['variation'] + + # Verify the variation was returned + self.assertEqual(entities.Variation('111129', 'variation'), variation) + + # CRITICAL: Verify that update_user_profile WAS called for non-CMAB experiment + # This ensures our change doesn't break existing functionality + self.assertEqual( + user_profile_tracker.get_user_profile().get_variation_for_experiment(experiment.id), + '111129' + ) + + def test_get_variation_cmab_with_ignore_user_profile_true(self): + """Test CMAB with ignore_user_profile=True does not update user profile.""" + + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + + mock_user_profile_service = mock.Mock(spec=user_profile.UserProfileService) + user_profile_tracker = user_profile.UserProfileTracker(user.user_id, mock_user_profile_service) + + cmab_experiment = entities.Experiment( + '111150', + 'cmab_experiment', + 'Running', + '111150', + [], + {}, + [entities.Variation('111151', 'variation_1')], + [{'entityId': '111151', 'endOfRange': 10000}], + cmab={'trafficAllocation': 5000} + ) + + with mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \ + 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: + + mock_cmab_service.get_decision.return_value = ( + {'variation_id': '111151', 'cmab_uuid': 'test-uuid'}, + [] + ) + + # Call with IGNORE_USER_PROFILE_SERVICE option + variation_result = self.decision_service.get_variation( + self.project_config, + cmab_experiment, + user, + user_profile_tracker, + options=['IGNORE_USER_PROFILE_SERVICE'] + ) + + # Verify variation was returned + self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation']) + + # Verify no user profile update (combination of ignore flag + CMAB) + mock_update.assert_not_called() + + def test_get_variation_cmab_with_null_user_profile_tracker(self): + """Test CMAB with user_profile_tracker=None does not cause errors.""" + + user = optimizely_user_context.OptimizelyUserContext( + optimizely_client=None, + logger=None, + user_id="test_user", + user_attributes={} + ) + + cmab_experiment = entities.Experiment( + '111150', + 'cmab_experiment', + 'Running', + '111150', + [], + {}, + [entities.Variation('111151', 'variation_1')], + [{'entityId': '111151', 'endOfRange': 10000}], + cmab={'trafficAllocation': 5000} + ) + + with mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \ + 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_cmab_service.get_decision.return_value = ( + {'variation_id': '111151', 'cmab_uuid': 'test-uuid'}, + [] + ) + + # Call with user_profile_tracker=None + variation_result = self.decision_service.get_variation( + self.project_config, + cmab_experiment, + user, + user_profile_tracker=None + ) + + # Should succeed without errors and return variation + self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])