From 404a66f4372e78ca093a857b8f71764eb02498ca Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 31 Jan 2018 12:59:26 +0100 Subject: [PATCH] DATAREDIS-765 - Enable pooling when configuring Jedis to use Redis Sentinel. We now enable pool usage when configuring JedisConnectionFactory with RedisSentinelConfiguration to prevent accidental connections using the non-pooled standalone configuration. Jedis can operate with Sentinel only with pooling. We also reject calls to disable pooling when JedisConnectionFactory is configured to use Sentinel. Original Pull request: #307 --- .../connection/jedis/JedisConnectionFactory.java | 11 +++++++++-- .../jedis/JedisConnectionFactoryTests.java | 13 +++++++++++-- .../jedis/JedisConnectionFactoryUnitTests.java | 8 ++++++++ 3 files changed, 28 insertions(+), 4 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 11522e2ff..c80d77a8e 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 @@ -1,5 +1,5 @@ /* - * Copyright 2011-2017 the original author or authors. + * Copyright 2011-2018 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. @@ -489,15 +489,22 @@ public class JedisConnectionFactory implements InitializingBean, DisposableBean, * @return the use of connection pooling. */ public boolean getUsePool() { - return usePool; + return isRedisSentinelAware() ? true : usePool; } /** * Turns on or off the use of connection pooling. * * @param usePool the usePool to set. + * @throws IllegalStateException if configured to use sentinel and {@code usePool} is {@literal false} as Jedis + * requires pooling for Redis sentinel use. */ public void setUsePool(boolean usePool) { + + if (isRedisSentinelAware() && !usePool) { + throw new IllegalStateException("Jedis requires pooling for Redis Sentinel use!"); + } + this.usePool = usePool; } diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryTests.java index ece335c32..4f390beb0 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2017 the original author or authors. + * Copyright 2014-2018 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. @@ -15,7 +15,8 @@ */ package org.springframework.data.redis.connection.jedis; -import static org.hamcrest.core.IsEqual.*; +import static org.hamcrest.Matchers.*; +import static org.hamcrest.core.IsEqual.equalTo; import static org.junit.Assert.*; import org.junit.After; @@ -26,8 +27,11 @@ import org.springframework.data.redis.connection.RedisSentinelConfiguration; import org.springframework.data.redis.test.util.RedisSentinelRule; /** + * Sentinel integration tests for {@link JedisConnectionFactory}. + * * @author Christoph Strobl * @author Fu Jian + * @author Mark Paluch */ public class JedisConnectionFactoryTests { @@ -49,6 +53,11 @@ public class JedisConnectionFactoryTests { factory.destroy(); } + @Test // DATAREDIS-765 + public void shouldInitializeWithSentinelConfiguration() { + assertThat(factory.getUsePool(), is(true)); + } + @Test // DATAREDIS-324 public void shouldSendCommandCorrectlyViaConnectionFactoryUsingSentinel() { assertThat(factory.getConnection().ping(), equalTo("PONG")); 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 94cc20394..a50c12b7a 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 @@ -62,6 +62,14 @@ public class JedisConnectionFactoryUnitTests { verify(connectionFactory, never()).createRedisSentinelPool(Matchers.any(RedisSentinelConfiguration.class)); } + @Test(expected = IllegalStateException.class) // DATAREDIS-765 + public void shouldRejectPoolDisablingWhenSentinelConfigPresent() { + + connectionFactory = new JedisConnectionFactory(new RedisSentinelConfiguration()); + + connectionFactory.setUsePool(false); + } + @Test // DATAREDIS-315 public void shouldInitConnectionCorrectlyWhenClusterConfigPresent() {