backported @PathVariable matching fix (SPR-8543)

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -430,10 +430,19 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 		return mav;
 	}
 
+	/**
+	 * This method always returns -1 since an annotated controller can have many methods,
+	 * each requiring separate lastModified calculations. Instead, an
+	 * @{@link RequestMapping}-annotated method can calculate the lastModified value, call
+	 * {@link org.springframework.web.context.request.WebRequest#checkNotModified(long)}
+	 * to check it, and return {@code null} if that returns {@code true}.
+	 * @see org.springframework.web.context.request.WebRequest#checkNotModified(long)
+	 */
 	public long getLastModified(HttpServletRequest request, Object handler) {
 		return -1;
 	}
 
+
 	/**
 	 * Build a HandlerMethodResolver for the given handler type.
 	 */
@@ -463,8 +472,7 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 	 * @see ServletRequestDataBinder#bind(javax.servlet.ServletRequest)
 	 * @see ServletRequestDataBinder#convertIfNecessary(Object, Class, org.springframework.core.MethodParameter) 
 	 */
-	protected ServletRequestDataBinder createBinder(HttpServletRequest request, Object target, String objectName)
+	protected ServletRequestDataBinder createBinder(HttpServletRequest request, Object target, String objectName) throws Exception {
-			throws Exception {
 		return new ServletRequestDataBinder(target, objectName);
 	}
 
@@ -620,21 +628,20 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 			}
 			else {
 				if (!allowedMethods.isEmpty()) {
-					throw new HttpRequestMethodNotSupportedException(request.getMethod(),
+					throw new HttpRequestMethodNotSupportedException(request.getMethod(), StringUtils.toStringArray(allowedMethods));
-							StringUtils.toStringArray(allowedMethods));
 				}
-				throw new NoSuchRequestHandlingMethodException(lookupPath, request.getMethod(),
+				throw new NoSuchRequestHandlingMethodException(lookupPath, request.getMethod(), request.getParameterMap());
-						request.getParameterMap());
 			}
 		}
 
 		/**
 		 * Determines the combined pattern for the given methodLevelPattern and path.
-		 * <p>Uses the following algorithm: <ol>
+		 * <p>Uses the following algorithm:
+		 * <ol>
 		 * <li>If there is a type-level mapping with path information, it is {@linkplain
 		 * PathMatcher#combine(String, String) combined} with the method-level pattern.</li>
-		 * <li>If there is a {@linkplain HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE best matching pattern} in the
+		 * <li>If there is a {@linkplain HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE best matching pattern}
-		 * request, it is combined with the method-level pattern.</li>
+		 * in the request, it is combined with the method-level pattern.</li>
 		 * <li>Otherwise, the method-level pattern is returned.</li>
 		 * </ol>
 		 */
@@ -646,8 +653,9 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 						typeLevelPattern = "/" + typeLevelPattern;
 					}
 					String combinedPattern = pathMatcher.combine(typeLevelPattern, methodLevelPattern);
-					if (isPathMatchInternal(combinedPattern, lookupPath)) {
+					String matchingPattern = getMatchingPattern(combinedPattern, lookupPath);
-						return combinedPattern;
+					if (matchingPattern != null) {
+						return matchingPattern;
 					}
 				}
 				return null;
@@ -655,44 +663,44 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator
 			String bestMatchingPattern = (String) request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
 			if (StringUtils.hasText(bestMatchingPattern) && bestMatchingPattern.endsWith("*")) {
 				String combinedPattern = pathMatcher.combine(bestMatchingPattern, methodLevelPattern);
-				if (!combinedPattern.equals(bestMatchingPattern) &&
+				String matchingPattern = getMatchingPattern(combinedPattern, lookupPath);
-						(isPathMatchInternal(combinedPattern, lookupPath))) {
+				if (matchingPattern != null && !matchingPattern.equals(bestMatchingPattern)) {
-					return combinedPattern;
+					return matchingPattern;
 				}
 			}
-			if (isPathMatchInternal(methodLevelPattern, lookupPath)) {
+			return getMatchingPattern(methodLevelPattern, lookupPath);
-				return methodLevelPattern;
-			}
-			return null;
 		}
 
-		private boolean isPathMatchInternal(String pattern, String lookupPath) {
+		private String getMatchingPattern(String pattern, String lookupPath) {
-			if (pattern.equals(lookupPath) || pathMatcher.match(pattern, lookupPath)) {
+			if (pattern.equals(lookupPath)) {
-				return true;
+				return pattern;
 			}
 			boolean hasSuffix = pattern.indexOf('.') != -1;
-			if (!hasSuffix && pathMatcher.match(pattern + ".*", lookupPath)) {
+			if (!hasSuffix) {
-				return true;
+				String patternWithSuffix = pattern + ".*";
+				if (pathMatcher.match(patternWithSuffix, lookupPath)) {
+					return patternWithSuffix;
+				}
+			}
+			if (pathMatcher.match(pattern, lookupPath)) {
+				return pattern;
 			}
 			boolean endsWithSlash = pattern.endsWith("/");
-			if (!endsWithSlash && pathMatcher.match(pattern + "/", lookupPath)) {
+			if (!endsWithSlash) {
-				return true;
+				String patternWithSlash = pattern + "/";
+				if (pathMatcher.match(patternWithSlash, lookupPath)) {
+					return patternWithSlash;
 				}
-			return false;
+			}
+			return null;
 		}
 
 		@SuppressWarnings("unchecked")
-		private void extractHandlerMethodUriTemplates(String mappedPattern,
+		private void extractHandlerMethodUriTemplates(String mappedPattern, String lookupPath, HttpServletRequest request) {
-				String lookupPath,
-				HttpServletRequest request) {
-
 			Map<String, String> variables =
 					(Map<String, String>) request.getAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE);
-
 			int patternVariableCount = StringUtils.countOccurrencesOf(mappedPattern, "{");
-			
+			if ((variables == null || patternVariableCount != variables.size()) && pathMatcher.match(mappedPattern, lookupPath)) {
-			if ( (variables == null || patternVariableCount != variables.size())  
-					&& pathMatcher.match(mappedPattern, lookupPath)) {
 				variables = pathMatcher.extractUriTemplateVariables(mappedPattern, lookupPath);
 				request.setAttribute(HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE, variables);
 			}

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/UriTemplateServletAnnotationControllerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2011 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,7 +23,6 @@ import java.util.Date;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletResponse;
 
-import static org.junit.Assert.*;
 import org.junit.Test;
 
 import org.springframework.beans.BeansException;
@@ -43,7 +42,12 @@ import org.springframework.web.context.support.GenericWebApplicationContext;
 import org.springframework.web.servlet.DispatcherServlet;
 import org.springframework.web.servlet.mvc.support.ControllerClassNameHandlerMapping;
 
-/** @author Arjen Poutsma */
+import static org.junit.Assert.*;
+
+/**
+ * @author Arjen Poutsma
+ * @author Rossen Stoyanchev
+ */
 public class UriTemplateServletAnnotationControllerTests {
 
 	private DispatcherServlet servlet;
@@ -309,9 +313,7 @@ public class UriTemplateServletAnnotationControllerTests {
 		assertEquals("test-42", response.getContentAsString());
 	}
 
-	/*
+	// SPR-6640
-	 * See SPR-6640 
-	 */
 	@Test
 	public void menuTree() throws Exception {
 		initServlet(MenuTreeController.class);
@@ -322,9 +324,7 @@ public class UriTemplateServletAnnotationControllerTests {
 		assertEquals("M5", response.getContentAsString());
 	}
 
-	/*
+	// SPR-6876
-	 * See SPR-6876
-	 */
 	@Test
 	public void variableNames() throws Exception {
 		initServlet(VariableNamesController.class);
@@ -340,9 +340,18 @@ public class UriTemplateServletAnnotationControllerTests {
 		assertEquals("bar-bar", response.getContentAsString());
 	}
 
-	/*
+	// SPR-8543
-	 * See SPR-6906
+	@Test
-	 */
+	public void variableNamesWithUrlExtension() throws Exception {
+		initServlet(VariableNamesController.class);
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/test/foo.json");
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals("foo-foo", response.getContentAsString());
+	}
+
+	// SPR-6906
 	@Test
 	public void controllerClassName() throws Exception {
 		servlet = new DispatcherServlet() {
@@ -376,9 +385,7 @@ public class UriTemplateServletAnnotationControllerTests {
 		assertEquals("plain-bar", response.getContentAsString());
 	}
 
-	/*
+	// SPR-6978
-	 * See SPR-6978
-	 */
 	@Test
 	public void doIt() throws Exception {
 		initServlet(Spr6978Controller.class);
@@ -406,10 +413,7 @@ public class UriTemplateServletAnnotationControllerTests {
 	}
 
 
-
+	// Controllers
-	/*
-	 * Controllers
-	 */
 
 	@Controller
 	public static class SimpleUriTemplateController {
@@ -548,7 +552,6 @@ public class UriTemplateServletAnnotationControllerTests {
 
 	}
 
-
 	@Controller
 	@RequestMapping("hotels")
 	public static class CrudController {
@@ -676,5 +679,4 @@ public class UriTemplateServletAnnotationControllerTests {
 		}
 	}
 
-
 }