Skip to content

Commit 4a1c988

Browse files
author
Oppodelldog
committed
moved validation logic into its own scope, and cleanup
1 parent dc41608 commit 4a1c988

File tree

16 files changed

+216
-152
lines changed

16 files changed

+216
-152
lines changed

config/configuration_test.go

Lines changed: 0 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -44,76 +44,6 @@ func TestGetProjectConfigurationByName_NotFound(t *testing.T) {
4444
assert.Contains(t, err.Error(), "not found")
4545
}
4646

47-
func TestValidate(t *testing.T) {
48-
49-
testDataSet := []struct {
50-
BranchName string
51-
CommitMessage string
52-
ErrorContains string
53-
}{
54-
{
55-
BranchName: "branch1",
56-
CommitMessage: "commit message that contains A.",
57-
ErrorContains: "",
58-
},
59-
{
60-
BranchName: "branch1",
61-
CommitMessage: "commit message that contains B.",
62-
ErrorContains: "",
63-
},
64-
{
65-
BranchName: "branch2",
66-
CommitMessage: "commit message that contains A.",
67-
ErrorContains: "",
68-
},
69-
{
70-
BranchName: "branch2",
71-
CommitMessage: "commit message that contains B.",
72-
ErrorContains: "",
73-
},
74-
{
75-
BranchName: "branch3",
76-
CommitMessage: "does not matter what this contains, since there are no validator for branch3",
77-
ErrorContains: "",
78-
},
79-
{
80-
BranchName: "branch1",
81-
CommitMessage: "commit message that contains Z.",
82-
ErrorContains: "validation error for branch 'branch1'",
83-
},
84-
}
85-
86-
for testName, testData := range testDataSet {
87-
t.Run(string(testName), func(t *testing.T) {
88-
89-
cfg := &Project{
90-
BranchTypes: map[string]BranchTypePattern{
91-
"branch1": `^branch1$`,
92-
"branch2": `^branch2$`,
93-
"branch3": `^branch3$`,
94-
},
95-
Validation: map[string]BranchValidationConfiguration{
96-
"branch1": map[string]string{
97-
"(A)": "contains B",
98-
"(B)": "contains A",
99-
},
100-
"branch2": map[string]string{
101-
"([AB])": "contains A or B (one or multiple)",
102-
},
103-
"branch3": map[string]string{},
104-
},
105-
}
106-
107-
err := cfg.Validate(testData.BranchName, testData.CommitMessage)
108-
if testData.ErrorContains != "" {
109-
assert.Contains(t, err.Error(), testData.ErrorContains)
110-
} else {
111-
assert.NoError(t, err)
112-
}
113-
})
114-
}
115-
}
116-
11747
func createTestConfiguration() Configuration {
11848
cfg := Configuration{
11949
"test1": Project{Path: "/home"},

config/parsing_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func TestParse(t *testing.T) {
2626
"*": `{.BranchName}: {.CommitMessage}`,
2727
},
2828
Validation: map[string]BranchValidationConfiguration{
29-
"feature": {
29+
"*": {
3030
`(?m)(?:\s|^|/)(([A-Z](_)*)+-[0-9]+)([\s,;:!.-]|$)`: "valid ticket ID",
3131
},
3232
"develop": {

config/project.go

Lines changed: 5 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
package config
22

33
import (
4-
"bytes"
5-
"errors"
6-
"fmt"
7-
84
"github.com/glenn-brown/golang-pkg-pcre/src/pkg/pcre"
95
)
106

@@ -35,11 +31,9 @@ func (projConf *Project) GetBranchType(branchName string) string {
3531
return ""
3632
}
3733

38-
func regexMatchesString(pattern string, branchName string) bool {
39-
return pcre.MustCompile(pattern, 0).MatcherString(branchName, 0).Matches()
40-
}
41-
42-
func (projConf *Project) getValidators(branchType string) map[string]string {
34+
// GetValidator returns the validator that matches the given branch type
35+
// if no
36+
func (projConf *Project) GetValidator(branchType string) map[string]string {
4337
var foundValidators map[string]string
4438
for configBranchName, validators := range projConf.Validation {
4539
if configBranchName == branchType || configBranchName == "*" && foundValidators == nil {
@@ -50,36 +44,6 @@ func (projConf *Project) getValidators(branchType string) map[string]string {
5044
return foundValidators
5145
}
5246

53-
// Validate validates the given commitMessage. It uses the validations configured for the given branchName.
54-
// As soon as one validation check succeeds, the validation passes.
55-
func (projConf *Project) Validate(branchName string, commitMessage string) error {
56-
57-
branchType := projConf.GetBranchType(branchName)
58-
validators := projConf.getValidators(branchType)
59-
if len(validators) == 0 {
60-
return nil
61-
}
62-
63-
for validationPattern := range validators {
64-
if regexMatchesString(validationPattern, commitMessage) {
65-
return nil
66-
}
67-
}
68-
69-
return prepareError(branchName, validators)
70-
71-
}
72-
73-
func prepareError(branchName string, validators map[string]string) error {
74-
buffer := bytes.NewBufferString("validation error for branch ")
75-
buffer.WriteString(fmt.Sprintf("'%s'\n", branchName))
76-
buffer.WriteString("at least expected one of the following to match\n")
77-
78-
for _, validationDescription := range validators {
79-
buffer.WriteString(" - ")
80-
buffer.WriteString(validationDescription)
81-
buffer.WriteString("\n")
82-
}
83-
84-
return errors.New(buffer.String())
47+
func regexMatchesString(pattern string, branchName string) bool {
48+
return pcre.MustCompile(pattern, 0).MatcherString(branchName, 0).Matches()
8549
}

config/test-data.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
# define validation rules per branch type
2020
# for example "commit messages in feature branches should always contain a ticket reference"
2121
validation:
22-
feature:
23-
"(?m)(?:\\s|^|/)(([A-Z](_)*)+-[0-9]+)([\\s,;:!.-]|$)" : "valid ticket ID"
2422
develop:
2523
"(?m)(?:\\s|^|/)(([A-Z](_)*)+-[0-9]+)([\\s,;:!.-]|$)" : "valid ticket ID"
2624
"(?m)@noissue" : "@noissue"
@@ -31,3 +29,5 @@
3129
master:
3230
"(?m)(?:\\s|^|/)(([A-Z](_)*)+-[0-9]+)([\\s,;:!.-]|$)" : "valid ticket ID"
3331
"(?m)@noissue" : "@noissue"
32+
"*":
33+
"(?m)(?:\\s|^|/)(([A-Z](_)*)+-[0-9]+)([\\s,;:!.-]|$)" : "valid ticket ID (fallback validator)"

hook/modifier.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,15 @@ type (
1818

1919
createViewModelFuncDef func(gitCommitMessage string, branchName string) ViewModel
2020
validateCommitMessageFuncDef func(branchName, modifiedCommitMessage string) error
21-
renderCommitMessageFuncDef func(branchName string, viewModel ViewModel) (string, error)
21+
renderCommitMessageFuncDef func(viewModel ViewModel) (string, error)
2222
)
2323

2424
// NewCommitMessageModifier create a CommitMessageModifier
2525
func NewCommitMessageModifier(projectConfiguration config.Project) CommitMessageModifier {
26-
commitMessageRenderer := &CommitMessageRenderer{projectConfiguration}
2726
return &commitMessageModifier{
2827
createViewModelFunc: createViewModel,
29-
renderCommitMessageFunc: commitMessageRenderer.RenderCommitMessage,
30-
validateCommitmessageFunc: projectConfiguration.Validate,
28+
renderCommitMessageFunc: NewCommitMessageRenderer(projectConfiguration).Render,
29+
validateCommitmessageFunc: NewCommitMessageValidator(projectConfiguration).Validate,
3130
}
3231
}
3332

@@ -49,7 +48,7 @@ func (m *commitMessageModifier) ModifyGitCommitMessage(gitCommitMessage string,
4948

5049
viewModel := createViewModel(gitCommitMessage, branchName)
5150

52-
modifiedCommitMessage, err = m.renderCommitMessageFunc(branchName, viewModel)
51+
modifiedCommitMessage, err = m.renderCommitMessageFunc(viewModel)
5352
if err != nil {
5453
return
5554
}

hook/modifier_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func TestModifyGitCommitMessage_commitMessageRendererReturnsError_ExpectError(t
9696
branchName := "feature123"
9797
errStub := errors.New("stubbed renderer error")
9898
modifier := NewCommitMessageModifier(config.Project{})
99-
modifier.(*commitMessageModifier).renderCommitMessageFunc = func(branchName string, viewModel ViewModel) (string, error) {
99+
modifier.(*commitMessageModifier).renderCommitMessageFunc = func(viewModel ViewModel) (string, error) {
100100
return "", errStub
101101
}
102102
_, err := modifier.ModifyGitCommitMessage(commitMessage, branchName)

hook/renderer.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,26 @@ import (
77
"github.com/Oppodelldog/git-commit-hook/config"
88
)
99

10-
// CommitMessageRenderer renders a commit message by resolving the branch names template from project configuration
11-
type CommitMessageRenderer struct {
10+
// CommitMessageRenderer implements rendering a commit message by a given
11+
type CommitMessageRenderer interface {
12+
Render(viewModel ViewModel) (string, error)
13+
}
14+
15+
//NewCommitMessageRenderer create a new CommitMessageRenderer
16+
func NewCommitMessageRenderer(projConf config.Project) CommitMessageRenderer {
17+
return &commitMessageRenderer{
18+
projConf: projConf,
19+
}
20+
}
21+
22+
// commitMessageRenderer renders a commit message by resolving the branch names template from project configuration
23+
type commitMessageRenderer struct {
1224
projConf config.Project
1325
}
1426

15-
// RenderCommitMessage renders a commit message using the template defined for the given resolveBranchNameFunc
16-
func (r *CommitMessageRenderer) RenderCommitMessage(branchName string, viewModel ViewModel) (string, error) {
17-
branchType := r.projConf.GetBranchType(branchName)
27+
// Render renders a commit message using the template defined for the given resolveBranchNameFunc
28+
func (r *commitMessageRenderer) Render(viewModel ViewModel) (string, error) {
29+
branchType := r.projConf.GetBranchType(viewModel.BranchName)
1830
commitMessageTemplate := r.getTemplate(branchType)
1931
if commitMessageTemplate == "" {
2032
commitMessageTemplate = getFallbackCommitMessageTemplate()
@@ -33,7 +45,7 @@ func getFallbackCommitMessageTemplate() string {
3345
return "{{.CommitMessage}}"
3446
}
3547

36-
func (r *CommitMessageRenderer) getTemplate(branchType string) string {
48+
func (r *commitMessageRenderer) getTemplate(branchType string) string {
3749
foundTemplate := ""
3850
for configBranchType, branchTypeTemplate := range r.projConf.Templates {
3951
if configBranchType == branchType || configBranchType == "*" && foundTemplate == "" {

hook/renderer_test.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
)
99

1010
func TestRenderCommitMessage(t *testing.T) {
11-
configBranchName := "feature"
1211
viewModel := ViewModel{
1312
BranchName: "feature/PRJ_TEST-1242",
1413
CommitMessage: "initial commit",
@@ -20,18 +19,17 @@ func TestRenderCommitMessage(t *testing.T) {
2019
Templates: map[string]config.BranchTypeTemplate{
2120
"feature": "{{.BranchName}}: {{.CommitMessage}}",
2221
}}
23-
renderer := CommitMessageRenderer{*cfg}
24-
modifiedCommitMessage, err := renderer.RenderCommitMessage(configBranchName, viewModel)
22+
renderer := commitMessageRenderer{*cfg}
23+
modifiedCommitMessage, err := renderer.Render(viewModel)
2524
if err != nil {
26-
t.Fatalf("Did not expect RenderCommitMessage to return an error, but got: %v ", err)
25+
t.Fatalf("Did not expect Render to return an error, but got: %v ", err)
2726
}
2827

2928
expectedCommitMessage := "feature/PRJ_TEST-1242: initial commit"
3029
assert.Exactly(t, expectedCommitMessage, modifiedCommitMessage)
3130
}
3231

3332
func TestRenderCommitMessage_InvalidTemplate_ReturnsError(t *testing.T) {
34-
configBranchName := "feature"
3533
invalidTemplate := "{{{{{ HELLO"
3634
viewModel := ViewModel{}
3735
cfg := &config.Project{
@@ -42,15 +40,14 @@ func TestRenderCommitMessage_InvalidTemplate_ReturnsError(t *testing.T) {
4240
"feature": config.BranchTypeTemplate(invalidTemplate),
4341
}}
4442

45-
renderer := CommitMessageRenderer{*cfg}
46-
_, err := renderer.RenderCommitMessage(configBranchName, viewModel)
43+
renderer := commitMessageRenderer{*cfg}
44+
_, err := renderer.Render(viewModel)
4745

4846
assert.Contains(t, err.Error(), "template:")
4947
}
5048

5149
func TestRenderCommitMessage_NoTemplateFound_PassesBackTheGivenCommitMessage(t *testing.T) {
5250
givenCommitMessage := "some commit message"
53-
configBranchName := "feature"
5451
viewModel := ViewModel{
5552
CommitMessage: givenCommitMessage,
5653
}
@@ -60,10 +57,10 @@ func TestRenderCommitMessage_NoTemplateFound_PassesBackTheGivenCommitMessage(t *
6057
},
6158
Templates: map[string]config.BranchTypeTemplate{}}
6259

63-
renderer := CommitMessageRenderer{*cfg}
64-
modifiedCommitMessage, err := renderer.RenderCommitMessage(configBranchName, viewModel)
60+
renderer := commitMessageRenderer{*cfg}
61+
modifiedCommitMessage, err := renderer.Render(viewModel)
6562
if err != nil {
66-
t.Fatalf("Did not expect RenderCommitMessage to return an error, but got: %v ", err)
63+
t.Fatalf("Did not expect Render to return an error, but got: %v ", err)
6764
}
6865

6966
assert.Exactly(t, givenCommitMessage, modifiedCommitMessage)
@@ -78,7 +75,7 @@ func TestGetTemplate(t *testing.T) {
7875
"branch4": "templ4",
7976
},
8077
}
81-
renderer := CommitMessageRenderer{*cfg}
78+
renderer := commitMessageRenderer{*cfg}
8279
template := renderer.getTemplate("branch2")
8380
assert.Exactly(t, "templ2", template)
8481

@@ -91,18 +88,17 @@ func TestGetTemplate(t *testing.T) {
9188

9289
func TestRenderCommitMessage_NoBranchConfiguration_PassesBackTheGivenCommitMessage(t *testing.T) {
9390
givenCommitMessage := "some commit message"
94-
configBranchName := "feature"
9591
viewModel := ViewModel{
9692
CommitMessage: givenCommitMessage,
9793
}
9894
cfg := &config.Project{
9995
BranchTypes: map[string]config.BranchTypePattern{},
10096
Templates: map[string]config.BranchTypeTemplate{}}
10197

102-
renderer := CommitMessageRenderer{*cfg}
103-
modifiedCommitMessage, err := renderer.RenderCommitMessage(configBranchName, viewModel)
98+
renderer := commitMessageRenderer{*cfg}
99+
modifiedCommitMessage, err := renderer.Render(viewModel)
104100
if err != nil {
105-
t.Fatalf("Did not expect RenderCommitMessage to return an error, but got: %v ", err)
101+
t.Fatalf("Did not expect Render to return an error, but got: %v ", err)
106102
}
107103

108104
assert.Exactly(t, givenCommitMessage, modifiedCommitMessage)

0 commit comments

Comments
 (0)