From d8dfaacb4ea3d0c28baa982d77de3b18b3b8bb13 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 1 Jul 2019 21:28:40 -0400 Subject: [PATCH 1/4] Generalize querier pagination --- client/utils.go | 25 +++++++++++++++++++++++++ x/slashing/querier.go | 17 +++-------------- x/staking/keeper/querier.go | 17 +++-------------- x/supply/keeper/querier.go | 16 +++------------- 4 files changed, 34 insertions(+), 41 deletions(-) create mode 100644 client/utils.go diff --git a/client/utils.go b/client/utils.go new file mode 100644 index 000000000000..01f963a6d4ac --- /dev/null +++ b/client/utils.go @@ -0,0 +1,25 @@ +package client + +// Paginate returns the correct starting and ending index for a paginated query, +// given that client provides a desired page and limit of objects and the handler +// provides the total number of objects. If the start page is invalid, non-positive +// values are returned signaling the request is invalid. +func Paginate(numObjs, page, limit, defLimit int) (start, end int) { + if limit == 0 { + limit = defLimit + } + + start = (page - 1) * limit + end = limit + start + + if end >= numObjs { + end = numObjs + } + + if start >= numObjs { + // page is out of bounds + return -1, -1 + } + + return start, end +} diff --git a/x/slashing/querier.go b/x/slashing/querier.go index fa4b56aefcca..32be853f9d41 100644 --- a/x/slashing/querier.go +++ b/x/slashing/querier.go @@ -5,6 +5,7 @@ import ( abci "github.com/tendermint/tendermint/abci/types" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -65,11 +66,6 @@ func querySigningInfos(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byte return nil, sdk.ErrInternal(fmt.Sprintf("failed to parse params: %s", err)) } - if params.Limit == 0 { - // set the default limit to max bonded if no limit was provided - params.Limit = int(k.sk.MaxValidators(ctx)) - } - var signingInfos []ValidatorSigningInfo k.IterateValidatorSigningInfos(ctx, func(consAddr sdk.ConsAddress, info ValidatorSigningInfo) (stop bool) { @@ -77,15 +73,8 @@ func querySigningInfos(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byte return false }) - // get pagination bounds - start := (params.Page - 1) * params.Limit - end := params.Limit + start - if end >= len(signingInfos) { - end = len(signingInfos) - } - - if start >= len(signingInfos) { - // page is out of bounds + start, end := client.Paginate(len(signingInfos), params.Page, params.Limit, int(k.sk.MaxValidators(ctx))) + if start < 0 || end < 0 { signingInfos = []ValidatorSigningInfo{} } else { signingInfos = signingInfos[start:end] diff --git a/x/staking/keeper/querier.go b/x/staking/keeper/querier.go index 0feb465e0658..4cc4be19d841 100644 --- a/x/staking/keeper/querier.go +++ b/x/staking/keeper/querier.go @@ -6,6 +6,7 @@ import ( abci "github.com/tendermint/tendermint/abci/types" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -55,11 +56,6 @@ func queryValidators(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byte, return nil, sdk.ErrInternal(fmt.Sprintf("failed to parse params: %s", err)) } - stakingParams := k.GetParams(ctx) - if params.Limit == 0 { - params.Limit = int(stakingParams.MaxValidators) - } - validators := k.GetAllValidators(ctx) filteredVals := make([]types.Validator, 0, len(validators)) @@ -69,15 +65,8 @@ func queryValidators(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byte, } } - // get pagination bounds - start := (params.Page - 1) * params.Limit - end := params.Limit + start - if end >= len(filteredVals) { - end = len(filteredVals) - } - - if start >= len(filteredVals) { - // page is out of bounds + start, end := client.Paginate(len(filteredVals), params.Page, params.Limit, int(k.GetParams(ctx).MaxValidators)) + if start < 0 || end < 0 { filteredVals = []types.Validator{} } else { filteredVals = filteredVals[start:end] diff --git a/x/supply/keeper/querier.go b/x/supply/keeper/querier.go index 8410a09fb37f..fe34d9a52db1 100644 --- a/x/supply/keeper/querier.go +++ b/x/supply/keeper/querier.go @@ -5,6 +5,7 @@ import ( abci "github.com/tendermint/tendermint/abci/types" + "github.com/cosmos/cosmos-sdk/client" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/supply/types" ) @@ -32,20 +33,9 @@ func queryTotalSupply(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byte, } totalSupply := k.GetSupply(ctx).Total - totalSupplyLen := len(totalSupply) - if params.Limit == 0 { - params.Limit = totalSupplyLen - } - - start := (params.Page - 1) * params.Limit - end := params.Limit + start - if end >= totalSupplyLen { - end = totalSupplyLen - } - - if start >= totalSupplyLen { - // page is out of bounds + start, end := client.Paginate(len(totalSupply), params.Page, params.Limit, 100) + if start < 0 || end < 0 { totalSupply = sdk.Coins{} } else { totalSupply = totalSupply[start:end] From 561773b872a547be7f299ed34b4bb6026fd75cd0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 1 Jul 2019 21:47:52 -0400 Subject: [PATCH 2/4] Implement unit tests --- client/utils.go | 2 ++ client/utils_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 client/utils_test.go diff --git a/client/utils.go b/client/utils.go index 01f963a6d4ac..f61116ba7c53 100644 --- a/client/utils.go +++ b/client/utils.go @@ -4,6 +4,8 @@ package client // given that client provides a desired page and limit of objects and the handler // provides the total number of objects. If the start page is invalid, non-positive // values are returned signaling the request is invalid. +// +// NOTE: The start page is assumed to be 1-indexed. func Paginate(numObjs, page, limit, defLimit int) (start, end int) { if limit == 0 { limit = defLimit diff --git a/client/utils_test.go b/client/utils_test.go new file mode 100644 index 000000000000..a89b4ad82f47 --- /dev/null +++ b/client/utils_test.go @@ -0,0 +1,56 @@ +package client_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/client" +) + +func TestPaginate(t *testing.T) { + testCases := []struct { + name string + numObjs, page, limit, defLimit int + expectedStart, expectedEnd int + }{ + { + "all objects in a single page", + 100, 1, 100, 100, + 0, 100, + }, + { + "page one of three", + 75, 1, 25, 100, + 0, 25, + }, + { + "page two of three", + 75, 2, 25, 100, + 25, 50, + }, + { + "page three of three", + 75, 3, 25, 100, + 50, 75, + }, + { + "end is greater than total number of objects", + 75, 2, 50, 100, + 50, 75, + }, + { + "invalid start page", + 75, 4, 25, 100, + -1, -1, + }, + } + + for i, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + start, end := client.Paginate(tc.numObjs, tc.page, tc.limit, tc.defLimit) + require.Equal(t, tc.expectedStart, start, "invalid result; test case #%d", i) + require.Equal(t, tc.expectedEnd, end, "invalid result; test case #%d", i) + }) + } +} From 7ca950250082ce81616ec0238ba3bedac97d563a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 1 Jul 2019 21:52:53 -0400 Subject: [PATCH 3/4] Add pending log entry --- .pending/improvements/sdk/4601-Implement-gener | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .pending/improvements/sdk/4601-Implement-gener diff --git a/.pending/improvements/sdk/4601-Implement-gener b/.pending/improvements/sdk/4601-Implement-gener new file mode 100644 index 000000000000..e3f951b50352 --- /dev/null +++ b/.pending/improvements/sdk/4601-Implement-gener @@ -0,0 +1,2 @@ +#4601 Implement generic pangination helper function to be used in +REST handlers and queriers. From ee130bb542606a7688986ea553aa02634de42f60 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 2 Jul 2019 08:52:06 -0400 Subject: [PATCH 4/4] Add check for zero page --- client/utils.go | 5 ++++- client/utils_test.go | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/client/utils.go b/client/utils.go index f61116ba7c53..1a71dfcea66d 100644 --- a/client/utils.go +++ b/client/utils.go @@ -7,7 +7,10 @@ package client // // NOTE: The start page is assumed to be 1-indexed. func Paginate(numObjs, page, limit, defLimit int) (start, end int) { - if limit == 0 { + if page == 0 { + // invalid start page + return -1, -1 + } else if limit == 0 { limit = defLimit } diff --git a/client/utils_test.go b/client/utils_test.go index a89b4ad82f47..ebc00573ffe1 100644 --- a/client/utils_test.go +++ b/client/utils_test.go @@ -44,6 +44,11 @@ func TestPaginate(t *testing.T) { 75, 4, 25, 100, -1, -1, }, + { + "invalid zero start page", + 75, 0, 25, 100, + -1, -1, + }, } for i, tc := range testCases {