Support single-value request param resolution for data class constructors

Prior to this commit, when Web MVC attempted to resolve a constructor
argument for a data class constructor with @ModelAttribute binding,
ModelAttributeMethodProcessor failed to unwrap the array returned by
WebRequest.getParameter(String).

According to the Javadoc for WebRequest.getParameter(String), "a
single-value parameter will be exposed as an array with a single
element."

This commit fixes this issue by extracting the single value from such
an array and using that as the constructor argument (potentially
converted by the WebDataBinder).

Closes gh-25200

Author: encircled

spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java

@@ -18,6 +18,7 @@
 
 import java.beans.ConstructorProperties;
 import java.lang.annotation.Annotation;
+import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
@@ -71,6 +72,7 @@
  * @author Rossen Stoyanchev
  * @author Juergen Hoeller
  * @author Sebastien Deleuze
+ * @author Vladislav Kisel
  * @since 3.1
  */
 public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResolver, HandlerMethodReturnValueHandler {
@@ -285,6 +287,12 @@ protected Object constructAttribute(Constructor<?> ctor, String attributeName, M
 					}
 				}
 			}
+
+			// Singular web parameters are wrapped with array, extract it so it can be picked up by conversion service later on
+			if (value != null && value.getClass().isArray() && Array.getLength(value) == 1) {
+				value = Array.get(value, 0);
+			}
+
 			try {
 				MethodParameter methodParam = new FieldAwareConstructorParameter(ctor, i, paramName);
 				if (value == null && methodParam.isOptional()) {

spring-web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 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.
@@ -19,13 +19,16 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.List;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.beans.testfixture.beans.TestBean;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.annotation.SynthesizingMethodParameter;
+import org.springframework.format.support.DefaultFormattingConversionService;
 import org.springframework.validation.BindException;
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.Errors;
@@ -58,6 +61,7 @@
  * Test fixture with {@link ModelAttributeMethodProcessor}.
  *
  * @author Rossen Stoyanchev
+ * @author Vladislav Kisel
  */
 public class ModelAttributeMethodProcessorTests {
 
@@ -73,6 +77,7 @@ public class ModelAttributeMethodProcessorTests {
 	private MethodParameter paramModelAttr;
 	private MethodParameter paramBindingDisabledAttr;
 	private MethodParameter paramNonSimpleType;
+	private MethodParameter beanWithConstructorArgs;
 
 	private MethodParameter returnParamNamedModelAttr;
 	private MethodParameter returnParamNonSimpleType;
@@ -86,14 +91,15 @@ public void setup() throws Exception {
 
 		Method method = ModelAttributeHandler.class.getDeclaredMethod("modelAttribute",
 				TestBean.class, Errors.class, int.class, TestBean.class,
-				TestBean.class, TestBean.class);
+				TestBean.class, TestBean.class, TestBeanWithConstructorArgs.class);
 
 		this.paramNamedValidModelAttr = new SynthesizingMethodParameter(method, 0);
 		this.paramErrors = new SynthesizingMethodParameter(method, 1);
 		this.paramInt = new SynthesizingMethodParameter(method, 2);
 		this.paramModelAttr = new SynthesizingMethodParameter(method, 3);
 		this.paramBindingDisabledAttr = new SynthesizingMethodParameter(method, 4);
 		this.paramNonSimpleType = new SynthesizingMethodParameter(method, 5);
+		this.beanWithConstructorArgs = new SynthesizingMethodParameter(method, 6);
 
 		method = getClass().getDeclaredMethod("annotatedReturnValue");
 		this.returnParamNamedModelAttr = new MethodParameter(method, -1);
@@ -264,6 +270,26 @@ public void handleNotAnnotatedReturnValue() throws Exception {
 		assertThat(this.container.getModel().get("testBean")).isSameAs(testBean);
 	}
 
+	@Test // gh-25182
+	public void testResolveConstructorListParameter() throws Exception {
+		MockHttpServletRequest mockRequest = new MockHttpServletRequest();
+		mockRequest.addParameter("listOfStrings", "1,2");
+		ServletWebRequest requestWithParam = new ServletWebRequest(mockRequest);
+
+		WebDataBinderFactory factory = mock(WebDataBinderFactory.class);
+		given(factory.createBinder(any(), any(), eq("testBeanWithConstructorArgs")))
+				.willAnswer(invocation -> {
+					WebRequestDataBinder binder = new WebRequestDataBinder(invocation.getArgument(1));
+
+					// Add conversion service which will convert "1,2" to List of size 2
+					binder.setConversionService(new DefaultFormattingConversionService());
+					return binder;
+				});
+
+		Object resolved = this.processor.resolveArgument(this.beanWithConstructorArgs, this.container, requestWithParam, factory);
+		assertThat(resolved).isNotNull();
+		assertThat(((TestBeanWithConstructorArgs) resolved).listOfStrings).isEqualTo(Arrays.asList("1", "2"));
+	}
 
 	private void testGetAttributeFromModel(String expectedAttrName, MethodParameter param) throws Exception {
 		Object target = new TestBean();
@@ -330,10 +356,20 @@ public void modelAttribute(
 				int intArg,
 				@ModelAttribute TestBean defaultNameAttr,
 				@ModelAttribute(name="noBindAttr", binding=false) @Valid TestBean noBindAttr,
-				TestBean notAnnotatedAttr) {
+				TestBean notAnnotatedAttr,
+				TestBeanWithConstructorArgs beanWithConstructorArgs) {
 		}
 	}
 
+	static class TestBeanWithConstructorArgs {
+
+		final List<String> listOfStrings;
+
+		public TestBeanWithConstructorArgs(List<String> listOfStrings) {
+			this.listOfStrings = listOfStrings;
+		}
+
+	}
 
 	@ModelAttribute("modelAttrName") @SuppressWarnings("unused")
 	private String annotatedReturnValue() {