diff options
author | Michael Muré <batolettre@gmail.com> | 2020-11-08 19:13:55 +0100 |
---|---|---|
committer | Michael Muré <batolettre@gmail.com> | 2021-02-14 12:17:17 +0100 |
commit | 5ae8a132772385c903a62de2ceec02a97f108a01 (patch) | |
tree | 6ab3c3d460a830a1e459633f29ba989ca580b76f | |
parent | fb0c5fd06184f33a03d8d4fb29a3aef8b1dafe78 (diff) | |
download | git-bug-5ae8a132772385c903a62de2ceec02a97f108a01.tar.gz |
identity: Id from data, not git + hold multiple lamport clocks
-rw-r--r-- | go.sum | 18 | ||||
-rw-r--r-- | identity/identity.go | 270 | ||||
-rw-r--r-- | identity/identity_actions_test.go | 36 | ||||
-rw-r--r-- | identity/identity_stub.go | 25 | ||||
-rw-r--r-- | identity/identity_test.go | 205 | ||||
-rw-r--r-- | identity/interface.go | 24 | ||||
-rw-r--r-- | identity/version.go | 169 | ||||
-rw-r--r-- | identity/version_test.go | 71 |
8 files changed, 432 insertions, 386 deletions
@@ -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= diff --git a/identity/identity.go b/identity/identity.go index 8182e263..6352212d 100644 --- a/identity/identity.go +++ b/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) -} diff --git a/identity/identity_actions_test.go b/identity/identity_actions_test.go index 773574c6..63f6aacd 100644 --- a/identity/identity_actions_test.go +++ b/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) diff --git a/identity/identity_stub.go b/identity/identity_stub.go index f4bf1d37..fec92010 100644 --- a/identity/identity_stub.go +++ b/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()") } diff --git a/identity/identity_test.go b/identity/identity_test.go index dc5925d9..36d07be6 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.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) diff --git a/identity/interface.go b/identity/interface.go index a7174962..92a03c51 100644 --- a/identity/interface.go +++ b/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 diff --git a/identity/version.go b/identity/version.go index bbf93575..bbf0a3f5 100644 --- a/identity/version.go +++ b/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 } diff --git a/identity/version_test.go b/identity/version_test.go index 25848eb5..1efa0d03 100644 --- a/identity/version_test.go +++ b/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) } |