Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
gthea committed Dec 11, 2024
1 parent a5015f7 commit 2656eb5
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 21 deletions.
16 changes: 11 additions & 5 deletions src/main/java/io/split/android/client/EvaluationResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,22 @@ public final class EvaluationResult {
private final String mLabel;
private final Long mChangeNumber;
private final String mConfigurations;
private final boolean mTrackImpression;

public EvaluationResult(String treatment, String label) {
this(treatment, label, null);
public EvaluationResult(String treatment, String label, boolean trackImpression) {
this(treatment, label, null, null, trackImpression);
}

EvaluationResult(String treatment, String label, Long changeNumber) {
this(treatment, label, changeNumber, null);
EvaluationResult(String treatment, String label, Long changeNumber, boolean trackImpression) {
this(treatment, label, changeNumber, null, trackImpression);
}

public EvaluationResult(String treatment, String label, Long changeNumber, String configurations) {
public EvaluationResult(String treatment, String label, Long changeNumber, String configurations, boolean trackImpression) {
mTreatment = treatment;
mLabel = label;
mChangeNumber = changeNumber;
mConfigurations = configurations;
mTrackImpression = trackImpression;
}

public String getTreatment() {
Expand All @@ -36,4 +38,8 @@ public Long getChangeNumber() {
public String getConfigurations() {
return mConfigurations;
}

public boolean getTrackImpression() {
return mTrackImpression;
}
}
14 changes: 7 additions & 7 deletions src/main/java/io/split/android/client/EvaluatorImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ public EvaluationResult getTreatment(String matchingKey, String bucketingKey, St
try {
ParsedSplit parsedSplit = mSplitParser.parse(mSplitsStorage.get(splitName), matchingKey);
if (parsedSplit == null) {
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.DEFINITION_NOT_FOUND);
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.DEFINITION_NOT_FOUND, true);
}

return getTreatment(matchingKey, bucketingKey, parsedSplit, attributes);
} catch (ChangeNumberExceptionWrapper ex) {
Logger.e(ex, "Catch Change Number Exception");
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, ex.changeNumber());
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, ex.changeNumber(), true);
} catch (Exception e) {
Logger.e(e, "Catch All Exception");
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION);
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, true);
}
}

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

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

if (bucket > parsedSplit.trafficAllocation()) {
// out of split
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.NOT_IN_SPLIT, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()));
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.NOT_IN_SPLIT, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpression());
}

}
Expand All @@ -84,11 +84,11 @@ private EvaluationResult getTreatment(String matchingKey, String bucketingKey, P

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

return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.DEFAULT_RULE, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()));
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.DEFAULT_RULE, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpression());
} catch (Exception e) {
throw new ChangeNumberExceptionWrapper(e, parsedSplit.changeNumber());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.split.android.client.impressions;

import java.util.Map;

public class DecoratedImpression extends Impression {

private final boolean mTrackImpression;

public DecoratedImpression(String key, String bucketingKey, String split, String treatment, long time, String appliedRule, Long changeNumber, Map<String, Object> atributes, boolean trackImpression) {
super(key, bucketingKey, split, treatment, time, appliedRule, changeNumber, atributes);
mTrackImpression = trackImpression;
}

public boolean getTrackImpression() {
return mTrackImpression;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;

import io.split.android.client.impressions.DecoratedImpression;
import io.split.android.client.impressions.Impression;
import io.split.android.client.service.impressions.strategy.PeriodicTracker;
import io.split.android.client.service.impressions.strategy.ProcessStrategy;
Expand Down Expand Up @@ -39,7 +40,7 @@ public void pushImpression(Impression impression) {
return;
}

if (track(impression)) {
if (shouldTrack(impression)) {
mProcessStrategy.apply(impression);
} else {
mNoneStrategy.apply(impression);
Expand Down Expand Up @@ -72,7 +73,12 @@ public void stopPeriodicRecording() {
}
}

private static boolean track(Impression impression) {
return true; // TODO: Placeholder method
private static boolean shouldTrack(Impression impression) {
if (impression instanceof DecoratedImpression) {
return ((DecoratedImpression) impression).getTrackImpression();
} else {
// default behaviour; will never get here.
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import io.split.android.client.attributes.AttributesMerger;
import io.split.android.client.events.ListenableEventsManager;
import io.split.android.client.events.SplitEvent;
import io.split.android.client.impressions.Impression;
import io.split.android.client.impressions.DecoratedImpression;
import io.split.android.client.impressions.ImpressionListener;
import io.split.android.client.storage.splits.SplitsStorage;
import io.split.android.client.telemetry.model.Method;
Expand Down Expand Up @@ -294,7 +294,8 @@ private TreatmentResult getTreatmentWithConfigWithoutMetrics(String split, Map<S
evaluationResult.getTreatment(),
mLabelsEnabled ? evaluationResult.getLabel() : null,
evaluationResult.getChangeNumber(),
mergedAttributes);
mergedAttributes,
evaluationResult.getTrackImpression());

return new TreatmentResult(splitResult, false);
} catch (Exception ex) {
Expand All @@ -307,16 +308,17 @@ private TreatmentResult getTreatmentWithConfigWithoutMetrics(String split, Map<S
Treatments.CONTROL,
TreatmentLabels.EXCEPTION,
(evaluationResult != null) ? evaluationResult.getChangeNumber() : null,
mergedAttributes);
mergedAttributes,
evaluationResult == null || evaluationResult.getTrackImpression());
}

return new TreatmentResult(new SplitResult(Treatments.CONTROL), true);
}
}

private void logImpression(String matchingKey, String bucketingKey, String splitName, String result, String label, Long changeNumber, Map<String, Object> attributes) {
private void logImpression(String matchingKey, String bucketingKey, String splitName, String result, String label, Long changeNumber, Map<String, Object> attributes, boolean trackImpression) {
try {
mImpressionListener.log(new Impression(matchingKey, bucketingKey, splitName, result, System.currentTimeMillis(), label, changeNumber, attributes));
mImpressionListener.log(new DecoratedImpression(matchingKey, bucketingKey, splitName, result, System.currentTimeMillis(), label, changeNumber, attributes, trackImpression));
} catch (Throwable t) {
Logger.e("An error occurred logging impression: " + t.getLocalizedMessage());
}
Expand All @@ -339,7 +341,7 @@ private EvaluationResult evaluateIfReady(String featureFlagName,
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);
mTelemetryStorageProducer.recordNonReadyUsage();

return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.NOT_READY, null, null);
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.NOT_READY, null, null, true);
}
return mEvaluator.getTreatment(mMatchingKey, mBucketingKey, featureFlagName, attributes);
}
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/io/split/android/client/TreatmentManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand All @@ -27,6 +28,7 @@
import io.split.android.client.dtos.Split;
import io.split.android.client.events.ListenableEventsManager;
import io.split.android.client.events.SplitEvent;
import io.split.android.client.impressions.DecoratedImpression;
import io.split.android.client.impressions.ImpressionListener;
import io.split.android.client.storage.mysegments.MySegmentsStorage;
import io.split.android.client.storage.mysegments.MySegmentsStorageContainer;
Expand Down Expand Up @@ -316,6 +318,20 @@ public void evaluationWhenNotReadyLogsCorrectMessage() {
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());
}

@Test
public void trackValueFromEvaluationResultGetsPassedInToImpression() {
Evaluator evaluatorMock = mock(Evaluator.class);
when(evaluatorMock.getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_split"), anyMap()))
.thenReturn(new EvaluationResult("test", "test"));
TreatmentManagerImpl tManager = initializeTreatmentManager(evaluatorMock);

tManager.getTreatment("test_split", null, false);

verify(impressionListener).log(argThat(argument -> {
return ((DecoratedImpression) argument).getTrackImpression();
}));
}

private void assertControl(List<String> splitList, String treatment, Map<String, String> treatmentList, SplitResult splitResult, Map<String, SplitResult> splitResultList) {
Assert.assertNotNull(treatment);
Assert.assertEquals(Treatments.CONTROL, treatment);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.split.android.client.service.impressions

import io.split.android.client.impressions.DecoratedImpression
import io.split.android.client.impressions.Impression
import io.split.android.client.service.impressions.strategy.PeriodicTracker
import io.split.android.client.service.impressions.strategy.ProcessStrategy
Expand All @@ -8,6 +9,7 @@ import org.junit.Test
import org.mockito.Mock
import org.mockito.Mockito.mock
import org.mockito.Mockito.verify
import org.mockito.Mockito.`when`
import org.mockito.MockitoAnnotations

class StrategyImpressionManagerTest {
Expand Down Expand Up @@ -60,4 +62,17 @@ class StrategyImpressionManagerTest {

verify(strategy).apply(impression)
}

@Test
fun `pushImpression when it is decorated uses value from trackImpression to track`() {
val impression = mock(DecoratedImpression::class.java)
val impression2 = mock(DecoratedImpression::class.java)
`when`(impression.trackImpression).thenReturn(false)
`when`(impression2.trackImpression).thenReturn(true)
impressionManager.pushImpression(impression)
impressionManager.pushImpression(impression2)

verify(strategy).apply(impression2)
verify(noneStrategy).apply(impression)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -495,6 +496,18 @@ public void inLargeSegmentMatcherParsingTest() {
verify(mMySegmentsStorageContainer, never()).getStorageForKey("matching_key");
}

@Test
public void trackImpressionsParsingTest(){
SplitParser parser = createParser();

Split split = makeSplit("splitName", Collections.emptyList());
split.trackImpressions = false;

ParsedSplit actual = parser.parse(split);

assertFalse(actual.trackImpression());
}

private void set_matcher_test(Condition c, io.split.android.engine.matchers.Matcher m) {

SplitParser parser = createParser();
Expand Down Expand Up @@ -536,6 +549,7 @@ private Split makeSplit(String name, List<Condition> conditions, long changeNumb
split.algo = 1;
split.configurations = configurations;
split.sets = Collections.emptySet();
split.trackImpressions = true;
return split;
}

Expand Down

0 comments on commit 2656eb5

Please sign in to comment.