feat: reference canonicalization

While giving users the capacity to say "me" instead of "peername" is nice, it creates a number of dangers. We need to make sure that no dataset is ever stored as `me/dataset`, as that'll collide like crazy. I'm calling this process of `me -> [peername]` _canonicalization_, because it's getting a dataset name into it's canonical form. We need to do this process as early as possible. Doing this canonicalization, however, requires knowing what the current peername is, and that's stored in a repo. So I've added a new method to the Repo interface: `ParseDatasetRef`. Implementations of repo are expected to use another new method: `repo.CanonicalizeDatasetRef` to turn any dataset reference that uses "me" (or the edge case of a blank peername) to the canonical name for the reference. While we're in there, `CanoncializeDatasetRef` also tries to proactively set the reference path using any info in the repo. This gives us a great starting point for checking when we do and don't need to connect to the network.

However, This repo requirement creates a number of headaches for RPC, so I think the next commit will refactor _when_ we canonicalize references to post-rpc points in core methods, when we're garunteed to have a repo instance.

api/handlers/context.go

@@ -20,17 +20,17 @@ const DatasetRefCtxKey QriCtxKey = "datasetRef"
 
 // DatasetRefFromReq examines the path element of a request URL
 // to
-func DatasetRefFromReq(r *http.Request) (repo.DatasetRef, error) {
+func DatasetRefFromReq(rp repo.Repo, r *http.Request) (repo.DatasetRef, error) {
  if r.URL.String() == "" || r.URL.Path == "" {
  return repo.DatasetRef{}, nil
  }
- return DatasetRefFromPath(r.URL.Path)
+ return DatasetRefFromPath(rp, r.URL.Path)
 }
 
 // DatasetRefFromPath parses a path and returns a datasetRef
-func DatasetRefFromPath(path string) (repo.DatasetRef, error) {
+func DatasetRefFromPath(r repo.Repo, path string) (repo.DatasetRef, error) {
  refstr := HTTPPathToQriPath(path)
- return repo.ParseDatasetRef(refstr)
+ return r.ParseDatasetRef(refstr)
 }
 
 // DatasetRefFromCtx extracts a Dataset reference from a given

api/handlers/context_test.go

@@ -6,9 +6,16 @@ import (
  "testing"
 
  "github.com/qri-io/qri/repo"
+ "github.com/qri-io/qri/repo/test"
 )
 
 func TestDatasetRefFromReq(t *testing.T) {
+ mr, err := test.NewTestRepo()
+ if err != nil {
+ t.Errorf("error creating test repo: %s", err.Error())
+ return
+ }
+
  cases := []struct {
  url string
  expected repo.DatasetRef
@@ -35,7 +42,7 @@ func TestDatasetRefFromReq(t *testing.T) {
  if err != nil {
  t.Error("case %d, error making request: %s", i, err)
  }
- got, err := DatasetRefFromReq(r)
+ got, err := DatasetRefFromReq(mr, r)
  if (c.err != "" && err == nil) || (err != nil && c.err != err.Error()) {
  t.Errorf("case %d, error mismatch: expected '%s' but got '%s'", i, c.err, err)
  continue

api/handlers/datasets.go

@@ -142,7 +142,7 @@ func (h *DatasetHandlers) ZipDatasetHandler(w http.ResponseWriter, r *http.Reque
 }
 
 func (h *DatasetHandlers) zipDatasetHandler(w http.ResponseWriter, r *http.Request) {
- args, err := DatasetRefFromPath(r.URL.Path[len("/export/"):])
+ args, err := DatasetRefFromPath(h.repo, r.URL.Path[len("/export/"):])
  if err != nil {
  util.WriteErrResponse(w, http.StatusBadRequest, err)
  return
@@ -176,7 +176,7 @@ func (h *DatasetHandlers) listHandler(w http.ResponseWriter, r *http.Request) {
 
 func (h *DatasetHandlers) getHandler(w http.ResponseWriter, r *http.Request) {
  res := &repo.DatasetRef{}
- args, err := DatasetRefFromPath(r.URL.Path)
+ args, err := DatasetRefFromPath(h.repo, r.URL.Path)
  if err != nil {
  util.WriteErrResponse(w, http.StatusBadRequest, err)
  return
@@ -195,7 +195,7 @@ func (h *DatasetHandlers) getHandler(w http.ResponseWriter, r *http.Request) {
 
 func (h *DatasetHandlers) peerListHandler(w http.ResponseWriter, r *http.Request) {
  p := core.ListParamsFromRequest(r)
- ref, err := DatasetRefFromPath(r.URL.Path[len("/list/"):])
+ ref, err := DatasetRefFromPath(h.repo, r.URL.Path[len("/list/"):])
  if err != nil {
  util.WriteErrResponse(w, http.StatusBadRequest, err)
  return
@@ -313,7 +313,7 @@ func (h *DatasetHandlers) initHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h *DatasetHandlers) addHandler(w http.ResponseWriter, r *http.Request) {
- ref, err := DatasetRefFromPath(r.URL.Path[len("/add/"):])
+ ref, err := DatasetRefFromPath(h.repo, r.URL.Path[len("/add/"):])
  if err != nil {
  util.WriteErrResponse(w, http.StatusBadRequest, err)
  return
@@ -324,8 +324,8 @@ func (h *DatasetHandlers) addHandler(w http.ResponseWriter, r *http.Request) {
  return
  }
 
- res := &repo.DatasetRef{}
- err = h.Add(ref, res)
+ res := repo.DatasetRef{}
+ err = h.Add(&ref, &res)
  if err != nil {
  util.WriteErrResponse(w, http.StatusInternalServerError, err)
  return
@@ -435,7 +435,7 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h *DatasetHandlers) removeHandler(w http.ResponseWriter, r *http.Request) {
- p, err := DatasetRefFromPath(r.URL.Path[len("/remove/"):])
+ p, err := DatasetRefFromPath(h.repo, r.URL.Path[len("/remove/"):])
  if err != nil {
  util.WriteErrResponse(w, http.StatusBadRequest, err)
  return

api/handlers/history.go

@@ -37,7 +37,7 @@ func (h *HistoryHandlers) LogHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func (h *HistoryHandlers) logHandler(w http.ResponseWriter, r *http.Request) {
- args, err := DatasetRefFromPath(r.URL.Path[len("/history/"):])
+ args, err := DatasetRefFromPath(h.repo, r.URL.Path[len("/history/"):])
  if err != nil {
  util.WriteErrResponse(w, http.StatusBadRequest, err)
  return

api/middleware.go

@@ -52,7 +52,7 @@ func (s *Server) addCORSHeaders(w http.ResponseWriter, r *http.Request) {
 
 func (s *Server) datasetRefMiddleware(handler http.HandlerFunc) http.HandlerFunc {
  return func(w http.ResponseWriter, r *http.Request) {
- if ref, _ := handlers.DatasetRefFromReq(r); !ref.IsEmpty() {
+ if ref, _ := handlers.DatasetRefFromReq(s.qriNode.Repo, r); !ref.IsEmpty() {
  ctx := context.WithValue(r.Context(), handlers.DatasetRefCtxKey, ref)
  r = r.WithContext(ctx)
  }

cmd/add.go

@@ -53,16 +53,17 @@ changes to qri.`,
  ErrExit(fmt.Errorf("adding datasets with --structure, --meta, or --data requires exactly 1 argument for the new dataset name"))
  }
 
+ r := getRepo(false)
  if ingest {
- ref, err := repo.ParseDatasetRef(args[0])
+ ref, err := r.ParseDatasetRef(args[0])
  ExitIfErr(err)
 
  initDataset(ref)
  return
  }
 
  for _, arg := range args {
- ref, err := repo.ParseDatasetRef(arg)
+ ref, err := r.ParseDatasetRef(arg)
  ExitIfErr(err)
 
  req, err := datasetRequests(true)
@@ -94,6 +95,7 @@ func initDataset(name repo.DatasetRef) {
  ExitIfErr(err)
 
  p := &core.InitParams{
+ Peername: name.Peername,
  Name: name.Name,
  URL: addDsURL,
  DataFilename: filepath.Base(addDsFilepath),

cmd/cmd_test.go

@@ -2,10 +2,10 @@ package cmd
 
 import (
  "bytes"
- "fmt"
  "io/ioutil"
  "os"
  "path/filepath"
+ "strings"
  "testing"
 
  "github.com/spf13/cobra"
@@ -90,7 +90,6 @@ func TestCommandsIntegration(t *testing.T) {
  os.Setenv("QRI_PATH", filepath.Join(path, "qri"))
 
  t.Log("PATH:", path)
- fmt.Println(path)
 
  commands := [][]string{
  {"help"},
@@ -100,10 +99,10 @@ func TestCommandsIntegration(t *testing.T) {
  {"profile", "set", "-f" + profileDataFilepath},
  {"config", "get"},
  {"info"},
- {"add", "-s", "--data=" + moviesFilePath, "me/movies"},
+ {"add", "--data=" + moviesFilePath, "me/movies"},
  {"add", "--data=" + movies2FilePath, "me/movies2"},
  {"list"},
- {"save", "-s", "--data=" + movies2FilePath, "-t" + "commit_1", "me/movies"},
+ {"save", "--data=" + movies2FilePath, "-t" + "commit_1", "me/movies"},
  {"log", "me/movies"},
  {"diff", "me/movies", "me/movies2", "-d", "detail"},
  {"export", "--dataset", "me/movies", "-o" + path},
@@ -116,13 +115,13 @@ func TestCommandsIntegration(t *testing.T) {
  func() {
  defer func() {
  if e := recover(); e != nil {
- t.Errorf("case %d unexpected panic executing command: %s", i, e)
+ t.Errorf("case %d unexpected panic executing command\n%s\n%s", i, strings.Join(args, " "), e)
  return
  }
  }()
  _, err := executeCommand(RootCmd, args...)
  if err != nil {
- t.Errorf("case %d unexpected error executing command: %s", i, err.Error())
+ t.Errorf("case %d unexpected error executing command\n%s\n%s", i, strings.Join(args, " "), err.Error())
  return
  }
  }()

cmd/diff.go

@@ -27,10 +27,11 @@ either by name or by their hash`,
  ErrExit(fmt.Errorf("please provide names for two datsets"))
  }
 
- leftRef, err := repo.ParseDatasetRef(args[0])
+ r := getRepo(false)
+ leftRef, err := r.ParseDatasetRef(args[0])
  ExitIfErr(err)
 
- rightRef, err := repo.ParseDatasetRef(args[1])
+ rightRef, err := r.ParseDatasetRef(args[1])
  ExitIfErr(err)
 
  req, err := datasetRequests(false)

cmd/export.go

@@ -45,7 +45,7 @@ To export everything about a dataset, use the --dataset flag.`,
  r := getRepo(false)
  req := core.NewDatasetRequests(r, nil)
 
- dsr, err := repo.ParseDatasetRef(args[0])
+ dsr, err := r.ParseDatasetRef(args[0])
  ExitIfErr(err)
 
  p := &repo.DatasetRef{

cmd/info.go

@@ -45,11 +45,9 @@ var infoCmd = &cobra.Command{
  // check to see if we're all local
  r := getRepo(false)
  for _, arg := range args {
- ref, err := repo.ParseDatasetRef(arg)
- ExitIfErr(err)
- local, err := repo.IsLocalRef(r, ref)
+ ref, err := r.ParseDatasetRef(arg)
  ExitIfErr(err)
- if !local {
+ if ref.Path == "" {
  online = true
  }
  }

cmd/list.go

@@ -42,7 +42,7 @@ qri repository.`,
  switch outformat {
  case "":
  for _, ref := range refs {
- printInfo("%s\t\t\t: %s", ref.Name, ref.Path)
+ printInfo(ref.String())
  }
  case dataset.JSONDataFormat.String():
  data, err := json.MarshalIndent(refs, "", " ")
@@ -75,7 +75,7 @@ qri repository.`,
  printInfo("%s has no datasets", args[0])
  }
  for _, ref := range refs {
- printInfo("%s\t\t: %s", ref.Name, ref.Path)
+ printInfo(ref.String())
  }
  case dataset.JSONDataFormat.String():
  data, err := json.MarshalIndent(refs, "", " ")

cmd/log.go

@@ -40,10 +40,10 @@ working backwards in time.`,
  online := false
 
  r := getRepo(false)
- ref, err := repo.ParseDatasetRef(args[0])
+ ref, err := r.ParseDatasetRef(args[0])
  ExitIfErr(err)
- local, err := repo.IsLocalRef(r, ref)
- if !local {
+
+ if ref.Path == "" {
  online = true
  }
 

cmd/remove.go

@@ -3,7 +3,6 @@ package cmd
 import (
  "fmt"
 
- "github.com/qri-io/qri/repo"
  "github.com/spf13/cobra"
 )
 
@@ -32,8 +31,9 @@ both qri & IPFS. Promise.`,
  req, err := datasetRequests(false)
  ExitIfErr(err)
 
+ r := getRepo(false)
  for _, arg := range args {
- ref, err := repo.ParseDatasetRef(arg)
+ ref, err := r.ParseDatasetRef(arg)
  ExitIfErr(err)
 
  res := false

cmd/rename.go

@@ -23,10 +23,11 @@ with it, especially if you want other people to like your datasets.`,
  ErrExit(fmt.Errorf("please provide current & new dataset names"))
  }
 
- current, err := repo.ParseDatasetRef(args[0])
+ r := getRepo(false)
+ current, err := r.ParseDatasetRef(args[0])
  ExitIfErr(err)
 
- next, err := repo.ParseDatasetRef(args[1])
+ next, err := r.ParseDatasetRef(args[1])
  ExitIfErr(err)
 
  req, err := datasetRequests(false)

cmd/save.go

@@ -51,12 +51,10 @@ collaboration are in the works. Sit tight sportsfans.`,
  ErrExit(fmt.Errorf("one of --structure, --meta or --data is required"))
  }
 
- ref, err := repo.ParseDatasetRef(args[0])
+ r := getRepo(false)
+ ref, err := r.ParseDatasetRef(args[0])
  ExitIfErr(err)
 
- // TODO - this is silly:
- ref.Peername = "me"
-
  req := core.NewDatasetRequests(getRepo(false), nil)
  save := &core.SaveParams{
  Prev: ref,

cmd/validate.go

@@ -56,7 +56,8 @@ will affect a dataset before saving changes to a dataset.`,
  )
 
  if len(args) == 1 {
- ref, err = repo.ParseDatasetRef(args[0])
+ r := getRepo(false)
+ ref, err = r.ParseDatasetRef(args[0])
  ExitIfErr(err)
  }
 
@@ -73,7 +74,7 @@ will affect a dataset before saving changes to a dataset.`,
  ExitIfErr(err)
 
  p := &core.ValidateDatasetParams{
- Name: ref.Name,
+ Ref: ref,
  // URL: addDsURL,
  DataFilename: filepath.Base(validateDsSchemaFilepath),
  }

core/datasets.go

@@ -186,8 +186,6 @@ type InitParams struct {
  Metadata io.Reader // reader of json-formatted metadata
  StructureFilename string // filename of metadata file. optional.
  Structure io.Reader // reader of json-formatted metadata
- // TODO - add support for adding via path/hash
- // DataPath datastore.Key // path to structured data
 }
 
 // Init creates a new qri dataset from a source of data
@@ -610,9 +608,8 @@ func (r *DatasetRequests) Add(ref *repo.DatasetRef, res *repo.DatasetRef) (err e
 // ValidateDatasetParams defines paremeters for dataset
 // data validation
 type ValidateDatasetParams struct {
- Name string
+ Ref repo.DatasetRef
  // URL string
- Path datastore.Key
  DataFilename string
  Data io.Reader
  Schema io.Reader
@@ -631,12 +628,9 @@ func (r *DatasetRequests) Validate(p *ValidateDatasetParams, errors *[]jsonschem
  )
 
  // if a dataset is specified, load it
- if p.Name != "" || p.Path.String() != "" {
- ref = &repo.DatasetRef{}
- err = r.Get(&repo.DatasetRef{
- Name: p.Name,
- Path: p.Path.String(),
- }, ref)
+ if p.Ref.Name != "" || p.Ref.Path != "" {
+ ref := repo.DatasetRef{}
+ err = r.Get(&p.Ref, &ref)
 
  if err != nil {
  return err

repo/fs/fs.go

@@ -154,6 +154,18 @@ func (r *Repo) SetPrivateKey(pk crypto.PrivKey) error {
  return nil
 }
 
+// ParseDatasetRef decodes a dataset reference from a string value in the context
+// of this repo, so names like me/dataset are returned as [peername]/dataset
+func (r *Repo) ParseDatasetRef(refstr string) (ref repo.DatasetRef, err error) {
+ ref, err = repo.ParseDatasetRef(refstr)
+ if err != nil {
+ return
+ }
+
+ err = repo.CanonicalizeRef(r, &ref)
+ return
+}
+
 // func (r *Repo) Peers() (map[string]*profile.Profile, error) {
 // p := map[string]*profile.Profile{}
 // data, err := ioutil.ReadFile(r.filepath(FilePeers))

repo/mem_refstore.go

@@ -41,7 +41,7 @@ func (r MemRefstore) GetRef(get DatasetRef) (ref DatasetRef, err error) {
  return
 }
 
-// DeleteName removes a name from the store
+// DeleteRef removes a name from the store
 func (r *MemRefstore) DeleteRef(del DatasetRef) error {
  refs := *r
  for i, ref := range refs {