From 018231d20a291587dc17c2be3c9f0d13c450bcb0 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 17 Jun 2013 10:29:44 -0700 Subject: [PATCH] Polish --- .../web/MultipartAutoConfiguration.java | 16 ++--- .../EmbeddedWebApplicationContext.java | 30 ++++---- .../embedded/ServletRegistrationBean.java | 31 +++++--- .../web/MultipartAutoConfigurationTests.java | 70 ++++++++++--------- 4 files changed, 78 insertions(+), 69 deletions(-) diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/autoconfigure/web/MultipartAutoConfiguration.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/autoconfigure/web/MultipartAutoConfiguration.java index b87e997e43..b8b14ee2a2 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/autoconfigure/web/MultipartAutoConfiguration.java +++ b/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(); } } diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/EmbeddedWebApplicationContext.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/EmbeddedWebApplicationContext.java index 33131fe6d7..e95f7f5cc7 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/EmbeddedWebApplicationContext.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/EmbeddedWebApplicationContext.java @@ -23,7 +23,6 @@ import java.util.Comparator; 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 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext } initializers.add(initializer); } - - Map multipartConfigBeans; - MultipartConfigElement multipartConfigElement = null; - multipartConfigBeans = getBeanFactory().getBeansOfType(MultipartConfigElement.class); - for (MultipartConfigElement bean : multipartConfigBeans.values()) { - multipartConfigElement = bean; - } List> servletBeans = getOrderedBeansOfType(Servlet.class); for (Entry servletBean : servletBeans) { @@ -229,15 +221,11 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext 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 filterBean : getOrderedBeansOfType(Filter.class)) { @@ -254,6 +242,14 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext return initializers; } + private MultipartConfigElement getMultipartConfig() { + List> 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 diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/ServletRegistrationBean.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/ServletRegistrationBean.java index b796bb959c..a5022ec1ce 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/embedded/ServletRegistrationBean.java +++ b/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 filters = new LinkedHashSet(); - - private MultipartConfigElement multipartConfigElement = null; + + private MultipartConfigElement multipartConfig; /** * Create a new {@link ServletRegistrationBean} instance. @@ -67,7 +67,7 @@ public class ServletRegistrationBean extends RegistrationBean { /** * 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 */ @@ -75,11 +75,6 @@ public class ServletRegistrationBean extends RegistrationBean { 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 class ServletRegistrationBean extends RegistrationBean { 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 @@ public class ServletRegistrationBean extends RegistrationBean { } registration.addMapping(urlMapping); registration.setLoadOnStartup(this.loadOnStartup); - if (multipartConfigElement != null) { - registration.setMultipartConfig(multipartConfigElement); + if (this.multipartConfig != null) { + registration.setMultipartConfig(this.multipartConfig); } } } diff --git a/spring-bootstrap/src/test/java/org/springframework/bootstrap/autoconfigure/web/MultipartAutoConfigurationTests.java b/spring-bootstrap/src/test/java/org/springframework/bootstrap/autoconfigure/web/MultipartAutoConfigurationTests.java index 47b9d4d182..a9e29fc7c6 100644 --- a/spring-bootstrap/src/test/java/org/springframework/bootstrap/autoconfigure/web/MultipartAutoConfigurationTests.java +++ b/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.assertNull; 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,19 +122,6 @@ public class MultipartAutoConfigurationTests { verifyServletWorks(); } - @Configuration - public static class ContainerWithNoMultipartTomcat { - @Bean - TomcatEmbeddedServletContainerFactory containerFactory() { - return new TomcatEmbeddedServletContainerFactory(); - } - - @Bean - WebController controller() { - return new WebController(); - } - } - @Test public void containerWithAutomatedMultipartJettyConfiguration() { this.context = new AnnotationConfigEmbeddedWebApplicationContext( @@ -146,6 +134,38 @@ public class MultipartAutoConfigurationTests { 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(); + } + + @Bean + WebController controller() { + return new WebController(); + } + } + @Configuration public static class ContainerWithEverythingJetty { @Bean @@ -164,18 +184,6 @@ public class MultipartAutoConfigurationTests { } } - @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 @@ public class MultipartAutoConfigurationTests { } } - private void verifyServletWorks() { - RestTemplate restTemplate = new RestTemplate(); - assertEquals(restTemplate.getForObject("http://localhost:8080/", String.class), - "Hello"); - } - }