-
Notifications
You must be signed in to change notification settings - Fork 451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrap GZIPInputStream with a ConsumingInputStream to drain on close #749
Conversation
@@ -356,6 +357,44 @@ public InputStream getContent() throws IOException { | |||
return content; | |||
} | |||
|
|||
static class ConsumingInputStream extends InputStream { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An outer class would better separate unrelated code. This is complex enough to justify it.
public void close() throws IOException { | ||
if (!closed && inputStream != null) { | ||
try { | ||
drainInputStream(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it important to drain the input stream and throw away the excess data, instead of simply closing the stream?
try (ByteArrayOutputStream byteStream = new ByteArrayOutputStream(dataToCompress.length)) { | ||
GZIPOutputStream zipStream = new GZIPOutputStream((byteStream)); | ||
zipStream.write(dataToCompress); | ||
zipStream.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
} | ||
} | ||
|
||
static void drainInputStream(final InputStream inputStream) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
* 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
🤖 I have created a release \*beep\* \*boop\* --- ## [1.2.0](https://www.github.com/googleapis/google-auth-library-java/compare/v1.1.0...v1.2.0) (2021-09-30) ### Features * add support for Workforce Pools ([googleapis#729](https://www.github.com/googleapis/google-auth-library-java/issues/729)) ([5f3fed7](https://www.github.com/googleapis/google-auth-library-java/commit/5f3fed79e22f3c2d585c5b03c01791b0f8109929)) ### Bug Fixes * allow empty workforce_pool_user_project ([googleapis#752](https://www.github.com/googleapis/google-auth-library-java/issues/752)) ([e1cbce1](https://www.github.com/googleapis/google-auth-library-java/commit/e1cbce1a5cb269c6613bc6d40f06145bd45099c0)) * timing of stale token refreshes on ComputeEngine ([googleapis#749](https://www.github.com/googleapis/google-auth-library-java/issues/749)) ([c813d55](https://www.github.com/googleapis/google-auth-library-java/commit/c813d55a78053ecbec1a9640e6c9814da87319eb)) * workforce audience ([googleapis#741](https://www.github.com/googleapis/google-auth-library-java/issues/741)) ([a08cacc](https://www.github.com/googleapis/google-auth-library-java/commit/a08cacc7990b9058c8f1af3f9d8d816119562cc4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Fixes #367