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

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

Merged
merged 6 commits into from
Apr 14, 2023
Merged

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

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
87eff34
fix
wxiaoguang Apr 12, 2023
d528337
add ctx to ToggleIssueAssignee
wxiaoguang Apr 12, 2023
dda1754
add more context
wxiaoguang Apr 12, 2023
32d31f2
Merge branch 'main' into refactor-ctx
lunny Apr 14, 2023
8528e42
Merge branch 'main' into refactor-ctx
GiteaBot Apr 14, 2023
3a355cb
Merge branch 'main' into refactor-ctx
GiteaBot Apr 14, 2023
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
4 changes: 2 additions & 2 deletions models/issues/assignees.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ func IsUserAssignedToIssue(ctx context.Context, issue *Issue, user *user_model.U
}

// ToggleIssueAssignee changes a user between assigned and not assigned for this issue, and make issue comment for it.
func ToggleIssueAssignee(issue *Issue, doer *user_model.User, assigneeID int64) (removed bool, comment *Comment, err error) {
ctx, committer, err := db.TxContext(db.DefaultContext)
func ToggleIssueAssignee(ctx context.Context, issue *Issue, doer *user_model.User, assigneeID int64) (removed bool, comment *Comment, err error) {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return false, nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions models/issues/assignees_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ func TestUpdateAssignee(t *testing.T) {
// Assign multiple users
user2, err := user_model.GetUserByID(db.DefaultContext, 2)
assert.NoError(t, err)
_, _, err = issues_model.ToggleIssueAssignee(issue, &user_model.User{ID: 1}, user2.ID)
_, _, err = issues_model.ToggleIssueAssignee(db.DefaultContext, issue, &user_model.User{ID: 1}, user2.ID)
assert.NoError(t, err)

user3, err := user_model.GetUserByID(db.DefaultContext, 3)
assert.NoError(t, err)
_, _, err = issues_model.ToggleIssueAssignee(issue, &user_model.User{ID: 1}, user3.ID)
_, _, err = issues_model.ToggleIssueAssignee(db.DefaultContext, issue, &user_model.User{ID: 1}, user3.ID)
assert.NoError(t, err)

user1, err := user_model.GetUserByID(db.DefaultContext, 1) // This user is already assigned (see the definition in fixtures), so running UpdateAssignee should unassign him
assert.NoError(t, err)
_, _, err = issues_model.ToggleIssueAssignee(issue, &user_model.User{ID: 1}, user1.ID)
_, _, err = issues_model.ToggleIssueAssignee(db.DefaultContext, issue, &user_model.User{ID: 1}, user1.ID)
assert.NoError(t, err)

// Check if he got removed
Expand Down
4 changes: 2 additions & 2 deletions models/issues/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,8 +743,8 @@ func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User,
}

// ChangeIssueTitle changes the title of this issue, as the given user.
func ChangeIssueTitle(issue *Issue, doer *user_model.User, oldTitle string) (err error) {
ctx, committer, err := db.TxContext(db.DefaultContext)
func ChangeIssueTitle(ctx context.Context, issue *Issue, doer *user_model.User, oldTitle string) (err error) {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion models/issues/issue_xref_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestXRef_NeuterCrossReferences(t *testing.T) {

d := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
i.Title = "title2, no mentions"
assert.NoError(t, issues_model.ChangeIssueTitle(i, d, title))
assert.NoError(t, issues_model.ChangeIssueTitle(db.DefaultContext, i, d, title))

ref = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{IssueID: itarget.ID, RefIssueID: i.ID, RefCommentID: 0})
assert.Equal(t, issues_model.CommentTypeIssueRef, ref.Type)
Expand Down
8 changes: 4 additions & 4 deletions routers/api/v1/repo/issue.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
25 changes: 12 additions & 13 deletions services/issue/assignee.go
View file
Open in desktop
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) {
removed, comment, err = issues_model.ToggleIssueAssignee(issue, doer, assigneeID)
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(ctx, issue, doer, assigneeID)
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
View file
Open in desktop
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
View file
Open in desktop
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