Polishing

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java

@@ -113,25 +113,25 @@ public static void clearParsedRequestPath(ServletRequest request) {
 	// Methods to select either parsed RequestPath or resolved String lookupPath
 
 	/**
-	 * Return the {@link UrlPathHelper#resolveAndCacheLookupPath pre-resolved}
-	 * String lookupPath or the {@link #parseAndCache(HttpServletRequest)
+	 * Return either a {@link UrlPathHelper#resolveAndCacheLookupPath pre-resolved}
+	 * String lookupPath or a {@link #parseAndCache(HttpServletRequest)
 	 * pre-parsed} {@code RequestPath}.
 	 * <p>In Spring MVC, when at least one {@code HandlerMapping} has parsed
-	 * {@code PathPatterns} enabled, the {@code DispatcherServlet} eagerly parses
-	 * and caches the {@code RequestPath} and the same can be also done earlier with
+	 * {@code PathPatterns} enabled, the {@code DispatcherServlet} parses and
+	 * caches the {@code RequestPath} which can be also done even earlier with
 	 * {@link org.springframework.web.filter.ServletRequestPathFilter
 	 * ServletRequestPathFilter}. In other cases where {@code HandlerMapping}s
 	 * use String pattern matching with {@code PathMatcher}, the String
-	 * lookupPath is resolved separately by each {@code HandlerMapping}.
+	 * lookupPath is resolved separately in each {@code HandlerMapping}.
 	 * @param request the current request
 	 * @return a String lookupPath or a {@code RequestPath}
 	 * @throws IllegalArgumentException if neither is available
 	 */
 	public static Object getCachedPath(ServletRequest request) {
 
-		// The RequestPath is pre-parsed if any HandlerMapping uses PathPatterns.
-		// The lookupPath is re-resolved or cleared per HandlerMapping.
-		// So check for lookupPath first.
+		// RequestPath is parsed once and cached in the DispatcherServlet if any HandlerMapping uses PathPatterns.
+		// A String lookupPath is resolved and cached in each HandlerMapping that uses String matching.
+		// So we try lookupPath first, then RequestPath second
 
 		String lookupPath = (String) request.getAttribute(UrlPathHelper.PATH_ATTRIBUTE);
 		if (lookupPath != null) {
@@ -246,41 +246,24 @@ public String toString() {
 
 		public static RequestPath parse(HttpServletRequest request) {
 			String requestUri = (String) request.getAttribute(WebUtils.INCLUDE_REQUEST_URI_ATTRIBUTE);
-			if (requestUri == null) {
-				requestUri = request.getRequestURI();
-			}
-
-			String servletPathPrefix = Servlet4Delegate.getServletPathPrefix(request);
-			if (StringUtils.hasText(servletPathPrefix)) {
-				if (servletPathPrefix.endsWith("/")) {
-					servletPathPrefix = servletPathPrefix.substring(0, servletPathPrefix.length() - 1);
-				}
-				return new ServletRequestPath(requestUri, request.getContextPath(), servletPathPrefix);
-			}
-
-			return RequestPath.parse(requestUri, request.getContextPath());
+			requestUri = (requestUri != null ? requestUri : request.getRequestURI());
+			String servletPathPrefix = getServletPathPrefix(request);
+			return (StringUtils.hasText(servletPathPrefix) ?
+					new ServletRequestPath(requestUri, request.getContextPath(), servletPathPrefix) :
+					RequestPath.parse(requestUri, request.getContextPath()));
 		}
-	}
-
-
-	/**
-	 * Inner class to avoid a hard dependency on Servlet 4 {@link HttpServletMapping}
-	 * and {@link MappingMatch} at runtime.
-	 */
-	private static class Servlet4Delegate {
 
 		@Nullable
-		public static String getServletPathPrefix(HttpServletRequest request) {
+		private static String getServletPathPrefix(HttpServletRequest request) {
 			HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING);
-			if (mapping == null) {
-				mapping = request.getHttpServletMapping();
-			}
-			if (!ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) {
-				return null;
+			mapping = (mapping != null ? mapping : request.getHttpServletMapping());
+			if (ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) {
+				String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE);
+				servletPath = (servletPath != null ? servletPath : request.getServletPath());
+				servletPath = (servletPath.endsWith("/") ? servletPath.substring(0, servletPath.length() - 1) : servletPath);
+				return UriUtils.encodePath(servletPath, StandardCharsets.UTF_8);
 			}
-			String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE);
-			servletPath = (servletPath != null ? servletPath : request.getServletPath());
-			return UriUtils.encodePath(servletPath, StandardCharsets.UTF_8);
+			return null;
 		}
 	}
 

spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java

@@ -242,7 +242,7 @@ public String getLookupPathForRequest(HttpServletRequest request, @Nullable Stri
 	public String getLookupPathForRequest(HttpServletRequest request) {
 		String pathWithinApp = getPathWithinApplication(request);
 		// Always use full path within current servlet context?
-		if (this.alwaysUseFullPath || skipServletPathDetermination(request)) {
+		if (this.alwaysUseFullPath || ignoreServletPath(request)) {
 			return pathWithinApp;
 		}
 		// Else, use path within current servlet mapping if applicable
@@ -256,16 +256,13 @@ public String getLookupPathForRequest(HttpServletRequest request) {
 	}
 
 	/**
-	 * Check whether servlet path determination can be skipped for the given request.
-	 * @param request current HTTP request
-	 * @return {@code true} if the request mapping has not been achieved using a path
-	 * or if the servlet has been mapped to root; {@code false} otherwise
+	 * Whether we can ignore the servletPath and pathInfo for mapping purposes,
+	 * which is the case when we can establish that the Servlet is not mapped
+	 * by servletPath prefix.
 	 */
-	private boolean skipServletPathDetermination(HttpServletRequest request) {
+	private boolean ignoreServletPath(HttpServletRequest request) {
 		HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING);
-		if (mapping == null) {
-			mapping = request.getHttpServletMapping();
-		}
+		mapping = (mapping == null ? request.getHttpServletMapping() : mapping);
 		MappingMatch match = mapping.getMappingMatch();
 		return (match != null && (!match.equals(MappingMatch.PATH) || mapping.getPattern().equals("/*")));
 	}