Add StreamUtils class#935
Conversation
|
(Also I have vague hopes that this will help with some of the transient build failures we're observing on Travis from time to time, but I'm not holding my breath.) |
brandur-stripe
left a comment
brandur-stripe
left a comment
There was a problem hiding this comment.
I'll just inject two general points that reading to the end of the stream can be a little risky in the case of:
- Huge response bodies (say you were reading back a large file from S3).
- Streaming APIs that don't immediately terminate a response.
However, we're just dealing with trusted endpoints here, and it's no worse than before anyway, so not a big deal. LGTM!
|
Thanks Brandur! And yes, good point about this being potentially dangerous. Gson does have support for streams, so I think in theory we could deserialize the stream directly into a model, without first loading the entire stream into memory as a string. In practice I don't think we need to bother with this for the reasons you listed. |
* Refactor form encoding * Refactor request telemetry * Move HTTP request methods into new `HttpClient` class * Add `StripeRequest` object * Add `HttpClient` abstract class * Stop disabling the DNS cache * Fix deprecation warnings (#895) * Add HttpContent class (#896) * Add Stopwatch class (#897) * Move all request properties in `StripeRequest` (#898) * Remove ApiResource.RequestType (#899) * Add support for automatic request retries (#900) * Minor fixes (#902) * `StringUtils` class & better API key validation (#928) * Remove support for custom `URLStreamHandler` (#927) * Refactor HTTP headers handling (#931) * Add `CaseInsensitiveMap` class * Add `HttpHeaders` class * Use `HttpHeaders` in `StripeRequest` * Use `HttpHeaders` in `StripeResponse` * Address review comments * Modernize `StripeResponse` (#932) * Add `maxNetworkRetries` as a global and per-request setting (#934) * Add `StreamUtils` class (#935) * Remove support for `count` and `total_count` in list objects (#936) * Codegen for openapi e07de1a (#938) * Update README (#939) Co-authored-by: remi-stripe <remi@stripe.com>
4 participants
r? @brandur-stripe @richardm-stripe
cc @stripe/api-libraries
Amazingly, Java doesn't provide a standard way of extracting the contents of an
InputStreaminto aString. There are many ways of doing so, this StackOverflow answer shows and compares 11 (!) different ways, some of them relying on third-party libraries.Previously we were using #3, i.e. using
Scannerto tokenize the stream with\Aas the delimiter to tokenize the entire stream into a single string. This 1. feels hacky and 2. has pretty bad performance.In this PR I added a new utility method
StreamUtils.readToEnd()that uses #6, i.e. usingStringBuilderandInputStreamReader, which feel like the right tools for the job (and should have better performance too).Tagged as breaking because I changed the type of
ApiResource.CHARSETfromStringtoCharset, as well as the constructor ofMultipartProcessor, though this shouldn't impact many users.