diff --git a/.pending/breaking/_All-REST-responses- b/.pending/breaking/_All-REST-responses- new file mode 100644 index 0000000000..9b61f0f085 --- /dev/null +++ b/.pending/breaking/_All-REST-responses- @@ -0,0 +1,2 @@ +All REST responses now wrap the original resource/result. The response +will contain two fields: height and result. diff --git a/.pending/bugfixes/_Return-height-in-re b/.pending/bugfixes/_Return-height-in-re new file mode 100644 index 0000000000..f4c332a690 --- /dev/null +++ b/.pending/bugfixes/_Return-height-in-re @@ -0,0 +1 @@ +Return height in responses when querying against BaseApp \ No newline at end of file diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index d421a4f076..b086737b7e 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -463,6 +463,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc return abci.ResponseQuery{ Code: uint32(sdk.CodeOK), Codespace: string(sdk.CodespaceRoot), + Height: req.Height, Value: []byte(app.appVersion), } @@ -474,6 +475,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc return abci.ResponseQuery{ Code: uint32(sdk.CodeOK), Codespace: string(sdk.CodespaceRoot), + Height: req.Height, Value: value, } } @@ -482,7 +484,7 @@ func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) (res abc return sdk.ErrUnknownRequest(msg).QueryResult() } -func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) (res abci.ResponseQuery) { +func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery { // "/store" prefix for store queries queryable, ok := app.cms.(sdk.Queryable) if !ok { @@ -491,7 +493,11 @@ func handleQueryStore(app *BaseApp, path []string, req abci.RequestQuery) (res a } req.Path = "/" + strings.Join(path[1:], "/") - return queryable.Query(req) + + resp := queryable.Query(req) + resp.Height = req.Height + + return resp } func handleQueryP2P(app *BaseApp, path []string, _ abci.RequestQuery) (res abci.ResponseQuery) { @@ -503,9 +509,11 @@ func handleQueryP2P(app *BaseApp, path []string, _ abci.RequestQuery) (res abci. switch typ { case "addr": return app.FilterPeerByAddrPort(arg) + case "id": return app.FilterPeerByID(arg) } + default: msg := "Expected second parameter to be filter" return sdk.ErrUnknownRequest(msg).QueryResult() @@ -554,13 +562,15 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res return abci.ResponseQuery{ Code: uint32(err.Code()), Codespace: string(err.Codespace()), + Height: req.Height, Log: err.ABCILog(), } } return abci.ResponseQuery{ - Code: uint32(sdk.CodeOK), - Value: resBytes, + Code: uint32(sdk.CodeOK), + Height: req.Height, + Value: resBytes, } } diff --git a/client/context/query.go b/client/context/query.go index 87f96aece1..2dc8ab217f 100644 --- a/client/context/query.go +++ b/client/context/query.go @@ -81,6 +81,16 @@ func (ctx CLIContext) query(path string, key cmn.HexBytes) (res []byte, height i return res, height, err } + // When a client did not provide a query height, manually query for it so it can + // be injected downstream into responses. + if ctx.Height == 0 { + status, err := node.Status() + if err != nil { + return res, height, err + } + ctx = ctx.WithHeight(status.SyncInfo.LatestBlockHeight) + } + opts := rpcclient.ABCIQueryOptions{ Height: ctx.Height, Prove: !ctx.TrustNode, diff --git a/types/rest/rest.go b/types/rest/rest.go index 3041b67796..c138ceb084 100644 --- a/types/rest/rest.go +++ b/types/rest/rest.go @@ -24,6 +24,21 @@ const ( DefaultLimit = 30 // should be consistent with tendermint/tendermint/rpc/core/pipe.go:19 ) +// ResponseWithHeight defines a response object type that wraps an original +// response with a height. +type ResponseWithHeight struct { + Height int64 `json:"height"` + Result json.RawMessage `json:"result"` +} + +// NewResponseWithHeight creates a new ResponseWithHeight instance +func NewResponseWithHeight(height int64, result json.RawMessage) ResponseWithHeight { + return ResponseWithHeight{ + Height: height, + Result: result, + } +} + // GasEstimateResponse defines a response definition for tx gas estimation. type GasEstimateResponse struct { GasEstimate uint64 `json:"gas_estimate"` @@ -222,28 +237,27 @@ func ParseQueryHeightOrReturnBadRequest(w http.ResponseWriter, cliCtx context.CL return cliCtx, true } -// PostProcessResponse performs post processing for a REST response. -// If the height is greater than zero it will be injected into the body -// of the response. An internal server error is written to the response -// if the height is negative or an encoding/decoding error occurs. -func PostProcessResponse(w http.ResponseWriter, cliCtx context.CLIContext, response interface{}) { - var output []byte +// PostProcessResponse performs post processing for a REST response. The result +// returned to clients will contain two fields, the height at which the resource +// was queried at and the original result. +func PostProcessResponse(w http.ResponseWriter, cliCtx context.CLIContext, resp interface{}) { + var result []byte if cliCtx.Height < 0 { WriteErrorResponse(w, http.StatusInternalServerError, fmt.Errorf("negative height in response").Error()) return } - switch response.(type) { + switch resp.(type) { case []byte: - output = response.([]byte) + result = resp.([]byte) default: var err error if cliCtx.Indent { - output, err = cliCtx.Codec.MarshalJSONIndent(response, "", " ") + result, err = cliCtx.Codec.MarshalJSONIndent(resp, "", " ") } else { - output, err = cliCtx.Codec.MarshalJSON(response) + result, err = cliCtx.Codec.MarshalJSON(resp) } if err != nil { @@ -252,29 +266,22 @@ func PostProcessResponse(w http.ResponseWriter, cliCtx context.CLIContext, respo } } - // inject the height into the response by: - // - decoding into a map - // - adding the height to the map - // - encoding using standard JSON library - if cliCtx.Height > 0 { - m := make(map[string]interface{}) - err := json.Unmarshal(output, &m) - if err != nil { - WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) - return - } + wrappedResp := NewResponseWithHeight(cliCtx.Height, result) - m["height"] = cliCtx.Height + var ( + output []byte + err error + ) - if cliCtx.Indent { - output, err = json.MarshalIndent(m, "", " ") - } else { - output, err = json.Marshal(m) - } - if err != nil { - WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) - return - } + if cliCtx.Indent { + output, err = cliCtx.Codec.MarshalJSONIndent(wrappedResp, "", " ") + } else { + output, err = cliCtx.Codec.MarshalJSON(wrappedResp) + } + + if err != nil { + WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) + return } w.Header().Set("Content-Type", "application/json") diff --git a/types/rest/rest_test.go b/types/rest/rest_test.go index e14f88490b..dd9d800b1a 100644 --- a/types/rest/rest_test.go +++ b/types/rest/rest_test.go @@ -3,12 +3,10 @@ package rest import ( - "encoding/json" "io" "io/ioutil" "net/http" "net/http/httptest" - "strconv" "testing" "github.com/stretchr/testify/require" @@ -148,6 +146,7 @@ func TestProcessPostResponse(t *testing.T) { // mock account // PubKey field ensures amino encoding is used first since standard // JSON encoding will panic on crypto.PubKey + type mockAccount struct { Address types.AccAddress `json:"address"` Coins types.Coins `json:"coins"` @@ -173,23 +172,17 @@ func TestProcessPostResponse(t *testing.T) { cdc.RegisterConcrete(&mockAccount{}, "cosmos-sdk/mockAccount", nil) ctx = ctx.WithCodec(cdc) - // setup expected json responses with zero height - jsonNoHeight, err := cdc.MarshalJSON(acc) + // setup expected results + jsonNoIndent, err := ctx.Codec.MarshalJSON(acc) require.Nil(t, err) - require.NotNil(t, jsonNoHeight) - jsonIndentNoHeight, err := cdc.MarshalJSONIndent(acc, "", " ") + jsonWithIndent, err := ctx.Codec.MarshalJSONIndent(acc, "", " ") require.Nil(t, err) - require.NotNil(t, jsonIndentNoHeight) - - // decode into map to order alphabetically - m := make(map[string]interface{}) - err = json.Unmarshal(jsonNoHeight, &m) + respNoIndent := NewResponseWithHeight(height, jsonNoIndent) + respWithIndent := NewResponseWithHeight(height, jsonWithIndent) + expectedNoIndent, err := ctx.Codec.MarshalJSON(respNoIndent) require.Nil(t, err) - jsonMap, err := json.Marshal(m) + expectedWithIndent, err := ctx.Codec.MarshalJSONIndent(respWithIndent, "", " ") require.Nil(t, err) - jsonWithHeight := append(append([]byte(`{"height":`), []byte(strconv.Itoa(int(height))+",")...), jsonMap[1:]...) - jsonIndentMap, err := json.MarshalIndent(m, "", " ") - jsonIndentWithHeight := append(append([]byte(`{`+"\n "+` "height": `), []byte(strconv.Itoa(int(height))+",")...), jsonIndentMap[1:]...) // check that negative height writes an error w := httptest.NewRecorder() @@ -197,24 +190,17 @@ func TestProcessPostResponse(t *testing.T) { PostProcessResponse(w, ctx, acc) require.Equal(t, http.StatusInternalServerError, w.Code) - // check that zero height returns expected response - ctx = ctx.WithHeight(0) - runPostProcessResponse(t, ctx, acc, jsonNoHeight, false) - // check zero height with indent - runPostProcessResponse(t, ctx, acc, jsonIndentNoHeight, true) // check that height returns expected response ctx = ctx.WithHeight(height) - runPostProcessResponse(t, ctx, acc, jsonWithHeight, false) + runPostProcessResponse(t, ctx, acc, expectedNoIndent, false) // check height with indent - runPostProcessResponse(t, ctx, acc, jsonIndentWithHeight, true) + runPostProcessResponse(t, ctx, acc, expectedWithIndent, true) } // asserts that ResponseRecorder returns the expected code and body // runs PostProcessResponse on the objects regular interface and on // the marshalled struct. -func runPostProcessResponse(t *testing.T, ctx context.CLIContext, obj interface{}, - expectedBody []byte, indent bool, -) { +func runPostProcessResponse(t *testing.T, ctx context.CLIContext, obj interface{}, expectedBody []byte, indent bool) { if indent { ctx.Indent = indent }