ZOOKEEPER-4966: Decouple ZKConfig from QuorumPeerConfig.ConfigException

Reviewers: anmolnar, cnauroth
Author: kezhuw
Closes #2306 from kezhuw/ZOOKEEPER-4966-decouple-ZKConfig-from-QuorumPeerConfig

Author: kezhuw

zookeeper-server/src/main/java/org/apache/zookeeper/client/ZKClientConfig.java

@@ -23,7 +23,6 @@
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.common.ZKConfig;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
 
 /**
  * Handles client specific properties
@@ -78,22 +77,22 @@ public ZKClientConfig() {
     /**
      * <p><b>Use {@link ZKClientConfig#ZKClientConfig(Path configPath)} instead.</b>
      *
-     * <p><b>The signature of this method will be changed to throw {@link ConfigException}
-     * instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
+     * <p><b>The signature of this method has been changed to throw {@link ConfigException}
+     * instead of {@code QuorumPeerConfig.ConfigException}.</b>
      */
     @Deprecated
-    public ZKClientConfig(File configFile) throws QuorumPeerConfig.ConfigException {
+    public ZKClientConfig(File configFile) throws ConfigException {
         super(configFile);
     }
 
     /**
      * <p><b>Use {@link ZKClientConfig#ZKClientConfig(Path configPath)} instead.</b>
      *
-     * <p><b>The signature of this method will be changed to throw {@link ConfigException}
-     * instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
+     * <p><b>The signature of this method has been changed to throw {@link ConfigException}
+     * instead of {@code QuorumPeerConfig.ConfigException}.</b>
      */
     @Deprecated
-    public ZKClientConfig(String configPath) throws QuorumPeerConfig.ConfigException {
+    public ZKClientConfig(String configPath) throws ConfigException {
         super(configPath);
     }
 

zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java

@@ -21,13 +21,14 @@
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.lang.reflect.Constructor;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Properties;
 import org.apache.zookeeper.Environment;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
 import org.apache.zookeeper.server.util.VerifyingFileFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -65,35 +66,33 @@ public ZKConfig() {
     /**
      * <p><b>Use {@link ZKConfig#ZKConfig(Path configPath)} instead.</b>
      *
-     * <p><b>The signature of this method will be changed to throw {@link ConfigException}
-     * instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
+     * <p><b>The signature of this method has been changed to throw {@link ConfigException}
+     * instead of {@code QuorumPeerConfig.ConfigException}.</b>
      *
      * @param configPath
      *            Configuration file path
      * @throws ConfigException
      *             if failed to load configuration properties
      */
     @Deprecated
-    public ZKConfig(String configPath) throws QuorumPeerConfig.ConfigException {
-        this(new File(configPath));
+    public ZKConfig(String configPath) throws ConfigException {
+        this(Paths.get(configPath));
     }
 
     /**
      * <p><b>Use {@link ZKConfig#ZKConfig(Path configPath)} instead.</b>
      *
-     * <p><b>The signature of this method will be changed to throw {@link ConfigException}
-     * instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
+     * <p><b>The signature of this method has been changed to throw {@link ConfigException}
+     * instead of {@code QuorumPeerConfig.ConfigException}.</b>
      *
      * @param configFile
      *            Configuration file
      * @throws ConfigException
      *             if failed to load configuration properties
      */
     @Deprecated
-    public ZKConfig(File configFile) throws QuorumPeerConfig.ConfigException {
-        this();
-        addConfiguration(configFile);
-        LOG.info("ZK Config {}", this.properties);
+    public ZKConfig(File configFile) throws ConfigException {
+        this(configFile.toPath());
     }
 
     /**
@@ -102,9 +101,10 @@ public ZKConfig(File configFile) throws QuorumPeerConfig.ConfigException {
      * @param configPath path to configuration file
      * @throws ConfigException
      */
-    @SuppressWarnings("deprecation")
     public ZKConfig(Path configPath) throws ConfigException {
-        this(configPath.toFile());
+        this();
+        addConfiguration(configPath);
+        LOG.info("ZK Config {}", this.properties);
     }
 
     private void init() {
@@ -216,16 +216,39 @@ public void setProperty(String key, String value) {
      *
      * @param configPath path to Configuration file.
      */
-    @SuppressWarnings("deprecation")
     public void addConfiguration(Path configPath) throws ConfigException {
-        addConfiguration(configPath.toFile());
+        Path absoluteConfigPath = configPath.toAbsolutePath();
+        LOG.info("Reading configuration from: {}", absoluteConfigPath);
+        try {
+            File configFile = (new VerifyingFileFactory.Builder(LOG).warnForRelativePath()
+                                                               .failForNonExistingPath()
+                                                               .build()).validate(configPath.toFile());
+            Properties cfg = new Properties();
+            try (FileInputStream in = new FileInputStream(configFile)) {
+                cfg.load(in);
+            }
+            parseProperties(cfg);
+        } catch (IOException | IllegalArgumentException e) {
+            LOG.error("Error while configuration from: {}", absoluteConfigPath, e);
+            String msg = "Error while processing " + absoluteConfigPath;
+            try {
+                Class<?> clazz = Class.forName("org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException");
+                Class<? extends ConfigException> exceptionClass = clazz.asSubclass(ConfigException.class);
+                Constructor<? extends ConfigException> constructor = exceptionClass.getDeclaredConstructor(String.class, Exception.class);
+                throw constructor.newInstance(msg, e);
+            } catch (ClassNotFoundException ignored) {
+            } catch (Exception ignored) {
+                LOG.warn("Fail to construct QuorumPeerConfig.ConfigException", e);
+            }
+            throw new ConfigException(msg, e);
+        }
     }
 
     /**
      * <p><b>Use {@link #addConfiguration(Path)} instead.</b></p>
      *
-     * <p><b>The signature of this method will be changed to throw {@link ConfigException}
-     * instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
+     * <p><b>The signature of this method has been changed to throw {@link ConfigException}
+     * instead of {@code QuorumPeerConfig.ConfigException}.</b>
      *
      * <p>Add a configuration resource. The properties form this configuration will
      * overwrite corresponding already loaded property and system property
@@ -234,31 +257,15 @@ public void addConfiguration(Path configPath) throws ConfigException {
      *            Configuration file.
      */
     @Deprecated
-    public void addConfiguration(File configFile) throws QuorumPeerConfig.ConfigException {
-        LOG.info("Reading configuration from: {}", configFile.getAbsolutePath());
-        try {
-            configFile = (new VerifyingFileFactory.Builder(LOG).warnForRelativePath()
-                                                               .failForNonExistingPath()
-                                                               .build()).validate(configFile);
-            Properties cfg = new Properties();
-            FileInputStream in = new FileInputStream(configFile);
-            try {
-                cfg.load(in);
-            } finally {
-                in.close();
-            }
-            parseProperties(cfg);
-        } catch (IOException | IllegalArgumentException e) {
-            LOG.error("Error while configuration from: {}", configFile.getAbsolutePath(), e);
-            throw new QuorumPeerConfig.ConfigException("Error while processing " + configFile.getAbsolutePath(), e);
-        }
+    public void addConfiguration(File configFile) throws ConfigException {
+        addConfiguration(configFile.toPath());
     }
 
     /**
      * <p><b>Use {@link #addConfiguration(Path)} instead.</b></p>
      *
-     * <p><b>The signature of this method will be changed to throw {@link ConfigException}
-     * instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
+     * <p><b>The signature of this method has been changed to throw {@link ConfigException}
+     * instead of {@code QuorumPeerConfig.ConfigException}.</b>
      *
      * <p>Add a configuration resource. The properties form this configuration will
      * overwrite corresponding already loaded property and system property
@@ -267,8 +274,8 @@ public void addConfiguration(File configFile) throws QuorumPeerConfig.ConfigExce
      *            Configuration file path.
      */
     @Deprecated
-    public void addConfiguration(String configPath) throws QuorumPeerConfig.ConfigException {
-        addConfiguration(new File(configPath));
+    public void addConfiguration(String configPath) throws ConfigException {
+        addConfiguration(Paths.get(configPath));
     }
 
     private void parseProperties(Properties cfg) {

zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java

@@ -62,6 +62,7 @@
 import javax.net.ssl.SSLSession;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.common.NettyUtils;
 import org.apache.zookeeper.common.X509Exception;
 import org.apache.zookeeper.common.X509Exception.SSLContextException;
@@ -529,7 +530,7 @@ private ServerBootstrap configureBootstrapAllocator(ServerBootstrap bootstrap) {
         if (usePortUnification) {
             try {
                 QuorumPeerConfig.configureSSLAuth();
-            } catch (QuorumPeerConfig.ConfigException e) {
+            } catch (ConfigException e) {
                 LOG.error("unable to set up SslAuthProvider, turning off client port unification", e);
                 usePortUnification = false;
             }

zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java

@@ -40,6 +40,7 @@
 import org.apache.zookeeper.Op;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.ZooDefs.OpCode;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.common.PathUtils;
 import org.apache.zookeeper.common.StringUtils;
 import org.apache.zookeeper.common.Time;
@@ -60,7 +61,6 @@
 import org.apache.zookeeper.server.quorum.LeaderZooKeeperServer;
 import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
 import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.apache.zookeeper.server.quorum.flexible.QuorumMaj;
 import org.apache.zookeeper.server.quorum.flexible.QuorumOracleMaj;
 import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier;

zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerConfig.java

@@ -23,9 +23,9 @@
 import java.util.Arrays;
 import java.util.Properties;
 import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.metrics.impl.DefaultMetricsProvider;
 import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 
 /**
  * Server configuration storage.

zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java

@@ -23,6 +23,7 @@
 import javax.management.JMException;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.zookeeper.audit.ZKAuditProvider;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.jmx.ManagedUtil;
 import org.apache.zookeeper.metrics.MetricsProvider;
 import org.apache.zookeeper.metrics.MetricsProviderLifeCycleException;
@@ -33,7 +34,6 @@
 import org.apache.zookeeper.server.auth.ProviderRegistry;
 import org.apache.zookeeper.server.persistence.FileTxnSnapLog;
 import org.apache.zookeeper.server.persistence.FileTxnSnapLog.DatadirException;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.apache.zookeeper.server.util.JvmPauseMonitor;
 import org.apache.zookeeper.util.ServiceUtils;
 import org.slf4j.Logger;

zookeeper-server/src/main/java/org/apache/zookeeper/server/controller/ControllerServerConfig.java

@@ -25,6 +25,7 @@
 import java.net.ServerSocket;
 import java.util.HashMap;
 import java.util.Map;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.server.ServerConfig;
 import org.apache.zookeeper.server.quorum.QuorumPeer;
 import org.apache.zookeeper.server.quorum.QuorumPeerConfig;

zookeeper-server/src/main/java/org/apache/zookeeper/server/controller/ControllerService.java

@@ -19,6 +19,7 @@
 package org.apache.zookeeper.server.controller;
 
 import java.io.IOException;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.server.ExitCode;
 import org.apache.zookeeper.server.ServerCnxnFactory;
 import org.apache.zookeeper.server.quorum.QuorumPeer;
@@ -119,15 +120,15 @@ protected void cleanup() {
     /**
      * Runs the main loop for this application but does not exit the process.
      */
-    public void initializeAndRun(String[] args) throws QuorumPeerConfig.ConfigException {
+    public void initializeAndRun(String[] args) throws ConfigException {
         initConfig(args);
         run();
     }
 
     /**
      * Derived classes may override to do custom initialization of command line args.
      */
-    protected void initConfig(String[] args) throws QuorumPeerConfig.ConfigException {
+    protected void initConfig(String[] args) throws ConfigException {
         if (args.length == 1) {
             config.parse(args[0]);
         }

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/FastLeaderElection.java

@@ -27,13 +27,13 @@
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.jmx.MBeanRegistry;
 import org.apache.zookeeper.server.ZooKeeperThread;
 import org.apache.zookeeper.server.quorum.QuorumCnxManager.Message;
 import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType;
 import org.apache.zookeeper.server.quorum.QuorumPeer.ServerState;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.apache.zookeeper.server.quorum.flexible.QuorumOracleMaj;
 import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier;
 import org.apache.zookeeper.server.util.ZxidUtils;

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java

@@ -61,11 +61,11 @@
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import javax.net.ssl.SSLSocket;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.common.NetUtils;
 import org.apache.zookeeper.common.X509Exception;
 import org.apache.zookeeper.server.ExitCode;
 import org.apache.zookeeper.server.ZooKeeperThread;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.apache.zookeeper.server.quorum.auth.QuorumAuthLearner;
 import org.apache.zookeeper.server.quorum.auth.QuorumAuthServer;
 import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier;