Skip to content

Commit

Permalink
feat(replace-save dataset): support dataset save without patching fro…
Browse files Browse the repository at this point in the history
…m prior verion

actions.SaveDataset has been refactored to accept a struct of switches. That struct has a new switch `Replace` that instructs qri to only use input data when creating a dataset commit. This has the effect of "replacing" the dataset instead of applying the input dataset as modifications to the prior version.

We need this for FSI, which operates on a file-system-reflects-current-state paradigm
  • Loading branch information
b5 committed Jul 19, 2019
1 parent e5c7fde commit 9906727
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 26 deletions.
6 changes: 3 additions & 3 deletions actions/actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func addCitiesDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
t.Fatal(err.Error())
}

ref, err := SaveDataset(node, tc.Input, nil, nil, false, true, false, false, true)
ref, err := SaveDataset(node, tc.Input, nil, nil, SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Fatal(err.Error())
}
Expand All @@ -129,7 +129,7 @@ func addFlourinatedCompoundsDataset(t *testing.T, node *p2p.QriNode) repo.Datase
t.Fatal(err.Error())
}

ref, err := SaveDataset(node, tc.Input, nil, nil, false, true, false, false, true)
ref, err := SaveDataset(node, tc.Input, nil, nil, SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Fatal(err.Error())
}
Expand All @@ -145,7 +145,7 @@ func addNowTransformDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
// this was put here to satisfy qri-io/qri/actions.TestUpdateDatasetLocal
tc.Input.Peername = "peer"

ref, err := SaveDataset(node, tc.Input, nil, nil, false, true, false, false, true)
ref, err := SaveDataset(node, tc.Input, nil, nil, SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Fatal(err.Error())
}
Expand Down
31 changes: 23 additions & 8 deletions actions/dataset.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,27 @@ import (
"github.com/qri-io/qri/repo/profile"
)

// SaveDatasetSwitches provides togglable flags to SaveDataset that control
// save behaviour
// dryRun, pin, convertFormatToPrev, force, shouldRender bool
type SaveDatasetSwitches struct {
Replace bool //
DryRun bool
Pin bool
ConvertFormatToPrev bool
Force bool
ShouldRender bool
}

// SaveDataset initializes a dataset from a dataset pointer and data file
func SaveDataset(node *p2p.QriNode, changes *dataset.Dataset, secrets map[string]string, scriptOut io.Writer, dryRun, pin, convertFormatToPrev, force, shouldRender bool) (ref repo.DatasetRef, err error) {
func SaveDataset(node *p2p.QriNode, changes *dataset.Dataset, secrets map[string]string, scriptOut io.Writer, sw SaveDatasetSwitches) (ref repo.DatasetRef, err error) {
var (
prevPath string
pro *profile.Profile
r = node.Repo
)


prev, mutable, prevPath, err := base.PrepareDatasetSave(r, changes.Peername, changes.Name)
if err != nil {
return
Expand All @@ -33,7 +46,7 @@ func SaveDataset(node *p2p.QriNode, changes *dataset.Dataset, secrets map[string
return
}

if dryRun {
if sw.DryRun {
node.LocalStreams.PrintErr("🏃🏽‍♀️ dry run\n")
// dry-runs store to an in-memory repo
r, err = repo.NewMemRepo(pro, cafs.NewMapstore(), node.Repo.Filesystem(), profile.NewMemStore(), nil)
Expand Down Expand Up @@ -61,7 +74,7 @@ func SaveDataset(node *p2p.QriNode, changes *dataset.Dataset, secrets map[string
}

if changes.BodyFile() != nil && prev.Structure != nil && changes.Structure != nil && prev.Structure.Format != changes.Structure.Format {
if convertFormatToPrev {
if sw.ConvertFormatToPrev {
var f qfs.File
f, err = base.ConvertBodyFormat(changes.BodyFile(), changes.Structure, prev.Structure)
if err != nil {
Expand All @@ -77,24 +90,26 @@ func SaveDataset(node *p2p.QriNode, changes *dataset.Dataset, secrets map[string
}
}

// apply the changes to the previous dataset.
mutable.Assign(changes)
changes = mutable
if !sw.Replace {
// apply the changes to the previous dataset.
mutable.Assign(changes)
changes = mutable
}

// infer missing values
if err = base.InferValues(pro, changes); err != nil {
return
}

// add a default viz if one is needed
if shouldRender {
if sw.ShouldRender {
base.MaybeAddDefaultViz(changes)
}

// let's make history, if it exists
changes.PreviousPath = prevPath

return base.CreateDataset(r, node.LocalStreams, changes, prev, dryRun, pin, force, shouldRender)
return base.CreateDataset(r, node.LocalStreams, changes, prev, sw.DryRun, sw.Pin, sw.Force, sw.ShouldRender)
}

// UpdateRemoteDataset brings a reference to the latest version, syncing to the
Expand Down
57 changes: 49 additions & 8 deletions actions/dataset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestUpdateRemoteDataset(t *testing.T) {
ds.SetBodyFile(qfs.NewMemfileBytes("body.json", []byte("[]")))

// run a local update to advance history
now0, err := SaveDataset(peers[0], ds, nil, nil, false, true, false, false, true)
now0, err := SaveDataset(peers[0], ds, nil, nil, SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -122,7 +122,7 @@ func TestSaveDataset(t *testing.T) {
}
ds.SetBodyFile(qfs.NewMemfileBytes("body.json", []byte("[]")))

ref, err := SaveDataset(n, ds, nil, nil, true, false, false, false, true)
ref, err := SaveDataset(n, ds, nil, nil, SaveDatasetSwitches{ DryRun: true, ShouldRender: true })
if err != nil {
t.Errorf("dry run error: %s", err.Error())
}
Expand All @@ -145,7 +145,7 @@ func TestSaveDataset(t *testing.T) {
ds.SetBodyFile(qfs.NewMemfileBytes("body.json", []byte("[]")))

// test save
ref, err = SaveDataset(n, ds, nil, nil, false, true, false, false, true)
ref, err = SaveDataset(n, ds, nil, nil, SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -174,7 +174,7 @@ func TestSaveDataset(t *testing.T) {
ds.Transform.OpenScriptFile(nil)

// dryrun should work
ref, err = SaveDataset(n, ds, secrets, nil, true, false, false, false, true)
ref, err = SaveDataset(n, ds, secrets, nil, SaveDatasetSwitches{ DryRun: true, ShouldRender: true })
if err != nil {
t.Fatal(err)
}
Expand All @@ -200,7 +200,7 @@ func TestSaveDataset(t *testing.T) {
ds.Transform.OpenScriptFile(nil)

// test save with transform
ref, err = SaveDataset(n, ds, secrets, nil, false, true, false, false, true)
ref, err = SaveDataset(n, ds, secrets, nil, SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Fatal(err)
}
Expand All @@ -219,7 +219,7 @@ func TestSaveDataset(t *testing.T) {
},
}

ref, err = SaveDataset(n, ds, nil, nil, false, true, false, false, true)
ref, err = SaveDataset(n, ds, nil, nil, SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -247,7 +247,7 @@ func TestSaveDataset(t *testing.T) {
t.Error(err)
}

ref, err = SaveDataset(n, ds, secrets, nil, false, true, false, false, true)
ref, err = SaveDataset(n, ds, secrets, nil, SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Error(err)
}
Expand All @@ -266,13 +266,54 @@ func TestSaveDatasetWithoutStructureOrBody(t *testing.T) {
},
}

_, err := SaveDataset(n, ds, nil, nil, false, false, false, false, true)
_, err := SaveDataset(n, ds, nil, nil, SaveDatasetSwitches{ ShouldRender: true })
expect := "creating a new dataset requires a structure or a body"
if err == nil || err.Error() != expect {
t.Errorf("expected error, but got %s", err.Error())
}
}

func TestSaveDatasetReplace(t *testing.T) {
n := newTestNode(t)

ds := &dataset.Dataset{
Peername: "me",
Name: "test_save",
Meta: &dataset.Meta{
Title: "another test dataset",
},
Structure: &dataset.Structure{Format: "json", Schema: map[string]interface{}{"type": "array"}},
}
ds.SetBodyFile(qfs.NewMemfileBytes("body.json", []byte("[]")))


// test save
_, err := SaveDataset(n, ds, nil, nil, SaveDatasetSwitches{ Pin: true })
if err != nil {
t.Error(err)
}

ds = &dataset.Dataset{
Peername: "me",
Name: "test_save",
Structure: &dataset.Structure{Format: "json", Schema: map[string]interface{}{"type": "object"}},
}
ds.SetBodyFile(qfs.NewMemfileBytes("body.json", []byte(`{"foo":"bar"}`)))

ref, err := SaveDataset(n, ds, nil, nil, SaveDatasetSwitches{ Replace: true, Pin: true })
if err != nil {
t.Error(err)
}

if err := base.ReadDataset(n.Repo, &ref); err != nil {
t.Error(err)
}

if ref.Dataset.Meta != nil {
t.Error("expected overwritten meta to be nil")
}
}

type RepoMakerFunc func(t *testing.T) repo.Repo
type RepoTestFunc func(t *testing.T, rmf RepoMakerFunc)

Expand Down
5 changes: 2 additions & 3 deletions cmd/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,8 @@ type SaveOptions struct {

Title string
Message string

Passive bool
Rescursive bool

Replace bool
ShowValidation bool
Publish bool
DryRun bool
Expand Down
2 changes: 1 addition & 1 deletion fsi/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (fsi *FSI) Status(dir string) (changes []StatusItem, err error) {
for cmpName := range storedComponents {
// when reporting deletes, ignore "bound" components that must/must-not
// exist based on external conditions
if cmpName != componentNameStructure && cmpName != componentNameCommit {
if cmpName != componentNameDataset && cmpName != componentNameStructure && cmpName != componentNameCommit {
if _, ok := mapping[cmpName]; !ok {
change := StatusItem{
Path: cmpName,
Expand Down
14 changes: 13 additions & 1 deletion lib/datasets.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ type SaveParams struct {
// dataset supplies params directly, all other param fields override values
// supplied by dataset
Dataset *dataset.Dataset

// dataset reference string, the name to save to
Ref string
// commit title, defaults to a generated string based on diff
Expand All @@ -208,6 +209,9 @@ type SaveParams struct {
FilePaths []string
// secrets for transform execution
Secrets map[string]string
// Replace writes the entire given dataset as a new snapshot instead of
// applying save params as agumentations to the existing history
Replace bool
// option to make dataset private. private data is not currently implimented,
// see https://github.com/qri-io/qri/issues/291 for updates
Private bool
Expand Down Expand Up @@ -328,7 +332,15 @@ func (r *DatasetRequests) Save(p *SaveParams, res *repo.DatasetRef) (err error)
return
}

ref, err = actions.SaveDataset(r.node, ds, p.Secrets, p.ScriptOutput, p.DryRun, true, p.ConvertFormatToPrev, p.Force, p.ShouldRender)
switches := actions.SaveDatasetSwitches{
Replace: p.Replace,
DryRun: p.DryRun,
Pin: true,
ConvertFormatToPrev: p.ConvertFormatToPrev,
Force: p.Force,
ShouldRender: p.ShouldRender,
}
ref, err = actions.SaveDataset(r.node, ds, p.Secrets, p.ScriptOutput, switches)
if err != nil {
log.Debugf("create ds error: %s\n", err.Error())
return err
Expand Down
5 changes: 3 additions & 2 deletions lib/lib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ func addCitiesDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
ds.Name = tc.Name
ds.BodyBytes = tc.Body

ref, err := actions.SaveDataset(node, ds, nil, nil, false, true, false, false, true)
// dryRun, pin, convertFormatToPrev, force, shouldRender bool
ref, err := actions.SaveDataset(node, ds, nil, nil, actions.SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Fatal(err.Error())
}
Expand All @@ -178,7 +179,7 @@ func addNowTransformDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
ds.Name = tc.Name
ds.Transform.ScriptPath = "testdata/now_tf/transform.star"

ref, err := actions.SaveDataset(node, ds, nil, nil, false, true, false, false, true)
ref, err := actions.SaveDataset(node, ds, nil, nil, actions.SaveDatasetSwitches{ Pin: true, ShouldRender: true })
if err != nil {
t.Fatal(err.Error())
}
Expand Down

0 comments on commit 9906727

Please sign in to comment.