routes/api: change status handle to new style

Also fixed one bug that did not catch team not found error.
This commit is contained in:
Unknwon 2018-12-01 21:41:30 -05:00
parent ce13fbb98a
commit 69c1cd3f38
No known key found for this signature in database
GPG Key ID: 25B575AE3213B2B3
5 changed files with 64 additions and 37 deletions

21
models/errors/org.go Normal file
View File

@ -0,0 +1,21 @@
// Copyright 2018 The Gogs 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 errors
import "fmt"
type TeamNotExist struct {
TeamID int64
Name string
}
func IsTeamNotExist(err error) bool {
_, ok := err.(TeamNotExist)
return ok
}
func (err TeamNotExist) Error() string {
return fmt.Sprintf("team does not exist [team_id: %d, name: %s]", err.TeamID, err.Name)
}

View File

@ -16,7 +16,6 @@ import (
var ( var (
ErrOrgNotExist = errors.New("Organization does not exist") ErrOrgNotExist = errors.New("Organization does not exist")
ErrTeamNotExist = errors.New("Team does not exist")
) )
// IsOwnedBy returns true if given user is in the owner team. // IsOwnedBy returns true if given user is in the owner team.

View File

@ -5,11 +5,12 @@
package models package models
import ( import (
"errors"
"fmt" "fmt"
"strings" "strings"
"github.com/go-xorm/xorm" "github.com/go-xorm/xorm"
"github.com/gogs/gogs/models/errors"
) )
const OWNER_TEAM = "Owners" const OWNER_TEAM = "Owners"
@ -274,7 +275,7 @@ func getTeamOfOrgByName(e Engine, orgID int64, name string) (*Team, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} else if !has { } else if !has {
return nil, ErrTeamNotExist return nil, errors.TeamNotExist{0, name}
} }
return t, nil return t, nil
} }
@ -290,7 +291,7 @@ func getTeamByID(e Engine, teamID int64) (*Team, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} else if !has { } else if !has {
return nil, ErrTeamNotExist return nil, errors.TeamNotExist{teamID, ""}
} }
return t, nil return t, nil
} }

View File

@ -6,6 +6,7 @@ package context
import ( import (
"fmt" "fmt"
"net/http"
"strings" "strings"
"github.com/Unknwon/paginater" "github.com/Unknwon/paginater"
@ -33,7 +34,7 @@ func (c *APIContext) Error(status int, title string, obj interface{}) {
message = obj.(string) message = obj.(string)
} }
if status == 500 { if status == http.StatusInternalServerError {
log.Error(3, "%s: %s", title, message) log.Error(3, "%s: %s", title, message)
} }
@ -43,6 +44,27 @@ func (c *APIContext) Error(status int, title string, obj interface{}) {
}) })
} }
// NotFound renders the 404 response.
func (c *APIContext) NotFound() {
c.Status(http.StatusNotFound)
}
// ServerError renders the 500 response.
func (c *APIContext) ServerError(title string, err error) {
c.Error(http.StatusInternalServerError, title, err)
}
// NotFoundOrServerError use error check function to determine if the error
// is about not found. It responses with 404 status code for not found error,
// or error context description for logging purpose of 500 server error.
func (c *APIContext) NotFoundOrServerError(title string, errck func(error) bool, err error) {
if errck(err) {
c.NotFound()
return
}
c.ServerError(title, err)
}
// SetLinkHeader sets pagination link header by given total number and page size. // SetLinkHeader sets pagination link header by given total number and page size.
func (c *APIContext) SetLinkHeader(total, pageSize int) { func (c *APIContext) SetLinkHeader(total, pageSize int) {
page := paginater.New(total, pageSize, c.QueryInt("page"), 0) page := paginater.New(total, pageSize, c.QueryInt("page"), 0)

View File

@ -5,6 +5,7 @@
package v1 package v1
import ( import (
"net/http"
"strings" "strings"
"github.com/go-macaron/binding" "github.com/go-macaron/binding"
@ -39,43 +40,34 @@ func repoAssignment() macaron.Handler {
} else { } else {
owner, err = models.GetUserByName(userName) owner, err = models.GetUserByName(userName)
if err != nil { if err != nil {
if errors.IsUserNotExist(err) { c.NotFoundOrServerError("GetUserByName", errors.IsUserNotExist, err)
c.Status(404)
} else {
c.Error(500, "GetUserByName", err)
}
return return
} }
} }
c.Repo.Owner = owner c.Repo.Owner = owner
// Get repository.
repo, err := models.GetRepositoryByName(owner.ID, repoName) repo, err := models.GetRepositoryByName(owner.ID, repoName)
if err != nil { if err != nil {
if errors.IsRepoNotExist(err) { c.NotFoundOrServerError("GetRepositoryByName", errors.IsRepoNotExist, err)
c.Status(404)
} else {
c.Error(500, "GetRepositoryByName", err)
}
return return
} else if err = repo.GetOwner(); err != nil { } else if err = repo.GetOwner(); err != nil {
c.Error(500, "GetOwner", err) c.ServerError("GetOwner", err)
return return
} }
if c.IsTokenAuth && c.User.IsAdmin { if c.IsTokenAuth && c.User.IsAdmin {
c.Repo.AccessMode = models.ACCESS_MODE_OWNER c.Repo.AccessMode = models.ACCESS_MODE_OWNER
} else { } else {
mode, err := models.AccessLevel(c.User.ID, repo) mode, err := models.AccessLevel(c.UserID(), repo)
if err != nil { if err != nil {
c.Error(500, "AccessLevel", err) c.ServerError("AccessLevel", err)
return return
} }
c.Repo.AccessMode = mode c.Repo.AccessMode = mode
} }
if !c.Repo.HasAccess() { if !c.Repo.HasAccess() {
c.Status(404) c.NotFound()
return return
} }
@ -87,7 +79,7 @@ func repoAssignment() macaron.Handler {
func reqToken() macaron.Handler { func reqToken() macaron.Handler {
return func(c *context.Context) { return func(c *context.Context) {
if !c.IsTokenAuth { if !c.IsTokenAuth {
c.Error(401) c.Error(http.StatusUnauthorized)
return return
} }
} }
@ -96,7 +88,7 @@ func reqToken() macaron.Handler {
func reqBasicAuth() macaron.Handler { func reqBasicAuth() macaron.Handler {
return func(c *context.Context) { return func(c *context.Context) {
if !c.IsBasicAuth { if !c.IsBasicAuth {
c.Error(401) c.Error(http.StatusUnauthorized)
return return
} }
} }
@ -105,7 +97,7 @@ func reqBasicAuth() macaron.Handler {
func reqAdmin() macaron.Handler { func reqAdmin() macaron.Handler {
return func(c *context.Context) { return func(c *context.Context) {
if !c.IsLogged || !c.User.IsAdmin { if !c.IsLogged || !c.User.IsAdmin {
c.Error(403) c.Error(http.StatusForbidden)
return return
} }
} }
@ -114,7 +106,7 @@ func reqAdmin() macaron.Handler {
func reqRepoWriter() macaron.Handler { func reqRepoWriter() macaron.Handler {
return func(c *context.Context) { return func(c *context.Context) {
if !c.Repo.IsWriter() { if !c.Repo.IsWriter() {
c.Error(403) c.Error(http.StatusForbidden)
return return
} }
} }
@ -138,11 +130,7 @@ func orgAssignment(args ...bool) macaron.Handler {
if assignOrg { if assignOrg {
c.Org.Organization, err = models.GetUserByName(c.Params(":orgname")) c.Org.Organization, err = models.GetUserByName(c.Params(":orgname"))
if err != nil { if err != nil {
if errors.IsUserNotExist(err) { c.NotFoundOrServerError("GetUserByName", errors.IsUserNotExist, err)
c.Status(404)
} else {
c.Error(500, "GetUserByName", err)
}
return return
} }
} }
@ -150,11 +138,7 @@ func orgAssignment(args ...bool) macaron.Handler {
if assignTeam { if assignTeam {
c.Org.Team, err = models.GetTeamByID(c.ParamsInt64(":teamid")) c.Org.Team, err = models.GetTeamByID(c.ParamsInt64(":teamid"))
if err != nil { if err != nil {
if errors.IsUserNotExist(err) { c.NotFoundOrServerError("GetTeamByID", errors.IsTeamNotExist, err)
c.Status(404)
} else {
c.Error(500, "GetTeamById", err)
}
return return
} }
} }
@ -163,7 +147,7 @@ func orgAssignment(args ...bool) macaron.Handler {
func mustEnableIssues(c *context.APIContext) { func mustEnableIssues(c *context.APIContext) {
if !c.Repo.Repository.EnableIssues || c.Repo.Repository.EnableExternalTracker { if !c.Repo.Repository.EnableIssues || c.Repo.Repository.EnableExternalTracker {
c.Status(404) c.NotFound()
return return
} }
} }
@ -326,7 +310,7 @@ func RegisterRoutes(m *macaron.Macaron) {
}, orgAssignment(true)) }, orgAssignment(true))
m.Any("/*", func(c *context.Context) { m.Any("/*", func(c *context.Context) {
c.Error(404) c.NotFound()
}) })
m.Group("/admin", func() { m.Group("/admin", func() {