merged in golang/protobuf commit 951a149f90371fb8858c6c979d03bb258361…

…1052 - proto: deprecate {Unm,M}arshalMessageSet{JSON}

proto/deprecated.go

@@ -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) {}

proto/message_set.go

@@ -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.
@@ -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:
@@ -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,
-	}
-}

proto/message_set_test.go

@@ -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/gogo/protobuf/proto"
+	. "github.com/gogo/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: {}}
-	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)
 	}
 }

proto/table_unmarshal.go

@@ -138,7 +138,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

protoc-gen-gogo/generator/generator.go

@@ -3095,22 +3095,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() {
-			// isMessageSet = true
-			g.P()
-			g.P("func (m *", mc.goName, ") MarshalJSON() ([]byte, error) {")
-			g.In()
-			g.P("return ", g.Pkg["proto"], ".MarshalMessageSetJSON(&m.XXX_InternalExtensions)")
-			g.Out()
-			g.P("}")
-			g.P("func (m *", mc.goName, ") UnmarshalJSON(buf []byte) error {")
-			g.In()
-			g.P("return ", g.Pkg["proto"], ".UnmarshalMessageSetJSON(buf, &m.XXX_InternalExtensions)")
-			g.Out()
-			g.P("}")
-		}
-
 		g.P()
 		g.P("var extRange_", mc.goName, " = []", g.Pkg["proto"], ".ExtensionRange{")
 		g.In()
@@ -3626,10 +3610,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,
@@ -3653,13 +3635,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)
-		if gogoproto.ImportsGoGoProto(g.file.FileDescriptorProto) && gogoproto.RegistersGolangProto(g.file.FileDescriptorProto) {
-			g.addInitf("%s.RegisterMessageSetType((%s)(nil), %d, %q)", g.Pkg["golang_proto"], fieldType, *field.Number, extName)
-		}
-	}
 
 	g.file.addExport(ext, constOrVarSymbol{ccTypeName, "var", ""})
 }