Merge branch '7.x'

Automattic · Jan 9, 2024 · 3bc4482 · 3bc4482
2 parents 2c2377d + ac9af5b
commit 3bc4482
Show file tree

Hide file tree

Showing 14 changed files with 251 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,22 @@
+7.6.8 / 2024-01-08
+==================
+ * perf(schema): remove unnecessary lookahead in numeric subpath check
+ * fix(discriminator): handle reusing schema with embedded discriminators defined using Schema.prototype.discriminator #14202 #14162
+ * fix(ChangeStream): avoid suppressing errors in closed change stream #14206 #14177
+
+6.12.5 / 2024-01-03
+===================
+ * perf(schema): remove unnecessary lookahead in numeric subpath check
+ * fix(document): allow setting nested path to null #14226
+ * fix(document): avoid flattening dotted paths in mixed path underneath nested path #14198 #14178
+ * fix: add ignoreAtomics option to isModified() for better backwards compatibility with Mongoose 5 #14213
+
+6.12.4 / 2023-12-27
+===================
+ * fix: upgrade mongodb driver -> 4.17.2
+ * fix(document): avoid treating nested projection as inclusive when applying defaults #14173 #14115
+ * fix: account for null values when assigning isNew property #14172 #13883
+
 8.0.3 / 2023-12-07
 ==================
  * fix(schema): avoid creating unnecessary clone of schematype in nested array so nested document arrays use correct constructor #14128 #14101

diff --git a/lib/cursor/changeStream.js b/lib/cursor/changeStream.js
@@ -60,12 +60,6 @@ class ChangeStream extends EventEmitter {
 
         driverChangeStreamEvents.forEach(ev => {
           this.driverChangeStream.on(ev, data => {
-            // Sometimes Node driver still polls after close, so
-            // avoid any uncaught exceptions due to closed change streams
-            // See tests for gh-7022
-            if (ev === 'error' && this.closed) {
-              return;
-            }
             if (data != null && data.fullDocument != null && this.options && this.options.hydrate) {
               data.fullDocument = this.options.model.hydrate(data.fullDocument);
             }
@@ -83,12 +77,6 @@ class ChangeStream extends EventEmitter {
 
     driverChangeStreamEvents.forEach(ev => {
       this.driverChangeStream.on(ev, data => {
-        // Sometimes Node driver still polls after close, so
-        // avoid any uncaught exceptions due to closed change streams
-        // See tests for gh-7022
-        if (ev === 'error' && this.closed) {
-          return;
-        }
         if (data != null && data.fullDocument != null && this.options && this.options.hydrate) {
           data.fullDocument = this.options.model.hydrate(data.fullDocument);
         }

diff --git a/lib/document.js b/lib/document.js
@@ -1122,6 +1122,8 @@ Document.prototype.$set = function $set(path, val, type, options) {
           } else {
             throw new StrictModeError(key);
           }
+        } else if (pathtype === 'nested' && valForKey == null) {
+          this.$set(pathName, valForKey, constructing, options);
         }
       } else if (valForKey !== void 0) {
         this.$set(pathName, valForKey, constructing, options);
@@ -2229,12 +2231,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;
@@ -2255,7 +2260,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 + '.');
@@ -2677,7 +2691,7 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {
         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/helpers/discriminator/applyEmbeddedDiscriminators.js b/lib/helpers/discriminator/applyEmbeddedDiscriminators.js
@@ -16,8 +16,15 @@ function applyEmbeddedDiscriminators(schema, seen = new WeakSet()) {
     if (!schemaType.schema._applyDiscriminators) {
       continue;
     }
+    if (schemaType._appliedDiscriminators) {
+      continue;
+    }
     for (const disc of schemaType.schema._applyDiscriminators.keys()) {
-      schemaType.discriminator(disc, schemaType.schema._applyDiscriminators.get(disc));
+      schemaType.discriminator(
+        disc,
+        schemaType.schema._applyDiscriminators.get(disc)
+      );
     }
+    schemaType._appliedDiscriminators = true;
   }
 }
diff --git a/lib/helpers/document/applyDefaults.js b/lib/helpers/document/applyDefaults.js
@@ -1,5 +1,7 @@
 'use strict';
 
+const isNestedProjection = require('../projection/isNestedProjection');
+
 module.exports = function applyDefaults(doc, fields, exclude, hasIncludedChildren, isBeforeSetters, pathsToSkip) {
   const paths = Object.keys(doc.$__schema.paths);
   const plen = paths.length;
@@ -32,7 +34,7 @@ module.exports = function applyDefaults(doc, fields, exclude, hasIncludedChildre
         }
       } else if (exclude === false && fields && !included) {
         const hasSubpaths = type.$isSingleNested || type.$isMongooseDocumentArray;
-        if (curPath in fields || (j === len - 1 && hasSubpaths && hasIncludedChildren != null && hasIncludedChildren[curPath])) {
+        if ((curPath in fields && !isNestedProjection(fields[curPath])) || (j === len - 1 && hasSubpaths && hasIncludedChildren != null && hasIncludedChildren[curPath])) {
           included = true;
         } else if (hasIncludedChildren != null && !hasIncludedChildren[curPath]) {
           break;

diff --git a/lib/helpers/projection/hasIncludedChildren.js b/lib/helpers/projection/hasIncludedChildren.js
@@ -21,6 +21,7 @@ module.exports = function hasIncludedChildren(fields) {
   const keys = Object.keys(fields);
 
   for (const key of keys) {
+
     if (key.indexOf('.') === -1) {
       hasIncludedChildren[key] = 1;
       continue;

diff --git a/lib/helpers/projection/isNestedProjection.js b/lib/helpers/projection/isNestedProjection.js
@@ -0,0 +1,8 @@
+'use strict';
+
+module.exports = function isNestedProjection(val) {
+  if (val == null || typeof val !== 'object') {
+    return false;
+  }
+  return val.$slice == null && val.$elemMatch == null && val.$meta == null && val.$ == null;
+};
diff --git a/lib/schema.js b/lib/schema.js
@@ -25,6 +25,8 @@ const utils = require('./utils');
 const validateRef = require('./helpers/populate/validateRef');
 const util = require('util');
 
+const hasNumericSubpathRegex = /\.\d+(\.|$)/;
+
 let MongooseTypes;
 
 const queryHooks = require('./helpers/query/applyQueryMiddleware').
@@ -1008,7 +1010,7 @@ Schema.prototype.path = function(path, obj) {
     }
 
     // subpaths?
-    return /\.\d+\.?.*$/.test(path)
+    return hasNumericSubpathRegex.test(path)
       ? getPositionalPath(this, path, cleanPath)
       : undefined;
   }

diff --git a/lib/types/subdocument.js b/lib/types/subdocument.js
@@ -182,7 +182,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') {
@@ -192,10 +192,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
@@ -197,6 +197,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/test/document.test.js b/test/document.test.js
@@ -12769,6 +12769,28 @@ describe('document', function() {
     );
   });
 
+  it('handles reusing schema with embedded discriminators defined using Schema.prototype.discriminator (gh-14162)', async function() {
+    const discriminated = new Schema({
+      type: { type: Number, required: true }
+    }, { discriminatorKey: 'type' });
+
+    discriminated.discriminator(1, new Schema({ prop1: String }));
+    discriminated.discriminator(3, new Schema({ prop2: String }));
+
+    const containerSchema = new Schema({ items: [discriminated] });
+    const containerModel = db.model('Test', containerSchema);
+    const containerModel2 = db.model('Test1', containerSchema);
+    const doc1 = new containerModel({ items: [{ type: 1, prop1: 'foo' }, { type: 3, prop2: 'bar' }] });
+    const doc2 = new containerModel2({ items: [{ type: 1, prop1: 'baz' }, { type: 3, prop2: 'qux' }] });
+    await doc1.save();
+    await doc2.save();
+
+    doc1.items.push({ type: 3, prop2: 'test1' });
+    doc2.items.push({ type: 3, prop2: 'test1' });
+    await doc1.save();
+    await doc2.save();
+  });
+
   it('can use `collection` as schema name (gh-13956)', async function() {
     const schema = new mongoose.Schema({ name: String, collection: String });
     const Test = db.model('Test', schema);
@@ -12802,6 +12824,98 @@ describe('document', function() {
       ['__stateBeforeSuspension', '__stateBeforeSuspension.jsonField']
     );
   });
+
+  it('should allow null values in list in self assignment (gh-14172) (gh-13859)', async function() {
+    const objSchema = new Schema({
+      date: Date,
+      value: Number
+    });
+
+    const testSchema = new Schema({
+      intArray: [Number],
+      strArray: [String],
+      objArray: [objSchema]
+    });
+    const Test = db.model('Test', testSchema);
+
+    const doc = new Test({
+      intArray: [1, 2, 3, null],
+      strArray: ['b', null, 'c'],
+      objArray: [
+        { date: new Date(1000), value: 1 },
+        null,
+        { date: new Date(3000), value: 3 }
+      ]
+    });
+    await doc.save();
+    doc.intArray = doc.intArray;
+    doc.strArray = doc.strArray;
+    doc.objArray = doc.objArray; // this is the trigger for the error
+    assert.ok(doc);
+    await doc.save();
+    assert.ok(doc);
+  });
+
+  it('avoids overwriting dotted paths in mixed path underneath nested path (gh-14178)', async function() {
+    const testSchema = new Schema({
+      __stateBeforeSuspension: {
+        field1: String,
+        field3: { type: Schema.Types.Mixed }
+      }
+    });
+    const Test = db.model('Test', testSchema);
+    const eventObj = new Test({
+      __stateBeforeSuspension: { field1: 'test' }
+    });
+    await eventObj.save();
+    const newO = eventObj.toObject();
+    newO.__stateBeforeSuspension.field3 = { '.ippo': 5 };
+    eventObj.set(newO);
+    await eventObj.save();
+
+    assert.strictEqual(eventObj.__stateBeforeSuspension.field3['.ippo'], 5);
+
+    const fromDb = await Test.findById(eventObj._id).lean().orFail();
+    assert.strictEqual(fromDb.__stateBeforeSuspension.field3['.ippo'], 5);
+  });
+
+  it('handles setting nested path to null (gh-14205)', function() {
+    const schema = new mongoose.Schema({
+      nested: {
+        key1: String,
+        key2: String
+      }
+    });
+
+    const Model = db.model('Test', schema);
+
+    const doc = new Model();
+    doc.init({
+      nested: { key1: 'foo', key2: 'bar' }
+    });
+
+    doc.set({ nested: null });
+    assert.strictEqual(doc.toObject().nested, null);
+  });
+
+  it('handles setting nested path to undefined (gh-14205)', function() {
+    const schema = new mongoose.Schema({
+      nested: {
+        key1: String,
+        key2: String
+      }
+    });
+
+    const Model = db.model('Test', schema);
+
+    const doc = new Model();
+    doc.init({
+      nested: { key1: 'foo', key2: 'bar' }
+    });
+
+    doc.set({ nested: void 0 });
+    assert.strictEqual(doc.toObject().nested, void 0);
+  });
 });
 
 describe('Check if instance function that is supplied in schema option is availabe', function() {

diff --git a/test/model.test.js b/test/model.test.js
@@ -3550,6 +3550,7 @@ describe('Model', function() {
           assert.equal(changeData.operationType, 'insert');
           assert.equal(changeData.fullDocument.name, 'Ned Stark');
 
+          await changeStream.close();
           await db.close();
         });