Log a warning if param not annotated with @ProjectedPayload.

This commit introduces a warning log if a parameter is not annotated with `@ProjectedPayload` that this style is deprecated and that we will drop support for projections if a parameter (or the parameter type) is not explicitly annotated with `@ProjectedPayload`.

Resolves #3300

Signed-off-by: Chris Bono <[email protected]>

Author: onobc

src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java

@@ -18,6 +18,7 @@
 import java.lang.annotation.Annotation;
 import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.springframework.beans.BeansException;
 import org.springframework.beans.MutablePropertyValues;
@@ -28,6 +29,7 @@
 import org.springframework.core.MethodParameter;
 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.web.bind.WebDataBinder;
@@ -52,9 +54,10 @@ public class ProxyingHandlerMethodArgumentResolver extends ModelAttributeMethodP
 
 	private final SpelAwareProxyProjectionFactory proxyFactory;
 	private final ObjectFactory<ConversionService> conversionService;
+	private final ProjectedPayloadDeprecationLogger deprecationLogger = new ProjectedPayloadDeprecationLogger();
 
 	/**
-	 * Creates a new {@link PageableHandlerMethodArgumentResolver} using the given {@link ConversionService}.
+	 * Creates a new {@link ProxyingHandlerMethodArgumentResolver} using the given {@link ConversionService}.
 	 *
 	 * @param conversionService must not be {@literal null}.
 	 */
@@ -80,28 +83,36 @@ public void setBeanClassLoader(ClassLoader classLoader) {
 	@Override
 	public boolean supportsParameter(MethodParameter parameter) {
 
+		// Simple type or not annotated with @ModelAttribute (and flag set to require annotation)
 		if (!super.supportsParameter(parameter)) {
 			return false;
 		}
 
 		Class<?> type = parameter.getParameterType();
 
+		// Only interfaces can be proxied
 		if (!type.isInterface()) {
 			return false;
 		}
 
-		// Annotated parameter (excluding multipart)
-		if ((parameter.hasParameterAnnotation(ProjectedPayload.class) || parameter.hasParameterAnnotation(
-				ModelAttribute.class)) && !MultipartResolutionDelegate.isMultipartArgument(parameter)) {
+		// Multipart not currently supported
+		if (MultipartResolutionDelegate.isMultipartArgument(parameter)) {
+			return false;
+		}
+
+		// Type or parameter explicitly annotated with @ProjectedPayload
+		if (parameter.hasParameterAnnotation(ProjectedPayload.class) || AnnotatedElementUtils.findMergedAnnotation(type,
+				ProjectedPayload.class) != null) {
 			return true;
 		}
 
-		// Annotated type
-		if (AnnotatedElementUtils.findMergedAnnotation(type, ProjectedPayload.class) != null) {
+		// Parameter annotated with @ModelAttribute
+		if (parameter.hasParameterAnnotation(ModelAttribute.class)) {
+			this.deprecationLogger.logDeprecationForParameter(parameter);
 			return true;
 		}
 
-		// Exclude parameters annotated with Spring annotation
+		// Exclude any other parameters annotated with Spring annotation
 		if (Arrays.stream(parameter.getParameterAnnotations())
 				.map(Annotation::annotationType)
 				.map(Class::getPackageName)
@@ -112,8 +123,12 @@ public boolean supportsParameter(MethodParameter parameter) {
 
 		// Fallback for only user defined interfaces
 		String packageName = ClassUtils.getPackageName(type);
+		if (IGNORED_PACKAGES.stream().noneMatch(packageName::startsWith)) {
+			this.deprecationLogger.logDeprecationForParameter(parameter);
+			return true;
+		}
 
-		return !IGNORED_PACKAGES.stream().anyMatch(it -> packageName.startsWith(it));
+		return false;
 	}
 
 	@Override
@@ -128,4 +143,33 @@ 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}.
+	 * <p>
+	 * 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 %s (%s) is not annotated with @ProjectedPayload - support for parameters not explicitly annotated with @ProjectedPayload (at the parameter or type level) will be dropped in a future version.";
+
+		private final LogAccessor logger = new LogAccessor(ProxyingHandlerMethodArgumentResolver.class);
+
+		private final ConcurrentHashMap<MethodParameter, Boolean> loggedParameters = new ConcurrentHashMap<>();
+
+		/**
+		 * Log a warning the first time a non-annotated method parameter is encountered.
+		 *
+		 * @param parameter the parameter
+		 */
+		void logDeprecationForParameter(MethodParameter parameter) {
+
+			if (this.loggedParameters.putIfAbsent(parameter, Boolean.TRUE) == null) {
+				var paramName = parameter.getParameterName();
+				this.logger.warn(() -> MESSAGE.formatted(paramName != null ? paramName : "", parameter));
+			}
+		}
+
+	}
+
 }

src/test/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolverUnitTests.java

@@ -16,16 +16,21 @@
 package org.springframework.data.web;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
 
 import example.SampleInterface;
 
 import java.util.List;
+import java.util.function.Supplier;
 
 import org.junit.jupiter.api.Test;
 
 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.data.web.ProxyingHandlerMethodArgumentResolver.ProjectedPayloadDeprecationLogger;
+import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.web.bind.annotation.ModelAttribute;
 import org.springframework.web.multipart.MultipartFile;
@@ -115,6 +120,25 @@ void doesNotSupportAtProjectedPayloadForMultipartParam() {
 		assertThat(resolver.supportsParameter(parameter)).isFalse();
 	}
 
+	@Test // GH-3300
+	@SuppressWarnings("unchecked")
+	void deprecationLoggerOnlyLogsOncePerParameter() {
+
+		var parameter = getParameter("withModelAttribute", SampleInterface.class);
+
+		// Spy on the actual logger in the helper class
+		var deprecationLogger = (ProjectedPayloadDeprecationLogger) ReflectionTestUtils.getField(resolver, "deprecationLogger");
+		var actualLogger = (LogAccessor) ReflectionTestUtils.getField(deprecationLogger, "logger");
+		var actualLoggerSpy = spy(actualLogger);
+		ReflectionTestUtils.setField(deprecationLogger, "logger", actualLoggerSpy);
+
+		// Invoke twice but should only log the first time
+		assertThat(resolver.supportsParameter(parameter)).isTrue();
+		verify(actualLoggerSpy, times(1)).warn(any(Supplier.class));
+		assertThat(resolver.supportsParameter(parameter)).isTrue();
+		verifyNoMoreInteractions(actualLoggerSpy);
+	}
+
 	private static MethodParameter getParameter(String methodName, Class<?> parameterType) {
 
 		var method = ReflectionUtils.findMethod(Controller.class, methodName, parameterType);