fixed @ExceptionHandler resolution in case of multiple matches at different inheritance levels (SPR-7085)

org.springframework.core/src/main/java/org/springframework/core/ExceptionDepthComparator.java

@@ -0,0 +1,96 @@
+/*
+ * Copyright 2002-2010 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.core;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+import org.springframework.util.Assert;
+
+/**
+ * Comparator capable of sorting exceptions based on their depth from the thrown exception type.
+ *
+ * @author Juergen Hoeller
+ * @author Arjen Poutsma
+ * @since 3.0.3
+ */
+public class ExceptionDepthComparator implements Comparator<Class<? extends Throwable>> {
+
+	private final Class<? extends Throwable> targetException;
+
+
+	/**
+	 * Create a new ExceptionDepthComparator for the given exception.
+	 * @param exception the target exception to compare to when sorting by depth
+	 */
+	public ExceptionDepthComparator(Throwable exception) {
+		Assert.notNull(exception, "Target exception must not be null");
+		this.targetException = exception.getClass();
+	}
+
+	/**
+	 * Create a new ExceptionDepthComparator for the given exception type.
+	 * @param exceptionType the target exception type to compare to when sorting by depth
+	 */
+	public ExceptionDepthComparator(Class<? extends Throwable> exceptionType) {
+		Assert.notNull(exceptionType, "Target exception type must not be null");
+		this.targetException = exceptionType;
+	}
+
+
+	public int compare(Class<? extends Throwable> o1, Class<? extends Throwable> o2) {
+		int depth1 = getDepth(o1, this.targetException, 0);
+		int depth2 = getDepth(o2, this.targetException, 0);
+		return (depth1 - depth2);
+	}
+
+	private int getDepth(Class declaredException, Class exceptionToMatch, int depth) {
+		if (declaredException.equals(exceptionToMatch)) {
+			// Found it!
+			return depth;
+		}
+		// If we've gone as far as we can go and haven't found it...
+		if (Throwable.class.equals(exceptionToMatch)) {
+			return -1;
+		}
+		return getDepth(declaredException, exceptionToMatch.getSuperclass(), depth + 1);
+	}
+
+
+	/**
+	 * Obtain the closest match from the given exception types for the given target exception.
+	 * @param exceptionTypes the collection of exception types
+	 * @param targetException the target exception to find a match for
+	 * @return the closest matching exception type from the given collection
+	 */
+	public static Class<? extends Throwable> findClosestMatch(
+			Collection<Class<? extends Throwable>> exceptionTypes, Throwable targetException) {
+
+		Assert.notEmpty(exceptionTypes, "Exception types must not be empty");
+		if (exceptionTypes.size() == 1) {
+			return exceptionTypes.iterator().next();
+		}
+		List<Class<? extends Throwable>> handledExceptions =
+				new ArrayList<Class<? extends Throwable>>(exceptionTypes);
+		Collections.sort(handledExceptions, new ExceptionDepthComparator(targetException));
+		return handledExceptions.get(0);
+	}
+
+}

org.springframework.web.portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java

@@ -25,8 +25,6 @@ import java.lang.reflect.Method;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
-import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
@@ -43,6 +41,7 @@ import javax.portlet.PortletResponse;
 import javax.portlet.PortletSession;
 import javax.portlet.WindowState;
 
+import org.springframework.core.ExceptionDepthComparator;
 import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.MethodParameter;
 import org.springframework.ui.Model;
@@ -65,6 +64,7 @@ import org.springframework.web.servlet.View;
  * <p>This exception resolver is enabled by default in the {@link org.springframework.web.portlet.DispatcherPortlet}.
  *
  * @author Arjen Poutsma
+ * @author Juergen Hoeller
  * @since 3.0
  */
 public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExceptionResolver {
@@ -90,8 +90,8 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 
 
 	@Override
-	protected ModelAndView doResolveException(PortletRequest request, MimeResponse response,
+	protected ModelAndView doResolveException(
-			Object handler, Exception ex) {
+			PortletRequest request, MimeResponse response, Object handler, Exception ex) {
 
 		if (handler != null) {
 			Method handlerMethod = findBestExceptionHandlerMethod(handler, ex);
@@ -146,7 +146,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 			}
 		});
 
-		return getBestMatchingMethod(thrownException, resolverMethods);
+		return getBestMatchingMethod(resolverMethods, thrownException);
 	}
 
 	/**
@@ -179,15 +179,13 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 	/**
 	 * Returns the best matching method. Uses the {@link DepthComparator}.
 	 */
-	private Method getBestMatchingMethod(Exception thrownException,
+	private Method getBestMatchingMethod(
-			Map<Class<? extends Throwable>, Method> resolverMethods) {
+			Map<Class<? extends Throwable>, Method> resolverMethods, Exception thrownException) {
 
 		if (!resolverMethods.isEmpty()) {
-			List<Class<? extends Throwable>> handledExceptions =
+			Class<? extends Throwable> closestMatch =
-					new ArrayList<Class<? extends Throwable>>(resolverMethods.keySet());
+					ExceptionDepthComparator.findClosestMatch(resolverMethods.keySet(), thrownException);
-			Collections.sort(handledExceptions, new DepthComparator(thrownException));
+			return resolverMethods.get(closestMatch);
-			Class<? extends Throwable> bestMatchMethod = handledExceptions.get(0);
-			return resolverMethods.get(bestMatchMethod);
 		}
 		else {
 			return null;
@@ -373,37 +371,4 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 		}
 	}
 
-
-	/**
-	 * Comparator capable of sorting exceptions based on their depth from the thrown exception type.
-	 */
-	private static class DepthComparator implements Comparator<Class<? extends Throwable>> {
-
-		private final Class<? extends Throwable> handlerExceptionType;
-
-		private DepthComparator(Exception handlerException) {
-			this.handlerExceptionType = handlerException.getClass();
-		}
-
-		public int compare(Class<? extends Throwable> o1, Class<? extends Throwable> o2) {
-			int depth1 = getDepth(o1, 0);
-			int depth2 = getDepth(o2, 0);
-
-			return depth2 - depth1;
-		}
-
-		private int getDepth(Class exceptionType, int depth) {
-			if (exceptionType.equals(handlerExceptionType)) {
-				// Found it!
-				return depth;
-			}
-			// If we've gone as far as we can go and haven't found it...
-			if (Throwable.class.equals(exceptionType)) {
-				return -1;
-			}
-			return getDepth(exceptionType.getSuperclass(), depth + 1);
-		}
-
-	}
-
 }

org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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.
@@ -16,9 +16,10 @@
 
 package org.springframework.web.portlet.mvc.annotation;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.BindException;
-
+import java.net.SocketException;
 import javax.portlet.PortletRequest;
 import javax.portlet.PortletResponse;
 
@@ -26,8 +27,6 @@ import static org.junit.Assert.*;
 import org.junit.Before;
 import org.junit.Test;
 
-import org.springframework.mock.web.portlet.MockPortletRequest;
-import org.springframework.mock.web.portlet.MockPortletResponse;
 import org.springframework.mock.web.portlet.MockRenderRequest;
 import org.springframework.mock.web.portlet.MockRenderResponse;
 import org.springframework.stereotype.Controller;
@@ -35,7 +34,10 @@ import org.springframework.util.ClassUtils;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.portlet.ModelAndView;
 
-/** @author Arjen Poutsma */
+/**
+ * @author Arjen Poutsma
+ * @author Juergen Hoeller
+ */
 public class AnnotationMethodHandlerExceptionResolverTests {
 
 	private AnnotationMethodHandlerExceptionResolver exceptionResolver;
@@ -44,6 +46,7 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 
 	private MockRenderResponse response;
 
+
 	@Before
 	public void setUp() {
 		exceptionResolver = new AnnotationMethodHandlerExceptionResolver();
@@ -52,12 +55,39 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 	}
 
 	@Test
-	public void simple() {
+	public void simpleWithIOException() {
+		IOException ex = new IOException();
+		SimpleController controller = new SimpleController();
+		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
+		assertNotNull("No ModelAndView returned", mav);
+		assertEquals("Invalid view name returned", "X:IOException", mav.getViewName());
+	}
+
+	@Test
+	public void simpleWithSocketException() {
+		SocketException ex = new SocketException();
+		SimpleController controller = new SimpleController();
+		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
+		assertNotNull("No ModelAndView returned", mav);
+		assertEquals("Invalid view name returned", "Y:SocketException", mav.getViewName());
+	}
+
+	@Test
+	public void simpleWithFileNotFoundException() {
+		FileNotFoundException ex = new FileNotFoundException();
+		SimpleController controller = new SimpleController();
+		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
+		assertNotNull("No ModelAndView returned", mav);
+		assertEquals("Invalid view name returned", "X:FileNotFoundException", mav.getViewName());
+	}
+
+	@Test
+	public void simpleWithBindException() {
 		BindException ex = new BindException();
 		SimpleController controller = new SimpleController();
 		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
 		assertNotNull("No ModelAndView returned", mav);
-		assertEquals("Invalid view name returned", "BindException", mav.getViewName());
+		assertEquals("Invalid view name returned", "Y:BindException", mav.getViewName());
 	}
 
 	@Test(expected = IllegalStateException.class)
@@ -67,17 +97,18 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		exceptionResolver.resolveException(request, response, controller, ex);
 	}
 
+
 	@Controller
 	private static class SimpleController {
 
 		@ExceptionHandler(IOException.class)
 		public String handleIOException(IOException ex, PortletRequest request) {
-			return ClassUtils.getShortName(ex.getClass());
+			return "X:" + ClassUtils.getShortName(ex.getClass());
 		}
 
-		@ExceptionHandler(BindException.class)
+		@ExceptionHandler(SocketException.class)
-		public String handleBindException(Exception ex, PortletResponse response) {
+		public String handleSocketException(Exception ex, PortletResponse response) {
-			return ClassUtils.getShortName(ex.getClass());
+			return "Y:" + ClassUtils.getShortName(ex.getClass());
 		}
 
 		@ExceptionHandler(IllegalArgumentException.class)
@@ -87,12 +118,12 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 
 	}
 
+
 	@Controller
 	private static class AmbiguousController {
 
 		@ExceptionHandler({BindException.class, IllegalArgumentException.class})
-		public String handle1(Exception ex, PortletRequest request, PortletResponse response)
+		public String handle1(Exception ex, PortletRequest request, PortletResponse response) {
-				throws IOException {
 			return ClassUtils.getShortName(ex.getClass());
 		}
 
@@ -102,4 +133,5 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		}
 
 	}
-}
+
+}

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java

@@ -27,7 +27,6 @@ import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
@@ -39,6 +38,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
+import org.springframework.core.ExceptionDepthComparator;
 import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.annotation.AnnotationUtils;
@@ -75,6 +75,7 @@ import org.springframework.web.servlet.support.RequestContextUtils;
  * <p>This exception resolver is enabled by default in the {@link org.springframework.web.servlet.DispatcherServlet}.
  *
  * @author Arjen Poutsma
+ * @author Juergen Hoeller
  * @since 3.0
  */
 public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExceptionResolver {
@@ -114,8 +115,8 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 
 
 	@Override
-	protected ModelAndView doResolveException(HttpServletRequest request, HttpServletResponse response,
+	protected ModelAndView doResolveException(
-			Object handler, Exception ex) {
+			HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex) {
 
 		if (handler != null) {
 			Method handlerMethod = findBestExceptionHandlerMethod(handler, ex);
@@ -171,7 +172,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 			}
 		});
 
-		return getBestMatchingMethod(thrownException, resolverMethods);
+		return getBestMatchingMethod(resolverMethods, thrownException);
 	}
 
 	/**
@@ -204,15 +205,13 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 	/**
 	 * Returns the best matching method. Uses the {@link DepthComparator}.
 	 */
-	private Method getBestMatchingMethod(Exception thrownException,
+	private Method getBestMatchingMethod(
-			Map<Class<? extends Throwable>, Method> resolverMethods) {
+			Map<Class<? extends Throwable>, Method> resolverMethods, Exception thrownException) {
 
 		if (!resolverMethods.isEmpty()) {
-			List<Class<? extends Throwable>> handledExceptions =
+			Class<? extends Throwable> closestMatch =
-					new ArrayList<Class<? extends Throwable>>(resolverMethods.keySet());
+					ExceptionDepthComparator.findClosestMatch(resolverMethods.keySet(), thrownException);
-			Collections.sort(handledExceptions, new DepthComparator(thrownException));
+			return resolverMethods.get(closestMatch);
-			Class<? extends Throwable> bestMatchMethod = handledExceptions.get(0);
-			return resolverMethods.get(bestMatchMethod);
 		}
 		else {
 			return null;
@@ -410,36 +409,4 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc
 		return null;
 	}
 
-
-	/**
-	 * Comparator capable of sorting exceptions based on their depth from the thrown exception type.
-	 */
-	private static class DepthComparator implements Comparator<Class<? extends Throwable>> {
-
-		private final Class<? extends Throwable> handlerExceptionType;
-
-		private DepthComparator(Exception handlerException) {
-			this.handlerExceptionType = handlerException.getClass();
-		}
-
-		public int compare(Class<? extends Throwable> o1, Class<? extends Throwable> o2) {
-			int depth1 = getDepth(o1, 0);
-			int depth2 = getDepth(o2, 0);
-
-			return depth2 - depth1;
-		}
-
-		private int getDepth(Class exceptionType, int depth) {
-			if (exceptionType.equals(handlerExceptionType)) {
-				// Found it!
-				return depth;
-			}
-			// If we've gone as far as we can go and haven't found it...
-			if (Throwable.class.equals(exceptionType)) {
-				return -1;
-			}
-			return getDepth(exceptionType.getSuperclass(), depth + 1);
-		}
-	}
-
 }

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * Copyright 2002-2010 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.
@@ -16,10 +16,12 @@
 
 package org.springframework.web.servlet.mvc.annotation;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.io.Writer;
 import java.io.UnsupportedEncodingException;
+import java.io.Writer;
 import java.net.BindException;
+import java.net.SocketException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -33,12 +35,13 @@ import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.stereotype.Controller;
 import org.springframework.util.ClassUtils;
 import org.springframework.web.bind.annotation.ExceptionHandler;
-import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.servlet.ModelAndView;
 
 /**
  * @author Arjen Poutsma
+ * @author Juergen Hoeller
  */
 public class AnnotationMethodHandlerExceptionResolverTests {
 
@@ -48,6 +51,7 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 
 	private MockHttpServletResponse response;
 
+
 	@Before
 	public void setUp() {
 		exceptionResolver = new AnnotationMethodHandlerExceptionResolver();
@@ -57,15 +61,45 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 	}
 
 	@Test
-	public void simple() {
+	public void simpleWithIOException() {
+		IOException ex = new IOException();
+		SimpleController controller = new SimpleController();
+		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
+		assertNotNull("No ModelAndView returned", mav);
+		assertEquals("Invalid view name returned", "X:IOException", mav.getViewName());
+		assertEquals("Invalid status code returned", 500, response.getStatus());
+	}
+
+	@Test
+	public void simpleWithSocketException() {
+		SocketException ex = new SocketException();
+		SimpleController controller = new SimpleController();
+		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
+		assertNotNull("No ModelAndView returned", mav);
+		assertEquals("Invalid view name returned", "Y:SocketException", mav.getViewName());
+		assertEquals("Invalid status code returned", 406, response.getStatus());
+	}
+
+	@Test
+	public void simpleWithFileNotFoundException() {
+		FileNotFoundException ex = new FileNotFoundException();
+		SimpleController controller = new SimpleController();
+		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
+		assertNotNull("No ModelAndView returned", mav);
+		assertEquals("Invalid view name returned", "X:FileNotFoundException", mav.getViewName());
+		assertEquals("Invalid status code returned", 500, response.getStatus());
+	}
+
+	@Test
+	public void simpleWithBindException() {
 		BindException ex = new BindException();
 		SimpleController controller = new SimpleController();
 		ModelAndView mav = exceptionResolver.resolveException(request, response, controller, ex);
 		assertNotNull("No ModelAndView returned", mav);
-		assertEquals("Invalid view name returned", "BindException", mav.getViewName());
+		assertEquals("Invalid view name returned", "Y:BindException", mav.getViewName());
 		assertEquals("Invalid status code returned", 406, response.getStatus());
 	}
-	
+
 	@Test
 	public void inherited()	{
 		IOException ex = new IOException();
@@ -104,37 +138,39 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		assertEquals("Invalid response written", "IllegalArgumentException", response.getContentAsString());
 	}
 
+
 	@Controller
 	private static class SimpleController {
 
 		@ExceptionHandler(IOException.class)
 		@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
 		public String handleIOException(IOException ex, HttpServletRequest request) {
-			return ClassUtils.getShortName(ex.getClass());
+			return "X:" + ClassUtils.getShortName(ex.getClass());
 		}
 
-		@ExceptionHandler(BindException.class)
+		@ExceptionHandler(SocketException.class)
 		@ResponseStatus(HttpStatus.NOT_ACCEPTABLE)
-		public String handleBindException(Exception ex, HttpServletResponse response) {
+		public String handleSocketException(Exception ex, HttpServletResponse response) {
-			return ClassUtils.getShortName(ex.getClass());
+			return "Y:" + ClassUtils.getShortName(ex.getClass());
 		}
 
 		@ExceptionHandler(IllegalArgumentException.class)
 		public String handleIllegalArgumentException(Exception ex) {
 			return ClassUtils.getShortName(ex.getClass());
 		}
-
 	}
 
+
 	@Controller
-	private static class InheritedController extends SimpleController
+	private static class InheritedController extends SimpleController {
-	{
+
 		@Override
 		public String handleIOException(IOException ex, HttpServletRequest request)	{
 			return "GenericError";
 		}
 	}
 
+
 	@Controller
 	private static class AmbiguousController {
 
@@ -148,9 +184,9 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		public String handle2(IllegalArgumentException ex) {
 			return ClassUtils.getShortName(ex.getClass());
 		}
-
 	}
 
+
 	@Controller
 	private static class NoMAVReturningController {
 
@@ -160,6 +196,7 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 		}
 	}
 
+
 	@Controller
 	private static class ResponseBodyController {
 
@@ -169,4 +206,5 @@ public class AnnotationMethodHandlerExceptionResolverTests {
 			return ClassUtils.getShortName(ex.getClass());
 		}
 	}
+
 }