DATAREDIS-552 - Polishing.

Apply client name to Sentinel and unpooled connections if set. Add ticket references to JavaDoc. Add tests. Fix existing JavaDoc letter casing and punctuation. Rearrange isClusterAware/isRedisSentinelAware methods.

Original pull request: #219.
This commit is contained in:
Mark Paluch
2016-11-30 11:55:59 +01:00
parent 9117352ac0
commit 116be39a7e
7 changed files with 148 additions and 74 deletions

View File

@@ -68,6 +68,7 @@ import org.springframework.util.ClassUtils;
import org.springframework.util.CollectionUtils;
import org.springframework.util.ObjectUtils;
import org.springframework.util.ReflectionUtils;
import org.springframework.util.StringUtils;
import redis.clients.jedis.BinaryJedis;
import redis.clients.jedis.BinaryJedisPubSub;
@@ -149,6 +150,7 @@ public class JedisConnection extends AbstractRedisConnection {
private volatile JedisSubscription subscription;
private volatile Pipeline pipeline;
private final int dbIndex;
private final String clientName;
private boolean convertPipelineAndTxResults = true;
private List<FutureResult<Response<?>>> pipelinedResults = new ArrayList<FutureResult<Response<?>>>();
private Queue<FutureResult<Response<?>>> txResults = new LinkedList<FutureResult<Response<?>>>();
@@ -192,15 +194,30 @@ public class JedisConnection extends AbstractRedisConnection {
*
* @param jedis
* @param pool can be null, if no pool is used
* @param dbIndex
*/
public JedisConnection(Jedis jedis, Pool<Jedis> pool, int dbIndex) {
this.jedis = jedis;
this(jedis, pool, dbIndex, null);
}
/**
* 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 dbIndex
* @param clientName the client name, can be {@literal null}.
* @since 1.8
*/
protected JedisConnection(Jedis jedis, Pool<Jedis> pool, int dbIndex, String clientName) {
// extract underlying connection for batch operations
client = (Client) ReflectionUtils.getField(CLIENT_FIELD, jedis);
this.jedis = jedis;
this.pool = pool;
this.dbIndex = dbIndex;
this.clientName = clientName;
// select the db
// if this fail, do manual clean-up before propagating the exception
@@ -3988,7 +4005,14 @@ public class JedisConnection extends AbstractRedisConnection {
}
protected Jedis getJedis(RedisNode node) {
return new Jedis(node.getHost(), node.getPort());
Jedis jedis = new Jedis(node.getHost(), node.getPort());
if (StringUtils.hasText(clientName)) {
jedis.clientSetname(clientName);
}
return jedis;
}
@Override

View File

@@ -193,9 +193,12 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
if (usePool && pool != null) {
return pool.getResource();
}
Jedis jedis = new Jedis(getShardInfo());
// force initialization (see Jedis issue #82)
jedis.connect();
potentiallySetClientName(jedis);
return jedis;
} catch (Exception ex) {
throw new RedisConnectionFailureException("Cannot get Jedis connection", ex);
@@ -343,8 +346,8 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
}
Jedis jedis = fetchJedisConnector();
JedisConnection connection = (usePool ? new JedisConnection(jedis, pool, dbIndex)
: new JedisConnection(jedis, null, dbIndex));
JedisConnection connection = (usePool ? new JedisConnection(jedis, pool, dbIndex, clientName)
: new JedisConnection(jedis, null, dbIndex, clientName));
connection.setConvertPipelineAndTxResults(convertPipelineAndTxResults);
return postProcessConnection(connection);
}
@@ -373,7 +376,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Returns the Redis hostName.
*
* @return Returns the hostName
* @return the hostName.
*/
public String getHostName() {
return hostName;
@@ -382,7 +385,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Sets the Redis hostName.
*
* @param hostName The hostName to set.
* @param hostName the hostName to set.
*/
public void setHostName(String hostName) {
this.hostName = hostName;
@@ -401,7 +404,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Returns whether to use SSL.
*
* @return use of SSL
* @return use of SSL.
* @since 1.8
*/
public boolean isUseSsl() {
@@ -411,7 +414,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Returns the password used for authenticating with the Redis server.
*
* @return password for authentication
* @return password for authentication.
*/
public String getPassword() {
return password;
@@ -420,7 +423,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Sets the password used for authenticating with the Redis server.
*
* @param password the password to set
* @param password the password to set.
*/
public void setPassword(String password) {
this.password = password;
@@ -429,7 +432,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Returns the port used to connect to the Redis instance.
*
* @return Redis port.
* @return the Redis port.
*/
public int getPort() {
return port;
@@ -438,7 +441,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Sets the port used to connect to the Redis instance.
*
* @param port Redis port
* @param port the Redis port.
*/
public void setPort(int port) {
this.port = port;
@@ -447,7 +450,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Returns the shardInfo.
*
* @return Returns the shardInfo
* @return the shardInfo.
*/
public JedisShardInfo getShardInfo() {
return shardInfo;
@@ -456,7 +459,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Sets the shard info for this factory.
*
* @param shardInfo The shardInfo to set.
* @param shardInfo the shardInfo to set.
*/
public void setShardInfo(JedisShardInfo shardInfo) {
this.shardInfo = shardInfo;
@@ -465,14 +468,16 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Returns the timeout.
*
* @return Returns the timeout
* @return the timeout.
*/
public int getTimeout() {
return timeout;
}
/**
* @param timeout The timeout to set.
* Sets the timeout.
*
* @param timeout the timeout to set.
*/
public void setTimeout(int timeout) {
this.timeout = timeout;
@@ -481,7 +486,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Indicates the use of a connection pool.
*
* @return Returns the use of connection pooling.
* @return the use of connection pooling.
*/
public boolean getUsePool() {
return usePool;
@@ -490,7 +495,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Turns on or off the use of connection pooling.
*
* @param usePool The usePool to set.
* @param usePool the usePool to set.
*/
public void setUsePool(boolean usePool) {
this.usePool = usePool;
@@ -499,7 +504,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Returns the poolConfig.
*
* @return Returns the poolConfig
* @return the poolConfig
*/
public JedisPoolConfig getPoolConfig() {
return poolConfig;
@@ -508,7 +513,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Sets the pool configuration for this factory.
*
* @param poolConfig The poolConfig to set.
* @param poolConfig the poolConfig to set.
*/
public void setPoolConfig(JedisPoolConfig poolConfig) {
this.poolConfig = poolConfig;
@@ -517,7 +522,7 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Returns the index of the database.
*
* @return Returns the database index
* @return the database index.
*/
public int getDatabase() {
return dbIndex;
@@ -526,26 +531,28 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Sets the index of the database used by this connection factory. Default is 0.
*
* @param index database index
* @param index database index.
*/
public void setDatabase(int index) {
Assert.isTrue(index >= 0, "invalid DB index (a positive index required)");
this.dbIndex = index;
}
/**
* Returns the client name.
*
* @return Returns the client name
*
* @return the client name.
* @since 1.8
*/
public String getClientName() {
return clientName;
}
/**
* Sets the client name used by this connection factory. Default is empty.
*
* @param clientName client name
* Sets the client name used by this connection factory. Defaults to none which does not set a client name.
*
* @param clientName the client name.
* @since 1.8
*/
public void setClientName(String clientName) {
this.clientName = clientName;
@@ -554,9 +561,9 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Specifies if pipelined results should be converted to the expected data type. If false, results of
* {@link JedisConnection#closePipeline()} and {@link JedisConnection#exec()} will be of the type returned by the
* Jedis driver
* Jedis driver.
*
* @return Whether or not to convert pipeline and tx results
* @return Whether or not to convert pipeline and tx results.
*/
public boolean getConvertPipelineAndTxResults() {
return convertPipelineAndTxResults;
@@ -565,9 +572,9 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
/**
* Specifies if pipelined results should be converted to the expected data type. If false, results of
* {@link JedisConnection#closePipeline()} and {@link JedisConnection#exec()} will be of the type returned by the
* Jedis driver
* Jedis driver.
*
* @param convertPipelineAndTxResults Whether or not to convert pipeline and tx results
* @param convertPipelineAndTxResults Whether or not to convert pipeline and tx results.
*/
public void setConvertPipelineAndTxResults(boolean convertPipelineAndTxResults) {
this.convertPipelineAndTxResults = convertPipelineAndTxResults;
@@ -597,14 +604,19 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
private Jedis getActiveSentinel() {
Assert.notNull(this.sentinelConfig);
for (RedisNode node : this.sentinelConfig.getSentinels()) {
Jedis jedis = new Jedis(node.getHost(), node.getPort());
if (jedis.ping().equalsIgnoreCase("pong")) {
potentiallySetClientName(jedis);
return jedis;
}
}
throw new InvalidDataAccessResourceUsageException("no sentinel found");
throw new InvalidDataAccessResourceUsageException("No Sentinel found");
}
private Set<String> convertToJedisSentinelSet(Collection<RedisNode> nodes) {
@@ -622,6 +634,13 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean,
return convertedNodes;
}
private void potentiallySetClientName(Jedis jedis) {
if (StringUtils.hasText(clientName)) {
jedis.clientSetname(clientName);
}
}
private void setTimeoutOn(JedisShardInfo shardInfo, int timeout) {
ReflectionUtils.invokeMethod(SET_TIMEOUT_METHOD, shardInfo, timeout);
}

View File

@@ -257,7 +257,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns the current host.
*
* @return the host
* @return the host.
*/
public String getHostName() {
return hostName;
@@ -266,7 +266,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Sets the host.
*
* @param host the host to set
* @param host the host to set.
*/
public void setHostName(String host) {
this.hostName = host;
@@ -275,7 +275,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns the current port.
*
* @return the port
* @return the port.
*/
public int getPort() {
return port;
@@ -284,7 +284,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Sets the port.
*
* @param port the port to set
* @param port the port to set.
*/
public void setPort(int port) {
this.port = port;
@@ -293,7 +293,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns the connection timeout (in milliseconds).
*
* @return connection timeout
* @return connection timeout.
*/
public long getTimeout() {
return timeout;
@@ -302,14 +302,14 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Sets the connection timeout (in milliseconds).
*
* @param timeout connection timeout
* @param timeout connection timeout.
*/
public void setTimeout(long timeout) {
this.timeout = timeout;
}
/**
* Sets to use SSL connection
* Sets to use SSL connection.
*
* @param useSsl {@literal true} to use SSL.
*/
@@ -320,7 +320,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns whether to use SSL.
*
* @return use of SSL
* @return use of SSL.
*/
public boolean isUseSsl() {
return useSsl;
@@ -338,7 +338,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns whether to verify certificate validity/hostname check when SSL is used.
*
* @return verify peers when using SSL
* @return verify peers when using SSL.
*/
public boolean isVerifyPeer() {
return verifyPeer;
@@ -347,7 +347,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns whether to issue a StartTLS.
*
* @return use of StartTLS
* @return use of StartTLS.
*/
public boolean isStartTls() {
return startTls;
@@ -365,7 +365,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Indicates if validation of the native Lettuce connection is enabled.
*
* @return connection validation enabled
* @return connection validation enabled.
*/
public boolean getValidateConnection() {
return validateConnection;
@@ -382,7 +382,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
* only be used if connection sharing is enabled and there is code that is actively closing the native Lettuce
* connection.
*
* @param validateConnection enable connection validation
* @param validateConnection enable connection validation.
*/
public void setValidateConnection(boolean validateConnection) {
this.validateConnection = validateConnection;
@@ -391,7 +391,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Indicates if multiple {@link LettuceConnection}s should share a single native connection.
*
* @return native connection shared
* @return native connection shared.
*/
public boolean getShareNativeConnection() {
return shareNativeConnection;
@@ -401,7 +401,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
* Enables multiple {@link LettuceConnection}s to share a single native connection. If set to false, every operation
* on {@link LettuceConnection} will open and close a socket.
*
* @param shareNativeConnection enable connection sharing
* @param shareNativeConnection enable connection sharing.
*/
public void setShareNativeConnection(boolean shareNativeConnection) {
this.shareNativeConnection = shareNativeConnection;
@@ -410,7 +410,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns the index of the database.
*
* @return Returns the database index
* @return the database index.
*/
public int getDatabase() {
return dbIndex;
@@ -429,7 +429,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns the password used for authenticating with the Redis server.
*
* @return password for authentication
* @return password for authentication.
*/
public String getPassword() {
return password;
@@ -447,7 +447,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Returns the shutdown timeout for shutting down the RedisClient (in milliseconds).
*
* @return shutdown timeout
* @return shutdown timeout.
* @since 1.6
*/
public long getShutdownTimeout() {
@@ -457,7 +457,7 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Sets the shutdown timeout for shutting down the RedisClient (in milliseconds).
*
* @param shutdownTimeout the shutdown timeout
* @param shutdownTimeout the shutdown timeout.
* @since 1.6
*/
public void setShutdownTimeout(long shutdownTimeout) {
@@ -488,9 +488,9 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Specifies if pipelined results should be converted to the expected data type. If false, results of
* {@link LettuceConnection#closePipeline()} and {LettuceConnection#exec()} will be of the type returned by the
* Lettuce driver
* Lettuce driver.
*
* @return Whether or not to convert pipeline and tx results
* @return Whether or not to convert pipeline and tx results.
*/
public boolean getConvertPipelineAndTxResults() {
return convertPipelineAndTxResults;
@@ -499,14 +499,30 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
/**
* Specifies if pipelined and transaction results should be converted to the expected data type. If false, results of
* {@link LettuceConnection#closePipeline()} and {LettuceConnection#exec()} will be of the type returned by the
* Lettuce driver
* Lettuce driver.
*
* @param convertPipelineAndTxResults Whether or not to convert pipeline and tx results
* @param convertPipelineAndTxResults Whether or not to convert pipeline and tx results.
*/
public void setConvertPipelineAndTxResults(boolean convertPipelineAndTxResults) {
this.convertPipelineAndTxResults = convertPipelineAndTxResults;
}
/**
* @return true when {@link RedisSentinelConfiguration} is present.
* @since 1.5
*/
public boolean isRedisSentinelAware() {
return sentinelConfiguration != null;
}
/**
* @return true when {@link RedisClusterConfiguration} is present.
* @since 1.7
*/
public boolean isClusterAware() {
return clusterConfiguration != null;
}
protected StatefulRedisConnection<byte[], byte[]> getSharedConnection() {
if (shareNativeConnection) {
synchronized (this.connectionMonitor) {
@@ -604,21 +620,6 @@ public class LettuceConnectionFactory implements InitializingBean, DisposableBea
return builder.build();
}
/**
* @return true when {@link RedisSentinelConfiguration} is present.
* @since 1.5
*/
public boolean isRedisSentinelAware() {
return sentinelConfiguration != null;
}
/**
* @return since 1.7
*/
public boolean isClusterAware() {
return clusterConfiguration != null;
}
@Override
public RedisSentinelConnection getSentinelConnection() {

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014 the original author or authors.
* Copyright 2014-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -27,6 +27,7 @@ import org.springframework.data.redis.test.util.RedisSentinelRule;
/**
* @author Christoph Strobl
* @author Fu Jian
*/
public class JedisConnectionFactoryTests {
@@ -55,7 +56,10 @@ public class JedisConnectionFactoryTests {
public void shouldSendCommandCorrectlyViaConnectionFactoryUsingSentinel() {
assertThat(factory.getConnection().ping(), equalTo("PONG"));
}
/**
* @see DATAREDIS-552
*/
@Test
public void getClientNameShouldEqualWithFactorySetting() {
assertThat(factory.getConnection().getClientName(), equalTo("clientName"));

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2011-2015 the original author or authors.
* Copyright 2011-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -13,7 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.redis.connection.jedis;
import static org.hamcrest.CoreMatchers.*;
@@ -62,6 +61,7 @@ import redis.clients.jedis.JedisPoolConfig;
* @author Thomas Darimont
* @author Christoph Strobl
* @author David Liu
* @author Mark Paluch
*/
@RunWith(RelaxedJUnit4ClassRunner.class)
@ContextConfiguration
@@ -402,6 +402,14 @@ public class JedisConnectionIntegrationTests extends AbstractConnectionIntegrati
assertThat(connection.getSentinelConnection(), notNullValue());
}
/**
* @see DATAREDIS-552
*/
@Test
public void shouldSetClientName() {
assertThat(connection.getClientName(), is(equalTo("jedis-client")));
}
/**
* @see DATAREDIS-106
*/

View File

@@ -18,6 +18,8 @@ package org.springframework.data.redis.connection.jedis;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import redis.clients.jedis.Jedis;
import java.util.Collection;
import java.util.List;
@@ -36,10 +38,12 @@ import org.springframework.data.redis.connection.ReturnType;
import org.springframework.data.redis.test.util.MinimumRedisVersionRule;
import org.springframework.data.redis.test.util.RedisSentinelRule;
import org.springframework.test.annotation.IfProfileValue;
import org.springframework.test.util.ReflectionTestUtils;
/**
* @author Christoph Strobl
* @author Thomas Darimont
* @author Mark Paluch
*/
public class JedisSentinelIntegrationTests extends AbstractConnectionIntegrationTests {
@@ -59,6 +63,7 @@ public class JedisSentinelIntegrationTests extends AbstractConnectionIntegration
@Before
public void setUp() {
JedisConnectionFactory jedisConnectionFactory = new JedisConnectionFactory(SENTINEL_CONFIG);
jedisConnectionFactory.setClientName("jedis-client");
jedisConnectionFactory.afterPropertiesSet();
connectionFactory = jedisConnectionFactory;
super.setUp();
@@ -149,4 +154,16 @@ public class JedisSentinelIntegrationTests extends AbstractConnectionIntegration
assertThat(slaves, hasItems(SLAVE_0, SLAVE_1));
}
/**
* @see DATAREDIS-552
*/
@Test
public void shouldSetClientName() {
RedisSentinelConnection sentinelConnection = connectionFactory.getSentinelConnection();
Jedis jedis = (Jedis) ReflectionTestUtils.getField(sentinelConnection, "jedis");
assertThat(jedis.clientGetname(), is(equalTo("jedis-client")));
}
}

View File

@@ -14,6 +14,7 @@
<bean class="org.springframework.data.redis.SettingsUtils"
factory-method="getPort" />
</property>
<property name="clientName" value="jedis-client" />
</bean>
</beans>