From d5ddc8faa61f11bd81723a3a91f81800117cb9c8 Mon Sep 17 00:00:00 2001 From: b5 Date: Sun, 26 Apr 2020 00:18:50 -0400 Subject: [PATCH] feat(RefResolver): introduce RefResolver interface, use in DatasetRequests.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. --- api/fsi_test.go | 2 +- api/testdata/api.snapshot | Bin 191466 -> 191436 bytes cmd/rename_integration_test.go | 2 +- dscache/dscache.go | 32 +++++++ lib/datasets.go | 86 ++++++------------- lib/datasets_test.go | 32 +++---- lib/resolve.go | 92 +++++++++++++++++++++ logbook/logbook.go | 52 ++++++++++++ logbook/logbook_test.go | 58 ++++++++++++- repo/fs/fs.go | 33 ++++++++ repo/mem_repo.go | 33 ++++++++ repo/repo.go | 8 ++ repo/store.go | 1 - {resolver => resolve}/loader/loader.go | 6 +- {resolver => resolve}/mem_resolver.go | 2 +- {resolver => resolve}/mem_resolver_test.go | 2 +- resolve/resolver.go | 31 +++++++ resolver/resolver.go | 22 ----- 18 files changed, 386 insertions(+), 108 deletions(-) create mode 100644 lib/resolve.go rename {resolver => resolve}/loader/loader.go (93%) rename {resolver => resolve}/mem_resolver.go (98%) rename {resolver => resolve}/mem_resolver_test.go (98%) create mode 100644 resolve/resolver.go delete mode 100644 resolver/resolver.go diff --git a/api/fsi_test.go b/api/fsi_test.go index 35aa34af9..451ed9fea 100644 --- a/api/fsi_test.go +++ b/api/fsi_test.go @@ -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) diff --git a/api/testdata/api.snapshot b/api/testdata/api.snapshot index 85bb0e12c6a76168747c5c1f8a837c0d1bfb3ceb..1c4e1d06750e4c2834978da7a83fdc8a6797d151 100755 GIT binary patch delta 75 zcmaELo%_sn?uHh|EllfJr`v}zv28!f%EZJZoSc}KmtUe#lv3+pbJllU3F@3B601p!y`v3p{ delta 60 zcmX?eo%_{w?uHh|EllfJr>C(nbx!AJXA+z~^C#ns>F3#)o=-o^#^kj94?9yz7D&2b Px= 0; i-- { + op := branchLog.Ops[i] + if op.Model == CommitModel { + switch op.Type { + case oplog.OpTypeRemove: + removes += int(op.Size) + case oplog.OpTypeInit, oplog.OpTypeAmend: + if removes > 0 { + removes-- + } + if removes == 0 { + return op.Ref + } + } + } + } + return "" +} + // save writes the book to book.fsLocation func (book *Book) save(ctx context.Context) (err error) { if al, ok := book.store.(oplog.AuthorLogstore); ok { diff --git a/logbook/logbook_test.go b/logbook/logbook_test.go index 3505f7a8d..26a513dfe 100644 --- a/logbook/logbook_test.go +++ b/logbook/logbook_test.go @@ -13,6 +13,7 @@ import ( "github.com/qri-io/qfs" "github.com/qri-io/qri/dsref" "github.com/qri-io/qri/logbook/oplog" + "github.com/qri-io/qri/resolve" ) func Example() { @@ -228,6 +229,57 @@ func TestNilCallable(t *testing.T) { if err = book.WriteVersionSave(ctx, initID, nil); err != ErrNoLogbook { t.Errorf("expected '%s', got: %v", ErrNoLogbook, err) } + if err = book.ResolveRef(ctx, nil); err != resolve.ErrCannotResolveName { + t.Errorf("expected '%s', got: %v", resolve.ErrCannotResolveName, err) + } +} + +func TestResolveRef(t *testing.T) { + tr, cleanup := newTestRunner(t) + defer cleanup() + + if err := (*Book)(nil).ResolveRef(tr.Ctx, nil); err != resolve.ErrCannotResolveName { + t.Errorf("book ResolveRef must be nil-callable. expected: %q, got %v", resolve.ErrCannotResolveName, err) + } + + if err := tr.Book.ResolveRef(tr.Ctx, &dsref.Ref{Username: "username", Name: "does_not_exist"}); err != resolve.ErrCannotResolveName { + t.Errorf("expeted standard error resolving nonexistent ref: %q, got: %q", resolve.ErrCannotResolveName, err) + } + + tr.WriteWorldBankExample(t) + + resolveMe := dsref.Ref{ + Username: tr.WorldBankRef().Username, + Name: tr.WorldBankRef().Name, + } + + if err := tr.Book.ResolveRef(tr.Ctx, &resolveMe); err != nil { + t.Error(err) + } + + expect := dsref.Ref{ + Username: tr.WorldBankRef().Username, + Name: tr.WorldBankRef().Name, + Path: "QmHashOfVersion3", + ID: tr.WorldBankID(), + } + + if diff := cmp.Diff(expect, resolveMe); diff != "" { + t.Errorf("result mismatch. (-want +got):\n%s", diff) + } + + resolveMe = dsref.Ref{ + Username: "me", + Name: tr.WorldBankRef().Name, + } + + if err := tr.Book.ResolveRef(tr.Ctx, &resolveMe); err != nil { + t.Error(err) + } + + if diff := cmp.Diff(expect, resolveMe); diff != "" { + t.Errorf("'me' shortcut result mismatch. (-want +got):\n%s", diff) + } } func TestBookLogEntries(t *testing.T) { @@ -843,7 +895,11 @@ func (tr *testRunner) WorldBankRef() dsref.Ref { return dsref.Ref{Username: tr.Username, Name: "world_bank_population"} } -func (tr *testRunner) WriteWorldBankExample(t *testing.T) string { +func (tr *testRunner) WorldBankID() string { + return "crwd4wku64be6uxu3wbfqj7z65vtps4jt5ayx5dpjq4e2k72ks7q" +} + +func (tr *testRunner) WriteWorldBankExample(t *testing.T) { book := tr.Book name := "world_bank_population" diff --git a/repo/fs/fs.go b/repo/fs/fs.go index 31835b67d..c88811392 100644 --- a/repo/fs/fs.go +++ b/repo/fs/fs.go @@ -2,6 +2,7 @@ package fsrepo import ( + "context" "fmt" "os" @@ -11,9 +12,12 @@ import ( "github.com/qri-io/qfs" "github.com/qri-io/qfs/cafs" "github.com/qri-io/qri/dscache" + "github.com/qri-io/qri/dsref" "github.com/qri-io/qri/logbook" "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/resolve" ) var log = golog.Logger("fsrepo") @@ -79,6 +83,35 @@ func NewRepo(store cafs.Filestore, fsys qfs.Filesystem, book *logbook.Book, cach return r, nil } +// ResolveRef implements the resolve.RefResolver interface +func (r *Repo) ResolveRef(ctx context.Context, ref *dsref.Ref) error { + if r == nil { + return resolve.ErrCannotResolveName + } + + if ref.Username == "me" { + ref.Username = r.profile.Peername + } + + match, err := r.GetRef(reporef.RefFromDsref(*ref)) + if err != nil { + return resolve.ErrCannotResolveName + } + + // TODO (b5) - repo doens't store IDs yet, breaking the assertion that ResolveRef + // will set the ID of a dataset. Need to fix that before we can ship this + + if ref.Path == "" { + if match.FSIPath != "" { + ref.Path = fmt.Sprintf("/fsi%s", match.FSIPath) + } else { + ref.Path = match.Path + } + } + + return nil +} + // Path returns the path to the root of the repo directory func (r Repo) Path() string { return string(r.basepath) diff --git a/repo/mem_repo.go b/repo/mem_repo.go index 850b6ab95..9658a6572 100644 --- a/repo/mem_repo.go +++ b/repo/mem_repo.go @@ -2,15 +2,19 @@ package repo import ( "context" + "fmt" crypto "github.com/libp2p/go-libp2p-core/crypto" "github.com/qri-io/dataset/dsgraph" "github.com/qri-io/qfs" "github.com/qri-io/qfs/cafs" "github.com/qri-io/qri/dscache" + "github.com/qri-io/qri/dsref" "github.com/qri-io/qri/event/hook" "github.com/qri-io/qri/logbook" "github.com/qri-io/qri/repo/profile" + reporef "github.com/qri-io/qri/repo/ref" + "github.com/qri-io/qri/resolve" ) // MemRepo is an in-memory implementation of the Repo interface @@ -54,6 +58,35 @@ func NewMemRepo(p *profile.Profile, store cafs.Filestore, fsys qfs.Filesystem, p }, nil } +// ResolveRef implements the resolve.RefResolver interface +func (r *MemRepo) ResolveRef(ctx context.Context, ref *dsref.Ref) error { + if r == nil { + return resolve.ErrCannotResolveName + } + + if ref.Username == "me" { + ref.Username = r.profile.Peername + } + + match, err := r.GetRef(reporef.RefFromDsref(*ref)) + if err != nil { + return resolve.ErrCannotResolveName + } + + // TODO (b5) - repo doens't store IDs yet, breaking the assertion that ResolveRef + // will set the ID of a dataset. Need to fix that before we can ship this + + if ref.Path == "" { + if match.FSIPath != "" { + ref.Path = fmt.Sprintf("/fsi%s", match.FSIPath) + } else { + ref.Path = match.Path + } + } + + return nil +} + // Store returns the underlying cafs.Filestore for this repo func (r *MemRepo) Store() cafs.Filestore { return r.store diff --git a/repo/repo.go b/repo/repo.go index 8f853a7b0..7c9650463 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -4,6 +4,7 @@ package repo import ( + "context" "fmt" golog "github.com/ipfs/go-log" @@ -11,6 +12,7 @@ import ( "github.com/qri-io/qfs" "github.com/qri-io/qfs/cafs" "github.com/qri-io/qri/dscache" + "github.com/qri-io/qri/dsref" "github.com/qri-io/qri/logbook" "github.com/qri-io/qri/repo/profile" reporef "github.com/qri-io/qri/repo/ref" @@ -58,6 +60,7 @@ type Repo interface { // All repositories wraps a content-addressed filestore as the cannonical // record of this repository's data. Store gives direct access to the // cafs.Filestore instance any given repo is using. + // TODO (b5) - deprecate store in favour of the Filesystem interface Store() cafs.Filestore // Filesystem is currently a read-only source of Filesystem-like data @@ -66,6 +69,11 @@ type Repo interface { // the long term-plan is to merge Filestore & Store Filesystem() qfs.Filesystem + // A Repo can resolve dataset references it knows about locally from its + // Refstore + // NOTE (b5): this implementation will be dropped when the Refstore interface + // is removed from the repo, delegating local ref resolution to dscache + ResolveRef(ctx context.Context, ref *dsref.Ref) error // All Repos must keep a Refstore, defining a store of known datasets // NOTE(dlong): Refstore is going away soon, everything is going to move to Dscache Refstore diff --git a/repo/store.go b/repo/store.go index d9a162705..87d51f158 100644 --- a/repo/store.go +++ b/repo/store.go @@ -16,7 +16,6 @@ type Refstore interface { PutRef(ref reporef.DatasetRef) error // GetRef "completes" a passed in alias (reporef.DatasetRef with at least Peername // and Name field specified), filling in missing fields with a stored ref - // TODO - should we rename this to "CompleteRef"? GetRef(ref reporef.DatasetRef) (reporef.DatasetRef, error) // DeleteRef removes a reference from the store DeleteRef(ref reporef.DatasetRef) error diff --git a/resolver/loader/loader.go b/resolve/loader/loader.go similarity index 93% rename from resolver/loader/loader.go rename to resolve/loader/loader.go index c394bf0b1..5be9e9a95 100644 --- a/resolver/loader/loader.go +++ b/resolve/loader/loader.go @@ -11,14 +11,14 @@ import ( "github.com/qri-io/qri/dscache" "github.com/qri-io/qri/dsref" "github.com/qri-io/qri/fsi" - "github.com/qri-io/qri/resolver" + "github.com/qri-io/qri/resolve" ) var ( log = golog.Logger("loader") ) -var _ resolver.Resolver = (*DatasetResolver)(nil) +var _ resolve.Resolver = (*DatasetResolver)(nil) // DatasetResolver is a high-level component that can resolve dataset references type DatasetResolver struct { @@ -63,7 +63,7 @@ func (dr *DatasetResolver) LoadDsref(ctx context.Context, refstr string) (*datas // Resolve username to profileID, lookup dataset by profileID + prettyName info, err := dr.Dscache.LookupByName(ref) if err != nil { - return nil, "", ref, nil, fmt.Errorf("%w: %s", resolver.ErrCannotResolveName, err) + return nil, "", ref, nil, fmt.Errorf("%w: %s", resolve.ErrCannotResolveName, err) } // Found a versionInfo, fill in ref. diff --git a/resolver/mem_resolver.go b/resolve/mem_resolver.go similarity index 98% rename from resolver/mem_resolver.go rename to resolve/mem_resolver.go index 6f815af95..126009cf5 100644 --- a/resolver/mem_resolver.go +++ b/resolve/mem_resolver.go @@ -1,4 +1,4 @@ -package resolver +package resolve import ( "fmt" diff --git a/resolver/mem_resolver_test.go b/resolve/mem_resolver_test.go similarity index 98% rename from resolver/mem_resolver_test.go rename to resolve/mem_resolver_test.go index 371d1f02f..c095a4186 100644 --- a/resolver/mem_resolver_test.go +++ b/resolve/mem_resolver_test.go @@ -1,4 +1,4 @@ -package resolver +package resolve import ( "testing" diff --git a/resolve/resolver.go b/resolve/resolver.go new file mode 100644 index 000000000..0adf5b81c --- /dev/null +++ b/resolve/resolver.go @@ -0,0 +1,31 @@ +// Package resolve centralizes logic for resolving dataset names. It provides a +// low-dependency interface that can be used throughout the stack, and also an +// implementation that ties together multiple subsystems to be used as a +// high-level mechanism for name resolution. +package resolve + +import ( + "context" + "fmt" + + "github.com/qri-io/qri/dsref" +) + +// RefResolver finds the identifier and HEAD path for a dataset reference +type RefResolver interface { + // ResolveRef uses ref as an outParam, setting ref.ID and ref.Path on success + // some implementations of name resolution may make network calls + ResolveRef(ctx context.Context, ref *dsref.Ref) error +} + +// Resolver resolves identifiers into info about datasets +type Resolver interface { + // GetInfo takes an initID for a dataset and returns the most recent VersionInfo + GetInfo(initID string) *dsref.VersionInfo + // GetInfoByDsref takes a dsref and returns the most recent VersionInfo. GetInfo should be + // used instead when possible. + GetInfoByDsref(dr dsref.Ref) *dsref.VersionInfo +} + +// ErrCannotResolveName is an error representing common name resolution problems +var ErrCannotResolveName = fmt.Errorf("cannot resolve name") diff --git a/resolver/resolver.go b/resolver/resolver.go deleted file mode 100644 index 98fb5e8c0..000000000 --- a/resolver/resolver.go +++ /dev/null @@ -1,22 +0,0 @@ -// Package resolver centralizes logic for resolving dataset names. It provides a low-dependency -// interface that can be used throughout the stack, and also an implementation that ties together -// multiple subsystems to be used as a high-level mechanism for name resolution. -package resolver - -import ( - "fmt" - - "github.com/qri-io/qri/dsref" -) - -// Resolver resolves identifiers into info about datasets -type Resolver interface { - // GetInfo takes an initID for a dataset and returns the most recent VersionInfo - GetInfo(initID string) *dsref.VersionInfo - // GetInfoByDsref takes a dsref and returns the most recent VersionInfo. GetInfo should be - // used instead when possible. - GetInfoByDsref(dr dsref.Ref) *dsref.VersionInfo -} - -// ErrCannotResolveName is an error representing common name resolution problems -var ErrCannotResolveName = fmt.Errorf("cannot resolve name")