From c83fdd482503d316d3fc97e354070a326826c01e Mon Sep 17 00:00:00 2001 From: Alexander Mazuruk Date: Tue, 11 Sep 2018 16:22:44 +0200 Subject: [PATCH] Set defaults on JobSorter Additionally, this commit unifies how server treats query params (pagination). They are always ignored if pagination is turned off (either by user or admin) Test have been hotfixed to pass and will be fixed in future commits on this branch. Change-Id: I12eb875972ac818b0630f10ae95289ff7aef7f88 Signed-off-by: Alexander Mazuruk --- server/job_lister_handler.go | 48 +++++++++++++++++++++++++-------------- server/job_lister_handler_test.go | 35 ++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/server/job_lister_handler.go b/server/job_lister_handler.go index 12aa285..0a2a662 100644 --- a/server/job_lister_handler.go +++ b/server/job_lister_handler.go @@ -23,27 +23,21 @@ import ( // JobLister is a handler which passess requests for listing jobs to jobmanager. func (a *APIDefaults) JobLister(params jobs.JobListerParams) middleware.Responder { - if (params.After != nil) && (params.Before != nil) { - return jobs.NewJobListerBadRequest().WithPayload( - &weles.ErrResponse{Message: weles.ErrBeforeAfterNotAllowed.Error()}) - } - - var jobInfoReceived []weles.JobInfo - var listInfo weles.ListInfo - var err error paginator := weles.JobPagination{} - if a.PageLimit != 0 { + if (params.After != nil) && (params.Before != nil) { + return jobs.NewJobListerBadRequest().WithPayload( + &weles.ErrResponse{Message: weles.ErrBeforeAfterNotAllowed.Error()}) + } paginator = setJobPaginator(params, a.PageLimit) } - - if params.JobFilterAndSort.Filter != nil || params.JobFilterAndSort.Sorter != nil { - jobInfoReceived, listInfo, err = a.Managers.JM.ListJobs( - *params.JobFilterAndSort.Filter, *params.JobFilterAndSort.Sorter, paginator) - } else { - jobInfoReceived, listInfo, err = a.Managers.JM.ListJobs( - weles.JobFilter{}, weles.JobSorter{}, paginator) + filter := weles.JobFilter{} + if params.JobFilterAndSort.Filter != nil { + filter = *params.JobFilterAndSort.Filter } + sorter := setJobSorter(params.JobFilterAndSort.Sorter) + + jobInfoReceived, listInfo, err := a.Managers.JM.ListJobs(filter, sorter, paginator) if err != nil { // due to weles.ErrInvalidArgument implementing error interface rather than being error // (which is intentional as we want to pass underlying error) switch err.(type) checks only @@ -68,7 +62,6 @@ func (a *APIDefaults) JobLister(params jobs.JobListerParams) middleware.Responde return responder200(listInfo, paginator, jobInfoReturned, a.PageLimit) } return responder206(listInfo, paginator, jobInfoReturned, a.PageLimit) - } func responder206(listInfo weles.ListInfo, paginator weles.JobPagination, @@ -152,6 +145,27 @@ func setJobPaginator(params jobs.JobListerParams, defaultPageLimit int32, return } +// setJobSorter sets default sorter values. +func setJobSorter(si *weles.JobSorter) (so weles.JobSorter) { + if si == nil { + return weles.JobSorter{ + SortOrder: weles.SortOrderAscending, + SortBy: weles.JobSortByID, + } + } + if si.SortOrder == "" { + so.SortOrder = weles.SortOrderAscending + } else { + so.SortOrder = si.SortOrder + } + if si.SortBy == "" { + so.SortBy = weles.JobSortByID + } else { + so.SortBy = si.SortBy + } + return +} + //jobInfoReceivedToReturned is a function which changes the slice of values to slice of pointers. //It is required due to fact that swagger generates responses as slices of pointers rather than //slices of values that the interface provides. diff --git a/server/job_lister_handler_test.go b/server/job_lister_handler_test.go index 4127f4a..11847cd 100644 --- a/server/job_lister_handler_test.go +++ b/server/job_lister_handler_test.go @@ -133,7 +133,7 @@ var _ = Describe("JobListerHandler", func() { emptyFilter := weles.JobFilter{} filledSorter1 := weles.JobSorter{ - SortBy: weles.JobSortByCreatedDate, + SortBy: weles.JobSortByID, SortOrder: weles.SortOrderAscending} filledSorter2 := weles.JobSorter{ @@ -209,7 +209,12 @@ var _ = Describe("JobListerHandler", func() { listInfo := weles.ListInfo{ TotalRecords: uint64(len(jobInfoSlice420)), RemainingRecords: 0} - + if sorter.SortOrder == "" { + sorter.SortOrder = weles.SortOrderAscending + } + if sorter.SortBy == "" { + sorter.SortBy = weles.JobSortByID + } mockJobManager.EXPECT().ListJobs( filter, sorter, emptyPaginator).Return( jobInfoSlice420, listInfo, nil) @@ -276,6 +281,12 @@ var _ = Describe("JobListerHandler", func() { TotalRecords: uint64(len(jobInfo)), RemainingRecords: 0} + if sorter.SortOrder == "" { + sorter.SortOrder = weles.SortOrderAscending + } + if sorter.SortBy == "" { + sorter.SortBy = weles.JobSortByID + } mockJobManager.EXPECT().ListJobs( filter, sorter, paginator).Return( jobInfo, listInfo, nil) @@ -349,6 +360,12 @@ var _ = Describe("JobListerHandler", func() { jobInfoStartingPage)), } + if sorter.SortOrder == "" { + sorter.SortOrder = weles.SortOrderAscending + } + if sorter.SortBy == "" { + sorter.SortBy = weles.JobSortByID + } first := mockJobManager.EXPECT().ListJobs(filter, sorter, paginator).Return( jobInfoStartingPage, listInfo, nil) @@ -502,6 +519,12 @@ var _ = Describe("JobListerHandler", func() { } + if sorter.SortOrder == "" { + sorter.SortOrder = weles.SortOrderAscending + } + if sorter.SortBy == "" { + sorter.SortBy = weles.JobSortByID + } first := mockJobManager.EXPECT().ListJobs( filter, sorter, paginator).Return(jobInfoStartingPage, listInfo, nil) @@ -643,6 +666,12 @@ var _ = Describe("JobListerHandler", func() { } listInfo := weles.ListInfo{TotalRecords: uint64(aviJobs), RemainingRecords: 0} + if sorter.SortOrder == "" { + sorter.SortOrder = weles.SortOrderAscending + } + if sorter.SortBy == "" { + sorter.SortBy = weles.JobSortByID + } mockJobManager.EXPECT().ListJobs(filter, sorter, paginator).Return( jobInfo, listInfo, jmerr) reqBody := filterSorterReqBody(filter, sorter, JSON) @@ -712,8 +741,6 @@ var _ = Describe("JobListerHandler", func() { Expect(resp.Header.Get("RemainingRecords")).To(Equal("")) }, - Entry("json, pagination off", - int32(0), "?before=10&after=20", JSON, JSON, emptyFilter, emptySorter), Entry("json, pagination on", int32(100), "?before=10&after=20", JSON, JSON, emptyFilter, emptySorter), ) -- 2.7.4