Skip to content

Commit ae54f2b

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

File tree

2 files changed

+219
-2
lines changed

2 files changed

+219
-2
lines changed

‎optimizely/decision_service.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,17 @@ 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+
# Skip UPS lookup for CMAB experiments - CMAB decisions are dynamic and
461+
# should not use sticky bucketing via UserProfileService
462+
if experiment.cmab:
463+
if user_profile_tracker is not None and not ignore_user_profile:
464+
message = (
465+
f'Skipping user profile service lookup for CMAB experiment "{experiment.key}". '
466+
f'CMAB decisions should not use sticky bucketing.'
467+
)
468+
self.logger.info(message)
469+
decide_reasons.append(message)
470+
elif user_profile_tracker is not None and not ignore_user_profile:
461471
variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile())
462472
if variation:
463473
message = f'Returning previously activated variation ID "{variation}" of experiment ' \
@@ -529,7 +539,9 @@ def get_variation(
529539
self.logger.info(message)
530540
decide_reasons.append(message)
531541
# Store this new decision and return the variation for the user
532-
if user_profile_tracker is not None and not ignore_user_profile:
542+
# Skip UPS save for CMAB experiments - CMAB decisions are dynamic
543+
# and should not be persisted via UserProfileService
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: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,211 @@ 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+
# Create a user context
1081+
user = optimizely_user_context.OptimizelyUserContext(
1082+
optimizely_client=None,
1083+
logger=None,
1084+
user_id="test_user",
1085+
user_attributes={}
1086+
)
1087+
1088+
# Create a CMAB experiment
1089+
cmab_experiment = entities.Experiment(
1090+
'111150',
1091+
'cmab_experiment',
1092+
'Running',
1093+
'111150',
1094+
[],
1095+
{},
1096+
[
1097+
entities.Variation('111151', 'variation_1'),
1098+
entities.Variation('111152', 'variation_2')
1099+
],
1100+
[
1101+
{'entityId': '111151', 'endOfRange': 5000},
1102+
{'entityId': '111152', 'endOfRange': 10000}
1103+
],
1104+
cmab={'trafficAllocation': 5000}
1105+
)
1106+
1107+
# Set up a user profile tracker with a stored variation for this experiment
1108+
ups = user_profile.UserProfileService()
1109+
tracker = user_profile.UserProfileTracker("test_user", ups, self.decision_service.logger)
1110+
stored_profile = user_profile.UserProfile(
1111+
"test_user",
1112+
{"111150": {"variation_id": "111152"}}
1113+
)
1114+
tracker.user_profile = stored_profile
1115+
1116+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1117+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1118+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1119+
return_value=['$', []]), \
1120+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1121+
mock.patch.object(self.project_config, 'get_variation_from_id',
1122+
return_value=entities.Variation('111151', 'variation_1')), \
1123+
mock.patch.object(self.decision_service, 'get_stored_variation') as mock_stored_variation, \
1124+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1125+
1126+
# Configure CMAB service to return a decision
1127+
mock_cmab_service.get_decision.return_value = (
1128+
{
1129+
'variation_id': '111151',
1130+
'cmab_uuid': 'test-cmab-uuid-456'
1131+
},
1132+
[]
1133+
)
1134+
1135+
# Call get_variation with the CMAB experiment and a user profile tracker
1136+
variation_result = self.decision_service.get_variation(
1137+
self.project_config,
1138+
cmab_experiment,
1139+
user,
1140+
tracker
1141+
)
1142+
1143+
# Verify UPS lookup was NOT called for CMAB experiment
1144+
mock_stored_variation.assert_not_called()
1145+
1146+
# Verify the CMAB decision was used instead
1147+
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
1148+
self.assertEqual('test-cmab-uuid-456', variation_result['cmab_uuid'])
1149+
self.assertStrictFalse(variation_result['error'])
1150+
1151+
# Verify the UPS exclusion reason is in the decision reasons
1152+
ups_skip_reasons = [r for r in variation_result['reasons']
1153+
if 'Skipping user profile service lookup for CMAB experiment' in r]
1154+
self.assertEqual(len(ups_skip_reasons), 1)
1155+
1156+
def test_get_variation_cmab_experiment_skips_ups_save(self):
1157+
"""Test that get_variation skips UPS save for CMAB experiments."""
1158+
1159+
# Create a user context
1160+
user = optimizely_user_context.OptimizelyUserContext(
1161+
optimizely_client=None,
1162+
logger=None,
1163+
user_id="test_user",
1164+
user_attributes={}
1165+
)
1166+
1167+
# Create a CMAB experiment
1168+
cmab_experiment = entities.Experiment(
1169+
'111150',
1170+
'cmab_experiment',
1171+
'Running',
1172+
'111150',
1173+
[],
1174+
{},
1175+
[
1176+
entities.Variation('111151', 'variation_1'),
1177+
entities.Variation('111152', 'variation_2')
1178+
],
1179+
[
1180+
{'entityId': '111151', 'endOfRange': 5000},
1181+
{'entityId': '111152', 'endOfRange': 10000}
1182+
],
1183+
cmab={'trafficAllocation': 5000}
1184+
)
1185+
1186+
# Set up a user profile tracker
1187+
ups = user_profile.UserProfileService()
1188+
tracker = user_profile.UserProfileTracker("test_user", ups, self.decision_service.logger)
1189+
1190+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1191+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1192+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1193+
return_value=['$', []]), \
1194+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1195+
mock.patch.object(self.project_config, 'get_variation_from_id',
1196+
return_value=entities.Variation('111151', 'variation_1')), \
1197+
mock.patch.object(tracker, 'update_user_profile') as mock_update_profile, \
1198+
mock.patch.object(self.decision_service, 'logger'):
1199+
1200+
# Configure CMAB service to return a decision
1201+
mock_cmab_service.get_decision.return_value = (
1202+
{
1203+
'variation_id': '111151',
1204+
'cmab_uuid': 'test-cmab-uuid-789'
1205+
},
1206+
[]
1207+
)
1208+
1209+
# Call get_variation with the CMAB experiment and a user profile tracker
1210+
variation_result = self.decision_service.get_variation(
1211+
self.project_config,
1212+
cmab_experiment,
1213+
user,
1214+
tracker
1215+
)
1216+
1217+
# Verify the variation was assigned
1218+
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
1219+
1220+
# Verify UPS save was NOT called for CMAB experiment
1221+
mock_update_profile.assert_not_called()
1222+
1223+
def test_get_variation_non_cmab_experiment_still_uses_ups(self):
1224+
"""Test that get_variation still uses UPS for non-CMAB experiments."""
1225+
1226+
# Create a user context
1227+
user = optimizely_user_context.OptimizelyUserContext(
1228+
optimizely_client=None,
1229+
logger=None,
1230+
user_id="test_user",
1231+
user_attributes={}
1232+
)
1233+
1234+
# Create a non-CMAB experiment (no cmab attribute)
1235+
experiment = entities.Experiment(
1236+
'111150',
1237+
'regular_experiment',
1238+
'Running',
1239+
'111150',
1240+
[],
1241+
{},
1242+
[
1243+
entities.Variation('111151', 'variation_1'),
1244+
],
1245+
[
1246+
{'entityId': '111151', 'endOfRange': 10000},
1247+
],
1248+
)
1249+
1250+
# Set up a user profile tracker with a stored variation
1251+
ups = user_profile.UserProfileService()
1252+
tracker = user_profile.UserProfileTracker("test_user", ups, self.decision_service.logger)
1253+
1254+
stored_variation = entities.Variation('111151', 'variation_1')
1255+
1256+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1257+
mock.patch.object(self.decision_service, 'get_forced_variation', return_value=[None, []]), \
1258+
mock.patch.object(self.decision_service, 'get_whitelisted_variation', return_value=[None, []]), \
1259+
mock.patch.object(self.decision_service, 'get_stored_variation',
1260+
return_value=stored_variation) as mock_stored_variation, \
1261+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1262+
1263+
# Call get_variation with a non-CMAB experiment and a user profile tracker
1264+
variation_result = self.decision_service.get_variation(
1265+
self.project_config,
1266+
experiment,
1267+
user,
1268+
tracker
1269+
)
1270+
1271+
# Verify UPS lookup WAS called for non-CMAB experiment
1272+
mock_stored_variation.assert_called_once()
1273+
1274+
# Verify the stored variation was returned
1275+
self.assertEqual(stored_variation, variation_result['variation'])
1276+
1277+
# Verify there is no UPS exclusion reason
1278+
ups_skip_reasons = [r for r in variation_result['reasons']
1279+
if 'Skipping user profile service lookup for CMAB experiment' in r]
1280+
self.assertEqual(len(ups_skip_reasons), 0)
1281+
10771282

10781283
class FeatureFlagDecisionTests(base.BaseTest):
10791284
def setUp(self):

0 commit comments

Comments
 (0)