Skip to content

GH-2884: RedisUtils improvements #2885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ public class RedisMessageStore extends AbstractKeyValueMessageStore implements B

private final RedisTemplate<Object, Object> redisTemplate;

private final boolean unlinkAvailable;

private boolean valueSerializerSet;

/**
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down