From c01a4bf33e1d9edf17a55b16d01954816ce54a50 Mon Sep 17 00:00:00 2001 From: Dustin Long Date: Thu, 14 Jan 2021 19:03:12 -0500 Subject: [PATCH] feat(apply): Apply command, and --apply flag for save --- api/api.go | 3 + api/datasets.go | 1 + api/transform.go | 38 ++++++++++ base/body.go | 7 +- base/transform_apply.go | 4 + cmd/apply.go | 113 ++++++++++++++++++++++++++++ cmd/apply_test.go | 84 +++++++++++++++++++++ cmd/cmd_test.go | 14 ++-- cmd/factory.go | 1 + cmd/factory_test.go | 5 ++ cmd/qri.go | 9 +++ cmd/save.go | 55 ++++++++++---- cmd/save_test.go | 62 +++++++++++++-- lib/datasets.go | 14 +++- lib/datasets_test.go | 83 ++++++++++++++++++++ lib/integration_test.go | 1 + lib/test_runner_test.go | 19 +++++ lib/testdata/cities_2/add_city.star | 4 + lib/transform.go | 102 +++++++++++++++++++++++++ lib/transform_test.go | 44 +++++++++++ 20 files changed, 627 insertions(+), 36 deletions(-) create mode 100644 api/transform.go create mode 100644 cmd/apply.go create mode 100644 cmd/apply_test.go create mode 100644 lib/testdata/cities_2/add_city.star create mode 100644 lib/transform.go create mode 100644 lib/transform_test.go diff --git a/api/api.go b/api/api.go index d1198cb44..cc4188fe5 100644 --- a/api/api.go +++ b/api/api.go @@ -204,6 +204,9 @@ func NewServerRoutes(s Server) *http.ServeMux { sqlh := NewSQLHandlers(s.Instance, cfg.API.ReadOnly) m.Handle("/sql", s.middleware(sqlh.QueryHandler("/sql"))) + tfh := NewTransformHandlers(s.Instance) + m.Handle("/apply", s.middleware(tfh.ApplyHandler("/apply"))) + if !cfg.API.DisableWebui { m.Handle("/webui", s.middleware(WebuiHandler)) } diff --git a/api/datasets.go b/api/datasets.go index 32725e8a7..29cf39265 100644 --- a/api/datasets.go +++ b/api/datasets.go @@ -482,6 +482,7 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) { p := &lib.SaveParams{ Ref: ref.AliasString(), Dataset: ds, + Apply: r.FormValue("apply") == "true", Private: r.FormValue("private") == "true", Force: r.FormValue("force") == "true", ShouldRender: !(r.FormValue("no_render") == "true"), diff --git a/api/transform.go b/api/transform.go new file mode 100644 index 000000000..d6859d997 --- /dev/null +++ b/api/transform.go @@ -0,0 +1,38 @@ +package api + +import ( + "encoding/json" + "net/http" + + "github.com/qri-io/qri/api/util" + "github.com/qri-io/qri/lib" +) + +// TransformHandlers connects HTTP requests to the TransformMethods subsystem +type TransformHandlers struct { + *lib.TransformMethods +} + +// NewTransformHandlers constructs a TrasnformHandlers struct +func NewTransformHandlers(inst *lib.Instance) TransformHandlers { + return TransformHandlers{TransformMethods: lib.NewTransformMethods(inst)} +} + +// ApplyHandler is an HTTP handler function for executing a transform script +func (h TransformHandlers) ApplyHandler(prefix string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + p := lib.ApplyParams{} + if err := json.NewDecoder(r.Body).Decode(&p); err != nil { + util.WriteErrResponse(w, http.StatusBadRequest, err) + return + } + + res := lib.ApplyResult{} + if err := h.TransformMethods.Apply(&p, &res); err != nil { + util.WriteErrResponse(w, http.StatusBadRequest, err) + return + } + + util.WriteResponse(w, res) + } +} diff --git a/base/body.go b/base/body.go index c961a9a61..1ed78ef27 100644 --- a/base/body.go +++ b/base/body.go @@ -3,6 +3,7 @@ package base import ( "bytes" "encoding/json" + "errors" "fmt" "io/ioutil" @@ -11,6 +12,9 @@ import ( "github.com/qri-io/qfs" ) +// ErrNoBodyToInline is an error returned when a dataset has no body for inlining +var ErrNoBodyToInline = errors.New("no body to inline") + // ReadBody grabs some or all of a dataset's body, writing an output in the desired format func ReadBody(ds *dataset.Dataset, format dataset.DataFormat, fcfg dataset.FormatConfig, limit, offset int, all bool) (data []byte, err error) { if ds == nil { @@ -78,8 +82,7 @@ func ReadEntries(reader dsio.EntryReader) (interface{}, error) { func InlineJSONBody(ds *dataset.Dataset) error { file := ds.BodyFile() if file == nil { - log.Error("no body file") - return fmt.Errorf("no response body file") + return ErrNoBodyToInline } if ds.Structure.Format == dataset.JSONDataFormat.String() { diff --git a/base/transform_apply.go b/base/transform_apply.go index 17ebb8878..678d27340 100644 --- a/base/transform_apply.go +++ b/base/transform_apply.go @@ -35,6 +35,10 @@ func TransformApply( head *dataset.Dataset ) + if target.Transform == nil || target.Transform.ScriptFile() == nil { + return errors.New("apply requires a transform with script file") + } + if ds.Name != "" { head, err = loader(ctx, fmt.Sprintf("%s/%s", pro.Peername, ds.Name)) if errors.Is(err, dsref.ErrRefNotFound) || errors.Is(err, dsref.ErrNoHistory) { diff --git a/cmd/apply.go b/cmd/apply.go new file mode 100644 index 000000000..78b2ebf78 --- /dev/null +++ b/cmd/apply.go @@ -0,0 +1,113 @@ +package cmd + +import ( + "encoding/json" + "errors" + "path/filepath" + "strings" + + "github.com/qri-io/dataset" + "github.com/qri-io/ioes" + "github.com/qri-io/qri/lib" + "github.com/qri-io/qri/repo" + "github.com/spf13/cobra" +) + +// NewApplyCommand creates a new `qri apply` cobra command for applying transformations +func NewApplyCommand(f Factory, ioStreams ioes.IOStreams) *cobra.Command { + o := &ApplyOptions{IOStreams: ioStreams} + cmd := &cobra.Command{ + Use: "apply", + Short: "apply a transform to a dataset", + Long: `Apply runs a transform script. The result of the transform is displayed after +the command completes. + +Nothing is saved in the user's repository.`, + Example: ` # Apply a transform and display the output: + $ qri apply --script transform.star`, + Annotations: map[string]string{ + "group": "dataset", + }, + RunE: func(cmd *cobra.Command, args []string) error { + if err := o.Complete(f, args); err != nil { + return err + } + return o.Run() + }, + } + + cmd.Flags().StringVar(&o.FilePath, "file", "", "path of transform script file") + cmd.MarkFlagRequired("file") + cmd.Flags().StringSliceVar(&o.Secrets, "secrets", nil, "transform secrets as comma separated key,value,key,value,... sequence") + + return cmd +} + +// ApplyOptions encapsulates state for the apply command +type ApplyOptions struct { + ioes.IOStreams + + Refs *RefSelect + FilePath string + Secrets []string + + TransformMethods *lib.TransformMethods +} + +// Complete adds any missing configuration that can only be added just before calling Run +func (o *ApplyOptions) Complete(f Factory, args []string) (err error) { + if o.TransformMethods, err = f.TransformMethods(); err != nil { + return err + } + if o.Refs, err = GetCurrentRefSelect(f, args, -1, nil); err != nil { + // This error will be handled during validation + if err != repo.ErrEmptyRef { + return err + } + err = nil + } + o.FilePath, err = filepath.Abs(o.FilePath) + if err != nil { + return err + } + return nil +} + +// Run executes the apply command +func (o *ApplyOptions) Run() error { + printRefSelect(o.ErrOut, o.Refs) + + var err error + + if !strings.HasSuffix(o.FilePath, ".star") { + return errors.New("only transform scripts are supported by --file") + } + + tf := dataset.Transform{ + ScriptPath: o.FilePath, + } + + if len(o.Secrets) > 0 { + tf.Secrets, err = parseSecrets(o.Secrets...) + if err != nil { + return err + } + } + + params := lib.ApplyParams{ + Refstr: o.Refs.Ref(), + Transform: &tf, + ScriptOutput: o.Out, + } + res := lib.ApplyResult{} + if err = o.TransformMethods.Apply(¶ms, &res); err != nil { + return err + } + + data, err := json.MarshalIndent(res.Data, "", " ") + if err != nil { + return err + } + printSuccess(o.Out, string(data)) + return nil +} diff --git a/cmd/apply_test.go b/cmd/apply_test.go new file mode 100644 index 000000000..16e4d7843 --- /dev/null +++ b/cmd/apply_test.go @@ -0,0 +1,84 @@ +package cmd + +import ( + "strings" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestTransformApply(t *testing.T) { + run := NewTestRunner(t, "test_peer_transform_apply", "qri_test_transform_apply") + defer run.Delete() + + // Apply a transform which makes a body + output := run.MustExec(t, "qri apply --file testdata/movies/tf_one_movie.star") + expectContains := ` "body": [ + [ + "Spectre", + 148 + ] + ],` + if !strings.Contains(output, expectContains) { + t.Errorf("contents mismatch, want: %s, got: %s", expectContains, output) + } + + // Save a first version with a normal body + run.MustExec(t, "qri save --body testdata/movies/body_ten.csv me/movies") + + // Apply a transform which sets a meta on the existing dataset + output = run.MustExec(t, "qri apply --file testdata/movies/tf_set_meta.star me/movies") + expectContains = `"title": "Did Set Title"` + if !strings.Contains(output, expectContains) { + t.Errorf("contents mismatch, want: %s, got: %s", expectContains, output) + } +} + +func TestApplyRefNotFound(t *testing.T) { + run := NewTestRunner(t, "test_peer_transform_apply", "qri_test_transform_apply") + defer run.Delete() + + // Error to apply a transform using a dataset ref that doesn't exist. + err := run.ExecCommand("qri apply --file testdata/movies/tf_one_movie.star me/not_found") + if err == nil { + t.Errorf("error expected, did not get one") + } + expectErr := `reference not found` + if diff := cmp.Diff(expectErr, err.Error()); diff != "" { + t.Errorf("result mismatch (-want +got):%s\n", diff) + } +} + +func TestApplyMetaOnly(t *testing.T) { + run := NewTestRunner(t, "test_peer_apply_meta_only", "qri_test_apply_meta_only") + defer run.Delete() + + // Apply a transform which sets a meta to the existing dataset + output := run.MustExec(t, "qri apply --file testdata/movies/tf_set_meta.star") + expectContains := `"title": "Did Set Title"` + if !strings.Contains(output, expectContains) { + t.Errorf("contents mismatch, want: %s, got: %s", expectContains, output) + } +} + +func TestApplyModifyBody(t *testing.T) { + run := NewTestRunner(t, "test_peer_apply_mod_body", "qri_test_apply_mod_body") + defer run.Delete() + + // Save two versions, the second of which uses get_body in a transformation + run.MustExec(t, "qri save --body=testdata/movies/body_two.json me/test_ds") + output := run.MustExec(t, "qri apply --file=testdata/movies/tf_add_one.star me/test_ds") + expectContains := `"body": [ + [ + "Avatar", + 179 + ], + [ + "Pirates of the Caribbean: At World's End", + 170 + ] + ],` + if !strings.Contains(output, expectContains) { + t.Errorf("contents mismatch, want: %s, got: %s", expectContains, output) + } +} diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index c70094dc0..274da3210 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -301,7 +301,7 @@ func TestSaveThenOverrideTransform(t *testing.T) { // Save a version, then save another with a transform run.MustExec(t, "qri save --file=testdata/movies/ds_ten.yaml me/test_ds") - run.MustExec(t, "qri save --file=testdata/movies/tf.star me/test_ds") + run.MustExec(t, "qri save --apply --file=testdata/movies/tf.star me/test_ds") // Read head from the dataset that was saved, as json string. dsPath := run.GetPathForDataset(t, 0) @@ -343,7 +343,7 @@ func TestSaveThenOverrideMetaAndTransformAndViz(t *testing.T) { // Save a version, then save another with three components at once run.MustExec(t, "qri save --file=testdata/movies/ds_ten.yaml me/test_ds") - run.MustExec(t, "qri save --file=testdata/movies/meta_override.yaml --file=testdata/movies/tf.star --file=testdata/template.html me/test_ds") + run.MustExec(t, "qri save --apply --file=testdata/movies/meta_override.yaml --file=testdata/movies/tf.star --file=testdata/template.html me/test_ds") // Read head from the dataset that was saved, as json string. dsPath := run.GetPathForDataset(t, 0) @@ -404,14 +404,14 @@ func TestSaveTransformWithoutChanges(t *testing.T) { defer run.Delete() // Save a version, then another with no changes - run.MustExec(t, "qri save --file=testdata/movies/tf_123.star me/test_ds") + run.MustExec(t, "qri save --apply --file=testdata/movies/tf_123.star me/test_ds") errOut := run.GetCommandErrOutput() if !strings.Contains(errOut, "setting body") { t.Errorf("expected ErrOutput to contain print statement from transform script. errOutput:\n%s", errOut) } - err := run.ExecCommand("qri save --file=testdata/movies/tf_123.star me/test_ds") + err := run.ExecCommand("qri save --apply --file=testdata/movies/tf_123.star me/test_ds") expect := `error saving: no changes` if err == nil { t.Fatalf("expected error: did not get one") @@ -432,7 +432,7 @@ func TestTransformUsingGetBodyAndSetBody(t *testing.T) { // Save two versions, the second of which uses get_body in a transformation run.MustExec(t, "qri save --body=testdata/movies/body_two.json me/test_ds") - run.MustExec(t, "qri save --file=testdata/movies/tf_add_one.star me/test_ds") + run.MustExec(t, "qri save --apply --file=testdata/movies/tf_add_one.star me/test_ds") // Read body from the dataset that was created with the transform dsPath := run.GetPathForDataset(t, 0) @@ -455,10 +455,10 @@ func TestSaveTransformModifiedButSameBody(t *testing.T) { defer run.Delete() // Save a version - run.MustExec(t, "qri save --file=testdata/movies/tf_123.star me/test_ds") + run.MustExec(t, "qri save --apply --file=testdata/movies/tf_123.star me/test_ds") // Save another version with a modified transform that produces the same body - err := run.ExecCommand("qri save --file=testdata/movies/tf_modified.star me/test_ds") + err := run.ExecCommand("qri save --apply --file=testdata/movies/tf_modified.star me/test_ds") if err != nil { t.Errorf("unexpected error: %q", err) diff --git a/cmd/factory.go b/cmd/factory.go index 8f14a2ef3..f6549fcf6 100644 --- a/cmd/factory.go +++ b/cmd/factory.go @@ -37,6 +37,7 @@ type Factory interface { SQLMethods() (*lib.SQLMethods, error) FSIMethods() (*lib.FSIMethods, error) RenderMethods() (*lib.RenderMethods, error) + TransformMethods() (*lib.TransformMethods, error) } // StandardRepoPath returns qri paths based on the QRI_PATH environment diff --git a/cmd/factory_test.go b/cmd/factory_test.go index 6fe4042bf..49f6c0ab8 100644 --- a/cmd/factory_test.go +++ b/cmd/factory_test.go @@ -185,3 +185,8 @@ func (t TestFactory) SQLMethods() (*lib.SQLMethods, error) { func (t TestFactory) RenderMethods() (*lib.RenderMethods, error) { return lib.NewRenderMethods(t.inst), nil } + +// TransformMethods generates a lib.TransformMethods from internal state +func (t TestFactory) TransformMethods() (*lib.TransformMethods, error) { + return lib.NewTransformMethods(t.inst), nil +} diff --git a/cmd/qri.go b/cmd/qri.go index 00ae2104a..8f0526388 100644 --- a/cmd/qri.go +++ b/cmd/qri.go @@ -43,6 +43,7 @@ https://github.com/qri-io/qri/issues`, cmd.PersistentFlags().BoolVarP(&opt.LogAll, "log-all", "", false, "log all activity") cmd.AddCommand( + NewApplyCommand(opt, ioStreams), NewAutocompleteCommand(opt, ioStreams), NewCheckoutCommand(opt, ioStreams), NewConfigCommand(opt, ioStreams), @@ -238,6 +239,14 @@ func (o *QriOptions) DatasetMethods() (*lib.DatasetMethods, error) { return lib.NewDatasetMethods(o.inst), nil } +// TransformMethods generates a lib.TransformMethods from internal state +func (o *QriOptions) TransformMethods() (*lib.TransformMethods, error) { + if err := o.Init(); err != nil { + return nil, err + } + return lib.NewTransformMethods(o.inst), nil +} + // RemoteMethods generates a lib.RemoteMethods from internal state func (o *QriOptions) RemoteMethods() (*lib.RemoteMethods, error) { if err := o.Init(); err != nil { diff --git a/cmd/save.go b/cmd/save.go index bfae95092..0490c6041 100644 --- a/cmd/save.go +++ b/cmd/save.go @@ -2,6 +2,7 @@ package cmd import ( "fmt" + "strings" "github.com/qri-io/dataset" "github.com/qri-io/ioes" @@ -65,6 +66,8 @@ commit message and title to the save.`, cmd.Flags().StringVarP(&o.BodyPath, "body", "", "", "path to file or url of data to add as dataset contents") cmd.MarkFlagFilename("body") // cmd.Flags().BoolVarP(&o.ShowValidation, "show-validation", "s", false, "display a list of validation errors upon adding") + cmd.Flags().BoolVar(&o.Apply, "apply", false, "apply a transformation and save the result") + cmd.Flags().BoolVar(&o.NoApply, "no-apply", false, "don't apply any transforms that are added") cmd.Flags().StringSliceVar(&o.Secrets, "secrets", nil, "transform secrets as comma separated key,value,key,value,... sequence") cmd.Flags().BoolVar(&o.DeprecatedDryRun, "dry-run", false, "deprecated: use `qri apply` instead") cmd.Flags().BoolVar(&o.Force, "force", false, "force a new commit, even if no changes are detected") @@ -90,17 +93,18 @@ type SaveOptions struct { Title string Message string - Replace bool - ShowValidation bool - + Apply bool + NoApply bool DeprecatedDryRun bool + Secrets []string - KeepFormat bool - Force bool - NoRender bool - Secrets []string - NewName bool - UseDscache bool + Replace bool + ShowValidation bool + KeepFormat bool + Force bool + NoRender bool + NewName bool + UseDscache bool DatasetMethods *lib.DatasetMethods FSIMethods *lib.FSIMethods @@ -154,17 +158,36 @@ func (o *SaveOptions) Run() (err error) { Title: o.Title, Message: o.Message, - ScriptOutput: o.ErrOut, - FilePaths: o.FilePaths, - Private: false, - Drop: o.Drop, + ScriptOutput: o.ErrOut, + FilePaths: o.FilePaths, + Private: false, + Apply: o.Apply, + Drop: o.Drop, + ConvertFormatToPrev: o.KeepFormat, Force: o.Force, - ShouldRender: !o.NoRender, - NewName: o.NewName, - UseDscache: o.UseDscache, + + ShouldRender: !o.NoRender, + NewName: o.NewName, + UseDscache: o.UseDscache, + } + + // Check if file ends in '.star'. If so, either Apply or NoApply is required. + // Apply is passed down to the lib level, NoApply ends here. NoApply's only purpose + // is to ensure that the user wants to add a transform without running it, and explicitly + // agrees that they're not expecting the old behavior: wherein adding a transform + // would always run it. + for _, file := range o.FilePaths { + if strings.HasSuffix(file, ".star") { + if !o.Apply && !o.NoApply { + return fmt.Errorf("saving with a new transform requires either --apply or --no-apply flag") + } + } } + // TODO(dustmop): Support component files, like .json and .yaml, which can contain + // transform scripts. + if o.Secrets != nil { if !confirm(o.ErrOut, o.In, ` Warning: You are providing secrets to a dataset transformation. diff --git a/cmd/save_test.go b/cmd/save_test.go index 5b3bc4af4..43255dc1c 100644 --- a/cmd/save_test.go +++ b/cmd/save_test.go @@ -997,7 +997,7 @@ func TestSaveTwiceWithTransform(t *testing.T) { run.MustExec(t, "qri save --body testdata/movies/body_ten.csv test_peer_save_twice_with_xform/my_ds") // Save a second version with a transform - run.MustExec(t, "qri save --file testdata/movies/tf_one_movie.star test_peer_save_twice_with_xform/my_ds") + run.MustExec(t, "qri save --apply --file testdata/movies/tf_one_movie.star test_peer_save_twice_with_xform/my_ds") // Get the saved transform, make sure it matches the source file output := run.MustExec(t, "qri get transform.script test_peer_save_twice_with_xform/my_ds") @@ -1017,7 +1017,7 @@ func TestSaveTransformUsingPrev(t *testing.T) { run.MustExec(t, "qri save --body testdata/movies/body_ten.csv test_peer_save_using_prev/my_ds") // Save a second version with a transform - run.MustExec(t, "qri save --file testdata/movies/tf_set_len.star test_peer_save_using_prev/my_ds") + run.MustExec(t, "qri save --apply --file testdata/movies/tf_set_len.star test_peer_save_using_prev/my_ds") // Read body from the dataset that was saved. dsPath := run.GetPathForDataset(t, 0) @@ -1037,7 +1037,7 @@ func TestSaveTransformUsingConfigSecret(t *testing.T) { defer run.Delete() // Save a version with a transform that has config and secret data - run.MustExec(t, "qri save --file testdata/movies/tf_using_config_secret.json --secrets animal_sound,meow test_peer_save_twice_with_xform/my_ds") + run.MustExec(t, "qri save --apply --file testdata/movies/tf_using_config_secret.json --secrets animal_sound,meow test_peer_save_twice_with_xform/my_ds") // Read body from the dataset that was saved. dsPath := run.GetPathForDataset(t, 0) @@ -1060,7 +1060,7 @@ func TestSaveTransformSetMeta(t *testing.T) { run.MustExec(t, "qri save --body testdata/movies/body_ten.csv test_peer_save_set_meta/my_ds") // Save another version with a transform that sets the meta - run.MustExec(t, "qri save --file testdata/movies/tf_set_meta.star test_peer_save_set_meta/my_ds") + run.MustExec(t, "qri save --apply --file testdata/movies/tf_set_meta.star test_peer_save_set_meta/my_ds") // Read body from the dataset that was saved. dsPath := run.GetPathForDataset(t, 0) @@ -1083,7 +1083,7 @@ func TestSaveTransformChangeMetaAndBody(t *testing.T) { run.MustExec(t, "qri save --body testdata/movies/body_ten.csv test_peer_save_set_meta_and_body/my_ds") // Save another version with a transform that sets the body and a manual meta change - err := run.ExecCommand("qri save --file testdata/movies/tf_set_len.star --file testdata/movies/meta_override.yaml test_peer_save_set_meta_and_body/my_ds") + err := run.ExecCommand("qri save --apply --file testdata/movies/tf_set_len.star --file testdata/movies/meta_override.yaml test_peer_save_set_meta_and_body/my_ds") if err != nil { t.Errorf("unexpected error: %q", err) } @@ -1097,7 +1097,7 @@ func TestSaveTransformConflictWithBody(t *testing.T) { run.MustExec(t, "qri save --body testdata/movies/body_ten.csv test_peer_save_conflict_with_body/my_ds") // Save another version with a transform that sets the body and a manual body change - err := run.ExecCommand("qri save --file testdata/movies/tf_set_len.star --body testdata/movies/body_twenty.csv test_peer_save_conflict_with_body/my_ds") + err := run.ExecCommand("qri save --apply --file testdata/movies/tf_set_len.star --body testdata/movies/body_twenty.csv test_peer_save_conflict_with_body/my_ds") if err == nil { t.Fatal("expected error trying to save, did not get an error") } @@ -1115,7 +1115,7 @@ func TestSaveTransformConflictWithMeta(t *testing.T) { run.MustExec(t, "qri save --body testdata/movies/body_ten.csv test_peer_save_conflict_with_meta/my_ds") // Save another version with a transform that sets the meta and a manual meta change - err := run.ExecCommand("qri save --file testdata/movies/tf_set_meta.star --file testdata/movies/meta_override.yaml test_peer_save_conflict_with_meta/my_ds") + err := run.ExecCommand("qri save --apply --file testdata/movies/tf_set_meta.star --file testdata/movies/meta_override.yaml test_peer_save_conflict_with_meta/my_ds") if err == nil { t.Fatal("expected error trying to save, did not get an error") } @@ -1125,6 +1125,54 @@ func TestSaveTransformConflictWithMeta(t *testing.T) { } } +func TestDryRunIsAnError(t *testing.T) { + run := NewTestRunner(t, "test_peer_dry_run_err", "qri_test_dry_run_err") + defer run.Delete() + + err := run.ExecCommand("qri save --dry-run --body testdata/movies/body_ten.csv test_peer_dry_run_err/my_ds") + if err == nil { + t.Fatal("expectd error trying to dry run, did not get an error") + } + expectErr := "--dry-run has been removed, use `qri apply` command instead" + if diff := cmp.Diff(expectErr, err.Error()); diff != "" { + t.Errorf("error mismatch (-want +got):%s\n", diff) + } +} + +func TestSaveApply(t *testing.T) { + run := NewTestRunner(t, "test_peer_save_apply", "qri_test_save_apply") + defer run.Delete() + + // Error to use --file with neither --apply nor --no-apply + err := run.ExecCommand("qri save --file testdata/movies/tf_one_movie.star test_peer_save_apply/my_ds") + if err == nil { + t.Fatal("expectd error trying to dry run, did not get an error") + } + expectErr := `saving with a new transform requires either --apply or --no-apply flag` + if diff := cmp.Diff(expectErr, err.Error()); diff != "" { + t.Errorf("error mismatch (-want +got):%s\n", diff) + } + + // Save using --apply and --file + err = run.ExecCommand("qri save --apply --file testdata/movies/tf_one_movie.star test_peer_save_apply/one_movie") + if err != nil { + t.Error(err) + } + + // Save using --no-apply, adds a transform but doesn't run it + err = run.ExecCommand("qri save --no-apply --file testdata/movies/tf_set_meta.star test_peer_save_apply/one_movie") + if err != nil { + t.Error(err) + } + + // No meta, because the previous transform wasn't applied + output := run.MustExec(t, "qri get meta me/one_movie") + expect := "null\n\n" + if diff := cmp.Diff(expect, output); diff != "" { + t.Errorf("result mismatch (-want +got):%s\n", diff) + } +} + // TODO(dustmop): Test that if the result has a different shape than the previous version, // the error message should be reasonable and understandable //func TestSaveWithBadShape(t *testing.T) { diff --git a/lib/datasets.go b/lib/datasets.go index 48ba4d389..2edcf05db 100644 --- a/lib/datasets.go +++ b/lib/datasets.go @@ -448,6 +448,11 @@ type SaveParams struct { // note: this won't work over RPC, only on local calls ScriptOutput io.Writer + // TODO(dustmop): add `Wait bool`, if false, run the save asynchronously + // and return events on the bus that provide the progress of the save operation + + // Apply runs a transform script to create the next version to save + Apply bool // Replace writes the entire given dataset as a new snapshot instead of // applying save params as augmentations to the existing history Replace bool @@ -600,10 +605,11 @@ func (m *DatasetMethods) Save(p *SaveParams, res *dataset.Dataset) error { return err } - // If a transform is being provided, execute its script - // TODO(dustmop): This will become a call to `apply` in the future, and will require the - // `--apply` flag to be true. - if ds.Transform != nil { + // If applying a transform, execute its script before saving + if p.Apply { + if ds.Transform == nil { + return fmt.Errorf("cannot apply while saving without a transform") + } str := m.inst.node.LocalStreams scriptOut := p.ScriptOutput secrets := p.Secrets diff --git a/lib/datasets_test.go b/lib/datasets_test.go index bb8999f9a..5e792aaba 100644 --- a/lib/datasets_test.go +++ b/lib/datasets_test.go @@ -195,6 +195,89 @@ func TestDatasetRequestsSaveZip(t *testing.T) { t.Fatalf("Expected 'Test Repo', got '%s'", res.Meta.Title) } } + +func TestDatasetRequestsSaveApply(t *testing.T) { + run := newTestRunner(t) + defer run.Delete() + + // Trying to save using apply without a transform is an error + _, err := run.SaveWithParams(&SaveParams{ + Ref: "me/cities_ds", + BodyPath: "testdata/cities_2/body.csv", + Apply: true, + }) + if err == nil { + t.Fatal("expected an error, did not get one") + } + expectErr := `cannot apply while saving without a transform` + if diff := cmp.Diff(expectErr, err.Error()); diff != "" { + t.Errorf("error mismatch (-want +got):%s\n", diff) + } + + // Save using apply and a transform, for a new dataset + _, err = run.SaveWithParams(&SaveParams{ + Ref: "me/hello", + FilePaths: []string{"testdata/tf/transform.star"}, + Apply: true, + }) + if err != nil { + t.Error(err) + } + + // Save another dataset with a body + _, err = run.SaveWithParams(&SaveParams{ + Ref: "me/existing_ds", + BodyPath: "testdata/cities_2/body.csv", + }) + if err != nil { + t.Error(err) + } + + ds := run.MustGet(t, "me/existing_ds") + bodyPath := ds.BodyPath + + // Save using apply and a transform, for dataset that already exists + _, err = run.SaveWithParams(&SaveParams{ + Ref: "me/existing_ds", + FilePaths: []string{"testdata/cities_2/add_city.star"}, + Apply: true, + }) + if err != nil { + t.Error(err) + } + + ds = run.MustGet(t, "me/existing_ds") + if ds.BodyPath == bodyPath { + t.Error("expected body path to change, but it did not change") + } + + // Save another dataset with a body + _, err = run.SaveWithParams(&SaveParams{ + Ref: "me/another_ds", + BodyPath: "testdata/cities_2/body.csv", + }) + if err != nil { + t.Error(err) + } + + ds = run.MustGet(t, "me/another_ds") + bodyPath = ds.BodyPath + + // Save by adding a transform, but do not apply it. Body is unchanged. + _, err = run.SaveWithParams(&SaveParams{ + Ref: "me/another_ds", + FilePaths: []string{"testdata/tf/transform.star"}, + }) + if err != nil { + t.Error(err) + } + + ds = run.MustGet(t, "me/another_ds") + if ds.BodyPath != bodyPath { + t.Error("unexpected: body path changed") + } +} + func TestDatasetRequestsList(t *testing.T) { ctx, done := context.WithCancel(context.Background()) defer done() diff --git a/lib/integration_test.go b/lib/integration_test.go index 993c55d6e..90a4e521a 100644 --- a/lib/integration_test.go +++ b/lib/integration_test.go @@ -190,6 +190,7 @@ def transform(ds, ctx): FilePaths: []string{ scriptPath, }, + Apply: true, } res := &dataset.Dataset{} if err := dsm.Save(saveParams, res); err != nil { diff --git a/lib/test_runner_test.go b/lib/test_runner_test.go index 4e6ee18e0..c3471882b 100644 --- a/lib/test_runner_test.go +++ b/lib/test_runner_test.go @@ -168,6 +168,25 @@ func (tr *testRunner) SaveWithParams(p *SaveParams) (dsref.Ref, error) { return dsref.ConvertDatasetToVersionInfo(res).SimpleRef(), nil } +func (tr *testRunner) MustGet(t *testing.T, refstr string) *dataset.Dataset { + m := NewDatasetMethods(tr.Instance) + p := GetParams{Refstr: refstr} + res := GetResult{} + if err := m.Get(&p, &res); err != nil { + t.Fatal(err) + } + return res.Dataset +} + +func (tr *testRunner) ApplyWithParams(p *ApplyParams) (*dataset.Dataset, error) { + m := NewTransformMethods(tr.Instance) + res := ApplyResult{} + if err := m.Apply(p, &res); err != nil { + return nil, err + } + return res.Data, nil +} + func (tr *testRunner) Diff(left, right, selector string) (string, error) { m := NewDatasetMethods(tr.Instance) p := DiffParams{ diff --git a/lib/testdata/cities_2/add_city.star b/lib/testdata/cities_2/add_city.star new file mode 100644 index 000000000..add359e09 --- /dev/null +++ b/lib/testdata/cities_2/add_city.star @@ -0,0 +1,4 @@ +def transform(ds,ctx): + body = ds.get_body() + body.append(["tokyo", 9200000, 48.5, False]) + ds.set_body(body) diff --git a/lib/transform.go b/lib/transform.go new file mode 100644 index 000000000..a75cb9bc4 --- /dev/null +++ b/lib/transform.go @@ -0,0 +1,102 @@ +package lib + +import ( + "context" + "fmt" + "io" + + "github.com/qri-io/dataset" + "github.com/qri-io/qri/base" + "github.com/qri-io/qri/dsref" +) + +// TransformMethods encapsulates business logic for transforms +// TODO (b5): switch to using an Instance instead of separate fields +type TransformMethods struct { + inst *Instance +} + +// CoreRequestsName implements the Requests interface +func (TransformMethods) CoreRequestsName() string { return "apply" } + +// NewTransformMethods creates a TransformMethods pointer from a qri instance +func NewTransformMethods(inst *Instance) *TransformMethods { + return &TransformMethods{ + inst: inst, + } +} + +// ApplyParams are parameters for the apply command +type ApplyParams struct { + Refstr string + Transform *dataset.Transform + Source string + Secrets map[string]string + + // TODO(dustmop): add `Wait bool`, if false, run the save asynchronously + // and return events on the bus that provide the progress of the save operation + + ScriptOutput io.Writer +} + +// Valid returns an error if ApplyParams fields are in an invalid state +func (p *ApplyParams) Valid() error { + if p.Refstr == "" && p.Transform == nil { + return fmt.Errorf("one or both of Reference, Transform are required") + } + return nil +} + +// ApplyResult is the result of an apply command +type ApplyResult struct { + Data *dataset.Dataset + // TODO(dustmop): Make Apply asynchronous, running the transform in a go-routine. + // Return a channel that will send progress on the execution. +} + +// Apply runs a transform script +func (m *TransformMethods) Apply(p *ApplyParams, res *ApplyResult) error { + err := p.Valid() + if err != nil { + return err + } + + if m.inst.rpc != nil { + return checkRPCError(m.inst.rpc.Call("TransformMethods.Apply", p, res)) + } + + ctx := context.TODO() + ref := dsref.Ref{} + if p.Refstr != "" { + ref, _, err = m.inst.ParseAndResolveRefWithWorkingDir(ctx, p.Refstr, "") + if err != nil { + return err + } + } + + ds := &dataset.Dataset{} + if !ref.IsEmpty() { + ds.Name = ref.Name + ds.Peername = ref.Username + } + if p.Transform != nil { + ds.Transform = p.Transform + ds.Transform.OpenScriptFile(ctx, m.inst.repo.Filesystem()) + } + + r := m.inst.Repo() + str := m.inst.node.LocalStreams + loader := NewParseResolveLoadFunc("", m.inst.defaultResolver(), m.inst) + + scriptOut := p.ScriptOutput + err = base.TransformApply(ctx, ds, r, loader, str, scriptOut, p.Secrets) + if err != nil { + return err + } + + if err = base.InlineJSONBody(ds); err != nil && err != base.ErrNoBodyToInline { + return err + } + res.Data = ds + return nil +} diff --git a/lib/transform_test.go b/lib/transform_test.go new file mode 100644 index 000000000..73e315d1c --- /dev/null +++ b/lib/transform_test.go @@ -0,0 +1,44 @@ +package lib + +import ( + "encoding/json" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/qri-io/dataset" +) + +func TestApplyTransform(t *testing.T) { + tr := newTestRunner(t) + defer tr.Delete() + + // Save a dataset with a body + _, err := tr.SaveWithParams(&SaveParams{ + Ref: "me/cities_ds", + BodyPath: "testdata/cities_2/body.csv", + }) + if err != nil { + t.Error(err) + } + + // Apply a transformation + res, err := tr.ApplyWithParams(&ApplyParams{ + Refstr: "me/cities_ds", + Transform: &dataset.Transform{ + ScriptPath: "testdata/cities_2/add_city.star", + }, + }) + if err != nil { + t.Error(err) + } + + output, err := json.Marshal(res.Body) + if err != nil { + t.Fatal(err) + } + actual := string(output) + expect := `[["toronto",50000000,55.5,false],["new york",8500000,44.4,true],["chicago",300000,44.4,true],["chatham",35000,65.25,true],["raleigh",250000,50.65,true],["tokyo",9200000,48.5,false]]` + if diff := cmp.Diff(expect, actual); diff != "" { + t.Errorf("qri list (-want +got):\n%s", diff) + } +}