Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Use getters to sync BulkWriteResult wrappers #2716

Merged
merged 2 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 64 additions & 57 deletions src/bulk/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,50 +165,51 @@ export class Batch<T = Document> {
export class BulkWriteResult {
result: BulkResult;

/** Indicates whether this write result was acknowledged. If not, then all other members of this result will be undefined */
// acknowledged: Boolean;
/** Number of documents inserted. */
insertedCount: number;
/** Number of documents matched for update. */
matchedCount: number;
/** Number of documents modified. */
modifiedCount: number;
/** Number of documents deleted. */
deletedCount: number;
/** Number of documents upserted. */
upsertedCount: number;
/** Inserted document generated Id's, hash key is the index of the originating operation */
insertedIds: { [key: number]: ObjectId };
/** Upserted document generated Id's, hash key is the index of the originating operation */
upsertedIds: { [key: number]: ObjectId };

/**
* Create a new BulkWriteResult instance
* @internal
*/
constructor(bulkResult: BulkResult) {
this.result = bulkResult;
this.insertedCount = bulkResult.nInserted;
this.matchedCount = bulkResult.nMatched;
this.modifiedCount = bulkResult.nModified || 0;
this.deletedCount = bulkResult.nRemoved;
this.upsertedCount = bulkResult.upserted.length;
this.upsertedIds = {};
this.insertedIds = {};

// Inserted documents
const inserted = bulkResult.insertedIds;
// Map inserted ids
for (let i = 0; i < inserted.length; i++) {
this.insertedIds[inserted[i].index] = inserted[i]._id;
}

/** Number of documents inserted. */
get insertedCount(): number {
return this.result.nInserted ?? 0;
}
/** Number of documents matched for update. */
get matchedCount(): number {
return this.result.nMatched ?? 0;
}
/** Number of documents modified. */
get modifiedCount(): number {
return this.result.nModified ?? 0;
}
/** Number of documents deleted. */
get deletedCount(): number {
return this.result.nRemoved ?? 0;
}
/** Number of documents upserted. */
get upsertedCount(): number {
return this.result.upserted.length ?? 0;
}

/** Upserted document generated Id's, hash key is the index of the originating operation */
get upsertedIds(): { [key: number]: any } {
const upserted: { [index: number]: any } = {};
for (const doc of this.result.upserted ?? []) {
upserted[doc.index] = doc._id;
}
return upserted;
}

// Upserted documents
const upserted = bulkResult.upserted;
// Map upserted ids
for (let i = 0; i < upserted.length; i++) {
this.upsertedIds[upserted[i].index] = upserted[i]._id;
/** Inserted document generated Id's, hash key is the index of the originating operation */
get insertedIds(): { [key: number]: any } {
const inserted: { [index: number]: any } = {};
for (const doc of this.result.insertedIds ?? []) {
inserted[doc.index] = doc._id;
}
return inserted;
}

/** Evaluates to true if the bulk operation correctly executes */
Expand Down Expand Up @@ -674,36 +675,42 @@ function handleMongoWriteConcernError(
export class MongoBulkWriteError extends MongoError {
result: BulkWriteResult;

/** Number of documents inserted. */
insertedCount: number;
/** Number of documents matched for update. */
matchedCount: number;
/** Number of documents modified. */
modifiedCount: number;
/** Number of documents deleted. */
deletedCount: number;
/** Number of documents upserted. */
upsertedCount: number;
/** Inserted document generated Id's, hash key is the index of the originating operation */
insertedIds: { [key: number]: ObjectId };
/** Upserted document generated Id's, hash key is the index of the originating operation */
upsertedIds: { [key: number]: ObjectId };

/** Creates a new MongoBulkWriteError */
constructor(error: AnyError, result: BulkWriteResult) {
super(error as Error);
Object.assign(this, error);

this.name = 'MongoBulkWriteError';
this.result = result;
}

this.insertedCount = result.insertedCount;
this.matchedCount = result.matchedCount;
this.modifiedCount = result.modifiedCount;
this.deletedCount = result.deletedCount;
this.upsertedCount = result.upsertedCount;
this.insertedIds = result.insertedIds;
this.upsertedIds = result.upsertedIds;
/** Number of documents inserted. */
get insertedCount(): number {
return this.result.insertedCount;
}
/** Number of documents matched for update. */
get matchedCount(): number {
return this.result.matchedCount;
}
/** Number of documents modified. */
get modifiedCount(): number {
return this.result.modifiedCount;
}
/** Number of documents deleted. */
get deletedCount(): number {
return this.result.deletedCount;
}
/** Number of documents upserted. */
get upsertedCount(): number {
return this.result.upsertedCount;
}
/** Inserted document generated Id's, hash key is the index of the originating operation */
get insertedIds(): { [key: number]: any } {
return this.result.insertedIds;
}
/** Upserted document generated Id's, hash key is the index of the originating operation */
get upsertedIds(): { [key: number]: any } {
return this.result.upsertedIds;
}
}

Expand Down
46 changes: 44 additions & 2 deletions test/functional/insert.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const { assert: test, withClient } = require('./shared');
const { assert: test, withClient, ignoreNsNotFound } = require('./shared');
const { setupDatabase } = require('./shared');
const Script = require('vm');
const { expect } = require('chai');
Expand All @@ -15,7 +15,8 @@ const {
Binary,
MinKey,
MaxKey,
Code
Code,
MongoBulkWriteError
} = require('../../src');

/**
Expand Down Expand Up @@ -2583,4 +2584,45 @@ describe('Insert', function () {
});
}
});

it('MongoBulkWriteError.insertedCount should respect BulkWriteResult.insertedCount should respect BulkWrite.nInserted', function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('MongoBulkWriteError.insertedCount should respect BulkWriteResult.insertedCount should respect BulkWrite.nInserted', function () {
it('MongoBulkWriteError.insertedCount should respect BulkWrite.nInserted', function () {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give this a better name but this was actually correct MongoBulkWriteError holds a BulkWriteResult which holds the BulkWrite so this test makes sure the values thread correctly through the two wrapping classes

const client = this.configuration.newClient();
return client
.connect()
.then(() => {
return client.db().collection('test_insertMany_bulkResult').drop();
})
.catch(ignoreNsNotFound)
.then(() => {
const collection = client.db().collection('test_insertMany_bulkResult');
return collection.insertMany(
[
{ _id: 2, x: 22 },
{ _id: 2, x: 22 },
{ _id: 3, x: 33 }
],
{ ordered: false }
);
})
.then(() => {
expect.fail('InsertMany should fail with multi key error');
})
.catch(error => {
expect(error).to.be.instanceOf(MongoBulkWriteError);
expect(
error.insertedCount,
'MongoBulkWriteError.insertedCount did not respect BulkResult.nInserted'
).to.equal(error.result.result.nInserted);
expect(
error.result.insertedCount,
'BulkWriteResult.insertedCount did not respect BulkResult.nInserted'
).to.equal(error.result.result.nInserted);
expect(
error.result.result.nInserted,
'BulkWrite did not correctly represent the operation'
).to.equal(2);
})
.finally(() => client.db().collection('test_insertMany_bulkResult').drop())
.finally(() => client.close());
});
});