chore: remove deprecated default timeout from AbstractRedisClient (#3328) (#3344)

* chore: remove deprecated AbstractRedisClient#getDefaultTimeout
- remove usages from RedisClient and replaced with timeout value from redisUri
- fixed failing tests in RedisClientConnectIntegrationTests by asserting timeout to be equal to default timeout from RedisURI
- refactored DefaultRedisClient test fixture to not set timeout

Signed-off-by: The-East-Wind <[email protected]>

* chore: remove deprecated default timeout from AbstractRedisClient
- refactor AtLeastOnce and AtMostOnce IntegrationTests to remove setting timeout on the client level and instead set it in the URI level
- removed default timeout and usages which set it from AbstractRedisClient, RedisClient and RedisClusterClient

Signed-off-by: The-East-Wind <[email protected]>

* docs: update relevant documentation to remove usage of RedisClient#setDefaultTimeout

Signed-off-by: The-East-Wind <[email protected]>

---------

Signed-off-by: The-East-Wind <[email protected]>

Author: The-East-Wind

docs/user-guide/connecting-redis.md

@@ -195,8 +195,8 @@ the error message. `RedisException` is a `RuntimeException`.
 ### Examples
 
 ``` java
-RedisClient client = RedisClient.create(RedisURI.create("localhost", 6379));
-client.setDefaultTimeout(20, TimeUnit.SECONDS);
+RedisURI uri = new RedisURI("localhost", 6379, Duration.ofSeconds(20));
+RedisClient client = RedisClient.create(uri);
 
 // …
 

src/main/java/io/lettuce/core/AbstractRedisClient.java

@@ -109,8 +109,6 @@ public abstract class AbstractRedisClient implements AutoCloseable {
 
     private volatile ClientOptions clientOptions = ClientOptions.create();
 
-    private volatile Duration defaultTimeout = RedisURI.DEFAULT_TIMEOUT_DURATION;
-
     /**
      * Create a new instance with client resources.
      *
@@ -134,47 +132,6 @@ protected int getChannelCount() {
         return channels.size();
     }
 
-    /**
-     * Returns the default {@link Duration timeout} for commands.
-     *
-     * @return the default {@link Duration timeout} for commands.
-     * @deprecated since 6.2, use {@link RedisURI#getTimeout()} to control timeouts.
-     */
-    @Deprecated
-    public Duration getDefaultTimeout() {
-        return defaultTimeout;
-    }
-
-    /**
-     * Set the default timeout for connections created by this client. The timeout applies to connection attempts and
-     * non-blocking commands.
-     *
-     * @param timeout default connection timeout, must not be {@code null}.
-     * @since 5.0
-     * @deprecated since 6.2, use {@link RedisURI#getTimeout()} to control timeouts.
-     */
-    @Deprecated
-    public void setDefaultTimeout(Duration timeout) {
-
-        LettuceAssert.notNull(timeout, "Timeout duration must not be null");
-        LettuceAssert.isTrue(!timeout.isNegative(), "Timeout duration must be greater or equal to zero");
-
-        this.defaultTimeout = timeout;
-    }
-
-    /**
-     * Set the default timeout for connections created by this client. The timeout applies to connection attempts and
-     * non-blocking commands.
-     *
-     * @param timeout Default connection timeout.
-     * @param unit Unit of time for the timeout.
-     * @deprecated since 6.2, use {@link RedisURI#getTimeout()} to control timeouts.
-     */
-    @Deprecated
-    public void setDefaultTimeout(long timeout, TimeUnit unit) {
-        setDefaultTimeout(Duration.ofNanos(unit.toNanos(timeout)));
-    }
-
     /**
      * Returns the {@link ClientOptions} which are valid for that client. Connections inherit the current options at the moment
      * the connection is created. Changes to options will not affect existing connections.

src/main/java/io/lettuce/core/RedisClient.java

@@ -105,7 +105,6 @@ protected RedisClient(ClientResources clientResources, RedisURI redisURI) {
         assertNotNull(redisURI);
 
         this.redisURI = redisURI;
-        setDefaultTimeout(redisURI.getTimeout());
     }
 
     /**
@@ -217,7 +216,7 @@ public <K, V> StatefulRedisConnection<K, V> connect(RedisCodec<K, V> codec) {
 
         checkForRedisURI();
 
-        return getConnection(connectStandaloneAsync(codec, this.redisURI, getDefaultTimeout()));
+        return getConnection(connectStandaloneAsync(codec, this.redisURI, this.redisURI.getTimeout()));
     }
 
     /**
@@ -339,7 +338,7 @@ private <K, V, S> ConnectionFuture<S> connectStatefulAsync(StatefulRedisConnecti
      * @return A new stateful pub/sub connection
      */
     public StatefulRedisPubSubConnection<String, String> connectPubSub() {
-        return getConnection(connectPubSubAsync(newStringStringCodec(), this.redisURI, getDefaultTimeout()));
+        return getConnection(connectPubSubAsync(newStringStringCodec(), this.redisURI, this.redisURI.getTimeout()));
     }
 
     /**
@@ -366,7 +365,7 @@ public StatefulRedisPubSubConnection<String, String> connectPubSub(RedisURI redi
      */
     public <K, V> StatefulRedisPubSubConnection<K, V> connectPubSub(RedisCodec<K, V> codec) {
         checkForRedisURI();
-        return getConnection(connectPubSubAsync(codec, this.redisURI, getDefaultTimeout()));
+        return getConnection(connectPubSubAsync(codec, this.redisURI, this.redisURI.getTimeout()));
     }
 
     /**
@@ -453,7 +452,7 @@ public StatefulRedisSentinelConnection<String, String> connectSentinel() {
      */
     public <K, V> StatefulRedisSentinelConnection<K, V> connectSentinel(RedisCodec<K, V> codec) {
         checkForRedisURI();
-        return getConnection(connectSentinelAsync(codec, this.redisURI, getDefaultTimeout()));
+        return getConnection(connectSentinelAsync(codec, this.redisURI, this.redisURI.getTimeout()));
     }
 
     /**

src/main/java/io/lettuce/core/cluster/RedisClusterClient.java

@@ -198,7 +198,6 @@ protected RedisClusterClient(ClientResources clientResources, Iterable<RedisURI>
         this.initialUris = Collections.unmodifiableList(LettuceLists.newList(redisURIs));
         this.refresh = createTopologyRefresh();
 
-        setDefaultTimeout(getFirstUri().getTimeout());
         setOptions(ClusterClientOptions.create());
     }
 

src/main/javadoc/overview.html

@@ -27,10 +27,10 @@
   </p>
 
   <p>
-    All connections inherit a default timeout from their {@link io.lettuce.core.RedisClient}
+    All connections inherit a default timeout from their {@link io.lettuce.core.RedisURI}
     and will throw a {@link io.lettuce.core.RedisException} when non-blocking commands fail
     to return a result before the timeout expires. The timeout defaults to 60 seconds and
-    may be changed via {@link io.lettuce.core.RedisClient#setDefaultTimeout} or for
+    may be changed via {@link io.lettuce.core.RedisURI#setTimeout} or for
     each individual connection.
   </p>
 </body>

src/test/java/io/lettuce/core/RedisClientConnectIntegrationTests.java

@@ -33,7 +33,6 @@
 import javax.inject.Inject;
 
 import io.lettuce.core.protocol.ProtocolVersion;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
@@ -48,40 +47,34 @@
 /**
  * @author Mark Paluch
  * @author Jongyeol Choi
+ * @author Hari Mani
  */
 @ExtendWith(LettuceExtension.class)
 @Tag(INTEGRATION_TEST)
 class RedisClientConnectIntegrationTests extends TestSupport {
 
-    private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500);
-
     private final RedisClient client;
 
     @Inject
     RedisClientConnectIntegrationTests(RedisClient client) {
         this.client = client;
     }
 
-    @BeforeEach
-    void before() {
-        client.setDefaultTimeout(EXPECTED_TIMEOUT);
-    }
-
     /*
      * Standalone/Stateful
      */
     @Test
     void connectClientUri() {
 
         StatefulRedisConnection<String, String> connection = client.connect();
-        assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
+        assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
         connection.close();
     }
 
     @Test
     void connectCodecClientUri() {
         StatefulRedisConnection<String, String> connection = client.connect(UTF8);
-        assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
+        assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
         connection.close();
     }
 
@@ -133,29 +126,29 @@ void connectcodecSentinelMissingHostAndSocketUri() {
     @Test
     @Disabled("Non-deterministic behavior. Can cause a deadlock")
     void shutdownSyncInRedisFutureTest() {
-
-        RedisClient redisClient = RedisClient.create();
-        StatefulRedisConnection<String, String> connection = redisClient.connect(redis(host, port).build());
-
-        CompletableFuture<String> f = connection.async().get("key1").whenComplete((result, e) -> {
-            connection.close();
-            redisClient.shutdown(0, 0, SECONDS); // deadlock expected.
-        }).toCompletableFuture();
-
-        assertThatThrownBy(() -> TestFutures.awaitOrTimeout(f)).isInstanceOf(TimeoutException.class);
+        try (final RedisClient redisClient = RedisClient.create();
+                final StatefulRedisConnection<String, String> connection = redisClient.connect(redis(host, port).build())) {
+            CompletableFuture<String> f = connection.async().get("key1").whenComplete((result, e) -> {
+                connection.close();
+                redisClient.shutdown(0, 0, SECONDS); // deadlock expected.
+            }).toCompletableFuture();
+
+            assertThatThrownBy(() -> TestFutures.awaitOrTimeout(f)).isInstanceOf(TimeoutException.class);
+        }
     }
 
     @Test
     void shutdownAsyncInRedisFutureTest() {
 
-        RedisClient redisClient = RedisClient.create();
-        StatefulRedisConnection<String, String> connection = redisClient.connect(redis(host, port).build());
-        CompletableFuture<Void> f = connection.async().get("key1").thenCompose(result -> {
-            connection.close();
-            return redisClient.shutdownAsync(0, 0, SECONDS);
-        }).toCompletableFuture();
+        try (final RedisClient redisClient = RedisClient.create();
+                final StatefulRedisConnection<String, String> connection = redisClient.connect(redis(host, port).build())) {
+            CompletableFuture<Void> f = connection.async().get("key1").thenCompose(result -> {
+                connection.close();
+                return redisClient.shutdownAsync(0, 0, SECONDS);
+            }).toCompletableFuture();
 
-        TestFutures.awaitOrTimeout(f);
+            TestFutures.awaitOrTimeout(f);
+        }
     }
 
     /*
@@ -164,14 +157,14 @@ void shutdownAsyncInRedisFutureTest() {
     @Test
     void connectPubSubClientUri() {
         StatefulRedisPubSubConnection<String, String> connection = client.connectPubSub();
-        assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
+        assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
         connection.close();
     }
 
     @Test
     void connectPubSubCodecClientUri() {
         StatefulRedisPubSubConnection<String, String> connection = client.connectPubSub(UTF8);
-        assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
+        assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
         connection.close();
     }
 
@@ -243,14 +236,14 @@ void connectPubSubAsyncReauthNotSupportedWithRESP2() {
     @Test
     void connectSentinelClientUri() {
         StatefulRedisSentinelConnection<String, String> connection = client.connectSentinel();
-        assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
+        assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
         connection.close();
     }
 
     @Test
     void connectSentinelCodecClientUri() {
         StatefulRedisSentinelConnection<String, String> connection = client.connectSentinel(UTF8);
-        assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
+        assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
         connection.close();
     }
 

src/test/java/io/lettuce/core/reliability/AtLeastOnceIntegrationTests.java

@@ -10,13 +10,17 @@
 import java.util.concurrent.TimeUnit;
 import java.util.function.Predicate;
 
+import io.lettuce.core.RedisClient;
+import io.lettuce.core.RedisURI;
 import io.lettuce.core.TimeoutOptions;
 import io.lettuce.core.protocol.RedisCommand;
+import io.lettuce.test.resource.FastShutdown;
+import io.lettuce.test.settings.TestSettings;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 
-import io.lettuce.core.AbstractRedisClientTest;
 import io.lettuce.core.ClientOptions;
 import io.lettuce.core.RedisChannelWriter;
 import io.lettuce.core.RedisCommandTimeoutException;
@@ -42,26 +46,37 @@
 
 /**
  * @author Mark Paluch
+ * @author Hari Mani
  */
 @Tag(INTEGRATION_TEST)
-class AtLeastOnceIntegrationTests extends AbstractRedisClientTest {
+class AtLeastOnceIntegrationTests {
 
-    private String key = "key";
+    private static final String key = "key";
 
-    @BeforeEach
-    void before() {
-        client.setOptions(ClientOptions.builder().autoReconnect(true)
-                .timeoutOptions(TimeoutOptions.builder().timeoutCommands(false).build()).build());
+    private final RedisClient client;
 
+    public AtLeastOnceIntegrationTests() {
         // needs to be increased on slow systems...perhaps...
-        client.setDefaultTimeout(3, TimeUnit.SECONDS);
+        final RedisURI uri = RedisURI.Builder.redis(TestSettings.host(), TestSettings.port()).withTimeout(Duration.ofSeconds(3))
+                .build();
+        this.client = RedisClient.create(uri);
+        this.client.setOptions(ClientOptions.builder().autoReconnect(true)
+                .timeoutOptions(TimeoutOptions.builder().timeoutCommands(false).build()).build());
+    }
 
+    @BeforeEach
+    void before() {
         RedisCommands<String, String> connection = client.connect().sync();
         connection.flushall();
         connection.flushdb();
         connection.getStatefulConnection().close();
     }
 
+    @AfterEach
+    void tearDown() {
+        FastShutdown.shutdown(client);
+    }
+
     @Test
     void connectionIsConnectedAfterConnect() {
 
@@ -382,9 +397,6 @@ void retryAfterConnectionIsDisconnectedButFiltered() throws Exception {
         client.setOptions(ClientOptions.builder().autoReconnect(true).replayFilter(filter)
                 .timeoutOptions(TimeoutOptions.builder().timeoutCommands(false).build()).build());
 
-        // needs to be increased on slow systems...perhaps...
-        client.setDefaultTimeout(3, TimeUnit.SECONDS);
-
         StatefulRedisConnection<String, String> connection = client.connect();
         RedisCommands<String, String> verificationConnection = client.connect().sync();