Handled Surface API review comments

Author: joviegas

.brazil.json

@@ -4,7 +4,7 @@
     "modules": {
         "annotations": { "packageName": "AwsJavaSdk-Core-Annotations" },
         "apache-client": { "packageName": "AwsJavaSdk-HttpClient-ApacheClient" },
-        "apache5-client": { "packageName": "AwsJavaSdk-HttpClient-Apache5Client-preview" },
+        "apache5-client": { "packageName": "AwsJavaSdk-HttpClient-Apache5Client-Preview" },
         "arns": { "packageName": "AwsJavaSdk-Core-Arns" },
         "auth": { "packageName": "AwsJavaSdk-Core-Auth" },
         "auth-crt": { "packageName": "AwsJavaSdk-Core-AuthCrt" },

http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java

@@ -25,6 +25,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.net.InetAddress;
 import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
@@ -48,6 +49,7 @@
 import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
 import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
+import org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner;
 import org.apache.hc.client5.http.io.HttpClientConnectionManager;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.client5.http.routing.HttpRoutePlanner;
@@ -57,10 +59,13 @@
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpEntity;
+import org.apache.hc.core5.http.HttpException;
+import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpResponse;
 import org.apache.hc.core5.http.config.Registry;
 import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
 import org.apache.hc.core5.http.io.SocketConfig;
+import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.pool.PoolStats;
 import org.apache.hc.core5.ssl.SSLInitializationException;
@@ -108,7 +113,7 @@
 @SdkPublicApi
 public final class Apache5HttpClient implements SdkHttpClient {
 
-    public static final String CLIENT_NAME = "Apache5";
+    public static final String CLIENT_NAME = "Apache5Preview";
 
     private static final Logger log = Logger.loggerFor(Apache5HttpClient.class);
     private static final HostnameVerifier DEFAULT_HOSTNAME_VERIFIER = new DefaultHostnameVerifier();
@@ -206,7 +211,12 @@ private void addProxyConfig(HttpClientBuilder builder,
         }
 
         if (routePlanner != null) {
+            if (configuration.localAddress != null) {
+                log.debug(() -> "localAddress configuration was ignored since Route planner was explicitly provided");
+            }
             builder.setRoutePlanner(routePlanner);
+        } else if (configuration.localAddress != null) {
+            builder.setRoutePlanner(new LocalAddressRoutePlanner(configuration.localAddress));
         }
 
         if (credentialsProvider != null) {
@@ -404,6 +414,11 @@ public interface Builder extends SdkHttpClient.Builder<Apache5HttpClient.Builder
          */
         Builder proxyConfiguration(ProxyConfiguration proxyConfiguration);
 
+        /**
+         * Configure the local address that the HTTP client should use for communication.
+         */
+        Builder localAddress(InetAddress localAddress);
+
         /**
          * Configure whether the client should send an HTTP expect-continue handshake before each request.
          */
@@ -495,6 +510,7 @@ private static final class DefaultBuilder implements Builder {
         private final AttributeMap.Builder standardOptions = AttributeMap.builder();
         private Registry<AuthSchemeFactory> authSchemeRegistry;
         private ProxyConfiguration proxyConfiguration = ProxyConfiguration.builder().build();
+        private InetAddress localAddress;
         private Boolean expectContinueEnabled;
         private HttpRoutePlanner httpRoutePlanner;
         private CredentialsProvider credentialsProvider;
@@ -560,6 +576,16 @@ public void setProxyConfiguration(ProxyConfiguration proxyConfiguration) {
             proxyConfiguration(proxyConfiguration);
         }
 
+        @Override
+        public Builder localAddress(InetAddress localAddress) {
+            this.localAddress = localAddress;
+            return this;
+        }
+
+        public void setLocalAddress(InetAddress localAddress) {
+            localAddress(localAddress);
+        }
+
         @Override
         public Builder expectContinueEnabled(Boolean expectContinueEnabled) {
             this.expectContinueEnabled = expectContinueEnabled;
@@ -794,4 +820,18 @@ private SocketConfig buildSocketConfig(AttributeMap standardOptions) {
         }
 
     }
+
+    private static class LocalAddressRoutePlanner extends DefaultRoutePlanner {
+        private final InetAddress localAddress;
+
+        LocalAddressRoutePlanner(InetAddress localAddress) {
+            super(DefaultSchemePortResolver.INSTANCE);
+            this.localAddress = localAddress;
+        }
+
+        @Override
+        protected InetAddress determineLocalAddress(HttpHost firstHop, HttpContext context) throws HttpException {
+            return localAddress;
+        }
+    }
 }

http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientLocalAddressOnBuilderFunctionalTest.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file 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 software.amazon.awssdk.http.apache5;
+
+import java.net.InetAddress;
+import java.time.Duration;
+import org.junit.jupiter.api.DisplayName;
+import software.amazon.awssdk.http.SdkHttpClient;
+import software.amazon.awssdk.http.SdkHttpClientLocalAddressFunctionalTestSuite;
+
+@DisplayName("Apache5 HTTP Client - Local Address Functional Tests")
+class Apache5HttpClientLocalAddressOnBuilderFunctionalTest extends SdkHttpClientLocalAddressFunctionalTestSuite {
+
+    @Override
+    protected SdkHttpClient createHttpClient(InetAddress localAddress, Duration connectionTimeout) {
+
+        return Apache5HttpClient.builder()
+                                .localAddress(localAddress)
+                                .connectionTimeout(connectionTimeout)
+                                .build();
+    }
+}

http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientLocalAddressFunctionalTest.java renamed to http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/Apache5HttpClientLocalAddressRoutePlannerFunctionalTest.java

@@ -28,7 +28,7 @@
 import software.amazon.awssdk.http.SdkHttpClientLocalAddressFunctionalTestSuite;
 
 @DisplayName("Apache5 HTTP Client - Local Address Functional Tests")
-class Apache5HttpClientLocalAddressFunctionalTest extends SdkHttpClientLocalAddressFunctionalTestSuite {
+class Apache5HttpClientLocalAddressRoutePlannerFunctionalTest extends SdkHttpClientLocalAddressFunctionalTestSuite {
 
     @Override
     protected SdkHttpClient createHttpClient(InetAddress localAddress, Duration connectionTimeout) {

http-clients/apache5-client/src/test/java/software/amazon/awssdk/http/apache5/MetricReportingTest.java

@@ -81,7 +81,7 @@ public void prepareRequest_callableCalled_metricsReported() throws IOException {
 
         client.prepareRequest(executeRequest).call();
         MetricCollection collected = collector.collect();
-        assertThat(collected.metricValues(HTTP_CLIENT_NAME)).containsExactly("Apache5");
+        assertThat(collected.metricValues(HTTP_CLIENT_NAME)).containsExactly("Apache5Preview");
         assertThat(collected.metricValues(LEASED_CONCURRENCY)).containsExactly(1);
         assertThat(collected.metricValues(PENDING_CONCURRENCY_ACQUIRES)).containsExactly(2);
         assertThat(collected.metricValues(AVAILABLE_CONCURRENCY)).containsExactly(3);
@@ -99,7 +99,7 @@ public void prepareRequest_connectionManagerNotPooling_callableCalled_metricsRep
 
         MetricCollection collected = collector.collect();
 
-        assertThat(collected.metricValues(HTTP_CLIENT_NAME)).containsExactly("Apache5");
+        assertThat(collected.metricValues(HTTP_CLIENT_NAME)).containsExactly("Apache5Preview");
         assertThat(collected.metricValues(LEASED_CONCURRENCY)).isEmpty();
         assertThat(collected.metricValues(PENDING_CONCURRENCY_ACQUIRES)).isEmpty();
         assertThat(collected.metricValues(AVAILABLE_CONCURRENCY)).isEmpty();