fix(dscache): A few dscache improvements

Allow nilable dscache. Require context for constructor. Minor style cleanups.

cmd/save_test.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+ "context"
  "strings"
  "testing"
  "time"
@@ -250,15 +251,21 @@ func TestSaveDscache(t *testing.T) {
  logbook.NewTimestamp = prevTimestampFunc
  }()
 
- // Save a dataset with an inferred name.
+ // Save a dataset with one version.
  run.MustExec(t, "qri save --body testdata/movies/body_two.json me/movie_ds")
 
  // List with the --use-dscache flag, which builds the dscache from the logbook.
  run.MustExec(t, "qri list --use-dscache")
 
+ // Access the dscache
+ repo, err := run.RepoRoot.Repo()
+ if err != nil {
+ t.Fatal(err)
+ }
+ cache := repo.Dscache()
+
  // Dscache should have one reference. It has topIndex 1 because there are two logbook
  // elements in the branch, one for "init", one for "commit".
- cache := run.Dscache(t)
  actual := cache.VerboseString(false)
  expect := `Dscache:
  Dscache.Users:
@@ -285,10 +292,11 @@ func TestSaveDscache(t *testing.T) {
  // the dscache is not reloaded. Manually reload it here by constructing a dscache from the
  // same filename.
  fs := localfs.NewFS()
- cacheFilename := run.Dscache(t).Filename
+ cacheFilename := cache.Filename
+ ctx := context.Background()
+ cache = dscache.NewDscache(ctx, fs, cacheFilename)
 
  // Dscache should now have one reference. Now topIndex is 2 because there is another "commit".
- cache = dscache.NewDscache(fs, cacheFilename)
  actual = cache.VerboseString(false)
  // TODO(dlong): bodySize, bodyRows, commitTime should all be filled in
  expect = `Dscache:

cmd/test_runner_test.go

@@ -12,7 +12,6 @@ import (
 
  "github.com/qri-io/ioes"
  "github.com/qri-io/qri/base/dsfs"
- "github.com/qri-io/qri/dscache"
  "github.com/qri-io/qri/lib"
  "github.com/qri-io/qri/registry"
  "github.com/qri-io/qri/registry/regserver"
@@ -169,15 +168,6 @@ func (run *TestRunner) DatasetMarshalJSON(t *testing.T, ref string) (data string
  return data
 }
 
-// Dscache returns the dscache from the repo
-func (run *TestRunner) Dscache(t *testing.T) *dscache.Dscache {
- repo, err := run.RepoRoot.Repo()
- if err != nil {
- t.Fatal(err)
- }
- return repo.Dscache()
-}
-
 // MustExec runs a command, returning standard output, failing the test if there's an error
 func (run *TestRunner) MustExec(t *testing.T, cmdText string) string {
  if err := run.ExecCommand(cmdText); err != nil {

dscache/dscache.go

@@ -18,6 +18,8 @@ import (
 
 var (
  log = golog.Logger("dscache")
+ // ErrNoDscache is returned when methods are called on a non-existant Dscache
+ ErrNoDscache = fmt.Errorf("dscache: does not exist")
 )
 
 // Dscache represents an in-memory serialized dscache flatbuffer
@@ -28,10 +30,9 @@ type Dscache struct {
  ProfileIDToUsername map[string]string
 }
 
-// NewDscache will construct a dscache from the given file, or will construct an empty dscache
-// that will save to the given filename
-func NewDscache(fsys qfs.Filesystem, filename string) *Dscache {
- ctx := context.TODO()
+// NewDscache will construct a dscache from the given filename, or will construct an empty dscache
+// that will save to the given filename. Using an empty filename will disable loading and saving
+func NewDscache(ctx context.Context, fsys qfs.Filesystem, filename string) *Dscache {
  f, err := fsys.Get(ctx, filename)
  if err != nil {
  // Ignore error, as dscache loading is optional
@@ -49,11 +50,17 @@ func NewDscache(fsys qfs.Filesystem, filename string) *Dscache {
 
 // IsEmpty returns whether the dscache has any constructed data in it
 func (d *Dscache) IsEmpty() bool {
+ if d == nil {
+ return true
+ }
  return d.Root == nil
 }
 
 // Assign assigns the data from one dscache to this one
 func (d *Dscache) Assign(other *Dscache) error {
+ if d == nil {
+ return ErrNoDscache
+ }
  d.Root = other.Root
  d.Buffer = other.Buffer
  return d.save()
@@ -116,6 +123,9 @@ func (d *Dscache) VerboseString(showEmpty bool) string {
 // ListRefs returns references to each dataset in the cache
 // TODO(dlong): Not alphabetized, which lib assumes it is
 func (d *Dscache) ListRefs() ([]reporef.DatasetRef, error) {
+ if d.IsEmpty() {
+ return nil, ErrNoDscache
+ }
  d.ensureProToUserMap()
  refs := make([]reporef.DatasetRef, 0, d.Root.RefsLength())
  for i := 0; i < d.Root.RefsLength(); i++ {
@@ -153,7 +163,7 @@ func (d *Dscache) ListRefs() ([]reporef.DatasetRef, error) {
 // Update modifies the dscache according to the provided action.
 func (d *Dscache) Update(act *logbook.Action) error {
  if d.IsEmpty() {
- return fmt.Errorf("dscache: cannot Update an empty dscache")
+ return ErrNoDscache
  }
 
  if act.Type != logbook.ActionMoveCursor {

dscache/dscache_test.go

@@ -1,5 +1,33 @@
 package dscache
 
-// TODO(dlong): Test LoadDscacheFromFile
-// TODO(dlong): Test SaveTo
-// TODO(dlong): Test ListRefs
+import (
+ "strings"
+ "testing"
+
+ "github.com/qri-io/qri/logbook"
+)
+
+// TODO(dlong): Test NewDscache, IsEmpty, Assign, ListRefs, Update
+
+func TestNilCallable(t *testing.T) {
+ var (
+ cache *Dscache
+ err error
+ )
+
+ if !cache.IsEmpty() {
+ t.Errorf("expected IsEmpty: got !IsEmpty")
+ }
+ if err = cache.Assign(&Dscache{}); err != ErrNoDscache {
+ t.Errorf("expected '%s': got '%s'", ErrNoDscache, err)
+ }
+ if str := cache.VerboseString(true); !strings.Contains(str, "empty dscache") {
+ t.Errorf("expected str to Contain 'empty dscache': got '%s'", str)
+ }
+ if _, err = cache.ListRefs(); err != ErrNoDscache {
+ t.Errorf("expected '%s': got '%s'", ErrNoDscache, err)
+ }
+ if err = cache.Update(&logbook.Action{}); err != ErrNoDscache {
+ t.Errorf("expected '%s': got '%s'", ErrNoDscache, err)
+ }
+}

lib/lib.go

@@ -364,7 +364,7 @@ func NewInstance(ctx context.Context, repoPath string, opts ...Option) (qri *Ins
  }
 
  if inst.dscache == nil {
- inst.dscache, err = newDscache(inst.qfs, cfg, inst.repoPath)
+ inst.dscache, err = newDscache(ctx, inst.qfs, cfg, inst.repoPath)
  if err != nil {
  return nil, fmt.Errorf("newDsache: %w", err)
  }
@@ -474,9 +474,9 @@ func newLogbook(fs qfs.Filesystem, cfg *config.Config, repoPath string) (book *l
  return logbook.NewJournal(pro.PrivKey, pro.Peername, fs, logbookPath)
 }
 
-func newDscache(fs qfs.Filesystem, cfg *config.Config, repoPath string) (*dscache.Dscache, error) {
+func newDscache(ctx context.Context, fs qfs.Filesystem, cfg *config.Config, repoPath string) (*dscache.Dscache, error) {
  dscachePath := filepath.Join(repoPath, "dscache.qfb")
- dscache := dscache.NewDscache(fs, dscachePath)
+ dscache := dscache.NewDscache(ctx, fs, dscachePath)
  return dscache, nil
 }
 

repo/buildrepo/build.go

@@ -71,7 +71,7 @@ func New(ctx context.Context, path string, cfg *config.Config) (repo.Repo, error
  return nil, err
  }
 
- cache, err := newDscache(fs, path)
+ cache, err := newDscache(ctx, fs, path)
  if err != nil {
  return nil, err
  }
@@ -153,11 +153,11 @@ func newLogbook(fs qfs.Filesystem, pro *profile.Profile, repoPath string) (book
  return logbook.NewJournal(pro.PrivKey, pro.Peername, fs, logbookPath)
 }
 
-func newDscache(fs qfs.Filesystem, repoPath string) (*dscache.Dscache, error) {
+func newDscache(ctx context.Context, fs qfs.Filesystem, repoPath string) (*dscache.Dscache, error) {
  // This seems to be a bug, the repoPath does not end in "qri" in some tests.
  if !strings.HasSuffix(repoPath, "qri") {
  repoPath = repoPath + "/qri"
  }
  dscachePath := filepath.Join(repoPath, "dscache.qfb")
- return dscache.NewDscache(fs, dscachePath), nil
+ return dscache.NewDscache(ctx, fs, dscachePath), nil
 }

repo/fs/fs_test.go

@@ -1,6 +1,7 @@
 package fsrepo
 
 import (
+ "context"
  "os"
  "path/filepath"
  "testing"
@@ -34,7 +35,8 @@ func TestRepo(t *testing.T) {
  t.Fatal(err)
  }
 
- cache := dscache.NewDscache(fs, "")
+ ctx := context.Background()
+ cache := dscache.NewDscache(ctx, fs, "")
 
  store := cafs.NewMapstore()
  r, err := NewRepo(store, fs, book, cache, pro, path)

repo/mem_repo.go

@@ -1,6 +1,8 @@
 package repo
 
 import (
+ "context"
+
  crypto "github.com/libp2p/go-libp2p-core/crypto"
  "github.com/qri-io/dataset/dsgraph"
  "github.com/qri-io/qfs"
@@ -33,13 +35,14 @@ func NewMemRepo(p *profile.Profile, store cafs.Filestore, fsys qfs.Filesystem, p
  if err != nil {
  return nil, err
  }
+ ctx := context.Background()
  return &MemRepo{
  store: store,
  filesystem: fsys,
  MemRefstore: &MemRefstore{},
  refCache: &MemRefstore{},
  logbook: book,
- dscache: dscache.NewDscache(fsys, ""),
+ dscache: dscache.NewDscache(ctx, fsys, ""),
  profile: p,
  profiles: ps,
  }, nil

repo/repo.go

@@ -65,14 +65,14 @@ type Repo interface {
  Filesystem() qfs.Filesystem
 
  // 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
+ // Dscache is a cache of datasets that have been built according to logbook
+ Dscache() *dscache.Dscache
 
  // Repos have a logbook for recording & storing operation logs
  Logbook() *logbook.Book
 
- // Dscache is a cache of datasets that have been built according to logbook
- Dscache() *dscache.Dscache
-
  // A repository must maintain profile information about the owner of this dataset.
  // The value returned by Profile() should represent the peer.
  Profile() (*profile.Profile, error)