Skip to content

Commit

Permalink
fix(collection): only send bypassDocumentValidation if true
Browse files Browse the repository at this point in the history
The bypassDocumentValidation key will only be set if it explicitly
receives a true, otherwise it remains undefined.

Fixes NODE-1492
  • Loading branch information
Sophie Saskin authored Jun 18, 2018
1 parent b3ff3ed commit fdb828b
Show file tree
Hide file tree
Showing 6 changed files with 260 additions and 15 deletions.
2 changes: 1 addition & 1 deletion lib/apm.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Instrumentation extends EventEmitter {
instrument(MongoClient, callback) {
// store a reference to the original functions
this.$MongoClient = MongoClient;
const $prototypeConnect = this.$prototypeConnect = MongoClient.prototype.connect; // eslint-disable-line
const $prototypeConnect = (this.$prototypeConnect = MongoClient.prototype.connect);

const instrumentation = this;
MongoClient.prototype.connect = function(callback) {
Expand Down
13 changes: 8 additions & 5 deletions lib/bulk/ordered.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,13 @@ function OrderedBulkOperation(topology, collection, options) {
promiseLibrary: promiseLibrary,
// Fundamental error
err: null,
// Bypass validation
bypassDocumentValidation:
typeof options.bypassDocumentValidation === 'boolean'
? options.bypassDocumentValidation
: false,
// check keys
checkKeys: typeof options.checkKeys === 'boolean' ? options.checkKeys : true
};
// bypass Validation
if (options.bypassDocumentValidation === true) {
this.s.bypassDocumentValidation = true;
}
}

OrderedBulkOperation.prototype.raw = function(op) {
Expand Down Expand Up @@ -490,6 +489,10 @@ var executeCommands = function(self, options, callback) {
finalOptions.writeConcern = self.s.writeConcern;
}

if (finalOptions.bypassDocumentValidation !== true) {
delete finalOptions.bypassDocumentValidation;
}

// Set an operationIf if provided
if (self.operationId) {
resultHandler.operationId = self.operationId;
Expand Down
13 changes: 8 additions & 5 deletions lib/bulk/unordered.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,13 @@ var UnorderedBulkOperation = function(topology, collection, options) {
collection: collection,
// Promise Library
promiseLibrary: promiseLibrary,
// Bypass validation
bypassDocumentValidation:
typeof options.bypassDocumentValidation === 'boolean'
? options.bypassDocumentValidation
: false,
// check keys
checkKeys: typeof options.checkKeys === 'boolean' ? options.checkKeys : true
};
// bypass Validation
if (options.bypassDocumentValidation === true) {
this.s.bypassDocumentValidation = true;
}
};

/**
Expand Down Expand Up @@ -440,6 +439,10 @@ var executeBatch = function(self, batch, options, callback) {
finalOptions.writeConcern = self.s.writeConcern;
}

if (finalOptions.bypassDocumentValidation !== true) {
delete finalOptions.bypassDocumentValidation;
}

var resultHandler = function(err, result) {
// Error is a driver related error not a bulk op error, terminate
if ((err && err.driver) || (err && err.message)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -1720,7 +1720,7 @@ Collection.prototype.aggregate = function(pipeline, options, callback) {
decorateWithCollation(command, this, options);

// If we have bypassDocumentValidation set
if (typeof options.bypassDocumentValidation === 'boolean') {
if (options.bypassDocumentValidation === true) {
command.bypassDocumentValidation = options.bypassDocumentValidation;
}

Expand Down
6 changes: 3 additions & 3 deletions lib/operations/collection_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ function findAndModify(coll, query, sort, doc, options, callback) {
}

// Have we specified bypassDocumentValidation
if (typeof finalOptions.bypassDocumentValidation === 'boolean') {
if (finalOptions.bypassDocumentValidation === true) {
queryObject.bypassDocumentValidation = finalOptions.bypassDocumentValidation;
}

Expand Down Expand Up @@ -874,7 +874,7 @@ function mapReduce(coll, map, reduce, options, callback) {
};

// Exclusion list
const exclusionList = ['readPreference', 'session'];
const exclusionList = ['readPreference', 'session', 'bypassDocumentValidation'];

// Add any other options passed in
for (let n in options) {
Expand Down Expand Up @@ -909,7 +909,7 @@ function mapReduce(coll, map, reduce, options, callback) {
}

// Is bypassDocumentValidation specified
if (typeof options.bypassDocumentValidation === 'boolean') {
if (options.bypassDocumentValidation === true) {
mapCommandHash.bypassDocumentValidation = options.bypassDocumentValidation;
}

Expand Down
239 changes: 239 additions & 0 deletions test/unit/bypass_validation_tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
'use strict';

const MongoClient = require('../..').MongoClient;
const expect = require('chai').expect;
const mock = require('mongodb-mock-server');

describe('bypass document validation', function() {
const test = {};
beforeEach(() => {
return mock.createServer().then(server => {
test.server = server;
});
});
afterEach(() => mock.cleanup());

// general test for aggregate function
function testAggregate(config, done) {
const client = new MongoClient(`mongodb://${test.server.uri()}/test`);
let close = e => {
close = () => {};
client.close(() => done(e));
};

test.server.setMessageHandler(request => {
const doc = request.document;
if (doc.aggregate) {
try {
expect(doc.bypassDocumentValidation).equal(config.expected);
request.reply({
ok: 1,
cursor: {
firstBatch: [{}],
id: 23,
ns: 'test.test'
}
});
} catch (e) {
close(e);
}
}

if (doc.ismaster) {
request.reply(Object.assign({}, mock.DEFAULT_ISMASTER));
} else if (doc.endSessions) {
request.reply({ ok: 1 });
}
});

client.connect(function(err, client) {
expect(err).to.not.exist;
const db = client.db('test');
const collection = db.collection('test_c');

const options = { bypassDocumentValidation: config.actual };

const pipeline = [
{
$project: {}
}
];
collection.aggregate(pipeline, options).next(() => close());
});
}
// aggregate
it('should only set bypass document validation if strictly true in aggregate', function(done) {
testAggregate({ expected: true, actual: true }, done);
});

it('should not set bypass document validation if not strictly true in aggregate', function(done) {
testAggregate({ expected: undefined, actual: false }, done);
});

// general test for mapReduce function
function testMapReduce(config, done) {
const client = new MongoClient(`mongodb://${test.server.uri()}/test`);
let close = e => {
close = () => {};
client.close(() => done(e));
};

test.server.setMessageHandler(request => {
const doc = request.document;
if (doc.mapreduce) {
try {
expect(doc.bypassDocumentValidation).equal(config.expected);
request.reply({
results: 't',
ok: 1
});
} catch (e) {
close(e);
}
}

if (doc.ismaster) {
request.reply(Object.assign({}, mock.DEFAULT_ISMASTER));
} else if (doc.endSessions) {
request.reply({ ok: 1 });
}
});

client.connect(function(err, client) {
expect(err).to.not.exist;
const db = client.db('test');
const collection = db.collection('test_c');

const options = {
out: 'test_c',
bypassDocumentValidation: config.actual
};

collection.mapReduce(function map() {}, function reduce() {}, options, e => {
close(e);
});
});
}
// map reduce
it('should only set bypass document validation if strictly true in mapReduce', function(done) {
testMapReduce({ expected: true, actual: true }, done);
});

it('should not set bypass document validation if not strictly true in mapReduce', function(done) {
testMapReduce({ expected: undefined, actual: false }, done);
});

// general test for findAndModify function
function testFindAndModify(config, done) {
const client = new MongoClient(`mongodb://${test.server.uri()}/test`);
let close = e => {
close = () => {};
client.close(() => done(e));
};

test.server.setMessageHandler(request => {
const doc = request.document;
if (doc.findAndModify) {
try {
expect(doc.bypassDocumentValidation).equal(config.expected);
request.reply({
ok: 1
});
} catch (e) {
close(e);
}
}

if (doc.ismaster) {
request.reply(Object.assign({}, mock.DEFAULT_ISMASTER));
} else if (doc.endSessions) {
request.reply({ ok: 1 });
}
});

client.connect(function(err, client) {
expect(err).to.not.exist;
const db = client.db('test');
const collection = db.collection('test_c');

const options = { bypassDocumentValidation: config.actual };

collection.findAndModify(
{ name: 'Andy' },
{ rating: 1 },
{ $inc: { score: 1 } },
options,
e => {
close(e);
}
);
});
}
// find and modify
it('should only set bypass document validation if strictly true in findAndModify', function(done) {
testFindAndModify({ expected: true, actual: true }, done);
});

it('should not set bypass document validation if not strictly true in findAndModify', function(done) {
testFindAndModify({ expected: undefined, actual: false }, done);
});

// general test for BlukWrite to test changes made in ordered.js and unordered.js
function testBulkWrite(config, done) {
const client = new MongoClient(`mongodb://${test.server.uri()}/test`);
let close = e => {
close = () => {};
client.close(() => done(e));
};

test.server.setMessageHandler(request => {
const doc = request.document;
if (doc.insert) {
try {
expect(doc.bypassDocumentValidation).equal(config.expected);
request.reply({
ok: 1
});
} catch (e) {
close(e);
}
}

if (doc.ismaster) {
request.reply(Object.assign({}, mock.DEFAULT_ISMASTER));
} else if (doc.endSessions) {
request.reply({ ok: 1 });
}
});

client.connect(function(err, client) {
expect(err).to.not.exist;
const db = client.db('test');
const collection = db.collection('test_c');

const options = {
bypassDocumentValidation: config.actual,
ordered: config.ordered
};

collection.bulkWrite([{ insertOne: { document: { a: 1 } } }], options, () => close());
});
}
// ordered bulk write, testing change in ordered.js
it('should only set bypass document validation if strictly true in ordered bulkWrite', function(done) {
testBulkWrite({ expected: true, actual: true, ordered: true }, done);
});

it('should not set bypass document validation if not strictly true in ordered bulkWrite', function(done) {
testBulkWrite({ expected: undefined, actual: false, ordered: true }, done);
});

// unordered bulk write, testing change in ordered.js
it('should only set bypass document validation if strictly true in unordered bulkWrite', function(done) {
testBulkWrite({ expected: true, actual: true, ordered: false }, done);
});

it('should not set bypass document validation if not strictly true in unordered bulkWrite', function(done) {
testBulkWrite({ expected: undefined, actual: false, ordered: false }, done);
});
});

0 comments on commit fdb828b

Please sign in to comment.