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

Use API error helpers and improve response codes #2366

Merged
merged 8 commits into from
Sep 2, 2023
Merged

Use API error helpers and improve response codes #2366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3aa3878
Use API error helpers
qwerty287 Sep 2, 2023
ea7e7fa
Refactor deleting
qwerty287 Sep 2, 2023
59e6f72
Rename func
qwerty287 Sep 2, 2023
338c0e5
Correct response codes
qwerty287 Sep 2, 2023
8ac2e29
Some more stats code improvements
qwerty287 Sep 2, 2023
7f39313
Fix handleDbError can't work
qwerty287 Sep 2, 2023
480a92b
Merge branch 'main' into user-err-helper
qwerty287 Sep 2, 2023
470131d
fix tests
qwerty287 Sep 2, 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
36 changes: 15 additions & 21 deletions cmd/server/docs/docs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ const docTemplate = `{
},
"/healthz": {
"get": {
"description": "If everything is fine, just a 200 will be returned, a 500 signals server state is unhealthy.",
"description": "If everything is fine, just a 204 will be returned, a 500 signals server state is unhealthy.",
"produces": [
"text/plain"
],
Expand All @@ -614,8 +614,8 @@ const docTemplate = `{
],
"summary": "Health information",
"responses": {
"200": {
"description": "OK"
"204": {
"description": "No Content"
},
"500": {
"description": "Internal Server Error"
Expand Down Expand Up @@ -869,7 +869,7 @@ const docTemplate = `{
"delete": {
"description": "Deletes the given org. Requires admin rights.",
"produces": [
"application/json"
"text/plain"
],
"tags": [
"Orgs"
Expand All @@ -894,10 +894,7 @@ const docTemplate = `{
],
"responses": {
"204": {
"description": "No Content",
"schema": {
"$ref": "#/definitions/Org"
}
"description": "No Content"
}
}
}
Expand Down Expand Up @@ -1838,8 +1835,8 @@ const docTemplate = `{
}
],
"responses": {
"200": {
"description": "OK"
"204": {
"description": "No Content"
}
}
},
Expand Down Expand Up @@ -1928,8 +1925,8 @@ const docTemplate = `{
}
],
"responses": {
"200": {
"description": "OK"
"204": {
"description": "No Content"
}
}
}
Expand Down Expand Up @@ -2667,8 +2664,8 @@ const docTemplate = `{
}
],
"responses": {
"200": {
"description": "OK"
"204": {
"description": "No Content"
}
}
},
Expand Down Expand Up @@ -3307,7 +3304,7 @@ const docTemplate = `{
"tags": [
"User"
],
"summary": "Return the token of the current user as stringª",
"summary": "Return the token of the current user as string",
"parameters": [
{
"type": "string",
Expand Down Expand Up @@ -3473,7 +3470,7 @@ const docTemplate = `{
"delete": {
"description": "Deletes the given user. Requires admin rights.",
"produces": [
"application/json"
"text/plain"
],
"tags": [
"Users"
Expand All @@ -3497,11 +3494,8 @@ const docTemplate = `{
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/User"
}
"204": {
"description": "No Content"
}
}
},
Expand Down
10 changes: 5 additions & 5 deletions server/api/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func GetAgent(c *gin.Context) {

agent, err := store.FromContext(c).AgentFind(agentID)
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
c.JSON(http.StatusOK, agent)
Expand All @@ -89,7 +89,7 @@ func GetAgentTasks(c *gin.Context) {

agent, err := store.FromContext(c).AgentFind(agentID)
if err != nil {
c.String(http.StatusNotFound, "Cannot find agent. %s", err)
handleDbError(c, err)
return
}

Expand Down Expand Up @@ -132,7 +132,7 @@ func PatchAgent(c *gin.Context) {

agent, err := _store.AgentFind(agentID)
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
agent.Name = in.Name
Expand Down Expand Up @@ -201,12 +201,12 @@ func DeleteAgent(c *gin.Context) {

agent, err := _store.AgentFind(agentID)
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
if err = _store.AgentDelete(agent); err != nil {
c.String(http.StatusInternalServerError, "Error deleting user. %s", err)
return
}
c.String(http.StatusNoContent, "")
c.Status(http.StatusNoContent)
}
8 changes: 2 additions & 6 deletions server/api/badge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,7 @@ func GetBadge(c *gin.Context) {
}

if err != nil {
if errors.Is(err, types.RecordNotExist) {
c.AbortWithStatus(http.StatusNotFound)
return
}
_ = c.AbortWithError(http.StatusInternalServerError, err)
handleDbError(c, err)
return
}

Expand Down Expand Up @@ -107,7 +103,7 @@ func GetCC(c *gin.Context) {
_store := store.FromContext(c)
repo, err := _store.GetRepoName(c.Param("owner") + "/" + c.Param("name"))
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}

Expand Down
13 changes: 7 additions & 6 deletions server/api/cron.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"time"

"github.com/gin-gonic/gin"

"github.com/woodpecker-ci/woodpecker/server"
cronScheduler "github.com/woodpecker-ci/woodpecker/server/cron"
"github.com/woodpecker-ci/woodpecker/server/model"
Expand Down Expand Up @@ -48,7 +49,7 @@ func GetCron(c *gin.Context) {

cron, err := store.FromContext(c).CronFind(repo, id)
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
c.JSON(http.StatusOK, cron)
Expand All @@ -75,7 +76,7 @@ func RunCron(c *gin.Context) {

cron, err := _store.CronFind(repo, id)
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}

Expand Down Expand Up @@ -182,7 +183,7 @@ func PatchCron(c *gin.Context) {

cron, err := _store.CronFind(repo, id)
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
if in.Branch != "" {
Expand Down Expand Up @@ -245,7 +246,7 @@ func GetCronList(c *gin.Context) {
// @Summary Delete a cron job by id
// @Router /repos/{repo_id}/cron/{cron} [delete]
// @Produce plain
// @Success 200
// @Success 204
// @Tags Repository cron jobs
// @Param Authorization header string true "Insert your personal access token" default(Bearer <personal access token>)
// @Param repo_id path int true "the repository id"
Expand All @@ -258,8 +259,8 @@ func DeleteCron(c *gin.Context) {
return
}
if err := store.FromContext(c).CronDelete(repo, id); err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
c.String(http.StatusNoContent, "")
c.Status(http.StatusNoContent)
}
9 changes: 5 additions & 4 deletions server/api/global_secret.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"net/http"

"github.com/gin-gonic/gin"

"github.com/woodpecker-ci/woodpecker/server/router/middleware/session"

"github.com/woodpecker-ci/woodpecker/server"
Expand Down Expand Up @@ -61,7 +62,7 @@ func GetGlobalSecret(c *gin.Context) {
name := c.Param("secret")
secret, err := server.Config.Services.Secrets.GlobalSecretFind(name)
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
c.JSON(http.StatusOK, secret.Copy())
Expand Down Expand Up @@ -122,7 +123,7 @@ func PatchGlobalSecret(c *gin.Context) {

secret, err := server.Config.Services.Secrets.GlobalSecretFind(name)
if err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
if in.Value != "" {
Expand Down Expand Up @@ -159,8 +160,8 @@ func PatchGlobalSecret(c *gin.Context) {
func DeleteGlobalSecret(c *gin.Context) {
name := c.Param("secret")
if err := server.Config.Services.Secrets.GlobalSecretDelete(name); err != nil {
handleDbGetError(c, err)
handleDbError(c, err)
return
}
c.String(http.StatusNoContent, "")
c.Status(http.StatusNoContent)
}
4 changes: 2 additions & 2 deletions server/api/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ func handlePipelineErr(c *gin.Context, err error) {
} else if errors.Is(err, &pipeline.ErrBadRequest{}) {
c.String(http.StatusBadRequest, "%s", err)
} else if errors.Is(err, &pipeline.ErrFiltered{}) {
c.String(http.StatusNoContent, "%s", err)
c.Status(http.StatusNoContent)
} else {
_ = c.AbortWithError(http.StatusInternalServerError, err)
}
}

func handleDbGetError(c *gin.Context, err error) {
func handleDbError(c *gin.Context, err error) {
if errors.Is(err, types.RecordNotExist) {
c.AbortWithStatus(http.StatusNotFound)
return
Expand Down
20 changes: 8 additions & 12 deletions server/api/hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,23 +143,20 @@ func PostHook(c *gin.Context) {

repo, err := _store.GetRepoNameFallback(tmpRepo.ForgeRemoteID, tmpRepo.FullName)
if err != nil {
msg := fmt.Sprintf("failure to get repo %s from store", tmpRepo.FullName)
log.Error().Err(err).Msg(msg)
c.String(http.StatusNotFound, msg)
log.Error().Err(err).Msgf("failure to get repo %s from store", tmpRepo.FullName)
handleDbError(c, err)
return
}
if !repo.IsActive {
msg := fmt.Sprintf("ignoring hook: repo %s is inactive", tmpRepo.FullName)
log.Debug().Msg(msg)
c.String(http.StatusNoContent, msg)
log.Debug().Msgf("ignoring hook: repo %s is inactive", tmpRepo.FullName)
c.Status(http.StatusNoContent)
return
}
oldFullName := repo.FullName

if repo.UserID == 0 {
msg := fmt.Sprintf("ignoring hook. repo %s has no owner.", repo.FullName)
log.Warn().Msg(msg)
c.String(http.StatusNoContent, msg)
log.Warn().Msgf("ignoring hook. repo %s has no owner.", repo.FullName)
c.Status(http.StatusNoContent)
return
}

Expand Down Expand Up @@ -220,9 +217,8 @@ func PostHook(c *gin.Context) {
//

if tmpPipeline.Event == model.EventPull && !repo.AllowPull {
msg := "ignoring hook: pull requests are disabled for this repo in woodpecker"
log.Debug().Str("repo", repo.FullName).Msg(msg)
c.String(http.StatusNoContent, msg)
log.Debug().Str("repo", repo.FullName).Msg("ignoring hook: pull requests are disabled for this repo in woodpecker")
c.Status(http.StatusNoContent)
return
}

Expand Down
2 changes: 1 addition & 1 deletion server/api/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func GetLoginToken(c *gin.Context) {

user, err := _store.GetUserLogin(login)
if err != nil {
_ = c.AbortWithError(http.StatusNotFound, err)
handleDbError(c, err)
return
}

Expand Down
4 changes: 2 additions & 2 deletions server/api/metrics/prometheus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ func PromHandler() gin.HandlerFunc {
header := c.Request.Header.Get("Authorization")

if header == "" {
c.String(401, errInvalidToken.Error())
c.String(http.StatusUnauthorized, errInvalidToken.Error())
return
}

bearer := fmt.Sprintf("Bearer %s", token)

if header != bearer {
c.String(401, errInvalidToken.Error())
c.String(http.StatusForbidden, errInvalidToken.Error())
return
}

Expand Down
Loading