goprotobuf: Handle XXX_unrecognized correctly.
In particular,
- add it to marshaled messages
- examine it in Clone, Equal
R=r
CC=golang-dev
http://codereview.appspot.com/6449091
diff --git a/proto/all_test.go b/proto/all_test.go
index fb6f91a..f83ae3e 100644
--- a/proto/all_test.go
+++ b/proto/all_test.go
@@ -940,6 +940,55 @@
}
}
+// Check that unrecognized fields of a submessage are preserved.
+func TestSubmessageUnrecognizedFields(t *testing.T) {
+ nm := &NewMessage{
+ Nested: &NewMessage_Nested{
+ Name: String("Nigel"),
+ FoodGroup: String("carbs"),
+ },
+ }
+ b, err := Marshal(nm)
+ if err != nil {
+ t.Fatalf("Marshal of NewMessage: %v", err)
+ }
+
+ // Unmarshal into an OldMessage.
+ om := new(OldMessage)
+ if err := Unmarshal(b, om); err != nil {
+ t.Fatalf("Unmarshal to OldMessage: %v", err)
+ }
+ exp := &OldMessage{
+ Nested: &OldMessage_Nested{
+ Name: String("Nigel"),
+ // normal protocol buffer users should not do this
+ XXX_unrecognized: []byte("\x12\x05carbs"),
+ },
+ }
+ if !Equal(om, exp) {
+ t.Errorf("om = %v, want %v", om, exp)
+ }
+
+ // Clone the OldMessage.
+ om = Clone(om).(*OldMessage)
+ if !Equal(om, exp) {
+ t.Errorf("Clone(om) = %v, want %v", om, exp)
+ }
+
+ // Marshal the OldMessage, then unmarshal it into an empty NewMessage.
+ if b, err = Marshal(om); err != nil {
+ t.Fatalf("Marshal of OldMessage: %v", err)
+ }
+ t.Logf("Marshal(%v) -> %q", om, b)
+ nm2 := new(NewMessage)
+ if err := Unmarshal(b, nm2); err != nil {
+ t.Fatalf("Unmarshal to NewMessage: %v", err)
+ }
+ if !Equal(nm, nm2) {
+ t.Errorf("NewMessage round-trip: %v => %v", nm, nm2)
+ }
+}
+
// Check that we can grow an array (repeated field) to have many elements.
// This test doesn't depend only on our encoding; for variety, it makes sure
// we create, encode, and decode the correct contents explicitly. It's therefore
diff --git a/proto/clone.go b/proto/clone.go
index a48f3bc..b23f46b 100644
--- a/proto/clone.go
+++ b/proto/clone.go
@@ -43,8 +43,8 @@
// Clone returns a deep copy of a protocol buffer.
func Clone(pb Message) Message {
in := reflect.ValueOf(pb)
- if in.Kind() != reflect.Ptr || in.Elem().Kind() != reflect.Struct {
- return nil
+ if in.IsNil() {
+ return pb
}
out := reflect.New(in.Type().Elem())
@@ -66,12 +66,17 @@
copyExtension(emOut.ExtensionMap(), emIn.ExtensionMap())
}
- // TODO: Deal with XXX_unrecognized.
+ uin := in.FieldByName("XXX_unrecognized").Bytes()
+ if len(uin) > 0 {
+ out.FieldByName("XXX_unrecognized").SetBytes(append([]byte(nil), uin...))
+ }
}
func copyAny(out, in reflect.Value) {
if in.Type() == protoMessageType {
- out.Set(reflect.ValueOf(Clone(in.Interface().(Message))))
+ if !in.IsNil() {
+ out.Set(reflect.ValueOf(Clone(in.Interface().(Message))))
+ }
return
}
switch in.Kind() {
diff --git a/proto/clone_test.go b/proto/clone_test.go
index c24dca9..d111b47 100644
--- a/proto/clone_test.go
+++ b/proto/clone_test.go
@@ -78,3 +78,10 @@
t.Error("Mutating clone changed the original")
}
}
+
+func TestCloneNil(t *testing.T) {
+ var m *pb.MyMessage
+ if c := proto.Clone(m); !proto.Equal(m, c) {
+ t.Errorf("Clone(%v) = %v", m, c)
+ }
+}
diff --git a/proto/encode.go b/proto/encode.go
index 13a8123..d2d63a8 100644
--- a/proto/encode.go
+++ b/proto/encode.go
@@ -580,5 +580,11 @@
return &ErrRequiredNotSet{t}
}
+ // Add unrecognized fields at the end.
+ v := *(*[]byte)(unsafe.Pointer(base + prop.unrecOffset))
+ if prop.unrecOffset > 0 && len(v) > 0 {
+ o.buf = append(o.buf, v...)
+ }
+
return nil
}
diff --git a/proto/equal.go b/proto/equal.go
index f68ffcc..8c26e75 100644
--- a/proto/equal.go
+++ b/proto/equal.go
@@ -126,7 +126,11 @@
}
}
- // TODO: Deal with XXX_unrecognized.
+ u1 := v1.FieldByName("XXX_unrecognized").Bytes()
+ u2 := v2.FieldByName("XXX_unrecognized").Bytes()
+ if !bytes.Equal(u1, u2) {
+ return false
+ }
return true
}
diff --git a/proto/message_set.go b/proto/message_set.go
index bc9efbe..0571c6e 100644
--- a/proto/message_set.go
+++ b/proto/message_set.go
@@ -36,7 +36,6 @@
*/
import (
- "bytes"
"errors"
"reflect"
)
@@ -70,7 +69,7 @@
type MessageSet struct {
Item []*_MessageSet_Item `protobuf:"group,1,rep"`
- XXX_unrecognized *bytes.Buffer
+ XXX_unrecognized []byte
// TODO: caching?
}
diff --git a/proto/properties.go b/proto/properties.go
index 0db56b5..3da6997 100644
--- a/proto/properties.go
+++ b/proto/properties.go
@@ -82,6 +82,8 @@
tags map[int]int // map from proto tag to struct field number
origNames map[string]int // map from original name to struct field number
order []int // list of struct field numbers in tag order
+
+ unrecOffset uintptr // offset of the XXX_unrecognized []byte field
}
// Implement the sorting interface so we can sort the fields in tag order, as recommended by the spec.
@@ -458,6 +460,9 @@
p.enc = (*Buffer).enc_map
p.dec = nil // not needed
}
+ if f.Name == "XXX_unrecognized" { // special case
+ prop.unrecOffset = f.Offset
+ }
prop.Prop[i] = p
prop.order[i] = i
if debug {
diff --git a/proto/testdata/test.pb.go b/proto/testdata/test.pb.go
index 0a4e5b6..681ad30 100644
--- a/proto/testdata/test.pb.go
+++ b/proto/testdata/test.pb.go
@@ -837,6 +837,78 @@
return ""
}
+type OldMessage struct {
+ Nested *OldMessage_Nested `protobuf:"bytes,1,opt,name=nested" json:"nested,omitempty"`
+ XXX_unrecognized []byte `json:"-"`
+}
+
+func (this *OldMessage) Reset() { *this = OldMessage{} }
+func (this *OldMessage) String() string { return proto.CompactTextString(this) }
+func (*OldMessage) ProtoMessage() {}
+
+func (this *OldMessage) GetNested() *OldMessage_Nested {
+ if this != nil {
+ return this.Nested
+ }
+ return nil
+}
+
+type OldMessage_Nested struct {
+ Name *string `protobuf:"bytes,1,opt,name=name" json:"name,omitempty"`
+ XXX_unrecognized []byte `json:"-"`
+}
+
+func (this *OldMessage_Nested) Reset() { *this = OldMessage_Nested{} }
+func (this *OldMessage_Nested) String() string { return proto.CompactTextString(this) }
+func (*OldMessage_Nested) ProtoMessage() {}
+
+func (this *OldMessage_Nested) GetName() string {
+ if this != nil && this.Name != nil {
+ return *this.Name
+ }
+ return ""
+}
+
+type NewMessage struct {
+ Nested *NewMessage_Nested `protobuf:"bytes,1,opt,name=nested" json:"nested,omitempty"`
+ XXX_unrecognized []byte `json:"-"`
+}
+
+func (this *NewMessage) Reset() { *this = NewMessage{} }
+func (this *NewMessage) String() string { return proto.CompactTextString(this) }
+func (*NewMessage) ProtoMessage() {}
+
+func (this *NewMessage) GetNested() *NewMessage_Nested {
+ if this != nil {
+ return this.Nested
+ }
+ return nil
+}
+
+type NewMessage_Nested struct {
+ Name *string `protobuf:"bytes,1,opt,name=name" json:"name,omitempty"`
+ FoodGroup *string `protobuf:"bytes,2,opt,name=food_group" json:"food_group,omitempty"`
+ XXX_unrecognized []byte `json:"-"`
+}
+
+func (this *NewMessage_Nested) Reset() { *this = NewMessage_Nested{} }
+func (this *NewMessage_Nested) String() string { return proto.CompactTextString(this) }
+func (*NewMessage_Nested) ProtoMessage() {}
+
+func (this *NewMessage_Nested) GetName() string {
+ if this != nil && this.Name != nil {
+ return *this.Name
+ }
+ return ""
+}
+
+func (this *NewMessage_Nested) GetFoodGroup() string {
+ if this != nil && this.FoodGroup != nil {
+ return *this.FoodGroup
+ }
+ return ""
+}
+
type InnerMessage struct {
Host *string `protobuf:"bytes,1,req,name=host" json:"host,omitempty"`
Port *int32 `protobuf:"varint,2,opt,name=port,def=4000" json:"port,omitempty"`
diff --git a/proto/testdata/test.proto b/proto/testdata/test.proto
index 97a28dd..7e76607 100644
--- a/proto/testdata/test.proto
+++ b/proto/testdata/test.proto
@@ -198,6 +198,23 @@
optional string last_field = 536870911;
}
+message OldMessage {
+ message Nested {
+ optional string name = 1;
+ }
+ optional Nested nested = 1;
+}
+
+// NewMessage is wire compatible with OldMessage;
+// imagine it as a future version.
+message NewMessage {
+ message Nested {
+ optional string name = 1;
+ optional string food_group = 2;
+ }
+ optional Nested nested = 1;
+}
+
// Smaller tests for ASCII formatting.
message InnerMessage {