Handle old servers in handshake

4 files changed, 58 insertions, 17 deletions

diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java
index c70cb7cd850..bc801dabf2d 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java
@@ -172,7 +172,7 @@ public class FeedClientBuilderImpl implements FeedClientBuilder {
     }
 
     @Override
-    public FeedClientBuilder setSpeedTest(boolean enabled) {
+    public FeedClientBuilderImpl setSpeedTest(boolean enabled) {
         this.speedTest = enabled;
         return this;
     }
@@ -194,7 +194,11 @@ public class FeedClientBuilderImpl implements FeedClientBuilder {
         return this;
     }
 
-    @Override public FeedClientBuilder setProxy(URI uri) { this.proxy = uri; return this; }
+    @Override
+    public FeedClientBuilderImpl setProxy(URI uri) {
+        this.proxy = uri;
+        return this;
+    }
 
     /** Constructs instance of {@link ai.vespa.feed.client.FeedClient} from builder configuration */
     @Override
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java
index 2427b71a104..cad2da4d242 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java
@@ -51,14 +51,18 @@ class HttpFeedClient implements FeedClient {
     private final boolean speedTest;
 
     HttpFeedClient(FeedClientBuilderImpl builder) throws IOException {
-        this(builder, new HttpRequestStrategy(builder));
+        this(builder, builder.dryrun ? new DryrunCluster() : new ApacheCluster(builder));
     }
 
-    HttpFeedClient(FeedClientBuilderImpl builder, RequestStrategy requestStrategy) {
+    HttpFeedClient(FeedClientBuilderImpl builder, Cluster cluster) {
+        this(builder, cluster, new HttpRequestStrategy(builder, cluster));
+    }
+
+    HttpFeedClient(FeedClientBuilderImpl builder, Cluster cluster, RequestStrategy requestStrategy) {
         this.requestHeaders = new HashMap<>(builder.requestHeaders);
         this.requestStrategy = requestStrategy;
         this.speedTest = builder.speedTest;
-        verifyConnection(builder);
+        verifyConnection(builder, cluster);
     }
 
     @Override
@@ -121,9 +125,8 @@ class HttpFeedClient implements FeedClient {
         return promise;
     }
 
-    private void verifyConnection(FeedClientBuilderImpl builder) {
-        if (builder.dryrun) return;
-        try (Cluster cluster = new ApacheCluster(builder)) {
+    private void verifyConnection(FeedClientBuilderImpl builder, Cluster cluster) {
+        try {
             HttpRequest request = new HttpRequest("POST",
                                                   getPath(DocumentId.of("feeder", "handshake", "dummy")) + getQuery(empty(), true),
                                                   requestHeaders,
@@ -140,12 +143,15 @@ class HttpFeedClient implements FeedClient {
                     default: message = response.toString(); break;
                 }
                 else message = response.toString();
+
+                // Old server ignores ?dryRun=true, but getting this particular error message means everything else is OK.
+                if (response.code() == 400 && "Could not read document, no document?".equals(message)) {
+                    if (builder.speedTest) throw new FeedException("server does not support speed test; upgrade to a newer version");
+                    return;
+                }
                 throw new FeedException("server responded non-OK to handshake: " + message);
             }
         }
-        catch (IOException e) {
-            throw new FeedException("failed initiating handshake with server: " + e, e);
-        }
         catch (ExecutionException e) {
             throw new FeedException("failed handshake with server: " + e.getCause(), e.getCause());
         }
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java
index 2d052e8a6af..ce86ad59ffe 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java
@@ -68,10 +68,6 @@ class HttpRequestStrategy implements RequestStrategy {
         return thread;
     });
 
-    HttpRequestStrategy(FeedClientBuilderImpl builder) throws IOException {
-        this(builder, builder.dryrun ? new DryrunCluster() : new ApacheCluster(builder));
-    }
-
     HttpRequestStrategy(FeedClientBuilderImpl builder, Cluster cluster) {
         this.cluster = builder.benchmark ? new BenchmarkingCluster(cluster) : cluster;
         this.strategy = builder.retryStrategy;
diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpFeedClientTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpFeedClientTest.java
index a0b3049c2a0..295108b5ed5 100644
--- a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpFeedClientTest.java
+++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpFeedClientTest.java
@@ -3,7 +3,6 @@ package ai.vespa.feed.client.impl;
 
 import ai.vespa.feed.client.DocumentId;
 import ai.vespa.feed.client.FeedClient;
-import ai.vespa.feed.client.FeedClientBuilder;
 import ai.vespa.feed.client.FeedException;
 import ai.vespa.feed.client.HttpResponse;
 import ai.vespa.feed.client.OperationParameters;
@@ -44,6 +43,7 @@ class HttpFeedClientTest {
             @Override public CompletableFuture<HttpResponse> enqueue(DocumentId documentId, HttpRequest request) { return dispatch.get().apply(documentId, request); }
         }
         FeedClient client = new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))).setDryrun(true),
+                                               new DryrunCluster(),
                                                new MockRequestStrategy());
 
         // Update is a PUT, and 200 OK is a success.
@@ -212,9 +212,44 @@ class HttpFeedClientTest {
 
     @Test
     void testHandshake() {
+        // dummy:123 does not exist, and results in a host-not-found exception.
         assertTrue(assertThrows(FeedException.class,
-                                () -> new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))), null))
+                                () -> new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123")))))
                            .getMessage().startsWith("failed handshake with server: java.net.UnknownHostException"));
+
+        HttpResponse oldResponse = HttpResponse.of(400, "{\"pathId\":\"/document/v1/test/build/docid/foo\",\"message\":\"Could not read document, no document?\"}".getBytes(UTF_8));
+        HttpResponse okResponse = HttpResponse.of(200, null);
+        AtomicReference<HttpResponse> response = new AtomicReference<>(oldResponse);
+        Cluster cluster = (request, vessel) -> {
+            try {
+                assertNull(request.body());
+                assertEquals("POST", request.method());
+                assertEquals("/document/v1/feeder/handshake/docid/dummy?dryRun=true", request.path());
+                vessel.complete(response.get());
+            }
+            catch (Throwable t) {
+                vessel.completeExceptionally(t);
+            }
+        };
+
+        // Old server, and speed-test.
+        assertEquals("server does not support speed test; upgrade to a newer version",
+                     assertThrows(FeedException.class,
+                                  () -> new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))).setSpeedTest(true),
+                                                           cluster,
+                                                           null))
+                             .getMessage());
+
+        // Old server.
+        new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))),
+                           cluster,
+                           null);
+
+        // New server.
+        response.set(okResponse);
+        new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))),
+                           cluster,
+                           null);
     }
 
 }