Skip to content

Commit 15fd1d3

Browse files
mp911dechristophstrobl
mp911de
authored and
christophstrobl
committed
DATAREDIS-588 - Polishing.
Simplify SET command usage with Jedis using various expiry and presence/absence flags to use consistently SetParams. Original Pull Request: spring-projects#463
1 parent f6e0574 commit 15fd1d3

File tree

4 files changed

+31
-107
lines changed

4 files changed

+31
-107
lines changed

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

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import java.util.Arrays;
2626
import java.util.List;
2727
import java.util.Map;
28-
import java.util.concurrent.TimeUnit;
2928

3029
import org.springframework.dao.DataAccessException;
3130
import org.springframework.dao.InvalidDataAccessApiUsageException;
@@ -40,7 +39,6 @@
4039
import org.springframework.data.redis.core.types.Expiration;
4140
import org.springframework.data.redis.util.ByteUtils;
4241
import org.springframework.util.Assert;
43-
import org.springframework.util.ObjectUtils;
4442

4543
/**
4644
* @author Christoph Strobl
@@ -134,39 +132,12 @@ public Boolean set(byte[] key, byte[] value, Expiration expiration, SetOption op
134132
Assert.notNull(expiration, "Expiration must not be null!");
135133
Assert.notNull(option, "Option must not be null!");
136134

137-
if (expiration.isPersistent()) {
135+
SetParams setParams = JedisConverters.toSetCommandExPxArgument(expiration, JedisConverters.toSetCommandNxXxArgument(option));
138136

139-
if (ObjectUtils.nullSafeEquals(SetOption.UPSERT, option)) {
140-
return set(key, value);
141-
} else {
142-
143-
// BinaryCluster does not support set with nxxx and binary key/value pairs.
144-
if (ObjectUtils.nullSafeEquals(SetOption.SET_IF_PRESENT, option)) {
145-
throw new UnsupportedOperationException("Jedis does not support SET XX without PX or EX on BinaryCluster.");
146-
}
147-
148-
return setNX(key, value);
149-
}
150-
} else {
151-
152-
if (ObjectUtils.nullSafeEquals(SetOption.UPSERT, option)) {
153-
154-
if (ObjectUtils.nullSafeEquals(TimeUnit.MILLISECONDS, expiration.getTimeUnit())) {
155-
return pSetEx(key, expiration.getExpirationTime(), value);
156-
} else {
157-
return setEx(key, expiration.getExpirationTime(), value);
158-
}
159-
} else {
160-
161-
SetParams params = JedisConverters.toSetCommandNxXxArgument(option);
162-
JedisConverters.toSetCommandExPxArgument(expiration, params);
163-
164-
try {
165-
return Converters.stringToBoolean(connection.getCluster().set(key, value, params));
166-
} catch (Exception ex) {
167-
throw convertJedisAccessException(ex);
168-
}
169-
}
137+
try {
138+
return Converters.stringToBoolean(connection.getCluster().set(key, value, setParams));
139+
} catch (Exception ex) {
140+
throw convertJedisAccessException(ex);
170141
}
171142
}
172143

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -533,11 +533,15 @@ public static SetParams toSetCommandExPxArgument(Expiration expiration, SetParam
533533

534534
SetParams paramsToUse = params == null ? SetParams.setParams() : params;
535535

536-
if (expiration.getTimeUnit() == TimeUnit.MILLISECONDS) {
537-
return paramsToUse.px(expiration.getExpirationTime());
536+
if (!expiration.isPersistent()) {
537+
if (expiration.getTimeUnit() == TimeUnit.MILLISECONDS) {
538+
return paramsToUse.px(expiration.getExpirationTime());
539+
}
540+
541+
return paramsToUse.ex((int) expiration.getExpirationTime());
538542
}
539543

540-
return paramsToUse.ex((int) expiration.getExpirationTime());
544+
return params;
541545
}
542546

543547
/**

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

Lines changed: 14 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323

2424
import java.util.List;
2525
import java.util.Map;
26-
import java.util.concurrent.TimeUnit;
2726

2827
import org.springframework.data.domain.Range;
2928
import org.springframework.data.redis.connection.BitFieldSubCommands;
@@ -33,7 +32,6 @@
3332
import org.springframework.data.redis.util.ByteUtils;
3433
import org.springframework.lang.Nullable;
3534
import org.springframework.util.Assert;
36-
import org.springframework.util.ObjectUtils;
3735

3836
/**
3937
* @author Christoph Strobl
@@ -159,79 +157,26 @@ public Boolean set(byte[] key, byte[] value, Expiration expiration, SetOption op
159157
Assert.notNull(expiration, "Expiration must not be null!");
160158
Assert.notNull(option, "Option must not be null!");
161159

162-
if (expiration.isPersistent()) {
160+
SetParams params = JedisConverters.toSetCommandExPxArgument(expiration, JedisConverters.toSetCommandNxXxArgument(option));
163161

164-
if (ObjectUtils.nullSafeEquals(SetOption.UPSERT, option)) {
165-
return set(key, value);
166-
} else {
167-
168-
try {
169-
SetParams nxxx = JedisConverters.toSetCommandNxXxArgument(option);
170-
171-
if (isPipelined()) {
172-
173-
pipeline(connection.newJedisResult(connection.getRequiredPipeline().set(key, value, nxxx),
174-
Converters.stringToBooleanConverter(), () -> false));
175-
return null;
176-
}
177-
if (isQueueing()) {
178-
179-
transaction(connection.newJedisResult(connection.getRequiredTransaction().set(key, value, nxxx),
180-
Converters.stringToBooleanConverter(), () -> false));
181-
return null;
182-
}
162+
try {
163+
if (isPipelined()) {
183164

184-
return Converters.stringToBoolean(connection.getJedis().set(key, value, nxxx));
185-
} catch (Exception ex) {
186-
throw convertJedisAccessException(ex);
187-
}
165+
pipeline(connection.newJedisResult(connection.getRequiredPipeline().set(key, value, params),
166+
Converters.stringToBooleanConverter(), () -> false));
167+
return null;
188168
}
169+
if (isQueueing()) {
189170

190-
} else {
191-
192-
if (ObjectUtils.nullSafeEquals(SetOption.UPSERT, option)) {
193-
194-
if (ObjectUtils.nullSafeEquals(TimeUnit.MILLISECONDS, expiration.getTimeUnit())) {
195-
return pSetEx(key, expiration.getExpirationTime(), value);
196-
} else {
197-
return setEx(key, expiration.getExpirationTime(), value);
198-
}
199-
} else {
200-
201-
SetParams params = JedisConverters.toSetCommandNxXxArgument(option, null);
202-
JedisConverters.toSetCommandExPxArgument(expiration, params);
203-
204-
try {
205-
if (isPipelined()) {
206-
207-
if (expiration.getExpirationTime() > Integer.MAX_VALUE) {
208-
209-
throw new IllegalArgumentException(
210-
"Expiration.expirationTime must be less than Integer.MAX_VALUE for pipeline in Jedis.");
211-
}
212-
213-
pipeline(connection.newJedisResult(connection.getRequiredPipeline().set(key, value, params),
214-
Converters.stringToBooleanConverter(), () -> false));
215-
return null;
216-
}
217-
if (isQueueing()) {
218-
219-
if (expiration.getExpirationTime() > Integer.MAX_VALUE) {
220-
throw new IllegalArgumentException(
221-
"Expiration.expirationTime must be less than Integer.MAX_VALUE for transactions in Jedis.");
222-
}
223-
224-
transaction(connection.newJedisResult(connection.getRequiredTransaction().set(key, value, params),
225-
Converters.stringToBooleanConverter(), () -> false));
226-
return null;
227-
}
171+
transaction(connection.newJedisResult(connection.getRequiredTransaction().set(key, value, params),
172+
Converters.stringToBooleanConverter(), () -> false));
173+
return null;
174+
}
228175

229-
return Converters.stringToBoolean(connection.getJedis().set(key, value, params));
176+
return Converters.stringToBoolean(connection.getJedis().set(key, value, params));
230177

231-
} catch (Exception ex) {
232-
throw convertJedisAccessException(ex);
233-
}
234-
}
178+
} catch (Exception ex) {
179+
throw convertJedisAccessException(ex);
235180
}
236181
}
237182

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1830,11 +1830,15 @@ public void setWithOptionIfAbsentShouldWorkCorrectly() {
18301830
assertThat(nativeConnection.ttl(KEY_1_BYTES)).isEqualTo(-1L);
18311831
}
18321832

1833-
@Test(expected = UnsupportedOperationException.class) // DATAREDIS-316
1833+
@Test // DATAREDIS-316, DATAREDIS-588
18341834
public void setWithOptionIfPresentShouldWorkCorrectly() {
18351835

18361836
nativeConnection.set(KEY_1_BYTES, VALUE_1_BYTES);
18371837
clusterConnection.set(KEY_1_BYTES, VALUE_2_BYTES, Expiration.persistent(), SetOption.ifPresent());
1838+
1839+
assertThat(nativeConnection.exists(KEY_1_BYTES), is(true));
1840+
assertThat(clusterConnection.get(KEY_1_BYTES), is(VALUE_2_BYTES));
1841+
assertThat(nativeConnection.ttl(KEY_1_BYTES), is(-1L));
18381842
}
18391843

18401844
@Test // DATAREDIS-315

0 commit comments

Comments
 (0)