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

Add encoder/decoder interface to configure encoding/decoding #47

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
52 changes: 49 additions & 3 deletions decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@ import (
"github.com/ajg/form"
)

// Decoder is a generic interface to decode arbitrary data from a reader `r` to
// a value `v`.
type Decoder interface {
// Decodes a given reader into an interface
Decode(r io.Reader, req *http.Request, v interface{}) error
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'll need to revisit the discussion at #40.

I see a r io.Reader -- but req.Body is also an io.Reader. Do we need both?

Copy link
Author

Choose a reason for hiding this comment

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

Oh well that's right r will be req.Body. But see the comment below for the discussion of the function signature.

}

// Package-level variables for decoding the supported formats. They are set to
// our default implementations. By setting render.Decode{JSON,XML,Form} you can
// customize Decoding (e.g. you might want to configure the JSON-decoder)
var (
DecoderJSON Decoder = DecodeJSONInter{}
Copy link
Contributor

Choose a reason for hiding this comment

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

What does Inter stand for?

Copy link
Author

Choose a reason for hiding this comment

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

Well as I needed to keep the DecodeJSON function (see below), I needed to call the struct which implements the interface differently and decided to go with DecodeJSONInter to signal that this is the version using/conforming to the interface.
Maybe Impl or so would be the more typical choice.

DecoderXML Decoder = DecodeXMLInter{}
DecoderForm Decoder = DecodeFormInter{}
)

// Decode is a package-level variable set to our default Decoder. We do this
// because it allows you to set render.Decode to another function with the
// same function signature, while also utilizing the render.Decoder() function
Expand All @@ -26,31 +42,61 @@ func DefaultDecoder(r *http.Request, v interface{}) error {

switch GetRequestContentType(r) {
case ContentTypeJSON:
err = DecodeJSON(r.Body, v)
err = DecoderJSON.Decode(r.Body, r, v)
case ContentTypeXML:
err = DecodeXML(r.Body, v)
err = DecoderXML.Decode(r.Body, r, v)
case ContentTypeForm:
err = DecodeForm(r.Body, v)
err = DecoderForm.Decode(r.Body, r, v)
Comment on lines +45 to +49
Copy link
Contributor

@VojtechVitek VojtechVitek May 8, 2023

Choose a reason for hiding this comment

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

Looking at this change, I think I was wrong to suggest adding a request object to this func signature.

default:
err = errors.New("render: unable to automatically decode the request content type")
}

return err
}

type DecodeJSONInter struct{}

// Decodes a given reader into an interface using the json decoder.
func (DecodeJSONInter) Decode(r io.Reader, req *http.Request, v interface{}) error {
defer io.Copy(ioutil.Discard, r) //nolint:errcheck
return json.NewDecoder(r).Decode(v)
}

// DecodeJSON decodes a given reader into an interface using the json decoder.
//
// Deprecated: DecoderJSON.Decode() should be used.
func DecodeJSON(r io.Reader, v interface{}) error {
defer io.Copy(ioutil.Discard, r) //nolint:errcheck
return json.NewDecoder(r).Decode(v)
}

type DecodeXMLInter struct{}

// Decodes a given reader into an interface using the xml decoder.
func (DecodeXMLInter) Decode(r io.Reader, req *http.Request, v interface{}) error {
defer io.Copy(ioutil.Discard, r) //nolint:errcheck
return xml.NewDecoder(r).Decode(v)
}

// DecodeXML decodes a given reader into an interface using the xml decoder.
//
// Deprecated: DecoderXML.Decode() should be used.
func DecodeXML(r io.Reader, v interface{}) error {
defer io.Copy(ioutil.Discard, r) //nolint:errcheck
return xml.NewDecoder(r).Decode(v)
}

type DecodeFormInter struct{}

// Decodes a given reader into an interface using the form decoder.
func (DecodeFormInter) Decode(r io.Reader, req *http.Request, v interface{}) error {
decoder := form.NewDecoder(r) //nolint:errcheck
return decoder.Decode(v)
}

// DecodeForm decodes a given reader into an interface using the form decoder.
//
// Deprecated: DecoderForm.Decode() should be used.
func DecodeForm(r io.Reader, v interface{}) error {
decoder := form.NewDecoder(r) //nolint:errcheck
return decoder.Decode(v)
Expand Down
145 changes: 142 additions & 3 deletions responder.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,34 @@ import (
"context"
"encoding/json"
"encoding/xml"
"errors"
"fmt"
"net/http"
"reflect"
)

var ErrInvalidType error = errors.New("Invalid Type passed")

// Encoder is a generic interface to encode arbitrary data from value `v` to a
// reader `r`
type Encoder interface {
// Marshals 'v' to 'w'
Encode(w http.ResponseWriter, req *http.Request, v interface{}) error
}

// Package-level variables for encoding the supported formats. They are set to
// our default implementations. By setting
// render.Encode{JSON,XML,Data,PlainText} you can customize Encoding (e.g. you
// might want to configure the JSON-encoder)
var (
EncoderJSON Encoder = EncodeJSON{}
EncoderXML Encoder = EncodeXML{}
// EncoderData.Decode(w, req, v): v must be []byte
EncoderData Encoder = EncodeData{}
// EncoderPlainText.Decode(w, req, v): v must be string
EncoderPlainText Encoder = EncodePlainText{}
)

// M is a convenience alias for quickly building a map structure that is going
// out to a responder. Just a short-hand.
type M map[string]interface{}
Expand Down Expand Up @@ -51,16 +74,36 @@ func DefaultResponder(w http.ResponseWriter, r *http.Request, v interface{}) {
// Format response based on request Accept header.
switch GetAcceptedContentType(r) {
case ContentTypeJSON:
JSON(w, r, v)
EncoderJSON.Encode(w, r, v)
case ContentTypeXML:
XML(w, r, v)
EncoderXML.Encode(w, r, v)
default:
JSON(w, r, v)
EncoderJSON.Encode(w, r, v)
}
}

type EncodePlainText struct{}

// Writes a string to the response, setting the Content-Type as
// text/plain.
// vi has to be string
func (EncodePlainText) Encode(w http.ResponseWriter, r *http.Request, vi interface{}) error {
v, ok := vi.(string)
if !ok {
return ErrInvalidType
}
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
w.WriteHeader(status)
}
w.Write([]byte(v)) //nolint:errcheck
return nil
}

// PlainText writes a string to the response, setting the Content-Type as
// text/plain.
//
// Deprecated: EncoderPlainText.Encode() should be used.
func PlainText(w http.ResponseWriter, r *http.Request, v string) {
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
Expand All @@ -69,8 +112,28 @@ func PlainText(w http.ResponseWriter, r *http.Request, v string) {
w.Write([]byte(v)) //nolint:errcheck
}

type EncodeData struct{}

// Writes raw bytes to the response, setting the Content-Type as
// application/octet-stream.
// vi has to be []byte
func (EncodeData) Encode(w http.ResponseWriter, r *http.Request, vi interface{}) error {
v, ok := vi.([]byte)
if !ok {
return ErrInvalidType
}
w.Header().Set("Content-Type", "application/octet-stream")
if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
w.WriteHeader(status)
}
w.Write(v) //nolint:errcheck
return nil
}

// Data writes raw bytes to the response, setting the Content-Type as
// application/octet-stream.
//
// Deprecated: EncoderXML.Encode() should be used.
func Data(w http.ResponseWriter, r *http.Request, v []byte) {
w.Header().Set("Content-Type", "application/octet-stream")
if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
Expand All @@ -79,7 +142,26 @@ func Data(w http.ResponseWriter, r *http.Request, v []byte) {
w.Write(v) //nolint:errcheck
}

type EncodeHTML struct{}

// Writes a string to the response, setting the Content-Type as text/html.
// vi has to be a string.
func (EncodeHTML) Encode(w http.ResponseWriter, r *http.Request, vi interface{}) error {
v, ok := vi.(string)
if !ok {
return ErrInvalidType
}
w.Header().Set("Content-Type", "text/html; charset=utf-8")
if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
w.WriteHeader(status)
}
w.Write([]byte(v)) //nolint:errcheck
return nil
}

// HTML writes a string to the response, setting the Content-Type as text/html.
//
// Deprecated: EncoderHTML.Encode() should be used.
func HTML(w http.ResponseWriter, r *http.Request, v string) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
Expand All @@ -88,8 +170,32 @@ func HTML(w http.ResponseWriter, r *http.Request, v string) {
w.Write([]byte(v)) //nolint:errcheck
}

type EncodeJSON struct{}

// Marshals 'v' to JSON, automatically escaping HTML and setting the
// Content-Type as application/json.
func (EncodeJSON) Encode(w http.ResponseWriter, r *http.Request, v interface{}) error {
buf := &bytes.Buffer{}
enc := json.NewEncoder(buf)
enc.SetEscapeHTML(true)
if err := enc.Encode(v); err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return err
}

w.Header().Set("Content-Type", "application/json")
if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
w.WriteHeader(status)
}
w.Write(buf.Bytes()) //nolint:errcheck
return nil
}


// JSON marshals 'v' to JSON, automatically escaping HTML and setting the
// Content-Type as application/json.
//
// Deprecated: EncoderJSON.Encode() should be used.
func JSON(w http.ResponseWriter, r *http.Request, v interface{}) {
buf := &bytes.Buffer{}
enc := json.NewEncoder(buf)
Expand All @@ -106,9 +212,42 @@ func JSON(w http.ResponseWriter, r *http.Request, v interface{}) {
w.Write(buf.Bytes()) //nolint:errcheck
}

type EncodeXML struct{}

// Marshals 'v' to XML, setting the Content-Type as application/xml. It
// will automatically prepend a generic XML header (see encoding/xml.Header) if
// one is not found in the first 100 bytes of 'v'.
func (EncodeXML) Encode(w http.ResponseWriter, r *http.Request, v interface{}) error {
b, err := xml.Marshal(v)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return err
}

w.Header().Set("Content-Type", "application/xml; charset=utf-8")
if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
w.WriteHeader(status)
}

// Try to find <?xml header in first 100 bytes (just in case there're some XML comments).
findHeaderUntil := len(b)
if findHeaderUntil > 100 {
findHeaderUntil = 100
}
if !bytes.Contains(b[:findHeaderUntil], []byte("<?xml")) {
// No header found. Print it out first.
w.Write([]byte(xml.Header)) //nolint:errcheck
}

w.Write(b) //nolint:errcheck
return nil
}

// XML marshals 'v' to XML, setting the Content-Type as application/xml. It
// will automatically prepend a generic XML header (see encoding/xml.Header) if
// one is not found in the first 100 bytes of 'v'.
//
// Deprecated: EncoderXML.Encode() should be used.
Copy link
Contributor

@VojtechVitek VojtechVitek May 8, 2023

Choose a reason for hiding this comment

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

I don't think we want to "deprecate" these functions.

I thought we would just let users overwrite them with a custom implementation.

Copy link
Author

Choose a reason for hiding this comment

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

Well this issue is linked with your next annotation. Because we wanted to add the Request object, the Interface of the function changes and there was no way of keeping the old function and the new function, with the old function calling simply redirecting to the new one (the request object has to come from somewhere).

So in the end it's all about changing the function signature (deprecating and in a future breaking change remove the old functions) or not. Personally I'd be satisfied with the current signature as well, right now I don't have a use-case where I need the Request object in the en-/decoder implementation.

func XML(w http.ResponseWriter, r *http.Request, v interface{}) {
b, err := xml.Marshal(v)
if err != nil {
Expand Down