Migrate RedisConnectionFactory to Lifecycle beans.

Closes: #2503
Original pull request: #2627

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

@@ -34,6 +34,7 @@ import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLParameters;
@@ -42,9 +43,9 @@ import javax.net.ssl.SSLSocketFactory;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
-
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.context.SmartLifecycle;
 import org.springframework.dao.DataAccessException;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.dao.InvalidDataAccessResourceUsageException;
@@ -85,7 +86,7 @@ import org.springframework.util.ObjectUtils;
  * @see JedisClientConfiguration
  * @see Jedis
  */
-public class JedisConnectionFactory implements InitializingBean, DisposableBean, RedisConnectionFactory {
+public class JedisConnectionFactory implements RedisConnectionFactory, InitializingBean, DisposableBean, SmartLifecycle {
 
 	private final static Log log = LogFactory.getLog(JedisConnectionFactory.class);
 	private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new PassThroughExceptionTranslationStrategy(
@@ -104,8 +105,11 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
 	private @Nullable ClusterTopologyProvider topologyProvider;
 	private @Nullable ClusterCommandExecutor clusterCommandExecutor;
 
-	private boolean initialized;
-	private boolean destroyed;
+	enum State {
+		CREATED, STARTING, STARTED, STOPPING, STOPPED, DESTROYED;
+	}
+
+	private AtomicReference<State> state = new AtomicReference<>(State.CREATED);
 
 	/**
 	 * Constructs a new {@link JedisConnectionFactory} instance with default settings (default connection pooling).
@@ -287,24 +291,80 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
 		return connection;
 	}
 
+	@Override
+	public void start() {
+
+		State current = state.getAndUpdate(state -> {
+			if (State.CREATED.equals(state) || State.STOPPED.equals(state)) {
+				return State.STARTING;
+			}
+			return state;
+		});
+
+		if (State.CREATED.equals(current) || State.STOPPED.equals(current)) {
+
+			if (getUsePool() && !isRedisClusterAware()) {
+				this.pool = createPool();
+			}
+
+			if (isRedisClusterAware()) {
+
+				this.cluster = createCluster();
+				this.topologyProvider = createTopologyProvider(this.cluster);
+				this.clusterCommandExecutor = new ClusterCommandExecutor(this.topologyProvider,
+						new JedisClusterConnection.JedisClusterNodeResourceProvider(this.cluster, this.topologyProvider),
+						EXCEPTION_TRANSLATION);
+			}
+
+			state.set(State.STARTED);
+		}
+	}
+
+	@Override
+	public void stop() {
+
+		if (state.compareAndSet(State.STARTED, State.STOPPING)) {
+			if (getUsePool() && !isRedisClusterAware()) {
+				if (pool != null) {
+					try {
+						this.pool.close();
+					} catch (Exception ex) {
+						log.warn("Cannot properly close Jedis pool", ex);
+					}
+					this.pool = null;
+				}
+			}
+
+			if(this.clusterCommandExecutor != null) {
+				try {
+					this.clusterCommandExecutor.destroy();
+				} catch (Exception e) {
+					throw new RuntimeException(e);
+				}
+			}
+
+			if (this.cluster != null) {
+
+				this.topologyProvider = null;
+
+				try {
+					cluster.close();
+				} catch (Exception ex) {
+					log.warn("Cannot properly close Jedis cluster", ex);
+				}
+			}
+			state.set(State.STOPPED);
+		}
+	}
+
+	@Override
+	public boolean isRunning() {
+		return State.STARTED.equals(state.get());
+	}
+
+	@Override
 	public void afterPropertiesSet() {
-
 		clientConfig = createClientConfig(getDatabase(), getRedisUsername(), getRedisPassword());
-
-		if (getUsePool() && !isRedisClusterAware()) {
-			this.pool = createPool();
-		}
-
-		if (isRedisClusterAware()) {
-
-			this.cluster = createCluster();
-			this.topologyProvider = createTopologyProvider(this.cluster);
-			this.clusterCommandExecutor = new ClusterCommandExecutor(this.topologyProvider,
-					new JedisClusterConnection.JedisClusterNodeResourceProvider(this.cluster, this.topologyProvider),
-					EXCEPTION_TRANSLATION);
-		}
-
-		this.initialized = true;
 	}
 
 	JedisClientConfig createSentinelClientConfig(SentinelConfiguration sentinelConfiguration) {
@@ -415,32 +475,8 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
 
 	public void destroy() {
 
-		if (getUsePool() && pool != null) {
-
-			try {
-				pool.destroy();
-			} catch (Exception ex) {
-				log.warn("Cannot properly close Jedis pool", ex);
-			}
-			pool = null;
-		}
-
-		if (cluster != null) {
-
-			try {
-				cluster.close();
-			} catch (Exception ex) {
-				log.warn("Cannot properly close Jedis cluster", ex);
-			}
-
-			try {
-				clusterCommandExecutor.destroy();
-			} catch (Exception ex) {
-				log.warn("Cannot properly close cluster command executor", ex);
-			}
-		}
-
-		this.destroyed = true;
+		stop();
+		state.set(State.DESTROYED);
 	}
 
 	public RedisConnection getConnection() {
@@ -866,8 +902,19 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
 	}
 
 	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");
+
+		State current = state.get();
+
+		if (State.STARTED.equals(current)) {
+			return;
+		}
+
+		switch (current) {
+			case CREATED, STOPPED -> throw new IllegalStateException(String.format("JedisConnectionFactory has been %s. Use start() to initialize it", current));
+			case DESTROYED -> throw new IllegalStateException(
+					"JedisConnectionFactory was destroyed and cannot be used anymore");
+			default -> throw new IllegalStateException(String.format("JedisConnectionFactory is %s", current));
+		}
 	}
 
 	/**

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

@@ -26,11 +26,13 @@ import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.context.SmartLifecycle;
 import org.springframework.dao.DataAccessException;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.redis.ExceptionTranslationStrategy;
@@ -116,8 +118,8 @@ import org.apache.commons.logging.LogFactory;
  * @author Andrea Como
  * @author Chris Bono
  */
-public class LettuceConnectionFactory
-		implements InitializingBean, DisposableBean, RedisConnectionFactory, ReactiveRedisConnectionFactory {
+public class LettuceConnectionFactory implements RedisConnectionFactory, ReactiveRedisConnectionFactory,
+		InitializingBean, DisposableBean, SmartLifecycle {
 
 	private static final ExceptionTranslationStrategy EXCEPTION_TRANSLATION = new PassThroughExceptionTranslationStrategy(
 			LettuceExceptionConverter.INSTANCE);
@@ -144,8 +146,11 @@ public class LettuceConnectionFactory
 
 	private @Nullable ClusterCommandExecutor clusterCommandExecutor;
 
-	private boolean initialized;
-	private boolean destroyed;
+	enum State {
+		CREATED, STARTING, STARTED, STOPPING, STOPPED, DESTROYED;
+	}
+
+	private AtomicReference<State> state = new AtomicReference<>(State.CREATED);
 
 	/**
 	 * Constructs a new {@link LettuceConnectionFactory} instance with default settings.
@@ -333,33 +338,78 @@ public class LettuceConnectionFactory
 		return LettuceConverters.createRedisStandaloneConfiguration(redisUri);
 	}
 
-	public void afterPropertiesSet() {
+	@Override
+	public void start() {
 
-		this.client = createClient();
+		State current = state.getAndUpdate(state -> {
+			if (State.CREATED.equals(state) || State.STOPPED.equals(state)) {
+				return State.STARTING;
+			}
+			return state;
+		});
 
-		this.connectionProvider = new ExceptionTranslatingConnectionProvider(createConnectionProvider(client, CODEC));
-		this.reactiveConnectionProvider = new ExceptionTranslatingConnectionProvider(
-				createConnectionProvider(client, LettuceReactiveRedisConnection.CODEC));
+		if (State.CREATED.equals(current) || State.STOPPED.equals(current)) {
 
-		if (isClusterAware()) {
+			this.client = createClient();
 
-			this.clusterCommandExecutor = new ClusterCommandExecutor(
-					new LettuceClusterTopologyProvider((RedisClusterClient) client),
-					new LettuceClusterConnection.LettuceClusterNodeResourceProvider(this.connectionProvider),
-					EXCEPTION_TRANSLATION);
-		}
+			this.connectionProvider = new ExceptionTranslatingConnectionProvider(createConnectionProvider(client, CODEC));
+			this.reactiveConnectionProvider = new ExceptionTranslatingConnectionProvider(
+					createConnectionProvider(client, LettuceReactiveRedisConnection.CODEC));
 
-		this.initialized = true;
+			if (isClusterAware()) {
 
-		if (getEagerInitialization() && getShareNativeConnection()) {
-			initConnection();
+				this.clusterCommandExecutor = new ClusterCommandExecutor(
+						new LettuceClusterTopologyProvider((RedisClusterClient) client),
+						new LettuceClusterConnection.LettuceClusterNodeResourceProvider(this.connectionProvider),
+						EXCEPTION_TRANSLATION);
+			}
+
+			state.set(State.STARTED);
+
+			if (getEagerInitialization() && getShareNativeConnection()) {
+				initConnection();
+			}
 		}
 	}
 
+	@Override
+	public void stop() {
+
+		if (state.compareAndSet(State.STARTED, State.STOPPING)) {
+			resetConnection();
+			dispose(connectionProvider);
+			dispose(reactiveConnectionProvider);
+			try {
+				Duration quietPeriod = clientConfiguration.getShutdownQuietPeriod();
+				Duration timeout = clientConfiguration.getShutdownTimeout();
+				client.shutdown(quietPeriod.toMillis(), timeout.toMillis(), TimeUnit.MILLISECONDS);
+				state.set(State.STOPPED);
+			} catch (Exception e) {
+
+				if (log.isWarnEnabled()) {
+					log.warn((client != null ? ClassUtils.getShortName(client.getClass()) : "LettuceClient")
+							+ " did not shut down gracefully.", e);
+				}
+			}
+			state.set(State.STOPPED);
+		}
+	}
+
+	@Override
+	public boolean isRunning() {
+		return State.STARTED.equals(state.get());
+	}
+
+	@Override
+	public void afterPropertiesSet() {
+		// customization hook. initialization happens in start
+	}
+
+	@Override
 	public void destroy() {
 
-		resetConnection();
-
+		stop();
+		client = null;
 		if (clusterCommandExecutor != null) {
 
 			try {
@@ -368,23 +418,7 @@ public class LettuceConnectionFactory
 				log.warn("Cannot properly close cluster command executor", ex);
 			}
 		}
-
-		dispose(connectionProvider);
-		dispose(reactiveConnectionProvider);
-
-		try {
-			Duration quietPeriod = clientConfiguration.getShutdownQuietPeriod();
-			Duration timeout = clientConfiguration.getShutdownTimeout();
-			client.shutdown(quietPeriod.toMillis(), timeout.toMillis(), TimeUnit.MILLISECONDS);
-		} catch (Exception e) {
-
-			if (log.isWarnEnabled()) {
-				log.warn((client != null ? ClassUtils.getShortName(client.getClass()) : "LettuceClient")
-						+ " did not shut down gracefully.", e);
-			}
-		}
-
-		this.destroyed = true;
+		state.set(State.DESTROYED);
 	}
 
 	private void dispose(LettuceConnectionProvider connectionProvider) {
@@ -532,8 +566,6 @@ public class LettuceConnectionFactory
 	 */
 	public void resetConnection() {
 
-		assertInitialized();
-
 		Optionals.toStream(Optional.ofNullable(connection), Optional.ofNullable(reactiveConnection))
 				.forEach(SharedConnection::resetConnection);
 
@@ -1267,8 +1299,19 @@ public class LettuceConnectionFactory
 	}
 
 	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");
+
+		State current = state.get();
+
+		if (State.STARTED.equals(current)) {
+			return;
+		}
+
+		switch (current) {
+			case CREATED, STOPPED -> throw new IllegalStateException(String.format("LettuceConnectionFactory has been %s. Use start() to initialize it", current));
+			case DESTROYED -> throw new IllegalStateException(
+					"LettuceConnectionFactory was destroyed and cannot be used anymore");
+			default -> throw new IllegalStateException(String.format("LettuceConnectionFactory is %s", current));
+		}
 	}
 
 	private static void applyToAll(RedisURI source, Consumer<RedisURI> action) {

src/main/java/org/springframework/data/redis/support/collections/RedisCollectionFactoryBean.java

@@ -16,10 +16,11 @@
 package org.springframework.data.redis.support.collections;
 
 import org.springframework.beans.factory.BeanNameAware;
-import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.beans.factory.SmartFactoryBean;
 import org.springframework.data.redis.connection.DataType;
 import org.springframework.data.redis.core.RedisTemplate;
+import org.springframework.data.util.Lazy;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
@@ -30,8 +31,9 @@ import org.springframework.util.StringUtils;
  * Otherwise uses the provided type (default is list).
  *
  * @author Costin Leau
+ * @author Christoph Strobl
  */
-public class RedisCollectionFactoryBean implements InitializingBean, BeanNameAware, FactoryBean<RedisStore> {
+public class RedisCollectionFactoryBean implements SmartFactoryBean<RedisStore>, BeanNameAware, InitializingBean {
 
 	/**
 	 * Collection types supported by this factory.
@@ -73,13 +75,15 @@ public class RedisCollectionFactoryBean implements InitializingBean, BeanNameAwa
 		abstract DataType dataType();
 	}
 
-	private @Nullable RedisStore store;
+	private @Nullable Lazy<RedisStore> store;
 	private @Nullable CollectionType type = null;
 	private @Nullable RedisTemplate<String, ?> template;
 	private @Nullable String key;
 	private @Nullable String beanName;
 
+	@Override
 	public void afterPropertiesSet() {
+
 		if (!StringUtils.hasText(key)) {
 			key = beanName;
 		}
@@ -87,19 +91,23 @@ public class RedisCollectionFactoryBean implements InitializingBean, BeanNameAwa
 		Assert.hasText(key, "Collection key is required - no key or bean name specified");
 		Assert.notNull(template, "Redis template is required");
 
-		DataType dt = template.type(key);
+		store = Lazy.of(() -> {
 
-		// can't create store
-		Assert.isTrue(!DataType.STRING.equals(dt), "Cannot create store on keys of type 'string'");
+			DataType dt = template.type(key);
 
-		store = createStore(dt);
+			// can't create store
+			Assert.isTrue(!DataType.STRING.equals(dt), "Cannot create store on keys of type 'string'");
 
-		if (store == null) {
-			if (type == null) {
-				type = CollectionType.LIST;
+			RedisStore tmp = createStore(dt);
+
+			if (tmp == null) {
+				if (type == null) {
+					type = CollectionType.LIST;
+				}
+				tmp = createStore(type.dataType());
 			}
-			store = createStore(type.dataType());
-		}
+			return tmp;
+		});
 	}
 
 	@SuppressWarnings("unchecked")
@@ -123,18 +131,17 @@ public class RedisCollectionFactoryBean implements InitializingBean, BeanNameAwa
 		return null;
 	}
 
+	@Override
 	public RedisStore getObject() {
-		return store;
+		return store.get();
 	}
 
+	@Override
 	public Class<?> getObjectType() {
-		return (store != null ? store.getClass() : RedisStore.class);
-	}
-
-	public boolean isSingleton() {
-		return true;
+		return (store != null ? store.get().getClass() : RedisStore.class);
 	}
 
+	@Override
 	public void setBeanName(String name) {
 		this.beanName = name;
 	}