identity: add more test for serialisation and push/pull/merge + fixes

6 files changed, 302 insertions, 8 deletions

diff --git a/identity/bare_test.go b/identity/bare_test.go
index 4b28c7ad..7db9f644 100644
--- a/identity/bare_test.go
+++ b/identity/bare_test.go
@@ -1,6 +1,7 @@
 package identity
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -11,3 +12,21 @@ func TestBare_Id(t *testing.T) {
 	id := i.Id()
 	assert.Equal(t, "7b226e616d65223a226e616d65222c22", id)
 }
+
+func TestBareSerialize(t *testing.T) {
+	before := &Bare{
+		login:     "login",
+		email:     "email",
+		name:      "name",
+		avatarUrl: "avatar",
+	}
+
+	data, err := json.Marshal(before)
+	assert.NoError(t, err)
+
+	var after Bare
+	err = json.Unmarshal(data, &after)
+	assert.NoError(t, err)
+
+	assert.Equal(t, before, &after)
+}
diff --git a/identity/identity.go b/identity/identity.go
index 59973489..725362f9 100644
--- a/identity/identity.go
+++ b/identity/identity.go
@@ -18,6 +18,8 @@ const identityRemoteRefPattern = "refs/remotes/%s/identities/"
 const versionEntryName = "version"
 const identityConfigKey = "git-bug.identity"
 
+var ErrNonFastForwardMerge = errors.New("non fast-forward identity merge")
+
 var _ Interface = &Identity{}
 
 type Identity struct {
@@ -136,6 +138,50 @@ func read(repo repository.Repo, ref string) (*Identity, error) {
 	return i, nil
 }
 
+type StreamedIdentity struct {
+	Identity *Identity
+	Err      error
+}
+
+// ReadAllLocalIdentities read and parse all local Identity
+func ReadAllLocalIdentities(repo repository.ClockedRepo) <-chan StreamedIdentity {
+	return readAllIdentities(repo, identityRefPattern)
+}
+
+// ReadAllRemoteIdentities read and parse all remote Identity for a given remote
+func ReadAllRemoteIdentities(repo repository.ClockedRepo, remote string) <-chan StreamedIdentity {
+	refPrefix := fmt.Sprintf(identityRemoteRefPattern, remote)
+	return readAllIdentities(repo, refPrefix)
+}
+
+// Read and parse all available bug with a given ref prefix
+func readAllIdentities(repo repository.ClockedRepo, refPrefix string) <-chan StreamedIdentity {
+	out := make(chan StreamedIdentity)
+
+	go func() {
+		defer close(out)
+
+		refs, err := repo.ListRefs(refPrefix)
+		if err != nil {
+			out <- StreamedIdentity{Err: err}
+			return
+		}
+
+		for _, ref := range refs {
+			b, err := read(repo, ref)
+
+			if err != nil {
+				out <- StreamedIdentity{Err: err}
+				return
+			}
+
+			out <- StreamedIdentity{Identity: b}
+		}
+	}()
+
+	return out
+}
+
 // NewFromGitUser will query the repository for user detail and
 // build the corresponding Identity
 func NewFromGitUser(repo repository.Repo) (*Identity, error) {
@@ -195,6 +241,22 @@ func (i *Identity) AddVersion(version *Version) {
 func (i *Identity) Commit(repo repository.Repo) error {
 	// Todo: check for mismatch between memory and commited data
 
+	needCommit := false
+	for _, v := range i.versions {
+		if v.commitHash == "" {
+			needCommit = true
+			break
+		}
+	}
+
+	if !needCommit {
+		return fmt.Errorf("can't commit an identity with no pending version")
+	}
+
+	if err := i.Validate(); err != nil {
+		return errors.Wrap(err, "can't commit an identity with invalid data")
+	}
+
 	for _, v := range i.versions {
 		if v.commitHash != "" {
 			i.lastCommit = v.commitHash
@@ -299,7 +361,7 @@ func (i *Identity) Merge(repo repository.Repo, other *Identity) (bool, error) {
 		// we have a non fast-forward merge.
 		// as explained in the doc above, refusing to merge
 		if i.versions[j].commitHash != otherVersion.commitHash {
-			return false, errors.New("non fast-forward identity merge")
+			return false, ErrNonFastForwardMerge
 		}
 	}
 
diff --git a/identity/identity_actions.go b/identity/identity_actions.go
index da7a064c..53997eef 100644
--- a/identity/identity_actions.go
+++ b/identity/identity_actions.go
@@ -12,7 +12,7 @@ import (
 // This does not change the local identities state
 func Fetch(repo repository.Repo, remote string) (string, error) {
 	remoteRefSpec := fmt.Sprintf(identityRemoteRefPattern, remote)
-	fetchRefSpec := fmt.Sprintf("%s:%s*", identityRefPattern, remoteRefSpec)
+	fetchRefSpec := fmt.Sprintf("%s*:%s*", identityRefPattern, remoteRefSpec)
 
 	return repo.FetchRefs(remote, fetchRefSpec)
 }
@@ -23,8 +23,7 @@ func Push(repo repository.Repo, remote string) (string, error) {
 }
 
 // Pull will do a Fetch + MergeAll
-// This function won't give details on the underlying process. If you need more,
-// use Fetch and MergeAll separately.
+// This function will return an error if a merge fail
 func Pull(repo repository.ClockedRepo, remote string) error {
 	_, err := Fetch(repo, remote)
 	if err != nil {
@@ -36,9 +35,7 @@ func Pull(repo repository.ClockedRepo, remote string) error {
 			return merge.Err
 		}
 		if merge.Status == MergeStatusInvalid {
-			// Not awesome: simply output the merge failure here as this function
-			// is only used in tests for now.
-			fmt.Println(merge)
+			return errors.Errorf("merge failure: %s", merge.Reason)
 		}
 	}
 
@@ -64,7 +61,7 @@ func MergeAll(repo repository.ClockedRepo, remote string) <-chan MergeResult {
 			refSplitted := strings.Split(remoteRef, "/")
 			id := refSplitted[len(refSplitted)-1]
 
-			remoteIdentity, err := ReadLocal(repo, remoteRef)
+			remoteIdentity, err := read(repo, remoteRef)
 
 			if err != nil {
 				out <- newMergeInvalidStatus(id, errors.Wrap(err, "remote identity is not readable").Error())
diff --git a/identity/identity_actions_test.go b/identity/identity_actions_test.go
new file mode 100644
index 00000000..42563374
--- /dev/null
+++ b/identity/identity_actions_test.go
@@ -0,0 +1,151 @@
+package identity
+
+import (
+	"testing"
+
+	"github.com/MichaelMure/git-bug/util/test"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPushPull(t *testing.T) {
+	repoA, repoB, remote := test.SetupReposAndRemote(t)
+	defer test.CleanupRepos(repoA, repoB, remote)
+
+	identity1 := NewIdentity("name1", "email1")
+	err := identity1.Commit(repoA)
+	require.NoError(t, err)
+
+	// A --> remote --> B
+	_, err = Push(repoA, "origin")
+	require.NoError(t, err)
+
+	err = Pull(repoB, "origin")
+	require.NoError(t, err)
+
+	identities := allIdentities(t, ReadAllLocalIdentities(repoB))
+
+	if len(identities) != 1 {
+		t.Fatal("Unexpected number of bugs")
+	}
+
+	// B --> remote --> A
+	identity2 := NewIdentity("name2", "email2")
+	err = identity2.Commit(repoB)
+	require.NoError(t, err)
+
+	_, err = Push(repoB, "origin")
+	require.NoError(t, err)
+
+	err = Pull(repoA, "origin")
+	require.NoError(t, err)
+
+	identities = allIdentities(t, ReadAllLocalIdentities(repoA))
+
+	if len(identities) != 2 {
+		t.Fatal("Unexpected number of bugs")
+	}
+
+	// Update both
+
+	identity1.AddVersion(&Version{
+		name:  "name1b",
+		email: "email1b",
+	})
+	err = identity1.Commit(repoA)
+	require.NoError(t, err)
+
+	identity2.AddVersion(&Version{
+		name:  "name2b",
+		email: "email2b",
+	})
+	err = identity2.Commit(repoB)
+	require.NoError(t, err)
+
+	//  A --> remote --> B
+
+	_, err = Push(repoA, "origin")
+	require.NoError(t, err)
+
+	err = Pull(repoB, "origin")
+	require.NoError(t, err)
+
+	identities = allIdentities(t, ReadAllLocalIdentities(repoB))
+
+	if len(identities) != 2 {
+		t.Fatal("Unexpected number of bugs")
+	}
+
+	// B --> remote --> A
+
+	_, err = Push(repoB, "origin")
+	require.NoError(t, err)
+
+	err = Pull(repoA, "origin")
+	require.NoError(t, err)
+
+	identities = allIdentities(t, ReadAllLocalIdentities(repoA))
+
+	if len(identities) != 2 {
+		t.Fatal("Unexpected number of bugs")
+	}
+
+	// Concurrent update
+
+	identity1.AddVersion(&Version{
+		name:  "name1c",
+		email: "email1c",
+	})
+	err = identity1.Commit(repoA)
+	require.NoError(t, err)
+
+	identity1B, err := ReadLocal(repoB, identity1.Id())
+	require.NoError(t, err)
+
+	identity1B.AddVersion(&Version{
+		name:  "name1concurrent",
+		email: "email1concurrent",
+	})
+	err = identity1B.Commit(repoB)
+	require.NoError(t, err)
+
+	//  A --> remote --> B
+
+	_, err = Push(repoA, "origin")
+	require.NoError(t, err)
+
+	// Pulling a non-fast-forward update should fail
+	err = Pull(repoB, "origin")
+	require.Error(t, err)
+
+	identities = allIdentities(t, ReadAllLocalIdentities(repoB))
+
+	if len(identities) != 2 {
+		t.Fatal("Unexpected number of bugs")
+	}
+
+	// B --> remote --> A
+
+	// Pushing a non-fast-forward update should fail
+	_, err = Push(repoB, "origin")
+	require.Error(t, err)
+
+	err = Pull(repoA, "origin")
+	require.NoError(t, err)
+
+	identities = allIdentities(t, ReadAllLocalIdentities(repoA))
+
+	if len(identities) != 2 {
+		t.Fatal("Unexpected number of bugs")
+	}
+}
+
+func allIdentities(t testing.TB, identities <-chan StreamedIdentity) []*Identity {
+	var result []*Identity
+	for streamed := range identities {
+		if streamed.Err != nil {
+			t.Fatal(streamed.Err)
+		}
+		result = append(result, streamed.Identity)
+	}
+	return result
+}
diff --git a/identity/identity_stub_test.go b/identity/identity_stub_test.go
new file mode 100644
index 00000000..3d94cd3e
--- /dev/null
+++ b/identity/identity_stub_test.go
@@ -0,0 +1,23 @@
+package identity
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIdentityStubSerialize(t *testing.T) {
+	before := &IdentityStub{
+		id: "id1234",
+	}
+
+	data, err := json.Marshal(before)
+	assert.NoError(t, err)
+
+	var after IdentityStub
+	err = json.Unmarshal(data, &after)
+	assert.NoError(t, err)
+
+	assert.Equal(t, before, &after)
+}
diff --git a/identity/version_test.go b/identity/version_test.go
new file mode 100644
index 00000000..8c4c8d99
--- /dev/null
+++ b/identity/version_test.go
@@ -0,0 +1,42 @@
+package identity
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestVersionSerialize(t *testing.T) {
+	before := &Version{
+		login:     "login",
+		name:      "name",
+		email:     "email",
+		avatarURL: "avatarUrl",
+		keys: []Key{
+			{
+				Fingerprint: "fingerprint1",
+				PubKey:      "pubkey1",
+			},
+			{
+				Fingerprint: "fingerprint2",
+				PubKey:      "pubkey2",
+			},
+		},
+		nonce: makeNonce(20),
+		metadata: map[string]string{
+			"key1": "value1",
+			"key2": "value2",
+		},
+		time: 3,
+	}
+
+	data, err := json.Marshal(before)
+	assert.NoError(t, err)
+
+	var after Version
+	err = json.Unmarshal(data, &after)
+	assert.NoError(t, err)
+
+	assert.Equal(t, before, &after)
+}