From a944aa2d1bd3551cf2146cdf4e03ed10c1f112a3 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 1 Sep 2021 16:05:06 +0200 Subject: [PATCH] Upgrade to Jedis 3.7. Enable support for SCAN with type, call appropriate shutdown methods with save/nosave flags. Closes: #2153 Original Pull Request: #2154 --- pom.xml | 2 +- .../connection/jedis/JedisKeyCommands.java | 26 ++++++++-- .../connection/jedis/JedisServerCommands.java | 6 ++- .../AbstractConnectionIntegrationTests.java | 3 +- ...disConnectionPipelineIntegrationTests.java | 3 -- .../jedis/JedisConnectionUnitTests.java | 50 +++++++------------ 6 files changed, 44 insertions(+), 46 deletions(-) diff --git a/pom.xml b/pom.xml index f00668b4f..63931511f 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ 1.4.18 2.9.0 6.1.4.RELEASE - 3.6.3 + 3.7.0 1.01 4.1.65.Final spring.data.redis diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisKeyCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisKeyCommands.java index d44525c55..f721ffc0b 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisKeyCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisKeyCommands.java @@ -18,8 +18,10 @@ package org.springframework.data.redis.connection.jedis; import redis.clients.jedis.BinaryJedis; import redis.clients.jedis.MultiKeyPipelineBase; import redis.clients.jedis.ScanParams; +import redis.clients.jedis.ScanResult; import redis.clients.jedis.SortingParams; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.List; import java.util.Set; @@ -38,6 +40,7 @@ import org.springframework.data.redis.core.ScanIteration; import org.springframework.data.redis.core.ScanOptions; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * @author Christoph Strobl @@ -172,10 +175,6 @@ class JedisKeyCommands implements RedisKeyCommands { */ public Cursor scan(long cursorId, ScanOptions options) { - if (options instanceof KeyScanOptions && ((KeyScanOptions) options).getType() != null) { - throw new UnsupportedOperationException("'SCAN' with type is not yet supported using the Jedis driver"); - } - return new ScanCursor(cursorId, options) { @Override @@ -186,8 +185,25 @@ class JedisKeyCommands implements RedisKeyCommands { } ScanParams params = JedisConverters.toScanParams(options); - redis.clients.jedis.ScanResult result = connection.getJedis().scan(Long.toString(cursorId).getBytes(), + + ScanResult result; + byte[] type = null; + + if (options instanceof KeyScanOptions) { + String typeAsString = ((KeyScanOptions) options).getType(); + + if (!ObjectUtils.isEmpty(typeAsString)) { + type = typeAsString.getBytes(StandardCharsets.US_ASCII); + } + } + + if (type != null) { + result = connection.getJedis().scan(Long.toString(cursorId).getBytes(), params, type); + } else { + result = connection.getJedis().scan(Long.toString(cursorId).getBytes(), params); + } + return new ScanIteration<>(Long.parseLong(result.getCursor()), result.getResult()); } diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisServerCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisServerCommands.java index 99e83bc6c..f0ea3f1cf 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisServerCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisServerCommands.java @@ -18,6 +18,7 @@ package org.springframework.data.redis.connection.jedis; import redis.clients.jedis.BinaryJedis; import redis.clients.jedis.Jedis; import redis.clients.jedis.MultiKeyPipelineBase; +import redis.clients.jedis.args.SaveMode; import java.util.List; import java.util.Properties; @@ -25,7 +26,6 @@ import java.util.concurrent.TimeUnit; import org.springframework.data.redis.connection.RedisNode; import org.springframework.data.redis.connection.RedisServerCommands; -import org.springframework.data.redis.connection.ReturnType; import org.springframework.data.redis.connection.convert.Converters; import org.springframework.data.redis.core.types.RedisClientInfo; import org.springframework.lang.Nullable; @@ -151,7 +151,9 @@ class JedisServerCommands implements RedisServerCommands { return; } - connection.eval(String.format(SHUTDOWN_SCRIPT, option.name()).getBytes(), ReturnType.STATUS, 0); + SaveMode saveMode = (option == ShutdownOption.NOSAVE) ? SaveMode.NOSAVE : SaveMode.SAVE; + + connection.getJedis().shutdown(saveMode); } /* diff --git a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java index cc542172e..5a26103fb 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionIntegrationTests.java @@ -2600,8 +2600,7 @@ public abstract class AbstractConnectionIntegrationTests { assertThat(i).isEqualTo(itemCount); } - @Test // GH-2089 - @EnabledOnRedisDriver(RedisDriver.LETTUCE) + @Test // GH-2089, GH-2153 @EnabledOnRedisVersion("6.0") void scanWithType() { diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionPipelineIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionPipelineIntegrationTests.java index 8710ed5e7..4b629fa99 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionPipelineIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionPipelineIntegrationTests.java @@ -61,9 +61,6 @@ public class JedisConnectionPipelineIntegrationTests extends AbstractConnectionP connection = null; } - @Disabled("Jedis issue: Pipeline tries to return String instead of List") - public void testGetConfig() {} - @Test public void testWatch() { connection.set("testitnow", "willdo"); diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTests.java index 204c6f2da..c14262830 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTests.java @@ -22,6 +22,8 @@ import redis.clients.jedis.Client; import redis.clients.jedis.Jedis; import redis.clients.jedis.ScanParams; import redis.clients.jedis.ScanResult; +import redis.clients.jedis.args.SaveMode; +import redis.clients.jedis.exceptions.JedisException; import java.io.IOException; import java.util.Collections; @@ -30,8 +32,8 @@ import java.util.Map.Entry; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.mockito.ArgumentCaptor; +import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.dao.InvalidDataAccessResourceUsageException; import org.springframework.data.redis.connection.AbstractConnectionUnitTestBase; import org.springframework.data.redis.connection.RedisServerCommands.ShutdownOption; @@ -58,34 +60,32 @@ class JedisConnectionUnitTests { connection = new JedisConnection(jedisSpy); } - @Test // DATAREDIS-184 + @Test // DATAREDIS-184, GH-2153 void shutdownWithNullShouldDelegateCommandCorrectly() { - connection.shutdown(null); + try { + connection.shutdown(null); + } catch (InvalidDataAccessApiUsageException e) { + // all good. Sometimes it throws an Exception. + } verifyNativeConnectionInvocation().shutdown(); } - @Test // DATAREDIS-184 - public void shutdownNosaveShouldBeSentCorrectlyUsingLuaScript() { + @Test // DATAREDIS-184, GH-2153 + void shutdownNosaveShouldBeSentCorrectly() { - connection.shutdown(ShutdownOption.NOSAVE); + assertThatExceptionOfType(JedisException.class).isThrownBy(() -> connection.shutdown(ShutdownOption.NOSAVE)); - ArgumentCaptor captor = ArgumentCaptor.forClass(byte[].class); - verifyNativeConnectionInvocation().eval(captor.capture(), any(byte[].class), any(byte[][].class)); - - assertThat(captor.getValue()).isEqualTo("return redis.call('SHUTDOWN','NOSAVE')".getBytes()); + verifyNativeConnectionInvocation().shutdown(SaveMode.NOSAVE); } - @Test // DATAREDIS-184 - public void shutdownSaveShouldBeSentCorrectlyUsingLuaScript() { + @Test // DATAREDIS-184, GH-2153 + void shutdownSaveShouldBeSentCorrectly() { - connection.shutdown(ShutdownOption.SAVE); + assertThatExceptionOfType(JedisException.class).isThrownBy(() -> connection.shutdown(ShutdownOption.SAVE)); - ArgumentCaptor captor = ArgumentCaptor.forClass(byte[].class); - verifyNativeConnectionInvocation().eval(captor.capture(), any(byte[].class), any(byte[][].class)); - - assertThat(captor.getValue()).isEqualTo("return redis.call('SHUTDOWN','SAVE')".getBytes()); + verifyNativeConnectionInvocation().shutdown(SaveMode.SAVE); } @Test // DATAREDIS-267 @@ -279,22 +279,6 @@ class JedisConnectionUnitTests { connection.openPipeline(); } - @Test - @Override - // DATAREDIS-184 - public void shutdownNosaveShouldBeSentCorrectlyUsingLuaScript() { - assertThatExceptionOfType(UnsupportedOperationException.class) - .isThrownBy(() -> super.shutdownNosaveShouldBeSentCorrectlyUsingLuaScript()); - } - - @Test - @Override - // DATAREDIS-184 - public void shutdownSaveShouldBeSentCorrectlyUsingLuaScript() { - assertThatExceptionOfType(UnsupportedOperationException.class) - .isThrownBy(() -> super.shutdownSaveShouldBeSentCorrectlyUsingLuaScript()); - } - @Test // DATAREDIS-267 public void killClientShouldDelegateCallCorrectly() { assertThatExceptionOfType(UnsupportedOperationException.class)