Skip to content

Commit dca8fde

Browse files
chillaqchillaq
committed
polish
1 parent aacfd86 commit dca8fde

File tree

4 files changed

+69
-28
lines changed

4 files changed

+69
-28
lines changed

‎client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ private List<String> getFeatureFlagNamesByFlagSets(List<String> flagSets) {
8686
private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bucketingKey, ParsedSplit parsedSplit, Map<String,
8787
Object> attributes) throws ChangeNumberExceptionWrapper {
8888
try {
89-
String config = parsedSplit.configurations() != null ? parsedSplit.configurations().get(parsedSplit.defaultTreatment()) : null;
89+
String config = getConfig(parsedSplit, parsedSplit.defaultTreatment());
9090
if (parsedSplit.killed()) {
9191
return new TreatmentLabelAndChangeNumber(
9292
parsedSplit.defaultTreatment(),
@@ -96,7 +96,7 @@ private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bu
9696
parsedSplit.impressionsDisabled());
9797
}
9898

99-
String bk = (bucketingKey == null) ? matchingKey : bucketingKey;
99+
String bk = getBucketingKey(bucketingKey, matchingKey);
100100

101101
if (!parsedSplit.prerequisitesMatcher().match(matchingKey, bk, attributes, _evaluationContext)) {
102102
return new TreatmentLabelAndChangeNumber(
@@ -125,8 +125,7 @@ private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bu
125125

126126
if (bucket > parsedSplit.trafficAllocation()) {
127127
// out of split
128-
config = parsedSplit.configurations() != null ?
129-
parsedSplit.configurations().get(parsedSplit.defaultTreatment()) : null;
128+
config = getConfig(parsedSplit, parsedSplit.defaultTreatment());
130129
return new TreatmentLabelAndChangeNumber(parsedSplit.defaultTreatment(), Labels.NOT_IN_SPLIT,
131130
parsedSplit.changeNumber(), config, parsedSplit.impressionsDisabled());
132131
}
@@ -137,7 +136,7 @@ private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bu
137136

138137
if (parsedCondition.matcher().match(matchingKey, bucketingKey, attributes, _evaluationContext)) {
139138
String treatment = Splitter.getTreatment(bk, parsedSplit.seed(), parsedCondition.partitions(), parsedSplit.algo());
140-
config = parsedSplit.configurations() != null ? parsedSplit.configurations().get(treatment) : null;
139+
config = getConfig(parsedSplit, treatment);
141140
return new TreatmentLabelAndChangeNumber(
142141
treatment,
143142
parsedCondition.label(),
@@ -147,7 +146,8 @@ private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bu
147146
}
148147
}
149148

150-
config = parsedSplit.configurations() != null ? parsedSplit.configurations().get(parsedSplit.defaultTreatment()) : null;
149+
config = getConfig(parsedSplit, parsedSplit.defaultTreatment());
150+
151151
return new TreatmentLabelAndChangeNumber(
152152
parsedSplit.defaultTreatment(),
153153
Labels.DEFAULT_RULE,
@@ -159,6 +159,14 @@ private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bu
159159
}
160160
}
161161

162+
private String getBucketingKey(String bucketingKey, String matchingKey) {
163+
return (bucketingKey == null) ? matchingKey : bucketingKey;
164+
}
165+
166+
private String getConfig(ParsedSplit parsedSplit, String returnedTreatment) {
167+
return parsedSplit.configurations() != null ? parsedSplit.configurations().get(returnedTreatment) : null;
168+
}
169+
162170
private TreatmentLabelAndChangeNumber evaluateParsedSplit(String matchingKey, String bucketingKey, Map<String, Object> attributes,
163171
ParsedSplit parsedSplit) {
164172
try {

‎client/src/main/java/io/split/engine/experiments/ParsedSplit.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,16 +204,18 @@ public boolean equals(Object obj) {
204204
if (!(obj instanceof ParsedSplit)) return false;
205205

206206
ParsedSplit other = (ParsedSplit) obj;
207+
boolean trafficTypeCond = _trafficTypeName == null ? other._trafficTypeName == null : _trafficTypeName.equals(other._trafficTypeName);
208+
boolean configCond = _configurations == null ? other._configurations == null : _configurations.equals(other._configurations);
207209

208210
return _split.equals(other._split)
209211
&& _seed == other._seed
210212
&& _killed == other._killed
211213
&& _defaultTreatment.equals(other._defaultTreatment)
212214
&& _parsedCondition.equals(other._parsedCondition)
213-
&& _trafficTypeName == null ? other._trafficTypeName == null : _trafficTypeName.equals(other._trafficTypeName)
215+
&& trafficTypeCond
214216
&& _changeNumber == other._changeNumber
215217
&& _algo == other._algo
216-
&& _configurations == null ? other._configurations == null : _configurations.equals(other._configurations)
218+
&& configCond
217219
&& _impressionsDisabled == other._impressionsDisabled
218220
&& _prerequisitesMatcher == other._prerequisitesMatcher;
219221
}

‎client/src/test/java/io/split/engine/experiments/SplitParserTest.java‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ public void works() {
9595
ParsedCondition parsedCondition = ParsedCondition.createParsedConditionForTests(combiningMatcher, partitions);
9696
List<ParsedCondition> listOfMatcherAndSplits = Lists.newArrayList(parsedCondition);
9797

98-
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, new HashSet<>(), false, null);
98+
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, null, false, new PrerequisitesMatcher(null));
9999

100-
Assert.assertEquals(actual, expected);
100+
compareParsed(actual, expected);
101101
assertTrue(expected.hashCode() != 0);
102102
assertTrue(expected.equals(expected));
103103
}
@@ -146,7 +146,7 @@ public void worksWithConfig() {
146146
Assert.assertEquals(actual.defaultTreatment(), expected.defaultTreatment());
147147
Assert.assertEquals(actual.killed(), expected.killed());
148148
Assert.assertEquals(actual.impressionsDisabled(), expected.impressionsDisabled());
149-
Assert.assertEquals(actual.flagSets(), null);
149+
Assert.assertEquals(null, actual.flagSets());
150150
Assert.assertEquals(actual.algo(), expected.algo());
151151
Assert.assertEquals(actual.seed(), expected.seed());
152152
Assert.assertEquals(actual.trafficAllocation(), expected.trafficAllocation());
@@ -188,12 +188,12 @@ public void worksForTwoConditions() {
188188
ParsedSplit actual = parser.parse(split);
189189

190190
ParsedCondition parsedCondition1 = ParsedCondition.createParsedConditionForTests(CombiningMatcher.of(new UserDefinedSegmentMatcher(EMPLOYEES)), fullyRollout);
191-
ParsedCondition parsedCondition2 = ParsedCondition.createParsedConditionForTests(CombiningMatcher.of(new UserDefinedSegmentMatcher(EMPLOYEES)), turnOff);
191+
ParsedCondition parsedCondition2 = ParsedCondition.createParsedConditionForTests(CombiningMatcher.of(new UserDefinedSegmentMatcher(SALES_PEOPLE)), turnOff);
192192
List<ParsedCondition> listOfParsedConditions = Lists.newArrayList(parsedCondition1, parsedCondition2);
193193

194-
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfParsedConditions, "user", 1, 1, new HashSet<>(), false, null);
194+
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfParsedConditions, "user", 1, 1, null, false, new PrerequisitesMatcher(null));
195195

196-
Assert.assertEquals(actual, expected);
196+
compareParsed(actual, expected);
197197
}
198198

199199
@Test
@@ -260,9 +260,9 @@ public void worksWithAttributes() {
260260
ParsedCondition parsedCondition = ParsedCondition.createParsedConditionForTests(combiningMatcher, partitions);
261261
List<ParsedCondition> listOfMatcherAndSplits = Lists.newArrayList(parsedCondition);
262262

263-
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, new HashSet<>(), false, null);
263+
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, null, false, new PrerequisitesMatcher(null));
264264

265-
Assert.assertEquals(actual, expected);
265+
compareParsed(actual, expected);
266266
}
267267

268268
@Test
@@ -293,9 +293,9 @@ public void lessThanOrEqualTo() {
293293
ParsedCondition parsedCondition = ParsedCondition.createParsedConditionForTests(combiningMatcher, partitions);
294294
List<ParsedCondition> listOfMatcherAndSplits = Lists.newArrayList(parsedCondition);
295295

296-
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, new HashSet<>(), false, null);
296+
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, null, false, new PrerequisitesMatcher(null));
297297

298-
Assert.assertEquals(actual, expected);
298+
compareParsed(actual, expected);
299299
}
300300

301301
@Test
@@ -325,9 +325,9 @@ public void equalTo() {
325325
ParsedCondition parsedCondition = ParsedCondition.createParsedConditionForTests(combiningMatcher, partitions);
326326
List<ParsedCondition> listOfMatcherAndSplits = Lists.newArrayList(parsedCondition);
327327

328-
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, new HashSet<>(), false, null);
328+
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, null, false, new PrerequisitesMatcher(null));
329329

330-
Assert.assertEquals(actual, expected);
330+
compareParsed(actual, expected);
331331
}
332332

333333
@Test
@@ -356,9 +356,9 @@ public void equalToNegativeNumber() {
356356
ParsedCondition parsedCondition = ParsedCondition.createParsedConditionForTests(combiningMatcher, partitions);
357357
List<ParsedCondition> listOfMatcherAndSplits = Lists.newArrayList(parsedCondition);
358358

359-
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, new HashSet<>(), false, null);
359+
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, null, false, new PrerequisitesMatcher(null));
360360

361-
Assert.assertEquals(actual, expected);
361+
compareParsed(actual, expected);
362362
}
363363

364364
@Test
@@ -392,9 +392,9 @@ public void between() {
392392
ParsedCondition parsedCondition = ParsedCondition.createParsedConditionForTests(combiningMatcher, partitions);
393393
List<ParsedCondition> listOfMatcherAndSplits = Lists.newArrayList(parsedCondition);
394394

395-
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, new HashSet<>(), false, null);
395+
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("first.name", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, null, false, new PrerequisitesMatcher(null));
396396

397-
Assert.assertEquals(actual, expected);
397+
compareParsed(actual, expected);
398398
}
399399

400400
@Test
@@ -708,11 +708,28 @@ public void setMatcherTest(Condition c, io.split.engine.matchers.Matcher m) {
708708
ParsedCondition parsedCondition = ParsedCondition.createParsedConditionForTests(combiningMatcher, partitions);
709709
List<ParsedCondition> listOfMatcherAndSplits = Lists.newArrayList(parsedCondition);
710710

711-
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("splitName", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, new HashSet<>(), false, null);
711+
ParsedSplit expected = ParsedSplit.createParsedSplitForTests("splitName", 123, false, Treatments.OFF, listOfMatcherAndSplits, "user", 1, 1, null, false, new PrerequisitesMatcher(null));
712712

713-
Assert.assertEquals(actual, expected);
713+
compareParsed(actual, expected);
714714
}
715715

716+
private void compareParsed(ParsedSplit actual, ParsedSplit expected) {
717+
Assert.assertEquals(actual.getRuleBasedSegmentsNames(), expected.getRuleBasedSegmentsNames());
718+
Assert.assertEquals(actual.seed(), expected.seed());
719+
Assert.assertEquals(actual.algo(), expected.algo());
720+
Assert.assertEquals(actual.trafficAllocationSeed(), expected.trafficAllocationSeed());
721+
Assert.assertEquals(actual.flagSets(), expected.flagSets());
722+
Assert.assertEquals(actual.parsedConditions(), expected.parsedConditions());
723+
Assert.assertEquals(actual.trafficAllocation(), expected.trafficAllocation());
724+
Assert.assertEquals(actual.getSegmentsNames(), expected.getSegmentsNames());
725+
Assert.assertEquals(actual.impressionsDisabled(), expected.impressionsDisabled());
726+
Assert.assertEquals(actual.killed(), expected.killed());
727+
Assert.assertEquals(actual.defaultTreatment(), expected.defaultTreatment());
728+
Assert.assertEquals(actual.changeNumber(), expected.changeNumber());
729+
Assert.assertEquals(actual.feature(), expected.feature());
730+
Assert.assertEquals(actual.configurations(), expected.configurations());
731+
Assert.assertEquals(actual.prerequisitesMatcher().toString(), expected.prerequisitesMatcher().toString());
732+
}
716733
private Split makeSplit(String name, int seed, List<Condition> conditions, long changeNumber) {
717734
return makeSplit(name, seed, conditions, changeNumber, null);
718735
}

‎client/src/test/java/io/split/storages/pluggable/adapters/UserCustomSplitAdapterConsumerTest.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,23 @@ public void testGetSplit() {
7676
SplitParser splitParser = new SplitParser();
7777
Split split = getSplit(SPLIT_NAME);
7878
Mockito.when(_userStorageWrapper.get(PrefixAdapter.buildSplitKey(SPLIT_NAME))).thenReturn(getSplitAsJson(split));
79-
ParsedSplit result = _userCustomSplitAdapterConsumer.get(SPLIT_NAME);
79+
ParsedSplit actual = _userCustomSplitAdapterConsumer.get(SPLIT_NAME);
8080
ParsedSplit expected = splitParser.parse(split);
81-
Assert.assertEquals(expected, result);
81+
Assert.assertEquals(actual.getRuleBasedSegmentsNames(), expected.getRuleBasedSegmentsNames());
82+
Assert.assertEquals(actual.seed(), expected.seed());
83+
Assert.assertEquals(actual.algo(), expected.algo());
84+
Assert.assertEquals(actual.trafficAllocationSeed(), expected.trafficAllocationSeed());
85+
Assert.assertEquals(actual.flagSets(), expected.flagSets());
86+
Assert.assertEquals(actual.parsedConditions(), expected.parsedConditions());
87+
Assert.assertEquals(actual.trafficAllocation(), expected.trafficAllocation());
88+
Assert.assertEquals(actual.getSegmentsNames(), expected.getSegmentsNames());
89+
Assert.assertEquals(actual.impressionsDisabled(), expected.impressionsDisabled());
90+
Assert.assertEquals(actual.killed(), expected.killed());
91+
Assert.assertEquals(actual.defaultTreatment(), expected.defaultTreatment());
92+
Assert.assertEquals(actual.changeNumber(), expected.changeNumber());
93+
Assert.assertEquals(actual.feature(), expected.feature());
94+
Assert.assertEquals(actual.configurations(), expected.configurations());
95+
Assert.assertEquals(actual.prerequisitesMatcher().toString(), expected.prerequisitesMatcher().toString());
8296
}
8397

8498
@Test

0 commit comments

Comments
 (0)