Improve no-match handling for @RequestMapping methods

Issue: SPR-9603

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java

@@ -152,30 +152,47 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe
 	 */
 	@Override
 	protected HandlerMethod handleNoMatch(Set<RequestMappingInfo> requestMappingInfos,
-										  String lookupPath,
+			String lookupPath, HttpServletRequest request) throws ServletException {
-										  HttpServletRequest request) throws ServletException {
+
 		Set<String> allowedMethods = new HashSet<String>(6);
-		Set<MediaType> consumableMediaTypes = new HashSet<MediaType>();
+
-		Set<MediaType> producibleMediaTypes = new HashSet<MediaType>();
+		Set<RequestMappingInfo> patternMatches = new HashSet<RequestMappingInfo>();
+		Set<RequestMappingInfo> patternAndMethodMatches = new HashSet<RequestMappingInfo>();
+
 		for (RequestMappingInfo info : requestMappingInfos) {
 			if (info.getPatternsCondition().getMatchingCondition(request) != null) {
-				if (info.getMethodsCondition().getMatchingCondition(request) == null) {
+				patternMatches.add(info);
+				if (info.getMethodsCondition().getMatchingCondition(request) != null) {
+					patternAndMethodMatches.add(info);
+				}
+				else {
 					for (RequestMethod method : info.getMethodsCondition().getMethods()) {
 						allowedMethods.add(method.name());
 					}
 				}
-				if (info.getConsumesCondition().getMatchingCondition(request) == null) {
-					consumableMediaTypes.addAll(info.getConsumesCondition().getConsumableMediaTypes());
-				}
-				if (info.getProducesCondition().getMatchingCondition(request) == null) {
-					producibleMediaTypes.addAll(info.getProducesCondition().getProducibleMediaTypes());
-				}
 			}
 		}
-		if (!allowedMethods.isEmpty()) {
+
+		if (patternMatches.isEmpty()) {
+			return null;
+		}
+		else if (patternAndMethodMatches.isEmpty() && !allowedMethods.isEmpty()) {
 			throw new HttpRequestMethodNotSupportedException(request.getMethod(), allowedMethods);
 		}
-		else if (!consumableMediaTypes.isEmpty()) {
+
+		Set<MediaType> consumableMediaTypes;
+		Set<MediaType> producibleMediaTypes;
+
+		if (patternAndMethodMatches.isEmpty()) {
+			consumableMediaTypes = getConsumableMediaTypes(request, patternMatches);
+			producibleMediaTypes = getProdicubleMediaTypes(request, patternMatches);
+		}
+		else {
+			consumableMediaTypes = getConsumableMediaTypes(request, patternAndMethodMatches);
+			producibleMediaTypes = getProdicubleMediaTypes(request, patternAndMethodMatches);
+		}
+
+		if (!consumableMediaTypes.isEmpty()) {
 			MediaType contentType = null;
 			if (StringUtils.hasLength(request.getContentType())) {
 				contentType = MediaType.parseMediaType(request.getContentType());
@@ -190,4 +207,24 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe
 		}
 	}
 
+	private Set<MediaType> getConsumableMediaTypes(HttpServletRequest request, Set<RequestMappingInfo> partialMatches) {
+		Set<MediaType> result = new HashSet<MediaType>();
+		for (RequestMappingInfo partialMatch : partialMatches) {
+			if (partialMatch.getConsumesCondition().getMatchingCondition(request) == null) {
+				result.addAll(partialMatch.getConsumesCondition().getConsumableMediaTypes());
+			}
+		}
+		return result;
+	}
+
+	private Set<MediaType> getProdicubleMediaTypes(HttpServletRequest request, Set<RequestMappingInfo> partialMatches) {
+		Set<MediaType> result = new HashSet<MediaType>();
+		for (RequestMappingInfo partialMatch : partialMatches) {
+			if (partialMatch.getProducesCondition().getMatchingCondition(request) == null) {
+				result.addAll(partialMatch.getProducesCondition().getProducibleMediaTypes());
+			}
+		}
+		return result;
+	}
+
 }

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java

@@ -70,8 +70,6 @@ public class RequestMappingInfoHandlerMappingTests {
 
 	private TestRequestMappingInfoHandlerMapping handlerMapping;
 
-	private Handler handler;
-
 	private HandlerMethod fooMethod;
 
 	private HandlerMethod fooParamMethod;
@@ -82,14 +80,15 @@ public class RequestMappingInfoHandlerMappingTests {
 
 	@Before
 	public void setUp() throws Exception {
-		this.handler = new Handler();
+		TestController testController = new TestController();
-		this.fooMethod = new HandlerMethod(handler, "foo");
+
-		this.fooParamMethod = new HandlerMethod(handler, "fooParam");
+		this.fooMethod = new HandlerMethod(testController, "foo");
-		this.barMethod = new HandlerMethod(handler, "bar");
+		this.fooParamMethod = new HandlerMethod(testController, "fooParam");
-		this.emptyMethod = new HandlerMethod(handler, "empty");
+		this.barMethod = new HandlerMethod(testController, "bar");
+		this.emptyMethod = new HandlerMethod(testController, "empty");
 
 		this.handlerMapping = new TestRequestMappingInfoHandlerMapping();
-		this.handlerMapping.registerHandler(this.handler);
+		this.handlerMapping.registerHandler(testController);
 		this.handlerMapping.setRemoveSemicolonContent(false);
 	}
 
@@ -148,10 +147,23 @@ public class RequestMappingInfoHandlerMappingTests {
 		}
 	}
 
+	// SPR-9603
+
+	@Test(expected=HttpMediaTypeNotAcceptableException.class)
+	public void requestMethodMatchFalsePositive() throws Exception {
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/users");
+		request.addHeader("Accept", "application/xml");
+
+		this.handlerMapping.registerHandler(new UserController());
+		this.handlerMapping.getHandler(request);
+	}
+
 	@Test
 	public void mediaTypeNotSupported() throws Exception {
 		testMediaTypeNotSupported("/person/1");
-		testMediaTypeNotSupported("/person/1/");	// SPR-8462
+
+		// SPR-8462
+		testMediaTypeNotSupported("/person/1/");
 		testMediaTypeNotSupported("/person/1.json");
 	}
 
@@ -171,7 +183,9 @@ public class RequestMappingInfoHandlerMappingTests {
 	@Test
 	public void mediaTypeNotAccepted() throws Exception {
 		testMediaTypeNotAccepted("/persons");
-		testMediaTypeNotAccepted("/persons/");	// SPR-8462
+
+		// SPR-8462
+		testMediaTypeNotAccepted("/persons/");
 		testMediaTypeNotAccepted("/persons.json");
 	}
 
@@ -276,7 +290,7 @@ public class RequestMappingInfoHandlerMappingTests {
 		MappedInterceptor mappedInterceptor = new MappedInterceptor(new String[] {path}, interceptor);
 
 		TestRequestMappingInfoHandlerMapping hm = new TestRequestMappingInfoHandlerMapping();
-		hm.registerHandler(this.handler);
+		hm.registerHandler(new TestController());
 		hm.setInterceptors(new Object[] { mappedInterceptor });
 		hm.setApplicationContext(new StaticWebApplicationContext());
 
@@ -358,7 +372,7 @@ public class RequestMappingInfoHandlerMappingTests {
 
 
 	@Controller
-	private static class Handler {
+	private static class TestController {
 
 		@RequestMapping(value = "/foo", method = RequestMethod.GET)
 		public void foo() {
@@ -396,6 +410,18 @@ public class RequestMappingInfoHandlerMappingTests {
 		}
 	}
 
+	@Controller
+	private static class UserController {
+
+		@RequestMapping(value = "/users", method = RequestMethod.GET, produces = "application/json")
+		public void getUser() {
+		}
+
+		@RequestMapping(value = "/users", method = RequestMethod.PUT)
+		public void saveUser() {
+		}
+	}
+
 	private static class TestRequestMappingInfoHandlerMapping extends RequestMappingInfoHandlerMapping {
 
 		public void registerHandler(Object handler) {