From 96757295d0e4c1657d581c6f46ffe4cb9dc4e487 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Wed, 2 Jun 2021 12:15:38 +0200 Subject: [PATCH] Polish "Enable Redis connection pool if commons-pool2 is available" See gh-26326 --- .../redis/JedisConnectionConfiguration.java | 3 +- .../redis/LettuceConnectionConfiguration.java | 3 +- .../redis/RedisConnectionConfiguration.java | 12 ++++- .../data/redis/RedisProperties.java | 18 +++---- ...rationLettuceWithoutCommonsPool2Tests.java | 51 +++++++++++++++++++ .../redis/RedisAutoConfigurationTests.java | 32 ++++++------ .../src/docs/asciidoc/features/nosql.adoc | 2 +- .../src/main/resources/application.properties | 1 - 8 files changed, 89 insertions(+), 33 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationLettuceWithoutCommonsPool2Tests.java delete mode 100644 spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-redis/src/main/resources/application.properties diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/JedisConnectionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/JedisConnectionConfiguration.java index 9da3b1d953d7..62dd4685fcd9 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/JedisConnectionConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/JedisConnectionConfiguration.java @@ -41,7 +41,6 @@ * * @author Mark Paluch * @author Stephane Nicoll - * @author Weix Sun */ @Configuration(proxyBeanMethods = false) @ConditionalOnClass({ GenericObjectPool.class, JedisConnection.class, Jedis.class }) @@ -77,7 +76,7 @@ private JedisClientConfiguration getJedisClientConfiguration( ObjectProvider builderCustomizers) { JedisClientConfigurationBuilder builder = applyProperties(JedisClientConfiguration.builder()); RedisProperties.Pool pool = getProperties().getJedis().getPool(); - if (pool.isEnabled()) { + if (isPoolEnabled(pool)) { applyPooling(pool, builder); } if (StringUtils.hasText(getProperties().getUrl())) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java index 4a87dccafca9..2496e687d70b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java @@ -51,7 +51,6 @@ * * @author Mark Paluch * @author Andy Wilkinson - * @author Weix Sun */ @Configuration(proxyBeanMethods = false) @ConditionalOnClass(RedisClient.class) @@ -105,7 +104,7 @@ private LettuceClientConfiguration getLettuceClientConfiguration( } private LettuceClientConfigurationBuilder createBuilder(Pool pool) { - if (pool.isEnabled()) { + if (isPoolEnabled(pool)) { return new PoolBuilderFactory().createBuilder(pool); } return LettuceClientConfiguration.builder(); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java index 15bb5b24154f..4940843b8ac8 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisConnectionConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 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. @@ -22,12 +22,14 @@ import java.util.List; import org.springframework.beans.factory.ObjectProvider; +import org.springframework.boot.autoconfigure.data.redis.RedisProperties.Pool; import org.springframework.data.redis.connection.RedisClusterConfiguration; import org.springframework.data.redis.connection.RedisNode; import org.springframework.data.redis.connection.RedisPassword; import org.springframework.data.redis.connection.RedisSentinelConfiguration; import org.springframework.data.redis.connection.RedisStandaloneConfiguration; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; /** @@ -40,6 +42,9 @@ */ abstract class RedisConnectionConfiguration { + private static final boolean COMMONS_POOL2_AVAILABLE = ClassUtils.isPresent("org.apache.commons.pool2.ObjectPool", + RedisConnectionConfiguration.class.getClassLoader()); + private final RedisProperties properties; private final RedisSentinelConfiguration sentinelConfiguration; @@ -122,6 +127,11 @@ protected final RedisProperties getProperties() { return this.properties; } + protected boolean isPoolEnabled(Pool pool) { + Boolean enabled = pool.getEnabled(); + return (enabled != null) ? enabled : COMMONS_POOL2_AVAILABLE; + } + private List createSentinels(RedisProperties.Sentinel sentinel) { List nodes = new ArrayList<>(); for (String node : sentinel.getNodes()) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java index a9e46e7fe069..df525d60de1d 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java @@ -233,7 +233,11 @@ public enum ClientType { */ public static class Pool { - private boolean enabled; + /** + * Whether to enable the pool. Enabled automatically if "commons-pool2" is + * available. + */ + private Boolean enabled; /** * Maximum number of "idle" connections in the pool. Use a negative value to @@ -267,11 +271,11 @@ public static class Pool { */ private Duration timeBetweenEvictionRuns; - public boolean isEnabled() { + public Boolean getEnabled() { return this.enabled; } - public void setEnabled(boolean enabled) { + public void setEnabled(Boolean enabled) { this.enabled = enabled; } @@ -406,13 +410,7 @@ public static class Jedis { /** * Jedis pool configuration. */ - private final Pool pool; - - public Jedis() { - Pool pool = new Pool(); - pool.setEnabled(true); - this.pool = pool; - } + private final Pool pool = new Pool(); public Pool getPool() { return this.pool; diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationLettuceWithoutCommonsPool2Tests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationLettuceWithoutCommonsPool2Tests.java new file mode 100644 index 000000000000..2e8c0d64812d --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationLettuceWithoutCommonsPool2Tests.java @@ -0,0 +1,51 @@ +/* + * Copyright 2012-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.autoconfigure.data.redis; + +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.boot.testsupport.classpath.ClassPathExclusions; +import org.springframework.data.redis.connection.lettuce.LettuceConnectionFactory; +import org.springframework.data.redis.connection.lettuce.LettucePoolingClientConfiguration; +import org.springframework.test.util.ReflectionTestUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link RedisAutoConfiguration} when commons-pool2 is not on the classpath. + * + * @author Stephane Nicoll + */ +@ClassPathExclusions("commons-pool2-*.jar") +public class RedisAutoConfigurationLettuceWithoutCommonsPool2Tests { + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(RedisAutoConfiguration.class)); + + @Test + void poolWithoutCommonsPool2IsDisabledByDefault() { + this.contextRunner.withPropertyValues("spring.redis.host:foo").run((context) -> { + LettuceConnectionFactory cf = context.getBean(LettuceConnectionFactory.class); + assertThat(cf.getHostName()).isEqualTo("foo"); + assertThat(ReflectionTestUtils.getField(cf, "clientConfiguration")) + .isNotInstanceOf(LettucePoolingClientConfiguration.class); + }); + } + +} diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java index fc858f6883bd..7579e15a887a 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java @@ -31,6 +31,7 @@ import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.autoconfigure.data.redis.RedisProperties.Pool; import org.springframework.boot.test.context.assertj.AssertableApplicationContext; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.boot.test.context.runner.ContextConsumer; @@ -156,25 +157,24 @@ void testPasswordInUrlStartsWithColon() { } @Test - void testRedisConfigurationWithPoolUsingDefaultValue() { - this.contextRunner.withPropertyValues("spring.redis.host:foo", "spring.redis.lettuce.pool.enabled:true") - .run((context) -> { - LettuceConnectionFactory cf = context.getBean(LettuceConnectionFactory.class); - assertThat(cf.getHostName()).isEqualTo("foo"); - GenericObjectPoolConfig poolConfig = getPoolingClientConfiguration(cf).getPoolConfig(); - assertThat(poolConfig.getMinIdle()).isEqualTo(0); - assertThat(poolConfig.getMaxIdle()).isEqualTo(8); - assertThat(poolConfig.getMaxTotal()).isEqualTo(8); - assertThat(poolConfig.getMaxWaitMillis()).isEqualTo(-1); - }); + void testRedisConfigurationUsePoolByDefault() { + Pool defaultPool = new RedisProperties().getLettuce().getPool(); + this.contextRunner.withPropertyValues("spring.redis.host:foo").run((context) -> { + LettuceConnectionFactory cf = context.getBean(LettuceConnectionFactory.class); + assertThat(cf.getHostName()).isEqualTo("foo"); + GenericObjectPoolConfig poolConfig = getPoolingClientConfiguration(cf).getPoolConfig(); + assertThat(poolConfig.getMinIdle()).isEqualTo(defaultPool.getMinIdle()); + assertThat(poolConfig.getMaxIdle()).isEqualTo(defaultPool.getMaxIdle()); + assertThat(poolConfig.getMaxTotal()).isEqualTo(defaultPool.getMaxActive()); + assertThat(poolConfig.getMaxWaitMillis()).isEqualTo(defaultPool.getMaxWait().toMillis()); + }); } @Test - void testRedisConfigurationWithPoolUsingCustomValue() { - this.contextRunner.withPropertyValues("spring.redis.host:foo", "spring.redis.lettuce.pool.enabled:true", - "spring.redis.lettuce.pool.min-idle:1", "spring.redis.lettuce.pool.max-idle:4", - "spring.redis.lettuce.pool.max-active:16", "spring.redis.lettuce.pool.max-wait:2000", - "spring.redis.lettuce.pool.time-between-eviction-runs:30000", + void testRedisConfigurationWithCustomPoolSettings() { + this.contextRunner.withPropertyValues("spring.redis.host:foo", "spring.redis.lettuce.pool.min-idle:1", + "spring.redis.lettuce.pool.max-idle:4", "spring.redis.lettuce.pool.max-active:16", + "spring.redis.lettuce.pool.max-wait:2000", "spring.redis.lettuce.pool.time-between-eviction-runs:30000", "spring.redis.lettuce.shutdown-timeout:1000").run((context) -> { LettuceConnectionFactory cf = context.getBean(LettuceConnectionFactory.class); assertThat(cf.getHostName()).isEqualTo("foo"); diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/features/nosql.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/features/nosql.adoc index ff1da00025d0..7d3fe3b45c10 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/features/nosql.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/features/nosql.adoc @@ -46,7 +46,7 @@ If you use Jedis, `JedisClientConfigurationBuilderCustomizer` is also available. If you add your own `@Bean` of any of the auto-configured types, it replaces the default (except in the case of `RedisTemplate`, when the exclusion is based on the bean name, `redisTemplate`, not its type). -A pooled connection factory is auto-configured if `commons-pool2` is on the classpath and at least one `Pool` option of {spring-boot-autoconfigure-module-code}/data/redis/RedisProperties.java[`RedisProperties`] is set. +By default, a pooled connection factory is auto-configured if `commons-pool2` is on the classpath. diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-redis/src/main/resources/application.properties b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-redis/src/main/resources/application.properties deleted file mode 100644 index ce8e5c6d7824..000000000000 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-redis/src/main/resources/application.properties +++ /dev/null @@ -1 +0,0 @@ -spring.redis.lettuce.pool.enabled=false