fix(get): Get with a path will ignore fsi, to get old versions

api/datasets.go

@@ -282,7 +282,7 @@ func (h *DatasetHandlers) listHandler(w http.ResponseWriter, r *http.Request) {
 // otherwise, resolve the peername and proceed as normal
 func (h *DatasetHandlers) getHandler(w http.ResponseWriter, r *http.Request) {
 	p := lib.GetParams{
-		Path:   HTTPPathToQriPath(r.URL.Path),
+		Ref: HTTPPathToQriPath(r.URL.Path),
 	}
 	res := lib.GetResult{}
 	err := h.Get(&p, &res)
@@ -582,7 +582,7 @@ func getParamsFromRequest(r *http.Request, readOnly bool, path string) (*lib.Get
 	}
 
 	p := &lib.GetParams{
-		Path:     path,
+		Ref:      path,
 		Format:   format,
 		Selector: "body",
 		Limit:    listParams.Limit,
@@ -659,7 +659,7 @@ func (h DatasetHandlers) bodyHandler(w http.ResponseWriter, r *http.Request) {
 
 func (h DatasetHandlers) statsHandler(w http.ResponseWriter, r *http.Request) {
 	p := lib.GetParams{
-		Path:     HTTPPathToQriPath(r.URL.Path[len("/stats/"):]),
+		Ref:      HTTPPathToQriPath(r.URL.Path[len("/stats/"):]),
 		Selector: "stats",
 	}
 	res := lib.GetResult{}

api/fsi.go

@@ -165,7 +165,7 @@ func (h *FSIHandlers) initHandler(routePrefix string) http.HandlerFunc {
 		// Get code taken
 		// taken from ./root.go
 		gp := lib.GetParams{
-			Path:   name,
+			Ref: name,
 		}
 		res := lib.GetResult{}
 		err := h.dsm.Get(&gp, &res)

api/root.go

@@ -53,7 +53,7 @@ func (mh *RootHandler) Handler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	p := lib.GetParams{
-		Path:   ref.String(),
+		Ref: ref.String(),
 	}
 	res := lib.GetResult{}
 	err := mh.dsh.Get(&p, &res)

cmd/fsi_integration_test.go

@@ -731,6 +731,68 @@ run ` + "`qri save`" + ` to commit this dataset
 	}
 }
 
+// Test that get for a previous version works for checked out datasets
+func TestGetPreviousVersionExplicitPath(t *testing.T) {
+	run := NewFSITestRunner(t, "qri_test_get_prev_version")
+	defer run.Delete()
+
+	// First version has only a body
+	output := run.MustExec(t, "qri save --body=testdata/movies/body_two.json me/get_ver")
+	ref1 := parseRefFromSave(output)
+
+	// Add a meta
+	output = run.MustExec(t, "qri save --file=testdata/movies/meta_override.yaml me/get_ver")
+	_ = parseRefFromSave(output)
+
+	// Modify the body
+	output = run.MustExec(t, "qri save --body=testdata/movies/body_four.json me/get_ver")
+	ref3 := parseRefFromSave(output)
+
+	// Change the meta
+	output = run.MustExec(t, "qri save --file=testdata/movies/meta_another.yaml me/get_ver")
+	_ = parseRefFromSave(output)
+
+	run.ChdirToRoot()
+
+	// Checkout the newly created dataset.
+	run.MustExec(t, "qri checkout me/get_ver")
+
+	// Get meta from an old reference
+	output = run.MustExec(t, fmt.Sprintf("qri get meta %s", ref1))
+	expect := `null
+
+`
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Errorf("get mismatch (-want +got):\n%s", diff)
+	}
+
+	// Get meta from another reference
+	output = run.MustExec(t, fmt.Sprintf("qri get meta %s", ref3))
+	expect = `qri: md:0
+title: different title
+
+`
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Errorf("get mismatch (-want +got):\n%s", diff)
+	}
+
+	// Get body from an old reference
+	output = run.MustExec(t, fmt.Sprintf("qri get body %s", ref1))
+	expect = `[["Avatar",178],["Pirates of the Caribbean: At World's End",169]]
+`
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Errorf("get mismatch (-want +got):\n%s", diff)
+	}
+
+	// Get body from another reference
+	output = run.MustExec(t, fmt.Sprintf("qri get body %s", ref3))
+	expect = `[["Avatar",178],["Pirates of the Caribbean: At World's End",169],["Spectre",148],["The Dark Knight Rises",164]]
+`
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Errorf("get mismatch (-want +got):\n%s", diff)
+	}
+}
+
 // Test restoring previous version
 func TestRestorePreviousVersion(t *testing.T) {
 	run := NewFSITestRunner(t, "qri_test_restore_prev_version")

cmd/get.go

@@ -131,7 +131,7 @@ func (o *GetOptions) Run() (err error) {
 	// convert Page and PageSize to Limit and Offset
 	page := util.NewPage(o.Page, o.PageSize)
 	p := lib.GetParams{
-		Path:         o.Refs.Ref(),
+		Ref:          o.Refs.Ref(),
 		Selector:     o.Selector,
 		Format:       o.Format,
 		FormatConfig: fc,

lib/datasets.go

@@ -202,8 +202,8 @@ func (r *DatasetRequests) ListRawRefs(p *ListParams, text *string) (err error) {
 
 // GetParams defines parameters for looking up the body of a dataset
 type GetParams struct {
-	// Path to get, this will often be a dataset reference like me/dataset
-	Path string
+	// Ref to get, for example a dataset reference like me/dataset
+	Ref string
 
 	// read from a filesystem link instead of stored version
 	Format       string
@@ -223,7 +223,7 @@ type GetResult struct {
 }
 
 // Get retrieves datasets and components for a given reference. If p.Ref is provided, it is
-// used to load the dataset, otherwise p.Path is parsed to create a reference. The
+// used to load the dataset, otherwise p.Ref is parsed to create a reference. The
 // dataset will be loaded from the local repo if available, or by asking peers for it.
 // Using p.Selector will control what components are returned in res.Bytes. The default,
 // a blank selector, will also fill the entire dataset at res.Data. If the selector is "body"
@@ -236,18 +236,19 @@ func (r *DatasetRequests) Get(p *GetParams, res *GetResult) (err error) {
 	ctx := context.TODO()
 
 	// Check if the dataset ref uses bad-case characters, show a warning.
-	if _, err := dsref.Parse(p.Path); err == dsref.ErrBadCaseName {
+	dr, err := dsref.Parse(p.Ref)
+	if err == dsref.ErrBadCaseName {
 		log.Error(dsref.ErrBadCaseShouldRename)
 	}
 
-	ref, err := base.ToDatasetRef(p.Path, r.node.Repo, true)
+	ref, err := base.ToDatasetRef(p.Ref, r.node.Repo, true)
 	if err != nil {
-		log.Debugf("Get dataset, base.ToDatasetRef %q failed, error: %s", p.Path, err)
+		log.Debugf("Get dataset, base.ToDatasetRef %q failed, error: %s", p.Ref, err)
 		return err
 	}
 
 	var ds *dataset.Dataset
-	if ref.FSIPath != "" {
+	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)
@@ -282,7 +283,7 @@ func (r *DatasetRequests) Get(p *GetParams, res *GetResult) (err error) {
 		}
 
 		var bufData []byte
-		if ref.FSIPath != "" {
+		if dr.Path == "" && ref.FSIPath != "" {
 			// 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.
@@ -640,7 +641,7 @@ func (r *DatasetRequests) Save(p *SaveParams, res *reporef.DatasetRef) (err erro
 // See this issue: https://github.com/qri-io/qri/issues/1132
 func (r *DatasetRequests) nameIsInUse(ref dsref.Ref) bool {
 	param := GetParams{
-		Path: ref.Alias(),
+		Ref: ref.Alias(),
 	}
 	res := GetResult{}
 	err := r.Get(&param, &res)

lib/datasets_test.go

@@ -420,78 +420,78 @@ func TestDatasetRequestsGet(t *testing.T) {
 		expect      string
 	}{
 		{"invalid peer name",
-			&GetParams{Path: "peer/ABC@abc"}, "'peer/ABC@abc' is not a valid dataset reference"},
+			&GetParams{Ref: "peer/ABC@abc"}, "'peer/ABC@abc' is not a valid dataset reference"},
 
 		{"peername without path",
-			&GetParams{Path: "peer/movies"},
+			&GetParams{Ref: "peer/movies"},
 			componentToString(setDatasetName(moviesDs, "peer/movies"), "yaml")},
 
 		{"peername with path",
-			&GetParams{Path: fmt.Sprintf("peer/movies@%s", ref.Path)},
+			&GetParams{Ref: fmt.Sprintf("peer/movies@%s", ref.Path)},
 			componentToString(setDatasetName(moviesDs, "peer/movies"), "yaml")},
 
 		{"peername as json format",
-			&GetParams{Path: "peer/movies", Format: "json"},
+			&GetParams{Ref: "peer/movies", Format: "json"},
 			componentToString(setDatasetName(moviesDs, "peer/movies"), "json")},
 
 		{"commit component",
-			&GetParams{Path: "peer/movies", Selector: "commit"},
+			&GetParams{Ref: "peer/movies", Selector: "commit"},
 			componentToString(moviesDs.Commit, "yaml")},
 
 		{"commit component as json format",
-			&GetParams{Path: "peer/movies", Selector: "commit", Format: "json"},
+			&GetParams{Ref: "peer/movies", Selector: "commit", Format: "json"},
 			componentToString(moviesDs.Commit, "json")},
 
 		{"title field of commit component",
-			&GetParams{Path: "peer/movies", Selector: "commit.title"}, "initial commit\n"},
+			&GetParams{Ref: "peer/movies", Selector: "commit.title"}, "initial commit\n"},
 
 		{"title field of commit component as json",
-			&GetParams{Path: "peer/movies", Selector: "commit.title", Format: "json"},
+			&GetParams{Ref: "peer/movies", Selector: "commit.title", Format: "json"},
 			"\"initial commit\""},
 
 		{"title field of commit component as yaml",
-			&GetParams{Path: "peer/movies", Selector: "commit.title", Format: "yaml"},
+			&GetParams{Ref: "peer/movies", Selector: "commit.title", Format: "yaml"},
 			"initial commit\n"},
 
 		{"title field of commit component as mispelled format",
-			&GetParams{Path: "peer/movies", Selector: "commit.title", Format: "jason"},
+			&GetParams{Ref: "peer/movies", Selector: "commit.title", Format: "jason"},
 			"unknown format: \"jason\""},
 
 		{"body as json",
-			&GetParams{Path: "peer/movies", Selector: "body", Format: "json"}, "[]"},
+			&GetParams{Ref: "peer/movies", Selector: "body", Format: "json"}, "[]"},
 
 		{"dataset empty",
-			&GetParams{Path: "", Selector: "body", Format: "json"}, "repo: empty dataset reference"},
+			&GetParams{Ref: "", Selector: "body", Format: "json"}, "repo: empty dataset reference"},
 
 		{"body as csv",
-			&GetParams{Path: "peer/movies", Selector: "body", Format: "csv"}, "title,duration\n"},
+			&GetParams{Ref: "peer/movies", Selector: "body", Format: "csv"}, "title,duration\n"},
 
 		{"body with limit and offfset",
-			&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			&GetParams{Ref: "peer/movies", Selector: "body", Format: "json",
 				Limit: 5, Offset: 0, All: false}, bodyToString(moviesBody[:5])},
 
 		{"body with invalid limit and offset",
-			&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			&GetParams{Ref: "peer/movies", Selector: "body", Format: "json",
 				Limit: -5, Offset: -100, All: false}, "invalid limit / offset settings"},
 
 		{"body with all flag ignores invalid limit and offset",
-			&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			&GetParams{Ref: "peer/movies", Selector: "body", Format: "json",
 				Limit: -5, Offset: -100, All: true}, bodyToString(moviesBody)},
 
 		{"body with all flag",
-			&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			&GetParams{Ref: "peer/movies", Selector: "body", Format: "json",
 				Limit: 0, Offset: 0, All: true}, bodyToString(moviesBody)},
 
 		{"body with limit and non-zero offset",
-			&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			&GetParams{Ref: "peer/movies", Selector: "body", Format: "json",
 				Limit: 2, Offset: 10, All: false}, bodyToString(moviesBody[10:12])},
 
 		{"head non-pretty json",
-			&GetParams{Path: "peer/movies", Format: "json", FormatConfig: nonprettyJSONConfig},
+			&GetParams{Ref: "peer/movies", Format: "json", FormatConfig: nonprettyJSONConfig},
 			componentToString(setDatasetName(moviesDs, "peer/movies"), "non-pretty json")},
 
 		{"body pretty json",
-			&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			&GetParams{Ref: "peer/movies", Selector: "body", Format: "json",
 				FormatConfig: prettyJSONConfig, Limit: 3, Offset: 0, All: false},
 			bodyToPrettyString(moviesBody[:3])},
 	}
@@ -598,7 +598,7 @@ func TestDatasetRequestsGetP2p(t *testing.T) {
 
 			dsr := NewDatasetRequests(node, nil)
 			got := &GetResult{}
-			err = dsr.Get(&GetParams{Path: ref.String()}, got)
+			err = dsr.Get(&GetParams{Ref: ref.String()}, got)
 			if err != nil {
 				t.Errorf("error listing dataset for %s: %s", ref.Name, err.Error())
 			}