Skip to content

Commit

Permalink
Marshal non proto fields with indents (#4027)
Browse files Browse the repository at this point in the history 
* Marshal non proto fields with indents

* - Added tests for marshal non proto fields with indents
- Added JSONBuiltin.MarshalIndent for tests
- Multiple json.MarshalIndent calls changed to json.Indent call after
marshaling

* go fmt and coderabbit review

* do not skip indent tests when encoder setup fails

* coderabbit review

* added missed test variant
  • Loading branch information
@gknw
gknw authored Feb 26, 2024
1 parent f2fd9a8 commit 3b5b1f0
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 13 deletions.
5 changes: 5 additions & 0 deletions runtime/marshal_json.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ func (j *JSONBuiltin) Marshal(v interface{}) ([]byte, error) {
return json.Marshal(v)
}

// MarshalIndent is like Marshal but applies Indent to format the output
func (j *JSONBuiltin) MarshalIndent(v interface{}, prefix, indent string) ([]byte, error) {
return json.MarshalIndent(v, prefix, indent)
}

// Unmarshal unmarshals JSON data into "v".
func (j *JSONBuiltin) Unmarshal(data []byte, v interface{}) error {
return json.Unmarshal(data, v)
Expand Down
89 changes: 83 additions & 6 deletions runtime/marshal_json_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,25 @@ func TestJSONBuiltinMarshal(t *testing.T) {
}

func TestJSONBuiltinMarshalField(t *testing.T) {
var m runtime.JSONBuiltin
var (
m runtime.JSONBuiltin
buf []byte
err error
)

for _, fixt := range builtinFieldFixtures {
buf, err := m.Marshal(fixt.data)
if err != nil {
t.Errorf("m.Marshal(%v) failed with %v; want success", fixt.data, err)
if len(fixt.indent) == 0 {
buf, err = m.Marshal(fixt.data)
if err != nil {
t.Errorf("m.Marshal(%v) failed with %v; want success", fixt.data, err)
}
} else {
buf, err = m.MarshalIndent(fixt.data, "", fixt.indent)
if err != nil {
t.Errorf("m.MarshalIndent(%v, \"\", \"%s\") failed with %v; want success", fixt.data, fixt.indent, err)
}
}

if got, want := string(buf), fixt.json; got != want {
t.Errorf("got = %q; want %q; data = %#v", got, want, fixt.data)
}
Expand Down Expand Up @@ -140,6 +153,18 @@ func TestJSONBuiltinEncoderFields(t *testing.T) {
for _, fixt := range builtinFieldFixtures {
var buf bytes.Buffer
enc := m.NewEncoder(&buf)

if fixt.indent != "" {
if e, ok := enc.(*json.Encoder); ok {
e.SetIndent("", fixt.indent)
} else {
// By default, JSONBuiltin.NewEncoder returns *json.Encoder as runtime.Encoder.
// Otherwise it's better to fail the tests than skip fixtures with non empty indent
t.Errorf("enc is not *json.Encoder, unable to set indentation settings. " +
"This failure prevents testing the correctness of indentation in JSON output.")
}
}

if err := enc.Encode(fixt.data); err != nil {
t.Errorf("enc.Encode(%#v) failed with %v; want success", fixt.data, err)
}
Expand Down Expand Up @@ -189,44 +214,96 @@ func TestJSONBuiltinDecoderFields(t *testing.T) {
}

var (
defaultIndent = " "
builtinFieldFixtures = []struct {
data interface{}
json string
data interface{}
indent string
json string
}{
{data: "", json: `""`},
{data: "", indent: defaultIndent, json: `""`},
{data: proto.String(""), json: `""`},
{data: proto.String(""), indent: defaultIndent, json: `""`},
{data: "foo", json: `"foo"`},
{data: "foo", indent: defaultIndent, json: `"foo"`},
{data: []byte("foo"), json: `"Zm9v"`},
{data: []byte("foo"), indent: defaultIndent, json: `"Zm9v"`},
{data: []byte{}, json: `""`},
{data: []byte{}, indent: defaultIndent, json: `""`},
{data: proto.String("foo"), json: `"foo"`},
{data: proto.String("foo"), indent: defaultIndent, json: `"foo"`},
{data: int32(-1), json: "-1"},
{data: int32(-1), indent: defaultIndent, json: "-1"},
{data: proto.Int32(-1), json: "-1"},
{data: proto.Int32(-1), indent: defaultIndent, json: "-1"},
{data: int64(-1), json: "-1"},
{data: int64(-1), indent: defaultIndent, json: "-1"},
{data: proto.Int64(-1), json: "-1"},
{data: proto.Int64(-1), indent: defaultIndent, json: "-1"},
{data: uint32(123), json: "123"},
{data: uint32(123), indent: defaultIndent, json: "123"},
{data: proto.Uint32(123), json: "123"},
{data: proto.Uint32(123), indent: defaultIndent, json: "123"},
{data: uint64(123), json: "123"},
{data: uint64(123), indent: defaultIndent, json: "123"},
{data: proto.Uint64(123), json: "123"},
{data: proto.Uint64(123), indent: defaultIndent, json: "123"},
{data: float32(-1.5), json: "-1.5"},
{data: float32(-1.5), indent: defaultIndent, json: "-1.5"},
{data: proto.Float32(-1.5), json: "-1.5"},
{data: proto.Float32(-1.5), indent: defaultIndent, json: "-1.5"},
{data: float64(-1.5), json: "-1.5"},
{data: float64(-1.5), indent: defaultIndent, json: "-1.5"},
{data: proto.Float64(-1.5), json: "-1.5"},
{data: proto.Float64(-1.5), indent: defaultIndent, json: "-1.5"},
{data: true, json: "true"},
{data: true, indent: defaultIndent, json: "true"},
{data: proto.Bool(true), json: "true"},
{data: proto.Bool(true), indent: defaultIndent, json: "true"},
{data: (*string)(nil), json: "null"},
{data: (*string)(nil), indent: defaultIndent, json: "null"},
{data: new(emptypb.Empty), json: "{}"},
{data: new(emptypb.Empty), indent: defaultIndent, json: "{}"},
{data: examplepb.NumericEnum_ONE, json: "1"},
{data: examplepb.NumericEnum_ONE, indent: defaultIndent, json: "1"},
{data: nil, json: "null"},
{data: nil, indent: defaultIndent, json: "null"},
{data: (*string)(nil), json: "null"},
{data: (*string)(nil), indent: defaultIndent, json: "null"},
{data: []interface{}{nil, "foo", -1.0, 1.234, true}, json: `[null,"foo",-1,1.234,true]`},
{data: []interface{}{nil, "foo", -1.0, 1.234, true}, indent: defaultIndent, json: "[\n null,\n \"foo\",\n -1,\n 1.234,\n true\n]"},
{
data: map[string]interface{}{"bar": nil, "baz": -1.0, "fiz": 1.234, "foo": true},
json: `{"bar":null,"baz":-1,"fiz":1.234,"foo":true}`,
},
{
data: map[string]interface{}{"bar": nil, "baz": -1.0, "fiz": 1.234, "foo": true},
indent: defaultIndent,
json: "{\n \"bar\": null,\n \"baz\": -1,\n \"fiz\": 1.234,\n \"foo\": true\n}",
},
{
data: (*examplepb.NumericEnum)(proto.Int32(int32(examplepb.NumericEnum_ONE))),
json: "1",
},
{
data: (*examplepb.NumericEnum)(proto.Int32(int32(examplepb.NumericEnum_ONE))),
indent: defaultIndent,
json: "1",
},
{data: map[string]int{"FOO": 0, "BAR": -1}, json: "{\"BAR\":-1,\"FOO\":0}"},
{data: map[string]int{"FOO": 0, "BAR": -1}, indent: defaultIndent, json: "{\n \"BAR\": -1,\n \"FOO\": 0\n}"},
{data: struct {
A string
B int
C map[string]int
}{A: "Go", B: 3, C: map[string]int{"FOO": 0, "BAR": -1}},
json: "{\"A\":\"Go\",\"B\":3,\"C\":{\"BAR\":-1,\"FOO\":0}}"},
{data: struct {
A string
B int
C map[string]int
}{A: "Go", B: 3, C: map[string]int{"FOO": 0, "BAR": -1}}, indent: defaultIndent,
json: "{\n \"A\": \"Go\",\n \"B\": 3,\n \"C\": {\n \"BAR\": -1,\n \"FOO\": 0\n }\n}"},
}
builtinKnownErrors = []struct {
data interface{}
Expand Down
15 changes: 8 additions & 7 deletions runtime/marshal_jsonpb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ func (*JSONPb) ContentType(_ interface{}) string {

// Marshal marshals "v" into JSON.
func (j *JSONPb) Marshal(v interface{}) ([]byte, error) {
if _, ok := v.(proto.Message); !ok {
return j.marshalNonProtoField(v)
}

var buf bytes.Buffer
if err := j.marshalTo(&buf, v); err != nil {
return nil, err
Expand All @@ -48,9 +44,17 @@ func (j *JSONPb) marshalTo(w io.Writer, v interface{}) error {
if err != nil {
return err
}
if j.Indent != "" {
b := &bytes.Buffer{}
if err := json.Indent(b, buf, "", j.Indent); err != nil {
return err
}
buf = b.Bytes()
}
_, err = w.Write(buf)
return err
}

b, err := j.MarshalOptions.Marshal(p)
if err != nil {
return err
Expand Down Expand Up @@ -150,9 +154,6 @@ func (j *JSONPb) marshalNonProtoField(v interface{}) ([]byte, error) {
}
m[fmt.Sprintf("%v", k.Interface())] = (*json.RawMessage)(&buf)
}
if j.Indent != "" {
return json.MarshalIndent(m, "", j.Indent)
}
return json.Marshal(m)
}
if enum, ok := rv.Interface().(protoEnum); ok && !j.UseEnumNumbers {
Expand Down
7 changes: 7 additions & 0 deletions runtime/marshal_jsonpb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,23 @@ func TestJSONPbMarshal(t *testing.T) {
func TestJSONPbMarshalFields(t *testing.T) {
var m runtime.JSONPb
m.UseEnumNumbers = true // builtin fixtures include an enum, expected to be marshaled as int

for _, spec := range builtinFieldFixtures {
m.Indent = spec.indent

buf, err := m.Marshal(spec.data)
if err != nil {
t.Errorf("m.Marshal(%#v) failed with %v; want success", spec.data, err)
}

if got, want := string(buf), spec.json; got != want {
t.Errorf("m.Marshal(%#v) = %q; want %q", spec.data, got, want)
}
}

// Reset m.Indent to ensure no unintended indentation settings carry over to subsequent tests
m.Indent = ""

nums := []examplepb.NumericEnum{examplepb.NumericEnum_ZERO, examplepb.NumericEnum_ONE}

buf, err := m.Marshal(nums)
Expand Down

0 comments on commit 3b5b1f0

Please sign in to comment.