diff --git a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java index 13955c415d6..35eb460a228 100644 --- a/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java +++ b/spring-webmvc-portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java @@ -25,6 +25,7 @@ import java.lang.reflect.Method; import java.security.Principal; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -147,7 +148,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc exceptionHandlerCache.put(handlerType, handlers); } - final Map, Method> resolverMethods = handlers; + final Map, Method> matchedHandlers = new HashMap, Method>(); ReflectionUtils.doWithMethods(handlerType, new ReflectionUtils.MethodCallback() { public void doWith(Method method) { @@ -155,11 +156,11 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc List> handledExceptions = getHandledExceptions(method); for (Class handledException : handledExceptions) { if (handledException.isAssignableFrom(thrownExceptionType)) { - if (!resolverMethods.containsKey(handledException)) { - resolverMethods.put(handledException, method); + if (!matchedHandlers.containsKey(handledException)) { + matchedHandlers.put(handledException, method); } else { - Method oldMappedMethod = resolverMethods.get(handledException); + Method oldMappedMethod = matchedHandlers.get(handledException); if (!oldMappedMethod.equals(method)) { throw new IllegalStateException( "Ambiguous exception handler mapped for " + handledException + "]: {" + @@ -171,7 +172,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc } }); - handlerMethod = getBestMatchingMethod(resolverMethods, thrownException); + handlerMethod = getBestMatchingMethod(matchedHandlers, thrownException); handlers.put(thrownExceptionType, (handlerMethod == null ? NO_METHOD_FOUND : handlerMethod)); return handlerMethod; } diff --git a/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java b/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java index 4dfe1f8681a..eb41b5be3f9 100644 --- a/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java +++ b/spring-webmvc-portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java @@ -16,17 +16,20 @@ package org.springframework.web.portlet.mvc.annotation; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + import java.io.FileNotFoundException; import java.io.IOException; import java.net.BindException; import java.net.SocketException; + import javax.portlet.PortletRequest; import javax.portlet.PortletResponse; -import static org.junit.Assert.*; import org.junit.Before; import org.junit.Test; - import org.springframework.mock.web.portlet.MockRenderRequest; import org.springframework.mock.web.portlet.MockRenderResponse; import org.springframework.stereotype.Controller; @@ -106,6 +109,20 @@ public class AnnotationMethodHandlerExceptionResolverTests { exceptionResolver.resolveException(request, response, controller, ex); } + // SPR-9209 + + @Test + public void cachingSideEffect() { + IllegalArgumentException ex = new IllegalArgumentException(); + SimpleController controller = new SimpleController(); + + ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex); + assertNotNull("No ModelAndView returned", mav); + + mav = exceptionResolver.resolveException(request, response, controller, new NullPointerException()); + assertNull(mav); + } + @Controller private static class SimpleController { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java index 2ff7987c9ff..012b3c85b85 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java @@ -27,6 +27,7 @@ import java.security.Principal; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -171,7 +172,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc exceptionHandlerCache.put(handlerType, handlers); } - final Map, Method> resolverMethods = handlers; + final Map, Method> matchedHandlers = new HashMap, Method>(); ReflectionUtils.doWithMethods(handlerType, new ReflectionUtils.MethodCallback() { public void doWith(Method method) { @@ -179,11 +180,11 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc List> handledExceptions = getHandledExceptions(method); for (Class handledException : handledExceptions) { if (handledException.isAssignableFrom(thrownExceptionType)) { - if (!resolverMethods.containsKey(handledException)) { - resolverMethods.put(handledException, method); + if (!matchedHandlers.containsKey(handledException)) { + matchedHandlers.put(handledException, method); } else { - Method oldMappedMethod = resolverMethods.get(handledException); + Method oldMappedMethod = matchedHandlers.get(handledException); if (!oldMappedMethod.equals(method)) { throw new IllegalStateException( "Ambiguous exception handler mapped for " + handledException + "]: {" + @@ -195,7 +196,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc } }); - handlerMethod = getBestMatchingMethod(resolverMethods, thrownException); + handlerMethod = getBestMatchingMethod(matchedHandlers, thrownException); handlers.put(thrownExceptionType, (handlerMethod == null ? NO_METHOD_FOUND : handlerMethod)); return handlerMethod; } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java index 412ed1b029d..f1b5946a9a6 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java @@ -110,7 +110,7 @@ public class AnnotationMethodHandlerExceptionResolverTests { assertEquals("Invalid view name returned", "GenericError", mav.getViewName()); assertEquals("Invalid status code returned", 500, response.getStatus()); } - + @Test(expected = IllegalStateException.class) public void ambiguous() { IllegalArgumentException ex = new IllegalArgumentException(); @@ -127,7 +127,7 @@ public class AnnotationMethodHandlerExceptionResolverTests { assertTrue("ModelAndView not empty", mav.isEmpty()); assertEquals("Invalid response written", "IllegalArgumentException", response.getContentAsString()); } - + @Test public void responseBody() throws UnsupportedEncodingException { IllegalArgumentException ex = new IllegalArgumentException(); @@ -139,6 +139,20 @@ public class AnnotationMethodHandlerExceptionResolverTests { assertEquals("Invalid response written", "IllegalArgumentException", response.getContentAsString()); } + // SPR-9209 + + @Test + public void cachingSideEffect() { + IllegalArgumentException ex = new IllegalArgumentException(); + SimpleController controller = new SimpleController(); + + ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex); + assertNotNull("No ModelAndView returned", mav); + + mav = exceptionResolver.resolveException(request, response, controller, new NullPointerException()); + assertNull(mav); + } + @Controller private static class SimpleController {