From 34c727c13af8380e567015031ec271817923ebeb Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Tue, 26 Oct 2021 13:33:42 -0400 Subject: [PATCH 01/17] refactor(NODE-3662): error checking to make sure that ObjectId results in object with correct properties --- src/objectid.ts | 66 +++++++++++++++++------------------- test/node/object_id_tests.js | 42 ++++++++++++++++++++++- 2 files changed, 72 insertions(+), 36 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index a13165e9..d630ef16 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -52,48 +52,44 @@ export class ObjectId { if (id instanceof ObjectId) { this[kId] = id.id; this.__id = id.__id; - } - - if (typeof id === 'object' && id && 'id' in id) { - if ('toHexString' in id && typeof id.toHexString === 'function') { - this[kId] = Buffer.from(id.toHexString(), 'hex'); + } else { + if (typeof id === 'object' && id && 'id' in id) { + if ('toHexString' in id && typeof id.toHexString === 'function') { + this[kId] = Buffer.from(id.toHexString(), 'hex'); + } else { + throw new TypeError( + 'Object passed in does not have toHexString() function' + ); + } + } else if (id == null || typeof id === 'number') { + // The most common use case (blank id, new objectId instance) + // Generate a new id + this[kId] = ObjectId.generate(typeof id === 'number' ? id : undefined); + } else if (ArrayBuffer.isView(id) && id.byteLength === 12) { + this[kId] = ensureBuffer(id); + } else if (typeof id === 'string') { + if (id.length === 12) { + const bytes = Buffer.from(id); + if (bytes.byteLength === 12) { + this[kId] = bytes; + } + } else if (id.length === 24 && checkForHexRegExp.test(id)) { + this[kId] = Buffer.from(id, 'hex'); + } else { + throw new BSONTypeError( + 'Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters' + ); + } } else { - this[kId] = typeof id.id === 'string' ? Buffer.from(id.id) : id.id; + throw new TypeError( + 'Argument passed in does not match the accepted types' + ); } - } - - // The most common use case (blank id, new objectId instance) - if (id == null || typeof id === 'number') { - // Generate a new id - this[kId] = ObjectId.generate(typeof id === 'number' ? id : undefined); // If we are caching the hex string if (ObjectId.cacheHexString) { this.__id = this.id.toString('hex'); } } - - if (ArrayBuffer.isView(id) && id.byteLength === 12) { - this[kId] = ensureBuffer(id); - } - - if (typeof id === 'string') { - if (id.length === 12) { - const bytes = Buffer.from(id); - if (bytes.byteLength === 12) { - this[kId] = bytes; - } - } else if (id.length === 24 && checkForHexRegExp.test(id)) { - this[kId] = Buffer.from(id, 'hex'); - } else { - throw new BSONTypeError( - 'Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters' - ); - } - } - - if (ObjectId.cacheHexString) { - this.__id = this.id.toString('hex'); - } } /** diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index 601768c0..b23fb45e 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -5,10 +5,11 @@ const BSON = require('../register-bson'); const util = require('util'); const ObjectId = BSON.ObjectId; -describe('ObjectId', function () { +describe.only('ObjectId', function () { /** * @ignore */ + it('should correctly handle objectId timestamps', function (done) { // var test_number = {id: ObjectI()}; var a = ObjectId.createFromTime(1); @@ -24,6 +25,45 @@ describe('ObjectId', function () { done(); }); + it('should throw error if empty array is passed in', function () { + expect(() => new ObjectId([])).to.throw(TypeError); + }); + + it('should throw error if nonempty array is passed in', function () { + expect(() => new ObjectId(['abcdefŽhijkl'])).to.throw(TypeError); + }); + + it('should throw error if empty object is passed in', function () { + expect(() => new ObjectId({})).to.throw(TypeError); + }); + + it('should throw error if object without an id property is passed in', function () { + var tmp = new ObjectId(); + var objectIdLike = { + toHexString: function () { + return tmp.toHexString(); + } + }; + + expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); + }); + + it('should throw error if object without toHexString function is passed in', function () { + var tmp = new ObjectId(); + var objectIdLike = { + id: tmp.id + }; + objectIdLike.id.toHexString = null; + + expect(() => new ObjectId(objectIdLike).toHexString()).to.throw(TypeError); + }); + + it('should correctly create ObjectId from number', function () { + expect(() => new ObjectId(42)).to.not.throw(); + expect(() => new ObjectId(0x2a)).to.not.throw(); + expect(() => new ObjectId(NaN)).to.not.throw(); + }); + /** * @ignore */ From fd0a5a480385a6404c523336c767a900538e9548 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Tue, 26 Oct 2021 15:28:54 -0400 Subject: [PATCH 02/17] test: remove .only and add back constructor logic --- src/objectid.ts | 4 +--- test/node/object_id_tests.js | 12 +----------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index d630ef16..10a34683 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -57,9 +57,7 @@ export class ObjectId { if ('toHexString' in id && typeof id.toHexString === 'function') { this[kId] = Buffer.from(id.toHexString(), 'hex'); } else { - throw new TypeError( - 'Object passed in does not have toHexString() function' - ); + this[kId] = typeof id.id === 'string' ? Buffer.from(id.id) : id.id; } } else if (id == null || typeof id === 'number') { // The most common use case (blank id, new objectId instance) diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index b23fb45e..f6d4e057 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -5,7 +5,7 @@ const BSON = require('../register-bson'); const util = require('util'); const ObjectId = BSON.ObjectId; -describe.only('ObjectId', function () { +describe('ObjectId', function () { /** * @ignore */ @@ -48,16 +48,6 @@ describe.only('ObjectId', function () { expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); }); - it('should throw error if object without toHexString function is passed in', function () { - var tmp = new ObjectId(); - var objectIdLike = { - id: tmp.id - }; - objectIdLike.id.toHexString = null; - - expect(() => new ObjectId(objectIdLike).toHexString()).to.throw(TypeError); - }); - it('should correctly create ObjectId from number', function () { expect(() => new ObjectId(42)).to.not.throw(); expect(() => new ObjectId(0x2a)).to.not.throw(); From ed748e176f6dc69b1478bb69758ac06ab8aafa5a Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Tue, 26 Oct 2021 16:11:28 -0400 Subject: [PATCH 03/17] refactor: validate that id satisfies the ObjectIdLike type def of id and add test for invalid id type --- src/objectid.ts | 8 +++++++- test/node/object_id_tests.js | 8 ++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/objectid.ts b/src/objectid.ts index 10a34683..10c3efec 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -56,8 +56,14 @@ export class ObjectId { if (typeof id === 'object' && id && 'id' in id) { if ('toHexString' in id && typeof id.toHexString === 'function') { this[kId] = Buffer.from(id.toHexString(), 'hex'); + } else if (typeof id.id === 'string') { + this[kId] = Buffer.from(id.id); } else { - this[kId] = typeof id.id === 'string' ? Buffer.from(id.id) : id.id; + try { + this[kId] = ensureBuffer(id.id); + } catch { + throw new BSONTypeError('Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters'); + } } } else if (id == null || typeof id === 'number') { // The most common use case (blank id, new objectId instance) diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index f6d4e057..bb5cdd94 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -48,6 +48,14 @@ describe('ObjectId', function () { expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); }); + it('should throw error if object without string or buffer id is passed in', function () { + var objectIdLike = { + id: 5 + }; + + expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); + }); + it('should correctly create ObjectId from number', function () { expect(() => new ObjectId(42)).to.not.throw(); expect(() => new ObjectId(0x2a)).to.not.throw(); From b44a687fa2824ced6837f5b9a07891abd4b61c72 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Tue, 26 Oct 2021 16:26:11 -0400 Subject: [PATCH 04/17] chore: change wording of error message --- src/objectid.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objectid.ts b/src/objectid.ts index 10c3efec..dfeb3088 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -62,7 +62,7 @@ export class ObjectId { try { this[kId] = ensureBuffer(id.id); } catch { - throw new BSONTypeError('Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters'); + throw new BSONTypeError('Argument passed in must have an id that is of type string or Buffer'); } } } else if (id == null || typeof id === 'number') { From e2cdd0f0f97007dad40b18205fdf4ad93a91008e Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Tue, 26 Oct 2021 22:36:31 -0400 Subject: [PATCH 05/17] test: add additional tests for acceptable and invalid inputs for constructor to test for errors --- src/objectid.ts | 8 ++--- test/node/object_id_tests.js | 62 +++++++++++++++++++++++++++++++++--- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index dfeb3088..198b73ea 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -62,7 +62,9 @@ export class ObjectId { try { this[kId] = ensureBuffer(id.id); } catch { - throw new BSONTypeError('Argument passed in must have an id that is of type string or Buffer'); + throw new BSONTypeError( + 'Argument passed in must have an id that is of type string or Buffer' + ); } } } else if (id == null || typeof id === 'number') { @@ -85,9 +87,7 @@ export class ObjectId { ); } } else { - throw new TypeError( - 'Argument passed in does not match the accepted types' - ); + throw new BSONTypeError('Argument passed in does not match the accepted types'); } // If we are caching the hex string if (ObjectId.cacheHexString) { diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index bb5cdd94..8125a98a 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -9,7 +9,7 @@ describe('ObjectId', function () { /** * @ignore */ - + it('should correctly handle objectId timestamps', function (done) { // var test_number = {id: ObjectI()}; var a = ObjectId.createFromTime(1); @@ -25,6 +25,11 @@ describe('ObjectId', function () { done(); }); + it('should correctly create ObjectId from ObjectId', function () { + var tmp = new ObjectId(); + expect(() => new ObjectId(tmp)).to.not.throw(); + }); + it('should throw error if empty array is passed in', function () { expect(() => new ObjectId([])).to.throw(TypeError); }); @@ -48,18 +53,65 @@ describe('ObjectId', function () { expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); }); - it('should throw error if object without string or buffer id is passed in', function () { - var objectIdLike = { + it('should throw error if object with non-Buffer non-string id is passed in', function () { + var objectNumId = { id: 5 }; + var objectNullId = { + id: null + }; - expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); + expect(() => new ObjectId(objectNumId)).to.throw(TypeError); + expect(() => new ObjectId(objectNullId)).to.throw(TypeError); + }); + + it('should correctly create ObjectId with objectIdLike properties', function () { + var tmp = new ObjectId(); + var objectIdLike = { + id: tmp.id, + toHexString: function () { + return tmp.toHexString(); + } + }; + + expect(() => new ObjectId(objectIdLike)).to.not.throw(TypeError); }); - it('should correctly create ObjectId from number', function () { + it('should correctly create ObjectId from number or null', function () { expect(() => new ObjectId(42)).to.not.throw(); expect(() => new ObjectId(0x2a)).to.not.throw(); expect(() => new ObjectId(NaN)).to.not.throw(); + expect(() => new ObjectId(null)).to.not.throw(); + }); + + it('should correctly create ObjectId with Buffer or string id', function () { + var objectStringId = { + id: 'thisisastringid' + }; + var objectBufferId = { + id: Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex') + }; + var objectBufferFromArray = { + id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]) + }; + + expect(() => new ObjectId(objectStringId)).to.not.throw(TypeError); + expect(() => new ObjectId(objectBufferId)).to.not.throw(TypeError); + expect(() => new ObjectId(objectBufferFromArray)).to.not.throw(TypeError); + }); + + it('should throw error if non-12 byte non-24 hex string passed in', function () { + expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(); + expect(() => new ObjectId('thisismorethan12chars')).to.throw(); + expect(() => new ObjectId('101010')).to.throw(); + expect(() => new ObjectId('')).to.throw(); + }); + + it('should correctly create ObjectId from 12 byte or 24 hex string', function () { + expect(() => new ObjectId('AAAAAAAAAAAAAAAAAAAAAAAA')).to.not.throw(); + expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFF')).to.not.throw(); + expect(() => new ObjectId('111111111111')).to.not.throw(); + expect(() => new ObjectId('abcdefghijkl')).to.not.throw(); }); /** From 0d943b7dfdcea52b0fee7fd468ff43e3e72f38c4 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Wed, 27 Oct 2021 13:39:59 -0400 Subject: [PATCH 06/17] test: add toHexString() to correct tests to check for valid id property --- test/node/object_id_tests.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index 8125a98a..e9ff66c9 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -27,7 +27,7 @@ describe('ObjectId', function () { it('should correctly create ObjectId from ObjectId', function () { var tmp = new ObjectId(); - expect(() => new ObjectId(tmp)).to.not.throw(); + expect(() => new ObjectId(tmp).toHexString()).to.not.throw(); }); it('should throw error if empty array is passed in', function () { @@ -74,14 +74,14 @@ describe('ObjectId', function () { } }; - expect(() => new ObjectId(objectIdLike)).to.not.throw(TypeError); + expect(() => new ObjectId(objectIdLike).toHexString()).to.not.throw(TypeError); }); it('should correctly create ObjectId from number or null', function () { - expect(() => new ObjectId(42)).to.not.throw(); - expect(() => new ObjectId(0x2a)).to.not.throw(); - expect(() => new ObjectId(NaN)).to.not.throw(); - expect(() => new ObjectId(null)).to.not.throw(); + expect(() => new ObjectId(42).toHexString()).to.not.throw(); + expect(() => new ObjectId(0x2a).toHexString()).to.not.throw(); + expect(() => new ObjectId(NaN).toHexString()).to.not.throw(); + expect(() => new ObjectId(null).toHexString()).to.not.throw(); }); it('should correctly create ObjectId with Buffer or string id', function () { @@ -95,9 +95,9 @@ describe('ObjectId', function () { id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]) }; - expect(() => new ObjectId(objectStringId)).to.not.throw(TypeError); - expect(() => new ObjectId(objectBufferId)).to.not.throw(TypeError); - expect(() => new ObjectId(objectBufferFromArray)).to.not.throw(TypeError); + expect(() => new ObjectId(objectStringId).toHexString()).to.not.throw(TypeError); + expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); + expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); }); it('should throw error if non-12 byte non-24 hex string passed in', function () { @@ -108,10 +108,10 @@ describe('ObjectId', function () { }); it('should correctly create ObjectId from 12 byte or 24 hex string', function () { - expect(() => new ObjectId('AAAAAAAAAAAAAAAAAAAAAAAA')).to.not.throw(); - expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFF')).to.not.throw(); - expect(() => new ObjectId('111111111111')).to.not.throw(); - expect(() => new ObjectId('abcdefghijkl')).to.not.throw(); + expect(() => new ObjectId('AAAAAAAAAAAAAAAAAAAAAAAA').toHexString()).to.not.throw(); + expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFF').toHexString()).to.not.throw(); + expect(() => new ObjectId('111111111111').toHexString()).to.not.throw(); + expect(() => new ObjectId('abcdefghijkl').toHexString()).to.not.throw(); }); /** From fd67de375bbb7b81905f4d06ee263d5a17ea92b3 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Wed, 27 Oct 2021 14:28:21 -0400 Subject: [PATCH 07/17] refactor: rename id parameter to inputId and separating assertion to its own test --- src/objectid.ts | 42 ++++++++++++++++++------------------ test/node/object_id_tests.js | 7 +++++- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index 198b73ea..cefbf6de 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -43,44 +43,44 @@ export class ObjectId { /** * Create an ObjectId type * - * @param id - Can be a 24 character hex string, 12 byte binary Buffer, or a number. + * @param inputId - Can be a 24 character hex string, 12 byte binary Buffer, or a number. */ - constructor(id?: string | Buffer | number | ObjectIdLike | ObjectId) { - if (!(this instanceof ObjectId)) return new ObjectId(id); + constructor(inputId?: string | Buffer | number | ObjectIdLike | ObjectId) { + if (!(this instanceof ObjectId)) return new ObjectId(inputId); // Duck-typing to support ObjectId from different npm packages - if (id instanceof ObjectId) { - this[kId] = id.id; - this.__id = id.__id; + if (inputId instanceof ObjectId) { + this[kId] = inputId.id; + this.__id = inputId.__id; } else { - if (typeof id === 'object' && id && 'id' in id) { - if ('toHexString' in id && typeof id.toHexString === 'function') { - this[kId] = Buffer.from(id.toHexString(), 'hex'); - } else if (typeof id.id === 'string') { - this[kId] = Buffer.from(id.id); + if (typeof inputId === 'object' && inputId && 'id' in inputId) { + if ('toHexString' in inputId && typeof inputId.toHexString === 'function') { + this[kId] = Buffer.from(inputId.toHexString(), 'hex'); + } else if (typeof inputId.id === 'string') { + this[kId] = Buffer.from(inputId.id); } else { try { - this[kId] = ensureBuffer(id.id); + this[kId] = ensureBuffer(inputId.id); } catch { throw new BSONTypeError( 'Argument passed in must have an id that is of type string or Buffer' ); } } - } else if (id == null || typeof id === 'number') { + } else if (inputId == null || typeof inputId === 'number') { // The most common use case (blank id, new objectId instance) // Generate a new id - this[kId] = ObjectId.generate(typeof id === 'number' ? id : undefined); - } else if (ArrayBuffer.isView(id) && id.byteLength === 12) { - this[kId] = ensureBuffer(id); - } else if (typeof id === 'string') { - if (id.length === 12) { - const bytes = Buffer.from(id); + this[kId] = ObjectId.generate(typeof inputId === 'number' ? inputId : undefined); + } else if (ArrayBuffer.isView(inputId) && inputId.byteLength === 12) { + this[kId] = ensureBuffer(inputId); + } else if (typeof inputId === 'string') { + if (inputId.length === 12) { + const bytes = Buffer.from(inputId); if (bytes.byteLength === 12) { this[kId] = bytes; } - } else if (id.length === 24 && checkForHexRegExp.test(id)) { - this[kId] = Buffer.from(id, 'hex'); + } else if (inputId.length === 24 && checkForHexRegExp.test(inputId)) { + this[kId] = Buffer.from(inputId, 'hex'); } else { throw new BSONTypeError( 'Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters' diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index e9ff66c9..69377d79 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -110,10 +110,15 @@ describe('ObjectId', function () { it('should correctly create ObjectId from 12 byte or 24 hex string', function () { expect(() => new ObjectId('AAAAAAAAAAAAAAAAAAAAAAAA').toHexString()).to.not.throw(); expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFF').toHexString()).to.not.throw(); - expect(() => new ObjectId('111111111111').toHexString()).to.not.throw(); expect(() => new ObjectId('abcdefghijkl').toHexString()).to.not.throw(); }); + it('should correctly create ObjectId from 12 byte sequence', function () { + var a = '111111111111'; + expect(() => new ObjectId(a).toHexString()).to.not.throw(); + expect(Buffer.from(new ObjectId(a).id).equals(Buffer.from('111111111111', 'latin1'))); + }); + /** * @ignore */ From e49868219fc6e328a1a8d76687242ea06effe8db Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Fri, 29 Oct 2021 10:27:27 -0400 Subject: [PATCH 08/17] refactor: add new test cases and address PR comments --- src/objectid.ts | 87 ++++++++++++++++++++---------------- test/node/object_id_tests.js | 83 ++++++++++++++++++++++++++-------- 2 files changed, 114 insertions(+), 56 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index cefbf6de..745429da 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -48,52 +48,63 @@ export class ObjectId { constructor(inputId?: string | Buffer | number | ObjectIdLike | ObjectId) { if (!(this instanceof ObjectId)) return new ObjectId(inputId); - // Duck-typing to support ObjectId from different npm packages - if (inputId instanceof ObjectId) { - this[kId] = inputId.id; - this.__id = inputId.__id; - } else { - if (typeof inputId === 'object' && inputId && 'id' in inputId) { - if ('toHexString' in inputId && typeof inputId.toHexString === 'function') { - this[kId] = Buffer.from(inputId.toHexString(), 'hex'); - } else if (typeof inputId.id === 'string') { - this[kId] = Buffer.from(inputId.id); - } else { - try { - this[kId] = ensureBuffer(inputId.id); - } catch { - throw new BSONTypeError( - 'Argument passed in must have an id that is of type string or Buffer' - ); - } - } - } else if (inputId == null || typeof inputId === 'number') { - // The most common use case (blank id, new objectId instance) - // Generate a new id - this[kId] = ObjectId.generate(typeof inputId === 'number' ? inputId : undefined); - } else if (ArrayBuffer.isView(inputId) && inputId.byteLength === 12) { - this[kId] = ensureBuffer(inputId); - } else if (typeof inputId === 'string') { - if (inputId.length === 12) { - const bytes = Buffer.from(inputId); - if (bytes.byteLength === 12) { - this[kId] = bytes; - } - } else if (inputId.length === 24 && checkForHexRegExp.test(inputId)) { - this[kId] = Buffer.from(inputId, 'hex'); - } else { + let workingId; + if (typeof inputId === 'object' && inputId && 'id' in inputId) { + if (typeof inputId.id !== 'string') { + try { + workingId = ensureBuffer(inputId.id); + } catch { throw new BSONTypeError( - 'Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters' + 'Argument passed in must have an id that is of type string or Buffer' ); } + } else { + workingId = Buffer.from(inputId.id); + this.checkValidString(inputId.id); + } + if ('toHexString' in inputId && typeof inputId.toHexString === 'function') { + workingId = Buffer.from(inputId.toHexString(), 'hex'); + } + this[kId] = workingId; + } else { + workingId = inputId; + if (workingId == null || typeof workingId === 'number') { + // The most common use case (blank id, new objectId instance) + // Generate a new id + this[kId] = ObjectId.generate(typeof workingId === 'number' ? workingId : undefined); + } else if (ArrayBuffer.isView(workingId) && workingId.byteLength === 12) { + this[kId] = ensureBuffer(workingId); + } else if (typeof workingId === 'string') { + this.checkValidString(workingId); } else { throw new BSONTypeError('Argument passed in does not match the accepted types'); } - // If we are caching the hex string - if (ObjectId.cacheHexString) { - this.__id = this.id.toString('hex'); + } + // If we are caching the hex string + if (ObjectId.cacheHexString) { + this.__id = this.id.toString('hex'); + } + } + + /** + * Returns Buffer for valid string of 12 bytes or string of 24 hex characters, otherwise throws BSONTypeError + * + * @param str - pass in a string to validate + */ + checkValidString(str: string): Buffer { + if (str.length === 12) { + const bytes = Buffer.from(str); + if (bytes.byteLength === 12) { + this[kId] = bytes; } + } else if (str.length === 24 && checkForHexRegExp.test(str)) { + this[kId] = Buffer.from(str, 'hex'); + } else { + throw new BSONTypeError( + 'Argument passed in must be a string of 12 bytes or a string of 24 hex characters' + ); } + return Buffer.from(str); } /** diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index 69377d79..ce95fd90 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -5,7 +5,7 @@ const BSON = require('../register-bson'); const util = require('util'); const ObjectId = BSON.ObjectId; -describe('ObjectId', function () { +describe.only('ObjectId', function () { /** * @ignore */ @@ -53,6 +53,49 @@ describe('ObjectId', function () { expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); }); + it('should correctly create ObjectId from object with Buffer or string id', function () { + var objectStringId = { + id: 'aaaaaaaaaaaaaaaaaaaaaaaa' + }; + var objectBufferId = { + id: Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex') + }; + var objectBufferFromArray = { + id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]) + }; + + expect(() => new ObjectId(objectStringId).toHexString()).to.not.throw(TypeError); + expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); + expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); + }); + + it('should correctly create ObjectId from object with Buffer or string id and override toHexString', function () { + function newToHexString() { + return 'BBBBBBBBBBBBBBBBBBBBBBBB'; + } + var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + var objectStringId = { + id: 'aaaaaaaaaaaaaaaaaaaaaaaa', + toHexString: newToHexString + }; + var objectBufferId = { + id: Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'), + toHexString: newToHexString + }; + var objectBufferFromArray = { + id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]), + toHexString: newToHexString + }; + expect(() => new ObjectId(objectStringId).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectStringId).id).equals(buf)); + + expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectBufferId).id).equals(buf)); + + expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectBufferFromArray).id).equals(buf)); + }); + it('should throw error if object with non-Buffer non-string id is passed in', function () { var objectNumId = { id: 5 @@ -65,7 +108,21 @@ describe('ObjectId', function () { expect(() => new ObjectId(objectNullId)).to.throw(TypeError); }); - it('should correctly create ObjectId with objectIdLike properties', function () { + it('should throw an error if object with invalid string id is passed in', function () { + var objectInvalidString = { + id: 'FFFFFFFFFFFFFFFFFFFFFFFG' + }; + expect(() => new ObjectId(objectInvalidString)).to.throw(TypeError); + }); + + it('should throw an error if object with invalid Buffer id is passed in', function () { + var objectInvalidBuffer = { + id: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13] + }; + expect(() => new ObjectId(objectInvalidBuffer)).to.throw(TypeError); + }); + + it('should correctly create ObjectId from object with objectIdLike properties', function () { var tmp = new ObjectId(); var objectIdLike = { id: tmp.id, @@ -84,22 +141,6 @@ describe('ObjectId', function () { expect(() => new ObjectId(null).toHexString()).to.not.throw(); }); - it('should correctly create ObjectId with Buffer or string id', function () { - var objectStringId = { - id: 'thisisastringid' - }; - var objectBufferId = { - id: Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex') - }; - var objectBufferFromArray = { - id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]) - }; - - expect(() => new ObjectId(objectStringId).toHexString()).to.not.throw(TypeError); - expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); - expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); - }); - it('should throw error if non-12 byte non-24 hex string passed in', function () { expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(); expect(() => new ObjectId('thisismorethan12chars')).to.throw(); @@ -155,6 +196,12 @@ describe('ObjectId', function () { done(); }); + it('should throw an error if invalid Buffer passed in', function (done) { + var a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); + expect(() => new ObjectId(a)).to.throw(TypeError); + done(); + }); + /** * @ignore */ From 35e1d88271bf5173a743951c07b980997be7a0c8 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Fri, 29 Oct 2021 13:27:02 -0400 Subject: [PATCH 09/17] refactor: update logic, remove unnecessary helper, rearrange tests for clarity --- src/objectid.ts | 68 +++++++++-------------- test/node/object_id_tests.js | 102 ++++++++++++++++++++++++++--------- 2 files changed, 104 insertions(+), 66 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index 745429da..3898be85 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -50,35 +50,40 @@ export class ObjectId { let workingId; if (typeof inputId === 'object' && inputId && 'id' in inputId) { - if (typeof inputId.id !== 'string') { - try { - workingId = ensureBuffer(inputId.id); - } catch { - throw new BSONTypeError( - 'Argument passed in must have an id that is of type string or Buffer' - ); - } - } else { - workingId = Buffer.from(inputId.id); - this.checkValidString(inputId.id); + if (typeof inputId.id !== 'string' && !ArrayBuffer.isView(inputId.id)) { + throw new BSONTypeError( + 'Argument passed in must have an id that is of type string or Buffer' + ); } if ('toHexString' in inputId && typeof inputId.toHexString === 'function') { workingId = Buffer.from(inputId.toHexString(), 'hex'); + } else { + workingId = inputId.id; } - this[kId] = workingId; } else { workingId = inputId; - if (workingId == null || typeof workingId === 'number') { - // The most common use case (blank id, new objectId instance) - // Generate a new id - this[kId] = ObjectId.generate(typeof workingId === 'number' ? workingId : undefined); - } else if (ArrayBuffer.isView(workingId) && workingId.byteLength === 12) { - this[kId] = ensureBuffer(workingId); - } else if (typeof workingId === 'string') { - this.checkValidString(workingId); + } + if (workingId == null || typeof workingId === 'number') { + // The most common use case (blank id, new objectId instance) + // Generate a new id + this[kId] = ObjectId.generate(typeof workingId === 'number' ? workingId : undefined); + } else if (ArrayBuffer.isView(workingId) && workingId.byteLength === 12) { + this[kId] = ensureBuffer(workingId); + } else if (typeof workingId === 'string') { + if (workingId.length === 12) { + const bytes = Buffer.from(workingId); + if (bytes.byteLength === 12) { + this[kId] = bytes; + } + } else if (workingId.length === 24 && checkForHexRegExp.test(workingId)) { + this[kId] = Buffer.from(workingId, 'hex'); } else { - throw new BSONTypeError('Argument passed in does not match the accepted types'); + throw new BSONTypeError( + 'Argument passed in must be a string of 12 bytes or a string of 24 hex characters' + ); } + } else { + throw new BSONTypeError('Argument passed in does not match the accepted types'); } // If we are caching the hex string if (ObjectId.cacheHexString) { @@ -86,27 +91,6 @@ export class ObjectId { } } - /** - * Returns Buffer for valid string of 12 bytes or string of 24 hex characters, otherwise throws BSONTypeError - * - * @param str - pass in a string to validate - */ - checkValidString(str: string): Buffer { - if (str.length === 12) { - const bytes = Buffer.from(str); - if (bytes.byteLength === 12) { - this[kId] = bytes; - } - } else if (str.length === 24 && checkForHexRegExp.test(str)) { - this[kId] = Buffer.from(str, 'hex'); - } else { - throw new BSONTypeError( - 'Argument passed in must be a string of 12 bytes or a string of 24 hex characters' - ); - } - return Buffer.from(str); - } - /** * The ObjectId bytes * @readonly diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index ce95fd90..d896f31a 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -27,7 +27,7 @@ describe.only('ObjectId', function () { it('should correctly create ObjectId from ObjectId', function () { var tmp = new ObjectId(); - expect(() => new ObjectId(tmp).toHexString()).to.not.throw(); + expect(() => new ObjectId(tmp)).to.not.throw(); }); it('should throw error if empty array is passed in', function () { @@ -53,47 +53,75 @@ describe.only('ObjectId', function () { expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); }); - it('should correctly create ObjectId from object with Buffer or string id', function () { - var objectStringId = { + it('should correctly create ObjectId from object with valid string id', function () { + var objectValidStringId1 = { id: 'aaaaaaaaaaaaaaaaaaaaaaaa' }; + var objectValidStringId2 = { + id: 'abcdefghijkl' + }; + var buf1 = Buffer.from('aaaaaaaaaaaaaaaaaaaaaaaa', 'hex'); + var buf2 = Buffer.from('abcdefghijkl', 'hex'); + expect(() => new ObjectId(objectValidStringId1).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectValidStringId1).id).equals(buf1)); + expect(() => new ObjectId(objectValidStringId2).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectValidStringId2).id).equals(buf2)); + }); + + it('should correctly create ObjectId from object with valid string id and toHexString method', function () { + function newToHexString() { + return 'BBBBBBBBBBBBBBBBBBBBBBBB'; + } + var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + var objectValidStringId1 = { + id: 'aaaaaaaaaaaaaaaaaaaaaaaa', + toHexString: newToHexString + }; + var objectValidStringId2 = { + id: 'abcdefghijkl', + toHexString: newToHexString + }; + expect(() => new ObjectId(objectValidStringId1).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectValidStringId1).id).equals(buf)); + expect(() => new ObjectId(objectValidStringId2).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectValidStringId2).id).equals(buf)); + }); + + it('should correctly create ObjectId from object with valid Buffer id', function () { + var validBuffer1 = Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'); + var validBuffer2 = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); var objectBufferId = { - id: Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex') + id: validBuffer1 }; var objectBufferFromArray = { - id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]) + id: validBuffer2 }; - - expect(() => new ObjectId(objectStringId).toHexString()).to.not.throw(TypeError); expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectBufferId).id).equals(validBuffer1)); expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectBufferId).id).equals(validBuffer2)); }); - it('should correctly create ObjectId from object with Buffer or string id and override toHexString', function () { + it('should correctly create ObjectId from object with valid Buffer id and toHexString method', function () { + var validBuffer1 = Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'); + var validBuffer2 = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); + var bufferToHex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); function newToHexString() { return 'BBBBBBBBBBBBBBBBBBBBBBBB'; } - var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); - var objectStringId = { - id: 'aaaaaaaaaaaaaaaaaaaaaaaa', - toHexString: newToHexString - }; var objectBufferId = { - id: Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'), + id: validBuffer1, toHexString: newToHexString }; var objectBufferFromArray = { - id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]), + id: validBuffer2, toHexString: newToHexString }; - expect(() => new ObjectId(objectStringId).toHexString()).to.not.throw(TypeError); - expect(Buffer.from(new ObjectId(objectStringId).id).equals(buf)); - expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); - expect(Buffer.from(new ObjectId(objectBufferId).id).equals(buf)); + expect(Buffer.from(new ObjectId(objectBufferId).id).equals(bufferToHex)); expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); - expect(Buffer.from(new ObjectId(objectBufferFromArray).id).equals(buf)); + expect(Buffer.from(new ObjectId(objectBufferFromArray).id).equals(bufferToHex)); }); it('should throw error if object with non-Buffer non-string id is passed in', function () { @@ -103,7 +131,6 @@ describe.only('ObjectId', function () { var objectNullId = { id: null }; - expect(() => new ObjectId(objectNumId)).to.throw(TypeError); expect(() => new ObjectId(objectNullId)).to.throw(TypeError); }); @@ -115,13 +142,39 @@ describe.only('ObjectId', function () { expect(() => new ObjectId(objectInvalidString)).to.throw(TypeError); }); + it('should correctly create ObjectId from object with invalid string id and toHexString method', function () { + function newToHexString() { + return 'BBBBBBBBBBBBBBBBBBBBBBBB'; + } + var objectInvalidString = { + id: 'FFFFFFFFFFFFFFFFFFFFFFFG', + toHexString: newToHexString + }; + var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + expect(() => new ObjectId(objectInvalidString)).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectInvalidString).id).equals(buf)); + }); + it('should throw an error if object with invalid Buffer id is passed in', function () { var objectInvalidBuffer = { - id: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13] + id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]) }; expect(() => new ObjectId(objectInvalidBuffer)).to.throw(TypeError); }); + it('should correctly create ObjectId from object with invalid Buffer id and toHexString method', function () { + function newToHexString() { + return 'BBBBBBBBBBBBBBBBBBBBBBBB'; + } + var objectInvalidBuffer = { + id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]), + toHexString: newToHexString + }; + var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + expect(() => new ObjectId(objectInvalidBuffer)).to.not.throw(TypeError); + expect(Buffer.from(new ObjectId(objectInvalidBuffer).id).equals(buf)); + }); + it('should correctly create ObjectId from object with objectIdLike properties', function () { var tmp = new ObjectId(); var objectIdLike = { @@ -143,7 +196,8 @@ describe.only('ObjectId', function () { it('should throw error if non-12 byte non-24 hex string passed in', function () { expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(); - expect(() => new ObjectId('thisismorethan12chars')).to.throw(); + expect(() => new ObjectId('thisstringisdefinitelytoolong')).to.throw(); + expect(() => new ObjectId('tooshort')).to.throw(); expect(() => new ObjectId('101010')).to.throw(); expect(() => new ObjectId('')).to.throw(); }); @@ -181,7 +235,7 @@ describe.only('ObjectId', function () { /** * @ignore */ - it('should correctly create ObjectId from Buffer', function (done) { + it('should correctly create ObjectId from valid Buffer', function (done) { if (!Buffer.from) return done(); var a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; var b = new ObjectId(Buffer.from(a, 'hex')); From 3355447c1ae39399abe8064b340d0cd26c0d18eb Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Fri, 29 Oct 2021 16:45:34 -0400 Subject: [PATCH 10/17] chore: fix nits and add id validation for additional correct ObjectId tests --- src/objectid.ts | 1 + test/node/object_id_tests.js | 29 ++++++++++++++++++++--------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index 3898be85..ec09c79a 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -63,6 +63,7 @@ export class ObjectId { } else { workingId = inputId; } + // the following checks 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 diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index d896f31a..e09c4a40 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -5,7 +5,7 @@ const BSON = require('../register-bson'); const util = require('util'); const ObjectId = BSON.ObjectId; -describe.only('ObjectId', function () { +describe('ObjectId', function () { /** * @ignore */ @@ -28,6 +28,7 @@ describe.only('ObjectId', function () { it('should correctly create ObjectId from ObjectId', function () { var tmp = new ObjectId(); expect(() => new ObjectId(tmp)).to.not.throw(); + expect(() => new ObjectId(tmp).id.equals(Buffer.from(tmp, 'hex'))); }); it('should throw error if empty array is passed in', function () { @@ -119,7 +120,6 @@ describe.only('ObjectId', function () { }; expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectBufferId).id).equals(bufferToHex)); - expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectBufferFromArray).id).equals(bufferToHex)); }); @@ -183,15 +183,21 @@ describe.only('ObjectId', function () { return tmp.toHexString(); } }; - expect(() => new ObjectId(objectIdLike).toHexString()).to.not.throw(TypeError); + expect(() => new ObjectId(objectIdLike).id.equals(Buffer.from(tmp, 'hex'))); }); it('should correctly create ObjectId from number or null', function () { expect(() => new ObjectId(42).toHexString()).to.not.throw(); + expect(() => new ObjectId(42).id.equals(Buffer.from(42, 'hex'))); expect(() => new ObjectId(0x2a).toHexString()).to.not.throw(); + expect(() => new ObjectId(0x2a).id.equals(Buffer.from(0x2a, 'hex'))); + expect(() => new ObjectId(4.2).toHexString()).to.not.throw(); + expect(() => new ObjectId(4.2).id.equals(Buffer.from(4, 'hex'))); expect(() => new ObjectId(NaN).toHexString()).to.not.throw(); + expect(() => new ObjectId(NaN).id.equals(Buffer.from(NaN, 'hex'))); expect(() => new ObjectId(null).toHexString()).to.not.throw(); + expect(() => new ObjectId(null).id.equals(Buffer.from(null, 'hex'))); }); it('should throw error if non-12 byte non-24 hex string passed in', function () { @@ -203,15 +209,21 @@ describe.only('ObjectId', function () { }); it('should correctly create ObjectId from 12 byte or 24 hex string', function () { - expect(() => new ObjectId('AAAAAAAAAAAAAAAAAAAAAAAA').toHexString()).to.not.throw(); - expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFF').toHexString()).to.not.throw(); - expect(() => new ObjectId('abcdefghijkl').toHexString()).to.not.throw(); + var str1 = 'AAAAAAAAAAAAAAAAAAAAAAAA'; + var str2 = 'FFFFFFFFFFFFFFFFFFFFFFFF'; + var str3 = 'abcdefghijkl'; + expect(() => new ObjectId(str1).toHexString()).to.not.throw(); + expect(() => new ObjectId(str1).id.equals(Buffer.from(str1, 'hex'))); + expect(() => new ObjectId(str2).toHexString()).to.not.throw(); + expect(() => new ObjectId(str2).id.equals(Buffer.from(str2, 'hex'))); + expect(() => new ObjectId(str3).toHexString()).to.not.throw(); + expect(() => new ObjectId(str3).id.equals(Buffer.from(str3, 'hex'))); }); it('should correctly create ObjectId from 12 byte sequence', function () { var a = '111111111111'; expect(() => new ObjectId(a).toHexString()).to.not.throw(); - expect(Buffer.from(new ObjectId(a).id).equals(Buffer.from('111111111111', 'latin1'))); + expect(Buffer.from(new ObjectId(a).id).equals(Buffer.from(a, 'latin1'))); }); /** @@ -250,10 +262,9 @@ describe.only('ObjectId', function () { done(); }); - it('should throw an error if invalid Buffer passed in', function (done) { + it('should throw an error if invalid Buffer passed in', function () { var a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); expect(() => new ObjectId(a)).to.throw(TypeError); - done(); }); /** From 597a74e860da3d03d60d0b3456b907b161d116f7 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Fri, 29 Oct 2021 18:11:56 -0400 Subject: [PATCH 11/17] chore: remove unnecessary tests and fix numeric tests --- src/objectid.ts | 8 +++++--- test/node/object_id_tests.js | 32 ++++++-------------------------- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index ec09c79a..03d0552d 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -31,7 +31,7 @@ export class ObjectId { _bsontype!: 'ObjectId'; /** @internal */ - static index = ~~(Math.random() * 0xffffff); + static index = Math.floor(Math.random() * 0xffffff); static cacheHexString: boolean; @@ -48,6 +48,7 @@ export class ObjectId { constructor(inputId?: string | Buffer | number | ObjectIdLike | ObjectId) { if (!(this instanceof ObjectId)) return new ObjectId(inputId); + // workingId is set based on type of input and whether valid id exists for the input let workingId; if (typeof inputId === 'object' && inputId && 'id' in inputId) { if (typeof inputId.id !== 'string' && !ArrayBuffer.isView(inputId.id)) { @@ -63,7 +64,8 @@ export class ObjectId { } else { workingId = inputId; } - // the following checks use workingId to construct an ObjectId + + // 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 @@ -152,7 +154,7 @@ export class ObjectId { */ static generate(time?: number): Buffer { if ('number' !== typeof time) { - time = ~~(Date.now() / 1000); + time = Math.floor(Date.now() / 1000); } const inc = ObjectId.getInc(); diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index e09c4a40..b8a3d84f 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -27,7 +27,6 @@ describe('ObjectId', function () { it('should correctly create ObjectId from ObjectId', function () { var tmp = new ObjectId(); - expect(() => new ObjectId(tmp)).to.not.throw(); expect(() => new ObjectId(tmp).id.equals(Buffer.from(tmp, 'hex'))); }); @@ -63,9 +62,7 @@ describe('ObjectId', function () { }; var buf1 = Buffer.from('aaaaaaaaaaaaaaaaaaaaaaaa', 'hex'); var buf2 = Buffer.from('abcdefghijkl', 'hex'); - expect(() => new ObjectId(objectValidStringId1).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectValidStringId1).id).equals(buf1)); - expect(() => new ObjectId(objectValidStringId2).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectValidStringId2).id).equals(buf2)); }); @@ -82,9 +79,7 @@ describe('ObjectId', function () { id: 'abcdefghijkl', toHexString: newToHexString }; - expect(() => new ObjectId(objectValidStringId1).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectValidStringId1).id).equals(buf)); - expect(() => new ObjectId(objectValidStringId2).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectValidStringId2).id).equals(buf)); }); @@ -97,10 +92,8 @@ describe('ObjectId', function () { var objectBufferFromArray = { id: validBuffer2 }; - expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectBufferId).id).equals(validBuffer1)); - expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); - expect(Buffer.from(new ObjectId(objectBufferId).id).equals(validBuffer2)); + expect(Buffer.from(new ObjectId(objectBufferFromArray).id).equals(validBuffer2)); }); it('should correctly create ObjectId from object with valid Buffer id and toHexString method', function () { @@ -118,9 +111,7 @@ describe('ObjectId', function () { id: validBuffer2, toHexString: newToHexString }; - expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectBufferId).id).equals(bufferToHex)); - expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectBufferFromArray).id).equals(bufferToHex)); }); @@ -151,7 +142,6 @@ describe('ObjectId', function () { toHexString: newToHexString }; var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); - expect(() => new ObjectId(objectInvalidString)).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectInvalidString).id).equals(buf)); }); @@ -171,7 +161,6 @@ describe('ObjectId', function () { toHexString: newToHexString }; var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); - expect(() => new ObjectId(objectInvalidBuffer)).to.not.throw(TypeError); expect(Buffer.from(new ObjectId(objectInvalidBuffer).id).equals(buf)); }); @@ -183,21 +172,16 @@ describe('ObjectId', function () { return tmp.toHexString(); } }; - expect(() => new ObjectId(objectIdLike).toHexString()).to.not.throw(TypeError); expect(() => new ObjectId(objectIdLike).id.equals(Buffer.from(tmp, 'hex'))); }); it('should correctly create ObjectId from number or null', function () { - expect(() => new ObjectId(42).toHexString()).to.not.throw(); - expect(() => new ObjectId(42).id.equals(Buffer.from(42, 'hex'))); - expect(() => new ObjectId(0x2a).toHexString()).to.not.throw(); - expect(() => new ObjectId(0x2a).id.equals(Buffer.from(0x2a, 'hex'))); - expect(() => new ObjectId(4.2).toHexString()).to.not.throw(); - expect(() => new ObjectId(4.2).id.equals(Buffer.from(4, 'hex'))); - expect(() => new ObjectId(NaN).toHexString()).to.not.throw(); - expect(() => new ObjectId(NaN).id.equals(Buffer.from(NaN, 'hex'))); - expect(() => new ObjectId(null).toHexString()).to.not.throw(); + expect(new ObjectId(42).id.readUint32BE(0)).to.equal(42); + expect(new ObjectId(0x2a).id.readUint32BE(0)).to.equal(0x2a); + expect(new ObjectId(4.2).id.readUint32BE(0)).to.equal(4); + expect(new ObjectId(NaN).id.readUint32BE(0)).to.equal(0); expect(() => new ObjectId(null).id.equals(Buffer.from(null, 'hex'))); + expect(() => new ObjectId(undefined).id.equals(Buffer.from(undefined, 'hex'))); }); it('should throw error if non-12 byte non-24 hex string passed in', function () { @@ -212,17 +196,13 @@ describe('ObjectId', function () { var str1 = 'AAAAAAAAAAAAAAAAAAAAAAAA'; var str2 = 'FFFFFFFFFFFFFFFFFFFFFFFF'; var str3 = 'abcdefghijkl'; - expect(() => new ObjectId(str1).toHexString()).to.not.throw(); expect(() => new ObjectId(str1).id.equals(Buffer.from(str1, 'hex'))); - expect(() => new ObjectId(str2).toHexString()).to.not.throw(); expect(() => new ObjectId(str2).id.equals(Buffer.from(str2, 'hex'))); - expect(() => new ObjectId(str3).toHexString()).to.not.throw(); expect(() => new ObjectId(str3).id.equals(Buffer.from(str3, 'hex'))); }); it('should correctly create ObjectId from 12 byte sequence', function () { var a = '111111111111'; - expect(() => new ObjectId(a).toHexString()).to.not.throw(); expect(Buffer.from(new ObjectId(a).id).equals(Buffer.from(a, 'latin1'))); }); From 20e813c3995fa3bba83030018362a76139ebe2f7 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Mon, 1 Nov 2021 10:59:12 -0400 Subject: [PATCH 12/17] chore: change tests for consistency --- test/node/object_id_tests.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index b8a3d84f..d41b0897 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -176,10 +176,10 @@ describe('ObjectId', function () { }); it('should correctly create ObjectId from number or null', function () { - expect(new ObjectId(42).id.readUint32BE(0)).to.equal(42); - expect(new ObjectId(0x2a).id.readUint32BE(0)).to.equal(0x2a); - expect(new ObjectId(4.2).id.readUint32BE(0)).to.equal(4); - expect(new ObjectId(NaN).id.readUint32BE(0)).to.equal(0); + expect(() => new ObjectId(42).id.readUint32BE(0).equals(42)); + expect(() => new ObjectId(0x2a).id.readUint32BE(0).equals(0x2a)); + expect(() => new ObjectId(4.2).id.readUint32BE(0).equals(4)); + expect(() => new ObjectId(NaN).id.readUint32BE(0).equals(0)); expect(() => new ObjectId(null).id.equals(Buffer.from(null, 'hex'))); expect(() => new ObjectId(undefined).id.equals(Buffer.from(undefined, 'hex'))); }); From 9407bbecaa3462c030183f35a2bc272ea104fbeb Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Mon, 1 Nov 2021 15:27:39 -0400 Subject: [PATCH 13/17] fix: fix numeric issues and condense tests --- test/node/object_id_tests.js | 80 +++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index d41b0897..0e864766 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -1,5 +1,4 @@ 'use strict'; - const Buffer = require('buffer').Buffer; const BSON = require('../register-bson'); const util = require('util'); @@ -27,20 +26,20 @@ describe('ObjectId', function () { it('should correctly create ObjectId from ObjectId', function () { var tmp = new ObjectId(); - expect(() => new ObjectId(tmp).id.equals(Buffer.from(tmp, 'hex'))); + expect(new ObjectId(tmp).id.equals(Buffer.from(tmp.id, 'hex'))); }); - it('should throw error if empty array is passed in', function () { - expect(() => new ObjectId([])).to.throw(TypeError); - }); + const invalidInputs = [ + {input: [], description: 'empty array'}, + {input: ['abcdefŽhijkl'], description: 'nonempty array'}, + {input: {}, description: 'empty object'} + ] - it('should throw error if nonempty array is passed in', function () { - expect(() => new ObjectId(['abcdefŽhijkl'])).to.throw(TypeError); - }); - - it('should throw error if empty object is passed in', function () { - expect(() => new ObjectId({})).to.throw(TypeError); - }); + for (const {input, description} of invalidInputs) { + it(`should throw error if ${description} is passed in`, function () { + expect(() => new ObjectId(input)).to.throw(TypeError); + }); + } it('should throw error if object without an id property is passed in', function () { var tmp = new ObjectId(); @@ -164,41 +163,46 @@ describe('ObjectId', function () { expect(Buffer.from(new ObjectId(objectInvalidBuffer).id).equals(buf)); }); - it('should correctly create ObjectId from object with objectIdLike properties', function () { - var tmp = new ObjectId(); - var objectIdLike = { - id: tmp.id, - toHexString: function () { - return tmp.toHexString(); - } - }; - expect(() => new ObjectId(objectIdLike).id.equals(Buffer.from(tmp, 'hex'))); - }); - - it('should correctly create ObjectId from number or null', function () { - expect(() => new ObjectId(42).id.readUint32BE(0).equals(42)); - expect(() => new ObjectId(0x2a).id.readUint32BE(0).equals(0x2a)); - expect(() => new ObjectId(4.2).id.readUint32BE(0).equals(4)); - expect(() => new ObjectId(NaN).id.readUint32BE(0).equals(0)); - expect(() => new ObjectId(null).id.equals(Buffer.from(null, 'hex'))); - expect(() => new ObjectId(undefined).id.equals(Buffer.from(undefined, 'hex'))); + const numericIO = [ + {input: 42, output: 42}, + {input: 0x2a, output: 0x2a}, + {input: 4.2, output: 4}, + {input: NaN, output: 0} + ] + + for (const {input, output} of numericIO) { + it(`should correctly create ObjectId from ${input} and result in ${output}`, function () { + const objId = new ObjectId(input); + expect(objId).to.have.property('id'); + expect(objId.id).to.be.instanceOf(Buffer); + expect(objId.id.readUInt32BE(0)).to.equal(output); + }); + } + + it('should correctly create ObjectId undefined or null', function () { + const objNull = new ObjectId(null); + const objNoArg = new ObjectId(); + const objUndef = new ObjectId(undefined); + expect(objNull.id).to.be.instanceOf(Buffer); + expect(objNoArg.id).to.be.instanceOf(Buffer); + expect(objUndef.id).to.be.instanceOf(Buffer); }); it('should throw error if non-12 byte non-24 hex string passed in', function () { - expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(); - expect(() => new ObjectId('thisstringisdefinitelytoolong')).to.throw(); - expect(() => new ObjectId('tooshort')).to.throw(); - expect(() => new ObjectId('101010')).to.throw(); - expect(() => new ObjectId('')).to.throw(); + expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(TypeError); + expect(() => new ObjectId('thisstringisdefinitelytoolong')).to.throw(TypeError); + expect(() => new ObjectId('tooshort')).to.throw(TypeError); + expect(() => new ObjectId('101010')).to.throw(TypeError); + expect(() => new ObjectId('')).to.throw(TypeError); }); it('should correctly create ObjectId from 12 byte or 24 hex string', function () { var str1 = 'AAAAAAAAAAAAAAAAAAAAAAAA'; var str2 = 'FFFFFFFFFFFFFFFFFFFFFFFF'; var str3 = 'abcdefghijkl'; - expect(() => new ObjectId(str1).id.equals(Buffer.from(str1, 'hex'))); - expect(() => new ObjectId(str2).id.equals(Buffer.from(str2, 'hex'))); - expect(() => new ObjectId(str3).id.equals(Buffer.from(str3, 'hex'))); + expect(new ObjectId(str1).id.equals(Buffer.from(str1, 'hex'))); + expect(new ObjectId(str2).id.equals(Buffer.from(str2, 'hex'))); + expect(new ObjectId(str3).id.equals(Buffer.from(str3, 'hex'))); }); it('should correctly create ObjectId from 12 byte sequence', function () { From b825cb9f564831b56ad6c8ca993094ad66506a08 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Mon, 1 Nov 2021 15:30:42 -0400 Subject: [PATCH 14/17] chore: run lint --- test/node/object_id_tests.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index 0e864766..a0595665 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -30,12 +30,12 @@ describe('ObjectId', function () { }); const invalidInputs = [ - {input: [], description: 'empty array'}, - {input: ['abcdefŽhijkl'], description: 'nonempty array'}, - {input: {}, description: 'empty object'} - ] + { input: [], description: 'empty array' }, + { input: ['abcdefŽhijkl'], description: 'nonempty array' }, + { input: {}, description: 'empty object' } + ]; - for (const {input, description} of invalidInputs) { + for (const { input, description } of invalidInputs) { it(`should throw error if ${description} is passed in`, function () { expect(() => new ObjectId(input)).to.throw(TypeError); }); @@ -164,13 +164,13 @@ describe('ObjectId', function () { }); const numericIO = [ - {input: 42, output: 42}, - {input: 0x2a, output: 0x2a}, - {input: 4.2, output: 4}, - {input: NaN, output: 0} - ] + { input: 42, output: 42 }, + { input: 0x2a, output: 0x2a }, + { input: 4.2, output: 4 }, + { input: NaN, output: 0 } + ]; - for (const {input, output} of numericIO) { + for (const { input, output } of numericIO) { it(`should correctly create ObjectId from ${input} and result in ${output}`, function () { const objId = new ObjectId(input); expect(objId).to.have.property('id'); From 1896f7ca97916fd6bdb16bc8399116047cf67da7 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Mon, 1 Nov 2021 16:46:13 -0400 Subject: [PATCH 15/17] test: specify equality checks to check content --- test/node/object_id_tests.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index a0595665..44b2d25e 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -26,7 +26,7 @@ describe('ObjectId', function () { it('should correctly create ObjectId from ObjectId', function () { var tmp = new ObjectId(); - expect(new ObjectId(tmp).id.equals(Buffer.from(tmp.id, 'hex'))); + expect(new ObjectId(tmp).id).to.deep.equal(Buffer.from(tmp.id, 'hex')); }); const invalidInputs = [ @@ -78,8 +78,8 @@ describe('ObjectId', function () { id: 'abcdefghijkl', toHexString: newToHexString }; - expect(Buffer.from(new ObjectId(objectValidStringId1).id).equals(buf)); - expect(Buffer.from(new ObjectId(objectValidStringId2).id).equals(buf)); + expect(Buffer.from(new ObjectId(objectValidStringId1).id)).to.deep.equal(buf); + expect(Buffer.from(new ObjectId(objectValidStringId2).id)).to.deep.equal(buf); }); it('should correctly create ObjectId from object with valid Buffer id', function () { @@ -91,8 +91,8 @@ describe('ObjectId', function () { var objectBufferFromArray = { id: validBuffer2 }; - expect(Buffer.from(new ObjectId(objectBufferId).id).equals(validBuffer1)); - expect(Buffer.from(new ObjectId(objectBufferFromArray).id).equals(validBuffer2)); + expect(Buffer.from(new ObjectId(objectBufferId).id)).to.deep.equals(validBuffer1); + expect(Buffer.from(new ObjectId(objectBufferFromArray).id)).to.deep.equals(validBuffer2); }); it('should correctly create ObjectId from object with valid Buffer id and toHexString method', function () { @@ -110,8 +110,8 @@ describe('ObjectId', function () { id: validBuffer2, toHexString: newToHexString }; - expect(Buffer.from(new ObjectId(objectBufferId).id).equals(bufferToHex)); - expect(Buffer.from(new ObjectId(objectBufferFromArray).id).equals(bufferToHex)); + expect(Buffer.from(new ObjectId(objectBufferId).id)).to.deep.equal(bufferToHex); + expect(Buffer.from(new ObjectId(objectBufferFromArray).id)).to.deep.equal(bufferToHex); }); it('should throw error if object with non-Buffer non-string id is passed in', function () { @@ -141,7 +141,7 @@ describe('ObjectId', function () { toHexString: newToHexString }; var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); - expect(Buffer.from(new ObjectId(objectInvalidString).id).equals(buf)); + expect(Buffer.from(new ObjectId(objectInvalidString).id)).to.deep.equal(buf); }); it('should throw an error if object with invalid Buffer id is passed in', function () { @@ -160,7 +160,7 @@ describe('ObjectId', function () { toHexString: newToHexString }; var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); - expect(Buffer.from(new ObjectId(objectInvalidBuffer).id).equals(buf)); + expect(Buffer.from(new ObjectId(objectInvalidBuffer).id)).to.deep.equal(buf); }); const numericIO = [ @@ -207,7 +207,7 @@ describe('ObjectId', function () { it('should correctly create ObjectId from 12 byte sequence', function () { var a = '111111111111'; - expect(Buffer.from(new ObjectId(a).id).equals(Buffer.from(a, 'latin1'))); + expect(Buffer.from(new ObjectId(a).id)).to.deep.equal(Buffer.from(a, 'latin1')); }); /** From 624bcae0f0147e77b82e78346f21a92e9b1ac18e Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Tue, 2 Nov 2021 13:01:41 -0400 Subject: [PATCH 16/17] fix: address PR comments and add error for 12 length strings that arent 12 bytes --- src/objectid.ts | 2 + test/node/object_id_tests.js | 178 ++++++++++++++++------------------- 2 files changed, 85 insertions(+), 95 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index 03d0552d..dcd8cd6a 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -77,6 +77,8 @@ export class ObjectId { const bytes = Buffer.from(workingId); if (bytes.byteLength === 12) { this[kId] = bytes; + } else { + throw new BSONTypeError('Argument passed in must be a string of 12 bytes'); } } else if (workingId.length === 24 && checkForHexRegExp.test(workingId)) { this[kId] = Buffer.from(workingId, 'hex'); diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index 44b2d25e..8125ca31 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -1,21 +1,17 @@ 'use strict'; + const Buffer = require('buffer').Buffer; const BSON = require('../register-bson'); const util = require('util'); const ObjectId = BSON.ObjectId; describe('ObjectId', function () { - /** - * @ignore - */ - it('should correctly handle objectId timestamps', function (done) { - // var test_number = {id: ObjectI()}; - var a = ObjectId.createFromTime(1); + let a = ObjectId.createFromTime(1); expect(Buffer.from([0, 0, 0, 1])).to.deep.equal(a.id.slice(0, 4)); expect(1000).to.equal(a.getTimestamp().getTime()); - var b = new ObjectId(); + let b = new ObjectId(); b.generationTime = 1; expect(Buffer.from([0, 0, 0, 1])).to.deep.equal(b.id.slice(0, 4)); expect(1).to.equal(b.generationTime); @@ -25,8 +21,8 @@ describe('ObjectId', function () { }); it('should correctly create ObjectId from ObjectId', function () { - var tmp = new ObjectId(); - expect(new ObjectId(tmp).id).to.deep.equal(Buffer.from(tmp.id, 'hex')); + const noArgObjID = new ObjectId(); + expect(new ObjectId(noArgObjID).id).to.deep.equal(Buffer.from(noArgObjID.id, 'hex')); }); const invalidInputs = [ @@ -42,10 +38,10 @@ describe('ObjectId', function () { } it('should throw error if object without an id property is passed in', function () { - var tmp = new ObjectId(); - var objectIdLike = { + const noArgObjID = new ObjectId(); + const objectIdLike = { toHexString: function () { - return tmp.toHexString(); + return noArgObjID.toHexString(); } }; @@ -53,72 +49,72 @@ describe('ObjectId', function () { }); it('should correctly create ObjectId from object with valid string id', function () { - var objectValidStringId1 = { + const objectValidString24Hex = { id: 'aaaaaaaaaaaaaaaaaaaaaaaa' }; - var objectValidStringId2 = { + const objectValidString12Bytes = { id: 'abcdefghijkl' }; - var buf1 = Buffer.from('aaaaaaaaaaaaaaaaaaaaaaaa', 'hex'); - var buf2 = Buffer.from('abcdefghijkl', 'hex'); - expect(Buffer.from(new ObjectId(objectValidStringId1).id).equals(buf1)); - expect(Buffer.from(new ObjectId(objectValidStringId2).id).equals(buf2)); + const buf24Hex = Buffer.from('aaaaaaaaaaaaaaaaaaaaaaaa', 'hex'); + const buf12Bytes = Buffer.from('abcdefghijkl'); + expect(new ObjectId(objectValidString24Hex).id).to.deep.equal(buf24Hex); + expect(new ObjectId(objectValidString12Bytes).id).to.deep.equal(buf12Bytes); }); it('should correctly create ObjectId from object with valid string id and toHexString method', function () { - function newToHexString() { + function new24HexToHexString() { return 'BBBBBBBBBBBBBBBBBBBBBBBB'; } - var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); - var objectValidStringId1 = { + const buf24hex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + const objectValidString24Hex = { id: 'aaaaaaaaaaaaaaaaaaaaaaaa', - toHexString: newToHexString + toHexString: new24HexToHexString }; - var objectValidStringId2 = { + const objectValidString12Bytes = { id: 'abcdefghijkl', - toHexString: newToHexString + toHexString: new24HexToHexString }; - expect(Buffer.from(new ObjectId(objectValidStringId1).id)).to.deep.equal(buf); - expect(Buffer.from(new ObjectId(objectValidStringId2).id)).to.deep.equal(buf); + expect(new ObjectId(objectValidString24Hex).id).to.deep.equal(buf24hex); + expect(new ObjectId(objectValidString12Bytes).id).to.deep.equal(buf24hex); }); it('should correctly create ObjectId from object with valid Buffer id', function () { - var validBuffer1 = Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'); - var validBuffer2 = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); - var objectBufferId = { - id: validBuffer1 + const validBuffer24Hex = Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'); + const validBuffer12Array = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); + const objectBufferId = { + id: validBuffer24Hex }; - var objectBufferFromArray = { - id: validBuffer2 + const objectBufferFromArray = { + id: validBuffer12Array }; - expect(Buffer.from(new ObjectId(objectBufferId).id)).to.deep.equals(validBuffer1); - expect(Buffer.from(new ObjectId(objectBufferFromArray).id)).to.deep.equals(validBuffer2); + expect(new ObjectId(objectBufferId).id).to.deep.equals(validBuffer24Hex); + expect(new ObjectId(objectBufferFromArray).id).to.deep.equals(validBuffer12Array); }); it('should correctly create ObjectId from object with valid Buffer id and toHexString method', function () { - var validBuffer1 = Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'); - var validBuffer2 = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); - var bufferToHex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + const validBuffer24Hex = Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'); + const validBuffer12Array = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); + const bufferNew24Hex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); function newToHexString() { return 'BBBBBBBBBBBBBBBBBBBBBBBB'; } - var objectBufferId = { - id: validBuffer1, + const objectBufferHex = { + id: validBuffer24Hex, toHexString: newToHexString }; - var objectBufferFromArray = { - id: validBuffer2, + const objectBufferArray = { + id: validBuffer12Array, toHexString: newToHexString }; - expect(Buffer.from(new ObjectId(objectBufferId).id)).to.deep.equal(bufferToHex); - expect(Buffer.from(new ObjectId(objectBufferFromArray).id)).to.deep.equal(bufferToHex); + expect(new ObjectId(objectBufferHex).id).to.deep.equal(bufferNew24Hex); + expect(new ObjectId(objectBufferArray).id).to.deep.equal(bufferNew24Hex); }); it('should throw error if object with non-Buffer non-string id is passed in', function () { - var objectNumId = { + const objectNumId = { id: 5 }; - var objectNullId = { + const objectNullId = { id: null }; expect(() => new ObjectId(objectNumId)).to.throw(TypeError); @@ -126,26 +122,26 @@ describe('ObjectId', function () { }); it('should throw an error if object with invalid string id is passed in', function () { - var objectInvalidString = { + const objectInvalid24HexStr = { id: 'FFFFFFFFFFFFFFFFFFFFFFFG' }; - expect(() => new ObjectId(objectInvalidString)).to.throw(TypeError); + expect(() => new ObjectId(objectInvalid24HexStr)).to.throw(TypeError); }); it('should correctly create ObjectId from object with invalid string id and toHexString method', function () { function newToHexString() { return 'BBBBBBBBBBBBBBBBBBBBBBBB'; } - var objectInvalidString = { + const objectInvalid24HexStr = { id: 'FFFFFFFFFFFFFFFFFFFFFFFG', toHexString: newToHexString }; - var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); - expect(Buffer.from(new ObjectId(objectInvalidString).id)).to.deep.equal(buf); + const bufferNew24Hex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + expect(new ObjectId(objectInvalid24HexStr).id).to.deep.equal(bufferNew24Hex); }); it('should throw an error if object with invalid Buffer id is passed in', function () { - var objectInvalidBuffer = { + const objectInvalidBuffer = { id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]) }; expect(() => new ObjectId(objectInvalidBuffer)).to.throw(TypeError); @@ -155,19 +151,19 @@ describe('ObjectId', function () { function newToHexString() { return 'BBBBBBBBBBBBBBBBBBBBBBBB'; } - var objectInvalidBuffer = { + const objectInvalidBuffer = { id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]), toHexString: newToHexString }; - var buf = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); - expect(Buffer.from(new ObjectId(objectInvalidBuffer).id)).to.deep.equal(buf); + const bufferNew24Hex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + expect(new ObjectId(objectInvalidBuffer).id).to.deep.equal(bufferNew24Hex); }); const numericIO = [ - { input: 42, output: 42 }, - { input: 0x2a, output: 0x2a }, - { input: 4.2, output: 4 }, - { input: NaN, output: 0 } + { input: 42, output: 42, description: '42' }, + { input: 0x2a, output: 0x2a, description: '0x2a' }, + { input: 4.2, output: 4, description: '4.2' }, + { input: NaN, output: 0, description: 'NaN' } ]; for (const { input, output } of numericIO) { @@ -196,27 +192,20 @@ describe('ObjectId', function () { expect(() => new ObjectId('')).to.throw(TypeError); }); - it('should correctly create ObjectId from 12 byte or 24 hex string', function () { - var str1 = 'AAAAAAAAAAAAAAAAAAAAAAAA'; - var str2 = 'FFFFFFFFFFFFFFFFFFFFFFFF'; - var str3 = 'abcdefghijkl'; - expect(new ObjectId(str1).id.equals(Buffer.from(str1, 'hex'))); - expect(new ObjectId(str2).id.equals(Buffer.from(str2, 'hex'))); - expect(new ObjectId(str3).id.equals(Buffer.from(str3, 'hex'))); + it('should correctly create ObjectId from 24 hex string', function () { + const validStr24Hex = 'FFFFFFFFFFFFFFFFFFFFFFFF'; + expect(new ObjectId(validStr24Hex).id).to.deep.equal(Buffer.from(validStr24Hex, 'hex')); }); it('should correctly create ObjectId from 12 byte sequence', function () { - var a = '111111111111'; - expect(Buffer.from(new ObjectId(a).id)).to.deep.equal(Buffer.from(a, 'latin1')); + const byteSequence12 = '111111111111'; + expect(new ObjectId(byteSequence12).id).to.deep.equal(Buffer.from(byteSequence12, 'latin1')); }); - /** - * @ignore - */ it('should correctly create ObjectId from uppercase hexstring', function (done) { - var a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; - var b = new ObjectId(a); - var c = b.equals(a); // => false + let a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; + let b = new ObjectId(a); + let c = b.equals(a); // => false expect(true).to.equal(c); a = 'aaaaaaaaaaaaaaaaaaaaaaaa'; @@ -228,14 +217,11 @@ describe('ObjectId', function () { done(); }); - /** - * @ignore - */ it('should correctly create ObjectId from valid Buffer', function (done) { if (!Buffer.from) return done(); - var a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; - var b = new ObjectId(Buffer.from(a, 'hex')); - var c = b.equals(a); // => false + let a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; + let b = new ObjectId(Buffer.from(a, 'hex')); + let c = b.equals(a); // => false expect(true).to.equal(c); a = 'aaaaaaaaaaaaaaaaaaaaaaaa'; @@ -247,28 +233,22 @@ describe('ObjectId', function () { }); it('should throw an error if invalid Buffer passed in', function () { - var a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); + const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); expect(() => new ObjectId(a)).to.throw(TypeError); }); - /** - * @ignore - */ it('should correctly allow for node.js inspect to work with ObjectId', function (done) { - var a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; - var b = new ObjectId(a); + const a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; + const b = new ObjectId(a); expect(util.inspect(b)).to.equal('new ObjectId("aaaaaaaaaaaaaaaaaaaaaaaa")'); done(); }); - /** - * @ignore - */ it('should isValid check input Buffer length', function (done) { - var buffTooShort = Buffer.from([]); - var buffTooLong = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); - var buff12Bytes = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); + const buffTooShort = Buffer.from([]); + const buffTooLong = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); + const buff12Bytes = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); expect(ObjectId.isValid(buffTooShort)).to.be.false; expect(ObjectId.isValid(buffTooLong)).to.be.false; @@ -276,13 +256,21 @@ describe('ObjectId', function () { done(); }); - it('should throw if a 12-char string is passed in with character codes greater than 256', function () { - expect(() => new ObjectId('abcdefghijkl').toHexString()).to.not.throw(); - expect(() => new ObjectId('abcdefŽhijkl').toHexString()).to.throw(TypeError); + it('should throw if a 12-char length but non-12 byte string is passed in', function () { + const characterCodesLargerThan256 = 'abcdefŽhijkl'; + const length12Not12Bytes = '🐶🐶🐶🐶🐶🐶'; + expect(() => new ObjectId(characterCodesLargerThan256).toHexString()).to.throw( + TypeError, + 'Argument passed in must be a string of 12 bytes' + ); + expect(() => new ObjectId(length12Not12Bytes).id).to.throw( + TypeError, + 'Argument passed in must be a string of 12 bytes' + ); }); it('should correctly interpret timestamps beyond 2038', function () { - var farFuture = new Date('2040-01-01T00:00:00.000Z').getTime(); + const farFuture = new Date('2040-01-01T00:00:00.000Z').getTime(); expect( new BSON.ObjectId(BSON.ObjectId.generate(farFuture / 1000)).getTimestamp().getTime() ).to.equal(farFuture); From 6962e2dc118bb701cdc94114d222aeea6f433d19 Mon Sep 17 00:00:00 2001 From: Grace Chong Date: Tue, 2 Nov 2021 15:13:27 -0400 Subject: [PATCH 17/17] chore: change to const --- test/node/object_id_tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index 8125ca31..4d18b69f 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -7,11 +7,11 @@ const ObjectId = BSON.ObjectId; describe('ObjectId', function () { it('should correctly handle objectId timestamps', function (done) { - let a = ObjectId.createFromTime(1); + const a = ObjectId.createFromTime(1); expect(Buffer.from([0, 0, 0, 1])).to.deep.equal(a.id.slice(0, 4)); expect(1000).to.equal(a.getTimestamp().getTime()); - let b = new ObjectId(); + const b = new ObjectId(); b.generationTime = 1; expect(Buffer.from([0, 0, 0, 1])).to.deep.equal(b.id.slice(0, 4)); expect(1).to.equal(b.generationTime);