[java] Reworking NettyClient to start a client per session and close the client when quit is called. This also allows to use different ClientConfig-s for different client instances

Author: barancev

java/client/src/org/openqa/selenium/remote/HttpCommandExecutor.java

@@ -195,6 +195,7 @@ public Response execute(Command command) throws IOException {
         }
       }
       if (QUIT.equals(command.getName())) {
+        client.close();
         httpClientFactory.cleanupIdleClients();
       }
       return response;

java/client/src/org/openqa/selenium/remote/http/HttpClient.java

@@ -17,6 +17,7 @@
 
 package org.openqa.selenium.remote.http;
 
+import java.io.IOException;
 import java.net.URL;
 import java.util.ServiceLoader;
 import java.util.Set;
@@ -34,6 +35,8 @@ public interface HttpClient extends HttpHandler {
 
   WebSocket openSocket(HttpRequest request, WebSocket.Listener listener);
 
+  default void close() throws IOException {}
+
   interface Factory {
 
     /**

java/client/src/org/openqa/selenium/remote/http/netty/NettyClient.java

@@ -32,27 +32,42 @@
 import org.openqa.selenium.remote.http.HttpResponse;
 import org.openqa.selenium.remote.http.WebSocket;
 
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.function.BiFunction;
 import java.io.IOException;
+import java.util.function.BiFunction;
 
 public class NettyClient implements HttpClient {
 
-  private static final AsyncHttpClient httpClient =
-      Dsl.asyncHttpClient(
-          new DefaultAsyncHttpClientConfig.Builder()
-              .setThreadFactory(new DefaultThreadFactory("AsyncHttpClient", true))
-              .setUseInsecureTrustManager(true)
-              .setAggregateWebSocketFrameFragments(true)
-              .setWebSocketMaxBufferSize(Integer.MAX_VALUE)
-              .setWebSocketMaxFrameSize(Integer.MAX_VALUE));
-
+  private final ClientConfig config;
+  private final AsyncHttpClient client;
   private final HttpHandler handler;
-  private BiFunction<HttpRequest, WebSocket.Listener, WebSocket> toWebSocket;
+  private final BiFunction<HttpRequest, WebSocket.Listener, WebSocket> toWebSocket;
 
-  private NettyClient(HttpHandler handler, BiFunction<HttpRequest, WebSocket.Listener, WebSocket> toWebSocket) {
-    this.handler = Require.nonNull("Handler", handler);
-    this.toWebSocket = Require.nonNull("WebSocket creation function", toWebSocket);
+  private NettyClient(ClientConfig config) {
+    this.config = Require.nonNull("HTTP client config", config);
+    this.client = createHttpClient(config);
+    this.handler = new NettyHttpHandler(config, this.client).with(config.filter());
+    this.toWebSocket = NettyWebSocket.create(config, this.client);
+  }
+
+  private AsyncHttpClient createHttpClient(ClientConfig config) {
+    DefaultAsyncHttpClientConfig.Builder builder =
+      new DefaultAsyncHttpClientConfig.Builder()
+        .setThreadFactory(new DefaultThreadFactory("AsyncHttpClient", true))
+        .setUseInsecureTrustManager(true)
+        .setAggregateWebSocketFrameFragments(true)
+        .setWebSocketMaxBufferSize(Integer.MAX_VALUE)
+        .setWebSocketMaxFrameSize(Integer.MAX_VALUE)
+        .setConnectTimeout((int) config.connectionTimeout().toMillis());
+
+    String info = config.baseUrl().getUserInfo();
+    if (info != null && !info.equals("")) {
+      String[] parts = info.split(":", 2);
+      String user = parts[0];
+      String pass = parts.length > 1 ? parts[1] : null;
+      builder.setRealm(Dsl.basicAuthRealm(user, pass).setUsePreemptiveAuth(true).build());
+    }
+
+    return Dsl.asyncHttpClient(builder);
   }
 
   @Override
@@ -73,29 +88,18 @@ public HttpClient with(Filter filter) {
     Require.nonNull("Filter", filter);
 
     // TODO: We should probably ensure that websocket requests are run through the filter.
-    return new NettyClient(handler.with(filter), toWebSocket);
+    return new NettyClient(config.withFilter(filter));
+  }
+
+  @Override
+  public void close() throws IOException {
+    client.close();
   }
 
   @AutoService(HttpClient.Factory.class)
   @HttpClientName("netty")
   public static class Factory implements HttpClient.Factory {
 
-    private static final AtomicBoolean addedHook = new AtomicBoolean();
-
-    public Factory() {
-      if (!addedHook.get()) {
-        Runtime.getRuntime().addShutdownHook(new Thread(this::callAsyncClientShutdown));
-        addedHook.set(true);
-      }
-    }
-
-    private void callAsyncClientShutdown() {
-      try {
-        httpClient.close();
-      } catch (IOException ignore) {
-      }
-    }
-
     @Override
     public HttpClient createClient(ClientConfig config) {
       Require.nonNull("Client config", config);
@@ -104,7 +108,7 @@ public HttpClient createClient(ClientConfig config) {
         return new NettyDomainSocketClient(config);
       }
 
-      return new NettyClient(new NettyHttpHandler(config, httpClient).with(config.filter()), NettyWebSocket.create(config, httpClient));
+      return new NettyClient(config);
     }
   }
 }