Reverse SPR-10402 change that caused 3.2.3 regression

rstoyanchev · rstoyanchev · commit 2d8315fba04f · 2013-06-20T16:20:01.000-04:00
SPR-10402 in Spring Framework 3.2.3 treated empty request parameter
values as missing values, if the empty value was turned into a null
by a PropertyEditor or Converter. This caused the regression.

Issue: SPR-10578, SPR-10402, SPR-10584
diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -97,17 +97,11 @@ else if ("".equals(arg) && (namedValueInfo.defaultValue != null)) {
 			arg = resolveDefaultValue(namedValueInfo.defaultValue);
 		}
 
-		boolean emptyArgValue = "".equals(arg);
-
 		if (binderFactory != null) {
 			WebDataBinder binder = binderFactory.createBinder(webRequest, null, namedValueInfo.name);
 			arg = binder.convertIfNecessary(arg, paramType, parameter);
 		}
 
-		if (emptyArgValue && (arg == null)) {
-			handleMissingValue(namedValueInfo.name, parameter);
-		}
-
 		handleResolvedValue(arg, namedValueInfo.name, parameter, mavContainer, webRequest);
 
 		return arg;
diff --git a/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -49,6 +49,7 @@
 import static org.mockito.BDDMockito.*;
 import static org.mockito.Mockito.*;
 
+
 /**
  * Test fixture with {@link org.springframework.web.method.annotation.RequestParamMethodArgumentResolver}.
  *
@@ -70,6 +71,7 @@ public class RequestParamMethodArgumentResolverTests {
 	private MethodParameter paramServlet30Part;
 	private MethodParameter paramRequestPartAnnot;
 	private MethodParameter paramRequired;
+	private MethodParameter paramNotRequired;
 
 	private NativeWebRequest webRequest;
 
@@ -81,8 +83,10 @@ public void setUp() throws Exception {
 
 		ParameterNameDiscoverer paramNameDiscoverer = new LocalVariableTableParameterNameDiscoverer();
 
-		Method method = getClass().getMethod("params", String.class, String[].class, Map.class, MultipartFile.class,
-				Map.class, String.class, MultipartFile.class, List.class, Part.class, MultipartFile.class, String.class);
+		Method method = getClass().getMethod("params", String.class, String[].class,
+				Map.class, MultipartFile.class, Map.class, String.class,
+				MultipartFile.class, List.class, Part.class, MultipartFile.class,
+				String.class, String.class);
 
 		paramNamedDefaultValueString = new MethodParameter(method, 0);
 		paramNamedStringArray = new MethodParameter(method, 1);
@@ -99,6 +103,7 @@ public void setUp() throws Exception {
 		paramServlet30Part.initParameterNameDiscovery(paramNameDiscoverer);
 		paramRequestPartAnnot = new MethodParameter(method, 9);
 		paramRequired = new MethodParameter(method, 10);
+		paramNotRequired = new MethodParameter(method, 11);
 
 		request = new MockHttpServletRequest();
 		webRequest = new ServletWebRequest(request, new MockHttpServletResponse());
@@ -243,9 +248,9 @@ public void missingRequestParam() throws Exception {
 		fail("Expected exception");
 	}
 
-	// SPR-10402
+	// SPR-10578
 
-	@Test(expected = MissingServletRequestParameterException.class)
+	@Test
 	public void missingRequestParamEmptyValueConvertedToNull() throws Exception {
 
 		WebDataBinder binder = new WebRequestDataBinder(null);
@@ -256,8 +261,25 @@ public void missingRequestParamEmptyValueConvertedToNull() throws Exception {
 
 		this.request.addParameter("stringNotAnnot", "");
 
-		resolver.resolveArgument(paramStringNotAnnot, null, webRequest, binderFactory);
-		fail("Expected exception");
+		Object arg = resolver.resolveArgument(paramStringNotAnnot, null, webRequest, binderFactory);
+
+		assertNull(arg);
+	}
+
+	@Test
+	public void missingRequestParamEmptyValueNotRequired() throws Exception {
+
+		WebDataBinder binder = new WebRequestDataBinder(null);
+		binder.registerCustomEditor(String.class, new StringTrimmerEditor(true));
+
+		WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
+		given(binderFactory.createBinder(webRequest, null, "name")).willReturn(binder);
+
+		this.request.addParameter("name", "");
+
+		Object arg = resolver.resolveArgument(paramNotRequired, null, webRequest, binderFactory);
+
+		assertNull(arg);
 	}
 
 	@Test
@@ -311,7 +333,8 @@ public void params(@RequestParam(value = "name", defaultValue = "bar") String pa
 			List<MultipartFile> multipartFileList,
 			Part servlet30Part,
 			@RequestPart MultipartFile requestPartAnnot,
-			@RequestParam(value = "name") String paramRequired) {
+			@RequestParam(value = "name") String paramRequired,
+			@RequestParam(value = "name", required=false) String paramNotRequired) {
 	}
 
 }
