Skip to content

Commit e9b5e6e

Browse files
mp911demp911de
committed
DATAREDIS-1005 - Polishing.
Implement cluster-wide scriptFlush(), scriptKill(), and scriptLoad(). Add method argument assertions and author tags. Adapt and fix tests. Original pull request: #460.
1 parent 988a7e7 commit e9b5e6e

File tree

6 files changed

+108
-66
lines changed

6 files changed

+108
-66
lines changed

‎src/main/asciidoc/new-features.adoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ This section briefly covers items that are new and noteworthy in the latest rele
99
* <<redis.streams>>
1010
* Refined `union`/`diff`/`intersect` set-operation methods accepting a single collection of keys.
1111
* Upgrade to Jedis 3.
12+
* Add support for scripting commands using Jedis Cluster.
1213

1314
[[new-in-2.1.0]]
1415
== New in Spring Data Redis 2.1

‎src/main/java/org/springframework/data/redis/connection/ClusterCommandExecutor.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,8 @@ public T getFirstNonNullNotEmptyOrDefault(@Nullable T returnValue) {
587587
if (CollectionUtils.isEmpty((Map<?, ?>) nodeResult.getValue())) {
588588
return nodeResult.getValue();
589589
}
590-
} else if (CollectionUtils.isEmpty((Collection<?>) nodeResult.getValue())) {
590+
} else if (nodeResult.getValue() instanceof Collection
591+
&& CollectionUtils.isEmpty((Collection<?>) nodeResult.getValue())) {
591592
return nodeResult.getValue();
592593
} else {
593594
return nodeResult.getValue();

‎src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
* @author Ninad Divadkar
7070
* @author Tao Chen
7171
* @author Chen Guanqun
72+
* @author Pavel Khokhlov
7273
* @since 1.7
7374
*/
7475
public class JedisClusterConnection implements DefaultedRedisClusterConnection {

‎src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterScriptingCommands.java

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,32 +15,42 @@
1515
*/
1616
package org.springframework.data.redis.connection.jedis;
1717

18-
import java.util.List;
19-
2018
import lombok.NonNull;
2119
import lombok.RequiredArgsConstructor;
22-
import org.springframework.dao.DataAccessException;
20+
import redis.clients.jedis.BinaryJedis;
21+
import redis.clients.jedis.JedisCluster;
22+
23+
import java.util.List;
24+
2325
import org.springframework.dao.InvalidDataAccessApiUsageException;
26+
import org.springframework.data.redis.connection.ClusterCommandExecutor;
2427
import org.springframework.data.redis.connection.RedisScriptingCommands;
2528
import org.springframework.data.redis.connection.ReturnType;
26-
import redis.clients.jedis.JedisCluster;
29+
import org.springframework.util.Assert;
2730

2831
/**
2932
* @author Mark Paluch
33+
* @author Pavel Khokhlov
3034
* @since 2.0
3135
*/
3236
@RequiredArgsConstructor
3337
class JedisClusterScriptingCommands implements RedisScriptingCommands {
3438

35-
private final @NonNull JedisClusterConnection clusterConnection;
39+
private final @NonNull JedisClusterConnection connection;
3640

3741
/*
3842
* (non-Javadoc)
3943
* @see org.springframework.data.redis.connection.RedisScriptingCommands#scriptFlush()
4044
*/
4145
@Override
4246
public void scriptFlush() {
43-
throw new InvalidDataAccessApiUsageException("ScriptFlush is not supported in cluster environment.");
47+
48+
try {
49+
connection.getClusterCommandExecutor().executeCommandOnAllNodes(
50+
(JedisClusterConnection.JedisClusterCommandCallback<String>) BinaryJedis::scriptFlush);
51+
} catch (Exception ex) {
52+
throw convertJedisAccessException(ex);
53+
}
4454
}
4555

4656
/*
@@ -49,7 +59,13 @@ public void scriptFlush() {
4959
*/
5060
@Override
5161
public void scriptKill() {
52-
throw new InvalidDataAccessApiUsageException("ScriptKill is not supported in cluster environment.");
62+
63+
try {
64+
connection.getClusterCommandExecutor().executeCommandOnAllNodes(
65+
(JedisClusterConnection.JedisClusterCommandCallback<String>) BinaryJedis::scriptKill);
66+
} catch (Exception ex) {
67+
throw convertJedisAccessException(ex);
68+
}
5369
}
5470

5571
/*
@@ -58,7 +74,18 @@ public void scriptKill() {
5874
*/
5975
@Override
6076
public String scriptLoad(byte[] script) {
61-
throw new InvalidDataAccessApiUsageException("ScriptLoad is not supported in cluster environment.");
77+
78+
Assert.notNull(script, "Script must not be null!");
79+
80+
try {
81+
ClusterCommandExecutor.MultiNodeResult<byte[]> multiNodeResult = connection.getClusterCommandExecutor()
82+
.executeCommandOnAllNodes(
83+
(JedisClusterConnection.JedisClusterCommandCallback<byte[]>) client -> client.scriptLoad(script));
84+
85+
return JedisConverters.toString(multiNodeResult.getFirstNonNullNotEmptyOrDefault(new byte[0]));
86+
} catch (Exception ex) {
87+
throw convertJedisAccessException(ex);
88+
}
6289
}
6390

6491
/*
@@ -77,7 +104,9 @@ public List<Boolean> scriptExists(String... scriptShas) {
77104
@Override
78105
@SuppressWarnings("unchecked")
79106
public <T> T eval(byte[] script, ReturnType returnType, int numKeys, byte[]... keysAndArgs) {
80-
checkConnection();
107+
108+
Assert.notNull(script, "Script must not be null!");
109+
81110
try {
82111
return (T) new JedisScriptReturnConverter(returnType)
83112
.convert(getCluster().eval(script, JedisConverters.toBytes(numKeys), keysAndArgs));
@@ -102,7 +131,9 @@ public <T> T evalSha(String scriptSha, ReturnType returnType, int numKeys, byte[
102131
@Override
103132
@SuppressWarnings("unchecked")
104133
public <T> T evalSha(byte[] scriptSha, ReturnType returnType, int numKeys, byte[]... keysAndArgs) {
105-
checkConnection();
134+
135+
Assert.notNull(scriptSha, "Script digest must not be null!");
136+
106137
try {
107138
return (T) new JedisScriptReturnConverter(returnType)
108139
.convert(getCluster().evalsha(scriptSha, numKeys, keysAndArgs));
@@ -111,32 +142,11 @@ public <T> T evalSha(byte[] scriptSha, ReturnType returnType, int numKeys, byte[
111142
}
112143
}
113144

114-
public JedisClusterConnection getClusterConnection() {
115-
return clusterConnection;
116-
}
117-
118145
protected RuntimeException convertJedisAccessException(Exception ex) {
119-
return clusterConnection.convertJedisAccessException(ex);
146+
return connection.convertJedisAccessException(ex);
120147
}
121148

122149
private JedisCluster getCluster() {
123-
return clusterConnection.getCluster();
124-
}
125-
126-
protected void checkConnection() {
127-
if (isQueueing()) {
128-
throw new UnsupportedOperationException();
129-
}
130-
if (isPipelined()) {
131-
throw new UnsupportedOperationException();
132-
}
133-
}
134-
135-
private boolean isPipelined() {
136-
return clusterConnection.isPipelined();
137-
}
138-
139-
private boolean isQueueing() {
140-
return clusterConnection.isQueueing();
150+
return connection.getCluster();
141151
}
142152
}

‎src/main/java/org/springframework/data/redis/connection/jedis/JedisScriptingCommands.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,17 +147,17 @@ public <T> T evalSha(String scriptSha1, ReturnType returnType, int numKeys, byte
147147
*/
148148
@Override
149149
@SuppressWarnings("unchecked")
150-
public <T> T evalSha(byte[] scriptSha1, ReturnType returnType, int numKeys, byte[]... keysAndArgs) {
150+
public <T> T evalSha(byte[] scriptSha, ReturnType returnType, int numKeys, byte[]... keysAndArgs) {
151151

152-
Assert.notNull(scriptSha1, "Script digest must not be null!");
152+
Assert.notNull(scriptSha, "Script digest must not be null!");
153153

154154
if (isQueueing() || isPipelined()) {
155155
throw new UnsupportedOperationException();
156156
}
157157

158158
try {
159159
return (T) new JedisScriptReturnConverter(returnType)
160-
.convert(connection.getJedis().evalsha(scriptSha1, numKeys, keysAndArgs));
160+
.convert(connection.getJedis().evalsha(scriptSha, numKeys, keysAndArgs));
161161
} catch (Exception ex) {
162162
throw convertJedisAccessException(ex);
163163
}

‎src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.junit.ClassRule;
4242
import org.junit.Rule;
4343
import org.junit.Test;
44+
4445
import org.springframework.dao.DataAccessException;
4546
import org.springframework.dao.InvalidDataAccessApiUsageException;
4647
import org.springframework.data.domain.Range.Bound;
@@ -51,7 +52,6 @@
5152
import org.springframework.data.redis.connection.ClusterConnectionTests;
5253
import org.springframework.data.redis.connection.ClusterSlotHashUtil;
5354
import org.springframework.data.redis.connection.DataType;
54-
import org.springframework.data.redis.connection.ReturnType;
5555
import org.springframework.data.redis.connection.DefaultSortParameters;
5656
import org.springframework.data.redis.connection.DefaultTuple;
5757
import org.springframework.data.redis.connection.RedisClusterNode;
@@ -63,19 +63,21 @@
6363
import org.springframework.data.redis.connection.RedisStringCommands.SetOption;
6464
import org.springframework.data.redis.connection.RedisZSetCommands.Range;
6565
import org.springframework.data.redis.connection.RedisZSetCommands.Tuple;
66+
import org.springframework.data.redis.connection.ReturnType;
6667
import org.springframework.data.redis.connection.ValueEncoding.RedisValueEncoding;
6768
import org.springframework.data.redis.core.Cursor;
6869
import org.springframework.data.redis.core.ScanOptions;
70+
import org.springframework.data.redis.core.script.DigestUtils;
6971
import org.springframework.data.redis.core.types.Expiration;
7072
import org.springframework.data.redis.test.util.HexStringUtils;
7173
import org.springframework.data.redis.test.util.MinimumRedisVersionRule;
7274
import org.springframework.data.redis.test.util.RedisClusterRule;
7375
import org.springframework.test.annotation.IfProfileValue;
74-
import org.springframework.data.redis.core.script.DigestUtils;
7576

7677
/**
7778
* @author Christoph Strobl
7879
* @author Mark Paluch
80+
* @author Pavel Khokhlov
7981
*/
8082
public class JedisClusterConnectionTests implements ClusterConnectionTests {
8183

@@ -2368,10 +2370,8 @@ public void bitFieldSetShouldWorkCorrectly() {
23682370
@IfProfileValue(name = "redisVersion", value = "3.2+")
23692371
public void bitFieldGetShouldWorkCorrectly() {
23702372

2371-
assertThat(
2372-
clusterConnection.stringCommands().bitField(JedisConverters.toBytes(KEY_1),
2373-
create().get(INT_8).valueAt(offset(0L))),
2374-
contains(0L));
2373+
assertThat(clusterConnection.stringCommands().bitField(JedisConverters.toBytes(KEY_1),
2374+
create().get(INT_8).valueAt(offset(0L))), contains(0L));
23752375
}
23762376

23772377
@Test // DATAREDIS-562
@@ -2392,59 +2392,88 @@ public void bitFieldIncrByWithOverflowShouldWorkCorrectly() {
23922392
create().incr(unsigned(2)).valueAt(offset(102L)).overflow(FAIL).by(1L)), contains(2L));
23932393
assertThat(clusterConnection.stringCommands().bitField(JedisConverters.toBytes(KEY_1),
23942394
create().incr(unsigned(2)).valueAt(offset(102L)).overflow(FAIL).by(1L)), contains(3L));
2395-
assertThat(
2396-
clusterConnection.stringCommands()
2397-
.bitField(JedisConverters.toBytes(KEY_1),
2398-
create().incr(unsigned(2)).valueAt(offset(102L)).overflow(FAIL).by(1L))
2399-
.get(0),
2400-
is(nullValue()));
2395+
assertThat(clusterConnection.stringCommands().bitField(JedisConverters.toBytes(KEY_1),
2396+
create().incr(unsigned(2)).valueAt(offset(102L)).overflow(FAIL).by(1L)).get(0), is(nullValue()));
24012397
}
24022398

24032399
@Test // DATAREDIS-562
24042400
@IfProfileValue(name = "redisVersion", value = "3.2+")
24052401
public void bitfieldShouldAllowMultipleSubcommands() {
24062402

2407-
assertThat(
2408-
clusterConnection.stringCommands().bitField(JedisConverters.toBytes(KEY_1),
2409-
create().incr(signed(5)).valueAt(offset(100L)).by(1L).get(unsigned(4)).valueAt(0L)),
2410-
contains(1L, 0L));
2403+
assertThat(clusterConnection.stringCommands().bitField(JedisConverters.toBytes(KEY_1),
2404+
create().incr(signed(5)).valueAt(offset(100L)).by(1L).get(unsigned(4)).valueAt(0L)), contains(1L, 0L));
24112405
}
24122406

24132407
@Test // DATAREDIS-562
24142408
@IfProfileValue(name = "redisVersion", value = "3.2+")
24152409
public void bitfieldShouldWorkUsingNonZeroBasedOffset() {
24162410

24172411
assertThat(clusterConnection.stringCommands().bitField(JedisConverters.toBytes(KEY_1),
2418-
create().set(INT_8).valueAt(offset(0L).multipliedByTypeLength())
2419-
.to(100L).set(INT_8).valueAt(offset(1L).multipliedByTypeLength()).to(200L)), contains(0L, 0L));
2412+
create().set(INT_8).valueAt(offset(0L).multipliedByTypeLength()).to(100L).set(INT_8)
2413+
.valueAt(offset(1L).multipliedByTypeLength()).to(200L)),
2414+
contains(0L, 0L));
24202415
assertThat(
2421-
clusterConnection.stringCommands()
2422-
.bitField(JedisConverters.toBytes(KEY_1), create().get(INT_8)
2423-
.valueAt(offset(0L).multipliedByTypeLength())
2424-
.get(INT_8).valueAt(offset(1L).multipliedByTypeLength())), contains(100L, -56L));
2416+
clusterConnection.stringCommands().bitField(JedisConverters.toBytes(KEY_1), create().get(INT_8)
2417+
.valueAt(offset(0L).multipliedByTypeLength()).get(INT_8).valueAt(offset(1L).multipliedByTypeLength())),
2418+
contains(100L, -56L));
24252419
}
24262420

24272421
@Test // DATAREDIS-1005
24282422
@IfProfileValue(name = "redisVersion", value = "2.6+")
2429-
public void evalOK() {
2430-
byte[] keyAndArgs = JedisConverters.toBytes("FOO");
2423+
public void evalShouldRunScript() {
24312424

2425+
byte[] keyAndArgs = JedisConverters.toBytes("FOO");
24322426
String luaScript = "return redis.call(\"INCR\", KEYS[1])";
24332427
byte[] luaScriptBin = JedisConverters.toBytes(luaScript);
2428+
24342429
Long result = clusterConnection.scriptingCommands().eval(luaScriptBin, ReturnType.VALUE, 1, keyAndArgs);
2430+
24352431
assertThat(result, is(1L));
24362432
}
24372433

24382434
@Test // DATAREDIS-1005
24392435
@IfProfileValue(name = "redisVersion", value = "2.6+")
2440-
public void evalShaOK() {
2441-
byte[] keyAndArgs = JedisConverters.toBytes("FOO");
2436+
public void scriptLoadShouldLoadScript() {
24422437

24432438
String luaScript = "return redis.call(\"INCR\", KEYS[1])";
24442439
String digest = DigestUtils.sha1DigestAsHex(luaScript);
2445-
byte[] luaScriptBin = JedisConverters.toBytes(digest);
2446-
Long result = clusterConnection.scriptingCommands().evalSha(luaScriptBin, ReturnType.VALUE, 1, keyAndArgs);
2447-
assertThat(result, is(1L));
2440+
byte[] luaScriptBin = JedisConverters.toBytes(luaScript);
2441+
2442+
String result = clusterConnection.scriptingCommands().scriptLoad(luaScriptBin);
2443+
2444+
assertThat(result, is(digest));
24482445
}
24492446

2447+
@Test // DATAREDIS-1005
2448+
@IfProfileValue(name = "redisVersion", value = "2.6+")
2449+
public void scriptFlushShouldRemoveScripts() {
2450+
2451+
byte[] keyAndArgs = JedisConverters.toBytes("FOO");
2452+
String luaScript = "return redis.call(\"GET\", KEYS[1])";
2453+
byte[] luaScriptBin = JedisConverters.toBytes(luaScript);
2454+
2455+
clusterConnection.scriptingCommands().scriptLoad(luaScriptBin);
2456+
clusterConnection.scriptingCommands().scriptFlush();
2457+
2458+
try {
2459+
clusterConnection.scriptingCommands().evalSha(luaScriptBin, ReturnType.VALUE, 1, keyAndArgs);
2460+
fail("expected InvalidDataAccessApiUsageException");
2461+
} catch (InvalidDataAccessApiUsageException e) {
2462+
assertThat(e.getMessage(), containsString("NOSCRIPT"));
2463+
}
2464+
}
2465+
2466+
@Test // DATAREDIS-1005
2467+
@IfProfileValue(name = "redisVersion", value = "2.6+")
2468+
public void evelShaShouldRunScript() {
2469+
2470+
byte[] keyAndArgs = JedisConverters.toBytes("FOO");
2471+
String luaScript = "return redis.call(\"INCR\", KEYS[1])";
2472+
byte[] digest = JedisConverters.toBytes(DigestUtils.sha1DigestAsHex(luaScript));
2473+
2474+
clusterConnection.scriptingCommands().scriptLoad(JedisConverters.toBytes(luaScript));
2475+
2476+
Long result = clusterConnection.scriptingCommands().evalSha(digest, ReturnType.VALUE, 1, keyAndArgs);
2477+
assertThat(result, is(1L));
2478+
}
24502479
}

0 commit comments

Comments
 (0)