From b1f6bc769870eb9e06a158c95beab92edea1a08f Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
Date: Wed, 31 Jan 2018 12:59:26 +0100
Subject: [PATCH] DATAREDIS-765 - Enable pooling when configuring Jedis to use
 Redis Sentinel.

We now enable pool usage when configuring JedisConnectionFactory with RedisSentinelConfiguration to prevent accidental connections using the non-pooled standalone configuration. Jedis can operate with Sentinel only with pooling.

We also reject calls to disable pooling when JedisConnectionFactory is configured to use Sentinel.

Original Pull request: #307
---
 .../connection/jedis/JedisConnectionFactory.java    | 13 +++++++++++++
 ...isConnectionFactorySentinelIntegrationTests.java | 10 +++++++---
 .../jedis/JedisConnectionFactoryUnitTests.java      |  8 ++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java
index b86dc3869..e3d778064 100644
--- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java
+++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java
@@ -660,6 +660,12 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
 	 * @return the use of connection pooling.
 	 */
 	public boolean getUsePool() {
+
+		// Jedis Sentinel cannot operate without a pool.
+		if (isRedisSentinelAware()) {
+			return true;
+		}
+
 		return clientConfiguration.isUsePooling();
 	}
 
@@ -669,9 +675,16 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
 	 * @param usePool the usePool to set.
 	 * @deprecated since 2.0, configure pooling usage with {@link JedisClientConfiguration}.
 	 * @throws IllegalStateException if {@link JedisClientConfiguration} is immutable.
+	 * @throws IllegalStateException if configured to use sentinel and {@code usePool} is {@literal false} as Jedis
+	 *           requires pooling for Redis sentinel use.
 	 */
 	@Deprecated
 	public void setUsePool(boolean usePool) {
+
+		if (isRedisSentinelAware() && !usePool) {
+			throw new IllegalStateException("Jedis requires pooling for Redis Sentinel use!");
+		}
+
 		getMutableConfiguration().setUsePooling(usePool);
 	}
 
diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java
index 353217b61..7e50e6322 100644
--- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java
+++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java
@@ -15,12 +15,13 @@
  */
 package org.springframework.data.redis.connection.jedis;
 
-import static org.hamcrest.core.IsEqual.*;
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 
 import org.junit.After;
 import org.junit.Rule;
 import org.junit.Test;
+import org.springframework.data.redis.connection.RedisConnection;
 import org.springframework.data.redis.connection.RedisSentinelConfiguration;
 import org.springframework.data.redis.test.util.RedisSentinelRule;
 
@@ -47,7 +48,7 @@ public class JedisConnectionFactorySentinelIntegrationTests {
 		}
 	}
 
-	@Test // DATAREDIS-574
+	@Test // DATAREDIS-574, DATAREDIS-765
 	public void shouldInitializeWithSentinelConfiguration() {
 
 		JedisClientConfiguration clientConfiguration = JedisClientConfiguration.builder() //
@@ -57,7 +58,10 @@ public class JedisConnectionFactorySentinelIntegrationTests {
 		factory = new JedisConnectionFactory(SENTINEL_CONFIG, clientConfiguration);
 		factory.afterPropertiesSet();
 
-		assertThat(factory.getConnection().getClientName(), equalTo("clientName"));
+		RedisConnection connection = factory.getConnection();
+
+		assertThat(factory.getUsePool(), is(true));
+		assertThat(connection.getClientName(), equalTo("clientName"));
 	}
 
 	@Test // DATAREDIS-324
diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java
index 48dbfe82b..15b6e28d3 100644
--- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java
+++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java
@@ -75,6 +75,14 @@ public class JedisConnectionFactoryUnitTests {
 		verify(connectionFactory, never()).createRedisSentinelPool(any(RedisSentinelConfiguration.class));
 	}
 
+	@Test(expected = IllegalStateException.class) // DATAREDIS-765
+	public void shouldRejectPoolDisablingWhenSentinelConfigPresent() {
+
+		connectionFactory = new JedisConnectionFactory(new RedisSentinelConfiguration());
+
+		connectionFactory.setUsePool(false);
+	}
+
 	@Test // DATAREDIS-315
 	public void shouldInitConnectionCorrectlyWhenClusterConfigPresent() {