GODRIVER-2612 Merge bsoncodec, bsontype, bsonrw, primitive into bson.
#1565
Conversation
API Change Report./bsonincompatible changes##(*Decoder).Reset: changed from func(./bson/bsonrw.ValueReader) to func(ValueReader) compatible changesArrayCodec: added ./bson/bsoncodecincompatible changespackage removed ./bson/bsonrwincompatible changespackage removed ./bson/bsonrw/bsonrwtestincompatible changespackage removed ./bson/bsontypeincompatible changespackage removed ./bson/mgocompatincompatible changes##GetterEncodeValue: changed from func(./bson/bsoncodec.EncodeContext, ./bson/bsonrw.ValueWriter, reflect.Value) error to func(./bson.EncodeContext, ./bson.ValueWriter, reflect.Value) error compatible changesRespectNilValuesRegistry: added ./bson/primitiveincompatible changespackage removed ./eventincompatible changes##CommandFailedEvent.ServiceID: changed from *./bson/primitive.ObjectID to *./bson.ObjectID ./mongoincompatible changes##(*ClientEncryption).AddKeyAltName: changed from func(context.Context, ./bson/primitive.Binary, string) *SingleResult to func(context.Context, ./bson.Binary, string) SingleResult ./mongo/descriptionincompatible changes##SelectedServer.ElectionID: changed from ./bson/primitive.ObjectID to ./bson.ObjectID ./mongo/optionsincompatible changes##(*AggregateOptions).SetCustom: changed from func(./bson/primitive.M) *AggregateOptions to func(./bson.M) *AggregateOptions ./mongo/readconcernincompatible changes##(*ReadConcern).MarshalBSONValue: changed from func() (./bson/bsontype.Type, []byte, error) to func() (./bson.Type, []byte, error) ./mongo/writeconcernincompatible changes##(*WriteConcern).MarshalBSONValue: changed from func() (./bson/bsontype.Type, []byte, error) to func() (./bson.Type, []byte, error) ./x/bsonx/bsoncoreincompatible changes##(*ArrayBuilder).AppendDBPointer: changed from func(string, ./bson/primitive.ObjectID) *ArrayBuilder to func(string, [12]byte) *ArrayBuilder compatible changesType: added ./x/mongo/driver/mongocrypt/optionsincompatible changes##(*ExplicitEncryptionOptions).SetKeyID: changed from func(./bson/primitive.Binary) *ExplicitEncryptionOptions to func(./bson.Binary) *ExplicitEncryptionOptions ./x/mongo/driver/sessionincompatible changes##(Client).AdvanceOperationTime: changed from func(./bson/primitive.Timestamp) error to func(*./bson.Timestamp) error ./x/mongo/driver/topologyincompatible changes##(*Server).ProcessHandshakeError: changed from func(error, uint64, *./bson/primitive.ObjectID) to func(error, uint64, *./bson.ObjectID) |
qingyang-hu
force-pushed
the
godriver2612
branch
8 times, most recently
from
c21ae43 to
8e381fb
Compare
qingyang-hu
force-pushed
the
godriver2612
branch
from
8e381fb to
2ad5ae5
Compare
bson/bsonrw/bsonrwtest/bsonrwtest.go
Outdated
There was a problem hiding this comment.
Move to bson/bsonrw_test.go
bson/primitive/primitive.go
Outdated
There was a problem hiding this comment.
Merge to bson/bson.go
bson/primitive/primitive_test.go
Outdated
There was a problem hiding this comment.
Merge to bson/bson_test.go
bson/bsoncodec/registry.go
Outdated
There was a problem hiding this comment.
Merge to bson/registry.go
bson/bsoncodec/mode.go
Outdated
There was a problem hiding this comment.
Merge to bson/mode.go
bson/bsoncodec/types.go
Outdated
There was a problem hiding this comment.
Merge to bson/types.go
qingyang-hu
force-pushed
the
godriver2612
branch
from
18247ab to
bb61899
Compare
blink1073
changed the title
qingyang-hu
force-pushed
the
godriver2612
branch
5 times, most recently
from
1fe66e2 to
8d409a1
Compare
qingyang-hu
force-pushed
the
godriver2612
branch
from
8d409a1 to
8091e36
Compare
qingyang-hu
changed the title
bsoncodec , bsontype, bsonrw, mgocompat, primitive into bson.
qingyang-hu
requested review from
blink1073,
matthewdale and
prestonvasquez
and removed request for
a team and
blink1073
mongodb-drivers-pr-bot
bot
added
the
review-priority-low
qingyang-hu
added
review-priority-normal
bson/setter_getter.go
Outdated
| type Setter interface { | ||
| SetBSON(raw bson.RawValue) error | ||
| SetBSON(raw RawValue) error | ||
| } |
There was a problem hiding this comment.
I'm concerned merging the mgocompat package into the bson package may create some namespacing problems. I know that was part of the original scope, but looking at the results, it's not clear that it makes sense. Since mgocompat only configures alternative encode/decode behaviors via a Registry and is not used by any of the core BSON logic, there's little risk of import cycles. Should we keep mgocompat a separate package?
There was a problem hiding this comment.
I wonder how long we will support the compatibility with mgo?
There was a problem hiding this comment.
As far as mgo BSON marshal/unmarshal compatibility, I think we need to support it indefinitely. Customers with data in a database that was marshaled with mgo may continue to need unmarshaling compatibility unless they rewrite all incompatible documents with the Go Driver.
qingyang-hu
force-pushed
the
godriver2612
branch
from
7088702 to
2ba80e8
Compare
blink1073
changed the title
bsoncodec, bsontype, bsonrw, mgocompat, primitive into bson.bsoncodec, bsontype, bsonrw, primitive into bson.
|
@qingyang-hu there is a conflict in bson/marshal.go |
| err = src.ReadMaxKey() | ||
| if err != nil { | ||
| break | ||
| } | ||
| err = dst.WriteMaxKey() | ||
| default: | ||
| err = fmt.Errorf("Cannot copy unknown BSON type %s", src.Type()) | ||
| err = fmt.Errorf("cannot copy unknown BSON type %s", src.Type()) |
There was a problem hiding this comment.
| err = fmt.Errorf("cannot copy unknown BSON type %s", src.Type()) | |
| err = fmt.Errorf("cannot copy unknown BSON type %q", src.Type()) |
| return primitive.NilObjectID, fmt.Errorf("$oid value should be string, but instead is %s", ejv.t) | ||
| func (ejv *extJSONValue) parseObjectID() (ObjectID, error) { | ||
| if ejv.t != TypeString { | ||
| return NilObjectID, fmt.Errorf("$oid value should be string, but instead is %s", ejv.t) |
There was a problem hiding this comment.
| return NilObjectID, fmt.Errorf("$oid value should be string, but instead is %s", ejv.t) | |
| return NilObjectID, fmt.Errorf("$oid value should be string, but instead is %q", ejv.t) |
| @@ -128,14 +126,14 @@ func (rv RawValue) String() string { return convertToCoreValue(rv).String() } | |||
| func (rv RawValue) DebugString() string { return convertToCoreValue(rv).DebugString() } | |||
|
|
|||
| // Double returns the float64 value for this element. | |||
| // It panics if e's BSON type is not bsontype.Double. | |||
| // It panics if e's BSON type is not bson.TypeDouble. | |||
There was a problem hiding this comment.
Should we specify "bson.*" in comments in the bson package?
There was a problem hiding this comment.
It seems the convention of the bson package.
| oid, ok := v.ObjectIDOK() | ||
| if !ok { | ||
| return "" | ||
| } | ||
| return fmt.Sprintf(`{"$oid":"%s"}`, oid.Hex()) | ||
| case bsontype.Boolean: | ||
| return fmt.Sprintf(`{"$oid":"%s"}`, idHex(oid)) |
There was a problem hiding this comment.
Why doesn't the original method work?
There was a problem hiding this comment.
The v.ObjectIDOK() at L254 returns [12]byte in this PR to avoid the import cycle. See the change at L523.
| if subtype != bsontype.BinaryGeneric && subtype != bsontype.BinaryBinaryOld { | ||
| return fmt.Errorf("SliceDecodeValue can only be used to decode subtype 0x00 or 0x02 for %s, got %v", bsontype.Binary, subtype) | ||
| if subtype != TypeBinaryGeneric && subtype != TypeBinaryBinaryOld { | ||
| return fmt.Errorf("SliceDecodeValue can only be used to decode subtype 0x00 or 0x02 for %s, got %v", TypeBinary, subtype) |
There was a problem hiding this comment.
| return fmt.Errorf("SliceDecodeValue can only be used to decode subtype 0x00 or 0x02 for %s, got %v", TypeBinary, subtype) | |
| return fmt.Errorf("SliceDecodeValue can only be used to decode subtype 0x00 or 0x02 for %q, got %q", TypeBinary, subtype) |
There was a problem hiding this comment.
This package seems dedicated to decimal128 functionality, so makes more sense to move to internal/decimal128.
x/bsonx/bsoncore/bsoncore.go
Outdated
| } | ||
|
|
||
| // AppendObjectID will append oid to dst and return the extended buffer. | ||
| func AppendObjectID(dst []byte, oid primitive.ObjectID) []byte { return append(dst, oid[:]...) } | ||
| func AppendObjectID(dst []byte, oid [idLen]byte) []byte { return append(dst, oid[:]...) } |
There was a problem hiding this comment.
Instead of defining idLen = 12 and then using type [idLen]byte everywhere, you can use a type alias to prevent having to type [12]byte everywhere.
E.g.
type objectID = [12]byte
func AppendObjectID(dst []byte, oid objectID) []byte { return append(dst, oid[:]...) }To callers outside of the bsoncore package, the function signature will just look like
func AppendObjectID(dst []byte, oid [12]byte) []bytebut you won't have to type [12]byte everywhere.
GODRIVER-2612 Remove references to the
bsonpackages from thebsoncore.GODRIVER-2708 Merge the
bsoncodecGODRIVER-2723 Merge the
bsontypeGODRIVER-2707, GODRIVER-2710 Merge the
bsonrwGODRIVER-2718 Merge themgocompatGODRIVER-2611 Merge the
primitiveSummary
bsoncodec,bsontype,bsonrw,mgocompatprimitiveinto thebson.bsonpackages from thebsoncoreto prevent import cycles.TypeXXXconstants from thebsontypepackage to thebsoncorepackage, so importing is unidirectional from thebsoncore.Copiertype.Background & Motivation
This PR simplifies the following deprecation in GODRIVER-2612 by avoiding import cycles.
Most functions from
bsoncodecwill be deprecated in the following PRs.