Polishing #1326

Rename option to handshakeTimeout. Add unit test.

Original pull request: #1338

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

@@ -56,6 +56,7 @@
  *
  * @author Mark Paluch
  * @author Amin Mohtashami
+ * @author Felipe Ruiz
  */
 public class SslConnectionBuilder extends ConnectionBuilder {
 
@@ -181,10 +182,9 @@ public void channelInactive(ChannelHandlerContext ctx) throws Exception {
             }
 
             SslHandler sslHandler = new SslHandler(sslEngine, startTls);
-            Duration sslHandshakeTimeout = sslOptions.getSslHandshakeTimeout();
-            if (sslHandshakeTimeout != null) {
-                sslHandler.setHandshakeTimeoutMillis(sslHandshakeTimeout.toMillis());
-            }
+            Duration sslHandshakeTimeout = sslOptions.getHandshakeTimeout();
+            sslHandler.setHandshakeTimeoutMillis(sslHandshakeTimeout.toMillis());
+
             channel.pipeline().addLast(sslHandler);
 
             if (channel.pipeline().get("channelActivator") == null) {

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

@@ -44,6 +44,7 @@
  *
  * @author Mark Paluch
  * @author Amin Mohtashami
+ * @author Felipe Ruiz
  * @since 4.3
  */
 public class SslOptions {
@@ -74,11 +75,12 @@ public class SslOptions {
 
     private final KeystoreAction trustmanager;
 
-    private final Duration sslHandshakeTimeout;
+    private final Duration handshakeTimeout;
 
     protected SslOptions(Builder builder) {
         this.keyStoreType = builder.keyStoreType;
         this.sslProvider = builder.sslProvider;
+        this.handshakeTimeout = builder.sslHandshakeTimeout;
         this.keystore = builder.keystore;
         this.keystorePassword = builder.keystorePassword;
         this.truststore = builder.truststore;
@@ -91,12 +93,12 @@ protected SslOptions(Builder builder) {
         this.sslParametersSupplier = builder.sslParametersSupplier;
         this.keymanager = builder.keymanager;
         this.trustmanager = builder.trustmanager;
-        this.sslHandshakeTimeout = builder.sslHandshakeTimeout;
     }
 
     protected SslOptions(SslOptions original) {
         this.keyStoreType = original.keyStoreType;
         this.sslProvider = original.getSslProvider();
+        this.handshakeTimeout = original.handshakeTimeout;
         this.keystore = original.keystore;
         this.keystorePassword = original.keystorePassword;
         this.truststore = original.getTruststore();
@@ -109,7 +111,6 @@ protected SslOptions(SslOptions original) {
         this.sslParametersSupplier = original.sslParametersSupplier;
         this.keymanager = original.keymanager;
         this.trustmanager = original.trustmanager;
-        this.sslHandshakeTimeout = original.sslHandshakeTimeout;
     }
 
     /**
@@ -171,7 +172,7 @@ public static class Builder {
 
         private KeystoreAction trustmanager = KeystoreAction.NO_OP;
 
-        private Duration sslHandshakeTimeout = null;
+        private Duration sslHandshakeTimeout = Duration.ofSeconds(10);
 
         private Builder() {
         }
@@ -225,6 +226,21 @@ private Builder sslProvider(SslProvider sslProvider) {
             return this;
         }
 
+        /**
+         * Sets a timeout for the SSL handshake.
+         *
+         * @param timeout {@link Duration}.
+         * @return {@code this}
+         * @since 5.3.2
+         */
+        public Builder handshakeTimeout(Duration timeout) {
+
+            LettuceAssert.notNull(timeout, "SSL Handshake Timeout must not be null");
+
+            this.sslHandshakeTimeout = timeout;
+            return this;
+        }
+
         /**
          * Sets the KeyStore type. Defaults to {@link KeyStore#getDefaultType()} if not set.
          *
@@ -585,18 +601,6 @@ public Builder sslParameters(Supplier<SSLParameters> sslParametersSupplier) {
             return this;
         }
 
-        /**
-         * Sets a timeout for the SSL handshake.
-         *
-         * @param timeout {@link Duration}.
-         * @return {@code this}
-         */
-        public Builder sslHandshakeTimeout(Duration timeout) {
-
-            this.sslHandshakeTimeout = timeout;
-            return this;
-        }
-
         /**
          * Create a new instance of {@link SslOptions}
          *
@@ -681,13 +685,12 @@ public SslOptions.Builder mutate() {
         builder.sslParametersSupplier = this.sslParametersSupplier;
         builder.keymanager = this.keymanager;
         builder.trustmanager = this.trustmanager;
-        builder.sslHandshakeTimeout = this.sslHandshakeTimeout;
+        builder.sslHandshakeTimeout = this.handshakeTimeout;
 
         return builder;
     }
 
     /**
-     *
      * @return the configured {@link SslProvider}.
      */
     @Deprecated
@@ -696,7 +699,6 @@ public SslProvider getSslProvider() {
     }
 
     /**
-     *
      * @return the keystore {@link URL}.
      * @deprecated since 5.3, {@link javax.net.ssl.KeyManager} is configured via {@link #createSslContextBuilder()}.
      */
@@ -706,23 +708,28 @@ public URL getKeystore() {
     }
 
     /**
-     *
      * @return the set of protocols.
      */
     public String[] getProtocols() {
         return protocols;
     }
 
     /**
-     *
      * @return the set of cipher suites.
      */
     public String[] getCipherSuites() {
         return cipherSuites;
     }
 
     /**
-     *
+     * @return the SSL handshake timeout
+     * @since 5.3.2
+     */
+    public Duration getHandshakeTimeout() {
+        return handshakeTimeout;
+    }
+
+    /**
      * @return the password for the keystore. May be empty.
      * @deprecated since 5.3, {@link javax.net.ssl.KeyManager} is configured via {@link #createSslContextBuilder()}.
      */
@@ -732,7 +739,6 @@ public char[] getKeystorePassword() {
     }
 
     /**
-     *
      * @return the truststore {@link URL}.
      * @deprecated since 5.3, {@link javax.net.ssl.TrustManager} is configured via {@link #createSslContextBuilder()}.
      */
@@ -751,13 +757,6 @@ public char[] getTruststorePassword() {
         return Arrays.copyOf(truststorePassword, truststorePassword.length);
     }
 
-    /**
-     * @return the SSL handshake timeout
-     */
-    public Duration getSslHandshakeTimeout() {
-        return sslHandshakeTimeout;
-    }
-
     private static KeyManagerFactory createKeyManagerFactory(InputStream inputStream, char[] storePassword, String keyStoreType)
             throws GeneralSecurityException, IOException {
 
@@ -827,7 +826,7 @@ public interface Resource {
          * @param url the URL to obtain the {@link InputStream} from.
          * @return a {@link Resource} that opens a connection to the URL and obtains the {@link InputStream} for it.
          */
-        public static Resource from(URL url) {
+        static Resource from(URL url) {
 
             LettuceAssert.notNull(url, "URL must not be null");
 
@@ -840,7 +839,7 @@ public static Resource from(URL url) {
          * @param file the File to obtain the {@link InputStream} from.
          * @return a {@link Resource} that obtains the {@link FileInputStream} for the given {@link File}.
          */
-        public static Resource from(File file) {
+        static Resource from(File file) {
 
             LettuceAssert.notNull(file, "File must not be null");
 

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

@@ -17,6 +17,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.time.Duration;
 import java.util.Collections;
 
 import javax.net.ssl.SSLParameters;
@@ -41,6 +42,14 @@ void shouldCreateEmptySslOptions() throws Exception {
         assertThat(options.createSslContextBuilder()).isNotNull();
     }
 
+    @Test
+    void shouldConfigureSslHandshakeTimeout() {
+
+        SslOptions options = SslOptions.builder().handshakeTimeout(Duration.ofSeconds(1)).build();
+
+        assertThat(options.getHandshakeTimeout()).isEqualTo(Duration.ofSeconds(1));
+    }
+
     @Test
     void shouldConfigureCipherSuiteAndProtocol() {