diff --git a/jetty-server/src/main/config/etc/jetty.xml b/jetty-server/src/main/config/etc/jetty.xml index 03e916b9d277..f5a59870251b 100644 --- a/jetty-server/src/main/config/etc/jetty.xml +++ b/jetty-server/src/main/config/etc/jetty.xml @@ -1,8 +1,27 @@ - + + + + + + + + + + + + - + + + + + + + + + diff --git a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java index 286e28b8c394..b70b8c3152da 100644 --- a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java +++ b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java @@ -396,6 +396,8 @@ public Object configure(Object obj) throws Exception */ public Object configure() throws Exception { + if (LOG.isDebugEnabled()) + LOG.debug("Configure {}", _location); return _processor.configure(); } @@ -442,7 +444,12 @@ public Object configure(Object obj) throws Exception String id = _root.getAttribute("id"); if (id != null) _configuration.getIdMap().put(id, obj); - configure(obj, _root, 0); + + AttrOrElementNode aoeNode = new AttrOrElementNode(obj, _root, "Arg"); + // The Object already existed, if it has nodes, warn about them not being used. + aoeNode.getNodes("Arg") + .forEach((node) -> LOG.warn("Ignored arg {} in {}", node, this._configuration._location)); + configure(obj, _root, aoeNode.getNext()); return obj; } @@ -454,23 +461,32 @@ public Object configure() throws Exception String id = _root.getAttribute("id"); Object obj = id == null ? null : _configuration.getIdMap().get(id); - int index = 0; + AttrOrElementNode aoeNode; + if (obj == null && oClass != null) { + aoeNode = new AttrOrElementNode(_root, "Arg"); try { - obj = construct(oClass, new Args(null, oClass, XmlConfiguration.getNodes(_root, "Arg"))); + obj = construct(oClass, new Args(null, oClass, aoeNode.getNodes("Arg"))); } catch (NoSuchMethodException x) { throw new IllegalStateException(String.format("No matching constructor %s in %s", oClass, _configuration)); } } + else + { + aoeNode = new AttrOrElementNode(obj, _root, "Arg"); + // The Object already existed, if it has nodes, warn about them not being used. + aoeNode.getNodes("Arg") + .forEach((node) -> LOG.warn("Ignored arg {} in {}", node, this._configuration._location)); + } if (id != null) _configuration.getIdMap().put(id, obj); _configuration.initializeDefaults(obj); - configure(obj, _root, index); + configure(obj, _root, aoeNode.getNext()); return obj; } @@ -493,21 +509,6 @@ private static Class nodeClass(XmlParser.Node node) throws ClassNotFoundExcep */ public void configure(Object obj, XmlParser.Node cfg, int i) throws Exception { - // Object already constructed so skip any arguments - for (; i < cfg.size(); i++) - { - Object o = cfg.get(i); - if (o instanceof String) - continue; - XmlParser.Node node = (XmlParser.Node)o; - if ("Arg".equals(node.getTag())) - { - LOG.warn("Ignored arg: " + node); - continue; - } - break; - } - // Process real arguments for (; i < cfg.size(); i++) { @@ -521,6 +522,10 @@ public void configure(Object obj, XmlParser.Node cfg, int i) throws Exception String tag = node.getTag(); switch (tag) { + case "Arg": + case "Class": + case "Id": + throw new IllegalStateException("Element '" + tag + "' not skipped"); case "Set": set(obj, node); break; @@ -1894,7 +1899,7 @@ else if (arg.toLowerCase(Locale.ENGLISH).endsWith(".properties")) { if (!arg.toLowerCase(Locale.ENGLISH).endsWith(".properties") && (arg.indexOf('=') < 0)) { - XmlConfiguration configuration = new XmlConfiguration(Resource.newResource(arg).getURI().toURL()); + XmlConfiguration configuration = new XmlConfiguration(Resource.newResource(arg)); if (last != null) configuration.getIdMap().putAll(last.getIdMap()); if (properties.size() > 0) diff --git a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java index ff780086d865..280f64467b9f 100644 --- a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java +++ b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java @@ -41,6 +41,7 @@ import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; +import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.log.StdErrLog; @@ -60,7 +61,10 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; @@ -265,7 +269,12 @@ public void initializeDefaults(Object object) public XmlConfiguration asXmlConfiguration(String rawXml) throws IOException, SAXException { - Path testFile = workDir.getEmptyPathDir().resolve("raw.xml"); + return asXmlConfiguration("raw.xml", rawXml); + } + + public XmlConfiguration asXmlConfiguration(String filename, String rawXml) throws IOException, SAXException + { + Path testFile = workDir.getEmptyPathDir().resolve(filename); try (BufferedWriter writer = Files.newBufferedWriter(testFile, UTF_8)) { writer.write(rawXml); @@ -1309,6 +1318,185 @@ public void testJettyStandardIdsAndPropertiesAndJettyHomeUriAndJettyBaseUri() th } } + public static class BarNamed + { + private String foo; + private List zeds; + + public BarNamed(@Name("foo") String foo) + { + this.foo = foo; + } + + public void addZed(String zed) + { + if (zeds == null) + zeds = new ArrayList<>(); + zeds.add(zed); + } + + public List getZeds() + { + return zeds; + } + + public String getFoo() + { + return foo; + } + } + + @Test + public void testConfiguredWithNamedArg() throws Exception + { + XmlConfiguration xmlFoo = asXmlConfiguration("foo.xml", + "\n" + + " \n" + + " foozball\n" + + " \n" + + ""); + XmlConfiguration xmlBar = asXmlConfiguration("bar.xml", + "\n" + + " \n" + + ""); + + try (StdErrCapture logCapture = new StdErrCapture(XmlConfiguration.class)) + { + Map idMap = mimicXmlConfigurationMain(xmlFoo, xmlBar); + Object obj = idMap.get("bar"); + assertThat("BarNamed instance created", obj, instanceOf(BarNamed.class)); + BarNamed bar = (BarNamed)obj; + assertThat("BarNamed has foo", bar.getFoo(), is("foozball")); + + List warnLogs = logCapture.getLines() + .stream().filter(line -> line.contains(":WARN:")) + .collect(Collectors.toList()); + + assertThat("WARN logs size", warnLogs.size(), is(0)); + } + } + + @Test + public void testConfiguredWithArgNotUsingName() throws Exception + { + XmlConfiguration xmlFoo = asXmlConfiguration("foo.xml", + "\n" + + " \n" + + " foozball\n" + + " \n" + + ""); + XmlConfiguration xmlBar = asXmlConfiguration("bar.xml", + "\n" + + " \n" + // no name specified + ""); + + try (StdErrCapture logCapture = new StdErrCapture(XmlConfiguration.class)) + { + Map idMap = mimicXmlConfigurationMain(xmlFoo, xmlBar); + Object obj = idMap.get("bar"); + assertThat("BarNamed instance created", obj, instanceOf(BarNamed.class)); + BarNamed bar = (BarNamed)obj; + assertThat("BarNamed has foo", bar.getFoo(), is("foozball")); + + List warnLogs = logCapture.getLines() + .stream().filter(line -> line.contains(":WARN:")) + .collect(Collectors.toList()); + + assertThat("WARN logs size", warnLogs.size(), is(0)); + } + } + + @Test + public void testConfiguredWithBadNamedArg() throws Exception + { + XmlConfiguration xmlBar = asXmlConfiguration("bar.xml", + "\n" + + " foozball\n" + // wrong name specified + ""); + + IllegalStateException cause = assertThrows(IllegalStateException.class, () -> + mimicXmlConfigurationMain(xmlBar)); + + assertThat("Cause message", cause.getMessage(), containsString("No matching constructor")); + } + + @Test + public void testConfiguredWithTooManyNamedArgs() throws Exception + { + XmlConfiguration xmlBar = asXmlConfiguration("bar.xml", + "\n" + + " foozball\n" + + " soccer\n" + // neither should win + ""); + + IllegalStateException cause = assertThrows(IllegalStateException.class, () -> + mimicXmlConfigurationMain(xmlBar)); + + assertThat("Cause message", cause.getMessage(), containsString("No matching constructor")); + } + + @Test + public void testConfiguredSameWithNamedArgTwice() throws Exception + { + XmlConfiguration xmlFoo = asXmlConfiguration("foo.xml", + "\n" + + " \n" + + " foozball\n" + + " \n" + + ""); + XmlConfiguration xmlBar = asXmlConfiguration("bar.xml", + "\n" + + " \n" + + ""); + XmlConfiguration xmlAddZed = asXmlConfiguration("zed.xml", + "\n" + + " baz\n" + // the invalid line + " \n" + + " plain-zero\n" + + " \n" + + ""); + + try (StdErrCapture logCapture = new StdErrCapture(XmlConfiguration.class)) + { + Map idMap = mimicXmlConfigurationMain(xmlFoo, xmlBar, xmlAddZed); + Object obj = idMap.get("bar"); + assertThat("BarNamed instance created", obj, instanceOf(BarNamed.class)); + BarNamed bar = (BarNamed)obj; + assertThat("BarNamed has foo", bar.getFoo(), is("foozball")); + List zeds = bar.getZeds(); + assertThat("BarNamed has zeds", zeds, not(empty())); + assertThat("Zeds[0]", zeds.get(0), is("plain-zero")); + + List warnLogs = logCapture.getLines() + .stream().filter(line -> line.contains(":WARN:")) + .collect(Collectors.toList()); + + assertThat("WARN logs count", warnLogs.size(), is(1)); + + String actualWarn = warnLogs.get(0); + assertThat("WARN logs", actualWarn, + allOf(containsString("Ignored arg mimicXmlConfigurationMain(XmlConfiguration... configurations) throws Exception + { + XmlConfiguration last = null; + for (XmlConfiguration configuration : configurations) + { + if (last != null) + configuration.getIdMap().putAll(last.getIdMap()); + configuration.configure(); + last = configuration; + } + return last.getIdMap(); + } + @Test public void testDeprecatedMany() throws Exception {