Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Consistent ControllerAdvice applicability against user-declared class

Browse Source 
Issue: SPR-16496
This commit is contained in:
Juergen Hoeller
2018-02-14 14:41:16 +01:00
parent 766e6028d7
commit 46cbdff5c3
2 changed files with 63 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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; package org.springframework.web.servlet.mvc.method.annotation;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
@@ -27,6 +28,7 @@ import java.util.concurrent.ConcurrentHashMap;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import org.springframework.aop.support.AopUtils;
import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.InitializingBean;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware; import org.springframework.context.ApplicationContextAware;
@@ -440,13 +442,18 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce
* Spring-managed beans were detected. * Spring-managed beans were detected.
* @param handlerMethod the method where the exception was raised (may be {@code null}) * @param handlerMethod the method where the exception was raised (may be {@code null})
* @param exception the raised exception * @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 @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) { 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); ExceptionHandlerMethodResolver resolver = this.exceptionHandlerCache.get(handlerType);
if (resolver == null) { if (resolver == null) {
resolver = new ExceptionHandlerMethodResolver(handlerType); resolver = new ExceptionHandlerMethodResolver(handlerType);
@@ -456,14 +463,20 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce
if (method != null) { if (method != null) {
return new ServletInvocableHandlerMethod(handlerMethod.getBean(), method); 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()) { 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(); ExceptionHandlerMethodResolver resolver = entry.getValue();
Method method = resolver.resolveMethod(exception); Method method = resolver.resolveMethod(exception);
if (method != null) { if (method != null) {
return new ServletInvocableHandlerMethod(entry.getKey().resolveBean(), method); return new ServletInvocableHandlerMethod(advice.resolveBean(), method);
} }
} }
} }

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

@@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.BeforeClass;
import org.junit.Test; import org.junit.Test;
import org.springframework.aop.framework.ProxyFactory;
import org.springframework.beans.FatalBeanException; import org.springframework.beans.FatalBeanException;
import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean; 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.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.bind.annotation.RestControllerAdvice; 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.HandlerMethod;
import org.springframework.web.method.annotation.ModelMethodProcessor; import org.springframework.web.method.annotation.ModelMethodProcessor;
import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.HandlerMethodArgumentResolver;
@@ -213,8 +215,8 @@ public class ExceptionHandlerExceptionResolverTests {
@Test @Test
public void resolveExceptionGlobalHandler() throws Exception { 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(); this.resolver.afterPropertiesSet();
IllegalAccessException ex = new IllegalAccessException(); IllegalAccessException ex = new IllegalAccessException();
@@ -228,8 +230,8 @@ public class ExceptionHandlerExceptionResolverTests {
@Test @Test
public void resolveExceptionGlobalHandlerOrdered() throws Exception { 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(); this.resolver.afterPropertiesSet();
IllegalStateException ex = new IllegalStateException(); IllegalStateException ex = new IllegalStateException();
@@ -243,8 +245,8 @@ public class ExceptionHandlerExceptionResolverTests {
@Test // SPR-12605 @Test // SPR-12605
public void resolveExceptionWithHandlerMethodArg() throws Exception { 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(); this.resolver.afterPropertiesSet();
ArrayIndexOutOfBoundsException ex = new ArrayIndexOutOfBoundsException(); ArrayIndexOutOfBoundsException ex = new ArrayIndexOutOfBoundsException();
@@ -258,8 +260,8 @@ public class ExceptionHandlerExceptionResolverTests {
@Test @Test
public void resolveExceptionWithAssertionError() throws Exception { 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(); this.resolver.afterPropertiesSet();
AssertionError err = new AssertionError("argh"); AssertionError err = new AssertionError("argh");
@@ -274,8 +276,8 @@ public class ExceptionHandlerExceptionResolverTests {
@Test @Test
public void resolveExceptionWithAssertionErrorAsRootCause() throws Exception { 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(); this.resolver.afterPropertiesSet();
AssertionError err = new AssertionError("argh"); AssertionError err = new AssertionError("argh");
@@ -290,8 +292,8 @@ public class ExceptionHandlerExceptionResolverTests {
@Test @Test
public void resolveExceptionControllerAdviceHandler() throws Exception { 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(); this.resolver.afterPropertiesSet();
IllegalStateException ex = new IllegalStateException(); IllegalStateException ex = new IllegalStateException();
@@ -305,8 +307,8 @@ public class ExceptionHandlerExceptionResolverTests {
@Test @Test
public void resolveExceptionControllerAdviceNoHandler() throws Exception { 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(); this.resolver.afterPropertiesSet();
IllegalStateException ex = new IllegalStateException(); IllegalStateException ex = new IllegalStateException();
@@ -317,6 +319,21 @@ public class ExceptionHandlerExceptionResolverTests {
assertEquals("DefaultTestExceptionResolver: IllegalStateException", this.response.getContentAsString()); 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) { private void assertMethodProcessorCount(int resolverCount, int handlerCount) {
assertEquals(resolverCount, this.resolver.getArgumentResolvers().getResolvers().size()); 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 @Controller
static class ResponseBodyController { static class ResponseBodyController extends WebApplicationObjectSupport implements ResponseBodyInterface {
public void handle() {} public void handle() {}
@@ -454,7 +481,7 @@ public class ExceptionHandlerExceptionResolverTests {
} }
@RestControllerAdvice("org.springframework.web.servlet.mvc.method.annotation") @RestControllerAdvice(assignableTypes = WebApplicationObjectSupport.class)
@Order(2) @Order(2)
static class BasePackageTestExceptionResolver { static class BasePackageTestExceptionResolver {