From 076cead40dd4cd498a70f4bd09b0f0077b26144e Mon Sep 17 00:00:00 2001 From: Aravinth Manivannan Date: Mon, 17 Jan 2022 18:31:58 +0000 Subject: [PATCH] Fix CheckRepoStats and reuse it during migration (#18264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CheckRepoStats function missed the following counters: - label num_closed_issues & num_closed_pulls - milestone num_closed_issues & num_closed_pulls The update SQL statements for updating the repository num_closed_issues & num_closed_pulls fields were repeated in three functions (repo.CheckRepoStats, migrate.insertIssues and models.Issue.updateClosedNum) and were moved to a single helper. The UpdateRepoStats is implemented and called in the Finish migration method so that it happens immediately instead of wating for the CheckRepoStats to run. Signed-off-by: Loïc Dachary loic@dachary.org --- [source](https://lab.forgefriends.org/forgefriends/forgefriends/-/merge_requests/34) --- models/issue.go | 37 ++--- models/issue_comment.go | 4 +- models/issue_lock.go | 2 +- models/issue_milestone.go | 40 +++-- models/issue_milestone_test.go | 4 +- models/issue_test.go | 2 +- models/migrate.go | 67 ++------ models/repo.go | 222 ++++++++++++++++++-------- models/repo_test.go | 5 + services/migrations/gitea_uploader.go | 4 + 10 files changed, 219 insertions(+), 168 deletions(-) diff --git a/models/issue.go b/models/issue.go index 108d9b217..cb5791be9 100644 --- a/models/issue.go +++ b/models/issue.go @@ -595,8 +595,8 @@ func (issue *Issue) ReadBy(userID int64) error { return setIssueNotificationStatusReadIfUnread(db.GetEngine(db.DefaultContext), userID, issue.ID) } -func updateIssueCols(e db.Engine, issue *Issue, cols ...string) error { - if _, err := e.ID(issue.ID).Cols(cols...).Update(issue); err != nil { +func updateIssueCols(ctx context.Context, issue *Issue, cols ...string) error { + if _, err := db.GetEngine(ctx).ID(issue.ID).Cols(cols...).Update(issue); err != nil { return err } return nil @@ -646,7 +646,7 @@ func (issue *Issue) doChangeStatus(ctx context.Context, doer *user_model.User, i issue.ClosedUnix = 0 } - if err := updateIssueCols(e, issue, "is_closed", "closed_unix"); err != nil { + if err := updateIssueCols(ctx, issue, "is_closed", "closed_unix"); err != nil { return nil, err } @@ -662,12 +662,12 @@ func (issue *Issue) doChangeStatus(ctx context.Context, doer *user_model.User, i // Update issue count of milestone if issue.MilestoneID > 0 { - if err := updateMilestoneCounters(e, issue.MilestoneID); err != nil { + if err := updateMilestoneCounters(ctx, issue.MilestoneID); err != nil { return nil, err } } - if err := issue.updateClosedNum(e); err != nil { + if err := issue.updateClosedNum(ctx); err != nil { return nil, err } @@ -722,7 +722,7 @@ func (issue *Issue) ChangeTitle(doer *user_model.User, oldTitle string) (err err } defer committer.Close() - if err = updateIssueCols(db.GetEngine(ctx), issue, "name"); err != nil { + if err = updateIssueCols(ctx, issue, "name"); err != nil { return fmt.Errorf("updateIssueCols: %v", err) } @@ -756,7 +756,7 @@ func (issue *Issue) ChangeRef(doer *user_model.User, oldRef string) (err error) } defer committer.Close() - if err = updateIssueCols(db.GetEngine(ctx), issue, "ref"); err != nil { + if err = updateIssueCols(ctx, issue, "ref"); err != nil { return fmt.Errorf("updateIssueCols: %v", err) } @@ -847,7 +847,7 @@ func (issue *Issue) ChangeContent(doer *user_model.User, content string) (err er issue.Content = content - if err = updateIssueCols(db.GetEngine(ctx), issue, "content"); err != nil { + if err = updateIssueCols(ctx, issue, "content"); err != nil { return fmt.Errorf("UpdateIssueCols: %v", err) } @@ -956,7 +956,7 @@ func newIssue(ctx context.Context, doer *user_model.User, opts NewIssueOptions) } if opts.Issue.MilestoneID > 0 { - if err := updateMilestoneCounters(e, opts.Issue.MilestoneID); err != nil { + if err := updateMilestoneCounters(ctx, opts.Issue.MilestoneID); err != nil { return err } @@ -1970,10 +1970,9 @@ func UpdateIssueDeadline(issue *Issue, deadlineUnix timeutil.TimeStamp, doer *us return err } defer committer.Close() - sess := db.GetEngine(ctx) // Update the deadline - if err = updateIssueCols(sess, &Issue{ID: issue.ID, DeadlineUnix: deadlineUnix}, "deadline_unix"); err != nil { + if err = updateIssueCols(ctx, &Issue{ID: issue.ID, DeadlineUnix: deadlineUnix}, "deadline_unix"); err != nil { return err } @@ -2059,21 +2058,11 @@ func (issue *Issue) BlockingDependencies() ([]*DependencyInfo, error) { return issue.getBlockingDependencies(db.GetEngine(db.DefaultContext)) } -func (issue *Issue) updateClosedNum(e db.Engine) (err error) { +func (issue *Issue) updateClosedNum(ctx context.Context) (err error) { if issue.IsPull { - _, err = e.Exec("UPDATE `repository` SET num_closed_pulls=(SELECT count(*) FROM issue WHERE repo_id=? AND is_pull=? AND is_closed=?) WHERE id=?", - issue.RepoID, - true, - true, - issue.RepoID, - ) + err = repoStatsCorrectNumClosed(ctx, issue.RepoID, true, "num_closed_pulls") } else { - _, err = e.Exec("UPDATE `repository` SET num_closed_issues=(SELECT count(*) FROM issue WHERE repo_id=? AND is_pull=? AND is_closed=?) WHERE id=?", - issue.RepoID, - false, - true, - issue.RepoID, - ) + err = repoStatsCorrectNumClosed(ctx, issue.RepoID, false, "num_closed_issues") } return } diff --git a/models/issue_comment.go b/models/issue_comment.go index 03a2a630d..99c38bcf5 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -856,12 +856,12 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment } } case CommentTypeReopen, CommentTypeClose: - if err = opts.Issue.updateClosedNum(e); err != nil { + if err = opts.Issue.updateClosedNum(ctx); err != nil { return err } } // update the issue's updated_unix column - return updateIssueCols(e, opts.Issue, "updated_unix") + return updateIssueCols(ctx, opts.Issue, "updated_unix") } func createDeadlineComment(ctx context.Context, doer *user_model.User, issue *Issue, newDeadlineUnix timeutil.TimeStamp) (*Comment, error) { diff --git a/models/issue_lock.go b/models/issue_lock.go index 66a932225..20e94c7b2 100644 --- a/models/issue_lock.go +++ b/models/issue_lock.go @@ -46,7 +46,7 @@ func updateIssueLock(opts *IssueLockOptions, lock bool) error { } defer committer.Close() - if err := updateIssueCols(db.GetEngine(ctx), opts.Issue, "is_locked"); err != nil { + if err := updateIssueCols(ctx, opts.Issue, "is_locked"); err != nil { return err } diff --git a/models/issue_milestone.go b/models/issue_milestone.go index c79a99eb6..7f2fd9a1f 100644 --- a/models/issue_milestone.go +++ b/models/issue_milestone.go @@ -158,19 +158,17 @@ func UpdateMilestone(m *Milestone, oldIsClosed bool) error { } defer committer.Close() - sess := db.GetEngine(ctx) - if m.IsClosed && !oldIsClosed { m.ClosedDateUnix = timeutil.TimeStampNow() } - if err := updateMilestone(sess, m); err != nil { + if err := updateMilestone(ctx, m); err != nil { return err } // if IsClosed changed, update milestone numbers of repository if oldIsClosed != m.IsClosed { - if err := updateRepoMilestoneNum(sess, m.RepoID); err != nil { + if err := updateRepoMilestoneNum(ctx, m.RepoID); err != nil { return err } } @@ -178,17 +176,18 @@ func UpdateMilestone(m *Milestone, oldIsClosed bool) error { return committer.Commit() } -func updateMilestone(e db.Engine, m *Milestone) error { +func updateMilestone(ctx context.Context, m *Milestone) error { m.Name = strings.TrimSpace(m.Name) - _, err := e.ID(m.ID).AllCols().Update(m) + _, err := db.GetEngine(ctx).ID(m.ID).AllCols().Update(m) if err != nil { return err } - return updateMilestoneCounters(e, m.ID) + return updateMilestoneCounters(ctx, m.ID) } // updateMilestoneCounters calculates NumIssues, NumClosesIssues and Completeness -func updateMilestoneCounters(e db.Engine, id int64) error { +func updateMilestoneCounters(ctx context.Context, id int64) error { + e := db.GetEngine(ctx) _, err := e.ID(id). SetExpr("num_issues", builder.Select("count(*)").From("issue").Where( builder.Eq{"milestone_id": id}, @@ -217,21 +216,19 @@ func ChangeMilestoneStatusByRepoIDAndID(repoID, milestoneID int64, isClosed bool } defer committer.Close() - sess := db.GetEngine(ctx) - m := &Milestone{ ID: milestoneID, RepoID: repoID, } - has, err := sess.ID(milestoneID).Where("repo_id = ?", repoID).Get(m) + has, err := db.GetEngine(ctx).ID(milestoneID).Where("repo_id = ?", repoID).Get(m) if err != nil { return err } else if !has { return ErrMilestoneNotExist{ID: milestoneID, RepoID: repoID} } - if err := changeMilestoneStatus(sess, m, isClosed); err != nil { + if err := changeMilestoneStatus(ctx, m, isClosed); err != nil { return err } @@ -246,43 +243,42 @@ func ChangeMilestoneStatus(m *Milestone, isClosed bool) (err error) { } defer committer.Close() - if err := changeMilestoneStatus(db.GetEngine(ctx), m, isClosed); err != nil { + if err := changeMilestoneStatus(ctx, m, isClosed); err != nil { return err } return committer.Commit() } -func changeMilestoneStatus(e db.Engine, m *Milestone, isClosed bool) error { +func changeMilestoneStatus(ctx context.Context, m *Milestone, isClosed bool) error { m.IsClosed = isClosed if isClosed { m.ClosedDateUnix = timeutil.TimeStampNow() } - count, err := e.ID(m.ID).Where("repo_id = ? AND is_closed = ?", m.RepoID, !isClosed).Cols("is_closed", "closed_date_unix").Update(m) + count, err := db.GetEngine(ctx).ID(m.ID).Where("repo_id = ? AND is_closed = ?", m.RepoID, !isClosed).Cols("is_closed", "closed_date_unix").Update(m) if err != nil { return err } if count < 1 { return nil } - return updateRepoMilestoneNum(e, m.RepoID) + return updateRepoMilestoneNum(ctx, m.RepoID) } func changeMilestoneAssign(ctx context.Context, doer *user_model.User, issue *Issue, oldMilestoneID int64) error { - e := db.GetEngine(ctx) - if err := updateIssueCols(e, issue, "milestone_id"); err != nil { + if err := updateIssueCols(ctx, issue, "milestone_id"); err != nil { return err } if oldMilestoneID > 0 { - if err := updateMilestoneCounters(e, oldMilestoneID); err != nil { + if err := updateMilestoneCounters(ctx, oldMilestoneID); err != nil { return err } } if issue.MilestoneID > 0 { - if err := updateMilestoneCounters(e, issue.MilestoneID); err != nil { + if err := updateMilestoneCounters(ctx, issue.MilestoneID); err != nil { return err } } @@ -634,8 +630,8 @@ func CountMilestonesByRepoCondAndKw(repoCond builder.Cond, keyword string, isClo return countMap, nil } -func updateRepoMilestoneNum(e db.Engine, repoID int64) error { - _, err := e.Exec("UPDATE `repository` SET num_milestones=(SELECT count(*) FROM milestone WHERE repo_id=?),num_closed_milestones=(SELECT count(*) FROM milestone WHERE repo_id=? AND is_closed=?) WHERE id=?", +func updateRepoMilestoneNum(ctx context.Context, repoID int64) error { + _, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET num_milestones=(SELECT count(*) FROM milestone WHERE repo_id=?),num_closed_milestones=(SELECT count(*) FROM milestone WHERE repo_id=? AND is_closed=?) WHERE id=?", repoID, repoID, true, diff --git a/models/issue_milestone_test.go b/models/issue_milestone_test.go index 9b40144e6..6593f78fa 100644 --- a/models/issue_milestone_test.go +++ b/models/issue_milestone_test.go @@ -228,14 +228,14 @@ func TestUpdateMilestoneCounters(t *testing.T) { issue.ClosedUnix = timeutil.TimeStampNow() _, err := db.GetEngine(db.DefaultContext).ID(issue.ID).Cols("is_closed", "closed_unix").Update(issue) assert.NoError(t, err) - assert.NoError(t, updateMilestoneCounters(db.GetEngine(db.DefaultContext), issue.MilestoneID)) + assert.NoError(t, updateMilestoneCounters(db.DefaultContext, issue.MilestoneID)) unittest.CheckConsistencyFor(t, &Milestone{}) issue.IsClosed = false issue.ClosedUnix = 0 _, err = db.GetEngine(db.DefaultContext).ID(issue.ID).Cols("is_closed", "closed_unix").Update(issue) assert.NoError(t, err) - assert.NoError(t, updateMilestoneCounters(db.GetEngine(db.DefaultContext), issue.MilestoneID)) + assert.NoError(t, updateMilestoneCounters(db.DefaultContext, issue.MilestoneID)) unittest.CheckConsistencyFor(t, &Milestone{}) } diff --git a/models/issue_test.go b/models/issue_test.go index cebf20af9..aee9a5018 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -129,7 +129,7 @@ func TestUpdateIssueCols(t *testing.T) { issue.Content = "This should have no effect" now := time.Now().Unix() - assert.NoError(t, updateIssueCols(db.GetEngine(db.DefaultContext), issue, "name")) + assert.NoError(t, updateIssueCols(db.DefaultContext, issue, "name")) then := time.Now().Unix() updatedIssue := unittest.AssertExistsAndLoadBean(t, &Issue{ID: issue.ID}).(*Issue) diff --git a/models/migrate.go b/models/migrate.go index ce529efc4..4da426887 100644 --- a/models/migrate.go +++ b/models/migrate.go @@ -5,6 +5,8 @@ package models import ( + "context" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/structs" @@ -46,25 +48,28 @@ func InsertIssues(issues ...*Issue) error { defer committer.Close() for _, issue := range issues { - if err := insertIssue(db.GetEngine(ctx), issue); err != nil { + if err := insertIssue(ctx, issue); err != nil { return err } } + err = UpdateRepoStats(ctx, issues[0].RepoID) + if err != nil { + return err + } return committer.Commit() } -func insertIssue(sess db.Engine, issue *Issue) error { +func insertIssue(ctx context.Context, issue *Issue) error { + sess := db.GetEngine(ctx) if _, err := sess.NoAutoTime().Insert(issue); err != nil { return err } issueLabels := make([]IssueLabel, 0, len(issue.Labels)) - labelIDs := make([]int64, 0, len(issue.Labels)) for _, label := range issue.Labels { issueLabels = append(issueLabels, IssueLabel{ IssueID: issue.ID, LabelID: label.ID, }) - labelIDs = append(labelIDs, label.ID) } if len(issueLabels) > 0 { if _, err := sess.Insert(issueLabels); err != nil { @@ -82,54 +87,6 @@ func insertIssue(sess db.Engine, issue *Issue) error { } } - cols := make([]string, 0) - if !issue.IsPull { - sess.ID(issue.RepoID).Incr("num_issues") - cols = append(cols, "num_issues") - if issue.IsClosed { - sess.Incr("num_closed_issues") - cols = append(cols, "num_closed_issues") - } - } else { - sess.ID(issue.RepoID).Incr("num_pulls") - cols = append(cols, "num_pulls") - if issue.IsClosed { - sess.Incr("num_closed_pulls") - cols = append(cols, "num_closed_pulls") - } - } - if _, err := sess.NoAutoTime().Cols(cols...).Update(issue.Repo); err != nil { - return err - } - - cols = []string{"num_issues"} - sess.Incr("num_issues") - if issue.IsClosed { - sess.Incr("num_closed_issues") - cols = append(cols, "num_closed_issues") - } - if _, err := sess.In("id", labelIDs).NoAutoTime().Cols(cols...).Update(new(Label)); err != nil { - return err - } - - if issue.MilestoneID > 0 { - cols = []string{"num_issues"} - sess.Incr("num_issues") - cl := "num_closed_issues" - if issue.IsClosed { - sess.Incr("num_closed_issues") - cols = append(cols, "num_closed_issues") - cl = "(num_closed_issues + 1)" - } - - if _, err := sess.ID(issue.MilestoneID). - SetExpr("completeness", cl+" * 100 / (num_issues + 1)"). - NoAutoTime().Cols(cols...). - Update(new(Milestone)); err != nil { - return err - } - } - return nil } @@ -182,7 +139,7 @@ func InsertPullRequests(prs ...*PullRequest) error { defer committer.Close() sess := db.GetEngine(ctx) for _, pr := range prs { - if err := insertIssue(sess, pr.Issue); err != nil { + if err := insertIssue(ctx, pr.Issue); err != nil { return err } pr.IssueID = pr.Issue.ID @@ -191,6 +148,10 @@ func InsertPullRequests(prs ...*PullRequest) error { } } + err = UpdateRepoStats(ctx, prs[0].Issue.RepoID) + if err != nil { + return err + } return committer.Commit() } diff --git a/models/repo.go b/models/repo.go index fcb2cae3f..83031c508 100644 --- a/models/repo.go +++ b/models/repo.go @@ -961,12 +961,13 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { } type repoChecker struct { - querySQL, correctSQL string - desc string + querySQL func(ctx context.Context) ([]map[string][]byte, error) + correctSQL func(ctx context.Context, id int64) error + desc string } func repoStatsCheck(ctx context.Context, checker *repoChecker) { - results, err := db.GetEngine(db.DefaultContext).Query(checker.querySQL) + results, err := checker.querySQL(ctx) if err != nil { log.Error("Select %s: %v", checker.desc, err) return @@ -975,18 +976,112 @@ func repoStatsCheck(ctx context.Context, checker *repoChecker) { id, _ := strconv.ParseInt(string(result["id"]), 10, 64) select { case <-ctx.Done(): - log.Warn("CheckRepoStats: Cancelled before checking %s for Repo[%d]", checker.desc, id) + log.Warn("CheckRepoStats: Cancelled before checking %s for with id=%d", checker.desc, id) return default: } log.Trace("Updating %s: %d", checker.desc, id) - _, err = db.GetEngine(db.DefaultContext).Exec(checker.correctSQL, id, id) + err = checker.correctSQL(ctx, id) if err != nil { log.Error("Update %s[%d]: %v", checker.desc, id, err) } } } +func StatsCorrectSQL(ctx context.Context, sql string, id int64) error { + _, err := db.GetEngine(ctx).Exec(sql, id, id) + return err +} + +func repoStatsCorrectNumWatches(ctx context.Context, id int64) error { + return StatsCorrectSQL(ctx, "UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=? AND mode<>2) WHERE id=?", id) +} + +func repoStatsCorrectNumStars(ctx context.Context, id int64) error { + return StatsCorrectSQL(ctx, "UPDATE `repository` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE repo_id=?) WHERE id=?", id) +} + +func labelStatsCorrectNumIssues(ctx context.Context, id int64) error { + return StatsCorrectSQL(ctx, "UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=?) WHERE id=?", id) +} + +func labelStatsCorrectNumIssuesRepo(ctx context.Context, id int64) error { + _, err := db.GetEngine(ctx).Exec("UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=id) WHERE repo_id=?", id) + return err +} + +func labelStatsCorrectNumClosedIssues(ctx context.Context, id int64) error { + _, err := db.GetEngine(ctx).Exec("UPDATE `label` SET num_closed_issues=(SELECT COUNT(*) FROM `issue_label`,`issue` WHERE `issue_label`.label_id=`label`.id AND `issue_label`.issue_id=`issue`.id AND `issue`.is_closed=?) WHERE `label`.id=?", true, id) + return err +} + +func labelStatsCorrectNumClosedIssuesRepo(ctx context.Context, id int64) error { + _, err := db.GetEngine(ctx).Exec("UPDATE `label` SET num_closed_issues=(SELECT COUNT(*) FROM `issue_label`,`issue` WHERE `issue_label`.label_id=`label`.id AND `issue_label`.issue_id=`issue`.id AND `issue`.is_closed=?) WHERE `label`.repo_id=?", true, id) + return err +} + +var milestoneStatsQueryNumIssues = "SELECT `milestone`.id FROM `milestone` WHERE `milestone`.num_closed_issues!=(SELECT COUNT(*) FROM `issue` WHERE `issue`.milestone_id=`milestone`.id AND `issue`.is_closed=?) OR `milestone`.num_issues!=(SELECT COUNT(*) FROM `issue` WHERE `issue`.milestone_id=`milestone`.id)" + +func milestoneStatsCorrectNumIssues(ctx context.Context, id int64) error { + return updateMilestoneCounters(ctx, id) +} + +func milestoneStatsCorrectNumIssuesRepo(ctx context.Context, id int64) error { + e := db.GetEngine(ctx) + results, err := e.Query(milestoneStatsQueryNumIssues+" AND `milestone`.repo_id = ?", true, id) + if err != nil { + return err + } + for _, result := range results { + id, _ := strconv.ParseInt(string(result["id"]), 10, 64) + err = milestoneStatsCorrectNumIssues(ctx, id) + if err != nil { + return err + } + } + return nil +} + +func userStatsCorrectNumRepos(ctx context.Context, id int64) error { + return StatsCorrectSQL(ctx, "UPDATE `user` SET num_repos=(SELECT COUNT(*) FROM `repository` WHERE owner_id=?) WHERE id=?", id) +} + +func repoStatsCorrectIssueNumComments(ctx context.Context, id int64) error { + return StatsCorrectSQL(ctx, "UPDATE `issue` SET num_comments=(SELECT COUNT(*) FROM `comment` WHERE issue_id=? AND type=0) WHERE id=?", id) +} + +func repoStatsCorrectNumIssues(ctx context.Context, id int64) error { + return repoStatsCorrectNum(ctx, id, false, "num_issues") +} + +func repoStatsCorrectNumPulls(ctx context.Context, id int64) error { + return repoStatsCorrectNum(ctx, id, true, "num_pulls") +} + +func repoStatsCorrectNum(ctx context.Context, id int64, isPull bool, field string) error { + _, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET "+field+"=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_pull=?) WHERE id=?", id, isPull, id) + return err +} + +func repoStatsCorrectNumClosedIssues(ctx context.Context, id int64) error { + return repoStatsCorrectNumClosed(ctx, id, false, "num_closed_issues") +} + +func repoStatsCorrectNumClosedPulls(ctx context.Context, id int64) error { + return repoStatsCorrectNumClosed(ctx, id, true, "num_closed_pulls") +} + +func repoStatsCorrectNumClosed(ctx context.Context, id int64, isPull bool, field string) error { + _, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET "+field+"=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_closed=? AND is_pull=?) WHERE id=?", id, true, isPull, id) + return err +} + +func statsQuery(args ...interface{}) func(context.Context) ([]map[string][]byte, error) { + return func(ctx context.Context) ([]map[string][]byte, error) { + return db.GetEngine(ctx).Query(args...) + } +} + // CheckRepoStats checks the repository stats func CheckRepoStats(ctx context.Context) error { log.Trace("Doing: CheckRepoStats") @@ -994,32 +1089,56 @@ func CheckRepoStats(ctx context.Context) error { checkers := []*repoChecker{ // Repository.NumWatches { - "SELECT repo.id FROM `repository` repo WHERE repo.num_watches!=(SELECT COUNT(*) FROM `watch` WHERE repo_id=repo.id AND mode<>2)", - "UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=? AND mode<>2) WHERE id=?", + statsQuery("SELECT repo.id FROM `repository` repo WHERE repo.num_watches!=(SELECT COUNT(*) FROM `watch` WHERE repo_id=repo.id AND mode<>2)"), + repoStatsCorrectNumWatches, "repository count 'num_watches'", }, // Repository.NumStars { - "SELECT repo.id FROM `repository` repo WHERE repo.num_stars!=(SELECT COUNT(*) FROM `star` WHERE repo_id=repo.id)", - "UPDATE `repository` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE repo_id=?) WHERE id=?", + statsQuery("SELECT repo.id FROM `repository` repo WHERE repo.num_stars!=(SELECT COUNT(*) FROM `star` WHERE repo_id=repo.id)"), + repoStatsCorrectNumStars, "repository count 'num_stars'", }, + // Repository.NumClosedIssues + { + statsQuery("SELECT repo.id FROM `repository` repo WHERE repo.num_closed_issues!=(SELECT COUNT(*) FROM `issue` WHERE repo_id=repo.id AND is_closed=? AND is_pull=?)", true, false), + repoStatsCorrectNumClosedIssues, + "repository count 'num_closed_issues'", + }, + // Repository.NumClosedPulls + { + statsQuery("SELECT repo.id FROM `repository` repo WHERE repo.num_closed_issues!=(SELECT COUNT(*) FROM `issue` WHERE repo_id=repo.id AND is_closed=? AND is_pull=?)", true, true), + repoStatsCorrectNumClosedPulls, + "repository count 'num_closed_pulls'", + }, // Label.NumIssues { - "SELECT label.id FROM `label` WHERE label.num_issues!=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=label.id)", - "UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=?) WHERE id=?", + statsQuery("SELECT label.id FROM `label` WHERE label.num_issues!=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=label.id)"), + labelStatsCorrectNumIssues, "label count 'num_issues'", }, + // Label.NumClosedIssues + { + statsQuery("SELECT `label`.id FROM `label` WHERE `label`.num_closed_issues!=(SELECT COUNT(*) FROM `issue_label`,`issue` WHERE `issue_label`.label_id=`label`.id AND `issue_label`.issue_id=`issue`.id AND `issue`.is_closed=?)", true), + labelStatsCorrectNumClosedIssues, + "label count 'num_closed_issues'", + }, + // Milestone.Num{,Closed}Issues + { + statsQuery(milestoneStatsQueryNumIssues, true), + milestoneStatsCorrectNumIssues, + "milestone count 'num_closed_issues' and 'num_issues'", + }, // User.NumRepos { - "SELECT `user`.id FROM `user` WHERE `user`.num_repos!=(SELECT COUNT(*) FROM `repository` WHERE owner_id=`user`.id)", - "UPDATE `user` SET num_repos=(SELECT COUNT(*) FROM `repository` WHERE owner_id=?) WHERE id=?", + statsQuery("SELECT `user`.id FROM `user` WHERE `user`.num_repos!=(SELECT COUNT(*) FROM `repository` WHERE owner_id=`user`.id)"), + userStatsCorrectNumRepos, "user count 'num_repos'", }, // Issue.NumComments { - "SELECT `issue`.id FROM `issue` WHERE `issue`.num_comments!=(SELECT COUNT(*) FROM `comment` WHERE issue_id=`issue`.id AND type=0)", - "UPDATE `issue` SET num_comments=(SELECT COUNT(*) FROM `comment` WHERE issue_id=? AND type=0) WHERE id=?", + statsQuery("SELECT `issue`.id FROM `issue` WHERE `issue`.num_comments!=(SELECT COUNT(*) FROM `comment` WHERE issue_id=`issue`.id AND type=0)"), + repoStatsCorrectIssueNumComments, "issue count 'num_comments'", }, } @@ -1033,55 +1152,10 @@ func CheckRepoStats(ctx context.Context) error { } } - // ***** START: Repository.NumClosedIssues ***** - desc := "repository count 'num_closed_issues'" - results, err := db.GetEngine(db.DefaultContext).Query("SELECT repo.id FROM `repository` repo WHERE repo.num_closed_issues!=(SELECT COUNT(*) FROM `issue` WHERE repo_id=repo.id AND is_closed=? AND is_pull=?)", true, false) - if err != nil { - log.Error("Select %s: %v", desc, err) - } else { - for _, result := range results { - id, _ := strconv.ParseInt(string(result["id"]), 10, 64) - select { - case <-ctx.Done(): - log.Warn("CheckRepoStats: Cancelled during %s for repo ID %d", desc, id) - return db.ErrCancelledf("during %s for repo ID %d", desc, id) - default: - } - log.Trace("Updating %s: %d", desc, id) - _, err = db.GetEngine(db.DefaultContext).Exec("UPDATE `repository` SET num_closed_issues=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_closed=? AND is_pull=?) WHERE id=?", id, true, false, id) - if err != nil { - log.Error("Update %s[%d]: %v", desc, id, err) - } - } - } - // ***** END: Repository.NumClosedIssues ***** - - // ***** START: Repository.NumClosedPulls ***** - desc = "repository count 'num_closed_pulls'" - results, err = db.GetEngine(db.DefaultContext).Query("SELECT repo.id FROM `repository` repo WHERE repo.num_closed_pulls!=(SELECT COUNT(*) FROM `issue` WHERE repo_id=repo.id AND is_closed=? AND is_pull=?)", true, true) - if err != nil { - log.Error("Select %s: %v", desc, err) - } else { - for _, result := range results { - id, _ := strconv.ParseInt(string(result["id"]), 10, 64) - select { - case <-ctx.Done(): - log.Warn("CheckRepoStats: Cancelled") - return db.ErrCancelledf("during %s for repo ID %d", desc, id) - default: - } - log.Trace("Updating %s: %d", desc, id) - _, err = db.GetEngine(db.DefaultContext).Exec("UPDATE `repository` SET num_closed_pulls=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_closed=? AND is_pull=?) WHERE id=?", id, true, true, id) - if err != nil { - log.Error("Update %s[%d]: %v", desc, id, err) - } - } - } - // ***** END: Repository.NumClosedPulls ***** - // FIXME: use checker when stop supporting old fork repo format. // ***** START: Repository.NumForks ***** - results, err = db.GetEngine(db.DefaultContext).Query("SELECT repo.id FROM `repository` repo WHERE repo.num_forks!=(SELECT COUNT(*) FROM `repository` WHERE fork_id=repo.id)") + e := db.GetEngine(ctx) + results, err := e.Query("SELECT repo.id FROM `repository` repo WHERE repo.num_forks!=(SELECT COUNT(*) FROM `repository` WHERE fork_id=repo.id)") if err != nil { log.Error("Select repository count 'num_forks': %v", err) } else { @@ -1090,7 +1164,7 @@ func CheckRepoStats(ctx context.Context) error { select { case <-ctx.Done(): log.Warn("CheckRepoStats: Cancelled") - return db.ErrCancelledf("during %s for repo ID %d", desc, id) + return db.ErrCancelledf("during repository count 'num_fork' for repo ID %d", id) default: } log.Trace("Updating repository count 'num_forks': %d", id) @@ -1118,6 +1192,28 @@ func CheckRepoStats(ctx context.Context) error { return nil } +func UpdateRepoStats(ctx context.Context, id int64) error { + var err error + + for _, f := range []func(ctx context.Context, id int64) error{ + repoStatsCorrectNumWatches, + repoStatsCorrectNumStars, + repoStatsCorrectNumIssues, + repoStatsCorrectNumPulls, + repoStatsCorrectNumClosedIssues, + repoStatsCorrectNumClosedPulls, + labelStatsCorrectNumIssuesRepo, + labelStatsCorrectNumClosedIssuesRepo, + milestoneStatsCorrectNumIssuesRepo, + } { + err = f(ctx, id) + if err != nil { + return err + } + } + return nil +} + func updateUserStarNumbers(users []user_model.User) error { ctx, committer, err := db.TxContext() if err != nil { diff --git a/models/repo_test.go b/models/repo_test.go index 45e016a8f..c0790e532 100644 --- a/models/repo_test.go +++ b/models/repo_test.go @@ -17,6 +17,11 @@ import ( "github.com/stretchr/testify/assert" ) +func TestCheckRepoStats(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + assert.NoError(t, CheckRepoStats(db.DefaultContext)) +} + func TestWatchRepo(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) const repoID = 3 diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 79225d75a..035ca2e5a 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -978,6 +978,10 @@ func (g *GiteaLocalUploader) Finish() error { return err } + if err := models.UpdateRepoStats(db.DefaultContext, g.repo.ID); err != nil { + return err + } + g.repo.Status = repo_model.RepositoryReady return repo_model.UpdateRepositoryCols(g.repo, "status") }