Fixing up confusion over OPTIONS vs OPTION

jetty-start/src/main/java/org/eclipse/jetty/start/Main.java

@@ -52,6 +52,8 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.regex.Pattern;
 
+import org.eclipse.jetty.start.StartIni.IncludeListener;
+
 /*-------------------------------------------*/
 /**
  * <p>
@@ -63,7 +65,7 @@ import java.util.regex.Pattern;
  * The behaviour of Main is controlled by the parsing of the {@link Config} "org/eclipse/start/start.config" file obtained as a resource or file.
  * </p>
  */
-public class Main
+public class Main implements IncludeListener
 {
     private static final String START_LOG_FILENAME = "start.log";
     private static final SimpleDateFormat START_LOG_ROLLOVER_DATEFORMAT = new SimpleDateFormat("yyyy_MM_dd-HHmmSSSSS.'" + START_LOG_FILENAME + "'");
@@ -212,20 +214,7 @@ public class Main
                 {
                     String name = arg.substring(6);
                     File file = _config.getHomeBase().getFile(name);
-                    StartIni startini = new StartIni(file);
+                    StartIni startini = new StartIni(file,this);
-                    int idx;
-                    while ((idx = startini.lineIndexOf(0,Pattern.compile("^.*/$"))) >= 0)
-                    {
-                        String subPath = startini.getLines().get(idx);
-                        startini.getLines().remove(idx);
-                        // find the sub ini files (based on HomeBase)
-                        List<File> childInis = _config.getHomeBase().listFiles(subPath,new FS.IniFilter());
-                        for (File childIni : childInis)
-                        {
-                            StartIni cini = new StartIni(childIni);
-                            idx += startini.overlayAt(idx,cini);
-                        }
-                    }
                     arguments.addAll(i + 1,startini.getLines());
                 }
 
@@ -380,6 +369,17 @@ public class Main
 
         return xmls;
     }
+    
+    @Override
+    public List<StartIni> onIniInclude(String path) throws IOException
+    {
+        List<StartIni> included = new ArrayList<>();
+        for(File file: _config.getHomeBase().listFiles(path,new FS.IniFilter()))
+        {
+            included.add(new StartIni(file));
+        }
+        return included;
+    }
 
     private void download(String arg)
     {

jetty-start/src/main/java/org/eclipse/jetty/start/StartIni.java

@@ -34,10 +34,20 @@ import java.util.regex.Pattern;
  */
 public class StartIni implements Iterable<String>
 {
+    public static interface IncludeListener
+    {
+        public List<StartIni> onIniInclude(String path) throws IOException;
+    }
+
     private final File file;
     private final LinkedList<String> lines;
 
     public StartIni(File file) throws FileNotFoundException, IOException
+    {
+        this(file,null);
+    }
+
+    public StartIni(File file, IncludeListener listener) throws FileNotFoundException, IOException
     {
         this.file = file;
         this.lines = new LinkedList<>();
@@ -54,48 +64,68 @@ public class StartIni implements Iterable<String>
                         // skip (empty line)
                         continue;
                     }
+
                     if (line.charAt(0) == '#')
                     {
                         // skip (comment)
                         continue;
                     }
 
-                    // Smart Handling, split into multiple OPTIONS lines
+                    // Smart Handling, split into multiple OPTIONS lines (for dup check reasons)
                     if (line.startsWith("OPTIONS="))
                     {
-                        for (String part : line.split(","))
+                        int idx = line.indexOf('=');
+                        String value = line.substring(idx + 1);
+                        for (String part : value.split(","))
                         {
-                            lines.add("OPTIONS=" + part);
+                            addUniqueLine("OPTION=" + part);
+                        }
+                    }
+                    // Smart Handling, includes
+                    else if (line.endsWith("/"))
+                    {
+                        if (listener == null)
+                        {
+                            System.err.printf("Nested includes not supported: %s (found in %s)%n",line,file.getAbsolutePath());
+                        }
+                        else
+                        {
+                            // Collect HomeBase resolved included StartIni's
+                            for (StartIni included : listener.onIniInclude(line))
+                            {
+                                // Merge each line with prior lines to prevent duplicates
+                                for (String includedLine : included)
+                                {
+                                    addUniqueLine(includedLine);
+                                }
+                            }
                         }
                     }
                     else
                     {
                         // Add line as-is
-                        lines.add(line);
+                        addUniqueLine(line);
                     }
                 }
             }
         }
     }
 
+    private void addUniqueLine(String line)
+    {
+        if (lines.contains(line))
+        {
+            // skip
+            return;
+        }
+        lines.add(line);
+    }
+
     public File getFile()
     {
         return file;
     }
 
-    public int lineIndexOf(int offset, Pattern pattern)
-    {
-        int len = lines.size();
-        for (int i = offset; i < len; i++)
-        {
-            if (pattern.matcher(lines.get(i)).matches())
-            {
-                return i;
-            }
-        }
-        return -1;
-    }
-
     public List<String> getLineMatches(Pattern pattern)
     {
         List<String> ret = new ArrayList<>();
@@ -119,31 +149,4 @@ public class StartIni implements Iterable<String>
     {
         return lines.iterator();
     }
-
-    public int overlayAt(int index, StartIni child)
-    {
-        int idx = index;
-        int count = 0;
-        for (String line : child)
-        {
-            if (this.hasLine(line))
-            {
-                // skip
-                continue;
-            }
-            lines.add(idx++,line);
-            count++;
-        }
-        return count;
-    }
-
-    private boolean hasLine(String line)
-    {
-        return lines.contains(line);
-    }
-
-    public void removeLine(String line)
-    {
-        lines.remove(line);
-    }
 }

jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java

@@ -25,13 +25,12 @@ import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.Vector;
 
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
-import org.hamcrest.Matchers;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -56,7 +55,8 @@ public class MainTest
     {
         Main main = new Main();
         List<String> xmls = main.processCommandLine(new String[] {});
-        
+
+        // Order is important here
         List<String> expectedXmls = new ArrayList<String>();
         expectedXmls.add("etc/jetty.xml"); // from start.ini
         expectedXmls.add("etc/jetty-jmx.xml"); // from start.d/10-jmx.ini
@@ -65,11 +65,26 @@ public class MainTest
         expectedXmls.add("etc/jetty-deploy.xml"); // from start.ini
         expectedXmls.add("etc/jetty-webapps.xml"); // from start.ini
         expectedXmls.add("etc/jetty-contexts.xml"); // from start.ini
-        
-        Assert.assertThat("XML Resolution Order "+xmls, xmls, contains(expectedXmls.toArray()));
 
+        assertThat("XML Resolution Order " + xmls,xmls,contains(expectedXmls.toArray()));
+
+        // Order is irrelevant here
         Set<String> options = main.getConfig().getOptions();
-        assertThat(options,Matchers.contains("Server","ext","jmx","jsp","newOption","resources","websocket"));
+        Set<String> expectedOptions = new HashSet<>();
+        // from start.ini
+        expectedOptions.add("Server");
+        expectedOptions.add("jsp");
+        expectedOptions.add("resources");
+        expectedOptions.add("websocket");
+        expectedOptions.add("ext");
+        expectedOptions.add("newOption");
+        // from start.d/10-jmx.ini
+        expectedOptions.add("jmx");
+        // from start.d/20-websocket.ini
+        expectedOptions.add("websocket");
+        // no options from start.d/90-testrealm.ini
+
+        assertThat("Options " + options,options,containsInAnyOrder(expectedOptions.toArray()));
     }
 
     @Test
@@ -99,14 +114,14 @@ public class MainTest
                 hasItems("/jetty/home with spaces/somejar.jar:/jetty/home with spaces/someotherjar.jar"));
         assertThat("args does not contain --exec",commandArgs,hasItems("--exec"));
         assertThat("CommandLine should contain jvmArgs",commandArgs,hasItems("-Xms1024m"));
-        assertThat("CommandLine should contain jvmArgs", commandArgs, hasItems("-Xmx1024m"));
+        assertThat("CommandLine should contain jvmArgs",commandArgs,hasItems("-Xmx1024m"));
         assertThat("CommandLine should contain xmls",commandArgs,hasItems("jetty.xml"));
         assertThat("CommandLine should contain xmls",commandArgs,hasItems("jetty-jmx.xml"));
-        assertThat("CommandLine should contain xmls", commandArgs, hasItems("jetty-logging.xml"));
+        assertThat("CommandLine should contain xmls",commandArgs,hasItems("jetty-logging.xml"));
 
         String commandLine = cmd.toString();
-        assertThat("cmd.toString() should be properly escaped",commandLine,containsString("-cp /jetty/home\\ with\\ " +
+        assertThat("cmd.toString() should be properly escaped",commandLine,containsString("-cp /jetty/home\\ with\\ "
-                "spaces/somejar.jar:/jetty/home\\ with\\ spaces/someotherjar.jar"));
+                + "spaces/somejar.jar:/jetty/home\\ with\\ spaces/someotherjar.jar"));
         assertThat("cmd.toString() doesn't contain xml config files",commandLine,containsString(" jetty.xml jetty-jmx.xml jetty-logging.xml"));
     }