Skip to content

Commit 912a491

Browse files
committed
[AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit
1 parent 13ff35a commit 912a491

6 files changed

Lines changed: 446 additions & 50 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: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -351,38 +351,42 @@ 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)) {
362365
decisions.add(new DecisionResponse(decision, reasons, error, decision.cmabUuid));
363366
continue flagLoop;
364367
}
368+
if (decision != null) {
369+
holdoutDecisions.addAll(decision.holdoutDecisions);
370+
}
365371
}
366372

367373
DecisionResponse<FeatureDecision> decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig);
368374
reasons.merge(decisionFeatureResponse.getReasons());
369375
FeatureDecision decision = decisionFeatureResponse.getResult();
370376

371-
if (decision != null && decision.variation != null) {
372-
if (globalHoldoutDecision != null) {
373-
decision.setHoldoutDecision(globalHoldoutDecision);
377+
if (!holdoutDecisions.isEmpty()) {
378+
if (decision == null) {
379+
decision = new FeatureDecision(null, null, null);
374380
}
381+
decision.holdoutDecisions.addAll(0, holdoutDecisions);
382+
}
383+
384+
if (decision != null && decision.variation != null) {
375385
String message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".",
376386
user.getUserId(), featureFlag.getKey());
377387
logger.info(message);
378388
decisions.add(new DecisionResponse(decision, reasons));
379389
} else {
380-
if (globalHoldoutDecision != null) {
381-
if (decision == null) {
382-
decision = new FeatureDecision(null, null, null);
383-
}
384-
decision.setHoldoutDecision(globalHoldoutDecision);
385-
}
386390
String message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".",
387391
user.getUserId(), featureFlag.getKey());
388392
logger.info(message);
@@ -422,6 +426,7 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
422426
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
423427
// Cache flagKey once to avoid multiple getKey() calls (important for mock-based tests)
424428
String flagKey = featureFlag.getKey();
429+
List<FeatureDecision> holdoutDecisions = new ArrayList<>();
425430
if (!featureFlag.getExperimentIds().isEmpty()) {
426431
for (String experimentId : featureFlag.getExperimentIds()) {
427432
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
@@ -430,7 +435,12 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
430435
getVariationFromExperimentRule(projectConfig, flagKey, experiment, user, options, userProfileTracker, decisionPath);
431436
reasons.merge(decisionVariation.getReasons());
432437
FeatureDecision featureDecision = decisionVariation.getResult();
438+
if (featureDecision != null && featureDecision.decisionSource == FeatureDecision.DecisionSource.HOLDOUT) {
439+
holdoutDecisions.add(featureDecision);
440+
continue;
441+
}
433442
if (decisionVariation.isError() || (featureDecision != null && featureDecision.variation != null)) {
443+
featureDecision.holdoutDecisions.addAll(holdoutDecisions);
434444
return new DecisionResponse(featureDecision, reasons, decisionVariation.isError(), decisionVariation.getCmabUuid());
435445
}
436446
}
@@ -439,6 +449,11 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
439449
logger.info(message);
440450
}
441451

452+
if (!holdoutDecisions.isEmpty()) {
453+
FeatureDecision emptyDecision = new FeatureDecision(null, null, null);
454+
emptyDecision.holdoutDecisions.addAll(holdoutDecisions);
455+
return new DecisionResponse(emptyDecision, reasons);
456+
}
442457
return new DecisionResponse(null, reasons);
443458

444459
}
@@ -480,6 +495,7 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu
480495
}
481496

482497

498+
List<FeatureDecision> holdoutDecisions = new ArrayList<>();
483499
int index = 0;
484500
while (index < rolloutRulesLength) {
485501
Experiment rolloutRule = rollout.getExperiments().get(index);
@@ -496,15 +512,23 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu
496512
AbstractMap.SimpleEntry<FeatureDecision, Boolean> response = decisionVariationResponse.getResult();
497513
FeatureDecision featureDecision = response.getKey();
498514
Boolean skipToEveryoneElse = response.getValue();
515+
if (featureDecision != null && featureDecision.decisionSource == FeatureDecision.DecisionSource.HOLDOUT) {
516+
holdoutDecisions.add(featureDecision);
517+
index++;
518+
continue;
519+
}
499520
if (featureDecision != null) {
521+
featureDecision.holdoutDecisions.addAll(holdoutDecisions);
500522
return new DecisionResponse(featureDecision, reasons);
501523
}
502524

503525
// The last rule is special for "Everyone Else"
504526
index = skipToEveryoneElse ? (rolloutRulesLength - 1) : (index + 1);
505527
}
506528

507-
return new DecisionResponse(new FeatureDecision(null, null, null), reasons);
529+
FeatureDecision emptyDecision = new FeatureDecision(null, null, null);
530+
emptyDecision.holdoutDecisions.addAll(holdoutDecisions);
531+
return new DecisionResponse(emptyDecision, reasons);
508532
}
509533

510534
/**

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

Lines changed: 7 additions & 7 deletions
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,12 +49,8 @@ public class FeatureDecision {
4549
@Nullable
4650
public String cmabUuid;
4751

48-
@Nullable
49-
public FeatureDecision holdoutDecision;
50-
51-
public void setHoldoutDecision(@Nullable FeatureDecision holdoutDecision) {
52-
this.holdoutDecision = holdoutDecision;
53-
}
52+
@Nonnull
53+
public List<FeatureDecision> holdoutDecisions = new ArrayList<>();
5454

5555
public enum DecisionSource {
5656
FEATURE_TEST("feature-test"),

‎core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,4 +2204,158 @@ public void decide_for_keys_with_holdout() throws Exception {
22042204
logbackVerifier.expectMessage(Level.INFO, expectedReason);
22052205
}
22062206

2207+
private Optimizely createOptimizelyWithLocalHoldouts(boolean sendFlagDecisions) throws Exception {
2208+
String datafile = Resources.toString(Resources.getResource("config/local-holdouts-project-config.json"), Charsets.UTF_8);
2209+
if (!sendFlagDecisions) {
2210+
datafile = datafile.replace("\"sendFlagDecisions\": true", "\"sendFlagDecisions\": false");
2211+
}
2212+
return new Optimizely.Builder().withDatafile(datafile).withEventProcessor(new ForwardingEventProcessor(eventHandler, null)).build();
2213+
}
2214+
2215+
private void expectLocalHoldoutImpression(String holdoutId, String holdoutKey, String flagKey, Map<String, ?> attributes) {
2216+
DecisionMetadata metadata = new DecisionMetadata.Builder()
2217+
.setFlagKey(flagKey)
2218+
.setRuleKey(holdoutKey)
2219+
.setRuleType("holdout")
2220+
.setVariationKey("ho_off_key")
2221+
.setEnabled(false)
2222+
.build();
2223+
eventHandler.expectImpression(holdoutId, null, userId, attributes, metadata);
2224+
}
2225+
2226+
private void expectRuleImpression(String experimentId, String variationId, String flagKey, String ruleKey, String ruleType, String variationKey, Map<String, ?> attributes) {
2227+
DecisionMetadata metadata = new DecisionMetadata.Builder()
2228+
.setFlagKey(flagKey)
2229+
.setRuleKey(ruleKey)
2230+
.setRuleType(ruleType)
2231+
.setVariationKey(variationKey)
2232+
.setEnabled(true)
2233+
.build();
2234+
eventHandler.expectImpression(experimentId, variationId, userId, attributes, metadata);
2235+
}
2236+
2237+
@Test
2238+
public void decide_localHoldoutOnFirstExperimentRule_servedBySecondExperimentRule() throws Exception {
2239+
Optimizely opt = createOptimizelyWithLocalHoldouts(true);
2240+
Map<String, Object> attrs = Collections.singletonMap("lh", "exp1");
2241+
2242+
OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_flag");
2243+
2244+
assertEquals("exp_2_on", decision.getVariationKey());
2245+
assertEquals("exp_rule_2", decision.getRuleKey());
2246+
assertTrue(decision.getEnabled());
2247+
expectRuleImpression("5200000002", "5200000012", "lh_flag", "exp_rule_2", "feature-test", "exp_2_on", attrs);
2248+
expectLocalHoldoutImpression("5300000001", "lh_exp_rule_1", "lh_flag", attrs);
2249+
}
2250+
2251+
@Test
2252+
public void decide_localHoldoutsOnSeveralRules_sendEventPerHit() throws Exception {
2253+
Optimizely opt = createOptimizelyWithLocalHoldouts(true);
2254+
Map<String, Object> attrs = Collections.singletonMap("lh", "exp1,exp2");
2255+
2256+
OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_flag");
2257+
2258+
assertEquals("delivery_on", decision.getVariationKey());
2259+
assertEquals("delivery_rule", decision.getRuleKey());
2260+
expectRuleImpression("5400000001", "5400000011", "lh_flag", "delivery_rule", "rollout", "delivery_on", attrs);
2261+
expectLocalHoldoutImpression("5300000001", "lh_exp_rule_1", "lh_flag", attrs);
2262+
expectLocalHoldoutImpression("5300000002", "lh_exp_rule_2", "lh_flag", attrs);
2263+
}
2264+
2265+
@Test
2266+
public void decide_localHoldoutHitAndNoRuleServes_stillSendsHoldoutEvents() throws Exception {
2267+
Optimizely opt = createOptimizelyWithLocalHoldouts(true);
2268+
Map<String, Object> attrs = Collections.singletonMap("lh", "delivery,everyone");
2269+
2270+
OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_delivery_flag");
2271+
2272+
assertNull(decision.getVariationKey());
2273+
assertNull(decision.getRuleKey());
2274+
assertFalse(decision.getEnabled());
2275+
DecisionMetadata offMetadata = new DecisionMetadata.Builder()
2276+
.setFlagKey("lh_delivery_flag")
2277+
.setRuleKey("")
2278+
.setRuleType("rollout")
2279+
.setVariationKey("")
2280+
.setEnabled(false)
2281+
.build();
2282+
eventHandler.expectImpression(null, null, userId, attrs, offMetadata);
2283+
expectLocalHoldoutImpression("5300000003", "lh_delivery_rule", "lh_delivery_flag", attrs);
2284+
expectLocalHoldoutImpression("5300000004", "lh_everyone_else_rule", "lh_delivery_flag", attrs);
2285+
}
2286+
2287+
@Test
2288+
public void decide_localHoldoutOnDeliveryRule_sendsHoldoutEventWhenSendFlagDecisionsOff() throws Exception {
2289+
Optimizely opt = createOptimizelyWithLocalHoldouts(false);
2290+
Map<String, Object> attrs = Collections.singletonMap("lh", "delivery");
2291+
2292+
OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_delivery_flag");
2293+
2294+
assertEquals("everyone_else_on", decision.getVariationKey());
2295+
assertEquals("everyone_else_rule", decision.getRuleKey());
2296+
assertTrue(decision.getEnabled());
2297+
expectLocalHoldoutImpression("5300000003", "lh_delivery_rule", "lh_delivery_flag", attrs);
2298+
}
2299+
2300+
@Test
2301+
public void decide_localHoldoutOnFirstDeliveryRule_servedBySecondDeliveryRule() throws Exception {
2302+
Optimizely opt = createOptimizelyWithLocalHoldouts(true);
2303+
Map<String, Object> attrs = Collections.emptyMap();
2304+
2305+
OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_multi_delivery_flag");
2306+
2307+
assertEquals("second_delivery_on", decision.getVariationKey());
2308+
assertEquals("second_delivery_rule", decision.getRuleKey());
2309+
assertTrue(decision.getEnabled());
2310+
expectRuleImpression("5400000004", "5400000014", "lh_multi_delivery_flag", "second_delivery_rule", "rollout", "second_delivery_on", attrs);
2311+
expectLocalHoldoutImpression("5300000006", "lh_first_delivery_rule", "lh_multi_delivery_flag", attrs);
2312+
}
2313+
2314+
@Test
2315+
public void decide_localHoldoutWithDisableDecisionEvent_sendsNoEvents() throws Exception {
2316+
Optimizely opt = createOptimizelyWithLocalHoldouts(true);
2317+
Map<String, Object> attrs = Collections.singletonMap("lh", "exp1,delivery");
2318+
2319+
OptimizelyDecision decision = opt.createUserContext(userId, attrs)
2320+
.decide("lh_flag", Collections.singletonList(OptimizelyDecideOption.DISABLE_DECISION_EVENT));
2321+
2322+
assertEquals("exp_2_on", decision.getVariationKey());
2323+
eventHandler.expectCalls(0);
2324+
}
2325+
2326+
@Test
2327+
public void decide_oneLocalHoldoutOnTwoRules_sendsEventPerRule() throws Exception {
2328+
Optimizely opt = createOptimizelyWithLocalHoldouts(true);
2329+
Map<String, Object> attrs = Collections.singletonMap("lh", "multi");
2330+
2331+
OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_flag");
2332+
2333+
assertEquals("delivery_on", decision.getVariationKey());
2334+
expectRuleImpression("5400000001", "5400000011", "lh_flag", "delivery_rule", "rollout", "delivery_on", attrs);
2335+
expectLocalHoldoutImpression("5300000005", "lh_both_exp_rules", "lh_flag", attrs);
2336+
expectLocalHoldoutImpression("5300000005", "lh_both_exp_rules", "lh_flag", attrs);
2337+
}
2338+
2339+
@Test
2340+
public void decide_noLocalHoldoutHit_servedByFirstRule() throws Exception {
2341+
Optimizely opt = createOptimizelyWithLocalHoldouts(true);
2342+
Map<String, Object> attrs = Collections.singletonMap("lh", "none");
2343+
2344+
OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_flag");
2345+
2346+
assertEquals("exp_1_on", decision.getVariationKey());
2347+
assertEquals("exp_rule_1", decision.getRuleKey());
2348+
expectRuleImpression("5200000001", "5200000011", "lh_flag", "exp_rule_1", "feature-test", "exp_1_on", attrs);
2349+
}
2350+
2351+
@Test
2352+
public void isFeatureEnabled_skipsLocallyHeldOutRuleWithoutHoldoutEvent() throws Exception {
2353+
Optimizely opt = createOptimizelyWithLocalHoldouts(true);
2354+
Map<String, Object> attrs = Collections.singletonMap("lh", "exp1");
2355+
2356+
assertTrue(opt.isFeatureEnabled("lh_flag", userId, attrs));
2357+
2358+
expectRuleImpression("5200000002", "5200000012", "lh_flag", "exp_rule_2", "feature-test", "exp_2_on", attrs);
2359+
}
2360+
22072361
}

0 commit comments

Comments
 (0)