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.

Sign up for GitHub

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

Jump to bottom

GODRIVER-2714 POC: Eliminate *Context types used by BSON Encoder or Decoder. #1612

Closed
wants to merge 16 commits into from
Closed

GODRIVER-2714 POC: Eliminate *Context types used by BSON Encoder or Decoder. #1612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9f587a2
WIP
qingyang-hu Apr 24, 2024
d8fba65
WIP
qingyang-hu Apr 29, 2024
1834500
WIP
qingyang-hu Apr 30, 2024
dfdb9c3
WIP
qingyang-hu May 1, 2024
394b263
WIP
qingyang-hu May 10, 2024
51517bc
WIP
qingyang-hu May 14, 2024
7938e90
WIP
qingyang-hu May 14, 2024
b2a1c15
WIP
qingyang-hu May 20, 2024
7a0b8bf
WIP
qingyang-hu May 20, 2024
5ce2670
WIP
qingyang-hu May 21, 2024
6ca8e11
WIP
qingyang-hu May 22, 2024
9c32dc7
WIP
qingyang-hu May 23, 2024
7f7a86b
WIP
qingyang-hu May 24, 2024
e9b3bfa
WIP
qingyang-hu May 25, 2024
757b0b2
Merge branch 'master' into godriver2796
qingyang-hu May 25, 2024
c15afa8
update struct codec
qingyang-hu May 26, 2024
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
21 changes: 4 additions & 17 deletions bson/array_codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,11 @@ import (
"go.mongodb.org/mongo-driver/x/bsonx/bsoncore"
)

// ArrayCodec is the Codec used for bsoncore.Array values.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.NewRegistry] to get a registry with the
// ArrayCodec registered.
type ArrayCodec struct{}

var defaultArrayCodec = NewArrayCodec()

// NewArrayCodec returns an ArrayCodec.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.NewRegistry] to get a registry with the
// ArrayCodec registered.
func NewArrayCodec() *ArrayCodec {
return &ArrayCodec{}
}
// arrayCodec is the Codec used for bsoncore.Array values.
type arrayCodec struct{}

// EncodeValue is the ValueEncoder for bsoncore.Array values.
func (ac *ArrayCodec) EncodeValue(_ EncodeContext, vw ValueWriter, val reflect.Value) error {
func (ac *arrayCodec) EncodeValue(_ EncoderRegistry, vw ValueWriter, val reflect.Value) error {
if !val.IsValid() || val.Type() != tCoreArray {
return ValueEncoderError{Name: "CoreArrayEncodeValue", Types: []reflect.Type{tCoreArray}, Received: val}
}
Expand All @@ -39,7 +26,7 @@ func (ac *ArrayCodec) EncodeValue(_ EncodeContext, vw ValueWriter, val reflect.V
}

// DecodeValue is the ValueDecoder for bsoncore.Array values.
func (ac *ArrayCodec) DecodeValue(_ DecodeContext, vr ValueReader, val reflect.Value) error {
func (ac *arrayCodec) DecodeValue(_ DecoderRegistry, vr ValueReader, val reflect.Value) error {
if !val.CanSet() || val.Type() != tCoreArray {
return ValueDecoderError{Name: "CoreArrayDecodeValue", Types: []reflect.Type{tCoreArray}, Received: val}
}
Expand Down
21 changes: 9 additions & 12 deletions bson/bson_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"go.mongodb.org/mongo-driver/bson/bsonoptions"
"go.mongodb.org/mongo-driver/internal/assert"
"go.mongodb.org/mongo-driver/internal/require"
"go.mongodb.org/mongo-driver/x/bsonx/bsoncore"
Expand Down Expand Up @@ -349,23 +348,21 @@ func TestMapCodec(t *testing.T) {
strstr := stringerString("foo")
mapObj := map[stringerString]int{strstr: 1}
testCases := []struct {
name string
opts *bsonoptions.MapCodecOptions
key string
name string
codec *mapCodec
key string
}{
{"default", bsonoptions.MapCodec(), "foo"},
{"true", bsonoptions.MapCodec().SetEncodeKeysWithStringer(true), "bar"},
{"false", bsonoptions.MapCodec().SetEncodeKeysWithStringer(false), "foo"},
{"default", &mapCodec{}, "foo"},
{"true", &mapCodec{encodeKeysWithStringer: true}, "bar"},
{"false", &mapCodec{encodeKeysWithStringer: false}, "foo"},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
mapCodec := NewMapCodec(tc.opts)
mapRegistry := NewRegistry()
mapRegistry.RegisterKindEncoder(reflect.Map, mapCodec)
mapRegistry := NewRegistryBuilder()
mapRegistry.RegisterKindEncoder(reflect.Map, func(*Registry) ValueEncoder { return tc.codec })
buf := new(bytes.Buffer)
vw := NewValueWriter(buf)
enc := NewEncoder(vw)
enc.SetRegistry(mapRegistry)
enc := NewEncoderWithRegistry(mapRegistry.Build(), vw)
err := enc.Encode(mapObj)
assert.Nil(t, err, "Encode error: %v", err)
str := buf.String()
Expand Down
241 changes: 27 additions & 214 deletions bson/bsoncodec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,202 +72,33 @@ func (vde ValueDecoderError) Error() string {
return fmt.Sprintf("%s can only decode valid and settable %s, but got %s", vde.Name, strings.Join(typeKinds, ", "), received)
}

// EncodeContext is the contextual information required for a Codec to encode a
// value.
type EncodeContext struct {
*Registry

// MinSize causes the Encoder to marshal Go integer values (int, int8, int16, int32, int64,
// uint, uint8, uint16, uint32, or uint64) as the minimum BSON int size (either 32 or 64 bits)
// that can represent the integer value.
//
// Deprecated: Use bson.Encoder.IntMinSize instead.
MinSize bool

errorOnInlineDuplicates bool
stringifyMapKeysWithFmt bool
nilMapAsEmpty bool
nilSliceAsEmpty bool
nilByteSliceAsEmpty bool
omitZeroStruct bool
useJSONStructTags bool
}

// ErrorOnInlineDuplicates causes the Encoder to return an error if there is a duplicate field in
// the marshaled BSON when the "inline" struct tag option is set.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Encoder.ErrorOnInlineDuplicates] instead.
func (ec *EncodeContext) ErrorOnInlineDuplicates() {
ec.errorOnInlineDuplicates = true
}

// StringifyMapKeysWithFmt causes the Encoder to convert Go map keys to BSON document field name
// strings using fmt.Sprintf() instead of the default string conversion logic.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Encoder.StringifyMapKeysWithFmt] instead.
func (ec *EncodeContext) StringifyMapKeysWithFmt() {
ec.stringifyMapKeysWithFmt = true
}

// NilMapAsEmpty causes the Encoder to marshal nil Go maps as empty BSON documents instead of BSON
// null.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Encoder.NilMapAsEmpty] instead.
func (ec *EncodeContext) NilMapAsEmpty() {
ec.nilMapAsEmpty = true
}

// NilSliceAsEmpty causes the Encoder to marshal nil Go slices as empty BSON arrays instead of BSON
// null.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Encoder.NilSliceAsEmpty] instead.
func (ec *EncodeContext) NilSliceAsEmpty() {
ec.nilSliceAsEmpty = true
}

// NilByteSliceAsEmpty causes the Encoder to marshal nil Go byte slices as empty BSON binary values
// instead of BSON null.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Encoder.NilByteSliceAsEmpty] instead.
func (ec *EncodeContext) NilByteSliceAsEmpty() {
ec.nilByteSliceAsEmpty = true
}

// OmitZeroStruct causes the Encoder to consider the zero value for a struct (e.g. MyStruct{})
// as empty and omit it from the marshaled BSON when the "omitempty" struct tag option is set.
//
// Note that the Encoder only examines exported struct fields when determining if a struct is the
// zero value. It considers pointers to a zero struct value (e.g. &MyStruct{}) not empty.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Encoder.OmitZeroStruct] instead.
func (ec *EncodeContext) OmitZeroStruct() {
ec.omitZeroStruct = true
}

// UseJSONStructTags causes the Encoder to fall back to using the "json" struct tag if a "bson"
// struct tag is not specified.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Encoder.UseJSONStructTags] instead.
func (ec *EncodeContext) UseJSONStructTags() {
ec.useJSONStructTags = true
}

// DecodeContext is the contextual information required for a Codec to decode a
// value.
type DecodeContext struct {
*Registry

// Truncate, if true, instructs decoders to to truncate the fractional part of BSON "double"
// values when attempting to unmarshal them into a Go integer (int, int8, int16, int32, int64,
// uint, uint8, uint16, uint32, or uint64) struct field. The truncation logic does not apply to
// BSON "decimal128" values.
//
// Deprecated: Use bson.Decoder.AllowTruncatingDoubles instead.
Truncate bool

// Ancestor is the type of a containing document. This is mainly used to determine what type
// should be used when decoding an embedded document into an empty interface. For example, if
// Ancestor is a bson.M, BSON embedded document values being decoded into an empty interface
// will be decoded into a bson.M.
//
// Deprecated: Use bson.Decoder.DefaultDocumentM or bson.Decoder.DefaultDocumentD instead.
Ancestor reflect.Type

// defaultDocumentType specifies the Go type to decode top-level and nested BSON documents into. In particular, the
// usage for this field is restricted to data typed as "interface{}" or "map[string]interface{}". If DocumentType is
// set to a type that a BSON document cannot be unmarshaled into (e.g. "string"), unmarshalling will result in an
// error. DocumentType overrides the Ancestor field.
defaultDocumentType reflect.Type

binaryAsSlice bool
useJSONStructTags bool
useLocalTimeZone bool
zeroMaps bool
zeroStructs bool
}

// BinaryAsSlice causes the Decoder to unmarshal BSON binary field values that are the "Generic" or
// "Old" BSON binary subtype as a Go byte slice instead of a Binary.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Decoder.BinaryAsSlice] instead.
func (dc *DecodeContext) BinaryAsSlice() {
dc.binaryAsSlice = true
}

// UseJSONStructTags causes the Decoder to fall back to using the "json" struct tag if a "bson"
// struct tag is not specified.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Decoder.UseJSONStructTags] instead.
func (dc *DecodeContext) UseJSONStructTags() {
dc.useJSONStructTags = true
}

// UseLocalTimeZone causes the Decoder to unmarshal time.Time values in the local timezone instead
// of the UTC timezone.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Decoder.UseLocalTimeZone] instead.
func (dc *DecodeContext) UseLocalTimeZone() {
dc.useLocalTimeZone = true
}

// ZeroMaps causes the Decoder to delete any existing values from Go maps in the destination value
// passed to Decode before unmarshaling BSON documents into them.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Decoder.ZeroMaps] instead.
func (dc *DecodeContext) ZeroMaps() {
dc.zeroMaps = true
}

// ZeroStructs causes the Decoder to delete any existing values from Go structs in the destination
// value passed to Decode before unmarshaling BSON documents into them.
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Decoder.ZeroStructs] instead.
func (dc *DecodeContext) ZeroStructs() {
dc.zeroStructs = true
}

// DefaultDocumentM causes the Decoder to always unmarshal documents into the M type. This
// behavior is restricted to data typed as "interface{}" or "map[string]interface{}".
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Decoder.DefaultDocumentM] instead.
func (dc *DecodeContext) DefaultDocumentM() {
dc.defaultDocumentType = reflect.TypeOf(M{})
}

// DefaultDocumentD causes the Decoder to always unmarshal documents into the D type. This
// behavior is restricted to data typed as "interface{}" or "map[string]interface{}".
//
// Deprecated: Use [go.mongodb.org/mongo-driver/bson.Decoder.DefaultDocumentD] instead.
func (dc *DecodeContext) DefaultDocumentD() {
dc.defaultDocumentType = reflect.TypeOf(D{})
}

// ValueCodec is an interface for encoding and decoding a reflect.Value.
// values.
//
// Deprecated: Use [ValueEncoder] and [ValueDecoder] instead.
type ValueCodec interface {
ValueEncoder
ValueDecoder
// EncoderRegistry is an interface provides a ValueEncoder based on the given reflect.Type.
type EncoderRegistry interface {
LookupEncoder(reflect.Type) (ValueEncoder, error)
}

// ValueEncoder is the interface implemented by types that can encode a provided Go type to BSON.
// The value to encode is provided as a reflect.Value and a bson.ValueWriter is used within the
// EncodeValue method to actually create the BSON representation. For convenience, ValueEncoderFunc
// is provided to allow use of a function with the correct signature as a ValueEncoder. An
// EncodeContext instance is provided to allow implementations to lookup further ValueEncoders and
// to provide configuration information.
// is provided to allow use of a function with the correct signature as a ValueEncoder. A pointer
// to a Registry instance is provided to allow implementations to lookup further ValueEncoders.
type ValueEncoder interface {
EncodeValue(EncodeContext, ValueWriter, reflect.Value) error
EncodeValue(EncoderRegistry, ValueWriter, reflect.Value) error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to remove EncodeContext from the ValueEncoder API, only from the bson.Encoder API. Keeping EncodeContext in the EncodeValue function may significantly simplify this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current EncodeContext/DecodeContext is coupled with the fields in Encoder/Decoder and codecs. For example, OverwriteDuplicatedInlinedFields exists in StructCodec, EncodeContext, and Encoder. Moreover, the example here, OverwriteDuplicatedInlinedFields, is default true. Considering future non-zero default codec fields as well as the current use cases of codec configurations for mgocompat, using an empty struct, such as type structCodec struct {}, and passing EncodeContext/DecodeContext to modify the encoding/decoding will be similarly complex. Meanwhile, the *Context becomes a superset of all codec attributes since it has to be passed into different codecs. Also for custom codecs, *Context is either hard to be extended or useless.

Though SetBehavior and Registry.SetCodecOption are out of the scope of GODRIVER-2714, they also decouple the logic with individual codecs by distributing the configuration response to each codec, yet providing a single entry from outside.

However, I agree that SetBehavior and Registry.SetCodecOption are redundant. We can make Registry.SetCodecOption accessible to the bson package only.

A less radical PR keeping both *Context and codec fields is available at #1659.

}

// ValueEncoderFunc is an adapter function that allows a function with the correct signature to be
// used as a ValueEncoder.
type ValueEncoderFunc func(EncodeContext, ValueWriter, reflect.Value) error
type ValueEncoderFunc func(EncoderRegistry, ValueWriter, reflect.Value) error

// EncodeValue implements the ValueEncoder interface.
func (fn ValueEncoderFunc) EncodeValue(ec EncodeContext, vw ValueWriter, val reflect.Value) error {
return fn(ec, vw, val)
func (fn ValueEncoderFunc) EncodeValue(reg EncoderRegistry, vw ValueWriter, val reflect.Value) error {
return fn(reg, vw, val)
}

// DecoderRegistry is an interface provides a ValueDecoder based on the given reflect.Type.
type DecoderRegistry interface {
LookupDecoder(reflect.Type) (ValueDecoder, error)
LookupTypeMapEntry(Type) (reflect.Type, error)
}

// ValueDecoder is the interface implemented by types that can decode BSON to a provided Go type.
Expand All @@ -276,28 +107,28 @@ func (fn ValueEncoderFunc) EncodeValue(ec EncodeContext, vw ValueWriter, val ref
// ValueDecoder. A DecodeContext instance is provided and serves similar functionality to the
// EncodeContext.
type ValueDecoder interface {
DecodeValue(DecodeContext, ValueReader, reflect.Value) error
DecodeValue(DecoderRegistry, ValueReader, reflect.Value) error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to remove DecodeContext from the ValueDecoder API, only from the bson.Decoder API. Keeping DecodeContext in the DecodeValue function may significantly simplify this PR.

}

// ValueDecoderFunc is an adapter function that allows a function with the correct signature to be
// used as a ValueDecoder.
type ValueDecoderFunc func(DecodeContext, ValueReader, reflect.Value) error
type ValueDecoderFunc func(DecoderRegistry, ValueReader, reflect.Value) error

// DecodeValue implements the ValueDecoder interface.
func (fn ValueDecoderFunc) DecodeValue(dc DecodeContext, vr ValueReader, val reflect.Value) error {
return fn(dc, vr, val)
func (fn ValueDecoderFunc) DecodeValue(reg DecoderRegistry, vr ValueReader, val reflect.Value) error {
return fn(reg, vr, val)
}

// typeDecoder is the interface implemented by types that can handle the decoding of a value given its type.
type typeDecoder interface {
decodeType(DecodeContext, ValueReader, reflect.Type) (reflect.Value, error)
decodeType(DecoderRegistry, ValueReader, reflect.Type) (reflect.Value, error)
}

// typeDecoderFunc is an adapter function that allows a function with the correct signature to be used as a typeDecoder.
type typeDecoderFunc func(DecodeContext, ValueReader, reflect.Type) (reflect.Value, error)
type typeDecoderFunc func(DecoderRegistry, ValueReader, reflect.Type) (reflect.Value, error)

func (fn typeDecoderFunc) decodeType(dc DecodeContext, vr ValueReader, t reflect.Type) (reflect.Value, error) {
return fn(dc, vr, t)
func (fn typeDecoderFunc) decodeType(reg DecoderRegistry, vr ValueReader, t reflect.Type) (reflect.Value, error) {
return fn(reg, vr, t)
}

// decodeAdapter allows two functions with the correct signatures to be used as both a ValueDecoder and typeDecoder.
Expand All @@ -309,31 +140,13 @@ type decodeAdapter struct {
var _ ValueDecoder = decodeAdapter{}
var _ typeDecoder = decodeAdapter{}

// decodeTypeOrValue calls decoder.decodeType is decoder is a typeDecoder. Otherwise, it allocates a new element of type
// t and calls decoder.DecodeValue on it.
func decodeTypeOrValue(decoder ValueDecoder, dc DecodeContext, vr ValueReader, t reflect.Type) (reflect.Value, error) {
td, _ := decoder.(typeDecoder)
return decodeTypeOrValueWithInfo(decoder, td, dc, vr, t, true)
}

func decodeTypeOrValueWithInfo(vd ValueDecoder, td typeDecoder, dc DecodeContext, vr ValueReader, t reflect.Type, convert bool) (reflect.Value, error) {
if td != nil {
val, err := td.decodeType(dc, vr, t)
if err == nil && convert && val.Type() != t {
// This conversion step is necessary for slices and maps. If a user declares variables like:
//
// type myBool bool
// var m map[string]myBool
//
// and tries to decode BSON bytes into the map, the decoding will fail if this conversion is not present
// because we'll try to assign a value of type bool to one of type myBool.
val = val.Convert(t)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ancestor type used by the empty interface codec is passed as the last argument of the decodeType, so the caller will be responsible for the conversion.

}
return val, err
func decodeTypeOrValueWithInfo(vd ValueDecoder, reg DecoderRegistry, vr ValueReader, t reflect.Type) (reflect.Value, error) {
if td, _ := vd.(typeDecoder); td != nil {
return td.decodeType(reg, vr, t)
}

val := reflect.New(t).Elem()
err := vd.DecodeValue(dc, vr, val)
err := vd.DecodeValue(reg, vr, val)
return val, err
}

Expand Down
Loading
Loading