Filter grouping refactor

src/main/java/io/split/android/client/FilterBuilder.java

@@ -6,14 +6,14 @@
 import androidx.annotation.Nullable;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Map;
 import java.util.SortedSet;
+import java.util.TreeMap;
 import java.util.TreeSet;
 
 import io.split.android.client.utils.logger.Logger;
-import io.split.android.client.utils.StringHelper;
 
 public class FilterBuilder {
 
@@ -30,18 +30,15 @@ public FilterBuilder(List<SplitFilter> filters) {
     }
 
     public String buildQueryString() {
-
-        if (mFilters.size() == 0) {
+        if (mFilters.isEmpty()) {
             return "";
         }
 
-        StringHelper stringHelper = new StringHelper();
         StringBuilder queryString = new StringBuilder();
 
-        List<SplitFilter> sortedFilters = getGroupedFilter();
-        Collections.sort(sortedFilters, new SplitFilterComparator());
+        Map<SplitFilter.Type, SplitFilter> sortedFilters = getGroupedFilter();
 
-        for (SplitFilter splitFilter : sortedFilters) {
+        for (SplitFilter splitFilter : sortedFilters.values()) {
             SplitFilter.Type filterType = splitFilter.getType();
             SortedSet<String> deduptedValues = new TreeSet<>(splitFilter.getValues());
             if (deduptedValues.size() < splitFilter.getValues().size()) {
@@ -56,15 +53,18 @@ public String buildQueryString() {
             queryString.append("&");
             queryString.append(filterType.queryStringField());
             queryString.append("=");
-            queryString.append(stringHelper.join(",", deduptedValues));
+            queryString.append(String.join(",", deduptedValues));
         }
 
         return queryString.toString();
     }
 
     @NonNull
-    public List<SplitFilter> getGroupedFilter() {
-        return new ArrayList<>(mFilterGrouper.group(mFilters));
+    public Map<SplitFilter.Type, SplitFilter> getGroupedFilter() {
+        TreeMap<SplitFilter.Type, SplitFilter> sortedFilters = new TreeMap<>(new SplitFilterTypeComparator());
+        sortedFilters.putAll(mFilterGrouper.group(mFilters));
+
+        return sortedFilters;
     }
 
     private void addFilters(List<SplitFilter> filters) {
@@ -103,10 +103,10 @@ private void validateFilterSize(SplitFilter.Type type, int size) {
         }
     }
 
-    private static class SplitFilterComparator implements Comparator<SplitFilter> {
+    private static class SplitFilterTypeComparator implements Comparator<SplitFilter.Type> {
         @Override
-        public int compare(SplitFilter o1, SplitFilter o2) {
-            return o1.getType().compareTo(o2.getType());
+        public int compare(SplitFilter.Type o1, SplitFilter.Type o2) {
+            return o1.compareTo(o2);
         }
     }
 }

src/main/java/io/split/android/client/FilterGrouper.java

@@ -7,7 +7,12 @@
 
 class FilterGrouper {
 
-    List<SplitFilter> group(List<SplitFilter> filters) {
+    /**
+     * Groups filters by type
+     * @param filters list of filters to group
+     * @return map of grouped filters. The key is the filter type, the value is the filter
+     */
+    Map<SplitFilter.Type, SplitFilter> group(List<SplitFilter> filters) {
         Map<SplitFilter.Type, List<String>> groupedValues = new HashMap<>();
         for (SplitFilter filter : filters) {
             List<String> groupValues = groupedValues.get(filter.getType());
@@ -18,12 +23,13 @@ List<SplitFilter> group(List<SplitFilter> filters) {
             groupValues.addAll(filter.getValues());
         }
 
-        List<SplitFilter> groupedFilters = new ArrayList<>();
+        Map<SplitFilter.Type, SplitFilter> groupedFilters = new HashMap<>();
         for (Map.Entry<SplitFilter.Type, List<String>> filterEntry : groupedValues.entrySet()) {
             if (filterEntry.getValue().size() > 0) {
-                groupedFilters.add(new SplitFilter(filterEntry.getKey(), filterEntry.getValue()));
+                groupedFilters.put(filterEntry.getKey(), new SplitFilter(filterEntry.getKey(), filterEntry.getValue()));
             }
         }
+
         return groupedFilters;
     }
 }

src/main/java/io/split/android/client/SplitFactoryHelper.java

@@ -10,9 +10,8 @@
 import java.io.File;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.BlockingQueue;
@@ -189,9 +188,12 @@ SplitApiFacade buildApiFacade(SplitClientConfig splitClientConfig,
     }
 
     WorkManagerWrapper buildWorkManagerWrapper(Context context, SplitClientConfig splitClientConfig,
-                                               String apiKey, String databaseName, List<SplitFilter> filters) {
+                                               String apiKey, String databaseName, Map<SplitFilter.Type, SplitFilter> filters) {
+        SplitFilter filter = filters.get(SplitFilter.Type.BY_SET) != null ?
+                filters.get(SplitFilter.Type.BY_SET) :
+                filters.get(SplitFilter.Type.BY_NAME);
         return new WorkManagerWrapper(
-                WorkManager.getInstance(context), splitClientConfig, apiKey, databaseName, filters);
+                WorkManager.getInstance(context), splitClientConfig, apiKey, databaseName, filter);
 
     }
 
@@ -414,27 +416,29 @@ SplitUpdatesWorker getSplitUpdatesWorker(SplitClientConfig config,
         return null;
     }
 
-    Pair<Pair<List<SplitFilter>, String>, Set<String>> getFilterConfiguration(SyncConfig syncConfig) {
+    Pair<Map<SplitFilter.Type, SplitFilter>, String> getFilterConfiguration(SyncConfig syncConfig) {
         String splitsFilterQueryString = null;
-        List<SplitFilter> groupedFilters = new ArrayList<>();
-        Set<String> configuredFlagSets = new HashSet<>();
+        Map<SplitFilter.Type, SplitFilter> groupedFilters = new HashMap<>();
 
         if (syncConfig != null) {
             FilterBuilder filterBuilder = new FilterBuilder(syncConfig.getFilters());
             groupedFilters = filterBuilder.getGroupedFilter();
             splitsFilterQueryString = filterBuilder.buildQueryString();
+        }
 
-            if (!groupedFilters.isEmpty()) {
-                SplitFilter splitFilter = groupedFilters.get(0);
+        return new Pair<>(groupedFilters, splitsFilterQueryString);
+    }
 
-                // In the case of BY_SET, all filters will be grouped into one with the {@link SplitFilter.Type#BY_SET} type
-                if (splitFilter != null && splitFilter.getType() == SplitFilter.Type.BY_SET) {
-                    configuredFlagSets.addAll(splitFilter.getValues());
-                }
-            }
+    @NonNull
+    Set<String> getConfiguredFlagSets(Map<SplitFilter.Type, SplitFilter> filters) {
+        Set<String> configuredFlagSets;
+        SplitFilter flagSetSplitFilter = filters.get(SplitFilter.Type.BY_SET);
+        if (flagSetSplitFilter != null) {
+            configuredFlagSets = new HashSet<>(flagSetSplitFilter.getValues());
+        } else {
+            configuredFlagSets = new HashSet<>();
         }
-
-        return new Pair<>(new Pair<>(groupedFilters, splitsFilterQueryString), configuredFlagSets);
+        return configuredFlagSets;
     }
 
     private TelemetryStorage getTelemetryStorage(boolean shouldRecordTelemetry, TelemetryStorage telemetryStorage) {

src/main/java/io/split/android/client/SplitFactoryImpl.java

@@ -7,7 +7,9 @@
 
 import java.net.URISyntaxException;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import io.split.android.client.api.Key;
@@ -171,10 +173,9 @@ public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) {
         mStorageContainer = factoryHelper.buildStorageContainer(config.userConsent(),
                 splitDatabase, config.shouldRecordTelemetry(), splitCipher, telemetryStorage);
 
-        Pair<Pair<List<SplitFilter>, String>, Set<String>> filtersConfig = factoryHelper.getFilterConfiguration(config.syncConfig());
-        List<SplitFilter> filters = filtersConfig.first.first;
-        String splitsFilterQueryStringFromConfig = filtersConfig.first.second;
-        Set<String> configuredFlagSets = filtersConfig.second;
+        Pair<Map<SplitFilter.Type, SplitFilter>, String> filtersConfig = factoryHelper.getFilterConfiguration(config.syncConfig());
+        Map<SplitFilter.Type, SplitFilter> filters = filtersConfig.first;
+        String splitsFilterQueryStringFromConfig = filtersConfig.second;
 
         SplitApiFacade splitApiFacade = factoryHelper.buildApiFacade(
                 config, defaultHttpClient, splitsFilterQueryStringFromConfig);
@@ -269,7 +270,7 @@ public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) {
                 telemetrySynchronizer, mStorageContainer, splitTaskExecutor, splitApiFacade,
                 validationLogger, keyValidator, customerImpressionListener,
                 streamingComponents.getPushNotificationManager(), componentsRegister, workManagerWrapper,
-                eventsTracker, configuredFlagSets);
+                eventsTracker, factoryHelper.getConfiguredFlagSets(filters));
         mDestroyer = new Runnable() {
             public void run() {
                 Logger.w("Shutdown called for split");

src/main/java/io/split/android/client/localhost/LocalhostSplitFactory.java

@@ -9,6 +9,7 @@
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import io.split.android.client.FilterBuilder;
@@ -76,11 +77,14 @@ public LocalhostSplitFactory(String key, Context context,
 
         Set<String> configuredSets = new HashSet<>();
         if (config.syncConfig() != null) {
-            List<SplitFilter> groupedFilters = new FilterBuilder(config.syncConfig().getFilters())
+            Map<SplitFilter.Type, SplitFilter> groupedFilters = new FilterBuilder(config.syncConfig().getFilters())
                     .getGroupedFilter();
 
-            if (!groupedFilters.isEmpty() && groupedFilters.get(0) != null && groupedFilters.get(0).getType() == SplitFilter.Type.BY_SET) {
-                configuredSets.addAll(groupedFilters.get(0).getValues());
+            if (!groupedFilters.isEmpty()) {
+                SplitFilter bySetFilter = groupedFilters.get(SplitFilter.Type.BY_SET);
+                if (bySetFilter != null) {
+                    configuredSets.addAll(bySetFilter.getValues());
+                }
             }
         }
 

src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java

@@ -66,7 +66,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig,
                                 @NonNull SplitStorageContainer splitStorageContainer,
                                 @Nullable String splitsFilterQueryString,
                                 ISplitEventsManager eventsManager,
-                                @Nullable List<SplitFilter> filters,
+                                @Nullable Map<SplitFilter.Type, SplitFilter> filters,
                                 @Nullable TestingConfig testingConfig) {
 
         mSplitClientConfig = checkNotNull(splitClientConfig);
@@ -91,23 +91,8 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig,
                     mTelemetryRuntimeProducer);
         }
 
-        mFilters = (filters == null) ? new ArrayList<>() : filters;
-
-        int flagSetCount = 0;
-        int invalidFlagSetCount = 0;
-        if (!mFilters.isEmpty() && mFilters.get(0) != null && mFilters.get(0).getType() == SplitFilter.Type.BY_SET) {
-            flagSetCount = mFilters.get(0).getValues().size();
-            invalidFlagSetCount = mFilters.get(0).getInvalidValueCount();
-        }
-
-        mTelemetryTaskFactory = new TelemetryTaskFactoryImpl(mSplitApiFacade.getTelemetryConfigRecorder(),
-                mSplitApiFacade.getTelemetryStatsRecorder(),
-                telemetryStorage,
-                splitClientConfig,
-                mSplitsStorageContainer.getSplitsStorage(),
-                mSplitsStorageContainer.getMySegmentsStorageContainer(),
-                flagSetCount,
-                invalidFlagSetCount);
+        mFilters = (filters == null) ? new ArrayList<>() : new ArrayList<>(filters.values());
+        mTelemetryTaskFactory = initializeTelemetryTaskFactory(splitClientConfig, filters, telemetryStorage);
     }
 
     @Override
@@ -209,4 +194,28 @@ public TelemetryStatsRecorderTask getTelemetryStatsRecorderTask() {
     public SplitInPlaceUpdateTask createSplitsUpdateTask(Split featureFlag, long since) {
         return new SplitInPlaceUpdateTask(mSplitsStorageContainer.getSplitsStorage(), mSplitChangeProcessor, mEventsManager, mTelemetryRuntimeProducer, featureFlag, since);
     }
+
+    @NonNull
+    private TelemetryTaskFactory initializeTelemetryTaskFactory(@NonNull SplitClientConfig splitClientConfig, @Nullable Map<SplitFilter.Type, SplitFilter> filters, TelemetryStorage telemetryStorage) {
+        final TelemetryTaskFactory mTelemetryTaskFactory;
+        int flagSetCount = 0;
+        int invalidFlagSetCount = 0;
+        if (filters != null && !filters.isEmpty()) {
+            SplitFilter bySetFilter = filters.get(SplitFilter.Type.BY_SET);
+            if (bySetFilter != null) {
+                flagSetCount = bySetFilter.getValues().size();
+                invalidFlagSetCount = bySetFilter.getInvalidValueCount();
+            }
+        }
+
+        mTelemetryTaskFactory = new TelemetryTaskFactoryImpl(mSplitApiFacade.getTelemetryConfigRecorder(),
+                mSplitApiFacade.getTelemetryStatsRecorder(),
+                telemetryStorage,
+                splitClientConfig,
+                mSplitsStorageContainer.getSplitsStorage(),
+                mSplitsStorageContainer.getMySegmentsStorageContainer(),
+                flagSetCount,
+                invalidFlagSetCount);
+        return mTelemetryTaskFactory;
+    }
 }

src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java

@@ -7,11 +7,11 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 
 import io.split.android.client.SplitFilter;
 import io.split.android.client.dtos.Split;
 import io.split.android.client.dtos.SplitChange;
-import io.split.android.client.dtos.Status;
 import io.split.android.client.storage.splits.ProcessedSplitChange;
 
 public class SplitChangeProcessor {
@@ -25,12 +25,12 @@ public class SplitChangeProcessor {
         this((SplitFilter) null);
     }
 
-    public SplitChangeProcessor(@Nullable List<SplitFilter> filters) {
+    public SplitChangeProcessor(@Nullable Map<SplitFilter.Type, SplitFilter> filters) {
         // We're only supporting one filter type
         if (filters == null || filters.isEmpty()) {
             mSplitFilter = null;
         } else {
-            mSplitFilter = filters.get(0);
+            mSplitFilter = filters.values().iterator().next();
         }
 
         mStatusProcessStrategy = new StatusProcessStrategy();

src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java

@@ -53,14 +53,14 @@ public WorkManagerWrapper(@NonNull WorkManager workManager,
                               @NonNull SplitClientConfig splitClientConfig,
                               @NonNull String apiKey,
                               @NonNull String databaseName,
-                              @Nullable List<SplitFilter> filters) {
+                              @Nullable SplitFilter filter) {
         mWorkManager = checkNotNull(workManager);
         mDatabaseName = checkNotNull(databaseName);
         mSplitClientConfig = checkNotNull(splitClientConfig);
         mApiKey = checkNotNull(apiKey);
         mShouldLoadFromLocal = new HashSet<>();
         mConstraints = buildConstraints();
-        mFilter = (filters != null && filters.size() == 1) ? filters.get(0) : null;
+        mFilter = filter;
     }
 
     public void setFetcherExecutionListener(SplitTaskExecutionListener fetcherExecutionListener) {

src/test/java/io/split/android/client/FilterGrouperTest.java

@@ -1,30 +1,40 @@
 package io.split.android.client;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import org.junit.Assert;
 import org.junit.Test;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
 public class FilterGrouperTest {
 
     FilterGrouper mFilterGrouper = new FilterGrouper();
 
     @Test
     public void groupingFilters() {
-        List<SplitFilter> ungropedFilters = new ArrayList<>();
-        ungropedFilters.add(SplitFilter.byName(Arrays.asList("f1", "f2", "f3")));
-        ungropedFilters.add(SplitFilter.byName(Arrays.asList("f2", "f3", "f4")));
-        ungropedFilters.add(SplitFilter.byName(Arrays.asList("f4", "f5", "f6")));
-        ungropedFilters.add(SplitFilter.byPrefix(Arrays.asList("f1", "f2", "f3")));
-        ungropedFilters.add(SplitFilter.byPrefix(Arrays.asList("f2", "f3", "f4")));
-        ungropedFilters.add(SplitFilter.byPrefix(Arrays.asList("f4", "f5", "f6")));
-
-        List<SplitFilter> groupedFiltes = mFilterGrouper.group(ungropedFilters);
-
-        /// This compoe
-        Assert.assertEquals(2, groupedFiltes.size());
+        List<SplitFilter> ungroupedFilters = new ArrayList<>();
+        ungroupedFilters.add(SplitFilter.byName(Arrays.asList("f1", "f2", "f3")));
+        ungroupedFilters.add(SplitFilter.byName(Arrays.asList("f2", "f3", "f4")));
+        ungroupedFilters.add(SplitFilter.byName(Arrays.asList("f4", "f5", "f6")));
+        ungroupedFilters.add(SplitFilter.byPrefix(Arrays.asList("f1", "f2", "f3")));
+        ungroupedFilters.add(SplitFilter.byPrefix(Arrays.asList("f2", "f3", "f4")));
+        ungroupedFilters.add(SplitFilter.byPrefix(Arrays.asList("f4", "f5", "f6")));
+        ungroupedFilters.add(SplitFilter.bySet(Arrays.asList("f1", "f2", "f3")));
+        ungroupedFilters.add(SplitFilter.bySet(Arrays.asList("f2", "f3", "f4")));
+        ungroupedFilters.add(SplitFilter.bySet(Arrays.asList("f4", "f5", "f6")));
+
+        Map<SplitFilter.Type, SplitFilter> groupedFilters = mFilterGrouper.group(ungroupedFilters);
+
+        // this class only merges filters of the same type
+        assertEquals(3, groupedFilters.size());
+        assertTrue(groupedFilters.containsKey(SplitFilter.Type.BY_NAME));
+        assertTrue(groupedFilters.containsKey(SplitFilter.Type.BY_PREFIX));
+        assertTrue(groupedFilters.containsKey(SplitFilter.Type.BY_SET));
     }
-
 }