Skip to content

Commit e6341ad

Browse files
sazzad16sazzad16
authored
More RESP3 coverage (#3387)
* More RESP3 coverage * fix * quick fix * Modify credentials provider test with JedisPooled
1 parent 57b77ef commit e6341ad

12 files changed

+97
-26
lines changed

‎src/main/java/redis/clients/jedis/ClusterPipeline.java

+15-4
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,39 @@
55
import redis.clients.jedis.providers.ClusterConnectionProvider;
66
import redis.clients.jedis.util.IOUtils;
77

8-
// TODO: RESP3
98
public class ClusterPipeline extends MultiNodePipelineBase {
109

1110
private final ClusterConnectionProvider provider;
1211
private AutoCloseable closeable = null;
1312

1413
public ClusterPipeline(Set<HostAndPort> clusterNodes, JedisClientConfig clientConfig) {
15-
this(new ClusterConnectionProvider(clusterNodes, clientConfig));
14+
this(new ClusterConnectionProvider(clusterNodes, clientConfig),
15+
createClusterCommandObjects(clientConfig.getRedisProtocol()));
1616
this.closeable = this.provider;
1717
}
1818

1919
public ClusterPipeline(Set<HostAndPort> clusterNodes, JedisClientConfig clientConfig,
2020
GenericObjectPoolConfig<Connection> poolConfig) {
21-
this(new ClusterConnectionProvider(clusterNodes, clientConfig, poolConfig));
21+
this(new ClusterConnectionProvider(clusterNodes, clientConfig, poolConfig),
22+
createClusterCommandObjects(clientConfig.getRedisProtocol()));
2223
this.closeable = this.provider;
2324
}
2425

2526
public ClusterPipeline(ClusterConnectionProvider provider) {
26-
super(new ClusterCommandObjects());
27+
this(provider, new ClusterCommandObjects());
28+
}
29+
30+
public ClusterPipeline(ClusterConnectionProvider provider, ClusterCommandObjects commandObjects) {
31+
super(commandObjects);
2732
this.provider = provider;
2833
}
2934

35+
private static ClusterCommandObjects createClusterCommandObjects(RedisProtocol protocol) {
36+
ClusterCommandObjects cco = new ClusterCommandObjects();
37+
if (protocol == RedisProtocol.RESP3) cco.setProtocol(protocol);
38+
return cco;
39+
}
40+
3041
@Override
3142
public void close() {
3243
try {

‎src/main/java/redis/clients/jedis/CommandObjects.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -4178,8 +4178,8 @@ public final CommandObject<List<String>> graphExplain(String graphName, String q
41784178
return new CommandObject<>(commandArguments(GraphCommand.EXPLAIN).key(graphName).add(query), BuilderFactory.STRING_LIST);
41794179
}
41804180

4181-
public final CommandObject<List<List<String>>> graphSlowlog(String graphName) {
4182-
return new CommandObject<>(commandArguments(GraphCommand.SLOWLOG).key(graphName), BuilderFactory.STRING_LIST_LIST);
4181+
public final CommandObject<List<List<Object>>> graphSlowlog(String graphName) {
4182+
return new CommandObject<>(commandArguments(GraphCommand.SLOWLOG).key(graphName), BuilderFactory.ENCODED_OBJECT_LIST_LIST);
41834183
}
41844184

41854185
public final CommandObject<String> graphConfigSet(String configName, Object value) {

‎src/main/java/redis/clients/jedis/Connection.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,17 @@ public Connection(final HostAndPort hostAndPort, final JedisClientConfig clientC
5656
initializeFromClientConfig(clientConfig);
5757
}
5858

59+
public Connection(final JedisSocketFactory socketFactory) {
60+
this.socketFactory = socketFactory;
61+
}
62+
5963
public Connection(final JedisSocketFactory socketFactory, JedisClientConfig clientConfig) {
6064
this.socketFactory = socketFactory;
6165
this.soTimeout = clientConfig.getSocketTimeoutMillis();
6266
this.infiniteSoTimeout = clientConfig.getBlockingSocketTimeoutMillis();
6367
initializeFromClientConfig(clientConfig);
6468
}
6569

66-
public Connection(final JedisSocketFactory socketFactory) {
67-
this.socketFactory = socketFactory;
68-
}
69-
7070
@Override
7171
public String toString() {
7272
return "Connection{" + socketFactory + "}";

‎src/main/java/redis/clients/jedis/DefaultJedisClientConfig.java

+7
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,13 @@ public DefaultJedisClientConfig build() {
159159
sslSocketFactory, sslParameters, hostnameVerifier, hostAndPortMapper);
160160
}
161161

162+
/**
163+
* Shortcut to {@link Builder#protocol(redis.clients.jedis.RedisProtocol)} with {@link RedisProtocol#RESP3}.
164+
*/
165+
public Builder resp3() {
166+
return protocol(RedisProtocol.RESP3);
167+
}
168+
162169
public Builder protocol(RedisProtocol protocol) {
163170
this.redisProtocol = protocol;
164171
return this;

‎src/main/java/redis/clients/jedis/JedisCluster.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public Connection getConnectionFromSlot(int slot) {
207207

208208
@Override
209209
public ClusterPipeline pipelined() {
210-
return new ClusterPipeline((ClusterConnectionProvider) provider);
210+
return new ClusterPipeline((ClusterConnectionProvider) provider, (ClusterCommandObjects) commandObjects);
211211
}
212212

213213
/**

‎src/main/java/redis/clients/jedis/JedisSharding.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
* @deprecated Sharding/Sharded feature will be removed in next major release.
1111
*/
1212
@Deprecated
13-
// TODO: RESP3
1413
public class JedisSharding extends UnifiedJedis {
1514

1615
public static final Pattern DEFAULT_KEY_TAG_PATTERN = Pattern.compile("\\{(.+?)\\}");
@@ -21,20 +20,24 @@ public JedisSharding(List<HostAndPort> shards) {
2120

2221
public JedisSharding(List<HostAndPort> shards, JedisClientConfig clientConfig) {
2322
this(new ShardedConnectionProvider(shards, clientConfig));
23+
setProtocol(clientConfig);
2424
}
2525

2626
public JedisSharding(List<HostAndPort> shards, JedisClientConfig clientConfig,
2727
GenericObjectPoolConfig<Connection> poolConfig) {
2828
this(new ShardedConnectionProvider(shards, clientConfig, poolConfig));
29+
setProtocol(clientConfig);
2930
}
3031

3132
public JedisSharding(List<HostAndPort> shards, JedisClientConfig clientConfig, Hashing algo) {
3233
this(new ShardedConnectionProvider(shards, clientConfig, algo));
34+
setProtocol(clientConfig);
3335
}
3436

3537
public JedisSharding(List<HostAndPort> shards, JedisClientConfig clientConfig,
3638
GenericObjectPoolConfig<Connection> poolConfig, Hashing algo) {
3739
this(new ShardedConnectionProvider(shards, clientConfig, poolConfig, algo));
40+
setProtocol(clientConfig);
3841
}
3942

4043
public JedisSharding(ShardedConnectionProvider provider) {
@@ -45,6 +48,11 @@ public JedisSharding(ShardedConnectionProvider provider, Pattern tagPattern) {
4548
super(provider, tagPattern);
4649
}
4750

51+
private void setProtocol(JedisClientConfig clientConfig) {
52+
RedisProtocol proto = clientConfig.getRedisProtocol();
53+
if (proto == RedisProtocol.RESP3) commandObjects.setProtocol(proto);
54+
}
55+
4856
@Override
4957
public ShardedPipeline pipelined() {
5058
return new ShardedPipeline((ShardedConnectionProvider) provider);

‎src/main/java/redis/clients/jedis/MultiNodePipelineBase.java

-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import redis.clients.jedis.util.IOUtils;
4040
import redis.clients.jedis.util.KeyValue;
4141

42-
// TODO: RESP3
4342
public abstract class MultiNodePipelineBase implements PipelineCommands, PipelineBinaryCommands,
4443
RedisModulePipelineCommands, Closeable {
4544

‎src/main/java/redis/clients/jedis/UnifiedJedis.java

+30-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import redis.clients.jedis.commands.SampleBinaryKeyedCommands;
1818
import redis.clients.jedis.commands.SampleKeyedCommands;
1919
import redis.clients.jedis.commands.RedisModuleCommands;
20+
import redis.clients.jedis.exceptions.JedisException;
2021
import redis.clients.jedis.executors.*;
2122
import redis.clients.jedis.graph.GraphCommandObjects;
2223
import redis.clients.jedis.graph.ResultSet;
@@ -41,9 +42,10 @@ public class UnifiedJedis implements JedisCommands, JedisBinaryCommands,
4142
SampleKeyedCommands, SampleBinaryKeyedCommands, RedisModuleCommands,
4243
AutoCloseable {
4344

45+
protected RedisProtocol protocol = null;
4446
protected final ConnectionProvider provider;
4547
protected final CommandExecutor executor;
46-
private final CommandObjects commandObjects;
48+
protected final CommandObjects commandObjects;
4749
private final GraphCommandObjects graphCommandObjects;
4850
private JedisBroadcastAndRoundRobinConfig broadcastAndRoundRobinConfig = null;
4951

@@ -91,6 +93,15 @@ public UnifiedJedis(ConnectionProvider provider) {
9193
this.commandObjects = new CommandObjects();
9294
this.graphCommandObjects = new GraphCommandObjects(this);
9395
this.graphCommandObjects.setBaseCommandArgumentsCreator((comm) -> this.commandObjects.commandArguments(comm));
96+
try (Connection conn = this.provider.getConnection()) {
97+
if (conn != null) {
98+
RedisProtocol proto = conn.getRedisProtocol();
99+
if (proto != null) commandObjects.setProtocol(proto);
100+
}
101+
//} catch (JedisAccessControlException ace) {
102+
} catch (JedisException je) { // TODO: use specific exception(s)
103+
// use default protocol
104+
}
94105
}
95106

96107
/**
@@ -103,6 +114,16 @@ public UnifiedJedis(JedisSocketFactory socketFactory) {
103114
this(new Connection(socketFactory));
104115
}
105116

117+
/**
118+
* The constructor to directly use a custom {@link JedisSocketFactory}.
119+
* <p>
120+
* WARNING: Using this constructor means a {@link NullPointerException} will be occurred if
121+
* {@link UnifiedJedis#provider} is accessed.
122+
*/
123+
public UnifiedJedis(JedisSocketFactory socketFactory, JedisClientConfig clientConfig) {
124+
this(new Connection(socketFactory, clientConfig));
125+
}
126+
106127
/**
107128
* The constructor to directly use a {@link Connection}.
108129
* <p>
@@ -114,6 +135,8 @@ public UnifiedJedis(Connection connection) {
114135
this.executor = new SimpleCommandExecutor(connection);
115136
this.commandObjects = new CommandObjects();
116137
this.graphCommandObjects = new GraphCommandObjects(this);
138+
RedisProtocol proto = connection.getRedisProtocol();
139+
if (proto == RedisProtocol.RESP3) this.commandObjects.setProtocol(proto);
117140
}
118141

119142
public UnifiedJedis(Set<HostAndPort> jedisClusterNodes, JedisClientConfig clientConfig, int maxAttempts) {
@@ -195,6 +218,11 @@ public void close() {
195218
IOUtils.closeQuietly(this.executor);
196219
}
197220

221+
protected final void setProtocol(RedisProtocol protocol) {
222+
this.protocol = protocol;
223+
this.commandObjects.setProtocol(this.protocol);
224+
}
225+
198226
public final <T> T executeCommand(CommandObject<T> commandObject) {
199227
return executor.executeCommand(commandObject);
200228
}
@@ -4685,7 +4713,7 @@ public List<String> graphExplain(String graphName, String query) {
46854713
}
46864714

46874715
@Override
4688-
public List<List<String>> graphSlowlog(String graphName) {
4716+
public List<List<Object>> graphSlowlog(String graphName) {
46894717
return executeCommand(commandObjects.graphSlowlog(graphName));
46904718
}
46914719

‎src/main/java/redis/clients/jedis/graph/RedisGraphCommands.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,7 @@ public interface RedisGraphCommands {
113113
/**
114114
* Returns a list containing up to 10 of the slowest queries issued against the given graph ID.
115115
*/
116-
// TODO: RESP3 --> List<List<Object>> ?
117-
List<List<String>> graphSlowlog(String graphName);
116+
List<List<Object>> graphSlowlog(String graphName);
118117

119118
String graphConfigSet(String configName, Object value);
120119

‎src/test/java/redis/clients/jedis/JedisPooledTest.java

+11-7
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
package redis.clients.jedis;
22

3+
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.equalTo;
5+
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
36
import static org.junit.Assert.assertEquals;
47
import static org.junit.Assert.assertNull;
58
import static org.junit.Assert.assertTrue;
69
import static org.junit.Assert.fail;
710

811
import java.net.URI;
912
import java.net.URISyntaxException;
13+
import java.util.concurrent.atomic.AtomicBoolean;
1014
import java.util.concurrent.atomic.AtomicInteger;
1115
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
1216
import org.junit.Test;
@@ -196,9 +200,9 @@ public void testResetValidCredentials() {
196200
public void testCredentialsProvider() {
197201
final AtomicInteger prepareCount = new AtomicInteger();
198202
final AtomicInteger cleanupCount = new AtomicInteger();
203+
final AtomicBoolean validPassword = new AtomicBoolean(false);
199204

200205
RedisCredentialsProvider credentialsProvider = new RedisCredentialsProvider() {
201-
boolean firstCall = true;
202206

203207
@Override
204208
public void prepare() {
@@ -207,8 +211,7 @@ public void prepare() {
207211

208212
@Override
209213
public RedisCredentials get() {
210-
if (firstCall) {
211-
firstCall = false;
214+
if (!validPassword.get()) {
212215
return new RedisCredentials() { };
213216
}
214217

@@ -244,12 +247,13 @@ public void cleanUp() {
244247
} catch (JedisException e) {
245248
}
246249
assertEquals(0, pool.getPool().getNumActive() + pool.getPool().getNumIdle() + pool.getPool().getNumWaiters());
247-
assertEquals(1, prepareCount.get());
248-
assertEquals(1, cleanupCount.get());
250+
assertThat(prepareCount.getAndSet(0), greaterThanOrEqualTo(1));
251+
assertThat(cleanupCount.getAndSet(0), greaterThanOrEqualTo(1));
249252

253+
validPassword.set(true);
250254
assertNull(pool.get("foo"));
251-
assertEquals(2, prepareCount.get());
252-
assertEquals(2, cleanupCount.get());
255+
assertThat(prepareCount.get(), equalTo(1));
256+
assertThat(cleanupCount.get(), equalTo(1));
253257
}
254258
}
255259
}

‎src/test/java/redis/clients/jedis/UdsTest.java

+16
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,29 @@ public void jedisConnectsToUds() {
1919
}
2020
}
2121

22+
@Test
23+
public void jedisConnectsToUdsResp3() {
24+
try (Jedis jedis = new Jedis(new UdsJedisSocketFactory(),
25+
DefaultJedisClientConfig.builder().resp3().build())) {
26+
assertEquals("PONG", jedis.ping());
27+
}
28+
}
29+
2230
@Test
2331
public void unifiedJedisConnectsToUds() {
2432
try (UnifiedJedis jedis = new UnifiedJedis(new UdsJedisSocketFactory())) {
2533
assertEquals("PONG", jedis.ping());
2634
}
2735
}
2836

37+
@Test
38+
public void unifiedJedisConnectsToUdsResp3() {
39+
try (UnifiedJedis jedis = new UnifiedJedis(new UdsJedisSocketFactory(),
40+
DefaultJedisClientConfig.builder().resp3().build())) {
41+
assertEquals("PONG", jedis.ping());
42+
}
43+
}
44+
2945
private static class UdsJedisSocketFactory implements JedisSocketFactory {
3046

3147
private static final File UDS_SOCKET = new File("/tmp/redis_uds.sock");

‎src/test/java/redis/clients/jedis/modules/graph/GraphAPITest.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -766,12 +766,11 @@ public void explain() {
766766
}
767767

768768
@Test
769-
// TODO: RESP3
770769
public void slowlog() {
771770
assertNotNull(client.graphProfile("social", "CREATE (:person{name:'roi',age:32})"));
772771
assertNotNull(client.graphProfile("social", "CREATE (:person{name:'amit',age:30})"));
773772

774-
List<List<String>> slowlogs = client.graphSlowlog("social");
773+
List<List<Object>> slowlogs = client.graphSlowlog("social");
775774
assertEquals(2, slowlogs.size());
776775
slowlogs.forEach(sl -> assertFalse(sl.isEmpty()));
777776
slowlogs.forEach(sl -> sl.forEach(Assert::assertNotNull));

0 commit comments

Comments
 (0)