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.

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

Make more functions use ctx instead of db.DefaultContext #24068

Merged
merged 6 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
8 changes: 4 additions & 4 deletions routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ func CreateIssue(ctx *context.APIContext) {
form.Labels = make([]int64, 0)
}

if err := issue_service.NewIssue(ctx.Repo.Repository, issue, form.Labels, nil, assigneeIDs); err != nil {
if err := issue_service.NewIssue(ctx, ctx.Repo.Repository, issue, form.Labels, nil, assigneeIDs); err != nil {
if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err)
return
Expand Down Expand Up @@ -752,7 +752,7 @@ func EditIssue(ctx *context.APIContext) {
issue.Content = *form.Body
}
if form.Ref != nil {
err = issue_service.ChangeIssueRef(issue, ctx.Doer, *form.Ref)
err = issue_service.ChangeIssueRef(ctx, issue, ctx.Doer, *form.Ref)
if err != nil {
ctx.Error(http.StatusInternalServerError, "UpdateRef", err)
return
Expand Down Expand Up @@ -790,7 +790,7 @@ func EditIssue(ctx *context.APIContext) {
oneAssignee = *form.Assignee
}

err = issue_service.UpdateAssignees(issue, oneAssignee, form.Assignees, ctx.Doer)
err = issue_service.UpdateAssignees(ctx, issue, oneAssignee, form.Assignees, ctx.Doer)
if err != nil {
ctx.Error(http.StatusInternalServerError, "UpdateAssignees", err)
return
Expand Down Expand Up @@ -887,7 +887,7 @@ func DeleteIssue(ctx *context.APIContext) {
return
}

if err = issue_service.DeleteIssue(ctx.Doer, ctx.Repo.GitRepo, issue); err != nil {
if err = issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil {
ctx.Error(http.StatusInternalServerError, "DeleteIssueByID", err)
return
}
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ func EditPullRequest(ctx *context.APIContext) {
// Send an empty array ([]) to clear all assignees from the Issue.

if ctx.Repo.CanWrite(unit.TypePullRequests) && (form.Assignees != nil || len(form.Assignee) > 0) {
err = issue_service.UpdateAssignees(issue, form.Assignee, form.Assignees, ctx.Doer)
err = issue_service.UpdateAssignees(ctx, issue, form.Assignee, form.Assignees, ctx.Doer)
if err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Sprintf("Assignee does not exist: [name: %s]", err))
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/repo/pull_review.go
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
}

for _, reviewer := range reviewers {
comment, err := issue_service.ReviewRequest(pr.Issue, ctx.Doer, reviewer, isAdd)
comment, err := issue_service.ReviewRequest(ctx, pr.Issue, ctx.Doer, reviewer, isAdd)
if err != nil {
ctx.Error(http.StatusInternalServerError, "ReviewRequest", err)
return
Expand Down Expand Up @@ -750,7 +750,7 @@ func apiReviewRequest(ctx *context.APIContext, opts api.PullReviewRequestOptions
}

for _, teamReviewer := range teamReviewers {
comment, err := issue_service.TeamReviewRequest(pr.Issue, ctx.Doer, teamReviewer, isAdd)
comment, err := issue_service.TeamReviewRequest(ctx, pr.Issue, ctx.Doer, teamReviewer, isAdd)
if err != nil {
ctx.ServerError("TeamReviewRequest", err)
return
Expand Down
16 changes: 8 additions & 8 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ func DeleteIssue(ctx *context.Context) {
return
}

if err := issue_service.DeleteIssue(ctx.Doer, ctx.Repo.GitRepo, issue); err != nil {
if err := issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil {
ctx.ServerError("DeleteIssueByID", err)
return
}
Expand Down Expand Up @@ -1132,7 +1132,7 @@ func NewIssuePost(ctx *context.Context) {
Ref: form.Ref,
}

if err := issue_service.NewIssue(repo, issue, labelIDs, attachments, assigneeIDs); err != nil {
if err := issue_service.NewIssue(ctx, repo, issue, labelIDs, attachments, assigneeIDs); err != nil {
if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())
return
Expand Down Expand Up @@ -2013,7 +2013,7 @@ func UpdateIssueTitle(ctx *context.Context) {
return
}

if err := issue_service.ChangeTitle(issue, ctx.Doer, title); err != nil {
if err := issue_service.ChangeTitle(ctx, issue, ctx.Doer, title); err != nil {
ctx.ServerError("ChangeTitle", err)
return
}
Expand All @@ -2037,7 +2037,7 @@ func UpdateIssueRef(ctx *context.Context) {

ref := ctx.FormTrim("ref")

if err := issue_service.ChangeIssueRef(issue, ctx.Doer, ref); err != nil {
if err := issue_service.ChangeIssueRef(ctx, issue, ctx.Doer, ref); err != nil {
ctx.ServerError("ChangeRef", err)
return
}
Expand Down Expand Up @@ -2161,7 +2161,7 @@ func UpdateIssueAssignee(ctx *context.Context) {
for _, issue := range issues {
switch action {
case "clear":
if err := issue_service.DeleteNotPassedAssignee(issue, ctx.Doer, []*user_model.User{}); err != nil {
if err := issue_service.DeleteNotPassedAssignee(ctx, issue, ctx.Doer, []*user_model.User{}); err != nil {
ctx.ServerError("ClearAssignees", err)
return
}
Expand All @@ -2182,7 +2182,7 @@ func UpdateIssueAssignee(ctx *context.Context) {
return
}

_, _, err = issue_service.ToggleAssignee(issue, ctx.Doer, assigneeID)
_, _, err = issue_service.ToggleAssignee(ctx, issue, ctx.Doer, assigneeID)
if err != nil {
ctx.ServerError("ToggleAssignee", err)
return
Expand Down Expand Up @@ -2269,7 +2269,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
return
}

_, err = issue_service.TeamReviewRequest(issue, ctx.Doer, team, action == "attach")
_, err = issue_service.TeamReviewRequest(ctx, issue, ctx.Doer, team, action == "attach")
if err != nil {
ctx.ServerError("TeamReviewRequest", err)
return
Expand Down Expand Up @@ -2307,7 +2307,7 @@ func UpdatePullReviewRequest(ctx *context.Context) {
return
}

_, err = issue_service.ReviewRequest(issue, ctx.Doer, reviewer, action == "attach")
_, err = issue_service.ReviewRequest(ctx, issue, ctx.Doer, reviewer, action == "attach")
if err != nil {
ctx.ServerError("ReviewRequest", err)
return
Expand Down
23 changes: 11 additions & 12 deletions services/issue/assignee.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package issue
import (
"context"

"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/perm"
Expand All @@ -18,7 +17,7 @@ import (
)

// DeleteNotPassedAssignee deletes all assignees who aren't passed via the "assignees" array
func DeleteNotPassedAssignee(issue *issues_model.Issue, doer *user_model.User, assignees []*user_model.User) (err error) {
func DeleteNotPassedAssignee(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assignees []*user_model.User) (err error) {
var found bool
oriAssignes := make([]*user_model.User, len(issue.Assignees))
_ = copy(oriAssignes, issue.Assignees)
Expand All @@ -34,7 +33,7 @@ func DeleteNotPassedAssignee(issue *issues_model.Issue, doer *user_model.User, a

if !found {
// This function also does comments and hooks, which is why we call it separately instead of directly removing the assignees here
if _, _, err := ToggleAssignee(issue, doer, assignee.ID); err != nil {
if _, _, err := ToggleAssignee(ctx, issue, doer, assignee.ID); err != nil {
return err
}
}
Expand All @@ -44,25 +43,25 @@ func DeleteNotPassedAssignee(issue *issues_model.Issue, doer *user_model.User, a
}

// ToggleAssignee changes a user between assigned and not assigned for this issue, and make issue comment for it.
func ToggleAssignee(issue *issues_model.Issue, doer *user_model.User, assigneeID int64) (removed bool, comment *issues_model.Comment, err error) {
func ToggleAssignee(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assigneeID int64) (removed bool, comment *issues_model.Comment, err error) {
removed, comment, err = issues_model.ToggleIssueAssignee(issue, doer, assigneeID)
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return
}

assignee, err1 := user_model.GetUserByID(db.DefaultContext, assigneeID)
assignee, err1 := user_model.GetUserByID(ctx, assigneeID)
if err1 != nil {
err = err1
return
}

notification.NotifyIssueChangeAssignee(db.DefaultContext, doer, issue, assignee, removed, comment)
notification.NotifyIssueChangeAssignee(ctx, doer, issue, assignee, removed, comment)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notify could be failed, so it should not has the same ctx.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if Notify fails, how does it affect the ctx?

And I can see other code also does so (although I do not think it is ideal, either)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm… What can we do about it?
A separate notification context?
(But not in this PR)


return removed, comment, err
}

// ReviewRequest add or remove a review request from a user for this PR, and make comment for it.
func ReviewRequest(issue *issues_model.Issue, doer, reviewer *user_model.User, isAdd bool) (comment *issues_model.Comment, err error) {
func ReviewRequest(ctx context.Context, issue *issues_model.Issue, doer, reviewer *user_model.User, isAdd bool) (comment *issues_model.Comment, err error) {
if isAdd {
comment, err = issues_model.AddReviewRequest(issue, reviewer, doer)
} else {
Expand All @@ -74,7 +73,7 @@ func ReviewRequest(issue *issues_model.Issue, doer, reviewer *user_model.User, i
}

if comment != nil {
notification.NotifyPullReviewRequest(db.DefaultContext, doer, issue, reviewer, isAdd, comment)
notification.NotifyPullReviewRequest(ctx, doer, issue, reviewer, isAdd, comment)
}

return comment, err
Expand Down Expand Up @@ -229,7 +228,7 @@ func IsValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team,
}

// TeamReviewRequest add or remove a review request from a team for this PR, and make comment for it.
func TeamReviewRequest(issue *issues_model.Issue, doer *user_model.User, reviewer *organization.Team, isAdd bool) (comment *issues_model.Comment, err error) {
func TeamReviewRequest(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, reviewer *organization.Team, isAdd bool) (comment *issues_model.Comment, err error) {
if isAdd {
comment, err = issues_model.AddTeamReviewRequest(issue, reviewer, doer)
} else {
Expand All @@ -245,11 +244,11 @@ func TeamReviewRequest(issue *issues_model.Issue, doer *user_model.User, reviewe
}

// notify all user in this team
if err = comment.LoadIssue(db.DefaultContext); err != nil {
if err = comment.LoadIssue(ctx); err != nil {
return
}

members, err := organization.GetTeamMembers(db.DefaultContext, &organization.SearchMembersOptions{
members, err := organization.GetTeamMembers(ctx, &organization.SearchMembersOptions{
TeamID: reviewer.ID,
})
if err != nil {
Expand All @@ -261,7 +260,7 @@ func TeamReviewRequest(issue *issues_model.Issue, doer *user_model.User, reviewe
continue
}
comment.AssigneeID = member.ID
notification.NotifyPullReviewRequest(db.DefaultContext, doer, issue, member, isAdd, comment)
notification.NotifyPullReviewRequest(ctx, doer, issue, member, isAdd, comment)
}

return comment, err
Expand Down
2 changes: 1 addition & 1 deletion services/issue/assignee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestDeleteNotPassedAssignee(t *testing.T) {
assert.True(t, isAssigned)

// Clean everyone
err = DeleteNotPassedAssignee(issue, user1, []*user_model.User{})
err = DeleteNotPassedAssignee(db.DefaultContext, issue, user1, []*user_model.User{})
assert.NoError(t, err)
assert.EqualValues(t, 0, len(issue.Assignees))

Expand Down
8 changes: 4 additions & 4 deletions services/issue/comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import (
)

// CreateComment creates comment of issue or commit.
func CreateComment(opts *issues_model.CreateCommentOptions) (comment *issues_model.Comment, err error) {
ctx, committer, err := db.TxContext(db.DefaultContext)
func CreateComment(ctx context.Context, opts *issues_model.CreateCommentOptions) (comment *issues_model.Comment, err error) {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -53,7 +53,7 @@ func CreateRefComment(doer *user_model.User, repo *repo_model.Repository, issue
return nil
}

_, err = CreateComment(&issues_model.CreateCommentOptions{
_, err = CreateComment(db.DefaultContext, &issues_model.CreateCommentOptions{
delvh marked this conversation as resolved.
Show resolved Hide resolved
Type: issues_model.CommentTypeCommitRef,
Doer: doer,
Repo: repo,
Expand All @@ -66,7 +66,7 @@ func CreateRefComment(doer *user_model.User, repo *repo_model.Repository, issue

// CreateIssueComment creates a plain issue comment.
func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content string, attachments []string) (*issues_model.Comment, error) {
comment, err := CreateComment(&issues_model.CreateCommentOptions{
comment, err := CreateComment(ctx, &issues_model.CreateCommentOptions{
Type: issues_model.CommentTypeComment,
Doer: doer,
Repo: repo,
Expand Down
Loading