ZOOKEEPER-4970: Deprecate ZKConfig methods which throw QuorumPeerConfig.ConfigException

`QuorumPeerConfig` belongs to and ties with other server code. To
separate a slimmer `zookeeper-client` jar, we have to break the
dependency from `ZKConfig` to `QuorumPeerConfig`. But this is a breaking
change, it would be good to deprecate these methods first to ease future
migration.

Refs: ZOOKEEPER-4966, ZOOKEEPER-4970, ZOOKEEPER-233.

Reviewers: tisonkun
Author: kezhuw

Closes apache#2309 from kezhuw/ZOOKEEPER-4970-deprecate-ConfigException-from-ZKConfig

(cherry picked from bb76c97)

Author: kezhuw

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

@@ -23,6 +23,7 @@
 import java.io.InputStreamReader;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -45,8 +46,8 @@
 import org.apache.zookeeper.cli.CommandNotFoundException;
 import org.apache.zookeeper.cli.MalformedCommandException;
 import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.server.ExitCode;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
 import org.apache.zookeeper.util.ServiceUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -266,8 +267,8 @@ protected void connectToZK(String newHost) throws InterruptedException, IOExcept
 
         if (cl.getOption("client-configuration") != null) {
             try {
-                clientConfig = new ZKClientConfig(cl.getOption("client-configuration"));
-            } catch (QuorumPeerConfig.ConfigException e) {
+                clientConfig = new ZKClientConfig(Paths.get(cl.getOption("client-configuration")));
+            } catch (ConfigException e) {
                 e.printStackTrace();
                 ServiceUtils.requestSystemExit(ExitCode.INVALID_INVOCATION.getValue());
             }

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

@@ -19,9 +19,11 @@
 package org.apache.zookeeper.client;
 
 import java.io.File;
+import java.nio.file.Path;
 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.ConfigException;
+import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
 
 /**
  * Handles client specific properties
@@ -64,11 +66,29 @@ public ZKClientConfig() {
         initFromJavaSystemProperties();
     }
 
-    public ZKClientConfig(File configFile) throws ConfigException {
+    /**
+     * <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>
+     */
+    @Deprecated
+    public ZKClientConfig(File configFile) throws QuorumPeerConfig.ConfigException {
         super(configFile);
     }
 
-    public ZKClientConfig(String configPath) throws ConfigException {
+    /**
+     * <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>
+     */
+    @Deprecated
+    public ZKClientConfig(String configPath) throws QuorumPeerConfig.ConfigException {
+        super(configPath);
+    }
+
+    public ZKClientConfig(Path configPath) throws ConfigException {
         super(configPath);
     }
 

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

@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
+
+/**
+ * Configuration related exception.
+ */
+public class ConfigException extends Exception {
+    public ConfigException(String msg) {
+        super(msg);
+    }
+    public ConfigException(String msg, Exception e) {
+        super(msg, e);
+    }
+}

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

@@ -21,12 +21,13 @@
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.nio.file.Path;
 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.ConfigException;
+import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
 import org.apache.zookeeper.server.util.VerifyingFileFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -62,29 +63,50 @@ 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>
+     *
      * @param configPath
      *            Configuration file path
      * @throws ConfigException
      *             if failed to load configuration properties
      */
-
-    public ZKConfig(String configPath) throws ConfigException {
+    @Deprecated
+    public ZKConfig(String configPath) throws QuorumPeerConfig.ConfigException {
         this(new File(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>
      *
      * @param configFile
      *            Configuration file
      * @throws ConfigException
      *             if failed to load configuration properties
      */
-    public ZKConfig(File configFile) throws ConfigException {
+    @Deprecated
+    public ZKConfig(File configFile) throws QuorumPeerConfig.ConfigException {
         this();
         addConfiguration(configFile);
         LOG.info("ZK Config {}", this.properties);
     }
 
+    /**
+     * Constructs a {@link ZKConfig} with properties from file.
+     *
+     * @param configPath path to configuration file
+     * @throws ConfigException
+     */
+    @SuppressWarnings("deprecation")
+    public ZKConfig(Path configPath) throws ConfigException {
+        this(configPath.toFile());
+    }
+
     private void init() {
         /**
          * backward compatibility for all currently available client properties
@@ -191,10 +213,27 @@ public void setProperty(String key, String value) {
      * Add a configuration resource. The properties form this configuration will
      * overwrite corresponding already loaded property and system property
      *
+     * @param configPath path to Configuration file.
+     */
+    @SuppressWarnings("deprecation")
+    public void addConfiguration(Path configPath) throws ConfigException {
+        addConfiguration(configPath.toFile());
+    }
+
+    /**
+     * <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>Add a configuration resource. The properties form this configuration will
+     * overwrite corresponding already loaded property and system property
+     *
      * @param configFile
      *            Configuration file.
      */
-    public void addConfiguration(File configFile) throws ConfigException {
+    @Deprecated
+    public void addConfiguration(File configFile) throws QuorumPeerConfig.ConfigException {
         LOG.info("Reading configuration from: {}", configFile.getAbsolutePath());
         try {
             configFile = (new VerifyingFileFactory.Builder(LOG).warnForRelativePath()
@@ -210,18 +249,24 @@ public void addConfiguration(File configFile) throws ConfigException {
             parseProperties(cfg);
         } catch (IOException | IllegalArgumentException e) {
             LOG.error("Error while configuration from: {}", configFile.getAbsolutePath(), e);
-            throw new ConfigException("Error while processing " + configFile.getAbsolutePath(), e);
+            throw new QuorumPeerConfig.ConfigException("Error while processing " + configFile.getAbsolutePath(), e);
         }
     }
 
     /**
-     * Add a configuration resource. The properties form this configuration will
+     * <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>Add a configuration resource. The properties form this configuration will
      * overwrite corresponding already loaded property and system property
      *
      * @param configPath
      *            Configuration file path.
      */
-    public void addConfiguration(String configPath) throws ConfigException {
+    @Deprecated
+    public void addConfiguration(String configPath) throws QuorumPeerConfig.ConfigException {
         addConfiguration(new File(configPath));
     }
 

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

@@ -157,7 +157,7 @@ public class QuorumPeerConfig {
     protected long jvmPauseSleepTimeMs = JvmPauseMonitor.SLEEP_TIME_MS_DEFAULT;
 
     @SuppressWarnings("serial")
-    public static class ConfigException extends Exception {
+    public static class ConfigException extends org.apache.zookeeper.common.ConfigException {
 
         public ConfigException(String msg) {
             super(msg);

zookeeper-server/src/test/java/org/apache/zookeeper/client/ZKClientConfigTest.java

@@ -33,11 +33,12 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.nio.file.Paths;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
+import org.apache.zookeeper.common.ConfigException;
 import org.apache.zookeeper.common.ZKConfig;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Timeout;
 import org.junit.jupiter.api.io.TempDir;
@@ -116,7 +117,7 @@ public void testReadConfigurationFile(@TempDir File testDataDir) throws IOExcept
         }
 
         ZKClientConfig conf = new ZKClientConfig();
-        conf.addConfiguration(file.getAbsolutePath());
+        conf.addConfiguration(Paths.get(file.getAbsolutePath()));
         assertEquals(conf.getProperty(ENABLE_CLIENT_SASL_KEY), "true");
         assertEquals(conf.getProperty(ZK_SASL_CLIENT_USERNAME), "ZK");
         assertEquals(conf.getProperty(LOGIN_CONTEXT_NAME_KEY), "MyClient");