entity: more testing and bug fixing

8 files changed, 221 insertions, 89 deletions

diff --git a/bug/bug_actions.go b/bug/bug_actions.go
index 0e240711..bf894ef8 100644
--- a/bug/bug_actions.go
+++ b/bug/bug_actions.go
@@ -14,19 +14,12 @@ import (
 // Fetch retrieve updates from a remote
 // This does not change the local bugs state
 func Fetch(repo repository.Repo, remote string) (string, error) {
-	// "refs/bugs/*:refs/remotes/<remote>>/bugs/*"
-	remoteRefSpec := fmt.Sprintf(bugsRemoteRefPattern, remote)
-	fetchRefSpec := fmt.Sprintf("%s*:%s*", bugsRefPattern, remoteRefSpec)
-
-	return repo.FetchRefs(remote, fetchRefSpec)
+	return repo.FetchRefs(remote, "bugs")
 }
 
 // Push update a remote with the local changes
 func Push(repo repository.Repo, remote string) (string, error) {
-	// "refs/bugs/*:refs/bugs/*"
-	refspec := fmt.Sprintf("%s*:%s*", bugsRefPattern, bugsRefPattern)
-
-	return repo.PushRefs(remote, refspec)
+	return repo.PushRefs(remote, "bugs")
 }
 
 // Pull will do a Fetch + MergeAll
diff --git a/entity/dag/common_test.go b/entity/dag/common_test.go
index b822fc79..05d85898 100644
--- a/entity/dag/common_test.go
+++ b/entity/dag/common_test.go
@@ -3,6 +3,9 @@ package dag
 import (
 	"encoding/json"
 	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
 
 	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/identity"
@@ -94,23 +97,28 @@ func makeTestContext() (repository.ClockedRepo, identity.Interface, identity.Int
 	return repo, id1, id2, def
 }
 
-func makeTestContextRemote() (repository.ClockedRepo, repository.ClockedRepo, repository.ClockedRepo, identity.Interface, identity.Interface, Definition) {
+func makeTestContextRemote(t *testing.T) (repository.ClockedRepo, repository.ClockedRepo, repository.ClockedRepo, identity.Interface, identity.Interface, Definition) {
 	repoA := repository.CreateGoGitTestRepo(false)
 	repoB := repository.CreateGoGitTestRepo(false)
 	remote := repository.CreateGoGitTestRepo(true)
 
-	err := repoA.AddRemote("origin", remote.GetLocalRemote())
-	if err != nil {
-		panic(err)
-	}
-
-	err = repoB.AddRemote("origin", remote.GetLocalRemote())
-	if err != nil {
-		panic(err)
-	}
+	err := repoA.AddRemote("remote", remote.GetLocalRemote())
+	require.NoError(t, err)
+	err = repoA.AddRemote("repoB", repoB.GetLocalRemote())
+	require.NoError(t, err)
+	err = repoB.AddRemote("remote", remote.GetLocalRemote())
+	require.NoError(t, err)
+	err = repoB.AddRemote("repoA", repoA.GetLocalRemote())
+	require.NoError(t, err)
 
 	id1, id2, def := makeTestContextInternal(repoA)
 
+	// distribute the identities
+	_, err = identity.Push(repoA, "remote")
+	require.NoError(t, err)
+	err = identity.Pull(repoB, "remote")
+	require.NoError(t, err)
+
 	return repoA, repoB, remote, id1, id2, def
 }
 
diff --git a/entity/dag/entity_actions.go b/entity/dag/entity_actions.go
index db3a545c..edc47d52 100644
--- a/entity/dag/entity_actions.go
+++ b/entity/dag/entity_actions.go
@@ -21,20 +21,12 @@ func ListLocalIds(def Definition, repo repository.RepoData) ([]entity.Id, error)
 // Fetch retrieve updates from a remote
 // This does not change the local entity state
 func Fetch(def Definition, repo repository.Repo, remote string) (string, error) {
-	// "refs/<entity>/*:refs/remotes/<remote>/<entity>/*"
-	fetchRefSpec := fmt.Sprintf("refs/%s/*:refs/remotes/%s/%s/*",
-		def.namespace, remote, def.namespace)
-
-	return repo.FetchRefs(remote, fetchRefSpec)
+	return repo.FetchRefs(remote, def.namespace)
 }
 
 // Push update a remote with the local changes
 func Push(def Definition, repo repository.Repo, remote string) (string, error) {
-	// "refs/<entity>/*:refs/<entity>/*"
-	refspec := fmt.Sprintf("refs/%s/*:refs/%s/*",
-		def.namespace, def.namespace)
-
-	return repo.PushRefs(remote, refspec)
+	return repo.PushRefs(remote, def.namespace)
 }
 
 // Pull will do a Fetch + MergeAll
diff --git a/entity/dag/entity_actions_test.go b/entity/dag/entity_actions_test.go
index 6cc544b6..d7717056 100644
--- a/entity/dag/entity_actions_test.go
+++ b/entity/dag/entity_actions_test.go
@@ -1,62 +1,58 @@
 package dag
 
 import (
+	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 
-	"github.com/MichaelMure/git-bug/identity"
+	"github.com/MichaelMure/git-bug/entity"
 	"github.com/MichaelMure/git-bug/repository"
 )
 
 func allEntities(t testing.TB, bugs <-chan StreamedEntity) []*Entity {
+	t.Helper()
+
 	var result []*Entity
 	for streamed := range bugs {
-		if streamed.Err != nil {
-			t.Fatal(streamed.Err)
-		}
+		require.NoError(t, streamed.Err)
+
 		result = append(result, streamed.Entity)
 	}
 	return result
 }
 
 func TestPushPull(t *testing.T) {
-	repoA, repoB, remote, id1, id2, def := makeTestContextRemote()
+	repoA, repoB, remote, id1, id2, def := makeTestContextRemote(t)
 	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
-	// distribute the identities
-	_, err := identity.Push(repoA, "origin")
-	require.NoError(t, err)
-	err = identity.Pull(repoB, "origin")
-	require.NoError(t, err)
-
 	// A --> remote --> B
-	entity := New(def)
-	entity.Append(newOp1(id1, "foo"))
+	e := New(def)
+	e.Append(newOp1(id1, "foo"))
 
-	err = entity.Commit(repoA)
+	err := e.Commit(repoA)
 	require.NoError(t, err)
 
-	_, err = Push(def, repoA, "origin")
+	_, err = Push(def, repoA, "remote")
 	require.NoError(t, err)
 
-	err = Pull(def, repoB, "origin")
+	err = Pull(def, repoB, "remote")
 	require.NoError(t, err)
 
 	entities := allEntities(t, ReadAll(def, repoB))
 	require.Len(t, entities, 1)
 
 	// B --> remote --> A
-	entity = New(def)
-	entity.Append(newOp2(id2, "bar"))
+	e = New(def)
+	e.Append(newOp2(id2, "bar"))
 
-	err = entity.Commit(repoB)
+	err = e.Commit(repoB)
 	require.NoError(t, err)
 
-	_, err = Push(def, repoB, "origin")
+	_, err = Push(def, repoB, "remote")
 	require.NoError(t, err)
 
-	err = Pull(def, repoA, "origin")
+	err = Pull(def, repoA, "remote")
 	require.NoError(t, err)
 
 	entities = allEntities(t, ReadAll(def, repoB))
@@ -64,39 +60,33 @@ func TestPushPull(t *testing.T) {
 }
 
 func TestListLocalIds(t *testing.T) {
-	repoA, repoB, remote, id1, id2, def := makeTestContextRemote()
+	repoA, repoB, remote, id1, id2, def := makeTestContextRemote(t)
 	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
-	// distribute the identities
-	_, err := identity.Push(repoA, "origin")
-	require.NoError(t, err)
-	err = identity.Pull(repoB, "origin")
-	require.NoError(t, err)
-
 	// A --> remote --> B
-	entity := New(def)
-	entity.Append(newOp1(id1, "foo"))
-	err = entity.Commit(repoA)
+	e := New(def)
+	e.Append(newOp1(id1, "foo"))
+	err := e.Commit(repoA)
 	require.NoError(t, err)
 
-	entity = New(def)
-	entity.Append(newOp2(id2, "bar"))
-	err = entity.Commit(repoA)
+	e = New(def)
+	e.Append(newOp2(id2, "bar"))
+	err = e.Commit(repoA)
 	require.NoError(t, err)
 
 	listLocalIds(t, def, repoA, 2)
 	listLocalIds(t, def, repoB, 0)
 
-	_, err = Push(def, repoA, "origin")
+	_, err = Push(def, repoA, "remote")
 	require.NoError(t, err)
 
-	_, err = Fetch(def, repoB, "origin")
+	_, err = Fetch(def, repoB, "remote")
 	require.NoError(t, err)
 
 	listLocalIds(t, def, repoA, 2)
 	listLocalIds(t, def, repoB, 0)
 
-	err = Pull(def, repoB, "origin")
+	err = Pull(def, repoB, "remote")
 	require.NoError(t, err)
 
 	listLocalIds(t, def, repoA, 2)
@@ -108,3 +98,120 @@ func listLocalIds(t *testing.T, def Definition, repo repository.RepoData, expect
 	require.NoError(t, err)
 	require.Len(t, ids, expectedCount)
 }
+
+func assertMergeResults(t *testing.T, expected []entity.MergeResult, results <-chan entity.MergeResult) {
+	t.Helper()
+
+	var allResults []entity.MergeResult
+	for result := range results {
+		allResults = append(allResults, result)
+	}
+
+	require.Equal(t, len(expected), len(allResults))
+
+	sort.Slice(allResults, func(i, j int) bool {
+		return allResults[i].Id < allResults[j].Id
+	})
+	sort.Slice(expected, func(i, j int) bool {
+		return expected[i].Id < expected[j].Id
+	})
+
+	for i, result := range allResults {
+		require.NoError(t, result.Err)
+
+		require.Equal(t, expected[i].Id, result.Id)
+		require.Equal(t, expected[i].Status, result.Status)
+
+		switch result.Status {
+		case entity.MergeStatusNew, entity.MergeStatusUpdated:
+			require.NotNil(t, result.Entity)
+			require.Equal(t, expected[i].Id, result.Entity.Id())
+		}
+
+		i++
+	}
+}
+
+func TestMerge(t *testing.T) {
+	repoA, repoB, remote, id1, id2, def := makeTestContextRemote(t)
+	defer repository.CleanupTestRepos(repoA, repoB, remote)
+
+	// SCENARIO 1
+	// if the remote Entity doesn't exist locally, it's created
+
+	// 2 entities in repoA + push to remote
+	e1 := New(def)
+	e1.Append(newOp1(id1, "foo"))
+	err := e1.Commit(repoA)
+	require.NoError(t, err)
+
+	e2 := New(def)
+	e2.Append(newOp2(id2, "bar"))
+	err = e2.Commit(repoA)
+	require.NoError(t, err)
+
+	_, err = Push(def, repoA, "remote")
+	require.NoError(t, err)
+
+	// repoB: fetch + merge from remote
+
+	_, err = Fetch(def, repoB, "remote")
+	require.NoError(t, err)
+
+	results := MergeAll(def, repoB, "remote")
+
+	assertMergeResults(t, []entity.MergeResult{
+		{
+			Id:     e1.Id(),
+			Status: entity.MergeStatusNew,
+		},
+		{
+			Id:     e2.Id(),
+			Status: entity.MergeStatusNew,
+		},
+	}, results)
+
+	// SCENARIO 2
+	// if the remote and local Entity have the same state, nothing is changed
+
+	results = MergeAll(def, repoB, "remote")
+
+	assertMergeResults(t, []entity.MergeResult{
+		{
+			Id:     e1.Id(),
+			Status: entity.MergeStatusNothing,
+		},
+		{
+			Id:     e2.Id(),
+			Status: entity.MergeStatusNothing,
+		},
+	}, results)
+
+	// SCENARIO 3
+	// if the local Entity has new commits but the remote don't, nothing is changed
+
+	e1.Append(newOp1(id1, "barbar"))
+	err = e1.Commit(repoA)
+	require.NoError(t, err)
+
+	e2.Append(newOp2(id2, "barbarbar"))
+	err = e2.Commit(repoA)
+	require.NoError(t, err)
+
+	results = MergeAll(def, repoA, "remote")
+
+	assertMergeResults(t, []entity.MergeResult{
+		{
+			Id:     e1.Id(),
+			Status: entity.MergeStatusNothing,
+		},
+		{
+			Id:     e2.Id(),
+			Status: entity.MergeStatusNothing,
+		},
+	}, results)
+
+	// SCENARIO 4
+	// if the remote has new commit, the local bug is updated to match the same history
+	// (fast-forward update)
+}
diff --git a/identity/identity_actions.go b/identity/identity_actions.go
index 21ce3fa6..b58bb2d9 100644
--- a/identity/identity_actions.go
+++ b/identity/identity_actions.go
@@ -13,19 +13,12 @@ import (
 // Fetch retrieve updates from a remote
 // This does not change the local identities state
 func Fetch(repo repository.Repo, remote string) (string, error) {
-	// "refs/identities/*:refs/remotes/<remote>/identities/*"
-	remoteRefSpec := fmt.Sprintf(identityRemoteRefPattern, remote)
-	fetchRefSpec := fmt.Sprintf("%s*:%s*", identityRefPattern, remoteRefSpec)
-
-	return repo.FetchRefs(remote, fetchRefSpec)
+	return repo.FetchRefs(remote, "identities")
 }
 
 // Push update a remote with the local changes
 func Push(repo repository.Repo, remote string) (string, error) {
-	// "refs/identities/*:refs/identities/*"
-	refspec := fmt.Sprintf("%s*:%s*", identityRefPattern, identityRefPattern)
-
-	return repo.PushRefs(remote, refspec)
+	return repo.PushRefs(remote, "identities")
 }
 
 // Pull will do a Fetch + MergeAll
diff --git a/repository/gogit.go b/repository/gogit.go
index fe434d88..c26fde9f 100644
--- a/repository/gogit.go
+++ b/repository/gogit.go
@@ -353,13 +353,17 @@ func (repo *GoGitRepo) ClearBleveIndex(name string) error {
 	return nil
 }
 
-// FetchRefs fetch git refs from a remote
-func (repo *GoGitRepo) FetchRefs(remote string, refSpec string) (string, error) {
+// FetchRefs fetch git refs matching a directory prefix to a remote
+// Ex: prefix="foo" will fetch any remote refs matching "refs/foo/*" locally.
+// The equivalent git refspec would be "refs/foo/*:refs/remotes/<remote>/foo/*"
+func (repo *GoGitRepo) FetchRefs(remote string, prefix string) (string, error) {
+	refspec := fmt.Sprintf("refs/%s/*:refs/remotes/%s/%s/*", prefix, remote, prefix)
+
 	buf := bytes.NewBuffer(nil)
 
 	err := repo.r.Fetch(&gogit.FetchOptions{
 		RemoteName: remote,
-		RefSpecs:   []config.RefSpec{config.RefSpec(refSpec)},
+		RefSpecs:   []config.RefSpec{config.RefSpec(refspec)},
 		Progress:   buf,
 	})
 	if err == gogit.NoErrAlreadyUpToDate {
@@ -372,13 +376,41 @@ func (repo *GoGitRepo) FetchRefs(remote string, refSpec string) (string, error)
 	return buf.String(), nil
 }
 
-// PushRefs push git refs to a remote
-func (repo *GoGitRepo) PushRefs(remote string, refSpec string) (string, error) {
+// PushRefs push git refs matching a directory prefix to a remote
+// Ex: prefix="foo" will push any local refs matching "refs/foo/*" to the remote.
+// The equivalent git refspec would be "refs/foo/*:refs/foo/*"
+//
+// Additionally, PushRefs will update the local references in refs/remotes/<remote>/foo to match
+// the remote state.
+func (repo *GoGitRepo) PushRefs(remote string, prefix string) (string, error) {
+	refspec := fmt.Sprintf("refs/%s/*:refs/%s/*", prefix, prefix)
+
+	remo, err := repo.r.Remote(remote)
+	if err != nil {
+		return "", err
+	}
+
+	// to make sure that the push also create the corresponding refs/remotes/<remote>/... references,
+	// we need to have a default fetch refspec configured on the remote, to make our refs "track" the remote ones.
+	// This does not change the config on disk, only on memory.
+	hasCustomFetch := false
+	fetchRefspec := fmt.Sprintf("refs/%s/*:refs/remotes/%s/%s/*", prefix, remote, prefix)
+	for _, r := range remo.Config().Fetch {
+		if string(r) == fetchRefspec {
+			hasCustomFetch = true
+			break
+		}
+	}
+
+	if !hasCustomFetch {
+		remo.Config().Fetch = append(remo.Config().Fetch, config.RefSpec(fetchRefspec))
+	}
+
 	buf := bytes.NewBuffer(nil)
 
-	err := repo.r.Push(&gogit.PushOptions{
+	err = remo.Push(&gogit.PushOptions{
 		RemoteName: remote,
-		RefSpecs:   []config.RefSpec{config.RefSpec(refSpec)},
+		RefSpecs:   []config.RefSpec{config.RefSpec(refspec)},
 		Progress:   buf,
 	})
 	if err == gogit.NoErrAlreadyUpToDate {
diff --git a/repository/mock_repo.go b/repository/mock_repo.go
index adbceec2..12cf375b 100644
--- a/repository/mock_repo.go
+++ b/repository/mock_repo.go
@@ -201,12 +201,12 @@ func NewMockRepoData() *mockRepoData {
 	}
 }
 
-func (r *mockRepoData) FetchRefs(remote string, refSpec string) (string, error) {
+func (r *mockRepoData) FetchRefs(remote string, prefix string) (string, error) {
 	panic("implement me")
 }
 
 // PushRefs push git refs to a remote
-func (r *mockRepoData) PushRefs(remote string, refSpec string) (string, error) {
+func (r *mockRepoData) PushRefs(remote string, prefix string) (string, error) {
 	panic("implement me")
 }
 
diff --git a/repository/repo.go b/repository/repo.go
index c31afa22..8d162c6d 100644
--- a/repository/repo.go
+++ b/repository/repo.go
@@ -100,11 +100,18 @@ type Commit struct {
 
 // RepoData give access to the git data storage
 type RepoData interface {
-	// FetchRefs fetch git refs from a remote
-	FetchRefs(remote string, refSpec string) (string, error)
-
-	// PushRefs push git refs to a remote
-	PushRefs(remote string, refSpec string) (string, error)
+	// FetchRefs fetch git refs matching a directory prefix to a remote
+	// Ex: prefix="foo" will fetch any remote refs matching "refs/foo/*" locally.
+	// The equivalent git refspec would be "refs/foo/*:refs/remotes/<remote>/foo/*"
+	FetchRefs(remote string, prefix string) (string, error)
+
+	// PushRefs push git refs matching a directory prefix to a remote
+	// Ex: prefix="foo" will push any local refs matching "refs/foo/*" to the remote.
+	// The equivalent git refspec would be "refs/foo/*:refs/foo/*"
+	//
+	// Additionally, PushRefs will update the local references in refs/remotes/<remote>/foo to match
+	// the remote state.
+	PushRefs(remote string, prefix string) (string, error)
 
 	// StoreData will store arbitrary data and return the corresponding hash
 	StoreData(data []byte) (Hash, error)