From 1bf268cebc84a9de1e538cbb54bcc0f434022192 Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Fri, 28 Sep 2018 20:39:39 +0200 Subject: merge package operations into bug, they are tightly coupled anyway --- bug/bug.go | 4 +- bug/bug_actions_test.go | 362 +++++++++++++++++++++++++++++++++++++++++ bug/bug_test.go | 82 ++++++++++ bug/op_add_comment.go | 78 +++++++++ bug/op_create.go | 93 +++++++++++ bug/op_create_test.go | 35 ++++ bug/op_label_change.go | 189 +++++++++++++++++++++ bug/op_set_status.go | 63 +++++++ bug/op_set_title.go | 93 +++++++++++ bug/operation.go | 28 +--- bug/operation_iterator_test.go | 59 +++++++ bug/operation_pack.go | 53 +++--- bug/operation_pack_test.go | 53 ++++++ bug/operation_test.go | 70 ++++++++ 14 files changed, 1217 insertions(+), 45 deletions(-) create mode 100644 bug/bug_actions_test.go create mode 100644 bug/bug_test.go create mode 100644 bug/op_add_comment.go create mode 100644 bug/op_create.go create mode 100644 bug/op_create_test.go create mode 100644 bug/op_label_change.go create mode 100644 bug/op_set_status.go create mode 100644 bug/op_set_title.go create mode 100644 bug/operation_iterator_test.go create mode 100644 bug/operation_pack_test.go create mode 100644 bug/operation_test.go (limited to 'bug') diff --git a/bug/bug.go b/bug/bug.go index 993d3d7c..b252b176 100644 --- a/bug/bug.go +++ b/bug/bug.go @@ -300,7 +300,7 @@ func (bug *Bug) Validate() error { // The very first Op should be a CreateOp firstOp := bug.FirstOp() - if firstOp == nil || firstOp.OpType() != CreateOp { + if firstOp == nil || firstOp.base().OperationType != CreateOp { return fmt.Errorf("first operation should be a Create op") } @@ -308,7 +308,7 @@ func (bug *Bug) Validate() error { it := NewOperationIterator(bug) createCount := 0 for it.Next() { - if it.Value().OpType() == CreateOp { + if it.Value().base().OperationType == CreateOp { createCount++ } } diff --git a/bug/bug_actions_test.go b/bug/bug_actions_test.go new file mode 100644 index 00000000..44409296 --- /dev/null +++ b/bug/bug_actions_test.go @@ -0,0 +1,362 @@ +package bug + +import ( + "github.com/MichaelMure/git-bug/repository" + "io/ioutil" + "log" + "os" + "testing" +) + +func createRepo(bare bool) *repository.GitRepo { + dir, err := ioutil.TempDir("", "") + if err != nil { + log.Fatal(err) + } + + // fmt.Println("Creating repo:", dir) + + var creator func(string) (*repository.GitRepo, error) + + if bare { + creator = repository.InitBareGitRepo + } else { + creator = repository.InitGitRepo + } + + repo, err := creator(dir) + if err != nil { + log.Fatal(err) + } + + return repo +} + +func cleanupRepo(repo repository.Repo) error { + path := repo.GetPath() + // fmt.Println("Cleaning repo:", path) + return os.RemoveAll(path) +} + +func setupRepos(t testing.TB) (repoA, repoB, remote *repository.GitRepo) { + repoA = createRepo(false) + repoB = createRepo(false) + remote = createRepo(true) + + remoteAddr := "file://" + remote.GetPath() + + err := repoA.AddRemote("origin", remoteAddr) + if err != nil { + t.Fatal(err) + } + + err = repoB.AddRemote("origin", remoteAddr) + if err != nil { + t.Fatal(err) + } + + return repoA, repoB, remote +} + +func cleanupRepos(repoA, repoB, remote *repository.GitRepo) { + cleanupRepo(repoA) + cleanupRepo(repoB) + cleanupRepo(remote) +} + +func TestPushPull(t *testing.T) { + repoA, repoB, remote := setupRepos(t) + defer cleanupRepos(repoA, repoB, remote) + + bug1, err := Create(rene, unix, "bug1", "message") + checkErr(t, err) + err = bug1.Commit(repoA) + checkErr(t, err) + + // A --> remote --> B + _, err = Push(repoA, "origin") + checkErr(t, err) + + err = Pull(repoB, "origin") + checkErr(t, err) + + bugs := allBugs(t, ReadAllLocalBugs(repoB)) + + if len(bugs) != 1 { + t.Fatal("Unexpected number of bugs") + } + + // B --> remote --> A + bug2, err := Create(rene, unix, "bug2", "message") + checkErr(t, err) + err = bug2.Commit(repoB) + checkErr(t, err) + + _, err = Push(repoB, "origin") + checkErr(t, err) + + err = Pull(repoA, "origin") + checkErr(t, err) + + bugs = allBugs(t, ReadAllLocalBugs(repoA)) + + if len(bugs) != 2 { + t.Fatal("Unexpected number of bugs") + } +} + +func checkErr(t testing.TB, err error) { + if err != nil { + t.Fatal(err) + } +} + +func allBugs(t testing.TB, bugs <-chan StreamedBug) []*Bug { + var result []*Bug + for streamed := range bugs { + if streamed.Err != nil { + t.Fatal(streamed.Err) + } + result = append(result, streamed.Bug) + } + return result +} + +func TestRebaseTheirs(t *testing.T) { + _RebaseTheirs(t) +} + +func BenchmarkRebaseTheirs(b *testing.B) { + for n := 0; n < b.N; n++ { + _RebaseTheirs(b) + } +} + +func _RebaseTheirs(t testing.TB) { + repoA, repoB, remote := setupRepos(t) + defer cleanupRepos(repoA, repoB, remote) + + bug1, err := Create(rene, unix, "bug1", "message") + checkErr(t, err) + err = bug1.Commit(repoA) + checkErr(t, err) + + // A --> remote + _, err = Push(repoA, "origin") + checkErr(t, err) + + // remote --> B + err = Pull(repoB, "origin") + checkErr(t, err) + + bug2, err := ReadLocalBug(repoB, bug1.Id()) + checkErr(t, err) + + AddComment(bug2, rene, unix, "message2") + AddComment(bug2, rene, unix, "message3") + AddComment(bug2, rene, unix, "message4") + err = bug2.Commit(repoB) + checkErr(t, err) + + // B --> remote + _, err = Push(repoB, "origin") + checkErr(t, err) + + // remote --> A + err = Pull(repoA, "origin") + checkErr(t, err) + + bugs := allBugs(t, ReadAllLocalBugs(repoB)) + + if len(bugs) != 1 { + t.Fatal("Unexpected number of bugs") + } + + bug3, err := ReadLocalBug(repoA, bug1.Id()) + checkErr(t, err) + + if nbOps(bug3) != 4 { + t.Fatal("Unexpected number of operations") + } +} + +func TestRebaseOurs(t *testing.T) { + _RebaseOurs(t) +} + +func BenchmarkRebaseOurs(b *testing.B) { + for n := 0; n < b.N; n++ { + _RebaseOurs(b) + } +} + +func _RebaseOurs(t testing.TB) { + repoA, repoB, remote := setupRepos(t) + defer cleanupRepos(repoA, repoB, remote) + + bug1, err := Create(rene, unix, "bug1", "message") + checkErr(t, err) + err = bug1.Commit(repoA) + checkErr(t, err) + + // A --> remote + _, err = Push(repoA, "origin") + checkErr(t, err) + + // remote --> B + err = Pull(repoB, "origin") + checkErr(t, err) + + AddComment(bug1, rene, unix, "message2") + AddComment(bug1, rene, unix, "message3") + AddComment(bug1, rene, unix, "message4") + err = bug1.Commit(repoA) + checkErr(t, err) + + AddComment(bug1, rene, unix, "message5") + AddComment(bug1, rene, unix, "message6") + AddComment(bug1, rene, unix, "message7") + err = bug1.Commit(repoA) + checkErr(t, err) + + AddComment(bug1, rene, unix, "message8") + AddComment(bug1, rene, unix, "message9") + AddComment(bug1, rene, unix, "message10") + err = bug1.Commit(repoA) + checkErr(t, err) + + // remote --> A + err = Pull(repoA, "origin") + checkErr(t, err) + + bugs := allBugs(t, ReadAllLocalBugs(repoA)) + + if len(bugs) != 1 { + t.Fatal("Unexpected number of bugs") + } + + bug2, err := ReadLocalBug(repoA, bug1.Id()) + checkErr(t, err) + + if nbOps(bug2) != 10 { + t.Fatal("Unexpected number of operations") + } +} + +func nbOps(b *Bug) int { + it := NewOperationIterator(b) + counter := 0 + for it.Next() { + counter++ + } + return counter +} + +func TestRebaseConflict(t *testing.T) { + _RebaseConflict(t) +} + +func BenchmarkRebaseConflict(b *testing.B) { + for n := 0; n < b.N; n++ { + _RebaseConflict(b) + } +} + +func _RebaseConflict(t testing.TB) { + repoA, repoB, remote := setupRepos(t) + defer cleanupRepos(repoA, repoB, remote) + + bug1, err := Create(rene, unix, "bug1", "message") + checkErr(t, err) + err = bug1.Commit(repoA) + checkErr(t, err) + + // A --> remote + _, err = Push(repoA, "origin") + checkErr(t, err) + + // remote --> B + err = Pull(repoB, "origin") + checkErr(t, err) + + AddComment(bug1, rene, unix, "message2") + AddComment(bug1, rene, unix, "message3") + AddComment(bug1, rene, unix, "message4") + err = bug1.Commit(repoA) + checkErr(t, err) + + AddComment(bug1, rene, unix, "message5") + AddComment(bug1, rene, unix, "message6") + AddComment(bug1, rene, unix, "message7") + err = bug1.Commit(repoA) + checkErr(t, err) + + AddComment(bug1, rene, unix, "message8") + AddComment(bug1, rene, unix, "message9") + AddComment(bug1, rene, unix, "message10") + err = bug1.Commit(repoA) + checkErr(t, err) + + bug2, err := ReadLocalBug(repoB, bug1.Id()) + checkErr(t, err) + + AddComment(bug2, rene, unix, "message11") + AddComment(bug2, rene, unix, "message12") + AddComment(bug2, rene, unix, "message13") + err = bug2.Commit(repoB) + checkErr(t, err) + + AddComment(bug2, rene, unix, "message14") + AddComment(bug2, rene, unix, "message15") + AddComment(bug2, rene, unix, "message16") + err = bug2.Commit(repoB) + checkErr(t, err) + + AddComment(bug2, rene, unix, "message17") + AddComment(bug2, rene, unix, "message18") + AddComment(bug2, rene, unix, "message19") + err = bug2.Commit(repoB) + checkErr(t, err) + + // A --> remote + _, err = Push(repoA, "origin") + checkErr(t, err) + + // remote --> B + err = Pull(repoB, "origin") + checkErr(t, err) + + bugs := allBugs(t, ReadAllLocalBugs(repoB)) + + if len(bugs) != 1 { + t.Fatal("Unexpected number of bugs") + } + + bug3, err := ReadLocalBug(repoB, bug1.Id()) + checkErr(t, err) + + if nbOps(bug3) != 19 { + t.Fatal("Unexpected number of operations") + } + + // B --> remote + _, err = Push(repoB, "origin") + checkErr(t, err) + + // remote --> A + err = Pull(repoA, "origin") + checkErr(t, err) + + bugs = allBugs(t, ReadAllLocalBugs(repoA)) + + if len(bugs) != 1 { + t.Fatal("Unexpected number of bugs") + } + + bug4, err := ReadLocalBug(repoA, bug1.Id()) + checkErr(t, err) + + if nbOps(bug4) != 19 { + t.Fatal("Unexpected number of operations") + } +} diff --git a/bug/bug_test.go b/bug/bug_test.go new file mode 100644 index 00000000..d85db6d2 --- /dev/null +++ b/bug/bug_test.go @@ -0,0 +1,82 @@ +package bug + +import ( + "github.com/MichaelMure/git-bug/repository" + "github.com/go-test/deep" + + "testing" +) + +func TestBugId(t *testing.T) { + mockRepo := repository.NewMockRepoForTest() + + bug1 := NewBug() + + bug1.Append(createOp) + + err := bug1.Commit(mockRepo) + + if err != nil { + t.Fatal(err) + } + + bug1.Id() +} + +func TestBugValidity(t *testing.T) { + mockRepo := repository.NewMockRepoForTest() + + bug1 := NewBug() + + if bug1.Validate() == nil { + t.Fatal("Empty bug should be invalid") + } + + bug1.Append(createOp) + + if bug1.Validate() != nil { + t.Fatal("Bug with just a CreateOp should be valid") + } + + err := bug1.Commit(mockRepo) + if err != nil { + t.Fatal(err) + } + + 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) { + bug1 := NewBug() + + bug1.Append(createOp) + bug1.Append(setTitleOp) + bug1.Append(setTitleOp) + bug1.Append(addCommentOp) + + repo := repository.NewMockRepoForTest() + + bug1.Commit(repo) + + bug2, err := ReadLocalBug(repo, bug1.Id()) + if err != nil { + t.Error(err) + } + + // ignore some fields + bug2.packs[0].commitHash = bug1.packs[0].commitHash + + deep.CompareUnexportedFields = true + if diff := deep.Equal(bug1, bug2); diff != nil { + t.Fatal(diff) + } +} diff --git a/bug/op_add_comment.go b/bug/op_add_comment.go new file mode 100644 index 00000000..0a3a5a37 --- /dev/null +++ b/bug/op_add_comment.go @@ -0,0 +1,78 @@ +package bug + +import ( + "fmt" + + "github.com/MichaelMure/git-bug/util/git" + "github.com/MichaelMure/git-bug/util/text" +) + +// AddCommentOperation will add a new comment in the bug + +var _ Operation = AddCommentOperation{} + +type AddCommentOperation struct { + *OpBase + Message string `json:"message"` + // TODO: change for a map[string]util.hash to store the filename ? + Files []git.Hash `json:"files"` +} + +func (op AddCommentOperation) base() *OpBase { + return op.OpBase +} + +func (op AddCommentOperation) Apply(snapshot Snapshot) Snapshot { + comment := Comment{ + Message: op.Message, + Author: op.Author, + Files: op.Files, + UnixTime: op.UnixTime, + } + + snapshot.Comments = append(snapshot.Comments, comment) + + return snapshot +} + +func (op AddCommentOperation) GetFiles() []git.Hash { + return op.Files +} + +func (op AddCommentOperation) Validate() error { + if err := opBaseValidate(op, 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 Person, unixTime int64, message string, files []git.Hash) AddCommentOperation { + return AddCommentOperation{ + OpBase: newOpBase(AddCommentOp, author, unixTime), + Message: message, + Files: files, + } +} + +// Convenience function to apply the operation +func AddComment(b Interface, author Person, unixTime int64, message string) error { + return AddCommentWithFiles(b, author, unixTime, message, nil) +} + +func AddCommentWithFiles(b Interface, author Person, unixTime int64, message string, files []git.Hash) error { + addCommentOp := NewAddCommentOp(author, unixTime, message, files) + if err := addCommentOp.Validate(); err != nil { + return err + } + b.Append(addCommentOp) + return nil +} diff --git a/bug/op_create.go b/bug/op_create.go new file mode 100644 index 00000000..2852a519 --- /dev/null +++ b/bug/op_create.go @@ -0,0 +1,93 @@ +package bug + +import ( + "fmt" + "strings" + + "github.com/MichaelMure/git-bug/util/git" + "github.com/MichaelMure/git-bug/util/text" +) + +// CreateOperation define the initial creation of a bug + +var _ Operation = CreateOperation{} + +type CreateOperation struct { + *OpBase + Title string `json:"title"` + Message string `json:"message"` + Files []git.Hash `json:"files"` +} + +func (op CreateOperation) base() *OpBase { + return op.OpBase +} + +func (op CreateOperation) Apply(snapshot Snapshot) Snapshot { + snapshot.Title = op.Title + snapshot.Comments = []Comment{ + { + Message: op.Message, + Author: op.Author, + UnixTime: op.UnixTime, + }, + } + snapshot.Author = op.Author + snapshot.CreatedAt = op.Time() + return snapshot +} + +func (op CreateOperation) GetFiles() []git.Hash { + return op.Files +} + +func (op CreateOperation) Validate() error { + if err := opBaseValidate(op, 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 Person, unixTime int64, title, message string, files []git.Hash) CreateOperation { + return CreateOperation{ + OpBase: newOpBase(CreateOp, author, unixTime), + Title: title, + Message: message, + Files: files, + } +} + +// Convenience function to apply the operation +func Create(author Person, unixTime int64, title, message string) (*Bug, error) { + return CreateWithFiles(author, unixTime, title, message, nil) +} + +func CreateWithFiles(author Person, unixTime int64, title, message string, files []git.Hash) (*Bug, error) { + newBug := NewBug() + createOp := NewCreateOp(author, unixTime, title, message, files) + + if err := createOp.Validate(); err != nil { + return nil, err + } + + newBug.Append(createOp) + + return newBug, nil +} diff --git a/bug/op_create_test.go b/bug/op_create_test.go new file mode 100644 index 00000000..338067aa --- /dev/null +++ b/bug/op_create_test.go @@ -0,0 +1,35 @@ +package bug + +import ( + "reflect" + "testing" + "time" +) + +func TestCreate(t *testing.T) { + snapshot := Snapshot{} + + var rene = Person{ + Name: "René Descartes", + Email: "rene@descartes.fr", + } + + unix := time.Now().Unix() + + create := NewCreateOp(rene, unix, "title", "message", nil) + + snapshot = create.Apply(snapshot) + + expected := Snapshot{ + Title: "title", + Comments: []Comment{ + {Author: rene, Message: "message", UnixTime: create.UnixTime}, + }, + Author: rene, + CreatedAt: create.Time(), + } + + if !reflect.DeepEqual(snapshot, expected) { + t.Fatalf("%v different than %v", snapshot, expected) + } +} diff --git a/bug/op_label_change.go b/bug/op_label_change.go new file mode 100644 index 00000000..120671fb --- /dev/null +++ b/bug/op_label_change.go @@ -0,0 +1,189 @@ +package bug + +import ( + "fmt" + "sort" + + "github.com/pkg/errors" +) + +var _ Operation = LabelChangeOperation{} + +// LabelChangeOperation define a Bug operation to add or remove labels +type LabelChangeOperation struct { + *OpBase + Added []Label `json:"added"` + Removed []Label `json:"removed"` +} + +func (op LabelChangeOperation) base() *OpBase { + return op.OpBase +} + +// Apply apply the operation +func (op LabelChangeOperation) Apply(snapshot Snapshot) Snapshot { + // Add in the set +AddLoop: + for _, added := range op.Added { + for _, label := range snapshot.Labels { + if label == added { + // Already exist + continue AddLoop + } + } + + snapshot.Labels = append(snapshot.Labels, added) + } + + // Remove in the set + for _, removed := range op.Removed { + for i, label := range snapshot.Labels { + if label == removed { + snapshot.Labels[i] = snapshot.Labels[len(snapshot.Labels)-1] + snapshot.Labels = snapshot.Labels[:len(snapshot.Labels)-1] + } + } + } + + // Sort + sort.Slice(snapshot.Labels, func(i, j int) bool { + return string(snapshot.Labels[i]) < string(snapshot.Labels[j]) + }) + + return snapshot +} + +func (op LabelChangeOperation) Validate() error { + if err := opBaseValidate(op, 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 Person, unixTime int64, added, removed []Label) LabelChangeOperation { + return LabelChangeOperation{ + OpBase: newOpBase(LabelChangeOp, author, unixTime), + Added: added, + Removed: removed, + } +} + +// ChangeLabels is a convenience function to apply the operation +func ChangeLabels(b Interface, author Person, unixTime int64, add, remove []string) ([]LabelChangeResult, error) { + var added, removed []Label + var results []LabelChangeResult + + snap := b.Compile() + + for _, str := range add { + label := Label(str) + + // check for duplicate + if labelExist(added, label) { + results = append(results, LabelChangeResult{Label: label, Status: LabelChangeDuplicateInOp}) + continue + } + + // check that the label doesn't already exist + if labelExist(snap.Labels, label) { + results = append(results, LabelChangeResult{Label: label, Status: LabelChangeAlreadySet}) + continue + } + + added = append(added, label) + results = append(results, LabelChangeResult{Label: label, Status: LabelChangeAdded}) + } + + for _, str := range remove { + label := Label(str) + + // check for duplicate + if labelExist(removed, label) { + results = append(results, LabelChangeResult{Label: label, Status: LabelChangeDuplicateInOp}) + continue + } + + // check that the label actually exist + if !labelExist(snap.Labels, label) { + results = append(results, LabelChangeResult{Label: label, Status: LabelChangeDoesntExist}) + continue + } + + removed = append(removed, label) + results = append(results, LabelChangeResult{Label: label, Status: LabelChangeRemoved}) + } + + if len(added) == 0 && len(removed) == 0 { + return results, fmt.Errorf("no label added or removed") + } + + labelOp := NewLabelChangeOperation(author, unixTime, added, removed) + + if err := labelOp.Validate(); err != nil { + return nil, err + } + + b.Append(labelOp) + + return results, nil +} + +func labelExist(labels []Label, label Label) bool { + for _, l := range labels { + if l == label { + return true + } + } + + return false +} + +type LabelChangeStatus int + +const ( + _ LabelChangeStatus = iota + LabelChangeAdded + LabelChangeRemoved + LabelChangeDuplicateInOp + LabelChangeAlreadySet + LabelChangeDoesntExist +) + +type LabelChangeResult struct { + Label Label + Status LabelChangeStatus +} + +func (l LabelChangeResult) String() string { + switch l.Status { + case LabelChangeAdded: + return fmt.Sprintf("label %s added", l.Label) + case LabelChangeRemoved: + return fmt.Sprintf("label %s removed", l.Label) + case LabelChangeDuplicateInOp: + return fmt.Sprintf("label %s is a duplicate", l.Label) + case LabelChangeAlreadySet: + return fmt.Sprintf("label %s was already set", l.Label) + case LabelChangeDoesntExist: + return fmt.Sprintf("label %s doesn't exist on this bug", l.Label) + default: + panic(fmt.Sprintf("unknown label change status %v", l.Status)) + } +} diff --git a/bug/op_set_status.go b/bug/op_set_status.go new file mode 100644 index 00000000..d6df3da4 --- /dev/null +++ b/bug/op_set_status.go @@ -0,0 +1,63 @@ +package bug + +import ( + "github.com/pkg/errors" +) + +// SetStatusOperation will change the status of a bug + +var _ Operation = SetStatusOperation{} + +type SetStatusOperation struct { + *OpBase + Status Status `json:"status"` +} + +func (op SetStatusOperation) base() *OpBase { + return op.OpBase +} + +func (op SetStatusOperation) Apply(snapshot Snapshot) Snapshot { + snapshot.Status = op.Status + + return snapshot +} + +func (op SetStatusOperation) Validate() error { + if err := opBaseValidate(op, SetStatusOp); err != nil { + return err + } + + if err := op.Status.Validate(); err != nil { + return errors.Wrap(err, "status") + } + + return nil +} + +func NewSetStatusOp(author Person, unixTime int64, status Status) SetStatusOperation { + return SetStatusOperation{ + OpBase: newOpBase(SetStatusOp, author, unixTime), + Status: status, + } +} + +// Convenience function to apply the operation +func Open(b Interface, author Person, unixTime int64) error { + op := NewSetStatusOp(author, unixTime, OpenStatus) + if err := op.Validate(); err != nil { + return err + } + b.Append(op) + return nil +} + +// Convenience function to apply the operation +func Close(b Interface, author Person, unixTime int64) error { + op := NewSetStatusOp(author, unixTime, ClosedStatus) + if err := op.Validate(); err != nil { + return err + } + b.Append(op) + return nil +} diff --git a/bug/op_set_title.go b/bug/op_set_title.go new file mode 100644 index 00000000..e8c5caf8 --- /dev/null +++ b/bug/op_set_title.go @@ -0,0 +1,93 @@ +package bug + +import ( + "fmt" + "strings" + + "github.com/MichaelMure/git-bug/util/text" +) + +// SetTitleOperation will change the title of a bug + +var _ Operation = SetTitleOperation{} + +type SetTitleOperation struct { + *OpBase + Title string `json:"title"` + Was string `json:"was"` +} + +func (op SetTitleOperation) base() *OpBase { + return op.OpBase +} + +func (op SetTitleOperation) Apply(snapshot Snapshot) Snapshot { + snapshot.Title = op.Title + + return snapshot +} + +func (op SetTitleOperation) Validate() error { + if err := opBaseValidate(op, 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 Person, unixTime int64, title string, was string) SetTitleOperation { + return SetTitleOperation{ + OpBase: newOpBase(SetTitleOp, author, unixTime), + Title: title, + Was: was, + } +} + +// Convenience function to apply the operation +func SetTitle(b Interface, author Person, unixTime int64, title string) error { + it := NewOperationIterator(b) + + var lastTitleOp Operation + for it.Next() { + op := it.Value() + if op.base().OperationType == SetTitleOp { + lastTitleOp = op + } + } + + var was string + if lastTitleOp != nil { + was = lastTitleOp.(SetTitleOperation).Title + } else { + was = b.FirstOp().(CreateOperation).Title + } + + setTitleOp := NewSetTitleOp(author, unixTime, title, was) + + if err := setTitleOp.Validate(); err != nil { + return err + } + + b.Append(setTitleOp) + return nil +} diff --git a/bug/operation.go b/bug/operation.go index a408e167..cd4094dc 100644 --- a/bug/operation.go +++ b/bug/operation.go @@ -22,14 +22,12 @@ const ( // Operation define the interface to fulfill for an edit operation of a Bug type Operation interface { - // OpType return the type of operation - OpType() OperationType + // base return the OpBase of the Operation, for package internal use + base() *OpBase // Time return the time when the operation was added Time() time.Time // GetUnixTime return the unix timestamp when the operation was added GetUnixTime() int64 - // GetAuthor return the author of the operation - GetAuthor() Person // GetFiles return the files needed by this operation GetFiles() []git.Hash // Apply the operation to a Snapshot to create the final state @@ -50,8 +48,8 @@ type OpBase struct { Metadata map[string]string `json:"metadata,omitempty"` } -// NewOpBase is the constructor for an OpBase -func NewOpBase(opType OperationType, author Person, unixTime int64) *OpBase { +// newOpBase is the constructor for an OpBase +func newOpBase(opType OperationType, author Person, unixTime int64) *OpBase { return &OpBase{ OperationType: opType, Author: author, @@ -59,11 +57,6 @@ func NewOpBase(opType OperationType, author Person, unixTime int64) *OpBase { } } -// OpType return the type of operation -func (op *OpBase) OpType() OperationType { - return op.OperationType -} - // Time return the time when the operation was added func (op *OpBase) Time() time.Time { return time.Unix(op.UnixTime, 0) @@ -74,27 +67,22 @@ 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()) +func opBaseValidate(op Operation, opType OperationType) error { + if op.base().OperationType != opType { + return fmt.Errorf("incorrect operation type (expected: %v, actual: %v)", opType, op.base().OperationType) } if op.GetUnixTime() == 0 { return fmt.Errorf("time not set") } - if err := op.GetAuthor().Validate(); err != nil { + if err := op.base().Author.Validate(); err != nil { return errors.Wrap(err, "author") } diff --git a/bug/operation_iterator_test.go b/bug/operation_iterator_test.go new file mode 100644 index 00000000..506cc94f --- /dev/null +++ b/bug/operation_iterator_test.go @@ -0,0 +1,59 @@ +package bug + +import ( + "github.com/MichaelMure/git-bug/repository" + "testing" + "time" +) + +var ( + rene = Person{ + Name: "René Descartes", + Email: "rene@descartes.fr", + } + + unix = time.Now().Unix() + + createOp = NewCreateOp(rene, unix, "title", "message", nil) + setTitleOp = NewSetTitleOp(rene, unix, "title2", "title1") + addCommentOp = NewAddCommentOp(rene, unix, "message2", nil) + setStatusOp = NewSetStatusOp(rene, unix, ClosedStatus) + labelChangeOp = NewLabelChangeOperation(rene, unix, []Label{"added"}, []Label{"removed"}) +) + +func TestOpIterator(t *testing.T) { + mockRepo := repository.NewMockRepoForTest() + + bug1 := NewBug() + + // first pack + bug1.Append(createOp) + bug1.Append(setTitleOp) + bug1.Append(addCommentOp) + bug1.Append(setStatusOp) + bug1.Append(labelChangeOp) + bug1.Commit(mockRepo) + + // second pack + bug1.Append(setTitleOp) + bug1.Append(setTitleOp) + bug1.Append(setTitleOp) + bug1.Commit(mockRepo) + + // staging + bug1.Append(setTitleOp) + bug1.Append(setTitleOp) + bug1.Append(setTitleOp) + + it := NewOperationIterator(bug1) + + counter := 0 + for it.Next() { + _ = it.Value() + counter++ + } + + if counter != 11 { + t.Fatalf("Wrong count of value iterated (%d instead of 8)", counter) + } +} diff --git a/bug/operation_pack.go b/bug/operation_pack.go index 03d538d5..2da8bee0 100644 --- a/bug/operation_pack.go +++ b/bug/operation_pack.go @@ -3,7 +3,6 @@ package bug import ( "encoding/json" "fmt" - "reflect" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/git" @@ -25,17 +24,6 @@ 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) - } - operations[t] = reflect.TypeOf(op) -} - func (opp *OperationPack) MarshalJSON() ([]byte, error) { return json.Marshal(struct { Version uint `json:"version"` @@ -69,25 +57,44 @@ func (opp *OperationPack) UnmarshalJSON(data []byte) error { return err } - opType, ok := operations[t.OperationType] - if !ok { - return fmt.Errorf("unknown operation type %v", t.OperationType) - } - - op := reflect.New(opType).Interface() - - if err := json.Unmarshal(raw, op); err != nil { + op, err := opp.unmarshalOp(raw, t.OperationType) + if err != nil { return err } - deref := reflect.ValueOf(op).Elem().Interface() - - opp.Operations = append(opp.Operations, deref.(Operation)) + opp.Operations = append(opp.Operations, op) } return nil } +func (opp *OperationPack) unmarshalOp(raw []byte, _type OperationType) (Operation, error) { + switch _type { + case CreateOp: + op := CreateOperation{} + err := json.Unmarshal(raw, &op) + return op, err + case SetTitleOp: + op := SetTitleOperation{} + err := json.Unmarshal(raw, &op) + return op, err + case AddCommentOp: + op := AddCommentOperation{} + err := json.Unmarshal(raw, &op) + return op, err + case SetStatusOp: + op := SetStatusOperation{} + err := json.Unmarshal(raw, &op) + return op, err + case LabelChangeOp: + op := LabelChangeOperation{} + err := json.Unmarshal(raw, &op) + return op, err + default: + return nil, fmt.Errorf("unknown operation type %v", _type) + } +} + // Append a new operation to the pack func (opp *OperationPack) Append(op Operation) { opp.Operations = append(opp.Operations, op) diff --git a/bug/operation_pack_test.go b/bug/operation_pack_test.go new file mode 100644 index 00000000..48f9f80c --- /dev/null +++ b/bug/operation_pack_test.go @@ -0,0 +1,53 @@ +package bug + +import ( + "encoding/json" + "testing" + + "github.com/MichaelMure/git-bug/util/git" + "github.com/go-test/deep" +) + +func TestOperationPackSerialize(t *testing.T) { + opp := &OperationPack{} + + opp.Append(createOp) + opp.Append(setTitleOp) + opp.Append(addCommentOp) + opp.Append(setStatusOp) + opp.Append(labelChangeOp) + + opMeta := NewCreateOp(rene, unix, "title", "message", nil) + opMeta.SetMetadata("key", "value") + opp.Append(opMeta) + + if len(opMeta.Metadata) != 1 { + t.Fatal() + } + + opFile := NewCreateOp(rene, unix, "title", "message", []git.Hash{ + "abcdef", + "ghijkl", + }) + opp.Append(opFile) + + if len(opFile.Files) != 2 { + t.Fatal() + } + + data, err := json.Marshal(opp) + if err != nil { + t.Fatal(err) + } + + var opp2 *OperationPack + err = json.Unmarshal(data, &opp2) + if err != nil { + t.Fatal(err) + } + + deep.CompareUnexportedFields = false + if diff := deep.Equal(opp, opp2); diff != nil { + t.Fatal(diff) + } +} diff --git a/bug/operation_test.go b/bug/operation_test.go new file mode 100644 index 00000000..8561adf3 --- /dev/null +++ b/bug/operation_test.go @@ -0,0 +1,70 @@ +package bug + +import ( + "testing" + "time" + + "github.com/MichaelMure/git-bug/util/git" +) + +func TestValidate(t *testing.T) { + rene := Person{ + Name: "René Descartes", + Email: "rene@descartes.fr", + } + + unix := time.Now().Unix() + + good := []Operation{ + NewCreateOp(rene, unix, "title", "message", nil), + NewSetTitleOp(rene, unix, "title2", "title1"), + NewAddCommentOp(rene, unix, "message2", nil), + NewSetStatusOp(rene, unix, ClosedStatus), + NewLabelChangeOperation(rene, unix, []Label{"added"}, []Label{"removed"}), + } + + for _, op := range good { + if err := op.Validate(); err != nil { + t.Fatal(err) + } + } + + bad := []Operation{ + // opbase + NewSetStatusOp(Person{Name: "", Email: "rene@descartes.fr"}, unix, ClosedStatus), + NewSetStatusOp(Person{Name: "René Descartes\u001b", Email: "rene@descartes.fr"}, unix, ClosedStatus), + NewSetStatusOp(Person{Name: "René Descartes", Email: "rene@descartes.fr\u001b"}, unix, ClosedStatus), + NewSetStatusOp(Person{Name: "René \nDescartes", Email: "rene@descartes.fr"}, unix, ClosedStatus), + NewSetStatusOp(Person{Name: "René Descartes", Email: "rene@\ndescartes.fr"}, unix, ClosedStatus), + CreateOperation{OpBase: &OpBase{ + Author: rene, + UnixTime: 0, + OperationType: CreateOp, + }, + Title: "title", + Message: "message", + }, + + NewCreateOp(rene, unix, "multi\nline", "message", nil), + NewCreateOp(rene, unix, "title", "message", []git.Hash{git.Hash("invalid")}), + NewCreateOp(rene, unix, "title\u001b", "message", nil), + NewCreateOp(rene, unix, "title", "message\u001b", nil), + NewSetTitleOp(rene, unix, "multi\nline", "title1"), + NewSetTitleOp(rene, unix, "title", "multi\nline"), + NewSetTitleOp(rene, unix, "title\u001b", "title2"), + NewSetTitleOp(rene, unix, "title", "title2\u001b"), + NewAddCommentOp(rene, unix, "", nil), + NewAddCommentOp(rene, unix, "message\u001b", nil), + NewAddCommentOp(rene, unix, "message", []git.Hash{git.Hash("invalid")}), + NewSetStatusOp(rene, unix, 1000), + NewSetStatusOp(rene, unix, 0), + NewLabelChangeOperation(rene, unix, []Label{}, []Label{}), + NewLabelChangeOperation(rene, unix, []Label{"multi\nline"}, []Label{}), + } + + for i, op := range bad { + if err := op.Validate(); err == nil { + t.Fatal("validation should have failed", i, op) + } + } +} -- cgit