Merge branch 'master' into 4.13

Automattic · Oct 30, 2017 · b9c2d3a · b9c2d3a
2 parents a51e1f5 + f43a984
commit b9c2d3a
Show file tree

Hide file tree

Showing 15 changed files with 302 additions and 15 deletions.
diff --git a/History.md b/History.md
@@ -1,3 +1,12 @@
+4.12.5 / 2017-10-29
+===================
+ * fix(query): correctly handle `$in` and required for $pull and update validators #5744
+ * feat(aggegate): add $addFields pipeline operator #5740 [AyushG3112](https://github.com/AyushG3112)
+ * fix(document): catch sync errors in document pre hooks and report as error #5738
+ * fix(populate): handle slice projections correctly when automatically selecting populated fields #5737
+ * fix(discriminator): fix hooks for embedded discriminators #5706 [wlingke](https://github.com/wlingke)
+ * fix(model): throw sane error when customer calls `mongoose.Model()` over `mongoose.model()` #2005
+
 4.12.4 / 2017-10-21
 ===================
  * test(plugins): add coverage for idGetter with id as a schema property #5713 [wlingke](https://github.com/wlingke)

diff --git a/lib/query.js b/lib/query.js
@@ -15,6 +15,7 @@ var mquery = require('mquery');
 var readPref = require('./drivers').ReadPreference;
 var selectPopulatedFields = require('./services/query/selectPopulatedFields');
 var setDefaultsOnInsert = require('./services/setDefaultsOnInsert');
+var slice = require('sliced');
 var updateValidators = require('./services/updateValidators');
 var util = require('util');
 var utils = require('./utils');
@@ -263,6 +264,49 @@ Query.prototype.toConstructor = function toConstructor() {
  * @api public
  */
 
+Query.prototype.slice = function() {
+  if (arguments.length === 0) {
+    return this;
+  }
+
+  this._validate('slice');
+
+  var path;
+  var val;
+
+  if (arguments.length === 1) {
+    var arg = arguments[0];
+    if (typeof arg === 'object' && !Array.isArray(arg)) {
+      var keys = Object.keys(arg);
+      var numKeys = keys.length;
+      for (var i = 0; i < numKeys; ++i) {
+        this.slice(keys[i], arg[keys[i]]);
+      }
+      return this;
+    }
+    this._ensurePath('slice');
+    path = this._path;
+    val = arguments[0];
+  } else if (arguments.length === 2) {
+    if ('number' === typeof arguments[0]) {
+      this._ensurePath('slice');
+      path = this._path;
+      val = slice(arguments);
+    } else {
+      path = arguments[0];
+      val = arguments[1];
+    }
+  } else if (arguments.length === 3) {
+    path = arguments[0];
+    val = slice(arguments, 1);
+  }
+
+  var p = {};
+  p[path] = { $slice: val };
+  return this.select(p);
+};
+
+
 /**
  * Specifies the complementary comparison value for paths specified with `where()`
  *

diff --git a/lib/schema.js b/lib/schema.js
@@ -299,11 +299,13 @@ Schema.prototype.tree;
 Schema.prototype.clone = function() {
   var s = new Schema(this.paths, this.options);
   // Clone the call queue
+  var cloneOpts = { retainKeyOrder: true };
   s.callQueue = this.callQueue.map(function(f) { return f; });
-  s.methods = utils.clone(this.methods);
-  s.statics = utils.clone(this.statics);
+  s.methods = utils.clone(this.methods, cloneOpts);
+  s.statics = utils.clone(this.statics, cloneOpts);
+  s.query = utils.clone(this.query, cloneOpts);
   s.plugins = Array.prototype.slice.call(this.plugins);
-  s._indexes = utils.clone(this._indexes);
+  s._indexes = utils.clone(this._indexes, cloneOpts);
   s.s.hooks = this.s.hooks.clone();
   return s;
 };

diff --git a/lib/schema/documentarray.js b/lib/schema/documentarray.js
@@ -11,6 +11,7 @@ var EventEmitter = require('events').EventEmitter;
 var MongooseDocumentArray = require('../types/documentarray');
 var SchemaType = require('../schematype');
 var Subdocument = require('../types/embedded');
+var applyHooks = require('../services/model/applyHooks');
 var discriminator = require('../services/model/discriminator');
 var util = require('util');
 var utils = require('../utils');
@@ -119,6 +120,8 @@ DocumentArray.prototype.discriminator = function(name, schema) {
 
   this.casterConstructor.discriminators[name] = EmbeddedDocument;
 
+  applyHooks(EmbeddedDocument, schema);
+
   return this.casterConstructor.discriminators[name];
 };
 

diff --git a/lib/schema/embedded.js b/lib/schema/embedded.js
@@ -8,6 +8,7 @@ var $exists = require('./operators/exists');
 var EventEmitter = require('events').EventEmitter;
 var SchemaType = require('../schematype');
 var Subdocument = require('../types/subdocument');
+var applyHooks = require('../services/model/applyHooks');
 var castToNumber = require('./operators/helpers').castToNumber;
 var discriminator = require('../services/model/discriminator');
 var geospatial = require('./operators/geospatial');
@@ -253,5 +254,8 @@ Embedded.prototype.discriminator = function(name, schema) {
   discriminator(this.caster, name, schema);
 
   this.caster.discriminators[name] = _createConstructor(schema);
+
+  applyHooks(this.caster.discriminators[name], schema);
+
   return this.caster.discriminators[name];
 };
diff --git a/lib/schema/objectid.js b/lib/schema/objectid.js
@@ -20,7 +20,7 @@ var SchemaType = require('../schematype'),
  */
 
 function ObjectId(key, options) {
-  var isKeyHexStr = typeof key === 'string' && /^a-f0-9$/i.test(key);
+  var isKeyHexStr = typeof key === 'string' && key.length === 24 && /^a-f0-9$/i.test(key);
   var suppressWarning = options && options.suppressWarning;
   if ((isKeyHexStr || typeof key === 'undefined') && !suppressWarning) {
     console.warn('mongoose: To create a new ObjectId please try ' +

diff --git a/lib/services/query/selectPopulatedFields.js b/lib/services/query/selectPopulatedFields.js
@@ -41,5 +41,5 @@ function isPathInFields(userProvidedFields, path) {
     }
     cur += '.' + pieces[i];
   }
-  return false;
+  return userProvidedFields[cur] != null;
 }
diff --git a/lib/services/updateValidators.js b/lib/services/updateValidators.js
@@ -30,11 +30,12 @@ module.exports = function(query, schema, castedDoc, options) {
   var hasDollarUpdate = false;
   var modified = {};
   var currentUpdate;
+  var key;
 
   for (var i = 0; i < numKeys; ++i) {
     if (keys[i].charAt(0) === '$') {
-      if (keys[i] === '$push' || keys[i] === '$addToSet' ||
-          keys[i] === '$pull' || keys[i] === '$pullAll') {
+      hasDollarUpdate = true;
+      if (keys[i] === '$push' || keys[i] === '$addToSet') {
         _keys = Object.keys(castedDoc[keys[i]]);
         for (var ii = 0; ii < _keys.length; ++ii) {
           currentUpdate = castedDoc[keys[i]][_keys[ii]];
@@ -55,14 +56,15 @@ module.exports = function(query, schema, castedDoc, options) {
       for (var j = 0; j < numPaths; ++j) {
         var updatedPath = paths[j].replace('.$.', '.0.');
         updatedPath = updatedPath.replace(/\.\$$/, '.0');
-        if (keys[i] === '$set' || keys[i] === '$setOnInsert') {
+        key = keys[i];
+        if (key === '$set' || key === '$setOnInsert' ||
+            key === '$pull' || key === '$pullAll') {
           updatedValues[updatedPath] = flat[paths[j]];
-        } else if (keys[i] === '$unset') {
+        } else if (key === '$unset') {
           updatedValues[updatedPath] = undefined;
         }
         updatedKeys[updatedPath] = true;
       }
-      hasDollarUpdate = true;
     }
   }
 

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -21,7 +21,7 @@
   "dependencies": {
     "async": "2.1.4",
     "bson": "~1.0.4",
-    "hooks-fixed": "2.0.0",
+    "hooks-fixed": "2.0.2",
     "kareem": "1.5.0",
     "lodash.get": "4.4.2",
     "mongodb": "2.2.33",

diff --git a/test/document.hooks.test.js b/test/document.hooks.test.js
@@ -763,6 +763,24 @@ describe('document: hooks:', function() {
     done();
   });
 
+  it('sync exceptions get passed as errors (gh-5738)', function(done) {
+    var bookSchema = new Schema({ title: String });
+
+    /* eslint-disable no-unused-vars */
+    bookSchema.pre('save', function(next) {
+      throw new Error('woops!');
+    });
+
+    var Book = mongoose.model('gh5738', bookSchema);
+
+    var book = new Book({ title: 'Professional AngularJS' });
+    book.save(function(error) {
+      assert.ok(error);
+      assert.equal(error.message, 'woops!');
+      done();
+    });
+  });
+
   it('nested subdocs only fire once (gh-3281)', function(done) {
     var L3Schema = new Schema({
       title: String

diff --git a/test/model.discriminator.test.js b/test/model.discriminator.test.js
@@ -842,5 +842,126 @@ describe('model', function() {
         }).
         catch(done);
     });
+    describe('embedded discriminators + hooks (gh-5706)', function(){
+      var counters = {
+        eventPreSave: 0,
+        eventPostSave: 0,
+        purchasePreSave: 0,
+        purchasePostSave: 0,
+        eventPreValidate: 0,
+        eventPostValidate: 0,
+        purchasePreValidate: 0,
+        purchasePostValidate: 0,
+      };
+      var eventSchema = new Schema(
+        { message: String },
+        { discriminatorKey: 'kind', _id: false }
+      );
+      eventSchema.pre('validate', function(next) {
+        counters.eventPreValidate++;
+        next();
+      });
+
+      eventSchema.post('validate', function(doc) {
+        counters.eventPostValidate++;
+      });
+
+      eventSchema.pre('save', function(next) {
+        counters.eventPreSave++;
+        next();
+      });
+
+      eventSchema.post('save', function(doc) {
+        counters.eventPostSave++;
+      });
+
+      var purchasedSchema = new Schema({
+        product: String,
+      }, { _id: false });
+
+      purchasedSchema.pre('validate', function(next) {
+        counters.purchasePreValidate++;
+        next();
+      });
+
+      purchasedSchema.post('validate', function(doc) {
+        counters.purchasePostValidate++;
+      });
+
+      purchasedSchema.pre('save', function(next) {
+        counters.purchasePreSave++;
+        next();
+      });
+
+      purchasedSchema.post('save', function(doc) {
+        counters.purchasePostSave++;
+      });
+
+      beforeEach(function() {
+        Object.keys(counters).forEach(function(i) {
+          counters[i] = 0;
+        });
+      });
+
+      it('should call the hooks on the embedded document defined by both the parent and discriminated schemas', function(done){
+        var trackSchema = new Schema({
+          event: eventSchema,
+        });
+
+        var embeddedEventSchema = trackSchema.path('event');
+        embeddedEventSchema.discriminator('Purchased', purchasedSchema.clone());
+
+        var TrackModel = db.model('Track', trackSchema);
+        var doc = new TrackModel({
+          event: {
+            message: 'Test',
+            kind: 'Purchased'
+          }
+        });
+        doc.save(function(err){
+          assert.ok(!err);
+          assert.equal(doc.event.message, 'Test')
+          assert.equal(doc.event.kind, 'Purchased')
+          Object.keys(counters).forEach(function(i) {
+            assert.equal(counters[i], 1);
+          });
+          done();
+        })
+      })
+
+      it('should call the hooks on the embedded document in an embedded array defined by both the parent and discriminated schemas', function(done){
+        var trackSchema = new Schema({
+          events: [eventSchema],
+        });
+
+        var embeddedEventSchema = trackSchema.path('events');
+        embeddedEventSchema.discriminator('Purchased', purchasedSchema.clone());
+
+        var TrackModel = db.model('Track2', trackSchema);
+        var doc = new TrackModel({
+          events: [
+            {
+              message: 'Test',
+              kind: 'Purchased'
+            },
+            {
+              message: 'TestAgain',
+              kind: 'Purchased'
+            }
+          ]
+        });
+        doc.save(function(err){
+          assert.ok(!err);
+          assert.equal(doc.events[0].kind, 'Purchased');
+          assert.equal(doc.events[0].message, 'Test');
+          assert.equal(doc.events[1].kind, 'Purchased');
+          assert.equal(doc.events[1].message, 'TestAgain');
+          Object.keys(counters).forEach(function(i) {
+            assert.equal(counters[i], 2);
+          });
+          done();
+        })
+      })
+    })
   });
 });
diff --git a/test/model.populate.test.js b/test/model.populate.test.js
@@ -3251,6 +3251,42 @@ describe('model: populate:', function() {
       db.close(done);
     });
 
+    it('populating an array of refs, slicing, and fetching many (gh-5737)', function(done) {
+      var BlogPost = db.model('gh5737_0', new Schema({
+        title: String,
+        fans: [{ type: ObjectId, ref: 'gh5737' }]
+      }));
+      var User = db.model('gh5737', new Schema({ name: String }));
+
+      User.create([{ name: 'Fan 1' }, { name: 'Fan 2' }], function(error, fans) {
+        assert.ifError(error);
+        var posts = [
+          { title: 'Test 1', fans: [fans[0]._id, fans[1]._id] },
+          { title: 'Test 2', fans: [fans[1]._id, fans[0]._id] }
+        ];
+        BlogPost.create(posts, function(error) {
+          assert.ifError(error);
+          BlogPost.
+            find({}).
+            slice('fans', [0, 5]).
+            populate('fans').
+            exec(function(err, blogposts) {
+              assert.ifError(error);
+
+              assert.equal(blogposts[0].title, 'Test 1');
+              assert.equal(blogposts[1].title, 'Test 2');
+
+              assert.equal(blogposts[0].fans[0].name, 'Fan 1');
+              assert.equal(blogposts[0].fans[1].name, 'Fan 2');
+
+              assert.equal(blogposts[1].fans[0].name, 'Fan 2');
+              assert.equal(blogposts[1].fans[1].name, 'Fan 1');
+              done();
+            });
+        });
+      });
+    });
+
     it('maps results back to correct document (gh-1444)', function(done) {
       var articleSchema = new Schema({
         body: String,