fix(format): Detect format change when saving, either error or rewrite

When a save command is run, and the new format is different from the format of the previous version, either return an error, or if the ConvertFormatToPrev flag is set on SaveParams, rewrite the body to match the previous version’s format. The API always sets ConvertFormatToPrev, but the command line does not (yet).

Author: dustmop

actions/actions_test.go

@@ -105,7 +105,7 @@ func addCitiesDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
 	dsp.Name = tc.Name
 	dsp.BodyBytes = tc.Body
 
-	ref, _, err := SaveDataset(node, dsp, nil, false, true)
+	ref, _, err := SaveDataset(node, dsp, nil, false, true, false)
 	if err != nil {
 		t.Fatal(err.Error())
 	}
@@ -121,7 +121,7 @@ func addFlourinatedCompoundsDataset(t *testing.T, node *p2p.QriNode) repo.Datase
 	dsp.Name = tc.Name
 	dsp.BodyBytes = tc.Body
 
-	ref, _, err := SaveDataset(node, dsp, nil, false, true)
+	ref, _, err := SaveDataset(node, dsp, nil, false, true, false)
 	if err != nil {
 		t.Fatal(err.Error())
 	}
@@ -137,7 +137,7 @@ func addNowTransformDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
 	dsp.Name = tc.Name
 	dsp.Transform.ScriptPath = "testdata/now_tf/transform.star"
 
-	ref, _, err := SaveDataset(node, dsp, nil, false, true)
+	ref, _, err := SaveDataset(node, dsp, nil, false, true, false)
 	if err != nil {
 		t.Fatal(err.Error())
 	}

actions/dataset.go

@@ -17,7 +17,7 @@ import (
 )
 
 // SaveDataset initializes a dataset from a dataset pointer and data file
-func SaveDataset(node *p2p.QriNode, changesPod *dataset.DatasetPod, secrets map[string]string, dryRun, pin bool) (ref repo.DatasetRef, body cafs.File, err error) {
+func SaveDataset(node *p2p.QriNode, changesPod *dataset.DatasetPod, secrets map[string]string, dryRun, pin bool, convertFormatToPrev bool) (ref repo.DatasetRef, body cafs.File, err error) {
 	var (
 		prev                     *dataset.Dataset
 		prevPath                 string
@@ -33,12 +33,13 @@ func SaveDataset(node *p2p.QriNode, changesPod *dataset.DatasetPod, secrets map[
 		return
 	}
 
+	pro, err = r.Profile()
+	if err != nil {
+		return
+	}
+
 	if dryRun {
 		node.LocalStreams.Print("🏃🏽‍♀️ dry run\n")
-		pro, err = r.Profile()
-		if err != nil {
-			return
-		}
 		// dry-runs store to an in-memory repo
 		r, err = repo.NewMemRepo(pro, cafs.NewMapstore(), profile.NewMemStore(), nil)
 		if err != nil {
@@ -90,6 +91,26 @@ func SaveDataset(node *p2p.QriNode, changesPod *dataset.DatasetPod, secrets map[
 		node.LocalStreams.Print("✅ transform complete\n")
 	}
 
+	// Infer any values about the incoming change before merging it with the previous version.
+	changeBodyFile, err = base.InferValues(pro, &changesPod.Name, changes, changeBodyFile)
+	if err != nil {
+		return
+	}
+
+	if prev.Structure != nil && changes.Structure != nil && prev.Structure.Format != changes.Structure.Format {
+		if convertFormatToPrev {
+			changeBodyFile, err = base.ConvertBodyFormat(changeBodyFile, changes.Structure,
+				prev.Structure)
+			if err != nil {
+				return
+			}
+		} else {
+			err = fmt.Errorf("Refusing to change structure from %s to %s",
+				prev.Structure.Format, changes.Structure.Format)
+			return
+		}
+	}
+
 	// apply changes to the previous path, set changes to the result
 	prev.Assign(changes)
 	changes = prev

actions/dataset_test.go

@@ -126,7 +126,7 @@ func TestSaveDataset(t *testing.T) {
 		BodyBytes: []byte("[]"),
 	}
 
-	ref, _, err := SaveDataset(n, ds, nil, true, false)
+	ref, _, err := SaveDataset(n, ds, nil, true, false, false)
 	if err != nil {
 		t.Errorf("dry run error: %s", err.Error())
 	}
@@ -147,7 +147,7 @@ func TestSaveDataset(t *testing.T) {
 		Structure: &dataset.StructurePod{Format: dataset.JSONDataFormat.String(), Schema: map[string]interface{}{"type": "array"}},
 		BodyBytes: []byte("[]"),
 	}
-	ref, _, err = SaveDataset(n, ds, nil, false, true)
+	ref, _, err = SaveDataset(n, ds, nil, false, true, false)
 	if err != nil {
 		t.Error(err)
 	}
@@ -176,7 +176,7 @@ func TestSaveDataset(t *testing.T) {
   ds.set_body(bd)`),
 		},
 	}
-	ref, _, err = SaveDataset(n, ds, secrets, false, true)
+	ref, _, err = SaveDataset(n, ds, secrets, false, true, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -194,7 +194,7 @@ func TestSaveDataset(t *testing.T) {
 			Description: "updated description",
 		},
 	}
-	ref, _, err = SaveDataset(n, ds, nil, false, true)
+	ref, _, err = SaveDataset(n, ds, nil, false, true, false)
 	if err != nil {
 		t.Error(err)
 	}
@@ -217,7 +217,7 @@ func TestSaveDataset(t *testing.T) {
 		},
 		Transform: tfds.Transform,
 	}
-	ref, _, err = SaveDataset(n, ds, secrets, false, true)
+	ref, _, err = SaveDataset(n, ds, secrets, false, true, false)
 	if err != nil {
 		t.Error(err)
 	}

api/datasets.go

@@ -481,10 +481,11 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
 
 	res := &repo.DatasetRef{}
 	p := &lib.SaveParams{
-		Dataset:    dsp,
-		Private:    r.FormValue("private") == "true",
-		DryRun:     r.FormValue("dry_run") == "true",
-		ReturnBody: r.FormValue("return_body") == "true",
+		Dataset:             dsp,
+		Private:             r.FormValue("private") == "true",
+		DryRun:              r.FormValue("dry_run") == "true",
+		ReturnBody:          r.FormValue("return_body") == "true",
+		ConvertFormatToPrev: true,
 	}
 
 	if r.FormValue("secrets") != "" {

base/dataset.go

@@ -1,6 +1,7 @@
 package base
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -13,6 +14,7 @@ import (
 	"github.com/qri-io/cafs"
 	"github.com/qri-io/dataset"
 	"github.com/qri-io/dataset/dsfs"
+	"github.com/qri-io/dataset/dsio"
 	"github.com/qri-io/ioes"
 	"github.com/qri-io/qri/repo"
 	"github.com/qri-io/qri/repo/profile"
@@ -74,10 +76,6 @@ func CreateDataset(r repo.Repo, streams ioes.IOStreams, name string, ds *dataset
 		return
 	}
 
-	if body, err = InferValues(pro, &name, ds, body); err != nil {
-		return
-	}
-
 	// TODO - we should remove the need for this by having viz always be kept in the right
 	// state until this point
 	if err = prepareViz(r, ds); err != nil {
@@ -265,3 +263,32 @@ func DatasetPodBodyFile(store cafs.Filestore, dsp *dataset.DatasetPod) (cafs.Fil
 
 	return cafs.NewMemfileReader(filepath.Base(dsp.BodyPath), file), nil
 }
+
+// ConvertBodyFormat rewrites a body from a source format to a destination format.
+func ConvertBodyFormat(bodyFile cafs.File, fromSt *dataset.Structure, toSt *dataset.Structure) (cafs.File, error) {
+	// Reader for entries of the source body.
+	r, err := dsio.NewEntryReader(fromSt, bodyFile)
+	if err != nil {
+		return nil, err
+	}
+
+	// Writes entries to a new body.
+	buffer := bytes.NewBufferString("")
+	w, err := dsio.NewEntryWriter(toSt, buffer)
+	if err != nil {
+		return nil, err
+	}
+
+	err = dsio.Copy(r, w)
+	if err != nil {
+		return nil, err
+	}
+	err = w.Close()
+	if err != nil {
+		return nil, err
+	}
+
+	// Set the new format on the structure.
+	fromSt.Format = toSt.Format
+	return cafs.NewMemfileReader("", buffer), nil
+}

cmd/save_test.go

@@ -141,7 +141,7 @@ func TestSaveRun(t *testing.T) {
 		{"me/cities", "bad/filpath.json", "", "", "", false, "", "open bad/filpath.json: no such file or directory", ""},
 		{"me/cities", "", "bad/bodypath.csv", "", "", false, "", "body file: open bad/bodypath.csv: no such file or directory", ""},
 		{"me/movies", "testdata/movies/dataset.json", "testdata/movies/body_ten.csv", "", "", true, "dataset saved: peer/movies@QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt/map/QmVxUpVVVNedQ645nC25zu6ZtW3yWSiknVmAePLXQ2YSPR\nthis dataset has 1 validation errors\n", "", ""},
-		{"me/movies", "", "testdata/movies/body_twenty.csv", "Added 10 more rows", "Adding to the number of rows in dataset", true, "dataset saved: peer/movies@QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt/map/QmW8999UBCFoBBwjFiSgm53znp8e5eWEP1kodJeev5CJM9\nthis dataset has 1 validation errors\n", "", ""},
+		{"me/movies", "", "testdata/movies/body_twenty.csv", "Added 10 more rows", "Adding to the number of rows in dataset", true, "dataset saved: peer/movies@QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt/map/QmPeUYwHEUh3xV5C6k6YvyaDaHix1B7TMbMwG2ffXYYuEX\nthis dataset has 1 validation errors\n", "", ""},
 		{"me/movies", "", "testdata/movies/body_twenty.csv", "trying to add again", "hopefully this errors", false, "", "error saving: no changes detected", ""},
 	}
 

lib/datasets.go

@@ -101,6 +101,8 @@ type SaveParams struct {
 	DryRun bool
 	// if true, res.Dataset.Body will be a cafs.file of the body
 	ReturnBody bool
+	// if true, convert body to the format of the previous version, if applicable
+	ConvertFormatToPrev bool
 	// string of references to recall before saving
 	Recall string
 }
@@ -159,7 +161,7 @@ func (r *DatasetRequests) Save(p *SaveParams, res *repo.DatasetRef) (err error)
 		return fmt.Errorf("no changes to save")
 	}
 
-	ref, body, err := actions.SaveDataset(r.node, ds, p.Secrets, p.DryRun, true)
+	ref, body, err := actions.SaveDataset(r.node, ds, p.Secrets, p.DryRun, true, p.ConvertFormatToPrev)
 	if err != nil {
 		log.Debugf("create ds error: %s\n", err.Error())
 		return err

lib/lib_test.go

@@ -78,7 +78,7 @@ func addCitiesDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
 	dsp.Name = tc.Name
 	dsp.BodyBytes = tc.Body
 
-	ref, _, err := actions.SaveDataset(node, dsp, nil, false, true)
+	ref, _, err := actions.SaveDataset(node, dsp, nil, false, true, false)
 	if err != nil {
 		t.Fatal(err.Error())
 	}
@@ -94,7 +94,7 @@ func addNowTransformDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
 	dsp.Name = tc.Name
 	dsp.Transform.ScriptPath = "testdata/now_tf/transform.star"
 
-	ref, _, err := actions.SaveDataset(node, dsp, nil, false, true)
+	ref, _, err := actions.SaveDataset(node, dsp, nil, false, true, false)
 	if err != nil {
 		t.Fatal(err.Error())
 	}