From a5b9b2cab066bad1433197003d927c31771d4b0c Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 28 Jun 2024 11:35:23 -0400 Subject: [PATCH 1/4] GODRIVER-2796 Update `ObjectIDAsHexString` behavior. --- bson/bsoncodec.go | 145 ++-------------------------- bson/decoder.go | 52 +++------- bson/decoder_test.go | 19 ++++ bson/default_value_decoders.go | 4 +- bson/default_value_decoders_test.go | 8 +- bson/default_value_encoders.go | 2 +- bson/default_value_encoders_test.go | 36 +++---- bson/encoder.go | 52 ++-------- bson/objectid.go | 8 -- bson/objectid_test.go | 21 ---- bson/string_codec.go | 9 +- bson/string_codec_test.go | 13 ++- bson/struct_codec.go | 5 +- bson/truncation_test.go | 4 +- bson/uint_codec.go | 4 +- 15 files changed, 93 insertions(+), 289 deletions(-) diff --git a/bson/bsoncodec.go b/bson/bsoncodec.go index ad1d4a8ded..9bce8f7d35 100644 --- a/bson/bsoncodec.go +++ b/bson/bsoncodec.go @@ -77,12 +77,10 @@ func (vde ValueDecoderError) Error() string { type EncodeContext struct { *Registry - // MinSize causes the Encoder to marshal Go integer values (int, int8, int16, int32, int64, + // 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 + minSize bool errorOnInlineDuplicates bool stringifyMapKeysWithFmt bool @@ -93,65 +91,6 @@ type EncodeContext struct { 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 { @@ -161,9 +100,7 @@ type DecodeContext struct { // 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 + 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 @@ -179,76 +116,12 @@ type DecodeContext struct { // 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 + binaryAsSlice bool + objectIDAsHexString bool + useJSONStructTags bool + useLocalTimeZone bool + zeroMaps bool + zeroStructs bool } // ValueEncoder is the interface implemented by types that can encode a provided Go type to BSON. diff --git a/bson/decoder.go b/bson/decoder.go index 482927ef3d..653b73e445 100644 --- a/bson/decoder.go +++ b/bson/decoder.go @@ -30,15 +30,6 @@ var decPool = sync.Pool{ type Decoder struct { dc DecodeContext vr ValueReader - - defaultDocumentM bool - defaultDocumentD bool - - binaryAsSlice bool - useJSONStructTags bool - useLocalTimeZone bool - zeroMaps bool - zeroStructs bool } // NewDecoder returns a new decoder that uses the DefaultRegistry to read from vr. @@ -82,28 +73,6 @@ func (d *Decoder) Decode(val interface{}) error { return err } - if d.defaultDocumentM { - d.dc.DefaultDocumentM() - } - if d.defaultDocumentD { - d.dc.DefaultDocumentD() - } - if d.binaryAsSlice { - d.dc.BinaryAsSlice() - } - if d.useJSONStructTags { - d.dc.UseJSONStructTags() - } - if d.useLocalTimeZone { - d.dc.UseLocalTimeZone() - } - if d.zeroMaps { - d.dc.ZeroMaps() - } - if d.zeroStructs { - d.dc.ZeroStructs() - } - return decoder.DecodeValue(d.dc, d.vr, rval) } @@ -121,48 +90,53 @@ func (d *Decoder) SetRegistry(r *Registry) { // DefaultDocumentM causes the Decoder to always unmarshal documents into the primitive.M type. This // behavior is restricted to data typed as "interface{}" or "map[string]interface{}". func (d *Decoder) DefaultDocumentM() { - d.defaultDocumentM = true + d.dc.defaultDocumentType = reflect.TypeOf(M{}) } // DefaultDocumentD causes the Decoder to always unmarshal documents into the primitive.D type. This // behavior is restricted to data typed as "interface{}" or "map[string]interface{}". func (d *Decoder) DefaultDocumentD() { - d.defaultDocumentD = true + d.dc.defaultDocumentType = reflect.TypeOf(D{}) } // AllowTruncatingDoubles causes the Decoder to truncate the fractional part of BSON "double" values // when attempting to unmarshal them into a Go integer (int, int8, int16, int32, or int64) struct // field. The truncation logic does not apply to BSON "decimal128" values. func (d *Decoder) AllowTruncatingDoubles() { - d.dc.Truncate = true + d.dc.truncate = true } // 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 primitive.Binary. func (d *Decoder) BinaryAsSlice() { - d.binaryAsSlice = true + d.dc.binaryAsSlice = true +} + +// ObjectIDAsHexString causes the Decoder to decode object IDs to their hex representation. +func (d *Decoder) ObjectIDAsHexString() { + d.dc.objectIDAsHexString = true } // UseJSONStructTags causes the Decoder to fall back to using the "json" struct tag if a "bson" // struct tag is not specified. func (d *Decoder) UseJSONStructTags() { - d.useJSONStructTags = true + d.dc.useJSONStructTags = true } // UseLocalTimeZone causes the Decoder to unmarshal time.Time values in the local timezone instead // of the UTC timezone. func (d *Decoder) UseLocalTimeZone() { - d.useLocalTimeZone = true + d.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. func (d *Decoder) ZeroMaps() { - d.zeroMaps = true + d.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. func (d *Decoder) ZeroStructs() { - d.zeroStructs = true + d.dc.zeroStructs = true } diff --git a/bson/decoder_test.go b/bson/decoder_test.go index dbef3e7fb0..93afc91557 100644 --- a/bson/decoder_test.go +++ b/bson/decoder_test.go @@ -408,6 +408,10 @@ func TestDecoderConfiguration(t *testing.T) { MyUint64 uint64 } + type objectIDTest struct { + ID string + } + type jsonStructTest struct { StructFieldName string `json:"jsonFieldName"` } @@ -512,6 +516,21 @@ func TestDecoderConfiguration(t *testing.T) { {Key: "myDocument", Value: M{"myString": "test value"}}, }, }, + // Test that ObjectIDAsHexString causes the Decoder to decode object ID to hex. + { + description: "ObjectIDAsHexString", + configure: func(dec *Decoder) { + dec.ObjectIDAsHexString() + }, + input: bsoncore.NewDocumentBuilder(). + AppendObjectID("id", func() ObjectID { + id, _ := ObjectIDFromHex("5ef7fdd91c19e3222b41b839") + return id + }()). + Build(), + decodeInto: func() interface{} { return &objectIDTest{} }, + want: &objectIDTest{ID: "5ef7fdd91c19e3222b41b839"}, + }, // Test that UseJSONStructTags causes the Decoder to fall back to "json" struct tags if // "bson" struct tags are not available. { diff --git a/bson/default_value_decoders.go b/bson/default_value_decoders.go index 2075c0ad3d..63f52fd52d 100644 --- a/bson/default_value_decoders.go +++ b/bson/default_value_decoders.go @@ -243,7 +243,7 @@ func intDecodeType(dc DecodeContext, vr ValueReader, t reflect.Type) (reflect.Va if err != nil { return emptyValue, err } - if !dc.Truncate && math.Floor(f64) != f64 { + if !dc.truncate && math.Floor(f64) != f64 { return emptyValue, errCannotTruncate } if f64 > float64(math.MaxInt64) { @@ -368,7 +368,7 @@ func floatDecodeType(dc DecodeContext, vr ValueReader, t reflect.Type) (reflect. switch t.Kind() { case reflect.Float32: - if !dc.Truncate && float64(float32(f)) != f { + if !dc.truncate && float64(float32(f)) != f { return emptyValue, errCannotTruncate } diff --git a/bson/default_value_decoders_test.go b/bson/default_value_decoders_test.go index 9d95af257e..3c40170462 100644 --- a/bson/default_value_decoders_test.go +++ b/bson/default_value_decoders_test.go @@ -187,7 +187,7 @@ func TestDefaultValueDecoders(t *testing.T) { nil, }, { - "ReadDouble (truncate)", int64(3), &DecodeContext{Truncate: true}, + "ReadDouble (truncate)", int64(3), &DecodeContext{truncate: true}, &valueReaderWriter{BSONType: TypeDouble, Return: float64(3.14)}, readDouble, nil, }, @@ -419,7 +419,7 @@ func TestDefaultValueDecoders(t *testing.T) { nil, }, { - "ReadDouble (truncate)", uint64(3), &DecodeContext{Truncate: true}, + "ReadDouble (truncate)", uint64(3), &DecodeContext{truncate: true}, &valueReaderWriter{BSONType: TypeDouble, Return: float64(3.14)}, readDouble, nil, }, @@ -670,7 +670,7 @@ func TestDefaultValueDecoders(t *testing.T) { nil, }, { - "float32/fast path (truncate)", float32(3.14), &DecodeContext{Truncate: true}, + "float32/fast path (truncate)", float32(3.14), &DecodeContext{truncate: true}, &valueReaderWriter{BSONType: TypeDouble, Return: float64(3.14)}, readDouble, nil, }, @@ -708,7 +708,7 @@ func TestDefaultValueDecoders(t *testing.T) { nil, }, { - "float32/reflection path (truncate)", myfloat32(3.14), &DecodeContext{Truncate: true}, + "float32/reflection path (truncate)", myfloat32(3.14), &DecodeContext{truncate: true}, &valueReaderWriter{BSONType: TypeDouble, Return: float64(3.14)}, readDouble, nil, }, diff --git a/bson/default_value_encoders.go b/bson/default_value_encoders.go index 2e8ae5830c..0c4586ba79 100644 --- a/bson/default_value_encoders.go +++ b/bson/default_value_encoders.go @@ -125,7 +125,7 @@ func intEncodeValue(ec EncodeContext, vw ValueWriter, val reflect.Value) error { return vw.WriteInt64(i64) case reflect.Int64: i64 := val.Int() - if ec.MinSize && fitsIn32Bits(i64) { + if ec.minSize && fitsIn32Bits(i64) { return vw.WriteInt32(int32(i64)) } return vw.WriteInt64(i64) diff --git a/bson/default_value_encoders_test.go b/bson/default_value_encoders_test.go index ba7b70de02..62cdd20da5 100644 --- a/bson/default_value_encoders_test.go +++ b/bson/default_value_encoders_test.go @@ -113,9 +113,9 @@ func TestDefaultValueEncoders(t *testing.T) { {"int16/fast path", int16(32767), nil, nil, writeInt32, nil}, {"int32/fast path", int32(2147483647), nil, nil, writeInt32, nil}, {"int64/fast path", int64(1234567890987), nil, nil, writeInt64, nil}, - {"int64/fast path - minsize", int64(math.MaxInt32), &EncodeContext{MinSize: true}, nil, writeInt32, nil}, - {"int64/fast path - minsize too large", int64(math.MaxInt32 + 1), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, - {"int64/fast path - minsize too small", int64(math.MinInt32 - 1), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, + {"int64/fast path - minsize", int64(math.MaxInt32), &EncodeContext{minSize: true}, nil, writeInt32, nil}, + {"int64/fast path - minsize too large", int64(math.MaxInt32 + 1), &EncodeContext{minSize: true}, nil, writeInt64, nil}, + {"int64/fast path - minsize too small", int64(math.MinInt32 - 1), &EncodeContext{minSize: true}, nil, writeInt64, nil}, {"int/fast path - positive int32", int(math.MaxInt32 - 1), nil, nil, writeInt32, nil}, {"int/fast path - negative int32", int(math.MinInt32 + 1), nil, nil, writeInt32, nil}, {"int/fast path - MaxInt32", int(math.MaxInt32), nil, nil, writeInt32, nil}, @@ -124,9 +124,9 @@ func TestDefaultValueEncoders(t *testing.T) { {"int16/reflection path", myint16(32767), nil, nil, writeInt32, nil}, {"int32/reflection path", myint32(2147483647), nil, nil, writeInt32, nil}, {"int64/reflection path", myint64(1234567890987), nil, nil, writeInt64, nil}, - {"int64/reflection path - minsize", myint64(math.MaxInt32), &EncodeContext{MinSize: true}, nil, writeInt32, nil}, - {"int64/reflection path - minsize too large", myint64(math.MaxInt32 + 1), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, - {"int64/reflection path - minsize too small", myint64(math.MinInt32 - 1), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, + {"int64/reflection path - minsize", myint64(math.MaxInt32), &EncodeContext{minSize: true}, nil, writeInt32, nil}, + {"int64/reflection path - minsize too large", myint64(math.MaxInt32 + 1), &EncodeContext{minSize: true}, nil, writeInt64, nil}, + {"int64/reflection path - minsize too small", myint64(math.MinInt32 - 1), &EncodeContext{minSize: true}, nil, writeInt64, nil}, {"int/reflection path - positive int32", myint(math.MaxInt32 - 1), nil, nil, writeInt32, nil}, {"int/reflection path - negative int32", myint(math.MinInt32 + 1), nil, nil, writeInt32, nil}, {"int/reflection path - MaxInt32", myint(math.MaxInt32), nil, nil, writeInt32, nil}, @@ -154,23 +154,23 @@ func TestDefaultValueEncoders(t *testing.T) { {"uint32/fast path", uint32(2147483647), nil, nil, writeInt64, nil}, {"uint64/fast path", uint64(1234567890987), nil, nil, writeInt64, nil}, {"uint/fast path", uint(1234567), nil, nil, writeInt64, nil}, - {"uint32/fast path - minsize", uint32(2147483647), &EncodeContext{MinSize: true}, nil, writeInt32, nil}, - {"uint64/fast path - minsize", uint64(2147483647), &EncodeContext{MinSize: true}, nil, writeInt32, nil}, - {"uint/fast path - minsize", uint(2147483647), &EncodeContext{MinSize: true}, nil, writeInt32, nil}, - {"uint32/fast path - minsize too large", uint32(2147483648), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, - {"uint64/fast path - minsize too large", uint64(2147483648), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, - {"uint/fast path - minsize too large", uint(2147483648), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, + {"uint32/fast path - minsize", uint32(2147483647), &EncodeContext{minSize: true}, nil, writeInt32, nil}, + {"uint64/fast path - minsize", uint64(2147483647), &EncodeContext{minSize: true}, nil, writeInt32, nil}, + {"uint/fast path - minsize", uint(2147483647), &EncodeContext{minSize: true}, nil, writeInt32, nil}, + {"uint32/fast path - minsize too large", uint32(2147483648), &EncodeContext{minSize: true}, nil, writeInt64, nil}, + {"uint64/fast path - minsize too large", uint64(2147483648), &EncodeContext{minSize: true}, nil, writeInt64, nil}, + {"uint/fast path - minsize too large", uint(2147483648), &EncodeContext{minSize: true}, nil, writeInt64, nil}, {"uint64/fast path - overflow", uint64(1 << 63), nil, nil, nothing, fmt.Errorf("%d overflows int64", uint64(1<<63))}, {"uint8/reflection path", myuint8(127), nil, nil, writeInt32, nil}, {"uint16/reflection path", myuint16(32767), nil, nil, writeInt32, nil}, {"uint32/reflection path", myuint32(2147483647), nil, nil, writeInt64, nil}, {"uint64/reflection path", myuint64(1234567890987), nil, nil, writeInt64, nil}, - {"uint32/reflection path - minsize", myuint32(2147483647), &EncodeContext{MinSize: true}, nil, writeInt32, nil}, - {"uint64/reflection path - minsize", myuint64(2147483647), &EncodeContext{MinSize: true}, nil, writeInt32, nil}, - {"uint/reflection path - minsize", myuint(2147483647), &EncodeContext{MinSize: true}, nil, writeInt32, nil}, - {"uint32/reflection path - minsize too large", myuint(1 << 31), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, - {"uint64/reflection path - minsize too large", myuint64(1 << 31), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, - {"uint/reflection path - minsize too large", myuint(2147483648), &EncodeContext{MinSize: true}, nil, writeInt64, nil}, + {"uint32/reflection path - minsize", myuint32(2147483647), &EncodeContext{minSize: true}, nil, writeInt32, nil}, + {"uint64/reflection path - minsize", myuint64(2147483647), &EncodeContext{minSize: true}, nil, writeInt32, nil}, + {"uint/reflection path - minsize", myuint(2147483647), &EncodeContext{minSize: true}, nil, writeInt32, nil}, + {"uint32/reflection path - minsize too large", myuint(1 << 31), &EncodeContext{minSize: true}, nil, writeInt64, nil}, + {"uint64/reflection path - minsize too large", myuint64(1 << 31), &EncodeContext{minSize: true}, nil, writeInt64, nil}, + {"uint/reflection path - minsize too large", myuint(2147483648), &EncodeContext{minSize: true}, nil, writeInt64, nil}, {"uint64/reflection path - overflow", myuint64(1 << 63), nil, nil, nothing, fmt.Errorf("%d overflows int64", uint64(1<<63))}, }, }, diff --git a/bson/encoder.go b/bson/encoder.go index fb865cd285..1b348f9488 100644 --- a/bson/encoder.go +++ b/bson/encoder.go @@ -25,15 +25,6 @@ var encPool = sync.Pool{ type Encoder struct { ec EncodeContext vw ValueWriter - - errorOnInlineDuplicates bool - intMinSize bool - stringifyMapKeysWithFmt bool - nilMapAsEmpty bool - nilSliceAsEmpty bool - nilByteSliceAsEmpty bool - omitZeroStruct bool - useJSONStructTags bool } // NewEncoder returns a new encoder that uses the DefaultRegistry to write to vw. @@ -62,33 +53,6 @@ func (e *Encoder) Encode(val interface{}) error { return err } - // Copy the configurations applied to the Encoder over to the EncodeContext, which actually - // communicates those configurations to the default ValueEncoders. - if e.errorOnInlineDuplicates { - e.ec.ErrorOnInlineDuplicates() - } - if e.intMinSize { - e.ec.MinSize = true - } - if e.stringifyMapKeysWithFmt { - e.ec.StringifyMapKeysWithFmt() - } - if e.nilMapAsEmpty { - e.ec.NilMapAsEmpty() - } - if e.nilSliceAsEmpty { - e.ec.NilSliceAsEmpty() - } - if e.nilByteSliceAsEmpty { - e.ec.NilByteSliceAsEmpty() - } - if e.omitZeroStruct { - e.ec.OmitZeroStruct() - } - if e.useJSONStructTags { - e.ec.UseJSONStructTags() - } - return encoder.EncodeValue(e.ec, e.vw, reflect.ValueOf(val)) } @@ -106,38 +70,38 @@ func (e *Encoder) SetRegistry(r *Registry) { // 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. func (e *Encoder) ErrorOnInlineDuplicates() { - e.errorOnInlineDuplicates = true + e.ec.errorOnInlineDuplicates = true } // IntMinSize 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. func (e *Encoder) IntMinSize() { - e.intMinSize = true + e.ec.minSize = true } // StringifyMapKeysWithFmt causes the Encoder to convert Go map keys to BSON document field name // strings using fmt.Sprint instead of the default string conversion logic. func (e *Encoder) StringifyMapKeysWithFmt() { - e.stringifyMapKeysWithFmt = true + e.ec.stringifyMapKeysWithFmt = true } // NilMapAsEmpty causes the Encoder to marshal nil Go maps as empty BSON documents instead of BSON // null. func (e *Encoder) NilMapAsEmpty() { - e.nilMapAsEmpty = true + e.ec.nilMapAsEmpty = true } // NilSliceAsEmpty causes the Encoder to marshal nil Go slices as empty BSON arrays instead of BSON // null. func (e *Encoder) NilSliceAsEmpty() { - e.nilSliceAsEmpty = true + e.ec.nilSliceAsEmpty = true } // NilByteSliceAsEmpty causes the Encoder to marshal nil Go byte slices as empty BSON binary values // instead of BSON null. func (e *Encoder) NilByteSliceAsEmpty() { - e.nilByteSliceAsEmpty = true + e.ec.nilByteSliceAsEmpty = true } // TODO(GODRIVER-2820): Update the description to remove the note about only examining exported @@ -149,11 +113,11 @@ func (e *Encoder) NilByteSliceAsEmpty() { // 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. func (e *Encoder) OmitZeroStruct() { - e.omitZeroStruct = true + e.ec.omitZeroStruct = true } // UseJSONStructTags causes the Encoder to fall back to using the "json" struct tag if a "bson" // struct tag is not specified. func (e *Encoder) UseJSONStructTags() { - e.useJSONStructTags = true + e.ec.useJSONStructTags = true } diff --git a/bson/objectid.go b/bson/objectid.go index ccb0b78338..024281eb7f 100644 --- a/bson/objectid.go +++ b/bson/objectid.go @@ -91,14 +91,6 @@ func ObjectIDFromHex(s string) (ObjectID, error) { return oid, nil } -// IsValidObjectID returns true if the provided hex string represents a valid ObjectID and false if not. -// -// Deprecated: Use ObjectIDFromHex and check the error instead. -func IsValidObjectID(s string) bool { - _, err := ObjectIDFromHex(s) - return err == nil -} - // MarshalText returns the ObjectID as UTF-8-encoded text. Implementing this allows us to use ObjectID // as a map key when marshalling JSON. See https://pkg.go.dev/encoding#TextMarshaler func (id ObjectID) MarshalText() ([]byte, error) { diff --git a/bson/objectid_test.go b/bson/objectid_test.go index 12d4214241..f7291c5a2c 100644 --- a/bson/objectid_test.go +++ b/bson/objectid_test.go @@ -108,27 +108,6 @@ func TestFromHex_WrongLength(t *testing.T) { require.Equal(t, ErrInvalidHex, err) } -func TestIsValidObjectID(t *testing.T) { - testCases := []struct { - givenID string - expected bool - }{ - { - givenID: "5ef7fdd91c19e3222b41b839", - expected: true, - }, - { - givenID: "5ef7fdd91c19e3222b41b83", - expected: false, - }, - } - - for _, testcase := range testCases { - got := IsValidObjectID(testcase.givenID) - assert.Equal(t, testcase.expected, got, "expected hex string to be valid ObjectID: %v, got %v", testcase.expected, got) - } -} - func TestTimeStamp(t *testing.T) { testCases := []struct { Hex string diff --git a/bson/string_codec.go b/bson/string_codec.go index fcda72af90..3f370d6005 100644 --- a/bson/string_codec.go +++ b/bson/string_codec.go @@ -14,7 +14,6 @@ import ( // stringCodec is the Codec used for string values. type stringCodec struct { // DecodeObjectIDAsHex specifies if object IDs should be decoded as their hex representation. - // If false, a string made from the raw object ID bytes will be used. Defaults to true. decodeObjectIDAsHex bool } @@ -36,7 +35,7 @@ func (sc *stringCodec) EncodeValue(_ EncodeContext, vw ValueWriter, val reflect. return vw.WriteString(val.String()) } -func (sc *stringCodec) decodeType(_ DecodeContext, vr ValueReader, t reflect.Type) (reflect.Value, error) { +func (sc *stringCodec) decodeType(dc DecodeContext, vr ValueReader, t reflect.Type) (reflect.Value, error) { if t.Kind() != reflect.String { return emptyValue, ValueDecoderError{ Name: "StringDecodeValue", @@ -58,12 +57,10 @@ func (sc *stringCodec) decodeType(_ DecodeContext, vr ValueReader, t reflect.Typ if err != nil { return emptyValue, err } - if sc.decodeObjectIDAsHex { + if sc.decodeObjectIDAsHex || dc.objectIDAsHexString { str = oid.Hex() } else { - // TODO(GODRIVER-2796): Return an error here instead of decoding to a garbled string. - byteArray := [12]byte(oid) - str = string(byteArray[:]) + return emptyValue, fmt.Errorf("decoding an object ID to a hexadecimal string is disabled by default") } case TypeSymbol: str, err = vr.ReadSymbol() diff --git a/bson/string_codec_test.go b/bson/string_codec_test.go index d44c042653..057cdd270e 100644 --- a/bson/string_codec_test.go +++ b/bson/string_codec_test.go @@ -7,6 +7,7 @@ package bson import ( + "errors" "reflect" "testing" @@ -16,21 +17,25 @@ import ( func TestStringCodec(t *testing.T) { t.Run("ObjectIDAsHex", func(t *testing.T) { oid := NewObjectID() - byteArray := [12]byte(oid) reader := &valueReaderWriter{BSONType: TypeObjectID, Return: oid} testCases := []struct { name string stringCodec *stringCodec result string + err error }{ - {"true", &stringCodec{decodeObjectIDAsHex: true}, oid.Hex()}, - {"false", &stringCodec{decodeObjectIDAsHex: false}, string(byteArray[:])}, + {"true", &stringCodec{decodeObjectIDAsHex: true}, oid.Hex(), nil}, + {"false", &stringCodec{decodeObjectIDAsHex: false}, "", errors.New("decoding an ObjectID to a hexadecimal string is disabled by default")}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { actual := reflect.New(reflect.TypeOf("")).Elem() err := tc.stringCodec.DecodeValue(DecodeContext{}, reader, actual) - assert.Nil(t, err, "StringCodec.DecodeValue error: %v", err) + if tc.err == nil { + assert.NoErrorf(t, err, "StringCodec.DecodeValue error: %q", err) + } else { + assert.EqualErrorf(t, err, tc.err.Error(), "Expected error %q, got %q", tc.err, err) + } actualString := actual.Interface().(string) assert.Equal(t, tc.result, actualString, "Expected string %v, got %v", tc.result, actualString) diff --git a/bson/struct_codec.go b/bson/struct_codec.go index 3825cf8469..8249a68616 100644 --- a/bson/struct_codec.go +++ b/bson/struct_codec.go @@ -166,7 +166,7 @@ func (sc *structCodec) EncodeValue(ec EncodeContext, vw ValueWriter, val reflect ectx := EncodeContext{ Registry: ec.Registry, - MinSize: desc.minSize || ec.MinSize, + minSize: desc.minSize || ec.minSize, errorOnInlineDuplicates: ec.errorOnInlineDuplicates, stringifyMapKeysWithFmt: ec.stringifyMapKeysWithFmt, nilMapAsEmpty: ec.nilMapAsEmpty, @@ -341,9 +341,10 @@ func (sc *structCodec) DecodeValue(dc DecodeContext, vr ValueReader, val reflect dctx := DecodeContext{ Registry: dc.Registry, - Truncate: fd.truncate || dc.Truncate, + truncate: fd.truncate || dc.truncate, defaultDocumentType: dc.defaultDocumentType, binaryAsSlice: dc.binaryAsSlice, + objectIDAsHexString: dc.objectIDAsHexString, useJSONStructTags: dc.useJSONStructTags, useLocalTimeZone: dc.useLocalTimeZone, zeroMaps: dc.zeroMaps, diff --git a/bson/truncation_test.go b/bson/truncation_test.go index 865917cfe4..a9aeea278b 100644 --- a/bson/truncation_test.go +++ b/bson/truncation_test.go @@ -41,7 +41,7 @@ func TestTruncation(t *testing.T) { var output outputArgs dc := DecodeContext{ Registry: DefaultRegistry, - Truncate: true, + truncate: true, } err = UnmarshalWithContext(dc, buf.Bytes(), &output) @@ -67,7 +67,7 @@ func TestTruncation(t *testing.T) { var output outputArgs dc := DecodeContext{ Registry: DefaultRegistry, - Truncate: false, + truncate: false, } // case throws an error when truncation is disabled diff --git a/bson/uint_codec.go b/bson/uint_codec.go index 3bd752ad02..29b59f526b 100644 --- a/bson/uint_codec.go +++ b/bson/uint_codec.go @@ -32,7 +32,7 @@ func (uic *uintCodec) EncodeValue(ec EncodeContext, vw ValueWriter, val reflect. u64 := val.Uint() // If ec.MinSize or if encodeToMinSize is true for a non-uint64 value we should write val as an int32 - useMinSize := ec.MinSize || (uic.encodeToMinSize && val.Kind() != reflect.Uint64) + useMinSize := ec.minSize || (uic.encodeToMinSize && val.Kind() != reflect.Uint64) if u64 <= math.MaxInt32 && useMinSize { return vw.WriteInt32(int32(u64)) @@ -70,7 +70,7 @@ func (uic *uintCodec) decodeType(dc DecodeContext, vr ValueReader, t reflect.Typ if err != nil { return emptyValue, err } - if !dc.Truncate && math.Floor(f64) != f64 { + if !dc.truncate && math.Floor(f64) != f64 { return emptyValue, errCannotTruncate } if f64 > float64(math.MaxInt64) { From 220dd1a189ea1048df05a08e3064c73418f06168 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 28 Jun 2024 14:12:43 -0400 Subject: [PATCH 2/4] Fix mgo compatibility. --- bson/mgoregistry.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/bson/mgoregistry.go b/bson/mgoregistry.go index 15c67c43dc..c9b39556e3 100644 --- a/bson/mgoregistry.go +++ b/bson/mgoregistry.go @@ -39,6 +39,7 @@ func NewMgoRegistry() *Registry { reg := NewRegistry() reg.RegisterTypeDecoder(tEmpty, &emptyInterfaceCodec{decodeBinaryAsSlice: true}) + reg.RegisterKindDecoder(reflect.String, ValueDecoderFunc(mgoStringDecodeValue)) reg.RegisterKindDecoder(reflect.Struct, structCodec) reg.RegisterKindDecoder(reflect.Map, mapCodec) reg.RegisterTypeEncoder(tByteSlice, &byteSliceCodec{encodeNilAsEmpty: true}) @@ -75,6 +76,30 @@ func NewRespectNilValuesMgoRegistry() *Registry { return reg } +func mgoStringDecodeValue(dc DecodeContext, vr ValueReader, val reflect.Value) error { + if val.Kind() != reflect.String { + return ValueDecoderError{ + Name: "StringDecodeValue", + Kinds: []reflect.Kind{reflect.String}, + Received: reflect.Zero(val.Type()), + } + } + + if vr.Type() == TypeObjectID { + oid, err := vr.ReadObjectID() + if err != nil { + return err + } + if dc.objectIDAsHexString { + val.SetString(oid.Hex()) + } else { + val.SetString(string(oid[:])) + } + return nil + } + return (&stringCodec{}).DecodeValue(dc, vr, val) +} + // setter interface: a value implementing the bson.Setter interface will receive the BSON // value via the SetBSON method during unmarshaling, and the object // itself will not be changed as usual. From 60f3f6b0113a0aa575c1b1de673a5cd789a45b38 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 28 Jun 2024 14:37:36 -0400 Subject: [PATCH 3/4] minor fix --- bson/string_codec_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bson/string_codec_test.go b/bson/string_codec_test.go index 057cdd270e..5d9d3d4713 100644 --- a/bson/string_codec_test.go +++ b/bson/string_codec_test.go @@ -25,7 +25,7 @@ func TestStringCodec(t *testing.T) { err error }{ {"true", &stringCodec{decodeObjectIDAsHex: true}, oid.Hex(), nil}, - {"false", &stringCodec{decodeObjectIDAsHex: false}, "", errors.New("decoding an ObjectID to a hexadecimal string is disabled by default")}, + {"false", &stringCodec{decodeObjectIDAsHex: false}, "", errors.New("decoding an object ID to a hexadecimal string is disabled by default")}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { From 203b6d7d0ccd33bf2147c91ee9294fb80c722664 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 19 Jul 2024 11:41:04 -0400 Subject: [PATCH 4/4] update comments and tests --- bson/bsoncodec.go | 15 +++++++------ bson/decoder_test.go | 20 +++++++++++++++++ bson/string_codec.go | 9 +++----- bson/string_codec_test.go | 45 --------------------------------------- 4 files changed, 32 insertions(+), 57 deletions(-) delete mode 100644 bson/string_codec_test.go diff --git a/bson/bsoncodec.go b/bson/bsoncodec.go index 228a4afa8d..bacc99fbb7 100644 --- a/bson/bsoncodec.go +++ b/bson/bsoncodec.go @@ -96,7 +96,7 @@ type EncodeContext struct { type DecodeContext struct { *Registry - // Truncate, if true, instructs decoders to to truncate the fractional part of BSON "double" + // 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. @@ -108,12 +108,15 @@ type DecodeContext struct { // error. defaultDocumentType reflect.Type - binaryAsSlice bool + binaryAsSlice bool + + // a false value results in a decoding error. objectIDAsHexString bool - useJSONStructTags bool - useLocalTimeZone bool - zeroMaps bool - zeroStructs bool + + useJSONStructTags bool + useLocalTimeZone bool + zeroMaps bool + zeroStructs bool } // ValueEncoder is the interface implemented by types that can encode a provided Go type to BSON. diff --git a/bson/decoder_test.go b/bson/decoder_test.go index 52e3b6a73f..9c917e6727 100644 --- a/bson/decoder_test.go +++ b/bson/decoder_test.go @@ -607,6 +607,26 @@ func TestDecoderConfiguration(t *testing.T) { }) } + t.Run("Decoding an object ID to string", func(t *testing.T) { + t.Parallel() + + type objectIDTest struct { + ID string + } + + doc := bsoncore.NewDocumentBuilder(). + AppendObjectID("id", func() ObjectID { + id, _ := ObjectIDFromHex("5ef7fdd91c19e3222b41b839") + return id + }()). + Build() + + dec := NewDecoder(NewValueReader(doc)) + + var got objectIDTest + err := dec.Decode(&got) + assert.EqualError(t, err, "error decoding key id: decoding an object ID to a non-hexadecimal string representation is not supported") + }) t.Run("DefaultDocumentM top-level", func(t *testing.T) { t.Parallel() diff --git a/bson/string_codec.go b/bson/string_codec.go index 3f370d6005..a780677013 100644 --- a/bson/string_codec.go +++ b/bson/string_codec.go @@ -12,10 +12,7 @@ import ( ) // stringCodec is the Codec used for string values. -type stringCodec struct { - // DecodeObjectIDAsHex specifies if object IDs should be decoded as their hex representation. - decodeObjectIDAsHex bool -} +type stringCodec struct{} // Assert that stringCodec satisfies the typeDecoder interface, which allows it to be // used by collection type decoders (e.g. map, slice, etc) to set individual values in a @@ -57,10 +54,10 @@ func (sc *stringCodec) decodeType(dc DecodeContext, vr ValueReader, t reflect.Ty if err != nil { return emptyValue, err } - if sc.decodeObjectIDAsHex || dc.objectIDAsHexString { + if dc.objectIDAsHexString { str = oid.Hex() } else { - return emptyValue, fmt.Errorf("decoding an object ID to a hexadecimal string is disabled by default") + return emptyValue, fmt.Errorf("decoding an object ID to a non-hexadecimal string representation is not supported") } case TypeSymbol: str, err = vr.ReadSymbol() diff --git a/bson/string_codec_test.go b/bson/string_codec_test.go deleted file mode 100644 index 5d9d3d4713..0000000000 --- a/bson/string_codec_test.go +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (C) MongoDB, Inc. 2017-present. -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may -// not use this file except in compliance with the License. You may obtain -// a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - -package bson - -import ( - "errors" - "reflect" - "testing" - - "go.mongodb.org/mongo-driver/internal/assert" -) - -func TestStringCodec(t *testing.T) { - t.Run("ObjectIDAsHex", func(t *testing.T) { - oid := NewObjectID() - reader := &valueReaderWriter{BSONType: TypeObjectID, Return: oid} - testCases := []struct { - name string - stringCodec *stringCodec - result string - err error - }{ - {"true", &stringCodec{decodeObjectIDAsHex: true}, oid.Hex(), nil}, - {"false", &stringCodec{decodeObjectIDAsHex: false}, "", errors.New("decoding an object ID to a hexadecimal string is disabled by default")}, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - actual := reflect.New(reflect.TypeOf("")).Elem() - err := tc.stringCodec.DecodeValue(DecodeContext{}, reader, actual) - if tc.err == nil { - assert.NoErrorf(t, err, "StringCodec.DecodeValue error: %q", err) - } else { - assert.EqualErrorf(t, err, tc.err.Error(), "Expected error %q, got %q", tc.err, err) - } - - actualString := actual.Interface().(string) - assert.Equal(t, tc.result, actualString, "Expected string %v, got %v", tc.result, actualString) - }) - } - }) -}