Make HttpClient an HttpHandler

SeleniumHQ · Jul 4, 2019 · 2c24f30 · 2c24f30
1 parent bf74b7d
commit 2c24f30
Show file tree

Hide file tree

Showing 15 changed files with 127 additions and 176 deletions.
diff --git a/java/client/src/org/openqa/selenium/remote/http/HttpClient.java b/java/client/src/org/openqa/selenium/remote/http/HttpClient.java
@@ -17,23 +17,22 @@
 
 package org.openqa.selenium.remote.http;
 
-import static com.google.common.base.Preconditions.checkArgument;
-import static java.util.Objects.requireNonNull;
-
 import org.openqa.selenium.BuildInfo;
 import org.openqa.selenium.Platform;
 
-import java.io.IOException;
 import java.net.Proxy;
 import java.net.URL;
 import java.time.Duration;
 import java.util.Locale;
 import java.util.Objects;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.Objects.requireNonNull;
+
 /**
  * Defines a simple client for making HTTP requests.
  */
-public interface HttpClient {
+public interface HttpClient extends HttpHandler {
 
   String USER_AGENT = String.format(
       "selenium/%s (java %s)",
@@ -42,15 +41,6 @@ public interface HttpClient {
        Platform.getCurrent().toString().toLowerCase(Locale.US) :
        Platform.getCurrent().family().toString().toLowerCase(Locale.US)));
 
-  /**
-   * Executes the given request, following any redirects if necessary.
-   *
-   * @param request the request to execute.
-   * @return the final response.
-   * @throws IOException if an I/O error occurs.
-   */
-  HttpResponse execute(HttpRequest request) throws IOException;
-
   WebSocket openSocket(HttpRequest request, WebSocket.Listener listener);
 
   interface Factory {

diff --git a/java/client/src/org/openqa/selenium/remote/http/HttpHandler.java b/java/client/src/org/openqa/selenium/remote/http/HttpHandler.java
@@ -17,10 +17,12 @@
 
 package org.openqa.selenium.remote.http;
 
+import java.io.UncheckedIOException;
+
 @FunctionalInterface
 public interface HttpHandler {
 
-  HttpResponse execute(HttpRequest req);
+  HttpResponse execute(HttpRequest req) throws UncheckedIOException;
 
   default HttpHandler with(Filter filter) {
     return filter.andFinally(this);

diff --git a/java/client/src/org/openqa/selenium/remote/internal/OkHttpClient.java b/java/client/src/org/openqa/selenium/remote/internal/OkHttpClient.java
@@ -20,6 +20,7 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.openqa.selenium.remote.http.Contents.bytes;
 import static org.openqa.selenium.remote.http.Contents.empty;
+import static org.openqa.selenium.remote.http.Contents.memoize;
 
 import com.google.common.base.Strings;
 
@@ -53,13 +54,18 @@ public OkHttpClient(okhttp3.OkHttpClient client, URL url) {
   }
 
   @Override
-  public HttpResponse execute(HttpRequest request) throws IOException {
+  public HttpResponse execute(HttpRequest request) throws UncheckedIOException {
     Request okHttpRequest = buildOkHttpRequest(request);
 
-    Response response = client.newCall(okHttpRequest).execute();
+    Response response;
+    try {
+      response = client.newCall(okHttpRequest).execute();
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
+    }
 
     HttpResponse toReturn = new HttpResponse();
-    toReturn.setContent(response.body() == null ? empty() : bytes(response.body().bytes()));
+    toReturn.setContent(response.body() == null ? empty() : memoize(() -> response.body().byteStream()));
     toReturn.setStatus(response.code());
     response.headers().names().forEach(
         name -> response.headers(name).forEach(value -> toReturn.addHeader(name, value)));

diff --git a/java/client/test/org/openqa/selenium/testing/drivers/GridSupplier.java b/java/client/test/org/openqa/selenium/testing/drivers/GridSupplier.java
@@ -17,8 +17,6 @@
 
 package org.openqa.selenium.testing.drivers;
 
-import static org.openqa.selenium.remote.http.Contents.string;
-
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.firefox.FirefoxOptions;
@@ -38,6 +36,8 @@
 import java.util.Map;
 import java.util.function.Supplier;
 
+import static org.openqa.selenium.remote.http.Contents.string;
+
 public class GridSupplier implements Supplier<WebDriver> {
 
   private static OutOfProcessSeleniumServer hub;
@@ -91,12 +91,7 @@ private synchronized void startServers() {
         .withTimeout(Duration.ofSeconds(30));
     wait.until(c -> {
       HttpRequest req = new HttpRequest(HttpMethod.GET, "/status");
-      HttpResponse response = null;
-      try {
-        response = c.execute(req);
-      } catch (IOException e) {
-        throw new RuntimeException(e);
-      }
+      HttpResponse response = c.execute(req);
       Map<?, ?> value = json.toType(string(response), Map.class);
 
       return ((Map<?, ?>) value.get("value")).get("ready") == Boolean.TRUE;

diff --git a/java/server/src/org/openqa/selenium/docker/Container.java b/java/server/src/org/openqa/selenium/docker/Container.java
@@ -17,27 +17,27 @@
 
 package org.openqa.selenium.docker;
 
-import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
-import static org.openqa.selenium.remote.http.HttpMethod.POST;
-
 import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.remote.http.Contents;
+import org.openqa.selenium.remote.http.HttpHandler;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
 
-import java.net.HttpURLConnection;
 import java.time.Duration;
 import java.util.Objects;
-import java.util.function.Function;
 import java.util.logging.Logger;
 
+import static java.net.HttpURLConnection.HTTP_OK;
+import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
+import static org.openqa.selenium.remote.http.HttpMethod.POST;
+
 public class Container {
 
   public static final Logger LOG = Logger.getLogger(Container.class.getName());
-  private final Function<HttpRequest, HttpResponse> client;
+  private final HttpHandler client;
   private final ContainerId id;
 
-  public Container(Function<HttpRequest, HttpResponse> client, ContainerId id) {
+  public Container(HttpHandler client, ContainerId id) {
     LOG.info("Created container " + id);
     this.client = Objects.requireNonNull(client);
     this.id = Objects.requireNonNull(id);
@@ -49,9 +49,8 @@ public ContainerId getId() {
 
   public void start() {
     LOG.info("Starting " + getId());
-    HttpResponse res = client.apply(
-        new HttpRequest(POST, String.format("/containers/%s/start", id)));
-    if (res.getStatus() != HttpURLConnection.HTTP_OK) {
+    HttpResponse res = client.execute(new HttpRequest(POST, String.format("/containers/%s/start", id)));
+    if (res.getStatus() != HTTP_OK) {
       throw new WebDriverException("Unable to start container: " + Contents.string(res));
     }
   }
@@ -63,21 +62,20 @@ public void stop(Duration timeout) {
 
     String seconds = String.valueOf(timeout.toMillis() / 1000);
 
-    HttpRequest request = new HttpRequest(POST, String.format("/containers/%s/stop", id));
-    request.addQueryParameter("t", seconds);
+    HttpRequest request = new HttpRequest(POST, String.format("/containers/%s/stop", id))
+      .addQueryParameter("t", seconds);
 
-    HttpResponse res = client.apply(request);
-    if (res.getStatus() != HttpURLConnection.HTTP_OK) {
+    HttpResponse res = client.execute(request);
+    if (res.getStatus() != HTTP_OK) {
       throw new WebDriverException("Unable to stop container: " + Contents.string(res));
     }
   }
 
   public void delete() {
     LOG.info("Removing " + getId());
 
-    HttpRequest request = new HttpRequest(DELETE, "/containers/" + id);
-    HttpResponse res = client.apply(request);
-    if (res.getStatus() != HttpURLConnection.HTTP_OK) {
+    HttpResponse res = client.execute(new HttpRequest(DELETE, "/containers/" + id));
+    if (res.getStatus() != HTTP_OK) {
       LOG.warning("Unable to delete container");
     }
   }

diff --git a/java/server/src/org/openqa/selenium/docker/Docker.java b/java/server/src/org/openqa/selenium/docker/Docker.java
@@ -17,71 +17,62 @@
 
 package org.openqa.selenium.docker;
 
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import static org.openqa.selenium.json.Json.MAP_TYPE;
-import static org.openqa.selenium.remote.http.Contents.string;
-import static org.openqa.selenium.remote.http.Contents.utf8String;
-import static org.openqa.selenium.remote.http.HttpMethod.GET;
-import static org.openqa.selenium.remote.http.HttpMethod.POST;
-
 import com.google.common.reflect.TypeToken;
-
 import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.json.JsonException;
 import org.openqa.selenium.json.JsonOutput;
-import org.openqa.selenium.remote.http.Contents;
-import org.openqa.selenium.remote.http.HttpClient;
+import org.openqa.selenium.remote.http.HttpHandler;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
 
-import java.io.IOException;
-import java.io.UncheckedIOException;
-import java.net.HttpURLConnection;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.logging.Logger;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static java.net.HttpURLConnection.HTTP_OK;
+import static org.openqa.selenium.json.Json.MAP_TYPE;
+import static org.openqa.selenium.remote.http.Contents.string;
+import static org.openqa.selenium.remote.http.Contents.utf8String;
+import static org.openqa.selenium.remote.http.HttpMethod.GET;
+import static org.openqa.selenium.remote.http.HttpMethod.POST;
+
 public class Docker {
 
   private static final Logger LOG = Logger.getLogger(Docker.class.getName());
   private static final Json JSON = new Json();
 
-  private final Function<HttpRequest, HttpResponse> client;
+  private final HttpHandler client;
 
-  public Docker(HttpClient client) {
+  public Docker(HttpHandler client) {
     Objects.requireNonNull(client, "Docker HTTP client must be set.");
 
     this.client = req -> {
-      try {
-        HttpResponse resp = client.execute(req);
-
-        if (resp.getStatus() < 200 && resp.getStatus() > 200) {
-          String value = string(resp);
-          try {
-            Object obj = JSON.toType(value, Object.class);
-            if (obj instanceof Map) {
-              Map<?, ?> map = (Map<?, ?>) obj;
-              String message = map.get("message") instanceof String ?
-                               (String) map.get("message") :
-                               value;
-              throw new RuntimeException(message);
-            }
-
-            throw new RuntimeException(value);
-          } catch (JsonException e) {
-            throw new RuntimeException(value);
+      HttpResponse resp = client.execute(req);
+
+      if (resp.getStatus() < 200 && resp.getStatus() > 200) {
+        String value = string(resp);
+        try {
+          Object obj = JSON.toType(value, Object.class);
+          if (obj instanceof Map) {
+            Map<?, ?> map = (Map<?, ?>) obj;
+            String message = map.get("message") instanceof String ?
+                             (String) map.get("message") :
+                             value;
+            throw new RuntimeException(message);
           }
-        }
 
-        return resp;
-      } catch (IOException e) {
-        throw new UncheckedIOException(e);
+          throw new RuntimeException(value);
+        } catch (JsonException e) {
+          throw new RuntimeException(value);
+        }
       }
+
+      return resp;
     };
   }
 
@@ -93,12 +84,12 @@ public Image pull(String name, String tag) {
 
     LOG.info(String.format("Pulling %s:%s", name, tag));
 
-    HttpRequest request = new HttpRequest(POST, "/images/create");
-    request.addQueryParameter("fromImage", name);
-    request.addQueryParameter("tag", tag);
+    HttpRequest request = new HttpRequest(POST, "/images/create")
+        .addQueryParameter("fromImage", name)
+        .addQueryParameter("tag", tag);
 
-    HttpResponse res = client.apply(request);
-    if (res.getStatus() != HttpURLConnection.HTTP_OK) {
+    HttpResponse res = client.execute(request);
+    if (res.getStatus() != HTTP_OK) {
       throw new WebDriverException("Unable to pull container: " + name);
     }
 
@@ -111,7 +102,7 @@ public Image pull(String name, String tag) {
 
   public List<Image> listImages() {
     LOG.fine("Listing images");
-    HttpResponse response = client.apply(new HttpRequest(GET, "/images/json"));
+    HttpResponse response = client.execute(new HttpRequest(GET, "/images/json"));
 
     List<ImageSummary> images =
         JSON.toType(string(response), new TypeToken<List<ImageSummary>>() {}.getType());
@@ -144,7 +135,7 @@ public Container create(ContainerInfo info) {
     HttpRequest request = new HttpRequest(POST, "/containers/create");
     request.setContent(utf8String(json));
 
-    HttpResponse response = client.apply(request);
+    HttpResponse response = client.execute(request);
 
     Map<String, Object> toRead = JSON.toType(string(response), MAP_TYPE);