entity: pass the identity resolver instead of defining it once

Having the resolver in Definition doesn't actually work well as the resolver
is very situational.

7 files changed, 53 insertions, 57 deletions

diff --git a/entity/dag/clock.go b/entity/dag/clock.go
index fa944b33..c9d2b94b 100644
--- a/entity/dag/clock.go
+++ b/entity/dag/clock.go
@@ -22,14 +22,10 @@ func ClockLoader(defs ...Definition) repository.ClockLoader {
 			resolver := identity.NewStubResolver()
 
 			for _, def := range defs {
-				// override the resolver
-				def := def
-				def.identityResolver = resolver
-
 				// we actually just need to read all entities,
 				// as that will create and update the clocks
 				// TODO: concurrent loading to be faster?
-				for b := range ReadAll(def, repo) {
+				for b := range ReadAll(def, repo, resolver) {
 					if b.Err != nil {
 						return b.Err
 					}
diff --git a/entity/dag/common_test.go b/entity/dag/common_test.go
index 05d85898..0ddbca47 100644
--- a/entity/dag/common_test.go
+++ b/entity/dag/common_test.go
@@ -91,13 +91,13 @@ func unmarshaler(author identity.Interface, raw json.RawMessage) (Operation, err
   Identities + repo + definition
 */
 
-func makeTestContext() (repository.ClockedRepo, identity.Interface, identity.Interface, Definition) {
+func makeTestContext() (repository.ClockedRepo, identity.Interface, identity.Interface, identity.Resolver, Definition) {
 	repo := repository.NewMockRepo()
-	id1, id2, def := makeTestContextInternal(repo)
-	return repo, id1, id2, def
+	id1, id2, resolver, def := makeTestContextInternal(repo)
+	return repo, id1, id2, resolver, def
 }
 
-func makeTestContextRemote(t *testing.T) (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, identity.Resolver, Definition) {
 	repoA := repository.CreateGoGitTestRepo(false)
 	repoB := repository.CreateGoGitTestRepo(false)
 	remote := repository.CreateGoGitTestRepo(true)
@@ -111,7 +111,7 @@ func makeTestContextRemote(t *testing.T) (repository.ClockedRepo, repository.Clo
 	err = repoB.AddRemote("repoA", repoA.GetLocalRemote())
 	require.NoError(t, err)
 
-	id1, id2, def := makeTestContextInternal(repoA)
+	id1, id2, resolver, def := makeTestContextInternal(repoA)
 
 	// distribute the identities
 	_, err = identity.Push(repoA, "remote")
@@ -119,10 +119,10 @@ func makeTestContextRemote(t *testing.T) (repository.ClockedRepo, repository.Clo
 	err = identity.Pull(repoB, "remote")
 	require.NoError(t, err)
 
-	return repoA, repoB, remote, id1, id2, def
+	return repoA, repoB, remote, id1, id2, resolver, def
 }
 
-func makeTestContextInternal(repo repository.ClockedRepo) (identity.Interface, identity.Interface, Definition) {
+func makeTestContextInternal(repo repository.ClockedRepo) (identity.Interface, identity.Interface, identity.Resolver, Definition) {
 	id1, err := identity.NewIdentity(repo, "name1", "email1")
 	if err != nil {
 		panic(err)
@@ -155,11 +155,10 @@ func makeTestContextInternal(repo repository.ClockedRepo) (identity.Interface, i
 		typename:             "foo",
 		namespace:            "foos",
 		operationUnmarshaler: unmarshaler,
-		identityResolver:     resolver,
 		formatVersion:        1,
 	}
 
-	return id1, id2, def
+	return id1, id2, resolver, def
 }
 
 type identityResolverFunc func(id entity.Id) (identity.Interface, error)
diff --git a/entity/dag/entity.go b/entity/dag/entity.go
index d92b386e..09576d28 100644
--- a/entity/dag/entity.go
+++ b/entity/dag/entity.go
@@ -27,8 +27,6 @@ type Definition struct {
 	namespace string
 	// a function decoding a JSON message into an Operation
 	operationUnmarshaler func(author identity.Interface, raw json.RawMessage) (Operation, error)
-	// a function loading an identity.Identity from its Id
-	identityResolver identity.Resolver
 	// the expected format version number, that can be used for data migration/upgrade
 	formatVersion uint
 }
@@ -59,29 +57,29 @@ func New(definition Definition) *Entity {
 }
 
 // Read will read and decode a stored local Entity from a repository
-func Read(def Definition, repo repository.ClockedRepo, id entity.Id) (*Entity, error) {
+func Read(def Definition, repo repository.ClockedRepo, resolver identity.Resolver, id entity.Id) (*Entity, error) {
 	if err := id.Validate(); err != nil {
 		return nil, errors.Wrap(err, "invalid id")
 	}
 
 	ref := fmt.Sprintf("refs/%s/%s", def.namespace, id.String())
 
-	return read(def, repo, ref)
+	return read(def, repo, resolver, ref)
 }
 
 // readRemote will read and decode a stored remote Entity from a repository
-func readRemote(def Definition, repo repository.ClockedRepo, remote string, id entity.Id) (*Entity, error) {
+func readRemote(def Definition, repo repository.ClockedRepo, resolver identity.Resolver, remote string, id entity.Id) (*Entity, error) {
 	if err := id.Validate(); err != nil {
 		return nil, errors.Wrap(err, "invalid id")
 	}
 
 	ref := fmt.Sprintf("refs/remotes/%s/%s/%s", def.namespace, remote, id.String())
 
-	return read(def, repo, ref)
+	return read(def, repo, resolver, ref)
 }
 
 // read fetch from git and decode an Entity at an arbitrary git reference.
-func read(def Definition, repo repository.ClockedRepo, ref string) (*Entity, error) {
+func read(def Definition, repo repository.ClockedRepo, resolver identity.Resolver, ref string) (*Entity, error) {
 	rootHash, err := repo.ResolveRef(ref)
 	if err != nil {
 		return nil, err
@@ -140,7 +138,7 @@ func read(def Definition, repo repository.ClockedRepo, ref string) (*Entity, err
 			return nil, fmt.Errorf("multiple leafs in the entity DAG")
 		}
 
-		opp, err := readOperationPack(def, repo, commit)
+		opp, err := readOperationPack(def, repo, resolver, commit)
 		if err != nil {
 			return nil, err
 		}
@@ -243,7 +241,7 @@ type StreamedEntity struct {
 }
 
 // ReadAll read and parse all local Entity
-func ReadAll(def Definition, repo repository.ClockedRepo) <-chan StreamedEntity {
+func ReadAll(def Definition, repo repository.ClockedRepo, resolver identity.Resolver) <-chan StreamedEntity {
 	out := make(chan StreamedEntity)
 
 	go func() {
@@ -258,7 +256,7 @@ func ReadAll(def Definition, repo repository.ClockedRepo) <-chan StreamedEntity
 		}
 
 		for _, ref := range refs {
-			e, err := read(def, repo, ref)
+			e, err := read(def, repo, resolver, ref)
 
 			if err != nil {
 				out <- StreamedEntity{Err: err}
diff --git a/entity/dag/entity_actions.go b/entity/dag/entity_actions.go
index fa50473c..707c93aa 100644
--- a/entity/dag/entity_actions.go
+++ b/entity/dag/entity_actions.go
@@ -32,13 +32,13 @@ func Push(def Definition, repo repository.Repo, remote string) (string, error) {
 
 // Pull will do a Fetch + MergeAll
 // Contrary to MergeAll, this function will return an error if a merge fail.
-func Pull(def Definition, repo repository.ClockedRepo, remote string, author identity.Interface) error {
+func Pull(def Definition, repo repository.ClockedRepo, resolver identity.Resolver, remote string, author identity.Interface) error {
 	_, err := Fetch(def, repo, remote)
 	if err != nil {
 		return err
 	}
 
-	for merge := range MergeAll(def, repo, remote, author) {
+	for merge := range MergeAll(def, repo, resolver, remote, author) {
 		if merge.Err != nil {
 			return merge.Err
 		}
@@ -65,7 +65,10 @@ func Pull(def Definition, repo repository.ClockedRepo, remote string, author ide
 // 5. if both local and remote Entity have new commits (that is, we have a concurrent edition),
 //    a merge commit with an empty operationPack is created to join both branch and form a DAG.
 //    --> emit entity.MergeStatusUpdated
-func MergeAll(def Definition, repo repository.ClockedRepo, remote string, author identity.Interface) <-chan entity.MergeResult {
+//
+// Note: an author is necessary for the case where a merge commit is created, as this commit will
+// have an author and may be signed if a signing key is available.
+func MergeAll(def Definition, repo repository.ClockedRepo, resolver identity.Resolver, remote string, author identity.Interface) <-chan entity.MergeResult {
 	out := make(chan entity.MergeResult)
 
 	go func() {
@@ -79,7 +82,7 @@ func MergeAll(def Definition, repo repository.ClockedRepo, remote string, author
 		}
 
 		for _, remoteRef := range remoteRefs {
-			out <- merge(def, repo, remoteRef, author)
+			out <- merge(def, repo, resolver, remoteRef, author)
 		}
 	}()
 
@@ -88,14 +91,14 @@ func MergeAll(def Definition, repo repository.ClockedRepo, remote string, author
 
 // merge perform a merge to make sure a local Entity is up to date.
 // See MergeAll for more details.
-func merge(def Definition, repo repository.ClockedRepo, remoteRef string, author identity.Interface) entity.MergeResult {
+func merge(def Definition, repo repository.ClockedRepo, resolver identity.Resolver, remoteRef string, author identity.Interface) entity.MergeResult {
 	id := entity.RefToId(remoteRef)
 
 	if err := id.Validate(); err != nil {
 		return entity.NewMergeInvalidStatus(id, errors.Wrap(err, "invalid ref").Error())
 	}
 
-	remoteEntity, err := read(def, repo, remoteRef)
+	remoteEntity, err := read(def, repo, resolver, remoteRef)
 	if err != nil {
 		return entity.NewMergeInvalidStatus(id,
 			errors.Wrapf(err, "remote %s is not readable", def.typename).Error())
@@ -194,7 +197,7 @@ func merge(def Definition, repo repository.ClockedRepo, remoteRef string, author
 	// an empty operationPack.
 	// First step is to collect those clocks.
 
-	localEntity, err := read(def, repo, localRef)
+	localEntity, err := read(def, repo, resolver, localRef)
 	if err != nil {
 		return entity.NewMergeError(err, id)
 	}
diff --git a/entity/dag/entity_actions_test.go b/entity/dag/entity_actions_test.go
index 79afe525..848d6468 100644
--- a/entity/dag/entity_actions_test.go
+++ b/entity/dag/entity_actions_test.go
@@ -24,7 +24,7 @@ func allEntities(t testing.TB, bugs <-chan StreamedEntity) []*Entity {
 }
 
 func TestPushPull(t *testing.T) {
-	repoA, repoB, remote, id1, id2, def := makeTestContextRemote(t)
+	repoA, repoB, remote, id1, id2, resolver, def := makeTestContextRemote(t)
 	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	// A --> remote --> B
@@ -37,10 +37,10 @@ func TestPushPull(t *testing.T) {
 	_, err = Push(def, repoA, "remote")
 	require.NoError(t, err)
 
-	err = Pull(def, repoB, "remote", id1)
+	err = Pull(def, repoB, resolver, "remote", id1)
 	require.NoError(t, err)
 
-	entities := allEntities(t, ReadAll(def, repoB))
+	entities := allEntities(t, ReadAll(def, repoB, resolver))
 	require.Len(t, entities, 1)
 
 	// B --> remote --> A
@@ -53,15 +53,15 @@ func TestPushPull(t *testing.T) {
 	_, err = Push(def, repoB, "remote")
 	require.NoError(t, err)
 
-	err = Pull(def, repoA, "remote", id1)
+	err = Pull(def, repoA, resolver, "remote", id1)
 	require.NoError(t, err)
 
-	entities = allEntities(t, ReadAll(def, repoB))
+	entities = allEntities(t, ReadAll(def, repoB, resolver))
 	require.Len(t, entities, 2)
 }
 
 func TestListLocalIds(t *testing.T) {
-	repoA, repoB, remote, id1, id2, def := makeTestContextRemote(t)
+	repoA, repoB, remote, id1, id2, resolver, def := makeTestContextRemote(t)
 	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	// A --> remote --> B
@@ -87,7 +87,7 @@ func TestListLocalIds(t *testing.T) {
 	listLocalIds(t, def, repoA, 2)
 	listLocalIds(t, def, repoB, 0)
 
-	err = Pull(def, repoB, "remote", id1)
+	err = Pull(def, repoB, resolver, "remote", id1)
 	require.NoError(t, err)
 
 	listLocalIds(t, def, repoA, 2)
@@ -206,7 +206,7 @@ func assertNotEqualRefs(t *testing.T, repoA, repoB repository.RepoData, prefix s
 }
 
 func TestMerge(t *testing.T) {
-	repoA, repoB, remote, id1, id2, def := makeTestContextRemote(t)
+	repoA, repoB, remote, id1, id2, resolver, def := makeTestContextRemote(t)
 	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	// SCENARIO 1
@@ -231,7 +231,7 @@ func TestMerge(t *testing.T) {
 	_, err = Fetch(def, repoB, "remote")
 	require.NoError(t, err)
 
-	results := MergeAll(def, repoB, "remote", id1)
+	results := MergeAll(def, repoB, resolver, "remote", id1)
 
 	assertMergeResults(t, []entity.MergeResult{
 		{
@@ -249,7 +249,7 @@ func TestMerge(t *testing.T) {
 	// SCENARIO 2
 	// if the remote and local Entity have the same state, nothing is changed
 
-	results = MergeAll(def, repoB, "remote", id1)
+	results = MergeAll(def, repoB, resolver, "remote", id1)
 
 	assertMergeResults(t, []entity.MergeResult{
 		{
@@ -275,7 +275,7 @@ func TestMerge(t *testing.T) {
 	err = e2A.Commit(repoA)
 	require.NoError(t, err)
 
-	results = MergeAll(def, repoA, "remote", id1)
+	results = MergeAll(def, repoA, resolver, "remote", id1)
 
 	assertMergeResults(t, []entity.MergeResult{
 		{
@@ -300,7 +300,7 @@ func TestMerge(t *testing.T) {
 	_, err = Fetch(def, repoB, "remote")
 	require.NoError(t, err)
 
-	results = MergeAll(def, repoB, "remote", id1)
+	results = MergeAll(def, repoB, resolver, "remote", id1)
 
 	assertMergeResults(t, []entity.MergeResult{
 		{
@@ -327,10 +327,10 @@ func TestMerge(t *testing.T) {
 	err = e2A.Commit(repoA)
 	require.NoError(t, err)
 
-	e1B, err := Read(def, repoB, e1A.Id())
+	e1B, err := Read(def, repoB, resolver, e1A.Id())
 	require.NoError(t, err)
 
-	e2B, err := Read(def, repoB, e2A.Id())
+	e2B, err := Read(def, repoB, resolver, e2A.Id())
 	require.NoError(t, err)
 
 	e1B.Append(newOp1(id1, "barbarfoofoo"))
@@ -347,7 +347,7 @@ func TestMerge(t *testing.T) {
 	_, err = Fetch(def, repoB, "remote")
 	require.NoError(t, err)
 
-	results = MergeAll(def, repoB, "remote", id1)
+	results = MergeAll(def, repoB, resolver, "remote", id1)
 
 	assertMergeResults(t, []entity.MergeResult{
 		{
@@ -387,7 +387,7 @@ func TestMerge(t *testing.T) {
 }
 
 func TestRemove(t *testing.T) {
-	repoA, repoB, remote, id1, _, def := makeTestContextRemote(t)
+	repoA, repoB, remote, id1, _, resolver, def := makeTestContextRemote(t)
 	defer repository.CleanupTestRepos(repoA, repoB, remote)
 
 	e := New(def)
@@ -400,10 +400,10 @@ func TestRemove(t *testing.T) {
 	err = Remove(def, repoA, e.Id())
 	require.NoError(t, err)
 
-	_, err = Read(def, repoA, e.Id())
+	_, err = Read(def, repoA, resolver, e.Id())
 	require.Error(t, err)
 
-	_, err = readRemote(def, repoA, "remote", e.Id())
+	_, err = readRemote(def, repoA, resolver, "remote", e.Id())
 	require.Error(t, err)
 
 	// Remove is idempotent
diff --git a/entity/dag/operation_pack.go b/entity/dag/operation_pack.go
index 959b1ae0..d6bce9f2 100644
--- a/entity/dag/operation_pack.go
+++ b/entity/dag/operation_pack.go
@@ -166,7 +166,7 @@ func (opp *operationPack) Write(def Definition, repo repository.Repo, parentComm
 // readOperationPack read the operationPack encoded in git at the given Tree hash.
 //
 // Validity of the Lamport clocks is left for the caller to decide.
-func readOperationPack(def Definition, repo repository.RepoData, commit repository.Commit) (*operationPack, error) {
+func readOperationPack(def Definition, repo repository.RepoData, resolver identity.Resolver, commit repository.Commit) (*operationPack, error) {
 	entries, err := repo.ReadTree(commit.TreeHash)
 	if err != nil {
 		return nil, err
@@ -207,7 +207,7 @@ func readOperationPack(def Definition, repo repository.RepoData, commit reposito
 			if err != nil {
 				return nil, errors.Wrap(err, "failed to read git blob data")
 			}
-			ops, author, err = unmarshallPack(def, data)
+			ops, author, err = unmarshallPack(def, resolver, data)
 			if err != nil {
 				return nil, err
 			}
@@ -251,7 +251,7 @@ func readOperationPack(def Definition, repo repository.RepoData, commit reposito
 // unmarshallPack delegate the unmarshalling of the Operation's JSON to the decoding
 // function provided by the concrete entity. This gives access to the concrete type of each
 // Operation.
-func unmarshallPack(def Definition, data []byte) ([]Operation, identity.Interface, error) {
+func unmarshallPack(def Definition, resolver identity.Resolver, data []byte) ([]Operation, identity.Interface, error) {
 	aux := struct {
 		Author     identity.IdentityStub `json:"author"`
 		Operations []json.RawMessage     `json:"ops"`
@@ -265,7 +265,7 @@ func unmarshallPack(def Definition, data []byte) ([]Operation, identity.Interfac
 		return nil, nil, fmt.Errorf("missing author")
 	}
 
-	author, err := def.identityResolver.ResolveIdentity(aux.Author.Id())
+	author, err := resolver.ResolveIdentity(aux.Author.Id())
 	if err != nil {
 		return nil, nil, err
 	}
diff --git a/entity/dag/operation_pack_test.go b/entity/dag/operation_pack_test.go
index ac979776..a12382af 100644
--- a/entity/dag/operation_pack_test.go
+++ b/entity/dag/operation_pack_test.go
@@ -9,7 +9,7 @@ import (
 )
 
 func TestOperationPackReadWrite(t *testing.T) {
-	repo, id1, _, def := makeTestContext()
+	repo, id1, _, resolver, def := makeTestContext()
 
 	opp := &operationPack{
 		Author: id1,
@@ -27,7 +27,7 @@ func TestOperationPackReadWrite(t *testing.T) {
 	commit, err := repo.ReadCommit(commitHash)
 	require.NoError(t, err)
 
-	opp2, err := readOperationPack(def, repo, commit)
+	opp2, err := readOperationPack(def, repo, resolver, commit)
 	require.NoError(t, err)
 
 	require.Equal(t, opp, opp2)
@@ -46,7 +46,7 @@ func TestOperationPackReadWrite(t *testing.T) {
 }
 
 func TestOperationPackSignedReadWrite(t *testing.T) {
-	repo, id1, _, def := makeTestContext()
+	repo, id1, _, resolver, def := makeTestContext()
 
 	err := id1.(*identity.Identity).Mutate(repo, func(orig *identity.Mutator) {
 		orig.Keys = append(orig.Keys, identity.GenerateKey())
@@ -69,7 +69,7 @@ func TestOperationPackSignedReadWrite(t *testing.T) {
 	commit, err := repo.ReadCommit(commitHash)
 	require.NoError(t, err)
 
-	opp2, err := readOperationPack(def, repo, commit)
+	opp2, err := readOperationPack(def, repo, resolver, commit)
 	require.NoError(t, err)
 
 	require.Equal(t, opp, opp2)