From a1722f4d9d988317387fb8223b3cdac37e95a692 Mon Sep 17 00:00:00 2001 From: Jacek Kucharczyk Date: Fri, 6 Sep 2019 12:29:54 +0200 Subject: [PATCH] Minor pagination issues fixed. --- config/processor.go | 14 +++++++++ internal/processes.go | 7 +++++ query/const.go | 1 + query/pagination.go | 64 ++++++++++++++++++++++++++++++++++++--- query/pagination_test.go | 60 +++++++++++++++++++++++++++++++++--- query/process-lists.go | 13 ++++++++ query/processor.go | 1 + query/scope-pagination.go | 6 ++-- 8 files changed, 153 insertions(+), 13 deletions(-) diff --git a/config/processor.go b/config/processor.go index 8227eab..e170656 100644 --- a/config/processor.go +++ b/config/processor.go @@ -71,9 +71,11 @@ func DefaultProcessorConfig() map[string]interface{} { internal.ProcessTxCommitOrRollback, }, "get_processes": []string{ + internal.ProcessCheckPagination, internal.ProcessFillEmptyFieldset, internal.ProcessConvertRelationFiltersSafe, internal.ProcessHookBeforeGet, + internal.ProcessCheckPagination, internal.ProcessConvertRelationFiltersSafe, internal.ProcessGet, internal.ProcessGetForeignRelations, @@ -81,9 +83,11 @@ func DefaultProcessorConfig() map[string]interface{} { internal.ProcessGetIncludedSafe, }, "list_processes": []string{ + internal.ProcessCheckPagination, internal.ProcessFillEmptyFieldset, internal.ProcessConvertRelationFiltersSafe, internal.ProcessHookBeforeList, + internal.ProcessCheckPagination, internal.ProcessConvertRelationFiltersSafe, internal.ProcessList, internal.ProcessGetForeignRelationsSafe, @@ -139,9 +143,11 @@ func ThreadSafeProcessor() *Processor { internal.ProcessGetIncludedSafe, }, ListProcesses: ProcessList{ + internal.ProcessCheckPagination, internal.ProcessFillEmptyFieldset, internal.ProcessConvertRelationFiltersSafe, internal.ProcessHookBeforeList, + internal.ProcessCheckPagination, internal.ProcessConvertRelationFiltersSafe, internal.ProcessList, internal.ProcessGetForeignRelationsSafe, @@ -187,18 +193,22 @@ func ConcurrentProcessor() *Processor { internal.ProcessTxCommitOrRollback, }, GetProcesses: ProcessList{ + internal.ProcessCheckPagination, internal.ProcessFillEmptyFieldset, internal.ProcessConvertRelationFilters, internal.ProcessHookBeforeGet, + internal.ProcessCheckPagination, internal.ProcessConvertRelationFilters, internal.ProcessGet, internal.ProcessGetForeignRelations, internal.ProcessHookAfterGet, }, ListProcesses: ProcessList{ + internal.ProcessCheckPagination, internal.ProcessFillEmptyFieldset, internal.ProcessConvertRelationFilters, internal.ProcessHookBeforeList, + internal.ProcessCheckPagination, internal.ProcessConvertRelationFilters, internal.ProcessList, internal.ProcessGetForeignRelations, @@ -244,17 +254,21 @@ func DefaultConcurrentProcessorConfig() map[string]interface{} { internal.ProcessTxCommitOrRollback, }, "get_processes": []string{ + internal.ProcessCheckPagination, internal.ProcessFillEmptyFieldset, internal.ProcessConvertRelationFilters, internal.ProcessHookBeforeGet, + internal.ProcessCheckPagination, internal.ProcessConvertRelationFilters, internal.ProcessGet, internal.ProcessGetForeignRelations, }, "list_processes": []string{ + internal.ProcessCheckPagination, internal.ProcessFillEmptyFieldset, internal.ProcessConvertRelationFilters, internal.ProcessHookBeforeList, + internal.ProcessCheckPagination, internal.ProcessConvertRelationFilters, internal.ProcessList, internal.ProcessGetForeignRelations, diff --git a/internal/processes.go b/internal/processes.go index 41c3a97..2380ce8 100644 --- a/internal/processes.go +++ b/internal/processes.go @@ -5,6 +5,7 @@ var Processes = make(map[string]struct{}) // Processes constant names. const ( + // Create processes ProcessHookBeforeCreate = "hook_before_create" ProcessSetBelongsToRelations = "set_belongs_to_relations" ProcessCreate = "create" @@ -13,6 +14,7 @@ const ( ProcessPatchForeignRelationsSafe = "patch_foreign_relationships_safe" ProcessHookAfterCreate = "hook_after_create" + // Get processes ProcessFillEmptyFieldset = "fill_empty_fieldset" ProcessConvertRelationFilters = "convert_relation_filters" ProcessConvertRelationFiltersSafe = "convert_relation_filters_safe" @@ -22,17 +24,21 @@ const ( ProcessGetForeignRelationsSafe = "get_foreign_relations_safe" ProcessHookAfterGet = "hook_after_get" + // List processes + ProcessCheckPagination = "check_pagination" ProcessHookBeforeList = "hook_before_list" ProcessList = "list" ProcessHookAfterList = "hook_after_list" ProcessGetIncluded = "get_included" ProcessGetIncludedSafe = "get_included_safe" + // Patch processes ProcessHookBeforePatch = "hook_before_patch" ProcessPatch = "patch" ProcessHookAfterPatch = "hook_after_patch" ProcessPatchBelongsToRelations = "patch_belongs_to_relations" + // Delete processes ProcessReducePrimaryFilters = "reduce_primary_filters" ProcessHookBeforeDelete = "hook_before_delete" ProcessDelete = "delete" @@ -40,6 +46,7 @@ const ( ProcessDeleteForeignRelations = "delete_foreign_relations" ProcessDeleteForeignRelationsSafe = "delete_foreign_relations_safe" + // Transaction processes ProcessTxBegin = "tx_begin" ProcessTxCommitOrRollback = "tx_commit_or_rollback" ) diff --git a/query/const.go b/query/const.go index d48b026..d8f8477 100644 --- a/query/const.go +++ b/query/const.go @@ -56,6 +56,7 @@ const ( ProcessGetForeignRelationsSafe = internal.ProcessGetForeignRelationsSafe ProcessHookAfterGet = internal.ProcessHookAfterGet + ProcessCheckPagination = internal.ProcessCheckPagination ProcessHookBeforeList = internal.ProcessHookBeforeList ProcessList = internal.ProcessList ProcessHookAfterList = internal.ProcessHookAfterList diff --git a/query/pagination.go b/query/pagination.go index 6eec648..d67f097 100644 --- a/query/pagination.go +++ b/query/pagination.go @@ -49,6 +49,10 @@ const ( // Pagination defines the query limits and offsets. // It defines the maximum size (Limit) as well as an offset at which // the query should start. +// If the pagination type is 'LimitOffsetPagination' the value of 'Size' defines the 'limit' +// where the value of 'Offset' defines it's 'offset'. +// If the pagination type is 'PageNumberPagination' the value of 'Size' defines 'pageSize' +// and the value of 'Offset' defines 'pageNumber'. The page number value starts from '1'. type Pagination struct { // Size is a pagination value that defines 'limit' or 'page size' Size int @@ -57,11 +61,61 @@ type Pagination struct { Type PaginationType } -// Check checks if the pagination is well formed. -func (p *Pagination) Check() error { +// IsValid checks if the pagination is well formed. +func (p *Pagination) IsValid() error { return p.checkValues() } +// GetLimitOffset gets the 'limit' and 'offset' values from the given pagination. +// If the pagination type is 'LimitOffsetPagination' then the value of 'limit' = p.Size +// and the value of 'offset' = p.Offset. +// In case when pagination type is 'PageNumberPagination' the 'limit' = p.Size and the +// offset is a result of multiplication of (pageNumber - 1) * pageSize = (p.Offset - 1) * p.Size. +// If the p.Offset is zero value then the (pageNumber - 1) value would be set previously to '0'. +func (p *Pagination) GetLimitOffset() (limit, offset int) { + switch p.Type { + case LimitOffsetPagination: + limit = p.Size + offset = p.Offset + case PageNumberPagination: + limit = p.Size + // the p.Offset value is a page number that starts it's value from '1'. + // If it's value is zero - the offset = 0. + if p.Offset >= 1 { + offset = (p.Offset - 1) * p.Size + } + default: + log.Warningf("Unknown pagination type: '%v'", p.Type) + } + return limit, offset +} + +// GetNumberSize gets the 'page number' and 'page size' from the pagination. +// If the pagination type is 'PageNumberPagination' the results are just the values of pagination. +// In case the pagination type is of 'LimitOffsetPagination' then 'limit'(Size) would be the page size +// and the page number would be 'offset' / limit + 1. PageNumberPagination starts it's page number counting from 1. +// If the offset % size != 0 - the offset is not dividable by the size without the rest - then the division +// rounds down it's value. +func (p *Pagination) GetNumberSize() (pageNumber, pageSize int) { + switch p.Type { + case LimitOffsetPagination: + pageSize = p.Size + // the default pageNumber value is '1'. + pageNumber = 1 + // if the 'limit' and 'offset' values are greater than zero - compute the value of pageNumber. + if p.Size > 0 && p.Offset > 0 { + // page numbering starts from '1' thus the result would be + pageNumber = p.Offset/p.Size + 1 + } + case PageNumberPagination: + pageSize = p.Size + pageNumber = p.Offset + default: + log.Warningf("Unknown pagination type: '%v'", p.Type) + } + return pageNumber, pageSize +} + // IsZero checks if the pagination is already set. func (p *Pagination) IsZero() bool { return p.Size == 0 && p.Offset == 0 @@ -94,7 +148,7 @@ func (p *Pagination) FormatQuery(q ...url.Values) url.Values { } case PageNumberPagination: number, size := p.Offset, p.Size - if number != 0 { + if number >= 1 { k = ParamPageNumber v = strconv.Itoa(number) query.Set(k, v) @@ -156,9 +210,9 @@ func (p *Pagination) checkPageBasedValues() error { return err } - if p.Offset < 0 { + if p.Offset <= 0 { err := errors.NewDet(class.QueryPaginationValue, "invalid pagination value") - err.SetDetails("Pagination page-number lower than 0") + err.SetDetails("Pagination page-number lower equalt to 0") return err } return nil diff --git a/query/pagination_test.go b/query/pagination_test.go index dff4320..cee5e3e 100644 --- a/query/pagination_test.go +++ b/query/pagination_test.go @@ -12,21 +12,21 @@ import ( func TestPaginationFormatQuery(t *testing.T) { t.Run("Paged", func(t *testing.T) { p := &Pagination{Size: 10, Offset: 2, Type: PageNumberPagination} - require.NoError(t, p.Check()) + require.NoError(t, p.IsValid()) q := url.Values{} p.FormatQuery(q) require.Len(t, q, 2) - assert.Equal(t, "10", q.Get(ParamPageSize), "%+v", p) + assert.Equal(t, "10", q.Get(ParamPageSize)) assert.Equal(t, "2", q.Get(ParamPageNumber)) }) t.Run("Limited", func(t *testing.T) { p := &Pagination{Size: 10} - require.NoError(t, p.Check()) + require.NoError(t, p.IsValid()) q := p.FormatQuery() require.Len(t, q, 1) @@ -37,7 +37,7 @@ func TestPaginationFormatQuery(t *testing.T) { t.Run("Offseted", func(t *testing.T) { p := &Pagination{Offset: 10} - require.NoError(t, p.Check()) + require.NoError(t, p.IsValid()) q := p.FormatQuery() require.Len(t, q, 1) @@ -48,7 +48,7 @@ func TestPaginationFormatQuery(t *testing.T) { t.Run("LimitOffset", func(t *testing.T) { p := &Pagination{10, 140, LimitOffsetPagination} - require.NoError(t, p.Check()) + require.NoError(t, p.IsValid()) q := p.FormatQuery() require.Len(t, q, 2) @@ -57,3 +57,53 @@ func TestPaginationFormatQuery(t *testing.T) { assert.Equal(t, "140", q.Get(ParamPageOffset)) }) } + +// TestGetLimitOffset tests the pagination GetLimitOffset method. +func TestGetLimitOffset(t *testing.T) { + // At first check the default 'LimitOffsetPagination'. + p := &Pagination{Size: 1, Offset: 10} + limit, offset := p.GetLimitOffset() + assert.Equal(t, 1, limit) + assert.Equal(t, 10, offset) + + // Check the pagination with PageNumberPagination type. + t.Run("PageNumber", func(t *testing.T) { + p = &Pagination{Type: PageNumberPagination, Size: 3, Offset: 9} + limit, offset = p.GetLimitOffset() + + // the offset would be (pageNumber - 1) * pageSize = 8 * 3 = 24 + assert.Equal(t, 24, offset) + assert.Equal(t, 3, limit) + + // Check the pagination without page number defined. + p = &Pagination{Type: PageNumberPagination, Size: 10} + limit, offset = p.GetLimitOffset() + + assert.Equal(t, 0, offset) + assert.Equal(t, 10, limit) + }) +} + +// TestGetNumberSize tests the pagination GetNumberSize method. +func TestGetNumberSize(t *testing.T) { + // At first check the 'PageNumberPagination'. + p := &Pagination{Type: PageNumberPagination, Size: 3, Offset: 9} + number, size := p.GetNumberSize() + + assert.Equal(t, 9, number) + assert.Equal(t, 3, size) + + t.Run("LimitOffsetTyped", func(t *testing.T) { + // Check the 'LimitOffsetPagination' with defined fields. + p = &Pagination{Size: 3, Offset: 10} + number, size = p.GetNumberSize() + assert.Equal(t, 3, size) + assert.Equal(t, 4, number) + + // Check the values with zero values. + p = &Pagination{Size: 10} + number, size = p.GetNumberSize() + assert.Equal(t, 10, size) + assert.Equal(t, 1, number) + }) +} diff --git a/query/process-lists.go b/query/process-lists.go index ec4bcf5..0e427e7 100644 --- a/query/process-lists.go +++ b/query/process-lists.go @@ -80,3 +80,16 @@ func beforeListFunc(ctx context.Context, s *Scope) error { return nil } + +// checkPaginationFunc is the process func that checks the validity of the pagination +func checkPaginationFunc(ctx context.Context, s *Scope) error { + if _, ok := s.StoreGet(processErrorKey); ok { + return nil + } + + if s.Pagination == nil { + return nil + } + + return s.Pagination.IsValid() +} diff --git a/query/processor.go b/query/processor.go index 565a069..04d7ac3 100644 --- a/query/processor.go +++ b/query/processor.go @@ -47,6 +47,7 @@ func init() { RegisterProcessFunc(ProcessHookAfterGet, afterGetFunc) // List + RegisterProcessFunc(ProcessCheckPagination, checkPaginationFunc) RegisterProcessFunc(ProcessHookBeforeList, beforeListFunc) RegisterProcessFunc(ProcessList, listFunc) RegisterProcessFunc(ProcessHookAfterList, afterListFunc) diff --git a/query/scope-pagination.go b/query/scope-pagination.go index 843173b..934cce4 100644 --- a/query/scope-pagination.go +++ b/query/scope-pagination.go @@ -15,7 +15,7 @@ func (s *Scope) Limit(limit, offset int) error { } s.Pagination = &Pagination{Size: limit, Offset: offset} - if err := s.Pagination.Check(); err != nil { + if err := s.Pagination.IsValid(); err != nil { return err } return nil @@ -27,8 +27,8 @@ func (s *Scope) Page(number, size int) error { return errors.NewDet(class.QueryPaginationAlreadySet, "pagination already set") } - s.Pagination = &Pagination{Size: size, Offset: number} - if err := s.Pagination.Check(); err != nil { + s.Pagination = &Pagination{Size: size, Offset: number, Type: PageNumberPagination} + if err := s.Pagination.IsValid(); err != nil { return err } return nil