Skip to content

Commit

Permalink
fix: only force server id generation if requested
Browse files Browse the repository at this point in the history
An inverted boolean expression accidentally always forced the
driver to force the server to generate document ids.

NODE-2923
  • Loading branch information
mbroadst committed Dec 2, 2020
1 parent 0cca729 commit 4de414a
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/bulk/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ export abstract class BulkOperationBase {
* ```
*/
insert(document: Document): BulkOperationBase {
if (document._id == null && shouldForceServerObjectId(this)) {
if (document._id == null && !shouldForceServerObjectId(this)) {
document._id = new ObjectId();
}

Expand Down
18 changes: 1 addition & 17 deletions test/functional/abstract_cursor.test.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,6 @@
'use strict';
const { expect } = require('chai');
const { filterForCommands } = require('./shared');

function withClientV2(callback) {
return function testFunction(done) {
const client = this.configuration.newClient({ monitorCommands: true });
client.connect(err => {
if (err) return done(err);
this.defer(() => client.close());

try {
callback.call(this, client, done);
} catch (err) {
done(err);
}
});
};
}
const { filterForCommands, withClientV2 } = require('./shared');

describe('AbstractCursor', function () {
before(
Expand Down
18 changes: 17 additions & 1 deletion test/functional/bulk.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const { withClient, setupDatabase, ignoreNsNotFound } = require('./shared');
const { withClient, withClientV2, setupDatabase, ignoreNsNotFound } = require('./shared');
const test = require('./shared').assert;
const { expect } = require('chai');
const { MongoError } = require('../../src/error');
Expand Down Expand Up @@ -1743,4 +1743,20 @@ describe('Bulk', function () {
.then(updatedAdam => expect(updatedAdam).property('age').to.equal(39));
});
});

it(
'should return correct ids for documents with generated ids',
withClientV2(function (client, done) {
const bulk = client.db().collection('coll').initializeUnorderedBulkOp();
for (let i = 0; i < 2; i++) bulk.insert({ x: 1 });
bulk.execute((err, result) => {
expect(err).to.not.exist;
expect(result).property('insertedIds').to.exist;
expect(Object.keys(result.insertedIds)).to.have.length(2);
expect(result.insertedIds[0]).to.exist;
expect(result.insertedIds[1]).to.exist;
done();
});
})
);
});
18 changes: 18 additions & 0 deletions test/functional/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,23 @@ class APMEventCollector {
}
}

// simplified `withClient` helper that only uses callbacks
function withClientV2(callback) {
return function testFunction(done) {
const client = this.configuration.newClient({ monitorCommands: true });
client.connect(err => {
if (err) return done(err);
this.defer(() => client.close());

try {
callback.call(this, client, done);
} catch (err) {
done(err);
}
});
};
}

module.exports = {
assert,
delay,
Expand All @@ -285,6 +302,7 @@ module.exports = {
ignoreNsNotFound,
setupDatabase,
withClient,
withClientV2,
withMonitoredClient,
withCursor,
APMEventCollector
Expand Down

0 comments on commit 4de414a

Please sign in to comment.