Revert "Bratseth/deprecate httclient session take 2"

14 files changed, 42 insertions, 69 deletions

diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/FeedClientFactory.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/FeedClientFactory.java
index 4d50905da7b..6095134b7a2 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/FeedClientFactory.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/FeedClientFactory.java
@@ -5,9 +5,7 @@ package com.yahoo.vespa.http.client;
 import com.yahoo.vespa.http.client.config.SessionParams;
 import com.yahoo.vespa.http.client.core.api.FeedClientImpl;
 
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
-import java.util.concurrent.ThreadFactory;
+import static com.yahoo.vespa.http.client.SessionFactory.createTimeoutExecutor;
 
 /**
  * Factory for creating FeedClient.
@@ -17,7 +15,7 @@ import java.util.concurrent.ThreadFactory;
 public class FeedClientFactory {
 
     /**
-     * Creates a FeedClient. Call this sparingly: Feed clients are expensive and should be as long-lived as possible.
+     * Creates a FeedClient.
      *
      * @param sessionParams parameters for connection, hosts, cluster configurations and more.
      * @param resultCallback on each result, this callback is called.
@@ -27,31 +25,4 @@ public class FeedClientFactory {
         return new FeedClientImpl(sessionParams, resultCallback, createTimeoutExecutor());
     }
 
-    static ScheduledThreadPoolExecutor createTimeoutExecutor() {
-        ScheduledThreadPoolExecutor timeoutExecutor;
-        timeoutExecutor = new ScheduledThreadPoolExecutor(1, new DaemonThreadFactory("timeout-"));
-        timeoutExecutor.setRemoveOnCancelPolicy(true);
-        timeoutExecutor.setContinueExistingPeriodicTasksAfterShutdownPolicy(false);
-        timeoutExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
-        return timeoutExecutor;
-    }
-
-    private static class DaemonThreadFactory implements ThreadFactory {
-
-        private final ThreadFactory defaultThreadFactory = Executors.defaultThreadFactory();
-        private final String prefix;
-
-        private DaemonThreadFactory(String prefix) {
-            this.prefix = prefix;
-        }
-
-        @Override
-        public Thread newThread(Runnable runnable) {
-            Thread t = defaultThreadFactory.newThread(runnable);
-            t.setDaemon(true);
-            t.setName(prefix + t.getName());
-            return t;
-        }
-    }
-
 }
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Session.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Session.java
index 3089717c260..de39eabd3ee 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Session.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/Session.java
@@ -15,9 +15,7 @@ import java.util.concurrent.BlockingQueue;
  *
  * @author Einar M R Rosenvinge
  * @see SessionFactory
- * @deprecated use either FeedClient or SyncFeedClient // TODO: Remove on Vespa 8
  */
-@Deprecated
 public interface Session extends AutoCloseable {
 
     /**
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/SessionFactory.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/SessionFactory.java
index b03a2541cd0..2e47e45dff0 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/SessionFactory.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/SessionFactory.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.http.client;
 import com.yahoo.vespa.http.client.config.Cluster;
 import com.yahoo.vespa.http.client.config.Endpoint;
 import com.yahoo.vespa.http.client.config.SessionParams;
+import com.yahoo.vespa.http.client.core.api.SessionImpl;
 
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -13,9 +14,7 @@ import java.util.concurrent.ThreadFactory;
  * Factory for creating {@link Session} instances.
  *
  * @author Einar M R Rosenvinge
- * @deprecated use either FeedClient or SyncFeedClient // TODO: Remove on Vespa 8
  */
-@Deprecated
 public final class SessionFactory {
 
     /**
@@ -30,7 +29,7 @@ public final class SessionFactory {
 
     @SuppressWarnings("deprecation")
     static Session createInternal(SessionParams params) {
-        return new com.yahoo.vespa.http.client.core.api.SessionImpl(params, createTimeoutExecutor());
+        return new SessionImpl(params, createTimeoutExecutor());
     }
 
     static ScheduledThreadPoolExecutor createTimeoutExecutor() {
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
index f503190864b..bd187ea3371 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
@@ -16,7 +16,10 @@ import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 
 /**
- * Connection level parameters.
+ * Parameters given to a {@link com.yahoo.vespa.http.client.SessionFactory}
+ * when creating {@link com.yahoo.vespa.http.client.Session}s. The parameters
+ * contained in this class are related to the connections from the node running
+ * the Session to the Vespa clusters.
  * This class is immutable
  * and has no public constructor - to instantiate one, use a {@link Builder}.
  *
@@ -28,9 +31,9 @@ public final class ConnectionParams {
      * Builder for {@link ConnectionParams}.
      */
     public static final class Builder {
-
         private SSLContext sslContext = null;
         private HostnameVerifier hostnameVerifier = SSLConnectionSocketFactory.getDefaultHostnameVerifier();
+        private long connectionTimeout = TimeUnit.SECONDS.toMillis(60);
         private final Multimap<String, String> headers = ArrayListMultimap.create();
         private final Map<String, HeaderProvider> headerProviders = new HashMap<>();
         private int numPersistentConnectionsPerEndpoint = 1;
@@ -222,6 +225,7 @@ public final class ConnectionParams {
             return new ConnectionParams(
                     sslContext,
                     hostnameVerifier,
+                    connectionTimeout,
                     headers,
                     headerProviders,
                     numPersistentConnectionsPerEndpoint,
@@ -276,6 +280,7 @@ public final class ConnectionParams {
     }
     private final SSLContext sslContext;
     private final HostnameVerifier hostnameVerifier;
+    private final long connectionTimeout;
     private final Multimap<String, String> headers = ArrayListMultimap.create();
     private final Map<String, HeaderProvider> headerProviders = new HashMap<>();
     private final int numPersistentConnectionsPerEndpoint;
@@ -292,6 +297,7 @@ public final class ConnectionParams {
     private ConnectionParams(
             SSLContext sslContext,
             HostnameVerifier hostnameVerifier,
+            long connectionTimeout,
             Multimap<String, String> headers,
             Map<String, HeaderProvider> headerProviders,
             int numPersistentConnectionsPerEndpoint,
@@ -306,6 +312,7 @@ public final class ConnectionParams {
             boolean printTraceToStdErr) {
         this.sslContext = sslContext;
         this.hostnameVerifier = hostnameVerifier;
+        this.connectionTimeout = connectionTimeout;
         this.headers.putAll(headers);
         this.headerProviders.putAll(headerProviders);
         this.numPersistentConnectionsPerEndpoint = numPersistentConnectionsPerEndpoint;
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/FeedParams.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/FeedParams.java
index 008f3b63a89..30f1ad11d3a 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/FeedParams.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/FeedParams.java
@@ -6,7 +6,8 @@ import com.google.common.annotations.Beta;
 import java.util.concurrent.TimeUnit;
 
 /**
- * Feed level parameters. This class is immutable
+ * Parameters given to a {@link com.yahoo.vespa.http.client.SessionFactory}
+ * when creating {@link com.yahoo.vespa.http.client.Session}s. This class is immutable
  * and has no public constructor - to instantiate one, use a {@link Builder}.
 
  * @author Einar M R Rosenvinge
@@ -24,8 +25,8 @@ public final class FeedParams {
     public boolean getSilentUpgrade() { return silentUpgrade; }
 
     /**
-     * Enumeration of data formats that are acceptable by the
-     * {@link com.yahoo.vespa.http.client.FeedClient} methods.
+     * Enumeration of data formats that are acceptable by the OutputStream
+     * returned by {@link com.yahoo.vespa.http.client.Session#stream(CharSequence)}.
      */
     public enum DataFormat {
         /** UTF-8-encoded XML. Preamble is not necessary. */
@@ -116,7 +117,9 @@ public final class FeedParams {
          *
          * Note that the TOTAL timeout of any one operation in this API would be
          * {@link #getServerTimeout(java.util.concurrent.TimeUnit)} +
-         * {@link #getClientTimeout(java.util.concurrent.TimeUnit)}.
+         * {@link #getClientTimeout(java.util.concurrent.TimeUnit)},
+         * after which {@link com.yahoo.vespa.http.client.Session#results()} is guaranteed
+         * to produce a Result.
          *
          * @param serverTimeout timeout value
          * @param unit unit of timeout value
@@ -132,13 +135,14 @@ public final class FeedParams {
 
         /**
          * Sets the client-side timeout for each operation.&nbsp;If BOTH the server-side
-         * timeout AND this timeout has passed, the {@link com.yahoo.vespa.http.client.FeedClient}
+         * timeout AND this timeout has passed, {@link com.yahoo.vespa.http.client.Session}
          * will synthesize a {@link com.yahoo.vespa.http.client.Result}.
          *
          * Note that the TOTAL timeout of any one operation in this API would be
          * {@link #getServerTimeout(java.util.concurrent.TimeUnit)} +
          * {@link #getClientTimeout(java.util.concurrent.TimeUnit)},
-         * after which a result callback is guaranteed to be made.
+         * after which {@link com.yahoo.vespa.http.client.Session#results()} is guaranteed
+         * to produce a Result.
          *
          * @param clientTimeout timeout value
          * @param unit unit of timeout value
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/SessionParams.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/SessionParams.java
index 48fd21e2b1f..daa6eecb823 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/SessionParams.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/SessionParams.java
@@ -8,12 +8,12 @@ import java.util.LinkedList;
 import java.util.List;
 
 /**
- * Parameters given to a {@link com.yahoo.vespa.http.client.FeedClientFactory}
- * when creating {@link com.yahoo.vespa.http.client.FeedClient}s. This class is immutable
+ * Parameters given to a {@link com.yahoo.vespa.http.client.SessionFactory}
+ * when creating {@link com.yahoo.vespa.http.client.Session}s. This class is immutable
  * and has no public constructor - to instantiate one, use a {@link Builder}.
  *
  * @author Einar M R Rosenvinge
- * @see com.yahoo.vespa.http.client.FeedClientFactory
+ * @see com.yahoo.vespa.http.client.SessionFactory
  * @see Builder
  */
 public final class SessionParams {
@@ -85,7 +85,8 @@ public final class SessionParams {
 
         /**
          * Sets the maximum number of document operations to hold in memory, waiting to be
-         * sent to Vespa. When this threshold is reached, {@link java.io.OutputStream#close()} will block.
+         * sent to Vespa. When this threshold is reached, {@link java.io.OutputStream#close()} will block,
+         * see {@link com.yahoo.vespa.http.client.Session#stream(CharSequence)}.
          *
          * @param clientQueueSize the maximum number of document operations to hold in memory.
          * @return pointer to builder.
@@ -110,7 +111,7 @@ public final class SessionParams {
         }
 
         /**
-         * Instantiates a {@link SessionParams} that can be given to a {@link com.yahoo.vespa.http.client.FeedClientFactory}.
+         * Instantiates a {@link SessionParams} that can be given to a {@link com.yahoo.vespa.http.client.SessionFactory}.
          *
          * @return a SessionParams object with the parameters of this Builder
          */
@@ -132,7 +133,6 @@ public final class SessionParams {
             return throttlerMinSize;
         }
     }
-
     private final List<Cluster> clusters;
     private final FeedParams feedParams;
     private final ConnectionParams connectionParams;
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/SessionImpl.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/SessionImpl.java
index e11903d7ba7..c3b5d9912de 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/SessionImpl.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/api/SessionImpl.java
@@ -16,10 +16,7 @@ import java.util.concurrent.ScheduledThreadPoolExecutor;
 
 /**
  * This class wires up the Session API using MultiClusterHandler and MultiClusterSessionOutputStream.
- *
- * @deprecated
  */
-@Deprecated // TODO: Remove on Vespa 8
 public class SessionImpl implements Session {
 
     private final OperationProcessor operationProcessor;
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
index 5289a7a562a..f9357ab16d8 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
@@ -361,12 +361,12 @@ class ApacheGatewayConnection implements GatewayConnection {
 
     @Override
     public void handshake() throws ServerResponseException, IOException {
-        boolean useCompression = false;
-        boolean drain = false;
-        boolean handshake = true;
+        final boolean useCompression = false;
+        final boolean drain = false;
+        final boolean handshake = true;
         HttpPost httpPost = createPost(drain, useCompression, handshake);
 
-        String oldSessionID = sessionId;
+        final String oldSessionID = sessionId;
         sessionId = null;
         try (InputStream stream = executePost(httpPost)) {
             if (oldSessionID != null && !oldSessionID.equals(sessionId)) {
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/package-info.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/package-info.java
index c54c19a2eb1..b14c2ffa4cc 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/package-info.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/package-info.java
@@ -1,7 +1,10 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 /**
  * Programmatic API for feeding to Vespa clusters independently of the
- * cluster configuration.
+ * cluster configuration. {@link com.yahoo.vespa.http.client.Session}
+ * is the central interface which is used to interact with a cluster.
+ * Use {@link com.yahoo.vespa.http.client.SessionFactory} to
+ * instantiate a {@link com.yahoo.vespa.http.client.Session}.
  *
  * NOTE: This is a PUBLIC API, but not annotated as such because this is not a bundle and
  *       we don't want to introduce Vespa dependencies.
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/FeedClientTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/FeedClientTest.java
index 88deaa07e12..33afc759f59 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/FeedClientTest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/FeedClientTest.java
@@ -34,14 +34,13 @@ public class FeedClientTest {
                     .build())
             .build();
     final AtomicInteger resultsReceived = new AtomicInteger(0);
-
     FeedClient.ResultCallback resultCallback = (docId, documentResult) -> {
         assertTrue(documentResult.isSuccess());
         assertEquals(DOCID, docId);
         resultsReceived.incrementAndGet();
     };
 
-    FeedClient feedClient = new FeedClientImpl(sessionParams, resultCallback, FeedClientFactory.createTimeoutExecutor());
+    FeedClient feedClient = new FeedClientImpl(sessionParams, resultCallback, SessionFactory.createTimeoutExecutor());
 
     @Test
     public void testStreamAndClose() {
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/QueueBoundsTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/QueueBoundsTest.java
index 344f8b46639..357296eb789 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/QueueBoundsTest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/QueueBoundsTest.java
@@ -33,7 +33,6 @@ import static org.junit.Assert.fail;
  *
  * @author Einar M R Rosenvinge
  */
-@SuppressWarnings("deprecation")
 public class QueueBoundsTest extends TestOnCiBuildingSystemOnly {
 
     public static final List<TestDocument> documents;
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/TestUtils.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/TestUtils.java
index b44385e11ff..8827c863024 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/TestUtils.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/TestUtils.java
@@ -13,11 +13,10 @@ import java.util.zip.GZIPInputStream;
 import static org.junit.Assert.assertNull;
 
 /**
- * @author Einar M R Rosenvinge
+ * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a>
+ * @since 5.1.20
  */
-@SuppressWarnings("deprecation")
 public class TestUtils {
-
     public static void writeDocuments(Session session, List<TestDocument> documents) throws IOException {
         for (TestDocument document : documents) {
             writeDocument(session, document);
@@ -58,5 +57,4 @@ public class TestUtils {
         }
         return rawContent.toString();
     }
-
 }
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java
index 8c4b24f74c2..88f5496d929 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPIMultiClusterTest.java
@@ -23,10 +23,9 @@ import static org.junit.Assert.fail;
 
 /**
  * Only runs on screwdriver to save time!
- *
- * @author Einar M R Rosenvinge
+ * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a>
+ * @since 5.1.27
  */
-@SuppressWarnings("deprecation")
 public class V3HttpAPIMultiClusterTest extends TestOnCiBuildingSystemOnly {
 
     private void writeDocuments(Session session) throws IOException {
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java
index ad54bbc6a90..594a49e7d91 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/V3HttpAPITest.java
@@ -28,7 +28,6 @@ import static org.junit.Assert.assertThat;
  *
  * @author Einar M R Rosenvinge
  */
-@SuppressWarnings("deprecation")
 public class V3HttpAPITest extends TestOnCiBuildingSystemOnly {
 
     public static final List<TestDocument> documents;