Doctor check & fix db consistency (#11111) (#11676)

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 <xiaolunwen@gmail.com>

Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

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
+}

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
+}

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
+}

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

models/repo.go

@@ -35,7 +35,6 @@ import (
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/unknwon/com"
-	"xorm.io/builder"
 )
 
 var (
@@ -1590,67 +1589,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 Issues and related objects
-	// Delete comments and attachments
+	var attachmentPaths []string
-	if _, err = sess.In("issue_id", deleteCond).
+	if attachmentPaths, err = deleteIssuesByRepoID(sess, repoID); err != nil {
-		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 {
 		return err
 	}