Revert "Jonmv/apache hc5 client"

3 files changed, 53 insertions, 57 deletions

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 @@
 
         <dependency>
             <groupId>org.junit.jupiter</groupId>
-            <artifactId>junit-jupiter</artifactId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>com.yahoo.vespa</groupId>
+            <artifactId>testutil</artifactId>
+            <version>${project.version}</version>
             <scope>test</scope>
         </dependency>
         <dependency>
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> T execute(RequestBuilder builder, Function<ClassicHttpResponse, T> handler, Function<IOException, T> catcher) {
+    private <T> T execute(RequestBuilder builder, BiFunction<ClassicHttpResponse, IOException, T> 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<String> 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<String> 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> T handle(Function<ClassicHttpResponse, T> handler, Function<IOException, T> catcher) throws UncheckedIOException {
-            return execute(this, requireNonNull(handler), requireNonNull(catcher));
+        public <T> T handle(BiFunction<ClassicHttpResponse, IOException, T> 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> T mapIfSuccess(Function<InputStream, T> 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<String> 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<String> 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.
          * <p>
-         * 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> T handle(Function<ClassicHttpResponse, T> handler, Function<IOException, T> catcher) throws UncheckedIOException;
+        <T> T handle(BiFunction<ClassicHttpResponse, IOException, T> handler) throws UncheckedIOException;
 
         /** Sets the response body mapper for this, for successful requests. */
         <T> T read(Function<byte[], T> mapper) throws UncheckedIOException, ConfigServerException;