From 864c4ab6945ec2d2e57109f76055e80a3618ab57 Mon Sep 17 00:00:00 2001 From: Kezhu Wang Date: Tue, 19 Aug 2025 21:03:17 +0800 Subject: [PATCH] Add builder to construct InstanceSpec It is hard for us to know what does `new InstanceSpec(null, -1, -1, -1, true, -1, -1, -1)` means now. Closes #1222. --- .../org/apache/curator/test/InstanceSpec.java | 52 +++++++- .../curator/test/InstanceSpecBuilder.java | 111 ++++++++++++++++++ .../apache/curator/test/TestingServer.java | 7 +- .../curator/test/TestQuorumConfigBuilder.java | 7 +- .../curator/test/TestTestingServer.java | 6 +- 5 files changed, 175 insertions(+), 8 deletions(-) create mode 100644 curator-test/src/main/java/org/apache/curator/test/InstanceSpecBuilder.java diff --git a/curator-test/src/main/java/org/apache/curator/test/InstanceSpec.java b/curator-test/src/main/java/org/apache/curator/test/InstanceSpec.java index 531f5af74..2fa502f43 100644 --- a/curator-test/src/main/java/org/apache/curator/test/InstanceSpec.java +++ b/curator-test/src/main/java/org/apache/curator/test/InstanceSpec.java @@ -70,8 +70,15 @@ public static void reset() { nextServerId.set(1); } + /** + * Create a builder to construct {@link InstanceSpec}. + */ + public static InstanceSpecBuilder builder() { + return new InstanceSpecBuilder(); + } + public static InstanceSpec newInstanceSpec() { - return new InstanceSpec(null, -1, -1, -1, true, -1, -1, -1); + return builder().build(); } public static int getRandomPort() { @@ -94,6 +101,8 @@ public static int getRandomPort() { } /** + * @deprecated Use {@link #builder()} instead. + * * @param dataDirectory where to store data/logs/etc. * @param port the port to listen on - each server in the ensemble must use a unique port * @param electionPort the electionPort to listen on - each server in the ensemble must use a unique electionPort @@ -101,6 +110,7 @@ public static int getRandomPort() { * @param deleteDataDirectoryOnClose if true, the data directory will be deleted when {@link TestingCluster#close()} is called * @param serverId the server ID for the instance */ + @Deprecated public InstanceSpec( File dataDirectory, int port, @@ -112,6 +122,8 @@ public InstanceSpec( } /** + * @deprecated Use {@link #builder()} instead. + * * @param dataDirectory where to store data/logs/etc. * @param port the port to listen on - each server in the ensemble must use a unique port * @param electionPort the electionPort to listen on - each server in the ensemble must use a unique electionPort @@ -121,6 +133,7 @@ public InstanceSpec( * @param tickTime tickTime. Set -1 to used fault server configuration * @param maxClientCnxns max number of client connections from the same IP. Set -1 to use default server configuration */ + @Deprecated public InstanceSpec( File dataDirectory, int port, @@ -144,6 +157,8 @@ public InstanceSpec( } /** + * @deprecated Use {@link #builder()} instead. + * * @param dataDirectory where to store data/logs/etc. * @param port the port to listen on - each server in the ensemble must use a unique port * @param electionPort the electionPort to listen on - each server in the ensemble must use a unique electionPort @@ -154,6 +169,7 @@ public InstanceSpec( * @param maxClientCnxns max number of client connections from the same IP. Set -1 to use default server configuration * @param customProperties other properties to be passed to the server */ + @Deprecated public InstanceSpec( File dataDirectory, int port, @@ -178,6 +194,8 @@ public InstanceSpec( } /** + * @deprecated Use {@link #builder()} instead. + * * @param dataDirectory where to store data/logs/etc. * @param port the port to listen on - each server in the ensemble must use a unique port * @param electionPort the electionPort to listen on - each server in the ensemble must use a unique electionPort @@ -189,6 +207,7 @@ public InstanceSpec( * @param customProperties other properties to be passed to the server * @param hostname Hostname or IP if the cluster is intending to be bounded into external interfaces */ + @Deprecated public InstanceSpec( File dataDirectory, int port, @@ -200,6 +219,32 @@ public InstanceSpec( int maxClientCnxns, Map customProperties, String hostname) { + this( + dataDirectory, + port, + electionPort, + quorumPort, + deleteDataDirectoryOnClose, + serverId, + tickTime, + maxClientCnxns, + customProperties != null ? enforceStringMap(customProperties) : null, + hostname, + false); + } + + InstanceSpec( + File dataDirectory, + int port, + int electionPort, + int quorumPort, + boolean deleteDataDirectoryOnClose, + int serverId, + int tickTime, + int maxClientCnxns, + Map customProperties, + String hostname, + boolean ignored) { this.dataDirectory = (dataDirectory != null) ? dataDirectory : DirectoryUtils.createTempDirectory(); this.port = (port >= 0) ? port : getRandomPort(); this.electionPort = (electionPort >= 0) ? electionPort : getRandomPort(); @@ -208,9 +253,8 @@ public InstanceSpec( this.serverId = (serverId >= 0) ? serverId : nextServerId.getAndIncrement(); this.tickTime = (tickTime > 0 ? tickTime : -1); // -1 to set default value this.maxClientCnxns = (maxClientCnxns >= 0 ? maxClientCnxns : -1); // -1 to set default value - this.customProperties = customProperties != null - ? Collections.unmodifiableMap(enforceStringMap(customProperties)) - : Collections.emptyMap(); + this.customProperties = + customProperties != null ? Collections.unmodifiableMap(customProperties) : Collections.emptyMap(); this.hostname = hostname == null ? localhost : hostname; } diff --git a/curator-test/src/main/java/org/apache/curator/test/InstanceSpecBuilder.java b/curator-test/src/main/java/org/apache/curator/test/InstanceSpecBuilder.java new file mode 100644 index 000000000..a453ef7c4 --- /dev/null +++ b/curator-test/src/main/java/org/apache/curator/test/InstanceSpecBuilder.java @@ -0,0 +1,111 @@ +/* + * 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.curator.test; + +import java.io.File; +import java.util.Map; + +/** + * Builder to construct {@link InstanceSpec}. + */ +public class InstanceSpecBuilder { + private File dataDirectory; + private boolean deleteDataDirectoryOnClose = true; + private int port = -1; + private int electionPort = -1; + private int quorumPort = -1; + private int serverId = -1; + private int tickTime = -1; + private int maxClientCnxns = -1; + private Map customProperties = null; + private String hostname = null; + + /** + * Data directory to store data of this server instance. It will be deleted upon sever close. + * + * @see #withDataDirectory(File, boolean) also + */ + public InstanceSpecBuilder withDataDirectory(File dataDirectory) { + this.dataDirectory = dataDirectory; + return this; + } + + public InstanceSpecBuilder withDataDirectory(File dataDirectory, boolean deleteDataDirectoryOnClose) { + this.dataDirectory = dataDirectory; + this.deleteDataDirectoryOnClose = deleteDataDirectoryOnClose; + return this; + } + + public InstanceSpecBuilder withPort(int port) { + this.port = port; + return this; + } + + public InstanceSpecBuilder withElectionPort(int electionPort) { + this.electionPort = electionPort; + return this; + } + + public InstanceSpecBuilder withQuorumPort(int quorumPort) { + this.quorumPort = quorumPort; + return this; + } + + public InstanceSpecBuilder withServerId(int serverId) { + this.serverId = serverId; + return this; + } + + public InstanceSpecBuilder withTickTime(int tickTime) { + this.tickTime = tickTime; + return this; + } + + public InstanceSpecBuilder withMaxClientCnxns(int maxClientCnxns) { + this.maxClientCnxns = maxClientCnxns; + return this; + } + + public InstanceSpecBuilder withCustomProperties(Map customProperties) { + this.customProperties = customProperties; + return this; + } + + public InstanceSpecBuilder withHostname(String hostname) { + this.hostname = hostname; + return this; + } + + @SuppressWarnings("unchecked") + public InstanceSpec build() { + return new InstanceSpec( + dataDirectory, + port, + electionPort, + quorumPort, + deleteDataDirectoryOnClose, + serverId, + tickTime, + maxClientCnxns, + (Map) customProperties, + hostname, + true); + } +} diff --git a/curator-test/src/main/java/org/apache/curator/test/TestingServer.java b/curator-test/src/main/java/org/apache/curator/test/TestingServer.java index e7ef2dcb9..dbaa6842a 100644 --- a/curator-test/src/main/java/org/apache/curator/test/TestingServer.java +++ b/curator-test/src/main/java/org/apache/curator/test/TestingServer.java @@ -94,7 +94,12 @@ public TestingServer(int port, File tempDirectory) throws Exception { * @throws Exception errors */ public TestingServer(int port, File tempDirectory, boolean start) throws Exception { - this(new InstanceSpec(tempDirectory, Math.max(0, port), -1, -1, true, -1), start); + this( + InstanceSpec.builder() + .withPort(Math.max(0, port)) + .withDataDirectory(tempDirectory) + .build(), + start); } /** diff --git a/curator-test/src/test/java/org/apache/curator/test/TestQuorumConfigBuilder.java b/curator-test/src/test/java/org/apache/curator/test/TestQuorumConfigBuilder.java index cdf6437ef..eb400b223 100644 --- a/curator-test/src/test/java/org/apache/curator/test/TestQuorumConfigBuilder.java +++ b/curator-test/src/test/java/org/apache/curator/test/TestQuorumConfigBuilder.java @@ -31,11 +31,14 @@ public class TestQuorumConfigBuilder { @Test public void testCustomProperties() throws Exception { - Map customProperties = new HashMap(); + Map customProperties = new HashMap(); customProperties.put("authProvider.1", "org.apache.zookeeper.server.auth.SASLAuthenticationProvider"); customProperties.put("kerberos.removeHostFromPrincipal", "true"); customProperties.put("kerberos.removeRealmFromPrincipal", "true"); - InstanceSpec spec = new InstanceSpec(null, -1, -1, -1, true, 1, -1, -1, customProperties); + InstanceSpec spec = InstanceSpec.builder() + .withServerId(1) + .withCustomProperties(customProperties) + .build(); TestingServer server = new TestingServer(spec, true); try { assertEquals( diff --git a/curator-test/src/test/java/org/apache/curator/test/TestTestingServer.java b/curator-test/src/test/java/org/apache/curator/test/TestTestingServer.java index c442533b7..631e4c358 100644 --- a/curator-test/src/test/java/org/apache/curator/test/TestTestingServer.java +++ b/curator-test/src/test/java/org/apache/curator/test/TestTestingServer.java @@ -41,7 +41,11 @@ public void setCustomTickTimeTest() throws Exception { } else { customTickMs = 100; } - final InstanceSpec spec = new InstanceSpec(zkTmpDir, -1, -1, -1, true, -1, customTickMs, -1); + final InstanceSpec spec = InstanceSpec.builder() + .withDataDirectory(zkTmpDir) + .withTickTime(customTickMs) + .build(); + ; final int zkTickTime; try (TestingServer testingServer = new TestingServer(spec, true)) { TestingZooKeeperMain main = (TestingZooKeeperMain)