diff options
Diffstat (limited to 'bug')
| -rw-r--r-- | bug/bug.go | 28 | ||||
| -rw-r--r-- | bug/bug_actions.go | 41 | ||||
| -rw-r--r-- | bug/interface.go | 4 | ||||
| -rw-r--r-- | bug/label.go | 21 | ||||
| -rw-r--r-- | bug/operation.go | 42 | ||||
| -rw-r--r-- | bug/operation_pack.go | 17 | ||||
| -rw-r--r-- | bug/person.go | 26 | ||||
| -rw-r--r-- | bug/status.go | 8 |
8 files changed, 152 insertions, 35 deletions
@@ -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 +} |