Modify cache filtering task (#525)

splitio · Aug 29, 2023 · 47aadb3 · 47aadb3
2 parents c67a6d7 + 4f22c8f
commit 47aadb3
Show file tree

Hide file tree

Showing 8 changed files with 199 additions and 88 deletions.
diff --git a/src/main/java/io/split/android/client/FilterBuilder.java b/src/main/java/io/split/android/client/FilterBuilder.java
@@ -1,5 +1,10 @@
 package io.split.android.client;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -13,42 +18,15 @@
 public class FilterBuilder {
 
     private final List<SplitFilter> mFilters = new ArrayList<>();
-    private final FilterGrouper mFilterGrouper = new FilterGrouper();
+    private final FilterGrouper mFilterGrouper;
 
-    static private class SplitFilterComparator implements Comparator<SplitFilter> {
-        @Override
-        public int compare(SplitFilter o1, SplitFilter o2) {
-            return o1.getType().compareTo(o2.getType());
-        }
+    public FilterBuilder(List<SplitFilter> filters) {
+        this(new FilterGrouper(), filters);
     }
 
-    public FilterBuilder addFilters(List<SplitFilter> filters) {
-        if (filters == null) {
-            return this;
-        }
-
-        boolean containsSetsFilter = false;
-        for (SplitFilter filter : filters) {
-            if (filter == null) {
-                continue;
-            }
-
-            if (filter.getType() == SplitFilter.Type.BY_SET) {
-                // BY_SET filter has precedence over other filters, so we remove all other filters
-                // and only add BY_SET filters
-                if (!containsSetsFilter) {
-                    mFilters.clear();
-                    containsSetsFilter = true;
-                }
-                mFilters.add(filter);
-            }
-
-            if (!containsSetsFilter) {
-                mFilters.add(filter);
-            }
-        }
-
-        return this;
+    FilterBuilder(@NonNull FilterGrouper filterGrouper, @Nullable List<SplitFilter> filters) {
+        mFilterGrouper = checkNotNull(filterGrouper);
+        addFilters(filters);
     }
 
     public String buildQueryString() {
@@ -60,7 +38,7 @@ public String buildQueryString() {
         StringHelper stringHelper = new StringHelper();
         StringBuilder queryString = new StringBuilder();
 
-        List<SplitFilter> sortedFilters = new ArrayList<>(mFilterGrouper.group(mFilters));
+        List<SplitFilter> sortedFilters = getGroupedFilter();
         Collections.sort(sortedFilters, new SplitFilterComparator());
 
         for (SplitFilter splitFilter : sortedFilters) {
@@ -84,6 +62,38 @@ public String buildQueryString() {
         return queryString.toString();
     }
 
+    @NonNull
+    public ArrayList<SplitFilter> getGroupedFilter() {
+        return new ArrayList<>(mFilterGrouper.group(mFilters));
+    }
+
+    private void addFilters(List<SplitFilter> filters) {
+        if (filters == null) {
+            return;
+        }
+
+        boolean containsSetsFilter = false;
+        for (SplitFilter filter : filters) {
+            if (filter == null) {
+                continue;
+            }
+
+            if (filter.getType() == SplitFilter.Type.BY_SET) {
+                // BY_SET filter has precedence over other filters, so we remove all other filters
+                // and only add BY_SET filters
+                if (!containsSetsFilter) {
+                    mFilters.clear();
+                    containsSetsFilter = true;
+                }
+                mFilters.add(filter);
+            }
+
+            if (!containsSetsFilter) {
+                mFilters.add(filter);
+            }
+        }
+    }
+
     private void validateFilterSize(SplitFilter.Type type, int size) {
         if (size > type.maxValuesCount()) {
             String message = "Error: " + type.maxValuesCount() + " different split " + type.queryStringField() +
@@ -92,4 +102,11 @@ private void validateFilterSize(SplitFilter.Type type, int size) {
             throw new IllegalArgumentException(message);
         }
     }
+
+    private static class SplitFilterComparator implements Comparator<SplitFilter> {
+        @Override
+        public int compare(SplitFilter o1, SplitFilter o2) {
+            return o1.getType().compareTo(o2.getType());
+        }
+    }
 }
diff --git a/src/main/java/io/split/android/client/FilterGrouper.java b/src/main/java/io/split/android/client/FilterGrouper.java
@@ -5,8 +5,9 @@
 import java.util.List;
 import java.util.Map;
 
-public class FilterGrouper {
-    public List<SplitFilter> group(List<SplitFilter> filters) {
+class FilterGrouper {
+
+    List<SplitFilter> group(List<SplitFilter> filters) {
         Map<SplitFilter.Type, List<String>> groupedValues = new HashMap<>();
         for (SplitFilter filter : filters) {
             List<String> groupValues = groupedValues.get(filter.getType());

diff --git a/src/main/java/io/split/android/client/SplitFactoryHelper.java b/src/main/java/io/split/android/client/SplitFactoryHelper.java
@@ -158,14 +158,6 @@ SplitStorageContainer buildStorageContainer(UserConsent userConsentStatus,
                 getTelemetryStorage(shouldRecordTelemetry, telemetryStorage));
     }
 
-    String buildSplitsFilterQueryString(SplitClientConfig config) {
-        SyncConfig syncConfig = config.syncConfig();
-        if (syncConfig != null) {
-            return new FilterBuilder().addFilters(syncConfig.getFilters()).buildQueryString();
-        }
-        return null;
-    }
-
     SplitApiFacade buildApiFacade(SplitClientConfig splitClientConfig,
                                   HttpClient httpClient,
                                   String splitsFilterQueryString) throws URISyntaxException {

diff --git a/src/main/java/io/split/android/client/SplitFactoryImpl.java b/src/main/java/io/split/android/client/SplitFactoryImpl.java
@@ -170,14 +170,21 @@ public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) {
         mStorageContainer = factoryHelper.buildStorageContainer(config.userConsent(),
                 splitDatabase, config.shouldRecordTelemetry(), splitCipher, telemetryStorage);
 
-        String splitsFilterQueryString = factoryHelper.buildSplitsFilterQueryString(config);
+        SyncConfig syncConfig = config.syncConfig();
+        String splitsFilterQueryString = null;
+        List<SplitFilter> filters = null;
+        if (syncConfig != null) {
+            FilterBuilder filterBuilder = new FilterBuilder(syncConfig.getFilters());
+            filters = filterBuilder.getGroupedFilter();
+            splitsFilterQueryString = filterBuilder.buildQueryString();
+        }
 
         SplitApiFacade splitApiFacade = factoryHelper.buildApiFacade(
                 config, defaultHttpClient, splitsFilterQueryString);
 
         SplitTaskFactory splitTaskFactory = new SplitTaskFactoryImpl(
                 config, splitApiFacade, mStorageContainer, splitsFilterQueryString, mEventsManagerCoordinator,
-                testingConfig);
+                filters, testingConfig);
 
         cleanUpDabase(splitTaskExecutor, splitTaskFactory);
         WorkManagerWrapper workManagerWrapper = factoryHelper.buildWorkManagerWrapper(context, config, apiToken, databaseName);

diff --git a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java
@@ -11,7 +11,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import io.split.android.client.FilterGrouper;
 import io.split.android.client.SplitClientConfig;
 import io.split.android.client.SplitFilter;
 import io.split.android.client.TestingConfig;
@@ -58,13 +57,16 @@ public class SplitTaskFactoryImpl implements SplitTaskFactory {
     private final TelemetryTaskFactory mTelemetryTaskFactory;
     private final SplitChangeProcessor mSplitChangeProcessor;
     private final TelemetryRuntimeProducer mTelemetryRuntimeProducer;
+    @Nullable
+    private final List<SplitFilter> mFilters;
 
     @SuppressLint("VisibleForTests")
     public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig,
                                 @NonNull SplitApiFacade splitApiFacade,
                                 @NonNull SplitStorageContainer splitStorageContainer,
                                 @Nullable String splitsFilterQueryString,
                                 ISplitEventsManager eventsManager,
+                                @Nullable List<SplitFilter> filters,
                                 @Nullable TestingConfig testingConfig) {
 
         mSplitClientConfig = checkNotNull(splitClientConfig);
@@ -95,6 +97,8 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig,
                 splitClientConfig,
                 mSplitsStorageContainer.getSplitsStorage(),
                 mSplitsStorageContainer.getMySegmentsStorageContainer());
+
+        mFilters = filters;
     }
 
     @Override
@@ -141,9 +145,8 @@ public SplitsUpdateTask createSplitsUpdateTask(long since) {
 
     @Override
     public FilterSplitsInCacheTask createFilterSplitsInCacheTask() {
-        List<SplitFilter> filters = new FilterGrouper().group(mSplitClientConfig.syncConfig().getFilters());
         return new FilterSplitsInCacheTask(mSplitsStorageContainer.getPersistentSplitsStorage(),
-                filters, mSplitsFilterQueryString);
+                mFilters, mSplitsFilterQueryString);
     }
 
     @Override

diff --git a/src/main/java/io/split/android/client/service/splits/FilterSplitsInCacheTask.java b/src/main/java/io/split/android/client/service/splits/FilterSplitsInCacheTask.java
@@ -37,14 +37,18 @@ public FilterSplitsInCacheTask(@NonNull PersistentSplitsStorage splitsStorage,
     @NonNull
     public SplitTaskExecutionInfo execute() {
 
-        if(!queryStringHasChanged()) {
+        if (!queryStringHasChanged()) {
             return SplitTaskExecutionInfo.success(SplitTaskType.FILTER_SPLITS_CACHE);
         }
 
         Set<String> namesToKeep = new HashSet<>();
         Set<String> prefixesToKeep = new HashSet<>();
+        Set<String> setsToKeep = new HashSet<>();
         for (SplitFilter filter : mSplitsFilter) {
             switch (filter.getType()) {
+                case BY_SET:
+                    setsToKeep.addAll(filter.getValues());
+                    break;
                 case BY_NAME:
                     namesToKeep.addAll(filter.getValues());
                     break;
@@ -60,14 +64,38 @@ public SplitTaskExecutionInfo execute() {
         List<Split> splitsInCache = mSplitsStorage.getAll();
         for (Split split : splitsInCache) {
             String splitName = split.name;
+
+            // Since sets filter takes precedence,
+            // if setsToKeep is not empty, we only keep splits that belong to the sets in setsToKeep
+            if (!setsToKeep.isEmpty()) {
+                boolean keepSplit = false;
+                if (split.sets != null) {
+                    for (String set : split.sets) {
+                        if (setsToKeep.contains(set)) {
+                            keepSplit = true;
+                            break;
+                        }
+                    }
+                }
+
+                if (!keepSplit) {
+                    splitsToDelete.add(splitName);
+                }
+
+                continue;
+            }
+
+            // legacy behaviour for names and prefix filters
             String splitPrefix = getPrefix(splitName);
             if (!namesToKeep.contains(split.name) && (splitPrefix == null || !prefixesToKeep.contains(splitPrefix))) {
                 splitsToDelete.add(splitName);
             }
         }
-        if (splitsToDelete.size() > 0) {
+
+        if (!splitsToDelete.isEmpty()) {
             mSplitsStorage.delete(splitsToDelete);
         }
+
         return SplitTaskExecutionInfo.success(SplitTaskType.FILTER_SPLITS_CACHE);
     }