From 7869d38e94d5e691f453880d1be40af60121b85a Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 15 Feb 2022 12:56:22 -0500 Subject: [PATCH] feat(orm): support nil PageRequest (#11171) ## Description Closes: #XXXX --- ### 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/internal/listinternal/options.go | 9 +++--- orm/model/ormlist/options.go | 28 ++++++++++++++-- orm/model/ormtable/iterator.go | 2 +- orm/model/ormtable/paginate.go | 3 ++ orm/model/ormtable/table_test.go | 13 ++++++++ .../ormtable/testdata/test_scenario.golden | 32 +++++++++++++++++++ 6 files changed, 79 insertions(+), 8 deletions(-) diff --git a/orm/internal/listinternal/options.go b/orm/internal/listinternal/options.go index 4bd65a4f2f9f..fb5c254dc88d 100644 --- a/orm/internal/listinternal/options.go +++ b/orm/internal/listinternal/options.go @@ -8,11 +8,10 @@ import ( // Options is the internal list options struct. type Options struct { - Reverse bool - Cursor []byte - Filter func(proto.Message) bool - Offset, Limit uint64 - CountTotal bool + Reverse, CountTotal bool + Offset, Limit, DefaultLimit uint64 + Cursor []byte + Filter func(proto.Message) bool } func (o Options) Validate() error { diff --git a/orm/model/ormlist/options.go b/orm/model/ormlist/options.go index 8a6007c8c629..16f216838cee 100644 --- a/orm/model/ormlist/options.go +++ b/orm/model/ormlist/options.go @@ -39,11 +39,26 @@ func Cursor(cursor CursorT) Option { // Paginate paginates iterator output based on the provided page request. // The Iterator.PageRequest value on the returned iterator will be non-nil // after Iterator.Next() returns false when this option is provided. -// Care should be taken when using Paginate together with Reverse and/or Cursor -// and generally this should be avoided. +// +// Care should be taken when using Paginate together with Reverse and/or Cursor. +// In the case of combining Reverse and Paginate, if pageRequest.Reverse is +// true then iteration will proceed in the forward direction. This allows +// the default iteration direction for a query to be reverse with the option +// to switch this (to forward in this case) using PageRequest. If Cursor +// and Paginate are used together, whichever option is used first wins. +// If pageRequest is nil, this option will be a no-op so the caller does not +// need to do a nil check. This function defines no default limit, so if +// the caller does not define a limit, this will return all results. To +// specify a default limit use the DefaultLimit option. func Paginate(pageRequest *queryv1beta1.PageRequest) Option { return listinternal.FuncOption(func(options *listinternal.Options) { + if pageRequest == nil { + return + } + if pageRequest.Reverse { + // if the reverse is true we invert the direction of iteration, + // meaning if iteration was already reversed we set it forward. options.Reverse = !options.Reverse } @@ -54,5 +69,14 @@ func Paginate(pageRequest *queryv1beta1.PageRequest) Option { }) } +// DefaultLimit specifies a default limit for iteration. This option can be +// combined with Paginate to ensure that there is a default limit if none +// is specified in PageRequest. +func DefaultLimit(defaultLimit uint64) Option { + return listinternal.FuncOption(func(options *listinternal.Options) { + options.DefaultLimit = defaultLimit + }) +} + // CursorT defines a cursor type. type CursorT []byte diff --git a/orm/model/ormtable/iterator.go b/orm/model/ormtable/iterator.go index a4300957fefb..3aa62d314acf 100644 --- a/orm/model/ormtable/iterator.go +++ b/orm/model/ormtable/iterator.go @@ -188,7 +188,7 @@ func applyCommonIteratorOptions(iterator Iterator, options *listinternal.Options iterator = &filterIterator{Iterator: iterator, filter: options.Filter} } - if options.CountTotal || options.Limit != 0 || options.Offset != 0 { + if options.CountTotal || options.Limit != 0 || options.Offset != 0 || options.DefaultLimit != 0 { iterator = paginate(iterator, options) } diff --git a/orm/model/ormtable/paginate.go b/orm/model/ormtable/paginate.go index b56a911eb5c9..c14040796d81 100644 --- a/orm/model/ormtable/paginate.go +++ b/orm/model/ormtable/paginate.go @@ -11,6 +11,9 @@ import ( func paginate(it Iterator, options *listinternal.Options) Iterator { offset := int(options.Offset) limit := int(options.Limit) + if limit == 0 { + limit = int(options.DefaultLimit) + } i := 0 if offset != 0 { diff --git a/orm/model/ormtable/table_test.go b/orm/model/ormtable/table_test.go index 69f96863885c..78e81a85d998 100644 --- a/orm/model/ormtable/table_test.go +++ b/orm/model/ormtable/table_test.go @@ -221,6 +221,19 @@ func runTestScenario(t *testing.T, table ormtable.Table, backend ormtable.Backen assert.Equal(t, uint64(10), res.Total) assert.Assert(t, res.NextKey != nil) + // let's use a default limit + it, err = store.List(ctx, testpb.ExampleTablePrimaryKey{}, + ormlist.DefaultLimit(4), + ormlist.Paginate(&queryv1beta1.PageRequest{ + CountTotal: true, + })) + assert.NilError(t, err) + assertIteratorItems(it, 0, 1, 2, 3) + res = it.PageResponse() + assert.Assert(t, res != nil) + assert.Equal(t, uint64(10), res.Total) + assert.Assert(t, res.NextKey != nil) + // read another page it, err = store.List(ctx, testpb.ExampleTablePrimaryKey{}, ormlist.Paginate(&queryv1beta1.PageRequest{ Key: res.NextKey, diff --git a/orm/model/ormtable/testdata/test_scenario.golden b/orm/model/ormtable/testdata/test_scenario.golden index 3f55342eeeec..be7a6b74ec31 100644 --- a/orm/model/ormtable/testdata/test_scenario.golden +++ b/orm/model/ormtable/testdata/test_scenario.golden @@ -412,6 +412,38 @@ ITERATOR 0100 -> 0101 VALID true NEXT VALID false +ITERATOR 0100 -> 0101 + VALID true + KEY 010000047ffffffffffffffe616263 1007 + PK testpb.ExampleTable 4/-2/abc -> {"u32":4,"u64":7,"str":"abc","i64":-2} + NEXT + VALID true + KEY 010000047ffffffffffffffe616264 1007 + PK testpb.ExampleTable 4/-2/abd -> {"u32":4,"u64":7,"str":"abd","i64":-2} + NEXT + VALID true + KEY 010000047fffffffffffffff616263 1008 + PK testpb.ExampleTable 4/-1/abc -> {"u32":4,"u64":8,"str":"abc","i64":-1} + NEXT + VALID true + KEY 010000057ffffffffffffffe616264 1008 + PK testpb.ExampleTable 5/-2/abd -> {"u32":5,"u64":8,"str":"abd","i64":-2} + KEY 010000057ffffffffffffffe616264 1008 + PK testpb.ExampleTable 5/-2/abd -> {"u32":5,"u64":8,"str":"abd","i64":-2} + NEXT + VALID true + NEXT + VALID true + NEXT + VALID true + NEXT + VALID true + NEXT + VALID true + NEXT + VALID true + NEXT + VALID false ITERATOR 010000057ffffffffffffffe61626400 -> 0101 VALID true KEY 010000057ffffffffffffffe616265 1009