Refactor test HttpServer implementations

Due to the static nature of JUnit parameterized test inputs, an
HttpServer instance is re-used for all tests per test class.

This commit adds lifecycle handling to AbstractHttpServer with a
lifecycle monitor to ensure test server fields are properly
initialized and reset after each test .

spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/HttpServerSupport.java → spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/AbstractHttpServer.java

@@ -20,12 +20,12 @@ import java.util.LinkedHashMap;
 import java.util.Map;
 
 import org.springframework.http.server.reactive.HttpHandler;
-import org.springframework.util.SocketUtils;
+import org.springframework.util.Assert;
 
 /**
  * @author Rossen Stoyanchev
  */
-public class HttpServerSupport {
+public abstract class AbstractHttpServer implements HttpServer {
 
 	private String host = "0.0.0.0";
 
@@ -35,6 +35,10 @@ public class HttpServerSupport {
 
 	private Map<String, HttpHandler> handlerMap;
 
+	private boolean running;
+
+	private final Object lifecycleMonitor = new Object();
+
 
 	public void setHost(String host) {
 		this.host = host;
@@ -49,9 +53,6 @@ public class HttpServerSupport {
 	}
 
 	public int getPort() {
-		if(this.port == -1) {
-			this.port = SocketUtils.findAvailableTcpPort(8080);
-		}
 		return this.port;
 	}
 
@@ -74,4 +75,76 @@ public class HttpServerSupport {
 		return this.handlerMap;
 	}
 
+
+	// InitializingBean
+
+	@Override
+	public final void afterPropertiesSet() throws Exception {
+		synchronized (this.lifecycleMonitor) {
+			Assert.isTrue(this.host != null);
+			Assert.isTrue(this.port != -1);
+			Assert.isTrue(this.httpHandler != null || this.handlerMap != null);
+			Assert.isTrue(!this.running);
+
+			initServer();
+		}
+	}
+
+	protected abstract void initServer() throws Exception;
+
+
+	// Lifecycle
+
+	@Override
+	public boolean isRunning() {
+		synchronized (this.lifecycleMonitor) {
+			return this.running;
+		}
+	}
+
+	@Override
+	public final void start() {
+		synchronized (this.lifecycleMonitor) {
+			if (!isRunning()) {
+				this.running = true;
+				try {
+					startInternal();
+				}
+				catch (Exception ex) {
+					throw new IllegalStateException(ex);
+				}
+			}
+		}
+
+	}
+
+	protected abstract void startInternal() throws Exception;
+
+	@Override
+	public final void stop() {
+		synchronized (this.lifecycleMonitor) {
+			if (isRunning()) {
+				this.running = false;
+				try {
+					stopInternal();
+				}
+				catch (Exception ex) {
+					throw new IllegalStateException(ex);
+				}
+				finally {
+					reset();
+				}
+			}
+		}
+	}
+
+	protected void reset() {
+		this.host = "0.0.0.0";
+		this.port = -1;
+		this.httpHandler = null;
+		this.handlerMap = null;
+	}
+
+	protected abstract void stopInternal() throws Exception;
+
 }

spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/JettyHttpServer.java

@@ -21,28 +21,25 @@ import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
 
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.http.server.reactive.JettyHttpHandlerAdapter;
 import org.springframework.http.server.reactive.ServletHttpHandlerAdapter;
-import org.springframework.util.Assert;
 
 /**
  * @author Rossen Stoyanchev
  */
-public class JettyHttpServer extends HttpServerSupport implements HttpServer, InitializingBean {
+public class JettyHttpServer extends AbstractHttpServer {
 
 	private Server jettyServer;
 
 	private ServletContextHandler contextHandler;
 
-	private boolean running;
-
 
 	@Override
-	public void afterPropertiesSet() throws Exception {
+	protected void initServer() throws Exception {
+
 		this.jettyServer = new Server();
 
-		ServletHttpHandlerAdapter servlet = initServletHttpHandlerAdapter();
+		ServletHttpHandlerAdapter servlet = createServletAdapter();
 		ServletHolder servletHolder = new ServletHolder(servlet);
 
 		this.contextHandler = new ServletContextHandler(this.jettyServer, "", false, false);
@@ -55,59 +52,54 @@ public class JettyHttpServer extends HttpServerSupport implements HttpServer, In
 		this.jettyServer.addConnector(connector);
 	}
 
-	private ServletHttpHandlerAdapter initServletHttpHandlerAdapter() {
-		if (getHttpHandlerMap() != null) {
-			return new JettyHttpHandlerAdapter(getHttpHandlerMap());
-		}
-		else {
-			Assert.notNull(getHttpHandler());
-			return new JettyHttpHandlerAdapter(getHttpHandler());
-		}
+	private ServletHttpHandlerAdapter createServletAdapter() {
+		return getHttpHandlerMap() != null ? new JettyHttpHandlerAdapter(getHttpHandlerMap()) :
+				new JettyHttpHandlerAdapter(getHttpHandler());
 	}
 
 	@Override
-	public void start() {
-		if (!this.running) {
-			try {
-				this.running = true;
-				this.jettyServer.start();
-			}
-			catch (Exception ex) {
-				throw new IllegalStateException(ex);
-			}
-		}
+	protected void startInternal() throws Exception {
+		this.jettyServer.start();
 	}
 
 	@Override
-	public void stop() {
-		if (this.running) {
+	protected void stopInternal() throws Exception {
+		try {
+			if (this.contextHandler.isRunning()) {
+				this.contextHandler.stop();
+			}
+		}
+		finally {
 			try {
-				this.running = false;
-				if (this.contextHandler.isRunning()) {
-					this.contextHandler.stop();
+				if (this.jettyServer.isRunning()) {
+					this.jettyServer.setStopTimeout(5000);
+					this.jettyServer.stop();
+					this.jettyServer.destroy();
 				}
 			}
 			catch (Exception ex) {
-				throw new IllegalStateException(ex);
-			}
-			finally {
-				try {
-					if (this.jettyServer.isRunning()) {
-						this.jettyServer.setStopTimeout(5000);
-						this.jettyServer.stop();
-						this.jettyServer.destroy();
-					}
-				}
-				catch (Exception ex) {
-					throw new IllegalStateException(ex);
-				}
+				// ignore
 			}
 		}
 	}
 
 	@Override
-	public boolean isRunning() {
-		return this.running;
+	protected void reset() {
+		super.reset();
+		try {
+			if (this.jettyServer.isRunning()) {
+				this.jettyServer.setStopTimeout(5000);
+				this.jettyServer.stop();
+				this.jettyServer.destroy();
+			}
+		}
+		catch (Exception ex) {
+			throw new IllegalStateException(ex);
+		}
+		finally {
+			this.jettyServer = null;
+			this.contextHandler = null;
+		}
 	}
 
 }

spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/ReactorHttpServer.java

@@ -18,16 +18,16 @@ package org.springframework.http.server.reactive.bootstrap;
 
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.jetbrains.annotations.NotNull;
 import reactor.core.Loopback;
 import reactor.ipc.netty.NettyContext;
 
 import org.springframework.http.server.reactive.ReactorHttpHandlerAdapter;
-import org.springframework.util.Assert;
 
 /**
  * @author Stephane Maldini
  */
-public class ReactorHttpServer extends HttpServerSupport implements HttpServer, Loopback {
+public class ReactorHttpServer extends AbstractHttpServer implements Loopback {
 
 	private ReactorHttpHandlerAdapter reactorHandler;
 
@@ -37,25 +37,19 @@ public class ReactorHttpServer extends HttpServerSupport implements HttpServer,
 
 
 	@Override
-	public void afterPropertiesSet() throws Exception {
-		if (getHttpHandlerMap() != null) {
-			this.reactorHandler = new ReactorHttpHandlerAdapter(getHttpHandlerMap());
-		}
-		else {
-			Assert.notNull(getHttpHandler());
-			this.reactorHandler = new ReactorHttpHandlerAdapter(getHttpHandler());
-		}
-		this.reactorServer = reactor.ipc.netty.http.server.HttpServer
-				.create(getHost(), getPort());
+	protected void initServer() throws Exception {
+		this.reactorHandler = createHttpHandlerAdapter();
+		this.reactorServer = reactor.ipc.netty.http.server.HttpServer.create(getHost(), getPort());
 	}
 
-
-	@Override
-	public boolean isRunning() {
-		NettyContext context = this.nettyContext.get();
-		return (context != null && context.channel().isActive());
+	@NotNull
+	private ReactorHttpHandlerAdapter createHttpHandlerAdapter() {
+		return getHttpHandlerMap() != null ?
+				new ReactorHttpHandlerAdapter(getHttpHandlerMap()) :
+				new ReactorHttpHandlerAdapter(getHttpHandler());
 	}
 
+
 	@Override
 	public Object connectedInput() {
 		return this.reactorServer;
@@ -67,17 +61,22 @@ public class ReactorHttpServer extends HttpServerSupport implements HttpServer,
 	}
 
 	@Override
-	public void start() {
-		if (this.nettyContext.get() == null) {
-			this.nettyContext.set(this.reactorServer.newHandler(reactorHandler).block());
-		}
+	protected void startInternal() {
+		NettyContext nettyContext = this.reactorServer.newHandler(this.reactorHandler).block();
+		this.nettyContext.set(nettyContext);
 	}
 
 	@Override
-	public void stop() {
-		NettyContext context = this.nettyContext.getAndSet(null);
-		if (context != null) {
-			context.dispose();
-		}
+	protected void stopInternal() {
+		this.nettyContext.get().dispose();
 	}
+
+	@Override
+	protected void reset() {
+		super.reset();
+		this.reactorServer = null;
+		this.reactorHandler = null;
+		this.nettyContext.set(null);
+	}
+
 }

spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/RxNettyHttpServer.java

@@ -19,58 +19,51 @@ package org.springframework.http.server.reactive.bootstrap;
 import java.net.InetSocketAddress;
 
 import io.netty.buffer.ByteBuf;
+import org.jetbrains.annotations.NotNull;
 
 import org.springframework.http.server.reactive.RxNettyHttpHandlerAdapter;
-import org.springframework.util.Assert;
 
 
 /**
  * @author Rossen Stoyanchev
  */
-public class RxNettyHttpServer extends HttpServerSupport implements HttpServer {
+public class RxNettyHttpServer extends AbstractHttpServer {
 
 	private RxNettyHttpHandlerAdapter rxNettyHandler;
 
 	private io.reactivex.netty.protocol.http.server.HttpServer<ByteBuf, ByteBuf> rxNettyServer;
 
-	private boolean running;
 
 	@Override
-	public void afterPropertiesSet() throws Exception {
-
-		if (getHttpHandlerMap() != null) {
-			this.rxNettyHandler = new RxNettyHttpHandlerAdapter(getHttpHandlerMap());
-		}
-		else {
-			Assert.notNull(getHttpHandler());
-			this.rxNettyHandler = new RxNettyHttpHandlerAdapter(getHttpHandler());
-		}
-
+	protected void initServer() throws Exception {
+		this.rxNettyHandler = createHttpHandlerAdapter();
 		this.rxNettyServer = io.reactivex.netty.protocol.http.server.HttpServer
 				.newServer(new InetSocketAddress(getHost(), getPort()));
 	}
 
+	@NotNull
+	private RxNettyHttpHandlerAdapter createHttpHandlerAdapter() {
+		return getHttpHandlerMap() != null ?
+				new RxNettyHttpHandlerAdapter(getHttpHandlerMap()) :
+				new RxNettyHttpHandlerAdapter(getHttpHandler());
+	}
+
 
 	@Override
-	public boolean isRunning() {
-		return this.running;
+	protected void startInternal() {
+		this.rxNettyServer.start(this.rxNettyHandler);
 	}
 
-
 	@Override
-	public void start() {
-		if (!this.running) {
-			this.running = true;
-			this.rxNettyServer.start(this.rxNettyHandler);
-		}
+	protected void stopInternal() {
+		this.rxNettyServer.shutdown();
 	}
 
 	@Override
-	public void stop() {
-		if (this.running) {
-			this.running = false;
-			this.rxNettyServer.shutdown();
-		}
+	protected void reset() {
+		super.reset();
+		this.rxNettyServer = null;
+		this.rxNettyHandler = null;
 	}
 
 }

spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/TomcatHttpServer.java

@@ -22,7 +22,6 @@ import org.apache.catalina.Context;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.startup.Tomcat;
 
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.http.server.reactive.ServletHttpHandlerAdapter;
 import org.springframework.http.server.reactive.TomcatHttpHandlerAdapter;
 import org.springframework.util.Assert;
@@ -30,40 +29,34 @@ import org.springframework.util.Assert;
 /**
  * @author Rossen Stoyanchev
  */
-public class TomcatHttpServer extends HttpServerSupport implements HttpServer, InitializingBean {
+public class TomcatHttpServer extends AbstractHttpServer {
 
-	private Tomcat tomcatServer;
-
-	private boolean running;
-
-	private String baseDir;
+	private final String baseDir;
 
-	private Class<?> wsListener;
+	private final Class<?> wsListener;
 
+	private Tomcat tomcatServer;
 
-	public TomcatHttpServer() {
-	}
 
 	public TomcatHttpServer(String baseDir) {
-		this.baseDir = baseDir;
+		this(baseDir, null);
 	}
 
 	public TomcatHttpServer(String baseDir, Class<?> wsListener) {
+		Assert.notNull(baseDir);
 		this.baseDir = baseDir;
 		this.wsListener = wsListener;
 	}
 
 
 	@Override
-	public void afterPropertiesSet() throws Exception {
+	protected void initServer() throws Exception {
 		this.tomcatServer = new Tomcat();
-		if (this.baseDir != null) {
-			this.tomcatServer.setBaseDir(baseDir);
-		}
+		this.tomcatServer.setBaseDir(baseDir);
 		this.tomcatServer.setHostname(getHost());
 		this.tomcatServer.setPort(getPort());
 
-		ServletHttpHandlerAdapter servlet = initServletHttpHandlerAdapter();
+		ServletHttpHandlerAdapter servlet = initServletAdapter();
 
 		File base = new File(System.getProperty("java.io.tmpdir"));
 		Context rootContext = tomcatServer.addContext("", base.getAbsolutePath());
@@ -74,47 +67,27 @@ public class TomcatHttpServer extends HttpServerSupport implements HttpServer, I
 		}
 	}
 
-	private ServletHttpHandlerAdapter initServletHttpHandlerAdapter() {
-		if (getHttpHandlerMap() != null) {
-			return new TomcatHttpHandlerAdapter(getHttpHandlerMap());
-		}
-		else {
-			Assert.notNull(getHttpHandler());
-			return new TomcatHttpHandlerAdapter(getHttpHandler());
-		}
+	private ServletHttpHandlerAdapter initServletAdapter() {
+		return getHttpHandlerMap() != null ?
+				new TomcatHttpHandlerAdapter(getHttpHandlerMap()) :
+				new TomcatHttpHandlerAdapter(getHttpHandler());
 	}
 
 
 	@Override
-	public void start() {
-		if (!this.running) {
-			try {
-				this.running = true;
-				this.tomcatServer.start();
-			}
-			catch (LifecycleException ex) {
-				throw new IllegalStateException(ex);
-			}
-		}
+	protected void startInternal() throws LifecycleException {
+		this.tomcatServer.start();
 	}
 
 	@Override
-	public void stop() {
-		if (this.running) {
-			try {
-				this.running = false;
-				this.tomcatServer.stop();
-				this.tomcatServer.destroy();
-			}
-			catch (LifecycleException ex) {
-				throw new IllegalStateException(ex);
-			}
-		}
+	protected void stopInternal() throws Exception {
+		this.tomcatServer.stop();
+		this.tomcatServer.destroy();
 	}
 
 	@Override
-	public boolean isRunning() {
-		return this.running;
+	protected void reset() {
+		this.tomcatServer = null;
 	}
 
 }

spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/UndertowHttpServer.java

@@ -19,55 +19,42 @@ package org.springframework.http.server.reactive.bootstrap;
 import io.undertow.Undertow;
 
 import org.springframework.http.server.reactive.UndertowHttpHandlerAdapter;
-import org.springframework.util.Assert;
 
 /**
  * @author Marek Hawrylczak
  */
-public class UndertowHttpServer extends HttpServerSupport implements HttpServer {
+public class UndertowHttpServer extends AbstractHttpServer {
 
 	private Undertow server;
 
-	private boolean running;
-
 
 	@Override
-	public void afterPropertiesSet() throws Exception {
+	protected void initServer() throws Exception {
 		this.server = Undertow.builder().addHttpListener(getPort(), getHost())
-				.setHandler(initUndertowHttpHandlerAdapter())
+				.setHandler(initHttpHandlerAdapter())
 				.build();
 	}
 
-	private UndertowHttpHandlerAdapter initUndertowHttpHandlerAdapter() {
-		if (getHttpHandlerMap() != null) {
-			return new UndertowHttpHandlerAdapter(getHttpHandlerMap());
-		}
-		else {
-			Assert.notNull(getHttpHandler());
-			return new UndertowHttpHandlerAdapter(getHttpHandler());
-		}
+	private UndertowHttpHandlerAdapter initHttpHandlerAdapter() {
+		return getHttpHandlerMap() != null ?
+				new UndertowHttpHandlerAdapter(getHttpHandlerMap()) :
+				new UndertowHttpHandlerAdapter(getHttpHandler());
 	}
 
 	@Override
-	public void start() {
-		if (!this.running) {
-			this.server.start();
-			this.running = true;
-		}
-
+	protected void startInternal() {
+		this.server.start();
 	}
 
 	@Override
-	public void stop() {
-		if (this.running) {
-			this.server.stop();
-			this.running = false;
-		}
+	protected void stopInternal() {
+		this.server.stop();
 	}
 
 	@Override
-	public boolean isRunning() {
-		return this.running;
+	protected void reset() {
+		super.reset();
+		this.server = null;
 	}
 
 }