bug: compute op's ID based on the serialized data on disk

Michael Muré created

Change summary

bug/bug_test.go             |  2 
bug/op_add_comment.go       | 15 ++-------
bug/op_add_comment_test.go  |  3 +
bug/op_create.go            | 15 ++-------
bug/op_create_test.go       | 11 ++++--
bug/op_edit_comment.go      | 24 +++++++--------
bug/op_edit_comment_test.go | 15 +++++---
bug/op_label_change.go      | 24 +++++----------
bug/op_label_change_test.go |  3 +
bug/op_noop.go              |  5 +-
bug/op_noop_test.go         |  3 +
bug/op_set_metadata.go      | 24 +++++----------
bug/op_set_metadata_test.go | 11 ++++--
bug/op_set_status.go        | 23 +++++---------
bug/op_set_status_test.go   |  3 +
bug/op_set_title.go         | 20 +++--------
bug/op_set_title_test.go    |  3 +
bug/operation.go            | 61 ++++++++++++++++++++++++++------------
bug/operation_pack.go       |  3 -
bug/operation_pack_test.go  |  8 ++--
bug/operation_test.go       | 18 +++++-----
bug/snapshot.go             |  4 +-
bug/timeline.go             | 14 ++++----
commands/select/select.go   | 10 +++---
24 files changed, 158 insertions(+), 164 deletions(-)

Detailed changes

bug/bug_test.go 🔗

@@ -103,7 +103,7 @@ func equivalentBug(t *testing.T, expected, actual *Bug) {
 
 	for i := range expected.packs {
 		for j := range expected.packs[i].Operations {
-			actual.packs[i].Operations[j].base().hash = expected.packs[i].Operations[j].base().hash
+			actual.packs[i].Operations[j].base().id = expected.packs[i].Operations[j].base().id
 		}
 	}
 

bug/op_add_comment.go 🔗

@@ -24,23 +24,16 @@ func (op *AddCommentOperation) base() *OpBase {
 	return &op.OpBase
 }
 
-func (op *AddCommentOperation) Hash() (git.Hash, error) {
-	return hashOperation(op)
+func (op *AddCommentOperation) ID() string {
+	return idOperation(op)
 }
 
 func (op *AddCommentOperation) Apply(snapshot *Snapshot) {
 	snapshot.addActor(op.Author)
 	snapshot.addParticipant(op.Author)
 
-	hash, err := op.Hash()
-	if err != nil {
-		// Should never error unless a programming error happened
-		// (covered in OpBase.Validate())
-		panic(err)
-	}
-
 	comment := Comment{
-		id:       string(hash),
+		id:       op.ID(),
 		Message:  op.Message,
 		Author:   op.Author,
 		Files:    op.Files,
@@ -50,7 +43,7 @@ func (op *AddCommentOperation) Apply(snapshot *Snapshot) {
 	snapshot.Comments = append(snapshot.Comments, comment)
 
 	item := &AddCommentTimelineItem{
-		CommentTimelineItem: NewCommentTimelineItem(hash, comment),
+		CommentTimelineItem: NewCommentTimelineItem(op.ID(), comment),
 	}
 
 	snapshot.Timeline = append(snapshot.Timeline, item)

bug/op_add_comment_test.go 🔗

@@ -21,5 +21,8 @@ func TestAddCommentSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
+	// enforce creating the ID
+	before.ID()
+
 	assert.Equal(t, before, &after)
 }

bug/op_create.go 🔗

@@ -25,25 +25,18 @@ func (op *CreateOperation) base() *OpBase {
 	return &op.OpBase
 }
 
-func (op *CreateOperation) Hash() (git.Hash, error) {
-	return hashOperation(op)
+func (op *CreateOperation) ID() string {
+	return idOperation(op)
 }
 
 func (op *CreateOperation) Apply(snapshot *Snapshot) {
 	snapshot.addActor(op.Author)
 	snapshot.addParticipant(op.Author)
 
-	hash, err := op.Hash()
-	if err != nil {
-		// Should never error unless a programming error happened
-		// (covered in OpBase.Validate())
-		panic(err)
-	}
-
 	snapshot.Title = op.Title
 
 	comment := Comment{
-		id:       string(hash),
+		id:       op.ID(),
 		Message:  op.Message,
 		Author:   op.Author,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
@@ -55,7 +48,7 @@ func (op *CreateOperation) Apply(snapshot *Snapshot) {
 
 	snapshot.Timeline = []TimelineItem{
 		&CreateTimelineItem{
-			CommentTimelineItem: NewCommentTimelineItem(hash, comment),
+			CommentTimelineItem: NewCommentTimelineItem(op.ID(), comment),
 		},
 	}
 }

bug/op_create_test.go 🔗

@@ -20,11 +20,11 @@ func TestCreate(t *testing.T) {
 
 	create.Apply(&snapshot)
 
-	hash, err := create.Hash()
-	assert.NoError(t, err)
+	id := create.ID()
+	assert.True(t, IDIsValid(id))
 
 	comment := Comment{
-		id:       string(hash),
+		id:       string(id),
 		Author:   rene,
 		Message:  "message",
 		UnixTime: timestamp.Timestamp(create.UnixTime),
@@ -41,7 +41,7 @@ func TestCreate(t *testing.T) {
 		CreatedAt:    create.Time(),
 		Timeline: []TimelineItem{
 			&CreateTimelineItem{
-				CommentTimelineItem: NewCommentTimelineItem(hash, comment),
+				CommentTimelineItem: NewCommentTimelineItem(id, comment),
 			},
 		},
 	}
@@ -61,5 +61,8 @@ func TestCreateSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
+	// enforce creating the ID
+	before.ID()
+
 	assert.Equal(t, before, &after)
 }

bug/op_edit_comment.go 🔗

@@ -16,7 +16,7 @@ var _ Operation = &EditCommentOperation{}
 // EditCommentOperation will change a comment in the bug
 type EditCommentOperation struct {
 	OpBase
-	Target  git.Hash
+	Target  string
 	Message string
 	Files   []git.Hash
 }
@@ -25,8 +25,8 @@ func (op *EditCommentOperation) base() *OpBase {
 	return &op.OpBase
 }
 
-func (op *EditCommentOperation) Hash() (git.Hash, error) {
-	return hashOperation(op)
+func (op *EditCommentOperation) ID() string {
+	return idOperation(op)
 }
 
 func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
@@ -38,9 +38,7 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 	var target TimelineItem
 
 	for i, item := range snapshot.Timeline {
-		h := item.Hash()
-
-		if h == op.Target {
+		if item.ID() == op.Target {
 			target = snapshot.Timeline[i]
 			break
 		}
@@ -52,7 +50,7 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 	}
 
 	comment := Comment{
-		id:       string(op.Target),
+		id:       op.Target,
 		Message:  op.Message,
 		Files:    op.Files,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
@@ -71,7 +69,7 @@ func (op *EditCommentOperation) Apply(snapshot *Snapshot) {
 	// Updating the corresponding comment
 
 	for i := range snapshot.Comments {
-		if snapshot.Comments[i].Id() == string(op.Target) {
+		if snapshot.Comments[i].Id() == op.Target {
 			snapshot.Comments[i].Message = op.Message
 			snapshot.Comments[i].Files = op.Files
 			break
@@ -88,7 +86,7 @@ func (op *EditCommentOperation) Validate() error {
 		return err
 	}
 
-	if !op.Target.IsValid() {
+	if !IDIsValid(op.Target) {
 		return fmt.Errorf("target hash is invalid")
 	}
 
@@ -132,7 +130,7 @@ func (op *EditCommentOperation) UnmarshalJSON(data []byte) error {
 	}
 
 	aux := struct {
-		Target  git.Hash   `json:"target"`
+		Target  string     `json:"target"`
 		Message string     `json:"message"`
 		Files   []git.Hash `json:"files"`
 	}{}
@@ -153,7 +151,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 git.Hash, message string, files []git.Hash) *EditCommentOperation {
+func NewEditCommentOp(author identity.Interface, unixTime int64, target string, message string, files []git.Hash) *EditCommentOperation {
 	return &EditCommentOperation{
 		OpBase:  newOpBase(EditCommentOp, author, unixTime),
 		Target:  target,
@@ -163,11 +161,11 @@ func NewEditCommentOp(author identity.Interface, unixTime int64, target git.Hash
 }
 
 // Convenience function to apply the operation
-func EditComment(b Interface, author identity.Interface, unixTime int64, target git.Hash, message string) (*EditCommentOperation, error) {
+func EditComment(b Interface, author identity.Interface, unixTime int64, target string, message string) (*EditCommentOperation, error) {
 	return EditCommentWithFiles(b, author, unixTime, target, message, nil)
 }
 
-func EditCommentWithFiles(b Interface, author identity.Interface, unixTime int64, target git.Hash, message string, files []git.Hash) (*EditCommentOperation, error) {
+func EditCommentWithFiles(b Interface, author identity.Interface, unixTime int64, target string, message string, files []git.Hash) (*EditCommentOperation, error) {
 	editCommentOp := NewEditCommentOp(author, unixTime, target, message, files)
 	if err := editCommentOp.Validate(); err != nil {
 		return nil, err

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, err := create.Hash()
-	require.NoError(t, err)
+	hash1 := create.ID()
+	require.True(t, IDIsValid(hash1))
 
 	comment1 := NewAddCommentOp(rene, unix, "comment 1", nil)
 	comment1.Apply(&snapshot)
 
-	hash2, err := comment1.Hash()
-	require.NoError(t, err)
+	hash2 := comment1.ID()
+	require.True(t, IDIsValid(hash2))
 
 	// add another unrelated op in between
 	setTitle := NewSetTitleOp(rene, unix, "edited title", "title")
@@ -36,8 +36,8 @@ func TestEdit(t *testing.T) {
 	comment2 := NewAddCommentOp(rene, unix, "comment 2", nil)
 	comment2.Apply(&snapshot)
 
-	hash3, err := comment2.Hash()
-	require.NoError(t, err)
+	hash3 := comment2.ID()
+	require.True(t, IDIsValid(hash3))
 
 	edit := NewEditCommentOp(rene, unix, hash1, "create edited", nil)
 	edit.Apply(&snapshot)
@@ -85,5 +85,8 @@ func TestEditCommentSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
+	// enforce creating the ID
+	before.ID()
+
 	assert.Equal(t, before, &after)
 }

bug/op_label_change.go 🔗

@@ -5,11 +5,10 @@ import (
 	"fmt"
 	"sort"
 
+	"github.com/pkg/errors"
+
 	"github.com/MichaelMure/git-bug/identity"
 	"github.com/MichaelMure/git-bug/util/timestamp"
-
-	"github.com/MichaelMure/git-bug/util/git"
-	"github.com/pkg/errors"
 )
 
 var _ Operation = &LabelChangeOperation{}
@@ -25,8 +24,8 @@ func (op *LabelChangeOperation) base() *OpBase {
 	return &op.OpBase
 }
 
-func (op *LabelChangeOperation) Hash() (git.Hash, error) {
-	return hashOperation(op)
+func (op *LabelChangeOperation) ID() string {
+	return idOperation(op)
 }
 
 // Apply apply the operation
@@ -61,15 +60,8 @@ AddLoop:
 		return string(snapshot.Labels[i]) < string(snapshot.Labels[j])
 	})
 
-	hash, err := op.Hash()
-	if err != nil {
-		// Should never error unless a programming error happened
-		// (covered in OpBase.Validate())
-		panic(err)
-	}
-
 	item := &LabelChangeTimelineItem{
-		hash:     hash,
+		id:       op.ID(),
 		Author:   op.Author,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
 		Added:    op.Added,
@@ -163,15 +155,15 @@ func NewLabelChangeOperation(author identity.Interface, unixTime int64, added, r
 }
 
 type LabelChangeTimelineItem struct {
-	hash     git.Hash
+	id       string
 	Author   identity.Interface
 	UnixTime timestamp.Timestamp
 	Added    []Label
 	Removed  []Label
 }
 
-func (l LabelChangeTimelineItem) Hash() git.Hash {
-	return l.hash
+func (l LabelChangeTimelineItem) ID() string {
+	return l.id
 }
 
 // Sign post method for gqlgen

bug/op_label_change_test.go 🔗

@@ -21,5 +21,8 @@ func TestLabelChangeSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
+	// enforce creating the ID
+	before.ID()
+
 	assert.Equal(t, before, &after)
 }

bug/op_noop.go 🔗

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 
 	"github.com/MichaelMure/git-bug/identity"
-	"github.com/MichaelMure/git-bug/util/git"
 )
 
 var _ Operation = &NoOpOperation{}
@@ -20,8 +19,8 @@ func (op *NoOpOperation) base() *OpBase {
 	return &op.OpBase
 }
 
-func (op *NoOpOperation) Hash() (git.Hash, error) {
-	return hashOperation(op)
+func (op *NoOpOperation) ID() string {
+	return idOperation(op)
 }
 
 func (op *NoOpOperation) Apply(snapshot *Snapshot) {

bug/op_noop_test.go 🔗

@@ -21,5 +21,8 @@ func TestNoopSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
+	// enforce creating the ID
+	before.ID()
+
 	assert.Equal(t, before, &after)
 }

bug/op_set_metadata.go 🔗

@@ -5,14 +5,13 @@ import (
 	"fmt"
 
 	"github.com/MichaelMure/git-bug/identity"
-	"github.com/MichaelMure/git-bug/util/git"
 )
 
 var _ Operation = &SetMetadataOperation{}
 
 type SetMetadataOperation struct {
 	OpBase
-	Target      git.Hash
+	Target      string
 	NewMetadata map[string]string
 }
 
@@ -20,20 +19,13 @@ func (op *SetMetadataOperation) base() *OpBase {
 	return &op.OpBase
 }
 
-func (op *SetMetadataOperation) Hash() (git.Hash, error) {
-	return hashOperation(op)
+func (op *SetMetadataOperation) ID() string {
+	return idOperation(op)
 }
 
 func (op *SetMetadataOperation) Apply(snapshot *Snapshot) {
 	for _, target := range snapshot.Operations {
-		hash, err := target.Hash()
-		if err != nil {
-			// Should never error unless a programming error happened
-			// (covered in OpBase.Validate())
-			panic(err)
-		}
-
-		if hash == op.Target {
+		if target.ID() == op.Target {
 			base := target.base()
 
 			if base.extraMetadata == nil {
@@ -56,7 +48,7 @@ func (op *SetMetadataOperation) Validate() error {
 		return err
 	}
 
-	if !op.Target.IsValid() {
+	if !IDIsValid(op.Target) {
 		return fmt.Errorf("target hash is invalid")
 	}
 
@@ -95,7 +87,7 @@ func (op *SetMetadataOperation) UnmarshalJSON(data []byte) error {
 	}
 
 	aux := struct {
-		Target      git.Hash          `json:"target"`
+		Target      string            `json:"target"`
 		NewMetadata map[string]string `json:"new_metadata"`
 	}{}
 
@@ -114,7 +106,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 git.Hash, newMetadata map[string]string) *SetMetadataOperation {
+func NewSetMetadataOp(author identity.Interface, unixTime int64, target string, newMetadata map[string]string) *SetMetadataOperation {
 	return &SetMetadataOperation{
 		OpBase:      newOpBase(SetMetadataOp, author, unixTime),
 		Target:      target,
@@ -123,7 +115,7 @@ func NewSetMetadataOp(author identity.Interface, unixTime int64, target git.Hash
 }
 
 // Convenience function to apply the operation
-func SetMetadata(b Interface, author identity.Interface, unixTime int64, target git.Hash, newMetadata map[string]string) (*SetMetadataOperation, error) {
+func SetMetadata(b Interface, author identity.Interface, unixTime int64, target string, newMetadata map[string]string) (*SetMetadataOperation, error) {
 	SetMetadataOp := NewSetMetadataOp(author, unixTime, target, newMetadata)
 	if err := SetMetadataOp.Validate(); err != nil {
 		return nil, err

bug/op_set_metadata_test.go 🔗

@@ -21,16 +21,16 @@ func TestSetMetadata(t *testing.T) {
 	create.Apply(&snapshot)
 	snapshot.Operations = append(snapshot.Operations, create)
 
-	hash1, err := create.Hash()
-	require.NoError(t, err)
+	hash1 := create.ID()
+	require.True(t, IDIsValid(hash1))
 
 	comment := NewAddCommentOp(rene, unix, "comment", nil)
 	comment.SetMetadata("key2", "value2")
 	comment.Apply(&snapshot)
 	snapshot.Operations = append(snapshot.Operations, comment)
 
-	hash2, err := comment.Hash()
-	require.NoError(t, err)
+	hash2 := comment.ID()
+	require.True(t, IDIsValid(hash2))
 
 	op1 := NewSetMetadataOp(rene, unix, hash1, map[string]string{
 		"key":  "override",
@@ -107,5 +107,8 @@ func TestSetMetadataSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
+	// enforce creating the ID
+	before.ID()
+
 	assert.Equal(t, before, &after)
 }

bug/op_set_status.go 🔗

@@ -3,10 +3,10 @@ package bug
 import (
 	"encoding/json"
 
+	"github.com/pkg/errors"
+
 	"github.com/MichaelMure/git-bug/identity"
-	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/timestamp"
-	"github.com/pkg/errors"
 )
 
 var _ Operation = &SetStatusOperation{}
@@ -21,23 +21,16 @@ func (op *SetStatusOperation) base() *OpBase {
 	return &op.OpBase
 }
 
-func (op *SetStatusOperation) Hash() (git.Hash, error) {
-	return hashOperation(op)
+func (op *SetStatusOperation) ID() string {
+	return idOperation(op)
 }
 
 func (op *SetStatusOperation) Apply(snapshot *Snapshot) {
 	snapshot.Status = op.Status
 	snapshot.addActor(op.Author)
 
-	hash, err := op.Hash()
-	if err != nil {
-		// Should never error unless a programming error happened
-		// (covered in OpBase.Validate())
-		panic(err)
-	}
-
 	item := &SetStatusTimelineItem{
-		hash:     hash,
+		id:       op.ID(),
 		Author:   op.Author,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
 		Status:   op.Status,
@@ -114,14 +107,14 @@ func NewSetStatusOp(author identity.Interface, unixTime int64, status Status) *S
 }
 
 type SetStatusTimelineItem struct {
-	hash     git.Hash
+	id       string
 	Author   identity.Interface
 	UnixTime timestamp.Timestamp
 	Status   Status
 }
 
-func (s SetStatusTimelineItem) Hash() git.Hash {
-	return s.hash
+func (s SetStatusTimelineItem) ID() string {
+	return s.id
 }
 
 // Sign post method for gqlgen

bug/op_set_status_test.go 🔗

@@ -21,5 +21,8 @@ func TestSetStatusSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
+	// enforce creating the ID
+	before.ID()
+
 	assert.Equal(t, before, &after)
 }

bug/op_set_title.go 🔗

@@ -8,7 +8,6 @@ import (
 	"github.com/MichaelMure/git-bug/identity"
 	"github.com/MichaelMure/git-bug/util/timestamp"
 
-	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/text"
 )
 
@@ -25,23 +24,16 @@ func (op *SetTitleOperation) base() *OpBase {
 	return &op.OpBase
 }
 
-func (op *SetTitleOperation) Hash() (git.Hash, error) {
-	return hashOperation(op)
+func (op *SetTitleOperation) ID() string {
+	return idOperation(op)
 }
 
 func (op *SetTitleOperation) Apply(snapshot *Snapshot) {
 	snapshot.Title = op.Title
 	snapshot.addActor(op.Author)
 
-	hash, err := op.Hash()
-	if err != nil {
-		// Should never error unless a programming error happened
-		// (covered in OpBase.Validate())
-		panic(err)
-	}
-
 	item := &SetTitleTimelineItem{
-		hash:     hash,
+		id:       op.ID(),
 		Author:   op.Author,
 		UnixTime: timestamp.Timestamp(op.UnixTime),
 		Title:    op.Title,
@@ -139,15 +131,15 @@ func NewSetTitleOp(author identity.Interface, unixTime int64, title string, was
 }
 
 type SetTitleTimelineItem struct {
-	hash     git.Hash
+	id       string
 	Author   identity.Interface
 	UnixTime timestamp.Timestamp
 	Title    string
 	Was      string
 }
 
-func (s SetTitleTimelineItem) Hash() git.Hash {
-	return s.hash
+func (s SetTitleTimelineItem) ID() string {
+	return s.id
 }
 
 // Sign post method for gqlgen

bug/op_set_title_test.go 🔗

@@ -21,5 +21,8 @@ func TestSetTitleSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
+	// enforce creating the ID
+	before.ID()
+
 	assert.Equal(t, before, &after)
 }

bug/operation.go 🔗

@@ -27,12 +27,14 @@ 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
-	// Hash return the hash of the operation, to be used for back references
-	Hash() (git.Hash, error)
+	// ID return the identifier of the operation, to be used for back references
+	ID() string
 	// Time return the time when the operation was added
 	Time() time.Time
 	// GetUnixTime return the unix timestamp when the operation was added
@@ -53,32 +55,47 @@ type Operation interface {
 	GetAuthor() identity.Interface
 }
 
-func hashRaw(data []byte) git.Hash {
+func hashRaw(data []byte) string {
 	hasher := sha256.New()
 	// Write can't fail
 	_, _ = hasher.Write(data)
-	return git.Hash(fmt.Sprintf("%x", hasher.Sum(nil)))
+	return fmt.Sprintf("%x", hasher.Sum(nil))
 }
 
-// hash compute the hash of the serialized operation
-func hashOperation(op Operation) (git.Hash, error) {
-	// TODO: this might not be the best idea: if a single bit change in the output of json.Marshal, this will break.
-	// Idea: hash the segment of serialized data (= immutable) instead of the go object in memory
-
+func idOperation(op Operation) string {
 	base := op.base()
 
-	if base.hash != "" {
-		return base.hash, nil
+	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
+		// 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.
+
+		data, err := json.Marshal(op)
+		if err != nil {
+			panic(err)
+		}
 
-	data, err := json.Marshal(op)
-	if err != nil {
-		return "", err
+		base.id = hashRaw(data)
 	}
+	return base.id
+}
 
-	base.hash = hashRaw(data)
-
-	return base.hash, nil
+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
@@ -87,8 +104,8 @@ type OpBase struct {
 	Author        identity.Interface
 	UnixTime      int64
 	Metadata      map[string]string
-	// Not serialized. Store the op's hash in memory.
-	hash git.Hash
+	// Not serialized. Store the op's id in memory.
+	id string
 	// Not serialized. Store the extra metadata in memory,
 	// compiled from SetMetadataOperation.
 	extraMetadata map[string]string
@@ -100,6 +117,7 @@ func newOpBase(opType OperationType, author identity.Interface, unixTime int64)
 		OperationType: opType,
 		Author:        author,
 		UnixTime:      unixTime,
+		id:            unsetIDMarker,
 	}
 }
 
@@ -118,6 +136,9 @@ func (op OpBase) MarshalJSON() ([]byte, error) {
 }
 
 func (op *OpBase) UnmarshalJSON(data []byte) error {
+	// Compute the ID when loading the op from disk.
+	op.id = hashRaw(data)
+
 	aux := struct {
 		OperationType OperationType     `json:"type"`
 		Author        json.RawMessage   `json:"author"`
@@ -192,7 +213,7 @@ func (op *OpBase) SetMetadata(key string, value string) {
 	}
 
 	op.Metadata[key] = value
-	op.hash = ""
+	op.id = unsetIDMarker
 }
 
 // GetMetadata retrieve arbitrary metadata about the operation

bug/operation_pack.go 🔗

@@ -63,9 +63,6 @@ func (opp *OperationPack) UnmarshalJSON(data []byte) error {
 			return err
 		}
 
-		// Compute the hash of the operation
-		op.base().hash = hashRaw(raw)
-
 		opp.Operations = append(opp.Operations, op)
 	}
 

bug/operation_pack_test.go 🔗

@@ -48,14 +48,14 @@ func TestOperationPackSerialize(t *testing.T) {
 	err = json.Unmarshal(data, &opp2)
 	assert.NoError(t, err)
 
-	ensureHash(t, opp)
+	ensureID(t, opp)
 
 	assert.Equal(t, opp, opp2)
 }
 
-func ensureHash(t *testing.T, opp *OperationPack) {
+func ensureID(t *testing.T, opp *OperationPack) {
 	for _, op := range opp.Operations {
-		_, err := op.Hash()
-		require.NoError(t, err)
+		id := op.ID()
+		require.True(t, IDIsValid(id))
 	}
 }

bug/operation_test.go 🔗

@@ -79,7 +79,7 @@ func TestMetadata(t *testing.T) {
 	require.Equal(t, val, "value")
 }
 
-func TestHash(t *testing.T) {
+func TestID(t *testing.T) {
 	repo := repository.CreateTestRepo(false)
 	defer repository.CleanupTestRepos(t, repo)
 
@@ -94,27 +94,27 @@ func TestHash(t *testing.T) {
 		b, op, err := Create(rene, time.Now().Unix(), "title", "message")
 		require.Nil(t, err)
 
-		h1, err := op.Hash()
-		require.Nil(t, err)
+		id1 := op.ID()
+		require.True(t, IDIsValid(id1))
 
 		err = b.Commit(repo)
 		require.Nil(t, err)
 
 		op2 := b.FirstOp()
 
-		h2, err := op2.Hash()
-		require.Nil(t, err)
+		id2 := op2.ID()
+		require.True(t, IDIsValid(id2))
 
-		require.Equal(t, h1, h2)
+		require.Equal(t, id1, id2)
 
 		b2, err := ReadLocalBug(repo, b.id)
 		require.Nil(t, err)
 
 		op3 := b2.FirstOp()
 
-		h3, err := op3.Hash()
-		require.Nil(t, err)
+		id3 := op3.ID()
+		require.True(t, IDIsValid(id3))
 
-		require.Equal(t, h1, h3)
+		require.Equal(t, id1, id3)
 	}
 }

bug/snapshot.go 🔗

@@ -60,9 +60,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(hash git.Hash) (TimelineItem, error) {
+func (snap *Snapshot) SearchTimelineItem(ID string) (TimelineItem, error) {
 	for i := range snap.Timeline {
-		if snap.Timeline[i].Hash() == hash {
+		if snap.Timeline[i].ID() == ID {
 			return snap.Timeline[i], nil
 		}
 	}

bug/timeline.go 🔗

@@ -9,8 +9,8 @@ import (
 )
 
 type TimelineItem interface {
-	// Hash return the hash of the item
-	Hash() git.Hash
+	// ID return the identifier of the item
+	ID() string
 }
 
 // CommentHistoryStep hold one version of a message in the history
@@ -25,7 +25,7 @@ type CommentHistoryStep struct {
 
 // CommentTimelineItem is a TimelineItem that holds a Comment and its edition history
 type CommentTimelineItem struct {
-	hash      git.Hash
+	id        string
 	Author    identity.Interface
 	Message   string
 	Files     []git.Hash
@@ -34,9 +34,9 @@ type CommentTimelineItem struct {
 	History   []CommentHistoryStep
 }
 
-func NewCommentTimelineItem(hash git.Hash, comment Comment) CommentTimelineItem {
+func NewCommentTimelineItem(ID string, comment Comment) CommentTimelineItem {
 	return CommentTimelineItem{
-		hash:      hash,
+		id:        ID,
 		Author:    comment.Author,
 		Message:   comment.Message,
 		Files:     comment.Files,
@@ -51,8 +51,8 @@ func NewCommentTimelineItem(hash git.Hash, comment Comment) CommentTimelineItem
 	}
 }
 
-func (c *CommentTimelineItem) Hash() git.Hash {
-	return c.hash
+func (c *CommentTimelineItem) ID() string {
+	return c.id
 }
 
 // Append will append a new comment in the history and update the other values

commands/select/select.go 🔗

@@ -7,11 +7,11 @@ import (
 	"os"
 	"path"
 
+	"github.com/pkg/errors"
+
 	"github.com/MichaelMure/git-bug/bug"
 	"github.com/MichaelMure/git-bug/cache"
 	"github.com/MichaelMure/git-bug/repository"
-	"github.com/MichaelMure/git-bug/util/git"
-	"github.com/pkg/errors"
 )
 
 const selectFile = "select"
@@ -112,8 +112,8 @@ func selected(repo *cache.RepoCache) (*cache.BugCache, error) {
 		return nil, fmt.Errorf("the select file should be < 100 bytes")
 	}
 
-	h := git.Hash(buf)
-	if !h.IsValid() {
+	id := string(buf)
+	if !bug.IDIsValid(id) {
 		err = os.Remove(selectPath)
 		if err != nil {
 			return nil, errors.Wrap(err, "error while removing invalid select file")
@@ -122,7 +122,7 @@ func selected(repo *cache.RepoCache) (*cache.BugCache, error) {
 		return nil, fmt.Errorf("select file in invalid, removing it")
 	}
 
-	b, err := repo.ResolveBug(string(h))
+	b, err := repo.ResolveBug(id)
 	if err != nil {
 		return nil, err
 	}