fix(model): make bulkSave() rely on document.validate() to validate docs and skip bulkWrite casting

Fix #15410

Author: vkarpov15

lib/model.js

@@ -3394,7 +3394,7 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
     return bulkWriteResult;
   }
 
-  const validations = ops.map(op => castBulkWrite(this, op, options));
+  const validations = options?._skipCastBulkWrite ? [] : ops.map(op => castBulkWrite(this, op, options));
   const asyncLocalStorage = this.db.base.transactionAsyncLocalStorage?.getStore();
   if ((!options || !options.hasOwnProperty('session')) && asyncLocalStorage?.session != null) {
     options = { ...options, session: asyncLocalStorage.session };
@@ -3431,6 +3431,9 @@ Model.bulkWrite = async function bulkWrite(ops, options) {
     let validationErrors = [];
     const results = [];
     await new Promise((resolve) => {
+      if (validations.length === 0) {
+        return resolve();
+      }
       for (let i = 0; i < validations.length; ++i) {
         validations[i]((err) => {
           if (err == null) {
@@ -3568,8 +3571,9 @@ Model.bulkSave = async function bulkSave(documents, options) {
 
   await Promise.all(documents.map(doc => buildPreSavePromise(doc, options)));
 
-  const writeOperations = this.buildBulkWriteOperations(documents, { skipValidation: true, timestamps: options.timestamps });
-  const { bulkWriteResult, bulkWriteError } = await this.bulkWrite(writeOperations, { skipValidation: true, ...options }).then(
+  const writeOperations = await this.buildBulkWriteOperations(documents, options);
+  const opts = { skipValidation: true, _skipCastBulkWrite: true, ...options };
+  const { bulkWriteResult, bulkWriteError } = await this.bulkWrite(writeOperations, opts).then(
     (res) => ({ bulkWriteResult: res, bulkWriteError: null }),
     (err) => ({ bulkWriteResult: null, bulkWriteError: err })
   );
@@ -3862,32 +3866,28 @@ Model.castObject = function castObject(obj, options) {
  * @api private
  */
 
-Model.buildBulkWriteOperations = function buildBulkWriteOperations(documents, options) {
+Model.buildBulkWriteOperations = async function buildBulkWriteOperations(documents, options) {
   if (!Array.isArray(documents)) {
     throw new Error(`bulkSave expects an array of documents to be passed, received \`${documents}\` instead`);
   }
 
   setDefaultOptions();
-  const discriminatorKey = this.schema.options.discriminatorKey;
 
-  const writeOperations = documents.reduce((accumulator, document, i) => {
+  const writeOperations = await Promise.all(documents.map(async(document, i) => {
     if (!options.skipValidation) {
       if (!(document instanceof Document)) {
         throw new Error(`documents.${i} was not a mongoose document, documents must be an array of mongoose documents (instanceof mongoose.Document).`);
       }
-      const validationError = document.validateSync();
-      if (validationError) {
-        throw validationError;
+      if (options.validateBeforeSave == null || options.validateBeforeSave) {
+        await document.validate();
       }
     }
 
     const isANewDocument = document.isNew;
     if (isANewDocument) {
       const writeOperation = { insertOne: { document } };
       utils.injectTimestampsOption(writeOperation.insertOne, options.timestamps);
-      accumulator.push(writeOperation);
-
-      return accumulator;
+      return writeOperation;
     }
 
     const delta = document.$__delta();
@@ -3910,22 +3910,14 @@ Model.buildBulkWriteOperations = function buildBulkWriteOperations(documents, op
         }
       }
 
-      // Set the discriminator key, so bulk write casting knows which
-      // schema to use re: gh-13907
-      if (document[discriminatorKey] != null && !(discriminatorKey in where)) {
-        where[discriminatorKey] = document[discriminatorKey];
-      }
-
       document.$__version(where, delta);
       const writeOperation = { updateOne: { filter: where, update: changes } };
       utils.injectTimestampsOption(writeOperation.updateOne, options.timestamps);
-      accumulator.push(writeOperation);
-
-      return accumulator;
+      return writeOperation;
     }
 
-    return accumulator;
-  }, []);
+    return null;
+  })).then(results => results.filter(op => op !== null));
 
   return writeOperations;
 

test/model.test.js

@@ -6780,7 +6780,7 @@ describe('Model', function() {
       await users[2].save();
       users[2].name = 'I am the updated third name';
 
-      const writeOperations = User.buildBulkWriteOperations(users);
+      const writeOperations = await User.buildBulkWriteOperations(users);
 
       const desiredWriteOperations = [
         { insertOne: { document: users[0] } },
@@ -6795,7 +6795,7 @@ describe('Model', function() {
 
     });
 
-    it('throws an error when one document is invalid', () => {
+    it('throws an error when one document is invalid', async() => {
       const userSchema = new Schema({
         name: { type: String, minLength: 5 }
       });
@@ -6810,12 +6810,11 @@ describe('Model', function() {
 
       let err;
       try {
-        User.buildBulkWriteOperations(users);
+        await User.buildBulkWriteOperations(users);
       } catch (error) {
         err = error;
       }
 
-
       assert.ok(err);
     });
 
@@ -6827,10 +6826,8 @@ describe('Model', function() {
       const User = db.model('User', userSchema);
 
 
-      assert.throws(
-        function() {
-          User.buildBulkWriteOperations(null);
-        },
+      assert.rejects(
+        User.buildBulkWriteOperations(null),
         /bulkSave expects an array of documents to be passed/
       );
     });
@@ -6842,17 +6839,15 @@ describe('Model', function() {
       const User = db.model('User', userSchema);
 
 
-      assert.throws(
-        function() {
-          User.buildBulkWriteOperations([
-            new User({ name: 'Hafez' }),
-            { name: 'I am not a document' }
-          ]);
-        },
+      assert.rejects(
+        User.buildBulkWriteOperations([
+          new User({ name: 'Hafez' }),
+          { name: 'I am not a document' }
+        ]),
         /documents\.1 was not a mongoose document/
       );
     });
-    it('skips validation when given `skipValidation` true', () => {
+    it('skips validation when given `skipValidation` true', async() => {
       const userSchema = new Schema({
         name: { type: String, minLength: 5 }
       });
@@ -6865,7 +6860,7 @@ describe('Model', function() {
         new User({ name: 'b' })
       ];
 
-      const writeOperations = User.buildBulkWriteOperations(users, { skipValidation: true });
+      const writeOperations = await User.buildBulkWriteOperations(users, { skipValidation: true });
 
       assert.equal(writeOperations.length, 3);
     });
@@ -6904,7 +6899,7 @@ describe('Model', function() {
       userToUpdate.name = 'John Doe';
 
       // Act
-      const writeOperations = User.buildBulkWriteOperations([newUser, userToUpdate], { timestamps: false, skipValidation: true });
+      const writeOperations = await User.buildBulkWriteOperations([newUser, userToUpdate], { timestamps: false, skipValidation: true });
 
       // Assert
       const timestampsOptions = writeOperations.map(writeOperationContainer => {
@@ -6926,7 +6921,7 @@ describe('Model', function() {
       userToUpdate.name = 'John Doe';
 
       // Act
-      const writeOperations = User.buildBulkWriteOperations([newUser, userToUpdate], { timestamps: true, skipValidation: true });
+      const writeOperations = await User.buildBulkWriteOperations([newUser, userToUpdate], { timestamps: true, skipValidation: true });
 
       // Assert
       const timestampsOptions = writeOperations.map(writeOperationContainer => {
@@ -6948,7 +6943,7 @@ describe('Model', function() {
       userToUpdate.name = 'John Doe';
 
       // Act
-      const writeOperations = User.buildBulkWriteOperations([newUser, userToUpdate], { skipValidation: true });
+      const writeOperations = await User.buildBulkWriteOperations([newUser, userToUpdate], { skipValidation: true });
 
       // Assert
       const timestampsOptions = writeOperations.map(writeOperationContainer => {
@@ -7062,6 +7057,63 @@ describe('Model', function() {
 
     });
 
+    it('saves documents with embedded discriminators (gh-15410)', async function() {
+      const requirementSchema = new Schema({
+        kind: { type: String, required: true },
+        quantity: Number,
+        notes: String
+      }, { _id: false, discriminatorKey: 'kind' });
+
+      const componentRequirementSchema = new Schema({
+        componentTest: 'ObjectId'
+      }, { _id: false });
+
+      const toolRequirementSchema = new Schema({
+        toolTest: 'ObjectId'
+      }, { _id: false });
+
+      const subRowSchema = new Schema({
+        requirements: [requirementSchema]
+      }, { _id: false });
+
+      const rowSchema = new Schema({
+        rows: [subRowSchema]
+      }, { _id: false });
+
+      const orderSchema = new Schema({
+        code: String,
+        rows: [rowSchema]
+      }, { timestamps: true });
+
+      const Requirement = requirementSchema;
+      Requirement.discriminators = {};
+      Requirement.discriminators['ComponentRequirement'] = componentRequirementSchema;
+      Requirement.discriminators['ToolRequirement'] = toolRequirementSchema;
+
+      subRowSchema.path('requirements').discriminator('ComponentRequirement', componentRequirementSchema);
+      subRowSchema.path('requirements').discriminator('ToolRequirement', toolRequirementSchema);
+
+      const Order = db.model('Order', orderSchema);
+
+      const order = await Order.create({
+        code: 'test-2',
+        rows: [{
+          rows: [{
+            requirements: [
+              { kind: 'ComponentRequirement', quantity: 1 },
+              { kind: 'ToolRequirement', quantity: 1 }
+            ]
+          }]
+        }]
+      });
+
+      const newObjectId = new mongoose.Types.ObjectId();
+      order.rows[0].rows[0].requirements[1].set({ toolTest: newObjectId.toString() });
+      await Order.bulkSave([order]);
+      const reread = await Order.findById(order._id).lean();
+      assert.strictEqual(reread.rows[0].rows[0].requirements[1].toolTest?.toHexString(), newObjectId.toHexString());
+    });
+
     it('insertMany should throw an error if there were operations that failed validation, ' +
         'but all operations that passed validation succeeded (gh-14572) (gh-13256)', async function() {
       const userSchema = new Schema({