Skip to content

Commit 9cabb70

Browse files
committed
Matchers refactor
1 parent 9130bb7 commit 9cabb70

24 files changed

+126
-153
lines changed

client/src/main/java/io/split/client/SplitClientImpl.java

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
package io.split.client;
22

3-
import com.google.common.annotations.VisibleForTesting;
43
import io.split.client.api.Key;
54
import io.split.client.api.SplitResult;
65
import io.split.client.dtos.Event;
7-
import io.split.client.exceptions.ChangeNumberExceptionWrapper;
86
import io.split.client.impressions.Impression;
97
import io.split.client.impressions.ImpressionsManager;
108
import io.split.engine.evaluator.Evaluator;
@@ -34,7 +32,6 @@ public final class SplitClientImpl implements SplitClient {
3432
public static final SplitResult SPLIT_RESULT_CONTROL = new SplitResult(Treatments.CONTROL, null);
3533

3634
private static final String GET_TREATMENT_LABEL = "sdk.getTreatment";
37-
private static final String EXCEPTION = "exception";
3835

3936
private static final Logger _log = LoggerFactory.getLogger(SplitClientImpl.class);
4037

@@ -315,7 +312,7 @@ private SplitResult getTreatmentWithConfigInternal(String label, String matching
315312

316313
long start = System.currentTimeMillis();
317314

318-
EvaluatorImp.TreatmentLabelAndChangeNumber result = getTreatmentResultWithoutImpressions(matchingKey, bucketingKey, split, attributes);
315+
EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(matchingKey, bucketingKey, split, attributes);
319316

320317
recordStats(
321318
matchingKey,
@@ -350,21 +347,6 @@ private void recordStats(String matchingKey, String bucketingKey, String split,
350347
}
351348
}
352349

353-
private EvaluatorImp.TreatmentLabelAndChangeNumber getTreatmentResultWithoutImpressions(String matchingKey, String bucketingKey, String split, Map<String, Object> attributes) {
354-
EvaluatorImp.TreatmentLabelAndChangeNumber result;
355-
try {
356-
result = _evaluator.evaluateFeature(matchingKey, bucketingKey, split, attributes, this);
357-
} catch (ChangeNumberExceptionWrapper e) {
358-
result = new EvaluatorImp.TreatmentLabelAndChangeNumber(Treatments.CONTROL, EXCEPTION, e.changeNumber());
359-
_log.error("Exception", e.wrappedException());
360-
} catch (Exception e) {
361-
result = new EvaluatorImp.TreatmentLabelAndChangeNumber(Treatments.CONTROL, EXCEPTION);
362-
_log.error("Exception", e);
363-
}
364-
365-
return result;
366-
}
367-
368350
private Event createEvent(String key, String trafficType, String eventType) {
369351
Event event = new Event();
370352
event.eventTypeId = eventType;
@@ -373,9 +355,4 @@ private Event createEvent(String key, String trafficType, String eventType) {
373355
event.timestamp = System.currentTimeMillis();
374356
return event;
375357
}
376-
377-
@VisibleForTesting
378-
public String getTreatmentWithoutImpressions(String matchingKey, String bucketingKey, String split, Map<String, Object> attributes) {
379-
return getTreatmentResultWithoutImpressions(matchingKey, bucketingKey, split, attributes).treatment;
380-
}
381358
}
Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package io.split.engine.evaluator;
22

3-
import io.split.client.SplitClientImpl;
4-
import io.split.client.exceptions.ChangeNumberExceptionWrapper;
5-
63
import java.util.Map;
74

85
public interface Evaluator {
9-
EvaluatorImp.TreatmentLabelAndChangeNumber evaluateFeature(String matchingKey, String bucketingKey, String split, Map<String, Object> attributes, SplitClientImpl splitClient) throws ChangeNumberExceptionWrapper;
6+
EvaluatorImp.TreatmentLabelAndChangeNumber evaluateFeature(String matchingKey, String bucketingKey, String split, Map<String, Object> attributes);
107
}

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

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package io.split.engine.evaluator;
22

3-
import io.split.client.SplitClientImpl;
43
import io.split.client.dtos.ConditionType;
54
import io.split.client.exceptions.ChangeNumberExceptionWrapper;
65
import io.split.engine.SDKReadinessGates;
@@ -21,6 +20,7 @@ public class EvaluatorImp implements Evaluator {
2120
private static final String DEFAULT_RULE = "default rule";
2221
private static final String KILLED = "killed";
2322
private static final String DEFINITION_NOT_FOUND = "definition not found";
23+
private static final String EXCEPTION = "exception";
2424

2525
private static final Logger _log = LoggerFactory.getLogger(EvaluatorImp.class);
2626

@@ -34,19 +34,28 @@ public EvaluatorImp(SDKReadinessGates gates,
3434
}
3535

3636
@Override
37-
public TreatmentLabelAndChangeNumber evaluateFeature(String matchingKey, String bucketingKey, String split, Map<String, Object> attributes, SplitClientImpl splitClient) throws ChangeNumberExceptionWrapper {
38-
ParsedSplit parsedSplit = _splitFetcher.fetch(split);
39-
40-
if (parsedSplit == null) {
41-
if (_gates.isSDKReadyNow()) {
42-
_log.warn(
43-
"getTreatment: you passed \"" + split + "\" that does not exist in this environment, " +
44-
"please double check what Splits exist in the web console.");
37+
public TreatmentLabelAndChangeNumber evaluateFeature(String matchingKey, String bucketingKey, String split, Map<String, Object> attributes) {
38+
try {
39+
ParsedSplit parsedSplit = _splitFetcher.fetch(split);
40+
41+
if (parsedSplit == null) {
42+
if (_gates.isSDKReadyNow()) {
43+
_log.warn(
44+
"getTreatment: you passed \"" + split + "\" that does not exist in this environment, " +
45+
"please double check what Splits exist in the web console.");
46+
}
47+
return new TreatmentLabelAndChangeNumber(Treatments.CONTROL, DEFINITION_NOT_FOUND);
4548
}
46-
return new TreatmentLabelAndChangeNumber(Treatments.CONTROL, DEFINITION_NOT_FOUND);
47-
}
4849

49-
return getTreatment(matchingKey, bucketingKey, parsedSplit, attributes, splitClient);
50+
return getTreatment(matchingKey, bucketingKey, parsedSplit, attributes);
51+
}
52+
catch (ChangeNumberExceptionWrapper e) {
53+
_log.error("Evaluator Exception", e.wrappedException());
54+
return new EvaluatorImp.TreatmentLabelAndChangeNumber(Treatments.CONTROL, EXCEPTION, e.changeNumber());
55+
} catch (Exception e) {
56+
_log.error("Evaluator Exception", e);
57+
return new EvaluatorImp.TreatmentLabelAndChangeNumber(Treatments.CONTROL, EXCEPTION);
58+
}
5059
}
5160

5261
/**
@@ -57,7 +66,7 @@ public TreatmentLabelAndChangeNumber evaluateFeature(String matchingKey, String
5766
* @return
5867
* @throws ChangeNumberExceptionWrapper
5968
*/
60-
private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bucketingKey, ParsedSplit parsedSplit, Map<String, Object> attributes, SplitClientImpl splitClient) throws ChangeNumberExceptionWrapper {
69+
private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bucketingKey, ParsedSplit parsedSplit, Map<String, Object> attributes) throws ChangeNumberExceptionWrapper {
6170
try {
6271
if (parsedSplit.killed()) {
6372
String config = parsedSplit.configurations() != null ? parsedSplit.configurations().get(parsedSplit.defaultTreatment()) : null;
@@ -92,7 +101,7 @@ private TreatmentLabelAndChangeNumber getTreatment(String matchingKey, String bu
92101
inRollout = true;
93102
}
94103

95-
if (parsedCondition.matcher().match(matchingKey, bucketingKey, attributes, splitClient)) {
104+
if (parsedCondition.matcher().match(matchingKey, bucketingKey, attributes, this)) {
96105
String treatment = Splitter.getTreatment(bk, parsedSplit.seed(), parsedCondition.partitions(), parsedSplit.algo());
97106
String config = parsedSplit.configurations() != null ? parsedSplit.configurations().get(treatment) : null;
98107
return new TreatmentLabelAndChangeNumber(treatment, parsedCondition.label(), parsedSplit.changeNumber(), config);

client/src/main/java/io/split/engine/matchers/AllKeysMatcher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package io.split.engine.matchers;
22

3-
import io.split.client.SplitClientImpl;
3+
import io.split.engine.evaluator.Evaluator;
44

55
import java.util.Map;
66

@@ -12,7 +12,7 @@
1212
public final class AllKeysMatcher implements Matcher {
1313

1414
@Override
15-
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
15+
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
1616
if (matchValue == null) {
1717
return false;
1818
}

client/src/main/java/io/split/engine/matchers/AttributeMatcher.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package io.split.engine.matchers;
22

3-
import io.split.client.SplitClientImpl;
3+
import io.split.engine.evaluator.Evaluator;
44

55
import java.util.Map;
66
import java.util.Objects;
@@ -27,9 +27,9 @@ public AttributeMatcher(String attribute, Matcher matcher, boolean negate) {
2727
_matcher = new NegatableMatcher(matcher, negate);
2828
}
2929

30-
public boolean match(String key, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
30+
public boolean match(String key, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
3131
if (_attribute == null) {
32-
return _matcher.match(key, bucketingKey, attributes, splitClient);
32+
return _matcher.match(key, bucketingKey, attributes, evaluator);
3333
}
3434

3535
if (attributes == null) {
@@ -95,8 +95,8 @@ public NegatableMatcher(Matcher matcher, boolean negate) {
9595

9696

9797
@Override
98-
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
99-
boolean result = _delegate.match(matchValue, bucketingKey, attributes, splitClient);
98+
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
99+
boolean result = _delegate.match(matchValue, bucketingKey, attributes, evaluator);
100100
return (_negate) ? !result : result;
101101
}
102102

client/src/main/java/io/split/engine/matchers/BetweenMatcher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package io.split.engine.matchers;
22

3-
import io.split.client.SplitClientImpl;
43
import io.split.client.dtos.DataType;
4+
import io.split.engine.evaluator.Evaluator;
55

66
import java.util.Map;
77

@@ -36,7 +36,7 @@ public BetweenMatcher(long start, long end, DataType dataType) {
3636
}
3737

3838
@Override
39-
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
39+
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
4040
Long keyAsLong;
4141

4242
if (_dataType == DataType.DATETIME) {

client/src/main/java/io/split/engine/matchers/BooleanMatcher.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package io.split.engine.matchers;
22

3-
import io.split.client.SplitClientImpl;
3+
import io.split.engine.evaluator.Evaluator;
44

55
import java.util.Map;
66

@@ -14,7 +14,7 @@ public BooleanMatcher(boolean booleanValue) {
1414
}
1515

1616
@Override
17-
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
17+
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
1818
if (matchValue == null) {
1919
return false;
2020
}
@@ -26,7 +26,7 @@ public boolean match(Object matchValue, String bucketingKey, Map<String, Object>
2626

2727
@Override
2828
public String toString() {
29-
return "is " + Boolean.toString(_booleanValue);
29+
return "is " + _booleanValue;
3030
}
3131

3232
@Override

client/src/main/java/io/split/engine/matchers/CombiningMatcher.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.google.common.collect.Lists;
55
import io.split.client.SplitClientImpl;
66
import io.split.client.dtos.MatcherCombiner;
7+
import io.split.engine.evaluator.Evaluator;
78

89
import java.util.List;
910
import java.util.Map;
@@ -38,24 +39,24 @@ public CombiningMatcher(MatcherCombiner combiner, List<AttributeMatcher> delegat
3839
checkArgument(_delegates.size() > 0);
3940
}
4041

41-
public boolean match(String key, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
42+
public boolean match(String key, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
4243
if (_delegates.isEmpty()) {
4344
return false;
4445
}
4546

4647
switch (_combiner) {
4748
case AND:
48-
return and(key, bucketingKey, attributes, splitClient);
49+
return and(key, bucketingKey, attributes, evaluator);
4950
default:
5051
throw new IllegalArgumentException("Unknown combiner: " + _combiner);
5152
}
5253

5354
}
5455

55-
private boolean and(String key, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
56+
private boolean and(String key, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
5657
boolean result = true;
5758
for (AttributeMatcher delegate : _delegates) {
58-
result &= (delegate.match(key, bucketingKey, attributes, splitClient));
59+
result &= (delegate.match(key, bucketingKey, attributes, evaluator));
5960
}
6061
return result;
6162
}

client/src/main/java/io/split/engine/matchers/DependencyMatcher.java

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package io.split.engine.matchers;
22

3-
import io.split.client.SplitClientImpl;
3+
import io.split.engine.evaluator.Evaluator;
44

55
import java.util.List;
66
import java.util.Map;
7+
import java.util.Objects;
78

89
/**
910
* Supports the logic: if user is in split "feature" treatments ["on","off"]
@@ -18,7 +19,7 @@ public DependencyMatcher(String split, List<String> treatments) {
1819
}
1920

2021
@Override
21-
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
22+
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
2223
if (matchValue == null) {
2324
return false;
2425
}
@@ -27,16 +28,7 @@ public boolean match(Object matchValue, String bucketingKey, Map<String, Object>
2728
return false;
2829
}
2930

30-
String result = splitClient.getTreatmentWithoutImpressions(
31-
(String) matchValue,
32-
bucketingKey,
33-
_split,
34-
attributes
35-
);
36-
37-
// if(Treatments.isControl(result)) {
38-
// throw new ParentIsControlException();
39-
// }
31+
String result = evaluator.evaluateFeature((String) matchValue, bucketingKey, _split, attributes).treatment;
4032

4133
return _treatments.contains(result);
4234
}
@@ -58,8 +50,8 @@ public boolean equals(Object o) {
5850

5951
DependencyMatcher that = (DependencyMatcher) o;
6052

61-
if (_split != null ? !_split.equals(that._split) : that._split != null) return false;
62-
return _treatments != null ? _treatments.equals(that._treatments) : that._treatments == null;
53+
if (!Objects.equals(_split, that._split)) return false;
54+
return Objects.equals(_treatments, that._treatments);
6355
}
6456

6557
@Override

client/src/main/java/io/split/engine/matchers/EqualToMatcher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package io.split.engine.matchers;
22

3-
import io.split.client.SplitClientImpl;
43
import io.split.client.dtos.DataType;
4+
import io.split.engine.evaluator.Evaluator;
55

66
import java.util.Map;
77

@@ -29,7 +29,7 @@ public EqualToMatcher(long compareTo, DataType dataType) {
2929
}
3030

3131
@Override
32-
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, SplitClientImpl splitClient) {
32+
public boolean match(Object matchValue, String bucketingKey, Map<String, Object> attributes, Evaluator evaluator) {
3333
Long keyAsLong;
3434

3535
if (_dataType == DataType.DATETIME) {

0 commit comments

Comments
 (0)