Fixed map functionality

mongodb · Jul 8, 2022 · d08bfe3 · d08bfe3
1 parent 68c3b37
commit d08bfe3
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 110 deletions.
diff --git 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<string, IndexDirection>
 >;
 
 /** @public */
@@ -87,7 +90,7 @@ export interface IndexDescription
   > {
   collation?: CollationOptions;
   name?: string;
-  key: Document;
+  key: Document | Map<string, IndexDirection>;
 }
 
 /** @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<string, IndexDirection> = new Map();
+    const fieldHash: Map<string, IndexDirection> = 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<IndexDescription, 'key'> & { key: Map<string, IndexDirection> } = {
+        ...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
@@ -12,12 +12,14 @@ describe('makeIndexSpec()', () => {
     {
       description: 'single string',
       input: 'sample_index',
-      mapData: new Map<string, IndexDirection>([['sample_index', 1]])
+      mapData: new Map<string, IndexDirection>([['sample_index', 1]]),
+      name: 'sample_index_1'
     },
     {
       description: 'single [string, IndexDirection]',
       input: ['sample_index', -1],
-      mapData: new Map<string, IndexDirection>([['sample_index', -1]])
+      mapData: new Map<string, IndexDirection>([['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<string, IndexDirection>([['sample_index', -1]])
+      mapData: new Map<string, IndexDirection>([['sample_index', -1]]),
+      name: 'sample_index_-1'
     },
     {
       description: 'array of { [key: string]: IndexDirection }',
@@ -53,31 +58,44 @@ 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<string, IndexDirection>([
         ['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);
     });
 
     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<string, IndexDirection>([
+      ['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
@@ -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);
-      });
-    }
-  });
 });