From 7605707c18d64979c9ba6652037f4ab519ffe096 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 27 Jul 2011 22:31:17 +0000 Subject: [PATCH] backported @PathVariable matching fix (SPR-8543) --- .../AnnotationMethodHandlerAdapter.java | 88 ++++++++++--------- ...plateServletAnnotationControllerTests.java | 44 +++++----- 2 files changed, 71 insertions(+), 61 deletions(-) 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 d0916c2aaa..8441928619 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 @@ -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) - throws Exception { + protected ServletRequestDataBinder createBinder(HttpServletRequest request, Object target, String objectName) 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(), - StringUtils.toStringArray(allowedMethods)); + throw new HttpRequestMethodNotSupportedException(request.getMethod(), StringUtils.toStringArray(allowedMethods)); } - throw new NoSuchRequestHandlingMethodException(lookupPath, request.getMethod(), - request.getParameterMap()); + throw new NoSuchRequestHandlingMethodException(lookupPath, request.getMethod(), request.getParameterMap()); } } /** * Determines the combined pattern for the given methodLevelPattern and path. - *

Uses the following algorithm:

    + *

    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. - *
    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. + *
    3. If there is a {@linkplain HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE best matching pattern} + * in the request, it is combined with the method-level pattern.
      4. *
    4. Otherwise, the method-level pattern is returned.
      5. *
    */ @@ -646,8 +653,9 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator typeLevelPattern = "/" + typeLevelPattern; } String combinedPattern = pathMatcher.combine(typeLevelPattern, methodLevelPattern); - if (isPathMatchInternal(combinedPattern, lookupPath)) { - return combinedPattern; + String matchingPattern = getMatchingPattern(combinedPattern, lookupPath); + 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) && - (isPathMatchInternal(combinedPattern, lookupPath))) { - return combinedPattern; + String matchingPattern = getMatchingPattern(combinedPattern, lookupPath); + if (matchingPattern != null && !matchingPattern.equals(bestMatchingPattern)) { + return matchingPattern; } } - if (isPathMatchInternal(methodLevelPattern, lookupPath)) { - return methodLevelPattern; + return getMatchingPattern(methodLevelPattern, lookupPath); + } + + private String getMatchingPattern(String pattern, String lookupPath) { + if (pattern.equals(lookupPath)) { + return pattern; + } + boolean hasSuffix = pattern.indexOf('.') != -1; + if (!hasSuffix) { + String patternWithSuffix = pattern + ".*"; + if (pathMatcher.match(patternWithSuffix, lookupPath)) { + return patternWithSuffix; + } + } + if (pathMatcher.match(pattern, lookupPath)) { + return pattern; + } + boolean endsWithSlash = pattern.endsWith("/"); + if (!endsWithSlash) { + String patternWithSlash = pattern + "/"; + if (pathMatcher.match(patternWithSlash, lookupPath)) { + return patternWithSlash; + } } return null; } - private boolean isPathMatchInternal(String pattern, String lookupPath) { - if (pattern.equals(lookupPath) || pathMatcher.match(pattern, lookupPath)) { - return true; - } - boolean hasSuffix = pattern.indexOf('.') != -1; - if (!hasSuffix && pathMatcher.match(pattern + ".*", lookupPath)) { - return true; - } - boolean endsWithSlash = pattern.endsWith("/"); - if (!endsWithSlash && pathMatcher.match(pattern + "/", lookupPath)) { - return true; - } - return false; - } - @SuppressWarnings("unchecked") - private void extractHandlerMethodUriTemplates(String mappedPattern, - String lookupPath, - HttpServletRequest request) { - + private void extractHandlerMethodUriTemplates(String mappedPattern, String lookupPath, HttpServletRequest request) { Map variables = (Map) 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); } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/UriTemplateServletAnnotationControllerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/UriTemplateServletAnnotationControllerTests.java index 2b436092e3..e5edf988d8 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/UriTemplateServletAnnotationControllerTests.java +++ b/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()); } - /* - * See SPR-6640 - */ + // SPR-6640 @Test public void menuTree() throws Exception { initServlet(MenuTreeController.class); @@ -322,9 +324,7 @@ public class UriTemplateServletAnnotationControllerTests { assertEquals("M5", response.getContentAsString()); } - /* - * See SPR-6876 - */ + // SPR-6876 @Test public void variableNames() throws Exception { initServlet(VariableNamesController.class); @@ -340,9 +340,18 @@ public class UriTemplateServletAnnotationControllerTests { assertEquals("bar-bar", response.getContentAsString()); } - /* - * See SPR-6906 - */ + // SPR-8543 + @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()); } - /* - * See SPR-6978 - */ + // 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 { } } - }