ZOOKEEPER-4963: Add ZooKeeper::builder to replace new ZooKeeperBuilder

ZOOKEEPER-4963: Add `ZooKeeper::builder` to replace `new ZooKeeperBuilder`
`ZooKeeper::builder` should be better:
1. More exposure chance as a newly introduce method in class `ZooKeeper`.
2. No need to import or remember the newly introduced class as most
   `ZooKeeper` instances could be built in one chain.
`ZooKeeperBuilder` is introduced in 3.10.0 so it safe to do this.
Refs: ZOOKEEPER-4697
Reviewers: anmolnar
Author: kezhuw
Closes #2301 from kezhuw/ZOOKEEPER-4963-ZooKeeper_builder

Author: kezhuw

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

@@ -418,6 +418,25 @@ public boolean isConnected() {
         }
     }
 
+    /**
+     * Creates a builder with given connect string and session timeout.
+     *
+     * @param connectString
+     *            comma separated host:port pairs, each corresponding to a zk
+     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002"
+     *            If the optional chroot suffix is used the example would look
+     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+     *            where the client would be rooted at "/app/a" and all paths
+     *            would be relative to this root - ie getting/setting/etc...
+     *            "/foo/bar" would result in operations being run on
+     *            "/app/a/foo/bar" (from the server perspective).
+     * @param sessionTimeout
+     *            session timeout
+     */
+    public static ZooKeeperBuilder builder(String connectString, Duration sessionTimeout) {
+        return new ZooKeeperBuilder(connectString, sessionTimeout);
+    }
+
     /**
      * To create a ZooKeeper client object, the application needs to pass a
      * connection string containing a comma separated list of host:port pairs,
@@ -460,9 +479,11 @@ public boolean isConnected() {
      *             in cases of network failure
      * @throws IllegalArgumentException
      *             if an invalid chroot path is specified
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .toOptions());
     }
@@ -511,13 +532,15 @@ public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) thro
      *             in cases of network failure
      * @throws IllegalArgumentException
      *             if an invalid chroot path is specified
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
         int sessionTimeout,
         Watcher watcher,
         ZKClientConfig conf) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .withClientConfig(conf)
             .toOptions());
@@ -579,14 +602,16 @@ public ZooKeeper(
      *             in cases of network failure
      * @throws IllegalArgumentException
      *             if an invalid chroot path is specified
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
         int sessionTimeout,
         Watcher watcher,
         boolean canBeReadOnly,
         HostProvider aHostProvider) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .withCanBeReadOnly(canBeReadOnly)
             .withHostProvider(ignored -> aHostProvider)
@@ -651,6 +676,8 @@ public ZooKeeper(
      *             in cases of network failure
      * @throws IllegalArgumentException
      *             if an invalid chroot path is specified
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
@@ -660,7 +687,7 @@ public ZooKeeper(
         HostProvider hostProvider,
         ZKClientConfig clientConfig
     ) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .withCanBeReadOnly(canBeReadOnly)
             .withHostProvider(ignored -> hostProvider)
@@ -740,13 +767,15 @@ ClientCnxn createConnection(
      *             in cases of network failure
      * @throws IllegalArgumentException
      *             if an invalid chroot path is specified
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
         int sessionTimeout,
         Watcher watcher,
         boolean canBeReadOnly) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .withCanBeReadOnly(canBeReadOnly)
             .toOptions());
@@ -805,14 +834,16 @@ public ZooKeeper(
      *             in cases of network failure
      * @throws IllegalArgumentException
      *             if an invalid chroot path is specified
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
         int sessionTimeout,
         Watcher watcher,
         boolean canBeReadOnly,
         ZKClientConfig conf) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .withCanBeReadOnly(canBeReadOnly)
             .withClientConfig(conf)
@@ -870,14 +901,16 @@ public ZooKeeper(
      * @throws IOException in cases of network failure
      * @throws IllegalArgumentException if an invalid chroot path is specified
      * @throws IllegalArgumentException for an invalid list of ZooKeeper hosts
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
         int sessionTimeout,
         Watcher watcher,
         long sessionId,
         byte[] sessionPasswd) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .withSession(sessionId, sessionPasswd)
             .toOptions());
@@ -946,6 +979,8 @@ public ZooKeeper(
      *            use this as HostProvider to enable custom behaviour.
      * @throws IOException in cases of network failure
      * @throws IllegalArgumentException if an invalid chroot path is specified
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
@@ -955,7 +990,7 @@ public ZooKeeper(
         byte[] sessionPasswd,
         boolean canBeReadOnly,
         HostProvider aHostProvider) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .withSession(sessionId, sessionPasswd)
             .withCanBeReadOnly(canBeReadOnly)
@@ -1031,6 +1066,8 @@ public ZooKeeper(
      * @throws IllegalArgumentException if an invalid chroot path is specified
      *
      * @since 3.5.5
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
@@ -1041,7 +1078,7 @@ public ZooKeeper(
         boolean canBeReadOnly,
         HostProvider hostProvider,
         ZKClientConfig clientConfig) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withSession(sessionId, sessionPasswd)
             .withDefaultWatcher(watcher)
             .withCanBeReadOnly(canBeReadOnly)
@@ -1053,6 +1090,8 @@ public ZooKeeper(
     /**
      * Create a ZooKeeper client and establish session asynchronously.
      *
+     * <p>This is private and export for internal usage.
+     *
      * <p>This constructor will initiate connection to the server and return
      * immediately - potentially (usually) before the session is fully established.
      * The watcher from options will be notified of any changes in state. This
@@ -1180,6 +1219,8 @@ public ZooKeeper(ZooKeeperOptions options) throws IOException {
      *            majority in the background.
      * @throws IOException in cases of network failure
      * @throws IllegalArgumentException if an invalid chroot path is specified
+     *
+     * @see #builder(String, Duration) for builder style construction
      */
     public ZooKeeper(
         String connectString,
@@ -1188,7 +1229,7 @@ public ZooKeeper(
         long sessionId,
         byte[] sessionPasswd,
         boolean canBeReadOnly) throws IOException {
-        this(new ZooKeeperBuilder(connectString, Duration.ofMillis(sessionTimeout))
+        this(builder(connectString, Duration.ofMillis(sessionTimeout))
             .withDefaultWatcher(watcher)
             .withSession(sessionId, sessionPasswd)
             .withCanBeReadOnly(canBeReadOnly)

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

@@ -47,22 +47,11 @@ public class ZooKeeperBuilder {
     private ZKClientConfig clientConfig;
 
     /**
-     * Creates a builder with given connect string and session timeout.
-     *
-     * @param connectString
-     *            comma separated host:port pairs, each corresponding to a zk
-     *            server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002"
-     *            If the optional chroot suffix is used the example would look
-     *            like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
-     *            where the client would be rooted at "/app/a" and all paths
-     *            would be relative to this root - ie getting/setting/etc...
-     *            "/foo/bar" would result in operations being run on
-     *            "/app/a/foo/bar" (from the server perspective).
-     * @param sessionTimeout
-     *            session timeout
+     * This is private and export for internal usage. Use {@link ZooKeeper#builder(String, Duration)} instead.
      */
+    @InterfaceAudience.Private
     public ZooKeeperBuilder(String connectString, Duration sessionTimeout) {
-        this.connectString = connectString;
+        this.connectString = Objects.requireNonNull(connectString, "connect string must not be null");
         this.sessionTimeout = Objects.requireNonNull(sessionTimeout, "session timeout must not be null");
     }
 
@@ -145,6 +134,8 @@ public ZooKeeperBuilder withClientConfig(ZKClientConfig clientConfig) {
     /**
      * Creates a {@link ZooKeeperOptions} with configured options.
      *
+     * <p>This is private and export for internal usage.
+     *
      * @apiNote helper to delegate existing constructors to {@link ZooKeeper#ZooKeeper(ZooKeeperOptions)}
      */
     @InterfaceAudience.Private

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

@@ -29,6 +29,7 @@
 import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.admin.ZooKeeperAdmin;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.test.ClientBase;
 import org.junit.jupiter.api.Test;
@@ -70,7 +71,7 @@ private void testClient(BlockingQueue<WatchedEvent> events, ZooKeeper zk) throws
     @Test
     public void testBuildClient() throws Exception {
         BlockingQueue<WatchedEvent> events = new LinkedBlockingQueue<>();
-        ZooKeeper zk = new ZooKeeperBuilder(hostPort, Duration.ofMillis(1000))
+        ZooKeeper zk = ZooKeeper.builder(hostPort, Duration.ofMillis(1000))
             .withDefaultWatcher(events::offer)
             .build();
         testClient(events, zk);
@@ -79,7 +80,7 @@ public void testBuildClient() throws Exception {
     @Test
     public void testBuildAdminClient() throws Exception {
         BlockingQueue<WatchedEvent> events = new LinkedBlockingQueue<>();
-        ZooKeeper zk = new ZooKeeperBuilder(hostPort, Duration.ofMillis(1000))
+        ZooKeeperAdmin zk = ZooKeeper.builder(hostPort, Duration.ofMillis(1000))
             .withDefaultWatcher(events::offer)
             .buildAdmin();
         testClient(events, zk);