diff --git a/optimizely/decision_service.py b/optimizely/decision_service.py index be2be2c5..11188908 100644 --- a/optimizely/decision_service.py +++ b/optimizely/decision_service.py @@ -610,6 +610,24 @@ def get_variation_for_rollout( return Decision(experiment=rule, variation=forced_decision_variation, source=enums.DecisionSources.ROLLOUT, cmab_uuid=None), decide_reasons + # Check local holdouts targeting this specific delivery rule (FSSDK-12369) + local_holdouts = project_config.get_holdouts_for_rule(rule.id) + for holdout in local_holdouts: + local_holdout_decision = self.get_variation_for_holdout( + holdout, user_context, project_config + ) + decide_reasons.extend(local_holdout_decision['reasons']) + + local_decision = local_holdout_decision['decision'] + if local_decision.variation is not None: + message = ( + f"The user '{user_id}' is bucketed into local holdout '{holdout.key}' " + f"for delivery rule '{rule.key}'." + ) + self.logger.info(message) + decide_reasons.append(message) + return local_decision, decide_reasons + bucketing_id, bucket_reasons = self._get_bucketing_id(user_id, attributes) decide_reasons += bucket_reasons @@ -733,9 +751,9 @@ def get_decision_for_flag( reasons = decide_reasons.copy() if decide_reasons else [] user_id = user_context.user_id - # Check holdouts - holdouts = project_config.get_holdouts_for_flag(feature_flag.key) - for holdout in holdouts: + # Check global holdouts (flag level — before any rules are evaluated) + global_holdouts = project_config.get_global_holdouts() + for holdout in global_holdouts: holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config) reasons.extend(holdout_decision['reasons']) @@ -756,7 +774,7 @@ def get_decision_for_flag( 'reasons': reasons } - # If no holdout decision, check experiments then rollouts + # If no global holdout decision, check experiments then rollouts if feature_flag.experimentIds: for experiment_id in feature_flag.experimentIds: experiment = project_config.get_experiment_from_id(experiment_id) @@ -778,6 +796,28 @@ def get_decision_for_flag( 'reasons': reasons } + # Check local holdouts targeting this specific experiment rule (FSSDK-12369) + local_holdouts = project_config.get_holdouts_for_rule(experiment.id) + for holdout in local_holdouts: + local_holdout_decision = self.get_variation_for_holdout( + holdout, user_context, project_config + ) + reasons.extend(local_holdout_decision['reasons']) + + local_decision = local_holdout_decision['decision'] + if local_decision.variation is not None: + message = ( + f"The user '{user_id}' is bucketed into local holdout '{holdout.key}' " + f"for experiment rule '{experiment.key}'." + ) + self.logger.info(message) + reasons.append(message) + return { + 'decision': local_holdout_decision['decision'], + 'error': False, + 'reasons': reasons + } + # Get variation for experiment variation_result = self.get_variation( project_config, experiment, user_context, user_profile_tracker, reasons, decide_options diff --git a/optimizely/entities.py b/optimizely/entities.py index f67dee89..e15595b4 100644 --- a/optimizely/entities.py +++ b/optimizely/entities.py @@ -223,6 +223,7 @@ def __init__( trafficAllocation: list[TrafficAllocation], audienceIds: list[str], audienceConditions: Optional[Sequence[str | list[str]]] = None, + includedRules: Optional[list[str]] = None, **kwargs: Any ): self.id = id @@ -232,6 +233,8 @@ def __init__( self.trafficAllocation = trafficAllocation self.audienceIds = audienceIds self.audienceConditions = audienceConditions + # None = global holdout (applies to all rules); list of rule IDs = local holdout + self.included_rules: Optional[list[str]] = includedRules def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]: """Returns audienceConditions if present, otherwise audienceIds. @@ -241,6 +244,18 @@ def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]: """ return self.audienceConditions if self.audienceConditions is not None else self.audienceIds + @property + def is_global(self) -> bool: + """Check if this is a global holdout (applies to all rules across all flags). + + A holdout is global when includedRules is None (absent from datafile). + An empty list [] is a local holdout that targets no rules (different from global). + + Returns: + True if included_rules is None (global), False otherwise (local). + """ + return self.included_rules is None + @property def is_activated(self) -> bool: """Check if the holdout is activated (running). diff --git a/optimizely/helpers/types.py b/optimizely/helpers/types.py index 6b704c36..7ee51373 100644 --- a/optimizely/helpers/types.py +++ b/optimizely/helpers/types.py @@ -130,3 +130,4 @@ class HoldoutDict(ExperimentDict): Extends ExperimentDict with holdout-specific properties. """ holdoutStatus: HoldoutStatus + includedRules: Optional[list[str]] # None = global holdout; list of rule IDs = local holdout diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 0fb0f35f..5ec5f85b 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -93,7 +93,10 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any): holdouts_data: list[types.HoldoutDict] = config.get('holdouts', []) self.holdouts: list[entities.Holdout] = [] self.holdout_id_map: dict[str, entities.Holdout] = {} - self.flag_holdouts_map: dict[str, list[entities.Holdout]] = {} + # Global holdouts (includedRules is None) — evaluated at flag level before any rule + self.global_holdouts: list[entities.Holdout] = [] + # Rule-level holdouts — map from rule ID to holdouts targeting that rule + self.rule_holdouts_map: dict[str, list[entities.Holdout]] = {} # Convert holdout dicts to Holdout entities for holdout_data in holdouts_data: @@ -108,6 +111,16 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any): # Map by ID for quick lookup self.holdout_id_map[holdout.id] = holdout + # Classify holdout as global or local based on includedRules + if holdout.is_global: + self.global_holdouts.append(holdout) + else: + # Local holdout — register for each targeted rule ID + for rule_id in (holdout.included_rules or []): + if rule_id not in self.rule_holdouts_map: + self.rule_holdouts_map[rule_id] = [] + self.rule_holdouts_map[rule_id].append(holdout) + # Utility maps for quick lookup self.group_id_map: dict[str, entities.Group] = self._generate_key_map(self.groups, 'id', entities.Group) self.experiment_id_map: dict[str, entities.Experiment] = self._generate_key_map( @@ -240,11 +253,6 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any): everyone_else_variation.variables, 'id', entities.Variation.VariableUsage ) - # Map all running holdouts to this flag - applicable_holdouts = list(self.holdout_id_map.values()) - if applicable_holdouts: - self.flag_holdouts_map[feature.key] = applicable_holdouts - rollout = None if len(feature.rolloutId) == 0 else self.rollout_id_map[feature.rolloutId] if rollout: for exp in rollout.experiments: @@ -878,19 +886,30 @@ def get_flag_variation( return None - def get_holdouts_for_flag(self, flag_key: str) -> list[entities.Holdout]: - """ Helper method to get holdouts from an applied feature flag. + def get_global_holdouts(self) -> list[entities.Holdout]: + """Return all global holdouts (includedRules is None). - Args: - flag_key: Key of the feature flag. + Global holdouts are evaluated at flag level before any rule is checked. Returns: - The holdouts that apply for a specific flag as Holdout entity objects. + List of global Holdout entities that are currently running. """ - if not self.holdouts: - return [] + return self.global_holdouts + + def get_holdouts_for_rule(self, rule_id: str) -> list[entities.Holdout]: + """Return local holdouts that target a specific rule. - return self.flag_holdouts_map.get(flag_key, []) + Local holdouts are evaluated per-rule, before the rule's audience and + traffic allocation checks. A rule ID not present in any holdout's + includedRules simply returns an empty list — silently skipped. + + Args: + rule_id: The experiment or delivery rule ID to look up. + + Returns: + List of local Holdout entities targeting the given rule ID. + """ + return self.rule_holdouts_map.get(rule_id, []) def get_holdout(self, holdout_id: str) -> Optional[entities.Holdout]: """ Helper method to get holdout from holdout ID. diff --git a/tests/test_decision_service_holdout.py b/tests/test_decision_service_holdout.py index b5584a37..1bf81c93 100644 --- a/tests/test_decision_service_holdout.py +++ b/tests/test_decision_service_holdout.py @@ -1,4 +1,4 @@ -# Copyright 2025 Optimizely and contributors +# Copyright 2026 Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -1331,3 +1331,356 @@ def test_holdout_with_audience_match(self): self.assertIsNotNone(decision_no_match) finally: opt.close() + + +# --------------------------------------------------------------------------- +# Level 2: Local Holdouts Decision Service Tests (FSSDK-12369) +# --------------------------------------------------------------------------- + +_HOLDOUT_VARIATION = [{'id': 'hvar_1', 'key': 'holdout_control', 'variables': []}] +_FULL_TRAFFIC = [{'entityId': 'hvar_1', 'endOfRange': 10000}] +_NO_TRAFFIC = [{'entityId': 'hvar_1', 'endOfRange': 0}] + + +def _holdout(holdout_id, key, included_rules=None, traffic=None, status='Running'): + """Build a minimal holdout datafile dict.""" + h = { + 'id': holdout_id, + 'key': key, + 'status': status, + 'audienceIds': [], + 'variations': _HOLDOUT_VARIATION, + 'trafficAllocation': traffic or _FULL_TRAFFIC, + } + if included_rules is not None: + h['includedRules'] = included_rules + return h + + +class LocalHoldoutDecisionServiceTest(base.BaseTest): + """Level 2 decision service tests for local holdouts (FSSDK-12369). + + These tests exercise the decision flow branches introduced by local holdouts + and must live in the decision service test file per [UNITTEST] spec. + + Experiment '111127' is the experiment rule for feature 'test_feature_in_experiment'. + Rollout '211111' with rule '211147' (no audience, 60% traffic) is the delivery rule + for 'test_feature_in_rollout'. + """ + + def setUp(self): + base.BaseTest.setUp(self) + self.error_handler = error_handler.NoOpErrorHandler() + self.spy_logger = mock.MagicMock(spec=logger.SimpleLogger) + self.spy_logger.logger = self.spy_logger + self.spy_user_profile_service = mock.MagicMock() + self.spy_cmab_service = mock.MagicMock() + + def tearDown(self): + if hasattr(self, 'opt_obj'): + self.opt_obj.close() + + def _make_opt(self, holdouts): + cfg = self.config_dict_with_features.copy() + cfg['holdouts'] = holdouts + self.opt_obj = optimizely_module.Optimizely(json.dumps(cfg)) + return self.opt_obj + + def _decision_svc(self): + return decision_service.DecisionService( + self.spy_logger, + self.spy_user_profile_service, + self.spy_cmab_service, + ) + + # ------------------------------------------------------------------ + # Branch 1: Global holdout evaluated at flag level + # ------------------------------------------------------------------ + + def test_global_holdout_evaluated_before_any_rule(self): + """User in a global holdout returns holdout decision before any rule is evaluated. + + The global holdout has full traffic allocation and no audience restriction, + so the user must be caught at flag level before experiments are checked. + """ + opt = self._make_opt([_holdout('gh1', 'global_full', traffic=_FULL_TRAFFIC)]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_experiment') + self.assertIsNotNone(feature_flag) + + ds = self._decision_svc() + + # Mock get_variation so we can assert it was NOT called (holdout short-circuits) + with mock.patch.object(ds, 'get_variation', wraps=ds.get_variation) as mock_get_variation: + user_ctx = opt.create_user_context('user_in_global_holdout', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + decision = result['decision'] + self.assertIsNotNone(decision) + # Source must be HOLDOUT + self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT) + # Experiment-level get_variation should not have been called + mock_get_variation.assert_not_called() + + def test_global_holdout_miss_falls_through_to_experiment(self): + """User not in the global holdout falls through to experiment evaluation.""" + # Zero-traffic global holdout — nobody bucketed in + opt = self._make_opt([_holdout('gh1', 'global_zero', traffic=_NO_TRAFFIC)]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_experiment') + self.assertIsNotNone(feature_flag) + + ds = self._decision_svc() + user_ctx = opt.create_user_context('testUserId', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + self.assertIsNotNone(result) + # Source must NOT be HOLDOUT — user fell through to experiment or rollout + decision = result['decision'] + self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT) + + # ------------------------------------------------------------------ + # Branch 2: Local holdout hit — user bucketed into local holdout for rule X + # ------------------------------------------------------------------ + + def test_local_holdout_hit_returns_holdout_decision_for_experiment_rule(self): + """User bucketed into local holdout for experiment rule X returns holdout decision. + + The experiment rule ID is '111127' (test_experiment linked to test_feature_in_experiment). + The local holdout targets only this rule with full traffic. + Audience and traffic allocation for the experiment itself must NOT be evaluated. + """ + experiment_rule_id = '111127' + opt = self._make_opt([ + _holdout('lh1', 'local_for_exp', included_rules=[experiment_rule_id], traffic=_FULL_TRAFFIC) + ]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_experiment') + self.assertIsNotNone(feature_flag) + + ds = self._decision_svc() + + with mock.patch.object(ds, 'get_variation', wraps=ds.get_variation) as mock_get_var: + user_ctx = opt.create_user_context('user_in_local_holdout', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + decision = result['decision'] + # User must be caught by local holdout + self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT) + # Regular experiment evaluation (get_variation) must not have run + mock_get_var.assert_not_called() + + def test_local_holdout_hit_returns_holdout_decision_for_delivery_rule(self): + """User bucketed into local holdout for delivery/rollout rule returns holdout decision. + + Rule '211147' is the everyone-else rollout rule for 'test_feature_in_rollout'. + The local holdout targets this delivery rule with full traffic. + """ + delivery_rule_id = '211147' + opt = self._make_opt([ + _holdout('lh2', 'local_for_delivery', included_rules=[delivery_rule_id], traffic=_FULL_TRAFFIC) + ]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_rollout') + self.assertIsNotNone(feature_flag) + + ds = self._decision_svc() + user_ctx = opt.create_user_context('user_delivery_holdout', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + self.assertIsNotNone(result) + decision = result['decision'] + self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT) + + # ------------------------------------------------------------------ + # Branch 3: Local holdout miss — user falls through to regular rule evaluation + # ------------------------------------------------------------------ + + def test_local_holdout_miss_falls_through_to_regular_rule_evaluation(self): + """User not bucketed into local holdout falls through to normal experiment evaluation. + + Zero-traffic local holdout — nobody bucketed in — user proceeds to experiment. + """ + experiment_rule_id = '111127' + opt = self._make_opt([ + _holdout('lh_miss', 'local_no_traffic', included_rules=[experiment_rule_id], traffic=_NO_TRAFFIC) + ]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_experiment') + self.assertIsNotNone(feature_flag) + + ds = self._decision_svc() + user_ctx = opt.create_user_context('testUserId', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + self.assertIsNotNone(result) + decision = result['decision'] + # Source must NOT be HOLDOUT — missed the local holdout + self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT) + + # ------------------------------------------------------------------ + # Branch 4: Rule specificity — local holdout for rule X does NOT affect rule Y + # ------------------------------------------------------------------ + + def test_local_holdout_for_rule_x_does_not_affect_rule_y(self): + """Local holdout targeting experiment rule '111127' must not apply to rollout rule '211147'. + + get_holdouts_for_rule('211147') should return empty — the holdout only targets '111127'. + """ + experiment_rule_id = '111127' + delivery_rule_id = '211147' + + opt = self._make_opt([ + _holdout('lh_x', 'local_for_exp_only', included_rules=[experiment_rule_id], traffic=_FULL_TRAFFIC) + ]) + config = opt.config_manager.get_config() + + # Verify rule map specificity via project config API + holdouts_for_exp = config.get_holdouts_for_rule(experiment_rule_id) + holdouts_for_delivery = config.get_holdouts_for_rule(delivery_rule_id) + + self.assertEqual(len(holdouts_for_exp), 1) + self.assertEqual(holdouts_for_exp[0].id, 'lh_x') + self.assertEqual(holdouts_for_delivery, []) + + def test_two_rules_each_with_own_local_holdout_are_independent(self): + """Each rule's local holdout evaluation is independent of other rules.""" + exp_rule_id = '111127' + delivery_rule_id = '211147' + + opt = self._make_opt([ + _holdout('lh_exp', 'for_exp', included_rules=[exp_rule_id], traffic=_FULL_TRAFFIC), + _holdout('lh_del', 'for_del', included_rules=[delivery_rule_id], traffic=_FULL_TRAFFIC), + ]) + config = opt.config_manager.get_config() + + exp_holdouts = config.get_holdouts_for_rule(exp_rule_id) + del_holdouts = config.get_holdouts_for_rule(delivery_rule_id) + + exp_ids = {h.id for h in exp_holdouts} + del_ids = {h.id for h in del_holdouts} + + self.assertIn('lh_exp', exp_ids) + self.assertNotIn('lh_del', exp_ids) + + self.assertIn('lh_del', del_ids) + self.assertNotIn('lh_exp', del_ids) + + # ------------------------------------------------------------------ + # Branch 5: Experiment vs delivery rules — local holdout applies to both + # ------------------------------------------------------------------ + + def test_local_holdout_applies_to_experiment_rule(self): + """Local holdout check applies to experiment rules in get_decision_for_flag.""" + experiment_rule_id = '111127' + opt = self._make_opt([ + _holdout('lh_exp', 'for_experiment', included_rules=[experiment_rule_id], traffic=_FULL_TRAFFIC) + ]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_experiment') + + ds = self._decision_svc() + user_ctx = opt.create_user_context('user_exp_holdout', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + self.assertEqual(result['decision'].source, enums.DecisionSources.HOLDOUT) + + def test_local_holdout_applies_to_rollout_delivery_rule(self): + """Local holdout check applies to rollout/delivery rules in get_decision_for_flag.""" + delivery_rule_id = '211147' + opt = self._make_opt([ + _holdout('lh_del', 'for_delivery', included_rules=[delivery_rule_id], traffic=_FULL_TRAFFIC) + ]) + config = opt.config_manager.get_config() + # test_feature_in_rollout only has rollout rules, no experiment rules + feature_flag = config.get_feature_from_key('test_feature_in_rollout') + self.assertIsNotNone(feature_flag) + + ds = self._decision_svc() + user_ctx = opt.create_user_context('user_delivery_holdout', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + self.assertIsNotNone(result) + decision = result['decision'] + self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT) + + # ------------------------------------------------------------------ + # Precedence: Global → Forced → Local → Regular rule + # ------------------------------------------------------------------ + + def test_global_holdout_takes_precedence_over_local_holdout(self): + """Global holdout evaluated first — user caught before local holdout check.""" + experiment_rule_id = '111127' + opt = self._make_opt([ + _holdout('gh1', 'global_full', traffic=_FULL_TRAFFIC), # global, full traffic + _holdout('lh1', 'local_full', included_rules=[experiment_rule_id], traffic=_FULL_TRAFFIC), + ]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_experiment') + + ds = self._decision_svc() + + # Mock local holdout evaluation to verify it is never reached + with mock.patch.object(config, 'get_holdouts_for_rule', wraps=config.get_holdouts_for_rule) as mock_local: + user_ctx = opt.create_user_context('user_global_first', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + self.assertEqual(result['decision'].source, enums.DecisionSources.HOLDOUT) + # get_holdouts_for_rule should NOT be called — global holdout short-circuited + mock_local.assert_not_called() + + def test_forced_decision_beats_100_percent_local_holdout(self): + """Forced decision MUST win over a 100% traffic local holdout targeting the same rule. + + MANDATORY ENFORCEMENT TEST (cross-sdk guideline): + Setup: User has a forced decision set for rule 'test_experiment'. A local holdout + targets rule '111127' (same rule) with 100% traffic allocation. + Assert: The forced decision variation is returned — NOT the holdout variation. + If this test fails, the per-rule ordering (forced → local holdout → regular) is wrong. + """ + from optimizely.optimizely_user_context import OptimizelyUserContext + + experiment_rule_id = '111127' # id of 'test_experiment' + forced_variation_key = 'control' # a valid variation key in test_experiment + + opt = self._make_opt([ + _holdout('lh_full', 'local_full_traffic', included_rules=[experiment_rule_id], traffic=_FULL_TRAFFIC) + ]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_experiment') + self.assertIsNotNone(feature_flag) + + ds = self._decision_svc() + user_ctx = opt.create_user_context('forced_user', {}) + + # Set forced decision for 'test_feature_in_experiment' with rule 'test_experiment' + decision_context = OptimizelyUserContext.OptimizelyDecisionContext( + flag_key='test_feature_in_experiment', + rule_key='test_experiment' + ) + forced_decision = OptimizelyUserContext.OptimizelyForcedDecision( + variation_key=forced_variation_key + ) + user_ctx.set_forced_decision(decision_context, forced_decision) + + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + decision = result['decision'] + self.assertIsNotNone(decision) + # The forced decision variation must be returned, NOT a holdout variation + self.assertEqual(decision.variation.key, forced_variation_key) + self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT) + + def test_no_holdouts_at_all_falls_through_to_experiment(self): + """When there are no holdouts, decision falls through to experiment evaluation.""" + opt = self._make_opt([]) + config = opt.config_manager.get_config() + feature_flag = config.get_feature_from_key('test_feature_in_experiment') + + ds = self._decision_svc() + user_ctx = opt.create_user_context('testUserId', {}) + result = ds.get_decision_for_flag(feature_flag, user_ctx, config) + + self.assertIsNotNone(result) + decision = result['decision'] + self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT) diff --git a/tests/test_holdout_config.py b/tests/test_holdout_config.py new file mode 100644 index 00000000..3d8cd08c --- /dev/null +++ b/tests/test_holdout_config.py @@ -0,0 +1,312 @@ +# Copyright 2026 Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Level 1 tests for holdout config parsing and data model (FSSDK-12369). + +Tests cover: +- isGlobal classification (includedRules == None vs list) +- get_global_holdouts() returns only global holdouts +- get_holdouts_for_rule() returns local holdouts for a specific rule +- Backward compatibility with old datafiles (no includedRules field) +- Edge cases: empty array vs None, unknown rule IDs, cross-flag targeting +""" + +import json +import unittest + +from optimizely import entities +from optimizely import optimizely as optimizely_module +from tests import base + + +HOLDOUT_VARIATION = [ + {'id': 'holdout_var_1', 'key': 'holdout_control', 'variables': []} +] + +FULL_TRAFFIC = [{'entityId': 'holdout_var_1', 'endOfRange': 10000}] +NO_TRAFFIC = [{'entityId': 'holdout_var_1', 'endOfRange': 0}] + + +def _make_holdout( + holdout_id: str, + key: str, + included_rules: object = 'MISSING', + status: str = 'Running', +) -> dict: + """Build a holdout datafile dict. Pass included_rules=None for global, + a list for local, or leave as 'MISSING' to omit the field entirely.""" + h: dict = { + 'id': holdout_id, + 'key': key, + 'status': status, + 'audienceIds': [], + 'variations': HOLDOUT_VARIATION, + 'trafficAllocation': FULL_TRAFFIC, + } + if included_rules != 'MISSING': + h['includedRules'] = included_rules + return h + + +def _build_config(holdouts: list, base_test: 'base.BaseTest') -> 'optimizely_module.Optimizely': + """Create an Optimizely instance with the given holdouts list.""" + config_dict = base_test.config_dict_with_features.copy() + config_dict['holdouts'] = holdouts + return optimizely_module.Optimizely(json.dumps(config_dict)) + + +class HoldoutEntityIsGlobalTest(unittest.TestCase): + """Tests for the Holdout.is_global computed property.""" + + def test_is_global_returns_true_when_included_rules_is_none(self): + """Holdout with includedRules=None should be classified as global.""" + holdout = entities.Holdout( + id='h1', key='global_holdout', status='Running', + variations=HOLDOUT_VARIATION, trafficAllocation=FULL_TRAFFIC, + audienceIds=[], includedRules=None + ) + self.assertTrue(holdout.is_global) + + def test_is_global_returns_true_when_included_rules_not_provided(self): + """Holdout with no includedRules kwarg (default None) is global.""" + holdout = entities.Holdout( + id='h1', key='global_holdout', status='Running', + variations=HOLDOUT_VARIATION, trafficAllocation=FULL_TRAFFIC, + audienceIds=[] + ) + self.assertTrue(holdout.is_global) + + def test_is_global_returns_false_when_included_rules_is_empty_list(self): + """Empty list [] is a local holdout targeting no rules — NOT global.""" + holdout = entities.Holdout( + id='h1', key='local_holdout_no_rules', status='Running', + variations=HOLDOUT_VARIATION, trafficAllocation=FULL_TRAFFIC, + audienceIds=[], includedRules=[] + ) + self.assertFalse(holdout.is_global) + + def test_is_global_returns_false_when_included_rules_has_rule_ids(self): + """Holdout with rule IDs is local — is_global should be False.""" + holdout = entities.Holdout( + id='h1', key='local_holdout', status='Running', + variations=HOLDOUT_VARIATION, trafficAllocation=FULL_TRAFFIC, + audienceIds=[], includedRules=['rule_1', 'rule_2'] + ) + self.assertFalse(holdout.is_global) + + def test_included_rules_stored_correctly(self): + """included_rules attribute should match the passed includedRules value.""" + holdout_global = entities.Holdout( + id='h1', key='g', status='Running', + variations=HOLDOUT_VARIATION, trafficAllocation=FULL_TRAFFIC, + audienceIds=[], includedRules=None + ) + holdout_local = entities.Holdout( + id='h2', key='l', status='Running', + variations=HOLDOUT_VARIATION, trafficAllocation=FULL_TRAFFIC, + audienceIds=[], includedRules=['rule_a', 'rule_b'] + ) + self.assertIsNone(holdout_global.included_rules) + self.assertEqual(holdout_local.included_rules, ['rule_a', 'rule_b']) + + +class HoldoutConfigGetGlobalHoldoutsTest(unittest.TestCase): + """Tests for ProjectConfig.get_global_holdouts().""" + + def setUp(self): + base.BaseTest.setUp(self) + + def tearDown(self): + if hasattr(self, 'opt_obj'): + self.opt_obj.close() + + def test_get_global_holdouts_returns_all_global_holdouts(self): + """All holdouts without includedRules should be returned.""" + self.opt_obj = _build_config([ + _make_holdout('h1', 'global_1'), # field absent → global + _make_holdout('h2', 'global_2', None), # explicitly None → global + _make_holdout('h3', 'local_1', ['rule_x']), # local + ], self) + config = self.opt_obj.config_manager.get_config() + global_holdouts = config.get_global_holdouts() + global_ids = {h.id for h in global_holdouts} + self.assertIn('h1', global_ids) + self.assertIn('h2', global_ids) + self.assertNotIn('h3', global_ids) + + def test_get_global_holdouts_returns_empty_when_no_holdouts(self): + """No holdouts in datafile → empty list.""" + self.opt_obj = _build_config([], self) + config = self.opt_obj.config_manager.get_config() + self.assertEqual(config.get_global_holdouts(), []) + + def test_get_global_holdouts_excludes_non_running_holdouts(self): + """Draft holdouts are not activated and should not appear.""" + self.opt_obj = _build_config([ + _make_holdout('h_running', 'running_global', status='Running'), + _make_holdout('h_draft', 'draft_global', status='Draft'), + ], self) + config = self.opt_obj.config_manager.get_config() + global_holdouts = config.get_global_holdouts() + ids = {h.id for h in global_holdouts} + self.assertIn('h_running', ids) + self.assertNotIn('h_draft', ids) + + def test_get_global_holdouts_returns_empty_when_all_local(self): + """If all holdouts are local, global list is empty.""" + self.opt_obj = _build_config([ + _make_holdout('h1', 'local_a', ['rule_1']), + _make_holdout('h2', 'local_b', ['rule_2']), + ], self) + config = self.opt_obj.config_manager.get_config() + self.assertEqual(config.get_global_holdouts(), []) + + +class HoldoutConfigGetHoldoutsForRuleTest(unittest.TestCase): + """Tests for ProjectConfig.get_holdouts_for_rule().""" + + def setUp(self): + base.BaseTest.setUp(self) + + def tearDown(self): + if hasattr(self, 'opt_obj'): + self.opt_obj.close() + + def test_get_holdouts_for_rule_returns_matching_local_holdouts(self): + """Holdout targeting rule_x should be returned for rule_x.""" + self.opt_obj = _build_config([ + _make_holdout('h1', 'local_h', ['rule_x', 'rule_y']), + ], self) + config = self.opt_obj.config_manager.get_config() + holdouts = config.get_holdouts_for_rule('rule_x') + self.assertEqual(len(holdouts), 1) + self.assertEqual(holdouts[0].id, 'h1') + + def test_get_holdouts_for_rule_returns_empty_for_unknown_rule(self): + """Rule ID not found in any holdout's includedRules returns empty list.""" + self.opt_obj = _build_config([ + _make_holdout('h1', 'local_h', ['rule_a']), + ], self) + config = self.opt_obj.config_manager.get_config() + # Silently skip unknown rule IDs + holdouts = config.get_holdouts_for_rule('nonexistent_rule') + self.assertEqual(holdouts, []) + + def test_get_holdouts_for_rule_returns_empty_when_no_holdouts(self): + """No holdouts defined → always returns empty list.""" + self.opt_obj = _build_config([], self) + config = self.opt_obj.config_manager.get_config() + self.assertEqual(config.get_holdouts_for_rule('any_rule'), []) + + def test_get_holdouts_for_rule_multiple_holdouts_for_same_rule(self): + """Multiple holdouts can target the same rule.""" + self.opt_obj = _build_config([ + _make_holdout('h1', 'local_h1', ['rule_x']), + _make_holdout('h2', 'local_h2', ['rule_x', 'rule_y']), + ], self) + config = self.opt_obj.config_manager.get_config() + holdouts = config.get_holdouts_for_rule('rule_x') + ids = {h.id for h in holdouts} + self.assertIn('h1', ids) + self.assertIn('h2', ids) + + def test_get_holdouts_for_rule_does_not_return_global_holdouts(self): + """Global holdouts should not appear in get_holdouts_for_rule results.""" + self.opt_obj = _build_config([ + _make_holdout('global', 'global_holdout'), # field absent → global + _make_holdout('local', 'local_holdout', ['rule_x']), + ], self) + config = self.opt_obj.config_manager.get_config() + holdouts = config.get_holdouts_for_rule('rule_x') + ids = {h.id for h in holdouts} + self.assertNotIn('global', ids) + self.assertIn('local', ids) + + def test_get_holdouts_for_rule_rule_specificity(self): + """A holdout targeting rule_x must not appear for rule_y.""" + self.opt_obj = _build_config([ + _make_holdout('h1', 'local_for_x', ['rule_x']), + ], self) + config = self.opt_obj.config_manager.get_config() + self.assertEqual(config.get_holdouts_for_rule('rule_y'), []) + + def test_get_holdouts_for_rule_cross_flag_targeting(self): + """One holdout can target rules from multiple different flags.""" + self.opt_obj = _build_config([ + _make_holdout('h1', 'cross_flag_holdout', ['rule_flag_a', 'rule_flag_b']), + ], self) + config = self.opt_obj.config_manager.get_config() + for rule_id in ['rule_flag_a', 'rule_flag_b']: + holdouts = config.get_holdouts_for_rule(rule_id) + self.assertEqual(len(holdouts), 1) + self.assertEqual(holdouts[0].id, 'h1') + + +class HoldoutBackwardCompatibilityTest(unittest.TestCase): + """Tests for backward compatibility with old datafiles (no includedRules field).""" + + def setUp(self): + base.BaseTest.setUp(self) + + def tearDown(self): + if hasattr(self, 'opt_obj'): + self.opt_obj.close() + + def test_old_datafile_without_included_rules_treated_as_global(self): + """Datafiles without includedRules field must default to global holdout.""" + # Simulate old datafile — no 'includedRules' key at all + old_holdout = { + 'id': 'old_h', + 'key': 'legacy_holdout', + 'status': 'Running', + 'audienceIds': [], + 'variations': HOLDOUT_VARIATION, + 'trafficAllocation': FULL_TRAFFIC, + } + self.opt_obj = _build_config([old_holdout], self) + config = self.opt_obj.config_manager.get_config() + + global_holdouts = config.get_global_holdouts() + self.assertEqual(len(global_holdouts), 1) + self.assertEqual(global_holdouts[0].id, 'old_h') + self.assertTrue(global_holdouts[0].is_global) + + def test_old_datafile_holdout_does_not_appear_in_rule_map(self): + """Legacy holdouts with no includedRules field must not pollute rule map.""" + old_holdout = { + 'id': 'old_h', + 'key': 'legacy_holdout', + 'status': 'Running', + 'audienceIds': [], + 'variations': HOLDOUT_VARIATION, + 'trafficAllocation': FULL_TRAFFIC, + } + self.opt_obj = _build_config([old_holdout], self) + config = self.opt_obj.config_manager.get_config() + self.assertEqual(config.get_holdouts_for_rule('any_rule'), []) + + def test_empty_included_rules_is_local_not_global(self): + """[] (empty array) is a local holdout targeting no rules — not the same as None.""" + holdout_empty = _make_holdout('h_empty', 'empty_local', included_rules=[]) + holdout_none = _make_holdout('h_none', 'global_none', included_rules=None) + + self.opt_obj = _build_config([holdout_empty, holdout_none], self) + config = self.opt_obj.config_manager.get_config() + + global_holdouts = config.get_global_holdouts() + global_ids = {h.id for h in global_holdouts} + + # None → global; [] → local (does not appear in global list) + self.assertIn('h_none', global_ids) + self.assertNotIn('h_empty', global_ids)