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.

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

switch the json backend to json-iterator #19

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ go 1.12

require (
github.com/davecgh/go-spew v1.1.1
github.com/json-iterator/go v1.1.7
gopkg.in/yaml.v2 v2.2.2
)
14 changes: 14 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/json-iterator/go v1.1.7 h1:KfgG9LzI+pYjr4xvmz/5H4FXjokeP+rlHLhv3iH62Fo=
github.com/json-iterator/go v1.1.7/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421 h1:ZqeYNhU3OHLH3mGKHDcjJRFFRrJa6eAM5H+CtDdOsPc=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742 h1:Esafd1046DLDQ0W1YjYsBW+p8U2u7vzgW2SQVmlNazg=
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
92 changes: 61 additions & 31 deletions yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,60 @@ import (
"bytes"
"encoding/json"
"fmt"
"io"
"reflect"
"strconv"

jsoniter "github.com/json-iterator/go"
"gopkg.in/yaml.v2"
)

// The json-iterator implementation is based on:
// https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go

type customNumberExtension struct {
jsoniter.DummyExtension
}

// createJSONIterator returns a jsoniterator API that's configured to be
// compatible with the encoding/json standard library.
func createJSONIterator() jsoniter.API {
config := jsoniter.Config{
EscapeHTML: true,
SortMapKeys: true,
ValidateJsonRawMessage: true,
}.Froze()
// Force jsoniter to decode number to interface{} via int64/float64, if possible.
config.RegisterExtension(&customNumberExtension{})
return config
}

// createCaseSensitiveStrictJSONIterator returns a jsoniterator API that's configured to be
// case-sensitive, but also disallow unknown fields when unmarshalling. It is compatible with
// the encoding/json standard library.
func createCaseSensitiveStrictJSONIterator() jsoniter.API {
config := jsoniter.Config{
EscapeHTML: true,
SortMapKeys: true,
ValidateJsonRawMessage: true,
CaseSensitive: true,
DisallowUnknownFields: true,
}.Froze()
// Force jsoniter to decode number to interface{} via int64/float64, if possible.
config.RegisterExtension(&customNumberExtension{})
return config
}

// Private copies of jsoniter to try to shield against possible mutations
// from outside. Still does not protect from package level jsoniter.Register*() functions - someone calling them
// in some other library will mess with every usage of the jsoniter library in the whole program.
// See https://github.com/json-iterator/go/issues/265
var jsonIterator = createJSONIterator()
var caseSensitiveStrictJSONIterator = createCaseSensitiveStrictJSONIterator()

// Marshal marshals the object into JSON then converts JSON to YAML and returns the
// YAML.
func Marshal(o interface{}) ([]byte, error) {
j, err := json.Marshal(o)
j, err := caseSensitiveStrictJSONIterator.Marshal(o)
Copy link

Choose a reason for hiding this comment

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

@smarterclayton, didn't you see performance regressions in json-iter marshaling?

Copy link

Choose a reason for hiding this comment

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

Would they really be relevant for YAML encoding? It's not that we do that in critical paths, do we?

Choose a reason for hiding this comment

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

Also, this doesn't affect all of YAML code paths in the current form.

Copy link
Member

Choose a reason for hiding this comment

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

Btw json-iterator isn't fully compatible with the standard library right now, created json-iterator/go#371. PTAL.

Copy link
Member

Choose a reason for hiding this comment

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

json-iterator/go#371 has merged.

if err != nil {
return nil, fmt.Errorf("error marshaling into JSON: %v", err)
}
Expand All @@ -28,23 +71,30 @@ func Marshal(o interface{}) ([]byte, error) {
}

// JSONOpt is a decoding option for decoding from JSON format.
// Deprecated as the backend for parsing JSON is now json-iter.
type JSONOpt func(*json.Decoder) *json.Decoder

// Unmarshal converts YAML to JSON then uses JSON to unmarshal into an object,
// optionally configuring the behavior of the JSON unmarshal.
// Unmarshal converts YAML to JSON then uses JSON to unmarshal into an object.
// Usage of JSONOpt is deprecated.
func Unmarshal(y []byte, o interface{}, opts ...JSONOpt) error {
return yamlUnmarshal(y, o, false, opts...)
return yamlUnmarshal(y, o, false, jsonIterator)
}

// UnmarshalStrict strictly converts YAML to JSON then uses JSON to unmarshal
// into an object, optionally configuring the behavior of the JSON unmarshal.
// into an object. Usage of JSONOpt is deprecated.
func UnmarshalStrict(y []byte, o interface{}, opts ...JSONOpt) error {
return yamlUnmarshal(y, o, true, append(opts, DisallowUnknownFields)...)
return yamlUnmarshal(y, o, true, caseSensitiveStrictJSONIterator)
}

// UnmarshalWithConfig converts YAML to JSON, optionally by using strict mode and then
// uses a custom json-iterator Config object to unmarshal the JSON bytes.
func UnmarshalWithConfig(y []byte, o interface{}, strictYAML bool, jsonConfig jsoniter.API) error {
return yamlUnmarshal(y, o, strictYAML, jsonConfig)
}

// yamlUnmarshal unmarshals the given YAML byte stream into the given interface,
// optionally performing the unmarshalling strictly
func yamlUnmarshal(y []byte, o interface{}, strict bool, opts ...JSONOpt) error {
// optionally performing the unmarshalling strictly.
func yamlUnmarshal(y []byte, o interface{}, strict bool, jsonConfig jsoniter.API) error {
vo := reflect.ValueOf(o)
unmarshalFn := yaml.Unmarshal
if strict {
Expand All @@ -55,27 +105,7 @@ func yamlUnmarshal(y []byte, o interface{}, strict bool, opts ...JSONOpt) error
return fmt.Errorf("error converting YAML to JSON: %v", err)
}

err = jsonUnmarshal(bytes.NewReader(j), o, opts...)
if err != nil {
return fmt.Errorf("error unmarshaling JSON: %v", err)
}

return nil
}

// jsonUnmarshal unmarshals the JSON byte stream from the given reader into the
// object, optionally applying decoder options prior to decoding. We are not
// using json.Unmarshal directly as we want the chance to pass in non-default
// options.
func jsonUnmarshal(r io.Reader, o interface{}, opts ...JSONOpt) error {
d := json.NewDecoder(r)
for _, opt := range opts {
d = opt(d)
}
if err := d.Decode(&o); err != nil {
return fmt.Errorf("while decoding JSON: %v", err)
}
return nil
return jsonConfig.Unmarshal(j, &o)
}

// JSONToYAML Converts JSON to YAML.
Expand Down Expand Up @@ -136,7 +166,7 @@ func yamlToJSON(y []byte, jsonTarget *reflect.Value, yamlUnmarshal func([]byte,
}

// Convert this object to JSON and return the data.
return json.Marshal(jsonObj)
return caseSensitiveStrictJSONIterator.Marshal(jsonObj)
}

func convertToJSONableObject(yamlObj interface{}, jsonTarget *reflect.Value) (interface{}, error) {
Expand Down
14 changes: 0 additions & 14 deletions yaml_go110.go

This file was deleted.

46 changes: 0 additions & 46 deletions yaml_go110_test.go

This file was deleted.

66 changes: 47 additions & 19 deletions yaml_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package yaml

import (
"encoding/json"
"fmt"
"math"
"reflect"
Expand All @@ -10,6 +9,7 @@ import (
"testing"

"github.com/davecgh/go-spew/spew"
jsoniter "github.com/json-iterator/go"
yaml "gopkg.in/yaml.v2"
)

Expand Down Expand Up @@ -38,8 +38,8 @@ func TestMarshal(t *testing.T) {
}

type UnmarshalString struct {
A string
True string
A string
B string
}

type UnmarshalStringMap struct {
Expand Down Expand Up @@ -74,9 +74,9 @@ func TestUnmarshal(t *testing.T) {
e1 = UnmarshalString{A: "true"}
unmarshal(t, y, &s1, &e1)

y = []byte("true: 1")
y = []byte("b: true")
s1 = UnmarshalString{}
e1 = UnmarshalString{True: "1"}
e1 = UnmarshalString{B: "true"}
unmarshal(t, y, &s1, &e1)

y = []byte("a:\n a: 1")
Expand Down Expand Up @@ -124,34 +124,34 @@ func unmarshal(t *testing.T, y []byte, s, e interface{}, opts ...JSONOpt) {
}

func TestUnmarshalStrict(t *testing.T) {
y := []byte("a: 1")
y := []byte("A: 1")
s1 := UnmarshalString{}
e1 := UnmarshalString{A: "1"}
unmarshalStrict(t, y, &s1, &e1)

y = []byte("a: true")
y = []byte("A: true")
s1 = UnmarshalString{}
e1 = UnmarshalString{A: "true"}
unmarshalStrict(t, y, &s1, &e1)

y = []byte("true: 1")
y = []byte("B: true")
s1 = UnmarshalString{}
e1 = UnmarshalString{True: "1"}
e1 = UnmarshalString{B: "true"}
unmarshalStrict(t, y, &s1, &e1)

y = []byte("a:\n a: 1")
y = []byte("A:\n A: 1")
s2 := UnmarshalNestedString{}
e2 := UnmarshalNestedString{NestedString{"1"}}
unmarshalStrict(t, y, &s2, &e2)

y = []byte("a:\n - b: abc\n c: def\n - b: 123\n c: 456\n")
y = []byte("A:\n - B: abc\n C: def\n - B: 123\n C: 456\n")
s3 := UnmarshalSlice{}
e3 := UnmarshalSlice{[]NestedSlice{NestedSlice{"abc", strPtr("def")}, NestedSlice{"123", strPtr("456")}}}
unmarshalStrict(t, y, &s3, &e3)

y = []byte("a:\n b: 1")
y = []byte("A:\n B: 1")
s4 := UnmarshalStringMap{}
e4 := UnmarshalStringMap{map[string]string{"b": "1"}}
e4 := UnmarshalStringMap{map[string]string{"B": "1"}}
unmarshalStrict(t, y, &s4, &e4)

y = []byte(`
Expand Down Expand Up @@ -190,15 +190,15 @@ a:
}

func TestUnmarshalStrictFails(t *testing.T) {
y := []byte("a: true\na: false")
y := []byte("A: true\nA: false")
s1 := UnmarshalString{}
unmarshalStrictFail(t, y, &s1)

y = []byte("a:\n - b: abc\n c: 32\n b: 123")
y = []byte("A:\n - B: abc\n C: 32\n B: 123")
s2 := UnmarshalSlice{}
unmarshalStrictFail(t, y, &s2)

y = []byte("a:\n b: 1\n c: 3")
y = []byte("A:\n B: 1\n C: 3")
s3 := UnmarshalStringMap{}
unmarshalStrictFail(t, y, &s3)

Expand All @@ -225,6 +225,12 @@ unknown: Some-Value
`)
s5 := NamedThing{}
unmarshalStrictFail(t, y, &s5)

// Strict unmarshal should fail for case-sensitive fields; 'a' should be 'A'.
y = []byte("a: test")
s6 := UnmarshalString{}
unmarshalStrictFail(t, y, &s6)

}

func unmarshalStrict(t *testing.T, y []byte, s, e interface{}, opts ...JSONOpt) {
Expand Down Expand Up @@ -489,9 +495,9 @@ func TestJSONObjectToYAMLObject(t *testing.T) {
t.Errorf("jsonToYAML() = %v, want %v", spew.Sdump(got), spew.Sdump(tt.expected))
}

jsonBytes, err := json.Marshal(tt.input)
jsonBytes, err := jsonIterator.Marshal(tt.input)
if err != nil {
t.Fatalf("unexpected json.Marshal error: %v", err)
t.Fatalf("unexpected jsonIterator.Marshal error: %v", err)
}
var gotByRoundtrip yaml.MapSlice
if err := yaml.Unmarshal(jsonBytes, &gotByRoundtrip); err != nil {
Expand Down Expand Up @@ -525,7 +531,7 @@ func TestJSONObjectToYAMLObject(t *testing.T) {
}

if !reflect.DeepEqual(got, gotByRoundtrip) {
t.Errorf("yaml.Unmarshal(json.Marshal(tt.input)) = %v, want %v\njson: %s", spew.Sdump(gotByRoundtrip), spew.Sdump(got), string(jsonBytes))
t.Errorf("yaml.Unmarshal(jsonIterator.Marshal(tt.input)) = %v, want %v\njson: %s", spew.Sdump(gotByRoundtrip), spew.Sdump(got), string(jsonBytes))
}
})
}
Expand All @@ -543,3 +549,25 @@ func sortMapSlicesInPlace(x interface{}) {
})
}
}

func TestUnmarshalWithConfig(t *testing.T) {
config := jsoniter.Config{
EscapeHTML: true,
SortMapKeys: true,
ValidateJsonRawMessage: true,
CaseSensitive: true,
DisallowUnknownFields: true,
}.Froze()

b := []byte("A: test")
s := &UnmarshalString{}
e := &UnmarshalString{A: "test"}

if err := UnmarshalWithConfig(b, s, true, config); err != nil {
t.Fatal("expected no error when using UnmarshalWithConfig")
}
if !reflect.DeepEqual(s, e) {
t.Fatalf("unmarshal YAML was unsuccessful, expected: %+#v, got: %+#v",
e, s)
}
}