From 844452d7271e1484ba91f958d053e3b4fc39d536 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 6 Sep 2023 17:51:15 -0300 Subject: [PATCH 1/6] byName & bySet processing of changes --- .../android/client/SplitFactoryHelper.java | 4 +- .../android/client/SplitFactoryImpl.java | 4 +- .../client/service/ServiceConstants.java | 3 +- .../executor/SplitTaskFactoryImpl.java | 3 +- .../service/splits/SplitChangeProcessor.java | 45 ++++++++++++---- .../synchronizer/WorkManagerWrapper.java | 11 ++-- .../service/workmanager/SplitsSyncWorker.java | 33 +++++++++--- .../splits/SplitChangeProcessorTest.java | 52 +++++++++++++++++-- .../synchronizer/WorkManagerWrapperTest.java | 7 ++- 9 files changed, 124 insertions(+), 38 deletions(-) rename src/test/java/io/split/android/{engine => client/service}/splits/SplitChangeProcessorTest.java (79%) diff --git a/src/main/java/io/split/android/client/SplitFactoryHelper.java b/src/main/java/io/split/android/client/SplitFactoryHelper.java index 76393dc34..e4c2fd4e7 100644 --- a/src/main/java/io/split/android/client/SplitFactoryHelper.java +++ b/src/main/java/io/split/android/client/SplitFactoryHelper.java @@ -189,9 +189,9 @@ SplitApiFacade buildApiFacade(SplitClientConfig splitClientConfig, } WorkManagerWrapper buildWorkManagerWrapper(Context context, SplitClientConfig splitClientConfig, - String apiKey, String databaseName, Set configuredFlagSets) { + String apiKey, String databaseName, List filters) { return new WorkManagerWrapper( - WorkManager.getInstance(context), splitClientConfig, apiKey, databaseName, configuredFlagSets); + WorkManager.getInstance(context), splitClientConfig, apiKey, databaseName, (filters != null && filters.size() == 1) ? filters.get(0) : null); } diff --git a/src/main/java/io/split/android/client/SplitFactoryImpl.java b/src/main/java/io/split/android/client/SplitFactoryImpl.java index a6404513a..af1cecedc 100644 --- a/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -181,10 +181,10 @@ public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) { SplitTaskFactory splitTaskFactory = new SplitTaskFactoryImpl( config, splitApiFacade, mStorageContainer, splitsFilterQueryStringFromConfig, mEventsManagerCoordinator, - filters, configuredFlagSets, testingConfig); + filters, testingConfig); cleanUpDabase(splitTaskExecutor, splitTaskFactory); - WorkManagerWrapper workManagerWrapper = factoryHelper.buildWorkManagerWrapper(context, config, apiToken, databaseName, configuredFlagSets); + WorkManagerWrapper workManagerWrapper = factoryHelper.buildWorkManagerWrapper(context, config, apiToken, databaseName, filters); SplitSingleThreadTaskExecutor splitSingleThreadTaskExecutor = new SplitSingleThreadTaskExecutor(); ImpressionManager impressionManager = new StrategyImpressionManager(factoryHelper.getImpressionStrategy(splitTaskExecutor, splitTaskFactory, mStorageContainer, config)); diff --git a/src/main/java/io/split/android/client/service/ServiceConstants.java b/src/main/java/io/split/android/client/service/ServiceConstants.java index f68dd503a..1ae1ffc41 100644 --- a/src/main/java/io/split/android/client/service/ServiceConstants.java +++ b/src/main/java/io/split/android/client/service/ServiceConstants.java @@ -29,7 +29,8 @@ public class ServiceConstants { public static final String WORKER_PARAM_UNIQUE_KEYS_PER_PUSH = "unique_keys_per_push"; public static final String WORKER_PARAM_UNIQUE_KEYS_ESTIMATED_SIZE_IN_BYTES = "unique_keys_estimated_size_in_bytes"; public static final String WORKER_PARAM_ENCRYPTION_ENABLED = "encryptionEnabled"; - public static final String WORKER_PARAM_CONFIGURED_SETS = "configuredSets"; + public static final String WORKER_PARAM_CONFIGURED_FILTER_VALUES = "configuredFilterValues"; + public static final String WORKER_PARAM_CONFIGURED_FILTER_TYPE = "configuredFilterType"; public static final long LAST_SEEN_IMPRESSION_CACHE_SIZE = 500; public static final int MY_SEGMENT_V2_DATA_SIZE = 1024 * 10;// bytes 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 index 142d8f14b..e828a1cdb 100644 --- a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java +++ b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java @@ -67,7 +67,6 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig, @Nullable String splitsFilterQueryString, ISplitEventsManager eventsManager, @Nullable List filters, - @NonNull Set configuredFlagSets, @Nullable TestingConfig testingConfig) { mSplitClientConfig = checkNotNull(splitClientConfig); @@ -75,7 +74,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig, mSplitsStorageContainer = checkNotNull(splitStorageContainer); mSplitsFilterQueryStringFromConfig = splitsFilterQueryString; mEventsManager = eventsManager; - mSplitChangeProcessor = new SplitChangeProcessor(configuredFlagSets); + mSplitChangeProcessor = new SplitChangeProcessor(filters); TelemetryStorage telemetryStorage = mSplitsStorageContainer.getTelemetryStorage(); mTelemetryRuntimeProducer = telemetryStorage; diff --git a/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java b/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java index 6f004b77b..0b88c0b25 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java +++ b/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java @@ -1,15 +1,14 @@ package io.split.android.client.service.splits; -import static com.google.common.base.Preconditions.checkNotNull; - import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Set; +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; @@ -17,15 +16,24 @@ public class SplitChangeProcessor { - private final Set mConfiguredSets; + private final SplitFilter mSplitFilter; @VisibleForTesting SplitChangeProcessor() { - this(Collections.emptySet()); + this((SplitFilter) null); + } + + public SplitChangeProcessor(@Nullable List filters) { + // We're only supporting one filter type + if (filters == null || filters.isEmpty()) { + mSplitFilter = null; + } else { + mSplitFilter = filters.get(0); + } } - public SplitChangeProcessor(@NonNull Set configuredSets) { - mConfiguredSets = checkNotNull(configuredSets); + public SplitChangeProcessor(@Nullable SplitFilter splitFilter) { + mSplitFilter = splitFilter; } public ProcessedSplitChange process(SplitChange splitChange) { @@ -44,15 +52,20 @@ public ProcessedSplitChange process(Split featureFlag, long changeNumber) { private ProcessedSplitChange buildProcessedSplitChange(List featureFlags, long changeNumber) { List activeFeatureFlags = new ArrayList<>(); List archivedFeatureFlags = new ArrayList<>(); + for (Split featureFlag : featureFlags) { if (featureFlag.name == null) { continue; } - if (mConfiguredSets.isEmpty()) { + if (mSplitFilter == null || mSplitFilter.getValues().isEmpty()) { processAccordingToStatus(activeFeatureFlags, archivedFeatureFlags, featureFlag); } else { - processAccordingToSets(activeFeatureFlags, archivedFeatureFlags, featureFlag); + if (mSplitFilter.getType() == SplitFilter.Type.BY_NAME) { + processAccordingToNames(activeFeatureFlags, archivedFeatureFlags, featureFlag, mSplitFilter.getValues()); + } else if (mSplitFilter.getType() == SplitFilter.Type.BY_SET) { + processAccordingToSets(activeFeatureFlags, archivedFeatureFlags, featureFlag, mSplitFilter.getValues()); + } } } @@ -70,10 +83,20 @@ private void processAccordingToStatus(List activeFeatureFlags, List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List configuredValues) { + // If the feature flag name is in the filter, we process it according to its status. Otherwise it's ignored + if (configuredValues.contains(featureFlag.name)) { + processAccordingToStatus(activeFeatureFlags, archivedFeatureFlags, featureFlag); + } + } + /** * Process the feature flag according to its sets */ - private void processAccordingToSets(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag) { + private void processAccordingToSets(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List configuredValues) { if (featureFlag.sets == null || featureFlag.sets.isEmpty()) { archivedFeatureFlags.add(featureFlag); return; @@ -81,7 +104,7 @@ private void processAccordingToSets(List activeFeatureFlags, List boolean shouldArchive = true; for (String set : featureFlag.sets) { - if (mConfiguredSets.contains(set)) { + if (configuredValues.contains(set)) { // If the feature flag has at least one set that matches the configured sets, // we process it according to its status processAccordingToStatus(activeFeatureFlags, archivedFeatureFlags, featureFlag); diff --git a/src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java b/src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java index 31c9cad96..07a60bc02 100644 --- a/src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java +++ b/src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java @@ -4,7 +4,6 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.annotation.VisibleForTesting; import androidx.lifecycle.Observer; import androidx.lifecycle.ProcessLifecycleOwner; import androidx.work.Constraints; @@ -23,6 +22,7 @@ import java.util.concurrent.TimeUnit; import io.split.android.client.SplitClientConfig; +import io.split.android.client.SplitFilter; import io.split.android.client.service.ServiceConstants; import io.split.android.client.service.executor.SplitTaskExecutionInfo; import io.split.android.client.service.executor.SplitTaskExecutionListener; @@ -46,20 +46,20 @@ public class WorkManagerWrapper implements MySegmentsWorkManagerWrapper { // This variable is used to avoid loading data first time // we receive enqueued event private final Set mShouldLoadFromLocal; - private final Set mConfiguredFlagSets; + private final SplitFilter mFilter; public WorkManagerWrapper(@NonNull WorkManager workManager, @NonNull SplitClientConfig splitClientConfig, @NonNull String apiKey, @NonNull String databaseName, - @NonNull Set configuredFlagSets) { + @Nullable SplitFilter filter) { mWorkManager = checkNotNull(workManager); mDatabaseName = checkNotNull(databaseName); mSplitClientConfig = checkNotNull(splitClientConfig); mApiKey = checkNotNull(apiKey); mShouldLoadFromLocal = new HashSet<>(); mConstraints = buildConstraints(); - mConfiguredFlagSets = checkNotNull(configuredFlagSets); + mFilter = filter; } public void setFetcherExecutionListener(SplitTaskExecutionListener fetcherExecutionListener) { @@ -187,7 +187,8 @@ private Data buildSplitSyncInputData() { dataBuilder.putLong(ServiceConstants.WORKER_PARAM_SPLIT_CACHE_EXPIRATION, mSplitClientConfig.cacheExpirationInSeconds()); dataBuilder.putString(ServiceConstants.WORKER_PARAM_ENDPOINT, mSplitClientConfig.endpoint()); dataBuilder.putBoolean(ServiceConstants.SHOULD_RECORD_TELEMETRY, mSplitClientConfig.shouldRecordTelemetry()); - dataBuilder.putStringArray(ServiceConstants.WORKER_PARAM_CONFIGURED_SETS, mConfiguredFlagSets.toArray(new String[0])); + dataBuilder.putString(ServiceConstants.WORKER_PARAM_CONFIGURED_FILTER_TYPE, (mFilter != null) ? mFilter.getType().queryStringField() : null); + dataBuilder.putStringArray(ServiceConstants.WORKER_PARAM_CONFIGURED_FILTER_VALUES, (mFilter != null) ? mFilter.getValues().toArray(new String[0]) : new String[0]); return buildInputData(dataBuilder.build()); } 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 index f1dab760e..bf7633d17 100644 --- a/src/main/java/io/split/android/client/service/workmanager/SplitsSyncWorker.java +++ b/src/main/java/io/split/android/client/service/workmanager/SplitsSyncWorker.java @@ -3,14 +3,16 @@ import android.content.Context; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.annotation.WorkerThread; import androidx.work.WorkerParameters; import java.net.URISyntaxException; +import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; +import java.util.List; +import io.split.android.client.SplitFilter; import io.split.android.client.dtos.SplitChange; import io.split.android.client.service.ServiceConstants; import io.split.android.client.service.ServiceFactory; @@ -35,11 +37,8 @@ public SplitsSyncWorker(@NonNull Context context, String apiKey = workerParams.getInputData().getString(ServiceConstants.WORKER_PARAM_API_KEY); boolean encryptionEnabled = workerParams.getInputData().getBoolean(ServiceConstants.WORKER_PARAM_ENCRYPTION_ENABLED, false); - String[] configuredFlagSetsArray = workerParams.getInputData().getStringArray(ServiceConstants.WORKER_PARAM_CONFIGURED_SETS); - Set configuredFlagSets = new HashSet<>(); - if (configuredFlagSetsArray != null) { - configuredFlagSets = new HashSet<>(Arrays.asList(configuredFlagSetsArray)); - } + SplitFilter filter = buildFilter(workerParams.getInputData().getString(ServiceConstants.WORKER_PARAM_CONFIGURED_FILTER_TYPE), + workerParams.getInputData().getStringArray(ServiceConstants.WORKER_PARAM_CONFIGURED_FILTER_VALUES)); SplitsStorage splitsStorage = StorageFactory.getSplitsStorageForWorker(getDatabase(), apiKey, encryptionEnabled); // StorageFactory.getSplitsStorageForWorker creates a new storage instance, so it needs @@ -51,7 +50,7 @@ public SplitsSyncWorker(@NonNull Context context, TelemetryStorage telemetryStorage = StorageFactory.getTelemetryStorage(shouldRecordTelemetry); SplitsSyncHelper splitsSyncHelper = new SplitsSyncHelper(splitsFetcher, splitsStorage, - new SplitChangeProcessor(configuredFlagSets), telemetryStorage); + new SplitChangeProcessor(filter), telemetryStorage); mSplitTask = buildSplitSyncTask(splitsStorage, telemetryStorage, splitsSyncHelper); } catch (URISyntaxException e) { @@ -59,6 +58,24 @@ public SplitsSyncWorker(@NonNull Context context, } } + @Nullable + private static SplitFilter buildFilter(String filterType, String[] filterValuesArray) { + SplitFilter filter = null; + if (filterType != null) { + List configuredFilterValues = new ArrayList<>(); + if (filterValuesArray != null) { + configuredFilterValues = Arrays.asList(filterValuesArray); + } + + if (SplitFilter.Type.BY_NAME.queryStringField().equals(filterType)) { + filter = SplitFilter.byName(configuredFilterValues); + } else if (SplitFilter.Type.BY_SET.queryStringField().equals(filterType)) { + filter = SplitFilter.bySet(configuredFilterValues); + } + } + return filter; + } + @NonNull private SplitTask buildSplitSyncTask(SplitsStorage splitsStorage, TelemetryStorage telemetryStorage, SplitsSyncHelper splitsSyncHelper) { return SplitsSyncTask.buildForBackground(splitsSyncHelper, diff --git a/src/test/java/io/split/android/engine/splits/SplitChangeProcessorTest.java b/src/test/java/io/split/android/client/service/splits/SplitChangeProcessorTest.java similarity index 79% rename from src/test/java/io/split/android/engine/splits/SplitChangeProcessorTest.java rename to src/test/java/io/split/android/client/service/splits/SplitChangeProcessorTest.java index f727fded3..a2830c7ef 100644 --- a/src/test/java/io/split/android/engine/splits/SplitChangeProcessorTest.java +++ b/src/test/java/io/split/android/client/service/splits/SplitChangeProcessorTest.java @@ -1,4 +1,4 @@ -package io.split.android.engine.splits; +package io.split.android.client.service.splits; import androidx.annotation.Nullable; @@ -13,11 +13,11 @@ import java.util.List; import java.util.Set; +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; -import io.split.android.client.service.splits.SplitChangeProcessor; public class SplitChangeProcessorTest { @@ -25,7 +25,7 @@ public class SplitChangeProcessorTest { @Before public void setup() { - mProcessor = new SplitChangeProcessor(Collections.emptySet()); + mProcessor = new SplitChangeProcessor(); } @Test @@ -148,7 +148,8 @@ public void processAddingWithFlagSets() { Set configuredSets = new HashSet<>(); configuredSets.add("set_1"); configuredSets.add("set_2"); - mProcessor = new SplitChangeProcessor(configuredSets); + SplitFilter filter = SplitFilter.bySet(new ArrayList<>(configuredSets)); + mProcessor = new SplitChangeProcessor(filter); Split split1 = newSplit("split_1", Status.ACTIVE, new HashSet<>(Arrays.asList("set_3", "set_1"))); Split split2 = newSplit("split_2", Status.ACTIVE, Collections.singleton("set_2")); @@ -168,7 +169,9 @@ public void featureFlagWithNoSetsIsArchivedWhenProcessingWithFlagSets() { Set configuredSets = new HashSet<>(); configuredSets.add("set_1"); configuredSets.add("set_2"); - mProcessor = new SplitChangeProcessor(configuredSets); + SplitFilter filter = SplitFilter.bySet(new ArrayList<>(configuredSets)); + + mProcessor = new SplitChangeProcessor(filter); Split split1 = newSplit("split_1", Status.ACTIVE, null); @@ -181,6 +184,45 @@ public void featureFlagWithNoSetsIsArchivedWhenProcessingWithFlagSets() { Assert.assertEquals(1, result.getArchivedSplits().size()); } + @Test + public void featureFlagsAreFilteredByNameWhenThereIsSplitFilterByName() { + SplitFilter filter = SplitFilter.byName(Arrays.asList("split_1", "split_2")); + + mProcessor = new SplitChangeProcessor(filter); + + Split split1 = newSplit("split_1", Status.ACTIVE); + Split split2 = newSplit("split_2", Status.ARCHIVED); + Split split3 = newSplit("split_3", Status.ACTIVE); + Split split4 = newSplit("split_4", Status.ARCHIVED); + + SplitChange splitChange = new SplitChange(); + splitChange.splits = Arrays.asList(split1, split2, split3, split4); + + ProcessedSplitChange result = mProcessor.process(splitChange); + + Assert.assertEquals(1, result.getActiveSplits().size()); + Assert.assertEquals(1, result.getArchivedSplits().size()); + } + + @Test + public void creatingWithNullFilterProcessesEverything() { + List filterList = null; + mProcessor = new SplitChangeProcessor(filterList); + + Split split1 = newSplit("split_1", Status.ACTIVE); + Split split2 = newSplit("split_2", Status.ARCHIVED); + Split split3 = newSplit("split_3", Status.ACTIVE); + Split split4 = newSplit("split_4", Status.ARCHIVED); + + SplitChange splitChange = new SplitChange(); + splitChange.splits = Arrays.asList(split1, split2, split3, split4); + + ProcessedSplitChange result = mProcessor.process(splitChange); + + Assert.assertEquals(2, result.getActiveSplits().size()); + Assert.assertEquals(2, result.getArchivedSplits().size()); + } + private List createSplits(int from, int count, Status status) { List splits = new ArrayList<>(); for (int i = from; i < count + from; i++) { diff --git a/src/test/java/io/split/android/client/service/synchronizer/WorkManagerWrapperTest.java b/src/test/java/io/split/android/client/service/synchronizer/WorkManagerWrapperTest.java index 13b9ce2dd..913e77351 100644 --- a/src/test/java/io/split/android/client/service/synchronizer/WorkManagerWrapperTest.java +++ b/src/test/java/io/split/android/client/service/synchronizer/WorkManagerWrapperTest.java @@ -19,12 +19,14 @@ import org.mockito.MockitoAnnotations; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.concurrent.TimeUnit; import io.split.android.client.ServiceEndpoints; import io.split.android.client.SplitClientConfig; +import io.split.android.client.SplitFilter; import io.split.android.client.service.executor.SplitTaskType; import io.split.android.client.service.workmanager.EventsRecorderWorker; import io.split.android.client.service.workmanager.ImpressionsRecorderWorker; @@ -70,7 +72,7 @@ public void setUp() throws Exception { splitClientConfig, "api_key", "test_database_name", - new HashSet<>(Arrays.asList("set_1", "set_2")) + SplitFilter.bySet(Arrays.asList("set_1", "set_2")) ); } @@ -92,7 +94,8 @@ public void scheduleWorkSchedulesSplitsJob() { .putLong("splitCacheExpiration", 864000) .putString("endpoint", "https://test.split.io/api") .putBoolean("shouldRecordTelemetry", true) - .putStringArray("configuredSets", new String[]{"set_1", "set_2"}) + .putStringArray("configuredFilterValues", new String[]{"set_1", "set_2"}) + .putString("configuredFilterType", SplitFilter.Type.BY_SET.queryStringField()) .build(); PeriodicWorkRequest expectedRequest = new PeriodicWorkRequest From 9b0e8fcb9410f1f96b8bcc9b8c56a9d27503a644 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 6 Sep 2023 19:06:27 -0300 Subject: [PATCH 2/6] Flag processing strategies --- .../splits/FeatureFlagProcessStrategy.java | 78 +++++++++++++++++++ .../service/splits/SplitChangeProcessor.java | 69 ++++------------ 2 files changed, 95 insertions(+), 52 deletions(-) create mode 100644 src/main/java/io/split/android/client/service/splits/FeatureFlagProcessStrategy.java 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 new file mode 100644 index 000000000..91f979d25 --- /dev/null +++ b/src/main/java/io/split/android/client/service/splits/FeatureFlagProcessStrategy.java @@ -0,0 +1,78 @@ +package io.split.android.client.service.splits; + +import androidx.annotation.NonNull; + +import java.util.List; + +import io.split.android.client.dtos.Split; +import io.split.android.client.dtos.Status; + +interface FeatureFlagProcessStrategy { + + void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List filterValues); +} + +class StatusProcessStrategy implements FeatureFlagProcessStrategy { + + @Override + public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List filterValues) { + if (featureFlag.status == Status.ACTIVE) { + activeFeatureFlags.add(featureFlag); + } else { + archivedFeatureFlags.add(featureFlag); + } + } +} + +class NamesProcessStrategy implements FeatureFlagProcessStrategy { + + private final List mConfiguredValues; + private final StatusProcessStrategy mStatusProcessStrategy; + + NamesProcessStrategy(@NonNull List configuredValues, @NonNull StatusProcessStrategy statusProcessStrategy) { + mConfiguredValues = configuredValues; + mStatusProcessStrategy = statusProcessStrategy; + } + + @Override + public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List filterValues) { + // If the feature flag name is in the filter, we process it according to its status. Otherwise it is ignored + if (mConfiguredValues.contains(featureFlag.name)) { + mStatusProcessStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag, filterValues); + } + } +} + +class SetsProcessStrategy implements FeatureFlagProcessStrategy { + + private final List mConfiguredValues; + private final StatusProcessStrategy mStatusProcessStrategy; + + SetsProcessStrategy(@NonNull List configuredValues, @NonNull StatusProcessStrategy statusProcessStrategy) { + mConfiguredValues = configuredValues; + mStatusProcessStrategy = statusProcessStrategy; + } + + @Override + public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List filterValues) { + if (featureFlag.sets == null || featureFlag.sets.isEmpty()) { + archivedFeatureFlags.add(featureFlag); + return; + } + + boolean shouldArchive = true; + for (String set : featureFlag.sets) { + if (mConfiguredValues.contains(set)) { + // If the feature flag has at least one set that matches the configured sets, + // we process it according to its status + mStatusProcessStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag, filterValues); + shouldArchive = false; + break; + } + } + + if (shouldArchive) { + archivedFeatureFlags.add(featureFlag); + } + } +} diff --git a/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java b/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java index 0b88c0b25..62eaa9660 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java +++ b/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java @@ -18,6 +18,8 @@ public class SplitChangeProcessor { private final SplitFilter mSplitFilter; + private final StatusProcessStrategy mStatusProcessStrategy; + @VisibleForTesting SplitChangeProcessor() { this((SplitFilter) null); @@ -30,10 +32,13 @@ public SplitChangeProcessor(@Nullable List filters) { } else { mSplitFilter = filters.get(0); } + + mStatusProcessStrategy = new StatusProcessStrategy(); } public SplitChangeProcessor(@Nullable SplitFilter splitFilter) { mSplitFilter = splitFilter; + mStatusProcessStrategy = new StatusProcessStrategy(); } public ProcessedSplitChange process(SplitChange splitChange) { @@ -53,68 +58,28 @@ private ProcessedSplitChange buildProcessedSplitChange(List featureFlags, List activeFeatureFlags = new ArrayList<>(); List archivedFeatureFlags = new ArrayList<>(); + SplitFilter.Type filterType = (mSplitFilter != null) ? mSplitFilter.getType() : null; + List filterValues = (mSplitFilter != null) ? mSplitFilter.getValues() : null; + + FeatureFlagProcessStrategy processStrategy = getProcessStrategy(filterType, filterValues); + for (Split featureFlag : featureFlags) { if (featureFlag.name == null) { continue; } - - if (mSplitFilter == null || mSplitFilter.getValues().isEmpty()) { - processAccordingToStatus(activeFeatureFlags, archivedFeatureFlags, featureFlag); - } else { - if (mSplitFilter.getType() == SplitFilter.Type.BY_NAME) { - processAccordingToNames(activeFeatureFlags, archivedFeatureFlags, featureFlag, mSplitFilter.getValues()); - } else if (mSplitFilter.getType() == SplitFilter.Type.BY_SET) { - processAccordingToSets(activeFeatureFlags, archivedFeatureFlags, featureFlag, mSplitFilter.getValues()); - } - } + processStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag, filterValues); } return new ProcessedSplitChange(activeFeatureFlags, archivedFeatureFlags, changeNumber, System.currentTimeMillis() / 100); } - /** - * Process the feature flag according to its status - */ - private void processAccordingToStatus(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag) { - if (featureFlag.status == Status.ACTIVE) { - activeFeatureFlags.add(featureFlag); + private FeatureFlagProcessStrategy getProcessStrategy(SplitFilter.Type filterType, List filterValues) { + if (filterType == SplitFilter.Type.BY_SET) { + return new NamesProcessStrategy(filterValues, mStatusProcessStrategy); + } else if (filterType == SplitFilter.Type.BY_NAME) { + return new SetsProcessStrategy(filterValues, mStatusProcessStrategy); } else { - archivedFeatureFlags.add(featureFlag); - } - } - - /** - * Process the feature flag according to its name - */ - private void processAccordingToNames(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List configuredValues) { - // If the feature flag name is in the filter, we process it according to its status. Otherwise it's ignored - if (configuredValues.contains(featureFlag.name)) { - processAccordingToStatus(activeFeatureFlags, archivedFeatureFlags, featureFlag); - } - } - - /** - * Process the feature flag according to its sets - */ - private void processAccordingToSets(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List configuredValues) { - if (featureFlag.sets == null || featureFlag.sets.isEmpty()) { - archivedFeatureFlags.add(featureFlag); - return; - } - - boolean shouldArchive = true; - for (String set : featureFlag.sets) { - if (configuredValues.contains(set)) { - // If the feature flag has at least one set that matches the configured sets, - // we process it according to its status - processAccordingToStatus(activeFeatureFlags, archivedFeatureFlags, featureFlag); - shouldArchive = false; - break; - } - } - - if (shouldArchive) { - archivedFeatureFlags.add(featureFlag); + return mStatusProcessStrategy; } } } From bba3517cce6c1bf969657d040233f58ef40e06f2 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 7 Sep 2023 10:47:17 -0300 Subject: [PATCH 3/6] Fix --- .../android/client/service/splits/SplitChangeProcessor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java b/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java index 62eaa9660..ff75b2d61 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java +++ b/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java @@ -75,9 +75,9 @@ private ProcessedSplitChange buildProcessedSplitChange(List featureFlags, private FeatureFlagProcessStrategy getProcessStrategy(SplitFilter.Type filterType, List filterValues) { if (filterType == SplitFilter.Type.BY_SET) { - return new NamesProcessStrategy(filterValues, mStatusProcessStrategy); - } else if (filterType == SplitFilter.Type.BY_NAME) { return new SetsProcessStrategy(filterValues, mStatusProcessStrategy); + } else if (filterType == SplitFilter.Type.BY_NAME) { + return new NamesProcessStrategy(filterValues, mStatusProcessStrategy); } else { return mStatusProcessStrategy; } From beb3d47fa39c74c8183ad2e02acec9fd487a3856 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 7 Sep 2023 12:21:17 -0300 Subject: [PATCH 4/6] Remove unnecessary param --- .../splits/FeatureFlagProcessStrategy.java | 12 +++++----- .../service/splits/SplitChangeProcessor.java | 22 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) 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 index 91f979d25..4de68e5a7 100644 --- a/src/main/java/io/split/android/client/service/splits/FeatureFlagProcessStrategy.java +++ b/src/main/java/io/split/android/client/service/splits/FeatureFlagProcessStrategy.java @@ -9,13 +9,13 @@ interface FeatureFlagProcessStrategy { - void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List filterValues); + void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag); } class StatusProcessStrategy implements FeatureFlagProcessStrategy { @Override - public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List filterValues) { + public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag) { if (featureFlag.status == Status.ACTIVE) { activeFeatureFlags.add(featureFlag); } else { @@ -35,10 +35,10 @@ class NamesProcessStrategy implements FeatureFlagProcessStrategy { } @Override - public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List filterValues) { + public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag) { // If the feature flag name is in the filter, we process it according to its status. Otherwise it is ignored if (mConfiguredValues.contains(featureFlag.name)) { - mStatusProcessStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag, filterValues); + mStatusProcessStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag); } } } @@ -54,7 +54,7 @@ class SetsProcessStrategy implements FeatureFlagProcessStrategy { } @Override - public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag, List filterValues) { + public void process(List activeFeatureFlags, List archivedFeatureFlags, Split featureFlag) { if (featureFlag.sets == null || featureFlag.sets.isEmpty()) { archivedFeatureFlags.add(featureFlag); return; @@ -65,7 +65,7 @@ public void process(List activeFeatureFlags, List archivedFeatureF if (mConfiguredValues.contains(set)) { // If the feature flag has at least one set that matches the configured sets, // we process it according to its status - mStatusProcessStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag, filterValues); + mStatusProcessStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag); shouldArchive = false; break; } diff --git a/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java b/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java index ff75b2d61..b709beb68 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java +++ b/src/main/java/io/split/android/client/service/splits/SplitChangeProcessor.java @@ -58,26 +58,28 @@ private ProcessedSplitChange buildProcessedSplitChange(List featureFlags, List activeFeatureFlags = new ArrayList<>(); List archivedFeatureFlags = new ArrayList<>(); - SplitFilter.Type filterType = (mSplitFilter != null) ? mSplitFilter.getType() : null; - List filterValues = (mSplitFilter != null) ? mSplitFilter.getValues() : null; - - FeatureFlagProcessStrategy processStrategy = getProcessStrategy(filterType, filterValues); + FeatureFlagProcessStrategy processStrategy = getProcessStrategy(mSplitFilter); for (Split featureFlag : featureFlags) { if (featureFlag.name == null) { continue; } - processStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag, filterValues); + + processStrategy.process(activeFeatureFlags, archivedFeatureFlags, featureFlag); } return new ProcessedSplitChange(activeFeatureFlags, archivedFeatureFlags, changeNumber, System.currentTimeMillis() / 100); } - private FeatureFlagProcessStrategy getProcessStrategy(SplitFilter.Type filterType, List filterValues) { - if (filterType == SplitFilter.Type.BY_SET) { - return new SetsProcessStrategy(filterValues, mStatusProcessStrategy); - } else if (filterType == SplitFilter.Type.BY_NAME) { - return new NamesProcessStrategy(filterValues, mStatusProcessStrategy); + private FeatureFlagProcessStrategy getProcessStrategy(SplitFilter splitFilter) { + if (splitFilter == null) { + return mStatusProcessStrategy; + } + + if (splitFilter.getType() == SplitFilter.Type.BY_SET) { + return new SetsProcessStrategy(splitFilter.getValues(), mStatusProcessStrategy); + } else if (splitFilter.getType() == SplitFilter.Type.BY_NAME) { + return new NamesProcessStrategy(splitFilter.getValues(), mStatusProcessStrategy); } else { return mStatusProcessStrategy; } From a0b9a08552897305a76e0d4225dcb7bae5d76a64 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 7 Sep 2023 13:47:42 -0300 Subject: [PATCH 5/6] Fix --- .../java/io/split/android/client/SplitFactoryHelper.java | 2 +- .../client/service/synchronizer/WorkManagerWrapper.java | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/split/android/client/SplitFactoryHelper.java b/src/main/java/io/split/android/client/SplitFactoryHelper.java index e4c2fd4e7..83cb7a841 100644 --- a/src/main/java/io/split/android/client/SplitFactoryHelper.java +++ b/src/main/java/io/split/android/client/SplitFactoryHelper.java @@ -191,7 +191,7 @@ SplitApiFacade buildApiFacade(SplitClientConfig splitClientConfig, WorkManagerWrapper buildWorkManagerWrapper(Context context, SplitClientConfig splitClientConfig, String apiKey, String databaseName, List filters) { return new WorkManagerWrapper( - WorkManager.getInstance(context), splitClientConfig, apiKey, databaseName, (filters != null && filters.size() == 1) ? filters.get(0) : null); + WorkManager.getInstance(context), splitClientConfig, apiKey, databaseName, filters); } diff --git a/src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java b/src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java index 07a60bc02..b2beb4b08 100644 --- a/src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java +++ b/src/main/java/io/split/android/client/service/synchronizer/WorkManagerWrapper.java @@ -46,20 +46,21 @@ public class WorkManagerWrapper implements MySegmentsWorkManagerWrapper { // This variable is used to avoid loading data first time // we receive enqueued event private final Set mShouldLoadFromLocal; + @Nullable private final SplitFilter mFilter; public WorkManagerWrapper(@NonNull WorkManager workManager, @NonNull SplitClientConfig splitClientConfig, @NonNull String apiKey, @NonNull String databaseName, - @Nullable SplitFilter filter) { + @Nullable List filters) { mWorkManager = checkNotNull(workManager); mDatabaseName = checkNotNull(databaseName); mSplitClientConfig = checkNotNull(splitClientConfig); mApiKey = checkNotNull(apiKey); mShouldLoadFromLocal = new HashSet<>(); mConstraints = buildConstraints(); - mFilter = filter; + mFilter = (filters != null && filters.size() == 1) ? filters.get(0) : null; } public void setFetcherExecutionListener(SplitTaskExecutionListener fetcherExecutionListener) { From d41b6ea8cde6eed61439446be6ed7f1deea38973 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 7 Sep 2023 14:33:47 -0300 Subject: [PATCH 6/6] Fix test; remove duplicated code --- .../localhost/LocalhostSplitClient.java | 52 +++++++++---------- .../synchronizer/WorkManagerWrapperTest.java | 3 +- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java b/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java index e82e33f16..b4dfbdbf3 100644 --- a/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java +++ b/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java @@ -149,13 +149,7 @@ public Map getTreatmentsByFlagSet(@NonNull String flagSet, @Null } catch (Exception exception) { Logger.e(exception); - Map result = new HashMap<>(); - Set namesByFlagSets = mSplitsStorage.getNamesByFlagSets(Collections.singletonList(flagSet)); - for (String featureFlagName : namesByFlagSets) { - result.put(featureFlagName, Treatments.CONTROL); - } - - return result; + return buildExceptionResult(Collections.singletonList(flagSet)); } } @@ -166,13 +160,7 @@ public Map getTreatmentsByFlagSets(@NonNull List flagSet } catch (Exception exception) { Logger.e(exception); - Map result = new HashMap<>(); - Set namesByFlagSets = mSplitsStorage.getNamesByFlagSets(flagSets); - for (String featureFlagName : namesByFlagSets) { - result.put(featureFlagName, Treatments.CONTROL); - } - - return result; + return buildExceptionResult(flagSets); } } @@ -183,13 +171,7 @@ public Map getTreatmentsWithConfigByFlagSet(@NonNull String } catch (Exception exception) { Logger.e(exception); - Map result = new HashMap<>(); - Set namesByFlagSets = mSplitsStorage.getNamesByFlagSets(Collections.singletonList(flagSet)); - for (String featureFlagName : namesByFlagSets) { - result.put(featureFlagName, new SplitResult(Treatments.CONTROL)); - } - - return result; + return buildExceptionResultWithConfig(Collections.singletonList(flagSet)); } } @@ -200,13 +182,7 @@ public Map getTreatmentsWithConfigByFlagSets(@NonNull List< } catch (Exception exception) { Logger.e(exception); - Map result = new HashMap<>(); - Set namesByFlagSets = mSplitsStorage.getNamesByFlagSets(flagSets); - for (String featureFlagName : namesByFlagSets) { - result.put(featureFlagName, new SplitResult(Treatments.CONTROL)); - } - - return result; + return buildExceptionResultWithConfig(flagSets); } } @@ -324,4 +300,24 @@ public boolean removeAttribute(String attributeName) { public boolean clearAttributes() { return true; } + + private Map buildExceptionResult(List flagSets) { + Map result = new HashMap<>(); + Set namesByFlagSets = mSplitsStorage.getNamesByFlagSets(flagSets); + for (String featureFlagName : namesByFlagSets) { + result.put(featureFlagName, Treatments.CONTROL); + } + + return result; + } + + private Map buildExceptionResultWithConfig(List flagSets) { + Map result = new HashMap<>(); + Set namesByFlagSets = mSplitsStorage.getNamesByFlagSets(flagSets); + for (String featureFlagName : namesByFlagSets) { + result.put(featureFlagName, new SplitResult(Treatments.CONTROL)); + } + + return result; + } } diff --git a/src/test/java/io/split/android/client/service/synchronizer/WorkManagerWrapperTest.java b/src/test/java/io/split/android/client/service/synchronizer/WorkManagerWrapperTest.java index 913e77351..a79498fa4 100644 --- a/src/test/java/io/split/android/client/service/synchronizer/WorkManagerWrapperTest.java +++ b/src/test/java/io/split/android/client/service/synchronizer/WorkManagerWrapperTest.java @@ -21,6 +21,7 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.concurrent.TimeUnit; @@ -72,7 +73,7 @@ public void setUp() throws Exception { splitClientConfig, "api_key", "test_database_name", - SplitFilter.bySet(Arrays.asList("set_1", "set_2")) + Collections.singletonList(SplitFilter.bySet(Arrays.asList("set_1", "set_2"))) ); }