Address PR feedback: reload-interval to use Optional internally and null in config, rather than using sentinel Duration.ZERO

Author: aratno

core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java

@@ -33,6 +33,7 @@
 import java.security.SecureRandom;
 import java.time.Duration;
 import java.util.List;
+import java.util.Optional;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
@@ -153,14 +154,11 @@ protected SSLContext buildContext(DriverExecutionProfile config) throws Exceptio
   private ReloadingKeyManagerFactory buildReloadingKeyManagerFactory(DriverExecutionProfile config)
       throws Exception {
     Path keystorePath = Paths.get(config.getString(DefaultDriverOption.SSL_KEYSTORE_PATH));
-    String password =
-        config.isDefined(DefaultDriverOption.SSL_KEYSTORE_PASSWORD)
-            ? config.getString(DefaultDriverOption.SSL_KEYSTORE_PASSWORD)
-            : null;
-    Duration reloadInterval =
-        config.isDefined(DefaultDriverOption.SSL_KEYSTORE_RELOAD_INTERVAL)
-            ? config.getDuration(DefaultDriverOption.SSL_KEYSTORE_RELOAD_INTERVAL)
-            : Duration.ZERO;
+    String password = config.getString(DefaultDriverOption.SSL_KEYSTORE_PASSWORD, null);
+    Optional<Duration> reloadInterval =
+        Optional.ofNullable(
+            config.getDuration(DefaultDriverOption.SSL_KEYSTORE_RELOAD_INTERVAL, null));
+
     return ReloadingKeyManagerFactory.create(keystorePath, password, reloadInterval);
   }
 

core/src/main/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactory.java

@@ -36,6 +36,7 @@
 import java.security.cert.X509Certificate;
 import java.time.Duration;
 import java.util.Arrays;
+import java.util.Optional;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -68,12 +69,12 @@ public class ReloadingKeyManagerFactory extends KeyManagerFactory implements Aut
    *
    * @param keystorePath the keystore file to reload
    * @param keystorePassword the keystore password
-   * @param reloadInterval the duration between reload attempts. Set to {@link
-   *     java.time.Duration#ZERO} to disable scheduled reloading.
+   * @param reloadInterval the duration between reload attempts. Set to {@link Optional#empty()} to
+   *     disable scheduled reloading.
    * @return
    */
-  public static ReloadingKeyManagerFactory create(
-      Path keystorePath, String keystorePassword, Duration reloadInterval)
+  static ReloadingKeyManagerFactory create(
+      Path keystorePath, String keystorePassword, Optional<Duration> reloadInterval)
       throws UnrecoverableKeyException, KeyStoreException, NoSuchAlgorithmException,
           CertificateException, IOException {
     KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
@@ -103,14 +104,24 @@ private ReloadingKeyManagerFactory(Spi spi, Provider provider, String algorithm)
     this.spi = spi;
   }
 
-  private void start(Path keystorePath, String keystorePassword, Duration reloadInterval) {
+  private void start(
+      Path keystorePath, String keystorePassword, Optional<Duration> reloadInterval) {
     this.keystorePath = keystorePath;
     this.keystorePassword = keystorePassword;
 
     // Ensure that reload is called once synchronously, to make sure the file exists etc.
     reload();
 
-    if (!reloadInterval.isZero()) {
+    if (!reloadInterval.isPresent() || reloadInterval.get().isZero()) {
+      final String msg =
+          "KeyStore reloading is disabled. If your Cassandra cluster requires client certificates, "
+              + "client application restarts are infrequent, and client certificates have short lifetimes, then your client "
+              + "may fail to re-establish connections to Cassandra hosts. To enable KeyStore reloading, see "
+              + "`advanced.ssl-engine-factory.keystore-reload-interval` in reference.conf.";
+      logger.info(msg);
+    } else {
+      logger.info("KeyStore reloading is enabled with interval {}", reloadInterval.get());
+
       this.executor =
           Executors.newScheduledThreadPool(
               1,
@@ -122,8 +133,8 @@ private void start(Path keystorePath, String keystorePassword, Duration reloadIn
               });
       this.executor.scheduleWithFixedDelay(
           this::reload,
-          reloadInterval.toMillis(),
-          reloadInterval.toMillis(),
+          reloadInterval.get().toMillis(),
+          reloadInterval.get().toMillis(),
           TimeUnit.MILLISECONDS);
     }
   }
@@ -135,7 +146,7 @@ void reload() {
     } catch (Exception e) {
       String msg =
           "Failed to reload KeyStore. If this continues to happen, your client may use stale identity"
-              + "certificates and fail to re-establish connections to Cassandra hosts.";
+              + " certificates and fail to re-establish connections to Cassandra hosts.";
       logger.warn(msg, e);
     }
   }

core/src/test/java/com/datastax/oss/driver/internal/core/ssl/ReloadingKeyManagerFactoryTest.java

@@ -34,7 +34,6 @@
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.security.cert.X509Certificate;
-import java.time.Duration;
 import java.util.Optional;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -86,7 +85,6 @@ public class ReloadingKeyManagerFactoryTest {
 
   static final Path CLIENT_TRUSTSTORE_PATH = CERT_BASE.resolve("client.truststore");
   static final String CERTSTORE_PASSWORD = "changeit";
-  static final Duration NO_SCHEDULED_RELOAD = Duration.ofMillis(0);
 
   private static TrustManagerFactory buildTrustManagerFactory() {
     TrustManagerFactory tmf;
@@ -186,7 +184,7 @@ public void client_certificates_should_reload() throws Exception {
 
     final ReloadingKeyManagerFactory kmf =
         ReloadingKeyManagerFactory.create(
-            TMP_CLIENT_KEYSTORE_PATH, CERTSTORE_PASSWORD, NO_SCHEDULED_RELOAD);
+            TMP_CLIENT_KEYSTORE_PATH, CERTSTORE_PASSWORD, Optional.empty());
     // Need a tmf that tells the server to send its certs
     final TrustManagerFactory tmf = buildTrustManagerFactory();