From 2f4131dacf9096b94f24f211ec0ef2d5602bda5e Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Tue, 13 Oct 2020 13:07:19 -0400 Subject: [PATCH 1/8] force from master --- src/collection.ts | 3 +- src/cursor/aggregation_cursor.ts | 2 +- src/cursor/cursor.ts | 49 ++-------- src/gridfs-stream/download.ts | 3 +- src/gridfs-stream/index.ts | 3 +- src/index.ts | 3 +- src/operations/common_functions.ts | 31 +------ src/operations/find.ts | 20 +--- src/operations/find_and_modify.ts | 5 +- src/operations/map_reduce.ts | 2 +- src/sort.ts | 94 +++++++++++++++++++ src/utils.ts | 59 ------------ test/functional/cursor.test.js | 141 ++++++++++++++++++++--------- 13 files changed, 215 insertions(+), 200 deletions(-) create mode 100644 src/sort.ts 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 89f39170e4..7ad64ad6c1 100644 --- a/src/cursor/aggregation_cursor.ts +++ b/src/cursor/aggregation_cursor.ts @@ -3,7 +3,7 @@ import { Cursor, CursorOptions } from './cursor'; import { CursorState } from './core_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 c4b2996d42..5461784486 100644 --- a/src/cursor/cursor.ts +++ b/src/cursor/cursor.ts @@ -12,16 +12,16 @@ import { CursorCloseOptions, DocumentTransforms } from './core_cursor'; -import { maybePromise, formattedOrderClause, Callback } from '../utils'; +import { maybePromise, Callback } from '../utils'; import { executeOperation } from '../operations/execute_operation'; import { each, EachCallback } from '../operations/cursor_ops'; import { CountOperation, CountOptions } from '../operations/count'; import type { Logger } from '../logger'; import type { Topology } from '../sdam/topology'; import type { CollationOptions } from '../cmap/wire_protocol/write_command'; -import type { Sort, SortDirection } from '../operations/find'; import type { Hint, OperationBase } from '../operations/operation'; import type { Document } from '../bson'; +import { Sort, SortDirection, formatSort } from '../sort'; /** @public Flags allowed for cursor */ export const FLAGS = [ @@ -52,6 +52,7 @@ export interface CursorOptions extends CoreCursorOptions { topology?: Topology; /** Session to use for the operation */ numberOfRetries?: number; + sort?: Sort; } const kCursor = Symbol('cursor'); @@ -237,6 +238,10 @@ export class Cursor< this.addCursorFlag('noCursorTimeout', true); } + if (this.options.sort) { + this.cmd.sort = formatSort(this.options.sort); + } + // Set the batch size this.cursorBatchSize = batchSize; } @@ -303,14 +308,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); - } - } - this._next((err, doc) => { if (err) return cb(err); this.s.state = CursorState.OPEN; @@ -587,37 +584,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 79728c2137..bdfd9222fe 100644 --- a/src/index.ts +++ b/src/index.ts @@ -210,7 +210,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/common_functions.ts b/src/operations/common_functions.ts index a1253538b5..6eec418899 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -1,12 +1,5 @@ import { MongoError } from '../error'; -import { CursorState, Cursor } from '../cursor'; -import { - applyRetryableWrites, - applyWriteConcern, - decorateWithCollation, - formattedOrderClause, - Callback -} from '../utils'; +import { applyRetryableWrites, applyWriteConcern, decorateWithCollation, Callback } from '../utils'; import type { Document } from '../bson'; import type { Db } from '../db'; import type { ClientSession } from '../sessions'; @@ -106,28 +99,6 @@ export function prepareDocs( }); } -// Get the next available document from the cursor, returns null if no more documents are available. -export function nextObject(cursor: Cursor, callback: Callback): void { - if (cursor.s.state === CursorState.CLOSED || (cursor.isDead && cursor.isDead())) { - return callback(MongoError.create({ message: 'Cursor is closed', driver: true })); - } - - if (cursor.s.state === CursorState.INIT && cursor.cmd && cursor.cmd.sort) { - try { - cursor.cmd.sort = formattedOrderClause(cursor.cmd.sort); - } catch (err) { - return callback(err); - } - } - - // Get the next object - cursor._next((err, doc) => { - cursor.s.state = CursorState.OPEN; - if (err) return callback(err); - callback(undefined, doc); - }); -} - export function removeDocuments(server: Server, coll: Collection, callback?: Callback): void; export function removeDocuments( server: Server, diff --git a/src/operations/find.ts b/src/operations/find.ts index 55cf558c22..10bdef801f 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 { @@ -153,9 +140,8 @@ 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 || options.fields) { let projection = options.projection || options.fields; if (projection && !Buffer.isBuffer(projection) && Array.isArray(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..e596afbd0c --- /dev/null +++ b/src/sort.ts @@ -0,0 +1,94 @@ +/** @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 */ +class SortDigest { + static prepareDirection(direction: any = 1): SortDirectionForCmd { + const value = ('' + direction).toLowerCase(); + if (SortDigest.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)}`); + } + } + static isMeta(t: SortDirection): t is { $meta: string } { + return typeof t === 'object' && t !== null && '$meta' in t && typeof t.$meta === 'string'; + } + static isPair(t: Sort): t is [string, SortDirection] { + if (Array.isArray(t) && t.length === 2) { + try { + SortDigest.prepareDirection(t[1]); + return true; + } catch (e) { + return false; + } + } + return false; + } + static pairToObject(v: [string, SortDirection]): SortForCmd { + return { [v[0]]: SortDigest.prepareDirection(v[1]) }; + } + static isDeep(t: Sort): t is [string, SortDirection][] { + return Array.isArray(t) && Array.isArray(t[0]); + } + static deepToObject(t: [string, SortDirection][]): SortForCmd { + return t.reduce((acq, i) => { + return { ...acq, ...SortDigest.pairToObject(i) }; + }, {}); + } + static stringsToObject(t: string[]): SortForCmd { + return t.reduce((acq, key) => { + return { ...acq, [key]: 1 }; + }, {}); + } + static validate(t: { [key: string]: SortDirection }): SortForCmd { + return Object.keys(t).reduce((acq, key) => { + return { ...acq, [key]: SortDigest.prepareDirection(t[key]) }; + }, {}); + } + static prepare(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]: SortDigest.prepareDirection(direction) }; + if (SortDigest.isPair(sort)) return SortDigest.pairToObject(sort); + if (SortDigest.isDeep(sort)) return SortDigest.deepToObject(sort); + if (Array.isArray(sort)) return SortDigest.stringsToObject(sort); + if (typeof sort === 'object') return SortDigest.validate(sort); + throw new Error(`Invalid sort format: ${JSON.stringify(sort)}`); + } +} + +/** converts a Sort type into a type that is valid for the server (SortForCmd) */ +export const formatSort = SortDigest.prepare; 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 750c788555..39d68345fc 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,64 @@ 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', () => { + 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', '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' } + }); + }); + }); }); From 62644ef00de6aacf2adb967c9938cdb5cdf24e40 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 15 Oct 2020 12:24:23 -0400 Subject: [PATCH 2/8] multi-key test --- test/functional/cursor.test.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/functional/cursor.test.js b/test/functional/cursor.test.js index 39d68345fc..fcdbd048ca 100644 --- a/test/functional/cursor.test.js +++ b/test/functional/cursor.test.js @@ -4347,11 +4347,12 @@ describe('Cursor', function () { 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', () => { + 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 }); @@ -4364,5 +4365,14 @@ describe('Cursor', function () { 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 + }); + }); }); }); From 5a975244a8bd1e7cb2b62d6469f137405dae2642 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 15 Oct 2020 12:33:27 -0400 Subject: [PATCH 3/8] fix lint --- test/functional/cursor.test.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/functional/cursor.test.js b/test/functional/cursor.test.js index fcdbd048ca..d612ea0aa4 100644 --- a/test/functional/cursor.test.js +++ b/test/functional/cursor.test.js @@ -4369,9 +4369,15 @@ describe('Cursor', function () { 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', '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 + alpha: { $meta: 'hi' }, + beta: 1 }); }); }); From b832325d0bafe6744233de3c7f9c8a466c68d0cc Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 15 Oct 2020 13:53:21 -0400 Subject: [PATCH 4/8] get rid of class / static --- src/sort.ts | 136 +++++++++++++++++++++++++++++----------------------- 1 file changed, 75 insertions(+), 61 deletions(-) diff --git a/src/sort.ts b/src/sort.ts index e596afbd0c..aef0f0f8ea 100644 --- a/src/sort.ts +++ b/src/sort.ts @@ -25,70 +25,84 @@ type SortDirectionForCmd = 1 | -1 | { $meta: string }; type SortForCmd = { [key: string]: SortDirectionForCmd }; /** @internal */ -class SortDigest { - static prepareDirection(direction: any = 1): SortDirectionForCmd { - const value = ('' + direction).toLowerCase(); - if (SortDigest.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)}`); - } - } - static isMeta(t: SortDirection): t is { $meta: string } { - return typeof t === 'object' && t !== null && '$meta' in t && typeof t.$meta === 'string'; +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)}`); } - static isPair(t: Sort): t is [string, SortDirection] { - if (Array.isArray(t) && t.length === 2) { - try { - SortDigest.prepareDirection(t[1]); - return true; - } catch (e) { - return false; - } +} + +/** @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; - } - static pairToObject(v: [string, SortDirection]): SortForCmd { - return { [v[0]]: SortDigest.prepareDirection(v[1]) }; - } - static isDeep(t: Sort): t is [string, SortDirection][] { - return Array.isArray(t) && Array.isArray(t[0]); - } - static deepToObject(t: [string, SortDirection][]): SortForCmd { - return t.reduce((acq, i) => { - return { ...acq, ...SortDigest.pairToObject(i) }; - }, {}); - } - static stringsToObject(t: string[]): SortForCmd { - return t.reduce((acq, key) => { - return { ...acq, [key]: 1 }; - }, {}); - } - static validate(t: { [key: string]: SortDirection }): SortForCmd { - return Object.keys(t).reduce((acq, key) => { - return { ...acq, [key]: SortDigest.prepareDirection(t[key]) }; - }, {}); - } - static prepare(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]: SortDigest.prepareDirection(direction) }; - if (SortDigest.isPair(sort)) return SortDigest.pairToObject(sort); - if (SortDigest.isDeep(sort)) return SortDigest.deepToObject(sort); - if (Array.isArray(sort)) return SortDigest.stringsToObject(sort); - if (typeof sort === 'object') return SortDigest.validate(sort); - throw new Error(`Invalid sort format: ${JSON.stringify(sort)}`); } + 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 { + return t.reduce((acq, i) => { + return { ...acq, ...pairToObject(i) }; + }, {}); +} + +/** @internal */ +function stringsToObject(t: string[]): SortForCmd { + return t.reduce((acq, key) => { + return { ...acq, [key]: 1 }; + }, {}); +} + +/** @internal */ +function validate(t: { [key: string]: SortDirection }): SortForCmd { + return Object.keys(t).reduce((acq, key) => { + return { ...acq, [key]: prepareDirection(t[key]) }; + }, {}); } /** converts a Sort type into a type that is valid for the server (SortForCmd) */ -export const formatSort = SortDigest.prepare; +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 validate(sort); + throw new Error(`Invalid sort format: ${JSON.stringify(sort)}`); +} From d0689270ad35bad36bb52d2fd0620417395f1c6e Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 15 Oct 2020 13:59:13 -0400 Subject: [PATCH 5/8] neals simpler deep to object --- src/sort.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/sort.ts b/src/sort.ts index aef0f0f8ea..2b9870325d 100644 --- a/src/sort.ts +++ b/src/sort.ts @@ -72,9 +72,11 @@ function isDeep(t: Sort): t is [string, SortDirection][] { /** @internal */ function deepToObject(t: [string, SortDirection][]): SortForCmd { - return t.reduce((acq, i) => { - return { ...acq, ...pairToObject(i) }; - }, {}); + const sortObject: SortForCmd = {}; + for (const [name, value] of t) { + sortObject[name] = prepareDirection(value); + } + return sortObject; } /** @internal */ From 76ce5559bc3360deb14d5388e3ed991feec78e5b Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Thu, 15 Oct 2020 15:56:03 -0400 Subject: [PATCH 6/8] removed all reducers --- src/sort.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/sort.ts b/src/sort.ts index 2b9870325d..c742cc8ceb 100644 --- a/src/sort.ts +++ b/src/sort.ts @@ -81,16 +81,20 @@ function deepToObject(t: [string, SortDirection][]): SortForCmd { /** @internal */ function stringsToObject(t: string[]): SortForCmd { - return t.reduce((acq, key) => { - return { ...acq, [key]: 1 }; - }, {}); + const sortObject: SortForCmd = {}; + for (const key of t) { + sortObject[key] = 1; + } + return sortObject; } /** @internal */ -function validate(t: { [key: string]: SortDirection }): SortForCmd { - return Object.keys(t).reduce((acq, key) => { - return { ...acq, [key]: prepareDirection(t[key]) }; - }, {}); +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) */ @@ -105,6 +109,6 @@ export function formatSort( if (isPair(sort)) return pairToObject(sort); if (isDeep(sort)) return deepToObject(sort); if (Array.isArray(sort)) return stringsToObject(sort); - if (typeof sort === 'object') return validate(sort); + if (typeof sort === 'object') return objectToObject(sort); throw new Error(`Invalid sort format: ${JSON.stringify(sort)}`); } From 4bf2892798e913ca1202064de219d8a2655ec504 Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Fri, 16 Oct 2020 10:44:58 -0400 Subject: [PATCH 7/8] linter issue --- src/operations/find.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/operations/find.ts b/src/operations/find.ts index 8a1b061445..6d611b98ae 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -127,7 +127,7 @@ export class FindOperation extends CommandOperation { if (options.sort) { findCommand.sort = formatSort(options.sort); } - + if (options.projection) { let projection = options.projection; if (projection && !Buffer.isBuffer(projection) && Array.isArray(projection)) { From d558e7b399ada3463a78119037c32eba324abccd Mon Sep 17 00:00:00 2001 From: Thomas Reggi Date: Mon, 19 Oct 2020 10:58:25 -0400 Subject: [PATCH 8/8] reduce document import --- src/cursor/cursor.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cursor/cursor.ts b/src/cursor/cursor.ts index e0176b4960..71cf35b9a7 100644 --- a/src/cursor/cursor.ts +++ b/src/cursor/cursor.ts @@ -1,7 +1,7 @@ import { EventEmitter } from 'events'; import { Readable } from 'stream'; import { deprecate } from 'util'; -import { Long, BSONSerializeOptions } from '../bson'; +import { Long, Document, BSONSerializeOptions } from '../bson'; import { MongoError, MongoNetworkError, AnyError } from '../error'; import { Logger } from '../logger'; import { executeOperation } from '../operations/execute_operation'; @@ -13,7 +13,6 @@ 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 { Document } from '../bson'; import type { Topology } from '../sdam/topology'; import type { CommandOperationOptions } from '../operations/command'; import type { ReadConcern } from '../read_concern';