From 90a96fe40eb6691e115ce9bd35a0b2e0fd20c00f Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Tue, 3 Feb 2015 10:45:36 +0100 Subject: [PATCH] DATAREDIS-328 - RedisCacheManager should not instantiate caches in setCacheNames(). We now construct the caches for the configured cache-names in afterPropertiesSet(). Previously the caches were created in the setter which lead to unwanted property-set order dependencies. Original pull request: #123. --- .../data/redis/cache/RedisCacheManager.java | 48 ++++++++++++++++--- .../cache/RedisCacheManagerUnitTests.java | 21 ++++++-- .../data/redis/cache/RedisCacheTest.java | 14 +++--- 3 files changed, 66 insertions(+), 17 deletions(-) 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 82ca0ae07..65d2ed99a 100644 --- a/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java +++ b/src/main/java/org/springframework/data/redis/cache/RedisCacheManager.java @@ -18,6 +18,7 @@ package org.springframework.data.redis.cache; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -67,6 +68,8 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager private long defaultExpiration = 0; private Map expires = null; + private Set configuredCacheNames; + /** * Construct a {@link RedisCacheManager}. * @@ -107,12 +110,11 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager */ public void setCacheNames(Collection cacheNames) { - if (!CollectionUtils.isEmpty(cacheNames)) { - for (String cacheName : cacheNames) { - createAndAddCache(cacheName); - } - this.dynamic = false; - } + Set newCacheNames = CollectionUtils.isEmpty(cacheNames) ? Collections. emptySet() + : new HashSet(cacheNames); + + this.configuredCacheNames = newCacheNames; + this.dynamic = newCacheNames.isEmpty(); } public void setUsePrefix(boolean usePrefix) { @@ -264,4 +266,38 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager } }); } + + @SuppressWarnings("rawtypes") + protected RedisTemplate getTemplate() { + return template; + } + + protected RedisCachePrefix getCachePrefix() { + return cachePrefix; + } + + protected boolean isUsePrefix() { + return usePrefix; + } + + /** + * The number of caches and their names will be fixed after a call to this method, with no creation of further cache + * regions at runtime. + * + * @see org.springframework.cache.support.AbstractCacheManager#afterPropertiesSet() + */ + @Override + public void afterPropertiesSet() { + + if (!CollectionUtils.isEmpty(configuredCacheNames)) { + + for (String cacheName : configuredCacheNames) { + createAndAddCache(cacheName); + } + + configuredCacheNames.clear(); + } + + super.afterPropertiesSet(); + } } 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 ab7e8e3cb..8b06c4cdb 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheManagerUnitTests.java @@ -65,6 +65,7 @@ public class RedisCacheManagerUnitTests { redisTemplate.afterPropertiesSet(); cacheManager = new RedisCacheManager(redisTemplate); + cacheManager.afterPropertiesSet(); } /** @@ -92,8 +93,6 @@ public class RedisCacheManagerUnitTests { */ @Test public void testCacheInitSouldNotRequestRemoteKeysByDefault() { - - cacheManager.afterPropertiesSet(); Mockito.verifyZeroInteractions(redisConnectionMock); } @@ -103,6 +102,7 @@ public class RedisCacheManagerUnitTests { @Test public void testCacheInitShouldFetchAllCacheKeysWhenLoadingRemoteCachesOnStartupIsEnabled() { + cacheManager = new RedisCacheManager(redisTemplate); cacheManager.setLoadRemoteCachesOnStartup(true); cacheManager.afterPropertiesSet(); @@ -122,6 +122,7 @@ public class RedisCacheManagerUnitTests { .serialize("remote-cache~keys"))); when(redisConnectionMock.keys(any(byte[].class))).thenReturn(keys); + cacheManager = new RedisCacheManager(redisTemplate); cacheManager.setLoadRemoteCachesOnStartup(true); cacheManager.afterPropertiesSet(); @@ -135,6 +136,8 @@ public class RedisCacheManagerUnitTests { public void testCacheInitShouldNotInitialzeCachesWhenLoadingRemoteCachesOnStartupIsEnabledAndNoCachesAvailableOnRemoteServer() { when(redisConnectionMock.keys(any(byte[].class))).thenReturn(Collections. emptySet()); + + cacheManager = new RedisCacheManager(redisTemplate); cacheManager.setLoadRemoteCachesOnStartup(true); cacheManager.afterPropertiesSet(); @@ -147,7 +150,10 @@ public class RedisCacheManagerUnitTests { @Test public void testCacheManagerShouldNotDynamicallyCreateCachesWhenInStaticMode() { + cacheManager = new RedisCacheManager(redisTemplate); cacheManager.setCacheNames(Arrays.asList("spring", "data")); + cacheManager.afterPropertiesSet(); + assertThat(cacheManager.getCache("redis"), nullValue()); } @@ -157,7 +163,10 @@ public class RedisCacheManagerUnitTests { @Test public void testCacheManagerShouldRetrunRegisteredCacheWhenInStaticMode() { + cacheManager = new RedisCacheManager(redisTemplate); cacheManager.setCacheNames(Arrays.asList("spring", "data")); + cacheManager.afterPropertiesSet(); + assertThat(cacheManager.getCache("spring"), notNullValue()); } @@ -167,8 +176,11 @@ public class RedisCacheManagerUnitTests { @Test public void testPuttingCacheManagerIntoStaticModeShouldNotRemoveAlreadyRegisteredCaches() { + cacheManager = new RedisCacheManager(redisTemplate); cacheManager.getCache("redis"); cacheManager.setCacheNames(Arrays.asList("spring", "data")); + cacheManager.afterPropertiesSet(); + assertThat(cacheManager.getCache("redis"), notNullValue()); } @@ -178,6 +190,7 @@ public class RedisCacheManagerUnitTests { @Test public void testRetainConfiguredCachesAfterBeanInitialization() { + cacheManager = new RedisCacheManager(redisTemplate); cacheManager.setCacheNames(Arrays.asList("spring", "data")); cacheManager.afterPropertiesSet(); @@ -191,10 +204,12 @@ public class RedisCacheManagerUnitTests { @Test public void testRetainConfiguredCachesAfterBeanInitializationWithLoadingOfRemoteKeys() { - cacheManager.setCacheNames(Arrays.asList("spring", "data")); Set keys = new HashSet(Arrays.asList(redisTemplate.getKeySerializer() .serialize("remote-cache~keys"))); when(redisConnectionMock.keys(any(byte[].class))).thenReturn(keys); + + cacheManager = new RedisCacheManager(redisTemplate); + cacheManager.setCacheNames(Arrays.asList("spring", "data")); cacheManager.setLoadRemoteCachesOnStartup(true); cacheManager.afterPropertiesSet(); diff --git a/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java b/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java index 7400a6e73..8f1880720 100644 --- a/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java +++ b/src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java @@ -18,13 +18,9 @@ package org.springframework.data.redis.cache; import static org.hamcrest.core.IsInstanceOf.*; import static org.hamcrest.core.IsNull.*; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import static org.junit.Assume.*; -import static org.springframework.data.redis.matcher.RedisTestMatchers.isEqual; +import static org.springframework.data.redis.matcher.RedisTestMatchers.*; import java.util.Collection; import java.util.concurrent.CountDownLatch; @@ -39,7 +35,6 @@ import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; import org.springframework.cache.Cache; import org.springframework.cache.Cache.ValueWrapper; -import org.springframework.cache.CacheManager; import org.springframework.data.redis.ConnectionFactoryTracker; import org.springframework.data.redis.ObjectFactory; import org.springframework.data.redis.core.AbstractOperationsTestParams; @@ -147,7 +142,10 @@ public class RedisCacheTest extends AbstractNativeCacheTest { @Test public void testCacheName() throws Exception { - CacheManager redisCM = new RedisCacheManager(template); + + RedisCacheManager redisCM = new RedisCacheManager(template); + redisCM.afterPropertiesSet(); + String cacheName = "s2gx11"; Cache cache = redisCM.getCache(cacheName); assertNotNull(cache);