Fix filters for MultiAddressUrlHttpClientBuilder (#2071)

Motivation:

Recently the API of filters changed to align better with the way
`ExecutionStrategy` should be set. A temporary additional filter
bridging calls via new API to old API was created to assure compatibility.
Unfortunately, the `MultiAddressUrlHttpClientBuilder` was not updated
and legacy filters may not be exercised.

Modifications:

- Properly appending the `NewToDeprecatedFilter` to filters added via
  `MultiAddressUrlHttpClientBuilder`,
- Legacy `StrategyInfluencerAwareConversions#toMultiAddressClientFactory`
  was also updated to include the bridge,
- `MixedFiltersTest` include a case validating the
  `MultiAddressUrlHttpClientBuilder` behavior.

Result:

Deprecated methods on filters are properly executed when using
`MultiAddressUrlHttpClientBuilder`.

servicetalk-http-api/src/main/java/io/servicetalk/http/api/StrategyInfluencerAwareConversions.java

@@ -21,6 +21,7 @@
 import java.util.function.Predicate;
 import javax.annotation.Nullable;
 
+import static io.servicetalk.http.api.NewToDeprecatedFilter.NEW_TO_DEPRECATED_FILTER;
 import static java.util.Objects.requireNonNull;
 
 final class StrategyInfluencerAwareConversions {
@@ -67,7 +68,8 @@ public StreamingHttpClientFilter create(final U address, final FilterableStreami
                 }
             };
         }
-        return (address, client) -> new StreamingHttpClientFilter(original.create(client));
+        return (address, client) -> new StreamingHttpClientFilter(
+                NEW_TO_DEPRECATED_FILTER.create(original.create(client)));
     }
 
     static StreamingHttpServiceFilterFactory toConditionalServiceFilterFactory(

servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java

@@ -66,6 +66,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.BooleanSupplier;
 import java.util.function.Function;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import static io.netty.handler.codec.http.HttpScheme.HTTP;
@@ -75,6 +76,7 @@
 import static io.servicetalk.concurrent.api.Single.defer;
 import static io.servicetalk.concurrent.internal.SubscriberUtils.deliverCompleteFromSource;
 import static io.servicetalk.http.netty.DefaultSingleAddressHttpClientBuilder.defaultReqRespFactory;
+import static io.servicetalk.http.netty.NewToDeprecatedFilter.NEW_TO_DEPRECATED_FILTER;
 import static java.util.Objects.requireNonNull;
 
 /**
@@ -127,8 +129,8 @@ public StreamingHttpClient buildStreaming() {
                                     buildContext.executionContext.bufferAllocator())));
 
             // Need to wrap the top level client (group) in order for non-relative redirects to work
-            urlClient = redirectConfig == null ? urlClient :
-                    new RedirectingHttpRequesterFilter(redirectConfig).create(urlClient);
+            urlClient = redirectConfig == null ? urlClient : new RedirectingHttpRequesterFilter(redirectConfig)
+                    .create(NEW_TO_DEPRECATED_FILTER.create(urlClient));
 
             return new FilterableClientToClient(urlClient, buildContext.executionContext.executionStrategy(),
                     buildContext.builder.buildStrategyInfluencerForClient(
@@ -481,6 +483,14 @@ private MultiAddressHttpClientFilterFactory<HostAndPort> appendClientFilter(
             final MultiAddressHttpClientFilterFactory<HostAndPort> next) {
         requireNonNull(next);
         builderTemplate.appendToStrategyInfluencer(next);
+        return appendClientFilter0(appendClientFilter0(current, next),
+                NEW_TO_DEPRECATED_FILTER.asMultiAddressClientFilter());
+    }
+
+    @Nonnull
+    private MultiAddressHttpClientFilterFactory<HostAndPort> appendClientFilter0(
+            final @Nullable MultiAddressHttpClientFilterFactory<HostAndPort> current,
+            final MultiAddressHttpClientFilterFactory<HostAndPort> next) {
         return current == null ? next : (group, client) -> current.create(group, next.create(group, client));
     }
 

servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/MixedFiltersTest.java

@@ -21,6 +21,7 @@
 import io.servicetalk.http.api.FilterableStreamingHttpConnection;
 import io.servicetalk.http.api.HttpExecutionStrategy;
 import io.servicetalk.http.api.HttpResponse;
+import io.servicetalk.http.api.MultiAddressHttpClientBuilder;
 import io.servicetalk.http.api.ReservedBlockingHttpConnection;
 import io.servicetalk.http.api.SingleAddressHttpClientBuilder;
 import io.servicetalk.http.api.StreamingHttpClientFilter;
@@ -94,15 +95,15 @@ public static Collection<Arguments> arguments() {
     @ParameterizedTest(name = "{displayName} [{index}] filters={0}, forClient={1}, reservedConnection={2}, " +
             "conditional={3}")
     @MethodSource("arguments")
-    void test(List<ConditionalFilterFactory> filters, boolean forClient, boolean reservedConnection,
-              boolean conditional) throws Exception {
+    void testSingleClient(List<ConditionalFilterFactory> filters, boolean forClient, boolean reservedConnection,
+                          boolean conditional) throws Exception {
 
         String expected = filters.stream()
                 .filter(ConditionalFilterFactory::apply)
                 .map(ConditionalFilterFactory::toString)
                 .reduce((first, second) -> first + ',' + second)
                 .get();
-        test(builder -> {
+        testSingleClient(builder -> {
             if (forClient) {
                 if (conditional) {
                     filters.forEach(cff -> builder.appendClientFilter(__ -> cff.apply(), cff));
@@ -118,8 +119,36 @@ void test(List<ConditionalFilterFactory> filters, boolean forClient, boolean res
         }, reservedConnection, expected);
     }
 
-    private static void test(UnaryOperator<SingleAddressHttpClientBuilder<HostAndPort, InetSocketAddress>> filters,
-                             boolean reservedConnection, CharSequence expectedValue) throws Exception {
+    @ParameterizedTest(name = "{displayName} [{index}] filters={0}, forClient={1}, reservedConnection={2}, " +
+            "conditional={3}")
+    @MethodSource("arguments")
+    void testMultiClient(List<ConditionalFilterFactory> filters, boolean forClient, boolean reservedConnection,
+                         boolean conditional) throws Exception {
+
+        String expected = filters.stream()
+                .filter(ConditionalFilterFactory::apply)
+                .map(ConditionalFilterFactory::toString)
+                .reduce((first, second) -> first + ',' + second)
+                .get();
+        testMultiClient(builder -> {
+            if (forClient) {
+                if (conditional) {
+                    filters.forEach(cff -> builder.appendClientFilter(__ -> cff.apply(), cff));
+                } else {
+                    filters.forEach(builder::appendClientFilter);
+                }
+            } else if (conditional) {
+                filters.forEach(cff -> builder.appendConnectionFilter(__ -> cff.apply(), cff));
+            } else {
+                filters.forEach(builder::appendConnectionFilter);
+            }
+            return builder;
+        }, reservedConnection, expected);
+    }
+
+    private static void testSingleClient(
+            UnaryOperator<SingleAddressHttpClientBuilder<HostAndPort, InetSocketAddress>> filters,
+            boolean reservedConnection, CharSequence expectedValue) throws Exception {
         try (ServerContext serverContext = HttpServers.forAddress(localAddress(0))
                 .listenBlockingAndAwait((ctx, request, responseFactory) -> responseFactory.ok()
                         .setHeader(FILTERS_HEADER, requireNonNull(request.headers().get(FILTERS_HEADER))));
@@ -139,6 +168,29 @@ private static void test(UnaryOperator<SingleAddressHttpClientBuilder<HostAndPor
         }
     }
 
+    private static void testMultiClient(
+            UnaryOperator<MultiAddressHttpClientBuilder<HostAndPort, InetSocketAddress>> filters,
+            boolean reservedConnection, CharSequence expectedValue) throws Exception {
+        try (ServerContext serverContext = HttpServers.forAddress(localAddress(0))
+                .listenBlockingAndAwait((ctx, request, responseFactory) -> responseFactory.ok()
+                        .setHeader(FILTERS_HEADER, requireNonNull(request.headers().get(FILTERS_HEADER))));
+             BlockingHttpClient client = filters.apply(HttpClients.forMultiAddressUrl())
+                     .buildBlocking()) {
+
+            HttpResponse response;
+            final String requestTarget = "http://" + serverHostAndPort(serverContext) + "/";
+            if (reservedConnection) {
+                ReservedBlockingHttpConnection conn = client.reserveConnection(client.get(requestTarget));
+                response = conn.request(conn.get(requestTarget));
+                conn.release();
+            } else {
+                response = client.request(client.get(requestTarget));
+            }
+            assertThat(response.status(), is(OK));
+            assertThat(response.headers().get(FILTERS_HEADER), contentEqualTo(expectedValue));
+        }
+    }
+
     private abstract static class ConditionalFilterFactory implements StreamingHttpClientFilterFactory,
                                                                       StreamingHttpConnectionFilterFactory {