Skip to content

Commit 911a92f

Browse files
jaeoptjaeopt
committed
[AI-FSSDK] [FSSDK-12262] Exclude CMAB from UserProfileService
1 parent 88b0644 commit 911a92f

2 files changed

Lines changed: 183 additions & 2 deletions

File tree

‎optimizely/decision_service.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,18 @@ def get_variation(
457457
}
458458

459459
# Check to see if user has a decision available for the given experiment
460-
if user_profile_tracker is not None and not ignore_user_profile:
460+
# CMAB experiments are excluded from UPS because UPS maintains decisions
461+
# across the experiment lifetime without considering TTL or user attributes,
462+
# which contradicts CMAB's dynamic nature.
463+
if experiment.cmab:
464+
if user_profile_tracker is not None and not ignore_user_profile:
465+
message = (
466+
f'Skipping user profile service for CMAB experiment "{experiment.key}". '
467+
f'CMAB decisions are excluded from UPS.'
468+
)
469+
self.logger.info(message)
470+
decide_reasons.append(message)
471+
elif user_profile_tracker is not None and not ignore_user_profile:
461472
variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile())
462473
if variation:
463474
message = f'Returning previously activated variation ID "{variation}" of experiment ' \
@@ -529,7 +540,8 @@ def get_variation(
529540
self.logger.info(message)
530541
decide_reasons.append(message)
531542
# Store this new decision and return the variation for the user
532-
if user_profile_tracker is not None and not ignore_user_profile:
543+
# CMAB experiments are excluded from UPS to preserve dynamic decision-making
544+
if user_profile_tracker is not None and not ignore_user_profile and not experiment.cmab:
533545
try:
534546
user_profile_tracker.update_user_profile(experiment, variation)
535547
except:

‎tests/test_decision_service.py‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,175 @@ def test_get_variation_cmab_experiment_with_whitelisted_variation(self):
10741074
mock_bucket.assert_not_called()
10751075
mock_cmab_decision.assert_not_called()
10761076

1077+
def test_get_variation_cmab_experiment_skips_ups_lookup(self):
1078+
"""Test that get_variation skips UPS lookup for CMAB experiments."""
1079+
1080+
user = optimizely_user_context.OptimizelyUserContext(
1081+
optimizely_client=None,
1082+
logger=None,
1083+
user_id="test_user",
1084+
user_attributes={}
1085+
)
1086+
1087+
cmab_experiment = entities.Experiment(
1088+
'111150',
1089+
'cmab_experiment',
1090+
'Running',
1091+
'111150',
1092+
[],
1093+
{},
1094+
[
1095+
entities.Variation('111151', 'variation_1'),
1096+
entities.Variation('111152', 'variation_2')
1097+
],
1098+
[
1099+
{'entityId': '111151', 'endOfRange': 5000},
1100+
{'entityId': '111152', 'endOfRange': 10000}
1101+
],
1102+
cmab={'trafficAllocation': 5000}
1103+
)
1104+
1105+
user_profile_service = user_profile.UserProfileService()
1106+
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)
1107+
1108+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1109+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1110+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1111+
return_value=['$', []]), \
1112+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1113+
mock.patch.object(self.project_config, 'get_variation_from_id',
1114+
return_value=entities.Variation('111151', 'variation_1')), \
1115+
mock.patch.object(self.decision_service, 'get_stored_variation') as mock_get_stored, \
1116+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1117+
1118+
mock_cmab_service.get_decision.return_value = (
1119+
{'variation_id': '111151', 'cmab_uuid': 'test-cmab-uuid-123'},
1120+
[]
1121+
)
1122+
1123+
variation_result = self.decision_service.get_variation(
1124+
self.project_config,
1125+
cmab_experiment,
1126+
user,
1127+
user_profile_tracker
1128+
)
1129+
1130+
# UPS lookup should NOT be called for CMAB experiments
1131+
mock_get_stored.assert_not_called()
1132+
1133+
# Should still return valid variation from CMAB
1134+
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
1135+
self.assertEqual('test-cmab-uuid-123', variation_result['cmab_uuid'])
1136+
self.assertStrictFalse(variation_result['error'])
1137+
1138+
# Should log the UPS exclusion reason
1139+
mock_logger.info.assert_any_call(
1140+
'Skipping user profile service for CMAB experiment "cmab_experiment". '
1141+
'CMAB decisions are excluded from UPS.'
1142+
)
1143+
1144+
# Decision reasons should include UPS exclusion message
1145+
self.assertIn(
1146+
'Skipping user profile service for CMAB experiment "cmab_experiment". '
1147+
'CMAB decisions are excluded from UPS.',
1148+
variation_result['reasons']
1149+
)
1150+
1151+
def test_get_variation_cmab_experiment_skips_ups_save(self):
1152+
"""Test that get_variation does not save to UPS for CMAB experiments."""
1153+
1154+
user = optimizely_user_context.OptimizelyUserContext(
1155+
optimizely_client=None,
1156+
logger=None,
1157+
user_id="test_user",
1158+
user_attributes={}
1159+
)
1160+
1161+
cmab_experiment = entities.Experiment(
1162+
'111150',
1163+
'cmab_experiment',
1164+
'Running',
1165+
'111150',
1166+
[],
1167+
{},
1168+
[
1169+
entities.Variation('111151', 'variation_1'),
1170+
entities.Variation('111152', 'variation_2')
1171+
],
1172+
[
1173+
{'entityId': '111151', 'endOfRange': 5000},
1174+
{'entityId': '111152', 'endOfRange': 10000}
1175+
],
1176+
cmab={'trafficAllocation': 5000}
1177+
)
1178+
1179+
user_profile_service = user_profile.UserProfileService()
1180+
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)
1181+
1182+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1183+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1184+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1185+
return_value=['$', []]), \
1186+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1187+
mock.patch.object(self.project_config, 'get_variation_from_id',
1188+
return_value=entities.Variation('111151', 'variation_1')), \
1189+
mock.patch.object(user_profile_tracker, 'update_user_profile') as mock_update_profile, \
1190+
mock.patch.object(self.decision_service, 'logger'):
1191+
1192+
mock_cmab_service.get_decision.return_value = (
1193+
{'variation_id': '111151', 'cmab_uuid': 'test-cmab-uuid-123'},
1194+
[]
1195+
)
1196+
1197+
variation_result = self.decision_service.get_variation(
1198+
self.project_config,
1199+
cmab_experiment,
1200+
user,
1201+
user_profile_tracker
1202+
)
1203+
1204+
# UPS save should NOT be called for CMAB experiments
1205+
mock_update_profile.assert_not_called()
1206+
1207+
# Should still return valid variation
1208+
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
1209+
self.assertStrictFalse(variation_result['error'])
1210+
1211+
def test_get_variation_non_cmab_experiment_still_uses_ups(self):
1212+
"""Test that non-CMAB experiments still use UPS normally."""
1213+
1214+
user = optimizely_user_context.OptimizelyUserContext(
1215+
optimizely_client=None,
1216+
logger=None,
1217+
user_id="test_user",
1218+
user_attributes={}
1219+
)
1220+
1221+
experiment = self.project_config.get_experiment_from_key('test_experiment')
1222+
1223+
user_profile_service = user_profile.UserProfileService()
1224+
user_profile_tracker_instance = user_profile.UserProfileTracker(user.user_id, user_profile_service)
1225+
1226+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1227+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1228+
mock.patch.object(self.decision_service, 'get_stored_variation',
1229+
return_value=entities.Variation('111128', 'control')) as mock_get_stored, \
1230+
mock.patch.object(self.decision_service, 'logger'):
1231+
1232+
variation_result = self.decision_service.get_variation(
1233+
self.project_config,
1234+
experiment,
1235+
user,
1236+
user_profile_tracker_instance
1237+
)
1238+
1239+
# UPS lookup SHOULD be called for non-CMAB experiments
1240+
mock_get_stored.assert_called_once()
1241+
1242+
# Should return stored variation
1243+
self.assertEqual(entities.Variation('111128', 'control'), variation_result['variation'])
1244+
self.assertStrictFalse(variation_result['error'])
1245+
10771246

10781247
class FeatureFlagDecisionTests(base.BaseTest):
10791248
def setUp(self):

0 commit comments

Comments
 (0)