From ba26ccd90fee4873f360898167096cdf9beb5d0d Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 14 Sep 2023 10:10:56 -0300 Subject: [PATCH 1/7] Additional query string test --- .../integration/sets/FlagSetsPollingTest.java | 20 ++++++++++++++++++- .../service/splits/SplitChangeProcessor.java | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/androidTest/java/tests/integration/sets/FlagSetsPollingTest.java b/src/androidTest/java/tests/integration/sets/FlagSetsPollingTest.java index 3a549259d..8c3c095f7 100644 --- a/src/androidTest/java/tests/integration/sets/FlagSetsPollingTest.java +++ b/src/androidTest/java/tests/integration/sets/FlagSetsPollingTest.java @@ -14,11 +14,13 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import fake.HttpClientMock; import fake.HttpResponseMock; @@ -46,6 +48,7 @@ public class FlagSetsPollingTest { private CountDownLatch secondChangeLatch; private CountDownLatch thirdChangeLatch; private SplitRoomDatabase mRoomDb; + private volatile String mSplitChangesUri; @Before public void setUp() throws Exception { @@ -157,6 +160,19 @@ public void featureFlagSetsAreIgnoredWhenSetsAreNotConfigured() throws IOExcepti assertTrue(awaitHits); } + @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); + + String uri = mSplitChangesUri; + + assertTrue(awaitFirst); + assertEquals("https://sdk.split.io/api/splitChanges?since=-1&sets=set_2,set_3,set_ww,set_x", uri); + } + private SplitFactory createFactory( Context mContext, SplitRoomDatabase splitRoomDatabase, @@ -169,6 +185,8 @@ private SplitFactory createFactory( .impressionsCountersRefreshRate(1000) .syncConfig(SyncConfig.builder() .addSplitFilter(SplitFilter.bySet(Arrays.asList(sets))) + .addSplitFilter(SplitFilter.byName(Arrays.asList("workm", "workm_set_3"))) // added to test that this filter is ignored + .addSplitFilter(SplitFilter.byPrefix(Collections.singletonList("pref"))) // added to test that this filter is ignored .build()) .featuresRefreshRate(2) .streamingEnabled(false) @@ -177,8 +195,8 @@ private SplitFactory createFactory( Map responses = new HashMap<>(); responses.put("splitChanges", (uri, httpMethod, body) -> { + mSplitChangesUri = uri.toString(); String since = getSinceFromUri(uri); - hitsLatch.countDown(); if (since.equals("-1")) { firstChangeLatch.countDown(); 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 a24848d42..88abaf09d 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 @@ -61,7 +61,7 @@ private ProcessedSplitChange buildProcessedSplitChange(List featureFlags, FeatureFlagProcessStrategy processStrategy = getProcessStrategy(mSplitFilter); for (Split featureFlag : featureFlags) { - if (featureFlag.name == null) { + if (featureFlag == null || featureFlag.name == null) { continue; } From 74d4d2a4c292160d73ebd24ea23eb9cdcd2c7941 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 14 Sep 2023 10:14:28 -0300 Subject: [PATCH 2/7] FlagSetsFilter --- .../split/android/client/FlagSetsFilter.java | 10 ++++ .../android/client/FlagSetsFilterImpl.java | 47 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 src/main/java/io/split/android/client/FlagSetsFilter.java create mode 100644 src/main/java/io/split/android/client/FlagSetsFilterImpl.java diff --git a/src/main/java/io/split/android/client/FlagSetsFilter.java b/src/main/java/io/split/android/client/FlagSetsFilter.java new file mode 100644 index 000000000..06850ca11 --- /dev/null +++ b/src/main/java/io/split/android/client/FlagSetsFilter.java @@ -0,0 +1,10 @@ +package io.split.android.client; + +import java.util.Set; + +interface FlagSetsFilter { + + boolean intersect(Set sets); + + boolean intersect(String set); +} diff --git a/src/main/java/io/split/android/client/FlagSetsFilterImpl.java b/src/main/java/io/split/android/client/FlagSetsFilterImpl.java new file mode 100644 index 000000000..3464ff70d --- /dev/null +++ b/src/main/java/io/split/android/client/FlagSetsFilterImpl.java @@ -0,0 +1,47 @@ +package io.split.android.client; + +import java.util.HashSet; +import java.util.Set; + +class FlagSetsFilterImpl implements FlagSetsFilter { + + private final boolean mShouldFilter; + private final Set mFlagSets; + + FlagSetsFilterImpl(Set flagSets) { + mFlagSets = new HashSet<>(flagSets); + mShouldFilter = !mFlagSets.isEmpty(); + } + + @Override + public boolean intersect(Set sets) { + if (!mShouldFilter) { + return true; + } + + if (sets == null) { + return false; + } + + for (String set : sets) { + if (mFlagSets.contains(set)) { + return true; + } + } + + return false; + } + + @Override + public boolean intersect(String set) { + if (!mShouldFilter) { + return true; + } + + if (set == null) { + return false; + } + + return mFlagSets.contains(set); + } +} From 0bedb89f03b1b286fc38cdd30c3df5816174a088 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 14 Sep 2023 15:42:22 -0300 Subject: [PATCH 3/7] Flag sets filter initial integration --- .../android/client/FeatureFlagFilter.java | 10 +++ .../split/android/client/FlagSetsFilter.java | 6 +- .../android/client/FlagSetsFilterImpl.java | 4 +- .../client/SplitClientFactoryImpl.java | 5 +- .../android/client/SplitFactoryHelper.java | 14 ---- .../android/client/SplitFactoryImpl.java | 9 ++- .../localhost/LocalhostSplitClient.java | 5 +- .../localhost/LocalhostSplitFactory.java | 8 +- .../LocalhostSplitClientContainerImpl.java | 9 ++- .../executor/SplitTaskFactoryImpl.java | 4 +- .../splits/FeatureFlagProcessStrategy.java | 20 +++-- .../service/splits/SplitChangeProcessor.java | 16 ++-- .../service/workmanager/SplitsSyncWorker.java | 9 ++- .../shared/SplitClientContainerImpl.java | 5 +- .../TreatmentManagerFactoryImpl.java | 10 ++- .../validators/TreatmentManagerImpl.java | 9 ++- .../client/FlagSetsFilterImplTest.java | 73 +++++++++++++++++++ .../splits/SplitChangeProcessorTest.java | 15 ++-- 18 files changed, 163 insertions(+), 68 deletions(-) create mode 100644 src/main/java/io/split/android/client/FeatureFlagFilter.java create mode 100644 src/test/java/io/split/android/client/FlagSetsFilterImplTest.java diff --git a/src/main/java/io/split/android/client/FeatureFlagFilter.java b/src/main/java/io/split/android/client/FeatureFlagFilter.java new file mode 100644 index 000000000..fb4dc7764 --- /dev/null +++ b/src/main/java/io/split/android/client/FeatureFlagFilter.java @@ -0,0 +1,10 @@ +package io.split.android.client; + +import java.util.Set; + +interface FeatureFlagFilter { + + boolean intersect(Set values); + + boolean intersect(String values); +} diff --git a/src/main/java/io/split/android/client/FlagSetsFilter.java b/src/main/java/io/split/android/client/FlagSetsFilter.java index 06850ca11..4c982e365 100644 --- a/src/main/java/io/split/android/client/FlagSetsFilter.java +++ b/src/main/java/io/split/android/client/FlagSetsFilter.java @@ -2,9 +2,5 @@ import java.util.Set; -interface FlagSetsFilter { - - boolean intersect(Set sets); - - boolean intersect(String set); +public interface FlagSetsFilter extends FeatureFlagFilter { } diff --git a/src/main/java/io/split/android/client/FlagSetsFilterImpl.java b/src/main/java/io/split/android/client/FlagSetsFilterImpl.java index 3464ff70d..84ab5c839 100644 --- a/src/main/java/io/split/android/client/FlagSetsFilterImpl.java +++ b/src/main/java/io/split/android/client/FlagSetsFilterImpl.java @@ -3,12 +3,12 @@ import java.util.HashSet; import java.util.Set; -class FlagSetsFilterImpl implements FlagSetsFilter { +public class FlagSetsFilterImpl implements FlagSetsFilter { private final boolean mShouldFilter; private final Set mFlagSets; - FlagSetsFilterImpl(Set flagSets) { + public FlagSetsFilterImpl(Set flagSets) { mFlagSets = new HashSet<>(flagSets); mShouldFilter = !mFlagSets.isEmpty(); } diff --git a/src/main/java/io/split/android/client/SplitClientFactoryImpl.java b/src/main/java/io/split/android/client/SplitClientFactoryImpl.java index 69e7dc998..e5690d3f8 100644 --- a/src/main/java/io/split/android/client/SplitClientFactoryImpl.java +++ b/src/main/java/io/split/android/client/SplitClientFactoryImpl.java @@ -3,6 +3,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import java.util.Set; @@ -59,7 +60,7 @@ public SplitClientFactoryImpl(@NonNull SplitFactory splitFactory, @NonNull KeyValidator keyValidator, @NonNull EventsTracker eventsTracker, @NonNull ImpressionListener customerImpressionListener, - @NonNull Set configuredFlagSets) { + @Nullable FlagSetsFilter flagSetsFilter) { mSplitFactory = checkNotNull(splitFactory); mClientContainer = checkNotNull(clientContainer); mConfig = checkNotNull(config); @@ -85,7 +86,7 @@ public SplitClientFactoryImpl(@NonNull SplitFactory splitFactory, new AttributesMergerImpl(), mStorageContainer.getTelemetryStorage(), mSplitParser, - configuredFlagSets, + flagSetsFilter, splitsStorage ); } diff --git a/src/main/java/io/split/android/client/SplitFactoryHelper.java b/src/main/java/io/split/android/client/SplitFactoryHelper.java index 399a8740a..6541a05e7 100644 --- a/src/main/java/io/split/android/client/SplitFactoryHelper.java +++ b/src/main/java/io/split/android/client/SplitFactoryHelper.java @@ -11,9 +11,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingDeque; @@ -429,18 +427,6 @@ Pair, String> getFilterConfiguration(SyncConf return new Pair<>(groupedFilters, splitsFilterQueryString); } - @NonNull - Set getConfiguredFlagSets(Map filters) { - Set configuredFlagSets; - SplitFilter flagSetSplitFilter = filters.get(SplitFilter.Type.BY_SET); - if (flagSetSplitFilter != null) { - configuredFlagSets = new HashSet<>(flagSetSplitFilter.getValues()); - } else { - configuredFlagSets = new HashSet<>(); - } - return configuredFlagSets; - } - private TelemetryStorage getTelemetryStorage(boolean shouldRecordTelemetry, TelemetryStorage telemetryStorage) { if (telemetryStorage != null) { return telemetryStorage; diff --git a/src/main/java/io/split/android/client/SplitFactoryImpl.java b/src/main/java/io/split/android/client/SplitFactoryImpl.java index c387d9d55..11519d18b 100644 --- a/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -180,9 +180,14 @@ public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) { SplitApiFacade splitApiFacade = factoryHelper.buildApiFacade( config, defaultHttpClient, splitsFilterQueryStringFromConfig); + FlagSetsFilter flagSetsFilter = null; + if (filters.get(SplitFilter.Type.BY_SET) != null) { + flagSetsFilter = new FlagSetsFilterImpl(new HashSet<>(filters.get(SplitFilter.Type.BY_SET).getValues())); + } + SplitTaskFactory splitTaskFactory = new SplitTaskFactoryImpl( config, splitApiFacade, mStorageContainer, splitsFilterQueryStringFromConfig, mEventsManagerCoordinator, - filters, testingConfig); + filters, flagSetsFilter, testingConfig); cleanUpDabase(splitTaskExecutor, splitTaskFactory); WorkManagerWrapper workManagerWrapper = factoryHelper.buildWorkManagerWrapper(context, config, apiToken, databaseName, filters); @@ -270,7 +275,7 @@ public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) { telemetrySynchronizer, mStorageContainer, splitTaskExecutor, splitApiFacade, validationLogger, keyValidator, customerImpressionListener, streamingComponents.getPushNotificationManager(), componentsRegister, workManagerWrapper, - eventsTracker, factoryHelper.getConfiguredFlagSets(filters)); + eventsTracker, flagSetsFilter); mDestroyer = new Runnable() { public void run() { Logger.w("Shutdown called for split"); 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 b4dfbdbf3..941f3ee7a 100644 --- a/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java +++ b/src/main/java/io/split/android/client/localhost/LocalhostSplitClient.java @@ -14,6 +14,7 @@ import io.split.android.client.Evaluator; import io.split.android.client.EvaluatorImpl; +import io.split.android.client.FlagSetsFilter; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitFactory; @@ -61,7 +62,7 @@ public LocalhostSplitClient(@NonNull LocalhostSplitFactory container, @NonNull AttributesManager attributesManager, @NonNull AttributesMerger attributesMerger, @NonNull TelemetryStorageProducer telemetryStorageProducer, - @NonNull Set configuredFlagSets) { + @Nullable FlagSetsFilter flagSetsFilter) { mFactoryRef = new WeakReference<>(checkNotNull(container)); mClientContainer = new WeakReference<>(checkNotNull(clientContainer)); @@ -72,7 +73,7 @@ public LocalhostSplitClient(@NonNull LocalhostSplitFactory container, new EvaluatorImpl(splitsStorage, splitParser), new KeyValidatorImpl(), new SplitValidatorImpl(), getImpressionsListener(splitClientConfig), splitClientConfig.labelsEnabled(), eventsManager, attributesManager, attributesMerger, - telemetryStorageProducer, configuredFlagSets, splitsStorage); + telemetryStorageProducer, flagSetsFilter, splitsStorage); } @Override diff --git a/src/main/java/io/split/android/client/localhost/LocalhostSplitFactory.java b/src/main/java/io/split/android/client/localhost/LocalhostSplitFactory.java index 1f79f0e8d..0948fad70 100644 --- a/src/main/java/io/split/android/client/localhost/LocalhostSplitFactory.java +++ b/src/main/java/io/split/android/client/localhost/LocalhostSplitFactory.java @@ -13,6 +13,8 @@ import java.util.Set; import io.split.android.client.FilterBuilder; +import io.split.android.client.FlagSetsFilter; +import io.split.android.client.FlagSetsFilterImpl; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitFactory; @@ -75,7 +77,7 @@ public LocalhostSplitFactory(String key, Context context, mManager = new SplitManagerImpl(splitsStorage, new SplitValidatorImpl(), splitParser); - Set configuredSets = new HashSet<>(); + FlagSetsFilter flagSetsFilter = null; if (config.syncConfig() != null) { Map groupedFilters = new FilterBuilder(config.syncConfig().getFilters()) .getGroupedFilter(); @@ -83,7 +85,7 @@ public LocalhostSplitFactory(String key, Context context, if (!groupedFilters.isEmpty()) { SplitFilter bySetFilter = groupedFilters.get(SplitFilter.Type.BY_SET); if (bySetFilter != null) { - configuredSets.addAll(bySetFilter.getValues()); + flagSetsFilter = new FlagSetsFilterImpl(new HashSet<>(bySetFilter.getValues())); } } } @@ -97,7 +99,7 @@ public LocalhostSplitFactory(String key, Context context, new NoOpTelemetryStorage(), eventsManagerCoordinator, taskExecutor, - configuredSets); + flagSetsFilter); mSynchronizer = new LocalhostSynchronizer(taskExecutor, config, splitsStorage, buildQueryString(config.syncConfig())); mSynchronizer.start(); diff --git a/src/main/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImpl.java b/src/main/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImpl.java index e4a6d2a69..f0eb208e5 100644 --- a/src/main/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImpl.java +++ b/src/main/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImpl.java @@ -2,6 +2,7 @@ import java.util.Set; +import io.split.android.client.FlagSetsFilter; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.api.Key; @@ -31,7 +32,7 @@ public class LocalhostSplitClientContainerImpl extends BaseSplitClientContainer private final TelemetryStorageProducer mTelemetryStorageProducer; private final EventsManagerCoordinator mEventsManagerCoordinator; private final SplitTaskExecutor mSplitTaskExecutor; - private final Set mConfiguredSets; + private final FlagSetsFilter mFlagSetsFilter; public LocalhostSplitClientContainerImpl(LocalhostSplitFactory splitFactory, SplitClientConfig config, @@ -42,7 +43,7 @@ public LocalhostSplitClientContainerImpl(LocalhostSplitFactory splitFactory, TelemetryStorageProducer telemetryStorageProducer, EventsManagerCoordinator eventsManagerCoordinator, SplitTaskExecutor taskExecutor, - Set configuredSets) { + FlagSetsFilter flagSetsFilter) { mSplitFactory = splitFactory; mConfig = config; mSplitStorage = splitsStorage; @@ -52,7 +53,7 @@ public LocalhostSplitClientContainerImpl(LocalhostSplitFactory splitFactory, mTelemetryStorageProducer = telemetryStorageProducer; mEventsManagerCoordinator = eventsManagerCoordinator; mSplitTaskExecutor = taskExecutor; - mConfiguredSets = configuredSets; + mFlagSetsFilter = flagSetsFilter; } @Override @@ -76,7 +77,7 @@ protected void createNewClient(Key key) { attributesManager, mAttributesMerger, mTelemetryStorageProducer, - mConfiguredSets + mFlagSetsFilter ); eventsManager.getExecutorResources().setSplitClient(client); 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 1341f4d5f..dde6c846c 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 @@ -12,6 +12,7 @@ import java.util.Map; import java.util.Set; +import io.split.android.client.FlagSetsFilter; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitFilter; import io.split.android.client.TestingConfig; @@ -67,6 +68,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig, @Nullable String splitsFilterQueryString, ISplitEventsManager eventsManager, @Nullable Map filters, + @Nullable FlagSetsFilter flagSetsFilter, @Nullable TestingConfig testingConfig) { mSplitClientConfig = checkNotNull(splitClientConfig); @@ -74,7 +76,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig, mSplitsStorageContainer = checkNotNull(splitStorageContainer); mSplitsFilterQueryStringFromConfig = splitsFilterQueryString; mEventsManager = eventsManager; - mSplitChangeProcessor = new SplitChangeProcessor(filters); + mSplitChangeProcessor = new SplitChangeProcessor(filters, flagSetsFilter); TelemetryStorage telemetryStorage = mSplitsStorageContainer.getTelemetryStorage(); mTelemetryRuntimeProducer = telemetryStorage; 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 2f50f6bbb..f61871de0 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 @@ -2,8 +2,11 @@ import androidx.annotation.NonNull; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import io.split.android.client.FlagSetsFilter; import io.split.android.client.dtos.Split; import io.split.android.client.dtos.Status; @@ -45,12 +48,13 @@ public void process(List activeFeatureFlags, List archivedFeatureF class SetsProcessStrategy implements FeatureFlagProcessStrategy { - private final List mConfiguredValues; + private final FlagSetsFilter mFlagSetsFilter; private final StatusProcessStrategy mStatusProcessStrategy; - SetsProcessStrategy(@NonNull List configuredValues, @NonNull StatusProcessStrategy statusProcessStrategy) { - mConfiguredValues = configuredValues; + SetsProcessStrategy(@NonNull FlagSetsFilter flagSetsFilter, @NonNull StatusProcessStrategy statusProcessStrategy) { + mStatusProcessStrategy = statusProcessStrategy; + mFlagSetsFilter = flagSetsFilter; } @Override @@ -61,19 +65,21 @@ public void process(List activeFeatureFlags, List archivedFeatureF } boolean shouldArchive = true; + Set newSets = new HashSet<>(); for (String set : featureFlag.sets) { - if (mConfiguredValues.contains(set)) { - featureFlag.sets.retainAll(mConfiguredValues); // Remove all sets that don't match the configured sets + if (mFlagSetsFilter.intersect(set)) { + newSets.add(set); // Remove all sets that don't match the configured sets // Since 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); shouldArchive = false; - break; } } if (shouldArchive) { archivedFeatureFlags.add(featureFlag); + } else { + featureFlag.sets = newSets; + mStatusProcessStrategy.process(activeFeatureFlags, archivedFeatureFlags, 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 88abaf09d..18cd9cea2 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 @@ -9,6 +9,7 @@ import java.util.List; import java.util.Map; +import io.split.android.client.FlagSetsFilter; import io.split.android.client.SplitFilter; import io.split.android.client.dtos.Split; import io.split.android.client.dtos.SplitChange; @@ -20,12 +21,9 @@ public class SplitChangeProcessor { private final StatusProcessStrategy mStatusProcessStrategy; - @VisibleForTesting - SplitChangeProcessor() { - this((SplitFilter) null); - } + private final FlagSetsFilter mFlagSetsFilter; - public SplitChangeProcessor(@Nullable Map filters) { + public SplitChangeProcessor(@Nullable Map filters, FlagSetsFilter flagSetsFilter) { // We're only supporting one filter type if (filters == null || filters.isEmpty()) { mSplitFilter = null; @@ -34,10 +32,12 @@ public SplitChangeProcessor(@Nullable Map filters } mStatusProcessStrategy = new StatusProcessStrategy(); + mFlagSetsFilter = flagSetsFilter; } - public SplitChangeProcessor(@Nullable SplitFilter splitFilter) { + public SplitChangeProcessor(@Nullable SplitFilter splitFilter, @Nullable FlagSetsFilter flagSetsFilter) { mSplitFilter = splitFilter; + mFlagSetsFilter = flagSetsFilter; mStatusProcessStrategy = new StatusProcessStrategy(); } @@ -76,8 +76,8 @@ private FeatureFlagProcessStrategy getProcessStrategy(SplitFilter splitFilter) { return mStatusProcessStrategy; } - if (splitFilter.getType() == SplitFilter.Type.BY_SET) { - return new SetsProcessStrategy(splitFilter.getValues(), mStatusProcessStrategy); + if (splitFilter.getType() == SplitFilter.Type.BY_SET && mFlagSetsFilter != null) { + return new SetsProcessStrategy(mFlagSetsFilter, mStatusProcessStrategy); } else if (splitFilter.getType() == SplitFilter.Type.BY_NAME) { return new NamesProcessStrategy(splitFilter.getValues(), mStatusProcessStrategy); } else { 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 bf7633d17..0745bdb45 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 @@ -10,8 +10,11 @@ import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import io.split.android.client.FlagSetsFilter; +import io.split.android.client.FlagSetsFilterImpl; import io.split.android.client.SplitFilter; import io.split.android.client.dtos.SplitChange; import io.split.android.client.service.ServiceConstants; @@ -49,8 +52,12 @@ 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); + SplitsSyncHelper splitsSyncHelper = new SplitsSyncHelper(splitsFetcher, splitsStorage, - new SplitChangeProcessor(filter), telemetryStorage); + splitChangeProcessor, + telemetryStorage); mSplitTask = buildSplitSyncTask(splitsStorage, telemetryStorage, splitsSyncHelper); } catch (URISyntaxException e) { diff --git a/src/main/java/io/split/android/client/shared/SplitClientContainerImpl.java b/src/main/java/io/split/android/client/shared/SplitClientContainerImpl.java index a23d5e73c..007e868f6 100644 --- a/src/main/java/io/split/android/client/shared/SplitClientContainerImpl.java +++ b/src/main/java/io/split/android/client/shared/SplitClientContainerImpl.java @@ -10,6 +10,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import io.split.android.client.EventsTracker; +import io.split.android.client.FlagSetsFilter; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitClientFactory; @@ -72,7 +73,7 @@ public SplitClientContainerImpl(@NonNull String defaultMatchingKey, @NonNull ClientComponentsRegister clientComponentsRegister, @NonNull MySegmentsWorkManagerWrapper workManagerWrapper, @NonNull EventsTracker eventsTracker, - @NonNull Set configuredFlagSets) { + @Nullable FlagSetsFilter flagSetsFilter) { mDefaultMatchingKey = checkNotNull(defaultMatchingKey); mPushNotificationManager = pushNotificationManager; mStreamingEnabled = config.streamingEnabled(); @@ -91,7 +92,7 @@ public SplitClientContainerImpl(@NonNull String defaultMatchingKey, keyValidator, eventsTracker, customerImpressionListener, - configuredFlagSets + flagSetsFilter ); mClientComponentsRegister = checkNotNull(clientComponentsRegister); mSplitTaskExecutor = checkNotNull(splitTaskExecutor); diff --git a/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java b/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java index 956a94b32..fd4ea363f 100644 --- a/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java +++ b/src/main/java/io/split/android/client/validators/TreatmentManagerFactoryImpl.java @@ -3,11 +3,13 @@ import static com.google.common.base.Preconditions.checkNotNull; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; 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.api.Key; import io.split.android.client.attributes.AttributesManager; import io.split.android.client.attributes.AttributesMerger; @@ -27,7 +29,7 @@ public class TreatmentManagerFactoryImpl implements TreatmentManagerFactory { private final AttributesMerger mAttributesMerger; private final TelemetryStorageProducer mTelemetryStorageProducer; private final Evaluator mEvaluator; - private final Set mConfiguredFlagSets; + private final FlagSetsFilter mFlagSetsFilter; private final SplitsStorage mSplitsStorage; public TreatmentManagerFactoryImpl(@NonNull KeyValidator keyValidator, @@ -37,7 +39,7 @@ public TreatmentManagerFactoryImpl(@NonNull KeyValidator keyValidator, @NonNull AttributesMerger attributesMerger, @NonNull TelemetryStorageProducer telemetryStorageProducer, @NonNull SplitParser splitParser, - @NonNull Set configuredFlagSets, + @Nullable FlagSetsFilter flagSetsFilter, @NonNull SplitsStorage splitsStorage) { mKeyValidator = checkNotNull(keyValidator); mSplitValidator = checkNotNull(splitValidator); @@ -46,7 +48,7 @@ public TreatmentManagerFactoryImpl(@NonNull KeyValidator keyValidator, mAttributesMerger = checkNotNull(attributesMerger); mTelemetryStorageProducer = checkNotNull(telemetryStorageProducer); mEvaluator = new EvaluatorImpl(splitsStorage, splitParser); - mConfiguredFlagSets = checkNotNull(configuredFlagSets); + mFlagSetsFilter = flagSetsFilter; mSplitsStorage = checkNotNull(splitsStorage); } @@ -64,7 +66,7 @@ public TreatmentManager getTreatmentManager(Key key, ListenableEventsManager eve attributesManager, mAttributesMerger, mTelemetryStorageProducer, - mConfiguredFlagSets, + mFlagSetsFilter, mSplitsStorage ); } diff --git a/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java b/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java index c45f553d5..3d4be2839 100644 --- a/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java +++ b/src/main/java/io/split/android/client/validators/TreatmentManagerImpl.java @@ -15,6 +15,7 @@ import io.split.android.client.EvaluationResult; import io.split.android.client.Evaluator; +import io.split.android.client.FlagSetsFilter; import io.split.android.client.SplitResult; import io.split.android.client.TreatmentLabels; import io.split.android.client.attributes.AttributesManager; @@ -58,7 +59,7 @@ private static class ValidationTag { @NonNull private final AttributesMerger mAttributesMerger; private final TelemetryStorageProducer mTelemetryStorageProducer; - private final Set mConfiguredFlagSets; + private final FlagSetsFilter mFlagSetsFilter; private final SplitsStorage mSplitsStorage; private final SplitFilterValidator mFlagSetsValidator; @@ -73,7 +74,7 @@ public TreatmentManagerImpl(String matchingKey, @NonNull AttributesManager attributesManager, @NonNull AttributesMerger attributesMerger, @NonNull TelemetryStorageProducer telemetryStorageProducer, - @NonNull Set configuredFlagSets, + @Nullable FlagSetsFilter flagSetsFilter, @NonNull SplitsStorage splitsStorage) { mEvaluator = evaluator; mKeyValidator = keyValidator; @@ -87,7 +88,7 @@ public TreatmentManagerImpl(String matchingKey, mAttributesManager = checkNotNull(attributesManager); mAttributesMerger = checkNotNull(attributesMerger); mTelemetryStorageProducer = checkNotNull(telemetryStorageProducer); - mConfiguredFlagSets = checkNotNull(configuredFlagSets); + mFlagSetsFilter = flagSetsFilter; mSplitsStorage = checkNotNull(splitsStorage); mFlagSetsValidator = new FlagSetsValidatorImpl(); } @@ -415,7 +416,7 @@ private Set getNamesFromSet(String validationTag, } boolean isValid = mFlagSetsValidator.isValid(flagSet); - boolean isConfigured = mConfiguredFlagSets.isEmpty() || mConfiguredFlagSets.contains(flagSet); + boolean isConfigured = mFlagSetsFilter.intersect(flagSet); if (!isValid) { mValidationLogger.e("you passed " + flagSet + " which is not valid.", validationTag); diff --git a/src/test/java/io/split/android/client/FlagSetsFilterImplTest.java b/src/test/java/io/split/android/client/FlagSetsFilterImplTest.java new file mode 100644 index 000000000..97e68c155 --- /dev/null +++ b/src/test/java/io/split/android/client/FlagSetsFilterImplTest.java @@ -0,0 +1,73 @@ +package io.split.android.client; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import java.util.HashSet; +import java.util.Set; + +public class FlagSetsFilterImplTest { + + + @Test + public void intersectReturnsTrueWhenShouldFilterIsFalse() { + Set flagSets = new HashSet<>(); + FlagSetsFilterImpl filter = new FlagSetsFilterImpl(flagSets); + assertTrue(filter.intersect(new HashSet<>())); + } + + @Test + public void intersectReturnsTrueWhenSetsIsNull() { + Set flagSets = new HashSet<>(); + flagSets.add("test"); + FlagSetsFilterImpl filter = new FlagSetsFilterImpl(flagSets); + assertFalse(filter.intersect((Set) null)); + } + + @Test + public void intersectReturnsTrueWhenSetIsContained() { + Set flagSets = new HashSet<>(); + flagSets.add("test"); + FlagSetsFilterImpl filter = new FlagSetsFilterImpl(flagSets); + Set testSet = new HashSet<>(); + testSet.add("test"); + assertTrue(filter.intersect(testSet)); + } + + @Test + public void intersectReturnsFalseWhenSetIsNotContained() { + Set flagSets = new HashSet<>(); + flagSets.add("test"); + FlagSetsFilterImpl filter = new FlagSetsFilterImpl(flagSets); + Set testSet = new HashSet<>(); + testSet.add("other"); + assertFalse(filter.intersect(testSet)); + } + + @Test + public void intersectReturnsTrueWhenStringSetIsNull() { + Set flagSets = new HashSet<>(); + flagSets.add("test"); + FlagSetsFilterImpl filter = new FlagSetsFilterImpl(flagSets); + assertFalse(filter.intersect((String) null)); + } + + @Test + public void intersectReturnsTrueWhenStringSetIsContained() { + Set flagSets = new HashSet<>(); + flagSets.add("test"); + FlagSetsFilterImpl filter = new FlagSetsFilterImpl(flagSets); + assertTrue(filter.intersect("test")); + } + + @Test + public void intersectReturnsFalseWhenStringSetIsNotContained() { + Set flagSets = new HashSet<>(); + flagSets.add("test"); + FlagSetsFilterImpl filter = new FlagSetsFilterImpl(flagSets); + assertFalse(filter.intersect("other")); + } + +} diff --git a/src/test/java/io/split/android/client/service/splits/SplitChangeProcessorTest.java b/src/test/java/io/split/android/client/service/splits/SplitChangeProcessorTest.java index d0944a07b..191896074 100644 --- a/src/test/java/io/split/android/client/service/splits/SplitChangeProcessorTest.java +++ b/src/test/java/io/split/android/client/service/splits/SplitChangeProcessorTest.java @@ -14,6 +14,7 @@ import java.util.Map; import java.util.Set; +import io.split.android.client.FlagSetsFilterImpl; import io.split.android.client.SplitFilter; import io.split.android.client.dtos.Split; import io.split.android.client.dtos.SplitChange; @@ -26,7 +27,7 @@ public class SplitChangeProcessorTest { @Before public void setup() { - mProcessor = new SplitChangeProcessor(); + mProcessor = new SplitChangeProcessor((SplitFilter) null, null); } @Test @@ -150,7 +151,7 @@ public void processAddingWithFlagSets() { configuredSets.add("set_1"); configuredSets.add("set_2"); SplitFilter filter = SplitFilter.bySet(new ArrayList<>(configuredSets)); - mProcessor = new SplitChangeProcessor(filter); + mProcessor = new SplitChangeProcessor(filter, new FlagSetsFilterImpl(configuredSets)); 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")); @@ -172,7 +173,7 @@ public void featureFlagWithNoSetsIsArchivedWhenProcessingWithFlagSets() { configuredSets.add("set_2"); SplitFilter filter = SplitFilter.bySet(new ArrayList<>(configuredSets)); - mProcessor = new SplitChangeProcessor(filter); + mProcessor = new SplitChangeProcessor(filter, new FlagSetsFilterImpl(configuredSets)); Split split1 = newSplit("split_1", Status.ACTIVE, null); @@ -189,7 +190,7 @@ public void featureFlagWithNoSetsIsArchivedWhenProcessingWithFlagSets() { public void featureFlagsAreFilteredByNameWhenThereIsSplitFilterByName() { SplitFilter filter = SplitFilter.byName(Arrays.asList("split_1", "split_2")); - mProcessor = new SplitChangeProcessor(filter); + mProcessor = new SplitChangeProcessor(filter, null); Split split1 = newSplit("split_1", Status.ACTIVE); Split split2 = newSplit("split_2", Status.ARCHIVED); @@ -208,7 +209,7 @@ public void featureFlagsAreFilteredByNameWhenThereIsSplitFilterByName() { @Test public void creatingWithNullFilterProcessesEverything() { Map filterMap = null; - mProcessor = new SplitChangeProcessor(filterMap); + mProcessor = new SplitChangeProcessor(filterMap, null); Split split1 = newSplit("split_1", Status.ACTIVE); Split split2 = newSplit("split_2", Status.ARCHIVED); @@ -228,7 +229,7 @@ public void creatingWithNullFilterProcessesEverything() { public void creatingWithFilterWithEmptyConfiguredValuesProcessesEverything() { Map filterMap = Collections.singletonMap(SplitFilter.Type.BY_SET, SplitFilter.bySet(Collections.emptyList())); - mProcessor = new SplitChangeProcessor(filterMap); + mProcessor = new SplitChangeProcessor(filterMap, new FlagSetsFilterImpl(Collections.emptySet())); Split split1 = newSplit("split_1", Status.ACTIVE); Split split2 = newSplit("split_2", Status.ARCHIVED); @@ -250,7 +251,7 @@ public void nonConfiguredSetsAreRemovedFromSplit() { configuredSets.add("set_1"); configuredSets.add("set_2"); SplitFilter filter = SplitFilter.bySet(new ArrayList<>(configuredSets)); - mProcessor = new SplitChangeProcessor(filter); + mProcessor = new SplitChangeProcessor(filter, new FlagSetsFilterImpl(configuredSets)); Split split1 = newSplit("split_1", Status.ACTIVE, new HashSet<>(Arrays.asList("set_1", "set_3"))); Split split2 = newSplit("split_2", Status.ACTIVE, new HashSet<>(Arrays.asList("set_2", "set_asd"))); From bf91a01bf79a2e2caba8d0ec02926f7a5f0a838a Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 14 Sep 2023 16:12:16 -0300 Subject: [PATCH 4/7] Update tests --- .../client/TreatmentManagerTelemetryTest.java | 7 +-- .../android/client/TreatmentManagerTest.java | 8 +-- .../TreatmentManagerWithFlagSetsTest.java | 63 +++++++++++-------- ...LocalhostSplitClientContainerImplTest.java | 9 +-- .../client/utils/SplitClientImplFactory.java | 4 +- 5 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/test/java/io/split/android/client/TreatmentManagerTelemetryTest.java b/src/test/java/io/split/android/client/TreatmentManagerTelemetryTest.java index feb47d7ce..74681726d 100644 --- a/src/test/java/io/split/android/client/TreatmentManagerTelemetryTest.java +++ b/src/test/java/io/split/android/client/TreatmentManagerTelemetryTest.java @@ -17,7 +17,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.Set; import io.split.android.client.attributes.AttributesManager; import io.split.android.client.attributes.AttributesMerger; @@ -52,14 +51,14 @@ public class TreatmentManagerTelemetryTest { @Mock private SplitsStorage mSplitsStorage; - private Set mConfiguredFlagSets = new HashSet<>(); + private FlagSetsFilter mFlagSetsFilter; private TreatmentManagerImpl treatmentManager; private AutoCloseable mAutoCloseable; @Before public void setUp() { mAutoCloseable = MockitoAnnotations.openMocks(this); - + mFlagSetsFilter = new FlagSetsFilterImpl(new HashSet<>()); treatmentManager = new TreatmentManagerImpl( "test_key", "test_key", @@ -72,7 +71,7 @@ public void setUp() { attributesManager, attributesMerger, telemetryStorageProducer, - mConfiguredFlagSets, + mFlagSetsFilter, mSplitsStorage); when(evaluator.getTreatment(anyString(), anyString(), anyString(), anyMap())).thenReturn(new EvaluationResult("test", "label")); diff --git a/src/test/java/io/split/android/client/TreatmentManagerTest.java b/src/test/java/io/split/android/client/TreatmentManagerTest.java index edb116fd3..cd2423bf8 100644 --- a/src/test/java/io/split/android/client/TreatmentManagerTest.java +++ b/src/test/java/io/split/android/client/TreatmentManagerTest.java @@ -54,13 +54,13 @@ public class TreatmentManagerTest { ListenableEventsManager eventsManagerStub; AttributesManager attributesManager = mock(AttributesManager.class); TelemetryStorageProducer telemetryStorageProducer = mock(TelemetryStorageProducer.class); - private Set mConfiguredFlagSets; + private FlagSetsFilter mFlagSetsFilter; TreatmentManagerImpl treatmentManager; private SplitsStorage mSplitsStorage; @Before public void loadSplitsFromFile() { - mConfiguredFlagSets = new HashSet<>(); + mFlagSetsFilter = new FlagSetsFilterImpl(new HashSet<>()); mSplitsStorage = mock(SplitsStorage.class); treatmentManager = initializeTreatmentManager(); if (evaluator == null) { @@ -325,7 +325,7 @@ private TreatmentManager createTreatmentManager(String matchingKey, String bucke new KeyValidatorImpl(), new SplitValidatorImpl(), new ImpressionListenerMock(), config.labelsEnabled(), eventsManagerStub, mock(AttributesManager.class), mock(AttributesMerger.class), - mock(TelemetryStorageProducer.class), mConfiguredFlagSets, mSplitsStorage); + mock(TelemetryStorageProducer.class), mFlagSetsFilter, mSplitsStorage); } private TreatmentManagerImpl initializeTreatmentManager() { @@ -353,7 +353,7 @@ private TreatmentManagerImpl initializeTreatmentManager(Evaluator evaluator) { attributesManager, mock(AttributesMerger.class), telemetryStorageProducer, - mConfiguredFlagSets, + mFlagSetsFilter, mSplitsStorage); } diff --git a/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java b/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java index 42281896f..dc2cc12e8 100644 --- a/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java +++ b/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java @@ -55,7 +55,7 @@ public class TreatmentManagerWithFlagSetsTest { @Mock private SplitsStorage mSplitsStorage; - private Set mConfiguredFlagSets; + private FlagSetsFilter mFlagSetsFilter; private TreatmentManagerImpl mTreatmentManager; private AutoCloseable mAutoCloseable; @@ -63,24 +63,11 @@ public class TreatmentManagerWithFlagSetsTest { public void setUp() { mAutoCloseable = MockitoAnnotations.openMocks(this); - mConfiguredFlagSets = new HashSet<>(); + mFlagSetsFilter = new FlagSetsFilterImpl(new HashSet<>()); when(mEventsManager.eventAlreadyTriggered(SplitEvent.SDK_READY)).thenReturn(true); when(mEventsManager.eventAlreadyTriggered(SplitEvent.SDK_READY_FROM_CACHE)).thenReturn(true); - mTreatmentManager = new TreatmentManagerImpl( - "matching_key", - "bucketing_key", - mEvaluator, - mKeyValidator, - mSplitValidator, - mImpressionListener, - SplitClientConfig.builder().build().labelsEnabled(), - mEventsManager, - mAttributesManager, - mAttributesMerger, - mTelemetryStorageProducer, - mConfiguredFlagSets, - mSplitsStorage); + initializeTreatmentManager(); when(mEvaluator.getTreatment(anyString(), anyString(), eq("test_1"), anyMap())) .thenReturn(new EvaluationResult("result_1", "label")); @@ -129,7 +116,7 @@ public void getTreatmentsByFlagSetWithNoConfiguredSetsInvalidSetDoesNotQueryStor @Test public void getTreatmentsByFlagSetWithConfiguredSetsExistingSetQueriesStorageAndUsesEvaluator() { - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); @@ -141,7 +128,8 @@ public void getTreatmentsByFlagSetWithConfiguredSetsExistingSetQueriesStorageAnd @Test public void getTreatmentsByFlagSetWithConfiguredSetsNonExistingSetDoesNotQueryStorageNorUseEvaluator() { - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); + initializeTreatmentManager(); when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_split"))); @@ -151,13 +139,30 @@ public void getTreatmentsByFlagSetWithConfiguredSetsNonExistingSetDoesNotQuerySt verify(mEvaluator, times(0)).getTreatment(any(), any(), any(), anyMap()); } + private void initializeTreatmentManager() { + mTreatmentManager = new TreatmentManagerImpl( + "matching_key", + "bucketing_key", + mEvaluator, + mKeyValidator, + mSplitValidator, + mImpressionListener, + SplitClientConfig.builder().build().labelsEnabled(), + mEventsManager, + mAttributesManager, + mAttributesMerger, + mTelemetryStorageProducer, + mFlagSetsFilter, + mSplitsStorage); + } + @Test public void getTreatmentsByFlagSetReturnsCorrectFormat() { Set mockNames = new HashSet<>(); mockNames.add("test_1"); mockNames.add("test_2"); when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))).thenReturn(mockNames); - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); Map result = mTreatmentManager.getTreatmentsByFlagSet("set_1", null, false); @@ -209,7 +214,8 @@ public void getTreatmentsByFlagSetsWithNoConfiguredSetsInvalidSetDoesNotQuerySto @Test public void getTreatmentsByFlagSetsWithConfiguredSetsExistingSetQueriesStorageForConfiguredSetOnlyAndUsesEvaluator() { - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); + initializeTreatmentManager(); when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); @@ -221,7 +227,8 @@ public void getTreatmentsByFlagSetsWithConfiguredSetsExistingSetQueriesStorageFo @Test public void getTreatmentsByFlagSetsWithConfiguredSetsNonExistingSetDoesNotQueryStorageNorUseEvaluator() { - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); + initializeTreatmentManager(); mTreatmentManager.getTreatmentsByFlagSets(Arrays.asList("set_2", "set_3"), null, false); @@ -301,7 +308,7 @@ public void getTreatmentsWithConfigByFlagSetWithNoConfiguredSetsInvalidSetDoesNo @Test public void getTreatmentsWithConfigByFlagSetWithConfiguredSetsExistingSetQueriesStorageAndUsesEvaluator() { - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); @@ -313,7 +320,8 @@ public void getTreatmentsWithConfigByFlagSetWithConfiguredSetsExistingSetQueries @Test public void getTreatmentsWithConfigByFlagSetWithConfiguredSetsNonExistingSetDoesNotQueryStorageNorUseEvaluator() { - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); + initializeTreatmentManager(); when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_split"))); @@ -329,7 +337,7 @@ public void getTreatmentsWithConfigByFlagSetReturnsCorrectFormat() { mockNames.add("test_1"); mockNames.add("test_2"); when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))).thenReturn(mockNames); - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); Map result = mTreatmentManager.getTreatmentsWithConfigByFlagSet("set_1", null, false); @@ -381,7 +389,9 @@ public void getTreatmentsWithConfigByFlagSetsWithNoConfiguredSetsInvalidSetDoesN @Test public void getTreatmentsWithConfigByFlagSetsWithConfiguredSetsExistingSetQueriesStorageForConfiguredSetOnlyAndUsesEvaluator() { - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); + initializeTreatmentManager(); + when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); @@ -393,7 +403,8 @@ public void getTreatmentsWithConfigByFlagSetsWithConfiguredSetsExistingSetQuerie @Test public void getTreatmentsWithConfigByFlagSetsWithConfiguredSetsNonExistingSetDoesNotQueryStorageNorUseEvaluator() { - mConfiguredFlagSets.add("set_1"); + mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); + initializeTreatmentManager(); mTreatmentManager.getTreatmentsWithConfigByFlagSets(Arrays.asList("set_2", "set_3"), null, false); diff --git a/src/test/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImplTest.java b/src/test/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImplTest.java index 9d855de66..2088b392a 100644 --- a/src/test/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImplTest.java +++ b/src/test/java/io/split/android/client/localhost/shared/LocalhostSplitClientContainerImplTest.java @@ -17,8 +17,9 @@ import java.util.Collection; import java.util.HashSet; -import java.util.Set; +import io.split.android.client.FlagSetsFilter; +import io.split.android.client.FlagSetsFilterImpl; import io.split.android.client.SplitClient; import io.split.android.client.SplitClientConfig; import io.split.android.client.api.Key; @@ -52,14 +53,14 @@ public class LocalhostSplitClientContainerImplTest { private SplitClientConfig mConfig; @Mock private SplitTaskExecutor mTaskExecutor; - private Set mConfiguredSets; + private FlagSetsFilter mFlagSetsFilter; private LocalhostSplitClientContainerImpl mClientContainer; @Before public void setUp() { MockitoAnnotations.openMocks(this); when(mAttributesManagerFactory.getManager(any(), any())).thenReturn(mock(AttributesManager.class)); - mConfiguredSets = new HashSet<>(); + mFlagSetsFilter = new FlagSetsFilterImpl(new HashSet<>()); mClientContainer = getClientContainer(); } @@ -108,6 +109,6 @@ private LocalhostSplitClientContainerImpl getClientContainer() { mTelemetryStorageProducer, mEventsManagerCoordinator, mTaskExecutor, - mConfiguredSets); + mFlagSetsFilter); } } diff --git a/src/test/java/io/split/android/client/utils/SplitClientImplFactory.java b/src/test/java/io/split/android/client/utils/SplitClientImplFactory.java index 03fe34446..e20344b3e 100644 --- a/src/test/java/io/split/android/client/utils/SplitClientImplFactory.java +++ b/src/test/java/io/split/android/client/utils/SplitClientImplFactory.java @@ -4,8 +4,8 @@ import java.util.Collections; -import io.split.android.client.EvaluatorImpl; import io.split.android.client.EventsTracker; +import io.split.android.client.FlagSetsFilterImpl; import io.split.android.client.SplitClientConfig; import io.split.android.client.SplitClientImpl; import io.split.android.client.SplitFactory; @@ -39,7 +39,7 @@ public static SplitClientImpl get(Key key, SplitsStorage splitsStorage) { TreatmentManagerFactory treatmentManagerFactory = new TreatmentManagerFactoryImpl( new KeyValidatorImpl(), new SplitValidatorImpl(), new ImpressionListener.NoopImpressionListener(), false, new AttributesMergerImpl(), telemetryStorage, splitParser, - Collections.emptySet(), splitsStorage); + new FlagSetsFilterImpl(Collections.emptySet()), splitsStorage); AttributesManager attributesManager = mock(AttributesManager.class); SplitClientImpl c = new SplitClientImpl( From ef81e369d02a9a19ea5e44d702133f18963e0f7c Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 14 Sep 2023 16:45:00 -0300 Subject: [PATCH 5/7] Fix test --- .../client/service/splits/SplitChangeProcessor.java | 7 +++++++ 1 file changed, 7 insertions(+) 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 18cd9cea2..3c0534bd7 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 @@ -23,6 +23,13 @@ public class SplitChangeProcessor { private final FlagSetsFilter mFlagSetsFilter; + /** @noinspection unused*/ // Used in tests + private SplitChangeProcessor() { + mSplitFilter = null; + mStatusProcessStrategy = new StatusProcessStrategy(); + mFlagSetsFilter = null; + } + public SplitChangeProcessor(@Nullable Map filters, FlagSetsFilter flagSetsFilter) { // We're only supporting one filter type if (filters == null || filters.isEmpty()) { From 9c284d75ed8a9ff25b70e2bd2689b23dcdd5d72e Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 15 Sep 2023 12:02:47 -0300 Subject: [PATCH 6/7] Filter integration --- .../integration/sets/FlagSetsPollingTest.java | 1 - .../userconsent/UserConsentModeDebugTest.kt | 4 +- .../userconsent/UserConsentModeNoneTest.kt | 4 +- .../UserConsentModeOptimizedTest.kt | 4 +- .../android/client/FlagSetsFilterImpl.java | 3 +- .../android/client/SplitFactoryImpl.java | 2 +- .../io/split/android/client/SplitFilter.java | 2 +- .../localhost/LocalhostSplitClient.java | 1 - .../localhost/LocalhostSplitFactory.java | 2 +- .../localhost/LocalhostSplitsStorage.java | 3 +- .../splits/FeatureFlagProcessStrategy.java | 3 +- .../service/workmanager/SplitsSyncWorker.java | 2 +- .../client/storage/splits/SplitsStorage.java | 3 +- .../storage/splits/SplitsStorageImpl.java | 3 +- .../validators/FlagSetsValidatorImpl.java | 34 +++++++-- .../validators/SplitFilterValidator.java | 7 +- .../validators/TreatmentManagerImpl.java | 33 ++------- .../TreatmentManagerWithFlagSetsTest.java | 70 +++++++++---------- .../validators/FlagSetsValidatorImplTest.java | 20 +++--- 19 files changed, 107 insertions(+), 94 deletions(-) diff --git a/src/androidTest/java/tests/integration/sets/FlagSetsPollingTest.java b/src/androidTest/java/tests/integration/sets/FlagSetsPollingTest.java index 8c3c095f7..ccb4e94c1 100644 --- 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 index b162d2a54..4c0d9eb02 100644 --- 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 index dddec5f6a..106bfd65b 100644 --- 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 index 4a6ab033b..28e9fdb77 100644 --- 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 index 84ab5c839..d8c731280 100644 --- 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 mFlagSets; - public FlagSetsFilterImpl(Set flagSets) { + public FlagSetsFilterImpl(Collection 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 index 11519d18b..e798dcdcd 100644 --- 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 index 908f01c34..38b1a0721 100644 --- 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 values) { SplitFilter(Type type, List 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 index 941f3ee7a..c70d79420 100644 --- 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 index 0948fad70..e65838367 100644 --- 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 index 76bc60b3d..989d9a16c 100644 --- 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 getNamesByFlagSets(List sets) { + public Set getNamesByFlagSets(Collection sets) { Set 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 index f61871de0..1cffb6669 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 @@ -68,7 +68,7 @@ public void process(List activeFeatureFlags, List archivedFeatureF Set 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 activeFeatureFlags, List 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 index 0745bdb45..982e2fcae 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 @@ -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 index d152243b5..62af70e7b 100644 --- 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 getNamesByFlagSets(List flagSets); + Set getNamesByFlagSets(Collection 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 index 2f3eb99f6..b143c209c 100644 --- 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 getNamesByFlagSets(List sets) { + public Set getNamesByFlagSets(Collection sets) { Set 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 index 5e14a45a3..a9dbcb6b5 100644 --- 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 values) { + public ValidationResult cleanup(String method, List values) { if (values == null || values.isEmpty()) { return new ValidationResult(Collections.emptyList(), 0); } @@ -34,12 +37,12 @@ public ValidationResult cleanup(List 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(); } @@ -47,7 +50,7 @@ public ValidationResult cleanup(List values) { 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 values) { public boolean isValid(String value) { return value != null && value.trim().matches(FLAG_SET_REGEX); } + + @Override + public Set items(List values, FlagSetsFilter flagSetsFilter) { + Set 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 index 110043d47..b5f0d1b2b 100644 --- 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 values); + ValidationResult cleanup(String method, List values); boolean isValid(String value); + Set items(List values, FlagSetsFilter flagSetsFilter); + class ValidationResult { private final List 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 index 3d4be2839..8a5511467 100644 --- 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 getTreatmentsByFlagSet(@NonNull String flagSet, @Null String validationTag = ValidationTag.GET_TREATMENTS_BY_FLAG_SET; Set 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 getTreatmentsByFlagSets(@NonNull List flagSet String validationTag = ValidationTag.GET_TREATMENTS_BY_FLAG_SETS; Set 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 getTreatmentsWithConfigByFlagSet(@NonNull String String validationTag = ValidationTag.GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SET; Set 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 getTreatmentsWithConfigByFlagSets(@NonNull List< String validationTag = ValidationTag.GET_TREATMENTS_WITH_CONFIG_BY_FLAG_SETS; Set 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 getNamesFromSet(String validationTag, - @NonNull List flagSets) { + private Set getNamesFromSet(@NonNull List flagSets) { - if (flagSets == null) { - return new HashSet<>(); - } - - List 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 setsToEvaluate = mFlagSetsValidator.items(flagSets, mFlagSetsFilter); if (setsToEvaluate.isEmpty()) { return new HashSet<>(); diff --git a/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java b/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java index dc2cc12e8..dc1f4efc5 100644 --- a/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java +++ b/src/test/java/io/split/android/client/TreatmentManagerWithFlagSetsTest.java @@ -94,18 +94,18 @@ public void getTreatmentsByFlagSetDestroyedDoesNotUseEvaluator() { @Test public void getTreatmentsByFlagSetWithNoConfiguredSetsQueriesStorageAndUsesEvaluator() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); mTreatmentManager.getTreatmentsByFlagSet("set_1", null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); verify(mEvaluator).getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_1"), anyMap()); } @Test public void getTreatmentsByFlagSetWithNoConfiguredSetsInvalidSetDoesNotQueryStorageNorUseEvaluator() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_split"))); mTreatmentManager.getTreatmentsByFlagSet("SET!", null, false); @@ -117,12 +117,12 @@ public void getTreatmentsByFlagSetWithNoConfiguredSetsInvalidSetDoesNotQueryStor @Test public void getTreatmentsByFlagSetWithConfiguredSetsExistingSetQueriesStorageAndUsesEvaluator() { mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); mTreatmentManager.getTreatmentsByFlagSet("set_1", null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); verify(mEvaluator).getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_1"), anyMap()); } @@ -130,7 +130,7 @@ public void getTreatmentsByFlagSetWithConfiguredSetsExistingSetQueriesStorageAnd public void getTreatmentsByFlagSetWithConfiguredSetsNonExistingSetDoesNotQueryStorageNorUseEvaluator() { mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); initializeTreatmentManager(); - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_split"))); mTreatmentManager.getTreatmentsByFlagSet("set_2", null, false); @@ -161,7 +161,7 @@ public void getTreatmentsByFlagSetReturnsCorrectFormat() { Set mockNames = new HashSet<>(); mockNames.add("test_1"); mockNames.add("test_2"); - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))).thenReturn(mockNames); + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))).thenReturn(mockNames); mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); Map result = mTreatmentManager.getTreatmentsByFlagSet("set_1", null, false); @@ -173,7 +173,7 @@ public void getTreatmentsByFlagSetReturnsCorrectFormat() { @Test public void getTreatmentsByFlagSetRecordsTelemetry() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))).thenReturn(Collections.singleton("test_1")); + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))).thenReturn(Collections.singleton("test_1")); mTreatmentManager.getTreatmentsByFlagSet("set_1", null, false); @@ -191,24 +191,24 @@ public void getTreatmentsByFlagSetsDestroyedDoesNotUseEvaluator() { @Test public void getTreatmentsByFlagSetsWithNoConfiguredSetsQueriesStorageAndUsesEvaluator() { - when(mSplitsStorage.getNamesByFlagSets(Arrays.asList("set_1", "set_2"))) + when(mSplitsStorage.getNamesByFlagSets(new HashSet<>(Arrays.asList("set_1", "set_2")))) .thenReturn(new HashSet<>(Arrays.asList("test_1", "test_2"))); mTreatmentManager.getTreatmentsByFlagSets(Arrays.asList("set_1", "set_2"), null, false); - verify(mSplitsStorage).getNamesByFlagSets(Arrays.asList("set_1", "set_2")); + verify(mSplitsStorage).getNamesByFlagSets(new HashSet<>(Arrays.asList("set_1", "set_2"))); verify(mEvaluator).getTreatment(anyString(), anyString(), eq("test_1"), anyMap()); verify(mEvaluator).getTreatment(anyString(), anyString(), eq("test_2"), anyMap()); } @Test public void getTreatmentsByFlagSetsWithNoConfiguredSetsInvalidSetDoesNotQueryStorageForInvalidSet() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); mTreatmentManager.getTreatmentsByFlagSets(Arrays.asList("set_1", "SET!"), null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); verify(mEvaluator).getTreatment(any(), any(), eq("test_1"), anyMap()); } @@ -216,12 +216,12 @@ public void getTreatmentsByFlagSetsWithNoConfiguredSetsInvalidSetDoesNotQuerySto public void getTreatmentsByFlagSetsWithConfiguredSetsExistingSetQueriesStorageForConfiguredSetOnlyAndUsesEvaluator() { mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); initializeTreatmentManager(); - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); mTreatmentManager.getTreatmentsByFlagSets(Arrays.asList("set_1", "set_2"), null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); verify(mEvaluator).getTreatment(anyString(), anyString(), eq("test_1"), anyMap()); } @@ -241,7 +241,7 @@ public void getTreatmentsByFlagSetsReturnsCorrectFormat() { Set mockNames = new HashSet<>(); mockNames.add("test_1"); mockNames.add("test_2"); - when(mSplitsStorage.getNamesByFlagSets(Arrays.asList("set_1", "set_2"))).thenReturn(mockNames); + when(mSplitsStorage.getNamesByFlagSets(new HashSet<>(Arrays.asList("set_1", "set_2")))).thenReturn(mockNames); Map result = mTreatmentManager.getTreatmentsByFlagSets(Arrays.asList("set_1", "set_2"), null, false); @@ -254,7 +254,7 @@ public void getTreatmentsByFlagSetsReturnsCorrectFormat() { public void getTreatmentsByFlagSetsWithDuplicatedSetDeduplicates() { mTreatmentManager.getTreatmentsByFlagSets(Arrays.asList("set_1", "set_1"), null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); } @Test @@ -268,7 +268,7 @@ public void getTreatmentsByFlagSetsWithNullSetListReturnsEmpty() { @Test public void getTreatmentsByFlagSetsRecordsTelemetry() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))).thenReturn(Collections.singleton("test_1")); + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))).thenReturn(Collections.singleton("test_1")); mTreatmentManager.getTreatmentsByFlagSets(Arrays.asList("set_1", "set_2"), null, false); @@ -286,18 +286,18 @@ public void getTreatmentsWithConfigByFlagSetDestroyedDoesNotUseEvaluator() { @Test public void getTreatmentsWithConfigByFlagSetWithNoConfiguredSetsQueriesStorageAndUsesEvaluator() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) - .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) + .thenReturn(Collections.singleton("test_1")); mTreatmentManager.getTreatmentsWithConfigByFlagSet("set_1", null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); verify(mEvaluator).getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_1"), anyMap()); } @Test public void getTreatmentsWithConfigByFlagSetWithNoConfiguredSetsInvalidSetDoesNotQueryStorageNorUseEvaluator() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_split"))); mTreatmentManager.getTreatmentsWithConfigByFlagSet("SET!", null, false); @@ -309,12 +309,12 @@ public void getTreatmentsWithConfigByFlagSetWithNoConfiguredSetsInvalidSetDoesNo @Test public void getTreatmentsWithConfigByFlagSetWithConfiguredSetsExistingSetQueriesStorageAndUsesEvaluator() { mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); mTreatmentManager.getTreatmentsWithConfigByFlagSet("set_1", null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); verify(mEvaluator).getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_1"), anyMap()); } @@ -322,7 +322,7 @@ public void getTreatmentsWithConfigByFlagSetWithConfiguredSetsExistingSetQueries public void getTreatmentsWithConfigByFlagSetWithConfiguredSetsNonExistingSetDoesNotQueryStorageNorUseEvaluator() { mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); initializeTreatmentManager(); - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_split"))); mTreatmentManager.getTreatmentsWithConfigByFlagSet("set_2", null, false); @@ -336,7 +336,7 @@ public void getTreatmentsWithConfigByFlagSetReturnsCorrectFormat() { Set mockNames = new HashSet<>(); mockNames.add("test_1"); mockNames.add("test_2"); - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))).thenReturn(mockNames); + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))).thenReturn(mockNames); mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); Map result = mTreatmentManager.getTreatmentsWithConfigByFlagSet("set_1", null, false); @@ -348,7 +348,7 @@ public void getTreatmentsWithConfigByFlagSetReturnsCorrectFormat() { @Test public void getTreatmentsWithConfigByFlagSetRecordsTelemetry() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))).thenReturn(Collections.singleton("test_1")); + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))).thenReturn(Collections.singleton("test_1")); mTreatmentManager.getTreatmentsWithConfigByFlagSet("set_1", null, false); @@ -366,24 +366,24 @@ public void getTreatmentsWithConfigByFlagSetsDestroyedDoesNotUseEvaluator() { @Test public void getTreatmentsWithConfigByFlagSetsWithNoConfiguredSetsQueriesStorageAndUsesEvaluator() { - when(mSplitsStorage.getNamesByFlagSets(Arrays.asList("set_1", "set_2"))) + when(mSplitsStorage.getNamesByFlagSets(new HashSet<>(Arrays.asList("set_1", "set_2")))) .thenReturn(new HashSet<>(Arrays.asList("test_1", "test_2"))); mTreatmentManager.getTreatmentsWithConfigByFlagSets(Arrays.asList("set_1", "set_2"), null, false); - verify(mSplitsStorage).getNamesByFlagSets(Arrays.asList("set_1", "set_2")); + verify(mSplitsStorage).getNamesByFlagSets(new HashSet<>(Arrays.asList("set_1", "set_2"))); verify(mEvaluator).getTreatment(anyString(), anyString(), eq("test_1"), anyMap()); verify(mEvaluator).getTreatment(anyString(), anyString(), eq("test_2"), anyMap()); } @Test public void getTreatmentsWithConfigByFlagSetsWithNoConfiguredSetsInvalidSetDoesNotQueryStorageForInvalidSet() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); mTreatmentManager.getTreatmentsWithConfigByFlagSets(Arrays.asList("set_1", "SET!"), null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); verify(mEvaluator).getTreatment(any(), any(), eq("test_1"), anyMap()); } @@ -392,12 +392,12 @@ public void getTreatmentsWithConfigByFlagSetsWithConfiguredSetsExistingSetQuerie mFlagSetsFilter = new FlagSetsFilterImpl(Collections.singleton("set_1")); initializeTreatmentManager(); - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))) + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))) .thenReturn(new HashSet<>(Collections.singletonList("test_1"))); mTreatmentManager.getTreatmentsWithConfigByFlagSets(Arrays.asList("set_1", "set_2"), null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); verify(mEvaluator).getTreatment(anyString(), anyString(), eq("test_1"), anyMap()); } @@ -417,7 +417,7 @@ public void getTreatmentsWithConfigByFlagSetsReturnsCorrectFormat() { Set mockNames = new HashSet<>(); mockNames.add("test_1"); mockNames.add("test_2"); - when(mSplitsStorage.getNamesByFlagSets(Arrays.asList("set_1", "set_2"))).thenReturn(mockNames); + when(mSplitsStorage.getNamesByFlagSets(new HashSet<>(Arrays.asList("set_1", "set_2")))).thenReturn(mockNames); Map result = mTreatmentManager.getTreatmentsWithConfigByFlagSets(Arrays.asList("set_1", "set_2"), null, false); @@ -430,7 +430,7 @@ public void getTreatmentsWithConfigByFlagSetsReturnsCorrectFormat() { public void getTreatmentsWithConfigByFlagSetsWithDuplicatedSetDeduplicates() { mTreatmentManager.getTreatmentsWithConfigByFlagSets(Arrays.asList("set_1", "set_1"), null, false); - verify(mSplitsStorage).getNamesByFlagSets(Collections.singletonList("set_1")); + verify(mSplitsStorage).getNamesByFlagSets(Collections.singleton("set_1")); } @Test @@ -444,7 +444,7 @@ public void getTreatmentsWithConfigByFlagSetsWithNullSetListReturnsEmpty() { @Test public void getTreatmentsWithConfigByFlagSetsRecordsTelemetry() { - when(mSplitsStorage.getNamesByFlagSets(Collections.singletonList("set_1"))).thenReturn(Collections.singleton("test_1")); + when(mSplitsStorage.getNamesByFlagSets(Collections.singleton("set_1"))).thenReturn(Collections.singleton("test_1")); mTreatmentManager.getTreatmentsWithConfigByFlagSets(Arrays.asList("set_1", "set_2"), null, false); diff --git a/src/test/java/io/split/android/client/validators/FlagSetsValidatorImplTest.java b/src/test/java/io/split/android/client/validators/FlagSetsValidatorImplTest.java index 72e3482db..573d74a9e 100644 --- a/src/test/java/io/split/android/client/validators/FlagSetsValidatorImplTest.java +++ b/src/test/java/io/split/android/client/validators/FlagSetsValidatorImplTest.java @@ -15,21 +15,21 @@ public class FlagSetsValidatorImplTest { @Test public void nullInputReturnsEmptyList() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(null); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", null); assertTrue(result.getValues().isEmpty()); assertEquals(0, result.getInvalidValueCount()); } @Test public void emptyInputReturnsEmptyList() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Collections.emptyList()); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Collections.emptyList()); assertTrue(result.getValues().isEmpty()); assertEquals(0, result.getInvalidValueCount()); } @Test public void duplicatedInputValuesAreRemoved() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Arrays.asList("set1", "set1")); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("set1", "set1")); assertEquals(1, result.getValues().size()); assertTrue(result.getValues().contains("set1")); assertEquals(0, result.getInvalidValueCount()); @@ -37,7 +37,7 @@ public void duplicatedInputValuesAreRemoved() { @Test public void valuesAreSortedAlphanumerically() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Arrays.asList("set2", "set1", "set_1", "1set")); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("set2", "set1", "set_1", "1set")); assertEquals(4, result.getValues().size()); assertEquals("1set", result.getValues().get(0)); assertEquals("set1", result.getValues().get(1)); @@ -48,7 +48,7 @@ public void valuesAreSortedAlphanumerically() { @Test public void invalidValuesAreRemoved() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Arrays.asList("set1", "set2", "set_1", "set-1", "set 1", "set 2")); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("set1", "set2", "set_1", "set-1", "set 1", "set 2")); assertEquals(3, result.getValues().size()); assertEquals("set1", result.getValues().get(0)); assertEquals("set2", result.getValues().get(1)); @@ -59,7 +59,7 @@ public void invalidValuesAreRemoved() { @Test public void setWithMoreThan50CharsIsRemoved() { String longSet = "abcdfghijklmnopqrstuvwxyz1234567890abcdfghijklmnopq"; - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Arrays.asList("set1", longSet)); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("set1", longSet)); assertEquals(51, longSet.length()); assertEquals(1, result.getValues().size()); assertEquals("set1", result.getValues().get(0)); @@ -68,7 +68,7 @@ public void setWithMoreThan50CharsIsRemoved() { @Test public void setWithLessThanOneCharIsOrEmptyRemoved() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Arrays.asList("set1", "", " ")); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("set1", "", " ")); assertEquals(1, result.getValues().size()); assertEquals("set1", result.getValues().get(0)); assertEquals(2, result.getInvalidValueCount()); @@ -76,7 +76,7 @@ public void setWithLessThanOneCharIsOrEmptyRemoved() { @Test public void nullSetIsRemoved() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Arrays.asList("set1", null)); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("set1", null)); assertEquals(1, result.getValues().size()); assertEquals("set1", result.getValues().get(0)); assertEquals(1, result.getInvalidValueCount()); @@ -84,7 +84,7 @@ public void nullSetIsRemoved() { @Test public void setWithExtraWhitespaceIsTrimmed() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Arrays.asList("set1 ", " set2\r", "set3 ", "set 4\n")); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("set1 ", " set2\r", "set3 ", "set 4\n")); assertEquals(3, result.getValues().size()); assertEquals("set1", result.getValues().get(0)); assertEquals("set2", result.getValues().get(1)); @@ -94,7 +94,7 @@ public void setWithExtraWhitespaceIsTrimmed() { @Test public void setsAreLowercase() { - SplitFilterValidator.ValidationResult result = mValidator.cleanup(Arrays.asList("SET1", "Set2", "SET_3")); + SplitFilterValidator.ValidationResult result = mValidator.cleanup("method", Arrays.asList("SET1", "Set2", "SET_3")); assertEquals(3, result.getValues().size()); assertEquals("set1", result.getValues().get(0)); assertEquals("set2", result.getValues().get(1)); From ee1d73c57c89afdb106d66402b9d102fb1e763bc Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 15 Sep 2023 12:10:48 -0300 Subject: [PATCH 7/7] Refactor --- .../java/io/split/android/client/SplitFactoryHelper.java | 9 +++++++++ .../java/io/split/android/client/SplitFactoryImpl.java | 7 +------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/split/android/client/SplitFactoryHelper.java b/src/main/java/io/split/android/client/SplitFactoryHelper.java index 6541a05e7..4550db1ab 100644 --- a/src/main/java/io/split/android/client/SplitFactoryHelper.java +++ b/src/main/java/io/split/android/client/SplitFactoryHelper.java @@ -427,6 +427,15 @@ Pair, String> getFilterConfiguration(SyncConf return new Pair<>(groupedFilters, splitsFilterQueryString); } + @Nullable + FlagSetsFilter getFlagSetsFilter(Map filters) { + if (filters.get(SplitFilter.Type.BY_SET) != null) { + return new FlagSetsFilterImpl(filters.get(SplitFilter.Type.BY_SET).getValues()); + } + + return null; + } + private TelemetryStorage getTelemetryStorage(boolean shouldRecordTelemetry, TelemetryStorage telemetryStorage) { if (telemetryStorage != null) { return telemetryStorage; diff --git a/src/main/java/io/split/android/client/SplitFactoryImpl.java b/src/main/java/io/split/android/client/SplitFactoryImpl.java index e798dcdcd..2f67145d8 100644 --- a/src/main/java/io/split/android/client/SplitFactoryImpl.java +++ b/src/main/java/io/split/android/client/SplitFactoryImpl.java @@ -7,10 +7,8 @@ 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; import io.split.android.client.common.CompressionUtilProvider; @@ -180,10 +178,7 @@ public void taskExecuted(@NonNull SplitTaskExecutionInfo taskInfo) { SplitApiFacade splitApiFacade = factoryHelper.buildApiFacade( config, defaultHttpClient, splitsFilterQueryStringFromConfig); - FlagSetsFilter flagSetsFilter = null; - if (filters.get(SplitFilter.Type.BY_SET) != null) { - flagSetsFilter = new FlagSetsFilterImpl(filters.get(SplitFilter.Type.BY_SET).getValues()); - } + FlagSetsFilter flagSetsFilter = factoryHelper.getFlagSetsFilter(filters); SplitTaskFactory splitTaskFactory = new SplitTaskFactoryImpl( config, splitApiFacade, mStorageContainer, splitsFilterQueryStringFromConfig, mEventsManagerCoordinator,