Revert "[FSSDK-12369] Fix local holdout ordering: check after forced decision, not before"

This reverts commit 59256c1.

Author: Mat001

core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java

@@ -398,6 +398,20 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
             for (String experimentId : featureFlag.getExperimentIds()) {
                 Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
 
+                // Check local holdouts targeting this experiment rule before regular evaluation (FSSDK-12369)
+                if (experiment != null) {
+                    List<Holdout> localHoldouts = projectConfig.getHoldoutConfig().getHoldoutsForRule(experiment.getId());
+                    for (Holdout holdout : localHoldouts) {
+                        DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
+                        reasons.merge(holdoutDecision.getReasons());
+                        if (holdoutDecision.getResult() != null) {
+                            return new DecisionResponse<>(
+                                new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT),
+                                reasons);
+                        }
+                    }
+                }
+
                 DecisionResponse<Variation> decisionVariation =
                     getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker, decisionPath);
                 reasons.merge(decisionVariation.getReasons());
@@ -469,6 +483,18 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu
         while (index < rolloutRulesLength) {
             Experiment rule = rollout.getExperiments().get(index);
 
+            // Check local holdouts targeting this delivery rule before regular evaluation (FSSDK-12369)
+            List<Holdout> localHoldoutsForRule = projectConfig.getHoldoutConfig().getHoldoutsForRule(rule.getId());
+            boolean localHoldoutHit = false;
+            for (Holdout holdout : localHoldoutsForRule) {
+                DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
+                reasons.merge(holdoutDecision.getReasons());
+                if (holdoutDecision.getResult() != null) {
+                    FeatureDecision holdoutFeatureDecision = new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT);
+                    return new DecisionResponse(holdoutFeatureDecision, reasons);
+                }
+            }
+
             DecisionResponse<AbstractMap.SimpleEntry> decisionVariationResponse = getVariationFromDeliveryRule(
                 projectConfig,
                 featureFlag.getKey(),
@@ -846,16 +872,6 @@ private DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proj
             return new DecisionResponse(variation, reasons);
         }
 
-        // Check local holdouts targeting this specific experiment rule (FSSDK-12369)
-        List<Holdout> localHoldouts = projectConfig.getHoldoutConfig().getHoldoutsForRule(rule.getId());
-        for (Holdout holdout : localHoldouts) {
-            DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
-            reasons.merge(holdoutDecision.getReasons());
-            if (holdoutDecision.getResult() != null) {
-                return new DecisionResponse<>(holdoutDecision.getResult(), reasons);
-            }
-        }
-
         //regular decision
         DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null, decisionPath);
         reasons.merge(decisionResponse.getReasons());
@@ -906,17 +922,6 @@ DecisionResponse<AbstractMap.SimpleEntry> getVariationFromDeliveryRule(@Nonnull
             return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons);
         }
 
-        // Check local holdouts targeting this specific delivery rule (FSSDK-12369)
-        List<Holdout> localHoldouts = projectConfig.getHoldoutConfig().getHoldoutsForRule(rule.getId());
-        for (Holdout holdout : localHoldouts) {
-            DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
-            reasons.merge(holdoutDecision.getReasons());
-            if (holdoutDecision.getResult() != null) {
-                variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(holdoutDecision.getResult(), false);
-                return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons);
-            }
-        }
-
         // Handle a regular decision
         String bucketingId = getBucketingId(user.getUserId(), user.getAttributes());
         Boolean everyoneElse = (ruleIndex == rules.size() - 1);

core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java

@@ -86,9 +86,7 @@
 import static com.optimizely.ab.config.ValidProjectConfigV4.ROLLOUT_2;
 import static com.optimizely.ab.config.ValidProjectConfigV4.ROLLOUT_3_EVERYONE_ELSE_RULE;
 import static com.optimizely.ab.config.ValidProjectConfigV4.ROLLOUT_3_EVERYONE_ELSE_RULE_ENABLED_VARIATION;
-import static com.optimizely.ab.config.ValidProjectConfigV4.EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY;
 import static com.optimizely.ab.config.ValidProjectConfigV4.VARIATION_HOLDOUT_VARIATION_OFF;
-import static com.optimizely.ab.config.ValidProjectConfigV4.VARIATION_MULTIVARIATE_EXPERIMENT_GRED_KEY;
 import static com.optimizely.ab.config.ValidProjectConfigV4.generateValidProjectConfigV4_holdout;
 import static com.optimizely.ab.config.ValidProjectConfigV4.generateValidProjectConfigV4WithHoldouts;
 import com.optimizely.ab.config.Variation;
@@ -1968,54 +1966,6 @@ public void localHoldouts_appliesToDeliveryRules() {
         assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.decisionSource);
     }
 
-    /**
-     * Forced decision takes priority over a 100%-traffic local holdout.
-     * Even when a local holdout would bucket the user, a forced decision for the same rule must win.
-     * This is the mandatory ordering enforcement test — if it fails, the per-rule ordering is wrong.
-     */
-    @Test
-    public void localHoldouts_forcedDecisionTakesPriorityOverLocalHoldout() {
-        final String experimentRuleId = "3262035800"; // EXPERIMENT_MULTIVARIATE_EXPERIMENT_ID
-        final String flagKey = FEATURE_FLAG_MULTI_VARIATE_FEATURE.getKey(); // "multi_variate_feature"
-        final String ruleKey = EXPERIMENT_MULTIVARIATE_EXPERIMENT_KEY; // "multivariate_experiment"
-
-        // 100% traffic local holdout targeting the same experiment rule
-        Holdout localHoldout = new Holdout(
-            "local_ho_forced_test",
-            "local_holdout_forced_decision_test",
-            Holdout.HoldoutStatus.RUNNING.toString(),
-            Collections.emptyList(),
-            null,
-            DatafileProjectConfigTestUtils.createListOfObjects(VARIATION_HOLDOUT_VARIATION_OFF),
-            DatafileProjectConfigTestUtils.createListOfObjects(
-                new TrafficAllocation("$opt_dummy_variation_id", 10000) // 100% — would always bucket
-            ),
-            Collections.singletonList(experimentRuleId)
-        );
-
-        ProjectConfig config = buildHoldoutProjectConfig(Collections.singletonList(localHoldout));
-        Bucketer bucketer = new Bucketer();
-        DecisionService svc = new DecisionService(bucketer, mockErrorHandler, null, mockCmabService);
-
-        // Set a forced decision for the same rule
-        OptimizelyUserContext userCtx = optimizely.createUserContext("anyUser", Collections.emptyMap());
-        OptimizelyDecisionContext ctx = new OptimizelyDecisionContext(flagKey, ruleKey);
-        OptimizelyForcedDecision forcedDecision = new OptimizelyForcedDecision(VARIATION_MULTIVARIATE_EXPERIMENT_GRED_KEY);
-        userCtx.setForcedDecision(ctx, forcedDecision);
-
-        FeatureDecision decision = svc.getVariationForFeature(
-            FEATURE_FLAG_MULTI_VARIATE_FEATURE,
-            userCtx,
-            config
-        ).getResult();
-
-        // Forced decision must win — NOT the holdout
-        assertNotEquals("Holdout must not win when forced decision is set",
-            FeatureDecision.DecisionSource.HOLDOUT, decision.decisionSource);
-        assertEquals("Forced variation key must be returned",
-            VARIATION_MULTIVARIATE_EXPERIMENT_GRED_KEY, decision.variation.getKey());
-    }
-
     /**
      * Builds a full DatafileProjectConfig using the same setup as generateValidProjectConfigV4_holdout()
      * but with the given holdout list substituted.  This avoids spy-based mocking and ensures that