diff --git a/lib/model.js b/lib/model.js index e83a61be4f..0a1091ab2b 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3368,11 +3368,19 @@ Model.bulkWrite = async function bulkWrite(ops, options) { }; /** - * takes an array of documents, gets the changes and inserts/updates documents in the database - * according to whether or not the document is new, or whether it has changes or not. + * Takes an array of documents, gets the changes and inserts/updates documents in the database + * according to whether or not the document is new, or whether it has changes or not. * * `bulkSave` uses `bulkWrite` under the hood, so it's mostly useful when dealing with many documents (10K+) * + * `bulkSave()` throws errors under the following conditions: + * + * - one of the provided documents fails validation. In this case, `bulkSave()` does not send a `bulkWrite()`, and throws the first validation error. + * - `bulkWrite()` fails (for example, due to being unable to connect to MongoDB or due to duplicate key error) + * - `bulkWrite()` did not insert or update **any** documents. In this case, `bulkSave()` will throw a DocumentNotFound error. + * + * Note that `bulkSave()` will **not** throw an error if only some of the `save()` calls succeeded. + * * @param {Array} documents * @param {Object} [options] options passed to the underlying `bulkWrite()` * @param {Boolean} [options.timestamps] defaults to `null`, when set to false, mongoose will not add/update timestamps to the documents. @@ -3380,7 +3388,7 @@ Model.bulkWrite = async function bulkWrite(ops, options) { * @param {String|number} [options.w=1] The [write concern](https://www.mongodb.com/docs/manual/reference/write-concern/). See [`Query#w()`](https://mongoosejs.com/docs/api/query.html#Query.prototype.w()) for more information. * @param {number} [options.wtimeout=null] The [write concern timeout](https://www.mongodb.com/docs/manual/reference/write-concern/#wtimeout). * @param {Boolean} [options.j=true] If false, disable [journal acknowledgement](https://www.mongodb.com/docs/manual/reference/write-concern/#j-option) - * + * @return {BulkWriteResult} the return value from `bulkWrite()` */ Model.bulkSave = async function bulkSave(documents, options) { options = options || {}; @@ -3408,18 +3416,31 @@ Model.bulkSave = async function bulkSave(documents, options) { (err) => ({ bulkWriteResult: null, bulkWriteError: err }) ); - await Promise.all( - documents.map(async(document) => { - const documentError = bulkWriteError && bulkWriteError.writeErrors.find(writeError => { - const writeErrorDocumentId = writeError.err.op._id || writeError.err.op.q._id; - return writeErrorDocumentId.toString() === document._doc._id.toString(); - }); + const matchedCount = bulkWriteResult?.matchedCount ?? 0; + const insertedCount = bulkWriteResult?.insertedCount ?? 0; + if (writeOperations.length > 0 && matchedCount + insertedCount === 0 && !bulkWriteError) { + throw new DocumentNotFoundError( + writeOperations.filter(op => op.updateOne).map(op => op.updateOne.filter), + this.modelName, + writeOperations.length, + bulkWriteResult + ); + } - if (documentError == null) { - await handleSuccessfulWrite(document); - } - }) - ); + const successfulDocuments = []; + for (let i = 0; i < documents.length; i++) { + const document = documents[i]; + const documentError = bulkWriteError && bulkWriteError.writeErrors.find(writeError => { + const writeErrorDocumentId = writeError.err.op._id || writeError.err.op.q._id; + return writeErrorDocumentId.toString() === document._doc._id.toString(); + }); + + if (documentError == null) { + successfulDocuments.push(document); + } + } + + await Promise.all(successfulDocuments.map(document => handleSuccessfulWrite(document))); if (bulkWriteError && bulkWriteError.writeErrors && bulkWriteError.writeErrors.length) { throw bulkWriteError; diff --git a/test/model.test.js b/test/model.test.js index f6943d96fc..8e94a87669 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -6973,6 +6973,48 @@ describe('Model', function() { assert.ok(err == null); }); + it('should error if no documents were inserted or updated (gh-14763)', async function() { + const fooSchema = new mongoose.Schema({ + bar: { type: Number } + }, { optimisticConcurrency: true }); + const TestModel = db.model('Test', fooSchema); + + const foo = await TestModel.create({ + bar: 0 + }); + + // update 1 + foo.bar = 1; + await foo.save(); + + // parallel update + const fooCopy = await TestModel.findById(foo._id); + fooCopy.bar = 99; + await fooCopy.save(); + + foo.bar = 2; + const err = await TestModel.bulkSave([foo]).then(() => null, err => err); + assert.equal(err.name, 'DocumentNotFoundError'); + assert.equal(err.numAffected, 1); + assert.ok(Array.isArray(err.filter)); + }); + it('should error if there is a validation error', async function() { + const fooSchema = new mongoose.Schema({ + bar: { type: Number } + }, { optimisticConcurrency: true }); + const TestModel = db.model('Test', fooSchema); + + const docs = [ + new TestModel({ bar: 42 }), + new TestModel({ bar: 'taco' }) + ]; + const err = await TestModel.bulkSave(docs).then(() => null, err => err); + assert.equal(err.name, 'ValidationError'); + + // bulkSave() does not save any documents if any documents fail validation + const fromDb = await TestModel.find(); + assert.equal(fromDb.length, 0); + }); it('Using bulkSave should not trigger an error (gh-11071)', async function() { const pairSchema = mongoose.Schema({