From bc2e9d1de3aa4c663bf30458dec3f314a42ae13a Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 15 Oct 2020 17:08:24 -0400 Subject: [PATCH 01/16] store properly inherited bson options in op base --- src/bson.ts | 18 ++++++++++++++++ src/collection.ts | 36 ------------------------------- src/operations/bulk_write.ts | 14 ++++++------ src/operations/command.ts | 19 +++++++++++++--- src/operations/delete.ts | 4 ++-- src/operations/find.ts | 9 -------- src/operations/find_and_modify.ts | 2 +- src/operations/find_one.ts | 7 ++++-- src/operations/insert.ts | 2 +- src/operations/insert_many.ts | 7 ++++-- src/operations/operation.ts | 21 +++++++++++++++++- src/operations/replace_one.ts | 2 +- src/operations/update.ts | 4 ++-- 13 files changed, 77 insertions(+), 68 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index 2af80efb71e..b2d2062054a 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -69,3 +69,21 @@ export function pluckBSONSerializeOptions(options: BSONSerializeOptions): BSONSe raw }; } + +// TODO: naming +// TODO: testing +export function inheritOrDefaultBSONSerializableOptions( + options?: BSONSerializeOptions, + parentOptions?: BSONSerializeOptions +): BSONSerializeOptions { + // Merge the BSONSerializeOptions, preferring options over parentOptions, and substituting a + // default for values not set. + // Note that we exclude fieldsAsRaw and serializeFunctions because I was not sure about their usage + return { + raw: options?.raw ?? parentOptions?.raw ?? false, + promoteLongs: options?.promoteLongs ?? parentOptions?.promoteLongs ?? true, + promoteValues: options?.promoteValues ?? parentOptions?.promoteValues ?? true, + promoteBuffers: options?.promoteBuffers ?? parentOptions?.promoteBuffers ?? false, + ignoreUndefined: options?.ignoreUndefined ?? parentOptions?.ignoreUndefined ?? false + }; +} diff --git a/src/collection.ts b/src/collection.ts index 7dacd0e07e5..dfb93686ef3 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -301,12 +301,6 @@ export class Collection implements OperationParent { if (typeof options === 'function') (callback = options), (options = {}); options = options || {}; - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - return executeOperation(this.s.topology, new InsertOneOperation(this, doc, options), callback); } @@ -428,12 +422,6 @@ export class Collection implements OperationParent { if (typeof options === 'function') (callback = options), (options = {}); options = Object.assign({}, options); - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - return executeOperation( this.s.topology, new UpdateOneOperation(this, filter, update, options), @@ -471,12 +459,6 @@ export class Collection implements OperationParent { if (typeof options === 'function') (callback = options), (options = {}); options = Object.assign({}, options); - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - return executeOperation( this.s.topology, new ReplaceOneOperation(this, filter, replacement, options), @@ -510,12 +492,6 @@ export class Collection implements OperationParent { if (typeof options === 'function') (callback = options), (options = {}); options = Object.assign({}, options); - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - return executeOperation( this.s.topology, new UpdateManyOperation(this, filter, update, options), @@ -542,12 +518,6 @@ export class Collection implements OperationParent { if (typeof options === 'function') (callback = options), (options = {}); options = Object.assign({}, options); - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - return executeOperation( this.s.topology, new DeleteOneOperation(this, filter, options), @@ -586,12 +556,6 @@ export class Collection implements OperationParent { options = Object.assign({}, options); - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - return executeOperation( this.s.topology, new DeleteManyOperation(this, filter, options), diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 9656cefd860..f4c572fa323 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -1,5 +1,6 @@ import { applyRetryableWrites, applyWriteConcern, Callback } from '../utils'; import { OperationBase } from './operation'; +import { inheritOrDefaultBSONSerializableOptions } from '../bson'; import { WriteConcern } from '../write_concern'; import type { Collection } from '../collection'; import type { @@ -24,18 +25,15 @@ export class BulkWriteOperation extends OperationBase): void { const coll = this.collection; const operations = this.operations; - let options = this.options; - - // Add ignoreUndefined - if (coll.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = coll.s.options.ignoreUndefined; - } + const options = this.options; // Create the bulk operation const bulk: BulkOperationBase = @@ -53,7 +51,7 @@ export class BulkWriteOperation extends OperationBase): void; @@ -98,7 +111,7 @@ export abstract class CommandOperation< // TODO: consider making this a non-enumerable property this.server = server; - const options = this.options; + const options = Object.assign({}, this.options, this.bsonOptions); const serverWireVersion = maxWireVersion(server); const inTransaction = this.session && this.session.inTransaction(); diff --git a/src/operations/delete.ts b/src/operations/delete.ts index a36a03e8dbd..a856539ea14 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -65,7 +65,7 @@ export class DeleteOneOperation extends CommandOperation): void { const coll = this.collection; const filter = this.filter; - const options = this.options; + const options = Object.assign({}, this.options, this.bsonOptions); options.single = true; removeDocuments(server, coll, filter, options, (err, r) => { @@ -99,7 +99,7 @@ export class DeleteManyOperation extends CommandOperation): void { const coll = this.collection; const filter = this.filter; - const options = this.options; + const options = Object.assign({}, this.options, this.bsonOptions); // a user can pass `single: true` in to `deleteMany` to remove a single document, theoretically if (typeof options.single !== 'boolean') { diff --git a/src/operations/find.ts b/src/operations/find.ts index 55cf558c229..0b1ad968f93 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -128,15 +128,6 @@ export class FindOperation extends CommandOperation { find: this.ns.toString(), query: this.filter }; - - // TODO: figure out our story about inheriting BSON serialization options - this.bsonOptions = { - raw: options.raw ?? collection.s.raw ?? false, - promoteLongs: options.promoteLongs ?? collection.s.promoteLongs ?? true, - promoteValues: options.promoteValues ?? collection.s.promoteValues ?? true, - promoteBuffers: options.promoteBuffers ?? collection.s.promoteBuffers ?? false, - ignoreUndefined: options.ignoreUndefined ?? collection.s.ignoreUndefined ?? false - }; } execute(server: Server, callback: Callback): void { diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 748fd1d8af0..8812c5481d7 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -71,7 +71,7 @@ export class FindAndModifyOperation extends CommandOperation { this.collection = collection; this.query = query; + + // Assign all bsonOptions to OperationBase obj, preferring command options over parent options + Object.assign(this, inheritOrDefaultBSONSerializableOptions(options, collection.s)); } execute(server: Server, callback: Callback): void { const coll = this.collection; const query = this.query; - const options = this.options; + const options = Object.assign({}, this.options, this.bsonOptions); try { const cursor = coll.find(query, options).limit(-1).batchSize(1); diff --git a/src/operations/insert.ts b/src/operations/insert.ts index 772c7ac8c8f..c4d7e14cccb 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -65,7 +65,7 @@ export class InsertOneOperation extends CommandOperation): void { const coll = this.collection; const doc = this.doc; - const options = this.options; + const options = Object.assign({}, this.options, this.bsonOptions); if (Array.isArray(doc)) { return callback( diff --git a/src/operations/insert_many.ts b/src/operations/insert_many.ts index afd87c95780..eeb17117401 100644 --- a/src/operations/insert_many.ts +++ b/src/operations/insert_many.ts @@ -4,7 +4,7 @@ import { MongoError } from '../error'; import { prepareDocs } from './common_functions'; import type { Callback } from '../utils'; import type { Collection } from '../collection'; -import type { ObjectId, Document } from '../bson'; +import { ObjectId, Document, inheritOrDefaultBSONSerializableOptions } from '../bson'; import type { BulkWriteResult, BulkWriteOptions } from '../bulk/common'; import type { Server } from '../sdam/server'; @@ -30,12 +30,15 @@ export class InsertManyOperation extends OperationBase): void { const coll = this.collection; let docs = this.docs; - const options = this.options; + const options = Object.assign({}, this.options, this.bsonOptions); if (!Array.isArray(docs)) { return callback( diff --git a/src/operations/operation.ts b/src/operations/operation.ts index ec012f19e39..1c32f4d4512 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -46,13 +46,32 @@ export abstract class OperationBase< fullResponse?: boolean; // BSON serialization options - bsonOptions?: BSONSerializeOptions; + // fieldsAsRaw?: { [key: string]: boolean }; + promoteValues?: boolean; + promoteBuffers?: boolean; + promoteLongs?: boolean; + // serializeFunctions?: boolean; + ignoreUndefined?: boolean; + raw?: boolean; constructor(options: T = {} as T) { this.options = Object.assign({}, options); this.readPreference = ReadPreference.primary; } + // BSON serialization options + // Have omitted fieldsAsRaw and serializeFunctions because I am not sure if we want those used... + get bsonOptions(): BSONSerializeOptions { + const bsonOptions: Document = { + promoteBuffers: this.promoteBuffers, + promoteValues: this.promoteValues, + promoteLongs: this.promoteLongs, + raw: this.raw, + ignoreUndefined: this.ignoreUndefined + }; + return bsonOptions; + } + abstract execute(server: Server, callback: Callback): void; hasAspect(aspect: symbol): boolean { diff --git a/src/operations/replace_one.ts b/src/operations/replace_one.ts index 50ba6903b2a..aa45d51bc06 100644 --- a/src/operations/replace_one.ts +++ b/src/operations/replace_one.ts @@ -50,7 +50,7 @@ export class ReplaceOneOperation extends CommandOperation Date: Fri, 16 Oct 2020 11:00:01 -0400 Subject: [PATCH 02/16] remove ignore undefined checks from deprecated coll ops and db --- src/collection.ts | 12 ------------ src/db.ts | 5 ----- 2 files changed, 17 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index dfb93686ef3..e734d3d9255 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -1378,12 +1378,6 @@ export class Collection implements OperationParent { if (typeof options === 'function') (callback = options), (options = {}); options = options || {}; - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - return this.updateMany(selector, update, options, callback); } @@ -1403,12 +1397,6 @@ export class Collection implements OperationParent { if (typeof options === 'function') (callback = options), (options = {}); options = options || {}; - // Add ignoreUndefined - if (this.s.options.ignoreUndefined) { - options = Object.assign({}, options); - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - return this.deleteMany(selector, options, callback); } diff --git a/src/db.ts b/src/db.ts index d8c8dab79b7..0cf12dbcdcb 100644 --- a/src/db.ts +++ b/src/db.ts @@ -340,11 +340,6 @@ export class Db implements OperationParent { // If we have not set a collection level readConcern set the db level one options.readConcern = ReadConcern.fromOptions(options) ?? this.readConcern; - // Do we have ignoreUndefined set - if (this.s.options?.ignoreUndefined) { - options.ignoreUndefined = this.s.options.ignoreUndefined; - } - // Merge in all needed options and ensure correct writeConcern merging from db level const finalOptions = mergeOptionsAndWriteConcern( options, From 6a20101163b6d27cd89b4c398233ba0077e4aba6 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Fri, 16 Oct 2020 11:44:26 -0400 Subject: [PATCH 03/16] first step to handle all parent impls --- src/operations/command.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/operations/command.ts b/src/operations/command.ts index 3d05fa8004a..b74e2abe7fa 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -7,7 +7,7 @@ import { commandSupportsReadConcern } from '../sessions'; import { MongoError } from '../error'; import type { Logger } from '../logger'; import type { Server } from '../sdam/server'; -import { Document, inheritOrDefaultBSONSerializableOptions } from '../bson'; +import { BSONSerializeOptions, Document, inheritOrDefaultBSONSerializableOptions } from '../bson'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ReadConcernLike } from './../read_concern'; @@ -45,6 +45,7 @@ export interface OperationParent { promoteValues?: boolean; promoteBuffers?: boolean; ignoreUndefined?: boolean; + options?: BSONSerializeOptions; }; readConcern?: ReadConcern; writeConcern?: WriteConcern; @@ -102,7 +103,8 @@ export abstract class CommandOperation< // Assign all bsonOptions to OperationBase obj, preferring command options over parent options // TODO, for collection it makes sense to take it from parent.s -- is this true for others? // TODO: downside of this is after the command is created these values are *fixed* because they are already set to default/inherited values - Object.assign(this, inheritOrDefaultBSONSerializableOptions(options, parent?.s)); + const base = Object.assign({}, parent?.s.options, parent?.s); + Object.assign(this, inheritOrDefaultBSONSerializableOptions(options, base)); } abstract execute(server: Server, callback: Callback): void; From 14e0cdf26dfbc8ece21af48c5c7e6387a5e6afca Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Mon, 19 Oct 2020 10:18:02 -0400 Subject: [PATCH 04/16] add tests to check bson option inheritance --- src/bson.ts | 2 +- src/operations/command.ts | 3 +- test/functional/ignore_undefined.test.js | 69 ++++++++++++++++++++++++ test/functional/insert.test.js | 63 ++++++++++++++++++++++ 4 files changed, 134 insertions(+), 3 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index b2d2062054a..0f8004392aa 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -78,7 +78,7 @@ export function inheritOrDefaultBSONSerializableOptions( ): BSONSerializeOptions { // Merge the BSONSerializeOptions, preferring options over parentOptions, and substituting a // default for values not set. - // Note that we exclude fieldsAsRaw and serializeFunctions because I was not sure about their usage + // Note that we exclude fieldsAsRaw and serializeFunctions because I was not sure about their usage //TODO: serialize functions? return { raw: options?.raw ?? parentOptions?.raw ?? false, promoteLongs: options?.promoteLongs ?? parentOptions?.promoteLongs ?? true, diff --git a/src/operations/command.ts b/src/operations/command.ts index b74e2abe7fa..5fcdf955ae6 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -101,8 +101,7 @@ export abstract class CommandOperation< } // Assign all bsonOptions to OperationBase obj, preferring command options over parent options - // TODO, for collection it makes sense to take it from parent.s -- is this true for others? - // TODO: downside of this is after the command is created these values are *fixed* because they are already set to default/inherited values + // base accounts for the fact that Collection stores bson options in s, while Db stores bson options in s.options const base = Object.assign({}, parent?.s.options, parent?.s); Object.assign(this, inheritOrDefaultBSONSerializableOptions(options, base)); } diff --git a/test/functional/ignore_undefined.test.js b/test/functional/ignore_undefined.test.js index 35f20fdc481..06cc61b08db 100644 --- a/test/functional/ignore_undefined.test.js +++ b/test/functional/ignore_undefined.test.js @@ -38,6 +38,75 @@ describe('Ignore Undefined', function () { } }); + it('Should correctly inherit ignore undefined field from collection during insert', { + metadata: { requires: { topology: ['single'] } }, + + test: function (done) { + var configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1, + ignoreUndefined: false + }); + + client.connect(function (err, client) { + var db = client.db(configuration.db); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue', { + ignoreUndefined: true + }); + + // Ignore the undefined field + collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), function (err) { + expect(err).to.not.exist; + + // Locate the doument + collection.findOne(function (err, item) { + test.equal(1, item.a); + test.ok(item.b === undefined); + client.close(done); + }); + }); + }); + } + }); + + it('Should correctly inherit ignore undefined field from operation during findOneAndReplace', { + metadata: { requires: { topology: ['single'] } }, + + test: function (done) { + var configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1, + ignoreUndefined: false + }); + + client.connect(function (err, client) { + var db = client.db(configuration.db); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue'); + + collection.insert({ a: 1, b: 2 }, configuration.writeConcernMax(), function (err) { + expect(err).to.not.exist; + + // Replace the doument, ignoring undefined fields + collection.findOneAndReplace( + {}, + { a: 1, b: undefined }, + { ignoreUndefined: true }, + function (err) { + expect(err).to.not.exist; + + // Locate the doument + collection.findOne(function (err, item) { + test.equal(1, item.a); + test.ok(item.b === undefined); + client.close(done); + }); + } + ); + }); + }); + } + }); + it( 'Should correctly connect using MongoClient and perform insert document ignoring undefined field', { diff --git a/test/functional/insert.test.js b/test/functional/insert.test.js index 78387639a87..0c2f7ac55bb 100644 --- a/test/functional/insert.test.js +++ b/test/functional/insert.test.js @@ -1902,6 +1902,69 @@ describe('Insert', function () { } }); + it('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore', { + // Add a tag that our runner can trigger on + // in this case we are setting that node needs to be higher than 0.10.X to run + metadata: { + requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } + }, + + test: function (done) { + var configuration = this.configuration; + + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1, + promoteLongs: true + }); + client.connect(function (err, client) { + var db = client.db(configuration.db); + db.collection('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore').insertMany( + [ + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) } + ], + function (err, doc) { + expect(err).to.not.exist; + test.ok(doc); + + db.collection('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore') + .find({}, { promoteLongs: false }) + .batchSize(2) + .toArray(function (err, docs) { + expect(err).to.not.exist; + var doc = docs.pop(); + + test.ok(doc.a._bsontype === 'Long'); + client.close(done); + }); + } + ); + }); + } + }); + it('shouldCorrectlyHonorPromoteLongTrueNativeBSON', { // Add a tag that our runner can trigger on // in this case we are setting that node needs to be higher than 0.10.X to run From f30ca602c215e7f992662684f59a5e212a577fbd Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Mon, 19 Oct 2020 10:43:26 -0400 Subject: [PATCH 05/16] include serialize fns and fields as raw in bson opts --- src/bson.ts | 5 +++-- src/operations/bulk_write.ts | 4 ++-- src/operations/find_and_modify.ts | 4 ---- src/operations/insert_many.ts | 3 --- src/operations/operation.ts | 9 +++++---- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index 0f8004392aa..4e866ae4d33 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -78,12 +78,13 @@ export function inheritOrDefaultBSONSerializableOptions( ): BSONSerializeOptions { // Merge the BSONSerializeOptions, preferring options over parentOptions, and substituting a // default for values not set. - // Note that we exclude fieldsAsRaw and serializeFunctions because I was not sure about their usage //TODO: serialize functions? return { raw: options?.raw ?? parentOptions?.raw ?? false, promoteLongs: options?.promoteLongs ?? parentOptions?.promoteLongs ?? true, promoteValues: options?.promoteValues ?? parentOptions?.promoteValues ?? true, promoteBuffers: options?.promoteBuffers ?? parentOptions?.promoteBuffers ?? false, - ignoreUndefined: options?.ignoreUndefined ?? parentOptions?.ignoreUndefined ?? false + ignoreUndefined: options?.ignoreUndefined ?? parentOptions?.ignoreUndefined ?? false, + serializeFunctions: options?.serializeFunctions ?? parentOptions?.serializeFunctions ?? false, + fieldsAsRaw: options?.fieldsAsRaw ?? parentOptions?.fieldsAsRaw ?? {} }; } diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index f4c572fa323..20cf4903ae7 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -33,7 +33,7 @@ export class BulkWriteOperation extends OperationBase): void { const coll = this.collection; const operations = this.operations; - const options = this.options; + const options = Object.assign({}, this.options, this.bsonOptions); // Create the bulk operation const bulk: BulkOperationBase = @@ -51,7 +51,7 @@ export class BulkWriteOperation extends OperationBase Date: Mon, 19 Oct 2020 12:45:32 -0400 Subject: [PATCH 06/16] small clean up and more tests --- src/bson.ts | 6 +- src/operations/command.ts | 5 +- test/functional/ignore_undefined.test.js | 229 ++++++++++++++++------- test/functional/insert.test.js | 4 +- 4 files changed, 167 insertions(+), 77 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index 4e866ae4d33..b8fdc433a78 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -70,14 +70,12 @@ export function pluckBSONSerializeOptions(options: BSONSerializeOptions): BSONSe }; } -// TODO: naming -// TODO: testing +// Merge the given BSONSerializeOptions, preferring options over parentOptions, and substituting a +// default for values not set. export function inheritOrDefaultBSONSerializableOptions( options?: BSONSerializeOptions, parentOptions?: BSONSerializeOptions ): BSONSerializeOptions { - // Merge the BSONSerializeOptions, preferring options over parentOptions, and substituting a - // default for values not set. return { raw: options?.raw ?? parentOptions?.raw ?? false, promoteLongs: options?.promoteLongs ?? parentOptions?.promoteLongs ?? true, diff --git a/src/operations/command.ts b/src/operations/command.ts index 5fcdf955ae6..1bf2ca0658a 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -100,8 +100,9 @@ export abstract class CommandOperation< this.logger = parent.logger; } - // Assign all bsonOptions to OperationBase obj, preferring command options over parent options - // base accounts for the fact that Collection stores bson options in s, while Db stores bson options in s.options + // Assign all bsonOptions to OperationBase obj, preferring command options over parent options. + // base accounts for the fact that Collection stores bson options in s, while other parents, + // like Db, stores bson options in s.options const base = Object.assign({}, parent?.s.options, parent?.s); Object.assign(this, inheritOrDefaultBSONSerializableOptions(options, base)); } diff --git a/test/functional/ignore_undefined.test.js b/test/functional/ignore_undefined.test.js index 06cc61b08db..95e7d06be54 100644 --- a/test/functional/ignore_undefined.test.js +++ b/test/functional/ignore_undefined.test.js @@ -38,75 +38,6 @@ describe('Ignore Undefined', function () { } }); - it('Should correctly inherit ignore undefined field from collection during insert', { - metadata: { requires: { topology: ['single'] } }, - - test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1, - ignoreUndefined: false - }); - - client.connect(function (err, client) { - var db = client.db(configuration.db); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue', { - ignoreUndefined: true - }); - - // Ignore the undefined field - collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), function (err) { - expect(err).to.not.exist; - - // Locate the doument - collection.findOne(function (err, item) { - test.equal(1, item.a); - test.ok(item.b === undefined); - client.close(done); - }); - }); - }); - } - }); - - it('Should correctly inherit ignore undefined field from operation during findOneAndReplace', { - metadata: { requires: { topology: ['single'] } }, - - test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1, - ignoreUndefined: false - }); - - client.connect(function (err, client) { - var db = client.db(configuration.db); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue'); - - collection.insert({ a: 1, b: 2 }, configuration.writeConcernMax(), function (err) { - expect(err).to.not.exist; - - // Replace the doument, ignoring undefined fields - collection.findOneAndReplace( - {}, - { a: 1, b: undefined }, - { ignoreUndefined: true }, - function (err) { - expect(err).to.not.exist; - - // Locate the doument - collection.findOne(function (err, item) { - test.equal(1, item.a); - test.ok(item.b === undefined); - client.close(done); - }); - } - ); - }); - }); - } - }); - it( 'Should correctly connect using MongoClient and perform insert document ignoring undefined field', { @@ -217,4 +148,164 @@ describe('Ignore Undefined', function () { }); } }); + + it('Should correctly inherit ignore undefined field from db during insert', { + metadata: { requires: { topology: ['single'] } }, + + test: function (done) { + var configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1, + ignoreUndefined: false + }); + + client.connect(function (err, client) { + var db = client.db(configuration.db, { ignoreUndefined: true }); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue3'); + + // Ignore the undefined field + collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), function (err) { + expect(err).to.not.exist; + + // Locate the doument + collection.findOne(function (err, item) { + test.equal(1, item.a); + test.ok(item.b === undefined); + client.close(done); + }); + }); + }); + } + }); + + it('Should correctly inherit ignore undefined field from collection during insert', { + metadata: { requires: { topology: ['single'] } }, + + test: function (done) { + var configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1 + }); + + client.connect(function (err, client) { + var db = client.db(configuration.db, { ignoreUndefined: false }); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue4', { + ignoreUndefined: true + }); + + // Ignore the undefined field + collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), function (err) { + expect(err).to.not.exist; + + // Locate the doument + collection.findOne(function (err, item) { + test.equal(1, item.a); + test.ok(item.b === undefined); + client.close(done); + }); + }); + }); + } + }); + + it('Should correctly inherit ignore undefined field from operation during insert', { + metadata: { requires: { topology: ['single'] } }, + + test: function (done) { + var configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1 + }); + + client.connect(function (err, client) { + var db = client.db(configuration.db); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue5', { + ignoreUndefined: false + }); + + // Ignore the undefined field + collection.insert({ a: 1, b: undefined }, { ignoreUndefined: true }, function (err) { + expect(err).to.not.exist; + + // Locate the doument + collection.findOne({}, function (err, item) { + expect(err).to.not.exist; + test.equal(1, item.a); + test.ok(item.b === undefined); + client.close(done); + }); + }); + }); + } + }); + + it('Should correctly inherit ignore undefined field from operation during findOneAndReplace', { + metadata: { requires: { topology: ['single'] } }, + + test: function (done) { + var configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1, + ignoreUndefined: false + }); + + client.connect(function (err, client) { + var db = client.db(configuration.db); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue6'); + + collection.insert({ a: 1, b: 2 }, configuration.writeConcernMax(), function (err) { + expect(err).to.not.exist; + + // Replace the doument, ignoring undefined fields + collection.findOneAndReplace( + {}, + { a: 1, b: undefined }, + { ignoreUndefined: true }, + function (err) { + expect(err).to.not.exist; + + // Locate the doument + collection.findOne(function (err, item) { + test.equal(1, item.a); + test.ok(item.b === undefined); + client.close(done); + }); + } + ); + }); + }); + } + }); + + it('Should correctly ignore undefined field during bulk write', { + metadata: { requires: { topology: ['single'] } }, + + test: function (done) { + var configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1 + }); + + client.connect(function (err, client) { + var db = client.db(configuration.db); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue7'); + + // Ignore the undefined field + collection.bulkWrite( + [{ insertOne: { a: 0, b: undefined } }], + { ignoreUndefined: true }, + function (err) { + expect(err).to.not.exist; + + // Locate the doument + collection.findOne(function (err, item) { + test.equal(0, item.a); + test.ok(item.b === undefined); + client.close(done); + }); + } + ); + }); + } + }); }); diff --git a/test/functional/insert.test.js b/test/functional/insert.test.js index 0c2f7ac55bb..92e1b5a3677 100644 --- a/test/functional/insert.test.js +++ b/test/functional/insert.test.js @@ -1917,7 +1917,7 @@ describe('Insert', function () { promoteLongs: true }); client.connect(function (err, client) { - var db = client.db(configuration.db); + var db = client.db(configuration.db, { promoteLongs: false }); db.collection('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore').insertMany( [ { a: Long.fromNumber(10) }, @@ -1950,7 +1950,7 @@ describe('Insert', function () { test.ok(doc); db.collection('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore') - .find({}, { promoteLongs: false }) + .find({}) .batchSize(2) .toArray(function (err, docs) { expect(err).to.not.exist; From dcffd13916bb96d473f5d12a8f33aa9c26b3e03b Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Mon, 19 Oct 2020 15:09:30 -0400 Subject: [PATCH 07/16] small cleanup --- src/bson.ts | 4 ++-- src/operations/bulk_write.ts | 2 +- src/operations/command.ts | 4 ++-- src/operations/find_one.ts | 2 +- src/operations/insert_many.ts | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index b8fdc433a78..bb618e3658d 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -70,8 +70,8 @@ export function pluckBSONSerializeOptions(options: BSONSerializeOptions): BSONSe }; } -// Merge the given BSONSerializeOptions, preferring options over parentOptions, and substituting a -// default for values not set. +// Merge the given BSONSerializeOptions, preferring options over parentOptions, and substituting +// defaults for values not set. export function inheritOrDefaultBSONSerializableOptions( options?: BSONSerializeOptions, parentOptions?: BSONSerializeOptions diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 20cf4903ae7..2f2e0e8e481 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -26,7 +26,7 @@ export class BulkWriteOperation extends OperationBase { this.collection = collection; this.query = query; - // Assign all bsonOptions to OperationBase obj, preferring command options over parent options + // Assign BSON serialize options to OperationBase, preferring options over collection options Object.assign(this, inheritOrDefaultBSONSerializableOptions(options, collection.s)); } diff --git a/src/operations/insert_many.ts b/src/operations/insert_many.ts index f19493bf112..8d90227a7c4 100644 --- a/src/operations/insert_many.ts +++ b/src/operations/insert_many.ts @@ -31,7 +31,7 @@ export class InsertManyOperation extends OperationBase Date: Mon, 19 Oct 2020 16:30:32 -0400 Subject: [PATCH 08/16] use withClient for some tests --- test/functional/ignore_undefined.test.js | 187 +++++++++-------------- test/functional/insert.test.js | 13 +- 2 files changed, 83 insertions(+), 117 deletions(-) diff --git a/test/functional/ignore_undefined.test.js b/test/functional/ignore_undefined.test.js index 95e7d06be54..14e8b5c2d8b 100644 --- a/test/functional/ignore_undefined.test.js +++ b/test/functional/ignore_undefined.test.js @@ -3,6 +3,7 @@ var test = require('./shared').assert; const { expect } = require('chai'); var setupDatabase = require('./shared').setupDatabase; const { ObjectId } = require('../../src'); +const withClient = require('./shared').withClient; describe('Ignore Undefined', function () { before(function () { @@ -152,23 +153,23 @@ describe('Ignore Undefined', function () { it('Should correctly inherit ignore undefined field from db during insert', { metadata: { requires: { topology: ['single'] } }, - test: function (done) { + test: function () { var configuration = this.configuration; var client = configuration.newClient(configuration.writeConcernMax(), { poolSize: 1, ignoreUndefined: false }); - client.connect(function (err, client) { + return withClient(client, (client, done) => { var db = client.db(configuration.db, { ignoreUndefined: true }); var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue3'); // Ignore the undefined field - collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), function (err) { + collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), err => { expect(err).to.not.exist; // Locate the doument - collection.findOne(function (err, item) { + collection.findOne((err, item) => { test.equal(1, item.a); test.ok(item.b === undefined); client.close(done); @@ -178,134 +179,98 @@ describe('Ignore Undefined', function () { } }); - it('Should correctly inherit ignore undefined field from collection during insert', { - metadata: { requires: { topology: ['single'] } }, - - test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1 + it( + 'Should correctly inherit ignore undefined field from collection during insert', + withClient(function (client, done) { + var db = client.db('shouldCorrectlyIgnoreUndefinedValue4', { ignoreUndefined: false }); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue4', { + ignoreUndefined: true }); - client.connect(function (err, client) { - var db = client.db(configuration.db, { ignoreUndefined: false }); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue4', { - ignoreUndefined: true - }); - - // Ignore the undefined field - collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), function (err) { - expect(err).to.not.exist; + // Ignore the undefined field + collection.insert({ a: 1, b: undefined }, err => { + expect(err).to.not.exist; - // Locate the doument - collection.findOne(function (err, item) { - test.equal(1, item.a); - test.ok(item.b === undefined); - client.close(done); - }); + // Locate the doument + collection.findOne((err, item) => { + test.equal(1, item.a); + test.ok(item.b === undefined); + done(); }); }); - } - }); - - it('Should correctly inherit ignore undefined field from operation during insert', { - metadata: { requires: { topology: ['single'] } }, + }) + ); - test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1 + it( + 'Should correctly inherit ignore undefined field from operation during insert', + withClient(function (client, done) { + var db = client.db('shouldCorrectlyIgnoreUndefinedValue5'); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue5', { + ignoreUndefined: false }); - client.connect(function (err, client) { - var db = client.db(configuration.db); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue5', { - ignoreUndefined: false - }); + // Ignore the undefined field + collection.insert({ a: 1, b: undefined }, { ignoreUndefined: true }, err => { + expect(err).to.not.exist; - // Ignore the undefined field - collection.insert({ a: 1, b: undefined }, { ignoreUndefined: true }, function (err) { + // Locate the doument + collection.findOne({}, (err, item) => { expect(err).to.not.exist; - - // Locate the doument - collection.findOne({}, function (err, item) { - expect(err).to.not.exist; - test.equal(1, item.a); - test.ok(item.b === undefined); - client.close(done); - }); + test.equal(1, item.a); + test.ok(item.b === undefined); + done(); }); }); - } - }); - - it('Should correctly inherit ignore undefined field from operation during findOneAndReplace', { - metadata: { requires: { topology: ['single'] } }, + }) + ); - test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1, + it( + 'Should correctly inherit ignore undefined field from operation during findOneAndReplace', + withClient(function (client, done) { + var db = client.db('shouldCorrectlyIgnoreUndefinedValue6'); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue6', { ignoreUndefined: false }); - client.connect(function (err, client) { - var db = client.db(configuration.db); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue6'); + collection.insert({ a: 1, b: 2 }, err => { + expect(err).to.not.exist; - collection.insert({ a: 1, b: 2 }, configuration.writeConcernMax(), function (err) { + // Replace the doument, ignoring undefined fields + collection.findOneAndReplace({}, { a: 1, b: undefined }, { ignoreUndefined: true }, err => { expect(err).to.not.exist; - // Replace the doument, ignoring undefined fields - collection.findOneAndReplace( - {}, - { a: 1, b: undefined }, - { ignoreUndefined: true }, - function (err) { - expect(err).to.not.exist; - - // Locate the doument - collection.findOne(function (err, item) { - test.equal(1, item.a); - test.ok(item.b === undefined); - client.close(done); - }); - } - ); + // Locate the doument + collection.findOne((err, item) => { + test.equal(1, item.a); + test.ok(item.b === undefined); + done(); + }); }); }); - } - }); - - it('Should correctly ignore undefined field during bulk write', { - metadata: { requires: { topology: ['single'] } }, - - test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1 - }); - - client.connect(function (err, client) { - var db = client.db(configuration.db); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue7'); + }) + ); - // Ignore the undefined field - collection.bulkWrite( - [{ insertOne: { a: 0, b: undefined } }], - { ignoreUndefined: true }, - function (err) { - expect(err).to.not.exist; + it( + 'Should correctly ignore undefined field during bulk write', + withClient(function (client, done) { + var db = client.db('shouldCorrectlyIgnoreUndefinedValue7'); + var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue7'); + + // Ignore the undefined field + collection.bulkWrite( + [{ insertOne: { a: 0, b: undefined } }], + { ignoreUndefined: true }, + err => { + expect(err).to.not.exist; - // Locate the doument - collection.findOne(function (err, item) { - test.equal(0, item.a); - test.ok(item.b === undefined); - client.close(done); - }); - } - ); - }); - } - }); + // Locate the doument + collection.findOne((err, item) => { + test.equal(0, item.a); + test.ok(item.b === undefined); + done(); + }); + } + ); + }) + ); }); diff --git a/test/functional/insert.test.js b/test/functional/insert.test.js index 92e1b5a3677..6b356198876 100644 --- a/test/functional/insert.test.js +++ b/test/functional/insert.test.js @@ -1,5 +1,5 @@ 'use strict'; -const { assert: test } = require('./shared'); +const { assert: test, withClient } = require('./shared'); const { setupDatabase } = require('./shared'); const Script = require('vm'); const { expect } = require('chai'); @@ -1909,14 +1909,15 @@ describe('Insert', function () { requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } }, - test: function (done) { + test: function () { var configuration = this.configuration; var client = configuration.newClient(configuration.writeConcernMax(), { poolSize: 1, promoteLongs: true }); - client.connect(function (err, client) { + + return withClient(client, (client, done) => { var db = client.db(configuration.db, { promoteLongs: false }); db.collection('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore').insertMany( [ @@ -1945,19 +1946,19 @@ describe('Insert', function () { { a: Long.fromNumber(10) }, { a: Long.fromNumber(10) } ], - function (err, doc) { + (err, doc) => { expect(err).to.not.exist; test.ok(doc); db.collection('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore') .find({}) .batchSize(2) - .toArray(function (err, docs) { + .toArray((err, docs) => { expect(err).to.not.exist; var doc = docs.pop(); test.ok(doc.a._bsontype === 'Long'); - client.close(done); + done(); }); } ); From 72f6794d9f6aeec1afd58dd88340e8ba5855a011 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 20 Oct 2020 09:20:29 -0400 Subject: [PATCH 09/16] fix utils bug and test --- src/utils.ts | 2 +- test/functional/ignore_undefined.test.js | 4 +- test/functional/insert.test.js | 97 ++++++++++++------------ 3 files changed, 50 insertions(+), 53 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 8275ed8d13c..268601be0d5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -276,7 +276,7 @@ export function mergeOptionsAndWriteConcern( ): AnyOptions { // Mix in any allowed options for (let i = 0; i < keys.length; i++) { - if (!targetOptions[keys[i]] && sourceOptions[keys[i]] !== undefined) { + if (targetOptions[keys[i]] === undefined && sourceOptions[keys[i]] !== undefined) { targetOptions[keys[i]] = sourceOptions[keys[i]]; } } diff --git a/test/functional/ignore_undefined.test.js b/test/functional/ignore_undefined.test.js index 14e8b5c2d8b..38280dbb2d5 100644 --- a/test/functional/ignore_undefined.test.js +++ b/test/functional/ignore_undefined.test.js @@ -153,14 +153,14 @@ describe('Ignore Undefined', function () { it('Should correctly inherit ignore undefined field from db during insert', { metadata: { requires: { topology: ['single'] } }, - test: function () { + test: function (done) { var configuration = this.configuration; var client = configuration.newClient(configuration.writeConcernMax(), { poolSize: 1, ignoreUndefined: false }); - return withClient(client, (client, done) => { + client.connect(function (err, client) { var db = client.db(configuration.db, { ignoreUndefined: true }); var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue3'); diff --git a/test/functional/insert.test.js b/test/functional/insert.test.js index 6b356198876..b6300de817a 100644 --- a/test/functional/insert.test.js +++ b/test/functional/insert.test.js @@ -1909,61 +1909,58 @@ describe('Insert', function () { requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } }, - test: function () { - var configuration = this.configuration; - - var client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1, + test: withClient((client, done) => { + const db = client.db('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore', { promoteLongs: true }); + const collection = db.collection('test', { promoteLongs: false }); + collection.insertMany( + [ + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) }, + { a: Long.fromNumber(10) } + ], + (err, doc) => { + expect(err).to.not.exist; + test.ok(doc); - return withClient(client, (client, done) => { - var db = client.db(configuration.db, { promoteLongs: false }); - db.collection('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore').insertMany( - [ - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) }, - { a: Long.fromNumber(10) } - ], - (err, doc) => { - expect(err).to.not.exist; - test.ok(doc); - - db.collection('shouldCorrectlyInheritPromoteLongFalseNativeBSONWithGetMore') - .find({}) - .batchSize(2) - .toArray((err, docs) => { - expect(err).to.not.exist; - var doc = docs.pop(); + collection + .find({}) + .batchSize(2) + .toArray((err, docs) => { + expect(err).to.not.exist; - test.ok(doc.a._bsontype === 'Long'); - done(); + docs.forEach((d, i) => { + expect(d.a, `Failed on the document at index ${i}`).to.not.be.a('number'); + expect(d.a, `Failed on the document at index ${i}`).to.have.property('_bsontype'); + expect(d.a._bsontype, `Failed on the document at index ${i}`).to.be.equal('Long'); }); - } - ); - }); - } + done(); + }); + } + ); + }) }); it('shouldCorrectlyHonorPromoteLongTrueNativeBSON', { From 8a75dc8615c17d3cad95ec10129ec93f055c600d Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 20 Oct 2020 14:55:03 -0400 Subject: [PATCH 10/16] rename helper and add docs comments --- src/bson.ts | 9 ++++++--- src/operations/bulk_write.ts | 6 +++--- src/operations/command.ts | 9 ++++----- src/operations/delete.ts | 4 ++-- src/operations/find_and_modify.ts | 2 +- src/operations/find_one.ts | 6 +++--- src/operations/insert.ts | 2 +- src/operations/insert_many.ts | 6 +++--- src/operations/replace_one.ts | 2 +- src/operations/update.ts | 4 ++-- test/functional/insert.test.js | 2 -- 11 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index bb618e3658d..2554cd722af 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -70,9 +70,12 @@ export function pluckBSONSerializeOptions(options: BSONSerializeOptions): BSONSe }; } -// Merge the given BSONSerializeOptions, preferring options over parentOptions, and substituting -// defaults for values not set. -export function inheritOrDefaultBSONSerializableOptions( +/** + * Merge the given BSONSerializeOptions, preferring options over parentOptions, and substituting + * defaults for values not set. + * @internal + */ +export function inheritBSONOptions( options?: BSONSerializeOptions, parentOptions?: BSONSerializeOptions ): BSONSerializeOptions { diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 2f2e0e8e481..1f0f46349a9 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -1,6 +1,6 @@ import { applyRetryableWrites, applyWriteConcern, Callback } from '../utils'; import { OperationBase } from './operation'; -import { inheritOrDefaultBSONSerializableOptions } from '../bson'; +import { inheritBSONOptions } from '../bson'; import { WriteConcern } from '../write_concern'; import type { Collection } from '../collection'; import type { @@ -27,13 +27,13 @@ export class BulkWriteOperation extends OperationBase): void { const coll = this.collection; const operations = this.operations; - const options = Object.assign({}, this.options, this.bsonOptions); + const options = { ...this.options, ...this.bsonOptions }; // Create the bulk operation const bulk: BulkOperationBase = diff --git a/src/operations/command.ts b/src/operations/command.ts index 2ba0449726d..eaa6cbb4d72 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -7,7 +7,7 @@ import { commandSupportsReadConcern } from '../sessions'; import { MongoError } from '../error'; import type { Logger } from '../logger'; import type { Server } from '../sdam/server'; -import { BSONSerializeOptions, Document, inheritOrDefaultBSONSerializableOptions } from '../bson'; +import { BSONSerializeOptions, Document, inheritBSONOptions } from '../bson'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ReadConcernLike } from './../read_concern'; @@ -101,10 +101,9 @@ export abstract class CommandOperation< } // Assign BSON serialize options to OperationBase, preferring options over parent options. - // base accounts for the fact that Collection stores bson options in s, while other parents, + // Here, we account for the fact that Collection stores bson options in s, while other parents, // like Db, stores bson options in s.options - const base = Object.assign({}, parent?.s.options, parent?.s); - Object.assign(this, inheritOrDefaultBSONSerializableOptions(options, base)); + Object.assign(this, inheritBSONOptions(options, { ...parent?.s.options, ...parent?.s })); } abstract execute(server: Server, callback: Callback): void; @@ -113,7 +112,7 @@ export abstract class CommandOperation< // TODO: consider making this a non-enumerable property this.server = server; - const options = Object.assign({}, this.options, this.bsonOptions); + const options = { ...this.options, ...this.bsonOptions }; const serverWireVersion = maxWireVersion(server); const inTransaction = this.session && this.session.inTransaction(); diff --git a/src/operations/delete.ts b/src/operations/delete.ts index a856539ea14..5ed6b750b11 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -65,7 +65,7 @@ export class DeleteOneOperation extends CommandOperation): void { const coll = this.collection; const filter = this.filter; - const options = Object.assign({}, this.options, this.bsonOptions); + const options = { ...this.options, ...this.bsonOptions }; options.single = true; removeDocuments(server, coll, filter, options, (err, r) => { @@ -99,7 +99,7 @@ export class DeleteManyOperation extends CommandOperation): void { const coll = this.collection; const filter = this.filter; - const options = Object.assign({}, this.options, this.bsonOptions); + const options = { ...this.options, ...this.bsonOptions }; // a user can pass `single: true` in to `deleteMany` to remove a single document, theoretically if (typeof options.single !== 'boolean') { diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 19ca8ac41dd..aa5545638d0 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -71,7 +71,7 @@ export class FindAndModifyOperation extends CommandOperation { this.query = query; // Assign BSON serialize options to OperationBase, preferring options over collection options - Object.assign(this, inheritOrDefaultBSONSerializableOptions(options, collection.s)); + Object.assign(this, inheritBSONOptions(options, collection.s)); } execute(server: Server, callback: Callback): void { const coll = this.collection; const query = this.query; - const options = Object.assign({}, this.options, this.bsonOptions); + const options = { ...this.options, ...this.bsonOptions }; try { const cursor = coll.find(query, options).limit(-1).batchSize(1); diff --git a/src/operations/insert.ts b/src/operations/insert.ts index c4d7e14cccb..58b84cb09da 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -65,7 +65,7 @@ export class InsertOneOperation extends CommandOperation): void { const coll = this.collection; const doc = this.doc; - const options = Object.assign({}, this.options, this.bsonOptions); + const options = { ...this.options, ...this.bsonOptions }; if (Array.isArray(doc)) { return callback( diff --git a/src/operations/insert_many.ts b/src/operations/insert_many.ts index 8d90227a7c4..5edefb095e3 100644 --- a/src/operations/insert_many.ts +++ b/src/operations/insert_many.ts @@ -4,7 +4,7 @@ import { MongoError } from '../error'; import { prepareDocs } from './common_functions'; import type { Callback } from '../utils'; import type { Collection } from '../collection'; -import { ObjectId, Document, inheritOrDefaultBSONSerializableOptions } from '../bson'; +import { ObjectId, Document, inheritBSONOptions } from '../bson'; import type { BulkWriteResult, BulkWriteOptions } from '../bulk/common'; import type { Server } from '../sdam/server'; @@ -32,13 +32,13 @@ export class InsertManyOperation extends OperationBase): void { const coll = this.collection; let docs = this.docs; - const options = Object.assign({}, this.options, this.bsonOptions); + const options = { ...this.options, ...this.bsonOptions }; if (!Array.isArray(docs)) { return callback( diff --git a/src/operations/replace_one.ts b/src/operations/replace_one.ts index aa45d51bc06..d27e02f151e 100644 --- a/src/operations/replace_one.ts +++ b/src/operations/replace_one.ts @@ -50,7 +50,7 @@ export class ReplaceOneOperation extends CommandOperation Date: Wed, 21 Oct 2020 10:49:23 -0400 Subject: [PATCH 11/16] move collection bson opts to s.options --- src/bson.ts | 31 +++++++++++++++++---------- src/collection.ts | 34 +++++------------------------- src/operations/bulk_write.ts | 2 +- src/operations/command.ts | 10 +-------- src/operations/common_functions.ts | 2 +- src/operations/find_one.ts | 2 +- src/operations/insert.ts | 1 - src/operations/insert_many.ts | 2 +- 8 files changed, 30 insertions(+), 54 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index 2554cd722af..3f7320b6e8d 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -71,21 +71,30 @@ export function pluckBSONSerializeOptions(options: BSONSerializeOptions): BSONSe } /** - * Merge the given BSONSerializeOptions, preferring options over parentOptions, and substituting - * defaults for values not set. + * Merge the given BSONSerializeOptions, preferring options over parentOptions, and optionally + * substituting defaults for values not set. + * * @internal + * + * @param options - The primary options to use + * @param parentOptions - The secondary options to use + * @param includeDefaultOptions - Whether to include default options for any values not set */ export function inheritBSONOptions( options?: BSONSerializeOptions, - parentOptions?: BSONSerializeOptions + parentOptions?: BSONSerializeOptions, + includeDefaultOptions?: boolean ): BSONSerializeOptions { - return { - raw: options?.raw ?? parentOptions?.raw ?? false, - promoteLongs: options?.promoteLongs ?? parentOptions?.promoteLongs ?? true, - promoteValues: options?.promoteValues ?? parentOptions?.promoteValues ?? true, - promoteBuffers: options?.promoteBuffers ?? parentOptions?.promoteBuffers ?? false, - ignoreUndefined: options?.ignoreUndefined ?? parentOptions?.ignoreUndefined ?? false, - serializeFunctions: options?.serializeFunctions ?? parentOptions?.serializeFunctions ?? false, - fieldsAsRaw: options?.fieldsAsRaw ?? parentOptions?.fieldsAsRaw ?? {} + const defaults = { + raw: false, + promoteLongs: true, + promoteValues: true, + promoteBuffers: false, + ignoreUndefined: false, + serializeFunctions: false, + fieldsAsRaw: {} }; + + const base = includeDefaultOptions ? defaults : {}; + return pluckBSONSerializeOptions({ ...base, ...parentOptions, ...options }); } diff --git a/src/collection.ts b/src/collection.ts index e734d3d9255..3a679a9c617 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -8,7 +8,7 @@ import { MongoDBNamespace, Callback } from './utils'; -import { ObjectId, Document, BSONSerializeOptions } from './bson'; +import { ObjectId, Document, BSONSerializeOptions, inheritBSONOptions } from './bson'; import { MongoError } from './error'; import { UnorderedBulkOperation } from './bulk/unordered'; import { OrderedBulkOperation } from './bulk/ordered'; @@ -127,12 +127,6 @@ export interface CollectionPrivate { namespace: MongoDBNamespace; readPreference?: ReadPreference; slaveOk?: boolean; - serializeFunctions?: boolean; - raw?: boolean; - promoteLongs?: boolean; - promoteValues?: boolean; - promoteBuffers?: boolean; - ignoreUndefined?: boolean; collectionHint?: Hint; readConcern?: ReadConcern; writeConcern?: WriteConcern; @@ -190,29 +184,11 @@ export class Collection implements OperationParent { readPreference: ReadPreference.fromOptions(options), readConcern: ReadConcern.fromOptions(options), writeConcern: WriteConcern.fromOptions(options), - slaveOk: options == null || options.slaveOk == null ? db.slaveOk : options.slaveOk, - serializeFunctions: - options == null || options.serializeFunctions == null - ? db.s.options?.serializeFunctions - : options.serializeFunctions, - raw: options == null || options.raw == null ? db.s.options?.raw : options.raw, - promoteLongs: - options == null || options.promoteLongs == null - ? db.s.options?.promoteLongs - : options.promoteLongs, - promoteValues: - options == null || options.promoteValues == null - ? db.s.options?.promoteValues - : options.promoteValues, - promoteBuffers: - options == null || options.promoteBuffers == null - ? db.s.options?.promoteBuffers - : options.promoteBuffers, - ignoreUndefined: - options == null || options.ignoreUndefined == null - ? db.s.options?.ignoreUndefined - : options.ignoreUndefined + slaveOk: options == null || options.slaveOk == null ? db.slaveOk : options.slaveOk }; + + // Modify internal state with inherited BSON options + this.s.options = { ...this.s.options, ...inheritBSONOptions(options, db.s.options, false) }; } /** diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 1f0f46349a9..3ef075d3722 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -27,7 +27,7 @@ export class BulkWriteOperation extends OperationBase): void { diff --git a/src/operations/command.ts b/src/operations/command.ts index eaa6cbb4d72..0d29a651467 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -39,12 +39,6 @@ export interface CommandOperationOptions extends OperationOptions, WriteConcernO export interface OperationParent { s: { namespace: MongoDBNamespace; - serializeFunctions?: boolean; - raw?: boolean; - promoteLongs?: boolean; - promoteValues?: boolean; - promoteBuffers?: boolean; - ignoreUndefined?: boolean; options?: BSONSerializeOptions; }; readConcern?: ReadConcern; @@ -101,9 +95,7 @@ export abstract class CommandOperation< } // Assign BSON serialize options to OperationBase, preferring options over parent options. - // Here, we account for the fact that Collection stores bson options in s, while other parents, - // like Db, stores bson options in s.options - Object.assign(this, inheritBSONOptions(options, { ...parent?.s.options, ...parent?.s })); + Object.assign(this, inheritBSONOptions(options, parent?.s.options, true)); } abstract execute(server: Server, callback: Callback): void; diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index a1253538b59..3f293addd5c 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -250,7 +250,7 @@ export function updateDocuments( // Do we return the actual result document // Either use override on the function, or go back to default on either the collection // level or db - finalOptions.serializeFunctions = options.serializeFunctions || coll.s.serializeFunctions; + finalOptions.serializeFunctions = options.serializeFunctions || coll.s.options.serializeFunctions; // Execute the operation const op: Document = { q: selector, u: document }; diff --git a/src/operations/find_one.ts b/src/operations/find_one.ts index 342e8060860..5402d2d94df 100644 --- a/src/operations/find_one.ts +++ b/src/operations/find_one.ts @@ -18,7 +18,7 @@ export class FindOneOperation extends OperationBase { this.query = query; // Assign BSON serialize options to OperationBase, preferring options over collection options - Object.assign(this, inheritBSONOptions(options, collection.s)); + Object.assign(this, inheritBSONOptions(options, collection.s.options, true)); } execute(server: Server, callback: Callback): void { diff --git a/src/operations/insert.ts b/src/operations/insert.ts index 58b84cb09da..0b4b431f765 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -105,7 +105,6 @@ function insertDocuments( // If keep going set unordered if (finalOptions.keepGoing === true) finalOptions.ordered = false; - finalOptions.serializeFunctions = options.serializeFunctions || coll.s.serializeFunctions; docs = prepareDocs(coll, docs, options); diff --git a/src/operations/insert_many.ts b/src/operations/insert_many.ts index 5edefb095e3..99e8a506bc7 100644 --- a/src/operations/insert_many.ts +++ b/src/operations/insert_many.ts @@ -32,7 +32,7 @@ export class InsertManyOperation extends OperationBase): void { From 4baafa65b19793690ed265889228edd24690fed6 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 21 Oct 2020 11:27:30 -0400 Subject: [PATCH 12/16] respond to test nits --- test/functional/ignore_undefined.test.js | 84 ++++++++++++------------ 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/test/functional/ignore_undefined.test.js b/test/functional/ignore_undefined.test.js index 38280dbb2d5..08f0ccd0e9e 100644 --- a/test/functional/ignore_undefined.test.js +++ b/test/functional/ignore_undefined.test.js @@ -153,16 +153,16 @@ describe('Ignore Undefined', function () { it('Should correctly inherit ignore undefined field from db during insert', { metadata: { requires: { topology: ['single'] } }, - test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { + test: function () { + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { poolSize: 1, ignoreUndefined: false }); - client.connect(function (err, client) { - var db = client.db(configuration.db, { ignoreUndefined: true }); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue3'); + return withClient.call(this, client, (client, done) => { + const db = client.db(configuration.db, { ignoreUndefined: true }); + const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue3'); // Ignore the undefined field collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), err => { @@ -170,20 +170,20 @@ describe('Ignore Undefined', function () { // Locate the doument collection.findOne((err, item) => { - test.equal(1, item.a); - test.ok(item.b === undefined); - client.close(done); + expect(item).to.have.property('a', 1); + expect(item.b).to.be.undefined; + done(); }); }); }); } }); - it( - 'Should correctly inherit ignore undefined field from collection during insert', - withClient(function (client, done) { - var db = client.db('shouldCorrectlyIgnoreUndefinedValue4', { ignoreUndefined: false }); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue4', { + it('Should correctly inherit ignore undefined field from collection during insert', { + metadata: { requires: { topology: ['single'] } }, + test: withClient(function (client, done) { + const db = client.db('shouldCorrectlyIgnoreUndefinedValue4', { ignoreUndefined: false }); + const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue4', { ignoreUndefined: true }); @@ -193,19 +193,19 @@ describe('Ignore Undefined', function () { // Locate the doument collection.findOne((err, item) => { - test.equal(1, item.a); - test.ok(item.b === undefined); + expect(item).to.have.property('a', 1); + expect(item.b).to.be.undefined; done(); }); }); }) - ); + }); - it( - 'Should correctly inherit ignore undefined field from operation during insert', - withClient(function (client, done) { - var db = client.db('shouldCorrectlyIgnoreUndefinedValue5'); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue5', { + it('Should correctly inherit ignore undefined field from operation during insert', { + metadata: { requires: { topology: ['single'] } }, + test: withClient(function (client, done) { + const db = client.db('shouldCorrectlyIgnoreUndefinedValue5'); + const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue5', { ignoreUndefined: false }); @@ -216,19 +216,19 @@ describe('Ignore Undefined', function () { // Locate the doument collection.findOne({}, (err, item) => { expect(err).to.not.exist; - test.equal(1, item.a); - test.ok(item.b === undefined); + expect(item).to.have.property('a', 1); + expect(item.b).to.be.undefined; done(); }); }); }) - ); + }); - it( - 'Should correctly inherit ignore undefined field from operation during findOneAndReplace', - withClient(function (client, done) { - var db = client.db('shouldCorrectlyIgnoreUndefinedValue6'); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue6', { + it('Should correctly inherit ignore undefined field from operation during findOneAndReplace', { + metadata: { requires: { topology: ['single'] } }, + test: withClient(function (client, done) { + const db = client.db('shouldCorrectlyIgnoreUndefinedValue6'); + const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue6', { ignoreUndefined: false }); @@ -241,36 +241,36 @@ describe('Ignore Undefined', function () { // Locate the doument collection.findOne((err, item) => { - test.equal(1, item.a); - test.ok(item.b === undefined); + expect(item).to.have.property('a', 1); + expect(item.b).to.be.undefined; done(); }); }); }); }) - ); + }); - it( - 'Should correctly ignore undefined field during bulk write', - withClient(function (client, done) { - var db = client.db('shouldCorrectlyIgnoreUndefinedValue7'); - var collection = db.collection('shouldCorrectlyIgnoreUndefinedValue7'); + it('Should correctly ignore undefined field during bulk write', { + metadata: { requires: { topology: ['single'] } }, + test: withClient(function (client, done) { + const db = client.db('shouldCorrectlyIgnoreUndefinedValue7'); + const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue7'); // Ignore the undefined field collection.bulkWrite( - [{ insertOne: { a: 0, b: undefined } }], + [{ insertOne: { a: 1, b: undefined } }], { ignoreUndefined: true }, err => { expect(err).to.not.exist; // Locate the doument collection.findOne((err, item) => { - test.equal(0, item.a); - test.ok(item.b === undefined); + expect(item).to.have.property('a', 1); + expect(item.b).to.be.undefined; done(); }); } ); }) - ); + }); }); From 8db696160aa1f389fbe3d789d7e54772114fba46 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 21 Oct 2020 13:47:56 -0400 Subject: [PATCH 13/16] more idiomatic property test --- test/functional/ignore_undefined.test.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/functional/ignore_undefined.test.js b/test/functional/ignore_undefined.test.js index 08f0ccd0e9e..73ab6cf252e 100644 --- a/test/functional/ignore_undefined.test.js +++ b/test/functional/ignore_undefined.test.js @@ -170,8 +170,9 @@ describe('Ignore Undefined', function () { // Locate the doument collection.findOne((err, item) => { + expect(err).to.not.exist; expect(item).to.have.property('a', 1); - expect(item.b).to.be.undefined; + expect(item).to.not.have.property('b'); done(); }); }); @@ -193,8 +194,9 @@ describe('Ignore Undefined', function () { // Locate the doument collection.findOne((err, item) => { + expect(err).to.not.exist; expect(item).to.have.property('a', 1); - expect(item.b).to.be.undefined; + expect(item).to.not.have.property('b'); done(); }); }); @@ -217,7 +219,7 @@ describe('Ignore Undefined', function () { collection.findOne({}, (err, item) => { expect(err).to.not.exist; expect(item).to.have.property('a', 1); - expect(item.b).to.be.undefined; + expect(item).to.not.have.property('b'); done(); }); }); @@ -241,8 +243,9 @@ describe('Ignore Undefined', function () { // Locate the doument collection.findOne((err, item) => { + expect(err).to.not.exist; expect(item).to.have.property('a', 1); - expect(item.b).to.be.undefined; + expect(item).to.not.have.property('b'); done(); }); }); @@ -265,8 +268,9 @@ describe('Ignore Undefined', function () { // Locate the doument collection.findOne((err, item) => { + expect(err).to.not.exist; expect(item).to.have.property('a', 1); - expect(item.b).to.be.undefined; + expect(item).to.not.have.property('b'); done(); }); } From 7e111c1928f027567ae7fad0571f3ce1c580a021 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 22 Oct 2020 12:55:47 -0400 Subject: [PATCH 14/16] store bson options in single property --- src/operations/bulk_write.ts | 2 +- src/operations/command.ts | 2 +- src/operations/find_one.ts | 2 +- src/operations/insert_many.ts | 2 +- src/operations/operation.ts | 22 +--------------------- 5 files changed, 5 insertions(+), 25 deletions(-) diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 3ef075d3722..4ee385c894d 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -27,7 +27,7 @@ export class BulkWriteOperation extends OperationBase): void { diff --git a/src/operations/command.ts b/src/operations/command.ts index 0d29a651467..446c9295842 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -95,7 +95,7 @@ export abstract class CommandOperation< } // Assign BSON serialize options to OperationBase, preferring options over parent options. - Object.assign(this, inheritBSONOptions(options, parent?.s.options, true)); + this.bsonOptions = inheritBSONOptions(options, parent?.s.options, true); } abstract execute(server: Server, callback: Callback): void; diff --git a/src/operations/find_one.ts b/src/operations/find_one.ts index 5402d2d94df..d0a15543e95 100644 --- a/src/operations/find_one.ts +++ b/src/operations/find_one.ts @@ -18,7 +18,7 @@ export class FindOneOperation extends OperationBase { this.query = query; // Assign BSON serialize options to OperationBase, preferring options over collection options - Object.assign(this, inheritBSONOptions(options, collection.s.options, true)); + this.bsonOptions = inheritBSONOptions(options, collection.s.options, true); } execute(server: Server, callback: Callback): void { diff --git a/src/operations/insert_many.ts b/src/operations/insert_many.ts index 99e8a506bc7..8633797ee08 100644 --- a/src/operations/insert_many.ts +++ b/src/operations/insert_many.ts @@ -32,7 +32,7 @@ export class InsertManyOperation extends OperationBase): void { diff --git a/src/operations/operation.ts b/src/operations/operation.ts index d80614272af..ec012f19e39 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -46,33 +46,13 @@ export abstract class OperationBase< fullResponse?: boolean; // BSON serialization options - fieldsAsRaw?: { [key: string]: boolean }; - promoteValues?: boolean; - promoteBuffers?: boolean; - promoteLongs?: boolean; - serializeFunctions?: boolean; - ignoreUndefined?: boolean; - raw?: boolean; + bsonOptions?: BSONSerializeOptions; constructor(options: T = {} as T) { this.options = Object.assign({}, options); this.readPreference = ReadPreference.primary; } - // BSON serialization options - get bsonOptions(): BSONSerializeOptions { - const bsonOptions: Document = { - promoteBuffers: this.promoteBuffers, - promoteValues: this.promoteValues, - promoteLongs: this.promoteLongs, - raw: this.raw, - ignoreUndefined: this.ignoreUndefined, - serializeFunctions: this.serializeFunctions, - fieldsAsRaw: this.fieldsAsRaw - }; - return bsonOptions; - } - abstract execute(server: Server, callback: Callback): void; hasAspect(aspect: symbol): boolean { From e436fbab8d8eddbb8abb423cfd55f88875db1e9d Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Thu, 22 Oct 2020 18:30:25 -0400 Subject: [PATCH 15/16] put bsonOptions property on parents --- src/bson.ts | 32 +++++++++++----------------- src/collection.ts | 15 +++++++------ src/db.ts | 9 +++++++- src/mongo_client.ts | 8 ++++++- src/operations/bulk_write.ts | 4 ++-- src/operations/command.ts | 10 ++++----- src/operations/common_functions.ts | 3 ++- src/operations/find_one.ts | 4 ++-- src/operations/insert_many.ts | 4 ++-- test/functional/mongo_client.test.js | 4 ++-- 10 files changed, 51 insertions(+), 42 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index 3f7320b6e8d..1abd2e70409 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -1,3 +1,4 @@ +import type { OperationParent } from './operations/command'; // import type * as _BSON from 'bson'; // let BSON: typeof _BSON = require('bson'); // try { @@ -71,30 +72,23 @@ export function pluckBSONSerializeOptions(options: BSONSerializeOptions): BSONSe } /** - * Merge the given BSONSerializeOptions, preferring options over parentOptions, and optionally + * Merge the given BSONSerializeOptions, preferring options over the parent's options, and * substituting defaults for values not set. * * @internal - * - * @param options - The primary options to use - * @param parentOptions - The secondary options to use - * @param includeDefaultOptions - Whether to include default options for any values not set */ -export function inheritBSONOptions( +export function resolveBSONOptions( options?: BSONSerializeOptions, - parentOptions?: BSONSerializeOptions, - includeDefaultOptions?: boolean + parent?: OperationParent ): BSONSerializeOptions { - const defaults = { - raw: false, - promoteLongs: true, - promoteValues: true, - promoteBuffers: false, - ignoreUndefined: false, - serializeFunctions: false, - fieldsAsRaw: {} + const parentOptions = parent?.bsonOptions; + return { + raw: options?.raw ?? parentOptions?.raw ?? false, + promoteLongs: options?.promoteLongs ?? parentOptions?.promoteLongs ?? true, + promoteValues: options?.promoteValues ?? parentOptions?.promoteValues ?? true, + promoteBuffers: options?.promoteBuffers ?? parentOptions?.promoteBuffers ?? false, + ignoreUndefined: options?.ignoreUndefined ?? parentOptions?.ignoreUndefined ?? false, + serializeFunctions: options?.serializeFunctions ?? parentOptions?.serializeFunctions ?? false, + fieldsAsRaw: options?.fieldsAsRaw ?? parentOptions?.fieldsAsRaw ?? {} }; - - const base = includeDefaultOptions ? defaults : {}; - return pluckBSONSerializeOptions({ ...base, ...parentOptions, ...options }); } diff --git a/src/collection.ts b/src/collection.ts index 3a679a9c617..e59474fb28f 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -8,7 +8,7 @@ import { MongoDBNamespace, Callback } from './utils'; -import { ObjectId, Document, BSONSerializeOptions, inheritBSONOptions } from './bson'; +import { ObjectId, Document, BSONSerializeOptions, resolveBSONOptions } from './bson'; import { MongoError } from './error'; import { UnorderedBulkOperation } from './bulk/unordered'; import { OrderedBulkOperation } from './bulk/ordered'; @@ -126,6 +126,7 @@ export interface CollectionPrivate { options: any; namespace: MongoDBNamespace; readPreference?: ReadPreference; + bsonOptions: BSONSerializeOptions; slaveOk?: boolean; collectionHint?: Hint; readConcern?: ReadConcern; @@ -182,13 +183,11 @@ export class Collection implements OperationParent { } }, readPreference: ReadPreference.fromOptions(options), + bsonOptions: resolveBSONOptions(options, db), readConcern: ReadConcern.fromOptions(options), writeConcern: WriteConcern.fromOptions(options), slaveOk: options == null || options.slaveOk == null ? db.slaveOk : options.slaveOk }; - - // Modify internal state with inherited BSON options - this.s.options = { ...this.s.options, ...inheritBSONOptions(options, db.s.options, false) }; } /** @@ -236,6 +235,10 @@ export class Collection implements OperationParent { return this.s.readPreference; } + get bsonOptions(): BSONSerializeOptions { + return this.s.bsonOptions; + } + /** * The current writeConcern of the collection. If not explicitly defined for * this collection, will be inherited from the parent DB @@ -1284,7 +1287,7 @@ export class Collection implements OperationParent { options = options || {}; // Give function's options precedence over session options. if (options.ignoreUndefined == null) { - options.ignoreUndefined = this.s.options.ignoreUndefined; + options.ignoreUndefined = this.bsonOptions.ignoreUndefined; } return new UnorderedBulkOperation(this, options); @@ -1295,7 +1298,7 @@ export class Collection implements OperationParent { options = options || {}; // Give function's options precedence over session's options. if (options.ignoreUndefined == null) { - options.ignoreUndefined = this.s.options.ignoreUndefined; + options.ignoreUndefined = this.bsonOptions.ignoreUndefined; } return new OrderedBulkOperation(this, options); diff --git a/src/db.ts b/src/db.ts index 0cf12dbcdcb..fe8eda00cc5 100644 --- a/src/db.ts +++ b/src/db.ts @@ -2,7 +2,7 @@ import { deprecate } from 'util'; import { emitDeprecatedOptionWarning, Callback } from './utils'; import { loadAdmin } from './dynamic_loaders'; import { AggregationCursor, CommandCursor } from './cursor'; -import { ObjectId, Code, Document, BSONSerializeOptions } from './bson'; +import { ObjectId, Code, Document, BSONSerializeOptions, resolveBSONOptions } from './bson'; import { ReadPreference, ReadPreferenceLike } from './read_preference'; import { MongoError } from './error'; import { Collection, CollectionOptions } from './collection'; @@ -94,6 +94,7 @@ export interface DbPrivate { readPreference?: ReadPreference; pkFactory: PkFactory; readConcern?: ReadConcern; + bsonOptions: BSONSerializeOptions; writeConcern?: WriteConcern; namespace: MongoDBNamespace; } @@ -171,6 +172,8 @@ export class Db implements OperationParent { logger: new Logger('Db', options), // Unpack read preference readPreference: ReadPreference.fromOptions(options), + // Merge bson options TODO: include client bson options, after NODE-2850 + bsonOptions: resolveBSONOptions(options), // Set up the primary key factory or fallback to ObjectId pkFactory: options?.pkFactory ?? { createPk() { @@ -218,6 +221,10 @@ export class Db implements OperationParent { return this.s.readPreference; } + get bsonOptions(): BSONSerializeOptions { + return this.s.bsonOptions; + } + // get the write Concern get writeConcern(): WriteConcern | undefined { return this.s.writeConcern; diff --git a/src/mongo_client.ts b/src/mongo_client.ts index c5a275f8636..8b82fb890f5 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -10,7 +10,7 @@ import { connect, validOptions } from './operations/connect'; import { PromiseProvider } from './promise_provider'; import { Logger } from './logger'; import { ReadConcern, ReadConcernLevelLike, ReadConcernLike } from './read_concern'; -import type { BSONSerializeOptions, Document } from './bson'; +import { BSONSerializeOptions, Document, resolveBSONOptions } from './bson'; import type { AutoEncryptionOptions } from './deps'; import type { CompressorName } from './cmap/wire_protocol/compression'; import type { AuthMechanism } from './cmap/auth/defaultAuthProviders'; @@ -222,6 +222,7 @@ export interface MongoClientPrivate { readConcern?: ReadConcern; writeConcern?: WriteConcern; readPreference: ReadPreference; + bsonOptions: BSONSerializeOptions; namespace: MongoDBNamespace; logger: Logger; } @@ -284,6 +285,7 @@ export class MongoClient extends EventEmitter implements OperationParent { readConcern: ReadConcern.fromOptions(options), writeConcern: WriteConcern.fromOptions(options), readPreference: ReadPreference.fromOptions(options) || ReadPreference.primary, + bsonOptions: resolveBSONOptions(options), namespace: new MongoDBNamespace('admin'), logger: options?.logger ?? new Logger('MongoClient') }; @@ -301,6 +303,10 @@ export class MongoClient extends EventEmitter implements OperationParent { return this.s.readPreference; } + get bsonOptions(): BSONSerializeOptions { + return this.s.bsonOptions; + } + get logger(): Logger { return this.s.logger; } diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 4ee385c894d..6838f6fe5eb 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -1,6 +1,6 @@ import { applyRetryableWrites, applyWriteConcern, Callback } from '../utils'; import { OperationBase } from './operation'; -import { inheritBSONOptions } from '../bson'; +import { resolveBSONOptions } from '../bson'; import { WriteConcern } from '../write_concern'; import type { Collection } from '../collection'; import type { @@ -27,7 +27,7 @@ export class BulkWriteOperation extends OperationBase): void { diff --git a/src/operations/command.ts b/src/operations/command.ts index 446c9295842..818610402ac 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -7,7 +7,7 @@ import { commandSupportsReadConcern } from '../sessions'; import { MongoError } from '../error'; import type { Logger } from '../logger'; import type { Server } from '../sdam/server'; -import { BSONSerializeOptions, Document, inheritBSONOptions } from '../bson'; +import { BSONSerializeOptions, Document, resolveBSONOptions } from '../bson'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { ReadConcernLike } from './../read_concern'; @@ -37,14 +37,12 @@ export interface CommandOperationOptions extends OperationOptions, WriteConcernO /** @internal */ export interface OperationParent { - s: { - namespace: MongoDBNamespace; - options?: BSONSerializeOptions; - }; + s: { namespace: MongoDBNamespace }; readConcern?: ReadConcern; writeConcern?: WriteConcern; readPreference?: ReadPreference; logger?: Logger; + bsonOptions?: BSONSerializeOptions; } /** @internal */ @@ -95,7 +93,7 @@ export abstract class CommandOperation< } // Assign BSON serialize options to OperationBase, preferring options over parent options. - this.bsonOptions = inheritBSONOptions(options, parent?.s.options, true); + this.bsonOptions = resolveBSONOptions(options, parent); } abstract execute(server: Server, callback: Callback): void; diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index 3f293addd5c..be66758e061 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -250,7 +250,8 @@ export function updateDocuments( // Do we return the actual result document // Either use override on the function, or go back to default on either the collection // level or db - finalOptions.serializeFunctions = options.serializeFunctions || coll.s.options.serializeFunctions; + finalOptions.serializeFunctions = + options.serializeFunctions || coll.bsonOptions.serializeFunctions; // Execute the operation const op: Document = { q: selector, u: document }; diff --git a/src/operations/find_one.ts b/src/operations/find_one.ts index d0a15543e95..2c8b89e470e 100644 --- a/src/operations/find_one.ts +++ b/src/operations/find_one.ts @@ -1,6 +1,6 @@ import { OperationBase } from './operation'; import type { Callback } from '../utils'; -import { Document, inheritBSONOptions } from '../bson'; +import { Document, resolveBSONOptions } from '../bson'; import type { Collection } from '../collection'; import type { FindOptions } from './find'; import { MongoError } from '../error'; @@ -18,7 +18,7 @@ export class FindOneOperation extends OperationBase { this.query = query; // Assign BSON serialize options to OperationBase, preferring options over collection options - this.bsonOptions = inheritBSONOptions(options, collection.s.options, true); + this.bsonOptions = resolveBSONOptions(options, collection); } execute(server: Server, callback: Callback): void { diff --git a/src/operations/insert_many.ts b/src/operations/insert_many.ts index 8633797ee08..e5a15fc0c6e 100644 --- a/src/operations/insert_many.ts +++ b/src/operations/insert_many.ts @@ -4,7 +4,7 @@ import { MongoError } from '../error'; import { prepareDocs } from './common_functions'; import type { Callback } from '../utils'; import type { Collection } from '../collection'; -import { ObjectId, Document, inheritBSONOptions } from '../bson'; +import { ObjectId, Document, resolveBSONOptions } from '../bson'; import type { BulkWriteResult, BulkWriteOptions } from '../bulk/common'; import type { Server } from '../sdam/server'; @@ -32,7 +32,7 @@ export class InsertManyOperation extends OperationBase): void { diff --git a/test/functional/mongo_client.test.js b/test/functional/mongo_client.test.js index 90dbf01d11f..860de1afdad 100644 --- a/test/functional/mongo_client.test.js +++ b/test/functional/mongo_client.test.js @@ -57,8 +57,8 @@ describe('MongoClient', function () { test.equal(true, db.s.options.forceServerObjectId); test.equal(1, db.s.pkFactory.createPk()); - test.equal(true, db.s.options.serializeFunctions); - test.equal(true, db.s.options.raw); + test.equal(true, db.bsonOptions.serializeFunctions); + test.equal(true, db.bsonOptions.raw); test.equal(10, db.s.options.numberOfRetries); test.equal(0, db.s.options.bufferMaxEntries); From b81c3092e3cf5acf416f55a9fd9154f8a3044726 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Fri, 23 Oct 2020 09:41:46 -0400 Subject: [PATCH 16/16] remove metadata from ignore undefined tests --- test/functional/ignore_undefined.test.js | 72 +++++++++++------------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/test/functional/ignore_undefined.test.js b/test/functional/ignore_undefined.test.js index 73ab6cf252e..63e507f622f 100644 --- a/test/functional/ignore_undefined.test.js +++ b/test/functional/ignore_undefined.test.js @@ -150,39 +150,35 @@ describe('Ignore Undefined', function () { } }); - it('Should correctly inherit ignore undefined field from db during insert', { - metadata: { requires: { topology: ['single'] } }, + it('Should correctly inherit ignore undefined field from db during insert', function () { + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1, + ignoreUndefined: false + }); - test: function () { - const configuration = this.configuration; - const client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1, - ignoreUndefined: false - }); + return withClient.call(this, client, (client, done) => { + const db = client.db(configuration.db, { ignoreUndefined: true }); + const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue3'); - return withClient.call(this, client, (client, done) => { - const db = client.db(configuration.db, { ignoreUndefined: true }); - const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue3'); + // Ignore the undefined field + collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), err => { + expect(err).to.not.exist; - // Ignore the undefined field - collection.insert({ a: 1, b: undefined }, configuration.writeConcernMax(), err => { + // Locate the doument + collection.findOne((err, item) => { expect(err).to.not.exist; - - // Locate the doument - collection.findOne((err, item) => { - expect(err).to.not.exist; - expect(item).to.have.property('a', 1); - expect(item).to.not.have.property('b'); - done(); - }); + expect(item).to.have.property('a', 1); + expect(item).to.not.have.property('b'); + done(); }); }); - } + }); }); - it('Should correctly inherit ignore undefined field from collection during insert', { - metadata: { requires: { topology: ['single'] } }, - test: withClient(function (client, done) { + it( + 'Should correctly inherit ignore undefined field from collection during insert', + withClient(function (client, done) { const db = client.db('shouldCorrectlyIgnoreUndefinedValue4', { ignoreUndefined: false }); const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue4', { ignoreUndefined: true @@ -201,11 +197,11 @@ describe('Ignore Undefined', function () { }); }); }) - }); + ); - it('Should correctly inherit ignore undefined field from operation during insert', { - metadata: { requires: { topology: ['single'] } }, - test: withClient(function (client, done) { + it( + 'Should correctly inherit ignore undefined field from operation during insert', + withClient(function (client, done) { const db = client.db('shouldCorrectlyIgnoreUndefinedValue5'); const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue5', { ignoreUndefined: false @@ -224,11 +220,11 @@ describe('Ignore Undefined', function () { }); }); }) - }); + ); - it('Should correctly inherit ignore undefined field from operation during findOneAndReplace', { - metadata: { requires: { topology: ['single'] } }, - test: withClient(function (client, done) { + it( + 'Should correctly inherit ignore undefined field from operation during findOneAndReplace', + withClient(function (client, done) { const db = client.db('shouldCorrectlyIgnoreUndefinedValue6'); const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue6', { ignoreUndefined: false @@ -251,11 +247,11 @@ describe('Ignore Undefined', function () { }); }); }) - }); + ); - it('Should correctly ignore undefined field during bulk write', { - metadata: { requires: { topology: ['single'] } }, - test: withClient(function (client, done) { + it( + 'Should correctly ignore undefined field during bulk write', + withClient(function (client, done) { const db = client.db('shouldCorrectlyIgnoreUndefinedValue7'); const collection = db.collection('shouldCorrectlyIgnoreUndefinedValue7'); @@ -276,5 +272,5 @@ describe('Ignore Undefined', function () { } ); }) - }); + ); });