fix: better command line boolean parsing (#1184)

aws-greengrass · Mar 28, 2022 · bb9f055 · bb9f055
1 parent f9b5a0d
commit bb9f055
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 7 deletions.
diff --git a/src/main/java/com/aws/greengrass/easysetup/GreengrassSetup.java b/src/main/java/com/aws/greengrass/easysetup/GreengrassSetup.java
@@ -426,15 +426,15 @@ void parseArgs() {
                     break;
                 case PROVISION_THING_ARG:
                 case PROVISION_THING_ARG_SHORT:
-                    this.needProvisioning = Coerce.toBoolean(getArg());
+                    this.needProvisioning = parseBooleanArg();
                     break;
                 case SETUP_SYSTEM_SERVICE_ARG:
                 case SETUP_SYSTEM_SERVICE_ARG_SHORT:
-                    this.setupSystemService = Coerce.toBoolean(getArg());
+                    this.setupSystemService = parseBooleanArg();
                     break;
                 case KERNEL_START_ARG:
                 case KERNEL_START_ARG_SHORT:
-                    this.kernelStart = Coerce.toBoolean(getArg());
+                    this.kernelStart = parseBooleanArg();
                     break;
                 case DEFAULT_USER_ARG:
                 case DEFAULT_USER_ARG_SHORT:
@@ -448,7 +448,7 @@ void parseArgs() {
                     break;
                 case DEPLOY_DEV_TOOLS_ARG:
                 case DEPLOY_DEV_TOOLS_ARG_SHORT:
-                    this.deployDevTools = Coerce.toBoolean(getArg());
+                    this.deployDevTools = parseBooleanArg();
                     break;
                 case TRUSTED_PLUGIN_ARG:
                 case TRUSTED_PLUGIN_ARG_SHORT:
@@ -468,6 +468,16 @@ void parseArgs() {
         }
     }
 
+    private boolean parseBooleanArg() {
+        String peeked = peekArg();
+        if (peeked == null || peeked.startsWith("-")) {
+            // default is true when an option is supplied with nothing after. ex: --deploy-dev-tools
+            // default is true when an option is supplied with another option after. ex: --deploy-dev-tools --provision
+            return true;
+        }
+        return Coerce.toBoolean(getArg());
+    }
+
     private void validatePluginJarPath(String pluginJarPath) {
         String nm = Utils.namePart(pluginJarPath);
         if (!nm.endsWith(EZPlugins.JAR_FILE_EXTENSION)) {
@@ -481,9 +491,15 @@ private void validatePluginJarPath(String pluginJarPath) {
         // which will be thrown as RuntimeException when copying the plugin jar.
     }
 
-    @SuppressWarnings("PMD.NullAssignment")
     private String getArg() {
-        return arg = setupArgs == null || argpos >= setupArgs.length ? null : setupArgs[argpos++];
+        String peek = peekArg();
+        argpos++;
+        return peek;
+    }
+
+    @SuppressWarnings("PMD.NullAssignment")
+    private String peekArg() {
+        return arg = setupArgs == null || argpos >= setupArgs.length ? null : setupArgs[argpos];
     }
 
     void provision(Kernel kernel) throws IOException, DeviceConfigurationException {

diff --git a/src/test/java/com/aws/greengrass/easysetup/GreengrassSetupTest.java b/src/test/java/com/aws/greengrass/easysetup/GreengrassSetupTest.java
@@ -98,7 +98,7 @@ void GIVEN_setup_script_WHEN_script_is_used_THEN_setup_actions_are_performed() t
                         "mock_config_path", "--root", "mock_root", "--thing-name", "mock_thing_name",
                         "--thing-group-name", "mock_thing_group_name", "--thing-policy-name", "mock_thing_policy_name",
                         "--tes-role-name", "mock_tes_role_name", "--tes-role-alias-name", "mock_tes_role_alias_name",
-                        "--provision", "y", "--aws-region","us-east-1", "-ss", "false");
+                        "--provision", "--aws-region","us-east-1", "-ss", "false");
         greengrassSetup.parseArgs();
         greengrassSetup.setDeviceProvisioningHelper(deviceProvisioningHelper);
         greengrassSetup.provision(kernel);