Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow per-request base URIs in HttpClient #7922

Merged
merged 7 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ enum Method {
DELETE;
}

HttpRequest newRequest();
default HttpRequest newRequest() {
return newRequest(getBaseUri());
}

HttpRequest newRequest(URI baseUri);

static Builder builder() {
return new HttpClientBuilderImpl();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,6 @@ public HttpClient.Builder addTracing() {

@Override
public HttpClient build() {
HttpUtils.checkArgument(
baseUri != null, "Cannot construct Http client. Must have a non-null uri");
HttpUtils.checkArgument(
mapper != null, "Cannot construct Http client. Must have a non-null object mapper");
if (sslContext == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
*/
package org.projectnessie.client.http;

import static java.util.Objects.requireNonNull;
import static org.projectnessie.client.http.impl.HttpUtils.checkArgument;
import static org.projectnessie.client.http.impl.HttpUtils.isHttpUri;

import java.net.URI;
import org.projectnessie.client.http.HttpClient.Method;
import org.projectnessie.client.http.impl.HttpHeaders;
import org.projectnessie.client.http.impl.HttpRuntimeConfig;
Expand All @@ -30,8 +35,11 @@ public abstract class HttpRequest
protected String contentsType = "application/json; charset=utf-8";
protected String accept = "application/json; charset=utf-8";

protected HttpRequest(HttpRuntimeConfig config) {
this.uriBuilder = new UriBuilder(config.getBaseUri());
protected HttpRequest(HttpRuntimeConfig config, URI baseUri) {
requireNonNull(baseUri, "Base URI cannot be null");
checkArgument(
isHttpUri(baseUri), "Base URI must be a valid http or https address: %s", baseUri);
this.uriBuilder = new UriBuilder(baseUri);
this.config = config;

int clientSpec = config.getClientSpec();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.fasterxml.jackson.databind.ObjectWriter;
import java.io.IOException;
import java.io.OutputStream;
import java.net.URI;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Map;
Expand All @@ -43,8 +44,8 @@ public abstract class BaseHttpRequest extends HttpRequest {
private static final TypeReference<Map<String, Object>> MAP_TYPE =
new TypeReference<Map<String, Object>>() {};

protected BaseHttpRequest(HttpRuntimeConfig config) {
super(config);
protected BaseHttpRequest(HttpRuntimeConfig config, URI baseUri) {
super(config, baseUri);
}

protected boolean prepareRequest(RequestContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@ static ImmutableHttpRuntimeConfig.Builder builder() {
@Value.Check
default void check() {
URI baseUri = getBaseUri();
if (!"http".equals(baseUri.getScheme()) && !"https".equals(baseUri.getScheme())) {
if (baseUri != null && !HttpUtils.isHttpUri(baseUri)) {
throw new IllegalArgumentException(
String.format(
"Cannot start http client. %s must be a valid http or https address", baseUri));
}
}

@Nullable
@jakarta.annotation.Nullable
URI getBaseUri();

ObjectMapper getMapper();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.errorprone.annotations.FormatMethod;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URLConnection;
import java.net.URLDecoder;
import java.util.HashMap;
Expand Down Expand Up @@ -97,4 +98,8 @@ public static Map<String, String> parseQueryString(String query) {
}
return params;
}

public static boolean isHttpUri(URI uri) {
return "http".equalsIgnoreCase(uri.getScheme()) || "https".equalsIgnoreCase(uri.getScheme());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ public JavaHttpClient(HttpRuntimeConfig config) {
}

@Override
public HttpRequest newRequest() {
return new JavaRequest(this.config, (req, handler) -> client.send(req, handler));
public HttpRequest newRequest(URI baseUri) {
return new JavaRequest(this.config, baseUri, client::send);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ HttpResponse<T> send(HttpRequest request, HttpResponse.BodyHandler<T> responseBo

private final HttpExchange<InputStream> exchange;

JavaRequest(HttpRuntimeConfig config, HttpExchange<InputStream> exchange) {
super(config);
JavaRequest(HttpRuntimeConfig config, URI baseUri, HttpExchange<InputStream> exchange) {
super(config, baseUri);
this.exchange = exchange;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public UrlConnectionClient(HttpRuntimeConfig config) {
}

@Override
public HttpRequest newRequest() {
return new UrlConnectionRequest(config);
public HttpRequest newRequest(URI baseUri) {
return new UrlConnectionRequest(config, baseUri);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@
/** Class to hold an ongoing HTTP request and its parameters/filters. */
final class UrlConnectionRequest extends BaseHttpRequest {

UrlConnectionRequest(HttpRuntimeConfig config) {
super(config);
UrlConnectionRequest(HttpRuntimeConfig config, URI baseUri) {
super(config, baseUri);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,41 @@ void testGet() throws Exception {
}
}

@Test
void testPerRequestBaseUri() throws Exception {
ExampleBean inputBean = new ExampleBean("x", 1, NOW);
HttpTestServer.RequestHandler handler =
(req, resp) -> {
soft.assertThat(req.getMethod()).isEqualTo("GET");
String response = MAPPER.writeValueAsString(inputBean);
writeResponseBody(resp, response);
};
try (HttpTestServer server = new HttpTestServer(handler)) {
try (HttpClient client = createClient(null, b -> {})) {
ExampleBean bean = client.newRequest(server.getUri()).get().readEntity(ExampleBean.class);
soft.assertThat(bean).isEqualTo(inputBean);
}
}
}

@Test
void testNullPerRequestBaseUri() {
try (HttpClient client = createClient(null, b -> {})) {
assertThatThrownBy(() -> client.newRequest(null))
.isInstanceOf(NullPointerException.class)
.hasMessage("Base URI cannot be null");
}
}

@Test
void testInvalidPerRequestBaseUri() {
try (HttpClient client = createClient(null, b -> {})) {
assertThatThrownBy(() -> client.newRequest(URI.create("file:///foo/bar/baz")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Base URI must be a valid http or https address");
}
}

@Test
void testPut() throws Exception {
ExampleBean inputBean = new ExampleBean("x", 1, NOW);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,24 +69,6 @@ void testIncompatibleAuthProvider() {
.hasMessage("HttpClientBuilder only accepts instances of HttpAuthentication");
}

@Test
void testNullUri() {
assertThatThrownBy(
() ->
createClientBuilderFromSystemSettings()
.withUri((URI) null)
.build(NessieApiV1.class))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot construct Http client. Must have a non-null uri");
}

@Test
void testNoUri() {
assertThatThrownBy(() -> createClientBuilderFromSystemSettings().build(NessieApiV1.class))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot construct Http client. Must have a non-null uri");
}

@Test
void testInvalidUriScheme() {
assertThatThrownBy(
Expand Down