Change default value for KeyspaceEventMessageListener on keyspace event notifications

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-redis</artifactId>
-	<version>3.2.0-SNAPSHOT</version>
+	<version>3.2.0-GH-2670-SNAPSHOT</version>
 
 	<name>Spring Data Redis</name>
 	<description>Spring Data module for Redis</description>

src/main/java/org/springframework/data/redis/listener/KeyExpirationEventMessageListener.java

@@ -25,24 +25,35 @@
 /**
  * {@link MessageListener} publishing {@link RedisKeyExpiredEvent}s via {@link ApplicationEventPublisher} by listening
  * to Redis keyspace notifications for key expirations.
+ * <p>
+ * For development-time convenience the {@link #setKeyspaceNotificationsConfigParameter(String)} is set to
+ * {@literal "Ex"}, by default. However, it is strongly recommended that users specifically set
+ * {@literal notify-keyspace-events} to the appropriate value on the Redis server, in {@literal redis.conf}.
+ * <p>
+ * Any Redis server configuration coming from your Spring (Data Redis) application only occurs during Spring container
+ * initialization, and is not persisted across Redis server restarts.
  *
  * @author Christoph Strobl
+ * @author John Blum
  * @since 1.7
  */
 public class KeyExpirationEventMessageListener extends KeyspaceEventMessageListener implements
 		ApplicationEventPublisherAware {
 
+	private static final String EXPIRED_KEY_EVENTS = "Ex";
+
 	private static final Topic KEYEVENT_EXPIRED_TOPIC = new PatternTopic("__keyevent@*__:expired");
 
 	private @Nullable ApplicationEventPublisher publisher;
 
 	/**
-	 * Creates new {@link MessageListener} for {@code __keyevent@*__:expired} messages.
+	 * Creates new {@link MessageListener} for {@code __keyevent@*__:expired} messages and configures notification on
+	 * expired keys ({@literal Ex}).
 	 *
 	 * @param listenerContainer must not be {@literal null}.
 	 */
 	public KeyExpirationEventMessageListener(RedisMessageListenerContainer listenerContainer) {
-		super(listenerContainer);
+		super(listenerContainer, EXPIRED_KEY_EVENTS);
 	}
 
 	@Override

src/main/java/org/springframework/data/redis/listener/KeyspaceEventMessageListener.java

@@ -17,101 +17,199 @@
 
 import java.util.Properties;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.data.redis.connection.Message;
 import org.springframework.data.redis.connection.MessageListener;
 import org.springframework.data.redis.connection.RedisConnection;
+import org.springframework.data.redis.connection.RedisConnectionFactory;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
 /**
  * Base {@link MessageListener} implementation for listening to Redis keyspace notifications.
+ * <p>
+ * By default, this {@link MessageListener} does not listen for, or notify on, any keyspace events. You must explicitly
+ * set the {@link #setKeyspaceNotificationsConfigParameter(String)} to a valid {@literal redis.conf},
+ * {@literal notify-keyspace-events} value (for example: {@literal EA}) to enable keyspace event notifications
+ * from your Redis server.
+ * <p>
+ * Any configuration set in the Redis server take precedence. Therefore, if the Redis server already set a value
+ * for {@literal notify-keyspace-events}, then any {@link #setKeyspaceNotificationsConfigParameter(String)}
+ * specified on this listener will be ignored.
+ * <p>
+ * It is recommended that all infrastructure settings, such as {@literal notify-keyspace-events}, be configured on
+ * the Redis server itself. If the Redis server is rebooted, then any keyspace event configuration coming from
+ * the application will be lost when the Redis server is restarted since Redis server configuration is not persistent,
+ * and any configuration coming from your application only occurs during Spring container initialization.
  *
  * @author Christoph Strobl
  * @author Mark Paluch
+ * @author John Blum
  * @since 1.7
  */
 public abstract class KeyspaceEventMessageListener implements MessageListener, InitializingBean, DisposableBean {
 
+	protected static final String DISABLED_KEY_EVENTS = "";
+	protected static final String NOTIFY_KEYSPACE_EVENTS = "notify-keyspace-events";
+
 	private static final Topic TOPIC_ALL_KEYEVENTS = new PatternTopic("__keyevent@*");
 
-	private final RedisMessageListenerContainer listenerContainer;
+	private final Logger logger = LoggerFactory.getLogger(getClass());
+
+	private final RedisMessageListenerContainer messageListenerContainer;
+
+	private @Nullable String keyspaceNotificationsConfigParameter;
+
+	/**
+	 * Creates a new {@link KeyspaceEventMessageListener}.
+	 *
+	 * @param messageListenerContainer {@link RedisMessageListenerContainer} in which this listener will be registered;
+	 * must not be {@literal null}.
+	 */
+	public KeyspaceEventMessageListener(RedisMessageListenerContainer messageListenerContainer) {
+		this(messageListenerContainer, DISABLED_KEY_EVENTS);
+	}
+
+	/**
+	 * Creates a new {@link KeyspaceEventMessageListener} along with initialization for
+	 * {@literal notify-keyspace-events}.
+	 *
+	 * @param messageListenerContainer {@link RedisMessageListenerContainer} in which this listener will be registered;
+	 * must not be {@literal null}.
+	 * @param keyspaceNotificationsConfigParameter {@link String default value} for {@literal notify-keyspace-events};
+	 * may be {@literal null}.
+	 */
+	protected KeyspaceEventMessageListener(RedisMessageListenerContainer messageListenerContainer,
+			@Nullable String keyspaceNotificationsConfigParameter) {
+
+		Assert.notNull(messageListenerContainer, "RedisMessageListenerContainer to run in must not be null");
 
-	private String keyspaceNotificationsConfigParameter = "EA";
+		this.messageListenerContainer = messageListenerContainer;
+		this.keyspaceNotificationsConfigParameter = keyspaceNotificationsConfigParameter;
+	}
 
 	/**
-	 * Creates new {@link KeyspaceEventMessageListener}.
+	 * Returns a reference to the configured {@link Logger}.
 	 *
-	 * @param listenerContainer must not be {@literal null}.
+	 * @return a reference to the configured {@link Logger}.
 	 */
-	public KeyspaceEventMessageListener(RedisMessageListenerContainer listenerContainer) {
+	protected Logger getLogger() {
+		return this.logger;
+	}
 
-		Assert.notNull(listenerContainer, "RedisMessageListenerContainer to run in must not be null");
-		this.listenerContainer = listenerContainer;
+	/**
+	 * Returns a configured reference to the {@link RedisMessageListenerContainer} to which this {@link MessageListener}
+	 * is registered.
+	 *
+	 * @return a configured reference to the {@link RedisMessageListenerContainer} to which this {@link MessageListener}
+	 * is registered.
+	 */
+	protected RedisMessageListenerContainer getMessageListenerContainer() {
+		return this.messageListenerContainer;
 	}
 
 	@Override
 	public void onMessage(Message message, @Nullable byte[] pattern) {
 
-		if (ObjectUtils.isEmpty(message.getChannel()) || ObjectUtils.isEmpty(message.getBody())) {
-			return;
+		if (containsChannelContent(message)) {
+			doHandleMessage(message);
 		}
+	}
 
-		doHandleMessage(message);
+	// Message must have a channel and body (contain content)
+	private boolean containsChannelContent(Message message) {
+		return !(ObjectUtils.isEmpty(message.getChannel()) || ObjectUtils.isEmpty(message.getBody()));
 	}
 
 	/**
-	 * Handle the actual message
+	 * Handle the actual {@link Message}.
 	 *
-	 * @param message never {@literal null}.
+	 * @param message {@link Message} to process; never {@literal null}.
 	 */
 	protected abstract void doHandleMessage(Message message);
 
+	@Override
+	public void afterPropertiesSet() throws Exception {
+		init();
+	}
+
 	/**
-	 * Initialize the message listener by writing requried redis config for {@literal notify-keyspace-events} and
-	 * registering the listener within the container.
+	 * Initialize this {@link MessageListener} by writing required Redis server config
+	 * for {@literal notify-keyspace-events} and registering this {@link MessageListener}
+	 * with the {@link RedisMessageListenerContainer}.
 	 */
 	public void init() {
 
-		if (StringUtils.hasText(keyspaceNotificationsConfigParameter)) {
+		String keyspaceNotificationsConfigParameter = getKeyspaceNotificationsConfigParameter();
 
-			RedisConnection connection = listenerContainer.getConnectionFactory().getConnection();
+		if (isSet(keyspaceNotificationsConfigParameter)) {
+			configureKeyspaceEventNotifications(keyspaceNotificationsConfigParameter);
+		}
 
-			try {
+		doRegister(getMessageListenerContainer());
+	}
 
-				Properties config = connection.getConfig("notify-keyspace-events");
+	private boolean isSet(@Nullable String value) {
+		return StringUtils.hasText(value);
+	}
 
-				if (!StringUtils.hasText(config.getProperty("notify-keyspace-events"))) {
-					connection.setConfig("notify-keyspace-events", keyspaceNotificationsConfigParameter);
-				}
+	void configureKeyspaceEventNotifications(String keyspaceNotificationsConfigParameter) {
+
+		RedisConnectionFactory connectionFactory = getMessageListenerContainer().getConnectionFactory();
 
-			} finally {
-				connection.close();
+		if (connectionFactory != null) {
+			try (RedisConnection connection = connectionFactory.getConnection()) {
+				if (canChangeNotifyKeyspaceEvents(connection)) {
+					setKeyspaceEventNotifications(connection, keyspaceNotificationsConfigParameter);
+				}
 			}
 		}
+		else {
+			if (getLogger().isWarnEnabled()) {
+				getLogger().warn("Unable to configure notification on keyspace events;"
+					+ " no RedisConnectionFactory was configured in the RedisMessageListenerContainer");
+			}
+		}
+	}
+
+	private boolean canChangeNotifyKeyspaceEvents(@Nullable RedisConnection connection) {
+
+		if (connection != null) {
+
+			Properties config = connection.serverCommands().getConfig(NOTIFY_KEYSPACE_EVENTS);
+
+			return config == null || !isSet(config.getProperty(NOTIFY_KEYSPACE_EVENTS));
+		}
 
-		doRegister(listenerContainer);
+		return false;
+	}
+
+	void setKeyspaceEventNotifications(RedisConnection connection, String keyspaceNotificationsConfigParameter) {
+		connection.serverCommands().setConfig(NOTIFY_KEYSPACE_EVENTS, keyspaceNotificationsConfigParameter);
+	}
+
+	@Override
+	public void destroy() throws Exception {
+		getMessageListenerContainer().removeMessageListener(this);
 	}
 
 	/**
-	 * Register instance within the container.
+	 * Register instance within the {@link RedisMessageListenerContainer}.
 	 *
 	 * @param container never {@literal null}.
 	 */
 	protected void doRegister(RedisMessageListenerContainer container) {
-		listenerContainer.addMessageListener(this, TOPIC_ALL_KEYEVENTS);
-	}
-
-	@Override
-	public void destroy() throws Exception {
-		listenerContainer.removeMessageListener(this);
+		container.addMessageListener(this, TOPIC_ALL_KEYEVENTS);
 	}
 
 	/**
-	 * Set the configuration string to use for {@literal notify-keyspace-events}.
+	 * Set the {@link String configuration setting} (for example: {@literal EA}) to use
+	 * for {@literal notify-keyspace-events}.
 	 *
 	 * @param keyspaceNotificationsConfigParameter can be {@literal null}.
 	 * @since 1.8
@@ -120,8 +218,13 @@ public void setKeyspaceNotificationsConfigParameter(String keyspaceNotifications
 		this.keyspaceNotificationsConfigParameter = keyspaceNotificationsConfigParameter;
 	}
 
-	@Override
-	public void afterPropertiesSet() throws Exception {
-		init();
+	/**
+	 * Get the configured {@link String setting} for {@literal notify-keyspace-events}.
+	 *
+	 * @return the configured {@link String setting} for {@literal notify-keyspace-events}.
+	 */
+	@Nullable
+	protected String getKeyspaceNotificationsConfigParameter() {
+		return this.keyspaceNotificationsConfigParameter;
 	}
 }

src/test/java/org/springframework/data/redis/listener/KeyExpirationEventMessageListenerIntegrationTests.java

@@ -27,12 +27,10 @@
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
 
-import org.springframework.beans.factory.DisposableBean;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.data.redis.connection.RedisConnection;
 import org.springframework.data.redis.connection.RedisConnectionFactory;
-import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
 import org.springframework.data.redis.connection.jedis.extension.JedisConnectionFactoryExtension;
 import org.springframework.data.redis.test.extension.RedisStanalone;
 
@@ -103,7 +101,7 @@ void listenerShouldPublishEventCorrectly() {
 	@Test // DATAREDIS-425
 	void listenerShouldNotReactToDeleteEvents() throws InterruptedException {
 
-		byte[] key = ("to-delete:" + UUID.randomUUID().toString()).getBytes();
+		byte[] key = ("to-delete:" + UUID.randomUUID()).getBytes();
 
 		try (RedisConnection connection = connectionFactory.getConnection()) {