Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

custom marshaler: handle Accept headers correctly #152

Merged
merged 1 commit into from
May 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 43 additions & 89 deletions runtime/marshaler_registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ import (
"net/http"
)

const mimeWildcard = "*"
// MIMEWildCard is the fallback MIME type used for requests which do not match
// a registered MIME type.
const MIMEWildcard = "*"

var (
defaultMarshaler = &JSONPb{OrigName: true}

acceptHeader = http.CanonicalHeaderKey("Accept")
contentTypeHeader = http.CanonicalHeaderKey("Content-Type")

defaultMarshaler = &JSONPb{OrigName: true}
)

// MarshalerForRequest returns the inbound/outbound marshalers for this request.
Expand All @@ -20,117 +23,68 @@ var (
// exactly match in the registry.
// Otherwise, it follows the above logic for "*"/InboundMarshaler/OutboundMarshaler.
func MarshalerForRequest(mux *ServeMux, r *http.Request) (inbound Marshaler, outbound Marshaler) {
headerVals := append(append([]string(nil), r.Header[contentTypeHeader]...), "*")

for _, val := range headerVals {
m := mux.marshalers.lookup(val)
if m != nil {
if inbound == nil {
inbound = m.inbound
}
if outbound == nil {
outbound = m.outbound
}
for _, acceptVal := range r.Header[acceptHeader] {
if m, ok := mux.marshalers.mimeMap[acceptVal]; ok {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keeping lookup method to encapsulate internal structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

none of this is exported and the tuple form of map lookup is more correct.

outbound = m
break
}
if inbound != nil && outbound != nil {
// Got them both, return
return inbound, outbound
}

for _, contentTypeVal := range r.Header[contentTypeHeader] {
if m, ok := mux.marshalers.mimeMap[contentTypeVal]; ok {
inbound = m
break
}
}

if inbound == nil {
inbound = defaultMarshaler
inbound = mux.marshalers.mimeMap[MIMEWildcard]
}
if outbound == nil {
outbound = defaultMarshaler
outbound = inbound
}
return inbound, outbound

return inbound, outbound
}

// marshalerRegistry keeps a mapping from MIME types to mimeMarshalers.
type marshalerRegistry map[string]*mimeMarshaler

type mimeMarshaler struct {
inbound Marshaler
outbound Marshaler
// marshalerRegistry is a mapping from MIME types to Marshalers.
type marshalerRegistry struct {
mimeMap map[string]Marshaler
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's unnecessary? there's no longer a mapping to two marshalers.

}

// addMarshaler adds an inbound and outbund marshaler for a case-sensitive MIME type string ("*" to match any MIME type).
// Inbound is the marshaler that is used when marshaling inbound requests from the client.
// Outbound is the marshaler that is used when marshaling outbound responses to the client.
func (r *marshalerRegistry) add(mime string, inbound, outbound Marshaler) error {
if mime == "" {
// add adds a marshaler for a case-sensitive MIME type string ("*" to match any
// MIME type).
func (m marshalerRegistry) add(mime string, marshaler Marshaler) error {
if len(mime) == 0 {
return errors.New("empty MIME type")
}
(*r)[mime] = &mimeMarshaler{
inbound: inbound,
outbound: outbound,
}
return nil
}

// addInboundMarshaler adds an inbound marshaler for a case-sensitive MIME type string ("*" to match any MIME type).
// Inbound is the marshaler that is used when marshaling inbound requests from the client.
func (r *marshalerRegistry) addInbound(mime string, inbound Marshaler) error {
if mime == "" {
return errors.New("empty MIME type")
}
if entry := (*r)[mime]; entry != nil {
entry.inbound = inbound
return nil
}
(*r)[mime] = &mimeMarshaler{inbound: inbound}
return nil
}
m.mimeMap[mime] = marshaler

// addOutBound adds an outbund marshaler for a case-sensitive MIME type string ("*" to match any MIME type).
// Outbound is the marshaler that is used when marshaling outbound responses to the client.
func (r *marshalerRegistry) addOutbound(mime string, outbound Marshaler) error {
mime = http.CanonicalHeaderKey(mime)
if mime == "" {
return errors.New("empty MIME type")
}
if entry := (*r)[mime]; entry != nil {
entry.outbound = outbound
return nil
}
(*r)[mime] = &mimeMarshaler{outbound: outbound}
return nil

}

func (r *marshalerRegistry) lookup(mime string) *mimeMarshaler {
if r == nil {
return nil
// makeMarshalerMIMERegistry returns a new registry of marshalers.
// It allows for a mapping of case-sensitive Content-Type MIME type string to runtime.Marshaler interfaces.
//
// For example, you could allow the client to specify the use of the runtime.JSONPb marshaler
// with a "applicaton/jsonpb" Content-Type and the use of the runtime.JSONBuiltin marshaler
// with a "application/json" Content-Type.
// "*" can be used to match any Content-Type.
// This can be attached to a ServerMux with the marshaler option.
func makeMarshalerMIMERegistry() marshalerRegistry {
return marshalerRegistry{
mimeMap: map[string]Marshaler{
MIMEWildcard: defaultMarshaler,
},
}
return (*r)[mime]
}

// WithMarshalerOption returns a ServeMuxOption which associates inbound and outbound
// Marshalers to a MIME type in mux.
func WithMarshalerOption(mime string, in, out Marshaler) ServeMuxOption {
return func(mux *ServeMux) {
if err := mux.marshalers.add(mime, in, out); err != nil {
panic(err)
}
}
}

// WithInboundMarshalerOption returns a ServeMuxOption which associates an inbound
// Marshaler to a MIME type in mux.
func WithInboundMarshalerOption(mime string, in Marshaler) ServeMuxOption {
return func(mux *ServeMux) {
if err := mux.marshalers.addInbound(mime, in); err != nil {
panic(err)
}
}
}

// WithOutboundMarshalerOption returns a ServeMuxOption which associates an outbound
// Marshaler to a MIME type in mux.
func WithOutboundMarshalerOption(mime string, out Marshaler) ServeMuxOption {
func WithMarshalerOption(mime string, marshaler Marshaler) ServeMuxOption {
return func(mux *ServeMux) {
if err := mux.marshalers.addOutbound(mime, out); err != nil {
if err := mux.marshalers.add(mime, marshaler); err != nil {
panic(err)
}
}
Expand Down
21 changes: 6 additions & 15 deletions runtime/marshaler_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ func TestMarshalerForRequest(t *testing.T) {
if err != nil {
t.Fatalf(`http.NewRequest("GET", "http://example.com", nil) failed with %v; want success`, err)
}
r.Header.Set("Content-Type", "application/x-example")
r.Header.Set("Accept", "application/x-out")
r.Header.Set("Content-Type", "application/x-in")

mux := runtime.NewServeMux()

Expand All @@ -26,38 +27,28 @@ func TestMarshalerForRequest(t *testing.T) {
t.Errorf("out = %#v; want a runtime.JSONPb", in)
}

var marshalers [6]dummyMarshaler
var marshalers [3]dummyMarshaler
specs := []struct {
opt runtime.ServeMuxOption

wantIn runtime.Marshaler
wantOut runtime.Marshaler
}{
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add scenario for Accept header?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

opt: runtime.WithMarshalerOption("*", &marshalers[0], &marshalers[0]),
opt: runtime.WithMarshalerOption(runtime.MIMEWildcard, &marshalers[0]),
wantIn: &marshalers[0],
wantOut: &marshalers[0],
},
{
opt: runtime.WithInboundMarshalerOption("*", &marshalers[1]),
opt: runtime.WithMarshalerOption("application/x-in", &marshalers[1]),
wantIn: &marshalers[1],
wantOut: &marshalers[0],
},
{
opt: runtime.WithOutboundMarshalerOption("application/x-example", &marshalers[2]),
opt: runtime.WithMarshalerOption("application/x-out", &marshalers[2]),
wantIn: &marshalers[1],
wantOut: &marshalers[2],
},
{
opt: runtime.WithInboundMarshalerOption("application/x-example", &marshalers[3]),
wantIn: &marshalers[3],
wantOut: &marshalers[2],
},
{
opt: runtime.WithMarshalerOption("application/x-example", &marshalers[4], &marshalers[5]),
wantIn: &marshalers[4],
wantOut: &marshalers[5],
},
}
for i, spec := range specs {
var opts []runtime.ServeMuxOption
Expand Down
2 changes: 1 addition & 1 deletion runtime/mux.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func NewServeMux(opts ...ServeMuxOption) *ServeMux {
serveMux := &ServeMux{
handlers: make(map[string][]handler),
forwardResponseOptions: make([]func(context.Context, http.ResponseWriter, proto.Message) error, 0),
marshalers: make(marshalerRegistry),
marshalers: makeMarshalerMIMERegistry(),
}

for _, opt := range opts {
Expand Down