Skip to content

Commit ac0fa24

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

2 files changed

Lines changed: 197 additions & 2 deletions

File tree

‎optimizely/decision_service.py‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,14 @@ 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 lookup 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+
message = f'Skipping UPS lookup for CMAB experiment "{experiment.key}".'
465+
self.logger.info(message)
466+
decide_reasons.append(message)
467+
elif user_profile_tracker is not None and not ignore_user_profile:
461468
variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile())
462469
if variation:
463470
message = f'Returning previously activated variation ID "{variation}" of experiment ' \
@@ -529,7 +536,14 @@ def get_variation(
529536
self.logger.info(message)
530537
decide_reasons.append(message)
531538
# Store this new decision and return the variation for the user
532-
if user_profile_tracker is not None and not ignore_user_profile:
539+
# CMAB experiments are excluded from UPS save because UPS maintains decisions
540+
# across the experiment lifetime without considering TTL or user attributes,
541+
# which contradicts CMAB's dynamic nature.
542+
if experiment.cmab:
543+
message = f'Skipping UPS save for CMAB experiment "{experiment.key}".'
544+
self.logger.info(message)
545+
decide_reasons.append(message)
546+
elif user_profile_tracker is not None and not ignore_user_profile:
533547
try:
534548
user_profile_tracker.update_user_profile(experiment, variation)
535549
except:

‎tests/test_decision_service.py‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,187 @@ 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+
user_profile_service = user_profile.UserProfileService()
1088+
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)
1089+
1090+
# Create a CMAB experiment
1091+
cmab_experiment = entities.Experiment(
1092+
'111150',
1093+
'cmab_experiment',
1094+
'Running',
1095+
[],
1096+
[
1097+
entities.Variation('111151', 'variation_1'),
1098+
entities.Variation('111152', 'variation_2')
1099+
],
1100+
{},
1101+
[
1102+
{'entityId': '111151', 'endOfRange': 5000},
1103+
{'entityId': '111152', 'endOfRange': 10000}
1104+
],
1105+
'111150',
1106+
cmab={'trafficAllocation': 5000}
1107+
)
1108+
1109+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1110+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1111+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1112+
return_value=['$', []]), \
1113+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1114+
mock.patch.object(self.project_config, 'get_variation_from_id',
1115+
return_value=entities.Variation('111151', 'variation_1')), \
1116+
mock.patch(
1117+
"optimizely.decision_service.DecisionService.get_stored_variation"
1118+
) as mock_get_stored_variation, \
1119+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1120+
1121+
mock_cmab_service.get_decision.return_value = (
1122+
{'variation_id': '111151', 'cmab_uuid': 'test-cmab-uuid'},
1123+
[]
1124+
)
1125+
1126+
variation_result = self.decision_service.get_variation(
1127+
self.project_config,
1128+
cmab_experiment,
1129+
user,
1130+
user_profile_tracker
1131+
)
1132+
reasons = variation_result['reasons']
1133+
1134+
# Assert that stored variation was NOT looked up for CMAB experiment
1135+
self.assertEqual(0, mock_get_stored_variation.call_count)
1136+
1137+
# Assert that the UPS skip reason is in the decision reasons
1138+
self.assertIn(
1139+
'Skipping UPS lookup for CMAB experiment "cmab_experiment".',
1140+
reasons
1141+
)
1142+
1143+
# Assert the logger was called with the skip message
1144+
mock_logger.info.assert_any_call(
1145+
'Skipping UPS lookup for CMAB experiment "cmab_experiment".'
1146+
)
1147+
1148+
def test_get_variation__cmab_experiment_skips_ups_save(self):
1149+
"""Test that get_variation skips UPS save for CMAB experiments."""
1150+
1151+
user = optimizely_user_context.OptimizelyUserContext(
1152+
optimizely_client=None,
1153+
logger=None,
1154+
user_id="test_user",
1155+
user_attributes={}
1156+
)
1157+
1158+
user_profile_service = user_profile.UserProfileService()
1159+
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)
1160+
1161+
# Create a CMAB experiment
1162+
cmab_experiment = entities.Experiment(
1163+
'111150',
1164+
'cmab_experiment',
1165+
'Running',
1166+
[],
1167+
[
1168+
entities.Variation('111151', 'variation_1'),
1169+
entities.Variation('111152', 'variation_2')
1170+
],
1171+
{},
1172+
[
1173+
{'entityId': '111151', 'endOfRange': 5000},
1174+
{'entityId': '111152', 'endOfRange': 10000}
1175+
],
1176+
'111150',
1177+
cmab={'trafficAllocation': 5000}
1178+
)
1179+
1180+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1181+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1182+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1183+
return_value=['$', []]), \
1184+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1185+
mock.patch.object(self.project_config, 'get_variation_from_id',
1186+
return_value=entities.Variation('111151', 'variation_1')), \
1187+
mock.patch.object(user_profile_tracker, 'update_user_profile') as mock_update_profile, \
1188+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1189+
1190+
mock_cmab_service.get_decision.return_value = (
1191+
{'variation_id': '111151', 'cmab_uuid': 'test-cmab-uuid'},
1192+
[]
1193+
)
1194+
1195+
variation_result = self.decision_service.get_variation(
1196+
self.project_config,
1197+
cmab_experiment,
1198+
user,
1199+
user_profile_tracker
1200+
)
1201+
reasons = variation_result['reasons']
1202+
1203+
# Assert that user profile was NOT updated for CMAB experiment
1204+
self.assertEqual(0, mock_update_profile.call_count)
1205+
1206+
# Assert that the UPS save skip reason is in the decision reasons
1207+
self.assertIn(
1208+
'Skipping UPS save for CMAB experiment "cmab_experiment".',
1209+
reasons
1210+
)
1211+
1212+
# Assert the logger was called with the skip message
1213+
mock_logger.info.assert_any_call(
1214+
'Skipping UPS save for CMAB experiment "cmab_experiment".'
1215+
)
1216+
1217+
def test_get_variation__non_cmab_experiment_uses_ups_normally(self):
1218+
"""Test that get_variation still uses UPS for non-CMAB experiments."""
1219+
1220+
user = optimizely_user_context.OptimizelyUserContext(
1221+
optimizely_client=None,
1222+
logger=None,
1223+
user_id="test_user",
1224+
user_attributes={}
1225+
)
1226+
1227+
user_profile_service = user_profile.UserProfileService()
1228+
user_profile_tracker = user_profile.UserProfileTracker(user.user_id, user_profile_service)
1229+
experiment = self.project_config.get_experiment_from_key("test_experiment")
1230+
1231+
with mock.patch(
1232+
"optimizely.decision_service.DecisionService.get_whitelisted_variation",
1233+
return_value=[None, []],
1234+
), mock.patch(
1235+
"optimizely.decision_service.DecisionService.get_stored_variation",
1236+
return_value=entities.Variation("111128", "control"),
1237+
) as mock_get_stored_variation, mock.patch(
1238+
"optimizely.helpers.audience.does_user_meet_audience_conditions"
1239+
) as mock_audience_check, mock.patch(
1240+
"optimizely.bucketer.Bucketer.bucket"
1241+
) as mock_bucket:
1242+
variation_result = self.decision_service.get_variation(
1243+
self.project_config, experiment, user, user_profile_tracker
1244+
)
1245+
variation = variation_result['variation']
1246+
reasons = variation_result['reasons']
1247+
1248+
# Assert that stored variation IS looked up for non-CMAB experiment
1249+
mock_get_stored_variation.assert_called_once()
1250+
self.assertEqual(entities.Variation("111128", "control"), variation)
1251+
1252+
# Assert that UPS skip message is NOT in the reasons
1253+
self.assertNotIn(
1254+
'Skipping UPS lookup for CMAB experiment "test_experiment".',
1255+
reasons
1256+
)
1257+
10771258

10781259
class FeatureFlagDecisionTests(base.BaseTest):
10791260
def setUp(self):

0 commit comments

Comments
 (0)