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 2d9bb367657..db8fcf154cd 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 @@ -566,6 +566,7 @@ public class XmlConfiguration String attr = node.getAttribute("name"); String name = "set" + attr.substring(0, 1).toUpperCase(Locale.ENGLISH) + attr.substring(1); Object value = value(obj, node); + String defaultValue = defaultValue(obj, node); Object[] arg = {value}; Class oClass = nodeClass(node); @@ -578,8 +579,16 @@ public class XmlConfiguration if (value != null) vClass[0] = value.getClass(); + boolean isUsingDefaultValue = ((value != null) && (defaultValue.equalsIgnoreCase(value.toString()))); + if (LOG.isDebugEnabled()) - LOG.debug("XML " + (obj != null ? obj.toString() : oClass.getName()) + "." + name + "(" + value + ")"); + { + LOG.debug("XML {}.{}({}) [{}]", + (obj != null ? obj.toString() : oClass.getName()), + name, + value, + isUsingDefaultValue ? "DEFAULT" : "NEW"); + } MultiException me = new MultiException(); @@ -587,7 +596,7 @@ public class XmlConfiguration try { Method set = oClass.getMethod(name, vClass); - invokeMethod(set, obj, arg); + invokeMethod(set, obj, arg, isUsingDefaultValue); return; } catch (IllegalArgumentException | IllegalAccessException | NoSuchMethodException e) @@ -602,7 +611,7 @@ public class XmlConfiguration Field type = vClass[0].getField("TYPE"); vClass[0] = (Class)type.get(null); Method set = oClass.getMethod(name, vClass); - invokeMethod(set, obj, arg); + invokeMethod(set, obj, arg, isUsingDefaultValue); return; } catch (NoSuchFieldException | IllegalArgumentException | IllegalAccessException | NoSuchMethodException e) @@ -619,7 +628,7 @@ public class XmlConfiguration { try { - setField(field, obj, value); + setField(field, obj, value, isUsingDefaultValue); return; } catch (IllegalArgumentException e) @@ -630,7 +639,7 @@ public class XmlConfiguration try { value = TypeUtil.valueOf(field.getType(), ((String)value).trim()); - setField(field, obj, value); + setField(field, obj, value, isUsingDefaultValue); return; } catch (Exception e2) @@ -664,7 +673,7 @@ public class XmlConfiguration try { set = setter; - invokeMethod(set, obj, arg); + invokeMethod(set, obj, arg, isUsingDefaultValue); return; } catch (IllegalArgumentException | IllegalAccessException e) @@ -679,7 +688,8 @@ public class XmlConfiguration { if (paramTypes[0].isAssignableFrom(c)) { - invokeMethod(setter, obj, convertArrayToCollection(value, c)); + Object[] args = {convertArrayToCollection(value, c)}; + invokeMethod(setter, obj, args, isUsingDefaultValue); return; } } @@ -712,7 +722,7 @@ public class XmlConfiguration Constructor cons = sClass.getConstructor(vClass); arg[0] = cons.newInstance(arg); _configuration.initializeDefaults(arg[0]); - invokeMethod(set, obj, arg); + invokeMethod(set, obj, arg, isUsingDefaultValue); return; } catch (NoSuchMethodException | IllegalAccessException | InstantiationException e) @@ -737,36 +747,47 @@ public class XmlConfiguration private Object invokeConstructor(Constructor constructor, Object... args) throws IllegalAccessException, InvocationTargetException, InstantiationException { Object result = constructor.newInstance(args); - if (LOG.isDebugEnabled()) - if (constructor.getAnnotation(Deprecated.class) != null) - LOG.warn("Deprecated constructor {} in {}", constructor, _configuration); + if (constructor.getAnnotation(Deprecated.class) != null) + LOG.warn("Deprecated constructor {} in {}", constructor, _configuration); return result; } - private Object invokeMethod(Method method, Object obj, Object... args) throws IllegalAccessException, InvocationTargetException + private Object invokeMethod(Method method, Object obj, Object[] args) throws IllegalAccessException, InvocationTargetException + { + return invokeMethod(method, obj, args, false); + } + + private Object invokeMethod(Method method, Object obj, Object[] args, boolean isUsingDefaultValue) throws IllegalAccessException, InvocationTargetException { Object result = method.invoke(obj, args); - if (LOG.isDebugEnabled()) - if (method.getAnnotation(Deprecated.class) != null) + if (method.getAnnotation(Deprecated.class) != null) + { + if (isUsingDefaultValue) + LOG.debug("Deprecated method {} in {}", method, _configuration); + else LOG.warn("Deprecated method {} in {}", method, _configuration); + } return result; } private Object getField(Field field, Object object) throws IllegalAccessException { Object result = field.get(object); - if (LOG.isDebugEnabled()) - if (field.getAnnotation(Deprecated.class) != null) - LOG.warn("Deprecated field {} in {}", field, _configuration); + if (field.getAnnotation(Deprecated.class) != null) + LOG.warn("Deprecated field {} in {}", field, _configuration); return result; } - private void setField(Field field, Object obj, Object arg) throws IllegalAccessException + private void setField(Field field, Object obj, Object arg, boolean isUsingDefaultValue) throws IllegalAccessException { field.set(obj, arg); - if (LOG.isDebugEnabled()) - if (field.getAnnotation(Deprecated.class) != null) + if (field.getAnnotation(Deprecated.class) != null) + { + if (isUsingDefaultValue) + LOG.debug("Deprecated field {} in {}", field, _configuration); + else LOG.warn("Deprecated field {} in {}", field, _configuration); + } } /** @@ -855,7 +876,7 @@ public class XmlConfiguration { // Try calling a getXxx method. Method method = oClass.getMethod("get" + name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1)); - obj = invokeMethod(method, obj); + obj = invokeMethod(method, obj, null, false); } if (id != null) _configuration.getIdMap().put(id, obj); @@ -1398,6 +1419,45 @@ public class XmlConfiguration return value; } + /** + * Check children for all {@code } and {@code } and return + * the String representation of any declared {@code default="value"} attributes. + * + * @param obj the enclosing obj + * @param node the XML node + * @return a String representing all {@code } and {@code } values appended together + */ + private String defaultValue(Object obj, XmlParser.Node node) throws Exception + { + StringBuilder ret = new StringBuilder(); + + appendDefaultPropertyValues(ret, node); + + return ret.toString(); + } + + private void appendDefaultPropertyValues(StringBuilder defValues, XmlParser.Node node) throws Exception + { + for (Object child : node) + { + if (child instanceof XmlParser.Node) + { + XmlParser.Node childNode = (XmlParser.Node)child; + String tag = childNode.getTag(); + if ("Property".equals(tag) || "SystemProperty".equals(tag)) + { + AttrOrElementNode aoeNode = new AttrOrElementNode(childNode, "Id", "Name", "Deprecated", "Default"); + String dftValue = aoeNode.getString("Default"); + if (dftValue != null) + { + defValues.append(dftValue); + } + } + appendDefaultPropertyValues(defValues, childNode); + } + } + } + /** *

Returns the scalar value of an element

. *

If no value type is specified, then white space is trimmed out of the value. diff --git a/jetty-xml/src/test/java/org/eclipse/jetty/xml/AnnotatedTestConfiguration.java b/jetty-xml/src/test/java/org/eclipse/jetty/xml/AnnotatedTestConfiguration.java index 9d1ff60db16..32796563ece 100644 --- a/jetty-xml/src/test/java/org/eclipse/jetty/xml/AnnotatedTestConfiguration.java +++ b/jetty-xml/src/test/java/org/eclipse/jetty/xml/AnnotatedTestConfiguration.java @@ -27,6 +27,10 @@ public class AnnotatedTestConfiguration private String third; private String deprecated; private AnnotatedTestConfiguration nested; + + // Do not remove deprecation, used in tests. + @Deprecated + private long timeout = -1; // Do not remove deprecation, used in tests. @Deprecated public String obsolete; @@ -97,4 +101,18 @@ public class AnnotatedTestConfiguration { return deprecated; } + + // Do not remove deprecation, used in tests. + @Deprecated + public long getTimeout() + { + return timeout; + } + + // Do not remove deprecation, used in tests. + @Deprecated + public void setTimeout(long value) + { + this.timeout = value; + } } 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 6b911a49412..f00ad2389a3 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 @@ -22,15 +22,18 @@ import java.io.BufferedWriter; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintStream; +import java.io.UnsupportedEncodingException; import java.lang.reflect.InvocationTargetException; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; @@ -1061,45 +1064,278 @@ public class XmlConfigurationTest } @Test - public void testDeprecated() throws Exception + public void testDeprecatedMany() throws Exception { Class testClass = AnnotatedTestConfiguration.class; XmlConfiguration xmlConfiguration = asXmlConfiguration( "" + " foo" + + " " + " " + " " + " " + " " + ""); - ByteArrayOutputStream logBytes = null; - Logger logger = Log.getLogger(XmlConfiguration.class); - logger.setDebugEnabled(true); - if (logger instanceof StdErrLog) + List logLines; + try (StdErrCapture logCapture = new StdErrCapture(XmlConfiguration.class)) { - StdErrLog stdErrLog = (StdErrLog)logger; - logBytes = new ByteArrayOutputStream(); - stdErrLog.setStdErrStream(new PrintStream(logBytes)); + xmlConfiguration.getProperties().put("test.timeout", "-1"); + xmlConfiguration.configure(); + logLines = logCapture.getLines(); } - xmlConfiguration.configure(); + List warnings = logLines.stream() + .filter(line -> line.contains(":WARN:")) + .filter(line -> line.contains(testClass.getSimpleName())) + .collect(Collectors.toList()); + // 1. Deprecated constructor + // 2. Deprecated method + // 3. Deprecated method + // 4. Deprecated method + // 5. Deprecated field + // 6. Deprecated field + assertEquals(6, warnings.size()); + } - logger.setDebugEnabled(false); - if (logBytes != null) + @Test + public void testDeprecatedPropertyUnSet() throws Exception + { + Class testClass = AnnotatedTestConfiguration.class; + XmlConfiguration xmlConfiguration = asXmlConfiguration( + "" + + " " + + ""); + assertDeprecatedPropertyUnSet(testClass, xmlConfiguration); + } + + @Test + public void testDeprecatedPropertyUnSetWhiteSpace() throws Exception + { + Class testClass = AnnotatedTestConfiguration.class; + XmlConfiguration xmlConfiguration = asXmlConfiguration( + "" + + " " + + " " + + " " + + ""); + assertDeprecatedPropertyUnSet(testClass, xmlConfiguration); + } + + private void assertDeprecatedPropertyUnSet(Class testClass, XmlConfiguration xmlConfiguration) throws Exception + { + List logLines; + try (StdErrCapture logCapture = new StdErrCapture(XmlConfiguration.class)) { + // Leave this line alone, as this tests what happens if property is unset, + // so that it relies on the value + // xmlConfiguration.getProperties().put("test.timeout", "-1"); + xmlConfiguration.configure(); + logLines = logCapture.getLines(); + } + + List warnings = logLines.stream() + .filter(LogPredicates.deprecatedWarnings(testClass)) + .collect(Collectors.toList()); + String[] expected = { + "Deprecated constructor public org.eclipse.jetty.xml.AnnotatedTestConfiguration" + }; + + assertHasExpectedLines("Warnings", warnings, expected); + } + + @Test + public void testDeprecatedPropertySetToDefaultValue() throws Exception + { + Class testClass = AnnotatedTestConfiguration.class; + XmlConfiguration xmlConfiguration = asXmlConfiguration( + "" + + " " + + ""); + + assertDeprecatedPropertySetToDefaultValue(testClass, xmlConfiguration); + } + + @Test + public void testDeprecatedPropertySetToDefaultValueWhiteSpace() throws Exception + { + Class testClass = AnnotatedTestConfiguration.class; + XmlConfiguration xmlConfiguration = asXmlConfiguration( + "" + + " " + + " " + + " " + + ""); + + assertDeprecatedPropertySetToDefaultValue(testClass, xmlConfiguration); + } + + private void assertDeprecatedPropertySetToDefaultValue(Class testClass, XmlConfiguration xmlConfiguration) throws Exception + { + List logLines; + try (StdErrCapture logCapture = new StdErrCapture(XmlConfiguration.class)) + { + // Leave this line alone, as this tests what happens if property is set, + // and has the same value as declared on + xmlConfiguration.getProperties().put("test.timeout", "-1"); + xmlConfiguration.configure(); + logLines = logCapture.getLines(); + } + + List warnings = logLines.stream() + .filter(LogPredicates.deprecatedWarnings(testClass)) + .collect(Collectors.toList()); + + String[] expected = { + "Deprecated constructor public org.eclipse.jetty.xml.AnnotatedTestConfiguration", + }; + assertHasExpectedLines("Warnings", warnings, expected); + + List debugs = logLines.stream() + .filter(LogPredicates.deprecatedDebug(testClass)) + .collect(Collectors.toList()); + + expected = new String[]{ + "Deprecated method public void org.eclipse.jetty.xml.AnnotatedTestConfiguration.setTimeout(long)" + }; + + assertHasExpectedLines("Debugs", debugs, expected); + } + + @Test + public void testDeprecatedPropertySetToNewValue() throws Exception + { + Class testClass = AnnotatedTestConfiguration.class; + XmlConfiguration xmlConfiguration = asXmlConfiguration( + "" + + " " + + ""); + + List logLines; + try (StdErrCapture logCapture = new StdErrCapture(XmlConfiguration.class)) + { + // Leave this line alone, as this tests what happens if property is set, + // and has the same value as declared on + xmlConfiguration.getProperties().put("test.timeout", "30000"); + xmlConfiguration.configure(); + logLines = logCapture.getLines(); + } + + List warnings = logLines.stream() + .filter(LogPredicates.deprecatedWarnings(testClass)) + .collect(Collectors.toList()); + String[] expected = { + "Deprecated constructor public org.eclipse.jetty.xml.AnnotatedTestConfiguration", + "Deprecated method public void org.eclipse.jetty.xml.AnnotatedTestConfiguration.setTimeout(long)" + }; + assertThat("Count of warnings", warnings.size(), is(expected.length)); + for (int i = 0; i < expected.length; i++) + { + assertThat("Warning[" + i + "]", warnings.get(i), containsString(expected[i])); + } + } + + @Test + public void testSetDeprecatedMultipleProperties() throws Exception + { + Class testClass = AnnotatedTestConfiguration.class; + XmlConfiguration xmlConfiguration = asXmlConfiguration( + "" + + " " + + " " + + " " + + " " + + ""); + + List logLines; + try (StdErrCapture logCapture = new StdErrCapture(XmlConfiguration.class)) + { + // Leave this line alone, as this tests what happens if property is set, + // and has the same value as declared on + // xmlConfiguration.getProperties().put("obs.1", "30000"); + xmlConfiguration.configure(); + logLines = logCapture.getLines(); + } + + List warnings = logLines.stream() + .filter(LogPredicates.deprecatedWarnings(testClass)) + .collect(Collectors.toList()); + String[] expected = { + "Deprecated constructor public org.eclipse.jetty.xml.AnnotatedTestConfiguration", + "Deprecated field public java.lang.String org.eclipse.jetty.xml.AnnotatedTestConfiguration.obsolete" + }; + assertThat("Count of warnings", warnings.size(), is(expected.length)); + for (int i = 0; i < expected.length; i++) + { + assertThat("Warning[" + i + "]", warnings.get(i), containsString(expected[i])); + } + } + + private static class LogPredicates + { + public static Predicate deprecatedWarnings(Class testClass) + { + return (line) -> line.contains(":WARN:") && + line.contains(": Deprecated ") && + line.contains(testClass.getName()); + } + + public static Predicate deprecatedDebug(Class testClass) + { + return (line) -> line.contains(":DBUG:") && + line.contains(": Deprecated ") && + line.contains(testClass.getName()); + } + } + + private void assertHasExpectedLines(String type, List actualLines, String[] expectedLines) + { + assertThat("Count of " + type, actualLines.size(), is(expectedLines.length)); + for (int i = 0; i < expectedLines.length; i++) + { + assertThat(type + "[" + i + "]", actualLines.get(i), containsString(expectedLines[i])); + } + } + + private static class StdErrCapture implements AutoCloseable + { + private ByteArrayOutputStream logBytes; + private List loggers = new ArrayList<>(); + private final PrintStream logStream; + + public StdErrCapture(Class... classes) + { + for (Class clazz : classes) + { + Logger logger = Log.getLogger(clazz); + loggers.add(logger); + } + + logBytes = new ByteArrayOutputStream(); + logStream = new PrintStream(logBytes); + + loggers.forEach((logger) -> + { + logger.setDebugEnabled(true); + if (logger instanceof StdErrLog) + { + StdErrLog stdErrLog = (StdErrLog)logger; + stdErrLog.setStdErrStream(logStream); + } + }); + } + + public List getLines() throws UnsupportedEncodingException + { + logStream.flush(); String[] lines = logBytes.toString(UTF_8.name()).split(System.lineSeparator()); - List warnings = Arrays.stream(lines) - .filter(line -> line.contains(":WARN:")) - .filter(line -> line.contains(testClass.getSimpleName())) - .collect(Collectors.toList()); - // 1. Deprecated constructor - // 2. Deprecated method - // 3. Deprecated method - // 4. Deprecated method - // 5. Deprecated field - // 6. Deprecated field - assertEquals(6, warnings.size()); + return Arrays.asList(lines); + } + + @Override + public void close() + { + loggers.forEach((logger) -> logger.setDebugEnabled(false)); } } }