Polish Redis sentinel support

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/redis/RedisAutoConfiguration.java

@@ -38,6 +38,7 @@ import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
 import org.springframework.data.redis.core.RedisOperations;
 import org.springframework.data.redis.core.RedisTemplate;
 import org.springframework.data.redis.core.StringRedisTemplate;
+import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
 import redis.clients.jedis.Jedis;
@@ -50,6 +51,7 @@ import redis.clients.jedis.JedisPoolConfig;
  * @author Andy Wilkinson
  * @author Christian Dupuis
  * @author Christoph Strobl
+ * @author Phillip Webb
  */
 @Configuration
 @ConditionalOnClass({ JedisConnection.class, RedisOperations.class, Jedis.class })
@@ -62,7 +64,10 @@ public class RedisAutoConfiguration {
 		return new RedisProperties();
 	}
 
-	protected abstract static class RedisHAConnectionConfiguration {
+	/**
+	 * Base class for Redis configurations.
+	 */
+	protected static abstract class AbstractRedisConfiguration {
 
 		@Autowired
 		protected RedisProperties properties;
@@ -70,78 +75,87 @@ public class RedisAutoConfiguration {
 		@Autowired(required = false)
 		private RedisSentinelConfiguration sentinelConfiguration;
 
-		protected RedisSentinelConfiguration potentiallyGetSentinelConfig() {
+		protected final JedisConnectionFactory applyProperties(
-
+				JedisConnectionFactory factory) {
-			if (this.sentinelConfiguration == null
+			factory.setHostName(this.properties.getHost());
-					&& this.properties.getSentinel() == null) {
+			factory.setPort(this.properties.getPort());
-				return null;
+			if (this.properties.getPassword() != null) {
+				factory.setPassword(this.properties.getPassword());
 			}
-
+			factory.setDatabase(this.properties.getDatabase());
-			RedisSentinelConfiguration sentinelConfig = this.sentinelConfiguration;
+			return factory;
-			if (sentinelConfig == null && this.properties.getSentinel() != null) {
-				sentinelConfig = new RedisSentinelConfiguration().master(this.properties
-						.getSentinel().getMaster());
-				sentinelConfig.setSentinels(createRedisNodesForSentinel(this.properties
-						.getSentinel()));
-			}
-			return sentinelConfig;
 		}
 
-		private List<RedisNode> createRedisNodesForSentinel(Sentinel sentinel) {
+		protected final RedisSentinelConfiguration getSentinelConfig() {
-
+			if (this.sentinelConfiguration != null) {
-			String[] nodeStrings = StringUtils.commaDelimitedListToStringArray(sentinel
+				return this.sentinelConfiguration;
-					.getNodes());
-
-			List<RedisNode> nodes = new ArrayList<RedisNode>(nodeStrings.length);
-
-			for (String hostAndPort : nodeStrings) {
-				String[] args = StringUtils.split(hostAndPort, ":");
-				nodes.add(new RedisNode(args[0], Integer.valueOf(args[1])));
 			}
+			Sentinel sentinelProperties = this.properties.getSentinel();
+			if (sentinelProperties != null) {
+				RedisSentinelConfiguration config = new RedisSentinelConfiguration();
+				config.master(sentinelProperties.getMaster());
+				config.setSentinels(createSentinels(sentinelProperties));
+				return config;
+			}
+			return null;
+		}
 
-			return nodes;
+		private List<RedisNode> createSentinels(Sentinel sentinel) {
+			List<RedisNode> sentinels = new ArrayList<RedisNode>();
+			String nodes = sentinel.getNodes();
+			for (String node : StringUtils.commaDelimitedListToStringArray(nodes)) {
+				try {
+					String[] parts = StringUtils.split(node, ":");
+					Assert.state(parts.length == 2, "Must be defined as 'host:port'");
+					sentinels.add(new RedisNode(parts[0], Integer.valueOf(parts[1])));
+				}
+				catch (RuntimeException ex) {
+					throw new IllegalStateException("Invalid redis sentinel "
+							+ "property '" + node + "'", ex);
+				}
+			}
+			return sentinels;
 		}
 
 	}
 
+	/**
+	 * Redis connection configuration.
+	 */
 	@Configuration
 	@ConditionalOnMissingClass(name = "org.apache.commons.pool2.impl.GenericObjectPool")
 	protected static class RedisConnectionConfiguration extends
-			RedisHAConnectionConfiguration {
+			AbstractRedisConfiguration {
 
 		@Bean
 		@ConditionalOnMissingBean
-		RedisConnectionFactory redisConnectionFactory() throws UnknownHostException {
+		public RedisConnectionFactory redisConnectionFactory()
-			JedisConnectionFactory factory = new JedisConnectionFactory(
+				throws UnknownHostException {
-					potentiallyGetSentinelConfig());
+			return applyProperties(new JedisConnectionFactory(getSentinelConfig()));
-			applyConnectionFactoryProperties(factory, this.properties);
-			return factory;
 		}
 
 	}
 
+	/**
+	 * Redis pooled connection configuration.
+	 */
 	@Configuration
 	@ConditionalOnClass(GenericObjectPool.class)
 	protected static class RedisPooledConnectionConfiguration extends
-			RedisHAConnectionConfiguration {
+			AbstractRedisConfiguration {
 
 		@Bean
 		@ConditionalOnMissingBean
-		RedisConnectionFactory redisConnectionFactory() throws UnknownHostException {
+		public RedisConnectionFactory redisConnectionFactory()
-			JedisConnectionFactory factory = createJedisConnectionFactory();
+				throws UnknownHostException {
-			applyConnectionFactoryProperties(factory, this.properties);
+			return applyProperties(createJedisConnectionFactory());
-			return factory;
 		}
 
 		private JedisConnectionFactory createJedisConnectionFactory() {
-
 			if (this.properties.getPool() != null) {
-				return new JedisConnectionFactory(potentiallyGetSentinelConfig(),
+				return new JedisConnectionFactory(getSentinelConfig(), jedisPoolConfig());
-						jedisPoolConfig());
-			}
-			else {
-				return new JedisConnectionFactory(potentiallyGetSentinelConfig());
 			}
+			return new JedisConnectionFactory(getSentinelConfig());
 		}
 
 		private JedisPoolConfig jedisPoolConfig() {
@@ -156,12 +170,12 @@ public class RedisAutoConfiguration {
 
 	}
 
+	/**
+	 * Standard Redis configuration.
+	 */
 	@Configuration
 	protected static class RedisConfiguration {
 
-		@Autowired
-		private RedisProperties properties;
-
 		@Bean
 		@ConditionalOnMissingBean(name = "redisTemplate")
 		RedisOperations<Object, Object> redisTemplate(
@@ -184,14 +198,4 @@ public class RedisAutoConfiguration {
 
 	}
 
-	static void applyConnectionFactoryProperties(JedisConnectionFactory factory,
-			RedisProperties properties) {
-		factory.setHostName(properties.getHost());
-		factory.setPort(properties.getPort());
-		if (properties.getPassword() != null) {
-			factory.setPassword(properties.getPassword());
-		}
-		factory.setDatabase(properties.getDatabase());
-	}
-
 }

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/redis/RedisProperties.java

@@ -35,9 +35,9 @@ public class RedisProperties {
 
 	private int port = 6379;
 
-	private RedisProperties.Pool pool;
+	private Pool pool;
 
-    private RedisProperties.Sentinel sentinel;
+	private Sentinel sentinel;
 
 	public String getHost() {
 		return this.host;
@@ -63,11 +63,11 @@ public class RedisProperties {
 		this.password = password;
 	}
 
-	public RedisProperties.Pool getPool() {
+	public Pool getPool() {
 		return this.pool;
 	}
 
-	public void setPool(RedisProperties.Pool pool) {
+	public void setPool(Pool pool) {
 		this.pool = pool;
 	}
 
@@ -79,16 +79,15 @@ public class RedisProperties {
 		this.database = database;
 	}
 
+	public Sentinel getSentinel() {
+		return this.sentinel;
+	}
 
 	public void setSentinel(Sentinel sentinel) {
 		this.sentinel = sentinel;
 	}
 
-	public Sentinel getSentinel() {
+	/**
-		return sentinel;
-	}
-
-    /**
 	 * Pool properties.
 	 */
 	public static class Pool {
@@ -134,28 +133,29 @@ public class RedisProperties {
 		}
 	}
 
-    /**
+	/**
-     * Properties for configuring redis sentinels.
+	 * Redis sentinel properties.
-     */
+	 */
-    public static class Sentinel {
+	public static class Sentinel {
 
-        private String master;
+		private String master;
-        private String nodes;
 
-        public String getMaster() {
+		private String nodes;
-            return master;
-        }
 
-        public void setMaster(String master) {
+		public String getMaster() {
-            this.master = master;
+			return this.master;
-        }
+		}
 
-        public String getNodes() {
+		public void setMaster(String master) {
-            return nodes;
+			this.master = master;
-        }
+		}
 
-        public void setNodes(String nodes) {
+		public String getNodes() {
-            this.nodes = nodes;
+			return this.nodes;
-        }
+		}
-    }
+
+		public void setNodes(String nodes) {
+			this.nodes = nodes;
+		}
+	}
 }

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitPropertiesTests.java

@@ -63,7 +63,7 @@ public class RabbitPropertiesTests {
 	public void addressesDoubleValuedPreservesOrder() {
 		this.properties.setAddresses("myhost:9999,ahost:1111/host");
 		assertNull(this.properties.getHost());
-		assertEquals("myhost:9999,ahost:1111", properties.getAddresses());
+		assertEquals("myhost:9999,ahost:1111", this.properties.getAddresses());
 	}
 
 	@Test