Inject server HTTP handler as a constructor arg.

Author: shs96c

java/server/src/org/openqa/selenium/grid/commands/Hub.java

@@ -121,8 +121,7 @@ public Executable configure(String... args) {
       handler.addHandler(distributor);
       Router router = new Router(clientFactory, sessions, distributor);
 
-      Server<?> server = new JettyServer(serverOptions);
-      server.setHandler(router);
+      Server<?> server = new JettyServer(serverOptions, router);
       server.start();
 
       BuildInfo info = new BuildInfo();

java/server/src/org/openqa/selenium/grid/commands/Standalone.java

@@ -158,8 +158,7 @@ public Executable configure(String... args) {
       combinedHandler.addHandler(node);
       distributor.add(node);
 
-      Server<?> server = new JettyServer(new BaseServerOptions(config));
-      server.setHandler(router);
+      Server<?> server = new JettyServer(new BaseServerOptions(config), router);
       server.start();
 
       BuildInfo info = new BuildInfo();

java/server/src/org/openqa/selenium/grid/distributor/httpd/DistributorServer.java

@@ -116,8 +116,7 @@ public Executable configure(String... args) {
 
       BaseServerOptions serverOptions = new BaseServerOptions(config);
 
-      Server<?> server = new JettyServer(serverOptions);
-      server.setHandler(distributor);
+      Server<?> server = new JettyServer(serverOptions, distributor);
       server.start();
 
       BuildInfo info = new BuildInfo();

java/server/src/org/openqa/selenium/grid/node/httpd/NodeServer.java

@@ -124,8 +124,7 @@ public Executable configure(String... args) {
 
       LocalNode node = builder.build();
 
-      Server<?> server = new JettyServer(serverOptions);
-      server.setHandler(node);
+      Server<?> server = new JettyServer(serverOptions, node);
       server.start();
 
       BuildInfo info = new BuildInfo();

java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java

@@ -111,8 +111,7 @@ public Executable configure(String... args) {
 
       Router router = new Router(clientFactory, sessions, distributor);
 
-      Server<?> server = new JettyServer(serverOptions);
-      server.setHandler(router);
+      Server<?> server = new JettyServer(serverOptions, router);
       server.start();
 
       BuildInfo info = new BuildInfo();

java/server/src/org/openqa/selenium/grid/server/Server.java

@@ -18,16 +18,12 @@
 package org.openqa.selenium.grid.server;
 
 import org.openqa.selenium.grid.component.HasLifecycle;
-import org.openqa.selenium.remote.http.HttpHandler;
 
-import javax.servlet.Servlet;
 import java.net.URL;
 
 public interface Server<T extends Server> extends HasLifecycle<T> {
 
   boolean isStarted();
 
-  T setHandler(HttpHandler handler);
-
   URL getUrl();
 }

java/server/src/org/openqa/selenium/grid/sessionmap/httpd/SessionMapServer.java

@@ -101,8 +101,7 @@ public Executable configure(String... args) {
 
       BaseServerOptions serverOptions = new BaseServerOptions(config);
 
-      Server<?> server = new JettyServer(serverOptions);
-      server.setHandler(sessions);
+      Server<?> server = new JettyServer(serverOptions, sessions);
       server.start();
 
       BuildInfo info = new BuildInfo();

java/server/src/org/openqa/selenium/jetty/server/JettyServer.java

@@ -59,9 +59,10 @@ public class JettyServer implements Server<JettyServer> {
   private final org.eclipse.jetty.server.Server server;
   private final ServletContextHandler servletContextHandler;
   private final URL url;
-  private HttpHandler handler;
+  private final HttpHandler handler;
 
-  public JettyServer(BaseServerOptions options) {
+  public JettyServer(BaseServerOptions options, HttpHandler handler) {
+    this.handler = Objects.requireNonNull(handler, "Handler to use must be set.");
     int port = options.getPort() == 0 ? PortProber.findFreePort() : options.getPort();
 
     String host = options.getHostname().orElseGet(() -> {
@@ -131,15 +132,6 @@ public JettyServer(BaseServerOptions options) {
     server.setConnectors(new Connector[]{http});
   }
 
-  @Override
-  public JettyServer setHandler(HttpHandler handler) {
-    if (server.isRunning()) {
-      throw new IllegalStateException("You may not add a handler to a running server");
-    }
-    this.handler = Objects.requireNonNull(handler, "Handler to use must be set.");
-    return this;
-  }
-
   @Override
   public boolean isStarted() {
     return server.isStarted();

java/server/test/org/openqa/selenium/grid/router/EndToEndTest.java

@@ -146,8 +146,7 @@ private static Object[] createInMemory() throws URISyntaxException, MalformedURL
 
     Router router = new Router(clientFactory, sessions, distributor);
 
-    Server<?> server = createServer();
-    server.setHandler(router);
+    Server<?> server = createServer(router);
     server.start();
 
     return new Object[] { server, clientFactory };
@@ -164,8 +163,7 @@ private static Object[] createRemotes() throws URISyntaxException {
 
     HttpClient.Factory clientFactory = HttpClient.Factory.createDefault();
 
-    Server<?> sessionServer = createServer();
-    sessionServer.setHandler(localSessions);
+    Server<?> sessionServer = createServer(localSessions);
     sessionServer.start();
 
     HttpClient client = HttpClient.Factory.createDefault().createClient(sessionServer.getUrl());
@@ -175,8 +173,7 @@ private static Object[] createRemotes() throws URISyntaxException {
         bus,
         clientFactory,
         sessions);
-    Server<?> distributorServer = createServer();
-    distributorServer.setHandler(localDistributor);
+    Server<?> distributorServer = createServer(localDistributor);
     distributorServer.start();
 
     Distributor distributor = new RemoteDistributor(
@@ -190,24 +187,24 @@ private static Object[] createRemotes() throws URISyntaxException {
         .build();
     Server<?> nodeServer = new JettyServer(
         new BaseServerOptions(
-            new MapConfig(ImmutableMap.of("server", ImmutableMap.of("port", port)))));
-    nodeServer.setHandler(localNode);
+            new MapConfig(ImmutableMap.of("server", ImmutableMap.of("port", port)))),
+      localNode);
     nodeServer.start();
 
     distributor.add(localNode);
 
     Router router = new Router(clientFactory, sessions, distributor);
-    Server<?> routerServer = createServer();
-    routerServer.setHandler(router);
+    Server<?> routerServer = createServer(router);
     routerServer.start();
 
     return new Object[] { routerServer, clientFactory };
   }
 
-  private static Server<?> createServer() {
+  private static Server<?> createServer(HttpHandler handler) {
     int port = PortProber.findFreePort();
     return new JettyServer(new BaseServerOptions(
-        new MapConfig(ImmutableMap.of("server", ImmutableMap.of("port", port)))));
+        new MapConfig(ImmutableMap.of("server", ImmutableMap.of("port", port)))),
+      handler);
   }
 
   private static SessionFactory createFactory(URI serverUri) {

java/server/test/org/openqa/selenium/grid/server/JettyServerTest.java

@@ -29,13 +29,11 @@
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
 
-import java.io.IOException;
 import java.net.URL;
 
 import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
 import static java.net.HttpURLConnection.HTTP_OK;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.fail;
 import static org.junit.Assert.assertEquals;
 import static org.openqa.selenium.json.Json.MAP_TYPE;
@@ -49,8 +47,8 @@ public class JettyServerTest {
   private BaseServerOptions emptyOptions = new BaseServerOptions(new MapConfig(ImmutableMap.of()));
 
   @Test
-  public void baseServerStartsAndDoesNothing() throws IOException {
-    Server<?> server = new JettyServer(emptyOptions).setHandler(req -> new HttpResponse()).start();
+  public void baseServerStartsAndDoesNothing() {
+    Server<?> server = new JettyServer(emptyOptions, req -> new HttpResponse()).start();
 
     URL url = server.getUrl();
     HttpClient client = HttpClient.Factory.createDefault().createClient(url);
@@ -64,9 +62,10 @@ public void baseServerStartsAndDoesNothing() throws IOException {
   }
 
   @Test
-  public void shouldAllowAHandlerToBeRegistered() throws IOException {
-    Server<?> server = new JettyServer(emptyOptions);
-    server.setHandler(get("/cheese").to(() -> req -> new HttpResponse().setContent(utf8String("cheddar"))));
+  public void shouldAllowAHandlerToBeRegistered() {
+    Server<?> server = new JettyServer(
+      emptyOptions,
+      get("/cheese").to(() -> req -> new HttpResponse().setContent(utf8String("cheddar"))));
 
     server.start();
     URL url = server.getUrl();
@@ -77,21 +76,12 @@ public void shouldAllowAHandlerToBeRegistered() throws IOException {
   }
 
   @Test
-  public void addHandlersOnceServerIsStartedIsAnError() {
-    Server<?> server = new JettyServer(emptyOptions);
-    server.setHandler(req -> new HttpResponse());
-    server.start();
-
-    assertThatExceptionOfType(IllegalStateException.class).isThrownBy(
-        () -> server.setHandler(get("/foo").to(() -> req -> new HttpResponse())));
-  }
-
-  @Test
-  public void exceptionsThrownByHandlersAreConvertedToAProperPayload() throws IOException {
-    Server<?> server = new JettyServer(emptyOptions);
-    server.setHandler(req -> {
-      throw new UnableToSetCookieException("Yowza");
-    });
+  public void exceptionsThrownByHandlersAreConvertedToAProperPayload() {
+    Server<?> server = new JettyServer(
+      emptyOptions,
+      req -> {
+        throw new UnableToSetCookieException("Yowza");
+      });
 
     server.start();
     URL url = server.getUrl();