Skip to content

Commit

Permalink
fix(dataset): File components store content as text, instead of scrip…
Browse files Browse the repository at this point in the history
…tBytes
  • Loading branch information
dustmop committed Jul 12, 2021
1 parent c934d34 commit 3a958ee
Show file tree
Hide file tree
Showing 23 changed files with 203 additions and 69 deletions.
4 changes: 2 additions & 2 deletions api/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestGetZip(t *testing.T) {
// Save a version of the dataset
ds := run.BuildDataset("test_ds")
ds.Meta = &dataset.Meta{Title: "some title"}
ds.Readme = &dataset.Readme{ScriptBytes: []byte("# hi\n\nthis is a readme")}
ds.Readme = &dataset.Readme{Text: "# hi\n\nthis is a readme"}
run.SaveDataset(ds, "testdata/cities/data.csv")

// Get a zip file binary over the API
Expand Down Expand Up @@ -82,7 +82,7 @@ func TestDatasetGet(t *testing.T) {
Title: "title one",
},
Readme: &dataset.Readme{
ScriptBytes: []byte(`hello world`),
Text: "hello world",
},
}
run.SaveDataset(&ds, "testdata/cities/data.csv")
Expand Down
4 changes: 2 additions & 2 deletions api/spec/testdata/automation.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"body": {
"ref": "peer/movies",
"transform": {
"scriptBytes": "ZGVmIHRyYW5zZm9ybShkcyxjdHgpOlxucmV0dXJu"
"text": "def transform(ds,ctx):\nreturn"
}
},
"expect": {
Expand All @@ -18,4 +18,4 @@
}
}
}
]
]
Binary file modified api/testdata/cities/exported.zip
Binary file not shown.
4 changes: 2 additions & 2 deletions api/testdata/expect/TestDatasetGet.test_ds.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"peername": "peer",
"readme": {
"qri": "rm:0",
"scriptPath": "/mem/QmaozNR7DZHQK1ZcU9p7QdrshMvXqWK6gpu5rmrkPdT3L4"
"text": "hello world"
},
"qri": "ds:0",
"structure": {
Expand Down Expand Up @@ -135,4 +135,4 @@
}
]
}
}
}
10 changes: 5 additions & 5 deletions base/archive/archive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,17 @@ func testFSWithVizAndTransform() (qfs.Filesystem, map[string]string, error) {
},
},
Transform: &dataset.Transform{
ScriptPath: "/transform_script",
ScriptBytes: []byte("def transform(ds):\nreturn ds\n"),
ScriptPath: "/transform_script",
Text: "def transform(ds):\nreturn ds\n",
},
Viz: &dataset.Viz{
ScriptPath: dsfs.PackageFileVizScript.Filename(),
ScriptBytes: []byte("<html>template</html>\n"),
ScriptPath: dsfs.PackageFileVizScript.Filename(),
Text: "<html>template</html>\n",
},
}
// load scripts into file pointers, time for a NewDataset function?
// ds.Transform.OpenScriptFile(ctx, nil)
ds.Transform.SetScriptFile(qfs.NewMemfileBytes(ds.Viz.ScriptPath, ds.Viz.ScriptBytes))
ds.Transform.SetScriptFile(qfs.NewMemfileBytes(ds.Transform.ScriptPath, []byte(ds.Transform.Text)))
ds.Viz.OpenScriptFile(ctx, nil)
ds.Viz.SetRenderedFile(qfs.NewMemfileBytes("index.html", []byte("<html>rendered</html<\n")))

Expand Down
4 changes: 2 additions & 2 deletions base/archive/zip.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,15 @@ func UnzipDataset(r io.ReaderAt, size int64, ds *dataset.Dataset) error {
if ds.Transform == nil {
ds.Transform = &dataset.Transform{}
}
ds.Transform.ScriptBytes = tfScriptData
ds.Transform.Text = string(tfScriptData)
ds.Transform.ScriptPath = ""
}

if vizScriptData, ok := contents["viz.html"]; ok {
if ds.Viz == nil {
ds.Viz = &dataset.Viz{}
}
ds.Viz.ScriptBytes = vizScriptData
ds.Viz.Text = string(vizScriptData)
ds.Viz.ScriptPath = ""
}

Expand Down
8 changes: 4 additions & 4 deletions base/component/kinds.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ func (rc *ReadmeComponent) WriteTo(dirPath string) (targetFile string, err error
}
if rc.Value != nil && !rc.Value.IsEmpty() {
targetFile = filepath.Join(dirPath, fmt.Sprintf("readme.%s", rc.Format))
if err = ioutil.WriteFile(targetFile, rc.Value.ScriptBytes, WritePerm); err != nil {
if err = ioutil.WriteFile(targetFile, []byte(rc.Value.Text), WritePerm); err != nil {
return
}
}
Expand Down Expand Up @@ -679,7 +679,7 @@ func (tc *TransformComponent) Compare(compare Component) (bool, error) {
// configuration won't be detected by things like status, What's more, because stored transforms include
// a starlark syntax and a "qri" key, comparing FSI to stored JSON won't be equal
// Let's clean this up
return bytes.Equal(tc.Value.ScriptBytes, other.Value.ScriptBytes), nil
return tc.Value.Text == other.Value.Text, nil
}

// WriteTo writes the component as a file to the directory
Expand All @@ -689,7 +689,7 @@ func (tc *TransformComponent) WriteTo(dirPath string) (targetFile string, err er
}
if tc.Value != nil && !tc.Value.IsEmpty() {
targetFile = filepath.Join(dirPath, fmt.Sprintf("transform.%s", tc.Format))
if err = ioutil.WriteFile(targetFile, tc.Value.ScriptBytes, WritePerm); err != nil {
if err = ioutil.WriteFile(targetFile, []byte(tc.Value.Text), WritePerm); err != nil {
return
}
}
Expand Down Expand Up @@ -782,7 +782,7 @@ func (bc *BaseComponent) LoadFile() (map[string]interface{}, error) {
}
return fields, nil
case "html", "md", "star":
fields["ScriptBytes"] = data
fields["text"] = string(data)
return fields, nil
}
return nil, fmt.Errorf("unknown local file format \"%s\"", bc.Format)
Expand Down
2 changes: 1 addition & 1 deletion base/component/list_dir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestListDirectoryComponents(t *testing.T) {
readmeComponent := components.Base().GetSubcomponent("readme").(*ReadmeComponent)
readmeComponent.LoadAndFill(nil)
expectStr = "# Readme\n\nDescribes this dataset.\n"
if diff := cmp.Diff(expectStr, string(readmeComponent.Value.ScriptBytes)); diff != "" {
if diff := cmp.Diff(expectStr, readmeComponent.Value.Text); diff != "" {
t.Errorf("readme component (-want +got):\n%s", diff)
}
}
Expand Down
12 changes: 8 additions & 4 deletions base/dsfs/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,11 @@ func generateCommitDescriptions(ctx context.Context, fs qfs.Filesystem, ds, prev
log.Error("prev.Transform.ScriptPath %q open err: %s", prev.Transform.ScriptPath, err)
} else {
tfFile := prev.Transform.ScriptFile()
prev.Transform.ScriptBytes, err = ioutil.ReadAll(tfFile)
content, err := ioutil.ReadAll(tfFile)
if err != nil {
log.Error("prev.Transform.ScriptPath %q read err: %s", prev.Transform.ScriptPath, err)
}
prev.Transform.Text = string(content)
}
}
if ds.Transform != nil && ds.Transform.ScriptPath != "" {
Expand All @@ -258,10 +259,11 @@ func generateCommitDescriptions(ctx context.Context, fs qfs.Filesystem, ds, prev
log.Errorf("ds.Transform.ScriptPath %q open err: %s", ds.Transform.ScriptPath, err)
} else {
tfFile := ds.Transform.ScriptFile()
ds.Transform.ScriptBytes, err = ioutil.ReadAll(tfFile)
content, err := ioutil.ReadAll(tfFile)
if err != nil {
log.Errorf("ds.Transform.ScriptPath %q read err: %s", ds.Transform.ScriptPath, err)
}
ds.Transform.Text = string(content)
}
// Reopen the transform file so that WriteDataset will be able to write it to the store.
if reopenErr := ds.Transform.OpenScriptFile(ctx, fs); reopenErr != nil {
Expand All @@ -278,10 +280,11 @@ func generateCommitDescriptions(ctx context.Context, fs qfs.Filesystem, ds, prev
log.Error("prev.Readme.ScriptPath %q open err: %s", prev.Readme.ScriptPath, err)
} else {
tfFile := prev.Readme.ScriptFile()
prev.Readme.ScriptBytes, err = ioutil.ReadAll(tfFile)
content, err := ioutil.ReadAll(tfFile)
if err != nil {
log.Error("prev.Readme.ScriptPath %q read err: %s", prev.Readme.ScriptPath, err)
}
prev.Readme.Text = string(content)
}
}
if ds.Readme != nil && ds.Readme.ScriptPath != "" {
Expand All @@ -292,10 +295,11 @@ func generateCommitDescriptions(ctx context.Context, fs qfs.Filesystem, ds, prev
err = nil
} else {
tfFile := ds.Readme.ScriptFile()
ds.Readme.ScriptBytes, err = ioutil.ReadAll(tfFile)
content, err := ioutil.ReadAll(tfFile)
if err != nil {
log.Errorf("ds.Readme.ScriptPath %q read err: %s", ds.Readme.ScriptPath, err)
}
ds.Readme.Text = string(content)
}
if reopenErr := ds.Readme.OpenScriptFile(ctx, fs); reopenErr != nil {
log.Debugf("error reopening readme script file: %q", reopenErr)
Expand Down
32 changes: 16 additions & 16 deletions base/dsfs/dataset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -613,17 +613,17 @@ func TestGenerateCommitMessage(t *testing.T) {
{
"readme modified",
&dataset.Dataset{Readme: &dataset.Readme{
Format: "md",
ScriptBytes: []byte("# hello\n\ncontent\n\n"),
Format: "md",
Text: "# hello\n\ncontent\n\n",
}},
&dataset.Dataset{Readme: &dataset.Readme{
Format: "md",
ScriptBytes: []byte("# hello\n\ncontent\n\nanother line\n\n"),
Format: "md",
Text: "# hello\n\ncontent\n\nanother line\n\n",
}},
false,
// TODO(dlong): Should mention the line added.
"readme updated scriptBytes",
"readme:\n\tupdated scriptBytes",
// TODO(dustmop): Should mention the line added.
"readme updated text",
"readme:\n\tupdated text",
},
{
"body with a small number of changes",
Expand Down Expand Up @@ -690,8 +690,8 @@ hen,twenty-nine,30`),
},
},
Readme: &dataset.Readme{
Format: "md",
ScriptBytes: []byte("# hello\n\ncontent\n\n"),
Format: "md",
Text: "# hello\n\ncontent\n\n",
},
},
&dataset.Dataset{
Expand All @@ -702,13 +702,13 @@ hen,twenty-nine,30`),
},
},
Readme: &dataset.Readme{
Format: "md",
ScriptBytes: []byte("# hello\n\ncontent\n\nanother line\n\n"),
Format: "md",
Text: "# hello\n\ncontent\n\nanother line\n\n",
},
},
false,
"updated meta, structure, and readme",
"meta:\n\tupdated title\nstructure:\n\tupdated formatConfig.headerRow\nreadme:\n\tupdated scriptBytes",
"meta:\n\tupdated title\nstructure:\n\tupdated formatConfig.headerRow\nreadme:\n\tupdated text",
},
{
"meta removed but everything else is the same",
Expand All @@ -720,8 +720,8 @@ hen,twenty-nine,30`),
},
},
Readme: &dataset.Readme{
Format: "md",
ScriptBytes: []byte("# hello\n\ncontent\n\n"),
Format: "md",
Text: "# hello\n\ncontent\n\n",
},
},
&dataset.Dataset{
Expand All @@ -731,8 +731,8 @@ hen,twenty-nine,30`),
},
},
Readme: &dataset.Readme{
Format: "md",
ScriptBytes: []byte("# hello\n\ncontent\n\n"),
Format: "md",
Text: "# hello\n\ncontent\n\n",
},
},
false,
Expand Down
Binary file modified base/dsfs/testdata/zip/exported.zip
Binary file not shown.
8 changes: 4 additions & 4 deletions changes/changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,12 +484,12 @@ func (svc *service) Report(ctx context.Context, leftRef, rightRef string) (*Chan
if leftDs.Readme != nil || rightDs.Readme != nil {
res.Readme = &ChangeReportComponent{}
if leftDs.Readme != nil {
res.Readme.Left = string(leftDs.Readme.ScriptBytes)
res.Readme.Left = leftDs.Readme.Text
} else {
res.Readme.Left = ""
}
if rightDs.Readme != nil {
res.Readme.Right = string(rightDs.Readme.ScriptBytes)
res.Readme.Right = rightDs.Readme.Text
} else {
res.Readme.Right = ""
}
Expand Down Expand Up @@ -548,12 +548,12 @@ func (svc *service) Report(ctx context.Context, leftRef, rightRef string) (*Chan
if leftDs.Transform != nil || rightDs.Transform != nil {
res.Transform = &ChangeReportComponent{}
if leftDs.Transform != nil {
res.Transform.Left = string(leftDs.Transform.ScriptBytes)
res.Transform.Left = leftDs.Transform.Text
} else {
res.Transform.Left = ""
}
if rightDs.Transform != nil {
res.Transform.Right = string(rightDs.Transform.ScriptBytes)
res.Transform.Right = rightDs.Transform.Text
} else {
res.Transform.Right = ""
}
Expand Down
18 changes: 9 additions & 9 deletions cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,9 +470,9 @@ func TestSaveTransformModifiedButSameBody(t *testing.T) {
Storage: local
Size: 7 B
transform updated scriptBytes
transform updated text
transform:
updated scriptBytes
updated text
2 Commit: {{ .commit2 }}
Date: Sun Dec 31 20:01:01 EST 2000
Expand All @@ -482,7 +482,7 @@ func TestSaveTransformModifiedButSameBody(t *testing.T) {
created dataset from tf_123.star
`, map[string]string{
"commit1": "/ipfs/QmbwrkBnhCVNQjitkJeoqBbRYYWoJcmWLwM765acuKPyeG",
"commit1": "/ipfs/QmYiwHLqQHMBaut6QVHqSpvmL7a7kHoFNB13NNCFiykoT7",
"commit2": "/ipfs/QmZx6mYranv8ZFgwq7QVFoVVje2vPueC3MJYPy1LVnANmJ",
})
if diff := cmp.Diff(expect, output); diff != "" {
Expand All @@ -501,14 +501,14 @@ func TestSaveReadmeFromFile(t *testing.T) {

// Verify we can get the readme back
actual := run.MustExec(t, "qri get readme me/save_readme_file")
expect := dstest.Template(t, `format: md
expect := `format: md
qri: rm:0
scriptBytes: IyBUaXRsZQoKVGhpcyBpcyBhIGRhdGFzZXQgYWJvdXQgbW92aWVzCg==
scriptPath: {{ .scriptPath }}
text: |
# Title
`, map[string]string{
"scriptPath": "/ipfs/QmQPbLdDwyAzCmKayuHGeNGx5eboDv5aXTMuw2daUuneCb",
})
This is a dataset about movies
`

if diff := cmp.Diff(expect, actual); diff != "" {
t.Errorf("readme.md contents (-want +got):\n%s", diff)
Expand Down
4 changes: 2 additions & 2 deletions cmd/preview.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ func datasetPreview(ds *dataset.Dataset) *bytes.Buffer {
}
b.WriteString("\n")

if ds.Readme.ScriptBytes != nil {
if ds.Readme.Text != "" {
b.WriteString("Readme:\n")
b.WriteString(fmt.Sprintf(" %s", strings.ReplaceAll(string(ds.Readme.ScriptBytes), "\n", "\n ")))
b.WriteString(fmt.Sprintf(" %s", strings.ReplaceAll(ds.Readme.Text, "\n", "\n ")))
}

return b
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"qri": "md:0",
"title": "different title"
},
"path": "/ipfs/QmamrdwTa3x6woBPpwTpvFFGHAJ2bJudPokTn9jn1kFsiu",
"path": "/ipfs/Qmc8F9NbT41btv5LiTojfZRhDDhFMXNf2gjAhgUKncSBto",
"previousPath": "/ipfs/QmZpWS4BxQkDFd3Vrsnex7EFmCU3zex9GpnfmTmyjCwbfE",
"qri": "ds:0",
"structure": {
Expand Down Expand Up @@ -104,7 +104,7 @@
},
"transform": {
"qri": "tf:0",
"scriptBytes": "IyB0cmFuc2Zvcm0gdGhhdCBkb2Vzbid0IGNoYW5nZSBhbnl0aGluZy4KZGVmIHRyYW5zZm9ybShkcywgY3R4KToKICBwcmludCgiaGVsbG8gUXJpISBcbiIp",
"text": "# transform that doesn't change anything.\ndef transform(ds, ctx):\n print(\"hello Qri! \\n\")",
"scriptPath": "/ipfs/Qmb69tx5VCL7q7EfkGKpDgESBysmDbohoLvonpbgri48NN",
"syntax": "starlark",
"syntaxVersion": "test_version"
Expand All @@ -115,4 +115,4 @@
"renderedPath": "/ipfs/QmW3V8zbPU4wAnzv2zbCjkiTuo7NcsVmaLfFrnHJV1fpKV",
"scriptPath": "/ipfs/QmRaVGip3V9fVBJheZN6FbUajD3ZLNjHhXdjrmfg2JPoo5"
}
}
}
6 changes: 3 additions & 3 deletions cmd/testdata/expect/TestSaveThenOverrideTransform.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"qri": "md:0",
"title": "example movie data"
},
"path": "/ipfs/QmZkjdUKqdw52JwfDww33vG4M1gfWENybXJgCUe35bhtmi",
"path": "/ipfs/QmVAXPQumN3Tb1x9iBQ5xKQDmTX9ZFMq1DRg8NMV7xX9My",
"previousPath": "/ipfs/QmZpWS4BxQkDFd3Vrsnex7EFmCU3zex9GpnfmTmyjCwbfE",
"qri": "ds:0",
"structure": {
Expand Down Expand Up @@ -104,9 +104,9 @@
},
"transform": {
"qri": "tf:0",
"scriptBytes": "IyB0cmFuc2Zvcm0gdGhhdCBkb2Vzbid0IGNoYW5nZSBhbnl0aGluZy4KZGVmIHRyYW5zZm9ybShkcywgY3R4KToKICBwcmludCgiaGVsbG8gUXJpISBcbiIp",
"text": "# transform that doesn't change anything.\ndef transform(ds, ctx):\n print(\"hello Qri! \\n\")",
"scriptPath": "/ipfs/Qmb69tx5VCL7q7EfkGKpDgESBysmDbohoLvonpbgri48NN",
"syntax": "starlark",
"syntaxVersion": "test_version"
}
}
}
Loading

0 comments on commit 3a958ee

Please sign in to comment.