From 57bc2b85dca5119125f8f13ae2b734ba5c8adca9 Mon Sep 17 00:00:00 2001 From: gknw Date: Wed, 21 Feb 2024 23:08:49 +0500 Subject: [PATCH 1/6] Marshal non proto fields with indents --- runtime/marshal_jsonpb.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/runtime/marshal_jsonpb.go b/runtime/marshal_jsonpb.go index 51b8247da2a..4a22aae169c 100644 --- a/runtime/marshal_jsonpb.go +++ b/runtime/marshal_jsonpb.go @@ -156,8 +156,14 @@ func (j *JSONPb) marshalNonProtoField(v interface{}) ([]byte, error) { return json.Marshal(m) } if enum, ok := rv.Interface().(protoEnum); ok && !j.UseEnumNumbers { + if j.Indent != "" { + return json.MarshalIndent(enum.String(), "", j.Indent) + } return json.Marshal(enum.String()) } + if j.Indent != "" { + return json.MarshalIndent(rv.Interface(), "", j.Indent) + } return json.Marshal(rv.Interface()) } From f8cd1b7186a556b5760f318f69a3ac727edb32f8 Mon Sep 17 00:00:00 2001 From: gknw Date: Mon, 26 Feb 2024 18:23:38 +0500 Subject: [PATCH 2/6] - 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 --- runtime/marshal_json.go | 5 +++ runtime/marshal_json_test.go | 65 +++++++++++++++++++++++++++++++--- runtime/marshal_jsonpb.go | 21 +++++------ runtime/marshal_jsonpb_test.go | 6 ++++ 4 files changed, 80 insertions(+), 17 deletions(-) diff --git a/runtime/marshal_json.go b/runtime/marshal_json.go index d6aa8257836..fe52081ab94 100644 --- a/runtime/marshal_json.go +++ b/runtime/marshal_json.go @@ -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) diff --git a/runtime/marshal_json_test.go b/runtime/marshal_json_test.go index 8fe08aeb9bf..ea29c1e444e 100644 --- a/runtime/marshal_json_test.go +++ b/runtime/marshal_json_test.go @@ -38,9 +38,18 @@ 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 len(fixt.indent) == 0 { + buf, err = m.Marshal(fixt.data) + } else { + buf, err = m.MarshalIndent(fixt.data, "", fixt.indent) + } if err != nil { t.Errorf("m.Marshal(%v) failed with %v; want success", fixt.data, err) } @@ -140,6 +149,15 @@ 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 { + continue + } + } + if err := enc.Encode(fixt.data); err != nil { t.Errorf("enc.Encode(%#v) failed with %v; want success", fixt.data, err) } @@ -189,44 +207,83 @@ 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: 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{} diff --git a/runtime/marshal_jsonpb.go b/runtime/marshal_jsonpb.go index 4a22aae169c..8376d1e0efd 100644 --- a/runtime/marshal_jsonpb.go +++ b/runtime/marshal_jsonpb.go @@ -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 @@ -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 @@ -150,20 +154,11 @@ 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 { - if j.Indent != "" { - return json.MarshalIndent(enum.String(), "", j.Indent) - } return json.Marshal(enum.String()) } - if j.Indent != "" { - return json.MarshalIndent(rv.Interface(), "", j.Indent) - } return json.Marshal(rv.Interface()) } diff --git a/runtime/marshal_jsonpb_test.go b/runtime/marshal_jsonpb_test.go index 43029736e37..274182b4323 100644 --- a/runtime/marshal_jsonpb_test.go +++ b/runtime/marshal_jsonpb_test.go @@ -132,16 +132,22 @@ 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) } } + m.Indent = "" + nums := []examplepb.NumericEnum{examplepb.NumericEnum_ZERO, examplepb.NumericEnum_ONE} buf, err := m.Marshal(nums) From e447298ee69e4c4f1eb2168ce9cf3d0059689916 Mon Sep 17 00:00:00 2001 From: gknw Date: Mon, 26 Feb 2024 18:53:16 +0500 Subject: [PATCH 3/6] go fmt and coderabbit review --- runtime/marshal_json_test.go | 12 ++++++++++-- runtime/marshal_jsonpb_test.go | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/runtime/marshal_json_test.go b/runtime/marshal_json_test.go index ea29c1e444e..10d64948ee3 100644 --- a/runtime/marshal_json_test.go +++ b/runtime/marshal_json_test.go @@ -280,9 +280,17 @@ var ( }, {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}}, + {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, + {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 { diff --git a/runtime/marshal_jsonpb_test.go b/runtime/marshal_jsonpb_test.go index 274182b4323..806cdb53578 100644 --- a/runtime/marshal_jsonpb_test.go +++ b/runtime/marshal_jsonpb_test.go @@ -146,6 +146,7 @@ func TestJSONPbMarshalFields(t *testing.T) { } } + // 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} From 6b320e8ee3996d007c3ac162a3d9e8011a47cfb0 Mon Sep 17 00:00:00 2001 From: gknw Date: Mon, 26 Feb 2024 21:22:16 +0500 Subject: [PATCH 4/6] do not skip indent tests when encoder setup fails --- runtime/marshal_json_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/runtime/marshal_json_test.go b/runtime/marshal_json_test.go index 10d64948ee3..b7ec0530d90 100644 --- a/runtime/marshal_json_test.go +++ b/runtime/marshal_json_test.go @@ -154,7 +154,9 @@ func TestJSONBuiltinEncoderFields(t *testing.T) { if e, ok := enc.(*json.Encoder); ok { e.SetIndent("", fixt.indent) } else { - continue + // 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; it is necessary to implement the encoder configuration") } } From 780e50ab5b4b1e82c9a8ee8c0b14d0634024cf45 Mon Sep 17 00:00:00 2001 From: gknw Date: Mon, 26 Feb 2024 21:40:06 +0500 Subject: [PATCH 5/6] coderabbit review --- runtime/marshal_json_test.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/runtime/marshal_json_test.go b/runtime/marshal_json_test.go index b7ec0530d90..89c684eac97 100644 --- a/runtime/marshal_json_test.go +++ b/runtime/marshal_json_test.go @@ -47,12 +47,16 @@ func TestJSONBuiltinMarshalField(t *testing.T) { for _, fixt := range builtinFieldFixtures { 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 err != nil { - t.Errorf("m.Marshal(%v) failed with %v; want success", fixt.data, err) - } + if got, want := string(buf), fixt.json; got != want { t.Errorf("got = %q; want %q; data = %#v", got, want, fixt.data) } @@ -154,9 +158,10 @@ func TestJSONBuiltinEncoderFields(t *testing.T) { 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; it is necessary to implement the encoder configuration") + // 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.") } } From 5e477960bd5c0cce353628d159316bbb881ffa40 Mon Sep 17 00:00:00 2001 From: gknw Date: Mon, 26 Feb 2024 22:07:35 +0500 Subject: [PATCH 6/6] added missed test variant --- runtime/marshal_json_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/runtime/marshal_json_test.go b/runtime/marshal_json_test.go index 89c684eac97..936e10c8ace 100644 --- a/runtime/marshal_json_test.go +++ b/runtime/marshal_json_test.go @@ -285,6 +285,11 @@ var ( 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 {