From 386fbcf8fa2b0eed62b28a2bf94be972e07adf30 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 14:11:24 +0800 Subject: [PATCH 01/10] test: case for single label --- modules/issue/template/template_test.go | 216 +++++++++++++----------- 1 file changed, 120 insertions(+), 96 deletions(-) diff --git a/modules/issue/template/template_test.go b/modules/issue/template/template_test.go index 883e1e0780510..93e254967285e 100644 --- a/modules/issue/template/template_test.go +++ b/modules/issue/template/template_test.go @@ -6,17 +6,18 @@ package template import ( "net/url" - "reflect" "testing" - "code.gitea.io/gitea/modules/json" api "code.gitea.io/gitea/modules/structs" + + "github.com/stretchr/testify/assert" ) func TestValidate(t *testing.T) { tests := []struct { name string content string + want *api.IssueTemplate wantErr string }{ { @@ -316,21 +317,9 @@ body: `, wantErr: "body[0](checkboxes), option[0]: 'required' should be a bool", }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tmpl, err := unmarshal("test.yaml", []byte(tt.content)) - if err != nil { - t.Fatal(err) - } - if err := Validate(tmpl); (err == nil) != (tt.wantErr == "") || err != nil && err.Error() != tt.wantErr { - t.Errorf("Validate() error = %v, wantErr %q", err, tt.wantErr) - } - }) - } - - t.Run("valid", func(t *testing.T) { - content := ` + { + name: "valid", + content: ` name: Name title: Title about: About @@ -386,96 +375,131 @@ body: required: false - label: Option 3 of checkboxes required: true -` - want := &api.IssueTemplate{ - Name: "Name", - Title: "Title", - About: "About", - Labels: []string{"label1", "label2"}, - Ref: "Ref", - Fields: []*api.IssueFormField{ - { - Type: "markdown", - ID: "id1", - Attributes: map[string]interface{}{ - "value": "Value of the markdown", - }, - }, - { - Type: "textarea", - ID: "id2", - Attributes: map[string]interface{}{ - "label": "Label of textarea", - "description": "Description of textarea", - "placeholder": "Placeholder of textarea", - "value": "Value of textarea", - "render": "bash", - }, - Validations: map[string]interface{}{ - "required": true, +`, + want: &api.IssueTemplate{ + Name: "Name", + Title: "Title", + About: "About", + Labels: []string{"label1", "label2"}, + Ref: "Ref", + Fields: []*api.IssueFormField{ + { + Type: "markdown", + ID: "id1", + Attributes: map[string]interface{}{ + "value": "Value of the markdown", + }, }, - }, - { - Type: "input", - ID: "id3", - Attributes: map[string]interface{}{ - "label": "Label of input", - "description": "Description of input", - "placeholder": "Placeholder of input", - "value": "Value of input", + { + Type: "textarea", + ID: "id2", + Attributes: map[string]interface{}{ + "label": "Label of textarea", + "description": "Description of textarea", + "placeholder": "Placeholder of textarea", + "value": "Value of textarea", + "render": "bash", + }, + Validations: map[string]interface{}{ + "required": true, + }, }, - Validations: map[string]interface{}{ - "required": true, - "is_number": true, - "regex": "[a-zA-Z0-9]+", + { + Type: "input", + ID: "id3", + Attributes: map[string]interface{}{ + "label": "Label of input", + "description": "Description of input", + "placeholder": "Placeholder of input", + "value": "Value of input", + }, + Validations: map[string]interface{}{ + "required": true, + "is_number": true, + "regex": "[a-zA-Z0-9]+", + }, }, - }, - { - Type: "dropdown", - ID: "id4", - Attributes: map[string]interface{}{ - "label": "Label of dropdown", - "description": "Description of dropdown", - "multiple": true, - "options": []interface{}{ - "Option 1 of dropdown", - "Option 2 of dropdown", - "Option 3 of dropdown", + { + Type: "dropdown", + ID: "id4", + Attributes: map[string]interface{}{ + "label": "Label of dropdown", + "description": "Description of dropdown", + "multiple": true, + "options": []interface{}{ + "Option 1 of dropdown", + "Option 2 of dropdown", + "Option 3 of dropdown", + }, + }, + Validations: map[string]interface{}{ + "required": true, }, }, - Validations: map[string]interface{}{ - "required": true, + { + Type: "checkboxes", + ID: "id5", + Attributes: map[string]interface{}{ + "label": "Label of checkboxes", + "description": "Description of checkboxes", + "options": []interface{}{ + map[interface{}]interface{}{"label": "Option 1 of checkboxes", "required": true}, + map[interface{}]interface{}{"label": "Option 2 of checkboxes", "required": false}, + map[interface{}]interface{}{"label": "Option 3 of checkboxes", "required": true}, + }, + }, }, }, - { - Type: "checkboxes", - ID: "id5", - Attributes: map[string]interface{}{ - "label": "Label of checkboxes", - "description": "Description of checkboxes", - "options": []interface{}{ - map[interface{}]interface{}{"label": "Option 1 of checkboxes", "required": true}, - map[interface{}]interface{}{"label": "Option 2 of checkboxes", "required": false}, - map[interface{}]interface{}{"label": "Option 3 of checkboxes", "required": true}, + FileName: "test.yaml", + }, + wantErr: "", + }, + { + name: "single_label", + content: ` +name: Name +title: Title +about: About +labels: label1 +ref: Ref +body: + - type: markdown + id: id1 + attributes: + value: Value of the markdown +`, + want: &api.IssueTemplate{ + Name: "Name", + Title: "Title", + About: "About", + Labels: []string{"label1"}, + Ref: "Ref", + Fields: []*api.IssueFormField{ + { + Type: "markdown", + ID: "id1", + Attributes: map[string]interface{}{ + "value": "Value of the markdown", }, }, }, + FileName: "test.yaml", }, - FileName: "test.yaml", - } - got, err := unmarshal("test.yaml", []byte(content)) - if err != nil { - t.Fatal(err) - } - if err := Validate(got); err != nil { - t.Errorf("Validate() error = %v", err) - } - if !reflect.DeepEqual(want, got) { - jsonWant, _ := json.Marshal(want) - jsonGot, _ := json.Marshal(got) - t.Errorf("want:\n%s\ngot:\n%s", jsonWant, jsonGot) - } - }) + wantErr: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpl, err := unmarshal("test.yaml", []byte(tt.content)) + if assert.NoError(t, err) { + if tt.wantErr != "" { + assert.EqualError(t, Validate(tmpl), tt.wantErr) + } else if assert.NoError(t, Validate(tmpl)) { + assert.Equal(t, tt.want, tmpl) + } + } + }) + } } func TestRenderToMarkdown(t *testing.T) { From 56a630b7073189f7acbd0659dab15e4604d9a6b7 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 14:53:54 +0800 Subject: [PATCH 02/10] chore: switch to yaml.v3 --- modules/issue/template/template.go | 4 ++-- modules/issue/template/template_test.go | 6 +++--- modules/issue/template/unmarshal.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/issue/template/template.go b/modules/issue/template/template.go index 3b33852cb58d5..0bdf5a198746d 100644 --- a/modules/issue/template/template.go +++ b/modules/issue/template/template.go @@ -165,7 +165,7 @@ func validateOptions(field *api.IssueFormField, idx int) error { return position.Errorf("should be a string") } case api.IssueFormFieldTypeCheckboxes: - opt, ok := option.(map[interface{}]interface{}) + opt, ok := option.(map[string]interface{}) if !ok { return position.Errorf("should be a dictionary") } @@ -351,7 +351,7 @@ func (o *valuedOption) Label() string { return label } case api.IssueFormFieldTypeCheckboxes: - if vs, ok := o.data.(map[interface{}]interface{}); ok { + if vs, ok := o.data.(map[string]interface{}); ok { if v, ok := vs["label"].(string); ok { return v } diff --git a/modules/issue/template/template_test.go b/modules/issue/template/template_test.go index 93e254967285e..c8601a2dad06d 100644 --- a/modules/issue/template/template_test.go +++ b/modules/issue/template/template_test.go @@ -443,9 +443,9 @@ body: "label": "Label of checkboxes", "description": "Description of checkboxes", "options": []interface{}{ - map[interface{}]interface{}{"label": "Option 1 of checkboxes", "required": true}, - map[interface{}]interface{}{"label": "Option 2 of checkboxes", "required": false}, - map[interface{}]interface{}{"label": "Option 3 of checkboxes", "required": true}, + map[string]interface{}{"label": "Option 1 of checkboxes", "required": true}, + map[string]interface{}{"label": "Option 2 of checkboxes", "required": false}, + map[string]interface{}{"label": "Option 3 of checkboxes", "required": true}, }, }, }, diff --git a/modules/issue/template/unmarshal.go b/modules/issue/template/unmarshal.go index 24587b0fed2b1..3398719cf687e 100644 --- a/modules/issue/template/unmarshal.go +++ b/modules/issue/template/unmarshal.go @@ -16,7 +16,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" ) // CouldBe indicates a file with the filename could be a template, From 78681d1b261f29bc1d8057be50518b2e2b12f127 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 15:02:01 +0800 Subject: [PATCH 03/10] feat: support single label --- modules/issue/template/template_test.go | 17 ++++++++------- modules/issue/template/unmarshal.go | 28 +++++++++++++++++++++++++ modules/structs/issue.go | 19 ++++++++++------- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/modules/issue/template/template_test.go b/modules/issue/template/template_test.go index c8601a2dad06d..0a2c03134b3de 100644 --- a/modules/issue/template/template_test.go +++ b/modules/issue/template/template_test.go @@ -8,9 +8,10 @@ import ( "net/url" "testing" + "code.gitea.io/gitea/modules/json" api "code.gitea.io/gitea/modules/structs" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidate(t *testing.T) { @@ -491,12 +492,14 @@ body: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tmpl, err := unmarshal("test.yaml", []byte(tt.content)) - if assert.NoError(t, err) { - if tt.wantErr != "" { - assert.EqualError(t, Validate(tmpl), tt.wantErr) - } else if assert.NoError(t, Validate(tmpl)) { - assert.Equal(t, tt.want, tmpl) - } + require.NoError(t, err) + if tt.wantErr != "" { + require.EqualError(t, Validate(tmpl), tt.wantErr) + } else { + require.NoError(t, Validate(tmpl)) + want, _ := json.Marshal(tt.want) + got, _ := json.Marshal(tmpl) + require.JSONEq(t, string(want), string(got)) } }) } diff --git a/modules/issue/template/unmarshal.go b/modules/issue/template/unmarshal.go index 3398719cf687e..c66b429b59e3b 100644 --- a/modules/issue/template/unmarshal.go +++ b/modules/issue/template/unmarshal.go @@ -123,6 +123,10 @@ func unmarshal(filename string, content []byte) (*api.IssueTemplate, error) { if err := yaml.Unmarshal(content, it); err != nil { return nil, fmt.Errorf("yaml unmarshal: %w", err) } + var err error + if it.Labels, err = decodeLabels(it.LabelsNode); err != nil { + return nil, fmt.Errorf("yaml unmarshal: %w", err) + } if it.About == "" { if err := yaml.Unmarshal(content, compatibleTemplate); err == nil && compatibleTemplate.About != "" { it.About = compatibleTemplate.About @@ -137,3 +141,27 @@ func unmarshal(filename string, content []byte) (*api.IssueTemplate, error) { return it, nil } + +func decodeLabels(node yaml.Node) ([]string, error) { + if node.IsZero() { + return nil, nil + } + switch node.Kind { + case yaml.ScalarNode: + label := "" + err := node.Decode(&label) + if err != nil { + return nil, err + } + return []string{label}, nil + case yaml.SequenceNode: + var labels []string + err := node.Decode(&labels) + if err != nil { + return nil, err + } + return labels, nil + } + return nil, fmt.Errorf("line %d: cannot unmarshal %s%s into []string", + node.Line, node.ShortTag(), node.Value) +} diff --git a/modules/structs/issue.go b/modules/structs/issue.go index 70f5e1ba8eaa1..aa95f28c1f618 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -7,6 +7,8 @@ package structs import ( "path/filepath" "time" + + "gopkg.in/yaml.v3" ) // StateType issue state type @@ -143,14 +145,15 @@ type IssueFormField struct { // IssueTemplate represents an issue template for a repository // swagger:model type IssueTemplate struct { - Name string `json:"name" yaml:"name"` - Title string `json:"title" yaml:"title"` - About string `json:"about" yaml:"about"` // Using "description" in a template file is compatible - Labels []string `json:"labels" yaml:"labels"` - Ref string `json:"ref" yaml:"ref"` - Content string `json:"content" yaml:"-"` - Fields []*IssueFormField `json:"body" yaml:"body"` - FileName string `json:"file_name" yaml:"-"` + Name string `json:"name" yaml:"name"` + Title string `json:"title" yaml:"title"` + About string `json:"about" yaml:"about"` // Using "description" in a template file is compatible + Labels []string `json:"labels" yaml:"-"` + LabelsNode yaml.Node `json:"-" yaml:"labels"` + Ref string `json:"ref" yaml:"ref"` + Content string `json:"content" yaml:"-"` + Fields []*IssueFormField `json:"body" yaml:"body"` + FileName string `json:"file_name" yaml:"-"` } // IssueTemplateType defines issue template type From 824a43c1f0e901fe5d1e12b396ce8e1f051a4acc Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 15:09:46 +0800 Subject: [PATCH 04/10] feat: support comma delimited lables --- modules/issue/template/template_test.go | 35 ++++++++++++++++++++++++- modules/issue/template/unmarshal.go | 7 ++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/modules/issue/template/template_test.go b/modules/issue/template/template_test.go index 0a2c03134b3de..71af92a125bb2 100644 --- a/modules/issue/template/template_test.go +++ b/modules/issue/template/template_test.go @@ -456,7 +456,7 @@ body: wantErr: "", }, { - name: "single_label", + name: "single label", content: ` name: Name title: Title @@ -488,6 +488,39 @@ body: }, wantErr: "", }, + { + name: "comma delimited labels", + content: ` +name: Name +title: Title +about: About +labels: label1,label2 +ref: Ref +body: + - type: markdown + id: id1 + attributes: + value: Value of the markdown +`, + want: &api.IssueTemplate{ + Name: "Name", + Title: "Title", + About: "About", + Labels: []string{"label1", "label2"}, + Ref: "Ref", + Fields: []*api.IssueFormField{ + { + Type: "markdown", + ID: "id1", + Attributes: map[string]interface{}{ + "value": "Value of the markdown", + }, + }, + }, + FileName: "test.yaml", + }, + wantErr: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/modules/issue/template/unmarshal.go b/modules/issue/template/unmarshal.go index c66b429b59e3b..28ac840673e49 100644 --- a/modules/issue/template/unmarshal.go +++ b/modules/issue/template/unmarshal.go @@ -9,6 +9,7 @@ import ( "io" "path/filepath" "strconv" + "strings" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup/markdown" @@ -148,12 +149,12 @@ func decodeLabels(node yaml.Node) ([]string, error) { } switch node.Kind { case yaml.ScalarNode: - label := "" - err := node.Decode(&label) + labels := "" + err := node.Decode(&labels) if err != nil { return nil, err } - return []string{label}, nil + return strings.Split(labels, ","), nil case yaml.SequenceNode: var labels []string err := node.Decode(&labels) From 0be9079df65dccd5aa704cd022b97b1b6877b40d Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 15:37:42 +0800 Subject: [PATCH 05/10] fix: check lables in md template --- modules/issue/template/template_test.go | 39 +++++++++++++++++++++---- modules/issue/template/unmarshal.go | 3 ++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/modules/issue/template/template_test.go b/modules/issue/template/template_test.go index 71af92a125bb2..d4878c7ab81e9 100644 --- a/modules/issue/template/template_test.go +++ b/modules/issue/template/template_test.go @@ -16,10 +16,11 @@ import ( func TestValidate(t *testing.T) { tests := []struct { - name string - content string - want *api.IssueTemplate - wantErr string + name string + filename string + content string + want *api.IssueTemplate + wantErr string }{ { name: "miss name", @@ -521,10 +522,38 @@ body: }, wantErr: "", }, + { + name: "comma delimited labels in markdown", + filename: "test.md", + content: `--- +name: Name +title: Title +about: About +labels: label1,label2 +ref: Ref +--- +Content +`, + want: &api.IssueTemplate{ + Name: "Name", + Title: "Title", + About: "About", + Labels: []string{"label1", "label2"}, + Ref: "Ref", + Fields: nil, + Content: "Content\n", + FileName: "test.md", + }, + wantErr: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tmpl, err := unmarshal("test.yaml", []byte(tt.content)) + filename := "test.yaml" + if tt.filename != "" { + filename = tt.filename + } + tmpl, err := unmarshal(filename, []byte(tt.content)) require.NoError(t, err) if tt.wantErr != "" { require.EqualError(t, Validate(tmpl), tt.wantErr) diff --git a/modules/issue/template/unmarshal.go b/modules/issue/template/unmarshal.go index 28ac840673e49..4401a45563703 100644 --- a/modules/issue/template/unmarshal.go +++ b/modules/issue/template/unmarshal.go @@ -113,6 +113,9 @@ func unmarshal(filename string, content []byte) (*api.IssueTemplate, error) { it.Name = filepath.Base(it.FileName) it.About, _ = util.SplitStringAtByteN(it.Content, 80) } else { + if it.Labels, err = decodeLabels(it.LabelsNode); err != nil { + return nil, fmt.Errorf("yaml unmarshal: %w", err) + } it.Content = templateBody if it.About == "" { if _, err := markdown.ExtractMetadata(string(content), compatibleTemplate); err == nil && compatibleTemplate.About != "" { From 4d6c2bb7e4dd209adea2bb2e62d3a4c18674dee7 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 15:47:23 +0800 Subject: [PATCH 06/10] fix: ignore empty item in comma-delimited labels --- modules/issue/template/template_test.go | 43 ++++++++++++++++++++++--- modules/issue/template/unmarshal.go | 14 +++++--- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/modules/issue/template/template_test.go b/modules/issue/template/template_test.go index d4878c7ab81e9..c3863a64a6e16 100644 --- a/modules/issue/template/template_test.go +++ b/modules/issue/template/template_test.go @@ -490,12 +490,12 @@ body: wantErr: "", }, { - name: "comma delimited labels", + name: "comma-delimited labels", content: ` name: Name title: Title about: About -labels: label1,label2 +labels: label1,label2,,label3 ,, ref: Ref body: - type: markdown @@ -507,7 +507,40 @@ body: Name: "Name", Title: "Title", About: "About", - Labels: []string{"label1", "label2"}, + Labels: []string{"label1", "label2", "label3"}, + Ref: "Ref", + Fields: []*api.IssueFormField{ + { + Type: "markdown", + ID: "id1", + Attributes: map[string]interface{}{ + "value": "Value of the markdown", + }, + }, + }, + FileName: "test.yaml", + }, + wantErr: "", + }, + { + name: "empty string as labels", + content: ` +name: Name +title: Title +about: About +labels: '' +ref: Ref +body: + - type: markdown + id: id1 + attributes: + value: Value of the markdown +`, + want: &api.IssueTemplate{ + Name: "Name", + Title: "Title", + About: "About", + Labels: nil, Ref: "Ref", Fields: []*api.IssueFormField{ { @@ -529,7 +562,7 @@ body: name: Name title: Title about: About -labels: label1,label2 +labels: label1,label2,,label3 ,, ref: Ref --- Content @@ -538,7 +571,7 @@ Content Name: "Name", Title: "Title", About: "About", - Labels: []string{"label1", "label2"}, + Labels: []string{"label1", "label2", "label3"}, Ref: "Ref", Fields: nil, Content: "Content\n", diff --git a/modules/issue/template/unmarshal.go b/modules/issue/template/unmarshal.go index 4401a45563703..f02ce26c1265d 100644 --- a/modules/issue/template/unmarshal.go +++ b/modules/issue/template/unmarshal.go @@ -150,16 +150,22 @@ func decodeLabels(node yaml.Node) ([]string, error) { if node.IsZero() { return nil, nil } + var labels []string switch node.Kind { case yaml.ScalarNode: - labels := "" - err := node.Decode(&labels) + str := "" + err := node.Decode(&str) if err != nil { return nil, err } - return strings.Split(labels, ","), nil + for _, v := range strings.Split(str, ",") { + if v = strings.TrimSpace(v); v == "" { + continue + } + labels = append(labels, v) + } + return labels, nil case yaml.SequenceNode: - var labels []string err := node.Decode(&labels) if err != nil { return nil, err From 1ffbb114701f46b73e08ca62de4afaebcd0a89fe Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 17:53:52 +0800 Subject: [PATCH 07/10] test: don't import structs in unit tests --- modules/markup/markdown/meta_test.go | 48 +++++++++++++++------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/modules/markup/markdown/meta_test.go b/modules/markup/markdown/meta_test.go index 720d0066f4f0b..1e9768e618694 100644 --- a/modules/markup/markdown/meta_test.go +++ b/modules/markup/markdown/meta_test.go @@ -9,82 +9,86 @@ import ( "strings" "testing" - "code.gitea.io/gitea/modules/structs" - "github.com/stretchr/testify/assert" ) -func validateMetadata(it structs.IssueTemplate) bool { - /* - A legacy to keep the unit tests working. - Copied from the method "func (it IssueTemplate) Valid() bool", the original method has been removed. - Because it becomes quite complicated to validate an issue template which is support yaml form now. - The new way to validate an issue template is to call the Validate in modules/issue/template, - */ +/* +IssueTemplate is a legacy to keep the unit tests working. +Copied from structs.IssueTemplate, the original type has been changed a lot to support yaml template. +*/ +type IssueTemplate struct { + Name string `json:"name" yaml:"name"` + Title string `json:"title" yaml:"title"` + About string `json:"about" yaml:"about"` + Labels []string `json:"labels" yaml:"labels"` + Ref string `json:"ref" yaml:"ref"` +} + +func (it *IssueTemplate) Valid() bool { return strings.TrimSpace(it.Name) != "" && strings.TrimSpace(it.About) != "" } func TestExtractMetadata(t *testing.T) { t.Run("ValidFrontAndBody", func(t *testing.T) { - var meta structs.IssueTemplate + var meta IssueTemplate body, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s\n%s", sepTest, frontTest, sepTest, bodyTest), &meta) assert.NoError(t, err) assert.Equal(t, bodyTest, body) assert.Equal(t, metaTest, meta) - assert.True(t, validateMetadata(meta)) + assert.True(t, meta.Valid()) }) t.Run("NoFirstSeparator", func(t *testing.T) { - var meta structs.IssueTemplate + var meta IssueTemplate _, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", frontTest, sepTest, bodyTest), &meta) assert.Error(t, err) }) t.Run("NoLastSeparator", func(t *testing.T) { - var meta structs.IssueTemplate + var meta IssueTemplate _, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, bodyTest), &meta) assert.Error(t, err) }) t.Run("NoBody", func(t *testing.T) { - var meta structs.IssueTemplate + var meta IssueTemplate body, err := ExtractMetadata(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, sepTest), &meta) assert.NoError(t, err) assert.Equal(t, "", body) assert.Equal(t, metaTest, meta) - assert.True(t, validateMetadata(meta)) + assert.True(t, meta.Valid()) }) } func TestExtractMetadataBytes(t *testing.T) { t.Run("ValidFrontAndBody", func(t *testing.T) { - var meta structs.IssueTemplate + var meta IssueTemplate body, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s\n%s", sepTest, frontTest, sepTest, bodyTest)), &meta) assert.NoError(t, err) assert.Equal(t, bodyTest, string(body)) assert.Equal(t, metaTest, meta) - assert.True(t, validateMetadata(meta)) + assert.True(t, meta.Valid()) }) t.Run("NoFirstSeparator", func(t *testing.T) { - var meta structs.IssueTemplate + var meta IssueTemplate _, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", frontTest, sepTest, bodyTest)), &meta) assert.Error(t, err) }) t.Run("NoLastSeparator", func(t *testing.T) { - var meta structs.IssueTemplate + var meta IssueTemplate _, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, bodyTest)), &meta) assert.Error(t, err) }) t.Run("NoBody", func(t *testing.T) { - var meta structs.IssueTemplate + var meta IssueTemplate body, err := ExtractMetadataBytes([]byte(fmt.Sprintf("%s\n%s\n%s", sepTest, frontTest, sepTest)), &meta) assert.NoError(t, err) assert.Equal(t, "", string(body)) assert.Equal(t, metaTest, meta) - assert.True(t, validateMetadata(meta)) + assert.True(t, meta.Valid()) }) } @@ -97,7 +101,7 @@ labels: - bug - "test label"` bodyTest = "This is the body" - metaTest = structs.IssueTemplate{ + metaTest = IssueTemplate{ Name: "Test", About: "A Test", Title: "Test Title", From 1140265d01c1535c511d28dd2519d48ac502d7c1 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 19:51:47 +0800 Subject: [PATCH 08/10] fix: use Unmarshaler --- modules/issue/template/unmarshal.go | 38 --------------------- modules/structs/issue.go | 51 ++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/modules/issue/template/unmarshal.go b/modules/issue/template/unmarshal.go index f02ce26c1265d..3398719cf687e 100644 --- a/modules/issue/template/unmarshal.go +++ b/modules/issue/template/unmarshal.go @@ -9,7 +9,6 @@ import ( "io" "path/filepath" "strconv" - "strings" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup/markdown" @@ -113,9 +112,6 @@ func unmarshal(filename string, content []byte) (*api.IssueTemplate, error) { it.Name = filepath.Base(it.FileName) it.About, _ = util.SplitStringAtByteN(it.Content, 80) } else { - if it.Labels, err = decodeLabels(it.LabelsNode); err != nil { - return nil, fmt.Errorf("yaml unmarshal: %w", err) - } it.Content = templateBody if it.About == "" { if _, err := markdown.ExtractMetadata(string(content), compatibleTemplate); err == nil && compatibleTemplate.About != "" { @@ -127,10 +123,6 @@ func unmarshal(filename string, content []byte) (*api.IssueTemplate, error) { if err := yaml.Unmarshal(content, it); err != nil { return nil, fmt.Errorf("yaml unmarshal: %w", err) } - var err error - if it.Labels, err = decodeLabels(it.LabelsNode); err != nil { - return nil, fmt.Errorf("yaml unmarshal: %w", err) - } if it.About == "" { if err := yaml.Unmarshal(content, compatibleTemplate); err == nil && compatibleTemplate.About != "" { it.About = compatibleTemplate.About @@ -145,33 +137,3 @@ func unmarshal(filename string, content []byte) (*api.IssueTemplate, error) { return it, nil } - -func decodeLabels(node yaml.Node) ([]string, error) { - if node.IsZero() { - return nil, nil - } - var labels []string - switch node.Kind { - case yaml.ScalarNode: - str := "" - err := node.Decode(&str) - if err != nil { - return nil, err - } - for _, v := range strings.Split(str, ",") { - if v = strings.TrimSpace(v); v == "" { - continue - } - labels = append(labels, v) - } - return labels, nil - case yaml.SequenceNode: - err := node.Decode(&labels) - if err != nil { - return nil, err - } - return labels, nil - } - return nil, fmt.Errorf("line %d: cannot unmarshal %s%s into []string", - node.Line, node.ShortTag(), node.Value) -} diff --git a/modules/structs/issue.go b/modules/structs/issue.go index aa95f28c1f618..f338449123d68 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -5,7 +5,9 @@ package structs import ( + "fmt" "path/filepath" + "strings" "time" "gopkg.in/yaml.v3" @@ -145,15 +147,46 @@ type IssueFormField struct { // IssueTemplate represents an issue template for a repository // swagger:model type IssueTemplate struct { - Name string `json:"name" yaml:"name"` - Title string `json:"title" yaml:"title"` - About string `json:"about" yaml:"about"` // Using "description" in a template file is compatible - Labels []string `json:"labels" yaml:"-"` - LabelsNode yaml.Node `json:"-" yaml:"labels"` - Ref string `json:"ref" yaml:"ref"` - Content string `json:"content" yaml:"-"` - Fields []*IssueFormField `json:"body" yaml:"body"` - FileName string `json:"file_name" yaml:"-"` + Name string `json:"name" yaml:"name"` + Title string `json:"title" yaml:"title"` + About string `json:"about" yaml:"about"` // Using "description" in a template file is compatible + Labels IssueTemplateLabels `json:"labels" yaml:"labels"` + Ref string `json:"ref" yaml:"ref"` + Content string `json:"content" yaml:"-"` + Fields []*IssueFormField `json:"body" yaml:"body"` + FileName string `json:"file_name" yaml:"-"` +} + +type IssueTemplateLabels []string + +func (labels *IssueTemplateLabels) UnmarshalYAML(value *yaml.Node) error { + if value.IsZero() { + return nil + } + switch value.Kind { + case yaml.ScalarNode: + str := "" + err := value.Decode(&str) + if err != nil { + return err + } + for _, v := range strings.Split(str, ",") { + if v = strings.TrimSpace(v); v == "" { + continue + } + *labels = append(*labels, v) + } + return nil + case yaml.SequenceNode: + var strs []string + if err := value.Decode(&strs); err != nil { + return err + } + *labels = strs + return nil + } + return fmt.Errorf("line %d: cannot unmarshal %s%s into IssueTemplateLabels", + value.Line, value.ShortTag(), value.Value) } // IssueTemplateType defines issue template type From 40330f086f3eebaae7ad1e24511fc82dea069ab1 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 20:09:43 +0800 Subject: [PATCH 09/10] chore: update swagger --- templates/swagger/v1_json.tmpl | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index fe3185ea77ea4..ddafc146a159c 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -16818,11 +16818,7 @@ "x-go-name": "FileName" }, "labels": { - "type": "array", - "items": { - "type": "string" - }, - "x-go-name": "Labels" + "$ref": "#/definitions/IssueTemplateLabels" }, "name": { "type": "string", @@ -16839,6 +16835,13 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "IssueTemplateLabels": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "Label": { "description": "Label a label to an issue or a pr", "type": "object", From cc8f1c47d968bf97f2759efaed86905e34463af3 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 16 Nov 2022 21:42:41 +0800 Subject: [PATCH 10/10] fix: UnmarshalYAML --- modules/structs/issue.go | 15 +++++---- modules/structs/issue_test.go | 63 +++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/modules/structs/issue.go b/modules/structs/issue.go index 0e4439e0ae37b..45c3f6294ab59 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -159,8 +159,10 @@ type IssueTemplate struct { type IssueTemplateLabels []string -func (labels *IssueTemplateLabels) UnmarshalYAML(value *yaml.Node) error { +func (l *IssueTemplateLabels) UnmarshalYAML(value *yaml.Node) error { + var labels []string if value.IsZero() { + *l = labels return nil } switch value.Kind { @@ -174,19 +176,18 @@ func (labels *IssueTemplateLabels) UnmarshalYAML(value *yaml.Node) error { if v = strings.TrimSpace(v); v == "" { continue } - *labels = append(*labels, v) + labels = append(labels, v) } + *l = labels return nil case yaml.SequenceNode: - var strs []string - if err := value.Decode(&strs); err != nil { + if err := value.Decode(&labels); err != nil { return err } - *labels = strs + *l = labels return nil } - return fmt.Errorf("line %d: cannot unmarshal %s%s into IssueTemplateLabels", - value.Line, value.ShortTag(), value.Value) + return fmt.Errorf("line %d: cannot unmarshal %s into IssueTemplateLabels", value.Line, value.ShortTag()) } // IssueTemplateType defines issue template type diff --git a/modules/structs/issue_test.go b/modules/structs/issue_test.go index 5312585d0f0f4..72b40f7cf24c3 100644 --- a/modules/structs/issue_test.go +++ b/modules/structs/issue_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" ) func TestIssueTemplate_Type(t *testing.T) { @@ -41,3 +42,65 @@ func TestIssueTemplate_Type(t *testing.T) { }) } } + +func TestIssueTemplateLabels_UnmarshalYAML(t *testing.T) { + tests := []struct { + name string + content string + tmpl *IssueTemplate + want *IssueTemplate + wantErr string + }{ + { + name: "array", + content: `labels: ["a", "b", "c"]`, + tmpl: &IssueTemplate{ + Labels: []string{"should_be_overwrote"}, + }, + want: &IssueTemplate{ + Labels: []string{"a", "b", "c"}, + }, + }, + { + name: "string", + content: `labels: "a,b,c"`, + tmpl: &IssueTemplate{ + Labels: []string{"should_be_overwrote"}, + }, + want: &IssueTemplate{ + Labels: []string{"a", "b", "c"}, + }, + }, + { + name: "empty", + content: `labels:`, + tmpl: &IssueTemplate{ + Labels: []string{"should_be_overwrote"}, + }, + want: &IssueTemplate{ + Labels: nil, + }, + }, + { + name: "error", + content: ` +labels: + a: aa + b: bb +`, + tmpl: &IssueTemplate{}, + wantErr: "line 3: cannot unmarshal !!map into IssueTemplateLabels", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := yaml.Unmarshal([]byte(tt.content), tt.tmpl) + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.want, tt.tmpl) + } + }) + } +}