Skip to content

Commit 7d3012c

Browse files
committed
fix compile errors
1 parent c37a6d6 commit 7d3012c

10 files changed

Lines changed: 61 additions & 144 deletions

File tree

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

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,19 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
338338
}
339339
}
340340

341+
// Evaluate local holdouts targeting this specific feature flag
342+
List<Holdout> localHoldouts = projectConfig.getHoldoutsForRule(featureFlag.getId());
343+
if (!localHoldouts.isEmpty()) {
344+
for (Holdout holdout : localHoldouts) {
345+
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
346+
reasons.merge(holdoutDecision.getReasons());
347+
if (holdoutDecision.getResult() != null) {
348+
decisions.add(new DecisionResponse<>(new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT), reasons));
349+
continue flagLoop;
350+
}
351+
}
352+
}
353+
341354
DecisionResponse<FeatureDecision> decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker, decisionPath);
342355
reasons.merge(decisionVariationResponse.getReasons());
343356

@@ -849,14 +862,16 @@ private DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proj
849862
}
850863

851864
// Check local holdouts targeting this rule
852-
List<Holdout> localHoldouts = projectConfig.getHoldoutsForRule(rule.getId());
853-
if (!localHoldouts.isEmpty()) {
854-
for (Holdout holdout : localHoldouts) {
855-
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
856-
reasons.merge(holdoutDecision.getReasons());
857-
if (holdoutDecision.getResult() != null) {
858-
// User is in holdout - return holdout variation immediately, skip this rule
859-
return new DecisionResponse(holdoutDecision.getResult(), reasons);
865+
if (rule != null) {
866+
List<Holdout> localHoldouts = projectConfig.getHoldoutsForRule(rule.getId());
867+
if (!localHoldouts.isEmpty()) {
868+
for (Holdout holdout : localHoldouts) {
869+
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
870+
reasons.merge(holdoutDecision.getReasons());
871+
if (holdoutDecision.getResult() != null) {
872+
// User is in holdout - return holdout variation immediately, skip this rule
873+
return new DecisionResponse(holdoutDecision.getResult(), reasons);
874+
}
860875
}
861876
}
862877
}

core-api/src/main/java/com/optimizely/ab/config/Holdout.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public class Holdout implements ExperimentCore {
3838
private final String id;
3939
private final String key;
4040
private final String status;
41-
41+
4242
private final List<String> audienceIds;
4343
private final Condition<AudienceIdCondition> audienceConditions;
4444
private final List<Variation> variations;

core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -202,23 +202,16 @@ static Holdout parseHoldout(JsonObject holdoutJson, JsonDeserializationContext c
202202
List<TrafficAllocation> trafficAllocations =
203203
parseTrafficAllocation(holdoutJson.getAsJsonArray("trafficAllocation"));
204204

205-
List<String> includedFlags = new ArrayList<>();
206-
if (holdoutJson.has("includedFlags")) {
207-
JsonArray includedIdsJson = holdoutJson.getAsJsonArray("includedFlags");
208-
for (JsonElement hoIdObj : includedIdsJson) {
209-
includedFlags.add(hoIdObj.getAsString());
205+
List<String> includedRules = null;
206+
if (holdoutJson.has("includedRules")) {
207+
JsonArray includedRulesJson = holdoutJson.getAsJsonArray("includedRules");
208+
includedRules = new ArrayList<>();
209+
for (JsonElement ruleIdObj : includedRulesJson) {
210+
includedRules.add(ruleIdObj.getAsString());
210211
}
211212
}
212213

213-
List<String> excludedFlags = new ArrayList<>();
214-
if (holdoutJson.has("excludedFlags")) {
215-
JsonArray excludedIdsJson = holdoutJson.getAsJsonArray("excludedFlags");
216-
for (JsonElement hoIdObj : excludedIdsJson) {
217-
excludedFlags.add(hoIdObj.getAsString());
218-
}
219-
}
220-
221-
return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations, includedFlags, excludedFlags);
214+
return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations, includedRules);
222215
}
223216

224217
static FeatureFlag parseFeatureFlag(JsonObject featureFlagJson, JsonDeserializationContext context) {

core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -242,34 +242,19 @@ private List<Holdout> parseHoldouts(JSONArray holdoutJson) {
242242
List<TrafficAllocation> trafficAllocations =
243243
parseTrafficAllocation(holdoutObject.getJSONArray("trafficAllocation"));
244244

245-
List<String> includedFlags;
246-
if (holdoutObject.has("includedFlags")) {
247-
JSONArray includedIdsJson = holdoutObject.getJSONArray("includedFlags");
248-
includedFlags = new ArrayList<>(includedIdsJson.length());
249-
250-
for (int j = 0; j < includedIdsJson.length(); j++) {
251-
Object idObj = includedIdsJson.get(j);
252-
includedFlags.add((String) idObj);
245+
List<String> includedRules = null;
246+
if (holdoutObject.has("includedRules")) {
247+
JSONArray includedRulesJson = holdoutObject.getJSONArray("includedRules");
248+
includedRules = new ArrayList<>(includedRulesJson.length());
249+
250+
for (int j = 0; j < includedRulesJson.length(); j++) {
251+
Object idObj = includedRulesJson.get(j);
252+
includedRules.add((String) idObj);
253253
}
254-
} else {
255-
includedFlags = Collections.emptyList();
256-
}
257-
258-
List<String> excludedFlags;
259-
if (holdoutObject.has("excludedFlags")) {
260-
JSONArray excludedIdsJson = holdoutObject.getJSONArray("excludedFlags");
261-
excludedFlags = new ArrayList<>(excludedIdsJson.length());
262-
263-
for (int j = 0; j < excludedIdsJson.length(); j++) {
264-
Object idObj = excludedIdsJson.get(j);
265-
excludedFlags.add((String) idObj);
266-
}
267-
} else {
268-
excludedFlags = Collections.emptyList();
269254
}
270255

271256
holdouts.add(new Holdout(id, key, status, audienceIds, conditions, variations,
272-
trafficAllocations, includedFlags, excludedFlags));
257+
trafficAllocations, includedRules));
273258
}
274259

275260
return holdouts;

core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -261,22 +261,13 @@ private List<Holdout> parseHoldouts(JSONArray holdoutJson) {
261261
List<TrafficAllocation> trafficAllocations =
262262
parseTrafficAllocation((JSONArray) hoObject.get("trafficAllocation"));
263263

264-
List<String> includedFlags;
265-
if (hoObject.containsKey("includedFlags")) {
266-
includedFlags = new ArrayList<String>((JSONArray) hoObject.get("includedFlags"));
267-
} else {
268-
includedFlags = Collections.emptyList();
269-
}
270-
271-
List<String> excludedFlags;
272-
if (hoObject.containsKey("excludedFlags")) {
273-
excludedFlags = new ArrayList<String>((JSONArray) hoObject.get("excludedFlags"));
274-
} else {
275-
excludedFlags = Collections.emptyList();
264+
List<String> includedRules = null;
265+
if (hoObject.containsKey("includedRules")) {
266+
includedRules = new ArrayList<String>((JSONArray) hoObject.get("includedRules"));
276267
}
277268

278269
holdouts.add(new Holdout(id, key, status, audienceIds, conditions, variations,
279-
trafficAllocations, includedFlags, excludedFlags));
270+
trafficAllocations, includedRules));
280271
}
281272

282273
return holdouts;

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

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,59 +1324,6 @@ public void getVariationForFeatureReturnHoldoutDecisionForGlobalHoldout() {
13241324
logbackVerifier.expectMessage(Level.INFO, "User (user123) is in variation (ho_off_key) of holdout (basic_holdout).");
13251325
}
13261326

1327-
@Test
1328-
public void includedFlagsHoldoutOnlyAppliestoSpecificFlags() {
1329-
ProjectConfig holdoutProjectConfig = generateValidProjectConfigV4_holdout();
1330-
1331-
Bucketer mockBucketer = new Bucketer();
1332-
CmabService cmabService = mock(CmabService.class);
1333-
DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, null, cmabService);
1334-
1335-
Map<String, Object> attributes = new HashMap<>();
1336-
attributes.put("$opt_bucketing_id", "ppid120000");
1337-
FeatureDecision featureDecision = decisionService.getVariationForFeature(
1338-
FEATURE_FLAG_BOOLEAN_FEATURE,
1339-
optimizely.createUserContext("user123", attributes),
1340-
holdoutProjectConfig
1341-
).getResult();
1342-
1343-
assertEquals(HOLDOUT_INCLUDED_FLAGS_HOLDOUT, featureDecision.experiment);
1344-
assertEquals(VARIATION_HOLDOUT_VARIATION_OFF, featureDecision.variation);
1345-
assertEquals(FeatureDecision.DecisionSource.HOLDOUT, featureDecision.decisionSource);
1346-
1347-
logbackVerifier.expectMessage(Level.INFO, "User (user123) is in variation (ho_off_key) of holdout (holdout_included_flags).");
1348-
}
1349-
1350-
@Test
1351-
public void excludedFlagsHoldoutAppliesToAllExceptSpecified() {
1352-
ProjectConfig holdoutProjectConfig = generateValidProjectConfigV4_holdout();
1353-
1354-
Bucketer mockBucketer = new Bucketer();
1355-
1356-
DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, null, mockCmabService);
1357-
1358-
Map<String, Object> attributes = new HashMap<>();
1359-
attributes.put("$opt_bucketing_id", "ppid300002");
1360-
FeatureDecision excludedDecision = decisionService.getVariationForFeature(
1361-
FEATURE_FLAG_SINGLE_VARIABLE_BOOLEAN, // excluded from ho (holdout_excluded_flags)
1362-
optimizely.createUserContext("user123", attributes),
1363-
holdoutProjectConfig
1364-
).getResult();
1365-
1366-
assertNotEquals(FeatureDecision.DecisionSource.HOLDOUT, excludedDecision.decisionSource);
1367-
1368-
FeatureDecision featureDecision = decisionService.getVariationForFeature(
1369-
FEATURE_FLAG_SINGLE_VARIABLE_INTEGER, // excluded from ho (holdout_excluded_flags)
1370-
optimizely.createUserContext("user123", attributes),
1371-
holdoutProjectConfig
1372-
).getResult();
1373-
1374-
assertEquals(HOLDOUT_EXCLUDED_FLAGS_HOLDOUT, featureDecision.experiment);
1375-
assertEquals(VARIATION_HOLDOUT_VARIATION_OFF, featureDecision.variation);
1376-
assertEquals(FeatureDecision.DecisionSource.HOLDOUT, featureDecision.decisionSource);
1377-
1378-
logbackVerifier.expectMessage(Level.INFO, "User (user123) is in variation (ho_off_key) of holdout (holdout_excluded_flags).");
1379-
}
13801327

13811328
@Test
13821329
public void userMeetsHoldoutAudienceConditions() {

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
package com.optimizely.ab.bucketing;
1818

19+
import com.optimizely.ab.Optimizely;
1920
import com.optimizely.ab.OptimizelyUserContext;
2021
import com.optimizely.ab.config.*;
2122
import com.optimizely.ab.error.ErrorHandler;
@@ -25,7 +26,7 @@
2526
import org.junit.Test;
2627
import org.junit.runner.RunWith;
2728
import org.mockito.Mock;
28-
import org.mockito.junit.MockitoJUnitRunner;
29+
import org.mockito.runners.MockitoJUnitRunner;
2930

3031
import java.util.*;
3132

@@ -46,6 +47,7 @@ public class LocalHoldoutsDecisionServiceTest {
4647

4748
private DecisionService decisionService;
4849
private ErrorHandler errorHandler;
50+
private Optimizely optimizely;
4951

5052
private Variation controlVariation;
5153
private Variation treatmentVariation;
@@ -59,6 +61,7 @@ public class LocalHoldoutsDecisionServiceTest {
5961

6062
@Before
6163
public void setUp() {
64+
optimizely = Optimizely.builder().build();
6265
errorHandler = new NoOpErrorHandler();
6366
decisionService = new DecisionService(mockBucketer, errorHandler, null);
6467

@@ -82,25 +85,23 @@ public void setUp() {
8285
// Create experiment rule
8386
experimentRule = new Experiment("experiment_rule_id", "experiment_rule",
8487
"Running", "layer_id", Collections.emptyList(), null,
85-
variations, Collections.emptyMap(), Collections.emptyList(),
86-
Collections.emptyMap());
88+
variations, Collections.emptyMap(), Collections.emptyList());
8789

8890
// Create delivery rule
8991
deliveryRule = new Experiment("delivery_rule_id", "delivery_rule",
9092
"Running", "layer_id", Collections.emptyList(), null,
91-
variations, Collections.emptyMap(), Collections.emptyList(),
92-
Collections.emptyMap());
93+
variations, Collections.emptyMap(), Collections.emptyList());
9394

9495
// Create feature flag
9596
featureFlag = new FeatureFlag("flag_id", "test_flag", "layer_id",
96-
Collections.singletonList("experiment_rule_id"), "rollout_id",
97+
Collections.singletonList("experiment_rule_id"),
9798
Collections.emptyList());
9899
}
99100

100101
@Test
101102
public void testGlobalHoldoutEvaluatedAtFlagLevel() {
102103
// Arrange
103-
OptimizelyUserContext user = new OptimizelyUserContext(null, "user123", Collections.emptyMap());
104+
OptimizelyUserContext user = new OptimizelyUserContext(optimizely, "user123", Collections.emptyMap());
104105

105106
when(mockProjectConfig.getGlobalHoldouts()).thenReturn(Collections.singletonList(globalHoldout));
106107
when(mockBucketer.bucket(eq(globalHoldout), anyString(), eq(mockProjectConfig)))
@@ -118,7 +119,7 @@ public void testGlobalHoldoutEvaluatedAtFlagLevel() {
118119
@Test
119120
public void testLocalHoldoutEvaluatedAtRuleLevel() {
120121
// Arrange
121-
OptimizelyUserContext user = new OptimizelyUserContext(null, "user123", Collections.emptyMap());
122+
OptimizelyUserContext user = new OptimizelyUserContext(optimizely, "user123", Collections.emptyMap());
122123

123124
when(mockProjectConfig.getHoldoutsForRule("experiment_rule_id"))
124125
.thenReturn(Collections.singletonList(localHoldout));
@@ -136,7 +137,7 @@ public void testLocalHoldoutEvaluatedAtRuleLevel() {
136137
@Test
137138
public void testLocalHoldoutNotAppliedToNonTargetedRule() {
138139
// Arrange - local holdout only targets "experiment_rule_id"
139-
OptimizelyUserContext user = new OptimizelyUserContext(null, "user123", Collections.emptyMap());
140+
OptimizelyUserContext user = new OptimizelyUserContext(optimizely, "user123", Collections.emptyMap());
140141

141142
when(mockProjectConfig.getHoldoutsForRule("other_rule_id"))
142143
.thenReturn(Collections.emptyList());
@@ -281,7 +282,7 @@ public void testInactiveHoldoutNotApplied() {
281282
Collections.emptyList(), null, Arrays.asList(holdoutVariation), Collections.emptyList(),
282283
Arrays.asList("rule1"));
283284

284-
OptimizelyUserContext user = new OptimizelyUserContext(null, "user123", Collections.emptyMap());
285+
OptimizelyUserContext user = new OptimizelyUserContext(optimizely, "user123", Collections.emptyMap());
285286

286287
// Act
287288
DecisionResponse<Variation> result = decisionService.getVariationForHoldout(draftHoldout, user, mockProjectConfig);

core-api/src/test/java/com/optimizely/ab/config/DatafileProjectConfigTestUtils.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,7 @@ private static void verifyHoldouts(List<Holdout> actual, List<Holdout> expected)
527527
// System.out.println("Expected holdouts: " + expected);
528528
// System.out.println("Actual size: " + actual.size());
529529
// System.out.println("Expected size: " + expected.size());
530-
530+
531531
assertThat(actual.size(), is(expected.size()));
532532

533533

@@ -544,8 +544,7 @@ private static void verifyHoldouts(List<Holdout> actual, List<Holdout> expected)
544544
// System.out.println("Actual audience conditions: " + actualHoldout.getAudienceConditions());
545545
// System.out.println("Expected audience conditions: " + expectedHoldout.getAudienceConditions());
546546
assertThat(actualHoldout.getAudienceConditions(), is(expectedHoldout.getAudienceConditions()));
547-
assertThat(actualHoldout.getIncludedFlags(), is(expectedHoldout.getIncludedFlags()));
548-
assertThat(actualHoldout.getExcludedFlags(), is(expectedHoldout.getExcludedFlags()));
547+
assertThat(actualHoldout.getIncludedRules(), is(expectedHoldout.getIncludedRules()));
549548
verifyVariations(actualHoldout.getVariations(), expectedHoldout.getVariations());
550549
verifyTrafficAllocations(actualHoldout.getTrafficAllocation(),
551550
expectedHoldout.getTrafficAllocation());

core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,6 @@ public class ValidProjectConfigV4 {
551551
500
552552
)
553553
),
554-
null,
555554
null
556555
);
557556

@@ -570,7 +569,6 @@ public class ValidProjectConfigV4 {
570569
0
571570
)
572571
),
573-
null,
574572
null
575573
);
576574

@@ -593,8 +591,7 @@ public class ValidProjectConfigV4 {
593591
"4195505407",
594592
"3926744821",
595593
"3281420120"
596-
),
597-
null
594+
)
598595
);
599596

600597
public static final Holdout HOLDOUT_EXCLUDED_FLAGS_HOLDOUT = new Holdout(
@@ -612,12 +609,7 @@ public class ValidProjectConfigV4 {
612609
1500
613610
)
614611
),
615-
null,
616-
DatafileProjectConfigTestUtils.createListOfObjects(
617-
"2591051011",
618-
"2079378557",
619-
"3263342226"
620-
)
612+
null
621613
);
622614

623615
public static final Holdout HOLDOUT_TYPEDAUDIENCE_HOLDOUT = new Holdout(
@@ -640,8 +632,7 @@ public class ValidProjectConfigV4 {
640632
1000
641633
)
642634
),
643-
Collections.<String>emptyList(),
644-
Collections.<String>emptyList()
635+
null
645636
);
646637
private static final String LAYER_TYPEDAUDIENCE_EXPERIMENT_ID = "1630555627";
647638
private static final String EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT_ID = "1323241597";

core-api/src/test/resources/config/holdouts-project-config.json

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@
512512
"key": "ho_off_key"
513513
}
514514
],
515-
"includedFlags": [
515+
"includedRules": [
516516
"4195505407",
517517
"3926744821",
518518
"3281420120"
@@ -574,11 +574,6 @@
574574
"id": "$opt_dummy_variation_id",
575575
"key": "ho_off_key"
576576
}
577-
],
578-
"excludedFlags": [
579-
"2591051011",
580-
"2079378557",
581-
"3263342226"
582577
]
583578
}
584579
],

0 commit comments

Comments
 (0)