Jetty 9.4.x #2279 start update params (#2285)

Jetty 9.4.x #2279 start update params (#2285):
* Cleanup and Unify param update logic #2279
* Cleanup system property source #2279
* Cleanup property source #2279
* Property cleanup for #2279
* Removed duplicate Prop and Property classes
* properties are only updated/set if value source is not from ?=

Signed-off-by: Greg Wilkins <gregw@webtide.com>

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

@@ -39,7 +39,6 @@ import java.net.SocketTimeoutException;
 import java.nio.file.Path;
 import java.util.List;
 import java.util.Locale;
-import java.util.regex.Matcher;
 
 import org.eclipse.jetty.start.Props.Prop;
 import org.eclipse.jetty.start.config.CommandLineConfigSource;
@@ -295,18 +294,18 @@ public class Main
         args.parse(baseHome.getConfigSources());
 
         Props props = baseHome.getConfigSources().getProps();
-        Props.Prop home = props.getProp(BaseHome.JETTY_HOME);
+        Prop home = props.getProp(BaseHome.JETTY_HOME);
         if (!args.getProperties().containsKey(BaseHome.JETTY_HOME))
             args.getProperties().setProperty(home);
         args.getProperties().setProperty(BaseHome.JETTY_HOME+".uri",
             normalizeURI(baseHome.getHomePath().toUri().toString()),
-            home.origin);
+            home.source);
-        Props.Prop base = props.getProp(BaseHome.JETTY_BASE);
+        Prop base = props.getProp(BaseHome.JETTY_BASE);
         if (!args.getProperties().containsKey(BaseHome.JETTY_BASE))
             args.getProperties().setProperty(base);
         args.getProperties().setProperty(BaseHome.JETTY_BASE+".uri",
             normalizeURI(baseHome.getBasePath().toUri().toString()),
-            base.origin);
+            base.source);
         
         // ------------------------------------------------------------
         // 3) Module Registration
@@ -426,25 +425,8 @@ public class Main
         {
             for (ConfigSource config : baseHome.getConfigSources())
             {
-                System.out.printf("ConfigSource %s%n",config.getId());
                 for (StartIni ini : config.getStartInis())
-                {
-                    for (String line : ini.getAllLines())
-                    {
-                        Matcher m = Module.SET_PROPERTY.matcher(line);
-                        if (m.matches() && m.groupCount()==3)
-                        {
-                            String name = m.group(2);
-                            String value = m.group(3);
-                            Prop p = args.getProperties().getProp(name);
-                            if (p!=null && ("#".equals(m.group(1)) || !value.equals(p.value)))
-                            {
                     ini.update(baseHome,args.getProperties());
-                                break;
-                            }
-                        }
-                    }
-                }
             }
         }
         
@@ -512,10 +494,10 @@ public class Main
 
     private void doStop(StartArgs args)
     {
-        Props.Prop stopHostProp = args.getProperties().getProp("STOP.HOST", true);
+        Prop stopHostProp = args.getProperties().getProp("STOP.HOST", true);
-        Props.Prop stopPortProp = args.getProperties().getProp("STOP.PORT", true);
+        Prop stopPortProp = args.getProperties().getProp("STOP.PORT", true);
-        Props.Prop stopKeyProp = args.getProperties().getProp("STOP.KEY", true);
+        Prop stopKeyProp = args.getProperties().getProp("STOP.KEY", true);
-        Props.Prop stopWaitProp = args.getProperties().getProp("STOP.WAIT", true);
+        Prop stopWaitProp = args.getProperties().getProp("STOP.WAIT", true);
         
         String stopHost = "127.0.0.1";
         int stopPort = -1;

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

@@ -37,7 +37,6 @@ import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import org.eclipse.jetty.start.Props.Prop;
-import org.eclipse.jetty.start.config.CommandLineConfigSource;
 
 /**
  * Represents a Module metadata, as defined in Jetty.
@@ -527,9 +526,12 @@ public class Module implements Comparable<Module>
             if (m.matches() && m.groupCount()==3)
             {
                 String name = m.group(2);
+                String value = m.group(3);
                 Prop p = props.getProp(name);
-                if (p!=null && p.origin.startsWith(CommandLineConfigSource.ORIGIN_CMD_LINE))
+                
+                if (p!=null && (p.source==null || !p.source.endsWith("?=")) && ("#".equals(m.group(1)) || !value.equals(p.value)))
                 {
+                    System.err.printf("%s == %s :: %s%n",name,value,p.source);
                     StartLog.info("%-15s property set %s=%s",this._name,name,p.value);
                     out.printf("%s=%s%n",name,p.value);
                 }

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

@@ -48,20 +48,13 @@ public final class Props implements Iterable<Prop>
     {
         public String key;
         public String value;
-        public String origin;
+        public String source;
-        public Prop overrides;
 
-        public Prop(String key, String value, String origin)
+        public Prop(String key, String value, String source)
         {
             this.key = key;
             this.value = value;
-            this.origin = origin;
+            this.source = source;
-        }
-
-        public Prop(String key, String value, String origin, Prop overrides)
-        {
-            this(key,value,origin);
-            this.overrides = overrides;
         }
 
         @Override
@@ -72,15 +65,12 @@ public final class Props implements Iterable<Prop>
             builder.append(key);
             builder.append(", value=");
             builder.append(value);
-            builder.append(", origin=");
+            builder.append(", source=");
-            builder.append(origin);
+            builder.append(source);
-            builder.append(", overrides=");
-            builder.append(overrides);
             builder.append("]");
             return builder.toString();
         }
     }
-
     public static final String ORIGIN_SYSPROP = "<system-property>";
     
     public static String getValue(String arg)
@@ -342,16 +332,7 @@ public final class Props implements Iterable<Prop>
 
     public void setProperty(String key, String value, String origin)
     {
-        Prop prop = props.get(key);
+        props.put(key,new Prop(key,value,origin));
-        if (prop == null)
-        {
-            prop = new Prop(key,value,origin);
-        }
-        else
-        {
-            prop = new Prop(key,value,origin,prop);
-        }
-        props.put(key,prop);
     }
 
     public int size()

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

@@ -35,7 +35,6 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
 import java.util.StringTokenizer;
-import java.util.function.Function;
 
 import org.eclipse.jetty.start.Props.Prop;
 import org.eclipse.jetty.start.config.ConfigSource;
@@ -123,9 +122,6 @@ public class StartArgs
     /** Map of enabled modules to the source of where that activation occurred */
     Map<String, List<String>> sources = new HashMap<>();
 
-    /** Map of properties to where that property was declared */
-    private Map<String, String> propertySource = new HashMap<>();
-
     /** List of all active [files] sections from enabled modules */
     private List<FileArg> files = new ArrayList<>();
 
@@ -148,7 +144,7 @@ public class StartArgs
     private List<Path> propertyFiles = new ArrayList<>();
 
     private Props properties = new Props();
-    private Set<String> systemPropertyKeys = new HashSet<>();
+    private Map<String,String> systemPropertySource = new HashMap<>();
     private List<String> rawLibs = new ArrayList<>();
 
     // jetty.base - build out commands
@@ -205,12 +201,6 @@ public class StartArgs
         }
     }
 
-    public void addSystemProperty(String key, String value)
-    {
-        this.systemPropertyKeys.add(key);
-        System.setProperty(key,value);
-    }
-
     private void addUniqueXmlFile(String xmlRef, Path xmlfile) throws IOException
     {
         if (!FS.canReadFile(xmlfile))
@@ -337,7 +327,7 @@ public class StartArgs
         List<String> sortedKeys = new ArrayList<>();
         for (Prop prop : properties)
         {
-            if (prop.origin.equals(Props.ORIGIN_SYSPROP))
+            if (prop.source.equals(Props.ORIGIN_SYSPROP))
             {
                 continue; // skip
             }
@@ -369,16 +359,7 @@ public class StartArgs
         {
             System.out.printf(" %s = %s%n",key,prop.value);
             if (StartLog.isDebugEnabled())
-            {
+                System.out.printf("   origin: %s%n",prop.source);
-                System.out.printf("   origin: %s%n",prop.origin);
-                while (prop.overrides != null)
-                {
-                    prop = prop.overrides;
-                    System.out.printf("   (overrides)%n");
-                    System.out.printf("     %s = %s%n",key,prop.value);
-                    System.out.printf("     origin: %s%n",prop.origin);
-                }
-            }
         }
     }
 
@@ -388,26 +369,25 @@ public class StartArgs
         System.out.println("System Properties:");
         System.out.println("------------------");
 
-        if (systemPropertyKeys.isEmpty())
+        if (systemPropertySource.keySet().isEmpty())
         {
             System.out.println(" (no system properties specified)");
             return;
         }
 
         List<String> sortedKeys = new ArrayList<>();
-        sortedKeys.addAll(systemPropertyKeys);
+        sortedKeys.addAll(systemPropertySource.keySet());
         Collections.sort(sortedKeys);
 
         for (String key : sortedKeys)
-        {
+            dumpSystemProperty(key);
-            String value = System.getProperty(key);
-            System.out.printf(" %s = %s%n",key,value);
-        }
     }
 
     private void dumpSystemProperty(String key)
     {
-        System.out.printf(" %s = %s%n",key,System.getProperty(key));
+        String value = System.getProperty(key);
+        String source = systemPropertySource.get(key);
+        System.out.printf(" %s = %s (%s)%n",key,value,source);
     }
 
     /**
@@ -418,20 +398,20 @@ public class StartArgs
      */
     private void ensureSystemPropertySet(String key)
     {
-        if (systemPropertyKeys.contains(key))
+        if (systemPropertySource.containsKey(key))
         {
             return; // done
         }
 
         if (properties.containsKey(key))
         {
-            String val = properties.expand(properties.getString(key));
+            Prop prop = properties.getProp(key);
-            if (val == null)
+            if (prop==null)
-            {
+                return; // no value set;
-                return; // no value to set
+            
-            }
+            String val = properties.expand(prop.value);
             // setup system property
-            systemPropertyKeys.add(key);
+            systemPropertySource.put(key,"property:"+prop.source);
             System.setProperty(key,val);
         }
     }
@@ -446,7 +426,7 @@ public class StartArgs
     {
         StartLog.debug("Expanding System Properties");
         
-        for (String key : systemPropertyKeys)
+        for (String key : systemPropertySource.keySet())
         {
             String value = properties.getString(key);
             if (value!=null)
@@ -588,12 +568,9 @@ public class StartArgs
                     String key = assign[0];
                     String value = assign.length==1?"":assign[1];
 
-                    Property p = processProperty(key,value,"modules",k->{return System.getProperty(k);});
+                    Prop p = processSystemProperty(key,value,null);
-                    if (p!=null)
-                    {   
                     cmd.addRawArg("-D"+p.key+"="+getProperties().expand(p.value));
                 }
-                }
                 else
                 {
                     cmd.addRawArg(x);
@@ -601,7 +578,7 @@ public class StartArgs
             }
 
             // System Properties
-            for (String propKey : systemPropertyKeys)
+            for (String propKey : systemPropertySource.keySet())
             {
                 String value = System.getProperty(propKey);
                 cmd.addEqualsArg("-D" + propKey,value);
@@ -737,7 +714,7 @@ public class StartArgs
 
     public boolean hasSystemProperties()
     {
-        for (String key : systemPropertyKeys)
+        for (String key : systemPropertySource.keySet())
         {
             // ignored keys
             if ("jetty.home".equals(key) || "jetty.base".equals(key) || "main.class".equals(key))
@@ -1096,13 +1073,10 @@ public class StartArgs
             String key = assign[0];
             String value = assign.length==1?"":assign[1];
             
-            Property p = processProperty(key,value,source,k->{return System.getProperty(k);});
+            Prop p = processSystemProperty(key,value,source);
-            if (p!=null)
+            systemPropertySource.put(p.key,p.source);
-            {   
-                systemPropertyKeys.add(p.key);
             setProperty(p.key,p.value,p.source);
             System.setProperty(p.key,p.value);
-            }
             return;
         }
         
@@ -1124,11 +1098,8 @@ public class StartArgs
             String key = arg.substring(0,equals);
             String value = arg.substring(equals + 1);
 
-            Property p = processProperty(key,value,source,k->{return getProperties().getString(k);});
+            processAndSetProperty(key,value,source);
-            if (p!=null)
+            
-            {
-                setProperty(p.key,p.value,p.source);
-            }
             return;
         }
 
@@ -1157,12 +1128,12 @@ public class StartArgs
         throw new UsageException(UsageException.ERR_BAD_ARG,"Unrecognized argument: \"%s\" in %s",arg,source);
     }
     
-    protected Property processProperty(String key,String value,String source, Function<String, String> getter)
+    protected Prop processSystemProperty(String key, String value, String source)
     {
         if (key.endsWith("+"))
         {
             key = key.substring(0,key.length() - 1);
-            String orig = getter.apply(key);
+            String orig = System.getProperty(key);
             if (orig == null || orig.isEmpty())
             {
                 if (value.startsWith(","))
@@ -1171,27 +1142,55 @@ public class StartArgs
             else 
             {
                 value = orig + value;
-                source = propertySource.get(key) + "," + source;
+                if (source!=null && systemPropertySource.containsKey(key))
+                    source = systemPropertySource.get(key) + "," + source;
             }
         }
-        if (key.endsWith("?"))
+        else if (key.endsWith("?"))
         {
             key = key.substring(0,key.length() - 1);
-            String preset = getter.apply(key);
+            String preset = System.getProperty(key);
             if (preset!=null)
             {
-                source = source+"?=";
                 value = preset;
+                source = systemPropertySource.get(key);
             }
-        }
+            else if (source!=null)
-        else if (propertySource.containsKey(key))
+                source = source+"?=";
-        {
-            if (!propertySource.get(key).endsWith("[ini]"))
-                StartLog.warn("Property %s in %s already set in %s",key,source,propertySource.get(key));
-            propertySource.put(key,source);
         }
 
-        return new Property(key,value,source);
+        return new Prop(key, value, source);
+    }
+    
+    protected void processAndSetProperty(String key,String value,String source)
+    {
+        if (key.endsWith("+"))
+        {
+            key = key.substring(0,key.length() - 1);
+            Prop orig = getProperties().getProp(key);
+            if (orig == null)
+            {
+                if (value.startsWith(","))
+                    value = value.substring(1);
+            }
+            else
+            {
+                value = orig.value + value;
+                source = orig.source + "," + source;
+            }
+        }
+        else if (key.endsWith("?"))
+        {
+            key = key.substring(0,key.length() - 1);
+            Prop preset = getProperties().getProp(key);
+            if (preset!=null)
+                return;
+            
+            if (source!=null)
+                source = source+"?=";
+        }
+        
+        setProperty(key,value,source);
     }
     
     private void enableModules(String source, List<String> moduleNames)
@@ -1303,22 +1302,4 @@ public class StartArgs
         return builder.toString();
     }
     
-    static class Property 
-    {
-        String key;
-        String value;
-        String source;
-        public Property(String key, String value, String source)
-        {
-            this.key = key;
-            this.value = value;
-            this.source = source;
-        }  
-        
-        @Override
-        public String toString()
-        {
-            return String.format("%s=%s(%s)",key,value,source);
-        }
-    }
 }

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

@@ -92,7 +92,9 @@ public class StartIni extends TextFile
         update = update.substring(0,update.lastIndexOf("."));
         String source = baseHome.toShortForm(getFile());
         
-        try (PrintWriter writer = new PrintWriter(Files.newBufferedWriter(getFile(),StandardCharsets.UTF_8,StandardOpenOption.TRUNCATE_EXISTING,StandardOpenOption.CREATE)))
+        PrintWriter writer = null;
+        
+        try
         {
             for (String line : getAllLines())
             {
@@ -102,23 +104,42 @@ public class StartIni extends TextFile
                     String name = m.group(2);
                     String value = m.group(3);
                     Prop p = props.getProp(name);
-                    if (p!=null && ("#".equals(m.group(1)) || !value.equals(p.value)))
+                    
+                    if (p!=null && (p.source==null || !p.source.endsWith("?=")) && ("#".equals(m.group(1)) || !value.equals(p.value)))
                     {
-                        StartLog.info("%-15s property updated %s=%s",update,name,p.value);
+                        if (writer==null)
-                        writer.printf("%s=%s%n",name,p.value);
-                    }
-                    else
                         {
-                        writer.println(line);
+                            writer = new PrintWriter(Files.newBufferedWriter(getFile(),StandardCharsets.UTF_8,StandardOpenOption.TRUNCATE_EXISTING,StandardOpenOption.CREATE));
-                    }
+                            for (String l : getAllLines())
-                }
-                else
                             {
-                    writer.println(line);
+                                if (line.equals(l))
-                }
+                                    break;
+                                writer.println(l);
                             }
                         }
                         
+                        StartLog.info("%-15s property updated %s=%s",update,name,p.value);
+                        writer.printf("%s=%s%n",name,p.value);
+                    }
+                    else if (writer!=null)
+                    {
+                        writer.println(line);
+                    }
+                }
+                else if (writer!=null)
+                {
+                    writer.println(line);
+                }
+            }
+        }
+        finally
+        {
+            if (writer!=null)
+            {
                 StartLog.info("%-15s updated %s",update,source);
+                writer.close();
+            }
+        }
+
     }
 }

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

@@ -142,7 +142,7 @@ public class ConfigurationAssert
                 "jetty.home.uri".equals(name) ||
                 "jetty.base.uri".equals(name) ||
                 "user.dir".equals(name) ||
-                prop.origin.equals(Props.ORIGIN_SYSPROP) ||
+                prop.source.equals(Props.ORIGIN_SYSPROP) ||
                 name.startsWith("java."))
             {
                 // strip these out from assertion, to make assertions easier.

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

@@ -20,7 +20,6 @@ package org.eclipse.jetty.start;
 
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
@@ -36,7 +35,7 @@ public class PropsTest
         assertThat(prefix,prop,notNullValue());
         assertThat(prefix + ".key",prop.key,is(expectedKey));
         assertThat(prefix + ".value",prop.value,is(expectedValue));
-        assertThat(prefix + ".origin",prop.origin,is(expectedOrigin));
+        assertThat(prefix + ".origin",prop.source,is(expectedOrigin));
     }
 
     @Test
@@ -49,7 +48,6 @@ public class PropsTest
 
         Prop prop = props.getProp("java.io.tmpdir");
         assertProp("System Prop",prop,"java.io.tmpdir",expected,Props.ORIGIN_SYSPROP);
-        assertThat("System Prop.overrides",prop.overrides,nullValue());
     }
 
     @Test
@@ -63,25 +61,6 @@ public class PropsTest
 
         Prop prop = props.getProp("name");
         assertProp(prefix,prop,"name","jetty",FROM_TEST);
-        assertThat(prefix + ".overrides",prop.overrides,nullValue());
-    }
-
-    @Test
-    public void testOverride()
-    {
-        Props props = new Props();
-        props.setProperty("name","jetty",FROM_TEST);
-        props.setProperty("name","altjetty","(Alt-Jetty)");
-
-        String prefix = "Overriden";
-        assertThat(prefix,props.getString("name"),is("altjetty"));
-
-        Prop prop = props.getProp("name");
-        assertProp(prefix,prop,"name","altjetty","(Alt-Jetty)");
-        Prop older = prop.overrides;
-        assertThat(prefix + ".overrides",older,notNullValue());
-        assertProp(prefix + ".overridden",older,"name","jetty",FROM_TEST);
-        assertThat(prefix + ".overridden",older.overrides,nullValue());
     }
 
     @Test

jetty-start/src/test/resources/usecases/parameterized.addToStart.assert.txt

@@ -12,6 +12,7 @@ PROP|main.prop=value0
 PROP|name=value
 PROP|name0=changed0
 PROP|name1=changed1
+PROP|name2=two
 PROP|property=value
 PROP|property0=value0
 

jetty-start/src/test/resources/usecases/parameterized.commands.assert.txt

@@ -12,6 +12,7 @@ PROP|main.prop=value0
 PROP|name=value
 PROP|name0=changed0
 PROP|name1=changed1
+PROP|name2=two
 PROP|property=value
 PROP|property0=value0
 

jetty-start/src/test/resources/usecases/parameterized.update.assert.txt

@@ -10,8 +10,9 @@ LIB|${jetty.home}/lib/other.jar
 # The Properties we expect (order is irrelevant)
 PROP|main.prop=value0
 PROP|name=value
-PROP|name0=changed0
+PROP|name0=updated0
 PROP|name1=changed1
+PROP|name2=two
 PROP|property=value
 PROP|property0=changed0
 PROP|property1=changed1

jetty-start/src/test/resources/usecases/parameterized.update.prepare.txt

@@ -8,3 +8,4 @@ name1=changed1
 --update-ini
 property0=changed0
 property1=changed1
+name0=updated0

jetty-start/src/test/resources/usecases/parameterized/modules/parameterized.mod

@@ -4,7 +4,10 @@ main
 
 [ini]
 name=value
+name0?=default
+name2?=two
 
 [ini-template]
 name0=value0
 # name1=value1
+# name2=too