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.

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<String, Long> expires = null;
 
+	private Set<String> configuredCacheNames;
+
 	/**
 	 * Construct a {@link RedisCacheManager}.
 	 * 
@@ -107,12 +110,11 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager
 	 */
 	public void setCacheNames(Collection<String> cacheNames) {
 
-		if (!CollectionUtils.isEmpty(cacheNames)) {
+		Set<String> newCacheNames = CollectionUtils.isEmpty(cacheNames) ? Collections.<String> emptySet()
-			for (String cacheName : cacheNames) {
+				: new HashSet<String>(cacheNames);
-				createAndAddCache(cacheName);
+
-			}
+		this.configuredCacheNames = newCacheNames;
-			this.dynamic = false;
+		this.dynamic = newCacheNames.isEmpty();
-		}
 	}
 
 	public void setUsePrefix(boolean usePrefix) {
@@ -277,4 +279,25 @@ public class RedisCacheManager extends AbstractTransactionSupportingCacheManager
 	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();
+	}
 }

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.<byte[]> 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<byte[]> keys = new HashSet<byte[]>(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();
 

src/test/java/org/springframework/data/redis/cache/RedisCacheTest.java

@@ -16,18 +16,14 @@
 
 package org.springframework.data.redis.cache;
 
-import static org.hamcrest.core.IsEqual.equalTo;
+import static org.hamcrest.core.IsEqual.*;
-import static org.hamcrest.core.IsInstanceOf.instanceOf;
+import static org.hamcrest.core.IsInstanceOf.*;
-import static org.hamcrest.core.IsNot.not;
+import static org.hamcrest.core.IsNot.*;
-import static org.hamcrest.core.IsNull.nullValue;
+import static org.hamcrest.core.IsNull.*;
-import static org.hamcrest.core.IsSame.sameInstance;
+import static org.hamcrest.core.IsSame.*;
-import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertNotNull;
+import static org.junit.Assume.*;
-import static org.junit.Assert.assertNull;
+import static org.springframework.data.redis.matcher.RedisTestMatchers.*;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeThat;
-import static org.springframework.data.redis.matcher.RedisTestMatchers.isEqual;
 
 import java.util.Collection;
 import java.util.concurrent.CountDownLatch;
@@ -42,7 +38,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;
@@ -150,7 +145,10 @@ public class RedisCacheTest extends AbstractNativeCacheTest<RedisTemplate> {
 
 	@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);
@@ -258,8 +256,8 @@ public class RedisCacheTest extends AbstractNativeCacheTest<RedisTemplate> {
 
 		assumeThat(cache, instanceOf(RedisCache.class));
 
-		RedisCache redisCache = (RedisCache)cache;
+		RedisCache redisCache = (RedisCache) cache;
-		
+
 		Object key = getKey();
 		template.delete(key);