From 2bcdd36a368d68e5ecffc204294c890556017079 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 10 Jan 2025 14:01:05 +0100 Subject: [PATCH] Polishing. Prepare pool for Jedis as well. Introduce generics for Add ticket references to tests. Fix generics for GenericObjectPoolConfig using Lettuce. See #3072 --- .../jedis/JedisConnectionFactory.java | 6 ++++ ...aultLettucePoolingClientConfiguration.java | 7 +++-- .../LettucePoolingClientConfiguration.java | 8 +++-- .../LettucePoolingConnectionProvider.java | 3 +- .../JedisConnectionFactoryUnitTests.java | 30 +++++++++++++++++-- ...ucePoolingConnectionProviderUnitTests.java | 5 ++-- 6 files changed, 46 insertions(+), 13 deletions(-) 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 419b82550..01cb4bade 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 @@ -730,6 +730,12 @@ public class JedisConnectionFactory if (getUsePool() && !isRedisClusterAware()) { this.pool = createPool(); + + try { + this.pool.preparePool(); + } catch (Exception ex) { + throw new PoolException("Could not prepare the pool", ex); + } } if (isRedisClusterAware()) { diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/DefaultLettucePoolingClientConfiguration.java b/src/main/java/org/springframework/data/redis/connection/lettuce/DefaultLettucePoolingClientConfiguration.java index 40d02becd..7d8cfa4d7 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/DefaultLettucePoolingClientConfiguration.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/DefaultLettucePoolingClientConfiguration.java @@ -18,6 +18,7 @@ package org.springframework.data.redis.connection.lettuce; import io.lettuce.core.ClientOptions; import io.lettuce.core.ReadFrom; import io.lettuce.core.SslVerifyMode; +import io.lettuce.core.api.StatefulConnection; import io.lettuce.core.resource.ClientResources; import java.time.Duration; @@ -37,10 +38,10 @@ import org.apache.commons.pool2.impl.GenericObjectPoolConfig; class DefaultLettucePoolingClientConfiguration implements LettucePoolingClientConfiguration { private final LettuceClientConfiguration clientConfiguration; - private final GenericObjectPoolConfig poolConfig; + private final GenericObjectPoolConfig> poolConfig; DefaultLettucePoolingClientConfiguration(LettuceClientConfiguration clientConfiguration, - GenericObjectPoolConfig poolConfig) { + GenericObjectPoolConfig> poolConfig) { this.clientConfiguration = clientConfiguration; this.poolConfig = poolConfig; @@ -108,7 +109,7 @@ class DefaultLettucePoolingClientConfiguration implements LettucePoolingClientCo } @Override - public GenericObjectPoolConfig getPoolConfig() { + public GenericObjectPoolConfig> getPoolConfig() { return poolConfig; } } diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfiguration.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfiguration.java index bb72d32cc..dd06a6d3b 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfiguration.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfiguration.java @@ -17,6 +17,7 @@ package org.springframework.data.redis.connection.lettuce; import io.lettuce.core.ClientOptions; import io.lettuce.core.ReadFrom; +import io.lettuce.core.api.StatefulConnection; import io.lettuce.core.resource.ClientResources; import java.time.Duration; @@ -39,7 +40,7 @@ public interface LettucePoolingClientConfiguration extends LettuceClientConfigur /** * @return the {@link GenericObjectPoolConfig}. Never {@literal null}. */ - GenericObjectPoolConfig getPoolConfig(); + GenericObjectPoolConfig> getPoolConfig(); /** * Creates a new {@link LettucePoolingClientConfigurationBuilder} to build {@link LettucePoolingClientConfiguration} @@ -91,7 +92,7 @@ public interface LettucePoolingClientConfiguration extends LettuceClientConfigur */ class LettucePoolingClientConfigurationBuilder extends LettuceClientConfigurationBuilder { - GenericObjectPoolConfig poolConfig = new GenericObjectPoolConfig(); + GenericObjectPoolConfig> poolConfig = new GenericObjectPoolConfig<>(); LettucePoolingClientConfigurationBuilder() { super(); @@ -163,7 +164,8 @@ public interface LettucePoolingClientConfiguration extends LettuceClientConfigur * * @param poolConfig must not be {@literal null}. */ - public LettucePoolingClientConfigurationBuilder poolConfig(GenericObjectPoolConfig poolConfig) { + public LettucePoolingClientConfigurationBuilder poolConfig( + GenericObjectPoolConfig> poolConfig) { Assert.notNull(poolConfig, "PoolConfig must not be null"); diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingConnectionProvider.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingConnectionProvider.java index 57d3ad084..086c9ce76 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingConnectionProvider.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingConnectionProvider.java @@ -64,7 +64,7 @@ class LettucePoolingConnectionProvider implements LettuceConnectionProvider, Red private static final Log log = LogFactory.getLog(LettucePoolingConnectionProvider.class); private final LettuceConnectionProvider connectionProvider; - private final GenericObjectPoolConfig poolConfig; + private final GenericObjectPoolConfig> poolConfig; private final Map, GenericObjectPool>> poolRef = new ConcurrentHashMap<>( 32); @@ -97,7 +97,6 @@ class LettucePoolingConnectionProvider implements LettuceConnectionProvider, Red try { newPool.preparePool(); - } catch (Exception ex) { throw new PoolException("Could not prepare the pool", ex); } 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 9b1496c71..d766502a8 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 @@ -19,10 +19,12 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; import redis.clients.jedis.DefaultJedisClientConfig; +import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisClientConfig; import redis.clients.jedis.JedisCluster; import redis.clients.jedis.JedisPoolConfig; import redis.clients.jedis.RedisProtocol; +import redis.clients.jedis.util.Pool; import java.io.IOException; import java.security.NoSuchAlgorithmException; @@ -265,6 +267,28 @@ class JedisConnectionFactoryUnitTests { assertThat(connectionFactory.getPoolConfig()).isSameAs(poolConfig); } + @Test // GH-3072 + void shouldInitializePool() throws Exception { + + JedisPoolConfig poolConfig = new JedisPoolConfig(); + Pool poolMock = mock(Pool.class); + + JedisClientConfiguration configuration = JedisClientConfiguration.builder() // + .usePooling().poolConfig(poolConfig) // + .build(); + + connectionFactory = new JedisConnectionFactory(new RedisStandaloneConfiguration(), configuration) { + @Override + protected Pool createRedisPool() { + return poolMock; + } + }; + + connectionFactory.afterPropertiesSet(); + + verify(poolMock).preparePool(); + } + @Test // DATAREDIS-574 void shouldReturnStandaloneConfiguration() { @@ -382,12 +406,12 @@ class JedisConnectionFactoryUnitTests { private JedisConnectionFactory initSpyedConnectionFactory(RedisSentinelConfiguration sentinelConfiguration, @Nullable JedisPoolConfig poolConfig) { + Pool poolMock = mock(Pool.class); // we have to use a spy here as jedis would start connecting to redis sentinels when the pool is created. JedisConnectionFactory connectionFactorySpy = spy(new JedisConnectionFactory(sentinelConfiguration, poolConfig)); - doReturn(null).when(connectionFactorySpy).createRedisSentinelPool(any(RedisSentinelConfiguration.class)); - - doReturn(null).when(connectionFactorySpy).createRedisPool(); + doReturn(poolMock).when(connectionFactorySpy).createRedisSentinelPool(any(RedisSentinelConfiguration.class)); + doReturn(poolMock).when(connectionFactorySpy).createRedisPool(); return connectionFactorySpy; } diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettucePoolingConnectionProviderUnitTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettucePoolingConnectionProviderUnitTests.java index 1722319ff..4c4cfe735 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettucePoolingConnectionProviderUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettucePoolingConnectionProviderUnitTests.java @@ -17,6 +17,7 @@ package org.springframework.data.redis.connection.lettuce; import static org.mockito.Mockito.*; +import io.lettuce.core.api.StatefulConnection; import io.lettuce.core.api.StatefulRedisConnection; import io.lettuce.core.api.async.RedisAsyncCommands; @@ -74,10 +75,10 @@ class LettucePoolingConnectionProviderUnitTests { verify(commandsMock).discard(); } - @Test + @Test // GH-3072 void shouldPrepareThePool() { - GenericObjectPoolConfig poolConfig = new GenericObjectPoolConfig(); + GenericObjectPoolConfig> poolConfig = new GenericObjectPoolConfig<>(); poolConfig.setMinIdle(5); poolConfig.setMaxIdle(8); poolConfig.setMaxTotal(10);