Introducing AbstractArgsParser which removes the boilerplate of printing help and exiting the process on cmd line errors. (#3040)

node/src/main/kotlin/net/corda/node/NodeArgsParser.kt

@@ -1,7 +1,7 @@
 package net.corda.node
 
 import com.typesafe.config.ConfigFactory
-import joptsimple.OptionParser
+import joptsimple.OptionSet
 import joptsimple.util.EnumConverter
 import joptsimple.util.PathConverter
 import net.corda.core.internal.div
@@ -9,21 +9,21 @@ import net.corda.core.internal.exists
 import net.corda.node.services.config.ConfigHelper
 import net.corda.node.services.config.NodeConfiguration
 import net.corda.node.services.config.parseAsNodeConfiguration
+import net.corda.node.utilities.AbstractArgsParser
 import net.corda.nodeapi.internal.config.UnknownConfigKeysPolicy
 import org.slf4j.event.Level
-import java.io.PrintStream
 import java.nio.file.Path
 import java.nio.file.Paths
 
 // NOTE: Do not use any logger in this class as args parsing is done before the logger is setup.
-class ArgsParser {
-    private val optionParser = OptionParser()
+class NodeArgsParser : AbstractArgsParser<CmdLineOptions>() {
     // The intent of allowing a command line configurable directory and config path is to allow deployment flexibility.
     // Other general configuration should live inside the config file unless we regularly need temporary overrides on the command line
     private val baseDirectoryArg = optionParser
             .accepts("base-directory", "The node working directory where all the files are kept")
             .withRequiredArg()
-            .defaultsTo(".")
+            .withValuesConvertedBy(PathConverter())
+            .defaultsTo(Paths.get("."))
     private val configFileArg = optionParser
             .accepts("config-file", "The path to the config file")
             .withRequiredArg()
@@ -43,7 +43,7 @@ class ArgsParser {
             .defaultsTo((Paths.get("certificates") / "network-root-truststore.jks"))
     private val networkRootTrustStorePasswordArg = optionParser.accepts("network-root-truststore-password", "Network root trust store password obtained from network operator.")
             .withRequiredArg()
-    private val unknownConfigKeysPolicy = optionParser.accepts("on-unknown-config-keys", "How to behave on unknown node configuration property keys: [WARN, FAIL, IGNORE].")
+    private val unknownConfigKeysPolicy = optionParser.accepts("on-unknown-config-keys", "How to behave on unknown node configuration.")
             .withRequiredArg()
             .withValuesConvertedBy(object : EnumConverter<UnknownConfigKeysPolicy>(UnknownConfigKeysPolicy::class.java) {})
             .defaultsTo(UnknownConfigKeysPolicy.FAIL)
@@ -52,16 +52,13 @@ class ArgsParser {
     private val justGenerateNodeInfoArg = optionParser.accepts("just-generate-node-info",
             "Perform the node start-up task necessary to generate its nodeInfo, save it to disk, then quit")
     private val bootstrapRaftClusterArg = optionParser.accepts("bootstrap-raft-cluster", "Bootstraps Raft cluster. The node forms a single node cluster (ignoring otherwise configured peer addresses), acting as a seed for other nodes to join the cluster.")
-    private val helpArg = optionParser.accepts("help").forHelp()
 
-    fun parse(vararg args: String): CmdLineOptions {
-        val optionSet = optionParser.parse(*args)
+    override fun doParse(optionSet: OptionSet): CmdLineOptions {
         require(!optionSet.has(baseDirectoryArg) || !optionSet.has(configFileArg)) {
             "${baseDirectoryArg.options()[0]} and ${configFileArg.options()[0]} cannot be specified together"
         }
-        val baseDirectory = Paths.get(optionSet.valueOf(baseDirectoryArg)).normalize().toAbsolutePath()
+        val baseDirectory = optionSet.valueOf(baseDirectoryArg).normalize().toAbsolutePath()
         val configFile = baseDirectory / optionSet.valueOf(configFileArg)
-        val help = optionSet.has(helpArg)
         val loggingLevel = optionSet.valueOf(loggerLevel)
         val logToConsole = optionSet.has(logToConsoleArg)
         val isRegistration = optionSet.has(isRegistrationArg)
@@ -84,7 +81,6 @@ class ArgsParser {
 
         return CmdLineOptions(baseDirectory,
                 configFile,
-                help,
                 loggingLevel,
                 logToConsole,
                 registrationConfig,
@@ -95,15 +91,12 @@ class ArgsParser {
                 bootstrapRaftCluster,
                 unknownConfigKeysPolicy)
     }
-
-    fun printHelp(sink: PrintStream) = optionParser.printHelpOn(sink)
 }
 
 data class NodeRegistrationOption(val networkRootTrustStorePath: Path, val networkRootTrustStorePassword: String)
 
 data class CmdLineOptions(val baseDirectory: Path,
                           val configFile: Path,
-                          val help: Boolean,
                           val loggingLevel: Level,
                           val logToConsole: Boolean,
                           val nodeRegistrationOption: NodeRegistrationOption?,

node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt

@@ -1,7 +1,6 @@
 package net.corda.node.internal
 
 import com.jcabi.manifests.Manifests
-import joptsimple.OptionException
 import net.corda.core.internal.Emoji
 import net.corda.core.internal.concurrent.thenMatch
 import net.corda.core.internal.createDirectories
@@ -29,14 +28,13 @@ import java.net.InetAddress
 import java.nio.file.Path
 import java.nio.file.Paths
 import java.util.*
-import kotlin.system.exitProcess
 
 /** This class is responsible for starting a Node from command line arguments. */
 open class NodeStartup(val args: Array<String>) {
     companion object {
         private val logger by lazy { loggerFor<Node>() } // I guess this is lazy to allow for logging init, but why Node?
-        val LOGS_DIRECTORY_NAME = "logs"
-        val LOGS_CAN_BE_FOUND_IN_STRING = "Logs can be found in"
+        const val LOGS_DIRECTORY_NAME = "logs"
+        const val LOGS_CAN_BE_FOUND_IN_STRING = "Logs can be found in"
     }
 
     /**
@@ -49,7 +47,7 @@ open class NodeStartup(val args: Array<String>) {
             println("Corda will now exit...")
             return false
         }
-        val (argsParser, cmdlineOptions) = parseArguments()
+        val cmdlineOptions = NodeArgsParser().parseOrExit(*args)
 
         // We do the single node check before we initialise logging so that in case of a double-node start it
         // doesn't mess with the running node's logs.
@@ -66,12 +64,6 @@ open class NodeStartup(val args: Array<String>) {
             return true
         }
 
-        // Maybe render command line help.
-        if (cmdlineOptions.help) {
-            argsParser.printHelp(System.out)
-            return true
-        }
-
         drawBanner(versionInfo)
         Node.printBasicNodeInfo(LOGS_CAN_BE_FOUND_IN_STRING, System.getProperty("log-path"))
         val conf = try {
@@ -246,18 +238,6 @@ open class NodeStartup(val args: Array<String>) {
         pidFileRw.write(ourProcessID.toByteArray())
     }
 
-    private fun parseArguments(): Pair<ArgsParser, CmdLineOptions> {
-        val argsParser = ArgsParser()
-        val cmdlineOptions = try {
-            argsParser.parse(*args)
-        } catch (ex: OptionException) {
-            println("Invalid command line arguments: ${ex.message}")
-            argsParser.printHelp(System.out)
-            exitProcess(1)
-        }
-        return Pair(argsParser, cmdlineOptions)
-    }
-
     open protected fun initLogging(cmdlineOptions: CmdLineOptions) {
         val loggingLevel = cmdlineOptions.loggingLevel.name.toLowerCase(Locale.ENGLISH)
         System.setProperty("defaultLogLevel", loggingLevel) // These properties are referenced from the XML config file.

node/src/main/kotlin/net/corda/node/utilities/AbstractArgsParser.kt

@@ -0,0 +1,34 @@
+package net.corda.node.utilities
+
+import joptsimple.OptionException
+import joptsimple.OptionParser
+import joptsimple.OptionSet
+import kotlin.system.exitProcess
+
+abstract class AbstractArgsParser<out T : Any> {
+    protected val optionParser = OptionParser()
+    private val helpOption = optionParser.acceptsAll(listOf("h", "help"), "show help").forHelp()
+
+    /**
+     * Parses the given [args] or exits the process if unable to, printing the help output to stderr.
+     * If the help option is specified then the process is also shutdown after printing the help output to stdout.
+     */
+    fun parseOrExit(vararg args: String): T {
+        val optionSet = try {
+            optionParser.parse(*args)
+        } catch (e: OptionException) {
+            System.err.println(e.message ?: "Unable to parse arguments.")
+            optionParser.printHelpOn(System.err)
+            exitProcess(1)
+        }
+        if (optionSet.has(helpOption)) {
+            optionParser.printHelpOn(System.out)
+            exitProcess(0)
+        }
+        return doParse(optionSet)
+    }
+
+    fun parse(vararg args: String): T = doParse(optionParser.parse(*args))
+
+    protected abstract fun doParse(optionSet: OptionSet): T
+}

node/src/test/kotlin/net/corda/node/NodeArgsParserTest.kt

@@ -15,8 +15,8 @@ import java.nio.file.Paths
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
 
-class ArgsParserTest {
-    private val parser = ArgsParser()
+class NodeArgsParserTest {
+    private val parser = NodeArgsParser()
 
     companion object {
         private lateinit var workingDirectory: Path
@@ -35,7 +35,6 @@ class ArgsParserTest {
         assertThat(parser.parse()).isEqualTo(CmdLineOptions(
                 baseDirectory = workingDirectory,
                 configFile = workingDirectory / "node.conf",
-                help = false,
                 logToConsole = false,
                 loggingLevel = Level.INFO,
                 nodeRegistrationOption = null,
@@ -166,7 +165,6 @@ class ArgsParserTest {
 
     @Test
     fun `on-unknown-config-keys options`() {
-
         UnknownConfigKeysPolicy.values().forEach { onUnknownConfigKeyPolicy ->
             val cmdLineOptions = parser.parse("--on-unknown-config-keys", onUnknownConfigKeyPolicy.name)
             assertThat(cmdLineOptions.unknownConfigKeysPolicy).isEqualTo(onUnknownConfigKeyPolicy)