From 726e1e5279b6461cad8eee2e461c2c6d8ab0f075 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 29 May 2020 15:24:15 +0200 Subject: [PATCH] Doctor check & fix db consistency (#11111) needed to fix issue as described in #10280 * rename check-db to check-db-version * add check-db-consistency: * find issues without existing repository * find pulls without existing issues * find tracked times without existing issues/pulls * find labels without repository or org reference Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com> Co-authored-by: Lunny Xiao --- cmd/doctor.go | 87 ++++++++++++++++++++++++++++++- models/consistency.go | 116 ++++++++++++++++++++++++++++++++++++++++++ models/issue.go | 67 ++++++++++++++++++++++++ models/models.go | 4 ++ models/repo.go | 65 ++--------------------- 5 files changed, 275 insertions(+), 64 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index f469496cf..3456f26f7 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -85,10 +85,16 @@ var checklist = []check{ }, { title: "Check Database Version", - name: "check-db", + name: "check-db-version", isDefault: true, f: runDoctorCheckDBVersion, - abortIfFailed: true, + abortIfFailed: false, + }, + { + title: "Check consistency of database", + name: "check-db-consistency", + isDefault: false, + f: runDoctorCheckDBConsistency, }, { title: "Check if OpenSSH authorized_keys file is up-to-date", @@ -495,3 +501,80 @@ func runDoctorScriptType(ctx *cli.Context) ([]string, error) { } return []string{fmt.Sprintf("ScriptType %s is on the current PATH at %s", setting.ScriptType, path)}, nil } + +func runDoctorCheckDBConsistency(ctx *cli.Context) ([]string, error) { + var results []string + + // make sure DB version is uptodate + if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil { + return nil, fmt.Errorf("model version on the database does not match the current Gitea version. Model consistency will not be checked until the database is upgraded") + } + + //find labels without existing repo or org + count, err := models.CountOrphanedLabels() + if err != nil { + return nil, err + } + if count > 0 { + if ctx.Bool("fix") { + if err = models.DeleteOrphanedLabels(); err != nil { + return nil, err + } + results = append(results, fmt.Sprintf("%d labels without existing repository/organisation deleted", count)) + } else { + results = append(results, fmt.Sprintf("%d labels without existing repository/organisation", count)) + } + } + + //find issues without existing repository + count, err = models.CountOrphanedIssues() + if err != nil { + return nil, err + } + if count > 0 { + if ctx.Bool("fix") { + if err = models.DeleteOrphanedIssues(); err != nil { + return nil, err + } + results = append(results, fmt.Sprintf("%d issues without existing repository deleted", count)) + } else { + results = append(results, fmt.Sprintf("%d issues without existing repository", count)) + } + } + + //find pulls without existing issues + count, err = models.CountOrphanedObjects("pull_request", "issue", "pull_request.issue_id=issue.id") + if err != nil { + return nil, err + } + if count > 0 { + if ctx.Bool("fix") { + if err = models.DeleteOrphanedObjects("pull_request", "issue", "pull_request.issue_id=issue.id"); err != nil { + return nil, err + } + results = append(results, fmt.Sprintf("%d pull requests without existing issue deleted", count)) + } else { + results = append(results, fmt.Sprintf("%d pull requests without existing issue", count)) + } + } + + //find tracked times without existing issues/pulls + count, err = models.CountOrphanedObjects("tracked_time", "issue", "tracked_time.issue_id=issue.id") + if err != nil { + return nil, err + } + if count > 0 { + if ctx.Bool("fix") { + if err = models.DeleteOrphanedObjects("tracked_time", "issue", "tracked_time.issue_id=issue.id"); err != nil { + return nil, err + } + results = append(results, fmt.Sprintf("%d tracked times without existing issue deleted", count)) + } else { + results = append(results, fmt.Sprintf("%d tracked times without existing issue", count)) + } + } + + //ToDo: function to recalc all counters + + return results, nil +} diff --git a/models/consistency.go b/models/consistency.go index 62d1d2e87..d6a158840 100644 --- a/models/consistency.go +++ b/models/consistency.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "xorm.io/builder" ) // consistencyCheckable a type that can be tested for database consistency @@ -167,3 +168,118 @@ func (action *Action) checkForConsistency(t *testing.T) { repo := AssertExistsAndLoadBean(t, &Repository{ID: action.RepoID}).(*Repository) assert.Equal(t, repo.IsPrivate, action.IsPrivate, "action: %+v", action) } + +// CountOrphanedLabels return count of labels witch are broken and not accessible via ui anymore +func CountOrphanedLabels() (int64, error) { + noref, err := x.Table("label").Where("repo_id=? AND org_id=?", 0, 0).Count("label.id") + if err != nil { + return 0, err + } + + norepo, err := x.Table("label"). + Join("LEFT", "repository", "label.repo_id=repository.id"). + Where(builder.IsNull{"repository.id"}).And(builder.Gt{"label.repo_id": 0}). + Count("id") + if err != nil { + return 0, err + } + + noorg, err := x.Table("label"). + Join("LEFT", "`user`", "label.org_id=`user`.id"). + Where(builder.IsNull{"`user`.id"}).And(builder.Gt{"label.org_id": 0}). + Count("id") + if err != nil { + return 0, err + } + + return noref + norepo + noorg, nil +} + +// DeleteOrphanedLabels delete labels witch are broken and not accessible via ui anymore +func DeleteOrphanedLabels() error { + // delete labels with no reference + if _, err := x.Table("label").Where("repo_id=? AND org_id=?", 0, 0).Delete(new(Label)); err != nil { + return err + } + + // delete labels with none existing repos + if _, err := x.In("id", builder.Select("label.id").From("label"). + Join("LEFT", "repository", "label.repo_id=repository.id"). + Where(builder.IsNull{"repository.id"}).And(builder.Gt{"label.repo_id": 0})). + Delete(Label{}); err != nil { + return err + } + + // delete labels with none existing orgs + if _, err := x.In("id", builder.Select("label.id").From("label"). + Join("LEFT", "`user`", "label.org_id=`user`.id"). + Where(builder.IsNull{"`user`.id"}).And(builder.Gt{"label.org_id": 0})). + Delete(Label{}); err != nil { + return err + } + + return nil +} + +// CountOrphanedIssues count issues without a repo +func CountOrphanedIssues() (int64, error) { + return x.Table("issue"). + Join("LEFT", "repository", "issue.repo_id=repository.id"). + Where(builder.IsNull{"repository.id"}). + Count("id") +} + +// DeleteOrphanedIssues delete issues without a repo +func DeleteOrphanedIssues() error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + var ids []int64 + + if err := sess.Table("issue").Distinct("issue.repo_id"). + Join("LEFT", "repository", "issue.repo_id=repository.id"). + Where(builder.IsNull{"repository.id"}).GroupBy("issue.repo_id"). + Find(&ids); err != nil { + return err + } + + var attachmentPaths []string + for i := range ids { + paths, err := deleteIssuesByRepoID(sess, ids[i]) + if err != nil { + return err + } + attachmentPaths = append(attachmentPaths, paths...) + } + + if err := sess.Commit(); err != nil { + return err + } + + // Remove issue attachment files. + for i := range attachmentPaths { + removeAllWithNotice(x, "Delete issue attachment", attachmentPaths[i]) + } + return nil +} + +// CountOrphanedObjects count subjects with have no existing refobject anymore +func CountOrphanedObjects(subject, refobject, joinCond string) (int64, error) { + return x.Table("`"+subject+"`"). + Join("LEFT", refobject, joinCond). + Where(builder.IsNull{"`" + refobject + "`.id"}). + Count("id") +} + +// DeleteOrphanedObjects delete subjects with have no existing refobject anymore +func DeleteOrphanedObjects(subject, refobject, joinCond string) error { + _, err := x.In("id", builder.Select("`"+subject+"`.id"). + From("`"+subject+"`"). + Join("LEFT", "`"+refobject+"`", joinCond). + Where(builder.IsNull{"`" + refobject + "`.id"})). + Delete("`" + subject + "`") + return err +} diff --git a/models/issue.go b/models/issue.go index 157ad48de..82f6c926e 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1916,3 +1916,70 @@ func UpdateReactionsMigrationsByType(gitServiceType structs.GitServiceType, orig }) return err } + +func deleteIssuesByRepoID(sess Engine, repoID int64) (attachmentPaths []string, err error) { + deleteCond := builder.Select("id").From("issue").Where(builder.Eq{"issue.repo_id": repoID}) + + // Delete comments and attachments + if _, err = sess.In("issue_id", deleteCond). + Delete(&Comment{}); err != nil { + return + } + + // Dependencies for issues in this repository + if _, err = sess.In("issue_id", deleteCond). + Delete(&IssueDependency{}); err != nil { + return + } + + // Delete dependencies for issues in other repositories + if _, err = sess.In("dependency_id", deleteCond). + Delete(&IssueDependency{}); err != nil { + return + } + + if _, err = sess.In("issue_id", deleteCond). + Delete(&IssueUser{}); err != nil { + return + } + + if _, err = sess.In("issue_id", deleteCond). + Delete(&Reaction{}); err != nil { + return + } + + if _, err = sess.In("issue_id", deleteCond). + Delete(&IssueWatch{}); err != nil { + return + } + + if _, err = sess.In("issue_id", deleteCond). + Delete(&Stopwatch{}); err != nil { + return + } + + if _, err = sess.In("issue_id", deleteCond). + Delete(&TrackedTime{}); err != nil { + return + } + + var attachments []*Attachment + if err = sess.In("issue_id", deleteCond). + Find(&attachments); err != nil { + return + } + for j := range attachments { + attachmentPaths = append(attachmentPaths, attachments[j].LocalPath()) + } + + if _, err = sess.In("issue_id", deleteCond). + Delete(&Attachment{}); err != nil { + return + } + + if _, err = sess.Delete(&Issue{RepoID: repoID}); err != nil { + return + } + + return +} diff --git a/models/models.go b/models/models.go index c818c6510..7f12d6260 100644 --- a/models/models.go +++ b/models/models.go @@ -182,6 +182,10 @@ func SetEngine() (err error) { } // NewEngine initializes a new xorm.Engine +// This function must never call .Sync2() if the provided migration function fails. +// When called from the "doctor" command, the migration function is a version check +// that prevents the doctor from fixing anything in the database if the migration level +// is different from the expected value. func NewEngine(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err error) { if err = SetEngine(); err != nil { return err diff --git a/models/repo.go b/models/repo.go index abf32360a..d31d262c8 100644 --- a/models/repo.go +++ b/models/repo.go @@ -35,7 +35,6 @@ import ( "code.gitea.io/gitea/modules/util" "github.com/unknwon/com" - "xorm.io/builder" ) var ( @@ -1601,67 +1600,9 @@ func DeleteRepository(doer *User, uid, repoID int64) error { return fmt.Errorf("deleteBeans: %v", err) } - deleteCond := builder.Select("id").From("issue").Where(builder.Eq{"repo_id": repoID}) - // Delete comments and attachments - if _, err = sess.In("issue_id", deleteCond). - Delete(&Comment{}); err != nil { - return err - } - - // Dependencies for issues in this repository - if _, err = sess.In("issue_id", deleteCond). - Delete(&IssueDependency{}); err != nil { - return err - } - - // Delete dependencies for issues in other repositories - if _, err = sess.In("dependency_id", deleteCond). - Delete(&IssueDependency{}); err != nil { - return err - } - - if _, err = sess.In("issue_id", deleteCond). - Delete(&IssueUser{}); err != nil { - return err - } - - if _, err = sess.In("issue_id", deleteCond). - Delete(&Reaction{}); err != nil { - return err - } - - if _, err = sess.In("issue_id", deleteCond). - Delete(&IssueWatch{}); err != nil { - return err - } - - if _, err = sess.In("issue_id", deleteCond). - Delete(&Stopwatch{}); err != nil { - return err - } - - if _, err = sess.In("issue_id", deleteCond). - Delete(&TrackedTime{}); err != nil { - return err - } - - attachments = attachments[:0] - if err = sess.Join("INNER", "issue", "issue.id = attachment.issue_id"). - Where("issue.repo_id = ?", repoID). - Find(&attachments); err != nil { - return err - } - attachmentPaths := make([]string, 0, len(attachments)) - for j := range attachments { - attachmentPaths = append(attachmentPaths, attachments[j].LocalPath()) - } - - if _, err = sess.In("issue_id", deleteCond). - Delete(&Attachment{}); err != nil { - return err - } - - if _, err = sess.Delete(&Issue{RepoID: repoID}); err != nil { + // Delete Issues and related objects + var attachmentPaths []string + if attachmentPaths, err = deleteIssuesByRepoID(sess, repoID); err != nil { return err }