refactor: make selectPopulatedFields only take in POJOs, rather tha…

…n whole query object re: #9973
Automattic · Mar 10, 2021 · c2582d5 · c2582d5
1 parent b5abbe6
commit c2582d5
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 86 deletions.
diff --git a/lib/helpers/query/selectPopulatedFields.js b/lib/helpers/query/selectPopulatedFields.js
@@ -7,25 +7,25 @@ const isInclusive = require('../projection/isInclusive');
  * ignore
  */
 
-module.exports = function selectPopulatedFields(query) {
-  const opts = query._mongooseOptions;
+module.exports = function selectPopulatedFields(fields, userProvidedFields, populateOptions) {
+  if (populateOptions == null) {
+    return;
+  }
 
-  if (opts.populate != null) {
-    const paths = Object.keys(opts.populate);
-    const userProvidedFields = query._userProvidedFields || {};
-    if (isInclusive(query._fields)) {
-      for (const path of paths) {
-        if (!isPathInFields(userProvidedFields, path)) {
-          query.select(path);
-        } else if (userProvidedFields[path] === 0) {
-          delete query._fields[path];
-        }
+  const paths = Object.keys(populateOptions);
+  userProvidedFields = userProvidedFields || {};
+  if (isInclusive(fields)) {
+    for (const path of paths) {
+      if (!isPathInFields(userProvidedFields, path)) {
+        fields[path] = 1;
+      } else if (userProvidedFields[path] === 0) {
+        delete fields[path];
       }
-    } else if (isExclusive(query._fields)) {
-      for (const path of paths) {
-        if (userProvidedFields[path] == null) {
-          delete query._fields[path];
-        }
+    }
+  } else if (isExclusive(fields)) {
+    for (const path of paths) {
+      if (userProvidedFields[path] == null) {
+        delete fields[path];
       }
     }
   }

diff --git a/lib/query.js b/lib/query.js
@@ -4854,7 +4854,7 @@ Query.prototype._applyPaths = function applyPaths() {
   }
 
   if (_selectPopulatedPaths) {
-    selectPopulatedFields(this);
+    selectPopulatedFields(this._fields, this._userProvidedFields, this._mongooseOptions.populate);
   }
 };
 

diff --git a/test/helpers/query.test.js b/test/helpers/query.test.js
@@ -2,98 +2,42 @@
 
 require('../common');
 
-const Query = require('../../lib/query');
-const Schema = require('../../lib/schema');
 const assert = require('assert');
 const selectPopulatedFields = require('../../lib/helpers/query/selectPopulatedFields');
 
 describe('Query helpers', function() {
   describe('selectPopulatedFields', function() {
     it('handles nested populate if parent key is projected in (gh-5669)', function(done) {
-      const schema = new Schema({
-        nested: {
-          key1: String,
-          key2: String
-        }
-      });
+      const fields = { nested: 1 };
+      selectPopulatedFields(fields, { nested: 1 }, { 'nested.key1': true });
 
-      const q = new Query({});
-      q.schema = schema;
-
-      assert.strictEqual(q._fields, void 0);
-
-      q.select('nested');
-      q.populate('nested.key1');
-      assert.deepEqual(q._fields, { nested: 1 });
-
-      selectPopulatedFields(q);
-
-      assert.deepEqual(q._fields, { nested: 1 });
+      assert.deepEqual(fields, { nested: 1 });
 
       done();
     });
 
     it('handles nested populate if parent key is projected out (gh-5669)', function(done) {
-      const schema = new Schema({
-        nested: {
-          key1: String,
-          key2: String
-        }
-      });
-
-      const q = new Query({});
-      q.schema = schema;
-
-      assert.strictEqual(q._fields, void 0);
-
-      q.select('-nested');
-      q.populate('nested.key1');
-      assert.deepEqual(q._fields, { nested: 0 });
-
-      selectPopulatedFields(q);
+      const fields = { nested: 0 };
+      selectPopulatedFields(fields, { nested: 0 }, { 'nested.key1': true });
 
-      assert.deepEqual(q._fields, { nested: 0 });
+      assert.deepEqual(fields, { nested: 0 });
 
       done();
     });
 
     it('handle explicitly excluded paths (gh-7383)', function(done) {
-      const schema = new Schema({
-        name: String,
-        other: String
-      });
+      const fields = { name: 1, other: 0 };
+      selectPopulatedFields(fields, Object.assign({}, fields), { other: 1 });
 
-      const q = new Query({});
-      q.schema = schema;
-
-      assert.strictEqual(q._fields, void 0);
-
-      q.select({ name: 1, other: 0 });
-      q.populate('other');
-      assert.deepEqual(q._fields, { name: 1, other: 0 });
-
-      selectPopulatedFields(q);
-
-      assert.deepEqual(q._fields, { name: 1 });
+      assert.deepEqual(fields, { name: 1 });
 
       done();
     });
 
     it('handles paths selected with elemMatch (gh-9973)', function(done) {
-      const schema = new Schema({
-        name: String,
-        arr: [{ el: String }]
-      });
-
-      const q = new Query({});
-      q.schema = schema;
-
-      assert.strictEqual(q._fields, void 0);
-
-      q.select({ 'arr.$': 1 });
-      q.populate('arr.el');
-      selectPopulatedFields(q);
-      assert.deepEqual(q._fields, { 'arr.$': 1 });
+      const fields = { 'arr.$': 1 };
+      selectPopulatedFields(fields, Object.assign({}, fields), { 'arr.el': 1 });
+      assert.deepEqual(fields, { 'arr.$': 1 });
 
       done();
     });