From 34288d4a9bf91d4a2c04f74c4ad08ae9b98620d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Dachary?= Date: Tue, 11 Jan 2022 21:38:09 +0100 Subject: [PATCH 1/5] fix CheckRepoStats and reuse it during migration 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 --- models/issue.go | 14 +- models/migrate.go | 52 +------ models/repo.go | 209 ++++++++++++++++++-------- services/migrations/gitea_uploader.go | 4 + 4 files changed, 156 insertions(+), 123 deletions(-) diff --git a/models/issue.go b/models/issue.go index 108d9b217afd8..d2348d919d2d2 100644 --- a/models/issue.go +++ b/models/issue.go @@ -2061,19 +2061,9 @@ func (issue *Issue) BlockingDependencies() ([]*DependencyInfo, error) { func (issue *Issue) updateClosedNum(e db.Engine) (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(e, 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(e, issue.RepoID, false, "num_closed_issues") } return } diff --git a/models/migrate.go b/models/migrate.go index ce529efc4bd55..44ea6986c07df 100644 --- a/models/migrate.go +++ b/models/migrate.go @@ -58,13 +58,11 @@ func insertIssue(sess db.Engine, issue *Issue) error { 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,55 +80,7 @@ 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 + return UpdateRepoStats(sess, issue.RepoID) } // InsertIssueComments inserts many comments of issues. diff --git a/models/repo.go b/models/repo.go index fcb2cae3f9a4f..202df4b4b2de0 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 string + correctSQL func(e db.Engine, id int64) error + desc string } -func repoStatsCheck(ctx context.Context, checker *repoChecker) { - results, err := db.GetEngine(db.DefaultContext).Query(checker.querySQL) +func repoStatsCheck(ctx context.Context, e db.Engine, checker *repoChecker) { + results, err := e.Query(checker.querySQL) if err != nil { log.Error("Select %s: %v", checker.desc, err) return @@ -975,18 +976,105 @@ 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(e, id) if err != nil { log.Error("Update %s[%d]: %v", checker.desc, id, err) } } } +func StatsCorrectSQL(e db.Engine, sql string, id int64) error { + _, err := e.Exec(sql, id, id) + return err +} + +func repoStatsCorrectNumWatches(e db.Engine, id int64) error { + return StatsCorrectSQL(e, "UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=? AND mode<>2) WHERE id=?", id) +} + +func repoStatsCorrectNumStars(e db.Engine, id int64) error { + return StatsCorrectSQL(e, "UPDATE `repository` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE repo_id=?) WHERE id=?", id) +} + +func labelStatsCorrectNumIssues(e db.Engine, id int64) error { + return StatsCorrectSQL(e, "UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=?) WHERE id=?", id) +} + +func labelStatsCorrectNumIssuesRepo(e db.Engine, id int64) error { + _, err := e.Exec("UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=id) WHERE repo_id=?", id) + return err +} + +func labelStatsCorrectNumClosedIssues(e db.Engine, id int64) error { + _, err := e.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=TRUE) WHERE `label`.id=?", id) + return err +} + +func labelStatsCorrectNumClosedIssuesRepo(e db.Engine, id int64) error { + _, err := e.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=TRUE) WHERE `label`.repo_id=?", 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=TRUE) OR `milestone`.num_issues!=(SELECT COUNT(*) FROM `issue` WHERE `issue`.milestone_id=`milestone`.id)" + +func milestoneStatsCorrectNumIssues(e db.Engine, id int64) error { + return updateMilestoneCounters(e, id) +} + +func milestoneStatsCorrectNumIssuesRepo(e db.Engine, id int64) error { + results, err := e.Query(milestoneStatsQueryNumIssues+" AND `milestone`.repo_id = ?", id) + if err != nil { + return err + } + for _, result := range results { + id, _ := strconv.ParseInt(string(result["id"]), 10, 64) + err = milestoneStatsCorrectNumIssues(e, id) + if err != nil { + return err + } + } + return nil +} + +func userStatsCorrectNumRepos(e db.Engine, id int64) error { + return StatsCorrectSQL(e, "UPDATE `user` SET num_repos=(SELECT COUNT(*) FROM `repository` WHERE owner_id=?) WHERE id=?", id) +} + +func repoStatsCorrectIssueNumComments(e db.Engine, id int64) error { + return StatsCorrectSQL(e, "UPDATE `issue` SET num_comments=(SELECT COUNT(*) FROM `comment` WHERE issue_id=? AND type=0) WHERE id=?", id) +} + +func repoStatsCorrectNumIssues(e db.Engine, id int64) error { + return repoStatsCorrectNum(e, id, false, "num_issues") +} + +func repoStatsCorrectNumPulls(e db.Engine, id int64) error { + return repoStatsCorrectNum(e, id, true, "num_pulls") +} + +func repoStatsCorrectNum(e db.Engine, id int64, isPull bool, field string) error { + _, err := e.Exec("UPDATE `repository` SET "+field+"=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_pull=?) WHERE id=?", id, isPull, id) + return err +} + +func repoStatsCorrectNumClosedIssues(e db.Engine, id int64) error { + return repoStatsCorrectNumClosed(e, id, false, "num_closed_issues") +} + +func repoStatsCorrectNumClosedPulls(e db.Engine, id int64) error { + return repoStatsCorrectNumClosed(e, id, true, "num_closed_pulls") +} + +func repoStatsCorrectNumClosed(e db.Engine, id int64, isPull bool, field string) error { + _, err := e.Exec("UPDATE `repository` SET "+field+"=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_closed=TRUE AND is_pull=?) WHERE id=?", id, isPull, id) + return err +} + // CheckRepoStats checks the repository stats func CheckRepoStats(ctx context.Context) error { log.Trace("Doing: CheckRepoStats") @@ -995,93 +1083,72 @@ func CheckRepoStats(ctx context.Context) error { // 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=?", + 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=?", + repoStatsCorrectNumStars, "repository count 'num_stars'", }, + // Repository.NumClosedIssues + { + "SELECT repo.id FROM `repository` repo WHERE repo.num_closed_issues!=(SELECT COUNT(*) FROM `issue` WHERE repo_id=repo.id AND is_closed=TRUE AND is_pull=FALSE)", + repoStatsCorrectNumClosedIssues, + "repository count 'num_closed_issues'", + }, + // Repository.NumClosedPulls + { + "SELECT repo.id FROM `repository` repo WHERE repo.num_closed_issues!=(SELECT COUNT(*) FROM `issue` WHERE repo_id=repo.id AND is_closed=TRUE AND is_pull=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=?", + labelStatsCorrectNumIssues, "label count 'num_issues'", }, + // Label.NumClosedIssues + { + "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 + { + milestoneStatsQueryNumIssues, + 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=?", + 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=?", + repoStatsCorrectIssueNumComments, "issue count 'num_comments'", }, } + e := db.GetEngine(db.DefaultContext) for _, checker := range checkers { select { case <-ctx.Done(): log.Warn("CheckRepoStats: Cancelled before %s", checker.desc) return db.ErrCancelledf("before checking %s", checker.desc) default: - repoStatsCheck(ctx, checker) - } - } - - // ***** 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) - } + repoStatsCheck(ctx, e, checker) } } - // ***** 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)") + 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 +1157,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 +1185,28 @@ func CheckRepoStats(ctx context.Context) error { return nil } +func UpdateRepoStats(e db.Engine, id int64) error { + var err error + + for _, f := range []func(e db.Engine, id int64) error{ + repoStatsCorrectNumWatches, + repoStatsCorrectNumStars, + repoStatsCorrectNumIssues, + repoStatsCorrectNumPulls, + repoStatsCorrectNumClosedIssues, + repoStatsCorrectNumClosedPulls, + labelStatsCorrectNumIssuesRepo, + labelStatsCorrectNumClosedIssuesRepo, + milestoneStatsCorrectNumIssuesRepo, + } { + err = f(e, 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/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 79225d75a0588..25c3b09ab06b3 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.GetEngine(db.DefaultContext), g.repo.ID); err != nil { + return err + } + g.repo.Status = repo_model.RepositoryReady return repo_model.UpdateRepositoryCols(g.repo, "status") } From 26b2ff7528ad07fab4aedf6309027d967ce18841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Dachary?= Date: Fri, 14 Jan 2022 13:29:38 +0100 Subject: [PATCH 2/5] use context.Context as function arguments instead of db.Engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Loïc Dachary --- models/issue.go | 27 ++++----- 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 | 11 ++-- models/repo.go | 87 ++++++++++++++------------- services/migrations/gitea_uploader.go | 2 +- 9 files changed, 89 insertions(+), 90 deletions(-) diff --git a/models/issue.go b/models/issue.go index d2348d919d2d2..cb5791be9e0c4 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,11 +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 = repoStatsCorrectNumClosed(e, issue.RepoID, true, "num_closed_pulls") + err = repoStatsCorrectNumClosed(ctx, issue.RepoID, true, "num_closed_pulls") } else { - err = repoStatsCorrectNumClosed(e, issue.RepoID, false, "num_closed_issues") + err = repoStatsCorrectNumClosed(ctx, issue.RepoID, false, "num_closed_issues") } return } diff --git a/models/issue_comment.go b/models/issue_comment.go index 03a2a630de859..99c38bcf59cf2 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 66a932225ad68..20e94c7b21765 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 c79a99eb6f9e1..7f2fd9a1f3b60 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 9b40144e65014..6593f78fa171b 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 cebf20af9bbb7..aee9a50184ac6 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 44ea6986c07df..4e1337a39fe5f 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,14 +48,15 @@ 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 } } 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 } @@ -80,7 +83,7 @@ func insertIssue(sess db.Engine, issue *Issue) error { } } - return UpdateRepoStats(sess, issue.RepoID) + return UpdateRepoStats(ctx, issue.RepoID) } // InsertIssueComments inserts many comments of issues. @@ -132,7 +135,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 diff --git a/models/repo.go b/models/repo.go index 202df4b4b2de0..9158f8943f914 100644 --- a/models/repo.go +++ b/models/repo.go @@ -962,12 +962,12 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { type repoChecker struct { querySQL string - correctSQL func(e db.Engine, id int64) error + correctSQL func(ctx context.Context, id int64) error desc string } -func repoStatsCheck(ctx context.Context, e db.Engine, checker *repoChecker) { - results, err := e.Query(checker.querySQL) +func repoStatsCheck(ctx context.Context, checker *repoChecker) { + results, err := db.GetEngine(ctx).Query(checker.querySQL) if err != nil { log.Error("Select %s: %v", checker.desc, err) return @@ -981,59 +981,60 @@ func repoStatsCheck(ctx context.Context, e db.Engine, checker *repoChecker) { default: } log.Trace("Updating %s: %d", checker.desc, id) - err = checker.correctSQL(e, id) + err = checker.correctSQL(ctx, id) if err != nil { log.Error("Update %s[%d]: %v", checker.desc, id, err) } } } -func StatsCorrectSQL(e db.Engine, sql string, id int64) error { - _, err := e.Exec(sql, id, id) +func StatsCorrectSQL(ctx context.Context, sql string, id int64) error { + _, err := db.GetEngine(ctx).Exec(sql, id, id) return err } -func repoStatsCorrectNumWatches(e db.Engine, id int64) error { - return StatsCorrectSQL(e, "UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=? AND mode<>2) WHERE id=?", id) +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(e db.Engine, id int64) error { - return StatsCorrectSQL(e, "UPDATE `repository` SET num_stars=(SELECT COUNT(*) FROM `star` WHERE repo_id=?) 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(e db.Engine, id int64) error { - return StatsCorrectSQL(e, "UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_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(e db.Engine, id int64) error { - _, err := e.Exec("UPDATE `label` SET num_issues=(SELECT COUNT(*) FROM `issue_label` WHERE label_id=id) WHERE repo_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(e db.Engine, id int64) error { - _, err := e.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=TRUE) WHERE `label`.id=?", id) +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=TRUE) WHERE `label`.id=?", id) return err } -func labelStatsCorrectNumClosedIssuesRepo(e db.Engine, id int64) error { - _, err := e.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=TRUE) WHERE `label`.repo_id=?", id) +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=TRUE) WHERE `label`.repo_id=?", 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=TRUE) OR `milestone`.num_issues!=(SELECT COUNT(*) FROM `issue` WHERE `issue`.milestone_id=`milestone`.id)" -func milestoneStatsCorrectNumIssues(e db.Engine, id int64) error { - return updateMilestoneCounters(e, id) +func milestoneStatsCorrectNumIssues(ctx context.Context, id int64) error { + return updateMilestoneCounters(ctx, id) } -func milestoneStatsCorrectNumIssuesRepo(e db.Engine, id int64) error { +func milestoneStatsCorrectNumIssuesRepo(ctx context.Context, id int64) error { + e := db.GetEngine(ctx) results, err := e.Query(milestoneStatsQueryNumIssues+" AND `milestone`.repo_id = ?", id) if err != nil { return err } for _, result := range results { id, _ := strconv.ParseInt(string(result["id"]), 10, 64) - err = milestoneStatsCorrectNumIssues(e, id) + err = milestoneStatsCorrectNumIssues(ctx, id) if err != nil { return err } @@ -1041,37 +1042,37 @@ func milestoneStatsCorrectNumIssuesRepo(e db.Engine, id int64) error { return nil } -func userStatsCorrectNumRepos(e db.Engine, id int64) error { - return StatsCorrectSQL(e, "UPDATE `user` SET num_repos=(SELECT COUNT(*) FROM `repository` WHERE owner_id=?) WHERE id=?", id) +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(e db.Engine, id int64) error { - return StatsCorrectSQL(e, "UPDATE `issue` SET num_comments=(SELECT COUNT(*) FROM `comment` WHERE issue_id=? AND type=0) 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(e db.Engine, id int64) error { - return repoStatsCorrectNum(e, id, false, "num_issues") +func repoStatsCorrectNumIssues(ctx context.Context, id int64) error { + return repoStatsCorrectNum(ctx, id, false, "num_issues") } -func repoStatsCorrectNumPulls(e db.Engine, id int64) error { - return repoStatsCorrectNum(e, id, true, "num_pulls") +func repoStatsCorrectNumPulls(ctx context.Context, id int64) error { + return repoStatsCorrectNum(ctx, id, true, "num_pulls") } -func repoStatsCorrectNum(e db.Engine, id int64, isPull bool, field string) error { - _, err := e.Exec("UPDATE `repository` SET "+field+"=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_pull=?) WHERE id=?", id, isPull, id) +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(e db.Engine, id int64) error { - return repoStatsCorrectNumClosed(e, id, false, "num_closed_issues") +func repoStatsCorrectNumClosedIssues(ctx context.Context, id int64) error { + return repoStatsCorrectNumClosed(ctx, id, false, "num_closed_issues") } -func repoStatsCorrectNumClosedPulls(e db.Engine, id int64) error { - return repoStatsCorrectNumClosed(e, id, true, "num_closed_pulls") +func repoStatsCorrectNumClosedPulls(ctx context.Context, id int64) error { + return repoStatsCorrectNumClosed(ctx, id, true, "num_closed_pulls") } -func repoStatsCorrectNumClosed(e db.Engine, id int64, isPull bool, field string) error { - _, err := e.Exec("UPDATE `repository` SET "+field+"=(SELECT COUNT(*) FROM `issue` WHERE repo_id=? AND is_closed=TRUE AND is_pull=?) WHERE id=?", id, isPull, id) +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=TRUE AND is_pull=?) WHERE id=?", id, isPull, id) return err } @@ -1135,19 +1136,19 @@ func CheckRepoStats(ctx context.Context) error { "issue count 'num_comments'", }, } - e := db.GetEngine(db.DefaultContext) for _, checker := range checkers { select { case <-ctx.Done(): log.Warn("CheckRepoStats: Cancelled before %s", checker.desc) return db.ErrCancelledf("before checking %s", checker.desc) default: - repoStatsCheck(ctx, e, checker) + repoStatsCheck(ctx, checker) } } // FIXME: use checker when stop supporting old fork repo format. // ***** START: Repository.NumForks ***** + 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) @@ -1185,10 +1186,10 @@ func CheckRepoStats(ctx context.Context) error { return nil } -func UpdateRepoStats(e db.Engine, id int64) error { +func UpdateRepoStats(ctx context.Context, id int64) error { var err error - for _, f := range []func(e db.Engine, id int64) error{ + for _, f := range []func(ctx context.Context, id int64) error{ repoStatsCorrectNumWatches, repoStatsCorrectNumStars, repoStatsCorrectNumIssues, @@ -1199,7 +1200,7 @@ func UpdateRepoStats(e db.Engine, id int64) error { labelStatsCorrectNumClosedIssuesRepo, milestoneStatsCorrectNumIssuesRepo, } { - err = f(e, id) + err = f(ctx, id) if err != nil { return err } diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 25c3b09ab06b3..035ca2e5a2381 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -978,7 +978,7 @@ func (g *GiteaLocalUploader) Finish() error { return err } - if err := models.UpdateRepoStats(db.GetEngine(db.DefaultContext), g.repo.ID); err != nil { + if err := models.UpdateRepoStats(db.DefaultContext, g.repo.ID); err != nil { return err } From 099c7cf3f2e6ae347f3283997e8ea6966d586f4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Dachary?= Date: Sun, 16 Jan 2022 23:25:37 +0100 Subject: [PATCH 3/5] UpdateRepoStats must be run from InsertIssues not insertIssue --- models/migrate.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/models/migrate.go b/models/migrate.go index 4e1337a39fe5f..1422723c854e4 100644 --- a/models/migrate.go +++ b/models/migrate.go @@ -52,6 +52,10 @@ func InsertIssues(issues ...*Issue) error { return err } } + err = UpdateRepoStats(ctx, issues[0].RepoID) + if err != nil { + return err + } return committer.Commit() } @@ -83,7 +87,7 @@ func insertIssue(ctx context.Context, issue *Issue) error { } } - return UpdateRepoStats(ctx, issue.RepoID) + return nil } // InsertIssueComments inserts many comments of issues. From d606fbab93f95c63e94e7ddf7475d3e3b75e5084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Dachary?= Date: Mon, 17 Jan 2022 10:12:14 +0100 Subject: [PATCH 4/5] TRUE/FALSE are not valid literals for mssql --- models/repo.go | 38 ++++++++++++++++++++++---------------- models/repo_test.go | 5 +++++ 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/models/repo.go b/models/repo.go index 9158f8943f914..83031c508cdce 100644 --- a/models/repo.go +++ b/models/repo.go @@ -961,13 +961,13 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { } type repoChecker struct { - querySQL 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(ctx).Query(checker.querySQL) + results, err := checker.querySQL(ctx) if err != nil { log.Error("Select %s: %v", checker.desc, err) return @@ -1011,16 +1011,16 @@ func labelStatsCorrectNumIssuesRepo(ctx context.Context, id int64) error { } 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=TRUE) WHERE `label`.id=?", id) + _, 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=TRUE) WHERE `label`.repo_id=?", id) + _, 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=TRUE) OR `milestone`.num_issues!=(SELECT COUNT(*) FROM `issue` WHERE `issue`.milestone_id=`milestone`.id)" +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) @@ -1028,7 +1028,7 @@ func milestoneStatsCorrectNumIssues(ctx context.Context, id int64) error { func milestoneStatsCorrectNumIssuesRepo(ctx context.Context, id int64) error { e := db.GetEngine(ctx) - results, err := e.Query(milestoneStatsQueryNumIssues+" AND `milestone`.repo_id = ?", id) + results, err := e.Query(milestoneStatsQueryNumIssues+" AND `milestone`.repo_id = ?", true, id) if err != nil { return err } @@ -1072,10 +1072,16 @@ func repoStatsCorrectNumClosedPulls(ctx context.Context, id int64) error { } 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=TRUE AND is_pull=?) WHERE id=?", id, isPull, id) + _, 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") @@ -1083,55 +1089,55 @@ 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)", + 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)", + 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 { - "SELECT repo.id FROM `repository` repo WHERE repo.num_closed_issues!=(SELECT COUNT(*) FROM `issue` WHERE repo_id=repo.id AND is_closed=TRUE AND is_pull=FALSE)", + 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 { - "SELECT repo.id FROM `repository` repo WHERE repo.num_closed_issues!=(SELECT COUNT(*) FROM `issue` WHERE repo_id=repo.id AND is_closed=TRUE AND is_pull=TRUE)", + 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)", + 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 { - "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)", + 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 { - milestoneStatsQueryNumIssues, + 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)", + 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)", + 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'", }, diff --git a/models/repo_test.go b/models/repo_test.go index 45e016a8fc0c8..c0790e532d751 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 From c19bfa341f2f6c90b4ae91bc76cc979a2eedbd41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Dachary?= Date: Mon, 17 Jan 2022 12:24:08 +0100 Subject: [PATCH 5/5] also update stats when restoring pull requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Loïc Dachary --- models/migrate.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/migrate.go b/models/migrate.go index 1422723c854e4..4da426887bfe8 100644 --- a/models/migrate.go +++ b/models/migrate.go @@ -148,6 +148,10 @@ func InsertPullRequests(prs ...*PullRequest) error { } } + err = UpdateRepoStats(ctx, prs[0].Issue.RepoID) + if err != nil { + return err + } return committer.Commit() }