fix: more durable handling for minus paths re: #11694

lib/model.js

@@ -4885,13 +4885,15 @@ function _execPopulateQuery(mod, match, select, assignmentOpts, callback) {
   } else if (queryOptions.limit != null) {
     queryOptions.limit = queryOptions.limit * mod.ids.length;
   }
+
   const query = mod.model.find(match, select, queryOptions);
   // If we're doing virtual populate and projection is inclusive and foreign
   // field is not selected, automatically select it because mongoose needs it.
   // If projection is exclusive and client explicitly unselected the foreign
   // field, that's the client's fault.
   for (const foreignField of mod.foreignField) {
-    if (foreignField !== '_id' && query.selectedInclusively() &&
+    if (foreignField !== '_id' &&
+        query.selectedInclusively() &&
         !isPathSelectedInclusive(query._fields, foreignField)) {
       query.select(foreignField);
     }

lib/query.js

@@ -1120,11 +1120,11 @@ Query.prototype.select = function select() {
   if (utils.isObject(arg)) {
     if (this.selectedInclusively()) {
       Object.entries(arg).forEach(([key, value]) => {
-        if (key[0] === '-') {
-          key = key.slice(1);
-        }
         if (value) {
           // Add the field to the projection
+          if (fields['-' + key] != null) {
+            delete fields['-' + key];
+          }
           fields[key] = userProvidedFields[key] = sanitizeValue(value);
         } else {
           // Remove the field from the projection
@@ -1138,11 +1138,11 @@ Query.prototype.select = function select() {
       });
     } else if (this.selectedExclusively()) {
       Object.entries(arg).forEach(([key, value]) => {
-        if (key[0] === '-') {
-          key = key.slice(1);
-        }
         if (!value) {
           // Add the field to the projection
+          if (fields['+' + key] != null) {
+            delete fields['+' + key];
+          }
           fields[key] = userProvidedFields[key] = sanitizeValue(value);
         } else {
           // Remove the field from the projection
@@ -1158,14 +1158,12 @@ Query.prototype.select = function select() {
       const keys = Object.keys(arg);
       for (let i = 0; i < keys.length; ++i) {
         const value = arg[keys[i]];
-        let key = keys[i];
-        if (key[0] === '-') {
-          key = key.slice(1);
-        }
+        const key = keys[i];
         fields[key] = sanitizeValue(value);
         userProvidedFields[key] = sanitizeValue(value);
       }
     }
+
     return this;
   }
 
@@ -2521,7 +2519,7 @@ Query.prototype._completeOne = function(doc, res, callback) {
  * @api private
  */
 
-Query.prototype._findOne = wrapThunk(function(callback) {
+Query.prototype._findOne = wrapThunk(function _findOne(callback) {
   this._castConditions();
 
   if (this.error()) {

lib/queryhelpers.js

@@ -148,27 +148,30 @@ exports.applyPaths = function applyPaths(fields, schema) {
   // determine if query is selecting or excluding fields
   let exclude;
   let keys;
+  const minusPathsToSkip = new Set();
 
   if (fields) {
     keys = Object.keys(fields);
 
     // Collapse minus paths
+    const minusPaths = [];
     for (let i = 0; i < keys.length; ++i) {
       const key = keys[i];
       if (keys[i][0] !== '-') {
         continue;
       }
-      const type = schema.path(key);
 
       delete fields[key];
-      if (!type || !type.selected) {
-        fields[key.slice(1)] = 0;
+      if (key === '-_id') {
+        fields['_id'] = 0;
+      } else {
+        minusPaths.push(key.slice(1));
       }
     }
 
     keys = Object.keys(fields);
     for (let keyIndex = 0; keyIndex < keys.length; ++keyIndex) {
-      if (keys[keyIndex][0] === '+' || keys[keyIndex][0] === '-') {
+      if (keys[keyIndex][0] === '+') {
         continue;
       }
       const field = fields[keys[keyIndex]];
@@ -184,10 +187,27 @@ exports.applyPaths = function applyPaths(fields, schema) {
       exclude = !field;
       break;
     }
+
+    // Potentially add back minus paths based on schema-level path config
+    // and whether the projection is inclusive
+    for (const path of minusPaths) {
+      const type = schema.path(path);
+      // If the path isn't selected by default or the projection is not
+      // inclusive, minus path is treated as equivalent to `key: 0`.
+      // But we also allow using `-name` to remove `name` from an inclusive
+      // projection if `name` has schema-level `select: true`.
+      if ((!type || !type.selected) || exclude !== false) {
+        fields[path] = 0;
+      } else if (type && type.selected && exclude === false) {
+        // Make a note of minus paths that are overwriting paths that are
+        // included by default.
+        minusPathsToSkip.add(path);
+      }
+    }
   }
+
   // if selecting, apply default schematype select:true fields
   // if excluding, apply schematype select:false fields
-
   const selected = [];
   const excluded = [];
   const stack = [];
@@ -208,6 +228,9 @@ exports.applyPaths = function applyPaths(fields, schema) {
       }
 
       for (const fieldName of selected) {
+        if (minusPathsToSkip.has(fieldName)) {
+          continue;
+        }
         fields[fieldName] = fields[fieldName] || 1;
       }
       break;

test/model.findOneAndDelete.test.js

@@ -232,10 +232,12 @@ describe('model: findOneAndDelete:', function() {
     let query;
 
     query = M.findByIdAndDelete(_id, { select: 'author -title' });
+    query._applyPaths();
     assert.strictEqual(1, query._fields.author);
     assert.strictEqual(0, query._fields.title);
 
     query = M.findOneAndDelete({}, { select: 'author -title' });
+    query._applyPaths();
     assert.strictEqual(1, query._fields.author);
     assert.strictEqual(0, query._fields.title);
   });

test/model.findOneAndRemove.test.js

@@ -239,10 +239,12 @@ describe('model: findOneAndRemove:', async function() {
     let query;
 
     query = M.findByIdAndRemove(_id, { select: 'author -title' });
+    query._applyPaths();
     assert.strictEqual(1, query._fields.author);
     assert.strictEqual(0, query._fields.title);
 
     query = M.findOneAndRemove({}, { select: 'author -title' });
+    query._applyPaths();
     assert.strictEqual(1, query._fields.author);
     assert.strictEqual(0, query._fields.title);
     done();

test/model.findOneAndReplace.test.js

@@ -226,6 +226,7 @@ describe('model: findOneAndReplace:', function() {
     const M = BlogPost;
 
     const query = M.findOneAndReplace({}, {}, { select: 'author -title' });
+    query._applyPaths();
     assert.strictEqual(1, query._fields.author);
     assert.strictEqual(0, query._fields.title);
   });

test/model.findOneAndUpdate.test.js

@@ -652,10 +652,12 @@ describe('model: findOneAndUpdate:', function() {
     let query;
 
     query = M.findByIdAndUpdate(_id, { $set: { date: now } }, { select: 'author -title' });
+    query._applyPaths();
     assert.strictEqual(1, query._fields.author);
     assert.strictEqual(0, query._fields.title);
 
     query = M.findOneAndUpdate({}, { $set: { date: now } }, { select: 'author -title' });
+    query._applyPaths();
     assert.strictEqual(1, query._fields.author);
     assert.strictEqual(0, query._fields.title);
   });