Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better support for populating maps of arrays of refs #12601

Merged
merged 4 commits into from Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/helpers/populate/assignVals.js
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion lib/helpers/populate/getModelsMapForPopulate.js
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down
6 changes: 5 additions & 1 deletion lib/model.js
Expand Up @@ -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) {
Expand Down
27 changes: 23 additions & 4 deletions 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');
Expand Down Expand Up @@ -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.
Expand Down
48 changes: 48 additions & 0 deletions test/document.test.js
Expand Up @@ -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({
Expand Down
52 changes: 52 additions & 0 deletions test/types.map.test.js
Expand Up @@ -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 new AddressModel({ city: 'London' }).save();
vkarpov15 marked this conversation as resolved.
Show resolved Hide resolved

const { _id } = await new UserModel({
name: 'Name',
addresses: {
home: [address._id]
}
}).save();
vkarpov15 marked this conversation as resolved.
Show resolved Hide resolved

// 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');
});
});