Skip to content

Commit

Permalink
Replace trackImpressions property (#728)
Browse files Browse the repository at this point in the history
  • Loading branch information
gthea authored Jan 7, 2025
1 parent 662cd83 commit 20c8ece
Show file tree
Hide file tree
Showing 21 changed files with 90 additions and 83 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ apply plugin: 'kotlin-android'
apply from: 'spec.gradle'

ext {
splitVersion = '5.1.0-alpha.2'
splitVersion = '5.1.0-alpha.3'
}

android {
Expand Down
4 changes: 2 additions & 2 deletions src/androidTest/assets/split_changes_imp_toggle.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"defaultTreatment": "off",
"changeNumber": 1506703262916,
"algo": 2,
"trackImpressions": true,
"impressionsDisabled": false,
"conditions": [
{
"conditionType": "ROLLOUT",
Expand Down Expand Up @@ -70,7 +70,7 @@
"defaultTreatment": "off",
"changeNumber": 1506703262916,
"algo": 2,
"trackImpressions": false,
"impressionsDisabled": true,
"conditions": [
{
"conditionType": "ROLLOUT",
Expand Down
9 changes: 7 additions & 2 deletions src/androidTest/java/fake/HttpClientMock.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@
import io.split.android.client.network.HttpStreamRequest;
import io.split.android.client.network.HttpStreamResponse;

import static java.lang.Thread.sleep;

/**
* Prefer using {@link okhttp3.mockwebserver.MockWebServer} to mock / intercept responses.
* <p>
* That will ensure the SDK uses the default HTTP client.
*/
@Deprecated
public class HttpClientMock implements HttpClient {
HttpResponseMockDispatcher mResponseDispatcher;

public HttpClientMock(HttpResponseMockDispatcher responseDispatcher) throws IOException {
mResponseDispatcher = responseDispatcher;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void onPostExecutionView(SplitClient client) {

assertTrue(latch.await(5, TimeUnit.SECONDS));
mLifecycleManager.simulateOnPause();
Thread.sleep(200);
Thread.sleep(500);
mLifecycleManager.simulateOnResume();
Thread.sleep(500);

Expand Down Expand Up @@ -327,11 +327,11 @@ private SplitFactory createFactory(
SplitClientConfig config = new TestableSplitConfigBuilder().ready(30000)
.trafficType("client")
.impressionsMode(impressionsMode)
.impressionsRefreshRate(1000)
.impressionsCountersRefreshRate(1000)
.impressionsRefreshRate(99999)
.impressionsCountersRefreshRate(99999)
.streamingEnabled(false)
.enableDebug()
.eventFlushInterval(1000)
.eventFlushInterval(99999)
.encryptionEnabled(encryptionEnabled)
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ public void managerContainsProperty() throws InterruptedException {
SplitManager manager = splitFactory.manager();
List<SplitView> splits = manager.splits();

assertTrue(manager.split("tracked").trackImpressions);
assertFalse(manager.split("not_tracked").trackImpressions);
assertFalse(manager.split("tracked").impressionsDisabled);
assertTrue(manager.split("not_tracked").impressionsDisabled);
assertEquals(2, splits.size());
}

Expand All @@ -94,7 +94,7 @@ public void testNoneMode() throws InterruptedException {
// 1. Initialize SDK in impressions NONE mode
SplitFactory splitFactory = getReadyFactory(ImpressionsMode.NONE);

// 2. Fetch splitChanges with both flags with trackImpressions true & false
// 2. Fetch splitChanges with both flags with impressionsDisabled true & false
SplitClient client = splitFactory.client();
client.getTreatment("tracked");
client.getTreatment("not_tracked");
Expand All @@ -117,7 +117,7 @@ public void testDebugMode() throws InterruptedException {
// 1. Initialize SDK in impressions DEBUG mode
SplitFactory splitFactory = getReadyFactory(ImpressionsMode.DEBUG);

// 2. Fetch splitChanges with both flags with trackImpressions true & false
// 2. Fetch splitChanges with both flags with impressionsDisabled true & false
SplitClient client = splitFactory.client();
client.getTreatment("tracked");
client.getTreatment("not_tracked");
Expand All @@ -140,7 +140,7 @@ public void testOptimizedMode() throws InterruptedException {
// 1. Initialize SDK in impressions OPTIMIZED mode
SplitFactory splitFactory = getReadyFactory(ImpressionsMode.OPTIMIZED);

// 2. Fetch splitChanges with both flags with trackImpressions true & false
// 2. Fetch splitChanges with both flags with impressionsDisabled true & false
SplitClient client = splitFactory.client();
client.getTreatment("tracked");
client.getTreatment("not_tracked");
Expand Down
20 changes: 10 additions & 10 deletions src/main/java/io/split/android/client/EvaluationResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,27 @@ public final class EvaluationResult {
private final String mLabel;
private final Long mChangeNumber;
private final String mConfigurations;
private final boolean mTrackImpression;
private final boolean mImpressionsDisabled;

@VisibleForTesting
public EvaluationResult(String treatment, String label) {
this(treatment, label, null, null, true);
this(treatment, label, null, null, false);
}

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

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

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

public String getTreatment() {
Expand All @@ -46,7 +46,7 @@ public String getConfigurations() {
return mConfigurations;
}

public boolean getTrackImpression() {
return mTrackImpression;
public boolean isImpressionsDisabled() {
return mImpressionsDisabled;
}
}
8 changes: 4 additions & 4 deletions src/main/java/io/split/android/client/EvaluatorImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()), parsedSplit.trackImpressions());
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.KILLED, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.impressionsDisabled());
}

/*
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()), parsedSplit.trackImpressions());
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.NOT_IN_SPLIT, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.impressionsDisabled());
}

}
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), parsedSplit.trackImpressions());
return new EvaluationResult(treatment, parsedCondition.label(), parsedSplit.changeNumber(), configForTreatment(parsedSplit, treatment), parsedSplit.impressionsDisabled());
}
}

return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.DEFAULT_RULE, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpressions());
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.DEFAULT_RULE, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.impressionsDisabled());
} catch (Exception e) {
throw new ChangeNumberExceptionWrapper(e, parsedSplit.changeNumber());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ private SplitView toSplitView(ParsedSplit parsedSplit) {
splitView.configs = parsedSplit.configurations();
splitView.sets = new ArrayList<>(parsedSplit.sets() == null ? new HashSet<>() : parsedSplit.sets());
splitView.defaultTreatment = parsedSplit.defaultTreatment();
splitView.trackImpressions = parsedSplit.trackImpressions();
splitView.impressionsDisabled = parsedSplit.impressionsDisabled();

Set<String> treatments = new HashSet<>();
for (ParsedCondition condition : parsedSplit.parsedConditions()) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/split/android/client/api/SplitView.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ public class SplitView {
@NonNull
public List<String> sets = new ArrayList<>();
public String defaultTreatment;
public boolean trackImpressions;
public boolean impressionsDisabled;
}
4 changes: 2 additions & 2 deletions src/main/java/io/split/android/client/dtos/Split.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@ public class Split {
@SerializedName("sets")
public Set<String> sets;

@SerializedName("trackImpressions")
public boolean trackImpressions = true;
@SerializedName("impressionsDisabled")
public boolean impressionsDisabled = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@
public class DecoratedImpression {

private final Impression mImpression;
private final boolean mTrackImpressions;
private final boolean mDisabled;

public DecoratedImpression(Impression impression, boolean trackImpressions) {
public DecoratedImpression(Impression impression, boolean disabled) {
mImpression = impression;
mTrackImpressions = trackImpressions;
mDisabled = disabled;
}

public Impression getImpression() {
return mImpression;
}

public boolean getTrackImpressions() {
return mTrackImpressions;
public boolean isImpressionsDisabled() {
return mDisabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ public void pushImpression(DecoratedImpression impression) {
return;
}

if (impression.getTrackImpressions()) {
mProcessStrategy.apply(impression.getImpression());
} else {
if (impression.isImpressionsDisabled()) {
mNoneStrategy.apply(impression.getImpression());
} else {
mProcessStrategy.apply(impression.getImpression());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ private TreatmentResult getTreatmentWithConfigWithoutMetrics(String split, Map<S
mLabelsEnabled ? evaluationResult.getLabel() : null,
evaluationResult.getChangeNumber(),
mergedAttributes,
evaluationResult.getTrackImpression());
evaluationResult.isImpressionsDisabled());

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

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, boolean trackImpression) {
private void logImpression(String matchingKey, String bucketingKey, String splitName, String result, String label, Long changeNumber, Map<String, Object> attributes, boolean impressionsDisabled) {
try {
Impression impression = new Impression(matchingKey, bucketingKey, splitName, result, System.currentTimeMillis(), label, changeNumber, attributes);
DecoratedImpression decoratedImpression = new DecoratedImpression(impression, trackImpression);
DecoratedImpression decoratedImpression = new DecoratedImpression(impression, impressionsDisabled);
mImpressionListener.log(decoratedImpression);
mImpressionListener.log(impression);
} catch (Throwable t) {
Expand All @@ -345,7 +345,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, true);
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.NOT_READY, null, null, false);
}
return mEvaluator.getTreatment(mMatchingKey, mBucketingKey, featureFlagName, attributes);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class ParsedSplit {
private final int mAlgo;
private final Map<String, String> mConfigurations;
private final Set<String> mSets;
private final boolean mTrackImpressions;
private final boolean mImpressionsDisabled;

public ParsedSplit(
String feature,
Expand All @@ -38,7 +38,7 @@ public ParsedSplit(
int algo,
Map<String, String> configurations,
Set<String> sets,
boolean trackImpressions
boolean impressionsDisabled
) {
mSplit = feature;
mSeed = seed;
Expand All @@ -49,7 +49,7 @@ public ParsedSplit(
mChangeNumber = changeNumber;
mAlgo = algo;
mConfigurations = configurations;
mTrackImpressions = trackImpressions;
mImpressionsDisabled = impressionsDisabled;

if (mDefaultTreatment == null) {
throw new IllegalArgumentException("DefaultTreatment is null");
Expand Down Expand Up @@ -107,8 +107,8 @@ public Set<String> sets() {
return mSets;
}

public boolean trackImpressions() {
return mTrackImpressions;
public boolean impressionsDisabled() {
return mImpressionsDisabled;
}

@Override
Expand All @@ -123,7 +123,7 @@ public int hashCode() {
result = 31 * result + (int) (mChangeNumber ^ (mChangeNumber >>> 32));
result = 31 * result + (mAlgo ^ (mAlgo >>> 32));
result = 31 * result + ((mSets != null) ? mSets.hashCode() : 0);
result = 31 * result + (mTrackImpressions ? 1 : 0);
result = 31 * result + (mImpressionsDisabled ? 1 : 0);
return result;
}

Expand All @@ -144,7 +144,7 @@ public boolean equals(Object obj) {
&& mAlgo == other.mAlgo
&& (Objects.equals(mConfigurations, other.mConfigurations))
&& (Objects.equals(mSets, other.mSets)
&& mTrackImpressions == other.mTrackImpressions);
&& mImpressionsDisabled == other.mImpressionsDisabled);

}

Expand All @@ -155,7 +155,7 @@ public String toString() {
", default treatment:" + mDefaultTreatment +
", parsedConditions:" + mParsedCondition +
", trafficTypeName:" + mTrafficTypeName + ", changeNumber:" + mChangeNumber +
", algo:" + mAlgo + ", config:" + mConfigurations + ", sets:" + mSets + ", trackImpressions:" + mTrackImpressions;
", algo:" + mAlgo + ", config:" + mConfigurations + ", sets:" + mSets + ", impressionsDisabled:" + mImpressionsDisabled;

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ private ParsedSplit parseWithoutExceptionHandling(Split split, String matchingKe
split.algo,
split.configurations,
split.sets,
split.trackImpressions);
split.impressionsDisabled);
}

private CombiningMatcher toMatcher(MatcherGroup matcherGroup, String matchingKey) throws UnsupportedMatcherException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,16 @@ public void defaultTreatmentIsPresentWhenFetchingMultipleSplits() {
}

@Test
public void trackImpressionsIsPresent() {
public void impressionsDisabledIsPresent() {
Split split = SplitHelper.createSplit("FeatureName", 123, true,
"some_treatment", Arrays.asList(getTestCondition()),
"traffic", 456L, 1, null);
split.trackImpressions = false;
split.impressionsDisabled = false;
when(mSplitsStorage.get("FeatureName")).thenReturn(split);

SplitView featureFlag = mSplitManager.split("FeatureName");

assertFalse(featureFlag.trackImpressions);
assertFalse(featureFlag.impressionsDisabled);
}

private Condition getTestCondition() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,15 @@ public void evaluationWhenNotReadyLogsCorrectMessage() {
@Test
public void trackValueFromEvaluationResultGetsPassedInToImpression() {
Evaluator evaluatorMock = mock(Evaluator.class);
when(evaluatorMock.getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_split"), anyMap()))
when(evaluatorMock.getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_impressions_disabled"), eq(new HashMap<>())))
.thenReturn(new EvaluationResult("test", "test", true));
TreatmentManagerImpl tManager = initializeTreatmentManager(evaluatorMock);

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

verify(impressionListener).log(argThat(DecoratedImpression::getTrackImpressions));
verify(impressionListener).log(argThat((DecoratedImpression decoratedImpression) -> {
return decoratedImpression.isImpressionsDisabled();
}));
}

private void assertControl(List<String> splitList, String treatment, Map<String, String> treatmentList, SplitResult splitResult, Map<String, SplitResult> splitResultList) {
Expand Down
Loading

0 comments on commit 20c8ece

Please sign in to comment.