Skip to content

Commit

Permalink
fix(ORM): paginated iterator bug (#11432)
Browse files Browse the repository at this point in the history
## 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)
  • Loading branch information
technicallyty authored Mar 25, 2022
1 parent a69764f commit 719030f
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 9 deletions.
22 changes: 16 additions & 6 deletions orm/model/ormtable/paginate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
39 changes: 36 additions & 3 deletions orm/model/ormtable/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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(),
Expand Down

0 comments on commit 719030f

Please sign in to comment.