Skip to content

Commit

Permalink
Merge pull request #15238 from Automattic/revert-15231-vkarpov15/gh-1…
Browse files Browse the repository at this point in the history
…5210

Revert "Introduce populate ordered option for populating in series rather than in parallel"
  • Loading branch information
vkarpov15 authored Feb 6, 2025
2 parents 9d68b0a + 1668bbb commit 16d2407
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 85 deletions.
3 changes: 0 additions & 3 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -4509,8 +4509,6 @@ Document.prototype.equals = function(doc) {
* @param {Object|Function} [options.match=null] Add an additional filter to the populate query. Can be a filter object containing [MongoDB query syntax](https://www.mongodb.com/docs/manual/tutorial/query-documents/), or a function that returns a filter object.
* @param {Function} [options.transform=null] Function that Mongoose will call on every populated document that allows you to transform the populated document.
* @param {Object} [options.options=null] Additional options like `limit` and `lean`.
* @param {Boolean} [options.forceRepopulate=true] Set to `false` to prevent Mongoose from repopulating paths that are already populated
* @param {Boolean} [options.ordered=false] Set to `true` to execute any populate queries one at a time, as opposed to in parallel. We recommend setting this option to `true` if using transactions, especially if also populating multiple paths or paths with multiple models. MongoDB server does **not** support multiple operations in parallel on a single transaction.
* @param {Function} [callback] Callback
* @see population https://mongoosejs.com/docs/populate.html
* @see Query#select https://mongoosejs.com/docs/api/query.html#Query.prototype.select()
Expand All @@ -4537,7 +4535,6 @@ Document.prototype.populate = async function populate() {
}

const paths = utils.object.vals(pop);

let topLevelModel = this.constructor;
if (this.$__isNested) {
topLevelModel = this.$__[scopeSymbol].constructor;
Expand Down
37 changes: 8 additions & 29 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -4369,7 +4369,6 @@ Model.validate = async function validate(obj, pathsOrOptions, context) {
* @param {Object} [options.options=null] Additional options like `limit` and `lean`.
* @param {Function} [options.transform=null] Function that Mongoose will call on every populated document that allows you to transform the populated document.
* @param {Boolean} [options.forceRepopulate=true] Set to `false` to prevent Mongoose from repopulating paths that are already populated
* @param {Boolean} [options.ordered=false] Set to `true` to execute any populate queries one at a time, as opposed to in parallel. Set this option to `true` if populating multiple paths or paths with multiple models in transactions.
* @return {Promise}
* @api public
*/
Expand All @@ -4387,21 +4386,11 @@ Model.populate = async function populate(docs, paths) {
}

// each path has its own query options and must be executed separately
if (paths.find(p => p.ordered)) {
// Populate in series, primarily for transactions because MongoDB doesn't support multiple operations on
// one transaction in parallel.
// Note that if _any_ path has `ordered`, we make the top-level populate `ordered` as well.
for (const path of paths) {
await _populatePath(this, docs, path);
}
} else {
// By default, populate in parallel
const promises = [];
for (const path of paths) {
promises.push(_populatePath(this, docs, path));
}
await Promise.all(promises);
const promises = [];
for (const path of paths) {
promises.push(_populatePath(this, docs, path));
}
await Promise.all(promises);

return docs;
};
Expand Down Expand Up @@ -4521,22 +4510,12 @@ async function _populatePath(model, docs, populateOptions) {
return;
}

if (populateOptions.ordered) {
// Populate in series, primarily for transactions because MongoDB doesn't support multiple operations on
// one transaction in parallel.
for (const arr of params) {
await _execPopulateQuery.apply(null, arr).then(valsFromDb => { vals = vals.concat(valsFromDb); });
}
} else {
// By default, populate in parallel
const promises = [];
for (const arr of params) {
promises.push(_execPopulateQuery.apply(null, arr).then(valsFromDb => { vals = vals.concat(valsFromDb); }));
}

await Promise.all(promises);
const promises = [];
for (const arr of params) {
promises.push(_execPopulateQuery.apply(null, arr).then(valsFromDb => { vals = vals.concat(valsFromDb); }));
}

await Promise.all(promises);

for (const arr of params) {
const mod = arr[0];
Expand Down
10 changes: 3 additions & 7 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -551,8 +551,8 @@ exports.populate = function populate(path, select, model, match, options, subPop
};
}

if (typeof obj.path !== 'string' && !(Array.isArray(obj.path) && obj.path.every(el => typeof el === 'string'))) {
throw new TypeError('utils.populate: invalid path. Expected string or array of strings. Got typeof `' + typeof path + '`');
if (typeof obj.path !== 'string') {
throw new TypeError('utils.populate: invalid path. Expected string. Got typeof `' + typeof path + '`');
}

return _populateObj(obj);
Expand Down Expand Up @@ -600,11 +600,7 @@ function _populateObj(obj) {
}

const ret = [];
const paths = oneSpaceRE.test(obj.path)
? obj.path.split(manySpaceRE)
: Array.isArray(obj.path)
? obj.path
: [obj.path];
const paths = oneSpaceRE.test(obj.path) ? obj.path.split(manySpaceRE) : [obj.path];
if (obj.options != null) {
obj.options = clone(obj.options);
}
Expand Down
40 changes: 0 additions & 40 deletions test/document.populate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1075,44 +1075,4 @@ describe('document.populate', function() {
assert.deepStrictEqual(codeUser.extras[0].config.paymentConfiguration.paymentMethods[0]._id, code._id);
assert.strictEqual(codeUser.extras[0].config.paymentConfiguration.paymentMethods[0].code, 'test code');
});

it('supports populate with ordered option (gh-15231)', async function() {
const docSchema = new Schema({
refA: { type: Schema.Types.ObjectId, ref: 'Test1' },
refB: { type: Schema.Types.ObjectId, ref: 'Test2' },
refC: { type: Schema.Types.ObjectId, ref: 'Test3' }
});

const doc1Schema = new Schema({ name: String });
const doc2Schema = new Schema({ title: String });
const doc3Schema = new Schema({ content: String });

const Doc = db.model('Test', docSchema);
const Doc1 = db.model('Test1', doc1Schema);
const Doc2 = db.model('Test2', doc2Schema);
const Doc3 = db.model('Test3', doc3Schema);

const doc1 = await Doc1.create({ name: 'test 1' });
const doc2 = await Doc2.create({ title: 'test 2' });
const doc3 = await Doc3.create({ content: 'test 3' });

const docD = await Doc.create({
refA: doc1._id,
refB: doc2._id,
refC: doc3._id
});

await docD.populate({
path: ['refA', 'refB', 'refC'],
ordered: true
});

assert.ok(docD.populated('refA'));
assert.ok(docD.populated('refB'));
assert.ok(docD.populated('refC'));

assert.equal(docD.refA.name, 'test 1');
assert.equal(docD.refB.title, 'test 2');
assert.equal(docD.refC.content, 'test 3');
});
});
6 changes: 0 additions & 6 deletions types/populate.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,6 @@ declare module 'mongoose' {
foreignField?: string;
/** Set to `false` to prevent Mongoose from repopulating paths that are already populated */
forceRepopulate?: boolean;
/**
* Set to `true` to execute any populate queries one at a time, as opposed to in parallel.
* We recommend setting this option to `true` if using transactions, especially if also populating multiple paths or paths with multiple models.
* MongoDB server does **not** support multiple operations in parallel on a single transaction.
*/
ordered?: boolean;
}

interface PopulateOption {
Expand Down

0 comments on commit 16d2407

Please sign in to comment.