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

distribution/server/src/assemble/LICENSE.bin.txt

@@ -244,6 +244,9 @@ This projects includes binary packages with the following licenses:
 
 The Apache Software License, Version 2.0
  * JCommander -- com.beust-jcommander-1.82.jar
+ * Picocli
+     - info.picocli-picocli-4.7.5.jar
+     - info.picocli-picocli-shell-jline3-4.7.5.jar
  * High Performance Primitive Collections for Java -- com.carrotsearch-hppc-0.9.1.jar
  * Jackson
      - com.fasterxml.jackson.core-jackson-annotations-2.14.2.jar

distribution/shell/src/assemble/LICENSE.bin.txt

@@ -310,6 +310,9 @@ This projects includes binary packages with the following licenses:
 
 The Apache Software License, Version 2.0
  * JCommander -- jcommander-1.82.jar
+ * Picocli
+     - picocli-4.7.5.jar
+     - picocli-shell-jline3-4.7.5.jar
  * Jackson
      - jackson-annotations-2.14.2.jar
      - jackson-core-2.14.2.jar

distribution/shell/src/assemble/NOTICE.bin.txt

@@ -17,6 +17,9 @@ Copyright (c) 2005 Brian Goetz and Tim Peierls
 JCommander
 Copyright 2010 Cedric Beust cedric@beust.com
 
+picocli (http://picocli.info)
+Copyright 2017 Remko Popma
+
 EA Agent Loader
 Copyright (C) 2015 Electronic Arts Inc.  All rights reserved.
 

pom.xml

@@ -259,6 +259,7 @@ flexible messaging model and an intuitive client API.</description>
     <opentelemetry.alpha.version>1.34.1-alpha</opentelemetry.alpha.version>
     <opentelemetry.instrumentation.version>1.32.1-alpha</opentelemetry.instrumentation.version>
     <opentelemetry.semconv.version>1.23.1-alpha</opentelemetry.semconv.version>
+    <picocli.version>4.7.5</picocli.version>
 
     <!-- test dependencies -->
     <testcontainers.version>1.18.3</testcontainers.version>
@@ -691,6 +692,18 @@ flexible messaging model and an intuitive client API.</description>
         <version>${jcommander.version}</version>
       </dependency>
 
+      <dependency>
+        <groupId>info.picocli</groupId>
+        <artifactId>picocli</artifactId>
+        <version>${picocli.version}</version>
+      </dependency>
+
+      <dependency>
+        <groupId>info.picocli</groupId>
+        <artifactId>picocli-shell-jline3</artifactId>
+        <version>${picocli.version}</version>
+      </dependency>
+
       <dependency>
         <groupId>com.google.guava</groupId>
         <artifactId>guava</artifactId>

pulsar-cli-utils/pom.xml

@@ -40,7 +40,11 @@
       <artifactId>jcommander</artifactId>
       <scope>compile</scope>
     </dependency>
-
+    <dependency>
+      <groupId>info.picocli</groupId>
+      <artifactId>picocli</artifactId>
+      <scope>compile</scope>
+    </dependency>
     <dependency>
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-lang3</artifactId>

pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/ValueValidationUtil.java

@@ -18,7 +18,6 @@
  */
 package org.apache.pulsar.cli;
 
-import com.beust.jcommander.ParameterException;
 import lombok.experimental.UtilityClass;
 import org.apache.commons.lang3.StringUtils;
 
@@ -27,31 +26,31 @@ public class ValueValidationUtil {
 
     public static void maxValueCheck(String paramName, long value, long maxValue) {
         if (value > maxValue) {
-            throw new ParameterException(paramName + " cannot be bigger than <" + maxValue + ">!");
+            throw new IllegalArgumentException(paramName + " cannot be bigger than <" + maxValue + ">!");
         }
     }
 
     public static void positiveCheck(String paramName, long value) {
         if (value <= 0) {
-            throw new ParameterException(paramName + " cannot be less than or equal to <0>!");
+            throw new IllegalArgumentException(paramName + " cannot be less than or equal to <0>!");
         }
     }
 
     public static void positiveCheck(String paramName, int value) {
         if (value <= 0) {
-            throw new ParameterException(paramName + " cannot be less than or equal to <0>!");
+            throw new IllegalArgumentException(paramName + " cannot be less than or equal to <0>!");
         }
     }
 
     public static void emptyCheck(String paramName, String value) {
         if (StringUtils.isEmpty(value)) {
-            throw new ParameterException("The value of " + paramName + " can't be empty");
+            throw new IllegalArgumentException("The value of " + paramName + " can't be empty");
         }
     }
 
     public static void minValueCheck(String name, Long value, long min) {
         if (value < min) {
-            throw new ParameterException(name + " cannot be less than <" + min + ">!");
+            throw new IllegalArgumentException(name + " cannot be less than <" + min + ">!");
         }
     }
 }

pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/ByteUnitUtil.java

@@ -18,20 +18,19 @@
  */
 package org.apache.pulsar.cli.converters;
 
-import com.beust.jcommander.ParameterException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 import lombok.experimental.UtilityClass;
 
 @UtilityClass
-class ByteUnitUtil {
+public class ByteUnitUtil {
 
     private static Set<Character> sizeUnit = Collections.unmodifiableSet(
             new HashSet<>(Arrays.asList('k', 'K', 'm', 'M', 'g', 'G', 't', 'T')));
 
-    static long validateSizeString(String byteStr) {
+    public static long validateSizeString(String byteStr) {
         if (byteStr.isEmpty()) {
             throw new IllegalArgumentException("byte string cannot be empty");
         }
@@ -44,7 +43,7 @@ static long validateSizeString(String byteStr) {
                     ? Long.parseLong(subStr)
                     : Long.parseLong(byteStr);
         } catch (IllegalArgumentException e) {
-            throw new ParameterException(String.format("Invalid size '%s'. Valid formats are: %s",
+            throw new IllegalArgumentException(String.format("Invalid size '%s'. Valid formats are: %s",
                     byteStr, "(4096, 100K, 10M, 16G, 2T)"));
         }
         switch (last) {

pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/ByteUnitIntegerConverter.java → pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/picocli/ByteUnitToIntegerConverter.java

@@ -16,26 +16,20 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pulsar.cli.converters;
+package org.apache.pulsar.cli.converters.picocli;
 
-import static org.apache.pulsar.cli.ValueValidationUtil.emptyCheck;
 import static org.apache.pulsar.cli.converters.ByteUnitUtil.validateSizeString;
-import com.beust.jcommander.converters.BaseConverter;
-
-public class ByteUnitIntegerConverter extends BaseConverter<Integer> {
-
-    public ByteUnitIntegerConverter(String optionName) {
-        super(optionName);
-    }
+import picocli.CommandLine.ITypeConverter;
+import picocli.CommandLine.TypeConversionException;
 
+public class ByteUnitToIntegerConverter implements ITypeConverter<Integer> {
     @Override
-    public Integer convert(String argStr) {
-        return parseBytes(argStr).intValue();
-    }
-
-    Long parseBytes(String argStr) {
-        emptyCheck(getOptionName(), argStr);
-        long valueInBytes = validateSizeString(argStr);
-        return valueInBytes;
+    public Integer convert(String value) throws Exception {
+        try {
+            long l = validateSizeString(value);
+            return (int) l;
+        } catch (Exception e) {
+            throw new TypeConversionException(e.getMessage());
+        }
     }
 }

pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/picocli/ByteUnitToLongConverter.java

@@ -0,0 +1,34 @@
+/*
+ * 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.pulsar.cli.converters.picocli;
+
+import static org.apache.pulsar.cli.converters.ByteUnitUtil.validateSizeString;
+import picocli.CommandLine.ITypeConverter;
+import picocli.CommandLine.TypeConversionException;
+
+public class ByteUnitToLongConverter implements ITypeConverter<Long> {
+    @Override
+    public Long convert(String value) throws Exception {
+        try {
+            return validateSizeString(value);
+        } catch (Exception e) {
+            throw new TypeConversionException(e.getMessage());
+        }
+    }
+}

pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/TimeUnitToMillisConverter.java → pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/picocli/TimeUnitToMillisConverter.java

@@ -16,27 +16,20 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pulsar.cli.converters;
+package org.apache.pulsar.cli.converters.picocli;
 
-import static org.apache.pulsar.cli.ValueValidationUtil.emptyCheck;
-import com.beust.jcommander.ParameterException;
-import com.beust.jcommander.converters.BaseConverter;
 import java.util.concurrent.TimeUnit;
+import org.apache.pulsar.cli.converters.RelativeTimeUtil;
+import picocli.CommandLine.ITypeConverter;
+import picocli.CommandLine.TypeConversionException;
 
-public class TimeUnitToMillisConverter extends BaseConverter<Long> {
-
-    public TimeUnitToMillisConverter(String optionName) {
-        super(optionName);
-    }
-
+public class TimeUnitToMillisConverter implements ITypeConverter<Long> {
     @Override
-    public Long convert(String str) {
-        emptyCheck(getOptionName(), str);
+    public Long convert(String value) throws Exception {
         try {
-            return TimeUnit.SECONDS.toMillis(
-                    RelativeTimeUtil.parseRelativeTimeInSeconds(str.trim()));
-        } catch (IllegalArgumentException exception) {
-            throw new ParameterException("For input " + getOptionName() + ": " + exception.getMessage());
+            return TimeUnit.SECONDS.toMillis(RelativeTimeUtil.parseRelativeTimeInSeconds(value));
+        } catch (Exception e) {
+            throw new TypeConversionException(e.getMessage());
         }
     }
 }

pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/picocli/TimeUnitToSecondsConverter.java

@@ -0,0 +1,35 @@
+/*
+ * 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.pulsar.cli.converters.picocli;
+
+import java.util.concurrent.TimeUnit;
+import org.apache.pulsar.cli.converters.RelativeTimeUtil;
+import picocli.CommandLine.ITypeConverter;
+import picocli.CommandLine.TypeConversionException;
+
+public class TimeUnitToSecondsConverter implements ITypeConverter<Long> {
+    @Override
+    public Long convert(String value) throws Exception {
+        try {
+            return TimeUnit.SECONDS.toSeconds(RelativeTimeUtil.parseRelativeTimeInSeconds(value));
+        } catch (Exception e) {
+            throw new TypeConversionException(e.getMessage());
+        }
+    }
+}

pulsar-cli-utils/src/main/java/org/apache/pulsar/cli/converters/picocli/package-info.java

@@ -0,0 +1,19 @@
+/*
+ * 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.pulsar.cli.converters.picocli;

pulsar-cli-utils/src/test/java/org/apache/pulsar/cli/ValueValidationUtilTest.java

@@ -19,49 +19,48 @@
 package org.apache.pulsar.cli;
 
 import static org.testng.Assert.assertThrows;
-import com.beust.jcommander.ParameterException;
 import org.testng.annotations.Test;
 
 public class ValueValidationUtilTest {
 
     @Test
     public void testMaxValueCheck() {
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.maxValueCheck("param1", 11L, 10L));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.maxValueCheck("param1", 11L, 10L));
         ValueValidationUtil.maxValueCheck("param2", 10L, 10L);
         ValueValidationUtil.maxValueCheck("param3", 9L, 10L);
     }
 
     @Test
     public void testPositiveCheck() {
         // Long
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.positiveCheck("param1", 0L));
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.positiveCheck("param2", -1L));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.positiveCheck("param1", 0L));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.positiveCheck("param2", -1L));
         ValueValidationUtil.positiveCheck("param3", 1L);
 
         // Integer
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.positiveCheck("param4", 0));
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.positiveCheck("param5", -1));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.positiveCheck("param4", 0));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.positiveCheck("param5", -1));
         ValueValidationUtil.positiveCheck("param6", 1);
     }
 
     @Test
     public void testEmptyCheck() {
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.emptyCheck("param1", ""));
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.emptyCheck("param2", null));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.emptyCheck("param1", ""));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.emptyCheck("param2", null));
         ValueValidationUtil.emptyCheck("param3", "nonEmpty");
     }
 
     @Test
     public void testMinValueCheck() {
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.minValueCheck("param1", 9L, 10L));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.minValueCheck("param1", 9L, 10L));
         ValueValidationUtil.minValueCheck("param2", 10L, 10L);
         ValueValidationUtil.minValueCheck("param3", 11L, 10L);
     }
 
     @Test
     public void testPositiveCheckInt() {
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.positiveCheck("param1", 0));
-        assertThrows(ParameterException.class, () -> ValueValidationUtil.positiveCheck("param2", -1));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.positiveCheck("param1", 0));
+        assertThrows(IllegalArgumentException.class, () -> ValueValidationUtil.positiveCheck("param2", -1));
         ValueValidationUtil.positiveCheck("param3", 1);
     }
 }