Skip to content

Commit

Permalink
Merge pull request #14599 from Automattic/vkarpov15/gh-14572-3
Browse files Browse the repository at this point in the history
feat(model): add throwOnValidationError option for opting into getting MongooseBulkWriteError if all valid operations succeed in bulkWrite() and insertMany()
  • Loading branch information
vkarpov15 authored Jun 2, 2024
2 parents c3b4bdb + f4cfe1e commit 37e73b8
Show file tree
Hide file tree
Showing 4 changed files with 195 additions and 0 deletions.
41 changes: 41 additions & 0 deletions lib/error/bulkWriteError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*!
* Module dependencies.
*/

'use strict';

const MongooseError = require('./');


/**
* If `bulkWrite()` or `insertMany()` has validation errors, but
* all valid operations succeed, and 'throwOnValidationError' is true,
* Mongoose will throw this error.
*
* @api private
*/

class MongooseBulkWriteError extends MongooseError {
constructor(validationErrors, results, rawResult, operation) {
let preview = validationErrors.map(e => e.message).join(', ');
if (preview.length > 200) {
preview = preview.slice(0, 200) + '...';
}
super(`${operation} failed with ${validationErrors.length} Mongoose validation errors: ${preview}`);

this.validationErrors = validationErrors;
this.results = results;
this.rawResult = rawResult;
this.operation = operation;
}
}

Object.defineProperty(MongooseBulkWriteError.prototype, 'name', {
value: 'MongooseBulkWriteError'
});

/*!
* exports
*/

module.exports = MongooseBulkWriteError;
48 changes: 48 additions & 0 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const Document = require('./document');
const DocumentNotFoundError = require('./error/notFound');
const DivergentArrayError = require('./error/divergentArray');
const EventEmitter = require('events').EventEmitter;
const MongooseBulkWriteError = require('./error/bulkWriteError');
const MongooseBuffer = require('./types/buffer');
const MongooseError = require('./error/index');
const OverwriteModelError = require('./error/overwriteModel');
Expand Down Expand Up @@ -3375,6 +3376,7 @@ Model.startSession = function() {
* @param {Boolean} [options.lean=false] if `true`, skips hydrating the documents. This means Mongoose will **not** cast or validate any of the documents passed to `insertMany()`. This option is useful if you need the extra performance, but comes with data integrity risk. Consider using with [`castObject()`](#model_Model-castObject).
* @param {Number} [options.limit=null] this limits the number of documents being processed (validation/casting) by mongoose in parallel, this does **NOT** send the documents in batches to MongoDB. Use this option if you're processing a large number of documents and your app is running out of memory.
* @param {String|Object|Array} [options.populate=null] populates the result documents. This option is a no-op if `rawResult` is set.
* @param {Boolean} [options.throwOnValidationError=false] If true and `ordered: false`, throw an error if one of the operations failed validation, but all valid operations completed successfully.
* @param {Function} [callback] callback
* @return {Promise} resolving to the raw result from the MongoDB driver if `options.rawResult` was `true`, or the documents that passed validation, otherwise
* @api public
Expand Down Expand Up @@ -3419,6 +3421,7 @@ Model.$__insertMany = function(arr, options, callback) {
const limit = options.limit || 1000;
const rawResult = !!options.rawResult;
const ordered = typeof options.ordered === 'boolean' ? options.ordered : true;
const throwOnValidationError = typeof options.throwOnValidationError === 'boolean' ? options.throwOnValidationError : false;
const lean = !!options.lean;

if (!Array.isArray(arr)) {
Expand Down Expand Up @@ -3496,6 +3499,14 @@ Model.$__insertMany = function(arr, options, callback) {

// Quickly escape while there aren't any valid docAttributes
if (docAttributes.length === 0) {
if (throwOnValidationError) {
return callback(new MongooseBulkWriteError(
validationErrors,
results,
null,
'insertMany'
));
}
if (rawResult) {
const res = {
acknowledged: true,
Expand Down Expand Up @@ -3598,6 +3609,20 @@ Model.$__insertMany = function(arr, options, callback) {
}
}

if (ordered === false && throwOnValidationError && validationErrors.length > 0) {
for (let i = 0; i < results.length; ++i) {
if (results[i] === void 0) {
results[i] = docs[i];
}
}
return callback(new MongooseBulkWriteError(
validationErrors,
results,
res,
'insertMany'
));
}

if (rawResult) {
if (ordered === false) {
for (let i = 0; i < results.length; ++i) {
Expand Down Expand Up @@ -3728,6 +3753,7 @@ function _setIsNew(doc, val) {
* @param {Boolean} [options.skipValidation=false] Set to true to skip Mongoose schema validation on bulk write operations. Mongoose currently runs validation on `insertOne` and `replaceOne` operations by default.
* @param {Boolean} [options.bypassDocumentValidation=false] If true, disable [MongoDB server-side schema validation](https://www.mongodb.com/docs/manual/core/schema-validation/) for all writes in this bulk.
* @param {Boolean} [options.strict=null] Overwrites the [`strict` option](/docs/guide.html#strict) on schema. If false, allows filtering and writing fields not defined in the schema for all writes in this bulk.
* @param {Boolean} [options.throwOnValidationError=false] If true and `ordered: false`, throw an error if one of the operations failed validation, but all valid operations completed successfully.
* @param {Function} [callback] callback `function(error, bulkWriteOpResult) {}`
* @return {Promise} resolves to a [`BulkWriteOpResult`](https://mongodb.github.io/node-mongodb-native/4.9/classes/BulkWriteResult.html) if the operation succeeds
* @api public
Expand Down Expand Up @@ -3777,6 +3803,7 @@ Model.bulkWrite = function(ops, options, callback) {
let remaining = validations.length;
let validOps = [];
let validationErrors = [];
const results = [];
if (remaining === 0) {
completeUnorderedValidation.call(this);
} else {
Expand All @@ -3786,6 +3813,7 @@ Model.bulkWrite = function(ops, options, callback) {
validOps.push(i);
} else {
validationErrors.push({ index: i, error: err });
results[i] = err;
}
if (--remaining <= 0) {
completeUnorderedValidation.call(this);
Expand All @@ -3799,13 +3827,25 @@ Model.bulkWrite = function(ops, options, callback) {
map(v => v.error);

function completeUnorderedValidation() {
const validOpIndexes = validOps;
validOps = validOps.sort().map(index => ops[index]);

if (validOps.length === 0) {
if ('throwOnValidationError' in options && options.throwOnValidationError && validationErrors.length > 0) {
return cb(new MongooseBulkWriteError(
validationErrors.map(err => err.error),
results,
getDefaultBulkwriteResult(),
'bulkWrite'
));
}
return cb(null, getDefaultBulkwriteResult());
}

this.$__collection.bulkWrite(validOps, options, (error, res) => {
for (let i = 0; i < validOpIndexes.length; ++i) {
results[validOpIndexes[i]] = null;
}
if (error) {
if (validationErrors.length > 0) {
error.mongoose = error.mongoose || {};
Expand All @@ -3816,6 +3856,14 @@ Model.bulkWrite = function(ops, options, callback) {
}

if (validationErrors.length > 0) {
if ('throwOnValidationError' in options && options.throwOnValidationError) {
return cb(new MongooseBulkWriteError(
validationErrors,
results,
res,
'bulkWrite'
));
}
res.mongoose = res.mongoose || {};
res.mongoose.validationErrors = validationErrors;
}
Expand Down
104 changes: 104 additions & 0 deletions test/model.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6111,6 +6111,71 @@ describe('Model', function() {
const { num } = await Test.findById(_id);
assert.equal(num, 99);
});

it('bulkWrite should throw an error if there were operations that failed validation, ' +
'but all operations that passed validation succeeded (gh-13256)', async function() {
const userSchema = new Schema({ age: { type: Number } });
const User = db.model('User', userSchema);

const createdUser = await User.create({ name: 'Test' });

const err = await User.bulkWrite([
{
updateOne: {
filter: { _id: createdUser._id },
update: { $set: { age: 'NaN' } },
upsert: true
}
},
{
updateOne: {
filter: { _id: createdUser._id },
update: { $set: { age: 13 } },
upsert: true
}
},
{
updateOne: {
filter: { _id: createdUser._id },
update: { $set: { age: 12 } },
upsert: true
}
}
], { ordered: false, throwOnValidationError: true })
.then(() => null)
.catch(err => err);

assert.ok(err);
assert.equal(err.name, 'MongooseBulkWriteError');
assert.equal(err.validationErrors[0].path, 'age');
assert.equal(err.results[0].path, 'age');
});

it('throwOnValidationError (gh-14572) (gh-13256)', async function() {
const schema = new Schema({
num: Number
});

const M = db.model('Test', schema);

const ops = [
{
insertOne: {
document: {
num: 'not a number'
}
}
}
];

const err = await M.bulkWrite(
ops,
{ ordered: false, throwOnValidationError: true }
).then(() => null, err => err);
assert.ok(err);
assert.equal(err.name, 'MongooseBulkWriteError');
assert.equal(err.validationErrors[0].errors['num'].name, 'CastError');
});
});

it('insertMany with Decimal (gh-5190)', async function() {
Expand Down Expand Up @@ -9028,6 +9093,45 @@ describe('Model', function() {
assert.equal(TestModel.staticFn(), 'Returned from staticFn');
});
});

it('insertMany should throw an error if there were operations that failed validation, ' +
'but all operations that passed validation succeeded (gh-13256)', async function() {
const userSchema = new Schema({
age: { type: Number }
});

const User = db.model('User', userSchema);

let err = await User.insertMany([
new User({ age: 12 }),
new User({ age: 12 }),
new User({ age: 'NaN' })
], { ordered: false, throwOnValidationError: true })
.then(() => null)
.catch(err => err);

assert.ok(err);
assert.equal(err.name, 'MongooseBulkWriteError');
assert.equal(err.validationErrors[0].errors['age'].name, 'CastError');
assert.ok(err.results[2] instanceof Error);
assert.equal(err.results[2].errors['age'].name, 'CastError');

let docs = await User.find();
assert.deepStrictEqual(docs.map(doc => doc.age), [12, 12]);

err = await User.insertMany([
new User({ age: 'NaN' })
], { ordered: false, throwOnValidationError: true })
.then(() => null)
.catch(err => err);

assert.ok(err);
assert.equal(err.name, 'MongooseBulkWriteError');
assert.equal(err.validationErrors[0].errors['age'].name, 'CastError');

docs = await User.find();
assert.deepStrictEqual(docs.map(doc => doc.age), [12, 12]);
});
});


Expand Down
2 changes: 2 additions & 0 deletions types/models.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ declare module 'mongoose' {
skipValidation?: boolean;
strict?: boolean;
timestamps?: boolean | 'throw';
throwOnValidationError?: boolean;
}

interface InsertManyOptions extends
Expand All @@ -28,6 +29,7 @@ declare module 'mongoose' {
rawResult?: boolean;
ordered?: boolean;
lean?: boolean;
throwOnValidationError?: boolean;
}

type InsertManyResult<T> = mongodb.InsertManyResult<T> & {
Expand Down

0 comments on commit 37e73b8

Please sign in to comment.