Filter integration

splitio · Sep 15, 2023 · 9c284d7 · 9c284d7
1 parent ef81e36
commit 9c284d7
Show file tree

Hide file tree

Showing 19 changed files with 107 additions and 94 deletions.
diff --git a/src/androidTest/java/tests/integration/sets/FlagSetsPollingTest.java b/src/androidTest/java/tests/integration/sets/FlagSetsPollingTest.java
@@ -162,7 +162,6 @@ public void featureFlagSetsAreIgnoredWhenSetsAreNotConfigured() throws IOExcepti
 
     @Test
     public void queryStringIsBuiltCorrectlyWhenSetsAreConfigured() throws IOException, InterruptedException {
-        // 1. Initialize a factory with polling and sets set_1 & set_2 configured.
         createFactory(mContext, mRoomDb, "set_x", "set_x", "set_3", "set_2", "set_3", "set_ww", "invalid+");
 
         boolean awaitFirst = firstChangeLatch.await(5, TimeUnit.SECONDS);

diff --git a/src/androidTest/java/tests/integration/userconsent/UserConsentModeDebugTest.kt b/src/androidTest/java/tests/integration/userconsent/UserConsentModeDebugTest.kt
@@ -231,7 +231,7 @@ class UserConsentModeDebugTest {
                 } else if (uri.path.contains("/splitChanges")) {
                     if (mChangeHit == 0) {
                         mChangeHit+=1
-                        return getSplitsMockResponse("", "")
+                        return getSplitsMockResponse("")
                     }
                     return HttpResponseMock(200, IntegrationHelper.emptySplitChanges(99999999, 99999999))
                 } else if (uri.path.contains("/testImpressions/bulk")) {
@@ -259,7 +259,7 @@ class UserConsentModeDebugTest {
         }
     }
 
-    private fun getSplitsMockResponse(since: String, till: String): HttpResponseMock {
+    private fun getSplitsMockResponse(since: String): HttpResponseMock {
         return HttpResponseMock(200, loadSplitChanges())
     }
 

diff --git a/src/androidTest/java/tests/integration/userconsent/UserConsentModeNoneTest.kt b/src/androidTest/java/tests/integration/userconsent/UserConsentModeNoneTest.kt
@@ -232,7 +232,7 @@ class UserConsentModeNoneTest {
                 } else if (uri.path.contains("/splitChanges")) {
                     if (mChangeHit == 0) {
                         mChangeHit+=1
-                        return getSplitsMockResponse("", "")
+                        return getSplitsMockResponse("")
                     }
                     return HttpResponseMock(200, IntegrationHelper.emptySplitChanges(99999999, 99999999))
                 } else if (uri.path.contains("/testImpressions/bulk")) {
@@ -260,7 +260,7 @@ class UserConsentModeNoneTest {
         }
     }
 
-    private fun getSplitsMockResponse(since: String, till: String): HttpResponseMock {
+    private fun getSplitsMockResponse(since: String): HttpResponseMock {
         return HttpResponseMock(200, loadSplitChanges())
     }
 

diff --git a/src/androidTest/java/tests/integration/userconsent/UserConsentModeOptimizedTest.kt b/src/androidTest/java/tests/integration/userconsent/UserConsentModeOptimizedTest.kt
@@ -242,7 +242,7 @@ class UserConsentModeOptimizedTest {
                 } else if (uri.path.contains("/splitChanges")) {
                     if (mChangeHit == 0) {
                         mChangeHit+=1
-                        return getSplitsMockResponse("", "")
+                        return getSplitsMockResponse("")
                     }
                     return HttpResponseMock(200, IntegrationHelper.emptySplitChanges(99999999, 99999999))
                 } else if (uri.path.contains("/testImpressions/bulk")) {
@@ -270,7 +270,7 @@ class UserConsentModeOptimizedTest {
         }
     }
 
-    private fun getSplitsMockResponse(since: String, till: String): HttpResponseMock {
+    private fun getSplitsMockResponse(since: String): HttpResponseMock {
         return HttpResponseMock(200, loadSplitChanges())
     }
 

diff --git a/src/main/java/io/split/android/client/FlagSetsFilterImpl.java b/src/main/java/io/split/android/client/FlagSetsFilterImpl.java
@@ -1,5 +1,6 @@
 package io.split.android.client;
 
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -8,7 +9,7 @@ public class FlagSetsFilterImpl implements FlagSetsFilter {
     private final boolean mShouldFilter;
     private final Set<String> mFlagSets;
 
-    public FlagSetsFilterImpl(Set<String> flagSets) {
+    public FlagSetsFilterImpl(Collection<String> flagSets) {
         mFlagSets = new HashSet<>(flagSets);
         mShouldFilter = !mFlagSets.isEmpty();
     }

diff --git a/src/main/java/io/split/android/client/SplitFactoryImpl.java b/src/main/java/io/split/android/client/SplitFactoryImpl.java
@@ -182,7 +182,7 @@ public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) {
 
         FlagSetsFilter flagSetsFilter = null;
         if (filters.get(SplitFilter.Type.BY_SET) != null) {
-            flagSetsFilter = new FlagSetsFilterImpl(new HashSet<>(filters.get(SplitFilter.Type.BY_SET).getValues()));
+            flagSetsFilter = new FlagSetsFilterImpl(filters.get(SplitFilter.Type.BY_SET).getValues());
         }
 
         SplitTaskFactory splitTaskFactory = new SplitTaskFactoryImpl(

diff --git a/src/main/java/io/split/android/client/SplitFilter.java b/src/main/java/io/split/android/client/SplitFilter.java
@@ -86,7 +86,7 @@ public static SplitFilter bySet(@NonNull List<String> values) {
 
     SplitFilter(Type type, List<String> values, SplitFilterValidator validator) {
         mType = type;
-        SplitFilterValidator.ValidationResult validationResult = validator.cleanup(values);
+        SplitFilterValidator.ValidationResult validationResult = validator.cleanup("SDK config", values);
         mValues = validationResult.getValues();
         mInvalidValueCount = validationResult.getInvalidValueCount();
     }

diff --git a/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java b/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java
@@ -12,7 +12,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import io.split.android.client.Evaluator;
 import io.split.android.client.EvaluatorImpl;
 import io.split.android.client.FlagSetsFilter;
 import io.split.android.client.SplitClient;

diff --git a/src/main/java/io/split/android/client/localhost/LocalhostSplitFactory.java b/src/main/java/io/split/android/client/localhost/LocalhostSplitFactory.java
@@ -85,7 +85,7 @@ public LocalhostSplitFactory(String key, Context context,
             if (!groupedFilters.isEmpty()) {
                 SplitFilter bySetFilter = groupedFilters.get(SplitFilter.Type.BY_SET);
                 if (bySetFilter != null) {
-                    flagSetsFilter = new FlagSetsFilterImpl(new HashSet<>(bySetFilter.getValues()));
+                    flagSetsFilter = new FlagSetsFilterImpl(bySetFilter.getValues());
                 }
             }
         }

diff --git a/src/main/java/io/split/android/client/localhost/LocalhostSplitsStorage.java b/src/main/java/io/split/android/client/localhost/LocalhostSplitsStorage.java
@@ -9,6 +9,7 @@
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -126,7 +127,7 @@ public void clear() {
 
     @NonNull
     @Override
-    public Set<String> getNamesByFlagSets(List<String> sets) {
+    public Set<String> getNamesByFlagSets(Collection<String> sets) {
         Set<String> namesToReturn = new HashSet<>();
         if (sets == null || sets.isEmpty()) {
             return namesToReturn;

diff --git a/src/main/java/io/split/android/client/service/splits/FeatureFlagProcessStrategy.java b/src/main/java/io/split/android/client/service/splits/FeatureFlagProcessStrategy.java
@@ -68,7 +68,7 @@ public void process(List<Split> activeFeatureFlags, List<Split> archivedFeatureF
         Set<String> newSets = new HashSet<>();
         for (String set : featureFlag.sets) {
             if (mFlagSetsFilter.intersect(set)) {
-                newSets.add(set); // Remove all sets that don't match the configured sets
+                newSets.add(set); // Add the flag set to the valid group
                 // Since the feature flag has at least one set that matches the configured sets,
                 // we process it according to its status
                 shouldArchive = false;
@@ -78,6 +78,7 @@ public void process(List<Split> activeFeatureFlags, List<Split> archivedFeatureF
         if (shouldArchive) {
             archivedFeatureFlags.add(featureFlag);
         } else {
+            // Replace the feature flag sets with the intersection of the configured sets and the feature flag sets
             featureFlag.sets = newSets;
             mStatusProcessStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag);
         }

diff --git a/src/main/java/io/split/android/client/service/workmanager/SplitsSyncWorker.java b/src/main/java/io/split/android/client/service/workmanager/SplitsSyncWorker.java
@@ -53,7 +53,7 @@ public SplitsSyncWorker(@NonNull Context context,
             TelemetryStorage telemetryStorage = StorageFactory.getTelemetryStorage(shouldRecordTelemetry);
 
             SplitChangeProcessor splitChangeProcessor = new SplitChangeProcessor(filter, (filter != null && filter.getType() == SplitFilter.Type.BY_SET) ?
-                    new FlagSetsFilterImpl(new HashSet<>(filter.getValues())) : null);
+                    new FlagSetsFilterImpl(filter.getValues()) : null);
 
             SplitsSyncHelper splitsSyncHelper = new SplitsSyncHelper(splitsFetcher, splitsStorage,
                     splitChangeProcessor,

diff --git a/src/main/java/io/split/android/client/storage/splits/SplitsStorage.java b/src/main/java/io/split/android/client/storage/splits/SplitsStorage.java
@@ -3,6 +3,7 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -35,5 +36,5 @@ public interface SplitsStorage {
     void clear();
 
     @NonNull
-    Set<String> getNamesByFlagSets(List<String> flagSets);
+    Set<String> getNamesByFlagSets(Collection<String> flagSets);
 }
diff --git a/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java b/src/main/java/io/split/android/client/storage/splits/SplitsStorageImpl.java
@@ -6,6 +6,7 @@
 import androidx.annotation.Nullable;
 import androidx.annotation.WorkerThread;
 
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -150,7 +151,7 @@ public void clear() {
 
     @NonNull
     @Override
-    public Set<String> getNamesByFlagSets(List<String> sets) {
+    public Set<String> getNamesByFlagSets(Collection<String> sets) {
         Set<String> namesToReturn = new HashSet<>();
         if (sets == null || sets.isEmpty()) {
             return namesToReturn;

diff --git a/src/main/java/io/split/android/client/validators/FlagSetsValidatorImpl.java b/src/main/java/io/split/android/client/validators/FlagSetsValidatorImpl.java
@@ -2,9 +2,12 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.TreeSet;
 
+import io.split.android.client.FlagSetsFilter;
 import io.split.android.client.utils.logger.Logger;
 
 public class FlagSetsValidatorImpl implements SplitFilterValidator {
@@ -19,7 +22,7 @@ public class FlagSetsValidatorImpl implements SplitFilterValidator {
      * @return list of unique alphanumerically ordered valid flag sets
      */
     @Override
-    public ValidationResult cleanup(List<String> values) {
+    public ValidationResult cleanup(String method, List<String> values) {
         if (values == null || values.isEmpty()) {
             return new ValidationResult(Collections.emptyList(), 0);
         }
@@ -34,20 +37,20 @@ public ValidationResult cleanup(List<String> values) {
             }
 
             if (set.trim().length() != set.length()) {
-                Logger.w("SDK config: Flag Set name " + set + " has extra whitespace, trimming");
+                Logger.w(method + ": Flag Set name " + set + " has extra whitespace, trimming");
                 set = set.trim();
             }
 
             if (!set.toLowerCase().equals(set)) {
-                Logger.w("SDK config: Flag Set name "+set+" should be all lowercase - converting string to lowercase");
+                Logger.w(method + ": Flag Set name "+set+" should be all lowercase - converting string to lowercase");
                 set = set.toLowerCase();
             }
 
             if (set.matches(FLAG_SET_REGEX)) {
                 cleanedUpSets.add(set);
             } else {
                 invalidValueCount++;
-                Logger.w("SDK config: you passed "+ set +", Flag Set must adhere to the regular expressions "+ FLAG_SET_REGEX +". This means a Flag Set must be start with a letter, be in lowercase, alphanumeric and have a max length of 50 characters. "+ set +" was discarded.");
+                Logger.w(method + ": you passed "+ set +", Flag Set must adhere to the regular expressions "+ FLAG_SET_REGEX +". This means a Flag Set must be start with a letter, be in lowercase, alphanumeric and have a max length of 50 characters. "+ set +" was discarded.");
             }
         }
 
@@ -58,4 +61,27 @@ public ValidationResult cleanup(List<String> values) {
     public boolean isValid(String value) {
         return value != null && value.trim().matches(FLAG_SET_REGEX);
     }
+
+    @Override
+    public Set<String> items(List<String> values, FlagSetsFilter flagSetsFilter) {
+        Set<String> setsToReturn = new HashSet<>();
+
+        if (values == null || values.isEmpty()) {
+            return setsToReturn;
+        }
+
+        for (String flagSet : values) {
+            if (!isValid(flagSet)) {
+                continue;
+            }
+
+            if (flagSetsFilter != null && !flagSetsFilter.intersect(flagSet)) {
+                continue;
+            }
+
+            setsToReturn.add(flagSet);
+        }
+
+        return setsToReturn;
+    }
 }
diff --git a/src/main/java/io/split/android/client/validators/SplitFilterValidator.java b/src/main/java/io/split/android/client/validators/SplitFilterValidator.java
@@ -1,13 +1,18 @@
 package io.split.android.client.validators;
 
 import java.util.List;
+import java.util.Set;
+
+import io.split.android.client.FlagSetsFilter;
 
 public interface SplitFilterValidator {
 
-    ValidationResult cleanup(List<String> values);
+    ValidationResult cleanup(String method, List<String> values);
 
     boolean isValid(String value);
 
+    Set<String> items(List<String> values, FlagSetsFilter flagSetsFilter);
+
     class ValidationResult {
 
         private final List<String> mValues;

diff --git a/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java b/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java
@@ -186,7 +186,7 @@ public Map<String, String> getTreatmentsByFlagSet(@NonNull String flagSet, @Null
         String validationTag = ValidationTag.GET_TREATMENTS_BY_FLAG_SET;
         Set<String> names = new HashSet<>();
         try {
-            names = getNamesFromSet(validationTag, Collections.singletonList(flagSet));
+            names = getNamesFromSet(Collections.singletonList(flagSet));
             if (isClientDestroyed) {
                 mValidationLogger.e(CLIENT_DESTROYED_MESSAGE, validationTag);
                 return controlTreatmentsForSplits(new ArrayList<>(names), validationTag);
@@ -211,7 +211,7 @@ public Map<String, String> getTreatmentsByFlagSets(@NonNull List<String> flagSet
         String validationTag = ValidationTag.GET_TREATMENTS_BY_FLAG_SETS;
         Set<String> names = new HashSet<>();
         try {
-            names = getNamesFromSet(validationTag, flagSets);
+            names = getNamesFromSet(flagSets);
             if (isClientDestroyed) {
                 mValidationLogger.e(CLIENT_DESTROYED_MESSAGE, validationTag);
                 return controlTreatmentsForSplits(new ArrayList<>(names), validationTag);
@@ -236,7 +236,7 @@ public Map<String, SplitResult> getTreatmentsWithConfigByFlagSet(@NonNull String
         String validationTag = ValidationTag.GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET;
         Set<String> names = new HashSet<>();
         try {
-            names = getNamesFromSet(validationTag, Collections.singletonList(flagSet));
+            names = getNamesFromSet(Collections.singletonList(flagSet));
             if (isClientDestroyed) {
                 mValidationLogger.e(CLIENT_DESTROYED_MESSAGE, validationTag);
                 return controlTreatmentsForSplitsWithConfig(new ArrayList<>(names), validationTag);
@@ -261,7 +261,7 @@ public Map<String, SplitResult> getTreatmentsWithConfigByFlagSets(@NonNull List<
         String validationTag = ValidationTag.GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS;
         Set<String> names = new HashSet<>();
         try {
-            names = getNamesFromSet(validationTag, flagSets);
+            names = getNamesFromSet(flagSets);
             if (isClientDestroyed) {
                 mValidationLogger.e(CLIENT_DESTROYED_MESSAGE, validationTag);
                 return controlTreatmentsForSplitsWithConfig(new ArrayList<>(names), validationTag);
@@ -402,30 +402,9 @@ private void recordLatency(Method treatment, long startTime) {
     }
 
     @NonNull
-    private Set<String> getNamesFromSet(String validationTag,
-                                        @NonNull List<String> flagSets) {
+    private Set<String> getNamesFromSet(@NonNull List<String> flagSets) {
 
-        if (flagSets == null) {
-            return new HashSet<>();
-        }
-
-        List<String> setsToEvaluate = new ArrayList<>();
-        for (String flagSet : flagSets) {
-            if (setsToEvaluate.contains(flagSet)) {
-                continue;
-            }
-
-            boolean isValid = mFlagSetsValidator.isValid(flagSet);
-            boolean isConfigured = mFlagSetsFilter.intersect(flagSet);
-
-            if (!isValid) {
-                mValidationLogger.e("you passed " + flagSet + " which is not valid.", validationTag);
-            } else if (!isConfigured) {
-                mValidationLogger.e("you passed " + flagSet + " which is not defined in the configuration.", validationTag);
-            } else {
-                setsToEvaluate.add(flagSet);
-            }
-        }
+        Set<String> setsToEvaluate = mFlagSetsValidator.items(flagSets, mFlagSetsFilter);
 
         if (setsToEvaluate.isEmpty()) {
             return new HashSet<>();