Skip to content

Commit

Permalink
Inject server HTTP handler as a constructor arg.
Browse files Browse the repository at this point in the history
  • Loading branch information
shs96c committed Oct 15, 2019
1 parent 953ef02 commit 75aa2dd
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 61 deletions.
3 changes: 1 addition & 2 deletions java/server/src/org/openqa/selenium/grid/commands/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 0 additions & 4 deletions java/server/src/org/openqa/selenium/grid/server/Server.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
14 changes: 3 additions & 11 deletions java/server/src/org/openqa/selenium/jetty/server/JettyServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(() -> {
Expand Down Expand Up @@ -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();
Expand Down
21 changes: 9 additions & 12 deletions java/server/test/org/openqa/selenium/grid/router/EndToEndTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -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());
Expand All @@ -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(
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit 75aa2dd

Please sign in to comment.