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..ac50501 100644 --- a/tests/test_decision_service.py +++ b/tests/test_decision_service.py @@ -1890,3 +1890,238 @@ 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 variations from User Profile Service. """ + # Create a CMAB experiment following existing pattern + cmab_experiment = entities.Experiment( + id='1323241597', + key='cmab_ups_test', + status='Running', + audienceIds=[], + variations=[ + entities.Variation('128', 'variation_1'), + entities.Variation('129', 'variation_2') + ], + forcedVariations={}, + trafficAllocation=[ + {'entityId': '128', 'endOfRange': 5000}, + {'entityId': '129', 'endOfRange': 10000} + ], + layerId='42', + cmab={'trafficAllocation': 5000} + ) + + # Create a user profile with stored decision for this experiment + user_profile_service = mock.MagicMock() + stored_user_profile = { + 'user_id': 'test_user', + 'experiment_bucket_map': { + '1323241597': { + 'variation_id': '128' # Previously bucketed to variation '128' + } + } + } + user_profile_service.lookup.return_value = stored_user_profile + + self.decision_service.user_profile_service = user_profile_service + user_context = optimizely_user_context.OptimizelyUserContext( + self.optimizely, None, 'test_user', {} + ) + + 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=['$', []] + ) as mock_bucket_to_entity, \ + 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('129', 'variation_2') + ): + + # CMAB service returns variation 129 (different from UPS stored 128) + mock_cmab_service.get_decision.return_value = ( + { + 'variation_id': '129', + 'cmab_uuid': 'test-cmab-uuid' + }, + [] + ) + + user_profile_tracker = user_profile.UserProfileTracker( + 'test_user', user_profile_service, self.decision_service.logger + ) + user_profile_tracker.load_user_profile([], None) + + variation_result = self.decision_service.get_variation( + self.project_config, + cmab_experiment, + user_context, + user_profile_tracker + ) + + # Verify that the variation from CMAB (129) is returned, NOT the UPS stored variation (128) + self.assertEqual('variation_2', variation_result['variation'].key) + self.assertEqual('129', variation_result['variation'].id) + + # Verify UPS lookup was NOT called (because CMAB bypasses UPS) + # The stored variation should not have been used + + def test_get_variation__cmab_experiment__does_not_save_to_ups(self): + """ Test that CMAB experiments do not save variations to User Profile Service. """ + # Create a CMAB experiment following existing pattern + cmab_experiment = entities.Experiment( + id='1323241598', + key='cmab_ups_save_test', + status='Running', + audienceIds=[], + variations=[ + entities.Variation('228', 'var_1'), + entities.Variation('229', 'var_2') + ], + forcedVariations={}, + trafficAllocation=[ + {'entityId': '228', 'endOfRange': 5000}, + {'entityId': '229', 'endOfRange': 10000} + ], + layerId='43', + cmab={'trafficAllocation': 5000} + ) + + user_profile_service = mock.MagicMock() + user_profile_service.lookup.return_value = None + + self.decision_service.user_profile_service = user_profile_service + user_context = optimizely_user_context.OptimizelyUserContext( + self.optimizely, None, 'test_user_save', {} + ) + + 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=['$', []] + ) as mock_bucket_to_entity, \ + 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('229', 'var_2') + ): + + # CMAB service returns a variation + mock_cmab_service.get_decision.return_value = ( + { + 'variation_id': '229', + 'cmab_uuid': 'test-cmab-uuid-save' + }, + [] + ) + + user_profile_tracker = user_profile.UserProfileTracker( + 'test_user_save', user_profile_service, self.decision_service.logger + ) + user_profile_tracker.load_user_profile([], None) + + variation_result = self.decision_service.get_variation( + self.project_config, + cmab_experiment, + user_context, + user_profile_tracker + ) + + # Save the user profile + user_profile_tracker.save_user_profile() + + # Verify that the variation was returned + self.assertEqual('var_2', variation_result['variation'].key) + + # Verify UPS save was NOT called for CMAB experiment + user_profile_service.save.assert_not_called() + + def test_get_variation__non_cmab_experiment__retrieves_from_ups(self): + """ Test that non-CMAB experiments still retrieve from User Profile Service (regression test). """ + # Use a regular (non-CMAB) experiment from project config + experiment = self.project_config.get_experiment_from_key('test_experiment') + + # Get the first variation for this experiment + first_variation = experiment.variations[0] + variation_id = first_variation['id'] + variation_key = first_variation['key'] + + # Create a user profile with stored decision for this experiment + user_profile_service = mock.MagicMock() + stored_user_profile = { + 'user_id': 'test_user', + 'experiment_bucket_map': { + experiment.id: { + 'variation_id': variation_id # Stored variation + } + } + } + user_profile_service.lookup.return_value = stored_user_profile + + self.decision_service.user_profile_service = user_profile_service + user_context = optimizely_user_context.OptimizelyUserContext( + self.optimizely, None, 'test_user', {} + ) + + user_profile_tracker = user_profile.UserProfileTracker( + 'test_user', user_profile_service, self.decision_service.logger + ) + user_profile_tracker.load_user_profile([], None) + + variation_result = self.decision_service.get_variation( + self.project_config, + experiment, + user_context, + user_profile_tracker + ) + + # Verify that the stored variation from UPS was returned + self.assertEqual(variation_key, variation_result['variation'].key) + self.assertEqual(variation_id, variation_result['variation'].id) + + def test_get_variation__non_cmab_experiment__saves_to_ups(self): + """ Test that non-CMAB experiments still save to User Profile Service (regression test). """ + # Use a regular (non-CMAB) experiment from project config + experiment = self.project_config.get_experiment_from_key('test_experiment') + + user_profile_service = mock.MagicMock() + user_profile_service.lookup.return_value = None + + self.decision_service.user_profile_service = user_profile_service + user_context = optimizely_user_context.OptimizelyUserContext( + self.optimizely, None, 'test_user', {} + ) + + with mock.patch.object( + self.decision_service.bucketer, 'bucket' + ) as mock_bucket: + # Mock bucketing to return a specific variation + mock_variation = entities.Variation('111129', 'control') + mock_bucket.return_value = (mock_variation, []) + + user_profile_tracker = user_profile.UserProfileTracker( + 'test_user', user_profile_service, self.decision_service.logger + ) + user_profile_tracker.load_user_profile([], None) + + variation_result = self.decision_service.get_variation( + self.project_config, + experiment, + user_context, + user_profile_tracker + ) + + # Save the user profile + user_profile_tracker.save_user_profile() + + # Verify that the variation was returned + self.assertEqual('control', variation_result['variation'].key) + + # Verify UPS save WAS called for non-CMAB experiment + user_profile_service.save.assert_called_once()