-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(NODE-2944): Reintroduce bson-ext support #2823
Changes from 8 commits
c6fa310
22c3522
e62c508
9aa4082
dd1531b
a973508
06f5643
4ed77b3
4090343
aadc3ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
#!/bin/bash | ||
|
||
[ -s "$PROJECT_DIRECTORY/node-artifacts/nvm/nvm.sh" ] && source "$PROJECT_DIRECTORY"/node-artifacts/nvm/nvm.sh | ||
|
||
set -o xtrace # Write all commands first to stderr | ||
set -o errexit # Exit the script with error if any of the commands fail | ||
|
||
# Supported/used environment variables: | ||
# SSL Set to enable SSL. Defaults to "nossl" | ||
# MONGODB_URI Set the suggested connection MONGODB_URI (including credentials and topology info) | ||
# TEST_NPM_SCRIPT Script to npm run. Defaults to "check:test" | ||
|
||
MONGODB_URI=${MONGODB_URI:-} | ||
TEST_NPM_SCRIPT=${TEST_NPM_SCRIPT:-check:test} | ||
|
||
# ssl setup | ||
SSL=${SSL:-nossl} | ||
if [ "$SSL" != "nossl" ]; then | ||
export SSL_KEY_FILE="$DRIVERS_TOOLS/.evergreen/x509gen/client.pem" | ||
export SSL_CA_FILE="$DRIVERS_TOOLS/.evergreen/x509gen/ca.pem" | ||
fi | ||
|
||
# run tests | ||
echo "Running $AUTH tests over $SSL, connecting to $MONGODB_URI" | ||
|
||
npm install bson-ext | ||
|
||
export MONGODB_API_VERSION=${MONGODB_API_VERSION} | ||
export MONGODB_URI=${MONGODB_URI} | ||
|
||
npm run "${TEST_NPM_SCRIPT}" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import { expectType } from 'tsd'; | ||
import type { BSONSerializeOptions, Document } from '../../src/bson'; | ||
|
||
const options: BSONSerializeOptions = {}; | ||
|
||
expectType<boolean | undefined>(options.checkKeys); | ||
expectType<boolean | undefined>(options.serializeFunctions); | ||
expectType<boolean | undefined>(options.ignoreUndefined); | ||
expectType<boolean | undefined>(options.promoteLongs); | ||
expectType<boolean | undefined>(options.promoteBuffers); | ||
expectType<boolean | undefined>(options.promoteValues); | ||
expectType<Document | undefined>(options.fieldsAsRaw); | ||
|
||
type PermittedBSONOptionKeys = | ||
| 'checkKeys' | ||
| 'serializeFunctions' | ||
| 'ignoreUndefined' | ||
| 'promoteLongs' | ||
| 'promoteBuffers' | ||
| 'promoteValues' | ||
| 'fieldsAsRaw' | ||
| 'raw'; | ||
|
||
const keys = (null as unknown) as PermittedBSONOptionKeys; | ||
// creates an explicit allow list assertion | ||
expectType<keyof BSONSerializeOptions>(keys); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
'use strict'; | ||
|
||
const { expect } = require('chai'); | ||
const BSON = require('../../src/bson'); | ||
|
||
function isBSONExtInstalled() { | ||
try { | ||
require.resolve('bson-ext'); | ||
return true; | ||
} catch (_) { | ||
return false; | ||
} | ||
} | ||
|
||
describe('When importing BSON', function () { | ||
const types = [ | ||
['Long', 23], | ||
['ObjectId', '123456789123456789123456'], | ||
['Binary', Buffer.from('abc', 'ascii')], | ||
['Timestamp', 23], | ||
['Code', 'function(){}'], | ||
['MinKey', undefined], | ||
['MaxKey', undefined], | ||
['Decimal128', '2.34'], | ||
['Int32', 23], | ||
['Double', 2.3], | ||
['BSONRegExp', 'abc'] | ||
]; | ||
// Omitted types since they're deprecated: | ||
// BSONSymbol | ||
// DBRef | ||
|
||
const options = { | ||
promoteValues: false, | ||
bsonRegExp: true | ||
}; | ||
|
||
function testTypes() { | ||
for (const [type, ctorArg] of types) { | ||
it(`should correctly round trip ${type}`, function () { | ||
const typeCtor = BSON[type]; | ||
expect(typeCtor).to.be.a('function'); | ||
const doc = { key: new typeCtor(ctorArg) }; | ||
const outputDoc = BSON.deserialize(BSON.serialize(doc), options); | ||
expect(outputDoc).to.have.property('key').that.is.instanceOf(typeCtor); | ||
expect(outputDoc).to.deep.equal(doc); | ||
}); | ||
} | ||
|
||
it('should correctly round trip Map', function () { | ||
dariakp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
expect(BSON.Map).to.be.a('function'); | ||
const doc = { key: new BSON.Map([['2', 2]]) }; | ||
const outputDoc = BSON.deserialize(BSON.serialize(doc)); | ||
expect(outputDoc).to.have.nested.property('key.2', 2); | ||
}); | ||
} | ||
|
||
describe('bson-ext should be imported if it exists', function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One tiny suggestion, can we make this describe just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, did this 👍 |
||
before(function () { | ||
if (!isBSONExtInstalled()) { | ||
this.skip(); | ||
} | ||
|
||
expect(BSON.deserialize.toString()).to.include('[native code]'); | ||
expect(BSON.serialize.toString()).to.include('[native code]'); | ||
expect(BSON.calculateObjectSize.toString()).to.include('[native code]'); | ||
}); | ||
|
||
testTypes(); | ||
}); | ||
|
||
describe('js-bson should be imported by default', function () { | ||
before(function () { | ||
if (isBSONExtInstalled()) { | ||
this.skip(); | ||
} | ||
|
||
expect(BSON.deserialize.toString()).to.not.include('[native code]'); | ||
expect(BSON.serialize.toString()).to.not.include('[native code]'); | ||
expect(BSON.calculateObjectSize.toString()).to.not.include('[native code]'); | ||
}); | ||
|
||
testTypes(); | ||
}); | ||
}); |
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 discovered that Serialize and Deserialize options end up being effectively the same here, so this code can be simplified accordingly
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.
I looked over this too quickly in our discussion, there is a difference and these keys below are the ones we desire from the underlying BSON library:
serialize options:
checkKeys
serializeFunctions
ignoreUndefined
deserialize options:
promoteLongs
promoteBuffers
promoteValues
fieldsAsRaw
I added a typescript test to cover this expectation.