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

Add JSONObjectToYAMLObject #14

Merged
merged 1 commit into from
Jun 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 61 additions & 0 deletions yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,3 +317,64 @@ func convertToJSONableObject(yamlObj interface{}, jsonTarget *reflect.Value) (in
return yamlObj, nil
}
}

// JSONObjectToYAMLObject converts an in-memory JSON object into a YAML in-memory MapSlice,
// without going through a byte representation. A nil or empty map[string]interface{} input is
// converted to an empty map, i.e. yaml.MapSlice(nil).
//
// interface{} slices stay interface{} slices. map[string]interface{} becomes yaml.MapSlice.
//
// int64 and float64 are down casted following the logic of github.com/go-yaml/yaml:
// - float64s are down-casted as far as possible without data-loss to int, int64, uint64.
// - int64s are down-casted to int if possible without data-loss.
//
// Big int/int64/uint64 do not lose precision as in the json-yaml roundtripping case.
//
// string, bool and any other types are unchanged.
func JSONObjectToYAMLObject(j map[string]interface{}) yaml.MapSlice {
sttts marked this conversation as resolved.
Show resolved Hide resolved
if len(j) == 0 {
return nil
Copy link

Choose a reason for hiding this comment

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

this is surprising... doesn't this mean that an empty map doesn't round trip? if so, doesn't that break uses like CRD status (status: {})

Copy link
Author

Choose a reason for hiding this comment

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

It is what the yaml library does, compare the tests.

Copy link

Choose a reason for hiding this comment

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

hmm... it's still not quite clear to me who is intending to call this helper method, and whether this len(0) => nil treatment is going to bite us... any chance we can fix the yaml library instead?

Copy link
Author

Choose a reason for hiding this comment

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

Not that this is just the top-level conversion func. Further down we have jsonToYAMLValue which is faithful with empty maps.

Copy link
Author

Choose a reason for hiding this comment

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

@liggitt actually this semantics is not that surprising and nothing to change:

A yaml in-memory data structure has interface{}{nil} for null values. yaml.MapSlice(nil) is not null, but the empty map. It marshals to {}.

Copy link

Choose a reason for hiding this comment

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

Ah, ok

}
ret := make(yaml.MapSlice, 0, len(j))
for k, v := range j {
ret = append(ret, yaml.MapItem{Key: k, Value: jsonToYAMLValue(v)})
}
return ret
}

func jsonToYAMLValue(j interface{}) interface{} {
switch j := j.(type) {
case map[string]interface{}:
if j == nil {
return interface{}(nil)
}
return JSONObjectToYAMLObject(j)
case []interface{}:
if j == nil {
return interface{}(nil)
}
ret := make([]interface{}, len(j))
for i := range j {
ret[i] = jsonToYAMLValue(j[i])
}
return ret
case float64:
// replicate the logic in https://github.com/go-yaml/yaml/blob/51d6538a90f86fe93ac480b35f37b2be17fef232/resolve.go#L151
if i64 := int64(j); j == float64(i64) {
if i := int(i64); i64 == int64(i) {
return i
}
return i64
}
if ui64 := uint64(j); j == float64(ui64) {
return ui64
}
return j
case int64:
if i := int(j); j == int64(i) {
return i
}
return j
}
return j
}
122 changes: 122 additions & 0 deletions yaml_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
package yaml

import (
"encoding/json"
"fmt"
"math"
"reflect"
"sort"
"strconv"
"testing"

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

type MarshalTest struct {
Expand Down Expand Up @@ -421,3 +426,120 @@ foo: baz
t.Error("expected YAMLtoJSONStrict to fail on duplicate field names")
}
}

func TestJSONObjectToYAMLObject(t *testing.T) {
intOrInt64 := func(i64 int64) interface{} {
if i := int(i64); i64 == int64(i) {
return i
}
return i64
}

tests := []struct {
name string
input map[string]interface{}
expected yaml.MapSlice
}{
{name: "nil", expected: yaml.MapSlice(nil)},
{name: "empty", input: map[string]interface{}{}, expected: yaml.MapSlice(nil)},
{
name: "values",
input: map[string]interface{}{
"nil slice": []interface{}(nil),
"nil map": map[string]interface{}(nil),
"empty slice": []interface{}{},
"empty map": map[string]interface{}{},
"bool": true,
"float64": float64(42.1),
"fractionless": float64(42),
"int": int(42),
"int64": int64(42),
"int64 big": float64(math.Pow(2, 62)),
"negative int64 big": -float64(math.Pow(2, 62)),
"map": map[string]interface{}{"foo": "bar"},
"slice": []interface{}{"foo", "bar"},
"string": string("foo"),
"uint64 big": float64(math.Pow(2, 63)),
},
expected: yaml.MapSlice{
{Key: "nil slice"},
{Key: "nil map"},
{Key: "empty slice", Value: []interface{}{}},
{Key: "empty map", Value: yaml.MapSlice(nil)},
{Key: "bool", Value: true},
{Key: "float64", Value: float64(42.1)},
{Key: "fractionless", Value: int(42)},
{Key: "int", Value: int(42)},
{Key: "int64", Value: int(42)},
{Key: "int64 big", Value: intOrInt64(int64(1) << 62)},
{Key: "negative int64 big", Value: intOrInt64(-(1 << 62))},
{Key: "map", Value: yaml.MapSlice{{Key: "foo", Value: "bar"}}},
{Key: "slice", Value: []interface{}{"foo", "bar"}},
{Key: "string", Value: string("foo")},
{Key: "uint64 big", Value: uint64(1) << 63},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := JSONObjectToYAMLObject(tt.input)
sttts marked this conversation as resolved.
Show resolved Hide resolved
sortMapSlicesInPlace(tt.expected)
sortMapSlicesInPlace(got)
if !reflect.DeepEqual(got, tt.expected) {
t.Errorf("jsonToYAML() = %v, want %v", spew.Sdump(got), spew.Sdump(tt.expected))
}

jsonBytes, err := json.Marshal(tt.input)
if err != nil {
t.Fatalf("unexpected json.Marshal error: %v", err)
}
var gotByRoundtrip yaml.MapSlice
if err := yaml.Unmarshal(jsonBytes, &gotByRoundtrip); err != nil {
t.Fatalf("unexpected yaml.Unmarshal error: %v", err)
}

// yaml.Unmarshal loses precision, it's rounding to the 4th last digit.
// Replicate this here in the test, but don't change the type.
for i := range got {
switch got[i].Key {
case "int64 big", "uint64 big", "negative int64 big":
switch v := got[i].Value.(type) {
case int64:
d := int64(500)
if v < 0 {
d = -500
}
got[i].Value = int64((v+d)/1000) * 1000
case uint64:
got[i].Value = uint64((v+500)/1000) * 1000
case int:
d := int(500)
if v < 0 {
d = -500
}
got[i].Value = int((v+d)/1000) * 1000
default:
t.Fatalf("unexpected type for key %s: %v:%T", got[i].Key, v, v)
}
}
}

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))
}
})
}
}

func sortMapSlicesInPlace(x interface{}) {
switch x := x.(type) {
case []interface{}:
for i := range x {
sortMapSlicesInPlace(x[i])
}
case yaml.MapSlice:
sort.Slice(x, func(a, b int) bool {
return x[a].Key.(string) < x[b].Key.(string)
})
}
}