Remove HttpExecutionStrategy from requester and reserveConnection A…

…PI (#1956) Motivation: used to propagate additional meta-information to the transport layers, like a required `HttpExecutionStrategy` or `FlushStrategy`. It allows us to simplify requester and filters API and remove an overload that requires `HttpExecutionStrategy` from there. Modifications: - Add `HttpContextKeys` with `HTTP_EXECUTION_STRATEGY_KEY`; - Promote `request(RequestType)` method to `StreamingHttpRequester`, `HttpRequester`, `BlockingHttpRequester`, and `BlockingStreamingHttpRequester` and deprecate `request(HttpExecutionStrategy, RequestType)` method; - Deprecate `reserveConnection(HttpExecutionStrategy, HttpRequestMetaData)` method in `FilterableStreamingHttpClient`, `HttpClient`, `BlockingHttpClient`, `BlockingStreamingHttpClient`; - All default implementation delegate to a new method; - Add `NewToDeprecatedFilter` to allow users to have mixed filters in the pipeline (migrated and deprecated); - `MixedFiltersTest`: test all different combinations of filters to make sure both can work together; - Remove unused `FilterableConnectionToConnection`; - Update `evolve-to-async.adoc`; Result: Less overloads on client and requester API, simplified filtering, users can incrementally migrate to simplified API.
apple · Nov 19, 2021 · 3c2d0e2 · 3c2d0e2
1 parent 34f726e
commit 3c2d0e2
Show file tree

Hide file tree

Showing 36 changed files with 820 additions and 187 deletions.
diff --git a/servicetalk-http-api/docs/modules/ROOT/pages/evolve-to-async.adoc b/servicetalk-http-api/docs/modules/ROOT/pages/evolve-to-async.adoc
@@ -28,16 +28,9 @@ of operation.
 
 === No offloads per request
 
-HTTP clients for all programming models provide methods to make requests with or without `HttpExecutionStrategy`.
-
-See
-link:{source-root}/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpClient.java[HttpClient].
-[source, java]
-----
-Single<HttpResponse> request(HttpRequest request);
-
-Single<HttpResponse> request(HttpExecutionStrategy strategy, HttpRequest request);
-----
+HTTP clients for all programming models recognize `HTTP_EXECUTION_STRATEGY_KEY` from
+link:{source-root}/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpContextKeys.java[HttpContextKeys] that
+allows to override offloading by passing its value in `HttpRequestMetaData#context()`.
 
 In order to disable offloading for a specific request one has to use a specific `HttpExecutionStrategy` that does not
 do any offloading. Such a strategy is available out of the box in
@@ -47,7 +40,9 @@ and can be used as below:
 [source, java]
 ----
 // Processing of this request does not have any blocking code
-httpClient.request(HttpExecutionStrategies.noOffloadsStrategy(), aRequest);
+HttpRequest request = httpClient.newRequest(method, requestTarget);
+request.context().put(HTTP_EXECUTION_STRATEGY_KEY, HttpExecutionStrategies.noOffloadsStrategy());
+HttpResponse  response = httpClient.request(request);
 ----
 
 For a request made using this strategy, ServiceTalk will not perform any offloading and all user calls may be invoked

diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingHttpClient.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingHttpClient.java
@@ -15,6 +15,8 @@
  */
 package io.servicetalk.http.api;
 
+import static io.servicetalk.http.api.HttpContextKeys.HTTP_EXECUTION_STRATEGY_KEY;
+
 /**
  * The equivalent of {@link HttpClient} but with synchronous/blocking APIs instead of asynchronous APIs.
  */
@@ -26,6 +28,7 @@ public interface BlockingHttpClient extends BlockingHttpRequester {
      * @return The response.
      * @throws Exception if an exception occurs during the request processing.
      */
+    @Override   // FIXME: 0.42 - remove, this method is defined in BlockingHttpRequester
     HttpResponse request(HttpRequest request) throws Exception;
 
     /**
@@ -47,10 +50,17 @@ public interface BlockingHttpClient extends BlockingHttpRequester {
      * reserve for future {@link HttpRequest requests} with the same {@link HttpRequestMetaData}.
      * For example this may provide some insight into shard or other info.
      * @return a {@link ReservedBlockingHttpConnection}.
-     * @throws Exception if a exception occurs during the reservation process.
+     * @throws Exception if an exception occurs during the reservation process.
+     * @deprecated Use {@link #reserveConnection(HttpRequestMetaData)}. If an {@link HttpExecutionStrategy} needs to be
+     * altered, provide a value for {@link HttpContextKeys#HTTP_EXECUTION_STRATEGY_KEY} in the
+     * {@link HttpRequestMetaData#context() request context}.
      */
-    ReservedBlockingHttpConnection reserveConnection(HttpExecutionStrategy strategy,
-                                                     HttpRequestMetaData metaData) throws Exception;
+    @Deprecated
+    default ReservedBlockingHttpConnection reserveConnection(HttpExecutionStrategy strategy,
+                                                             HttpRequestMetaData metaData) throws Exception {
+        metaData.context().put(HTTP_EXECUTION_STRATEGY_KEY, strategy);
+        return reserveConnection(metaData);
+    }
 
     /**
      * Convert this {@link BlockingHttpClient} to the {@link StreamingHttpClient} API.

diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingHttpConnection.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingHttpConnection.java
@@ -29,6 +29,7 @@ public interface BlockingHttpConnection extends BlockingHttpRequester {
      * @return The response.
      * @throws Exception if an exception occurs during the request processing.
      */
+    @Override   // FIXME: 0.42 - remove, this method is defined in BlockingHttpRequester
     HttpResponse request(HttpRequest request) throws Exception;
 
     /**

diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingHttpRequester.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingHttpRequester.java
@@ -17,19 +17,40 @@
 
 import io.servicetalk.concurrent.GracefulAutoCloseable;
 
+import static io.servicetalk.http.api.HttpContextKeys.HTTP_EXECUTION_STRATEGY_KEY;
+
 /**
  * The equivalent of {@link HttpRequester} with synchronous/blocking APIs instead of asynchronous APIs.
  */
 public interface BlockingHttpRequester extends HttpRequestFactory, GracefulAutoCloseable {
+    /**
+     * Send a {@code request}.
+     *
+     * @param request the request to send.
+     * @return The response.
+     */
+    default HttpResponse request(HttpRequest request) throws Exception {
+        // FIXME: 0.42 - remove default impl
+        throw new UnsupportedOperationException("Method request(HttpRequest) is not supported by " +
+                getClass().getName());
+    }
+
     /**
      * Send a {@code request} using the passed {@link HttpExecutionStrategy strategy}.
      *
      * @param strategy {@link HttpExecutionStrategy} to use.
      * @param request the request to send.
      * @return The response.
      * @throws Exception if an exception occurs during the request processing.
+     * @deprecated Use {@link #request(HttpRequest)}. If an {@link HttpExecutionStrategy} needs to be altered, provide a
+     * value for {@link HttpContextKeys#HTTP_EXECUTION_STRATEGY_KEY} in the
+     * {@link HttpRequestMetaData#context() request context}.
      */
-    HttpResponse request(HttpExecutionStrategy strategy, HttpRequest request) throws Exception;
+    @Deprecated
+    default HttpResponse request(HttpExecutionStrategy strategy, HttpRequest request) throws Exception {
+        request.context().put(HTTP_EXECUTION_STRATEGY_KEY, strategy);
+        return request(request);
+    }
 
     /**
      * Get the {@link HttpExecutionContext} used during construction of this object.

diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingStreamingHttpClient.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingStreamingHttpClient.java
@@ -15,6 +15,8 @@
  */
 package io.servicetalk.http.api;
 
+import static io.servicetalk.http.api.HttpContextKeys.HTTP_EXECUTION_STRATEGY_KEY;
+
 /**
  * The equivalent of {@link StreamingHttpClient} but with synchronous/blocking APIs instead of asynchronous APIs.
  */
@@ -26,6 +28,7 @@ public interface BlockingStreamingHttpClient extends BlockingStreamingHttpReques
      * @return The response.
      * @throws Exception if an exception occurs during the request processing.
      */
+    @Override   // FIXME: 0.42 - remove, this method is defined in BlockingHttpRequester
     BlockingStreamingHttpResponse request(BlockingStreamingHttpRequest request) throws Exception;
 
     /**
@@ -47,10 +50,17 @@ public interface BlockingStreamingHttpClient extends BlockingStreamingHttpReques
      * reserve for future {@link BlockingStreamingHttpRequest requests} with the same {@link HttpRequestMetaData}.
      * For example this may provide some insight into shard or other info.
      * @return a {@link ReservedBlockingStreamingHttpConnection}.
-     * @throws Exception if a exception occurs during the reservation process.
+     * @throws Exception if an exception occurs during the reservation process.
+     * @deprecated Use {@link #reserveConnection(HttpRequestMetaData)}. If an {@link HttpExecutionStrategy} needs to be
+     * altered, provide a value for {@link HttpContextKeys#HTTP_EXECUTION_STRATEGY_KEY} in the
+     * {@link HttpRequestMetaData#context() request context}.
      */
-    ReservedBlockingStreamingHttpConnection reserveConnection(
-            HttpExecutionStrategy strategy, HttpRequestMetaData metaData) throws Exception;
+    @Deprecated
+    default ReservedBlockingStreamingHttpConnection reserveConnection(
+            HttpExecutionStrategy strategy, HttpRequestMetaData metaData) throws Exception {
+        metaData.context().put(HTTP_EXECUTION_STRATEGY_KEY, strategy);
+        return reserveConnection(metaData);
+    }
 
     /**
      * Convert this {@link BlockingStreamingHttpClient} to the {@link StreamingHttpClient} API.

diff --git a/...etalk-http-api/src/main/java/io/servicetalk/http/api/BlockingStreamingHttpConnection.java b/...etalk-http-api/src/main/java/io/servicetalk/http/api/BlockingStreamingHttpConnection.java
@@ -29,6 +29,7 @@ public interface BlockingStreamingHttpConnection extends BlockingStreamingHttpRe
      * @return The response.
      * @throws Exception if an exception occurs during the request processing.
      */
+    @Override   // FIXME: 0.42 - remove, this method is defined in BlockingHttpRequester
     BlockingStreamingHttpResponse request(BlockingStreamingHttpRequest request) throws Exception;
 
     /**

diff --git a/...cetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingStreamingHttpRequester.java b/...cetalk-http-api/src/main/java/io/servicetalk/http/api/BlockingStreamingHttpRequester.java
@@ -17,20 +17,41 @@
 
 import io.servicetalk.concurrent.GracefulAutoCloseable;
 
+import static io.servicetalk.http.api.HttpContextKeys.HTTP_EXECUTION_STRATEGY_KEY;
+
 /**
  * The equivalent of {@link StreamingHttpRequester} but with synchronous/blocking APIs instead of asynchronous APIs.
  */
 public interface BlockingStreamingHttpRequester extends BlockingStreamingHttpRequestFactory, GracefulAutoCloseable {
+    /**
+     * Send a {@code request}.
+     *
+     * @param request the request to send.
+     * @return The response.
+     */
+    default BlockingStreamingHttpResponse request(BlockingStreamingHttpRequest request) throws Exception {
+        // FIXME: 0.42 - remove default impl
+        throw new UnsupportedOperationException("Method request(HttpRequest) is not supported by " +
+                getClass().getName());
+    }
+
     /**
      * Send a {@code request} using the passed {@link HttpExecutionStrategy strategy}.
      *
      * @param strategy {@link HttpExecutionStrategy} to use.
      * @param request the request to send.
      * @return The response.
      * @throws Exception if an exception occurs during the request processing.
+     * @deprecated Use {@link #request(BlockingStreamingHttpRequest)}. If an {@link HttpExecutionStrategy} needs to be
+     * altered, provide a value for {@link HttpContextKeys#HTTP_EXECUTION_STRATEGY_KEY} in the
+     * {@link HttpRequestMetaData#context() request context}.
      */
-    BlockingStreamingHttpResponse request(HttpExecutionStrategy strategy,
-                                          BlockingStreamingHttpRequest request) throws Exception;
+    @Deprecated
+    default BlockingStreamingHttpResponse request(HttpExecutionStrategy strategy,
+                                          BlockingStreamingHttpRequest request) throws Exception {
+        request.context().put(HTTP_EXECUTION_STRATEGY_KEY, strategy);
+        return request(request);
+    }
 
     /**
      * Get the {@link HttpExecutionContext} used during construction of this object.

diff --git a/...-api/src/main/java/io/servicetalk/http/api/FilterableReservedStreamingHttpConnection.java b/...-api/src/main/java/io/servicetalk/http/api/FilterableReservedStreamingHttpConnection.java
@@ -19,7 +19,7 @@
 
 /**
  * A special type of {@link StreamingHttpConnection} for the exclusive use of the caller of
- * {@link StreamingHttpClient#reserveConnection(HttpExecutionStrategy, HttpRequestMetaData)}.
+ * {@link StreamingHttpClient#reserveConnection(HttpRequestMetaData)}.
  */
 public interface FilterableReservedStreamingHttpConnection extends FilterableStreamingHttpConnection {
     /**

diff --git a/...icetalk-http-api/src/main/java/io/servicetalk/http/api/FilterableStreamingHttpClient.java b/...icetalk-http-api/src/main/java/io/servicetalk/http/api/FilterableStreamingHttpClient.java
@@ -17,10 +17,27 @@
 
 import io.servicetalk.concurrent.api.Single;
 
+import static io.servicetalk.http.api.HttpContextKeys.HTTP_EXECUTION_STRATEGY_KEY;
+
 /**
  * A {@link StreamingHttpClient} that supports filtering.
  */
 public interface FilterableStreamingHttpClient extends StreamingHttpRequester {
+    /**
+     * Reserve a {@link StreamingHttpConnection} based on provided {@link HttpRequestMetaData}.
+     *
+     * @param metaData Allows the underlying layers to know what {@link StreamingHttpConnection}s are valid to
+     * reserve for future {@link StreamingHttpRequest requests} with the same {@link HttpRequestMetaData}.
+     * For example this may provide some insight into shard or other info.
+     * @return a {@link Single} that provides the {@link ReservedStreamingHttpConnection} upon completion.
+     */
+    default Single<? extends FilterableReservedStreamingHttpConnection> reserveConnection(
+            HttpRequestMetaData metaData) {
+        // FIXME: 0.42 - remove default impl
+        throw new UnsupportedOperationException("Method reserveConnection(HttpRequestMetaData) is not supported by " +
+                getClass().getName());
+    }
+
     /**
      * Reserve a {@link StreamingHttpConnection} based on provided {@link HttpRequestMetaData}.
      *
@@ -29,7 +46,16 @@ public interface FilterableStreamingHttpClient extends StreamingHttpRequester {
      * reserve for future {@link StreamingHttpRequest requests} with the same {@link HttpRequestMetaData}.
      * For example this may provide some insight into shard or other info.
      * @return a {@link Single} that provides the {@link ReservedStreamingHttpConnection} upon completion.
+     * @deprecated Use {@link #reserveConnection(HttpRequestMetaData)}. If an {@link HttpExecutionStrategy} needs to be
+     * altered, provide a value for {@link HttpContextKeys#HTTP_EXECUTION_STRATEGY_KEY} in the
+     * {@link HttpRequestMetaData#context() request context}.
      */
-    Single<? extends FilterableReservedStreamingHttpConnection> reserveConnection(HttpExecutionStrategy strategy,
-                                                                                  HttpRequestMetaData metaData);
+    @Deprecated
+    default Single<? extends FilterableReservedStreamingHttpConnection> reserveConnection(
+            HttpExecutionStrategy strategy, HttpRequestMetaData metaData) {
+        return Single.defer(() -> {
+            metaData.context().put(HTTP_EXECUTION_STRATEGY_KEY, strategy);
+            return reserveConnection(metaData).subscribeShareContext();
+        });
+    }
 }
diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpApiConversions.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpApiConversions.java
@@ -19,6 +19,8 @@
 import io.servicetalk.http.api.StreamingHttpClientToBlockingStreamingHttpClient.ReservedStreamingHttpConnectionToBlockingStreaming;
 import io.servicetalk.http.api.StreamingHttpClientToHttpClient.ReservedStreamingHttpConnectionToReservedHttpConnection;
 
+import static io.servicetalk.http.api.HttpContextKeys.HTTP_EXECUTION_STRATEGY_KEY;
+
 /**
  * Conversion routines to {@link StreamingHttpService}.
  */
@@ -265,4 +267,9 @@ public static HttpService toHttpService(StreamingHttpService service) {
     public static BlockingStreamingHttpService toBlockingStreamingHttpService(StreamingHttpService service) {
         return new StreamingHttpServiceToBlockingStreamingHttpService(service);
     }
+
+    static HttpExecutionStrategy requestStrategy(HttpRequestMetaData metaData, HttpExecutionStrategy fallback) {
+        final HttpExecutionStrategy strategy = metaData.context().get(HTTP_EXECUTION_STRATEGY_KEY);
+        return strategy != null ? strategy : fallback;
+    }
 }
diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpClient.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpClient.java
@@ -19,6 +19,7 @@
 import io.servicetalk.concurrent.api.Single;
 
 import static io.servicetalk.concurrent.internal.FutureUtils.awaitTermination;
+import static io.servicetalk.http.api.HttpContextKeys.HTTP_EXECUTION_STRATEGY_KEY;
 
 /**
  * Provides a means to issue requests against HTTP service. The implementation is free to maintain a collection of
@@ -31,6 +32,7 @@ public interface HttpClient extends HttpRequester, GracefulAutoCloseable {
      * @param request the request to send.
      * @return The response.
      */
+    @Override   // FIXME: 0.42 - remove, this method is defined in HttpRequester
     Single<HttpResponse> request(HttpRequest request);
 
     /**
@@ -53,8 +55,18 @@ public interface HttpClient extends HttpRequester, GracefulAutoCloseable {
      * reserve for future {@link HttpRequest requests} with the same {@link HttpRequestMetaData}.
      * For example this may provide some insight into shard or other info.
      * @return a {@link Single} that provides the {@link ReservedHttpConnection} upon completion.
+     * @deprecated Use {@link #reserveConnection(HttpRequestMetaData)}. If an {@link HttpExecutionStrategy} needs to be
+     * altered, provide a value for {@link HttpContextKeys#HTTP_EXECUTION_STRATEGY_KEY} in the
+     * {@link HttpRequestMetaData#context() request context}.
      */
-    Single<ReservedHttpConnection> reserveConnection(HttpExecutionStrategy strategy, HttpRequestMetaData metaData);
+    @Deprecated
+    default Single<ReservedHttpConnection> reserveConnection(HttpExecutionStrategy strategy,
+                                                             HttpRequestMetaData metaData) {
+        return Single.defer(() -> {
+            metaData.context().put(HTTP_EXECUTION_STRATEGY_KEY, strategy);
+            return reserveConnection(metaData).subscribeShareContext();
+        });
+    }
 
     /**
      * Convert this {@link HttpClient} to the {@link StreamingHttpClient} API.

diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpConnection.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpConnection.java
@@ -32,6 +32,7 @@ public interface HttpConnection extends HttpRequester, GracefulAutoCloseable {
      * @param request the request to send.
      * @return The response.
      */
+    @Override   // FIXME: 0.42 - remove, this method is defined in HttpRequester
     Single<HttpResponse> request(HttpRequest request);
 
     /**

diff --git a/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpContextKeys.java b/servicetalk-http-api/src/main/java/io/servicetalk/http/api/HttpContextKeys.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright © 2021 Apple Inc. and the ServiceTalk project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.servicetalk.http.api;
+
+import io.servicetalk.context.api.ContextMap;
+import io.servicetalk.context.api.ContextMap.Key;
+
+import static io.servicetalk.context.api.ContextMap.Key.newKey;
+
+/**
+ * All {@link ContextMap.Key}(s) defined for HTTP.
+ */
+public final class HttpContextKeys {
+
+    /**
+     * Allows using a custom {@link HttpExecutionStrategy} for the HTTP message execution, when present in the meta-data
+     * {@link HttpMetaData#context() context}. Otherwise, an automatically inferred strategy will be used by a client
+     * or server.
+     */
+    public static final Key<HttpExecutionStrategy> HTTP_EXECUTION_STRATEGY_KEY =
+            newKey("HTTP_EXECUTION_STRATEGY_KEY", HttpExecutionStrategy.class);
+
+    private HttpContextKeys() {
+        // No instances
+    }
+}