Merge pull request #14213 from Automattic/vkarpov15/gh-14024

fix: add ignoreAtomics option to isModified() for better backwards compatibility with Mongoose 5
Automattic · Jan 3, 2024 · b4e3b2f · b4e3b2f
2 parents e3c12cf + 0960fae
commit b4e3b2f
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 7 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -2221,12 +2221,15 @@ Document.prototype[documentModifiedPaths] = Document.prototype.modifiedPaths;
  *     doc.isDirectModified('documents')     // false
  *
  * @param {String} [path] optional
+ * @param {Object} [options]
+ * @param {Boolean} [options.ignoreAtomics=false] If true, doesn't return true if path is underneath an array that was modified with atomic operations like `push()`
  * @return {Boolean}
  * @api public
  */
 
-Document.prototype.isModified = function(paths, modifiedPaths) {
+Document.prototype.isModified = function(paths, options, modifiedPaths) {
   if (paths) {
+    const ignoreAtomics = options && options.ignoreAtomics;
     const directModifiedPathsObj = this.$__.activePaths.states.modify;
     if (directModifiedPathsObj == null) {
       return false;
@@ -2247,7 +2250,16 @@ Document.prototype.isModified = function(paths, modifiedPaths) {
       return !!~modified.indexOf(path);
     });
 
-    const directModifiedPaths = Object.keys(directModifiedPathsObj);
+    let directModifiedPaths = Object.keys(directModifiedPathsObj);
+    if (ignoreAtomics) {
+      directModifiedPaths = directModifiedPaths.filter(path => {
+        const value = this.$__getValue(path);
+        if (value != null && value[arrayAtomicsSymbol] != null && value[arrayAtomicsSymbol].$set === undefined) {
+          return false;
+        }
+        return true;
+      });
+    }
     return isModifiedChild || paths.some(function(path) {
       return directModifiedPaths.some(function(mod) {
         return mod === path || path.startsWith(mod + '.');
@@ -2681,7 +2693,7 @@ function _getPathsToValidate(doc) {
         paths.delete(fullPathToSubdoc + '.' + modifiedPath);
       }
 
-      if (doc.$isModified(fullPathToSubdoc, modifiedPaths) &&
+      if (doc.$isModified(fullPathToSubdoc, null, modifiedPaths) &&
             !doc.isDirectModified(fullPathToSubdoc) &&
             !doc.$isDefault(fullPathToSubdoc)) {
         paths.add(fullPathToSubdoc);

diff --git a/lib/types/subdocument.js b/lib/types/subdocument.js
@@ -178,7 +178,7 @@ Subdocument.prototype.markModified = function(path) {
  * ignore
  */
 
-Subdocument.prototype.isModified = function(paths, modifiedPaths) {
+Subdocument.prototype.isModified = function(paths, options, modifiedPaths) {
   const parent = this.$parent();
   if (parent != null) {
     if (Array.isArray(paths) || typeof paths === 'string') {
@@ -188,10 +188,10 @@ Subdocument.prototype.isModified = function(paths, modifiedPaths) {
       paths = this.$__pathRelativeToParent();
     }
 
-    return parent.$isModified(paths, modifiedPaths);
+    return parent.$isModified(paths, options, modifiedPaths);
   }
 
-  return Document.prototype.isModified.call(this, paths, modifiedPaths);
+  return Document.prototype.isModified.call(this, paths, options, modifiedPaths);
 };
 
 /**

diff --git a/test/document.modified.test.js b/test/document.modified.test.js
@@ -208,6 +208,43 @@ describe('document modified', function() {
         assert.equal(post.isModified('comments.0.title'), true);
         assert.equal(post.isDirectModified('comments.0.title'), true);
       });
+      it('with push (gh-14024)', async function() {
+        const post = new BlogPost();
+        post.init({
+          title: 'Test',
+          slug: 'test',
+          comments: [{ title: 'Test', date: new Date(), body: 'Test' }]
+        });
+
+        post.comments.push({ title: 'new comment', body: 'test' });
+
+        assert.equal(post.isModified('comments.0.title', { ignoreAtomics: true }), false);
+        assert.equal(post.isModified('comments.0.body', { ignoreAtomics: true }), false);
+        assert.equal(post.get('comments')[0].isModified('body', { ignoreAtomics: true }), false);
+      });
+      it('with push and set (gh-14024)', async function() {
+        const post = new BlogPost();
+        post.init({
+          title: 'Test',
+          slug: 'test',
+          comments: [{ title: 'Test', date: new Date(), body: 'Test' }]
+        });
+
+        post.comments.push({ title: 'new comment', body: 'test' });
+        post.get('comments')[0].set('title', 'Woot');
+
+        assert.equal(post.isModified('comments', { ignoreAtomics: true }), true);
+        assert.equal(post.isModified('comments.0.title', { ignoreAtomics: true }), true);
+        assert.equal(post.isDirectModified('comments.0.title'), true);
+        assert.equal(post.isDirectModified('comments.0.body'), false);
+        assert.equal(post.isModified('comments.0.body', { ignoreAtomics: true }), false);
+
+        assert.equal(post.isModified('comments', { ignoreAtomics: true }), true);
+        assert.equal(post.isModified('comments.0.title', { ignoreAtomics: true }), true);
+        assert.equal(post.isDirectModified('comments.0.title'), true);
+        assert.equal(post.isDirectModified('comments.0.body'), false);
+        assert.equal(post.isModified('comments.0.body', { ignoreAtomics: true }), false);
+      });
       it('with accessors', function() {
         const post = new BlogPost();
         post.init({

diff --git a/types/document.d.ts b/types/document.d.ts
@@ -179,7 +179,7 @@ declare module 'mongoose' {
      * Returns true if any of the given paths are modified, else false. If no arguments, returns `true` if any path
      * in this document is modified.
      */
-    isModified(path?: string | Array<string>): boolean;
+    isModified(path?: string | Array<string>, options?: { ignoreAtomics?: boolean } | null): boolean;
 
     /** Boolean flag specifying if the document is new. */
     isNew: boolean;