Skip to content

Commit 56da8c2

Browse files
authored
[AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit (#640)
1 parent 13ff35a commit 56da8c2

6 files changed

Lines changed: 561 additions & 36 deletions

File tree

‎core-api/src/main/java/com/optimizely/ab/Optimizely.java‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2016-2024, Optimizely, Inc. and contributors *
2+
* Copyright 2016-2024, 2026, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -1356,17 +1356,19 @@ private OptimizelyDecision createOptimizelyDecision(
13561356
cmabUuid);
13571357
}
13581358

1359-
if (flagDecision.holdoutDecision != null && !allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) {
1360-
decisionEventDispatched = sendImpression(
1361-
projectConfig,
1362-
flagDecision.holdoutDecision.experiment,
1363-
userId,
1364-
copiedAttributes,
1365-
flagDecision.holdoutDecision.variation,
1366-
flagKey,
1367-
flagDecision.holdoutDecision.decisionSource != null ? flagDecision.holdoutDecision.decisionSource.toString() : FeatureDecision.DecisionSource.HOLDOUT.toString(),
1368-
flagDecision.holdoutDecision.variation != null && flagDecision.holdoutDecision.variation.getFeatureEnabled(),
1369-
null) || decisionEventDispatched;
1359+
if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) {
1360+
for (FeatureDecision holdoutDecision : flagDecision.holdoutDecisions) {
1361+
decisionEventDispatched = sendImpression(
1362+
projectConfig,
1363+
holdoutDecision.experiment,
1364+
userId,
1365+
copiedAttributes,
1366+
holdoutDecision.variation,
1367+
flagKey,
1368+
holdoutDecision.decisionSource != null ? holdoutDecision.decisionSource.toString() : FeatureDecision.DecisionSource.HOLDOUT.toString(),
1369+
holdoutDecision.variation != null && holdoutDecision.variation.getFeatureEnabled(),
1370+
null) || decisionEventDispatched;
1371+
}
13701372
}
13711373

13721374
DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder()

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

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -351,38 +351,44 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
351351
reasons.addInfo("Holdout \"%s\" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.", globalHoldoutDecision.experiment.getKey());
352352
}
353353

354-
if (globalHoldoutDecision == null) {
354+
List<FeatureDecision> holdoutDecisions = new ArrayList<>();
355+
if (globalHoldoutDecision != null) {
356+
holdoutDecisions.add(globalHoldoutDecision);
357+
} else {
355358
DecisionResponse<FeatureDecision> decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker, decisionPath);
356359
reasons.merge(decisionVariationResponse.getReasons());
357360

358361
FeatureDecision decision = decisionVariationResponse.getResult();
359362
boolean error = decisionVariationResponse.isError();
360363

361-
if (decision != null) {
364+
if (decision != null && (decision.variation != null || error)) {
365+
syncDeprecatedHoldoutDecision(decision);
362366
decisions.add(new DecisionResponse(decision, reasons, error, decision.cmabUuid));
363367
continue flagLoop;
364368
}
369+
if (decision != null) {
370+
holdoutDecisions.addAll(decision.holdoutDecisions);
371+
}
365372
}
366373

367374
DecisionResponse<FeatureDecision> decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig);
368375
reasons.merge(decisionFeatureResponse.getReasons());
369376
FeatureDecision decision = decisionFeatureResponse.getResult();
370377

371-
if (decision != null && decision.variation != null) {
372-
if (globalHoldoutDecision != null) {
373-
decision.setHoldoutDecision(globalHoldoutDecision);
378+
if (!holdoutDecisions.isEmpty()) {
379+
if (decision == null) {
380+
decision = new FeatureDecision(null, null, null);
374381
}
382+
decision.holdoutDecisions.addAll(0, holdoutDecisions);
383+
}
384+
syncDeprecatedHoldoutDecision(decision);
385+
386+
if (decision != null && decision.variation != null) {
375387
String message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".",
376388
user.getUserId(), featureFlag.getKey());
377389
logger.info(message);
378390
decisions.add(new DecisionResponse(decision, reasons));
379391
} else {
380-
if (globalHoldoutDecision != null) {
381-
if (decision == null) {
382-
decision = new FeatureDecision(null, null, null);
383-
}
384-
decision.setHoldoutDecision(globalHoldoutDecision);
385-
}
386392
String message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".",
387393
user.getUserId(), featureFlag.getKey());
388394
logger.info(message);
@@ -397,6 +403,13 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
397403
return decisions;
398404
}
399405

406+
@SuppressWarnings("deprecation")
407+
private static void syncDeprecatedHoldoutDecision(@Nullable FeatureDecision decision) {
408+
if (decision != null && !decision.holdoutDecisions.isEmpty()) {
409+
decision.setHoldoutDecision(decision.holdoutDecisions.get(0));
410+
}
411+
}
412+
400413
@Nonnull
401414
public DecisionResponse<FeatureDecision> getVariationForFeature(@Nonnull FeatureFlag featureFlag,
402415
@Nonnull OptimizelyUserContext user,
@@ -422,6 +435,7 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
422435
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
423436
// Cache flagKey once to avoid multiple getKey() calls (important for mock-based tests)
424437
String flagKey = featureFlag.getKey();
438+
List<FeatureDecision> holdoutDecisions = new ArrayList<>();
425439
if (!featureFlag.getExperimentIds().isEmpty()) {
426440
for (String experimentId : featureFlag.getExperimentIds()) {
427441
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
@@ -430,7 +444,12 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
430444
getVariationFromExperimentRule(projectConfig, flagKey, experiment, user, options, userProfileTracker, decisionPath);
431445
reasons.merge(decisionVariation.getReasons());
432446
FeatureDecision featureDecision = decisionVariation.getResult();
447+
if (featureDecision != null && featureDecision.decisionSource == FeatureDecision.DecisionSource.HOLDOUT) {
448+
holdoutDecisions.add(featureDecision);
449+
continue;
450+
}
433451
if (decisionVariation.isError() || (featureDecision != null && featureDecision.variation != null)) {
452+
featureDecision.holdoutDecisions.addAll(holdoutDecisions);
434453
return new DecisionResponse(featureDecision, reasons, decisionVariation.isError(), decisionVariation.getCmabUuid());
435454
}
436455
}
@@ -439,6 +458,11 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
439458
logger.info(message);
440459
}
441460

461+
if (!holdoutDecisions.isEmpty()) {
462+
FeatureDecision emptyDecision = new FeatureDecision(null, null, null);
463+
emptyDecision.holdoutDecisions.addAll(holdoutDecisions);
464+
return new DecisionResponse(emptyDecision, reasons);
465+
}
442466
return new DecisionResponse(null, reasons);
443467

444468
}
@@ -480,6 +504,7 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu
480504
}
481505

482506

507+
List<FeatureDecision> holdoutDecisions = new ArrayList<>();
483508
int index = 0;
484509
while (index < rolloutRulesLength) {
485510
Experiment rolloutRule = rollout.getExperiments().get(index);
@@ -496,15 +521,23 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu
496521
AbstractMap.SimpleEntry<FeatureDecision, Boolean> response = decisionVariationResponse.getResult();
497522
FeatureDecision featureDecision = response.getKey();
498523
Boolean skipToEveryoneElse = response.getValue();
524+
if (featureDecision != null && featureDecision.decisionSource == FeatureDecision.DecisionSource.HOLDOUT) {
525+
holdoutDecisions.add(featureDecision);
526+
index++;
527+
continue;
528+
}
499529
if (featureDecision != null) {
530+
featureDecision.holdoutDecisions.addAll(holdoutDecisions);
500531
return new DecisionResponse(featureDecision, reasons);
501532
}
502533

503534
// The last rule is special for "Everyone Else"
504535
index = skipToEveryoneElse ? (rolloutRulesLength - 1) : (index + 1);
505536
}
506537

507-
return new DecisionResponse(new FeatureDecision(null, null, null), reasons);
538+
FeatureDecision emptyDecision = new FeatureDecision(null, null, null);
539+
emptyDecision.holdoutDecisions.addAll(holdoutDecisions);
540+
return new DecisionResponse(emptyDecision, reasons);
508541
}
509542

510543
/**

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2017, 2019, Optimizely, Inc. and contributors *
2+
* Copyright 2017, 2019, 2026, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -15,8 +15,12 @@
1515
***************************************************************************/
1616
package com.optimizely.ab.bucketing;
1717

18+
import javax.annotation.Nonnull;
1819
import javax.annotation.Nullable;
1920

21+
import java.util.ArrayList;
22+
import java.util.List;
23+
2024
import com.optimizely.ab.config.ExperimentCore;
2125
import com.optimizely.ab.config.Variation;
2226

@@ -45,9 +49,20 @@ public class FeatureDecision {
4549
@Nullable
4650
public String cmabUuid;
4751

52+
/**
53+
* @deprecated use {@link #holdoutDecisions}
54+
*/
55+
@Deprecated
4856
@Nullable
4957
public FeatureDecision holdoutDecision;
5058

59+
@Nonnull
60+
public List<FeatureDecision> holdoutDecisions = new ArrayList<>();
61+
62+
/**
63+
* @deprecated use {@link #holdoutDecisions}
64+
*/
65+
@Deprecated
5166
public void setHoldoutDecision(@Nullable FeatureDecision holdoutDecision) {
5267
this.holdoutDecision = holdoutDecision;
5368
}

0 commit comments

Comments
 (0)