Polish

Author: philwebb

spring-bootstrap/src/main/java/org/springframework/bootstrap/autoconfigure/web/MultipartAutoConfiguration.java

@@ -13,31 +13,31 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.bootstrap.autoconfigure.web;
 
 import javax.servlet.MultipartConfigElement;
 
 import org.springframework.bootstrap.context.annotation.ConditionalOnBean;
+import org.springframework.bootstrap.context.annotation.EnableAutoConfiguration;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.web.multipart.support.StandardServletMultipartResolver;
 
 /**
- * Autoconfiguration for multipart uploads. It detects the existence of a
- * {@link MultipartConfigElement} in the app context and then adds critical beans
- * while also autowiring it into the Jetty/Tomcat embedded containers.
+ * {@link EnableAutoConfiguration Auto-configuration} for multi-part uploads. It detects
+ * the existence of a {@link MultipartConfigElement} in the app context and then adds
+ * critical beans while also autowiring it into the Jetty/Tomcat embedded containers.
  * 
  * @author Greg Turnquist
- *
  */
 @Configuration
 public class MultipartAutoConfiguration {
-	
-	@ConditionalOnBean(MultipartConfigElement.class)
+
 	@Bean
+	@ConditionalOnBean(MultipartConfigElement.class)
 	public StandardServletMultipartResolver multipartResolver() {
-		System.out.println("Loading up a MultipartResolver!!!");
-	    return new StandardServletMultipartResolver();
+		return new StandardServletMultipartResolver();
 	}
 
 }

spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/EmbeddedWebApplicationContext.java

@@ -23,7 +23,6 @@
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
 
@@ -210,13 +209,6 @@ protected Collection<ServletContextInitializer> getServletContextInitializerBean
 			}
 			initializers.add(initializer);
 		}
-		
-		Map<String, MultipartConfigElement> multipartConfigBeans;
-		MultipartConfigElement multipartConfigElement = null;
-		multipartConfigBeans = getBeanFactory().getBeansOfType(MultipartConfigElement.class);
-		for (MultipartConfigElement bean : multipartConfigBeans.values()) {
-			multipartConfigElement = bean;
-		}
 
 		List<Entry<String, Servlet>> servletBeans = getOrderedBeansOfType(Servlet.class);
 		for (Entry<String, Servlet> servletBean : servletBeans) {
@@ -229,15 +221,11 @@ protected Collection<ServletContextInitializer> getServletContextInitializerBean
 			if (name.equals(DISPATCHER_SERVLET_NAME)) {
 				url = "/"; // always map the main dispatcherServlet to "/"
 			}
-			if (multipartConfigElement != null) {
-				initializers.add(new ServletRegistrationBean(servlet, multipartConfigElement, url) {{
-					setName(name);
-				}});
-			} else {
-				initializers.add(new ServletRegistrationBean(servlet, url) {{
-					setName(name);
-				}});
-			}
+			ServletRegistrationBean registration = new ServletRegistrationBean(servlet,
+					url);
+			registration.setName(name);
+			registration.setMultipartConfig(getMultipartConfig());
+			initializers.add(registration);
 		}
 
 		for (Entry<String, Filter> filterBean : getOrderedBeansOfType(Filter.class)) {
@@ -254,6 +242,14 @@ protected Collection<ServletContextInitializer> getServletContextInitializerBean
 		return initializers;
 	}
 
+	private MultipartConfigElement getMultipartConfig() {
+		List<Entry<String, MultipartConfigElement>> beans = getOrderedBeansOfType(MultipartConfigElement.class);
+		if (beans.isEmpty()) {
+			return null;
+		}
+		return beans.get(0).getValue();
+	}
+
 	/**
 	 * Prepare the {@link WebApplicationContext} with the given fully loaded
 	 * {@link ServletContext}. This method is usually called from

spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/ServletRegistrationBean.java

@@ -56,8 +56,8 @@ public class ServletRegistrationBean extends RegistrationBean {
 	private int loadOnStartup = 1;
 
 	private Set<Filter> filters = new LinkedHashSet<Filter>();
-	
-	private MultipartConfigElement multipartConfigElement = null;
+
+	private MultipartConfigElement multipartConfig;
 
 	/**
 	 * Create a new {@link ServletRegistrationBean} instance.
@@ -67,19 +67,14 @@ public ServletRegistrationBean() {
 
 	/**
 	 * Create a new {@link ServletRegistrationBean} instance with the specified
-	 * {@link Servlet} and URL mapping.
+	 * {@link Servlet} and URL mappings.
 	 * @param servlet the servlet being mapped
 	 * @param urlMappings the URLs being mapped
 	 */
 	public ServletRegistrationBean(Servlet servlet, String... urlMappings) {
 		setServlet(servlet);
 		addUrlMappings(urlMappings);
 	}
-	
-	public ServletRegistrationBean(Servlet servlet, MultipartConfigElement multipartConfigElement, String... urlMappings) {
-		this(servlet, urlMappings);
-		this.multipartConfigElement = multipartConfigElement;
-	}
 
 	/**
 	 * Sets the servlet to be registered.
@@ -152,6 +147,22 @@ public void addFilters(Filter... filters) {
 		this.filters.addAll(Arrays.asList(filters));
 	}
 
+	/**
+	 * Set the the {@link MultipartConfigElement multi-part configuration}.
+	 * @param multipartConfig the muti-part configuration to set or {@code null}
+	 */
+	public void setMultipartConfig(MultipartConfigElement multipartConfig) {
+		this.multipartConfig = multipartConfig;
+	}
+
+	/**
+	 * Returns the {@link MultipartConfigElement multi-part configuration} to be applied
+	 * or {@code null}.
+	 */
+	public MultipartConfigElement getMultipartConfig() {
+		return this.multipartConfig;
+	}
+
 	/**
 	 * Returns the servlet name that will be registered.
 	 */
@@ -189,8 +200,8 @@ protected void configure(ServletRegistration.Dynamic registration) {
 		}
 		registration.addMapping(urlMapping);
 		registration.setLoadOnStartup(this.loadOnStartup);
-		if (multipartConfigElement != null) {
-			registration.setMultipartConfig(multipartConfigElement);
+		if (this.multipartConfig != null) {
+			registration.setMultipartConfig(this.multipartConfig);
 		}
 	}
 }

spring-bootstrap/src/test/java/org/springframework/bootstrap/autoconfigure/web/MultipartAutoConfigurationTests.java

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.bootstrap.autoconfigure.web;
 
 import javax.servlet.MultipartConfigElement;
@@ -40,8 +41,8 @@
 import static org.junit.Assert.assertSame;
 
 /**
- * A series of embedded unit tests, based on an empty configuration, no multipart
- * configuration, and a multipart configuration, with both Jetty and Tomcat.
+ * Tests for {@link MultipartAutoConfiguration}. Tests an empty configuration, no
+ * multipart configuration, and a multipart configuration (with both Jetty and Tomcat).
  * 
  * @author Greg Turnquist
  * @author Dave Syer
@@ -51,7 +52,7 @@ public class MultipartAutoConfigurationTests {
 	private AnnotationConfigEmbeddedWebApplicationContext context;
 
 	@Rule
-	public ExpectedException exception = ExpectedException.none();
+	public ExpectedException thrown = ExpectedException.none();
 
 	@After
 	public void close() {
@@ -121,8 +122,39 @@ public void containerWithNoMultipartTomcatConfiguration() {
 		verifyServletWorks();
 	}
 
+	@Test
+	public void containerWithAutomatedMultipartJettyConfiguration() {
+		this.context = new AnnotationConfigEmbeddedWebApplicationContext(
+				ContainerWithEverythingJetty.class,
+				EmbeddedServletContainerAutoConfiguration.class,
+				MultipartAutoConfiguration.class);
+		this.context.getBean(MultipartConfigElement.class);
+		assertSame(this.context.getBean(DispatcherServlet.class).getMultipartResolver(),
+				this.context.getBean(StandardServletMultipartResolver.class));
+		verifyServletWorks();
+	}
+
+	@Test
+	public void containerWithAutomatedMultipartTomcatConfiguration() {
+		this.context = new AnnotationConfigEmbeddedWebApplicationContext(
+				ContainerWithEverythingTomcat.class,
+				EmbeddedServletContainerAutoConfiguration.class,
+				MultipartAutoConfiguration.class);
+		this.context.getBean(MultipartConfigElement.class);
+		assertSame(this.context.getBean(DispatcherServlet.class).getMultipartResolver(),
+				this.context.getBean(StandardServletMultipartResolver.class));
+		verifyServletWorks();
+	}
+
+	private void verifyServletWorks() {
+		RestTemplate restTemplate = new RestTemplate();
+		assertEquals(restTemplate.getForObject("http://localhost:8080/", String.class),
+				"Hello");
+	}
+
 	@Configuration
 	public static class ContainerWithNoMultipartTomcat {
+
 		@Bean
 		TomcatEmbeddedServletContainerFactory containerFactory() {
 			return new TomcatEmbeddedServletContainerFactory();
@@ -134,18 +166,6 @@ WebController controller() {
 		}
 	}
 
-	@Test
-	public void containerWithAutomatedMultipartJettyConfiguration() {
-		this.context = new AnnotationConfigEmbeddedWebApplicationContext(
-				ContainerWithEverythingJetty.class,
-				EmbeddedServletContainerAutoConfiguration.class,
-				MultipartAutoConfiguration.class);
-		this.context.getBean(MultipartConfigElement.class);
-		assertSame(this.context.getBean(DispatcherServlet.class).getMultipartResolver(),
-				this.context.getBean(StandardServletMultipartResolver.class));
-		verifyServletWorks();
-	}
-
 	@Configuration
 	public static class ContainerWithEverythingJetty {
 		@Bean
@@ -164,18 +184,6 @@ WebController webController() {
 		}
 	}
 
-	@Test
-	public void containerWithAutomatedMultipartTomcatConfiguration() {
-		this.context = new AnnotationConfigEmbeddedWebApplicationContext(
-				ContainerWithEverythingTomcat.class,
-				EmbeddedServletContainerAutoConfiguration.class,
-				MultipartAutoConfiguration.class);
-		this.context.getBean(MultipartConfigElement.class);
-		assertSame(this.context.getBean(DispatcherServlet.class).getMultipartResolver(),
-				this.context.getBean(StandardServletMultipartResolver.class));
-		verifyServletWorks();
-	}
-
 	@Configuration
 	@EnableWebMvc
 	public static class ContainerWithEverythingTomcat {
@@ -204,10 +212,4 @@ String index() {
 		}
 	}
 
-	private void verifyServletWorks() {
-		RestTemplate restTemplate = new RestTemplate();
-		assertEquals(restTemplate.getForObject("http://localhost:8080/", String.class),
-				"Hello");
-	}
-
 }