GODRIVER-1953 Remove bsonx from production code paths. #893
Conversation
mongo/gridfs/bucket.go
Outdated
| doc, err := bsonx.ReadDoc(raw) | ||
| if err != nil { | ||
| return nil, err | ||
| doc := &bson.D{} |
There was a problem hiding this comment.
[opt] Usually I see var doc bson.D and then &doc in the call to UnmarshalWithRegistry. Then you could avoid the dereference with upload.metadata = doc.
There was a problem hiding this comment.
Good suggestion, will update.
Removed the cases where it checks/creates a bsonx.Doc, and replaced them with bson.D type if necessary. Removed functions that seemed only to transform bsonx to another type. Removed a test for an specific function for the same reason. The bsonx package is largely replaced by bson and bsoncore. bsonx was a proof of concept, and it should be completely replaced. There is still various places that use bsonx: - benchmark directory - mongo/gridfs/bucket.go - mongo/gridfs/upload_stream.go - mongo/integration/collection_test.go - mongo/integration/gridfs_spec_test.go And a file inside the same bsonx package uses bsonx package: - x/bsonx/array.go
There was a problem hiding this comment.
@benjirewis here are where the remaining bsonx package references are:
❯ grep -Rl "bsonx\." ./* | sort | uniq
./benchmark/bson.go
./benchmark/bson_document.go
./benchmark/multi.go
./benchmark/single.go
./mongo/cursor.go
./mongo/integration/collection_test.go
./mongo/integration/gridfs_spec_test.go
./mongo/mongo.go
./x/bsonx/document.go
The only remaining references in "production" driver paths are mongo/mongo.go (see here) and mongo/cursor.go (see here). Both of those are in switch statements to support bsonx.Doc input parameters. For example:
func transformAndEnsureID(...) {
// ...
switch tt := val.(type) {
// ...
case bsonx.Doc:
val = tt.Copy()
// ...
}And you're correct, removing the bsonx package would break our compatibility guarantee, so we still need to keep the bsonx package and keep support in the driver for bsonx types.
mongo/gridfs/bucket.go
Outdated
| doc, err := bsonx.ReadDoc(raw) | ||
| if err != nil { | ||
| return nil, err | ||
| doc := &bson.D{} |
There was a problem hiding this comment.
Good suggestion, will update.
matthewdale
force-pushed
the
godriver1953-reduce-bsonx
branch
from
8fa14fd to
6386fb9
Compare
GODRIVER-1953
Stop using types and functions from the
bsonxpackage in "production" code paths. Leave references in (most) tests and non-production code (e.g. benchmarks) to make reviewing easier and ensure backward compatibility of the changes. We can remove those additionalbsonxreferences (and maybe thebsonxpackage) in a later change.Based on #692 by RammasEchor