Skip to content

Commit

Permalink
Implement oneof support.
Browse files Browse the repository at this point in the history
This includes the code generation changes,
and the infrastructure to wire it up to the encode/decode machinery.

The overall API changes are these:
  - oneofs in a message are replaced by a single interface field
  - each field in a oneof gets a distinguished type that satisfies
    the corresponding interface
  - a type switch may be used to distinguish between oneof fields

Fixes #29.
  • Loading branch information
dsymonds committed Aug 24, 2015
1 parent 1e35a3a commit 59b73b3
Show file tree
Hide file tree
Showing 28 changed files with 1,821 additions and 120 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,6 @@ nuke:
regenerate:
make -C protoc-gen-go/descriptor regenerate
make -C protoc-gen-go/plugin regenerate
make -C protoc-gen-go/testdata regenerate
make -C proto/testdata regenerate
make -C jsonpb/jsonpb_test_proto regenerate
50 changes: 50 additions & 0 deletions jsonpb/jsonpb.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,18 @@ func (m *Marshaler) marshalObject(out *errWriter, v proto.Message, indent string
}
}

// Oneof fields need special handling.
if valueField.Tag.Get("protobuf_oneof") != "" {
// value is an interface containing &T{real_value}.
sv := value.Elem().Elem() // interface -> *T -> T
value = sv.Field(0)
valueField = sv.Type().Field(0)

var p proto.Properties
p.Parse(sv.Type().Field(0).Tag.Get("protobuf"))
fieldName = p.OrigName
}

out.write(writeBeforeField)
if m.Indent != "" {
out.write(indent)
Expand Down Expand Up @@ -319,6 +331,44 @@ func unmarshalValue(target reflect.Value, inputValue json.RawMessage) error {
delete(jsonFields, fieldName)
}
}
// Check for any oneof fields.
// This might be slow; we can optimise it if it becomes a problem.
type oneofMessage interface {
XXX_OneofFuncs() (func(proto.Message, *proto.Buffer) error, func(proto.Message, int, int, *proto.Buffer) (bool, error), []interface{})
}
var oneofTypes []interface{}
if om, ok := reflect.Zero(reflect.PtrTo(targetType)).Interface().(oneofMessage); ok {
_, _, oneofTypes = om.XXX_OneofFuncs()
}
for fname, raw := range jsonFields {
for _, oot := range oneofTypes {
sp := reflect.ValueOf(oot).Type() // *T
var props proto.Properties
props.Parse(sp.Elem().Field(0).Tag.Get("protobuf"))
if props.OrigName != fname {
continue
}
nv := reflect.New(sp.Elem())
// There will be exactly one interface field that
// this new value is assignable to.
for i := 0; i < targetType.NumField(); i++ {
f := targetType.Field(i)
if f.Type.Kind() != reflect.Interface {
continue
}
if !nv.Type().AssignableTo(f.Type) {
continue
}
target.Field(i).Set(nv)
break
}
if err := unmarshalValue(nv.Elem().Field(0), raw); err != nil {
return err
}
delete(jsonFields, fname)
break
}
}
if len(jsonFields) > 0 {
// Pick any field to be the scapegoat.
var f string
Expand Down
7 changes: 5 additions & 2 deletions jsonpb/jsonpb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
package jsonpb

import (
"reflect"
"testing"

pb "github.com/golang/protobuf/jsonpb/jsonpb_test_proto"
Expand Down Expand Up @@ -291,6 +292,8 @@ var marshalingTests = []struct {
{"proto2 map<bool, Object>", marshaler,
&pb.Maps{MBoolSimple: map[bool]*pb.Simple{true: &pb.Simple{OInt32: proto.Int32(1)}}},
`{"m_bool_simple":{"true":{"o_int32":1}}}`},
{"oneof, not set", marshaler, &pb.MsgWithOneof{}, `{}`},
{"oneof, set", marshaler, &pb.MsgWithOneof{Union: &pb.MsgWithOneof_Title{"Grand Poobah"}}, `{"title":"Grand Poobah"}`},
}

func TestMarshaling(t *testing.T) {
Expand Down Expand Up @@ -325,13 +328,13 @@ var unmarshalingTests = []struct {
{"map<int64, int32>", `{"nummy":{"1":2,"3":4}}`, &pb.Mappy{Nummy: map[int64]int32{1: 2, 3: 4}}},
{"map<string, string>", `{"strry":{"\"one\"":"two","three":"four"}}`, &pb.Mappy{Strry: map[string]string{`"one"`: "two", "three": "four"}}},
{"map<int32, Object>", `{"objjy":{"1":{"dub":1}}}`, &pb.Mappy{Objjy: map[int32]*pb.Simple3{1: &pb.Simple3{Dub: 1}}}},
{"oneof", `{"salary":31000}`, &pb.MsgWithOneof{Union: &pb.MsgWithOneof_Salary{31000}}},
}

func TestUnmarshaling(t *testing.T) {
for _, tt := range unmarshalingTests {
// Make a new instance of the type of our expected object.
p := proto.Clone(tt.pb)
p.Reset()
p := reflect.New(reflect.TypeOf(tt.pb).Elem()).Interface().(proto.Message)

err := UnmarshalString(tt.json, p)
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions jsonpb/jsonpb_test_proto/more_test_objects.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

96 changes: 96 additions & 0 deletions jsonpb/jsonpb_test_proto/test_objects.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions jsonpb/jsonpb_test_proto/test_objects.proto
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,10 @@ message Maps {
map<int64, string> m_int64_str = 1;
map<bool, Simple> m_bool_simple = 2;
}

message MsgWithOneof {
oneof union {
string title = 1;
int64 salary = 2;
}
}
52 changes: 52 additions & 0 deletions proto/all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1958,6 +1958,58 @@ func TestMapFieldWithNil(t *testing.T) {
}
}

func TestOneof(t *testing.T) {
m := &Communique{}
b, err := Marshal(m)
if err != nil {
t.Fatalf("Marshal of empty message with oneof: %v", err)
}
if len(b) != 0 {
t.Errorf("Marshal of empty message yielded too many bytes: %v", b)
}

m = &Communique{
Union: &Communique_Name{"Barry"},
}

// Round-trip.
b, err = Marshal(m)
if err != nil {
t.Fatalf("Marshal of message with oneof: %v", err)
}
if len(b) != 7 { // name tag/wire (1) + name len (1) + name (5)
t.Errorf("Incorrect marshal of message with oneof: %v", b)
}
m.Reset()
if err := Unmarshal(b, m); err != nil {
t.Fatalf("Unmarshal of message with oneof: %v", err)
}
if x, ok := m.Union.(*Communique_Name); !ok || x.Name != "Barry" {
t.Errorf("After round trip, Union = %+v", m.Union)
}
if name := m.GetName(); name != "Barry" {
t.Errorf("After round trip, GetName = %q, want %q", name, "Barry")
}

// Let's try with a message in the oneof.
m.Union = &Communique_Msg{&Strings{StringField: String("deep deep string")}}
b, err = Marshal(m)
if err != nil {
t.Fatalf("Marshal of message with oneof set to message: %v", err)
}
if len(b) != 20 { // msg tag/wire (1) + msg len (1) + msg (1 + 1 + 16)
t.Errorf("Incorrect marshal of message with oneof set to message: %v", b)
}
m.Reset()
if err := Unmarshal(b, m); err != nil {
t.Fatalf("Unmarshal of message with oneof set to message: %v", err)
}
ss, ok := m.Union.(*Communique_Msg)
if !ok || ss.Msg.GetStringField() != "deep deep string" {
t.Errorf("After round trip with oneof set to message, Union = %+v", m.Union)
}
}

// Benchmarks

func testMsg() *GoTest {
Expand Down
7 changes: 7 additions & 0 deletions proto/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,13 @@ func mergeAny(out, in reflect.Value, viaPtr bool, prop *Properties) {
return
}
out.Set(in)
case reflect.Interface:
// Probably a oneof field; copy non-nil values.
if in.IsNil() {
return
}
out.Set(reflect.New(in.Elem().Elem().Type())) // interface -> *T -> T -> new(T)
mergeAny(out.Elem(), in.Elem(), false, nil)
case reflect.Map:
if in.Len() == 0 {
return
Expand Down
22 changes: 22 additions & 0 deletions proto/clone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,28 @@ var mergeTests = []struct {
Data: []byte("texas!"),
},
},
// Oneof fields should merge by assignment.
{
src: &pb.Communique{
Union: &pb.Communique_Number{41},
},
dst: &pb.Communique{
Union: &pb.Communique_Name{"Bobby Tables"},
},
want: &pb.Communique{
Union: &pb.Communique_Number{41},
},
},
// Oneof nil is the same as not set.
{
src: &pb.Communique{},
dst: &pb.Communique{
Union: &pb.Communique_Name{"Bobby Tables"},
},
want: &pb.Communique{
Union: &pb.Communique_Name{"Bobby Tables"},
},
},
}

func TestMerge(t *testing.T) {
Expand Down
Loading

0 comments on commit 59b73b3

Please sign in to comment.