-
Notifications
You must be signed in to change notification settings - Fork 98
fix(NODE-4556): attempt to use exported BSON #443
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.
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
Conversation
nbbeeken
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
d773cf7 to
45e6ba7
Compare
| // This case is driver 4.9.0 and higher. | ||
| context('when mongodb exports BSON', function () { | ||
| context('when a bson option is provided', function () { | ||
| const bson = Object.assign({}, BSON); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We clone the original BSON module and use the clone as the bson option so it does not have object equality and then matches.
| ); | ||
|
|
||
| it('uses the mongodb exported BSON', function () { | ||
| expect(encrypter._bson).to.equal(BSON); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we provide no bson option we use the exported BSON from the driver.
| const encrypter = new AutoEncrypter( | ||
| {}, | ||
| { | ||
| bson: bson, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We ensure here we use the bson option.
| }); | ||
|
|
||
| context('when a bson option is not provided', function () { | ||
| const mongoNoBson = { ...mongodb, BSON: undefined }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to ensure BSON is not exported by the module we inject.
|
This has also been rebased/squashed. |
|
filed a flakey test ticket for socks5 tests and restarted the run - https://jira.mongodb.org/browse/NODE-4567 |
This updates the Node bindings to first attempt to use the exported
BSONfrom themongodbmodule before falling back to the old behaviour. This means that:BSONfrom the driver itself.bsonobject passed in the options or onclient.topologyThis allows driver 4.9.0 and higher to not need a connected client to be able to user csfle.
Main of the Node driver is now pinning on the latest commit in this PR and passing, running all live tests: https://spruce.mongodb.com/task/mongo_node_driver_next_ubuntu1804_custom_dependency_tests_run_custom_csfle_tests_latest_pinned_commit_40d485c292a14f8251a4e3724d1bb5f8cf11577c_22_08_17_20_53_00/tests?execution=0&sortBy=STATUS&sortDir=ASC
The patch build shows this works (since the tests pass and bson is no longer a dev dependency) but the dev dependency on mongodb will need to bump to the next release once mongodb/node-mongodb-native#3367 is merged.