Polishing.

Refine message. Use explicit variable types instead of var. Add tests.

See #3300
Original pull request: #3303

Author: mp911de

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

@@ -17,8 +17,10 @@
 
 import java.lang.annotation.Annotation;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.Set;
 
 import org.springframework.beans.BeansException;
 import org.springframework.beans.MutablePropertyValues;
@@ -45,6 +47,7 @@
  *
  * @author Oliver Gierke
  * @author Chris Bono
+ * @author Mark Paluch
  * @since 1.10
  */
 public class ProxyingHandlerMethodArgumentResolver extends ModelAttributeMethodProcessor
@@ -154,9 +157,9 @@ protected void bindRequestParameters(WebDataBinder binder, NativeWebRequest requ
 	 */
 	static class ProjectedPayloadDeprecationLogger {
 
-		private static final String MESSAGE = "Parameter%sat position %s in %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 static final String MESSAGE = "Parameter %sat index %s in [%s] is not annotated with @ProjectedPayload. Support for parameters not annotated with @ProjectedPayload (at the parameter or type level) will be dropped in a future version.";
 
-		private final ConcurrentHashMap<MethodParameter, Boolean> loggedParameters = new ConcurrentHashMap<>();
+		private final Set<MethodParameter> loggedParameters = Collections.synchronizedSet(new HashSet<>());
 
 		/**
 		 * Log a warning the first time a non-annotated method parameter is encountered.
@@ -165,12 +168,15 @@ static class ProjectedPayloadDeprecationLogger {
 		 */
 		void logDeprecationForParameter(MethodParameter parameter) {
 
-			if (this.loggedParameters.putIfAbsent(parameter, Boolean.TRUE) == null) {
-				var paramName = parameter.getParameterName();
-				var paramNameOrEmpty = paramName != null ? (" '" + paramName + "' ") : " ";
-				var methodName = parameter.getMethod() != null ? parameter.getMethod().getName() : "constructor";
-				LOGGER.warn(() -> MESSAGE.formatted(paramNameOrEmpty, parameter.getParameterIndex(), parameter.getContainingClass().getName(), methodName));
+			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));
 		}
 
 	}

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

@@ -20,10 +20,13 @@
 
 import example.SampleInterface;
 
+import java.lang.reflect.Method;
 import java.util.List;
 import java.util.function.Supplier;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.MethodParameter;
@@ -39,6 +42,7 @@
  *
  * @author Oliver Gierke
  * @author Chris Bono
+ * @author Mark Paluch
  * @soundtrack Karlijn Langendijk & Sönke Meinen - Englishman In New York (Sting,
  *             https://www.youtube.com/watch?v=O7LZsqrnaaA)
  */
@@ -126,9 +130,9 @@ void deprecationLoggerOnlyLogsOncePerParameter() {
 		var parameter = getParameter("withModelAttribute", SampleInterface.class);
 
 		// Spy on the actual logger
-		var actualLogger = (LogAccessor) ReflectionTestUtils.getField(ProxyingHandlerMethodArgumentResolver.class, "LOGGER");
-		var actualLoggerSpy = spy(actualLogger);
-		ReflectionTestUtils.setField(ProxyingHandlerMethodArgumentResolver.class, "LOGGER", actualLoggerSpy, LogAccessor.class);
+		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)).isTrue();
@@ -137,12 +141,40 @@ void deprecationLoggerOnlyLogsOncePerParameter() {
 		verifyNoMoreInteractions(actualLoggerSpy);
 	}
 
+	@ParameterizedTest // GH-3300
+	@ValueSource(strings = { "withProjectedPayload", "withAnnotatedInterface" })
+	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 twice but should only log the first time
+		assertThat(resolver.supportsParameter(parameter)).isTrue();
+		verifyNoInteractions(actualLoggerSpy);
+	}
+
 	private static MethodParameter getParameter(String methodName, Class<?> parameterType) {
 
 		var method = ReflectionUtils.findMethod(Controller.class, methodName, parameterType);
 		return new MethodParameter(method, 0);
 	}
 
+	private static MethodParameter getParameter(String methodName) {
+
+		for (Method method : Controller.class.getMethods()) {
+
+			if (method.getName().equals(methodName)) {
+				return new MethodParameter(method, 0);
+			}
+		}
+
+		throw new NoSuchMethodError(methodName);
+	}
+
 	@ProjectedPayload
 	interface AnnotatedInterface {}
 
@@ -152,6 +184,8 @@ interface Controller {
 
 		void with(AnnotatedInterface param);
 
+		void withAnnotatedInterface(@ModelAttribute AnnotatedInterface param);
+
 		void with(UnannotatedInterface param);
 
 		void with(SampleInterface param);