feat(save): Save a dataset with multiple file arguments

cmd/save.go

@@ -57,7 +57,7 @@ commit message and title to the save.`,
 		},
 	}
 
-	cmd.Flags().StringVarP(&o.FilePath, "file", "f", "", "dataset data file (yaml or json)")
+	cmd.Flags().StringSliceVarP(&o.FilePaths, "file", "f", nil, "dataset or component file (yaml or json)")
 	cmd.Flags().StringVarP(&o.Title, "title", "t", "", "title of commit message for save")
 	cmd.Flags().StringVarP(&o.Message, "message", "m", "", "commit message for save")
 	cmd.Flags().StringVarP(&o.BodyPath, "body", "", "", "path to file or url of data to add as dataset contents")
@@ -78,7 +78,7 @@ type SaveOptions struct {
 	ioes.IOStreams
 
 	Ref            string
-	FilePath       string
+	FilePaths      []string
 	BodyPath       string
 	Title          string
 	Message        string
@@ -105,8 +105,10 @@ func (o *SaveOptions) Complete(f Factory, args []string) (err error) {
 	// Make all paths absolute. Especially important if we are running
 	// `qri connect` in a different terminal, and that instance is in a different directory;
 	// that instance won't correctly find the body file we want to load if it's not absolute.
-	if err := lib.AbsPath(&o.FilePath); err != nil {
-		return err
+	for i, _ := range o.FilePaths {
+		if err := lib.AbsPath(&o.FilePaths[i]); err != nil {
+			return err
+		}
 	}
 
 	if err := lib.AbsPath(&o.BodyPath); err != nil {
@@ -128,7 +130,7 @@ func (o *SaveOptions) Run() (err error) {
 	defer o.StopSpinner()
 
 	ref, err := parseCmdLineDatasetRef(o.Ref)
-	if err != nil && o.FilePath == "" {
+	if err != nil && len(o.FilePaths) == 0 {
 		return lib.NewError(lib.ErrBadArgs, "error parsing dataset reference '"+o.Ref+"'")
 	}
 
@@ -144,7 +146,7 @@ func (o *SaveOptions) Run() (err error) {
 
 	p := &lib.SaveParams{
 		Dataset:             dsp,
-		FilePath:            o.FilePath,
+		FilePaths:           o.FilePaths,
 		Private:             false,
 		Publish:             o.Publish,
 		DryRun:              o.DryRun,

cmd/save_test.go

@@ -74,9 +74,9 @@ func TestSaveValidate(t *testing.T) {
 	}
 	for i, c := range cases {
 		opt := &SaveOptions{
-			Ref:      c.ref,
-			FilePath: c.filepath,
-			BodyPath: c.bodypath,
+			Ref:       c.ref,
+			FilePaths: []string{c.filepath},
+			BodyPath:  c.bodypath,
 		}
 
 		err := opt.Validate()
@@ -168,10 +168,15 @@ func TestSaveRun(t *testing.T) {
 			continue
 		}
 
+		pathList := []string{}
+		if c.filepath != "" {
+			pathList = []string{c.filepath}
+		}
+
 		opt := &SaveOptions{
 			IOStreams:       streams,
 			Ref:             c.ref,
-			FilePath:        c.filepath,
+			FilePaths:       pathList,
 			BodyPath:        c.bodypath,
 			Title:           c.title,
 			Message:         c.message,
@@ -203,3 +208,10 @@ func TestSaveRun(t *testing.T) {
 		}
 	}
 }
+
+// TODO(dlong): Add tests for saving with multiple file compoents. Handle these cases:
+// save with a single file, which is a dataset
+// save with a single file which is only a meta component
+// save with multiple components, meta and structure and title
+// error when saving with a dataset and a meta component
+// error when saving with a zip file and a meta component

lib/datasets.go

@@ -192,8 +192,8 @@ type SaveParams struct {
 	// dataset to create. If both Dataset and FilePath are provided
 	// dataset values will override any values in the document at FilePath
 	Dataset *dataset.Dataset
-	// absolute path or URL to a dataset file or component to load
-	FilePath string
+	// absolute path or URL to the list of dataset files or components to load
+	FilePaths []string
 	// secrets for transform execution
 	Secrets map[string]string
 	// option to make dataset private. private data is not currently implimented,
@@ -230,7 +230,7 @@ func (r *DatasetRequests) Save(p *SaveParams, res *repo.DatasetRef) (err error)
 	}
 
 	ds := p.Dataset
-	if ds == nil && p.FilePath == "" {
+	if ds == nil && len(p.FilePaths) == 0 {
 		return fmt.Errorf("at least one of Dataset, FilePath is required")
 	}
 
@@ -251,9 +251,9 @@ func (r *DatasetRequests) Save(p *SaveParams, res *repo.DatasetRef) (err error)
 		ds = recall
 	}
 
-	if p.FilePath != "" {
+	if len(p.FilePaths) > 0 {
 		// TODO (b5): handle this with a qfs.Filesystem
-		dsf, err := ReadDatasetFile(p.FilePath)
+		dsf, err := ReadDatasetFiles(p.FilePaths)
 		if err != nil {
 			return err
 		}

lib/datasets_test.go

@@ -208,7 +208,7 @@ func TestDatasetRequestsSaveZip(t *testing.T) {
 
 	dsp := &dataset.Dataset{Peername: "me"}
 	res := repo.DatasetRef{}
-	err = req.Save(&SaveParams{Dataset: dsp, FilePath: "testdata/import.zip"}, &res)
+	err = req.Save(&SaveParams{Dataset: dsp, FilePaths: []string{"testdata/import.zip"}}, &res)
 	if err != nil {
 		t.Fatal(err.Error())
 	}

lib/file.go

@@ -58,51 +58,75 @@ func pathKind(path string) string {
 	return "file"
 }
 
-// ReadDatasetFile decodes a dataset document into a Dataset
-func ReadDatasetFile(path string) (ds *dataset.Dataset, err error) {
-	var (
-		resp *http.Response
-		f    *os.File
-		data []byte
-	)
+// ReadDatasetFiles decodes a dataset document into a Dataset
+func ReadDatasetFiles(pathList []string) (*dataset.Dataset, error) {
+	// If there's only a single file provided, read it and return the dataset.
+	if len(pathList) == 1 {
+		ds, _, err := ReadSingleFile(pathList[0])
+		return ds, err
+	}
+
+	// If there's multiple files provided, read each one and merge them. Any exclusive
+	// component is an error, any component showing up multiple times is an error.
+	foundKinds := make(map[string]bool)
+	ds := dataset.Dataset{}
+	for _, p := range pathList {
+		component, kind, err := ReadSingleFile(p)
+		if err != nil {
+			return nil, err
+		}
+
+		if kind == "zip" || kind == "ds" {
+			return nil, fmt.Errorf("")
+		}
+		if _, ok := foundKinds[kind]; ok {
+			return nil, fmt.Errorf("conflict, multiple components of kind %s", kind)
+		}
+		foundKinds[kind] = true
+
+		ds.Assign(component)
+	}
 
-	ds = &dataset.Dataset{}
+	return &ds, nil
+}
 
+func ReadSingleFile(path string) (*dataset.Dataset, string, error) {
+	ds := dataset.Dataset{}
 	switch pathKind(path) {
 	case "http":
 		// currently the only supported type of file url is a zip archive
-		resp, err = http.Get(path)
+		resp, err := http.Get(path)
 		if err != nil {
-			return
+			return nil, "", err
 		}
-		data, err = ioutil.ReadAll(resp.Body)
+		data, err := ioutil.ReadAll(resp.Body)
 		if err != nil {
-			return
+			return nil, "", err
 		}
 		resp.Body.Close()
-		err = dsutil.UnzipDatasetBytes(data, ds)
-		return
+		err = dsutil.UnzipDatasetBytes(data, &ds)
+		return &ds, "zip", nil
 
 	case "ipfs":
-		return nil, fmt.Errorf("reading dataset files from IPFS currently unsupported")
+		return nil, "", fmt.Errorf("reading dataset files from IPFS currently unsupported")
 
 	case "file":
-		f, err = os.Open(path)
+		f, err := os.Open(path)
 		if err != nil {
-			return
+			return nil, "", err
 		}
 
 		fileExt := strings.ToLower(filepath.Ext(path))
 		switch fileExt {
 		case ".yaml", ".yml":
-			data, err = ioutil.ReadAll(f)
+			data, err := ioutil.ReadAll(f)
 			if err != nil {
-				return
+				return nil, "", err
 			}
 
 			fields := make(map[string]interface{})
 			if err = yaml.Unmarshal(data, fields); err != nil {
-				return
+				return nil, "", err
 			}
 
 			// TODO (b5): temp hack to deal with terrible interaction with fill_struct,
@@ -112,45 +136,48 @@ func ReadDatasetFile(path string) (ds *dataset.Dataset, err error) {
 				fields["structure"] = toMapIface(sti)
 			}
 
-			err = fillDatasetOrComponent(fields, path, ds)
+			kind, err := fillDatasetOrComponent(fields, path, &ds)
+			return &ds, kind, err
 
 		case ".json":
 			fields := make(map[string]interface{})
 			if err = json.NewDecoder(f).Decode(&fields); err != nil {
 				if strings.HasPrefix(err.Error(), "json: cannot unmarshal array") {
 					err = fmt.Errorf("json has top-level type \"array\", cannot be a dataset file")
 				}
-				return
+				return nil, "", err
 			}
-			err = fillDatasetOrComponent(fields, path, ds)
+			kind, err := fillDatasetOrComponent(fields, path, &ds)
+			return &ds, kind, err
 
 		case ".zip":
-			data, err = ioutil.ReadAll(f)
+			data, err := ioutil.ReadAll(f)
 			if err != nil {
-				return
+				return nil, "", err
 			}
-			err = dsutil.UnzipDatasetBytes(data, ds)
-			return
+			err = dsutil.UnzipDatasetBytes(data, &ds)
+			return &ds, "zip", err
 
 		case ".star":
 			// starlark files are assumed to be a transform script with no additional
 			// tranform component details:
 			ds.Transform = &dataset.Transform{ScriptPath: path}
 			ds.Transform.SetScriptFile(qfs.NewMemfileReader("transform.star", f))
-			return
+			return &ds, "tf", nil
 
 		case ".html":
 			// html files are assumped to be a viz script with no additional viz
 			// component details
 			ds.Viz = &dataset.Viz{ScriptPath: path}
 			ds.Viz.SetScriptFile(qfs.NewMemfileReader("viz.html", f))
-			return
+			return &ds, "vz", nil
 
 		default:
-			return nil, fmt.Errorf("error, unrecognized file extension: \"%s\"", fileExt)
+			return nil, "", fmt.Errorf("error, unrecognized file extension: \"%s\"", fileExt)
 		}
+	default:
+		return nil, "", fmt.Errorf("error, unknown path kind: \"%s\"", pathKind(path))
 	}
-	return
 }
 
 func toMapIface(i map[interface{}]interface{}) map[string]interface{} {
@@ -174,29 +201,33 @@ func toMapIface(i map[interface{}]interface{}) map[string]interface{} {
 	return mapi
 }
 
-func fillDatasetOrComponent(fields map[string]interface{}, path string, ds *dataset.Dataset) (err error) {
+func fillDatasetOrComponent(fields map[string]interface{}, path string, ds *dataset.Dataset) (string, error) {
 	var target interface{}
 	target = ds
+	kind := "ds"
 
 	if kindStr, ok := fields["qri"].(string); ok && len(kindStr) > 3 {
 		switch kindStr[:2] {
 		case "md":
 			ds.Meta = &dataset.Meta{}
 			target = ds.Meta
+			kind = "md"
 		case "cm":
 			ds.Commit = &dataset.Commit{}
 			target = ds.Commit
+			kind = "cm"
 		case "st":
 			ds.Structure = &dataset.Structure{}
 			target = ds.Structure
+			kind = "st"
 		}
 	}
 
-	if err = fill.Struct(fields, target); err != nil {
-		return err
+	if err := fill.Struct(fields, target); err != nil {
+		return "", err
 	}
 	absDatasetPaths(path, ds)
-	return nil
+	return kind, nil
 }
 
 // absDatasetPaths converts any relative filepath references in a Dataset to

lib/file_test.go

@@ -74,7 +74,7 @@ func TestReadDatasetFile(t *testing.T) {
 	}
 
 	for i, c := range cases {
-		got, err := ReadDatasetFile(c.path)
+		got, err := ReadDatasetFiles([]string{c.path})
 		if err != nil {
 			t.Errorf("case %d %s unexpected error: %s", i, c.description, err.Error())
 			continue