From f7ba863dadf9eddf142b42fdbed2e532f1fb9049 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2020 17:18:19 -0800 Subject: [PATCH 1/9] implement new DataStore API and fix cache stats logic --- .../com/launchdarkly/client/Components.java | 31 +++ .../client/FeatureStoreCacheConfig.java | 19 ++ .../client/RedisFeatureStoreBuilder.java | 7 + .../client/integrations/CacheMonitor.java | 146 ++++++++++++ .../PersistentDataStoreBuilder.java | 216 ++++++++++++++++++ .../integrations/RedisDataStoreBuilder.java | 26 ++- .../PersistentDataStoreFactory.java | 53 +++++ .../client/utils/CachingStoreWrapper.java | 94 +++++--- .../DeprecatedRedisFeatureStoreTest.java | 36 +++ .../client/utils/CachingStoreWrapperTest.java | 54 ++++- 10 files changed, 647 insertions(+), 35 deletions(-) create mode 100644 src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java create mode 100644 src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java create mode 100644 src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java diff --git a/src/main/java/com/launchdarkly/client/Components.java b/src/main/java/com/launchdarkly/client/Components.java index fb48bc1dd..fbc03a0fb 100644 --- a/src/main/java/com/launchdarkly/client/Components.java +++ b/src/main/java/com/launchdarkly/client/Components.java @@ -1,5 +1,8 @@ package com.launchdarkly.client; +import com.launchdarkly.client.integrations.PersistentDataStoreBuilder; +import com.launchdarkly.client.interfaces.PersistentDataStoreFactory; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,6 +32,34 @@ public abstract class Components { public static FeatureStoreFactory inMemoryDataStore() { return inMemoryFeatureStoreFactory; } + + /** + * Returns a configurable factory for some implementation of a persistent data store. + *

+ * This method is used in conjunction with another factory object provided by specific components + * such as the Redis integration. The latter provides builder methods for options that are specific + * to that integration, while the {@link PersistentDataStoreBuilder} provides options like + * that are + * applicable to any persistent data store (such as caching). For example: + * + * 


+   *     LDConfig config = new LDConfig.Builder()
+   *         .dataStore(
+   *             Components.persistentDataStore(
+   *                 Redis.dataStore().url("redis://my-redis-host")
+   *             ).ttlSeconds(15)
+   *         )
+   *         .build();
+   *
+ * + * See {@link PersistentDataStoreBuilder} for more on how this method is used. + * + * @param storeFactory the factory/builder for the specific kind of persistent data store + * @return a {@link PersistentDataStoreBuilder} + */ + public static PersistentDataStoreBuilder persistentDataStore(PersistentDataStoreFactory storeFactory) { + return new PersistentDataStoreBuilder(storeFactory); + } /** * Deprecated name for {@link #inMemoryDataStore()}. diff --git a/src/main/java/com/launchdarkly/client/FeatureStoreCacheConfig.java b/src/main/java/com/launchdarkly/client/FeatureStoreCacheConfig.java index 9cfedf383..65451b712 100644 --- a/src/main/java/com/launchdarkly/client/FeatureStoreCacheConfig.java +++ b/src/main/java/com/launchdarkly/client/FeatureStoreCacheConfig.java @@ -1,6 +1,7 @@ package com.launchdarkly.client; import com.google.common.cache.CacheBuilder; +import com.launchdarkly.client.integrations.PersistentDataStoreBuilder; import java.util.Objects; import java.util.concurrent.TimeUnit; @@ -25,7 +26,9 @@ * * @see RedisFeatureStoreBuilder#caching(FeatureStoreCacheConfig) * @since 4.6.0 + * @deprecated This has been superseded by the {@link PersistentDataStoreBuilder} interface. */ +@Deprecated public final class FeatureStoreCacheConfig { /** * The default TTL, in seconds, used by {@link #DEFAULT}. @@ -236,6 +239,22 @@ public FeatureStoreCacheConfig staleValuesPolicy(StaleValuesPolicy policy) { return new FeatureStoreCacheConfig(cacheTime, cacheTimeUnit, policy); } + /** + * Used internally for backward compatibility from the newer builder API. + * + * @param policy a {@link com.launchdarkly.client.integrations.PersistentDataStoreBuilder.StaleValuesPolicy} constant + * @return an updated parameters object + */ + public FeatureStoreCacheConfig staleValuesPolicy(PersistentDataStoreBuilder.StaleValuesPolicy policy) { + switch (policy) { + case REFRESH: + return staleValuesPolicy(StaleValuesPolicy.REFRESH); + case REFRESH_ASYNC: + return staleValuesPolicy(StaleValuesPolicy.REFRESH_ASYNC); + default: + return staleValuesPolicy(StaleValuesPolicy.EVICT); + } + } @Override public boolean equals(Object other) { if (other instanceof FeatureStoreCacheConfig) { diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java index d84e3aa58..07e8684aa 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java @@ -1,5 +1,6 @@ package com.launchdarkly.client; +import com.launchdarkly.client.integrations.CacheMonitor; import com.launchdarkly.client.integrations.Redis; import com.launchdarkly.client.integrations.RedisDataStoreBuilder; @@ -48,6 +49,12 @@ public final class RedisFeatureStoreBuilder implements FeatureStoreFactory { // These constructors are called only from Components RedisFeatureStoreBuilder() { wrappedBuilder = Redis.dataStore(); + + // In order to make the cacheStats() method on the deprecated RedisFeatureStore class work, we need to + // turn on cache monitoring. In the newer API, cache monitoring would only be turned on if the application + // specified its own CacheMonitor, but in the deprecated API there's no way to know if they will want the + // statistics or not. + wrappedBuilder.cacheMonitor(new CacheMonitor()); } RedisFeatureStoreBuilder(URI uri) { diff --git a/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java b/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java new file mode 100644 index 000000000..67e8e7d4b --- /dev/null +++ b/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java @@ -0,0 +1,146 @@ +package com.launchdarkly.client.integrations; + +import java.util.Objects; +import java.util.concurrent.Callable; + +/** + * A conduit that an application can use to monitor caching behavior of a persistent data store. + *

+ * See {@link PersistentDataStoreBuilder#cacheMonitor(CacheMonitor)} + * @since 4.11.0 + */ +public final class CacheMonitor { + private Callable source; + + /** + * Constructs a new instance. + */ + public CacheMonitor() {} + + /** + * Called internally by the SDK to establish a source for the statistics. + * @param source provided by an internal SDK component + */ + public void setSource(Callable source) { + this.source = source; + } + + /** + * Queries the current cache statistics. + * + * @return a {@link CacheStats} instance, or null if not available + */ + public CacheStats getCacheStats() { + try { + return source == null ? null : source.call(); + } catch (Exception e) { + return null; + } + } + + /** + * A snapshot of cache statistics. The statistics are cumulative across the lifetime of the data store. + *

+ * This is based on the data provided by Guava's caching framework. The SDK currently uses Guava + * internally, but is not guaranteed to always do so, and to avoid embedding Guava API details in + * the SDK API this is provided as a separate class. + */ + public static final class CacheStats { + private final long hitCount; + private final long missCount; + private final long loadSuccessCount; + private final long loadExceptionCount; + private final long totalLoadTime; + private final long evictionCount; + + /** + * Constructs a new instance. + * + * @param hitCount + * @param missCount + * @param loadSuccessCount + * @param loadExceptionCount + * @param totalLoadTime + * @param evictionCount + */ + public CacheStats(long hitCount, long missCount, long loadSuccessCount, long loadExceptionCount, + long totalLoadTime, long evictionCount) { + this.hitCount = hitCount; + this.missCount = missCount; + this.loadSuccessCount = loadSuccessCount; + this.loadExceptionCount = loadExceptionCount; + this.totalLoadTime = totalLoadTime; + this.evictionCount = evictionCount; + } + + /** + * The number of data queries that received cached data instead of going to the underlying data store. + * @return the number of cache hits + */ + public long getHitCount() { + return hitCount; + } + + /** + * The number of data queries that did not find cached data and went to the underlying data store. + * @return the number of cache misses + */ + public long getMissCount() { + return missCount; + } + + /** + * The number of times a cache miss resulted in successfully loading a data store item (or finding + * that it did not exist in the store). + * @return the number of successful loads + */ + public long getLoadSuccessCount() { + return loadSuccessCount; + } + + /** + * The number of times that an error occurred while querying the underlying data store. + * @return the number of failed loads + */ + public long getLoadExceptionCount() { + return loadExceptionCount; + } + + /** + * The total number of nanoseconds that the cache has spent loading new values. + * @return total time spent for all cache loads + */ + public long getTotalLoadTime() { + return totalLoadTime; + } + + /** + * The number of times cache entries have been evicted. + * @return the number of evictions + */ + public long getEvictionCount() { + return evictionCount; + } + + @Override + public boolean equals(Object other) { + if (!(other instanceof CacheStats)) { + return false; + } + CacheStats o = (CacheStats)other; + return hitCount == o.hitCount && missCount == o.missCount && loadSuccessCount == o.loadSuccessCount && + loadExceptionCount == o.loadExceptionCount && totalLoadTime == o.totalLoadTime && evictionCount == o.evictionCount; + } + + @Override + public int hashCode() { + return Objects.hash(hitCount, missCount, loadSuccessCount, loadExceptionCount, totalLoadTime, evictionCount); + } + + @Override + public String toString() { + return "{hit=" + hitCount + ", miss=" + missCount + ", loadSuccess=" + loadSuccessCount + + ", loadException=" + loadExceptionCount + ", totalLoadTime=" + totalLoadTime + ", evictionCount=" + evictionCount + "}"; + } + } +} diff --git a/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java b/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java new file mode 100644 index 000000000..6f69ac4a5 --- /dev/null +++ b/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java @@ -0,0 +1,216 @@ +package com.launchdarkly.client.integrations; + +import com.launchdarkly.client.Components; +import com.launchdarkly.client.FeatureStore; +import com.launchdarkly.client.FeatureStoreFactory; +import com.launchdarkly.client.interfaces.PersistentDataStoreFactory; + +import java.util.concurrent.TimeUnit; + +/** + * A configurable factory for a persistent data store. + *

+ * Several database integrations exist for the LaunchDarkly SDK, each with its own behavior and options + * specific to that database; this is described via some implementation of {@link PersistentDataStoreFactory}. + * There is also universal behavior that the SDK provides for all persistent data stores, such as caching; + * the {@link PersistentDataStoreBuilder} adds this. + *

+ * An example of using + *

+ * options that are specific to that implementation; the builder implements {@link PersistentDataStoreFactory}. + * Then call {@link Components#persistentDataStore(PersistentDataStoreFactory)} to wrap that object in the + * standard SDK persistent data store behavior, in the form of a {@link PersistentDataStoreBuilder} + * which can be configured with caching options. Finally, pass this to + * {@link com.launchdarkly.client.LDConfig.Builder#dataStore(FeatureStoreFactory)}. + * For example, using the Redis integration: + * + * 


+   *     LDConfig config = new LDConfig.Builder()
+   *         .dataStore(
+   *             Components.persistentDataStore(
+   *                 Redis.dataStore().url("redis://my-redis-host")
+   *             ).cacheSeconds(15)
+   *         )
+   *         .build();
+   *
+ * + * In this example, {@code .url()} is an option specifically for the Redis integration, whereas + * {@code ttlSeconds()} is an option that can be used for any persistent data store. + * + * @since 4.11.0 + */ +public final class PersistentDataStoreBuilder implements FeatureStoreFactory { + /** + * The default value for the cache TTL. + */ + public static final int DEFAULT_CACHE_TTL_SECONDS = 15; + + private final PersistentDataStoreFactory persistentDataStoreFactory; + + /** + * Possible values for {@link #staleValuesPolicy(StaleValuesPolicy)}. + */ + public enum StaleValuesPolicy { + /** + * Indicates that when the cache TTL expires for an item, it is evicted from the cache. The next + * attempt to read that item causes a synchronous read from the underlying data store; if that + * fails, no value is available. This is the default behavior. + * + * @see com.google.common.cache.CacheBuilder#expireAfterWrite(long, TimeUnit) + */ + EVICT, + /** + * Indicates that the cache should refresh stale values instead of evicting them. + *

+ * In this mode, an attempt to read an expired item causes a synchronous read from the underlying + * data store, like {@link #EVICT}--but if an error occurs during this refresh, the cache will + * continue to return the previously cached values (if any). This is useful if you prefer the most + * recently cached feature rule set to be returned for evaluation over the default value when + * updates go wrong. + *

+ * See: CacheBuilder + * for more specific information on cache semantics. This mode is equivalent to {@code expireAfterWrite}. + */ + REFRESH, + /** + * Indicates that the cache should refresh stale values asynchronously instead of evicting them. + *

+ * This is the same as {@link #REFRESH}, except that the attempt to refresh the value is done + * on another thread (using a {@link java.util.concurrent.Executor}). In the meantime, the cache + * will continue to return the previously cached value (if any) in a non-blocking fashion to threads + * requesting the stale key. Any exception encountered during the asynchronous reload will cause + * the previously cached value to be retained. + *

+ * This setting is ideal to enable when you desire high performance reads and can accept returning + * stale values for the period of the async refresh. For example, configuring this feature store + * with a very low cache time and enabling this feature would see great performance benefit by + * decoupling calls from network I/O. + *

+ * See: CacheBuilder for + * more specific information on cache semantics. + */ + REFRESH_ASYNC + }; + + /** + * Creates a new builder. + * + * @param persistentDataStoreFactory the factory implementation for the specific data store type + */ + public PersistentDataStoreBuilder(PersistentDataStoreFactory persistentDataStoreFactory) { + this.persistentDataStoreFactory = persistentDataStoreFactory; + } + + @Override + public FeatureStore createFeatureStore() { + return persistentDataStoreFactory.createFeatureStore(); + } + + /** + * Specifies that the SDK should not use an in-memory cache for the persistent data store. + * This means that every feature flag evaluation will trigger a data store query. + * + * @return the builder + */ + public PersistentDataStoreBuilder noCaching() { + return cacheTime(0, TimeUnit.MILLISECONDS); + } + + /** + * Specifies the cache TTL. Items will be evicted or refreshed (depending on the StaleValuesPolicy) + * after this amount of time from the time when they were originally cached. + *

+ * If the value is zero, caching is disabled (equivalent to {@link #noCaching()}). + *

+ * If the value is negative, data is cached forever (equivalent to {@link #cacheForever()}). + * + * @param cacheTime the cache TTL in whatever units you wish + * @param cacheTimeUnit the time unit + * @return the builder + */ + @SuppressWarnings("deprecation") + public PersistentDataStoreBuilder cacheTime(long cacheTime, TimeUnit cacheTimeUnit) { + persistentDataStoreFactory.cacheTime(cacheTime, cacheTimeUnit); + return this; + } + + /** + * Shortcut for calling {@link #cacheTime(long, TimeUnit)} with {@link TimeUnit#MILLISECONDS}. + * + * @param millis the cache TTL in milliseconds + * @return the builder + */ + public PersistentDataStoreBuilder cacheTtlMillis(long millis) { + return cacheTime(millis, TimeUnit.MILLISECONDS); + } + + /** + * Shortcut for calling {@link #cacheTime(long, TimeUnit)} with {@link TimeUnit#SECONDS}. + * + * @param seconds the cache TTL in seconds + * @return the builder + */ + public PersistentDataStoreBuilder cacheTtlSeconds(long seconds) { + return cacheTime(seconds, TimeUnit.SECONDS); + } + + /** + * Specifies that the in-memory cache should never expire. In this mode, data will be written + * to both the underlying persistent store and the cache, but will only ever be read from + * persistent store if the SDK is restarted. + *

+ * Use this mode with caution: it means that in a scenario where multiple processes are sharing + * the database, and the current process loses connectivity to LaunchDarkly while other processes + * are still receiving updates and writing them to the database, the current process will have + * stale data. + * + * @return the builder + */ + public PersistentDataStoreBuilder cacheForever() { + return cacheTime(-1, TimeUnit.MILLISECONDS); + } + + /** + * Specifies how the cache (if any) should deal with old values when the cache TTL expires. The default + * is {@link StaleValuesPolicy#EVICT}. This property has no effect if caching is disabled. + * + * @param staleValuesPolicy a {@link StaleValuesPolicy} constant + * @return the builder + */ + @SuppressWarnings("deprecation") + public PersistentDataStoreBuilder staleValuesPolicy(StaleValuesPolicy staleValuesPolicy) { + persistentDataStoreFactory.staleValuesPolicy(staleValuesPolicy); + return this; + } + + /** + * Provides a conduit for an application to monitor the effectiveness of the in-memory cache. + *

+ * Create an instance of {@link CacheMonitor}; retain a reference to it, and also pass it to this + * method when you are configuring the persistent data store. The store will use + * {@link CacheMonitor#setSource(java.util.concurrent.Callable)} to make the caching + * statistics available through that {@link CacheMonitor} instance. + *

+ * Note that turning on cache monitoring may slightly decrease performance, due to the need to + * record statistics for each cache operation. + *

+ * Example usage: + * + * 


+   *     CacheMonitor cacheMonitor = new CacheMonitor();
+   *     LDConfig config = new LDConfig.Builder()
+   *         .dataStore(Components.persistentDataStore(Redis.dataStore()).cacheMonitor(cacheMonitor))
+   *         .build();
+   *     // later...
+   *     CacheMonitor.CacheStats stats = cacheMonitor.getCacheStats();
+   *
+ * + * @param cacheMonitor an instance of {@link CacheMonitor} + * @return the builder + */ + @SuppressWarnings("deprecation") + public PersistentDataStoreBuilder cacheMonitor(CacheMonitor cacheMonitor) { + persistentDataStoreFactory.cacheMonitor(cacheMonitor); + return this; + } +} diff --git a/src/main/java/com/launchdarkly/client/integrations/RedisDataStoreBuilder.java b/src/main/java/com/launchdarkly/client/integrations/RedisDataStoreBuilder.java index bbc50491d..605bc4f60 100644 --- a/src/main/java/com/launchdarkly/client/integrations/RedisDataStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/integrations/RedisDataStoreBuilder.java @@ -2,7 +2,8 @@ import com.launchdarkly.client.FeatureStore; import com.launchdarkly.client.FeatureStoreCacheConfig; -import com.launchdarkly.client.FeatureStoreFactory; +import com.launchdarkly.client.integrations.PersistentDataStoreBuilder.StaleValuesPolicy; +import com.launchdarkly.client.interfaces.PersistentDataStoreFactory; import com.launchdarkly.client.utils.CachingStoreWrapper; import java.net.URI; @@ -36,7 +37,8 @@ * * @since 4.11.0 */ -public final class RedisDataStoreBuilder implements FeatureStoreFactory { +@SuppressWarnings("deprecation") +public final class RedisDataStoreBuilder implements PersistentDataStoreFactory { /** * The default value for the Redis URI: {@code redis://localhost:6379} */ @@ -55,6 +57,7 @@ public final class RedisDataStoreBuilder implements FeatureStoreFactory { String password = null; boolean tls = false; FeatureStoreCacheConfig caching = FeatureStoreCacheConfig.DEFAULT; + CacheMonitor cacheMonitor = null; JedisPoolConfig poolConfig = null; // These constructors are called only from Implementations @@ -122,12 +125,29 @@ public RedisDataStoreBuilder uri(URI redisUri) { * * @param caching a {@link FeatureStoreCacheConfig} object specifying caching parameters * @return the builder + * @deprecated This has been superseded by the {@link PersistentDataStoreBuilder} interface. */ + @Deprecated public RedisDataStoreBuilder caching(FeatureStoreCacheConfig caching) { this.caching = caching; return this; } + @Override + public void cacheTime(long cacheTime, TimeUnit cacheTimeUnit) { + this.caching = caching.ttl(cacheTime, cacheTimeUnit); + } + + @Override + public void staleValuesPolicy(StaleValuesPolicy staleValuesPolicy) { + this.caching = caching.staleValuesPolicy(staleValuesPolicy); + } + + @Override + public void cacheMonitor(CacheMonitor cacheMonitor) { + this.cacheMonitor = cacheMonitor; + } + /** * Optionally configures the namespace prefix for all keys stored in Redis. * @@ -182,6 +202,6 @@ public RedisDataStoreBuilder socketTimeout(int socketTimeout, TimeUnit timeUnit) */ public FeatureStore createFeatureStore() { RedisDataStoreImpl core = new RedisDataStoreImpl(this); - return CachingStoreWrapper.builder(core).caching(this.caching).build(); + return CachingStoreWrapper.builder(core).caching(this.caching).cacheMonitor(this.cacheMonitor).build(); } } diff --git a/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java b/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java new file mode 100644 index 000000000..1148a3566 --- /dev/null +++ b/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java @@ -0,0 +1,53 @@ +package com.launchdarkly.client.interfaces; + +import com.launchdarkly.client.FeatureStoreFactory; +import com.launchdarkly.client.integrations.CacheMonitor; +import com.launchdarkly.client.integrations.PersistentDataStoreBuilder; +import com.launchdarkly.client.integrations.PersistentDataStoreBuilder.StaleValuesPolicy; + +import java.util.concurrent.TimeUnit; + +/** + * Interface for a factory that creates some implementation of a persistent data store. + *

+ * Note that currently this interface contains methods that are duplicates of the methods in + * {@link PersistentDataStoreBuilder}. This is necessary to preserve backward compatibility with the + * implementation of persistent data stores in earlier versions of the SDK. The new recommended usage + * is described in {@link com.launchdarkly.client.Components#persistentDataStore}, and starting in + * version 5.0 these redundant methods will be removed. + * + * @see com.launchdarkly.client.Components + * @since 4.11.0 + */ +public interface PersistentDataStoreFactory extends FeatureStoreFactory { + /** + * Called internally from {@link PersistentDataStoreBuilder}. + * + * @param cacheTime the cache TTL in whatever units you wish + * @param cacheTimeUnit the time unit + * @deprecated Calling this method directly on this component is deprecated. See {@link com.launchdarkly.client.Components#persistentDataStore} + * for the new usage. + */ + @Deprecated + void cacheTime(long cacheTime, TimeUnit cacheTimeUnit); + + /** + * Called internally from {@link PersistentDataStoreBuilder}. + * + * @param staleValuesPolicy a {@link StaleValuesPolicy} constant + * @deprecated Calling this method directly on this component is deprecated. See {@link com.launchdarkly.client.Components#persistentDataStore} + * for the new usage. + */ + @Deprecated + void staleValuesPolicy(StaleValuesPolicy staleValuesPolicy); + + /** + * Called internally from {@link PersistentDataStoreBuilder}. + * + * @param cacheMonitor an instance of {@link CacheMonitor} + * @deprecated Calling this method directly on this component is deprecated. See {@link com.launchdarkly.client.Components#persistentDataStore} + * for the new usage. + */ + @Deprecated + void cacheMonitor(CacheMonitor cacheMonitor); +} diff --git a/src/main/java/com/launchdarkly/client/utils/CachingStoreWrapper.java b/src/main/java/com/launchdarkly/client/utils/CachingStoreWrapper.java index 47de79688..8e841a52e 100644 --- a/src/main/java/com/launchdarkly/client/utils/CachingStoreWrapper.java +++ b/src/main/java/com/launchdarkly/client/utils/CachingStoreWrapper.java @@ -13,10 +13,12 @@ import com.launchdarkly.client.FeatureStoreCacheConfig; import com.launchdarkly.client.VersionedData; import com.launchdarkly.client.VersionedDataKind; +import com.launchdarkly.client.integrations.CacheMonitor; import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; @@ -32,6 +34,7 @@ * * @since 4.6.0 */ +@SuppressWarnings("deprecation") public class CachingStoreWrapper implements FeatureStore { private static final String CACHE_REFRESH_THREAD_POOL_NAME_FORMAT = "CachingStoreWrapper-refresher-pool-%d"; @@ -52,7 +55,7 @@ public static CachingStoreWrapper.Builder builder(FeatureStoreCore core) { return new Builder(core); } - protected CachingStoreWrapper(final FeatureStoreCore core, FeatureStoreCacheConfig caching) { + protected CachingStoreWrapper(final FeatureStoreCore core, FeatureStoreCacheConfig caching, CacheMonitor cacheMonitor) { this.core = core; this.caching = caching; @@ -81,40 +84,45 @@ public Boolean load(String key) throws Exception { } }; - if (caching.isInfiniteTtl()) { - itemCache = CacheBuilder.newBuilder().build(itemLoader); - allCache = CacheBuilder.newBuilder().build(allLoader); - executorService = null; - } else if (caching.getStaleValuesPolicy() == FeatureStoreCacheConfig.StaleValuesPolicy.EVICT) { - // We are using an "expire after write" cache. This will evict stale values and block while loading the latest - // from the underlying data store. - - itemCache = CacheBuilder.newBuilder().expireAfterWrite(caching.getCacheTime(), caching.getCacheTimeUnit()).build(itemLoader); - allCache = CacheBuilder.newBuilder().expireAfterWrite(caching.getCacheTime(), caching.getCacheTimeUnit()).build(allLoader); - executorService = null; - } else { - // We are using a "refresh after write" cache. This will not automatically evict stale values, allowing them - // to be returned if failures occur when updating them. Optionally set the cache to refresh values asynchronously, - // which always returns the previously cached value immediately (this is only done for itemCache, not allCache, - // since retrieving all flags is less frequently needed and we don't want to incur the extra overhead). - + if (caching.getStaleValuesPolicy() == FeatureStoreCacheConfig.StaleValuesPolicy.REFRESH_ASYNC) { ThreadFactory threadFactory = new ThreadFactoryBuilder().setNameFormat(CACHE_REFRESH_THREAD_POOL_NAME_FORMAT).setDaemon(true).build(); ExecutorService parentExecutor = Executors.newSingleThreadExecutor(threadFactory); executorService = MoreExecutors.listeningDecorator(parentExecutor); - - if (caching.getStaleValuesPolicy() == FeatureStoreCacheConfig.StaleValuesPolicy.REFRESH_ASYNC) { - itemLoader = CacheLoader.asyncReloading(itemLoader, executorService); - } - itemCache = CacheBuilder.newBuilder().refreshAfterWrite(caching.getCacheTime(), caching.getCacheTimeUnit()).build(itemLoader); - allCache = CacheBuilder.newBuilder().refreshAfterWrite(caching.getCacheTime(), caching.getCacheTimeUnit()).build(allLoader); + + // Note that the REFRESH_ASYNC mode is only used for itemCache, not allCache, since retrieving all flags is + // less frequently needed and we don't want to incur the extra overhead. + itemLoader = CacheLoader.asyncReloading(itemLoader, executorService); + } else { + executorService = null; } - - if (caching.isInfiniteTtl()) { - initCache = CacheBuilder.newBuilder().build(initLoader); + + itemCache = newCacheBuilder(caching, cacheMonitor).build(itemLoader); + allCache = newCacheBuilder(caching, cacheMonitor).build(allLoader); + initCache = newCacheBuilder(caching, cacheMonitor).build(initLoader); + + if (cacheMonitor != null) { + cacheMonitor.setSource(new CacheStatsSource()); + } + } + } + + private static CacheBuilder newCacheBuilder(FeatureStoreCacheConfig caching, CacheMonitor cacheMonitor) { + CacheBuilder builder = CacheBuilder.newBuilder(); + if (!caching.isInfiniteTtl()) { + if (caching.getStaleValuesPolicy() == FeatureStoreCacheConfig.StaleValuesPolicy.EVICT) { + // We are using an "expire after write" cache. This will evict stale values and block while loading the latest + // from the underlying data store. + builder = builder.expireAfterWrite(caching.getCacheTime(), caching.getCacheTimeUnit()); } else { - initCache = CacheBuilder.newBuilder().expireAfterWrite(caching.getCacheTime(), caching.getCacheTimeUnit()).build(initLoader); + // We are using a "refresh after write" cache. This will not automatically evict stale values, allowing them + // to be returned if failures occur when updating them. + builder = builder.refreshAfterWrite(caching.getCacheTime(), caching.getCacheTimeUnit()); } } + if (cacheMonitor != null) { + builder = builder.recordStats(); + } + return builder; } @Override @@ -291,6 +299,23 @@ private ImmutableMap itemsOnlyIfNotDeleted( return builder.build(); } + private final class CacheStatsSource implements Callable { + public CacheMonitor.CacheStats call() { + if (itemCache == null || allCache == null) { + return null; + } + CacheStats itemStats = itemCache.stats(); + CacheStats allStats = allCache.stats(); + return new CacheMonitor.CacheStats( + itemStats.hitCount() + allStats.hitCount(), + itemStats.missCount() + allStats.missCount(), + itemStats.loadSuccessCount() + allStats.loadSuccessCount(), + itemStats.loadExceptionCount() + allStats.loadExceptionCount(), + itemStats.totalLoadTime() + allStats.totalLoadTime(), + itemStats.evictionCount() + allStats.evictionCount()); + } + } + private static class CacheKey { final VersionedDataKind kind; final String key; @@ -326,6 +351,7 @@ public int hashCode() { public static class Builder { private final FeatureStoreCore core; private FeatureStoreCacheConfig caching = FeatureStoreCacheConfig.DEFAULT; + private CacheMonitor cacheMonitor = null; Builder(FeatureStoreCore core) { this.core = core; @@ -341,12 +367,22 @@ public Builder caching(FeatureStoreCacheConfig caching) { return this; } + /** + * Sets the cache monitor instance. + * @param cacheMonitor an instance of {@link CacheMonitor} + * @return the builder + */ + public Builder cacheMonitor(CacheMonitor cacheMonitor) { + this.cacheMonitor = cacheMonitor; + return this; + } + /** * Creates and configures the wrapper object. * @return a {@link CachingStoreWrapper} instance */ public CachingStoreWrapper build() { - return new CachingStoreWrapper(core, caching); + return new CachingStoreWrapper(core, caching, cacheMonitor); } } } diff --git a/src/test/java/com/launchdarkly/client/DeprecatedRedisFeatureStoreTest.java b/src/test/java/com/launchdarkly/client/DeprecatedRedisFeatureStoreTest.java index 08febe5ad..058b252d0 100644 --- a/src/test/java/com/launchdarkly/client/DeprecatedRedisFeatureStoreTest.java +++ b/src/test/java/com/launchdarkly/client/DeprecatedRedisFeatureStoreTest.java @@ -1,9 +1,17 @@ package com.launchdarkly.client; +import com.google.common.cache.CacheStats; + import org.junit.BeforeClass; +import org.junit.Test; import java.net.URI; +import static com.launchdarkly.client.VersionedDataKind.FEATURES; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assume.assumeThat; import static org.junit.Assume.assumeTrue; import redis.clients.jedis.Jedis; @@ -41,4 +49,32 @@ protected void clearAllData() { client.flushDB(); } } + + @Test + public void canGetCacheStats() { + assumeThat(cached, is(true)); + + CacheStats stats = store.getCacheStats(); + + assertThat(stats, equalTo(new CacheStats(0, 0, 0, 0, 0, 0))); + + // Cause a cache miss + store.get(FEATURES, "key1"); + stats = store.getCacheStats(); + assertThat(stats.hitCount(), equalTo(0L)); + assertThat(stats.missCount(), equalTo(1L)); + assertThat(stats.loadSuccessCount(), equalTo(1L)); // even though it's a miss, it's a "success" because there was no exception + assertThat(stats.loadExceptionCount(), equalTo(0L)); + + // Cause a cache hit + store.upsert(FEATURES, new FeatureFlagBuilder("key2").version(1).build()); // inserting the item also caches it + store.get(FEATURES, "key2"); // now it's a cache hit + stats = store.getCacheStats(); + assertThat(stats.hitCount(), equalTo(1L)); + assertThat(stats.missCount(), equalTo(1L)); + assertThat(stats.loadSuccessCount(), equalTo(1L)); + assertThat(stats.loadExceptionCount(), equalTo(0L)); + + // We have no way to force a load exception with a real Redis store + } } diff --git a/src/test/java/com/launchdarkly/client/utils/CachingStoreWrapperTest.java b/src/test/java/com/launchdarkly/client/utils/CachingStoreWrapperTest.java index 6419b6db1..db8e5ceba 100644 --- a/src/test/java/com/launchdarkly/client/utils/CachingStoreWrapperTest.java +++ b/src/test/java/com/launchdarkly/client/utils/CachingStoreWrapperTest.java @@ -4,7 +4,9 @@ import com.launchdarkly.client.FeatureStoreCacheConfig; import com.launchdarkly.client.VersionedData; import com.launchdarkly.client.VersionedDataKind; +import com.launchdarkly.client.integrations.CacheMonitor; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -21,9 +23,10 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.fail; import static org.junit.Assume.assumeThat; -@SuppressWarnings("javadoc") +@SuppressWarnings({ "javadoc", "deprecation" }) @RunWith(Parameterized.class) public class CachingStoreWrapperTest { @@ -62,7 +65,7 @@ public static Iterable data() { public CachingStoreWrapperTest(CachingMode cachingMode) { this.cachingMode = cachingMode; this.core = new MockCore(); - this.wrapper = new CachingStoreWrapper(core, cachingMode.toCacheConfig()); + this.wrapper = new CachingStoreWrapper(core, cachingMode.toCacheConfig(), null); } @Test @@ -388,7 +391,7 @@ public void initializedCanCacheFalseResult() throws Exception { assumeThat(cachingMode.isCached(), is(true)); // We need to create a different object for this test so we can set a short cache TTL - try (CachingStoreWrapper wrapper1 = new CachingStoreWrapper(core, FeatureStoreCacheConfig.enabled().ttlMillis(500))) { + try (CachingStoreWrapper wrapper1 = new CachingStoreWrapper(core, FeatureStoreCacheConfig.enabled().ttlMillis(500), null)) { assertThat(wrapper1.initialized(), is(false)); assertThat(core.initedQueryCount, equalTo(1)); @@ -406,6 +409,51 @@ public void initializedCanCacheFalseResult() throws Exception { } } + @Test + public void canGetCacheStats() throws Exception { + assumeThat(cachingMode, is(CachingMode.CACHED_WITH_FINITE_TTL)); + + CacheMonitor cacheMonitor = new CacheMonitor(); + + try (CachingStoreWrapper w = new CachingStoreWrapper(core, FeatureStoreCacheConfig.enabled().ttlSeconds(30), cacheMonitor)) { + CacheMonitor.CacheStats stats = cacheMonitor.getCacheStats(); + + assertThat(stats, equalTo(new CacheMonitor.CacheStats(0, 0, 0, 0, 0, 0))); + + // Cause a cache miss + w.get(THINGS, "key1"); + stats = cacheMonitor.getCacheStats(); + assertThat(stats.getHitCount(), equalTo(0L)); + assertThat(stats.getMissCount(), equalTo(1L)); + assertThat(stats.getLoadSuccessCount(), equalTo(1L)); // even though it's a miss, it's a "success" because there was no exception + assertThat(stats.getLoadExceptionCount(), equalTo(0L)); + + // Cause a cache hit + core.forceSet(THINGS, new MockItem("key2", 1, false)); + w.get(THINGS, "key2"); // this one is a cache miss, but causes the item to be loaded and cached + w.get(THINGS, "key2"); // now it's a cache hit + stats = cacheMonitor.getCacheStats(); + assertThat(stats.getHitCount(), equalTo(1L)); + assertThat(stats.getMissCount(), equalTo(2L)); + assertThat(stats.getLoadSuccessCount(), equalTo(2L)); + assertThat(stats.getLoadExceptionCount(), equalTo(0L)); + + // Cause a load exception + core.fakeError = new RuntimeException("sorry"); + try { + w.get(THINGS, "key3"); // cache miss -> tries to load the item -> gets an exception + fail("expected exception"); + } catch (RuntimeException e) { + assertThat(e.getCause(), is((Throwable)core.fakeError)); + } + stats = cacheMonitor.getCacheStats(); + assertThat(stats.getHitCount(), equalTo(1L)); + assertThat(stats.getMissCount(), equalTo(3L)); + assertThat(stats.getLoadSuccessCount(), equalTo(2L)); + assertThat(stats.getLoadExceptionCount(), equalTo(1L)); + } + } + private Map, Map> makeData(MockItem... items) { Map innerMap = new HashMap<>(); for (MockItem item: items) { From ce4a764a2e899ccbdb785e245e5587c40f8f4ac6 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2020 17:46:16 -0800 Subject: [PATCH 2/9] misc fixes --- .../com/launchdarkly/client/Components.java | 20 +++++---- .../com/launchdarkly/client/LDConfig.java | 6 +-- .../client/RedisFeatureStore.java | 3 ++ .../client/integrations/CacheMonitor.java | 6 ++- .../PersistentDataStoreBuilder.java | 41 ++++++++----------- .../client/integrations/Redis.java | 14 ++++++- .../client/FeatureStoreCachingTest.java | 2 +- .../client/LDClientEvaluationTest.java | 10 ++--- .../client/LDClientEventTest.java | 4 +- .../client/LDClientLddModeTest.java | 1 + .../client/LDClientOfflineTest.java | 1 + .../com/launchdarkly/client/LDClientTest.java | 2 +- .../RedisFeatureStoreBuilderTest.java | 2 +- .../integrations/RedisFeatureStoreTest.java | 15 +++++-- .../client/utils/CachingStoreWrapperTest.java | 1 - 15 files changed, 75 insertions(+), 53 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/Components.java b/src/main/java/com/launchdarkly/client/Components.java index fbc03a0fb..7049deb9e 100644 --- a/src/main/java/com/launchdarkly/client/Components.java +++ b/src/main/java/com/launchdarkly/client/Components.java @@ -21,13 +21,13 @@ public abstract class Components { /** * Returns a factory for the default in-memory implementation of a data store. - * + *

* Note that the interface is still named {@link FeatureStoreFactory}, but in a future version it * will be renamed to {@code DataStoreFactory}. * * @return a factory object - * @since 4.11.0 * @see LDConfig.Builder#dataStore(FeatureStoreFactory) + * @since 4.11.0 */ public static FeatureStoreFactory inMemoryDataStore() { return inMemoryFeatureStoreFactory; @@ -39,15 +39,14 @@ public static FeatureStoreFactory inMemoryDataStore() { * This method is used in conjunction with another factory object provided by specific components * such as the Redis integration. The latter provides builder methods for options that are specific * to that integration, while the {@link PersistentDataStoreBuilder} provides options like - * that are - * applicable to any persistent data store (such as caching). For example: + * that are applicable to any persistent data store (such as caching). For example: * * 


    *     LDConfig config = new LDConfig.Builder()
    *         .dataStore(
    *             Components.persistentDataStore(
    *                 Redis.dataStore().url("redis://my-redis-host")
-   *             ).ttlSeconds(15)
+   *             ).cacheSeconds(15)
    *         )
    *         .build();
    *
@@ -56,6 +55,9 @@ public static FeatureStoreFactory inMemoryDataStore() { * * @param storeFactory the factory/builder for the specific kind of persistent data store * @return a {@link PersistentDataStoreBuilder} + * @see LDConfig.Builder#dataStore(FeatureStoreFactory) + * @see com.launchdarkly.client.integrations.Redis + * @since 4.11.0 */ public static PersistentDataStoreBuilder persistentDataStore(PersistentDataStoreFactory storeFactory) { return new PersistentDataStoreBuilder(storeFactory); @@ -74,7 +76,8 @@ public static FeatureStoreFactory inMemoryFeatureStore() { /** * Deprecated name for {@link com.launchdarkly.client.integrations.Redis#dataStore()}. * @return a factory/builder object - * @deprecated Use {@link com.launchdarkly.client.integrations.Redis#dataStore()}. + * @deprecated Use {@link #persistentDataStore(PersistentDataStoreFactory)} with + * {@link com.launchdarkly.client.integrations.Redis#dataStore()}. */ @Deprecated public static RedisFeatureStoreBuilder redisFeatureStore() { @@ -85,8 +88,9 @@ public static RedisFeatureStoreBuilder redisFeatureStore() { * Deprecated name for {@link com.launchdarkly.client.integrations.Redis#dataStore()}. * @param redisUri the URI of the Redis host * @return a factory/builder object - * @deprecated Use {@link com.launchdarkly.client.integrations.Redis#dataStore()} and - * {@link com.launchdarkly.client.integrations.RedisDataStoreBuilder#uri(URI)}. + * @deprecated Use {@link #persistentDataStore(PersistentDataStoreFactory)} with + * {@link com.launchdarkly.client.integrations.Redis#dataStore()} and + * {@link com.launchdarkly.client.integrations.RedisDataStoreBuilder#uri(URI)}. */ @Deprecated public static RedisFeatureStoreBuilder redisFeatureStore(URI redisUri) { diff --git a/src/main/java/com/launchdarkly/client/LDConfig.java b/src/main/java/com/launchdarkly/client/LDConfig.java index 00db0bbe0..bc73cfb14 100644 --- a/src/main/java/com/launchdarkly/client/LDConfig.java +++ b/src/main/java/com/launchdarkly/client/LDConfig.java @@ -210,9 +210,9 @@ public Builder streamURI(URI streamURI) { /** * Sets the implementation of the data store to be used for holding feature flags and * related data received from LaunchDarkly, using a factory object. The default is - * {@link Components#inMemoryDataStore()}, but you may use {@link Components#redisFeatureStore()} - * or a custom implementation. - * + * {@link Components#inMemoryDataStore()}; for database integrations, use + * {@link Components#persistentDataStore(com.launchdarkly.client.interfaces.PersistentDataStoreFactory)}. + *

* Note that the interface is still called {@link FeatureStoreFactory}, but in a future version * it will be renamed to {@code DataStoreFactory}. * diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java index 9713704a7..955cb2bab 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java @@ -57,6 +57,9 @@ public void close() throws IOException { /** * Return the underlying Guava cache stats object. + *

+ * In the newer data store API, there is a different way to do this. See + * {@link com.launchdarkly.client.integrations.PersistentDataStoreBuilder#cacheMonitor(com.launchdarkly.client.integrations.CacheMonitor)}. * * @return the cache statistics object. */ diff --git a/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java b/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java index 67e8e7d4b..b1e8bd6e6 100644 --- a/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java +++ b/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java @@ -5,8 +5,8 @@ /** * A conduit that an application can use to monitor caching behavior of a persistent data store. - *

- * See {@link PersistentDataStoreBuilder#cacheMonitor(CacheMonitor)} + * + * @see PersistentDataStoreBuilder#cacheMonitor(CacheMonitor) * @since 4.11.0 */ public final class CacheMonitor { @@ -44,6 +44,8 @@ public CacheStats getCacheStats() { * This is based on the data provided by Guava's caching framework. The SDK currently uses Guava * internally, but is not guaranteed to always do so, and to avoid embedding Guava API details in * the SDK API this is provided as a separate class. + * + * @since 4.11.0 */ public static final class CacheStats { private final long hitCount; diff --git a/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java b/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java index 6f69ac4a5..d2462228e 100644 --- a/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java @@ -1,6 +1,5 @@ package com.launchdarkly.client.integrations; -import com.launchdarkly.client.Components; import com.launchdarkly.client.FeatureStore; import com.launchdarkly.client.FeatureStoreFactory; import com.launchdarkly.client.interfaces.PersistentDataStoreFactory; @@ -15,27 +14,21 @@ * There is also universal behavior that the SDK provides for all persistent data stores, such as caching; * the {@link PersistentDataStoreBuilder} adds this. *

- * An example of using - *

- * options that are specific to that implementation; the builder implements {@link PersistentDataStoreFactory}. - * Then call {@link Components#persistentDataStore(PersistentDataStoreFactory)} to wrap that object in the - * standard SDK persistent data store behavior, in the form of a {@link PersistentDataStoreBuilder} - * which can be configured with caching options. Finally, pass this to - * {@link com.launchdarkly.client.LDConfig.Builder#dataStore(FeatureStoreFactory)}. - * For example, using the Redis integration: - * - * 


-   *     LDConfig config = new LDConfig.Builder()
-   *         .dataStore(
-   *             Components.persistentDataStore(
-   *                 Redis.dataStore().url("redis://my-redis-host")
-   *             ).cacheSeconds(15)
-   *         )
-   *         .build();
-   *
- * - * In this example, {@code .url()} is an option specifically for the Redis integration, whereas - * {@code ttlSeconds()} is an option that can be used for any persistent data store. + * After configuring this object, pass it to {@link com.launchdarkly.client.LDConfig.Builder#dataStore(FeatureStoreFactory)} + * to use it in the SDK configuration. For example, using the Redis integration: + * + * 

+ *     LDConfig config = new LDConfig.Builder()
+ *         .dataStore(
+ *             Components.persistentDataStore(
+ *                 Redis.dataStore().url("redis://my-redis-host")
+ *             ).cacheSeconds(15)
+ *         )
+ *         .build();
+ *
+ * + * In this example, {@code .url()} is an option specifically for the Redis integration, whereas + * {@code ttlSeconds()} is an option that can be used for any persistent data store. * * @since 4.11.0 */ @@ -140,7 +133,7 @@ public PersistentDataStoreBuilder cacheTime(long cacheTime, TimeUnit cacheTimeUn * @param millis the cache TTL in milliseconds * @return the builder */ - public PersistentDataStoreBuilder cacheTtlMillis(long millis) { + public PersistentDataStoreBuilder cacheMillis(long millis) { return cacheTime(millis, TimeUnit.MILLISECONDS); } @@ -150,7 +143,7 @@ public PersistentDataStoreBuilder cacheTtlMillis(long millis) { * @param seconds the cache TTL in seconds * @return the builder */ - public PersistentDataStoreBuilder cacheTtlSeconds(long seconds) { + public PersistentDataStoreBuilder cacheSeconds(long seconds) { return cacheTime(seconds, TimeUnit.SECONDS); } diff --git a/src/main/java/com/launchdarkly/client/integrations/Redis.java b/src/main/java/com/launchdarkly/client/integrations/Redis.java index 050b581bb..7a167ae9d 100644 --- a/src/main/java/com/launchdarkly/client/integrations/Redis.java +++ b/src/main/java/com/launchdarkly/client/integrations/Redis.java @@ -10,8 +10,20 @@ public abstract class Redis { * Returns a builder object for creating a Redis-backed data store. *

* This object can be modified with {@link RedisDataStoreBuilder} methods for any desired - * custom settings, before including it in the SDK configuration with + * custom Redis options. Then, pass it to {@link com.launchdarkly.client.Components#persistentDataStore(com.launchdarkly.client.interfaces.PersistentDataStoreFactory)} + * and set any desired caching options. Finally, pass the result to * {@link com.launchdarkly.client.LDConfig.Builder#dataStore(com.launchdarkly.client.FeatureStoreFactory)}. + * For example: + * + * 


+   *     LDConfig config = new LDConfig.Builder()
+   *         .dataStore(
+   *             Components.persistentDataStore(
+   *                 Redis.dataStore().url("redis://my-redis-host")
+   *             ).cacheSeconds(15)
+   *         )
+   *         .build();
+   *
* * @return a data store configuration object */ diff --git a/src/test/java/com/launchdarkly/client/FeatureStoreCachingTest.java b/src/test/java/com/launchdarkly/client/FeatureStoreCachingTest.java index c9259b622..2d90cd4a6 100644 --- a/src/test/java/com/launchdarkly/client/FeatureStoreCachingTest.java +++ b/src/test/java/com/launchdarkly/client/FeatureStoreCachingTest.java @@ -10,7 +10,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; -@SuppressWarnings("javadoc") +@SuppressWarnings({ "deprecation", "javadoc" }) public class FeatureStoreCachingTest { @Test public void disabledHasExpectedProperties() { diff --git a/src/test/java/com/launchdarkly/client/LDClientEvaluationTest.java b/src/test/java/com/launchdarkly/client/LDClientEvaluationTest.java index ff41ad1c1..5ef7397d0 100644 --- a/src/test/java/com/launchdarkly/client/LDClientEvaluationTest.java +++ b/src/test/java/com/launchdarkly/client/LDClientEvaluationTest.java @@ -35,8 +35,8 @@ public class LDClientEvaluationTest { private LDConfig config = new LDConfig.Builder() .dataStore(specificFeatureStore(featureStore)) - .eventProcessorFactory(Components.nullEventProcessor()) - .dataSource(Components.nullUpdateProcessor()) + .eventProcessor(Components.nullEventProcessor()) + .dataSource(Components.nullDataSource()) .build(); private LDClientInterface client = new LDClient("SDK_KEY", config); @@ -223,7 +223,7 @@ public void appropriateErrorIfClientNotInitialized() throws Exception { FeatureStore badFeatureStore = new InMemoryFeatureStore(); LDConfig badConfig = new LDConfig.Builder() .dataStore(specificFeatureStore(badFeatureStore)) - .eventProcessorFactory(Components.nullEventProcessor()) + .eventProcessor(Components.nullEventProcessor()) .dataSource(specificUpdateProcessor(failedUpdateProcessor())) .startWaitMillis(0) .build(); @@ -264,8 +264,8 @@ public void appropriateErrorForUnexpectedException() throws Exception { FeatureStore badFeatureStore = featureStoreThatThrowsException(new RuntimeException("sorry")); LDConfig badConfig = new LDConfig.Builder() .dataStore(specificFeatureStore(badFeatureStore)) - .eventProcessorFactory(Components.nullEventProcessor()) - .dataSource(Components.nullUpdateProcessor()) + .eventProcessor(Components.nullEventProcessor()) + .dataSource(Components.nullDataSource()) .build(); try (LDClientInterface badClient = new LDClient("SDK_KEY", badConfig)) { EvaluationDetail expectedResult = EvaluationDetail.fromValue(false, null, diff --git a/src/test/java/com/launchdarkly/client/LDClientEventTest.java b/src/test/java/com/launchdarkly/client/LDClientEventTest.java index 941c2615d..caf90b6fe 100644 --- a/src/test/java/com/launchdarkly/client/LDClientEventTest.java +++ b/src/test/java/com/launchdarkly/client/LDClientEventTest.java @@ -30,8 +30,8 @@ public class LDClientEventTest { private TestUtil.TestEventProcessor eventSink = new TestUtil.TestEventProcessor(); private LDConfig config = new LDConfig.Builder() .dataStore(specificFeatureStore(featureStore)) - .eventProcessorFactory(specificEventProcessor(eventSink)) - .dataSource(Components.nullUpdateProcessor()) + .eventProcessor(specificEventProcessor(eventSink)) + .dataSource(Components.nullDataSource()) .build(); private LDClientInterface client = new LDClient("SDK_KEY", config); diff --git a/src/test/java/com/launchdarkly/client/LDClientLddModeTest.java b/src/test/java/com/launchdarkly/client/LDClientLddModeTest.java index afc4ca6c5..21a142823 100644 --- a/src/test/java/com/launchdarkly/client/LDClientLddModeTest.java +++ b/src/test/java/com/launchdarkly/client/LDClientLddModeTest.java @@ -15,6 +15,7 @@ @SuppressWarnings("javadoc") public class LDClientLddModeTest { + @SuppressWarnings("deprecation") @Test public void lddModeClientHasNullUpdateProcessor() throws IOException { LDConfig config = new LDConfig.Builder() diff --git a/src/test/java/com/launchdarkly/client/LDClientOfflineTest.java b/src/test/java/com/launchdarkly/client/LDClientOfflineTest.java index 45aa48bfa..2785fc5d1 100644 --- a/src/test/java/com/launchdarkly/client/LDClientOfflineTest.java +++ b/src/test/java/com/launchdarkly/client/LDClientOfflineTest.java @@ -21,6 +21,7 @@ public class LDClientOfflineTest { private static final LDUser user = new LDUser("user"); + @SuppressWarnings("deprecation") @Test public void offlineClientHasNullUpdateProcessor() throws IOException { LDConfig config = new LDConfig.Builder() diff --git a/src/test/java/com/launchdarkly/client/LDClientTest.java b/src/test/java/com/launchdarkly/client/LDClientTest.java index 02dc39657..ae4a20bd1 100644 --- a/src/test/java/com/launchdarkly/client/LDClientTest.java +++ b/src/test/java/com/launchdarkly/client/LDClientTest.java @@ -341,7 +341,7 @@ private void expectEventsSent(int count) { private LDClientInterface createMockClient(LDConfig.Builder config) { config.dataSource(TestUtil.specificUpdateProcessor(updateProcessor)); - config.eventProcessorFactory(TestUtil.specificEventProcessor(eventProcessor)); + config.eventProcessor(TestUtil.specificEventProcessor(eventProcessor)); return new LDClient("SDK_KEY", config.build()); } diff --git a/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreBuilderTest.java b/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreBuilderTest.java index de8cf2570..b537d68ad 100644 --- a/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreBuilderTest.java +++ b/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreBuilderTest.java @@ -13,7 +13,7 @@ import redis.clients.jedis.JedisPoolConfig; import redis.clients.jedis.Protocol; -@SuppressWarnings("javadoc") +@SuppressWarnings({ "deprecation", "javadoc" }) public class RedisFeatureStoreBuilderTest { @Test public void testDefaultValues() { diff --git a/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreTest.java b/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreTest.java index 8e4f6ea1b..889ec344d 100644 --- a/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreTest.java +++ b/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreTest.java @@ -1,7 +1,7 @@ package com.launchdarkly.client.integrations; +import com.launchdarkly.client.Components; import com.launchdarkly.client.FeatureStore; -import com.launchdarkly.client.FeatureStoreCacheConfig; import com.launchdarkly.client.FeatureStoreDatabaseTestBase; import com.launchdarkly.client.integrations.RedisDataStoreImpl.UpdateListener; import com.launchdarkly.client.utils.CachingStoreWrapper; @@ -31,14 +31,21 @@ public static void maybeSkipDatabaseTests() { @Override protected FeatureStore makeStore() { - RedisDataStoreBuilder builder = Redis.dataStore().uri(REDIS_URI); - builder.caching(cached ? FeatureStoreCacheConfig.enabled().ttlSeconds(30) : FeatureStoreCacheConfig.disabled()); + RedisDataStoreBuilder redisBuilder = Redis.dataStore().uri(REDIS_URI); + PersistentDataStoreBuilder builder = Components.persistentDataStore(redisBuilder); + if (cached) { + builder.cacheSeconds(30); + } else { + builder.noCaching(); + } return builder.createFeatureStore(); } @Override protected FeatureStore makeStoreWithPrefix(String prefix) { - return Redis.dataStore().uri(REDIS_URI).caching(FeatureStoreCacheConfig.disabled()).prefix(prefix).createFeatureStore(); + return Components.persistentDataStore( + Redis.dataStore().uri(REDIS_URI).prefix(prefix) + ).noCaching().createFeatureStore(); } @Override diff --git a/src/test/java/com/launchdarkly/client/utils/CachingStoreWrapperTest.java b/src/test/java/com/launchdarkly/client/utils/CachingStoreWrapperTest.java index db8e5ceba..306501bd9 100644 --- a/src/test/java/com/launchdarkly/client/utils/CachingStoreWrapperTest.java +++ b/src/test/java/com/launchdarkly/client/utils/CachingStoreWrapperTest.java @@ -6,7 +6,6 @@ import com.launchdarkly.client.VersionedDataKind; import com.launchdarkly.client.integrations.CacheMonitor; -import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; From 16201e94346fb2fdaef65891f046c50b1b7545b0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2020 21:09:40 -0800 Subject: [PATCH 3/9] remove some unnecessary 4.x-style methods from new types --- .../client/FeatureStoreCacheConfig.java | 51 ++++++++++++------- .../client/RedisFeatureStore.java | 2 +- .../client/RedisFeatureStoreBuilder.java | 29 +++++------ .../PersistentDataStoreBuilder.java | 23 ++++++--- .../integrations/RedisDataStoreBuilder.java | 50 ++---------------- .../PersistentDataStoreFactory.java | 40 +++------------ .../RedisFeatureStoreBuilderTest.java | 5 +- 7 files changed, 77 insertions(+), 123 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/FeatureStoreCacheConfig.java b/src/main/java/com/launchdarkly/client/FeatureStoreCacheConfig.java index 65451b712..1860e7b3e 100644 --- a/src/main/java/com/launchdarkly/client/FeatureStoreCacheConfig.java +++ b/src/main/java/com/launchdarkly/client/FeatureStoreCacheConfig.java @@ -91,7 +91,40 @@ public enum StaleValuesPolicy { * See: CacheBuilder for * more specific information on cache semantics. */ - REFRESH_ASYNC + REFRESH_ASYNC; + + /** + * Used internally for backward compatibility. + * @return the equivalent enum value + * @since 4.11.0 + */ + public PersistentDataStoreBuilder.StaleValuesPolicy toNewEnum() { + switch (this) { + case REFRESH: + return PersistentDataStoreBuilder.StaleValuesPolicy.REFRESH; + case REFRESH_ASYNC: + return PersistentDataStoreBuilder.StaleValuesPolicy.REFRESH_ASYNC; + default: + return PersistentDataStoreBuilder.StaleValuesPolicy.EVICT; + } + } + + /** + * Used internally for backward compatibility. + * @param policy the enum value in the new API + * @return the equivalent enum value + * @since 4.11.0 + */ + public static StaleValuesPolicy fromNewEnum(PersistentDataStoreBuilder.StaleValuesPolicy policy) { + switch (policy) { + case REFRESH: + return StaleValuesPolicy.REFRESH; + case REFRESH_ASYNC: + return StaleValuesPolicy.REFRESH_ASYNC; + default: + return StaleValuesPolicy.EVICT; + } + } }; /** @@ -239,22 +272,6 @@ public FeatureStoreCacheConfig staleValuesPolicy(StaleValuesPolicy policy) { return new FeatureStoreCacheConfig(cacheTime, cacheTimeUnit, policy); } - /** - * Used internally for backward compatibility from the newer builder API. - * - * @param policy a {@link com.launchdarkly.client.integrations.PersistentDataStoreBuilder.StaleValuesPolicy} constant - * @return an updated parameters object - */ - public FeatureStoreCacheConfig staleValuesPolicy(PersistentDataStoreBuilder.StaleValuesPolicy policy) { - switch (policy) { - case REFRESH: - return staleValuesPolicy(StaleValuesPolicy.REFRESH); - case REFRESH_ASYNC: - return staleValuesPolicy(StaleValuesPolicy.REFRESH_ASYNC); - default: - return staleValuesPolicy(StaleValuesPolicy.EVICT); - } - } @Override public boolean equals(Object other) { if (other instanceof FeatureStoreCacheConfig) { diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java index 955cb2bab..ebd36913c 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java @@ -75,7 +75,7 @@ public CacheStats getCacheStats() { * @param builder the configured builder to construct the store with. */ protected RedisFeatureStore(RedisFeatureStoreBuilder builder) { - wrappedStore = builder.wrappedBuilder.createFeatureStore(); + wrappedStore = builder.wrappedOuterBuilder.createFeatureStore(); } /** diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java index 07e8684aa..c447da57c 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java @@ -1,6 +1,7 @@ package com.launchdarkly.client; import com.launchdarkly.client.integrations.CacheMonitor; +import com.launchdarkly.client.integrations.PersistentDataStoreBuilder; import com.launchdarkly.client.integrations.Redis; import com.launchdarkly.client.integrations.RedisDataStoreBuilder; @@ -39,22 +40,23 @@ public final class RedisFeatureStoreBuilder implements FeatureStoreFactory { */ public static final long DEFAULT_CACHE_TIME_SECONDS = FeatureStoreCacheConfig.DEFAULT_TIME_SECONDS; + final PersistentDataStoreBuilder wrappedOuterBuilder; final RedisDataStoreBuilder wrappedBuilder; // We have to keep track of these caching parameters separately in order to support some deprecated setters - FeatureStoreCacheConfig caching = FeatureStoreCacheConfig.DEFAULT; boolean refreshStaleValues = false; boolean asyncRefresh = false; // These constructors are called only from Components RedisFeatureStoreBuilder() { wrappedBuilder = Redis.dataStore(); + wrappedOuterBuilder = Components.persistentDataStore(wrappedBuilder); // In order to make the cacheStats() method on the deprecated RedisFeatureStore class work, we need to // turn on cache monitoring. In the newer API, cache monitoring would only be turned on if the application // specified its own CacheMonitor, but in the deprecated API there's no way to know if they will want the // statistics or not. - wrappedBuilder.cacheMonitor(new CacheMonitor()); + wrappedOuterBuilder.cacheMonitor(new CacheMonitor()); } RedisFeatureStoreBuilder(URI uri) { @@ -72,8 +74,7 @@ public final class RedisFeatureStoreBuilder implements FeatureStoreFactory { public RedisFeatureStoreBuilder(URI uri, long cacheTimeSecs) { this(); wrappedBuilder.uri(uri); - caching = caching.ttlSeconds(cacheTimeSecs); - wrappedBuilder.caching(caching); + wrappedOuterBuilder.cacheSeconds(cacheTimeSecs); } /** @@ -89,8 +90,7 @@ public RedisFeatureStoreBuilder(URI uri, long cacheTimeSecs) { public RedisFeatureStoreBuilder(String scheme, String host, int port, long cacheTimeSecs) throws URISyntaxException { this(); wrappedBuilder.uri(new URI(scheme, null, host, port, null, null, null)); - caching = caching.ttlSeconds(cacheTimeSecs); - wrappedBuilder.caching(caching); + wrappedOuterBuilder.cacheSeconds(cacheTimeSecs); } /** @@ -153,8 +153,8 @@ public RedisFeatureStoreBuilder tls(boolean tls) { * @since 4.6.0 */ public RedisFeatureStoreBuilder caching(FeatureStoreCacheConfig caching) { - this.caching = caching; - wrappedBuilder.caching(caching); + wrappedOuterBuilder.cacheTime(caching.getCacheTime(), caching.getCacheTimeUnit()); + wrappedOuterBuilder.staleValuesPolicy(caching.getStaleValuesPolicy().toNewEnum()); return this; } @@ -194,13 +194,12 @@ private void updateCachingStaleValuesPolicy() { // We need this logic in order to support the existing behavior of the deprecated methods above: // asyncRefresh is supposed to have no effect unless refreshStaleValues is true if (refreshStaleValues) { - caching = caching.staleValuesPolicy(this.asyncRefresh ? - FeatureStoreCacheConfig.StaleValuesPolicy.REFRESH_ASYNC : - FeatureStoreCacheConfig.StaleValuesPolicy.REFRESH); + wrappedOuterBuilder.staleValuesPolicy(this.asyncRefresh ? + PersistentDataStoreBuilder.StaleValuesPolicy.REFRESH_ASYNC : + PersistentDataStoreBuilder.StaleValuesPolicy.REFRESH); } else { - caching = caching.staleValuesPolicy(FeatureStoreCacheConfig.StaleValuesPolicy.EVICT); + wrappedOuterBuilder.staleValuesPolicy(PersistentDataStoreBuilder.StaleValuesPolicy.EVICT); } - wrappedBuilder.caching(caching); } /** @@ -225,9 +224,7 @@ public RedisFeatureStoreBuilder prefix(String prefix) { * @deprecated use {@link #caching(FeatureStoreCacheConfig)} and {@link FeatureStoreCacheConfig#ttl(long, TimeUnit)}. */ public RedisFeatureStoreBuilder cacheTime(long cacheTime, TimeUnit timeUnit) { - caching = caching.ttl(cacheTime, timeUnit) - .staleValuesPolicy(this.caching.getStaleValuesPolicy()); - wrappedBuilder.caching(caching); + wrappedOuterBuilder.cacheTime(cacheTime, timeUnit); return this; } diff --git a/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java b/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java index d2462228e..fd6884a12 100644 --- a/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java @@ -1,8 +1,11 @@ package com.launchdarkly.client.integrations; import com.launchdarkly.client.FeatureStore; +import com.launchdarkly.client.FeatureStoreCacheConfig; import com.launchdarkly.client.FeatureStoreFactory; import com.launchdarkly.client.interfaces.PersistentDataStoreFactory; +import com.launchdarkly.client.utils.CachingStoreWrapper; +import com.launchdarkly.client.utils.FeatureStoreCore; import java.util.concurrent.TimeUnit; @@ -32,6 +35,7 @@ * * @since 4.11.0 */ +@SuppressWarnings("deprecation") public final class PersistentDataStoreBuilder implements FeatureStoreFactory { /** * The default value for the cache TTL. @@ -39,7 +43,9 @@ public final class PersistentDataStoreBuilder implements FeatureStoreFactory { public static final int DEFAULT_CACHE_TTL_SECONDS = 15; private final PersistentDataStoreFactory persistentDataStoreFactory; - + FeatureStoreCacheConfig caching = FeatureStoreCacheConfig.DEFAULT; + CacheMonitor cacheMonitor = null; + /** * Possible values for {@link #staleValuesPolicy(StaleValuesPolicy)}. */ @@ -96,7 +102,11 @@ public PersistentDataStoreBuilder(PersistentDataStoreFactory persistentDataStore @Override public FeatureStore createFeatureStore() { - return persistentDataStoreFactory.createFeatureStore(); + FeatureStoreCore core = persistentDataStoreFactory.createPersistentDataStore(); + return CachingStoreWrapper.builder(core) + .caching(caching) + .cacheMonitor(cacheMonitor) + .build(); } /** @@ -121,9 +131,8 @@ public PersistentDataStoreBuilder noCaching() { * @param cacheTimeUnit the time unit * @return the builder */ - @SuppressWarnings("deprecation") public PersistentDataStoreBuilder cacheTime(long cacheTime, TimeUnit cacheTimeUnit) { - persistentDataStoreFactory.cacheTime(cacheTime, cacheTimeUnit); + caching = caching.ttl(cacheTime, cacheTimeUnit); return this; } @@ -170,9 +179,8 @@ public PersistentDataStoreBuilder cacheForever() { * @param staleValuesPolicy a {@link StaleValuesPolicy} constant * @return the builder */ - @SuppressWarnings("deprecation") public PersistentDataStoreBuilder staleValuesPolicy(StaleValuesPolicy staleValuesPolicy) { - persistentDataStoreFactory.staleValuesPolicy(staleValuesPolicy); + caching = caching.staleValuesPolicy(FeatureStoreCacheConfig.StaleValuesPolicy.fromNewEnum(staleValuesPolicy)); return this; } @@ -201,9 +209,8 @@ public PersistentDataStoreBuilder staleValuesPolicy(StaleValuesPolicy staleValue * @param cacheMonitor an instance of {@link CacheMonitor} * @return the builder */ - @SuppressWarnings("deprecation") public PersistentDataStoreBuilder cacheMonitor(CacheMonitor cacheMonitor) { - persistentDataStoreFactory.cacheMonitor(cacheMonitor); + this.cacheMonitor = cacheMonitor; return this; } } diff --git a/src/main/java/com/launchdarkly/client/integrations/RedisDataStoreBuilder.java b/src/main/java/com/launchdarkly/client/integrations/RedisDataStoreBuilder.java index 605bc4f60..70b25b792 100644 --- a/src/main/java/com/launchdarkly/client/integrations/RedisDataStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/integrations/RedisDataStoreBuilder.java @@ -1,10 +1,7 @@ package com.launchdarkly.client.integrations; -import com.launchdarkly.client.FeatureStore; -import com.launchdarkly.client.FeatureStoreCacheConfig; -import com.launchdarkly.client.integrations.PersistentDataStoreBuilder.StaleValuesPolicy; import com.launchdarkly.client.interfaces.PersistentDataStoreFactory; -import com.launchdarkly.client.utils.CachingStoreWrapper; +import com.launchdarkly.client.utils.FeatureStoreCore; import java.net.URI; import java.util.concurrent.TimeUnit; @@ -20,7 +17,7 @@ * Obtain an instance of this class by calling {@link Redis#dataStore()}. After calling its methods * to specify any desired custom settings, you can pass it directly into the SDK configuration with * {@link com.launchdarkly.client.LDConfig.Builder#dataStore(com.launchdarkly.client.FeatureStoreFactory)}. - * You do not need to call {@link #createFeatureStore()} yourself to build the actual data store; that + * You do not need to call {@link #createPersistentDataStore()} yourself to build the actual data store; that * will be done by the SDK. *

* Builder calls can be chained, for example: @@ -37,7 +34,6 @@ * * @since 4.11.0 */ -@SuppressWarnings("deprecation") public final class RedisDataStoreBuilder implements PersistentDataStoreFactory { /** * The default value for the Redis URI: {@code redis://localhost:6379} @@ -56,8 +52,6 @@ public final class RedisDataStoreBuilder implements PersistentDataStoreFactory { Integer database = null; String password = null; boolean tls = false; - FeatureStoreCacheConfig caching = FeatureStoreCacheConfig.DEFAULT; - CacheMonitor cacheMonitor = null; JedisPoolConfig poolConfig = null; // These constructors are called only from Implementations @@ -118,36 +112,6 @@ public RedisDataStoreBuilder uri(URI redisUri) { return this; } - /** - * Specifies whether local caching should be enabled and if so, sets the cache properties. Local - * caching is enabled by default; see {@link FeatureStoreCacheConfig#DEFAULT}. To disable it, pass - * {@link FeatureStoreCacheConfig#disabled()} to this method. - * - * @param caching a {@link FeatureStoreCacheConfig} object specifying caching parameters - * @return the builder - * @deprecated This has been superseded by the {@link PersistentDataStoreBuilder} interface. - */ - @Deprecated - public RedisDataStoreBuilder caching(FeatureStoreCacheConfig caching) { - this.caching = caching; - return this; - } - - @Override - public void cacheTime(long cacheTime, TimeUnit cacheTimeUnit) { - this.caching = caching.ttl(cacheTime, cacheTimeUnit); - } - - @Override - public void staleValuesPolicy(StaleValuesPolicy staleValuesPolicy) { - this.caching = caching.staleValuesPolicy(staleValuesPolicy); - } - - @Override - public void cacheMonitor(CacheMonitor cacheMonitor) { - this.cacheMonitor = cacheMonitor; - } - /** * Optionally configures the namespace prefix for all keys stored in Redis. * @@ -196,12 +160,8 @@ public RedisDataStoreBuilder socketTimeout(int socketTimeout, TimeUnit timeUnit) return this; } - /** - * Called internally by the SDK to create the actual data store instance. - * @return the data store configured by this builder - */ - public FeatureStore createFeatureStore() { - RedisDataStoreImpl core = new RedisDataStoreImpl(this); - return CachingStoreWrapper.builder(core).caching(this.caching).cacheMonitor(this.cacheMonitor).build(); + @Override + public FeatureStoreCore createPersistentDataStore() { + return new RedisDataStoreImpl(this); } } diff --git a/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java b/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java index 1148a3566..030dedd12 100644 --- a/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java +++ b/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java @@ -1,11 +1,7 @@ package com.launchdarkly.client.interfaces; -import com.launchdarkly.client.FeatureStoreFactory; -import com.launchdarkly.client.integrations.CacheMonitor; import com.launchdarkly.client.integrations.PersistentDataStoreBuilder; -import com.launchdarkly.client.integrations.PersistentDataStoreBuilder.StaleValuesPolicy; - -import java.util.concurrent.TimeUnit; +import com.launchdarkly.client.utils.FeatureStoreCore; /** * Interface for a factory that creates some implementation of a persistent data store. @@ -19,35 +15,15 @@ * @see com.launchdarkly.client.Components * @since 4.11.0 */ -public interface PersistentDataStoreFactory extends FeatureStoreFactory { - /** - * Called internally from {@link PersistentDataStoreBuilder}. - * - * @param cacheTime the cache TTL in whatever units you wish - * @param cacheTimeUnit the time unit - * @deprecated Calling this method directly on this component is deprecated. See {@link com.launchdarkly.client.Components#persistentDataStore} - * for the new usage. - */ - @Deprecated - void cacheTime(long cacheTime, TimeUnit cacheTimeUnit); - - /** - * Called internally from {@link PersistentDataStoreBuilder}. - * - * @param staleValuesPolicy a {@link StaleValuesPolicy} constant - * @deprecated Calling this method directly on this component is deprecated. See {@link com.launchdarkly.client.Components#persistentDataStore} - * for the new usage. - */ - @Deprecated - void staleValuesPolicy(StaleValuesPolicy staleValuesPolicy); - +public interface PersistentDataStoreFactory { /** - * Called internally from {@link PersistentDataStoreBuilder}. + * Called internally from {@link PersistentDataStoreBuilder} to create the implementation object + * for the specific type of data store. * - * @param cacheMonitor an instance of {@link CacheMonitor} - * @deprecated Calling this method directly on this component is deprecated. See {@link com.launchdarkly.client.Components#persistentDataStore} - * for the new usage. + * @return the implementation object + * @deprecated Do not reference this method directly, as the {@link FeatureStoreCore} interface + * will be replaced in 5.0. */ @Deprecated - void cacheMonitor(CacheMonitor cacheMonitor); + FeatureStoreCore createPersistentDataStore(); } diff --git a/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreBuilderTest.java b/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreBuilderTest.java index b537d68ad..3da39b69b 100644 --- a/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreBuilderTest.java +++ b/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreBuilderTest.java @@ -1,7 +1,5 @@ package com.launchdarkly.client.integrations; -import com.launchdarkly.client.FeatureStoreCacheConfig; - import org.junit.Test; import java.net.URISyntaxException; @@ -13,13 +11,12 @@ import redis.clients.jedis.JedisPoolConfig; import redis.clients.jedis.Protocol; -@SuppressWarnings({ "deprecation", "javadoc" }) +@SuppressWarnings("javadoc") public class RedisFeatureStoreBuilderTest { @Test public void testDefaultValues() { RedisDataStoreBuilder conf = Redis.dataStore(); assertEquals(RedisDataStoreBuilder.DEFAULT_URI, conf.uri); - assertEquals(FeatureStoreCacheConfig.DEFAULT, conf.caching); assertEquals(Protocol.DEFAULT_TIMEOUT, conf.connectTimeout); assertEquals(Protocol.DEFAULT_TIMEOUT, conf.socketTimeout); assertEquals(RedisDataStoreBuilder.DEFAULT_PREFIX, conf.prefix); From d00065d1edc768ff64bdac2bcda28eac64003e1e Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2020 21:09:47 -0800 Subject: [PATCH 4/9] add package info --- .../com/launchdarkly/client/interfaces/package-info.java | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 src/main/java/com/launchdarkly/client/interfaces/package-info.java diff --git a/src/main/java/com/launchdarkly/client/interfaces/package-info.java b/src/main/java/com/launchdarkly/client/interfaces/package-info.java new file mode 100644 index 000000000..d798dc8f0 --- /dev/null +++ b/src/main/java/com/launchdarkly/client/interfaces/package-info.java @@ -0,0 +1,7 @@ +/** + * The package for interfaces that allow customization of LaunchDarkly components. + *

+ * You will not need to refer to these types in your code unless you are creating a + * plug-in component, such as a database integration. + */ +package com.launchdarkly.client.interfaces; From f2d54daf0e9d26958953e9d3d299128250f24bce Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2020 21:10:11 -0800 Subject: [PATCH 5/9] deprecation --- .../com/launchdarkly/client/utils/CachingStoreWrapper.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/launchdarkly/client/utils/CachingStoreWrapper.java b/src/main/java/com/launchdarkly/client/utils/CachingStoreWrapper.java index 8e841a52e..a4c7853ce 100644 --- a/src/main/java/com/launchdarkly/client/utils/CachingStoreWrapper.java +++ b/src/main/java/com/launchdarkly/client/utils/CachingStoreWrapper.java @@ -33,8 +33,11 @@ * Construct instances of this class with {@link CachingStoreWrapper#builder(FeatureStoreCore)}. * * @since 4.6.0 + * @deprecated Referencing this class directly is deprecated; it is now part of the implementation + * of {@link com.launchdarkly.client.integrations.PersistentDataStoreBuilder} and will be made + * package-private starting in version 5.0. */ -@SuppressWarnings("deprecation") +@Deprecated public class CachingStoreWrapper implements FeatureStore { private static final String CACHE_REFRESH_THREAD_POOL_NAME_FORMAT = "CachingStoreWrapper-refresher-pool-%d"; From bf75d5c4d7664cc4c80d31da29f08d5451fbe6b0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2020 21:11:15 -0800 Subject: [PATCH 6/9] misc cleanup --- .../launchdarkly/client/integrations/RedisFeatureStoreTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreTest.java b/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreTest.java index 889ec344d..b93718a2b 100644 --- a/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreTest.java +++ b/src/test/java/com/launchdarkly/client/integrations/RedisFeatureStoreTest.java @@ -14,7 +14,7 @@ import redis.clients.jedis.Jedis; -@SuppressWarnings("javadoc") +@SuppressWarnings({ "deprecation", "javadoc" }) public class RedisFeatureStoreTest extends FeatureStoreDatabaseTestBase { private static final URI REDIS_URI = URI.create("redis://localhost:6379"); From 81c937f5173a933bca1dd2edeeb715e6ab025cb1 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 13 Jan 2020 21:42:25 -0800 Subject: [PATCH 7/9] misc cleanup --- .../client/integrations/CacheMonitor.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java b/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java index b1e8bd6e6..618edc63d 100644 --- a/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java +++ b/src/main/java/com/launchdarkly/client/integrations/CacheMonitor.java @@ -20,7 +20,10 @@ public CacheMonitor() {} /** * Called internally by the SDK to establish a source for the statistics. * @param source provided by an internal SDK component + * @deprecated Referencing this method directly is deprecated. In a future version, it will + * only be visible to SDK implementation code. */ + @Deprecated public void setSource(Callable source) { this.source = source; } @@ -58,12 +61,12 @@ public static final class CacheStats { /** * Constructs a new instance. * - * @param hitCount - * @param missCount - * @param loadSuccessCount - * @param loadExceptionCount - * @param totalLoadTime - * @param evictionCount + * @param hitCount number of queries that produced a cache hit + * @param missCount number of queries that produced a cache miss + * @param loadSuccessCount number of cache misses that loaded a value without an exception + * @param loadExceptionCount number of cache misses that tried to load a value but got an exception + * @param totalLoadTime number of nanoseconds spent loading new values + * @param evictionCount number of cache entries that have been evicted */ public CacheStats(long hitCount, long missCount, long loadSuccessCount, long loadExceptionCount, long totalLoadTime, long evictionCount) { From d5c85c54f8f1d7bd7ab01a037f16f8ffdc6990fa Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 14 Jan 2020 19:55:18 -0800 Subject: [PATCH 8/9] fix comment --- .../client/interfaces/PersistentDataStoreFactory.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java b/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java index 030dedd12..8931247d3 100644 --- a/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java +++ b/src/main/java/com/launchdarkly/client/interfaces/PersistentDataStoreFactory.java @@ -6,11 +6,8 @@ /** * Interface for a factory that creates some implementation of a persistent data store. *

- * Note that currently this interface contains methods that are duplicates of the methods in - * {@link PersistentDataStoreBuilder}. This is necessary to preserve backward compatibility with the - * implementation of persistent data stores in earlier versions of the SDK. The new recommended usage - * is described in {@link com.launchdarkly.client.Components#persistentDataStore}, and starting in - * version 5.0 these redundant methods will be removed. + * This interface is implemented by database integrations. Usage is described in + * {@link com.launchdarkly.client.Components#persistentDataStore}. * * @see com.launchdarkly.client.Components * @since 4.11.0 From 1cf368a93aa9bb7dd1eb6e7bbbea38dcd62e9cf5 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 15 Jan 2020 10:43:20 -0800 Subject: [PATCH 9/9] comment fixes --- src/main/java/com/launchdarkly/client/Components.java | 4 ++-- .../client/integrations/PersistentDataStoreBuilder.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/launchdarkly/client/Components.java b/src/main/java/com/launchdarkly/client/Components.java index 7049deb9e..e673b8f20 100644 --- a/src/main/java/com/launchdarkly/client/Components.java +++ b/src/main/java/com/launchdarkly/client/Components.java @@ -38,8 +38,8 @@ public static FeatureStoreFactory inMemoryDataStore() { *

* This method is used in conjunction with another factory object provided by specific components * such as the Redis integration. The latter provides builder methods for options that are specific - * to that integration, while the {@link PersistentDataStoreBuilder} provides options like - * that are applicable to any persistent data store (such as caching). For example: + * to that integration, while the {@link PersistentDataStoreBuilder} provides options that are + * applicable to any persistent data store (such as caching). For example: * * 


    *     LDConfig config = new LDConfig.Builder()
diff --git a/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java b/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java
index fd6884a12..43a1b42b3 100644
--- a/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java
+++ b/src/main/java/com/launchdarkly/client/integrations/PersistentDataStoreBuilder.java
@@ -159,7 +159,7 @@ public PersistentDataStoreBuilder cacheSeconds(long seconds) {
   /**
    * Specifies that the in-memory cache should never expire. In this mode, data will be written
    * to both the underlying persistent store and the cache, but will only ever be read from
-   * persistent store if the SDK is restarted.
+   * the persistent store if the SDK is restarted.
    * 

    * Use this mode with caution: it means that in a scenario where multiple processes are sharing
    * the database, and the current process loses connectivity to LaunchDarkly while other processes