From 7e84230c892865b2aad5ada2c9d5b94012662d19 Mon Sep 17 00:00:00 2001 From: Sam Zhang Date: Thu, 16 Dec 2021 16:09:26 -0500 Subject: [PATCH 1/5] update constructor and add test assertions --- src/bson.ts | 2 +- src/decimal128.ts | 9 ++++++++- test/node/decimal128_tests.js | 23 +++++++++++++++++++++-- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index 924235aa..904a5b56 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -112,7 +112,7 @@ export interface Document { const MAXSIZE = 1024 * 1024 * 17; // Current Internal Temporary Serialization Buffer -let buffer = Buffer.alloc(MAXSIZE); +let buffer = Buffer.allocUnsafe(MAXSIZE); /** * Sets the size of the internal serialization buffer. diff --git a/src/decimal128.ts b/src/decimal128.ts index 0427dad8..daf01c78 100644 --- a/src/decimal128.ts +++ b/src/decimal128.ts @@ -1,6 +1,7 @@ import { Buffer } from 'buffer'; import { BSONTypeError } from './error'; import { Long } from './long'; +import { isUint8Array } from './parser/utils'; const PARSE_STRING_REGEXP = /^(\+|-)?(\d+|(\d*\.\d*))?(E|e)?([-+])?(\d+)?$/; const PARSE_INF_REGEXP = /^(\+|-)?(Infinity|inf)$/i; @@ -132,8 +133,14 @@ export class Decimal128 { if (typeof bytes === 'string') { this.bytes = Decimal128.fromString(bytes).bytes; + } else if (isUint8Array(bytes)) { + if (bytes.byteLength === 16) { + this.bytes = bytes; + } else { + throw new BSONTypeError('Decimal128 must take a Buffer of 16 bytes'); + } } else { - this.bytes = bytes; + throw new BSONTypeError('Decimal128 must take a Buffer or string'); } } diff --git a/test/node/decimal128_tests.js b/test/node/decimal128_tests.js index e5ba7df3..94dae003 100644 --- a/test/node/decimal128_tests.js +++ b/test/node/decimal128_tests.js @@ -1,5 +1,6 @@ 'use strict'; +// const { BSONTypeError } = require('../register-bson'); const BSON = require('../register-bson'); const Decimal128 = BSON.Decimal128; @@ -1205,7 +1206,7 @@ describe('Decimal128', function () { done(); }); - it('accepts strings in the constructor', function (done) { + it('accepts strings in the constructor', () => { expect(new Decimal128('0').toString()).to.equal('0'); expect(new Decimal128('00').toString()).to.equal('0'); expect(new Decimal128('0.5').toString()).to.equal('0.5'); @@ -1216,6 +1217,24 @@ describe('Decimal128', function () { expect(new Decimal128('0.0011').toString()).to.equal('0.0011'); expect(new Decimal128('0.00110').toString()).to.equal('0.00110'); expect(new Decimal128('-1e400').toString()).to.equal('-1E+400'); - done(); + }); + + it.only('throws appropriate error for invalid constructor arguments', () => { + const byteLengthErrMsg = 'Decimal128 must take a Buffer of 16 bytes'; + const constructorArgErrMsg = 'Decimal128 must take a Buffer or string'; + + expect(() => new Decimal128(-0)).to.throw(constructorArgErrMsg); + expect(() => new Decimal128(-1)).to.throw(constructorArgErrMsg); + expect(() => new Decimal128(10)).to.throw(constructorArgErrMsg); + expect(() => new Decimal128(1111111111111111)).to.throw(constructorArgErrMsg); + + expect(() => new Decimal128(new Uint8Array(0))).to.throw(byteLengthErrMsg); + expect(() => new Decimal128(Buffer.alloc(0))).to.throw(byteLengthErrMsg); + expect(() => new Decimal128(new Uint8Array(3))).to.throw(byteLengthErrMsg); + expect(() => new Decimal128(Buffer.alloc(3))).to.throw(byteLengthErrMsg); + expect(() => new Decimal128(new Uint8Array(17))).to.throw(byteLengthErrMsg); + expect(() => new Decimal128(Buffer.alloc(17))).to.throw(byteLengthErrMsg); + new Decimal128(Buffer.alloc(16)); + new Decimal128(new Uint8Array(16)); }); }); From 2362471516ae5601e5d65ab55a30519ab3e35c9d Mon Sep 17 00:00:00 2001 From: Sam Zhang Date: Thu, 16 Dec 2021 16:26:05 -0500 Subject: [PATCH 2/5] add remove it.only lint task --- src/decimal128.ts | 5 ++--- test/.eslintrc.json | 17 ++++++++++++++++- test/node/decimal128_tests.js | 2 +- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/decimal128.ts b/src/decimal128.ts index daf01c78..3f8dc734 100644 --- a/src/decimal128.ts +++ b/src/decimal128.ts @@ -134,11 +134,10 @@ export class Decimal128 { if (typeof bytes === 'string') { this.bytes = Decimal128.fromString(bytes).bytes; } else if (isUint8Array(bytes)) { - if (bytes.byteLength === 16) { - this.bytes = bytes; - } else { + if (bytes.byteLength !== 16) { throw new BSONTypeError('Decimal128 must take a Buffer of 16 bytes'); } + this.bytes = bytes; } else { throw new BSONTypeError('Decimal128 must take a Buffer or string'); } diff --git a/test/.eslintrc.json b/test/.eslintrc.json index 9e002e3e..9f0d77ef 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -31,6 +31,21 @@ "error", "global" ], - "promise/no-native": "off" + "promise/no-native": "off", + "no-restricted-properties": [ + "error", + { + "object": "describe", + "property": "only" + }, + { + "object": "it", + "property": "only" + }, + { + "object": "context", + "property": "only" + } + ] } } diff --git a/test/node/decimal128_tests.js b/test/node/decimal128_tests.js index 94dae003..9a6fe0c8 100644 --- a/test/node/decimal128_tests.js +++ b/test/node/decimal128_tests.js @@ -1219,7 +1219,7 @@ describe('Decimal128', function () { expect(new Decimal128('-1e400').toString()).to.equal('-1E+400'); }); - it.only('throws appropriate error for invalid constructor arguments', () => { + it('throws appropriate error for invalid constructor arguments', () => { const byteLengthErrMsg = 'Decimal128 must take a Buffer of 16 bytes'; const constructorArgErrMsg = 'Decimal128 must take a Buffer or string'; From 9acd6b6c14573c5d90825b32699333a48c2ce828 Mon Sep 17 00:00:00 2001 From: Sam Zhang Date: Thu, 16 Dec 2021 16:42:41 -0500 Subject: [PATCH 3/5] fix buggy test --- test/node/extended_json_tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/node/extended_json_tests.js b/test/node/extended_json_tests.js index 926a242d..2be132ff 100644 --- a/test/node/extended_json_tests.js +++ b/test/node/extended_json_tests.js @@ -227,7 +227,7 @@ describe('Extended JSON', function () { binary: new Binary(''), code: new Code('function() {}'), dbRef: new DBRef('tests', new Int32(1), 'test'), - decimal128: new Decimal128(128), + decimal128: new Decimal128('128'), double: new Double(10.1), int32: new Int32(10), long: new Long(234), @@ -248,7 +248,7 @@ describe('Extended JSON', function () { binary: { $binary: { base64: '', subType: '00' } }, code: { $code: 'function() {}' }, dbRef: { $ref: 'tests', $id: { $numberInt: '1' }, $db: 'test' }, - decimal128: { $numberDecimal: '0E-6176' }, + decimal128: { $numberDecimal: '128' }, double: { $numberDouble: '10.1' }, int32: { $numberInt: '10' }, long: { $numberLong: '234' }, From 07ace37daa60f6eba5482a550f188ded7d188421 Mon Sep 17 00:00:00 2001 From: Sam Zhang Date: Fri, 17 Dec 2021 15:13:47 -0500 Subject: [PATCH 4/5] clean up comment + test suites --- src/bson.ts | 2 +- test/node/decimal128_tests.js | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index 904a5b56..924235aa 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -112,7 +112,7 @@ export interface Document { const MAXSIZE = 1024 * 1024 * 17; // Current Internal Temporary Serialization Buffer -let buffer = Buffer.allocUnsafe(MAXSIZE); +let buffer = Buffer.alloc(MAXSIZE); /** * Sets the size of the internal serialization buffer. diff --git a/test/node/decimal128_tests.js b/test/node/decimal128_tests.js index 9a6fe0c8..873d1f1c 100644 --- a/test/node/decimal128_tests.js +++ b/test/node/decimal128_tests.js @@ -1,6 +1,5 @@ 'use strict'; -// const { BSONTypeError } = require('../register-bson'); const BSON = require('../register-bson'); const Decimal128 = BSON.Decimal128; @@ -1219,14 +1218,17 @@ describe('Decimal128', function () { expect(new Decimal128('-1e400').toString()).to.equal('-1E+400'); }); - it('throws appropriate error for invalid constructor arguments', () => { - const byteLengthErrMsg = 'Decimal128 must take a Buffer of 16 bytes'; + it('throws correct error for invalid constructor argument type', () => { const constructorArgErrMsg = 'Decimal128 must take a Buffer or string'; expect(() => new Decimal128(-0)).to.throw(constructorArgErrMsg); expect(() => new Decimal128(-1)).to.throw(constructorArgErrMsg); expect(() => new Decimal128(10)).to.throw(constructorArgErrMsg); expect(() => new Decimal128(1111111111111111)).to.throw(constructorArgErrMsg); + }); + + it('throws correct error for an invalid Buffer constructor argument', () => { + const byteLengthErrMsg = 'Decimal128 must take a Buffer of 16 bytes'; expect(() => new Decimal128(new Uint8Array(0))).to.throw(byteLengthErrMsg); expect(() => new Decimal128(Buffer.alloc(0))).to.throw(byteLengthErrMsg); From b34169d0e71ddf2ea5990111fc51e1255ab6a21c Mon Sep 17 00:00:00 2001 From: Sam Zhang Date: Mon, 20 Dec 2021 17:38:52 -0500 Subject: [PATCH 5/5] refactor out tests to new suite --- test/node/decimal128_tests.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/node/decimal128_tests.js b/test/node/decimal128_tests.js index 873d1f1c..edd8e80c 100644 --- a/test/node/decimal128_tests.js +++ b/test/node/decimal128_tests.js @@ -1236,7 +1236,10 @@ describe('Decimal128', function () { expect(() => new Decimal128(Buffer.alloc(3))).to.throw(byteLengthErrMsg); expect(() => new Decimal128(new Uint8Array(17))).to.throw(byteLengthErrMsg); expect(() => new Decimal128(Buffer.alloc(17))).to.throw(byteLengthErrMsg); - new Decimal128(Buffer.alloc(16)); - new Decimal128(new Uint8Array(16)); + }); + + it('does not throw error for an empty Buffer of correct length constructor argument', () => { + expect(() => new Decimal128(Buffer.alloc(16))).to.not.throw(); + expect(() => new Decimal128(new Uint8Array(16))).to.not.throw(); }); });