GODRIVER-1466 Respect type map entries for bsontype.EmbeddedDocument (#281)

Author: Divjot Arora

bson/bsoncodec/default_value_decoders.go

@@ -99,6 +99,7 @@ func (dvd DefaultValueDecoders) RegisterDefaultDecoders(rb *RegistryBuilder) {
 		RegisterTypeMapEntry(bsontype.MinKey, tMinKey).
 		RegisterTypeMapEntry(bsontype.MaxKey, tMaxKey).
 		RegisterTypeMapEntry(bsontype.Type(0), tD).
+		RegisterTypeMapEntry(bsontype.EmbeddedDocument, tD).
 		RegisterHookDecoder(tValueUnmarshaler, ValueDecoderFunc(dvd.ValueUnmarshalerDecodeValue)).
 		RegisterHookDecoder(tUnmarshaler, ValueDecoderFunc(dvd.UnmarshalerDecodeValue))
 }

bson/bsoncodec/default_value_decoders_test.go

@@ -2660,7 +2660,7 @@ func TestDefaultValueDecoders(t *testing.T) {
 		})
 	})
 
-	t.Run("EmptyInterfaceDecodeValue", func(t *testing.T) {
+	t.Run("defaultEmptyInterfaceCodec", func(t *testing.T) {
 		t.Run("DecodeValue", func(t *testing.T) {
 			testCases := []struct {
 				name     string
@@ -2777,7 +2777,7 @@ func TestDefaultValueDecoders(t *testing.T) {
 						val := reflect.New(tEmpty).Elem()
 						dc := DecodeContext{Registry: NewRegistryBuilder().Build()}
 						want := ErrNoTypeMapEntry{Type: tc.bsontype}
-						got := dvd.EmptyInterfaceDecodeValue(dc, llvr, val)
+						got := defaultEmptyInterfaceCodec.DecodeValue(dc, llvr, val)
 						if !compareErrors(got, want) {
 							t.Errorf("Errors are not equal. got %v; want %v", got, want)
 						}
@@ -2794,7 +2794,7 @@ func TestDefaultValueDecoders(t *testing.T) {
 								Build(),
 						}
 						want := ErrNoDecoder{Type: reflect.TypeOf(tc.val)}
-						got := dvd.EmptyInterfaceDecodeValue(dc, llvr, val)
+						got := defaultEmptyInterfaceCodec.DecodeValue(dc, llvr, val)
 						if !compareErrors(got, want) {
 							t.Errorf("Errors are not equal. got %v; want %v", got, want)
 						}
@@ -2812,7 +2812,7 @@ func TestDefaultValueDecoders(t *testing.T) {
 								RegisterTypeMapEntry(tc.bsontype, reflect.TypeOf(tc.val)).
 								Build(),
 						}
-						got := dvd.EmptyInterfaceDecodeValue(dc, llvr, reflect.New(tEmpty).Elem())
+						got := defaultEmptyInterfaceCodec.DecodeValue(dc, llvr, reflect.New(tEmpty).Elem())
 						if !compareErrors(got, want) {
 							t.Errorf("Errors are not equal. got %v; want %v", got, want)
 						}
@@ -2828,7 +2828,7 @@ func TestDefaultValueDecoders(t *testing.T) {
 								Build(),
 						}
 						got := reflect.New(tEmpty).Elem()
-						err := dvd.EmptyInterfaceDecodeValue(dc, llvr, got)
+						err := defaultEmptyInterfaceCodec.DecodeValue(dc, llvr, got)
 						noerr(t, err)
 						if !cmp.Equal(got.Interface(), want, cmp.Comparer(compareDecimal128)) {
 							t.Errorf("Did not receive expected value. got %v; want %v", got.Interface(), want)
@@ -2841,7 +2841,7 @@ func TestDefaultValueDecoders(t *testing.T) {
 		t.Run("non-interface{}", func(t *testing.T) {
 			val := uint64(1234567890)
 			want := ValueDecoderError{Name: "EmptyInterfaceDecodeValue", Types: []reflect.Type{tEmpty}, Received: reflect.ValueOf(val)}
-			got := dvd.EmptyInterfaceDecodeValue(DecodeContext{}, nil, reflect.ValueOf(val))
+			got := defaultEmptyInterfaceCodec.DecodeValue(DecodeContext{}, nil, reflect.ValueOf(val))
 			if !compareErrors(got, want) {
 				t.Errorf("Errors are not equal. got %v; want %v", got, want)
 			}
@@ -2850,7 +2850,7 @@ func TestDefaultValueDecoders(t *testing.T) {
 		t.Run("nil *interface{}", func(t *testing.T) {
 			var val interface{}
 			want := ValueDecoderError{Name: "EmptyInterfaceDecodeValue", Types: []reflect.Type{tEmpty}, Received: reflect.ValueOf(val)}
-			got := dvd.EmptyInterfaceDecodeValue(DecodeContext{}, nil, reflect.ValueOf(val))
+			got := defaultEmptyInterfaceCodec.DecodeValue(DecodeContext{}, nil, reflect.ValueOf(val))
 			if !compareErrors(got, want) {
 				t.Errorf("Errors are not equal. got %v; want %v", got, want)
 			}
@@ -2860,7 +2860,7 @@ func TestDefaultValueDecoders(t *testing.T) {
 			llvr := &bsonrwtest.ValueReaderWriter{BSONType: bsontype.Double}
 			want := ErrNoTypeMapEntry{Type: bsontype.Double}
 			val := reflect.New(tEmpty).Elem()
-			got := dvd.EmptyInterfaceDecodeValue(DecodeContext{Registry: NewRegistryBuilder().Build()}, llvr, val)
+			got := defaultEmptyInterfaceCodec.DecodeValue(DecodeContext{Registry: NewRegistryBuilder().Build()}, llvr, val)
 			if !compareErrors(got, want) {
 				t.Errorf("Errors are not equal. got %v; want %v", got, want)
 			}
@@ -2871,12 +2871,94 @@ func TestDefaultValueDecoders(t *testing.T) {
 			want := primitive.D{{"pi", 3.14159}}
 			var got interface{}
 			val := reflect.ValueOf(&got).Elem()
-			err := dvd.EmptyInterfaceDecodeValue(DecodeContext{Registry: buildDefaultRegistry()}, vr, val)
+			err := defaultEmptyInterfaceCodec.DecodeValue(DecodeContext{Registry: buildDefaultRegistry()}, vr, val)
 			noerr(t, err)
 			if !cmp.Equal(got, want) {
 				t.Errorf("Did not get correct result. got %v; want %v", got, want)
 			}
 		})
+		t.Run("custom type map entry", func(t *testing.T) {
+			// registering a custom type map entry for both bsontype.Type(0) anad bsontype.EmbeddedDocument should cause
+			// both top-level and embedded documents to decode to registered type when unmarshalling to interface{}
+
+			topLevelRb := NewRegistryBuilder()
+			defaultValueEncoders.RegisterDefaultEncoders(topLevelRb)
+			defaultValueDecoders.RegisterDefaultDecoders(topLevelRb)
+			topLevelRb.RegisterTypeMapEntry(bsontype.Type(0), reflect.TypeOf(primitive.M{}))
+
+			embeddedRb := NewRegistryBuilder()
+			defaultValueEncoders.RegisterDefaultEncoders(embeddedRb)
+			defaultValueDecoders.RegisterDefaultDecoders(embeddedRb)
+			embeddedRb.RegisterTypeMapEntry(bsontype.Type(0), reflect.TypeOf(primitive.M{}))
+
+			// create doc {"nested": {"foo": 1}}
+			innerDoc := bsoncore.BuildDocument(
+				nil,
+				bsoncore.AppendInt32Element(nil, "foo", 1),
+			)
+			doc := bsoncore.BuildDocument(
+				nil,
+				bsoncore.AppendDocumentElement(nil, "nested", innerDoc),
+			)
+			want := primitive.M{
+				"nested": primitive.M{
+					"foo": int32(1),
+				},
+			}
+
+			testCases := []struct {
+				name     string
+				registry *Registry
+			}{
+				{"top level", topLevelRb.Build()},
+				{"embedded", embeddedRb.Build()},
+			}
+			for _, tc := range testCases {
+				var got interface{}
+				vr := bsonrw.NewBSONDocumentReader(doc)
+				val := reflect.ValueOf(&got).Elem()
+
+				err := defaultEmptyInterfaceCodec.DecodeValue(DecodeContext{Registry: tc.registry}, vr, val)
+				noerr(t, err)
+				if !cmp.Equal(got, want) {
+					t.Fatalf("got %v, want %v", got, want)
+				}
+			}
+		})
+		t.Run("ancestor info is used over custom type map entry", func(t *testing.T) {
+			// If a type map entry is registered for bsontype.EmbeddedDocument, the decoder should use ancestor
+			// information if available instead of the registered entry.
+
+			rb := NewRegistryBuilder()
+			defaultValueEncoders.RegisterDefaultEncoders(rb)
+			defaultValueDecoders.RegisterDefaultDecoders(rb)
+			rb.RegisterTypeMapEntry(bsontype.EmbeddedDocument, reflect.TypeOf(primitive.M{}))
+			reg := rb.Build()
+
+			// build document {"nested": {"foo": 10}}
+			inner := bsoncore.BuildDocument(
+				nil,
+				bsoncore.AppendInt32Element(nil, "foo", 10),
+			)
+			doc := bsoncore.BuildDocument(
+				nil,
+				bsoncore.AppendDocumentElement(nil, "nested", inner),
+			)
+			want := primitive.D{
+				{"nested", primitive.D{
+					{"foo", int32(10)},
+				}},
+			}
+
+			var got primitive.D
+			vr := bsonrw.NewBSONDocumentReader(doc)
+			val := reflect.ValueOf(&got).Elem()
+			err := defaultSliceCodec.DecodeValue(DecodeContext{Registry: reg}, vr, val)
+			noerr(t, err)
+			if !cmp.Equal(got, want) {
+				t.Fatalf("got %v, want %v", got, want)
+			}
+		})
 	})
 }
 

bson/bsoncodec/empty_interface_codec.go

@@ -19,7 +19,6 @@ var defaultEmptyInterfaceCodec = NewEmptyInterfaceCodec()
 
 // EmptyInterfaceCodec is the Codec used for interface{} values.
 type EmptyInterfaceCodec struct {
-	DecodeAsMap         bool
 	DecodeBinaryAsSlice bool
 }
 
@@ -30,9 +29,6 @@ func NewEmptyInterfaceCodec(opts ...*bsonoptions.EmptyInterfaceCodecOptions) *Em
 	interfaceOpt := bsonoptions.MergeEmptyInterfaceCodecOptions(opts...)
 
 	codec := EmptyInterfaceCodec{}
-	if interfaceOpt.DecodeAsMap != nil {
-		codec.DecodeAsMap = *interfaceOpt.DecodeAsMap
-	}
 	if interfaceOpt.DecodeBinaryAsSlice != nil {
 		codec.DecodeBinaryAsSlice = *interfaceOpt.DecodeBinaryAsSlice
 	}
@@ -56,24 +52,49 @@ func (eic EmptyInterfaceCodec) EncodeValue(ec EncodeContext, vw bsonrw.ValueWrit
 	return encoder.EncodeValue(ec, vw, val.Elem())
 }
 
+func (eic EmptyInterfaceCodec) getEmptyInterfaceDecodeType(dc DecodeContext, valueType bsontype.Type) (reflect.Type, error) {
+	isDocument := valueType == bsontype.Type(0) || valueType == bsontype.EmbeddedDocument
+	if isDocument && dc.Ancestor != nil {
+		// Using ancestor information rather than looking up the type map entry forces consistent decoding.
+		// If we're decoding into a bson.D, subdocuments should also be decoded as bson.D, even if a type map entry
+		// has been registered.
+		return dc.Ancestor, nil
+	}
+
+	rtype, err := dc.LookupTypeMapEntry(valueType)
+	if err == nil {
+		return rtype, nil
+	}
+
+	if isDocument {
+		// For documents, fallback to looking up a type map entry for bsontype.Type(0) or bsontype.EmbeddedDocument,
+		// depending on the original valueType.
+		var lookupType bsontype.Type
+		switch valueType {
+		case bsontype.Type(0):
+			lookupType = bsontype.EmbeddedDocument
+		case bsontype.EmbeddedDocument:
+			lookupType = bsontype.Type(0)
+		}
+
+		rtype, err = dc.LookupTypeMapEntry(lookupType)
+		if err == nil {
+			return rtype, nil
+		}
+	}
+
+	return nil, err
+}
+
 // DecodeValue is the ValueDecoderFunc for interface{}.
 func (eic EmptyInterfaceCodec) DecodeValue(dc DecodeContext, vr bsonrw.ValueReader, val reflect.Value) error {
 	if !val.CanSet() || val.Type() != tEmpty {
 		return ValueDecoderError{Name: "EmptyInterfaceDecodeValue", Types: []reflect.Type{tEmpty}, Received: val}
 	}
 
-	rtype, err := dc.LookupTypeMapEntry(vr.Type())
+	rtype, err := eic.getEmptyInterfaceDecodeType(dc, vr.Type())
 	if err != nil {
 		switch vr.Type() {
-		case bsontype.EmbeddedDocument:
-			if dc.Ancestor != nil {
-				rtype = dc.Ancestor
-				break
-			}
-			rtype = tD
-			if eic.DecodeAsMap {
-				rtype = tM
-			}
 		case bsontype.Null:
 			val.Set(reflect.Zero(val.Type()))
 			return vr.ReadNull()

bson/bsoncodec/registry.go

@@ -255,11 +255,10 @@ func (rb *RegistryBuilder) RegisterDefaultDecoder(kind reflect.Kind, dec ValueDe
 // mapping is decoding situations where an empty interface is used and a default type needs to be
 // created and decoded into.
 //
-// NOTE: It is unlikely that registering a type for BSON Embedded Document is actually desired. By
-// registering a type map entry for BSON Embedded Document the type registered will be used in any
-// case where a BSON Embedded Document will be decoded into an empty interface. For example, if you
-// register primitive.M, the EmptyInterface decoder will always use primitive.M, even if an ancestor
-// was a primitive.D.
+// By default, BSON documents will decode into interface{} values as bson.D. To change the default type for BSON
+// documents, a type map entry for bsontype.EmbeddedDocument should be registered. For example, to force BSON documents
+// to decode to bson.Raw, use the following code:
+//	rb.RegisterTypeMapEntry(bsontype.EmbeddedDocument, reflect.TypeOf(bson.Raw{}))
 func (rb *RegistryBuilder) RegisterTypeMapEntry(bt bsontype.Type, rt reflect.Type) *RegistryBuilder {
 	rb.typeMap[bt] = rt
 	return rb

bson/bsonoptions/empty_interface_codec_options.go

@@ -8,7 +8,6 @@ package bsonoptions
 
 // EmptyInterfaceCodecOptions represents all possible options for interface{} encoding and decoding.
 type EmptyInterfaceCodecOptions struct {
-	DecodeAsMap         *bool // Specifies if the default type for decoding should be a bson.M instead of a bson.D. Defaults to false.
 	DecodeBinaryAsSlice *bool // Specifies if Old and Generic type binarys should default to []slice instead of primitive.Binary. Defaults to false.
 }
 
@@ -17,12 +16,6 @@ func EmptyInterfaceCodec() *EmptyInterfaceCodecOptions {
 	return &EmptyInterfaceCodecOptions{}
 }
 
-// SetDecodeAsMap specifies if the default type for decoding should be a bson.M instead of a bson.D. Defaults to false.
-func (e *EmptyInterfaceCodecOptions) SetDecodeAsMap(t bool) *EmptyInterfaceCodecOptions {
-	e.DecodeAsMap = &t
-	return e
-}
-
 // SetDecodeBinaryAsSlice specifies if Old and Generic type binarys should default to []slice instead of primitive.Binary. Defaults to false.
 func (e *EmptyInterfaceCodecOptions) SetDecodeBinaryAsSlice(b bool) *EmptyInterfaceCodecOptions {
 	e.DecodeBinaryAsSlice = &b
@@ -36,9 +29,6 @@ func MergeEmptyInterfaceCodecOptions(opts ...*EmptyInterfaceCodecOptions) *Empty
 		if opt == nil {
 			continue
 		}
-		if opt.DecodeAsMap != nil {
-			e.DecodeAsMap = opt.DecodeAsMap
-		}
 		if opt.DecodeBinaryAsSlice != nil {
 			e.DecodeBinaryAsSlice = opt.DecodeBinaryAsSlice
 		}

bson/mgocompat/registry.go

@@ -55,7 +55,6 @@ func newMgoRegistryBuilder() *bsoncodec.RegistryBuilder {
 			SetAllowUnexportedFields(true))
 	emptyInterCodec := bsoncodec.NewEmptyInterfaceCodec(
 		bsonoptions.EmptyInterfaceCodec().
-			SetDecodeAsMap(true).
 			SetDecodeBinaryAsSlice(true))
 	mapCodec := bsoncodec.NewMapCodec(
 		bsonoptions.MapCodec().
@@ -77,9 +76,10 @@ func newMgoRegistryBuilder() *bsoncodec.RegistryBuilder {
 		RegisterDefaultEncoder(reflect.Uint32, uintcodec).
 		RegisterDefaultEncoder(reflect.Uint64, uintcodec).
 		RegisterTypeMapEntry(bsontype.Int32, tInt).
-		RegisterTypeMapEntry(bsontype.Type(0), tM).
 		RegisterTypeMapEntry(bsontype.DateTime, tTime).
 		RegisterTypeMapEntry(bsontype.Array, tInterfaceSlice).
+		RegisterTypeMapEntry(bsontype.Type(0), tM).
+		RegisterTypeMapEntry(bsontype.EmbeddedDocument, tM).
 		RegisterHookEncoder(tGetter, bsoncodec.ValueEncoderFunc(GetterEncodeValue)).
 		RegisterHookDecoder(tSetter, bsoncodec.ValueDecoderFunc(SetterDecodeValue))