Guard non-initialized RedisConnectionFactory.getConnection() with IllegalStateException.

Obtaining a Connection from RedisConnectionFactory and its reactive variant is now guarded by IllegalStateException that is thrown if the connection factory was not yet initialized.

Previously, connections could be obtained where configuration was partially applied.

Closes #2057

Author: mp911de

Diff for: src/main/java/org/springframework/data/redis/connection/ReactiveRedisConnectionFactory.java

@@ -32,13 +32,15 @@ public interface ReactiveRedisConnectionFactory extends PersistenceExceptionTran
 
 	/**
 	 * @return a reactive Redis connection.
-	 * @since 2.0.
+	 * @throws IllegalStateException if the connection factory requires initialization and the factory was not yet
+	 *           initialized.
 	 */
 	ReactiveRedisConnection getReactiveConnection();
 
 	/**
 	 * @return a reactive Redis Cluster connection.
-	 * @since 2.0
+	 * @throws IllegalStateException if the connection factory requires initialization and the factory was not yet
+	 *           initialized.
 	 */
 	ReactiveRedisClusterConnection getReactiveClusterConnection();
 }

Diff for: src/main/java/org/springframework/data/redis/connection/RedisConnectionFactory.java

@@ -30,13 +30,17 @@ public interface RedisConnectionFactory extends PersistenceExceptionTranslator {
 	 * Provides a suitable connection for interacting with Redis.
 	 *
 	 * @return connection for interacting with Redis.
+	 * @throws IllegalStateException if the connection factory requires initialization and the factory was not yet
+	 *           initialized.
 	 */
 	RedisConnection getConnection();
 
 	/**
 	 * Provides a suitable connection for interacting with Redis Cluster.
 	 *
 	 * @return
+	 * @throws IllegalStateException if the connection factory requires initialization and the factory was not yet
+	 *           initialized.
 	 * @since 1.7
 	 */
 	RedisClusterConnection getClusterConnection();
@@ -55,6 +59,8 @@ public interface RedisConnectionFactory extends PersistenceExceptionTranslator {
 	 * Provides a suitable connection for interacting with Redis Sentinel.
 	 *
 	 * @return connection for interacting with Redis Sentinel.
+	 * @throws IllegalStateException if the connection factory requires initialization and the factory was not yet
+	 *           initialized.
 	 * @since 1.4
 	 */
 	RedisSentinelConnection getSentinelConnection();

Diff for: src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java

@@ -73,6 +73,9 @@
  * <li>{@link RedisSentinelConfiguration}</li>
  * <li>{@link RedisClusterConfiguration}</li>
  * </ul>
+ * <p>
+ * This connection factory must be {@link #afterPropertiesSet() initialized} prior to {@link #getConnection obtaining
+ * connections}.
  *
  * @author Costin Leau
  * @author Thomas Darimont
@@ -104,6 +107,9 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
 	private @Nullable ClusterTopologyProvider topologyProvider;
 	private @Nullable ClusterCommandExecutor clusterCommandExecutor;
 
+	private boolean initialized;
+	private boolean destroyed;
+
 	/**
 	 * Constructs a new <code>JedisConnectionFactory</code> instance with default settings (default connection pooling, no
 	 * shard information).
@@ -350,6 +356,8 @@ public void afterPropertiesSet() {
 					new JedisClusterConnection.JedisClusterNodeResourceProvider(this.cluster, this.topologyProvider),
 					EXCEPTION_TRANSLATION);
 		}
+
+		this.initialized = true;
 	}
 
 	private JedisClientConfig createClientConfig(@Nullable String username, RedisPassword password) {
@@ -484,6 +492,8 @@ public void destroy() {
 				log.warn("Cannot properly close cluster command executor", ex);
 			}
 		}
+
+		this.destroyed = true;
 	}
 
 	/*
@@ -492,6 +502,8 @@ public void destroy() {
 	 */
 	public RedisConnection getConnection() {
 
+		assertInitialized();
+
 		if (isRedisClusterAware()) {
 			return getClusterConnection();
 		}
@@ -517,6 +529,8 @@ public RedisConnection getConnection() {
 	@Override
 	public RedisClusterConnection getClusterConnection() {
 
+		assertInitialized();
+
 		if (!isRedisClusterAware()) {
 			throw new InvalidDataAccessApiUsageException("Cluster is not configured!");
 		}
@@ -876,6 +890,8 @@ public boolean isRedisClusterAware() {
 	@Override
 	public RedisSentinelConnection getSentinelConnection() {
 
+		assertInitialized();
+
 		if (!isRedisSentinelAware()) {
 			throw new InvalidDataAccessResourceUsageException("No Sentinels configured");
 		}
@@ -945,6 +961,11 @@ private MutableJedisClientConfiguration getMutableConfiguration() {
 		return (MutableJedisClientConfiguration) clientConfiguration;
 	}
 
+	private void assertInitialized() {
+		Assert.state(this.initialized, "JedisConnectionFactory was not initialized through afterPropertiesSet()");
+		Assert.state(!this.destroyed, "JedisConnectionFactory was destroyed and cannot be used anymore");
+	}
+
 	/**
 	 * Mutable implementation of {@link JedisClientConfiguration}.
 	 *

Diff for: src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java

@@ -84,6 +84,9 @@
  * <li>{@link RedisSentinelConfiguration}</li>
  * <li>{@link RedisClusterConfiguration}</li>
  * </ul>
+ * <p>
+ * This connection factory must be {@link #afterPropertiesSet() initialized} prior to {@link #getConnection obtaining
+ * connections}.
  *
  * @author Costin Leau
  * @author Jennifer Hickey
@@ -124,6 +127,9 @@ public class LettuceConnectionFactory
 
 	private @Nullable ClusterCommandExecutor clusterCommandExecutor;
 
+	private boolean initialized;
+	private boolean destroyed;
+
 	/**
 	 * Constructs a new {@link LettuceConnectionFactory} instance with default settings.
 	 */
@@ -293,6 +299,8 @@ public void afterPropertiesSet() {
 					EXCEPTION_TRANSLATION);
 		}
 
+		this.initialized = true;
+
 		if (getEagerInitialization() && getShareNativeConnection()) {
 			initConnection();
 		}
@@ -329,6 +337,8 @@ public void destroy() {
 				log.warn("Cannot properly close cluster command executor", ex);
 			}
 		}
+
+		this.destroyed = true;
 	}
 
 	private void dispose(LettuceConnectionProvider connectionProvider) {
@@ -351,6 +361,8 @@ private void dispose(LettuceConnectionProvider connectionProvider) {
 	 */
 	public RedisConnection getConnection() {
 
+		assertInitialized();
+
 		if (isClusterAware()) {
 			return getClusterConnection();
 		}
@@ -368,6 +380,8 @@ public RedisConnection getConnection() {
 	@Override
 	public RedisClusterConnection getClusterConnection() {
 
+		assertInitialized();
+
 		if (!isClusterAware()) {
 			throw new InvalidDataAccessApiUsageException("Cluster is not configured!");
 		}
@@ -437,6 +451,8 @@ protected LettuceClusterConnection doCreateLettuceClusterConnection(
 	@Override
 	public LettuceReactiveRedisConnection getReactiveConnection() {
 
+		assertInitialized();
+
 		if (isClusterAware()) {
 			return getReactiveClusterConnection();
 		}
@@ -453,6 +469,8 @@ public LettuceReactiveRedisConnection getReactiveConnection() {
 	@Override
 	public LettuceReactiveRedisClusterConnection getReactiveClusterConnection() {
 
+		assertInitialized();
+
 		if (!isClusterAware()) {
 			throw new InvalidDataAccessApiUsageException("Cluster is not configured!");
 		}
@@ -481,6 +499,8 @@ public void initConnection() {
 	 */
 	public void resetConnection() {
 
+		assertInitialized();
+
 		Optionals.toStream(Optional.ofNullable(connection), Optional.ofNullable(reactiveConnection))
 				.forEach(SharedConnection::resetConnection);
 
@@ -496,6 +516,8 @@ public void resetConnection() {
 	 */
 	public void validateConnection() {
 
+		assertInitialized();
+
 		getOrCreateSharedConnection().validateConnection();
 		getOrCreateSharedReactiveConnection().validateConnection();
 	}
@@ -801,6 +823,7 @@ public void setClientName(@Nullable String clientName) {
 	 */
 	@Nullable
 	public AbstractRedisClient getNativeClient() {
+		assertInitialized();
 		return this.client;
 	}
 
@@ -1167,6 +1190,11 @@ private RedisURI getSentinelRedisURI() {
 		return redisUri;
 	}
 
+	private void assertInitialized() {
+		Assert.state(this.initialized, "LettuceConnectionFactory was not initialized through afterPropertiesSet()");
+		Assert.state(!this.destroyed, "LettuceConnectionFactory was destroyed and cannot be used anymore");
+	}
+
 	private static void applyToAll(RedisURI source, Consumer<RedisURI> action) {
 
 		action.accept(source);
@@ -1214,6 +1242,9 @@ private void applyAuthentication(RedisURI.Builder builder) {
 
 	@Override
 	public RedisSentinelConnection getSentinelConnection() {
+
+		assertInitialized();
+
 		return new LettuceSentinelConnection(connectionProvider);
 	}
 

Diff for: src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java

@@ -90,6 +90,7 @@ void shouldNotFailOnFirstSentinelDown() {
 				.sentinel("127.0.0.1", 1).sentinel("127.0.0.1", 26379);
 
 		factory = new JedisConnectionFactory(oneDownSentinelConfig);
+		factory.afterPropertiesSet();
 		assertThat(factory.getSentinelConnection().isOpen()).isTrue();
 	}
 }

Diff for: src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java

@@ -346,6 +346,16 @@ void shouldDenyChangesToImmutableClientConfiguration() throws NoSuchAlgorithmExc
 		assertThatIllegalStateException().isThrownBy(() -> connectionFactory.setClientName("foo"));
 	}
 
+	@Test // GH-2057
+	void getConnectionShouldFailIfNotInitialized() {
+
+		JedisConnectionFactory connectionFactory = new JedisConnectionFactory();
+
+		assertThatIllegalStateException().isThrownBy(connectionFactory::getConnection);
+		assertThatIllegalStateException().isThrownBy(connectionFactory::getClusterConnection);
+		assertThatIllegalStateException().isThrownBy(connectionFactory::getSentinelConnection);
+	}
+
 	private JedisConnectionFactory initSpyedConnectionFactory(RedisSentinelConfiguration sentinelConfig,
 			JedisPoolConfig poolConfig) {
 

Diff for: src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactoryUnitTests.java

@@ -1011,7 +1011,19 @@ void getNativeClientShouldFailIfNotInitialized() {
 		LettuceConnectionFactory connectionFactory = new LettuceConnectionFactory();
 
 		assertThatIllegalStateException().isThrownBy(connectionFactory::getRequiredNativeClient)
-				.withMessageContaining("Client not yet initialized");
+				.withMessageContaining("was not initialized through");
+	}
+
+	@Test // GH-2057
+	void getConnectionShouldFailIfNotInitialized() {
+
+		LettuceConnectionFactory connectionFactory = new LettuceConnectionFactory();
+
+		assertThatIllegalStateException().isThrownBy(connectionFactory::getConnection);
+		assertThatIllegalStateException().isThrownBy(connectionFactory::getClusterConnection);
+		assertThatIllegalStateException().isThrownBy(connectionFactory::getSentinelConnection);
+		assertThatIllegalStateException().isThrownBy(connectionFactory::getReactiveConnection);
+		assertThatIllegalStateException().isThrownBy(connectionFactory::getReactiveClusterConnection);
 	}
 
 	@Data