From e4fada56ab2e254004001f3773b00bdf9db1840b Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 18 Nov 2011 11:32:01 +0000 Subject: [PATCH] SPR-8859 Fix issue with prototype controllers in RequestMappingHandlerAdapter. --- .../resources/changelog.txt | 1 + .../RequestMappingHandlerAdapter.java | 58 +++++++++---------- ...MappingHandlerAdapterIntegrationTests.java | 17 +++--- ...nnotationControllerHandlerMethodTests.java | 46 +++++++++++++++ 4 files changed, 83 insertions(+), 39 deletions(-) diff --git a/build-spring-framework/resources/changelog.txt b/build-spring-framework/resources/changelog.txt index 4ca2d0063c9..83d3b8b0b8c 100644 --- a/build-spring-framework/resources/changelog.txt +++ b/build-spring-framework/resources/changelog.txt @@ -24,6 +24,7 @@ Changes in version 3.1 RC2 (2011-11-15) * added methods to UriComponentsBuilder for replacing the path or the query * added ServletUriComponentsBuilder to build a UriComponents instance starting with a ServletRequest * MockHttpServletRequest and MockHttpServletResponse now keep contentType field and Content-Type header in sync +* Fix issue with cache ignoring prototype-scoped controllers in RequestMappingHandlerAdapter Changes in version 3.1 RC1 (2011-10-11) diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 44b98e1d3e7..021982464fd 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -20,6 +20,7 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import javax.servlet.http.HttpServletRequest; @@ -143,10 +144,9 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter i private HandlerMethodReturnValueHandlerComposite returnValueHandlers; - private final Map, WebDataBinderFactory> dataBinderFactoryCache = - new ConcurrentHashMap, WebDataBinderFactory>(); + private final Map, Set> dataBinderFactoryCache = new ConcurrentHashMap, Set>(); - private final Map, ModelFactory> modelFactoryCache = new ConcurrentHashMap, ModelFactory>(); + private final Map, Set> modelFactoryCache = new ConcurrentHashMap, Set>(); /** * Default constructor. @@ -660,38 +660,38 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter i private ModelFactory getModelFactory(HandlerMethod handlerMethod, WebDataBinderFactory binderFactory) { SessionAttributesHandler sessionAttrHandler = getSessionAttributesHandler(handlerMethod); Class handlerType = handlerMethod.getBeanType(); - ModelFactory modelFactory = this.modelFactoryCache.get(handlerType); - if (modelFactory == null) { - List attrMethods = new ArrayList(); - for (Method method : HandlerMethodSelector.selectMethods(handlerType, MODEL_ATTRIBUTE_METHODS)) { - InvocableHandlerMethod attrMethod = new InvocableHandlerMethod(handlerMethod.getBean(), method); - attrMethod.setHandlerMethodArgumentResolvers(this.argumentResolvers); - attrMethod.setParameterNameDiscoverer(this.parameterNameDiscoverer); - attrMethod.setDataBinderFactory(binderFactory); - attrMethods.add(attrMethod); - } - modelFactory = new ModelFactory(attrMethods, binderFactory, sessionAttrHandler); - this.modelFactoryCache.put(handlerType, modelFactory); + Set methods = this.modelFactoryCache.get(handlerType); + if (methods == null) { + methods = HandlerMethodSelector.selectMethods(handlerType, MODEL_ATTRIBUTE_METHODS); + this.modelFactoryCache.put(handlerType, methods); } - return modelFactory; + List attrMethods = new ArrayList(); + for (Method method : methods) { + InvocableHandlerMethod attrMethod = new InvocableHandlerMethod(handlerMethod.getBean(), method); + attrMethod.setHandlerMethodArgumentResolvers(this.argumentResolvers); + attrMethod.setParameterNameDiscoverer(this.parameterNameDiscoverer); + attrMethod.setDataBinderFactory(binderFactory); + attrMethods.add(attrMethod); + } + return new ModelFactory(attrMethods, binderFactory, sessionAttrHandler); } private WebDataBinderFactory getDataBinderFactory(HandlerMethod handlerMethod) throws Exception { Class handlerType = handlerMethod.getBeanType(); - WebDataBinderFactory binderFactory = this.dataBinderFactoryCache.get(handlerType); - if (binderFactory == null) { - List binderMethods = new ArrayList(); - for (Method method : HandlerMethodSelector.selectMethods(handlerType, INIT_BINDER_METHODS)) { - InvocableHandlerMethod binderMethod = new InvocableHandlerMethod(handlerMethod.getBean(), method); - binderMethod.setHandlerMethodArgumentResolvers(this.initBinderArgumentResolvers); - binderMethod.setDataBinderFactory(new DefaultDataBinderFactory(this.webBindingInitializer)); - binderMethod.setParameterNameDiscoverer(this.parameterNameDiscoverer); - binderMethods.add(binderMethod); - } - binderFactory = createDataBinderFactory(binderMethods); - this.dataBinderFactoryCache.put(handlerType, binderFactory); + Set methods = this.dataBinderFactoryCache.get(handlerType); + if (methods == null) { + methods = HandlerMethodSelector.selectMethods(handlerType, INIT_BINDER_METHODS); + this.dataBinderFactoryCache.put(handlerType, methods); } - return binderFactory; + List binderMethods = new ArrayList(); + for (Method method : methods) { + InvocableHandlerMethod binderMethod = new InvocableHandlerMethod(handlerMethod.getBean(), method); + binderMethod.setHandlerMethodArgumentResolvers(this.initBinderArgumentResolvers); + binderMethod.setDataBinderFactory(new DefaultDataBinderFactory(this.webBindingInitializer)); + binderMethod.setParameterNameDiscoverer(this.parameterNameDiscoverer); + binderMethods.add(binderMethod); + } + return createDataBinderFactory(binderMethods); } /** diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java index a239e23a74c..ee2e052b3da 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterIntegrationTests.java @@ -87,17 +87,14 @@ import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.mvc.method.annotation.support.ServletWebArgumentResolverAdapter; /** - * Serves as a sandbox to invoke all types of controller methods using all features except for the kitchen sink. - * Once a problem has been debugged and understood, tests demonstrating the issue are preferably added to the - * appropriate, more fine-grained test fixture. - * - *

If you wish to add high-level tests, consider the following other "integration"-style tests: - *

    - *
  • {@link HandlerMethodAnnotationDetectionTests} - *
  • {@link ServletAnnotationControllerHandlerMethodTests} - *
+ * A test fixture with a controller with all supported method signature styles + * and arguments. A convenient place to test or confirm a problem with a + * specific argument or return value type. * * @author Rossen Stoyanchev + * + * @see HandlerMethodAnnotationDetectionTests + * @see ServletAnnotationControllerHandlerMethodTests */ public class RequestMappingHandlerAdapterIntegrationTests { @@ -261,7 +258,7 @@ public class RequestMappingHandlerAdapterIntegrationTests { @Test public void handleAndCompleteSession() throws Exception { HandlerMethod handlerMethod = handlerMethod("handleAndCompleteSession", SessionStatus.class); - ModelAndView mav = handlerAdapter.handle(request, response, handlerMethod); + handlerAdapter.handle(request, response, handlerMethod); assertFalse(request.getSession().getAttributeNames().hasMoreElements()); } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index d738461c7a4..6d16ce4b12d 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java @@ -71,6 +71,7 @@ import org.springframework.beans.TestBean; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.propertyeditors.CustomDateEditor; @@ -1494,6 +1495,29 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl assertTrue(RequestContextUtils.getOutputFlashMap(request).isEmpty()); } + @Test + public void prototypeController() throws Exception { + initServlet(new ApplicationContextInitializer() { + public void initialize(GenericWebApplicationContext context) { + RootBeanDefinition beanDef = new RootBeanDefinition(PrototypeController.class); + beanDef.setScope(BeanDefinition.SCOPE_PROTOTYPE); + context.registerBeanDefinition("controller", beanDef); + } + }); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); + request.addParameter("param", "1"); + MockHttpServletResponse response = new MockHttpServletResponse(); + getServlet().service(request, response); + + assertEquals("count:3", response.getContentAsString()); + + response = new MockHttpServletResponse(); + getServlet().service(request, response); + + assertEquals("count:3", response.getContentAsString()); + } + /* * Controllers */ @@ -2828,6 +2852,28 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } } + @Controller + static class PrototypeController { + + private int count; + + @InitBinder + public void initBinder(WebDataBinder dataBinder) { + this.count++; + } + + @ModelAttribute + public void populate(Model model) { + this.count++; + } + + @RequestMapping("/") + public void message(int param, Writer writer) throws IOException { + this.count++; + writer.write("count:" + this.count); + } + } + // Test cases deleted from the original SevletAnnotationControllerTests: // @Ignore("Controller interface => no method-level @RequestMapping annotation")