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.

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

proto: deprecate {Unm,M}arshalMessageSet{JSON} #741

Merged
merged 1 commit into from
Nov 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions proto/deprecated.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,33 @@

package proto

import "errors"

// Deprecated: do not use.
type Stats struct{ Emalloc, Dmalloc, Encode, Decode, Chit, Cmiss, Size uint64 }

// Deprecated: do not use.
func GetStats() Stats { return Stats{} }

// Deprecated: do not use.
func MarshalMessageSet(interface{}) ([]byte, error) {
return nil, errors.New("proto: not implemented")
}

// Deprecated: do not use.
func UnmarshalMessageSet([]byte, interface{}) error {
return errors.New("proto: not implemented")
}

// Deprecated: do not use.
func MarshalMessageSetJSON(interface{}) ([]byte, error) {
return nil, errors.New("proto: not implemented")
}

// Deprecated: do not use.
func UnmarshalMessageSetJSON([]byte, interface{}) error {
return errors.New("proto: not implemented")
}

// Deprecated: do not use.
func RegisterMessageSetType(Message, int32, string) {}
137 changes: 2 additions & 135 deletions proto/message_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,7 @@ package proto
*/

import (
"bytes"
"encoding/json"
"errors"
"fmt"
"reflect"
"sort"
"sync"
)

// errNoMessageTypeID occurs when a protocol buffer does not have a message type ID.
Expand Down Expand Up @@ -145,46 +139,9 @@ func skipVarint(buf []byte) []byte {
return buf[i+1:]
}

// MarshalMessageSet encodes the extension map represented by m in the message set wire format.
// It is called by generated Marshal methods on protocol buffer messages with the message_set_wire_format option.
func MarshalMessageSet(exts interface{}) ([]byte, error) {
return marshalMessageSet(exts, false)
}

// marshaMessageSet implements above function, with the opt to turn on / off deterministic during Marshal.
func marshalMessageSet(exts interface{}, deterministic bool) ([]byte, error) {
switch exts := exts.(type) {
case *XXX_InternalExtensions:
var u marshalInfo
siz := u.sizeMessageSet(exts)
b := make([]byte, 0, siz)
return u.appendMessageSet(b, exts, deterministic)

case map[int32]Extension:
// This is an old-style extension map.
// Wrap it in a new-style XXX_InternalExtensions.
ie := XXX_InternalExtensions{
p: &struct {
mu sync.Mutex
extensionMap map[int32]Extension
}{
extensionMap: exts,
},
}

var u marshalInfo
siz := u.sizeMessageSet(&ie)
b := make([]byte, 0, siz)
return u.appendMessageSet(b, &ie, deterministic)

default:
return nil, errors.New("proto: not an extension map")
}
}

// UnmarshalMessageSet decodes the extension map encoded in buf in the message set wire format.
// unmarshalMessageSet decodes the extension map encoded in buf in the message set wire format.
// It is called by Unmarshal methods on protocol buffer messages with the message_set_wire_format option.
func UnmarshalMessageSet(buf []byte, exts interface{}) error {
func unmarshalMessageSet(buf []byte, exts interface{}) error {
var m map[int32]Extension
switch exts := exts.(type) {
case *XXX_InternalExtensions:
Expand Down Expand Up @@ -222,93 +179,3 @@ func UnmarshalMessageSet(buf []byte, exts interface{}) error {
}
return nil
}

// MarshalMessageSetJSON encodes the extension map represented by m in JSON format.
// It is called by generated MarshalJSON methods on protocol buffer messages with the message_set_wire_format option.
func MarshalMessageSetJSON(exts interface{}) ([]byte, error) {
var m map[int32]Extension
switch exts := exts.(type) {
case *XXX_InternalExtensions:
var mu sync.Locker
m, mu = exts.extensionsRead()
if m != nil {
// Keep the extensions map locked until we're done marshaling to prevent
// races between marshaling and unmarshaling the lazily-{en,de}coded
// values.
mu.Lock()
defer mu.Unlock()
}
case map[int32]Extension:
m = exts
default:
return nil, errors.New("proto: not an extension map")
}
var b bytes.Buffer
b.WriteByte('{')

// Process the map in key order for deterministic output.
ids := make([]int32, 0, len(m))
for id := range m {
ids = append(ids, id)
}
sort.Sort(int32Slice(ids)) // int32Slice defined in text.go

for i, id := range ids {
ext := m[id]
msd, ok := messageSetMap[id]
if !ok {
// Unknown type; we can't render it, so skip it.
continue
}

if i > 0 && b.Len() > 1 {
b.WriteByte(',')
}

fmt.Fprintf(&b, `"[%s]":`, msd.name)

x := ext.value
if x == nil {
x = reflect.New(msd.t.Elem()).Interface()
if err := Unmarshal(ext.enc, x.(Message)); err != nil {
return nil, err
}
}
d, err := json.Marshal(x)
if err != nil {
return nil, err
}
b.Write(d)
}
b.WriteByte('}')
return b.Bytes(), nil
}

// UnmarshalMessageSetJSON decodes the extension map encoded in buf in JSON format.
// It is called by generated UnmarshalJSON methods on protocol buffer messages with the message_set_wire_format option.
func UnmarshalMessageSetJSON(buf []byte, exts interface{}) error {
// Common-case fast path.
if len(buf) == 0 || bytes.Equal(buf, []byte("{}")) {
return nil
}

// This is fairly tricky, and it's not clear that it is needed.
return errors.New("TODO: UnmarshalMessageSetJSON not yet implemented")
}

// A global registry of types that can be used in a MessageSet.

var messageSetMap = make(map[int32]messageSetDesc)

type messageSetDesc struct {
t reflect.Type // pointer to struct
name string
}

// RegisterMessageSetType is called from the generated code.
func RegisterMessageSetType(m Message, fieldNum int32, name string) {
messageSetMap[fieldNum] = messageSetDesc{
t: reflect.TypeOf(m),
name: name,
}
}
77 changes: 44 additions & 33 deletions proto/message_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,49 +29,60 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

package proto
package proto_test

import (
"bytes"
"fmt"
"testing"

"github.com/golang/protobuf/proto"
. "github.com/golang/protobuf/proto/test_proto"
)

func TestUnmarshalMessageSetWithDuplicate(t *testing.T) {
// Check that a repeated message set entry will be concatenated.
in := &messageSet{
Item: []*_MessageSet_Item{
{TypeId: Int32(12345), Message: []byte("hoo")},
{TypeId: Int32(12345), Message: []byte("hah")},
},
}
b, err := Marshal(in)
if err != nil {
t.Fatalf("Marshal: %v", err)
}
t.Logf("Marshaled bytes: %q", b)
/*
Message{
Tag{1, StartGroup},
Message{
Tag{2, Varint}, Uvarint(12345),
Tag{3, Bytes}, Bytes("hoo"),
},
Tag{1, EndGroup},
Tag{1, StartGroup},
Message{
Tag{2, Varint}, Uvarint(12345),
Tag{3, Bytes}, Bytes("hah"),
},
Tag{1, EndGroup},
}
*/
var in []byte
fmt.Sscanf("0b10b9601a03686f6f0c0b10b9601a036861680c", "%x", &in)

var extensions XXX_InternalExtensions
if err := UnmarshalMessageSet(b, &extensions); err != nil {
t.Fatalf("UnmarshalMessageSet: %v", err)
}
ext, ok := extensions.p.extensionMap[12345]
if !ok {
t.Fatalf("Didn't retrieve extension 12345; map is %v", extensions.p.extensionMap)
}
// Skip wire type/field number and length varints.
got := skipVarint(skipVarint(ext.enc))
if want := []byte("hoohah"); !bytes.Equal(got, want) {
t.Errorf("Combined extension is %q, want %q", got, want)
}
}
/*
Message{
Tag{1, StartGroup},
Message{
Tag{2, Varint}, Uvarint(12345),
Tag{3, Bytes}, Bytes("hoohah"),
},
Tag{1, EndGroup},
}
*/
var want []byte
fmt.Sscanf("0b10b9601a06686f6f6861680c", "%x", &want)

func TestMarshalMessageSetJSON_UnknownType(t *testing.T) {
extMap := map[int32]Extension{12345: Extension{}}
got, err := MarshalMessageSetJSON(extMap)
var m MyMessageSet
if err := proto.Unmarshal(in, &m); err != nil {
t.Fatalf("unexpected Unmarshal error: %v", err)
}
got, err := proto.Marshal(&m)
if err != nil {
t.Fatalf("MarshalMessageSetJSON: %v", err)
t.Fatalf("unexpected Marshal error: %v", err)
}
if want := []byte("{}"); !bytes.Equal(got, want) {
t.Errorf("MarshalMessageSetJSON(%v) = %q, want %q", extMap, got, want)

if !bytes.Equal(got, want) {
t.Errorf("output mismatch:\ngot %x\nwant %x", got, want)
}
}
2 changes: 1 addition & 1 deletion proto/table_unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func (u *unmarshalInfo) unmarshal(m pointer, b []byte) error {
u.computeUnmarshalInfo()
}
if u.isMessageSet {
return UnmarshalMessageSet(b, m.offset(u.extensions).toExtensions())
return unmarshalMessageSet(b, m.offset(u.extensions).toExtensions())
}
var reqMask uint64 // bitmask of required fields we've seen.
var errLater error
Expand Down
7 changes: 0 additions & 7 deletions proto/test_proto/test.pb.go

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

17 changes: 0 additions & 17 deletions protoc-gen-go/generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2370,17 +2370,6 @@ func (g *Generator) generateCommonMethods(mc *msgCtx) {

// Extension support methods
if len(mc.message.ExtensionRange) > 0 {
// message_set_wire_format only makes sense when extensions are defined.
if opts := mc.message.Options; opts != nil && opts.GetMessageSetWireFormat() {
g.P()
g.P("func (m *", mc.goName, ") MarshalJSON() ([]byte, error) {")
g.P("return ", g.Pkg["proto"], ".MarshalMessageSetJSON(&m.XXX_InternalExtensions)")
g.P("}")
g.P("func (m *", mc.goName, ") UnmarshalJSON(buf []byte) error {")
g.P("return ", g.Pkg["proto"], ".UnmarshalMessageSetJSON(buf, &m.XXX_InternalExtensions)")
g.P("}")
}

g.P()
g.P("var extRange_", mc.goName, " = []", g.Pkg["proto"], ".ExtensionRange{")
for _, r := range mc.message.ExtensionRange {
Expand Down Expand Up @@ -2788,10 +2777,8 @@ func (g *Generator) generateExtension(ext *ExtensionDescriptor) {
// In addition, the situation for when to apply this special case is implemented
// differently in other languages:
// https://github.com/google/protobuf/blob/aff10976/src/google/protobuf/text_format.cc#L1560
mset := false
if extDesc.GetOptions().GetMessageSetWireFormat() && typeName[len(typeName)-1] == "message_set_extension" {
typeName = typeName[:len(typeName)-1]
mset = true
}

// For text formatting, the package must be exactly what the .proto file declares,
Expand All @@ -2813,10 +2800,6 @@ func (g *Generator) generateExtension(ext *ExtensionDescriptor) {
g.P()

g.addInitf("%s.RegisterExtension(%s)", g.Pkg["proto"], ext.DescName())
if mset {
// Generate a bit more code to register with message_set.go.
g.addInitf("%s.RegisterMessageSetType((%s)(nil), %d, %q)", g.Pkg["proto"], fieldType, *field.Number, extName)
}

g.file.addExport(ext, constOrVarSymbol{ccTypeName, "var", ""})
}
Expand Down
7 changes: 0 additions & 7 deletions protoc-gen-go/testdata/extension_base/extension_base.pb.go

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

1 change: 0 additions & 1 deletion protoc-gen-go/testdata/extension_user/extension_user.pb.go

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

7 changes: 0 additions & 7 deletions protoc-gen-go/testdata/my_test/test.pb.go

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