diff --git a/src/main/antora/modules/ROOT/pages/repositories/core-extensions-web.adoc b/src/main/antora/modules/ROOT/pages/repositories/core-extensions-web.adoc index 246947709a..15f5cc075c 100644 --- a/src/main/antora/modules/ROOT/pages/repositories/core-extensions-web.adoc +++ b/src/main/antora/modules/ROOT/pages/repositories/core-extensions-web.adoc @@ -340,6 +340,9 @@ public interface UserPayload { } ---- +For Spring MVC handler method argument resolution, annotate either the handler method parameter or the projection interface with `@ProjectedPayload`. +Interfaces without `@ProjectedPayload` on either the handler method parameter or the interface type are not considered projection payloads. + You can use the type shown in the preceding example as a Spring MVC handler method argument or by using `ParameterizedTypeReference` on one of methods of the `RestTemplate`. The preceding method declarations would try to find `firstname` anywhere in the given document. The `lastname` XML lookup is performed on the top-level of the incoming document. diff --git a/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java b/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java index b0b2c17fde..4a1f22d859 100644 --- a/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java +++ b/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java @@ -15,13 +15,6 @@ */ package org.springframework.data.web; -import java.lang.annotation.Annotation; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - import org.springframework.beans.BeansException; import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.factory.BeanClassLoaderAware; @@ -32,13 +25,10 @@ import org.springframework.core.SpringProperties; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.convert.ConversionService; -import org.springframework.core.log.LogAccessor; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; -import org.springframework.util.ClassUtils; import org.springframework.util.NumberUtils; import org.springframework.util.StringUtils; import org.springframework.web.bind.WebDataBinder; -import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.annotation.ModelAttributeMethodProcessor; @@ -55,20 +45,16 @@ * @author Chris Bono * @author Mark Paluch * @author Christoph Strobl + * @author hutiefang * @since 1.10 */ public class ProxyingHandlerMethodArgumentResolver extends ModelAttributeMethodProcessor implements BeanFactoryAware, BeanClassLoaderAware { - // NonFinalForTesting - private static LogAccessor LOGGER = new LogAccessor(ProxyingHandlerMethodArgumentResolver.class); public static final String COLLECTION_SIZE_LIMIT_PARAM = "spring.data.web.projection.collection-limit"; - private static final List IGNORED_PACKAGES = List.of("java", "org.springframework"); - private final SpelAwareProxyProjectionFactory proxyFactory; private final ObjectFactory conversionService; - private final ProjectedPayloadDeprecationLogger deprecationLogger = new ProjectedPayloadDeprecationLogger(); private final int collectionSizeLimit; /** @@ -122,35 +108,8 @@ public boolean supportsParameter(MethodParameter parameter) { return false; } - // Type or parameter explicitly annotated with @ProjectedPayload - if (parameter.hasParameterAnnotation(ProjectedPayload.class) || AnnotatedElementUtils.findMergedAnnotation(type, - ProjectedPayload.class) != null) { - return true; - } - - // Parameter annotated with @ModelAttribute - if (parameter.hasParameterAnnotation(ModelAttribute.class)) { - this.deprecationLogger.logDeprecationForParameter(parameter); - return false; - } - - // Exclude any other parameters annotated with Spring annotation - if (Arrays.stream(parameter.getParameterAnnotations()) - .map(Annotation::annotationType) - .map(Class::getPackageName) - .anyMatch(it -> it.startsWith("org.springframework"))) { - - return false; - } - - // Fallback for only user defined interfaces - String packageName = ClassUtils.getPackageName(type); - if (IGNORED_PACKAGES.stream().noneMatch(packageName::startsWith)) { - this.deprecationLogger.logDeprecationForParameter(parameter); - return false; - } - - return false; + return parameter.hasParameterAnnotation(ProjectedPayload.class) + || AnnotatedElementUtils.findMergedAnnotation(type, ProjectedPayload.class) != null; } @Override @@ -166,35 +125,4 @@ protected Object createAttribute(String attributeName, MethodParameter parameter @Override protected void bindRequestParameters(WebDataBinder binder, NativeWebRequest request) {} - /** - * Logs a warning message when a parameter is proxied but not explicitly annotated with {@link @ProjectedPayload}. - *

- * To avoid log spamming, the message is only logged the first time the parameter is encountered. - */ - static class ProjectedPayloadDeprecationLogger { - - private static final String MESSAGE = "Parameter %sat index %s in [%s] is not annotated with @ProjectedPayload. Make sure to annotate it with @ProjectedPayload (at the parameter or type level) to use it for projections."; - - private final Set loggedParameters = Collections.synchronizedSet(new HashSet<>()); - - /** - * Log a warning the first time a non-annotated method parameter is encountered. - * - * @param parameter the parameter - */ - void logDeprecationForParameter(MethodParameter parameter) { - - if (!this.loggedParameters.add(parameter)) { - return; - } - - String paramName = parameter.getParameterName(); - String paramNameOrEmpty = paramName != null ? ("'" + paramName + "' ") : ""; - String methodName = parameter.getMethod() != null ? parameter.getMethod().toGenericString() : "constructor"; - - LOGGER.warn(() -> MESSAGE.formatted(paramNameOrEmpty, parameter.getParameterIndex(), methodName)); - } - - } - } diff --git a/src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java b/src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java index c145b63e00..c96951c064 100755 --- a/src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java +++ b/src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java @@ -23,17 +23,20 @@ import java.lang.reflect.Method; import java.util.List; -import java.util.function.Supplier; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.Appender; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.MethodParameter; import org.springframework.core.convert.support.DefaultConversionService; -import org.springframework.core.log.LogAccessor; -import org.springframework.test.util.ReflectionTestUtils; import org.springframework.util.ReflectionUtils; import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.multipart.MultipartFile; @@ -44,6 +47,7 @@ * @author Oliver Gierke * @author Chris Bono * @author Mark Paluch + * @author hutiefang * @soundtrack Karlijn Langendijk & Sönke Meinen - Englishman In New York (Sting, * https://www.youtube.com/watch?v=O7LZsqrnaaA) */ @@ -132,39 +136,24 @@ void doesNotSupportAtProjectedPayloadForMultipartParam() { assertThat(resolver.supportsParameter(parameter)).isFalse(); } - @ParameterizedTest // GH-3300 + @ParameterizedTest // GH-3302 @ValueSource(strings = { "withModelAttribute", "withUserUnannotatedInterface" }) - @SuppressWarnings("unchecked") - void deprecationLoggerOnlyLogsOncePerParameter(String methodName) { + void doesNotLogDeprecationForUnsupportedParameter(String methodName) { var parameter = getParameter(methodName); - - // Spy on the actual logger - var actualLoggerSpy = spy(new LogAccessor(ProxyingHandlerMethodArgumentResolver.class)); - ReflectionTestUtils.setField(ProxyingHandlerMethodArgumentResolver.class, "LOGGER", actualLoggerSpy, - LogAccessor.class); - - // Invoke twice but should only log the first time - assertThat(resolver.supportsParameter(parameter)).isFalse(); - verify(actualLoggerSpy, times(1)).warn(any(Supplier.class)); - assertThat(resolver.supportsParameter(parameter)).isFalse(); - verifyNoMoreInteractions(actualLoggerSpy); - } - - @ParameterizedTest // GH-3300 - @ValueSource(strings = { "withProjectedPayload", "withSpringAnnotatedInterface", "withUserAnnotatedInterface" }) - void shouldNotLogDeprecationForValidUsage(String methodName) { - - var parameter = getParameter(methodName); - - // Spy on the actual logger - var actualLoggerSpy = spy(new LogAccessor(ProxyingHandlerMethodArgumentResolver.class)); - ReflectionTestUtils.setField(ProxyingHandlerMethodArgumentResolver.class, "LOGGER", actualLoggerSpy, - LogAccessor.class); - - // Invoke should not log - assertThat(resolver.supportsParameter(parameter)).isTrue(); - verifyNoInteractions(actualLoggerSpy); + Appender appender = mock(); + Logger logger = (Logger) LoggerFactory.getLogger(ProxyingHandlerMethodArgumentResolver.class); + Level previousLevel = logger.getLevel(); + logger.addAppender(appender); + logger.setLevel(Level.WARN); + + try { + assertThat(resolver.supportsParameter(parameter)).isFalse(); + verify(appender, never()).doAppend(any(ILoggingEvent.class)); + } finally { + logger.setLevel(previousLevel); + logger.detachAppender(appender); + } } private static MethodParameter getParameter(String methodName, Class parameterType) {