ClusterTopologyRefreshOptions builder should fail when `enableAdapt…

…iveRefreshTrigger` is called without options #2575 Improve usage by throwing an exception to early fail on improper API usage.
redis · Dec 27, 2023 · cb36ac6 · cb36ac6
1 parent 6ef7817
commit cb36ac6
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 9 deletions.
diff --git a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java
@@ -165,6 +165,7 @@ public Builder enableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) {
 
             LettuceAssert.notNull(refreshTrigger, "RefreshTriggers must not be null");
             LettuceAssert.noNullElements(refreshTrigger, "RefreshTriggers must not contain null elements");
+            LettuceAssert.notEmpty(refreshTrigger, "RefreshTriggers must not contain at least one element");
 
             adaptiveRefreshTriggers.addAll(Arrays.asList(refreshTrigger));
             return this;

diff --git a/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptionsUnitTests.java b/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptionsUnitTests.java
@@ -15,7 +15,7 @@
  */
 package io.lettuce.core.cluster;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.*;
 
 import java.time.Duration;
 import java.util.concurrent.TimeUnit;
@@ -25,6 +25,8 @@
 import io.lettuce.core.cluster.ClusterTopologyRefreshOptions.RefreshTrigger;
 
 /**
+ * Unit tests for {@link ClusterTopologyRefreshOptions}.
+ *
  * @author Mark Paluch
  */
 class ClusterTopologyRefreshOptionsUnitTests {
@@ -80,16 +82,16 @@ void testDefault() {
 
         assertThat(options.getRefreshPeriod()).isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_REFRESH_PERIOD_DURATION);
         assertThat(options.isCloseStaleConnections()).isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_CLOSE_STALE_CONNECTIONS);
-        assertThat(options.isPeriodicRefreshEnabled())
-                .isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_PERIODIC_REFRESH_ENABLED).isFalse();
+        assertThat(options.isPeriodicRefreshEnabled()).isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_PERIODIC_REFRESH_ENABLED)
+                .isFalse();
         assertThat(options.useDynamicRefreshSources()).isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_DYNAMIC_REFRESH_SOURCES)
                 .isTrue();
-        assertThat(options.getAdaptiveRefreshTimeout()).isEqualTo(
-                ClusterTopologyRefreshOptions.DEFAULT_ADAPTIVE_REFRESH_TIMEOUT_DURATION);
-        assertThat(options.getAdaptiveRefreshTriggers()).isEqualTo(
-                ClusterTopologyRefreshOptions.DEFAULT_ADAPTIVE_REFRESH_TRIGGERS);
-        assertThat(options.getRefreshTriggersReconnectAttempts()).isEqualTo(
-                ClusterTopologyRefreshOptions.DEFAULT_REFRESH_TRIGGERS_RECONNECT_ATTEMPTS);
+        assertThat(options.getAdaptiveRefreshTimeout())
+                .isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_ADAPTIVE_REFRESH_TIMEOUT_DURATION);
+        assertThat(options.getAdaptiveRefreshTriggers())
+                .isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_ADAPTIVE_REFRESH_TRIGGERS);
+        assertThat(options.getRefreshTriggersReconnectAttempts())
+                .isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_REFRESH_TRIGGERS_RECONNECT_ATTEMPTS);
     }
 
     @Test
@@ -102,4 +104,13 @@ void testEnabled() {
         assertThat(options.getAdaptiveRefreshTriggers()).contains(RefreshTrigger.ASK_REDIRECT, RefreshTrigger.MOVED_REDIRECT,
                 RefreshTrigger.PERSISTENT_RECONNECTS);
     }
+
+    @Test
+    void emptyTriggersShouldFail() {
+
+        ClusterTopologyRefreshOptions.Builder builder = ClusterTopologyRefreshOptions.builder();
+
+        assertThatIllegalArgumentException().isThrownBy(builder::enableAdaptiveRefreshTrigger);
+    }
+
 }