From ad7ccb3c9bb3ae7542b50bf804d0a5d4f1379363 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 5 Apr 2019 14:32:55 -0400 Subject: [PATCH] GH-2884: RedisUtils improvements Fixes https://github.com/spring-projects/spring-integration/issues/2884 - synchronize the map - only call once per component --- .../redis/store/RedisMessageStore.java | 7 +++++-- .../redis/util/RedisLockRegistry.java | 7 ++++++- .../integration/redis/util/RedisUtils.java | 5 +++-- .../redis/util/RedisLockRegistryTests.java | 20 +++++++++++++++++++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/spring-integration-redis/src/main/java/org/springframework/integration/redis/store/RedisMessageStore.java b/spring-integration-redis/src/main/java/org/springframework/integration/redis/store/RedisMessageStore.java index 8e73190780..a078978dae 100644 --- a/spring-integration-redis/src/main/java/org/springframework/integration/redis/store/RedisMessageStore.java +++ b/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 redisTemplate; + private final boolean unlinkAvailable; + private boolean valueSerializerSet; /** @@ -72,6 +74,7 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B this.redisTemplate.setKeySerializer(new StringRedisSerializer()); this.redisTemplate.setValueSerializer(new JdkSerializationRedisSerializer()); this.redisTemplate.afterPropertiesSet(); + this.unlinkAvailable = RedisUtils.isUnlinkAvailable(this.redisTemplate); } @Override @@ -131,7 +134,7 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B 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 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B @Override protected void doRemoveAll(Collection ids) { - if (RedisUtils.isUnlinkAvailable(this.redisTemplate)) { + if (this.unlinkAvailable) { this.redisTemplate.unlink(ids); } else { diff --git a/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java b/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java index 49f18ed748..48f097ec1b 100644 --- a/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java +++ b/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 obtainLockScript; @@ -138,6 +140,7 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl 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 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl 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 final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl } private void removeLockKey() { - if (RedisUtils.isUnlinkAvailable(RedisLockRegistry.this.redisTemplate)) { + if (this.unlinkAvailable) { RedisLockRegistry.this.redisTemplate.unlink(this.lockKey); } else { diff --git a/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisUtils.java b/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisUtils.java index 7e8e43de55..b84d1664ac 100644 --- a/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisUtils.java +++ b/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, Boolean> unlinkAvailable = - new LinkedHashMap, Boolean>() { + Collections.synchronizedMap(new LinkedHashMap, Boolean>() { @Override protected boolean removeEldestEntry(Entry, Boolean> eldest) { return size() > 100; } - }; + }); /** * Perform an {@code INFO} command on the provided {@link RedisOperations} to check diff --git a/spring-integration-redis/src/test/java/org/springframework/integration/redis/util/RedisLockRegistryTests.java b/spring-integration-redis/src/test/java/org/springframework/integration/redis/util/RedisLockRegistryTests.java index c2856ea9ea..c459ca3df1 100644 --- a/spring-integration-redis/src/test/java/org/springframework/integration/redis/util/RedisLockRegistryTests.java +++ b/spring-integration-redis/src/test/java/org/springframework/integration/redis/util/RedisLockRegistryTests.java @@ -18,8 +18,12 @@ package org.springframework.integration.redis.util; 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.Before; 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 class RedisLockRegistryTests extends RedisAvailableTests { 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);