fix: only force server id generation if requested (#2653)

An inverted boolean expression accidentally always forced the
driver to force the server to generate document ids.

NODE-2923

Author: mbroadst

src/bulk/common.ts

@@ -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();
     }
 

test/functional/abstract_cursor.test.js

@@ -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(

test/functional/bulk.test.js

@@ -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');
@@ -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();
+      });
+    })
+  );
 });

test/functional/shared.js

@@ -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,
@@ -285,6 +302,7 @@ module.exports = {
   ignoreNsNotFound,
   setupDatabase,
   withClient,
+  withClientV2,
   withMonitoredClient,
   withCursor,
   APMEventCollector