Consistent ControllerAdvice applicability against user-declared class
Issue: SPR-16496
This commit is contained in:
parent
766e6028d7
commit
46cbdff5c3
|
|
@ -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) {
|
||||
Class<?> handlerType = (handlerMethod != null ? handlerMethod.getBeanType() : null);
|
||||
protected ServletInvocableHandlerMethod getExceptionHandlerMethod(
|
||||
@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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
this.resolver.setApplicationContext(cxt);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
|
||||
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);
|
||||
this.resolver.setApplicationContext(cxt);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
|
||||
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);
|
||||
this.resolver.setApplicationContext(cxt);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
|
||||
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);
|
||||
this.resolver.setApplicationContext(cxt);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
|
||||
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);
|
||||
this.resolver.setApplicationContext(cxt);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
|
||||
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);
|
||||
this.resolver.setApplicationContext(cxt);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
|
||||
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);
|
||||
this.resolver.setApplicationContext(cxt);
|
||||
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
|
||||
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 {
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue