From f1a19ba5d5306a3e46d31d4516f405f5fe5c1b46 Mon Sep 17 00:00:00 2001 From: Dustin Long Date: Tue, 24 Jul 2018 17:39:24 -0400 Subject: [PATCH] feat(profile): Detect peername renames when listing datasets. --- lib/datasets.go | 8 +++++--- lib/profile.go | 2 +- repo/ref.go | 28 +++++++++++++++++++++------- repo/ref_test.go | 42 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 68 insertions(+), 12 deletions(-) diff --git a/lib/datasets.go b/lib/datasets.go index 716ca51a1..d4f770a48 100644 --- a/lib/datasets.go +++ b/lib/datasets.go @@ -94,7 +94,7 @@ func (r *DatasetRequests) List(p *ListParams, res *[]repo.DatasetRef) error { ds.ProfileID = pro.ID } - if err := repo.CanonicalizeProfile(r.repo, ds); err != nil { + if err := repo.CanonicalizeProfile(r.repo, ds, nil); err != nil { return fmt.Errorf("error canonicalizing peer: %s", err.Error()) } @@ -156,9 +156,10 @@ func (r *DatasetRequests) List(p *ListParams, res *[]repo.DatasetRef) error { return fmt.Errorf("error getting dataset list: %s", err.Error()) } + renames := repo.NewNeedPeernameRenames() for i, ref := range replies { - if err := repo.CanonicalizeProfile(r.repo, &replies[i]); err != nil { - log.Debug(err.Error()) + // May need to change peername. + if err := repo.CanonicalizeProfile(r.repo, &replies[i], &renames); err != nil { return fmt.Errorf("error canonicalizing dataset peername: %s", err.Error()) } @@ -171,6 +172,7 @@ func (r *DatasetRequests) List(p *ListParams, res *[]repo.DatasetRef) error { replies[i].Dataset.Structure.Schema = nil } } + // TODO: If renames.Renames is non-empty, apply it to r.repo *res = replies return nil diff --git a/lib/profile.go b/lib/profile.go index 79f04099b..9e1bf475c 100644 --- a/lib/profile.go +++ b/lib/profile.go @@ -76,7 +76,7 @@ func (r *ProfileRequests) getProfile(idStr, peername string) (pro *profile.Profi ref := &repo.DatasetRef{ Peername: peername, } - if err = repo.CanonicalizeProfile(r.repo, ref); err != nil { + if err = repo.CanonicalizeProfile(r.repo, ref, nil); err != nil { log.Error("error canonicalizing profile", err.Error()) return nil, err } diff --git a/repo/ref.go b/repo/ref.go index fdefa1cce..8407e96ec 100644 --- a/repo/ref.go +++ b/repo/ref.go @@ -298,6 +298,18 @@ func isBase58Multihash(hash string) bool { return true } +// NeedPeernameRenames represents which peernames need to be renamed, and to what. +type NeedPeernameRenames struct { + Renames map[string]string +} + +// NewNeedPeernameRenames returns a new NeedPeerNameRenames struct. +func NewNeedPeernameRenames() NeedPeernameRenames { + return NeedPeernameRenames{ + Renames: make(map[string]string), + } +} + // CanonicalizeDatasetRef uses a repo to turn any local aliases into known // canonical peername for a dataset and populates a missing path // if the repo has path information for a peername/name combo @@ -312,7 +324,7 @@ func CanonicalizeDatasetRef(r Repo, ref *DatasetRef) error { return nil } - if err := CanonicalizeProfile(r, ref); err != nil { + if err := CanonicalizeProfile(r, ref, nil); err != nil { return err } @@ -344,7 +356,7 @@ func CanonicalizeDatasetRef(r Repo, ref *DatasetRef) error { // CanonicalizeProfile populates dataset DatasetRef ProfileID and Peername properties, // changing aliases to known names, and adding ProfileID from a peerstore -func CanonicalizeProfile(r Repo, ref *DatasetRef) error { +func CanonicalizeProfile(r Repo, ref *DatasetRef, need *NeedPeernameRenames) error { if ref.Peername == "" && ref.ProfileID == "" { return nil } @@ -354,6 +366,7 @@ func CanonicalizeProfile(r Repo, ref *DatasetRef) error { return err } + // If this is a dataset ref that a peer of the user owns. if ref.Peername == "me" || ref.Peername == p.Peername || ref.ProfileID == p.ID { if ref.Peername == "me" { ref.ProfileID = p.ID @@ -365,6 +378,12 @@ func CanonicalizeProfile(r Repo, ref *DatasetRef) error { return fmt.Errorf("Peername and ProfileID combination not valid: Peername = %s, ProfileID = %s, but was given ProfileID = %s", p.Peername, p.ID, ref.ProfileID) } if ref.ProfileID == p.ID && ref.Peername != p.Peername { + // Rename may have happended, record it if requested by caller. + if need != nil { + need.Renames[ref.Peername] = p.Peername + ref.Peername = p.Peername + return nil + } return fmt.Errorf("Peername and ProfileID combination not valid: ProfileID = %s, Peername = %s, but was given Peername = %s", p.ID, p.Peername, ref.Peername) } if ref.Peername == p.Peername && ref.ProfileID == p.ID { @@ -389,11 +408,6 @@ func CanonicalizeProfile(r Repo, ref *DatasetRef) error { return nil } if ref.ProfileID != "" { - // pid, err := profile.NewB58ID(ref.ProfileID) - // if err != nil { - // return fmt.Errorf("error converting ProfileID to base58 hash: %s", err) - // } - profile, err := r.Profiles().GetProfile(ref.ProfileID) if err != nil { return fmt.Errorf("error fetching peers from store: %s", err) diff --git a/repo/ref_test.go b/repo/ref_test.go index 2c5709a51..f3e7923ae 100644 --- a/repo/ref_test.go +++ b/repo/ref_test.go @@ -1,6 +1,7 @@ package repo import ( + "reflect" "testing" "github.com/qri-io/cafs" @@ -439,7 +440,7 @@ func TestCanonicalizeProfile(t *testing.T) { } got := &ref - err = CanonicalizeProfile(repo, got) + err = CanonicalizeProfile(repo, got, nil) if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) { t.Errorf("case %d error mismatch. expected: '%s', got: '%s'", i, c.err, err) continue @@ -461,3 +462,42 @@ func TestCanonicalizeProfile(t *testing.T) { } } } + +func TestCanonicalizeProfileWithRename(t *testing.T) { + repo, err := NewMemRepo(&profile.Profile{ + Peername: "lucille", + ID: profile.IDB58MustDecode("QmYCvbfNbCwFR45HiNP45rwJgvatpiW38D961L5qAhUM5Y"), + }, cafs.NewMapstore(), profile.NewMemStore(), nil) + if err != nil { + t.Errorf("error allocating mem repo: %s", err.Error()) + return + } + + lucy := DatasetRef{ + ProfileID: profile.IDB58MustDecode("QmYCvbfNbCwFR45HiNP45rwJgvatpiW38D961L5qAhUM5Y"), + Peername: "lucy", + Name: "ball", + Path: "/ipfs/QmRdexT18WuAKVX3vPusqmJTWLeNSeJgjmMbaF5QLGHna1", + } + + renames := NewNeedPeernameRenames() + err = CanonicalizeProfile(repo, &lucy, &renames) + if err != nil { + t.Errorf("error canonicalizing: %s", err.Error()) + return + } + + keys := make([]string, 0) + for k := range renames.Renames { + keys = append(keys, k) + } + expect := []string{"lucy"} + if !reflect.DeepEqual(keys, expect) { + t.Errorf("error, expected keys %s, got %s", expect, keys) + } + expectVal := "lucille" + actualVal := renames.Renames["lucy"] + if actualVal != expectVal { + t.Errorf("error, expected value %s, got %s", expectVal, actualVal) + } +}