diff options
40 files changed, 375 insertions, 464 deletions
@@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/git" @@ -26,20 +27,24 @@ const createClockEntryPattern = "create-clock-%d" const editClockEntryPrefix = "edit-clock-" const editClockEntryPattern = "edit-clock-%d" -const idLength = 40 -const humanIdLength = 7 - var ErrBugNotExist = errors.New("bug doesn't exist") type ErrMultipleMatch struct { - Matching []string + Matching []entity.Id } func (e ErrMultipleMatch) Error() string { - return fmt.Sprintf("Multiple matching bug found:\n%s", strings.Join(e.Matching, "\n")) + matching := make([]string, len(e.Matching)) + + for i, match := range e.Matching { + matching[i] = match.String() + } + + return fmt.Sprintf("Multiple matching bug found:\n%s", strings.Join(matching, "\n")) } var _ Interface = &Bug{} +var _ entity.Interface = &Bug{} // Bug hold the data of a bug thread, organized in a way close to // how it will be persisted inside Git. This is the data structure @@ -53,7 +58,7 @@ type Bug struct { editTime lamport.Time // Id used as unique identifier - id string + id entity.Id lastCommit git.Hash rootPack git.Hash @@ -82,10 +87,10 @@ func FindLocalBug(repo repository.ClockedRepo, prefix string) (*Bug, error) { } // preallocate but empty - matching := make([]string, 0, 5) + matching := make([]entity.Id, 0, 5) for _, id := range ids { - if strings.HasPrefix(id, prefix) { + if id.HasPrefix(prefix) { matching = append(matching, id) } } @@ -102,8 +107,8 @@ func FindLocalBug(repo repository.ClockedRepo, prefix string) (*Bug, error) { } // ReadLocalBug will read a local bug from its hash -func ReadLocalBug(repo repository.ClockedRepo, id string) (*Bug, error) { - ref := bugsRefPattern + id +func ReadLocalBug(repo repository.ClockedRepo, id entity.Id) (*Bug, error) { + ref := bugsRefPattern + id.String() return readBug(repo, ref) } @@ -116,10 +121,10 @@ func ReadRemoteBug(repo repository.ClockedRepo, remote string, id string) (*Bug, // readBug will read and parse a Bug from git func readBug(repo repository.ClockedRepo, ref string) (*Bug, error) { refSplit := strings.Split(ref, "/") - id := refSplit[len(refSplit)-1] + id := entity.Id(refSplit[len(refSplit)-1]) - if len(id) != idLength { - return nil, fmt.Errorf("invalid ref length") + if err := id.Validate(); err != nil { + return nil, errors.Wrap(err, "invalid ref ") } hashes, err := repo.ListCommits(ref) @@ -278,7 +283,7 @@ func readAllBugs(repo repository.ClockedRepo, refPrefix string) <-chan StreamedB } // ListLocalIds list all the available local bug ids -func ListLocalIds(repo repository.Repo) ([]string, error) { +func ListLocalIds(repo repository.Repo) ([]entity.Id, error) { refs, err := repo.ListRefs(bugsRefPattern) if err != nil { return nil, err @@ -287,12 +292,12 @@ func ListLocalIds(repo repository.Repo) ([]string, error) { return refsToIds(refs), nil } -func refsToIds(refs []string) []string { - ids := make([]string, len(refs)) +func refsToIds(refs []string) []entity.Id { + ids := make([]entity.Id, len(refs)) for i, ref := range refs { split := strings.Split(ref, "/") - ids[i] = split[len(split)-1] + ids[i] = entity.Id(split[len(split)-1]) } return ids @@ -325,8 +330,8 @@ func (bug *Bug) Validate() error { return fmt.Errorf("first operation should be a Create op") } - // The bug ID should be the hash of the first commit - if len(bug.packs) > 0 && string(bug.packs[0].commitHash) != bug.id { + // The bug Id should be the hash of the first commit + if len(bug.packs) > 0 && string(bug.packs[0].commitHash) != bug.id.String() { return fmt.Errorf("bug id should be the first commit hash") } @@ -456,7 +461,7 @@ func (bug *Bug) Commit(repo repository.ClockedRepo) error { // if it was the first commit, use the commit hash as bug id if bug.id == "" { - bug.id = string(hash) + bug.id = entity.Id(hash) } // Create or update the Git reference for this bug @@ -594,7 +599,7 @@ func (bug *Bug) Merge(repo repository.Repo, other Interface) (bool, error) { } // Update the git ref - err = repo.UpdateRef(bugsRefPattern+bug.id, bug.lastCommit) + err = repo.UpdateRef(bugsRefPattern+bug.id.String(), bug.lastCommit) if err != nil { return false, err } @@ -603,7 +608,7 @@ func (bug *Bug) Merge(repo repository.Repo, other Interface) (bool, error) { } // Id return the Bug identifier -func (bug *Bug) Id() string { +func (bug *Bug) Id() entity.Id { if bug.id == "" { // simply panic as it would be a coding error // (using an id of a bug not stored yet) @@ -612,16 +617,6 @@ func (bug *Bug) Id() string { return bug.id } -// HumanId return the Bug identifier truncated for human consumption -func (bug *Bug) HumanId() string { - return FormatHumanID(bug.Id()) -} - -func FormatHumanID(id string) string { - format := fmt.Sprintf("%%.%ds", humanIdLength) - return fmt.Sprintf(format, id) -} - // CreateLamportTime return the Lamport time of creation func (bug *Bug) CreateLamportTime() lamport.Time { return bug.createTime diff --git a/bug/bug_actions.go b/bug/bug_actions.go index b26d080f..e6fc12d5 100644 --- a/bug/bug_actions.go +++ b/bug/bug_actions.go @@ -81,7 +81,7 @@ func MergeAll(repo repository.ClockedRepo, remote string) <-chan entity.MergeRes continue } - localRef := bugsRefPattern + remoteBug.Id() + localRef := bugsRefPattern + remoteBug.Id().String() localExist, err := repo.RefExist(localRef) if err != nil { diff --git a/bug/comment.go b/bug/comment.go index 5db0b18d..47c1ff05 100644 --- a/bug/comment.go +++ b/bug/comment.go @@ -1,6 +1,7 @@ package bug import ( + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/git" "github.com/MichaelMure/git-bug/util/timestamp" @@ -9,7 +10,7 @@ import ( // Comment represent a comment in a Bug type Comment struct { - id string + id entity.Id Author identity.Interface Message string Files []git.Hash @@ -20,7 +21,7 @@ type Comment struct { } // Id return the Comment identifier -func (c Comment) Id() string { +func (c Comment) Id() entity.Id { if c.id == "" { // simply panic as it would be a coding error // (using an id of an identity not stored yet) @@ -29,11 +30,6 @@ func (c Comment) Id() string { return c.id } -// HumanId return the Comment identifier truncated for human consumption -func (c Comment) HumanId() string { - return FormatHumanID(c.Id()) -} - // FormatTimeRel format the UnixTime of the comment for human consumption func (c Comment) FormatTimeRel() string { return humanize.Time(c.UnixTime.Time()) diff --git a/bug/interface.go b/bug/interface.go index 186c26fc..8266e99e 100644 --- a/bug/interface.go +++ b/bug/interface.go @@ -1,16 +1,14 @@ package bug import ( + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/lamport" ) type Interface interface { // Id return the Bug identifier - Id() string - - // HumanId return the Bug identifier truncated for human consumption - HumanId() string + Id() entity.Id // Validate check if the Bug data is valid Validate() error diff --git a/bug/op_add_comment.go b/bug/op_add_comment.go index 1ff2ae87..e16ea0dd 100644 --- a/bug/op_add_comment.go +++ b/bug/op_add_comment.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/git" "github.com/MichaelMure/git-bug/util/text" @@ -15,16 +16,16 @@ var _ Operation = &AddCommentOperation{} // AddCommentOperation will add a new comment in the bug type AddCommentOperation struct { OpBase - Message string + Message string `json:"message"` // TODO: change for a map[string]util.hash to store the filename ? - Files []git.Hash + Files []git.Hash `json:"files"` } func (op *AddCommentOperation) base() *OpBase { return &op.OpBase } -func (op *AddCommentOperation) ID() string { +func (op *AddCommentOperation) Id() entity.Id { return idOperation(op) } @@ -33,7 +34,7 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) { snapshot.addParticipant(op.Author) comment := Comment{ - id: op.ID(), + id: op.Id(), Message: op.Message, Author: op.Author, Files: op.Files, @@ -43,7 +44,7 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) { snapshot.Comments = append(snapshot.Comments, comment) item := &AddCommentTimelineItem{ - CommentTimelineItem: NewCommentTimelineItem(op.ID(), comment), + CommentTimelineItem: NewCommentTimelineItem(op.Id(), comment), } snapshot.Timeline = append(snapshot.Timeline, item) @@ -65,28 +66,9 @@ func (op *AddCommentOperation) Validate() error { return nil } -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON -func (op *AddCommentOperation) MarshalJSON() ([]byte, error) { - base, err := json.Marshal(op.OpBase) - if err != nil { - return nil, err - } - - // revert back to a flat map to be able to add our own fields - var data map[string]interface{} - if err := json.Unmarshal(base, &data); err != nil { - return nil, err - } - - data["message"] = op.Message - data["files"] = op.Files - - return json.Marshal(data) -} - -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON +// UnmarshalJSON is a two step JSON unmarshaling +// This workaround is necessary to avoid the inner OpBase.MarshalJSON +// overriding the outer op's MarshalJSON func (op *AddCommentOperation) UnmarshalJSON(data []byte) error { // Unmarshal OpBase and the op separately diff --git a/bug/op_add_comment_test.go b/bug/op_add_comment_test.go index a1789aa0..55bafe36 100644 --- a/bug/op_add_comment_test.go +++ b/bug/op_add_comment_test.go @@ -2,6 +2,7 @@ package bug import ( "encoding/json" + "fmt" "testing" "time" @@ -21,8 +22,9 @@ func TestAddCommentSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) - // enforce creating the ID - before.ID() + // enforce creating the IDs + before.Id() + rene.Id() assert.Equal(t, before, &after) } diff --git a/bug/op_create.go b/bug/op_create.go index e3d0a63a..0da95d4d 100644 --- a/bug/op_create.go +++ b/bug/op_create.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/git" "github.com/MichaelMure/git-bug/util/text" @@ -16,16 +17,16 @@ var _ Operation = &CreateOperation{} // CreateOperation define the initial creation of a bug type CreateOperation struct { OpBase - Title string - Message string - Files []git.Hash + Title string `json:"title"` + Message string `json:"message"` + Files []git.Hash `json:"files"` } func (op *CreateOperation) base() *OpBase { return &op.OpBase } -func (op *CreateOperation) ID() string { +func (op *CreateOperation) Id() entity.Id { return idOperation(op) } @@ -36,7 +37,7 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) { snapshot.Title = op.Title comment := Comment{ - id: op.ID(), + id: op.Id(), Message: op.Message, Author: op.Author, UnixTime: timestamp.Timestamp(op.UnixTime), @@ -48,7 +49,7 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) { snapshot.Timeline = []TimelineItem{ &CreateTimelineItem{ - CommentTimelineItem: NewCommentTimelineItem(op.ID(), comment), + CommentTimelineItem: NewCommentTimelineItem(op.Id(), comment), }, } } @@ -81,29 +82,9 @@ func (op *CreateOperation) Validate() error { return nil } -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON -func (op *CreateOperation) MarshalJSON() ([]byte, error) { - base, err := json.Marshal(op.OpBase) - if err != nil { - return nil, err - } - - // revert back to a flat map to be able to add our own fields - var data map[string]interface{} - if err := json.Unmarshal(base, &data); err != nil { - return nil, err - } - - data["title"] = op.Title - data["message"] = op.Message - data["files"] = op.Files - - return json.Marshal(data) -} - -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON +// UnmarshalJSON is a two step JSON unmarshaling +// This workaround is necessary to avoid the inner OpBase.MarshalJSON +// overriding the outer op's MarshalJSON func (op *CreateOperation) UnmarshalJSON(data []byte) error { // Unmarshal OpBase and the op separately diff --git a/bug/op_create_test.go b/bug/op_create_test.go index 20c8ec69..ec53b04b 100644 --- a/bug/op_create_test.go +++ b/bug/op_create_test.go @@ -20,11 +20,11 @@ func TestCreate(t *testing.T) { create.Apply(&snapshot) - id := create.ID() - assert.True(t, IDIsValid(id)) + id := create.Id() + assert.NoError(t, id.Validate()) comment := Comment{ - id: string(id), + id: id, Author: rene, Message: "message", UnixTime: timestamp.Timestamp(create.UnixTime), @@ -61,8 +61,9 @@ func TestCreateSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) - // enforce creating the ID - before.ID() + // enforce creating the IDs + before.Id() + rene.Id() assert.Equal(t, before, &after) } diff --git a/bug/op_edit_comment.go b/bug/op_edit_comment.go index 2f269b02..a37ce8f7 100644 --- a/bug/op_edit_comment.go +++ b/bug/op_edit_comment.go @@ -4,6 +4,9 @@ import ( "encoding/json" "fmt" + "github.com/pkg/errors" + + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/timestamp" @@ -16,16 +19,16 @@ var _ Operation = &EditCommentOperation{} // EditCommentOperation will change a comment in the bug type EditCommentOperation struct { OpBase - Target string - Message string - Files []git.Hash + Target entity.Id `json:"target"` + Message string `json:"message"` + Files []git.Hash `json:"files"` } func (op *EditCommentOperation) base() *OpBase { return &op.OpBase } -func (op *EditCommentOperation) ID() string { +func (op *EditCommentOperation) Id() entity.Id { return idOperation(op) } @@ -38,7 +41,7 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) { var target TimelineItem for i, item := range snapshot.Timeline { - if item.ID() == op.Target { + if item.Id() == op.Target { target = snapshot.Timeline[i] break } @@ -86,8 +89,8 @@ func (op *EditCommentOperation) Validate() error { return err } - if !IDIsValid(op.Target) { - return fmt.Errorf("target hash is invalid") + if err := op.Target.Validate(); err != nil { + return errors.Wrap(err, "target hash is invalid") } if !text.Safe(op.Message) { @@ -97,29 +100,9 @@ func (op *EditCommentOperation) Validate() error { return nil } -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON -func (op *EditCommentOperation) MarshalJSON() ([]byte, error) { - base, err := json.Marshal(op.OpBase) - if err != nil { - return nil, err - } - - // revert back to a flat map to be able to add our own fields - var data map[string]interface{} - if err := json.Unmarshal(base, &data); err != nil { - return nil, err - } - - data["target"] = op.Target - data["message"] = op.Message - data["files"] = op.Files - - return json.Marshal(data) -} - -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON +// UnmarshalJSON is a two step JSON unmarshaling +// This workaround is necessary to avoid the inner OpBase.MarshalJSON +// overriding the outer op's MarshalJSON func (op *EditCommentOperation) UnmarshalJSON(data []byte) error { // Unmarshal OpBase and the op separately @@ -130,7 +113,7 @@ func (op *EditCommentOperation) UnmarshalJSON(data []byte) error { } aux := struct { - Target string `json:"target"` + Target entity.Id `json:"target"` Message string `json:"message"` Files []git.Hash `json:"files"` }{} @@ -151,7 +134,7 @@ func (op *EditCommentOperation) UnmarshalJSON(data []byte) error { // Sign post method for gqlgen func (op *EditCommentOperation) IsAuthored() {} -func NewEditCommentOp(author identity.Interface, unixTime int64, target string, message string, files []git.Hash) *EditCommentOperation { +func NewEditCommentOp(author identity.Interface, unixTime int64, target entity.Id, message string, files []git.Hash) *EditCommentOperation { return &EditCommentOperation{ OpBase: newOpBase(EditCommentOp, author, unixTime), Target: target, @@ -161,11 +144,11 @@ func NewEditCommentOp(author identity.Interface, unixTime int64, target string, } // Convenience function to apply the operation -func EditComment(b Interface, author identity.Interface, unixTime int64, target string, message string) (*EditCommentOperation, error) { +func EditComment(b Interface, author identity.Interface, unixTime int64, target entity.Id, message string) (*EditCommentOperation, error) { return EditCommentWithFiles(b, author, unixTime, target, message, nil) } -func EditCommentWithFiles(b Interface, author identity.Interface, unixTime int64, target string, message string, files []git.Hash) (*EditCommentOperation, error) { +func EditCommentWithFiles(b Interface, author identity.Interface, unixTime int64, target entity.Id, message string, files []git.Hash) (*EditCommentOperation, error) { editCommentOp := NewEditCommentOp(author, unixTime, target, message, files) if err := editCommentOp.Validate(); err != nil { return nil, err diff --git a/bug/op_edit_comment_test.go b/bug/op_edit_comment_test.go index 9e8739da..abd550cb 100644 --- a/bug/op_edit_comment_test.go +++ b/bug/op_edit_comment_test.go @@ -20,14 +20,14 @@ func TestEdit(t *testing.T) { create := NewCreateOp(rene, unix, "title", "create", nil) create.Apply(&snapshot) - hash1 := create.ID() - require.True(t, IDIsValid(hash1)) + id1 := create.Id() + require.NoError(t, id1.Validate()) comment1 := NewAddCommentOp(rene, unix, "comment 1", nil) comment1.Apply(&snapshot) - hash2 := comment1.ID() - require.True(t, IDIsValid(hash2)) + id2 := comment1.Id() + require.NoError(t, id2.Validate()) // add another unrelated op in between setTitle := NewSetTitleOp(rene, unix, "edited title", "title") @@ -36,10 +36,10 @@ func TestEdit(t *testing.T) { comment2 := NewAddCommentOp(rene, unix, "comment 2", nil) comment2.Apply(&snapshot) - hash3 := comment2.ID() - require.True(t, IDIsValid(hash3)) + id3 := comment2.Id() + require.NoError(t, id3.Validate()) - edit := NewEditCommentOp(rene, unix, hash1, "create edited", nil) + edit := NewEditCommentOp(rene, unix, id1, "create edited", nil) edit.Apply(&snapshot) assert.Equal(t, len(snapshot.Timeline), 4) @@ -50,7 +50,7 @@ func TestEdit(t *testing.T) { assert.Equal(t, snapshot.Comments[1].Message, "comment 1") assert.Equal(t, snapshot.Comments[2].Message, "comment 2") - edit2 := NewEditCommentOp(rene, unix, hash2, "comment 1 edited", nil) + edit2 := NewEditCommentOp(rene, unix, id2, "comment 1 edited", nil) edit2.Apply(&snapshot) assert.Equal(t, len(snapshot.Timeline), 4) @@ -61,7 +61,7 @@ func TestEdit(t *testing.T) { assert.Equal(t, snapshot.Comments[1].Message, "comment 1 edited") assert.Equal(t, snapshot.Comments[2].Message, "comment 2") - edit3 := NewEditCommentOp(rene, unix, hash3, "comment 2 edited", nil) + edit3 := NewEditCommentOp(rene, unix, id3, "comment 2 edited", nil) edit3.Apply(&snapshot) assert.Equal(t, len(snapshot.Timeline), 4) @@ -85,8 +85,9 @@ func TestEditCommentSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) - // enforce creating the ID - before.ID() + // enforce creating the IDs + before.Id() + rene.Id() assert.Equal(t, before, &after) } diff --git a/bug/op_label_change.go b/bug/op_label_change.go index 5afcceb7..c911de26 100644 --- a/bug/op_label_change.go +++ b/bug/op_label_change.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/timestamp" ) @@ -16,15 +17,15 @@ var _ Operation = &LabelChangeOperation{} // LabelChangeOperation define a Bug operation to add or remove labels type LabelChangeOperation struct { OpBase - Added []Label - Removed []Label + Added []Label `json:"added"` + Removed []Label `json:"removed"` } func (op *LabelChangeOperation) base() *OpBase { return &op.OpBase } -func (op *LabelChangeOperation) ID() string { +func (op *LabelChangeOperation) Id() entity.Id { return idOperation(op) } @@ -61,7 +62,7 @@ AddLoop: }) item := &LabelChangeTimelineItem{ - id: op.ID(), + id: op.Id(), Author: op.Author, UnixTime: timestamp.Timestamp(op.UnixTime), Added: op.Added, @@ -95,28 +96,9 @@ func (op *LabelChangeOperation) Validate() error { return nil } -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON -func (op *LabelChangeOperation) MarshalJSON() ([]byte, error) { - base, err := json.Marshal(op.OpBase) - if err != nil { - return nil, err - } - - // revert back to a flat map to be able to add our own fields - var data map[string]interface{} - if err := json.Unmarshal(base, &data); err != nil { - return nil, err - } - - data["added"] = op.Added - data["removed"] = op.Removed - - return json.Marshal(data) -} - -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON +// UnmarshalJSON is a two step JSON unmarshaling +// This workaround is necessary to avoid the inner OpBase.MarshalJSON +// overriding the outer op's MarshalJSON func (op *LabelChangeOperation) UnmarshalJSON(data []byte) error { // Unmarshal OpBase and the op separately @@ -155,14 +137,14 @@ func NewLabelChangeOperation(author identity.Interface, unixTime int64, added, r } type LabelChangeTimelineItem struct { - id string + id entity.Id Author identity.Interface UnixTime timestamp.Timestamp Added []Label Removed []Label } -func (l LabelChangeTimelineItem) ID() string { +func (l LabelChangeTimelineItem) Id() entity.Id { return l.id } diff --git a/bug/op_label_change_test.go b/bug/op_label_change_test.go index 7c311a8b..2a93e362 100644 --- a/bug/op_label_change_test.go +++ b/bug/op_label_change_test.go @@ -21,8 +21,9 @@ func TestLabelChangeSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) - // enforce creating the ID - before.ID() + // enforce creating the IDs + before.Id() + rene.Id() assert.Equal(t, before, &after) } diff --git a/bug/op_noop.go b/bug/op_noop.go index a1648434..16d32297 100644 --- a/bug/op_noop.go +++ b/bug/op_noop.go @@ -3,6 +3,7 @@ package bug import ( "encoding/json" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" ) @@ -19,7 +20,7 @@ func (op *NoOpOperation) base() *OpBase { return &op.OpBase } -func (op *NoOpOperation) ID() string { +func (op *NoOpOperation) Id() entity.Id { return idOperation(op) } @@ -31,25 +32,9 @@ func (op *NoOpOperation) Validate() error { return opBaseValidate(op, NoOpOp) } -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON -func (op *NoOpOperation) MarshalJSON() ([]byte, error) { - base, err := json.Marshal(op.OpBase) - if err != nil { - return nil, err - } - - // revert back to a flat map to be able to add our own fields - var data map[string]interface{} - if err := json.Unmarshal(base, &data); err != nil { - return nil, err - } - - return json.Marshal(data) -} - -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON +// UnmarshalJSON is a two step JSON unmarshaling +// This workaround is necessary to avoid the inner OpBase.MarshalJSON +// overriding the outer op's MarshalJSON func (op *NoOpOperation) UnmarshalJSON(data []byte) error { // Unmarshal OpBase and the op separately diff --git a/bug/op_noop_test.go b/bug/op_noop_test.go index 33e37bec..ea815948 100644 --- a/bug/op_noop_test.go +++ b/bug/op_noop_test.go @@ -21,8 +21,9 @@ func TestNoopSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) - // enforce creating the ID - before.ID() + // enforce creating the IDs + before.Id() + rene.Id() assert.Equal(t, before, &after) } diff --git a/bug/op_set_metadata.go b/bug/op_set_metadata.go index 19ddc3e3..f99f836b 100644 --- a/bug/op_set_metadata.go +++ b/bug/op_set_metadata.go @@ -2,8 +2,10 @@ package bug import ( "encoding/json" - "fmt" + "github.com/pkg/errors" + + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" ) @@ -11,21 +13,21 @@ var _ Operation = &SetMetadataOperation{} type SetMetadataOperation struct { OpBase - Target string - NewMetadata map[string]string + Target entity.Id `json:"target"` + NewMetadata map[string]string `json:"new_metadata"` } func (op *SetMetadataOperation) base() *OpBase { return &op.OpBase } -func (op *SetMetadataOperation) ID() string { +func (op *SetMetadataOperation) Id() entity.Id { return idOperation(op) } func (op *SetMetadataOperation) Apply(snapshot *Snapshot) { for _, target := range snapshot.Operations { - if target.ID() == op.Target { + if target.Id() == op.Target { base := target.base() if base.extraMetadata == nil { @@ -48,35 +50,16 @@ func (op *SetMetadataOperation) Validate() error { return err } - if !IDIsValid(op.Target) { - return fmt.Errorf("target hash is invalid") + if err := op.Target.Validate(); err != nil { + return errors.Wrap(err, "target invalid") } return nil } -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON -func (op *SetMetadataOperation) MarshalJSON() ([]byte, error) { - base, err := json.Marshal(op.OpBase) - if err != nil { - return nil, err - } - - // revert back to a flat map to be able to add our own fields - var data map[string]interface{} - if err := json.Unmarshal(base, &data); err != nil { - return nil, err - } - - data["target"] = op.Target - data["new_metadata"] = op.NewMetadata - - return json.Marshal(data) -} - -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON +// UnmarshalJSON is a two step JSON unmarshaling +// This workaround is necessary to avoid the inner OpBase.MarshalJSON +// overriding the outer op's MarshalJSON func (op *SetMetadataOperation) UnmarshalJSON(data []byte) error { // Unmarshal OpBase and the op separately @@ -87,7 +70,7 @@ func (op *SetMetadataOperation) UnmarshalJSON(data []byte) error { } aux := struct { - Target string `json:"target"` + Target entity.Id `json:"target"` NewMetadata map[string]string `json:"new_metadata"` }{} @@ -106,7 +89,7 @@ func (op *SetMetadataOperation) UnmarshalJSON(data []byte) error { // Sign post method for gqlgen func (op *SetMetadataOperation) IsAuthored() {} -func NewSetMetadataOp(author identity.Interface, unixTime int64, target string, newMetadata map[string]string) *SetMetadataOperation { +func NewSetMetadataOp(author identity.Interface, unixTime int64, target entity.Id, newMetadata map[string]string) *SetMetadataOperation { return &SetMetadataOperation{ OpBase: newOpBase(SetMetadataOp, author, unixTime), Target: target, @@ -115,7 +98,7 @@ func NewSetMetadataOp(author identity.Interface, unixTime int64, target string, } // Convenience function to apply the operation -func SetMetadata(b Interface, author identity.Interface, unixTime int64, target string, newMetadata map[string]string) (*SetMetadataOperation, error) { +func SetMetadata(b Interface, author identity.Interface, unixTime int64, target entity.Id, newMetadata map[string]string) (*SetMetadataOperation, error) { SetMetadataOp := NewSetMetadataOp(author, unixTime, target, newMetadata) if err := SetMetadataOp.Validate(); err != nil { return nil, err diff --git a/bug/op_set_metadata_test.go b/bug/op_set_metadata_test.go index 97e70cf4..389e91ac 100644 --- a/bug/op_set_metadata_test.go +++ b/bug/op_set_metadata_test.go @@ -21,18 +21,18 @@ func TestSetMetadata(t *testing.T) { create.Apply(&snapshot) snapshot.Operations = append(snapshot.Operations, create) - hash1 := create.ID() - require.True(t, IDIsValid(hash1)) + id1 := create.Id() + require.NoError(t, id1.Validate()) comment := NewAddCommentOp(rene, unix, "comment", nil) comment.SetMetadata("key2", "value2") comment.Apply(&snapshot) snapshot.Operations = append(snapshot.Operations, comment) - hash2 := comment.ID() - require.True(t, IDIsValid(hash2)) + id2 := comment.Id() + require.NoError(t, id2.Validate()) - op1 := NewSetMetadataOp(rene, unix, hash1, map[string]string{ + op1 := NewSetMetadataOp(rene, unix, id1, map[string]string{ "key": "override", "key2": "value", }) @@ -51,7 +51,7 @@ func TestSetMetadata(t *testing.T) { assert.Equal(t, len(commentMetadata), 1) assert.Equal(t, commentMetadata["key2"], "value2") - op2 := NewSetMetadataOp(rene, unix, hash2, map[string]string{ + op2 := NewSetMetadataOp(rene, unix, id2, map[string]string{ "key2": "value", "key3": "value3", }) @@ -71,7 +71,7 @@ func TestSetMetadata(t *testing.T) { // new key is set assert.Equal(t, commentMetadata["key3"], "value3") - op3 := NewSetMetadataOp(rene, unix, hash1, map[string]string{ + op3 := NewSetMetadataOp(rene, unix, id1, map[string]string{ "key": "override", "key2": "override", }) @@ -107,8 +107,9 @@ func TestSetMetadataSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) - // enforce creating the ID - before.ID() + // enforce creating the IDs + before.Id() + rene.Id() assert.Equal(t, before, &after) } diff --git a/bug/op_set_status.go b/bug/op_set_status.go index 76117b19..8a245184 100644 --- a/bug/op_set_status.go +++ b/bug/op_set_status.go @@ -5,6 +5,7 @@ import ( "github.com/pkg/errors" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/timestamp" ) @@ -14,14 +15,14 @@ var _ Operation = &SetStatusOperation{} // SetStatusOperation will change the status of a bug type SetStatusOperation struct { OpBase - Status Status + Status Status `json:"status"` } func (op *SetStatusOperation) base() *OpBase { return &op.OpBase } -func (op *SetStatusOperation) ID() string { +func (op *SetStatusOperation) Id() entity.Id { return idOperation(op) } @@ -30,7 +31,7 @@ func (op *SetStatusOperation) Apply(snapshot *Snapshot) { snapshot.addActor(op.Author) item := &SetStatusTimelineItem{ - id: op.ID(), + id: op.Id(), Author: op.Author, UnixTime: timestamp.Timestamp(op.UnixTime), Status: op.Status, @@ -51,27 +52,9 @@ func (op *SetStatusOperation) Validate() error { return nil } -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON -func (op *SetStatusOperation) MarshalJSON() ([]byte, error) { - base, err := json.Marshal(op.OpBase) - if err != nil { - return nil, err - } - - // revert back to a flat map to be able to add our own fields - var data map[string]interface{} - if err := json.Unmarshal(base, &data); err != nil { - return nil, err - } - - data["status"] = op.Status - - return json.Marshal(data) -} - -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON +// UnmarshalJSON is a two step JSON unmarshaling +// This workaround is necessary to avoid the inner OpBase.MarshalJSON +// overriding the outer op's MarshalJSON func (op *SetStatusOperation) UnmarshalJSON(data []byte) error { // Unmarshal OpBase and the op separately @@ -107,13 +90,13 @@ func NewSetStatusOp(author identity.Interface, unixTime int64, status Status) *S } type SetStatusTimelineItem struct { - id string + id entity.Id Author identity.Interface UnixTime timestamp.Timestamp Status Status } -func (s SetStatusTimelineItem) ID() string { +func (s SetStatusTimelineItem) Id() entity.Id { return s.id } diff --git a/bug/op_set_status_test.go b/bug/op_set_status_test.go index fc999234..ea032184 100644 --- a/bug/op_set_status_test.go +++ b/bug/op_set_status_test.go @@ -21,8 +21,9 @@ func TestSetStatusSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) - // enforce creating the ID - before.ID() + // enforce creating the IDs + before.Id() + rene.Id() assert.Equal(t, before, &after) } diff --git a/bug/op_set_title.go b/bug/op_set_title.go index 7bebfadc..fadd29a9 100644 --- a/bug/op_set_title.go +++ b/bug/op_set_title.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/timestamp" @@ -16,15 +17,15 @@ var _ Operation = &SetTitleOperation{} // SetTitleOperation will change the title of a bug type SetTitleOperation struct { OpBase - Title string - Was string + Title string `json:"title"` + Was string `json:"was"` } func (op *SetTitleOperation) base() *OpBase { return &op.OpBase } -func (op *SetTitleOperation) ID() string { +func (op *SetTitleOperation) Id() entity.Id { return idOperation(op) } @@ -33,7 +34,7 @@ func (op *SetTitleOperation) Apply(snapshot *Snapshot) { snapshot.addActor(op.Author) item := &SetTitleTimelineItem{ - id: op.ID(), + id: op.Id(), Author: op.Author, UnixTime: timestamp.Timestamp(op.UnixTime), Title: op.Title, @@ -71,28 +72,9 @@ func (op *SetTitleOperation) Validate() error { return nil } -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON -func (op *SetTitleOperation) MarshalJSON() ([]byte, error) { - base, err := json.Marshal(op.OpBase) - if err != nil { - return nil, err - } - - // revert back to a flat map to be able to add our own fields - var data map[string]interface{} - if err := json.Unmarshal(base, &data); err != nil { - return nil, err - } - - data["title"] = op.Title - data["was"] = op.Was - - return json.Marshal(data) -} - -// Workaround to avoid the inner OpBase.MarshalJSON overriding the outer op -// MarshalJSON +// UnmarshalJSON is a two step JSON unmarshaling +// This workaround is necessary to avoid the inner OpBase.MarshalJSON +// overriding the outer op's MarshalJSON func (op *SetTitleOperation) UnmarshalJSON(data []byte) error { // Unmarshal OpBase and the op separately @@ -131,14 +113,14 @@ func NewSetTitleOp(author identity.Interface, unixTime int64, title string, was } type SetTitleTimelineItem struct { - id string + id entity.Id Author identity.Interface UnixTime timestamp.Timestamp Title string Was string } -func (s SetTitleTimelineItem) ID() string { +func (s SetTitleTimelineItem) Id() entity.Id { return s.id } diff --git a/bug/op_set_title_test.go b/bug/op_set_title_test.go index 8ffe4f3c..19cbb12b 100644 --- a/bug/op_set_title_test.go +++ b/bug/op_set_title_test.go @@ -21,8 +21,9 @@ func TestSetTitleSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) - // enforce creating the ID - before.ID() + // enforce creating the IDs + before.Id() + rene.Id() assert.Equal(t, before, &after) } diff --git a/bug/operation.go b/bug/operation.go index 2298a008..dd95e096 100644 --- a/bug/operation.go +++ b/bug/operation.go @@ -6,10 +6,11 @@ import ( "fmt" "time" - "github.com/MichaelMure/git-bug/identity" + "github.com/pkg/errors" + "github.com/MichaelMure/git-bug/entity" + "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/git" - "github.com/pkg/errors" ) // OperationType is an operation type identifier @@ -27,14 +28,12 @@ const ( SetMetadataOp ) -const unsetIDMarker = "unset" - // Operation define the interface to fulfill for an edit operation of a Bug type Operation interface { // base return the OpBase of the Operation, for package internal use base() *OpBase - // ID return the identifier of the operation, to be used for back references - ID() string + // Id return the identifier of the operation, to be used for back references + Id() entity.Id // Time return the time when the operation was added Time() time.Time // GetUnixTime return the unix timestamp when the operation was added @@ -55,57 +54,42 @@ type Operation interface { GetAuthor() identity.Interface } -func hashRaw(data []byte) string { - hasher := sha256.New() - // Write can't fail - _, _ = hasher.Write(data) - return fmt.Sprintf("%x", hasher.Sum(nil)) +func deriveId(data []byte) entity.Id { + sum := sha256.Sum256(data) + return entity.Id(fmt.Sprintf("%x", sum)) } -func idOperation(op Operation) string { +func idOperation(op Operation) entity.Id { base := op.base() if base.id == "" { // something went really wrong panic("op's id not set") } - if base.id == "unset" { - // This means we are trying to get the op's ID *before* it has been stored, for instance when + if base.id == entity.UnsetId { + // This means we are trying to get the op's Id *before* it has been stored, for instance when // adding multiple ops in one go in an OperationPack. - // As the ID is computed based on the actual bytes written on the disk, we are going to predict - // those and then get the ID. This is safe as it will be the exact same code writing on disk later. + // As the Id is computed based on the actual bytes written on the disk, we are going to predict + // those and then get the Id. This is safe as it will be the exact same code writing on disk later. data, err := json.Marshal(op) if err != nil { panic(err) } - base.id = hashRaw(data) + base.id = deriveId(data) } return base.id } -func IDIsValid(id string) bool { - // IDs have the same format as a git hash - if len(id) != 40 && len(id) != 64 { - return false - } - for _, r := range id { - if (r < 'a' || r > 'z') && (r < '0' || r > '9') { - return false - } - } - return true -} - // OpBase implement the common code for all operations type OpBase struct { - OperationType OperationType - Author identity.Interface - UnixTime int64 - Metadata map[string]string + OperationType OperationType `json:"type"` + Author identity.Interface `json:"author"` + UnixTime int64 `json:"timestamp"` + Metadata map[string]string `json:"metadata,omitempty"` // Not serialized. Store the op's id in memory. - id string + id entity.Id // Not serialized. Store the extra metadata in memory, // compiled from SetMetadataOperation. extraMetadata map[string]string @@ -117,27 +101,13 @@ func newOpBase(opType OperationType, author identity.Interface, unixTime int64) OperationType: opType, Author: author, UnixTime: unixTime, - id: unsetIDMarker, + id: entity.UnsetId, } } -func (op OpBase) MarshalJSON() ([]byte, error) { - return json.Marshal(struct { - OperationType OperationType `json:"type"` - Author identity.Interface `json:"author"` - UnixTime int64 `json:"timestamp"` - Metadata map[string]string `json:"metadata,omitempty"` - }{ - OperationType: op.OperationType, - Author: op.Author, - UnixTime: op.UnixTime, - Metadata: op.Metadata, - }) -} - func (op *OpBase) UnmarshalJSON(data []byte) error { - // Compute the ID when loading the op from disk. - op.id = hashRaw(data) + // Compute the Id when loading the op from disk. + op.id = deriveId(data) aux := struct { OperationType OperationType `json:"type"` @@ -213,7 +183,7 @@ func (op *OpBase) SetMetadata(key string, value string) { } op.Metadata[key] = value - op.id = unsetIDMarker + op.id = entity.UnsetId } // GetMetadata retrieve arbitrary metadata about the operation diff --git a/bug/operation_pack_test.go b/bug/operation_pack_test.go index 713be7f0..21ac0a00 100644 --- a/bug/operation_pack_test.go +++ b/bug/operation_pack_test.go @@ -48,14 +48,16 @@ func TestOperationPackSerialize(t *testing.T) { err = json.Unmarshal(data, &opp2) assert.NoError(t, err) - ensureID(t, opp) + ensureIDs(t, opp) assert.Equal(t, opp, opp2) } -func ensureID(t *testing.T, opp *OperationPack) { +func ensureIDs(t *testing.T, opp *OperationPack) { for _, op := range opp.Operations { - id := op.ID() - require.True(t, IDIsValid(id)) + id := op.Id() + require.NoError(t, id.Validate()) + id = op.GetAuthor().Id() + require.NoError(t, id.Validate()) } } diff --git a/bug/operation_test.go b/bug/operation_test.go index c8dc246e..69c66dc8 100644 --- a/bug/operation_test.go +++ b/bug/operation_test.go @@ -94,26 +94,26 @@ func TestID(t *testing.T) { b, op, err := Create(rene, time.Now().Unix(), "title", "message") require.Nil(t, err) - id1 := op.ID() - require.True(t, IDIsValid(id1)) + id1 := op.Id() + require.NoError(t, id1.Validate()) err = b.Commit(repo) require.Nil(t, err) op2 := b.FirstOp() - id2 := op2.ID() - require.True(t, IDIsValid(id2)) + id2 := op2.Id() + require.NoError(t, id2.Validate()) require.Equal(t, id1, id2) - b2, err := ReadLocalBug(repo, b.id) + b2, err := ReadLocalBug(repo, b.Id()) require.Nil(t, err) op3 := b2.FirstOp() - id3 := op3.ID() - require.True(t, IDIsValid(id3)) + id3 := op3.Id() + require.NoError(t, id3.Validate()) require.Equal(t, id1, id3) } diff --git a/bug/snapshot.go b/bug/snapshot.go index b2e6f130..a937200f 100644 --- a/bug/snapshot.go +++ b/bug/snapshot.go @@ -4,12 +4,13 @@ import ( "fmt" "time" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" ) // Snapshot is a compiled form of the Bug data structure used for storage and merge type Snapshot struct { - id string + id entity.Id Status Status Title string @@ -26,15 +27,10 @@ type Snapshot struct { } // Return the Bug identifier -func (snap *Snapshot) Id() string { +func (snap *Snapshot) Id() entity.Id { return snap.id } -// Return the Bug identifier truncated for human consumption -func (snap *Snapshot) HumanId() string { - return FormatHumanID(snap.id) -} - // Return the last time a bug was modified func (snap *Snapshot) LastEditTime() time.Time { if len(snap.Operations) == 0 { @@ -59,9 +55,9 @@ func (snap *Snapshot) GetCreateMetadata(key string) (string, bool) { } // SearchTimelineItem will search in the timeline for an item matching the given hash -func (snap *Snapshot) SearchTimelineItem(ID string) (TimelineItem, error) { +func (snap *Snapshot) SearchTimelineItem(id entity.Id) (TimelineItem, error) { for i := range snap.Timeline { - if snap.Timeline[i].ID() == ID { + if snap.Timeline[i].Id() == id { return snap.Timeline[i], nil } } @@ -72,7 +68,7 @@ func (snap *Snapshot) SearchTimelineItem(ID string) (TimelineItem, error) { // SearchComment will search for a comment matching the given hash func (snap *Snapshot) SearchComment(id string) (*Comment, error) { for _, c := range snap.Comments { - if c.id == id { + if c.id.String() == id { return &c, nil } } @@ -103,7 +99,7 @@ func (snap *Snapshot) addParticipant(participant identity.Interface) { } // HasParticipant return true if the id is a participant -func (snap *Snapshot) HasParticipant(id string) bool { +func (snap *Snapshot) HasParticipant(id entity.Id) bool { for _, p := range snap.Participants { if p.Id() == id { return true @@ -113,7 +109,7 @@ func (snap *Snapshot) HasParticipant(id string) bool { } // HasAnyParticipant return true if one of the ids is a participant -func (snap *Snapshot) HasAnyParticipant(ids ...string) bool { +func (snap *Snapshot) HasAnyParticipant(ids ...entity.Id) bool { for _, id := range ids { if snap.HasParticipant(id) { return true @@ -123,7 +119,7 @@ func (snap *Snapshot) HasAnyParticipant(ids ...string) bool { } // HasActor return true if the id is a actor -func (snap *Snapshot) HasActor(id string) bool { +func (snap *Snapshot) HasActor(id entity.Id) bool { for _, p := range snap.Actors { if p.Id() == id { return true @@ -133,7 +129,7 @@ func (snap *Snapshot) HasActor(id string) bool { } // HasAnyActor return true if one of the ids is a actor -func (snap *Snapshot) HasAnyActor(ids ...string) bool { +func (snap *Snapshot) HasAnyActor(ids ...entity.Id) bool { for _, id := range ids { if snap.HasActor(id) { return true diff --git a/bug/timeline.go b/bug/timeline.go index 247c2927..4af1b92a 100644 --- a/bug/timeline.go +++ b/bug/timeline.go @@ -3,6 +3,7 @@ package bug import ( "strings" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/git" "github.com/MichaelMure/git-bug/util/timestamp" @@ -10,7 +11,7 @@ import ( type TimelineItem interface { // ID return the identifier of the item - ID() string + Id() entity.Id } // CommentHistoryStep hold one version of a message in the history @@ -25,7 +26,7 @@ type CommentHistoryStep struct { // CommentTimelineItem is a TimelineItem that holds a Comment and its edition history type CommentTimelineItem struct { - id string + id entity.Id Author identity.Interface Message string Files []git.Hash @@ -34,7 +35,7 @@ type CommentTimelineItem struct { History []CommentHistoryStep } -func NewCommentTimelineItem(ID string, comment Comment) CommentTimelineItem { +func NewCommentTimelineItem(ID entity.Id, comment Comment) CommentTimelineItem { return CommentTimelineItem{ id: ID, Author: comment.Author, @@ -51,7 +52,7 @@ func NewCommentTimelineItem(ID string, comment Comment) CommentTimelineItem { } } -func (c *CommentTimelineItem) ID() string { +func (c *CommentTimelineItem) Id() entity.Id { return c.id } diff --git a/cache/bug_excerpt.go b/cache/bug_excerpt.go index 8e9e5e37..8efc11e2 100644 --- a/cache/bug_excerpt.go +++ b/cache/bug_excerpt.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/MichaelMure/git-bug/bug" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" "github.com/MichaelMure/git-bug/util/lamport" ) @@ -17,7 +18,7 @@ func init() { // BugExcerpt hold a subset of the bug values to be able to sort and filter bugs // efficiently without having to read and compile each raw bugs. type BugExcerpt struct { - Id string + Id entity.ID CreateLamportTime lamport.Time EditLamportTime lamport.Time @@ -28,8 +29,8 @@ type BugExcerpt struct { Labels []bug.Label Title string LenComments int - Actors []string - Participants []string + Actors []entity.ID + Participants []entity.ID // If author is identity.Bare, LegacyAuthor is set // If author is identity.Identity, AuthorId is set and data is deported diff --git a/entity/err.go b/entity/err.go new file mode 100644 index 00000000..9356433d --- /dev/null +++ b/entity/err.go @@ -0,0 +1 @@ +package entity diff --git a/entity/id.go b/entity/id.go new file mode 100644 index 00000000..7fa1785a --- /dev/null +++ b/entity/id.go @@ -0,0 +1,65 @@ +package entity + +import ( + "fmt" + "io" + "strings" + + "github.com/pkg/errors" +) + +const IdLengthSHA1 = 40 +const IdLengthSHA256 = 64 +const humanIdLength = 7 + +const UnsetId = Id("unset") + +// Id is an identifier for an entity or part of an entity +type Id string + +func (i Id) String() string { + return string(i) +} + +func (i Id) Human() string { + format := fmt.Sprintf("%%.%ds", humanIdLength) + return fmt.Sprintf(format, i) +} + +func (i Id) HasPrefix(prefix string) bool { + return strings.HasPrefix(string(i), prefix) +} + +// UnmarshalGQL implement the Unmarshaler interface for gqlgen +func (i *Id) UnmarshalGQL(v interface{}) error { + _, ok := v.(string) + if !ok { + return fmt.Errorf("IDs must be strings") + } + + *i = v.(Id) + + if err := i.Validate(); err != nil { + return errors.Wrap(err, "invalid ID") + } + + return nil +} + +// MarshalGQL implement the Marshaler interface for gqlgen +func (i Id) MarshalGQL(w io.Writer) { + _, _ = w.Write([]byte(`"` + i.String() + `"`)) +} + +// IsValid tell if the Id is valid +func (i Id) Validate() error { + if len(i) != IdLengthSHA1 && len(i) != IdLengthSHA256 { + return fmt.Errorf("invalid length") + } + for _, r := range i { + if (r < 'a' || r > 'z') && (r < '0' || r > '9') { + return fmt.Errorf("invalid character") + } + } + return nil +} diff --git a/entity/interface.go b/entity/interface.go index 62b92a58..dd5d69b1 100644 --- a/entity/interface.go +++ b/entity/interface.go @@ -2,7 +2,5 @@ package entity type Interface interface { // Id return the Entity identifier - Id() string - // HumanId return the Entity identifier truncated for human consumption - HumanId() string + Id() Id } diff --git a/identity/bare.go b/identity/bare.go index 6af794dd..93cea91f 100644 --- a/identity/bare.go +++ b/identity/bare.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/lamport" "github.com/MichaelMure/git-bug/util/text" @@ -13,6 +14,7 @@ import ( ) var _ Interface = &Bare{} +var _ entity.Interface = &Bare{} // Bare is a very minimal identity, designed to be fully embedded directly along // other data. @@ -20,7 +22,7 @@ var _ Interface = &Bare{} // in particular, this identity is designed to be compatible with the handling of // identities in the early version of git-bug. type Bare struct { - id string + id entity.Id name string email string login string @@ -28,11 +30,16 @@ type Bare struct { } func NewBare(name string, email string) *Bare { - return &Bare{name: name, email: email} + return &Bare{id: entity.UnsetId, name: name, email: email} } func NewBareFull(name string, email string, login string, avatarUrl string) *Bare { - return &Bare{name: name, email: email, login: login, avatarUrl: avatarUrl} + return &Bare{id: entity.UnsetId, name: name, email: email, login: login, avatarUrl: avatarUrl} +} + +func deriveId(data []byte) entity.Id { + sum := sha256.Sum256(data) + return entity.Id(fmt.Sprintf("%x", sum)) } type bareIdentityJSON struct { @@ -43,15 +50,19 @@ type bareIdentityJSON struct { } func (i *Bare) MarshalJSON() ([]byte, error) { - return json.Marshal(bareIdentityJSON{ + data, err := json.Marshal(bareIdentityJSON{ Name: i.name, Email: i.email, Login: i.login, AvatarUrl: i.avatarUrl, }) + return data, err } func (i *Bare) UnmarshalJSON(data []byte) error { + // Compute the Id when loading the op from disk. + i.id = deriveId(data) + aux := bareIdentityJSON{} if err := json.Unmarshal(data, &aux); err != nil { @@ -67,30 +78,28 @@ func (i *Bare) UnmarshalJSON(data []byte) error { } // Id return the Identity identifier -func (i *Bare) Id() string { - // We don't have a proper ID at hand, so let's hash all the data to get one. - // Hopefully the - - if i.id != "" { - return i.id - } +func (i *Bare) Id() entity.Id { + // We don't have a proper Id at hand, so let's hash all the data to get one. - data, err := json.Marshal(i) - if err != nil { - panic(err) + if i.id == "" { + // something went really wrong + panic("identity's id not set") } + if i.id == entity.UnsetId { + // This means we are trying to get the identity identifier *before* it has been stored + // As the Id is computed based on the actual bytes written on the disk, we are going to predict + // those and then get the Id. This is safe as it will be the exact same code writing on disk later. - h := fmt.Sprintf("%x", sha256.New().Sum(data)[:16]) - i.id = string(h) + data, err := json.Marshal(i) + if err != nil { + panic(err) + } + i.id = deriveId(data) + } return i.id } -// HumanId return the Identity identifier truncated for human consumption -func (i *Bare) HumanId() string { - return FormatHumanID(i.Id()) -} - // Name return the last version of the name func (i *Bare) Name() string { return i.name diff --git a/identity/bare_test.go b/identity/bare_test.go index 7db9f644..335c8d37 100644 --- a/identity/bare_test.go +++ b/identity/bare_test.go @@ -5,12 +5,15 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/MichaelMure/git-bug/entity" ) func TestBare_Id(t *testing.T) { i := NewBare("name", "email") id := i.Id() - assert.Equal(t, "7b226e616d65223a226e616d65222c22", id) + expected := entity.Id("e18b853fbd89d5d40ca24811539c9a800c705abd9232f396954e8ca8bb63fa8a") + assert.Equal(t, expected, id) } func TestBareSerialize(t *testing.T) { @@ -28,5 +31,7 @@ func TestBareSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) + before.id = after.id + assert.Equal(t, before, &after) } diff --git a/identity/common.go b/identity/common.go index 2f2b9042..68a6d57c 100644 --- a/identity/common.go +++ b/identity/common.go @@ -37,11 +37,11 @@ func UnmarshalJSON(raw json.RawMessage) (Interface, error) { } // Fallback on a legacy Bare identity - var b Bare + b := &Bare{} - err = json.Unmarshal(raw, &b) + err = json.Unmarshal(raw, b) if err == nil && (b.name != "" || b.login != "") { - return &b, nil + return b, nil } // abort if we have an error other than the wrong type diff --git a/identity/identity.go b/identity/identity.go index f952bbf9..765b77cd 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -10,6 +10,7 @@ import ( "github.com/pkg/errors" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/git" "github.com/MichaelMure/git-bug/util/lamport" @@ -21,18 +22,16 @@ const identityRemoteRefPattern = "refs/remotes/%s/identities/" const versionEntryName = "version" const identityConfigKey = "git-bug.identity" -const idLength = 40 -const humanIdLength = 7 - var ErrNonFastForwardMerge = errors.New("non fast-forward identity merge") var ErrNoIdentitySet = errors.New("to interact with bugs, an identity first needs to be created using \"git bug user create\" or \"git bug user adopt\"") var ErrMultipleIdentitiesSet = errors.New("multiple user identities set") var _ Interface = &Identity{} +var _ entity.Interface = &Identity{} type Identity struct { // Id used as unique identifier - id string + id entity.Id // all the successive version of the identity versions []*Version @@ -43,6 +42,7 @@ type Identity struct { func NewIdentity(name string, email string) *Identity { return &Identity{ + id: entity.UnsetId, versions: []*Version{ { name: name, @@ -55,6 +55,7 @@ func NewIdentity(name string, email string) *Identity { func NewIdentityFull(name string, email string, login string, avatarUrl string) *Identity { return &Identity{ + id: entity.UnsetId, versions: []*Version{ { name: name, @@ -70,7 +71,7 @@ func NewIdentityFull(name string, email string, login string, avatarUrl string) // MarshalJSON will only serialize the id func (i *Identity) MarshalJSON() ([]byte, error) { return json.Marshal(&IdentityStub{ - id: i.Id(), + id: i.id, }) } @@ -82,7 +83,7 @@ func (i *Identity) UnmarshalJSON(data []byte) error { } // ReadLocal load a local Identity from the identities data available in git -func ReadLocal(repo repository.Repo, id string) (*Identity, error) { +func ReadLocal(repo repository.Repo, id entity.Id) (*Identity, error) { ref := fmt.Sprintf("%s%s", identityRefPattern, id) return read(repo, ref) } @@ -96,10 +97,10 @@ func ReadRemote(repo repository.Repo, remote string, id string) (*Identity, erro // read will load and parse an identity from git func read(repo repository.Repo, ref string) (*Identity, error) { refSplit := strings.Split(ref, "/") - id := refSplit[len(refSplit)-1] + id := entity.Id(refSplit[len(refSplit)-1]) - if len(id) != idLength { - return nil, fmt.Errorf("invalid ref length") + if err := id.Validate(); err != nil { + return nil, errors.Wrap(err, "invalid ref") } hashes, err := repo.ListCommits(ref) @@ -233,7 +234,7 @@ func IsUserIdentitySet(repo repository.RepoCommon) (bool, error) { // SetUserIdentity store the user identity's id in the git config func SetUserIdentity(repo repository.RepoCommon, identity *Identity) error { - return repo.StoreConfig(identityConfigKey, identity.Id()) + return repo.StoreConfig(identityConfigKey, identity.Id().String()) } // GetUserIdentity read the current user identity, set with a git config entry @@ -251,9 +252,13 @@ func GetUserIdentity(repo repository.Repo) (*Identity, error) { return nil, ErrMultipleIdentitiesSet } - var id string + var id entity.Id for _, val := range configs { - id = val + id = entity.Id(val) + } + + if err := id.Validate(); err != nil { + return nil, err } i, err := ReadLocal(repo, id) @@ -326,8 +331,8 @@ func (i *Identity) Commit(repo repository.ClockedRepo) error { v.commitHash = commitHash // if it was the first commit, use the commit hash as the Identity id - if i.id == "" { - i.id = string(commitHash) + if i.id == "" || i.id == entity.UnsetId { + i.id = entity.Id(commitHash) } } @@ -410,7 +415,7 @@ func (i *Identity) Merge(repo repository.Repo, other *Identity) (bool, error) { } if modified { - err := repo.UpdateRef(identityRefPattern+i.id, i.lastCommit) + err := repo.UpdateRef(identityRefPattern+i.id.String(), i.lastCommit) if err != nil { return false, err } @@ -439,8 +444,8 @@ func (i *Identity) Validate() error { lastTime = v.time } - // The identity ID should be the hash of the first commit - if i.versions[0].commitHash != "" && string(i.versions[0].commitHash) != i.id { + // The identity Id should be the hash of the first commit + if i.versions[0].commitHash != "" && string(i.versions[0].commitHash) != i.id.String() { return fmt.Errorf("identity id should be the first commit hash") } @@ -456,7 +461,7 @@ func (i *Identity) lastVersion() *Version { } // Id return the Identity identifier -func (i *Identity) Id() string { +func (i *Identity) Id() entity.Id { if i.id == "" { // simply panic as it would be a coding error // (using an id of an identity not stored yet) @@ -465,16 +470,6 @@ func (i *Identity) Id() string { return i.id } -// HumanId return the Identity identifier truncated for human consumption -func (i *Identity) HumanId() string { - return FormatHumanID(i.Id()) -} - -func FormatHumanID(id string) string { - format := fmt.Sprintf("%%.%ds", humanIdLength) - return fmt.Sprintf(format, id) -} - // Name return the last version of the name func (i *Identity) Name() string { return i.lastVersion().name diff --git a/identity/identity_actions.go b/identity/identity_actions.go index 38d83b19..40aab8e2 100644 --- a/identity/identity_actions.go +++ b/identity/identity_actions.go @@ -75,7 +75,7 @@ func MergeAll(repo repository.ClockedRepo, remote string) <-chan entity.MergeRes continue } - localRef := identityRefPattern + remoteIdentity.Id() + localRef := identityRefPattern + remoteIdentity.Id().String() localExist, err := repo.RefExist(localRef) if err != nil { diff --git a/identity/identity_stub.go b/identity/identity_stub.go index a2cee0ad..be52ffc0 100644 --- a/identity/identity_stub.go +++ b/identity/identity_stub.go @@ -3,6 +3,7 @@ package identity import ( "encoding/json" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/lamport" "github.com/MichaelMure/git-bug/util/timestamp" @@ -16,13 +17,13 @@ var _ Interface = &IdentityStub{} // When this JSON is deserialized, an IdentityStub is returned instead, to be replaced // later by the proper Identity, loaded from the Repo. type IdentityStub struct { - id string + id entity.Id } func (i *IdentityStub) MarshalJSON() ([]byte, error) { // TODO: add a type marker return json.Marshal(struct { - Id string `json:"id"` + Id entity.Id `json:"id"` }{ Id: i.id, }) @@ -30,7 +31,7 @@ func (i *IdentityStub) MarshalJSON() ([]byte, error) { func (i *IdentityStub) UnmarshalJSON(data []byte) error { aux := struct { - Id string `json:"id"` + Id entity.Id `json:"id"` }{} if err := json.Unmarshal(data, &aux); err != nil { @@ -43,15 +44,10 @@ func (i *IdentityStub) UnmarshalJSON(data []byte) error { } // Id return the Identity identifier -func (i *IdentityStub) Id() string { +func (i *IdentityStub) Id() entity.Id { return i.id } -// HumanId return the Identity identifier truncated for human consumption -func (i *IdentityStub) HumanId() string { - return FormatHumanID(i.Id()) -} - func (IdentityStub) Name() string { panic("identities needs to be properly loaded with identity.ReadLocal()") } diff --git a/identity/identity_stub_test.go b/identity/identity_stub_test.go index 3d94cd3e..b01a718c 100644 --- a/identity/identity_stub_test.go +++ b/identity/identity_stub_test.go @@ -19,5 +19,8 @@ func TestIdentityStubSerialize(t *testing.T) { err = json.Unmarshal(data, &after) assert.NoError(t, err) + // enforce creating the Id + before.Id() + assert.Equal(t, before, &after) } diff --git a/identity/identity_test.go b/identity/identity_test.go index 2ddb64ea..f91c548f 100644 --- a/identity/identity_test.go +++ b/identity/identity_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "testing" + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/repository" "github.com/stretchr/testify/assert" ) @@ -15,6 +16,7 @@ func TestIdentityCommitLoad(t *testing.T) { // single version identity := &Identity{ + id: entity.UnsetId, versions: []*Version{ { name: "René Descartes", @@ -36,6 +38,7 @@ func TestIdentityCommitLoad(t *testing.T) { // multiple version identity = &Identity{ + id: entity.UnsetId, versions: []*Version{ { time: 100, @@ -114,6 +117,7 @@ func commitsAreSet(t *testing.T, identity *Identity) { // Test that the correct crypto keys are returned for a given lamport time func TestIdentity_ValidKeysAtTime(t *testing.T) { identity := Identity{ + id: entity.UnsetId, versions: []*Version{ { time: 100, @@ -215,6 +219,7 @@ func TestJSON(t *testing.T) { mockRepo := repository.NewMockRepoForTest() identity := &Identity{ + id: entity.UnsetId, versions: []*Version{ { name: "René Descartes", @@ -223,7 +228,7 @@ func TestJSON(t *testing.T) { }, } - // commit to make sure we have an ID + // commit to make sure we have an Id err := identity.Commit(mockRepo) assert.Nil(t, err) assert.NotEmpty(t, identity.id) diff --git a/identity/interface.go b/identity/interface.go index 88f1d9a7..54a9da78 100644 --- a/identity/interface.go +++ b/identity/interface.go @@ -1,6 +1,7 @@ package identity import ( + "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/repository" "github.com/MichaelMure/git-bug/util/lamport" "github.com/MichaelMure/git-bug/util/timestamp" @@ -8,10 +9,7 @@ import ( type Interface interface { // Id return the Identity identifier - Id() string - - // HumanId return the Identity identifier truncated for human consumption - HumanId() string + Id() entity.Id // Name return the last version of the name Name() string diff --git a/identity/resolver.go b/identity/resolver.go index 7facfc0c..70fac74c 100644 --- a/identity/resolver.go +++ b/identity/resolver.go @@ -1,11 +1,14 @@ package identity -import "github.com/MichaelMure/git-bug/repository" +import ( + "github.com/MichaelMure/git-bug/entity" + "github.com/MichaelMure/git-bug/repository" +) // Resolver define the interface of an Identity resolver, able to load // an identity from, for example, a repo or a cache. type Resolver interface { - ResolveIdentity(id string) (Interface, error) + ResolveIdentity(id entity.Id) (Interface, error) } // DefaultResolver is a Resolver loading Identities directly from a Repo @@ -17,6 +20,6 @@ func NewSimpleResolver(repo repository.Repo) *SimpleResolver { return &SimpleResolver{repo: repo} } -func (r *SimpleResolver) ResolveIdentity(id string) (Interface, error) { +func (r *SimpleResolver) ResolveIdentity(id entity.Id) (Interface, error) { return ReadLocal(r.repo, id) } diff --git a/util/git/hash.go b/util/git/hash.go index d9160d75..d7254178 100644 --- a/util/git/hash.go +++ b/util/git/hash.go @@ -5,6 +5,9 @@ import ( "io" ) +const idLengthSHA1 = 40 +const idLengthSHA256 = 64 + // Hash is a git hash type Hash string @@ -16,7 +19,7 @@ func (h Hash) String() string { func (h *Hash) UnmarshalGQL(v interface{}) error { _, ok := v.(string) if !ok { - return fmt.Errorf("labels must be strings") + return fmt.Errorf("hashes must be strings") } *h = v.(Hash) @@ -35,7 +38,8 @@ func (h Hash) MarshalGQL(w io.Writer) { // IsValid tell if the hash is valid func (h *Hash) IsValid() bool { - if len(*h) != 40 && len(*h) != 64 { + // Support for both sha1 and sha256 git hashes + if len(*h) != idLengthSHA1 && len(*h) != idLengthSHA256 { return false } for _, r := range *h { |