[AI-FSSDK] [FSSDK-12735] Add holdout exclusion logic for Targeted Delivery rules

optimizely/decision_service.py

@@ -610,9 +610,11 @@ 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:
+                if holdout.exclude_targeted_deliveries:
+                    continue
+
                 local_holdout_decision = self.get_variation_for_holdout(
                     holdout, user_context, project_config
                 )
@@ -751,14 +753,15 @@ def get_decision_for_flag(
         reasons = decide_reasons.copy() if decide_reasons else []
         user_id = user_context.user_id
 
+        global_holdout_result: DecisionResult | None = None
+
         # 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'])
 
             decision = holdout_decision['decision']
-            # Check if user was bucketed into holdout (has a variation)
             if decision.variation is None:
                 continue
 
@@ -768,18 +771,32 @@ def get_decision_for_flag(
             )
             self.logger.info(message)
             reasons.append(message)
-            return {
-                'decision': holdout_decision['decision'],
-                'error': False,
-                'reasons': reasons
-            }
 
-        # If no global holdout decision, check experiments then rollouts
+            if not holdout.exclude_targeted_deliveries:
+                return {
+                    'decision': holdout_decision['decision'],
+                    'error': False,
+                    'reasons': reasons
+                }
+
+            message = (
+                f"Holdout '{holdout.key}' excludes targeted deliveries. "
+                f"Targeted delivery rules will be evaluated."
+            )
+            self.logger.info(message)
+            reasons.append(message)
+            global_holdout_result = holdout_decision
+            break
+
+        # Check experiments then rollouts
         if feature_flag.experimentIds:
             for experiment_id in feature_flag.experimentIds:
                 experiment = project_config.get_experiment_from_id(experiment_id)
 
                 if experiment:
+                    if global_holdout_result is not None and experiment.type != enums.ExperimentTypes.td:
+                        continue
+
                     # Check for forced decision
                     optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(
                         feature_flag.key, experiment.key)
@@ -796,9 +813,11 @@ 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:
+                        if holdout.exclude_targeted_deliveries and experiment.type == enums.ExperimentTypes.td:
+                            continue
+
                         local_holdout_decision = self.get_variation_for_holdout(
                             holdout, user_context, project_config
                         )
@@ -863,6 +882,13 @@ def get_decision_for_flag(
         else:
             self.logger.debug(f'User "{user_id}" not bucketed into any rollout for feature "{feature_flag.key}".')
 
+        if global_holdout_result is not None and not has_variation:
+            return {
+                'decision': global_holdout_result['decision'],
+                'error': False,
+                'reasons': reasons
+            }
+
         return {
             'decision': rollout_decision,
             'error': False,

optimizely/entities.py

@@ -224,6 +224,7 @@ def __init__(
         audienceIds: list[str],
         audienceConditions: Optional[Sequence[str | list[str]]] = None,
         includedRules: Optional[list[str]] = None,
+        excludeTargetedDeliveries: bool = False,
         **kwargs: Any
     ):
         self.id = id
@@ -236,6 +237,7 @@ def __init__(
         # Per-rule targeting for local holdouts. Scope comes from the datafile
         # section, not this field; ProjectConfig strips it on 'holdouts' entries.
         self.included_rules: Optional[list[str]] = includedRules
+        self.exclude_targeted_deliveries: bool = excludeTargetedDeliveries
 
     def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]:
         """Returns audienceConditions if present, otherwise audienceIds.

tests/test_decision_service_holdout.py

@@ -1708,3 +1708,285 @@ def test_no_holdouts_at_all_falls_through_to_experiment(self):
         self.assertIsNotNone(result)
         decision = result['decision']
         self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT)
+
+
+# ---------------------------------------------------------------------------
+# Exclude Targeted Deliveries Tests
+# ---------------------------------------------------------------------------
+
+def _holdout_with_etd(holdout_id, key, exclude_targeted_deliveries=False,
+                      included_rules=None, traffic=None, status='Running'):
+    """Build a holdout dict with excludeTargetedDeliveries field."""
+    h = {
+        'id': holdout_id,
+        'key': key,
+        'status': status,
+        'audienceIds': [],
+        'variations': _HOLDOUT_VARIATION,
+        'trafficAllocation': traffic or _FULL_TRAFFIC,
+        'excludeTargetedDeliveries': exclude_targeted_deliveries,
+    }
+    if included_rules is not None:
+        h['includedRules'] = included_rules
+    return h
+
+
+class ExcludeTargetedDeliveriesTest(base.BaseTest):
+    """Tests for exclude_targeted_deliveries behavior on holdouts.
+
+    When a holdout has exclude_targeted_deliveries=True:
+    - TD (targeted delivery) experiments should bypass the holdout
+    - A/B and other experiment types should still be blocked by the holdout
+    - Delivery (rollout) rules should bypass local holdouts with this flag
+    """
+
+    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_with_td(self, holdouts, local_holdouts=None, experiment_type=None):
+        """Build Optimizely instance, optionally setting type on experiment '111127'."""
+        cfg = self.config_dict_with_features.copy()
+
+        if experiment_type is not None:
+            experiments = []
+            for exp in cfg['experiments']:
+                exp_copy = exp.copy()
+                if exp_copy['id'] == '111127':
+                    exp_copy['type'] = experiment_type
+                experiments.append(exp_copy)
+            cfg['experiments'] = experiments
+
+        if local_holdouts is None:
+            globals_list: list = []
+            locals_list: list = []
+            for h in holdouts:
+                if h.get('includedRules') is not None:
+                    locals_list.append(h)
+                else:
+                    globals_list.append(h)
+            cfg['holdouts'] = globals_list
+            if locals_list:
+                cfg['localHoldouts'] = locals_list
+        else:
+            cfg['holdouts'] = holdouts
+            cfg['localHoldouts'] = local_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,
+        )
+
+    # ------------------------------------------------------------------
+    # Test 1: Global holdout with exclude_targeted_deliveries=False (default)
+    # ------------------------------------------------------------------
+
+    def test_global_holdout_exclude_td_false_blocks_all_rules(self):
+        """Global holdout with exclude_targeted_deliveries=False blocks all rules as before."""
+        opt = self._make_opt_with_td(
+            [_holdout_with_etd('gh1', 'global_no_exclude', exclude_targeted_deliveries=False)],
+            experiment_type='td',
+        )
+        config = opt.config_manager.get_config()
+        feature_flag = config.get_feature_from_key('test_feature_in_experiment')
+
+        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_blocked', {})
+            result = ds.get_decision_for_flag(feature_flag, user_ctx, config)
+
+        decision = result['decision']
+        self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT)
+        mock_get_var.assert_not_called()
+
+    # ------------------------------------------------------------------
+    # Test 2: Global holdout with exclude_targeted_deliveries=True, TD experiment
+    # ------------------------------------------------------------------
+
+    def test_global_holdout_exclude_td_true_allows_td_experiment(self):
+        """Global holdout with exclude_targeted_deliveries=True allows TD rules through."""
+        opt = self._make_opt_with_td(
+            [_holdout_with_etd('gh1', 'global_exclude_td', exclude_targeted_deliveries=True)],
+            experiment_type='td',
+        )
+        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)
+
+        decision = result['decision']
+        # TD experiment should be evaluated, not blocked by holdout
+        self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT)
+
+    # ------------------------------------------------------------------
+    # Test 3: Global holdout with exclude_targeted_deliveries=True, A/B experiment
+    # ------------------------------------------------------------------
+
+    def test_global_holdout_exclude_td_true_blocks_ab_experiment(self):
+        """Global holdout with exclude_targeted_deliveries=True still blocks A/B rules."""
+        opt = self._make_opt_with_td(
+            [_holdout_with_etd('gh1', 'global_exclude_td', exclude_targeted_deliveries=True)],
+            experiment_type='ab',
+        )
+        config = opt.config_manager.get_config()
+        feature_flag = config.get_feature_from_key('test_feature_in_experiment')
+
+        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_blocked_ab', {})
+            result = ds.get_decision_for_flag(feature_flag, user_ctx, config)
+
+        decision = result['decision']
+        # A/B experiment should be skipped; user stays in holdout
+        self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT)
+        mock_get_var.assert_not_called()
+
+    # ------------------------------------------------------------------
+    # Test 4: Global holdout with exclude_targeted_deliveries=True, no TD matches
+    # ------------------------------------------------------------------
+
+    def test_global_holdout_exclude_td_true_no_td_falls_back_to_holdout(self):
+        """When exclude_targeted_deliveries=True but no TD experiments match,
+        falls back to holdout decision."""
+        # No type set on experiment (defaults to None, not 'td')
+        opt = self._make_opt_with_td(
+            [_holdout_with_etd('gh1', 'global_exclude_td', exclude_targeted_deliveries=True)],
+        )
+        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_no_td', {})
+        result = ds.get_decision_for_flag(feature_flag, user_ctx, config)
+
+        decision = result['decision']
+        # No TD experiment available, so holdout decision should be returned
+        self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT)
+
+    # ------------------------------------------------------------------
+    # Test 5: Local holdout on delivery rule with exclude_targeted_deliveries=True
+    # ------------------------------------------------------------------
+
+    def test_local_holdout_delivery_rule_exclude_td_true_skips_holdout(self):
+        """Local holdout on delivery rule with exclude_targeted_deliveries=True
+        skips the holdout and evaluates the delivery rule."""
+        delivery_rule_id = '211147'
+        opt = self._make_opt_with_td(
+            [_holdout_with_etd(
+                'lh1', 'local_delivery_exclude',
+                exclude_targeted_deliveries=True,
+                included_rules=[delivery_rule_id],
+            )],
+        )
+        config = opt.config_manager.get_config()
+        feature_flag = config.get_feature_from_key('test_feature_in_rollout')
+
+        ds = self._decision_svc()
+        user_ctx = opt.create_user_context('user_delivery_exclude', {})
+        result = ds.get_decision_for_flag(feature_flag, user_ctx, config)
+
+        decision = result['decision']
+        # Delivery rules are targeted deliveries, so holdout should be skipped
+        self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT)
+
+    # ------------------------------------------------------------------
+    # Test 6: Local holdout on experiment rule (TD type) with exclude_targeted_deliveries=True
+    # ------------------------------------------------------------------
+
+    def test_local_holdout_td_experiment_exclude_td_true_skips_holdout(self):
+        """Local holdout on TD experiment with exclude_targeted_deliveries=True
+        skips the holdout."""
+        experiment_rule_id = '111127'
+        opt = self._make_opt_with_td(
+            [_holdout_with_etd(
+                'lh1', 'local_td_exclude',
+                exclude_targeted_deliveries=True,
+                included_rules=[experiment_rule_id],
+            )],
+            experiment_type='td',
+        )
+        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)
+
+        decision = result['decision']
+        # TD experiment should bypass the local holdout
+        self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT)
+
+    # ------------------------------------------------------------------
+    # Test 7: Local holdout on experiment rule (A/B type) with exclude_targeted_deliveries=True
+    # ------------------------------------------------------------------
+
+    def test_local_holdout_ab_experiment_exclude_td_true_still_applies(self):
+        """Local holdout on A/B experiment with exclude_targeted_deliveries=True
+        still applies the holdout (A/B is not a targeted delivery)."""
+        experiment_rule_id = '111127'
+        opt = self._make_opt_with_td(
+            [_holdout_with_etd(
+                'lh1', 'local_ab_exclude',
+                exclude_targeted_deliveries=True,
+                included_rules=[experiment_rule_id],
+            )],
+            experiment_type='ab',
+        )
+        config = opt.config_manager.get_config()
+        feature_flag = config.get_feature_from_key('test_feature_in_experiment')
+
+        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_ab_holdout', {})
+            result = ds.get_decision_for_flag(feature_flag, user_ctx, config)
+
+        decision = result['decision']
+        # A/B experiment should still be blocked by local holdout
+        self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT)
+        mock_get_var.assert_not_called()
+
+    # ------------------------------------------------------------------
+    # Test 8: Missing field (backward compatibility)
+    # ------------------------------------------------------------------
+
+    def test_missing_exclude_td_field_defaults_to_false(self):
+        """When excludeTargetedDeliveries is not in the datafile, defaults to False
+        and holdout works as before (blocks all rules)."""
+        # Use _holdout helper which does NOT set excludeTargetedDeliveries
+        opt = self._make_opt_with_td(
+            [_holdout('gh1', 'legacy_holdout', traffic=_FULL_TRAFFIC)],
+            experiment_type='td',
+        )
+        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_legacy', {})
+        result = ds.get_decision_for_flag(feature_flag, user_ctx, config)
+
+        decision = result['decision']
+        # Without the field, holdout should block all rules including TD
+        self.assertEqual(decision.source, enums.DecisionSources.HOLDOUT)
+
+        # Verify entity defaults
+        holdout = config.holdouts[0] if config.holdouts else None
+        self.assertIsNotNone(holdout)
+        self.assertFalse(holdout.exclude_targeted_deliveries)