Merge pull request #12601 from Automattic/vkarpov15/gh-12494

Better support for populating maps of arrays of refs

lib/helpers/populate/assignVals.js

@@ -52,6 +52,11 @@ module.exports = function assignVals(o) {
 
     const _allIds = o.allIds[i];
 
+    if (o.path.endsWith('.$*')) {
+      // Skip maps re: gh-12494
+      return valueFilter(val, options, populateOptions, _allIds);
+    }
+
     if (o.justOne === true && Array.isArray(val)) {
       // Might be an embedded discriminator (re: gh-9244) with multiple models, so make sure to pick the right
       // model before assigning.

lib/helpers/populate/getModelsMapForPopulate.js

@@ -207,6 +207,7 @@ module.exports = function getModelsMapForPopulate(model, docs, options) {
     let isRefPath = false;
     let justOne = null;
 
+    const originalSchema = schema;
     if (schema && schema.instance === 'Array') {
       schema = schema.caster;
     }
@@ -277,7 +278,9 @@ module.exports = function getModelsMapForPopulate(model, docs, options) {
         schemaForCurrentDoc = schema;
       }
 
-      if (schemaForCurrentDoc != null) {
+      if (originalSchema && originalSchema.path.endsWith('.$*')) {
+        justOne = !originalSchema.$isMongooseArray && !originalSchema._arrayPath;
+      } else if (schemaForCurrentDoc != null) {
         justOne = !schemaForCurrentDoc.$isMongooseArray && !schemaForCurrentDoc._arrayPath;
       }
 

lib/model.js

@@ -4784,7 +4784,11 @@ function populate(model, docs, options, callback) {
       for (const val of vals) {
         mod.options._childDocs.push(val);
       }
-      _assign(model, vals, mod, assignmentOpts);
+      try {
+        _assign(model, vals, mod, assignmentOpts);
+      } catch (err) {
+        return callback(err);
+      }
     }
 
     for (const arr of params) {

lib/types/map.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const Mixed = require('../schema/mixed');
+const MongooseError = require('../error/mongooseError');
 const clone = require('../helpers/clone');
 const deepEqual = require('../utils').deepEqual;
 const getConstructorName = require('../helpers/getConstructorName');
@@ -97,15 +98,33 @@ class MongooseMap extends Map {
 
     const fullPath = this.$__path + '.' + key;
     const populated = this.$__parent != null && this.$__parent.$__ ?
-      this.$__parent.$populated(fullPath) || this.$__parent.$populated(this.$__path) :
+      this.$__parent.$populated(fullPath, true) || this.$__parent.$populated(this.$__path, true) :
       null;
     const priorVal = this.get(key);
 
     if (populated != null) {
-      if (value.$__ == null) {
-        value = new populated.options[populateModelSymbol](value);
+      if (this.$__schemaType.$isSingleNested) {
+        throw new MongooseError(
+          'Cannot manually populate single nested subdoc underneath Map ' +
+          `at path "${this.$__path}". Try using an array instead of a Map.`
+        );
+      }
+      if (Array.isArray(value) && this.$__schemaType.$isMongooseArray) {
+        value = value.map(v => {
+          if (v.$__ == null) {
+            v = new populated.options[populateModelSymbol](v);
+          }
+          // Doesn't support single nested "in-place" populate
+          v.$__.wasPopulated = { value: v._id };
+          return v;
+        });
+      } else {
+        if (value.$__ == null) {
+          value = new populated.options[populateModelSymbol](value);
+        }
+        // Doesn't support single nested "in-place" populate
+        value.$__.wasPopulated = { value: value._id };
       }
-      value.$__.wasPopulated = { value: populated.value };
     } else {
       try {
         value = this.$__schemaType.

test/document.test.js

@@ -11016,6 +11016,54 @@ describe('document', function() {
     assert.equal(foo.get('bar.another'), 2);
   });
 
+  it('populating subdocument refs underneath maps throws (gh-12494) (gh-10856)', async function() {
+    // Bar model, has a name property and some other properties that we are interested in
+    const BarSchema = new Schema({
+      name: String,
+      more: String,
+      another: Number
+    });
+    const Bar = db.model('Bar', BarSchema);
+
+    // Denormalised Bar schema with just the name, for use on the Foo model
+    const BarNameSchema = new Schema({
+      _id: {
+        type: Schema.Types.ObjectId,
+        ref: 'Bar'
+      },
+      name: String
+    });
+
+    // Foo model, which contains denormalized bar data (just the name)
+    const FooSchema = new Schema({
+      something: String,
+      other: Number,
+      map: {
+        type: Map,
+        of: {
+          type: BarNameSchema,
+          ref: 'Bar'
+        }
+      }
+    });
+    const Foo = db.model('Foo', FooSchema);
+
+    const bar = await Bar.create({
+      name: 'I am Bar',
+      more: 'With more data',
+      another: 2
+    });
+    const { _id } = await Foo.create({
+      something: 'I am Foo',
+      other: 1,
+      map: { test: bar }
+    });
+
+    const err = await Foo.findById(_id).populate('map').then(() => null, err => err);
+    assert.ok(err);
+    assert.ok(err.message.includes('Cannot manually populate single nested subdoc underneath Map'), err.message);
+  });
+
   it('handles save with undefined nested doc under subdoc (gh-11110)', async function() {
     const testSchema = new Schema({
       level_1_array: [new Schema({

test/types.map.test.js

@@ -1050,4 +1050,56 @@ describe('Map', function() {
     const res = doc.toObject({ flattenMaps: true });
     assert.equal(res.l1.l1key.l2.l2key.value, 'abc');
   });
+
+  it('handles populating map of arrays (gh-12494)', async function() {
+    const User = new mongoose.Schema({
+      name: String,
+      addresses: {
+        type: Map,
+        of: [{
+          type: mongoose.Schema.Types.ObjectId,
+          ref: 'Address'
+        }],
+        default: {}
+      }
+    });
+
+    const Address = new mongoose.Schema({
+      city: String
+    });
+
+    const UserModel = db.model('User', User);
+    const AddressModel = db.model('Address', Address);
+
+    const address = await AddressModel.create({ city: 'London' });
+
+    const { _id } = await UserModel.create({
+      name: 'Name',
+      addresses: {
+        home: [address._id]
+      }
+    });
+
+    // Using `.$*`
+    let query = UserModel.findById(_id);
+    query.populate({
+      path: 'addresses.$*'
+    });
+
+    let doc = await query.exec();
+    assert.ok(Array.isArray(doc.addresses.get('home')));
+    assert.equal(doc.addresses.get('home').length, 1);
+    assert.equal(doc.addresses.get('home')[0].city, 'London');
+
+    // Populating just one path in the map
+    query = UserModel.findById(_id);
+    query.populate({
+      path: 'addresses.home'
+    });
+
+    doc = await query.exec();
+    assert.ok(Array.isArray(doc.addresses.get('home')));
+    assert.equal(doc.addresses.get('home').length, 1);
+    assert.equal(doc.addresses.get('home')[0].city, 'London');
+  });
 });