From 55d9d7ef6ff1f62e9fcdc9bcbdcb44fc6c806372 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 12 May 2024 08:04:39 -0400 Subject: [PATCH 1/2] fix(model): make `bulkWrite()` and `insertMany()` throw if `throwOnValidationError` set and all ops invalid Fix #14572 --- lib/model.js | 16 ++++++++++++++++ test/model.test.js | 44 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/lib/model.js b/lib/model.js index 5e0a105c47..13a0a877a8 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3233,6 +3233,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, @@ -3588,6 +3596,14 @@ Model.bulkWrite = async function bulkWrite(ops, options) { validOps = validOps.sort().map(index => ops[index]); if (validOps.length === 0) { + if (options.throwOnValidationError && validationErrors.length) { + throw new MongooseBulkWriteError( + validationErrors, + results, + res, + 'bulkWrite' + ); + } return getDefaultBulkwriteResult(); } diff --git a/test/model.test.js b/test/model.test.js index 9d35f20700..74ba446383 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -4062,7 +4062,32 @@ describe('Model', function() { assert.ok(doc.createdAt.valueOf() >= now.valueOf()); assert.ok(doc.updatedAt); assert.ok(doc.updatedAt.valueOf() >= now.valueOf()); + }); + + it('throwOnValidationError (gh-14572)', 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('with child timestamps and array filters (gh-7032)', async function() { @@ -6602,14 +6627,14 @@ 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() { + 'but all operations that passed validation succeeded (gh-14572) (gh-13256)', async function() { const userSchema = new Schema({ age: { type: Number } }); const User = db.model('User', userSchema); - const err = await User.insertMany([ + let err = await User.insertMany([ new User({ age: 12 }), new User({ age: 12 }), new User({ age: 'NaN' }) @@ -6623,7 +6648,20 @@ describe('Model', function() { assert.ok(err.results[2] instanceof Error); assert.equal(err.results[2].errors['age'].name, 'CastError'); - const docs = await User.find(); + 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]); }); From f8268c56a6679680954dc7cd2a9fc656d200a099 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 16 May 2024 18:08:47 -0400 Subject: [PATCH 2/2] fix(document): ensure `transform` function passed to `toObject()` options applies to subdocs Fix #14589 Re: #14565 --- lib/document.js | 6 ++---- test/document.test.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/document.js b/lib/document.js index 54267d26b9..264a94a668 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3831,16 +3831,14 @@ Document.prototype.$toObject = function(options, json) { // need the original options the user passed in, plus `_isNested` and // `_parentOptions` for checking whether we need to depopulate. const cloneOptions = { + ...options, _isNested: true, json: json, minimize: _minimize, flattenMaps: flattenMaps, flattenObjectIds: flattenObjectIds, _seen: (options && options._seen) || new Map(), - _calledWithOptions: options._calledWithOptions, - virtuals: options.virtuals, - getters: options.getters, - depopulate: options.depopulate + _calledWithOptions: options._calledWithOptions }; const depopulate = options.depopulate || diff --git a/test/document.test.js b/test/document.test.js index b8030900f9..3ce45f384d 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -520,6 +520,34 @@ describe('document', function() { docs.toObject({ transform: true }); }); + it('propagates toObject transform function to all subdocuments (gh-14589)', async function() { + const schema = new mongoose.Schema({ + name: String, + docArr: [{ name: String }], + subdoc: new mongoose.Schema({ name: String }) + }); + const TestModel = db.model('Test', schema); + + const doc = new TestModel({ + name: 'test', + docArr: [{ name: 'test' }], + subdoc: { name: 'test' } + }); + + // pass the transform as an inline option. Deletes `_id` property + // from both the top-level document and the subdocument. + const obj = doc.toObject({ transform: deleteId }); + + assert.equal(obj._id, undefined); + assert.equal(obj.subdoc._id, undefined); + assert.equal(obj.docArr[0]._id, undefined); + + function deleteId(doc, ret) { + delete ret._id; + return ret; + } + }); + it('disabling aliases in toObject options (gh-7548)', function() { const schema = new mongoose.Schema({ name: {