Merge remote-tracking branch 'origin/master' into dev-gh-bridge

11 files changed, 773 insertions, 418 deletions

diff --git a/identity/identity.go b/identity/identity.go
index 8182e263..ad5f1efd 100644
--- a/identity/identity.go
+++ b/identity/identity.go
@@ -5,8 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
-	"strings"
-	"time"
 
 	"github.com/pkg/errors"
 
@@ -35,47 +33,27 @@ var _ Interface = &Identity{}
 var _ entity.Interface = &Identity{}
 
 type Identity struct {
-	// Id used as unique identifier
-	id entity.Id
-
 	// all the successive version of the identity
-	versions []*Version
-
-	// not serialized
-	lastCommit repository.Hash
+	versions []*version
 }
 
-func NewIdentity(name string, email string) *Identity {
-	return &Identity{
-		id: entity.UnsetId,
-		versions: []*Version{
-			{
-				name:  name,
-				email: email,
-				nonce: makeNonce(20),
-			},
-		},
-	}
+func NewIdentity(repo repository.RepoClock, name string, email string) (*Identity, error) {
+	return NewIdentityFull(repo, name, email, "", "", nil)
 }
 
-func NewIdentityFull(name string, email string, login string, avatarUrl string) *Identity {
-	return &Identity{
-		id: entity.UnsetId,
-		versions: []*Version{
-			{
-				name:      name,
-				email:     email,
-				login:     login,
-				avatarURL: avatarUrl,
-				nonce:     makeNonce(20),
-			},
-		},
+func NewIdentityFull(repo repository.RepoClock, name string, email string, login string, avatarUrl string, keys []*Key) (*Identity, error) {
+	v, err := newVersion(repo, name, email, login, avatarUrl, keys)
+	if err != nil {
+		return nil, err
 	}
+	return &Identity{
+		versions: []*version{v},
+	}, nil
 }
 
 // NewFromGitUser will query the repository for user detail and
 // build the corresponding Identity
-func NewFromGitUser(repo repository.Repo) (*Identity, error) {
+func NewFromGitUser(repo repository.ClockedRepo) (*Identity, error) {
 	name, err := repo.GetUserName()
 	if err != nil {
 		return nil, err
@@ -92,13 +70,13 @@ func NewFromGitUser(repo repository.Repo) (*Identity, error) {
 		return nil, errors.New("user name is not configured in git yet. Please use `git config --global user.email johndoe@example.com`")
 	}
 
-	return NewIdentity(name, email), nil
+	return NewIdentity(repo, name, email)
 }
 
 // MarshalJSON will only serialize the id
 func (i *Identity) MarshalJSON() ([]byte, error) {
 	return json.Marshal(&IdentityStub{
-		id: i.id,
+		id: i.Id(),
 	})
 }
 
@@ -123,36 +101,32 @@ 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 := entity.Id(refSplit[len(refSplit)-1])
+	id := entity.RefToId(ref)
 
 	if err := id.Validate(); err != nil {
 		return nil, errors.Wrap(err, "invalid ref")
 	}
 
 	hashes, err := repo.ListCommits(ref)
-
-	// TODO: this is not perfect, it might be a command invoke error
 	if err != nil {
 		return nil, ErrIdentityNotExist
 	}
-
-	i := &Identity{
-		id: id,
+	if len(hashes) == 0 {
+		return nil, fmt.Errorf("empty identity")
 	}
 
+	i := &Identity{}
+
 	for _, hash := range hashes {
 		entries, err := repo.ReadTree(hash)
 		if err != nil {
 			return nil, errors.Wrap(err, "can't list git tree entries")
 		}
-
 		if len(entries) != 1 {
 			return nil, fmt.Errorf("invalid identity data at hash %s", hash)
 		}
 
 		entry := entries[0]
-
 		if entry.Name != versionEntryName {
 			return nil, fmt.Errorf("invalid identity data at hash %s", hash)
 		}
@@ -162,20 +136,22 @@ func read(repo repository.Repo, ref string) (*Identity, error) {
 			return nil, errors.Wrap(err, "failed to read git blob data")
 		}
 
-		var version Version
+		var version version
 		err = json.Unmarshal(data, &version)
-
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to decode Identity version json %s", hash)
 		}
 
 		// tag the version with the commit hash
 		version.commitHash = hash
-		i.lastCommit = hash
 
 		i.versions = append(i.versions, &version)
 	}
 
+	if id != i.versions[0].Id() {
+		return nil, fmt.Errorf("identity ID doesn't math the first version ID")
+	}
+
 	return i, nil
 }
 
@@ -292,32 +268,49 @@ type Mutator struct {
 }
 
 // Mutate allow to create a new version of the Identity in one go
-func (i *Identity) Mutate(f func(orig Mutator) Mutator) {
+func (i *Identity) Mutate(repo repository.RepoClock, f func(orig *Mutator)) error {
+	copyKeys := func(keys []*Key) []*Key {
+		result := make([]*Key, len(keys))
+		for i, key := range keys {
+			result[i] = key.Clone()
+		}
+		return result
+	}
+
 	orig := Mutator{
 		Name:      i.Name(),
 		Email:     i.Email(),
 		Login:     i.Login(),
 		AvatarUrl: i.AvatarUrl(),
-		Keys:      i.Keys(),
+		Keys:      copyKeys(i.Keys()),
 	}
-	mutated := f(orig)
+	mutated := orig
+	mutated.Keys = copyKeys(orig.Keys)
+
+	f(&mutated)
+
 	if reflect.DeepEqual(orig, mutated) {
-		return
-	}
-	i.versions = append(i.versions, &Version{
-		name:      mutated.Name,
-		email:     mutated.Email,
-		login:     mutated.Login,
-		avatarURL: mutated.AvatarUrl,
-		keys:      mutated.Keys,
-	})
+		return nil
+	}
+
+	v, err := newVersion(repo,
+		mutated.Name,
+		mutated.Email,
+		mutated.Login,
+		mutated.AvatarUrl,
+		mutated.Keys,
+	)
+	if err != nil {
+		return err
+	}
+
+	i.versions = append(i.versions, v)
+	return nil
 }
 
 // Write the identity into the Repository. In particular, this ensure that
 // the Id is properly set.
 func (i *Identity) Commit(repo repository.ClockedRepo) error {
-	// Todo: check for mismatch between memory and commit data
-
 	if !i.NeedCommit() {
 		return fmt.Errorf("can't commit an identity with no pending version")
 	}
@@ -326,24 +319,14 @@ func (i *Identity) Commit(repo repository.ClockedRepo) error {
 		return errors.Wrap(err, "can't commit an identity with invalid data")
 	}
 
+	var lastCommit repository.Hash
 	for _, v := range i.versions {
 		if v.commitHash != "" {
-			i.lastCommit = v.commitHash
+			lastCommit = v.commitHash
 			// ignore already commit versions
 			continue
 		}
 
-		// get the times where new versions starts to be valid
-		// TODO: instead of this hardcoded clock for bugs only, this need to be
-		// a vector of edit clock, one for each entity (bug, PR, config ..)
-		bugEditClock, err := repo.GetOrCreateClock("bug-edit")
-		if err != nil {
-			return err
-		}
-
-		v.time = bugEditClock.Time()
-		v.unixTime = time.Now().Unix()
-
 		blobHash, err := v.Write(repo)
 		if err != nil {
 			return err
@@ -360,37 +343,21 @@ func (i *Identity) Commit(repo repository.ClockedRepo) error {
 		}
 
 		var commitHash repository.Hash
-		if i.lastCommit != "" {
-			commitHash, err = repo.StoreCommitWithParent(treeHash, i.lastCommit)
+		if lastCommit != "" {
+			commitHash, err = repo.StoreCommit(treeHash, lastCommit)
 		} else {
 			commitHash, err = repo.StoreCommit(treeHash)
 		}
-
 		if err != nil {
 			return err
 		}
 
-		i.lastCommit = commitHash
+		lastCommit = commitHash
 		v.commitHash = commitHash
-
-		// if it was the first commit, use the commit hash as the Identity id
-		if i.id == "" || i.id == entity.UnsetId {
-			i.id = entity.Id(commitHash)
-		}
-	}
-
-	if i.id == "" {
-		panic("identity with no id")
 	}
 
-	ref := fmt.Sprintf("%s%s", identityRefPattern, i.id)
-	err := repo.UpdateRef(ref, i.lastCommit)
-
-	if err != nil {
-		return err
-	}
-
-	return nil
+	ref := fmt.Sprintf("%s%s", identityRefPattern, i.Id().String())
+	return repo.UpdateRef(ref, lastCommit)
 }
 
 func (i *Identity) CommitAsNeeded(repo repository.ClockedRepo) error {
@@ -433,20 +400,17 @@ func (i *Identity) NeedCommit() bool {
 // confident enough to implement that. I choose the strict fast-forward only approach,
 // despite it's potential problem with two different version as mentioned above.
 func (i *Identity) Merge(repo repository.Repo, other *Identity) (bool, error) {
-	if i.id != other.id {
+	if i.Id() != other.Id() {
 		return false, errors.New("merging unrelated identities is not supported")
 	}
 
-	if i.lastCommit == "" || other.lastCommit == "" {
-		return false, errors.New("can't merge identities that has never been stored")
-	}
-
 	modified := false
+	var lastCommit repository.Hash
 	for j, otherVersion := range other.versions {
 		// if there is more version in other, take them
 		if len(i.versions) == j {
 			i.versions = append(i.versions, otherVersion)
-			i.lastCommit = otherVersion.commitHash
+			lastCommit = otherVersion.commitHash
 			modified = true
 		}
 
@@ -458,7 +422,7 @@ func (i *Identity) Merge(repo repository.Repo, other *Identity) (bool, error) {
 	}
 
 	if modified {
-		err := repo.UpdateRef(identityRefPattern+i.id.String(), i.lastCommit)
+		err := repo.UpdateRef(identityRefPattern+i.Id().String(), lastCommit)
 		if err != nil {
 			return false, err
 		}
@@ -469,7 +433,7 @@ func (i *Identity) Merge(repo repository.Repo, other *Identity) (bool, error) {
 
 // Validate check if the Identity data is valid
 func (i *Identity) Validate() error {
-	lastTime := lamport.Time(0)
+	lastTimes := make(map[string]lamport.Time)
 
 	if len(i.versions) == 0 {
 		return fmt.Errorf("no version")
@@ -480,22 +444,27 @@ func (i *Identity) Validate() error {
 			return err
 		}
 
-		if v.commitHash != "" && v.time < lastTime {
-			return fmt.Errorf("non-chronological version (%d --> %d)", lastTime, v.time)
+		// check for always increasing lamport time
+		// check that a new version didn't drop a clock
+		for name, previous := range lastTimes {
+			if now, ok := v.times[name]; ok {
+				if now < previous {
+					return fmt.Errorf("non-chronological lamport clock %s (%d --> %d)", name, previous, now)
+				}
+			} else {
+				return fmt.Errorf("version has less lamport clocks than before (missing %s)", name)
+			}
 		}
 
-		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.String() {
-		return fmt.Errorf("identity id should be the first commit hash")
+		for name, now := range v.times {
+			lastTimes[name] = now
+		}
 	}
 
 	return nil
 }
 
-func (i *Identity) lastVersion() *Version {
+func (i *Identity) lastVersion() *version {
 	if len(i.versions) <= 0 {
 		panic("no version at all")
 	}
@@ -505,12 +474,8 @@ func (i *Identity) lastVersion() *Version {
 
 // Id return the Identity identifier
 func (i *Identity) Id() entity.Id {
-	if i.id == "" || i.id == entity.UnsetId {
-		// simply panic as it would be a coding error
-		// (using an id of an identity not stored yet)
-		panic("no id yet")
-	}
-	return i.id
+	// id is the id of the first version
+	return i.versions[0].Id()
 }
 
 // Name return the last version of the name
@@ -518,6 +483,21 @@ func (i *Identity) Name() string {
 	return i.lastVersion().name
 }
 
+// DisplayName return a non-empty string to display, representing the
+// identity, based on the non-empty values.
+func (i *Identity) DisplayName() string {
+	switch {
+	case i.Name() == "" && i.Login() != "":
+		return i.Login()
+	case i.Name() != "" && i.Login() == "":
+		return i.Name()
+	case i.Name() != "" && i.Login() != "":
+		return fmt.Sprintf("%s (%s)", i.Name(), i.Login())
+	}
+
+	panic("invalid person data")
+}
+
 // Email return the last version of the email
 func (i *Identity) Email() string {
 	return i.lastVersion().email
@@ -538,12 +518,35 @@ func (i *Identity) Keys() []*Key {
 	return i.lastVersion().keys
 }
 
+// SigningKey return the key that should be used to sign new messages. If no key is available, return nil.
+func (i *Identity) SigningKey(repo repository.RepoKeyring) (*Key, error) {
+	keys := i.Keys()
+	for _, key := range keys {
+		err := key.ensurePrivateKey(repo)
+		if err == errNoPrivateKey {
+			continue
+		}
+		if err != nil {
+			return nil, err
+		}
+		return key, nil
+	}
+	return nil, nil
+}
+
 // ValidKeysAtTime return the set of keys valid at a given lamport time
-func (i *Identity) ValidKeysAtTime(time lamport.Time) []*Key {
+func (i *Identity) ValidKeysAtTime(clockName string, time lamport.Time) []*Key {
 	var result []*Key
 
+	var lastTime lamport.Time
 	for _, v := range i.versions {
-		if v.time > time {
+		refTime, ok := v.times[clockName]
+		if !ok {
+			refTime = lastTime
+		}
+		lastTime = refTime
+
+		if refTime > time {
 			return result
 		}
 
@@ -553,19 +556,14 @@ func (i *Identity) ValidKeysAtTime(time lamport.Time) []*Key {
 	return result
 }
 
-// DisplayName return a non-empty string to display, representing the
-// identity, based on the non-empty values.
-func (i *Identity) DisplayName() string {
-	switch {
-	case i.Name() == "" && i.Login() != "":
-		return i.Login()
-	case i.Name() != "" && i.Login() == "":
-		return i.Name()
-	case i.Name() != "" && i.Login() != "":
-		return fmt.Sprintf("%s (%s)", i.Name(), i.Login())
-	}
+// LastModification return the timestamp at which the last version of the identity became valid.
+func (i *Identity) LastModification() timestamp.Timestamp {
+	return timestamp.Timestamp(i.lastVersion().unixTime)
+}
 
-	panic("invalid person data")
+// LastModificationLamports return the lamport times at which the last version of the identity became valid.
+func (i *Identity) LastModificationLamports() map[string]lamport.Time {
+	return i.lastVersion().times
 }
 
 // IsProtected return true if the chain of git commits started to be signed.
@@ -575,27 +573,23 @@ func (i *Identity) IsProtected() bool {
 	return false
 }
 
-// LastModificationLamportTime return the Lamport time at which the last version of the identity became valid.
-func (i *Identity) LastModificationLamport() lamport.Time {
-	return i.lastVersion().time
-}
-
-// LastModification return the timestamp at which the last version of the identity became valid.
-func (i *Identity) LastModification() timestamp.Timestamp {
-	return timestamp.Timestamp(i.lastVersion().unixTime)
-}
-
-// SetMetadata store arbitrary metadata along the last not-commit Version.
-// If the Version has been commit to git already, a new identical version is added and will need to be
+// SetMetadata store arbitrary metadata along the last not-commit version.
+// If the version has been commit to git already, a new identical version is added and will need to be
 // commit.
 func (i *Identity) SetMetadata(key string, value string) {
+	// once commit, data is immutable so we create a new version
 	if i.lastVersion().commitHash != "" {
 		i.versions = append(i.versions, i.lastVersion().Clone())
 	}
+	// if Id() has been called, we can't change the first version anymore, so we create a new version
+	if len(i.versions) == 1 && i.versions[0].id != entity.UnsetId && i.versions[0].id != "" {
+		i.versions = append(i.versions, i.lastVersion().Clone())
+	}
+
 	i.lastVersion().SetMetadata(key, value)
 }
 
-// ImmutableMetadata return all metadata for this Identity, accumulated from each Version.
+// ImmutableMetadata return all metadata for this Identity, accumulated from each version.
 // If multiple value are found, the first defined takes precedence.
 func (i *Identity) ImmutableMetadata() map[string]string {
 	metadata := make(map[string]string)
@@ -611,7 +605,7 @@ func (i *Identity) ImmutableMetadata() map[string]string {
 	return metadata
 }
 
-// MutableMetadata return all metadata for this Identity, accumulated from each Version.
+// MutableMetadata return all metadata for this Identity, accumulated from each version.
 // If multiple value are found, the last defined takes precedence.
 func (i *Identity) MutableMetadata() map[string]string {
 	metadata := make(map[string]string)
@@ -624,9 +618,3 @@ func (i *Identity) MutableMetadata() map[string]string {
 
 	return metadata
 }
-
-// addVersionForTest add a new version to the identity
-// Only for testing !
-func (i *Identity) addVersionForTest(version *Version) {
-	i.versions = append(i.versions, version)
-}
diff --git a/identity/identity_actions.go b/identity/identity_actions.go
index 2e804533..b58bb2d9 100644
--- a/identity/identity_actions.go
+++ b/identity/identity_actions.go
@@ -13,19 +13,12 @@ import (
 // Fetch retrieve updates from a remote
 // This does not change the local identities state
 func Fetch(repo repository.Repo, remote string) (string, error) {
-	// "refs/identities/*:refs/remotes/<remote>/identities/*"
-	remoteRefSpec := fmt.Sprintf(identityRemoteRefPattern, remote)
-	fetchRefSpec := fmt.Sprintf("%s*:%s*", identityRefPattern, remoteRefSpec)
-
-	return repo.FetchRefs(remote, fetchRefSpec)
+	return repo.FetchRefs(remote, "identities")
 }
 
 // Push update a remote with the local changes
 func Push(repo repository.Repo, remote string) (string, error) {
-	// "refs/identities/*:refs/identities/*"
-	refspec := fmt.Sprintf("%s*:%s*", identityRefPattern, identityRefPattern)
-
-	return repo.PushRefs(remote, refspec)
+	return repo.PushRefs(remote, "identities")
 }
 
 // Pull will do a Fetch + MergeAll
@@ -102,7 +95,7 @@ func MergeAll(repo repository.ClockedRepo, remote string) <-chan entity.MergeRes
 					return
 				}
 
-				out <- entity.NewMergeStatus(entity.MergeStatusNew, id, remoteIdentity)
+				out <- entity.NewMergeNewStatus(id, remoteIdentity)
 				continue
 			}
 
@@ -121,9 +114,9 @@ func MergeAll(repo repository.ClockedRepo, remote string) <-chan entity.MergeRes
 			}
 
 			if updated {
-				out <- entity.NewMergeStatus(entity.MergeStatusUpdated, id, localIdentity)
+				out <- entity.NewMergeUpdatedStatus(id, localIdentity)
 			} else {
-				out <- entity.NewMergeStatus(entity.MergeStatusNothing, id, localIdentity)
+				out <- entity.NewMergeNothingStatus(id)
 			}
 		}
 	}()
diff --git a/identity/identity_actions_test.go b/identity/identity_actions_test.go
index 773574c6..2a5954d6 100644
--- a/identity/identity_actions_test.go
+++ b/identity/identity_actions_test.go
@@ -8,12 +8,13 @@ import (
 	"github.com/MichaelMure/git-bug/repository"
 )
 
-func TestPushPull(t *testing.T) {
-	repoA, repoB, remote := repository.SetupReposAndRemote()
+func TestIdentityPushPull(t *testing.T) {
+	repoA, repoB, remote := repository.SetupGoGitReposAndRemote()
 	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
-	identity1 := NewIdentity("name1", "email1")
-	err := identity1.Commit(repoA)
+	identity1, err := NewIdentity(repoA, "name1", "email1")
+	require.NoError(t, err)
+	err = identity1.Commit(repoA)
 	require.NoError(t, err)
 
 	// A --> remote --> B
@@ -30,7 +31,8 @@ func TestPushPull(t *testing.T) {
 	}
 
 	// B --> remote --> A
-	identity2 := NewIdentity("name2", "email2")
+	identity2, err := NewIdentity(repoB, "name2", "email2")
+	require.NoError(t, err)
 	err = identity2.Commit(repoB)
 	require.NoError(t, err)
 
@@ -48,17 +50,19 @@ func TestPushPull(t *testing.T) {
 
 	// Update both
 
-	identity1.addVersionForTest(&Version{
-		name:  "name1b",
-		email: "email1b",
+	err = identity1.Mutate(repoA, func(orig *Mutator) {
+		orig.Name = "name1b"
+		orig.Email = "email1b"
 	})
+	require.NoError(t, err)
 	err = identity1.Commit(repoA)
 	require.NoError(t, err)
 
-	identity2.addVersionForTest(&Version{
-		name:  "name2b",
-		email: "email2b",
+	err = identity2.Mutate(repoB, func(orig *Mutator) {
+		orig.Name = "name2b"
+		orig.Email = "email2b"
 	})
+	require.NoError(t, err)
 	err = identity2.Commit(repoB)
 	require.NoError(t, err)
 
@@ -92,20 +96,22 @@ func TestPushPull(t *testing.T) {
 
 	// Concurrent update
 
-	identity1.addVersionForTest(&Version{
-		name:  "name1c",
-		email: "email1c",
+	err = identity1.Mutate(repoA, func(orig *Mutator) {
+		orig.Name = "name1c"
+		orig.Email = "email1c"
 	})
+	require.NoError(t, err)
 	err = identity1.Commit(repoA)
 	require.NoError(t, err)
 
 	identity1B, err := ReadLocal(repoB, identity1.Id())
 	require.NoError(t, err)
 
-	identity1B.addVersionForTest(&Version{
-		name:  "name1concurrent",
-		email: "email1concurrent",
+	err = identity1B.Mutate(repoB, func(orig *Mutator) {
+		orig.Name = "name1concurrent"
+		orig.Email = "name1concurrent"
 	})
+	require.NoError(t, err)
 	err = identity1B.Commit(repoB)
 	require.NoError(t, err)
 
diff --git a/identity/identity_stub.go b/identity/identity_stub.go
index f4bf1d37..fb5c90a5 100644
--- a/identity/identity_stub.go
+++ b/identity/identity_stub.go
@@ -52,6 +52,10 @@ func (IdentityStub) Name() string {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
+func (IdentityStub) DisplayName() string {
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
+}
+
 func (IdentityStub) Email() string {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
@@ -68,23 +72,19 @@ func (IdentityStub) Keys() []*Key {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
-func (IdentityStub) ValidKeysAtTime(_ lamport.Time) []*Key {
+func (i *IdentityStub) SigningKey(repo repository.RepoKeyring) (*Key, error) {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
-func (IdentityStub) DisplayName() string {
+func (IdentityStub) ValidKeysAtTime(_ string, _ lamport.Time) []*Key {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
-func (IdentityStub) Validate() error {
-	panic("identities needs to be properly loaded with identity.ReadLocal()")
-}
-
-func (IdentityStub) CommitWithRepo(repo repository.ClockedRepo) error {
+func (i *IdentityStub) LastModification() timestamp.Timestamp {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
-func (i *IdentityStub) CommitAsNeededWithRepo(repo repository.ClockedRepo) error {
+func (i *IdentityStub) LastModificationLamports() map[string]lamport.Time {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
@@ -92,11 +92,7 @@ func (IdentityStub) IsProtected() bool {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
-func (i *IdentityStub) LastModificationLamport() lamport.Time {
-	panic("identities needs to be properly loaded with identity.ReadLocal()")
-}
-
-func (i *IdentityStub) LastModification() timestamp.Timestamp {
+func (IdentityStub) Validate() error {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
diff --git a/identity/identity_test.go b/identity/identity_test.go
index 82e58b01..2cdb4b36 100644
--- a/identity/identity_test.go
+++ b/identity/identity_test.go
@@ -6,120 +6,108 @@ import (
 
 	"github.com/stretchr/testify/require"
 
-	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/repository"
+	"github.com/MichaelMure/git-bug/util/lamport"
 )
 
 // Test the commit and load of an Identity with multiple versions
 func TestIdentityCommitLoad(t *testing.T) {
-	mockRepo := repository.NewMockRepoForTest()
+	repo := makeIdentityTestRepo(t)
 
 	// single version
 
-	identity := &Identity{
-		id: entity.UnsetId,
-		versions: []*Version{
-			{
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-			},
-		},
-	}
+	identity, err := NewIdentity(repo, "René Descartes", "rene.descartes@example.com")
+	require.NoError(t, err)
 
-	err := identity.Commit(mockRepo)
+	idBeforeCommit := identity.Id()
 
+	err = identity.Commit(repo)
 	require.NoError(t, err)
-	require.NotEmpty(t, identity.id)
 
-	loaded, err := ReadLocal(mockRepo, identity.id)
+	commitsAreSet(t, identity)
+	require.NotEmpty(t, identity.Id())
+	require.Equal(t, idBeforeCommit, identity.Id())
+	require.Equal(t, idBeforeCommit, identity.versions[0].Id())
+
+	loaded, err := ReadLocal(repo, identity.Id())
 	require.NoError(t, err)
 	commitsAreSet(t, loaded)
 	require.Equal(t, identity, loaded)
 
-	// multiple version
+	// multiple versions
 
-	identity = &Identity{
-		id: entity.UnsetId,
-		versions: []*Version{
-			{
-				time:  100,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-				keys: []*Key{
-					{PubKey: "pubkeyA"},
-				},
-			},
-			{
-				time:  200,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-				keys: []*Key{
-					{PubKey: "pubkeyB"},
-				},
-			},
-			{
-				time:  201,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-				keys: []*Key{
-					{PubKey: "pubkeyC"},
-				},
-			},
-		},
-	}
+	identity, err = NewIdentityFull(repo, "René Descartes", "rene.descartes@example.com", "", "", []*Key{generatePublicKey()})
+	require.NoError(t, err)
 
-	err = identity.Commit(mockRepo)
+	idBeforeCommit = identity.Id()
 
+	err = identity.Mutate(repo, func(orig *Mutator) {
+		orig.Keys = []*Key{generatePublicKey()}
+	})
 	require.NoError(t, err)
-	require.NotEmpty(t, identity.id)
 
-	loaded, err = ReadLocal(mockRepo, identity.id)
+	err = identity.Mutate(repo, func(orig *Mutator) {
+		orig.Keys = []*Key{generatePublicKey()}
+	})
+	require.NoError(t, err)
+
+	require.Equal(t, idBeforeCommit, identity.Id())
+
+	err = identity.Commit(repo)
+	require.NoError(t, err)
+
+	commitsAreSet(t, identity)
+	require.NotEmpty(t, identity.Id())
+	require.Equal(t, idBeforeCommit, identity.Id())
+	require.Equal(t, idBeforeCommit, identity.versions[0].Id())
+
+	loaded, err = ReadLocal(repo, identity.Id())
 	require.NoError(t, err)
 	commitsAreSet(t, loaded)
 	require.Equal(t, identity, loaded)
 
 	// add more version
 
-	identity.addVersionForTest(&Version{
-		time:  201,
-		name:  "René Descartes",
-		email: "rene.descartes@example.com",
-		keys: []*Key{
-			{PubKey: "pubkeyD"},
-		},
+	err = identity.Mutate(repo, func(orig *Mutator) {
+		orig.Email = "rene@descartes.com"
+		orig.Keys = []*Key{generatePublicKey()}
 	})
+	require.NoError(t, err)
 
-	identity.addVersionForTest(&Version{
-		time:  300,
-		name:  "René Descartes",
-		email: "rene.descartes@example.com",
-		keys: []*Key{
-			{PubKey: "pubkeyE"},
-		},
+	err = identity.Mutate(repo, func(orig *Mutator) {
+		orig.Email = "rene@descartes.com"
+		orig.Keys = []*Key{generatePublicKey(), generatePublicKey()}
 	})
+	require.NoError(t, err)
 
-	err = identity.Commit(mockRepo)
-
+	err = identity.Commit(repo)
 	require.NoError(t, err)
-	require.NotEmpty(t, identity.id)
 
-	loaded, err = ReadLocal(mockRepo, identity.id)
+	commitsAreSet(t, identity)
+	require.NotEmpty(t, identity.Id())
+	require.Equal(t, idBeforeCommit, identity.Id())
+	require.Equal(t, idBeforeCommit, identity.versions[0].Id())
+
+	loaded, err = ReadLocal(repo, identity.Id())
 	require.NoError(t, err)
 	commitsAreSet(t, loaded)
 	require.Equal(t, identity, loaded)
 }
 
 func TestIdentityMutate(t *testing.T) {
-	identity := NewIdentity("René Descartes", "rene.descartes@example.com")
+	repo := makeIdentityTestRepo(t)
+
+	identity, err := NewIdentity(repo, "René Descartes", "rene.descartes@example.com")
+	require.NoError(t, err)
 
 	require.Len(t, identity.versions, 1)
 
-	identity.Mutate(func(orig Mutator) Mutator {
+	err = identity.Mutate(repo, func(orig *Mutator) {
 		orig.Email = "rene@descartes.fr"
 		orig.Name = "René"
 		orig.Login = "rene"
-		return orig
 	})
+	require.NoError(t, err)
 
 	require.Len(t, identity.versions, 2)
 	require.Equal(t, identity.Email(), "rene@descartes.fr")
@@ -135,97 +123,93 @@ 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) {
+	pubKeyA := generatePublicKey()
+	pubKeyB := generatePublicKey()
+	pubKeyC := generatePublicKey()
+	pubKeyD := generatePublicKey()
+	pubKeyE := generatePublicKey()
+
 	identity := Identity{
-		id: entity.UnsetId,
-		versions: []*Version{
+		versions: []*version{
 			{
-				time:  100,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-				keys: []*Key{
-					{PubKey: "pubkeyA"},
-				},
+				times: map[string]lamport.Time{"foo": 100},
+				keys:  []*Key{pubKeyA},
 			},
 			{
-				time:  200,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-				keys: []*Key{
-					{PubKey: "pubkeyB"},
-				},
+				times: map[string]lamport.Time{"foo": 200},
+				keys:  []*Key{pubKeyB},
 			},
 			{
-				time:  201,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-				keys: []*Key{
-					{PubKey: "pubkeyC"},
-				},
+				times: map[string]lamport.Time{"foo": 201},
+				keys:  []*Key{pubKeyC},
 			},
 			{
-				time:  201,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-				keys: []*Key{
-					{PubKey: "pubkeyD"},
-				},
+				times: map[string]lamport.Time{"foo": 201},
+				keys:  []*Key{pubKeyD},
 			},
 			{
-				time:  300,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-				keys: []*Key{
-					{PubKey: "pubkeyE"},
-				},
+				times: map[string]lamport.Time{"foo": 300},
+				keys:  []*Key{pubKeyE},
 			},
 		},
 	}
 
-	require.Nil(t, identity.ValidKeysAtTime(10))
-	require.Equal(t, identity.ValidKeysAtTime(100), []*Key{{PubKey: "pubkeyA"}})
-	require.Equal(t, identity.ValidKeysAtTime(140), []*Key{{PubKey: "pubkeyA"}})
-	require.Equal(t, identity.ValidKeysAtTime(200), []*Key{{PubKey: "pubkeyB"}})
-	require.Equal(t, identity.ValidKeysAtTime(201), []*Key{{PubKey: "pubkeyD"}})
-	require.Equal(t, identity.ValidKeysAtTime(202), []*Key{{PubKey: "pubkeyD"}})
-	require.Equal(t, identity.ValidKeysAtTime(300), []*Key{{PubKey: "pubkeyE"}})
-	require.Equal(t, identity.ValidKeysAtTime(3000), []*Key{{PubKey: "pubkeyE"}})
+	require.Nil(t, identity.ValidKeysAtTime("foo", 10))
+	require.Equal(t, identity.ValidKeysAtTime("foo", 100), []*Key{pubKeyA})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 140), []*Key{pubKeyA})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 200), []*Key{pubKeyB})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 201), []*Key{pubKeyD})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 202), []*Key{pubKeyD})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 300), []*Key{pubKeyE})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 3000), []*Key{pubKeyE})
 }
 
 // Test the immutable or mutable metadata search
 func TestMetadata(t *testing.T) {
-	mockRepo := repository.NewMockRepoForTest()
+	repo := makeIdentityTestRepo(t)
 
-	identity := NewIdentity("René Descartes", "rene.descartes@example.com")
+	identity, err := NewIdentity(repo, "René Descartes", "rene.descartes@example.com")
+	require.NoError(t, err)
 
 	identity.SetMetadata("key1", "value1")
 	assertHasKeyValue(t, identity.ImmutableMetadata(), "key1", "value1")
 	assertHasKeyValue(t, identity.MutableMetadata(), "key1", "value1")
 
-	err := identity.Commit(mockRepo)
+	err = identity.Commit(repo)
 	require.NoError(t, err)
 
 	assertHasKeyValue(t, identity.ImmutableMetadata(), "key1", "value1")
 	assertHasKeyValue(t, identity.MutableMetadata(), "key1", "value1")
 
 	// try override
-	identity.addVersionForTest(&Version{
-		name:  "René Descartes",
-		email: "rene.descartes@example.com",
+	err = identity.Mutate(repo, func(orig *Mutator) {
+		orig.Email = "rene@descartes.fr"
 	})
+	require.NoError(t, err)
 
 	identity.SetMetadata("key1", "value2")
 	assertHasKeyValue(t, identity.ImmutableMetadata(), "key1", "value1")
 	assertHasKeyValue(t, identity.MutableMetadata(), "key1", "value2")
 
-	err = identity.Commit(mockRepo)
+	err = identity.Commit(repo)
 	require.NoError(t, err)
 
 	// reload
-	loaded, err := ReadLocal(mockRepo, identity.id)
+	loaded, err := ReadLocal(repo, identity.Id())
 	require.NoError(t, err)
 
 	assertHasKeyValue(t, loaded.ImmutableMetadata(), "key1", "value1")
 	assertHasKeyValue(t, loaded.MutableMetadata(), "key1", "value2")
+
+	// set metadata after commit
+	versionCount := len(identity.versions)
+	identity.SetMetadata("foo", "bar")
+	require.True(t, identity.NeedCommit())
+	require.Len(t, identity.versions, versionCount+1)
+
+	err = identity.Commit(repo)
+	require.NoError(t, err)
+	require.Len(t, identity.versions, versionCount+1)
 }
 
 func assertHasKeyValue(t *testing.T, metadata map[string]string, key, value string) {
@@ -235,22 +219,15 @@ func assertHasKeyValue(t *testing.T, metadata map[string]string, key, value stri
 }
 
 func TestJSON(t *testing.T) {
-	mockRepo := repository.NewMockRepoForTest()
+	repo := makeIdentityTestRepo(t)
 
-	identity := &Identity{
-		id: entity.UnsetId,
-		versions: []*Version{
-			{
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
-			},
-		},
-	}
+	identity, err := NewIdentity(repo, "René Descartes", "rene.descartes@example.com")
+	require.NoError(t, err)
 
 	// commit to make sure we have an Id
-	err := identity.Commit(mockRepo)
+	err = identity.Commit(repo)
 	require.NoError(t, err)
-	require.NotEmpty(t, identity.id)
+	require.NotEmpty(t, identity.Id())
 
 	// serialize
 	data, err := json.Marshal(identity)
@@ -260,10 +237,10 @@ func TestJSON(t *testing.T) {
 	var i Interface
 	i, err = UnmarshalJSON(data)
 	require.NoError(t, err)
-	require.Equal(t, identity.id, i.Id())
+	require.Equal(t, identity.Id(), i.Id())
 
 	// make sure we can load the identity properly
-	i, err = ReadLocal(mockRepo, i.Id())
+	i, err = ReadLocal(repo, i.Id())
 	require.NoError(t, err)
 }
 
@@ -280,7 +257,9 @@ func TestIdentityRemove(t *testing.T) {
 	require.NoError(t, err)
 
 	// generate an identity for testing
-	rene := NewIdentity("René Descartes", "rene@descartes.fr")
+	rene, err := NewIdentity(repo, "René Descartes", "rene@descartes.fr")
+	require.NoError(t, err)
+
 	err = rene.Commit(repo)
 	require.NoError(t, err)
 
diff --git a/identity/interface.go b/identity/interface.go
index a7174962..5b14295b 100644
--- a/identity/interface.go
+++ b/identity/interface.go
@@ -2,6 +2,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"
 )
@@ -13,6 +14,10 @@ type Interface interface {
 	// Can be empty.
 	Name() string
 
+	// DisplayName return a non-empty string to display, representing the
+	// identity, based on the non-empty values.
+	DisplayName() string
+
 	// Email return the last version of the email
 	// Can be empty.
 	Email() string
@@ -32,26 +37,25 @@ type Interface interface {
 	// Can be empty.
 	Keys() []*Key
 
-	// ValidKeysAtTime return the set of keys valid at a given lamport time
+	// SigningKey return the key that should be used to sign new messages. If no key is available, return nil.
+	SigningKey(repo repository.RepoKeyring) (*Key, error)
+
+	// ValidKeysAtTime return the set of keys valid at a given lamport time for a given clock of another entity
 	// Can be empty.
-	ValidKeysAtTime(time lamport.Time) []*Key
+	ValidKeysAtTime(clockName string, time lamport.Time) []*Key
 
-	// DisplayName return a non-empty string to display, representing the
-	// identity, based on the non-empty values.
-	DisplayName() string
+	// LastModification return the timestamp at which the last version of the identity became valid.
+	LastModification() timestamp.Timestamp
 
-	// Validate check if the Identity data is valid
-	Validate() error
+	// LastModificationLamports return the lamport times at which the last version of the identity became valid.
+	LastModificationLamports() map[string]lamport.Time
 
 	// IsProtected return true if the chain of git commits started to be signed.
 	// If that's the case, only signed commit with a valid key for this identity can be added.
 	IsProtected() bool
 
-	// LastModificationLamportTime return the Lamport time at which the last version of the identity became valid.
-	LastModificationLamport() lamport.Time
-
-	// LastModification return the timestamp at which the last version of the identity became valid.
-	LastModification() timestamp.Timestamp
+	// Validate check if the Identity data is valid
+	Validate() error
 
 	// Indicate that the in-memory state changed and need to be commit in the repository
 	NeedCommit() bool
diff --git a/identity/key.go b/identity/key.go
index cc948394..daa66b0e 100644
--- a/identity/key.go
+++ b/identity/key.go
@@ -1,18 +1,224 @@
 package identity
 
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io"
+	"strings"
+	"time"
+
+	"github.com/pkg/errors"
+	"golang.org/x/crypto/openpgp"
+	"golang.org/x/crypto/openpgp/armor"
+	"golang.org/x/crypto/openpgp/packet"
+
+	"github.com/MichaelMure/git-bug/repository"
+)
+
+var errNoPrivateKey = fmt.Errorf("no private key")
+
 type Key struct {
-	// The GPG fingerprint of the key
-	Fingerprint string `json:"fingerprint"`
-	PubKey      string `json:"pub_key"`
+	public  *packet.PublicKey
+	private *packet.PrivateKey
+}
+
+// GenerateKey generate a keypair (public+private)
+// The type and configuration of the key is determined by the default value in go's OpenPGP.
+func GenerateKey() *Key {
+	entity, err := openpgp.NewEntity("", "", "", &packet.Config{
+		// The armored format doesn't include the creation time, which makes the round-trip data not being fully equal.
+		// We don't care about the creation time so we can set it to the zero value.
+		Time: func() time.Time {
+			return time.Time{}
+		},
+	})
+	if err != nil {
+		panic(err)
+	}
+	return &Key{
+		public:  entity.PrimaryKey,
+		private: entity.PrivateKey,
+	}
+}
+
+// generatePublicKey generate only a public key (only useful for testing)
+// See GenerateKey for the details.
+func generatePublicKey() *Key {
+	k := GenerateKey()
+	k.private = nil
+	return k
+}
+
+func (k *Key) Public() *packet.PublicKey {
+	return k.public
+}
+
+func (k *Key) Private() *packet.PrivateKey {
+	return k.private
 }
 
 func (k *Key) Validate() error {
-	// Todo
+	if k.public == nil {
+		return fmt.Errorf("nil public key")
+	}
+	if !k.public.CanSign() {
+		return fmt.Errorf("public key can't sign")
+	}
+
+	if k.private != nil {
+		if !k.private.CanSign() {
+			return fmt.Errorf("private key can't sign")
+		}
+	}
 
 	return nil
 }
 
 func (k *Key) Clone() *Key {
-	clone := *k
-	return &clone
+	clone := &Key{}
+
+	pub := *k.public
+	clone.public = &pub
+
+	if k.private != nil {
+		priv := *k.private
+		clone.private = &priv
+	}
+
+	return clone
+}
+
+func (k *Key) MarshalJSON() ([]byte, error) {
+	// Serialize only the public key, in the armored format.
+	var buf bytes.Buffer
+	w, err := armor.Encode(&buf, openpgp.PublicKeyType, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	err = k.public.Serialize(w)
+	if err != nil {
+		return nil, err
+	}
+	err = w.Close()
+	if err != nil {
+		return nil, err
+	}
+	return json.Marshal(buf.String())
+}
+
+func (k *Key) UnmarshalJSON(data []byte) error {
+	// De-serialize only the public key, in the armored format.
+	var armored string
+	err := json.Unmarshal(data, &armored)
+	if err != nil {
+		return err
+	}
+
+	block, err := armor.Decode(strings.NewReader(armored))
+	if err == io.EOF {
+		return fmt.Errorf("no armored data found")
+	}
+	if err != nil {
+		return err
+	}
+
+	if block.Type != openpgp.PublicKeyType {
+		return fmt.Errorf("invalid key type")
+	}
+
+	p, err := packet.Read(block.Body)
+	if err != nil {
+		return errors.Wrap(err, "failed to read public key packet")
+	}
+
+	public, ok := p.(*packet.PublicKey)
+	if !ok {
+		return errors.New("got no packet.publicKey")
+	}
+
+	// The armored format doesn't include the creation time, which makes the round-trip data not being fully equal.
+	// We don't care about the creation time so we can set it to the zero value.
+	public.CreationTime = time.Time{}
+
+	k.public = public
+	return nil
+}
+
+func (k *Key) loadPrivate(repo repository.RepoKeyring) error {
+	item, err := repo.Keyring().Get(k.public.KeyIdString())
+	if err == repository.ErrKeyringKeyNotFound {
+		return errNoPrivateKey
+	}
+	if err != nil {
+		return err
+	}
+
+	block, err := armor.Decode(bytes.NewReader(item.Data))
+	if err == io.EOF {
+		return fmt.Errorf("no armored data found")
+	}
+	if err != nil {
+		return err
+	}
+
+	if block.Type != openpgp.PrivateKeyType {
+		return fmt.Errorf("invalid key type")
+	}
+
+	p, err := packet.Read(block.Body)
+	if err != nil {
+		return errors.Wrap(err, "failed to read private key packet")
+	}
+
+	private, ok := p.(*packet.PrivateKey)
+	if !ok {
+		return errors.New("got no packet.privateKey")
+	}
+
+	// The armored format doesn't include the creation time, which makes the round-trip data not being fully equal.
+	// We don't care about the creation time so we can set it to the zero value.
+	private.CreationTime = time.Time{}
+
+	k.private = private
+	return nil
+}
+
+// ensurePrivateKey attempt to load the corresponding private key if it is not loaded already.
+// If no private key is found, returns errNoPrivateKey
+func (k *Key) ensurePrivateKey(repo repository.RepoKeyring) error {
+	if k.private != nil {
+		return nil
+	}
+
+	return k.loadPrivate(repo)
+}
+
+func (k *Key) storePrivate(repo repository.RepoKeyring) error {
+	var buf bytes.Buffer
+	w, err := armor.Encode(&buf, openpgp.PrivateKeyType, nil)
+	if err != nil {
+		return err
+	}
+	err = k.private.Serialize(w)
+	if err != nil {
+		return err
+	}
+	err = w.Close()
+	if err != nil {
+		return err
+	}
+
+	return repo.Keyring().Set(repository.Item{
+		Key:  k.public.KeyIdString(),
+		Data: buf.Bytes(),
+	})
+}
+
+func (k *Key) PGPEntity() *openpgp.Entity {
+	return &openpgp.Entity{
+		PrimaryKey: k.public,
+		PrivateKey: k.private,
+	}
 }
diff --git a/identity/key_test.go b/identity/key_test.go
new file mode 100644
index 00000000..6e320dc2
--- /dev/null
+++ b/identity/key_test.go
@@ -0,0 +1,60 @@
+package identity
+
+import (
+	"crypto/rsa"
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/MichaelMure/git-bug/repository"
+)
+
+func TestPublicKeyJSON(t *testing.T) {
+	k := generatePublicKey()
+
+	dataJSON, err := json.Marshal(k)
+	require.NoError(t, err)
+
+	var read Key
+	err = json.Unmarshal(dataJSON, &read)
+	require.NoError(t, err)
+
+	require.Equal(t, k, &read)
+}
+
+func TestStoreLoad(t *testing.T) {
+	repo := repository.NewMockRepoKeyring()
+
+	// public + private
+	k := GenerateKey()
+
+	// Store
+
+	dataJSON, err := json.Marshal(k)
+	require.NoError(t, err)
+
+	err = k.storePrivate(repo)
+	require.NoError(t, err)
+
+	// Load
+
+	var read Key
+	err = json.Unmarshal(dataJSON, &read)
+	require.NoError(t, err)
+
+	err = read.ensurePrivateKey(repo)
+	require.NoError(t, err)
+
+	require.Equal(t, k.public, read.public)
+
+	require.IsType(t, (*rsa.PrivateKey)(nil), k.private.PrivateKey)
+
+	// See https://github.com/golang/crypto/pull/175
+	rsaPriv := read.private.PrivateKey.(*rsa.PrivateKey)
+	back := rsaPriv.Primes[0]
+	rsaPriv.Primes[0] = rsaPriv.Primes[1]
+	rsaPriv.Primes[1] = back
+
+	require.True(t, k.private.PrivateKey.(*rsa.PrivateKey).Equal(read.private.PrivateKey))
+}
diff --git a/identity/resolver.go b/identity/resolver.go
index ab380a12..8e066e9d 100644
--- a/identity/resolver.go
+++ b/identity/resolver.go
@@ -1,6 +1,8 @@
 package identity
 
 import (
+	"sync"
+
 	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/repository"
 )
@@ -34,3 +36,36 @@ func NewStubResolver() *StubResolver {
 func (s *StubResolver) ResolveIdentity(id entity.Id) (Interface, error) {
 	return &IdentityStub{id: id}, nil
 }
+
+// CachedResolver is a resolver ensuring that loading is done only once through another Resolver.
+type CachedResolver struct {
+	mu         sync.RWMutex
+	resolver   Resolver
+	identities map[entity.Id]Interface
+}
+
+func NewCachedResolver(resolver Resolver) *CachedResolver {
+	return &CachedResolver{
+		resolver:   resolver,
+		identities: make(map[entity.Id]Interface),
+	}
+}
+
+func (c *CachedResolver) ResolveIdentity(id entity.Id) (Interface, error) {
+	c.mu.RLock()
+	if i, ok := c.identities[id]; ok {
+		c.mu.RUnlock()
+		return i, nil
+	}
+	c.mu.RUnlock()
+
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	i, err := c.resolver.ResolveIdentity(id)
+	if err != nil {
+		return nil, err
+	}
+	c.identities[id] = i
+	return i, nil
+}
diff --git a/identity/version.go b/identity/version.go
index bbf93575..1c35831e 100644
--- a/identity/version.go
+++ b/identity/version.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/pkg/errors"
 
@@ -15,76 +16,131 @@ import (
 )
 
 // 1: original format
-const formatVersion = 1
-
-// Version is a complete set of information about an Identity at a point in time.
-type Version struct {
-	// The lamport time at which this version become effective
-	// The reference time is the bug edition lamport clock
-	// It must be the first field in this struct due to https://github.com/golang/go/issues/599
-	//
-	// TODO: BREAKING CHANGE - this need to actually be one edition lamport time **per entity**
-	// This is not a problem right now but will be when more entities are added (pull-request, config ...)
-	time     lamport.Time
-	unixTime int64
+// 2: Identity Ids are generated from the first version serialized data instead of from the first git commit
+//    + Identity hold multiple lamport clocks from other entities, instead of just bug edit
+const formatVersion = 2
 
+// version is a complete set of information about an Identity at a point in time.
+type version struct {
 	name      string
 	email     string // as defined in git or from a bridge when importing the identity
 	login     string // from a bridge when importing the identity
 	avatarURL string
 
+	// The lamport times of the other entities at which this version become effective
+	times    map[string]lamport.Time
+	unixTime int64
+
 	// The set of keys valid at that time, from this version onward, until they get removed
 	// in a new version. This allow to have multiple key for the same identity (e.g. one per
 	// device) as well as revoke key.
 	keys []*Key
 
-	// This optional array is here to ensure a better randomness of the identity id to avoid collisions.
+	// mandatory random bytes to ensure a better randomness of the data of the first
+	// version of an identity, used to later generate the ID
+	// len(Nonce) should be > 20 and < 64 bytes
 	// It has no functional purpose and should be ignored.
-	// It is advised to fill this array if there is not enough entropy, e.g. if there is no keys.
+	// TODO: optional after first version?
 	nonce []byte
 
 	// A set of arbitrary key/value to store metadata about a version or about an Identity in general.
 	metadata map[string]string
 
+	// Not serialized. Store the version's id in memory.
+	id entity.Id
 	// Not serialized
 	commitHash repository.Hash
 }
 
-type VersionJSON struct {
+func newVersion(repo repository.RepoClock, name string, email string, login string, avatarURL string, keys []*Key) (*version, error) {
+	clocks, err := repo.AllClocks()
+	if err != nil {
+		return nil, err
+	}
+
+	times := make(map[string]lamport.Time)
+	for name, clock := range clocks {
+		times[name] = clock.Time()
+	}
+
+	return &version{
+		id:        entity.UnsetId,
+		name:      name,
+		email:     email,
+		login:     login,
+		avatarURL: avatarURL,
+		times:     times,
+		unixTime:  time.Now().Unix(),
+		keys:      keys,
+		nonce:     makeNonce(20),
+	}, nil
+}
+
+type versionJSON struct {
 	// Additional field to version the data
 	FormatVersion uint `json:"version"`
 
-	Time      lamport.Time      `json:"time"`
-	UnixTime  int64             `json:"unix_time"`
-	Name      string            `json:"name,omitempty"`
-	Email     string            `json:"email,omitempty"`
-	Login     string            `json:"login,omitempty"`
-	AvatarUrl string            `json:"avatar_url,omitempty"`
-	Keys      []*Key            `json:"pub_keys,omitempty"`
-	Nonce     []byte            `json:"nonce,omitempty"`
-	Metadata  map[string]string `json:"metadata,omitempty"`
+	Times     map[string]lamport.Time `json:"times"`
+	UnixTime  int64                   `json:"unix_time"`
+	Name      string                  `json:"name,omitempty"`
+	Email     string                  `json:"email,omitempty"`
+	Login     string                  `json:"login,omitempty"`
+	AvatarUrl string                  `json:"avatar_url,omitempty"`
+	Keys      []*Key                  `json:"pub_keys,omitempty"`
+	Nonce     []byte                  `json:"nonce"`
+	Metadata  map[string]string       `json:"metadata,omitempty"`
+}
+
+// Id return the identifier of the version
+func (v *version) Id() entity.Id {
+	if v.id == "" {
+		// something went really wrong
+		panic("version's id not set")
+	}
+	if v.id == entity.UnsetId {
+		// This means we are trying to get the version's Id *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.
+		data, err := json.Marshal(v)
+		if err != nil {
+			panic(err)
+		}
+		v.id = entity.DeriveId(data)
+	}
+	return v.id
 }
 
 // Make a deep copy
-func (v *Version) Clone() *Version {
-	clone := &Version{
-		name:      v.name,
-		email:     v.email,
-		avatarURL: v.avatarURL,
-		keys:      make([]*Key, len(v.keys)),
+func (v *version) Clone() *version {
+	// copy direct fields
+	clone := *v
+
+	// reset some fields
+	clone.commitHash = ""
+	clone.id = entity.UnsetId
+
+	clone.times = make(map[string]lamport.Time)
+	for name, t := range v.times {
+		clone.times[name] = t
 	}
 
+	clone.keys = make([]*Key, len(v.keys))
 	for i, key := range v.keys {
 		clone.keys[i] = key.Clone()
 	}
 
-	return clone
+	clone.nonce = make([]byte, len(v.nonce))
+	copy(clone.nonce, v.nonce)
+
+	// not copying metadata
+
+	return &clone
 }
 
-func (v *Version) MarshalJSON() ([]byte, error) {
-	return json.Marshal(VersionJSON{
+func (v *version) MarshalJSON() ([]byte, error) {
+	return json.Marshal(versionJSON{
 		FormatVersion: formatVersion,
-		Time:          v.time,
+		Times:         v.times,
 		UnixTime:      v.unixTime,
 		Name:          v.name,
 		Email:         v.email,
@@ -96,21 +152,19 @@ func (v *Version) MarshalJSON() ([]byte, error) {
 	})
 }
 
-func (v *Version) UnmarshalJSON(data []byte) error {
-	var aux VersionJSON
+func (v *version) UnmarshalJSON(data []byte) error {
+	var aux versionJSON
 
 	if err := json.Unmarshal(data, &aux); err != nil {
 		return err
 	}
 
-	if aux.FormatVersion < formatVersion {
-		return entity.NewErrOldFormatVersion(aux.FormatVersion)
-	}
-	if aux.FormatVersion > formatVersion {
-		return entity.NewErrNewFormatVersion(aux.FormatVersion)
+	if aux.FormatVersion != formatVersion {
+		return entity.NewErrInvalidFormat(aux.FormatVersion, formatVersion)
 	}
 
-	v.time = aux.Time
+	v.id = entity.DeriveId(data)
+	v.times = aux.Times
 	v.unixTime = aux.UnixTime
 	v.name = aux.Name
 	v.email = aux.Email
@@ -123,23 +177,18 @@ func (v *Version) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
-func (v *Version) Validate() error {
+func (v *version) Validate() error {
 	// time must be set after a commit
 	if v.commitHash != "" && v.unixTime == 0 {
 		return fmt.Errorf("unix time not set")
 	}
-	if v.commitHash != "" && v.time == 0 {
-		return fmt.Errorf("lamport time not set")
-	}
 
 	if text.Empty(v.name) && text.Empty(v.login) {
 		return fmt.Errorf("either name or login should be set")
 	}
-
 	if strings.Contains(v.name, "\n") {
 		return fmt.Errorf("name should be a single line")
 	}
-
 	if !text.Safe(v.name) {
 		return fmt.Errorf("name is not fully printable")
 	}
@@ -147,7 +196,6 @@ func (v *Version) Validate() error {
 	if strings.Contains(v.login, "\n") {
 		return fmt.Errorf("login should be a single line")
 	}
-
 	if !text.Safe(v.login) {
 		return fmt.Errorf("login is not fully printable")
 	}
@@ -155,7 +203,6 @@ func (v *Version) Validate() error {
 	if strings.Contains(v.email, "\n") {
 		return fmt.Errorf("email should be a single line")
 	}
-
 	if !text.Safe(v.email) {
 		return fmt.Errorf("email is not fully printable")
 	}
@@ -167,6 +214,9 @@ func (v *Version) Validate() error {
 	if len(v.nonce) > 64 {
 		return fmt.Errorf("nonce is too big")
 	}
+	if len(v.nonce) < 20 {
+		return fmt.Errorf("nonce is too small")
+	}
 
 	for _, k := range v.keys {
 		if err := k.Validate(); err != nil {
@@ -177,9 +227,9 @@ func (v *Version) Validate() error {
 	return nil
 }
 
-// Write will serialize and store the Version as a git blob and return
+// Write will serialize and store the version as a git blob and return
 // its hash
-func (v *Version) Write(repo repository.Repo) (repository.Hash, error) {
+func (v *version) Write(repo repository.Repo) (repository.Hash, error) {
 	// make sure we don't write invalid data
 	err := v.Validate()
 	if err != nil {
@@ -187,17 +237,18 @@ func (v *Version) Write(repo repository.Repo) (repository.Hash, error) {
 	}
 
 	data, err := json.Marshal(v)
-
 	if err != nil {
 		return "", err
 	}
 
 	hash, err := repo.StoreData(data)
-
 	if err != nil {
 		return "", err
 	}
 
+	// make sure we set the Id when writing in the repo
+	v.id = entity.DeriveId(data)
+
 	return hash, nil
 }
 
@@ -211,22 +262,22 @@ func makeNonce(len int) []byte {
 }
 
 // SetMetadata store arbitrary metadata about a version or an Identity in general
-// If the Version has been commit to git already, it won't be overwritten.
-func (v *Version) SetMetadata(key string, value string) {
+// If the version has been commit to git already, it won't be overwritten.
+// Beware: changing the metadata on a version will change it's ID
+func (v *version) SetMetadata(key string, value string) {
 	if v.metadata == nil {
 		v.metadata = make(map[string]string)
 	}
-
 	v.metadata[key] = value
 }
 
-// GetMetadata retrieve arbitrary metadata about the Version
-func (v *Version) GetMetadata(key string) (string, bool) {
+// GetMetadata retrieve arbitrary metadata about the version
+func (v *version) GetMetadata(key string) (string, bool) {
 	val, ok := v.metadata[key]
 	return val, ok
 }
 
-// AllMetadata return all metadata for this Version
-func (v *Version) AllMetadata() map[string]string {
+// AllMetadata return all metadata for this version
+func (v *version) AllMetadata() map[string]string {
 	return v.metadata
 }
diff --git a/identity/version_test.go b/identity/version_test.go
index 25848eb5..385ad4d7 100644
--- a/identity/version_test.go
+++ b/identity/version_test.go
@@ -3,39 +3,76 @@ package identity
 import (
 	"encoding/json"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/MichaelMure/git-bug/entity"
+	"github.com/MichaelMure/git-bug/repository"
+	"github.com/MichaelMure/git-bug/util/lamport"
 )
 
-func TestVersionSerialize(t *testing.T) {
-	before := &Version{
+func makeIdentityTestRepo(t *testing.T) repository.ClockedRepo {
+	repo := repository.NewMockRepo()
+
+	clock1, err := repo.GetOrCreateClock("foo")
+	require.NoError(t, err)
+	err = clock1.Witness(42)
+	require.NoError(t, err)
+
+	clock2, err := repo.GetOrCreateClock("bar")
+	require.NoError(t, err)
+	err = clock2.Witness(34)
+	require.NoError(t, err)
+
+	return repo
+}
+
+func TestVersionJSON(t *testing.T) {
+	repo := makeIdentityTestRepo(t)
+
+	keys := []*Key{
+		generatePublicKey(),
+		generatePublicKey(),
+	}
+
+	before, err := newVersion(repo, "name", "email", "login", "avatarUrl", keys)
+	require.NoError(t, err)
+
+	before.SetMetadata("key1", "value1")
+	before.SetMetadata("key2", "value2")
+
+	expected := &version{
+		id:        entity.UnsetId,
 		name:      "name",
 		email:     "email",
+		login:     "login",
 		avatarURL: "avatarUrl",
-		keys: []*Key{
-			{
-				Fingerprint: "fingerprint1",
-				PubKey:      "pubkey1",
-			},
-			{
-				Fingerprint: "fingerprint2",
-				PubKey:      "pubkey2",
-			},
+		unixTime:  time.Now().Unix(),
+		times: map[string]lamport.Time{
+			"foo": 42,
+			"bar": 34,
 		},
-		nonce: makeNonce(20),
+		keys:  keys,
+		nonce: before.nonce,
 		metadata: map[string]string{
 			"key1": "value1",
 			"key2": "value2",
 		},
-		time: 3,
 	}
 
+	require.Equal(t, expected, before)
+
 	data, err := json.Marshal(before)
 	assert.NoError(t, err)
 
-	var after Version
+	var after version
 	err = json.Unmarshal(data, &after)
 	assert.NoError(t, err)
 
-	assert.Equal(t, before, &after)
+	// make sure we now have an Id
+	expected.Id()
+
+	assert.Equal(t, expected, &after)
 }