Skip to content

Commit

Permalink
Switch to using ClientConfig to configure the HttpClient
Browse files Browse the repository at this point in the history
  • Loading branch information
shs96c committed Jul 4, 2019
1 parent 6be6012 commit 46d3642
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 209 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

public class AddSeleniumUserAgent implements Filter {

private static final String USER_AGENT = String.format(
public static final String USER_AGENT = String.format(
"selenium/%s (java %s)",
new BuildInfo().getReleaseLabel(),
(Platform.getCurrent().family() == null ?
Expand Down
32 changes: 23 additions & 9 deletions java/client/src/org/openqa/selenium/remote/http/ClientConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@

package org.openqa.selenium.remote.http;

import java.io.UncheckedIOException;
import java.net.MalformedURLException;
import java.net.Proxy;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.time.Duration;
import java.util.Objects;

import static com.google.common.base.Preconditions.checkArgument;

public class ClientConfig {

public static final AddSeleniumUserAgent DEFAULT_FILTER = new AddSeleniumUserAgent();
private static final AddSeleniumUserAgent DEFAULT_FILTER = new AddSeleniumUserAgent();
private final URI baseUri;
private final Duration connectionTimeout;
private final Duration readTimeout;
Expand All @@ -39,23 +43,25 @@ private ClientConfig(
Duration readTimeout,
Filter filters,
Proxy proxy) {
this.baseUri = Objects.requireNonNull(baseUri, "Base URI must be set.");
this.baseUri = baseUri;
this.connectionTimeout = Objects.requireNonNull(
connectionTimeout,
"Connection timeout must be set.");
this.readTimeout = Objects.requireNonNull(readTimeout, "Connection timeout must be set.");
this.filters = Objects.requireNonNull(filters, "Filters must be set.");
this.proxy = proxy;

checkArgument(!readTimeout.isNegative(), "Read time out cannot be negative");
checkArgument(!connectionTimeout.isNegative(), "Connection time out cannot be negative");
}

public static ClientConfig defaultConfig(URI baseUri) {
Objects.requireNonNull(baseUri, "Base URI to use must be set.");
public static ClientConfig defaultConfig() {
return new ClientConfig(
baseUri,
Duration.ofMinutes(2),
Duration.ofHours(3),
new AddSeleniumUserAgent(),
null);
null,
Duration.ofMinutes(2),
Duration.ofHours(3),
new AddSeleniumUserAgent(),
null);
}

public ClientConfig baseUri(URI baseUri) {
Expand All @@ -76,6 +82,14 @@ public URI baseUri() {
return baseUri;
}

public URL baseUrl() {
try {
return baseUri().toURL();
} catch (MalformedURLException e) {
throw new UncheckedIOException(e);
}
}

public ClientConfig connectionTimeout(Duration timeout) {
Objects.requireNonNull(timeout, "Connection timeout must be set.");
return new ClientConfig(baseUri, timeout, readTimeout, filters, proxy);
Expand Down
75 changes: 7 additions & 68 deletions java/client/src/org/openqa/selenium/remote/http/HttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,16 @@

package org.openqa.selenium.remote.http;

import org.openqa.selenium.BuildInfo;
import org.openqa.selenium.Platform;

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;
import static org.openqa.selenium.remote.http.ClientConfig.defaultConfig;

/**
* Defines a simple client for making HTTP requests.
*/
public interface HttpClient extends HttpHandler {

String USER_AGENT = String.format(
"selenium/%s (java %s)",
new BuildInfo().getReleaseLabel(),
(Platform.getCurrent().family() == null ?
Platform.getCurrent().toString().toLowerCase(Locale.US) :
Platform.getCurrent().family().toString().toLowerCase(Locale.US)));

WebSocket openSocket(HttpRequest request, WebSocket.Listener listener);

interface Factory {
Expand All @@ -65,70 +51,23 @@ static Factory createDefault() {
}
}

/**
* By default {@link #createClient(URL)} will pick sensible defaults for the {@link HttpClient}
* to use, but if more control is needed, the {@link Builder} gives access to this.
*/
Builder builder();

/**
* Creates a HTTP client that will send requests to the given URL.
*
* @param url URL The base URL for requests.
*/
default HttpClient createClient(URL url) {
return builder().createClient(url);
}

/**
* Closes idle clients.
*/
void cleanupIdleClients();
}

abstract class Builder {

protected Duration connectionTimeout = Duration.ofMinutes(2);
protected Duration readTimeout = Duration.ofHours(3);
protected Proxy proxy = null;

/**
* Set the connection timeout to a given value. Note that setting to negative values is not
* allowed, and that a timeout of {@code 0} results in unspecified behaviour.
*/
public Builder connectionTimeout(Duration duration) {
requireNonNull(duration, "Connection time out must be set");
checkArgument(!duration.isNegative(), "Connection time out cannot be negative");

this.connectionTimeout = duration;

return this;
Objects.requireNonNull(url, "URL to use as base URL must be set.");
return createClient(defaultConfig().baseUrl(url));
}

/**
* Set the read timeout to a given value. Note that setting to negative values is not
* allowed, and that a timeout of {@code 0} results in unspecified behaviour.
*/
public Builder readTimeout(Duration duration) {
requireNonNull(duration, "Read time out must be set");
checkArgument(!duration.isNegative(), "Read time out cannot be negative");

this.readTimeout = duration;

return this;
}
HttpClient createClient(ClientConfig config);

/**
* Set the {@link Proxy} that should be used by the {@link HttpClient} (<b>not</b> the
* {@link org.openqa.selenium.WebDriver} instance!). If this is not set, then an implementation
* specific method for selecting a proxy will be used.
* Closes idle clients.
*/
public Builder proxy(Proxy proxy) {
this.proxy = Objects.requireNonNull(proxy, "Proxy must be set");

return this;
default void cleanupIdleClients() {
// do nothing by default.
}

public abstract HttpClient createClient(URL url);
}
}
103 changes: 45 additions & 58 deletions java/client/src/org/openqa/selenium/remote/internal/OkHttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,38 +17,27 @@

package org.openqa.selenium.remote.internal;

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;

import org.openqa.selenium.remote.http.HttpClient;
import org.openqa.selenium.remote.http.HttpRequest;
import org.openqa.selenium.remote.http.HttpResponse;
import okhttp3.*;
import org.openqa.selenium.remote.http.WebSocket;

import okhttp3.ConnectionPool;
import okhttp3.Credentials;
import okhttp3.HttpUrl;
import okhttp3.MediaType;
import okhttp3.Request;
import okhttp3.RequestBody;
import okhttp3.Response;
import org.openqa.selenium.remote.http.*;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URL;
import java.util.Objects;
import java.util.Optional;

import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.openqa.selenium.remote.http.AddSeleniumUserAgent.USER_AGENT;
import static org.openqa.selenium.remote.http.Contents.*;

public class OkHttpClient implements HttpClient {

private final okhttp3.OkHttpClient client;
private final URL baseUrl;

public OkHttpClient(okhttp3.OkHttpClient client, URL url) {
private OkHttpClient(okhttp3.OkHttpClient client, URL url) {
this.client = client;
this.baseUrl = url;
}
Expand Down Expand Up @@ -148,48 +137,46 @@ public static class Factory implements HttpClient.Factory {
private final ConnectionPool pool = new ConnectionPool();

@Override
public Builder builder() {
return new Builder() {
@Override
public HttpClient createClient(URL url) {
okhttp3.OkHttpClient.Builder client = new okhttp3.OkHttpClient.Builder()
.connectionPool(pool)
.followRedirects(true)
.followSslRedirects(true)
.proxy(proxy)
.readTimeout(readTimeout.toMillis(), MILLISECONDS)
.connectTimeout(connectionTimeout.toMillis(), MILLISECONDS);

String info = url.getUserInfo();
if (!Strings.isNullOrEmpty(info)) {
String[] parts = info.split(":", 2);
String user = parts[0];
String pass = parts.length > 1 ? parts[1] : null;

String credentials = Credentials.basic(user, pass);

client.authenticator((route, response) -> {
if (response.request().header("Authorization") != null) {
return null; // Give up, we've already attempted to authenticate.
}

return response.request().newBuilder()
.header("Authorization", credentials)
.build();
});
public HttpClient createClient(ClientConfig config) {
Objects.requireNonNull(config, "Client config to use must be set.");

okhttp3.OkHttpClient.Builder client = new okhttp3.OkHttpClient.Builder()
.connectionPool(pool)
.followRedirects(true)
.followSslRedirects(true)
.proxy(config.proxy())
.readTimeout(config.readTimeout().toMillis(), MILLISECONDS)
.connectTimeout(config.connectionTimeout().toMillis(), MILLISECONDS);

URL baseUrl = config.baseUrl();
String info = baseUrl.getUserInfo();
if (!Strings.isNullOrEmpty(info)) {
String[] parts = info.split(":", 2);
String user = parts[0];
String pass = parts.length > 1 ? parts[1] : null;

String credentials = Credentials.basic(user, pass);

client.authenticator((route, response) -> {
if (response.request().header("Authorization") != null) {
return null; // Give up, we've already attempted to authenticate.
}

client.addNetworkInterceptor(chain -> {
Request request = chain.request();
Response response = chain.proceed(request);
return response.code() == 408
? response.newBuilder().code(500).message("Server-Side Timeout").build()
: response;
});

return new OkHttpClient(client.build(), url);
}
};
return response.request().newBuilder()
.header("Authorization", credentials)
.build();
});
}

client.addNetworkInterceptor(chain -> {
Request request = chain.request();
Response response = chain.proceed(request);
return response.code() == 408
? response.newBuilder().code(500).message("Server-Side Timeout").build()
: response;
});

return new OkHttpClient(client.build(), baseUrl);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,53 +17,30 @@

package org.openqa.selenium.remote.http.okhttp;

import org.openqa.selenium.remote.http.ClientConfig;
import org.openqa.selenium.remote.http.HttpClient;
import org.openqa.selenium.remote.http.HttpRequest;
import org.openqa.selenium.remote.http.HttpResponse;
import org.openqa.selenium.remote.http.WebSocket;
import org.openqa.selenium.remote.internal.HttpClientTestBase;

import java.net.URISyntaxException;
import java.net.URL;

public class OkHandlerTest extends HttpClientTestBase {

@Override
protected HttpClient.Factory createFactory() {
return new HttpClient.Factory() {
@Override
public HttpClient createClient(URL url) {
ClientConfig config = null;
try {
config = ClientConfig.defaultConfig(url.toURI());
} catch (URISyntaxException e) {
throw new RuntimeException(e);
}
OkHandler okHandler = new OkHandler(config);

return new HttpClient() {
@Override
public HttpResponse execute(HttpRequest request) {
return okHandler.execute(request);
}

@Override
public WebSocket openSocket(HttpRequest request, WebSocket.Listener listener) {
throw new UnsupportedOperationException("openSocket");
}
};
}
return config -> {
OkHandler okHandler = new OkHandler(config);

@Override
public HttpClient.Builder builder() {
throw new UnsupportedOperationException("builder");
}

@Override
public void cleanupIdleClients() {
return new HttpClient() {
@Override
public HttpResponse execute(HttpRequest request) {
return okHandler.execute(request);
}

}
@Override
public WebSocket openSocket(HttpRequest request, WebSocket.Listener listener) {
throw new UnsupportedOperationException("openSocket");
}
};
};
}
}

0 comments on commit 46d3642

Please sign in to comment.