From 9972c24924a8e29a6a319fc1207aab70f3f92dd5 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Thu, 25 Apr 2024 09:37:42 +0200 Subject: [PATCH] Add filter options to GetPipelines API (#3645) Separate this change from https://github.com/woodpecker-ci/woodpecker/pull/3506 I would like to get at least this change into v2.5.0 if possible. --------- Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com> --- cmd/server/docs/docs.go | 12 ++++ server/api/badge.go | 2 +- server/api/pipeline.go | 26 ++++++- server/api/pipeline_test.go | 81 ++++++++++++++++++++++ server/model/pipeline.go | 5 ++ server/services/mocks/manager.go | 2 +- server/store/datastore/pipeline.go | 17 ++++- server/store/datastore/pipeline_test.go | 25 ++++++- server/store/mocks/store.go | 90 ++++++++----------------- server/store/store.go | 2 +- 10 files changed, 194 insertions(+), 68 deletions(-) create mode 100644 server/api/pipeline_test.go diff --git a/cmd/server/docs/docs.go b/cmd/server/docs/docs.go index 1195f9829..dbe9d53c6 100644 --- a/cmd/server/docs/docs.go +++ b/cmd/server/docs/docs.go @@ -2174,6 +2174,18 @@ const docTemplate = `{ "description": "for response pagination, max items per page", "name": "perPage", "in": "query" + }, + { + "type": "string", + "description": "only return pipelines before this RFC3339 date", + "name": "before", + "in": "query" + }, + { + "type": "string", + "description": "only return pipelines after this RFC3339 date", + "name": "after", + "in": "query" } ], "responses": { diff --git a/server/api/badge.go b/server/api/badge.go index 6f4b491d9..898dd2126 100644 --- a/server/api/badge.go +++ b/server/api/badge.go @@ -120,7 +120,7 @@ func GetCC(c *gin.Context) { return } - pipelines, err := _store.GetPipelineList(repo, &model.ListOptions{Page: 1, PerPage: 1}) + pipelines, err := _store.GetPipelineList(repo, &model.ListOptions{Page: 1, PerPage: 1}, nil) if err != nil && !errors.Is(err, types.RecordNotExist) { log.Warn().Err(err).Msg("could not get pipeline list") c.AbortWithStatus(http.StatusInternalServerError) diff --git a/server/api/pipeline.go b/server/api/pipeline.go index 8803ab95f..1bb8725d7 100644 --- a/server/api/pipeline.go +++ b/server/api/pipeline.go @@ -109,10 +109,34 @@ func createTmpPipeline(event model.WebhookEvent, commit *model.Commit, user *mod // @Param repo_id path int true "the repository id" // @Param page query int false "for response pagination, page offset number" default(1) // @Param perPage query int false "for response pagination, max items per page" default(50) +// @Param before query string false "only return pipelines before this RFC3339 date" +// @Param after query string false "only return pipelines after this RFC3339 date" func GetPipelines(c *gin.Context) { repo := session.Repo(c) + before := c.Query("before") + after := c.Query("after") - pipelines, err := store.FromContext(c).GetPipelineList(repo, session.Pagination(c)) + filter := new(model.PipelineFilter) + + if before != "" { + beforeDt, err := time.Parse(time.RFC3339, before) + if err != nil { + _ = c.AbortWithError(http.StatusBadRequest, err) + return + } + filter.Before = beforeDt.Unix() + } + + if after != "" { + afterDt, err := time.Parse(time.RFC3339, after) + if err != nil { + _ = c.AbortWithError(http.StatusBadRequest, err) + return + } + filter.After = afterDt.Unix() + } + + pipelines, err := store.FromContext(c).GetPipelineList(repo, session.Pagination(c), filter) if err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return diff --git a/server/api/pipeline_test.go b/server/api/pipeline_test.go new file mode 100644 index 000000000..9f0608dc9 --- /dev/null +++ b/server/api/pipeline_test.go @@ -0,0 +1,81 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/franela/goblin" + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/store/mocks" +) + +var fakePipeline = &model.Pipeline{ + Status: model.StatusSuccess, +} + +func TestGetPipelines(t *testing.T) { + gin.SetMode(gin.TestMode) + + g := goblin.Goblin(t) + g.Describe("Pipeline", func() { + g.It("should get pipelines", func() { + pipelines := []*model.Pipeline{fakePipeline} + + mockStore := mocks.NewStore(t) + mockStore.On("GetPipelineList", mock.Anything, mock.Anything, mock.Anything).Return(pipelines, nil) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("store", mockStore) + + GetPipelines(c) + + mockStore.AssertCalled(t, "GetPipelineList", mock.Anything, mock.Anything, mock.Anything) + assert.Equal(t, http.StatusOK, c.Writer.Status()) + }) + + g.It("should not parse pipeline filter", func() { + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest("DELETE", "/?before=2023-01-16&after=2023-01-15", nil) + + GetPipelines(c) + + assert.Equal(t, http.StatusBadRequest, c.Writer.Status()) + }) + + g.It("should parse pipeline filter", func() { + pipelines := []*model.Pipeline{fakePipeline} + + mockStore := mocks.NewStore(t) + mockStore.On("GetPipelineList", mock.Anything, mock.Anything, mock.Anything).Return(pipelines, nil) + + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Set("store", mockStore) + c.Request, _ = http.NewRequest("DELETE", "/?2023-01-16T15:00:00Z&after=2023-01-15T15:00:00Z", nil) + + GetPipelines(c) + + assert.Equal(t, http.StatusOK, c.Writer.Status()) + }) + + g.It("should parse pipeline filter with tz offset", func() { + pipelines := []*model.Pipeline{fakePipeline} + + mockStore := mocks.NewStore(t) + mockStore.On("GetPipelineList", mock.Anything, mock.Anything, mock.Anything).Return(pipelines, nil) + + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Set("store", mockStore) + c.Request, _ = http.NewRequest("DELETE", "/?before=2023-01-16T15:00:00%2B01:00&after=2023-01-15T15:00:00%2B01:00", nil) + + GetPipelines(c) + + assert.Equal(t, http.StatusOK, c.Writer.Status()) + }) + }) +} diff --git a/server/model/pipeline.go b/server/model/pipeline.go index 72fea8192..9454b40e5 100644 --- a/server/model/pipeline.go +++ b/server/model/pipeline.go @@ -53,6 +53,11 @@ type Pipeline struct { IsPrerelease bool `json:"is_prerelease,omitempty" xorm:"is_prerelease"` } // @name Pipeline +type PipelineFilter struct { + Before int64 + After int64 +} + // TableName return database table name for xorm func (Pipeline) TableName() string { return "pipelines" diff --git a/server/services/mocks/manager.go b/server/services/mocks/manager.go index 3c67c2c44..1d64c2af3 100644 --- a/server/services/mocks/manager.go +++ b/server/services/mocks/manager.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.42.1. DO NOT EDIT. +// Code generated by mockery. DO NOT EDIT. package mocks diff --git a/server/store/datastore/pipeline.go b/server/store/datastore/pipeline.go index f72353db7..4ed6e9022 100644 --- a/server/store/datastore/pipeline.go +++ b/server/store/datastore/pipeline.go @@ -52,9 +52,22 @@ func (s storage) GetPipelineLastBefore(repo *model.Repo, branch string, num int6 Get(pipeline)) } -func (s storage) GetPipelineList(repo *model.Repo, p *model.ListOptions) ([]*model.Pipeline, error) { +func (s storage) GetPipelineList(repo *model.Repo, p *model.ListOptions, f *model.PipelineFilter) ([]*model.Pipeline, error) { pipelines := make([]*model.Pipeline, 0, 16) - return pipelines, s.paginate(p).Where("pipeline_repo_id = ?", repo.ID). + + cond := builder.NewCond().And(builder.Eq{"pipeline_repo_id": repo.ID}) + + if f != nil { + if f.After != 0 { + cond = cond.And(builder.Gt{"pipeline_started": f.After}) + } + + if f.Before != 0 { + cond = cond.And(builder.Lt{"pipeline_started": f.Before}) + } + } + + return pipelines, s.paginate(p).Where(cond). Desc("pipeline_number"). Find(&pipelines) } diff --git a/server/store/datastore/pipeline_test.go b/server/store/datastore/pipeline_test.go index 7ba30fd19..df1ead63f 100644 --- a/server/store/datastore/pipeline_test.go +++ b/server/store/datastore/pipeline_test.go @@ -18,6 +18,7 @@ package datastore import ( "fmt" "testing" + "time" "github.com/franela/goblin" "github.com/stretchr/testify/assert" @@ -221,13 +222,35 @@ func TestPipelines(t *testing.T) { g.Assert(err1).IsNil() err2 := store.CreatePipeline(pipeline2, []*model.Step{}...) g.Assert(err2).IsNil() - pipelines, err3 := store.GetPipelineList(&model.Repo{ID: 1}, &model.ListOptions{Page: 1, PerPage: 50}) + pipelines, err3 := store.GetPipelineList(&model.Repo{ID: 1}, &model.ListOptions{Page: 1, PerPage: 50}, nil) g.Assert(err3).IsNil() g.Assert(len(pipelines)).Equal(2) g.Assert(pipelines[0].ID).Equal(pipeline2.ID) g.Assert(pipelines[0].RepoID).Equal(pipeline2.RepoID) g.Assert(pipelines[0].Status).Equal(pipeline2.Status) }) + + g.It("Should get filtered pipelines", func() { + dt1, _ := time.Parse(time.RFC3339, "2023-01-15T15:00:00Z") + pipeline1 := &model.Pipeline{ + RepoID: repo.ID, + Started: dt1.Unix(), + } + dt2, _ := time.Parse(time.RFC3339, "2023-01-15T16:30:00Z") + pipeline2 := &model.Pipeline{ + RepoID: repo.ID, + Started: dt2.Unix(), + } + err1 := store.CreatePipeline(pipeline1, []*model.Step{}...) + g.Assert(err1).IsNil() + err2 := store.CreatePipeline(pipeline2, []*model.Step{}...) + g.Assert(err2).IsNil() + pipelines, err3 := store.GetPipelineList(&model.Repo{ID: 1}, &model.ListOptions{Page: 1, PerPage: 50}, &model.PipelineFilter{Before: dt2.Unix()}) + g.Assert(err3).IsNil() + g.Assert(len(pipelines)).Equal(1) + g.Assert(pipelines[0].ID).Equal(pipeline1.ID) + g.Assert(pipelines[0].RepoID).Equal(pipeline1.RepoID) + }) }) } diff --git a/server/store/mocks/store.go b/server/store/mocks/store.go index 3007b3ef4..ee1a825d0 100644 --- a/server/store/mocks/store.go +++ b/server/store/mocks/store.go @@ -543,6 +543,10 @@ func (_m *Store) DeleteUser(_a0 *model.User) error { func (_m *Store) ForgeCreate(_a0 *model.Forge) error { ret := _m.Called(_a0) + if len(ret) == 0 { + panic("no return value specified for ForgeCreate") + } + var r0 error if rf, ok := ret.Get(0).(func(*model.Forge) error); ok { r0 = rf(_a0) @@ -557,6 +561,10 @@ func (_m *Store) ForgeCreate(_a0 *model.Forge) error { func (_m *Store) ForgeDelete(_a0 *model.Forge) error { ret := _m.Called(_a0) + if len(ret) == 0 { + panic("no return value specified for ForgeDelete") + } + var r0 error if rf, ok := ret.Get(0).(func(*model.Forge) error); ok { r0 = rf(_a0) @@ -567,62 +575,14 @@ func (_m *Store) ForgeDelete(_a0 *model.Forge) error { return r0 } -// ForgeFindByRepo provides a mock function with given fields: _a0 -func (_m *Store) ForgeFindByRepo(_a0 *model.Repo) (*model.Forge, error) { - ret := _m.Called(_a0) - - var r0 *model.Forge - var r1 error - if rf, ok := ret.Get(0).(func(*model.Repo) (*model.Forge, error)); ok { - return rf(_a0) - } - if rf, ok := ret.Get(0).(func(*model.Repo) *model.Forge); ok { - r0 = rf(_a0) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Forge) - } - } - - if rf, ok := ret.Get(1).(func(*model.Repo) error); ok { - r1 = rf(_a0) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// ForgeFindByUser provides a mock function with given fields: _a0 -func (_m *Store) ForgeFindByUser(_a0 *model.User) (*model.Forge, error) { - ret := _m.Called(_a0) - - var r0 *model.Forge - var r1 error - if rf, ok := ret.Get(0).(func(*model.User) (*model.Forge, error)); ok { - return rf(_a0) - } - if rf, ok := ret.Get(0).(func(*model.User) *model.Forge); ok { - r0 = rf(_a0) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Forge) - } - } - - if rf, ok := ret.Get(1).(func(*model.User) error); ok { - r1 = rf(_a0) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // ForgeGet provides a mock function with given fields: _a0 func (_m *Store) ForgeGet(_a0 int64) (*model.Forge, error) { ret := _m.Called(_a0) + if len(ret) == 0 { + panic("no return value specified for ForgeGet") + } + var r0 *model.Forge var r1 error if rf, ok := ret.Get(0).(func(int64) (*model.Forge, error)); ok { @@ -649,6 +609,10 @@ func (_m *Store) ForgeGet(_a0 int64) (*model.Forge, error) { func (_m *Store) ForgeList(p *model.ListOptions) ([]*model.Forge, error) { ret := _m.Called(p) + if len(ret) == 0 { + panic("no return value specified for ForgeList") + } + var r0 []*model.Forge var r1 error if rf, ok := ret.Get(0).(func(*model.ListOptions) ([]*model.Forge, error)); ok { @@ -675,6 +639,10 @@ func (_m *Store) ForgeList(p *model.ListOptions) ([]*model.Forge, error) { func (_m *Store) ForgeUpdate(_a0 *model.Forge) error { ret := _m.Called(_a0) + if len(ret) == 0 { + panic("no return value specified for ForgeUpdate") + } + var r0 error if rf, ok := ret.Get(0).(func(*model.Forge) error); ok { r0 = rf(_a0) @@ -833,9 +801,9 @@ func (_m *Store) GetPipelineLastBefore(_a0 *model.Repo, _a1 string, _a2 int64) ( return r0, r1 } -// GetPipelineList provides a mock function with given fields: _a0, _a1 -func (_m *Store) GetPipelineList(_a0 *model.Repo, _a1 *model.ListOptions) ([]*model.Pipeline, error) { - ret := _m.Called(_a0, _a1) +// GetPipelineList provides a mock function with given fields: _a0, _a1, _a2 +func (_m *Store) GetPipelineList(_a0 *model.Repo, _a1 *model.ListOptions, _a2 *model.PipelineFilter) ([]*model.Pipeline, error) { + ret := _m.Called(_a0, _a1, _a2) if len(ret) == 0 { panic("no return value specified for GetPipelineList") @@ -843,19 +811,19 @@ func (_m *Store) GetPipelineList(_a0 *model.Repo, _a1 *model.ListOptions) ([]*mo var r0 []*model.Pipeline var r1 error - if rf, ok := ret.Get(0).(func(*model.Repo, *model.ListOptions) ([]*model.Pipeline, error)); ok { - return rf(_a0, _a1) + if rf, ok := ret.Get(0).(func(*model.Repo, *model.ListOptions, *model.PipelineFilter) ([]*model.Pipeline, error)); ok { + return rf(_a0, _a1, _a2) } - if rf, ok := ret.Get(0).(func(*model.Repo, *model.ListOptions) []*model.Pipeline); ok { - r0 = rf(_a0, _a1) + if rf, ok := ret.Get(0).(func(*model.Repo, *model.ListOptions, *model.PipelineFilter) []*model.Pipeline); ok { + r0 = rf(_a0, _a1, _a2) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.Pipeline) } } - if rf, ok := ret.Get(1).(func(*model.Repo, *model.ListOptions) error); ok { - r1 = rf(_a0, _a1) + if rf, ok := ret.Get(1).(func(*model.Repo, *model.ListOptions, *model.PipelineFilter) error); ok { + r1 = rf(_a0, _a1, _a2) } else { r1 = ret.Error(1) } diff --git a/server/store/store.go b/server/store/store.go index 6ba76573c..2a007e487 100644 --- a/server/store/store.go +++ b/server/store/store.go @@ -75,7 +75,7 @@ type Store interface { // GetPipelineLastBefore gets the last pipeline before pipeline number N. GetPipelineLastBefore(*model.Repo, string, int64) (*model.Pipeline, error) // GetPipelineList gets a list of pipelines for the repository - GetPipelineList(*model.Repo, *model.ListOptions) ([]*model.Pipeline, error) + GetPipelineList(*model.Repo, *model.ListOptions, *model.PipelineFilter) ([]*model.Pipeline, error) // GetActivePipelineList gets a list of the active pipelines for the repository GetActivePipelineList(repo *model.Repo) ([]*model.Pipeline, error) // GetPipelineQueue gets a list of pipelines in queue.