From 89975c8b79f052fa218e247d3c33e3ad1fb1bcf5 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Wed, 25 Nov 2009 11:12:03 +0000 Subject: [PATCH] SPR-6378 - RC2: Issue with RequestMethod.GET differs from M2 --- .../web/servlet/HandlerMapping.java | 11 ++- .../handler/AbstractUrlHandlerMapping.java | 50 +++++++------ .../AnnotationMethodHandlerAdapter.java | 37 ++++++++-- .../mvc/annotation/BookController.java | 52 +++++++++++++ .../ServletAnnotationControllerTests.java | 73 ++++++++++++++++++- 5 files changed, 191 insertions(+), 32 deletions(-) create mode 100644 org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/BookController.java diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/HandlerMapping.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/HandlerMapping.java index a447cd8b2f..76e3746308 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/HandlerMapping.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/HandlerMapping.java @@ -64,6 +64,16 @@ public interface HandlerMapping { */ String PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE = HandlerMapping.class.getName() + ".pathWithinHandlerMapping"; + /** + * Name of the {@link HttpServletRequest} attribute that contains the + * best matching pattern within the handler mapping. + *

Note: This attribute is not required to be supported by all + * HandlerMapping implementations. URL-based HandlerMappings will + * typically support it, but handlers should not necessarily expect + * this request attribute to be present in all scenarios. + */ + String BEST_MATCHING_PATTERN_ATTRIBUTE = HandlerMapping.class.getName() + ".bestMatchingPattern"; + /** * Name of the {@link HttpServletRequest} attribute that contains the URI * templates map, mapping variable names to values. @@ -74,7 +84,6 @@ public interface HandlerMapping { */ String URI_TEMPLATE_VARIABLES_ATTRIBUTE = HandlerMapping.class.getName() + ".uriTemplateVariables"; - /** * Return a handler and any interceptors for this request. The choice may be made * on request URL, session state, or any factor the implementing class chooses. diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java index 7aee2d76f0..534399c5b5 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/handler/AbstractUrlHandlerMapping.java @@ -178,7 +178,7 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { rawHandler = getApplicationContext().getBean(handlerName); } validateHandler(rawHandler, request); - handler = buildPathExposingHandler(rawHandler, lookupPath, null); + handler = buildPathExposingHandler(rawHandler, lookupPath, lookupPath, null); } } if (handler != null && logger.isDebugEnabled()) { @@ -213,35 +213,35 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { handler = getApplicationContext().getBean(handlerName); } validateHandler(handler, request); - return buildPathExposingHandler(handler, urlPath, null); + return buildPathExposingHandler(handler, urlPath, urlPath, null); } // Pattern match? - List matchingPaths = new ArrayList(); - for (String registeredPath : this.handlerMap.keySet()) { - if (getPathMatcher().match(registeredPath, urlPath)) { - matchingPaths.add(registeredPath); + List matchingPatterns = new ArrayList(); + for (String registeredPattern : this.handlerMap.keySet()) { + if (getPathMatcher().match(registeredPattern, urlPath)) { + matchingPatterns.add(registeredPattern); } } - String bestPathMatch = null; - if (!matchingPaths.isEmpty()) { - Collections.sort(matchingPaths, getPathMatcher().getPatternComparator(urlPath)); + String bestPatternMatch = null; + if (!matchingPatterns.isEmpty()) { + Collections.sort(matchingPatterns, getPathMatcher().getPatternComparator(urlPath)); if (logger.isDebugEnabled()) { - logger.debug("Matching path for request [" + urlPath + "] are " + matchingPaths); + logger.debug("Matching patterns for request [" + urlPath + "] are " + matchingPatterns); } - bestPathMatch = matchingPaths.get(0); + bestPatternMatch = matchingPatterns.get(0); } - if (bestPathMatch != null) { - handler = this.handlerMap.get(bestPathMatch); + if (bestPatternMatch != null) { + handler = this.handlerMap.get(bestPatternMatch); // Bean name or resolved handler? if (handler instanceof String) { String handlerName = (String) handler; handler = getApplicationContext().getBean(handlerName); } validateHandler(handler, request); - String pathWithinMapping = getPathMatcher().extractPathWithinPattern(bestPathMatch, urlPath); + String pathWithinMapping = getPathMatcher().extractPathWithinPattern(bestPatternMatch, urlPath); Map uriTemplateVariables = - getPathMatcher().extractUriTemplateVariables(bestPathMatch, urlPath); - return buildPathExposingHandler(handler, pathWithinMapping, uriTemplateVariables); + getPathMatcher().extractUriTemplateVariables(bestPatternMatch, urlPath); + return buildPathExposingHandler(handler, bestPatternMatch, pathWithinMapping, uriTemplateVariables); } // No handler found... return null; @@ -269,11 +269,13 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { * @param uriTemplateVariables the URI template variables, can be null if no variables found * @return the final handler object */ - protected Object buildPathExposingHandler( - Object rawHandler, String pathWithinMapping, Map uriTemplateVariables) { + protected Object buildPathExposingHandler(Object rawHandler, + String bestMatchingPattern, + String pathWithinMapping, + Map uriTemplateVariables) { HandlerExecutionChain chain = new HandlerExecutionChain(rawHandler); - chain.addInterceptor(new PathExposingHandlerInterceptor(pathWithinMapping)); + chain.addInterceptor(new PathExposingHandlerInterceptor(bestMatchingPattern, pathWithinMapping)); if (!CollectionUtils.isEmpty(uriTemplateVariables)) { chain.addInterceptor(new UriTemplateVariablesHandlerInterceptor(uriTemplateVariables)); } @@ -286,7 +288,8 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { * @param request the request to expose the path to * @see #PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE */ - protected void exposePathWithinMapping(String pathWithinMapping, HttpServletRequest request) { + protected void exposePathWithinMapping(String bestMatchingPattern, String pathWithinMapping, HttpServletRequest request) { + request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, bestMatchingPattern); request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, pathWithinMapping); } @@ -384,15 +387,18 @@ public abstract class AbstractUrlHandlerMapping extends AbstractHandlerMapping { */ private class PathExposingHandlerInterceptor extends HandlerInterceptorAdapter { + private final String bestMatchingPattern; + private final String pathWithinMapping; - private PathExposingHandlerInterceptor(String pathWithinMapping) { + private PathExposingHandlerInterceptor(String bestMatchingPattern, String pathWithinMapping) { + this.bestMatchingPattern = bestMatchingPattern; this.pathWithinMapping = pathWithinMapping; } @Override public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { - exposePathWithinMapping(this.pathWithinMapping, request); + exposePathWithinMapping(this.bestMatchingPattern, this.pathWithinMapping, request); return true; } } diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java index f0bcfda163..13b81c0fa1 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java @@ -432,7 +432,7 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator implemen if (mappingInfo.paths.length > 0) { List matchedPaths = new ArrayList(mappingInfo.paths.length); for (String methodLevelPattern : mappingInfo.paths) { - String matchedPattern = getMatchedPattern(methodLevelPattern, lookupPath); + String matchedPattern = getMatchedPattern(methodLevelPattern, lookupPath, request); if (matchedPattern != null) { if (mappingInfo.matches(request)) { match = true; @@ -518,12 +518,23 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator implemen } } - private String getMatchedPattern(String methodLevelPattern, String lookupPath) { - if ((!hasTypeLevelMapping() || ObjectUtils.isEmpty(getTypeLevelMapping().value())) && - isPathMatchInternal(methodLevelPattern, lookupPath)) { - return methodLevelPattern; - } - if (hasTypeLevelMapping()) { + /** + * Determines the matched pattern for the given methodLevelPattern and path. + * + *

Uses the following algorithm: + *

    + *
  1. If there is a type-level mapping with path information, it is + * {@linkplain PathMatcher#combine(String, String) combined} with the method-level pattern. + *
  2. If there is a {@linkplain HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE best matching pattern} in the + * request, it is combined with the method-level pattern. + *
  3. Otherwise, + * @param methodLevelPattern + * @param lookupPath + * @param request + * @return + */ + private String getMatchedPattern(String methodLevelPattern, String lookupPath, HttpServletRequest request) { + if (hasTypeLevelMapping() && (!ObjectUtils.isEmpty(getTypeLevelMapping().value()))) { String[] typeLevelPatterns = getTypeLevelMapping().value(); for (String typeLevelPattern : typeLevelPatterns) { if (!typeLevelPattern.startsWith("/")) { @@ -534,7 +545,17 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator implemen return combinedPattern; } } - + return null; + } + String bestMatchingPattern = (String) request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + if (StringUtils.hasText(bestMatchingPattern)) { + String combinedPattern = pathMatcher.combine(bestMatchingPattern, methodLevelPattern); + if (!combinedPattern.equals(bestMatchingPattern) && (isPathMatchInternal(combinedPattern, lookupPath))) { + return combinedPattern; + } + } + if (isPathMatchInternal(methodLevelPattern, lookupPath)) { + return methodLevelPattern; } return null; } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/BookController.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/BookController.java new file mode 100644 index 0000000000..996dc252b4 --- /dev/null +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/BookController.java @@ -0,0 +1,52 @@ +/* + * Copyright 2002-2009 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.servlet.mvc.annotation; + +import java.io.IOException; +import java.io.Writer; + +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RequestParam; + +/** + * Used for testing the combination of ControllerClassNameHandlerMapping/SimpleUrlHandlerMapping with @RequestParam in + * {@link ServletAnnotationControllerTests}. Implemented as a top-level class (rather than an inner class) to make the + * ControllerClassNameHandlerMapping work. + * + * @author Arjen Poutsma + */ +@Controller +public class BookController { + + @RequestMapping("list") + public void list(Writer writer) throws IOException { + writer.write("list"); + } + + @RequestMapping("show") + public void show(@RequestParam(required = true) Long id, Writer writer) throws IOException { + writer.write("show-id=" + id); + } + + @RequestMapping(method = RequestMethod.POST) + public void create(Writer writer) throws IOException { + writer.write("create"); + } + +} diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java index ad855f88c1..26e8760e50 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java @@ -55,6 +55,7 @@ import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.beans.DerivedTestBean; import org.springframework.beans.ITestBean; import org.springframework.beans.TestBean; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer; @@ -107,6 +108,7 @@ import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewResolver; +import org.springframework.web.servlet.handler.SimpleUrlHandlerMapping; import org.springframework.web.servlet.mvc.AbstractController; import org.springframework.web.servlet.mvc.multiaction.InternalPathMethodNameResolver; import org.springframework.web.servlet.mvc.support.ControllerClassNameHandlerMapping; @@ -1159,6 +1161,75 @@ public class ServletAnnotationControllerTests { assertEquals("Content-Type=[text/html],Custom-Header=[value21,value22]", response.getContentAsString()); } + @Test + public void controllerClassNameNoTypeLevelAnn() throws Exception { + servlet = new DispatcherServlet() { + @Override + protected WebApplicationContext createWebApplicationContext(WebApplicationContext parent) + throws BeansException { + GenericWebApplicationContext wac = new GenericWebApplicationContext(); + wac.registerBeanDefinition("controller", new RootBeanDefinition(BookController.class)); + RootBeanDefinition mapping = new RootBeanDefinition(ControllerClassNameHandlerMapping.class); + mapping.getPropertyValues().add("excludedPackages", null); + wac.registerBeanDefinition("handlerMapping", mapping); + wac.refresh(); + return wac; + } + }; + servlet.init(new MockServletConfig()); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/book/list"); + MockHttpServletResponse response = new MockHttpServletResponse(); + servlet.service(request, response); + assertEquals("list", response.getContentAsString()); + + request = new MockHttpServletRequest("GET", "/book/show"); + request.addParameter("id", "12"); + response = new MockHttpServletResponse(); + servlet.service(request, response); + assertEquals("show-id=12", response.getContentAsString()); + + request = new MockHttpServletRequest("POST", "/book"); + response = new MockHttpServletResponse(); + servlet.service(request, response); + assertEquals("create", response.getContentAsString()); + } + + @Test + public void simpleUrlHandlerMapping() throws Exception { + servlet = new DispatcherServlet() { + @Override + protected WebApplicationContext createWebApplicationContext(WebApplicationContext parent) + throws BeansException { + GenericWebApplicationContext wac = new GenericWebApplicationContext(); + wac.registerBeanDefinition("controller", new RootBeanDefinition(BookController.class)); + RootBeanDefinition hmDef = new RootBeanDefinition(SimpleUrlHandlerMapping.class); + hmDef.getPropertyValues().add("mappings", "/book/*=controller\n/book=controller"); + wac.registerBeanDefinition("handlerMapping", hmDef); + wac.refresh(); + return wac; + } + }; + servlet.init(new MockServletConfig()); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/book/list"); + MockHttpServletResponse response = new MockHttpServletResponse(); + servlet.service(request, response); + assertEquals("list", response.getContentAsString()); + + request = new MockHttpServletRequest("GET", "/book/show"); + request.addParameter("id", "12"); + response = new MockHttpServletResponse(); + servlet.service(request, response); + assertEquals("show-id=12", response.getContentAsString()); + + request = new MockHttpServletRequest("POST", "/book"); + response = new MockHttpServletResponse(); + servlet.service(request, response); + assertEquals("create", response.getContentAsString()); + } + + /* * Controllers @@ -2033,5 +2104,5 @@ public class ServletAnnotationControllerTests { } - + }