prepare for production

Author: diegomarquezp

google-http-client-apache-v3/src/main/java/com/google/api/client/http/apache/v3/ApacheHttpRequest.java

@@ -18,8 +18,8 @@
 import com.google.api.client.http.LowLevelHttpResponse;
 import java.io.IOException;
 import java.util.concurrent.TimeUnit;
-import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
+import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.apache.hc.core5.util.Timeout;
 
@@ -35,10 +35,7 @@ final class ApacheHttpRequest extends LowLevelHttpRequest {
     this.httpClient = httpClient;
     this.request = request;
     // disable redirects as google-http-client handles redirects
-    this.requestConfig =
-        RequestConfig.custom()
-            .setRedirectsEnabled(false)
-    ;
+    this.requestConfig = RequestConfig.custom().setRedirectsEnabled(false);
   }
 
   @Override
@@ -48,23 +45,19 @@ public void addHeader(String name, String value) {
 
   @Override
   public void setTimeout(int connectTimeout, int readTimeout) throws IOException {
-    // TODO: these methods are deprecated - we will need a more up-to-date way of setting timeouts
-    // on existing requests. Also, since we can't control the lower level socket configuration,
-    // we indirectly set a read timeout via ResponseTimeout
+    // TODO: find a way to not use the @Deprecated setConnectTimeout method
     requestConfig
         .setConnectTimeout(Timeout.of(connectTimeout, TimeUnit.MILLISECONDS))
+        // ResponseTimeout behaves the same as 4.x's SocketTimeout
         .setResponseTimeout(Timeout.of(readTimeout, TimeUnit.MILLISECONDS));
   }
 
   @Override
   public LowLevelHttpResponse execute() throws IOException {
     if (getStreamingContent() != null) {
-      // Preconditions.checkState(
-      //     request instanceof HttpEntityEnclosingRequest,
-      //     "Apache HTTP client does not support %s requests with content.",
-      //     request.getRequestLine().getMethod());
-      ContentEntity entity = new ContentEntity(getContentLength(), getStreamingContent(),
-          getContentType(), getContentEncoding());
+      ContentEntity entity =
+          new ContentEntity(
+              getContentLength(), getStreamingContent(), getContentType(), getContentEncoding());
       request.setEntity(entity);
     }
     request.setConfig(requestConfig.build());

google-http-client-apache-v3/src/main/java/com/google/api/client/http/apache/v3/ApacheHttpResponse.java

@@ -102,7 +102,7 @@ public String getHeaderValue(int index) {
   /**
    * Aborts execution of the request.
    *
-   * @since 1.30
+   * @since 1.44
    */
   @Override
   public void disconnect() {

google-http-client-apache-v3/src/main/java/com/google/api/client/http/apache/v3/ApacheHttpTransport.java

@@ -21,7 +21,6 @@
 import java.io.IOException;
 import java.net.ProxySelector;
 import java.net.URI;
-import java.sql.Time;
 import java.util.concurrent.TimeUnit;
 import org.apache.hc.client5.http.classic.HttpClient;
 import org.apache.hc.client5.http.classic.methods.HttpDelete;
@@ -31,17 +30,17 @@
 import org.apache.hc.client5.http.classic.methods.HttpPatch;
 import org.apache.hc.client5.http.classic.methods.HttpPost;
 import org.apache.hc.client5.http.classic.methods.HttpPut;
-import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
 import org.apache.hc.client5.http.classic.methods.HttpTrace;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
 import org.apache.hc.client5.http.config.ConnectionConfig;
-import org.apache.hc.client5.http.impl.classic.HttpClients;
-import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
-import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
 import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
 import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
+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.SystemDefaultRoutePlanner;
-import org.apache.hc.core5.http.io.SocketConfig;
+import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
+import org.apache.hc.core5.io.CloseMode;
 
 /**
  * Thread-safe HTTP transport based on the Apache HTTP Client library.
@@ -51,12 +50,12 @@
  * applications should use a single globally-shared instance of the HTTP transport.
  *
  * <p>Default settings are specified in {@link #newDefaultHttpClient()}. Use the {@link
- * #ApacheHttpTransport(CloseableHttpClient)} constructor to override the Apache HTTP Client used. Please
- * read the <a
- * href="https://hc.apache.org/httpcomponents-client-4.5.x/current/tutorial/pdf/httpclient-tutorial.pdf">
- * Apache HTTP Client connection management tutorial</a> for more complex configuration options.
+ * #ApacheHttpTransport(CloseableHttpClient)} constructor to override the Apache HTTP Client used.
+ * Please read the <a
+ * href="https://github.com/apache/httpcomponents-client/blob/f898f1aca38f77f62a007856a674629cae5a02e6/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientConfiguration.java">
+ * Apache HTTP Client 5.x configuration example</a> for more complex configuration options.
  *
- * @since 1.30
+ * @since 1.44
  */
 public final class ApacheHttpTransport extends HttpTransport {
 
@@ -69,22 +68,20 @@ public final class ApacheHttpTransport extends HttpTransport {
   /**
    * Constructor that uses {@link #newDefaultHttpClient()} for the Apache HTTP client.
    *
-   * @since 1.30
+   * @since 1.44
    */
   public ApacheHttpTransport() {
     this(newDefaultHttpClient(), false);
   }
 
   /**
+   * {@link Beta} <br>
    * Constructor that allows an alternative Apache HTTP client to be used.
    *
-   * <p>Note that in the previous version, we overrode several settings. However, we are no longer
-   * able to do so.
-   *
    * <p>If you choose to provide your own Apache HttpClient implementation, be sure that
    *
    * <ul>
-   *   <li>Redirects are disabled (google-http-client handles redirects).
+   *   <li>HTTP version is set to 1.1.
    *   <li>Retries are disabled (google-http-client handles retries).
    * </ul>
    *
@@ -99,13 +96,10 @@ public ApacheHttpTransport(CloseableHttpClient httpClient) {
    * {@link Beta} <br>
    * Constructor that allows an alternative CLoseable Apache HTTP client to be used.
    *
-   * <p>Note that in the previous version, we overrode several settings. However, we are no longer
-   * able to do so.
-   *
    * <p>If you choose to provide your own Apache HttpClient implementation, be sure that
    *
    * <ul>
-   *   <li>Redirects are disabled (google-http-client handles redirects).
+   *   <li>HTTP version is set to 1.1.
    *   <li>Retries are disabled (google-http-client handles retries).
    * </ul>
    *
@@ -119,6 +113,7 @@ public ApacheHttpTransport(CloseableHttpClient httpClient, boolean isMtls) {
   }
 
   /**
+   * {@link Beta} <br>
    * Creates a new instance of the Apache HTTP client that is used by the {@link
    * #ApacheHttpTransport()} constructor.
    *
@@ -130,12 +125,12 @@ public ApacheHttpTransport(CloseableHttpClient httpClient, boolean isMtls) {
    *       HttpClientBuilder#disableRedirectHandling}.
    *   <li>The route planner uses {@link SystemDefaultRoutePlanner} with {@link
    *       ProxySelector#getDefault()}, which uses the proxy settings from <a
-   *       href="https://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
+   *       href="https://docs.oracle.com/javase/8/docs/api/java/net/doc-files/net-properties.html">system
    *       properties</a>.
    * </ul>
    *
    * @return new instance of the Apache HTTP client
-   * @since 1.30
+   * @since 1.44
    */
   public static CloseableHttpClient newDefaultHttpClient() {
     return newDefaultCloseableHttpClientBuilder().build();
@@ -149,26 +144,27 @@ public static CloseableHttpClient newDefaultHttpClient() {
    *
    * <ul>
    *   <li>The client connection manager is set to {@link PoolingHttpClientConnectionManager}.
-   *   <li><The retry mechanism is turned off using {@link
+   *   <li>The retry mechanism is turned off using {@link
    *       HttpClientBuilder#disableRedirectHandling}.
+   *   <li>Redirects are turned off using {@link HttpClientBuilder#disableAutomaticRetries()}.
    *   <li>The route planner uses {@link SystemDefaultRoutePlanner} with {@link
    *       ProxySelector#getDefault()}, which uses the proxy settings from <a
-   *       href="http://docs.oracle.com/javase/7/docs/api/java/net/doc-files/net-properties.html">system
+   *       href="http://docs.oracle.com/javase/8/docs/api/java/net/doc-files/net-properties.html">system
    *       properties</a>.
    * </ul>
    *
    * @return new instance of the Apache HTTP client
-   * @since 1.31
+   * @since 1.44
    */
   public static HttpClientBuilder newDefaultCloseableHttpClientBuilder() {
-    PoolingHttpClientConnectionManager connectionManager = PoolingHttpClientConnectionManagerBuilder.create()
-        .setSSLSocketFactory(SSLConnectionSocketFactory.getSocketFactory())
-        .setMaxConnTotal(200)
-        .setMaxConnPerRoute(20)
-        .setDefaultConnectionConfig(ConnectionConfig.custom()
-            .setTimeToLive(-1, TimeUnit.MILLISECONDS)
-            .build())
-        .build();
+    PoolingHttpClientConnectionManager connectionManager =
+        PoolingHttpClientConnectionManagerBuilder.create()
+            .setSSLSocketFactory(SSLConnectionSocketFactory.getSocketFactory())
+            .setMaxConnTotal(200)
+            .setMaxConnPerRoute(20)
+            .setDefaultConnectionConfig(
+                ConnectionConfig.custom().setTimeToLive(-1, TimeUnit.MILLISECONDS).build())
+            .build();
 
     return HttpClients.custom()
         .useSystemProperties()
@@ -209,20 +205,20 @@ protected ApacheHttpRequest buildRequest(String method, String url) {
   }
 
   /**
-   * Shuts down the connection manager and releases allocated resources. This closes all
+   * Gracefully shuts down the connection manager and releases allocated resources. This closes all
    * connections, whether they are currently used or not.
    *
-   * @since 1.30
+   * @since 1.44
    */
   @Override
   public void shutdown() throws IOException {
-    httpClient.close();
+    httpClient.close(CloseMode.GRACEFUL);
   }
 
   /**
    * Returns the Apache HTTP client.
    *
-   * @since 1.30
+   * @since 1.44
    */
   public HttpClient getHttpClient() {
     return httpClient;

google-http-client-apache-v3/src/main/java/com/google/api/client/http/apache/v3/ContentEntity.java

@@ -19,7 +19,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.io.entity.AbstractHttpEntity;
 
 final class ContentEntity extends AbstractHttpEntity {
@@ -34,7 +33,11 @@ final class ContentEntity extends AbstractHttpEntity {
    * @param contentLength content length or less than zero if not known
    * @param streamingContent streaming content
    */
-  ContentEntity(long contentLength, StreamingContent streamingContent, String contentType, String contentEncoding) {
+  ContentEntity(
+      long contentLength,
+      StreamingContent streamingContent,
+      String contentType,
+      String contentEncoding) {
     super(contentType, contentEncoding, contentLength == -1);
     this.contentLength = contentLength;
     this.streamingContent = Preconditions.checkNotNull(streamingContent);
@@ -68,6 +71,5 @@ public void writeTo(OutputStream out) throws IOException {
   }
 
   @Override
-  public void close() throws IOException {
-  }
+  public void close() throws IOException {}
 }

google-http-client-apache-v3/src/test/java/com/google/api/client/http/apache/v3/ApacheHttpRequestTest.java

@@ -14,18 +14,7 @@
 
 package com.google.api.client.http.apache.v3;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 
-import com.google.api.client.http.ByteArrayContent;
-import com.google.api.client.http.HttpContent;
-import com.google.api.client.http.InputStreamContent;
-import com.google.api.client.testing.http.apache.MockHttpClient;
-import java.io.ByteArrayInputStream;
-import java.nio.charset.StandardCharsets;
-import java.util.Arrays;
-import org.junit.Test;
 
 public class ApacheHttpRequestTest {
 

google-http-client-apache-v3/src/test/java/com/google/api/client/http/apache/v3/ApacheHttpTransportTest.java

@@ -23,33 +23,46 @@
 import com.google.auth.http.HttpCredentialsAdapter;
 import com.google.auth.oauth2.GoogleCredentials;
 import java.io.IOException;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hc.client5.http.impl.classic.HttpClients;
 import org.junit.Test;
 
-/**
- * Tests {@link ApacheHttpTransport}.
- */
+/** Tests {@link ApacheHttpTransport}. */
 public class ApacheHttpTransportTest {
 
-  @Test
-  public void testClientUsingApacheV3Transport() throws IOException {
+  public void testRequest(HttpTransport transport) throws IOException {
     final String PROJECT_ID = System.getenv("PROJECT_ID");
-    HttpTransport transport = new ApacheHttpTransport();
 
     GoogleCredentials credentials = GoogleCredentials.getApplicationDefault();
     GsonFactory jsonFactory = GsonFactory.getDefaultInstance();
     CloudResourceManager.Builder resourceManagerBuilder =
         new CloudResourceManager.Builder(
-            transport, jsonFactory, new HttpCredentialsAdapter(credentials))
+                transport, jsonFactory, new HttpCredentialsAdapter(credentials))
             .setApplicationName("Example Java App");
     CloudResourceManager cloudResourceManager = resourceManagerBuilder.build();
 
     Projects projects = cloudResourceManager.projects();
-    Get get = projects.get("projects/"+PROJECT_ID);
+    Get get = projects.get("projects/" + PROJECT_ID);
     Project project = get.execute();
     System.out.println("Project display name: " + project.getDisplayName());
   }
 
-  // private static class MockHttpResponse extends BasicHttpResponse implements CloseableHttpResponse {
+  @Test
+  public void testClientUsingDefaultApacheV3Transport() throws IOException {
+    HttpTransport transport = new ApacheHttpTransport();
+    testRequest(transport);
+  }
+
+  @Test
+  public void testClientUsingApacheV3TransportWithCustomHttpClient() throws IOException {
+    CloseableHttpClient client =
+        HttpClients.custom().disableAutomaticRetries().disableRedirectHandling().build();
+    HttpTransport transport = new ApacheHttpTransport(client);
+    testRequest(transport);
+  }
+
+  // private static class MockHttpResponse extends BasicHttpResponse implements
+  // CloseableHttpResponse {
   //   public MockHttpResponse() {
   //     super(200, "OK");
   //   }
@@ -86,7 +99,8 @@ transport, jsonFactory, new HttpCredentialsAdapter(credentials))
   //
   // private void checkHttpClient(HttpClient client) {
   //   assertNotNull(client);
-  //   // TODO(chingor): Is it possible to test this effectively? The newer HttpClient implementations
+  //   // TODO(chingor): Is it possible to test this effectively? The newer HttpClient
+  // implementations
   //   // are read-only and we're testing that we built the client with the right configuration
   // }
   //