Version is missing in the response headers

[nicolaa]

Problem: ApiVersionConstraint requires clients to send Accept request headers that specify the API version, for example: Accept: "application/json; version=2". However, the API response does not match this expected behavior.

Solution: Add a default header to Api::ApiController that responds with the correct version.

For example: Content-Type: "application/json; version=2"

[Aliciawyse]

Hey there @nicolaa!

I'm working on this issue now (with a mentor through a program called RubyMe). I'll be sending a PR very soon! :-)

[brettfishman]

Hi @nicolaa - can you say a bit more about why this is a problem?

[nicolaa]

@brettfishman This actually came up when one of our service consumers wrote pact expectations for header values. They were calling the v2 endpoint and specifying application/json; version=2 for content-type and accept headers in the request, and expecting version=2 to appear in the content type response header. Since this is something we don't actually read in the application, we just removed those expectations; however, it did reveal that stitches is not propagating the information correctly.

[brettfishman]

@nicolaa Ah, got it. Thanks for adding the additional context. I agree that it seems appropriate for the server to acknowledge the version returned in the Content-Type response header.

UPDATE: In light of section 3.1.1.5 in the RFC for Content-Type I'm re-thinking this a bit. I'm going to sleep on it.

[brettfishman]

@nicolaa So after thinking on this a bit more, and conferring with @bwebster on it, I'm not sure we are convinced that this is an issue worth solving at this point for a couple reasons:

1. API consumers, as far as I know, currently do not depend upon the Content-Type header in the response to do any verification of the version returned. I'm not saying that this will always be true, but we do not currently have a use-case out there in the wild.

2. As noted above, the specification for this header talks about it containing a media type (e.g. application/json) along with how it is encoded (e.g. charset=utf8). The example you gave above (Content-Type: "application/json; version=2" omits the charset=utf8 part, and I haven't found evidence to suggest this is standard, or at least an acceptable alternative.

Would you be able to point me to a source that supports your suggested approach?

[nicolaa]

@brettfishman I think you may be right. I'm not sure the version info should be tied to the Content-Type header either. HOWEVER, we do attach it to both the Content-Type and Accept headers in the Request, and there is no corresponding Accept header in the Response, so it feels like there is still a gap somewhere. I think most REST standards propose a customer header like Accept-Version to handle this.

I don't feel super strongly about this either way, but in the example i was referring to, the v1 and v2 response bodies are different, so I do think that there should be some way in the Response to differentiate what "Type" of response body we are sending.

[brettfishman]

I do think that there should be some way in the Response to differentiate what "Type" of response body we are sending

@nicolaa I agree with this statement. I don't have bandwidth at the moment to research other potential solutions, but would welcome a proposal for how to achieve this. I don't think the current proposal totally works, but I'm happy to revisit my POV if a compelling solution is found.

[nicolaa]

@brettfishman TBH, I don't think a perfect solution exists. These are known limitations of HTTP. We are really trying to build on top of an infrastructure that wasn't designed for our needs.

[brettfishman]

@nicolaa yeah, agreed. To be clear, I'm definitely not seeking a perfect solution.

[Aliciawyse]

Hey there! I appreciate the consideration of this pull request!

-Alicia

On Thu, Jun 13, 2019 at 4:38 PM Brett Fishman [email protected] wrote:

@nicolaa https://github.com/nicolaa yeah, agreed. To be clear, I'm definitely not seeking a perfect solution.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stitchfix/stitches/issues/70?email_source=notifications&email_token=ADJTK2KGOWO4QOUK6SGEVSTP2KV5LA5CNFSM4HDEYG6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXU623I#issuecomment-501869933, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJTK2NCQKCH7P4ERTISBQDP2KV5LANCNFSM4HDEYG6A .