Polish "Add separate property for Redis read and connection timeout"

See gh-23137
This commit is contained in:
Stephane Nicoll 2020-09-11 11:34:41 +02:00
parent 2527fcac9c
commit 7a8b7b9fa7
5 changed files with 34 additions and 46 deletions

View File

@ -17,7 +17,6 @@
package org.springframework.boot.autoconfigure.data.redis; package org.springframework.boot.autoconfigure.data.redis;
import java.net.UnknownHostException; import java.net.UnknownHostException;
import java.time.Duration;
import org.apache.commons.pool2.impl.GenericObjectPool; import org.apache.commons.pool2.impl.GenericObjectPool;
import redis.clients.jedis.Jedis; import redis.clients.jedis.Jedis;
@ -27,6 +26,7 @@ import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.PropertyMapper;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.data.redis.connection.RedisClusterConfiguration; import org.springframework.data.redis.connection.RedisClusterConfiguration;
@ -89,20 +89,11 @@ class JedisConnectionConfiguration extends RedisConnectionConfiguration {
} }
private JedisClientConfigurationBuilder applyProperties(JedisClientConfigurationBuilder builder) { private JedisClientConfigurationBuilder applyProperties(JedisClientConfigurationBuilder builder) {
if (getProperties().isSsl()) { PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull();
builder.useSsl(); map.from(getProperties().isSsl()).whenTrue().toCall(builder::useSsl);
} map.from(getProperties().getTimeout()).to(builder::readTimeout);
Duration connectionTimeout = getProperties().getConnectionTimeout(); map.from(getProperties().getConnectTimeout()).to(builder::connectTimeout);
if (connectionTimeout != null) { map.from(getProperties().getClientName()).whenHasText().to(builder::clientName);
builder.connectTimeout(connectionTimeout);
}
Duration timeout = getProperties().getTimeout();
if (timeout != null) {
builder.readTimeout(timeout);
}
if (StringUtils.hasText(getProperties().getClientName())) {
builder.clientName(getProperties().getClientName());
}
return builder; return builder;
} }

View File

@ -98,15 +98,7 @@ class LettuceConnectionConfiguration extends RedisConnectionConfiguration {
if (StringUtils.hasText(getProperties().getUrl())) { if (StringUtils.hasText(getProperties().getUrl())) {
customizeConfigurationFromUrl(builder); customizeConfigurationFromUrl(builder);
} }
builder.clientOptions(createClientOptions());
ClientOptions.Builder clientOptionsBuilder = initializeClientOptionsBuilder();
Duration connectionTimeout = getProperties().getConnectionTimeout();
if (connectionTimeout != null) {
SocketOptions socketOptions = SocketOptions.builder().connectTimeout(connectionTimeout).build();
clientOptionsBuilder.socketOptions(socketOptions);
}
builder.clientOptions(clientOptionsBuilder.timeoutOptions(TimeoutOptions.enabled()).build());
builder.clientResources(clientResources); builder.clientResources(clientResources);
builderCustomizers.orderedStream().forEach((customizer) -> customizer.customize(builder)); builderCustomizers.orderedStream().forEach((customizer) -> customizer.customize(builder));
return builder.build(); return builder.build();
@ -124,9 +116,8 @@ class LettuceConnectionConfiguration extends RedisConnectionConfiguration {
if (getProperties().isSsl()) { if (getProperties().isSsl()) {
builder.useSsl(); builder.useSsl();
} }
Duration timeout = getProperties().getTimeout(); if (getProperties().getTimeout() != null) {
if (timeout != null) { builder.commandTimeout(getProperties().getTimeout());
builder.commandTimeout(timeout);
} }
if (getProperties().getLettuce() != null) { if (getProperties().getLettuce() != null) {
RedisProperties.Lettuce lettuce = getProperties().getLettuce(); RedisProperties.Lettuce lettuce = getProperties().getLettuce();
@ -140,6 +131,15 @@ class LettuceConnectionConfiguration extends RedisConnectionConfiguration {
return builder; return builder;
} }
private ClientOptions createClientOptions() {
ClientOptions.Builder builder = initializeClientOptionsBuilder();
Duration connectTimeout = getProperties().getConnectTimeout();
if (connectTimeout != null) {
builder.socketOptions(SocketOptions.builder().connectTimeout(connectTimeout).build());
}
return builder.timeoutOptions(TimeoutOptions.enabled()).build();
}
private ClientOptions.Builder initializeClientOptionsBuilder() { private ClientOptions.Builder initializeClientOptionsBuilder() {
if (getProperties().getCluster() != null) { if (getProperties().getCluster() != null) {
ClusterClientOptions.Builder builder = ClusterClientOptions.builder(); ClusterClientOptions.Builder builder = ClusterClientOptions.builder();

View File

@ -74,7 +74,7 @@ public class RedisProperties {
/** /**
* Connection timeout. * Connection timeout.
*/ */
private Duration connectionTimeout; private Duration connectTimeout;
/** /**
* Client name to be set on connections with CLIENT SETNAME. * Client name to be set on connections with CLIENT SETNAME.
@ -150,12 +150,12 @@ public class RedisProperties {
return this.timeout; return this.timeout;
} }
public Duration getConnectionTimeout() { public Duration getConnectTimeout() {
return this.connectionTimeout; return this.connectTimeout;
} }
public void setConnectionTimeout(Duration connectionTimeout) { public void setConnectTimeout(Duration connectTimeout) {
this.connectionTimeout = connectionTimeout; this.connectTimeout = connectTimeout;
} }
public String getClientName() { public String getClientName() {

View File

@ -16,14 +16,13 @@
package org.springframework.boot.autoconfigure.data.redis; package org.springframework.boot.autoconfigure.data.redis;
import io.lettuce.core.RedisClient;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.springframework.beans.BeansException; import org.springframework.beans.BeansException;
import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.beans.factory.config.BeanPostProcessor;
import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.FilteredClassLoader;
import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.boot.testsupport.classpath.ClassPathExclusions;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.data.redis.connection.RedisConnectionFactory; import org.springframework.data.redis.connection.RedisConnectionFactory;
@ -38,11 +37,11 @@ import static org.assertj.core.api.Assertions.assertThat;
* @author Mark Paluch * @author Mark Paluch
* @author Stephane Nicoll * @author Stephane Nicoll
*/ */
@ClassPathExclusions("lettuce-core-*.jar")
class RedisAutoConfigurationJedisTests { class RedisAutoConfigurationJedisTests {
private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() private final ApplicationContextRunner contextRunner = new ApplicationContextRunner()
.withConfiguration(AutoConfigurations.of(RedisAutoConfiguration.class)) .withConfiguration(AutoConfigurations.of(RedisAutoConfiguration.class));
.withClassLoader(new FilteredClassLoader(RedisClient.class));
@Test @Test
void connectionFactoryDefaultsToJedis() { void connectionFactoryDefaultsToJedis() {
@ -137,9 +136,9 @@ class RedisAutoConfigurationJedisTests {
} }
@Test @Test
void testRedisConfigurationWithTimeoutAndConnectionTimeout() { void testRedisConfigurationWithTimeoutAndConnectTimeout() {
this.contextRunner.withPropertyValues("spring.redis.host:foo", "spring.redis.timeout:250", this.contextRunner.withPropertyValues("spring.redis.host:foo", "spring.redis.timeout:250",
"spring.redis.connection-timeout:1000").run((context) -> { "spring.redis.connect-timeout:1000").run((context) -> {
JedisConnectionFactory cf = context.getBean(JedisConnectionFactory.class); JedisConnectionFactory cf = context.getBean(JedisConnectionFactory.class);
assertThat(cf.getHostName()).isEqualTo("foo"); assertThat(cf.getHostName()).isEqualTo("foo");
assertThat(cf.getTimeout()).isEqualTo(250); assertThat(cf.getTimeout()).isEqualTo(250);

View File

@ -168,15 +168,14 @@ class RedisAutoConfigurationTests {
} }
@Test @Test
void testRedisConfigurationWithTimeoutAndConnectionTimeout() { void testRedisConfigurationWithTimeoutAndConnectTimeout() {
this.contextRunner.withPropertyValues("spring.redis.host:foo", "spring.redis.timeout:250", this.contextRunner.withPropertyValues("spring.redis.host:foo", "spring.redis.timeout:250",
"spring.redis.connection-timeout:1000").run((context) -> { "spring.redis.connect-timeout:1000").run((context) -> {
LettuceConnectionFactory cf = context.getBean(LettuceConnectionFactory.class); LettuceConnectionFactory cf = context.getBean(LettuceConnectionFactory.class);
assertThat(cf.getHostName()).isEqualTo("foo"); assertThat(cf.getHostName()).isEqualTo("foo");
assertThat(cf.getTimeout()).isEqualTo(250); assertThat(cf.getTimeout()).isEqualTo(250);
long actualConnectionTimeout = cf.getClientConfiguration().getClientOptions().get() assertThat(cf.getClientConfiguration().getClientOptions().get().getSocketOptions()
.getSocketOptions().getConnectTimeout().toMillis(); .getConnectTimeout().toMillis()).isEqualTo(1000);
assertThat(actualConnectionTimeout).isEqualTo(1000);
}); });
} }
@ -186,9 +185,8 @@ class RedisAutoConfigurationTests {
LettuceConnectionFactory cf = context.getBean(LettuceConnectionFactory.class); LettuceConnectionFactory cf = context.getBean(LettuceConnectionFactory.class);
assertThat(cf.getHostName()).isEqualTo("foo"); assertThat(cf.getHostName()).isEqualTo("foo");
assertThat(cf.getTimeout()).isEqualTo(60000); assertThat(cf.getTimeout()).isEqualTo(60000);
long actualConnectionTimeout = cf.getClientConfiguration().getClientOptions().get().getSocketOptions() assertThat(cf.getClientConfiguration().getClientOptions().get().getSocketOptions().getConnectTimeout()
.getConnectTimeout().toMillis(); .toMillis()).isEqualTo(10000);
assertThat(actualConnectionTimeout).isEqualTo(10000);
}); });
} }