diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java index 77946ee7bcb..29045392fe7 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPattern.java @@ -693,12 +693,17 @@ public class PathPattern implements Comparable { public static final Comparator SPECIFICITY_COMPARATOR = (p1, p2) -> { - // 1) null is sorted last + // Same object or null == null? + if (p1 == p2) { + return 0; + } + + // null is sorted last if (p2 == null) { return -1; } - // 2) catchall patterns are sorted last. If both catchall then the + // catchall patterns are sorted last. If both catchall then the // length is considered if (p1.isCatchAll()) { if (p2.isCatchAll()) { @@ -715,14 +720,14 @@ public class PathPattern implements Comparable { return -1; } - // 3) This will sort such that if they differ in terms of wildcards or + // This will sort such that if they differ in terms of wildcards or // captured variable counts, the one with the most will be sorted last int score = p1.getScore() - p2.getScore(); if (score != 0) { return (score < 0) ? -1 : +1; } - // 4) longer is better + // longer is better int lenDifference = p1.getNormalizedLength() - p2.getNormalizedLength(); return Integer.compare(0, lenDifference); }; diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java index 0ff812d1cbd..56cd5d48de0 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java @@ -886,10 +886,7 @@ public class PathPatternTests { @Test public void patternComparator() { - Comparator comparator = (p1, p2) -> { - int index = p1.compareTo(p2); - return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString())); - }; + Comparator comparator = PathPattern.SPECIFICITY_COMPARATOR; assertEquals(0, comparator.compare(parse("/hotels/new"), parse("/hotels/new"))); @@ -970,7 +967,7 @@ public class PathPatternTests { } @Test - public void patternCompareTo() { + public void patternCompareToNull() { PathPatternParser p = new PathPatternParser(); PathPattern pp = p.parse("/abc"); assertEquals(-1, pp.compareTo(null)); @@ -978,58 +975,62 @@ public class PathPatternTests { @Test public void patternComparatorSort() { - Comparator comparator = (p1, p2) -> { - int index = p1.compareTo(p2); - return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString())); - }; + Comparator comparator = PathPattern.SPECIFICITY_COMPARATOR; List paths = new ArrayList<>(3); PathPatternParser pp = new PathPatternParser(); + paths.add(null); + paths.add(null); + paths.sort(comparator); + assertNull(paths.get(0)); + assertNull(paths.get(1)); + paths.clear(); + paths.add(null); paths.add(pp.parse("/hotels/new")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/new", paths.get(0).getPatternString()); assertNull(paths.get(1)); paths.clear(); paths.add(pp.parse("/hotels/*")); paths.add(pp.parse("/hotels/new")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/new", paths.get(0).getPatternString()); assertEquals("/hotels/*", paths.get(1).getPatternString()); paths.clear(); paths.add(pp.parse("/hotels/new")); paths.add(pp.parse("/hotels/*")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/new", paths.get(0).getPatternString()); assertEquals("/hotels/*", paths.get(1).getPatternString()); paths.clear(); paths.add(pp.parse("/hotels/**")); paths.add(pp.parse("/hotels/*")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/*", paths.get(0).getPatternString()); assertEquals("/hotels/**", paths.get(1).getPatternString()); paths.clear(); paths.add(pp.parse("/hotels/*")); paths.add(pp.parse("/hotels/**")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/*", paths.get(0).getPatternString()); assertEquals("/hotels/**", paths.get(1).getPatternString()); paths.clear(); paths.add(pp.parse("/hotels/{hotel}")); paths.add(pp.parse("/hotels/new")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/new", paths.get(0).getPatternString()); assertEquals("/hotels/{hotel}", paths.get(1).getPatternString()); paths.clear(); paths.add(pp.parse("/hotels/new")); paths.add(pp.parse("/hotels/{hotel}")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/new", paths.get(0).getPatternString()); assertEquals("/hotels/{hotel}", paths.get(1).getPatternString()); paths.clear(); @@ -1037,7 +1038,7 @@ public class PathPatternTests { paths.add(pp.parse("/hotels/*")); paths.add(pp.parse("/hotels/{hotel}")); paths.add(pp.parse("/hotels/new")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/new", paths.get(0).getPatternString()); assertEquals("/hotels/{hotel}", paths.get(1).getPatternString()); assertEquals("/hotels/*", paths.get(2).getPatternString()); @@ -1046,7 +1047,7 @@ public class PathPatternTests { paths.add(pp.parse("/hotels/ne*")); paths.add(pp.parse("/hotels/n*")); Collections.shuffle(paths); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/hotels/ne*", paths.get(0).getPatternString()); assertEquals("/hotels/n*", paths.get(1).getPatternString()); paths.clear(); @@ -1066,7 +1067,7 @@ public class PathPatternTests { }; paths.add(pp.parse("/*/login.*")); paths.add(pp.parse("/*/endUser/action/login.*")); - Collections.sort(paths, comparator); + paths.sort(comparator); assertEquals("/*/endUser/action/login.*", paths.get(0).getPatternString()); assertEquals("/*/login.*", paths.get(1).getPatternString()); paths.clear();