identity: more cleaning and fixes after a code review

8 files changed, 104 insertions, 75 deletions

diff --git a/identity/bare.go b/identity/bare.go
index 729dc2e0..5aaa0166 100644
--- a/identity/bare.go
+++ b/identity/bare.go
@@ -34,7 +34,7 @@ func NewBareFull(name string, email string, login string, avatarUrl string) *Bar
 	return &Bare{name: name, email: email, login: login, avatarUrl: avatarUrl}
 }
 
-type bareIdentityJson struct {
+type bareIdentityJSON struct {
 	Name      string `json:"name,omitempty"`
 	Email     string `json:"email,omitempty"`
 	Login     string `json:"login,omitempty"`
@@ -42,7 +42,7 @@ type bareIdentityJson struct {
 }
 
 func (i *Bare) MarshalJSON() ([]byte, error) {
-	return json.Marshal(bareIdentityJson{
+	return json.Marshal(bareIdentityJSON{
 		Name:      i.name,
 		Email:     i.email,
 		Login:     i.login,
@@ -51,7 +51,7 @@ func (i *Bare) MarshalJSON() ([]byte, error) {
 }
 
 func (i *Bare) UnmarshalJSON(data []byte) error {
-	aux := bareIdentityJson{}
+	aux := bareIdentityJSON{}
 
 	if err := json.Unmarshal(data, &aux); err != nil {
 		return err
@@ -125,14 +125,6 @@ func (i *Bare) DisplayName() string {
 	panic("invalid person data")
 }
 
-// Match tell is the Person match the given query string
-func (i *Bare) Match(query string) bool {
-	query = strings.ToLower(query)
-
-	return strings.Contains(strings.ToLower(i.name), query) ||
-		strings.Contains(strings.ToLower(i.login), query)
-}
-
 // Validate check if the Identity data is valid
 func (i *Bare) Validate() error {
 	if text.Empty(i.name) && text.Empty(i.login) {
diff --git a/identity/bare_test.go b/identity/bare_test.go
index 1458107a..4b28c7ad 100644
--- a/identity/bare_test.go
+++ b/identity/bare_test.go
@@ -3,7 +3,7 @@ package identity
 import (
 	"testing"
 
-	"github.com/magiconair/properties/assert"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestBare_Id(t *testing.T) {
diff --git a/identity/common.go b/identity/common.go
index 32dd3d9e..5301471a 100644
--- a/identity/common.go
+++ b/identity/common.go
@@ -51,3 +51,7 @@ func UnmarshalJSON(raw json.RawMessage) (Interface, error) {
 
 	return nil, fmt.Errorf("unknown identity type")
 }
+
+type Resolver interface {
+	ResolveIdentity(id string) (Interface, error)
+}
diff --git a/identity/identity.go b/identity/identity.go
index 38729e37..2a422789 100644
--- a/identity/identity.go
+++ b/identity/identity.go
@@ -4,7 +4,6 @@ package identity
 import (
 	"encoding/json"
 	"fmt"
-	"strings"
 
 	"github.com/MichaelMure/git-bug/repository"
 	"github.com/MichaelMure/git-bug/util/git"
@@ -49,13 +48,13 @@ func NewIdentityFull(name string, email string, login string, avatarUrl string)
 	}
 }
 
-type identityJson struct {
+type identityJSON struct {
 	Id string `json:"id"`
 }
 
 // MarshalJSON will only serialize the id
 func (i *Identity) MarshalJSON() ([]byte, error) {
-	return json.Marshal(identityJson{
+	return json.Marshal(identityJSON{
 		Id: i.Id(),
 	})
 }
@@ -64,7 +63,7 @@ func (i *Identity) MarshalJSON() ([]byte, error) {
 // Users of this package are expected to run Load() to load
 // the remaining data from the identities data in git.
 func (i *Identity) UnmarshalJSON(data []byte) error {
-	aux := identityJson{}
+	aux := identityJSON{}
 
 	if err := json.Unmarshal(data, &aux); err != nil {
 		return err
@@ -164,35 +163,13 @@ func NewFromGitUser(repo repository.Repo) (*Identity, error) {
 	return NewIdentity(name, email), nil
 }
 
-// BuildFromGit will query the repository for user detail and
-// build the corresponding Identity
-/*func BuildFromGit(repo repository.Repo) *Identity {
-	version := Version{}
-
-	name, err := repo.GetUserName()
-	if err == nil {
-		version.Name = name
-	}
-
-	email, err := repo.GetUserEmail()
-	if err == nil {
-		version.Email = email
-	}
-
-	return &Identity{
-		Versions: []Version{
-			version,
-		},
-	}
-}*/
-
-// SetIdentity store the user identity's id in the git config
-func SetIdentity(repo repository.RepoCommon, identity Identity) error {
+// SetUserIdentity store the user identity's id in the git config
+func SetUserIdentity(repo repository.RepoCommon, identity Identity) error {
 	return repo.StoreConfig(identityConfigKey, identity.Id())
 }
 
-// GetIdentity read the current user identity, set with a git config entry
-func GetIdentity(repo repository.Repo) (*Identity, error) {
+// GetUserIdentity read the current user identity, set with a git config entry
+func GetUserIdentity(repo repository.Repo) (*Identity, error) {
 	configs, err := repo.ReadConfigs(identityConfigKey)
 	if err != nil {
 		return nil, err
@@ -299,14 +276,6 @@ func (i *Identity) Validate() error {
 	return nil
 }
 
-func (i *Identity) firstVersion() *Version {
-	if len(i.Versions) <= 0 {
-		panic("no version at all")
-	}
-
-	return i.Versions[0]
-}
-
 func (i *Identity) lastVersion() *Version {
 	if len(i.Versions) <= 0 {
 		panic("no version at all")
@@ -372,14 +341,6 @@ func (i *Identity) ValidKeysAtTime(time lamport.Time) []Key {
 	return result
 }
 
-// Match tell is the Identity match the given query string
-func (i *Identity) Match(query string) bool {
-	query = strings.ToLower(query)
-
-	return strings.Contains(strings.ToLower(i.Name()), query) ||
-		strings.Contains(strings.ToLower(i.Login()), query)
-}
-
 // DisplayName return a non-empty string to display, representing the
 // identity, based on the non-empty values.
 func (i *Identity) DisplayName() string {
diff --git a/identity/identity_test.go b/identity/identity_test.go
index 914126be..afb804fc 100644
--- a/identity/identity_test.go
+++ b/identity/identity_test.go
@@ -8,12 +8,13 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// Test the commit and load of an Identity with multiple versions
 func TestIdentityCommitLoad(t *testing.T) {
 	mockRepo := repository.NewMockRepoForTest()
 
 	// single version
 
-	identity := Identity{
+	identity := &Identity{
 		Versions: []*Version{
 			{
 				Name:  "René Descartes",
@@ -30,11 +31,11 @@ func TestIdentityCommitLoad(t *testing.T) {
 	loaded, err := Read(mockRepo, identity.id)
 	assert.Nil(t, err)
 	commitsAreSet(t, loaded)
-	equivalentIdentity(t, &identity, loaded)
+	equivalentIdentity(t, identity, loaded)
 
 	// multiple version
 
-	identity = Identity{
+	identity = &Identity{
 		Versions: []*Version{
 			{
 				Time:  100,
@@ -71,7 +72,7 @@ func TestIdentityCommitLoad(t *testing.T) {
 	loaded, err = Read(mockRepo, identity.id)
 	assert.Nil(t, err)
 	commitsAreSet(t, loaded)
-	equivalentIdentity(t, &identity, loaded)
+	equivalentIdentity(t, identity, loaded)
 
 	// add more version
 
@@ -101,7 +102,7 @@ func TestIdentityCommitLoad(t *testing.T) {
 	loaded, err = Read(mockRepo, identity.id)
 	assert.Nil(t, err)
 	commitsAreSet(t, loaded)
-	equivalentIdentity(t, &identity, loaded)
+	equivalentIdentity(t, identity, loaded)
 }
 
 func commitsAreSet(t *testing.T, identity *Identity) {
@@ -120,6 +121,7 @@ func equivalentIdentity(t *testing.T, expected, actual *Identity) {
 	assert.Equal(t, expected, actual)
 }
 
+// Test that the correct crypto keys are returned for a given lamport time
 func TestIdentity_ValidKeysAtTime(t *testing.T) {
 	identity := Identity{
 		Versions: []*Version{
@@ -176,6 +178,7 @@ func TestIdentity_ValidKeysAtTime(t *testing.T) {
 	assert.Equal(t, identity.ValidKeysAtTime(3000), []Key{{PubKey: "pubkeyE"}})
 }
 
+// Test the immutable or mutable metadata search
 func TestMetadata(t *testing.T) {
 	mockRepo := repository.NewMockRepoForTest()
 
diff --git a/identity/interface.go b/identity/interface.go
index c784a7a6..1d534eb8 100644
--- a/identity/interface.go
+++ b/identity/interface.go
@@ -23,9 +23,6 @@ type Interface interface {
 	// identity, based on the non-empty values.
 	DisplayName() string
 
-	// Match tell is the Person match the given query string
-	Match(query string) bool
-
 	// Validate check if the Identity data is valid
 	Validate() error
 
diff --git a/identity/key.go b/identity/key.go
index c498ec09..90edfb60 100644
--- a/identity/key.go
+++ b/identity/key.go
@@ -5,3 +5,9 @@ type Key struct {
 	Fingerprint string `json:"fingerprint"`
 	PubKey      string `json:"pub_key"`
 }
+
+func (k *Key) Validate() error {
+	// Todo
+
+	return nil
+}
diff --git a/identity/version.go b/identity/version.go
index bc4561d9..d4afc893 100644
--- a/identity/version.go
+++ b/identity/version.go
@@ -10,34 +10,88 @@ import (
 	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/lamport"
 	"github.com/MichaelMure/git-bug/util/text"
+	"github.com/pkg/errors"
 )
 
+const formatVersion = 1
+
 // Version is a complete set of information about an Identity at a point in time.
 type Version struct {
-	// Private field so not serialized
+	// Not serialized
 	commitHash git.Hash
 
 	// The lamport time at which this version become effective
 	// The reference time is the bug edition lamport clock
-	Time lamport.Time `json:"time"`
+	Time lamport.Time
 
-	Name      string `json:"name"`
-	Email     string `json:"email"`
-	Login     string `json:"login"`
-	AvatarUrl string `json:"avatar_url"`
+	Name      string
+	Email     string
+	Login     string
+	AvatarUrl string
 
 	// 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 `json:"pub_keys"`
+	Keys []Key
 
 	// This optional array is here to ensure a better randomness of the identity id to avoid collisions.
 	// 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.
-	Nonce []byte `json:"nonce,omitempty"`
+	Nonce []byte
 
 	// A set of arbitrary key/value to store metadata about a version or about an Identity in general.
-	Metadata map[string]string `json:"metadata,omitempty"`
+	Metadata map[string]string
+}
+
+type VersionJSON struct {
+	// Additional field to version the data
+	FormatVersion uint `json:"version"`
+
+	Time      lamport.Time      `json:"time"`
+	Name      string            `json:"name"`
+	Email     string            `json:"email"`
+	Login     string            `json:"login"`
+	AvatarUrl string            `json:"avatar_url"`
+	Keys      []Key             `json:"pub_keys"`
+	Nonce     []byte            `json:"nonce,omitempty"`
+	Metadata  map[string]string `json:"metadata,omitempty"`
+}
+
+func (v *Version) MarshalJSON() ([]byte, error) {
+	return json.Marshal(VersionJSON{
+		FormatVersion: formatVersion,
+		Time:          v.Time,
+		Name:          v.Name,
+		Email:         v.Email,
+		Login:         v.Login,
+		AvatarUrl:     v.AvatarUrl,
+		Keys:          v.Keys,
+		Nonce:         v.Nonce,
+		Metadata:      v.Metadata,
+	})
+}
+
+func (v *Version) UnmarshalJSON(data []byte) error {
+	var aux VersionJSON
+
+	if err := json.Unmarshal(data, &aux); err != nil {
+		return err
+	}
+
+	if aux.FormatVersion != formatVersion {
+		return fmt.Errorf("unknown format version %v", aux.FormatVersion)
+	}
+
+	v.Time = aux.Time
+	v.Name = aux.Name
+	v.Email = aux.Email
+	v.Login = aux.Login
+	v.AvatarUrl = aux.AvatarUrl
+	v.Keys = aux.Keys
+	v.Nonce = aux.Nonce
+	v.Metadata = aux.Metadata
+
+	return nil
 }
 
 func (v *Version) Validate() error {
@@ -77,12 +131,24 @@ func (v *Version) Validate() error {
 		return fmt.Errorf("nonce is too big")
 	}
 
+	for _, k := range v.Keys {
+		if err := k.Validate(); err != nil {
+			return errors.Wrap(err, "invalid key")
+		}
+	}
+
 	return nil
 }
 
 // Write will serialize and store the Version as a git blob and return
 // its hash
 func (v *Version) Write(repo repository.Repo) (git.Hash, error) {
+	// make sure we don't write invalid data
+	err := v.Validate()
+	if err != nil {
+		return "", errors.Wrap(err, "validation error")
+	}
+
 	data, err := json.Marshal(v)
 
 	if err != nil {