[java] removed usage of FileBackedOutputStream in the client (#13308)

* [java] removed usage of FileBackedOutputStream in the client * [java] removed usage of FileBackedOutputStream in the client --------- Co-authored-by: Diego Molina <[email protected]>
SeleniumHQ · Mar 26, 2024 · 55e7a53 · 55e7a53
1 parent 4cfe983
commit 55e7a53
Show file tree

Hide file tree

Showing 13 changed files with 137 additions and 198 deletions.
diff --git a/java/src/org/openqa/selenium/devtools/idealized/Network.java b/java/src/org/openqa/selenium/devtools/idealized/Network.java
@@ -21,8 +21,6 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.logging.Level.WARNING;
 
-import java.io.ByteArrayInputStream;
-import java.io.InputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Base64;
@@ -293,13 +291,13 @@ protected HttpResponse createHttpResponse(
       String body,
       Boolean bodyIsBase64Encoded,
       List<Map.Entry<String, String>> headers) {
-    Supplier<InputStream> content;
+    Contents.Supplier content;
 
     if (body == null) {
       content = Contents.empty();
     } else if (bodyIsBase64Encoded != null && bodyIsBase64Encoded) {
       byte[] decoded = Base64.getDecoder().decode(body);
-      content = () -> new ByteArrayInputStream(decoded);
+      content = Contents.bytes(decoded);
     } else {
       content = Contents.string(body, UTF_8);
     }

diff --git a/java/src/org/openqa/selenium/grid/data/SessionRequest.java b/java/src/org/openqa/selenium/grid/data/SessionRequest.java
@@ -41,7 +41,6 @@
 import org.openqa.selenium.json.TypeToken;
 import org.openqa.selenium.remote.Dialect;
 import org.openqa.selenium.remote.NewSessionPayload;
-import org.openqa.selenium.remote.http.Contents;
 import org.openqa.selenium.remote.http.HttpRequest;
 
 public class SessionRequest {
@@ -61,7 +60,7 @@ public SessionRequest(RequestId requestId, HttpRequest request, Instant enqueued
     this.enqueued = Require.nonNull("Enqueued time", enqueued);
     Require.nonNull("Request", request);
 
-    try (NewSessionPayload payload = NewSessionPayload.create(Contents.reader(request))) {
+    try (NewSessionPayload payload = NewSessionPayload.create(request.getContent())) {
       desiredCapabilities =
           payload.stream()
               .filter(capabilities -> !capabilities.asMap().isEmpty())

diff --git a/java/src/org/openqa/selenium/netty/server/RequestConverter.java b/java/src/org/openqa/selenium/netty/server/RequestConverter.java
@@ -44,6 +44,7 @@
 import java.util.List;
 import java.util.logging.Logger;
 import org.openqa.selenium.internal.Debug;
+import org.openqa.selenium.remote.http.Contents;
 import org.openqa.selenium.remote.http.HttpMethod;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
@@ -55,6 +56,7 @@ class RequestConverter extends SimpleChannelInboundHandler<HttpObject> {
   private static final List<io.netty.handler.codec.http.HttpMethod> SUPPORTED_METHODS =
       Arrays.asList(DELETE, GET, POST, OPTIONS);
   private volatile FileBackedOutputStream buffer;
+  private volatile int length;
   private volatile HttpRequest request;
 
   @Override
@@ -91,6 +93,7 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex
           AttributeKey.HTTP_FLAVOR.getKey(), nettyRequest.protocolVersion().majorVersion());
 
       buffer = null;
+      length = -1;
     }
 
     if (msg instanceof HttpContent) {
@@ -100,10 +103,12 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex
       if (nBytes > 0) {
         if (buffer == null) {
           buffer = new FileBackedOutputStream(3 * 1024 * 1024, true);
+          length = 0;
         }
 
         try {
           buf.readBytes(buffer, nBytes);
+          length += nBytes;
         } finally {
           buf.release();
         }
@@ -114,29 +119,31 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex
 
         if (buffer != null) {
           ByteSource source = buffer.asByteSource();
+          int len = length;
 
           request.setContent(
-              () -> {
-                try {
-                  return source.openBufferedStream();
-                } catch (IOException e) {
-                  throw new UncheckedIOException(e);
+              new Contents.Supplier() {
+                @Override
+                public InputStream get() {
+                  try {
+                    return source.openBufferedStream();
+                  } catch (IOException e) {
+                    throw new UncheckedIOException(e);
+                  }
+                }
+
+                @Override
+                public int length() {
+                  return len;
+                }
+
+                @Override
+                public void close() throws IOException {
+                  buffer.reset();
                 }
               });
         } else {
-          request.setContent(
-              () ->
-                  new InputStream() {
-                    @Override
-                    public int read() throws IOException {
-                      return -1;
-                    }
-
-                    @Override
-                    public int read(byte[] b, int off, int len) throws IOException {
-                      return -1;
-                    }
-                  });
+          request.setContent(Contents.empty());
         }
 
         ctx.fireChannelRead(request);

diff --git a/java/src/org/openqa/selenium/remote/NewSessionPayload.java b/java/src/org/openqa/selenium/remote/NewSessionPayload.java
@@ -21,15 +21,10 @@
 import static org.openqa.selenium.json.Json.LIST_OF_MAPS_TYPE;
 import static org.openqa.selenium.json.Json.MAP_TYPE;
 
-import com.google.common.io.FileBackedOutputStream;
 import java.io.Closeable;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.io.OutputStreamWriter;
 import java.io.Reader;
-import java.io.StringReader;
 import java.io.UncheckedIOException;
-import java.io.Writer;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -50,32 +45,19 @@
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.json.JsonInput;
 import org.openqa.selenium.json.JsonOutput;
+import org.openqa.selenium.remote.http.Contents;
 
 public class NewSessionPayload implements Closeable {
 
   private static final Dialect DEFAULT_DIALECT = Dialect.W3C;
   private static final Predicate<String> ACCEPTED_W3C_PATTERNS = new AcceptedW3CCapabilityKeys();
 
   private final Json json = new Json();
-  private final FileBackedOutputStream backingStore;
+  private final Contents.Supplier supplier;
   private final Set<Dialect> dialects;
 
-  private NewSessionPayload(Reader source) {
-    // Dedicate up to 10% of all RAM or 20% of available RAM (whichever is smaller) to storing this
-    // payload.
-    int threshold =
-        (int)
-            Math.min(
-                Integer.MAX_VALUE,
-                Math.min(
-                    Runtime.getRuntime().freeMemory() / 5, Runtime.getRuntime().maxMemory() / 10));
-
-    backingStore = new FileBackedOutputStream(threshold);
-    try (Writer writer = new OutputStreamWriter(backingStore, UTF_8)) {
-      source.transferTo(writer);
-    } catch (IOException e) {
-      throw new UncheckedIOException(e);
-    }
+  private NewSessionPayload(Contents.Supplier supplier) {
+    this.supplier = supplier;
 
     Set<Dialect> dialects = new LinkedHashSet<>();
     try {
@@ -89,12 +71,6 @@ private NewSessionPayload(Reader source) {
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }
-
-    try {
-      source.close();
-    } catch (IOException e) {
-      // Ignore
-    }
   }
 
   public static NewSessionPayload create(Capabilities caps) {
@@ -120,12 +96,15 @@ public static NewSessionPayload create(Map<String, ?> source) {
     Require.precondition(
         source.containsKey("capabilities"), "New session payload must contain capabilities");
 
-    String json = new Json().toJson(Require.nonNull("Payload", source));
-    return new NewSessionPayload(new StringReader(json));
+    return new NewSessionPayload(Contents.asJson(Require.nonNull("Payload", source)));
+  }
+
+  public static NewSessionPayload create(Contents.Supplier supplier) {
+    return new NewSessionPayload(supplier);
   }
 
-  public static NewSessionPayload create(Reader source) {
-    return new NewSessionPayload(source);
+  public Contents.Supplier getSupplier() {
+    return supplier;
   }
 
   private void validate() throws IOException {
@@ -213,8 +192,7 @@ public void writeTo(Appendable appendable) throws IOException {
   }
 
   private void writeMetaData(JsonOutput out) throws IOException {
-    try (Reader reader =
-            new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8);
+    try (Reader reader = Contents.reader(supplier, UTF_8);
         JsonInput input = json.newInput(reader)) {
       input.beginObject();
       while (input.hasNext()) {
@@ -253,8 +231,7 @@ public Set<Dialect> getDownstreamDialects() {
   public Map<String, Object> getMetadata() {
     Set<String> ignoredMetadataKeys = Set.of("capabilities");
 
-    try (Reader reader =
-            new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8);
+    try (Reader reader = Contents.reader(supplier, UTF_8);
         JsonInput input = json.newInput(reader)) {
       Map<String, Object> toReturn = new LinkedHashMap<>();
 
@@ -284,7 +261,7 @@ public Map<String, Object> getMetadata() {
   @Override
   public void close() {
     try {
-      backingStore.reset();
+      supplier.close();
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }
@@ -324,8 +301,7 @@ private Stream<Map<String, Object>> getW3C() throws IOException {
   }
 
   private boolean isW3C() throws IOException {
-    try (Reader reader =
-            new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8);
+    try (Reader reader = Contents.reader(supplier, UTF_8);
         JsonInput input = json.newInput(reader)) {
       input.beginObject();
       while (input.hasNext()) {
@@ -341,8 +317,7 @@ private boolean isW3C() throws IOException {
   }
 
   private Map<String, Object> getAlwaysMatch() throws IOException {
-    try (Reader reader =
-            new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8);
+    try (Reader reader = Contents.reader(supplier, UTF_8);
         JsonInput input = json.newInput(reader)) {
       input.beginObject();
       while (input.hasNext()) {
@@ -367,8 +342,7 @@ private Map<String, Object> getAlwaysMatch() throws IOException {
   }
 
   private Collection<Map<String, Object>> getFirstMatches() throws IOException {
-    try (Reader reader =
-            new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8);
+    try (Reader reader = Contents.reader(supplier, UTF_8);
         JsonInput input = json.newInput(reader)) {
       input.beginObject();
       while (input.hasNext()) {

diff --git a/java/src/org/openqa/selenium/remote/ProtocolHandshake.java b/java/src/org/openqa/selenium/remote/ProtocolHandshake.java
@@ -17,23 +17,16 @@
 
 package org.openqa.selenium.remote;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Collections.singleton;
 import static org.openqa.selenium.json.Json.JSON_UTF_8;
 import static org.openqa.selenium.remote.CapabilityType.PROXY;
 import static org.openqa.selenium.remote.http.Contents.string;
 
-import com.google.common.io.CountingOutputStream;
-import com.google.common.io.FileBackedOutputStream;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStreamWriter;
-import java.io.Writer;
 import java.util.Collection;
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.Function;
-import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Stream;
@@ -45,6 +38,7 @@
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.json.JsonException;
+import org.openqa.selenium.remote.http.Contents;
 import org.openqa.selenium.remote.http.HttpHandler;
 import org.openqa.selenium.remote.http.HttpHeader;
 import org.openqa.selenium.remote.http.HttpMethod;
@@ -78,36 +72,17 @@ public Result createSession(HttpHandler client, Command command) throws IOExcept
 
   public Either<SessionNotCreatedException, Result> createSession(
       HttpHandler client, NewSessionPayload payload) throws IOException {
-    int threshold = (int) Math.min(Runtime.getRuntime().freeMemory() / 10, Integer.MAX_VALUE);
-    FileBackedOutputStream os = new FileBackedOutputStream(threshold, true);
-
-    try (CountingOutputStream counter = new CountingOutputStream(os);
-        Writer writer = new OutputStreamWriter(counter, UTF_8)) {
-      payload.writeTo(writer);
-      Supplier<InputStream> contentSupplier =
-          () -> {
-            try {
-              return os.asByteSource().openBufferedStream();
-            } catch (IOException e) {
-              throw new RuntimeException(e);
-            }
-          };
-      return createSession(client, contentSupplier, counter.getCount());
-    }
+    return createSession(client, payload.getSupplier());
   }
 
   private Either<SessionNotCreatedException, Result> createSession(
-      HttpHandler client, Supplier<InputStream> contentSupplier, long size) {
+      HttpHandler client, Contents.Supplier contentSupplier) {
     // Create the http request and send it
     HttpRequest request = new HttpRequest(HttpMethod.POST, "/session");
 
     HttpResponse response;
     long start = System.currentTimeMillis();
 
-    // Setting the CONTENT_LENGTH will allow a http client implementation not to read the data in
-    // memory. Usually the payload is small and buffering it to memory is okay, except for a new
-    // session e.g. with profiles.
-    request.setHeader(HttpHeader.ContentLength.getName(), String.valueOf(size));
     request.setHeader(HttpHeader.ContentType.getName(), JSON_UTF_8);
     request.setContent(contentSupplier);
 

diff --git a/java/src/org/openqa/selenium/remote/http/BUILD.bazel b/java/src/org/openqa/selenium/remote/http/BUILD.bazel
@@ -17,7 +17,6 @@ java_export(
         "//java:auto-service",
         "//java/src/org/openqa/selenium:core",
         "//java/src/org/openqa/selenium/json",
-        artifact("com.google.guava:guava"),
         artifact("dev.failsafe:failsafe"),
     ],
 )