identity: Id from data, not git + hold multiple lamport clocks

Michael Muré created

Change summary

go.sum                            |  18 +
identity/identity.go              | 270 ++++++++++++++------------------
identity/identity_actions_test.go |  36 ++-
identity/identity_stub.go         |  25 --
identity/identity_test.go         | 205 ++++++++++--------------
identity/interface.go             |  24 +-
identity/version.go               | 169 +++++++++++++------
identity/version_test.go          |  71 ++++++-
8 files changed, 432 insertions(+), 386 deletions(-)

Detailed changes

go.sum đź”—

@@ -101,6 +101,8 @@ github.com/blevesearch/zap/v11 v11.0.12/go.mod h1:JLfFhc8DWP01zMG/6VwEY2eAnlJsTN
 github.com/blevesearch/zap/v11 v11.0.13 h1:NDvmjAyeEQsBbPElubVPqrBtSDOftXYwxkHeZfflU4A=
 github.com/blevesearch/zap/v11 v11.0.13/go.mod h1:qKkNigeXbxZwym02wsxoQpbme1DgAwTvRlT/beIGfTM=
 github.com/blevesearch/zap/v11 v11.0.14 h1:IrDAvtlzDylh6H2QCmS0OGcN9Hpf6mISJlfKjcwJs7k=
+github.com/blevesearch/zap/v11 v11.0.14 h1:IrDAvtlzDylh6H2QCmS0OGcN9Hpf6mISJlfKjcwJs7k=
+github.com/blevesearch/zap/v11 v11.0.14/go.mod h1:MUEZh6VHGXv1PKx3WnCbdP404LGG2IZVa/L66pyFwnY=
 github.com/blevesearch/zap/v11 v11.0.14/go.mod h1:MUEZh6VHGXv1PKx3WnCbdP404LGG2IZVa/L66pyFwnY=
 github.com/blevesearch/zap/v12 v12.0.10 h1:T1/GXNBxC9eetfuMwCM5RLWXeharSMyAdNEdXVtBuHA=
 github.com/blevesearch/zap/v12 v12.0.10/go.mod h1:QtKkjpmV/sVFEnKSaIWPXZJAaekL97TrTV3ImhNx+nw=
@@ -108,6 +110,8 @@ github.com/blevesearch/zap/v12 v12.0.12/go.mod h1:1HrB4hhPfI8u8x4SPYbluhb8xhflpP
 github.com/blevesearch/zap/v12 v12.0.13 h1:05Ebdmv2tRTUytypG4DlOIHLLw995DtVV0Zl3YwwDew=
 github.com/blevesearch/zap/v12 v12.0.13/go.mod h1:0RTeU1uiLqsPoybUn6G/Zgy6ntyFySL3uWg89NgX3WU=
 github.com/blevesearch/zap/v12 v12.0.14 h1:2o9iRtl1xaRjsJ1xcqTyLX414qPAwykHNV7wNVmbp3w=
+github.com/blevesearch/zap/v12 v12.0.14 h1:2o9iRtl1xaRjsJ1xcqTyLX414qPAwykHNV7wNVmbp3w=
+github.com/blevesearch/zap/v12 v12.0.14/go.mod h1:rOnuZOiMKPQj18AEKEHJxuI14236tTQ1ZJz4PAnWlUg=
 github.com/blevesearch/zap/v12 v12.0.14/go.mod h1:rOnuZOiMKPQj18AEKEHJxuI14236tTQ1ZJz4PAnWlUg=
 github.com/blevesearch/zap/v13 v13.0.2 h1:quhI5OVFX33dhPpUW+nLyXGpu7QT8qTgzu6qA/fRRXM=
 github.com/blevesearch/zap/v13 v13.0.2/go.mod h1:/9QLKla8/8mloJvQQutPhB+tw6y35urvKeAFeun2JGA=
@@ -115,6 +119,8 @@ github.com/blevesearch/zap/v13 v13.0.4/go.mod h1:YdB7UuG7TBWu/1dz9e2SaLp1RKfFfdJ
 github.com/blevesearch/zap/v13 v13.0.5 h1:+Gcwl95uei3MgBlJAddBFRv9gl+FMNcXpMa7BX3byJw=
 github.com/blevesearch/zap/v13 v13.0.5/go.mod h1:HTfWECmzBN7BbdBxdEigpUsD6MOPFOO84tZ0z/g3CnE=
 github.com/blevesearch/zap/v13 v13.0.6 h1:r+VNSVImi9cBhTNNR+Kfl5uiGy8kIbb0JMz/h8r6+O4=
+github.com/blevesearch/zap/v13 v13.0.6 h1:r+VNSVImi9cBhTNNR+Kfl5uiGy8kIbb0JMz/h8r6+O4=
+github.com/blevesearch/zap/v13 v13.0.6/go.mod h1:L89gsjdRKGyGrRN6nCpIScCvvkyxvmeDCwZRcjjPCrw=
 github.com/blevesearch/zap/v13 v13.0.6/go.mod h1:L89gsjdRKGyGrRN6nCpIScCvvkyxvmeDCwZRcjjPCrw=
 github.com/blevesearch/zap/v14 v14.0.1 h1:s8KeqX53Vc4eRaziHsnY2bYUE+8IktWqRL9W5H5VDMY=
 github.com/blevesearch/zap/v14 v14.0.1/go.mod h1:Y+tUL9TypMca5+96m7iJb2lpcntETXSeDoI5BBX2tvY=
@@ -122,18 +128,12 @@ github.com/blevesearch/zap/v14 v14.0.3/go.mod h1:oObAhcDHw7p1ahiTCqhRkdxdl7UA8qp
 github.com/blevesearch/zap/v14 v14.0.4 h1:BnWWkdgmPhK50J9dkBlQrWB4UDa22OMPIUzn1oXcXfY=
 github.com/blevesearch/zap/v14 v14.0.4/go.mod h1:sTwuFoe1n/+VtaHNAjY3W5GzHZ5UxFkw1MZ82P/WKpA=
 github.com/blevesearch/zap/v14 v14.0.5 h1:NdcT+81Nvmp2zL+NhwSvGSLh7xNgGL8QRVZ67njR0NU=
+github.com/blevesearch/zap/v14 v14.0.5 h1:NdcT+81Nvmp2zL+NhwSvGSLh7xNgGL8QRVZ67njR0NU=
+github.com/blevesearch/zap/v14 v14.0.5/go.mod h1:bWe8S7tRrSBTIaZ6cLRbgNH4TUDaC9LZSpRGs85AsGY=
 github.com/blevesearch/zap/v14 v14.0.5/go.mod h1:bWe8S7tRrSBTIaZ6cLRbgNH4TUDaC9LZSpRGs85AsGY=
 github.com/blevesearch/zap/v15 v15.0.1/go.mod h1:ho0frqAex2ktT9cYFAxQpoQXsxb/KEfdjpx4s49rf/M=
 github.com/blevesearch/zap/v15 v15.0.2 h1:7wV4ksnKzBibLaWBolzbxngxdVAUmF7HJ+gMOqkzsdQ=
 github.com/blevesearch/zap/v15 v15.0.2/go.mod h1:nfycXPgfbio8l+ZSkXUkhSNjTpp57jZ0/MKa6TigWvM=
-github.com/blevesearch/zap/v11 v11.0.14 h1:IrDAvtlzDylh6H2QCmS0OGcN9Hpf6mISJlfKjcwJs7k=
-github.com/blevesearch/zap/v11 v11.0.14/go.mod h1:MUEZh6VHGXv1PKx3WnCbdP404LGG2IZVa/L66pyFwnY=
-github.com/blevesearch/zap/v12 v12.0.14 h1:2o9iRtl1xaRjsJ1xcqTyLX414qPAwykHNV7wNVmbp3w=
-github.com/blevesearch/zap/v12 v12.0.14/go.mod h1:rOnuZOiMKPQj18AEKEHJxuI14236tTQ1ZJz4PAnWlUg=
-github.com/blevesearch/zap/v13 v13.0.6 h1:r+VNSVImi9cBhTNNR+Kfl5uiGy8kIbb0JMz/h8r6+O4=
-github.com/blevesearch/zap/v13 v13.0.6/go.mod h1:L89gsjdRKGyGrRN6nCpIScCvvkyxvmeDCwZRcjjPCrw=
-github.com/blevesearch/zap/v14 v14.0.5 h1:NdcT+81Nvmp2zL+NhwSvGSLh7xNgGL8QRVZ67njR0NU=
-github.com/blevesearch/zap/v14 v14.0.5/go.mod h1:bWe8S7tRrSBTIaZ6cLRbgNH4TUDaC9LZSpRGs85AsGY=
 github.com/blevesearch/zap/v15 v15.0.3 h1:Ylj8Oe+mo0P25tr9iLPp33lN6d4qcztGjaIsP51UxaY=
 github.com/blevesearch/zap/v15 v15.0.3/go.mod h1:iuwQrImsh1WjWJ0Ue2kBqY83a0rFtJTqfa9fp1rbVVU=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
@@ -492,6 +492,7 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
 github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
 github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/syndtr/goleveldb v1.0.0 h1:fBdIW9lB4Iz0n9khmH8w27SJ3QEJ7+IgjPEwGSZiFdE=
@@ -674,6 +675,7 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.4 h1:0YWbFKbhXG/wIiuHDSKpS0Iy7FSA+u45VtBMfQcFTTc=
 golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/text v0.3.5 h1:i6eZZ+zk0SOf0xgBpEpPD18qWcJda6q1sxt3S0kzyUQ=
 golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=

identity/identity.go đź”—

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"reflect"
 	"strings"
-	"time"
 
 	"github.com/pkg/errors"
 
@@ -35,47 +34,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 +71,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(),
 	})
 }
 
@@ -131,28 +110,25 @@ func read(repo repository.Repo, ref string) (*Identity, error) {
 	}
 
 	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 +138,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 +270,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 +321,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 +345,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.StoreCommitWithParent(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 +402,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 +424,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 +435,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 +446,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 +476,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 +485,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
@@ -539,11 +521,18 @@ func (i *Identity) Keys() []*Key {
 }
 
 // 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 +542,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 +559,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 +591,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 +604,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)
-}

identity/identity_actions_test.go đź”—

@@ -12,8 +12,9 @@ func TestPushPull(t *testing.T) {
 	repoA, repoB, remote := repository.SetupReposAndRemote()
 	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)
 

identity/identity_stub.go đź”—

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 
 	"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"
 )
@@ -52,6 +51,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 +71,15 @@ func (IdentityStub) Keys() []*Key {
 	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
-func (IdentityStub) ValidKeysAtTime(_ lamport.Time) []*Key {
-	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) Validate() error {
+func (IdentityStub) ValidKeysAtTime(_ string, _ lamport.Time) []*Key {
 	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 +87,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()")
 }
 

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.NewMockRepo()
+	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{{PubKey: "pubkeyA"}})
+	require.NoError(t, err)
 
-	err = identity.Commit(mockRepo)
+	idBeforeCommit = identity.Id()
 
+	err = identity.Mutate(repo, func(orig *Mutator) {
+		orig.Keys = []*Key{{PubKey: "pubkeyB"}}
+	})
 	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{{PubKey: "pubkeyC"}}
+	})
+	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{{PubKey: "pubkeyD"}}
 	})
+	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{{PubKey: "pubkeyD"}, {PubKey: "pubkeyE"}}
 	})
+	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")
@@ -136,44 +124,33 @@ 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) {
 	identity := Identity{
-		id: entity.UnsetId,
-		versions: []*Version{
+		versions: []*version{
 			{
-				time:  100,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
+				times: map[string]lamport.Time{"foo": 100},
 				keys: []*Key{
 					{PubKey: "pubkeyA"},
 				},
 			},
 			{
-				time:  200,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
+				times: map[string]lamport.Time{"foo": 200},
 				keys: []*Key{
 					{PubKey: "pubkeyB"},
 				},
 			},
 			{
-				time:  201,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
+				times: map[string]lamport.Time{"foo": 201},
 				keys: []*Key{
 					{PubKey: "pubkeyC"},
 				},
 			},
 			{
-				time:  201,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
+				times: map[string]lamport.Time{"foo": 201},
 				keys: []*Key{
 					{PubKey: "pubkeyD"},
 				},
 			},
 			{
-				time:  300,
-				name:  "René Descartes",
-				email: "rene.descartes@example.com",
+				times: map[string]lamport.Time{"foo": 300},
 				keys: []*Key{
 					{PubKey: "pubkeyE"},
 				},
@@ -181,47 +158,48 @@ func TestIdentity_ValidKeysAtTime(t *testing.T) {
 		},
 	}
 
-	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{{PubKey: "pubkeyA"}})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 140), []*Key{{PubKey: "pubkeyA"}})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 200), []*Key{{PubKey: "pubkeyB"}})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 201), []*Key{{PubKey: "pubkeyD"}})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 202), []*Key{{PubKey: "pubkeyD"}})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 300), []*Key{{PubKey: "pubkeyE"}})
+	require.Equal(t, identity.ValidKeysAtTime("foo", 3000), []*Key{{PubKey: "pubkeyE"}})
 }
 
 // Test the immutable or mutable metadata search
 func TestMetadata(t *testing.T) {
-	mockRepo := repository.NewMockRepo()
+	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")
@@ -235,22 +213,15 @@ func assertHasKeyValue(t *testing.T, metadata map[string]string, key, value stri
 }
 
 func TestJSON(t *testing.T) {
-	mockRepo := repository.NewMockRepo()
+	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 +231,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 +251,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)
 

identity/interface.go đź”—

@@ -13,6 +13,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 +36,22 @@ type Interface interface {
 	// Can be empty.
 	Keys() []*Key
 
-	// ValidKeysAtTime return the set of keys valid at a given lamport time
+	// 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

identity/version.go đź”—

@@ -2,9 +2,11 @@ package identity
 
 import (
 	"crypto/rand"
+	"crypto/sha256"
 	"encoding/json"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/pkg/errors"
 
@@ -15,76 +17,133 @@ 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
+const formatVersion = 2
+
+// TODO ^^
 
+// 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 a bug, 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?
 	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 = deriveId(data)
+	}
+	return v.id
+}
+
+func deriveId(data []byte) entity.Id {
+	sum := sha256.Sum256(data)
+	return entity.Id(fmt.Sprintf("%x", sum))
 }
 
 // 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
+
+	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,8 +155,8 @@ 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
@@ -110,7 +169,8 @@ func (v *Version) UnmarshalJSON(data []byte) error {
 		return entity.NewErrNewFormatVersion(aux.FormatVersion)
 	}
 
-	v.time = aux.Time
+	v.id = deriveId(data)
+	v.times = aux.Times
 	v.unixTime = aux.UnixTime
 	v.name = aux.Name
 	v.email = aux.Email
@@ -123,23 +183,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 +202,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 +209,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 +220,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 +233,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 +243,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 = deriveId(data)
+
 	return hash, nil
 }
 
@@ -211,22 +268,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
 }

identity/version_test.go đź”—

@@ -3,39 +3,82 @@ 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 makeIdentityTestRepo(t *testing.T) repository.ClockedRepo {
+	repo := repository.NewMockRepo()
+
+	clock1, err := repo.GetOrCreateClock("foo")
+	require.NoError(t, err)
+	err = clock1.Witness(42) // clock goes to 43
+	require.NoError(t, err)
+
+	clock2, err := repo.GetOrCreateClock("bar")
+	require.NoError(t, err)
+	err = clock2.Witness(34) // clock goes to 35
+	require.NoError(t, err)
+
+	return repo
+}
+
 func TestVersionSerialize(t *testing.T) {
-	before := &Version{
+	repo := makeIdentityTestRepo(t)
+
+	keys := []*Key{
+		{
+			Fingerprint: "fingerprint1",
+			PubKey:      "pubkey1",
+		},
+		{
+			Fingerprint: "fingerprint2",
+			PubKey:      "pubkey2",
+		},
+	}
+
+	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": 43,
+			"bar": 35,
 		},
-		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)
 }