From bf04e3232b959735a89ea2a06f77295773fce976 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 23 Jan 2017 11:35:32 +0100 Subject: [PATCH] DATACMNS-776 - Made ProxyingHandlerMethodArgumentResolver to only support user types or annotated ones. ProxyingHandlerMethodArgumentResolver is now more lenient when it comes to which types to support for proxying. As indicated in the ticket, we've been to aggressive opting in for all interfaces which - depending on the order of converter registrations - caused us interfering with other interface based resolutions (e.g. in Spring Mobile, Security etc.). We now only aggressively kick in if either the type or parameter is annotated with @ProjectedPayload or the type itself is not a Spring Framework or native Java one. This should leave user defined types still be accepted whereas the types we previously erroneously interfered with should now be ignored. --- ...ProxyingHandlerMethodArgumentResolver.java | 33 ++++++- src/test/java/SampleInterface.java | 23 +++++ ...andlerMethodArgumentResolverUnitTests.java | 92 +++++++++++++++++++ .../data/web/config/SampleController.java | 4 +- 4 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 src/test/java/SampleInterface.java create mode 100644 src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java diff --git a/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java index b918f22ac..0ab0c9441 100644 --- a/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java @@ -15,6 +15,9 @@ */ package org.springframework.data.web; +import java.util.Arrays; +import java.util.List; + import org.springframework.beans.BeansException; import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.factory.BeanClassLoaderAware; @@ -22,9 +25,11 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.context.ResourceLoaderAware; import org.springframework.core.MethodParameter; +import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.convert.ConversionService; import org.springframework.core.io.ResourceLoader; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; +import org.springframework.util.ClassUtils; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; @@ -40,6 +45,8 @@ import org.springframework.web.method.support.HandlerMethodArgumentResolver; public class ProxyingHandlerMethodArgumentResolver extends ModelAttributeMethodProcessor implements BeanFactoryAware, ResourceLoaderAware, BeanClassLoaderAware { + private static final List IGNORED_PACKAGES = Arrays.asList("java", "org.springframework"); + private final SpelAwareProxyProjectionFactory proxyFactory; private final ConversionService conversionService; @@ -90,7 +97,31 @@ public class ProxyingHandlerMethodArgumentResolver extends ModelAttributeMethodP */ @Override public boolean supportsParameter(MethodParameter parameter) { - return parameter.getParameterType().isInterface(); + + Class type = parameter.getParameterType(); + + if (!type.isInterface()) { + return false; + } + + // Annotated parameter + if (parameter.getParameterAnnotation(ProjectedPayload.class) != null) { + return true; + } + + // Annotated type + if (AnnotatedElementUtils.findMergedAnnotation(type, ProjectedPayload.class) != null) { + return true; + } + + // Fallback for only user defined interfaces + for (String prefix : IGNORED_PACKAGES) { + if (ClassUtils.getPackageName(type).startsWith(prefix)) { + return false; + } + } + + return true; } /* diff --git a/src/test/java/SampleInterface.java b/src/test/java/SampleInterface.java new file mode 100644 index 000000000..f774b9ec6 --- /dev/null +++ b/src/test/java/SampleInterface.java @@ -0,0 +1,23 @@ +import org.springframework.data.web.ProxyingHandlerMethodArgumentResolverUnitTests; + +/* + * Copyright 2017 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. + */ + +/** + * @author Oliver Gierke + * @see ProxyingHandlerMethodArgumentResolverUnitTests + */ +public interface SampleInterface {} diff --git a/src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java new file mode 100644 index 000000000..276e43c8b --- /dev/null +++ b/src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java @@ -0,0 +1,92 @@ +/* + * Copyright 2017 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.data.web; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import java.lang.reflect.Method; +import java.util.List; + +import org.junit.Test; +import org.springframework.core.MethodParameter; +import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.data.web.ProjectingJackson2HttpMessageConverterUnitTests.SampleInterface; + +/** + * Unit tests for {@link ProxyingHandlerMethodArgumentResolver}. + * + * @author Oliver Gierke + * @soundtrack Karlijn Langendijk & Sönke Meinen - Englishman In New York (Sting, + * https://www.youtube.com/watch?v=O7LZsqrnaaA) + */ +public class ProxyingHandlerMethodArgumentResolverUnitTests { + + ProxyingHandlerMethodArgumentResolver resolver = new ProxyingHandlerMethodArgumentResolver( + new DefaultConversionService()); + + @Test // DATACMNS-776 + public void supportAnnotatedInterface() throws Exception { + + Method method = Controller.class.getMethod("with", AnnotatedInterface.class); + MethodParameter parameter = new MethodParameter(method, 0); + + assertThat(resolver.supportsParameter(parameter), is(true)); + } + + @Test // DATACMNS-776 + public void supportsUnannotatedInterfaceFromUserPackage() throws Exception { + + Method method = Controller.class.getMethod("with", SampleInterface.class); + MethodParameter parameter = new MethodParameter(method, 0); + + assertThat(resolver.supportsParameter(parameter), is(true)); + } + + @Test // DATACMNS-776 + public void doesNotSupportUnannotatedInterfaceFromSpringNamespace() throws Exception { + + Method method = Controller.class.getMethod("with", UnannotatedInterface.class); + MethodParameter parameter = new MethodParameter(method, 0); + + assertThat(resolver.supportsParameter(parameter), is(false)); + } + + @Test // DATACMNS-776 + public void doesNotSupportCoreJavaType() throws Exception { + + Method method = Controller.class.getMethod("with", List.class); + MethodParameter parameter = new MethodParameter(method, 0); + + assertThat(resolver.supportsParameter(parameter), is(false)); + } + + @ProjectedPayload + interface AnnotatedInterface {} + + interface UnannotatedInterface {} + + interface Controller { + + void with(AnnotatedInterface param); + + void with(UnannotatedInterface param); + + void with(SampleInterface param); + + void with(List param); + } +} diff --git a/src/test/java/org/springframework/data/web/config/SampleController.java b/src/test/java/org/springframework/data/web/config/SampleController.java index d5fda0c3b..ec47f5014 100644 --- a/src/test/java/org/springframework/data/web/config/SampleController.java +++ b/src/test/java/org/springframework/data/web/config/SampleController.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2017 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. @@ -21,6 +21,7 @@ import static org.junit.Assert.*; import java.util.Collection; import java.util.Date; +import org.springframework.data.web.ProjectedPayload; import org.springframework.data.web.config.SampleController.SampleDto.Address; import org.springframework.format.annotation.DateTimeFormat; import org.springframework.format.annotation.DateTimeFormat.ISO; @@ -53,6 +54,7 @@ public class SampleController { return "view"; } + @ProjectedPayload interface SampleDto { String getName();