bug: make sure there is no Operation's hash collision

4 files changed, 27 insertions, 18 deletions

diff --git a/bug/bug.go b/bug/bug.go
index ca817dc1..ae662ef1 100644
--- a/bug/bug.go
+++ b/bug/bug.go
@@ -330,12 +330,18 @@ func (bug *Bug) Validate() error {
 	}
 
 	// Check that there is no more CreateOp op
+	// Check that there is no colliding operation's ID
 	it := NewOperationIterator(bug)
 	createCount := 0
+	ids := make(map[entity.Id]struct{})
 	for it.Next() {
 		if it.Value().base().OperationType == CreateOp {
 			createCount++
 		}
+		if _, ok := ids[it.Value().Id()]; ok {
+			return fmt.Errorf("id collision: %s", it.Value().Id())
+		}
+		ids[it.Value().Id()] = struct{}{}
 	}
 
 	if createCount != 1 {
diff --git a/bug/bug_test.go b/bug/bug_test.go
index 35e8a395..480d312e 100644
--- a/bug/bug_test.go
+++ b/bug/bug_test.go
@@ -73,8 +73,6 @@ func TestBugCommitLoad(t *testing.T) {
 
 	bug1.Append(createOp)
 	bug1.Append(setTitleOp)
-	bug1.Append(setTitleOp)
-	bug1.Append(addCommentOp)
 
 	repo := repository.NewMockRepoForTest()
 
@@ -90,7 +88,6 @@ func TestBugCommitLoad(t *testing.T) {
 
 	// add more op
 
-	bug1.Append(setTitleOp)
 	bug1.Append(addCommentOp)
 
 	assert.True(t, bug1.NeedCommit())
diff --git a/bug/op_set_metadata.go b/bug/op_set_metadata.go
index f99f836b..67f7e009 100644
--- a/bug/op_set_metadata.go
+++ b/bug/op_set_metadata.go
@@ -34,6 +34,8 @@ func (op *SetMetadataOperation) Apply(snapshot *Snapshot) {
 				base.extraMetadata = make(map[string]string)
 			}
 
+			// Apply the metadata in an immutable way: if a metadata already
+			// exist, it's not possible to override it.
 			for key, val := range op.NewMetadata {
 				if _, exist := base.extraMetadata[key]; !exist {
 					base.extraMetadata[key] = val
diff --git a/bug/operation_iterator_test.go b/bug/operation_iterator_test.go
index b922bec1..fcc61d0f 100644
--- a/bug/operation_iterator_test.go
+++ b/bug/operation_iterator_test.go
@@ -1,12 +1,14 @@
 package bug
 
 import (
-	"github.com/MichaelMure/git-bug/identity"
-	"github.com/MichaelMure/git-bug/repository"
-	"github.com/stretchr/testify/assert"
-
+	"fmt"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/MichaelMure/git-bug/identity"
+	"github.com/MichaelMure/git-bug/repository"
 )
 
 func ExampleOperationIterator() {
@@ -29,16 +31,20 @@ func TestOpIterator(t *testing.T) {
 	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"})
 
+	var i int
+	genTitleOp := func() Operation {
+		i++
+		return NewSetTitleOp(rene, unix, fmt.Sprintf("title%d", i), "")
+	}
+
 	bug1 := NewBug()
 
 	// first pack
 	bug1.Append(createOp)
-	bug1.Append(setTitleOp)
 	bug1.Append(addCommentOp)
 	bug1.Append(setStatusOp)
 	bug1.Append(labelChangeOp)
@@ -46,16 +52,16 @@ func TestOpIterator(t *testing.T) {
 	assert.NoError(t, err)
 
 	// second pack
-	bug1.Append(setTitleOp)
-	bug1.Append(setTitleOp)
-	bug1.Append(setTitleOp)
+	bug1.Append(genTitleOp())
+	bug1.Append(genTitleOp())
+	bug1.Append(genTitleOp())
 	err = bug1.Commit(mockRepo)
 	assert.NoError(t, err)
 
 	// staging
-	bug1.Append(setTitleOp)
-	bug1.Append(setTitleOp)
-	bug1.Append(setTitleOp)
+	bug1.Append(genTitleOp())
+	bug1.Append(genTitleOp())
+	bug1.Append(genTitleOp())
 
 	it := NewOperationIterator(bug1)
 
@@ -65,7 +71,5 @@ func TestOpIterator(t *testing.T) {
 		counter++
 	}
 
-	if counter != 11 {
-		t.Fatalf("Wrong count of value iterated (%d instead of 8)", counter)
-	}
+	assert.Equal(t, 10, counter)
 }