Replace CompoundHandler with Routes

SeleniumHQ · Nov 9, 2018 · b162748 · b162748
1 parent 37ba6ab
commit b162748
Show file tree

Hide file tree

Showing 16 changed files with 109 additions and 254 deletions.
diff --git a/java/server/src/org/openqa/selenium/grid/commands/Hub.java b/java/server/src/org/openqa/selenium/grid/commands/Hub.java
@@ -40,6 +40,7 @@
 import org.openqa.selenium.grid.server.W3CCommandHandler;
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
+import org.openqa.selenium.grid.web.Routes;
 
 @AutoService(CliCommand.class)
 public class Hub implements CliCommand {
@@ -91,7 +92,7 @@ public Executable configure(String... args) {
       Router router = new Router(sessions, distributor);
 
       Server<?> server = new BaseServer<>(new BaseServerOptions(config));
-      server.addHandler(router, (inj, req) -> new W3CCommandHandler(router));
+      server.addRoute(Routes.matching(router).using(router).decorateWith(W3CCommandHandler.class));
       server.start();
     };
   }

diff --git a/java/server/src/org/openqa/selenium/grid/commands/Standalone.java b/java/server/src/org/openqa/selenium/grid/commands/Standalone.java
@@ -42,6 +42,7 @@
 import org.openqa.selenium.grid.server.W3CCommandHandler;
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
+import org.openqa.selenium.grid.web.Routes;
 import org.openqa.selenium.net.NetworkUtils;
 
 import java.net.URI;
@@ -119,7 +120,7 @@ public Executable configure(String... args) {
       distributor.add(node.build());
 
       Server<?> server = new BaseServer<>(new BaseServerOptions(config));
-      server.addHandler(router, (inj, req) -> new W3CCommandHandler(router));
+      server.addRoute(Routes.matching(router).using(router).decorateWith(W3CCommandHandler.class));
       server.start();
     };
   }

diff --git a/java/server/src/org/openqa/selenium/grid/distributor/httpd/DistributorServer.java b/java/server/src/org/openqa/selenium/grid/distributor/httpd/DistributorServer.java
@@ -36,6 +36,7 @@
 import org.openqa.selenium.grid.server.HelpFlags;
 import org.openqa.selenium.grid.server.Server;
 import org.openqa.selenium.grid.server.W3CCommandHandler;
+import org.openqa.selenium.grid.web.Routes;
 
 @AutoService(CliCommand.class)
 public class DistributorServer implements CliCommand {
@@ -86,7 +87,10 @@ public Executable configure(String... args) {
       BaseServerOptions serverOptions = new BaseServerOptions(config);
 
       Server<?> server = new BaseServer<>(serverOptions);
-      server.addHandler(distributor, (inj, req) -> new W3CCommandHandler(distributor));
+      server.addRoute(
+          Routes.matching(distributor)
+              .using(distributor)
+              .decorateWith(W3CCommandHandler.class));
       server.start();
     };
   }

diff --git a/java/server/src/org/openqa/selenium/grid/node/httpd/NodeServer.java b/java/server/src/org/openqa/selenium/grid/node/httpd/NodeServer.java
@@ -43,6 +43,7 @@
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.SessionMapOptions;
 import org.openqa.selenium.grid.sessionmap.remote.RemoteSessionMap;
+import org.openqa.selenium.grid.web.Routes;
 import org.openqa.selenium.remote.http.HttpClient;
 
 import java.net.URL;
@@ -116,7 +117,7 @@ public Executable configure(String... args) {
           HttpClient.Factory.createDefault().createClient(distributorUrl));
 
       Server<?> server = new BaseServer<>(serverOptions);
-      server.addHandler(node, (inj, req) -> new W3CCommandHandler(node));
+      server.addRoute(Routes.matching(node).using(node).decorateWith(W3CCommandHandler.class));
       server.start();
 
       Regularly regularly = new Regularly(

diff --git a/java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java b/java/server/src/org/openqa/selenium/grid/router/httpd/RouterServer.java
@@ -42,6 +42,7 @@
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.SessionMapOptions;
 import org.openqa.selenium.grid.sessionmap.remote.RemoteSessionMap;
+import org.openqa.selenium.grid.web.Routes;
 import org.openqa.selenium.remote.http.HttpClient;
 
 import java.net.URL;
@@ -108,7 +109,7 @@ public Executable configure(String... args) {
       Router router = new Router(sessions, distributor);
 
       Server<?> server = new BaseServer<>(serverOptions);
-      server.addHandler(router, (inj, req) -> new W3CCommandHandler(router));
+      server.addRoute(Routes.matching(router).using(router).decorateWith(W3CCommandHandler.class));
       server.start();
     };
   }

diff --git a/java/server/src/org/openqa/selenium/grid/server/BaseServer.java b/java/server/src/org/openqa/selenium/grid/server/BaseServer.java
@@ -20,14 +20,13 @@
 import static java.net.HttpURLConnection.HTTP_OK;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.concurrent.TimeUnit.SECONDS;
-import static org.openqa.selenium.grid.server.Server.get;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.net.MediaType;
 
 import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.grid.web.CommandHandler;
-import org.openqa.selenium.grid.web.CompoundHandler;
+import org.openqa.selenium.grid.web.Routes;
 import org.openqa.selenium.injector.Injector;
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.net.NetworkUtils;
@@ -48,7 +47,9 @@
 import java.net.BindException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.BiFunction;
@@ -65,6 +66,7 @@ public class BaseServer<T extends BaseServer> implements Server<T> {
   private final org.seleniumhq.jetty9.server.Server server;
   private final Map<Predicate<HttpRequest>, BiFunction<Injector, HttpRequest, CommandHandler>>
       handlers;
+  private final List<Routes> routes = new ArrayList<>();
   private final ServletContextHandler servletContextHandler;
   private final Injector injector;
   private final URL url;
@@ -97,7 +99,8 @@ public BaseServer(BaseServerOptions options) {
         .register(json)
         .build();
 
-    addHandler(get("/status"), (injector, req) ->
+    addRoute(
+        Routes.get("/status").using(
         (in, out) -> {
           String value = json.toJson(ImmutableMap.of(
               "value", ImmutableMap.of(
@@ -109,7 +112,7 @@ public BaseServer(BaseServerOptions options) {
           out.setStatus(HTTP_OK);
 
           out.setContent(value.getBytes(UTF_8));
-        });
+        }).build());
 
     this.servletContextHandler = new ServletContextHandler(ServletContextHandler.SECURITY);
     ConstraintSecurityHandler
@@ -170,13 +173,12 @@ public void addServlet(Servlet servlet, String pathSpec) {
   }
 
   @Override
-  public void addHandler(
-      Predicate<HttpRequest> selector,
-      BiFunction<Injector, HttpRequest, CommandHandler> handler) {
+  public void addRoute(Routes route) {
     if (server.isRunning()) {
       throw new IllegalStateException("You may not add a handler to a running server");
     }
-    handlers.put(Objects.requireNonNull(selector), Objects.requireNonNull(handler));
+
+    this.routes.add(route);
   }
 
   public boolean isStarted() {
@@ -186,9 +188,16 @@ public boolean isStarted() {
   @Override
   public T start() {
     try {
-      CommandHandler delegate = new CompoundHandler(injector, handlers);
-      W3CCommandHandler handler = new W3CCommandHandler(delegate);
-      addServlet(new CommandHandlerServlet(handler), "/*");
+      // If there are no routes, we've done something terribly wrong.
+      if (routes.isEmpty()) {
+        throw new IllegalStateException("There must be at least one route specified");
+      }
+      Routes first = routes.remove(0);
+      Routes routes = Routes.combine(first, this.routes.toArray(new Routes[0]))
+          .decorateWith(W3CCommandHandler.class)
+          .build();
+
+      addServlet(new CommandHandlerServlet(routes), "/*");
 
       server.start();
 

diff --git a/java/server/src/org/openqa/selenium/grid/server/CommandHandlerServlet.java b/java/server/src/org/openqa/selenium/grid/server/CommandHandlerServlet.java
@@ -17,23 +17,39 @@
 
 package org.openqa.selenium.grid.server;
 
+import static org.openqa.selenium.grid.web.Routes.combine;
+
+import org.openqa.selenium.UnsupportedCommandException;
 import org.openqa.selenium.grid.web.CommandHandler;
+import org.openqa.selenium.grid.web.Routes;
+import org.openqa.selenium.injector.Injector;
+import org.openqa.selenium.json.Json;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
 
 import java.io.IOException;
 import java.util.Objects;
+import java.util.Optional;
 
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 class CommandHandlerServlet extends HttpServlet {
 
-  private final CommandHandler handler;
+  private final Routes routes;
+  private final Injector injector;
 
-  public CommandHandlerServlet(CommandHandler handler) {
-    this.handler = Objects.requireNonNull(handler);
+  public CommandHandlerServlet(Routes routes) {
+    Objects.requireNonNull(routes);
+    this.routes = combine(routes)
+        .fallbackTo(
+            new W3CCommandHandler(
+                (req, res) -> {
+                  throw new UnsupportedCommandException(String.format(
+                      "Unknown command: (%s) %s", req.getMethod(), req.getUri()));
+                })).build();
+    this.injector = Injector.builder().register(new Json()).build();
   }
 
   @Override
@@ -42,6 +58,11 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws
     HttpRequest request = new ServletRequestWrappingHttpRequest(req);
     HttpResponse response = new ServletResponseWrappingHttpResponse(resp);
 
-    handler.execute(request, response);
+    Optional<CommandHandler> possibleMatch = routes.match(injector, request);
+    if (possibleMatch.isPresent()) {
+      possibleMatch.get().execute(request, response);
+    } else {
+      throw new IllegalStateException("It should not be possible to get here");
+    }
   }
 }
diff --git a/java/server/src/org/openqa/selenium/grid/server/Server.java b/java/server/src/org/openqa/selenium/grid/server/Server.java
@@ -17,22 +17,13 @@
 
 package org.openqa.selenium.grid.server;
 
-import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
-import static org.openqa.selenium.remote.http.HttpMethod.GET;
-import static org.openqa.selenium.remote.http.HttpMethod.POST;
-
 import org.openqa.selenium.grid.component.HasLifecycle;
 import org.openqa.selenium.grid.web.CommandHandler;
-import org.openqa.selenium.grid.web.UrlTemplate;
-import org.openqa.selenium.injector.Injector;
-import org.openqa.selenium.remote.SessionId;
-import org.openqa.selenium.remote.http.HttpMethod;
-import org.openqa.selenium.remote.http.HttpRequest;
+import org.openqa.selenium.grid.web.Route;
+import org.openqa.selenium.grid.web.Routes;
 
 import java.net.URL;
 import java.util.Objects;
-import java.util.function.BiFunction;
-import java.util.function.Predicate;
 
 import javax.servlet.Servlet;
 
@@ -56,47 +47,12 @@ public interface Server<T extends Server> extends HasLifecycle<T> {
   @Deprecated
   void addServlet(Servlet servlet, String pathSpec);
 
-  void addHandler(
-      Predicate<HttpRequest> selector,
-      BiFunction<Injector, HttpRequest, CommandHandler> handler);
-
-  URL getUrl();
-
-  default void addHandler(
-      HttpMethod method,
-      String urlTemplate,
-      BiFunction<Injector, HttpRequest, CommandHandler> handler) {
-    Objects.requireNonNull(method, "Method must be set");
-
-    UrlTemplate template = new UrlTemplate(urlTemplate);
+  void addRoute(Routes route);
 
-    addHandler(
-        req -> method == req.getMethod() && template.match(req.getUri()) != null,
-        (inj, req) -> {
-          UrlTemplate.Match match = template.match(req.getUri());
-          if (match != null && match.getParameters().get("sessionId") != null) {
-            inj = Injector.builder()
-                .parent(inj)
-                .register(new SessionId(match.getParameters().get("sessionId")))
-                .build();
-          }
-          return handler.apply(inj, req);
-        }
-    );
+  default void addRoute(Route<?> route) {
+    Objects.requireNonNull(route, "Route must not be null");
+    addRoute(route.build());
   }
 
-  static Predicate<HttpRequest> delete(String template) {
-    UrlTemplate urlTemplate = new UrlTemplate(template);
-    return req -> DELETE == req.getMethod() && urlTemplate.match(req.getUri()) != null;
-  }
-
-  static Predicate<HttpRequest> get(String template) {
-    UrlTemplate urlTemplate = new UrlTemplate(template);
-    return req -> GET == req.getMethod() && urlTemplate.match(req.getUri()) != null;
-  }
-
-  static Predicate<HttpRequest> post(String template) {
-    UrlTemplate urlTemplate = new UrlTemplate(template);
-    return req -> POST == req.getMethod() && urlTemplate.match(req.getUri()) != null;
-  }
+  URL getUrl();
 }
diff --git a/java/server/src/org/openqa/selenium/grid/sessionmap/httpd/SessionMapServer.java b/java/server/src/org/openqa/selenium/grid/sessionmap/httpd/SessionMapServer.java
@@ -17,6 +17,8 @@
 
 package org.openqa.selenium.grid.sessionmap.httpd;
 
+import static org.openqa.selenium.grid.web.Routes.matching;
+
 import com.google.auto.service.AutoService;
 
 import com.beust.jcommander.JCommander;
@@ -36,6 +38,7 @@
 import org.openqa.selenium.grid.server.W3CCommandHandler;
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
+import org.openqa.selenium.grid.web.Routes;
 
 @AutoService(CliCommand.class)
 public class SessionMapServer implements CliCommand {
@@ -86,7 +89,7 @@ public Executable configure(String... args) {
       BaseServerOptions serverOptions = new BaseServerOptions(config);
 
       Server<?> server = new BaseServer<>(serverOptions);
-      server.addHandler(sessions, (inj, req) -> new W3CCommandHandler(sessions));
+      server.addRoute(matching(sessions).using(sessions).decorateWith(W3CCommandHandler.class));
       server.start();
     };
   }