From 79da68fd191d02f90616cd0251fff2acd87ec1ce Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 12 Apr 2021 20:45:44 +0200 Subject: Revert "Jonmv/apache hc5 client" --- configserver-client/pom.xml | 8 ++- .../hosted/client/AbstractConfigServerClient.java | 83 +++++++++++----------- .../ai/vespa/hosted/client/ConfigServerClient.java | 19 ++--- 3 files changed, 53 insertions(+), 57 deletions(-) (limited to 'configserver-client') diff --git a/configserver-client/pom.xml b/configserver-client/pom.xml index 0a29ba003f4..cee6f6c067f 100644 --- a/configserver-client/pom.xml +++ b/configserver-client/pom.xml @@ -55,7 +55,13 @@ org.junit.jupiter - junit-jupiter + junit-jupiter-engine + test + + + com.yahoo.vespa + testutil + ${project.version} test diff --git a/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java b/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java index e11f0db7194..0ee9e320259 100644 --- a/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java +++ b/configserver-client/src/main/java/ai/vespa/hosted/client/AbstractConfigServerClient.java @@ -52,7 +52,7 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { protected abstract ClassicHttpResponse execute(ClassicHttpRequest request, HttpClientContext context) throws IOException; /** Executes the given request with response/error handling and retries. */ - private T execute(RequestBuilder builder, Function handler, Function catcher) { + private T execute(RequestBuilder builder, BiFunction handler) { HttpClientContext context = HttpClientContext.create(); context.setRequestConfig(builder.config); @@ -62,10 +62,10 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { request.setEntity(builder.entity); try { try { - return handler.apply(execute(request, context)); + return handler.apply(execute(request, context), null); } catch (IOException e) { - return catcher.apply(e); + return handler.apply(null, e); } } catch (RetryException e) { @@ -133,7 +133,7 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { } @Override - public RequestBuilder at(List pathSegments) { + public RequestBuilder at(String... pathSegments) { uriBuilder.setPathSegments(requireNonNull(pathSegments)); return this; } @@ -150,19 +150,19 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { } @Override - public RequestBuilder parameters(List pairs) { - if (pairs.size() % 2 != 0) + public RequestBuilder parameters(String... pairs) { + if (pairs.length % 2 != 0) throw new IllegalArgumentException("Must supply parameter key/values in pairs"); - for (int i = 0; i < pairs.size(); ) - uriBuilder.setParameter(pairs.get(i++), pairs.get(i++)); + for (int i = 0; i < pairs.length; ) + uriBuilder.setParameter(pairs[i++], pairs[i++]); return this; } @Override public RequestBuilder timeout(Duration timeout) { - return config(RequestConfig.copy(config) + return config(RequestConfig.copy(defaultRequestConfig) .setResponseTimeout(timeout.toMillis(), TimeUnit.MILLISECONDS) .build()); } @@ -175,8 +175,8 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { } @Override - public T handle(Function handler, Function catcher) throws UncheckedIOException { - return execute(this, requireNonNull(handler), requireNonNull(catcher)); + public T handle(BiFunction handler) throws UncheckedIOException { + return execute(this, requireNonNull(handler)); } @Override @@ -210,38 +210,37 @@ public abstract class AbstractConfigServerClient implements ConfigServerClient { /** Returns the mapped body, if successful, retrying any IOException. The caller must close the body stream. */ private T mapIfSuccess(Function mapper) { - return handle(response -> { - try { - InputStream body = response.getEntity() != null ? response.getEntity().getContent() - : InputStream.nullInputStream(); - if (response.getCode() >= HttpStatus.SC_REDIRECTION) - throw readException(body.readAllBytes()); - - return mapper.apply(new ForwardingInputStream(body) { - @Override - public void close() throws IOException { - super.close(); - response.close(); - } - }); + return handle((response, ioException) -> { + if (response != null) { + try { + InputStream body = response.getEntity() != null ? response.getEntity().getContent() + : InputStream.nullInputStream(); + if (response.getCode() >= HttpStatus.SC_REDIRECTION) + throw readException(body.readAllBytes()); + + return mapper.apply(new ForwardingInputStream(body) { + @Override + public void close() throws IOException { + super.close(); + response.close(); } - catch (IOException | RuntimeException | Error e) { - try { - response.close(); - } - catch (IOException f) { - e.addSuppressed(f); - } - if (e instanceof IOException) - throw new RetryException((IOException) e); - else - sneakyThrow(e); - throw new AssertionError("Should not happen"); - } - }, - ioException -> { - throw new RetryException(ioException); - }); + }); + } + catch (IOException | RuntimeException | Error e) { + try { + response.close(); + } + catch (IOException f) { + e.addSuppressed(f); + } + if (e instanceof IOException) + ioException = (IOException) e; + else + sneakyThrow(e); + } + } + throw new RetryException(ioException); + }); } } diff --git a/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java b/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java index 61b7edea0cc..234dbe9ee06 100644 --- a/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java +++ b/configserver-client/src/main/java/ai/vespa/hosted/client/ConfigServerClient.java @@ -12,7 +12,6 @@ import java.io.UncheckedIOException; import java.net.URI; import java.time.Duration; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.function.BiFunction; @@ -34,10 +33,7 @@ public interface ConfigServerClient extends AutoCloseable { interface RequestBuilder { /** Sets the request path. */ - default RequestBuilder at(String... pathSegments) { return at(List.of(pathSegments)); } - - /** Sets the request path. */ - RequestBuilder at(List pathSegments); + RequestBuilder at(String... pathSegments); /** Sets the request body as UTF-8 application/json. */ RequestBuilder body(byte[] json); @@ -46,29 +42,24 @@ public interface ConfigServerClient extends AutoCloseable { RequestBuilder body(HttpEntity entity); /** Sets the parameter key/values for the request. Number of arguments must be even. */ - default RequestBuilder parameters(String... pairs) { - return parameters(Arrays.asList(pairs)); - } - - /** Sets the parameter key/values for the request. Number of arguments must be even. */ - RequestBuilder parameters(List pairs); + RequestBuilder parameters(String... pairs); /** Overrides the default socket read timeout of the request. {@code Duration.ZERO} gives infinite timeout. */ RequestBuilder timeout(Duration timeout); - /** Overrides the default request config of the request. */ + /** Overrides the default socket read timeout of the request. {@code null} allows infinite timeout. */ RequestBuilder config(RequestConfig config); /** * Sets custom retry/failure logic for this. *

- * Exactly one of the callbacks will be invoked, with a non-null argument. + * Exactly one of the arguments (response, exception) are non-null. * Return a value to have that returned to the caller; * throw a {@link RetryException} to have the request retried; or * throw any other unchecked exception to have this propagate out to the caller. * The caller must close the provided response, if any. */ - T handle(Function handler, Function catcher) throws UncheckedIOException; + T handle(BiFunction handler) throws UncheckedIOException; /** Sets the response body mapper for this, for successful requests. */ T read(Function mapper) throws UncheckedIOException, ConfigServerException; -- cgit v1.2.3