From d67a62171542ef848f97eecf898b5ecf64fe83b5 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Mon, 31 Jul 2017 15:11:15 -0600 Subject: [PATCH] http: http-utils added after extraction Found common http utils that were being multiply duplicated across many libraries and since am moving things in basecoin/unstable to add for more functionality, it's better to put them in one place. Utilities and tests added: - [X] FparseJSON - [X] FparseAndValidateJSON - [X] ParseRequestJSON - [X] ParseAndValidateRequestJSON - [X] WriteCode - [X] WriteError - [X] WriteSuccess - [X] ErrorResponse During review from @ethanfrey, made updates: * Removed tt.want since it was a distraction/artifact that made the reviewer think the tests weren't testing for both failed and passed results. * Added ErrorWithCode as WithCode is a common options pattern in Go that could cause confusion: ErrorWithCode(error, int) ErrorResponse * Using json.NewDecoder(io.Reader) error instead of ioutil.ReadAll(io.Reader) to slurp all the bytes. * Added more test scenarios to achieve 100% coverage of http.go --- common/http.go | 153 +++++++++++++++++++++++++++ common/http_test.go | 250 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 403 insertions(+) create mode 100644 common/http.go create mode 100644 common/http_test.go diff --git a/common/http.go b/common/http.go new file mode 100644 index 000000000..56b5b6c63 --- /dev/null +++ b/common/http.go @@ -0,0 +1,153 @@ +package common + +import ( + "encoding/json" + "io" + "net/http" + + "gopkg.in/go-playground/validator.v9" + + "github.com/pkg/errors" +) + +type ErrorResponse struct { + Success bool `json:"success,omitempty"` + + // Err is the error message if Success is false + Err string `json:"error,omitempty"` + + // Code is set if Success is false + Code int `json:"code,omitempty"` +} + +// ErrorWithCode makes an ErrorResponse with the +// provided err's Error() content, and status code. +// It panics if err is nil. +func ErrorWithCode(err error, code int) *ErrorResponse { + return &ErrorResponse{ + Err: err.Error(), + Code: code, + } +} + +// Ensure that ErrorResponse implements error +var _ error = (*ErrorResponse)(nil) + +func (er *ErrorResponse) Error() string { + return er.Err +} + +// Ensure that ErrorResponse implements httpCoder +var _ httpCoder = (*ErrorResponse)(nil) + +func (er *ErrorResponse) HTTPCode() int { + return er.Code +} + +var errNilBody = errors.Errorf("expecting a non-nil body") + +// FparseJSON unmarshals into save, the body of the provided reader. +// Since it uses json.Unmarshal, save must be of a pointer type +// or compatible with json.Unmarshal. +func FparseJSON(r io.Reader, save interface{}) error { + if r == nil { + return errors.Wrap(errNilBody, "Reader") + } + + dec := json.NewDecoder(r) + if err := dec.Decode(save); err != nil { + return errors.Wrap(err, "Decode/Unmarshal") + } + return nil +} + +// ParseRequestJSON unmarshals into save, the body of the +// request. It closes the body of the request after parsing. +// Since it uses json.Unmarshal, save must be of a pointer type +// or compatible with json.Unmarshal. +func ParseRequestJSON(r *http.Request, save interface{}) error { + if r == nil || r.Body == nil { + return errNilBody + } + defer r.Body.Close() + + return FparseJSON(r.Body, save) +} + +// ParseRequestAndValidateJSON unmarshals into save, the body of the +// request and invokes a validator on the saved content. To ensure +// validation, make sure to set tags "validate" on your struct as +// per https://godoc.org/gopkg.in/go-playground/validator.v9. +// It closes the body of the request after parsing. +// Since it uses json.Unmarshal, save must be of a pointer type +// or compatible with json.Unmarshal. +func ParseRequestAndValidateJSON(r *http.Request, save interface{}) error { + if r == nil || r.Body == nil { + return errNilBody + } + defer r.Body.Close() + + return FparseAndValidateJSON(r.Body, save) +} + +// FparseAndValidateJSON like FparseJSON unmarshals into save, +// the body of the provided reader. However, it invokes the validator +// to check the set validators on your struct fields as per +// per https://godoc.org/gopkg.in/go-playground/validator.v9. +// Since it uses json.Unmarshal, save must be of a pointer type +// or compatible with json.Unmarshal. +func FparseAndValidateJSON(r io.Reader, save interface{}) error { + if err := FparseJSON(r, save); err != nil { + return err + } + return validate(save) +} + +var theValidator = validator.New() + +func validate(obj interface{}) error { + return errors.Wrap(theValidator.Struct(obj), "Validate") +} + +// WriteSuccess JSON marshals the content provided, to an HTTP +// response, setting the provided status code and setting header +// "Content-Type" to "application/json". +func WriteSuccess(w http.ResponseWriter, data interface{}) { + WriteCode(w, data, 200) +} + +// WriteCode JSON marshals content, to an HTTP response, +// setting the provided status code, and setting header +// "Content-Type" to "application/json". If JSON marshalling fails +// with an error, WriteCode instead writes out the error invoking +// WriteError. +func WriteCode(w http.ResponseWriter, out interface{}, code int) { + blob, err := json.MarshalIndent(out, "", " ") + if err != nil { + WriteError(w, err) + } else { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(code) + w.Write(blob) + } +} + +type httpCoder interface { + HTTPCode() int +} + +// WriteError is a convenience function to write out an +// error to an http.ResponseWriter, to send out an error +// that's structured as JSON i.e the form +// {"error": sss, "code": ddd} +// If err implements the interface HTTPCode() int, +// it will use that status code otherwise, it will +// set code to be http.StatusBadRequest +func WriteError(w http.ResponseWriter, err error) { + code := http.StatusBadRequest + if httpC, ok := err.(httpCoder); ok { + code = httpC.HTTPCode() + } + + WriteCode(w, ErrorWithCode(err, code), code) +} diff --git a/common/http_test.go b/common/http_test.go new file mode 100644 index 000000000..b207684b8 --- /dev/null +++ b/common/http_test.go @@ -0,0 +1,250 @@ +package common_test + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "io/ioutil" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/tendermint/tmlibs/common" +) + +func TestWriteSuccess(t *testing.T) { + w := httptest.NewRecorder() + common.WriteSuccess(w, "foo") + assert.Equal(t, w.Code, 200, "should get a 200") +} + +var blankErrResponse = new(common.ErrorResponse) + +func TestWriteError(t *testing.T) { + tests := [...]struct { + msg string + code int + }{ + 0: { + msg: "this is a message", + code: 419, + }, + } + + for i, tt := range tests { + w := httptest.NewRecorder() + msg := tt.msg + + // First check without a defined code, should send back a 400 + common.WriteError(w, errors.New(msg)) + assert.Equal(t, w.Code, http.StatusBadRequest, "#%d: should get a 400", i) + blob, err := ioutil.ReadAll(w.Body) + if err != nil { + assert.Failf(t, "expecting a successful ioutil.ReadAll", "#%d", i) + continue + } + + recv := new(common.ErrorResponse) + if err := json.Unmarshal(blob, recv); err != nil { + assert.Failf(t, "expecting a successful json.Unmarshal", "#%d", i) + continue + } + + assert.Equal(t, reflect.DeepEqual(recv, blankErrResponse), false, "expecting a non-blank error response") + + // Now test with an error that's .HTTPCode() int conforming + + // Reset w + w = httptest.NewRecorder() + + common.WriteError(w, common.ErrorWithCode(errors.New("foo"), tt.code)) + assert.Equal(t, w.Code, tt.code, "case #%d", i) + } +} + +type marshalFailer struct{} + +var errFooFailed = errors.New("foo failed here") + +func (mf *marshalFailer) MarshalJSON() ([]byte, error) { + return nil, errFooFailed +} + +func TestWriteCode(t *testing.T) { + codes := [...]int{ + 0: http.StatusOK, + 1: http.StatusBadRequest, + 2: http.StatusUnauthorized, + 3: http.StatusInternalServerError, + } + + for i, code := range codes { + w := httptest.NewRecorder() + common.WriteCode(w, "foo", code) + assert.Equal(t, w.Code, code, "#%d", i) + + // Then for the failed JSON marshaling + w = httptest.NewRecorder() + common.WriteCode(w, &marshalFailer{}, code) + wantCode := http.StatusBadRequest + assert.Equal(t, w.Code, wantCode, "#%d", i) + assert.True(t, strings.Contains(string(w.Body.Bytes()), errFooFailed.Error()), + "#%d: expected %q in the error message", i, errFooFailed) + } +} + +type saver struct { + Foo int `json:"foo" validate:"min=10"` + Bar string `json:"bar"` +} + +type rcloser struct { + closeOnce sync.Once + body *bytes.Buffer + closeChan chan bool +} + +var errAlreadyClosed = errors.New("already closed") + +func (rc *rcloser) Close() error { + var err = errAlreadyClosed + rc.closeOnce.Do(func() { + err = nil + rc.closeChan <- true + close(rc.closeChan) + }) + return err +} + +func (rc *rcloser) Read(b []byte) (int, error) { + return rc.body.Read(b) +} + +var _ io.ReadCloser = (*rcloser)(nil) + +func makeReq(strBody string) (*http.Request, <-chan bool) { + closeChan := make(chan bool, 1) + buf := new(bytes.Buffer) + buf.Write([]byte(strBody)) + req := &http.Request{ + Header: make(http.Header), + Body: &rcloser{body: buf, closeChan: closeChan}, + } + return req, closeChan +} + +func TestParseRequestJSON(t *testing.T) { + tests := [...]struct { + body string + wantErr bool + useNil bool + }{ + 0: {wantErr: true, body: ``}, + 1: {body: `{}`}, + 2: {body: `{"foo": 2}`}, // Not that the validate tags don't matter here since we are just parsing + 3: {body: `{"foo": "abcd"}`, wantErr: true}, + 4: {useNil: true, wantErr: true}, + } + + for i, tt := range tests { + req, closeChan := makeReq(tt.body) + if tt.useNil { + req.Body = nil + } + sav := new(saver) + err := common.ParseRequestJSON(req, sav) + if tt.wantErr { + assert.NotEqual(t, err, nil, "#%d: want non-nil error", i) + continue + } + assert.Equal(t, err, nil, "#%d: want nil error", i) + wasClosed := <-closeChan + assert.Equal(t, wasClosed, true, "#%d: should have invoked close", i) + } +} + +func TestFparseJSON(t *testing.T) { + r1 := strings.NewReader(`{"foo": 1}`) + sav := new(saver) + require.Equal(t, common.FparseJSON(r1, sav), nil, "expecting successful parsing") + r2 := strings.NewReader(`{"bar": "blockchain"}`) + require.Equal(t, common.FparseJSON(r2, sav), nil, "expecting successful parsing") + require.Equal(t, reflect.DeepEqual(sav, &saver{Foo: 1, Bar: "blockchain"}), true, "should have parsed both") + + // Now with a nil body + require.NotEqual(t, nil, common.FparseJSON(nil, sav), "expecting a nil error report") +} + +func TestFparseAndValidateJSON(t *testing.T) { + r1 := strings.NewReader(`{"foo": 1}`) + sav := new(saver) + require.NotEqual(t, common.FparseAndValidateJSON(r1, sav), nil, "expecting validation to fail") + r1 = strings.NewReader(`{"foo": 100}`) + require.Equal(t, common.FparseJSON(r1, sav), nil, "expecting successful parsing") + r2 := strings.NewReader(`{"bar": "blockchain"}`) + require.Equal(t, common.FparseAndValidateJSON(r2, sav), nil, "expecting successful parsing") + require.Equal(t, reflect.DeepEqual(sav, &saver{Foo: 100, Bar: "blockchain"}), true, "should have parsed both") + + // Now with a nil body + require.NotEqual(t, nil, common.FparseJSON(nil, sav), "expecting a nil error report") +} + +var blankSaver = new(saver) + +func TestParseAndValidateRequestJSON(t *testing.T) { + tests := [...]struct { + body string + wantErr bool + useNil bool + }{ + 0: {wantErr: true, body: ``}, + 1: {body: `{}`, wantErr: true}, // Here it should fail since Foo doesn't meet the minimum value + 2: {body: `{"foo": 2}`, wantErr: true}, // Here validation should fail + 3: {body: `{"foo": "abcd"}`, wantErr: true}, + 4: {useNil: true, wantErr: true}, + 5: {body: `{"foo": 100}`}, // Must succeed + } + + for i, tt := range tests { + req, closeChan := makeReq(tt.body) + if tt.useNil { + req.Body = nil + } + sav := new(saver) + err := common.ParseRequestAndValidateJSON(req, sav) + if tt.wantErr { + assert.NotEqual(t, err, nil, "#%d: want non-nil error", i) + continue + } + + assert.Equal(t, err, nil, "#%d: want nil error", i) + assert.False(t, reflect.DeepEqual(blankSaver, sav), "#%d: expecting a set saver", i) + + wasClosed := <-closeChan + assert.Equal(t, wasClosed, true, "#%d: should have invoked close", i) + } +} + +func TestErrorWithCode(t *testing.T) { + tests := [...]struct { + code int + err error + }{ + 0: {code: 500, err: errors.New("funky")}, + 1: {code: 406, err: errors.New("purist")}, + } + + for i, tt := range tests { + errRes := common.ErrorWithCode(tt.err, tt.code) + assert.Equal(t, errRes.Error(), tt.err.Error(), "#%d: expecting the error values to be equal", i) + assert.Equal(t, errRes.Code, tt.code, "expecting the same status code", i) + assert.Equal(t, errRes.HTTPCode(), tt.code, "expecting the same status code", i) + } +}