Better Access to Response Headers

[sedouard]

On a successful API request the Stripe Java SDK doesn't provide a way of accessing response headers. (At least not that I can tell).

This can be useful in the case you're interested in response metadata (such as Idempotency-Key and Request-ID).

For example in a GET /customers/:id request:

Customer customer = Customer.retrieve("cus_BhemmnI4d8eFjR");
System.out.println(customer);

The model doesn't provide any way to grab response headers from a given request (at least from what I can tell). However StripeResponse which the model is serialized from does.

It would be great if the experience could allow an optional way to include request metadata in the model response. Something like this:

Customer customer = Customer.retrieve("cus_BhemmnI4d8eFjR", true);
System.out.println(customer);
System.out.println(customer.getHeaders())

Because nearly all StripeObjects represent some kind of API response, I'm thinking it's a good place to add a private headers field plus getters/setters.

Afterwards depending on some kind of optional flag, we can include these headers to the serialized response model by safely casting to a StripeObject and setting it within the _request method inside of LiveStripeResponseGetter.

Would love to hear your thoughts.

[brandur-stripe]

+1.

I'd prefer to add something like a getResponse accessor (I like stripe-ruby's method the best where responses are available through a separate style of API call, but failing that) instead of doing it piecemeal by augment StripeObject with individual parts of the request. It's possible that someone may want access to the raw body or a status code at some point, and it'll be easier to do if we just have the whole of the response modeled. (It also has the advantage of following some precedent established by stripe-node.)

[sedouard]

Agreed that we should just expose the response instead of cherry picking things out of the response.

@brandur-stripe - Where are you suggesting getResponse live? Because every API response is deserialized to some sub-type of StripeObject I figured it should go there. Should getResponse return a StripeResponse?

[sedouard]

(also feel free to assign this to me as I'm happy to do a PR here)

[brandur-stripe]

@brandur-stripe - Where are you suggesting getResponse live? Because every API response is deserialized to some sub-type of StripeObject I figured it should go there.

Yeah, I think that's plausible given that we already have something similar in stripe-node, and it'd be suitable for most people's needs. That said, it might also be a good idea just to spend a few minutes thinking about alternatives just in case there's something better that we could do.

Should getResponse return a StripeResponse?

Reusing StripeResponse seems like a good idea. That said, given that we're exposing this as a new API, we should make sure that it's exposed in a way that we think is good, and to that end it might be worth taking a look at how response objects are modeled elsewhere in the Java world (say OKHTTP for example) and just see if there's anything better that we could do.

Just off the top of my head:

• The naming of getResponseCode and getResponseBody feels a little redundant. We already know that it's a StripeResponse so it shouldn't be necessary to have the word "response" in every method.
• The return value of Map<String, List<String>> for getResponseHeaders() is good because it's technically correct given that you can double up header names in HTTP, but it's also not how most people want to access information most of the time (you'll have something like getResponseHeaders().get("Idempotency-Key").get(0) because you almost always want the first header value everywhere). We should make sure that a header with multiple values is still accessible, but it might be worth adding a convenience helper to just get the first value because that's what most people want (I think Go's net/http does a pretty good job of this if you want to take a look).

And once again, coordinate with Olivier and company first, but it would be awesome if you could take this.

[sedouard]

@ob-stripe throw out a quick pr #421. Let's discuss the details there.

[ob-stripe]

Implemented in #421 and released in v5.24.0.