Merge pull request #15415 from Automattic/vkarpov15/gh-15410

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

Author: vkarpov15

lib/model.js

@@ -3393,7 +3393,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 };
@@ -3430,6 +3430,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) {
@@ -3567,8 +3570,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 = 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 })
   );
@@ -3867,26 +3871,25 @@ Model.buildBulkWriteOperations = function buildBulkWriteOperations(documents, op
   }
 
   setDefaultOptions();
-  const discriminatorKey = this.schema.options.discriminatorKey;
 
-  const writeOperations = documents.reduce((accumulator, document, i) => {
+  const writeOperations = documents.map((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) {
+        const err = document.validateSync();
+        if (err != null) {
+          throw err;
+        }
       }
     }
 
     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();
@@ -3909,22 +3912,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;
+  }).filter(op => op !== null);
 
   return writeOperations;
 

test/model.test.js

@@ -6808,15 +6808,10 @@ describe('Model', function() {
         new User({ name: 'b' })
       ];
 
-      let err;
-      try {
-        User.buildBulkWriteOperations(users);
-      } catch (error) {
-        err = error;
-      }
-
-
-      assert.ok(err);
+      assert.throws(
+        () => User.buildBulkWriteOperations(users),
+        /name: Path `name` \(`a`\) is shorter than the minimum allowed length/
+      );
     });
 
     it('throws an error if documents is not an array', function() {
@@ -6828,9 +6823,7 @@ describe('Model', function() {
 
 
       assert.throws(
-        function() {
-          User.buildBulkWriteOperations(null);
-        },
+        () => User.buildBulkWriteOperations(null),
         /bulkSave expects an array of documents to be passed/
       );
     });
@@ -6841,14 +6834,11 @@ describe('Model', function() {
 
       const User = db.model('User', userSchema);
 
-
       assert.throws(
-        function() {
-          User.buildBulkWriteOperations([
-            new User({ name: 'Hafez' }),
-            { name: 'I am not a document' }
-          ]);
-        },
+        () => User.buildBulkWriteOperations([
+          new User({ name: 'Hafez' }),
+          { name: 'I am not a document' }
+        ]),
         /documents\.1 was not a mongoose document/
       );
     });
@@ -7062,6 +7052,62 @@ 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 });
+
+      requirementSchema.discriminators = {};
+      requirementSchema.discriminators['ComponentRequirement'] = componentRequirementSchema;
+      requirementSchema.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({