diff --git a/pom.xml b/pom.xml index f09c2479a8..973a7d9c43 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-redis - 2.5.0-SNAPSHOT + 2.5.0-GH-SNAPSHOT Spring Data Redis @@ -25,7 +25,7 @@ 1.4.16 2.9.0 6.1.0.RELEASE - 3.5.2 + 3.6.0-RC1 1.01 4.1.60.Final spring.data.redis diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStreamCommands.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStreamCommands.java index 52900832dd..20e9ba9fe9 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterStreamCommands.java +++ b/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 response = connection.getCluster().xpending(key, group, + List response = connection.getCluster().xpending(key, group, JedisConverters.toBytes(getLowerValue(range)), JedisConverters.toBytes(getUpperValue(range)), options.getCount().intValue(), JedisConverters.toBytes(options.getConsumerName())); diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java index 22ab9a0b39..1524bff2dd 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnection.java +++ b/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 Jedis library. @@ -81,6 +72,8 @@ public class JedisConnection extends AbstractRedisConnection { private final @Nullable Pool pool; private final String clientName; + private final JedisClientConfig nodeConfig; + private final JedisClientConfig sentinelConfig; private List pipelinedResults = new ArrayList<>(); private Queue>> txResults = new LinkedList<>(); @@ -120,18 +113,38 @@ public JedisConnection(Jedis jedis, Pool pool, int dbIndex) { * @param clientName the client name, can be {@literal null}. * @since 1.8 */ - protected JedisConnection(Jedis jedis, @Nullable Pool pool, int dbIndex, String clientName) { + protected JedisConnection(Jedis jedis, @Nullable Pool 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 JedisConnection 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 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 diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java index e6ea59b1d3..397b4ab188 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java +++ b/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 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 createPool() { if (isRedisSentinelAware()) { @@ -374,13 +396,12 @@ private Pool createPool() { */ protected Pool createRedisSentinelPool(RedisSentinelConfiguration config) { - GenericObjectPoolConfig poolConfig = getPoolConfig() != null ? getPoolConfig() : new JedisPoolConfig(); + GenericObjectPoolConfig 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 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 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 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 convertToJedisSentinelSet(Collection nodes) { + private static Set convertToJedisSentinelSet(Collection nodes) { if (CollectionUtils.isEmpty(nodes)) { return Collections.emptySet(); } - Set convertedNodes = new LinkedHashSet<>(nodes.size()); + Set 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()); } diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java index 077793f18f..eddbd8464d 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactorySentinelIntegrationTests.java +++ b/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(); diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java index 12244a1f99..ae795fd845 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryUnitTests.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; +import redis.clients.jedis.JedisClientConfig; import redis.clients.jedis.JedisCluster; import redis.clients.jedis.JedisClusterConnectionHandler; import redis.clients.jedis.JedisClusterInfoCache; @@ -292,7 +293,7 @@ void shouldReturnClusterConfiguration() { assertThat(connectionFactory.getClusterConfiguration()).isSameAs(configuration); } - @Test // DATAREDIS-974 + @Test // DATAREDIS-974, GH-2017 void shouldApplySslConfigWhenCreatingClusterClient() throws NoSuchAlgorithmException { SSLParameters sslParameters = new SSLParameters(); @@ -323,16 +324,17 @@ void shouldApplySslConfigWhenCreatingClusterClient() throws NoSuchAlgorithmExcep JedisClusterConnectionHandler connectionHandler = (JedisClusterConnectionHandler) ReflectionTestUtils .getField(cluster, "connectionHandler"); JedisClusterInfoCache cache = (JedisClusterInfoCache) ReflectionTestUtils.getField(connectionHandler, "cache"); - - assertThat(ReflectionTestUtils.getField(cache, "connectionTimeout")).isEqualTo(60000); - assertThat(ReflectionTestUtils.getField(cache, "soTimeout")).isEqualTo(300000); - assertThat(ReflectionTestUtils.getField(cache, "password")).isNull(); - assertThat(ReflectionTestUtils.getField(cache, "clientName")).isEqualTo("my-client"); - assertThat(ReflectionTestUtils.getField(cache, "ssl")).isEqualTo(true); - assertThat(ReflectionTestUtils.getField(cache, "sslSocketFactory")).isEqualTo(socketFactory); - assertThat(ReflectionTestUtils.getField(cache, "sslParameters")).isEqualTo(sslParameters); - assertThat(ReflectionTestUtils.getField(cache, "hostnameVerifier")).isEqualTo(hostNameVerifier); - assertThat(ReflectionTestUtils.getField(cache, "hostAndPortMap")).isNull(); + JedisClientConfig clientConfig = (JedisClientConfig) ReflectionTestUtils.getField(cache, "clientConfig"); + + assertThat(clientConfig.getConnectionTimeoutMillis()).isEqualTo(60000); + assertThat(clientConfig.getSocketTimeoutMillis()).isEqualTo(300000); + assertThat(clientConfig.getPassword()).isNull(); + assertThat(clientConfig.getClientName()).isEqualTo("my-client"); + assertThat(clientConfig.isSsl()).isEqualTo(true); + assertThat(clientConfig.getSslSocketFactory()).isEqualTo(socketFactory); + assertThat(clientConfig.getSslParameters()).isEqualTo(sslParameters); + assertThat(clientConfig.getHostnameVerifier()).isEqualTo(hostNameVerifier); + assertThat(clientConfig.getHostAndPortMapper()).isNull(); } @Test // DATAREDIS-574 diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTests.java index 521950f851..d8533aba12 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionUnitTests.java @@ -38,6 +38,7 @@ import org.springframework.data.redis.connection.RedisZSetCommands.Tuple; import org.springframework.data.redis.core.Cursor; import org.springframework.data.redis.core.ScanOptions; +import org.springframework.test.util.ReflectionTestUtils; /** * @author Christoph Strobl @@ -379,7 +380,7 @@ private static class MockedClientJedis extends Jedis { MockedClientJedis(String host, Client client) { super(host); - this.client = client; + ReflectionTestUtils.setField(this, "client", client); } } }