[fix] Add Accept header to Retrofit clients

[pkoenig10]

Before this PR

The conjure spec states that:

Accept header - Clients MUST send an Accept: application/json header for all requests UNLESS the endpoint returns binary, in which case the client MUST send Accept: application/octet-stream. This ensures changes can be made to the wire format in a non-breaking way.

Feign clients created using conjure-java-runtime correctly implement the spec because Feign reads the JAX-RS annotations to set the Accept and Content-Type headers.

Retrofit clients created using conjure-java-runtime do not set either the Accept or Content-Type headers. The Content-Type header does exist on requests from Retrofit clients only because it is set by OkHttp.

After this PR

This PR adds the necessary annotations so requests from conjure-java-runtime Retrofit clients include the correct Accept header.

This implementation deviates slightly from a literal interpretation of the conjure spec and does not set the Accept header for requests that return void. If this was the intention when writing the conjure spec, it should be updated to make this clear. Otherwise, we should modify this PR.

[iamdanfox]

Thanks for hunting this down @pkoenig10 - this PR looks like a perfect solution for today's spec!

My one concern is that adding the header to the generated code actually backs us into a corner if we ever wanted to expand the types of wire formats that conjure understands. There's an RFC in flight at the moment to do exactly this (palantir/conjure#115) which proposes that clients should be able to negotiate with servers to figure out an optimal transport. This might require clients to send something like:

Accept: application/cbor; conjure=2, application/cbor; conjure=1, application/json; conjure=1

To ensure we're able to do this in the future, I think we should try to bake this logic into :conjure-java-retrofit2-client. This way, all retrofit users out there will immediately benefit from the fixed headers without requiring API authors to upgrade and re-release.

cc @robert3005

[iamdanfox]

Approving because the long-term solution is not really feasible... it turns out it's pretty painful to achieve this separation with the abstractions that Retrofit/Okhttp currently offer.

Given that a longer-term plan is to get rid of feign/retrofit entirely (and codegen a client), I don't want to spend ages hacking around retrofit to achieve the long-term dream solution straight away.

The tradeoff here is that your Accept headers will start working today, but if we do this format upgrading thing in the future then you'll probably have to generate a entirely new Service interfaces.

[pkoenig10]

adding the header to the generated code actually backs us into a corner

Is this actually true? If a service wanted to support a new MIME type, then that would require both the client and the server using new generated interfaces.

Setting the Accept header when using a Retrofit client is analogous to adding the Produces annotation when using a Feign client, since Feign uses that annotation to set the Accept header.