identity: wip push/pull

Michael Muré created

Change summary

bug/bug.go                   |   4 
bug/bug_actions.go           |  12 +
cache/repo_cache.go          |   2 
commands/id.go               |   2 
identity/identity.go         |  24 +++
identity/identity_actions.go | 211 ++++++++++++++++++++++++++++++++++++++
identity/identity_stub.go    |  20 +-
identity/identity_test.go    |  10 
identity/resolver.go         |   2 
identity/version.go          |   2 
10 files changed, 262 insertions(+), 27 deletions(-)

Detailed changes

bug/bug.go 🔗

@@ -6,12 +6,12 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/MichaelMure/git-bug/identity"
+	"github.com/pkg/errors"
 
+	"github.com/MichaelMure/git-bug/identity"
 	"github.com/MichaelMure/git-bug/repository"
 	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/lamport"
-	"github.com/pkg/errors"
 )
 
 const bugsRefPattern = "refs/bugs/"

bug/bug_actions.go 🔗

@@ -8,7 +8,7 @@ import (
 	"github.com/pkg/errors"
 )
 
-// Fetch retrieve update from a remote
+// Fetch retrieve updates from a remote
 // This does not change the local bugs state
 func Fetch(repo repository.Repo, remote string) (string, error) {
 	remoteRefSpec := fmt.Sprintf(bugsRemoteRefPattern, remote)
@@ -23,7 +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
+// This function won't give details on the underlying process. If you need more,
 // use Fetch and MergeAll separately.
 func Pull(repo repository.ClockedRepo, remote string) error {
 	_, err := Fetch(repo, remote)
@@ -45,7 +45,13 @@ func Pull(repo repository.ClockedRepo, remote string) error {
 	return nil
 }
 
-// MergeAll will merge all the available remote bug
+// MergeAll will merge all the available remote bug:
+//
+// - If the remote has new commit, the local bug is updated to match the same history
+//   (fast-forward update)
+// - if the local bug has new commits but the remote don't, nothing is changed
+// - if both local and remote bug have new commits (that is, we have a concurrent edition),
+//   new local commits are rewritten at the head of the remote history (that is, a rebase)
 func MergeAll(repo repository.ClockedRepo, remote string) <-chan MergeResult {
 	out := make(chan MergeResult)
 

cache/repo_cache.go 🔗

@@ -555,7 +555,7 @@ func (c *RepoCache) ResolveIdentity(id string) (*identity.Identity, error) {
 		return cached, nil
 	}
 
-	i, err := identity.Read(c.repo, id)
+	i, err := identity.ReadLocal(c.repo, id)
 	if err != nil {
 		return nil, err
 	}

commands/id.go 🔗

@@ -17,7 +17,7 @@ func runId(cmd *cobra.Command, args []string) error {
 	var err error
 
 	if len(args) == 1 {
-		id, err = identity.Read(repo, args[0])
+		id, err = identity.ReadLocal(repo, args[0])
 	} else {
 		id, err = identity.GetUserIdentity(repo)
 	}

identity/identity.go 🔗

@@ -4,14 +4,17 @@ package identity
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
+
+	"github.com/pkg/errors"
 
 	"github.com/MichaelMure/git-bug/repository"
 	"github.com/MichaelMure/git-bug/util/git"
 	"github.com/MichaelMure/git-bug/util/lamport"
-	"github.com/pkg/errors"
 )
 
 const identityRefPattern = "refs/identities/"
+const identityRemoteRefPattern = "refs/remotes/%s/identities/"
 const versionEntryName = "version"
 const identityConfigKey = "git-bug.identity"
 
@@ -62,9 +65,22 @@ func (i *Identity) UnmarshalJSON(data []byte) error {
 	panic("identity should be loaded with identity.UnmarshalJSON")
 }
 
-// Read load an Identity from the identities data available in git
-func Read(repo repository.Repo, id string) (*Identity, error) {
+// ReadLocal load a local Identity from the identities data available in git
+func ReadLocal(repo repository.Repo, id string) (*Identity, error) {
 	ref := fmt.Sprintf("%s%s", identityRefPattern, id)
+	return read(repo, ref)
+}
+
+// ReadRemote load a remote Identity from the identities data available in git
+func ReadRemote(repo repository.Repo, remote string, id string) (*Identity, error) {
+	ref := fmt.Sprintf(identityRemoteRefPattern, remote) + id
+	return read(repo, ref)
+}
+
+// read will load and parse an identity frdm git
+func read(repo repository.Repo, ref string) (*Identity, error) {
+	refSplit := strings.Split(ref, "/")
+	id := refSplit[len(refSplit)-1]
 
 	hashes, err := repo.ListCommits(ref)
 
@@ -162,7 +178,7 @@ func GetUserIdentity(repo repository.Repo) (*Identity, error) {
 		id = val
 	}
 
-	return Read(repo, id)
+	return ReadLocal(repo, id)
 }
 
 func (i *Identity) AddVersion(version *Version) {

identity/identity_actions.go 🔗

@@ -0,0 +1,211 @@
+package identity
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/MichaelMure/git-bug/repository"
+	"github.com/pkg/errors"
+)
+
+// Fetch retrieve updates from a remote
+// 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)
+
+	return repo.FetchRefs(remote, fetchRefSpec)
+}
+
+// Push update a remote with the local changes
+func Push(repo repository.Repo, remote string) (string, error) {
+	return repo.PushRefs(remote, identityRefPattern+"*")
+}
+
+// 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.
+func Pull(repo repository.ClockedRepo, remote string) error {
+	_, err := Fetch(repo, remote)
+	if err != nil {
+		return err
+	}
+
+	for merge := range MergeAll(repo, remote) {
+		if merge.Err != nil {
+			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 nil
+}
+
+// MergeAll will merge all the available remote identity
+// To make sure that an Identity history can't be altered, a strict fast-forward
+// only policy is applied here. As an Identity should be tied to a single user, this
+// should work in practice but it does leave a possibility that a user would edit his
+// Identity from two different repo concurrently and push the changes in a non-centralized
+// network of repositories. In this case, it would result some of the repo accepting one
+// version, some other accepting another, preventing the network in general to converge
+// to the same result. This would create a sort of partition of the network, and manual
+// cleaning would be required.
+//
+// An alternative approach would be to have a determinist rebase:
+// - any commits present in both local and remote version would be kept, never changed.
+// - newer commits would be merged in a linear chain of commits, ordered based on the
+//   Lamport time
+//
+// However, this approach leave the possibility, in the case of a compromised crypto keys,
+// of forging a new version with a bogus Lamport time to be inserted before a legit version,
+// invalidating the correct version and hijacking the Identity. There would only be a short
+// period of time where this would be possible (before the network converge) but I'm not
+// 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 MergeAll(repo repository.ClockedRepo, remote string) <-chan MergeResult {
+	out := make(chan MergeResult)
+
+	go func() {
+		defer close(out)
+
+		remoteRefSpec := fmt.Sprintf(identityRemoteRefPattern, remote)
+		remoteRefs, err := repo.ListRefs(remoteRefSpec)
+
+		if err != nil {
+			out <- MergeResult{Err: err}
+			return
+		}
+
+		for _, remoteRef := range remoteRefs {
+			refSplitted := strings.Split(remoteRef, "/")
+			id := refSplitted[len(refSplitted)-1]
+
+			remoteIdentity, err := ReadLocal(repo, remoteRef)
+			remoteBug, err := readBug(repo, remoteRef)
+
+			if err != nil {
+				out <- newMergeInvalidStatus(id, errors.Wrap(err, "remote bug is not readable").Error())
+				continue
+			}
+
+			// Check for error in remote data
+			if err := remoteBug.Validate(); err != nil {
+				out <- newMergeInvalidStatus(id, errors.Wrap(err, "remote bug is invalid").Error())
+				continue
+			}
+
+			localRef := bugsRefPattern + remoteBug.Id()
+			localExist, err := repo.RefExist(localRef)
+
+			if err != nil {
+				out <- newMergeError(err, id)
+				continue
+			}
+
+			// the bug is not local yet, simply create the reference
+			if !localExist {
+				err := repo.CopyRef(remoteRef, localRef)
+
+				if err != nil {
+					out <- newMergeError(err, id)
+					return
+				}
+
+				out <- newMergeStatus(MergeStatusNew, id, remoteBug)
+				continue
+			}
+
+			localBug, err := readBug(repo, localRef)
+
+			if err != nil {
+				out <- newMergeError(errors.Wrap(err, "local bug is not readable"), id)
+				return
+			}
+
+			updated, err := localBug.Merge(repo, remoteBug)
+
+			if err != nil {
+				out <- newMergeInvalidStatus(id, errors.Wrap(err, "merge failed").Error())
+				return
+			}
+
+			if updated {
+				out <- newMergeStatus(MergeStatusUpdated, id, localBug)
+			} else {
+				out <- newMergeStatus(MergeStatusNothing, id, localBug)
+			}
+		}
+	}()
+
+	return out
+}
+
+// MergeStatus represent the result of a merge operation of a bug
+type MergeStatus int
+
+const (
+	_ MergeStatus = iota
+	MergeStatusNew
+	MergeStatusInvalid
+	MergeStatusUpdated
+	MergeStatusNothing
+)
+
+// Todo: share a generalized MergeResult with the bug package ?
+type MergeResult struct {
+	// Err is set when a terminal error occur in the process
+	Err error
+
+	Id     string
+	Status MergeStatus
+
+	// Only set for invalid status
+	Reason string
+
+	// Not set for invalid status
+	Identity *Identity
+}
+
+func (mr MergeResult) String() string {
+	switch mr.Status {
+	case MergeStatusNew:
+		return "new"
+	case MergeStatusInvalid:
+		return fmt.Sprintf("invalid data: %s", mr.Reason)
+	case MergeStatusUpdated:
+		return "updated"
+	case MergeStatusNothing:
+		return "nothing to do"
+	default:
+		panic("unknown merge status")
+	}
+}
+
+func newMergeError(err error, id string) MergeResult {
+	return MergeResult{
+		Err: err,
+		Id:  id,
+	}
+}
+
+func newMergeStatus(status MergeStatus, id string, identity *Identity) MergeResult {
+	return MergeResult{
+		Id:     id,
+		Status: status,
+
+		// Identity is not set for an invalid merge result
+		Identity: identity,
+	}
+}
+
+func newMergeInvalidStatus(id string, reason string) MergeResult {
+	return MergeResult{
+		Id:     id,
+		Status: MergeStatusInvalid,
+		Reason: reason,
+	}
+}

identity/identity_stub.go 🔗

@@ -45,41 +45,41 @@ func (i *IdentityStub) Id() string {
 }
 
 func (IdentityStub) Name() string {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) Email() string {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) Login() string {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) AvatarUrl() string {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) Keys() []Key {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) ValidKeysAtTime(time lamport.Time) []Key {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) DisplayName() string {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) Validate() error {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) Commit(repo repository.Repo) error {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }
 
 func (IdentityStub) IsProtected() bool {
-	panic("identities needs to be properly loaded with identity.Read()")
+	panic("identities needs to be properly loaded with identity.ReadLocal()")
 }

identity/identity_test.go 🔗

@@ -29,7 +29,7 @@ func TestIdentityCommitLoad(t *testing.T) {
 	assert.Nil(t, err)
 	assert.NotEmpty(t, identity.id)
 
-	loaded, err := Read(mockRepo, identity.id)
+	loaded, err := ReadLocal(mockRepo, identity.id)
 	assert.Nil(t, err)
 	commitsAreSet(t, loaded)
 	equivalentIdentity(t, identity, loaded)
@@ -70,7 +70,7 @@ func TestIdentityCommitLoad(t *testing.T) {
 	assert.Nil(t, err)
 	assert.NotEmpty(t, identity.id)
 
-	loaded, err = Read(mockRepo, identity.id)
+	loaded, err = ReadLocal(mockRepo, identity.id)
 	assert.Nil(t, err)
 	commitsAreSet(t, loaded)
 	equivalentIdentity(t, identity, loaded)
@@ -100,7 +100,7 @@ func TestIdentityCommitLoad(t *testing.T) {
 	assert.Nil(t, err)
 	assert.NotEmpty(t, identity.id)
 
-	loaded, err = Read(mockRepo, identity.id)
+	loaded, err = ReadLocal(mockRepo, identity.id)
 	assert.Nil(t, err)
 	commitsAreSet(t, loaded)
 	equivalentIdentity(t, identity, loaded)
@@ -209,7 +209,7 @@ func TestMetadata(t *testing.T) {
 	assert.NoError(t, err)
 
 	// reload
-	loaded, err := Read(mockRepo, identity.id)
+	loaded, err := ReadLocal(mockRepo, identity.id)
 	assert.Nil(t, err)
 
 	assertHasKeyValue(t, loaded.ImmutableMetadata(), "key1", "value1")
@@ -250,6 +250,6 @@ func TestJSON(t *testing.T) {
 	assert.Equal(t, identity.id, i.Id())
 
 	// make sure we can load the identity properly
-	i, err = Read(mockRepo, i.Id())
+	i, err = ReadLocal(mockRepo, i.Id())
 	assert.NoError(t, err)
 }

identity/resolver.go 🔗

@@ -18,5 +18,5 @@ func NewSimpleResolver(repo repository.Repo) *SimpleResolver {
 }
 
 func (r *SimpleResolver) ResolveIdentity(id string) (Interface, error) {
-	return Read(r.repo, id)
+	return ReadLocal(r.repo, id)
 }

identity/version.go 🔗

@@ -20,6 +20,8 @@ type Version struct {
 	// Not serialized
 	commitHash git.Hash
 
+	// Todo: add unix timestamp for ordering with identical lamport time ?
+
 	// The lamport time at which this version become effective
 	// The reference time is the bug edition lamport clock
 	Time lamport.Time