SPR-9062 Fix bug with ambiguous path and HTTP method request mappings

A direct path match with incorrect HTTP request method was causing another
request mapping with a pattern and a correct HTTP method to be ignored.
The bug affects the new @MVC support classes
(i.e. RequestMappingHandlerMapping).

build-spring-framework/resources/changelog.txt

@@ -28,6 +28,7 @@ Changes in version 3.1.1 (2012-02-06)
 * preserve quotes in MediaType parameters
 * make flash attributes available in the model of ParameterizableViewController and UrlFilenameViewController
 * add property to RedirectView to disable expanding URI variables in redirect URL
+* fix request mapping bug involving direct vs pattern path matches with HTTP methods
 
 Changes in version 3.1 GA (2011-12-12)
 --------------------------------------

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java

@@ -18,12 +18,14 @@ package org.springframework.web.servlet.handler;
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 
@@ -237,18 +239,16 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
 	 * @see #handleNoMatch(Set, String, HttpServletRequest)
 	 */
 	protected HandlerMethod lookupHandlerMethod(String lookupPath, HttpServletRequest request) throws Exception {
-		List<T> mappings = urlMap.get(lookupPath);
-		if (mappings == null) {
-			mappings = new ArrayList<T>(handlerMethods.keySet());
-		}
-			
 		List<Match> matches = new ArrayList<Match>();
-		
+
-		for (T mapping : mappings) {
+		List<T> directPathMatches = this.urlMap.get(lookupPath);
-			T match = getMatchingMapping(mapping, request);
+		if (directPathMatches != null) {
-			if (match != null) {
+			addMatchingMappings(directPathMatches, matches, request);
-				matches.add(new Match(match, handlerMethods.get(mapping)));
+		}
-			}
+
+		if (matches.isEmpty()) {
+			// No choice but to go through all mappings
+			addMatchingMappings(this.handlerMethods.keySet(), matches, request);
 		}
 
 		if (!matches.isEmpty()) {
@@ -279,6 +279,15 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
 		}
 	}
 
+	private void addMatchingMappings(Collection<T> mappings, List<Match> matches, HttpServletRequest request) {
+		for (T mapping : mappings) {
+			T match = getMatchingMapping(mapping, request);
+			if (match != null) {
+				matches.add(new Match(match, handlerMethods.get(mapping)));
+			}
+		}
+	}
+
 	/**
 	 * Check if a mapping matches the current request and return a (potentially 
 	 * new) mapping with conditions relevant to the current request.

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java

@@ -1182,6 +1182,19 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		assertEquals("myParam-42", response.getContentAsString());
 	}
 
+	// SPR-9062
+
+	@Test
+	public void ambiguousPathAndRequestMethod() throws Exception {
+		initServletWithControllers(AmbiguousPathAndRequestMethodController.class);
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bug/EXISTING");
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		getServlet().service(request, response);
+		assertEquals(200, response.getStatus());
+		assertEquals("Pattern", response.getContentAsString());
+	}
+
 	@Test
 	public void bridgeMethods() throws Exception {
 		initServletWithControllers(TestControllerImpl.class);
@@ -2549,6 +2562,20 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		}
 	}
 
+	@Controller
+	static class AmbiguousPathAndRequestMethodController {
+
+		@RequestMapping(value = "/bug/EXISTING", method = RequestMethod.POST)
+		public void directMatch(Writer writer) throws IOException {
+			writer.write("Direct");
+		}
+
+		@RequestMapping(value = "/bug/{type}", method = RequestMethod.GET)
+		public void patternMatch(Writer writer) throws IOException {
+			writer.write("Pattern");
+		}
+	}
+
 	@Controller
 	@RequestMapping("/test*")
 	public static class BindingCookieValueController {