diff --git a/lib/model.js b/lib/model.js index e027087215..b3c79564b8 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3364,11 +3364,18 @@ 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: + * + * - `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 + * + * 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. @@ -3376,7 +3383,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 || {}; @@ -3404,18 +3411,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 855f1eecb8..6abf954885 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -6939,6 +6939,31 @@ describe('Model', function() { assert.ok(err == null); }); + it('should error if no documents were inserted (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('Using bulkSave should not trigger an error (gh-11071)', async function() { const pairSchema = mongoose.Schema({