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

jsonpb: handle map keys' and values' properties properly #645

Merged
merged 1 commit into from
Jul 6, 2018
Merged

jsonpb: handle map keys' and values' properties properly #645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 16 additions & 5 deletions jsonpb/jsonpb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ func (m *Marshaler) marshalValue(out *errWriter, prop *proto.Properties, v refle
out.write(m.Indent)
}

// TODO handle map key prop properly
b, err := json.Marshal(k.Interface())
if err != nil {
return err
Expand All @@ -586,7 +587,11 @@ func (m *Marshaler) marshalValue(out *errWriter, prop *proto.Properties, v refle
out.write(` `)
}

if err := m.marshalValue(out, prop, v.MapIndex(k), indent+m.Indent); err != nil {
vprop := prop
if prop != nil && prop.MapValProp != nil {
vprop = prop.MapValProp
}
if err := m.marshalValue(out, vprop, v.MapIndex(k), indent+m.Indent); err != nil {
return err
}
}
Expand Down Expand Up @@ -1010,16 +1015,22 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
k = reflect.ValueOf(ks)
} else {
k = reflect.New(targetType.Key()).Elem()
// TODO: pass the correct Properties if needed.
if err := u.unmarshalValue(k, json.RawMessage(ks), nil); err != nil {
var kprop *proto.Properties
if prop != nil && prop.MapKeyProp != nil {
kprop = prop.MapKeyProp
}
if err := u.unmarshalValue(k, json.RawMessage(ks), kprop); err != nil {
return err
}
}

// Unmarshal map value.
v := reflect.New(targetType.Elem()).Elem()
// TODO: pass the correct Properties if needed.
if err := u.unmarshalValue(v, raw, nil); err != nil {
var vprop *proto.Properties
if prop != nil && prop.MapValProp != nil {
vprop = prop.MapValProp
}
if err := u.unmarshalValue(v, raw, vprop); err != nil {
return err
}
target.SetMapIndex(k, v)
Expand Down
8 changes: 3 additions & 5 deletions jsonpb/jsonpb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,7 @@ var marshalingTests = []struct {
{"map<int64, string>", marshaler, &pb.Mappy{Buggy: map[int64]string{1234: "yup"}},
`{"buggy":{"1234":"yup"}}`},
{"map<bool, bool>", marshaler, &pb.Mappy{Booly: map[bool]bool{false: true}}, `{"booly":{"false":true}}`},
// TODO: This is broken.
//{"map<string, enum>", marshaler, &pb.Mappy{Enumy: map[string]pb.Numeral{"XIV": pb.Numeral_ROMAN}}, `{"enumy":{"XIV":"ROMAN"}`},
{"map<string, enum>", marshaler, &pb.Mappy{Enumy: map[string]pb.Numeral{"XIV": pb.Numeral_ROMAN}}, `{"enumy":{"XIV":"ROMAN"}}`},
{"map<string, enum as int>", Marshaler{EnumsAsInts: true}, &pb.Mappy{Enumy: map[string]pb.Numeral{"XIV": pb.Numeral_ROMAN}}, `{"enumy":{"XIV":2}}`},
{"map<int32, bool>", marshaler, &pb.Mappy{S32Booly: map[int32]bool{1: true, 3: false, 10: true, 12: false}}, `{"s32booly":{"1":true,"3":false,"10":true,"12":false}}`},
{"map<int64, bool>", marshaler, &pb.Mappy{S64Booly: map[int64]bool{1: true, 3: false, 10: true, 12: false}}, `{"s64booly":{"1":true,"3":false,"10":true,"12":false}}`},
Expand Down Expand Up @@ -748,8 +747,7 @@ var unmarshalingTests = []struct {
{"Any with message and indent", Unmarshaler{}, anySimplePrettyJSON, anySimple},
{"Any with WKT", Unmarshaler{}, anyWellKnownJSON, anyWellKnown},
{"Any with WKT and indent", Unmarshaler{}, anyWellKnownPrettyJSON, anyWellKnown},
// TODO: This is broken.
//{"map<string, enum>", Unmarshaler{}, `{"enumy":{"XIV":"ROMAN"}`, &pb.Mappy{Enumy: map[string]pb.Numeral{"XIV": pb.Numeral_ROMAN}}},
{"map<string, enum>", Unmarshaler{}, `{"enumy":{"XIV":"ROMAN"}}`, &pb.Mappy{Enumy: map[string]pb.Numeral{"XIV": pb.Numeral_ROMAN}}},
{"map<string, enum as int>", Unmarshaler{}, `{"enumy":{"XIV":2}}`, &pb.Mappy{Enumy: map[string]pb.Numeral{"XIV": pb.Numeral_ROMAN}}},
{"oneof", Unmarshaler{}, `{"salary":31000}`, &pb.MsgWithOneof{Union: &pb.MsgWithOneof_Salary{31000}}},
{"oneof spec name", Unmarshaler{}, `{"Country":"Australia"}`, &pb.MsgWithOneof{Union: &pb.MsgWithOneof_Country{"Australia"}}},
Expand Down Expand Up @@ -854,7 +852,7 @@ func TestUnmarshaling(t *testing.T) {

err := tt.unmarshaler.Unmarshal(strings.NewReader(tt.json), p)
if err != nil {
t.Errorf("%s: %v", tt.desc, err)
t.Errorf("unmarshalling %s: %v", tt.desc, err)
continue
}

Expand Down
14 changes: 7 additions & 7 deletions proto/properties.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,9 @@ type Properties struct {
stype reflect.Type // set for struct types only
sprop *StructProperties // set for struct types only

mtype reflect.Type // set for map types only
mkeyprop *Properties // set for map types only
mvalprop *Properties // set for map types only
mtype reflect.Type // set for map types only
MapKeyProp *Properties // set for map types only
MapValProp *Properties // set for map types only
}

// String formats the properties in the protobuf struct field tag style.
Expand Down Expand Up @@ -275,16 +275,16 @@ func (p *Properties) setFieldProps(typ reflect.Type, f *reflect.StructField, loc

case reflect.Map:
p.mtype = t1
p.mkeyprop = &Properties{}
p.mkeyprop.init(reflect.PtrTo(p.mtype.Key()), "Key", f.Tag.Get("protobuf_key"), nil, lockGetProp)
p.mvalprop = &Properties{}
p.MapKeyProp = &Properties{}
p.MapKeyProp.init(reflect.PtrTo(p.mtype.Key()), "Key", f.Tag.Get("protobuf_key"), nil, lockGetProp)
p.MapValProp = &Properties{}
vtype := p.mtype.Elem()
if vtype.Kind() != reflect.Ptr && vtype.Kind() != reflect.Slice {
// The value type is not a message (*T) or bytes ([]byte),
// so we need encoders for the pointer to this type.
vtype = reflect.PtrTo(vtype)
}
p.mvalprop.init(vtype, "Value", f.Tag.Get("protobuf_val"), nil, lockGetProp)
p.MapValProp.init(vtype, "Value", f.Tag.Get("protobuf_val"), nil, lockGetProp)
}

if p.stype != nil {
Expand Down
4 changes: 2 additions & 2 deletions proto/text.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ func (tm *TextMarshaler) writeStruct(w *textWriter, sv reflect.Value) error {
return err
}
}
if err := tm.writeAny(w, key, props.mkeyprop); err != nil {
if err := tm.writeAny(w, key, props.MapKeyProp); err != nil {
return err
}
if err := w.WriteByte('\n'); err != nil {
Expand All @@ -370,7 +370,7 @@ func (tm *TextMarshaler) writeStruct(w *textWriter, sv reflect.Value) error {
return err
}
}
if err := tm.writeAny(w, val, props.mvalprop); err != nil {
if err := tm.writeAny(w, val, props.MapValProp); err != nil {
return err
}
if err := w.WriteByte('\n'); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions proto/text_parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,17 +630,17 @@ func (p *textParser) readStruct(sv reflect.Value, terminator string) error {
if err := p.consumeToken(":"); err != nil {
return err
}
if err := p.readAny(key, props.mkeyprop); err != nil {
if err := p.readAny(key, props.MapKeyProp); err != nil {
return err
}
if err := p.consumeOptionalSeparator(); err != nil {
return err
}
case "value":
if err := p.checkForColon(props.mvalprop, dst.Type().Elem()); err != nil {
if err := p.checkForColon(props.MapValProp, dst.Type().Elem()); err != nil {
return err
}
if err := p.readAny(val, props.mvalprop); err != nil {
if err := p.readAny(val, props.MapValProp); err != nil {
return err
}
if err := p.consumeOptionalSeparator(); err != nil {
Expand Down