Skip to content

Commit f15669d

Browse files
committed
[FSSDK-11569][FSSDK-11570] Python: Holdout Project Config update
1 parent 116d23a commit f15669d

File tree

2 files changed

+251
-0
lines changed

2 files changed

+251
-0
lines changed

optimizely/project_config.py

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,38 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
8888
region_value = config.get('region')
8989
self.region: str = region_value or 'US'
9090

91+
self.holdouts: list[dict[str, Any]] = config.get('holdouts', [])
92+
self.holdout_id_map: dict[str, dict[str, Any]] = {}
93+
self.global_holdouts: dict[str, dict[str, Any]] = {}
94+
self.included_holdouts: dict[str, list[dict[str, Any]]] = {}
95+
self.excluded_holdouts: dict[str, list[dict[str, Any]]] = {}
96+
self.flag_holdouts_map: dict[str, list[dict[str, Any]]] = {}
97+
98+
for holdout in self.holdouts:
99+
if holdout.get('status') != 'Running':
100+
continue
101+
102+
holdout_id = holdout['id']
103+
self.holdout_id_map[holdout_id] = holdout
104+
105+
included_flags = holdout.get('includedFlags')
106+
if not included_flags:
107+
# This is a global holdout
108+
self.global_holdouts[holdout_id] = holdout
109+
110+
excluded_flags = holdout.get('excludedFlags')
111+
if excluded_flags:
112+
for flag_id in excluded_flags:
113+
if flag_id not in self.excluded_holdouts:
114+
self.excluded_holdouts[flag_id] = []
115+
self.excluded_holdouts[flag_id].append(holdout)
116+
else:
117+
# This holdout applies to specific flags
118+
for flag_id in included_flags:
119+
if flag_id not in self.included_holdouts:
120+
self.included_holdouts[flag_id] = []
121+
self.included_holdouts[flag_id].append(holdout)
122+
91123
# Utility maps for quick lookup
92124
self.group_id_map: dict[str, entities.Group] = self._generate_key_map(self.groups, 'id', entities.Group)
93125
self.experiment_id_map: dict[str, entities.Experiment] = self._generate_key_map(
@@ -752,3 +784,62 @@ def get_flag_variation(
752784
return variation
753785

754786
return None
787+
788+
def get_holdouts_for_flag(self, flag_key: str) -> list[Any]:
789+
""" Helper method to get holdouts from an applied feature flag.
790+
791+
Args:
792+
flag_key: Key of the feature flag.
793+
794+
Returns:
795+
The holdouts that apply for a specific flag.
796+
"""
797+
feature_flag = self.feature_key_map.get(flag_key)
798+
if not feature_flag:
799+
return []
800+
801+
flag_id = feature_flag.id
802+
803+
# Check cache first
804+
if flag_id in self.flag_holdouts_map:
805+
return self.flag_holdouts_map[flag_id]
806+
807+
holdouts = []
808+
809+
# Add global holdouts that don't exclude this flag
810+
for holdout in self.global_holdouts.values():
811+
is_excluded = False
812+
excluded_flags = holdout.get('excludedFlags')
813+
if excluded_flags:
814+
for excluded_flag_id in excluded_flags:
815+
if excluded_flag_id == flag_id:
816+
is_excluded = True
817+
break
818+
if not is_excluded:
819+
holdouts.append(holdout)
820+
821+
# Add holdouts that specifically include this flag
822+
if flag_id in self.included_holdouts:
823+
holdouts.extend(self.included_holdouts[flag_id])
824+
825+
# Cache the result
826+
self.flag_holdouts_map[flag_id] = holdouts
827+
828+
return holdouts
829+
830+
def get_holdout(self, holdout_id: str) -> Optional[dict[str, Any]]:
831+
""" Helper method to get holdout from holdout ID.
832+
833+
Args:
834+
holdout_id: ID of the holdout.
835+
836+
Returns:
837+
The holdout corresponding to the provided holdout ID.
838+
"""
839+
holdout = self.holdout_id_map.get(holdout_id)
840+
841+
if holdout:
842+
return holdout
843+
844+
self.logger.error(f'Holdout with ID "{holdout_id}" not found.')
845+
return None

tests/test_config.py

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,3 +1375,163 @@ def test_get_variation_from_key_by_experiment_id_missing(self):
13751375
variation = project_config.get_variation_from_key_by_experiment_id(experiment_id, variation_key)
13761376

13771377
self.assertIsNone(variation)
1378+
1379+
1380+
class HoldoutConfigTest(base.BaseTest):
1381+
def setUp(self):
1382+
base.BaseTest.setUp(self)
1383+
1384+
# Create config with holdouts
1385+
config_body_with_holdouts = self.config_dict_with_features.copy()
1386+
1387+
# Use correct feature flag IDs from the datafile
1388+
boolean_feature_id = '91111' # boolean_single_variable_feature
1389+
multi_variate_feature_id = '91114' # test_feature_in_experiment_and_rollout
1390+
string_feature_id = '91112' # test_feature_in_rollout
1391+
1392+
config_body_with_holdouts['holdouts'] = [
1393+
{
1394+
'id': 'holdout_1',
1395+
'key': 'global_holdout',
1396+
'status': 'Running',
1397+
'includedFlags': [],
1398+
'excludedFlags': [boolean_feature_id]
1399+
},
1400+
{
1401+
'id': 'holdout_2',
1402+
'key': 'specific_holdout',
1403+
'status': 'Running',
1404+
'includedFlags': [multi_variate_feature_id],
1405+
'excludedFlags': []
1406+
},
1407+
{
1408+
'id': 'holdout_3',
1409+
'key': 'inactive_holdout',
1410+
'status': 'Inactive',
1411+
'includedFlags': [boolean_feature_id],
1412+
'excludedFlags': []
1413+
}
1414+
]
1415+
1416+
self.config_json_with_holdouts = json.dumps(config_body_with_holdouts)
1417+
opt_obj = optimizely.Optimizely(self.config_json_with_holdouts)
1418+
self.config_with_holdouts = opt_obj.config_manager.get_config()
1419+
1420+
def test_get_holdouts_for_flag__non_existent_flag(self):
1421+
""" Test that get_holdouts_for_flag returns empty array for non-existent flag. """
1422+
1423+
holdouts = self.config_with_holdouts.get_holdouts_for_flag('non_existent_flag')
1424+
self.assertEqual([], holdouts)
1425+
1426+
def test_get_holdouts_for_flag__returns_global_and_specific_holdouts(self):
1427+
""" Test that get_holdouts_for_flag returns global holdouts that do not exclude the flag
1428+
and specific holdouts that include the flag. """
1429+
1430+
holdouts = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')
1431+
self.assertEqual(2, len(holdouts))
1432+
1433+
global_holdout = next((h for h in holdouts if h['key'] == 'global_holdout'), None)
1434+
self.assertIsNotNone(global_holdout)
1435+
self.assertEqual('holdout_1', global_holdout['id'])
1436+
1437+
specific_holdout = next((h for h in holdouts if h['key'] == 'specific_holdout'), None)
1438+
self.assertIsNotNone(specific_holdout)
1439+
self.assertEqual('holdout_2', specific_holdout['id'])
1440+
1441+
def test_get_holdouts_for_flag__excludes_global_holdouts_for_excluded_flags(self):
1442+
""" Test that get_holdouts_for_flag does not return global holdouts that exclude the flag. """
1443+
1444+
holdouts = self.config_with_holdouts.get_holdouts_for_flag('boolean_single_variable_feature')
1445+
self.assertEqual(0, len(holdouts))
1446+
1447+
global_holdout = next((h for h in holdouts if h['key'] == 'global_holdout'), None)
1448+
self.assertIsNone(global_holdout)
1449+
1450+
def test_get_holdouts_for_flag__caches_results(self):
1451+
""" Test that get_holdouts_for_flag caches results for subsequent calls. """
1452+
1453+
holdouts1 = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')
1454+
holdouts2 = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_experiment_and_rollout')
1455+
1456+
# Should be the same object (cached)
1457+
self.assertIs(holdouts1, holdouts2)
1458+
self.assertEqual(2, len(holdouts1))
1459+
1460+
def test_get_holdouts_for_flag__returns_only_global_for_non_targeted_flags(self):
1461+
""" Test that get_holdouts_for_flag returns only global holdouts for flags not specifically targeted. """
1462+
1463+
holdouts = self.config_with_holdouts.get_holdouts_for_flag('test_feature_in_rollout')
1464+
1465+
# Should only include global holdout (not excluded and no specific targeting)
1466+
self.assertEqual(1, len(holdouts))
1467+
self.assertEqual('global_holdout', holdouts[0]['key'])
1468+
1469+
def test_get_holdout__returns_holdout_for_valid_id(self):
1470+
""" Test that get_holdout returns holdout when valid ID is provided. """
1471+
1472+
holdout = self.config_with_holdouts.get_holdout('holdout_1')
1473+
self.assertIsNotNone(holdout)
1474+
self.assertEqual('holdout_1', holdout['id'])
1475+
self.assertEqual('global_holdout', holdout['key'])
1476+
self.assertEqual('Running', holdout['status'])
1477+
1478+
def test_get_holdout__returns_holdout_regardless_of_status(self):
1479+
""" Test that get_holdout returns holdout regardless of status when valid ID is provided. """
1480+
1481+
holdout = self.config_with_holdouts.get_holdout('holdout_2')
1482+
self.assertIsNotNone(holdout)
1483+
self.assertEqual('holdout_2', holdout['id'])
1484+
self.assertEqual('specific_holdout', holdout['key'])
1485+
self.assertEqual('Running', holdout['status'])
1486+
1487+
def test_get_holdout__returns_none_for_non_existent_id(self):
1488+
""" Test that get_holdout returns None for non-existent holdout ID. """
1489+
1490+
holdout = self.config_with_holdouts.get_holdout('non_existent_holdout')
1491+
self.assertIsNone(holdout)
1492+
1493+
def test_get_holdout__logs_error_when_not_found(self):
1494+
""" Test that get_holdout logs error when holdout is not found. """
1495+
1496+
with mock.patch.object(self.config_with_holdouts, 'logger') as mock_logger:
1497+
result = self.config_with_holdouts.get_holdout('invalid_holdout_id')
1498+
1499+
self.assertIsNone(result)
1500+
mock_logger.error.assert_called_once_with('Holdout with ID "invalid_holdout_id" not found.')
1501+
1502+
def test_get_holdout__does_not_log_when_found(self):
1503+
""" Test that get_holdout does not log when holdout is found. """
1504+
1505+
with mock.patch.object(self.config_with_holdouts, 'logger') as mock_logger:
1506+
result = self.config_with_holdouts.get_holdout('holdout_1')
1507+
1508+
self.assertIsNotNone(result)
1509+
mock_logger.error.assert_not_called()
1510+
1511+
def test_holdout_initialization__categorizes_holdouts_properly(self):
1512+
""" Test that holdouts are properly categorized during initialization. """
1513+
1514+
self.assertIn('holdout_1', self.config_with_holdouts.holdout_id_map)
1515+
self.assertIn('holdout_2', self.config_with_holdouts.holdout_id_map)
1516+
self.assertIn('holdout_1', self.config_with_holdouts.global_holdouts)
1517+
1518+
# Use correct feature flag IDs
1519+
boolean_feature_id = '91111'
1520+
multi_variate_feature_id = '91114'
1521+
1522+
self.assertIn(multi_variate_feature_id, self.config_with_holdouts.included_holdouts)
1523+
self.assertTrue(len(self.config_with_holdouts.included_holdouts[multi_variate_feature_id]) > 0)
1524+
self.assertNotIn(boolean_feature_id, self.config_with_holdouts.included_holdouts)
1525+
1526+
self.assertIn(boolean_feature_id, self.config_with_holdouts.excluded_holdouts)
1527+
self.assertTrue(len(self.config_with_holdouts.excluded_holdouts[boolean_feature_id]) > 0)
1528+
1529+
def test_holdout_initialization__only_processes_running_holdouts(self):
1530+
""" Test that only running holdouts are processed during initialization. """
1531+
1532+
self.assertNotIn('holdout_3', self.config_with_holdouts.holdout_id_map)
1533+
self.assertNotIn('holdout_3', self.config_with_holdouts.global_holdouts)
1534+
1535+
boolean_feature_id = '91111'
1536+
included_for_boolean = self.config_with_holdouts.included_holdouts.get(boolean_feature_id)
1537+
self.assertIsNone(included_for_boolean)

0 commit comments

Comments
 (0)