Skip to content

Commit 4e8e1bc

Browse files
joerg1985joerg1985
committed
[java] Filter related fixed in the NettyClient and the JdkHttpClient
1 parent 01f65d2 commit 4e8e1bc

File tree

5 files changed

+25
-57
lines changed

5 files changed

+25
-57
lines changed

‎java/src/org/openqa/selenium/remote/RemoteWebDriverBuilder.java

+10-32
Original file line numberDiff line numberDiff line change
@@ -101,31 +101,7 @@ public class RemoteWebDriverBuilder {
101101
config -> {
102102
HttpClient.Factory factory = HttpClient.Factory.createDefault();
103103
HttpClient client = factory.createClient(config);
104-
return client.with(
105-
next ->
106-
req -> {
107-
try {
108-
return client.execute(req);
109-
} finally {
110-
if (req.getMethod() == DELETE) {
111-
HttpSessionId.getSessionId(req.getUri())
112-
.ifPresent(
113-
id -> {
114-
if (("/session/" + id).equals(req.getUri())) {
115-
try {
116-
client.close();
117-
} catch (UncheckedIOException e) {
118-
LOG.log(
119-
WARNING,
120-
"Swallowing exception while closing http client",
121-
e);
122-
}
123-
factory.cleanupIdleClients();
124-
}
125-
});
126-
}
127-
}
128-
});
104+
return client.with(new CloseHttpClientFilter(factory, client));
129105
};
130106
private ClientConfig clientConfig = ClientConfig.defaultConfig();
131107
private URI remoteHost = null;
@@ -412,8 +388,7 @@ private WebDriver getRemoteDriver() {
412388
HttpHandler handler =
413389
Require.nonNull("Http handler", client)
414390
.with(
415-
new CloseHttpClientFilter(client)
416-
.andThen(new AddWebDriverSpecHeaders())
391+
new AddWebDriverSpecHeaders()
417392
.andThen(new ErrorFilter())
418393
.andThen(new DumpHttpExchangeFilter()));
419394

@@ -531,9 +506,11 @@ private NewSessionPayload getPayload() {
531506

532507
private static class CloseHttpClientFilter implements Filter {
533508

534-
private final HttpHandler client;
509+
private final HttpClient.Factory factory;
510+
private final HttpClient client;
535511

536-
CloseHttpClientFilter(HttpHandler client) {
512+
CloseHttpClientFilter(HttpClient.Factory factory, HttpClient client) {
513+
this.factory = Require.nonNull("Http client factory", factory);
537514
this.client = Require.nonNull("Http client", client);
538515
}
539516

@@ -543,16 +520,17 @@ public HttpHandler apply(HttpHandler next) {
543520
try {
544521
return next.execute(req);
545522
} finally {
546-
if (req.getMethod() == DELETE && client instanceof Closeable) {
523+
if (req.getMethod() == DELETE) {
547524
HttpSessionId.getSessionId(req.getUri())
548525
.ifPresent(
549526
id -> {
550527
if (("/session/" + id).equals(req.getUri())) {
551528
try {
552-
((Closeable) client).close();
553-
} catch (IOException e) {
529+
client.close();
530+
} catch (Exception e) {
554531
LOG.log(WARNING, "Exception swallowed while closing http client", e);
555532
}
533+
factory.cleanupIdleClients();
556534
}
557535
});
558536
}

‎java/src/org/openqa/selenium/remote/codec/w3c/W3CHttpResponseCodec.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.Map;
3434
import java.util.Optional;
3535
import java.util.function.Function;
36+
import java.util.logging.Level;
3637
import java.util.logging.Logger;
3738
import org.openqa.selenium.UnhandledAlertException;
3839
import org.openqa.selenium.WebDriverException;
@@ -74,10 +75,9 @@ public class W3CHttpResponseCodec extends AbstractHttpResponseCodec {
7475
@Override
7576
public Response decode(HttpResponse encodedResponse) {
7677
String content = string(encodedResponse).trim();
77-
LOG.fine(
78-
String.format(
79-
"Decoding response. Response code was: %d and content: %s",
80-
encodedResponse.getStatus(), content));
78+
LOG.log(Level.FINE,
79+
"Decoding response. Response code was: {0} and content: {1}",
80+
new Object[] { encodedResponse.getStatus(), content });
8181
String contentType = nullToEmpty(encodedResponse.getHeader(CONTENT_TYPE));
8282

8383
Response response = new Response();

‎java/src/org/openqa/selenium/remote/http/Contents.java

+3
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ public static <T> T fromJson(HttpMessage<?> message, Type typeOfT) {
136136
}
137137

138138
public static Supplier<InputStream> memoize(Supplier<InputStream> delegate) {
139+
if (delegate instanceof MemoizedSupplier) {
140+
return delegate;
141+
}
139142
return new MemoizedSupplier(delegate);
140143
}
141144

‎java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java

+7-12
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,12 @@
5151
import org.openqa.selenium.TimeoutException;
5252
import org.openqa.selenium.UsernameAndPassword;
5353
import org.openqa.selenium.WebDriverException;
54-
import org.openqa.selenium.remote.http.BinaryMessage;
55-
import org.openqa.selenium.remote.http.ClientConfig;
56-
import org.openqa.selenium.remote.http.CloseMessage;
57-
import org.openqa.selenium.remote.http.ConnectionFailedException;
58-
import org.openqa.selenium.remote.http.HttpClient;
59-
import org.openqa.selenium.remote.http.HttpClientName;
60-
import org.openqa.selenium.remote.http.HttpMethod;
61-
import org.openqa.selenium.remote.http.HttpRequest;
62-
import org.openqa.selenium.remote.http.HttpResponse;
63-
import org.openqa.selenium.remote.http.Message;
64-
import org.openqa.selenium.remote.http.TextMessage;
65-
import org.openqa.selenium.remote.http.WebSocket;
54+
import org.openqa.selenium.remote.http.*;
6655

6756
public class JdkHttpClient implements HttpClient {
6857
public static final Logger LOG = Logger.getLogger(JdkHttpClient.class.getName());
6958
private final JdkHttpMessages messages;
59+
private final HttpHandler handler;
7060
private java.net.http.HttpClient client;
7161
private final List<WebSocket> websockets;
7262
private final ExecutorService executorService;
@@ -78,6 +68,7 @@ public class JdkHttpClient implements HttpClient {
7868
this.messages = new JdkHttpMessages(config);
7969
this.readTimeout = config.readTimeout();
8070
this.websockets = new ArrayList<>();
71+
this.handler = config.filter().andFinally(this::execute0);
8172

8273
executorService = Executors.newCachedThreadPool();
8374

@@ -341,6 +332,10 @@ private URI getWebSocketUri(HttpRequest request) throws URISyntaxException {
341332

342333
@Override
343334
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
335+
return handler.execute(req);
336+
}
337+
338+
private HttpResponse execute0(HttpRequest req) throws UncheckedIOException {
344339
Objects.requireNonNull(req, "Request");
345340

346341
LOG.fine("Executing request: " + req);

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

+1-9
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public class NettyClient implements HttpClient {
5959

6060
private NettyClient(ClientConfig config) {
6161
this.config = Require.nonNull("HTTP client config", config);
62-
this.handler = new NettyHttpHandler(config, client).with(config.filter());
62+
this.handler = new NettyHttpHandler(config, client);
6363
this.toWebSocket = NettyWebSocket.create(config, client);
6464
}
6565

@@ -105,14 +105,6 @@ public WebSocket openSocket(HttpRequest request, WebSocket.Listener listener) {
105105
return toWebSocket.apply(request, listener);
106106
}
107107

108-
@Override
109-
public HttpClient with(Filter filter) {
110-
Require.nonNull("Filter", filter);
111-
112-
// TODO: We should probably ensure that websocket requests are run through the filter.
113-
return new NettyClient(config.withFilter(filter));
114-
}
115-
116108
@Override
117109
public void close() {
118110
// no-op -- closing the client when the JVM shuts down

0 commit comments

Comments
 (0)