Skip to content

Commit

Permalink
fix(model): throw error if bulkSave() did not insert or update any …
Browse files Browse the repository at this point in the history
…documents

Re: #14763
  • Loading branch information
vkarpov15 committed Aug 28, 2024
1 parent 05055f0 commit 8ce32c0
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 14 deletions.
48 changes: 34 additions & 14 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -3364,19 +3364,26 @@ 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<Document>} 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.
* @param {ClientSession} [options.session=null] The session associated with this bulk write. See [transactions docs](https://mongoosejs.com/docs/transactions.html).
* @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 || {};
Expand Down Expand Up @@ -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;
Expand Down
25 changes: 25 additions & 0 deletions test/model.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down

0 comments on commit 8ce32c0

Please sign in to comment.