Use ServletContainerInitializers to start servers

The Servlet spec prohibits ServletContextListeners from being registered
programatically other than from with a call to
`ServletContainerInitializer.onStartup`. This restriction is not
consistently enforced by the various embedded servlet containers that
Boot supports:

- Jetty 8 does not enforce the restriction.
- Jetty 9 enforces the restriction. We were working around it be calling
  setExendedListenerTypes(true) on the context.
- Tomcat somewhat enforces the restriction: it doesn't allow a
  ServletContextListener to be added once the first
  ServletContextListener has been called. We were using a
  LifecycleListener to drive the ServletContextListeners.
- Undertow enforces the restriction and we were not working around it.
  This resulted in gh-2192 being raised.

ServletListenerRegistrationBean is a ServletContextListener and is used
to register listeners, including ServletContextListeners, with the
servlet context. To adhere to the letter of the servlet spec this means
that ServletListenerRegistrationBeans need to be called from with
ServletContainerInitializer.onStartup. This commit updates all of the
embedded servlet container implementations to use a
ServletContainerInitializer to drive any ServletContextInitializers.

This makes the lifecycle more consistent across the supported containers
and allows ServletListenerRegistrationBeans to be able to register
ServletContextListeners on all supported embedded containers.

Fixes gh-2192

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/EmbeddedServletContainerServletContextListenerTests.java

+/*
+ * Copyright 2012-2014 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.boot.autoconfigure.web;
+
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
+
+import org.junit.Test;
+import org.springframework.boot.context.embedded.AnnotationConfigEmbeddedWebApplicationContext;
+import org.springframework.boot.context.embedded.EmbeddedServletContainer;
+import org.springframework.boot.context.embedded.EmbeddedServletContainerFactory;
+import org.springframework.boot.context.embedded.ServletListenerRegistrationBean;
+import org.springframework.boot.context.embedded.jetty.JettyEmbeddedServletContainerFactory;
+import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory;
+import org.springframework.boot.context.embedded.undertow.UndertowEmbeddedServletContainerFactory;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Tests for {@link EmbeddedServletContainer}s driving {@link ServletContextListener}s
+ * correctly
+ *
+ * @author Andy Wilkinson
+ */
+public class EmbeddedServletContainerServletContextListenerTests {
+
+	@Test
+	public void registeredServletContextListenerBeanIsCalledByJetty() {
+		registeredServletContextListenerBeanIsCalled(JettyConfiguration.class);
+	}
+
+	@Test
+	public void registeredServletContextListenerBeanIsCalledByTomcat() {
+		registeredServletContextListenerBeanIsCalled(TomcatConfiguration.class);
+	}
+
+	@Test
+	public void registeredServletContextListenerBeanIsCalledByUndertow() {
+		registeredServletContextListenerBeanIsCalled(UndertowConfiguration.class);
+	}
+
+	@Test
+	public void servletContextListenerBeanIsCalledByJetty() {
+		servletContextListenerBeanIsCalled(JettyConfiguration.class);
+	}
+
+	@Test
+	public void servletContextListenerBeanIsCalledByTomcat() {
+		servletContextListenerBeanIsCalled(TomcatConfiguration.class);
+	}
+
+	@Test
+	public void servletContextListenerBeanIsCalledByUndertow() {
+		servletContextListenerBeanIsCalled(UndertowConfiguration.class);
+	}
+
+	private void servletContextListenerBeanIsCalled(Class<?> configuration) {
+		AnnotationConfigEmbeddedWebApplicationContext context = new AnnotationConfigEmbeddedWebApplicationContext(
+				ServletContextListenerBeanConfiguration.class, configuration);
+		ServletContextListener servletContextListener = context.getBean(
+				"servletContextListener", ServletContextListener.class);
+		verify(servletContextListener).contextInitialized(any(ServletContextEvent.class));
+		context.close();
+	}
+
+	private void registeredServletContextListenerBeanIsCalled(Class<?> configuration) {
+		AnnotationConfigEmbeddedWebApplicationContext context = new AnnotationConfigEmbeddedWebApplicationContext(
+				ServletListenerRegistrationBeanConfiguration.class, configuration);
+		ServletContextListener servletContextListener = (ServletContextListener) context
+				.getBean("registration", ServletListenerRegistrationBean.class)
+				.getListener();
+		verify(servletContextListener).contextInitialized(any(ServletContextEvent.class));
+		context.close();
+	}
+
+	@Configuration
+	static class TomcatConfiguration {
+
+		@Bean
+		public EmbeddedServletContainerFactory servletContainerFactory() {
+			return new TomcatEmbeddedServletContainerFactory(0);
+		}
+
+	}
+
+	@Configuration
+	static class JettyConfiguration {
+
+		@Bean
+		public EmbeddedServletContainerFactory servletContainerFactory() {
+			return new JettyEmbeddedServletContainerFactory(0);
+		}
+
+	}
+
+	@Configuration
+	static class UndertowConfiguration {
+
+		@Bean
+		public EmbeddedServletContainerFactory servletContainerFactory() {
+			return new UndertowEmbeddedServletContainerFactory(0);
+		}
+
+	}
+
+	@Configuration
+	static class ServletContextListenerBeanConfiguration {
+
+		@Bean
+		public ServletContextListener servletContextListener() {
+			return mock(ServletContextListener.class);
+		}
+
+	}
+
+	@Configuration
+	static class ServletListenerRegistrationBeanConfiguration {
+
+		@Bean
+		public ServletListenerRegistrationBean<ServletContextListener> registration() {
+			return new ServletListenerRegistrationBean<ServletContextListener>(
+					mock(ServletContextListener.class));
+		}
+
+	}
+
+}

spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/JettyEmbeddedServletContainerFactory.java

@@ -70,6 +70,7 @@ import org.springframework.util.StringUtils;
  * @author Phillip Webb
  * @author Dave Syer
  * @author Andrey Hihlovskiy
+ * @author Andy Wilkinson
  * @see #setPort(int)
  * @see #setConfigurations(Collection)
  * @see JettyEmbeddedServletContainer
@@ -222,7 +223,6 @@ public class JettyEmbeddedServletContainerFactory extends
 			ServletContextInitializer... initializers) {
 		Assert.notNull(context, "Context must not be null");
 		context.setTempDirectory(getTempDirectory());
-		setExtendedListenerTypes(context);
 		if (this.resourceLoader != null) {
 			context.setClassLoader(this.resourceLoader.getClassLoader());
 		}
@@ -252,15 +252,6 @@ public class JettyEmbeddedServletContainerFactory extends
 		return (temp == null ? null : new File(temp));
 	}
 
-	private void setExtendedListenerTypes(WebAppContext context) {
-		try {
-			context.getServletContext().setExtendedListenerTypes(true);
-		}
-		catch (NoSuchMethodError ex) {
-			// Not available on Jetty 8
-		}
-	}
-
 	private void configureDocumentRoot(WebAppContext handler) {
 		File root = getValidDocumentRoot();
 		if (root != null) {
@@ -370,7 +361,7 @@ public class JettyEmbeddedServletContainerFactory extends
 	 */
 	protected Configuration getServletContextInitializerConfiguration(
 			WebAppContext webAppContext, ServletContextInitializer... initializers) {
-		return new ServletContextInitializerConfiguration(webAppContext, initializers);
+		return new ServletContextInitializerConfiguration(initializers);
 	}
 
 	/**

spring-boot/src/main/java/org/springframework/boot/context/embedded/jetty/ServletContextInitializerConfiguration.java

@@ -16,7 +16,7 @@
 
 package org.springframework.boot.context.embedded.jetty;
 
-import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
 
 import org.eclipse.jetty.server.handler.ContextHandler;
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
@@ -30,40 +30,83 @@ import org.springframework.util.Assert;
  * Jetty {@link Configuration} that calls {@link ServletContextInitializer}s.
  *
  * @author Phillip Webb
+ * @author Andy Wilkinson
  */
 public class ServletContextInitializerConfiguration extends AbstractConfiguration {
 
-	private final ContextHandler contextHandler;
-
 	private final ServletContextInitializer[] initializers;
 
 	/**
 	 * Create a new {@link ServletContextInitializerConfiguration}.
 	 * @param contextHandler the Jetty ContextHandler
 	 * @param initializers the initializers that should be invoked
+	 * @deprecated since 1.2.1 in favor of
+	 * {@link #ServletContextInitializerConfiguration(ServletContextInitializer...)}
 	 */
+	@Deprecated
 	public ServletContextInitializerConfiguration(ContextHandler contextHandler,
 			ServletContextInitializer... initializers) {
-		Assert.notNull(contextHandler, "Jetty ContextHandler must not be null");
+		this(initializers);
+	}
+
+	/**
+	 * Create a new {@link ServletContextInitializerConfiguration}.
+	 * @param initializers the initializers that should be invoked
+	 * @since 1.2.1
+	 */
+	public ServletContextInitializerConfiguration(
+			ServletContextInitializer... initializers) {
 		Assert.notNull(initializers, "Initializers must not be null");
-		this.contextHandler = contextHandler;
 		this.initializers = initializers;
-
 	}
 
 	@Override
 	public void configure(WebAppContext context) throws Exception {
-		context.addBean(new InitializerListener(), true);
+		context.addBean(new Initializer(context), true);
 	}
 
-	private class InitializerListener extends AbstractLifeCycle {
+	/**
+	 * Jetty {@link AbstractLifeCycle} to call the {@link ServletContextInitializer
+	 * ServletContextInitializers}.
+	 */
+	private class Initializer extends AbstractLifeCycle {
+
+		private final WebAppContext context;
+
+		public Initializer(WebAppContext context) {
+			this.context = context;
+		}
 
 		@Override
 		protected void doStart() throws Exception {
-			ServletContext servletContext = ServletContextInitializerConfiguration.this.contextHandler
-					.getServletContext();
-			for (ServletContextInitializer initializer : ServletContextInitializerConfiguration.this.initializers) {
-				initializer.onStartup(servletContext);
+			ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
+			Thread.currentThread().setContextClassLoader(this.context.getClassLoader());
+			try {
+				callInitializers();
+			}
+			finally {
+				Thread.currentThread().setContextClassLoader(classLoader);
+			}
+		}
+
+		private void callInitializers() throws ServletException {
+			try {
+				setExtendedListenerTypes(true);
+				for (ServletContextInitializer initializer : ServletContextInitializerConfiguration.this.initializers) {
+					initializer.onStartup(this.context.getServletContext());
+				}
+			}
+			finally {
+				setExtendedListenerTypes(false);
+			}
+		}
+
+		private void setExtendedListenerTypes(boolean extended) {
+			try {
+				this.context.getServletContext().setExtendedListenerTypes(extended);
+			}
+			catch (NoSuchMethodError ex) {
+				// Not available on Jetty 8
 			}
 		}
 	}

spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedContext.java

@@ -26,10 +26,11 @@ import org.springframework.util.ReflectionUtils;
  * support deferred initialization.
  *
  * @author Phillip Webb
+ * @author Andy Wilkinson
  */
 class TomcatEmbeddedContext extends StandardContext {
 
-	private ServletContextInitializerLifecycleListener starter;
+	private TomcatStarter starter;
 
 	private final boolean overrideLoadOnStart;
 
@@ -69,11 +70,11 @@ class TomcatEmbeddedContext extends StandardContext {
 		}
 	}
 
-	public void setStarter(ServletContextInitializerLifecycleListener starter) {
+	public void setStarter(TomcatStarter starter) {
 		this.starter = starter;
 	}
 
-	public ServletContextInitializerLifecycleListener getStarter() {
+	public TomcatStarter getStarter() {
 		return this.starter;
 	}
 

spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactory.java

@@ -25,7 +25,9 @@ import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import javax.servlet.ServletContainerInitializer;
@@ -75,6 +77,7 @@ import org.springframework.util.StringUtils;
  * @author Dave Syer
  * @author Brock Mills
  * @author Stephane Nicoll
+ * @author Andy Wilkinson
  * @see #setPort(int)
  * @see #setContextLifecycleListeners(Collection)
  * @see TomcatEmbeddedServletContainer
@@ -82,6 +85,8 @@ import org.springframework.util.StringUtils;
 public class TomcatEmbeddedServletContainerFactory extends
 		AbstractEmbeddedServletContainerFactory implements ResourceLoaderAware {
 
+	private static final Set<Class<?>> NO_CLASSES = Collections.emptySet();
+
 	public static final String DEFAULT_PROTOCOL = "org.apache.coyote.http11.Http11NioProtocol";
 
 	private File baseDirectory;
@@ -322,13 +327,12 @@ public class TomcatEmbeddedServletContainerFactory extends
 	 */
 	protected void configureContext(Context context,
 			ServletContextInitializer[] initializers) {
-		ServletContextInitializerLifecycleListener starter = new ServletContextInitializerLifecycleListener(
-				initializers);
+		TomcatStarter starter = new TomcatStarter(initializers);
 		if (context instanceof TomcatEmbeddedContext) {
 			// Should be true
 			((TomcatEmbeddedContext) context).setStarter(starter);
 		}
-		context.addLifecycleListener(starter);
+		context.addServletContainerInitializer(starter, NO_CLASSES);
 		for (LifecycleListener lifecycleListener : this.contextLifecycleListeners) {
 			context.addLifecycleListener(lifecycleListener);
 		}

spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/ServletContextInitializerLifecycleListener.java → spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatStarter.java

 /*
- * Copyright 2012-2013 the original author or authors.
+ * Copyright 2012-2014 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
+ *      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,
@@ -16,63 +16,56 @@
 
 package org.springframework.boot.context.embedded.tomcat;
 
-import org.apache.catalina.Lifecycle;
-import org.apache.catalina.LifecycleEvent;
-import org.apache.catalina.LifecycleListener;
-import org.apache.catalina.core.StandardContext;
+import java.util.Set;
+
+import javax.servlet.ServletContainerInitializer;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.boot.context.embedded.ServletContextInitializer;
-import org.springframework.util.Assert;
 
 /**
- * Tomcat {@link LifecycleListener} that calls {@link ServletContextInitializer}s.
+ * {@link ServletContainerInitializer} used to trigger {@link ServletContextInitializer
+ * ServletContextInitializers} and track startup errors.
  *
  * @author Phillip Webb
- * @author Dave Syer
+ * @author Andy Wilkinson
+ * @since 1.2.1
  */
-public class ServletContextInitializerLifecycleListener implements LifecycleListener {
+class TomcatStarter implements ServletContainerInitializer {
 
-	private static Log logger = LogFactory
-			.getLog(ServletContextInitializerLifecycleListener.class);
+	private static final Log logger = LogFactory.getLog(TomcatStarter.class);
 
 	private final ServletContextInitializer[] initializers;
 
-	private Exception startUpException;
+	private volatile Exception startUpException;
 
-	/**
-	 * Create a new {@link ServletContextInitializerLifecycleListener} instance with the
-	 * specified initializers.
-	 * @param initializers the initializers to call
-	 */
-	public ServletContextInitializerLifecycleListener(
-			ServletContextInitializer... initializers) {
+	public TomcatStarter(ServletContextInitializer[] initializers) {
 		this.initializers = initializers;
 	}
 
-	public Exception getStartUpException() {
-		return this.startUpException;
-	}
-
 	@Override
-	public void lifecycleEvent(LifecycleEvent event) {
-		if (Lifecycle.CONFIGURE_START_EVENT.equals(event.getType())) {
-			Assert.isInstanceOf(StandardContext.class, event.getSource());
-			StandardContext standardContext = (StandardContext) event.getSource();
+	public void onStartup(Set<Class<?>> classes, ServletContext servletContext)
+			throws ServletException {
+		try {
 			for (ServletContextInitializer initializer : this.initializers) {
-				try {
-					initializer.onStartup(standardContext.getServletContext());
-				}
-				catch (Exception ex) {
-					this.startUpException = ex;
-					// Prevent Tomcat from logging and re-throwing when we know we can
-					// deal with it in the main thread, but log for information here.
-					logger.error("Error starting Tomcat context: "
-							+ ex.getClass().getName());
-					break;
-				}
+				initializer.onStartup(servletContext);
 			}
 		}
+		catch (Exception ex) {
+			this.startUpException = ex;
+			// Prevent Tomcat from logging and re-throwing when we know we can
+			// deal with it in the main thread, but log for information here.
+			if (logger.isErrorEnabled()) {
+				logger.error("Error starting Tomcat context: " + ex.getClass().getName());
+			}
+		}
+	}
+
+	public Exception getStartUpException() {
+		return this.startUpException;
 	}
 
 }

spring-boot/src/main/java/org/springframework/boot/context/embedded/undertow/UndertowEmbeddedServletContainerFactory.java

@@ -29,8 +29,8 @@ import io.undertow.server.session.SessionManager;
 import io.undertow.servlet.Servlets;
 import io.undertow.servlet.api.DeploymentInfo;
 import io.undertow.servlet.api.DeploymentManager;
-import io.undertow.servlet.api.ListenerInfo;
 import io.undertow.servlet.api.MimeMapping;
+import io.undertow.servlet.api.ServletContainerInitializerInfo;
 import io.undertow.servlet.api.ServletStackTraces;
 import io.undertow.servlet.handlers.DefaultServlet;
 import io.undertow.servlet.util.ImmediateInstanceFactory;
@@ -44,15 +44,17 @@ import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
-import javax.servlet.ServletContextEvent;
-import javax.servlet.ServletContextListener;
+import javax.servlet.ServletContainerInitializer;
+import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 
 import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory;
@@ -87,6 +89,8 @@ import org.xnio.SslClientAuthMode;
 public class UndertowEmbeddedServletContainerFactory extends
 		AbstractEmbeddedServletContainerFactory implements ResourceLoaderAware {
 
+	private static final Set<Class<?>> NO_CLASSES = Collections.emptySet();
+
 	private List<UndertowBuilderCustomizer> builderCustomizers = new ArrayList<UndertowBuilderCustomizer>();
 
 	private List<UndertowDeploymentInfoCustomizer> deploymentInfoCustomizers = new ArrayList<UndertowDeploymentInfoCustomizer>();
@@ -323,10 +327,8 @@ public class UndertowEmbeddedServletContainerFactory extends
 	private DeploymentManager createDeploymentManager(
 			ServletContextInitializer... initializers) {
 		DeploymentInfo deployment = Servlets.deployment();
-		ServletContextInitializer[] mergeInitializers = mergeInitializers(initializers);
-		StartupListener startupListener = new StartupListener(mergeInitializers);
-		deployment.addListener(new ListenerInfo(StartupListener.class,
-				new ImmediateInstanceFactory<StartupListener>(startupListener)));
+		registerServletContainerInitializerToDriveServletContextInitializers(deployment,
+				initializers);
 		deployment.setClassLoader(getServletClassLoader());
 		String contextPath = getContextPath();
 		deployment.setContextPath(StringUtils.hasLength(contextPath) ? contextPath : "/");
@@ -349,6 +351,16 @@ public class UndertowEmbeddedServletContainerFactory extends
 		return manager;
 	}
 
+	private void registerServletContainerInitializerToDriveServletContextInitializers(
+			DeploymentInfo deployment, ServletContextInitializer... initializers) {
+		ServletContextInitializer[] mergedInitializers = mergeInitializers(initializers);
+		Initializer initializer = new Initializer(mergedInitializers);
+		deployment.addServletContainerInitalizer(new ServletContainerInitializerInfo(
+				Initializer.class,
+				new ImmediateInstanceFactory<ServletContainerInitializer>(initializer),
+				NO_CLASSES));
+	}
+
 	private ClassLoader getServletClassLoader() {
 		if (this.resourceLoader != null) {
 			return this.resourceLoader.getClassLoader();
@@ -490,33 +502,25 @@ public class UndertowEmbeddedServletContainerFactory extends
 	}
 
 	/**
-	 * {@link ServletContextListener} to trigger
-	 * {@link ServletContextInitializer#onStartup(javax.servlet.ServletContext)}.
+	 * {@link ServletContainerInitializer} to initialize {@link ServletContextInitializer
+	 * ServletContextInitializers}.
 	 */
-	private static class StartupListener implements ServletContextListener {
+	private static class Initializer implements ServletContainerInitializer {
 
 		private final ServletContextInitializer[] initializers;
 
-		public StartupListener(ServletContextInitializer... initializers) {
+		public Initializer(ServletContextInitializer[] initializers) {
 			this.initializers = initializers;
 		}
 
 		@Override
-		public void contextInitialized(ServletContextEvent event) {
-			try {
-				for (ServletContextInitializer initializer : this.initializers) {
-					initializer.onStartup(event.getServletContext());
-				}
-			}
-			catch (ServletException ex) {
-				throw new IllegalStateException(ex);
+		public void onStartup(Set<Class<?>> classes, ServletContext servletContext)
+				throws ServletException {
+			for (ServletContextInitializer initializer : this.initializers) {
+				initializer.onStartup(servletContext);
 			}
 		}
 
-		@Override
-		public void contextDestroyed(ServletContextEvent sce) {
-		}
-
 	}
 
 }