From 719030f4ed5bad44be0e2ee3ace12cc335e991ae Mon Sep 17 00:00:00 2001 From: Tyler <48813565+technicallyty@users.noreply.github.com> Date: Fri, 25 Mar 2022 09:22:30 -0700 Subject: [PATCH] fix(ORM): paginated iterator bug (#11432) ## Description - fixes a bug where iterators were invalid when limit == len(table entries) and CountTotal = true. Closes: #11431 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- orm/model/ormtable/paginate.go | 22 +++++++++++++----- orm/model/ormtable/table_test.go | 39 +++++++++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/orm/model/ormtable/paginate.go b/orm/model/ormtable/paginate.go index c14040796d..56f30cf8fb 100644 --- a/orm/model/ormtable/paginate.go +++ b/orm/model/ormtable/paginate.go @@ -55,18 +55,28 @@ func (it *paginationIterator) Next() bool { if it.i >= it.done { it.pageRes = &queryv1beta1.PageResponse{} cursor := it.Cursor() - if it.Iterator.Next() { + next := it.Iterator.Next() + if next { it.pageRes.NextKey = cursor it.i++ } if it.countTotal { - for { - if !it.Iterator.Next() { - it.pageRes.Total = uint64(it.i) - return false + // once it.Iterator.Next() returns false, another call to it will panic. + // we check next here to ensure we do not call it again. + if next { + for { + if !it.Iterator.Next() { + it.pageRes.Total = uint64(it.i) + return false + } + it.i++ } - it.i++ + } else { + // when next is false, the iterator can no longer move forward, + // so the index == total entries. + it.pageRes.Total = uint64(it.i) } + } return false } diff --git a/orm/model/ormtable/table_test.go b/orm/model/ormtable/table_test.go index 4b1a88ff07..06df3f3cb4 100644 --- a/orm/model/ormtable/table_test.go +++ b/orm/model/ormtable/table_test.go @@ -4,15 +4,14 @@ import ( "bytes" "context" "fmt" - "google.golang.org/protobuf/types/known/timestamppb" "sort" "strings" "testing" "time" - dbm "github.com/tendermint/tm-db" + "google.golang.org/protobuf/types/known/timestamppb" - "github.com/cosmos/cosmos-sdk/orm/types/kv" + dbm "github.com/tendermint/tm-db" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" @@ -21,6 +20,8 @@ import ( "gotest.tools/v3/golden" "pgregory.net/rapid" + "github.com/cosmos/cosmos-sdk/orm/types/kv" + queryv1beta1 "github.com/cosmos/cosmos-sdk/api/cosmos/base/query/v1beta1" sdkerrors "github.com/cosmos/cosmos-sdk/errors" "github.com/cosmos/cosmos-sdk/orm/encoding/ormkv" @@ -67,6 +68,38 @@ func TestScenario(t *testing.T) { checkEncodeDecodeEntries(t, table, store.IndexStoreReader()) } +// isolated test for bug - https://github.com/cosmos/cosmos-sdk/issues/11431 +func TestPaginationLimitCountTotal(t *testing.T) { + table, err := ormtable.Build(ormtable.Options{ + MessageType: (&testpb.ExampleTable{}).ProtoReflect().Type(), + }) + backend := testkv.NewSplitMemBackend() + ctx := ormtable.WrapContextDefault(backend) + store, err := testpb.NewExampleTableTable(table) + assert.NilError(t, err) + + assert.NilError(t, store.Insert(ctx, &testpb.ExampleTable{U32: 4, I64: 2, Str: "co"})) + assert.NilError(t, store.Insert(ctx, &testpb.ExampleTable{U32: 5, I64: 2, Str: "sm"})) + assert.NilError(t, store.Insert(ctx, &testpb.ExampleTable{U32: 6, I64: 2, Str: "os"})) + + it, err := store.List(ctx, &testpb.ExampleTablePrimaryKey{}, ormlist.Paginate(&queryv1beta1.PageRequest{Limit: 3, CountTotal: true})) + assert.NilError(t, err) + assert.Check(t, it.Next()) + + it, err = store.List(ctx, &testpb.ExampleTablePrimaryKey{}, ormlist.Paginate(&queryv1beta1.PageRequest{Limit: 4, CountTotal: true})) + assert.NilError(t, err) + assert.Check(t, it.Next()) + + it, err = store.List(ctx, &testpb.ExampleTablePrimaryKey{}, ormlist.Paginate(&queryv1beta1.PageRequest{Limit: 1, CountTotal: true})) + assert.NilError(t, err) + for it.Next() { + } + pr := it.PageResponse() + assert.Check(t, pr != nil) + assert.Equal(t, uint64(3), pr.Total) + +} + func TestImportedMessageIterator(t *testing.T) { table, err := ormtable.Build(ormtable.Options{ MessageType: (&testpb.ExampleTimestamp{}).ProtoReflect().Type(),