Skip to content

Clarify path param naming #32969

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

Merged
merged 3 commits into from
Dec 24, 2024
Merged

Clarify path param naming #32969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
adefa63
fix
wxiaoguang Dec 24, 2024
d4be7f0
fix
wxiaoguang Dec 24, 2024
2c2c002
Merge branch 'main' into fix-path-param
wxiaoguang Dec 24, 2024
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
6 changes: 6 additions & 0 deletions modules/setting/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,9 @@ func checkOverlappedPath(name, path string) {
}
configuredPaths[path] = name
}

func PanicInDevOrTesting(msg string, a ...any) {
if !IsProd || IsInTesting {
panic(fmt.Sprintf(msg, a...))
}
}
4 changes: 1 addition & 3 deletions modules/templates/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,5 @@ func QueryBuild(a ...any) template.URL {
}

func panicIfDevOrTesting() {
if !setting.IsProd || setting.IsInTesting {
panic("legacy template functions are for backward compatibility only, do not use them in new code")
}
setting.PanicInDevOrTesting("legacy template functions are for backward compatibility only, do not use them in new code")
}
8 changes: 4 additions & 4 deletions routers/api/v1/admin/adopt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ func AdoptRepository(ctx *context.APIContext) {
// "$ref": "#/responses/notFound"
// "403":
// "$ref": "#/responses/forbidden"
ownerName := ctx.PathParam(":username")
repoName := ctx.PathParam(":reponame")
ownerName := ctx.PathParam("username")
repoName := ctx.PathParam("reponame")

ctxUser, err := user_model.GetUserByName(ctx, ownerName)
if err != nil {
Expand Down Expand Up @@ -142,8 +142,8 @@ func DeleteUnadoptedRepository(ctx *context.APIContext) {
// "$ref": "#/responses/empty"
// "403":
// "$ref": "#/responses/forbidden"
ownerName := ctx.PathParam(":username")
repoName := ctx.PathParam(":reponame")
ownerName := ctx.PathParam("username")
repoName := ctx.PathParam("reponame")

ctxUser, err := user_model.GetUserByName(ctx, ownerName)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/admin/cron.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func PostCronTask(ctx *context.APIContext) {
// "$ref": "#/responses/empty"
// "404":
// "$ref": "#/responses/notFound"
task := cron.GetTask(ctx.PathParam(":task"))
task := cron.GetTask(ctx.PathParam("task"))
if task == nil {
ctx.NotFound()
return
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/admin/email.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func GetAllEmails(ctx *context.APIContext) {
listOptions := utils.GetListOptions(ctx)

emails, maxResults, err := user_model.SearchEmails(ctx, &user_model.SearchEmailOptions{
Keyword: ctx.PathParam(":email"),
Keyword: ctx.PathParam("email"),
ListOptions: listOptions,
})
if err != nil {
Expand Down Expand Up @@ -82,6 +82,6 @@ func SearchEmail(ctx *context.APIContext) {
// "403":
// "$ref": "#/responses/forbidden"

ctx.SetPathParam(":email", ctx.FormTrim("q"))
ctx.SetPathParam("email", ctx.FormTrim("q"))
GetAllEmails(ctx)
}
6 changes: 3 additions & 3 deletions routers/api/v1/admin/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func GetHook(ctx *context.APIContext) {
// "200":
// "$ref": "#/responses/Hook"

hookID := ctx.PathParamInt64(":id")
hookID := ctx.PathParamInt64("id")
hook, err := webhook.GetSystemOrDefaultWebhook(ctx, hookID)
if err != nil {
if errors.Is(err, util.ErrNotExist) {
Expand Down Expand Up @@ -142,7 +142,7 @@ func EditHook(ctx *context.APIContext) {
form := web.GetForm(ctx).(*api.EditHookOption)

// TODO in body params
hookID := ctx.PathParamInt64(":id")
hookID := ctx.PathParamInt64("id")
utils.EditSystemHook(ctx, form, hookID)
}

Expand All @@ -164,7 +164,7 @@ func DeleteHook(ctx *context.APIContext) {
// "204":
// "$ref": "#/responses/empty"

hookID := ctx.PathParamInt64(":id")
hookID := ctx.PathParamInt64("id")
if err := webhook.DeleteDefaultSystemWebhook(ctx, hookID); err != nil {
if errors.Is(err, util.ErrNotExist) {
ctx.NotFound()
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/admin/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ func DeleteUserPublicKey(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

if err := asymkey_service.DeletePublicKey(ctx, ctx.ContextUser, ctx.PathParamInt64(":id")); err != nil {
if err := asymkey_service.DeletePublicKey(ctx, ctx.ContextUser, ctx.PathParamInt64("id")); err != nil {
if asymkey_model.IsErrKeyNotExist(err) {
ctx.NotFound()
} else if asymkey_model.IsErrKeyAccessDenied(err) {
Expand Down
8 changes: 4 additions & 4 deletions routers/api/v1/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,12 +596,12 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) {

var err error
if assignOrg {
ctx.Org.Organization, err = organization.GetOrgByName(ctx, ctx.PathParam(":org"))
ctx.Org.Organization, err = organization.GetOrgByName(ctx, ctx.PathParam("org"))
if err != nil {
if organization.IsErrOrgNotExist(err) {
redirectUserID, err := user_model.LookupUserRedirect(ctx, ctx.PathParam(":org"))
redirectUserID, err := user_model.LookupUserRedirect(ctx, ctx.PathParam("org"))
if err == nil {
context.RedirectToUser(ctx.Base, ctx.PathParam(":org"), redirectUserID)
context.RedirectToUser(ctx.Base, ctx.PathParam("org"), redirectUserID)
} else if user_model.IsErrUserRedirectNotExist(err) {
ctx.NotFound("GetOrgByName", err)
} else {
Expand All @@ -616,7 +616,7 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) {
}

if assignTeam {
ctx.Org.Team, err = organization.GetTeamByID(ctx, ctx.PathParamInt64(":teamid"))
ctx.Org.Team, err = organization.GetTeamByID(ctx, ctx.PathParamInt64("teamid"))
if err != nil {
if organization.IsErrTeamNotExist(err) {
ctx.NotFound()
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/notify/threads.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func ReadThread(ctx *context.APIContext) {
}

func getThread(ctx *context.APIContext) *activities_model.Notification {
n, err := activities_model.GetNotificationByID(ctx, ctx.PathParamInt64(":id"))
n, err := activities_model.GetNotificationByID(ctx, ctx.PathParamInt64("id"))
if err != nil {
if db.IsErrNotExist(err) {
ctx.Error(http.StatusNotFound, "GetNotificationByID", err)
Expand Down
6 changes: 3 additions & 3 deletions routers/api/v1/org/label.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func GetLabel(ctx *context.APIContext) {
label *issues_model.Label
err error
)
strID := ctx.PathParam(":id")
strID := ctx.PathParam("id")
if intID, err2 := strconv.ParseInt(strID, 10, 64); err2 != nil {
label, err = issues_model.GetLabelInOrgByName(ctx, ctx.Org.Organization.ID, strID)
} else {
Expand Down Expand Up @@ -190,7 +190,7 @@ func EditLabel(ctx *context.APIContext) {
// "422":
// "$ref": "#/responses/validationError"
form := web.GetForm(ctx).(*api.EditLabelOption)
l, err := issues_model.GetLabelInOrgByID(ctx, ctx.Org.Organization.ID, ctx.PathParamInt64(":id"))
l, err := issues_model.GetLabelInOrgByID(ctx, ctx.Org.Organization.ID, ctx.PathParamInt64("id"))
if err != nil {
if issues_model.IsErrOrgLabelNotExist(err) {
ctx.NotFound()
Expand Down Expand Up @@ -249,7 +249,7 @@ func DeleteLabel(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

if err := issues_model.DeleteLabel(ctx, ctx.Org.Organization.ID, ctx.PathParamInt64(":id")); err != nil {
if err := issues_model.DeleteLabel(ctx, ctx.Org.Organization.ID, ctx.PathParamInt64("id")); err != nil {
ctx.Error(http.StatusInternalServerError, "DeleteLabel", err)
return
}
Expand Down
10 changes: 5 additions & 5 deletions routers/api/v1/org/member.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func IsMember(ctx *context.APIContext) {
// "404":
// description: user is not a member

userToCheck := user.GetUserByParams(ctx)
userToCheck := user.GetContextUserByPathParam(ctx)
if ctx.Written() {
return
}
Expand Down Expand Up @@ -194,7 +194,7 @@ func IsPublicMember(ctx *context.APIContext) {
// "404":
// description: user is not a public member

userToCheck := user.GetUserByParams(ctx)
userToCheck := user.GetContextUserByPathParam(ctx)
if ctx.Written() {
return
}
Expand Down Expand Up @@ -236,7 +236,7 @@ func PublicizeMember(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

userToPublicize := user.GetUserByParams(ctx)
userToPublicize := user.GetContextUserByPathParam(ctx)
if ctx.Written() {
return
}
Expand Down Expand Up @@ -278,7 +278,7 @@ func ConcealMember(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

userToConceal := user.GetUserByParams(ctx)
userToConceal := user.GetContextUserByPathParam(ctx)
if ctx.Written() {
return
}
Expand Down Expand Up @@ -318,7 +318,7 @@ func DeleteMember(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

member := user.GetUserByParams(ctx)
member := user.GetContextUserByPathParam(ctx)
if ctx.Written() {
return
}
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/org/org.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func GetUserOrgsPermissions(ctx *context.APIContext) {
// "$ref": "#/responses/notFound"

var o *user_model.User
if o = user.GetUserByParamsName(ctx, ":org"); o == nil {
if o = user.GetUserByPathParam(ctx, "org"); o == nil {
return
}

Expand Down
8 changes: 4 additions & 4 deletions routers/api/v1/org/team.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ func GetTeamMember(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

u := user.GetUserByParams(ctx)
u := user.GetContextUserByPathParam(ctx)
if ctx.Written() {
return
}
Expand Down Expand Up @@ -492,7 +492,7 @@ func AddTeamMember(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

u := user.GetUserByParams(ctx)
u := user.GetContextUserByPathParam(ctx)
if ctx.Written() {
return
}
Expand Down Expand Up @@ -532,7 +532,7 @@ func RemoveTeamMember(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

u := user.GetUserByParams(ctx)
u := user.GetContextUserByPathParam(ctx)
if ctx.Written() {
return
}
Expand Down Expand Up @@ -645,7 +645,7 @@ func GetTeamRepo(ctx *context.APIContext) {

// getRepositoryByParams get repository by a team's organization ID and repo name
func getRepositoryByParams(ctx *context.APIContext) *repo_model.Repository {
repo, err := repo_model.GetRepositoryByName(ctx, ctx.Org.Team.OrgID, ctx.PathParam(":reponame"))
repo, err := repo_model.GetRepositoryByName(ctx, ctx.Org.Team.OrgID, ctx.PathParam("reponame"))
if err != nil {
if repo_model.IsErrRepoNotExist(err) {
ctx.NotFound()
Expand Down
6 changes: 3 additions & 3 deletions routers/api/v1/repo/branch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ func GetBranchProtection(ctx *context.APIContext) {
// "$ref": "#/responses/notFound"

repo := ctx.Repo.Repository
bpName := ctx.PathParam(":name")
bpName := ctx.PathParam("name")
bp, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, bpName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err)
Expand Down Expand Up @@ -805,7 +805,7 @@ func EditBranchProtection(ctx *context.APIContext) {
// "$ref": "#/responses/repoArchivedError"
form := web.GetForm(ctx).(*api.EditBranchProtectionOption)
repo := ctx.Repo.Repository
bpName := ctx.PathParam(":name")
bpName := ctx.PathParam("name")
protectBranch, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, bpName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err)
Expand Down Expand Up @@ -1124,7 +1124,7 @@ func DeleteBranchProtection(ctx *context.APIContext) {
// "$ref": "#/responses/notFound"

repo := ctx.Repo.Repository
bpName := ctx.PathParam(":name")
bpName := ctx.PathParam("name")
bp, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, bpName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err)
Expand Down
10 changes: 5 additions & 5 deletions routers/api/v1/repo/collaborators.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func IsCollaborator(ctx *context.APIContext) {
// "422":
// "$ref": "#/responses/validationError"

user, err := user_model.GetUserByName(ctx, ctx.PathParam(":collaborator"))
user, err := user_model.GetUserByName(ctx, ctx.PathParam("collaborator"))
if err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
Expand Down Expand Up @@ -163,7 +163,7 @@ func AddOrUpdateCollaborator(ctx *context.APIContext) {

form := web.GetForm(ctx).(*api.AddCollaboratorOption)

collaborator, err := user_model.GetUserByName(ctx, ctx.PathParam(":collaborator"))
collaborator, err := user_model.GetUserByName(ctx, ctx.PathParam("collaborator"))
if err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
Expand Down Expand Up @@ -226,7 +226,7 @@ func DeleteCollaborator(ctx *context.APIContext) {
// "422":
// "$ref": "#/responses/validationError"

collaborator, err := user_model.GetUserByName(ctx, ctx.PathParam(":collaborator"))
collaborator, err := user_model.GetUserByName(ctx, ctx.PathParam("collaborator"))
if err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.Error(http.StatusUnprocessableEntity, "", err)
Expand Down Expand Up @@ -274,12 +274,12 @@ func GetRepoPermissions(ctx *context.APIContext) {
// "403":
// "$ref": "#/responses/forbidden"

if !ctx.Doer.IsAdmin && ctx.Doer.LoginName != ctx.PathParam(":collaborator") && !ctx.IsUserRepoAdmin() {
if !ctx.Doer.IsAdmin && ctx.Doer.LoginName != ctx.PathParam("collaborator") && !ctx.IsUserRepoAdmin() {
ctx.Error(http.StatusForbidden, "User", "Only admins can query all permissions, repo admins can query all repo permissions, collaborators can query only their own")
return
}

collaborator, err := user_model.GetUserByName(ctx, ctx.PathParam(":collaborator"))
collaborator, err := user_model.GetUserByName(ctx, ctx.PathParam("collaborator"))
if err != nil {
if user_model.IsErrUserNotExist(err) {
ctx.Error(http.StatusNotFound, "GetUserByName", err)
Expand Down
6 changes: 3 additions & 3 deletions routers/api/v1/repo/commits.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func GetSingleCommit(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

sha := ctx.PathParam(":sha")
sha := ctx.PathParam("sha")
if !git.IsValidRefPattern(sha) {
ctx.Error(http.StatusUnprocessableEntity, "no valid ref or sha", fmt.Sprintf("no valid ref or sha: %s", sha))
return
Expand Down Expand Up @@ -312,8 +312,8 @@ func DownloadCommitDiffOrPatch(ctx *context.APIContext) {
// "$ref": "#/responses/string"
// "404":
// "$ref": "#/responses/notFound"
sha := ctx.PathParam(":sha")
diffType := git.RawDiffType(ctx.PathParam(":diffType"))
sha := ctx.PathParam("sha")
diffType := git.RawDiffType(ctx.PathParam("diffType"))

if err := git.GetRawDiff(ctx.Repo.GitRepo, sha, diffType, ctx.Resp); err != nil {
if git.IsErrNotExist(err) {
Expand Down
6 changes: 3 additions & 3 deletions routers/api/v1/repo/git_hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func GetGitHook(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

hookID := ctx.PathParam(":id")
hookID := ctx.PathParam("id")
hook, err := ctx.Repo.GitRepo.GetHook(hookID)
if err != nil {
if err == git.ErrNotValidHook {
Expand Down Expand Up @@ -126,7 +126,7 @@ func EditGitHook(ctx *context.APIContext) {
// "$ref": "#/responses/notFound"

form := web.GetForm(ctx).(*api.EditGitHookOption)
hookID := ctx.PathParam(":id")
hookID := ctx.PathParam("id")
hook, err := ctx.Repo.GitRepo.GetHook(hookID)
if err != nil {
if err == git.ErrNotValidHook {
Expand Down Expand Up @@ -175,7 +175,7 @@ func DeleteGitHook(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

hookID := ctx.PathParam(":id")
hookID := ctx.PathParam("id")
hook, err := ctx.Repo.GitRepo.GetHook(hookID)
if err != nil {
if err == git.ErrNotValidHook {
Expand Down
Loading
Loading