From 93de5f407e394b70b9a3fa89e35340008d340cbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=85=A8=E5=9F=8E?= Date: Fri, 22 Mar 2019 14:51:29 +0800 Subject: [PATCH 1/3] Avoid duplicate registration of [RequestBody|ResponseBody]Advice @ControllerAdvice Prior to this commit, if a @ControllerAdvice implemented both RequestBodyAdvice and ResponseBodyAdvice, it was registered twice in RequestMappingHandlerAdapter. This commit ensures that such instances are only registered once. Closes gh-22638 --- .../method/annotation/RequestMappingHandlerAdapter.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 02f6afac8a..a89a584cea 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -598,10 +598,7 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter if (!binderMethods.isEmpty()) { this.initBinderAdviceCache.put(adviceBean, binderMethods); } - if (RequestBodyAdvice.class.isAssignableFrom(beanType)) { - requestResponseBodyAdviceBeans.add(adviceBean); - } - if (ResponseBodyAdvice.class.isAssignableFrom(beanType)) { + if (RequestBodyAdvice.class.isAssignableFrom(beanType) || ResponseBodyAdvice.class.isAssignableFrom(beanType)) { requestResponseBodyAdviceBeans.add(adviceBean); } } From 6870b9c69193fb163ad19c00248c2670d5de2dc2 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 26 Mar 2019 12:27:57 +0100 Subject: [PATCH 2/3] Test fix in gh-22638 --- .../RequestMappingHandlerAdapterTests.java | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java index 32dd1e09b3..c84cb116a7 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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.Type; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -29,15 +30,18 @@ import org.junit.BeforeClass; import org.junit.Test; import org.springframework.core.MethodParameter; +import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageConverter; +import org.springframework.http.converter.StringHttpMessageConverter; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.http.converter.json.MappingJacksonValue; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; import org.springframework.http.server.ServletServerHttpResponse; +import org.springframework.lang.Nullable; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.ui.Model; @@ -60,6 +64,7 @@ import static org.junit.Assert.*; * Unit tests for {@link RequestMappingHandlerAdapter}. * * @author Rossen Stoyanchev + * @author Sam Brannen * @see ServletAnnotationControllerHandlerMethodTests * @see HandlerMethodAnnotationDetectionTests * @see RequestMappingHandlerAdapterIntegrationTests @@ -342,8 +347,15 @@ public class RequestMappingHandlerAdapterTests { } } + /** + * This class additionally implements {@link RequestBodyAdvice} solely for the purpose + * of verifying that controller advice implementing both {@link ResponseBodyAdvice} + * and {@link RequestBodyAdvice} does not get registered twice. + * + * @see gh-22638 + */ @ControllerAdvice - private static class ResponseCodeSuppressingAdvice extends AbstractMappingJacksonResponseBodyAdvice { + private static class ResponseCodeSuppressingAdvice extends AbstractMappingJacksonResponseBodyAdvice implements RequestBodyAdvice { @Override protected void beforeBodyWriteInternal(MappingJacksonValue bodyContainer, MediaType contentType, @@ -357,6 +369,35 @@ public class RequestMappingHandlerAdapterTests { map.put("message", bodyContainer.getValue()); bodyContainer.setValue(map); } + + @Override + public boolean supports(MethodParameter methodParameter, Type targetType, + Class> converterType) { + + return StringHttpMessageConverter.class.equals(converterType); + } + + @Override + public HttpInputMessage beforeBodyRead(HttpInputMessage inputMessage, MethodParameter parameter, + Type targetType, Class> converterType) { + + return inputMessage; + } + + @Override + public Object afterBodyRead(Object body, HttpInputMessage inputMessage, MethodParameter parameter, + Type targetType, Class> converterType) { + + return body; + } + + @Override + public Object handleEmptyBody(@Nullable Object body, HttpInputMessage inputMessage, MethodParameter parameter, + Type targetType, Class> converterType) { + + return "default value for empty body"; + } + } } From fd6b64bfacf9d4224b07882def647fae4f16a021 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 26 Mar 2019 12:28:09 +0100 Subject: [PATCH 3/3] Polishing --- .../mvc/method/annotation/RequestMappingHandlerAdapter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index a89a584cea..132641f71f 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -99,7 +99,7 @@ import org.springframework.web.util.WebUtils; /** * Extension of {@link AbstractHandlerMethodAdapter} that supports - * {@link RequestMapping} annotated {@code HandlerMethod RequestMapping} annotated {@code HandlerMethods}. + * {@link RequestMapping @RequestMapping} annotated {@link HandlerMethod HandlerMethods}. * *

Support for custom argument and return value types can be added via * {@link #setCustomArgumentResolvers} and {@link #setCustomReturnValueHandlers},