refactor: use named Permission field in Repository struct instead of anonymous embedding (#37441)
The `Repository` struct in `services/context/repo.go` embedded `access_model.Permission` anonymously, causing all permission methods to be promoted directly onto `Repository`. This made it unclear at call sites whether a method belonged to `Repository` itself or to its embedded `Permission`. ### Changes - **`services/context/repo.go`**: Replace anonymous `access_model.Permission` with named field `Permission access_model.Permission` - **49 files** updated to route permission method calls through the named field: ```go // Before ctx.Repo.IsAdmin() ctx.Repo.CanWrite(unit.TypeCode) ctx.Repo.CanReadIssuesOrPulls(isPull) slices.ContainsFunc(unitTypes, ctx.Repo.CanWrite) // After ctx.Repo.Permission.IsAdmin() ctx.Repo.Permission.CanWrite(unit.TypeCode) ctx.Repo.Permission.CanReadIssuesOrPulls(isPull) slices.ContainsFunc(unitTypes, ctx.Repo.Permission.CanWrite) ``` Methods defined directly on `*Repository` (`CanWriteToBranch`, `CanCreateBranch`, etc.) are unchanged. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: Nicolas <bircni@icloud.com>
This commit is contained in:
@@ -80,7 +80,7 @@ func GetBranch(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
br, err := convert.ToBranch(ctx, ctx.Repo.Repository, branchName, c, branchProtection, ctx.Doer, ctx.Repo.IsAdmin())
|
||||
br, err := convert.ToBranch(ctx, ctx.Repo.Repository, branchName, c, branchProtection, ctx.Doer, ctx.Repo.Permission.IsAdmin())
|
||||
if err != nil {
|
||||
ctx.APIErrorInternal(err)
|
||||
return
|
||||
@@ -271,7 +271,7 @@ func CreateBranch(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
br, err := convert.ToBranch(ctx, ctx.Repo.Repository, opt.BranchName, commit, branchProtection, ctx.Doer, ctx.Repo.IsAdmin())
|
||||
br, err := convert.ToBranch(ctx, ctx.Repo.Repository, opt.BranchName, commit, branchProtection, ctx.Doer, ctx.Repo.Permission.IsAdmin())
|
||||
if err != nil {
|
||||
ctx.APIErrorInternal(err)
|
||||
return
|
||||
@@ -366,7 +366,7 @@ func ListBranches(ctx *context.APIContext) {
|
||||
}
|
||||
|
||||
branchProtection := rules.GetFirstMatched(branches[i].Name)
|
||||
apiBranch, err := convert.ToBranch(ctx, ctx.Repo.Repository, branches[i].Name, c, branchProtection, ctx.Doer, ctx.Repo.IsAdmin())
|
||||
apiBranch, err := convert.ToBranch(ctx, ctx.Repo.Repository, branches[i].Name, c, branchProtection, ctx.Doer, ctx.Repo.Permission.IsAdmin())
|
||||
if err != nil {
|
||||
ctx.APIErrorInternal(err)
|
||||
return
|
||||
|
||||
@@ -442,14 +442,14 @@ func ListIssues(ctx *context.APIContext) {
|
||||
isPull = optional.Some(false)
|
||||
}
|
||||
|
||||
if isPull.Has() && !ctx.Repo.CanReadIssuesOrPulls(isPull.Value()) {
|
||||
if isPull.Has() && !ctx.Repo.Permission.CanReadIssuesOrPulls(isPull.Value()) {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
}
|
||||
|
||||
if !isPull.Has() {
|
||||
canReadIssues := ctx.Repo.CanRead(unit.TypeIssues)
|
||||
canReadPulls := ctx.Repo.CanRead(unit.TypePullRequests)
|
||||
canReadIssues := ctx.Repo.Permission.CanRead(unit.TypeIssues)
|
||||
canReadPulls := ctx.Repo.Permission.CanRead(unit.TypePullRequests)
|
||||
if !canReadIssues && !canReadPulls {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
@@ -591,7 +591,7 @@ func GetIssue(ctx *context.APIContext) {
|
||||
}
|
||||
return
|
||||
}
|
||||
if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanReadIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
}
|
||||
@@ -638,7 +638,7 @@ func CreateIssue(ctx *context.APIContext) {
|
||||
|
||||
form := web.GetForm(ctx).(*api.CreateIssueOption)
|
||||
var deadlineUnix timeutil.TimeStamp
|
||||
if form.Deadline != nil && ctx.Repo.CanWrite(unit.TypeIssues) {
|
||||
if form.Deadline != nil && ctx.Repo.Permission.CanWrite(unit.TypeIssues) {
|
||||
deadlineUnix = timeutil.TimeStamp(form.Deadline.Unix())
|
||||
}
|
||||
|
||||
@@ -655,7 +655,7 @@ func CreateIssue(ctx *context.APIContext) {
|
||||
|
||||
assigneeIDs := make([]int64, 0)
|
||||
var err error
|
||||
if ctx.Repo.CanWrite(unit.TypeIssues) {
|
||||
if ctx.Repo.Permission.CanWrite(unit.TypeIssues) {
|
||||
issue.MilestoneID = form.Milestone
|
||||
assigneeIDs, err = issues_model.MakeIDsFromAPIAssigneesToAdd(ctx, form.Assignee, form.Assignees)
|
||||
if err != nil {
|
||||
@@ -775,7 +775,7 @@ func EditIssue(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
issue.Repo = ctx.Repo.Repository
|
||||
canWrite := ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull)
|
||||
canWrite := ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull)
|
||||
|
||||
err = issue.LoadAttributes(ctx)
|
||||
if err != nil {
|
||||
@@ -1020,7 +1020,7 @@ func UpdateIssueDeadline(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIError(http.StatusForbidden, "Not repo writer")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -371,7 +371,7 @@ func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *issues_model.Iss
|
||||
}
|
||||
|
||||
func canUserWriteIssueAttachment(ctx *context.APIContext, issue *issues_model.Issue) bool {
|
||||
canEditIssue := ctx.IsSigned && (ctx.Doer.ID == issue.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() || ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull))
|
||||
canEditIssue := ctx.IsSigned && (ctx.Doer.ID == issue.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin() || ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull))
|
||||
if !canEditIssue {
|
||||
ctx.APIError(http.StatusForbidden, "user should have permission to write issue")
|
||||
return false
|
||||
|
||||
@@ -73,7 +73,7 @@ func ListIssueComments(ctx *context.APIContext) {
|
||||
ctx.APIErrorInternal(err)
|
||||
return
|
||||
}
|
||||
if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanReadIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
}
|
||||
@@ -279,8 +279,8 @@ func ListRepoIssueComments(ctx *context.APIContext) {
|
||||
}
|
||||
|
||||
var isPull optional.Option[bool]
|
||||
canReadIssue := ctx.Repo.CanRead(unit.TypeIssues)
|
||||
canReadPull := ctx.Repo.CanRead(unit.TypePullRequests)
|
||||
canReadIssue := ctx.Repo.Permission.CanRead(unit.TypeIssues)
|
||||
canReadPull := ctx.Repo.Permission.CanRead(unit.TypePullRequests)
|
||||
if canReadIssue && canReadPull {
|
||||
isPull = optional.None[bool]()
|
||||
} else if canReadIssue {
|
||||
@@ -386,12 +386,12 @@ func CreateIssueComment(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanReadIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
}
|
||||
|
||||
if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.Doer.IsAdmin {
|
||||
if issue.IsLocked && !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.Doer.IsAdmin {
|
||||
ctx.APIError(http.StatusForbidden, errors.New(ctx.Locale.TrString("repo.issues.comment_on_locked")))
|
||||
return
|
||||
}
|
||||
@@ -455,7 +455,7 @@ func GetIssueComment(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanReadIssuesOrPulls(comment.Issue.IsPull) {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
}
|
||||
@@ -580,7 +580,7 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption)
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
|
||||
if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.Permission.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
|
||||
ctx.Status(http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
@@ -689,7 +689,7 @@ func deleteIssueComment(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
|
||||
if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.Permission.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
|
||||
ctx.Status(http.StatusForbidden)
|
||||
return
|
||||
} else if !comment.Type.HasContentSupport() {
|
||||
|
||||
@@ -358,7 +358,7 @@ func getIssueCommentSafe(ctx *context.APIContext) *issues_model.Comment {
|
||||
return nil
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanReadIssuesOrPulls(comment.Issue.IsPull) {
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -379,7 +379,7 @@ func getIssueCommentAttachmentSafeWrite(ctx *context.APIContext) *repo_model.Att
|
||||
}
|
||||
|
||||
func canUserWriteIssueCommentAttachment(ctx *context.APIContext, comment *issues_model.Comment) bool {
|
||||
canEditComment := ctx.IsSigned && (ctx.Doer.ID == comment.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin()) && ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)
|
||||
canEditComment := ctx.IsSigned && (ctx.Doer.ID == comment.PosterID || ctx.IsUserRepoAdmin() || ctx.IsUserSiteAdmin()) && ctx.Repo.Permission.CanWriteIssuesOrPulls(comment.Issue.IsPull)
|
||||
if !canEditComment {
|
||||
ctx.APIError(http.StatusForbidden, "user should have permission to edit comment")
|
||||
return false
|
||||
|
||||
@@ -173,7 +173,7 @@ func DeleteIssueLabel(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
ctx.Status(http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
@@ -295,7 +295,7 @@ func ClearIssueLabels(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
ctx.Status(http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
@@ -319,7 +319,7 @@ func prepareForReplaceOrAdd(ctx *context.APIContext, form api.IssueLabelsOption)
|
||||
return nil, nil, err
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIError(http.StatusForbidden, "write permission is required")
|
||||
return nil, nil, errors.New("permission denied")
|
||||
}
|
||||
|
||||
@@ -62,7 +62,7 @@ func LockIssue(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIError(http.StatusForbidden, errors.New("no permission to lock this issue"))
|
||||
return
|
||||
}
|
||||
@@ -129,7 +129,7 @@ func UnlockIssue(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIError(http.StatusForbidden, errors.New("no permission to unlock this issue"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -71,7 +71,7 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanReadIssuesOrPulls(comment.Issue.IsPull) {
|
||||
ctx.APIError(http.StatusForbidden, errors.New("no permission to get reactions"))
|
||||
return
|
||||
}
|
||||
@@ -208,12 +208,12 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanReadIssuesOrPulls(comment.Issue.IsPull) {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
}
|
||||
|
||||
if comment.Issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull) {
|
||||
if comment.Issue.IsLocked && !ctx.Repo.Permission.CanWriteIssuesOrPulls(comment.Issue.IsPull) {
|
||||
ctx.APIError(http.StatusForbidden, errors.New("no permission to change reaction"))
|
||||
return
|
||||
}
|
||||
@@ -304,7 +304,7 @@ func GetIssueReactions(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanReadIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIError(http.StatusForbidden, errors.New("no permission to get reactions"))
|
||||
return
|
||||
}
|
||||
@@ -428,7 +428,7 @@ func changeIssueReaction(ctx *context.APIContext, form api.EditReactionOption, i
|
||||
return
|
||||
}
|
||||
|
||||
if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
if issue.IsLocked && !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
ctx.APIError(http.StatusForbidden, errors.New("no permission to change reaction"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -178,7 +178,7 @@ func prepareIssueForStopwatch(ctx *context.APIContext) *issues_model.Issue {
|
||||
return nil
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
if !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) {
|
||||
ctx.Status(http.StatusForbidden)
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -51,7 +51,7 @@ func MirrorSync(ctx *context.APIContext) {
|
||||
|
||||
repo := ctx.Repo.Repository
|
||||
|
||||
if !ctx.Repo.CanWrite(unit.TypeCode) {
|
||||
if !ctx.Repo.Permission.CanWrite(unit.TypeCode) {
|
||||
ctx.APIError(http.StatusForbidden, "Must have write access")
|
||||
}
|
||||
|
||||
|
||||
@@ -653,7 +653,7 @@ func EditPullRequest(ctx *context.APIContext) {
|
||||
return
|
||||
}
|
||||
|
||||
if !issue.IsPoster(ctx.Doer.ID) && !ctx.Repo.CanWrite(unit.TypePullRequests) {
|
||||
if !issue.IsPoster(ctx.Doer.ID) && !ctx.Repo.Permission.CanWrite(unit.TypePullRequests) {
|
||||
ctx.Status(http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
@@ -715,7 +715,7 @@ func EditPullRequest(ctx *context.APIContext) {
|
||||
// Pass one or more user logins to replace the set of assignees on this Issue.
|
||||
// Send an empty array ([]) to clear all assignees from the Issue.
|
||||
|
||||
if ctx.Repo.CanWrite(unit.TypePullRequests) && (form.Assignees != nil || len(form.Assignee) > 0) {
|
||||
if ctx.Repo.Permission.CanWrite(unit.TypePullRequests) && (form.Assignees != nil || len(form.Assignee) > 0) {
|
||||
err = issue_service.UpdateAssignees(ctx, issue, form.Assignee, form.Assignees, ctx.Doer)
|
||||
if err != nil {
|
||||
if user_model.IsErrUserNotExist(err) {
|
||||
@@ -729,7 +729,7 @@ func EditPullRequest(ctx *context.APIContext) {
|
||||
}
|
||||
}
|
||||
|
||||
if ctx.Repo.CanWrite(unit.TypePullRequests) && form.Milestone != 0 &&
|
||||
if ctx.Repo.Permission.CanWrite(unit.TypePullRequests) && form.Milestone != 0 &&
|
||||
issue.MilestoneID != form.Milestone {
|
||||
oldMilestoneID := issue.MilestoneID
|
||||
issue.MilestoneID = form.Milestone
|
||||
@@ -744,7 +744,7 @@ func EditPullRequest(ctx *context.APIContext) {
|
||||
}
|
||||
}
|
||||
|
||||
if ctx.Repo.CanWrite(unit.TypePullRequests) && form.Labels != nil {
|
||||
if ctx.Repo.Permission.CanWrite(unit.TypePullRequests) && form.Labels != nil {
|
||||
labels, err := issues_model.GetLabelsInRepoByIDs(ctx, ctx.Repo.Repository.ID, form.Labels)
|
||||
if err != nil {
|
||||
ctx.APIErrorInternal(err)
|
||||
|
||||
@@ -1003,7 +1003,7 @@ func UnDismissPullReview(ctx *context.APIContext) {
|
||||
}
|
||||
|
||||
func dismissReview(ctx *context.APIContext, msg string, isDismiss, dismissPriors bool) {
|
||||
if !ctx.Repo.IsAdmin() {
|
||||
if !ctx.Repo.Permission.IsAdmin() {
|
||||
ctx.APIError(http.StatusForbidden, "Must be repo admin")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -22,7 +22,7 @@ import (
|
||||
)
|
||||
|
||||
func canAccessReleaseDraft(ctx *context.APIContext) bool {
|
||||
if !ctx.IsSigned || !ctx.Repo.CanWrite(unit.TypeReleases) {
|
||||
if !ctx.IsSigned || !ctx.Repo.Permission.CanWrite(unit.TypeReleases) {
|
||||
return false
|
||||
}
|
||||
if ctx.Data["IsApiToken"] != true {
|
||||
|
||||
@@ -60,7 +60,7 @@ func GetReleaseByTag(ctx *context.APIContext) {
|
||||
}
|
||||
|
||||
if release.IsDraft { // only the users with write access can see draft releases
|
||||
if !ctx.IsSigned || !ctx.Repo.CanWrite(unit_model.TypeReleases) {
|
||||
if !ctx.IsSigned || !ctx.Repo.Permission.CanWrite(unit_model.TypeReleases) {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
}
|
||||
|
||||
@@ -187,7 +187,7 @@ func changeRepoTeam(ctx *context.APIContext, add bool) {
|
||||
if !ctx.Repo.Owner.IsOrganization() {
|
||||
ctx.APIError(http.StatusMethodNotAllowed, "repo is not owned by an organization")
|
||||
}
|
||||
if !ctx.Repo.Owner.RepoAdminChangeTeamAccess && !ctx.Repo.IsOwner() {
|
||||
if !ctx.Repo.Owner.RepoAdminChangeTeamAccess && !ctx.Repo.Permission.IsOwner() {
|
||||
ctx.APIError(http.StatusForbidden, "user is nor repo admin nor owner")
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user