diff --git a/src/main/asciidoc/new-features.adoc b/src/main/asciidoc/new-features.adoc index d34879cef..b7827da4f 100644 --- a/src/main/asciidoc/new-features.adoc +++ b/src/main/asciidoc/new-features.adoc @@ -11,6 +11,7 @@ New and noteworthy in the latest releases. * Upgrade to `Lettuce` 5.0. * Reactive connection support using https://github.com/lettuce-io/lettuce-core[lettuce-io/lettuce-core]. * Introduce Redis feature-specific interfaces for `RedisConnection`. +* Revised `RedisCache` implementation. [[new-in-1.8.0]] diff --git a/src/main/asciidoc/reference/redis.adoc b/src/main/asciidoc/reference/redis.adoc index 84e443a86..4fa246c29 100644 --- a/src/main/asciidoc/reference/redis.adoc +++ b/src/main/asciidoc/reference/redis.adoc @@ -473,19 +473,19 @@ Spring Redis provides an implementation for Spring http://docs.spring.io/spring/ ---- @Bean public RedisCacheManager cacheManager(RedisConnectionFactory connectionFactory) { - return RedisCacheManager.defaultCacheManager(connectionFactory); + return RedisCacheManager.create(connectionFactory); } ---- -`RedisCacheManager` behavior can be configured via `RedisCacheManagerConfigurator` allowing to set the default `RedisCacheConfiguration`, transaction behaviour and predefined caches. +`RedisCacheManager` behavior can be configured via `RedisCacheManagerBuilder` allowing to set the default `RedisCacheConfiguration`, transaction behaviour and predefined caches. [source,java] ---- -RedisCacheManager cm = RedisCacheManager.usingRawConnectionFactory(connectionFactory) - .withCacheDefaults(defaultCacheConfig()) - .withInitialCacheConfigurations(singletonMap("predefined", defaultCacheConfig().disableCachingNullValues())) +RedisCacheManager cm = RedisCacheManager.builder(connectionFactory) + .cacheDefaults(defaultCacheConfig()) + .initialCacheConfigurations(singletonMap("predefined", defaultCacheConfig().disableCachingNullValues())) .transactionAware() - .createAndGet(); + .build(); ---- Behavior of `RedisCache` created via `RedisCacheManager` is defined via `RedisCacheConfiguration`. The configuration allows to set key expiration times, prefixes and ``RedisSerializer``s for converting to and from the binary storage format. @@ -498,16 +498,15 @@ RedisCacheConfiguration config = RedisCacheConfiguration.defaultCacheConfig() .disableCachingNullValues(); ---- -Using default `RedisCacheManager` uses a non locking `RedisCacheWriter` for reading & writing bits. -While this ensures a max of performance the lack of entry locking can lead to overlapping, non atomic commands, for `putIfAbsent` and `clean` as those methods combine a series of commands sent to Redis. -The locking counterpart prevents command overlap by setting an explicit lock key and checking against presence of this key which leads to additional requests and potential command wait times. +`RedisCacheManager` defaults to a lock-free `RedisCacheWriter` for reading & writing binary values. Lock-free caching improves throughput. The lack of entry locking can lead to overlapping, non atomic commands, for `putIfAbsent` and `clean` methods as those require multiple commands sent to Redis. +The locking counterpart prevents command overlap by setting an explicit lock key and checking against presence of this key, which leads to additional requests and potential command wait times. It is possible to opt in to the locking behavior as follows: [source,java] ---- -RedisCacheManager cm = RedisCacheManager.usingCacheWriter(DefaultRedisCacheWriter.lockingRedisCacheWriter()) - .withCacheDefaults(defaultCacheConfig()) +RedisCacheManager cm = RedisCacheManager.build(RedisCacheWriter.lockingRedisCacheWriter()) + .cacheDefaults(defaultCacheConfig()) ... ---- @@ -521,7 +520,7 @@ RedisCacheManager cm = RedisCacheManager.usingCacheWriter(DefaultRedisCacheWrite |non locking |Cache Configuration -|`RedisCacheConfiguraiton#defaultConfiguration` +|`RedisCacheConfiguration#defaultConfiguration` |Initial Caches |none @@ -550,5 +549,8 @@ RedisCacheManager cm = RedisCacheManager.usingCacheWriter(DefaultRedisCacheWrite |Value Serializer |`JdkSerializationRedisSerializer` + +|Conversion Service +|`DefaultFormattingConversionService` with default cache key converters |==== diff --git a/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java b/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java index b97b77e85..13867093b 100644 --- a/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java +++ b/src/main/java/org/springframework/data/redis/cache/DefaultRedisCacheWriter.java @@ -15,15 +15,16 @@ */ package org.springframework.data.redis.cache; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; +import java.util.function.Function; -import org.springframework.data.redis.connection.RedisClusterConnection; +import org.springframework.dao.PessimisticLockingFailureException; import org.springframework.data.redis.connection.RedisConnection; import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.data.redis.connection.RedisStringCommands.SetOption; -import org.springframework.data.redis.connection.ReturnType; import org.springframework.data.redis.core.types.Expiration; import org.springframework.util.Assert; @@ -31,20 +32,19 @@ import org.springframework.util.Assert; * {@link RedisCacheWriter} implementation capable of reading/writing binary data from/to Redis in {@literal standalone} * and {@literal cluster} environments. Works upon a given {@link RedisConnectionFactory} to obtain the actual * {@link RedisConnection}.
- * {@link DefaultRedisCacheWriter} can be used in {@link #lockingRedisCacheWriter(RedisConnectionFactory) locking} or - * {@link #nonLockingRedisCacheWriter(RedisConnectionFactory) non-locking} mode. While {@literal non-locking} aims for - * maximum performance it may result in overlapping, non atomic, command execution for operations spanning multiple - * Redis interactions like {@code putIfAbsent}. The {@literal locking} counterpart prevents command overlap by setting - * an explicit lock key and checking against presence of this key which leads to additional requests and potential - * command wait times. + * {@link DefaultRedisCacheWriter} can be used in + * {@link RedisCacheWriter#lockingRedisCacheWriter(RedisConnectionFactory) locking} or + * {@link RedisCacheWriter#nonLockingRedisCacheWriter(RedisConnectionFactory) non-locking} mode. While + * {@literal non-locking} aims for maximum performance it may result in overlapping, non atomic, command execution for + * operations spanning multiple Redis interactions like {@code putIfAbsent}. The {@literal locking} counterpart prevents + * command overlap by setting an explicit lock key and checking against presence of this key which leads to additional + * requests and potential command wait times. * * @author Christoph Strobl + * @author Mark Paluch * @since 2.0 */ -public class DefaultRedisCacheWriter implements RedisCacheWriter { - - private static final byte[] CLEAN_SCRIPT = "local keys = redis.call('KEYS', ARGV[1]); local keysCount = table.getn(keys); if(keysCount > 0) then for _, key in ipairs(keys) do redis.call('del', key); end; end; return keysCount;" - .getBytes(Charset.forName("UTF-8")); +class DefaultRedisCacheWriter implements RedisCacheWriter { private final RedisConnectionFactory connectionFactory; private final Duration sleepTime; @@ -70,30 +70,10 @@ public class DefaultRedisCacheWriter implements RedisCacheWriter { this.sleepTime = sleepTime; } - /** - * Create new {@link RedisCacheWriter} without locking behavior. - * - * @param connectionFactory must not be {@literal null}. - * @return new instance of {@link DefaultRedisCacheWriter}. + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.RedisCacheWriter#put(java.lang.String, byte[], byte[], java.time.Duration) */ - public static DefaultRedisCacheWriter nonLockingRedisCacheWriter(RedisConnectionFactory connectionFactory) { - - Assert.notNull(connectionFactory, "ConnectionFactory must not be null!"); - return new DefaultRedisCacheWriter(connectionFactory); - } - - /** - * Create new {@link RedisCacheWriter} with locking behavior. - * - * @param connectionFactory must not be {@literal null}. - * @return new instance of {@link DefaultRedisCacheWriter}. - */ - public static DefaultRedisCacheWriter lockingRedisCacheWriter(RedisConnectionFactory connectionFactory) { - - Assert.notNull(connectionFactory, "ConnectionFactory must not be null!"); - return new DefaultRedisCacheWriter(connectionFactory, Duration.ofMillis(50)); - } - @Override public void put(String name, byte[] key, byte[] value, Duration ttl) { @@ -114,6 +94,10 @@ public class DefaultRedisCacheWriter implements RedisCacheWriter { }); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.RedisCacheWriter#get(java.lang.String, byte[]) + */ @Override public byte[] get(String name, byte[] key) { @@ -123,6 +107,10 @@ public class DefaultRedisCacheWriter implements RedisCacheWriter { return execute(name, connection -> connection.get(key)); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.RedisCacheWriter#putIfAbsent(java.lang.String, byte[], byte[], java.time.Duration) + */ @Override public byte[] putIfAbsent(String name, byte[] key, byte[] value, Duration ttl) { @@ -156,6 +144,10 @@ public class DefaultRedisCacheWriter implements RedisCacheWriter { }); } + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.RedisCacheWriter#remove(java.lang.String, byte[]) + */ @Override public void remove(String name, byte[] key) { @@ -165,46 +157,10 @@ public class DefaultRedisCacheWriter implements RedisCacheWriter { execute(name, connection -> connection.del(key)); } - /** - * Explicitly set a write lock on a cache. - * - * @param name the name of the cache to lock. + /* + * (non-Javadoc) + * @see org.springframework.data.redis.cache.RedisCacheWriter#clean(java.lang.String, byte[]) */ - public void lock(String name) { - execute(name, connection -> doLock(name, connection)); - } - - private Boolean doLock(String name, RedisConnection connection) { - return connection.setNX(createCacheLockKey(name), new byte[] {}); - } - - /** - * Explicitly remove a write lock from a cache. - * - * @param name the name of the cache to unlock. - */ - public void unlock(String name) { - executeWithoutLockCheck(connection -> doUnlock(name, connection)); - } - - private Long doUnlock(String name, RedisConnection connection) { - return connection.del(createCacheLockKey(name)); - } - - /** - * Check if a cache has set a lock. - * - * @param name the name of the cache to check for presence of a lock. - * @return {@literal true} if lock found. - */ - public boolean isLoked(String name) { - return executeWithoutLockCheck(connection -> doCheckLock(name, connection)); - } - - private boolean doCheckLock(String name, RedisConnection connection) { - return connection.exists(createCacheLockKey(name)); - } - @Override public void clean(String name, byte[] pattern) { @@ -216,17 +172,16 @@ public class DefaultRedisCacheWriter implements RedisCacheWriter { boolean wasLocked = false; try { - if (connection instanceof RedisClusterConnection) { - if (isLockingCacheWriter()) { - doLock(name, connection); - wasLocked = true; - } + if (isLockingCacheWriter()) { + doLock(name, connection); + wasLocked = true; + } - byte[][] keys = connection.keys(pattern).stream().toArray(size -> new byte[size][]); + byte[][] keys = connection.keys(pattern).toArray(new byte[0][]); + + if (keys.length > 0) { connection.del(keys); - } else { - connection.eval(CLEAN_SCRIPT, ReturnType.INTEGER, 0, pattern); } } finally { @@ -239,31 +194,61 @@ public class DefaultRedisCacheWriter implements RedisCacheWriter { }); } + /** + * Explicitly set a write lock on a cache. + * + * @param name the name of the cache to lock. + */ + void lock(String name) { + execute(name, connection -> doLock(name, connection)); + } + + /** + * Explicitly remove a write lock from a cache. + * + * @param name the name of the cache to unlock. + */ + void unlock(String name) { + executeLockFree(connection -> doUnlock(name, connection)); + } + + private Boolean doLock(String name, RedisConnection connection) { + return connection.setNX(createCacheLockKey(name), new byte[0]); + } + + private Long doUnlock(String name, RedisConnection connection) { + return connection.del(createCacheLockKey(name)); + } + + boolean doCheckLock(String name, RedisConnection connection) { + return connection.exists(createCacheLockKey(name)); + } + /** * @return {@literal true} if {@link RedisCacheWriter} uses locks. */ - public boolean isLockingCacheWriter() { + private boolean isLockingCacheWriter() { return !sleepTime.isZero() && !sleepTime.isNegative(); } - T execute(String name, ConnectionCallback callback) { + private T execute(String name, Function callback) { RedisConnection connection = connectionFactory.getConnection(); try { checkAndPotentiallyWaitUntilUnlocked(name, connection); - return callback.doWithConnection(connection); + return callback.apply(connection); } finally { connection.close(); } } - private T executeWithoutLockCheck(ConnectionCallback callback) { + private void executeLockFree(Consumer callback) { RedisConnection connection = connectionFactory.getConnection(); try { - return callback.doWithConnection(connection); + callback.accept(connection); } finally { connection.close(); } @@ -271,35 +256,30 @@ public class DefaultRedisCacheWriter implements RedisCacheWriter { private void checkAndPotentiallyWaitUntilUnlocked(String name, RedisConnection connection) { - if (isLockingCacheWriter()) { + if (!isLockingCacheWriter()) { + return; + } - long timeout = sleepTime.toMillis(); + try { while (doCheckLock(name, connection)) { - try { - Thread.sleep(timeout); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } + Thread.sleep(sleepTime.toMillis()); } + } catch (InterruptedException ex) { + + // Re-interrupt current thread, to allow other participants to react. + Thread.currentThread().interrupt(); + + throw new PessimisticLockingFailureException(String.format("Interrupted while waiting to unlock cache %s", name), + ex); } } - private boolean shouldExpireWithin(Duration ttl) { + private static boolean shouldExpireWithin(Duration ttl) { return ttl != null && !ttl.isZero() && !ttl.isNegative(); } - byte[] createCacheLockKey(String name) { - return (name + "~lock").getBytes(Charset.forName("UTF-8")); - } - - /** - * @param - * @author Christoph Strobl - * @since 2.0 - */ - interface ConnectionCallback { - - T doWithConnection(RedisConnection connection); + private static byte[] createCacheLockKey(String name) { + return (name + "~lock").getBytes(StandardCharsets.UTF_8); } } diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCache.java b/src/main/java/org/springframework/data/redis/cache/RedisCache.java index 112f498e5..ba3183175 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCache.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCache.java @@ -15,24 +15,31 @@ */ package org.springframework.data.redis.cache; +import java.lang.reflect.Method; import java.nio.ByteBuffer; -import java.nio.charset.Charset; import java.util.concurrent.Callable; import org.springframework.cache.support.AbstractValueAdaptingCache; import org.springframework.cache.support.NullValue; import org.springframework.cache.support.SimpleValueWrapper; -import org.springframework.core.convert.support.ConfigurableConversionService; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer; -import org.springframework.format.support.DefaultFormattingConversionService; +import org.springframework.data.redis.util.ByteUtils; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; +import org.springframework.util.ReflectionUtils; /** * {@link org.springframework.cache.Cache} implementation using for Redis as underlying store. + *

+ * Use {@link RedisCacheManager} to create {@link RedisCache} instances. * * @author Christoph Strobl + * @author Mark Paluch * @since 2.0 + * @see RedisCacheConfiguration + * @see RedisCacheWriter */ public class RedisCache extends AbstractValueAdaptingCache { @@ -41,7 +48,7 @@ public class RedisCache extends AbstractValueAdaptingCache { private final String name; private final RedisCacheWriter cacheWriter; private final RedisCacheConfiguration cacheConfig; - private final ConfigurableConversionService conversionService = new DefaultFormattingConversionService(); + private final ConversionService conversionService; /** * Create new {@link RedisCache}. @@ -50,7 +57,7 @@ public class RedisCache extends AbstractValueAdaptingCache { * @param cacheWriter must not be {@literal null}. * @param cacheConfig must not be {@literal null}. */ - RedisCache(String name, RedisCacheWriter cacheWriter, RedisCacheConfiguration cacheConfig) { + protected RedisCache(String name, RedisCacheWriter cacheWriter, RedisCacheConfiguration cacheConfig) { super(cacheConfig.getAllowCacheNullValues()); @@ -61,10 +68,13 @@ public class RedisCache extends AbstractValueAdaptingCache { this.name = name; this.cacheWriter = cacheWriter; this.cacheConfig = cacheConfig; - - conversionService.addConverter(String.class, byte[].class, source -> source.getBytes(Charset.forName("UTF-8"))); + this.conversionService = cacheConfig.getConversionService(); } + /* + * (non-Javadoc) + * @see org.springframework.cache.support.AbstractValueAdaptingCache#lookup(java.lang.Object) + */ @Override protected Object lookup(Object key) { @@ -77,17 +87,30 @@ public class RedisCache extends AbstractValueAdaptingCache { return deserializeCacheValue(value); } + /* + * (non-Javadoc) + * @see org.springframework.cache.Cache#getName() + */ @Override public String getName() { return name; } + /* + * (non-Javadoc) + * @see org.springframework.cache.Cache#getNativeCache() + */ @Override public RedisCacheWriter getNativeCache() { return this.cacheWriter; } + /* + * (non-Javadoc) + * @see org.springframework.cache.Cache#get(java.lang.Object, java.util.concurrent.Callable) + */ @Override + @SuppressWarnings("unchecked") public synchronized T get(Object key, Callable valueLoader) { ValueWrapper result = get(key); @@ -101,6 +124,10 @@ public class RedisCache extends AbstractValueAdaptingCache { return value; } + /* + * (non-Javadoc) + * @see org.springframework.cache.Cache#put(java.lang.Object, java.lang.Object) + */ @Override public void put(Object key, Object value) { @@ -116,6 +143,10 @@ public class RedisCache extends AbstractValueAdaptingCache { cacheWriter.put(name, createAndConvertCacheKey(key), serializeCacheValue(cacheValue), cacheConfig.getTtl()); } + /* + * (non-Javadoc) + * @see org.springframework.cache.Cache#putIfAbsent(java.lang.Object, java.lang.Object) + */ @Override public ValueWrapper putIfAbsent(Object key, Object value) { @@ -135,11 +166,19 @@ public class RedisCache extends AbstractValueAdaptingCache { return new SimpleValueWrapper(fromStoreValue(deserializeCacheValue(result))); } + /* + * (non-Javadoc) + * @see org.springframework.cache.Cache#evict(java.lang.Object) + */ @Override public void evict(Object key) { cacheWriter.remove(name, createAndConvertCacheKey(key)); } + /* + * (non-Javadoc) + * @see org.springframework.cache.Cache#clear() + */ @Override public void clear() { @@ -163,7 +202,6 @@ public class RedisCache extends AbstractValueAdaptingCache { * @param value can be {@literal null}. * @return preprocessed value. Can be {@literal null}. */ - protected Object preProcessCacheValue(Object value) { if (value != null) { @@ -180,7 +218,7 @@ public class RedisCache extends AbstractValueAdaptingCache { * @return never {@literal null}. */ protected byte[] serializeCacheKey(String cacheKey) { - return cacheConfig.getKeySerializationPair().write(cacheKey).array(); + return ByteUtils.getBytes(cacheConfig.getKeySerializationPair().write(cacheKey)); } /** @@ -195,7 +233,7 @@ public class RedisCache extends AbstractValueAdaptingCache { return BINARY_NULL_VALUE; } - return cacheConfig.getValueSerializationPair().write(value).array(); + return ByteUtils.getBytes(cacheConfig.getValueSerializationPair().write(value)); } /** @@ -204,7 +242,6 @@ public class RedisCache extends AbstractValueAdaptingCache { * @param value must not be {@literal null}. * @return can be {@literal null}. */ - protected Object deserializeCacheValue(byte[] value) { if (isAllowNullValues() && ObjectUtils.nullSafeEquals(value, BINARY_NULL_VALUE)) { @@ -222,7 +259,8 @@ public class RedisCache extends AbstractValueAdaptingCache { */ protected String createCacheKey(Object key) { - String convertedKey = conversionService.convert(key, String.class); + String convertedKey = convertKey(key); + if (!cacheConfig.usePrefix()) { return convertedKey; } @@ -230,6 +268,30 @@ public class RedisCache extends AbstractValueAdaptingCache { return prefixCacheKey(convertedKey); } + /** + * Convert {@code key} to a {@link String} representation used for cache key creation. + * + * @param key will never be {@literal null}. + * @return never {@literal null}. + * @throws IllegalStateException if {@code key} cannot be converted to {@link String}. + */ + protected String convertKey(Object key) { + + TypeDescriptor source = TypeDescriptor.forObject(key); + if (conversionService.canConvert(source, TypeDescriptor.valueOf(String.class))) { + return conversionService.convert(key, String.class); + } + + Method toString = ReflectionUtils.findMethod(key.getClass(), "toString"); + + if (toString != null && !Object.class.equals(toString.getDeclaringClass())) { + return key.toString(); + } + + throw new IllegalStateException( + "Cannot convert " + source + " to String. Register a Converter or override toString()."); + } + private byte[] createAndConvertCacheKey(Object key) { return serializeCacheKey(createCacheKey(key)); } @@ -246,5 +308,4 @@ public class RedisCache extends AbstractValueAdaptingCache { throw new ValueRetrievalException(key, valueLoader, e); } } - } diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java b/src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java index 1c71e9bb8..aa347bcee 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCacheConfiguration.java @@ -15,22 +15,28 @@ */ package org.springframework.data.redis.cache; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Optional; import org.springframework.cache.Cache; +import org.springframework.cache.interceptor.SimpleKey; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer; import org.springframework.data.redis.serializer.RedisSerializationContext.SerializationPair; import org.springframework.data.redis.serializer.StringRedisSerializer; +import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.util.Assert; /** * Immutable {@link RedisCacheConfiguration} helps customizing {@link RedisCache} behaviour such as caching * {@literal null} values, cache key prefixes and binary serialization.
- * Start with {@link RedisCacheConfiguration#defaultCacheConfig()} and customize {@link RedisCache} behaviour from - * there on. + * Start with {@link RedisCacheConfiguration#defaultCacheConfig()} and customize {@link RedisCache} behaviour from there + * on. * * @author Christoph Strobl + * @author Mark Paluch * @since 2.0 */ public class RedisCacheConfiguration { @@ -41,17 +47,22 @@ public class RedisCacheConfiguration { private final boolean usePrefix; private final SerializationPair keySerializationPair; - private final SerializationPair valueSerializationPair; + private final SerializationPair valueSerializationPair; + private final ConversionService conversionService; + + @SuppressWarnings("unchecked") private RedisCacheConfiguration(Duration ttl, Boolean cacheNullValues, Boolean usePrefix, String keyPrefix, - SerializationPair keySerializationPair, SerializationPair valueSerializationPair) { + SerializationPair keySerializationPair, SerializationPair valueSerializationPair, + ConversionService conversionService) { this.ttl = ttl; this.cacheNullValues = cacheNullValues; this.usePrefix = usePrefix; this.keyPrefix = keyPrefix; this.keySerializationPair = keySerializationPair; - this.valueSerializationPair = valueSerializationPair; + this.valueSerializationPair = (SerializationPair) valueSerializationPair; + this.conversionService = conversionService; } /** @@ -69,29 +80,36 @@ public class RedisCacheConfiguration { *
StringRedisSerializer.class
*
value serializer
*
JdkSerializationRedisSerializer.class
+ *
conversion service
+ *
{@link DefaultFormattingConversionService} with {@link #registerDefaultConverters(ConverterRegistry) default} + * cache key converters
* - * + * * @return new {@link RedisCacheConfiguration}. */ public static RedisCacheConfiguration defaultCacheConfig() { + DefaultFormattingConversionService conversionService = new DefaultFormattingConversionService(); + + registerDefaultConverters(conversionService); + return new RedisCacheConfiguration(Duration.ZERO, true, true, null, SerializationPair.fromSerializer(new StringRedisSerializer()), - SerializationPair.fromSerializer(new JdkSerializationRedisSerializer())); + SerializationPair.fromSerializer(new JdkSerializationRedisSerializer()), conversionService); } /** - * Set the ttl to apply for cache entries. Use {@link Duration#ZERO} to have an eternal cache. + * Set the ttl to apply for cache entries. Use {@link Duration#ZERO} to declare an eternal cache. * * @param ttl must not be {@literal null}. * @return new {@link RedisCacheConfiguration}. */ public RedisCacheConfiguration entryTtl(Duration ttl) { - Assert.notNull(ttl, "Ttl must not be null!"); + Assert.notNull(ttl, "TTL duration must not be null!"); return new RedisCacheConfiguration(ttl, cacheNullValues, usePrefix, keyPrefix, keySerializationPair, - valueSerializationPair); + valueSerializationPair, conversionService); } /** @@ -104,8 +122,8 @@ public class RedisCacheConfiguration { Assert.notNull(prefix, "Prefix must not be null!"); - return new RedisCacheConfiguration(ttl, cacheNullValues, true, prefix, keySerializationPair, - valueSerializationPair); + return new RedisCacheConfiguration(ttl, cacheNullValues, true, prefix, keySerializationPair, valueSerializationPair, + conversionService); } /** @@ -117,7 +135,8 @@ public class RedisCacheConfiguration { * @return new {@link RedisCacheConfiguration}. */ public RedisCacheConfiguration disableCachingNullValues() { - return new RedisCacheConfiguration(ttl, false, usePrefix, keyPrefix, keySerializationPair, valueSerializationPair); + return new RedisCacheConfiguration(ttl, false, usePrefix, keyPrefix, keySerializationPair, valueSerializationPair, + conversionService); } /** @@ -130,7 +149,21 @@ public class RedisCacheConfiguration { public RedisCacheConfiguration disableKeyPrefix() { return new RedisCacheConfiguration(ttl, cacheNullValues, false, keyPrefix, keySerializationPair, - valueSerializationPair); + valueSerializationPair, conversionService); + } + + /** + * Define the {@link ConversionService} used for cache key to {@link String} conversion. + * + * @param conversionService must not be {@literal null}. + * @return new {@link RedisCacheConfiguration}. + */ + public RedisCacheConfiguration withConversionService(ConversionService conversionService) { + + Assert.notNull(conversionService, "ConversionService must not be null!"); + + return new RedisCacheConfiguration(ttl, cacheNullValues, usePrefix, keyPrefix, keySerializationPair, + valueSerializationPair, conversionService); } /** @@ -144,7 +177,7 @@ public class RedisCacheConfiguration { Assert.notNull(keySerializationPair, "KeySerializationPair must not be null!"); return new RedisCacheConfiguration(ttl, cacheNullValues, usePrefix, keyPrefix, keySerializationPair, - valueSerializationPair); + valueSerializationPair, conversionService); } /** @@ -158,7 +191,7 @@ public class RedisCacheConfiguration { Assert.notNull(valueSerializationPair, "ValueSerializationPair must not be null!"); return new RedisCacheConfiguration(ttl, cacheNullValues, usePrefix, keyPrefix, keySerializationPair, - valueSerializationPair); + valueSerializationPair, conversionService); } /** @@ -193,7 +226,7 @@ public class RedisCacheConfiguration { /** * @return never {@literal null}. */ - public SerializationPair getValueSerializationPair() { + public SerializationPair getValueSerializationPair() { return valueSerializationPair; } @@ -201,7 +234,29 @@ public class RedisCacheConfiguration { * @return The expiration time (ttl) for cache entries. Never {@literal null}. */ public Duration getTtl() { - return ttl != null ? ttl : Duration.ZERO; + return ttl; } + /** + * @return The {@link ConversionService} used for cache key to {@link String} conversion. Never {@literal null}. + */ + public ConversionService getConversionService() { + return conversionService; + } + + /** + * Registers default cache key converters. The following converters get registered: + *
    + *
  • {@link String} to {@link byte byte[]} using UTF-8 encoding.
    • + *
  • {@link SimpleKey} to {@link String}
    • + * + * @param registry must not be {@literal null}. + */ + public static void registerDefaultConverters(ConverterRegistry registry) { + + Assert.notNull(registry, "ConverterRegistry must not be null!"); + + registry.addConverter(String.class, byte[].class, source -> source.getBytes(StandardCharsets.UTF_8)); + registry.addConverter(SimpleKey.class, String.class, SimpleKey::toString); + } } diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java b/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java index f0f87552d..dc778cc09 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java @@ -27,11 +27,21 @@ import java.util.Set; import org.springframework.cache.transaction.AbstractTransactionSupportingCacheManager; import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; /** + * {@link org.springframework.cache.CacheManager} backed by a {@link RedisCache Redis} cache. + *

    + * This cache manager creates caches upon first write. Empty caches are not visible on Redis due to how Redis represents + * empty data structures. + *

    + * Caches requiring a different {@link RedisCacheConfiguration} than the default configuration can be specified via + * {@link RedisCacheManagerBuilder#withInitialCacheConfigurations(Map)}. + * * @author Christoph Strobl + * @author Mark Paluch * @since 2.0 + * @see RedisCacheConfiguration + * @see RedisCacheWriter */ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager { @@ -39,6 +49,14 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager private final RedisCacheConfiguration defaultCacheConfig; private final Map initialCacheConfiguration; + /** + * Creates new {@link RedisCacheManager} using given {@link RedisCacheWriter} and default + * {@link RedisCacheConfiguration}. + * + * @param cacheWriter must not be {@literal null}. + * @param defaultCacheConfiguration must not be {@literal null}. Maybe just use + * {@link RedisCacheConfiguration#defaultCacheConfig()}. + */ public RedisCacheManager(RedisCacheWriter cacheWriter, RedisCacheConfiguration defaultCacheConfiguration) { Assert.notNull(cacheWriter, "CacheWriter must not be null!"); @@ -60,7 +78,7 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager * {@literal defaultCacheConfiguration}. */ public RedisCacheManager(RedisCacheWriter cacheWriter, RedisCacheConfiguration defaultCacheConfiguration, - String... initialCacheNames) { + String... initialCacheNames) { this(cacheWriter, defaultCacheConfiguration); @@ -80,11 +98,12 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager * Must not be {@literal null}. */ public RedisCacheManager(RedisCacheWriter cacheWriter, RedisCacheConfiguration defaultCacheConfiguration, - Map initialCacheConfigurations) { + Map initialCacheConfigurations) { this(cacheWriter, defaultCacheConfiguration); Assert.notNull(initialCacheConfigurations, "InitialCacheConfigurations must not be null!"); + this.initialCacheConfiguration.putAll(initialCacheConfigurations); } @@ -104,7 +123,7 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager * @param connectionFactory must not be {@literal null}. * @return new instance of {@link RedisCacheManager}. */ - public static RedisCacheManager defaultCacheManager(RedisConnectionFactory connectionFactory) { + public static RedisCacheManager create(RedisConnectionFactory connectionFactory) { Assert.notNull(connectionFactory, "ConnectionFactory must not be null!"); @@ -116,38 +135,48 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager * Entry point for builder style {@link RedisCacheManager} configuration. * * @param connectionFactory must not be {@literal null}. - * @return new {@link RedisCacheManagerConfigurator}. + * @return new {@link RedisCacheManagerBuilder}. */ - public static RedisCacheManagerConfigurator usingRawConnectionFactory(RedisConnectionFactory connectionFactory) { + public static RedisCacheManagerBuilder builder(RedisConnectionFactory connectionFactory) { Assert.notNull(connectionFactory, "ConnectionFactory must not be null!"); - return RedisCacheManagerConfigurator.usingRawFactory(connectionFactory); + return RedisCacheManagerBuilder.fromConnectionFactory(connectionFactory); } /** * Entry point for builder style {@link RedisCacheManager} configuration. * * @param cacheWriter must not be {@literal null}. - * @return new {@link RedisCacheManagerConfigurator}. + * @return new {@link RedisCacheManagerBuilder}. */ - public static RedisCacheManagerConfigurator usingCacheWriter(RedisCacheWriter cacheWriter) { + public static RedisCacheManagerBuilder builder(RedisCacheWriter cacheWriter) { Assert.notNull(cacheWriter, "CacheWriter must not be null!"); - return RedisCacheManagerConfigurator.usingCacheWriter(cacheWriter); + return RedisCacheManagerBuilder.fromCacheWriter(cacheWriter); } + /* + * (non-Javadoc) + * @see org.springframework.cache.support.AbstractCacheManager#loadCaches() + */ @Override protected Collection loadCaches() { List caches = new LinkedList<>(); + for (Map.Entry entry : initialCacheConfiguration.entrySet()) { caches.add(createRedisCache(entry.getKey(), entry.getValue())); } + return caches; } + /* + * (non-Javadoc) + * @see org.springframework.cache.support.AbstractCacheManager#getMissingCache(java.lang.String) + */ @Override protected RedisCache getMissingCache(String name) { return createRedisCache(name, defaultCacheConfig); @@ -159,16 +188,18 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager public Map getCacheConfigurations() { Map configurationMap = new HashMap<>(getCacheNames().size()); + getCacheNames().forEach(it -> { RedisCache cache = RedisCache.class.cast(lookupCache(it)); configurationMap.put(it, cache != null ? cache.getCacheConfiguration() : null); }); + return Collections.unmodifiableMap(configurationMap); } /** - * Configuration hook for creating {@link RedisCache} with given name and cacheConfig. + * Configuration hook for creating {@link RedisCache} with given name and {@code cacheConfig}. * * @param name must not be {@literal null}. * @param cacheConfig can be {@literal null}. @@ -182,92 +213,88 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager * Configurator for creating {@link RedisCacheManager}. * * @author Christoph Strobl + * @author Mark Strobl * @since 2.0 */ - public static class RedisCacheManagerConfigurator { + public static class RedisCacheManagerBuilder { - private final RedisCacheConfiguration defaultCacheConfiguration; private final RedisCacheWriter cacheWriter; - private final boolean enableTransactions; - private final Map intialCaches = new LinkedHashMap<>(); - - private RedisCacheManagerConfigurator(RedisCacheWriter cacheWriter, - RedisCacheConfiguration defaultCacheConfiguration, Map intialCaches, - boolean enableTransactions) { + private RedisCacheConfiguration defaultCacheConfiguration = RedisCacheConfiguration.defaultCacheConfig(); + private Map intialCaches = new LinkedHashMap<>(); + private boolean enableTransactions; + private RedisCacheManagerBuilder(RedisCacheWriter cacheWriter) { this.cacheWriter = cacheWriter; - this.defaultCacheConfiguration = defaultCacheConfiguration; - - if (!CollectionUtils.isEmpty(intialCaches)) { - this.intialCaches.putAll(intialCaches); - } - - this.enableTransactions = enableTransactions; } /** * Entry point for builder style {@link RedisCacheManager} configuration. * * @param connectionFactory must not be {@literal null}. - * @return new {@link RedisCacheManagerConfigurator}. + * @return new {@link RedisCacheManagerBuilder}. */ - public static RedisCacheManagerConfigurator usingRawFactory(RedisConnectionFactory connectionFactory) { + public static RedisCacheManagerBuilder fromConnectionFactory(RedisConnectionFactory connectionFactory) { Assert.notNull(connectionFactory, "ConnectionFactory must not be null!"); - return usingCacheWriter(new DefaultRedisCacheWriter(connectionFactory)); + return builder(new DefaultRedisCacheWriter(connectionFactory)); } /** * Entry point for builder style {@link RedisCacheManager} configuration. * * @param cacheWriter must not be {@literal null}. - * @return new {@link RedisCacheManagerConfigurator}. + * @return new {@link RedisCacheManagerBuilder}. */ - public static RedisCacheManagerConfigurator usingCacheWriter(RedisCacheWriter cacheWriter) { + public static RedisCacheManagerBuilder fromCacheWriter(RedisCacheWriter cacheWriter) { Assert.notNull(cacheWriter, "CacheWriter must not be null!"); - return new RedisCacheManagerConfigurator(cacheWriter, RedisCacheConfiguration.defaultCacheConfig(), null, false); + return new RedisCacheManagerBuilder(cacheWriter); } /** * Define a default {@link RedisCacheConfiguration} applied to dynamically created {@link RedisCache}s. * * @param defaultCacheConfiguration must not be {@literal null}. - * @return new instance of {@link RedisCacheManagerConfigurator}. + * @return new instance of {@link RedisCacheManagerBuilder}. */ - public RedisCacheManagerConfigurator withCacheDefaults(RedisCacheConfiguration defaultCacheConfiguration) { + public RedisCacheManagerBuilder cacheDefaults(RedisCacheConfiguration defaultCacheConfiguration) { Assert.notNull(defaultCacheConfiguration, "DefaultCacheConfiguration must not be null!"); - return new RedisCacheManagerConfigurator(cacheWriter, defaultCacheConfiguration, intialCaches, - enableTransactions); + this.defaultCacheConfiguration = defaultCacheConfiguration; + + return this; } /** * Enable {@link RedisCache}s to synchronize cache put/evict operations with ongoing Spring-managed transactions. * - * @return new instance of {@link RedisCacheManagerConfigurator}. + * @return new instance of {@link RedisCacheManagerBuilder}. */ - public RedisCacheManagerConfigurator transactionAware() { - return new RedisCacheManagerConfigurator(cacheWriter, defaultCacheConfiguration, intialCaches, true); + public RedisCacheManagerBuilder transactionAware() { + + this.enableTransactions = true; + + return this; } /** * Append a {@link Set} of cache names to be pre initialized with current {@link RedisCacheConfiguration}. - * NOTE: This calls depends on {@link #withCacheDefaults(RedisCacheConfiguration)} using whatever + * NOTE: This calls depends on {@link #cacheDefaults(RedisCacheConfiguration)} using whatever * default {@link RedisCacheConfiguration} is present at the time of invoking this method. * * @param cacheNames must not be {@literal null}. - * @return new instance of {@link RedisCacheManagerConfigurator}. + * @return new instance of {@link RedisCacheManagerBuilder}. */ - public RedisCacheManagerConfigurator withInitialCacheNames(Set cacheNames) { + public RedisCacheManagerBuilder initialCacheNames(Set cacheNames) { Assert.notNull(cacheNames, "CacheNames must not be null!"); Map cacheConfigMap = new LinkedHashMap<>(cacheNames.size()); cacheNames.forEach(it -> cacheConfigMap.put(it, defaultCacheConfiguration)); + return withInitialCacheConfigurations(cacheConfigMap); } @@ -275,17 +302,21 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager * Append a {@link Map} of cache name/{@link RedisCacheConfiguration} pairs to be pre initialized. * * @param cacheConfigurations must not be {@literal null}. - * @return new instance of {@link RedisCacheManagerConfigurator}. + * @return new instance of {@link RedisCacheManagerBuilder}. */ - public RedisCacheManagerConfigurator withInitialCacheConfigurations( + public RedisCacheManagerBuilder withInitialCacheConfigurations( Map cacheConfigurations) { Assert.notNull(cacheConfigurations, "CacheConfigurations must not be null!"); + cacheConfigurations.forEach((cacheName, configuration) -> Assert.notNull(configuration, + String.format("RedisCacheConfiguration for cache %s must not be null!", cacheName))); Map cacheConfigMap = new LinkedHashMap<>(intialCaches); cacheConfigMap.putAll(cacheConfigurations); - return new RedisCacheManagerConfigurator(cacheWriter, defaultCacheConfiguration, cacheConfigMap, - enableTransactions); + + this.intialCaches = cacheConfigMap; + + return this; } /** @@ -293,27 +324,12 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager * * @return new instance of {@link RedisCacheManager}. */ - public RedisCacheManager createAndGet() { + public RedisCacheManager build() { - RedisCacheManager cm = new RedisCacheManager(cacheWriter, defaultCacheConfiguration, intialCaches) { - - boolean hasAlreadyBeenSet; - - @Override - public void setTransactionAware(boolean transactionAware) { - - if (hasAlreadyBeenSet) { - throw new IllegalStateException( - String.format("CacheManager transaction awareness has already been set to %s.", isTransactionAware())); - } - - super.setTransactionAware(transactionAware); - hasAlreadyBeenSet = true; - } - }; + RedisCacheManager cm = new RedisCacheManager(cacheWriter, defaultCacheConfiguration, intialCaches); cm.setTransactionAware(enableTransactions); - cm.afterPropertiesSet(); // is this a good idea? Still need to think about it. + return cm; } } diff --git a/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java b/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java index 87dee818d..7f1527f0e 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCacheWriter.java @@ -17,17 +17,47 @@ package org.springframework.data.redis.cache; import java.time.Duration; +import org.springframework.data.redis.connection.RedisConnectionFactory; +import org.springframework.util.Assert; + /** - * {@link RedisCacheWriter} provides low level access to redis commands ({@code SET, SETNX, GET, EXPIRE,...} used for + * {@link RedisCacheWriter} provides low level access to Redis commands ({@code SET, SETNX, GET, EXPIRE,...}) used for * caching.
    * The {@link RedisCacheWriter} may be shared by multiple cache implementations and is responsible for writing / reading * binary data to / from Redis. The implementation honors potential cache lock flags that might be set. * * @author Christoph Strobl + * @author Mark Paluch * @since 2.0 */ public interface RedisCacheWriter { + /** + * Create new {@link RedisCacheWriter} without locking behavior. + * + * @param connectionFactory must not be {@literal null}. + * @return new instance of {@link DefaultRedisCacheWriter}. + */ + static RedisCacheWriter nonLockingRedisCacheWriter(RedisConnectionFactory connectionFactory) { + + Assert.notNull(connectionFactory, "ConnectionFactory must not be null!"); + + return new DefaultRedisCacheWriter(connectionFactory); + } + + /** + * Create new {@link RedisCacheWriter} with locking behavior. + * + * @param connectionFactory must not be {@literal null}. + * @return new instance of {@link DefaultRedisCacheWriter}. + */ + static RedisCacheWriter lockingRedisCacheWriter(RedisConnectionFactory connectionFactory) { + + Assert.notNull(connectionFactory, "ConnectionFactory must not be null!"); + + return new DefaultRedisCacheWriter(connectionFactory, Duration.ofMillis(50)); + } + /** * Write the given key/value pair to Redis an set the expiration time if defined. * @@ -57,7 +87,6 @@ public interface RedisCacheWriter { * @param ttl Optional expiration time. Can be {@literal null}. * @return {@literal null} if the value has been written, the value stored for the key if it already exists. */ - byte[] putIfAbsent(String name, byte[] key, byte[] value, Duration ttl); /** @@ -75,5 +104,4 @@ public interface RedisCacheWriter { * @param pattern The pattern for the keys to remove. Must not be {@literal null}. */ void clean(String name, byte[] pattern); - } diff --git a/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java b/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java index 811e64269..9a107cf08 100644 --- a/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java +++ b/src/test/java/org/springframework/data/redis/cache/DefaultRedisCacheWriterTests.java @@ -16,12 +16,15 @@ package org.springframework.data.redis.cache; import static org.assertj.core.api.Assertions.*; -import static org.springframework.data.redis.cache.DefaultRedisCacheWriter.*; +import static org.springframework.data.redis.cache.RedisCacheWriter.*; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Collection; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import org.junit.AfterClass; @@ -39,6 +42,7 @@ import org.springframework.data.redis.core.types.Expiration; /** * @author Christoph Strobl + * @author Mark Paluch */ @RunWith(Parameterized.class) public class DefaultRedisCacheWriterTests { @@ -47,9 +51,9 @@ public class DefaultRedisCacheWriterTests { String key = "key-1"; String cacheKey = CACHE_NAME + "::" + key; - byte[] binaryCacheKey = cacheKey.getBytes(Charset.forName("UTF-8")); - byte[] binaryCacheValue = "value".getBytes(Charset.forName("UTF-8")); + byte[] binaryCacheKey = cacheKey.getBytes(StandardCharsets.UTF_8); + byte[] binaryCacheValue = "value".getBytes(StandardCharsets.UTF_8); RedisConnectionFactory connectionFactory; @@ -211,7 +215,7 @@ public class DefaultRedisCacheWriterTests { @Test // DATAREDIS-481 public void nonLockingCacheWriterShouldIgnoreExistingLock() { - lockingRedisCacheWriter(connectionFactory).lock(CACHE_NAME); + ((DefaultRedisCacheWriter) lockingRedisCacheWriter(connectionFactory)).lock(CACHE_NAME); nonLockingRedisCacheWriter(connectionFactory).put(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO); @@ -223,7 +227,7 @@ public class DefaultRedisCacheWriterTests { @Test // DATAREDIS-481 public void lockingCacheWriterShouldIgnoreExistingLockOnDifferenceCache() { - lockingRedisCacheWriter(connectionFactory).lock(CACHE_NAME); + ((DefaultRedisCacheWriter) lockingRedisCacheWriter(connectionFactory)).lock(CACHE_NAME); lockingRedisCacheWriter(connectionFactory).put(CACHE_NAME + "-no-the-other-cache", binaryCacheKey, binaryCacheValue, Duration.ZERO); @@ -236,15 +240,25 @@ public class DefaultRedisCacheWriterTests { @Test // DATAREDIS-481 public void lockingCacheWriterShouldWaitForLockRelease() throws InterruptedException { - DefaultRedisCacheWriter cw = lockingRedisCacheWriter(connectionFactory); + DefaultRedisCacheWriter cw = (DefaultRedisCacheWriter) lockingRedisCacheWriter(connectionFactory); cw.lock(CACHE_NAME); + CountDownLatch beforeWrite = new CountDownLatch(1); + CountDownLatch afterWrite = new CountDownLatch(1); + Thread th = new Thread(() -> { - lockingRedisCacheWriter(connectionFactory).put(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO); + + RedisCacheWriter writer = lockingRedisCacheWriter(connectionFactory); + beforeWrite.countDown(); + writer.put(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO); + afterWrite.countDown(); }); th.start(); try { + + beforeWrite.await(); + Thread.sleep(200); doWithConnection(connection -> { @@ -253,8 +267,7 @@ public class DefaultRedisCacheWriterTests { cw.unlock(CACHE_NAME); - Thread.sleep(200); - + afterWrite.await(); doWithConnection(connection -> { assertThat(connection.exists(binaryCacheKey)).isTrue(); }); @@ -263,7 +276,49 @@ public class DefaultRedisCacheWriterTests { } } - void doWithConnection(Consumer callback) { + @Test // DATAREDIS-481 + public void lockingCacheWriterShouldExitWhenInterruptedWaitForLockRelease() throws InterruptedException { + + DefaultRedisCacheWriter cw = (DefaultRedisCacheWriter) lockingRedisCacheWriter(connectionFactory); + cw.lock(CACHE_NAME); + + CountDownLatch beforeWrite = new CountDownLatch(1); + CountDownLatch afterWrite = new CountDownLatch(1); + AtomicReference exceptionRef = new AtomicReference<>(); + + Thread th = new Thread(() -> { + + DefaultRedisCacheWriter writer = new DefaultRedisCacheWriter(connectionFactory, Duration.ofMillis(50)) { + + @Override + boolean doCheckLock(String name, RedisConnection connection) { + beforeWrite.countDown(); + return super.doCheckLock(name, connection); + } + }; + + try { + writer.put(CACHE_NAME, binaryCacheKey, binaryCacheValue, Duration.ZERO); + } catch (Exception e) { + exceptionRef.set(e); + } finally { + afterWrite.countDown(); + } + }); + + th.start(); + beforeWrite.await(); + + th.interrupt(); + + afterWrite.await(); + + assertThat(exceptionRef.get()).hasMessageContaining("Interrupted while waiting to unlock") + .hasCauseInstanceOf(InterruptedException.class); + } + + private void doWithConnection(Consumer callback) { + RedisConnection connection = connectionFactory.getConnection(); try { callback.accept(connection); @@ -271,5 +326,4 @@ public class DefaultRedisCacheWriterTests { connection.close(); } } - } diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java index 4f8fdfc27..a1968f53f 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java @@ -25,12 +25,13 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.cache.Cache; import org.springframework.cache.transaction.TransactionAwareCacheDecorator; -import org.springframework.context.annotation.Bean; -import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.test.util.ReflectionTestUtils; /** + * Unit tests for {@link RedisCacheManager}. + * * @author Christoph Strobl + * @author Mark Paluch */ @RunWith(MockitoJUnitRunner.class) public class RedisCacheManagerUnitTests { @@ -42,17 +43,39 @@ public class RedisCacheManagerUnitTests { RedisCacheConfiguration configuration = RedisCacheConfiguration.defaultCacheConfig().disableKeyPrefix(); - RedisCacheManager cm = RedisCacheManager.usingCacheWriter(cacheWriter).withCacheDefaults(configuration).createAndGet(); + RedisCacheManager cm = RedisCacheManager.builder(cacheWriter).cacheDefaults(configuration).build(); + cm.afterPropertiesSet(); + assertThat(cm.getMissingCache("new-cache").getCacheConfiguration()).isEqualTo(configuration); } + @Test // DATAREDIS-481 + public void appliesDefaultConfigurationToInitialCache() { + + RedisCacheConfiguration withPrefix = RedisCacheConfiguration.defaultCacheConfig().disableKeyPrefix(); + RedisCacheConfiguration withoutPrefix = RedisCacheConfiguration.defaultCacheConfig().disableKeyPrefix(); + + RedisCacheManager cm = RedisCacheManager.builder(cacheWriter).cacheDefaults(withPrefix) // + .initialCacheNames(Collections.singleton("first-cache")) // + .cacheDefaults(withoutPrefix) // + .initialCacheNames(Collections.singleton("second-cache")) // + .build(); + + cm.afterPropertiesSet(); + + assertThat(((RedisCache) cm.getCache("first-cache")).getCacheConfiguration()).isEqualTo(withPrefix); + assertThat(((RedisCache) cm.getCache("second-cache")).getCacheConfiguration()).isEqualTo(withoutPrefix); + assertThat(((RedisCache) cm.getCache("other-cache")).getCacheConfiguration()).isEqualTo(withoutPrefix); + } + @Test // DATAREDIS-481 public void predefinedCacheShouldBeCreatedWithSpecificConfig() { RedisCacheConfiguration configuration = RedisCacheConfiguration.defaultCacheConfig().disableKeyPrefix(); - RedisCacheManager cm = RedisCacheManager.usingCacheWriter(cacheWriter) - .withInitialCacheConfigurations(Collections.singletonMap("predefined-cache", configuration)).createAndGet(); + RedisCacheManager cm = RedisCacheManager.builder(cacheWriter) + .withInitialCacheConfigurations(Collections.singletonMap("predefined-cache", configuration)).build(); + cm.afterPropertiesSet(); assertThat(((RedisCache) cm.getCache("predefined-cache")).getCacheConfiguration()).isEqualTo(configuration); assertThat(cm.getMissingCache("new-cache").getCacheConfiguration()).isNotEqualTo(configuration); @@ -61,20 +84,9 @@ public class RedisCacheManagerUnitTests { @Test // DATAREDIS-481 public void transactionAwareCacheManagerShouldDecoracteCache() { - Cache cache = RedisCacheManager.usingCacheWriter(cacheWriter).transactionAware().createAndGet() - .getCache("decoracted-cache"); + Cache cache = RedisCacheManager.builder(cacheWriter).transactionAware().build().getCache("decoracted-cache"); assertThat(cache).isInstanceOfAny(TransactionAwareCacheDecorator.class); assertThat(ReflectionTestUtils.getField(cache, "targetCache")).isInstanceOf(RedisCache.class); } - - @Bean - public RedisCacheManager cacheManager(RedisConnectionFactory connectionFactory) { - return RedisCacheManager.usingRawConnectionFactory(connectionFactory) - .withCacheDefaults(RedisCacheConfiguration.defaultCacheConfig()) - .withInitialCacheConfigurations(Collections.singletonMap("predefined", RedisCacheConfiguration.defaultCacheConfig().disableCachingNullValues())) - .transactionAware() - .createAndGet(); - } - } diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java index dadda93b7..67d98cf9e 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheTests.java @@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.*; import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import lombok.RequiredArgsConstructor; import java.io.Serializable; import java.nio.charset.Charset; @@ -34,6 +35,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; import org.springframework.cache.Cache.ValueWrapper; +import org.springframework.cache.interceptor.SimpleKey; import org.springframework.cache.support.NullValue; import org.springframework.data.redis.ConnectionFactoryTracker; import org.springframework.data.redis.connection.RedisConnection; @@ -41,13 +43,13 @@ import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.data.redis.serializer.JdkSerializationRedisSerializer; import org.springframework.data.redis.serializer.RedisSerializationContext.SerializationPair; import org.springframework.data.redis.serializer.RedisSerializer; -import org.springframework.test.util.ReflectionTestUtils; /** * Tests for {@link RedisCache} with {@link DefaultRedisCacheWriter} using different {@link RedisSerializer} and * {@link RedisConnectionFactory} pairs. * * @author Christoph Strobl + * @author Mark Paluch */ @RunWith(Parameterized.class) public class RedisCacheTests { @@ -59,8 +61,7 @@ public class RedisCacheTests { Person sample = new Person("calmity", new Date()); byte[] binarySample; - NullValue nullValue = NullValue.class.cast(ReflectionTestUtils.getField(NullValue.class, "INSTANCE")); - byte[] binaryNullValue = new JdkSerializationRedisSerializer().serialize(nullValue); + byte[] binaryNullValue = new JdkSerializationRedisSerializer().serialize(NullValue.INSTANCE); RedisConnectionFactory connectionFactory; RedisSerializer serializer; @@ -179,6 +180,38 @@ public class RedisCacheTests { assertThat(result.get()).isEqualTo(sample); } + @Test // DATAREDIS-481 + public void shouldReadAndWriteSimpleCacheKey() { + + SimpleKey key = new SimpleKey("param-1", "param-2"); + + cache.put(key, sample); + + ValueWrapper result = cache.get(key); + assertThat(result).isNotNull(); + assertThat(result.get()).isEqualTo(sample); + } + + @Test(expected = IllegalStateException.class) // DATAREDIS-481 + public void shouldRejectNonInvalidKey() { + + InvalidKey key = new InvalidKey(sample.getFirstame(), sample.getBirthdate()); + + cache.put(key, sample); + } + + @Test // DATAREDIS-481 + public void shouldAllowComplexKeyWithToStringMethod() { + + ComplexKey key = new ComplexKey(sample.getFirstame(), sample.getBirthdate()); + + cache.put(key, sample); + + ValueWrapper result = cache.get(key); + assertThat(result).isNotNull(); + assertThat(result.get()).isEqualTo(sample); + } + @Test // DATAREDIS-481 public void getShouldReturnNullWhenKeyDoesNotExist() { assertThat(cache.get(key)).isNull(); @@ -253,4 +286,16 @@ public class RedisCacheTests { Date birthdate; } + @RequiredArgsConstructor // toString not overridden + static class InvalidKey implements Serializable { + final String firstame; + final Date birthdate; + } + + @Data + @RequiredArgsConstructor + static class ComplexKey implements Serializable { + final String firstame; + final Date birthdate; + } }