From cb3efd2cd94cb18603a74fbd579146d4ba8ae462 Mon Sep 17 00:00:00 2001 From: qwerty287 <80460567+qwerty287@users.noreply.github.com> Date: Sun, 25 Feb 2024 10:37:10 +0100 Subject: [PATCH] Simplify store interfaces (#3437) Use `store.Store` interface if possible. --- server/model/environ.go | 5 - server/model/perm.go | 8 -- server/model/pipeline.go | 4 - server/model/registry.go | 9 -- server/model/server_config.go | 6 - server/model/step.go | 14 --- server/model/task.go | 7 -- server/model/workflow.go | 4 - .../{pipelineStatus.go => pipeline_status.go} | 13 +- ...Status_test.go => pipeline_status_test.go} | 23 ++-- server/pipeline/step_status.go | 11 +- server/pipeline/step_status_test.go | 39 +++--- server/pipeline/workflowStatus.go | 7 +- server/queue/persistent.go | 5 +- server/services/registry/db.go | 5 +- server/services/secret/db.go | 5 +- server/store/datastore/config.go | 12 -- server/store/datastore/config_test.go | 116 +----------------- server/store/datastore/log.go | 21 ---- server/store/datastore/log_test.go | 9 +- server/store/datastore/permission.go | 11 -- server/store/datastore/permission_test.go | 34 ----- server/store/datastore/pipeline.go | 14 --- server/store/datastore/pipeline_test.go | 72 ----------- server/store/datastore/redirection.go | 6 - server/store/datastore/redirection_test.go | 18 --- server/store/store.go | 11 -- 27 files changed, 69 insertions(+), 420 deletions(-) rename server/pipeline/{pipelineStatus.go => pipeline_status.go} (68%) rename server/pipeline/{pipelineStatus_test.go => pipeline_status_test.go} (72%) diff --git a/server/model/environ.go b/server/model/environ.go index 87581ecb4..577fb24ba 100644 --- a/server/model/environ.go +++ b/server/model/environ.go @@ -24,11 +24,6 @@ var ( errEnvironValueInvalid = errors.New("invalid Environment Variable Value") ) -// EnvironStore persists environment information to storage. -type EnvironStore interface { - EnvironList(*Repo) ([]*Environ, error) -} - // Environ represents an environment variable. type Environ struct { Name string `json:"name"` diff --git a/server/model/perm.go b/server/model/perm.go index d38bb1bb7..77fbcc7f5 100644 --- a/server/model/perm.go +++ b/server/model/perm.go @@ -15,14 +15,6 @@ package model -// PermStore persists repository permissions information to storage. -type PermStore interface { - PermFind(user *User, repo *Repo) (*Perm, error) - PermUpsert(perm *Perm) error - PermDelete(perm *Perm) error - PermFlush(user *User, before int64) error -} - // Perm defines a repository permission for an individual user. type Perm struct { UserID int64 `json:"-" xorm:"UNIQUE(s) INDEX NOT NULL 'perm_user_id'"` diff --git a/server/model/pipeline.go b/server/model/pipeline.go index 410ba3f11..1a0a7226d 100644 --- a/server/model/pipeline.go +++ b/server/model/pipeline.go @@ -63,10 +63,6 @@ func (p Pipeline) IsMultiPipeline() bool { return len(p.Workflows) > 1 } -type UpdatePipelineStore interface { - UpdatePipeline(*Pipeline) error -} - type PipelineOptions struct { Branch string `json:"branch"` Variables map[string]string `json:"variables"` diff --git a/server/model/registry.go b/server/model/registry.go index e2bfe9ee9..e5772b824 100644 --- a/server/model/registry.go +++ b/server/model/registry.go @@ -26,15 +26,6 @@ var ( errRegistryPasswordInvalid = errors.New("invalid registry password") ) -// RegistryStore persists registry information to storage. -type RegistryStore interface { - RegistryFind(*Repo, string) (*Registry, error) - RegistryList(*Repo, *ListOptions) ([]*Registry, error) - RegistryCreate(*Registry) error - RegistryUpdate(*Registry) error - RegistryDelete(repo *Repo, addr string) error -} - // Registry represents a docker registry with credentials. type Registry struct { ID int64 `json:"id" xorm:"pk autoincr 'registry_id'"` diff --git a/server/model/server_config.go b/server/model/server_config.go index 145c449eb..2c18bbdfd 100644 --- a/server/model/server_config.go +++ b/server/model/server_config.go @@ -14,12 +14,6 @@ package model -// ServerConfigStore persists key-value pairs for storing server configurations. -type ServerConfigStore interface { - ServerConfigGet(key string) (string, error) - ServerConfigSet(key int64, value string) error -} - // ServerConfig represents a key-value pair for storing server configurations. type ServerConfig struct { Key string `json:"key" xorm:"pk"` diff --git a/server/model/step.go b/server/model/step.go index 5341a7cd6..0b93159d6 100644 --- a/server/model/step.go +++ b/server/model/step.go @@ -15,16 +15,6 @@ package model -// StepStore persists process information to storage. -type StepStore interface { - StepLoad(int64) (*Step, error) - StepFind(*Pipeline, int) (*Step, error) - StepChild(*Pipeline, int, string) (*Step, error) - StepList(*Pipeline) ([]*Step, error) - StepCreate([]*Step) error - StepUpdate(*Step) error -} - // Different ways to handle failure states const ( FailureIgnore = "ignore" @@ -49,10 +39,6 @@ type Step struct { Type StepType `json:"type,omitempty" xorm:"step_type"` } // @name Step -type UpdateStepStore interface { - StepUpdate(*Step) error -} - // TableName return database table name for xorm func (Step) TableName() string { return "steps" diff --git a/server/model/task.go b/server/model/task.go index b9af56069..b30f1d0f3 100644 --- a/server/model/task.go +++ b/server/model/task.go @@ -19,13 +19,6 @@ import ( "strings" ) -// TaskStore defines storage for scheduled Tasks. -type TaskStore interface { - TaskList() ([]*Task, error) - TaskInsert(*Task) error - TaskDelete(string) error -} - // Task defines scheduled pipeline Task. type Task struct { ID string `json:"id" xorm:"PK UNIQUE 'task_id'"` diff --git a/server/model/workflow.go b/server/model/workflow.go index 2cceeee15..b1d6ad54c 100644 --- a/server/model/workflow.go +++ b/server/model/workflow.go @@ -32,10 +32,6 @@ type Workflow struct { Children []*Step `json:"children,omitempty" xorm:"-"` } -type UpdateWorkflowStore interface { - WorkflowUpdate(*Workflow) error -} - // TableName return database table name for xorm func (Workflow) TableName() string { return "workflows" diff --git a/server/pipeline/pipelineStatus.go b/server/pipeline/pipeline_status.go similarity index 68% rename from server/pipeline/pipelineStatus.go rename to server/pipeline/pipeline_status.go index 1081b4363..95ed21817 100644 --- a/server/pipeline/pipelineStatus.go +++ b/server/pipeline/pipeline_status.go @@ -20,15 +20,16 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors" "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/store" ) -func UpdateToStatusRunning(store model.UpdatePipelineStore, pipeline model.Pipeline, started int64) (*model.Pipeline, error) { +func UpdateToStatusRunning(store store.Store, pipeline model.Pipeline, started int64) (*model.Pipeline, error) { pipeline.Status = model.StatusRunning pipeline.Started = started return &pipeline, store.UpdatePipeline(&pipeline) } -func UpdateToStatusPending(store model.UpdatePipelineStore, pipeline model.Pipeline, reviewer string) (*model.Pipeline, error) { +func UpdateToStatusPending(store store.Store, pipeline model.Pipeline, reviewer string) (*model.Pipeline, error) { if reviewer != "" { pipeline.Reviewer = reviewer pipeline.Reviewed = time.Now().Unix() @@ -37,20 +38,20 @@ func UpdateToStatusPending(store model.UpdatePipelineStore, pipeline model.Pipel return &pipeline, store.UpdatePipeline(&pipeline) } -func UpdateToStatusDeclined(store model.UpdatePipelineStore, pipeline model.Pipeline, reviewer string) (*model.Pipeline, error) { +func UpdateToStatusDeclined(store store.Store, pipeline model.Pipeline, reviewer string) (*model.Pipeline, error) { pipeline.Reviewer = reviewer pipeline.Status = model.StatusDeclined pipeline.Reviewed = time.Now().Unix() return &pipeline, store.UpdatePipeline(&pipeline) } -func UpdateStatusToDone(store model.UpdatePipelineStore, pipeline model.Pipeline, status model.StatusValue, stopped int64) (*model.Pipeline, error) { +func UpdateStatusToDone(store store.Store, pipeline model.Pipeline, status model.StatusValue, stopped int64) (*model.Pipeline, error) { pipeline.Status = status pipeline.Finished = stopped return &pipeline, store.UpdatePipeline(&pipeline) } -func UpdateToStatusError(store model.UpdatePipelineStore, pipeline model.Pipeline, err error) (*model.Pipeline, error) { +func UpdateToStatusError(store store.Store, pipeline model.Pipeline, err error) (*model.Pipeline, error) { pipeline.Errors = errors.GetPipelineErrors(err) pipeline.Status = model.StatusError pipeline.Started = time.Now().Unix() @@ -58,7 +59,7 @@ func UpdateToStatusError(store model.UpdatePipelineStore, pipeline model.Pipelin return &pipeline, store.UpdatePipeline(&pipeline) } -func UpdateToStatusKilled(store model.UpdatePipelineStore, pipeline model.Pipeline) (*model.Pipeline, error) { +func UpdateToStatusKilled(store store.Store, pipeline model.Pipeline) (*model.Pipeline, error) { pipeline.Status = model.StatusKilled pipeline.Finished = time.Now().Unix() return &pipeline, store.UpdatePipeline(&pipeline) diff --git a/server/pipeline/pipelineStatus_test.go b/server/pipeline/pipeline_status_test.go similarity index 72% rename from server/pipeline/pipelineStatus_test.go rename to server/pipeline/pipeline_status_test.go index 05d906fc3..ce4ba657b 100644 --- a/server/pipeline/pipelineStatus_test.go +++ b/server/pipeline/pipeline_status_test.go @@ -21,20 +21,23 @@ import ( "time" "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" + "go.woodpecker-ci.org/woodpecker/v2/server/store/mocks" ) -type mockUpdatePipelineStore struct{} - -func (m *mockUpdatePipelineStore) UpdatePipeline(_ *model.Pipeline) error { - return nil +func mockStorePipeline(t *testing.T) store.Store { + s := mocks.NewStore(t) + s.On("UpdatePipeline", mock.Anything).Return(nil) + return s } func TestUpdateToStatusRunning(t *testing.T) { t.Parallel() - pipeline, _ := UpdateToStatusRunning(&mockUpdatePipelineStore{}, model.Pipeline{}, int64(1)) + pipeline, _ := UpdateToStatusRunning(mockStorePipeline(t), model.Pipeline{}, int64(1)) assert.Equal(t, model.StatusRunning, pipeline.Status) assert.EqualValues(t, 1, pipeline.Started) } @@ -44,7 +47,7 @@ func TestUpdateToStatusPending(t *testing.T) { now := time.Now().Unix() - pipeline, _ := UpdateToStatusPending(&mockUpdatePipelineStore{}, model.Pipeline{}, "Reviewer") + pipeline, _ := UpdateToStatusPending(mockStorePipeline(t), model.Pipeline{}, "Reviewer") assert.Equal(t, model.StatusPending, pipeline.Status) assert.Equal(t, "Reviewer", pipeline.Reviewer) @@ -56,7 +59,7 @@ func TestUpdateToStatusDeclined(t *testing.T) { now := time.Now().Unix() - pipeline, _ := UpdateToStatusDeclined(&mockUpdatePipelineStore{}, model.Pipeline{}, "Reviewer") + pipeline, _ := UpdateToStatusDeclined(mockStorePipeline(t), model.Pipeline{}, "Reviewer") assert.Equal(t, model.StatusDeclined, pipeline.Status) assert.Equal(t, "Reviewer", pipeline.Reviewer) @@ -66,7 +69,7 @@ func TestUpdateToStatusDeclined(t *testing.T) { func TestUpdateToStatusToDone(t *testing.T) { t.Parallel() - pipeline, _ := UpdateStatusToDone(&mockUpdatePipelineStore{}, model.Pipeline{}, "status", int64(1)) + pipeline, _ := UpdateStatusToDone(mockStorePipeline(t), model.Pipeline{}, "status", int64(1)) assert.Equal(t, model.StatusValue("status"), pipeline.Status) assert.EqualValues(t, 1, pipeline.Finished) @@ -77,7 +80,7 @@ func TestUpdateToStatusError(t *testing.T) { now := time.Now().Unix() - pipeline, _ := UpdateToStatusError(&mockUpdatePipelineStore{}, model.Pipeline{}, errors.New("this is an error")) + pipeline, _ := UpdateToStatusError(mockStorePipeline(t), model.Pipeline{}, errors.New("this is an error")) assert.Len(t, pipeline.Errors, 1) assert.Equal(t, "[generic] this is an error", pipeline.Errors[0].Error()) @@ -92,7 +95,7 @@ func TestUpdateToStatusKilled(t *testing.T) { now := time.Now().Unix() - pipeline, _ := UpdateToStatusKilled(&mockUpdatePipelineStore{}, model.Pipeline{}) + pipeline, _ := UpdateToStatusKilled(mockStorePipeline(t), model.Pipeline{}) assert.Equal(t, model.StatusKilled, pipeline.Status) assert.LessOrEqual(t, now, pipeline.Finished) diff --git a/server/pipeline/step_status.go b/server/pipeline/step_status.go index 3d7bc3767..8d0c1d538 100644 --- a/server/pipeline/step_status.go +++ b/server/pipeline/step_status.go @@ -20,9 +20,10 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/store" ) -func UpdateStepStatus(store model.UpdateStepStore, step *model.Step, state rpc.State) error { +func UpdateStepStatus(store store.Store, step *model.Step, state rpc.State) error { if state.Exited { step.Stopped = state.Finished step.ExitCode = state.ExitCode @@ -41,13 +42,13 @@ func UpdateStepStatus(store model.UpdateStepStore, step *model.Step, state rpc.S return store.StepUpdate(step) } -func UpdateStepToStatusStarted(store model.UpdateStepStore, step model.Step, state rpc.State) (*model.Step, error) { +func UpdateStepToStatusStarted(store store.Store, step model.Step, state rpc.State) (*model.Step, error) { step.Started = state.Started step.State = model.StatusRunning return &step, store.StepUpdate(&step) } -func UpdateStepToStatusSkipped(store model.UpdateStepStore, step model.Step, stopped int64) (*model.Step, error) { +func UpdateStepToStatusSkipped(store store.Store, step model.Step, stopped int64) (*model.Step, error) { step.State = model.StatusSkipped if step.Started != 0 { step.State = model.StatusSuccess // for daemons that are killed @@ -56,7 +57,7 @@ func UpdateStepToStatusSkipped(store model.UpdateStepStore, step model.Step, sto return &step, store.StepUpdate(&step) } -func UpdateStepStatusToDone(store model.UpdateStepStore, step model.Step, state rpc.State) (*model.Step, error) { +func UpdateStepStatusToDone(store store.Store, step model.Step, state rpc.State) (*model.Step, error) { step.Stopped = state.Finished step.Error = state.Error step.ExitCode = state.ExitCode @@ -71,7 +72,7 @@ func UpdateStepStatusToDone(store model.UpdateStepStore, step model.Step, state return &step, store.StepUpdate(&step) } -func UpdateStepToStatusKilled(store model.UpdateStepStore, step model.Step) (*model.Step, error) { +func UpdateStepToStatusKilled(store store.Store, step model.Step) (*model.Step, error) { step.State = model.StatusKilled step.Stopped = time.Now().Unix() if step.Started == 0 { diff --git a/server/pipeline/step_status_test.go b/server/pipeline/step_status_test.go index f786ca5cc..b0ecd85f4 100644 --- a/server/pipeline/step_status_test.go +++ b/server/pipeline/step_status_test.go @@ -20,15 +20,18 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/store" + "go.woodpecker-ci.org/woodpecker/v2/server/store/mocks" ) -type mockUpdateStepStore struct{} - -func (m *mockUpdateStepStore) StepUpdate(_ *model.Step) error { - return nil +func mockStoreStep(t *testing.T) store.Store { + s := mocks.NewStore(t) + s.On("StepUpdate", mock.Anything).Return(nil) + return s } func TestUpdateStepStatusNotExited(t *testing.T) { @@ -46,7 +49,7 @@ func TestUpdateStepStatusNotExited(t *testing.T) { Error: "not an error", } - err := UpdateStepStatus(&mockUpdateStepStore{}, step, state) + err := UpdateStepStatus(mockStoreStep(t), step, state) assert.NoError(t, err) assert.EqualValues(t, model.StatusRunning, step.State) assert.EqualValues(t, 42, step.Started) @@ -70,7 +73,7 @@ func TestUpdateStepStatusNotExitedButStopped(t *testing.T) { Error: "not an error", } - err := UpdateStepStatus(&mockUpdateStepStore{}, step, state) + err := UpdateStepStatus(mockStoreStep(t), step, state) assert.NoError(t, err) assert.EqualValues(t, model.StatusKilled, step.State) assert.EqualValues(t, 42, step.Started) @@ -94,7 +97,7 @@ func TestUpdateStepStatusExited(t *testing.T) { Error: "an error", } - err := UpdateStepStatus(&mockUpdateStepStore{}, step, state) + err := UpdateStepStatus(mockStoreStep(t), step, state) assert.NoError(t, err) assert.EqualValues(t, model.StatusKilled, step.State) assert.EqualValues(t, 42, step.Started) @@ -117,7 +120,7 @@ func TestUpdateStepStatusExitedButNot137(t *testing.T) { Error: "an error", } - err := UpdateStepStatus(&mockUpdateStepStore{}, step, state) + err := UpdateStepStatus(mockStoreStep(t), step, state) assert.NoError(t, err) assert.EqualValues(t, model.StatusFailure, step.State) assert.EqualValues(t, 42, step.Started) @@ -138,7 +141,7 @@ func TestUpdateStepStatusExitedWithCode(t *testing.T) { Error: "an error", } step := &model.Step{} - err := UpdateStepStatus(&mockUpdateStepStore{}, step, state) + err := UpdateStepStatus(mockStoreStep(t), step, state) assert.NoError(t, err) assert.Equal(t, model.StatusFailure, step.State) @@ -149,7 +152,7 @@ func TestUpdateStepPToStatusStarted(t *testing.T) { t.Parallel() state := rpc.State{Started: int64(42)} - step, _ := UpdateStepToStatusStarted(&mockUpdateStepStore{}, model.Step{}, state) + step, _ := UpdateStepToStatusStarted(mockStoreStep(t), model.Step{}, state) assert.Equal(t, model.StatusRunning, step.State) assert.EqualValues(t, 42, step.Started) @@ -158,7 +161,7 @@ func TestUpdateStepPToStatusStarted(t *testing.T) { func TestUpdateStepToStatusSkipped(t *testing.T) { t.Parallel() - step, _ := UpdateStepToStatusSkipped(&mockUpdateStepStore{}, model.Step{}, int64(1)) + step, _ := UpdateStepToStatusSkipped(mockStoreStep(t), model.Step{}, int64(1)) assert.Equal(t, model.StatusSkipped, step.State) assert.EqualValues(t, 0, step.Stopped) @@ -171,7 +174,7 @@ func TestUpdateStepToStatusSkippedButStarted(t *testing.T) { Started: int64(42), } - step, _ = UpdateStepToStatusSkipped(&mockUpdateStepStore{}, *step, int64(1)) + step, _ = UpdateStepToStatusSkipped(mockStoreStep(t), *step, int64(1)) assert.Equal(t, model.StatusSuccess, step.State) assert.EqualValues(t, 1, step.Stopped) @@ -184,7 +187,7 @@ func TestUpdateStepStatusToDoneSkipped(t *testing.T) { Finished: int64(34), } - step, _ := UpdateStepStatusToDone(&mockUpdateStepStore{}, model.Step{}, state) + step, _ := UpdateStepStatusToDone(mockStoreStep(t), model.Step{}, state) assert.Equal(t, model.StatusSkipped, step.State) assert.EqualValues(t, 34, step.Stopped) @@ -200,7 +203,7 @@ func TestUpdateStepStatusToDoneSuccess(t *testing.T) { Finished: int64(34), } - step, _ := UpdateStepStatusToDone(&mockUpdateStepStore{}, model.Step{}, state) + step, _ := UpdateStepStatusToDone(mockStoreStep(t), model.Step{}, state) assert.Equal(t, model.StatusSuccess, step.State) assert.EqualValues(t, 34, step.Stopped) @@ -213,7 +216,7 @@ func TestUpdateStepStatusToDoneFailureWithError(t *testing.T) { state := rpc.State{Error: "an error"} - step, _ := UpdateStepStatusToDone(&mockUpdateStepStore{}, model.Step{}, state) + step, _ := UpdateStepStatusToDone(mockStoreStep(t), model.Step{}, state) assert.Equal(t, model.StatusFailure, step.State) } @@ -223,7 +226,7 @@ func TestUpdateStepStatusToDoneFailureWithExitCode(t *testing.T) { state := rpc.State{ExitCode: 43} - step, _ := UpdateStepStatusToDone(&mockUpdateStepStore{}, model.Step{}, state) + step, _ := UpdateStepStatusToDone(mockStoreStep(t), model.Step{}, state) assert.Equal(t, model.StatusFailure, step.State) } @@ -233,7 +236,7 @@ func TestUpdateStepToStatusKilledStarted(t *testing.T) { now := time.Now().Unix() - step, _ := UpdateStepToStatusKilled(&mockUpdateStepStore{}, model.Step{}) + step, _ := UpdateStepToStatusKilled(mockStoreStep(t), model.Step{}) assert.Equal(t, model.StatusKilled, step.State) assert.LessOrEqual(t, now, step.Stopped) @@ -244,7 +247,7 @@ func TestUpdateStepToStatusKilledStarted(t *testing.T) { func TestUpdateStepToStatusKilledNotStarted(t *testing.T) { t.Parallel() - step, _ := UpdateStepToStatusKilled(&mockUpdateStepStore{}, model.Step{Started: int64(1)}) + step, _ := UpdateStepToStatusKilled(mockStoreStep(t), model.Step{Started: int64(1)}) assert.EqualValues(t, 1, step.Started) } diff --git a/server/pipeline/workflowStatus.go b/server/pipeline/workflowStatus.go index 1489dc56d..f29105ebd 100644 --- a/server/pipeline/workflowStatus.go +++ b/server/pipeline/workflowStatus.go @@ -17,20 +17,21 @@ package pipeline import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/store" ) -func UpdateWorkflowToStatusStarted(store model.UpdateWorkflowStore, workflow model.Workflow, state rpc.State) (*model.Workflow, error) { +func UpdateWorkflowToStatusStarted(store store.Store, workflow model.Workflow, state rpc.State) (*model.Workflow, error) { workflow.Started = state.Started workflow.State = model.StatusRunning return &workflow, store.WorkflowUpdate(&workflow) } -func UpdateWorkflowToStatusSkipped(store model.UpdateWorkflowStore, workflow model.Workflow) (*model.Workflow, error) { +func UpdateWorkflowToStatusSkipped(store store.Store, workflow model.Workflow) (*model.Workflow, error) { workflow.State = model.StatusSkipped return &workflow, store.WorkflowUpdate(&workflow) } -func UpdateWorkflowStatusToDone(store model.UpdateWorkflowStore, workflow model.Workflow, state rpc.State) (*model.Workflow, error) { +func UpdateWorkflowStatusToDone(store store.Store, workflow model.Workflow, state rpc.State) (*model.Workflow, error) { workflow.Stopped = state.Finished workflow.Error = state.Error if state.Started == 0 { diff --git a/server/queue/persistent.go b/server/queue/persistent.go index 354b309cb..96d9fff9f 100644 --- a/server/queue/persistent.go +++ b/server/queue/persistent.go @@ -22,11 +22,12 @@ import ( "github.com/rs/zerolog/log" "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/store" ) // WithTaskStore returns a queue that is backed by the TaskStore. This // ensures the task Queue can be restored when the system starts. -func WithTaskStore(q Queue, s model.TaskStore) Queue { +func WithTaskStore(q Queue, s store.Store) Queue { tasks, _ := s.TaskList() if err := q.PushAtOnce(context.Background(), tasks); err != nil { log.Error().Err(err).Msg("PushAtOnce failed") @@ -36,7 +37,7 @@ func WithTaskStore(q Queue, s model.TaskStore) Queue { type persistentQueue struct { Queue - store model.TaskStore + store store.Store } // Push pushes a task to the tail of this queue. diff --git a/server/services/registry/db.go b/server/services/registry/db.go index 5e7339557..1160905b6 100644 --- a/server/services/registry/db.go +++ b/server/services/registry/db.go @@ -16,14 +16,15 @@ package registry import ( "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/store" ) type db struct { - store model.RegistryStore + store store.Store } // New returns a new local registry service. -func NewDB(store model.RegistryStore) Service { +func NewDB(store store.Store) Service { return &db{store} } diff --git a/server/services/secret/db.go b/server/services/secret/db.go index b3d320261..a89dacf2a 100644 --- a/server/services/secret/db.go +++ b/server/services/secret/db.go @@ -16,14 +16,15 @@ package secret import ( "go.woodpecker-ci.org/woodpecker/v2/server/model" + "go.woodpecker-ci.org/woodpecker/v2/server/store" ) type db struct { - store model.SecretStore + store store.Store } // NewDB returns a new local secret service. -func NewDB(store model.SecretStore) Service { +func NewDB(store store.Store) Service { return &db{store: store} } diff --git a/server/store/datastore/config.go b/server/store/datastore/config.go index 4198d9c06..eda41bd25 100644 --- a/server/store/datastore/config.go +++ b/server/store/datastore/config.go @@ -69,18 +69,6 @@ func (s storage) ConfigPersist(conf *model.Config) (*model.Config, error) { return conf, sess.Commit() } -func (s storage) ConfigFindApproved(config *model.Config) (bool, error) { - return s.engine.Table("pipelines").Select("pipelines.pipeline_id"). - Join("INNER", "pipeline_config", "pipelines.pipeline_id = pipeline_config.pipeline_id"). - Where(builder.Eq{"pipelines.pipeline_repo_id": config.RepoID, "pipeline_config.config_id": config.ID}. - And(builder.NotIn("pipelines.pipeline_status", model.StatusBlocked, model.StatusPending))). - Exist() -} - -func (s storage) ConfigCreate(config *model.Config) error { - return s.configCreate(s.engine.NewSession(), config) -} - func (s storage) configCreate(sess *xorm.Session, config *model.Config) error { // should never happen but just in case if config.Name == "" { diff --git a/server/store/datastore/config_test.go b/server/store/datastore/config_test.go index 1e5f60805..a08c789df 100644 --- a/server/store/datastore/config_test.go +++ b/server/store/datastore/config_test.go @@ -46,7 +46,9 @@ func TestConfig(t *testing.T) { Hash: hash, Name: name, } - assert.NoError(t, store.ConfigCreate(config)) + + _, err := store.ConfigPersist(config) + assert.NoError(t, err) pipeline := &model.Pipeline{ RepoID: repo.ID, @@ -71,118 +73,6 @@ func TestConfig(t *testing.T) { assert.Equal(t, config.ID, loaded[0].ID) } -func TestConfigApproved(t *testing.T) { - store, closer := newTestStore(t, new(model.Config), new(model.PipelineConfig), new(model.Pipeline), new(model.Repo)) - defer closer() - - repo := &model.Repo{ - UserID: 1, - FullName: "bradrydzewski/test", - Owner: "bradrydzewski", - Name: "test", - } - assert.NoError(t, store.CreateRepo(repo)) - - var ( - pipelineBlocked = &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusBlocked, - Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", - } - pipelinePending = &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusPending, - Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", - } - pipelineRunning = &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusRunning, - Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", - } - ) - - assert.NoError(t, store.CreatePipeline(pipelineBlocked)) - assert.NoError(t, store.CreatePipeline(pipelinePending)) - conf := &model.Config{ - RepoID: repo.ID, - Data: data, - Hash: hash, - Name: name, - } - assert.NoError(t, store.ConfigCreate(conf)) - pipelineConfig := &model.PipelineConfig{ - ConfigID: conf.ID, - PipelineID: pipelineBlocked.ID, - } - assert.NoError(t, store.PipelineConfigCreate(pipelineConfig)) - - approved, err := store.ConfigFindApproved(conf) - if !assert.NoError(t, err) { - return - } - assert.False(t, approved, "want config not approved when blocked or pending.") - - assert.NoError(t, store.CreatePipeline(pipelineRunning)) - conf2 := &model.Config{ - RepoID: repo.ID, - Data: data, - Hash: "xxx", - Name: "xxx", - } - assert.NoError(t, store.ConfigCreate(conf2)) - pipelineConfig2 := &model.PipelineConfig{ - ConfigID: conf2.ID, - PipelineID: pipelineRunning.ID, - } - assert.NoError(t, store.PipelineConfigCreate(pipelineConfig2)) - - approved, err = store.ConfigFindApproved(conf2) - assert.NoError(t, err) - assert.True(t, approved) -} - -func TestConfigCreate(t *testing.T) { - store, closer := newTestStore(t, new(model.Config), new(model.Step), new(model.Pipeline), new(model.Repo)) - defer closer() - - // fail due to missing name - assert.Error(t, store.ConfigCreate( - &model.Config{ - RepoID: 2, - Data: data, - Hash: hash, - }, - )) - - // fail due to missing hash - assert.Error(t, store.ConfigCreate( - &model.Config{ - RepoID: 2, - Data: data, - Name: name, - }, - )) - - assert.NoError(t, store.ConfigCreate( - &model.Config{ - RepoID: 2, - Data: data, - Hash: hash, - Name: name, - }, - )) - - // fail due to duplicate sha - assert.Error(t, store.ConfigCreate( - &model.Config{ - RepoID: 2, - Data: data, - Hash: hash, - Name: name, - }, - )) -} - func TestConfigPersist(t *testing.T) { store, closer := newTestStore(t, new(model.Config)) defer closer() diff --git a/server/store/datastore/log.go b/server/store/datastore/log.go index 7ca187b8e..68c708c63 100644 --- a/server/store/datastore/log.go +++ b/server/store/datastore/log.go @@ -15,8 +15,6 @@ package datastore import ( - "fmt" - "go.woodpecker-ci.org/woodpecker/v2/server/model" ) @@ -25,25 +23,6 @@ func (s storage) LogFind(step *model.Step) ([]*model.LogEntry, error) { return logEntries, s.engine.Asc("id").Where("step_id = ?", step.ID).Find(&logEntries) } -func (s storage) LogSave(step *model.Step, logEntries []*model.LogEntry) error { - sess := s.engine.NewSession() - defer sess.Close() - if err := sess.Begin(); err != nil { - return err - } - - for _, logEntry := range logEntries { - if logEntry.StepID != step.ID { - return fmt.Errorf("got a log-entry with step id '%d' but expected '%d'", logEntry.StepID, step.ID) - } - if _, err := sess.Insert(logEntry); err != nil { - return err - } - } - - return sess.Commit() -} - func (s storage) LogAppend(logEntry *model.LogEntry) error { _, err := s.engine.Insert(logEntry) return err diff --git a/server/store/datastore/log_test.go b/server/store/datastore/log_test.go index 3e6729844..f2ee1ffc6 100644 --- a/server/store/datastore/log_test.go +++ b/server/store/datastore/log_test.go @@ -45,8 +45,9 @@ func TestLogCreateFindDelete(t *testing.T) { }, } - // first insert should just work - assert.NoError(t, store.LogSave(&step, logEntries)) + for _, logEntry := range logEntries { + assert.NoError(t, store.LogAppend(logEntry)) + } // we want to find our inserted logs _logEntries, err := store.LogFind(&step) @@ -82,7 +83,9 @@ func TestLogAppend(t *testing.T) { }, } - assert.NoError(t, store.LogSave(&step, logEntries)) + for _, logEntry := range logEntries { + assert.NoError(t, store.LogAppend(logEntry)) + } logEntry := &model.LogEntry{ StepID: step.ID, diff --git a/server/store/datastore/permission.go b/server/store/datastore/permission.go index fdfa1369e..255ede5ce 100644 --- a/server/store/datastore/permission.go +++ b/server/store/datastore/permission.go @@ -72,17 +72,6 @@ func (s storage) permUpsert(sess *xorm.Session, perm *model.Perm) error { return err } -func (s storage) PermDelete(perm *model.Perm) error { - return wrapDelete(s.engine.Where(userIDAndRepoIDCond(perm)).Delete(new(model.Perm))) -} - -func (s storage) PermFlush(user *model.User, before int64) error { - _, err := s.engine. - Where("perm_user_id = ? AND perm_synced < ?", user.ID, before). - Delete(new(model.Perm)) - return err -} - // userPushOrAdminCondition return condition where user must have push or admin rights // if used make sure to have permission table ("perms") joined func userPushOrAdminCondition(userID int64) builder.Cond { diff --git a/server/store/datastore/permission_test.go b/server/store/datastore/permission_test.go index 46c1f9651..2e7e8107a 100644 --- a/server/store/datastore/permission_test.go +++ b/server/store/datastore/permission_test.go @@ -110,37 +110,3 @@ func TestPermUpsert(t *testing.T) { assert.True(t, perm.Push) assert.True(t, perm.Admin) } - -func TestPermDelete(t *testing.T) { - store, closer := newTestStore(t, new(model.Repo), new(model.Perm), new(model.User)) - defer closer() - - user := &model.User{ID: 1} - repo := &model.Repo{ - UserID: 1, - FullName: "bradrydzewski/test", - Owner: "bradrydzewski", - Name: "test", - ForgeRemoteID: "1", - } - assert.NoError(t, store.CreateRepo(repo)) - - err := store.PermUpsert( - &model.Perm{ - UserID: user.ID, - RepoID: repo.ID, - Repo: repo, - Pull: true, - Push: false, - Admin: false, - }, - ) - assert.NoError(t, err) - - perm, err := store.PermFind(user, repo) - assert.NoError(t, err) - err = store.PermDelete(perm) - assert.NoError(t, err) - _, err = store.PermFind(user, repo) - assert.Error(t, err) -} diff --git a/server/store/datastore/pipeline.go b/server/store/datastore/pipeline.go index a3f436d5e..a397bc383 100644 --- a/server/store/datastore/pipeline.go +++ b/server/store/datastore/pipeline.go @@ -35,20 +35,6 @@ func (s storage) GetPipelineNumber(repo *model.Repo, num int64) (*model.Pipeline ).Get(pipeline)) } -func (s storage) GetPipelineRef(repo *model.Repo, ref string) (*model.Pipeline, error) { - pipeline := new(model.Pipeline) - return pipeline, wrapGet(s.engine.Where( - builder.Eq{"pipeline_repo_id": repo.ID, "pipeline_ref": ref}, - ).Get(pipeline)) -} - -func (s storage) GetPipelineCommit(repo *model.Repo, sha, branch string) (*model.Pipeline, error) { - pipeline := new(model.Pipeline) - return pipeline, wrapGet(s.engine.Where( - builder.Eq{"pipeline_repo_id": repo.ID, "pipeline_branch": branch, "pipeline_commit": sha}, - ).Get(pipeline)) -} - func (s storage) GetPipelineLast(repo *model.Repo, branch string) (*model.Pipeline, error) { pipeline := new(model.Pipeline) return pipeline, wrapGet(s.engine. diff --git a/server/store/datastore/pipeline_test.go b/server/store/datastore/pipeline_test.go index 8b86dde5e..7ba30fd19 100644 --- a/server/store/datastore/pipeline_test.go +++ b/server/store/datastore/pipeline_test.go @@ -144,78 +144,6 @@ func TestPipelines(t *testing.T) { g.Assert(pipeline2.Number).Equal(GetPipeline.Number) }) - g.It("Should Get a Pipeline by Ref", func() { - pipeline1 := &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusPending, - Ref: "refs/pull/5", - } - pipeline2 := &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusPending, - Ref: "refs/pull/6", - } - err1 := store.CreatePipeline(pipeline1, []*model.Step{}...) - g.Assert(err1).IsNil() - err2 := store.CreatePipeline(pipeline2, []*model.Step{}...) - g.Assert(err2).IsNil() - GetPipeline, err3 := store.GetPipelineRef(&model.Repo{ID: 1}, "refs/pull/6") - g.Assert(err3).IsNil() - g.Assert(pipeline2.ID).Equal(GetPipeline.ID) - g.Assert(pipeline2.RepoID).Equal(GetPipeline.RepoID) - g.Assert(pipeline2.Number).Equal(GetPipeline.Number) - g.Assert(pipeline2.Ref).Equal(GetPipeline.Ref) - }) - - g.It("Should Get a Pipeline by Ref", func() { - pipeline1 := &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusPending, - Ref: "refs/pull/5", - } - pipeline2 := &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusPending, - Ref: "refs/pull/6", - } - err1 := store.CreatePipeline(pipeline1, []*model.Step{}...) - g.Assert(err1).IsNil() - err2 := store.CreatePipeline(pipeline2, []*model.Step{}...) - g.Assert(err2).IsNil() - GetPipeline, err3 := store.GetPipelineRef(&model.Repo{ID: 1}, "refs/pull/6") - g.Assert(err3).IsNil() - g.Assert(pipeline2.ID).Equal(GetPipeline.ID) - g.Assert(pipeline2.RepoID).Equal(GetPipeline.RepoID) - g.Assert(pipeline2.Number).Equal(GetPipeline.Number) - g.Assert(pipeline2.Ref).Equal(GetPipeline.Ref) - }) - - g.It("Should Get a Pipeline by Commit", func() { - pipeline1 := &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusPending, - Branch: "main", - Commit: "85f8c029b902ed9400bc600bac301a0aadb144ac", - } - pipeline2 := &model.Pipeline{ - RepoID: repo.ID, - Status: model.StatusPending, - Branch: "dev", - Commit: "85f8c029b902ed9400bc600bac301a0aadb144aa", - } - err1 := store.CreatePipeline(pipeline1, []*model.Step{}...) - g.Assert(err1).IsNil() - err2 := store.CreatePipeline(pipeline2, []*model.Step{}...) - g.Assert(err2).IsNil() - GetPipeline, err3 := store.GetPipelineCommit(&model.Repo{ID: 1}, pipeline2.Commit, pipeline2.Branch) - g.Assert(err3).IsNil() - g.Assert(pipeline2.ID).Equal(GetPipeline.ID) - g.Assert(pipeline2.RepoID).Equal(GetPipeline.RepoID) - g.Assert(pipeline2.Number).Equal(GetPipeline.Number) - g.Assert(pipeline2.Commit).Equal(GetPipeline.Commit) - g.Assert(pipeline2.Branch).Equal(GetPipeline.Branch) - }) - g.It("Should Get the last Pipeline", func() { pipeline1 := &model.Pipeline{ RepoID: repo.ID, diff --git a/server/store/datastore/redirection.go b/server/store/datastore/redirection.go index 1c966f964..f9dd1a34a 100644 --- a/server/store/datastore/redirection.go +++ b/server/store/datastore/redirection.go @@ -21,12 +21,6 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/server/model" ) -func (s storage) GetRedirection(fullName string) (*model.Redirection, error) { - sess := s.engine.NewSession() - defer sess.Close() - return s.getRedirection(sess, fullName) -} - func (s storage) getRedirection(e *xorm.Session, fullName string) (*model.Redirection, error) { repo := new(model.Redirection) return repo, wrapGet(e.Where("repo_full_name = ?", fullName).Get(repo)) diff --git a/server/store/datastore/redirection_test.go b/server/store/datastore/redirection_test.go index a4d4c18de..8e72cd048 100644 --- a/server/store/datastore/redirection_test.go +++ b/server/store/datastore/redirection_test.go @@ -20,26 +20,8 @@ import ( "github.com/stretchr/testify/assert" "go.woodpecker-ci.org/woodpecker/v2/server/model" - "go.woodpecker-ci.org/woodpecker/v2/server/store/types" ) -func TestGetRedirection(t *testing.T) { - store, closer := newTestStore(t, new(model.Redirection)) - defer closer() - - redirection := &model.Redirection{ - RepoID: 1, - FullName: "foo/bar", - } - assert.NoError(t, store.CreateRedirection(redirection)) - redirectionFromStore, err := store.GetRedirection("foo/bar") - assert.NoError(t, err) - assert.NotNil(t, redirectionFromStore) - assert.Equal(t, redirection.RepoID, redirectionFromStore.RepoID) - _, err = store.GetRedirection("foo/baz") - assert.ErrorIs(t, err, types.RecordNotExist) -} - func TestCreateRedirection(t *testing.T) { store, closer := newTestStore(t, new(model.Redirection)) defer closer() diff --git a/server/store/store.go b/server/store/store.go index 8cd528c0c..04fcbadc3 100644 --- a/server/store/store.go +++ b/server/store/store.go @@ -61,8 +61,6 @@ type Store interface { DeleteRepo(*model.Repo) error // Redirections - // GetRedirection returns the redirection for the given full repo name - GetRedirection(string) (*model.Redirection, error) // CreateRedirection creates a redirection CreateRedirection(redirection *model.Redirection) error // HasRedirectionForRepo checks if there's a redirection for the given repo and full name @@ -73,10 +71,6 @@ type Store interface { GetPipeline(int64) (*model.Pipeline, error) // GetPipelineNumber gets a pipeline by number. GetPipelineNumber(*model.Repo, int64) (*model.Pipeline, error) - // GetPipelineRef gets a pipeline by its ref. - GetPipelineRef(*model.Repo, string) (*model.Pipeline, error) - // GetPipelineCommit gets a pipeline by its commit sha. - GetPipelineCommit(*model.Repo, string, string) (*model.Pipeline, error) // GetPipelineLast gets the last pipeline for the branch. GetPipelineLast(*model.Repo, string) (*model.Pipeline, error) // GetPipelineLastBefore gets the last pipeline before pipeline number N. @@ -107,14 +101,10 @@ type Store interface { // Permissions PermFind(user *model.User, repo *model.Repo) (*model.Perm, error) PermUpsert(perm *model.Perm) error - PermDelete(perm *model.Perm) error - PermFlush(user *model.User, before int64) error // Configs ConfigsForPipeline(pipelineID int64) ([]*model.Config, error) ConfigPersist(*model.Config) (*model.Config, error) - ConfigFindApproved(*model.Config) (bool, error) - ConfigCreate(*model.Config) error PipelineConfigCreate(*model.PipelineConfig) error // Secrets @@ -147,7 +137,6 @@ type Store interface { // Logs LogFind(*model.Step) ([]*model.LogEntry, error) - LogSave(*model.Step, []*model.LogEntry) error LogAppend(logEntry *model.LogEntry) error LogDelete(*model.Step) error