Merge pull request #445 from cherylking/fixContainerRunOptsHandling

Handle splitting of containerRunOpts correctly

src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java

@@ -1514,7 +1514,34 @@ private boolean alertOnServerError(String line, String errorCode, String gradleM
         return false;
     }
 
-    private String[] getCommandTokens(String command) {
+    // Add the passed runOpts to the commandElements as separate options with the option and value specified individually.
+    //
+    // Note: If an option's value in runOpts contains spaces and needs quoting/escaping, the getCommandTokens method won't handle it correctly.
+    // Two ways to approach this:
+    //  1) parse the opts into separate options ('-xyz abc' or '--xyz abc') and then break the option apart from its value at the first space
+    //  2) use the returned String[] from getCommandTokens and detect incorrectly broken up option values and splice them back together 
+    //
+    // Going with option 2.
+    protected void addContainerRunOpts(String runOpts, List<String> commandElements) {
+        String[] opts = getCommandTokens(runOpts);
+        int index = 0;
+        while (index < opts.length) {
+            if ((index == 0) || opts[index].startsWith("-")) {
+                commandElements.add(opts[index]);
+            } else {
+                String lastListElement = commandElements.get(commandElements.size()-1);
+                if (!lastListElement.startsWith("-")) {
+                    // add this to previous element
+                    commandElements.set(commandElements.size()-1, lastListElement+" "+opts[index]);
+                } else {
+                    commandElements.add(opts[index]);
+                }
+            }
+            index++;
+        }
+    }
+
+    protected String[] getCommandTokens(String command) {
         StringTokenizer stringTokenizer = new StringTokenizer(command);
         String[] commandTokens = new String[stringTokenizer.countTokens()];
         for (int i = 0; stringTokenizer.hasMoreTokens(); i++) {
@@ -1678,12 +1705,7 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept
 
         // Allow the user to add their own options to this command via a system property.
         if (containerRunOpts != null) {
-            // If something in containerRunOpts contains spaces and needs quoting/escaping, this getCommandTokens method won't handle it correctly.
-            // But what was here before would not either.
-            String[] runOpts = getCommandTokens(containerRunOpts);
-            for (int i=0; i < runOpts.length; i++) {
-                commandElements.add(runOpts[i]);
-            }
+            addContainerRunOpts(containerRunOpts, commandElements);
         }
 
         // Options must precede this in any order. Image name and command code follows.

src/test/java/io/openliberty/tools/common/plugins/util/DevUtilTest.java

@@ -34,6 +34,7 @@
 import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 
 import javax.xml.stream.FactoryConfigurationError;
@@ -47,6 +48,11 @@
 
 public class DevUtilTest extends BaseDevUtilTest {
 
+    public static final String RUN_OPTS_SINGLE = "-e MY_OPT=value";
+    public static final String RUN_OPTS_SINGLE_WITH_SPACE = "-e MY_OPT=value with space";
+    public static final String RUN_OPTS_DOUBLE = "-e MY_OPT=value -e MY_SECOND_OPT=value2";
+    public static final String RUN_OPTS_DOUBLE_WITH_EXTRA_SPACES = "  -e  MY_OPT=value with space   -e   MY_SECOND_OPT=value 2";
+
     File serverDirectory;
     File configDirectory;
     File srcDir;
@@ -132,6 +138,45 @@ public void testCleanupServerEnvBak() throws Exception {
         assertFalse(serverEnvBak.exists());
     }
 
+    @Test
+    public void testContainerRunOpts() throws Exception {
+        List<String> options = new ArrayList<String>();
+        String[] opts = util.getCommandTokens(RUN_OPTS_SINGLE);
+        assertTrue("RUN_OPTS_SINGLE incorrectly split into "+opts.length+" parameters. Expected 2 parameters.", opts.length == 2);
+        util.addContainerRunOpts(RUN_OPTS_SINGLE, options);
+        assertTrue("RUN_OPTS_SINGLE incorrectly split into "+options.size()+" parameters. Expected 2 parameters.", options.size() == 2);
+
+        options.clear();
+        opts = util.getCommandTokens(RUN_OPTS_DOUBLE);
+        assertTrue("RUN_OPTS_DOUBLE incorrectly split into "+opts.length+" parameters. Expected 4 parameters.", opts.length == 4);
+        util.addContainerRunOpts(RUN_OPTS_DOUBLE, options);
+        assertTrue("RUN_OPTS_DOUBLE incorrectly split into "+options.size()+" parameters. Expected 4 parameters.", options.size() == 4);
+
+        options.clear();
+        opts = util.getCommandTokens(RUN_OPTS_SINGLE_WITH_SPACE);
+        // note that we really want to see this split into two parameters instead of the four that getCommandTokens returns
+        assertTrue("RUN_OPTS_SINGLE_WITH_SPACE incorrectly split into "+opts.length+" parameters. Expected 4 parameters.", opts.length == 4);
+        util.addContainerRunOpts(RUN_OPTS_SINGLE_WITH_SPACE, options);
+        assertTrue("RUN_OPTS_SINGLE_WITH_SPACE incorrectly split into "+options.size()+" parameters. Expected 2 parameters.", options.size() == 2);
+
+        options.clear();
+        opts = util.getCommandTokens(RUN_OPTS_DOUBLE_WITH_EXTRA_SPACES);
+        // note that we really want to see this split into four parameters instead of the seven that getCommandTokens returns
+        assertTrue("RUN_OPTS_DOUBLE_WITH_SPACE incorrectly split into "+opts.length+" parameters. Expected 7 parameters.", opts.length == 7);
+        util.addContainerRunOpts(RUN_OPTS_DOUBLE_WITH_EXTRA_SPACES, options);
+        assertTrue("RUN_OPTS_DOUBLE_WITH_SPACE incorrectly split into "+options.size()+" parameters. Expected 4 parameters.", options.size() == 4);
+
+        // RUN_OPTS_DOUBLE_WITH_SPACE = "-e MY_OPT=value with space -e MY_SECOND_OPT=value 2";
+        List<String> expectedResult = new ArrayList<String>();
+        expectedResult.add("-e");
+        expectedResult.add("MY_OPT=value with space");
+        expectedResult.add("-e");
+        expectedResult.add("MY_SECOND_OPT=value 2");
+
+        assertTrue("List does not contain expected values.", expectedResult.equals(options));
+
+    }
+
     @Test
     public void testFindAvailablePort() throws Exception {
         // prefer a port that is known to be available