From 23735b7b758a85eed56eda0cf1c0774cd71777d7 Mon Sep 17 00:00:00 2001 From: Dustin Long Date: Fri, 22 Mar 2019 15:59:37 -0400 Subject: [PATCH] feat(save): Save a dataset with multiple file arguments --- cmd/save.go | 14 +++--- cmd/save_test.go | 20 +++++++-- lib/datasets.go | 10 ++--- lib/datasets_test.go | 2 +- lib/file.go | 101 ++++++++++++++++++++++++++++--------------- lib/file_test.go | 2 +- 6 files changed, 97 insertions(+), 52 deletions(-) diff --git a/cmd/save.go b/cmd/save.go index e3c680d30..c988dd37e 100644 --- a/cmd/save.go +++ b/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, diff --git a/cmd/save_test.go b/cmd/save_test.go index 328f76a78..a439ddf82 100644 --- a/cmd/save_test.go +++ b/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 diff --git a/lib/datasets.go b/lib/datasets.go index 4ba96eaa2..326b61aaf 100644 --- a/lib/datasets.go +++ b/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 } diff --git a/lib/datasets_test.go b/lib/datasets_test.go index d1498bda7..5ee927881 100644 --- a/lib/datasets_test.go +++ b/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()) } diff --git a/lib/file.go b/lib/file.go index 25e11c74d..0bb26b83e 100644 --- a/lib/file.go +++ b/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,7 +136,8 @@ 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{}) @@ -120,37 +145,39 @@ func ReadDatasetFile(path string) (ds *dataset.Dataset, err error) { 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 diff --git a/lib/file_test.go b/lib/file_test.go index fb59485e2..72c655f4f 100644 --- a/lib/file_test.go +++ b/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