From efc2ec4cdf656fbdc35461142ada75101ea34960 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 23 Oct 2022 13:20:19 +0800 Subject: [PATCH 1/5] Fix issues count bug --- models/issues/comment.go | 2 +- models/issues/issue.go | 12 ++-------- models/repo.go | 13 ++++------- models/repo/repo.go | 48 ++++++++++++++++++++-------------------- 4 files changed, 31 insertions(+), 44 deletions(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index 9ab6cab7d0812..eeb0d02cd4808 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -886,7 +886,7 @@ func updateCommentInfos(ctx context.Context, opts *CreateCommentOptions, comment } } case CommentTypeReopen, CommentTypeClose: - if err = updateIssueClosedNum(ctx, opts.Issue); err != nil { + if err = repo_model.UpdateRepoIssueNumbers(ctx, opts.Issue.RepoID, opts.Issue.IsPull, true); err != nil { return err } } diff --git a/models/issues/issue.go b/models/issues/issue.go index f77166db111ab..cf3a42e685c7a 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -725,7 +725,8 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use } } - if err := updateIssueClosedNum(ctx, issue); err != nil { + // update repository's issue closed number + if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { return nil, err } @@ -2137,15 +2138,6 @@ func (issue *Issue) BlockingDependencies(ctx context.Context) (issueDeps []*Depe return issueDeps, err } -func updateIssueClosedNum(ctx context.Context, issue *Issue) (err error) { - if issue.IsPull { - err = repo_model.StatsCorrectNumClosed(ctx, issue.RepoID, true, "num_closed_pulls") - } else { - err = repo_model.StatsCorrectNumClosed(ctx, issue.RepoID, false, "num_closed_issues") - } - return err -} - // FindAndUpdateIssueMentions finds users mentioned in the given content string, and saves them in the database. func FindAndUpdateIssueMentions(ctx context.Context, issue *Issue, doer *user_model.User, content string) (mentions []*user_model.User, err error) { rawMentions := references.FindAllMentionsMarkdown(content) diff --git a/models/repo.go b/models/repo.go index 08fbb0abeac2b..db1730f058b11 100644 --- a/models/repo.go +++ b/models/repo.go @@ -404,24 +404,19 @@ func repoStatsCorrectIssueNumComments(ctx context.Context, id int64) error { } func repoStatsCorrectNumIssues(ctx context.Context, id int64) error { - return repoStatsCorrectNum(ctx, id, false, "num_issues") + return repo_model.UpdateRepoIssueNumbers(ctx, id, false, false) } 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 + return repo_model.UpdateRepoIssueNumbers(ctx, id, true, false) } func repoStatsCorrectNumClosedIssues(ctx context.Context, id int64) error { - return repo_model.StatsCorrectNumClosed(ctx, id, false, "num_closed_issues") + return repo_model.UpdateRepoIssueNumbers(ctx, id, false, true) } func repoStatsCorrectNumClosedPulls(ctx context.Context, id int64) error { - return repo_model.StatsCorrectNumClosed(ctx, id, true, "num_closed_pulls") + return repo_model.UpdateRepoIssueNumbers(ctx, id, true, true) } func statsQuery(args ...interface{}) func(context.Context) ([]map[string][]byte, error) { diff --git a/models/repo/repo.go b/models/repo/repo.go index ce698baaefb9a..0e25a248f7f14 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -765,35 +765,35 @@ func CountRepositories(ctx context.Context, opts CountRepositoryOptions) (int64, return count, nil } -// StatsCorrectNumClosed update repository's issue related numbers -func StatsCorrectNumClosed(ctx context.Context, id int64, isPull bool, field string) error { - _, err := db.Exec(ctx, "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 -} - -// UpdateRepoIssueNumbers update repository issue numbers +// UpdateRepoIssueNumbers update repository's issue closed numbers func UpdateRepoIssueNumbers(ctx context.Context, repoID int64, isPull, isClosed bool) error { - e := db.GetEngine(ctx) - if isPull { - if _, err := e.ID(repoID).Decr("num_pulls").Update(new(Repository)); err != nil { - return err - } - if isClosed { - if _, err := e.ID(repoID).Decr("num_closed_pulls").Update(new(Repository)); err != nil { - return err - } + var field string + if isClosed { + field = "num_closed_issues" + if isPull { + field = "num_closed_pulls" } } else { - if _, err := e.ID(repoID).Decr("num_issues").Update(new(Repository)); err != nil { - return err - } - if isClosed { - if _, err := e.ID(repoID).Decr("num_closed_issues").Update(new(Repository)); err != nil { - return err - } + field = "num_issues" + if isPull { + field = "num_pulls" } } - return nil + + var cond builder.Cond = builder.Eq{ + "repo_id": repoID, + "is_pull": isPull, + } + if isClosed { + cond = cond.And(builder.Eq{"is_closed": isClosed}) + } + subQuery := builder.Select("count(*)").From("issue").Where(cond) + + query := builder.Update(builder.Expr(field, subQuery)). + From("repository"). + Where(builder.Eq{"id": repoID}) + _, err := db.Exec(ctx, query) + return err } // CountNullArchivedRepository counts the number of repositories with is_archived is null From 9bd29adaf29765e5a52fa6614a2075c633142407 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 23 Oct 2022 17:22:26 +0800 Subject: [PATCH 2/5] Fix bug --- models/repo/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 0e25a248f7f14..386e573f0a78b 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -789,7 +789,7 @@ func UpdateRepoIssueNumbers(ctx context.Context, repoID int64, isPull, isClosed } subQuery := builder.Select("count(*)").From("issue").Where(cond) - query := builder.Update(builder.Expr(field, subQuery)). + query := builder.Update(builder.Eq{field: subQuery}). From("repository"). Where(builder.Eq{"id": repoID}) _, err := db.Exec(ctx, query) From 3de4da0ff8116f818b22ab99746a7d5706e8128e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 24 Oct 2022 10:12:13 +0800 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: delvh --- models/repo/repo.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 386e573f0a78b..00f08e38dab81 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -765,20 +765,19 @@ func CountRepositories(ctx context.Context, opts CountRepositoryOptions) (int64, return count, nil } -// UpdateRepoIssueNumbers update repository's issue closed numbers +// UpdateRepoIssueNumbers updates one of a repositories amount of (open|closed) (issues|PRs) with the current count func UpdateRepoIssueNumbers(ctx context.Context, repoID int64, isPull, isClosed bool) error { - var field string + field := "num_" if isClosed { - field = "num_closed_issues" - if isPull { - field = "num_closed_pulls" - } + field += "closed_" + } + if isPull { + field += "pulls" } else { - field = "num_issues" - if isPull { - field = "num_pulls" - } + field += "issues" } + } + } else { var cond builder.Cond = builder.Eq{ "repo_id": repoID, From 2fda1135eee1025a119df7ce1f4d773bf9a84040 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 24 Oct 2022 10:16:23 +0800 Subject: [PATCH 4/5] fix build --- models/repo/repo.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 00f08e38dab81..305b18f5483cd 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -765,7 +765,7 @@ func CountRepositories(ctx context.Context, opts CountRepositoryOptions) (int64, return count, nil } -// UpdateRepoIssueNumbers updates one of a repositories amount of (open|closed) (issues|PRs) with the current count +// UpdateRepoIssueNumbers updates one of a repositories amount of (open|closed) (issues|PRs) with the current count func UpdateRepoIssueNumbers(ctx context.Context, repoID int64, isPull, isClosed bool) error { field := "num_" if isClosed { @@ -776,8 +776,6 @@ func UpdateRepoIssueNumbers(ctx context.Context, repoID int64, isPull, isClosed } else { field += "issues" } - } - } else { var cond builder.Cond = builder.Eq{ "repo_id": repoID, From 7433edcf380ec20d9d9d1b3011eace7ebe7bfc22 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Oct 2022 19:21:32 +0800 Subject: [PATCH 5/5] Add some comment --- models/repo/repo.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 305b18f5483cd..01f6c1635e629 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -777,15 +777,13 @@ func UpdateRepoIssueNumbers(ctx context.Context, repoID int64, isPull, isClosed field += "issues" } - var cond builder.Cond = builder.Eq{ + subQuery := builder.Select("count(*)"). + From("issue").Where(builder.Eq{ "repo_id": repoID, "is_pull": isPull, - } - if isClosed { - cond = cond.And(builder.Eq{"is_closed": isClosed}) - } - subQuery := builder.Select("count(*)").From("issue").Where(cond) + }.And(builder.If(isClosed, builder.Eq{"is_closed": isClosed}))) + // builder.Update(cond) will generate SQL like UPDATE ... SET cond query := builder.Update(builder.Eq{field: subQuery}). From("repository"). Where(builder.Eq{"id": repoID})