diff --git a/lib/aggregate.js b/lib/aggregate.js index 560c9e228c..bcf78b433b 100644 --- a/lib/aggregate.js +++ b/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 }); }; diff --git a/lib/helpers/query/castSort.js b/lib/helpers/query/castSort.js new file mode 100644 index 0000000000..21184ee73a --- /dev/null +++ b/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 + ' }'); +} \ No newline at end of file diff --git a/lib/query.js b/lib/query.js index 57bc291340..7d6f72f510 100644 --- a/lib/query.js +++ b/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 { diff --git a/test/aggregate.test.js b/test/aggregate.test.js index 91d427567b..837eda2ce0 100644 --- a/test/aggregate.test.js +++ b/test/aggregate.test.js @@ -281,6 +281,19 @@ describe('aggregate: ', function() { aggregate.sort(['a', 'b', 'c']); }, /Invalid sort/); }); + + it('should support Maps for sort', function() { + const aggregate = new Aggregate(); + const map = new Map([['name', 'asc'], ['age', -1]]); + aggregate.sort(map); + assert.deepEqual(aggregate._pipeline, [{ $sort: { name: 1, age: -1 } }]); + }); + + it('should support array of arrays for sort', function() { + const aggregate = new Aggregate(); + aggregate.sort([['name', 'asc'], ['age', -1]]); + assert.deepEqual(aggregate._pipeline, [{ $sort: { name: 1, age: -1 } }]); + }); }); describe('near', function() { @@ -398,11 +411,11 @@ describe('aggregate: ', function() { const aggregate = new Aggregate(); const obj = { output: - { - bootsSold: { value: 0 }, - sandalsSold: { value: 0 }, - sneakersSold: { value: 0 } - } + { + bootsSold: { value: 0 }, + sandalsSold: { value: 0 }, + sneakersSold: { value: 0 } + } }; aggregate.fill(obj); @@ -493,15 +506,15 @@ describe('aggregate: ', function() { describe('addFields', function() { it('should throw if passed a non object', function() { const aggregate = new Aggregate(); - assert.throws(() => {aggregate.addFields('invalid');}, /Invalid addFields\(\) argument\. Must be an object/); + assert.throws(() => { aggregate.addFields('invalid'); }, /Invalid addFields\(\) argument\. Must be an object/); }); it('should throw if passed null', function() { const aggregate = new Aggregate(); - assert.throws(() => {aggregate.addFields(null);}, /Invalid addFields\(\) argument\. Must be an object/); + assert.throws(() => { aggregate.addFields(null); }, /Invalid addFields\(\) argument\. Must be an object/); }); it('should throw if passed an Array', function() { const aggregate = new Aggregate(); - assert.throws(() => {aggregate.addFields([]);}, /Invalid addFields\(\) argument\. Must be an object/); + assert.throws(() => { aggregate.addFields([]); }, /Invalid addFields\(\) argument\. Must be an object/); }); it('(object)', function() { const aggregate = new Aggregate();