diff --git a/src/sort.ts b/src/sort.ts index c742cc8ceb9..3820ff196f4 100644 --- a/src/sort.ts +++ b/src/sort.ts @@ -13,6 +13,7 @@ export type Sort = | string | string[] | { [key: string]: SortDirection } + | Map | [string, SortDirection][] | [string, SortDirection]; @@ -22,7 +23,10 @@ export type Sort = type SortDirectionForCmd = 1 | -1 | { $meta: string }; /** @internal */ -type SortForCmd = { [key: string]: SortDirectionForCmd }; +type SortForCmd = Map; + +/** @internal */ +type SortPairForCmd = [string, SortDirectionForCmd]; /** @internal */ function prepareDirection(direction: any = 1): SortDirectionForCmd { @@ -60,41 +64,47 @@ function isPair(t: Sort): t is [string, SortDirection] { return false; } +function isDeep(t: Sort): t is [string, SortDirection][] { + return Array.isArray(t) && Array.isArray(t[0]); +} + +function isMap(t: Sort): t is Map { + return t instanceof Map && t.size > 0; +} + /** @internal */ -function pairToObject(v: [string, SortDirection]): SortForCmd { - return { [v[0]]: prepareDirection(v[1]) }; +function pairToMap(v: [string, SortDirection]): SortForCmd { + return new Map([[`${v[0]}`, prepareDirection([v[1]])]]); } /** @internal */ -function isDeep(t: Sort): t is [string, SortDirection][] { - return Array.isArray(t) && Array.isArray(t[0]); +function deepToMap(t: [string, SortDirection][]): SortForCmd { + const sortEntries: SortPairForCmd[] = t.map(([k, v]) => [`${k}`, prepareDirection(v)]); + return new Map(sortEntries); } /** @internal */ -function deepToObject(t: [string, SortDirection][]): SortForCmd { - const sortObject: SortForCmd = {}; - for (const [name, value] of t) { - sortObject[name] = prepareDirection(value); - } - return sortObject; +function stringsToMap(t: string[]): SortForCmd { + const sortEntries: SortPairForCmd[] = t.map(key => [`${key}`, 1]); + return new Map(sortEntries); } /** @internal */ -function stringsToObject(t: string[]): SortForCmd { - const sortObject: SortForCmd = {}; - for (const key of t) { - sortObject[key] = 1; - } - return sortObject; +function objectToMap(t: { [key: string]: SortDirection }): SortForCmd { + const sortEntries: SortPairForCmd[] = Object.entries(t).map(([k, v]) => [ + `${k}`, + prepareDirection(v) + ]); + return new Map(sortEntries); } /** @internal */ -function objectToObject(t: { [key: string]: SortDirection }): SortForCmd { - const sortObject: SortForCmd = {}; - for (const key in t) { - sortObject[key] = prepareDirection(t[key]); - } - return sortObject; +function mapToMap(t: Map): SortForCmd { + const sortEntries: SortPairForCmd[] = Array.from(t).map(([k, v]) => [ + `${k}`, + prepareDirection(v) + ]); + return new Map(sortEntries); } /** converts a Sort type into a type that is valid for the server (SortForCmd) */ @@ -103,12 +113,15 @@ export function formatSort( 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)}`); + if (typeof sort === 'string') return new Map([[sort, prepareDirection(direction)]]); + if (typeof sort !== 'object') { + throw new Error(`Invalid sort format: ${JSON.stringify(sort)}`); + } + if (!Array.isArray(sort)) { + return isMap(sort) ? mapToMap(sort) : Object.keys(sort).length ? objectToMap(sort) : undefined; + } + if (!sort.length) return undefined; + if (isDeep(sort)) return deepToMap(sort); + if (isPair(sort)) return pairToMap(sort); + return stringsToMap(sort); } diff --git a/test/functional/apm.test.js b/test/functional/apm.test.js index ab13d32270d..5c8c0d3f89c 100644 --- a/test/functional/apm.test.js +++ b/test/functional/apm.test.js @@ -673,6 +673,20 @@ describe('APM', function () { Object.keys(expected).forEach(key => { expect(actual).to.include.key(key); + // TODO: This is a workaround that works because all sorts in the specs + // are objects with one key; ideally we'd want to adjust the spec definitions + // to indicate whether order matters for any given key and set general + // expectations accordingly (see NODE-3235) + if (key === 'sort') { + expect(actual[key]).to.be.instanceOf(Map); + expect(Object.keys(expected[key])).to.have.lengthOf(1); + expect(actual[key].size).to.equal(1); + expect(actual[key].get(Object.keys(expected[key])[0])).to.equal( + Object.values(expected[key])[0] + ); + return; + } + if (Array.isArray(expected[key])) { expect(actual[key]).to.be.instanceOf(Array); expect(actual[key]).to.have.lengthOf(expected[key].length); diff --git a/test/functional/cursor.test.js b/test/functional/cursor.test.js index 7e13afaa7c0..3f32ec6f1e1 100644 --- a/test/functional/cursor.test.js +++ b/test/functional/cursor.test.js @@ -4175,7 +4175,8 @@ describe('Cursor', function () { const cursor = collection.find({}, { sort: input }); cursor.next(err => { expect(err).to.not.exist; - expect(events[0].command.sort).to.deep.equal(output); + expect(events[0].command.sort).to.be.instanceOf(Map); + expect(Array.from(events[0].command.sort)).to.deep.equal(Array.from(output)); cursor.close(done); }); }); @@ -4189,54 +4190,97 @@ describe('Cursor', function () { const cursor = collection.find({}).sort(input); cursor.next(err => { expect(err).to.not.exist; - expect(events[0].command.sort).to.deep.equal(output); + expect(events[0].command.sort).to.be.instanceOf(Map); + expect(Array.from(events[0].command.sort)).to.deep.equal(Array.from(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 find options object', findSort({ alpha: 1 }, new Map([['alpha', 1]]))); + it('should use find options string', findSort('alpha', new Map([['alpha', 1]]))); + it('should use find options shallow array', findSort(['alpha', 1], new Map([['alpha', 1]]))); + it('should use find options deep array', findSort([['alpha', 1]], new Map([['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('should use cursor.sort object', cursorSort({ alpha: 1 }, new Map([['alpha', 1]]))); + it('should use cursor.sort string', cursorSort('alpha', new Map([['alpha', 1]]))); + it('should use cursor.sort shallow array', cursorSort(['alpha', 1], new Map([['alpha', 1]]))); + it('should use cursor.sort deep array', cursorSort([['alpha', 1]], new Map([['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' } - }); + // TODO (NODE-3236): These are unit tests for a standalone function and should be moved out of the cursor context file + expect(formatSort('alpha')).to.deep.equal(new Map([['alpha', 1]])); + expect(formatSort(['alpha'])).to.deep.equal(new Map([['alpha', 1]])); + expect(formatSort('alpha', 1)).to.deep.equal(new Map([['alpha', 1]])); + expect(formatSort('alpha', 'asc')).to.deep.equal(new Map([['alpha', 1]])); + expect(formatSort([['alpha', 'asc']])).to.deep.equal(new Map([['alpha', 1]])); + expect(formatSort('alpha', 'ascending')).to.deep.equal(new Map([['alpha', 1]])); + expect(formatSort({ alpha: 1 })).to.deep.equal(new Map([['alpha', 1]])); + expect(formatSort('beta')).to.deep.equal(new Map([['beta', 1]])); + expect(formatSort(['beta'])).to.deep.equal(new Map([['beta', 1]])); + expect(formatSort('beta', -1)).to.deep.equal(new Map([['beta', -1]])); + expect(formatSort('beta', 'desc')).to.deep.equal(new Map([['beta', -1]])); + expect(formatSort('beta', 'descending')).to.deep.equal(new Map([['beta', -1]])); + expect(formatSort({ beta: -1 })).to.deep.equal(new Map([['beta', -1]])); + expect(formatSort({ alpha: { $meta: 'hi' } })).to.deep.equal( + new Map([['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', 'beta'])).to.deep.equal( + new Map([ + ['alpha', 1], + ['beta', 1] + ]) + ); + expect(formatSort({ alpha: 1, beta: 1 })).to.deep.equal( + new Map([ + ['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 - }); + ).to.deep.equal( + new Map([ + ['alpha', 1], + ['beta', 1] + ]) + ); + expect( + formatSort( + new Map([ + ['alpha', 'asc'], + ['beta', 'ascending'] + ]) + ) + ).to.deep.equal( + new Map([ + ['alpha', 1], + ['beta', 1] + ]) + ); + expect( + formatSort([ + ['3', 'asc'], + ['1', 'ascending'] + ]) + ).to.deep.equal( + new Map([ + ['3', 1], + ['1', 1] + ]) + ); + expect(formatSort({ alpha: { $meta: 'hi' }, beta: 'ascending' })).to.deep.equal( + new Map([ + ['alpha', { $meta: 'hi' }], + ['beta', 1] + ]) + ); }); it('should use allowDiskUse option on sort', { @@ -4249,7 +4293,7 @@ describe('Cursor', function () { cursor.next(err => { expect(err).to.not.exist; const { command } = events.shift(); - expect(command.sort).to.deep.equal({ alpha: 1 }); + expect(command.sort).to.deep.equal(new Map([['alpha', 1]])); expect(command.allowDiskUse).to.be.true; cursor.close(done); }); diff --git a/test/functional/spec-runner/index.js b/test/functional/spec-runner/index.js index a18338793ce..bed31325a59 100644 --- a/test/functional/spec-runner/index.js +++ b/test/functional/spec-runner/index.js @@ -414,14 +414,26 @@ function validateExpectations(commandEvents, spec, savedSessionData) { const actualCommand = actual.command; const expectedCommand = expected.command; + if (expectedCommand.sort) { + // TODO: This is a workaround that works because all sorts in the specs + // are objects with one key; ideally we'd want to adjust the spec definitions + // to indicate whether order matters for any given key and set general + // expectations accordingly (see NODE-3235) + expect(Object.keys(expectedCommand.sort)).to.have.lengthOf(1); + expect(actualCommand.sort).to.be.instanceOf(Map); + expect(actualCommand.sort.size).to.equal(1); + const expectedKey = Object.keys(expectedCommand.sort)[0]; + expect(actualCommand.sort).to.have.all.keys(expectedKey); + actualCommand.sort = { [expectedKey]: actualCommand.sort.get(expectedKey) }; + } expect(actualCommand).withSessionData(savedSessionData).to.matchMongoSpec(expectedCommand); }); } function normalizeCommandShapes(commands) { - return commands.map(def => - JSON.parse( + return commands.map(def => { + const output = JSON.parse( EJSON.stringify( { command: def.command, @@ -430,8 +442,13 @@ function normalizeCommandShapes(commands) { }, { relaxed: true } ) - ) - ); + ); + // TODO: this is a workaround to preserve sort Map type until NODE-3235 is completed + if (def.command.sort) { + output.command.sort = def.command.sort; + } + return output; + }); } function extractCrudResult(result, operation) { diff --git a/test/functional/unified-spec-runner/match.ts b/test/functional/unified-spec-runner/match.ts index 1becb0cdb2b..f452f2d6718 100644 --- a/test/functional/unified-spec-runner/match.ts +++ b/test/functional/unified-spec-runner/match.ts @@ -138,7 +138,21 @@ export function resultCheck( for (const [key, value] of expectedEntries) { path.push(Array.isArray(expected) ? `[${key}]` : `.${key}`); // record what key we're at depth += 1; - resultCheck(actual[key], value, entities, path, depth); + if (key === 'sort') { + // TODO: This is a workaround that works because all sorts in the specs + // are objects with one key; ideally we'd want to adjust the spec definitions + // to indicate whether order matters for any given key and set general + // expectations accordingly (see NODE-3235) + expect(Object.keys(value)).to.have.lengthOf(1); + expect(actual[key]).to.be.instanceOf(Map); + expect(actual[key].size).to.equal(1); + const expectedSortKey = Object.keys(value)[0]; + expect(actual[key]).to.have.all.keys(expectedSortKey); + const objFromActual = { [expectedSortKey]: actual[key].get(expectedSortKey) }; + resultCheck(objFromActual, value, entities, path, depth); + } else { + resultCheck(actual[key], value, entities, path, depth); + } depth -= 1; path.pop(); // if the recursion was successful we can drop the tested key }