[improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-perf

pulsar-cli-utils/src/test/java/org/apache/pulsar/cli/converters/ByteConversionTest.java

@@ -21,6 +21,7 @@
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertThrows;
 import org.apache.pulsar.cli.converters.picocli.ByteUnitToIntegerConverter;
+import org.apache.pulsar.cli.converters.picocli.ByteUnitToLongConverter;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import picocli.CommandLine.TypeConversionException;
@@ -59,8 +60,8 @@ public void testSuccessfulByteUnitUtilConversion(String input, long expected) {
     }
 
     @Test(dataProvider = "successfulByteUnitUtilTestCases")
-    public void testSuccessfulByteUnitToLongConverter(String input, long expected) {
-        ByteUnitToLongConverter converter = new ByteUnitToLongConverter("optionName");
+    public void testSuccessfulByteUnitToLongConverter(String input, long expected) throws Exception{
+        ByteUnitToLongConverter converter = new ByteUnitToLongConverter();
         assertEquals(converter.convert(input), Long.valueOf(expected));
     }
 
@@ -78,8 +79,8 @@ public void testFailedByteUnitUtilConversion(String input) {
 
     @Test(dataProvider = "failingByteUnitUtilTestCases")
     public void testFailedByteUnitToLongConverter(String input) {
-        ByteUnitToLongConverter converter = new ByteUnitToLongConverter("optionName");
-        assertThrows(IllegalArgumentException.class, () -> converter.convert(input));
+        ByteUnitToLongConverter converter = new ByteUnitToLongConverter();
+        assertThrows(TypeConversionException.class, () -> converter.convert(input));
     }
 
     @Test(dataProvider = "failingByteUnitUtilTestCases")

pulsar-testclient/pom.xml

@@ -97,8 +97,8 @@
 		</dependency>
 
 		<dependency>
-			<groupId>com.beust</groupId>
-			<artifactId>jcommander</artifactId>
+			<groupId>info.picocli</groupId>
+			<artifactId>picocli</artifactId>
 			<scope>compile</scope>
 		</dependency>
 

pulsar-testclient/src/main/java/org/apache/pulsar/proxy/socket/client/PerformanceClient.java

@@ -20,10 +20,6 @@
 
 import static org.apache.commons.lang3.StringUtils.isBlank;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
-import com.beust.jcommander.JCommander;
-import com.beust.jcommander.Parameter;
-import com.beust.jcommander.ParameterException;
-import com.beust.jcommander.Parameters;
 import com.google.common.util.concurrent.RateLimiter;
 import io.netty.util.concurrent.DefaultThreadFactory;
 import java.io.FileInputStream;
@@ -55,13 +51,18 @@
 import org.apache.pulsar.common.naming.TopicName;
 import org.apache.pulsar.testclient.IMessageFormatter;
 import org.apache.pulsar.testclient.PerfClientUtils;
-import org.apache.pulsar.testclient.PositiveNumberParameterValidator;
+import org.apache.pulsar.testclient.PositiveNumberParameterConvert;
 import org.apache.pulsar.testclient.utils.PaddingDecimalFormat;
 import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.eclipse.jetty.websocket.client.ClientUpgradeRequest;
 import org.eclipse.jetty.websocket.client.WebSocketClient;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+import picocli.CommandLine.ParameterException;
+import picocli.CommandLine.Parameters;
 
 public class PerformanceClient {
 
@@ -70,87 +71,87 @@ public class PerformanceClient {
     private static final LongAdder totalMessagesSent = new LongAdder();
     private static final LongAdder totalBytesSent = new LongAdder();
     private static IMessageFormatter messageFormatter = null;
-    private JCommander jc;
+    private CommandLine commander;
 
-    @Parameters(commandDescription = "Test pulsar websocket producer performance.")
+    @Command(description = "Test pulsar websocket producer performance.")
     static class Arguments {
 
-        @Parameter(names = { "-h", "--help" }, description = "Help message", help = true)
+        @Option(names = { "-h", "--help" }, description = "Help message", help = true)
         boolean help;
 
-        @Parameter(names = { "-cf", "--conf-file" }, description = "Configuration file")
+        @Option(names = { "-cf", "--conf-file" }, description = "Configuration file")
         public String confFile;
 
-        @Parameter(names = { "-u", "--proxy-url" }, description = "Pulsar Proxy URL, e.g., \"ws://localhost:8080/\"")
+        @Option(names = { "-u", "--proxy-url" }, description = "Pulsar Proxy URL, e.g., \"ws://localhost:8080/\"")
         public String proxyURL;
 
-        @Parameter(description = "persistent://tenant/ns/my-topic", required = true)
+        @Parameters(description = "persistent://tenant/ns/my-topic", arity = "1")
         public List<String> topics;
 
-        @Parameter(names = { "-r", "--rate" }, description = "Publish rate msg/s across topics")
+        @Option(names = { "-r", "--rate" }, description = "Publish rate msg/s across topics")
         public int msgRate = 100;
 
-        @Parameter(names = { "-s", "--size" }, description = "Message size in byte")
+        @Option(names = { "-s", "--size" }, description = "Message size in byte")
         public int msgSize = 1024;
 
-        @Parameter(names = { "-t", "--num-topic" }, description = "Number of topics",
-                validateWith = PositiveNumberParameterValidator.class)
+        @Option(names = { "-t", "--num-topic" }, description = "Number of topics",
+                converter = PositiveNumberParameterConvert.class
+        )
         public int numTopics = 1;
 
-        @Parameter(names = { "--auth_plugin" }, description = "Authentication plugin class name", hidden = true)
+        @Option(names = { "--auth_plugin" }, description = "Authentication plugin class name", hidden = true)
         public String deprecatedAuthPluginClassName;
 
-        @Parameter(names = { "--auth-plugin" }, description = "Authentication plugin class name")
+        @Option(names = { "--auth-plugin" }, description = "Authentication plugin class name")
         public String authPluginClassName;
 
-        @Parameter(
+        @Option(
             names = { "--auth-params" },
             description = "Authentication parameters, whose format is determined by the implementation "
                     + "of method `configure` in authentication plugin class, for example \"key1:val1,key2:val2\" "
                     + "or \"{\"key1\":\"val1\",\"key2\":\"val2\"}\".")
         public String authParams;
 
-        @Parameter(names = { "-m",
+        @Option(names = { "-m",
                 "--num-messages" }, description = "Number of messages to publish in total. If <= 0, it will keep"
                 + " publishing")
         public long numMessages = 0;
 
-        @Parameter(names = { "-f", "--payload-file" }, description = "Use payload from a file instead of empty buffer")
+        @Option(names = { "-f", "--payload-file" }, description = "Use payload from a file instead of empty buffer")
         public String payloadFilename = null;
 
-        @Parameter(names = { "-e", "--payload-delimiter" },
+        @Option(names = { "-e", "--payload-delimiter" },
                 description = "The delimiter used to split lines when using payload from a file")
         // here escaping \n since default value will be printed with the help text
         public String payloadDelimiter = "\\n";
 
-        @Parameter(names = { "-fp", "--format-payload" },
+        @Option(names = { "-fp", "--format-payload" },
                 description = "Format %i as a message index in the stream from producer and/or %t as the timestamp"
                         + " nanoseconds")
         public boolean formatPayload = false;
 
-        @Parameter(names = {"-fc", "--format-class"}, description = "Custom Formatter class name")
+        @Option(names = {"-fc", "--format-class"}, description = "Custom Formatter class name")
         public String formatterClass = "org.apache.pulsar.testclient.DefaultMessageFormatter";
 
-        @Parameter(names = { "-time",
+        @Option(names = { "-time",
                 "--test-duration" }, description = "Test duration in secs. If <= 0, it will keep publishing")
         public long testTime = 0;
     }
 
     public Arguments loadArguments(String[] args) {
         Arguments arguments = new Arguments();
-        jc = new JCommander(arguments);
-        jc.setProgramName("pulsar-perf websocket-producer");
-
+        commander = new CommandLine(arguments);
+        commander.setCommandName("pulsar-perf websocket-producer");
         try {
-            jc.parse(args);
+            commander.parseArgs(args);
         } catch (ParameterException e) {
             System.out.println(e.getMessage());
-            jc.usage();
+            commander.usage(commander.getOut());
             PerfClientUtils.exit(1);
         }
 
         if (arguments.help) {
-            jc.usage();
+            commander.usage(commander.getOut());
             PerfClientUtils.exit(1);
         }
 
@@ -160,7 +161,7 @@ public Arguments loadArguments(String[] args) {
 
         if (arguments.topics.size() != 1) {
             System.err.println("Only one topic name is allowed");
-            jc.usage();
+            commander.usage(commander.getOut());
             PerfClientUtils.exit(1);
         }
 
@@ -171,7 +172,7 @@ public Arguments loadArguments(String[] args) {
                 prop.load(new FileInputStream(arguments.confFile));
             } catch (IOException e) {
                 log.error("Error in loading config file");
-                jc.usage();
+                commander.usage(commander.getOut());
                 PerfClientUtils.exit(1);
             }
 

pulsar-testclient/src/main/java/org/apache/pulsar/testclient/BrokerMonitor.java

@@ -20,10 +20,6 @@
 
 import static org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerImpl.BROKER_LOAD_DATA_STORE_TOPIC;
 import static org.apache.pulsar.broker.resources.LoadBalanceResources.BROKER_TIME_AVERAGE_BASE_PATH;
-import com.beust.jcommander.JCommander;
-import com.beust.jcommander.Parameter;
-import com.beust.jcommander.ParameterException;
-import com.beust.jcommander.Parameters;
 import com.google.gson.Gson;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -50,6 +46,11 @@
 import org.apache.zookeeper.ZooKeeper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+import picocli.CommandLine.ParameterException;
+import picocli.CommandLine.ScopeType;
 
 /**
  * Monitors brokers and prints to the console information about their system resource usages, their topic and bundle
@@ -434,17 +435,18 @@ private synchronized void printBrokerData(final String broker, final LocalBroker
         }
     }
 
-    // JCommander arguments class.
-    @Parameters(commandDescription = "Monitors brokers and prints to the console information about their system "
-            + "resource usages, \ntheir topic and bundle counts, their message rates, and other metrics.")
+    // picocli arguments class.
+    @Command(description = "Monitors brokers and prints to the console information about their system "
+            + "resource usages, \ntheir topic and bundle counts, their message rates, and other metrics.",
+            showDefaultValues = true, scope = ScopeType.INHERIT)
     private static class Arguments {
-        @Parameter(names = { "-h", "--help" }, description = "Help message", help = true)
+        @Option(names = { "-h", "--help" }, description = "Help message", help = true)
         boolean help;
 
-        @Parameter(names = { "--connect-string" }, description = "Zookeeper or broker connect string", required = true)
+        @Option(names = { "--connect-string" }, description = "Zookeeper or broker connect string", required = true)
         public String connectString = null;
 
-        @Parameter(names = { "--extensions" }, description = "true to monitor Load Balance Extensions.")
+        @Option(names = { "--extensions" }, description = "true to monitor Load Balance Extensions.")
         boolean extensions = false;
     }
 
@@ -546,14 +548,14 @@ private void startBrokerLoadDataStoreMonitor() {
      */
     public static void main(String[] args) throws Exception {
         final Arguments arguments = new Arguments();
-        final JCommander jc = new JCommander(arguments);
-        jc.setProgramName("pulsar-perf monitor-brokers");
+        final CommandLine commander = new CommandLine(arguments);
+        commander.setCommandName("pulsar-perf monitor-brokers");
 
         try {
-            jc.parse(args);
+            commander.parseArgs(args);
         } catch (ParameterException e) {
             System.out.println(e.getMessage());
-            jc.usage();
+            commander.usage(commander.getOut());
             PerfClientUtils.exit(1);
         }