fix(get): Apply format to get when using a selector

Example: `qri get commit.title me/my_ds --format yaml`

Author: dustmop

api/testdata/api.snapshot

@@ -130,20 +130,7 @@ func (r *DatasetRequests) Get(p *GetParams, res *GetResult) (err error) {
 		return
 	}
 
-	if p.Selector == "" {
-		// `qri get` without a selector loads only the dataset head
-		switch p.Format {
-		case "json":
-			if p.Concise {
-				res.Bytes, err = json.Marshal(res.Dataset)
-			} else {
-				res.Bytes, err = json.MarshalIndent(res.Dataset, "", " ")
-			}
-		default:
-			res.Bytes, err = yaml.Marshal(res.Dataset)
-		}
-		return err
-	} else if p.Selector == "body" {
+	if p.Selector == "body" {
 		// `qri get body` loads the body
 		if !p.All && (p.Limit < 0 || p.Offset < 0) {
 			return fmt.Errorf("invalid limit / offset settings")
@@ -161,20 +148,36 @@ func (r *DatasetRequests) Get(p *GetParams, res *GetResult) (err error) {
 		res.Bytes = bufData
 		return err
 	} else if p.Selector == "transform.script" && ds.Transform != nil && ds.Transform.ScriptFile() != nil {
-		// accomodate two special case script file fields
+		// `qri get transform.script` loads the transform script, as a special case
 		// TODO (b5): this is a hack that should be generalized
 		res.Bytes, err = ioutil.ReadAll(ds.Transform.ScriptFile())
-		return
+		return err
 	} else if p.Selector == "viz.script" && ds.Viz != nil && ds.Viz.ScriptFile() != nil {
+		// `qri get viz.script` loads the visualization script, as a special case
 		res.Bytes, err = ioutil.ReadAll(ds.Viz.ScriptFile())
-		return
+		return err
 	} else {
-		// `qri get <selector>` loads the dataset but only returns the applicable component / field
-		value, err := base.ApplyPath(res.Dataset, p.Selector)
-		if err != nil {
-			return err
+		var value interface{}
+		if p.Selector == "" {
+			// `qri get` without a selector loads only the dataset head
+			value = res.Dataset
+		} else {
+			// `qri get <selector>` loads only the applicable component / field
+			value, err = base.ApplyPath(res.Dataset, p.Selector)
+			if err != nil {
+				return err
+			}
+		}
+		switch p.Format {
+		case "json":
+			if p.Concise {
+				res.Bytes, err = json.Marshal(value)
+			} else {
+				res.Bytes, err = json.MarshalIndent(value, "", " ")
+			}
+		default:
+			res.Bytes, err = yaml.Marshal(value)
 		}
-		res.Bytes, err = json.MarshalIndent(value, "", " ")
 		return err
 	}
 }

lib/datasets.go

@@ -1,20 +1,21 @@
 package lib
 
 import (
-	"bytes"
 	"context"
-	"encoding/csv"
 	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strconv"
+	"strings"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/ghodss/yaml"
 	"github.com/qri-io/dataset"
 	"github.com/qri-io/dataset/dsfs"
+	"github.com/qri-io/dataset/dsio"
 	"github.com/qri-io/dataset/dstest"
 	"github.com/qri-io/dsdiff"
 	"github.com/qri-io/jsonschema"
@@ -384,103 +385,118 @@ func TestDatasetRequestsGet(t *testing.T) {
 		t.Fatalf("error loading dataset: %s", err.Error())
 	}
 
+	moviesDs.OpenBodyFile(node.Repo.Filesystem())
+	moviesBody := make([]interface{}, 0)
+	moviesBodyFile := moviesDs.BodyFile()
+	reader := dsio.NewCSVReader(moviesDs.Structure, moviesBodyFile)
+	for {
+		ent, err := reader.ReadEntry()
+		if err != nil {
+			if err.Error() == "EOF" {
+				break
+			}
+			t.Fatal(err.Error())
+		}
+		moviesBody = append(moviesBody, ent.Value)
+	}
+
 	cases := []struct {
 		params *GetParams
-		res    *dataset.Dataset
-		err    string
+		expect string
 	}{
-		// TODO: probably delete some of these
-		{&GetParams{Path: "peer/ABC@abc"}, nil, "'peer/ABC@abc' is not a valid dataset reference"},
-		// repo.DatasetRef{Peername: "peer", Path: ref.Path, Name: "ABC"}.String()
-		{&GetParams{Path: fmt.Sprintf("peer/ABC@%s", ref.Path)}, nil, ""},
-		// repo.DatasetRef{Peername: "peer", Path: ref.Path, Name: "movies"}.String()
-		{&GetParams{Path: "peer/movies"}, moviesDs, ""},
-		// repo.DatasetRef{Peername: "peer", Path: ref.Path, Name: "cats"}.String()
-		// {, moviesDs, ""},
-	}
+		{&GetParams{Path: "peer/ABC@abc"}, "'peer/ABC@abc' is not a valid dataset reference"},
 
-	req := NewDatasetRequests(node, nil)
-	for i, c := range cases {
-		got := &GetResult{}
-		err := req.Get(c.params, got)
-		if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) {
-			t.Errorf("case %d error mismatch: expected: %s, got: %s", i, c.err, err)
-			continue
-		}
-		// TODO (dlong): Inspect the contents of `got`
-	}
-}
+		{&GetParams{Path: fmt.Sprintf("peer/ABC@%s", ref.Path)},
+			componentToString(setDatasetName(moviesDs, "peer/ABC"), "yaml")},
 
-func TestDatasetRequestsGetBody(t *testing.T) {
-	rc, _ := regmock.NewMockServer()
-	mr, err := testrepo.NewTestRepo(rc)
-	if err != nil {
-		t.Fatalf("error allocating test repo: %s", err.Error())
-		return
-	}
-	node, err := p2p.NewQriNode(mr, config.DefaultP2PForTesting())
-	if err != nil {
-		t.Fatal(err.Error())
-	}
+		{&GetParams{Path: "peer/movies"},
+			componentToString(setDatasetName(moviesDs, "peer/movies"), "yaml")},
 
-	moviesRef, err := mr.GetRef(repo.DatasetRef{Peername: "peer", Name: "movies"})
-	if err != nil {
-		t.Fatalf("error getting movies ref: %s", err.Error())
-	}
-	clRef, err := mr.GetRef(repo.DatasetRef{Peername: "peer", Name: "craigslist"})
-	if err != nil {
-		t.Fatalf("error getting craigslist ref: %s", err.Error())
-	}
-	sitemapRef, err := mr.GetRef(repo.DatasetRef{Peername: "peer", Name: "sitemap"})
-	if err != nil {
-		t.Fatalf("error getting sitemap ref: %s", err.Error())
-	}
+		{&GetParams{Path: "peer/movies", Format: "json"},
+			componentToString(setDatasetName(moviesDs, "peer/movies"), "json")},
 
-	cases := []struct {
-		p        *GetParams
-		resCount int
-		err      string
-	}{
-		{&GetParams{Selector: "body"}, 0, "repo: empty dataset reference"},
-		{&GetParams{Selector: "body", Format: "json", Path: moviesRef.String(), Limit: 5, Offset: 0, All: false}, 5, ""},
-		{&GetParams{Selector: "body", Format: "json", Path: moviesRef.String(), Limit: -5, Offset: -100, All: false}, 0, "invalid limit / offset settings"},
-		{&GetParams{Selector: "body", Format: "json", Path: moviesRef.String(), Limit: -5, Offset: -100, All: true}, 0, ""},
-		{&GetParams{Selector: "body", Format: "json", Path: clRef.String(), Limit: 0, Offset: 0, All: true}, 0, ""},
-		{&GetParams{Selector: "body", Format: "json", Path: clRef.String(), Limit: 2, Offset: 0, All: false}, 2, ""},
-		{&GetParams{Selector: "body", Format: "json", Path: sitemapRef.String(), Limit: 3, Offset: 0, All: false}, 3, ""},
+		{&GetParams{Path: "peer/movies", Selector: "commit"},
+			componentToString(moviesDs.Commit, "yaml")},
+
+		{&GetParams{Path: "peer/movies", Selector: "commit", Format: "json"},
+			componentToString(moviesDs.Commit, "json")},
+
+		{&GetParams{Path: "peer/movies", Selector: "commit.title"}, "initial commit\n"},
+
+		{&GetParams{Path: "peer/movies", Selector: "commit.title", Format: "json"}, "\"initial commit\""},
+
+		{&GetParams{Path: "peer/movies", Selector: "body", Format: "json"}, "[]"},
+
+		{&GetParams{Path: "", Selector: "body", Format: "json"}, "repo: empty dataset reference"},
+
+		{&GetParams{Path: "peer/movies", Selector: "body", Format: "csv"}, "title,duration\n"},
+
+		{&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			Limit: 5, Offset: 0, All: false}, bodyToString(moviesBody[:5])},
+
+		{&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			Limit: -5, Offset: -100, All: false}, "invalid limit / offset settings"},
+
+		{&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			Limit: -5, Offset: -100, All: true}, bodyToString(moviesBody)},
+
+		{&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			Limit: 0, Offset: 0, All: true}, bodyToString(moviesBody)},
+
+		{&GetParams{Path: "peer/movies", Selector: "body", Format: "json",
+			Limit: 2, Offset: 10, All: false}, bodyToString(moviesBody[10:12])},
 	}
 
 	req := NewDatasetRequests(node, nil)
 	for i, c := range cases {
 		got := &GetResult{}
-		err := req.Get(c.p, got)
-
-		if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) {
-			t.Errorf("case %d error mismatch: expected: %s, got: %s", i, c.err, err)
+		err := req.Get(c.params, got)
+		if err != nil {
+			if err.Error() != c.expect {
+				t.Errorf("case %d error mismatch: expected: %s, got: %s", i, c.expect, err)
+			}
 			continue
 		}
 
-		if got.Bytes == nil && c.resCount == 0 {
-			continue
+		result := string(got.Bytes)
+		if result != c.expect {
+			t.Errorf("case %d Bytes mismatch: expected:\n\"%s\", got:\n\"%s\"", i, c.expect, result)
 		}
+	}
+}
 
-		switch c.p.Format {
-		default:
-			// default should be json format
-			_, err := json.Marshal(got.Dataset)
-			if err != nil {
-				t.Errorf("case %d error parsing response data: %s", i, err.Error())
-				continue
-			}
-		case "csv":
-			r := csv.NewReader(bytes.NewBuffer(got.Bytes))
-			_, err := r.ReadAll()
-			if err != nil {
-				t.Errorf("case %d error parsing response data: %s", i, err.Error())
-				continue
-			}
+func setDatasetName(ds *dataset.Dataset, name string) *dataset.Dataset {
+	parts := strings.Split(name, "/")
+	ds.Peername = parts[0]
+	ds.Name = parts[1]
+	return ds
+}
+
+func componentToString(component interface{}, format string) string {
+	switch format {
+	case "json":
+		bytes, err := json.MarshalIndent(component, "", " ")
+		if err != nil {
+			return err.Error()
+		}
+		return string(bytes)
+	case "yaml":
+		bytes, err := yaml.Marshal(component)
+		if err != nil {
+			return err.Error()
 		}
+		return string(bytes)
+	default:
+		return "Unknown format"
+	}
+}
+
+func bodyToString(component interface{}) string {
+	bytes, err := json.Marshal(component)
+	if err != nil {
+		return err.Error()
 	}
+	return string(bytes)
 }
 
 func TestDatasetRequestsGetP2p(t *testing.T) {

lib/datasets_test.go

@@ -13,7 +13,7 @@ import (
 var log = golog.Logger("lib")
 
 // VersionNumber is the current version qri
-const VersionNumber = "0.7.0"
+const VersionNumber = "0.7.1-dev"
 
 // Requests defines a set of library methods
 type Requests interface {