fix all bits broken by viper API changes (#5982)

github.com/spf13/viper's recent releases introduced a semantic
change in some public API such as viper.IsSet(), which have
broken some of our flags checks. Instead of checking whether
users have changed a flag's default value we should rely on such
defaults and adjust runtime behaviour accordingly. In order to do
so, it's important that we pick sane defaults for all our flags.

The --pruning flag and configuration option now allow for a
fake custom strategy. When users elect custom, then the
pruning-{keep,snapshot}-every options are interpreted and
parsed; else they're ignored.
Zero is pruning-{keep,snapshot}-every default value. When
users choose to set a custom pruning strategy they are
signalling that they want more fine-grainted control, therefore
it's legitimate to expect them to know what they are doing and
enter valid values for both options.

Ref #5964
This commit is contained in:
Alessio Treglia
2020-04-14 17:24:27 +02:00
committed by GitHub
parent a6588a4d2d
commit e8cedf243f
12 changed files with 147 additions and 161 deletions
+51 -49
View File
@@ -1,6 +1,4 @@
// Package rest provides HTTP types and primitives for REST
// requests validation and responses handling.
package rest
package rest_test
import (
"errors"
@@ -12,24 +10,27 @@ import (
"strings"
"testing"
"github.com/spf13/viper"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/crypto"
"github.com/tendermint/tendermint/crypto/secp256k1"
"github.com/cosmos/cosmos-sdk/client/context"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/rest"
)
func TestBaseReq_Sanitize(t *testing.T) {
t.Parallel()
sanitized := BaseReq{ChainID: " test",
sanitized := rest.BaseReq{ChainID: " test",
Memo: "memo ",
From: " cosmos1cq0sxam6x4l0sv9yz3a2vlqhdhvt2k6jtgcse0 ",
Gas: " ",
GasAdjustment: " 0.3",
}.Sanitize()
require.Equal(t, BaseReq{ChainID: "test",
require.Equal(t, rest.BaseReq{ChainID: "test",
Memo: "memo",
From: "cosmos1cq0sxam6x4l0sv9yz3a2vlqhdhvt2k6jtgcse0",
Gas: "",
@@ -44,25 +45,25 @@ func TestBaseReq_ValidateBasic(t *testing.T) {
onestake, err := types.ParseDecCoins("1.0stake")
require.NoError(t, err)
req1 := NewBaseReq(
req1 := rest.NewBaseReq(
fromAddr, "", "nonempty", "", "", 0, 0, tenstakes, nil, false,
)
req2 := NewBaseReq(
req2 := rest.NewBaseReq(
"", "", "nonempty", "", "", 0, 0, tenstakes, nil, false,
)
req3 := NewBaseReq(
req3 := rest.NewBaseReq(
fromAddr, "", "", "", "", 0, 0, tenstakes, nil, false,
)
req4 := NewBaseReq(
req4 := rest.NewBaseReq(
fromAddr, "", "nonempty", "", "", 0, 0, tenstakes, onestake, false,
)
req5 := NewBaseReq(
req5 := rest.NewBaseReq(
fromAddr, "", "nonempty", "", "", 0, 0, types.Coins{}, types.DecCoins{}, false,
)
tests := []struct {
name string
req BaseReq
req rest.BaseReq
w http.ResponseWriter
want bool
}{
@@ -102,21 +103,21 @@ func TestParseHTTPArgs(t *testing.T) {
limit int
err bool
}{
{"no params", req0, httptest.NewRecorder(), []string{}, DefaultPage, DefaultLimit, false},
{"Limit", req1, httptest.NewRecorder(), []string{}, DefaultPage, 5, false},
{"Page", req2, httptest.NewRecorder(), []string{}, 5, DefaultLimit, false},
{"no params", req0, httptest.NewRecorder(), []string{}, rest.DefaultPage, rest.DefaultLimit, false},
{"Limit", req1, httptest.NewRecorder(), []string{}, rest.DefaultPage, 5, false},
{"Page", req2, httptest.NewRecorder(), []string{}, 5, rest.DefaultLimit, false},
{"Page and limit", req3, httptest.NewRecorder(), []string{}, 5, 5, false},
{"error page 0", reqE1, httptest.NewRecorder(), []string{}, DefaultPage, DefaultLimit, true},
{"error limit 0", reqE2, httptest.NewRecorder(), []string{}, DefaultPage, DefaultLimit, true},
{"error page 0", reqE1, httptest.NewRecorder(), []string{}, rest.DefaultPage, rest.DefaultLimit, true},
{"error limit 0", reqE2, httptest.NewRecorder(), []string{}, rest.DefaultPage, rest.DefaultLimit, true},
{"tags", req4, httptest.NewRecorder(), []string{"foo='faa'"}, DefaultPage, DefaultLimit, false},
{"tags", reqTxH, httptest.NewRecorder(), []string{"tx.height<=14", "tx.height>=12"}, DefaultPage, DefaultLimit, false},
{"tags", req4, httptest.NewRecorder(), []string{"foo='faa'"}, rest.DefaultPage, rest.DefaultLimit, false},
{"tags", reqTxH, httptest.NewRecorder(), []string{"tx.height<=14", "tx.height>=12"}, rest.DefaultPage, rest.DefaultLimit, false},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
tags, page, limit, err := ParseHTTPArgs(tt.req)
tags, page, limit, err := rest.ParseHTTPArgs(tt.req)
sort.Strings(tags)
@@ -158,7 +159,7 @@ func TestParseQueryHeight(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
cliCtx, ok := ParseQueryHeightOrReturnBadRequest(tt.w, tt.cliCtx, tt.req)
cliCtx, ok := rest.ParseQueryHeightOrReturnBadRequest(tt.w, tt.cliCtx, tt.req)
if tt.expectedOk {
require.True(t, ok)
require.Equal(t, tt.expectedHeight, cliCtx.Height)
@@ -185,6 +186,7 @@ func TestProcessPostResponse(t *testing.T) {
}
// setup
viper.Set(flags.FlagOffline, true)
ctx := context.NewCLIContext()
height := int64(194423)
@@ -205,7 +207,7 @@ func TestProcessPostResponse(t *testing.T) {
jsonNoIndent, err := ctx.Codec.MarshalJSON(acc)
require.Nil(t, err)
respNoIndent := NewResponseWithHeight(height, jsonNoIndent)
respNoIndent := rest.NewResponseWithHeight(height, jsonNoIndent)
expectedNoIndent, err := ctx.Codec.MarshalJSON(respNoIndent)
require.Nil(t, err)
@@ -215,7 +217,7 @@ func TestProcessPostResponse(t *testing.T) {
// check that negative height writes an error
w := httptest.NewRecorder()
ctx = ctx.WithHeight(-1)
PostProcessResponse(w, ctx, acc)
rest.PostProcessResponse(w, ctx, acc)
require.Equal(t, http.StatusInternalServerError, w.Code)
// check that height returns expected response
@@ -231,21 +233,21 @@ func TestReadRESTReq(t *testing.T) {
reqBody := ioutil.NopCloser(strings.NewReader(`{"chain_id":"alessio","memo":"text"}`))
req := &http.Request{Body: reqBody}
w := httptest.NewRecorder()
var br BaseReq
var br rest.BaseReq
// test OK
ReadRESTReq(w, req, codec.New(), &br)
rest.ReadRESTReq(w, req, codec.New(), &br)
res := w.Result()
t.Cleanup(func() { res.Body.Close() })
require.Equal(t, BaseReq{ChainID: "alessio", Memo: "text"}, br)
require.Equal(t, rest.BaseReq{ChainID: "alessio", Memo: "text"}, br)
require.Equal(t, http.StatusOK, res.StatusCode)
// test non valid JSON
reqBody = ioutil.NopCloser(strings.NewReader(`MALFORMED`))
req = &http.Request{Body: reqBody}
br = BaseReq{}
br = rest.BaseReq{}
w = httptest.NewRecorder()
ReadRESTReq(w, req, codec.New(), &br)
rest.ReadRESTReq(w, req, codec.New(), &br)
require.Equal(t, br, br)
res = w.Result()
t.Cleanup(func() { res.Body.Close() })
@@ -255,7 +257,7 @@ func TestReadRESTReq(t *testing.T) {
func TestWriteSimulationResponse(t *testing.T) {
t.Parallel()
w := httptest.NewRecorder()
WriteSimulationResponse(w, codec.New(), 10)
rest.WriteSimulationResponse(w, codec.New(), 10)
res := w.Result()
t.Cleanup(func() { res.Body.Close() })
require.Equal(t, http.StatusOK, res.StatusCode)
@@ -268,12 +270,12 @@ func TestWriteSimulationResponse(t *testing.T) {
func TestParseUint64OrReturnBadRequest(t *testing.T) {
t.Parallel()
w := httptest.NewRecorder()
_, ok := ParseUint64OrReturnBadRequest(w, "100")
_, ok := rest.ParseUint64OrReturnBadRequest(w, "100")
require.True(t, ok)
require.Equal(t, http.StatusOK, w.Result().StatusCode)
w = httptest.NewRecorder()
_, ok = ParseUint64OrReturnBadRequest(w, "-100")
_, ok = rest.ParseUint64OrReturnBadRequest(w, "-100")
require.False(t, ok)
require.Equal(t, http.StatusBadRequest, w.Result().StatusCode)
}
@@ -281,17 +283,17 @@ func TestParseUint64OrReturnBadRequest(t *testing.T) {
func TestParseFloat64OrReturnBadRequest(t *testing.T) {
t.Parallel()
w := httptest.NewRecorder()
_, ok := ParseFloat64OrReturnBadRequest(w, "100", 0)
_, ok := rest.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)
_, ok = rest.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)
ret, ok := rest.ParseFloat64OrReturnBadRequest(w, "", 9.0)
require.Equal(t, float64(9), ret)
require.True(t, ok)
require.Equal(t, http.StatusOK, w.Result().StatusCode)
@@ -299,11 +301,11 @@ func TestParseFloat64OrReturnBadRequest(t *testing.T) {
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"))
require.True(t, rest.ParseQueryParamBool(req, "boolean"))
require.False(t, rest.ParseQueryParamBool(req, "nokey"))
req = httptest.NewRequest("GET", "/target?boolean=false", nil)
require.False(t, ParseQueryParamBool(req, "boolean"))
require.False(t, ParseQueryParamBool(req, ""))
require.False(t, rest.ParseQueryParamBool(req, "boolean"))
require.False(t, rest.ParseQueryParamBool(req, ""))
}
func TestPostProcessResponseBare(t *testing.T) {
@@ -314,7 +316,7 @@ func TestPostProcessResponseBare(t *testing.T) {
w := httptest.NewRecorder()
bs := []byte("text string")
PostProcessResponseBare(w, ctx, bs)
rest.PostProcessResponseBare(w, ctx, bs)
res := w.Result()
require.Equal(t, http.StatusOK, res.StatusCode)
@@ -333,7 +335,7 @@ func TestPostProcessResponseBare(t *testing.T) {
S string `json:"s"`
}{X: 10, S: "test"}
PostProcessResponseBare(w, ctx, data)
rest.PostProcessResponseBare(w, ctx, data)
res = w.Result()
require.Equal(t, http.StatusOK, res.StatusCode)
@@ -355,7 +357,7 @@ func TestPostProcessResponseBare(t *testing.T) {
S string `json:"s"`
}{X: 10, S: "test"}
PostProcessResponseBare(w, ctx, data)
rest.PostProcessResponseBare(w, ctx, data)
res = w.Result()
require.Equal(t, http.StatusOK, res.StatusCode)
@@ -371,7 +373,7 @@ func TestPostProcessResponseBare(t *testing.T) {
w = httptest.NewRecorder()
data2 := badJSONMarshaller{}
PostProcessResponseBare(w, ctx, data2)
rest.PostProcessResponseBare(w, ctx, data2)
res = w.Result()
require.Equal(t, http.StatusInternalServerError, res.StatusCode)
@@ -401,7 +403,7 @@ func runPostProcessResponse(t *testing.T, ctx context.CLIContext, obj interface{
// test using regular struct
w := httptest.NewRecorder()
PostProcessResponse(w, ctx, obj)
rest.PostProcessResponse(w, ctx, obj)
require.Equal(t, http.StatusOK, w.Code, w.Body)
resp := w.Result()
@@ -421,7 +423,7 @@ func runPostProcessResponse(t *testing.T, ctx context.CLIContext, obj interface{
// test using marshalled struct
w = httptest.NewRecorder()
PostProcessResponse(w, ctx, marshalled)
rest.PostProcessResponse(w, ctx, marshalled)
require.Equal(t, http.StatusOK, w.Code, w.Body)
resp = w.Result()
@@ -452,12 +454,12 @@ func TestCheckErrors(t *testing.T) {
wantString string
wantStatus int
}{
{"500", CheckInternalServerError, err, true, `{"error":"ERROR"}`, http.StatusInternalServerError},
{"500 (no error)", CheckInternalServerError, nil, false, ``, http.StatusInternalServerError},
{"400", CheckBadRequestError, err, true, `{"error":"ERROR"}`, http.StatusBadRequest},
{"400 (no error)", CheckBadRequestError, nil, false, ``, http.StatusBadRequest},
{"404", CheckNotFoundError, err, true, `{"error":"ERROR"}`, http.StatusNotFound},
{"404 (no error)", CheckNotFoundError, nil, false, ``, http.StatusNotFound},
{"500", rest.CheckInternalServerError, err, true, `{"error":"ERROR"}`, http.StatusInternalServerError},
{"500 (no error)", rest.CheckInternalServerError, nil, false, ``, http.StatusInternalServerError},
{"400", rest.CheckBadRequestError, err, true, `{"error":"ERROR"}`, http.StatusBadRequest},
{"400 (no error)", rest.CheckBadRequestError, nil, false, ``, http.StatusBadRequest},
{"404", rest.CheckNotFoundError, err, true, `{"error":"ERROR"}`, http.StatusNotFound},
{"404 (no error)", rest.CheckNotFoundError, nil, false, ``, http.StatusNotFound},
}
for _, tt := range tests {