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.

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

[API] Fix inconsistent label color format #10129

Merged
merged 7 commits into from
Feb 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
71 changes: 71 additions & 0 deletions integrations/api_issue_label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package integrations
import (
"fmt"
"net/http"
"strings"
"testing"

"code.gitea.io/gitea/models"
Expand All @@ -15,6 +16,76 @@ import (
"github.com/stretchr/testify/assert"
)

func TestAPIModifyLabels(t *testing.T) {
assert.NoError(t, models.LoadFixtures())

repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 2}).(*models.Repository)
owner := models.AssertExistsAndLoadBean(t, &models.User{ID: repo.OwnerID}).(*models.User)
session := loginUser(t, owner.Name)
token := getTokenForLoggedInUser(t, session)
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/labels?token=%s", owner.Name, repo.Name, token)

// CreateLabel
req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{
Name: "TestL 1",
Color: "abcdef",
Description: "test label",
})
resp := session.MakeRequest(t, req, http.StatusCreated)
apiLabel := new(api.Label)
DecodeJSON(t, resp, &apiLabel)
dbLabel := models.AssertExistsAndLoadBean(t, &models.Label{ID: apiLabel.ID, RepoID: repo.ID}).(*models.Label)
assert.EqualValues(t, dbLabel.Name, apiLabel.Name)
assert.EqualValues(t, strings.TrimLeft(dbLabel.Color, "#"), apiLabel.Color)

req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{
Name: "TestL 2",
Color: "#123456",
Description: "jet another test label",
})
session.MakeRequest(t, req, http.StatusCreated)
req = NewRequestWithJSON(t, "POST", urlStr, &api.CreateLabelOption{
Name: "WrongTestL",
Color: "#12345g",
})
session.MakeRequest(t, req, http.StatusUnprocessableEntity)

//ListLabels
req = NewRequest(t, "GET", urlStr)
resp = session.MakeRequest(t, req, http.StatusOK)
var apiLabels []*api.Label
DecodeJSON(t, resp, &apiLabels)
assert.Len(t, apiLabels, 2)

//GetLabel
singleURLStr := fmt.Sprintf("/api/v1/repos/%s/%s/labels/%d?token=%s", owner.Name, repo.Name, dbLabel.ID, token)
req = NewRequest(t, "GET", singleURLStr)
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiLabel)
assert.EqualValues(t, strings.TrimLeft(dbLabel.Color, "#"), apiLabel.Color)

//EditLabel
newName := "LabelNewName"
newColor := "09876a"
newColorWrong := "09g76a"
req = NewRequestWithJSON(t, "PATCH", singleURLStr, &api.EditLabelOption{
Name: &newName,
Color: &newColor,
})
resp = session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &apiLabel)
assert.EqualValues(t, newColor, apiLabel.Color)
req = NewRequestWithJSON(t, "PATCH", singleURLStr, &api.EditLabelOption{
Color: &newColorWrong,
})
session.MakeRequest(t, req, http.StatusUnprocessableEntity)

//DeleteLabel
req = NewRequest(t, "DELETE", singleURLStr)
resp = session.MakeRequest(t, req, http.StatusNoContent)

}

func TestAPIAddIssueLabels(t *testing.T) {
assert.NoError(t, models.LoadFixtures())

Expand Down
80 changes: 47 additions & 33 deletions models/issue_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,34 @@ import (
"xorm.io/xorm"
)

var labelColorPattern = regexp.MustCompile("#([a-fA-F0-9]{6})")
// LabelColorPattern is a regexp witch can validate LabelColor
var LabelColorPattern = regexp.MustCompile("^#[0-9a-fA-F]{6}$")

// Label represents a label of repository for issues.
type Label struct {
ID int64 `xorm:"pk autoincr"`
RepoID int64 `xorm:"INDEX"`
Name string
Description string
Color string `xorm:"VARCHAR(7)"`
NumIssues int
NumClosedIssues int
NumOpenIssues int `xorm:"-"`
IsChecked bool `xorm:"-"`
QueryString string `xorm:"-"`
IsSelected bool `xorm:"-"`
IsExcluded bool `xorm:"-"`
}

// APIFormat converts a Label to the api.Label format
func (label *Label) APIFormat() *api.Label {
return &api.Label{
ID: label.ID,
Name: label.Name,
Color: strings.TrimLeft(label.Color, "#"),
Description: label.Description,
}
}

// GetLabelTemplateFile loads the label template file by given name,
// then parses and returns a list of name-color pairs and optionally description.
Expand All @@ -43,7 +70,11 @@ func GetLabelTemplateFile(name string) ([][3]string, error) {
return nil, fmt.Errorf("line is malformed: %s", line)
}

if !labelColorPattern.MatchString(fields[0]) {
color := strings.Trim(fields[0], " ")
if len(color) == 6 {
color = "#" + color
}
if !LabelColorPattern.MatchString(color) {
return nil, fmt.Errorf("bad HTML color code in line: %s", line)
}

Expand All @@ -54,38 +85,12 @@ func GetLabelTemplateFile(name string) ([][3]string, error) {
}

fields[1] = strings.TrimSpace(fields[1])
list = append(list, [3]string{fields[1], fields[0], description})
list = append(list, [3]string{fields[1], color, description})
}

return list, nil
}

// Label represents a label of repository for issues.
type Label struct {
ID int64 `xorm:"pk autoincr"`
RepoID int64 `xorm:"INDEX"`
Name string
Description string
Color string `xorm:"VARCHAR(7)"`
NumIssues int
NumClosedIssues int
NumOpenIssues int `xorm:"-"`
IsChecked bool `xorm:"-"`
QueryString string `xorm:"-"`
IsSelected bool `xorm:"-"`
IsExcluded bool `xorm:"-"`
}

// APIFormat converts a Label to the api.Label format
func (label *Label) APIFormat() *api.Label {
return &api.Label{
ID: label.ID,
Name: label.Name,
Color: strings.TrimLeft(label.Color, "#"),
Description: label.Description,
}
}

// CalOpenIssues calculates the open issues of label.
func (label *Label) CalOpenIssues() {
label.NumOpenIssues = label.NumIssues - label.NumClosedIssues
Expand Down Expand Up @@ -152,7 +157,7 @@ func LoadLabelsFormatted(labelTemplate string) (string, error) {
return strings.Join(labels, ", "), err
}

func initalizeLabels(e Engine, repoID int64, labelTemplate string) error {
func initializeLabels(e Engine, repoID int64, labelTemplate string) error {
list, err := GetLabelTemplateFile(labelTemplate)
if err != nil {
return ErrIssueLabelTemplateLoad{labelTemplate, err}
Expand All @@ -175,9 +180,9 @@ func initalizeLabels(e Engine, repoID int64, labelTemplate string) error {
return nil
}

// InitalizeLabels adds a label set to a repository using a template
func InitalizeLabels(ctx DBContext, repoID int64, labelTemplate string) error {
return initalizeLabels(ctx.e, repoID, labelTemplate)
// InitializeLabels adds a label set to a repository using a template
func InitializeLabels(ctx DBContext, repoID int64, labelTemplate string) error {
return initializeLabels(ctx.e, repoID, labelTemplate)
}

func newLabel(e Engine, label *Label) error {
Expand All @@ -187,6 +192,9 @@ func newLabel(e Engine, label *Label) error {

// NewLabel creates a new label for a repository
func NewLabel(label *Label) error {
if !LabelColorPattern.MatchString(label.Color) {
return fmt.Errorf("bad color code: %s", label.Color)
}
return newLabel(x, label)
}

Expand All @@ -198,6 +206,9 @@ func NewLabels(labels ...*Label) error {
return err
}
for _, label := range labels {
if !LabelColorPattern.MatchString(label.Color) {
return fmt.Errorf("bad color code: %s", label.Color)
}
if err := newLabel(sess, label); err != nil {
return err
}
Expand Down Expand Up @@ -359,6 +370,9 @@ func updateLabel(e Engine, l *Label) error {

// UpdateLabel updates label information.
func UpdateLabel(l *Label) error {
if !LabelColorPattern.MatchString(l.Color) {
return fmt.Errorf("bad color code: %s", l.Color)
}
return updateLabel(x, l)
}

Expand Down
5 changes: 4 additions & 1 deletion models/issue_label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ func TestNewLabels(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
labels := []*Label{
{RepoID: 2, Name: "labelName2", Color: "#123456"},
{RepoID: 3, Name: "labelName3", Color: "#234567"},
{RepoID: 3, Name: "labelName3", Color: "#23456F"},
}
assert.Error(t, NewLabel(&Label{RepoID: 3, Name: "invalid Color", Color: ""}))
assert.Error(t, NewLabel(&Label{RepoID: 3, Name: "invalid Color", Color: "123456"}))
assert.Error(t, NewLabel(&Label{RepoID: 3, Name: "invalid Color", Color: "#12345G"}))
for _, label := range labels {
AssertNotExistsBean(t, label)
}
Expand Down
2 changes: 1 addition & 1 deletion models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,7 @@ func ChangeUserName(u *User, newUserName string) (err error) {
} else if isExist {
return ErrUserAlreadyExist{newUserName}
}

sess := x.NewSession()
defer sess.Close()
if err = sess.Begin(); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions modules/repository/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (_ *m

// Initialize Issue Labels if selected
if len(opts.IssueLabels) > 0 {
if err = models.InitalizeLabels(ctx, repo.ID, opts.IssueLabels); err != nil {
return fmt.Errorf("initalizeLabels: %v", err)
if err = models.InitializeLabels(ctx, repo.ID, opts.IssueLabels); err != nil {
return fmt.Errorf("InitializeLabels: %v", err)
}
}

if stdout, err := git.NewCommand("update-server-info").
SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)).
RunInDir(repoPath); err != nil {
log.Error("CreateRepitory(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
log.Error("CreateRepository(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
return fmt.Errorf("CreateRepository(git update-server-info): %v", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/structs/issue_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type CreateLabelOption struct {
Name string `json:"name" binding:"Required"`
// required:true
// example: #00aabb
Color string `json:"color" binding:"Required;Size(7)"`
Color string `json:"color" binding:"Required"`
Description string `json:"description"`
}

Expand Down
24 changes: 23 additions & 1 deletion routers/api/v1/repo/label.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
package repo

import (
"fmt"
"net/http"
"strconv"
"strings"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/context"
Expand Down Expand Up @@ -135,6 +137,17 @@ func CreateLabel(ctx *context.APIContext, form api.CreateLabelOption) {
// responses:
// "201":
// "$ref": "#/responses/Label"
// "422":
// "$ref": "#/responses/validationError"

form.Color = strings.Trim(form.Color, " ")
if len(form.Color) == 6 {
form.Color = "#" + form.Color
}
if !models.LabelColorPattern.MatchString(form.Color) {
ctx.Error(http.StatusUnprocessableEntity, "ColorPattern", fmt.Errorf("bad color code: %s", form.Color))
return
}

label := &models.Label{
Name: form.Name,
Expand Down Expand Up @@ -182,6 +195,8 @@ func EditLabel(ctx *context.APIContext, form api.EditLabelOption) {
// responses:
// "200":
// "$ref": "#/responses/Label"
// "422":
// "$ref": "#/responses/validationError"

label, err := models.GetLabelInRepoByID(ctx.Repo.Repository.ID, ctx.ParamsInt64(":id"))
if err != nil {
Expand All @@ -197,7 +212,14 @@ func EditLabel(ctx *context.APIContext, form api.EditLabelOption) {
label.Name = *form.Name
}
if form.Color != nil {
label.Color = *form.Color
label.Color = strings.Trim(*form.Color, " ")
if len(label.Color) == 6 {
label.Color = "#" + label.Color
}
if !models.LabelColorPattern.MatchString(label.Color) {
ctx.Error(http.StatusUnprocessableEntity, "ColorPattern", fmt.Errorf("bad color code: %s", label.Color))
return
}
}
if form.Description != nil {
label.Description = *form.Description
Expand Down
4 changes: 2 additions & 2 deletions routers/repo/issue_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ func InitializeLabels(ctx *context.Context, form auth.InitializeLabelsForm) {
return
}

if err := models.InitalizeLabels(models.DefaultDBContext(), ctx.Repo.Repository.ID, form.TemplateName); err != nil {
if err := models.InitializeLabels(models.DefaultDBContext(), ctx.Repo.Repository.ID, form.TemplateName); err != nil {
if models.IsErrIssueLabelTemplateLoad(err) {
originalErr := err.(models.ErrIssueLabelTemplateLoad).OriginalError
ctx.Flash.Error(ctx.Tr("repo.issues.label_templates.fail_to_load_file", form.TemplateName, originalErr))
ctx.Redirect(ctx.Repo.RepoLink + "/labels")
return
}
ctx.ServerError("InitalizeLabels", err)
ctx.ServerError("InitializeLabels", err)
return
}
ctx.Redirect(ctx.Repo.RepoLink + "/labels")
Expand Down
6 changes: 6 additions & 0 deletions templates/swagger/v1_json.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -5317,6 +5317,9 @@
"responses": {
"201": {
"$ref": "#/responses/Label"
},
"422": {
"$ref": "#/responses/validationError"
}
}
}
Expand Down Expand Up @@ -5443,6 +5446,9 @@
"responses": {
"200": {
"$ref": "#/responses/Label"
},
"422": {
"$ref": "#/responses/validationError"
}
}
}
Expand Down