Add a Routable interface and make the old Route an HttpHandler

SeleniumHQ · Jul 4, 2019 · 48c45b5 · 48c45b5
1 parent bd03257
commit 48c45b5
Show file tree

Hide file tree

Showing 11 changed files with 286 additions and 153 deletions.
diff --git a/java/client/src/org/openqa/selenium/remote/http/Routable.java b/java/client/src/org/openqa/selenium/remote/http/Routable.java
@@ -0,0 +1,24 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.remote.http;
+
+public interface Routable {
+
+  boolean matches(HttpRequest req);
+
+}
diff --git a/java/client/src/org/openqa/selenium/remote/http/Route.java b/java/client/src/org/openqa/selenium/remote/http/Route.java
@@ -17,14 +17,6 @@
 
 package org.openqa.selenium.remote.http;
 
-import static com.google.common.base.Preconditions.checkArgument;
-import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
-import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
-import static org.openqa.selenium.remote.http.Contents.utf8String;
-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 com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -38,18 +30,31 @@
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
-public abstract class Route implements HttpHandler, Predicate<HttpRequest> {
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static org.openqa.selenium.remote.http.Contents.utf8String;
+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;
+
+public abstract class Route implements HttpHandler, Routable {
 
   public HttpHandler fallbackTo(Supplier<HttpHandler> handler) {
     Objects.requireNonNull(handler, "Handler to use must be set.");
     return req -> {
-      if (test(req)) {
+      if (matches(req)) {
         return Route.this.execute(req);
       }
       return Objects.requireNonNull(handler.get(), "Handler to use must be set.").execute(req);
     };
   }
 
+  public static PredicatedConfig matching(Predicate<HttpRequest> predicate) {
+    Objects.requireNonNull(predicate, "Predicate to use must be set.");
+    return new PredicatedConfig(predicate);
+  }
+
   public static TemplatizedRouteConfig delete(String template) {
     Objects.requireNonNull(template, "URL template to use must be set.");
     UrlTemplate urlTemplate = new UrlTemplate(template);
@@ -131,7 +136,7 @@ private TemplatizedRoute(
     }
 
     @Override
-    public boolean test(HttpRequest request) {
+    public boolean matches(HttpRequest request) {
       return predicate.test(request);
     }
 
@@ -183,7 +188,7 @@ public static class NestedRouteConfig {
 
     private final String prefix;
 
-    public NestedRouteConfig(String prefix) {
+    private NestedRouteConfig(String prefix) {
       this.prefix = Objects.requireNonNull(prefix, "Prefix must be set.");
     }
 
@@ -204,8 +209,8 @@ private NestedRoute(String prefix, Route route) {
     }
 
     @Override
-    public boolean test(HttpRequest request) {
-      return request.getUri().startsWith(prefix) && route.test(transform(request));
+    public boolean matches(HttpRequest request) {
+      return request.getUri().startsWith(prefix) && route.matches(transform(request));
     }
 
     @Override
@@ -246,14 +251,14 @@ public CombinedRoute(Stream<Route> routes) {
     }
 
     @Override
-    public boolean test(HttpRequest request) {
-      return allRoutes.stream().anyMatch(route -> route.test(request));
+    public boolean matches(HttpRequest request) {
+      return allRoutes.stream().anyMatch(route -> route.matches(request));
     }
 
     @Override
     public HttpResponse execute(HttpRequest request) {
       return allRoutes.stream()
-          .filter(route -> route.test(request))
+          .filter(route -> route.matches(request))
           .findFirst()
           .map(route -> (HttpHandler) route)
           .orElse(req -> new HttpResponse()
@@ -262,4 +267,42 @@ public HttpResponse execute(HttpRequest request) {
           .execute(request);
     }
   }
+
+  public static class PredicatedConfig {
+    private final Predicate<HttpRequest> predicate;
+
+    private PredicatedConfig(Predicate<HttpRequest> predicate) {
+        this.predicate = Objects.requireNonNull(predicate);
+    }
+
+    public Route to(Supplier<HttpHandler> handler) {
+      Objects.requireNonNull(handler, "Handler supplier must be set.");
+      return new PredicatedRoute(predicate, handler);
+    }
+  }
+
+  private static class PredicatedRoute extends Route {
+
+    private final Predicate<HttpRequest> predicate;
+    private final Supplier<HttpHandler> supplier;
+
+    public PredicatedRoute(Predicate<HttpRequest> predicate, Supplier<HttpHandler> supplier) {
+      this.predicate = Objects.requireNonNull(predicate);
+      this.supplier = Objects.requireNonNull(supplier);
+    }
+
+    @Override
+    public boolean matches(HttpRequest httpRequest) {
+      return predicate.test(httpRequest);
+    }
+
+    @Override
+    public HttpResponse execute(HttpRequest req) {
+      HttpHandler handler = supplier.get();
+      if (handler == null) {
+        throw new IllegalStateException("No handler available.");
+      }
+      return handler.execute(req);
+    }
+  }
 }
diff --git a/java/client/test/org/openqa/selenium/remote/http/RouteTest.java b/java/client/test/org/openqa/selenium/remote/http/RouteTest.java
@@ -37,7 +37,7 @@ public void shouldNotRouteUnhandledUrls() {
         new HttpResponse().setContent(utf8String("Hello, World!"))
     );
 
-    Assertions.assertThat(route.test(new HttpRequest(GET, "/greeting"))).isFalse();
+    Assertions.assertThat(route.matches(new HttpRequest(GET, "/greeting"))).isFalse();
   }
 
   @Test
@@ -47,7 +47,7 @@ public void shouldRouteSimplePaths() {
     );
 
     HttpRequest request = new HttpRequest(GET, "/hello");
-    Assertions.assertThat(route.test(request)).isTrue();
+    Assertions.assertThat(route.matches(request)).isTrue();
 
     HttpResponse res = route.execute(request);
     assertThat(string(res)).isEqualTo("Hello, World!");
@@ -59,7 +59,7 @@ public void shouldAllowRoutesToBeUrlTemplates() {
         new HttpResponse().setContent(utf8String(String.format("Hello, %s!", params.get("name")))));
 
     HttpRequest request = new HttpRequest(POST, "/greeting/cheese");
-    Assertions.assertThat(route.test(request)).isTrue();
+    Assertions.assertThat(route.matches(request)).isTrue();
 
     HttpResponse res = route.execute(request);
     assertThat(string(res)).isEqualTo("Hello, cheese!");
@@ -71,7 +71,7 @@ public void shouldAllowRoutesToBePrefixed() {
         .to(Route.get("/type").to(() -> req -> new HttpResponse().setContent(utf8String("brie"))));
 
     HttpRequest request = new HttpRequest(GET, "/cheese/type");
-    Assertions.assertThat(route.test(request)).isTrue();
+    Assertions.assertThat(route.matches(request)).isTrue();
     HttpResponse res = route.execute(request);
     assertThat(string(res)).isEqualTo("brie");
   }
@@ -84,12 +84,12 @@ public void shouldAllowRoutesToBeNested() {
                 params -> req -> new HttpResponse().setContent(Contents.utf8String(params.get("kind"))))));
 
     HttpRequest good = new HttpRequest(GET, "/cheese/favourite/is/stilton");
-    Assertions.assertThat(route.test(good)).isTrue();
+    Assertions.assertThat(route.matches(good)).isTrue();
     HttpResponse response = route.execute(good);
     assertThat(string(response)).isEqualTo("stilton");
 
     HttpRequest bad = new HttpRequest(GET, "/cheese/favourite/not-here");
-    Assertions.assertThat(route.test(bad)).isFalse();
+    Assertions.assertThat(route.matches(bad)).isFalse();
   }
 
   @Test
@@ -99,7 +99,7 @@ public void nestedRoutesShouldStripPrefixFromRequest() {
                 .get("/type").to(() -> req -> new HttpResponse().setContent(Contents.utf8String(req.getUri()))));
 
     HttpRequest request = new HttpRequest(GET, "/cheese/type");
-    Assertions.assertThat(route.test(request)).isTrue();
+    Assertions.assertThat(route.matches(request)).isTrue();
     HttpResponse res = route.execute(request);
     assertThat(string(res)).isEqualTo("/type");
   }
@@ -112,12 +112,12 @@ public void itShouldBePossibleToCombineRoutes() {
             () -> req -> new HttpResponse().setContent(utf8String("gouda"))));
 
     HttpRequest greet = new HttpRequest(GET, "/hello");
-    Assertions.assertThat(route.test(greet)).isTrue();
+    Assertions.assertThat(route.matches(greet)).isTrue();
     HttpResponse response = route.execute(greet);
     assertThat(string(response)).isEqualTo("world");
 
     HttpRequest cheese = new HttpRequest(POST, "/cheese");
-    Assertions.assertThat(route.test(cheese)).isTrue();
+    Assertions.assertThat(route.matches(cheese)).isTrue();
     response = route.execute(cheese);
     assertThat(string(response)).isEqualTo("gouda");
   }

diff --git a/java/server/src/org/openqa/selenium/grid/server/AddWebDriverSpecHeaders.java b/java/server/src/org/openqa/selenium/grid/server/AddWebDriverSpecHeaders.java
@@ -0,0 +1,45 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.grid.server;
+
+import org.openqa.selenium.remote.http.Filter;
+import org.openqa.selenium.remote.http.HttpHandler;
+import org.openqa.selenium.remote.http.HttpResponse;
+
+import static com.google.common.net.MediaType.JSON_UTF_8;
+
+public class AddWebDriverSpecHeaders implements Filter {
+  @Override
+  public HttpHandler apply(HttpHandler next) {
+    return req -> {
+      HttpResponse res = next.execute(req);
+      if (res == null) {
+        return res;
+      }
+
+      if (res.getHeader("Content-Type") == null) {
+        res.addHeader("Content-Type", JSON_UTF_8.toString());
+      }
+      if (res.getHeader("Cache-Control") == null) {
+        res.addHeader("Cache-Control", "none");
+      }
+
+      return res;
+    };
+  }
+}
diff --git a/java/server/src/org/openqa/selenium/grid/server/BaseServer.java b/java/server/src/org/openqa/selenium/grid/server/BaseServer.java
@@ -17,18 +17,10 @@
 
 package org.openqa.selenium.grid.server;
 
-import static java.net.HttpURLConnection.HTTP_OK;
-import static java.util.concurrent.TimeUnit.SECONDS;
-import static org.openqa.selenium.remote.http.Contents.utf8String;
-
-import com.google.common.collect.ImmutableMap;
-import com.google.common.net.MediaType;
-
 import org.openqa.selenium.WebDriverException;
-import org.openqa.selenium.grid.web.Routes;
-import org.openqa.selenium.json.Json;
 import org.openqa.selenium.net.NetworkUtils;
 import org.openqa.selenium.net.PortProber;
+import org.openqa.selenium.remote.http.HttpHandler;
 import org.seleniumhq.jetty9.security.ConstraintMapping;
 import org.seleniumhq.jetty9.security.ConstraintSecurityHandler;
 import org.seleniumhq.jetty9.server.Connector;
@@ -42,26 +34,25 @@
 import org.seleniumhq.jetty9.util.security.Constraint;
 import org.seleniumhq.jetty9.util.thread.QueuedThreadPool;
 
+import javax.servlet.Servlet;
 import java.io.UncheckedIOException;
 import java.net.BindException;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Objects;
 import java.util.logging.Logger;
 
-import javax.servlet.Servlet;
+import static java.util.concurrent.TimeUnit.SECONDS;
 
 public class BaseServer<T extends BaseServer> implements Server<T> {
 
   private static final Logger LOG = Logger.getLogger(BaseServer.class.getName());
   private static final int MAX_SHUTDOWN_RETRIES = 8;
 
   private final org.seleniumhq.jetty9.server.Server server;
-  private final List<Routes> routes = new ArrayList<>();
   private final ServletContextHandler servletContextHandler;
   private final URL url;
+  private HttpHandler handler;
 
   public BaseServer(BaseServerOptions options) {
     int port = options.getPort() == 0 ? PortProber.findFreePort() : options.getPort();
@@ -84,22 +75,6 @@ public BaseServer(BaseServerOptions options) {
     this.server = new org.seleniumhq.jetty9.server.Server(
         new QueuedThreadPool(options.getMaxServerThreads()));
 
-    Json json = new Json();
-    addRoute(
-        Routes.get("/status").using(
-        (in, out) -> {
-          String value = json.toJson(ImmutableMap.of(
-              "value", ImmutableMap.of(
-                  "ready", false,
-                  "message", "Stub server without handlers")));
-
-          out.setHeader("Content-Type", MediaType.JSON_UTF_8.toString());
-          out.setHeader("Cache-Control", "none");
-          out.setStatus(HTTP_OK);
-
-          out.setContent(utf8String(value));
-        }).build());
-
     this.servletContextHandler = new ServletContextHandler(ServletContextHandler.SECURITY);
     ConstraintSecurityHandler
         securityHandler =
@@ -159,12 +134,12 @@ public void addServlet(Servlet servlet, String pathSpec) {
   }
 
   @Override
-  public void addRoute(Routes route) {
+  public T setHandler(HttpHandler handler) {
     if (server.isRunning()) {
       throw new IllegalStateException("You may not add a handler to a running server");
     }
-
-    this.routes.add(route);
+    this.handler = Objects.requireNonNull(handler, "Handler to use must be set.");
+    return (T) this;
   }
 
   @Override
@@ -176,15 +151,11 @@ public boolean isStarted() {
   public T start() {
     try {
       // If there are no routes, we've done something terribly wrong.
-      if (routes.isEmpty()) {
+      if (handler == null) {
         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::new)
-          .build();
 
-      addServlet(new CommandHandlerServlet(routes), "/*");
+      addServlet(new HttpHandlerServlet(handler.with(new WrapExceptions().andThen(new AddWebDriverSpecHeaders()))), "/*");
 
       server.start();