Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions optimizely/decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,18 @@ 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 are excluded from UPS because UPS maintains decisions
# across the experiment lifetime without considering TTL or user attributes,
# which contradicts CMAB's dynamic nature.
if experiment.cmab:
if user_profile_tracker is not None and not ignore_user_profile:
message = (
f'Skipping user profile service for CMAB experiment "{experiment.key}". '
f'CMAB decisions are excluded from UPS.'
)
self.logger.info(message)
decide_reasons.append(message)
elif user_profile_tracker is not None and not ignore_user_profile:
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 ' \
Expand Down Expand Up @@ -529,7 +540,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 are excluded from UPS to preserve dynamic decision-making
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:
Expand Down
169 changes: 169 additions & 0 deletions tests/test_decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,175 @@ 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_skips_ups_lookup(self):
"""Test that get_variation skips UPS lookup for CMAB experiments."""

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'),
entities.Variation('111152', 'variation_2')
],
[
{'entityId': '111151', 'endOfRange': 5000},
{'entityId': '111152', 'endOfRange': 10000}
],
cmab={'trafficAllocation': 5000}
)

user_profile_service = user_profile.UserProfileService()
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)

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(self.decision_service, 'get_stored_variation') as mock_get_stored, \
mock.patch.object(self.decision_service, 'logger') as mock_logger:

mock_cmab_service.get_decision.return_value = (
{'variation_id': '111151', 'cmab_uuid': 'test-cmab-uuid-123'},
[]
)

variation_result = self.decision_service.get_variation(
self.project_config,
cmab_experiment,
user,
user_profile_tracker
)

# UPS lookup should NOT be called for CMAB experiments
mock_get_stored.assert_not_called()

# Should still return valid variation from CMAB
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
self.assertEqual('test-cmab-uuid-123', variation_result['cmab_uuid'])
self.assertStrictFalse(variation_result['error'])

# Should log the UPS exclusion reason
mock_logger.info.assert_any_call(
'Skipping user profile service for CMAB experiment "cmab_experiment". '
'CMAB decisions are excluded from UPS.'
)

# Decision reasons should include UPS exclusion message
self.assertIn(
'Skipping user profile service for CMAB experiment "cmab_experiment". '
'CMAB decisions are excluded from UPS.',
variation_result['reasons']
)

def test_get_variation_cmab_experiment_skips_ups_save(self):
"""Test that get_variation does not save to UPS for CMAB experiments."""

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'),
entities.Variation('111152', 'variation_2')
],
[
{'entityId': '111151', 'endOfRange': 5000},
{'entityId': '111152', 'endOfRange': 10000}
],
cmab={'trafficAllocation': 5000}
)

user_profile_service = user_profile.UserProfileService()
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)

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_profile, \
mock.patch.object(self.decision_service, 'logger'):

mock_cmab_service.get_decision.return_value = (
{'variation_id': '111151', 'cmab_uuid': 'test-cmab-uuid-123'},
[]
)

variation_result = self.decision_service.get_variation(
self.project_config,
cmab_experiment,
user,
user_profile_tracker
)

# UPS save should NOT be called for CMAB experiments
mock_update_profile.assert_not_called()

# Should still return valid variation
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
self.assertStrictFalse(variation_result['error'])

def test_get_variation_non_cmab_experiment_still_uses_ups(self):
"""Test that non-CMAB experiments still use UPS normally."""

user = optimizely_user_context.OptimizelyUserContext(
optimizely_client=None,
logger=None,
user_id="test_user",
user_attributes={}
)

experiment = self.project_config.get_experiment_from_key('test_experiment')

user_profile_service = user_profile.UserProfileService()
user_profile_tracker_instance = user_profile.UserProfileTracker(user.user_id, user_profile_service)

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_stored_variation',
return_value=entities.Variation('111128', 'control')) as mock_get_stored, \
mock.patch.object(self.decision_service, 'logger'):

variation_result = self.decision_service.get_variation(
self.project_config,
experiment,
user,
user_profile_tracker_instance
)

# UPS lookup SHOULD be called for non-CMAB experiments
mock_get_stored.assert_called_once()

# Should return stored variation
self.assertEqual(entities.Variation('111128', 'control'), variation_result['variation'])
self.assertStrictFalse(variation_result['error'])


class FeatureFlagDecisionTests(base.BaseTest):
def setUp(self):
Expand Down
Loading