From cb0d992303213c42403e7c0f2da6253d889b1e37 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 27 Jan 2017 10:31:08 -0500 Subject: [PATCH] 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 . --- ...erSupport.java => AbstractHttpServer.java} | 83 +++++++++++++++++-- .../reactive/bootstrap/JettyHttpServer.java | 82 +++++++++--------- .../reactive/bootstrap/ReactorHttpServer.java | 51 ++++++------ .../reactive/bootstrap/RxNettyHttpServer.java | 47 +++++------ .../reactive/bootstrap/TomcatHttpServer.java | 69 +++++---------- .../bootstrap/UndertowHttpServer.java | 41 ++++----- 6 files changed, 195 insertions(+), 178 deletions(-) rename spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/{HttpServerSupport.java => AbstractHttpServer.java} (52%) diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/HttpServerSupport.java b/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/AbstractHttpServer.java similarity index 52% rename from spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/HttpServerSupport.java rename to spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/AbstractHttpServer.java index fbe069a943..7fa8687222 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/HttpServerSupport.java +++ b/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 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; + } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/JettyHttpServer.java b/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/JettyHttpServer.java index 5e98637bf9..279b505bc5 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/JettyHttpServer.java +++ b/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; + } } } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/ReactorHttpServer.java b/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/ReactorHttpServer.java index 2588e3172a..e8dfd7cff2 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/ReactorHttpServer.java +++ b/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); + } + } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/RxNettyHttpServer.java b/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/RxNettyHttpServer.java index 91a2f52490..fd183cc34b 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/RxNettyHttpServer.java +++ b/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 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())); } - - @Override - public boolean isRunning() { - return this.running; + @NotNull + private RxNettyHttpHandlerAdapter createHttpHandlerAdapter() { + return getHttpHandlerMap() != null ? + new RxNettyHttpHandlerAdapter(getHttpHandlerMap()) : + new RxNettyHttpHandlerAdapter(getHttpHandler()); } @Override - public void start() { - if (!this.running) { - this.running = true; - this.rxNettyServer.start(this.rxNettyHandler); - } + protected void startInternal() { + this.rxNettyServer.start(this.rxNettyHandler); } @Override - public void stop() { - if (this.running) { - this.running = false; - this.rxNettyServer.shutdown(); - } + protected void stopInternal() { + this.rxNettyServer.shutdown(); + } + + @Override + protected void reset() { + super.reset(); + this.rxNettyServer = null; + this.rxNettyHandler = null; } } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/TomcatHttpServer.java b/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/TomcatHttpServer.java index 958901bbd4..fc41b35572 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/TomcatHttpServer.java +++ b/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 final String baseDir; + + private final Class wsListener; private Tomcat tomcatServer; - private boolean running; - - private String baseDir; - - private Class wsListener; - - - 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; } } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/UndertowHttpServer.java b/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/UndertowHttpServer.java index 4307ab5f4e..f682d94e83 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/bootstrap/UndertowHttpServer.java +++ b/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; } }