From 94f06cd54defa73f5e8b79345597279e454c78e6 Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Sun, 14 Feb 2021 10:02:01 +0100 Subject: 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. --- entity/dag/clock.go | 6 +----- entity/dag/common_test.go | 17 ++++++++--------- entity/dag/entity.go | 18 ++++++++---------- entity/dag/entity_actions.go | 17 ++++++++++------- entity/dag/entity_actions_test.go | 36 ++++++++++++++++++------------------ entity/dag/operation_pack.go | 8 ++++---- entity/dag/operation_pack_test.go | 8 ++++---- 7 files changed, 53 insertions(+), 57 deletions(-) (limited to 'entity') 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) -- cgit