Skip to content

Commit 7ab4943

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

File tree

3 files changed

+235
-2
lines changed

3 files changed

+235
-2
lines changed

‎optimizely/decision_service.py‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,16 @@ 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 = f'Skipping UPS lookup for CMAB experiment "{experiment.key}". ' \
466+
'CMAB decisions are excluded from user profile service.'
467+
self.logger.debug(message)
468+
decide_reasons.append(message)
469+
elif user_profile_tracker is not None and not ignore_user_profile:
461470
variation = self.get_stored_variation(project_config, experiment, user_profile_tracker.get_user_profile())
462471
if variation:
463472
message = f'Returning previously activated variation ID "{variation}" of experiment ' \
@@ -529,11 +538,17 @@ def get_variation(
529538
self.logger.info(message)
530539
decide_reasons.append(message)
531540
# Store this new decision and return the variation for the user
532-
if user_profile_tracker is not None and not ignore_user_profile:
541+
# CMAB experiments are excluded from UPS to preserve dynamic decision-making
542+
if user_profile_tracker is not None and not ignore_user_profile and not experiment.cmab:
533543
try:
534544
user_profile_tracker.update_user_profile(experiment, variation)
535545
except:
536546
self.logger.exception(f'Unable to save user profile for user "{user_id}".')
547+
elif experiment.cmab and user_profile_tracker is not None and not ignore_user_profile:
548+
message = f'Skipping UPS save for CMAB experiment "{experiment.key}". ' \
549+
'CMAB decisions are excluded from user profile service.'
550+
self.logger.debug(message)
551+
decide_reasons.append(message)
537552
return {
538553
'cmab_uuid': cmab_uuid,
539554
'error': False,

‎optimizely/helpers/enums.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ class Errors:
131131
CMAB_FETCH_FAILED: Final = 'CMAB decision fetch failed with status: {}.'
132132
INVALID_CMAB_FETCH_RESPONSE: Final = 'Invalid CMAB fetch response.'
133133
CMAB_FETCH_FAILED_DETAILED: Final = 'Failed to fetch CMAB data for experiment {}.'
134+
CMAB_UPS_EXCLUDED: Final = (
135+
'Skipping UPS {} for CMAB experiment "{}". '
136+
'CMAB decisions are excluded from user profile service.'
137+
)
134138

135139

136140
class ForcedDecisionLogs:

‎tests/test_decision_service.py‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,220 @@ 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+
# Create a UserProfileTracker with a stored variation for the CMAB experiment
1108+
ups = user_profile.UserProfileService()
1109+
upt = user_profile.UserProfileTracker("test_user", ups, self.decision_service.logger)
1110+
upt.user_profile = user_profile.UserProfile(
1111+
"test_user",
1112+
{"111150": {"variation_id": "111152"}}
1113+
)
1114+
1115+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1116+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1117+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1118+
return_value=['$', []]), \
1119+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1120+
mock.patch.object(self.project_config, 'get_variation_from_id',
1121+
return_value=entities.Variation('111151', 'variation_1')), \
1122+
mock.patch.object(self.decision_service, 'get_stored_variation') as mock_get_stored, \
1123+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1124+
1125+
mock_cmab_service.get_decision.return_value = (
1126+
{
1127+
'variation_id': '111151',
1128+
'cmab_uuid': 'test-cmab-uuid-456'
1129+
},
1130+
[]
1131+
)
1132+
1133+
variation_result = self.decision_service.get_variation(
1134+
self.project_config,
1135+
cmab_experiment,
1136+
user,
1137+
upt
1138+
)
1139+
1140+
# UPS lookup should NOT be called for CMAB experiments
1141+
mock_get_stored.assert_not_called()
1142+
1143+
# CMAB service should still be called
1144+
mock_cmab_service.get_decision.assert_called_once()
1145+
1146+
# Should get CMAB decision, not stored variation
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 logged
1152+
self.assertIn(
1153+
'Skipping UPS lookup for CMAB experiment "cmab_experiment". '
1154+
'CMAB decisions are excluded from user profile service.',
1155+
variation_result['reasons']
1156+
)
1157+
1158+
def test_get_variation_cmab_experiment_skips_ups_save(self):
1159+
"""Test that get_variation skips UPS save for CMAB experiments."""
1160+
1161+
# Create a user context
1162+
user = optimizely_user_context.OptimizelyUserContext(
1163+
optimizely_client=None,
1164+
logger=None,
1165+
user_id="test_user",
1166+
user_attributes={}
1167+
)
1168+
1169+
# Create a CMAB experiment
1170+
cmab_experiment = entities.Experiment(
1171+
'111150',
1172+
'cmab_experiment',
1173+
'Running',
1174+
'111150',
1175+
[],
1176+
{},
1177+
[
1178+
entities.Variation('111151', 'variation_1'),
1179+
entities.Variation('111152', 'variation_2')
1180+
],
1181+
[
1182+
{'entityId': '111151', 'endOfRange': 5000},
1183+
{'entityId': '111152', 'endOfRange': 10000}
1184+
],
1185+
cmab={'trafficAllocation': 5000}
1186+
)
1187+
1188+
# Create a UserProfileTracker
1189+
ups = user_profile.UserProfileService()
1190+
upt = user_profile.UserProfileTracker("test_user", ups, self.decision_service.logger)
1191+
1192+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1193+
mock.patch('optimizely.helpers.audience.does_user_meet_audience_conditions', return_value=[True, []]), \
1194+
mock.patch.object(self.decision_service.bucketer, 'bucket_to_entity_id',
1195+
return_value=['$', []]), \
1196+
mock.patch.object(self.decision_service, 'cmab_service') as mock_cmab_service, \
1197+
mock.patch.object(self.project_config, 'get_variation_from_id',
1198+
return_value=entities.Variation('111151', 'variation_1')), \
1199+
mock.patch.object(upt, 'update_user_profile') as mock_update_profile, \
1200+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1201+
1202+
mock_cmab_service.get_decision.return_value = (
1203+
{
1204+
'variation_id': '111151',
1205+
'cmab_uuid': 'test-cmab-uuid-789'
1206+
},
1207+
[]
1208+
)
1209+
1210+
variation_result = self.decision_service.get_variation(
1211+
self.project_config,
1212+
cmab_experiment,
1213+
user,
1214+
upt
1215+
)
1216+
1217+
# UPS save should NOT be called for CMAB experiments
1218+
mock_update_profile.assert_not_called()
1219+
1220+
# Should still get the correct variation
1221+
self.assertEqual(entities.Variation('111151', 'variation_1'), variation_result['variation'])
1222+
self.assertEqual('test-cmab-uuid-789', variation_result['cmab_uuid'])
1223+
self.assertStrictFalse(variation_result['error'])
1224+
1225+
# Verify UPS save exclusion reason is in decisions
1226+
self.assertIn(
1227+
'Skipping UPS save for CMAB experiment "cmab_experiment". '
1228+
'CMAB decisions are excluded from user profile service.',
1229+
variation_result['reasons']
1230+
)
1231+
1232+
def test_get_variation_non_cmab_experiment_still_uses_ups(self):
1233+
"""Test that non-CMAB experiments still use UPS for lookup and save."""
1234+
1235+
# Create a user context
1236+
user = optimizely_user_context.OptimizelyUserContext(
1237+
optimizely_client=None,
1238+
logger=None,
1239+
user_id="test_user",
1240+
user_attributes={}
1241+
)
1242+
1243+
# Create a non-CMAB experiment (cmab is None by default)
1244+
non_cmab_experiment = entities.Experiment(
1245+
'111150',
1246+
'regular_experiment',
1247+
'Running',
1248+
'111150',
1249+
[],
1250+
{},
1251+
[entities.Variation('111151', 'variation_1')],
1252+
[{'entityId': '111151', 'endOfRange': 10000}]
1253+
)
1254+
1255+
# Create a UserProfileTracker with stored variation
1256+
ups = user_profile.UserProfileService()
1257+
upt = user_profile.UserProfileTracker("test_user", ups, self.decision_service.logger)
1258+
upt.user_profile = user_profile.UserProfile(
1259+
"test_user",
1260+
{"111150": {"variation_id": "111151"}}
1261+
)
1262+
1263+
stored_variation = entities.Variation('111151', 'variation_1')
1264+
1265+
with mock.patch('optimizely.helpers.experiment.is_experiment_running', return_value=True), \
1266+
mock.patch.object(self.decision_service, 'get_forced_variation', return_value=[None, []]), \
1267+
mock.patch.object(self.decision_service, 'get_whitelisted_variation', return_value=[None, []]), \
1268+
mock.patch.object(self.decision_service, 'get_stored_variation',
1269+
return_value=stored_variation) as mock_get_stored, \
1270+
mock.patch.object(self.decision_service, 'logger') as mock_logger:
1271+
1272+
variation_result = self.decision_service.get_variation(
1273+
self.project_config,
1274+
non_cmab_experiment,
1275+
user,
1276+
upt
1277+
)
1278+
1279+
# UPS lookup SHOULD be called for non-CMAB experiments
1280+
mock_get_stored.assert_called_once()
1281+
1282+
# Should get the stored variation
1283+
self.assertEqual(stored_variation, variation_result['variation'])
1284+
self.assertIsNone(variation_result['cmab_uuid'])
1285+
self.assertStrictFalse(variation_result['error'])
1286+
1287+
# Should NOT have CMAB UPS exclusion reason
1288+
for reason in variation_result['reasons']:
1289+
self.assertNotIn('CMAB decisions are excluded', reason)
1290+
10771291

10781292
class FeatureFlagDecisionTests(base.BaseTest):
10791293
def setUp(self):

0 commit comments

Comments
 (0)