From 177f0ef7b324fbe3c6e789ab3dc3bfc02e0f80e9 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Tue, 27 Oct 2020 18:03:14 -0400 Subject: [PATCH 1/3] add bson options property to bulk ops state --- src/bulk/common.ts | 22 ++++---- src/collection.ts | 16 +----- test/functional/bulk.test.js | 104 +++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 26 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 335c426b86..25a94031c5 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -1,5 +1,5 @@ import { PromiseProvider } from '../promise_provider'; -import { Long, ObjectId, Document } from '../bson'; +import { Long, ObjectId, Document, BSONSerializeOptions, resolveBSONOptions } from '../bson'; import { MongoError, MongoWriteConcernError, AnyError } from '../error'; import { applyWriteConcern, @@ -570,7 +570,11 @@ function executeCommands( executeCommands(bulkOperation, options, callback); } - const finalOptions = Object.assign({ ordered: bulkOperation.isOrdered }, options); + const finalOptions = Object.assign( + { ordered: bulkOperation.isOrdered }, + bulkOperation.s.bsonOptions, + options + ); if (bulkOperation.s.writeConcern != null) { finalOptions.writeConcern = bulkOperation.s.writeConcern; } @@ -584,16 +588,6 @@ function executeCommands( resultHandler.operationId = bulkOperation.operationId; } - // Serialize functions - if (bulkOperation.s.options.serializeFunctions) { - finalOptions.serializeFunctions = true; - } - - // Ignore undefined - if (bulkOperation.s.options.ignoreUndefined) { - finalOptions.ignoreUndefined = true; - } - // Is the bypassDocumentValidation options specific if (bulkOperation.s.bypassDocumentValidation === true) { finalOptions.bypassDocumentValidation = true; @@ -870,6 +864,8 @@ interface BulkOperationPrivate { topology: Topology; // Options options: BulkWriteOptions; + // BSON options + bsonOptions: BSONSerializeOptions; // Document used to build a bulk operation currentOp?: Document; // Executed @@ -985,6 +981,8 @@ export abstract class BulkOperationBase { topology, // Options options: finalOptions, + // BSON options + bsonOptions: resolveBSONOptions(options, collection), // Current operation currentOp, // Executed diff --git a/src/collection.ts b/src/collection.ts index 523bf6ce01..c887856425 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -1285,24 +1285,12 @@ export class Collection implements OperationParent { /** Initiate an Out of order batch write operation. All operations will be buffered into insert/update/remove commands executed out of order. */ initializeUnorderedBulkOp(options?: BulkWriteOptions): any { - options = options || {}; - // Give function's options precedence over session options. - if (options.ignoreUndefined == null) { - options.ignoreUndefined = this.bsonOptions.ignoreUndefined; - } - - return new UnorderedBulkOperation(this, options); + return new UnorderedBulkOperation(this, options || {}); } /** Initiate an In order bulk write operation. Operations will be serially executed in the order they are added, creating a new operation for each switch in types. */ initializeOrderedBulkOp(options?: BulkWriteOptions): any { - options = options || {}; - // Give function's options precedence over session's options. - if (options.ignoreUndefined == null) { - options.ignoreUndefined = this.bsonOptions.ignoreUndefined; - } - - return new OrderedBulkOperation(this, options); + return new OrderedBulkOperation(this, options || {}); } /** Get the db scoped logger */ diff --git a/test/functional/bulk.test.js b/test/functional/bulk.test.js index 38a46974db..7edcf6bc69 100644 --- a/test/functional/bulk.test.js +++ b/test/functional/bulk.test.js @@ -1,10 +1,12 @@ 'use strict'; +const { withClient } = require('./shared'); const test = require('./shared').assert, setupDatabase = require('./shared').setupDatabase, expect = require('chai').expect; const MongoError = require('../../src/error').MongoError; const ignoreNsNotFound = require('./shared').ignoreNsNotFound; +const { Long } = require('../../src'); describe('Bulk', function () { before(function () { @@ -158,6 +160,108 @@ describe('Bulk', function () { } }); + it('should inherit promote long false from db during unordered bulk operation', function () { + const client = this.configuration.newClient(this.configuration.writeConcernMax(), { + promoteLongs: true + }); + + return withClient.call(this, client, (client, done) => { + const db = client.db('shouldInheritPromoteLongFalseFromDb1', { promoteLongs: false }); + const coll = db.collection('test'); + + const batch = coll.initializeUnorderedBulkOp(); + batch.insert({ a: Long.fromNumber(10) }); + batch.execute((err, result) => { + expect(err).to.not.exist; + expect(result).to.exist; + + coll.findOne((err, item) => { + expect(err).to.not.exist; + expect(item.a).to.not.be.a('number'); + expect(item.a).to.have.property('_bsontype'); + expect(item.a._bsontype).to.be.equal('Long'); + + done(); + }); + }); + }); + }); + + it( + 'should inherit promote long false from collection during unordered bulk operation', + withClient(function (client, done) { + const db = client.db('shouldInheritPromoteLongFalseFromColl1', { promoteLongs: true }); + const coll = db.collection('test', { promoteLongs: false }); + + const batch = coll.initializeUnorderedBulkOp(); + batch.insert({ a: Long.fromNumber(10) }); + batch.execute((err, result) => { + expect(err).to.not.exist; + expect(result).to.exist; + + coll.findOne((err, item) => { + expect(err).to.not.exist; + expect(item.a).to.not.be.a('number'); + expect(item.a).to.have.property('_bsontype'); + expect(item.a._bsontype).to.be.equal('Long'); + + done(); + }); + }); + }) + ); + + it('should inherit promote long false from db during ordered bulk operation', function () { + const client = this.configuration.newClient(this.configuration.writeConcernMax(), { + promoteLongs: true + }); + + return withClient.call(this, client, (client, done) => { + const db = client.db('shouldInheritPromoteLongFalseFromDb2', { promoteLongs: false }); + const coll = db.collection('test'); + + const batch = coll.initializeOrderedBulkOp(); + batch.insert({ a: Long.fromNumber(10) }); + batch.execute((err, result) => { + expect(err).to.not.exist; + expect(result).to.exist; + + coll.findOne((err, item) => { + expect(err).to.not.exist; + expect(item.a).to.not.be.a('number'); + expect(item.a).to.have.property('_bsontype'); + expect(item.a._bsontype).to.be.equal('Long'); + + done(); + }); + }); + }); + }); + + it( + 'should inherit promote long false from collection during ordered bulk operation', + withClient(function (client, done) { + const db = client.db('shouldInheritPromoteLongFalseFromColl2', { promoteLongs: true }); + const coll = db.collection('test', { promoteLongs: false }); + + const batch = coll.initializeOrderedBulkOp(); + batch.insert({ a: Long.fromNumber(10) }); + batch.execute((err, result) => { + expect(err).to.not.exist; + expect(result).to.exist; + + coll.findOne((err, item) => { + expect(err).to.not.exist; + expect(item.a).to.not.be.a('number'); + expect(item.a).to.have.property('_bsontype'); + expect(item.a._bsontype).to.be.equal('Long'); + + done(); + }); + }); + }) + ); + it('should correctly handle ordered multiple batch api write command errors', { metadata: { requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } From 09e6e4c2cbb6f2a799ea1bcceb985f1e01270b70 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Wed, 28 Oct 2020 11:24:45 -0400 Subject: [PATCH 2/3] add bsonOptions getter to bulk op base class --- src/bulk/common.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 25a94031c5..790b72e15e 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -572,7 +572,7 @@ function executeCommands( const finalOptions = Object.assign( { ordered: bulkOperation.isOrdered }, - bulkOperation.s.bsonOptions, + bulkOperation.bsonOptions, options ); if (bulkOperation.s.writeConcern != null) { @@ -1164,6 +1164,10 @@ export abstract class BulkOperationBase { ); } + get bsonOptions(): BSONSerializeOptions { + return this.s.bsonOptions; + } + /** An internal helper method. Do not invoke directly. Will be going away in the future */ execute( _writeConcern?: WriteConcern, From e3ef9e638dac7c2651a49e0d638bb30d594265d0 Mon Sep 17 00:00:00 2001 From: Hana Pearlman Date: Mon, 2 Nov 2020 10:37:06 -0500 Subject: [PATCH 3/3] apply suggested changes --- src/collection.ts | 4 ++-- test/functional/bulk.test.js | 11 ++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index c887856425..5fa74a0bc6 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -1285,12 +1285,12 @@ export class Collection implements OperationParent { /** Initiate an Out of order batch write operation. All operations will be buffered into insert/update/remove commands executed out of order. */ initializeUnorderedBulkOp(options?: BulkWriteOptions): any { - return new UnorderedBulkOperation(this, options || {}); + return new UnorderedBulkOperation(this, options ?? {}); } /** Initiate an In order bulk write operation. Operations will be serially executed in the order they are added, creating a new operation for each switch in types. */ initializeOrderedBulkOp(options?: BulkWriteOptions): any { - return new OrderedBulkOperation(this, options || {}); + return new OrderedBulkOperation(this, options ?? {}); } /** Get the db scoped logger */ diff --git a/test/functional/bulk.test.js b/test/functional/bulk.test.js index 7edcf6bc69..41bbf372a2 100644 --- a/test/functional/bulk.test.js +++ b/test/functional/bulk.test.js @@ -1,11 +1,8 @@ 'use strict'; -const { withClient } = require('./shared'); -const test = require('./shared').assert, - setupDatabase = require('./shared').setupDatabase, - expect = require('chai').expect; - -const MongoError = require('../../src/error').MongoError; -const ignoreNsNotFound = require('./shared').ignoreNsNotFound; +const { withClient, setupDatabase, ignoreNsNotFound } = require('./shared'); +const test = require('./shared').assert; +const { expect } = require('chai'); +const { MongoError } = require('../../src/error'); const { Long } = require('../../src'); describe('Bulk', function () {