Merge pull request #15266 from Automattic/vkarpov15/gh-15265

fix(model+connection): return MongoDB BulkWriteResult instance even if no valid ops

Author: vkarpov15

lib/connection.js

@@ -23,6 +23,7 @@ const CreateCollectionsError = require('./error/createCollectionsError');
 const castBulkWrite = require('./helpers/model/castBulkWrite');
 const { modelSymbol } = require('./helpers/symbols');
 const isPromise = require('./helpers/isPromise');
+const decorateBulkWriteResult = require('./helpers/model/decorateBulkWriteResult');
 
 const arrayAtomicsSymbol = require('./helpers/symbols').arrayAtomicsSymbol;
 const sessionNewDocuments = require('./helpers/symbols').sessionNewDocuments;
@@ -559,24 +560,27 @@ Connection.prototype.bulkWrite = async function bulkWrite(ops, options) {
           'bulkWrite'
         );
       }
-      return getDefaultBulkwriteResult();
+      const BulkWriteResult = this.base.driver.get().BulkWriteResult;
+      const res = new BulkWriteResult(getDefaultBulkwriteResult(), false);
+      return decorateBulkWriteResult(res, validationErrors, results);
     }
 
     let error;
     [res, error] = await this.client.bulkWrite(validOps, options).
       then(res => ([res, null])).
       catch(err => ([null, err]));
 
+    for (let i = 0; i < validOpIndexes.length; ++i) {
+      results[validOpIndexes[i]] = null;
+    }
     if (error) {
       if (validationErrors.length > 0) {
+        decorateBulkWriteResult(error, validationErrors, results);
         error.mongoose = error.mongoose || {};
         error.mongoose.validationErrors = validationErrors;
       }
     }
 
-    for (let i = 0; i < validOpIndexes.length; ++i) {
-      results[validOpIndexes[i]] = null;
-    }
     if (validationErrors.length > 0) {
       if (options.throwOnValidationError) {
         throw new MongooseBulkWriteError(
@@ -586,9 +590,7 @@ Connection.prototype.bulkWrite = async function bulkWrite(ops, options) {
           'bulkWrite'
         );
       } else {
-        res.mongoose = res.mongoose || {};
-        res.mongoose.validationErrors = validationErrors;
-        res.mongoose.results = results;
+        decorateBulkWriteResult(res, validationErrors, results);
       }
     }
   }

lib/drivers/browser/index.js

@@ -10,3 +10,4 @@ exports.Collection = function() {
 exports.Connection = function() {
   throw new Error('Cannot create a connection from browser library');
 };
+exports.BulkWriteResult = function() {};

lib/drivers/node-mongodb-native/bulkWriteResult.js

@@ -0,0 +1,5 @@
+'use strict';
+
+const BulkWriteResult = require('mongodb/lib/bulk/common').BulkWriteResult;
+
+module.exports = BulkWriteResult;

lib/drivers/node-mongodb-native/index.js

@@ -4,5 +4,6 @@
 
 'use strict';
 
+exports.BulkWriteResult = require('./bulkWriteResult');
 exports.Collection = require('./collection');
 exports.Connection = require('./connection');

lib/helpers/getDefaultBulkwriteResult.js

@@ -1,26 +1,17 @@
 'use strict';
+
 function getDefaultBulkwriteResult() {
   return {
-    result: {
-      ok: 1,
-      writeErrors: [],
-      writeConcernErrors: [],
-      insertedIds: [],
-      nInserted: 0,
-      nUpserted: 0,
-      nMatched: 0,
-      nModified: 0,
-      nRemoved: 0,
-      upserted: []
-    },
-    insertedCount: 0,
-    matchedCount: 0,
-    modifiedCount: 0,
-    deletedCount: 0,
-    upsertedCount: 0,
-    upsertedIds: {},
-    insertedIds: {},
-    n: 0
+    ok: 1,
+    nInserted: 0,
+    nUpserted: 0,
+    nMatched: 0,
+    nModified: 0,
+    nRemoved: 0,
+    upserted: [],
+    writeErrors: [],
+    insertedIds: [],
+    writeConcernErrors: []
   };
 }
 

lib/helpers/model/decorateBulkWriteResult.js

@@ -0,0 +1,8 @@
+'use strict';
+
+module.exports = function decorateBulkWriteResult(resultOrError, validationErrors, results) {
+  resultOrError.mongoose = resultOrError.mongoose || {};
+  resultOrError.mongoose.validationErrors = validationErrors;
+  resultOrError.mongoose.results = results;
+  return resultOrError;
+};

lib/model.js

@@ -10,7 +10,6 @@ const Document = require('./document');
 const DocumentNotFoundError = require('./error/notFound');
 const EventEmitter = require('events').EventEmitter;
 const Kareem = require('kareem');
-const { MongoBulkWriteError } = require('mongodb');
 const MongooseBulkWriteError = require('./error/bulkWriteError');
 const MongooseError = require('./error/index');
 const ObjectParameterError = require('./error/objectParameter');
@@ -69,6 +68,7 @@ const utils = require('./utils');
 const minimize = require('./helpers/minimize');
 const MongooseBulkSaveIncompleteError = require('./error/bulkSaveIncompleteError');
 const ObjectExpectedError = require('./error/objectExpected');
+const decorateBulkWriteResult = require('./helpers/model/decorateBulkWriteResult');
 
 const modelCollectionSymbol = Symbol('mongoose#Model#collection');
 const modelDbSymbol = Symbol('mongoose#Model#db');
@@ -3399,7 +3399,11 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
   const ordered = options.ordered == null ? true : options.ordered;
 
   if (ops.length === 0) {
-    return getDefaultBulkwriteResult();
+    const BulkWriteResult = this.base.driver.get().BulkWriteResult;
+    const bulkWriteResult = new BulkWriteResult(getDefaultBulkwriteResult(), false);
+    bulkWriteResult.n = 0;
+    decorateBulkWriteResult(bulkWriteResult, [], []);
+    return bulkWriteResult;
   }
 
   const validations = ops.map(op => castBulkWrite(this, op, options));
@@ -3470,18 +3474,24 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
           'bulkWrite'
         );
       }
-      return getDefaultBulkwriteResult();
+      const BulkWriteResult = this.base.driver.get().BulkWriteResult;
+      const bulkWriteResult = new BulkWriteResult(getDefaultBulkwriteResult(), false);
+      bulkWriteResult.result = getDefaultBulkwriteResult();
+      decorateBulkWriteResult(bulkWriteResult, validationErrors, results);
+      return bulkWriteResult;
     }
 
     let error;
     [res, error] = await this.$__collection.bulkWrite(validOps, options).
       then(res => ([res, null])).
       catch(error => ([null, error]));
 
+    for (let i = 0; i < validOpIndexes.length; ++i) {
+      results[validOpIndexes[i]] = null;
+    }
     if (error) {
       if (validationErrors.length > 0) {
-        error.mongoose = error.mongoose || {};
-        error.mongoose.validationErrors = validationErrors;
+        decorateBulkWriteResult(error, validationErrors, results);
       }
 
       await new Promise((resolve, reject) => {
@@ -3495,9 +3505,6 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
       });
     }
 
-    for (let i = 0; i < validOpIndexes.length; ++i) {
-      results[validOpIndexes[i]] = null;
-    }
     if (validationErrors.length > 0) {
       if (options.throwOnValidationError) {
         throw new MongooseBulkWriteError(
@@ -3507,9 +3514,7 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
           'bulkWrite'
         );
       } else {
-        res.mongoose = res.mongoose || {};
-        res.mongoose.validationErrors = validationErrors;
-        res.mongoose.results = results;
+        decorateBulkWriteResult(res, validationErrors, results);
       }
     }
   }
@@ -3575,7 +3580,7 @@ Model.bulkSave = async function bulkSave(documents, options) {
     (err) => ({ bulkWriteResult: null, bulkWriteError: err })
   );
   // If not a MongoBulkWriteError, treat this as all documents failed to save.
-  if (bulkWriteError != null && !(bulkWriteError instanceof MongoBulkWriteError)) {
+  if (bulkWriteError != null && bulkWriteError.name !== 'MongoBulkWriteError') {
     throw bulkWriteError;
   }
 

test/model.test.js

@@ -4121,7 +4121,7 @@ describe('Model', function() {
           { ordered: false, throwOnValidationError: true }
         ).then(() => null, err => err);
         assert.ok(err);
-        assert.equal(err.name, 'MongooseBulkWriteError');
+        assert.equal(err.name, 'MongooseBulkWriteError', err.stack);
         assert.equal(err.validationErrors[0].errors['num'].name, 'CastError');
       });
 
@@ -6492,17 +6492,9 @@ describe('Model', function() {
     assert.deepEqual(
       res,
       {
-        result: {
-          ok: 1,
-          writeErrors: [],
-          writeConcernErrors: [],
-          insertedIds: [],
-          nInserted: 0,
-          nUpserted: 0,
-          nMatched: 0,
-          nModified: 0,
-          nRemoved: 0,
-          upserted: []
+        mongoose: {
+          results: [],
+          validationErrors: []
         },
         insertedCount: 0,
         matchedCount: 0,
@@ -6514,7 +6506,20 @@ describe('Model', function() {
         n: 0
       }
     );
-
+    assert.deepEqual(res.result, {
+      ok: 1,
+      writeErrors: [],
+      writeConcernErrors: [],
+      insertedIds: [],
+      nInserted: 0,
+      nUpserted: 0,
+      nMatched: 0,
+      nModified: 0,
+      nRemoved: 0,
+      upserted: []
+    });
+
+    assert.equal(typeof res.getWriteErrorAt, 'function');
   });
 
   it('Model.bulkWrite(...) does not throw an error with upsert:true, setDefaultsOnInsert: true (gh-9157)', async function() {
@@ -6554,28 +6559,37 @@ describe('Model', function() {
     assert.deepEqual(
       res,
       {
-        result: {
-          ok: 1,
-          writeErrors: [],
-          writeConcernErrors: [],
-          insertedIds: [],
-          nInserted: 0,
-          nUpserted: 0,
-          nMatched: 0,
-          nModified: 0,
-          nRemoved: 0,
-          upserted: []
-        },
         insertedCount: 0,
         matchedCount: 0,
         modifiedCount: 0,
         deletedCount: 0,
         upsertedCount: 0,
         upsertedIds: {},
         insertedIds: {},
-        n: 0
+        n: 0,
+        mongoose: {
+          results: [],
+          validationErrors: []
+        }
       }
     );
+    assert.deepEqual(
+      res.result,
+      {
+        ok: 1,
+        writeErrors: [],
+        writeConcernErrors: [],
+        insertedIds: [],
+        nInserted: 0,
+        nUpserted: 0,
+        nMatched: 0,
+        nModified: 0,
+        nRemoved: 0,
+        upserted: []
+      }
+    );
+
+    assert.equal(typeof res.getWriteErrorAt, 'function');
   });
 
   it('allows calling `create()` after `bulkWrite()` (gh-9350)', async function() {

types/models.d.ts

@@ -308,7 +308,7 @@ declare module 'mongoose' {
     bulkWrite<DocContents = TRawDocType>(
       writes: Array<AnyBulkWriteOperation<DocContents extends Document ? any : (DocContents extends {} ? DocContents : any)>>,
       options: MongooseBulkWriteOptions & { ordered: false }
-    ): Promise<mongodb.BulkWriteResult & { mongoose?: { validationErrors: Error[] } }>;
+    ): Promise<mongodb.BulkWriteResult & { mongoose?: { validationErrors: Error[], results: Array<Error | null> } }>;
     bulkWrite<DocContents = TRawDocType>(
       writes: Array<AnyBulkWriteOperation<DocContents extends Document ? any : (DocContents extends {} ? DocContents : any)>>,
       options?: MongooseBulkWriteOptions