From 2073003a108cf0621d41790fd011c5bf51e065b0 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 22 Feb 2020 10:28:28 +0100 Subject: [PATCH 1/7] Issue #4550 XmlConfiguration argument matching Improve argument matching by: + rejecting obviously non matches (with allowance for unboxing) + sorting methods so that derived arguments are tried before more generic (eg String before Object) Signed-off-by: Greg Wilkins --- .../java/org/eclipse/jetty/util/TypeUtil.java | 27 ++++ .../eclipse/jetty/xml/XmlConfiguration.java | 123 ++++++++++++------ .../jetty/xml/AnnotatedTestConfiguration.java | 15 +++ .../jetty/xml/XmlConfigurationTest.java | 65 +++++++++ 4 files changed, 188 insertions(+), 42 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index bcbbd9103573..2fccddc570ae 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -60,6 +60,22 @@ public class TypeUtil private static final HashMap> name2Class = new HashMap<>(); + private static final Map, Class> __unbox; + + static + { + Map, Class> unbox = new HashMap<>(); + unbox.put(int.class, Integer.class); + unbox.put(long.class, Long.class); + unbox.put(byte.class, Byte.class); + unbox.put(char.class, Character.class); + unbox.put(float.class, Float.class); + unbox.put(double.class, Double.class); + unbox.put(short.class, Long.class); + unbox.put(boolean.class, Boolean.class); + __unbox = Collections.unmodifiableMap(unbox); + } + static { name2Class.put("boolean", java.lang.Boolean.TYPE); @@ -727,4 +743,15 @@ public static URI getModuleLocation(Class clazz) } return null; } + + public static boolean isUnboxable(Class type, Object arg) + { + if (!type.isPrimitive()) + return false; + if (arg == null) + return true; + + Class c = __unbox.get(type); + return arg.getClass() == __unbox.get(type); + } } 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 f8132f7ceb69..88c9afb1b79d 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 @@ -29,6 +29,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.lang.reflect.Parameter; import java.net.InetAddress; import java.net.MalformedURLException; import java.net.URL; @@ -99,20 +100,41 @@ public class XmlConfiguration }; private static final Iterable __factoryLoader = ServiceLoader.load(ConfigurationProcessorFactory.class); private static final XmlParser __parser = initParser(); - public static final Comparator EXECUTABLE_COMPARATOR = (o1, o2) -> + public static final Comparator EXECUTABLE_COMPARATOR = (e1, e2) -> { - int p1 = o1.getParameterCount(); - int p2 = o2.getParameterCount(); - int compare = Integer.compare(p1, p2); - if (compare == 0 && p1 > 0) + int count = e1.getParameterCount(); + int compare = Integer.compare(count, e2.getParameterCount()); + if (compare == 0 && count > 0) { - boolean a1 = o1.getParameterTypes()[p1 - 1].isArray(); - boolean a2 = o2.getParameterTypes()[p2 - 1].isArray(); - if (a1 && !a2) + Parameter[] p1 = e1.getParameters(); + Parameter[] p2 = e2.getParameters(); + boolean va1 = p1[count - 1].isVarArgs(); + boolean va2 = p2[count - 1].isVarArgs(); + if (va1 && !va2) compare = 1; - else if (!a1 && a2) + else if (!va1 && va2) compare = -1; + else + { + // Rank by assignability of the arguments + // This is only a rough guide that will favour call(String) vs call(Object), + // but it will not resolve call(String, Object) vs call (Object, String) + for (int i = 0; i < count; i++) + { + Class t1 = p1[i].getType(); + Class t2 = p2[i].getType(); + if (t1 != t2) + { + if (t1.isAssignableFrom(t2)) + compare++; + else if (t2.isAssignableFrom(t1)) + compare--; + } + } + } + compare = Math.min(1, Math.max(compare, -1)); } + return compare; }; @@ -1703,7 +1725,7 @@ Object[] applyTo(Executable executable) { // Could this be an empty varargs match? int count = executable.getParameterCount(); - if (count > 0 && executable.getParameterTypes()[count - 1].isArray()) + if (count > 0 && executable.getParameters()[count - 1].isVarArgs()) { // There is not a no varArgs alternative so let's try a an empty varArgs match args = asEmptyVarArgs(executable.getParameterTypes()[count - 1]).matchArgsToParameters(executable); @@ -1734,47 +1756,64 @@ Object[] matchArgsToParameters(Executable executable) return new Object[0]; // If no arg names are specified, keep the arg order + Object[] args; if (_names.stream().noneMatch(Objects::nonNull)) - return _arguments.toArray(new Object[0]); - - // If we don't have any parameters with names, then no match - Annotation[][] parameterAnnotations = executable.getParameterAnnotations(); - if (parameterAnnotations == null || parameterAnnotations.length == 0) - return null; - - // Find the position of all named parameters from the executable - Map position = new HashMap<>(); - int p = 0; - for (Annotation[] paramAnnotation : parameterAnnotations) { - Integer pos = p++; - Arrays.stream(paramAnnotation) - .filter(Name.class::isInstance) - .map(Name.class::cast) - .findFirst().ifPresent(n -> position.put(n.value(), pos)); + args = _arguments.toArray(new Object[0]); } - - List arguments = new ArrayList<>(_arguments); - List names = new ArrayList<>(_names); - // Map the actual arguments to the names - for (p = 0; p < count; p++) + else { - String name = names.get(p); - if (name != null) + // If we don't have any parameters with names, then no match + Annotation[][] parameterAnnotations = executable.getParameterAnnotations(); + if (parameterAnnotations == null || parameterAnnotations.length == 0) + return null; + + // Find the position of all named parameters from the executable + Map position = new HashMap<>(); + int p = 0; + for (Annotation[] paramAnnotation : parameterAnnotations) { - Integer pos = position.get(name); - if (pos == null) - return null; - if (pos != p) + Integer pos = p++; + Arrays.stream(paramAnnotation) + .filter(Name.class::isInstance) + .map(Name.class::cast) + .findFirst().ifPresent(n -> position.put(n.value(), pos)); + } + + List arguments = new ArrayList<>(_arguments); + List names = new ArrayList<>(_names); + // Map the actual arguments to the names + for (p = 0; p < count; p++) + { + String name = names.get(p); + if (name != null) { - // adjust position of parameter - arguments.add(pos, arguments.remove(p)); - names.add(pos, names.remove(p)); - p = Math.min(p, pos); + Integer pos = position.get(name); + if (pos == null) + return null; + if (pos != p) + { + // adjust position of parameter + arguments.add(pos, arguments.remove(p)); + names.add(pos, names.remove(p)); + p = Math.min(p, pos); + } } } + args = arguments.toArray(new Object[0]); } - return arguments.toArray(new Object[0]); + + // Check assignable + Parameter[] params = executable.getParameters(); + for (int i = 0; i < args.length; i++) + { + if (args[i] != null && + !params[i].getType().isAssignableFrom(args[i].getClass()) && + !TypeUtil.isUnboxable(params[i].getType(), args[i])) + return null; + } + + return args; } } } 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 d114fc3b78aa..a184d54bc156 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 @@ -92,6 +92,21 @@ public void setVarArgs(String first, String... theRest) this.third = theRest.length > 1 ? theRest[1] : null; } + public void call(Integer value) + { + this.first = String.valueOf(value); + } + + public void call(String value) + { + this.second = value; + } + + public void call(E value) + { + this.third = String.valueOf(value); + } + public String getFirst() { return first; 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 e730d35af1d2..98d2367ccca5 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 @@ -24,11 +24,13 @@ import java.io.PrintStream; import java.io.UnsupportedEncodingException; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; 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.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -43,7 +45,9 @@ import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.log.StdErrLog; import org.eclipse.jetty.util.resource.PathResource; +import org.hamcrest.Matchers; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; @@ -604,6 +608,67 @@ public void testNestedConstructorNamedInjectionOrdered() throws Exception assertEquals("arg3", atc.getNested().getThird(), "nested third parameter not wired correctly"); } + private static class TestOrder + { + public void call() + { + } + + public void call(Object o) + { + } + + public void call(String s) + { + } + + public void call(String... ss) + { + } + + public void call(String s, String... ss) + { + } + } + + @RepeatedTest(10) + public void testMethodOrdering() throws Exception + { + List methods = Arrays.stream(TestOrder.class.getMethods()).filter(m -> "call".equals(m.getName())).collect(Collectors.toList()); + Collections.shuffle(methods); + Collections.sort(methods, XmlConfiguration.EXECUTABLE_COMPARATOR); + assertThat(methods, Matchers.contains( + TestOrder.class.getMethod("call"), + TestOrder.class.getMethod("call", String.class), + TestOrder.class.getMethod("call", Object.class), + TestOrder.class.getMethod("call", String[].class), + TestOrder.class.getMethod("call", String.class, String[].class) + )); + } + + @Test + public void testOverloadedCall() throws Exception + { + XmlConfiguration xmlConfiguration = asXmlConfiguration( + "" + + " " + + " 1" + + " " + + " " + + " 2" + + " " + + " " + + " 3" + + " " + + ""); + + AnnotatedTestConfiguration atc = (AnnotatedTestConfiguration)xmlConfiguration.configure(); + + assertEquals("1", atc.getFirst()); + assertEquals("2", atc.getSecond()); + assertEquals("3", atc.getThird()); + } + @Test public void testNestedConstructorNamedInjectionUnOrdered() throws Exception { From f35522c6b42d0b757e899b2b8262a3c55c8e6f1d Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 22 Feb 2020 13:42:11 +0100 Subject: [PATCH 2/7] Issue #4550 XmlConfiguration argument matching Improve argument matching by: + can unbox from any Number to any Number Signed-off-by: Greg Wilkins --- jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index 2fccddc570ae..6ca9a524251a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -752,6 +752,7 @@ public static boolean isUnboxable(Class type, Object arg) return true; Class c = __unbox.get(type); - return arg.getClass() == __unbox.get(type); + Class ac = arg.getClass(); + return ac == __unbox.get(type) || (Number.class.isAssignableFrom(c) && Number.class.isAssignableFrom(ac)); } } From d6843a97416f7282556b23184b45808239f2cc3e Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 24 Feb 2020 19:49:47 +0100 Subject: [PATCH 3/7] Issue #4550 Do not check the assignability of the arguments. Instead rely on the order of the methods. Signed-off-by: Greg Wilkins --- .../java/org/eclipse/jetty/util/TypeUtil.java | 12 +++-- .../eclipse/jetty/xml/XmlConfiguration.java | 22 +++------- .../eclipse/jetty/xml/TestConfiguration.java | 11 +++++ .../jetty/xml/XmlConfigurationTest.java | 44 +++++++++++++++++++ 4 files changed, 66 insertions(+), 23 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index 6ca9a524251a..0b881f0f4b2c 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -65,14 +65,14 @@ public class TypeUtil static { Map, Class> unbox = new HashMap<>(); - unbox.put(int.class, Integer.class); - unbox.put(long.class, Long.class); + unbox.put(boolean.class, Boolean.class); unbox.put(byte.class, Byte.class); unbox.put(char.class, Character.class); + unbox.put(short.class, Short.class); + unbox.put(int.class, Integer.class); + unbox.put(long.class, Long.class); unbox.put(float.class, Float.class); unbox.put(double.class, Double.class); - unbox.put(short.class, Long.class); - unbox.put(boolean.class, Boolean.class); __unbox = Collections.unmodifiableMap(unbox); } @@ -751,8 +751,6 @@ public static boolean isUnboxable(Class type, Object arg) if (arg == null) return true; - Class c = __unbox.get(type); - Class ac = arg.getClass(); - return ac == __unbox.get(type) || (Number.class.isAssignableFrom(c) && Number.class.isAssignableFrom(ac)); + return __unbox.get(type) == arg.getClass(); } } 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 88c9afb1b79d..4bdeb7a512bf 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 @@ -117,8 +117,6 @@ else if (!va1 && va2) else { // Rank by assignability of the arguments - // This is only a rough guide that will favour call(String) vs call(Object), - // but it will not resolve call(String, Object) vs call (Object, String) for (int i = 0; i < count; i++) { Class t1 = p1[i].getType(); @@ -126,9 +124,12 @@ else if (!va1 && va2) if (t1 != t2) { if (t1.isAssignableFrom(t2)) - compare++; + compare = 1; else if (t2.isAssignableFrom(t1)) - compare--; + compare = -1; + else + compare = t1.getName().compareTo(t2.getName()); + break; } } } @@ -1803,16 +1804,6 @@ Object[] matchArgsToParameters(Executable executable) args = arguments.toArray(new Object[0]); } - // Check assignable - Parameter[] params = executable.getParameters(); - for (int i = 0; i < args.length; i++) - { - if (args[i] != null && - !params[i].getType().isAssignableFrom(args[i].getClass()) && - !TypeUtil.isUnboxable(params[i].getType(), args[i])) - return null; - } - return args; } } @@ -1835,9 +1826,8 @@ private static List getNodes(XmlParser.Node node, String element } } - for (int i = 0; i < node.size(); i++) + for (Object o : node) { - Object o = node.get(i); if (!(o instanceof XmlParser.Node)) continue; XmlParser.Node n = (XmlParser.Node)o; diff --git a/jetty-xml/src/test/java/org/eclipse/jetty/xml/TestConfiguration.java b/jetty-xml/src/test/java/org/eclipse/jetty/xml/TestConfiguration.java index 71e989cb5f28..bc4af813e5da 100644 --- a/jetty-xml/src/test/java/org/eclipse/jetty/xml/TestConfiguration.java +++ b/jetty-xml/src/test/java/org/eclipse/jetty/xml/TestConfiguration.java @@ -54,6 +54,7 @@ public class TestConfiguration extends HashMap private Set set; private ConstructorArgTestClass constructorArgTestClass; public Map map; + public Double number; public TestConfiguration() { @@ -65,6 +66,16 @@ public TestConfiguration(@Name("name") String n) name = n; } + public void setNumber(Object value) + { + testObject = value; + } + + public void setNumber(double value) + { + number = value; + } + public void setTest(Object value) { testObject = 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 98d2367ccca5..7eee97066eea 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 @@ -614,6 +614,10 @@ public void call() { } + public void call(int o) + { + } + public void call(Object o) { } @@ -639,6 +643,7 @@ public void testMethodOrdering() throws Exception Collections.sort(methods, XmlConfiguration.EXECUTABLE_COMPARATOR); assertThat(methods, Matchers.contains( TestOrder.class.getMethod("call"), + TestOrder.class.getMethod("call", int.class), TestOrder.class.getMethod("call", String.class), TestOrder.class.getMethod("call", Object.class), TestOrder.class.getMethod("call", String[].class), @@ -856,6 +861,45 @@ public void testCallMissingVarArgs() throws Exception assertNull(atc.getThird()); } + public static class NumberTypeProvider implements ArgumentsProvider + { + @Override + public Stream provideArguments(ExtensionContext context) + { + return Stream.of( + "byte", + "int", + // "char", + "short", + "long", + "float", + "double", + "Byte", + "Integer", + // "Character", + "Short", + "Long", + "Float", + "Double" + ).map(Arguments::of); + } + } + + @ParameterizedTest + @ArgumentsSource(NumberTypeProvider.class) + public void testCallNumberConversion(String type) throws Exception + { + XmlConfiguration xmlConfiguration = asXmlConfiguration( + "" + + " " + + " 42" + + " " + + ""); + + TestConfiguration tc = (TestConfiguration)xmlConfiguration.configure(); + assertEquals(42.0D, tc.number); + } + @Test public void testArgumentsGetIgnoredMissingDTD() throws Exception { From 3728314890d19a41e18a89955f1e03e35bf88ea6 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 24 Feb 2020 19:50:25 +0100 Subject: [PATCH 4/7] Issue #4550 unbox test no longer required Signed-off-by: Greg Wilkins --- .../java/org/eclipse/jetty/util/TypeUtil.java | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java index 0b881f0f4b2c..bcbbd9103573 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java @@ -60,22 +60,6 @@ public class TypeUtil private static final HashMap> name2Class = new HashMap<>(); - private static final Map, Class> __unbox; - - static - { - Map, Class> unbox = new HashMap<>(); - unbox.put(boolean.class, Boolean.class); - unbox.put(byte.class, Byte.class); - unbox.put(char.class, Character.class); - unbox.put(short.class, Short.class); - unbox.put(int.class, Integer.class); - unbox.put(long.class, Long.class); - unbox.put(float.class, Float.class); - unbox.put(double.class, Double.class); - __unbox = Collections.unmodifiableMap(unbox); - } - static { name2Class.put("boolean", java.lang.Boolean.TYPE); @@ -743,14 +727,4 @@ public static URI getModuleLocation(Class clazz) } return null; } - - public static boolean isUnboxable(Class type, Object arg) - { - if (!type.isPrimitive()) - return false; - if (arg == null) - return true; - - return __unbox.get(type) == arg.getClass(); - } } From 626fe7156fee8eefe4cb19680c502a8ab43d2d06 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 24 Feb 2020 23:08:44 +0100 Subject: [PATCH 5/7] Issue #4550 Simplified test Signed-off-by: Greg Wilkins --- .../jetty/xml/XmlConfigurationTest.java | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) 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 7eee97066eea..ff780086d865 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 @@ -55,6 +55,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; +import org.junit.jupiter.params.provider.MethodSource; import org.xml.sax.SAXException; import static java.nio.charset.StandardCharsets.UTF_8; @@ -861,32 +862,25 @@ public void testCallMissingVarArgs() throws Exception assertNull(atc.getThird()); } - public static class NumberTypeProvider implements ArgumentsProvider + public static List typeTestData() { - @Override - public Stream provideArguments(ExtensionContext context) - { - return Stream.of( - "byte", - "int", - // "char", - "short", - "long", - "float", - "double", - "Byte", - "Integer", - // "Character", - "Short", - "Long", - "Float", - "Double" - ).map(Arguments::of); - } + return Arrays.asList( + "byte", + "int", + "short", + "long", + "float", + "double", + "Byte", + "Integer", + "Short", + "Long", + "Float", + "Double"); } @ParameterizedTest - @ArgumentsSource(NumberTypeProvider.class) + @MethodSource("typeTestData") public void testCallNumberConversion(String type) throws Exception { XmlConfiguration xmlConfiguration = asXmlConfiguration( From 39dd24528688930539f09c09d71dcd1b478a4518 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 25 Feb 2020 10:14:57 +0100 Subject: [PATCH 6/7] Issue #4550 Cleanup comparator Signed-off-by: Greg Wilkins --- .../eclipse/jetty/xml/XmlConfiguration.java | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) 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 4bdeb7a512bf..c08bce027807 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 @@ -102,34 +102,39 @@ public class XmlConfiguration private static final XmlParser __parser = initParser(); public static final Comparator EXECUTABLE_COMPARATOR = (e1, e2) -> { + // Favour methods with less parameters int count = e1.getParameterCount(); int compare = Integer.compare(count, e2.getParameterCount()); if (compare == 0 && count > 0) { Parameter[] p1 = e1.getParameters(); Parameter[] p2 = e2.getParameters(); - boolean va1 = p1[count - 1].isVarArgs(); - boolean va2 = p2[count - 1].isVarArgs(); - if (va1 && !va2) - compare = 1; - else if (!va1 && va2) - compare = -1; - else + + // Favour methods without varargs + compare = Boolean.compare(p1[count - 1].isVarArgs(), p2[count - 1].isVarArgs()); + if (compare == 0) { - // Rank by assignability of the arguments + // Rank by differences in the parameters for (int i = 0; i < count; i++) { Class t1 = p1[i].getType(); Class t2 = p2[i].getType(); if (t1 != t2) { - if (t1.isAssignableFrom(t2)) - compare = 1; - else if (t2.isAssignableFrom(t1)) - compare = -1; - else - compare = t1.getName().compareTo(t2.getName()); - break; + // Favour derived type over base type + compare = Boolean.compare(t1.isAssignableFrom(t2), t2.isAssignableFrom(t1)); + if (compare == 0) + { + // favour primitive type over reference + compare = Boolean.compare(t2.isPrimitive(), t1.isPrimitive()); + if (compare == 0) + // Use name to avoid non determinant sorting + compare = t1.getName().compareTo(t2.getName()); + } + + // break on the first different parameter (should always be true) + if (compare != 0) + break; } } } From a95e40e01e5b42e7deeb66106478e1e1b23ce437 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 25 Feb 2020 16:17:33 +0100 Subject: [PATCH 7/7] Issue #4550 Cleanup comparator Signed-off-by: Greg Wilkins --- .../src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c08bce027807..286e28b8c394 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 @@ -126,7 +126,7 @@ public class XmlConfiguration if (compare == 0) { // favour primitive type over reference - compare = Boolean.compare(t2.isPrimitive(), t1.isPrimitive()); + compare = Boolean.compare(!t1.isPrimitive(), !t2.isPrimitive()); if (compare == 0) // Use name to avoid non determinant sorting compare = t1.getName().compareTo(t2.getName());