From fe6c999160f9db0fb697576c0ba19d2d3c513401 Mon Sep 17 00:00:00 2001 From: Anbraten Date: Sat, 11 Dec 2021 16:03:14 +0100 Subject: [PATCH] Let remove be a remove (#593) * UI: let remove be a remove * UI: add deactivate repo btn * Store: DeleteRepo also delete related * Store: more test coverage Co-authored-by: 6543 <6543@obermui.de> --- server/api/repo.go | 9 +-- server/model/log.go | 1 + server/store/datastore/build.go | 27 +++++++++ server/store/datastore/proc.go | 13 +++++ server/store/datastore/repo.go | 48 +++++++++++++++- server/store/datastore/repo_test.go | 56 +++++++++++++++---- web/src/components/atomic/Icon.vue | 4 +- .../components/repo/settings/ActionsTab.vue | 23 +++++++- web/src/lib/api/index.ts | 5 +- 9 files changed, 163 insertions(+), 23 deletions(-) diff --git a/server/api/repo.go b/server/api/repo.go index 243fc44ea..861b7cb84 100644 --- a/server/api/repo.go +++ b/server/api/repo.go @@ -199,7 +199,6 @@ func GetRepoBranches(c *gin.Context) { func DeleteRepo(c *gin.Context) { remove, _ := strconv.ParseBool(c.Query("remove")) - remote := server.Config.Services.Remote _store := store.FromContext(c) repo := session.Repo(c) @@ -208,21 +207,19 @@ func DeleteRepo(c *gin.Context) { repo.IsActive = false repo.UserID = 0 - err := _store.UpdateRepo(repo) - if err != nil { + if err := _store.UpdateRepo(repo); err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return } if remove { - err := _store.DeleteRepo(repo) - if err != nil { + if err := _store.DeleteRepo(repo); err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return } } - if err := remote.Deactivate(c, user, repo, server.Config.Server.Host); err != nil { + if err := server.Config.Services.Remote.Deactivate(c, user, repo, server.Config.Server.Host); err != nil { _ = c.AbortWithError(http.StatusInternalServerError, err) return } diff --git a/server/model/log.go b/server/model/log.go index 3fea5396a..8bf7f683a 100644 --- a/server/model/log.go +++ b/server/model/log.go @@ -18,4 +18,5 @@ type Logs struct { ID int64 `xorm:"pk autoincr 'log_id'"` ProcID int64 `xorm:"UNIQUE 'log_job_id'"` Data []byte `xorm:"log_data"` + // TODO: add create timestamp } diff --git a/server/store/datastore/build.go b/server/store/datastore/build.go index b32ef14d9..4b7e9eb18 100644 --- a/server/store/datastore/build.go +++ b/server/store/datastore/build.go @@ -17,6 +17,8 @@ package datastore import ( "time" + "xorm.io/xorm" + "github.com/woodpecker-ci/woodpecker/server/model" ) @@ -108,6 +110,7 @@ func (s storage) CreateBuild(build *model.Build, procList ...*model.Proc) error } for i := range procList { + procList[i].BuildID = build.ID // only Insert set auto created ID back to object if _, err := sess.Insert(procList[i]); err != nil { return err @@ -121,3 +124,27 @@ func (s storage) UpdateBuild(build *model.Build) error { _, err := s.engine.ID(build.ID).AllCols().Update(build) return err } + +func deleteBuild(sess *xorm.Session, buildID int64) error { + // delete related procs + for startProcs := 0; ; startProcs += perPage { + procIDs := make([]int64, 0, perPage) + if err := sess.Limit(perPage, startProcs).Table("procs").Cols("proc_id").Where("proc_build_id = ?", buildID).Find(&procIDs); err != nil { + return err + } + if len(procIDs) == 0 { + break + } + + for i := range procIDs { + if err := deleteProc(sess, procIDs[i]); err != nil { + return err + } + } + } + if _, err := sess.Where("build_id = ?", buildID).Delete(new(model.BuildConfig)); err != nil { + return err + } + _, err := sess.ID(buildID).Delete(new(model.Build)) + return err +} diff --git a/server/store/datastore/proc.go b/server/store/datastore/proc.go index 7924e62ec..9408d511c 100644 --- a/server/store/datastore/proc.go +++ b/server/store/datastore/proc.go @@ -15,6 +15,8 @@ package datastore import ( + "xorm.io/xorm" + "github.com/woodpecker-ci/woodpecker/server/model" ) @@ -84,3 +86,14 @@ func (s storage) ProcClear(build *model.Build) error { return sess.Commit() } + +func deleteProc(sess *xorm.Session, procID int64) error { + if _, err := sess.Where("log_job_id = ?", procID).Delete(new(model.Logs)); err != nil { + return err + } + if _, err := sess.Where("file_proc_id = ?", procID).Delete(new(model.File)); err != nil { + return err + } + _, err := sess.ID(procID).Delete(new(model.Proc)) + return err +} diff --git a/server/store/datastore/repo.go b/server/store/datastore/repo.go index f88646adf..97c036a69 100644 --- a/server/store/datastore/repo.go +++ b/server/store/datastore/repo.go @@ -54,9 +54,51 @@ func (s storage) UpdateRepo(repo *model.Repo) error { } func (s storage) DeleteRepo(repo *model.Repo) error { - _, err := s.engine.ID(repo.ID).Delete(new(model.Repo)) - // TODO: delete related within a session - return err + const batchSize = perPage + sess := s.engine.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + if _, err := sess.Where("sender_repo_id = ?", repo.ID).Delete(new(model.Sender)); err != nil { + return err + } + if _, err := sess.Where("config_repo_id = ?", repo.ID).Delete(new(model.Config)); err != nil { + return err + } + if _, err := sess.Where("perm_repo_id = ?", repo.ID).Delete(new(model.Perm)); err != nil { + return err + } + if _, err := sess.Where("registry_repo_id = ?", repo.ID).Delete(new(model.Registry)); err != nil { + return err + } + if _, err := sess.Where("secret_repo_id = ?", repo.ID).Delete(new(model.Secret)); err != nil { + return err + } + + // delete related builds + for startBuilds := 0; ; startBuilds += batchSize { + buildIDs := make([]int64, 0, batchSize) + if err := sess.Limit(batchSize, startBuilds).Table("builds").Cols("build_id").Where("build_repo_id = ?", repo.ID).Find(&buildIDs); err != nil { + return err + } + if len(buildIDs) == 0 { + break + } + + for i := range buildIDs { + if err := deleteBuild(sess, buildIDs[i]); err != nil { + return err + } + } + } + + if _, err := sess.ID(repo.ID).Delete(new(model.Repo)); err != nil { + return err + } + + return sess.Commit() } // RepoList list all repos where permissions fo specific user are stored diff --git a/server/store/datastore/repo_test.go b/server/store/datastore/repo_test.go index d9f53b548..5f5fccd6d 100644 --- a/server/store/datastore/repo_test.go +++ b/server/store/datastore/repo_test.go @@ -362,7 +362,19 @@ func TestRepoBatch(t *testing.T) { } func TestRepoCrud(t *testing.T) { - store, closer := newTestStore(t, new(model.Repo), new(model.User), new(model.Perm)) + store, closer := newTestStore(t, + new(model.Repo), + new(model.User), + new(model.Perm), + new(model.Build), + new(model.BuildConfig), + new(model.Logs), + new(model.Proc), + new(model.File), + new(model.Secret), + new(model.Sender), + new(model.Registry), + new(model.Config)) defer closer() repo := model.Repo{ @@ -372,16 +384,40 @@ func TestRepoCrud(t *testing.T) { Name: "test", } assert.NoError(t, store.CreateRepo(&repo)) - _, err1 := store.GetRepo(repo.ID) - err2 := store.DeleteRepo(&repo) - _, err3 := store.GetRepo(repo.ID) - if err1 != nil { - t.Errorf("Unexpected error: select repository: %s", err1) + build := model.Build{ + RepoID: repo.ID, } - if err2 != nil { - t.Errorf("Unexpected error: delete repository: %s", err2) + proc := model.Proc{ + Name: "a proc", } - if err3 == nil { - t.Errorf("Expected error: sql.ErrNoRows") + assert.NoError(t, store.CreateBuild(&build, &proc)) + + // create unrelated + repoUnrelated := model.Repo{ + UserID: 2, + FullName: "x/x", + Owner: "x", + Name: "x", } + assert.NoError(t, store.CreateRepo(&repoUnrelated)) + buildUnrelated := model.Build{ + RepoID: repoUnrelated.ID, + } + procUnrelated := model.Proc{ + Name: "a unrelated proc", + } + assert.NoError(t, store.CreateBuild(&buildUnrelated, &procUnrelated)) + + _, err := store.GetRepo(repo.ID) + assert.NoError(t, err) + assert.NoError(t, store.DeleteRepo(&repo)) + _, err = store.GetRepo(repo.ID) + assert.Error(t, err) + + procCount, err := store.engine.Count(new(model.Proc)) + assert.NoError(t, err) + assert.EqualValues(t, 1, procCount) + buildCount, err := store.engine.Count(new(model.Build)) + assert.NoError(t, err) + assert.EqualValues(t, 1, buildCount) } diff --git a/web/src/components/atomic/Icon.vue b/web/src/components/atomic/Icon.vue index 121f08d21..f0a84a7f8 100644 --- a/web/src/components/atomic/Icon.vue +++ b/web/src/components/atomic/Icon.vue @@ -30,6 +30,7 @@ +
@@ -68,7 +69,8 @@ export type IconNames = | 'dark' | 'light' | 'sync' - | 'heal'; + | 'heal' + | 'turn-off'; export default defineComponent({ name: 'Icon', diff --git a/web/src/components/repo/settings/ActionsTab.vue b/web/src/components/repo/settings/ActionsTab.vue index d1c69d0e3..36f955ed1 100644 --- a/web/src/components/repo/settings/ActionsTab.vue +++ b/web/src/components/repo/settings/ActionsTab.vue @@ -14,6 +14,15 @@ @click="repairRepo" /> +