From 297ee410dc1fcdf50ac7af5a62be8fc4bce62ef5 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 11 Sep 2024 11:31:24 +0200 Subject: [PATCH] Polishing. Enable SCRIPT FLUSH and SCRIPT KILL for pipelining/transaction usage. Use lambdas where possible. Original pull request: #2988 See #1455 --- .../jedis/JedisScriptingCommands.java | 35 ++++++++----------- ...ConnectionTransactionIntegrationTests.java | 7 ---- ...disConnectionPipelineIntegrationTests.java | 14 -------- ...ConnectionTransactionIntegrationTests.java | 12 ------- 4 files changed, 14 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisScriptingCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisScriptingCommands.java index 92d758b00..0c114dacb 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisScriptingCommands.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisScriptingCommands.java @@ -16,10 +16,10 @@ package org.springframework.data.redis.connection.jedis; import redis.clients.jedis.Jedis; +import redis.clients.jedis.commands.ScriptingKeyPipelineBinaryCommands; import java.util.List; -import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.redis.connection.RedisScriptingCommands; import org.springframework.data.redis.connection.ReturnType; import org.springframework.util.Assert; @@ -31,6 +31,7 @@ import org.springframework.util.Assert; */ class JedisScriptingCommands implements RedisScriptingCommands { + private static final byte[] SAMPLE_KEY = new byte[0]; private final JedisConnection connection; JedisScriptingCommands(JedisConnection connection) { @@ -39,27 +40,21 @@ class JedisScriptingCommands implements RedisScriptingCommands { @Override public void scriptFlush() { - - assertDirectMode(); - - connection.invoke().just(Jedis::scriptFlush); + connection.invoke().just(Jedis::scriptFlush, it -> it.scriptFlush(SAMPLE_KEY)); } @Override public void scriptKill() { - - assertDirectMode(); - - connection.invoke().just(Jedis::scriptKill); + connection.invoke().just(Jedis::scriptKill, it -> it.scriptKill(SAMPLE_KEY)); } @Override public String scriptLoad(byte[] script) { Assert.notNull(script, "Script must not be null"); - assertDirectMode(); - return connection.invoke().from(it -> it.scriptLoad(script)).get(JedisConverters::toString); + return connection.invoke().from(it -> it.scriptLoad(script), it -> it.scriptLoad(script, SAMPLE_KEY)) + .get(JedisConverters::toString); } @Override @@ -67,9 +62,13 @@ class JedisScriptingCommands implements RedisScriptingCommands { Assert.notNull(scriptSha1, "Script digests must not be null"); Assert.noNullElements(scriptSha1, "Script digests must not contain null elements"); - assertDirectMode(); - return connection.invoke().just(it -> it.scriptExists(scriptSha1)); + byte[][] sha1 = new byte[scriptSha1.length][]; + for (int i = 0; i < scriptSha1.length; i++) { + sha1[i] = JedisConverters.toBytes(scriptSha1[i]); + } + + return connection.invoke().just(it -> it.scriptExists(scriptSha1), it -> it.scriptExists(SAMPLE_KEY, sha1)); } @Override @@ -80,7 +79,7 @@ class JedisScriptingCommands implements RedisScriptingCommands { JedisScriptReturnConverter converter = new JedisScriptReturnConverter(returnType); return (T) connection.invoke() - .from(it -> it.eval(script, numKeys, keysAndArgs), t -> t.eval(script, numKeys, keysAndArgs)) + .from(Jedis::eval, ScriptingKeyPipelineBinaryCommands::eval, script, numKeys, keysAndArgs) .getOrElse(converter, () -> converter.convert(null)); } @@ -97,15 +96,9 @@ class JedisScriptingCommands implements RedisScriptingCommands { JedisScriptReturnConverter converter = new JedisScriptReturnConverter(returnType); return (T) connection.invoke() - .from(it -> it.evalsha(scriptSha, numKeys, keysAndArgs), t -> t.evalsha(scriptSha, numKeys, keysAndArgs)) + .from(Jedis::evalsha, ScriptingKeyPipelineBinaryCommands::evalsha, scriptSha, numKeys, keysAndArgs) .getOrElse(converter, () -> converter.convert(null) ); } - private void assertDirectMode() { - if (connection.isQueueing() || connection.isPipelined()) { - throw new InvalidDataAccessApiUsageException("Scripting commands not supported in pipelining/transaction mode"); - } - } - } diff --git a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionTransactionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionTransactionIntegrationTests.java index 1b5c4672f..1ece6b7ed 100644 --- a/src/test/java/org/springframework/data/redis/connection/AbstractConnectionTransactionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/AbstractConnectionTransactionIntegrationTests.java @@ -134,13 +134,6 @@ abstract public class AbstractConnectionTransactionIntegrationTests extends Abst .isThrownBy(() -> connection.watch("foo".getBytes())); } - @Test - public void testScriptKill() { - // Impossible to call script kill in a tx because you can't issue the - // exec command while Redis is running a script - assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(() -> connection.scriptKill()); - } - @Test // DATAREDIS-417 @Disabled @Override 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 dae84554c..cbd0edae6 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 @@ -76,20 +76,6 @@ public class JedisConnectionPipelineIntegrationTests extends AbstractConnectionP } // Unsupported Ops - @Test - public void testScriptExists() { - assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(super::testScriptExists); - } - - @Test - public void testScriptKill() { - assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(() -> connection.scriptKill()); - } - - @Test - @Disabled - public void testScriptFlush() {} - @Test // DATAREDIS-269 public void clientSetNameWorksCorrectly() { assertThatExceptionOfType(InvalidDataAccessApiUsageException.class).isThrownBy(super::clientSetNameWorksCorrectly); diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java index 5d61de88e..ba43a4fd2 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionTransactionIntegrationTests.java @@ -95,18 +95,6 @@ public class JedisConnectionTransactionIntegrationTests extends AbstractConnecti // Unsupported Ops - @Test - @Disabled - public void testScriptExists() {} - - @Test - @Disabled - public void testScriptKill() {} - - @Test - @Disabled - public void testScriptFlush() {} - @Test @Disabled public void testInfoBySection() {}