Skip to content

Commit d766ae2

Browse files
fix(NODE-5627): BulkWriteResult.insertedIds includes ids that were not inserted (#3870)
1 parent 6f67539 commit d766ae2

File tree

2 files changed

+154
-7
lines changed

2 files changed

+154
-7
lines changed

src/bulk/common.ts

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,18 +195,36 @@ export class BulkWriteResult {
195195
* Create a new BulkWriteResult instance
196196
* @internal
197197
*/
198-
constructor(bulkResult: BulkResult) {
198+
constructor(bulkResult: BulkResult, isOrdered: boolean) {
199199
this.result = bulkResult;
200200
this.insertedCount = this.result.nInserted ?? 0;
201201
this.matchedCount = this.result.nMatched ?? 0;
202202
this.modifiedCount = this.result.nModified ?? 0;
203203
this.deletedCount = this.result.nRemoved ?? 0;
204204
this.upsertedCount = this.result.upserted.length ?? 0;
205205
this.upsertedIds = BulkWriteResult.generateIdMap(this.result.upserted);
206-
this.insertedIds = BulkWriteResult.generateIdMap(this.result.insertedIds);
206+
this.insertedIds = BulkWriteResult.generateIdMap(
207+
this.getSuccessfullyInsertedIds(bulkResult, isOrdered)
208+
);
207209
Object.defineProperty(this, 'result', { value: this.result, enumerable: false });
208210
}
209211

212+
/**
213+
* Returns document_ids that were actually inserted
214+
* @internal
215+
*/
216+
private getSuccessfullyInsertedIds(bulkResult: BulkResult, isOrdered: boolean): Document[] {
217+
if (bulkResult.writeErrors.length === 0) return bulkResult.insertedIds;
218+
219+
if (isOrdered) {
220+
return bulkResult.insertedIds.slice(0, bulkResult.writeErrors[0].index);
221+
}
222+
223+
return bulkResult.insertedIds.filter(
224+
({ index }) => !bulkResult.writeErrors.some(writeError => index === writeError.index)
225+
);
226+
}
227+
210228
/** Evaluates to true if the bulk operation correctly executes */
211229
get ok(): number {
212230
return this.result.ok;
@@ -533,7 +551,10 @@ function executeCommands(
533551
callback: Callback<BulkWriteResult>
534552
) {
535553
if (bulkOperation.s.batches.length === 0) {
536-
return callback(undefined, new BulkWriteResult(bulkOperation.s.bulkResult));
554+
return callback(
555+
undefined,
556+
new BulkWriteResult(bulkOperation.s.bulkResult, bulkOperation.isOrdered)
557+
);
537558
}
538559

539560
const batch = bulkOperation.s.batches.shift() as Batch;
@@ -542,17 +563,26 @@ function executeCommands(
542563
// Error is a driver related error not a bulk op error, return early
543564
if (err && 'message' in err && !(err instanceof MongoWriteConcernError)) {
544565
return callback(
545-
new MongoBulkWriteError(err, new BulkWriteResult(bulkOperation.s.bulkResult))
566+
new MongoBulkWriteError(
567+
err,
568+
new BulkWriteResult(bulkOperation.s.bulkResult, bulkOperation.isOrdered)
569+
)
546570
);
547571
}
548572

549573
if (err instanceof MongoWriteConcernError) {
550-
return handleMongoWriteConcernError(batch, bulkOperation.s.bulkResult, err, callback);
574+
return handleMongoWriteConcernError(
575+
batch,
576+
bulkOperation.s.bulkResult,
577+
bulkOperation.isOrdered,
578+
err,
579+
callback
580+
);
551581
}
552582

553583
// Merge the results together
554584
mergeBatchResults(batch, bulkOperation.s.bulkResult, err, result);
555-
const writeResult = new BulkWriteResult(bulkOperation.s.bulkResult);
585+
const writeResult = new BulkWriteResult(bulkOperation.s.bulkResult, bulkOperation.isOrdered);
556586
if (bulkOperation.handleWriteError(callback, writeResult)) return;
557587

558588
// Execute the next command in line
@@ -626,6 +656,7 @@ function executeCommands(
626656
function handleMongoWriteConcernError(
627657
batch: Batch,
628658
bulkResult: BulkResult,
659+
isOrdered: boolean,
629660
err: MongoWriteConcernError,
630661
callback: Callback<BulkWriteResult>
631662
) {
@@ -637,7 +668,7 @@ function handleMongoWriteConcernError(
637668
message: err.result?.writeConcernError.errmsg,
638669
code: err.result?.writeConcernError.result
639670
},
640-
new BulkWriteResult(bulkResult)
671+
new BulkWriteResult(bulkResult, isOrdered)
641672
)
642673
);
643674
}

test/integration/crud/bulk.test.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
type Collection,
66
Long,
77
MongoBatchReExecutionError,
8+
MongoBulkWriteError,
89
type MongoClient,
910
MongoDriverError,
1011
MongoInvalidArgumentError
@@ -104,6 +105,121 @@ describe('Bulk', function () {
104105
}
105106
});
106107
});
108+
109+
context('when inserting duplicate values', function () {
110+
let col;
111+
112+
beforeEach(async function () {
113+
const db = client.db();
114+
col = db.collection('test');
115+
await col.createIndex([{ a: 1 }], { unique: true, sparse: false });
116+
});
117+
118+
async function assertFailsWithDuplicateFields(input, isOrdered, expectedInsertedIds) {
119+
const error = await col.insertMany(input, { ordered: isOrdered }).catch(error => error);
120+
expect(error).to.be.instanceOf(MongoBulkWriteError);
121+
expect(error.result.insertedCount).to.equal(Object.keys(error.result.insertedIds).length);
122+
expect(error.result.insertedIds).to.deep.equal(expectedInsertedIds);
123+
}
124+
125+
context('when the insert is ordered', function () {
126+
it('contains the correct insertedIds on one duplicate insert', async function () {
127+
await assertFailsWithDuplicateFields(
128+
[
129+
{ _id: 0, a: 1 },
130+
{ _id: 1, a: 1 }
131+
],
132+
true,
133+
{ 0: 0 }
134+
);
135+
});
136+
137+
it('contains the correct insertedIds on multiple duplicate inserts', async function () {
138+
await assertFailsWithDuplicateFields(
139+
[
140+
{ _id: 0, a: 1 },
141+
{ _id: 1, a: 1 },
142+
{ _id: 2, a: 1 },
143+
{ _id: 3, b: 2 }
144+
],
145+
true,
146+
{ 0: 0 }
147+
);
148+
});
149+
});
150+
151+
context('when the insert is unordered', function () {
152+
it('contains the correct insertedIds on multiple duplicate inserts', async function () {
153+
await assertFailsWithDuplicateFields(
154+
[
155+
{ _id: 0, a: 1 },
156+
{ _id: 1, a: 1 },
157+
{ _id: 2, a: 1 },
158+
{ _id: 3, b: 2 }
159+
],
160+
false,
161+
{ 0: 0, 3: 3 }
162+
);
163+
});
164+
});
165+
});
166+
});
167+
168+
describe('#bulkWrite()', function () {
169+
context('when inserting duplicate values', function () {
170+
let col;
171+
172+
beforeEach(async function () {
173+
const db = client.db();
174+
col = db.collection('test');
175+
await col.createIndex([{ a: 1 }], { unique: true, sparse: false });
176+
});
177+
178+
async function assertFailsWithDuplicateFields(input, isOrdered, expectedInsertedIds) {
179+
const error = await col.bulkWrite(input, { ordered: isOrdered }).catch(error => error);
180+
expect(error).to.be.instanceOf(MongoBulkWriteError);
181+
expect(error.result.insertedCount).to.equal(Object.keys(error.result.insertedIds).length);
182+
expect(error.result.insertedIds).to.deep.equal(expectedInsertedIds);
183+
}
184+
185+
context('when the insert is ordered', function () {
186+
it('contains the correct insertedIds on one duplicate insert', async function () {
187+
await assertFailsWithDuplicateFields(
188+
[{ insertOne: { _id: 0, a: 1 } }, { insertOne: { _id: 1, a: 1 } }],
189+
true,
190+
{ 0: 0 }
191+
);
192+
});
193+
194+
it('contains the correct insertedIds on multiple duplicate inserts', async function () {
195+
await assertFailsWithDuplicateFields(
196+
[
197+
{ insertOne: { _id: 0, a: 1 } },
198+
{ insertOne: { _id: 1, a: 1 } },
199+
{ insertOne: { _id: 2, a: 1 } },
200+
{ insertOne: { _id: 3, b: 2 } }
201+
],
202+
true,
203+
{ 0: 0 }
204+
);
205+
});
206+
});
207+
208+
context('when the insert is unordered', function () {
209+
it('contains the correct insertedIds on multiple duplicate inserts', async function () {
210+
await assertFailsWithDuplicateFields(
211+
[
212+
{ insertOne: { _id: 0, a: 1 } },
213+
{ insertOne: { _id: 1, a: 1 } },
214+
{ insertOne: { _id: 2, a: 1 } },
215+
{ insertOne: { _id: 3, b: 2 } }
216+
],
217+
false,
218+
{ 0: 0, 3: 3 }
219+
);
220+
});
221+
});
222+
});
107223
});
108224
});
109225

0 commit comments

Comments
 (0)