From d08bfe3fa4f5917e4adeb25cb93f6b5723f3689e Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Fri, 8 Jul 2022 17:10:42 -0400 Subject: [PATCH] Fixed map functionality --- src/operations/indexes.ts | 80 ++++++++++++++++++---------- test/unit/operations/indexes.test.ts | 38 +++++++++---- test/unit/utils.test.ts | 71 ------------------------ 3 files changed, 79 insertions(+), 110 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 82e0007d1ea..08de9a0faa6 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -59,7 +59,10 @@ function isIndexDirection(x: any): x is IndexDirection { /** @public */ export type IndexSpecification = OneOrMore< - string | [string, IndexDirection] | { [key: string]: IndexDirection } + | string + | [string, IndexDirection] + | { [key: string]: IndexDirection } + | Map >; /** @public */ @@ -87,7 +90,7 @@ export interface IndexDescription > { collation?: CollationOptions; name?: string; - key: Document; + key: Document | Map; } /** @public */ @@ -131,31 +134,39 @@ export interface CreateIndexesOptions extends CommandOperationOptions { hidden?: boolean; } +function isSingleIndexTuple(t: unknown): t is [string, IndexDirection] { + return Array.isArray(t) && t.length === 2 && isIndexDirection(t[1]); +} + export function makeIndexSpec(indexSpec: IndexSpecification, options: any): IndexDescription { function getFieldHash(indexSpec: IndexSpecification) { - let fieldHash: Map = new Map(); + const fieldHash: Map = new Map(); let indexSpecArr: IndexSpecification[]; - // wrap in array if needed - if (!Array.isArray(indexSpec) || (indexSpec.length === 2 && isIndexDirection(indexSpec[1]))) { + // Wrap indexSpec in array if needed + if (!Array.isArray(indexSpec) || isSingleIndexTuple(indexSpec)) { indexSpecArr = [indexSpec]; } else { indexSpecArr = indexSpec; } - // iterate through array and handle different types - indexSpecArr.forEach((f: any) => { - if ('string' === typeof f) { - fieldHash.set(f, 1); - } else if (Array.isArray(f)) { - fieldHash.set(f[0], f[1]); - } else if (isObject(f)) { - for (const [key, value] of Object.entries(f)) { + // Iterate through array and handle different types + for (const spec of indexSpecArr) { + if ('string' === typeof spec) { + fieldHash.set(spec, 1); + } else if (Array.isArray(spec)) { + fieldHash.set(spec[0], spec[1]); + } else if (spec instanceof Map) { + for (const [key, value] of spec) { + fieldHash.set(key, value); + } + } else if (isObject(spec)) { + for (const [key, value] of Object.entries(spec)) { fieldHash.set(key, value); } } - }); + } return fieldHash; } @@ -225,9 +236,32 @@ export class CreateIndexesOperation< this.options = options ?? {}; this.collectionName = collectionName; - this.indexes = indexes; + // Ensure we generate the correct name if the parameter is not set + const normalizedIndexes = []; + for (const userIndex of indexes) { + const key = + userIndex.key instanceof Map ? userIndex.key : new Map(Object.entries(userIndex.key)); + const index: Omit & { key: Map } = { + ...userIndex, + key + }; + if (index.name == null) { + const keys = []; + + for (const [name, direction] of index.key) { + keys.push(`${name}_${direction}`); + } + + // Set the name + index.name = keys.join('_'); + } + normalizedIndexes.push(index); + } + this.indexes = normalizedIndexes; } + /* TODO: create name in the parent class constructor */ + /* create a type assertion to stop typescript errors */ override execute( server: Server, session: ClientSession | undefined, @@ -238,10 +272,9 @@ export class CreateIndexesOperation< const serverWireVersion = maxWireVersion(server); - // Ensure we generate the correct name if the parameter is not set - for (let i = 0; i < indexes.length; i++) { + for (const index of indexes) { // Did the user pass in a collation, check if our write server supports it - if (indexes[i].collation && serverWireVersion < 5) { + if (index.collation && serverWireVersion < 5) { callback( new MongoCompatibilityError( `Server ${server.name}, which reports wire version ${serverWireVersion}, ` + @@ -250,17 +283,6 @@ export class CreateIndexesOperation< ); return; } - - if (indexes[i].name == null) { - const keys = []; - - for (const name in indexes[i].key) { - keys.push(`${name}_${indexes[i].key[name]}`); - } - - // Set the name - indexes[i].name = keys.join('_'); - } } const cmd: Document = { createIndexes: this.collectionName, indexes }; diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts index e2b0d34b2bd..aadfcb8dc75 100644 --- a/test/unit/operations/indexes.test.ts +++ b/test/unit/operations/indexes.test.ts @@ -12,12 +12,14 @@ describe('makeIndexSpec()', () => { { description: 'single string', input: 'sample_index', - mapData: new Map([['sample_index', 1]]) + mapData: new Map([['sample_index', 1]]), + name: 'sample_index_1' }, { description: 'single [string, IndexDirection]', input: ['sample_index', -1], - mapData: new Map([['sample_index', -1]]) + mapData: new Map([['sample_index', -1]]), + name: 'sample_index_-1' }, { description: 'array of strings', @@ -26,7 +28,8 @@ describe('makeIndexSpec()', () => { ['sample_index1', 1], ['sample_index2', 1], ['sample_index3', 1] - ]) + ]), + name: 'sample_index1_1_sample_index2_1_sample_index3_1' }, { description: 'array of [string, IndexDirection]', @@ -39,12 +42,14 @@ describe('makeIndexSpec()', () => { ['sample_index1', -1], ['sample_index2', 1], ['sample_index3', '2d'] - ]) + ]), + name: 'sample_index1_-1_sample_index2_1_sample_index3_2d' }, { description: 'single { [key: string]: IndexDirection }', input: { sample_index: -1 }, - mapData: new Map([['sample_index', -1]]) + mapData: new Map([['sample_index', -1]]), + name: 'sample_index_-1' }, { description: 'array of { [key: string]: IndexDirection }', @@ -53,23 +58,25 @@ describe('makeIndexSpec()', () => { ['sample_index1', -1], ['sample_index2', 1], ['sample_index3', '2d'] - ]) + ]), + name: 'sample_index1_-1_sample_index2_1_sample_index3_2d' }, { - name: 'mixed array of [string, [string, IndexDirection], { [key: string]: IndexDirection }]', + description: 'mixed array of [string, [string, IndexDirection], { [key: string]: IndexDirection }]', input: ['sample_index1', ['sample_index2', -1], { sample_index3: '2d' }], mapData: new Map([ ['sample_index1', 1], ['sample_index2', -1], ['sample_index3', '2d'] - ]) + ]), + name: 'sample_index1_1_sample_index2_-1_sample_index3_2d' } ]; const makeIndexOperation = (input, options: CreateIndexesOptions = {}) => new CreateIndexOperation({ s: { namespace: ns('a.b') } }, 'b', input, options); - for (const { description, input, mapData } of testCases) { + for (const { description, input, mapData, name } of testCases) { it(`should create fieldHash correctly when input is: ${description}`, () => { const realOutput = makeIndexOperation(input); expect(realOutput.indexes[0].key).to.deep.equal(mapData); @@ -77,7 +84,18 @@ describe('makeIndexSpec()', () => { it(`should set name to null if none provided with ${description} input `, () => { const realOutput = makeIndexOperation(input); - expect(realOutput.indexes[0].name).to.equal(null); + expect(realOutput.indexes[0].name).to.equal(name); }); } + + it('should keep numerical keys in chronological ordering', () => { + const desiredMapData = new Map([ + ['2', -1], + ['1', 1] + ]); + const realOutput = makeIndexOperation(desiredMapData); + const desiredName = '2_-1_1_1'; + expect(realOutput.indexes[0].key).to.deep.equal(desiredMapData); + expect(realOutput.indexes[0].name).to.equal(desiredName); + }); }); diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index 17e35f7279a..5b285173bf6 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -526,75 +526,4 @@ describe('driver utils', function () { expect(isHello(doc)).to.be.false; }); }); - - describe('parseIndexOptions()', () => { - const testCases = [ - { - description: 'single string', - input: 'sample_index', - output: { name: 'sample_index_1', keys: undefined, fieldHash: { sample_index: 1 } } - }, - { - description: 'single [string, IndexDirection]', - input: ['sample_index', -1], - output: { name: 'sample_index_1', keys: undefined, fieldHash: { sample_index: 1 } } - }, - { - description: 'array of strings', - input: ['sample_index1', 'sample_index2', 'sample_index3'], - output: { - name: 'sample_index1_1_sample_index2_1_sample_index3_1', - keys: undefined, - fieldHash: { sample_index1: 1, sample_index2: 1, sample_index3: 1 } - } - }, - { - description: 'array of [string, IndexDirection]', - input: [ - ['sample_index1', -1], - ['sample_index2', 1], - ['sample_index3', '2d'] - ], - output: { - name: 'sample_index1_-1_sample_index2_1_sample_index3_2d', - keys: undefined, - fieldHash: { sample_index1: -1, sample_index2: 1, sample_index3: '2d' } - } - }, - { - description: 'single { [key: string]: IndexDirection }', - input: { d: { sample_index: 1 } }, - output: { name: 'd_[object Object]', keys: ['d'], fieldHash: { d: { sample_index: 1 } } } - }, - { - description: 'array of { [key: string]: IndexDirection }', - input: { d: { sample_index1: -1 }, k: { sample_index2: 1 }, n: { sample_index2: '2d' }}, - output: { - name: 'd_[object Object]_k_[object Object]_n_[object Object]', - keys: ['d', 'k', 'n'], - fieldHash: { - d: { sample_index1: -1 }, - k: { sample_index2: 1 }, - n: { sample_index2: '2d' } - } - } - }, - { - name: 'mixed array of [string, [string, IndexDirection], { [key: string]: IndexDirection }]', - input: ['sample_index1', ['sample_index2', -1], { d: { sample_index2: '2d' } }], - output: { - name: 'sample_index1_1_sample_index2_-1_d_[object Object]', - keys: ['d'], - fieldHash: { sample_index1: 1, sample_index2: -1, d: { sample_index2: '2d' } } - } - } - ]; - - for (const { description, input, output } of testCases) { - it(`should parse index options correctly when input is: ${description}`, () => { - const real_output = parseIndexOptions(input); - expect(real_output).to.eql(output); - }); - } - }); });