From a90f97c83c738d085e72962ded778a3ee420346f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 3 Jan 2023 11:25:05 -0500 Subject: [PATCH 1/3] fix(model): ensure consistent ordering of validation errors in `insertMany()` with `ordered: false` and `rawResult: true` Re: #12791 --- lib/model.js | 18 +++++++++++++++++- test/model.test.js | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/model.js b/lib/model.js index a146eea1ef0..b77e8c5118a 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3411,7 +3411,8 @@ Model.$__insertMany = function(arr, options, callback) { } const validationErrors = []; - const toExecute = arr.map(doc => + const validationErrorsToOriginalOrder = new Map(); + const toExecute = arr.map((doc, index) => callback => { if (!(doc instanceof _this)) { try { @@ -3437,6 +3438,7 @@ Model.$__insertMany = function(arr, options, callback) { // failed. It's up to the next function to filter out all failed models if (ordered === false) { validationErrors.push(error); + validationErrorsToOriginalOrder.set(error, index); return callback(null, null); } return callback(error); @@ -3454,10 +3456,24 @@ Model.$__insertMany = function(arr, options, callback) { const docAttributes = docs.filter(function(doc) { return doc != null; }); + + // Make sure validation errors are in the same order as the + // original documents, so if both doc1 and doc2 both fail validation, + // `Model.insertMany([doc1, doc2])` will always have doc1's validation + // error before doc2's. Re: gh-12791. + if (validationErrors.length > 0) { + validationErrors.sort((err1, err2) => { + return validationErrorsToOriginalOrder.get(err1) - validationErrorsToOriginalOrder.get(err2); + }); + } + // Quickly escape while there aren't any valid docAttributes if (docAttributes.length === 0) { if (rawResult) { const res = { + acknowledged: true, + insertedCount: 0, + insertedIds: {}, mongoose: { validationErrors: validationErrors } diff --git a/test/model.test.js b/test/model.test.js index 069a5f64502..39ebf883cb5 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -4753,7 +4753,7 @@ describe('Model', function() { }); }); - it('insertMany() validation error with ordered true when all documents are invalid', function(done) { + it('insertMany() validation error with ordered true when all documents are invalid (', function(done) { const schema = new Schema({ name: { type: String, required: true } }); @@ -4791,6 +4791,38 @@ describe('Model', function() { }); }); + it('insertMany() validation error with ordered false and rawResult for checking which documents failed (gh-12791)', async function() { + const schema = new Schema({ + name: { type: String, required: true }, + year: { type: Number, required: true } + }); + const Movie = db.model('Movie', schema); + + const id1 = new mongoose.Types.ObjectId(); + const id2 = new mongoose.Types.ObjectId(); + const id3 = new mongoose.Types.ObjectId(); + const arr = [ + { _id: id1, foo: 'The Phantom Menace', year: 1999 }, + { _id: id2, name: 'The Force Awakens', bar: 2015 }, + { _id: id3, name: 'The Empire Strikes Back', year: 1980 } + ]; + const opts = { ordered: false, rawResult: true }; + const res = await Movie.insertMany(arr, opts); + // { + // acknowledged: true, + // insertedCount: 1, + // insertedIds: { '0': new ObjectId("63b34b062cfe38622738e510") }, + // mongoose: { validationErrors: [ [Error], [Error] ] } + // } + assert.equal(res.insertedCount, 1); + assert.equal(res.insertedIds[0].toHexString(), id3.toHexString()); + assert.equal(res.mongoose.validationErrors.length, 2); + assert.ok(res.mongoose.validationErrors[0].errors['name']); + assert.ok(!res.mongoose.validationErrors[0].errors['year']); + assert.ok(res.mongoose.validationErrors[1].errors['year']); + assert.ok(!res.mongoose.validationErrors[1].errors['name']); + }); + it('insertMany() populate option (gh-9720)', async function() { const schema = new Schema({ name: { type: String, required: true } From a1f7f0a631e847fdd2ac7f52b89cf9140246b6fb Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 3 Jan 2023 11:33:46 -0500 Subject: [PATCH 2/3] style: quick fix --- test/model.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/model.test.js b/test/model.test.js index 39ebf883cb5..768fc08ccc3 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -4753,7 +4753,7 @@ describe('Model', function() { }); }); - it('insertMany() validation error with ordered true when all documents are invalid (', function(done) { + it('insertMany() validation error with ordered true when all documents are invalid', function(done) { const schema = new Schema({ name: { type: String, required: true } }); From 9303f7dd07d146e2ef6d41f45dad8bcc333ae2df Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 3 Jan 2023 12:08:57 -0500 Subject: [PATCH 3/3] fix: correct writeError index when insertMany() with ordered: false, rawResult: true with mixed validation error and write error re: #12791 --- lib/model.js | 23 +++++++++++++++++++++++ test/model.test.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/lib/model.js b/lib/model.js index b77e8c5118a..d3b995eff8e 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3452,10 +3452,20 @@ Model.$__insertMany = function(arr, options, callback) { callback(error, null); return; } + + const originalDocIndex = new Map(); + const validDocIndexToOriginalIndex = new Map(); + for (let i = 0; i < docs.length; ++i) { + originalDocIndex.set(docs[i], i); + } + // We filter all failed pre-validations by removing nulls const docAttributes = docs.filter(function(doc) { return doc != null; }); + for (let i = 0; i < docAttributes.length; ++i) { + validDocIndexToOriginalIndex.set(i, originalDocIndex.get(docAttributes[i])); + } // Make sure validation errors are in the same order as the // original documents, so if both doc1 and doc2 both fail validation, @@ -3506,6 +3516,13 @@ Model.$__insertMany = function(arr, options, callback) { // `insertedDocs` is a Mongoose-specific property const erroredIndexes = new Set((error && error.writeErrors || []).map(err => err.index)); + for (let i = 0; i < error.writeErrors.length; ++i) { + error.writeErrors[i] = { + ...error.writeErrors[i], + index: validDocIndexToOriginalIndex.get(error.writeErrors[i].index) + }; + } + let firstErroredIndex = -1; error.insertedDocs = docAttributes. filter((doc, i) => { @@ -3529,6 +3546,12 @@ Model.$__insertMany = function(arr, options, callback) { return doc; }); + if (rawResult && ordered === false) { + error.mongoose = { + validationErrors: validationErrors + }; + } + callback(error, null); return; } diff --git a/test/model.test.js b/test/model.test.js index 768fc08ccc3..eb68df34476 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -4823,6 +4823,35 @@ describe('Model', function() { assert.ok(!res.mongoose.validationErrors[1].errors['name']); }); + it('insertMany() validation error with ordered false and rawResult for mixed write and validation error (gh-12791)', async function() { + const schema = new Schema({ + name: { type: String, required: true, unique: true }, + year: { type: Number, required: true } + }); + const Movie = db.model('Movie', schema); + await Movie.init(); + + const arr = [ + { foo: 'The Phantom Menace', year: 1999 }, + { name: 'The Force Awakens', bar: 2015 }, + { name: 'The Empire Strikes Back', year: 1980 }, + { name: 'The Empire Strikes Back', year: 1980 } + ]; + const opts = { ordered: false, rawResult: true }; + const err = await Movie.insertMany(arr, opts).then(() => null, err => err); + + assert.ok(err); + assert.equal(err.insertedDocs.length, 1); + assert.equal(err.insertedDocs[0].name, 'The Empire Strikes Back'); + assert.equal(err.writeErrors.length, 1); + assert.equal(err.writeErrors[0].index, 3); + assert.equal(err.mongoose.validationErrors.length, 2); + assert.ok(err.mongoose.validationErrors[0].errors['name']); + assert.ok(!err.mongoose.validationErrors[0].errors['year']); + assert.ok(err.mongoose.validationErrors[1].errors['year']); + assert.ok(!err.mongoose.validationErrors[1].errors['name']); + }); + it('insertMany() populate option (gh-9720)', async function() { const schema = new Schema({ name: { type: String, required: true }