From d766ae22f3cfc6679ee3f7c068ebb8596beb4331 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB <106987683+aditi-khare-mongoDB@users.noreply.github.com> Date: Fri, 29 Sep 2023 10:30:56 -0400 Subject: [PATCH] fix(NODE-5627): BulkWriteResult.insertedIds includes ids that were not inserted (#3870) --- src/bulk/common.ts | 45 +++++++++-- test/integration/crud/bulk.test.ts | 116 +++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 7 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 1c3b5fa5dc..328c6e2769 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -195,7 +195,7 @@ export class BulkWriteResult { * Create a new BulkWriteResult instance * @internal */ - constructor(bulkResult: BulkResult) { + constructor(bulkResult: BulkResult, isOrdered: boolean) { this.result = bulkResult; this.insertedCount = this.result.nInserted ?? 0; this.matchedCount = this.result.nMatched ?? 0; @@ -203,10 +203,28 @@ export class BulkWriteResult { this.deletedCount = this.result.nRemoved ?? 0; this.upsertedCount = this.result.upserted.length ?? 0; this.upsertedIds = BulkWriteResult.generateIdMap(this.result.upserted); - this.insertedIds = BulkWriteResult.generateIdMap(this.result.insertedIds); + this.insertedIds = BulkWriteResult.generateIdMap( + this.getSuccessfullyInsertedIds(bulkResult, isOrdered) + ); Object.defineProperty(this, 'result', { value: this.result, enumerable: false }); } + /** + * Returns document_ids that were actually inserted + * @internal + */ + private getSuccessfullyInsertedIds(bulkResult: BulkResult, isOrdered: boolean): Document[] { + if (bulkResult.writeErrors.length === 0) return bulkResult.insertedIds; + + if (isOrdered) { + return bulkResult.insertedIds.slice(0, bulkResult.writeErrors[0].index); + } + + return bulkResult.insertedIds.filter( + ({ index }) => !bulkResult.writeErrors.some(writeError => index === writeError.index) + ); + } + /** Evaluates to true if the bulk operation correctly executes */ get ok(): number { return this.result.ok; @@ -533,7 +551,10 @@ function executeCommands( callback: Callback ) { if (bulkOperation.s.batches.length === 0) { - return callback(undefined, new BulkWriteResult(bulkOperation.s.bulkResult)); + return callback( + undefined, + new BulkWriteResult(bulkOperation.s.bulkResult, bulkOperation.isOrdered) + ); } const batch = bulkOperation.s.batches.shift() as Batch; @@ -542,17 +563,26 @@ function executeCommands( // Error is a driver related error not a bulk op error, return early if (err && 'message' in err && !(err instanceof MongoWriteConcernError)) { return callback( - new MongoBulkWriteError(err, new BulkWriteResult(bulkOperation.s.bulkResult)) + new MongoBulkWriteError( + err, + new BulkWriteResult(bulkOperation.s.bulkResult, bulkOperation.isOrdered) + ) ); } if (err instanceof MongoWriteConcernError) { - return handleMongoWriteConcernError(batch, bulkOperation.s.bulkResult, err, callback); + return handleMongoWriteConcernError( + batch, + bulkOperation.s.bulkResult, + bulkOperation.isOrdered, + err, + callback + ); } // Merge the results together mergeBatchResults(batch, bulkOperation.s.bulkResult, err, result); - const writeResult = new BulkWriteResult(bulkOperation.s.bulkResult); + const writeResult = new BulkWriteResult(bulkOperation.s.bulkResult, bulkOperation.isOrdered); if (bulkOperation.handleWriteError(callback, writeResult)) return; // Execute the next command in line @@ -626,6 +656,7 @@ function executeCommands( function handleMongoWriteConcernError( batch: Batch, bulkResult: BulkResult, + isOrdered: boolean, err: MongoWriteConcernError, callback: Callback ) { @@ -637,7 +668,7 @@ function handleMongoWriteConcernError( message: err.result?.writeConcernError.errmsg, code: err.result?.writeConcernError.result }, - new BulkWriteResult(bulkResult) + new BulkWriteResult(bulkResult, isOrdered) ) ); } diff --git a/test/integration/crud/bulk.test.ts b/test/integration/crud/bulk.test.ts index e54b6fe86f..b583a008f1 100644 --- a/test/integration/crud/bulk.test.ts +++ b/test/integration/crud/bulk.test.ts @@ -5,6 +5,7 @@ import { type Collection, Long, MongoBatchReExecutionError, + MongoBulkWriteError, type MongoClient, MongoDriverError, MongoInvalidArgumentError @@ -104,6 +105,121 @@ describe('Bulk', function () { } }); }); + + context('when inserting duplicate values', function () { + let col; + + beforeEach(async function () { + const db = client.db(); + col = db.collection('test'); + await col.createIndex([{ a: 1 }], { unique: true, sparse: false }); + }); + + async function assertFailsWithDuplicateFields(input, isOrdered, expectedInsertedIds) { + const error = await col.insertMany(input, { ordered: isOrdered }).catch(error => error); + expect(error).to.be.instanceOf(MongoBulkWriteError); + expect(error.result.insertedCount).to.equal(Object.keys(error.result.insertedIds).length); + expect(error.result.insertedIds).to.deep.equal(expectedInsertedIds); + } + + context('when the insert is ordered', function () { + it('contains the correct insertedIds on one duplicate insert', async function () { + await assertFailsWithDuplicateFields( + [ + { _id: 0, a: 1 }, + { _id: 1, a: 1 } + ], + true, + { 0: 0 } + ); + }); + + it('contains the correct insertedIds on multiple duplicate inserts', async function () { + await assertFailsWithDuplicateFields( + [ + { _id: 0, a: 1 }, + { _id: 1, a: 1 }, + { _id: 2, a: 1 }, + { _id: 3, b: 2 } + ], + true, + { 0: 0 } + ); + }); + }); + + context('when the insert is unordered', function () { + it('contains the correct insertedIds on multiple duplicate inserts', async function () { + await assertFailsWithDuplicateFields( + [ + { _id: 0, a: 1 }, + { _id: 1, a: 1 }, + { _id: 2, a: 1 }, + { _id: 3, b: 2 } + ], + false, + { 0: 0, 3: 3 } + ); + }); + }); + }); + }); + + describe('#bulkWrite()', function () { + context('when inserting duplicate values', function () { + let col; + + beforeEach(async function () { + const db = client.db(); + col = db.collection('test'); + await col.createIndex([{ a: 1 }], { unique: true, sparse: false }); + }); + + async function assertFailsWithDuplicateFields(input, isOrdered, expectedInsertedIds) { + const error = await col.bulkWrite(input, { ordered: isOrdered }).catch(error => error); + expect(error).to.be.instanceOf(MongoBulkWriteError); + expect(error.result.insertedCount).to.equal(Object.keys(error.result.insertedIds).length); + expect(error.result.insertedIds).to.deep.equal(expectedInsertedIds); + } + + context('when the insert is ordered', function () { + it('contains the correct insertedIds on one duplicate insert', async function () { + await assertFailsWithDuplicateFields( + [{ insertOne: { _id: 0, a: 1 } }, { insertOne: { _id: 1, a: 1 } }], + true, + { 0: 0 } + ); + }); + + it('contains the correct insertedIds on multiple duplicate inserts', async function () { + await assertFailsWithDuplicateFields( + [ + { insertOne: { _id: 0, a: 1 } }, + { insertOne: { _id: 1, a: 1 } }, + { insertOne: { _id: 2, a: 1 } }, + { insertOne: { _id: 3, b: 2 } } + ], + true, + { 0: 0 } + ); + }); + }); + + context('when the insert is unordered', function () { + it('contains the correct insertedIds on multiple duplicate inserts', async function () { + await assertFailsWithDuplicateFields( + [ + { insertOne: { _id: 0, a: 1 } }, + { insertOne: { _id: 1, a: 1 } }, + { insertOne: { _id: 2, a: 1 } }, + { insertOne: { _id: 3, b: 2 } } + ], + false, + { 0: 0, 3: 3 } + ); + }); + }); + }); }); });