GH-2884: RedisUtils improvements

Fixes #2884

- synchronize the map
- only call once per component

Author: garyrussell

spring-integration-redis/src/main/java/org/springframework/integration/redis/store/RedisMessageStore.java

@@ -45,6 +45,8 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B
 
 	private final RedisTemplate<Object, Object> redisTemplate;
 
+	private final boolean unlinkAvailable;
+
 	private boolean valueSerializerSet;
 
 	/**
@@ -72,6 +74,7 @@ public RedisMessageStore(RedisConnectionFactory connectionFactory, String prefix
 		this.redisTemplate.setKeySerializer(new StringRedisSerializer());
 		this.redisTemplate.setValueSerializer(new JdkSerializationRedisSerializer());
 		this.redisTemplate.afterPropertiesSet();
+		this.unlinkAvailable = RedisUtils.isUnlinkAvailable(this.redisTemplate);
 	}
 
 	@Override
@@ -131,7 +134,7 @@ protected Object doRemove(Object id) {
 		Assert.notNull(id, "'id' must not be null");
 		Object removedObject = this.doRetrieve(id);
 		if (removedObject != null) {
-			if (RedisUtils.isUnlinkAvailable(this.redisTemplate)) {
+			if (this.unlinkAvailable) {
 				this.redisTemplate.unlink(id);
 			}
 			else {
@@ -143,7 +146,7 @@ protected Object doRemove(Object id) {
 
 	@Override
 	protected void doRemoveAll(Collection<Object> ids) {
-		if (RedisUtils.isUnlinkAvailable(this.redisTemplate)) {
+		if (this.unlinkAvailable) {
 			this.redisTemplate.unlink(ids);
 		}
 		else {

spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java

@@ -97,6 +97,8 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
 
 	private final String registryKey;
 
+	private final boolean unlinkAvailable;
+
 	private final StringRedisTemplate redisTemplate;
 
 	private final RedisScript<Boolean> obtainLockScript;
@@ -138,6 +140,7 @@ public RedisLockRegistry(RedisConnectionFactory connectionFactory, String regist
 		this.obtainLockScript = new DefaultRedisScript<>(OBTAIN_LOCK_SCRIPT, Boolean.class);
 		this.registryKey = registryKey;
 		this.expireAfter = expireAfter;
+		this.unlinkAvailable = RedisUtils.isUnlinkAvailable(this.redisTemplate);
 	}
 
 	/**
@@ -184,6 +187,8 @@ private final class RedisLock implements Lock {
 
 		private final ReentrantLock localLock = new ReentrantLock();
 
+		private final boolean unlinkAvailable = RedisLockRegistry.this.unlinkAvailable;
+
 		private volatile long lockedAt;
 
 		private RedisLock(String path) {
@@ -329,7 +334,7 @@ public void unlock() {
 		}
 
 		private void removeLockKey() {
-			if (RedisUtils.isUnlinkAvailable(RedisLockRegistry.this.redisTemplate)) {
+			if (this.unlinkAvailable) {
 				RedisLockRegistry.this.redisTemplate.unlink(this.lockKey);
 			}
 			else {

spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisUtils.java

@@ -16,6 +16,7 @@
 
 package org.springframework.integration.redis.util;
 
+import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -41,14 +42,14 @@ public final class RedisUtils {
 
 	@SuppressWarnings("serial")
 	private static final Map<RedisOperations<?, ?>, Boolean> unlinkAvailable =
-			new LinkedHashMap<RedisOperations<?, ?>, Boolean>() {
+			Collections.synchronizedMap(new LinkedHashMap<RedisOperations<?, ?>, Boolean>() {
 
 				@Override
 				protected boolean removeEldestEntry(Entry<RedisOperations<?, ?>, Boolean> eldest) {
 					return size() > 100;
 				}
 
-			};
+			});
 
 	/**
 	 * Perform an {@code INFO} command on the provided {@link RedisOperations} to check

spring-integration-redis/src/test/java/org/springframework/integration/redis/util/RedisLockRegistryTests.java

@@ -18,8 +18,12 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.willReturn;
+import static org.mockito.Mockito.mock;
 
 import java.util.Map;
+import java.util.Properties;
 import java.util.UUID;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executors;
@@ -35,6 +39,8 @@
 import org.junit.Test;
 
 import org.springframework.data.redis.connection.RedisConnectionFactory;
+import org.springframework.data.redis.core.RedisCallback;
+import org.springframework.data.redis.core.RedisOperations;
 import org.springframework.data.redis.core.StringRedisTemplate;
 import org.springframework.integration.redis.rules.RedisAvailable;
 import org.springframework.integration.redis.rules.RedisAvailableTests;
@@ -429,6 +435,20 @@ public void testExpireNotChanged() throws Exception {
 		lock.unlock();
 	}
 
+	@SuppressWarnings({ "unchecked", "rawtypes" })
+	@Test
+	public void ntestUlink() {
+		RedisOperations ops = mock(RedisOperations.class);
+		Properties props = new Properties();
+		willReturn(props).given(ops).execute(any(RedisCallback.class));
+		props.setProperty("redis_version", "3.0.0");
+		RedisLockRegistry registry = new RedisLockRegistry(mock(RedisConnectionFactory.class), "foo");
+		assertThat(TestUtils.getPropertyValue(registry, "ulinkAvailable", Boolean.class)).isFalse();
+		props.setProperty("redis_version", "4.0.0");
+		registry = new RedisLockRegistry(mock(RedisConnectionFactory.class), "foo");
+		assertThat(TestUtils.getPropertyValue(registry, "ulinkAvailable", Boolean.class)).isTrue();
+	}
+
 	private Long getExpire(RedisLockRegistry registry, String lockKey) {
 		StringRedisTemplate template = createTemplate();
 		String registryKey = TestUtils.getPropertyValue(registry, "registryKey", String.class);