Merge pull request #193 from MichaelMure/immutableID

Future proof the operation's ID

5 files changed, 63 insertions, 91 deletions

diff --git a/cache/bug_cache.go b/cache/bug_cache.go
index 758fb0b7..6a220f49 100644
--- a/cache/bug_cache.go
+++ b/cache/bug_cache.go
@@ -2,13 +2,15 @@ package cache
 
 import (
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/MichaelMure/git-bug/bug"
+	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/util/git"
 )
 
+var ErrNoMatchingOp = fmt.Errorf("no matching operation found")
+
 // BugCache is a wrapper around a Bug. It provide multiple functions:
 //
 // 1. Provide a higher level API to use than the raw API from Bug.
@@ -29,49 +31,25 @@ func (c *BugCache) Snapshot() *bug.Snapshot {
 	return c.bug.Snapshot()
 }
 
-func (c *BugCache) Id() string {
+func (c *BugCache) Id() entity.Id {
 	return c.bug.Id()
 }
 
-func (c *BugCache) HumanId() string {
-	return c.bug.HumanId()
-}
-
 func (c *BugCache) notifyUpdated() error {
 	return c.repoCache.bugUpdated(c.bug.Id())
 }
 
-var ErrNoMatchingOp = fmt.Errorf("no matching operation found")
-
-type ErrMultipleMatchOp struct {
-	Matching []git.Hash
-}
-
-func (e ErrMultipleMatchOp) Error() string {
-	casted := make([]string, len(e.Matching))
-
-	for i := range e.Matching {
-		casted[i] = string(e.Matching[i])
-	}
-
-	return fmt.Sprintf("Multiple matching operation found:\n%s", strings.Join(casted, "\n"))
-}
-
 // ResolveOperationWithMetadata will find an operation that has the matching metadata
-func (c *BugCache) ResolveOperationWithMetadata(key string, value string) (git.Hash, error) {
+func (c *BugCache) ResolveOperationWithMetadata(key string, value string) (entity.Id, error) {
 	// preallocate but empty
-	matching := make([]git.Hash, 0, 5)
+	matching := make([]entity.Id, 0, 5)
 
 	it := bug.NewOperationIterator(c.bug)
 	for it.Next() {
 		op := it.Value()
 		opValue, ok := op.GetMetadata(key)
 		if ok && value == opValue {
-			h, err := op.Hash()
-			if err != nil {
-				return "", err
-			}
-			matching = append(matching, h)
+			matching = append(matching, op.Id())
 		}
 	}
 
@@ -80,7 +58,7 @@ func (c *BugCache) ResolveOperationWithMetadata(key string, value string) (git.H
 	}
 
 	if len(matching) > 1 {
-		return "", ErrMultipleMatchOp{Matching: matching}
+		return "", bug.NewErrMultipleMatchOp(matching)
 	}
 
 	return matching[0], nil
@@ -232,7 +210,7 @@ func (c *BugCache) SetTitleRaw(author *IdentityCache, unixTime int64, title stri
 	return op, c.notifyUpdated()
 }
 
-func (c *BugCache) EditComment(target git.Hash, message string) (*bug.EditCommentOperation, error) {
+func (c *BugCache) EditComment(target entity.Id, message string) (*bug.EditCommentOperation, error) {
 	author, err := c.repoCache.GetUserIdentity()
 	if err != nil {
 		return nil, err
@@ -241,7 +219,7 @@ func (c *BugCache) EditComment(target git.Hash, message string) (*bug.EditCommen
 	return c.EditCommentRaw(author, time.Now().Unix(), target, message, nil)
 }
 
-func (c *BugCache) EditCommentRaw(author *IdentityCache, unixTime int64, target git.Hash, message string, metadata map[string]string) (*bug.EditCommentOperation, error) {
+func (c *BugCache) EditCommentRaw(author *IdentityCache, unixTime int64, target entity.Id, message string, metadata map[string]string) (*bug.EditCommentOperation, error) {
 	op, err := bug.EditComment(c.bug, author.Identity, unixTime, target, message)
 	if err != nil {
 		return nil, err
@@ -254,7 +232,7 @@ func (c *BugCache) EditCommentRaw(author *IdentityCache, unixTime int64, target
 	return op, c.notifyUpdated()
 }
 
-func (c *BugCache) SetMetadata(target git.Hash, newMetadata map[string]string) (*bug.SetMetadataOperation, error) {
+func (c *BugCache) SetMetadata(target entity.Id, newMetadata map[string]string) (*bug.SetMetadataOperation, error) {
 	author, err := c.repoCache.GetUserIdentity()
 	if err != nil {
 		return nil, err
@@ -263,7 +241,7 @@ func (c *BugCache) SetMetadata(target git.Hash, newMetadata map[string]string) (
 	return c.SetMetadataRaw(author, time.Now().Unix(), target, newMetadata)
 }
 
-func (c *BugCache) SetMetadataRaw(author *IdentityCache, unixTime int64, target git.Hash, newMetadata map[string]string) (*bug.SetMetadataOperation, error) {
+func (c *BugCache) SetMetadataRaw(author *IdentityCache, unixTime int64, target entity.Id, newMetadata map[string]string) (*bug.SetMetadataOperation, error) {
 	op, err := bug.SetMetadata(c.bug, author.Identity, unixTime, target, newMetadata)
 	if err != nil {
 		return nil, err
diff --git a/cache/bug_excerpt.go b/cache/bug_excerpt.go
index 8e9e5e37..e053f9e4 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,14 +29,14 @@ 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
 	// in a IdentityExcerpt
 	LegacyAuthor LegacyAuthorExcerpt
-	AuthorId     string
+	AuthorId     entity.Id
 
 	CreateMetadata map[string]string
 }
@@ -60,12 +61,12 @@ func (l LegacyAuthorExcerpt) DisplayName() string {
 }
 
 func NewBugExcerpt(b bug.Interface, snap *bug.Snapshot) *BugExcerpt {
-	participantsIds := make([]string, len(snap.Participants))
+	participantsIds := make([]entity.Id, len(snap.Participants))
 	for i, participant := range snap.Participants {
 		participantsIds[i] = participant.Id()
 	}
 
-	actorsIds := make([]string, len(snap.Actors))
+	actorsIds := make([]entity.Id, len(snap.Actors))
 	for i, actor := range snap.Actors {
 		actorsIds[i] = actor.Id()
 	}
@@ -100,10 +101,6 @@ func NewBugExcerpt(b bug.Interface, snap *bug.Snapshot) *BugExcerpt {
 	return e
 }
 
-func (b *BugExcerpt) HumanId() string {
-	return bug.FormatHumanID(b.Id)
-}
-
 /*
  * Sorting
  */
diff --git a/cache/identity_excerpt.go b/cache/identity_excerpt.go
index 3ac13903..18514e9a 100644
--- a/cache/identity_excerpt.go
+++ b/cache/identity_excerpt.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/identity"
 )
 
@@ -17,7 +18,7 @@ func init() {
 // filter identities efficiently without having to read and compile each raw
 // identity.
 type IdentityExcerpt struct {
-	Id string
+	Id entity.Id
 
 	Name              string
 	Login             string
@@ -33,10 +34,6 @@ func NewIdentityExcerpt(i *identity.Identity) *IdentityExcerpt {
 	}
 }
 
-func (i *IdentityExcerpt) HumanId() string {
-	return identity.FormatHumanID(i.Id)
-}
-
 // DisplayName return a non-empty string to display, representing the
 // identity, based on the non-empty values.
 func (i *IdentityExcerpt) DisplayName() string {
@@ -54,7 +51,7 @@ func (i *IdentityExcerpt) DisplayName() string {
 
 // Match matches a query with the identity name, login and ID prefixes
 func (i *IdentityExcerpt) Match(query string) bool {
-	return strings.HasPrefix(i.Id, query) ||
+	return i.Id.HasPrefix(query) ||
 		strings.Contains(strings.ToLower(i.Name), query) ||
 		strings.Contains(strings.ToLower(i.Login), query)
 }
diff --git a/cache/repo_cache.go b/cache/repo_cache.go
index a80bc7c9..d6e8857d 100644
--- a/cache/repo_cache.go
+++ b/cache/repo_cache.go
@@ -10,16 +10,16 @@ import (
 	"path"
 	"sort"
 	"strconv"
-	"strings"
 	"time"
 
+	"github.com/pkg/errors"
+
 	"github.com/MichaelMure/git-bug/bug"
 	"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"
 	"github.com/MichaelMure/git-bug/util/process"
-	"github.com/pkg/errors"
 )
 
 const bugCacheFile = "bug-cache"
@@ -58,24 +58,24 @@ type RepoCache struct {
 	repo repository.ClockedRepo
 
 	// excerpt of bugs data for all bugs
-	bugExcerpts map[string]*BugExcerpt
+	bugExcerpts map[entity.Id]*BugExcerpt
 	// bug loaded in memory
-	bugs map[string]*BugCache
+	bugs map[entity.Id]*BugCache
 
 	// excerpt of identities data for all identities
-	identitiesExcerpts map[string]*IdentityExcerpt
+	identitiesExcerpts map[entity.Id]*IdentityExcerpt
 	// identities loaded in memory
-	identities map[string]*IdentityCache
+	identities map[entity.Id]*IdentityCache
 
 	// the user identity's id, if known
-	userIdentityId string
+	userIdentityId entity.Id
 }
 
 func NewRepoCache(r repository.ClockedRepo) (*RepoCache, error) {
 	c := &RepoCache{
 		repo:       r,
-		bugs:       make(map[string]*BugCache),
-		identities: make(map[string]*IdentityCache),
+		bugs:       make(map[entity.Id]*BugCache),
+		identities: make(map[entity.Id]*IdentityCache),
 	}
 
 	err := c.lock()
@@ -191,7 +191,7 @@ func (c *RepoCache) Close() error {
 
 // bugUpdated is a callback to trigger when the excerpt of a bug changed,
 // that is each time a bug is updated
-func (c *RepoCache) bugUpdated(id string) error {
+func (c *RepoCache) bugUpdated(id entity.Id) error {
 	b, ok := c.bugs[id]
 	if !ok {
 		panic("missing bug in the cache")
@@ -205,7 +205,7 @@ func (c *RepoCache) bugUpdated(id string) error {
 
 // identityUpdated is a callback to trigger when the excerpt of an identity
 // changed, that is each time an identity is updated
-func (c *RepoCache) identityUpdated(id string) error {
+func (c *RepoCache) identityUpdated(id entity.Id) error {
 	i, ok := c.identities[id]
 	if !ok {
 		panic("missing identity in the cache")
@@ -237,7 +237,7 @@ func (c *RepoCache) loadBugCache() error {
 
 	aux := struct {
 		Version  uint
-		Excerpts map[string]*BugExcerpt
+		Excerpts map[entity.Id]*BugExcerpt
 	}{}
 
 	err = decoder.Decode(&aux)
@@ -266,7 +266,7 @@ func (c *RepoCache) loadIdentityCache() error {
 
 	aux := struct {
 		Version  uint
-		Excerpts map[string]*IdentityExcerpt
+		Excerpts map[entity.Id]*IdentityExcerpt
 	}{}
 
 	err = decoder.Decode(&aux)
@@ -299,7 +299,7 @@ func (c *RepoCache) writeBugCache() error {
 
 	aux := struct {
 		Version  uint
-		Excerpts map[string]*BugExcerpt
+		Excerpts map[entity.Id]*BugExcerpt
 	}{
 		Version:  formatVersion,
 		Excerpts: c.bugExcerpts,
@@ -331,7 +331,7 @@ func (c *RepoCache) writeIdentityCache() error {
 
 	aux := struct {
 		Version  uint
-		Excerpts map[string]*IdentityExcerpt
+		Excerpts map[entity.Id]*IdentityExcerpt
 	}{
 		Version:  formatVersion,
 		Excerpts: c.identitiesExcerpts,
@@ -368,7 +368,7 @@ func identityCacheFilePath(repo repository.Repo) string {
 func (c *RepoCache) buildCache() error {
 	_, _ = fmt.Fprintf(os.Stderr, "Building identity cache... ")
 
-	c.identitiesExcerpts = make(map[string]*IdentityExcerpt)
+	c.identitiesExcerpts = make(map[entity.Id]*IdentityExcerpt)
 
 	allIdentities := identity.ReadAllLocalIdentities(c.repo)
 
@@ -384,7 +384,7 @@ func (c *RepoCache) buildCache() error {
 
 	_, _ = fmt.Fprintf(os.Stderr, "Building bug cache... ")
 
-	c.bugExcerpts = make(map[string]*BugExcerpt)
+	c.bugExcerpts = make(map[entity.Id]*BugExcerpt)
 
 	allBugs := bug.ReadAllLocalBugs(c.repo)
 
@@ -402,7 +402,7 @@ func (c *RepoCache) buildCache() error {
 }
 
 // ResolveBug retrieve a bug matching the exact given id
-func (c *RepoCache) ResolveBug(id string) (*BugCache, error) {
+func (c *RepoCache) ResolveBug(id entity.Id) (*BugCache, error) {
 	cached, ok := c.bugs[id]
 	if ok {
 		return cached, nil
@@ -420,7 +420,7 @@ func (c *RepoCache) ResolveBug(id string) (*BugCache, error) {
 }
 
 // ResolveBugExcerpt retrieve a BugExcerpt matching the exact given id
-func (c *RepoCache) ResolveBugExcerpt(id string) (*BugExcerpt, error) {
+func (c *RepoCache) ResolveBugExcerpt(id entity.Id) (*BugExcerpt, error) {
 	e, ok := c.bugExcerpts[id]
 	if !ok {
 		return nil, bug.ErrBugNotExist
@@ -433,16 +433,16 @@ func (c *RepoCache) ResolveBugExcerpt(id string) (*BugExcerpt, error) {
 // bugs match.
 func (c *RepoCache) ResolveBugPrefix(prefix string) (*BugCache, error) {
 	// preallocate but empty
-	matching := make([]string, 0, 5)
+	matching := make([]entity.Id, 0, 5)
 
 	for id := range c.bugExcerpts {
-		if strings.HasPrefix(id, prefix) {
+		if id.HasPrefix(prefix) {
 			matching = append(matching, id)
 		}
 	}
 
 	if len(matching) > 1 {
-		return nil, bug.ErrMultipleMatch{Matching: matching}
+		return nil, bug.NewErrMultipleMatchBug(matching)
 	}
 
 	if len(matching) == 0 {
@@ -457,7 +457,7 @@ func (c *RepoCache) ResolveBugPrefix(prefix string) (*BugCache, error) {
 // match.
 func (c *RepoCache) ResolveBugCreateMetadata(key string, value string) (*BugCache, error) {
 	// preallocate but empty
-	matching := make([]string, 0, 5)
+	matching := make([]entity.Id, 0, 5)
 
 	for id, excerpt := range c.bugExcerpts {
 		if excerpt.CreateMetadata[key] == value {
@@ -466,7 +466,7 @@ func (c *RepoCache) ResolveBugCreateMetadata(key string, value string) (*BugCach
 	}
 
 	if len(matching) > 1 {
-		return nil, bug.ErrMultipleMatch{Matching: matching}
+		return nil, bug.NewErrMultipleMatchBug(matching)
 	}
 
 	if len(matching) == 0 {
@@ -477,7 +477,7 @@ func (c *RepoCache) ResolveBugCreateMetadata(key string, value string) (*BugCach
 }
 
 // QueryBugs return the id of all Bug matching the given Query
-func (c *RepoCache) QueryBugs(query *Query) []string {
+func (c *RepoCache) QueryBugs(query *Query) []entity.Id {
 	if query == nil {
 		return c.AllBugsIds()
 	}
@@ -509,7 +509,7 @@ func (c *RepoCache) QueryBugs(query *Query) []string {
 
 	sort.Sort(sorter)
 
-	result := make([]string, len(filtered))
+	result := make([]entity.Id, len(filtered))
 
 	for i, val := range filtered {
 		result[i] = val.Id
@@ -519,8 +519,8 @@ func (c *RepoCache) QueryBugs(query *Query) []string {
 }
 
 // AllBugsIds return all known bug ids
-func (c *RepoCache) AllBugsIds() []string {
-	result := make([]string, len(c.bugExcerpts))
+func (c *RepoCache) AllBugsIds() []entity.Id {
+	result := make([]entity.Id, len(c.bugExcerpts))
 
 	i := 0
 	for _, excerpt := range c.bugExcerpts {
@@ -778,7 +778,7 @@ func repoIsAvailable(repo repository.Repo) error {
 }
 
 // ResolveIdentity retrieve an identity matching the exact given id
-func (c *RepoCache) ResolveIdentity(id string) (*IdentityCache, error) {
+func (c *RepoCache) ResolveIdentity(id entity.Id) (*IdentityCache, error) {
 	cached, ok := c.identities[id]
 	if ok {
 		return cached, nil
@@ -796,7 +796,7 @@ func (c *RepoCache) ResolveIdentity(id string) (*IdentityCache, error) {
 }
 
 // ResolveIdentityExcerpt retrieve a IdentityExcerpt matching the exact given id
-func (c *RepoCache) ResolveIdentityExcerpt(id string) (*IdentityExcerpt, error) {
+func (c *RepoCache) ResolveIdentityExcerpt(id entity.Id) (*IdentityExcerpt, error) {
 	e, ok := c.identitiesExcerpts[id]
 	if !ok {
 		return nil, identity.ErrIdentityNotExist
@@ -809,16 +809,16 @@ func (c *RepoCache) ResolveIdentityExcerpt(id string) (*IdentityExcerpt, error)
 // It fails if multiple identities match.
 func (c *RepoCache) ResolveIdentityPrefix(prefix string) (*IdentityCache, error) {
 	// preallocate but empty
-	matching := make([]string, 0, 5)
+	matching := make([]entity.Id, 0, 5)
 
 	for id := range c.identitiesExcerpts {
-		if strings.HasPrefix(id, prefix) {
+		if id.HasPrefix(prefix) {
 			matching = append(matching, id)
 		}
 	}
 
 	if len(matching) > 1 {
-		return nil, identity.ErrMultipleMatch{Matching: matching}
+		return nil, identity.NewErrMultipleMatch(matching)
 	}
 
 	if len(matching) == 0 {
@@ -832,7 +832,7 @@ func (c *RepoCache) ResolveIdentityPrefix(prefix string) (*IdentityCache, error)
 // one of it's version. If multiple version have the same key, the first defined take precedence.
 func (c *RepoCache) ResolveIdentityImmutableMetadata(key string, value string) (*IdentityCache, error) {
 	// preallocate but empty
-	matching := make([]string, 0, 5)
+	matching := make([]entity.Id, 0, 5)
 
 	for id, i := range c.identitiesExcerpts {
 		if i.ImmutableMetadata[key] == value {
@@ -841,7 +841,7 @@ func (c *RepoCache) ResolveIdentityImmutableMetadata(key string, value string) (
 	}
 
 	if len(matching) > 1 {
-		return nil, identity.ErrMultipleMatch{Matching: matching}
+		return nil, identity.NewErrMultipleMatch(matching)
 	}
 
 	if len(matching) == 0 {
@@ -852,8 +852,8 @@ func (c *RepoCache) ResolveIdentityImmutableMetadata(key string, value string) (
 }
 
 // AllIdentityIds return all known identity ids
-func (c *RepoCache) AllIdentityIds() []string {
-	result := make([]string, len(c.identitiesExcerpts))
+func (c *RepoCache) AllIdentityIds() []entity.Id {
+	result := make([]entity.Id, len(c.identitiesExcerpts))
 
 	i := 0
 	for _, excerpt := range c.identitiesExcerpts {
diff --git a/cache/repo_cache_test.go b/cache/repo_cache_test.go
index 3e81674a..c3bd3cc4 100644
--- a/cache/repo_cache_test.go
+++ b/cache/repo_cache_test.go
@@ -57,14 +57,14 @@ func TestCache(t *testing.T) {
 	require.NoError(t, err)
 	_, err = cache.ResolveIdentityExcerpt(iden1.Id())
 	require.NoError(t, err)
-	_, err = cache.ResolveIdentityPrefix(iden1.Id()[:10])
+	_, err = cache.ResolveIdentityPrefix(iden1.Id().String()[:10])
 	require.NoError(t, err)
 
 	_, err = cache.ResolveBug(bug1.Id())
 	require.NoError(t, err)
 	_, err = cache.ResolveBugExcerpt(bug1.Id())
 	require.NoError(t, err)
-	_, err = cache.ResolveBugPrefix(bug1.Id()[:10])
+	_, err = cache.ResolveBugPrefix(bug1.Id().String()[:10])
 	require.NoError(t, err)
 
 	// Querying
@@ -91,14 +91,14 @@ func TestCache(t *testing.T) {
 	require.NoError(t, err)
 	_, err = cache.ResolveIdentityExcerpt(iden1.Id())
 	require.NoError(t, err)
-	_, err = cache.ResolveIdentityPrefix(iden1.Id()[:10])
+	_, err = cache.ResolveIdentityPrefix(iden1.Id().String()[:10])
 	require.NoError(t, err)
 
 	_, err = cache.ResolveBug(bug1.Id())
 	require.NoError(t, err)
 	_, err = cache.ResolveBugExcerpt(bug1.Id())
 	require.NoError(t, err)
-	_, err = cache.ResolveBugPrefix(bug1.Id()[:10])
+	_, err = cache.ResolveBugPrefix(bug1.Id().String()[:10])
 	require.NoError(t, err)
 }