SPR-8483 Add integration test for accessing multipart request parts with @RequestPart

org.springframework.web.servlet/.classpath

@@ -40,6 +40,8 @@
 	<classpathentry kind="var" path="IVY_CACHE/javax.validation/com.springsource.javax.validation/1.0.0.GA/com.springsource.javax.validation-1.0.0.GA.jar" sourcepath="/IVY_CACHE/javax.validation/com.springsource.javax.validation/1.0.0/com.springsource.javax.validation-sources-1.0.0.GA.jar"/>
 	<classpathentry kind="var" path="IVY_CACHE/org.slf4j/com.springsource.slf4j.jcl/1.5.3/com.springsource.slf4j.jcl-1.5.3.jar" sourcepath="/IVY_CACHE/org.slf4j/com.springsource.slf4j.jcl/1.5.3/com.springsource.slf4j.jcl-sources-1.5.3.jar"/>
 	<classpathentry kind="var" path="IVY_CACHE/org.slf4j/com.springsource.slf4j.api/1.5.3/com.springsource.slf4j.api-1.5.3.jar" sourcepath="/IVY_CACHE/org.slf4j/com.springsource.slf4j.api/1.5.3/com.springsource.slf4j.api-sources-1.5.3.jar"/>
+	<classpathentry kind="var" path="IVY_CACHE/org.mortbay.jetty/com.springsource.org.mortbay.jetty.server/6.1.9/com.springsource.org.mortbay.jetty.server-6.1.9.jar" sourcepath="/IVY_CACHE/org.mortbay.jetty/com.springsource.org.mortbay.jetty.server/6.1.9/com.springsource.org.mortbay.jetty.server-sources-6.1.9.jar"/>
+	<classpathentry kind="var" path="IVY_CACHE/org.mortbay.jetty/com.springsource.org.mortbay.util/6.1.9/com.springsource.org.mortbay.util-6.1.9.jar" sourcepath="/IVY_CACHE/org.mortbay.jetty/com.springsource.org.mortbay.util/6.1.9/com.springsource.org.mortbay.util-sources-6.1.9.jar"/>
 	<classpathentry combineaccessrules="false" kind="src" path="/org.springframework.aop"/>
 	<classpathentry combineaccessrules="false" kind="src" path="/org.springframework.beans"/>
 	<classpathentry combineaccessrules="false" kind="src" path="/org.springframework.context"/>

org.springframework.web.servlet/ivy.xml

@@ -109,6 +109,10 @@
 		<dependency org="org.hibernate" name="com.springsource.org.hibernate.validator" rev="4.1.0.GA" conf="test->runtime"/>
 		<dependency org="org.slf4j" name="com.springsource.slf4j.jcl" rev="${slf4j.version}" conf="test->runtime"/>
 		<dependency org="org.joda" name="com.springsource.org.joda.time" rev="1.6.0" conf="test->runtime"/>
+		<dependency org="org.mortbay.jetty" name="com.springsource.org.mortbay.jetty.server" rev="6.1.9"
+                    conf="test->compile"/>
+		<dependency org="org.apache.httpcomponents" name="com.springsource.org.apache.httpcomponents.httpclient" rev="4.1.1"
+                    conf="test->compile"/>
 	</dependencies>
 
 </ivy-module>

org.springframework.web.servlet/pom.xml

@@ -310,5 +310,17 @@
 			<version>1.6</version>
 			<scope>test</scope>
 		</dependency>
+		<dependency>
+			<groupId>org.mortbay.jetty</groupId>
+			<artifactId>jetty</artifactId>
+			<version>6.1.9</version>
+			<scope>test</scope>
+			<exclusions>
+				<exclusion>
+					<artifactId>servlet-api-2.5</artifactId>
+					<groupId>org.mortbay.jetty</groupId>
+				</exclusion>
+			</exclusions>
+		</dependency>
 	</dependencies>
 </project>

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/RequestPartMethodArgumentResolver.java

@@ -31,6 +31,7 @@ import org.springframework.validation.Errors;
 import org.springframework.web.bind.ServletRequestBindingException;
 import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RequestPart;
 import org.springframework.web.bind.support.WebDataBinderFactory;
 import org.springframework.web.context.request.NativeWebRequest;
@@ -79,23 +80,28 @@ public class RequestPartMethodArgumentResolver extends AbstractMessageConverterM
 	/**
 	 * Supports the following:
 	 * <ul>
-	 * 	<li>@RequestPart method arguments.
-	 * 	<li>Arguments of type {@link MultipartFile} even if not annotated.
-	 * 	<li>Arguments of type {@code javax.servlet.http.Part} even if not annotated.
+	 * 	<li>@RequestPart-annotated method arguments.
+	 * 	<li>Arguments of type {@link MultipartFile} unless annotated with {@link RequestParam}.
+	 * 	<li>Arguments of type {@code javax.servlet.http.Part} unless annotated with {@link RequestParam}.
 	 * </ul>
 	 */
 	public boolean supportsParameter(MethodParameter parameter) {
 		if (parameter.hasParameterAnnotation(RequestPart.class)) {
 			return true;
 		}
-		else if (MultipartFile.class.equals(parameter.getParameterType())) {
-			return true;
-		}
-		else if ("javax.servlet.http.Part".equals(parameter.getParameterType().getName())) {
-			return true;
-		}
 		else {
-			return false;
+			if (parameter.hasParameterAnnotation(RequestParam.class)){
+				return false;
+			}
+			else if (MultipartFile.class.equals(parameter.getParameterType())) {
+				return true;
+			}
+			else if ("javax.servlet.http.Part".equals(parameter.getParameterType().getName())) {
+				return true;
+			}
+			else {
+				return false;
+			}
 		}
 	}
 

org.springframework.web.servlet/src/test/java/org/springframework/http/client/FreePortScanner.java

+/*
+ * Copyright 2002-2010 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.http.client;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+import java.util.Random;
+
+import org.springframework.util.Assert;
+
+/**
+ * Utility class that finds free BSD ports for use in testing scenario's.
+ *
+ * @author Ben Hale
+ * @author Arjen Poutsma
+ */
+public abstract class FreePortScanner {
+
+    private static final int MIN_SAFE_PORT = 1024;
+
+    private static final int MAX_PORT = 65535;
+
+    private static final Random random = new Random();
+
+    /**
+     * Returns the number of a free port in the default range.
+     */
+    public static int getFreePort() {
+        return getFreePort(MIN_SAFE_PORT, MAX_PORT);
+    }
+
+    /**
+     * Returns the number of a free port in the given range.
+     */
+    public static int getFreePort(int minPort, int maxPort) {
+        Assert.isTrue(minPort > 0, "'minPort' must be larger than 0");
+        Assert.isTrue(maxPort > minPort, "'maxPort' must be larger than minPort");
+        int portRange = maxPort - minPort;
+        int candidatePort;
+        int searchCounter = 0;
+        do {
+            if (++searchCounter > portRange) {
+                throw new IllegalStateException(
+                        String.format("There were no ports available in the range %d to %d", minPort, maxPort));
+            }
+            candidatePort = getRandomPort(minPort, portRange);
+        }
+        while (!isPortAvailable(candidatePort));
+
+        return candidatePort;
+    }
+
+    private static int getRandomPort(int minPort, int portRange) {
+        return minPort + random.nextInt(portRange);
+    }
+
+    private static boolean isPortAvailable(int port) {
+        ServerSocket serverSocket;
+        try {
+            serverSocket = new ServerSocket();
+        }
+        catch (IOException ex) {
+            throw new IllegalStateException("Unable to create ServerSocket.", ex);
+        }
+
+        try {
+            InetSocketAddress sa = new InetSocketAddress(port);
+            serverSocket.bind(sa);
+            return true;
+        }
+        catch (IOException ex) {
+            return false;
+        }
+        finally {
+            try {
+                serverSocket.close();
+            }
+            catch (IOException ex) {
+                // ignore
+            }
+        }
+    }
+
+}

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/support/RequestPartIntegrationTests.java

+/*
+ * Copyright 2002-2011 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.web.servlet.mvc.method.annotation.support;
+
+import static org.junit.Assert.assertEquals;
+
+import java.net.URI;
+import java.util.Arrays;
+
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.mortbay.jetty.Server;
+import org.mortbay.jetty.servlet.Context;
+import org.mortbay.jetty.servlet.ServletHolder;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.http.HttpEntity;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.http.client.FreePortScanner;
+import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
+import org.springframework.http.converter.HttpMessageConverter;
+import org.springframework.http.converter.ResourceHttpMessageConverter;
+import org.springframework.http.converter.json.MappingJacksonHttpMessageConverter;
+import org.springframework.http.converter.xml.XmlAwareFormHttpMessageConverter;
+import org.springframework.stereotype.Controller;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RequestPart;
+import org.springframework.web.client.RestTemplate;
+import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
+import org.springframework.web.multipart.MultipartFile;
+import org.springframework.web.multipart.MultipartResolver;
+import org.springframework.web.multipart.commons.CommonsMultipartResolver;
+import org.springframework.web.servlet.DispatcherServlet;
+import org.springframework.web.servlet.config.annotation.EnableWebMvc;
+import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
+
+/**
+ * Test access to parts of a multipart request with {@link RequestPart}. 
+ * 
+ * @author Rossen Stoyanchev
+ */
+public class RequestPartIntegrationTests {
+
+	private RestTemplate restTemplate;
+
+	private static Server server;
+
+	private static String baseUrl;
+
+	@BeforeClass
+	public static void startServer() throws Exception {
+		
+		int port = FreePortScanner.getFreePort();
+		baseUrl = "http://localhost:" + port;
+
+		server = new Server(port);
+		Context context = new Context(server, "/");
+
+		ServletHolder commonsResolverServlet = new ServletHolder(DispatcherServlet.class);
+		commonsResolverServlet.setInitParameter("contextConfigLocation", CommonsMultipartResolverTestConfig.class.getName());
+		commonsResolverServlet.setInitParameter("contextClass", AnnotationConfigWebApplicationContext.class.getName());
+		context.addServlet(commonsResolverServlet, "/commons/*");
+
+		server.start();
+	}
+	
+	@Before
+	public void setUp() {
+		XmlAwareFormHttpMessageConverter converter = new XmlAwareFormHttpMessageConverter();
+		converter.setPartConverters(Arrays.<HttpMessageConverter<?>>asList(
+				new ResourceHttpMessageConverter(), new MappingJacksonHttpMessageConverter()));
+		
+ 		restTemplate = new RestTemplate(new HttpComponentsClientHttpRequestFactory());
+ 		restTemplate.setMessageConverters(Arrays.<HttpMessageConverter<?>>asList(converter));
+	}
+
+	@AfterClass
+	public static void stopServer() throws Exception {
+		if (server != null) {
+			server.stop();
+		}
+	}
+
+	@Test
+	public void commonsMultipartResolver() throws Exception {
+
+		MultiValueMap<String, Object> parts = new LinkedMultiValueMap<String, Object>();
+		HttpEntity<TestData> jsonEntity = new HttpEntity<TestData>(new TestData("Jason"));
+		parts.add("json-data", jsonEntity);
+		parts.add("file-data", new ClassPathResource("logo.jpg", this.getClass()));
+
+		URI location = restTemplate.postForLocation(baseUrl + "/commons/test", parts);
+		assertEquals("http://localhost:8080/test/Jason/logo.jpg", location.toString());
+	}	
+
+	@Configuration
+	@EnableWebMvc
+	static class RequestPartTestConfig extends WebMvcConfigurerAdapter {
+
+		@Bean
+		public RequestPartTestController controller() {
+			return new RequestPartTestController();
+		}
+	}
+	
+	static class CommonsMultipartResolverTestConfig extends RequestPartTestConfig {
+
+		@Bean
+		public MultipartResolver multipartResolver() {
+			return new CommonsMultipartResolver();
+		}
+		
+	}
+
+	@SuppressWarnings("unused")
+	@Controller
+	private static class RequestPartTestController {
+
+		@RequestMapping(value = "/test", method = RequestMethod.POST, consumes = { "multipart/mixed", "multipart/form-data" })
+		public ResponseEntity<Object> create(@RequestPart("json-data") TestData testData, @RequestPart("file-data") MultipartFile file) {
+	        String url = "http://localhost:8080/test/" + testData.getName() + "/" + file.getOriginalFilename();
+	        HttpHeaders headers = new HttpHeaders();
+			headers.setLocation(URI.create(url));
+	        return new ResponseEntity<Object>(headers, HttpStatus.CREATED);
+		}
+	}
+	
+	@SuppressWarnings("unused")
+	private static class TestData {
+		
+		private String name;
+
+		public TestData() {
+			super();
+		}
+
+		public TestData(String name) {
+			this.name = name;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+	}
+	
+}

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/support/RequestPartMethodArgumentResolverTests.java

@@ -54,6 +54,7 @@ import org.springframework.mock.web.MockPart;
 import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
 import org.springframework.web.bind.ServletRequestBindingException;
 import org.springframework.web.bind.WebDataBinder;
+import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RequestPart;
 import org.springframework.web.bind.support.WebDataBinderFactory;
 import org.springframework.web.context.request.NativeWebRequest;
@@ -84,6 +85,7 @@ public class RequestPartMethodArgumentResolverTests {
 	private MethodParameter paramInt;
 	private MethodParameter paramMultipartFileNotAnnot;
 	private MethodParameter paramServlet30Part;
+	private MethodParameter paramRequestParamAnnot;
 
 	private NativeWebRequest webRequest;
 
@@ -96,7 +98,7 @@ public class RequestPartMethodArgumentResolverTests {
 	public void setUp() throws Exception {
 		
 		Method method = getClass().getMethod("handle", SimpleBean.class, SimpleBean.class, SimpleBean.class,
-				MultipartFile.class, List.class, Integer.TYPE, MultipartFile.class, Part.class);
+				MultipartFile.class, List.class, Integer.TYPE, MultipartFile.class, Part.class, MultipartFile.class);
 		
 		paramRequestPart = new MethodParameter(method, 0);
 		paramRequestPart.initParameterNameDiscovery(new LocalVariableTableParameterNameDiscoverer());
@@ -109,6 +111,7 @@ public class RequestPartMethodArgumentResolverTests {
 		paramMultipartFileNotAnnot.initParameterNameDiscovery(new LocalVariableTableParameterNameDiscoverer());
 		paramServlet30Part = new MethodParameter(method, 7);
 		paramServlet30Part.initParameterNameDiscovery(new LocalVariableTableParameterNameDiscoverer());
+		paramRequestParamAnnot = new MethodParameter(method, 8);
 
 		messageConverter = createMock(HttpMessageConverter.class);
 		expect(messageConverter.getSupportedMediaTypes()).andReturn(Collections.singletonList(MediaType.TEXT_PLAIN));
@@ -132,6 +135,7 @@ public class RequestPartMethodArgumentResolverTests {
 		assertTrue("MultipartFile parameter not supported", resolver.supportsParameter(paramMultipartFileNotAnnot));
 		assertTrue("Part parameter not supported", resolver.supportsParameter(paramServlet30Part));
 		assertFalse("non-RequestPart parameter supported", resolver.supportsParameter(paramInt));
+		assertFalse("@RequestParam args not supported", resolver.supportsParameter(paramRequestParamAnnot));
 	}	
 
 	@Test 
@@ -266,7 +270,8 @@ public class RequestPartMethodArgumentResolverTests {
 					   @RequestPart("requestPart") List<MultipartFile> multipartFileList,
 					   int i,
 					   MultipartFile multipartFileNotAnnot,
-					   Part servlet30Part) {
+					   Part servlet30Part,
+					   @RequestParam MultipartFile requestParamAnnot) {
 	}
 
 }

org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/RequestParamMethodArgumentResolver.java

@@ -33,6 +33,7 @@ import org.springframework.util.StringUtils;
 import org.springframework.web.bind.MissingServletRequestParameterException;
 import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RequestPart;
 import org.springframework.web.bind.annotation.ValueConstants;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.multipart.MultipartFile;
@@ -85,35 +86,38 @@ public class RequestParamMethodArgumentResolver extends AbstractNamedValueMethod
 	/**
 	 * Supports the following:
 	 * <ul>
-	 * 	<li>@RequestParam method arguments. This excludes the case where a parameter is of type 
-	 * 		{@link Map} and the annotation does not specify a request parameter name. See 
-	 * 		{@link RequestParamMapMethodArgumentResolver} instead for such parameters.
-	 * 	<li>Arguments of type {@link MultipartFile} even if not annotated.
-	 * 	<li>Arguments of type {@code javax.servlet.http.Part} even if not annotated.
+	 * 	<li>@RequestParam-annotated method arguments. 
+	 * 		This excludes {@link Map} parameters where the annotation does not specify a name value.
+	 * 		See {@link RequestParamMapMethodArgumentResolver} instead for such parameters.
+	 * 	<li>Arguments of type {@link MultipartFile} unless annotated with {@link RequestPart}.
+	 * 	<li>Arguments of type {@code javax.servlet.http.Part} unless annotated with {@link RequestPart}.
+	 * 	<li>In default resolution mode, simple type arguments even if not with @RequestParam.
 	 * </ul>
-	 * 
-	 * <p>In default resolution mode, simple type arguments not annotated with @RequestParam are also supported.
 	 */
 	public boolean supportsParameter(MethodParameter parameter) {
 		Class<?> paramType = parameter.getParameterType();
-		RequestParam requestParamAnnot = parameter.getParameterAnnotation(RequestParam.class);
-		if (requestParamAnnot != null) {
+		if (parameter.hasParameterAnnotation(RequestParam.class)) {
 			if (Map.class.isAssignableFrom(paramType)) {
-				return StringUtils.hasText(requestParamAnnot.value());
+				String paramName = parameter.getParameterAnnotation(RequestParam.class).value();
+				return StringUtils.hasText(paramName);
+			}
+			else {
+				return true;
 			}
-			return true;
-		}
-		else if (MultipartFile.class.equals(paramType)) {
-			return true;
-		}
-		else if ("javax.servlet.http.Part".equals(parameter.getParameterType().getName())) {
-			return true;
-		}
-		else if (this.useDefaultResolution) {
-			return BeanUtils.isSimpleProperty(paramType);
 		}
 		else {
-			return false;
+			if (parameter.hasParameterAnnotation(RequestPart.class)) {
+				return false;
+			}
+			else if (MultipartFile.class.equals(paramType) || "javax.servlet.http.Part".equals(paramType.getName())) {
+				return true;
+			}
+			else if (this.useDefaultResolution) {
+				return BeanUtils.isSimpleProperty(paramType);
+			}
+			else {
+				return false;
+			}
 		}
 	}
 

org.springframework.web/src/test/java/org/springframework/web/method/annotation/support/RequestParamMethodArgumentResolverTests.java

@@ -41,6 +41,7 @@ import org.springframework.mock.web.MockMultipartHttpServletRequest;
 import org.springframework.mock.web.MockPart;
 import org.springframework.web.bind.MissingServletRequestParameterException;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RequestPart;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.context.request.ServletWebRequest;
 import org.springframework.web.multipart.MultipartFile;
@@ -64,6 +65,7 @@ public class RequestParamMethodArgumentResolverTests {
 	private MethodParameter paramMultipartFileNotAnnot;
 	private MethodParameter paramMultipartFileList;
 	private MethodParameter paramServlet30Part;
+	private MethodParameter paramRequestPartAnnot;
 
 	private NativeWebRequest webRequest;
 
@@ -76,7 +78,7 @@ public class RequestParamMethodArgumentResolverTests {
 		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);
+				Map.class, String.class, MultipartFile.class, List.class, Part.class, MultipartFile.class);
 		
 		paramNamedDefaultValueString = new MethodParameter(method, 0);
 		paramNamedStringArray = new MethodParameter(method, 1);
@@ -91,6 +93,7 @@ public class RequestParamMethodArgumentResolverTests {
 		paramMultipartFileList.initParameterNameDiscovery(paramNameDiscoverer);
 		paramServlet30Part = new MethodParameter(method, 8);
 		paramServlet30Part.initParameterNameDiscovery(paramNameDiscoverer);
+		paramRequestPartAnnot = new MethodParameter(method, 9);
 
 		request = new MockHttpServletRequest();
 		webRequest = new ServletWebRequest(request, new MockHttpServletResponse());
@@ -110,6 +113,7 @@ public class RequestParamMethodArgumentResolverTests {
 		
 		resolver = new RequestParamMethodArgumentResolver(null, false);
 		assertFalse(resolver.supportsParameter(paramStringNotAnnot));
+		assertFalse(resolver.supportsParameter(paramRequestPartAnnot));
 	}
 
 	@Test
@@ -225,7 +229,8 @@ public class RequestParamMethodArgumentResolverTests {
 					   String stringNotAnnot,
 					   MultipartFile multipartFileNotAnnot,
 					   List<MultipartFile> multipartFileList,
-					   Part servlet30Part) {
+					   Part servlet30Part,
+					   @RequestPart MultipartFile requestPartAnnot) {
 	}
 
 }