Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(json): add option to write json in multiline mode #213

Merged
merged 2 commits into from
May 17, 2023
Merged

feat(json): add option to write json in multiline mode #213

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e69fb1
add WriteMultiline to save to multiline json format
DmitriyLewen May 15, 2023
755da28
use WriteOption
DmitriyLewen May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions examples/14-tvtomultilinejson/exampletvtomultilinejson.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later

// Example for: *tagvalue*, *json*

// This example demonstrates loading an SPDX tag-value file from disk into memory,
// and re-saving it to a different multiline json file on disk.
// Run project: go run exampletvtomultilinejson.go ../sample-docs/tv/hello.spdx example.json
package main

import (
"fmt"
"os"

"github.com/spdx/tools-golang/json"
"github.com/spdx/tools-golang/tagvalue"
)

func main() {

// check that we've received the right number of arguments
args := os.Args
if len(args) != 3 {
fmt.Printf("Usage: %v <spdx-file-in> <json-file-out>\n", args[0])
fmt.Printf(" Load SPDX tag-value file <spdx-file-in>, and\n")
fmt.Printf(" save it out to multiline JSON <json-file-out>.\n")
return
}

// open the SPDX file
fileIn := args[1]
r, err := os.Open(fileIn)
if err != nil {
fmt.Printf("Error while opening %v for reading: %v", fileIn, err)
return
}
defer r.Close()

// try to load the SPDX file's contents as a tag-value file
doc, err := tagvalue.Read(r)
if err != nil {
fmt.Printf("Error while parsing %v: %v", args[1], err)
return
}

// if we got here, the file is now loaded into memory.
fmt.Printf("Successfully loaded %s\n", args[1])

// we can now save it back to disk, using jsonsaver.

// create a new file for writing
fileOut := args[2]
w, err := os.Create(fileOut)
if err != nil {
fmt.Printf("Error while opening %v for writing: %v", fileOut, err)
return
}
defer w.Close()

// try to save the document to disk as multiline JSON file
err = json.WriteMultiline(doc, w)
if err != nil {
fmt.Printf("Error while saving %v: %v", fileOut, err)
return
}

// it worked
fmt.Printf("Successfully saved %s\n", fileOut)
}
15 changes: 15 additions & 0 deletions json/writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,18 @@ func Write(doc common.AnyDocument, w io.Writer) error {

return nil
}

// WriteMultiline takes an SPDX Document and an io.Writer, and writes the document to the writer in JSON format with indents (multiline format).
func WriteMultiline(doc common.AnyDocument, w io.Writer) error {
Copy link
Collaborator

@kzantow kzantow May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having a second method for this, we might want to introduce a simple options interface so a user can configure this option as well as others (escaping HTML, for example). This would mean we would have to update the Write function. It would look something like (completely untested):

// Write takes an SPDX Document and an io.Writer, and writes the document to the writer in JSON format.
func Write(doc common.AnyDocument, w io.Writer, opts ...WriteOption) error {
	e := json.NewEncoder(w)
	for _, opt := range opts {
		opt(e)
	}
	return e.Encode(doc)
}

type WriteOption func(*json.Encoder)

func Indent(indent string) WriteOption {
	return func(e *json.Encoder) {
		e.SetIndent("", indent)
	}
}

func EscapeHTML(escape bool) WriteOption {
	return func(e *json.Encoder) {
		e.SetEscapeHTML(escape)
	}
}

... and you would call the Write function like:

json.Write(doc, writer, json.Indent("  "))

... or if you also wanted to also prevent HTML escaping (which only partially works, unfortunately; won't expand on why here):

json.Write(doc, writer, json.Indent("  "), json.EscapeHTML(false))

What would you think about this approach?

On a side-note, you can just call json.MarshalIndent(doc, "", " ") directly in your code for this, but agree it would be nice to have this library provide some of the formatting options.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen May 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kzantow Thanks for faster response!

I thought about this way, but i was not sure if you wanted to add arguments to Write() function, so that the other Write() functions ( i mean yaml, json, etc...) have same format.

What would you think about this approach?

I think your solution is better. Tell me if this solution works for you - I'll update this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since adding options as var args allows users to continue to use the same signature they already have been: json.Write(doc, writer), I think this stays consistent with the other Write functions. If you wanted to add analogous options for yaml.Write, I wouldn't be opposed to that, but I don't think it's necessary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on adding vargs which wouldn't change the interface, that would work nicely!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added changes in 755da28

buf, err := json.MarshalIndent(doc, "", " ")
if err != nil {
return err
}

_, err = w.Write(buf)
if err != nil {
return err
}

return nil
}
24 changes: 24 additions & 0 deletions spdx/v2/v2_2/json/json_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,30 @@ func Test_Write(t *testing.T) {
}
}

func Test_MultilineWrite(t *testing.T) {
want := example.Copy()

w := &bytes.Buffer{}

if err := json.WriteMultiline(&want, w); err != nil {
t.Errorf("Write() error = %v", err.Error())
return
}

// we should be able to parse what the writer wrote, and it should be identical to the original struct we wrote
var got spdx.Document
err := json.ReadInto(bytes.NewReader(w.Bytes()), &got)
if err != nil {
t.Errorf("failed to parse written document: %v", err.Error())
return
}

if !cmp.Equal(want, got, cmpopts.IgnoreUnexported(spdx.Package{})) {
t.Errorf("got incorrect struct after writing and re-parsing JSON example: %s", cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{})))
return
}
}

func Test_ShorthandFields(t *testing.T) {
contents := `{
"spdxVersion": "SPDX-2.2",
Expand Down
23 changes: 23 additions & 0 deletions spdx/v2/v2_3/json/json_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,29 @@ func Test_Write(t *testing.T) {
return
}
}
func Test_MultilineWrite(t *testing.T) {
want := example.Copy()

w := &bytes.Buffer{}

if err := json.WriteMultiline(&want, w); err != nil {
t.Errorf("Write() error = %v", err.Error())
return
}

// we should be able to parse what the writer wrote, and it should be identical to the original struct we wrote
var got spdx.Document
err := json.ReadInto(bytes.NewReader(w.Bytes()), &got)
if err != nil {
t.Errorf("failed to parse written document: %v", err.Error())
return
}

if !cmp.Equal(want, got, cmpopts.IgnoreUnexported(spdx.Package{})) {
t.Errorf("got incorrect struct after writing and re-parsing JSON example: %s", cmp.Diff(want, got, cmpopts.IgnoreUnexported(spdx.Package{})))
return
}
}

func Test_ShorthandFields(t *testing.T) {
contents := `{
Expand Down