From a128b87e3a3245fcf3c4f54483110bc9d68d8192 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 7 Mar 2018 16:33:28 +1100 Subject: [PATCH] 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 --- .../java/org/eclipse/jetty/start/Main.java | 36 +--- .../java/org/eclipse/jetty/start/Module.java | 6 +- .../java/org/eclipse/jetty/start/Props.java | 31 +--- .../org/eclipse/jetty/start/StartArgs.java | 161 ++++++++---------- .../org/eclipse/jetty/start/StartIni.java | 31 +++- .../jetty/start/ConfigurationAssert.java | 2 +- .../org/eclipse/jetty/start/PropsTest.java | 23 +-- .../parameterized.addToStart.assert.txt | 1 + .../parameterized.commands.assert.txt | 1 + .../usecases/parameterized.update.assert.txt | 3 +- .../usecases/parameterized.update.prepare.txt | 1 + .../parameterized/modules/parameterized.mod | 3 + 12 files changed, 126 insertions(+), 173 deletions(-) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java index 1bf7c8fef2a..59c7dc72a46 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java +++ b/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); - Props.Prop base = props.getProp(BaseHome.JETTY_BASE); + home.source); + 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; - } - } - } - } + ini.update(baseHome,args.getProperties()); } } @@ -512,10 +494,10 @@ public class Main private void doStop(StartArgs args) { - Props.Prop stopHostProp = args.getProperties().getProp("STOP.HOST", true); - Props.Prop stopPortProp = args.getProperties().getProp("STOP.PORT", true); - Props.Prop stopKeyProp = args.getProperties().getProp("STOP.KEY", true); - Props.Prop stopWaitProp = args.getProperties().getProp("STOP.WAIT", true); + Prop stopHostProp = args.getProperties().getProp("STOP.HOST", true); + Prop stopPortProp = args.getProperties().getProp("STOP.PORT", true); + Prop stopKeyProp = args.getProperties().getProp("STOP.KEY", true); + Prop stopWaitProp = args.getProperties().getProp("STOP.WAIT", true); String stopHost = "127.0.0.1"; int stopPort = -1; diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java index 45e1581a553..7bec2fe5b90 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java +++ b/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 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); } diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Props.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Props.java index c1dc1e13c34..73f9775ef6a 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Props.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Props.java @@ -48,20 +48,13 @@ public final class Props implements Iterable { public String key; public String value; - public String origin; - public Prop overrides; + public String source; - 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; - } - - public Prop(String key, String value, String origin, Prop overrides) - { - this(key,value,origin); - this.overrides = overrides; + this.source = source; } @Override @@ -72,15 +65,12 @@ public final class Props implements Iterable builder.append(key); builder.append(", value="); builder.append(value); - builder.append(", origin="); - builder.append(origin); - builder.append(", overrides="); - builder.append(overrides); + builder.append(", source="); + builder.append(source); builder.append("]"); return builder.toString(); } } - public static final String ORIGIN_SYSPROP = ""; public static String getValue(String arg) @@ -342,16 +332,7 @@ public final class Props implements Iterable public void setProperty(String key, String value, String origin) { - Prop prop = props.get(key); - if (prop == null) - { - prop = new Prop(key,value,origin); - } - else - { - prop = new Prop(key,value,origin,prop); - } - props.put(key,prop); + props.put(key,new Prop(key,value,origin)); } public int size() diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java index ed06cfc48a7..235b0358626 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java +++ b/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> sources = new HashMap<>(); - /** Map of properties to where that property was declared */ - private Map propertySource = new HashMap<>(); - /** List of all active [files] sections from enabled modules */ private List files = new ArrayList<>(); @@ -148,7 +144,7 @@ public class StartArgs private List propertyFiles = new ArrayList<>(); private Props properties = new Props(); - private Set systemPropertyKeys = new HashSet<>(); + private Map systemPropertySource = new HashMap<>(); private List 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 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.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); - } - } + System.out.printf(" origin: %s%n",prop.source); } } @@ -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 sortedKeys = new ArrayList<>(); - sortedKeys.addAll(systemPropertyKeys); + sortedKeys.addAll(systemPropertySource.keySet()); Collections.sort(sortedKeys); for (String key : sortedKeys) - { - String value = System.getProperty(key); - System.out.printf(" %s = %s%n",key,value); - } + dumpSystemProperty(key); } 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)); - if (val == null) - { - return; // no value to set - } + Prop prop = properties.getProp(key); + if (prop==null) + return; // no value 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,11 +568,8 @@ 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);}); - if (p!=null) - { - cmd.addRawArg("-D"+p.key+"="+getProperties().expand(p.value)); - } + Prop p = processSystemProperty(key,value,null); + cmd.addRawArg("-D"+p.key+"="+getProperties().expand(p.value)); } else { @@ -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);}); - if (p!=null) - { - systemPropertyKeys.add(p.key); - setProperty(p.key,p.value,p.source); - System.setProperty(p.key,p.value); - } + Prop p = processSystemProperty(key,value,source); + systemPropertySource.put(p.key,p.source); + 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);}); - if (p!=null) - { - setProperty(p.key,p.value,p.source); - } + processAndSetProperty(key,value,source); + return; } @@ -1157,41 +1128,69 @@ 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 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(",")) value = value.substring(1); } + else + { + value = orig + value; + if (source!=null && systemPropertySource.containsKey(key)) + source = systemPropertySource.get(key) + "," + source; + } + } + else if (key.endsWith("?")) + { + key = key.substring(0,key.length() - 1); + String preset = System.getProperty(key); + if (preset!=null) + { + value = preset; + source = systemPropertySource.get(key); + } + else if (source!=null) + source = 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; - source = propertySource.get(key) + "," + source; + value = orig.value + value; + source = orig.source + "," + source; } } - if (key.endsWith("?")) + else if (key.endsWith("?")) { key = key.substring(0,key.length() - 1); - String preset = getter.apply(key); + Prop preset = getProperties().getProp(key); if (preset!=null) - { + return; + + if (source!=null) source = source+"?="; - value = preset; - } - } - else if (propertySource.containsKey(key)) - { - 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); + setProperty(key,value,source); } private void enableModules(String source, List 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); - } - } } diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/StartIni.java b/jetty-start/src/main/java/org/eclipse/jetty/start/StartIni.java index ed212777a2b..08a2ddf43ad 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/StartIni.java +++ b/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))) { + if (writer==null) + { + writer = new PrintWriter(Files.newBufferedWriter(getFile(),StandardCharsets.UTF_8,StandardOpenOption.TRUNCATE_EXISTING,StandardOpenOption.CREATE)); + for (String l : getAllLines()) + { + 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 + else if (writer!=null) { writer.println(line); } } - else + else if (writer!=null) { writer.println(line); } } } + finally + { + if (writer!=null) + { + StartLog.info("%-15s updated %s",update,source); + writer.close(); + } + } - StartLog.info("%-15s updated %s",update,source); } } diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java b/jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java index b366117adea..6d19755d068 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/ConfigurationAssert.java +++ b/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. diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/PropsTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/PropsTest.java index b5605391f53..8b70512485d 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/PropsTest.java +++ b/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 diff --git a/jetty-start/src/test/resources/usecases/parameterized.addToStart.assert.txt b/jetty-start/src/test/resources/usecases/parameterized.addToStart.assert.txt index b31ef3c69c5..46344a3d993 100644 --- a/jetty-start/src/test/resources/usecases/parameterized.addToStart.assert.txt +++ b/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 diff --git a/jetty-start/src/test/resources/usecases/parameterized.commands.assert.txt b/jetty-start/src/test/resources/usecases/parameterized.commands.assert.txt index b31ef3c69c5..46344a3d993 100644 --- a/jetty-start/src/test/resources/usecases/parameterized.commands.assert.txt +++ b/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 diff --git a/jetty-start/src/test/resources/usecases/parameterized.update.assert.txt b/jetty-start/src/test/resources/usecases/parameterized.update.assert.txt index 4e4570e277d..a581efdad66 100644 --- a/jetty-start/src/test/resources/usecases/parameterized.update.assert.txt +++ b/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 diff --git a/jetty-start/src/test/resources/usecases/parameterized.update.prepare.txt b/jetty-start/src/test/resources/usecases/parameterized.update.prepare.txt index 6ecc31c4e0d..86a278a1792 100644 --- a/jetty-start/src/test/resources/usecases/parameterized.update.prepare.txt +++ b/jetty-start/src/test/resources/usecases/parameterized.update.prepare.txt @@ -8,3 +8,4 @@ name1=changed1 --update-ini property0=changed0 property1=changed1 +name0=updated0 \ No newline at end of file diff --git a/jetty-start/src/test/resources/usecases/parameterized/modules/parameterized.mod b/jetty-start/src/test/resources/usecases/parameterized/modules/parameterized.mod index c243eb73efa..7d09c7e9fe9 100644 --- a/jetty-start/src/test/resources/usecases/parameterized/modules/parameterized.mod +++ b/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