diff --git a/src/collection.ts b/src/collection.ts index 7dacd0e07e..01459bd077 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -41,7 +41,7 @@ import { EstimatedDocumentCountOperation, EstimatedDocumentCountOptions } from './operations/estimated_document_count'; -import { FindOperation, FindOptions, Sort } from './operations/find'; +import { FindOperation, FindOptions } from './operations/find'; import { FindOneOperation } from './operations/find_one'; import { FindAndModifyOperation, @@ -86,6 +86,7 @@ import type { PkFactory } from './mongo_client'; import type { Topology } from './sdam/topology'; import type { Logger, LoggerOptions } from './logger'; import type { OperationParent } from './operations/command'; +import type { Sort } from './sort'; /** @public */ export interface Collection { diff --git a/src/cursor/aggregation_cursor.ts b/src/cursor/aggregation_cursor.ts index 41b02c986e..3ed734c5ca 100644 --- a/src/cursor/aggregation_cursor.ts +++ b/src/cursor/aggregation_cursor.ts @@ -2,7 +2,7 @@ import { MongoError } from '../error'; import { Cursor, CursorOptions, CursorState } from './cursor'; import type { AggregateOperation, AggregateOptions } from '../operations/aggregate'; import type { Document } from '../bson'; -import type { Sort } from '../operations/find'; +import type { Sort } from '../sort'; import type { Topology } from '../sdam/topology'; /** @public */ diff --git a/src/cursor/cursor.ts b/src/cursor/cursor.ts index a393b4e70b..71cf35b9a7 100644 --- a/src/cursor/cursor.ts +++ b/src/cursor/cursor.ts @@ -7,21 +7,14 @@ import { Logger } from '../logger'; import { executeOperation } from '../operations/execute_operation'; import { CountOperation, CountOptions } from '../operations/count'; import { ReadPreference, ReadPreferenceLike } from '../read_preference'; -import { - Callback, - emitDeprecatedOptionWarning, - formattedOrderClause, - maybePromise, - MongoDBNamespace -} from '../utils'; - +import { Callback, emitDeprecatedOptionWarning, maybePromise, MongoDBNamespace } from '../utils'; +import { Sort, SortDirection, formatSort } from '../sort'; import type { OperationTime, ResumeToken } from '../change_stream'; import type { CloseOptions } from '../cmap/connection_pool'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; +import type { Hint, OperationBase } from '../operations/operation'; import type { Topology } from '../sdam/topology'; import type { CommandOperationOptions } from '../operations/command'; -import type { Sort, SortDirection } from '../operations/find'; -import type { Hint, OperationBase } from '../operations/operation'; import type { ReadConcern } from '../read_concern'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; @@ -99,6 +92,7 @@ export interface CursorOptions extends CommandOperationOptions { topology?: Topology; /** Session to use for the operation */ numberOfRetries?: number; + sort?: Sort; } /** @public */ @@ -399,6 +393,10 @@ export class Cursor< this.addCursorFlag('noCursorTimeout', true); } + if (this.options.sort) { + this.cmd.sort = formatSort(this.options.sort); + } + // Set the batch size this._batchSize = batchSize; } @@ -688,14 +686,6 @@ export class Cursor< return; } - if (this.s.state === CursorState.INIT && this.cmd.sort) { - try { - this.cmd.sort = formattedOrderClause(this.cmd.sort); - } catch (err) { - return cb(err); - } - } - nextFunction(this, (err, doc) => { if (err) return cb(err); this.s.state = CursorState.OPEN; @@ -940,37 +930,7 @@ export class Cursor< throw new MongoError('Cursor is closed'); } - let order = sort; - - // We have an array of arrays, we need to preserve the order of the sort - // so we will us a Map - if (Array.isArray(order) && Array.isArray(order[0])) { - this.cmd.sort = new Map( - (order as [string, SortDirection][]).map(([key, dir]) => { - if (dir === 'asc') { - return [key, 1]; - } else if (dir === 'desc') { - return [key, -1]; - } else if (dir === 1 || dir === -1 || dir.$meta) { - return [key, dir]; - } else { - throw new MongoError( - "Illegal sort clause, must be of the form [['field1', '(ascending|descending)'], ['field2', '(ascending|descending)']]" - ); - } - - return [key, null]; - }) - ); - - return this; - } - - if (direction != null) { - order = [[sort as string, direction]]; - } - - this.cmd.sort = order; + this.cmd.sort = formatSort(sort, direction); return this; } diff --git a/src/gridfs-stream/download.ts b/src/gridfs-stream/download.ts index 9693c080ba..a98499e3a6 100644 --- a/src/gridfs-stream/download.ts +++ b/src/gridfs-stream/download.ts @@ -1,7 +1,8 @@ import { Readable } from 'stream'; import type { AnyError } from '../error'; import type { Document } from '../bson'; -import type { FindOptions, Sort } from '../operations/find'; +import type { FindOptions } from '../operations/find'; +import type { Sort } from '../sort'; import type { Cursor } from './../cursor/cursor'; import type { Callback } from '../utils'; import type { Collection } from '../collection'; diff --git a/src/gridfs-stream/index.ts b/src/gridfs-stream/index.ts index 9af78aa3e7..d23fe6a6f7 100644 --- a/src/gridfs-stream/index.ts +++ b/src/gridfs-stream/index.ts @@ -13,7 +13,8 @@ import type { Db } from '../db'; import type { ReadPreference } from '../read_preference'; import type { Collection } from '../collection'; import type { Cursor } from './../cursor/cursor'; -import type { FindOptions, Sort } from './../operations/find'; +import type { FindOptions } from './../operations/find'; +import type { Sort } from '../sort'; import type { Logger } from '../logger'; const DEFAULT_GRIDFS_BUCKET_OPTIONS: { diff --git a/src/index.ts b/src/index.ts index 9199d90ea9..b2d2a1e510 100644 --- a/src/index.ts +++ b/src/index.ts @@ -204,7 +204,8 @@ export type { DistinctOptions } from './operations/distinct'; export type { DropCollectionOptions, DropDatabaseOptions } from './operations/drop'; export type { EstimatedDocumentCountOptions } from './operations/estimated_document_count'; export type { EvalOptions } from './operations/eval'; -export type { FindOptions, Sort, SortDirection } from './operations/find'; +export type { FindOptions } from './operations/find'; +export type { Sort, SortDirection } from './sort'; export type { FindAndModifyOptions } from './operations/find_and_modify'; export type { IndexSpecification, diff --git a/src/operations/find.ts b/src/operations/find.ts index 9344983128..6d611b98ae 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -1,12 +1,6 @@ import { Aspect, defineAspects, Hint } from './operation'; import { ReadPreference } from '../read_preference'; -import { - maxWireVersion, - MongoDBNamespace, - Callback, - formattedOrderClause, - normalizeHintField -} from '../utils'; +import { maxWireVersion, MongoDBNamespace, Callback, normalizeHintField } from '../utils'; import { MongoError } from '../error'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; @@ -14,14 +8,7 @@ import type { Collection } from '../collection'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; import type { QueryOptions } from '../cmap/wire_protocol/query'; import { CommandOperation, CommandOperationOptions } from './command'; - -/** @public */ -export type SortDirection = 1 | -1 | 'asc' | 'desc' | { $meta: string }; -/** @public */ -export type Sort = - | { [key: string]: SortDirection } - | [string, SortDirection][] - | [string, SortDirection]; +import { Sort, formatSort } from '../sort'; /** @public */ export interface FindOptions extends QueryOptions, CommandOperationOptions { @@ -138,7 +125,7 @@ export class FindOperation extends CommandOperation { const findCommand: Document = Object.assign({}, this.cmd); if (options.sort) { - findCommand.sort = formattedOrderClause(options.sort); + findCommand.sort = formatSort(options.sort); } if (options.projection) { diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 748fd1d8af..a9b03022b0 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -4,7 +4,6 @@ import { applyRetryableWrites, decorateWithCollation, applyWriteConcern, - formattedOrderClause, hasAtomicOperators, Callback } from '../utils'; @@ -14,7 +13,7 @@ import { defineAspects, Aspect } from './operation'; import type { Document } from '../bson'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import type { Sort } from './find'; +import { Sort, formatSort } from '../sort'; /** @public */ export interface FindAndModifyOptions extends CommandOperationOptions { @@ -69,7 +68,7 @@ export class FindAndModifyOperation extends CommandOperation): void { const coll = this.collection; const query = this.query; - const sort = formattedOrderClause(this.sort); + const sort = formatSort(this.sort); const doc = this.doc; let options = this.options; diff --git a/src/operations/map_reduce.ts b/src/operations/map_reduce.ts index 6c243112bf..fe703fd8ae 100644 --- a/src/operations/map_reduce.ts +++ b/src/operations/map_reduce.ts @@ -11,7 +11,7 @@ import { ReadPreference, ReadPreferenceMode } from '../read_preference'; import { CommandOperation, CommandOperationOptions } from './command'; import type { Server } from '../sdam/server'; import type { Collection } from '../collection'; -import type { Sort } from './find'; +import type { Sort } from '../sort'; import { MongoError } from '../error'; import type { ObjectId } from '../bson'; diff --git a/src/sort.ts b/src/sort.ts new file mode 100644 index 0000000000..c742cc8ceb --- /dev/null +++ b/src/sort.ts @@ -0,0 +1,114 @@ +/** @public */ +export type SortDirection = + | 1 + | -1 + | 'asc' + | 'desc' + | 'ascending' + | 'descending' + | { $meta: string }; + +/** @public */ +export type Sort = + | string + | string[] + | { [key: string]: SortDirection } + | [string, SortDirection][] + | [string, SortDirection]; + +/** Below stricter types were created for sort that correspond with type that the cmd takes */ + +/** @internal */ +type SortDirectionForCmd = 1 | -1 | { $meta: string }; + +/** @internal */ +type SortForCmd = { [key: string]: SortDirectionForCmd }; + +/** @internal */ +function prepareDirection(direction: any = 1): SortDirectionForCmd { + const value = ('' + direction).toLowerCase(); + if (isMeta(direction)) return direction; + switch (value) { + case 'ascending': + case 'asc': + case '1': + return 1; + case 'descending': + case 'desc': + case '-1': + return -1; + default: + throw new Error(`Invalid sort direction: ${JSON.stringify(direction)}`); + } +} + +/** @internal */ +function isMeta(t: SortDirection): t is { $meta: string } { + return typeof t === 'object' && t !== null && '$meta' in t && typeof t.$meta === 'string'; +} + +/** @internal */ +function isPair(t: Sort): t is [string, SortDirection] { + if (Array.isArray(t) && t.length === 2) { + try { + prepareDirection(t[1]); + return true; + } catch (e) { + return false; + } + } + return false; +} + +/** @internal */ +function pairToObject(v: [string, SortDirection]): SortForCmd { + return { [v[0]]: prepareDirection(v[1]) }; +} + +/** @internal */ +function isDeep(t: Sort): t is [string, SortDirection][] { + return Array.isArray(t) && Array.isArray(t[0]); +} + +/** @internal */ +function deepToObject(t: [string, SortDirection][]): SortForCmd { + const sortObject: SortForCmd = {}; + for (const [name, value] of t) { + sortObject[name] = prepareDirection(value); + } + return sortObject; +} + +/** @internal */ +function stringsToObject(t: string[]): SortForCmd { + const sortObject: SortForCmd = {}; + for (const key of t) { + sortObject[key] = 1; + } + return sortObject; +} + +/** @internal */ +function objectToObject(t: { [key: string]: SortDirection }): SortForCmd { + const sortObject: SortForCmd = {}; + for (const key in t) { + sortObject[key] = prepareDirection(t[key]); + } + return sortObject; +} + +/** converts a Sort type into a type that is valid for the server (SortForCmd) */ +export function formatSort( + sort: Sort | undefined, + direction?: SortDirection +): SortForCmd | undefined { + if (sort == null) return undefined; + if (Array.isArray(sort) && !sort.length) return undefined; + if (typeof sort === 'object' && !Object.keys(sort).length) return undefined; + if (typeof sort === 'string') return { [sort]: prepareDirection(direction) }; + if (isPair(sort)) return pairToObject(sort); + if (isDeep(sort)) return deepToObject(sort); + if (Array.isArray(sort)) return stringsToObject(sort); + if (typeof sort === 'object') return objectToObject(sort); + throw new Error(`Invalid sort format: ${JSON.stringify(sort)}`); +} diff --git a/src/utils.ts b/src/utils.ts index 8275ed8d13..f200ce6846 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -12,7 +12,6 @@ import type { OperationOptions, OperationBase, Hint } from './operations/operati import type { ClientSession } from './sessions'; import type { ReadConcern } from './read_concern'; import type { Connection } from './cmap/connection'; -import type { SortDirection, Sort } from './operations/find'; import { readFileSync } from 'fs'; import { resolve } from 'path'; import type { Document } from './bson'; @@ -44,64 +43,6 @@ export function getSingleProperty( }); } -/** - * Translate the variety of sort specifiers into 1 or -1 - * @internal - */ -export function formatSortValue(sortDirection: SortDirection): -1 | 1 { - const value = ('' + sortDirection).toLowerCase(); - - switch (value) { - case 'ascending': - case 'asc': - case '1': - return 1; - case 'descending': - case 'desc': - case '-1': - return -1; - default: - throw new Error( - 'Illegal sort clause, must be of the form ' + - "[['field1', '(ascending|descending)'], " + - "['field2', '(ascending|descending)']]" - ); - } -} - -/** - * Ensure the sort specifier is in a shape we expect, and maps keys to 1 or -1. - * @internal - */ -export function formattedOrderClause(sortValue?: unknown): Sort | null { - let orderBy: any = {}; - if (sortValue == null) return null; - if (Array.isArray(sortValue)) { - if (sortValue.length === 0) { - return null; - } - - for (let i = 0; i < sortValue.length; i++) { - if (sortValue[i].constructor === String) { - orderBy[sortValue[i]] = 1; - } else { - orderBy[sortValue[i][0]] = formatSortValue(sortValue[i][1]); - } - } - } else if (sortValue != null && typeof sortValue === 'object') { - orderBy = sortValue; - } else if (typeof sortValue === 'string') { - orderBy[sortValue] = 1; - } else { - throw new Error( - 'Illegal sort clause, must be of the form ' + - "[['field1', '(ascending|descending)'], ['field2', '(ascending|descending)']]" - ); - } - - return orderBy; -} - /** * Throws if collectionName is not a valid mongodb collection namespace. * @internal diff --git a/test/functional/cursor.test.js b/test/functional/cursor.test.js index b71bacf265..2e45145bd0 100644 --- a/test/functional/cursor.test.js +++ b/test/functional/cursor.test.js @@ -1,5 +1,5 @@ 'use strict'; -const { assert: test, filterForCommands } = require('./shared'); +const { assert: test, filterForCommands, withMonitoredClient } = require('./shared'); const { setupDatabase } = require('./shared'); const fs = require('fs'); const { expect } = require('chai'); @@ -8,6 +8,7 @@ const sinon = require('sinon'); const { Writable } = require('stream'); const { ReadPreference } = require('../../src/read_preference'); const { ServerType } = require('../../src/sdam/common'); +const { formatSort } = require('../../src/sort'); describe('Cursor', function () { before(function () { @@ -405,74 +406,66 @@ describe('Cursor', function () { } function f() { - var number_of_functions = 9; - var finished = function () { + var number_of_functions = 7; + var finished = function (cursor) { number_of_functions = number_of_functions - 1; if (number_of_functions === 0) { - done(); + cursor.close(done); + } else { + cursor.close(); } }; var cursor = collection.find().sort(['a', 1]); - test.deepEqual(['a', 1], cursor.sortValue); - finished(); + test.deepEqual({ a: 1 }, cursor.sortValue); + finished(cursor); cursor = collection.find().sort('a', 1); - test.deepEqual([['a', 1]], cursor.sortValue); - finished(); + test.deepEqual({ a: 1 }, cursor.sortValue); + finished(cursor); cursor = collection.find().sort('a', -1); - test.deepEqual([['a', -1]], cursor.sortValue); - finished(); + test.deepEqual({ a: -1 }, cursor.sortValue); + finished(cursor); cursor = collection.find().sort('a', 'asc'); - test.deepEqual([['a', 'asc']], cursor.sortValue); - finished(); - - cursor = collection.find().sort([ - ['a', -1], - ['b', 1] - ]); - var entries = cursor.sortValue.entries(); - test.deepEqual(['a', -1], entries.next().value); - test.deepEqual(['b', 1], entries.next().value); - finished(); + test.deepEqual({ a: 1 }, cursor.sortValue); + finished(cursor); cursor = collection.find().sort('a', 1).sort('a', -1); - test.deepEqual([['a', -1]], cursor.sortValue); - finished(); + test.deepEqual({ a: -1 }, cursor.sortValue); + finished(cursor); + cursor = collection.find(); cursor.next(err => { expect(err).to.not.exist; try { cursor.sort(['a']); } catch (err) { test.equal('Cursor is closed', err.message); - finished(); + finished(cursor); } }); - collection - .find() - .sort('a', 25) - .next(err => { - test.equal( - "Illegal sort clause, must be of the form [['field1', '(ascending|descending)'], ['field2', '(ascending|descending)']]", - err.message - ); - finished(); - }); + cursor = collection.find(); + try { + cursor.sort('a', 25); + } catch (err) { + test.equal('Invalid sort direction: 25', err.message); + } + cursor.next(() => { + finished(cursor); + }); - collection - .find() - .sort(25) - .next(err => { - test.equal( - "Illegal sort clause, must be of the form [['field1', '(ascending|descending)'], ['field2', '(ascending|descending)']]", - err.message - ); - finished(); - }); + cursor = collection.find(); + try { + cursor.sort(25); + } catch (err) { + test.equal('Invalid sort format: 25', err.message); + } + cursor.next(() => { + finished(cursor); + }); } insert(function () { @@ -4312,4 +4305,80 @@ describe('Cursor', function () { }); }); }); + + context('sort', function () { + const findSort = (input, output) => + withMonitoredClient('find', function (client, events, done) { + const db = client.db('test'); + db.collection('test_sort_dos', (err, collection) => { + expect(err).to.not.exist; + const cursor = collection.find({}, { sort: input }); + expect(cursor.sortValue).to.deep.equal(output); + cursor.next(err => { + expect(err).to.not.exist; + expect(events[0].command.sort).to.deep.equal(output); + cursor.close(done); + }); + }); + }); + + const cursorSort = (input, output) => + withMonitoredClient('find', function (client, events, done) { + const db = client.db('test'); + db.collection('test_sort_dos', (err, collection) => { + expect(err).to.not.exist; + const cursor = collection.find({}).sort(input); + expect(cursor.sortValue).to.deep.equal(output); + cursor.next(err => { + expect(err).to.not.exist; + expect(events[0].command.sort).to.deep.equal(output); + cursor.close(done); + }); + }); + }); + + it('should use find options object', findSort({ alpha: 1 }, { alpha: 1 })); + it('should use find options string', findSort('alpha', { alpha: 1 })); + it('should use find options shallow array', findSort(['alpha', 1], { alpha: 1 })); + it('should use find options deep array', findSort([['alpha', 1]], { alpha: 1 })); + + it('should use cursor.sort object', cursorSort({ alpha: 1 }, { alpha: 1 })); + it('should use cursor.sort string', cursorSort('alpha', { alpha: 1 })); + it('should use cursor.sort shallow array', cursorSort(['alpha', 1], { alpha: 1 })); + it('should use cursor.sort deep array', cursorSort([['alpha', 1]], { alpha: 1 })); + + it('formatSort - one key', () => { + expect(formatSort('alpha')).to.deep.equal({ alpha: 1 }); + expect(formatSort(['alpha'])).to.deep.equal({ alpha: 1 }); + expect(formatSort('alpha', 1)).to.deep.equal({ alpha: 1 }); + expect(formatSort('alpha', 'asc')).to.deep.equal({ alpha: 1 }); + expect(formatSort([['alpha', 'asc']])).to.deep.equal({ alpha: 1 }); + expect(formatSort('alpha', 'ascending')).to.deep.equal({ alpha: 1 }); + expect(formatSort({ alpha: 1 })).to.deep.equal({ alpha: 1 }); + expect(formatSort('beta')).to.deep.equal({ beta: 1 }); + expect(formatSort(['beta'])).to.deep.equal({ beta: 1 }); + expect(formatSort('beta', -1)).to.deep.equal({ beta: -1 }); + expect(formatSort('beta', 'desc')).to.deep.equal({ beta: -1 }); + expect(formatSort('beta', 'descending')).to.deep.equal({ beta: -1 }); + expect(formatSort({ beta: -1 })).to.deep.equal({ beta: -1 }); + expect(formatSort({ alpha: { $meta: 'hi' } })).to.deep.equal({ + alpha: { $meta: 'hi' } + }); + }); + + it('formatSort - multi key', () => { + expect(formatSort(['alpha', 'beta'])).to.deep.equal({ alpha: 1, beta: 1 }); + expect(formatSort({ alpha: 1, beta: 1 })).to.deep.equal({ alpha: 1, beta: 1 }); + expect( + formatSort([ + ['alpha', 'asc'], + ['beta', 'ascending'] + ]) + ).to.deep.equal({ alpha: 1, beta: 1 }); + expect(formatSort({ alpha: { $meta: 'hi' }, beta: 'ascending' })).to.deep.equal({ + alpha: { $meta: 'hi' }, + beta: 1 + }); + }); + }); });