Skip to content

Commit

Permalink
Merge pull request #13410 from Automattic/vkarpov15/gh-13256
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 May 19, 2023
2 parents 88ba969 + cc2adec commit 8b41e8a
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 2 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;
38 changes: 36 additions & 2 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const setDottedPath = require('./helpers/path/setDottedPath');
const STATES = require('./connectionstate');
const util = require('util');
const utils = require('./utils');
const MongooseBulkWriteError = require('./error/bulkWriteError');

const VERSION_WHERE = 1;
const VERSION_INC = 2;
Expand Down Expand Up @@ -2982,6 +2983,7 @@ Model.startSession = function() {
* @param {Boolean} [options.lean=false] if `true`, skips hydrating and validating the documents. This option is useful if you need the extra performance, but Mongoose won't validate the documents before inserting.
* @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.
* @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 @@ -3027,6 +3029,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 @@ -3133,6 +3136,20 @@ Model.$__insertMany = function(arr, options, callback) {
_setIsNew(attribute, false);
}

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 @@ -3328,6 +3345,7 @@ function _setIsNew(doc, val) {
* @param {Boolean} [options.j=true] If false, disable [journal acknowledgement](https://www.mongodb.com/docs/manual/reference/write-concern/#j-option)
* @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.throwOnValidationError=false] If true and `ordered: false`, throw an error if one of the operations failed validation, but all valid operations completed successfully.
* @param {Boolean} [options.strict=null] Overwrites the [`strict` option](https://mongoosejs.com/docs/guide.html#strict) on schema. If false, allows filtering and writing fields not defined in the schema for all writes in this bulk.
* @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 @@ -3375,12 +3393,14 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
let remaining = validations.length;
let validOps = [];
let validationErrors = [];
const results = [];
for (let i = 0; i < validations.length; ++i) {
validations[i]((err) => {
if (err == null) {
validOps.push(i);
} else {
validationErrors.push({ index: i, error: err });
results[i] = err;
}
if (--remaining <= 0) {
completeUnorderedValidation.call(this);
Expand All @@ -3393,6 +3413,7 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
map(v => v.error);

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

this.$__collection.bulkWrite(validOps, options, (error, res) => {
Expand All @@ -3405,9 +3426,22 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
return reject(error);
}

for (let i = 0; i < validOpIndexes.length; ++i) {
results[validOpIndexes[i]] = null;
}
if (validationErrors.length > 0) {
res.mongoose = res.mongoose || {};
res.mongoose.validationErrors = validationErrors;
if (options.throwOnValidationError) {
return reject(new MongooseBulkWriteError(
validationErrors,
results,
res,
'bulkWrite'
));
} else {
res.mongoose = res.mongoose || {};
res.mongoose.validationErrors = validationErrors;
res.mongoose.results = results;
}
}

resolve(res);
Expand Down
65 changes: 65 additions & 0 deletions test/model.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4189,6 +4189,45 @@ 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('deleteOne with cast error (gh-5323)', async function() {
Expand Down Expand Up @@ -6177,6 +6216,32 @@ describe('Model', function() {

});

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);

const 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');

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

it('returns writeResult on success', async() => {

const userSchema = new Schema({
Expand Down
2 changes: 2 additions & 0 deletions types/models.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ declare module 'mongoose' {

interface MongooseBulkWriteOptions {
skipValidation?: boolean;
throwOnValidationError?: boolean;
}

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

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

0 comments on commit 8b41e8a

Please sign in to comment.