Skip to content

Commit

Permalink
merged in golang/protobuf commit f05648d464991ab1aa8cf6a499122c56f0f5…
Browse files Browse the repository at this point in the history 
…0f2f - jsonpb: handle map key and value properties properly

Conflicts:
	proto/properties.go
  • Loading branch information
@jmarais
jmarais committed Dec 10, 2018
1 parent be27d1b commit bc71a26
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 30 deletions.
29 changes: 20 additions & 9 deletions jsonpb/jsonpb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,17 +329,17 @@ func (m *Marshaler) marshalObject(out *errWriter, v proto.Message, indent, typeU
m.writeSep(out)
}
// If the map value is a cast type, it may not implement proto.Message, therefore
// allow the struct tag to declare the underlying message type. Instead of changing
// the signatures of the child types (and because prop.mvalue is not public), use
// CustomType as a passer.
// allow the struct tag to declare the underlying message type. Change the property
// of the child types, use CustomType as a passer. CastType currently property is
// not used in json encoding.
if value.Kind() == reflect.Map {
if tag := valueField.Tag.Get("protobuf"); tag != "" {
for _, v := range strings.Split(tag, ",") {
if !strings.HasPrefix(v, "castvaluetype=") {
continue
}
v = strings.TrimPrefix(v, "castvaluetype=")
prop.CustomType = v
prop.MapValProp.CustomType = v
break
}
}
Expand Down Expand Up @@ -649,6 +649,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 @@ -670,7 +671,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 @@ -1151,8 +1156,11 @@ 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
}
}
Expand All @@ -1163,8 +1171,11 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe

// 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
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,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 @@ -745,8 +744,7 @@ var unmarshalingTests = []struct {
// TODO does not work with go version 1.7, but works with go version 1.8 {"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{Salary: 31000}}},
{"oneof spec name", Unmarshaler{}, `{"Country":"Australia"}`, &pb.MsgWithOneof{Union: &pb.MsgWithOneof_Country{Country: "Australia"}}},
Expand Down Expand Up @@ -848,7 +846,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
22 changes: 11 additions & 11 deletions proto/properties.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ type Properties struct {
ctype reflect.Type // set for custom 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 @@ -324,21 +324,21 @@ 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.CustomType = p.CustomType
p.mvalprop.StdDuration = p.StdDuration
p.mvalprop.StdTime = p.StdTime
p.mvalprop.WktPointer = p.WktPointer
p.mvalprop.init(vtype, "Value", f.Tag.Get("protobuf_val"), nil, lockGetProp)
p.MapValProp.CustomType = p.CustomType
p.MapValProp.StdDuration = p.StdDuration
p.MapValProp.StdTime = p.StdTime
p.MapValProp.WktPointer = p.WktPointer
p.MapValProp.init(vtype, "Value", f.Tag.Get("protobuf_val"), nil, lockGetProp)
}
p.setTag(lockGetProp)
}
Expand Down
4 changes: 2 additions & 2 deletions proto/text.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,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 @@ -381,7 +381,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
Original file line number Diff line number Diff line change
Expand Up @@ -636,17 +636,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

0 comments on commit bc71a26

Please sign in to comment.