diff --git a/CHANGELOG.md b/CHANGELOG.md index d198aa7bb015..ad2a9a102562 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ and provided directly the IAVL store. * (types) [\#5533](https://github.com/cosmos/cosmos-sdk/pull/5533) Refactored `AppModuleBasic` and `AppModuleGenesis` to now accept a `codec.JSONMarshaler` for modular serialization of genesis state. * (crypto/keys) [\#5735](https://github.com/cosmos/cosmos-sdk/pull/5735) Keyring's Update() function is now no-op. +* (types/rest) [\#5779](https://github.com/cosmos/cosmos-sdk/pull/5779) Drop unused Parse{Int64OrReturnBadRequest,QueryParamBool}() functions. ### Features diff --git a/types/module/module_test.go b/types/module/module_test.go index 5cafaf10939e..db2334696292 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -32,8 +32,8 @@ func TestBasicManager(t *testing.T) { mockAppModuleBasic1 := mocks.NewMockAppModuleBasic(mockCtrl) mockAppModuleBasic2 := mocks.NewMockAppModuleBasic(mockCtrl) - mockAppModuleBasic1.EXPECT().Name().Times(3).Return("mockAppModuleBasic1") - mockAppModuleBasic2.EXPECT().Name().Times(2).Return("mockAppModuleBasic2") + mockAppModuleBasic1.EXPECT().Name().AnyTimes().Return("mockAppModuleBasic1") + mockAppModuleBasic2.EXPECT().Name().AnyTimes().Return("mockAppModuleBasic2") mockAppModuleBasic1.EXPECT().DefaultGenesis(gomock.Eq(cdc)).Times(1).Return(json.RawMessage(``)) mockAppModuleBasic2.EXPECT().DefaultGenesis(gomock.Eq(cdc)).Times(1).Return(json.RawMessage(`{"key":"value"}`)) mockAppModuleBasic1.EXPECT().ValidateGenesis(gomock.Eq(cdc), gomock.Eq(wantDefaultGenesis["mockAppModuleBasic1"])).Times(1).Return(errFoo) diff --git a/types/rest/rest.go b/types/rest/rest.go index a6339097e765..258917586248 100644 --- a/types/rest/rest.go +++ b/types/rest/rest.go @@ -171,20 +171,6 @@ func WriteSimulationResponse(w http.ResponseWriter, cdc *codec.Codec, gas uint64 _, _ = w.Write(resp) } -// ParseInt64OrReturnBadRequest converts s to a int64 value. -func ParseInt64OrReturnBadRequest(w http.ResponseWriter, s string) (n int64, ok bool) { - var err error - - n, err = strconv.ParseInt(s, 10, 64) - if err != nil { - err := fmt.Errorf("'%s' is not a valid int64", s) - WriteErrorResponse(w, http.StatusBadRequest, err.Error()) - return n, false - } - - return n, true -} - // ParseUint64OrReturnBadRequest converts s to a uint64 value. func ParseUint64OrReturnBadRequest(w http.ResponseWriter, s string) (n uint64, ok bool) { var err error @@ -386,10 +372,8 @@ func ParseHTTPArgs(r *http.Request) (tags []string, page, limit int, err error) // ParseQueryParamBool parses the given param to a boolean. It returns false by // default if the string is not parseable to bool. func ParseQueryParamBool(r *http.Request, param string) bool { - valueStr := r.FormValue(param) - value := false - if ok, err := strconv.ParseBool(valueStr); err == nil { - value = ok + if value, err := strconv.ParseBool(r.FormValue(param)); err == nil { + return value } - return value + return false } diff --git a/types/rest/rest_test.go b/types/rest/rest_test.go index 6088569c8f09..e760627b4d4a 100644 --- a/types/rest/rest_test.go +++ b/types/rest/rest_test.go @@ -3,11 +3,13 @@ package rest import ( + "errors" "io" "io/ioutil" "net/http" "net/http/httptest" "sort" + "strings" "testing" "github.com/stretchr/testify/require" @@ -19,7 +21,23 @@ import ( "github.com/cosmos/cosmos-sdk/types" ) -func TestBaseReqValidateBasic(t *testing.T) { +func TestBaseReq_Sanitize(t *testing.T) { + t.Parallel() + sanitized := BaseReq{ChainID: " test", + Memo: "memo ", + From: " cosmos1cq0sxam6x4l0sv9yz3a2vlqhdhvt2k6jtgcse0 ", + Gas: " ", + GasAdjustment: " 0.3", + }.Sanitize() + require.Equal(t, BaseReq{ChainID: "test", + Memo: "memo", + From: "cosmos1cq0sxam6x4l0sv9yz3a2vlqhdhvt2k6jtgcse0", + Gas: "", + GasAdjustment: "0.3", + }, sanitized) +} + +func TestBaseReq_ValidateBasic(t *testing.T) { fromAddr := "cosmos1cq0sxam6x4l0sv9yz3a2vlqhdhvt2k6jtgcse0" tenstakes, err := types.ParseCoins("10stake") require.NoError(t, err) @@ -63,6 +81,7 @@ func TestBaseReqValidateBasic(t *testing.T) { } func TestParseHTTPArgs(t *testing.T) { + t.Parallel() req0 := mustNewRequest(t, "", "/", nil) req1 := mustNewRequest(t, "", "/?limit=5", nil) req2 := mustNewRequest(t, "", "/?page=5", nil) @@ -114,6 +133,7 @@ func TestParseHTTPArgs(t *testing.T) { } func TestParseQueryHeight(t *testing.T) { + t.Parallel() var emptyHeight int64 height := int64(1256756) @@ -155,6 +175,7 @@ func TestProcessPostResponse(t *testing.T) { // PubKey field ensures amino encoding is used first since standard // JSON encoding will panic on crypto.PubKey + t.Parallel() type mockAccount struct { Address types.AccAddress `json:"address"` Coins types.Coins `json:"coins"` @@ -205,6 +226,151 @@ func TestProcessPostResponse(t *testing.T) { runPostProcessResponse(t, ctx, acc, expectedWithIndent, true) } +func TestReadRESTReq(t *testing.T) { + t.Parallel() + reqBody := ioutil.NopCloser(strings.NewReader(`{"chain_id":"alessio","memo":"text"}`)) + req := &http.Request{Body: reqBody} + w := httptest.NewRecorder() + var br BaseReq + + // test OK + ReadRESTReq(w, req, codec.New(), &br) + require.Equal(t, BaseReq{ChainID: "alessio", Memo: "text"}, br) + res := w.Result() + require.Equal(t, http.StatusOK, res.StatusCode) + + // test non valid JSON + reqBody = ioutil.NopCloser(strings.NewReader(`MALFORMED`)) + req = &http.Request{Body: reqBody} + br = BaseReq{} + w = httptest.NewRecorder() + ReadRESTReq(w, req, codec.New(), &br) + require.Equal(t, br, br) + res = w.Result() + require.Equal(t, http.StatusBadRequest, res.StatusCode) +} + +func TestWriteSimulationResponse(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + WriteSimulationResponse(w, codec.New(), 10) + res := w.Result() + require.Equal(t, http.StatusOK, res.StatusCode) + bs, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + t.Cleanup(func() { res.Body.Close() }) + require.Equal(t, `{"gas_estimate":"10"}`, string(bs)) +} + +func TestParseUint64OrReturnBadRequest(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + _, ok := ParseUint64OrReturnBadRequest(w, "100") + require.True(t, ok) + require.Equal(t, http.StatusOK, w.Result().StatusCode) + + w = httptest.NewRecorder() + _, ok = ParseUint64OrReturnBadRequest(w, "-100") + require.False(t, ok) + require.Equal(t, http.StatusBadRequest, w.Result().StatusCode) +} + +func TestParseFloat64OrReturnBadRequest(t *testing.T) { + t.Parallel() + w := httptest.NewRecorder() + _, ok := ParseFloat64OrReturnBadRequest(w, "100", 0) + require.True(t, ok) + require.Equal(t, http.StatusOK, w.Result().StatusCode) + + w = httptest.NewRecorder() + _, ok = ParseFloat64OrReturnBadRequest(w, "bad request", 0) + require.False(t, ok) + require.Equal(t, http.StatusBadRequest, w.Result().StatusCode) + + w = httptest.NewRecorder() + ret, ok := ParseFloat64OrReturnBadRequest(w, "", 9.0) + require.Equal(t, float64(9), ret) + require.True(t, ok) + require.Equal(t, http.StatusOK, w.Result().StatusCode) +} + +func TestParseQueryParamBool(t *testing.T) { + req := httptest.NewRequest("GET", "/target?boolean=true", nil) + require.True(t, ParseQueryParamBool(req, "boolean")) + require.False(t, ParseQueryParamBool(req, "nokey")) + req = httptest.NewRequest("GET", "/target?boolean=false", nil) + require.False(t, ParseQueryParamBool(req, "boolean")) + require.False(t, ParseQueryParamBool(req, "")) +} + +func TestPostProcessResponseBare(t *testing.T) { + t.Parallel() + + // write bytes + ctx := context.CLIContext{} + w := httptest.NewRecorder() + bs := []byte("text string") + PostProcessResponseBare(w, ctx, bs) + res := w.Result() + require.Equal(t, http.StatusOK, res.StatusCode) + got, err := ioutil.ReadAll(res.Body) + require.NoError(t, err) + t.Cleanup(func() { res.Body.Close() }) + require.Equal(t, "text string", string(got)) + + // write struct and indent response + ctx = context.CLIContext{Indent: true}.WithCodec(codec.New()) + w = httptest.NewRecorder() + data := struct { + X int `json:"x"` + S string `json:"s"` + }{X: 10, S: "test"} + PostProcessResponseBare(w, ctx, data) + res = w.Result() + require.Equal(t, http.StatusOK, res.StatusCode) + got, err = ioutil.ReadAll(res.Body) + require.NoError(t, err) + t.Cleanup(func() { res.Body.Close() }) + require.Equal(t, `{ + "x": "10", + "s": "test" +}`, string(got)) + + // write struct, don't indent response + ctx = context.CLIContext{Indent: false}.WithCodec(codec.New()) + w = httptest.NewRecorder() + data = struct { + X int `json:"x"` + S string `json:"s"` + }{X: 10, S: "test"} + PostProcessResponseBare(w, ctx, data) + res = w.Result() + require.Equal(t, http.StatusOK, res.StatusCode) + got, err = ioutil.ReadAll(res.Body) + require.NoError(t, err) + t.Cleanup(func() { res.Body.Close() }) + require.Equal(t, `{"x":"10","s":"test"}`, string(got)) + + // test marshalling failure + ctx = context.CLIContext{Indent: false}.WithCodec(codec.New()) + w = httptest.NewRecorder() + data2 := badJSONMarshaller{} + PostProcessResponseBare(w, ctx, data2) + res = w.Result() + require.Equal(t, http.StatusInternalServerError, res.StatusCode) + got, err = ioutil.ReadAll(res.Body) + require.NoError(t, err) + t.Cleanup(func() { res.Body.Close() }) + require.Equal(t, []string([]string{"application/json"}), res.Header["Content-Type"]) + require.Equal(t, `{"error":"couldn't marshal"}`, string(got)) +} + +type badJSONMarshaller struct{} + +func (_ badJSONMarshaller) MarshalJSON() ([]byte, error) { + return nil, errors.New("couldn't marshal") +} + // asserts that ResponseRecorder returns the expected code and body // runs PostProcessResponse on the objects regular interface and on // the marshalled struct. @@ -218,7 +384,7 @@ func runPostProcessResponse(t *testing.T, ctx context.CLIContext, obj interface{ PostProcessResponse(w, ctx, obj) require.Equal(t, http.StatusOK, w.Code, w.Body) resp := w.Result() - defer resp.Body.Close() + t.Cleanup(func() { resp.Body.Close() }) body, err := ioutil.ReadAll(resp.Body) require.Nil(t, err) require.Equal(t, expectedBody, body) @@ -236,7 +402,7 @@ func runPostProcessResponse(t *testing.T, ctx context.CLIContext, obj interface{ PostProcessResponse(w, ctx, marshalled) require.Equal(t, http.StatusOK, w.Code, w.Body) resp = w.Result() - defer resp.Body.Close() + t.Cleanup(func() { resp.Body.Close() }) body, err = ioutil.ReadAll(resp.Body) require.Nil(t, err) require.Equal(t, expectedBody, body)