From f78a3557a7579be3f72971f448cff048bf13d32f Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Thu, 10 Dec 2020 18:30:07 -0300 Subject: [PATCH 1/6] renamed provider to task and refactor --- .../java/io/split/client/SplitClientImpl.java | 10 +- .../io/split/client/SplitFactoryImpl.java | 20 +- .../io/split/client/SplitManagerImpl.java | 20 +- .../io/split/client/jmx/SplitJmxMonitor.java | 15 +- .../split/engine/cache/InMemoryCacheImp.java | 20 +- .../io/split/engine/cache/SplitCache.java | 2 +- .../split/engine/common/SyncManagerImp.java | 10 +- .../split/engine/common/SynchronizerImp.java | 28 +- .../split/engine/evaluator/EvaluatorImp.java | 9 +- .../experiments/RefreshableSplitFetcher.java | 52 ---- ....java => RefreshableSplitFetcherTask.java} | 44 +-- .../engine/experiments/SplitFetcher.java | 12 - .../io/split/client/SplitClientImplTest.java | 290 +++++++++--------- .../io/split/client/SplitManagerImplTest.java | 48 +-- .../impressions/ImpressionObserverTest.java | 1 - .../split/engine/cache/InMemoryCacheTest.java | 23 ++ .../split/engine/common/SynchronizerTest.java | 61 ++-- .../split/engine/evaluator/EvaluatorTest.java | 36 +-- .../RefreshableSplitFetcherTest.java | 54 +--- 19 files changed, 333 insertions(+), 422 deletions(-) rename client/src/main/java/io/split/engine/experiments/{RefreshableSplitFetcherProvider.java => RefreshableSplitFetcherTask.java} (69%) diff --git a/client/src/main/java/io/split/client/SplitClientImpl.java b/client/src/main/java/io/split/client/SplitClientImpl.java index 4b61c7d6c..9723eb8ad 100644 --- a/client/src/main/java/io/split/client/SplitClientImpl.java +++ b/client/src/main/java/io/split/client/SplitClientImpl.java @@ -5,10 +5,10 @@ import io.split.client.dtos.Event; import io.split.client.impressions.Impression; import io.split.client.impressions.ImpressionsManager; +import io.split.engine.cache.SplitCache; import io.split.engine.evaluator.Evaluator; import io.split.engine.SDKReadinessGates; import io.split.engine.evaluator.EvaluatorImp; -import io.split.engine.experiments.SplitFetcher; import io.split.engine.metrics.Metrics; import io.split.grammar.Treatments; import org.slf4j.Logger; @@ -36,7 +36,7 @@ public final class SplitClientImpl implements SplitClient { private static final Logger _log = LoggerFactory.getLogger(SplitClientImpl.class); private final SplitFactory _container; - private final SplitFetcher _splitFetcher; + private final SplitCache _splitCache; private final ImpressionsManager _impressionManager; private final Metrics _metrics; private final SplitClientConfig _config; @@ -45,7 +45,7 @@ public final class SplitClientImpl implements SplitClient { private final Evaluator _evaluator; public SplitClientImpl(SplitFactory container, - SplitFetcher splitFetcher, + SplitCache splitCache, ImpressionsManager impressionManager, Metrics metrics, EventClient eventClient, @@ -53,7 +53,7 @@ public SplitClientImpl(SplitFactory container, SDKReadinessGates gates, Evaluator evaluator) { _container = container; - _splitFetcher = checkNotNull(splitFetcher); + _splitCache = checkNotNull(splitCache); _impressionManager = checkNotNull(impressionManager); _metrics = metrics; _eventClient = eventClient; @@ -192,7 +192,7 @@ private boolean track(Event event) { event.trafficTypeName = event.trafficTypeName.toLowerCase(); } - if (!_splitFetcher.trafficTypeExists(event.trafficTypeName)) { + if (!_splitCache.trafficTypeExists(event.trafficTypeName)) { _log.warn("track: Traffic Type " + event.trafficTypeName + " does not have any corresponding Splits in this environment, " + "make sure you’re tracking your events to a valid traffic type defined in the Split console."); } diff --git a/client/src/main/java/io/split/client/SplitFactoryImpl.java b/client/src/main/java/io/split/client/SplitFactoryImpl.java index dafda7bf6..3bfa116c0 100644 --- a/client/src/main/java/io/split/client/SplitFactoryImpl.java +++ b/client/src/main/java/io/split/client/SplitFactoryImpl.java @@ -18,9 +18,9 @@ import io.split.engine.SDKReadinessGates; import io.split.engine.common.SyncManager; import io.split.engine.common.SyncManagerImp; -import io.split.engine.experiments.RefreshableSplitFetcherProvider; +import io.split.engine.experiments.RefreshableSplitFetcher; +import io.split.engine.experiments.RefreshableSplitFetcherTask; import io.split.engine.experiments.SplitChangeFetcher; -import io.split.engine.experiments.SplitFetcher; import io.split.engine.experiments.SplitParser; import io.split.engine.segments.RefreshableSegmentFetcher; import io.split.engine.segments.SegmentChangeFetcher; @@ -203,7 +203,9 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn SplitChangeFetcher splitChangeFetcher = HttpSplitChangeFetcher.create(httpclient, rootTarget, uncachedFireAndForget); final SplitCache cache = new InMemoryCacheImp(); - final RefreshableSplitFetcherProvider splitFetcherProvider = new RefreshableSplitFetcherProvider(splitChangeFetcher, splitParser, findPollingPeriod(RANDOM, config.featuresRefreshRate()), gates, cache); + final SplitCache splitCache = new InMemoryCacheImp(-1); + final RefreshableSplitFetcher splitFetcher = new RefreshableSplitFetcher(splitChangeFetcher, splitParser, gates, splitCache); + final RefreshableSplitFetcherTask splitFetcherTask = new RefreshableSplitFetcherTask(splitFetcher, splitCache, findPollingPeriod(RANDOM, config.featuresRefreshRate())); List impressionListeners = new ArrayList<>(); @@ -227,7 +229,7 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn final EventClient eventClient = EventClientImpl.create(httpclient, eventsRootTarget, config.eventsQueueSize(), config.eventFlushIntervalInMillis(), config.waitBeforeShutdown()); // SyncManager - final SyncManager syncManager = SyncManagerImp.build(config.streamingEnabled(), splitFetcherProvider, segmentFetcher, config.authServiceURL(), httpclient, config.streamingServiceURL(), config.authRetryBackoffBase(), buildSSEdHttpClient(config)); + final SyncManager syncManager = SyncManagerImp.build(config.streamingEnabled(), splitFetcherTask, splitFetcher, segmentFetcher, splitCache, config.authServiceURL(), httpclient, config.streamingServiceURL(), config.authRetryBackoffBase(), buildSSEdHttpClient(config)); syncManager.start(); destroyer = new Runnable() { @@ -236,7 +238,7 @@ public void run() { try { segmentFetcher.close(); _log.info("Successful shutdown of segment fetchers"); - splitFetcherProvider.close(); + splitFetcherTask.close(); _log.info("Successful shutdown of splits"); impressionsManager.close(); _log.info("Successful shutdown of impressions manager"); @@ -266,19 +268,17 @@ public void run() { }); } - - SplitFetcher splitFetcher = splitFetcherProvider.getFetcher(); - Evaluator evaluator = new EvaluatorImp(gates, splitFetcher); + final Evaluator evaluator = new EvaluatorImp(gates, splitCache); _client = new SplitClientImpl(this, - splitFetcher, + splitCache, impressionsManager, cachedFireAndForgetMetrics, eventClient, config, gates, evaluator); - _manager = new SplitManagerImpl(splitFetcherProvider.getFetcher(), config, gates); + _manager = new SplitManagerImpl(splitCache, config, gates); } private static int findPollingPeriod(Random rand, int max) { diff --git a/client/src/main/java/io/split/client/SplitManagerImpl.java b/client/src/main/java/io/split/client/SplitManagerImpl.java index baa5fb462..32128efe8 100644 --- a/client/src/main/java/io/split/client/SplitManagerImpl.java +++ b/client/src/main/java/io/split/client/SplitManagerImpl.java @@ -4,17 +4,13 @@ import io.split.client.api.SplitView; import io.split.client.dtos.Partition; import io.split.engine.SDKReadinessGates; +import io.split.engine.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; -import io.split.engine.experiments.SplitFetcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import java.util.*; import java.util.concurrent.TimeoutException; /** @@ -24,23 +20,23 @@ public class SplitManagerImpl implements SplitManager { private static final Logger _log = LoggerFactory.getLogger(SplitManagerImpl.class); - private final SplitFetcher _splitFetcher; + private final SplitCache _splitCache; private final SplitClientConfig _config; private final SDKReadinessGates _gates; - public SplitManagerImpl(SplitFetcher splitFetcher, + public SplitManagerImpl(SplitCache splitCache, SplitClientConfig config, SDKReadinessGates gates) { _config = Preconditions.checkNotNull(config); - _splitFetcher = Preconditions.checkNotNull(splitFetcher); + _splitCache = Preconditions.checkNotNull(splitCache); _gates = Preconditions.checkNotNull(gates); } @Override public List splits() { List result = new ArrayList<>(); - List parsedSplits = _splitFetcher.fetchAll(); + Collection parsedSplits = _splitCache.getAll(); for (ParsedSplit split : parsedSplits) { result.add(toSplitView(split)); } @@ -57,7 +53,7 @@ public SplitView split(String featureName) { _log.error("split: you passed an empty split name, split name must be a non-empty string"); return null; } - ParsedSplit parsedSplit = _splitFetcher.fetch(featureName); + ParsedSplit parsedSplit = _splitCache.get(featureName); if (parsedSplit == null) { if (_gates.isSDKReadyNow()) { _log.warn("split: you passed \"" + featureName + "\" that does not exist in this environment, " + @@ -71,7 +67,7 @@ public SplitView split(String featureName) { @Override public List splitNames() { List result = new ArrayList<>(); - List parsedSplits = _splitFetcher.fetchAll(); + Collection parsedSplits = _splitCache.getAll(); for (ParsedSplit split : parsedSplits) { result.add(split.feature()); } diff --git a/client/src/main/java/io/split/client/jmx/SplitJmxMonitor.java b/client/src/main/java/io/split/client/jmx/SplitJmxMonitor.java index 5dd1167ed..b5bff41d8 100644 --- a/client/src/main/java/io/split/client/jmx/SplitJmxMonitor.java +++ b/client/src/main/java/io/split/client/jmx/SplitJmxMonitor.java @@ -1,11 +1,14 @@ package io.split.client.jmx; import io.split.client.SplitClient; +import io.split.engine.cache.SplitCache; import io.split.engine.experiments.SplitFetcher; import io.split.engine.segments.SegmentFetcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static com.google.common.base.Preconditions.checkNotNull; + /** * Created by patricioe on 1/18/16. */ @@ -15,12 +18,14 @@ public class SplitJmxMonitor implements SplitJmxMonitorMBean { private final SplitClient _client; private final SplitFetcher _featureFetcher; + private final SplitCache _splitCache; private final SegmentFetcher _segmentFetcher; - public SplitJmxMonitor(SplitClient splitClient, SplitFetcher fetcher, SegmentFetcher segmentFetcher) { - _client = splitClient; - _featureFetcher = fetcher; - _segmentFetcher = segmentFetcher; + public SplitJmxMonitor(SplitClient splitClient, SplitFetcher featureFetcher, SplitCache splitCache, SegmentFetcher segmentFetcher) { + _client = checkNotNull(splitClient); + _featureFetcher = checkNotNull(featureFetcher); + _splitCache = checkNotNull(splitCache); + _segmentFetcher = checkNotNull(segmentFetcher); } @Override @@ -44,7 +49,7 @@ public String getTreatment(String key, String featureName) { @Override public String fetchDefinition(String featureName) { - return _featureFetcher.fetch(featureName).toString(); + return _splitCache.get(featureName).toString(); } @Override diff --git a/client/src/main/java/io/split/engine/cache/InMemoryCacheImp.java b/client/src/main/java/io/split/engine/cache/InMemoryCacheImp.java index af4e5e4d1..003a3d1bb 100644 --- a/client/src/main/java/io/split/engine/cache/InMemoryCacheImp.java +++ b/client/src/main/java/io/split/engine/cache/InMemoryCacheImp.java @@ -11,7 +11,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Set; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicLong; @@ -100,6 +99,25 @@ public boolean trafficTypeExists(String trafficTypeName) { return Sets.newHashSet(_concurrentTrafficTypeNameSet.elementSet()).contains(trafficTypeName); } + @Override + public void kill(String splitName, String defaultTreatment, long changeNumber) { + ParsedSplit parsedSplit = _concurrentMap.get(splitName); + + ParsedSplit updatedSplit = new ParsedSplit(parsedSplit.feature(), + parsedSplit.seed(), + true, + defaultTreatment, + parsedSplit.parsedConditions(), + parsedSplit.trafficTypeName(), + changeNumber, + parsedSplit.trafficAllocation(), + parsedSplit.trafficAllocationSeed(), + parsedSplit.algo(), + parsedSplit.configurations()); + + _concurrentMap.put(splitName, updatedSplit); + } + @Override public void clear() { _concurrentMap.clear(); diff --git a/client/src/main/java/io/split/engine/cache/SplitCache.java b/client/src/main/java/io/split/engine/cache/SplitCache.java index 3ab9916bd..f79f27878 100644 --- a/client/src/main/java/io/split/engine/cache/SplitCache.java +++ b/client/src/main/java/io/split/engine/cache/SplitCache.java @@ -4,7 +4,6 @@ import java.util.Collection; import java.util.List; -import java.util.Set; public interface SplitCache { void put(ParsedSplit split); @@ -15,5 +14,6 @@ public interface SplitCache { long getChangeNumber(); void setChangeNumber(long changeNumber); boolean trafficTypeExists(String trafficTypeName); + void kill(String splitName, String defaultTreatment, long changeNumber); void clear(); } diff --git a/client/src/main/java/io/split/engine/common/SyncManagerImp.java b/client/src/main/java/io/split/engine/common/SyncManagerImp.java index 05bf44c46..1576cce80 100644 --- a/client/src/main/java/io/split/engine/common/SyncManagerImp.java +++ b/client/src/main/java/io/split/engine/common/SyncManagerImp.java @@ -2,7 +2,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; -import io.split.engine.experiments.RefreshableSplitFetcherProvider; +import io.split.engine.cache.SplitCache; +import io.split.engine.experiments.RefreshableSplitFetcher; +import io.split.engine.experiments.RefreshableSplitFetcherTask; import io.split.engine.segments.RefreshableSegmentFetcher; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.slf4j.Logger; @@ -44,15 +46,17 @@ public class SyncManagerImp implements SyncManager { } public static SyncManagerImp build(boolean streamingEnabledConfig, - RefreshableSplitFetcherProvider refreshableSplitFetcherProvider, + RefreshableSplitFetcherTask refreshableSplitFetcherTask, + RefreshableSplitFetcher splitFetcher, RefreshableSegmentFetcher segmentFetcher, + SplitCache splitCache, String authUrl, CloseableHttpClient httpClient, String streamingServiceUrl, int authRetryBackOffBase, CloseableHttpClient sseHttpClient) { LinkedBlockingQueue pushMessages = new LinkedBlockingQueue<>(); - Synchronizer synchronizer = new SynchronizerImp(refreshableSplitFetcherProvider, segmentFetcher); + Synchronizer synchronizer = new SynchronizerImp(refreshableSplitFetcherTask, splitFetcher, segmentFetcher, splitCache); PushManager pushManager = PushManagerImp.build(synchronizer, streamingServiceUrl, authUrl, httpClient, authRetryBackOffBase, pushMessages, sseHttpClient); return new SyncManagerImp(streamingEnabledConfig, synchronizer, pushManager, pushMessages); } diff --git a/client/src/main/java/io/split/engine/common/SynchronizerImp.java b/client/src/main/java/io/split/engine/common/SynchronizerImp.java index 9d370391c..fedcbc4f2 100644 --- a/client/src/main/java/io/split/engine/common/SynchronizerImp.java +++ b/client/src/main/java/io/split/engine/common/SynchronizerImp.java @@ -1,8 +1,10 @@ package io.split.engine.common; import com.google.common.util.concurrent.ThreadFactoryBuilder; + +import io.split.engine.cache.SplitCache; import io.split.engine.experiments.RefreshableSplitFetcher; -import io.split.engine.experiments.RefreshableSplitFetcherProvider; +import io.split.engine.experiments.RefreshableSplitFetcherTask; import io.split.engine.segments.RefreshableSegmentFetcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -17,16 +19,20 @@ public class SynchronizerImp implements Synchronizer { private static final Logger _log = LoggerFactory.getLogger(Synchronizer.class); - private final RefreshableSplitFetcherProvider _refreshableSplitFetcherProvider; + private final RefreshableSplitFetcherTask _refreshableSplitFetcherTask; private final RefreshableSplitFetcher _splitFetcher; private final RefreshableSegmentFetcher _segmentFetcher; private final ScheduledExecutorService _syncAllScheduledExecutorService; + private final SplitCache _splitCache; - public SynchronizerImp(RefreshableSplitFetcherProvider refreshableSplitFetcherProvider, - RefreshableSegmentFetcher segmentFetcher) { - _refreshableSplitFetcherProvider = checkNotNull(refreshableSplitFetcherProvider); - _splitFetcher = checkNotNull(_refreshableSplitFetcherProvider.getFetcher()); + public SynchronizerImp(RefreshableSplitFetcherTask refreshableSplitTask, + RefreshableSplitFetcher splitFetcher, + RefreshableSegmentFetcher segmentFetcher, + SplitCache splitCache) { + _refreshableSplitFetcherTask = checkNotNull(refreshableSplitTask); + _splitFetcher = checkNotNull(splitFetcher); _segmentFetcher = checkNotNull(segmentFetcher); + _splitCache = checkNotNull(splitCache); ThreadFactory splitsThreadFactory = new ThreadFactoryBuilder() .setDaemon(true) @@ -46,28 +52,28 @@ public void syncAll() { @Override public void startPeriodicFetching() { _log.debug("Starting Periodic Fetching ..."); - _refreshableSplitFetcherProvider.startPeriodicFetching(); + _refreshableSplitFetcherTask.startPeriodicFetching(); _segmentFetcher.startPeriodicFetching(); } @Override public void stopPeriodicFetching() { _log.debug("Stop Periodic Fetching ..."); - _refreshableSplitFetcherProvider.stop(); + _refreshableSplitFetcherTask.stop(); _segmentFetcher.stop(); } @Override public void refreshSplits(long targetChangeNumber) { - if (targetChangeNumber > _splitFetcher.changeNumber()) { + if (targetChangeNumber > _splitCache.getChangeNumber()) { _splitFetcher.forceRefresh(); } } @Override public void localKillSplit(String splitName, String defaultTreatment, long newChangeNumber) { - if (newChangeNumber > _splitFetcher.changeNumber()) { - _splitFetcher.killSplit(splitName, defaultTreatment, newChangeNumber); + if (newChangeNumber > _splitCache.getChangeNumber()) { + _splitCache.kill(splitName, defaultTreatment, newChangeNumber); refreshSplits(newChangeNumber); } } diff --git a/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java b/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java index 667be4163..9da48d60f 100644 --- a/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java +++ b/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java @@ -3,6 +3,7 @@ import io.split.client.dtos.ConditionType; import io.split.client.exceptions.ChangeNumberExceptionWrapper; import io.split.engine.SDKReadinessGates; +import io.split.engine.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; import io.split.engine.experiments.SplitFetcher; @@ -25,18 +26,18 @@ public class EvaluatorImp implements Evaluator { private static final Logger _log = LoggerFactory.getLogger(EvaluatorImp.class); private final SDKReadinessGates _gates; - private final SplitFetcher _splitFetcher; + private final SplitCache _splitCache; public EvaluatorImp(SDKReadinessGates gates, - SplitFetcher splitFetcher) { + SplitCache splitCache) { _gates = checkNotNull(gates); - _splitFetcher = checkNotNull(splitFetcher); + _splitCache = checkNotNull(splitCache); } @Override public TreatmentLabelAndChangeNumber evaluateFeature(String matchingKey, String bucketingKey, String split, Map attributes) { try { - ParsedSplit parsedSplit = _splitFetcher.fetch(split); + ParsedSplit parsedSplit = _splitCache.get(split); if (parsedSplit == null) { if (_gates.isSDKReadyNow()) { diff --git a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcher.java b/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcher.java index 982c7184a..df696ec47 100644 --- a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcher.java +++ b/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcher.java @@ -1,6 +1,5 @@ package io.split.engine.experiments; -import com.google.common.collect.Lists; import io.split.client.dtos.Split; import io.split.client.dtos.SplitChange; import io.split.client.dtos.Status; @@ -9,9 +8,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.Collection; -import java.util.List; - import static com.google.common.base.Preconditions.checkNotNull; /** @@ -67,54 +63,6 @@ public void forceRefresh() { } } - @Override - public long changeNumber() { - return _splitCache.getChangeNumber(); - } - - @Override - public void killSplit(String splitName, String defaultTreatment, long changeNumber) { - synchronized (_lock) { - ParsedSplit parsedSplit = _splitCache.get(splitName); - - ParsedSplit updatedSplit = new ParsedSplit(parsedSplit.feature(), - parsedSplit.seed(), - true, - defaultTreatment, - parsedSplit.parsedConditions(), - parsedSplit.trafficTypeName(), - changeNumber, - parsedSplit.trafficAllocation(), - parsedSplit.trafficAllocationSeed(), - parsedSplit.algo(), - parsedSplit.configurations()); - - _splitCache.put(updatedSplit); - } - } - - @Override - public ParsedSplit fetch(String test) { - return _splitCache.get(test); - } - - public List fetchAll() { - return Lists.newArrayList(_splitCache.getAll()); - } - - @Override - public boolean trafficTypeExists(String trafficTypeName) { - return _splitCache.trafficTypeExists(trafficTypeName); - } - - public Collection fetch() { - return _splitCache.getAll(); - } - - public void clear() { - _splitCache.clear(); - } - @Override public void run() { _log.debug("Fetch splits starting ..."); diff --git a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherProvider.java b/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java similarity index 69% rename from client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherProvider.java rename to client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java index 5113a9a85..9a9f5cdbf 100644 --- a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherProvider.java +++ b/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java @@ -1,7 +1,6 @@ package io.split.engine.experiments; import com.google.common.util.concurrent.ThreadFactoryBuilder; -import io.split.engine.SDKReadinessGates; import io.split.engine.cache.SplitCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -25,29 +24,23 @@ * * @author adil */ -public class RefreshableSplitFetcherProvider implements Closeable { - private static final Logger _log = LoggerFactory.getLogger(RefreshableSplitFetcherProvider.class); +public class RefreshableSplitFetcherTask implements Closeable { + private static final Logger _log = LoggerFactory.getLogger(RefreshableSplitFetcherTask.class); private final AtomicReference _splitFetcher = new AtomicReference(); + private final AtomicReference _splitCache = new AtomicReference(); private final AtomicReference _executorService = new AtomicReference<>(); - private final SplitParser _splitParser; - private final SplitChangeFetcher _splitChangeFetcher; private final AtomicLong _refreshEveryNSeconds; - private final SDKReadinessGates _gates; private final ScheduledExecutorService _scheduledExecutorService; - private final Object _lock = new Object(); private final AtomicBoolean _running; - private final SplitCache _splitCache; private ScheduledFuture _scheduledFuture; - public RefreshableSplitFetcherProvider(SplitChangeFetcher splitChangeFetcher, SplitParser splitParser, long refreshEveryNSeconds, SDKReadinessGates sdkBuildBlocker, SplitCache splitCache) { - _splitChangeFetcher = checkNotNull(splitChangeFetcher); - _splitParser = checkNotNull(splitParser); + public RefreshableSplitFetcherTask(RefreshableSplitFetcher splitFetcher, SplitCache splitCache, long refreshEveryNSeconds) { + _splitFetcher.set(checkNotNull(splitFetcher)); + _splitCache.set(checkNotNull(splitCache)); checkArgument(refreshEveryNSeconds >= 0L); _refreshEveryNSeconds = new AtomicLong(refreshEveryNSeconds); - _gates = checkNotNull(sdkBuildBlocker); - _splitCache = checkNotNull(splitCache); ThreadFactory threadFactory = new ThreadFactoryBuilder() .setDaemon(true) @@ -60,25 +53,6 @@ public RefreshableSplitFetcherProvider(SplitChangeFetcher splitChangeFetcher, Sp _running = new AtomicBoolean(); } - public RefreshableSplitFetcher getFetcher() { - if (_splitFetcher.get() != null) { - return _splitFetcher.get(); - } - - // we are locking here since we wanna make sure that we create only ONE RefreshableExperimentChangeFetcher - synchronized (_lock) { - // double check - if (_splitFetcher.get() != null) { - return _splitFetcher.get(); - } - - RefreshableSplitFetcher splitFetcher = new RefreshableSplitFetcher(_splitChangeFetcher, _splitParser, _gates, _splitCache); - - _splitFetcher.set(splitFetcher); - return splitFetcher; - } - } - public void startPeriodicFetching() { if (_running.getAndSet(true)) { _log.warn("Splits PeriodicFetching is running..."); @@ -86,7 +60,7 @@ public void startPeriodicFetching() { } _log.debug("Starting PeriodicFetching Splits ..."); - _scheduledFuture = _scheduledExecutorService.scheduleWithFixedDelay(getFetcher(), 0L, _refreshEveryNSeconds.get(), TimeUnit.SECONDS); + _scheduledFuture = _scheduledExecutorService.scheduleWithFixedDelay(_splitFetcher.get(), 0L, _refreshEveryNSeconds.get(), TimeUnit.SECONDS); } public void stop() { @@ -106,9 +80,11 @@ public void close() { } if (_splitFetcher.get() != null) { - _splitFetcher.get().clear(); + _splitCache.get().clear(); } + _running.set(false); + ScheduledExecutorService scheduledExecutorService = _executorService.get(); if (scheduledExecutorService.isShutdown()) { return; diff --git a/client/src/main/java/io/split/engine/experiments/SplitFetcher.java b/client/src/main/java/io/split/engine/experiments/SplitFetcher.java index 203fbb588..f47a0da0d 100644 --- a/client/src/main/java/io/split/engine/experiments/SplitFetcher.java +++ b/client/src/main/java/io/split/engine/experiments/SplitFetcher.java @@ -1,24 +1,12 @@ package io.split.engine.experiments; -import java.util.List; - /** * Created by adilaijaz on 5/8/15. */ public interface SplitFetcher { - ParsedSplit fetch(String splitName); - - List fetchAll(); - - boolean trafficTypeExists(String trafficTypeName); - /** * Forces a sync of splits, outside of any scheduled * syncs. This method MUST NOT throw any exceptions. */ void forceRefresh(); - - long changeNumber(); - - void killSplit(String splitName, String defaultTreatment, long changeNumber); } diff --git a/client/src/test/java/io/split/client/SplitClientImplTest.java b/client/src/test/java/io/split/client/SplitClientImplTest.java index 255032b63..0f6cfe68a 100644 --- a/client/src/test/java/io/split/client/SplitClientImplTest.java +++ b/client/src/test/java/io/split/client/SplitClientImplTest.java @@ -10,6 +10,8 @@ import io.split.client.dtos.Partition; import io.split.client.impressions.Impression; import io.split.client.impressions.ImpressionsManager; +import io.split.engine.cache.InMemoryCacheImp; +import io.split.engine.cache.SplitCache; import io.split.engine.evaluator.Evaluator; import io.split.engine.evaluator.EvaluatorImp; import io.split.engine.SDKReadinessGates; @@ -70,23 +72,23 @@ public void null_key_results_in_control() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment(null, "test1"), is(equalTo(Treatments.CONTROL))); - verifyZeroInteractions(splitFetcher); + verifyZeroInteractions(splitCache); } @Test @@ -97,44 +99,44 @@ public void null_test_results_in_control() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("adil@relateiq.com", null), is(equalTo(Treatments.CONTROL))); - verifyZeroInteractions(splitFetcher); + verifyZeroInteractions(splitCache); } @Test public void exceptions_result_in_control() { SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(anyString())).thenThrow(RuntimeException.class); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(anyString())).thenThrow(RuntimeException.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("adil@relateiq.com", "test1"), is(equalTo(Treatments.CONTROL))); - verify(splitFetcher).fetch("test1"); + verify(splitCache).get("test1"); } @Test @@ -146,18 +148,18 @@ public void works() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); int numKeys = 5; @@ -166,7 +168,7 @@ public void works() { assertThat(client.getTreatment(randomKey, test), is(equalTo("on"))); } - verify(splitFetcher, times(numKeys)).fetch(test); + verify(splitCache, times(numKeys)).get(test); } /** @@ -181,18 +183,18 @@ public void works_null_config() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); @@ -201,7 +203,7 @@ public void works_null_config() { assertThat(result.treatment(), is(equalTo(Treatments.ON))); assertThat(result.config(), is(nullValue())); - verify(splitFetcher).fetch(test); + verify(splitCache).get(test); } @Test @@ -218,18 +220,18 @@ public void worksAndHasConfig() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1, configurations); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); int numKeys = 5; @@ -240,10 +242,9 @@ public void worksAndHasConfig() { } // Times 2 because we are calling getTreatment twice. Once for getTreatment and one for getTreatmentWithConfig - verify(splitFetcher, times(numKeys * 2)).fetch(test); + verify(splitCache, times(numKeys * 2)).get(test); } - @Test public void last_condition_is_always_default() { String test = "test1"; @@ -253,23 +254,23 @@ public void last_condition_is_always_default() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, "user", 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("pato@codigo.com", test), is(equalTo(Treatments.OFF))); - verify(splitFetcher).fetch(test); + verify(splitCache).get(test); } /** @@ -289,25 +290,25 @@ public void last_condition_is_always_default_but_with_treatment() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, "user", 1, 1, configurations); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); SplitResult result = client.getTreatmentWithConfig("pato@codigo.com", test); assertThat(result.treatment(), is(equalTo(Treatments.OFF))); assertThat(result.config(), is(equalTo("{\"size\" : 30}"))); - verify(splitFetcher).fetch(test); + verify(splitCache).get(test); } @Test @@ -322,25 +323,25 @@ public void multiple_conditions_work() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("on"))); assertThat(client.getTreatment("pato@codigo.com", test), is(equalTo("off"))); assertThat(client.getTreatment("trevor@codigo.com", test), is(equalTo("on"))); - verify(splitFetcher, times(3)).fetch(test); + verify(splitCache, times(3)).get(test); } @@ -353,23 +354,23 @@ public void killed_test_always_goes_to_default() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, true, Treatments.OFF, conditions, "user", 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo(Treatments.OFF))); - verify(splitFetcher).fetch(test); + verify(splitCache).get(test); } /** @@ -389,25 +390,25 @@ public void killed_test_always_goes_to_default_has_config() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, true, Treatments.OFF, conditions, "user", 1, 1, configurations); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); SplitResult result = client.getTreatmentWithConfig("adil@codigo.com", test); assertThat(result.treatment(), is(equalTo(Treatments.OFF))); assertThat(result.config(), is(equalTo("{\"size\" : 30}"))); - verify(splitFetcher).fetch(test); + verify(splitCache).get(test); } @Test @@ -424,19 +425,19 @@ public void dependency_matcher_on() { ParsedSplit dependentSplit = ParsedSplit.createParsedSplitForTests(dependent, 123, false, Treatments.OFF, dependent_conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(parent)).thenReturn(parentSplit); - when(splitFetcher.fetch(dependent)).thenReturn(dependentSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(parent)).thenReturn(parentSplit); + when(splitCache.get(dependent)).thenReturn(dependentSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("key", parent), is(equalTo(Treatments.ON))); @@ -457,19 +458,19 @@ public void dependency_matcher_off() { ParsedSplit dependentSplit = ParsedSplit.createParsedSplitForTests(dependent, 123, false, Treatments.OFF, dependent_conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(parent)).thenReturn(parentSplit); - when(splitFetcher.fetch(dependent)).thenReturn(dependentSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(parent)).thenReturn(parentSplit); + when(splitCache.get(dependent)).thenReturn(dependentSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("key", parent), is(equalTo(Treatments.ON))); @@ -485,23 +486,21 @@ public void dependency_matcher_control() { ParsedSplit dependentSplit = ParsedSplit.createParsedSplitForTests(dependent, 123, false, Treatments.ON, dependent_conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(dependent)).thenReturn(dependentSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(dependent)).thenReturn(dependentSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); -// assertThat(client.getTreatment("key", dependent), is(equalTo(Treatments.CONTROL))); assertThat(client.getTreatment("key", dependent), is(equalTo(Treatments.ON))); - } @Test @@ -515,18 +514,18 @@ public void attributes_work() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("on"))); @@ -536,7 +535,7 @@ public void attributes_work() { assertThat(client.getTreatment("pato@codigo.com", test, ImmutableMap.of("age", 10)), is(equalTo("on"))); assertThat(client.getTreatment("pato@codigo.com", test, ImmutableMap.of("age", 9)), is(equalTo("off"))); - verify(splitFetcher, times(5)).fetch(test); + verify(splitCache, times(5)).get(test); } @Test @@ -549,18 +548,18 @@ public void attributes_work_2() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, "user", 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off"))); @@ -570,7 +569,7 @@ public void attributes_work_2() { assertThat(client.getTreatment("pato@codigo.com", test, ImmutableMap.of("age", 10)), is(equalTo("off"))); assertThat(client.getTreatment("pato@codigo.com", test, ImmutableMap.of("age", 0)), is(equalTo("on"))); - verify(splitFetcher, times(5)).fetch(test); + verify(splitCache, times(5)).get(test); } @Test @@ -583,18 +582,18 @@ public void attributes_greater_than_negative_number() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off"))); @@ -606,7 +605,7 @@ public void attributes_greater_than_negative_number() { assertThat(client.getTreatment("pato@codigo.com", test, ImmutableMap.of("age", 20)), is(equalTo("off"))); assertThat(client.getTreatment("pato@codigo.com", test, ImmutableMap.of("age", -21)), is(equalTo("off"))); - verify(splitFetcher, times(7)).fetch(test); + verify(splitCache, times(7)).get(test); } @@ -620,18 +619,18 @@ public void attributes_for_sets() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off"))); @@ -645,7 +644,7 @@ public void attributes_for_sets() { assertThat(client.getTreatment("pato@codigo.com", test, ImmutableMap.of("products", Lists.newArrayList("sms", "video"))), is(equalTo("on"))); assertThat(client.getTreatment("pato@codigo.com", test, ImmutableMap.of("products", Lists.newArrayList("video"))), is(equalTo("on"))); - verify(splitFetcher, times(9)).fetch(test); + verify(splitCache, times(9)).get(test); } @Test @@ -661,20 +660,20 @@ public void labels_are_populated() { List conditions = Lists.newArrayList(age_equal_to_0_should_be_on); ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SDKReadinessGates gates = mock(SDKReadinessGates.class); ImpressionsManager impressionsManager = mock(ImpressionsManager.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, impressionsManager, new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); Map attributes = ImmutableMap.of("age", -20, "acv", "1000000"); @@ -754,19 +753,19 @@ private void traffic_allocation(String key, int trafficAllocation, int trafficAl ParsedSplit parsedSplit = new ParsedSplit(test, 123, false, Treatments.OFF, conditions, null, 1, trafficAllocation, trafficAllocationSeed, 1, null); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); ImpressionsManager impressionsManager = mock(ImpressionsManager.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, impressionsManager, new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment(key, test), is(equalTo(expected_treatment_on_or_off))); @@ -800,21 +799,21 @@ public void notInTrafficAllocationDefaultConfig() { ParsedSplit parsedSplit = new ParsedSplit(test, 123, false, Treatments.OFF, conditions, null, 1, trafficAllocation, trafficAllocationSeed, 1, configurations); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); ImpressionsManager impressionsManager = mock(ImpressionsManager.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, impressionsManager, new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); assertThat(client.getTreatment("pato@split.io", test), is(equalTo(Treatments.OFF))); @@ -843,18 +842,18 @@ public void matching_bucketing_keys_work() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, "user", 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); Key bad_key = new Key("adil", "aijaz"); @@ -863,7 +862,7 @@ public void matching_bucketing_keys_work() { assertThat(client.getTreatment(bad_key, test, Collections.emptyMap()), is(equalTo("off"))); assertThat(client.getTreatment(good_key, test, Collections.emptyMap()), is(equalTo("on"))); - verify(splitFetcher, times(2)).fetch(test); + verify(splitCache, times(2)).get(test); } @Test @@ -880,19 +879,19 @@ public void impression_metadata_is_propagated() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); ImpressionsManager impressionsManager = mock(ImpressionsManager.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, impressionsManager, new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); Map attributes = ImmutableMap.of("age", -20, "acv", "1000000"); @@ -916,18 +915,19 @@ private Partition partition(String treatment, int size) { @Test public void block_until_ready_does_not_time_when_sdk_is_ready() throws TimeoutException, InterruptedException { - SplitFetcher splitFetcher = mock(SplitFetcher.class); + SplitCache splitCache = mock(InMemoryCacheImp.class); SDKReadinessGates ready = mock(SDKReadinessGates.class); when(ready.isSDKReady(100)).thenReturn(true); + SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, mock(ImpressionsManager.class), new Metrics.NoopMetrics(), NoopEventClient.create(), config, ready, - new EvaluatorImp(ready, splitFetcher) + new EvaluatorImp(ready, splitCache) ); client.blockUntilReady(); @@ -935,18 +935,19 @@ public void block_until_ready_does_not_time_when_sdk_is_ready() throws TimeoutEx @Test(expected = TimeoutException.class) public void block_until_ready_times_when_sdk_is_not_ready() throws TimeoutException, InterruptedException { - SplitFetcher splitFetcher = mock(SplitFetcher.class); + SplitCache splitCache = mock(InMemoryCacheImp.class); SDKReadinessGates ready = mock(SDKReadinessGates.class); when(ready.isSDKReady(100)).thenReturn(false); + SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, mock(ImpressionsManager.class), new Metrics.NoopMetrics(), NoopEventClient.create(), config, ready, - new EvaluatorImp(ready, splitFetcher) + new EvaluatorImp(ready, splitCache) ); client.blockUntilReady(); @@ -955,17 +956,17 @@ public void block_until_ready_times_when_sdk_is_not_ready() throws TimeoutExcept @Test public void track_with_valid_parameters() { SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); + SplitCache splitCache = mock(InMemoryCacheImp.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); Assert.assertThat(client.track("validKey", "valid_traffic_type", "valid_event"), @@ -981,17 +982,17 @@ public void track_with_valid_parameters() { @Test public void track_with_invalid_event_type_ids() { SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); + SplitCache splitCache = mock(InMemoryCacheImp.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); Assert.assertThat(client.track("validKey", "valid_traffic_type", ""), @@ -1012,17 +1013,17 @@ public void track_with_invalid_event_type_ids() { @Test public void track_with_invalid_traffic_type_names() { SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); + SplitCache splitCache = mock(InMemoryCacheImp.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); Assert.assertThat(client.track("validKey", "", "valid"), @@ -1035,17 +1036,17 @@ public void track_with_invalid_traffic_type_names() { @Test public void track_with_invalid_keys() { SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); + SplitCache splitCache = mock(InMemoryCacheImp.class); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); Assert.assertThat(client.track("", "valid_traffic_type", "valid"), @@ -1062,19 +1063,19 @@ public void track_with_invalid_keys() { @Test public void track_with_properties() { SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); + SplitCache splitCache = mock(InMemoryCacheImp.class); EventClient eventClientMock = Mockito.mock(EventClient.class); Mockito.when(eventClientMock.track((Event) Mockito.any(), Mockito.anyInt())).thenReturn(true); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), eventClientMock, config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); HashMap properties = new HashMap<>(); @@ -1134,8 +1135,6 @@ public void track_with_properties() { Assert.assertThat(captured.properties.size(), org.hamcrest.Matchers.is(1)); Assert.assertThat((Integer) captured.properties.get("ok_property"), org.hamcrest.Matchers.is(123)); - - properties.clear(); Mockito.reset(eventClientMock); Mockito.when(eventClientMock.track((Event) Mockito.any(), Mockito.anyInt())).thenReturn(true); @@ -1167,7 +1166,6 @@ public void track_with_properties() { Assert.assertThat(client.track("key1", "user", "purchase", properties), org.hamcrest.Matchers.is(false)); } - @Test public void getTreatment_with_invalid_keys() { String test = "split"; @@ -1177,22 +1175,20 @@ public void getTreatment_with_invalid_keys() { ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); - + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitClientImpl client = new SplitClientImpl( mock(SplitFactory.class), - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); - Assert.assertThat(client.getTreatment("valid", "split"), org.hamcrest.Matchers.is(org.hamcrest.Matchers.not(Treatments.CONTROL))); @@ -1248,8 +1244,8 @@ public void client_cannot_perform_actions_when_destroyed() throws InterruptedExc ParsedSplit parsedSplit = ParsedSplit.createParsedSplitForTests(test, 123, false, Treatments.OFF, conditions, null, 1, 1); SDKReadinessGates gates = mock(SDKReadinessGates.class); - SplitFetcher splitFetcher = mock(SplitFetcher.class); - when(splitFetcher.fetch(test)).thenReturn(parsedSplit); + SplitCache splitCache = mock(InMemoryCacheImp.class); + when(splitCache.get(test)).thenReturn(parsedSplit); SplitFactory mockFactory = new SplitFactory() { private boolean destroyed = false; @@ -1269,13 +1265,13 @@ public void client_cannot_perform_actions_when_destroyed() throws InterruptedExc SplitClientImpl client = new SplitClientImpl( mockFactory, - splitFetcher, + splitCache, new ImpressionsManager.NoOpImpressionsManager(), new Metrics.NoopMetrics(), NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitFetcher) + new EvaluatorImp(gates, splitCache) ); Assert.assertThat(client.getTreatment("valid", "split"), diff --git a/client/src/test/java/io/split/client/SplitManagerImplTest.java b/client/src/test/java/io/split/client/SplitManagerImplTest.java index f20fdca4b..0ff31ebb7 100644 --- a/client/src/test/java/io/split/client/SplitManagerImplTest.java +++ b/client/src/test/java/io/split/client/SplitManagerImplTest.java @@ -2,8 +2,10 @@ import com.google.common.collect.Lists; import io.split.client.api.SplitView; +import io.split.client.dtos.Split; import io.split.engine.ConditionsTestUtil; import io.split.engine.SDKReadinessGates; +import io.split.engine.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; import io.split.engine.experiments.SplitFetcher; @@ -33,10 +35,10 @@ public class SplitManagerImplTest { @Test public void splitCallWithNonExistentSplit() { String nonExistent = "nonExistent"; - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); - Mockito.when(splitFetcher.fetch(nonExistent)).thenReturn(null); + SplitCache splitCache = Mockito.mock(SplitCache.class); + Mockito.when(splitCache.get(nonExistent)).thenReturn(null); - SplitManagerImpl splitManager = new SplitManagerImpl(splitFetcher, + SplitManagerImpl splitManager = new SplitManagerImpl(splitCache, Mockito.mock(SplitClientConfig.class), Mockito.mock(SDKReadinessGates.class)); assertThat(splitManager.split("nonExistent"), is(nullValue())); @@ -45,12 +47,12 @@ public void splitCallWithNonExistentSplit() { @Test public void splitCallWithExistentSplit() { String existent = "existent"; - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); + SplitCache splitCache = Mockito.mock(SplitCache.class); ParsedSplit response = ParsedSplit.createParsedSplitForTests("FeatureName", 123, true, "off", Lists.newArrayList(getTestCondition("off")), "traffic", 456L, 1); - Mockito.when(splitFetcher.fetch(existent)).thenReturn(response); + Mockito.when(splitCache.get(existent)).thenReturn(response); - SplitManagerImpl splitManager = new SplitManagerImpl(splitFetcher, + SplitManagerImpl splitManager = new SplitManagerImpl(splitCache, Mockito.mock(SplitClientConfig.class), Mockito.mock(SDKReadinessGates.class)); SplitView theOne = splitManager.split(existent); @@ -66,16 +68,16 @@ public void splitCallWithExistentSplit() { @Test public void splitCallWithExistentSplitAndConfigs() { String existent = "existent"; - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); + SplitCache splitCache = Mockito.mock(SplitCache.class); // Add config for only one treatment(default) Map configurations = new HashMap<>(); configurations.put(Treatments.OFF, "{\"size\" : 30}"); ParsedSplit response = ParsedSplit.createParsedSplitForTests("FeatureName", 123, true, "off", Lists.newArrayList(getTestCondition("off")), "traffic", 456L, 1, configurations); - Mockito.when(splitFetcher.fetch(existent)).thenReturn(response); + Mockito.when(splitCache.get(existent)).thenReturn(response); - SplitManagerImpl splitManager = new SplitManagerImpl(splitFetcher, + SplitManagerImpl splitManager = new SplitManagerImpl(splitCache, Mockito.mock(SplitClientConfig.class), Mockito.mock(SDKReadinessGates.class)); SplitView theOne = splitManager.split(existent); @@ -90,9 +92,9 @@ public void splitCallWithExistentSplitAndConfigs() { @Test public void splitsCallWithNoSplit() { - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); - Mockito.when(splitFetcher.fetchAll()).thenReturn(Lists.newArrayList()); - SplitManagerImpl splitManager = new SplitManagerImpl(splitFetcher, + SplitCache splitCache = Mockito.mock(SplitCache.class); + Mockito.when(splitCache.getAll()).thenReturn(Lists.newArrayList()); + SplitManagerImpl splitManager = new SplitManagerImpl(splitCache, Mockito.mock(SplitClientConfig.class), Mockito.mock(SDKReadinessGates.class)); assertThat(splitManager.splits(), is(empty())); @@ -100,13 +102,13 @@ public void splitsCallWithNoSplit() { @Test public void splitsCallWithSplit() { - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); + SplitCache splitCache = Mockito.mock(SplitCache.class); List parsedSplits = Lists.newArrayList(); ParsedSplit response = ParsedSplit.createParsedSplitForTests("FeatureName", 123, true, "off", Lists.newArrayList(getTestCondition("off")), "traffic", 456L, 1); parsedSplits.add(response); - Mockito.when(splitFetcher.fetchAll()).thenReturn(parsedSplits); - SplitManagerImpl splitManager = new SplitManagerImpl(splitFetcher, + Mockito.when(splitCache.getAll()).thenReturn(parsedSplits); + SplitManagerImpl splitManager = new SplitManagerImpl(splitCache, Mockito.mock(SplitClientConfig.class), Mockito.mock(SDKReadinessGates.class)); List splits = splitManager.splits(); @@ -121,9 +123,9 @@ public void splitsCallWithSplit() { @Test public void splitNamesCallWithNoSplit() { - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); - Mockito.when(splitFetcher.fetchAll()).thenReturn(Lists.newArrayList()); - SplitManagerImpl splitManager = new SplitManagerImpl(splitFetcher, + SplitCache splitCache = Mockito.mock(SplitCache.class); + Mockito.when(splitCache.getAll()).thenReturn(Lists.newArrayList()); + SplitManagerImpl splitManager = new SplitManagerImpl(splitCache, Mockito.mock(SplitClientConfig.class), Mockito.mock(SDKReadinessGates.class)); assertThat(splitManager.splitNames(), is(empty())); @@ -131,13 +133,13 @@ public void splitNamesCallWithNoSplit() { @Test public void splitNamesCallWithSplit() { - SplitFetcher splitFetcher = Mockito.mock(SplitFetcher.class); + SplitCache splitCache = Mockito.mock(SplitCache.class); List parsedSplits = Lists.newArrayList(); ParsedSplit response = ParsedSplit.createParsedSplitForTests("FeatureName", 123, true, "off", Lists.newArrayList(getTestCondition("off")), "traffic", 456L, 1); parsedSplits.add(response); - Mockito.when(splitFetcher.fetchAll()).thenReturn(parsedSplits); - SplitManagerImpl splitManager = new SplitManagerImpl(splitFetcher, + Mockito.when(splitCache.getAll()).thenReturn(parsedSplits); + SplitManagerImpl splitManager = new SplitManagerImpl(splitCache, Mockito.mock(SplitClientConfig.class), Mockito.mock(SDKReadinessGates.class)); List splitNames = splitManager.splitNames(); @@ -149,7 +151,7 @@ public void splitNamesCallWithSplit() { public void block_until_ready_does_not_time_when_sdk_is_ready() throws TimeoutException, InterruptedException { SDKReadinessGates ready = mock(SDKReadinessGates.class); when(ready.isSDKReady(100)).thenReturn(true); - SplitManagerImpl splitManager = new SplitManagerImpl(mock(SplitFetcher.class), + SplitManagerImpl splitManager = new SplitManagerImpl(mock(SplitCache.class), config, ready); @@ -161,7 +163,7 @@ public void block_until_ready_times_when_sdk_is_not_ready() throws TimeoutExcept SDKReadinessGates ready = mock(SDKReadinessGates.class); when(ready.isSDKReady(100)).thenReturn(false); - SplitManagerImpl splitManager = new SplitManagerImpl(mock(SplitFetcher.class), + SplitManagerImpl splitManager = new SplitManagerImpl(mock(SplitCache.class), config, ready); diff --git a/client/src/test/java/io/split/client/impressions/ImpressionObserverTest.java b/client/src/test/java/io/split/client/impressions/ImpressionObserverTest.java index c4d794d6a..564ce9e34 100644 --- a/client/src/test/java/io/split/client/impressions/ImpressionObserverTest.java +++ b/client/src/test/java/io/split/client/impressions/ImpressionObserverTest.java @@ -1,7 +1,6 @@ package io.split.client.impressions; import com.google.common.base.Strings; -import io.split.client.dtos.KeyImpression; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/client/src/test/java/io/split/engine/cache/InMemoryCacheTest.java b/client/src/test/java/io/split/engine/cache/InMemoryCacheTest.java index 58db3905a..1a3b627c0 100644 --- a/client/src/test/java/io/split/engine/cache/InMemoryCacheTest.java +++ b/client/src/test/java/io/split/engine/cache/InMemoryCacheTest.java @@ -7,6 +7,9 @@ import java.util.*; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + public class InMemoryCacheTest { private SplitCache _cache; @@ -92,6 +95,26 @@ public void getMany() { Assert.assertEquals(2, result.size()); } + @Test + public void trafficTypesExist() { + SplitCache cache = new InMemoryCacheImp(-1); + + cache.put(ParsedSplit.createParsedSplitForTests("splitName_1", 0, false, "default_treatment", new ArrayList<>(), "tt", 123, 2)); + cache.put(ParsedSplit.createParsedSplitForTests("splitName_2", 0, false, "default_treatment", new ArrayList<>(), "tt", 123, 2)); + cache.put(ParsedSplit.createParsedSplitForTests("splitName_3", 0, false, "default_treatment", new ArrayList<>(), "tt_2", 123, 2)); + cache.put(ParsedSplit.createParsedSplitForTests("splitName_4", 0, false, "default_treatment", new ArrayList<>(), "tt_3", 123, 2)); + + assertTrue(cache.trafficTypeExists("tt_2")); + assertTrue(cache.trafficTypeExists("tt")); + assertFalse(cache.trafficTypeExists("tt_5")); + + cache.remove("splitName_2"); + assertTrue(cache.trafficTypeExists("tt")); + + cache.remove("splitName_1"); + assertFalse(cache.trafficTypeExists("tt")); + } + private ParsedSplit getParsedSplit(String splitName) { return ParsedSplit.createParsedSplitForTests(splitName, 0, false, "default_treatment", new ArrayList<>(), "tt", 123, 2); } diff --git a/client/src/test/java/io/split/engine/common/SynchronizerTest.java b/client/src/test/java/io/split/engine/common/SynchronizerTest.java index 95a65d4bf..67ab63f53 100644 --- a/client/src/test/java/io/split/engine/common/SynchronizerTest.java +++ b/client/src/test/java/io/split/engine/common/SynchronizerTest.java @@ -1,59 +1,52 @@ package io.split.engine.common; +import io.split.engine.cache.SplitCache; import io.split.engine.experiments.RefreshableSplitFetcher; -import io.split.engine.experiments.RefreshableSplitFetcherProvider; +import io.split.engine.experiments.RefreshableSplitFetcherTask; import io.split.engine.segments.RefreshableSegmentFetcher; +import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; public class SynchronizerTest { + private RefreshableSplitFetcherTask _refreshableSplitFetcherTask; + private RefreshableSegmentFetcher _segmentFetcher; + private RefreshableSplitFetcher _splitFetcher; + private SplitCache _splitCache; + private Synchronizer _synchronizer; + + @Before + public void beforeMethod() { + _refreshableSplitFetcherTask = Mockito.mock(RefreshableSplitFetcherTask.class); + _segmentFetcher = Mockito.mock(RefreshableSegmentFetcher.class); + _splitFetcher = Mockito.mock(RefreshableSplitFetcher.class); + _splitCache = Mockito.mock(SplitCache.class); + + _synchronizer = new SynchronizerImp(_refreshableSplitFetcherTask, _splitFetcher, _segmentFetcher, _splitCache); + } @Test public void syncAll() throws InterruptedException { - RefreshableSplitFetcherProvider refreshableSplitFetcherProvider = Mockito.mock(RefreshableSplitFetcherProvider.class); - RefreshableSegmentFetcher segmentFetcher = Mockito.mock(RefreshableSegmentFetcher.class); - RefreshableSplitFetcher splitFetcher = Mockito.mock(RefreshableSplitFetcher.class); - - Mockito.when(refreshableSplitFetcherProvider.getFetcher()) - .thenReturn(splitFetcher); - - Synchronizer synchronizer = new SynchronizerImp(refreshableSplitFetcherProvider, segmentFetcher); - synchronizer.syncAll(); + _synchronizer.syncAll(); Thread.sleep(100); - Mockito.verify(splitFetcher, Mockito.times(1)).run(); - Mockito.verify(segmentFetcher, Mockito.times(1)).forceRefreshAll(); + Mockito.verify(_splitFetcher, Mockito.times(1)).run(); + Mockito.verify(_segmentFetcher, Mockito.times(1)).forceRefreshAll(); } @Test public void startPeriodicFetching() { - RefreshableSplitFetcherProvider refreshableSplitFetcherProvider = Mockito.mock(RefreshableSplitFetcherProvider.class); - RefreshableSegmentFetcher segmentFetcher = Mockito.mock(RefreshableSegmentFetcher.class); - RefreshableSplitFetcher splitFetcher = Mockito.mock(RefreshableSplitFetcher.class); - - Mockito.when(refreshableSplitFetcherProvider.getFetcher()) - .thenReturn(splitFetcher); + _synchronizer.startPeriodicFetching(); - Synchronizer synchronizer = new SynchronizerImp(refreshableSplitFetcherProvider, segmentFetcher); - synchronizer.startPeriodicFetching(); - - Mockito.verify(refreshableSplitFetcherProvider, Mockito.times(1)).startPeriodicFetching(); - Mockito.verify(segmentFetcher, Mockito.times(1)).startPeriodicFetching(); + Mockito.verify(_refreshableSplitFetcherTask, Mockito.times(1)).startPeriodicFetching(); + Mockito.verify(_segmentFetcher, Mockito.times(1)).startPeriodicFetching(); } @Test public void stopPeriodicFetching() { - RefreshableSplitFetcherProvider refreshableSplitFetcherProvider = Mockito.mock(RefreshableSplitFetcherProvider.class); - RefreshableSegmentFetcher segmentFetcher = Mockito.mock(RefreshableSegmentFetcher.class); - RefreshableSplitFetcher splitFetcher = Mockito.mock(RefreshableSplitFetcher.class); - - Mockito.when(refreshableSplitFetcherProvider.getFetcher()) - .thenReturn(splitFetcher); - - Synchronizer synchronizer = new SynchronizerImp(refreshableSplitFetcherProvider, segmentFetcher); - synchronizer.stopPeriodicFetching(); + _synchronizer.stopPeriodicFetching(); - Mockito.verify(refreshableSplitFetcherProvider, Mockito.times(1)).stop(); - Mockito.verify(segmentFetcher, Mockito.times(1)).stop(); + Mockito.verify(_refreshableSplitFetcherTask, Mockito.times(1)).stop(); + Mockito.verify(_segmentFetcher, Mockito.times(1)).stop(); } } diff --git a/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java b/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java index 2b3994bd4..901352122 100644 --- a/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java +++ b/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java @@ -1,18 +1,12 @@ package io.split.engine.evaluator; -import io.split.client.EventClient; -import io.split.client.SplitClientConfig; -import io.split.client.SplitClientImpl; -import io.split.client.SplitFactory; import io.split.client.dtos.ConditionType; import io.split.client.dtos.Partition; -import io.split.client.impressions.ImpressionsManager; import io.split.engine.SDKReadinessGates; +import io.split.engine.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; -import io.split.engine.experiments.SplitFetcher; import io.split.engine.matchers.CombiningMatcher; -import io.split.engine.metrics.Metrics; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; @@ -34,7 +28,7 @@ public class EvaluatorTest { private static final String TRAFFIC_TYPE_VALUE = "tt"; private static final String TREATMENT_VALUE = "treatment_test"; - private SplitFetcher _splitFetcher; + private SplitCache _splitCache; private Evaluator _evaluator; private CombiningMatcher _matcher; private Map _configurations; @@ -44,8 +38,8 @@ public class EvaluatorTest { @Before public void before() { SDKReadinessGates gates = Mockito.mock(SDKReadinessGates.class); - _splitFetcher = Mockito.mock(SplitFetcher.class); - _evaluator = new EvaluatorImp(gates, _splitFetcher); + _splitCache = Mockito.mock(SplitCache.class); + _evaluator = new EvaluatorImp(gates, _splitCache); _matcher = Mockito.mock(CombiningMatcher.class); _configurations = new HashMap<>(); @@ -55,7 +49,7 @@ public void before() { @Test public void evaluateWhenSplitNameDoesNotExistReturnControl() { - Mockito.when(_splitFetcher.fetch(SPLIT_NAME)).thenReturn(null); + Mockito.when(_splitCache.get(SPLIT_NAME)).thenReturn(null); EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(MATCHING_KEY, BUCKETING_KEY, SPLIT_NAME, null); @@ -66,7 +60,7 @@ public void evaluateWhenSplitNameDoesNotExistReturnControl() { @Test public void evaluateWhenSplitIsKilledReturnDefaultTreatment() { ParsedSplit split = ParsedSplit.createParsedSplitForTests(SPLIT_NAME, 0, true, DEFAULT_TREATMENT_VALUE, _conditions, TRAFFIC_TYPE_VALUE, CHANGE_NUMBER, 2); - Mockito.when(_splitFetcher.fetch(SPLIT_NAME)).thenReturn(split); + Mockito.when(_splitCache.get(SPLIT_NAME)).thenReturn(split); EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(MATCHING_KEY, BUCKETING_KEY, SPLIT_NAME, null); @@ -78,7 +72,7 @@ public void evaluateWhenSplitIsKilledReturnDefaultTreatment() { @Test public void evaluateWithoutConditionsReturnDefaultTreatment() { ParsedSplit split = ParsedSplit.createParsedSplitForTests(SPLIT_NAME, 0, false, DEFAULT_TREATMENT_VALUE, _conditions, TRAFFIC_TYPE_VALUE, CHANGE_NUMBER, 2); - Mockito.when(_splitFetcher.fetch(SPLIT_NAME)).thenReturn(split); + Mockito.when(_splitCache.get(SPLIT_NAME)).thenReturn(split); EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(MATCHING_KEY, BUCKETING_KEY, SPLIT_NAME, null); @@ -98,7 +92,7 @@ public void evaluateWithRollOutConditionBucketIsBiggerTrafficAllocationReturnDef ParsedSplit split = new ParsedSplit(SPLIT_NAME, 0, false, DEFAULT_TREATMENT_VALUE, _conditions, TRAFFIC_TYPE_VALUE, CHANGE_NUMBER, 10, 12, 2, _configurations); - Mockito.when(_splitFetcher.fetch(SPLIT_NAME)).thenReturn(split); + Mockito.when(_splitCache.get(SPLIT_NAME)).thenReturn(split); Mockito.when(condition.matcher().match(MATCHING_KEY, BUCKETING_KEY, null, _evaluator)).thenReturn(true); EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(MATCHING_KEY, BUCKETING_KEY, SPLIT_NAME, null); @@ -119,7 +113,7 @@ public void evaluateWithRollOutConditionTrafficAllocationIsBiggerBucketReturnTre ParsedSplit split = new ParsedSplit(SPLIT_NAME, 0, false, DEFAULT_TREATMENT_VALUE, _conditions, TRAFFIC_TYPE_VALUE, CHANGE_NUMBER, 60, 18, 2, _configurations); - Mockito.when(_splitFetcher.fetch(SPLIT_NAME)).thenReturn(split); + Mockito.when(_splitCache.get(SPLIT_NAME)).thenReturn(split); Mockito.when(condition.matcher().match(MATCHING_KEY, BUCKETING_KEY, null, _evaluator)).thenReturn(true); EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(MATCHING_KEY, BUCKETING_KEY, SPLIT_NAME, null); @@ -140,7 +134,7 @@ public void evaluateWithWhitelistConditionReturnTreatment() { ParsedSplit split = new ParsedSplit(SPLIT_NAME, 0, false, DEFAULT_TREATMENT_VALUE, _conditions, TRAFFIC_TYPE_VALUE, CHANGE_NUMBER, 60, 18, 2, _configurations); - Mockito.when(_splitFetcher.fetch(SPLIT_NAME)).thenReturn(split); + Mockito.when(_splitCache.get(SPLIT_NAME)).thenReturn(split); Mockito.when(condition.matcher().match(MATCHING_KEY, BUCKETING_KEY, null, _evaluator)).thenReturn(true); EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(MATCHING_KEY, BUCKETING_KEY, SPLIT_NAME, null); @@ -149,14 +143,4 @@ public void evaluateWithWhitelistConditionReturnTreatment() { assertEquals("test whitelist label", result.label); assertEquals(CHANGE_NUMBER, result.changeNumber); } - - private SplitClientImpl getSplitClient(SplitFetcher splitFetcher, SDKReadinessGates gates, Evaluator evaluator) { - SplitFactory container = Mockito.mock(SplitFactory.class); - ImpressionsManager impressionManager = Mockito.mock(ImpressionsManager.class); - Metrics metrics = Mockito.mock(Metrics.class); - EventClient eventClient = Mockito.mock(EventClient.class); - SplitClientConfig config = Mockito.mock(SplitClientConfig.class); - - return new SplitClientImpl(container, splitFetcher, impressionManager, metrics, eventClient, config, gates, evaluator); - } } diff --git a/client/src/test/java/io/split/engine/experiments/RefreshableSplitFetcherTest.java b/client/src/test/java/io/split/engine/experiments/RefreshableSplitFetcherTest.java index 67e07b45b..a29f753e2 100644 --- a/client/src/test/java/io/split/engine/experiments/RefreshableSplitFetcherTest.java +++ b/client/src/test/java/io/split/engine/experiments/RefreshableSplitFetcherTest.java @@ -24,7 +24,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.concurrent.Executors; @@ -62,26 +61,25 @@ private void works(long startingChangeNumber) throws InterruptedException { SDKReadinessGates gates = new SDKReadinessGates(); SplitCache cache = new InMemoryCacheImp(startingChangeNumber); - RefreshableSplitFetcher fetcher = new RefreshableSplitFetcher(splitChangeFetcher, new SplitParser(segmentFetcher), gates, cache); // execute the fetcher for a little bit. executeWaitAndTerminate(fetcher, 1, 3, TimeUnit.SECONDS); assertThat(splitChangeFetcher.lastAdded(), is(greaterThan(startingChangeNumber))); - assertThat(fetcher.changeNumber(), is(equalTo(splitChangeFetcher.lastAdded()))); + assertThat(cache.getChangeNumber(), is(equalTo(splitChangeFetcher.lastAdded()))); // all previous splits have been removed since they are dead - for (long i = startingChangeNumber; i < fetcher.changeNumber(); i++) { - assertThat("Asking for " + i + " " + fetcher.fetchAll(), fetcher.fetch("" + i), is(not(nullValue()))); - assertThat(fetcher.fetch("" + i).killed(), is(true)); + for (long i = startingChangeNumber; i < cache.getChangeNumber(); i++) { + assertThat("Asking for " + i + " " + cache.getAll(), cache.get("" + i), is(not(nullValue()))); + assertThat(cache.get("" + i).killed(), is(true)); } ParsedCondition expectedParsedCondition = ParsedCondition.createParsedConditionForTests(CombiningMatcher.of(new AllKeysMatcher()), Lists.newArrayList(ConditionsTestUtil.partition("on", 10))); List expectedListOfMatcherAndSplits = Lists.newArrayList(expectedParsedCondition); - ParsedSplit expected = ParsedSplit.createParsedSplitForTests("" + fetcher.changeNumber(), (int) fetcher.changeNumber(), false, Treatments.OFF, expectedListOfMatcherAndSplits, null, fetcher.changeNumber(), 1); + ParsedSplit expected = ParsedSplit.createParsedSplitForTests("" + cache.getChangeNumber(), (int) cache.getChangeNumber(), false, Treatments.OFF, expectedListOfMatcherAndSplits, null, cache.getChangeNumber(), 1); - ParsedSplit actual = fetcher.fetch("" + fetcher.changeNumber()); + ParsedSplit actual = cache.get("" + cache.getChangeNumber()); assertThat(actual, is(equalTo(expected))); assertThat(gates.areSplitsReady(0), is(equalTo(true))); @@ -138,9 +136,9 @@ public void when_parser_fails_we_remove_the_experiment() throws InterruptedExcep // execute the fetcher for a little bit. executeWaitAndTerminate(fetcher, 1, 5, TimeUnit.SECONDS); - assertThat(fetcher.changeNumber(), is(equalTo(1L))); + assertThat(cache.getChangeNumber(), is(equalTo(1L))); // verify that the fetcher return null - assertThat(fetcher.fetch("-1"), is(nullValue())); + assertThat(cache.get("-1"), is(nullValue())); } @Test @@ -157,7 +155,7 @@ public void if_there_is_a_problem_talking_to_split_change_count_down_latch_is_no // execute the fetcher for a little bit. executeWaitAndTerminate(fetcher, 1, 5, TimeUnit.SECONDS); - assertThat(fetcher.changeNumber(), is(equalTo(-1L))); + assertThat(cache.getChangeNumber(), is(equalTo(-1L))); assertThat(gates.areSplitsReady(0), is(equalTo(false))); } @@ -197,12 +195,12 @@ public void works_with_user_defined_segments() throws Exception { executeWaitAndTerminate(fetcher, 1, 5, TimeUnit.SECONDS); assertThat(experimentChangeFetcher.lastAdded(), is(greaterThan(startingChangeNumber))); - assertThat(fetcher.changeNumber(), is(equalTo(experimentChangeFetcher.lastAdded()))); + assertThat(cache.getChangeNumber(), is(equalTo(experimentChangeFetcher.lastAdded()))); // all previous splits have been removed since they are dead - for (long i = startingChangeNumber; i < fetcher.changeNumber(); i++) { - assertThat("Asking for " + i + " " + fetcher.fetchAll(), fetcher.fetch("" + i), is(not(nullValue()))); - assertThat(fetcher.fetch("" + i).killed(), is(true)); + for (long i = startingChangeNumber; i < cache.getChangeNumber(); i++) { + assertThat("Asking for " + i + " " + cache.getAll(), cache.get("" + i), is(not(nullValue()))); + assertThat(cache.get("" + i).killed(), is(true)); } assertThat(gates.areSplitsReady(0), is(equalTo(true))); @@ -210,30 +208,4 @@ public void works_with_user_defined_segments() throws Exception { assertThat(gates.areSegmentsReady(100), is(equalTo(true))); assertThat(gates.isSDKReady(0), is(equalTo(true))); } - - @Test - public void trafficTypesExist() { - SplitChangeFetcherWithTrafficTypeNames changeFetcher = new SplitChangeFetcherWithTrafficTypeNames(); - SDKReadinessGates gates = new SDKReadinessGates(); - SplitCache cache = new InMemoryCacheImp(-1); - - SegmentChangeFetcher segmentChangeFetcher = new NoChangeSegmentChangeFetcher(); - SegmentFetcher segmentFetcher = new RefreshableSegmentFetcher(segmentChangeFetcher, 1,10, gates); - RefreshableSplitFetcher fetcher = new RefreshableSplitFetcher(changeFetcher, new SplitParser(segmentFetcher), gates, cache); - - cache.put(ParsedSplit.createParsedSplitForTests("splitName_1", 0, false, "default_treatment", new ArrayList<>(), "tt", 123, 2)); - cache.put(ParsedSplit.createParsedSplitForTests("splitName_2", 0, false, "default_treatment", new ArrayList<>(), "tt", 123, 2)); - cache.put(ParsedSplit.createParsedSplitForTests("splitName_3", 0, false, "default_treatment", new ArrayList<>(), "tt_2", 123, 2)); - cache.put(ParsedSplit.createParsedSplitForTests("splitName_4", 0, false, "default_treatment", new ArrayList<>(), "tt_3", 123, 2)); - - assertTrue(fetcher.trafficTypeExists("tt_2")); - assertTrue(fetcher.trafficTypeExists("tt")); - assertFalse(fetcher.trafficTypeExists("tt_5")); - - cache.remove("splitName_2"); - assertTrue(fetcher.trafficTypeExists("tt")); - - cache.remove("splitName_1"); - assertFalse(fetcher.trafficTypeExists("tt")); - } } From 9aded1648b7076b54df41a3d4a94ae366910d48e Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Fri, 11 Dec 2020 11:29:07 -0300 Subject: [PATCH 2/6] wip --- client/src/main/java/io/split/client/SplitFactoryImpl.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/client/src/main/java/io/split/client/SplitFactoryImpl.java b/client/src/main/java/io/split/client/SplitFactoryImpl.java index 3bfa116c0..e53df1d8b 100644 --- a/client/src/main/java/io/split/client/SplitFactoryImpl.java +++ b/client/src/main/java/io/split/client/SplitFactoryImpl.java @@ -177,7 +177,6 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn } - final CloseableHttpClient httpclient = buildHttpClient(apiToken, config); URI rootTarget = URI.create(config.endpoint()); @@ -207,7 +206,6 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn final RefreshableSplitFetcher splitFetcher = new RefreshableSplitFetcher(splitChangeFetcher, splitParser, gates, splitCache); final RefreshableSplitFetcherTask splitFetcherTask = new RefreshableSplitFetcherTask(splitFetcher, splitCache, findPollingPeriod(RANDOM, config.featuresRefreshRate())); - List impressionListeners = new ArrayList<>(); // Setup integrations if (config.integrationsConfig() != null) { @@ -232,6 +230,9 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn final SyncManager syncManager = SyncManagerImp.build(config.streamingEnabled(), splitFetcherTask, splitFetcher, segmentFetcher, splitCache, config.authServiceURL(), httpclient, config.streamingServiceURL(), config.authRetryBackoffBase(), buildSSEdHttpClient(config)); syncManager.start(); + // Evaluator + final Evaluator evaluator = new EvaluatorImp(gates, splitCache); + destroyer = new Runnable() { public void run() { _log.info("Shutdown called for split"); @@ -268,8 +269,6 @@ public void run() { }); } - final Evaluator evaluator = new EvaluatorImp(gates, splitCache); - _client = new SplitClientImpl(this, splitCache, impressionsManager, From 9eb18e54ad1cd5d73c0a9c60b03431913efd0552 Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Fri, 11 Dec 2020 12:38:18 -0300 Subject: [PATCH 3/6] wip --- client/src/main/java/io/split/client/SplitFactoryImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/src/main/java/io/split/client/SplitFactoryImpl.java b/client/src/main/java/io/split/client/SplitFactoryImpl.java index e53df1d8b..fe063252e 100644 --- a/client/src/main/java/io/split/client/SplitFactoryImpl.java +++ b/client/src/main/java/io/split/client/SplitFactoryImpl.java @@ -201,8 +201,7 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn // Feature Changes SplitChangeFetcher splitChangeFetcher = HttpSplitChangeFetcher.create(httpclient, rootTarget, uncachedFireAndForget); - final SplitCache cache = new InMemoryCacheImp(); - final SplitCache splitCache = new InMemoryCacheImp(-1); + final SplitCache splitCache = new InMemoryCacheImp(); final RefreshableSplitFetcher splitFetcher = new RefreshableSplitFetcher(splitChangeFetcher, splitParser, gates, splitCache); final RefreshableSplitFetcherTask splitFetcherTask = new RefreshableSplitFetcherTask(splitFetcher, splitCache, findPollingPeriod(RANDOM, config.featuresRefreshRate())); From f0d0bc69c051d2469c452a673cb5bcb6f189c962 Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Wed, 16 Dec 2020 10:32:53 -0300 Subject: [PATCH 4/6] pr feedback --- .../java/io/split/client/SplitClientImpl.java | 7 ++ .../io/split/client/SplitFactoryImpl.java | 2 +- .../split/engine/evaluator/EvaluatorImp.java | 12 +--- .../RefreshableSplitFetcherTask.java | 2 +- .../io/split/client/SplitClientImplTest.java | 64 +++++++++---------- .../split/engine/evaluator/EvaluatorTest.java | 2 +- 6 files changed, 43 insertions(+), 46 deletions(-) diff --git a/client/src/main/java/io/split/client/SplitClientImpl.java b/client/src/main/java/io/split/client/SplitClientImpl.java index 9723eb8ad..6807d5fdb 100644 --- a/client/src/main/java/io/split/client/SplitClientImpl.java +++ b/client/src/main/java/io/split/client/SplitClientImpl.java @@ -32,6 +32,7 @@ public final class SplitClientImpl implements SplitClient { public static final SplitResult SPLIT_RESULT_CONTROL = new SplitResult(Treatments.CONTROL, null); private static final String GET_TREATMENT_LABEL = "sdk.getTreatment"; + private static final String DEFINITION_NOT_FOUND = "definition not found"; private static final Logger _log = LoggerFactory.getLogger(SplitClientImpl.class); @@ -314,6 +315,12 @@ private SplitResult getTreatmentWithConfigInternal(String label, String matching EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(matchingKey, bucketingKey, split, attributes); + if (result.treatment.equals(Treatments.CONTROL) && result.label.equals(DEFINITION_NOT_FOUND) && _gates.isSDKReadyNow()) { + _log.warn( + "getTreatment: you passed \"" + split + "\" that does not exist in this environment, " + + "please double check what Splits exist in the web console."); + } + recordStats( matchingKey, bucketingKey, diff --git a/client/src/main/java/io/split/client/SplitFactoryImpl.java b/client/src/main/java/io/split/client/SplitFactoryImpl.java index fe063252e..229ae2c7a 100644 --- a/client/src/main/java/io/split/client/SplitFactoryImpl.java +++ b/client/src/main/java/io/split/client/SplitFactoryImpl.java @@ -230,7 +230,7 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn syncManager.start(); // Evaluator - final Evaluator evaluator = new EvaluatorImp(gates, splitCache); + final Evaluator evaluator = new EvaluatorImp(splitCache); destroyer = new Runnable() { public void run() { diff --git a/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java b/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java index 9da48d60f..df431acb8 100644 --- a/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java +++ b/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java @@ -2,11 +2,9 @@ import io.split.client.dtos.ConditionType; import io.split.client.exceptions.ChangeNumberExceptionWrapper; -import io.split.engine.SDKReadinessGates; import io.split.engine.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; -import io.split.engine.experiments.SplitFetcher; import io.split.engine.splitter.Splitter; import io.split.grammar.Treatments; import org.slf4j.Logger; @@ -25,12 +23,9 @@ public class EvaluatorImp implements Evaluator { private static final Logger _log = LoggerFactory.getLogger(EvaluatorImp.class); - private final SDKReadinessGates _gates; private final SplitCache _splitCache; - public EvaluatorImp(SDKReadinessGates gates, - SplitCache splitCache) { - _gates = checkNotNull(gates); + public EvaluatorImp(SplitCache splitCache) { _splitCache = checkNotNull(splitCache); } @@ -40,11 +35,6 @@ public TreatmentLabelAndChangeNumber evaluateFeature(String matchingKey, String ParsedSplit parsedSplit = _splitCache.get(split); if (parsedSplit == null) { - if (_gates.isSDKReadyNow()) { - _log.warn( - "getTreatment: you passed \"" + split + "\" that does not exist in this environment, " + - "please double check what Splits exist in the web console."); - } return new TreatmentLabelAndChangeNumber(Treatments.CONTROL, DEFINITION_NOT_FOUND); } diff --git a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java b/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java index 9a9f5cdbf..9e60cdd31 100644 --- a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java +++ b/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java @@ -83,7 +83,7 @@ public void close() { _splitCache.get().clear(); } - _running.set(false); + stop(); ScheduledExecutorService scheduledExecutorService = _executorService.get(); if (scheduledExecutorService.isShutdown()) { diff --git a/client/src/test/java/io/split/client/SplitClientImplTest.java b/client/src/test/java/io/split/client/SplitClientImplTest.java index 0f6cfe68a..4dffea065 100644 --- a/client/src/test/java/io/split/client/SplitClientImplTest.java +++ b/client/src/test/java/io/split/client/SplitClientImplTest.java @@ -83,7 +83,7 @@ public void null_key_results_in_control() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment(null, "test1"), is(equalTo(Treatments.CONTROL))); @@ -110,7 +110,7 @@ public void null_test_results_in_control() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("adil@relateiq.com", null), is(equalTo(Treatments.CONTROL))); @@ -132,7 +132,7 @@ public void exceptions_result_in_control() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("adil@relateiq.com", "test1"), is(equalTo(Treatments.CONTROL))); @@ -159,7 +159,7 @@ public void works() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); int numKeys = 5; @@ -194,7 +194,7 @@ public void works_null_config() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); @@ -231,7 +231,7 @@ public void worksAndHasConfig() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); int numKeys = 5; @@ -265,7 +265,7 @@ public void last_condition_is_always_default() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("pato@codigo.com", test), is(equalTo(Treatments.OFF))); @@ -301,7 +301,7 @@ public void last_condition_is_always_default_but_with_treatment() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); SplitResult result = client.getTreatmentWithConfig("pato@codigo.com", test); @@ -334,7 +334,7 @@ public void multiple_conditions_work() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("on"))); @@ -365,7 +365,7 @@ public void killed_test_always_goes_to_default() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo(Treatments.OFF))); @@ -401,7 +401,7 @@ public void killed_test_always_goes_to_default_has_config() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); SplitResult result = client.getTreatmentWithConfig("adil@codigo.com", test); @@ -437,7 +437,7 @@ public void dependency_matcher_on() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("key", parent), is(equalTo(Treatments.ON))); @@ -470,7 +470,7 @@ public void dependency_matcher_off() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("key", parent), is(equalTo(Treatments.ON))); @@ -497,7 +497,7 @@ public void dependency_matcher_control() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("key", dependent), is(equalTo(Treatments.ON))); @@ -525,7 +525,7 @@ public void attributes_work() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("on"))); @@ -559,7 +559,7 @@ public void attributes_work_2() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off"))); @@ -593,7 +593,7 @@ public void attributes_greater_than_negative_number() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off"))); @@ -630,7 +630,7 @@ public void attributes_for_sets() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("adil@codigo.com", test), is(equalTo("off"))); @@ -673,7 +673,7 @@ public void labels_are_populated() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Map attributes = ImmutableMap.of("age", -20, "acv", "1000000"); @@ -765,7 +765,7 @@ private void traffic_allocation(String key, int trafficAllocation, int trafficAl NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment(key, test), is(equalTo(expected_treatment_on_or_off))); @@ -813,7 +813,7 @@ public void notInTrafficAllocationDefaultConfig() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); assertThat(client.getTreatment("pato@split.io", test), is(equalTo(Treatments.OFF))); @@ -853,7 +853,7 @@ public void matching_bucketing_keys_work() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Key bad_key = new Key("adil", "aijaz"); @@ -891,7 +891,7 @@ public void impression_metadata_is_propagated() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Map attributes = ImmutableMap.of("age", -20, "acv", "1000000"); @@ -927,7 +927,7 @@ public void block_until_ready_does_not_time_when_sdk_is_ready() throws TimeoutEx NoopEventClient.create(), config, ready, - new EvaluatorImp(ready, splitCache) + new EvaluatorImp(splitCache) ); client.blockUntilReady(); @@ -947,7 +947,7 @@ public void block_until_ready_times_when_sdk_is_not_ready() throws TimeoutExcept NoopEventClient.create(), config, ready, - new EvaluatorImp(ready, splitCache) + new EvaluatorImp(splitCache) ); client.blockUntilReady(); @@ -966,7 +966,7 @@ public void track_with_valid_parameters() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Assert.assertThat(client.track("validKey", "valid_traffic_type", "valid_event"), @@ -992,7 +992,7 @@ public void track_with_invalid_event_type_ids() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Assert.assertThat(client.track("validKey", "valid_traffic_type", ""), @@ -1023,7 +1023,7 @@ public void track_with_invalid_traffic_type_names() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Assert.assertThat(client.track("validKey", "", "valid"), @@ -1046,7 +1046,7 @@ public void track_with_invalid_keys() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Assert.assertThat(client.track("", "valid_traffic_type", "valid"), @@ -1075,7 +1075,7 @@ public void track_with_properties() { eventClientMock, config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); HashMap properties = new HashMap<>(); @@ -1186,7 +1186,7 @@ public void getTreatment_with_invalid_keys() { NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Assert.assertThat(client.getTreatment("valid", "split"), @@ -1271,7 +1271,7 @@ public void client_cannot_perform_actions_when_destroyed() throws InterruptedExc NoopEventClient.create(), config, gates, - new EvaluatorImp(gates, splitCache) + new EvaluatorImp(splitCache) ); Assert.assertThat(client.getTreatment("valid", "split"), diff --git a/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java b/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java index 901352122..c8b0b7229 100644 --- a/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java +++ b/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java @@ -39,7 +39,7 @@ public class EvaluatorTest { public void before() { SDKReadinessGates gates = Mockito.mock(SDKReadinessGates.class); _splitCache = Mockito.mock(SplitCache.class); - _evaluator = new EvaluatorImp(gates, _splitCache); + _evaluator = new EvaluatorImp(_splitCache); _matcher = Mockito.mock(CombiningMatcher.class); _configurations = new HashMap<>(); From b698c90442cd2af627a9f3e44864f6db87c2d1cd Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Tue, 15 Dec 2020 16:15:38 -0300 Subject: [PATCH 5/6] rename and refactor --- .../{engine => }/cache/InMemoryCacheImp.java | 2 +- .../split/{engine => }/cache/SplitCache.java | 2 +- .../java/io/split/client/SplitClientImpl.java | 2 +- .../io/split/client/SplitFactoryImpl.java | 16 +++++++-------- .../io/split/client/SplitManagerImpl.java | 2 +- .../io/split/client/jmx/SplitJmxMonitor.java | 2 +- .../split/engine/common/SyncManagerImp.java | 12 +++++------ .../split/engine/common/SynchronizerImp.java | 20 +++++++++---------- .../split/engine/evaluator/EvaluatorImp.java | 2 +- ...SplitFetcher.java => SplitFetcherImp.java} | 8 ++++---- ...ask.java => SplitSynchronizationTask.java} | 10 +++++----- .../{engine => }/cache/InMemoryCacheTest.java | 2 +- .../io/split/client/SplitClientImplTest.java | 6 ++---- .../io/split/client/SplitManagerImplTest.java | 4 +--- .../split/engine/common/SynchronizerTest.java | 14 ++++++------- .../split/engine/evaluator/EvaluatorTest.java | 2 +- .../RefreshableSplitFetcherTest.java | 12 +++++------ client/src/test/resources/log4j.properties | 2 +- 18 files changed, 58 insertions(+), 62 deletions(-) rename client/src/main/java/io/split/{engine => }/cache/InMemoryCacheImp.java (99%) rename client/src/main/java/io/split/{engine => }/cache/SplitCache.java (94%) rename client/src/main/java/io/split/engine/experiments/{RefreshableSplitFetcher.java => SplitFetcherImp.java} (94%) rename client/src/main/java/io/split/engine/experiments/{RefreshableSplitFetcherTask.java => SplitSynchronizationTask.java} (88%) rename client/src/test/java/io/split/{engine => }/cache/InMemoryCacheTest.java (99%) diff --git a/client/src/main/java/io/split/engine/cache/InMemoryCacheImp.java b/client/src/main/java/io/split/cache/InMemoryCacheImp.java similarity index 99% rename from client/src/main/java/io/split/engine/cache/InMemoryCacheImp.java rename to client/src/main/java/io/split/cache/InMemoryCacheImp.java index 003a3d1bb..decc31d25 100644 --- a/client/src/main/java/io/split/engine/cache/InMemoryCacheImp.java +++ b/client/src/main/java/io/split/cache/InMemoryCacheImp.java @@ -1,4 +1,4 @@ -package io.split.engine.cache; +package io.split.cache; import com.google.common.collect.ConcurrentHashMultiset; import com.google.common.collect.Maps; diff --git a/client/src/main/java/io/split/engine/cache/SplitCache.java b/client/src/main/java/io/split/cache/SplitCache.java similarity index 94% rename from client/src/main/java/io/split/engine/cache/SplitCache.java rename to client/src/main/java/io/split/cache/SplitCache.java index f79f27878..c9769d176 100644 --- a/client/src/main/java/io/split/engine/cache/SplitCache.java +++ b/client/src/main/java/io/split/cache/SplitCache.java @@ -1,4 +1,4 @@ -package io.split.engine.cache; +package io.split.cache; import io.split.engine.experiments.ParsedSplit; diff --git a/client/src/main/java/io/split/client/SplitClientImpl.java b/client/src/main/java/io/split/client/SplitClientImpl.java index 6807d5fdb..4937123dd 100644 --- a/client/src/main/java/io/split/client/SplitClientImpl.java +++ b/client/src/main/java/io/split/client/SplitClientImpl.java @@ -5,7 +5,7 @@ import io.split.client.dtos.Event; import io.split.client.impressions.Impression; import io.split.client.impressions.ImpressionsManager; -import io.split.engine.cache.SplitCache; +import io.split.cache.SplitCache; import io.split.engine.evaluator.Evaluator; import io.split.engine.SDKReadinessGates; import io.split.engine.evaluator.EvaluatorImp; diff --git a/client/src/main/java/io/split/client/SplitFactoryImpl.java b/client/src/main/java/io/split/client/SplitFactoryImpl.java index 229ae2c7a..3c0692af9 100644 --- a/client/src/main/java/io/split/client/SplitFactoryImpl.java +++ b/client/src/main/java/io/split/client/SplitFactoryImpl.java @@ -11,15 +11,15 @@ import io.split.client.metrics.CachedMetrics; import io.split.client.metrics.FireAndForgetMetrics; import io.split.client.metrics.HttpMetrics; -import io.split.engine.cache.InMemoryCacheImp; -import io.split.engine.cache.SplitCache; +import io.split.cache.InMemoryCacheImp; +import io.split.cache.SplitCache; import io.split.engine.evaluator.Evaluator; import io.split.engine.evaluator.EvaluatorImp; import io.split.engine.SDKReadinessGates; import io.split.engine.common.SyncManager; import io.split.engine.common.SyncManagerImp; -import io.split.engine.experiments.RefreshableSplitFetcher; -import io.split.engine.experiments.RefreshableSplitFetcherTask; +import io.split.engine.experiments.SplitFetcherImp; +import io.split.engine.experiments.SplitSynchronizationTask; import io.split.engine.experiments.SplitChangeFetcher; import io.split.engine.experiments.SplitParser; import io.split.engine.segments.RefreshableSegmentFetcher; @@ -202,8 +202,8 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn SplitChangeFetcher splitChangeFetcher = HttpSplitChangeFetcher.create(httpclient, rootTarget, uncachedFireAndForget); final SplitCache splitCache = new InMemoryCacheImp(); - final RefreshableSplitFetcher splitFetcher = new RefreshableSplitFetcher(splitChangeFetcher, splitParser, gates, splitCache); - final RefreshableSplitFetcherTask splitFetcherTask = new RefreshableSplitFetcherTask(splitFetcher, splitCache, findPollingPeriod(RANDOM, config.featuresRefreshRate())); + final SplitFetcherImp splitFetcher = new SplitFetcherImp(splitChangeFetcher, splitParser, gates, splitCache); + final SplitSynchronizationTask splitSynchronizationTask = new SplitSynchronizationTask(splitFetcher, splitCache, findPollingPeriod(RANDOM, config.featuresRefreshRate())); List impressionListeners = new ArrayList<>(); // Setup integrations @@ -226,7 +226,7 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn final EventClient eventClient = EventClientImpl.create(httpclient, eventsRootTarget, config.eventsQueueSize(), config.eventFlushIntervalInMillis(), config.waitBeforeShutdown()); // SyncManager - final SyncManager syncManager = SyncManagerImp.build(config.streamingEnabled(), splitFetcherTask, splitFetcher, segmentFetcher, splitCache, config.authServiceURL(), httpclient, config.streamingServiceURL(), config.authRetryBackoffBase(), buildSSEdHttpClient(config)); + final SyncManager syncManager = SyncManagerImp.build(config.streamingEnabled(), splitSynchronizationTask, splitFetcher, segmentFetcher, splitCache, config.authServiceURL(), httpclient, config.streamingServiceURL(), config.authRetryBackoffBase(), buildSSEdHttpClient(config)); syncManager.start(); // Evaluator @@ -238,7 +238,7 @@ public void run() { try { segmentFetcher.close(); _log.info("Successful shutdown of segment fetchers"); - splitFetcherTask.close(); + splitSynchronizationTask.close(); _log.info("Successful shutdown of splits"); impressionsManager.close(); _log.info("Successful shutdown of impressions manager"); diff --git a/client/src/main/java/io/split/client/SplitManagerImpl.java b/client/src/main/java/io/split/client/SplitManagerImpl.java index 32128efe8..24f43a926 100644 --- a/client/src/main/java/io/split/client/SplitManagerImpl.java +++ b/client/src/main/java/io/split/client/SplitManagerImpl.java @@ -4,7 +4,7 @@ import io.split.client.api.SplitView; import io.split.client.dtos.Partition; import io.split.engine.SDKReadinessGates; -import io.split.engine.cache.SplitCache; +import io.split.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; import org.slf4j.Logger; diff --git a/client/src/main/java/io/split/client/jmx/SplitJmxMonitor.java b/client/src/main/java/io/split/client/jmx/SplitJmxMonitor.java index b5bff41d8..d44a298e7 100644 --- a/client/src/main/java/io/split/client/jmx/SplitJmxMonitor.java +++ b/client/src/main/java/io/split/client/jmx/SplitJmxMonitor.java @@ -1,7 +1,7 @@ package io.split.client.jmx; import io.split.client.SplitClient; -import io.split.engine.cache.SplitCache; +import io.split.cache.SplitCache; import io.split.engine.experiments.SplitFetcher; import io.split.engine.segments.SegmentFetcher; import org.slf4j.Logger; diff --git a/client/src/main/java/io/split/engine/common/SyncManagerImp.java b/client/src/main/java/io/split/engine/common/SyncManagerImp.java index 1576cce80..2f261a883 100644 --- a/client/src/main/java/io/split/engine/common/SyncManagerImp.java +++ b/client/src/main/java/io/split/engine/common/SyncManagerImp.java @@ -2,9 +2,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.ThreadFactoryBuilder; -import io.split.engine.cache.SplitCache; -import io.split.engine.experiments.RefreshableSplitFetcher; -import io.split.engine.experiments.RefreshableSplitFetcherTask; +import io.split.cache.SplitCache; +import io.split.engine.experiments.SplitFetcherImp; +import io.split.engine.experiments.SplitSynchronizationTask; import io.split.engine.segments.RefreshableSegmentFetcher; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.slf4j.Logger; @@ -46,8 +46,8 @@ public class SyncManagerImp implements SyncManager { } public static SyncManagerImp build(boolean streamingEnabledConfig, - RefreshableSplitFetcherTask refreshableSplitFetcherTask, - RefreshableSplitFetcher splitFetcher, + SplitSynchronizationTask splitSynchronizationTask, + SplitFetcherImp splitFetcher, RefreshableSegmentFetcher segmentFetcher, SplitCache splitCache, String authUrl, @@ -56,7 +56,7 @@ public static SyncManagerImp build(boolean streamingEnabledConfig, int authRetryBackOffBase, CloseableHttpClient sseHttpClient) { LinkedBlockingQueue pushMessages = new LinkedBlockingQueue<>(); - Synchronizer synchronizer = new SynchronizerImp(refreshableSplitFetcherTask, splitFetcher, segmentFetcher, splitCache); + Synchronizer synchronizer = new SynchronizerImp(splitSynchronizationTask, splitFetcher, segmentFetcher, splitCache); PushManager pushManager = PushManagerImp.build(synchronizer, streamingServiceUrl, authUrl, httpClient, authRetryBackOffBase, pushMessages, sseHttpClient); return new SyncManagerImp(streamingEnabledConfig, synchronizer, pushManager, pushMessages); } diff --git a/client/src/main/java/io/split/engine/common/SynchronizerImp.java b/client/src/main/java/io/split/engine/common/SynchronizerImp.java index fedcbc4f2..94b0f638b 100644 --- a/client/src/main/java/io/split/engine/common/SynchronizerImp.java +++ b/client/src/main/java/io/split/engine/common/SynchronizerImp.java @@ -2,9 +2,9 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; -import io.split.engine.cache.SplitCache; -import io.split.engine.experiments.RefreshableSplitFetcher; -import io.split.engine.experiments.RefreshableSplitFetcherTask; +import io.split.cache.SplitCache; +import io.split.engine.experiments.SplitFetcherImp; +import io.split.engine.experiments.SplitSynchronizationTask; import io.split.engine.segments.RefreshableSegmentFetcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -19,17 +19,17 @@ public class SynchronizerImp implements Synchronizer { private static final Logger _log = LoggerFactory.getLogger(Synchronizer.class); - private final RefreshableSplitFetcherTask _refreshableSplitFetcherTask; - private final RefreshableSplitFetcher _splitFetcher; + private final SplitSynchronizationTask _splitSynchronizationTask; + private final SplitFetcherImp _splitFetcher; private final RefreshableSegmentFetcher _segmentFetcher; private final ScheduledExecutorService _syncAllScheduledExecutorService; private final SplitCache _splitCache; - public SynchronizerImp(RefreshableSplitFetcherTask refreshableSplitTask, - RefreshableSplitFetcher splitFetcher, + public SynchronizerImp(SplitSynchronizationTask splitSynchronizationTask, + SplitFetcherImp splitFetcher, RefreshableSegmentFetcher segmentFetcher, SplitCache splitCache) { - _refreshableSplitFetcherTask = checkNotNull(refreshableSplitTask); + _splitSynchronizationTask = checkNotNull(splitSynchronizationTask); _splitFetcher = checkNotNull(splitFetcher); _segmentFetcher = checkNotNull(segmentFetcher); _splitCache = checkNotNull(splitCache); @@ -52,14 +52,14 @@ public void syncAll() { @Override public void startPeriodicFetching() { _log.debug("Starting Periodic Fetching ..."); - _refreshableSplitFetcherTask.startPeriodicFetching(); + _splitSynchronizationTask.startPeriodicFetching(); _segmentFetcher.startPeriodicFetching(); } @Override public void stopPeriodicFetching() { _log.debug("Stop Periodic Fetching ..."); - _refreshableSplitFetcherTask.stop(); + _splitSynchronizationTask.stop(); _segmentFetcher.stop(); } diff --git a/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java b/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java index df431acb8..7f974fc94 100644 --- a/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java +++ b/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java @@ -2,7 +2,7 @@ import io.split.client.dtos.ConditionType; import io.split.client.exceptions.ChangeNumberExceptionWrapper; -import io.split.engine.cache.SplitCache; +import io.split.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; import io.split.engine.splitter.Splitter; diff --git a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcher.java b/client/src/main/java/io/split/engine/experiments/SplitFetcherImp.java similarity index 94% rename from client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcher.java rename to client/src/main/java/io/split/engine/experiments/SplitFetcherImp.java index df696ec47..b62df6b50 100644 --- a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcher.java +++ b/client/src/main/java/io/split/engine/experiments/SplitFetcherImp.java @@ -4,7 +4,7 @@ import io.split.client.dtos.SplitChange; import io.split.client.dtos.Status; import io.split.engine.SDKReadinessGates; -import io.split.engine.cache.SplitCache; +import io.split.cache.SplitCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -15,9 +15,9 @@ * * @author adil */ -public class RefreshableSplitFetcher implements SplitFetcher, Runnable { +public class SplitFetcherImp implements SplitFetcher, Runnable { - private static final Logger _log = LoggerFactory.getLogger(RefreshableSplitFetcher.class); + private static final Logger _log = LoggerFactory.getLogger(SplitFetcherImp.class); private final SplitParser _parser; private final SplitChangeFetcher _splitChangeFetcher; @@ -35,7 +35,7 @@ public class RefreshableSplitFetcher implements SplitFetcher, Runnable { * an ARCHIVED split is received, we know if we need to remove a traffic type from the multiset. */ - public RefreshableSplitFetcher(SplitChangeFetcher splitChangeFetcher, SplitParser parser, SDKReadinessGates gates, SplitCache splitCache) { + public SplitFetcherImp(SplitChangeFetcher splitChangeFetcher, SplitParser parser, SDKReadinessGates gates, SplitCache splitCache) { _splitChangeFetcher = checkNotNull(splitChangeFetcher); _parser = checkNotNull(parser); _gates = checkNotNull(gates); diff --git a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java b/client/src/main/java/io/split/engine/experiments/SplitSynchronizationTask.java similarity index 88% rename from client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java rename to client/src/main/java/io/split/engine/experiments/SplitSynchronizationTask.java index 9e60cdd31..3e968c74a 100644 --- a/client/src/main/java/io/split/engine/experiments/RefreshableSplitFetcherTask.java +++ b/client/src/main/java/io/split/engine/experiments/SplitSynchronizationTask.java @@ -1,7 +1,7 @@ package io.split.engine.experiments; import com.google.common.util.concurrent.ThreadFactoryBuilder; -import io.split.engine.cache.SplitCache; +import io.split.cache.SplitCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -24,10 +24,10 @@ * * @author adil */ -public class RefreshableSplitFetcherTask implements Closeable { - private static final Logger _log = LoggerFactory.getLogger(RefreshableSplitFetcherTask.class); +public class SplitSynchronizationTask implements Closeable { + private static final Logger _log = LoggerFactory.getLogger(SplitSynchronizationTask.class); - private final AtomicReference _splitFetcher = new AtomicReference(); + private final AtomicReference _splitFetcher = new AtomicReference(); private final AtomicReference _splitCache = new AtomicReference(); private final AtomicReference _executorService = new AtomicReference<>(); private final AtomicLong _refreshEveryNSeconds; @@ -36,7 +36,7 @@ public class RefreshableSplitFetcherTask implements Closeable { private ScheduledFuture _scheduledFuture; - public RefreshableSplitFetcherTask(RefreshableSplitFetcher splitFetcher, SplitCache splitCache, long refreshEveryNSeconds) { + public SplitSynchronizationTask(SplitFetcherImp splitFetcher, SplitCache splitCache, long refreshEveryNSeconds) { _splitFetcher.set(checkNotNull(splitFetcher)); _splitCache.set(checkNotNull(splitCache)); checkArgument(refreshEveryNSeconds >= 0L); diff --git a/client/src/test/java/io/split/engine/cache/InMemoryCacheTest.java b/client/src/test/java/io/split/cache/InMemoryCacheTest.java similarity index 99% rename from client/src/test/java/io/split/engine/cache/InMemoryCacheTest.java rename to client/src/test/java/io/split/cache/InMemoryCacheTest.java index 1a3b627c0..23ea022f3 100644 --- a/client/src/test/java/io/split/engine/cache/InMemoryCacheTest.java +++ b/client/src/test/java/io/split/cache/InMemoryCacheTest.java @@ -1,4 +1,4 @@ -package io.split.engine.cache; +package io.split.cache; import io.split.engine.experiments.ParsedSplit; import org.junit.Assert; diff --git a/client/src/test/java/io/split/client/SplitClientImplTest.java b/client/src/test/java/io/split/client/SplitClientImplTest.java index 4dffea065..c43f4b33b 100644 --- a/client/src/test/java/io/split/client/SplitClientImplTest.java +++ b/client/src/test/java/io/split/client/SplitClientImplTest.java @@ -10,14 +10,12 @@ import io.split.client.dtos.Partition; import io.split.client.impressions.Impression; import io.split.client.impressions.ImpressionsManager; -import io.split.engine.cache.InMemoryCacheImp; -import io.split.engine.cache.SplitCache; -import io.split.engine.evaluator.Evaluator; +import io.split.cache.InMemoryCacheImp; +import io.split.cache.SplitCache; import io.split.engine.evaluator.EvaluatorImp; import io.split.engine.SDKReadinessGates; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; -import io.split.engine.experiments.SplitFetcher; import io.split.engine.matchers.AllKeysMatcher; import io.split.engine.matchers.CombiningMatcher; import io.split.engine.matchers.DependencyMatcher; diff --git a/client/src/test/java/io/split/client/SplitManagerImplTest.java b/client/src/test/java/io/split/client/SplitManagerImplTest.java index 0ff31ebb7..7ef068ac9 100644 --- a/client/src/test/java/io/split/client/SplitManagerImplTest.java +++ b/client/src/test/java/io/split/client/SplitManagerImplTest.java @@ -2,13 +2,11 @@ import com.google.common.collect.Lists; import io.split.client.api.SplitView; -import io.split.client.dtos.Split; import io.split.engine.ConditionsTestUtil; import io.split.engine.SDKReadinessGates; -import io.split.engine.cache.SplitCache; +import io.split.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; -import io.split.engine.experiments.SplitFetcher; import io.split.engine.matchers.AllKeysMatcher; import io.split.engine.matchers.CombiningMatcher; import io.split.grammar.Treatments; diff --git a/client/src/test/java/io/split/engine/common/SynchronizerTest.java b/client/src/test/java/io/split/engine/common/SynchronizerTest.java index 67ab63f53..9734f9095 100644 --- a/client/src/test/java/io/split/engine/common/SynchronizerTest.java +++ b/client/src/test/java/io/split/engine/common/SynchronizerTest.java @@ -1,25 +1,25 @@ package io.split.engine.common; -import io.split.engine.cache.SplitCache; -import io.split.engine.experiments.RefreshableSplitFetcher; -import io.split.engine.experiments.RefreshableSplitFetcherTask; +import io.split.cache.SplitCache; +import io.split.engine.experiments.SplitFetcherImp; +import io.split.engine.experiments.SplitSynchronizationTask; import io.split.engine.segments.RefreshableSegmentFetcher; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; public class SynchronizerTest { - private RefreshableSplitFetcherTask _refreshableSplitFetcherTask; + private SplitSynchronizationTask _refreshableSplitFetcherTask; private RefreshableSegmentFetcher _segmentFetcher; - private RefreshableSplitFetcher _splitFetcher; + private SplitFetcherImp _splitFetcher; private SplitCache _splitCache; private Synchronizer _synchronizer; @Before public void beforeMethod() { - _refreshableSplitFetcherTask = Mockito.mock(RefreshableSplitFetcherTask.class); + _refreshableSplitFetcherTask = Mockito.mock(SplitSynchronizationTask.class); _segmentFetcher = Mockito.mock(RefreshableSegmentFetcher.class); - _splitFetcher = Mockito.mock(RefreshableSplitFetcher.class); + _splitFetcher = Mockito.mock(SplitFetcherImp.class); _splitCache = Mockito.mock(SplitCache.class); _synchronizer = new SynchronizerImp(_refreshableSplitFetcherTask, _splitFetcher, _segmentFetcher, _splitCache); diff --git a/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java b/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java index c8b0b7229..10d4cf038 100644 --- a/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java +++ b/client/src/test/java/io/split/engine/evaluator/EvaluatorTest.java @@ -3,7 +3,7 @@ import io.split.client.dtos.ConditionType; import io.split.client.dtos.Partition; import io.split.engine.SDKReadinessGates; -import io.split.engine.cache.SplitCache; +import io.split.cache.SplitCache; import io.split.engine.experiments.ParsedCondition; import io.split.engine.experiments.ParsedSplit; import io.split.engine.matchers.CombiningMatcher; diff --git a/client/src/test/java/io/split/engine/experiments/RefreshableSplitFetcherTest.java b/client/src/test/java/io/split/engine/experiments/RefreshableSplitFetcherTest.java index a29f753e2..a1b4c9c6c 100644 --- a/client/src/test/java/io/split/engine/experiments/RefreshableSplitFetcherTest.java +++ b/client/src/test/java/io/split/engine/experiments/RefreshableSplitFetcherTest.java @@ -9,8 +9,8 @@ import io.split.client.dtos.Status; import io.split.engine.ConditionsTestUtil; import io.split.engine.SDKReadinessGates; -import io.split.engine.cache.InMemoryCacheImp; -import io.split.engine.cache.SplitCache; +import io.split.cache.InMemoryCacheImp; +import io.split.cache.SplitCache; import io.split.engine.matchers.AllKeysMatcher; import io.split.engine.matchers.CombiningMatcher; import io.split.engine.segments.NoChangeSegmentChangeFetcher; @@ -61,7 +61,7 @@ private void works(long startingChangeNumber) throws InterruptedException { SDKReadinessGates gates = new SDKReadinessGates(); SplitCache cache = new InMemoryCacheImp(startingChangeNumber); - RefreshableSplitFetcher fetcher = new RefreshableSplitFetcher(splitChangeFetcher, new SplitParser(segmentFetcher), gates, cache); + SplitFetcherImp fetcher = new SplitFetcherImp(splitChangeFetcher, new SplitParser(segmentFetcher), gates, cache); // execute the fetcher for a little bit. executeWaitAndTerminate(fetcher, 1, 3, TimeUnit.SECONDS); @@ -131,7 +131,7 @@ public void when_parser_fails_we_remove_the_experiment() throws InterruptedExcep when(splitChangeFetcher.fetch(1L)).thenReturn(noReturn); SplitCache cache = new InMemoryCacheImp(-1); - RefreshableSplitFetcher fetcher = new RefreshableSplitFetcher(splitChangeFetcher, new SplitParser(segmentFetcher), new SDKReadinessGates(), cache); + SplitFetcherImp fetcher = new SplitFetcherImp(splitChangeFetcher, new SplitParser(segmentFetcher), new SDKReadinessGates(), cache); // execute the fetcher for a little bit. executeWaitAndTerminate(fetcher, 1, 5, TimeUnit.SECONDS); @@ -150,7 +150,7 @@ public void if_there_is_a_problem_talking_to_split_change_count_down_latch_is_no SplitChangeFetcher splitChangeFetcher = mock(SplitChangeFetcher.class); when(splitChangeFetcher.fetch(-1L)).thenThrow(new RuntimeException()); - RefreshableSplitFetcher fetcher = new RefreshableSplitFetcher(splitChangeFetcher, new SplitParser(segmentFetcher), gates, cache); + SplitFetcherImp fetcher = new SplitFetcherImp(splitChangeFetcher, new SplitParser(segmentFetcher), gates, cache); // execute the fetcher for a little bit. executeWaitAndTerminate(fetcher, 1, 5, TimeUnit.SECONDS); @@ -189,7 +189,7 @@ public void works_with_user_defined_segments() throws Exception { SegmentChangeFetcher segmentChangeFetcher = new NoChangeSegmentChangeFetcher(); SegmentFetcher segmentFetcher = new RefreshableSegmentFetcher(segmentChangeFetcher, 1,10, gates); segmentFetcher.startPeriodicFetching(); - RefreshableSplitFetcher fetcher = new RefreshableSplitFetcher(experimentChangeFetcher, new SplitParser(segmentFetcher), gates, cache); + SplitFetcherImp fetcher = new SplitFetcherImp(experimentChangeFetcher, new SplitParser(segmentFetcher), gates, cache); // execute the fetcher for a little bit. executeWaitAndTerminate(fetcher, 1, 5, TimeUnit.SECONDS); diff --git a/client/src/test/resources/log4j.properties b/client/src/test/resources/log4j.properties index 0acac71c6..27f2cc155 100644 --- a/client/src/test/resources/log4j.properties +++ b/client/src/test/resources/log4j.properties @@ -14,4 +14,4 @@ log4j.logger.io.split.engine.SDKReadinessGates=WARN #log4j.logger.split.org.apache.http=info #log4j.logger.org.apache.http=info #log4j.logger.split.org.apache.http.wire=info -#log4j.logger.io.split.engine.experiments.RefreshableSplitFetcher=debug \ No newline at end of file +#log4j.logger.io.split.engine.experiments.SplitFetcherImp=debug \ No newline at end of file From e30b7b0078736239901bdfd0214e961dd16cdc1d Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Wed, 16 Dec 2020 18:56:05 -0300 Subject: [PATCH 6/6] pr feedback --- .../src/main/java/io/split/client/SplitManagerImpl.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/client/src/main/java/io/split/client/SplitManagerImpl.java b/client/src/main/java/io/split/client/SplitManagerImpl.java index 32128efe8..e87c422cc 100644 --- a/client/src/main/java/io/split/client/SplitManagerImpl.java +++ b/client/src/main/java/io/split/client/SplitManagerImpl.java @@ -10,7 +10,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + import java.util.concurrent.TimeoutException; /**