From 7fec663f2c0ef651b21a637d53fa9dd73958a2aa Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Fri, 27 Mar 2020 10:36:10 +0100 Subject: [PATCH] Add support for limit/filter --- compose/repository/attachment.go | 2 +- compose/repository/chart.go | 2 +- compose/repository/module.go | 2 +- compose/repository/namespace.go | 2 +- compose/repository/page.go | 2 +- compose/repository/record.go | 2 +- pkg/rh/paging.go | 91 ++++++++++++++++++++++++-------- pkg/rh/paging_test.go | 73 +++++++++++++++++++++++++ pkg/rh/selectors.go | 25 ++++++--- system/repository/application.go | 2 +- system/repository/reminder.go | 2 +- system/repository/role.go | 2 +- system/repository/user.go | 2 +- 13 files changed, 169 insertions(+), 40 deletions(-) create mode 100644 pkg/rh/paging_test.go diff --git a/compose/repository/attachment.go b/compose/repository/attachment.go index 53498b06f..4c45bae24 100644 --- a/compose/repository/attachment.go +++ b/compose/repository/attachment.go @@ -153,7 +153,7 @@ func (r attachment) Find(filter types.AttachmentFilter) (set types.AttachmentSet return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r attachment) Create(mod *types.Attachment) (*types.Attachment, error) { diff --git a/compose/repository/chart.go b/compose/repository/chart.go index b057bb187..c2047652a 100644 --- a/compose/repository/chart.go +++ b/compose/repository/chart.go @@ -133,7 +133,7 @@ func (r chart) Find(filter types.ChartFilter) (set types.ChartSet, f types.Chart return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r chart) Create(mod *types.Chart) (*types.Chart, error) { diff --git a/compose/repository/module.go b/compose/repository/module.go index b2fea3d77..80be22984 100644 --- a/compose/repository/module.go +++ b/compose/repository/module.go @@ -154,7 +154,7 @@ func (r module) Find(filter types.ModuleFilter) (set types.ModuleSet, f types.Mo return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r module) Create(mod *types.Module) (*types.Module, error) { diff --git a/compose/repository/namespace.go b/compose/repository/namespace.go index b7c48ee53..45edf2f13 100644 --- a/compose/repository/namespace.go +++ b/compose/repository/namespace.go @@ -131,7 +131,7 @@ func (r *namespace) Find(filter types.NamespaceFilter) (set types.NamespaceSet, return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r *namespace) Create(mod *types.Namespace) (*types.Namespace, error) { diff --git a/compose/repository/page.go b/compose/repository/page.go index daa1661ea..662888535 100644 --- a/compose/repository/page.go +++ b/compose/repository/page.go @@ -153,7 +153,7 @@ func (r page) Find(filter types.PageFilter) (set types.PageSet, f types.PageFilt return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r page) Reorder(namespaceID, parentID uint64, pageIDs []uint64) error { diff --git a/compose/repository/record.go b/compose/repository/record.go index 3dce83a41..57ad62417 100644 --- a/compose/repository/record.go +++ b/compose/repository/record.go @@ -137,7 +137,7 @@ func (r record) Find(module *types.Module, filter types.RecordFilter) (set types return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } // Export ignores paging and does not return filter diff --git a/pkg/rh/paging.go b/pkg/rh/paging.go index fb6897596..fc4944d23 100644 --- a/pkg/rh/paging.go +++ b/pkg/rh/paging.go @@ -9,15 +9,24 @@ const ( ) type ( - // @todo this needs to be refactored to support - // limit/offset params alongside page/perPage + // PageFilter supports page/perPage (one based) and limit/offset + // pagination. + // + // Limit/offset is prioritised over page/perPage + // PageFilter struct { - Page uint `json:"page"` - PerPage uint `json:"perPage"` - Count uint `json:"count"` + // If limit is set to a positive number, + // paging mechanisms will use limit/offset + // Otherwise page/perPage is used + Limit uint `json:"limit,omitempty"` + Offset uint `json:"offset,omitempty"` - // Limit uint `json:"limit"` - // Offset uint `json:"offset"` + Page uint `json:"page,omitempty"` + PerPage uint `json:"perPage,omitempty"` + + // Count is used when filter and pagination are send back + // with the response + Count uint `json:"count"` } ) @@ -32,24 +41,62 @@ func Paging(page, perPage uint) PageFilter { } } -func (pf *PageFilter) ParsePagination(input interface{}) { +// Limit creates PageFilter struct from limit and, optionally offset +func Limit(a ...uint) PageFilter { + switch len(a) { + case 1: + return PageFilter{Limit: a[0]} + case 2: + return PageFilter{Limit: a[0], Offset: a[1]} + } + + return PageFilter{} +} + +func (pf *PageFilter) ParsePagination(input interface{}) error { + return parsePagination(pf, input) +} + +func parsePagination(pf *PageFilter, input interface{}) (err error) { switch i := input.(type) { case map[string]string: - if len(i["limit"]+i["offset"]) > 0 { - // @todo to properly & fully support limit & offset - // we need to refactor pagination handling - limit, _ := strconv.ParseUint(i["limit"], 10, 32) - //offset, _ := strconv.ParseUint(i["offset"], 10, 32) + conv := func(v *uint, name string) error { + if _, has := i[name]; has { + pv, err := strconv.ParseUint(i[name], 10, 32) + if err != nil { + return err + } - // only basic support for now due to - // limitation of PageFilter implementation - pf.PerPage = uint(limit) - //pf.Page = offset / limit - } else if len(i["page"]+i["perPage"]) > 0 { - page, _ := strconv.ParseUint(i["page"], 10, 32) - perPage, _ := strconv.ParseUint(i["perPage"], 10, 32) - pf.Page = uint(page) - pf.PerPage = uint(perPage) + *v = uint(pv) + } + + return nil + } + + if len(i["limit"]+i["offset"]) > 0 { + if err = conv(&pf.Limit, "limit"); err != nil { + return + } + + if err = conv(&pf.Offset, "offset"); err != nil { + return + } + + return + } + + if len(i["page"]+i["perPage"]) > 0 { + if err = conv(&pf.Page, "page"); err != nil { + return + } + + if err = conv(&pf.PerPage, "perPage"); err != nil { + return + } + + return } } + + return nil } diff --git a/pkg/rh/paging_test.go b/pkg/rh/paging_test.go new file mode 100644 index 000000000..76d6ca465 --- /dev/null +++ b/pkg/rh/paging_test.go @@ -0,0 +1,73 @@ +package rh + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestLimit(t *testing.T) { + var ( + r = require.New(t) + ) + + r.Equal(Limit(42).Limit, uint(42)) + r.Equal(Limit(0, 42).Offset, uint(42)) +} + +func Test_parsePagination(t *testing.T) { + var ( + tests = []struct { + name string + args interface{} + pf PageFilter + wantErr bool + }{ + { + "empty", + nil, + PageFilter{}, + false, + }, + { + "valid l/o", + map[string]string{"limit": "42", "offset": "314"}, + PageFilter{Limit: 42, Offset: 314}, + false, + }, + { + "mixed", + map[string]string{"page": "42", "limit": "314"}, + PageFilter{Limit: 314, Offset: 0}, + false, + }, + { + "invalid limit", + map[string]string{"limit": "abc"}, + PageFilter{}, + true, + }, + { + "invalid page", + map[string]string{"page": "abc"}, + PageFilter{}, + true, + }, + } + ) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var ( + pf = PageFilter{} + ) + + if err := parsePagination(&pf, tt.args); (err != nil) != tt.wantErr { + t.Errorf("parsePagination() error = %v, wantErr %v", err, tt.wantErr) + } else if !reflect.DeepEqual(pf, tt.pf) { + t.Errorf("\n actual: %v\nexpected: %v\n", pf, tt.pf) + } + }) + } +} diff --git a/pkg/rh/selectors.go b/pkg/rh/selectors.go index 30c459dca..812146d38 100644 --- a/pkg/rh/selectors.go +++ b/pkg/rh/selectors.go @@ -45,18 +45,27 @@ func Count(db *factory.DB, q squirrel.SelectBuilder) (count uint, err error) { } // FetchPaged fetches paged rows -func FetchPaged(db *factory.DB, q squirrel.SelectBuilder, page, perPage uint, set interface{}) error { - if perPage > 0 { - q = q.Limit(uint64(perPage)) +func FetchPaged(db *factory.DB, q squirrel.SelectBuilder, p PageFilter, set interface{}) error { + if p.Limit+p.Offset == 0 { + // When both, offset & limit are 0, + // calculate both values from page/perPage params + if p.PerPage > 0 { + p.Limit = p.PerPage + } + + if p.Page < 1 { + p.Page = 1 + } + + p.Offset = uint((p.Page - 1) * p.PerPage) } - if page < 1 { - page = 1 + if p.Limit > 0 { + q = q.Limit(uint64(p.Limit)) } - var offset = uint64((page - 1) * perPage) - if offset > 0 { - q = q.Offset(offset) + if p.Offset > 0 { + q = q.Offset(uint64(p.Limit)) } return FetchAll(db, q, set) diff --git a/system/repository/application.go b/system/repository/application.go index 303e9a1ef..fa65556e1 100644 --- a/system/repository/application.go +++ b/system/repository/application.go @@ -126,7 +126,7 @@ func (r *application) Find(filter types.ApplicationFilter) (set types.Applicatio return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r *application) Create(mod *types.Application) (*types.Application, error) { diff --git a/system/repository/reminder.go b/system/repository/reminder.go index 950845603..218bd63e6 100644 --- a/system/repository/reminder.go +++ b/system/repository/reminder.go @@ -147,7 +147,7 @@ func (r reminder) Find(filter types.ReminderFilter) (set types.ReminderSet, f ty return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r reminder) Create(mod *types.Reminder) (rm *types.Reminder, err error) { diff --git a/system/repository/role.go b/system/repository/role.go index 19435e78c..2da62cfdc 100644 --- a/system/repository/role.go +++ b/system/repository/role.go @@ -170,7 +170,7 @@ func (r *role) Find(filter types.RoleFilter) (set types.RoleSet, f types.RoleFil return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r *role) Create(mod *types.Role) (*types.Role, error) { diff --git a/system/repository/user.go b/system/repository/user.go index cc888f800..f4eaa8b5e 100644 --- a/system/repository/user.go +++ b/system/repository/user.go @@ -207,7 +207,7 @@ func (r user) Find(filter types.UserFilter) (set types.UserSet, f types.UserFilt return } - return set, f, rh.FetchPaged(r.db(), query, f.Page, f.PerPage, &set) + return set, f, rh.FetchPaged(r.db(), query, f.PageFilter, &set) } func (r user) Total() (count uint) {