Skip to content

Commit f0d0bc6

Browse files
sanzmaurosanzmauro
committed
pr feedback
1 parent 9eb18e5 commit f0d0bc6

File tree

6 files changed

+43
-46
lines changed

6 files changed

+43
-46
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public final class SplitClientImpl implements SplitClient {
3232
public static final SplitResult SPLIT_RESULT_CONTROL = new SplitResult(Treatments.CONTROL, null);
3333

3434
private static final String GET_TREATMENT_LABEL = "sdk.getTreatment";
35+
private static final String DEFINITION_NOT_FOUND = "definition not found";
3536

3637
private static final Logger _log = LoggerFactory.getLogger(SplitClientImpl.class);
3738

@@ -314,6 +315,12 @@ private SplitResult getTreatmentWithConfigInternal(String label, String matching
314315

315316
EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(matchingKey, bucketingKey, split, attributes);
316317

318+
if (result.treatment.equals(Treatments.CONTROL) && result.label.equals(DEFINITION_NOT_FOUND) && _gates.isSDKReadyNow()) {
319+
_log.warn(
320+
"getTreatment: you passed \"" + split + "\" that does not exist in this environment, " +
321+
"please double check what Splits exist in the web console.");
322+
}
323+
317324
recordStats(
318325
matchingKey,
319326
bucketingKey,

‎client/src/main/java/io/split/client/SplitFactoryImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn
230230
syncManager.start();
231231

232232
// Evaluator
233-
final Evaluator evaluator = new EvaluatorImp(gates, splitCache);
233+
final Evaluator evaluator = new EvaluatorImp(splitCache);
234234

235235
destroyer = new Runnable() {
236236
public void run() {

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

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@
22

33
import io.split.client.dtos.ConditionType;
44
import io.split.client.exceptions.ChangeNumberExceptionWrapper;
5-
import io.split.engine.SDKReadinessGates;
65
import io.split.engine.cache.SplitCache;
76
import io.split.engine.experiments.ParsedCondition;
87
import io.split.engine.experiments.ParsedSplit;
9-
import io.split.engine.experiments.SplitFetcher;
108
import io.split.engine.splitter.Splitter;
119
import io.split.grammar.Treatments;
1210
import org.slf4j.Logger;
@@ -25,12 +23,9 @@ public class EvaluatorImp implements Evaluator {
2523

2624
private static final Logger _log = LoggerFactory.getLogger(EvaluatorImp.class);
2725

28-
private final SDKReadinessGates _gates;
2926
private final SplitCache _splitCache;
3027

31-
public EvaluatorImp(SDKReadinessGates gates,
32-
SplitCache splitCache) {
33-
_gates = checkNotNull(gates);
28+
public EvaluatorImp(SplitCache splitCache) {
3429
_splitCache = checkNotNull(splitCache);
3530
}
3631

@@ -40,11 +35,6 @@ public TreatmentLabelAndChangeNumber evaluateFeature(String matchingKey, String
4035
ParsedSplit parsedSplit = _splitCache.get(split);
4136

4237
if (parsedSplit == null) {
43-
if (_gates.isSDKReadyNow()) {
44-
_log.warn(
45-
"getTreatment: you passed \"" + split + "\" that does not exist in this environment, " +
46-
"please double check what Splits exist in the web console.");
47-
}
4838
return new TreatmentLabelAndChangeNumber(Treatments.CONTROL, DEFINITION_NOT_FOUND);
4939
}
5040

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void close() {
8383
_splitCache.get().clear();
8484
}
8585

86-
_running.set(false);
86+
stop();
8787

8888
ScheduledExecutorService scheduledExecutorService = _executorService.get();
8989
if (scheduledExecutorService.isShutdown()) {

‎client/src/test/java/io/split/client/SplitClientImplTest.java‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void null_key_results_in_control() {
8383
NoopEventClient.create(),
8484
config,
8585
gates,
86-
new EvaluatorImp(gates, splitCache)
86+
new EvaluatorImp(splitCache)
8787
);
8888

8989
assertThat(client.getTreatment(null, "test1"), is(equalTo(Treatments.CONTROL)));
@@ -110,7 +110,7 @@ public void null_test_results_in_control() {
110110
NoopEventClient.create(),
111111
config,
112112
gates,
113-
new EvaluatorImp(gates, splitCache)
113+
new EvaluatorImp(splitCache)
114114
);
115115

116116
assertThat(client.getTreatment("adil@relateiq.com", null), is(equalTo(Treatments.CONTROL)));
@@ -132,7 +132,7 @@ public void exceptions_result_in_control() {
132132
NoopEventClient.create(),
133133
config,
134134
gates,
135-
new EvaluatorImp(gates, splitCache)
135+
new EvaluatorImp(splitCache)
136136
);
137137
assertThat(client.getTreatment("adil@relateiq.com", "test1"), is(equalTo(Treatments.CONTROL)));
138138

@@ -159,7 +159,7 @@ public void works() {
159159
NoopEventClient.create(),
160160
config,
161161
gates,
162-
new EvaluatorImp(gates, splitCache)
162+
new EvaluatorImp(splitCache)
163163
);
164164

165165
int numKeys = 5;
@@ -194,7 +194,7 @@ public void works_null_config() {
194194
NoopEventClient.create(),
195195
config,
196196
gates,
197-
new EvaluatorImp(gates, splitCache)
197+
new EvaluatorImp(splitCache)
198198
);
199199

200200

@@ -231,7 +231,7 @@ public void worksAndHasConfig() {
231231
NoopEventClient.create(),
232232
config,
233233
gates,
234-
new EvaluatorImp(gates, splitCache)
234+
new EvaluatorImp(splitCache)
235235
);
236236

237237
int numKeys = 5;
@@ -265,7 +265,7 @@ public void last_condition_is_always_default() {
265265
NoopEventClient.create(),
266266
config,
267267
gates,
268-
new EvaluatorImp(gates, splitCache)
268+
new EvaluatorImp(splitCache)
269269
);
270270

271271
assertThat(client.getTreatment("pato@codigo.com", test), is(equalTo(Treatments.OFF)));
@@ -301,7 +301,7 @@ public void last_condition_is_always_default_but_with_treatment() {
301301
NoopEventClient.create(),
302302
config,
303303
gates,
304-
new EvaluatorImp(gates, splitCache)
304+
new EvaluatorImp(splitCache)
305305
);
306306

307307
SplitResult result = client.getTreatmentWithConfig("pato@codigo.com", test);
@@ -334,7 +334,7 @@ public void multiple_conditions_work() {
334334
NoopEventClient.create(),
335335
config,
336336
gates,
337-
new EvaluatorImp(gates, splitCache)
337+
new EvaluatorImp(splitCache)
338338
);
339339

340340
assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("on")));
@@ -365,7 +365,7 @@ public void killed_test_always_goes_to_default() {
365365
NoopEventClient.create(),
366366
config,
367367
gates,
368-
new EvaluatorImp(gates, splitCache)
368+
new EvaluatorImp(splitCache)
369369
);
370370

371371
assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo(Treatments.OFF)));
@@ -401,7 +401,7 @@ public void killed_test_always_goes_to_default_has_config() {
401401
NoopEventClient.create(),
402402
config,
403403
gates,
404-
new EvaluatorImp(gates, splitCache)
404+
new EvaluatorImp(splitCache)
405405
);
406406

407407
SplitResult result = client.getTreatmentWithConfig("adil@codigo.com", test);
@@ -437,7 +437,7 @@ public void dependency_matcher_on() {
437437
NoopEventClient.create(),
438438
config,
439439
gates,
440-
new EvaluatorImp(gates, splitCache)
440+
new EvaluatorImp(splitCache)
441441
);
442442

443443
assertThat(client.getTreatment("key", parent), is(equalTo(Treatments.ON)));
@@ -470,7 +470,7 @@ public void dependency_matcher_off() {
470470
NoopEventClient.create(),
471471
config,
472472
gates,
473-
new EvaluatorImp(gates, splitCache)
473+
new EvaluatorImp(splitCache)
474474
);
475475

476476
assertThat(client.getTreatment("key", parent), is(equalTo(Treatments.ON)));
@@ -497,7 +497,7 @@ public void dependency_matcher_control() {
497497
NoopEventClient.create(),
498498
config,
499499
gates,
500-
new EvaluatorImp(gates, splitCache)
500+
new EvaluatorImp(splitCache)
501501
);
502502

503503
assertThat(client.getTreatment("key", dependent), is(equalTo(Treatments.ON)));
@@ -525,7 +525,7 @@ public void attributes_work() {
525525
NoopEventClient.create(),
526526
config,
527527
gates,
528-
new EvaluatorImp(gates, splitCache)
528+
new EvaluatorImp(splitCache)
529529
);
530530

531531
assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("on")));
@@ -559,7 +559,7 @@ public void attributes_work_2() {
559559
NoopEventClient.create(),
560560
config,
561561
gates,
562-
new EvaluatorImp(gates, splitCache)
562+
new EvaluatorImp(splitCache)
563563
);
564564

565565
assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off")));
@@ -593,7 +593,7 @@ public void attributes_greater_than_negative_number() {
593593
NoopEventClient.create(),
594594
config,
595595
gates,
596-
new EvaluatorImp(gates, splitCache)
596+
new EvaluatorImp(splitCache)
597597
);
598598

599599
assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off")));
@@ -630,7 +630,7 @@ public void attributes_for_sets() {
630630
NoopEventClient.create(),
631631
config,
632632
gates,
633-
new EvaluatorImp(gates, splitCache)
633+
new EvaluatorImp(splitCache)
634634
);
635635

636636
assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off")));
@@ -673,7 +673,7 @@ public void labels_are_populated() {
673673
NoopEventClient.create(),
674674
config,
675675
gates,
676-
new EvaluatorImp(gates, splitCache)
676+
new EvaluatorImp(splitCache)
677677
);
678678

679679
Map<String, Object> attributes = ImmutableMap.<String, Object>of("age", -20, "acv", "1000000");
@@ -765,7 +765,7 @@ private void traffic_allocation(String key, int trafficAllocation, int trafficAl
765765
NoopEventClient.create(),
766766
config,
767767
gates,
768-
new EvaluatorImp(gates, splitCache)
768+
new EvaluatorImp(splitCache)
769769
);
770770

771771
assertThat(client.getTreatment(key, test), is(equalTo(expected_treatment_on_or_off)));
@@ -813,7 +813,7 @@ public void notInTrafficAllocationDefaultConfig() {
813813
NoopEventClient.create(),
814814
config,
815815
gates,
816-
new EvaluatorImp(gates, splitCache)
816+
new EvaluatorImp(splitCache)
817817
);
818818

819819
assertThat(client.getTreatment("pato@split.io", test), is(equalTo(Treatments.OFF)));
@@ -853,7 +853,7 @@ public void matching_bucketing_keys_work() {
853853
NoopEventClient.create(),
854854
config,
855855
gates,
856-
new EvaluatorImp(gates, splitCache)
856+
new EvaluatorImp(splitCache)
857857
);
858858

859859
Key bad_key = new Key("adil", "aijaz");
@@ -891,7 +891,7 @@ public void impression_metadata_is_propagated() {
891891
NoopEventClient.create(),
892892
config,
893893
gates,
894-
new EvaluatorImp(gates, splitCache)
894+
new EvaluatorImp(splitCache)
895895
);
896896

897897
Map<String, Object> attributes = ImmutableMap.<String, Object>of("age", -20, "acv", "1000000");
@@ -927,7 +927,7 @@ public void block_until_ready_does_not_time_when_sdk_is_ready() throws TimeoutEx
927927
NoopEventClient.create(),
928928
config,
929929
ready,
930-
new EvaluatorImp(ready, splitCache)
930+
new EvaluatorImp(splitCache)
931931
);
932932

933933
client.blockUntilReady();
@@ -947,7 +947,7 @@ public void block_until_ready_times_when_sdk_is_not_ready() throws TimeoutExcept
947947
NoopEventClient.create(),
948948
config,
949949
ready,
950-
new EvaluatorImp(ready, splitCache)
950+
new EvaluatorImp(splitCache)
951951
);
952952

953953
client.blockUntilReady();
@@ -966,7 +966,7 @@ public void track_with_valid_parameters() {
966966
NoopEventClient.create(),
967967
config,
968968
gates,
969-
new EvaluatorImp(gates, splitCache)
969+
new EvaluatorImp(splitCache)
970970
);
971971

972972
Assert.assertThat(client.track("validKey", "valid_traffic_type", "valid_event"),
@@ -992,7 +992,7 @@ public void track_with_invalid_event_type_ids() {
992992
NoopEventClient.create(),
993993
config,
994994
gates,
995-
new EvaluatorImp(gates, splitCache)
995+
new EvaluatorImp(splitCache)
996996
);
997997

998998
Assert.assertThat(client.track("validKey", "valid_traffic_type", ""),
@@ -1023,7 +1023,7 @@ public void track_with_invalid_traffic_type_names() {
10231023
NoopEventClient.create(),
10241024
config,
10251025
gates,
1026-
new EvaluatorImp(gates, splitCache)
1026+
new EvaluatorImp(splitCache)
10271027
);
10281028

10291029
Assert.assertThat(client.track("validKey", "", "valid"),
@@ -1046,7 +1046,7 @@ public void track_with_invalid_keys() {
10461046
NoopEventClient.create(),
10471047
config,
10481048
gates,
1049-
new EvaluatorImp(gates, splitCache)
1049+
new EvaluatorImp(splitCache)
10501050
);
10511051

10521052
Assert.assertThat(client.track("", "valid_traffic_type", "valid"),
@@ -1075,7 +1075,7 @@ public void track_with_properties() {
10751075
eventClientMock,
10761076
config,
10771077
gates,
1078-
new EvaluatorImp(gates, splitCache)
1078+
new EvaluatorImp(splitCache)
10791079
);
10801080

10811081
HashMap<String, Object> properties = new HashMap<>();
@@ -1186,7 +1186,7 @@ public void getTreatment_with_invalid_keys() {
11861186
NoopEventClient.create(),
11871187
config,
11881188
gates,
1189-
new EvaluatorImp(gates, splitCache)
1189+
new EvaluatorImp(splitCache)
11901190
);
11911191

11921192
Assert.assertThat(client.getTreatment("valid", "split"),
@@ -1271,7 +1271,7 @@ public void client_cannot_perform_actions_when_destroyed() throws InterruptedExc
12711271
NoopEventClient.create(),
12721272
config,
12731273
gates,
1274-
new EvaluatorImp(gates, splitCache)
1274+
new EvaluatorImp(splitCache)
12751275
);
12761276

12771277
Assert.assertThat(client.getTreatment("valid", "split"),

‎client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class EvaluatorTest {
3939
public void before() {
4040
SDKReadinessGates gates = Mockito.mock(SDKReadinessGates.class);
4141
_splitCache = Mockito.mock(SplitCache.class);
42-
_evaluator = new EvaluatorImp(gates, _splitCache);
42+
_evaluator = new EvaluatorImp(_splitCache);
4343
_matcher = Mockito.mock(CombiningMatcher.class);
4444

4545
_configurations = new HashMap<>();

0 commit comments

Comments
 (0)