Skip to content

Commit

Permalink
fix(populate+discriminator): handle populating document whose discrim…
Browse files Browse the repository at this point in the history
…inator value is different from discriminator model name

Fix #8324
  • Loading branch information
vkarpov15 committed Nov 19, 2019
1 parent a32d9a8 commit ff3d36f
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 39 deletions.
2 changes: 1 addition & 1 deletion lib/helpers/discriminator/getConstructor.js
@@ -1,6 +1,6 @@
'use strict';

const getDiscriminatorByValue = require('../../queryhelpers').getDiscriminatorByValue;
const getDiscriminatorByValue = require('./getDiscriminatorByValue');

/*!
* Find the correct constructor, taking into account discriminators
Expand Down
27 changes: 27 additions & 0 deletions lib/helpers/discriminator/getDiscriminatorByValue.js
@@ -0,0 +1,27 @@
'use strict';

/*!
* returns discriminator by discriminatorMapping.value
*
* @param {Model} model
* @param {string} value
*/

module.exports = function getDiscriminatorByValue(model, value) {
let discriminator = null;
if (!model.discriminators) {
return discriminator;
}
for (const name in model.discriminators) {
const it = model.discriminators[name];
if (
it.schema &&
it.schema.discriminatorMapping &&
it.schema.discriminatorMapping.value == value
) {
discriminator = it;
break;
}
}
return discriminator;
}
17 changes: 12 additions & 5 deletions lib/helpers/populate/getModelsMapForPopulate.js
Expand Up @@ -2,6 +2,7 @@

const MongooseError = require('../../error/index');
const get = require('../get');
const getDiscriminatorByValue = require('../discriminator/getDiscriminatorByValue');
const isPathExcluded = require('../projection/isPathExcluded');
const getSchemaTypes = require('./getSchemaTypes');
const getVirtual = require('./getVirtual');
Expand Down Expand Up @@ -297,12 +298,18 @@ module.exports = function getModelsMapForPopulate(model, docs, options) {

if (!schema && discriminatorKey) {
modelForFindSchema = utils.getValue(discriminatorKey, doc);

if (modelForFindSchema) {
try {
modelForCurrentDoc = model.db.model(modelForFindSchema);
} catch (error) {
return error;
// `modelForFindSchema` is the discriminator value, so we might need
// find the discriminated model name
const discriminatorModel = getDiscriminatorByValue(model, modelForFindSchema);
if (discriminatorModel != null) {
modelForCurrentDoc = discriminatorModel;
} else {
try {
modelForCurrentDoc = model.db.model(modelForFindSchema);
} catch (error) {
return error;
}
}

schemaForCurrentDoc = modelForCurrentDoc.schema._getSchema(options.path);
Expand Down
2 changes: 1 addition & 1 deletion lib/model.js
Expand Up @@ -30,7 +30,7 @@ const assignVals = require('./helpers/populate/assignVals');
const castBulkWrite = require('./helpers/model/castBulkWrite');
const discriminator = require('./helpers/model/discriminator');
const each = require('./helpers/each');
const getDiscriminatorByValue = require('./queryhelpers').getDiscriminatorByValue;
const getDiscriminatorByValue = require('./helpers/discriminator/getDiscriminatorByValue');
const getModelsMapForPopulate = require('./helpers/populate/getModelsMapForPopulate');
const immediate = require('./helpers/immediate');
const internalToObjectOptions = require('./options').internalToObjectOptions;
Expand Down
3 changes: 2 additions & 1 deletion lib/query.js
Expand Up @@ -18,6 +18,7 @@ const castArrayFilters = require('./helpers/update/castArrayFilters');
const castUpdate = require('./helpers/query/castUpdate');
const completeMany = require('./helpers/query/completeMany');
const get = require('./helpers/get');
const getDiscriminatorByValue = require('./helpers/discriminator/getDiscriminatorByValue');
const hasDollarKeys = require('./helpers/query/hasDollarKeys');
const helpers = require('./queryhelpers');
const isInclusive = require('./helpers/projection/isInclusive');
Expand Down Expand Up @@ -4429,7 +4430,7 @@ Query.prototype._castUpdate = function _castUpdate(obj, overwrite) {
typeof filter[schema.options.discriminatorKey] !== 'object' &&
schema.discriminators != null) {
const discriminatorValue = filter[schema.options.discriminatorKey];
const byValue = helpers.getDiscriminatorByValue(this.model, discriminatorValue);
const byValue = getDiscriminatorByValue(this.model, discriminatorValue);
schema = schema.discriminators[discriminatorValue] ||
(byValue && byValue.schema) ||
schema;
Expand Down
30 changes: 2 additions & 28 deletions lib/queryhelpers.js
Expand Up @@ -7,6 +7,8 @@
const checkEmbeddedDiscriminatorKeyProjection =
require('./helpers/discriminator/checkEmbeddedDiscriminatorKeyProjection');
const get = require('./helpers/get');
const getDiscriminatorByValue =
require('./helpers/discriminator/getDiscriminatorByValue');
const isDefiningProjection = require('./helpers/projection/isDefiningProjection');
const utils = require('./utils');

Expand Down Expand Up @@ -71,34 +73,6 @@ exports.preparePopulationOptionsMQ = function preparePopulationOptionsMQ(query,
return pop;
};


/*!
* returns discriminator by discriminatorMapping.value
*
* @param {Model} model
* @param {string} value
*/
function getDiscriminatorByValue(model, value) {
let discriminator = null;
if (!model.discriminators) {
return discriminator;
}
for (const name in model.discriminators) {
const it = model.discriminators[name];
if (
it.schema &&
it.schema.discriminatorMapping &&
it.schema.discriminatorMapping.value == value
) {
discriminator = it;
break;
}
}
return discriminator;
}

exports.getDiscriminatorByValue = getDiscriminatorByValue;

/*!
* If the document is a mapped discriminator type, it returns a model instance for that type, otherwise,
* it returns an instance of the given model.
Expand Down
2 changes: 1 addition & 1 deletion lib/schema/array.js
Expand Up @@ -17,7 +17,7 @@ const util = require('util');
const utils = require('../utils');
const castToNumber = require('./operators/helpers').castToNumber;
const geospatial = require('./operators/geospatial');
const getDiscriminatorByValue = require('../queryhelpers').getDiscriminatorByValue;
const getDiscriminatorByValue = require('../helpers/discriminator/getDiscriminatorByValue');

let MongooseArray;
let EmbeddedDoc;
Expand Down
2 changes: 1 addition & 1 deletion lib/types/documentarray.js
Expand Up @@ -8,7 +8,7 @@ const CoreMongooseArray = require('./core_array');
const Document = require('../document');
const ObjectId = require('./objectid');
const castObjectId = require('../cast/objectid');
const getDiscriminatorByValue = require('../queryhelpers').getDiscriminatorByValue;
const getDiscriminatorByValue = require('../helpers/discriminator/getDiscriminatorByValue');
const internalToObjectOptions = require('../options').internalToObjectOptions;
const util = require('util');
const utils = require('../utils');
Expand Down
1 change: 0 additions & 1 deletion test/model.populate.test.js
Expand Up @@ -8825,7 +8825,6 @@ describe('model: populate:', function() {
yield Model.create({ main: d._id });

const docs = yield Main.find().populate('virtualField').exec();
console.log(docs.map(d => d.virtualField))
assert.ok(docs[0].virtualField[0].main);
});
});
Expand Down

0 comments on commit ff3d36f

Please sign in to comment.