Fix nextKey bug in filteredPaginate (cosmos#6578)

* Fix nextKey in filteredpaginate * Fix example * Fix example * cleanup * cleanup * refactor Co-authored-by: sahith-narahari <[email protected]>
jackywww · Jul 3, 2020 · a966f1f · a966f1f
1 parent 6326374
commit a966f1f
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 22 deletions.
diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
@@ -9,9 +9,9 @@ import (
 	dbm "github.com/tendermint/tm-db"
 
 	"github.com/cosmos/cosmos-sdk/store/iavl"
+	sdkmaps "github.com/cosmos/cosmos-sdk/store/rootmulti/internal/maps"
 	"github.com/cosmos/cosmos-sdk/store/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-	sdkmaps "github.com/cosmos/cosmos-sdk/store/rootmulti/internal/maps"
 )
 
 func TestStoreType(t *testing.T) {

diff --git a/types/query/filtered_pagination.go b/types/query/filtered_pagination.go
@@ -85,6 +85,7 @@ func FilteredPaginate(
 		if iterator.Error() != nil {
 			return nil, iterator.Error()
 		}
+
 		accumulate := numHits >= offset && numHits < end
 		hit, err := onResult(iterator.Key(), iterator.Value(), accumulate)
 		if err != nil {
@@ -95,7 +96,7 @@ func FilteredPaginate(
 			numHits++
 		}
 
-		if numHits == end {
+		if numHits == end+1 {
 			nextKey = iterator.Key()
 
 			if !countTotal {

diff --git a/types/query/filtered_pagination_test.go b/types/query/filtered_pagination_test.go
@@ -20,7 +20,6 @@ func TestFilteredPaginations(t *testing.T) {
 	app, ctx, appCodec := setupTest()
 
 	var balances sdk.Coins
-
 	for i := 0; i < numBalances; i++ {
 		denom := fmt.Sprintf("foo%ddenom", i)
 		balances = append(balances, sdk.NewInt64Coin(denom, 100))
@@ -40,56 +39,52 @@ func TestFilteredPaginations(t *testing.T) {
 	// verify pagination with limit > total values
 	pageReq := &query.PageRequest{Key: nil, Limit: 5, CountTotal: true}
 	balances, res, err := execFilterPaginate(store, pageReq, appCodec)
-
 	require.NoError(t, err)
 	require.NotNil(t, res)
 	require.Equal(t, 4, len(balances))
 
-	// verify empty request
+	t.Log("verify empty request")
 	balances, res, err = execFilterPaginate(store, nil, appCodec)
-
 	require.NoError(t, err)
 	require.NotNil(t, res)
 	require.Equal(t, 4, len(balances))
 	require.Equal(t, uint64(4), res.Total)
+	require.Nil(t, res.NextKey)
 
-	// verify next key is returned
+	t.Log("verify nextKey is returned if there are more results")
 	pageReq = &query.PageRequest{Key: nil, Limit: 2, CountTotal: true}
 	balances, res, err = execFilterPaginate(store, pageReq, appCodec)
-
 	require.NoError(t, err)
 	require.NotNil(t, res)
 	require.Equal(t, 2, len(balances))
 	require.NotNil(t, res.NextKey)
+	require.Equal(t, string(res.NextKey), fmt.Sprintf("test2denom"))
 	require.Equal(t, uint64(4), res.Total)
 
-	// use next key for query
+	t.Log("verify both key and offset can't be given")
+	pageReq = &query.PageRequest{Key: res.NextKey, Limit: 1, Offset: 2, CountTotal: true}
+	_, _, err = execFilterPaginate(store, pageReq, appCodec)
+	require.Error(t, err)
+
+	t.Log("use nextKey for query")
 	pageReq = &query.PageRequest{Key: res.NextKey, Limit: 2, CountTotal: true}
 	balances, res, err = execFilterPaginate(store, pageReq, appCodec)
-
 	require.NoError(t, err)
 	require.NotNil(t, res)
 	require.Equal(t, 2, len(balances))
-	require.NotNil(t, res.NextKey)
-
-	// verify both key and offset can't be given
-	pageReq = &query.PageRequest{Key: res.NextKey, Limit: 1, Offset: 2, CountTotal: true}
-	balances, res, err = execFilterPaginate(store, pageReq, appCodec)
-	require.Error(t, err)
+	require.Nil(t, res.NextKey)
 
-	// verify default limit
+	t.Log("verify default limit")
 	pageReq = &query.PageRequest{Key: nil, Limit: 0}
 	balances, res, err = execFilterPaginate(store, pageReq, appCodec)
-
 	require.NoError(t, err)
 	require.NotNil(t, res)
 	require.Equal(t, 4, len(balances))
 	require.Equal(t, uint64(4), res.Total)
 
-	// verify offset
+	t.Log("verify with offset")
 	pageReq = &query.PageRequest{Offset: 2, Limit: 2}
 	balances, res, err = execFilterPaginate(store, pageReq, appCodec)
-
 	require.NoError(t, err)
 	require.NotNil(t, res)
 	require.LessOrEqual(t, len(balances), 2)
@@ -99,7 +94,6 @@ func ExampleFilteredPaginate() {
 	app, ctx, appCodec := setupTest()
 
 	var balances sdk.Coins
-
 	for i := 0; i < numBalances; i++ {
 		denom := fmt.Sprintf("foo%ddenom", i)
 		balances = append(balances, sdk.NewInt64Coin(denom, 100))
@@ -147,7 +141,7 @@ func ExampleFilteredPaginate() {
 	}
 	fmt.Println(&types.QueryAllBalancesResponse{Balances: balResult, Res: res})
 	// Output:
-	// balances:<denom:"test0denom" amount:"250" > res:<next_key:"test0denom" total:5 >
+	// balances:<denom:"test0denom" amount:"250" > res:<next_key:"test1denom" total:5 >
 }
 
 func execFilterPaginate(store sdk.KVStore, pageReq *query.PageRequest, appCodec codec.Marshaler) (balances sdk.Coins, res *query.PageResponse, err error) {