Skip to content

Commit

Permalink
fix: apply bson options for bulk operations (#2601)
Browse files Browse the repository at this point in the history
The logic for inheriting BSON options was refactored for bulk
operations, ensuring proper inheritance of BSON Serialize options.

NODE-1561
  • Loading branch information
HanaPearlman authored Nov 5, 2020
1 parent f921132 commit e01cafd
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 32 deletions.
26 changes: 14 additions & 12 deletions src/bulk/common.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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.bsonOptions,
options
);
if (bulkOperation.s.writeConcern != null) {
finalOptions.writeConcern = bulkOperation.s.writeConcern;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -985,6 +981,8 @@ export abstract class BulkOperationBase {
topology,
// Options
options: finalOptions,
// BSON options
bsonOptions: resolveBSONOptions(options, collection),
// Current operation
currentOp,
// Executed
Expand Down Expand Up @@ -1166,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,
Expand Down
16 changes: 2 additions & 14 deletions src/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1288,24 +1288,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 */
Expand Down
113 changes: 107 additions & 6 deletions test/functional/bulk.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
'use strict';
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 () {
before(function () {
Expand Down Expand Up @@ -158,6 +157,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'] }
Expand Down

0 comments on commit e01cafd

Please sign in to comment.