aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--bug/bug.go28
-rw-r--r--bug/bug_actions.go41
-rw-r--r--bug/interface.go4
-rw-r--r--bug/label.go21
-rw-r--r--bug/operation.go42
-rw-r--r--bug/operation_pack.go17
-rw-r--r--bug/person.go26
-rw-r--r--bug/status.go8
-rw-r--r--cache/bug_cache.go20
-rw-r--r--commands/pull.go2
-rw-r--r--misc/random_bugs/create_random_bugs.go18
-rw-r--r--operations/add_comment.go29
-rw-r--r--operations/create.go33
-rw-r--r--operations/label_change.go29
-rw-r--r--operations/operations_test.go69
-rw-r--r--operations/set_status.go25
-rw-r--r--operations/set_title.go40
-rw-r--r--termui/bug_table.go2
-rw-r--r--tests/bug_test.go26
-rw-r--r--tests/operation_iterator_test.go2
-rw-r--r--util/text/validate.go33
21 files changed, 452 insertions, 63 deletions
diff --git a/bug/bug.go b/bug/bug.go
index e548757e..3342ecfa 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 8bda15b7..7f2194ab 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 c397bfbc..72dee61c 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 b19a980f..b73222dd 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 6a8aa0cd..b148141c 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 fe26952f..03d538d5 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 d9f1108b..9fa86804 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 f15924e2..737c8d31 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 67c16e96..b0dbb6cc 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 64de8bf5..5ad4acc3 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 ad81e2ab..75ce5f82 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 8c4b8b9c..01572eb7 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 a671171e..efd4492f 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 a3cc9898..507651df 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 00000000..6fba7917
--- /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 37ebac10..07dcf208 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 7aa76268..46addce6 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 7cd3ba9a..8c32a631 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 fdda31b1..02336974 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 f1840091..790c8a62 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 00000000..68bdf48b
--- /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
+}
generated by cgit (git 2.34.1) at 2024-09-20 09:21:55 +0000