From 08da07393515491b24123a783cd5f922c611e294 Mon Sep 17 00:00:00 2001 From: gthea Date: Fri, 23 May 2025 14:39:31 -0300 Subject: [PATCH 1/5] Prerequisites in splitChanges DTO (#770) --- .../android/client/dtos/Prerequisite.java | 31 +++ .../io/split/android/client/dtos/Split.java | 9 + .../TargetingRulesResponseParserTest.java | 34 +++ .../split_changes_prerequisites.json | 253 ++++++++++++++++++ 4 files changed, 327 insertions(+) create mode 100644 src/main/java/io/split/android/client/dtos/Prerequisite.java create mode 100644 src/test/resources/split_changes_prerequisites.json diff --git a/src/main/java/io/split/android/client/dtos/Prerequisite.java b/src/main/java/io/split/android/client/dtos/Prerequisite.java new file mode 100644 index 000000000..153704e69 --- /dev/null +++ b/src/main/java/io/split/android/client/dtos/Prerequisite.java @@ -0,0 +1,31 @@ +package io.split.android.client.dtos; + +import com.google.gson.annotations.SerializedName; + +import java.util.HashSet; +import java.util.Set; + +public class Prerequisite { + + @SerializedName("n") + private String name; + + @SerializedName("ts") + private Set treatments; + + public Prerequisite() { + } + + Prerequisite(String name, Set treatments) { + this.name = name; + this.treatments = treatments; + } + + public String getName() { + return name; + } + + public Set getTreatments() { + return treatments == null ? new HashSet<>() : treatments; + } +} diff --git a/src/main/java/io/split/android/client/dtos/Split.java b/src/main/java/io/split/android/client/dtos/Split.java index e22838048..22869d3f7 100644 --- a/src/main/java/io/split/android/client/dtos/Split.java +++ b/src/main/java/io/split/android/client/dtos/Split.java @@ -4,6 +4,7 @@ import com.google.gson.annotations.SerializedName; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -53,6 +54,10 @@ public class Split { @SerializedName("impressionsDisabled") public boolean impressionsDisabled = false; + @Nullable + @SerializedName("prerequisites") + private List prerequisites; + public String json = null; public Split() { @@ -63,4 +68,8 @@ public Split(String name, String json) { this.name = name; this.json = json; } + + public List getPrerequisites() { + return prerequisites == null ? new ArrayList<>() : prerequisites; + } } diff --git a/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java b/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java index 2ce2ded65..19540d879 100644 --- a/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java +++ b/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java @@ -8,10 +8,13 @@ import org.junit.Before; import org.junit.Test; +import java.util.List; import java.util.Set; import io.split.android.client.dtos.Excluded; import io.split.android.client.dtos.ExcludedSegment; +import io.split.android.client.dtos.Prerequisite; +import io.split.android.client.dtos.Split; import io.split.android.client.dtos.TargetingRulesChange; import io.split.android.client.service.http.HttpResponseParserException; import io.split.android.helpers.FileHelper; @@ -63,6 +66,37 @@ public void parsesLegacySplitChangeJson() throws Exception { assertTrue(result.getRuleBasedSegmentsChange().getSegments().isEmpty()); } + @Test + public void parsesPrerequisites() throws HttpResponseParserException { + String json = fileHelper.loadFileContent("split_changes_prerequisites.json"); + TargetingRulesChange result = parser.parse(json); + + assertNotNull(result); + assertNotNull(result.getFeatureFlagsChange()); + Split firstSplit = result.getFeatureFlagsChange().splits.get(0); + assertEquals("FACUNDO_TEST", firstSplit.name); + List preReqs = firstSplit.getPrerequisites(); + assertEquals(2, preReqs.size()); + assertEquals("flag1", preReqs.get(0).getName()); + assertEquals("flag2", preReqs.get(1).getName()); + assertEquals(2, preReqs.get(0).getTreatments().size()); + assertEquals(1, preReqs.get(1).getTreatments().size()); + assertTrue(preReqs.get(0).getTreatments().contains("on")); + assertTrue(preReqs.get(0).getTreatments().contains("v1")); + assertTrue(preReqs.get(1).getTreatments().contains("off")); + } + + @Test + public void nonExistingPrerequisitesDefaultsToEmpty() throws HttpResponseParserException { + String json = fileHelper.loadFileContent("split_changes_prerequisites.json"); + TargetingRulesChange result = parser.parse(json); + assertNotNull(result); + Split split = result.getFeatureFlagsChange().splits.get(1); + assertEquals("FACUNDO_TEST_2", split.name); + List preReqs = split.getPrerequisites(); + assertEquals(0, preReqs.size()); + } + @Test public void parseNullReturnsNull() throws HttpResponseParserException { TargetingRulesChange result = parser.parse(null); diff --git a/src/test/resources/split_changes_prerequisites.json b/src/test/resources/split_changes_prerequisites.json new file mode 100644 index 000000000..e456d4cb2 --- /dev/null +++ b/src/test/resources/split_changes_prerequisites.json @@ -0,0 +1,253 @@ +{ + "ff": { + "d": [ + { + "trafficTypeName": "account", + "name": "FACUNDO_TEST", + "trafficAllocation": 59, + "trafficAllocationSeed": -2108186082, + "seed": -1947050785, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1506703262916, + "prerequisites": [ + { + "n": "flag1", + "ts": ["on","v1"] + }, + { + "n": "flag2", + "ts": ["off"] + } + ], + "algo": 2, + "conditions": [ + { + "conditionType": "WHITELIST", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": null, + "matcherType": "WHITELIST", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": { + "whitelist": [ + "nico_test", + "othertest" + ] + }, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 100 + } + ], + "label": "whitelisted" + }, + { + "conditionType": "WHITELIST", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": null, + "matcherType": "WHITELIST", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": { + "whitelist": [ + "bla" + ] + }, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "off", + "size": 100 + } + ], + "label": "whitelisted" + }, + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "account", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 0 + }, + { + "treatment": "off", + "size": 100 + }, + { + "treatment": "visa", + "size": 0 + } + ], + "label": "in segment all" + } + ] + }, + { + "trafficTypeName": "account", + "name": "FACUNDO_TEST_2", + "trafficAllocation": 59, + "trafficAllocationSeed": -2108186082, + "seed": -1947050785, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1506703262916, + "algo": 2, + "conditions": [ + { + "conditionType": "WHITELIST", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": null, + "matcherType": "WHITELIST", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": { + "whitelist": [ + "nico_test", + "othertest" + ] + }, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 100 + } + ], + "label": "whitelisted" + }, + { + "conditionType": "WHITELIST", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": null, + "matcherType": "WHITELIST", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": { + "whitelist": [ + "bla" + ] + }, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "off", + "size": 100 + } + ], + "label": "whitelisted" + }, + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "account", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 0 + }, + { + "treatment": "off", + "size": 100 + }, + { + "treatment": "visa", + "size": 0 + } + ], + "label": "in segment all" + } + ] + } + ], + "s": -1, + "t": 1506703262916 + }, + "rbs": { + "d": [], + "s": 1506703262920, + "t": 1506703263000 + } +} \ No newline at end of file From f1ad4758c63ea40be3ada5418ac48167349ae8e1 Mon Sep 17 00:00:00 2001 From: gthea Date: Fri, 23 May 2025 15:33:09 -0300 Subject: [PATCH 2/5] Prereqs in SplitView (#771) --- .../android/client/SplitManagerImpl.java | 1 + .../split/android/client/api/SplitView.java | 2 + .../android/client/dtos/Prerequisite.java | 10 ++-- .../io/split/android/client/dtos/Split.java | 2 +- .../engine/experiments/ParsedSplit.java | 19 ++++++-- .../engine/experiments/SplitParser.java | 4 +- .../android/client/SplitManagerImplTest.java | 47 +++++++++++++++++++ .../TargetingRulesResponseParserTest.java | 4 +- .../io/split/android/helpers/SplitHelper.java | 3 +- 9 files changed, 80 insertions(+), 12 deletions(-) diff --git a/src/main/java/io/split/android/client/SplitManagerImpl.java b/src/main/java/io/split/android/client/SplitManagerImpl.java index 81b856d5c..86c37f169 100644 --- a/src/main/java/io/split/android/client/SplitManagerImpl.java +++ b/src/main/java/io/split/android/client/SplitManagerImpl.java @@ -145,6 +145,7 @@ private SplitView toSplitView(ParsedSplit parsedSplit) { splitView.sets = new ArrayList<>(parsedSplit.sets() == null ? new HashSet<>() : parsedSplit.sets()); splitView.defaultTreatment = parsedSplit.defaultTreatment(); splitView.impressionsDisabled = parsedSplit.impressionsDisabled(); + splitView.prerequisites = parsedSplit.prerequisites(); Set treatments = new HashSet<>(); for (ParsedCondition condition : parsedSplit.parsedConditions()) { diff --git a/src/main/java/io/split/android/client/api/SplitView.java b/src/main/java/io/split/android/client/api/SplitView.java index 4c76bdc6d..84cc81a2d 100644 --- a/src/main/java/io/split/android/client/api/SplitView.java +++ b/src/main/java/io/split/android/client/api/SplitView.java @@ -7,6 +7,7 @@ import java.util.Map; import io.split.android.client.SplitManager; +import io.split.android.client.dtos.Prerequisite; /** * A view of a feature flag, meant for consumption through {@link SplitManager} interface. @@ -22,4 +23,5 @@ public class SplitView { public List sets = new ArrayList<>(); public String defaultTreatment; public boolean impressionsDisabled; + public List prerequisites = new ArrayList<>(); } diff --git a/src/main/java/io/split/android/client/dtos/Prerequisite.java b/src/main/java/io/split/android/client/dtos/Prerequisite.java index 153704e69..989edf894 100644 --- a/src/main/java/io/split/android/client/dtos/Prerequisite.java +++ b/src/main/java/io/split/android/client/dtos/Prerequisite.java @@ -1,5 +1,7 @@ package io.split.android.client.dtos; +import androidx.annotation.NonNull; + import com.google.gson.annotations.SerializedName; import java.util.HashSet; @@ -16,15 +18,17 @@ public class Prerequisite { public Prerequisite() { } - Prerequisite(String name, Set treatments) { + public Prerequisite(String name, Set treatments) { this.name = name; this.treatments = treatments; } - public String getName() { - return name; + @NonNull + public String getFlagName() { + return name == null ? "" : name; } + @NonNull public Set getTreatments() { return treatments == null ? new HashSet<>() : treatments; } diff --git a/src/main/java/io/split/android/client/dtos/Split.java b/src/main/java/io/split/android/client/dtos/Split.java index 22869d3f7..0c81bafbe 100644 --- a/src/main/java/io/split/android/client/dtos/Split.java +++ b/src/main/java/io/split/android/client/dtos/Split.java @@ -56,7 +56,7 @@ public class Split { @Nullable @SerializedName("prerequisites") - private List prerequisites; + public List prerequisites; public String json = null; diff --git a/src/main/java/io/split/android/engine/experiments/ParsedSplit.java b/src/main/java/io/split/android/engine/experiments/ParsedSplit.java index a7d936de9..ec7f784bc 100644 --- a/src/main/java/io/split/android/engine/experiments/ParsedSplit.java +++ b/src/main/java/io/split/android/engine/experiments/ParsedSplit.java @@ -9,6 +9,8 @@ import java.util.Objects; import java.util.Set; +import io.split.android.client.dtos.Prerequisite; + public class ParsedSplit { private final String mSplit; @@ -24,6 +26,7 @@ public class ParsedSplit { private final Map mConfigurations; private final Set mSets; private final boolean mImpressionsDisabled; + private final List mPrerequisites; public ParsedSplit( String feature, @@ -38,7 +41,8 @@ public ParsedSplit( int algo, Map configurations, Set sets, - boolean impressionsDisabled + boolean impressionsDisabled, + List prerequisites ) { mSplit = feature; mSeed = seed; @@ -57,6 +61,7 @@ public ParsedSplit( mTrafficAllocation = trafficAllocation; mTrafficAllocationSeed = trafficAllocationSeed; mSets = sets; + mPrerequisites = prerequisites; } public String feature() { @@ -111,6 +116,10 @@ public boolean impressionsDisabled() { return mImpressionsDisabled; } + public List prerequisites() { + return mPrerequisites; + } + @Override public int hashCode() { int result = 17; @@ -124,6 +133,7 @@ public int hashCode() { result = 31 * result + (mAlgo ^ (mAlgo >>> 32)); result = 31 * result + ((mSets != null) ? mSets.hashCode() : 0); result = 31 * result + (mImpressionsDisabled ? 1 : 0); + result = 31 * result + ((mPrerequisites != null) ? mPrerequisites.hashCode() : 0); return result; } @@ -144,8 +154,8 @@ public boolean equals(Object obj) { && mAlgo == other.mAlgo && (Objects.equals(mConfigurations, other.mConfigurations)) && (Objects.equals(mSets, other.mSets) - && mImpressionsDisabled == other.mImpressionsDisabled); - + && mImpressionsDisabled == other.mImpressionsDisabled + && (Objects.equals(mPrerequisites, other.mPrerequisites))); } @NonNull @@ -155,7 +165,8 @@ public String toString() { ", default treatment:" + mDefaultTreatment + ", parsedConditions:" + mParsedCondition + ", trafficTypeName:" + mTrafficTypeName + ", changeNumber:" + mChangeNumber + - ", algo:" + mAlgo + ", config:" + mConfigurations + ", sets:" + mSets + ", impressionsDisabled:" + mImpressionsDisabled; + ", algo:" + mAlgo + ", config:" + mConfigurations + ", sets:" + mSets + + ", impressionsDisabled:" + mImpressionsDisabled + ", prerequisites:" + mPrerequisites; } } diff --git a/src/main/java/io/split/android/engine/experiments/SplitParser.java b/src/main/java/io/split/android/engine/experiments/SplitParser.java index 03cbc27e4..3aa5198ea 100644 --- a/src/main/java/io/split/android/engine/experiments/SplitParser.java +++ b/src/main/java/io/split/android/engine/experiments/SplitParser.java @@ -4,6 +4,7 @@ import androidx.annotation.Nullable; +import java.util.ArrayList; import java.util.List; import io.split.android.client.dtos.Split; @@ -61,6 +62,7 @@ private ParsedSplit parseWithoutExceptionHandling(Split split, String matchingKe split.algo, split.configurations, split.sets, - split.impressionsDisabled); + split.impressionsDisabled, + new ArrayList<>(split.getPrerequisites())); } } diff --git a/src/test/java/io/split/android/client/SplitManagerImplTest.java b/src/test/java/io/split/android/client/SplitManagerImplTest.java index 180bbf3ff..fce55876d 100644 --- a/src/test/java/io/split/android/client/SplitManagerImplTest.java +++ b/src/test/java/io/split/android/client/SplitManagerImplTest.java @@ -8,6 +8,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; import org.junit.Before; @@ -18,12 +19,15 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import io.split.android.client.api.SplitView; import io.split.android.client.dtos.Condition; +import io.split.android.client.dtos.Prerequisite; import io.split.android.client.dtos.Split; import io.split.android.client.storage.mysegments.MySegmentsStorage; import io.split.android.client.storage.mysegments.MySegmentsStorageContainer; @@ -204,6 +208,49 @@ public void impressionsDisabledIsPresent() { assertFalse(featureFlag.impressionsDisabled); } + @Test + public void prerequisitesIsPresent() { + Split split = SplitHelper.createSplit("FeatureName", 123, true, + "some_treatment", Arrays.asList(getTestCondition()), + "traffic", 456L, 1, null); + Prerequisite p1 = new Prerequisite("prereq_feature", Collections.singleton("some_treatment")); + HashSet treatments = new HashSet<>(); + treatments.add("some_treatment_1"); + treatments.add("some_other_treatment"); + Prerequisite p2 = new Prerequisite("prereq_feature_2", treatments); + split.prerequisites = Arrays.asList(p1, p2); + + when(mSplitsStorage.get("FeatureName")).thenReturn(split); + + SplitView featureFlag = mSplitManager.split("FeatureName"); + + List prerequisites = featureFlag.prerequisites; + Prerequisite prereq1 = prerequisites.get(0); + Prerequisite prereq2 = prerequisites.get(1); + assertEquals(2, prerequisites.size()); + assertEquals("prereq_feature", prereq1.getFlagName()); + assertEquals(1, prereq1.getTreatments().size()); + assertEquals("some_treatment", prereq1.getTreatments().iterator().next()); + assertEquals("prereq_feature_2", prereq2.getFlagName()); + assertEquals(2, prereq2.getTreatments().size()); + assertTrue(prereq2.getTreatments().contains("some_treatment_1")); + assertTrue(prereq2.getTreatments().contains("some_other_treatment")); + } + + @Test + public void nullPrerequisitesDefaultToEmptyList() { + Split split = SplitHelper.createSplit("FeatureName", 123, true, + "some_treatment", Arrays.asList(getTestCondition()), + "traffic", 456L, 1, null); + split.prerequisites = null; + when(mSplitsStorage.get("FeatureName")).thenReturn(split); + + SplitView featureFlag = mSplitManager.split("FeatureName"); + + List prerequisites = featureFlag.prerequisites; + assertEquals(0, prerequisites.size()); + } + private Condition getTestCondition() { return SplitHelper.createCondition(CombiningMatcher.of(new AllKeysMatcher()), Arrays.asList(ConditionsTestUtil.partition("off", 10))); } diff --git a/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java b/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java index 19540d879..c5622d343 100644 --- a/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java +++ b/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java @@ -77,8 +77,8 @@ public void parsesPrerequisites() throws HttpResponseParserException { assertEquals("FACUNDO_TEST", firstSplit.name); List preReqs = firstSplit.getPrerequisites(); assertEquals(2, preReqs.size()); - assertEquals("flag1", preReqs.get(0).getName()); - assertEquals("flag2", preReqs.get(1).getName()); + assertEquals("flag1", preReqs.get(0).getFlagName()); + assertEquals("flag2", preReqs.get(1).getFlagName()); assertEquals(2, preReqs.get(0).getTreatments().size()); assertEquals(1, preReqs.get(1).getTreatments().size()); assertTrue(preReqs.get(0).getTreatments().contains("on")); diff --git a/src/test/java/io/split/android/helpers/SplitHelper.java b/src/test/java/io/split/android/helpers/SplitHelper.java index 636c376ca..a07c00bb8 100644 --- a/src/test/java/io/split/android/helpers/SplitHelper.java +++ b/src/test/java/io/split/android/helpers/SplitHelper.java @@ -123,7 +123,8 @@ public static ParsedSplit createParsedSplit( algo, configurations, Collections.emptySet(), - false + false, + new ArrayList<>() ); } From 146f5b83fc44ff9a21b7331898aaf66b35bde010 Mon Sep 17 00:00:00 2001 From: gthea Date: Mon, 26 May 2025 13:55:13 -0300 Subject: [PATCH 3/5] Prerequisites matcher (#772) --- .github/workflows/instrumented.yml | 1 + .../engine/matchers/PrerequisitesMatcher.java | 36 ++++ .../matchers/PrerequisitesMatcherTest.java | 197 ++++++++++++++++++ 3 files changed, 234 insertions(+) create mode 100644 src/main/java/io/split/android/engine/matchers/PrerequisitesMatcher.java create mode 100644 src/test/java/io/split/android/engine/matchers/PrerequisitesMatcherTest.java diff --git a/.github/workflows/instrumented.yml b/.github/workflows/instrumented.yml index 69ac5077c..6ec106f2e 100644 --- a/.github/workflows/instrumented.yml +++ b/.github/workflows/instrumented.yml @@ -4,6 +4,7 @@ on: pull_request: branches: - 'development' + - '*_baseline' concurrency: group: ${{ github.workflow }}-${{ github.ref }} diff --git a/src/main/java/io/split/android/engine/matchers/PrerequisitesMatcher.java b/src/main/java/io/split/android/engine/matchers/PrerequisitesMatcher.java new file mode 100644 index 000000000..6d87323d0 --- /dev/null +++ b/src/main/java/io/split/android/engine/matchers/PrerequisitesMatcher.java @@ -0,0 +1,36 @@ +package io.split.android.engine.matchers; + +import androidx.annotation.NonNull; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import io.split.android.client.EvaluationResult; +import io.split.android.client.Evaluator; +import io.split.android.client.dtos.Prerequisite; + +public class PrerequisitesMatcher implements Matcher { + + @NonNull + private final Set mPrerequisites; + + public PrerequisitesMatcher(Set prerequisites) { + mPrerequisites = prerequisites == null ? new HashSet<>() : prerequisites; + } + + @Override + public boolean match(Object matchValue, String bucketingKey, Map attributes, Evaluator evaluator) { + if (!(matchValue instanceof String)) { + return false; + } + + for (Prerequisite prerequisite : mPrerequisites) { + EvaluationResult treatment = evaluator.getTreatment((String) matchValue, bucketingKey, prerequisite.getFlagName(), attributes); + if (treatment == null || !prerequisite.getTreatments().contains(treatment.getTreatment())) { + return false; + } + } + return true; + } +} diff --git a/src/test/java/io/split/android/engine/matchers/PrerequisitesMatcherTest.java b/src/test/java/io/split/android/engine/matchers/PrerequisitesMatcherTest.java new file mode 100644 index 000000000..5c42eb090 --- /dev/null +++ b/src/test/java/io/split/android/engine/matchers/PrerequisitesMatcherTest.java @@ -0,0 +1,197 @@ +package io.split.android.engine.matchers; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import io.split.android.client.EvaluationResult; +import io.split.android.client.Evaluator; +import io.split.android.client.dtos.Condition; +import io.split.android.client.dtos.Partition; +import io.split.android.client.dtos.Prerequisite; +import io.split.android.client.dtos.Split; +import io.split.android.client.storage.splits.SplitsStorage; +import io.split.android.helpers.SplitHelper; + +public class PrerequisitesMatcherTest { + + @Mock + private SplitsStorage mSplitsStorage; + + @Mock + private Evaluator mEvaluator; + + private final Map mStoredSplits = new HashMap<>(); + + @Before + public void setUp() { + mSplitsStorage = mock(SplitsStorage.class); + mEvaluator = mock(Evaluator.class); + + List alwaysOnPartitions = createPartitions("on", 100, "off", 0); + List alwaysOnConditions = new ArrayList<>(); + alwaysOnConditions.add(SplitHelper.createCondition(null, alwaysOnPartitions)); + + Split alwaysOn = SplitHelper.createSplit( + "always-on", + -725161385, + false, + "off", + alwaysOnConditions, + "user", + 1494364996459L, + 1, + null); + + List alwaysOffPartitions = createPartitions("on", 0, "off", 100); + List alwaysOffConditions = new ArrayList<>(); + alwaysOffConditions.add(SplitHelper.createCondition(null, alwaysOffPartitions)); + + Split alwaysOff = SplitHelper.createSplit( + "always-off", + 403891040, + false, + "on", + alwaysOffConditions, + "user", + 1494365020316L, + 1, + null); + + mStoredSplits.put("always-on", alwaysOn); + mStoredSplits.put("always-off", alwaysOff); + + when(mSplitsStorage.get(eq("always-on"))).thenReturn(mStoredSplits.get("always-on")); + when(mSplitsStorage.get(eq("always-off"))).thenReturn(mStoredSplits.get("always-off")); + when(mSplitsStorage.get(eq("not-existent-feature-flag"))).thenReturn(null); + + when(mEvaluator.getTreatment(any(), any(), eq("always-on"), any())) + .thenReturn(new EvaluationResult("on", "in segment all")); + + when(mEvaluator.getTreatment(any(), any(), eq("always-off"), any())) + .thenReturn(new EvaluationResult("off", "in segment all")); + + when(mEvaluator.getTreatment(any(), any(), eq("not-existent-feature-flag"), any())) + .thenReturn(null); + } + + private List createPartitions(String treatment1, int size1, String treatment2, int size2) { + List partitions = new ArrayList<>(); + + Partition partition1 = new Partition(); + partition1.treatment = treatment1; + partition1.size = size1; + partitions.add(partition1); + + Partition partition2 = new Partition(); + partition2.treatment = treatment2; + partition2.size = size2; + partitions.add(partition2); + + return partitions; + } + + @Test + public void shouldReturnTrueWhenSinglePrerequisiteIsMetForAlwaysOn() { + Set prerequisites = new HashSet<>(); + prerequisites.add(new Prerequisite("always-on", new HashSet<>(Arrays.asList("not-existing", "on", "other")))); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); + + assertTrue(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnFalseWhenSinglePrerequisiteIsNotMetForAlwaysOn() { + Set prerequisites = new HashSet<>(); + prerequisites.add(new Prerequisite("always-on", new HashSet<>(Arrays.asList("off", "v1")))); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); + + assertFalse(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnTrueWhenSinglePrerequisiteIsMetForAlwaysOff() { + Set prerequisites = new HashSet<>(); + prerequisites.add(new Prerequisite("always-off", new HashSet<>(Arrays.asList("not-existing", "off")))); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); + + assertTrue(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnFalseWhenSinglePrerequisiteIsNotMetForAlwaysOff() { + Set prerequisites = new HashSet<>(); + prerequisites.add(new Prerequisite("always-off", new HashSet<>(Arrays.asList("v1", "on")))); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); + + assertFalse(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnTrueWhenAllMultiplePrerequisitesAreMet() { + Set prerequisites = new HashSet<>(); + prerequisites.add(new Prerequisite("always-on", new HashSet<>(Collections.singletonList("on")))); + prerequisites.add(new Prerequisite("always-off", new HashSet<>(Collections.singletonList("off")))); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); + + assertTrue(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnFalseWhenOneOfMultiplePrerequisitesIsNotMet() { + Set prerequisites = new HashSet<>(); + prerequisites.add(new Prerequisite("always-on", new HashSet<>(Collections.singletonList("on")))); + prerequisites.add(new Prerequisite("always-off", new HashSet<>(Collections.singletonList("on")))); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); + + assertFalse(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnTrueWithNullPrerequisites() { + PrerequisitesMatcher matcher = new PrerequisitesMatcher(null); + + assertTrue(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnTrueWithEmptyPrerequisites() { + PrerequisitesMatcher matcher = new PrerequisitesMatcher(new HashSet<>()); + + assertTrue(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnFalseWhenFeatureFlagDoesNotExist() { + Set prerequisites = new HashSet<>(); + prerequisites.add(new Prerequisite("not-existent-feature-flag", new HashSet<>(Arrays.asList("on", "off")))); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); + + assertFalse(matcher.match("a-key", null, null, mEvaluator)); + } + + @Test + public void shouldReturnFalseWithEmptyTreatmentsList() { + Set prerequisites = new HashSet<>(); + prerequisites.add(new Prerequisite("always-on", new HashSet<>())); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); + + assertFalse(matcher.match("a-key", null, null, mEvaluator)); + } +} From f41e5ff467a17610e00598d7af833232566d9316 Mon Sep 17 00:00:00 2001 From: gthea Date: Mon, 26 May 2025 15:37:44 -0300 Subject: [PATCH 4/5] Prereqs E2E tests (#773) --- .../assets/splitchanges_prerequisites.json | 165 ++++++++++ .../java/helper/IntegrationHelper.java | 1 + .../integration/init/InitializationTest.java | 9 +- .../matcher/PrerequisitesTest.java | 256 ++++++++++++++++ .../split/android/client/EvaluatorImpl.java | 12 + .../split/android/client/TreatmentLabels.java | 1 + .../engine/matchers/PrerequisitesMatcher.java | 10 +- .../PrerequisitesEvaluatorTest.java | 124 ++++++++ .../matchers/PrerequisitesMatcherTest.java | 19 +- .../split_changes_with_prerequisites.json | 283 ++++++++++++++++++ 10 files changed, 860 insertions(+), 20 deletions(-) create mode 100644 src/androidTest/assets/splitchanges_prerequisites.json create mode 100644 src/androidTest/java/tests/integration/matcher/PrerequisitesTest.java create mode 100644 src/test/java/io/split/android/engine/experiments/PrerequisitesEvaluatorTest.java create mode 100644 src/test/resources/split_changes_with_prerequisites.json diff --git a/src/androidTest/assets/splitchanges_prerequisites.json b/src/androidTest/assets/splitchanges_prerequisites.json new file mode 100644 index 000000000..6e9c1cc08 --- /dev/null +++ b/src/androidTest/assets/splitchanges_prerequisites.json @@ -0,0 +1,165 @@ +{ + "ff": { + "s": -1, + "t": 1602796638344, + "d": [ + { + "name": "always_on_if_prerequisite", + "trafficTypeName": "user", + "changeNumber": 5, + "seed": -790401604, + "trafficAllocation": 0, + "trafficAllocationSeed": 1828377380, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "prerequisites": [ + { + "n": "rbs_test_flag", + "ts": [ + "v1" + ] + } + ], + "conditions": [ + { + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 100 + } + ], + "label": "default rule" + } + ] + }, + { + "name": "rbs_test_flag", + "trafficTypeName": "user", + "trafficAllocation": 100, + "trafficAllocationSeed": 1828377380, + "seed": -286617921, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "algo": 2, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user" + }, + "matcherType": "IN_RULE_BASED_SEGMENT", + "negate": false, + "userDefinedSegmentMatcherData": { + "segmentName": "test_rule_based_segment" + } + } + ] + }, + "partitions": [ + { + "treatment": "v1", + "size": 100 + }, + { + "treatment": "v2", + "size": 0 + } + ], + "label": "in rule based segment test_rule_based_segment" + }, + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user" + }, + "matcherType": "ALL_KEYS", + "negate": false + } + ] + }, + "partitions": [ + { + "treatment": "v1", + "size": 0 + }, + { + "treatment": "v2", + "size": 100 + } + ], + "label": "default rule" + } + ], + "configurations": {}, + "sets": [], + "impressionsDisabled": false + } + ] + }, + "rbs": { + "s": -1, + "t": 100, + "d": [ + { + "name": "test_rule_based_segment", + "status": "ACTIVE", + "trafficTypeName": "user", + "excluded": { + "keys": [ + "mauro@split.io", + "gaston@split.io" + ], + "segments": [] + }, + "conditions": [ + { + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user" + }, + "matcherType": "ENDS_WITH", + "negate": false, + "whitelistMatcherData": { + "whitelist": [ + "@split.io" + ] + } + } + ] + } + } + ] + } + ] + } +} diff --git a/src/androidTest/java/helper/IntegrationHelper.java b/src/androidTest/java/helper/IntegrationHelper.java index df13116fb..40062cd6d 100644 --- a/src/androidTest/java/helper/IntegrationHelper.java +++ b/src/androidTest/java/helper/IntegrationHelper.java @@ -504,5 +504,6 @@ public static class ServicePath { public static final String UNIQUE_KEYS = "keys/cs"; public static final String COUNT = "testImpressions/count"; public static final String IMPRESSIONS = "testImpressions/bulk"; + public static final String AUTH = "v2/auth"; } } diff --git a/src/androidTest/java/tests/integration/init/InitializationTest.java b/src/androidTest/java/tests/integration/init/InitializationTest.java index 340b74889..cc0df9154 100644 --- a/src/androidTest/java/tests/integration/init/InitializationTest.java +++ b/src/androidTest/java/tests/integration/init/InitializationTest.java @@ -19,16 +19,13 @@ import java.util.concurrent.atomic.AtomicBoolean; import helper.DatabaseHelper; -import helper.FileHelper; import helper.IntegrationHelper; import io.split.android.client.ServiceEndpoints; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitFactory; import io.split.android.client.api.Key; -import io.split.android.client.dtos.SplitChange; import io.split.android.client.events.SplitEvent; -import io.split.android.client.utils.Json; import io.split.android.client.utils.logger.Logger; import io.split.android.client.utils.logger.SplitLogLevel; import okhttp3.mockwebserver.Dispatcher; @@ -44,12 +41,14 @@ public class InitializationTest { private MockWebServer mWebServer; private AtomicBoolean mEventSent; + private CountDownLatch mEventLatch; @Before public void setUp() { setupServer(); mRequestCountdownLatch = new CountDownLatch(1); mEventSent = new AtomicBoolean(false); + mEventLatch = new CountDownLatch(1); } @Test @@ -72,8 +71,7 @@ public void immediateClientRecreation() throws InterruptedException { factory.client(new Key("new_key")).on(SplitEvent.SDK_READY, new TestingHelper.TestEventTask(secondReadyLatch)); boolean awaitReady2 = secondReadyLatch.await(5, TimeUnit.SECONDS); - // Wait for events to be posted - Thread.sleep(500); + mEventLatch.await(5, TimeUnit.SECONDS); assertTrue(readyAwait); assertTrue(awaitReady2); @@ -171,6 +169,7 @@ public MockResponse dispatch(RecordedRequest request) throws InterruptedExceptio } } else if (request.getPath().contains("/" + IntegrationHelper.ServicePath.EVENTS)) { mEventSent.set(true); + mEventLatch.countDown(); return new MockResponse().setResponseCode(200); } else if (request.getPath().contains("/" + IntegrationHelper.ServicePath.COUNT)) { return new MockResponse().setResponseCode(200); diff --git a/src/androidTest/java/tests/integration/matcher/PrerequisitesTest.java b/src/androidTest/java/tests/integration/matcher/PrerequisitesTest.java new file mode 100644 index 000000000..9a34f73c9 --- /dev/null +++ b/src/androidTest/java/tests/integration/matcher/PrerequisitesTest.java @@ -0,0 +1,256 @@ +package tests.integration.matcher; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static helper.IntegrationHelper.getSinceFromUri; + +import android.content.Context; + +import androidx.test.platform.app.InstrumentationRegistry; + +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import fake.HttpClientMock; +import fake.HttpResponseMock; +import fake.HttpResponseMockDispatcher; +import helper.DatabaseHelper; +import helper.IntegrationHelper; +import helper.TestableSplitConfigBuilder; +import io.split.android.client.SplitClient; +import io.split.android.client.SplitFactory; +import io.split.android.client.SplitManager; +import io.split.android.client.api.Key; +import io.split.android.client.api.SplitView; +import io.split.android.client.events.SplitEvent; +import io.split.android.client.impressions.Impression; +import io.split.android.client.impressions.ImpressionListener; +import io.split.android.client.storage.db.ImpressionEntity; +import io.split.android.client.storage.db.SplitRoomDatabase; +import tests.integration.shared.TestingHelper; + +public class PrerequisitesTest { + + private final Context mContext = InstrumentationRegistry.getInstrumentation().getContext(); + private HttpClientMock mHttpClient; + private SplitRoomDatabase mDatabase; + private CountDownLatch mAuthLatch; + + @Before + public void setUp() throws IOException { + mAuthLatch = new CountDownLatch(1); + mDatabase = DatabaseHelper.getTestDatabase(mContext); + mDatabase.clearAllTables(); + mHttpClient = new HttpClientMock(getDispatcher()); + } + + @Test + public void prerequisiteNotSatisfiedForExcludedKey() throws InterruptedException { + // Test that a key excluded from the rule-based segment doesn't satisfy the prerequisite + SplitFactory splitFactory = initSplitFactory(new TestableSplitConfigBuilder(), mHttpClient, "test"); + SplitClient client = splitFactory.client(); + + String treatment = client.getTreatment("always_on_if_prerequisite"); + + assertEquals("off", treatment); + } + + @Test + public void prerequisiteSatisfiedForSplitIoUser() throws InterruptedException { + // Test that a key with @split.io satisfies the rule-based segment condition and the prerequisite + SplitFactory splitFactory = initSplitFactory(new TestableSplitConfigBuilder(), mHttpClient, "bilal@split.io"); + SplitClient client = splitFactory.client(); + + String treatment = client.getTreatment("always_on_if_prerequisite"); + + assertEquals("on", treatment); + } + + @Test + public void prerequisiteNotSatisfiedForNonSplitIoUser() throws InterruptedException { + // Test that a key without @split.io doesn't satisfy the rule-based segment condition + SplitFactory splitFactory = initSplitFactory(new TestableSplitConfigBuilder(), mHttpClient, "test"); + SplitClient client = splitFactory.client(); + + String treatment = client.getTreatment("always_on_if_prerequisite"); + + assertEquals("off", treatment); + } + + @Test + public void splitManagerContainsPrerequisiteInfo() throws InterruptedException { + // Test that the SplitManager returns the correct prerequisite information + SplitFactory splitFactory = initSplitFactory(new TestableSplitConfigBuilder(), mHttpClient, "test"); + SplitManager manager = splitFactory.manager(); + + SplitView split = manager.split("always_on_if_prerequisite"); + + assertNotNull(split); + assertEquals("user", split.trafficType); + assertEquals("always_on_if_prerequisite", split.name); + assertFalse(split.killed); + assertEquals(5, split.changeNumber); + assertEquals("off", split.defaultTreatment); + assertEquals(1, split.prerequisites.size()); + assertEquals("rbs_test_flag", split.prerequisites.get(0).getFlagName()); + assertEquals(1, split.prerequisites.get(0).getTreatments().size()); + assertTrue(split.prerequisites.get(0).getTreatments().contains("v1")); + } + + @Test + public void storedImpressionsForParentSplit() throws InterruptedException { + // Test that impressions are stored for the parent split + SplitFactory splitFactory = initSplitFactory(new TestableSplitConfigBuilder(), mHttpClient, "bilal@split.io"); + SplitClient client = splitFactory.client(); + + // Evaluate the parent split which should trigger prerequisite evaluation + client.getTreatment("always_on_if_prerequisite"); + + // Wait for impressions to be stored + Thread.sleep(200); + List storedImpressions = mDatabase.impressionDao().getAll(); + + // Should have one impression for the parent split + assertEquals(1, storedImpressions.size()); + + // Verify the impression is for the parent split + ImpressionEntity impression = storedImpressions.get(0); + String flagName = impression.getTestName(); + String body = impression.getBody(); + assertTrue("Parent split impression not found", flagName.contains("always_on_if_prerequisite")); + assertNotNull(body); + + // Verify the impression does not contain the prerequisite split name + assertFalse("Prerequisite split impression should not be generated", body.contains("rbs_test_flag")); + } + + @Test + public void impressionListenerReceivesDifferentAppliedRulesBasedOnPrerequisites() throws InterruptedException { + final List impressions = new ArrayList<>(); + CountDownLatch listenerLatch = new CountDownLatch(2); + + // Create a split factory with an impression listener + SplitFactory splitFactory = initSplitFactory( + new TestableSplitConfigBuilder().impressionListener(new ImpressionListener() { + @Override + public void log(Impression impression) { + synchronized (impressions) { + impressions.add(impression); + } + listenerLatch.countDown(); + } + + @Override + public void close() { + // No-op + } + }), + mHttpClient, + "bilal@split.io" + ); + + // Create two clients: one that will match the prerequisite and one that won't + SplitClient splitIoClient = splitFactory.client(); // Uses bilal@split.io which matches the prerequisite + SplitClient nonSplitIoClient = splitFactory.client("just_bilal"); // Won't match the prerequisite + + // Evaluate + splitIoClient.getTreatment("always_on_if_prerequisite"); + nonSplitIoClient.getTreatment("always_on_if_prerequisite"); + + boolean awaitResult = listenerLatch.await(1, TimeUnit.SECONDS); + assertTrue("Did not receive expected impressions", awaitResult); + assertEquals("Should have received 2 impressions", 2, impressions.size()); + + // Find impressions for each client + Impression splitIoImpression = null; + Impression nonSplitIoImpression = null; + + for (Impression imp : impressions) { + if (imp.key().equals("bilal@split.io")) { + splitIoImpression = imp; + } else if (imp.key().equals("just_bilal")) { + nonSplitIoImpression = imp; + } + } + + assertNotNull("Missing impression for Split.io user", splitIoImpression); + assertNotNull("Missing impression for non-Split.io user", nonSplitIoImpression); + + assertEquals("on", splitIoImpression.treatment()); + assertEquals("off", nonSplitIoImpression.treatment()); + + assertEquals("default rule", splitIoImpression.appliedRule()); + assertEquals("prerequisites not met", nonSplitIoImpression.appliedRule()); + + // Verify we don't have impressions for the prerequisite + boolean hasPrerequisiteImpression = false; + for (Impression imp : impressions) { + if ("rbs_test_flag".equals(imp.split())) { + hasPrerequisiteImpression = true; + break; + } + } + assertFalse("Should not have an impression for the prerequisite", hasPrerequisiteImpression); + } + + private HttpResponseMockDispatcher getDispatcher() { + Map responses = new HashMap<>(); + responses.put(IntegrationHelper.ServicePath.SPLIT_CHANGES, (uri, httpMethod, body) -> { + String since = getSinceFromUri(uri); + if (since.equals("-1")) { + return new HttpResponseMock(200, loadSplitChanges()); + } else { + return new HttpResponseMock(200, IntegrationHelper.emptySplitChanges(1602796638344L, 1602796638344L)); + } + }); + + responses.put(IntegrationHelper.ServicePath.MEMBERSHIPS + "/" + "/test", (uri, httpMethod, body) -> new HttpResponseMock(200, IntegrationHelper.emptyMySegments())); + responses.put(IntegrationHelper.ServicePath.MEMBERSHIPS + "/" + "/bilal@split.io", (uri, httpMethod, body) -> new HttpResponseMock(200, IntegrationHelper.emptyMySegments())); + + responses.put(IntegrationHelper.ServicePath.AUTH, (uri, httpMethod, body) -> { + mAuthLatch.countDown(); + return new HttpResponseMock(200, IntegrationHelper.streamingEnabledToken()); + }); + + return IntegrationHelper.buildDispatcher(responses); + } + + private SplitFactory initSplitFactory(TestableSplitConfigBuilder builder, HttpClientMock httpClient, String matchingKey) throws InterruptedException { + CountDownLatch innerLatch = new CountDownLatch(1); + SplitFactory factory = IntegrationHelper.buildFactory( + "sdk_key_1", + new Key(matchingKey), + builder + .enableDebug() + .build(), + mContext, + httpClient, + mDatabase, + null); + + SplitClient client = factory.client(); + client.on(SplitEvent.SDK_READY, new TestingHelper.TestEventTask(innerLatch)); + boolean await = innerLatch.await(5, TimeUnit.SECONDS); + if (!await) { + fail("Client is not ready"); + } + + return factory; + } + + private String loadSplitChanges() { + return IntegrationHelper.loadSplitChanges(mContext, "splitchanges_prerequisites.json"); + } +} + diff --git a/src/main/java/io/split/android/client/EvaluatorImpl.java b/src/main/java/io/split/android/client/EvaluatorImpl.java index 305405eb1..28166977b 100644 --- a/src/main/java/io/split/android/client/EvaluatorImpl.java +++ b/src/main/java/io/split/android/client/EvaluatorImpl.java @@ -9,6 +9,7 @@ import io.split.android.engine.experiments.ParsedCondition; import io.split.android.engine.experiments.ParsedSplit; import io.split.android.engine.experiments.SplitParser; +import io.split.android.engine.matchers.PrerequisitesMatcher; import io.split.android.engine.splitter.Splitter; import io.split.android.grammar.Treatments; @@ -55,6 +56,17 @@ private EvaluationResult getTreatment(String matchingKey, String bucketingKey, P return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.KILLED, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.impressionsDisabled()); } + if (!parsedSplit.prerequisites().isEmpty()) { + PrerequisitesMatcher matcher = new PrerequisitesMatcher(parsedSplit.prerequisites()); + if (!matcher.match(matchingKey, bucketingKey, attributes, this)) { + return new EvaluationResult(parsedSplit.defaultTreatment(), + TreatmentLabels.PREREQUISITES_NOT_MET, + parsedSplit.changeNumber(), + configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), + parsedSplit.impressionsDisabled()); + } + } + /* * There are three parts to a single Split: 1) Whitelists 2) Traffic Allocation * 3) Rollout. The flag inRollout is there to understand when we move into the Rollout diff --git a/src/main/java/io/split/android/client/TreatmentLabels.java b/src/main/java/io/split/android/client/TreatmentLabels.java index 604537132..9133fc788 100644 --- a/src/main/java/io/split/android/client/TreatmentLabels.java +++ b/src/main/java/io/split/android/client/TreatmentLabels.java @@ -8,4 +8,5 @@ public class TreatmentLabels { public static final String KILLED = "killed"; public static final String NOT_READY = "not ready"; public static final String UNSUPPORTED_MATCHER_TYPE = "targeting rule type unsupported by sdk"; + public static final String PREREQUISITES_NOT_MET = "prerequisites not met"; } diff --git a/src/main/java/io/split/android/engine/matchers/PrerequisitesMatcher.java b/src/main/java/io/split/android/engine/matchers/PrerequisitesMatcher.java index 6d87323d0..977548a08 100644 --- a/src/main/java/io/split/android/engine/matchers/PrerequisitesMatcher.java +++ b/src/main/java/io/split/android/engine/matchers/PrerequisitesMatcher.java @@ -2,9 +2,9 @@ import androidx.annotation.NonNull; -import java.util.HashSet; +import java.util.ArrayList; +import java.util.List; import java.util.Map; -import java.util.Set; import io.split.android.client.EvaluationResult; import io.split.android.client.Evaluator; @@ -13,10 +13,10 @@ public class PrerequisitesMatcher implements Matcher { @NonNull - private final Set mPrerequisites; + private final List mPrerequisites; - public PrerequisitesMatcher(Set prerequisites) { - mPrerequisites = prerequisites == null ? new HashSet<>() : prerequisites; + public PrerequisitesMatcher(List prerequisites) { + mPrerequisites = prerequisites == null ? new ArrayList<>() : prerequisites; } @Override diff --git a/src/test/java/io/split/android/engine/experiments/PrerequisitesEvaluatorTest.java b/src/test/java/io/split/android/engine/experiments/PrerequisitesEvaluatorTest.java new file mode 100644 index 000000000..601198904 --- /dev/null +++ b/src/test/java/io/split/android/engine/experiments/PrerequisitesEvaluatorTest.java @@ -0,0 +1,124 @@ +package io.split.android.engine.experiments; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import io.split.android.client.EvaluationResult; +import io.split.android.client.Evaluator; +import io.split.android.client.EvaluatorImpl; +import io.split.android.client.TreatmentLabels; +import io.split.android.client.dtos.Split; +import io.split.android.client.storage.mysegments.MySegmentsStorage; +import io.split.android.client.storage.mysegments.MySegmentsStorageContainer; +import io.split.android.client.storage.rbs.RuleBasedSegmentStorage; +import io.split.android.client.storage.splits.SplitsStorage; +import io.split.android.helpers.FileHelper; + +/** + * Tests for the prerequisite functionality in the Evaluator + */ +public class PrerequisitesEvaluatorTest { + + private Evaluator evaluator; + private Map splitsMap; + + @Before + public void loadSplitsFromFile() { + if (evaluator == null) { + FileHelper fileHelper = new FileHelper(); + MySegmentsStorage mySegmentsStorage = mock(MySegmentsStorage.class); + MySegmentsStorage myLargeSegmentsStorage = mock(MySegmentsStorage.class); + MySegmentsStorageContainer mySegmentsStorageContainer = mock(MySegmentsStorageContainer.class); + MySegmentsStorageContainer myLargeSegmentsStorageContainer = mock(MySegmentsStorageContainer.class); + RuleBasedSegmentStorage ruleBasedSegmentStorage = mock(RuleBasedSegmentStorage.class); + SplitsStorage splitsStorage = mock(SplitsStorage.class); + + List splits = fileHelper.loadAndParseSplitChangeFile("split_changes_with_prerequisites.json"); + SplitParser splitParser = new SplitParser(new ParserCommons(mySegmentsStorageContainer, myLargeSegmentsStorageContainer)); + + splitsMap = splitsMap(splits); + when(splitsStorage.getAll()).thenReturn(splitsMap); + when(splitsStorage.get(any())).thenAnswer(new Answer() { + @Override + public Split answer(InvocationOnMock invocation) throws Throwable { + return splitsMap.get(invocation.getArgument(0)); + } + }); + + when(splitsStorage.get("parent_split")).thenReturn(splitsMap.get("parent_split")); + when(splitsStorage.get("child_split_1")).thenReturn(splitsMap.get("child_split_1")); + when(splitsStorage.get("child_split_2")).thenReturn(splitsMap.get("child_split_2")); + when(splitsStorage.get("parent_split_with_one_failing_prerequisite")).thenReturn(splitsMap.get("parent_split_with_one_failing_prerequisite")); + when(splitsStorage.get("parent_split_with_non_existent_prerequisite")).thenReturn(splitsMap.get("parent_split_with_non_existent_prerequisite")); + when(splitsStorage.get("non_existent_split")).thenReturn(null); + + when(mySegmentsStorageContainer.getStorageForKey(any())).thenReturn(mySegmentsStorage); + when(myLargeSegmentsStorageContainer.getStorageForKey(any())).thenReturn(myLargeSegmentsStorage); + + evaluator = new EvaluatorImpl(splitsStorage, splitParser); + } + } + + @Test + public void testPrerequisitesAllMet() { + String matchingKey = "user1"; + String splitName = "parent_split"; + EvaluationResult result = evaluator.getTreatment(matchingKey, matchingKey, splitName, null); + + Assert.assertNotNull(result); + Assert.assertEquals("off", result.getTreatment()); + Assert.assertEquals(TreatmentLabels.PREREQUISITES_NOT_MET, result.getLabel()); + } + + @Test + public void testPrerequisitesNotMet() { + String matchingKey = "user1"; + String splitName = "parent_split_with_one_failing_prerequisite"; + EvaluationResult result = evaluator.getTreatment(matchingKey, matchingKey, splitName, null); + + Assert.assertNotNull(result); + Assert.assertEquals("off", result.getTreatment()); + Assert.assertEquals(TreatmentLabels.PREREQUISITES_NOT_MET, result.getLabel()); + } + + @Test + public void testPrerequisiteNonExistent() { + String matchingKey = "user1"; + String splitName = "parent_split_with_non_existent_prerequisite"; + EvaluationResult result = evaluator.getTreatment(matchingKey, matchingKey, splitName, null); + + Assert.assertNotNull(result); + Assert.assertEquals("off", result.getTreatment()); + Assert.assertEquals(TreatmentLabels.PREREQUISITES_NOT_MET, result.getLabel()); + } + + @Test + public void testChildSplitEvaluation() { + String matchingKey = "user1"; + String splitName = "child_split_1"; + EvaluationResult result = evaluator.getTreatment(matchingKey, matchingKey, splitName, null); + + Assert.assertNotNull(result); + Assert.assertEquals("on", result.getTreatment()); + Assert.assertEquals("in segment all", result.getLabel()); + } + + private Map splitsMap(List splits) { + Map splitsMap = new HashMap<>(); + for (Split split : splits) { + splitsMap.put(split.name, split); + } + return splitsMap; + } +} diff --git a/src/test/java/io/split/android/engine/matchers/PrerequisitesMatcherTest.java b/src/test/java/io/split/android/engine/matchers/PrerequisitesMatcherTest.java index 5c42eb090..5ad25788d 100644 --- a/src/test/java/io/split/android/engine/matchers/PrerequisitesMatcherTest.java +++ b/src/test/java/io/split/android/engine/matchers/PrerequisitesMatcherTest.java @@ -18,7 +18,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import io.split.android.client.EvaluationResult; import io.split.android.client.Evaluator; @@ -109,7 +108,7 @@ private List createPartitions(String treatment1, int size1, String tr @Test public void shouldReturnTrueWhenSinglePrerequisiteIsMetForAlwaysOn() { - Set prerequisites = new HashSet<>(); + List prerequisites = new ArrayList<>(); prerequisites.add(new Prerequisite("always-on", new HashSet<>(Arrays.asList("not-existing", "on", "other")))); PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); @@ -118,7 +117,7 @@ public void shouldReturnTrueWhenSinglePrerequisiteIsMetForAlwaysOn() { @Test public void shouldReturnFalseWhenSinglePrerequisiteIsNotMetForAlwaysOn() { - Set prerequisites = new HashSet<>(); + List prerequisites = new ArrayList<>(); prerequisites.add(new Prerequisite("always-on", new HashSet<>(Arrays.asList("off", "v1")))); PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); @@ -127,7 +126,7 @@ public void shouldReturnFalseWhenSinglePrerequisiteIsNotMetForAlwaysOn() { @Test public void shouldReturnTrueWhenSinglePrerequisiteIsMetForAlwaysOff() { - Set prerequisites = new HashSet<>(); + List prerequisites = new ArrayList<>(); prerequisites.add(new Prerequisite("always-off", new HashSet<>(Arrays.asList("not-existing", "off")))); PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); @@ -136,7 +135,7 @@ public void shouldReturnTrueWhenSinglePrerequisiteIsMetForAlwaysOff() { @Test public void shouldReturnFalseWhenSinglePrerequisiteIsNotMetForAlwaysOff() { - Set prerequisites = new HashSet<>(); + List prerequisites = new ArrayList<>(); prerequisites.add(new Prerequisite("always-off", new HashSet<>(Arrays.asList("v1", "on")))); PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); @@ -145,7 +144,7 @@ public void shouldReturnFalseWhenSinglePrerequisiteIsNotMetForAlwaysOff() { @Test public void shouldReturnTrueWhenAllMultiplePrerequisitesAreMet() { - Set prerequisites = new HashSet<>(); + List prerequisites = new ArrayList<>(); prerequisites.add(new Prerequisite("always-on", new HashSet<>(Collections.singletonList("on")))); prerequisites.add(new Prerequisite("always-off", new HashSet<>(Collections.singletonList("off")))); PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); @@ -155,7 +154,7 @@ public void shouldReturnTrueWhenAllMultiplePrerequisitesAreMet() { @Test public void shouldReturnFalseWhenOneOfMultiplePrerequisitesIsNotMet() { - Set prerequisites = new HashSet<>(); + List prerequisites = new ArrayList<>(); prerequisites.add(new Prerequisite("always-on", new HashSet<>(Collections.singletonList("on")))); prerequisites.add(new Prerequisite("always-off", new HashSet<>(Collections.singletonList("on")))); PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); @@ -172,14 +171,14 @@ public void shouldReturnTrueWithNullPrerequisites() { @Test public void shouldReturnTrueWithEmptyPrerequisites() { - PrerequisitesMatcher matcher = new PrerequisitesMatcher(new HashSet<>()); + PrerequisitesMatcher matcher = new PrerequisitesMatcher(new ArrayList<>()); assertTrue(matcher.match("a-key", null, null, mEvaluator)); } @Test public void shouldReturnFalseWhenFeatureFlagDoesNotExist() { - Set prerequisites = new HashSet<>(); + List prerequisites = new ArrayList<>(); prerequisites.add(new Prerequisite("not-existent-feature-flag", new HashSet<>(Arrays.asList("on", "off")))); PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); @@ -188,7 +187,7 @@ public void shouldReturnFalseWhenFeatureFlagDoesNotExist() { @Test public void shouldReturnFalseWithEmptyTreatmentsList() { - Set prerequisites = new HashSet<>(); + List prerequisites = new ArrayList<>(); prerequisites.add(new Prerequisite("always-on", new HashSet<>())); PrerequisitesMatcher matcher = new PrerequisitesMatcher(prerequisites); diff --git a/src/test/resources/split_changes_with_prerequisites.json b/src/test/resources/split_changes_with_prerequisites.json new file mode 100644 index 000000000..c499582a2 --- /dev/null +++ b/src/test/resources/split_changes_with_prerequisites.json @@ -0,0 +1,283 @@ +{ + "ff": { + "splits": [ + { + "trafficTypeName": "user", + "name": "parent_split", + "trafficAllocation": 100, + "trafficAllocationSeed": 1012950810, + "seed": -725161385, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1494364996459, + "algo": 2, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 100 + }, + { + "treatment": "off", + "size": 0 + } + ], + "label": "in segment all" + } + ], + "configurations": { + "on": "{\"color\": \"blue\"}", + "off": "{\"color\": \"red\"}" + }, + "prerequisites": [ + { + "n": "child_split_1", + "ts": ["on"] + }, + { + "n": "child_split_2", + "ts": ["on"] + } + ] + }, + { + "trafficTypeName": "user", + "name": "child_split_1", + "trafficAllocation": 100, + "trafficAllocationSeed": 1012950810, + "seed": -725161385, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1494364996459, + "algo": 2, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 100 + }, + { + "treatment": "off", + "size": 0 + } + ], + "label": "in segment all" + } + ] + }, + { + "trafficTypeName": "user", + "name": "child_split_2", + "trafficAllocation": 100, + "trafficAllocationSeed": 1012950810, + "seed": -725161385, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1494364996459, + "algo": 2, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 0 + }, + { + "treatment": "off", + "size": 100 + } + ], + "label": "in segment all" + } + ] + }, + { + "trafficTypeName": "user", + "name": "parent_split_with_one_failing_prerequisite", + "trafficAllocation": 100, + "trafficAllocationSeed": 1012950810, + "seed": -725161385, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1494364996459, + "algo": 2, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 100 + }, + { + "treatment": "off", + "size": 0 + } + ], + "label": "in segment all" + } + ], + "prerequisites": [ + { + "n": "child_split_1", + "ts": ["on"] + }, + { + "n": "child_split_2", + "ts": ["on"] + } + ] + }, + { + "trafficTypeName": "user", + "name": "parent_split_with_non_existent_prerequisite", + "trafficAllocation": 100, + "trafficAllocationSeed": 1012950810, + "seed": -725161385, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1494364996459, + "algo": 2, + "conditions": [ + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 100 + }, + { + "treatment": "off", + "size": 0 + } + ], + "label": "in segment all" + } + ], + "prerequisites": [ + { + "n": "non_existent_split", + "ts": ["on"] + } + ] + } + ], + "since": -1, + "till": 1506703262916 + }, + "rbs": { + "d": [], + "s": -1, + "t": 1506703261916 + } +} From 201f45782f35de0f5002030adaef9ae5f293bdcb Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Mon, 26 May 2025 15:39:23 -0300 Subject: [PATCH 5/5] Update version --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index f60bc39ea..5adbe505b 100644 --- a/build.gradle +++ b/build.gradle @@ -17,7 +17,7 @@ apply plugin: 'kotlin-android' apply from: 'spec.gradle' ext { - splitVersion = '5.3.0-rc1' + splitVersion = '5.3.0-rc2' } android {