Skip to content

Commit

Permalink
feat(profile): ResolveProfile replaces CanonicalizeProfile
Browse files Browse the repository at this point in the history
plus a bunch of cleanup, fixes & tests to profile store from a few subtle bugs
  • Loading branch information
b5 committed Feb 22, 2021
1 parent 8a9e02f commit 7bd848b
Show file tree
Hide file tree
Showing 17 changed files with 230 additions and 53 deletions.
9 changes: 9 additions & 0 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/qri-io/qri/config"
"github.com/qri-io/qri/event"
"github.com/qri-io/qri/lib"
"github.com/qri-io/qri/logbook"
"github.com/qri-io/qri/p2p"
"github.com/qri-io/qri/repo"
"github.com/qri-io/qri/repo/test"
Expand All @@ -46,6 +47,13 @@ func newTestRepo(t *testing.T) (r repo.Repo, teardown func()) {
prevTs := dsfs.Timestamp
dsfs.Timestamp = func() time.Time { return time.Date(2001, 01, 01, 01, 01, 01, 01, time.UTC) }

logbookTsSec := 0
prevLogbookTs := logbook.NewTimestamp
logbook.NewTimestamp = func() int64 {
logbookTsSec++
return time.Date(2001, 01, 01, 01, 01, logbookTsSec, 01, time.UTC).Unix()
}

if r, err = test.NewTestRepo(); err != nil {
t.Fatalf("error allocating test repo: %s", err.Error())
}
Expand All @@ -54,6 +62,7 @@ func newTestRepo(t *testing.T) (r repo.Repo, teardown func()) {
golog.SetLogLevel("qriapi", "info")
// lib.SaveConfig = prevSaveConfig
dsfs.Timestamp = prevTs
logbook.NewTimestamp = prevLogbookTs
}

return
Expand Down
2 changes: 2 additions & 0 deletions api/datasets.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
params := &lib.SaveParams{}
err := UnmarshalParams(r, params)
if err != nil {
log.Debugw("unmarshal dataset save error", "err", err)
util.WriteErrResponse(w, http.StatusBadRequest, err)
return
}
Expand All @@ -410,6 +411,7 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {

res, err := h.Save(r.Context(), params)
if err != nil {
log.Debugw("save dataset error", "err", err)
util.WriteErrResponse(w, http.StatusInternalServerError, err)
return
}
Expand Down
2 changes: 1 addition & 1 deletion api/datasets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func newMockDataServer(t *testing.T) *httptest.Server {
mockDataServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write(mockData)
}))
l, err := net.Listen("tcp", ":55555")
l, err := net.Listen("tcp", ":55556")
if err != nil {
t.Fatal(err.Error())
}
Expand Down
1 change: 0 additions & 1 deletion api/test_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ func NewAPITestRunner(t *testing.T) *APITestRunner {
Ctx: ctx,
}
run.Node, run.NodeTeardown = newTestNode(t)

run.Inst = newTestInstanceWithProfileFromNode(ctx, run.Node)

tmpDir, err := ioutil.TempDir("", "api_test")
Expand Down
Binary file modified api/testdata/api.snapshot
Binary file not shown.
2 changes: 1 addition & 1 deletion api/testdata/newRequestFromURL.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
"dataset": {
"peername":"peer",
"name":"family_relationships",
"bodypath":"http://127.0.0.1:55555/C2ImportFamRelSample.csv"
"bodypath":"http://127.0.0.1:55556/C2ImportFamRelSample.csv"
}
}
2 changes: 1 addition & 1 deletion base/dataset.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func ListDatasets(ctx context.Context, r repo.Repo, term string, offset, limit i
hasUnlistableRefs := false

for _, ref := range res {
if err := repo.CanonicalizeProfile(ctx, r, &ref); err != nil {
if pros, err := r.Profiles().ProfilesForUsername(ref.Peername); err != nil || len(pros) > 1 {
// This occurs when two profileIDs map to the same username, which can happen
// when a user creates a new profile using an old username. We should ignore
// references that can't be resolved this way, since other references in
Expand Down
1 change: 1 addition & 0 deletions base/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func SaveDataset(
) (ds *dataset.Dataset, err error) {
log.Debugf("SaveDataset initID=%q prevPath=%q", initID, prevPath)
pro := r.Profiles().Owner()
log.Debugw("owner", "peername", pro.Peername, "privKeyIsNil", pro.PrivKey == nil, "privKey", pro.PrivKey)
if initID == "" {
return nil, fmt.Errorf("SaveDataset requires an initID")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/remove_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func TestRemoveOneRevisionFromWorkingDirectory(t *testing.T) {
ref1 := parsePathFromRef(parseRefFromSave(output))
dsPath1 := run.GetPathForDataset(t, 0)
if ref1 != dsPath1 {
t.Fatal("ref from first save should match what is in qri repo")
t.Fatalf("ref from first save should match what is in qri repo.\nwant: %q\ngot: %q", ref1, dsPath1)
}

// Modify meta.json and body.csv.
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require (
github.com/libp2p/go-libp2p-circuit v0.3.1
github.com/libp2p/go-libp2p-connmgr v0.2.4
github.com/libp2p/go-libp2p-core v0.6.1
github.com/libp2p/go-libp2p-crypto v0.1.0
github.com/libp2p/go-libp2p-peerstore v0.2.6
github.com/libp2p/go-libp2p-swarm v0.2.8
github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect
Expand Down
31 changes: 12 additions & 19 deletions lib/datasets.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ func (m *DatasetMethods) List(ctx context.Context, p *ListParams) ([]dsref.Versi
return res, nil
}

pro := m.inst.repo.Profiles().Owner()

// ensure valid limit value
if p.Limit <= 0 {
p.Limit = 25
Expand All @@ -82,15 +80,10 @@ func (m *DatasetMethods) List(ctx context.Context, p *ListParams) ([]dsref.Versi
p.Offset = 0
}

// TODO (b5) - this logic around weather we're listing locally or
// a remote peer needs cleanup
ref := &reporef.DatasetRef{
Peername: p.Peername,
ProfileID: p.ProfileID,
}
err := repo.CanonicalizeProfile(ctx, m.inst.repo, ref)
reqProfile := m.inst.repo.Profiles().Owner()
listProfile, err := getProfile(ctx, m.inst.Repo().Profiles(), reqProfile.ID.String(), p.Peername)
if err != nil {
return nil, fmt.Errorf("error canonicalizing peer: %w", err)
return nil, err
}

// If the list operation leads to a warning, store it in this var
Expand Down Expand Up @@ -137,7 +130,7 @@ func (m *DatasetMethods) List(ctx context.Context, p *ListParams) ([]dsref.Versi
refs = refs[:p.Limit]
}
// TODO(dlong): Filtered by p.Published flag
} else if ref.Peername == "" || pro.Peername == ref.Peername {
} else if listProfile.Peername == "" || reqProfile.Peername == listProfile.Peername {
refs, err = base.ListDatasets(ctx, m.inst.repo, p.Term, p.Offset, p.Limit, p.RPC, p.Public, p.ShowNumVersions)
if errors.Is(err, ErrListWarning) {
listWarning = err
Expand Down Expand Up @@ -736,7 +729,7 @@ func (p *SaveParams) SetNonZeroDefaults() {

// Save adds a history entry, updating a dataset
func (m *DatasetMethods) Save(ctx context.Context, p *SaveParams) (*dataset.Dataset, error) {
log.Debugw("DatasetMethods.Save", "params", p)
log.Debugw("DatasetMethods.Save", "ref", p.Ref, "apply", p.Apply)
res := &dataset.Dataset{}

if m.inst.http != nil {
Expand Down Expand Up @@ -792,11 +785,13 @@ func (m *DatasetMethods) Save(ctx context.Context, p *SaveParams) (*dataset.Data

resolver, err := m.inst.resolverForMode("local")
if err != nil {
log.Debugw("save construct resolver", "mode", "local", "err", err)
return nil, err
}

ref, isNew, err := base.PrepareSaveRef(ctx, pro, m.inst.logbook, resolver, p.Ref, ds.BodyPath, p.NewName)
if err != nil {
log.Debugw("save PrepareSaveRef", "refParam", p.Ref, "wantNewName", p.NewName, "err", err)
return nil, err
}

Expand Down Expand Up @@ -846,7 +841,7 @@ func (m *DatasetMethods) Save(ctx context.Context, p *SaveParams) (*dataset.Data
}

if err = base.OpenDataset(ctx, m.inst.repo.Filesystem(), ds); err != nil {
log.Debugf("open ds error: %s", err.Error())
log.Debugw("save OpenDataset", "err", err.Error())
return nil, err
}

Expand Down Expand Up @@ -946,30 +941,28 @@ func (m *DatasetMethods) Save(ctx context.Context, p *SaveParams) (*dataset.Data
}
}

log.Debugf("create ds error: %s\n", err.Error())
log.Debugw("save base.SaveDataset", "err", err)
return nil, err
}

success = true
*res = *savedDs

// TODO (b5) - this should be integrated into base.SaveDataset
if fsiPath != "" {
vi := dsref.ConvertDatasetToVersionInfo(savedDs)
vi.FSIPath = fsiPath
if err = repo.PutVersionInfoShim(ctx, m.inst.repo, &vi); err != nil {
log.Debugw("save PutVersionInfoShim", "fsiPath", fsiPath, "err", err)
return nil, err
}
}

*res = *savedDs

if fsiPath != "" {
// Need to pass filesystem here so that we can read the README component and write it
// properly back to disk.
if writeErr := fsi.WriteComponents(savedDs, fsiPath, m.inst.repo.Filesystem()); err != nil {
log.Error(writeErr)
}
}

return res, nil
}

Expand Down
36 changes: 15 additions & 21 deletions lib/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
"github.com/qri-io/qri/config"
"github.com/qri-io/qri/profile"
"github.com/qri-io/qri/registry"
"github.com/qri-io/qri/repo"
reporef "github.com/qri-io/qri/repo/ref"
)

// ProfileMethods encapsulates business logic for this node's
Expand Down Expand Up @@ -46,7 +44,7 @@ func (m *ProfileMethods) GetProfile(ctx context.Context, in *bool, res *config.P
if res.ID == "" && res.Peername == "" {
pro = r.Profiles().Owner()
} else {
pro, err = m.getProfile(ctx, r, res.ID, res.Peername)
pro, err = getProfile(ctx, r.Profiles(), res.ID, res.Peername)
}

if err != nil {
Expand All @@ -69,26 +67,22 @@ func (m *ProfileMethods) GetProfile(ctx context.Context, in *bool, res *config.P
return nil
}

func (m *ProfileMethods) getProfile(ctx context.Context, r repo.Repo, idStr, peername string) (pro *profile.Profile, err error) {
var id profile.ID
func getProfile(ctx context.Context, pros profile.Store, idStr, peername string) (pro *profile.Profile, err error) {
if idStr == "" {
ref := &reporef.DatasetRef{
Peername: peername,
}
if err = repo.CanonicalizeProfile(ctx, r, ref); err != nil {
log.Error("error canonicalizing profile", err.Error())
return nil, err
}
id = ref.ProfileID
} else {
id, err = profile.IDB58Decode(idStr)
if err != nil {
log.Error("err decoding multihash", err.Error())
return nil, err
// TODO(b5): we're handling the "me" keyword here, should be handled as part of
// request scope construction
if peername == "me" {
return pros.Owner(), nil
}
return profile.ResolveUsername(pros, peername)
}

return r.Profiles().GetProfile(id)
id, err := profile.IDB58Decode(idStr)
if err != nil {
log.Debugw("decoding profile ID", "err", err)
return nil, err
}
return pros.GetProfile(id)
}

// SaveProfile stores changes to this peer's editable profile
Expand Down Expand Up @@ -161,7 +155,7 @@ func (m *ProfileMethods) ProfilePhoto(req *config.ProfilePod, res *[]byte) (err

r := m.inst.repo

pro, e := m.getProfile(ctx, r, req.ID, req.Peername)
pro, e := getProfile(ctx, r.Profiles(), req.ID, req.Peername)
if e != nil {
return e
}
Expand Down Expand Up @@ -258,7 +252,7 @@ func (m *ProfileMethods) PosterPhoto(req *config.ProfilePod, res *[]byte) (err e
ctx := context.TODO()

r := m.inst.repo
pro, e := m.getProfile(ctx, r, req.ID, req.Peername)
pro, e := getProfile(ctx, r.Profiles(), req.ID, req.Peername)
if e != nil {
return e
}
Expand Down
7 changes: 7 additions & 0 deletions profile/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@ func (p Profile) Encode() (*config.ProfilePod, error) {
PeerIDs: pids,
NetworkAddrs: addrs,
}
if p.PrivKey != nil {
pkB, err := p.PrivKey.Bytes()
if err != nil {
return nil, err
}
pp.PrivKey = base64.StdEncoding.EncodeToString(pkB)
}
return pp, nil
}

Expand Down
Loading

0 comments on commit 7bd848b

Please sign in to comment.