Skip to content

Commit

Permalink
Filter grouping refactor (#537)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gthea
gthea authored Sep 12, 2023
2 parents 406463b + 13cae88 commit 375f8f6
Show file tree
Hide file tree
Showing 11 changed files with 115 additions and 80 deletions.
28 changes: 14 additions & 14 deletions src/main/java/io/split/android/client/FilterBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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()) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
}
}
12 changes: 9 additions & 3 deletions src/main/java/io/split/android/client/FilterGrouper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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;
}
}
36 changes: 20 additions & 16 deletions src/main/java/io/split/android/client/SplitFactoryHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

}

Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/io/split/android/client/SplitFactoryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/io/split/android/client/localhost/LocalhostSplitFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
}

Expand Down
45 changes: 27 additions & 18 deletions src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
}
6 changes: 3 additions & 3 deletions src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
36 changes: 23 additions & 13 deletions src/test/java/io/split/android/client/FilterGrouperTest.java
View file
Original file line number Diff line number Diff line change
@@ -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));
}

}
Loading

0 comments on commit 375f8f6

Please sign in to comment.