Merge pull request #13774 from Automattic/vkarpov15/gh-13748

fix(document): make array getters avoid unintentionally modifying array, defer getters until index access instead
Automattic · Aug 28, 2023 · c1d5b76 · c1d5b76
2 parents fafa5d5 + 65245a4
commit c1d5b76
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 8 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -4091,6 +4091,14 @@ function applyGetters(self, json, options) {
       if (ii === last) {
         const val = self.$get(path);
         branch[part] = clone(val, options);
+        if (Array.isArray(branch[part]) && schema.paths[path].$embeddedSchemaType) {
+          for (let i = 0; i < branch[part].length; ++i) {
+            branch[part][i] = schema.paths[path].$embeddedSchemaType.applyGetters(
+              branch[part][i],
+              self
+            );
+          }
+        }
       } else if (v == null) {
         if (part in cur) {
           branch[part] = v;

diff --git a/lib/schema/array.js b/lib/schema/array.js
@@ -289,13 +289,6 @@ SchemaArray.prototype.applyGetters = function(value, scope) {
   }
 
   const ret = SchemaType.prototype.applyGetters.call(this, value, scope);
-  if (Array.isArray(ret)) {
-    const rawValue = utils.isMongooseArray(ret) ? ret.__array : ret;
-    const len = rawValue.length;
-    for (let i = 0; i < len; ++i) {
-      rawValue[i] = this.caster.applyGetters(rawValue[i], scope);
-    }
-  }
   return ret;
 };
 

diff --git a/lib/types/array/index.js b/lib/types/array/index.js
@@ -90,6 +90,9 @@ function MongooseArray(values, path, doc, schematype) {
       if (mongooseArrayMethods.hasOwnProperty(prop)) {
         return mongooseArrayMethods[prop];
       }
+      if (typeof prop === 'string' && numberRE.test(prop) && schematype?.$embeddedSchemaType != null) {
+        return schematype.$embeddedSchemaType.applyGetters(__array[prop], doc);
+      }
 
       return __array[prop];
     },

diff --git a/test/document.test.js b/test/document.test.js
@@ -5564,6 +5564,7 @@ describe('document', function() {
       const Test = db.model('Test', testSchema);
 
       const doc = new Test({ arr: [new mongoose.Types.ObjectId()] });
+      assert.equal(called, 0);
       assert.deepEqual(doc.toObject({ getters: true }).arr, [42]);
       assert.equal(called, 1);
     });
@@ -5582,7 +5583,6 @@ describe('document', function() {
 
       const Test = db.model('Test', testSchema);
 
-
       let doc = await Test.create({ arr: [new mongoose.Types.ObjectId()] });
       assert.equal(called, 1);
 
@@ -12343,6 +12343,51 @@ describe('document', function() {
     assert.strictEqual(Object.polluted, undefined);
   });
 
+  it('does not modify array when calling getters (gh-13748)', async function() {
+    // create simple setter to add a sufix
+    const addSufix = (name) => {
+      return name + '-sufix';
+    };
+
+    // create simple gettrer to remove last 6 letters (should be "-sufix")
+    const removeSufix = (name) => {
+      return ('' + name).slice(0, -6);
+    };
+
+    const userSchema = new mongoose.Schema(
+      {
+        name: String,
+        age: Number,
+        profession: {
+          type: String,
+          get: removeSufix,
+          set: addSufix
+        },
+        hobbies: [{ type: String, get: removeSufix, set: addSufix }]
+      },
+      {
+        toObject: { getters: true },
+        toJSON: { getters: true }
+      }
+    );
+    const User = db.model('User', userSchema);
+
+    const usr = await User.create({
+      name: 'John',
+      age: 18,
+      profession: 'teacher',
+      hobbies: ['swimming', 'football']
+    });
+
+    const oneUser = await User.findById(usr._id).orFail();
+    assert.equal(oneUser.profession, 'teacher');
+    assert.equal(oneUser.profession, 'teacher');
+    assert.equal(oneUser.profession, 'teacher');
+    assert.equal(oneUser.hobbies[0], 'swimming');
+    assert.equal(oneUser.hobbies[0], 'swimming');
+    assert.equal(oneUser.hobbies[0], 'swimming');
+  });
+
   it('sets defaults on subdocs with subdoc projection (gh-13720)', async function() {
     const subSchema = new mongoose.Schema({
       propertyA: { type: String, default: 'A' },