Wrap GZIPInputStream with a ConsumingInputStream to drain on close #749

chingor13 · 2019-07-15T23:40:15Z

Fixes #367

elharo · 2019-07-29T13:29:59Z

google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java

@@ -356,6 +357,44 @@ public InputStream getContent() throws IOException {
    return content;
  }

+  static class ConsumingInputStream extends InputStream {


An outer class would better separate unrelated code. This is complex enough to justify it.

elharo · 2019-07-29T13:31:01Z

google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java

+    public void close() throws IOException {
+      if (!closed && inputStream != null) {
+        try {
+          drainInputStream(this);


Why is it important to drain the input stream and throw away the excess data, instead of simply closing the stream?

elharo · 2019-07-29T13:33:08Z

google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java

+    try (ByteArrayOutputStream byteStream = new ByteArrayOutputStream(dataToCompress.length)) {
+      GZIPOutputStream zipStream = new GZIPOutputStream((byteStream));
+      zipStream.write(dataToCompress);
+      zipStream.close();


We should probably also have a case where there's extra data on the stream after the gzipped content, since handling that is the purpose of this PR.

elharo · 2019-07-29T13:34:59Z

google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java

+      }
+    }
+
+    static void drainInputStream(final InputStream inputStream) throws IOException {


I think this method can be private. Also you've reinvented ByteStreams.exhaust from Guava.

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. Checkpicked from: googleapis#749 Fixes: googleapis#367

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: googleapis#749 Fixes: googleapis#367

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

* fix: timing of stale token refreshes on ComputeEngine ComputeEngine metadata server has its own token caching mechanism that will return a cached token until the last 5 minutes of its expiration. This has a negative interaction with stale token refreshes because stale token refresh kicks in T-6mins until T-5mins. This will cause every stale refresh to return the same stale token. This PR updates the timing for ComputeEngineCredentials to start a stale refresh at T-4mins and consider the token expired at T-3 mins. The implementation is a bit noisy because it includes a change OAuth2Credentials to make the thresholds configureable and now that we targeting java8, I migrated to using java8 time data types * fmt * fix test * fix test again * remove debug code * comments

Wrap GZIPInputStream with a ConsumingInputStream to drain on close

9a3a9b1

chingor13 requested a review from a team as a code owner July 15, 2019 23:40

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 15, 2019

Remove unused import

f621038

elharo reviewed Jul 29, 2019

View reviewed changes

codyoss mentioned this pull request Oct 14, 2019

feat: wrap GZIPInputStream for connection reuse #840

Merged

chingor13 closed this Oct 22, 2019

chingor13 deleted the chunked-keepalive branch October 22, 2019 00:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap GZIPInputStream with a ConsumingInputStream to drain on close #749

Wrap GZIPInputStream with a ConsumingInputStream to drain on close #749

chingor13 commented Jul 15, 2019

elharo Jul 29, 2019

elharo Jul 29, 2019

elharo Jul 29, 2019

elharo Jul 29, 2019

Wrap GZIPInputStream with a ConsumingInputStream to drain on close #749

Wrap GZIPInputStream with a ConsumingInputStream to drain on close #749

Conversation

chingor13 commented Jul 15, 2019

elharo Jul 29, 2019

Choose a reason for hiding this comment

elharo Jul 29, 2019

Choose a reason for hiding this comment

elharo Jul 29, 2019

Choose a reason for hiding this comment

elharo Jul 29, 2019

Choose a reason for hiding this comment