refactor: reuse sort logic between Query and Aggregate

lib/aggregate.js

@@ -9,7 +9,8 @@ const MongooseError = require('./error/mongooseError');
 const Query = require('./query');
 const { applyGlobalMaxTimeMS, applyGlobalDiskUse } = require('./helpers/query/applyGlobalOption');
 const clone = require('./helpers/clone');
-const getConstructorName = require('./helpers/getConstructorName');
+const castSort = require('./helpers/query/castSort');
+
 const prepareDiscriminatorPipeline = require('./helpers/aggregate/prepareDiscriminatorPipeline');
 const stringifyFunctionOperators = require('./helpers/aggregate/stringifyFunctionOperators');
 const utils = require('./utils');
@@ -642,34 +643,10 @@ Aggregate.prototype.sample = function(size) {
  */
 
 Aggregate.prototype.sort = function(arg) {
-  // TODO refactor to reuse the query builder logic
-
-  const sort = {};
-
-  if (getConstructorName(arg) === 'Object') {
-    const desc = ['desc', 'descending', -1];
-    Object.keys(arg).forEach(function(field) {
-      // If sorting by text score, skip coercing into 1/-1
-      if (arg[field] instanceof Object && arg[field].$meta) {
-        sort[field] = arg[field];
-        return;
-      }
-      sort[field] = desc.indexOf(arg[field]) === -1 ? 1 : -1;
-    });
-  } else if (arguments.length === 1 && typeof arg === 'string') {
-    arg.split(/\s+/).forEach(function(field) {
-      if (!field) {
-        return;
-      }
-      const ascend = field[0] === '-' ? -1 : 1;
-      if (ascend === -1) {
-        field = field.substring(1);
-      }
-      sort[field] = ascend;
-    });
-  } else {
+  if (arguments.length > 1) {
     throw new TypeError('Invalid sort() argument. Must be a string or object.');
   }
+  const sort = castSort(arg);
 
   return this.append({ $sort: sort });
 };

lib/helpers/query/castSort.js

@@ -0,0 +1,73 @@
+"use strict";
+
+const specialProperties = require('../specialProperties');
+
+/**
+ * Casts a sort argument to a MongoDB sort object.
+ *
+ * @param {Object|String|Array|Map} arg The sort argument.
+ * @return {Object} The cast sort object.
+ */
+module.exports = function castSort(arg) {
+  const sort = {};
+
+  if (typeof arg === 'string') {
+    const properties = arg.indexOf(' ') === -1 ? [arg] : arg.split(' ');
+    for (let property of properties) {
+      if (!property) {
+        continue;
+      }
+      const ascend = '-' === property[0] ? -1 : 1;
+      if (ascend === -1) {
+        property = property.slice(1);
+      }
+      if (specialProperties.has(property)) {
+        continue;
+      }
+      sort[property] = ascend;
+    }
+  } else if (Array.isArray(arg)) {
+    for (const pair of arg) {
+      if (!Array.isArray(pair)) {
+        throw new TypeError('Invalid sort() argument, must be array of arrays');
+      }
+      const key = '' + pair[0];
+      if (specialProperties.has(key)) {
+        continue;
+      }
+      sort[key] = _handleSortValue(pair[1], key);
+    }
+  } else if (typeof arg === 'object' && arg != null && !(arg instanceof Map)) {
+    for (const key of Object.keys(arg)) {
+      if (specialProperties.has(key)) {
+        continue;
+      }
+      sort[key] = _handleSortValue(arg[key], key);
+    }
+  } else if (arg instanceof Map) {
+    for (let key of arg.keys()) {
+      key = '' + key;
+      if (specialProperties.has(key)) {
+        continue;
+      }
+      sort[key] = _handleSortValue(arg.get(key), key);
+    }
+  } else if (arg != null) {
+    throw new TypeError('Invalid sort() argument. Must be a string, object, array, or map.');
+  }
+
+  return sort;
+};
+
+function _handleSortValue(val, key) {
+  if (val === 1 || val === 'asc' || val === 'ascending') {
+    return 1;
+  }
+  if (val === -1 || val === 'desc' || val === 'descending') {
+    return -1;
+  }
+  if (val && val.$meta != null) {
+    return { $meta: val.$meta };
+  }
+  throw new TypeError('Invalid sort value: { ' + key + ': ' + val + ' }');
+}

lib/query.js

@@ -18,6 +18,7 @@ const applyWriteConcern = require('./helpers/schema/applyWriteConcern');
 const cast = require('./cast');
 const castArrayFilters = require('./helpers/update/castArrayFilters');
 const castNumber = require('./cast/number');
+const castSort = require('./helpers/query/castSort');
 const castUpdate = require('./helpers/query/castUpdate');
 const clone = require('./helpers/clone');
 const getDiscriminatorByValue = require('./helpers/discriminator/getDiscriminatorByValue');
@@ -35,7 +36,7 @@ const sanitizeFilter = require('./helpers/query/sanitizeFilter');
 const sanitizeProjection = require('./helpers/query/sanitizeProjection');
 const selectPopulatedFields = require('./helpers/query/selectPopulatedFields');
 const setDefaultsOnInsert = require('./helpers/setDefaultsOnInsert');
-const specialProperties = require('./helpers/specialProperties');
+
 const updateValidators = require('./helpers/updateValidators');
 const util = require('util');
 const utils = require('./utils');
@@ -1349,24 +1350,24 @@ Query.prototype.read = function read(mode, tags) {
 
 Query.prototype.toString = function toString() {
   if (this.op === 'count' ||
-      this.op === 'countDocuments' ||
-      this.op === 'find' ||
-      this.op === 'findOne' ||
-      this.op === 'deleteMany' ||
-      this.op === 'deleteOne' ||
-      this.op === 'findOneAndDelete' ||
-      this.op === 'remove') {
+    this.op === 'countDocuments' ||
+    this.op === 'find' ||
+    this.op === 'findOne' ||
+    this.op === 'deleteMany' ||
+    this.op === 'deleteOne' ||
+    this.op === 'findOneAndDelete' ||
+    this.op === 'remove') {
     return `${this.model.modelName}.${this.op}(${util.inspect(this._conditions)})`;
   }
   if (this.op === 'distinct') {
     return `${this.model.modelName}.distinct('${this._distinct}', ${util.inspect(this._conditions)})`;
   }
   if (this.op === 'findOneAndReplace' ||
-      this.op === 'findOneAndUpdate' ||
-      this.op === 'replaceOne' ||
-      this.op === 'update' ||
-      this.op === 'updateMany' ||
-      this.op === 'updateOne') {
+    this.op === 'findOneAndUpdate' ||
+    this.op === 'replaceOne' ||
+    this.op === 'update' ||
+    this.op === 'updateMany' ||
+    this.op === 'updateOne') {
     return `${this.model.modelName}.${this.op}(${util.inspect(this._conditions)}, ${util.inspect(this._update)})`;
   }
 
@@ -2105,9 +2106,9 @@ Query.prototype._optionsForExec = function(model) {
   }
 
   const readPreference = model &&
-  model.schema &&
-  model.schema.options &&
-  model.schema.options.read;
+    model.schema &&
+    model.schema.options &&
+    model.schema.options.read;
   if (!('readPreference' in options) && readPreference) {
     options.readPreference = readPreference;
   }
@@ -2481,7 +2482,7 @@ Query.prototype._find = async function _find() {
 
 Query.prototype.find = function(conditions) {
   if (typeof conditions === 'function' ||
-      typeof arguments[1] === 'function') {
+    typeof arguments[1] === 'function') {
     throw new MongooseError('Query.prototype.find() no longer accepts a callback');
   }
 
@@ -2760,9 +2761,9 @@ Query.prototype._findOne = async function _findOne() {
 
 Query.prototype.findOne = function(conditions, projection, options) {
   if (typeof conditions === 'function' ||
-      typeof projection === 'function' ||
-      typeof options === 'function' ||
-      typeof arguments[3] === 'function') {
+    typeof projection === 'function' ||
+    typeof options === 'function' ||
+    typeof arguments[3] === 'function') {
     throw new MongooseError('Query.prototype.findOne() no longer accepts a callback');
   }
 
@@ -2888,7 +2889,7 @@ Query.prototype._estimatedDocumentCount = async function _estimatedDocumentCount
 
 Query.prototype.estimatedDocumentCount = function(options) {
   if (typeof options === 'function' ||
-      typeof arguments[1] === 'function') {
+    typeof arguments[1] === 'function') {
     throw new MongooseError('Query.prototype.estimatedDocumentCount() no longer accepts a callback');
   }
 
@@ -2941,8 +2942,8 @@ Query.prototype.estimatedDocumentCount = function(options) {
 
 Query.prototype.countDocuments = function(conditions, options) {
   if (typeof conditions === 'function' ||
-      typeof options === 'function' ||
-      typeof arguments[2] === 'function') {
+    typeof options === 'function' ||
+    typeof arguments[2] === 'function') {
     throw new MongooseError('Query.prototype.countDocuments() no longer accepts a callback');
   }
 
@@ -3005,9 +3006,9 @@ Query.prototype.__distinct = async function __distinct() {
 
 Query.prototype.distinct = function(field, conditions, options) {
   if (typeof field === 'function' ||
-      typeof conditions === 'function' ||
-      typeof options === 'function' ||
-      typeof arguments[3] === 'function') {
+    typeof conditions === 'function' ||
+    typeof options === 'function' ||
+    typeof arguments[3] === 'function') {
     throw new MongooseError('Query.prototype.distinct() no longer accepts a callback');
   }
 
@@ -3078,48 +3079,9 @@ Query.prototype.sort = function(arg, options) {
   if (options && options.override) {
     this.options.sort = {};
   }
-  const sort = this.options.sort;
-  if (typeof arg === 'string') {
-    const properties = arg.indexOf(' ') === -1 ? [arg] : arg.split(' ');
-    for (let property of properties) {
-      const ascend = '-' == property[0] ? -1 : 1;
-      if (ascend === -1) {
-        property = property.slice(1);
-      }
-      if (specialProperties.has(property)) {
-        continue;
-      }
-      sort[property] = ascend;
-    }
-  } else if (Array.isArray(arg)) {
-    for (const pair of arg) {
-      if (!Array.isArray(pair)) {
-        throw new TypeError('Invalid sort() argument, must be array of arrays');
-      }
-      const key = '' + pair[0];
-      if (specialProperties.has(key)) {
-        continue;
-      }
-      sort[key] = _handleSortValue(pair[1], key);
-    }
-  } else if (typeof arg === 'object' && arg != null && !(arg instanceof Map)) {
-    for (const key of Object.keys(arg)) {
-      if (specialProperties.has(key)) {
-        continue;
-      }
-      sort[key] = _handleSortValue(arg[key], key);
-    }
-  } else if (arg instanceof Map) {
-    for (let key of arg.keys()) {
-      key = '' + key;
-      if (specialProperties.has(key)) {
-        continue;
-      }
-      sort[key] = _handleSortValue(arg.get(key), key);
-    }
-  } else if (arg != null) {
-    throw new TypeError('Invalid sort() argument. Must be a string, object, array, or map.');
-  }
+
+  const sort = castSort(arg);
+  Object.assign(this.options.sort, sort);
 
   return this;
 };
@@ -3128,18 +3090,6 @@ Query.prototype.sort = function(arg, options) {
  * Convert sort values
  */
 
-function _handleSortValue(val, key) {
-  if (val === 1 || val === 'asc' || val === 'ascending') {
-    return 1;
-  }
-  if (val === -1 || val === 'desc' || val === 'descending') {
-    return -1;
-  }
-  if (val?.$meta != null) {
-    return { $meta: val.$meta };
-  }
-  throw new TypeError('Invalid sort value: { ' + key + ': ' + val + ' }');
-}
 
 /**
  * Declare and/or execute this query as a `deleteOne()` operation. Works like
@@ -3413,9 +3363,9 @@ function prepareDiscriminatorCriteria(query) {
 
 Query.prototype.findOneAndUpdate = function(filter, update, options) {
   if (typeof filter === 'function' ||
-      typeof update === 'function' ||
-      typeof options === 'function' ||
-      typeof arguments[3] === 'function') {
+    typeof update === 'function' ||
+    typeof options === 'function' ||
+    typeof arguments[3] === 'function') {
     throw new MongooseError('Query.prototype.findOneAndUpdate() no longer accepts a callback');
   }
 
@@ -3592,8 +3542,8 @@ Query.prototype._findOneAndUpdate = async function _findOneAndUpdate() {
 
 Query.prototype.findOneAndDelete = function(filter, options) {
   if (typeof filter === 'function' ||
-      typeof options === 'function' ||
-      typeof arguments[2] === 'function') {
+    typeof options === 'function' ||
+    typeof arguments[2] === 'function') {
     throw new MongooseError('Query.prototype.findOneAndDelete() no longer accepts a callback');
   }
 
@@ -3693,9 +3643,9 @@ Query.prototype._findOneAndDelete = async function _findOneAndDelete() {
 
 Query.prototype.findOneAndReplace = function(filter, replacement, options) {
   if (typeof filter === 'function' ||
-      typeof replacement === 'function' ||
-      typeof options === 'function' ||
-      typeof arguments[4] === 'function') {
+    typeof replacement === 'function' ||
+    typeof options === 'function' ||
+    typeof arguments[4] === 'function') {
     throw new MongooseError('Query.prototype.findOneAndReplace() no longer accepts a callback');
   }
 
@@ -4398,8 +4348,8 @@ function _update(query, op, filter, doc, options, callback) {
   }
 
   if (!(filter instanceof Query) &&
-      filter != null &&
-      filter.toString() !== '[object Object]') {
+    filter != null &&
+    filter.toString() !== '[object Object]') {
     query.error(new ObjectParameterError(filter, 'filter', op));
   } else {
     query.merge(filter);
@@ -4812,8 +4762,8 @@ Query.prototype._castUpdate = function _castUpdate(obj) {
   const discriminatorKey = schema.options.discriminatorKey;
   const baseSchema = schema._baseSchema ? schema._baseSchema : schema;
   if (this._mongooseOptions.overwriteDiscriminatorKey &&
-      obj[discriminatorKey] != null &&
-      baseSchema.discriminators) {
+    obj[discriminatorKey] != null &&
+    baseSchema.discriminators) {
     const _schema = Object.values(baseSchema.discriminators).find(
       discriminator => discriminator.discriminatorMapping.value === obj[discriminatorKey]
     );
@@ -4995,7 +4945,7 @@ Query.prototype.cast = function(model, obj) {
   model = model || this.model;
   const discriminatorKey = model.schema.options.discriminatorKey;
   if (obj != null &&
-      obj.hasOwnProperty(discriminatorKey)) {
+    obj.hasOwnProperty(discriminatorKey)) {
     model = getDiscriminatorByValue(model.discriminators, obj[discriminatorKey]) || model;
   }
 
@@ -5372,7 +5322,7 @@ Query.prototype.near = function() {
     }
   } else if (arguments.length === 3) {
     if (typeof arguments[0] === 'string' && typeof arguments[1] === 'number'
-        && typeof arguments[2] === 'number') {
+      && typeof arguments[2] === 'number') {
       params.push(arguments[0]);
       params.push({ center: [arguments[1], arguments[2]], spherical: sphere });
     } else {