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

Marshal non proto fields with indents #4027

Merged
merged 7 commits into from
Feb 26, 2024
Merged

Marshal non proto fields with indents #4027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
57bc2b8
Marshal non proto fields with indents
gknw Feb 21, 2024
8b853cf
Merge branch 'grpc-ecosystem:main' into non-proto-ident-marshal
gknw Feb 22, 2024
f8cd1b7
- Added tests for marshal non proto fields with indents
gknw Feb 26, 2024
e447298
go fmt and coderabbit review
gknw Feb 26, 2024
6b320e8
do not skip indent tests when encoder setup fails
gknw Feb 26, 2024
780e50a
coderabbit review
gknw Feb 26, 2024
5e47796
added missed test variant
gknw Feb 26, 2024
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
5 changes: 5 additions & 0 deletions runtime/marshal_json.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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)
}
johanbrandhorst marked this conversation as resolved.
Show resolved Hide resolved

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
Open in desktop
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
Loading