Skip to content

Commit

Permalink
Updates to API 404 responses (#6077)
Browse files Browse the repository at this point in the history
  • Loading branch information
jolheiser authored and techknowlogick committed Mar 19, 2019
1 parent d10a668 commit cac9e6e
Show file tree
Hide file tree
Showing 30 changed files with 120 additions and 91 deletions.
29 changes: 29 additions & 0 deletions modules/context/api.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// Copyright 2016 The Gogs Authors. All rights reserved.
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package context

import (
"fmt"
"net/url"
"path"
"strings"

"github.com/go-macaron/csrf"
Expand Down Expand Up @@ -140,3 +143,29 @@ func ReferencesGitRepo() macaron.Handler {
}
}
}

// NotFound handles 404s for APIContext
// String will replace message, errors will be added to a slice
func (ctx *APIContext) NotFound(objs ...interface{}) {
var message = "Not Found"
var errors []string
for _, obj := range objs {
if err, ok := obj.(error); ok {
errors = append(errors, err.Error())
} else {
message = obj.(string)
}
}

u, err := url.Parse(setting.AppURL)
if err != nil {
ctx.Error(500, "Invalid AppURL", err)
return
}
u.Path = path.Join(u.Path, "api", "swagger")
ctx.JSON(404, map[string]interface{}{
"message": message,
"documentation_url": u.String(),
"errors": errors,
})
}
2 changes: 1 addition & 1 deletion routers/api/v1/admin/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func DeleteUserPublicKey(ctx *context.APIContext) {

if err := models.DeletePublicKey(u, ctx.ParamsInt64(":id")); err != nil {
if models.IsErrKeyNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else if models.IsErrKeyAccessDenied(err) {
ctx.Error(403, "", "You do not have access to this key")
} else {
Expand Down
40 changes: 20 additions & 20 deletions routers/api/v1/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ import (
api "code.gitea.io/sdk/gitea"

"github.com/go-macaron/binding"
macaron "gopkg.in/macaron.v1"
"gopkg.in/macaron.v1"
)

func sudo() macaron.Handler {
Expand All @@ -89,7 +89,7 @@ func sudo() macaron.Handler {
user, err := models.GetUserByName(sudo)
if err != nil {
if models.IsErrUserNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetUserByName", err)
}
Expand Down Expand Up @@ -124,7 +124,7 @@ func repoAssignment() macaron.Handler {
owner, err = models.GetUserByName(userName)
if err != nil {
if models.IsErrUserNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetUserByName", err)
}
Expand All @@ -141,7 +141,7 @@ func repoAssignment() macaron.Handler {
if err == nil {
context.RedirectToRepo(ctx.Context, redirectRepoID)
} else if models.IsErrRepoRedirectNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "LookupRepoRedirect", err)
}
Expand All @@ -160,7 +160,7 @@ func repoAssignment() macaron.Handler {
}

if !ctx.Repo.HasAccess() {
ctx.Status(404)
ctx.NotFound()
return
}
}
Expand Down Expand Up @@ -268,7 +268,7 @@ func reqOrgMembership() macaron.Handler {
if ctx.Org.Organization != nil {
ctx.Error(403, "", "Must be an organization member")
} else {
ctx.Status(404)
ctx.NotFound()
}
return
}
Expand All @@ -294,7 +294,7 @@ func reqOrgOwnership() macaron.Handler {
if ctx.Org.Organization != nil {
ctx.Error(403, "", "Must be an organization owner")
} else {
ctx.Status(404)
ctx.NotFound()
}
return
}
Expand All @@ -320,7 +320,7 @@ func orgAssignment(args ...bool) macaron.Handler {
ctx.Org.Organization, err = models.GetOrgByName(ctx.Params(":orgname"))
if err != nil {
if models.IsErrOrgNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetOrgByName", err)
}
Expand All @@ -332,7 +332,7 @@ func orgAssignment(args ...bool) macaron.Handler {
ctx.Org.Team, err = models.GetTeamByID(ctx.ParamsInt64(":teamid"))
if err != nil {
if models.IsErrUserNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetTeamById", err)
}
Expand All @@ -344,36 +344,36 @@ func orgAssignment(args ...bool) macaron.Handler {

func mustEnableIssues(ctx *context.APIContext) {
if !ctx.Repo.CanRead(models.UnitTypeIssues) {
ctx.Status(404)
ctx.NotFound()
return
}
}

func mustAllowPulls(ctx *context.Context) {
func mustAllowPulls(ctx *context.APIContext) {
if !(ctx.Repo.Repository.CanEnablePulls() && ctx.Repo.CanRead(models.UnitTypePullRequests)) {
ctx.Status(404)
ctx.NotFound()
return
}
}

func mustEnableIssuesOrPulls(ctx *context.Context) {
func mustEnableIssuesOrPulls(ctx *context.APIContext) {
if !ctx.Repo.CanRead(models.UnitTypeIssues) &&
!(ctx.Repo.Repository.CanEnablePulls() && ctx.Repo.CanRead(models.UnitTypePullRequests)) {
ctx.Status(404)
ctx.NotFound()
return
}
}

func mustEnableUserHeatmap(ctx *context.Context) {
func mustEnableUserHeatmap(ctx *context.APIContext) {
if !setting.Service.EnableUserHeatmap {
ctx.Status(404)
ctx.NotFound()
return
}
}

func mustNotBeArchived(ctx *context.Context) {
func mustNotBeArchived(ctx *context.APIContext) {
if ctx.Repo.Repository.IsArchived {
ctx.Status(404)
ctx.NotFound()
return
}
}
Expand Down Expand Up @@ -683,8 +683,8 @@ func RegisterRoutes(m *macaron.Macaron) {
})
}, orgAssignment(false, true), reqToken(), reqOrgMembership())

m.Any("/*", func(ctx *context.Context) {
ctx.Error(404)
m.Any("/*", func(ctx *context.APIContext) {
ctx.NotFound()
})

m.Group("/admin", func() {
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/org/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func DeleteHook(ctx *context.APIContext) {
hookID := ctx.ParamsInt64(":id")
if err := models.DeleteWebhookByOrgID(org.ID, hookID); err != nil {
if models.IsErrWebhookNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "DeleteWebhookByOrgID", err)
}
Expand Down
6 changes: 3 additions & 3 deletions routers/api/v1/org/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ func IsMember(ctx *context.APIContext) {
} else if userToCheckIsMember {
ctx.Status(204)
} else {
ctx.Status(404)
ctx.NotFound()
}
return
} else if ctx.User.ID == userToCheck.ID {
ctx.Status(404)
ctx.NotFound()
return
}
}
Expand Down Expand Up @@ -177,7 +177,7 @@ func IsPublicMember(ctx *context.APIContext) {
if userToCheck.IsPublicMember(ctx.Org.Organization.ID) {
ctx.Status(204)
} else {
ctx.Status(404)
ctx.NotFound()
}
}

Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/org/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func GetTeamMembers(ctx *context.APIContext) {
ctx.Error(500, "IsOrganizationMember", err)
return
} else if !isMember {
ctx.Status(404)
ctx.NotFound()
return
}
team := ctx.Org.Team
Expand Down Expand Up @@ -391,7 +391,7 @@ func getRepositoryByParams(ctx *context.APIContext) *models.Repository {
repo, err := models.GetRepositoryByName(ctx.Org.Team.OrgID, ctx.Params(":reponame"))
if err != nil {
if models.IsErrRepoNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetRepositoryByName", err)
}
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/repo/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ func GetBranch(ctx *context.APIContext) {
// if TreePath != "", then URL contained extra slashes
// (i.e. "master/subbranch" instead of "master"), so branch does
// not exist
ctx.Status(404)
ctx.NotFound()
return
}
branch, err := ctx.Repo.Repository.GetBranch(ctx.Repo.BranchName)
if err != nil {
if models.IsErrBranchNotExist(err) {
ctx.Error(404, "GetBranch", err)
ctx.NotFound(err)
} else {
ctx.Error(500, "GetBranch", err)
}
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/collaborators.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func IsCollaborator(ctx *context.APIContext) {
if isColab {
ctx.Status(204)
} else {
ctx.Status(404)
ctx.NotFound()
}
}

Expand Down
8 changes: 4 additions & 4 deletions routers/api/v1/repo/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ func GetRawFile(ctx *context.APIContext) {
// 200:
// description: success
if ctx.Repo.Repository.IsEmpty {
ctx.Status(404)
ctx.NotFound()
return
}

blob, err := ctx.Repo.Commit.GetBlobByPath(ctx.Repo.TreePath)
if err != nil {
if git.IsErrNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetBlobByPath", err)
}
Expand Down Expand Up @@ -124,7 +124,7 @@ func GetEditorconfig(ctx *context.APIContext) {
ec, err := ctx.Repo.GetEditorconfig()
if err != nil {
if git.IsErrNotExist(err) {
ctx.Error(404, "GetEditorconfig", err)
ctx.NotFound(err)
} else {
ctx.Error(500, "GetEditorconfig", err)
}
Expand All @@ -134,7 +134,7 @@ func GetEditorconfig(ctx *context.APIContext) {
fileName := ctx.Params("filename")
def := ec.GetDefinitionForFilename(fileName)
if def == nil {
ctx.Error(404, "GetDefinitionForFilename", err)
ctx.NotFound(err)
return
}
ctx.JSON(200, def)
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/git_ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func getGitRefsInternal(ctx *context.APIContext, filter string) {
}

if len(refs) == 0 {
ctx.Status(404)
ctx.NotFound()
return
}

Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func DeleteHook(ctx *context.APIContext) {
// "$ref": "#/responses/notFound"
if err := models.DeleteWebhookByRepoID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")); err != nil {
if models.IsErrWebhookNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "DeleteWebhookByRepoID", err)
}
Expand Down
10 changes: 5 additions & 5 deletions routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func GetIssue(ctx *context.APIContext) {
issue, err := models.GetIssueWithAttrsByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil {
if models.IsErrIssueNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetIssueByIndex", err)
}
Expand Down Expand Up @@ -283,7 +283,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil {
if models.IsErrIssueNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetIssueByIndex", err)
}
Expand Down Expand Up @@ -412,7 +412,7 @@ func UpdateIssueDeadline(ctx *context.APIContext, form api.EditDeadlineOption) {
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil {
if models.IsErrIssueNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetIssueByIndex", err)
}
Expand Down Expand Up @@ -478,7 +478,7 @@ func StartIssueStopwatch(ctx *context.APIContext) {
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil {
if models.IsErrIssueNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetIssueByIndex", err)
}
Expand Down Expand Up @@ -547,7 +547,7 @@ func StopIssueStopwatch(ctx *context.APIContext) {
issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil {
if models.IsErrIssueNotExist(err) {
ctx.Status(404)
ctx.NotFound()
} else {
ctx.Error(500, "GetIssueByIndex", err)
}
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/repo/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption)
comment, err := models.GetCommentByID(ctx.ParamsInt64(":id"))
if err != nil {
if models.IsErrCommentNotExist(err) {
ctx.Error(404, "GetCommentByID", err)
ctx.NotFound(err)
} else {
ctx.Error(500, "GetCommentByID", err)
}
Expand Down Expand Up @@ -361,7 +361,7 @@ func deleteIssueComment(ctx *context.APIContext) {
comment, err := models.GetCommentByID(ctx.ParamsInt64(":id"))
if err != nil {
if models.IsErrCommentNotExist(err) {
ctx.Error(404, "GetCommentByID", err)
ctx.NotFound(err)
} else {
ctx.Error(500, "GetCommentByID", err)
}
Expand Down
Loading

0 comments on commit cac9e6e

Please sign in to comment.