Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix CheckRepoStats and reuse it during migration #18264

Merged
merged 6 commits into from
Jan 17, 2022
Merged

Fix CheckRepoStats and reuse it during migration #18264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
34288d4
fix CheckRepoStats and reuse it during migration
Jan 11, 2022
26b2ff7
use context.Context as function arguments instead of db.Engine
Jan 14, 2022
099c7cf
UpdateRepoStats must be run from InsertIssues not insertIssue
Jan 16, 2022
d606fba
TRUE/FALSE are not valid literals for mssql
Jan 17, 2022
c19bfa3
also update stats when restoring pull requests
Jan 17, 2022
7eadc53
Merge branch 'main' into forgefriends-mr-34
zeripath Jan 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 13 additions & 24 deletions models/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions models/issue_comment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion models/issue_lock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
40 changes: 18 additions & 22 deletions models/issue_milestone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,37 +158,36 @@ 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
}
}

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},
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}
}
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions models/issue_milestone_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
}

Expand Down
2 changes: 1 addition & 1 deletion models/issue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading