Repository: googleapis/google-http-java-client
Pull request #840 (2019-10-14 18:33:12)

feat: wrap GZIPInputStream for connection reuse
If a connection is closed and there are some bytes that have not
been read that connection can't be reused. Now GZIPInputStream
will have all of its bytes read on close automatically to promote
connection reuse.

Cherry-picked: #749
Fixes: #367

codyoss: Noticed the #749 had not been updated in a while so I cherry picked and fixed the PR comments.
codyoss: @elharo I fixed all of the comments. Do you still have reservations around a possible infinite stream? If so, any thought on how we should move forward with this?(Maybe we just don't if we think the infinite stream is too big of a risk as this feature requires exhausting all of the extra bytes for it to work.)

Patch:
diff --git a/.kokoro/build.sh b/.kokoro/build.sh
index 2ffb5ef7..bcd1e410 100755
--- a/.kokoro/build.sh
+++ b/.kokoro/build.sh
@@ -39,6 +39,7 @@ case ${JOB_TYPE} in
 test)
     mvn test -B
     bash ${KOKORO_GFILE_DIR}/codecov.sh
+    bash .kokoro/coerce_logs.sh
     ;;
 lint)
     mvn com.coveo:fmt-maven-plugin:check
@@ -48,6 +49,7 @@ javadoc)
     ;;
 integration)
     mvn -B ${INTEGRATION_TEST_ARGS} -DtrimStackTrace=false -fae verify
+    bash .kokoro/coerce_logs.sh
     ;;
 *)
     ;;
diff --git a/.kokoro/coerce_logs.sh b/.kokoro/coerce_logs.sh
new file mode 100755
index 00000000..5cf7ba49
--- /dev/null
+++ b/.kokoro/coerce_logs.sh
@@ -0,0 +1,38 @@
+#!/bin/bash
+# Copyright 2019 Google LLC
+#
+# 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.
+
+# This script finds and moves sponge logs so that they can be found by placer
+# and are not flagged as flaky by sponge.
+
+set -eo pipefail
+
+## Get the directory of the build script
+scriptDir=$(realpath $(dirname "${BASH_SOURCE[0]}"))
+## cd to the parent directory, i.e. the root of the git repo
+cd ${scriptDir}/..
+
+job=$(basename ${KOKORO_JOB_NAME})
+
+echo "coercing sponge logs..."
+for xml in `find . -name *-sponge_log.xml`
+do
+  echo "processing ${xml}"
+  class=$(basename ${xml} | cut -d- -f2)
+  dir=$(dirname ${xml})/${job}/${class}
+  text=$(dirname ${xml})/${class}-sponge_log.txt
+  mkdir -p ${dir}
+  mv ${xml} ${dir}/sponge_log.xml
+  mv ${text} ${dir}/sponge_log.txt
+done
diff --git a/.kokoro/continuous/common.cfg b/.kokoro/continuous/common.cfg
index a5178e08..5ba7070f 100644
--- a/.kokoro/continuous/common.cfg
+++ b/.kokoro/continuous/common.cfg
@@ -4,6 +4,7 @@
 action {
   define_artifacts {
     regex: "**/*sponge_log.xml"
+    regex: "**/*sponge_log.txt"
   }
 }
 
diff --git a/.kokoro/nightly/common.cfg b/.kokoro/nightly/common.cfg
index a5178e08..5ba7070f 100644
--- a/.kokoro/nightly/common.cfg
+++ b/.kokoro/nightly/common.cfg
@@ -4,6 +4,7 @@
 action {
   define_artifacts {
     regex: "**/*sponge_log.xml"
+    regex: "**/*sponge_log.txt"
   }
 }
 
diff --git a/.kokoro/presubmit/common.cfg b/.kokoro/presubmit/common.cfg
index 709e429b..ad5913e4 100644
--- a/.kokoro/presubmit/common.cfg
+++ b/.kokoro/presubmit/common.cfg
@@ -4,6 +4,7 @@
 action {
   define_artifacts {
     regex: "**/*sponge_log.xml"
+    regex: "**/*sponge_log.txt"
   }
 }
 
diff --git a/.kokoro/release/snapshot.sh b/.kokoro/release/snapshot.sh
index bf738c56..098168a7 100755
--- a/.kokoro/release/snapshot.sh
+++ b/.kokoro/release/snapshot.sh
@@ -19,6 +19,9 @@ source $(dirname "$0")/common.sh
 MAVEN_SETTINGS_FILE=$(realpath $(dirname "$0")/../../)/settings.xml
 pushd $(dirname "$0")/../../
 
+# ensure we're trying to push a snapshot (no-result returns non-zero exit code)
+grep SNAPSHOT versions.txt
+
 setup_environment_secrets
 create_settings_xml_file "settings.xml"
 
@@ -27,4 +30,4 @@ mvn clean install deploy -B \
   -DperformRelease=true \
   -Dgpg.executable=gpg \
   -Dgpg.passphrase=${GPG_PASSPHRASE} \
-  -Dgpg.homedir=${GPG_HOMEDIR}
\ No newline at end of file
+  -Dgpg.homedir=${GPG_HOMEDIR}
diff --git a/.kokoro/release/stage.sh b/.kokoro/release/stage.sh
index b1b1b01c..3c482cbc 100755
--- a/.kokoro/release/stage.sh
+++ b/.kokoro/release/stage.sh
@@ -28,6 +28,7 @@ create_settings_xml_file "settings.xml"
 
 mvn clean install deploy -B \
   --settings ${MAVEN_SETTINGS_FILE} \
+  -DskipTests=true \
   -DperformRelease=true \
   -Dgpg.executable=gpg \
   -Dgpg.passphrase=${GPG_PASSPHRASE} \
diff --git a/google-http-client/src/main/java/com/google/api/client/http/ConsumingInputStream.java b/google-http-client/src/main/java/com/google/api/client/http/ConsumingInputStream.java
new file mode 100644
index 00000000..f0170a61
--- /dev/null
+++ b/google-http-client/src/main/java/com/google/api/client/http/ConsumingInputStream.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2019 Google LLC
+ *
+ * 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
+ *
+ *     https://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 com.google.api.client.http;
+
+import com.google.common.io.ByteStreams;
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * This class in meant to wrap an {@link InputStream} so that all bytes in the steam are read and
+ * discarded on {@link InputStream#close()}. This ensures that the underlying connection has the
+ * option to be reused.
+ */
+final class ConsumingInputStream extends FilterInputStream {
+  private boolean closed = false;
+
+  ConsumingInputStream(InputStream inputStream) {
+    super(inputStream);
+  }
+
+  @Override
+  public void close() throws IOException {
+    if (!closed && in != null) {
+      try {
+        ByteStreams.exhaust(this);
+        super.in.close();
+      } finally {
+        this.closed = true;
+      }
+    }
+  }
+}
diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
index 90c3812f..d972ab0d 100644
--- a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
+++ b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
@@ -331,7 +331,8 @@ public final class HttpResponse {
           if (!returnRawInputStream
               && contentEncoding != null
               && contentEncoding.contains("gzip")) {
-            lowLevelResponseContent = new GZIPInputStream(lowLevelResponseContent);
+            lowLevelResponseContent =
+                new ConsumingInputStream(new GZIPInputStream(lowLevelResponseContent));
           }
           // logging (wrap content with LoggingInputStream)
           Logger logger = HttpTransport.LOGGER;
diff --git a/pom.xml b/pom.xml
index 6ffe61f0..f97a4f7d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -234,7 +234,7 @@
       <dependency>
         <groupId>mysql</groupId>
         <artifactId>mysql-connector-java</artifactId>
-        <version>8.0.17</version>
+        <version>8.0.18</version>
       </dependency>
       <dependency>
         <groupId>com.google.j2objc</groupId>
diff --git a/synth.metadata b/synth.metadata
index b4ed968a..556bc58a 100644
--- a/synth.metadata
+++ b/synth.metadata
@@ -1,11 +1,11 @@
 {
-  "updateTime": "2019-10-10T08:02:08.936565Z",
+  "updateTime": "2019-10-18T07:51:39.452073Z",
   "sources": [
     {
       "template": {
         "name": "java_library",
         "origin": "synthtool.gcp",
-        "version": "2019.5.2"
+        "version": "2019.10.17"
       }
     }
   ]


Test patch:
diff --git a/google-http-client/src/test/java/com/google/api/client/http/ConsumingInputStreamTest.java b/google-http-client/src/test/java/com/google/api/client/http/ConsumingInputStreamTest.java
new file mode 100644
index 00000000..d55b5a0d
--- /dev/null
+++ b/google-http-client/src/test/java/com/google/api/client/http/ConsumingInputStreamTest.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2019 Google LLC
+ *
+ * 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
+ *
+ *     https://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 com.google.api.client.http;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import org.junit.Test;
+
+public class ConsumingInputStreamTest {
+
+  @Test
+  public void testClose_drainsBytesOnClose() throws IOException {
+    MockInputStream mockInputStream = new MockInputStream("abc123".getBytes(StandardCharsets.UTF_8));
+    InputStream consumingInputStream = new ConsumingInputStream(mockInputStream);
+
+    assertEquals(6, mockInputStream.getBytesToRead());
+
+    // read one byte
+    consumingInputStream.read();
+    assertEquals(5, mockInputStream.getBytesToRead());
+
+    // closing the stream should read the remaining bytes
+    consumingInputStream.close();
+    assertEquals(0, mockInputStream.getBytesToRead());
+  }
+
+  private class MockInputStream extends InputStream {
+    private int bytesToRead;
+
+    MockInputStream(byte[] data) {
+      this.bytesToRead = data.length;
+    }
+
+    @Override
+    public int read() throws IOException {
+      if (bytesToRead == 0) {
+        return -1;
+      }
+      bytesToRead--;
+      return 1;
+    }
+
+    int getBytesToRead() {
+      return bytesToRead;
+    }
+  }
+}
diff --git a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
index 7846778e..a611d774 100644
--- a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
+++ b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
@@ -26,10 +26,12 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.lang.reflect.Type;
+import java.nio.charset.StandardCharsets;
 import java.text.NumberFormat;
 import java.util.Arrays;
 import java.util.logging.Level;
 import java.util.zip.GZIPInputStream;
+import java.util.zip.GZIPOutputStream;
 import junit.framework.TestCase;
 
 /**
@@ -457,4 +459,40 @@ public class HttpResponseTest extends TestCase {
         "it should not decompress stream",
         request.execute().getContent() instanceof GZIPInputStream);
   }
+
+  public void testGetContent_gzipEncoding_finishReading() throws IOException {
+    byte[] dataToCompress = "abcd".getBytes(StandardCharsets.UTF_8);
+    byte[] mockBytes;
+    try (ByteArrayOutputStream byteStream = new ByteArrayOutputStream(dataToCompress.length)) {
+      GZIPOutputStream zipStream = new GZIPOutputStream((byteStream));
+      zipStream.write(dataToCompress);
+      zipStream.close();
+      mockBytes = byteStream.toByteArray();
+    }
+    final MockLowLevelHttpResponse mockResponse = new MockLowLevelHttpResponse();
+    mockResponse.setContent(mockBytes);
+    mockResponse.setContentEncoding("gzip");
+    mockResponse.setContentType("text/plain");
+
+    HttpTransport transport =
+        new MockHttpTransport() {
+          @Override
+          public LowLevelHttpRequest buildRequest(String method, final String url)
+              throws IOException {
+            return new MockLowLevelHttpRequest() {
+              @Override
+              public LowLevelHttpResponse execute() throws IOException {
+                return mockResponse;
+              }
+            };
+          }
+        };
+    HttpRequest request =
+        transport.createRequestFactory().buildHeadRequest(HttpTesting.SIMPLE_GENERIC_URL);
+    HttpResponse response = request.execute();
+    TestableByteArrayInputStream output = (TestableByteArrayInputStream) mockResponse.getContent();
+    assertFalse(output.isClosed());
+    assertEquals("abcd", response.parseAsString());
+    assertTrue(output.isClosed());
+  }
 }