Skip to content

Commit

Permalink
feat(RefResolver): introduce RefResolver interface, use in DatasetReq…
Browse files Browse the repository at this point in the history
…uests.Get

Reference resolution returns a "canonicalized" dsref with the InitID and HEAD
Path assigned. Multiple subsystems now implement the RefResolver interface.
lib.Instance composes these multiple implementations in a predetermined order to
resolve a reference. Future implementations of the RefResolver interface can use
network calls to do foreign reference resolution.

Thsi commit introduces `/fsi` as a path prefix for data persisted using qri's
file system integration, setting the stage for pushing fsi integration farther
down the qri import hierarchy, toward being another implementation of a
qfs.Filesystem on the read side.
  • Loading branch information
b5 committed May 19, 2020
1 parent c58c82a commit d5ddc8f
Show file tree
Hide file tree
Showing 18 changed files with 386 additions and 108 deletions.
2 changes: 1 addition & 1 deletion api/fsi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestNoHistory(t *testing.T) {
dsHandler := NewDatasetHandlers(run.Inst, false)

// Expected response for dataset head, regardless of fsi parameter
expectBody := `{"data":{"peername":"peer","name":"test_ds","fsiPath":"fsi_init_dir","dataset":{"bodyPath":"fsi_init_dir/body.csv","meta":{"qri":"md:0"},"name":"test_ds","peername":"peer","qri":"ds:0","structure":{"format":"csv","qri":"st:0"}},"published":false},"meta":{"code":200}}`
expectBody := `{"data":{"peername":"peer","name":"test_ds","dataset":{"bodyPath":"fsi_init_dir/body.csv","meta":{"qri":"md:0"},"name":"test_ds","peername":"peer","qri":"ds:0","structure":{"format":"csv","qri":"st:0"}},"published":false},"meta":{"code":200}}`

// Dataset with a link to the filesystem, but no history and the api request says fsi=false
gotStatusCode, gotBodyString := APICall("/peer/test_ds", dsHandler.GetHandler)
Expand Down
Binary file modified api/testdata/api.snapshot
Binary file not shown.
2 changes: 1 addition & 1 deletion cmd/rename_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestRenameNoHistory(t *testing.T) {
if err == nil {
t.Error("expected error, did not get one")
}
expect = "repo: not found"
expect = "cannot resolve name"
if err.Error() != expect {
t.Errorf("error mismatch, expect: %s, got: %s", expect, err.Error())
}
Expand Down
32 changes: 32 additions & 0 deletions dscache/dscache.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/qri-io/qri/event/hook"
"github.com/qri-io/qri/repo/profile"
reporef "github.com/qri-io/qri/repo/ref"
"github.com/qri-io/qri/resolve"
)

var (
Expand Down Expand Up @@ -177,6 +178,37 @@ func (d *Dscache) ListRefs() ([]reporef.DatasetRef, error) {
return refs, nil
}

// ResolveRef finds the identifier for a dataset reference
// implements resolve.RefResolver interface
func (d *Dscache) ResolveRef(ctx context.Context, ref *dsref.Ref) error {
// NOTE: isEmpty is nil-callable
if d.IsEmpty() {
return resolve.ErrCannotResolveName
}

// Handle the "me" convenience shortcut
if ref.Username == "me" && d.DefaultUsername != "" {
ref.Username = d.DefaultUsername
}

vi, err := d.LookupByName(*ref)
if err != nil {
return resolve.ErrCannotResolveName
}

ref.InitID = vi.InitID
if ref.Path == "" {
// empty paths with an FSI link default to their FSI path
if vi.FSIPath != "" {
ref.Path = fmt.Sprintf("/fsi%s", vi.FSIPath)
} else {
ref.Path = vi.Path
}
}

return nil
}

// LookupByName looks up a dataset by dsref and returns the latest VersionInfo if found
func (d *Dscache) LookupByName(ref dsref.Ref) (*dsref.VersionInfo, error) {
// Convert the username into a profileID
Expand Down
86 changes: 24 additions & 62 deletions lib/datasets.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"io/ioutil"
"net/rpc"
"os"
"path/filepath"
"strings"
Expand All @@ -25,21 +25,17 @@ import (
"github.com/qri-io/qri/base/fill"
"github.com/qri-io/qri/dscache/build"
"github.com/qri-io/qri/dsref"
"github.com/qri-io/qri/errors"
qrierr "github.com/qri-io/qri/errors"
"github.com/qri-io/qri/fsi"
"github.com/qri-io/qri/fsi/linkfile"
"github.com/qri-io/qri/p2p"
"github.com/qri-io/qri/repo"
"github.com/qri-io/qri/repo/profile"
reporef "github.com/qri-io/qri/repo/ref"
"github.com/qri-io/qri/resolver/loader"
"github.com/qri-io/qri/resolve"
)

// DatasetMethods encapsulates business logic for working with Datasets on Qri
type DatasetMethods struct {
// TODO (b5) - remove cli & node fields in favour of inst accessors:
cli *rpc.Client
node *p2p.QriNode
inst *Instance
}

Expand Down Expand Up @@ -234,57 +230,23 @@ func (m *DatasetMethods) Get(p *GetParams, res *GetResult) error {
}
ctx := context.TODO()

// Check if the dataset ref uses bad-case characters, show a warning.
dr, err := dsref.Parse(p.Refstr)
if err == dsref.ErrBadCaseName {
log.Error(dsref.ErrBadCaseShouldRename)
}

var ds *dataset.Dataset
c := m.inst.dscache

if c.IsEmpty() {
// The old lookup path, using repo and refstore
ref, err := base.ToDatasetRef(p.Refstr, m.inst.repo, true)
if err != nil {
log.Debugf("Get dataset, base.ToDatasetRef %q failed, error: %s", p.Refstr, err)
return err
}

if dr.Path == "" && ref.FSIPath != "" {
if ds, err = fsi.ReadDir(ref.FSIPath); err != nil {
log.Debugf("Get dataset, fsi.ReadDir %q failed, error: %s", ref.FSIPath, err)
return fmt.Errorf("loading linked dataset: %s", err)
}
} else {
ds, err = dsfs.LoadDataset(ctx, m.inst.repo.Store(), ref.Path)
if err != nil {
log.Debugf("Get dataset, dsfs.LoadDataset %q failed, error: %s", ref, err)
return fmt.Errorf("loading dataset: %s", err)
}
}
r := reporef.ConvertToDsref(*ref)
ds.Name = ref.Name
ds.Peername = ref.Peername
res.Ref = &r
res.Dataset = ds
res.FSIPath = ref.FSIPath
res.Published = ref.Published
} else {
// New lookup path, using dscache and resolver
rsolv := loader.NewDatasetResolver(c, m.inst.repo.Store())
loadedDs, initID, ref, info, err := rsolv.LoadDsref(ctx, p.Refstr)
if err != nil {
return fmt.Errorf("loading dataset: %s", err)
}
ds = loadedDs
res.Ref = &ref
res.Dataset = ds
res.FSIPath = info.FSIPath
res.Published = info.Published
_ = initID
ref, err := m.inst.ParseAndResolveRef(ctx, p.Refstr)
if err != nil {
return err
}
ds, err = m.inst.loadDataset(ctx, ref)
if err != nil {
return err
}

res.Ref = &ref
res.Dataset = ds
// TODO (b5) - FSIPath is determined differently now: by checking .Path for
// an /fsi prefix
// TODO (b5) - Published field is longer set as part of Reference Resolution
// getting publication status should be delegated to a new function

if err = base.OpenDataset(ctx, m.inst.repo.Filesystem(), ds); err != nil {
log.Debugf("Get dataset, base.OpenDataset failed, error: %s", err)
return err
Expand Down Expand Up @@ -339,11 +301,11 @@ func (m *DatasetMethods) Get(p *GetParams, res *GetResult) error {
return err
}

if dr.Path == "" && res.FSIPath != "" {
if strings.HasPrefix(ref.Path, "/fsi") {
// TODO(dustmop): Need to handle the special case where an FSI directory has a body
// but no structure, which should infer a schema in order to read the body. Once that
// works we can remove the fsi.GetBody call and just use base.ReadBody.
res.Bytes, err = fsi.GetBody(res.FSIPath, df, p.FormatConfig, p.Offset, p.Limit, p.All)
res.Bytes, err = fsi.GetBody(strings.TrimPrefix(ref.Path, "/fsi"), df, p.FormatConfig, p.Offset, p.Limit, p.All)
if err != nil {
log.Debugf("Get dataset, fsi.GetBody %q failed, error: %s", res.FSIPath, err)
return err
Expand Down Expand Up @@ -659,7 +621,7 @@ func (m *DatasetMethods) Save(p *SaveParams, res *reporef.DatasetRef) error {
fsiPath := datasetRef.FSIPath

if fsiPath != "" && p.Drop != "" {
return errors.New(fmt.Errorf("cannot drop while FSI-linked"), "can't drop component from a working-directory, delete files instead.")
return qrierr.New(fmt.Errorf("cannot drop while FSI-linked"), "can't drop component from a working-directory, delete files instead.")
}

fileHint := p.BodyPath
Expand Down Expand Up @@ -747,7 +709,7 @@ func (m *DatasetMethods) nameIsInUse(ref dsref.Ref) bool {
}
res := GetResult{}
err := m.Get(&param, &res)
if err == repo.ErrNotFound {
if errors.Is(err, resolve.ErrCannotResolveName) {
return false
}
if err != nil {
Expand Down Expand Up @@ -1089,15 +1051,15 @@ func (m *DatasetMethods) Validate(p *ValidateDatasetParams, valerrs *[]jsonschem

// TODO: restore validating data from a URL
// if p.URL != "" && ref.IsEmpty() && o.Schema == nil {
// return (errors.New(ErrBadArgs, "if you are validating data from a url, please include a dataset name or supply the --schema flag with a file path that Qri can validate against"))
// return (qrierr.New(ErrBadArgs, "if you are validating data from a url, please include a dataset name or supply the --schema flag with a file path that Qri can validate against"))
// }

// Schema can come from either schema.json or structure.json, or the dataset itself.
// schemaFlagType determines which of these three contains the schema.
schemaFlagType := ""
schemaFilename := ""
if p.SchemaFilename != "" && p.StructureFilename != "" {
return errors.New(ErrBadArgs, "cannot provide both --schema and --structure flags")
return qrierr.New(ErrBadArgs, "cannot provide both --schema and --structure flags")
} else if p.SchemaFilename != "" {
schemaFlagType = "schema"
schemaFilename = p.SchemaFilename
Expand All @@ -1107,7 +1069,7 @@ func (m *DatasetMethods) Validate(p *ValidateDatasetParams, valerrs *[]jsonschem
}

if p.Ref == "" && (p.BodyFilename == "" || schemaFlagType == "") {
return errors.New(ErrBadArgs, "please provide a dataset name, or a supply the --body and --schema or --structure flags")
return qrierr.New(ErrBadArgs, "please provide a dataset name, or a supply the --body and --schema or --structure flags")
}

ref, err := repo.ParseDatasetRef(p.Ref)
Expand Down
32 changes: 17 additions & 15 deletions lib/datasets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ func TestDatasetRequestsGet(t *testing.T) {
if err != nil {
t.Fatal(err.Error())
}
inst := NewInstanceFromConfigAndNode(config.DefaultConfigForTesting(), node)

ref, err := mr.GetRef(reporef.DatasetRef{Peername: "peer", Name: "movies"})
if err != nil {
Expand Down Expand Up @@ -444,7 +445,7 @@ func TestDatasetRequestsGet(t *testing.T) {
expect string
}{
{"invalid peer name",
&GetParams{Refstr: "peer/ABC@abc"}, "'peer/ABC@abc' is not a valid dataset reference"},
&GetParams{Refstr: "peer/ABC@abc"}, `"peer/ABC@abc" is not a valid dataset reference: parsing ref, unexpected character at position 8: '@'`},

{"peername without path",
&GetParams{Refstr: "peer/movies"},
Expand Down Expand Up @@ -485,7 +486,7 @@ func TestDatasetRequestsGet(t *testing.T) {
&GetParams{Refstr: "peer/movies", Selector: "body", Format: "json"}, "[]"},

{"dataset empty",
&GetParams{Refstr: "", Selector: "body", Format: "json"}, "repo: empty dataset reference"},
&GetParams{Refstr: "", Selector: "body", Format: "json"}, `"" is not a valid dataset reference: empty reference`},

{"body as csv",
&GetParams{Refstr: "peer/movies", Selector: "body", Format: "csv"}, "title,duration\n"},
Expand Down Expand Up @@ -520,22 +521,23 @@ func TestDatasetRequestsGet(t *testing.T) {
bodyToPrettyString(moviesBody[:3])},
}

inst := NewInstanceFromConfigAndNode(config.DefaultConfigForTesting(), node)
m := NewDatasetMethods(inst)
dsm := NewDatasetMethods(inst)
for _, c := range cases {
got := &GetResult{}
err := m.Get(c.params, got)
if err != nil {
if err.Error() != c.expect {
t.Errorf("case \"%s\": error mismatch: expected: %s, got: %s", c.description, c.expect, err)
t.Run(c.description, func(t *testing.T) {
got := &GetResult{}
err := dsm.Get(c.params, got)
if err != nil {
if err.Error() != c.expect {
t.Errorf("error mismatch: expected: %s, got: %s", c.expect, err)
}
return
}
continue
}

result := string(got.Bytes)
if result != c.expect {
t.Errorf("case \"%s\": failed, expected:\n\"%s\", got:\n\"%s\"", c.description, c.expect, result)
}
result := string(got.Bytes)
if result != c.expect {
t.Errorf("result mismatch expected:\n%q, got:\n%q", c.expect, result)
}
})
}
}

Expand Down
92 changes: 92 additions & 0 deletions lib/resolve.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package lib

import (
"context"
"errors"
"fmt"
"strings"

"github.com/qri-io/dataset"
"github.com/qri-io/qri/base/dsfs"
"github.com/qri-io/qri/dsref"
"github.com/qri-io/qri/fsi"
"github.com/qri-io/qri/resolve"
)

// assert at compile time that instance is a RefResolver
var _ resolve.RefResolver = (*Instance)(nil)

// ParseAndResolveRef combines reference parsing and resolution
func (inst *Instance) ParseAndResolveRef(ctx context.Context, refStr string) (dsref.Ref, error) {
ref, err := dsref.Parse(refStr)

// bad case references are allowed-but-warned for backwards compatibility
if errors.Is(err, dsref.ErrBadCaseName) {
log.Error(dsref.ErrBadCaseShouldRename)
err = nil
} else if err != nil {
return ref, fmt.Errorf("%q is not a valid dataset reference: %w", refStr, err)
}

err = inst.ResolveRef(ctx, &ref)
return ref, err
}

// ResolveRef finds the identifier for a dataset reference
func (inst *Instance) ResolveRef(ctx context.Context, ref *dsref.Ref) error {
if inst == nil {
return resolve.ErrCannotResolveName
}

resolvers := []resolve.RefResolver{
// local resolution
inst.dscache,
inst.repo,
inst.logbook,

// network resolution
// inst.registry,
// inst.QriNode
}

for _, r := range resolvers {
err := r.ResolveRef(ctx, ref)
if err == nil {
return nil
} else if errors.Is(err, resolve.ErrCannotResolveName) {
continue
}

return err
}

return resolve.ErrCannotResolveName
}

// TODO (b5) - this needs to move down into base, replacing base.LoadDataset with
// a version that can load paths with a /fsi prefix, but before that can happen
// base needs to be able to import FSI. Currently FSI imports base, and doesn't
// really need to. Most of the base package functions used by FSI should be in
// FSI, as they deal with filesystem interaction.
func (inst *Instance) loadDataset(ctx context.Context, ref dsref.Ref) (*dataset.Dataset, error) {
var (
ds *dataset.Dataset
err error
)

if strings.HasPrefix(ref.Path, "/fsi") {
// Has an FSI Path, load from working directory
if ds, err = fsi.ReadDir(strings.TrimPrefix(ref.Path, "/fsi")); err != nil {
return nil, err
}
} else {
// Load from dsfs
if ds, err = dsfs.LoadDataset(ctx, inst.store, ref.Path); err != nil {
return nil, err
}
}
// Set transient info on the returned dataset
ds.Name = ref.Name
ds.Peername = ref.Username
return ds, nil
}
Loading

0 comments on commit d5ddc8f

Please sign in to comment.