Skip to content

Commit 4b3ff65

Browse files
jencompgeekjencompgeek
committed
Fix ambiguous constructors in DefaultRedisScript
Can't specify a resource location for script as a String in Spring app context, as it would get injected as script text instead. Switched to property injection for clarity.
1 parent a28a4f1 commit 4b3ff65

File tree

4 files changed

+94
-62
lines changed

4 files changed

+94
-62
lines changed

‎src/main/java/org/springframework/data/redis/core/script/DefaultRedisScript.java

Lines changed: 43 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717

1818
import java.io.IOException;
1919

20+
import org.springframework.beans.factory.InitializingBean;
2021
import org.springframework.core.io.Resource;
2122
import org.springframework.scripting.ScriptSource;
2223
import org.springframework.scripting.support.ResourceScriptSource;
2324
import org.springframework.scripting.support.StaticScriptSource;
25+
import org.springframework.util.Assert;
2426

2527
/**
2628
* Default implementation of {@link RedisScript}. Delegates to an underlying {@link ScriptSource} to
@@ -34,7 +36,7 @@
3436
* The script result type. Should be one of Long, Boolean, List, or deserialized value
3537
* type. Can be null if the script returns a throw-away status (i.e "OK")
3638
*/
37-
public class DefaultRedisScript<T> implements RedisScript<T> {
39+
public class DefaultRedisScript<T> implements RedisScript<T>, InitializingBean {
3840

3941
private ScriptSource scriptSource;
4042

@@ -44,38 +46,9 @@ public class DefaultRedisScript<T> implements RedisScript<T> {
4446

4547
private final Object shaModifiedMonitor = new Object();
4648

47-
/**
48-
*
49-
* @param scriptLocation
50-
* The location of the script
51-
* @param resultType
52-
* The Script result type
53-
*/
54-
public DefaultRedisScript(Resource scriptLocation, Class<T> resultType) {
55-
this(new ResourceScriptSource(scriptLocation), resultType);
56-
}
57-
58-
/**
59-
*
60-
* @param script
61-
* The script
62-
* @param resultType
63-
* The Script result type
64-
*/
65-
public DefaultRedisScript(String script, Class<T> resultType) {
66-
this(new StaticScriptSource(script), resultType);
67-
}
68-
69-
/**
70-
*
71-
* @param scriptSource
72-
* The {@link ScriptSource} of the script
73-
* @param resultType
74-
* The Script result type
75-
*/
76-
public DefaultRedisScript(ScriptSource scriptSource, Class<T> resultType) {
77-
this.scriptSource = scriptSource;
78-
this.resultType = resultType;
49+
public void afterPropertiesSet() throws Exception {
50+
Assert.notNull(this.scriptSource, "Either script, script location,"
51+
+ " or script source is required");
7952
}
8053

8154
public String getSha1() {
@@ -98,4 +71,41 @@ public String getScriptAsString() {
9871
throw new ScriptingException("Error reading script text", e);
9972
}
10073
}
74+
75+
/**
76+
*
77+
* @param resultType
78+
* The script result type. Should be one of Long, Boolean, List, or deserialized
79+
* value type. Can be null if the script returns a throw-away status (i.e "OK")
80+
*/
81+
public void setResultType(Class<T> resultType) {
82+
this.resultType = resultType;
83+
}
84+
85+
/**
86+
*
87+
* @param script
88+
* The script text
89+
*/
90+
public void setScriptText(String scriptText) {
91+
this.scriptSource = new StaticScriptSource(scriptText);
92+
}
93+
94+
/**
95+
*
96+
* @param scriptLocation
97+
* The location of the script
98+
*/
99+
public void setLocation(Resource scriptLocation) {
100+
this.scriptSource = new ResourceScriptSource(scriptLocation);
101+
}
102+
103+
/**
104+
*
105+
* @param scriptSource
106+
* A @{link {@link ScriptSource} pointing to the script
107+
*/
108+
public void setScriptSource(ScriptSource scriptSource) {
109+
this.scriptSource = scriptSource;
110+
}
101111
}

‎src/test/java/org/springframework/data/redis/core/RedisTemplateTests.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,9 @@
5757
import org.springframework.data.redis.core.ZSetOperations.TypedTuple;
5858
import org.springframework.data.redis.core.query.SortQueryBuilder;
5959
import org.springframework.data.redis.core.script.DefaultRedisScript;
60-
import org.springframework.data.redis.core.script.RedisScript;
6160
import org.springframework.data.redis.serializer.GenericToStringSerializer;
6261
import org.springframework.data.redis.serializer.RedisSerializer;
6362
import org.springframework.data.redis.serializer.StringRedisSerializer;
64-
import org.springframework.scripting.support.StaticScriptSource;
6563

6664
/**
6765
*
@@ -616,8 +614,9 @@ public void testConvertAndSend() {
616614
public void testExecuteScriptCustomSerializers() {
617615
assumeTrue(RedisTestProfileValueSource.matches("redisVersion", "2.6"));
618616
K key1 = keyFactory.instance();
619-
final RedisScript<String> script = new DefaultRedisScript<String>(new StaticScriptSource(
620-
"return 'Hey'"), String.class);
617+
final DefaultRedisScript<String> script = new DefaultRedisScript<String>();
618+
script.setScriptText("return 'Hey'");
619+
script.setResultType(String.class);
621620
assertEquals("Hey", redisTemplate.execute(script, redisTemplate.getValueSerializer(), new StringRedisSerializer(),
622621
Collections.singletonList(key1)));
623622
}

‎src/test/java/org/springframework/data/redis/core/script/DefaultRedisScriptTests.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ public class DefaultRedisScriptTests {
3434
@Test
3535
public void testGetSha1() {
3636
StaticScriptSource script = new StaticScriptSource("return KEYS[1]");
37-
RedisScript<String> redisScript = new DefaultRedisScript<String>(script, String.class);
37+
DefaultRedisScript<String> redisScript = new DefaultRedisScript<String>();
38+
redisScript.setScriptSource(script);
39+
redisScript.setResultType(String.class);
3840
String sha1 = redisScript.getSha1();
3941
// Ensure multiple calls return same sha
4042
assertEquals(sha1, redisScript.getSha1());
@@ -45,15 +47,23 @@ public void testGetSha1() {
4547

4648
@Test
4749
public void testGetScriptAsString() {
48-
RedisScript<String> redisScript = new DefaultRedisScript<String>("return ARGS[1]",
49-
String.class);
50+
DefaultRedisScript<String> redisScript = new DefaultRedisScript<String>();
51+
redisScript.setScriptText("return ARGS[1]");
52+
redisScript.setResultType(String.class);
5053
assertEquals("return ARGS[1]", redisScript.getScriptAsString());
5154
}
5255

5356
@Test(expected = ScriptingException.class)
5457
public void testGetScriptAsStringError() {
55-
RedisScript<Long> redisScript = new DefaultRedisScript<Long>(new ResourceScriptSource(
56-
new ClassPathResource("nonexistent")), Long.class);
58+
DefaultRedisScript<Long> redisScript = new DefaultRedisScript<Long>();
59+
redisScript.setScriptSource(new ResourceScriptSource(new ClassPathResource("nonexistent")));
60+
redisScript.setResultType(Long.class);
5761
redisScript.getScriptAsString();
5862
}
63+
64+
@Test(expected = IllegalArgumentException.class)
65+
public void initializeWithNoScript() throws Exception {
66+
DefaultRedisScript<Long> redisScript = new DefaultRedisScript<Long>();
67+
redisScript.afterPropertiesSet();
68+
}
5969
}

‎src/test/java/org/springframework/data/redis/core/script/DefaultScriptExecutorTests.java

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,10 @@ public void testExecuteLongResult() {
8484
this.template = new StringRedisTemplate();
8585
template.setConnectionFactory(connFactory);
8686
template.afterPropertiesSet();
87-
RedisScript<Long> script = new DefaultRedisScript<Long>(new ClassPathResource(
88-
"org/springframework/data/redis/core/script/increment.lua"), Long.class);
87+
DefaultRedisScript<Long> script = new DefaultRedisScript<Long>();
88+
script.setLocation(new ClassPathResource(
89+
"org/springframework/data/redis/core/script/increment.lua"));
90+
script.setResultType(Long.class);
8991
ScriptExecutor<String> scriptExecutor = new DefaultScriptExecutor<String>(template);
9092
Long result = scriptExecutor.execute(script, Collections.singletonList("mykey"));
9193
assertNull(result);
@@ -102,8 +104,10 @@ public void testExecuteBooleanResult() {
102104
template.setValueSerializer(new GenericToStringSerializer<Long>(Long.class));
103105
template.setConnectionFactory(connFactory);
104106
template.afterPropertiesSet();
105-
RedisScript<Boolean> script = new DefaultRedisScript<Boolean>(new ClassPathResource(
106-
"org/springframework/data/redis/core/script/cas.lua"), Boolean.class);
107+
DefaultRedisScript<Boolean> script = new DefaultRedisScript<Boolean>();
108+
script.setLocation(new ClassPathResource(
109+
"org/springframework/data/redis/core/script/cas.lua"));
110+
script.setResultType(Boolean.class);
107111
ScriptExecutor<String> scriptExecutor = new DefaultScriptExecutor<String>(template);
108112
template.boundValueOps("counter").set(0l);
109113
Boolean valueSet = scriptExecutor.execute(script, Collections.singletonList("counter"), 0,
@@ -119,8 +123,10 @@ public void testExecuteListResultCustomArgsSerializer() {
119123
template.setConnectionFactory(connFactory);
120124
template.afterPropertiesSet();
121125
template.boundListOps("mylist").leftPushAll("a", "b", "c", "d");
122-
RedisScript<List<String>> script = new DefaultRedisScript(new ClassPathResource(
123-
"org/springframework/data/redis/core/script/bulkpop.lua"), List.class);
126+
DefaultRedisScript<List> script = new DefaultRedisScript<List>();
127+
script.setLocation(new ClassPathResource(
128+
"org/springframework/data/redis/core/script/bulkpop.lua"));
129+
script.setResultType(List.class);
124130
ScriptExecutor<String> scriptExecutor = new DefaultScriptExecutor<String>(template);
125131
List<String> result = scriptExecutor
126132
.execute(script, new GenericToStringSerializer<Long>(Long.class),
@@ -134,8 +140,9 @@ public void testExecuteMixedListResult() {
134140
this.template = new StringRedisTemplate();
135141
template.setConnectionFactory(connFactory);
136142
template.afterPropertiesSet();
137-
RedisScript<List<Object>> script = new DefaultRedisScript(new ClassPathResource(
138-
"org/springframework/data/redis/core/script/popandlength.lua"), List.class);
143+
DefaultRedisScript<List> script = new DefaultRedisScript<List>();
144+
script.setLocation(new ClassPathResource("org/springframework/data/redis/core/script/popandlength.lua"));
145+
script.setResultType(List.class);
139146
ScriptExecutor<String> scriptExecutor = new DefaultScriptExecutor<String>(template);
140147
List<Object> results = scriptExecutor.execute(script, Collections.singletonList("mylist"));
141148
assertEquals(Arrays.asList(new Object[] { null, 0l }), results);
@@ -150,8 +157,9 @@ public void testExecuteValueResult() {
150157
this.template = new StringRedisTemplate();
151158
template.setConnectionFactory(connFactory);
152159
template.afterPropertiesSet();
153-
RedisScript<String> script = new DefaultRedisScript<String>(new StaticScriptSource(
154-
"return redis.call('GET',KEYS[1])"), String.class);
160+
DefaultRedisScript<String> script = new DefaultRedisScript<String>();
161+
script.setScriptText("return redis.call('GET',KEYS[1])");
162+
script.setResultType(String.class);
155163
template.opsForValue().set("foo", "bar");
156164
ScriptExecutor<String> scriptExecutor = new DefaultScriptExecutor<String>(template);
157165
assertEquals("bar", scriptExecutor.execute(script, Collections.singletonList("foo")));
@@ -165,8 +173,8 @@ public void testExecuteStatusResult() {
165173
template.setValueSerializer(new GenericToStringSerializer<Long>(Long.class));
166174
template.setConnectionFactory(connFactory);
167175
template.afterPropertiesSet();
168-
RedisScript script = new DefaultRedisScript(new StaticScriptSource(
169-
"return redis.call('SET',KEYS[1], ARGV[1])"), null);
176+
DefaultRedisScript script = new DefaultRedisScript();
177+
script.setScriptText("return redis.call('SET',KEYS[1], ARGV[1])");
170178
ScriptExecutor<String> scriptExecutor = new DefaultScriptExecutor<String>(template);
171179
assertNull(scriptExecutor.execute(script, Collections.singletonList("foo"), 3l));
172180
assertEquals(Long.valueOf(3), template.opsForValue().get("foo"));
@@ -182,8 +190,10 @@ public void testExecuteCustomResultSerializer() {
182190
template.setValueSerializer(personSerializer);
183191
template.setConnectionFactory(connFactory);
184192
template.afterPropertiesSet();
185-
RedisScript<String> script = new DefaultRedisScript<String>(new StaticScriptSource(
186-
"redis.call('SET',KEYS[1], ARGV[1])\nreturn 'FOO'"), String.class);
193+
DefaultRedisScript<String> script = new DefaultRedisScript<String>();
194+
script.setScriptSource(new StaticScriptSource(
195+
"redis.call('SET',KEYS[1], ARGV[1])\nreturn 'FOO'"));
196+
script.setResultType(String.class);
187197
ScriptExecutor<String> scriptExecutor = new DefaultScriptExecutor<String>(template);
188198
Person joe = new Person("Joe", "Schmoe", 23);
189199
String result = scriptExecutor.execute(script, personSerializer,
@@ -198,8 +208,9 @@ public void testExecutePipelined() {
198208
this.template = new StringRedisTemplate();
199209
template.setConnectionFactory(connFactory);
200210
template.afterPropertiesSet();
201-
final RedisScript<String> script = new DefaultRedisScript<String>(new StaticScriptSource(
202-
"return KEYS[1]"), String.class);
211+
final DefaultRedisScript<String> script = new DefaultRedisScript<String>();
212+
script.setScriptText("return KEYS[1]");
213+
script.setResultType(String.class);
203214
List<Object> results = template.executePipelined(new SessionCallback<String>() {
204215
@SuppressWarnings("rawtypes")
205216
public String execute(RedisOperations operations) throws DataAccessException {
@@ -217,8 +228,9 @@ public void testExecuteTx() {
217228
this.template = new StringRedisTemplate();
218229
template.setConnectionFactory(connFactory);
219230
template.afterPropertiesSet();
220-
final RedisScript<String> script = new DefaultRedisScript<String>(new StaticScriptSource(
221-
"return 'bar'..KEYS[1]"), String.class);
231+
final DefaultRedisScript<String> script = new DefaultRedisScript<String>();
232+
script.setScriptText("return 'bar'..KEYS[1]");
233+
script.setResultType(String.class);
222234
List<Object> results = (List<Object>) template.execute(new SessionCallback<List<Object>>() {
223235
@SuppressWarnings("rawtypes")
224236
public List<Object> execute(RedisOperations operations) throws DataAccessException {
@@ -238,8 +250,9 @@ public void testExecuteCachedNullKeys() {
238250
this.template = new StringRedisTemplate();
239251
template.setConnectionFactory(connFactory);
240252
template.afterPropertiesSet();
241-
final RedisScript<String> script = new DefaultRedisScript<String>(new StaticScriptSource(
242-
"return 'HELLO'"), String.class);
253+
final DefaultRedisScript<String> script = new DefaultRedisScript<String>();
254+
script.setScriptText("return 'HELLO'");
255+
script.setResultType(String.class);
243256
ScriptExecutor<String> scriptExecutor = new DefaultScriptExecutor<String>(template);
244257
// Execute script twice, second time should be from cache
245258
assertEquals("HELLO", scriptExecutor.execute(script, null));

0 commit comments

Comments
 (0)