Upgrade to Jedis 3.6.0-RC1

pom.xml

@@ -7,7 +7,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-redis</artifactId>
-	<version>2.5.0-SNAPSHOT</version>
+	<version>2.5.0-GH-SNAPSHOT</version>
 
 	<name>Spring Data Redis</name>
 
@@ -25,7 +25,7 @@
 		<xstream>1.4.16</xstream>
 		<pool>2.9.0</pool>
 		<lettuce>6.1.0.RELEASE</lettuce>
-		<jedis>3.5.2</jedis>
+		<jedis>3.6.0-RC1</jedis>
 		<multithreadedtc>1.01</multithreadedtc>
 		<netty>4.1.60.Final</netty>
 		<java-module-name>spring.data.redis</java-module-name>

src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStreamCommands.java

@@ -274,7 +274,7 @@ public PendingMessages xPending(byte[] key, String groupName, XPendingOptions op
 
 		try {
 
-			List<byte[]> response = connection.getCluster().xpending(key, group,
+			List<Object> response = connection.getCluster().xpending(key, group,
 					JedisConverters.toBytes(getLowerValue(range)), JedisConverters.toBytes(getUpperValue(range)),
 					options.getCount().intValue(), JedisConverters.toBytes(options.getConsumerName()));
 

src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java

@@ -15,15 +15,7 @@
  */
 package org.springframework.data.redis.connection.jedis;
 
-import redis.clients.jedis.BinaryJedis;
-import redis.clients.jedis.BinaryJedisPubSub;
-import redis.clients.jedis.Client;
-import redis.clients.jedis.Connection;
-import redis.clients.jedis.Jedis;
-import redis.clients.jedis.MultiKeyPipelineBase;
-import redis.clients.jedis.Pipeline;
-import redis.clients.jedis.Response;
-import redis.clients.jedis.Transaction;
+import redis.clients.jedis.*;
 import redis.clients.jedis.exceptions.JedisDataException;
 import redis.clients.jedis.util.Pool;
 
@@ -49,7 +41,6 @@
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.CollectionUtils;
-import org.springframework.util.StringUtils;
 
 /**
  * {@code RedisConnection} implementation on top of <a href="https://github.com/xetorthio/jedis">Jedis</a> library.
@@ -81,6 +72,8 @@ public class JedisConnection extends AbstractRedisConnection {
 
 	private final @Nullable Pool<Jedis> pool;
 	private final String clientName;
+	private final JedisClientConfig nodeConfig;
+	private final JedisClientConfig sentinelConfig;
 
 	private List<JedisResult> pipelinedResults = new ArrayList<>();
 	private Queue<FutureResult<Response<?>>> txResults = new LinkedList<>();
@@ -120,18 +113,38 @@ public JedisConnection(Jedis jedis, Pool<Jedis> pool, int dbIndex) {
 	 * @param clientName the client name, can be {@literal null}.
 	 * @since 1.8
 	 */
-	protected JedisConnection(Jedis jedis, @Nullable Pool<Jedis> pool, int dbIndex, String clientName) {
+	protected JedisConnection(Jedis jedis, @Nullable Pool<Jedis> pool, int dbIndex, @Nullable String clientName) {
+		this(jedis, pool, createConfig(dbIndex, clientName), createConfig(dbIndex, clientName));
+	}
+
+	private static DefaultJedisClientConfig createConfig(int dbIndex, @Nullable String clientName) {
+		return DefaultJedisClientConfig.builder().database(dbIndex).clientName(clientName).build();
+	}
+
+	/**
+	 * Constructs a new <code>JedisConnection</code> instance backed by a jedis pool.
+	 *
+	 * @param jedis
+	 * @param pool can be null, if no pool is used
+	 * @param nodeConfig node configuration
+	 * @param sentinelConfig sentinel configuration
+	 * @since 2.5
+	 */
+	protected JedisConnection(Jedis jedis, @Nullable Pool<Jedis> pool, JedisClientConfig nodeConfig,
+			JedisClientConfig sentinelConfig) {
 
 		this.jedis = jedis;
 		this.pool = pool;
-		this.clientName = clientName;
+		this.clientName = nodeConfig.getClientName();
+		this.nodeConfig = nodeConfig;
+		this.sentinelConfig = sentinelConfig;
 
 		// select the db
 		// if this fail, do manual clean-up before propagating the exception
 		// as we're inside the constructor
-		if (dbIndex != jedis.getDB()) {
+		if (nodeConfig.getDatabase() != jedis.getDB()) {
 			try {
-				select(dbIndex);
+				select(nodeConfig.getDatabase());
 			} catch (DataAccessException ex) {
 				close();
 				throw ex;
@@ -776,14 +789,7 @@ protected JedisSentinelConnection getSentinelConnection(RedisNode sentinel) {
 	}
 
 	protected Jedis getJedis(RedisNode node) {
-
-		Jedis jedis = new Jedis(node.getHost(), node.getPort());
-
-		if (StringUtils.hasText(clientName)) {
-			jedis.clientSetname(clientName);
-		}
-
-		return jedis;
+		return new Jedis(new HostAndPort(node.getHost(), node.getPort()), this.sentinelConfig);
 	}
 
 	@Nullable

src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java

@@ -15,9 +15,10 @@
  */
 package org.springframework.data.redis.connection.jedis;
 
-import redis.clients.jedis.Client;
+import redis.clients.jedis.DefaultJedisClientConfig;
 import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.Jedis;
+import redis.clients.jedis.JedisClientConfig;
 import redis.clients.jedis.JedisCluster;
 import redis.clients.jedis.JedisPool;
 import redis.clients.jedis.JedisPoolConfig;
@@ -41,6 +42,7 @@
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
+
 import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.dao.DataAccessException;
@@ -58,6 +60,7 @@
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.CollectionUtils;
+import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
 /**
@@ -88,6 +91,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
 
 	private final JedisClientConfiguration clientConfiguration;
 	private @Nullable JedisShardInfo shardInfo;
+	private JedisClientConfig clientConfig = DefaultJedisClientConfig.builder().build();
 	private boolean providedShardInfo = false;
 	private @Nullable Pool<Jedis> pool;
 	private boolean convertPipelineAndTxResults = true;
@@ -277,7 +281,6 @@ protected Jedis fetchJedisConnector() {
 			// force initialization (see Jedis issue #82)
 			jedis.connect();
 
-			potentiallySetClientName(jedis);
 			return jedis;
 		} catch (Exception ex) {
 			throw new RedisConnectionFailureException("Cannot get Jedis connection", ex);
@@ -290,17 +293,7 @@ private Jedis createJedis() {
 			return new Jedis(getShardInfo());
 		}
 
-		Jedis jedis = new Jedis(getHostName(), getPort(), getConnectTimeout(), getReadTimeout(), isUseSsl(),
-				clientConfiguration.getSslSocketFactory().orElse(null), //
-				clientConfiguration.getSslParameters().orElse(null), //
-				clientConfiguration.getHostnameVerifier().orElse(null));
-
-		Client client = jedis.getClient();
-
-		getRedisPassword().map(String::new).ifPresent(client::setPassword);
-		client.setDb(getDatabase());
-
-		return jedis;
+		return new Jedis(new HostAndPort(getHostName(), getPort()), this.clientConfig);
 	}
 
 	/**
@@ -320,6 +313,8 @@ protected JedisConnection postProcessConnection(JedisConnection connection) {
 	 */
 	public void afterPropertiesSet() {
 
+		clientConfig = createClientConfig(getRedisUsername(), getRedisPassword());
+
 		if (shardInfo == null && clientConfiguration instanceof MutableJedisClientConfiguration) {
 
 			providedShardInfo = false;
@@ -357,6 +352,33 @@ public void afterPropertiesSet() {
 		}
 	}
 
+	private JedisClientConfig createClientConfig(@Nullable String username, RedisPassword password) {
+
+		DefaultJedisClientConfig.Builder builder = DefaultJedisClientConfig.builder();
+
+		clientConfiguration.getClientName().ifPresent(builder::clientName);
+		builder.connectionTimeoutMillis(getConnectTimeout());
+		builder.socketTimeoutMillis(getReadTimeout());
+
+		builder.database(getDatabase());
+
+		if (!ObjectUtils.isEmpty(username)) {
+			builder.user(username);
+		}
+		password.toOptional().map(String::new).ifPresent(builder::password);
+
+		if (isUseSsl()) {
+
+			builder.ssl(true);
+
+			clientConfiguration.getSslSocketFactory().ifPresent(builder::sslSocketFactory);
+			clientConfiguration.getHostnameVerifier().ifPresent(builder::hostnameVerifier);
+			clientConfiguration.getSslParameters().ifPresent(builder::sslParameters);
+		}
+
+		return builder.build();
+	}
+
 	private Pool<Jedis> createPool() {
 
 		if (isRedisSentinelAware()) {
@@ -374,13 +396,12 @@ private Pool<Jedis> createPool() {
 	 */
 	protected Pool<Jedis> createRedisSentinelPool(RedisSentinelConfiguration config) {
 
-		GenericObjectPoolConfig<?> poolConfig = getPoolConfig() != null ? getPoolConfig() : new JedisPoolConfig();
+		GenericObjectPoolConfig<Jedis> poolConfig = getPoolConfig() != null ? getPoolConfig() : new JedisPoolConfig();
 		String sentinelUser = null;
-		String sentinelPassword = config.getSentinelPassword().toOptional().map(String::new).orElse(null);
 
+		JedisClientConfig sentinelConfig = createClientConfig(sentinelUser, config.getSentinelPassword());
 		return new JedisSentinelPool(config.getMaster().getName(), convertToJedisSentinelSet(config.getSentinels()),
-				poolConfig, getConnectTimeout(), getReadTimeout(), getUsername(), getPassword(), getDatabase(), getClientName(),
-				getConnectTimeout(), getReadTimeout(), sentinelUser, sentinelPassword, getClientName());
+				poolConfig, this.clientConfig, sentinelConfig);
 	}
 
 	/**
@@ -390,12 +411,7 @@ poolConfig, getConnectTimeout(), getReadTimeout(), getUsername(), getPassword(),
 	 * @since 1.4
 	 */
 	protected Pool<Jedis> createRedisPool() {
-
-		return new JedisPool(getPoolConfig(), getHostName(), getPort(), getConnectTimeout(), getReadTimeout(),
-				getUsername(), getPassword(), getDatabase(), getClientName(), isUseSsl(),
-				clientConfiguration.getSslSocketFactory().orElse(null), //
-				clientConfiguration.getSslParameters().orElse(null), //
-				clientConfiguration.getHostnameVerifier().orElse(null));
+		return new JedisPool(getPoolConfig(), new HostAndPort(getHostName(), getPort()), this.clientConfig);
 	}
 
 	private JedisCluster createCluster() {
@@ -423,7 +439,8 @@ protected ClusterTopologyProvider createTopologyProvider(JedisCluster cluster) {
 	 * @return the actual {@link JedisCluster}.
 	 * @since 1.7
 	 */
-	protected JedisCluster createCluster(RedisClusterConfiguration clusterConfig, GenericObjectPoolConfig<?> poolConfig) {
+	protected JedisCluster createCluster(RedisClusterConfiguration clusterConfig,
+			GenericObjectPoolConfig<Jedis> poolConfig) {
 
 		Assert.notNull(clusterConfig, "Cluster configuration must not be null!");
 
@@ -434,10 +451,7 @@ protected JedisCluster createCluster(RedisClusterConfiguration clusterConfig, Ge
 
 		int redirects = clusterConfig.getMaxRedirects() != null ? clusterConfig.getMaxRedirects() : 5;
 
-		return new JedisCluster(hostAndPort, getConnectTimeout(), getReadTimeout(), redirects, getUsername(), getPassword(),
-				getClientName(), poolConfig, isUseSsl(), clientConfiguration.getSslSocketFactory().orElse(null),
-				clientConfiguration.getSslParameters().orElse(null), clientConfiguration.getHostnameVerifier().orElse(null),
-				null);
+		return new JedisCluster(hostAndPort, this.clientConfig, redirects, poolConfig);
 	}
 
 	/*
@@ -483,8 +497,15 @@ public RedisConnection getConnection() {
 		}
 
 		Jedis jedis = fetchJedisConnector();
-		JedisConnection connection = (getUsePool() ? new JedisConnection(jedis, pool, getDatabase(), getClientName())
-				: new JedisConnection(jedis, null, getDatabase(), getClientName()));
+		JedisClientConfig sentinelConfig = this.clientConfig;
+
+		SentinelConfiguration sentinelConfiguration = getSentinelConfiguration();
+		if (sentinelConfiguration != null) {
+			sentinelConfig = createClientConfig(null, sentinelConfiguration.getSentinelPassword());
+		}
+
+		JedisConnection connection = (getUsePool() ? new JedisConnection(jedis, pool, this.clientConfig, sentinelConfig)
+				: new JedisConnection(jedis, null, this.clientConfig, sentinelConfig));
 		connection.setConvertPipelineAndTxResults(convertPipelineAndTxResults);
 		return postProcessConnection(connection);
 	}
@@ -553,16 +574,6 @@ public void setUseSsl(boolean useSsl) {
 		getMutableConfiguration().setUseSsl(useSsl);
 	}
 
-	/**
-	 * Returns the username used for authenticating with the Redis server.
-	 *
-	 * @return username for authentication.
-	 */
-	@Nullable
-	private String getUsername() {
-		return getRedisUsername();
-	}
-
 	/**
 	 * Returns the password used for authenticating with the Redis server.
 	 *
@@ -715,7 +726,7 @@ public void setUsePool(boolean usePool) {
 	 * @return the poolConfig
 	 */
 	@Nullable
-	public GenericObjectPoolConfig getPoolConfig() {
+	public GenericObjectPoolConfig<Jedis> getPoolConfig() {
 		return clientConfiguration.getPoolConfig().orElse(null);
 	}
 
@@ -877,44 +888,46 @@ private Jedis getActiveSentinel() {
 		Assert.isTrue(RedisConfiguration.isSentinelConfiguration(configuration), "SentinelConfig must not be null!");
 		SentinelConfiguration sentinelConfiguration = (SentinelConfiguration) configuration;
 
+		JedisClientConfig clientConfig = createClientConfig(null, sentinelConfiguration.getSentinelPassword());
 		for (RedisNode node : sentinelConfiguration.getSentinels()) {
 
-			Jedis jedis = new Jedis(node.getHost(), node.getPort(), getConnectTimeout(), getReadTimeout());
-			sentinelConfiguration.getSentinelPassword().toOptional().map(String::new).ifPresent(jedis::auth);
+			Jedis jedis = null;
+			boolean success = false;
 
 			try {
-				if (jedis.ping().equalsIgnoreCase("pong")) {
 
-					potentiallySetClientName(jedis);
+				jedis = new Jedis(new HostAndPort(node.getHost(), node.getPort()), clientConfig);
+				if (jedis.ping().equalsIgnoreCase("pong")) {
+					success = true;
 					return jedis;
 				}
 			} catch (Exception ex) {
-				log.warn(String.format("Ping failed for sentinel host:%s", node.getHost()), ex);
+				log.warn(String.format("Ping failed for sentinel host: %s", node.getHost()), ex);
+			} finally {
+				if (!success && jedis != null) {
+					jedis.close();
+				}
 			}
 		}
 
 		throw new InvalidDataAccessResourceUsageException("No Sentinel found");
 	}
 
-	private Set<String> convertToJedisSentinelSet(Collection<RedisNode> nodes) {
+	private static Set<HostAndPort> convertToJedisSentinelSet(Collection<RedisNode> nodes) {
 
 		if (CollectionUtils.isEmpty(nodes)) {
 			return Collections.emptySet();
 		}
 
-		Set<String> convertedNodes = new LinkedHashSet<>(nodes.size());
+		Set<HostAndPort> convertedNodes = new LinkedHashSet<>(nodes.size());
 		for (RedisNode node : nodes) {
 			if (node != null) {
-				convertedNodes.add(node.asString());
+				convertedNodes.add(new HostAndPort(node.getHost(), node.getPort()));
 			}
 		}
 		return convertedNodes;
 	}
 
-	private void potentiallySetClientName(Jedis jedis) {
-		clientConfiguration.getClientName().ifPresent(jedis::clientSetname);
-	}
-
 	private int getReadTimeout() {
 		return Math.toIntExact(clientConfiguration.getReadTimeout().toMillis());
 	}

src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java

@@ -87,7 +87,7 @@ void getClientNameShouldEqualWithFactorySetting() {
 	void shouldNotFailOnFirstSentinelDown() {
 
 		RedisSentinelConfiguration oneDownSentinelConfig = new RedisSentinelConfiguration().master("mymaster")
-				.sentinel("any.unavailable.host", 26379).sentinel("127.0.0.1", 26379);
+				.sentinel("127.0.0.1", 1).sentinel("127.0.0.1", 26379);
 
 		factory = new JedisConnectionFactory(oneDownSentinelConfig);
 		assertThat(factory.getSentinelConnection().isOpen()).isTrue();