search

joukevandermaas / saule

JSON API library for ASP.Net Web API 2.
https://joukevandermaas.github.io/saule
MIT License
76 stars 37 forks source link

Do not require Accept header #151

Closed bjornharrtell closed 7 years ago

bjornharrtell commented 7 years ago

My take on https://github.com/joukevandermaas/saule/issues/100 and https://github.com/joukevandermaas/saule/issues/146.

joukevandermaas commented 7 years ago

Thanks for this!

I understood the requirements from #100 and #146 as follows:

  1. If the client does not send a request body, it does not need to send a Content-Type header
  2. The client does not need to send an accept header
  3. If the client does send an accept header, it must be application/vnd.api+json without any media type parameters

I don't really see these reflected in this PR though. My understanding of the code is these rules:

  1. The client does not need to send an accept header for GET and DELETE requests
  2. Other requests must have an accept header, and it must be application/vnd.api+json without any media type parameters
  3. Nothing about Content-Type in the diff.

4 and 5 don't make much sense to me, because I thought the Accept header was about the content-type of the response, not the request. It makes sense to validate it always (if it exists).

If I misunderstood something, please let me know!

bjornharrtell commented 7 years ago

You are absolutely correct, for some reason I mixed up these headers logically when taking a stab at this.

I suggest a more simple change to simply let requests through that have no application/vnd.api+json Accept header at all. For Content-Type I'm not sure there is an issue at all.

bjornharrtell commented 7 years ago

Reworked as suggested. I hope it makes sense this time.

joukevandermaas commented 7 years ago

Looks good! thanks!