custom marshaler: handle Accept headers correctly
#152
Conversation
tamird
changed the title
Accept headers correctlyAccept headers correctly
|
This failed on |
|
why the MakeMarshalerMIMERegistry naming change? |
|
Because it no longer returns a pointer. |
|
It is unlikely that I will merge this PR because
It would be even better if we can property handle Accept header, however, I don't think it is mandatory. If we handle the header, it would require:
|
|
Rebased.
Yeah, it's probably incomplete, but still better than the current handling which conflates "content-type" (which specifies the incoming content) with "accept" (which specifies the outgoing content).
This is not what this PR does. It uses "content-type" to identify the MIME type of the incoming request, but uses "accept" to determine the MIME type that should be used to encode the outgoing response.
Yes, we could do these things, but it doesn't seem "required" to me. I'm not sure splitting Marshaler and Unmarshaler is even worth it, but I'll leave that decision to you. Nevertheless, I don't think that refactoring should block this change. |
|
|
||
| for _, contentTypeVal := range r.Header[contentTypeHeader] { | ||
| if m, ok := mux.marshalers.mimeMap[contentTypeVal]; ok { | ||
| outbound = m |
|
@tamird I'll add some comments. |
| } | ||
| if outbound == nil { | ||
| outbound = defaultMarshaler | ||
| outbound = mux.marshalers.mimeMap[MIMEWildcard] |
There was a problem hiding this comment.
Could you try falling back to mux.marshalers.mimeMap[contentTypeHeader] before MIMEWildcard
So this line can be just outbound = inbound.
Also greatly simplifies existing logic to avoid special casing of defaults.
|
@yugui ping, could you take another look? |
|
LGTM. Thank you for your contribution. |
|
Thanks! |
…correctly custom marshaler: handle `Accept` headers correctly
Also greatly simplifies existing logic to avoid special casing of
defaults.