SPR-6640 - PathVariable does not work properly after updated to 3.0 GA

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

@@ -37,47 +37,37 @@ import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.servlet.handler.AbstractDetectingUrlHandlerMapping;
 
 /**
- * Implementation of the {@link org.springframework.web.servlet.HandlerMapping}
+ * Implementation of the {@link org.springframework.web.servlet.HandlerMapping} interface that maps handlers based on
- * interface that maps handlers based on HTTP paths expressed through the
+ * HTTP paths expressed through the {@link RequestMapping} annotation at the type or method level.
- * {@link RequestMapping} annotation at the type or method level.
  *
- * <p>Registered by default in {@link org.springframework.web.servlet.DispatcherServlet}
+ * <p>Registered by default in {@link org.springframework.web.servlet.DispatcherServlet} on Java 5+. <b>NOTE:</b> If you
- * on Java 5+. <b>NOTE:</b> If you define custom HandlerMapping beans in your
+ * define custom HandlerMapping beans in your DispatcherServlet context, you need to add a
- * DispatcherServlet context, you need to add a DefaultAnnotationHandlerMapping bean
+ * DefaultAnnotationHandlerMapping bean explicitly, since custom HandlerMapping beans replace the default mapping
- * explicitly, since custom HandlerMapping beans replace the default mapping strategies.
+ * strategies. Defining a DefaultAnnotationHandlerMapping also allows for registering custom interceptors:
- * Defining a DefaultAnnotationHandlerMapping also allows for registering custom
- * interceptors:
  *
- * <pre class="code">
+ * <pre class="code"> &lt;bean class="org.springframework.web.servlet.mvc.annotation.DefaultAnnotationHandlerMapping"&gt;
- * &lt;bean class="org.springframework.web.servlet.mvc.annotation.DefaultAnnotationHandlerMapping"&gt;
+ * &lt;property name="interceptors"&gt; ... &lt;/property&gt; &lt;/bean&gt;</pre>
- *   &lt;property name="interceptors"&gt;
- *     ...
- *   &lt;/property&gt;
- * &lt;/bean&gt;</pre>
  *
- * Annotated controllers are usually marked with the {@link Controller} stereotype
+ * Annotated controllers are usually marked with the {@link Controller} stereotype at the type level. This is not
- * at the type level. This is not strictly necessary when {@link RequestMapping} is
+ * strictly necessary when {@link RequestMapping} is applied at the type level (since such a handler usually implements
- * applied at the type level (since such a handler usually implements the
+ * the {@link org.springframework.web.servlet.mvc.Controller} interface). However, {@link Controller} is required for
- * {@link org.springframework.web.servlet.mvc.Controller} interface). However,
+ * detecting {@link RequestMapping} annotations at the method level if {@link RequestMapping} is not present at the type
- * {@link Controller} is required for detecting {@link RequestMapping} annotations
+ * level.
- * at the method level if {@link RequestMapping} is not present at the type level.
  *
- * <p><b>NOTE:</b> Method-level mappings are only allowed to narrow the mapping
+ * <p><b>NOTE:</b> Method-level mappings are only allowed to narrow the mapping expressed at the class level (if any).
- * expressed at the class level (if any). HTTP paths need to uniquely map onto
+ * HTTP paths need to uniquely map onto specific handler beans, with any given HTTP path only allowed to be mapped onto
- * specific handler beans, with any given HTTP path only allowed to be mapped
+ * one specific handler bean (not spread across multiple handler beans). It is strongly recommended to co-locate related
- * onto one specific handler bean (not spread across multiple handler beans).
+ * handler methods into the same bean.
- * It is strongly recommended to co-locate related handler methods into the same bean.
  *
- * <p>The {@link AnnotationMethodHandlerAdapter} is responsible for processing
+ * <p>The {@link AnnotationMethodHandlerAdapter} is responsible for processing annotated handler methods, as mapped by
- * annotated handler methods, as mapped by this HandlerMapping. For
+ * this HandlerMapping. For {@link RequestMapping} at the type level, specific HandlerAdapters such as {@link
- * {@link RequestMapping} at the type level, specific HandlerAdapters such as
+ * org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter} apply.
- * {@link org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter} apply.
  *
  * @author Juergen Hoeller
  * @author Arjen Poutsma
- * @since 2.5
  * @see RequestMapping
  * @see AnnotationMethodHandlerAdapter
+ * @since 2.5
  */
 public class DefaultAnnotationHandlerMapping extends AbstractDetectingUrlHandlerMapping {
 
@@ -85,23 +75,19 @@ public class DefaultAnnotationHandlerMapping extends AbstractDetectingUrlHandler
 
 	private final Map<Class, RequestMapping> cachedMappings = new HashMap<Class, RequestMapping>();
 
-
 	/**
-	 * Set whether to register paths using the default suffix pattern as well:
+	 * Set whether to register paths using the default suffix pattern as well: i.e. whether "/users" should be registered
-	 * i.e. whether "/users" should be registered as "/users.*" and "/users/" too.
+	 * as "/users.*" and "/users/" too. <p>Default is "true". Turn this convention off if you intend to interpret your
-	 * <p>Default is "true". Turn this convention off if you intend to interpret
+	 * <code>@RequestMapping</code> paths strictly. <p>Note that paths which include a ".xxx" suffix or end with "/"
-	 * your <code>@RequestMapping</code> paths strictly.
+	 * already will not be transformed using the default suffix pattern in any case.
-	 * <p>Note that paths which include a ".xxx" suffix or end with "/" already will not be
-	 * transformed using the default suffix pattern in any case.
 	 */
 	public void setUseDefaultSuffixPattern(boolean useDefaultSuffixPattern) {
 		this.useDefaultSuffixPattern = useDefaultSuffixPattern;
 	}
 
-
 	/**
-	 * Checks for presence of the {@link org.springframework.web.bind.annotation.RequestMapping}
+	 * Checks for presence of the {@link org.springframework.web.bind.annotation.RequestMapping} annotation on the handler
-	 * annotation on the handler class and on any of its methods.
+	 * class and on any of its methods.
 	 */
 	@Override
 	protected String[] determineUrlsForHandler(String beanName) {
@@ -120,12 +106,20 @@ public class DefaultAnnotationHandlerMapping extends AbstractDetectingUrlHandler
 					if (!typeLevelPattern.startsWith("/")) {
 						typeLevelPattern = "/" + typeLevelPattern;
 					}
+					boolean hasEmptyMethodLevelMappings = false;
 					for (String methodLevelPattern : methodLevelPatterns) {
+						if (methodLevelPattern == null) {
+							hasEmptyMethodLevelMappings = true;
+						} else {
 							String combinedPattern = getPathMatcher().combine(typeLevelPattern, methodLevelPattern);
 							addUrlsForPath(urls, combinedPattern);
 						}
+					}
+					if (hasEmptyMethodLevelMappings ||
+							org.springframework.web.servlet.mvc.Controller.class.isAssignableFrom(handlerType)) {
 						addUrlsForPath(urls, typeLevelPattern);
 					}
+				}
 				return StringUtils.toStringArray(urls);
 			}
 			else {
@@ -144,6 +138,9 @@ public class DefaultAnnotationHandlerMapping extends AbstractDetectingUrlHandler
 
 	/**
 	 * Derive URL mappings from the handler's method-level mappings.
+	 *
+	 * <p>The returned array may contain {@code null}, indicating an empty {@link RequestMapping} value.
+	 *
 	 * @param handlerType the handler type to introspect
 	 * @return the array of mapped URLs
 	 */
@@ -156,19 +153,25 @@ public class DefaultAnnotationHandlerMapping extends AbstractDetectingUrlHandler
 				public void doWith(Method method) {
 					RequestMapping mapping = AnnotationUtils.findAnnotation(method, RequestMapping.class);
 					if (mapping != null) {
-						String[] mappedPaths = mapping.value();
+						String[] mappedPatterns = mapping.value();
-						for (String mappedPath : mappedPaths) {
+						if (mappedPatterns.length > 0) {
-							addUrlsForPath(urls, mappedPath);
+							for (String mappedPattern : mappedPatterns) {
+								addUrlsForPath(urls, mappedPattern);
+							}
+						} else {
+							// empty method-level RequestMapping
+							urls.add(null);
 						}
 					}
 				}
-			});
+			}, ReflectionUtils.NON_BRIDGED_METHODS);
 		}
 		return StringUtils.toStringArray(urls);
 	}
 
 	/**
 	 * Add URLs and/or URL patterns for the given path.
+	 *
 	 * @param urls the Set of URLs for the current bean
 	 * @param path the currently introspected path
 	 */
@@ -180,9 +183,9 @@ public class DefaultAnnotationHandlerMapping extends AbstractDetectingUrlHandler
 		}
 	}
 
-
 	/**
 	 * Validate the given annotated handler against the current request.
+	 *
 	 * @see #validateMapping
 	 */
 	@Override
@@ -197,8 +200,9 @@ public class DefaultAnnotationHandlerMapping extends AbstractDetectingUrlHandler
 	}
 
 	/**
-	 * Validate the given type-level mapping metadata against the current request,
+	 * Validate the given type-level mapping metadata against the current request, checking HTTP request method and
-	 * checking HTTP request method and parameter conditions.
+	 * parameter conditions.
+	 *
 	 * @param mapping the mapping metadata to validate
 	 * @param request current HTTP request
 	 * @throws Exception if validation failed
@@ -220,8 +224,8 @@ public class DefaultAnnotationHandlerMapping extends AbstractDetectingUrlHandler
 
 		String[] mappedHeaders = mapping.headers();
 		if (!ServletAnnotationMappingUtils.checkHeaders(mappedHeaders, request)) {
-			throw new ServletRequestBindingException("Header conditions \"" +
+			throw new ServletRequestBindingException(
-					StringUtils.arrayToDelimitedString(mappedHeaders, ", ") +
+					"Header conditions \"" + StringUtils.arrayToDelimitedString(mappedHeaders, ", ") +
 							"\" not met for actual request");
 		}
 	}

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

@@ -308,6 +308,19 @@ public class UriTemplateServletAnnotationControllerTests {
 		assertEquals("test-42", response.getContentAsString());
 	}
 
+	/*
+	 * See SPR-6640 
+	 */
+	@Test
+	public void menuTree() throws Exception {
+		initServlet(MenuTreeController.class);
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/book/menu/type/M5");
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		servlet.service(request, response);
+		assertEquals("M5", response.getContentAsString());
+	}
+
 
 	/*
 	 * Controllers
@@ -533,4 +546,14 @@ public class UriTemplateServletAnnotationControllerTests {
 
 	}
 
+	@RequestMapping("/*/menu/**")
+	public static class MenuTreeController {
+
+		@RequestMapping("type/{var}")
+		public void getFirstLevelFunctionNodes(@PathVariable("var") String var, Writer writer) throws IOException {
+			writer.write(var);
+		}
+	}
+
+
 }