Replace Jersey in orchestrator with apache, remove jaxrx_client_utils

5 files changed, 92 insertions, 4 deletions

diff --git a/http-client/pom.xml b/http-client/pom.xml
index a452353cb8a..ebe83a80903 100644
--- a/http-client/pom.xml
+++ b/http-client/pom.xml
@@ -52,6 +52,12 @@
             <version>2.27.2</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.yahoo.vespa</groupId>
+            <artifactId>testutil</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
 
diff --git a/http-client/src/main/java/ai/vespa/hosted/client/AbstractHttpClient.java b/http-client/src/main/java/ai/vespa/hosted/client/AbstractHttpClient.java
index 21a6c3c9cb9..2055cd6d74a 100644
--- a/http-client/src/main/java/ai/vespa/hosted/client/AbstractHttpClient.java
+++ b/http-client/src/main/java/ai/vespa/hosted/client/AbstractHttpClient.java
@@ -1,9 +1,12 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package ai.vespa.hosted.client;
 
+import ai.vespa.hosted.client.HttpClient.RequestBuilder;
 import ai.vespa.http.HttpURL;
 import ai.vespa.http.HttpURL.Path;
 import ai.vespa.http.HttpURL.Query;
+import com.yahoo.concurrent.UncheckedTimeoutException;
+import com.yahoo.time.TimeBudget;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
@@ -16,12 +19,15 @@ import org.apache.hc.core5.http.ParseException;
 import org.apache.hc.core5.http.io.entity.EntityUtils;
 import org.apache.hc.core5.http.io.entity.HttpEntities;
 import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
+import org.apache.hc.core5.util.Timeout;
 
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.net.URI;
 import java.time.Duration;
+import java.time.Instant;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.function.BiFunction;
 import java.util.function.Function;
@@ -58,8 +64,6 @@ public abstract class AbstractHttpClient implements HttpClient {
     private <T> T execute(RequestBuilder builder,
                           BiFunction<ClassicHttpResponse, ClassicHttpRequest, T> handler,
                           ExceptionHandler catcher) {
-        HttpClientContext context = HttpClientContext.create();
-        context.setRequestConfig(builder.config);
 
         Throwable thrown = null;
         for (URI host : builder.hosts) {
@@ -72,7 +76,7 @@ public abstract class AbstractHttpClient implements HttpClient {
             request.setEntity(builder.entity);
             try {
                 try {
-                    return handler.apply(execute(request, context), request);
+                    return handler.apply(execute(request, contextWithTimeout(builder)), request);
                 }
                 catch (IOException e) {
                     catcher.handle(e, request);
@@ -104,6 +108,29 @@ public abstract class AbstractHttpClient implements HttpClient {
         throw new IllegalStateException("No hosts to perform the request against");
     }
 
+    private HttpClientContext contextWithTimeout(RequestBuilder builder) {
+        HttpClientContext context = HttpClientContext.create();
+        RequestConfig config = builder.config;
+        if (builder.deadline != null) {
+            Optional<Duration> remaining = builder.deadline.timeLeftOrThrow();
+            if (remaining.isPresent()) {
+                config = RequestConfig.copy(config)
+                                      .setConnectTimeout(min(config.getConnectTimeout(), remaining.get()))
+                                      .setConnectionRequestTimeout(min(config.getConnectionRequestTimeout(), remaining.get()))
+                                      .setResponseTimeout(min(config.getResponseTimeout(), remaining.get()))
+                                      .build();
+            }
+        }
+        context.setRequestConfig(config);
+        return context;
+    }
+
+    // TimeBudget guarantees remaining duration is positive.
+    static Timeout min(Timeout first, Duration second) {
+        long firstMillis = first == null || first.isDisabled() ? second.toMillis() : first.toMilliseconds();
+        return Timeout.ofMilliseconds(Math.min(firstMillis, second.toMillis()));
+    }
+
     @Override
     public HttpClient.RequestBuilder send(HostStrategy hosts, Method method) {
         return new RequestBuilder(hosts, method);
@@ -120,6 +147,7 @@ public abstract class AbstractHttpClient implements HttpClient {
         private RequestConfig config = HttpClient.defaultRequestConfig;
         private ResponseVerifier verifier = HttpClient.throwOnError;
         private ExceptionHandler catcher = HttpClient.retryAll;
+        private TimeBudget deadline;
 
         private RequestBuilder(HostStrategy hosts, Method method) {
             if ( ! hosts.iterator().hasNext())
@@ -182,6 +210,12 @@ public abstract class AbstractHttpClient implements HttpClient {
         }
 
         @Override
+        public HttpClient.RequestBuilder deadline(TimeBudget deadline) {
+            this.deadline = requireNonNull(deadline);
+            return this;
+        }
+
+        @Override
         public RequestBuilder config(RequestConfig config) {
             this.config = requireNonNull(config);
             return this;
diff --git a/http-client/src/main/java/ai/vespa/hosted/client/HttpClient.java b/http-client/src/main/java/ai/vespa/hosted/client/HttpClient.java
index ff29a51165c..f5805ce5b94 100644
--- a/http-client/src/main/java/ai/vespa/hosted/client/HttpClient.java
+++ b/http-client/src/main/java/ai/vespa/hosted/client/HttpClient.java
@@ -4,6 +4,7 @@ package ai.vespa.hosted.client;
 import ai.vespa.http.HttpURL;
 import ai.vespa.http.HttpURL.Path;
 import ai.vespa.http.HttpURL.Query;
+import com.yahoo.time.TimeBudget;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
@@ -19,6 +20,7 @@ import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URI;
 import java.time.Duration;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -100,6 +102,12 @@ public interface HttpClient extends Closeable {
         /** Overrides the default socket read timeout of the request. {@code Duration.ZERO} gives infinite timeout. */
         RequestBuilder timeout(Duration timeout);
 
+        /**
+         * Pseudo-deadline for the request, including retries.
+         * Pseudo- because it only ensures request timeouts are low enough to honour the deadline, but nothing else.
+         */
+        RequestBuilder deadline(TimeBudget deadline);
+
         /** Overrides the default request config of the request. */
         RequestBuilder config(RequestConfig config);
 
diff --git a/http-client/src/main/java/ai/vespa/hosted/client/MockHttpClient.java b/http-client/src/main/java/ai/vespa/hosted/client/MockHttpClient.java
index 09f618f70cb..6c2a882f990 100644
--- a/http-client/src/main/java/ai/vespa/hosted/client/MockHttpClient.java
+++ b/http-client/src/main/java/ai/vespa/hosted/client/MockHttpClient.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package ai.vespa.hosted.client;
 
+import ai.vespa.http.HttpURL;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
@@ -8,11 +9,17 @@ import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.io.entity.HttpEntities;
 import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.function.BiFunction;
 import java.util.function.Function;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 /**
  * @author jonmv
  */
@@ -39,7 +46,7 @@ public class MockHttpClient extends AbstractHttpClient {
         expectations.add(expectation);
     }
 
-    public void expect(int status, Function<ClassicHttpRequest, String> mapper) {
+    public void expect(Function<ClassicHttpRequest, String> mapper, int status) {
         expect(request -> {
             BasicClassicHttpResponse response = new BasicClassicHttpResponse(status);
             response.setEntity(HttpEntities.create(mapper.apply(request), ContentType.APPLICATION_JSON));
@@ -47,6 +54,22 @@ public class MockHttpClient extends AbstractHttpClient {
         });
     }
 
+    public void expect(BiFunction<HttpURL, String, String> mapper, int status) {
+        expect(request -> {
+            try {
+                BasicClassicHttpResponse response = new BasicClassicHttpResponse(status);
+                ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+                request.getEntity().writeTo(buffer);
+                response.setEntity(HttpEntities.create(mapper.apply(HttpURL.from(request.getUri()), buffer.toString(UTF_8)),
+                                                       ContentType.APPLICATION_JSON));
+                return response;
+            }
+            catch (URISyntaxException e) {
+                throw new IllegalArgumentException(e);
+            }
+        });
+    }
+
     @FunctionalInterface
     public interface Expectation {
 
diff --git a/http-client/src/test/java/ai/vespa/hosted/client/ApacheHttpClientTest.java b/http-client/src/test/java/ai/vespa/hosted/client/ApacheHttpClientTest.java
index 647da68ab07..271b8a11a90 100644
--- a/http-client/src/test/java/ai/vespa/hosted/client/ApacheHttpClientTest.java
+++ b/http-client/src/test/java/ai/vespa/hosted/client/ApacheHttpClientTest.java
@@ -4,6 +4,9 @@ package ai.vespa.hosted.client;
 import ai.vespa.hosted.client.HttpClient.HostStrategy;
 import ai.vespa.hosted.client.HttpClient.ResponseException;
 import com.github.tomakehurst.wiremock.http.Fault;
+import com.yahoo.concurrent.UncheckedTimeoutException;
+import com.yahoo.test.ManualClock;
+import com.yahoo.time.TimeBudget;
 import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.apache.hc.core5.http.Method;
 import org.junit.jupiter.api.AfterEach;
@@ -14,6 +17,7 @@ import org.junit.jupiter.api.extension.RegisterExtension;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.net.URI;
+import java.time.Duration;
 import java.util.List;
 
 import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
@@ -101,6 +105,19 @@ class ApacheHttpClientTest {
         server.verify(1, getRequestedFor(urlEqualTo("/")));
         server.verify(1, anyRequestedFor(anyUrl()));
         server.resetRequests();
+
+        // Timeout results in UncheckedTimeoutException
+        ManualClock clock = new ManualClock();
+        TimeBudget budget = TimeBudget.fromNow(clock, Duration.ofSeconds(1));
+        clock.advance(Duration.ofSeconds(1));
+        UncheckedTimeoutException timeout = assertThrows(UncheckedTimeoutException.class,
+                                                         () -> client.send(HostStrategy.repeating(URI.create("http://localhost:" + server.port()), 2),
+                                                                           Method.GET)
+                                                                     .deadline(budget)
+                                                                     .discard());
+        assertEquals("Time since start PT1S exceeds timeout PT1S", timeout.getMessage());
+        server.verify(0, anyRequestedFor(anyUrl()));
+        server.resetRequests();
     }
 
     @AfterEach