From f70be8d95070ff3637f56c5f298536713d6664f4 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 27 Mar 2024 16:08:19 -0400 Subject: [PATCH 01/18] chore(NODE-XXXX): add on demand document class --- package-lock.json | 6 +- package.json | 2 +- src/bson.ts | 9 +- src/cmap/wire_protocol/on_demand/document.ts | 78 +++++++++++++++++ .../wire_protocol/on_demand/string_finder.ts | 23 +++++ src/utils.ts | 66 +++++++++++++-- test/mongodb.ts | 2 + .../wire_protocol/on_demand/document.test.ts | 83 +++++++++++++++++++ .../on_demand/string_finder.test.ts | 24 ++++++ test/unit/utils.test.ts | 57 +++++++++++++ 10 files changed, 337 insertions(+), 13 deletions(-) create mode 100644 src/cmap/wire_protocol/on_demand/document.ts create mode 100644 src/cmap/wire_protocol/on_demand/string_finder.ts create mode 100644 test/unit/cmap/wire_protocol/on_demand/document.test.ts create mode 100644 test/unit/cmap/wire_protocol/on_demand/string_finder.test.ts diff --git a/package-lock.json b/package-lock.json index 7639d4bc479..b1daf7e8988 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "Apache-2.0", "dependencies": { "@mongodb-js/saslprep": "^1.1.5", - "bson": "^6.5.0", + "bson": "github:mongodb/js-bson#main", "mongodb-connection-string-url": "^3.0.0" }, "devDependencies": { @@ -3892,8 +3892,8 @@ }, "node_modules/bson": { "version": "6.5.0", - "resolved": "https://registry.npmjs.org/bson/-/bson-6.5.0.tgz", - "integrity": "sha512-DXf1BTAS8vKyR90BO4x5v3rKVarmkdkzwOrnYDFdjAY694ILNDkmA3uRh1xXJEl+C1DAh8XCvAQ+Gh3kzubtpg==", + "resolved": "git+ssh://git@github.com/mongodb/js-bson.git#d7898f9907d389e5bb40d5b52664a1ff341b49b5", + "license": "Apache-2.0", "engines": { "node": ">=16.20.1" } diff --git a/package.json b/package.json index ac31736335d..53f68f169f3 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ }, "dependencies": { "@mongodb-js/saslprep": "^1.1.5", - "bson": "^6.5.0", + "bson": "github:mongodb/js-bson#main", "mongodb-connection-string-url": "^3.0.0" }, "peerDependencies": { diff --git a/src/bson.ts b/src/bson.ts index 2c0b43df12a..d6621dd77ed 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -1,4 +1,4 @@ -import type { DeserializeOptions, SerializeOptions } from 'bson'; +import { BSON, type DeserializeOptions, type SerializeOptions } from 'bson'; export { Binary, @@ -25,6 +25,13 @@ export { UUID } from 'bson'; +export type BSONElement = BSON.OnDemand['BSONElement']; + +export function parseToElementsToArray(bytes: Uint8Array, offset?: number): BSONElement[] { + const res = BSON.onDemand.parseToElements(bytes, offset); + return Array.isArray(res) ? res : [...res]; +} + /** * BSON Serialization options. * @public diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts new file mode 100644 index 00000000000..0112a6bccf5 --- /dev/null +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -0,0 +1,78 @@ +import { BSONError } from 'bson'; + +import { + BSON, + type BSONElement, + type BSONSerializeOptions, + parseToElementsToArray +} from '../../../bson'; +import { StringFinder } from './string_finder'; + +// eslint-disable-next-line no-restricted-syntax +const enum BSONElementOffset { + nameOffset = 1 +} + +/** @internal */ +export class OnDemandDocument { + private readonly existenceOf = Object.create(null); + private readonly elementOf = Object.create(null); + private readonly elements: Array; + + /** The number of elements in the BSON document */ + public readonly length: number; + + constructor( + /** BSON bytes, this document begins at offset */ + protected readonly bson: Uint8Array, + /** The start of the document */ + private readonly offset = 0, + /** If this is an embedded document, indicates if this was a BSON array */ + public readonly isArray = false + ) { + this.elements = parseToElementsToArray(this.bson, offset); + this.length = this.elements.length; + } + + private getElement(name: string): BSONElement | null { + if (this.elementOf[name] != null) { + return this.elementOf[name]; + } + + for (let index = 0; index < this.elements.length; index++) { + const element = this.elements[index]; + + if ( + element != null && + StringFinder.includes(this.bson, name, element[BSONElementOffset.nameOffset]) + ) { + this.elementOf[name] = element; + this.elements[index] = null; + return this.elementOf[name]; + } + } + + return null; + } + + public hasElement(name: string): boolean { + if (name in this.existenceOf) return this.existenceOf[name]; + this.existenceOf[name] = this.getElement(name) != null; + return this.existenceOf[name]; + } + + public toObject(options?: BSONSerializeOptions): Record { + return BSON.deserialize(this.bson, { + ...options, + index: this.offset, + allowObjectSmallerThanBufferSize: true + }); + } + + public toArray(options?: BSONSerializeOptions): Array { + if (!this.isArray) { + throw new BSONError('Unexpected conversion of non-array value to array'); + } + return Array.from(Object.values(this.toObject(options))); + } +} diff --git a/src/cmap/wire_protocol/on_demand/string_finder.ts b/src/cmap/wire_protocol/on_demand/string_finder.ts new file mode 100644 index 00000000000..fadce3a3d5a --- /dev/null +++ b/src/cmap/wire_protocol/on_demand/string_finder.ts @@ -0,0 +1,23 @@ +import { ByteUtils } from '../../../utils'; + +/** + * @internal + * StringFinder is declared as a class so we can delete/test the cache + */ +export class StringFinder { + private static cache: Record = Object.create(null); + + /** + * Given a js string, determine if the element has that name in the BSON sequence. + * + * @remarks + * - Assumes basic latin strings only! + * - Caches the transformation of JS string to bytes for faster lookups + */ + public static includes(bytes: Uint8Array, name: string, at: number): boolean { + if (this.cache[name] == null) { + this.cache[name] = Uint8Array.from(name, c => c.charCodeAt(0)); + } + return ByteUtils.includes(bytes, at, this.cache[name]); + } +} diff --git a/src/utils.ts b/src/utils.ts index b25b3ebb0fc..746840b0154 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -56,8 +56,58 @@ export const ByteUtils = { return ByteUtils.toLocalBufferType(seqA).equals(seqB); }, - compare(this: void, seqA: Uint8Array, seqB: Uint8Array) { - return ByteUtils.toLocalBufferType(seqA).compare(seqB); + /** + * Returns true if subsequence is included in sequence starting at offset + */ + includes(this: void, sequence: Uint8Array, offset: number, subsequence: Uint8Array): boolean { + const subEnd = offset + subsequence.length; + const lengthToCheck = subEnd - offset; + + if (subsequence.length !== lengthToCheck) return false; + + if (lengthToCheck === 1) { + return subsequence[0] === sequence[offset]; + } + + if (lengthToCheck === 2) { + return subsequence[0] === sequence[offset] && subsequence[1] === sequence[offset + 1]; + } + + if (lengthToCheck === 3) { + return ( + subsequence[0] === sequence[offset] && + subsequence[1] === sequence[offset + 1] && + subsequence[2] === sequence[offset + 2] + ); + } + + if (lengthToCheck <= 25) { + for (let i = 0; i < lengthToCheck; i++) { + if (subsequence[i] !== sequence[offset + i]) return false; + } + return true; + } + + const comparison = ByteUtils.compare(subsequence, sequence, offset, subEnd); + return comparison === 0; + }, + + compare( + this: void, + source: Uint8Array, + target: Uint8Array, + targetStart?: number, + targetEnd?: number, + sourceStart?: number, + sourceEnd?: number + ) { + return ByteUtils.toLocalBufferType(source).compare( + target, + targetStart, + targetEnd, + sourceStart, + sourceEnd + ); }, toBase64(this: void, uint8array: Uint8Array) { @@ -1301,18 +1351,18 @@ export async function once(ee: EventEmitter, name: string): Promise { } } -export function maybeAddIdToDocuments( - coll: Collection, +export function maybeAddIdToDocuments( + coll: Collection, docs: Document[], options: { forceServerObjectId?: boolean } ): Document[]; -export function maybeAddIdToDocuments( - coll: Collection, +export function maybeAddIdToDocuments( + coll: Collection, docs: Document, options: { forceServerObjectId?: boolean } ): Document; -export function maybeAddIdToDocuments( - coll: Collection, +export function maybeAddIdToDocuments( + coll: Collection, docOrDocs: Document[] | Document, options: { forceServerObjectId?: boolean } ): Document[] | Document { diff --git a/test/mongodb.ts b/test/mongodb.ts index b043818115f..762a439e666 100644 --- a/test/mongodb.ts +++ b/test/mongodb.ts @@ -130,6 +130,8 @@ export * from '../src/cmap/metrics'; export * from '../src/cmap/stream_description'; export * from '../src/cmap/wire_protocol/compression'; export * from '../src/cmap/wire_protocol/constants'; +export * from '../src/cmap/wire_protocol/on_demand/document'; +export * from '../src/cmap/wire_protocol/on_demand/string_finder'; export * from '../src/cmap/wire_protocol/shared'; export * from '../src/collection'; export * from '../src/connection_string'; diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts new file mode 100644 index 00000000000..bc97660065b --- /dev/null +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -0,0 +1,83 @@ +import { expect } from 'chai'; + +import { BSON, OnDemandDocument } from '../../../../mongodb'; + +describe('class OnDemandDocument', () => { + context('when given an empty BSON sequence', () => { + it('has a length of 0', () => { + const emptyDocument = BSON.serialize({}); + const doc = new OnDemandDocument(emptyDocument, 0, false); + expect(doc).to.have.lengthOf(0); + }); + + it('sets exists cache to false for any key requested', () => { + const emptyDocument = BSON.serialize({}); + const doc = new OnDemandDocument(emptyDocument, 0, false); + expect(doc.hasElement('ok')).to.be.false; + expect(doc.hasElement('$clusterTime')).to.be.false; + expect(doc).to.have.nested.property('existenceOf.ok', false); + expect(doc).to.have.nested.property('existenceOf.$clusterTime', false); + }); + }); + + context('when given a BSON document with ok set to 1', () => { + it('has a length of 1', () => { + const emptyDocument = BSON.serialize({ ok: 1 }); + const doc = new OnDemandDocument(emptyDocument, 0, false); + expect(doc).to.have.lengthOf(1); + }); + + it('sets exists cache to true for ok', () => { + const emptyDocument = BSON.serialize({ ok: 1 }); + const doc = new OnDemandDocument(emptyDocument, 0, false); + expect(doc.hasElement('ok')).to.be.true; + expect(doc).to.have.nested.property('existenceOf.ok', true); + }); + + it('sets exists cache to false for any other key', () => { + const emptyDocument = BSON.serialize({ ok: 1 }); + const doc = new OnDemandDocument(emptyDocument, 0, false); + expect(doc.hasElement('$clusterTime')).to.be.false; + expect(doc).to.have.nested.property('existenceOf.$clusterTime', false); + }); + }); + + context('when given a BSON document with ok set to 0 and code set to 2', () => { + it('has a length of 2', () => { + const emptyDocument = BSON.serialize({ ok: 0, code: 2 }); + const doc = new OnDemandDocument(emptyDocument, 0, false); + expect(doc).to.have.lengthOf(2); + }); + + it('clears element position when finding match', () => { + const emptyDocument = BSON.serialize({ ok: 0, code: 2 }); + const doc = new OnDemandDocument(emptyDocument, 0, false); + expect(doc.hasElement('ok')).to.be.true; + expect(doc).to.have.nested.property('existenceOf.ok', true); + expect(doc).to.have.nested.property('elements[0]').to.be.null; + expect(doc).to.have.nested.property('elements[1]').to.not.be.null; + }); + }); + + context('toObject()', () => { + it('returns the results of calling BSON.deserialize on the document bytes', () => { + const offsetDocument = new Uint8Array([0, 0, 0, ...BSON.serialize({ ok: 0, code: 2 })]); + const doc = new OnDemandDocument(offsetDocument, 3, false); + expect(doc.toObject()).to.deep.equal( + BSON.deserialize(offsetDocument, { index: 3, allowObjectSmallerThanBufferSize: true }) + ); + }); + + it('supports BSON options', () => { + const offsetDocument = new Uint8Array([0, 0, 0, ...BSON.serialize({ ok: 0, code: 2 })]); + const doc = new OnDemandDocument(offsetDocument, 3, false); + expect(doc.toObject({ promoteValues: false })).to.deep.equal( + BSON.deserialize(offsetDocument, { + index: 3, + allowObjectSmallerThanBufferSize: true, + promoteValues: false + }) + ); + }); + }); +}); diff --git a/test/unit/cmap/wire_protocol/on_demand/string_finder.test.ts b/test/unit/cmap/wire_protocol/on_demand/string_finder.test.ts new file mode 100644 index 00000000000..7135bbc232a --- /dev/null +++ b/test/unit/cmap/wire_protocol/on_demand/string_finder.test.ts @@ -0,0 +1,24 @@ +import { expect } from 'chai'; + +import { BSON, StringFinder } from '../../../../mongodb'; + +describe('class StringFinder', () => { + context('includes', () => { + it('returns true for a matching string sequence', () => { + const doc = BSON.serialize({ iLoveJavascript: 1 }); + expect(StringFinder.includes(doc, 'iLoveJavascript', 5)).to.be.true; + }); + + it('returns false for a non-matching string sequence', () => { + const doc = BSON.serialize({ iLoveJavascript: 1 }); + expect(StringFinder.includes(doc, 'iHateJavascript', 5)).to.be.false; + }); + + it('caches the byte sequence of the search string', () => { + expect(StringFinder.includes(new Uint8Array(), 'iLikeJavascript', 0)).to.be.false; + expect(StringFinder) + .to.have.nested.property('cache.iLikeJavascript') + .that.deep.equal(Uint8Array.from('iLikeJavascript', c => c.charCodeAt(0))); + }); + }); +}); diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index d36ddbeb3be..8be465ba1cc 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -906,6 +906,63 @@ describe('driver utils', function () { }); }); + context('includes()', () => { + let compareSpy; + beforeEach(async function () { + compareSpy = sinon.spy(ByteUtils, 'compare'); + }); + + afterEach(async function () { + sinon.restore(); + }); + + it('is a function', () => expect(ByteUtils).property('includes').is.a('function')); + + it('returns true for equal Buffer or Uint8Array', () => { + const buffer = Buffer.from([1, 2, 3]); + const uint8array = new Uint8Array([1, 2, 3]); + + expect(ByteUtils.includes(buffer, 0, uint8array)).to.be.true; + expect(ByteUtils.includes(uint8array, 0, buffer)).to.be.true; + expect(ByteUtils.includes(uint8array, 0, uint8array)).to.be.true; + expect(ByteUtils.includes(buffer, 0, buffer)).to.be.true; + }); + + it('returns false for nonequal Buffer or Uint8Array', () => { + const buffer = Buffer.from([1, 2, 3]); + const uint8array = new Uint8Array([1, 2, 4]); + + expect(ByteUtils.includes(buffer, 0, uint8array)).to.be.false; + expect(ByteUtils.includes(uint8array, 0, buffer)).to.be.false; + }); + + it('returns true for equal subset', () => { + const sequence = Buffer.from([1, 2, 3, 4, 5, 6]); + const subsequence = new Uint8Array([3, 4, 5]); + expect(ByteUtils.includes(sequence, 2, subsequence)).to.be.true; + }); + + it('does not call compare for buffers 25 and under in length', () => { + const uint8array = new Uint8Array([1, 2, 3]); + + expect(ByteUtils.includes(uint8array, 0, new Uint8Array([1]))).to.be.true; + expect(ByteUtils.includes(uint8array, 0, new Uint8Array([1, 2]))).to.be.true; + expect(ByteUtils.includes(uint8array, 0, new Uint8Array([1, 2, 3]))).to.be.true; + expect( + ByteUtils.includes(Uint8Array.from({ length: 100 }), 0, Uint8Array.from({ length: 25 })) + ).to.be.true; + + expect(compareSpy).to.not.have.been.called; + }); + + it('does call compare for buffers over 25 in length', () => { + expect( + ByteUtils.includes(Uint8Array.from({ length: 100 }), 0, Uint8Array.from({ length: 26 })) + ).to.be.true; + expect(compareSpy).to.have.been.called; + }); + }); + context('compare()', () => { it('is a function', () => expect(ByteUtils).property('compare').is.a('function')); From 82c6dbbb1101acca5d49bc09ffeb4394cc1527e6 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 28 Mar 2024 13:27:18 -0400 Subject: [PATCH 02/18] Squashed commit of the following: commit 7768782191e140b71dc33183fa6052dc4667a11a Author: Neal Beeken Date: Thu Mar 28 10:28:55 2024 -0400 chore(NODE-XXXX): add getValue method to OnDemandDocument --- src/bson.ts | 6 + src/cmap/wire_protocol/on_demand/document.ts | 160 ++++++++++++++++-- .../wire_protocol/on_demand/document.test.ts | 137 ++++++++++++++- 3 files changed, 282 insertions(+), 21 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index d6621dd77ed..87f4212a636 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -3,6 +3,7 @@ import { BSON, type DeserializeOptions, type SerializeOptions } from 'bson'; export { Binary, BSON, + BSONError, BSONRegExp, BSONSymbol, BSONType, @@ -32,6 +33,11 @@ export function parseToElementsToArray(bytes: Uint8Array, offset?: number): BSON return Array.isArray(res) ? res : [...res]; } +export const getInt32LE = BSON.onDemand.NumberUtils.getInt32LE; +export const getFloat64LE = BSON.onDemand.NumberUtils.getFloat64LE; +export const getBigInt64LE = BSON.onDemand.NumberUtils.getBigInt64LE; +export const toUTF8 = BSON.onDemand.ByteUtils.toUTF8; + /** * BSON Serialization options. * @public diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index 0112a6bccf5..fe0f1dc7859 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -1,23 +1,49 @@ -import { BSONError } from 'bson'; - import { + Binary, BSON, type BSONElement, + BSONError, type BSONSerializeOptions, - parseToElementsToArray + BSONType, + getBigInt64LE, + getInt32LE, + ObjectId, + parseToElementsToArray, + Timestamp, + toUTF8 } from '../../../bson'; import { StringFinder } from './string_finder'; // eslint-disable-next-line no-restricted-syntax const enum BSONElementOffset { - nameOffset = 1 + type = 0, + nameOffset = 1, + nameLength = 2, + offset = 3, + length = 4 } +export type JSTypeOf = { + [BSONType.int]: number; + [BSONType.long]: bigint; + [BSONType.timestamp]: Timestamp; + [BSONType.binData]: Binary; + [BSONType.bool]: boolean; + [BSONType.objectId]: ObjectId; + [BSONType.string]: string; + [BSONType.date]: Date; + [BSONType.object]: OnDemandDocument; + [BSONType.array]: OnDemandDocument; +}; + /** @internal */ export class OnDemandDocument { - private readonly existenceOf = Object.create(null); - private readonly elementOf = Object.create(null); - private readonly elements: Array; + private readonly existenceOf: Record = Object.create(null); + private readonly elementOf: Record = Object.create(null); + private readonly valueOf: Record = Object.create(null); + private readonly indexFound: Record = Object.create(null); + + private readonly elements: BSONElement[]; /** The number of elements in the BSON document */ public readonly length: number; @@ -35,6 +61,8 @@ export class OnDemandDocument { } private getElement(name: string): BSONElement | null { + if (this.existenceOf[name] === false) return null; + if (this.elementOf[name] != null) { return this.elementOf[name]; } @@ -43,24 +71,116 @@ export class OnDemandDocument { const element = this.elements[index]; if ( - element != null && - StringFinder.includes(this.bson, name, element[BSONElementOffset.nameOffset]) + !this.indexFound[index] && // skip this element if it has already been associated with a name + name.length === element[BSONElementOffset.nameLength] && // Since we assume basic latin, check the js length against the BSON length before comparing + StringFinder.includes(this.bson, name, element[BSONElementOffset.nameOffset]) // compare ) { this.elementOf[name] = element; - this.elements[index] = null; + this.indexFound[index] = true; + this.existenceOf[name] = true; return this.elementOf[name]; } } + this.existenceOf[name] = false; return null; } + private reviveValue(element: BSONElement, as: T): JSTypeOf[T]; + private reviveValue(element: BSONElement, as: keyof JSTypeOf): any { + const type = element[BSONElementOffset.type]; + const offset = element[BSONElementOffset.offset]; + const length = element[BSONElementOffset.length]; + + if (as !== type) { + // FIXME need to translate minKey to unsigned value if support is added later + throw new BSONError(`Expected to find type ${as} at offset ${offset} but found ${type}`); + } + + switch (as) { + case BSONType.int: + return getInt32LE(this.bson, offset); + case BSONType.long: + return getBigInt64LE(this.bson, offset); + case BSONType.bool: + return Boolean(this.bson[offset]); + case BSONType.objectId: + return new ObjectId(this.bson.subarray(offset, offset + 12)); + case BSONType.timestamp: + return new Timestamp(getBigInt64LE(this.bson, offset)); + case BSONType.string: + return toUTF8(this.bson, offset + 4, offset + length - 1, false); + case BSONType.binData: { + const totalBinarySize = getInt32LE(this.bson, offset); + const subType = this.bson[offset + 4]; + + if (subType === 2) { + const subType2BinarySize = getInt32LE(this.bson, offset + 1 + 4); + if (subType2BinarySize < 0) + throw new BSONError('Negative binary type element size found for subtype 0x02'); + if (subType2BinarySize > totalBinarySize - 4) + throw new BSONError('Binary type with subtype 0x02 contains too long binary size'); + if (subType2BinarySize < totalBinarySize - 4) + throw new BSONError('Binary type with subtype 0x02 contains too short binary size'); + return new Binary( + this.bson.subarray(offset + 1 + 4 + 4, offset + 1 + 4 + 4 + subType2BinarySize), + 2 + ); + } + + return new Binary( + this.bson.subarray(offset + 1 + 4, offset + 1 + 4 + totalBinarySize), + subType + ); + } + case BSONType.date: + // Pretend this is correct. + return new Date(Number(getBigInt64LE(this.bson, offset))); + + case BSONType.object: + return new OnDemandDocument(this.bson, offset); + case BSONType.array: + return new OnDemandDocument(this.bson, offset, true); + + default: + throw new Error(`Unsupported BSON type: ${as}`); + } + } + public hasElement(name: string): boolean { - if (name in this.existenceOf) return this.existenceOf[name]; - this.existenceOf[name] = this.getElement(name) != null; - return this.existenceOf[name]; + return (this.existenceOf[name] ??= this.getElement(name) != null); } + public getValue( + name: string, + as: T, + required?: Req + ): Req extends true ? JSTypeOf[T] : JSTypeOf[T] | null; + public getValue( + name: string, + as: T, + required: boolean + ): JSTypeOf[T] | null { + const element = this.getElement(name); + if (element == null) { + if (required === true) { + throw new BSONError(`BSON element "${name}" is missing`); + } else { + return null; + } + } + + if (!(name in this.valueOf)) { + this.valueOf[name] = this.reviveValue(element, as); + } + + return this.valueOf[name]; + } + + /** + * Deserialize this object, will not cache result avoid multiple invocations + * @param options - BSON deserialization options + */ public toObject(options?: BSONSerializeOptions): Record { return BSON.deserialize(this.bson, { ...options, @@ -69,10 +189,20 @@ export class OnDemandDocument { }); } - public toArray(options?: BSONSerializeOptions): Array { + /** + * If this is an array with all elements being the same type + * Skip converting the keys and start iterating the values! + */ + public *valuesAs(as: T): Generator { if (!this.isArray) { throw new BSONError('Unexpected conversion of non-array value to array'); } - return Array.from(Object.values(this.toObject(options))); + let counter = 0; + for (const element of this.elements) { + const item = this.reviveValue(element, as); + this.valueOf[counter] = item; + yield item; + counter += 1; + } } } diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index bc97660065b..32249fa25ee 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -1,6 +1,14 @@ import { expect } from 'chai'; -import { BSON, OnDemandDocument } from '../../../../mongodb'; +import { + Binary, + BSON, + BSONError, + BSONType, + ObjectId, + OnDemandDocument, + Timestamp +} from '../../../../mongodb'; describe('class OnDemandDocument', () => { context('when given an empty BSON sequence', () => { @@ -49,13 +57,13 @@ describe('class OnDemandDocument', () => { expect(doc).to.have.lengthOf(2); }); - it('clears element position when finding match', () => { + it('tracks element position when finding match', () => { const emptyDocument = BSON.serialize({ ok: 0, code: 2 }); const doc = new OnDemandDocument(emptyDocument, 0, false); - expect(doc.hasElement('ok')).to.be.true; - expect(doc).to.have.nested.property('existenceOf.ok', true); - expect(doc).to.have.nested.property('elements[0]').to.be.null; - expect(doc).to.have.nested.property('elements[1]').to.not.be.null; + expect(doc.hasElement('code')).to.be.true; + expect(doc).to.have.nested.property('existenceOf.code', true); + expect(doc).to.not.have.nested.property('indexFound.0'); + expect(doc).to.have.nested.property('indexFound.1', true); }); }); @@ -80,4 +88,121 @@ describe('class OnDemandDocument', () => { ); }); }); + + context('getValue()', () => { + let document: OnDemandDocument; + const input = { + int: 1, + long: 2n, + timestamp: new Timestamp(2n), + binData: new Binary(Uint8Array.from([1, 2, 3]), 3), + bool: true, + objectId: new ObjectId('01'.repeat(12)), + string: 'abc', + date: new Date(0), + object: { a: 1 }, + array: [1, 2] + }; + + beforeEach(async function () { + const bytes = BSON.serialize(input); + document = new OnDemandDocument(bytes); + }); + + it('returns the javascript value matching the as parameter', () => { + expect(document.getValue('bool', BSONType.bool)).to.be.true; + }); + + it('throws if the BSON value mismatches the requested type', () => { + expect(() => document.getValue('bool', BSONType.int)).to.throw(BSONError); + }); + + it('throws if required is set to true and element name does not exist', () => { + expect(() => document.getValue('blah!', BSONType.bool, true)).to.throw(BSONError); + expect(document).to.have.nested.property('existenceOf.blah!', false); + }); + + it('throws if requested type is unsupported', () => { + // @ts-expect-error: checking a bad BSON type + expect(() => document.getValue('bool', 100)).to.throw(BSONError); + }); + + it('caches the value', () => { + document.getValue('int', BSONType.int); + expect(document).to.have.nested.property('valueOf.int', 1); + }); + + it('supports returning int', () => { + expect(document.getValue('int', BSONType.int, true)).to.deep.equal(input.int); + }); + + it('supports returning long', () => { + expect(document.getValue('long', BSONType.long, true)).to.deep.equal(input.long); + }); + + it('supports returning timestamp', () => { + expect(document.getValue('timestamp', BSONType.timestamp, true)).to.deep.equal( + input.timestamp + ); + }); + + it('supports returning binData', () => { + expect(document.getValue('binData', BSONType.binData, true)).to.deep.equal(input.binData); + }); + + it('supports returning bool', () => { + expect(document.getValue('bool', BSONType.bool, true)).to.deep.equal(input.bool); + }); + + it('supports returning objectId', () => { + expect(document.getValue('objectId', BSONType.objectId, true)).to.deep.equal(input.objectId); + }); + + it('supports returning string', () => { + expect(document.getValue('string', BSONType.string, true)).to.deep.equal(input.string); + }); + + it('supports returning date', () => { + expect(document.getValue('date', BSONType.date, true)).to.deep.equal(input.date); + }); + + it('supports returning object', () => { + const o = document.getValue('object', BSONType.object, true); + expect(o).to.be.instanceOf(OnDemandDocument); + expect(o).to.have.lengthOf(1); + }); + + it('supports returning array', () => { + const a = document.getValue('array', BSONType.array, true); + expect(a).to.be.instanceOf(OnDemandDocument); + expect(a).to.have.lengthOf(2); + }); + }); + + context('*valuesAs()', () => { + let array: OnDemandDocument; + beforeEach(async function () { + const bytes = BSON.serialize( + Object.fromEntries(Array.from({ length: 10 }, () => 1).entries()) + ); + array = new OnDemandDocument(bytes, 0, true); + }); + + it('returns a generator that yields values matching the as BSONType parameter', () => { + let didRun = false; + for (const item of array.valuesAs(BSONType.int)) { + didRun = true; + expect(item).to.equal(1); + } + expect(didRun).to.be.true; + }); + + it('caches the results in valueOf', () => { + const generator = array.valuesAs(BSONType.int); + generator.next(); + generator.next(); + expect(array).to.have.nested.property('valueOf.0', 1); + expect(array).to.have.nested.property('valueOf.1', 1); + }); + }); }); From 3b3ab1ac296529cbaf55d2848c9c27236a9613ef Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 28 Mar 2024 13:57:42 -0400 Subject: [PATCH 03/18] chore: add getNumber and docs --- src/cmap/wire_protocol/on_demand/document.ts | 96 ++++++++++++++++++- .../wire_protocol/on_demand/document.test.ts | 40 ++++++++ 2 files changed, 132 insertions(+), 4 deletions(-) diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index fe0f1dc7859..2188f30d93e 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -6,6 +6,7 @@ import { type BSONSerializeOptions, BSONType, getBigInt64LE, + getFloat64LE, getInt32LE, ObjectId, parseToElementsToArray, @@ -38,11 +39,16 @@ export type JSTypeOf = { /** @internal */ export class OnDemandDocument { + /** Caches the existence of a property */ private readonly existenceOf: Record = Object.create(null); + /** Caches a look up of name to element */ private readonly elementOf: Record = Object.create(null); + /** Caches the revived javascript value */ private readonly valueOf: Record = Object.create(null); + /** Caches the index of elements that have been named */ private readonly indexFound: Record = Object.create(null); + /** All bson elements in this document */ private readonly elements: BSONElement[]; /** The number of elements in the BSON document */ @@ -60,6 +66,18 @@ export class OnDemandDocument { this.length = this.elements.length; } + /** + * Seeks into the elements array for an element matching the given name. + * + * @remarks + * Caching: + * - Caches the existence of a property making subsequent look ups for non-existent properties return immediately + * - Caches names mapped to elements to avoid reiterating the array and comparing the name again + * - Caches the index at which an element has been found to prevent rechecking against elements already determined to belong to another name + * + * @param name - a basic latin string name of a BSON element + * @returns + */ private getElement(name: string): BSONElement | null { if (this.existenceOf[name] === false) return null; @@ -86,6 +104,13 @@ export class OnDemandDocument { return null; } + /** + * Translates BSON bytes into a javascript value. Checking `as` against the BSON element's type + * this methods returns the small subset of BSON types that the driver needs to function. + * + * @param element - The element to revive to a javascript value + * @param as - A type byte expected to be returned + */ private reviveValue(element: BSONElement, as: T): JSTypeOf[T]; private reviveValue(element: BSONElement, as: keyof JSTypeOf): any { const type = element[BSONElementOffset.type]; @@ -143,14 +168,32 @@ export class OnDemandDocument { return new OnDemandDocument(this.bson, offset, true); default: - throw new Error(`Unsupported BSON type: ${as}`); + throw new BSONError(`Unsupported BSON type: ${as}`); } } + /** + * Checks for the existence of an element by name. + * + * @remarks + * Uses `getElement` with the expectation that will populate caches such that a hasElement call + * followed by a `getElement` call will not repeat the cost paid by the first look up. + * + * @param name - element name + */ public hasElement(name: string): boolean { return (this.existenceOf[name] ??= this.getElement(name) != null); } + /** + * Turns BSON element with `name` into a javascript value. + * + * @typeParam T - must be one of the supported BSON types determined by `JSTypeOf` this will determine the return type of this function. + * @typeParam Req - A generic to determine the nullish return value of this function. If required is true the return value will not include null. + * @param name - the element name + * @param as - the bson type expected + * @param required - whether or not the element is expected to exist, if true this function will throw if it is not present + */ public getValue( name: string, as: T, @@ -178,7 +221,51 @@ export class OnDemandDocument { } /** - * Deserialize this object, will not cache result avoid multiple invocations + * Supports returning int, double, long, and bool as javascript numbers + * + * @remarks + * **NOTE:** + * - Use this _only_ when you believe the potential precision loss of an int64 is acceptable + * - This method does not cache the result as Longs or booleans would be stored incorrectly + * + * @param name - element name + * @param required - throws if name does not exist + */ + public getNumber( + name: string, + required?: Req + ): Req extends true ? number : number | null; + public getNumber(name: string, required: boolean): number | null { + const element = this.getElement(name); + if (element == null) { + if (required === true) { + throw new BSONError(`BSON element "${name}" is missing`); + } else { + return null; + } + } + + const type = element[BSONElementOffset.type]; + const offset = element[BSONElementOffset.offset]; + + if (type === BSONType.int) { + return getInt32LE(this.bson, offset); + } + if (type === BSONType.double) { + return getFloat64LE(this.bson, offset); + } + if (type === BSONType.long) { + return Number(getBigInt64LE(this.bson, offset)); + } + if (type === BSONType.bool) { + return this.bson[offset] ? 1 : 0; + } + + return null; + } + + /** + * Deserialize this object, DOES NOT cache result so avoid multiple invocations * @param options - BSON deserialization options */ public toObject(options?: BSONSerializeOptions): Record { @@ -190,8 +277,9 @@ export class OnDemandDocument { } /** - * If this is an array with all elements being the same type - * Skip converting the keys and start iterating the values! + * Iterates through the elements of a document reviving them using the `as` BSONType. + * + * @param as - The type to revive all elements as */ public *valuesAs(as: T): Generator { if (!this.isArray) { diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index 32249fa25ee..b80971d4a50 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -179,6 +179,46 @@ describe('class OnDemandDocument', () => { }); }); + context('getNumber()', () => { + let document: OnDemandDocument; + const input = { + int: 1, + long: 2n, + double: 2.3, + bool: false + }; + + beforeEach(async function () { + const bytes = BSON.serialize(input); + document = new OnDemandDocument(bytes); + }); + + it('does not cache the result', () => { + expect(document.getNumber('int')).to.equal(1); + expect(document).to.not.have.nested.property('valueOf.int'); + }); + + it('throws if required is set to true and element name does not exist', () => { + expect(() => document.getNumber('blah!', true)).to.throw(BSONError); + }); + + it('supports parsing int', () => { + expect(document.getNumber('int')).to.equal(1); + }); + + it('supports parsing long', () => { + expect(document.getNumber('long')).to.equal(2); + }); + + it('supports parsing double', () => { + expect(document.getNumber('double')).to.equal(2.3); + }); + + it('supports parsing bool', () => { + expect(document.getNumber('bool')).to.equal(0); + }); + }); + context('*valuesAs()', () => { let array: OnDemandDocument; beforeEach(async function () { From 9e9f265c8d5381b5af6396ea5227792fd2f89374 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 28 Mar 2024 14:03:42 -0400 Subject: [PATCH 04/18] chore: fix import syntax --- src/bson.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bson.ts b/src/bson.ts index 87f4212a636..46eaffd0ed8 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -1,4 +1,5 @@ -import { BSON, type DeserializeOptions, type SerializeOptions } from 'bson'; +import type { DeserializeOptions, SerializeOptions } from 'bson'; +import { BSON } from 'bson'; export { Binary, @@ -33,8 +34,11 @@ export function parseToElementsToArray(bytes: Uint8Array, offset?: number): BSON return Array.isArray(res) ? res : [...res]; } +// eslint-disable-next-line @typescript-eslint/unbound-method export const getInt32LE = BSON.onDemand.NumberUtils.getInt32LE; +// eslint-disable-next-line @typescript-eslint/unbound-method export const getFloat64LE = BSON.onDemand.NumberUtils.getFloat64LE; +// eslint-disable-next-line @typescript-eslint/unbound-method export const getBigInt64LE = BSON.onDemand.NumberUtils.getBigInt64LE; export const toUTF8 = BSON.onDemand.ByteUtils.toUTF8; From b7c385824478cd009437123ec196ae3f77af2058 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 28 Mar 2024 14:20:05 -0400 Subject: [PATCH 05/18] chore: fix required for getNumber --- src/cmap/wire_protocol/on_demand/document.ts | 6 ++++- .../wire_protocol/on_demand/document.test.ts | 22 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index 2188f30d93e..81ea53e5f20 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -261,7 +261,11 @@ export class OnDemandDocument { return this.bson[offset] ? 1 : 0; } - return null; + if (required === true) { + throw new BSONError(`BSON element "${name}" does not have numeric type: ${type}`); + } else { + return null; + } } /** diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index b80971d4a50..5b90c536fae 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -185,7 +185,8 @@ describe('class OnDemandDocument', () => { int: 1, long: 2n, double: 2.3, - bool: false + bool: false, + string: 'abc' }; beforeEach(async function () { @@ -202,6 +203,25 @@ describe('class OnDemandDocument', () => { expect(() => document.getNumber('blah!', true)).to.throw(BSONError); }); + it('throws if required is set to true and element is not numeric', () => { + // just making sure this test does not fail for the non-exist reason + expect(document.hasElement('string')).to.be.true; + expect(() => document.getNumber('string', true)).to.throw(BSONError); + }); + + it('returns null if required is set to false and element does not exist', () => { + expect(document.getNumber('blah!', false)).to.be.null; + expect(document.getNumber('blah!')).to.be.null; + }); + + it('returns null if required is set to false and element is not numeric', () => { + // just making sure this test does not fail for the non-exist reason + expect(document.hasElement('string')).to.be.true; + + expect(document.getNumber('string', false)).to.be.null; + expect(document.getNumber('string')).to.be.null; + }); + it('supports parsing int', () => { expect(document.getNumber('int')).to.equal(1); }); From 9863a53a3f13ae86ebedc161955533dd0879109e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 11:56:00 -0400 Subject: [PATCH 06/18] test: more coverage --- .../wire_protocol/on_demand/document.test.ts | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index 5b90c536fae..2e0ebeb5032 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -4,6 +4,7 @@ import { Binary, BSON, BSONError, + BSONSymbol, BSONType, ObjectId, OnDemandDocument, @@ -96,12 +97,14 @@ describe('class OnDemandDocument', () => { long: 2n, timestamp: new Timestamp(2n), binData: new Binary(Uint8Array.from([1, 2, 3]), 3), + binDataSubtype2: new Binary(Uint8Array.from([1, 2, 3]), 2), bool: true, objectId: new ObjectId('01'.repeat(12)), string: 'abc', date: new Date(0), object: { a: 1 }, - array: [1, 2] + array: [1, 2], + unsupportedType: /abc/ }; beforeEach(async function () { @@ -109,6 +112,10 @@ describe('class OnDemandDocument', () => { document = new OnDemandDocument(bytes); }); + it('returns null if the element does not exist', () => { + expect(document.getValue('blah', BSONType.bool)).to.be.null; + }); + it('returns the javascript value matching the as parameter', () => { expect(document.getValue('bool', BSONType.bool)).to.be.true; }); @@ -123,8 +130,10 @@ describe('class OnDemandDocument', () => { }); it('throws if requested type is unsupported', () => { - // @ts-expect-error: checking a bad BSON type - expect(() => document.getValue('bool', 100)).to.throw(BSONError); + expect(() => { + // @ts-expect-error: checking a bad BSON type + document.getValue('unsupportedType', BSONType.regex); + }).to.throw(BSONError, /unsupported/i); }); it('caches the value', () => { @@ -150,6 +159,12 @@ describe('class OnDemandDocument', () => { expect(document.getValue('binData', BSONType.binData, true)).to.deep.equal(input.binData); }); + it('supports returning binData, subtype 2', () => { + expect(document.getValue('binDataSubtype2', BSONType.binData, true)).to.deep.equal( + input.binDataSubtype2 + ); + }); + it('supports returning bool', () => { expect(document.getValue('bool', BSONType.bool, true)).to.deep.equal(input.bool); }); @@ -186,6 +201,7 @@ describe('class OnDemandDocument', () => { long: 2n, double: 2.3, bool: false, + boolTrue: true, string: 'abc' }; @@ -236,6 +252,7 @@ describe('class OnDemandDocument', () => { it('supports parsing bool', () => { expect(document.getNumber('bool')).to.equal(0); + expect(document.getNumber('boolTrue')).to.equal(1); }); }); @@ -248,6 +265,14 @@ describe('class OnDemandDocument', () => { array = new OnDemandDocument(bytes, 0, true); }); + it('throws if document is not an array', () => { + const bytes = BSON.serialize( + Object.fromEntries(Array.from({ length: 10 }, () => 1).entries()) + ); + array = new OnDemandDocument(bytes, 0, false); + expect(() => array.valuesAs(BSONType.int).next()).to.throw(); + }); + it('returns a generator that yields values matching the as BSONType parameter', () => { let didRun = false; for (const item of array.valuesAs(BSONType.int)) { From 2beee5cf10a9393396dd94b4a1dfe504eed00161 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 11:59:20 -0400 Subject: [PATCH 07/18] chore: simplify names --- src/cmap/wire_protocol/on_demand/document.ts | 8 +-- .../wire_protocol/on_demand/document.test.ts | 52 +++++++++---------- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index 81ea53e5f20..df2983ade95 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -176,12 +176,12 @@ export class OnDemandDocument { * Checks for the existence of an element by name. * * @remarks - * Uses `getElement` with the expectation that will populate caches such that a hasElement call + * Uses `getElement` with the expectation that will populate caches such that a `has` call * followed by a `getElement` call will not repeat the cost paid by the first look up. * * @param name - element name */ - public hasElement(name: string): boolean { + public has(name: string): boolean { return (this.existenceOf[name] ??= this.getElement(name) != null); } @@ -194,12 +194,12 @@ export class OnDemandDocument { * @param as - the bson type expected * @param required - whether or not the element is expected to exist, if true this function will throw if it is not present */ - public getValue( + public get( name: string, as: T, required?: Req ): Req extends true ? JSTypeOf[T] : JSTypeOf[T] | null; - public getValue( + public get( name: string, as: T, required: boolean diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index 2e0ebeb5032..b96c54c2f7a 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -22,8 +22,8 @@ describe('class OnDemandDocument', () => { it('sets exists cache to false for any key requested', () => { const emptyDocument = BSON.serialize({}); const doc = new OnDemandDocument(emptyDocument, 0, false); - expect(doc.hasElement('ok')).to.be.false; - expect(doc.hasElement('$clusterTime')).to.be.false; + expect(doc.has('ok')).to.be.false; + expect(doc.has('$clusterTime')).to.be.false; expect(doc).to.have.nested.property('existenceOf.ok', false); expect(doc).to.have.nested.property('existenceOf.$clusterTime', false); }); @@ -39,14 +39,14 @@ describe('class OnDemandDocument', () => { it('sets exists cache to true for ok', () => { const emptyDocument = BSON.serialize({ ok: 1 }); const doc = new OnDemandDocument(emptyDocument, 0, false); - expect(doc.hasElement('ok')).to.be.true; + expect(doc.has('ok')).to.be.true; expect(doc).to.have.nested.property('existenceOf.ok', true); }); it('sets exists cache to false for any other key', () => { const emptyDocument = BSON.serialize({ ok: 1 }); const doc = new OnDemandDocument(emptyDocument, 0, false); - expect(doc.hasElement('$clusterTime')).to.be.false; + expect(doc.has('$clusterTime')).to.be.false; expect(doc).to.have.nested.property('existenceOf.$clusterTime', false); }); }); @@ -61,7 +61,7 @@ describe('class OnDemandDocument', () => { it('tracks element position when finding match', () => { const emptyDocument = BSON.serialize({ ok: 0, code: 2 }); const doc = new OnDemandDocument(emptyDocument, 0, false); - expect(doc.hasElement('code')).to.be.true; + expect(doc.has('code')).to.be.true; expect(doc).to.have.nested.property('existenceOf.code', true); expect(doc).to.not.have.nested.property('indexFound.0'); expect(doc).to.have.nested.property('indexFound.1', true); @@ -90,7 +90,7 @@ describe('class OnDemandDocument', () => { }); }); - context('getValue()', () => { + context('get()', () => { let document: OnDemandDocument; const input = { int: 1, @@ -113,82 +113,80 @@ describe('class OnDemandDocument', () => { }); it('returns null if the element does not exist', () => { - expect(document.getValue('blah', BSONType.bool)).to.be.null; + expect(document.get('blah', BSONType.bool)).to.be.null; }); it('returns the javascript value matching the as parameter', () => { - expect(document.getValue('bool', BSONType.bool)).to.be.true; + expect(document.get('bool', BSONType.bool)).to.be.true; }); it('throws if the BSON value mismatches the requested type', () => { - expect(() => document.getValue('bool', BSONType.int)).to.throw(BSONError); + expect(() => document.get('bool', BSONType.int)).to.throw(BSONError); }); it('throws if required is set to true and element name does not exist', () => { - expect(() => document.getValue('blah!', BSONType.bool, true)).to.throw(BSONError); + expect(() => document.get('blah!', BSONType.bool, true)).to.throw(BSONError); expect(document).to.have.nested.property('existenceOf.blah!', false); }); it('throws if requested type is unsupported', () => { expect(() => { // @ts-expect-error: checking a bad BSON type - document.getValue('unsupportedType', BSONType.regex); + document.get('unsupportedType', BSONType.regex); }).to.throw(BSONError, /unsupported/i); }); it('caches the value', () => { - document.getValue('int', BSONType.int); + document.get('int', BSONType.int); expect(document).to.have.nested.property('valueOf.int', 1); }); it('supports returning int', () => { - expect(document.getValue('int', BSONType.int, true)).to.deep.equal(input.int); + expect(document.get('int', BSONType.int, true)).to.deep.equal(input.int); }); it('supports returning long', () => { - expect(document.getValue('long', BSONType.long, true)).to.deep.equal(input.long); + expect(document.get('long', BSONType.long, true)).to.deep.equal(input.long); }); it('supports returning timestamp', () => { - expect(document.getValue('timestamp', BSONType.timestamp, true)).to.deep.equal( - input.timestamp - ); + expect(document.get('timestamp', BSONType.timestamp, true)).to.deep.equal(input.timestamp); }); it('supports returning binData', () => { - expect(document.getValue('binData', BSONType.binData, true)).to.deep.equal(input.binData); + expect(document.get('binData', BSONType.binData, true)).to.deep.equal(input.binData); }); it('supports returning binData, subtype 2', () => { - expect(document.getValue('binDataSubtype2', BSONType.binData, true)).to.deep.equal( + expect(document.get('binDataSubtype2', BSONType.binData, true)).to.deep.equal( input.binDataSubtype2 ); }); it('supports returning bool', () => { - expect(document.getValue('bool', BSONType.bool, true)).to.deep.equal(input.bool); + expect(document.get('bool', BSONType.bool, true)).to.deep.equal(input.bool); }); it('supports returning objectId', () => { - expect(document.getValue('objectId', BSONType.objectId, true)).to.deep.equal(input.objectId); + expect(document.get('objectId', BSONType.objectId, true)).to.deep.equal(input.objectId); }); it('supports returning string', () => { - expect(document.getValue('string', BSONType.string, true)).to.deep.equal(input.string); + expect(document.get('string', BSONType.string, true)).to.deep.equal(input.string); }); it('supports returning date', () => { - expect(document.getValue('date', BSONType.date, true)).to.deep.equal(input.date); + expect(document.get('date', BSONType.date, true)).to.deep.equal(input.date); }); it('supports returning object', () => { - const o = document.getValue('object', BSONType.object, true); + const o = document.get('object', BSONType.object, true); expect(o).to.be.instanceOf(OnDemandDocument); expect(o).to.have.lengthOf(1); }); it('supports returning array', () => { - const a = document.getValue('array', BSONType.array, true); + const a = document.get('array', BSONType.array, true); expect(a).to.be.instanceOf(OnDemandDocument); expect(a).to.have.lengthOf(2); }); @@ -221,7 +219,7 @@ describe('class OnDemandDocument', () => { it('throws if required is set to true and element is not numeric', () => { // just making sure this test does not fail for the non-exist reason - expect(document.hasElement('string')).to.be.true; + expect(document.has('string')).to.be.true; expect(() => document.getNumber('string', true)).to.throw(BSONError); }); @@ -232,7 +230,7 @@ describe('class OnDemandDocument', () => { it('returns null if required is set to false and element is not numeric', () => { // just making sure this test does not fail for the non-exist reason - expect(document.hasElement('string')).to.be.true; + expect(document.has('string')).to.be.true; expect(document.getNumber('string', false)).to.be.null; expect(document.getNumber('string')).to.be.null; From a3b07ebad81f2eafc9c976ef7a51f656c849273c Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 12:13:18 -0400 Subject: [PATCH 08/18] chore: remove string caching --- src/cmap/wire_protocol/on_demand/document.ts | 21 ++++++-- .../wire_protocol/on_demand/string_finder.ts | 23 -------- src/utils.ts | 54 +------------------ test/mongodb.ts | 1 - .../wire_protocol/on_demand/document.test.ts | 1 - .../on_demand/string_finder.test.ts | 24 --------- 6 files changed, 19 insertions(+), 105 deletions(-) delete mode 100644 src/cmap/wire_protocol/on_demand/string_finder.ts delete mode 100644 test/unit/cmap/wire_protocol/on_demand/string_finder.test.ts diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index df2983ade95..071945e6e40 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -13,7 +13,6 @@ import { Timestamp, toUTF8 } from '../../../bson'; -import { StringFinder } from './string_finder'; // eslint-disable-next-line no-restricted-syntax const enum BSONElementOffset { @@ -66,6 +65,20 @@ export class OnDemandDocument { this.length = this.elements.length; } + /** Only supports basic latin strings */ + isElementName(name: string, element: BSONElement): boolean { + const nameLength = element[BSONElementOffset.nameLength]; + const nameOffset = element[BSONElementOffset.nameOffset]; + + if (name.length !== nameLength) return false; + + for (let i = 0; i < name.length; i++) { + if (this.bson[nameOffset + i] !== name.charCodeAt(i)) return false; + } + + return true; + } + /** * Seeks into the elements array for an element matching the given name. * @@ -89,9 +102,9 @@ export class OnDemandDocument { const element = this.elements[index]; if ( - !this.indexFound[index] && // skip this element if it has already been associated with a name - name.length === element[BSONElementOffset.nameLength] && // Since we assume basic latin, check the js length against the BSON length before comparing - StringFinder.includes(this.bson, name, element[BSONElementOffset.nameOffset]) // compare + // skip this element if it has already been associated with a name + !this.indexFound[index] && + this.isElementName(name, element) ) { this.elementOf[name] = element; this.indexFound[index] = true; diff --git a/src/cmap/wire_protocol/on_demand/string_finder.ts b/src/cmap/wire_protocol/on_demand/string_finder.ts deleted file mode 100644 index fadce3a3d5a..00000000000 --- a/src/cmap/wire_protocol/on_demand/string_finder.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { ByteUtils } from '../../../utils'; - -/** - * @internal - * StringFinder is declared as a class so we can delete/test the cache - */ -export class StringFinder { - private static cache: Record = Object.create(null); - - /** - * Given a js string, determine if the element has that name in the BSON sequence. - * - * @remarks - * - Assumes basic latin strings only! - * - Caches the transformation of JS string to bytes for faster lookups - */ - public static includes(bytes: Uint8Array, name: string, at: number): boolean { - if (this.cache[name] == null) { - this.cache[name] = Uint8Array.from(name, c => c.charCodeAt(0)); - } - return ByteUtils.includes(bytes, at, this.cache[name]); - } -} diff --git a/src/utils.ts b/src/utils.ts index 746840b0154..59603605b0f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -56,58 +56,8 @@ export const ByteUtils = { return ByteUtils.toLocalBufferType(seqA).equals(seqB); }, - /** - * Returns true if subsequence is included in sequence starting at offset - */ - includes(this: void, sequence: Uint8Array, offset: number, subsequence: Uint8Array): boolean { - const subEnd = offset + subsequence.length; - const lengthToCheck = subEnd - offset; - - if (subsequence.length !== lengthToCheck) return false; - - if (lengthToCheck === 1) { - return subsequence[0] === sequence[offset]; - } - - if (lengthToCheck === 2) { - return subsequence[0] === sequence[offset] && subsequence[1] === sequence[offset + 1]; - } - - if (lengthToCheck === 3) { - return ( - subsequence[0] === sequence[offset] && - subsequence[1] === sequence[offset + 1] && - subsequence[2] === sequence[offset + 2] - ); - } - - if (lengthToCheck <= 25) { - for (let i = 0; i < lengthToCheck; i++) { - if (subsequence[i] !== sequence[offset + i]) return false; - } - return true; - } - - const comparison = ByteUtils.compare(subsequence, sequence, offset, subEnd); - return comparison === 0; - }, - - compare( - this: void, - source: Uint8Array, - target: Uint8Array, - targetStart?: number, - targetEnd?: number, - sourceStart?: number, - sourceEnd?: number - ) { - return ByteUtils.toLocalBufferType(source).compare( - target, - targetStart, - targetEnd, - sourceStart, - sourceEnd - ); + compare(this: void, seqA: Uint8Array, seqB: Uint8Array) { + return ByteUtils.toLocalBufferType(seqA).compare(seqB); }, toBase64(this: void, uint8array: Uint8Array) { diff --git a/test/mongodb.ts b/test/mongodb.ts index 762a439e666..35a2213da65 100644 --- a/test/mongodb.ts +++ b/test/mongodb.ts @@ -131,7 +131,6 @@ export * from '../src/cmap/stream_description'; export * from '../src/cmap/wire_protocol/compression'; export * from '../src/cmap/wire_protocol/constants'; export * from '../src/cmap/wire_protocol/on_demand/document'; -export * from '../src/cmap/wire_protocol/on_demand/string_finder'; export * from '../src/cmap/wire_protocol/shared'; export * from '../src/collection'; export * from '../src/connection_string'; diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index b96c54c2f7a..6dfc4b41971 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -4,7 +4,6 @@ import { Binary, BSON, BSONError, - BSONSymbol, BSONType, ObjectId, OnDemandDocument, diff --git a/test/unit/cmap/wire_protocol/on_demand/string_finder.test.ts b/test/unit/cmap/wire_protocol/on_demand/string_finder.test.ts deleted file mode 100644 index 7135bbc232a..00000000000 --- a/test/unit/cmap/wire_protocol/on_demand/string_finder.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { expect } from 'chai'; - -import { BSON, StringFinder } from '../../../../mongodb'; - -describe('class StringFinder', () => { - context('includes', () => { - it('returns true for a matching string sequence', () => { - const doc = BSON.serialize({ iLoveJavascript: 1 }); - expect(StringFinder.includes(doc, 'iLoveJavascript', 5)).to.be.true; - }); - - it('returns false for a non-matching string sequence', () => { - const doc = BSON.serialize({ iLoveJavascript: 1 }); - expect(StringFinder.includes(doc, 'iHateJavascript', 5)).to.be.false; - }); - - it('caches the byte sequence of the search string', () => { - expect(StringFinder.includes(new Uint8Array(), 'iLikeJavascript', 0)).to.be.false; - expect(StringFinder) - .to.have.nested.property('cache.iLikeJavascript') - .that.deep.equal(Uint8Array.from('iLikeJavascript', c => c.charCodeAt(0))); - }); - }); -}); From b52ed7a1e0ffd95a75807e10d173a50357ae9417 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 12:25:42 -0400 Subject: [PATCH 09/18] test: remove includes --- test/unit/utils.test.ts | 57 ----------------------------------------- 1 file changed, 57 deletions(-) diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index 8be465ba1cc..d36ddbeb3be 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -906,63 +906,6 @@ describe('driver utils', function () { }); }); - context('includes()', () => { - let compareSpy; - beforeEach(async function () { - compareSpy = sinon.spy(ByteUtils, 'compare'); - }); - - afterEach(async function () { - sinon.restore(); - }); - - it('is a function', () => expect(ByteUtils).property('includes').is.a('function')); - - it('returns true for equal Buffer or Uint8Array', () => { - const buffer = Buffer.from([1, 2, 3]); - const uint8array = new Uint8Array([1, 2, 3]); - - expect(ByteUtils.includes(buffer, 0, uint8array)).to.be.true; - expect(ByteUtils.includes(uint8array, 0, buffer)).to.be.true; - expect(ByteUtils.includes(uint8array, 0, uint8array)).to.be.true; - expect(ByteUtils.includes(buffer, 0, buffer)).to.be.true; - }); - - it('returns false for nonequal Buffer or Uint8Array', () => { - const buffer = Buffer.from([1, 2, 3]); - const uint8array = new Uint8Array([1, 2, 4]); - - expect(ByteUtils.includes(buffer, 0, uint8array)).to.be.false; - expect(ByteUtils.includes(uint8array, 0, buffer)).to.be.false; - }); - - it('returns true for equal subset', () => { - const sequence = Buffer.from([1, 2, 3, 4, 5, 6]); - const subsequence = new Uint8Array([3, 4, 5]); - expect(ByteUtils.includes(sequence, 2, subsequence)).to.be.true; - }); - - it('does not call compare for buffers 25 and under in length', () => { - const uint8array = new Uint8Array([1, 2, 3]); - - expect(ByteUtils.includes(uint8array, 0, new Uint8Array([1]))).to.be.true; - expect(ByteUtils.includes(uint8array, 0, new Uint8Array([1, 2]))).to.be.true; - expect(ByteUtils.includes(uint8array, 0, new Uint8Array([1, 2, 3]))).to.be.true; - expect( - ByteUtils.includes(Uint8Array.from({ length: 100 }), 0, Uint8Array.from({ length: 25 })) - ).to.be.true; - - expect(compareSpy).to.not.have.been.called; - }); - - it('does call compare for buffers over 25 in length', () => { - expect( - ByteUtils.includes(Uint8Array.from({ length: 100 }), 0, Uint8Array.from({ length: 26 })) - ).to.be.true; - expect(compareSpy).to.have.been.called; - }); - }); - context('compare()', () => { it('is a function', () => expect(ByteUtils).property('compare').is.a('function')); From 0a9127915a82c7171ae6f976b0d6ab3f0f00c99d Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 13:20:21 -0400 Subject: [PATCH 10/18] chore: update toJSValue to support null and undefined --- src/cmap/wire_protocol/on_demand/document.ts | 41 ++++++++++++++----- .../wire_protocol/on_demand/document.test.ts | 24 ++++++++++- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index 071945e6e40..883fdbb1d44 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -24,6 +24,8 @@ const enum BSONElementOffset { } export type JSTypeOf = { + [BSONType.null]: null; + [BSONType.undefined]: null; [BSONType.int]: number; [BSONType.long]: bigint; [BSONType.timestamp]: Timestamp; @@ -121,21 +123,27 @@ export class OnDemandDocument { * Translates BSON bytes into a javascript value. Checking `as` against the BSON element's type * this methods returns the small subset of BSON types that the driver needs to function. * + * @remarks + * - BSONType.null and BSONType.undefined always return null + * - If the type requested does not match this returns null + * * @param element - The element to revive to a javascript value * @param as - A type byte expected to be returned */ - private reviveValue(element: BSONElement, as: T): JSTypeOf[T]; - private reviveValue(element: BSONElement, as: keyof JSTypeOf): any { + private toJSValue(element: BSONElement, as: T): JSTypeOf[T]; + private toJSValue(element: BSONElement, as: keyof JSTypeOf): any { const type = element[BSONElementOffset.type]; const offset = element[BSONElementOffset.offset]; const length = element[BSONElementOffset.length]; if (as !== type) { - // FIXME need to translate minKey to unsigned value if support is added later - throw new BSONError(`Expected to find type ${as} at offset ${offset} but found ${type}`); + return null; } switch (as) { + case BSONType.null: + case BSONType.undefined: + return null; case BSONType.int: return getInt32LE(this.bson, offset); case BSONType.long: @@ -207,15 +215,19 @@ export class OnDemandDocument { * @param as - the bson type expected * @param required - whether or not the element is expected to exist, if true this function will throw if it is not present */ - public get( + public get( name: string, as: T, - required?: Req - ): Req extends true ? JSTypeOf[T] : JSTypeOf[T] | null; + required?: false | undefined + ): JSTypeOf[T] | null; + + /** `required` will make `get` throw if name does not exist or is null/undefined */ + public get(name: string, as: T, required: true): JSTypeOf[T]; + public get( name: string, as: T, - required: boolean + required?: boolean ): JSTypeOf[T] | null { const element = this.getElement(name); if (element == null) { @@ -227,7 +239,16 @@ export class OnDemandDocument { } if (!(name in this.valueOf)) { - this.valueOf[name] = this.reviveValue(element, as); + const value = this.toJSValue(element, as); + if (value == null) { + if (required === true) { + throw new BSONError(`BSON element "${name}" is missing`); + } else { + return null; + } + } + // It is important to never store null + this.valueOf[name] = value; } return this.valueOf[name]; @@ -304,7 +325,7 @@ export class OnDemandDocument { } let counter = 0; for (const element of this.elements) { - const item = this.reviveValue(element, as); + const item = this.toJSValue(element, as); this.valueOf[counter] = item; yield item; counter += 1; diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index 6dfc4b41971..c46162b8976 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -119,8 +119,16 @@ describe('class OnDemandDocument', () => { expect(document.get('bool', BSONType.bool)).to.be.true; }); - it('throws if the BSON value mismatches the requested type', () => { - expect(() => document.get('bool', BSONType.int)).to.throw(BSONError); + it('returns null if the BSON value mismatches the requested type', () => { + expect(document.get('bool', BSONType.int)).to.be.null; + }); + + it('supports requesting multiple types', () => { + expect( + document.get('bool', BSONType.int) ?? + document.get('bool', BSONType.long) ?? + document.get('bool', BSONType.bool) + ).to.be.true; }); it('throws if required is set to true and element name does not exist', () => { @@ -140,6 +148,18 @@ describe('class OnDemandDocument', () => { expect(document).to.have.nested.property('valueOf.int', 1); }); + it('supports returning null for null and undefined bson elements', () => { + const bson = Uint8Array.from([ + ...[11, 0, 0, 0], // doc size + ...[6, 97, 0], // a: undefined (6) + ...[10, 98, 0], // b: null (10) + 0 // doc term + ]); + const document = new OnDemandDocument(bson, 0, false); + expect(document.get('a', BSONType.undefined)).to.be.null; + expect(document.get('b', BSONType.null)).to.be.null; + }); + it('supports returning int', () => { expect(document.get('int', BSONType.int, true)).to.deep.equal(input.int); }); From ff4886c43718489ba5869aacdad1ff78badebdac Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 13:28:30 -0400 Subject: [PATCH 11/18] chore: refactor getNumber --- src/cmap/wire_protocol/on_demand/document.ts | 38 ++++++------------- .../wire_protocol/on_demand/document.test.ts | 14 ++++--- 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index 883fdbb1d44..c7ad3c85ece 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -26,6 +26,7 @@ const enum BSONElementOffset { export type JSTypeOf = { [BSONType.null]: null; [BSONType.undefined]: null; + [BSONType.double]: number; [BSONType.int]: number; [BSONType.long]: bigint; [BSONType.timestamp]: Timestamp; @@ -144,6 +145,8 @@ export class OnDemandDocument { case BSONType.null: case BSONType.undefined: return null; + case BSONType.double: + return getFloat64LE(this.bson, offset); case BSONType.int: return getInt32LE(this.bson, offset); case BSONType.long: @@ -270,36 +273,19 @@ export class OnDemandDocument { required?: Req ): Req extends true ? number : number | null; public getNumber(name: string, required: boolean): number | null { - const element = this.getElement(name); - if (element == null) { - if (required === true) { - throw new BSONError(`BSON element "${name}" is missing`); - } else { - return null; - } - } + const maybeBool = this.get(name, BSONType.bool); + const bool = maybeBool == null ? null : maybeBool ? 1 : 0; - const type = element[BSONElementOffset.type]; - const offset = element[BSONElementOffset.offset]; + const maybeLong = this.get(name, BSONType.long); + const long = maybeLong == null ? null : Number(maybeLong); - if (type === BSONType.int) { - return getInt32LE(this.bson, offset); - } - if (type === BSONType.double) { - return getFloat64LE(this.bson, offset); - } - if (type === BSONType.long) { - return Number(getBigInt64LE(this.bson, offset)); - } - if (type === BSONType.bool) { - return this.bson[offset] ? 1 : 0; - } + const result = bool ?? long ?? this.get(name, BSONType.int) ?? this.get(name, BSONType.double); - if (required === true) { - throw new BSONError(`BSON element "${name}" does not have numeric type: ${type}`); - } else { - return null; + if (required === true && result == null) { + throw new BSONError(`BSON element "${name}" is missing`); } + + return result; } /** diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index c46162b8976..c4ccf3bf35a 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -93,6 +93,7 @@ describe('class OnDemandDocument', () => { let document: OnDemandDocument; const input = { int: 1, + double: 1.2, long: 2n, timestamp: new Timestamp(2n), binData: new Binary(Uint8Array.from([1, 2, 3]), 3), @@ -164,6 +165,10 @@ describe('class OnDemandDocument', () => { expect(document.get('int', BSONType.int, true)).to.deep.equal(input.int); }); + it('supports returning double', () => { + expect(document.get('double', BSONType.double, true)).to.deep.equal(input.double); + }); + it('supports returning long', () => { expect(document.get('long', BSONType.long, true)).to.deep.equal(input.long); }); @@ -227,11 +232,6 @@ describe('class OnDemandDocument', () => { document = new OnDemandDocument(bytes); }); - it('does not cache the result', () => { - expect(document.getNumber('int')).to.equal(1); - expect(document).to.not.have.nested.property('valueOf.int'); - }); - it('throws if required is set to true and element name does not exist', () => { expect(() => document.getNumber('blah!', true)).to.throw(BSONError); }); @@ -239,7 +239,9 @@ describe('class OnDemandDocument', () => { it('throws if required is set to true and element is not numeric', () => { // just making sure this test does not fail for the non-exist reason expect(document.has('string')).to.be.true; - expect(() => document.getNumber('string', true)).to.throw(BSONError); + expect(() => { + document.getNumber('string', true); + }).to.throw(BSONError); }); it('returns null if required is set to false and element does not exist', () => { From 55f1c071b30b9f5d82750fadb38d1777ee97405e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 14:30:29 -0400 Subject: [PATCH 12/18] chore: undo maybeAddIdToDocuments change --- package-lock.json | 2 +- package.json | 2 +- src/utils.ts | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index b1daf7e8988..8d5095df5d0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "Apache-2.0", "dependencies": { "@mongodb-js/saslprep": "^1.1.5", - "bson": "github:mongodb/js-bson#main", + "bson": "github:mongodb/js-bson#NODE-6059-cleanup", "mongodb-connection-string-url": "^3.0.0" }, "devDependencies": { diff --git a/package.json b/package.json index 53f68f169f3..a9da54a3907 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ }, "dependencies": { "@mongodb-js/saslprep": "^1.1.5", - "bson": "github:mongodb/js-bson#main", + "bson": "github:mongodb/js-bson#NODE-6059-cleanup", "mongodb-connection-string-url": "^3.0.0" }, "peerDependencies": { diff --git a/src/utils.ts b/src/utils.ts index 59603605b0f..b25b3ebb0fc 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1301,18 +1301,18 @@ export async function once(ee: EventEmitter, name: string): Promise { } } -export function maybeAddIdToDocuments( - coll: Collection, +export function maybeAddIdToDocuments( + coll: Collection, docs: Document[], options: { forceServerObjectId?: boolean } ): Document[]; -export function maybeAddIdToDocuments( - coll: Collection, +export function maybeAddIdToDocuments( + coll: Collection, docs: Document, options: { forceServerObjectId?: boolean } ): Document; -export function maybeAddIdToDocuments( - coll: Collection, +export function maybeAddIdToDocuments( + coll: Collection, docOrDocs: Document[] | Document, options: { forceServerObjectId?: boolean } ): Document[] | Document { From 61743193adb50dc6d605e6c223bfef9e70b4499d Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 14:35:30 -0400 Subject: [PATCH 13/18] chore: remove length --- src/cmap/wire_protocol/on_demand/document.ts | 4 ---- test/unit/cmap/wire_protocol/on_demand/document.test.ts | 6 ------ 2 files changed, 10 deletions(-) diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index c7ad3c85ece..44654c2cf12 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -53,9 +53,6 @@ export class OnDemandDocument { /** All bson elements in this document */ private readonly elements: BSONElement[]; - /** The number of elements in the BSON document */ - public readonly length: number; - constructor( /** BSON bytes, this document begins at offset */ protected readonly bson: Uint8Array, @@ -65,7 +62,6 @@ export class OnDemandDocument { public readonly isArray = false ) { this.elements = parseToElementsToArray(this.bson, offset); - this.length = this.elements.length; } /** Only supports basic latin strings */ diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index c4ccf3bf35a..7338c57d7e0 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -12,12 +12,6 @@ import { describe('class OnDemandDocument', () => { context('when given an empty BSON sequence', () => { - it('has a length of 0', () => { - const emptyDocument = BSON.serialize({}); - const doc = new OnDemandDocument(emptyDocument, 0, false); - expect(doc).to.have.lengthOf(0); - }); - it('sets exists cache to false for any key requested', () => { const emptyDocument = BSON.serialize({}); const doc = new OnDemandDocument(emptyDocument, 0, false); From 0d480fa142e86a01877bde7f8aa8e6c4499cb7d5 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 14:54:13 -0400 Subject: [PATCH 14/18] chore: isElementName -> private --- src/cmap/wire_protocol/on_demand/document.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index 44654c2cf12..c3ee084b62d 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -65,7 +65,7 @@ export class OnDemandDocument { } /** Only supports basic latin strings */ - isElementName(name: string, element: BSONElement): boolean { + private isElementName(name: string, element: BSONElement): boolean { const nameLength = element[BSONElementOffset.nameLength]; const nameOffset = element[BSONElementOffset.nameOffset]; From e318e7e09384a5d552145253cd314c359bae13a1 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 15:02:02 -0400 Subject: [PATCH 15/18] docs: fix --- src/cmap/wire_protocol/on_demand/document.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index c3ee084b62d..04f31281e95 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -209,7 +209,6 @@ export class OnDemandDocument { * Turns BSON element with `name` into a javascript value. * * @typeParam T - must be one of the supported BSON types determined by `JSTypeOf` this will determine the return type of this function. - * @typeParam Req - A generic to determine the nullish return value of this function. If required is true the return value will not include null. * @param name - the element name * @param as - the bson type expected * @param required - whether or not the element is expected to exist, if true this function will throw if it is not present From 4a7f7d4f3e65ae3df3dec474120b404259e1e048 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 29 Mar 2024 15:18:13 -0400 Subject: [PATCH 16/18] fix: unit --- .../wire_protocol/on_demand/document.test.ts | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index 7338c57d7e0..c0f567cbd5d 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -23,12 +23,6 @@ describe('class OnDemandDocument', () => { }); context('when given a BSON document with ok set to 1', () => { - it('has a length of 1', () => { - const emptyDocument = BSON.serialize({ ok: 1 }); - const doc = new OnDemandDocument(emptyDocument, 0, false); - expect(doc).to.have.lengthOf(1); - }); - it('sets exists cache to true for ok', () => { const emptyDocument = BSON.serialize({ ok: 1 }); const doc = new OnDemandDocument(emptyDocument, 0, false); @@ -45,12 +39,6 @@ describe('class OnDemandDocument', () => { }); context('when given a BSON document with ok set to 0 and code set to 2', () => { - it('has a length of 2', () => { - const emptyDocument = BSON.serialize({ ok: 0, code: 2 }); - const doc = new OnDemandDocument(emptyDocument, 0, false); - expect(doc).to.have.lengthOf(2); - }); - it('tracks element position when finding match', () => { const emptyDocument = BSON.serialize({ ok: 0, code: 2 }); const doc = new OnDemandDocument(emptyDocument, 0, false); @@ -200,13 +188,14 @@ describe('class OnDemandDocument', () => { it('supports returning object', () => { const o = document.get('object', BSONType.object, true); expect(o).to.be.instanceOf(OnDemandDocument); - expect(o).to.have.lengthOf(1); + expect(o.has('a')).to.be.true; }); it('supports returning array', () => { const a = document.get('array', BSONType.array, true); expect(a).to.be.instanceOf(OnDemandDocument); - expect(a).to.have.lengthOf(2); + expect(a.has('0')).to.be.true; + expect(a.has('1')).to.be.true; }); }); From 74d0730214482331d44ea098f25b4f30b02ea1b4 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 2 Apr 2024 11:36:27 -0400 Subject: [PATCH 17/18] chore: reduce caches --- package-lock.json | 5 +- package.json | 2 +- src/bson.ts | 4 -- src/cmap/wire_protocol/on_demand/document.ts | 61 +++++++++++-------- .../wire_protocol/on_demand/document.test.ts | 22 ++++--- 5 files changed, 49 insertions(+), 45 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8d5095df5d0..b52a50207a1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "Apache-2.0", "dependencies": { "@mongodb-js/saslprep": "^1.1.5", - "bson": "github:mongodb/js-bson#NODE-6059-cleanup", + "bson": "github:mongodb/js-bson#main", "mongodb-connection-string-url": "^3.0.0" }, "devDependencies": { @@ -3892,8 +3892,7 @@ }, "node_modules/bson": { "version": "6.5.0", - "resolved": "git+ssh://git@github.com/mongodb/js-bson.git#d7898f9907d389e5bb40d5b52664a1ff341b49b5", - "license": "Apache-2.0", + "resolved": "git+ssh://git@github.com/mongodb/js-bson.git#3289184ea6d42ccd67fc450393dc7594e9250418", "engines": { "node": ">=16.20.1" } diff --git a/package.json b/package.json index a9da54a3907..53f68f169f3 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ }, "dependencies": { "@mongodb-js/saslprep": "^1.1.5", - "bson": "github:mongodb/js-bson#NODE-6059-cleanup", + "bson": "github:mongodb/js-bson#main", "mongodb-connection-string-url": "^3.0.0" }, "peerDependencies": { diff --git a/src/bson.ts b/src/bson.ts index 46eaffd0ed8..a44f7e2519f 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -33,12 +33,8 @@ export function parseToElementsToArray(bytes: Uint8Array, offset?: number): BSON const res = BSON.onDemand.parseToElements(bytes, offset); return Array.isArray(res) ? res : [...res]; } - -// eslint-disable-next-line @typescript-eslint/unbound-method export const getInt32LE = BSON.onDemand.NumberUtils.getInt32LE; -// eslint-disable-next-line @typescript-eslint/unbound-method export const getFloat64LE = BSON.onDemand.NumberUtils.getFloat64LE; -// eslint-disable-next-line @typescript-eslint/unbound-method export const getBigInt64LE = BSON.onDemand.NumberUtils.getBigInt64LE; export const toUTF8 = BSON.onDemand.ByteUtils.toUTF8; diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts index 04f31281e95..96115a30848 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -39,14 +39,20 @@ export type JSTypeOf = { [BSONType.array]: OnDemandDocument; }; +/** @internal */ +type CachedBSONElement = { element: BSONElement; value: any | undefined }; + /** @internal */ export class OnDemandDocument { - /** Caches the existence of a property */ - private readonly existenceOf: Record = Object.create(null); - /** Caches a look up of name to element */ - private readonly elementOf: Record = Object.create(null); - /** Caches the revived javascript value */ - private readonly valueOf: Record = Object.create(null); + /** + * Maps JS strings to elements and jsValues for speeding up subsequent lookups. + * - If `false` then name does not exist in the BSON document + * - If `CachedBSONElement` instance name exists + * - If `cache[name].value == null` jsValue has not yet been parsed + * - Null/Undefined values do not get cached because they are zero-length values. + */ + private readonly cache: Record = + Object.create(null); /** Caches the index of elements that have been named */ private readonly indexFound: Record = Object.create(null); @@ -90,29 +96,27 @@ export class OnDemandDocument { * @param name - a basic latin string name of a BSON element * @returns */ - private getElement(name: string): BSONElement | null { - if (this.existenceOf[name] === false) return null; + private getElement(name: string): CachedBSONElement | null { + const cachedElement = this.cache[name]; + if (cachedElement === false) return null; - if (this.elementOf[name] != null) { - return this.elementOf[name]; + if (cachedElement != null) { + return cachedElement; } for (let index = 0; index < this.elements.length; index++) { const element = this.elements[index]; - if ( - // skip this element if it has already been associated with a name - !this.indexFound[index] && - this.isElementName(name, element) - ) { - this.elementOf[name] = element; + // skip this element if it has already been associated with a name + if (!this.indexFound[index] && this.isElementName(name, element)) { + const cachedElement = { element, value: undefined }; + this.cache[name] = cachedElement; this.indexFound[index] = true; - this.existenceOf[name] = true; - return this.elementOf[name]; + return cachedElement; } } - this.existenceOf[name] = false; + this.cache[name] = false; return null; } @@ -202,7 +206,10 @@ export class OnDemandDocument { * @param name - element name */ public has(name: string): boolean { - return (this.existenceOf[name] ??= this.getElement(name) != null); + const cachedElement = this.cache[name]; + if (cachedElement === false) return false; + if (cachedElement != null) return true; + return this.getElement(name) != null; } /** @@ -236,8 +243,8 @@ export class OnDemandDocument { } } - if (!(name in this.valueOf)) { - const value = this.toJSValue(element, as); + if (element.value == null) { + const value = this.toJSValue(element.element, as); if (value == null) { if (required === true) { throw new BSONError(`BSON element "${name}" is missing`); @@ -246,10 +253,10 @@ export class OnDemandDocument { } } // It is important to never store null - this.valueOf[name] = value; + element.value = value; } - return this.valueOf[name]; + return element.value; } /** @@ -306,9 +313,9 @@ export class OnDemandDocument { } let counter = 0; for (const element of this.elements) { - const item = this.toJSValue(element, as); - this.valueOf[counter] = item; - yield item; + const value = this.toJSValue(element, as); + this.cache[counter] = { element, value }; + yield value; counter += 1; } } diff --git a/test/unit/cmap/wire_protocol/on_demand/document.test.ts b/test/unit/cmap/wire_protocol/on_demand/document.test.ts index c0f567cbd5d..100062dc3f2 100644 --- a/test/unit/cmap/wire_protocol/on_demand/document.test.ts +++ b/test/unit/cmap/wire_protocol/on_demand/document.test.ts @@ -17,8 +17,8 @@ describe('class OnDemandDocument', () => { const doc = new OnDemandDocument(emptyDocument, 0, false); expect(doc.has('ok')).to.be.false; expect(doc.has('$clusterTime')).to.be.false; - expect(doc).to.have.nested.property('existenceOf.ok', false); - expect(doc).to.have.nested.property('existenceOf.$clusterTime', false); + expect(doc).to.have.nested.property('cache.ok', false); + expect(doc).to.have.nested.property('cache.$clusterTime', false); }); }); @@ -27,14 +27,14 @@ describe('class OnDemandDocument', () => { const emptyDocument = BSON.serialize({ ok: 1 }); const doc = new OnDemandDocument(emptyDocument, 0, false); expect(doc.has('ok')).to.be.true; - expect(doc).to.have.nested.property('existenceOf.ok', true); + expect(doc).to.have.nested.property('cache.ok').that.is.an('object'); }); it('sets exists cache to false for any other key', () => { const emptyDocument = BSON.serialize({ ok: 1 }); const doc = new OnDemandDocument(emptyDocument, 0, false); expect(doc.has('$clusterTime')).to.be.false; - expect(doc).to.have.nested.property('existenceOf.$clusterTime', false); + expect(doc).to.have.nested.property('cache.$clusterTime', false); }); }); @@ -43,7 +43,7 @@ describe('class OnDemandDocument', () => { const emptyDocument = BSON.serialize({ ok: 0, code: 2 }); const doc = new OnDemandDocument(emptyDocument, 0, false); expect(doc.has('code')).to.be.true; - expect(doc).to.have.nested.property('existenceOf.code', true); + expect(doc).to.have.nested.property('cache.code').that.is.an('object'); expect(doc).to.not.have.nested.property('indexFound.0'); expect(doc).to.have.nested.property('indexFound.1', true); }); @@ -116,7 +116,7 @@ describe('class OnDemandDocument', () => { it('throws if required is set to true and element name does not exist', () => { expect(() => document.get('blah!', BSONType.bool, true)).to.throw(BSONError); - expect(document).to.have.nested.property('existenceOf.blah!', false); + expect(document).to.have.nested.property('cache.blah!', false); }); it('throws if requested type is unsupported', () => { @@ -127,8 +127,10 @@ describe('class OnDemandDocument', () => { }); it('caches the value', () => { + document.has('int'); + expect(document).to.have.nested.property('cache.int.value', undefined); document.get('int', BSONType.int); - expect(document).to.have.nested.property('valueOf.int', 1); + expect(document).to.have.nested.property('cache.int.value', 1); }); it('supports returning null for null and undefined bson elements', () => { @@ -284,12 +286,12 @@ describe('class OnDemandDocument', () => { expect(didRun).to.be.true; }); - it('caches the results in valueOf', () => { + it('caches the results of array', () => { const generator = array.valuesAs(BSONType.int); generator.next(); generator.next(); - expect(array).to.have.nested.property('valueOf.0', 1); - expect(array).to.have.nested.property('valueOf.1', 1); + expect(array).to.have.nested.property('cache.0.value', 1); + expect(array).to.have.nested.property('cache.1.value', 1); }); }); }); From 52eed83c71a681543ba16b5ab4bc580a9f6f9b2c Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 2 Apr 2024 12:11:08 -0400 Subject: [PATCH 18/18] chore: bump to bson 6.6.0 --- package-lock.json | 7 ++++--- package.json | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index b52a50207a1..c67dd952565 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "Apache-2.0", "dependencies": { "@mongodb-js/saslprep": "^1.1.5", - "bson": "github:mongodb/js-bson#main", + "bson": "^6.6.0", "mongodb-connection-string-url": "^3.0.0" }, "devDependencies": { @@ -3891,8 +3891,9 @@ } }, "node_modules/bson": { - "version": "6.5.0", - "resolved": "git+ssh://git@github.com/mongodb/js-bson.git#3289184ea6d42ccd67fc450393dc7594e9250418", + "version": "6.6.0", + "resolved": "https://registry.npmjs.org/bson/-/bson-6.6.0.tgz", + "integrity": "sha512-BVINv2SgcMjL4oYbBuCQTpE3/VKOSxrOA8Cj/wQP7izSzlBGVomdm+TcUd0Pzy0ytLSSDweCKQ6X3f5veM5LQA==", "engines": { "node": ">=16.20.1" } diff --git a/package.json b/package.json index 53f68f169f3..ffb24187c31 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ }, "dependencies": { "@mongodb-js/saslprep": "^1.1.5", - "bson": "github:mongodb/js-bson#main", + "bson": "^6.6.0", "mongodb-connection-string-url": "^3.0.0" }, "peerDependencies": {