Skip to content

Commit

Permalink
fixed: use content-type to decode
Browse files Browse the repository at this point in the history 
This patch makes use of server response Content-Type header to decode data instead of blindly use configured internal encoding
  • Loading branch information
@primalmotion
primalmotion committed Apr 25, 2019
1 parent 184d789 commit c88478a
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 31 deletions.
12 changes: 6 additions & 6 deletions maniphttp/manipulator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (s *httpManipulator) RetrieveMany(mctx manipulate.Context, dest elemental.I

if response.StatusCode != http.StatusNoContent {
defer response.Body.Close() // nolint: errcheck
if err := decodeData(response, s.encoding, dest); err != nil {
if err := decodeData(response, dest); err != nil {
sp.SetTag("error", true)
sp.LogFields(log.Error(err))
return err
Expand Down Expand Up @@ -188,7 +188,7 @@ func (s *httpManipulator) Retrieve(mctx manipulate.Context, object elemental.Ide

if response.StatusCode != http.StatusNoContent {
defer response.Body.Close() // nolint: errcheck
if err := decodeData(response, s.encoding, object); err != nil {
if err := decodeData(response, object); err != nil {
sp.SetTag("error", true)
sp.LogFields(log.Error(err))
return err
Expand Down Expand Up @@ -241,7 +241,7 @@ func (s *httpManipulator) Create(mctx manipulate.Context, object elemental.Ident

if response.StatusCode != http.StatusNoContent {
defer response.Body.Close() // nolint: errcheck
if err := decodeData(response, s.encoding, object); err != nil {
if err := decodeData(response, object); err != nil {
sp.SetTag("error", true)
sp.LogFields(log.Error(err))
return err
Expand Down Expand Up @@ -303,7 +303,7 @@ func (s *httpManipulator) Update(mctx manipulate.Context, object elemental.Ident

if response.StatusCode != http.StatusNoContent {
defer response.Body.Close() // nolint: errcheck
if err := decodeData(response, s.encoding, object); err != nil {
if err := decodeData(response, object); err != nil {
sp.SetTag("error", true)
sp.LogFields(log.Error(err))
return err
Expand Down Expand Up @@ -348,7 +348,7 @@ func (s *httpManipulator) Delete(mctx manipulate.Context, object elemental.Ident

if response.StatusCode != http.StatusNoContent {
defer response.Body.Close() // nolint: errcheck
if err := decodeData(response, s.encoding, object); err != nil {
if err := decodeData(response, object); err != nil {
sp.SetTag("error", true)
sp.LogFields(log.Error(err))
return err
Expand Down Expand Up @@ -590,7 +590,7 @@ func (s *httpManipulator) send(mctx manipulate.Context, method string, requrl st
es := elemental.Errors{}

defer response.Body.Close() // nolint: errcheck
if err := decodeData(response, s.encoding, &es); err != nil {
if err := decodeData(response, &es); err != nil {
return nil, err
}

Expand Down
49 changes: 32 additions & 17 deletions maniphttp/manipulator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ func TestHTTP_Retrieve(t *testing.T) {
Convey("Given I have an http manipulator and a and the server will return an error", t, func() {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(422)
fmt.Fprint(w, `[{"code":422,"description":"nope.","subject":"elemental","title":"Read Only Error","data":null}]`)
}))
Expand Down Expand Up @@ -477,7 +478,8 @@ func TestHTTP_Update(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, "nope", 500)
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprint(w, "nope")
}))
defer ts.Close()

Expand Down Expand Up @@ -582,7 +584,8 @@ func TestHTTP_Delete(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, "nope", 500)
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprint(w, "nope")
}))
defer ts.Close()

Expand Down Expand Up @@ -726,7 +729,8 @@ func TestHTTP_RetrieveMany(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, "woops", 500)
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprint(w, "woops")
}))
defer ts.Close()

Expand Down Expand Up @@ -828,7 +832,8 @@ func TestHTTP_Create(t *testing.T) {
Convey("When I create a child and I got a communication error", func() {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "", 500)
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprint(w, "")
}))
defer ts.Close()

Expand Down Expand Up @@ -926,7 +931,8 @@ func TestHTTP_Count(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, "woops", 500)
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprint(w, "woops")
}))
defer ts.Close()

Expand Down Expand Up @@ -994,7 +1000,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 408, "title": "nope", "description": "boom"}]`, http.StatusRequestTimeout)
w.WriteHeader(http.StatusRequestTimeout)
fmt.Fprint(w, `[{"code": 408, "title": "nope", "description": "boom"}]`)
}))
defer ts.Close()

Expand All @@ -1019,7 +1026,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 502, "title": "nope", "description": "boom"}]`, http.StatusBadGateway)
w.WriteHeader(http.StatusBadGateway)
fmt.Fprint(w, `[{"code": 502, "title": "nope", "description": "boom"}]`)
}))
defer ts.Close()

Expand All @@ -1044,7 +1052,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 503, "title": "nope", "description": "boom"}]`, http.StatusServiceUnavailable)
w.WriteHeader(http.StatusServiceUnavailable)
fmt.Fprint(w, `[{"code": 503, "title": "nope", "description": "boom"}]`)
}))
defer ts.Close()

Expand All @@ -1069,7 +1078,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 504, "title": "nope", "description": "boom"}]`, http.StatusGatewayTimeout)
w.WriteHeader(http.StatusGatewayTimeout)
fmt.Fprint(w, `[{"code": 504, "title": "nope", "description": "boom"}]`)
}))
defer ts.Close()

Expand All @@ -1094,7 +1104,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 429, "title": "nope", "description": "boom"}]`, http.StatusTooManyRequests)
w.WriteHeader(http.StatusTooManyRequests)
fmt.Fprint(w, `[{"code": 429, "title": "nope", "description": "boom"}]`)
}))
defer ts.Close()

Expand All @@ -1119,7 +1130,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 403, "title": "nope", "description": "boom"}]`, http.StatusForbidden)
w.WriteHeader(http.StatusForbidden)
fmt.Fprint(w, `[{"code": 403, "title": "nope", "description": "boom"}]`)
}))
defer ts.Close()

Expand All @@ -1142,7 +1154,8 @@ func TestHTTP_send(t *testing.T) {
call++
if call == 1 {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 403, "title": "nope", "description": "boom"}]`, http.StatusForbidden)
w.WriteHeader(http.StatusForbidden)
fmt.Fprint(w, `[{"code": 403, "title": "nope", "description": "boom"}]`)
} else {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusNoContent)
Expand Down Expand Up @@ -1174,7 +1187,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 423, "title": "nope", "description": "boom"}]`, http.StatusLocked)
w.WriteHeader(http.StatusLocked)
fmt.Fprint(w, `[{"code": 423, "title": "nope", "description": "boom"}]`)
}))
defer ts.Close()

Expand All @@ -1199,7 +1213,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 423, "]`, 404)
w.WriteHeader(http.StatusNotFound)
fmt.Fprint(w, `[{"code": 423, "]`)
}))
defer ts.Close()

Expand All @@ -1212,8 +1227,7 @@ func TestHTTP_send(t *testing.T) {
So(err, ShouldNotBeNil)
So(err, ShouldHaveSameTypeAs, manipulate.ErrCannotUnmarshal{})
So(err.Error(), ShouldEqual, `Unable to unmarshal data: unable to decode application/json: EOF. original data:
[{"code": 423, "]
`)
[{"code": 423, "]`)
})
})
})
Expand All @@ -1226,7 +1240,8 @@ func TestHTTP_send(t *testing.T) {

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
http.Error(w, `[{"code": 500, "title": "nope", "description": "boom"}]`, 500)
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprint(w, `[{"code": 500, "title": "nope", "description": "boom"}]`)
}))
defer ts.Close()

Expand Down
12 changes: 10 additions & 2 deletions maniphttp/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func addQueryParameters(req *http.Request, ctx manipulate.Context) error {
return nil
}

func decodeData(r *http.Response, encodingType elemental.EncodingType, dest interface{}) (err error) {
func decodeData(r *http.Response, dest interface{}) (err error) {

if r.Body == nil {
return manipulate.NewErrCannotUnmarshal("nil reader")
Expand Down Expand Up @@ -97,7 +97,15 @@ func decodeData(r *http.Response, encodingType elemental.EncodingType, dest inte
return manipulate.NewErrCannotUnmarshal(fmt.Sprintf("unable to read data: %s", err.Error()))
}

if err = elemental.Decode(encodingType, data, dest); err != nil {
encoding := elemental.EncodingTypeJSON
if r.Header.Get("Content-Type") != "" {
encoding, _, err = elemental.EncodingFromHeaders(r.Header)
if err != nil {
return elemental.NewErrors(err)
}
}

if err = elemental.Decode(encoding, data, dest); err != nil {
return manipulate.NewErrCannotUnmarshal(fmt.Sprintf("%s. original data:\n%s", err.Error(), string(data)))
}

Expand Down
89 changes: 83 additions & 6 deletions maniphttp/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"net/url"
"testing"

"go.aporeto.io/elemental"

. "github.com/smartystreets/goconvey/convey"
"go.aporeto.io/manipulate"
)
Expand Down Expand Up @@ -171,7 +173,41 @@ func Test_decodeData(t *testing.T) {
Convey("When I call decodeData", func() {

dest := map[string]interface{}{}
err := decodeData(r, "", &dest)
err := decodeData(r, &dest)

Convey("Then err should be nil", func() {
So(err, ShouldBeNil)
})

Convey("Then the dest should be correct", func() {
So(len(dest), ShouldEqual, 2)
So(dest["name"].(string), ShouldEqual, "thename")
So(dest["age"].(uint64), ShouldEqual, 2)
})
})
})

Convey("Given I have valid json gzipped data in a reader with no content-type", t, func() {

buf := bytes.NewBuffer(nil)
zw := gzip.NewWriter(buf)
_, err := zw.Write([]byte(`{"name":"thename","age": 2}`))
if err != nil {
panic(err)
}
zw.Close() // nolint

r := &http.Response{
Body: ioutil.NopCloser(buf),
Header: http.Header{
"Content-Encoding": []string{"gzip"},
},
}

Convey("When I call decodeData", func() {

dest := map[string]interface{}{}
err := decodeData(r, &dest)

Convey("Then err should be nil", func() {
So(err, ShouldBeNil)
Expand All @@ -185,7 +221,7 @@ func Test_decodeData(t *testing.T) {
})
})

Convey("Given I have valid json gzipped data in a reader", t, func() {
Convey("Given I have valid json gzipped data in a reader with json content-type", t, func() {

buf := bytes.NewBuffer(nil)
zw := gzip.NewWriter(buf)
Expand All @@ -199,13 +235,14 @@ func Test_decodeData(t *testing.T) {
Body: ioutil.NopCloser(buf),
Header: http.Header{
"Content-Encoding": []string{"gzip"},
"Content-Type": []string{"application/json"},
},
}

Convey("When I call decodeData", func() {

dest := map[string]interface{}{}
err := decodeData(r, "", &dest)
err := decodeData(r, &dest)

Convey("Then err should be nil", func() {
So(err, ShouldBeNil)
Expand All @@ -219,6 +256,46 @@ func Test_decodeData(t *testing.T) {
})
})

Convey("Given I have valid msgpack gzipped data in a reader with msgpack content-type", t, func() {

buf := bytes.NewBuffer(nil)
zw := gzip.NewWriter(buf)

data, err := elemental.Encode(elemental.EncodingTypeMSGPACK, map[string]interface{}{"name": "thename", "age": 2})
if err != nil {
panic(err)
}
_, err = zw.Write(data)
if err != nil {
panic(err)
}
zw.Close() // nolint

r := &http.Response{
Body: ioutil.NopCloser(buf),
Header: http.Header{
"Content-Encoding": []string{"gzip"},
"Content-Type": []string{"application/msgpack"},
},
}

Convey("When I call decodeData", func() {

dest := map[string]interface{}{}
err := decodeData(r, &dest)

Convey("Then err should be nil", func() {
So(err, ShouldBeNil)
})

Convey("Then the dest should be correct", func() {
So(len(dest), ShouldEqual, 2)
So(dest["name"].(string), ShouldEqual, "thename")
So(dest["age"].(int64), ShouldEqual, 2)
})
})
})

Convey("Given I have invalid valid json data in a reader", t, func() {

r := &http.Response{
Expand All @@ -228,7 +305,7 @@ func Test_decodeData(t *testing.T) {
Convey("When I call decodeData", func() {

dest := map[string]interface{}{}
err := decodeData(r, "", &dest)
err := decodeData(r, &dest)

Convey("Then err should not be nil", func() {
So(err, ShouldNotBeNil)
Expand All @@ -250,7 +327,7 @@ func Test_decodeData(t *testing.T) {
}

dest := map[string]interface{}{}
err := decodeData(r, "", &dest)
err := decodeData(r, &dest)

Convey("Then err should not be nil", func() {
So(err, ShouldNotBeNil)
Expand All @@ -272,7 +349,7 @@ func Test_decodeData(t *testing.T) {
Convey("When I call decodeData", func() {

dest := map[string]interface{}{}
err := decodeData(r, "", &dest)
err := decodeData(r, &dest)

Convey("Then err should not be nil", func() {
So(err, ShouldNotBeNil)
Expand Down

0 comments on commit c88478a

Please sign in to comment.