From 7a67705ff39cefe718f203ebb973699e913ca254 Mon Sep 17 00:00:00 2001 From: Sean Reece Date: Tue, 6 Aug 2024 14:34:41 -0400 Subject: [PATCH 1/6] Initial commit --- src/objectid.ts | 152 +++++++++++++++++++++++++---------- src/parser/deserializer.ts | 4 +- src/utils/byte_utils.ts | 2 +- src/utils/node_byte_utils.ts | 6 +- src/utils/web_byte_utils.ts | 6 +- test/node/object_id.test.ts | 78 ++++++++++++++---- 6 files changed, 183 insertions(+), 65 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index 98daecc8..71aaceba 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -4,6 +4,25 @@ import { type InspectFn, defaultInspect } from './parser/utils'; import { ByteUtils } from './utils/byte_utils'; import { NumberUtils } from './utils/number_utils'; +const defaultPoolSize = 1000; // Hold 1000 ObjectId buffers in a pool +let pool: Uint8Array; +let poolOffset = 0; + +/** Internal pool accessors for objectId instance */ +/** @internal private instance pool accessor */ +export const _pool = Symbol('pool'); +/** @internal private instance offset accessor */ +export const _offset = Symbol('offset'); + +/** + * Create a new ObjectId buffer pool and reset the pool offset + * @internal + */ +function createPool(): void { + pool = ByteUtils.allocateUnsafe(ObjectId.poolSize * 12); + poolOffset = 0; +} + // Regular expression that checks for hex value const checkForHexRegExp = new RegExp('^[0-9a-fA-F]{24}$'); @@ -37,8 +56,16 @@ export class ObjectId extends BSONValue { static cacheHexString: boolean; - /** ObjectId Bytes @internal */ - private buffer!: Uint8Array; + /** + * The size of the buffer pool for ObjectId. + */ + static poolSize: number = defaultPoolSize; + + /** ObjectId buffer pool pointer @internal */ + private [_pool]: Uint8Array; + /** Buffer pool offset @internal */ + private [_offset]: number; + /** ObjectId hexString cache @internal */ private __id?: string; @@ -72,7 +99,7 @@ export class ObjectId extends BSONValue { * * @param inputId - A 12 byte binary Buffer. */ - constructor(inputId: Uint8Array); + constructor(inputId: Uint8Array, inputIndex?: number); /** To generate a new ObjectId, use ObjectId() with no argument. */ constructor(); /** @@ -86,7 +113,10 @@ export class ObjectId extends BSONValue { * * @param inputId - An input value to create a new ObjectId from. */ - constructor(inputId?: string | number | ObjectId | ObjectIdLike | Uint8Array) { + constructor( + inputId?: string | number | ObjectId | ObjectIdLike | Uint8Array, + inputIndex?: number + ) { super(); // workingId is set based on type of input and whether valid id exists for the input let workingId; @@ -103,17 +133,38 @@ export class ObjectId extends BSONValue { workingId = inputId; } + // If we have reached the end of the pool then create a new pool + if (!pool || poolOffset + 12 > pool.byteLength) { + createPool(); + } + this[_pool] = pool; + this[_offset] = poolOffset; + poolOffset += 12; + // The following cases use workingId to construct an ObjectId if (workingId == null || typeof workingId === 'number') { // The most common use case (blank id, new objectId instance) // Generate a new id - this.buffer = ObjectId.generate(typeof workingId === 'number' ? workingId : undefined); - } else if (ArrayBuffer.isView(workingId) && workingId.byteLength === 12) { - // If intstanceof matches we can escape calling ensure buffer in Node.js environments - this.buffer = ByteUtils.toLocalBufferType(workingId); + ObjectId.generate( + typeof workingId === 'number' ? workingId : undefined, + this[_pool], + this[_offset] + ); + } else if (ArrayBuffer.isView(workingId)) { + if (workingId.byteLength !== 12 && typeof inputIndex !== 'number') { + throw new BSONError('Buffer length must be 12 or offset must be specified'); + } + if ( + inputIndex && + (typeof inputIndex !== 'number' || inputIndex < 0 || workingId.byteLength < inputIndex + 12) + ) { + throw new BSONError('Buffer offset must be a non-negative number less than buffer length'); + } + inputIndex ??= 0; + for (let i = 0; i < 12; i++) this[_pool][this[_offset] + i] = workingId[inputIndex + i]; } else if (typeof workingId === 'string') { if (workingId.length === 24 && checkForHexRegExp.test(workingId)) { - this.buffer = ByteUtils.fromHex(workingId); + this[_pool].set(ByteUtils.fromHex(workingId), this[_offset]); } else { throw new BSONError( 'input must be a 24 character hex string, 12 byte Uint8Array, or an integer' @@ -124,20 +175,28 @@ export class ObjectId extends BSONValue { } // If we are caching the hex string if (ObjectId.cacheHexString) { - this.__id = ByteUtils.toHex(this.id); + this.__id = ByteUtils.toHex(this[_pool], this[_offset], this[_offset] + 12); } } + /** ObjectId bytes @internal */ + get buffer(): Uint8Array { + return this.id; + } + /** * The ObjectId bytes * @readonly */ get id(): Uint8Array { - return this.buffer; + return this[_pool].subarray(this[_offset], this[_offset] + 12); } set id(value: Uint8Array) { - this.buffer = value; + if (value.byteLength !== 12) { + throw new BSONError('input must be a 12 byte Uint8Array'); + } + this[_pool].set(value, this[_offset]); if (ObjectId.cacheHexString) { this.__id = ByteUtils.toHex(value); } @@ -149,7 +208,7 @@ export class ObjectId extends BSONValue { return this.__id; } - const hexString = ByteUtils.toHex(this.id); + const hexString = ByteUtils.toHex(this[_pool], this[_offset], this[_offset] + 12); if (ObjectId.cacheHexString && !this.__id) { this.__id = hexString; @@ -170,17 +229,21 @@ export class ObjectId extends BSONValue { * Generate a 12 byte id buffer used in ObjectId's * * @param time - pass in a second based timestamp. + * @param buffer - Optionally pass in a buffer instance. + * @param offset - Optionally pass in a buffer offset. */ - static generate(time?: number): Uint8Array { + static generate(time?: number, buffer?: Uint8Array, offset: number = 0): Uint8Array { if ('number' !== typeof time) { time = Math.floor(Date.now() / 1000); } const inc = ObjectId.getInc(); - const buffer = ByteUtils.allocateUnsafe(12); + if (!buffer) { + buffer = ByteUtils.allocateUnsafe(12); + } // 4-byte timestamp - NumberUtils.setInt32BE(buffer, 0, time); + NumberUtils.setInt32BE(buffer, offset, time); // set PROCESS_UNIQUE if yet not initialized if (PROCESS_UNIQUE === null) { @@ -188,16 +251,16 @@ export class ObjectId extends BSONValue { } // 5-byte process unique - buffer[4] = PROCESS_UNIQUE[0]; - buffer[5] = PROCESS_UNIQUE[1]; - buffer[6] = PROCESS_UNIQUE[2]; - buffer[7] = PROCESS_UNIQUE[3]; - buffer[8] = PROCESS_UNIQUE[4]; + buffer[offset + 4] = PROCESS_UNIQUE[0]; + buffer[offset + 5] = PROCESS_UNIQUE[1]; + buffer[offset + 6] = PROCESS_UNIQUE[2]; + buffer[offset + 7] = PROCESS_UNIQUE[3]; + buffer[offset + 8] = PROCESS_UNIQUE[4]; // 3-byte counter - buffer[11] = inc & 0xff; - buffer[10] = (inc >> 8) & 0xff; - buffer[9] = (inc >> 16) & 0xff; + buffer[offset + 11] = inc & 0xff; + buffer[offset + 10] = (inc >> 8) & 0xff; + buffer[offset + 9] = (inc >> 16) & 0xff; return buffer; } @@ -239,9 +302,16 @@ export class ObjectId extends BSONValue { } if (ObjectId.is(otherId)) { - return ( - this.buffer[11] === otherId.buffer[11] && ByteUtils.equals(this.buffer, otherId.buffer) - ); + if (otherId[_pool] && typeof otherId[_offset] === 'number') { + for (let i = 11; i >= 0; i--) { + if (this[_pool][this[_offset] + i] !== otherId[_pool][otherId[_offset] + i]) { + return false; + } + } + return true; + } + // If otherId does not have pool and offset, fallback to buffer comparison for compatibility + return ByteUtils.equals(this.buffer, otherId.buffer); } if (typeof otherId === 'string') { @@ -260,7 +330,7 @@ export class ObjectId extends BSONValue { /** Returns the generation date (accurate up to the second) that this ID was generated. */ getTimestamp(): Date { const timestamp = new Date(); - const time = NumberUtils.getUint32BE(this.buffer, 0); + const time = NumberUtils.getUint32BE(this[_pool], this[_offset]); timestamp.setTime(Math.floor(time) * 1000); return timestamp; } @@ -272,18 +342,18 @@ export class ObjectId extends BSONValue { /** @internal */ serializeInto(uint8array: Uint8Array, index: number): 12 { - uint8array[index] = this.buffer[0]; - uint8array[index + 1] = this.buffer[1]; - uint8array[index + 2] = this.buffer[2]; - uint8array[index + 3] = this.buffer[3]; - uint8array[index + 4] = this.buffer[4]; - uint8array[index + 5] = this.buffer[5]; - uint8array[index + 6] = this.buffer[6]; - uint8array[index + 7] = this.buffer[7]; - uint8array[index + 8] = this.buffer[8]; - uint8array[index + 9] = this.buffer[9]; - uint8array[index + 10] = this.buffer[10]; - uint8array[index + 11] = this.buffer[11]; + uint8array[index] = this[_pool][this[_offset]]; + uint8array[index + 1] = this[_pool][this[_offset] + 1]; + uint8array[index + 2] = this[_pool][this[_offset] + 2]; + uint8array[index + 3] = this[_pool][this[_offset] + 3]; + uint8array[index + 4] = this[_pool][this[_offset] + 4]; + uint8array[index + 5] = this[_pool][this[_offset] + 5]; + uint8array[index + 6] = this[_pool][this[_offset] + 6]; + uint8array[index + 7] = this[_pool][this[_offset] + 7]; + uint8array[index + 8] = this[_pool][this[_offset] + 8]; + uint8array[index + 9] = this[_pool][this[_offset] + 9]; + uint8array[index + 10] = this[_pool][this[_offset] + 10]; + uint8array[index + 11] = this[_pool][this[_offset] + 11]; return 12; } @@ -293,7 +363,7 @@ export class ObjectId extends BSONValue { * @param time - an integer number representing a number of seconds. */ static createFromTime(time: number): ObjectId { - const buffer = ByteUtils.allocate(12); + const buffer = ByteUtils.allocateUnsafe(12); for (let i = 11; i >= 4; i--) buffer[i] = 0; // Encode time into first 4 bytes NumberUtils.setInt32BE(buffer, 0, time); diff --git a/src/parser/deserializer.ts b/src/parser/deserializer.ts index 165a529c..0e5e5c91 100644 --- a/src/parser/deserializer.ts +++ b/src/parser/deserializer.ts @@ -263,9 +263,7 @@ function deserializeObject( value = ByteUtils.toUTF8(buffer, index, index + stringSize - 1, shouldValidateKey); index = index + stringSize; } else if (elementType === constants.BSON_DATA_OID) { - const oid = ByteUtils.allocateUnsafe(12); - for (let i = 0; i < 12; i++) oid[i] = buffer[index + i]; - value = new ObjectId(oid); + value = new ObjectId(buffer, index); index = index + 12; } else if (elementType === constants.BSON_DATA_INT && promoteValues === false) { value = new Int32(NumberUtils.getInt32LE(buffer, index)); diff --git a/src/utils/byte_utils.ts b/src/utils/byte_utils.ts index f3da53fd..4fb9d033 100644 --- a/src/utils/byte_utils.ts +++ b/src/utils/byte_utils.ts @@ -30,7 +30,7 @@ export type ByteUtils = { /** Create a Uint8Array from a hex string */ fromHex: (hex: string) => Uint8Array; /** Create a lowercase hex string from bytes */ - toHex: (buffer: Uint8Array) => string; + toHex: (buffer: Uint8Array, start?: number, end?: number) => string; /** Create a string from utf8 code units, fatal=true will throw an error if UTF-8 bytes are invalid, fatal=false will insert replacement characters */ toUTF8: (buffer: Uint8Array, start: number, end: number, fatal: boolean) => string; /** Get the utf8 code unit count from a string if it were to be transformed to utf8 */ diff --git a/src/utils/node_byte_utils.ts b/src/utils/node_byte_utils.ts index ca1482ca..f1614cbf 100644 --- a/src/utils/node_byte_utils.ts +++ b/src/utils/node_byte_utils.ts @@ -6,7 +6,7 @@ type NodeJsEncoding = 'base64' | 'hex' | 'utf8' | 'binary'; type NodeJsBuffer = ArrayBufferView & Uint8Array & { write(string: string, offset: number, length: undefined, encoding: 'utf8'): number; - copy(target: Uint8Array, targetStart: number, sourceStart: number, sourceEnd: number): number; + copy(target: Uint8Array, targetStart: number, sourceStart?: number, sourceEnd?: number): number; toString: (this: Uint8Array, encoding: NodeJsEncoding, start?: number, end?: number) => string; equals: (this: Uint8Array, other: Uint8Array) => boolean; }; @@ -124,8 +124,8 @@ export const nodeJsByteUtils = { return Buffer.from(hex, 'hex'); }, - toHex(buffer: Uint8Array): string { - return nodeJsByteUtils.toLocalBufferType(buffer).toString('hex'); + toHex(buffer: NodeJsBuffer, start?: number, end?: number): string { + return nodeJsByteUtils.toLocalBufferType(buffer).toString('hex', start, end); }, toUTF8(buffer: Uint8Array, start: number, end: number, fatal: boolean): string { diff --git a/src/utils/web_byte_utils.ts b/src/utils/web_byte_utils.ts index 0f79f0df..ad5450c4 100644 --- a/src/utils/web_byte_utils.ts +++ b/src/utils/web_byte_utils.ts @@ -170,8 +170,10 @@ export const webByteUtils = { return Uint8Array.from(buffer); }, - toHex(uint8array: Uint8Array): string { - return Array.from(uint8array, byte => byte.toString(16).padStart(2, '0')).join(''); + toHex(uint8array: Uint8Array, start?: number, end?: number): string { + return Array.from(uint8array.subarray(start, end), byte => + byte.toString(16).padStart(2, '0') + ).join(''); }, toUTF8(uint8array: Uint8Array, start: number, end: number, fatal: boolean): string { diff --git a/test/node/object_id.test.ts b/test/node/object_id.test.ts index 62344c3a..74b7d8cd 100644 --- a/test/node/object_id.test.ts +++ b/test/node/object_id.test.ts @@ -4,6 +4,9 @@ import * as util from 'util'; import { expect } from 'chai'; import { bufferFromHexArray } from './tools/utils'; import { isBufferOrUint8Array } from './tools/utils'; +import { _pool, _offset } from '../../src/objectid'; + +ObjectId.poolSize = 100; describe('ObjectId', function () { describe('static createFromTime()', () => { @@ -258,6 +261,41 @@ describe('ObjectId', function () { ).to.be.true; }); + it('should correctly use buffer pool for ObjectId creation', function () { + const obj = new ObjectId(); + const obj2 = new ObjectId(); + + expect(obj[_offset]).to.not.equal(obj2[_offset]); + expect(obj[_pool]).to.equal(obj2[_pool]); + + expect(obj.id).to.not.equal(obj2.id); + }); + + it('should respect buffer pool size for ObjectId creation', function () { + const oldPoolSize = ObjectId.poolSize; + ObjectId.poolSize = 2; + const test = new ObjectId(); + // Must fill current (large) pool first + const num = (test[_pool].byteLength - test[_offset]) / 12; + for (let i = 0; i < num + 1; i++) { + new ObjectId(); + } + + const obj = new ObjectId(); + const obj2 = new ObjectId(); + const obj3 = new ObjectId(); + + expect(obj[_offset]).to.equal(0); + expect(obj2[_offset]).to.equal(12); + expect(obj3[_offset]).to.equal(0); + expect(obj[_pool]).to.equal(obj2[_pool]); + expect(obj2[_pool]).to.not.equal(obj3[_pool]); + + expect(obj.id).to.not.equal(obj2.id); + expect(obj2.id).to.not.equal(obj3.id); + ObjectId.poolSize = oldPoolSize; + }); + it('should throw error if non-12 byte non-24 hex string passed in', function () { expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(BSONError); expect(() => new ObjectId('thisstringisdefinitelytoolong')).to.throw(BSONError); @@ -306,11 +344,31 @@ describe('ObjectId', function () { done(); }); + it('should correctly create ObjectId from valid Buffer and offset', function (done) { + if (!Buffer.from) return done(); + let a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; + let b = new ObjectId(Buffer.from(`aaaa${a}aaaa`, 'hex'), 2); + let c = b.equals(a); // => false + expect(true).to.equal(c); + + a = 'aaaaaaaaaaaaaaaaaaaaaaaa'; + b = new ObjectId(Buffer.from(`AAAA${a}AAAA`, 'hex'), 2); + c = b.equals(a); // => true + expect(a).to.equal(b.toString()); + expect(true).to.equal(c); + done(); + }); + it('should throw an error if invalid Buffer passed in', function () { const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); expect(() => new ObjectId(a)).to.throw(BSONError); }); + it('should throw an error if invalid Buffer offset passed in', function () { + const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); + expect(() => new ObjectId(a, 5)).to.throw(BSONError); + }); + it('should correctly allow for node.js inspect to work with ObjectId', function (done) { const a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; const b = new ObjectId(a); @@ -425,9 +483,12 @@ describe('ObjectId', function () { let equalId = { _bsontype: 'ObjectId', [oidKId]: oid.id }; const propAccessRecord: string[] = []; + equalId = new Proxy(equalId, { get(target, prop: string, recv) { - if (prop !== '_bsontype') { + if (typeof prop === 'symbol') { + propAccessRecord.push((prop as symbol).toString()); + } else if (prop !== '_bsontype') { propAccessRecord.push(prop); } return Reflect.get(target, prop, recv); @@ -437,23 +498,10 @@ describe('ObjectId', function () { expect(oid.equals(equalId)).to.be.true; // once for the 11th byte shortcut // once for the total equality - expect(propAccessRecord).to.deep.equal([oidKId, oidKId]); + expect(propAccessRecord).to.deep.equal(['Symbol(pool)', oidKId]); }); }); - it('should return the same instance if a buffer is passed in', function () { - const inBuffer = Buffer.from('00'.repeat(12), 'hex'); - - const outBuffer = new ObjectId(inBuffer); - - // instance equality - expect(inBuffer).to.equal(outBuffer.id); - // deep equality - expect(inBuffer).to.deep.equal(outBuffer.id); - // class method equality - expect(Buffer.prototype.equals.call(inBuffer, outBuffer.id)).to.be.true; - }); - context('createFromHexString()', () => { context('when called with a hex sequence', () => { it('returns a ObjectId instance with the decoded bytes', () => { From f41c6b651a3e33f3850a5d7e6906126e265fce5d Mon Sep 17 00:00:00 2001 From: Sean Reece Date: Wed, 14 Aug 2024 23:42:53 -0400 Subject: [PATCH 2/6] Improvements --- src/objectid.ts | 116 ++++++++++++++++++++--------------- src/utils/node_byte_utils.ts | 4 +- test/node/object_id.test.ts | 64 +++++++++++++++---- 3 files changed, 120 insertions(+), 64 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index 71aaceba..750f0ec1 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -4,23 +4,28 @@ import { type InspectFn, defaultInspect } from './parser/utils'; import { ByteUtils } from './utils/byte_utils'; import { NumberUtils } from './utils/number_utils'; -const defaultPoolSize = 1000; // Hold 1000 ObjectId buffers in a pool -let pool: Uint8Array; -let poolOffset = 0; +let currentPool: Uint8Array | null = null; +let poolSize = 1000; // Default: Hold 1000 ObjectId buffers in a pool +let currentPoolOffset = 0; -/** Internal pool accessors for objectId instance */ -/** @internal private instance pool accessor */ -export const _pool = Symbol('pool'); -/** @internal private instance offset accessor */ -export const _offset = Symbol('offset'); +/** + * Retrieves a ObjectId pool and offset. This function may create a new ObjectId buffer pool and reset the pool offset + * @internal + */ +function getPool(): [Uint8Array, number] { + if (!currentPool || currentPoolOffset + 12 > currentPool.byteLength) { + currentPool = ByteUtils.allocateUnsafe(poolSize * 12); + currentPoolOffset = 0; + } + return [currentPool, currentPoolOffset]; +} /** - * Create a new ObjectId buffer pool and reset the pool offset + * Increments the pool offset by 12 bytes * @internal */ -function createPool(): void { - pool = ByteUtils.allocateUnsafe(ObjectId.poolSize * 12); - poolOffset = 0; +function incrementPool(): void { + currentPoolOffset += 12; } // Regular expression that checks for hex value @@ -57,14 +62,24 @@ export class ObjectId extends BSONValue { static cacheHexString: boolean; /** - * The size of the buffer pool for ObjectId. + * The size of the current ObjectId buffer pool. */ - static poolSize: number = defaultPoolSize; + static get poolSize(): number { + return poolSize; + } + + static set poolSize(size: number) { + const iSize = Math.ceil(size); // Ensure new pool size is an integer + if (iSize <= 0) { + throw new BSONError('poolSize must be a positive integer greater than 0'); + } + poolSize = iSize; + } /** ObjectId buffer pool pointer @internal */ - private [_pool]: Uint8Array; + private pool: Uint8Array; /** Buffer pool offset @internal */ - private [_offset]: number; + private offset: number; /** ObjectId hexString cache @internal */ private __id?: string; @@ -99,6 +114,13 @@ export class ObjectId extends BSONValue { * * @param inputId - A 12 byte binary Buffer. */ + constructor(inputId: Uint8Array); + /** + * Create ObjectId from a large binary Buffer. Only 12 bytes starting from the offset are used. + * @internal + * @param inputId - A 12 byte binary Buffer. + * @param inputIndex - The offset to start reading the inputId buffer. + */ constructor(inputId: Uint8Array, inputIndex?: number); /** To generate a new ObjectId, use ObjectId() with no argument. */ constructor(); @@ -133,23 +155,13 @@ export class ObjectId extends BSONValue { workingId = inputId; } - // If we have reached the end of the pool then create a new pool - if (!pool || poolOffset + 12 > pool.byteLength) { - createPool(); - } - this[_pool] = pool; - this[_offset] = poolOffset; - poolOffset += 12; + const [pool, offset] = getPool(); // The following cases use workingId to construct an ObjectId if (workingId == null || typeof workingId === 'number') { // The most common use case (blank id, new objectId instance) // Generate a new id - ObjectId.generate( - typeof workingId === 'number' ? workingId : undefined, - this[_pool], - this[_offset] - ); + ObjectId.generate(typeof workingId === 'number' ? workingId : undefined, pool, offset); } else if (ArrayBuffer.isView(workingId)) { if (workingId.byteLength !== 12 && typeof inputIndex !== 'number') { throw new BSONError('Buffer length must be 12 or offset must be specified'); @@ -161,10 +173,10 @@ export class ObjectId extends BSONValue { throw new BSONError('Buffer offset must be a non-negative number less than buffer length'); } inputIndex ??= 0; - for (let i = 0; i < 12; i++) this[_pool][this[_offset] + i] = workingId[inputIndex + i]; + for (let i = 0; i < 12; i++) pool[offset + i] = workingId[inputIndex + i]; } else if (typeof workingId === 'string') { if (workingId.length === 24 && checkForHexRegExp.test(workingId)) { - this[_pool].set(ByteUtils.fromHex(workingId), this[_offset]); + pool.set(ByteUtils.fromHex(workingId), offset); } else { throw new BSONError( 'input must be a 24 character hex string, 12 byte Uint8Array, or an integer' @@ -175,8 +187,12 @@ export class ObjectId extends BSONValue { } // If we are caching the hex string if (ObjectId.cacheHexString) { - this.__id = ByteUtils.toHex(this[_pool], this[_offset], this[_offset] + 12); + this.__id = ByteUtils.toHex(pool, offset, offset + 12); } + // Increment pool offset once we have completed initialization + this.pool = pool; + this.offset = offset; + incrementPool(); } /** ObjectId bytes @internal */ @@ -189,14 +205,14 @@ export class ObjectId extends BSONValue { * @readonly */ get id(): Uint8Array { - return this[_pool].subarray(this[_offset], this[_offset] + 12); + return this.pool.subarray(this.offset, this.offset + 12); } set id(value: Uint8Array) { if (value.byteLength !== 12) { throw new BSONError('input must be a 12 byte Uint8Array'); } - this[_pool].set(value, this[_offset]); + this.pool.set(value, this.offset); if (ObjectId.cacheHexString) { this.__id = ByteUtils.toHex(value); } @@ -208,7 +224,7 @@ export class ObjectId extends BSONValue { return this.__id; } - const hexString = ByteUtils.toHex(this[_pool], this[_offset], this[_offset] + 12); + const hexString = ByteUtils.toHex(this.pool, this.offset, this.offset + 12); if (ObjectId.cacheHexString && !this.__id) { this.__id = hexString; @@ -302,9 +318,9 @@ export class ObjectId extends BSONValue { } if (ObjectId.is(otherId)) { - if (otherId[_pool] && typeof otherId[_offset] === 'number') { + if (otherId.pool && typeof otherId.offset === 'number') { for (let i = 11; i >= 0; i--) { - if (this[_pool][this[_offset] + i] !== otherId[_pool][otherId[_offset] + i]) { + if (this.pool[this.offset + i] !== otherId.pool[otherId.offset + i]) { return false; } } @@ -330,7 +346,7 @@ export class ObjectId extends BSONValue { /** Returns the generation date (accurate up to the second) that this ID was generated. */ getTimestamp(): Date { const timestamp = new Date(); - const time = NumberUtils.getUint32BE(this[_pool], this[_offset]); + const time = NumberUtils.getUint32BE(this.pool, this.offset); timestamp.setTime(Math.floor(time) * 1000); return timestamp; } @@ -342,18 +358,20 @@ export class ObjectId extends BSONValue { /** @internal */ serializeInto(uint8array: Uint8Array, index: number): 12 { - uint8array[index] = this[_pool][this[_offset]]; - uint8array[index + 1] = this[_pool][this[_offset] + 1]; - uint8array[index + 2] = this[_pool][this[_offset] + 2]; - uint8array[index + 3] = this[_pool][this[_offset] + 3]; - uint8array[index + 4] = this[_pool][this[_offset] + 4]; - uint8array[index + 5] = this[_pool][this[_offset] + 5]; - uint8array[index + 6] = this[_pool][this[_offset] + 6]; - uint8array[index + 7] = this[_pool][this[_offset] + 7]; - uint8array[index + 8] = this[_pool][this[_offset] + 8]; - uint8array[index + 9] = this[_pool][this[_offset] + 9]; - uint8array[index + 10] = this[_pool][this[_offset] + 10]; - uint8array[index + 11] = this[_pool][this[_offset] + 11]; + const pool = this.pool; + const offset = this.offset; + uint8array[index] = pool[offset]; + uint8array[index + 1] = pool[offset + 1]; + uint8array[index + 2] = pool[offset + 2]; + uint8array[index + 3] = pool[offset + 3]; + uint8array[index + 4] = pool[offset + 4]; + uint8array[index + 5] = pool[offset + 5]; + uint8array[index + 6] = pool[offset + 6]; + uint8array[index + 7] = pool[offset + 7]; + uint8array[index + 8] = pool[offset + 8]; + uint8array[index + 9] = pool[offset + 9]; + uint8array[index + 10] = pool[offset + 10]; + uint8array[index + 11] = pool[offset + 11]; return 12; } diff --git a/src/utils/node_byte_utils.ts b/src/utils/node_byte_utils.ts index f1614cbf..25eecb0a 100644 --- a/src/utils/node_byte_utils.ts +++ b/src/utils/node_byte_utils.ts @@ -6,7 +6,7 @@ type NodeJsEncoding = 'base64' | 'hex' | 'utf8' | 'binary'; type NodeJsBuffer = ArrayBufferView & Uint8Array & { write(string: string, offset: number, length: undefined, encoding: 'utf8'): number; - copy(target: Uint8Array, targetStart: number, sourceStart?: number, sourceEnd?: number): number; + copy(target: Uint8Array, targetStart: number, sourceStart: number, sourceEnd: number): number; toString: (this: Uint8Array, encoding: NodeJsEncoding, start?: number, end?: number) => string; equals: (this: Uint8Array, other: Uint8Array) => boolean; }; @@ -124,7 +124,7 @@ export const nodeJsByteUtils = { return Buffer.from(hex, 'hex'); }, - toHex(buffer: NodeJsBuffer, start?: number, end?: number): string { + toHex(buffer: Uint8Array, start?: number, end?: number): string { return nodeJsByteUtils.toLocalBufferType(buffer).toString('hex', start, end); }, diff --git a/test/node/object_id.test.ts b/test/node/object_id.test.ts index 74b7d8cd..27491c56 100644 --- a/test/node/object_id.test.ts +++ b/test/node/object_id.test.ts @@ -4,10 +4,16 @@ import * as util from 'util'; import { expect } from 'chai'; import { bufferFromHexArray } from './tools/utils'; import { isBufferOrUint8Array } from './tools/utils'; -import { _pool, _offset } from '../../src/objectid'; ObjectId.poolSize = 100; +declare module '../register-bson' { + interface ObjectId { + pool: Uint8Array; + offset: number; + } +} + describe('ObjectId', function () { describe('static createFromTime()', () => { it('creates an objectId with user defined value in the timestamp field', function () { @@ -265,8 +271,8 @@ describe('ObjectId', function () { const obj = new ObjectId(); const obj2 = new ObjectId(); - expect(obj[_offset]).to.not.equal(obj2[_offset]); - expect(obj[_pool]).to.equal(obj2[_pool]); + expect(obj.offset).to.not.equal(obj2.offset); + expect(obj.pool).to.equal(obj2.pool); expect(obj.id).to.not.equal(obj2.id); }); @@ -276,7 +282,32 @@ describe('ObjectId', function () { ObjectId.poolSize = 2; const test = new ObjectId(); // Must fill current (large) pool first - const num = (test[_pool].byteLength - test[_offset]) / 12; + const num = (test.pool.byteLength - test.offset) / 12; + for (let i = 0; i < num + 1; i++) { + new ObjectId(); + } + + const obj = new ObjectId(); + const obj2 = new ObjectId(); + const obj3 = new ObjectId(); + + expect(obj.offset).to.equal(0); + expect(obj2.offset).to.equal(12); + expect(obj3.offset).to.equal(0); + expect(obj.pool).to.equal(obj2.pool); + expect(obj2.pool).to.not.equal(obj3.pool); + + expect(obj.id).to.not.equal(obj2.id); + expect(obj2.id).to.not.equal(obj3.id); + ObjectId.poolSize = oldPoolSize; + }); + + it('should allow poolSize of 1', function () { + const oldPoolSize = ObjectId.poolSize; + ObjectId.poolSize = 1; + const test = new ObjectId(); + // Must fill current (large) pool first + const num = (test.pool.byteLength - test.offset) / 12; for (let i = 0; i < num + 1; i++) { new ObjectId(); } @@ -285,17 +316,26 @@ describe('ObjectId', function () { const obj2 = new ObjectId(); const obj3 = new ObjectId(); - expect(obj[_offset]).to.equal(0); - expect(obj2[_offset]).to.equal(12); - expect(obj3[_offset]).to.equal(0); - expect(obj[_pool]).to.equal(obj2[_pool]); - expect(obj2[_pool]).to.not.equal(obj3[_pool]); + expect(obj.offset).to.equal(0); + expect(obj2.offset).to.equal(0); + expect(obj3.offset).to.equal(0); + expect(obj.pool).to.not.equal(obj2.pool); + expect(obj2.pool).to.not.equal(obj3.pool); expect(obj.id).to.not.equal(obj2.id); expect(obj2.id).to.not.equal(obj3.id); ObjectId.poolSize = oldPoolSize; }); + it('should not allow 0 poolSize', function () { + const oldPoolSize = ObjectId.poolSize; + expect(() => { + ObjectId.poolSize = 0; + }).to.throw(BSONError); + + ObjectId.poolSize = oldPoolSize; + }); + it('should throw error if non-12 byte non-24 hex string passed in', function () { expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(BSONError); expect(() => new ObjectId('thisstringisdefinitelytoolong')).to.throw(BSONError); @@ -486,9 +526,7 @@ describe('ObjectId', function () { equalId = new Proxy(equalId, { get(target, prop: string, recv) { - if (typeof prop === 'symbol') { - propAccessRecord.push((prop as symbol).toString()); - } else if (prop !== '_bsontype') { + if (prop !== '_bsontype') { propAccessRecord.push(prop); } return Reflect.get(target, prop, recv); @@ -498,7 +536,7 @@ describe('ObjectId', function () { expect(oid.equals(equalId)).to.be.true; // once for the 11th byte shortcut // once for the total equality - expect(propAccessRecord).to.deep.equal(['Symbol(pool)', oidKId]); + expect(propAccessRecord).to.deep.equal(['pool', oidKId]); }); }); From e3bf7de34f7b1027c940c6d5e615a7646cf18cc1 Mon Sep 17 00:00:00 2001 From: Sean Reece Date: Thu, 15 Aug 2024 09:53:47 -0400 Subject: [PATCH 3/6] Fixup offset check --- src/objectid.ts | 16 ++++++++-------- test/node/object_id.test.ts | 7 +++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index 750f0ec1..7bce12e3 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -163,16 +163,16 @@ export class ObjectId extends BSONValue { // Generate a new id ObjectId.generate(typeof workingId === 'number' ? workingId : undefined, pool, offset); } else if (ArrayBuffer.isView(workingId)) { - if (workingId.byteLength !== 12 && typeof inputIndex !== 'number') { - throw new BSONError('Buffer length must be 12 or offset must be specified'); - } - if ( - inputIndex && - (typeof inputIndex !== 'number' || inputIndex < 0 || workingId.byteLength < inputIndex + 12) + if (workingId.byteLength === 12) { + inputIndex = 0; + } else if ( + typeof inputIndex !== 'number' || + inputIndex < 0 || + workingId.byteLength < inputIndex + 12 || + isNaN(inputIndex) ) { - throw new BSONError('Buffer offset must be a non-negative number less than buffer length'); + throw new BSONError('Buffer length must be 12 or a valid offset must be specified'); } - inputIndex ??= 0; for (let i = 0; i < 12; i++) pool[offset + i] = workingId[inputIndex + i]; } else if (typeof workingId === 'string') { if (workingId.length === 24 && checkForHexRegExp.test(workingId)) { diff --git a/test/node/object_id.test.ts b/test/node/object_id.test.ts index 27491c56..cecc2490 100644 --- a/test/node/object_id.test.ts +++ b/test/node/object_id.test.ts @@ -407,6 +407,13 @@ describe('ObjectId', function () { it('should throw an error if invalid Buffer offset passed in', function () { const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); expect(() => new ObjectId(a, 5)).to.throw(BSONError); + expect(() => new ObjectId(a, -1)).to.throw(BSONError); + expect(() => new ObjectId(a, 0n)).to.throw(BSONError); + expect(() => new ObjectId(a, '')).to.throw(BSONError); + expect(() => new ObjectId(a, NaN)).to.throw(BSONError); + expect(() => new ObjectId(a, {})).to.throw(BSONError); + expect(() => new ObjectId(a, false)).to.throw(BSONError); + expect(() => new ObjectId(a, '' + 1)).to.throw(BSONError); }); it('should correctly allow for node.js inspect to work with ObjectId', function (done) { From 5d01f609a2e39a4a598ce0a9bded125fb452f2cb Mon Sep 17 00:00:00 2001 From: Sean Reece Date: Thu, 15 Aug 2024 11:43:55 -0400 Subject: [PATCH 4/6] Fix tests --- test/node/bson_test.js | 12 ++++++++---- test/node/bson_type_classes.test.ts | 9 ++++++++- test/node/extended_json.test.ts | 6 ++++-- test/node/object_id.test.ts | 2 -- test/node/tools/utils.js | 22 ++++++++++++++++++++++ 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/test/node/bson_test.js b/test/node/bson_test.js index 2f495da7..12912b6e 100644 --- a/test/node/bson_test.js +++ b/test/node/bson_test.js @@ -20,7 +20,11 @@ const MaxKey = BSON.MaxKey; const BSONError = BSON.BSONError; const { BinaryParser } = require('./tools/binary_parser'); const vm = require('vm'); -const { assertBuffersEqual, isBufferOrUint8Array } = require('./tools/utils'); +const { + assertBuffersEqual, + isBufferOrUint8Array, + assertDeepEqualsWithObjectId +} = require('./tools/utils'); const { inspect } = require('util'); /** @@ -707,7 +711,7 @@ describe('BSON', function () { expect(serialized_data).to.deep.equal(serialized_data2); var doc2 = b.deserialize(serialized_data); - expect(doc).to.deep.equal(doc2); + assertDeepEqualsWithObjectId(doc, doc2); expect(doc2.dbref.oid.toHexString()).to.deep.equal(oid.toHexString()); done(); }); @@ -1001,7 +1005,7 @@ describe('BSON', function () { var deserialized_data = BSON.deserialize(serialized_data); expect(doc.b).to.deep.equal(deserialized_data.b); - expect(doc).to.deep.equal(deserialized_data); + assertDeepEqualsWithObjectId(doc, deserialized_data); done(); }); @@ -1213,7 +1217,7 @@ describe('BSON', function () { var doc2 = BSON.deserialize(serialized_data); - expect(doc).to.deep.equal(doc2); + assertDeepEqualsWithObjectId(doc, doc2); done(); }); diff --git a/test/node/bson_type_classes.test.ts b/test/node/bson_type_classes.test.ts index 1f843eb7..8ef7a422 100644 --- a/test/node/bson_type_classes.test.ts +++ b/test/node/bson_type_classes.test.ts @@ -18,6 +18,7 @@ import { UUID, BSONValue } from '../register-bson'; +import { assertDeepEqualsWithObjectId } from './tools/utils'; import * as vm from 'node:vm'; const BSONTypeClasses = [ @@ -128,7 +129,13 @@ describe('BSON Type classes common interfaces', () => { ctx.ObjectId = ObjectId; } vm.runInNewContext(`module.exports.result = ${bsonValue.inspect()}`, ctx); - expect(ctx.module.exports.result).to.deep.equal(bsonValue); + + if (ctx.ObjectId) { + // Since ObjectId uses a pool we expect offset to be different + assertDeepEqualsWithObjectId(ctx.module.exports.result, bsonValue); + } else { + expect(ctx.module.exports.result).to.deep.equal(bsonValue); + } }); } }); diff --git a/test/node/extended_json.test.ts b/test/node/extended_json.test.ts index bbeea163..51adce27 100644 --- a/test/node/extended_json.test.ts +++ b/test/node/extended_json.test.ts @@ -4,6 +4,7 @@ import * as vm from 'node:vm'; import { expect } from 'chai'; import { BSONVersionError, BSONRuntimeError } from '../../src'; import { BSONError } from '../register-bson'; +import { assertDeepEqualsWithObjectId } from './tools/utils'; // BSON types const Binary = BSON.Binary; @@ -144,9 +145,10 @@ describe('Extended JSON', function () { const input = '{"result":[{"_id":{"$oid":"591801a468f9e7024b623939"},"emptyField":null}]}'; const parsed = EJSON.parse(input); - expect(parsed).to.deep.equal({ + const expected = { result: [{ _id: new ObjectId('591801a468f9e7024b623939'), emptyField: null }] - }); + }; + assertDeepEqualsWithObjectId(parsed, expected); }); it('should correctly throw when passed a non-string to parse', function () { diff --git a/test/node/object_id.test.ts b/test/node/object_id.test.ts index cecc2490..aba16d33 100644 --- a/test/node/object_id.test.ts +++ b/test/node/object_id.test.ts @@ -5,8 +5,6 @@ import { expect } from 'chai'; import { bufferFromHexArray } from './tools/utils'; import { isBufferOrUint8Array } from './tools/utils'; -ObjectId.poolSize = 100; - declare module '../register-bson' { interface ObjectId { pool: Uint8Array; diff --git a/test/node/tools/utils.js b/test/node/tools/utils.js index 36d51c2a..856fd1e7 100644 --- a/test/node/tools/utils.js +++ b/test/node/tools/utils.js @@ -218,3 +218,25 @@ module.exports.sorted = (iterable, how) => { items.sort(how); return items; }; + +/** Deeply converts all ObjectIds into their hex value representation */ +const deepOidValue = obj => { + if (obj?._bsontype === 'ObjectId') { + return obj.toHexString(); + } + if (Array.isArray(obj)) { + return obj.map(item => deepOidValue(item)); + } else if (obj !== null && typeof obj === 'object' && !Buffer.isBuffer(obj)) { + return Object.entries(obj).reduce((acc, [key, value]) => { + acc[key] = deepOidValue(value); + return acc; + }, {}); + } + return obj; +}; + +module.exports.assertDeepEqualsWithObjectId = (actual, expected) => { + const a = deepOidValue(actual); + const b = deepOidValue(expected); + expect(a).to.deep.equal(b); +}; From 384d3c756cbf3bb1caa4500f0a744b68b62ebdc4 Mon Sep 17 00:00:00 2001 From: Sean Reece Date: Wed, 21 Aug 2024 23:13:04 -0400 Subject: [PATCH 5/6] Improvements --- src/objectid.ts | 21 ++++++++++++++++----- test/node/bson_test.js | 12 ++++-------- test/node/bson_type_classes.test.ts | 13 +++++-------- test/node/extended_json.test.ts | 10 +++++----- test/node/object_id.test.ts | 28 +++++++++++++++++----------- test/node/tools/utils.js | 22 ---------------------- 6 files changed, 47 insertions(+), 59 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index 7bce12e3..e98efdbc 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -69,11 +69,7 @@ export class ObjectId extends BSONValue { } static set poolSize(size: number) { - const iSize = Math.ceil(size); // Ensure new pool size is an integer - if (iSize <= 0) { - throw new BSONError('poolSize must be a positive integer greater than 0'); - } - poolSize = iSize; + poolSize = Math.max(Math.abs(Number(size)) >>> 0, 1); } /** ObjectId buffer pool pointer @internal */ @@ -241,6 +237,21 @@ export class ObjectId extends BSONValue { return (ObjectId.index = (ObjectId.index + 1) % 0xffffff); } + /** + * Generate a 12 byte id buffer used in ObjectId's + * + * @param time - pass in a second based timestamp. + */ + static generate(time?: number): Uint8Array; + /** + * Generate a 12 byte id buffer used in ObjectId's and write to the provided buffer at offset. + * @internal + * + * @param time - pass in a second based timestamp. + * @param buffer - Optionally pass in a buffer instance. + * @param offset - Optionally pass in a buffer offset. + */ + static generate(time?: number, buffer?: Uint8Array, offset?: number): Uint8Array; /** * Generate a 12 byte id buffer used in ObjectId's * diff --git a/test/node/bson_test.js b/test/node/bson_test.js index 12912b6e..9b5ba41f 100644 --- a/test/node/bson_test.js +++ b/test/node/bson_test.js @@ -20,11 +20,7 @@ const MaxKey = BSON.MaxKey; const BSONError = BSON.BSONError; const { BinaryParser } = require('./tools/binary_parser'); const vm = require('vm'); -const { - assertBuffersEqual, - isBufferOrUint8Array, - assertDeepEqualsWithObjectId -} = require('./tools/utils'); +const { assertBuffersEqual, isBufferOrUint8Array } = require('./tools/utils'); const { inspect } = require('util'); /** @@ -711,7 +707,7 @@ describe('BSON', function () { expect(serialized_data).to.deep.equal(serialized_data2); var doc2 = b.deserialize(serialized_data); - assertDeepEqualsWithObjectId(doc, doc2); + expect(b.serialize(doc)).to.deep.equal(b.serialize(doc2)); expect(doc2.dbref.oid.toHexString()).to.deep.equal(oid.toHexString()); done(); }); @@ -1005,7 +1001,7 @@ describe('BSON', function () { var deserialized_data = BSON.deserialize(serialized_data); expect(doc.b).to.deep.equal(deserialized_data.b); - assertDeepEqualsWithObjectId(doc, deserialized_data); + expect(BSON.serialize(doc)).to.deep.equal(BSON.serialize(deserialized_data)); done(); }); @@ -1217,7 +1213,7 @@ describe('BSON', function () { var doc2 = BSON.deserialize(serialized_data); - assertDeepEqualsWithObjectId(doc, doc2); + expect(BSON.serialize(doc)).to.deep.equal(BSON.serialize(doc2)); done(); }); diff --git a/test/node/bson_type_classes.test.ts b/test/node/bson_type_classes.test.ts index 8ef7a422..a3290091 100644 --- a/test/node/bson_type_classes.test.ts +++ b/test/node/bson_type_classes.test.ts @@ -16,9 +16,9 @@ import { ObjectId, Timestamp, UUID, - BSONValue + BSONValue, + BSON } from '../register-bson'; -import { assertDeepEqualsWithObjectId } from './tools/utils'; import * as vm from 'node:vm'; const BSONTypeClasses = [ @@ -130,12 +130,9 @@ describe('BSON Type classes common interfaces', () => { } vm.runInNewContext(`module.exports.result = ${bsonValue.inspect()}`, ctx); - if (ctx.ObjectId) { - // Since ObjectId uses a pool we expect offset to be different - assertDeepEqualsWithObjectId(ctx.module.exports.result, bsonValue); - } else { - expect(ctx.module.exports.result).to.deep.equal(bsonValue); - } + expect(BSON.serialize({ result: ctx.module.exports.result })).to.deep.equal( + BSON.serialize({ result: bsonValue }) + ); }); } }); diff --git a/test/node/extended_json.test.ts b/test/node/extended_json.test.ts index 51adce27..2760a4ce 100644 --- a/test/node/extended_json.test.ts +++ b/test/node/extended_json.test.ts @@ -4,7 +4,6 @@ import * as vm from 'node:vm'; import { expect } from 'chai'; import { BSONVersionError, BSONRuntimeError } from '../../src'; import { BSONError } from '../register-bson'; -import { assertDeepEqualsWithObjectId } from './tools/utils'; // BSON types const Binary = BSON.Binary; @@ -145,10 +144,11 @@ describe('Extended JSON', function () { const input = '{"result":[{"_id":{"$oid":"591801a468f9e7024b623939"},"emptyField":null}]}'; const parsed = EJSON.parse(input); - const expected = { - result: [{ _id: new ObjectId('591801a468f9e7024b623939'), emptyField: null }] - }; - assertDeepEqualsWithObjectId(parsed, expected); + expect(EJSON.serialize(parsed)).to.deep.equal( + EJSON.serialize({ + result: [{ _id: new ObjectId('591801a468f9e7024b623939'), emptyField: null }] + }) + ); }); it('should correctly throw when passed a non-string to parse', function () { diff --git a/test/node/object_id.test.ts b/test/node/object_id.test.ts index aba16d33..6fb93368 100644 --- a/test/node/object_id.test.ts +++ b/test/node/object_id.test.ts @@ -5,13 +5,6 @@ import { expect } from 'chai'; import { bufferFromHexArray } from './tools/utils'; import { isBufferOrUint8Array } from './tools/utils'; -declare module '../register-bson' { - interface ObjectId { - pool: Uint8Array; - offset: number; - } -} - describe('ObjectId', function () { describe('static createFromTime()', () => { it('creates an objectId with user defined value in the timestamp field', function () { @@ -325,11 +318,24 @@ describe('ObjectId', function () { ObjectId.poolSize = oldPoolSize; }); - it('should not allow 0 poolSize', function () { + it('should default to poolSize = 1 when invalid poolSize set', function () { const oldPoolSize = ObjectId.poolSize; - expect(() => { - ObjectId.poolSize = 0; - }).to.throw(BSONError); + + ObjectId.poolSize = 0; + expect(ObjectId.poolSize).to.equal(1); + ObjectId.poolSize = -1; + expect(ObjectId.poolSize).to.equal(1); + ObjectId.poolSize = 0n; + expect(ObjectId.poolSize).to.equal(1); + ObjectId.poolSize = ''; + expect(ObjectId.poolSize).to.equal(1); + ObjectId.poolSize = NaN; + expect(ObjectId.poolSize).to.equal(1); + ObjectId.poolSize = {}; + expect(ObjectId.poolSize).to.equal(1); + ObjectId.poolSize = false; + expect(ObjectId.poolSize).to.equal(1); + ObjectId.poolSize = '1'; ObjectId.poolSize = oldPoolSize; }); diff --git a/test/node/tools/utils.js b/test/node/tools/utils.js index 856fd1e7..36d51c2a 100644 --- a/test/node/tools/utils.js +++ b/test/node/tools/utils.js @@ -218,25 +218,3 @@ module.exports.sorted = (iterable, how) => { items.sort(how); return items; }; - -/** Deeply converts all ObjectIds into their hex value representation */ -const deepOidValue = obj => { - if (obj?._bsontype === 'ObjectId') { - return obj.toHexString(); - } - if (Array.isArray(obj)) { - return obj.map(item => deepOidValue(item)); - } else if (obj !== null && typeof obj === 'object' && !Buffer.isBuffer(obj)) { - return Object.entries(obj).reduce((acc, [key, value]) => { - acc[key] = deepOidValue(value); - return acc; - }, {}); - } - return obj; -}; - -module.exports.assertDeepEqualsWithObjectId = (actual, expected) => { - const a = deepOidValue(actual); - const b = deepOidValue(expected); - expect(a).to.deep.equal(b); -}; From 7aee118b7fea4a1bb4bc9a222620d08f0c991833 Mon Sep 17 00:00:00 2001 From: Sean Reece Date: Thu, 5 Sep 2024 13:33:26 -0400 Subject: [PATCH 6/6] Disable pool and improve perf for poolSize=1 --- src/objectid.ts | 95 ++++++++++++++++++++++--------------- test/node/object_id.test.ts | 35 ++++++++++++-- 2 files changed, 89 insertions(+), 41 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index e98efdbc..9866fc26 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -4,8 +4,11 @@ import { type InspectFn, defaultInspect } from './parser/utils'; import { ByteUtils } from './utils/byte_utils'; import { NumberUtils } from './utils/number_utils'; +// Settings for ObjectId Buffer pool +// Disable pool by default in order to ensure compatibility +// Specify larger poolSize to enable pool let currentPool: Uint8Array | null = null; -let poolSize = 1000; // Default: Hold 1000 ObjectId buffers in a pool +let poolSize = 1; // Disable pool by default. let currentPoolOffset = 0; /** @@ -13,7 +16,7 @@ let currentPoolOffset = 0; * @internal */ function getPool(): [Uint8Array, number] { - if (!currentPool || currentPoolOffset + 12 > currentPool.byteLength) { + if (!currentPool || currentPoolOffset + 12 > currentPool.length) { currentPool = ByteUtils.allocateUnsafe(poolSize * 12); currentPoolOffset = 0; } @@ -75,7 +78,7 @@ export class ObjectId extends BSONValue { /** ObjectId buffer pool pointer @internal */ private pool: Uint8Array; /** Buffer pool offset @internal */ - private offset: number; + private offset?: number; /** ObjectId hexString cache @internal */ private __id?: string; @@ -151,35 +154,44 @@ export class ObjectId extends BSONValue { workingId = inputId; } - const [pool, offset] = getPool(); - - // The following cases use workingId to construct an ObjectId - if (workingId == null || typeof workingId === 'number') { - // The most common use case (blank id, new objectId instance) - // Generate a new id - ObjectId.generate(typeof workingId === 'number' ? workingId : undefined, pool, offset); - } else if (ArrayBuffer.isView(workingId)) { - if (workingId.byteLength === 12) { - inputIndex = 0; - } else if ( - typeof inputIndex !== 'number' || - inputIndex < 0 || - workingId.byteLength < inputIndex + 12 || - isNaN(inputIndex) - ) { - throw new BSONError('Buffer length must be 12 or a valid offset must be specified'); - } - for (let i = 0; i < 12; i++) pool[offset + i] = workingId[inputIndex + i]; - } else if (typeof workingId === 'string') { - if (workingId.length === 24 && checkForHexRegExp.test(workingId)) { - pool.set(ByteUtils.fromHex(workingId), offset); + let pool: Uint8Array; + let offset: number; + + // Special case when poolSize === 1 and a 12 byte buffer is passed in - just persist buffer + if (poolSize === 1 && ArrayBuffer.isView(workingId) && workingId.length === 12) { + pool = ByteUtils.toLocalBufferType(workingId); + offset = 0; + } else { + [pool, offset] = getPool(); + + // The following cases use workingId to construct an ObjectId + if (workingId == null || typeof workingId === 'number') { + // The most common use case (blank id, new objectId instance) + // Generate a new id + ObjectId.generate(typeof workingId === 'number' ? workingId : undefined, pool, offset); + } else if (ArrayBuffer.isView(workingId)) { + if (workingId.length === 12) { + inputIndex = 0; + } else if ( + typeof inputIndex !== 'number' || + inputIndex < 0 || + workingId.length < inputIndex + 12 || + isNaN(inputIndex) + ) { + throw new BSONError('Buffer length must be 12 or a valid offset must be specified'); + } + for (let i = 0; i < 12; i++) pool[offset + i] = workingId[inputIndex + i]; + } else if (typeof workingId === 'string') { + if (workingId.length === 24 && checkForHexRegExp.test(workingId)) { + pool.set(ByteUtils.fromHex(workingId), offset); + } else { + throw new BSONError( + 'input must be a 24 character hex string, 12 byte Uint8Array, or an integer' + ); + } } else { - throw new BSONError( - 'input must be a 24 character hex string, 12 byte Uint8Array, or an integer' - ); + throw new BSONError('Argument passed in does not match the accepted types'); } - } else { - throw new BSONError('Argument passed in does not match the accepted types'); } // If we are caching the hex string if (ObjectId.cacheHexString) { @@ -187,7 +199,10 @@ export class ObjectId extends BSONValue { } // Increment pool offset once we have completed initialization this.pool = pool; - this.offset = offset; + // Only set offset if pool is used + if (poolSize > 1) { + this.offset = offset; + } incrementPool(); } @@ -201,6 +216,7 @@ export class ObjectId extends BSONValue { * @readonly */ get id(): Uint8Array { + if (this.offset === undefined) return this.pool; return this.pool.subarray(this.offset, this.offset + 12); } @@ -219,8 +235,9 @@ export class ObjectId extends BSONValue { if (ObjectId.cacheHexString && this.__id) { return this.__id; } + const start = this.offset ?? 0; - const hexString = ByteUtils.toHex(this.pool, this.offset, this.offset + 12); + const hexString = ByteUtils.toHex(this.pool, start, start + 12); if (ObjectId.cacheHexString && !this.__id) { this.__id = hexString; @@ -329,16 +346,20 @@ export class ObjectId extends BSONValue { } if (ObjectId.is(otherId)) { - if (otherId.pool && typeof otherId.offset === 'number') { + if (otherId.pool) { for (let i = 11; i >= 0; i--) { - if (this.pool[this.offset + i] !== otherId.pool[otherId.offset + i]) { + const offset = this.offset ?? 0; + const otherOffset = otherId.offset ?? 0; + if (this.pool[offset + i] !== otherId.pool[otherOffset + i]) { return false; } } return true; } // If otherId does not have pool and offset, fallback to buffer comparison for compatibility - return ByteUtils.equals(this.buffer, otherId.buffer); + return ( + this.buffer[11] === otherId.buffer[11] && ByteUtils.equals(this.buffer, otherId.buffer) + ); } if (typeof otherId === 'string') { @@ -357,7 +378,7 @@ export class ObjectId extends BSONValue { /** Returns the generation date (accurate up to the second) that this ID was generated. */ getTimestamp(): Date { const timestamp = new Date(); - const time = NumberUtils.getUint32BE(this.pool, this.offset); + const time = NumberUtils.getUint32BE(this.pool, this.offset ?? 0); timestamp.setTime(Math.floor(time) * 1000); return timestamp; } @@ -370,7 +391,7 @@ export class ObjectId extends BSONValue { /** @internal */ serializeInto(uint8array: Uint8Array, index: number): 12 { const pool = this.pool; - const offset = this.offset; + const offset = this.offset ?? 0; uint8array[index] = pool[offset]; uint8array[index + 1] = pool[offset + 1]; uint8array[index + 2] = pool[offset + 2]; diff --git a/test/node/object_id.test.ts b/test/node/object_id.test.ts index 6fb93368..f9f1b529 100644 --- a/test/node/object_id.test.ts +++ b/test/node/object_id.test.ts @@ -259,13 +259,18 @@ describe('ObjectId', function () { }); it('should correctly use buffer pool for ObjectId creation', function () { + const oldPoolSize = ObjectId.poolSize; + ObjectId.poolSize = 2; const obj = new ObjectId(); const obj2 = new ObjectId(); + expect(obj.offset).to.equal(0); + expect(obj2.offset).to.equal(12); expect(obj.offset).to.not.equal(obj2.offset); expect(obj.pool).to.equal(obj2.pool); expect(obj.id).to.not.equal(obj2.id); + ObjectId.poolSize = oldPoolSize; }); it('should respect buffer pool size for ObjectId creation', function () { @@ -307,9 +312,9 @@ describe('ObjectId', function () { const obj2 = new ObjectId(); const obj3 = new ObjectId(); - expect(obj.offset).to.equal(0); - expect(obj2.offset).to.equal(0); - expect(obj3.offset).to.equal(0); + expect(obj.offset).to.equal(undefined); + expect(obj2.offset).to.equal(undefined); + expect(obj3.offset).to.equal(undefined); expect(obj.pool).to.not.equal(obj2.pool); expect(obj2.pool).to.not.equal(obj3.pool); @@ -534,7 +539,28 @@ describe('ObjectId', function () { let equalId = { _bsontype: 'ObjectId', [oidKId]: oid.id }; const propAccessRecord: string[] = []; + equalId = new Proxy(equalId, { + get(target, prop: string, recv) { + if (prop !== '_bsontype') { + propAccessRecord.push(prop); + } + return Reflect.get(target, prop, recv); + } + }); + expect(oid.equals(equalId)).to.be.true; + // once for the 11th byte shortcut + // once for the total equality + expect(propAccessRecord).to.deep.equal(['pool', oidKId, oidKId]); + }); + + it('should use otherId[kId] Pool for equality when otherId has _bsontype === ObjectId when using pool', () => { + const oldPoolSize = ObjectId.poolSize; + ObjectId.poolSize = 2; + const oid = new ObjectId(oidString); + let equalId = new ObjectId(oidString); + + const propAccessRecord: string[] = []; equalId = new Proxy(equalId, { get(target, prop: string, recv) { if (prop !== '_bsontype') { @@ -547,7 +573,8 @@ describe('ObjectId', function () { expect(oid.equals(equalId)).to.be.true; // once for the 11th byte shortcut // once for the total equality - expect(propAccessRecord).to.deep.equal(['pool', oidKId]); + expect(propAccessRecord).to.contain('pool').contain('offset'); + ObjectId.poolSize = oldPoolSize; }); });