Skip to content

Commit 23d0758

Browse files
gtheagthea
committed
Decorated impression & evaluation result changes
1 parent 3845f39 commit 23d0758

File tree

7 files changed

+128
-22
lines changed

7 files changed

+128
-22
lines changed

‎src/main/java/io/split/android/client/EvaluationResult.java

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,33 @@
11
package io.split.android.client;
22

3+
import androidx.annotation.VisibleForTesting;
4+
35
public final class EvaluationResult {
46
private final String mTreatment;
57
private final String mLabel;
68
private final Long mChangeNumber;
79
private final String mConfigurations;
10+
private final boolean mTrackImpression;
811

12+
@VisibleForTesting
913
public EvaluationResult(String treatment, String label) {
10-
this(treatment, label, null);
14+
this(treatment, label, null, null, true);
15+
}
16+
17+
public EvaluationResult(String treatment, String label, boolean trackImpression) {
18+
this(treatment, label, null, null, trackImpression);
1119
}
1220

13-
EvaluationResult(String treatment, String label, Long changeNumber) {
14-
this(treatment, label, changeNumber, null);
21+
EvaluationResult(String treatment, String label, Long changeNumber, boolean trackImpression) {
22+
this(treatment, label, changeNumber, null, trackImpression);
1523
}
1624

17-
public EvaluationResult(String treatment, String label, Long changeNumber, String configurations) {
25+
public EvaluationResult(String treatment, String label, Long changeNumber, String configurations, boolean trackImpression) {
1826
mTreatment = treatment;
1927
mLabel = label;
2028
mChangeNumber = changeNumber;
2129
mConfigurations = configurations;
30+
mTrackImpression = trackImpression;
2231
}
2332

2433
public String getTreatment() {
@@ -36,4 +45,8 @@ public Long getChangeNumber() {
3645
public String getConfigurations() {
3746
return mConfigurations;
3847
}
48+
49+
public boolean getTrackImpression() {
50+
return mTrackImpression;
51+
}
3952
}

‎src/main/java/io/split/android/client/EvaluatorImpl.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@ public EvaluationResult getTreatment(String matchingKey, String bucketingKey, St
2828
try {
2929
ParsedSplit parsedSplit = mSplitParser.parse(mSplitsStorage.get(splitName), matchingKey);
3030
if (parsedSplit == null) {
31-
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.DEFINITION_NOT_FOUND);
31+
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.DEFINITION_NOT_FOUND, true);
3232
}
3333

3434
return getTreatment(matchingKey, bucketingKey, parsedSplit, attributes);
3535
} catch (ChangeNumberExceptionWrapper ex) {
3636
Logger.e(ex, "Catch Change Number Exception");
37-
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, ex.changeNumber());
37+
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, ex.changeNumber(), true);
3838
} catch (Exception e) {
3939
Logger.e(e, "Catch All Exception");
40-
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION);
40+
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, true);
4141
}
4242
}
4343

@@ -52,7 +52,7 @@ public EvaluationResult getTreatment(String matchingKey, String bucketingKey, St
5252
private EvaluationResult getTreatment(String matchingKey, String bucketingKey, ParsedSplit parsedSplit, Map<String, Object> attributes) throws ChangeNumberExceptionWrapper {
5353
try {
5454
if (parsedSplit.killed()) {
55-
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.KILLED, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()));
55+
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.KILLED, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpression());
5656
}
5757

5858
/*
@@ -75,7 +75,7 @@ private EvaluationResult getTreatment(String matchingKey, String bucketingKey, P
7575

7676
if (bucket > parsedSplit.trafficAllocation()) {
7777
// out of split
78-
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.NOT_IN_SPLIT, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()));
78+
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.NOT_IN_SPLIT, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpression());
7979
}
8080

8181
}
@@ -84,11 +84,11 @@ private EvaluationResult getTreatment(String matchingKey, String bucketingKey, P
8484

8585
if (parsedCondition.matcher().match(matchingKey, bucketingKey, attributes, this)) {
8686
String treatment = Splitter.getTreatment(bk, parsedSplit.seed(), parsedCondition.partitions(), parsedSplit.algo());
87-
return new EvaluationResult(treatment, parsedCondition.label(), parsedSplit.changeNumber(), configForTreatment(parsedSplit, treatment));
87+
return new EvaluationResult(treatment, parsedCondition.label(), parsedSplit.changeNumber(), configForTreatment(parsedSplit, treatment), parsedSplit.trackImpression());
8888
}
8989
}
9090

91-
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.DEFAULT_RULE, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()));
91+
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.DEFAULT_RULE, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpression());
9292
} catch (Exception e) {
9393
throw new ChangeNumberExceptionWrapper(e, parsedSplit.changeNumber());
9494
}

‎src/main/java/io/split/android/client/impressions/DecoratedImpression.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.split.android.client.impressions;
2+
3+
import java.util.Map;
4+
5+
public class DecoratedImpression extends Impression {
6+
7+
private final boolean mTrackImpressions;
8+
9+
public DecoratedImpression(String key, String bucketingKey, String split, String treatment, long time, String appliedRule, Long changeNumber, Map<String, Object> atributes, boolean trackImpressions) {
10+
super(key, bucketingKey, split, treatment, time, appliedRule, changeNumber, atributes);
11+
mTrackImpressions = trackImpressions;
12+
}
13+
14+
public boolean getTrackImpressions() {
15+
return mTrackImpressions;
16+
}
17+
}

‎src/main/java/io/split/android/client/service/impressions/StrategyImpressionManager.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.Set;
99
import java.util.concurrent.atomic.AtomicBoolean;
1010

11+
import io.split.android.client.impressions.DecoratedImpression;
1112
import io.split.android.client.impressions.Impression;
1213
import io.split.android.client.service.impressions.strategy.PeriodicTracker;
1314
import io.split.android.client.service.impressions.strategy.ProcessStrategy;
@@ -39,7 +40,7 @@ public void pushImpression(Impression impression) {
3940
return;
4041
}
4142

42-
if (track(impression)) {
43+
if (shouldTrack(impression)) {
4344
mProcessStrategy.apply(impression);
4445
} else {
4546
mNoneStrategy.apply(impression);
@@ -72,7 +73,12 @@ public void stopPeriodicRecording() {
7273
}
7374
}
7475

75-
private static boolean track(Impression impression) {
76-
return true; // TODO: Placeholder method
76+
private static boolean shouldTrack(Impression impression) {
77+
if (impression instanceof DecoratedImpression) {
78+
return ((DecoratedImpression) impression).getTrackImpressions();
79+
} else {
80+
// default behaviour; will never get here.
81+
return true;
82+
}
7783
}
7884
}

‎src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import io.split.android.client.attributes.AttributesMerger;
2222
import io.split.android.client.events.ListenableEventsManager;
2323
import io.split.android.client.events.SplitEvent;
24-
import io.split.android.client.impressions.Impression;
24+
import io.split.android.client.impressions.DecoratedImpression;
2525
import io.split.android.client.impressions.ImpressionListener;
2626
import io.split.android.client.storage.splits.SplitsStorage;
2727
import io.split.android.client.telemetry.model.Method;
@@ -294,7 +294,8 @@ private TreatmentResult getTreatmentWithConfigWithoutMetrics(String split, Map<S
294294
evaluationResult.getTreatment(),
295295
mLabelsEnabled ? evaluationResult.getLabel() : null,
296296
evaluationResult.getChangeNumber(),
297-
mergedAttributes);
297+
mergedAttributes,
298+
evaluationResult.getTrackImpression());
298299

299300
return new TreatmentResult(splitResult, false);
300301
} catch (Exception ex) {
@@ -307,16 +308,17 @@ private TreatmentResult getTreatmentWithConfigWithoutMetrics(String split, Map<S
307308
Treatments.CONTROL,
308309
TreatmentLabels.EXCEPTION,
309310
(evaluationResult != null) ? evaluationResult.getChangeNumber() : null,
310-
mergedAttributes);
311+
mergedAttributes,
312+
evaluationResult == null || evaluationResult.getTrackImpression());
311313
}
312314

313315
return new TreatmentResult(new SplitResult(Treatments.CONTROL), true);
314316
}
315317
}
316318

317-
private void logImpression(String matchingKey, String bucketingKey, String splitName, String result, String label, Long changeNumber, Map<String, Object> attributes) {
319+
private void logImpression(String matchingKey, String bucketingKey, String splitName, String result, String label, Long changeNumber, Map<String, Object> attributes, boolean trackImpression) {
318320
try {
319-
mImpressionListener.log(new Impression(matchingKey, bucketingKey, splitName, result, System.currentTimeMillis(), label, changeNumber, attributes));
321+
mImpressionListener.log(new DecoratedImpression(matchingKey, bucketingKey, splitName, result, System.currentTimeMillis(), label, changeNumber, attributes, trackImpression));
320322
} catch (Throwable t) {
321323
Logger.e("An error occurred logging impression: " + t.getLocalizedMessage());
322324
}
@@ -339,7 +341,7 @@ private EvaluationResult evaluateIfReady(String featureFlagName,
339341
mValidationLogger.w("the SDK is not ready, results may be incorrect for feature flag " + featureFlagName + ". Make sure to wait for SDK readiness before using this method", validationTag);
340342
mTelemetryStorageProducer.recordNonReadyUsage();
341343

342-
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.NOT_READY, null, null);
344+
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.NOT_READY, null, null, true);
343345
}
344346
return mEvaluator.getTreatment(mMatchingKey, mBucketingKey, featureFlagName, attributes);
345347
}

‎src/test/java/io/split/android/client/TreatmentManagerTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static org.mockito.ArgumentMatchers.any;
44
import static org.mockito.ArgumentMatchers.anyMap;
5+
import static org.mockito.ArgumentMatchers.argThat;
56
import static org.mockito.ArgumentMatchers.eq;
67
import static org.mockito.Mockito.mock;
78
import static org.mockito.Mockito.verify;
@@ -27,6 +28,7 @@
2728
import io.split.android.client.dtos.Split;
2829
import io.split.android.client.events.ListenableEventsManager;
2930
import io.split.android.client.events.SplitEvent;
31+
import io.split.android.client.impressions.DecoratedImpression;
3032
import io.split.android.client.impressions.ImpressionListener;
3133
import io.split.android.client.storage.mysegments.MySegmentsStorage;
3234
import io.split.android.client.storage.mysegments.MySegmentsStorageContainer;
@@ -316,6 +318,20 @@ public void evaluationWhenNotReadyLogsCorrectMessage() {
316318
verify(validationMessageLogger).w(eq("the SDK is not ready, results may be incorrect for feature flag test_split. Make sure to wait for SDK readiness before using this method"), any());
317319
}
318320

321+
@Test
322+
public void trackValueFromEvaluationResultGetsPassedInToImpression() {
323+
Evaluator evaluatorMock = mock(Evaluator.class);
324+
when(evaluatorMock.getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_split"), anyMap()))
325+
.thenReturn(new EvaluationResult("test", "test"));
326+
TreatmentManagerImpl tManager = initializeTreatmentManager(evaluatorMock);
327+
328+
tManager.getTreatment("test_split", null, false);
329+
330+
verify(impressionListener).log(argThat(argument -> {
331+
return ((DecoratedImpression) argument).getTrackImpressions();
332+
}));
333+
}
334+
319335
private void assertControl(List<String> splitList, String treatment, Map<String, String> treatmentList, SplitResult splitResult, Map<String, SplitResult> splitResultList) {
320336
Assert.assertNotNull(treatment);
321337
Assert.assertEquals(Treatments.CONTROL, treatment);

‎src/test/java/io/split/android/client/service/impressions/StrategyImpressionManagerTest.kt

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package io.split.android.client.service.impressions
22

3-
import io.split.android.client.impressions.Impression
3+
import io.split.android.client.impressions.DecoratedImpression
44
import io.split.android.client.service.impressions.strategy.PeriodicTracker
55
import io.split.android.client.service.impressions.strategy.ProcessStrategy
66
import org.junit.Before
77
import org.junit.Test
88
import org.mockito.Mock
99
import org.mockito.Mockito.mock
1010
import org.mockito.Mockito.verify
11+
import org.mockito.Mockito.verifyNoInteractions
12+
import org.mockito.Mockito.`when`
1113
import org.mockito.MockitoAnnotations
1214

1315
class StrategyImpressionManagerTest {
@@ -37,27 +39,77 @@ class StrategyImpressionManagerTest {
3739
impressionManager.flush()
3840

3941
verify(tracker).flush()
42+
verify(noneTracker).flush()
4043
}
4144

4245
@Test
4346
fun `startPeriodicRecording calls startPeriodicRecording on tracker`() {
4447
impressionManager.startPeriodicRecording()
4548

4649
verify(tracker).startPeriodicRecording()
50+
verify(noneTracker).startPeriodicRecording()
4751
}
4852

4953
@Test
5054
fun `stopPeriodicRecording() calls stopPeriodicRecording() on tracker`() {
5155
impressionManager.stopPeriodicRecording()
5256

5357
verify(tracker).stopPeriodicRecording()
58+
verify(noneTracker).stopPeriodicRecording()
5459
}
5560

5661
@Test
5762
fun `pushImpression calls apply on strategy`() {
58-
val impression = mock(Impression::class.java)
63+
val impression = mock(DecoratedImpression::class.java)
64+
`when`(impression.trackImpressions).thenReturn(true)
5965
impressionManager.pushImpression(impression)
6066

6167
verify(strategy).apply(impression)
68+
verifyNoInteractions(noneStrategy)
69+
}
70+
71+
@Test
72+
fun `pushImpression calls apply on noneStrategy when trackImpressions is false`() {
73+
val impression = mock(DecoratedImpression::class.java)
74+
`when`(impression.trackImpressions).thenReturn(false)
75+
impressionManager.pushImpression(impression)
76+
77+
verify(noneStrategy).apply(impression)
78+
verifyNoInteractions(strategy)
79+
}
80+
81+
@Test
82+
fun `pushImpression when it is decorated uses value from trackImpression to track`() {
83+
val impression = mock(DecoratedImpression::class.java)
84+
val impression2 = mock(DecoratedImpression::class.java)
85+
`when`(impression.trackImpressions).thenReturn(false)
86+
`when`(impression2.trackImpressions).thenReturn(true)
87+
impressionManager.pushImpression(impression)
88+
impressionManager.pushImpression(impression2)
89+
90+
verify(strategy).apply(impression2)
91+
verify(noneStrategy).apply(impression)
92+
}
93+
94+
@Test
95+
fun `enableTracking set to true causes impressions to be sent to strategy`() {
96+
impressionManager.enableTracking(true)
97+
val impression = mock(DecoratedImpression::class.java)
98+
`when`(impression.trackImpressions).thenReturn(true)
99+
impressionManager.pushImpression(impression)
100+
101+
verify(strategy).apply(impression)
102+
verifyNoInteractions(noneStrategy)
103+
}
104+
105+
@Test
106+
fun `enableTracking set to false causes impressions to not be tracked`() {
107+
impressionManager.enableTracking(false)
108+
val impression = mock(DecoratedImpression::class.java)
109+
`when`(impression.trackImpressions).thenReturn(true)
110+
impressionManager.pushImpression(impression)
111+
112+
verifyNoInteractions(noneStrategy)
113+
verifyNoInteractions(strategy)
62114
}
63115
}

0 commit comments

Comments
 (0)