Consistent ControllerAdvice applicability against user-declared class

Issue: SPR-16496

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -17,6 +17,7 @@
 package org.springframework.web.servlet.mvc.method.annotation;
 
 import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashMap;
@@ -27,6 +28,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.springframework.aop.support.AopUtils;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -440,13 +442,18 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce
 	 * Spring-managed beans were detected.
 	 * @param handlerMethod the method where the exception was raised (may be {@code null})
 	 * @param exception the raised exception
-	 * @return a method to handle the exception, or {@code null}
+	 * @return a method to handle the exception, or {@code null} if none
 	 */
 	@Nullable
-	protected ServletInvocableHandlerMethod getExceptionHandlerMethod(@Nullable HandlerMethod handlerMethod, Exception exception) {
+	protected ServletInvocableHandlerMethod getExceptionHandlerMethod(
-		Class<?> handlerType = (handlerMethod != null ? handlerMethod.getBeanType() : null);
+			@Nullable HandlerMethod handlerMethod, Exception exception) {
+
+		Class<?> handlerType = null;
 
 		if (handlerMethod != null) {
+			// Local exception handler methods on the controller class itself.
+			// To be invoked through the proxy, even in case of an interface-based proxy.
+			handlerType = handlerMethod.getBeanType();
 			ExceptionHandlerMethodResolver resolver = this.exceptionHandlerCache.get(handlerType);
 			if (resolver == null) {
 				resolver = new ExceptionHandlerMethodResolver(handlerType);
@@ -456,14 +463,20 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce
 			if (method != null) {
 				return new ServletInvocableHandlerMethod(handlerMethod.getBean(), method);
 			}
+			// For advice applicability check below (involving base packages, assignable types
+			// and annotation presence), use target class instead of interface-based proxy.
+			if (Proxy.isProxyClass(handlerType)) {
+				handlerType = AopUtils.getTargetClass(handlerMethod.getBean());
+			}
 		}
 
 		for (Entry<ControllerAdviceBean, ExceptionHandlerMethodResolver> entry : this.exceptionHandlerAdviceCache.entrySet()) {
-			if (entry.getKey().isApplicableToBeanType(handlerType)) {
+			ControllerAdviceBean advice = entry.getKey();
+			if (advice.isApplicableToBeanType(handlerType)) {
 				ExceptionHandlerMethodResolver resolver = entry.getValue();
 				Method method = resolver.resolveMethod(exception);
 				if (method != null) {
-					return new ServletInvocableHandlerMethod(entry.getKey().resolveBean(), method);
+					return new ServletInvocableHandlerMethod(advice.resolveBean(), method);
 				}
 			}
 		}

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -25,6 +25,7 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.FatalBeanException;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
 import org.springframework.context.annotation.Bean;
@@ -38,6 +39,7 @@ import org.springframework.util.ClassUtils;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.RestControllerAdvice;
+import org.springframework.web.context.support.WebApplicationObjectSupport;
 import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.method.annotation.ModelMethodProcessor;
 import org.springframework.web.method.support.HandlerMethodArgumentResolver;
@@ -213,8 +215,8 @@ public class ExceptionHandlerExceptionResolverTests {
 
 	@Test
 	public void resolveExceptionGlobalHandler() throws Exception {
-		AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class);
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
-		this.resolver.setApplicationContext(cxt);
+		this.resolver.setApplicationContext(ctx);
 		this.resolver.afterPropertiesSet();
 
 		IllegalAccessException ex = new IllegalAccessException();
@@ -228,8 +230,8 @@ public class ExceptionHandlerExceptionResolverTests {
 
 	@Test
 	public void resolveExceptionGlobalHandlerOrdered() throws Exception {
-		AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class);
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
-		this.resolver.setApplicationContext(cxt);
+		this.resolver.setApplicationContext(ctx);
 		this.resolver.afterPropertiesSet();
 
 		IllegalStateException ex = new IllegalStateException();
@@ -243,8 +245,8 @@ public class ExceptionHandlerExceptionResolverTests {
 
 	@Test  // SPR-12605
 	public void resolveExceptionWithHandlerMethodArg() throws Exception {
-		AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class);
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
-		this.resolver.setApplicationContext(cxt);
+		this.resolver.setApplicationContext(ctx);
 		this.resolver.afterPropertiesSet();
 
 		ArrayIndexOutOfBoundsException ex = new ArrayIndexOutOfBoundsException();
@@ -258,8 +260,8 @@ public class ExceptionHandlerExceptionResolverTests {
 
 	@Test
 	public void resolveExceptionWithAssertionError() throws Exception {
-		AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class);
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
-		this.resolver.setApplicationContext(cxt);
+		this.resolver.setApplicationContext(ctx);
 		this.resolver.afterPropertiesSet();
 
 		AssertionError err = new AssertionError("argh");
@@ -274,8 +276,8 @@ public class ExceptionHandlerExceptionResolverTests {
 
 	@Test
 	public void resolveExceptionWithAssertionErrorAsRootCause() throws Exception {
-		AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class);
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
-		this.resolver.setApplicationContext(cxt);
+		this.resolver.setApplicationContext(ctx);
 		this.resolver.afterPropertiesSet();
 
 		AssertionError err = new AssertionError("argh");
@@ -290,8 +292,8 @@ public class ExceptionHandlerExceptionResolverTests {
 
 	@Test
 	public void resolveExceptionControllerAdviceHandler() throws Exception {
-		AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
-		this.resolver.setApplicationContext(cxt);
+		this.resolver.setApplicationContext(ctx);
 		this.resolver.afterPropertiesSet();
 
 		IllegalStateException ex = new IllegalStateException();
@@ -305,8 +307,8 @@ public class ExceptionHandlerExceptionResolverTests {
 
 	@Test
 	public void resolveExceptionControllerAdviceNoHandler() throws Exception {
-		AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
-		this.resolver.setApplicationContext(cxt);
+		this.resolver.setApplicationContext(ctx);
 		this.resolver.afterPropertiesSet();
 
 		IllegalStateException ex = new IllegalStateException();
@@ -317,6 +319,21 @@ public class ExceptionHandlerExceptionResolverTests {
 		assertEquals("DefaultTestExceptionResolver: IllegalStateException", this.response.getContentAsString());
 	}
 
+	@Test  // SPR-16496
+	public void resolveExceptionControllerAdviceAgainstProxy() throws Exception {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
+		this.resolver.setApplicationContext(ctx);
+		this.resolver.afterPropertiesSet();
+
+		IllegalStateException ex = new IllegalStateException();
+		HandlerMethod handlerMethod = new HandlerMethod(new ProxyFactory(new ResponseBodyController()).getProxy(), "handle");
+		ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex);
+
+		assertNotNull("Exception was not handled", mav);
+		assertTrue(mav.isEmpty());
+		assertEquals("BasePackageTestExceptionResolver: IllegalStateException", this.response.getContentAsString());
+	}
+
 
 	private void assertMethodProcessorCount(int resolverCount, int handlerCount) {
 		assertEquals(resolverCount, this.resolver.getArgumentResolvers().getResolvers().size());
@@ -348,8 +365,18 @@ public class ExceptionHandlerExceptionResolverTests {
 	}
 
 
+	interface ResponseBodyInterface {
+
+		void handle();
+
+		@ExceptionHandler
+		@ResponseBody
+		String handleException(IllegalArgumentException ex);
+	}
+
+
 	@Controller
-	static class ResponseBodyController {
+	static class ResponseBodyController extends WebApplicationObjectSupport implements ResponseBodyInterface {
 
 		public void handle() {}
 
@@ -454,7 +481,7 @@ public class ExceptionHandlerExceptionResolverTests {
 	}
 
 
-	@RestControllerAdvice("org.springframework.web.servlet.mvc.method.annotation")
+	@RestControllerAdvice(assignableTypes = WebApplicationObjectSupport.class)
 	@Order(2)
 	static class BasePackageTestExceptionResolver {