identity: more refactoring progress

9 files changed, 231 insertions, 65 deletions

diff --git a/bug/bug.go b/bug/bug.go
index be3e2661..aec9a12e 100644
--- a/bug/bug.go
+++ b/bug/bug.go
@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"strings"
 
+	"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/lamport"
@@ -217,6 +219,13 @@ func readBug(repo repository.ClockedRepo, ref string) (*Bug, error) {
 		bug.packs = append(bug.packs, *opp)
 	}
 
+	// Make sure that the identities are properly loaded
+	resolver := identity.NewSimpleResolver(repo)
+	err = bug.EnsureIdentities(resolver)
+	if err != nil {
+		return nil, err
+	}
+
 	return &bug, nil
 }
 
diff --git a/bug/identity.go b/bug/identity.go
new file mode 100644
index 00000000..2eb2bcaf
--- /dev/null
+++ b/bug/identity.go
@@ -0,0 +1,27 @@
+package bug
+
+import (
+	"github.com/MichaelMure/git-bug/identity"
+)
+
+// EnsureIdentities walk the graph of operations and make sure that all Identity
+// are properly loaded. That is, it replace all the IdentityStub with the full
+// Identity, loaded through a Resolver.
+func (bug *Bug) EnsureIdentities(resolver identity.Resolver) error {
+	it := NewOperationIterator(bug)
+
+	for it.Next() {
+		op := it.Value()
+		base := op.base()
+
+		if stub, ok := base.Author.(*identity.IdentityStub); ok {
+			i, err := resolver.ResolveIdentity(stub.Id())
+			if err != nil {
+				return err
+			}
+
+			base.Author = i
+		}
+	}
+	return nil
+}
diff --git a/bug/operation_iterator_test.go b/bug/operation_iterator_test.go
index e1aa8911..a41120e2 100644
--- a/bug/operation_iterator_test.go
+++ b/bug/operation_iterator_test.go
@@ -20,6 +20,19 @@ var (
 	labelChangeOp = NewLabelChangeOperation(rene, unix, []Label{"added"}, []Label{"removed"})
 )
 
+func ExampleOperationIterator() {
+	b := NewBug()
+
+	// add operations
+
+	it := NewOperationIterator(b)
+
+	for it.Next() {
+		// do something with each operations
+		_ = it.Value()
+	}
+}
+
 func TestOpIterator(t *testing.T) {
 	mockRepo := repository.NewMockRepoForTest()
 
diff --git a/identity/common.go b/identity/common.go
index 5301471a..00feaa2d 100644
--- a/identity/common.go
+++ b/identity/common.go
@@ -23,12 +23,13 @@ func (e ErrMultipleMatch) Error() string {
 //
 // If the given message has a "id" field, it's considered being a proper Identity.
 func UnmarshalJSON(raw json.RawMessage) (Interface, error) {
-	// First try to decode as a normal Identity
-	var i Identity
+	aux := &IdentityStub{}
 
-	err := json.Unmarshal(raw, &i)
-	if err == nil && i.id != "" {
-		return &i, nil
+	// First try to decode and load as a normal Identity
+	err := json.Unmarshal(raw, &aux)
+	if err == nil && aux.Id() != "" {
+		return aux, nil
+		// return identityResolver.ResolveIdentity(aux.Id)
 	}
 
 	// abort if we have an error other than the wrong type
@@ -51,7 +52,3 @@ func UnmarshalJSON(raw json.RawMessage) (Interface, error) {
 
 	return nil, fmt.Errorf("unknown identity type")
 }
-
-type Resolver interface {
-	ResolveIdentity(id string) (Interface, error)
-}
diff --git a/identity/identity.go b/identity/identity.go
index 2a422789..2dafb353 100644
--- a/identity/identity.go
+++ b/identity/identity.go
@@ -48,14 +48,10 @@ func NewIdentityFull(name string, email string, login string, avatarUrl string)
 	}
 }
 
-type identityJSON struct {
-	Id string `json:"id"`
-}
-
 // MarshalJSON will only serialize the id
 func (i *Identity) MarshalJSON() ([]byte, error) {
-	return json.Marshal(identityJSON{
-		Id: i.Id(),
+	return json.Marshal(&IdentityStub{
+		id: i.Id(),
 	})
 }
 
@@ -63,35 +59,12 @@ func (i *Identity) MarshalJSON() ([]byte, error) {
 // Users of this package are expected to run Load() to load
 // the remaining data from the identities data in git.
 func (i *Identity) UnmarshalJSON(data []byte) error {
-	aux := identityJSON{}
-
-	if err := json.Unmarshal(data, &aux); err != nil {
-		return err
-	}
-
-	i.id = aux.Id
-
-	return nil
+	panic("identity should be loaded with identity.UnmarshalJSON")
 }
 
 // Read load an Identity from the identities data available in git
 func Read(repo repository.Repo, id string) (*Identity, error) {
-	i := &Identity{
-		id: id,
-	}
-
-	err := i.Load(repo)
-	if err != nil {
-		return nil, err
-	}
-
-	return i, nil
-}
-
-// Load will read the corresponding identity data from git and replace any
-// data already loaded if any.
-func (i *Identity) Load(repo repository.Repo) error {
-	ref := fmt.Sprintf("%s%s", identityRefPattern, i.Id())
+	ref := fmt.Sprintf("%s%s", identityRefPattern, id)
 
 	hashes, err := repo.ListCommits(ref)
 
@@ -99,35 +72,35 @@ func (i *Identity) Load(repo repository.Repo) error {
 
 	// TODO: this is not perfect, it might be a command invoke error
 	if err != nil {
-		return ErrIdentityNotExist
+		return nil, ErrIdentityNotExist
 	}
 
 	for _, hash := range hashes {
 		entries, err := repo.ListEntries(hash)
 		if err != nil {
-			return errors.Wrap(err, "can't list git tree entries")
+			return nil, errors.Wrap(err, "can't list git tree entries")
 		}
 
 		if len(entries) != 1 {
-			return fmt.Errorf("invalid identity data at hash %s", hash)
+			return nil, fmt.Errorf("invalid identity data at hash %s", hash)
 		}
 
 		entry := entries[0]
 
 		if entry.Name != versionEntryName {
-			return fmt.Errorf("invalid identity data at hash %s", hash)
+			return nil, fmt.Errorf("invalid identity data at hash %s", hash)
 		}
 
 		data, err := repo.ReadData(entry.Hash)
 		if err != nil {
-			return errors.Wrap(err, "failed to read git blob data")
+			return nil, errors.Wrap(err, "failed to read git blob data")
 		}
 
 		var version Version
 		err = json.Unmarshal(data, &version)
 
 		if err != nil {
-			return errors.Wrapf(err, "failed to decode Identity version json %s", hash)
+			return nil, errors.Wrapf(err, "failed to decode Identity version json %s", hash)
 		}
 
 		// tag the version with the commit hash
@@ -136,9 +109,10 @@ func (i *Identity) Load(repo repository.Repo) error {
 		versions = append(versions, &version)
 	}
 
-	i.Versions = versions
-
-	return nil
+	return &Identity{
+		id:       id,
+		Versions: versions,
+	}, nil
 }
 
 // NewFromGitUser will query the repository for user detail and
diff --git a/identity/identity_stub.go b/identity/identity_stub.go
new file mode 100644
index 00000000..0163e9d4
--- /dev/null
+++ b/identity/identity_stub.go
@@ -0,0 +1,85 @@
+package identity
+
+import (
+	"encoding/json"
+
+	"github.com/MichaelMure/git-bug/repository"
+	"github.com/MichaelMure/git-bug/util/lamport"
+)
+
+var _ Interface = &IdentityStub{}
+
+// IdentityStub is an almost empty Identity, holding only the id.
+// When a normal Identity is serialized into JSON, only the id is serialized.
+// All the other data are stored in git in a chain of commit + a ref.
+// 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
+}
+
+func (i *IdentityStub) MarshalJSON() ([]byte, error) {
+	return json.Marshal(struct {
+		Id string `json:"id"`
+	}{
+		Id: i.id,
+	})
+}
+
+func (i *IdentityStub) UnmarshalJSON(data []byte) error {
+	aux := struct {
+		Id string `json:"id"`
+	}{}
+
+	if err := json.Unmarshal(data, &aux); err != nil {
+		return err
+	}
+
+	i.id = aux.Id
+
+	return nil
+}
+
+func (i *IdentityStub) Id() string {
+	return i.id
+}
+
+func (IdentityStub) Name() string {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) Email() string {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) Login() string {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) AvatarUrl() string {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) Keys() []Key {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) ValidKeysAtTime(time lamport.Time) []Key {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) DisplayName() string {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) Validate() error {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) Commit(repo repository.Repo) error {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
+
+func (IdentityStub) IsProtected() bool {
+	panic("identities needs to be properly loaded with identity.Read()")
+}
diff --git a/identity/identity_test.go b/identity/identity_test.go
index afb804fc..f1c07e79 100644
--- a/identity/identity_test.go
+++ b/identity/identity_test.go
@@ -1,6 +1,7 @@
 package identity
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/MichaelMure/git-bug/repository"
@@ -220,3 +221,35 @@ func assertHasKeyValue(t *testing.T, metadata map[string]string, key, value stri
 	assert.True(t, ok)
 	assert.Equal(t, val, value)
 }
+
+func TestJSON(t *testing.T) {
+	mockRepo := repository.NewMockRepoForTest()
+
+	identity := &Identity{
+		Versions: []*Version{
+			{
+				Name:  "René Descartes",
+				Email: "rene.descartes@example.com",
+			},
+		},
+	}
+
+	// commit to make sure we have an ID
+	err := identity.Commit(mockRepo)
+	assert.Nil(t, err)
+	assert.NotEmpty(t, identity.id)
+
+	// serialize
+	data, err := json.Marshal(identity)
+	assert.NoError(t, err)
+
+	// deserialize, got a IdentityStub with the same id
+	var i Interface
+	i, err = UnmarshalJSON(data)
+	assert.NoError(t, err)
+	assert.Equal(t, identity.id, i.Id())
+
+	// make sure we can load the identity properly
+	i, err = Read(mockRepo, i.Id())
+	assert.NoError(t, err)
+}
diff --git a/identity/resolver.go b/identity/resolver.go
new file mode 100644
index 00000000..63dc994f
--- /dev/null
+++ b/identity/resolver.go
@@ -0,0 +1,22 @@
+package identity
+
+import "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)
+}
+
+// DefaultResolver is a Resolver loading Identities directly from a Repo
+type SimpleResolver struct {
+	repo repository.Repo
+}
+
+func NewSimpleResolver(repo repository.Repo) *SimpleResolver {
+	return &SimpleResolver{repo: repo}
+}
+
+func (r *SimpleResolver) ResolveIdentity(id string) (Interface, error) {
+	return Read(r.repo, id)
+}
diff --git a/misc/random_bugs/create_random_bugs.go b/misc/random_bugs/create_random_bugs.go
index 085e89f0..0657c808 100644
--- a/misc/random_bugs/create_random_bugs.go
+++ b/misc/random_bugs/create_random_bugs.go
@@ -34,7 +34,9 @@ func CommitRandomBugs(repo repository.ClockedRepo, opts Options) {
 }
 
 func CommitRandomBugsWithSeed(repo repository.ClockedRepo, opts Options, seed int64) {
-	bugs := GenerateRandomBugsWithSeed(opts, seed)
+	generateRandomPersons(repo, opts.PersonNumber)
+
+	bugs := generateRandomBugsWithSeed(opts, seed)
 
 	for _, b := range bugs {
 		err := b.Commit(repo)
@@ -44,11 +46,7 @@ func CommitRandomBugsWithSeed(repo repository.ClockedRepo, opts Options, seed in
 	}
 }
 
-func GenerateRandomBugs(opts Options) []*bug.Bug {
-	return GenerateRandomBugsWithSeed(opts, time.Now().UnixNano())
-}
-
-func GenerateRandomBugsWithSeed(opts Options, seed int64) []*bug.Bug {
+func generateRandomBugsWithSeed(opts Options, seed int64) []*bug.Bug {
 	rand.Seed(seed)
 	fake.Seed(seed)
 
@@ -67,7 +65,7 @@ func GenerateRandomBugsWithSeed(opts Options, seed int64) []*bug.Bug {
 		addedLabels = []string{}
 
 		b, _, err := bug.Create(
-			randomPerson(opts.PersonNumber),
+			randomPerson(),
 			time.Now().Unix(),
 			fake.Sentence(),
 			paragraphs(),
@@ -85,7 +83,7 @@ func GenerateRandomBugsWithSeed(opts Options, seed int64) []*bug.Bug {
 
 		for j := 0; j < nOps; j++ {
 			index := rand.Intn(len(opsGenerators))
-			opsGenerators[index](b, randomPerson(opts.PersonNumber))
+			opsGenerators[index](b, randomPerson())
 		}
 
 		result[i] = b
@@ -101,6 +99,9 @@ func GenerateRandomOperationPacks(packNumber int, opNumber int) []*bug.Operation
 func GenerateRandomOperationPacksWithSeed(packNumber int, opNumber int, seed int64) []*bug.OperationPack {
 	// Note: this is a bit crude, only generate a Create + Comments
 
+	panic("this piece of code needs to be updated to make sure that the identities " +
+		"are properly commit before usage. That is, generateRandomPersons() need to be called.")
+
 	rand.Seed(seed)
 	fake.Seed(seed)
 
@@ -112,7 +113,7 @@ func GenerateRandomOperationPacksWithSeed(packNumber int, opNumber int, seed int
 		var op bug.Operation
 
 		op = bug.NewCreateOp(
-			randomPerson(5),
+			randomPerson(),
 			time.Now().Unix(),
 			fake.Sentence(),
 			paragraphs(),
@@ -123,7 +124,7 @@ func GenerateRandomOperationPacksWithSeed(packNumber int, opNumber int, seed int
 
 		for j := 0; j < opNumber-1; j++ {
 			op = bug.NewAddCommentOp(
-				randomPerson(5),
+				randomPerson(),
 				time.Now().Unix(),
 				paragraphs(),
 				nil,
@@ -143,15 +144,20 @@ func person() identity.Interface {
 
 var persons []identity.Interface
 
-func randomPerson(personNumber int) identity.Interface {
-	if len(persons) == 0 {
-		persons = make([]identity.Interface, personNumber)
-		for i := range persons {
-			persons[i] = person()
+func generateRandomPersons(repo repository.ClockedRepo, n int) {
+	persons = make([]identity.Interface, n)
+	for i := range persons {
+		p := person()
+		err := p.Commit(repo)
+		if err != nil {
+			panic(err)
 		}
+		persons[i] = p
 	}
+}
 
-	index := rand.Intn(personNumber)
+func randomPerson() identity.Interface {
+	index := rand.Intn(len(persons))
 	return persons[index]
 }