From 8c4b1ab781c305328eb98ce7b2eb39f8b57c4753 Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@pivotal.io>
Date: Mon, 14 Aug 2017 16:32:40 +0300
Subject: [PATCH] Functional PathPattern comparator

---
 .../web/util/pattern/PathPattern.java         | 58 ++++++-------------
 .../web/util/pattern/PathPatternTests.java    |  8 +--
 2 files changed, 22 insertions(+), 44 deletions(-)

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 29045392fe..c442696945 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
@@ -692,44 +692,22 @@ public class PathPattern implements Comparable<PathPattern> {
 	}
 
 
-	public static final Comparator<PathPattern> SPECIFICITY_COMPARATOR = (p1, p2) -> {
-		// Same object or null == null?
-		if (p1 == p2) {
-			return 0;
-		}
-
-		// null is sorted last
-		if (p2 == null) {
-			return -1;
-		}
-
-		// catchall patterns are sorted last. If both catchall then the
-		// length is considered
-		if (p1.isCatchAll()) {
-			if (p2.isCatchAll()) {
-				int lenDifference = p1.getNormalizedLength() - p2.getNormalizedLength();
-				if (lenDifference != 0) {
-					return (lenDifference < 0) ? +1 : -1;
-				}
-			}
-			else {
-				return +1;
-			}
-		}
-		else if (p2.isCatchAll()) {
-			return -1;
-		}
-
-		// 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;
-		}
-
-		// longer is better
-		int lenDifference = p1.getNormalizedLength() - p2.getNormalizedLength();
-		return Integer.compare(0, lenDifference);
-	};
-
+	/**
+	 * Comparator that sorts patterns by specificity as follows:
+	 * <ol>
+	 * <li>Null instances are last.
+	 * <li>Catch-all patterns are last.
+	 * <li>If both patterns are catch-all, consider the length (longer wins).
+	 * <li>Compare wildcard and captured variable count (lower wins).
+	 * <li>Consider length (longer wins)
+	 * </ol>
+	 */
+	public static final Comparator<PathPattern> SPECIFICITY_COMPARATOR =
+			Comparator.nullsLast(
+					Comparator.<PathPattern>
+							comparingInt(p -> p.isCatchAll() ? 1 : 0)
+							.thenComparingInt(p -> p.isCatchAll() ? -1 * p.getNormalizedLength() : 0)
+							.thenComparing(PathPattern::getScore)
+							.thenComparingInt(p -> -1 * p.getNormalizedLength())
+			);
 }
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 56cd5d48de..bc82ea94cf 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
@@ -967,10 +967,10 @@ public class PathPatternTests {
 	}
 
 	@Test
-	public void patternCompareToNull() {
-		PathPatternParser p = new PathPatternParser();
-		PathPattern pp = p.parse("/abc");
-		assertEquals(-1, pp.compareTo(null));
+	public void patternCompareWithNull() {
+		assertTrue(PathPattern.SPECIFICITY_COMPARATOR.compare(null, null) == 0);
+		assertTrue(PathPattern.SPECIFICITY_COMPARATOR.compare(parse("/abc"), null) < 0);
+		assertTrue(PathPattern.SPECIFICITY_COMPARATOR.compare(null, parse("/abc")) > 0);
 	}
 
 	@Test