diff --git a/bug/bug.go b/bug/bug.go index e548757e130abef9b0f4c6203d3b26e12f2e36a6..3342ecfaf057719597a20355975d23868b2427ce 100644 --- a/bug/bug.go +++ b/bug/bug.go @@ -2,13 +2,13 @@ package bug import ( "encoding/json" - "errors" "fmt" "strings" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/git" "github.com/MichaelMure/git-bug/util/lamport" + "github.com/pkg/errors" ) const bugsRefPattern = "refs/bugs/" @@ -279,31 +279,31 @@ func refsToIds(refs []string) []string { return ids } -// IsValid check if the Bug data is valid -func (bug *Bug) IsValid() bool { +// Validate check if the Bug data is valid +func (bug *Bug) Validate() error { // non-empty if len(bug.packs) == 0 && bug.staging.IsEmpty() { - return false + return fmt.Errorf("bug has no operations") } - // check if each pack is valid + // check if each pack and operations are valid for _, pack := range bug.packs { - if !pack.IsValid() { - return false + if err := pack.Validate(); err != nil { + return err } } // check if staging is valid if needed if !bug.staging.IsEmpty() { - if !bug.staging.IsValid() { - return false + if err := bug.staging.Validate(); err != nil { + return errors.Wrap(err, "staging") } } // The very first Op should be a CreateOp firstOp := bug.FirstOp() if firstOp == nil || firstOp.OpType() != CreateOp { - return false + return fmt.Errorf("first operation should be a Create op") } // Check that there is no more CreateOp op @@ -316,10 +316,10 @@ func (bug *Bug) IsValid() bool { } if createCount != 1 { - return false + return fmt.Errorf("only one Create op allowed") } - return true + return nil } // Append an operation into the staging area, to be committed later @@ -338,6 +338,10 @@ func (bug *Bug) Commit(repo repository.Repo) error { return fmt.Errorf("can't commit a bug with no pending operation") } + if err := bug.Validate(); err != nil { + return errors.Wrap(err, "can't commit a bug with invalid data") + } + // Write the Ops as a Git blob containing the serialized array hash, err := bug.staging.Write(repo) if err != nil { diff --git a/bug/bug_actions.go b/bug/bug_actions.go index 8bda15b764c1946140c8de5d706a55841774ad37..7f2194aba1f22cd210314d0ccb6180c0c70ecc78 100644 --- a/bug/bug_actions.go +++ b/bug/bug_actions.go @@ -66,8 +66,8 @@ func MergeAll(repo repository.Repo, remote string) <-chan MergeResult { } // Check for error in remote data - if !remoteBug.IsValid() { - out <- newMergeStatus(MergeStatusInvalid, id, nil) + if err := remoteBug.Validate(); err != nil { + out <- newMergeInvalidStatus(id, err.Error()) continue } @@ -128,12 +128,26 @@ const ( MergeStatusNothing ) -func (ms MergeStatus) String() string { - switch ms { +type MergeResult struct { + // Err is set when a terminal error occur in the process + Err error + + Id string + Status MergeStatus + + // Only set for invalid status + Reason string + + // Not set for invalid status + Bug *Bug +} + +func (mr MergeResult) String() string { + switch mr.Status { case MergeStatusNew: return "new" case MergeStatusInvalid: - return "invalid data" + return fmt.Sprintf("invalid data: %s", mr.Reason) case MergeStatusUpdated: return "updated" case MergeStatusNothing: @@ -143,15 +157,6 @@ func (ms MergeStatus) String() string { } } -type MergeResult struct { - // Err is set when a terminal error occur in the process - Err error - - Id string - Status MergeStatus - Bug *Bug -} - func newMergeError(err error, id string) MergeResult { return MergeResult{ Err: err, @@ -168,3 +173,11 @@ func newMergeStatus(status MergeStatus, id string, bug *Bug) MergeResult { Bug: bug, } } + +func newMergeInvalidStatus(id string, reason string) MergeResult { + return MergeResult{ + Id: id, + Status: MergeStatusInvalid, + Reason: reason, + } +} diff --git a/bug/interface.go b/bug/interface.go index c397bfbc81349cca738d91245e46980c3288b6cf..72dee61cd1aaea01f8c05ef36869485dbb864b2f 100644 --- a/bug/interface.go +++ b/bug/interface.go @@ -12,8 +12,8 @@ type Interface interface { // HumanId return the Bug identifier truncated for human consumption HumanId() string - // IsValid check if the Bug data is valid - IsValid() bool + // Validate check if the Bug data is valid + Validate() error // Append an operation into the staging area, to be committed later Append(op Operation) diff --git a/bug/label.go b/bug/label.go index b19a980fed0f168c9864528e08b57fe28c7acdda..b73222ddb86117724e39c0e4c8e9075e2f0ede01 100644 --- a/bug/label.go +++ b/bug/label.go @@ -3,6 +3,9 @@ package bug import ( "fmt" "io" + "strings" + + "github.com/MichaelMure/git-bug/util/text" ) type Label string @@ -27,3 +30,21 @@ func (l *Label) UnmarshalGQL(v interface{}) error { func (l Label) MarshalGQL(w io.Writer) { w.Write([]byte(`"` + l.String() + `"`)) } + +func (l Label) Validate() error { + str := string(l) + + if text.Empty(str) { + return fmt.Errorf("empty") + } + + if strings.Contains(str, "\n") { + return fmt.Errorf("should be a single line") + } + + if !text.Safe(str) { + return fmt.Errorf("not fully printable") + } + + return nil +} diff --git a/bug/operation.go b/bug/operation.go index 6a8aa0cdab06742fa0a19670363933ce06c019ec..b148141c792b0b59d83bf3efaa318f2f64035f4d 100644 --- a/bug/operation.go +++ b/bug/operation.go @@ -2,6 +2,9 @@ package bug import ( "github.com/MichaelMure/git-bug/util/git" + "github.com/pkg/errors" + + "fmt" "time" ) @@ -25,13 +28,14 @@ type Operation interface { Time() time.Time // GetUnixTime return the unix timestamp when the operation was added GetUnixTime() int64 - // Apply the operation to a Snapshot to create the final state - Apply(snapshot Snapshot) Snapshot + // GetAuthor return the author of the operation + GetAuthor() Person // GetFiles return the files needed by this operation GetFiles() []git.Hash - - // TODO: data validation (ex: a title is a single line) - // Validate() bool + // Apply the operation to a Snapshot to create the final state + Apply(snapshot Snapshot) Snapshot + // Validate check if the operation is valid (ex: a title is a single line) + Validate() error } // OpBase implement the common code for all operations @@ -65,7 +69,35 @@ func (op OpBase) GetUnixTime() int64 { return op.UnixTime } +// GetAuthor return the author of the operation +func (op OpBase) GetAuthor() Person { + return op.Author +} + // GetFiles return the files needed by this operation func (op OpBase) GetFiles() []git.Hash { return nil } + +// Validate check the OpBase for errors +func OpBaseValidate(op Operation, opType OperationType) error { + if op.OpType() != opType { + return fmt.Errorf("incorrect operation type (expected: %v, actual: %v)", opType, op.OpType()) + } + + if op.GetUnixTime() == 0 { + return fmt.Errorf("time not set") + } + + if err := op.GetAuthor().Validate(); err != nil { + return errors.Wrap(err, "author") + } + + for _, hash := range op.GetFiles() { + if !hash.IsValid() { + return fmt.Errorf("file with invalid hash %v", hash) + } + } + + return nil +} diff --git a/bug/operation_pack.go b/bug/operation_pack.go index fe26952f194c9e5d2562a39d72fd324bef23cc53..03d538d5f8baab8f8230ddee33220140b0d58c6d 100644 --- a/bug/operation_pack.go +++ b/bug/operation_pack.go @@ -7,6 +7,7 @@ import ( "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/git" + "github.com/pkg/errors" ) const formatVersion = 1 @@ -24,8 +25,10 @@ type OperationPack struct { commitHash git.Hash } +// hold the different operation type to instantiate to parse JSON var operations map[OperationType]reflect.Type +// Register will register a new type of Operation to be able to parse the corresponding JSON func Register(t OperationType, op interface{}) { if operations == nil { operations = make(map[OperationType]reflect.Type) @@ -96,8 +99,18 @@ func (opp *OperationPack) IsEmpty() bool { } // IsValid tell if the OperationPack is considered valid -func (opp *OperationPack) IsValid() bool { - return !opp.IsEmpty() +func (opp *OperationPack) Validate() error { + if opp.IsEmpty() { + return fmt.Errorf("empty") + } + + for _, op := range opp.Operations { + if err := op.Validate(); err != nil { + return errors.Wrap(err, "op") + } + } + + return nil } // Write will serialize and store the OperationPack as a git blob and return diff --git a/bug/person.go b/bug/person.go index d9f1108b21997ba7ce87ee28043c5468197192b8..9fa86804bcf6e3ae9fcad0074299f1dad0d339c1 100644 --- a/bug/person.go +++ b/bug/person.go @@ -2,9 +2,11 @@ package bug import ( "errors" + "fmt" "strings" "github.com/MichaelMure/git-bug/repository" + "github.com/MichaelMure/git-bug/util/text" ) type Person struct { @@ -37,3 +39,27 @@ func GetUser(repo repository.Repo) (Person, error) { func (p Person) Match(query string) bool { return strings.Contains(strings.ToLower(p.Name), strings.ToLower(query)) } + +func (p Person) Validate() error { + if text.Empty(p.Name) { + return fmt.Errorf("name is not set") + } + + if strings.Contains(p.Name, "\n") { + return fmt.Errorf("name should be a single line") + } + + if !text.Safe(p.Name) { + return fmt.Errorf("name is not fully printable") + } + + if strings.Contains(p.Email, "\n") { + return fmt.Errorf("email should be a single line") + } + + if !text.Safe(p.Email) { + return fmt.Errorf("email is not fully printable") + } + + return nil +} diff --git a/bug/status.go b/bug/status.go index f15924e293cfe7c10cb582404a9dfc72ed4944c5..737c8d31daa933164e363fcb00de7208b52cd858 100644 --- a/bug/status.go +++ b/bug/status.go @@ -47,3 +47,11 @@ func StatusFromString(str string) (Status, error) { return 0, fmt.Errorf("unknow status") } } + +func (s Status) Validate() error { + if s != OpenStatus && s != ClosedStatus { + return fmt.Errorf("invalid") + } + + return nil +} diff --git a/cache/bug_cache.go b/cache/bug_cache.go index 67c16e9675e48502bca9ce4993030a4afb1fdfdd..b0dbb6cc4348abafe75dfaf1ff8d789abc645879 100644 --- a/cache/bug_cache.go +++ b/cache/bug_cache.go @@ -44,7 +44,10 @@ func (c *BugCache) AddCommentWithFiles(message string, files []git.Hash) error { return err } - operations.CommentWithFiles(c.bug, author, message, files) + err = operations.CommentWithFiles(c.bug, author, message, files) + if err != nil { + return err + } return c.notifyUpdated() } @@ -74,7 +77,10 @@ func (c *BugCache) Open() error { return err } - operations.Open(c.bug, author) + err = operations.Open(c.bug, author) + if err != nil { + return err + } return c.notifyUpdated() } @@ -85,7 +91,10 @@ func (c *BugCache) Close() error { return err } - operations.Close(c.bug, author) + err = operations.Close(c.bug, author) + if err != nil { + return err + } return c.notifyUpdated() } @@ -96,7 +105,10 @@ func (c *BugCache) SetTitle(title string) error { return err } - operations.SetTitle(c.bug, author, title) + err = operations.SetTitle(c.bug, author, title) + if err != nil { + return err + } return c.notifyUpdated() } diff --git a/commands/pull.go b/commands/pull.go index 64de8bf5ed24330b7eb2590482c2ac1d5364096d..5ad4acc3d258120bb6ad6961c7c6064f24222494 100644 --- a/commands/pull.go +++ b/commands/pull.go @@ -42,7 +42,7 @@ func runPull(cmd *cobra.Command, args []string) error { } if merge.Status != bug.MergeStatusNothing { - fmt.Printf("%s: %s\n", merge.Bug.HumanId(), merge.Status) + fmt.Printf("%s: %s\n", merge.Bug.HumanId(), merge) } } diff --git a/misc/random_bugs/create_random_bugs.go b/misc/random_bugs/create_random_bugs.go index ad81e2ab0cdd47143b8cb24db9fd9c2660b0f9cf..75ce5f82491b4f05054712b628946f0fd3a1f610 100644 --- a/misc/random_bugs/create_random_bugs.go +++ b/misc/random_bugs/create_random_bugs.go @@ -57,8 +57,8 @@ func GenerateRandomBugsWithSeed(opts Options, seed int64) []*bug.Bug { comment, title, labels, - operations.Open, - operations.Close, + open, + close, } result := make([]*bug.Bug, opts.BugNumber) @@ -148,11 +148,19 @@ func paragraphs() string { } func comment(b bug.Interface, p bug.Person) { - operations.Comment(b, p, paragraphs()) + _ = operations.Comment(b, p, paragraphs()) } func title(b bug.Interface, p bug.Person) { - operations.SetTitle(b, p, fake.Sentence()) + _ = operations.SetTitle(b, p, fake.Sentence()) +} + +func open(b bug.Interface, p bug.Person) { + _ = operations.Open(b, p) +} + +func close(b bug.Interface, p bug.Person) { + _ = operations.Close(b, p) } var addedLabels []string @@ -179,5 +187,5 @@ func labels(b bug.Interface, p bug.Person) { // ignore error // if the randomisation produce no changes, no op // is added to the bug - operations.ChangeLabels(b, p, added, removed) + _, _ = operations.ChangeLabels(b, p, added, removed) } diff --git a/operations/add_comment.go b/operations/add_comment.go index 8c4b8b9cf12477797e7cd993e218c40a0c86872c..01572eb7f96189d3135ac45ec94728c026827372 100644 --- a/operations/add_comment.go +++ b/operations/add_comment.go @@ -1,8 +1,11 @@ package operations import ( + "fmt" + "github.com/MichaelMure/git-bug/bug" "github.com/MichaelMure/git-bug/util/git" + "github.com/MichaelMure/git-bug/util/text" ) // AddCommentOperation will add a new comment in the bug @@ -33,6 +36,22 @@ func (op AddCommentOperation) GetFiles() []git.Hash { return op.Files } +func (op AddCommentOperation) Validate() error { + if err := bug.OpBaseValidate(op, bug.AddCommentOp); err != nil { + return err + } + + if text.Empty(op.Message) { + return fmt.Errorf("message is empty") + } + + if !text.Safe(op.Message) { + return fmt.Errorf("message is not fully printable") + } + + return nil +} + func NewAddCommentOp(author bug.Person, message string, files []git.Hash) AddCommentOperation { return AddCommentOperation{ OpBase: bug.NewOpBase(bug.AddCommentOp, author), @@ -42,11 +61,15 @@ func NewAddCommentOp(author bug.Person, message string, files []git.Hash) AddCom } // Convenience function to apply the operation -func Comment(b bug.Interface, author bug.Person, message string) { - CommentWithFiles(b, author, message, nil) +func Comment(b bug.Interface, author bug.Person, message string) error { + return CommentWithFiles(b, author, message, nil) } -func CommentWithFiles(b bug.Interface, author bug.Person, message string, files []git.Hash) { +func CommentWithFiles(b bug.Interface, author bug.Person, message string, files []git.Hash) error { addCommentOp := NewAddCommentOp(author, message, files) + if err := addCommentOp.Validate(); err != nil { + return err + } b.Append(addCommentOp) + return nil } diff --git a/operations/create.go b/operations/create.go index a671171e60466247993f46a43a224ee4a7b0b925..efd4492f2048cb25926330a1496a94d87f9f23f0 100644 --- a/operations/create.go +++ b/operations/create.go @@ -1,8 +1,12 @@ package operations import ( + "fmt" + "strings" + "github.com/MichaelMure/git-bug/bug" "github.com/MichaelMure/git-bug/util/git" + "github.com/MichaelMure/git-bug/util/text" ) // CreateOperation define the initial creation of a bug @@ -34,6 +38,30 @@ func (op CreateOperation) GetFiles() []git.Hash { return op.Files } +func (op CreateOperation) Validate() error { + if err := bug.OpBaseValidate(op, bug.CreateOp); err != nil { + return err + } + + if text.Empty(op.Title) { + return fmt.Errorf("title is empty") + } + + if strings.Contains(op.Title, "\n") { + return fmt.Errorf("title should be a single line") + } + + if !text.Safe(op.Title) { + return fmt.Errorf("title is not fully printable") + } + + if !text.Safe(op.Message) { + return fmt.Errorf("message is not fully printable") + } + + return nil +} + func NewCreateOp(author bug.Person, title, message string, files []git.Hash) CreateOperation { return CreateOperation{ OpBase: bug.NewOpBase(bug.CreateOp, author), @@ -51,6 +79,11 @@ func Create(author bug.Person, title, message string) (*bug.Bug, error) { func CreateWithFiles(author bug.Person, title, message string, files []git.Hash) (*bug.Bug, error) { newBug := bug.NewBug() createOp := NewCreateOp(author, title, message, files) + + if err := createOp.Validate(); err != nil { + return nil, err + } + newBug.Append(createOp) return newBug, nil diff --git a/operations/label_change.go b/operations/label_change.go index a3cc989834cb4d4bb842b37865c96638201aed36..507651dfc3562738c212fa9db6f6567f284f481c 100644 --- a/operations/label_change.go +++ b/operations/label_change.go @@ -5,6 +5,7 @@ import ( "sort" "github.com/MichaelMure/git-bug/bug" + "github.com/pkg/errors" ) var _ bug.Operation = LabelChangeOperation{} @@ -49,6 +50,30 @@ AddLoop: return snapshot } +func (op LabelChangeOperation) Validate() error { + if err := bug.OpBaseValidate(op, bug.LabelChangeOp); err != nil { + return err + } + + for _, l := range op.Added { + if err := l.Validate(); err != nil { + return errors.Wrap(err, "added label") + } + } + + for _, l := range op.Removed { + if err := l.Validate(); err != nil { + return errors.Wrap(err, "removed label") + } + } + + if len(op.Added)+len(op.Removed) <= 0 { + return fmt.Errorf("no label change") + } + + return nil +} + func NewLabelChangeOperation(author bug.Person, added, removed []bug.Label) LabelChangeOperation { return LabelChangeOperation{ OpBase: bug.NewOpBase(bug.LabelChangeOp, author), @@ -108,6 +133,10 @@ func ChangeLabels(b bug.Interface, author bug.Person, add, remove []string) ([]L labelOp := NewLabelChangeOperation(author, added, removed) + if err := labelOp.Validate(); err != nil { + return nil, err + } + b.Append(labelOp) return results, nil diff --git a/operations/operations_test.go b/operations/operations_test.go new file mode 100644 index 0000000000000000000000000000000000000000..6fba79171a336527e8d3b13e58af623be9628b86 --- /dev/null +++ b/operations/operations_test.go @@ -0,0 +1,69 @@ +package operations + +import ( + "testing" + + "github.com/MichaelMure/git-bug/bug" + "github.com/MichaelMure/git-bug/util/git" +) + +func TestValidate(t *testing.T) { + rene := bug.Person{ + Name: "René Descartes", + Email: "rene@descartes.fr", + } + + good := []bug.Operation{ + NewCreateOp(rene, "title", "message", nil), + NewSetTitleOp(rene, "title2", "title1"), + NewAddCommentOp(rene, "message2", nil), + NewSetStatusOp(rene, bug.ClosedStatus), + NewLabelChangeOperation(rene, []bug.Label{"added"}, []bug.Label{"removed"}), + } + + for _, op := range good { + if err := op.Validate(); err != nil { + t.Fatal(err) + } + } + + bad := []bug.Operation{ + // opbase + NewSetStatusOp(bug.Person{Name: "", Email: "rene@descartes.fr"}, bug.ClosedStatus), + NewSetStatusOp(bug.Person{Name: "René Descartes\u001b", Email: "rene@descartes.fr"}, bug.ClosedStatus), + NewSetStatusOp(bug.Person{Name: "René Descartes", Email: "rene@descartes.fr\u001b"}, bug.ClosedStatus), + NewSetStatusOp(bug.Person{Name: "René \nDescartes", Email: "rene@descartes.fr"}, bug.ClosedStatus), + NewSetStatusOp(bug.Person{Name: "René Descartes", Email: "rene@\ndescartes.fr"}, bug.ClosedStatus), + CreateOperation{OpBase: bug.OpBase{ + Author: rene, + UnixTime: 0, + OperationType: bug.CreateOp, + }, + Title: "title", + Message: "message", + }, + + NewCreateOp(rene, "multi\nline", "message", nil), + NewCreateOp(rene, "title", "message", []git.Hash{git.Hash("invalid")}), + NewCreateOp(rene, "title\u001b", "message", nil), + NewCreateOp(rene, "title", "message\u001b", nil), + NewSetTitleOp(rene, "multi\nline", "title1"), + NewSetTitleOp(rene, "title", "multi\nline"), + NewSetTitleOp(rene, "title\u001b", "title2"), + NewSetTitleOp(rene, "title", "title2\u001b"), + NewAddCommentOp(rene, "", nil), + NewAddCommentOp(rene, "message\u001b", nil), + NewAddCommentOp(rene, "message", []git.Hash{git.Hash("invalid")}), + NewSetStatusOp(rene, 1000), + NewSetStatusOp(rene, 0), + NewLabelChangeOperation(rene, []bug.Label{}, []bug.Label{}), + NewLabelChangeOperation(rene, []bug.Label{"multi\nline"}, []bug.Label{}), + } + + for i, op := range bad { + if err := op.Validate(); err == nil { + t.Fatal("validation should have failed", i, op) + } + } + +} diff --git a/operations/set_status.go b/operations/set_status.go index 37ebac109452c5968fb392f46f8521c7635151d3..07dcf208675b1c4156c0c14a9d7ec262f242462f 100644 --- a/operations/set_status.go +++ b/operations/set_status.go @@ -2,6 +2,7 @@ package operations import ( "github.com/MichaelMure/git-bug/bug" + "github.com/pkg/errors" ) // SetStatusOperation will change the status of a bug @@ -19,6 +20,18 @@ func (op SetStatusOperation) Apply(snapshot bug.Snapshot) bug.Snapshot { return snapshot } +func (op SetStatusOperation) Validate() error { + if err := bug.OpBaseValidate(op, bug.SetStatusOp); err != nil { + return err + } + + if err := op.Status.Validate(); err != nil { + return errors.Wrap(err, "status") + } + + return nil +} + func NewSetStatusOp(author bug.Person, status bug.Status) SetStatusOperation { return SetStatusOperation{ OpBase: bug.NewOpBase(bug.SetStatusOp, author), @@ -27,13 +40,21 @@ func NewSetStatusOp(author bug.Person, status bug.Status) SetStatusOperation { } // Convenience function to apply the operation -func Open(b bug.Interface, author bug.Person) { +func Open(b bug.Interface, author bug.Person) error { op := NewSetStatusOp(author, bug.OpenStatus) + if err := op.Validate(); err != nil { + return err + } b.Append(op) + return nil } // Convenience function to apply the operation -func Close(b bug.Interface, author bug.Person) { +func Close(b bug.Interface, author bug.Person) error { op := NewSetStatusOp(author, bug.ClosedStatus) + if err := op.Validate(); err != nil { + return err + } b.Append(op) + return nil } diff --git a/operations/set_title.go b/operations/set_title.go index 7aa76268dbc72a329b050a8698e1d2686c1c389c..46addce67dc70b2e9b113b5f890d21d4206dee5c 100644 --- a/operations/set_title.go +++ b/operations/set_title.go @@ -1,7 +1,11 @@ package operations import ( + "fmt" + "strings" + "github.com/MichaelMure/git-bug/bug" + "github.com/MichaelMure/git-bug/util/text" ) // SetTitleOperation will change the title of a bug @@ -20,6 +24,34 @@ func (op SetTitleOperation) Apply(snapshot bug.Snapshot) bug.Snapshot { return snapshot } +func (op SetTitleOperation) Validate() error { + if err := bug.OpBaseValidate(op, bug.SetTitleOp); err != nil { + return err + } + + if text.Empty(op.Title) { + return fmt.Errorf("title is empty") + } + + if strings.Contains(op.Title, "\n") { + return fmt.Errorf("title should be a single line") + } + + if !text.Safe(op.Title) { + return fmt.Errorf("title should be fully printable") + } + + if strings.Contains(op.Was, "\n") { + return fmt.Errorf("previous title should be a single line") + } + + if !text.Safe(op.Was) { + return fmt.Errorf("previous title should be fully printable") + } + + return nil +} + func NewSetTitleOp(author bug.Person, title string, was string) SetTitleOperation { return SetTitleOperation{ OpBase: bug.NewOpBase(bug.SetTitleOp, author), @@ -29,7 +61,7 @@ func NewSetTitleOp(author bug.Person, title string, was string) SetTitleOperatio } // Convenience function to apply the operation -func SetTitle(b bug.Interface, author bug.Person, title string) { +func SetTitle(b bug.Interface, author bug.Person, title string) error { it := bug.NewOperationIterator(b) var lastTitleOp bug.Operation @@ -48,5 +80,11 @@ func SetTitle(b bug.Interface, author bug.Person, title string) { } setTitleOp := NewSetTitleOp(author, title, was) + + if err := setTitleOp.Validate(); err != nil { + return err + } + b.Append(setTitleOp) + return nil } diff --git a/termui/bug_table.go b/termui/bug_table.go index 7cd3ba9a2f7ee19e44d6b6427a00bc6c3dc6ebac..8c32a6315bcdfc338c2a6ff038303ab47ba0a6de 100644 --- a/termui/bug_table.go +++ b/termui/bug_table.go @@ -434,7 +434,7 @@ func (bt *bugTable) pull(g *gocui.Gui, v *gocui.View) error { }) } else { fmt.Fprintf(&buffer, "%s%s: %s", - beginLine, colors.Cyan(merge.Bug.HumanId()), merge.Status, + beginLine, colors.Cyan(merge.Bug.HumanId()), merge, ) beginLine = "\n" diff --git a/tests/bug_test.go b/tests/bug_test.go index fdda31b193838cf1d92bbcfcf1bbfe1d39040d81..02336974971aaba7d2d2f174fb53c1861b214f10 100644 --- a/tests/bug_test.go +++ b/tests/bug_test.go @@ -2,10 +2,14 @@ package tests import ( "github.com/MichaelMure/git-bug/bug" + "github.com/MichaelMure/git-bug/repository" + "testing" ) func TestBugId(t *testing.T) { + mockRepo := repository.NewMockRepoForTest() + bug1 := bug.NewBug() bug1.Append(createOp) @@ -20,33 +24,35 @@ func TestBugId(t *testing.T) { } func TestBugValidity(t *testing.T) { + mockRepo := repository.NewMockRepoForTest() + bug1 := bug.NewBug() - if bug1.IsValid() { + if bug1.Validate() == nil { t.Fatal("Empty bug should be invalid") } bug1.Append(createOp) - if !bug1.IsValid() { + if bug1.Validate() != nil { t.Fatal("Bug with just a CreateOp should be valid") } - bug1.Append(createOp) - - if bug1.IsValid() { - t.Fatal("Bug with multiple CreateOp should be invalid") - } - err := bug1.Commit(mockRepo) - if err != nil { t.Fatal(err) } - if bug1.IsValid() { + bug1.Append(createOp) + + if bug1.Validate() == nil { t.Fatal("Bug with multiple CreateOp should be invalid") } + + err = bug1.Commit(mockRepo) + if err == nil { + t.Fatal("Invalid bug should not commit") + } } //func TestBugSerialisation(t *testing.T) { diff --git a/tests/operation_iterator_test.go b/tests/operation_iterator_test.go index f1840091a1939496d1a1e1fc5c1feb523bd8455d..790c8a62747b5fb681e36d94fda8a4fd0cb60061 100644 --- a/tests/operation_iterator_test.go +++ b/tests/operation_iterator_test.go @@ -18,10 +18,10 @@ var ( addCommentOp = operations.NewAddCommentOp(rene, "message2", nil) setStatusOp = operations.NewSetStatusOp(rene, bug.ClosedStatus) labelChangeOp = operations.NewLabelChangeOperation(rene, []bug.Label{"added"}, []bug.Label{"removed"}) - mockRepo = repository.NewMockRepoForTest() ) func TestOpIterator(t *testing.T) { + mockRepo := repository.NewMockRepoForTest() bug1 := bug.NewBug() diff --git a/util/text/validate.go b/util/text/validate.go new file mode 100644 index 0000000000000000000000000000000000000000..68bdf48b247012e637a53439b5409cfaa3715830 --- /dev/null +++ b/util/text/validate.go @@ -0,0 +1,33 @@ +package text + +import ( + "strings" + "unicode" +) + +// Empty tell if the string is considered empty once space +// and not graphics characters are removed +func Empty(s string) bool { + trim := strings.TrimFunc(s, func(r rune) bool { + return unicode.IsSpace(r) || !unicode.IsGraphic(r) + }) + + return trim == "" +} + +// Safe will tell if a character in the string is considered unsafe +// Currently trigger on unicode control character except \n, \t and \r +func Safe(s string) bool { + for _, r := range s { + switch r { + case '\t', '\r', '\n': + continue + } + + if unicode.IsControl(r) { + return false + } + } + + return true +}