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