Skip to content

Commit cb4338e

Browse files
committed
Polishing.
Refine message. Use explicit variable types instead of var. Add tests. See #3300 Original pull request: #3303
1 parent dc51b4a commit cb4338e

File tree

2 files changed

+51
-11
lines changed

2 files changed

+51
-11
lines changed

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717

1818
import java.lang.annotation.Annotation;
1919
import java.util.Arrays;
20+
import java.util.Collections;
21+
import java.util.HashSet;
2022
import java.util.List;
21-
import java.util.concurrent.ConcurrentHashMap;
23+
import java.util.Set;
2224

2325
import org.springframework.beans.BeansException;
2426
import org.springframework.beans.MutablePropertyValues;
@@ -45,6 +47,7 @@
4547
*
4648
* @author Oliver Gierke
4749
* @author Chris Bono
50+
* @author Mark Paluch
4851
* @since 1.10
4952
*/
5053
public class ProxyingHandlerMethodArgumentResolver extends ModelAttributeMethodProcessor
@@ -154,9 +157,9 @@ protected void bindRequestParameters(WebDataBinder binder, NativeWebRequest requ
154157
*/
155158
static class ProjectedPayloadDeprecationLogger {
156159

157-
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.";
160+
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.";
158161

159-
private final ConcurrentHashMap<MethodParameter, Boolean> loggedParameters = new ConcurrentHashMap<>();
162+
private final Set<MethodParameter> loggedParameters = Collections.synchronizedSet(new HashSet<>());
160163

161164
/**
162165
* Log a warning the first time a non-annotated method parameter is encountered.
@@ -165,12 +168,15 @@ static class ProjectedPayloadDeprecationLogger {
165168
*/
166169
void logDeprecationForParameter(MethodParameter parameter) {
167170

168-
if (this.loggedParameters.putIfAbsent(parameter, Boolean.TRUE) == null) {
169-
var paramName = parameter.getParameterName();
170-
var paramNameOrEmpty = paramName != null ? (" '" + paramName + "' ") : " ";
171-
var methodName = parameter.getMethod() != null ? parameter.getMethod().getName() : "constructor";
172-
LOGGER.warn(() -> MESSAGE.formatted(paramNameOrEmpty, parameter.getParameterIndex(), parameter.getContainingClass().getName(), methodName));
171+
if (!this.loggedParameters.add(parameter)) {
172+
return;
173173
}
174+
175+
String paramName = parameter.getParameterName();
176+
String paramNameOrEmpty = paramName != null ? ("'" + paramName + "' ") : "";
177+
String methodName = parameter.getMethod() != null ? parameter.getMethod().toGenericString() : "constructor";
178+
179+
LOGGER.warn(() -> MESSAGE.formatted(paramNameOrEmpty, parameter.getParameterIndex(), methodName));
174180
}
175181

176182
}

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

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,13 @@
2020

2121
import example.SampleInterface;
2222

23+
import java.lang.reflect.Method;
2324
import java.util.List;
2425
import java.util.function.Supplier;
2526

2627
import org.junit.jupiter.api.Test;
28+
import org.junit.jupiter.params.ParameterizedTest;
29+
import org.junit.jupiter.params.provider.ValueSource;
2730

2831
import org.springframework.beans.factory.annotation.Autowired;
2932
import org.springframework.core.MethodParameter;
@@ -39,6 +42,7 @@
3942
*
4043
* @author Oliver Gierke
4144
* @author Chris Bono
45+
* @author Mark Paluch
4246
* @soundtrack Karlijn Langendijk & Sönke Meinen - Englishman In New York (Sting,
4347
* https://www.youtube.com/watch?v=O7LZsqrnaaA)
4448
*/
@@ -126,9 +130,9 @@ void deprecationLoggerOnlyLogsOncePerParameter() {
126130
var parameter = getParameter("withModelAttribute", SampleInterface.class);
127131

128132
// Spy on the actual logger
129-
var actualLogger = (LogAccessor) ReflectionTestUtils.getField(ProxyingHandlerMethodArgumentResolver.class, "LOGGER");
130-
var actualLoggerSpy = spy(actualLogger);
131-
ReflectionTestUtils.setField(ProxyingHandlerMethodArgumentResolver.class, "LOGGER", actualLoggerSpy, LogAccessor.class);
133+
var actualLoggerSpy = spy(new LogAccessor(ProxyingHandlerMethodArgumentResolver.class));
134+
ReflectionTestUtils.setField(ProxyingHandlerMethodArgumentResolver.class, "LOGGER", actualLoggerSpy,
135+
LogAccessor.class);
132136

133137
// Invoke twice but should only log the first time
134138
assertThat(resolver.supportsParameter(parameter)).isTrue();
@@ -137,12 +141,40 @@ void deprecationLoggerOnlyLogsOncePerParameter() {
137141
verifyNoMoreInteractions(actualLoggerSpy);
138142
}
139143

144+
@ParameterizedTest // GH-3300
145+
@ValueSource(strings = { "withProjectedPayload", "withAnnotatedInterface" })
146+
void shouldNotLogDeprecationForValidUsage(String methodName) {
147+
148+
var parameter = getParameter(methodName);
149+
150+
// Spy on the actual logger
151+
var actualLoggerSpy = spy(new LogAccessor(ProxyingHandlerMethodArgumentResolver.class));
152+
ReflectionTestUtils.setField(ProxyingHandlerMethodArgumentResolver.class, "LOGGER", actualLoggerSpy,
153+
LogAccessor.class);
154+
155+
// Invoke twice but should only log the first time
156+
assertThat(resolver.supportsParameter(parameter)).isTrue();
157+
verifyNoInteractions(actualLoggerSpy);
158+
}
159+
140160
private static MethodParameter getParameter(String methodName, Class<?> parameterType) {
141161

142162
var method = ReflectionUtils.findMethod(Controller.class, methodName, parameterType);
143163
return new MethodParameter(method, 0);
144164
}
145165

166+
private static MethodParameter getParameter(String methodName) {
167+
168+
for (Method method : Controller.class.getMethods()) {
169+
170+
if (method.getName().equals(methodName)) {
171+
return new MethodParameter(method, 0);
172+
}
173+
}
174+
175+
throw new NoSuchMethodError(methodName);
176+
}
177+
146178
@ProjectedPayload
147179
interface AnnotatedInterface {}
148180

@@ -152,6 +184,8 @@ interface Controller {
152184

153185
void with(AnnotatedInterface param);
154186

187+
void withAnnotatedInterface(@ModelAttribute AnnotatedInterface param);
188+
155189
void with(UnannotatedInterface param);
156190

157191
void with(SampleInterface param);

0 commit comments

Comments
 (0)