Repository: skiptests/astraea
Pull request #34 (2021-09-29 12:03:28)

Use JCommander to analysis command line argument
使用JCommander來分析命令列參數。
統整所有命令列參數的錯誤輸出格式。
#30 
已經檢查參數合理性，應用程式可以不用檢查參數合理性。
未來新增工具時，可增加檔案，例如`toolName.java`
```java
package org.astraea.argument;
public class toolName{
  @Parameter(
      names = {"--newOption"},
      description = "String: something about this option",
      validateWith = ArgumentCheck.NotEmpty.class,
      required = true)
  public String optionName;
}
```
在`ArgumentCheck.java`的switch case中增加
```java
case "End2EndLatency":
  jc = builder.addObject(new toolName()).build();
  try {
    jc.parse(args.subList(1, args.size()).toArray(new String[0]));
  } catch (ParameterException pe) {
    jc.usage();
    throw pe; 
  }
  break;
```

wycccccc: 感謝貢獻，這隻patch看着不錯，review的同時讓我增加了點Jcommander的知識。
chia7712: @chinghongfang 要麻煩處理一下衝突喔
chinghongfang: 好的  馬上處理。

Patch:
diff --git a/app/build.gradle b/app/build.gradle
index 2a9bd8e8..efa86f96 100644
--- a/app/build.gradle
+++ b/app/build.gradle
@@ -19,12 +19,14 @@ repositories {
 def versions = [
     kafka: project.properties['kafka.version'] ?: "2.8.0",
     junit: project.properties['junit.version'] ?: "5.7.2",
+    jcommander: project.properties['jcommander.version'] ?: "1.81",
 ]
 
 dependencies {
     testImplementation "org.junit.jupiter:junit-jupiter:${versions["junit"]}"
 
     implementation "org.apache.kafka:kafka-clients:${versions["kafka"]}"
+    implementation "com.beust:jcommander:${versions["jcommander"]}"
 }
 
 application {
diff --git a/app/src/main/java/org/astraea/App.java b/app/src/main/java/org/astraea/App.java
index c0db58cf..37459985 100644
--- a/app/src/main/java/org/astraea/App.java
+++ b/app/src/main/java/org/astraea/App.java
@@ -4,6 +4,7 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
+import org.astraea.argument.ArgumentUtil;
 import org.astraea.metrics.kafka.KafkaMetricClientApp;
 import org.astraea.offset.OffsetExplorer;
 import org.astraea.performance.latency.End2EndLatency;
@@ -37,6 +38,8 @@ public class App {
                         new IllegalArgumentException(
                             "className: " + className + " is not matched to " + toString(mains)));
 
+    ArgumentUtil.checkArgument(targetClass, args.subList(1, args.size()));
+
     var method = targetClass.getDeclaredMethod("main", String[].class);
     method.invoke(null, (Object) args.subList(1, args.size()).toArray(String[]::new));
   }
diff --git a/app/src/main/java/org/astraea/argument/ArgumentUtil.java b/app/src/main/java/org/astraea/argument/ArgumentUtil.java
new file mode 100644
index 00000000..0ce911fc
--- /dev/null
+++ b/app/src/main/java/org/astraea/argument/ArgumentUtil.java
@@ -0,0 +1,94 @@
+package org.astraea.argument;
+
+import com.beust.jcommander.IParameterValidator;
+import com.beust.jcommander.IStringConverter;
+import com.beust.jcommander.JCommander;
+import com.beust.jcommander.ParameterException;
+import java.time.Duration;
+import java.util.List;
+import org.astraea.offset.OffsetExplorerArgument;
+import org.astraea.performance.latency.End2EndLatencyArgument;
+
+/*
+ * A tool used to parse command line arguments.
+ *
+ * To add new option, add in the corresponding file.
+ * @Parameter(names={"--option"}, description="")
+ * public <optionType> <optionName>;
+ *
+ * To add "NewTool"
+ * 1. Import the class of "NewTool" argument class.
+ * 2. add new case in this file.
+ * ```
+ * case "path.to.package.NewTool":
+ *   jc.addObject(new NewToolArgument()).build().parse(args.toArray(new String[0]));
+ * break;
+ * ```
+ * */
+
+public class ArgumentUtil {
+  // Do not instantiate.
+  private ArgumentUtil() {}
+
+  public static boolean checkArgument(Class<?> tool, List<String> args) {
+    JCommander.Builder builder = JCommander.newBuilder();
+    JCommander jc;
+    System.out.println(tool.getName());
+    switch (tool.getName()) {
+      case "org.astraea.performance.latency.End2EndLatency":
+        jc = builder.addObject(new End2EndLatencyArgument()).build();
+        try {
+          jc.parse(args.toArray(new String[0]));
+        } catch (ParameterException pe) {
+          jc.usage();
+          throw pe;
+        }
+        break;
+      case "org.astraea.offset.OffsetExplorer":
+        jc = builder.addObject(new OffsetExplorerArgument()).build();
+        try {
+          jc.parse(args.toArray(new String[0]));
+        } catch (ParameterException pe) {
+          jc.usage();
+          throw pe;
+        }
+        break;
+      default:
+        // Unknown tool.
+        return false;
+    }
+    return true;
+  }
+
+  /* Validate Classes */
+  public static class NotEmpty implements IParameterValidator {
+    @Override
+    public void validate(String name, String value) throws ParameterException {
+      if (value == null || value.equals(""))
+        throw new ParameterException(name + " should not be empty.");
+    }
+  }
+
+  public static class LongPositive implements IParameterValidator {
+    @Override
+    public void validate(String name, String value) throws ParameterException {
+      if (Long.parseLong(value) <= 0) throw new ParameterException(name + " should be positive.");
+    }
+  }
+
+  public static class LongNotNegative implements IParameterValidator {
+    @Override
+    public void validate(String name, String value) throws ParameterException {
+      if (Long.parseLong(value) < 0)
+        throw new ParameterException(name + " should not be negative.");
+    }
+  }
+
+  /* Converter classes */
+  public static class DurationConverter implements IStringConverter<Duration> {
+    @Override
+    public Duration convert(String value) {
+      return Duration.ofSeconds(Long.parseLong(value));
+    }
+  }
+}
diff --git a/app/src/main/java/org/astraea/offset/OffsetExplorerArgument.java b/app/src/main/java/org/astraea/offset/OffsetExplorerArgument.java
new file mode 100644
index 00000000..93699163
--- /dev/null
+++ b/app/src/main/java/org/astraea/offset/OffsetExplorerArgument.java
@@ -0,0 +1,20 @@
+package org.astraea.offset;
+
+import com.beust.jcommander.Parameter;
+import org.astraea.argument.ArgumentUtil;
+
+public class OffsetExplorerArgument {
+  @Parameter(
+      names = {"--bootstrap.servers"},
+      description = "String: server to connect to",
+      validateWith = ArgumentUtil.NotEmpty.class,
+      required = true)
+  public String brokers;
+
+  @Parameter(
+      names = {"--topic"},
+      description = "String: topic name",
+      validateWith = ArgumentUtil.NotEmpty.class,
+      required = true)
+  public String topic;
+}
diff --git a/app/src/main/java/org/astraea/performance/latency/End2EndLatencyArgument.java b/app/src/main/java/org/astraea/performance/latency/End2EndLatencyArgument.java
new file mode 100644
index 00000000..c199b0cc
--- /dev/null
+++ b/app/src/main/java/org/astraea/performance/latency/End2EndLatencyArgument.java
@@ -0,0 +1,53 @@
+package org.astraea.performance.latency;
+
+import com.beust.jcommander.Parameter;
+import java.time.Duration;
+import org.astraea.argument.ArgumentUtil;
+
+public class End2EndLatencyArgument {
+  @Parameter(
+      names = {"--bootstrap.servers"},
+      description = "String: server to connect to",
+      validateWith = ArgumentUtil.NotEmpty.class,
+      required = true)
+  public String brokers;
+
+  @Parameter(
+      names = {"--topic"},
+      description = "String: topic name",
+      validateWith = ArgumentUtil.NotEmpty.class,
+      required = true)
+  public String topic;
+
+  @Parameter(
+      names = {"--producers"},
+      description = "Integer: number of producers to create",
+      validateWith = ArgumentUtil.LongPositive.class)
+  public int producers = 1;
+
+  @Parameter(
+      names = {"--consumers"},
+      description = "Integer: number of consumers to create",
+      validateWith = ArgumentUtil.LongNotNegative.class)
+  public int consumers = 1;
+
+  @Parameter(
+      names = {"--duration"},
+      description = "Long: producing time in seconds",
+      validateWith = ArgumentUtil.LongPositive.class,
+      converter = ArgumentUtil.DurationConverter.class)
+  public Duration duration = Duration.ofSeconds(5);
+
+  @Parameter(
+      names = {"--valueSize"},
+      description = "Integer: bytes per record sent",
+      validateWith = ArgumentUtil.LongPositive.class)
+  public int valueSize = 100;
+
+  @Parameter(
+      names = {"--flushDuration"},
+      description = "Long: timeout for producer to flush the records",
+      validateWith = ArgumentUtil.LongPositive.class,
+      converter = ArgumentUtil.DurationConverter.class)
+  public Duration flushDuration = Duration.ofSeconds(2);
+}


Test patch:
diff --git a/app/src/test/java/org/astraea/argument/ArgumentUtilTest.java b/app/src/test/java/org/astraea/argument/ArgumentUtilTest.java
new file mode 100644
index 00000000..3c5d5a63
--- /dev/null
+++ b/app/src/test/java/org/astraea/argument/ArgumentUtilTest.java
@@ -0,0 +1,133 @@
+package org.astraea.argument;
+
+import com.beust.jcommander.ParameterException;
+import java.util.Arrays;
+import org.astraea.offset.OffsetExplorer;
+import org.astraea.performance.latency.End2EndLatency;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class ArgumentUtilTest {
+  @Test
+  public void testParse() {
+    Assertions.assertTrue(
+        ArgumentUtil.checkArgument(
+            End2EndLatency.class,
+            Arrays.asList(
+                "--bootstrap.servers",
+                "localhost:9092",
+                "--topic",
+                "testing",
+                "--producers",
+                "3",
+                "--consumers",
+                "4",
+                "--duration",
+                "5",
+                "--valueSize",
+                "6",
+                "--flushDuration",
+                "7")));
+  }
+
+  @Test
+  public void testArgumentLost() {
+    Assertions.assertEquals(
+        "--bootstrap.servers should not be empty.",
+        Assertions.assertThrows(
+                ParameterException.class,
+                () ->
+                    ArgumentUtil.checkArgument(
+                        OffsetExplorer.class,
+                        Arrays.asList("--bootstrap.servers", "", "--topic", "testing")))
+            .getMessage());
+    Assertions.assertEquals(
+        "--topic should not be empty.",
+        Assertions.assertThrows(
+                ParameterException.class,
+                () ->
+                    ArgumentUtil.checkArgument(
+                        OffsetExplorer.class,
+                        Arrays.asList("--bootstrap.servers", "localhost:9092", "--topic", "")))
+            .getMessage());
+  }
+
+  @Test
+  public void testInvalidArgument() {
+    Assertions.assertEquals(
+        "--producers should be positive.",
+        Assertions.assertThrows(
+                ParameterException.class,
+                () ->
+                    ArgumentUtil.checkArgument(
+                        End2EndLatency.class,
+                        Arrays.asList(
+                            "--bootstrap.servers",
+                            "localhost:9092",
+                            "--topic",
+                            "test",
+                            "--producers",
+                            "0")))
+            .getMessage());
+    Assertions.assertEquals(
+        "--consumers should not be negative.",
+        Assertions.assertThrows(
+                ParameterException.class,
+                () ->
+                    ArgumentUtil.checkArgument(
+                        End2EndLatency.class,
+                        Arrays.asList(
+                            "--bootstrap.servers",
+                            "localhost:9092",
+                            "--topic",
+                            "test",
+                            "--consumers",
+                            "-1")))
+            .getMessage());
+    Assertions.assertEquals(
+        "--duration should be positive.",
+        Assertions.assertThrows(
+                ParameterException.class,
+                () ->
+                    ArgumentUtil.checkArgument(
+                        End2EndLatency.class,
+                        Arrays.asList(
+                            "--bootstrap.servers",
+                            "localhost:9092",
+                            "--topic",
+                            "test",
+                            "--duration",
+                            "0")))
+            .getMessage());
+    Assertions.assertEquals(
+        "--valueSize should be positive.",
+        Assertions.assertThrows(
+                ParameterException.class,
+                () ->
+                    ArgumentUtil.checkArgument(
+                        End2EndLatency.class,
+                        Arrays.asList(
+                            "--bootstrap.servers",
+                            "localhost:9092",
+                            "--topic",
+                            "test",
+                            "--valueSize",
+                            "0")))
+            .getMessage());
+    Assertions.assertEquals(
+        "--flushDuration should be positive.",
+        Assertions.assertThrows(
+                ParameterException.class,
+                () ->
+                    ArgumentUtil.checkArgument(
+                        End2EndLatency.class,
+                        Arrays.asList(
+                            "--bootstrap.servers",
+                            "localhost:9092",
+                            "--topic",
+                            "test",
+                            "--flushDuration",
+                            "0")))
+            .getMessage());
+  }
+}