Merge pull request #14197 from Automattic/IslandRhythms/gh-14164

fix(model): deep clone bulkWrite() updateOne arguments to avoid mutating documents in update
Automattic · Dec 30, 2023 · d655898 · d655898
2 parents 913326d + 936d066
commit d655898
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 9 deletions.
diff --git a/lib/helpers/model/castBulkWrite.js b/lib/helpers/model/castBulkWrite.js
@@ -6,6 +6,7 @@ const applyTimestampsToChildren = require('../update/applyTimestampsToChildren')
 const applyTimestampsToUpdate = require('../update/applyTimestampsToUpdate');
 const cast = require('../../cast');
 const castUpdate = require('../query/castUpdate');
+const clone = require('../clone');
 const decorateUpdateWithVersionKey = require('../update/decorateUpdateWithVersionKey');
 const { inspect } = require('util');
 const setDefaultsOnInsert = require('../setDefaultsOnInsert');
@@ -64,30 +65,32 @@ module.exports = function castBulkWrite(originalModel, op, options) {
         const schema = model.schema;
         const strict = options.strict != null ? options.strict : model.schema.options.strict;
 
+        const update = clone(op['updateOne']['update']);
+
         _addDiscriminatorToObject(schema, op['updateOne']['filter']);
 
         if (model.schema.$timestamps != null && op['updateOne'].timestamps !== false) {
           const createdAt = model.schema.$timestamps.createdAt;
           const updatedAt = model.schema.$timestamps.updatedAt;
-          applyTimestampsToUpdate(now, createdAt, updatedAt, op['updateOne']['update'], {});
+          applyTimestampsToUpdate(now, createdAt, updatedAt, update, {});
         }
 
         if (op['updateOne'].timestamps !== false) {
-          applyTimestampsToChildren(now, op['updateOne']['update'], model.schema);
+          applyTimestampsToChildren(now, update, model.schema);
         }
 
         const shouldSetDefaultsOnInsert = op['updateOne'].setDefaultsOnInsert == null ?
           globalSetDefaultsOnInsert :
           op['updateOne'].setDefaultsOnInsert;
         if (shouldSetDefaultsOnInsert !== false) {
-          setDefaultsOnInsert(op['updateOne']['filter'], model.schema, op['updateOne']['update'], {
+          setDefaultsOnInsert(op['updateOne']['filter'], model.schema, update, {
             setDefaultsOnInsert: true,
             upsert: op['updateOne'].upsert
           });
         }
 
         decorateUpdateWithVersionKey(
-          op['updateOne']['update'],
+          update,
           op['updateOne'],
           model.schema.options.versionKey
         );
@@ -96,8 +99,7 @@ module.exports = function castBulkWrite(originalModel, op, options) {
           strict: strict,
           upsert: op['updateOne'].upsert
         });
-
-        op['updateOne']['update'] = castUpdate(model.schema, op['updateOne']['update'], {
+        op['updateOne']['update'] = castUpdate(model.schema, update, {
           strict: strict,
           upsert: op['updateOne'].upsert
         }, model, op['updateOne']['filter']);

diff --git a/lib/helpers/query/castUpdate.js b/lib/helpers/query/castUpdate.js
@@ -13,6 +13,7 @@ const moveImmutableProperties = require('../update/moveImmutableProperties');
 const schemaMixedSymbol = require('../../schema/symbols').schemaMixedSymbol;
 const setDottedPath = require('../path/setDottedPath');
 const utils = require('../../utils');
+const { internalToObjectOptions } = require('../../options');
 
 const mongodbUpdateOperators = new Set([
   '$currentDate',
@@ -99,7 +100,10 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
     const op = ops[i];
     val = ret[op];
     hasDollarKey = hasDollarKey || op.startsWith('$');
-
+    if (val != null && val.$__) {
+      val = val.toObject(internalToObjectOptions);
+      ret[op] = val;
+    }
     if (val &&
         typeof val === 'object' &&
         !Buffer.isBuffer(val) &&

diff --git a/test/model.test.js b/test/model.test.js
@@ -6392,6 +6392,47 @@ describe('Model', function() {
       });
       assert.deepEqual(timestampsOptions, [undefined, undefined]);
     });
+    it('should not modify the object in the $set clause and not error when dealing with or without timestamps (gh-14164)', async function() {
+      const timeSchema = new Schema({
+        name: String,
+        properties: { type: Schema.Types.Mixed, default: {} }
+      }, { timestamps: true });
+      const timelessSchema = new Schema({
+        name: String,
+        properties: { type: Schema.Types.Mixed, default: {} }
+      });
+
+      const Time = db.model('Time', timeSchema);
+      const Timeless = db.model('Timeless', timelessSchema);
+
+      const timeDoc = await Time.create({
+        name: 'Time Test'
+      });
+      timeDoc.properties.color = 'Red';
+      const beforeSet = {};
+      Object.assign(beforeSet, timeDoc.toObject());
+      await Time.bulkWrite([{
+        updateOne: {
+          filter: { _id: timeDoc._id },
+          update: { $set: timeDoc }
+        }
+      }]);
+      assert.deepStrictEqual(beforeSet, timeDoc.toObject());
+
+      const timelessDoc = await Timeless.create({
+        name: 'Timeless Test'
+      });
+      timelessDoc.properties.color = 'Red';
+      const timelessObj = {};
+      Object.assign(timelessObj, timelessDoc.toObject());
+      await Timeless.bulkWrite([{
+        updateOne: {
+          filter: { _id: timelessDoc._id },
+          update: { $set: timelessDoc }
+        }
+      }]);
+      assert.deepStrictEqual(timelessObj, timelessDoc.toObject());
+    });
   });
 
   describe('bulkSave() (gh-9673)', function() {

diff --git a/test/schema.select.test.js b/test/schema.select.test.js
@@ -15,12 +15,10 @@ describe('schema select option', function() {
 
   before(function() {
     db = start();
-    mongoose.set('debug', true);
   });
 
   after(async function() {
     await db.close();
-    mongoose.set('debug', false);
   });
 
   beforeEach(() => db.deleteModel(/.*/));