Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(model): ensure consistent ordering of validation errors in insertMany() with ordered: false and rawResult: true #12866

Merged
merged 3 commits into from Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 40 additions & 1 deletion lib/model.js
Expand Up @@ -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 {
Expand All @@ -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);
Expand All @@ -3450,14 +3452,38 @@ 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,
// `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
}
Expand Down Expand Up @@ -3490,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) => {
Expand All @@ -3513,6 +3546,12 @@ Model.$__insertMany = function(arr, options, callback) {
return doc;
});

if (rawResult && ordered === false) {
error.mongoose = {
validationErrors: validationErrors
};
}

callback(error, null);
return;
}
Expand Down
61 changes: 61 additions & 0 deletions test/model.test.js
Expand Up @@ -4791,6 +4791,67 @@ 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() 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 }
Expand Down