Expose parameter/field name for non-JavaBeans type conversion

Supports name-bound PropertyEditor registrations on data classes. Includes consistent support for field-aware method parameters. Closes gh-28284
spring-projects · Jun 2, 2023 · 8c6287e · 8c6287e
1 parent 322cbca
commit 8c6287e
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 77 deletions.
diff --git a/spring-beans/src/main/java/org/springframework/beans/TypeConverterSupport.java b/spring-beans/src/main/java/org/springframework/beans/TypeConverterSupport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -42,15 +42,15 @@ public abstract class TypeConverterSupport extends PropertyEditorRegistrySupport
 	@Override
 	@Nullable
 	public <T> T convertIfNecessary(@Nullable Object value, @Nullable Class<T> requiredType) throws TypeMismatchException {
-		return convertIfNecessary(value, requiredType, TypeDescriptor.valueOf(requiredType));
+		return convertIfNecessary(null, value, requiredType, TypeDescriptor.valueOf(requiredType));
 	}
 
 	@Override
 	@Nullable
 	public <T> T convertIfNecessary(@Nullable Object value, @Nullable Class<T> requiredType,
 			@Nullable MethodParameter methodParam) throws TypeMismatchException {
 
-		return convertIfNecessary(value, requiredType,
+		return convertIfNecessary((methodParam != null ? methodParam.getParameterName() : null), value, requiredType,
 				(methodParam != null ? new TypeDescriptor(methodParam) : TypeDescriptor.valueOf(requiredType)));
 	}
 
@@ -59,18 +59,26 @@ public <T> T convertIfNecessary(@Nullable Object value, @Nullable Class<T> requi
 	public <T> T convertIfNecessary(@Nullable Object value, @Nullable Class<T> requiredType, @Nullable Field field)
 			throws TypeMismatchException {
 
-		return convertIfNecessary(value, requiredType,
+		return convertIfNecessary((field != null ? field.getName() : null), value, requiredType,
 				(field != null ? new TypeDescriptor(field) : TypeDescriptor.valueOf(requiredType)));
 	}
 
-	@Nullable
 	@Override
+	@Nullable
 	public <T> T convertIfNecessary(@Nullable Object value, @Nullable Class<T> requiredType,
 			@Nullable TypeDescriptor typeDescriptor) throws TypeMismatchException {
 
+		return convertIfNecessary(null, value, requiredType, typeDescriptor);
+	}
+
+	@Nullable
+	private <T> T convertIfNecessary(@Nullable String propertyName, @Nullable Object value,
+			@Nullable Class<T> requiredType, @Nullable TypeDescriptor typeDescriptor) throws TypeMismatchException {
+
 		Assert.state(this.typeConverterDelegate != null, "No TypeConverterDelegate");
 		try {
-			return this.typeConverterDelegate.convertIfNecessary(null, null, value, requiredType, typeDescriptor);
+			return this.typeConverterDelegate.convertIfNecessary(
+					propertyName, null, value, requiredType, typeDescriptor);
 		}
 		catch (ConverterNotFoundException | IllegalStateException ex) {
 			throw new ConversionNotSupportedException(value, requiredType, ex);

diff --git a/spring-core/src/main/java/org/springframework/core/MethodParameter.java b/spring-core/src/main/java/org/springframework/core/MethodParameter.java
@@ -20,12 +20,16 @@
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Executable;
+import java.lang.reflect.Field;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.function.Predicate;
@@ -93,7 +97,7 @@ public class MethodParameter {
 	private volatile ParameterNameDiscoverer parameterNameDiscoverer;
 
 	@Nullable
-	private volatile String parameterName;
+	volatile String parameterName;
 
 	@Nullable
 	private volatile MethodParameter nestedMethodParameter;
@@ -780,6 +784,7 @@ public MethodParameter clone() {
 		return new MethodParameter(this);
 	}
 
+
 	/**
 	 * Create a new MethodParameter for the given method or constructor.
 	 * <p>This is a convenience factory method for scenarios where a
@@ -858,6 +863,74 @@ private static int validateIndex(Executable executable, int parameterIndex) {
 		return parameterIndex;
 	}
 
+	/**
+	 * Create a new MethodParameter for the given field-aware constructor,
+	 * e.g. on a data class or record type.
+	 * <p>A field-aware method parameter will detect field annotations as well,
+	 * as long as the field name matches the parameter name.
+	 * @param ctor the Constructor to specify a parameter for
+	 * @param parameterIndex the index of the parameter
+	 * @param fieldName the name of the underlying field,
+	 * matching the constructor's parameter name
+	 * @return the corresponding MethodParameter instance
+	 * @since 6.1
+	 */
+	public static MethodParameter forFieldAwareConstructor(Constructor<?> ctor, int parameterIndex, String fieldName) {
+		return new FieldAwareConstructorParameter(ctor, parameterIndex, fieldName);
+	}
+
+
+	/**
+	 * {@link MethodParameter} subclass which detects field annotations as well.
+	 */
+	private static class FieldAwareConstructorParameter extends MethodParameter {
+
+		@Nullable
+		private volatile Annotation[] combinedAnnotations;
+
+		public FieldAwareConstructorParameter(Constructor<?> constructor, int parameterIndex, String fieldName) {
+			super(constructor, parameterIndex);
+			this.parameterName = fieldName;
+		}
+
+		@Override
+		public Annotation[] getParameterAnnotations() {
+			String parameterName = this.parameterName;
+			Assert.state(parameterName != null, "Parameter name not initialized");
+
+			Annotation[] anns = this.combinedAnnotations;
+			if (anns == null) {
+				anns = super.getParameterAnnotations();
+				try {
+					Field field = getDeclaringClass().getDeclaredField(parameterName);
+					Annotation[] fieldAnns = field.getAnnotations();
+					if (fieldAnns.length > 0) {
+						List<Annotation> merged = new ArrayList<>(anns.length + fieldAnns.length);
+						merged.addAll(Arrays.asList(anns));
+						for (Annotation fieldAnn : fieldAnns) {
+							boolean existingType = false;
+							for (Annotation ann : anns) {
+								if (ann.annotationType() == fieldAnn.annotationType()) {
+									existingType = true;
+									break;
+								}
+							}
+							if (!existingType) {
+								merged.add(fieldAnn);
+							}
+						}
+						anns = merged.toArray(new Annotation[0]);
+					}
+				}
+				catch (NoSuchFieldException | SecurityException ex) {
+					// ignore
+				}
+				this.combinedAnnotations = anns;
+			}
+			return anns;
+		}
+	}
+
 
 	/**
 	 * Inner class to avoid a hard dependency on Kotlin at runtime.

diff --git a/...rc/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java b/...rc/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java
@@ -19,9 +19,6 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
-import java.lang.reflect.Field;
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -287,7 +284,7 @@ protected Object constructAttribute(Constructor<?> ctor, String attributeName, M
 			}
 
 			try {
-				MethodParameter methodParam = new FieldAwareConstructorParameter(ctor, i, paramName);
+				MethodParameter methodParam = MethodParameter.forFieldAwareConstructor(ctor, i, paramName);
 				if (value == null && methodParam.isOptional()) {
 					args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null);
 				}
@@ -487,61 +484,4 @@ public void handleReturnValue(@Nullable Object returnValue, MethodParameter retu
 		}
 	}
 
-
-	/**
-	 * {@link MethodParameter} subclass which detects field annotations as well.
-	 * @since 5.1
-	 */
-	private static class FieldAwareConstructorParameter extends MethodParameter {
-
-		private final String parameterName;
-
-		@Nullable
-		private volatile Annotation[] combinedAnnotations;
-
-		public FieldAwareConstructorParameter(Constructor<?> constructor, int parameterIndex, String parameterName) {
-			super(constructor, parameterIndex);
-			this.parameterName = parameterName;
-		}
-
-		@Override
-		public Annotation[] getParameterAnnotations() {
-			Annotation[] anns = this.combinedAnnotations;
-			if (anns == null) {
-				anns = super.getParameterAnnotations();
-				try {
-					Field field = getDeclaringClass().getDeclaredField(this.parameterName);
-					Annotation[] fieldAnns = field.getAnnotations();
-					if (fieldAnns.length > 0) {
-						List<Annotation> merged = new ArrayList<>(anns.length + fieldAnns.length);
-						merged.addAll(Arrays.asList(anns));
-						for (Annotation fieldAnn : fieldAnns) {
-							boolean existingType = false;
-							for (Annotation ann : anns) {
-								if (ann.annotationType() == fieldAnn.annotationType()) {
-									existingType = true;
-									break;
-								}
-							}
-							if (!existingType) {
-								merged.add(fieldAnn);
-							}
-						}
-						anns = merged.toArray(new Annotation[0]);
-					}
-				}
-				catch (NoSuchFieldException | SecurityException ex) {
-					// ignore
-				}
-				this.combinedAnnotations = anns;
-			}
-			return anns;
-		}
-
-		@Override
-		public String getParameterName() {
-			return this.parameterName;
-		}
-	}
-
 }
diff --git a/...framework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java b/...framework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java
@@ -250,7 +250,7 @@ private Mono<?> constructAttribute(Constructor<?> ctor, String attributeName,
 					}
 				}
 				value = (value instanceof List<?> list ? list.toArray() : value);
-				MethodParameter methodParam = new MethodParameter(ctor, i);
+				MethodParameter methodParam = MethodParameter.forFieldAwareConstructor(ctor, i, paramName);
 				if (value == null && methodParam.isOptional()) {
 					args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null);
 				}

diff --git a/...work/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java b/...work/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -28,6 +28,7 @@
 import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
+import org.springframework.beans.propertyeditors.StringTrimmerEditor;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.http.MediaType;
@@ -64,6 +65,8 @@ void setup() {
 		LocalValidatorFactoryBean validator = new LocalValidatorFactoryBean();
 		validator.afterPropertiesSet();
 		ConfigurableWebBindingInitializer initializer = new ConfigurableWebBindingInitializer();
+		initializer.setPropertyEditorRegistrar(registry ->
+				registry.registerCustomEditor(String.class, "name", new StringTrimmerEditor(true)));
 		initializer.setValidator(validator);
 		this.bindContext = new BindingContext(initializer);
 	}
@@ -268,7 +271,7 @@ private void testBindPojo(String modelKey, MethodParameter param, Function<Objec
 			throws Exception {
 
 		Object value = createResolver()
-				.resolveArgument(param, this.bindContext, postForm("name=Robert&age=25"))
+				.resolveArgument(param, this.bindContext, postForm("name= Robert&age=25"))
 				.block(Duration.ZERO);
 
 		Pojo pojo = valueExtractor.apply(value);
@@ -377,7 +380,7 @@ void bindDataClass() throws Exception {
 		MethodParameter parameter = this.testMethod.annotNotPresent(ModelAttribute.class).arg(DataClass.class);
 
 		Object value = createResolver()
-				.resolveArgument(parameter, this.bindContext, postForm("name=Robert&age=25&count=1"))
+				.resolveArgument(parameter, this.bindContext, postForm("name= Robert&age=25&count=1"))
 				.block(Duration.ZERO);
 
 		DataClass dataClass = (DataClass) value;

diff --git a/...work/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/...work/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java
@@ -70,6 +70,7 @@
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.beans.propertyeditors.CustomDateEditor;
+import org.springframework.beans.propertyeditors.StringTrimmerEditor;
 import org.springframework.beans.testfixture.beans.DerivedTestBean;
 import org.springframework.beans.testfixture.beans.GenericBean;
 import org.springframework.beans.testfixture.beans.ITestBean;
@@ -2051,7 +2052,7 @@ void dataClassBindingWithResult(boolean usePathPatterns) throws Exception {
 		initDispatcherServlet(ValidatedDataClassController.class, usePathPatterns);
 
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind");
-		request.addParameter("param1", "value1");
+		request.addParameter("param1", " value1");
 		request.addParameter("param2", "true");
 		request.addParameter("param3", "3");
 		MockHttpServletResponse response = new MockHttpServletResponse();
@@ -2064,7 +2065,7 @@ void dataClassBindingWithOptionalParameter(boolean usePathPatterns) throws Excep
 		initDispatcherServlet(ValidatedDataClassController.class, usePathPatterns);
 
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind");
-		request.addParameter("param1", "value1");
+		request.addParameter("param1", " value1");
 		request.addParameter("param2", "true");
 		request.addParameter("optionalParam", "8");
 		MockHttpServletResponse response = new MockHttpServletResponse();
@@ -2077,7 +2078,7 @@ void dataClassBindingWithMissingParameter(boolean usePathPatterns) throws Except
 		initDispatcherServlet(ValidatedDataClassController.class, usePathPatterns);
 
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind");
-		request.addParameter("param1", "value1");
+		request.addParameter("param1", " value1");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		getServlet().service(request, response);
 		assertThat(response.getContentAsString()).isEqualTo("1:value1-null-null");
@@ -2088,7 +2089,7 @@ void dataClassBindingWithConversionError(boolean usePathPatterns) throws Excepti
 		initDispatcherServlet(ValidatedDataClassController.class, usePathPatterns);
 
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind");
-		request.addParameter("param1", "value1");
+		request.addParameter("param1", " value1");
 		request.addParameter("param2", "x");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		getServlet().service(request, response);
@@ -2104,7 +2105,7 @@ void dataClassBindingWithValidationError(boolean usePathPatterns) throws Excepti
 		request.addParameter("param3", "0");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		getServlet().service(request, response);
-		assertThat(response.getContentAsString()).isEqualTo("1:null-true-0");
+		assertThat(response.getContentAsString()).isEqualTo("1:-true-0");
 	}
 
 	@PathPatternsParameterizedTest
@@ -3990,6 +3991,7 @@ static class ValidatedDataClassController {
 		@InitBinder
 		public void initBinder(WebDataBinder binder) {
 			binder.setConversionService(new DefaultFormattingConversionService());
+			binder.registerCustomEditor(String.class, "param1", new StringTrimmerEditor(true));
 			LocalValidatorFactoryBean vf = new LocalValidatorFactoryBean();
 			vf.afterPropertiesSet();
 			binder.setValidator(vf);