WIP debugging #15026, refactor populate() to use async/await under th…

…e hood for stack traces
Automattic · Dec 8, 2024 · 467db46 · 467db46
1 parent 8799da2
commit 467db46
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 76 deletions.
diff --git a/lib/helpers/populate/getModelsMapForPopulate.js b/lib/helpers/populate/getModelsMapForPopulate.js
@@ -45,6 +45,10 @@ module.exports = function getModelsMapForPopulate(model, docs, options) {
   let allSchemaTypes = getSchemaTypes(model, modelSchema, null, options.path);
   allSchemaTypes = Array.isArray(allSchemaTypes) ? allSchemaTypes : [allSchemaTypes].filter(v => v != null);
 
+  /*if (options.path === 'subChildA.grandChildA') {
+    console.log('H', options.path, options._fullPath, options.strictPopulate, options.options?.strictPopulate, allSchemaTypes.length, options._localModel?.modelName, model.modelName);
+    console.log(docs);
+  }*/
   const isStrictPopulateDisabled = options.strictPopulate === false || options.options?.strictPopulate === false;
   if (!isStrictPopulateDisabled && allSchemaTypes.length === 0 && options._localModel != null) {
     return new StrictPopulate(options._fullPath || options.path);

diff --git a/lib/model.js b/lib/model.js
@@ -4248,17 +4248,9 @@ Model.populate = async function populate(docs, paths) {
   const _this = this;
   // normalized paths
   paths = utils.populate(paths);
-  // data that should persist across subPopulate calls
-  const cache = {};
 
-  return new Promise((resolve, reject) => {
-    _populate(_this, docs, paths, cache, (err, res) => {
-      if (err) {
-        return reject(err);
-      }
-      resolve(res);
-    });
-  });
+  await _populate(_this, docs, paths);
+  return docs;
 };
 
 /**
@@ -4273,25 +4265,17 @@ Model.populate = async function populate(docs, paths) {
  * @api private
  */
 
-function _populate(model, docs, paths, cache, callback) {
-  let pending = paths.length;
+async function _populate(model, docs, paths) {
   if (paths.length === 0) {
-    return callback(null, docs);
+    return;
   }
   // each path has its own query options and must be executed separately
+  const promises = [];
   for (const path of paths) {
-    populate(model, docs, path, next);
+    promises.push(populate(model, docs, path));
   }
 
-  function next(err) {
-    if (err) {
-      return callback(err, null);
-    }
-    if (--pending) {
-      return;
-    }
-    callback(null, docs);
-  }
+  await Promise.all(promises);
 }
 
 /*!
@@ -4300,7 +4284,7 @@ function _populate(model, docs, paths, cache, callback) {
 const excludeIdReg = /\s?-_id\s?/;
 const excludeIdRegGlobal = /\s?-_id\s?/g;
 
-function populate(model, docs, options, callback) {
+async function populate(model, docs, options) {
   const populateOptions = options;
   if (options.strictPopulate == null) {
     if (options._localModel != null && options._localModel.schema._userProvidedOptions.strictPopulate != null) {
@@ -4317,15 +4301,13 @@ function populate(model, docs, options, callback) {
     docs = [docs];
   }
   if (docs.length === 0 || docs.every(utils.isNullOrUndefined)) {
-    return callback();
+    return;
   }
 
   const modelsMap = getModelsMapForPopulate(model, docs, populateOptions);
-
+  // console.log('ModelsMap', modelsMap);
   if (modelsMap instanceof MongooseError) {
-    return immediate(function() {
-      callback(modelsMap);
-    });
+    throw modelsMap;
   }
   const len = modelsMap.length;
   let vals = [];
@@ -4335,7 +4317,6 @@ function populate(model, docs, options, callback) {
     return undefined !== item;
   }
 
-  let _remaining = len;
   let hasOne = false;
   const params = [];
   for (let i = 0; i < len; ++i) {
@@ -4366,7 +4347,6 @@ function populate(model, docs, options, callback) {
       // Ensure that we set to 0 or empty array even
       // if we don't actually execute a query to make sure there's a value
       // and we know this path was populated for future sets. See gh-7731, gh-8230
-      --_remaining;
       _assign(model, [], mod, assignmentOpts);
       continue;
     }
@@ -4397,72 +4377,58 @@ function populate(model, docs, options, callback) {
     } else if (mod.options.limit != null) {
       assignmentOpts.originalLimit = mod.options.limit;
     }
-    params.push([mod, match, select, assignmentOpts, _next]);
+    params.push([mod, match, select, assignmentOpts]);
   }
   if (!hasOne) {
     // If models but no docs, skip further deep populate.
     if (modelsMap.length !== 0) {
-      return callback();
+      return;
     }
-    // If no models to populate but we have a nested populate,
-    // keep trying, re: gh-8946
+    // If no models and no docs to populate but we have a nested populate,
+    // probably a case of unnecessarily populating a non-ref path re: gh-8946
     if (populateOptions.populate != null) {
       const opts = utils.populate(populateOptions.populate).map(pop => Object.assign({}, pop, {
         path: populateOptions.path + '.' + pop.path
       }));
-      model.populate(docs, opts).then(res => { callback(null, res); }, err => { callback(err); });
-      return;
+      return model.populate(docs, opts);
     }
-    return callback();
+    return;
   }
 
+  const promises = [];
   for (const arr of params) {
-    _execPopulateQuery.apply(null, arr);
-  }
-  function _next(err, valsFromDb) {
-    if (err != null) {
-      return callback(err, null);
-    }
-    vals = vals.concat(valsFromDb);
-    if (--_remaining === 0) {
-      _done();
-    }
+    promises.push(_execPopulateQuery.apply(null, arr).then(valsFromDb => { vals = vals.concat(valsFromDb); }));
   }
 
-  function _done() {
-    for (const arr of params) {
-      const mod = arr[0];
-      const assignmentOpts = arr[3];
-      for (const val of vals) {
-        mod.options._childDocs.push(val);
-      }
-      try {
-        _assign(model, vals, mod, assignmentOpts);
-      } catch (err) {
-        return callback(err);
-      }
-    }
+  await Promise.all(promises);
 
-    for (const arr of params) {
-      removeDeselectedForeignField(arr[0].foreignField, arr[0].options, vals);
+  for (const arr of params) {
+    const mod = arr[0];
+    const assignmentOpts = arr[3];
+    for (const val of vals) {
+      mod.options._childDocs.push(val);
     }
-    for (const arr of params) {
-      const mod = arr[0];
-      if (mod.options && mod.options.options && mod.options.options._leanTransform) {
-        for (const doc of vals) {
-          mod.options.options._leanTransform(doc);
-        }
+    _assign(model, vals, mod, assignmentOpts);
+  }
+
+  for (const arr of params) {
+    removeDeselectedForeignField(arr[0].foreignField, arr[0].options, vals);
+  }
+  for (const arr of params) {
+    const mod = arr[0];
+    if (mod.options && mod.options.options && mod.options.options._leanTransform) {
+      for (const doc of vals) {
+        mod.options.options._leanTransform(doc);
       }
     }
-    callback();
   }
 }
 
 /*!
  * ignore
  */
 
-function _execPopulateQuery(mod, match, select, assignmentOpts, callback) {
+function _execPopulateQuery(mod, match, select) {
   let subPopulate = clone(mod.options.populate);
   const queryOptions = Object.assign({
     skip: mod.options.skip,
@@ -4528,15 +4494,12 @@ function _execPopulateQuery(mod, match, select, assignmentOpts, callback) {
     query.populate(subPopulate);
   }
 
-  query.exec().then(
+  return query.exec().then(
     docs => {
       for (const val of docs) {
         leanPopulateMap.set(val, mod.model);
       }
-      callback(null, docs);
-    },
-    err => {
-      callback(err);
+      return docs;
     }
   );
 }