Split up DispatcherServletConfiguration condition

Prior to this commit, defining a custom `DispatcherServlet` and/or a `ServletRegistrationBean` with the default name would turn off completely the `DispatcherServletAutoConfiguration`. This commit splits this auto-configuration in two parts: - First, a `DispatcherServlet` is automatically registered if no instance is already defined with the default name. - Then, a `ServletRegistrationBean` is registered is registered if a `DispatcherServlet` instance exists with the default name *and* no `ServletRegistrationBean` exists with the default name This allows developers to register manually a `ServletRegistrationBean` or a `DispatcherServlet` without having to redefine the whole auto-configuration. Fixes gh-4893 Closes gh-6108
9 years ago · 091664078f
parent d87287fe72
commit 091664078f
3 changed files with 108 additions and 54 deletions
--- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/DispatcherServletAutoConfiguration.java
+++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/DispatcherServletAutoConfiguration.java
@ -40,6 +40,7 @@ import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ConditionContext;
 import org.springframework.context.annotation.Conditional;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
 import org.springframework.core.type.AnnotatedTypeMetadata;
@ -55,6 +56,7 @@ import org.springframework.web.servlet.DispatcherServlet;
 * @author Phillip Webb
 * @author Dave Syer
 * @author Stephane Nicoll
+ * @author Brian Clozel
 */
@AutoConfigureOrder(Ordered.HIGHEST_PRECEDENCE)
@Configuration
@ -79,18 +81,10 @@ public class DispatcherServletAutoConfiguration {
 	@EnableConfigurationProperties(WebMvcProperties.class)
 	protected static class DispatcherServletConfiguration {

-		private final ServerProperties server;
-
 		private final WebMvcProperties webMvcProperties;

-		private final MultipartConfigElement multipartConfig;
-
-		public DispatcherServletConfiguration(ServerProperties server,
-				WebMvcProperties webMvcProperties,
-				ObjectProvider<MultipartConfigElement> multipartConfigProvider) {
-			this.server = server;
+		public DispatcherServletConfiguration(WebMvcProperties webMvcProperties) {
 			this.webMvcProperties = webMvcProperties;
-			this.multipartConfig = multipartConfigProvider.getIfAvailable();
 		}

 		@Bean(name = DEFAULT_DISPATCHER_SERVLET_BEAN_NAME)
@ -105,10 +99,43 @@ public class DispatcherServletAutoConfiguration {
 			return dispatcherServlet;
 		}

+		@Bean
+		@ConditionalOnBean(MultipartResolver.class)
+		@ConditionalOnMissingBean(name = DispatcherServlet.MULTIPART_RESOLVER_BEAN_NAME)
+		public MultipartResolver multipartResolver(MultipartResolver resolver) {
+			// Detect if the user has created a MultipartResolver but named it incorrectly
+			return resolver;
+		}
+
+	}
+
+	@Configuration
+	@Conditional(DispatcherServletRegistrationCondition.class)
+	@ConditionalOnClass(ServletRegistration.class)
+	@EnableConfigurationProperties(WebMvcProperties.class)
+	@Import(DispatcherServletConfiguration.class)
+	protected static class DispatcherServletRegistrationConfiguration {
+
+		private final ServerProperties serverProperties;
+
+		private final WebMvcProperties webMvcProperties;
+
+		private final MultipartConfigElement multipartConfig;
+
+		public DispatcherServletRegistrationConfiguration(
+				ServerProperties serverProperties, WebMvcProperties webMvcProperties,
+				ObjectProvider<MultipartConfigElement> multipartConfigProvider) {
+			this.serverProperties = serverProperties;
+			this.webMvcProperties = webMvcProperties;
+			this.multipartConfig = multipartConfigProvider.getIfAvailable();
+		}
+
 		@Bean(name = DEFAULT_DISPATCHER_SERVLET_REGISTRATION_BEAN_NAME)
-		public ServletRegistrationBean dispatcherServletRegistration() {
+		@ConditionalOnBean(value = DispatcherServlet.class, name = DEFAULT_DISPATCHER_SERVLET_BEAN_NAME)
+		public ServletRegistrationBean dispatcherServletRegistration(
+				DispatcherServlet dispatcherServlet) {
 			ServletRegistrationBean registration = new ServletRegistrationBean(
-					dispatcherServlet(), this.server.getServletMapping());
+					dispatcherServlet, this.serverProperties.getServletMapping());
 			registration.setName(DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
 			registration.setLoadOnStartup(
 					this.webMvcProperties.getServlet().getLoadOnStartup());
@ -118,57 +145,65 @@ public class DispatcherServletAutoConfiguration {
 			return registration;
 		}

-		@Bean
-		@ConditionalOnBean(MultipartResolver.class)
-		@ConditionalOnMissingBean(name = DispatcherServlet.MULTIPART_RESOLVER_BEAN_NAME)
-		public MultipartResolver multipartResolver(MultipartResolver resolver) {
-			// Detect if the user has created a MultipartResolver but named it incorrectly
-			return resolver;
+	}
+
+	@Order(Ordered.LOWEST_PRECEDENCE - 10)
+	private static class DefaultDispatcherServletCondition extends SpringBootCondition {
+
+		@Override
+		public ConditionOutcome getMatchOutcome(ConditionContext context,
+				AnnotatedTypeMetadata metadata) {
+			ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
+			List<String> dispatchServletBeans = Arrays.asList(beanFactory
+					.getBeanNamesForType(DispatcherServlet.class, false, false));
+			if (dispatchServletBeans.contains(DEFAULT_DISPATCHER_SERVLET_BEAN_NAME)) {
+				return ConditionOutcome.noMatch("found DispatcherServlet named "
+						+ DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
+			}
+			if (beanFactory.containsBean(DEFAULT_DISPATCHER_SERVLET_BEAN_NAME)) {
+				return ConditionOutcome.noMatch("found non-DispatcherServlet named "
+						+ DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
+			}
+			if (dispatchServletBeans.isEmpty()) {
+				return ConditionOutcome.match("no DispatcherServlet found");
+			}
+			return ConditionOutcome
+					.match("one or more DispatcherServlets found and none is named "
+							+ DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
 		}

 	}

 	@Order(Ordered.LOWEST_PRECEDENCE - 10)
-	private static class DefaultDispatcherServletCondition extends SpringBootCondition {
+	private static class DispatcherServletRegistrationCondition
+			extends SpringBootCondition {

 		@Override
 		public ConditionOutcome getMatchOutcome(ConditionContext context,
 				AnnotatedTypeMetadata metadata) {
 			ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
-			ConditionOutcome outcome = checkServlets(beanFactory);
+			ConditionOutcome outcome = checkDefaultDispatcherName(beanFactory);
 			if (!outcome.isMatch()) {
 				return outcome;
 			}
-			return checkServletRegistrations(beanFactory);
+			return checkServletRegistration(beanFactory);
 		}

-		private ConditionOutcome checkServlets(
+		private ConditionOutcome checkDefaultDispatcherName(
 				ConfigurableListableBeanFactory beanFactory) {
 			List<String> servlets = Arrays.asList(beanFactory
 					.getBeanNamesForType(DispatcherServlet.class, false, false));
 			boolean containsDispatcherBean = beanFactory
 					.containsBean(DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
-			if (servlets.isEmpty()) {
-				if (containsDispatcherBean) {
-					return ConditionOutcome.noMatch("found no DispatcherServlet "
-							+ "but a non-DispatcherServlet named "
-							+ DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
-				}
-				return ConditionOutcome.match("no DispatcherServlet found");
-			}
-			if (servlets.contains(DEFAULT_DISPATCHER_SERVLET_BEAN_NAME)) {
-				return ConditionOutcome.noMatch("found DispatcherServlet named "
-						+ DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
-			}
-			if (containsDispatcherBean) {
+			if (containsDispatcherBean
+					&& !servlets.contains(DEFAULT_DISPATCHER_SERVLET_BEAN_NAME)) {
 				return ConditionOutcome.noMatch("found non-DispatcherServlet named "
 						+ DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
 			}
-			return ConditionOutcome.match("one or more DispatcherServlets "
-					+ "found and none is named " + DEFAULT_DISPATCHER_SERVLET_BEAN_NAME);
+			return ConditionOutcome.match();
 		}

-		private ConditionOutcome checkServletRegistrations(
+		private ConditionOutcome checkServletRegistration(
 				ConfigurableListableBeanFactory beanFactory) {
 			List<String> registrations = Arrays.asList(beanFactory
 					.getBeanNamesForType(ServletRegistrationBean.class, false, false));
@ -194,7 +229,6 @@ public class DispatcherServletAutoConfiguration {
 			return ConditionOutcome
 					.match("one or more ServletRegistrationBeans is found and none is named "
 							+ DEFAULT_DISPATCHER_SERVLET_REGISTRATION_BEAN_NAME);
-
 		}
 	}

--- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/DispatcherServletAutoConfigurationTests.java
+++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/DispatcherServletAutoConfigurationTests.java
@ -23,7 +23,6 @@ import org.junit.After;
 import org.junit.Test;

 import org.springframework.beans.DirectFieldAccessor;
-import org.springframework.beans.factory.UnsatisfiedDependencyException;
 import org.springframework.boot.test.util.EnvironmentTestUtils;
 import org.springframework.boot.web.servlet.MultipartConfigFactory;
 import org.springframework.boot.web.servlet.ServletRegistrationBean;
@ -44,6 +43,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 *
 * @author Dave Syer
 * @author Andy Wilkinson
+ * @author Brian Clozel
 */
 public class DispatcherServletAutoConfigurationTests {

@ -70,24 +70,38 @@ public class DispatcherServletAutoConfigurationTests {
 	}

 	@Test
-	public void registrationOverride() throws Exception {
+	public void registrationNonServletBean() throws Exception {
 		this.context = new AnnotationConfigWebApplicationContext();
-		this.context.register(CustomDispatcherRegistration.class,
+		this.context.register(NonServletConfiguration.class,
+				ServerPropertiesAutoConfiguration.class,
+				DispatcherServletAutoConfiguration.class);
+		this.context.setServletContext(new MockServletContext());
+		this.context.refresh();
+		assertThat(this.context.getBeanNamesForType(ServletRegistrationBean.class).length)
+				.isEqualTo(0);
+		assertThat(this.context.getBeanNamesForType(DispatcherServlet.class).length)
+				.isEqualTo(0);
+	}
+
+	// If a DispatcherServlet instance is registered with a name different
+	// from the default one, we're registering one anyway
+	@Test
+	public void registrationOverrideWithDispatcherServletWrongName() throws Exception {
+		this.context = new AnnotationConfigWebApplicationContext();
+		this.context.register(CustomDispatcherServletWrongName.class,
 				ServerPropertiesAutoConfiguration.class,
 				DispatcherServletAutoConfiguration.class);
 		this.context.setServletContext(new MockServletContext());
 		this.context.refresh();
 		ServletRegistrationBean registration = this.context
 				.getBean(ServletRegistrationBean.class);
-		assertThat(registration.getUrlMappings().toString()).isEqualTo("[/foo]");
-		assertThat(registration.getServletName()).isEqualTo("customDispatcher");
+		assertThat(registration.getUrlMappings().toString()).isEqualTo("[/]");
+		assertThat(registration.getServletName()).isEqualTo("dispatcherServlet");
 		assertThat(this.context.getBeanNamesForType(DispatcherServlet.class).length)
-				.isEqualTo(0);
+				.isEqualTo(2);
 	}

-	// If you override either the dispatcherServlet or its registration you have to
-	// provide both...
-	@Test(expected = UnsatisfiedDependencyException.class)
+	@Test
 	public void registrationOverrideWithAutowiredServlet() throws Exception {
 		this.context = new AnnotationConfigWebApplicationContext();
 		this.context.register(CustomAutowiredRegistration.class,
@ -199,14 +213,11 @@ public class DispatcherServletAutoConfigurationTests {
 	}

 	@Configuration
-	protected static class CustomDispatcherRegistration {
+	protected static class CustomDispatcherServletWrongName {

 		@Bean
-		public ServletRegistrationBean dispatcherServletRegistration() {
-			ServletRegistrationBean registration = new ServletRegistrationBean(
-					new DispatcherServlet(), "/foo");
-			registration.setName("customDispatcher");
-			return registration;
+		public DispatcherServlet customDispatcherServlet() {
+			return new DispatcherServlet();
 		}

 	}
@ -225,6 +236,15 @@ public class DispatcherServletAutoConfigurationTests {

 	}

+	@Configuration
+	protected static class NonServletConfiguration {
+
+		@Bean
+		public String dispatcherServlet() {
+			return "spring";
+		}
+	}
+
 	@Configuration
 	protected static class MultipartResolverConfiguration {

--- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/EmbeddedServletContainerAutoConfigurationTests.java
+++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/EmbeddedServletContainerAutoConfigurationTests.java
@ -217,7 +217,7 @@ public class EmbeddedServletContainerAutoConfigurationTests {
 			return new DispatcherServlet();
 		}

-		@Bean
+		@Bean(name = DispatcherServletAutoConfiguration.DEFAULT_DISPATCHER_SERVLET_REGISTRATION_BEAN_NAME)
 		public ServletRegistrationBean dispatcherRegistration() {
 			return new ServletRegistrationBean(dispatcherServlet(), "/app/*");
 		}