From 65a67e958350734e1ab0372dac3a42f3a028aaf0 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Fri, 1 Jul 2022 15:27:37 -0400 Subject: [PATCH 01/29] Added unit tests --- src/operations/indexes.ts | 6 +--- src/utils.ts | 2 +- test/unit/utils.test.ts | 72 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index c979a64b7f..432a158bf5 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -54,11 +54,7 @@ export type IndexDirection = -1 | 1 | '2d' | '2dsphere' | 'text' | 'geoHaystack' /** @public */ export type IndexSpecification = OneOrMore< - | string - | [string, IndexDirection] - | { [key: string]: IndexDirection } - | [string, IndexDirection][] - | { [key: string]: IndexDirection }[] + string | [string, IndexDirection] | { [key: string]: IndexDirection } >; /** @public */ diff --git a/src/utils.ts b/src/utils.ts index abbdf02d1d..1d395408a6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -115,7 +115,7 @@ interface IndexOptions { * @internal */ export function parseIndexOptions(indexSpec: IndexSpecification): IndexOptions { - const fieldHash: { [key: string]: IndexDirection } = {}; + const fieldHash: { [key: string]: IndexDirection } ={}; const indexes = []; let keys; diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index f41673d038..96eba2b55d 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -9,6 +9,7 @@ import { isHello, makeInterruptibleAsyncInterval, MongoDBNamespace, + parseIndexOptions, shuffle } from '../../src/utils'; import { createTimerSandbox } from './timer_sandbox'; @@ -585,4 +586,75 @@ describe('driver utils', function () { }); }); }); + + 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); + }); + } + }); }); From 6fc9cc188d20bc9ddcba2887c821dc8688547ac6 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Wed, 6 Jul 2022 17:36:35 -0400 Subject: [PATCH 02/29] refactored parseIndexOptions and put it inside makeIndexSpec --- src/operations/indexes.ts | 48 ++++++++++++++++--- src/utils.ts | 57 ---------------------- test/unit/operations/indexes.test.ts | 71 ++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 64 deletions(-) create mode 100644 test/unit/operations/indexes.test.ts diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 432a158bf5..ec73639e31 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -1,3 +1,5 @@ +import { json } from 'stream/consumers'; + import type { Document } from '../bson'; import type { Collection } from '../collection'; import type { Db } from '../db'; @@ -6,7 +8,7 @@ import type { OneOrMore } from '../mongo_types'; import { ReadPreference } from '../read_preference'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; -import { Callback, maxWireVersion, MongoDBNamespace, parseIndexOptions } from '../utils'; +import { Callback, isObject, maxWireVersion, MongoDBNamespace } from '../utils'; import { CollationOptions, CommandOperation, @@ -51,6 +53,11 @@ const VALID_INDEX_OPTIONS = new Set([ /** @public */ export type IndexDirection = -1 | 1 | '2d' | '2dsphere' | 'text' | 'geoHaystack' | number; +function isIndexDirection(x: any): x is IndexDirection { + return ( + typeof x === 'number' || x === '2d' || x === '2dsphere' || x === 'text' || x === 'geoHaystack' + ); +} /** @public */ export type IndexSpecification = OneOrMore< @@ -126,14 +133,42 @@ export interface CreateIndexesOptions extends CommandOperationOptions { hidden?: boolean; } -function makeIndexSpec(indexSpec: IndexSpecification, options: any): IndexDescription { - const indexParameters = parseIndexOptions(indexSpec); +export function makeIndexSpec(indexSpec: IndexSpecification, options: any): IndexDescription { + function getFieldHash(indexSpec: IndexSpecification) { + let fieldHash: Map = new Map(); + + let indexSpecArr: IndexSpecification[]; + + // wrap in array if needed + if (!Array.isArray(indexSpec) || (indexSpec.length === 2 && isIndexDirection(indexSpec[1]))) { + 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)) { + fieldHash.set(key, value); + } + } + }); + + return fieldHash; + } + + const fieldHash = getFieldHash(indexSpec); // Generate the index name - const name = typeof options.name === 'string' ? options.name : indexParameters.name; + const name = typeof options.name === 'string' ? options.name : null; // Set up the index - const finalIndexSpec: Document = { name, key: indexParameters.fieldHash }; + const finalIndexSpec: Document = { name, key: fieldHash }; // merge valid index options into the index spec for (const optionName in options) { @@ -272,8 +307,7 @@ export class CreateIndexOperation extends CreateIndexesOperation { // coll.createIndex({ a: 1 }); // coll.createIndex([['a', 1]]); // createIndexes is always called with an array of index spec objects - - super(parent, collectionName, [makeIndexSpec(indexSpec, options)], options); + super(parent, collectionName, [makeIndexSpec(indexSpec, options)]); } override execute( server: Server, diff --git a/src/utils.ts b/src/utils.ts index 1d395408a6..bfacbcbebb 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -104,63 +104,6 @@ export function normalizeHintField(hint?: Hint): Hint | undefined { return finalHint; } -interface IndexOptions { - name: string; - keys?: string[]; - fieldHash: Document; -} - -/** - * Create an index specifier based on - * @internal - */ -export function parseIndexOptions(indexSpec: IndexSpecification): IndexOptions { - const fieldHash: { [key: string]: IndexDirection } ={}; - const indexes = []; - let keys; - - // Get all the fields accordingly - if ('string' === typeof indexSpec) { - // 'type' - indexes.push(indexSpec + '_' + 1); - fieldHash[indexSpec] = 1; - } else if (Array.isArray(indexSpec)) { - indexSpec.forEach((f: any) => { - if ('string' === typeof f) { - // [{location:'2d'}, 'type'] - indexes.push(f + '_' + 1); - fieldHash[f] = 1; - } else if (Array.isArray(f)) { - // [['location', '2d'],['type', 1]] - indexes.push(f[0] + '_' + (f[1] || 1)); - fieldHash[f[0]] = f[1] || 1; - } else if (isObject(f)) { - // [{location:'2d'}, {type:1}] - keys = Object.keys(f); - keys.forEach(k => { - indexes.push(k + '_' + (f as AnyOptions)[k]); - fieldHash[k] = (f as AnyOptions)[k]; - }); - } else { - // undefined (ignore) - } - }); - } else if (isObject(indexSpec)) { - // {location:'2d', type:1} - keys = Object.keys(indexSpec); - Object.entries(indexSpec).forEach(([key, value]) => { - indexes.push(key + '_' + value); - fieldHash[key] = value; - }); - } - - return { - name: indexes.join('_'), - keys: keys, - fieldHash: fieldHash - }; -} - const TO_STRING = (object: unknown) => Object.prototype.toString.call(object); /** * Checks if arg is an Object: diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts new file mode 100644 index 0000000000..98e59f4cca --- /dev/null +++ b/test/unit/operations/indexes.test.ts @@ -0,0 +1,71 @@ +import { expect } from 'chai'; + +import { IndexDirection, makeIndexSpec } from '../../../src/operations/indexes'; + +describe('makeIndexSpec()', () => { + const testCases = [ + { + description: 'single string', + input: 'sample_index', + mapData: [['sample_index', 1]] + }, + { + description: 'single [string, IndexDirection]', + input: ['sample_index', -1], + mapData: [['sample_index', -1]] + }, + { + description: 'array of strings', + input: ['sample_index1', 'sample_index2', 'sample_index3'], + mapData: [ + ['sample_index1', 1], + ['sample_index2', 1], + ['sample_index3', 1] + ] + }, + { + description: 'array of [string, IndexDirection]', + input: [ + ['sample_index1', -1], + ['sample_index2', 1], + ['sample_index3', '2d'] + ], + mapData: [ + ['sample_index1', -1], + ['sample_index2', 1], + ['sample_index3', '2d'] + ] + }, + { + description: 'single { [key: string]: IndexDirection }', + input: { sample_index: -1 }, + mapData: [['sample_index', -1]] + }, + { + description: 'array of { [key: string]: IndexDirection }', + input: [{ sample_index1: -1 }, { sample_index2: 1 }, { sample_index3: '2d' }], + mapData: [ + ['sample_index1', -1], + ['sample_index2', 1], + ['sample_index3', '2d'] + ] + }, + { + name: 'mixed array of [string, [string, IndexDirection], { [key: string]: IndexDirection }]', + input: ['sample_index1', ['sample_index2', -1], { sample_index3: '2d' }], + mapData: [ + ['sample_index1', 1], + ['sample_index2', -1], + ['sample_index3', '2d'] + ] + } + ]; + + for (const { description, input, mapData } of testCases) { + it(`should parse index options correctly when input is: ${description}`, () => { + const desiredOutput: Map = new Map(mapData); + const realOutput = makeIndexSpec(input, {}); + expect(realOutput.key).to.eql(desiredOutput); + }); + } +}); From 053c047acaf1a7f0c2f0a0cc50ddfa2868ba57e5 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Wed, 6 Jul 2022 17:43:13 -0400 Subject: [PATCH 03/29] refactored parseIndexOptions and moved it inside makeIndexSpec --- src/utils.ts | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/utils.ts b/src/utils.ts index bfacbcbebb..5dfe5b620f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -104,7 +104,67 @@ export function normalizeHintField(hint?: Hint): Hint | undefined { return finalHint; } +<<<<<<< HEAD const TO_STRING = (object: unknown) => Object.prototype.toString.call(object); +======= +interface IndexOptions { + name: string; + keys?: string[]; + fieldHash: Document; +} + +/** + * Create an index specifier based on + * @internal + */ +export function parseIndexOptions(indexSpec: IndexSpecification): IndexOptions { + const fieldHash: { [key: string]: IndexDirection } ={}; + const indexes = []; + let keys; + + // Get all the fields accordingly + if ('string' === typeof indexSpec) { + // 'type' + indexes.push(indexSpec + '_' + 1); + fieldHash[indexSpec] = 1; + } else if (Array.isArray(indexSpec)) { + indexSpec.forEach((f: any) => { + if ('string' === typeof f) { + // [{location:'2d'}, 'type'] + indexes.push(f + '_' + 1); + fieldHash[f] = 1; + } else if (Array.isArray(f)) { + // [['location', '2d'],['type', 1]] + indexes.push(f[0] + '_' + (f[1] || 1)); + fieldHash[f[0]] = f[1] || 1; + } else if (isObject(f)) { + // [{location:'2d'}, {type:1}] + keys = Object.keys(f); + keys.forEach(k => { + indexes.push(k + '_' + (f as AnyOptions)[k]); + fieldHash[k] = (f as AnyOptions)[k]; + }); + } else { + // undefined (ignore) + } + }); + } else if (isObject(indexSpec)) { + // {location:'2d', type:1} + keys = Object.keys(indexSpec); + Object.entries(indexSpec).forEach(([key, value]) => { + indexes.push(key + '_' + value); + fieldHash[key] = value; + }); + } + + return { + name: indexes.join('_'), + keys: keys, + fieldHash: fieldHash + }; +} + +>>>>>>> 45d12392f94a9abd3c735bdc515964ea8d48f48d /** * Checks if arg is an Object: * - **NOTE**: the check is based on the `[Symbol.toStringTag]() === 'Object'` From 1fa6b632e82e0cb90d051ce9f4bf7f19458693c2 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Thu, 7 Jul 2022 11:17:25 -0400 Subject: [PATCH 04/29] reorganized tests --- src/operations/indexes.ts | 2 - src/utils.ts | 61 ---------------------------- test/unit/operations/indexes.test.ts | 44 ++++++++++++-------- 3 files changed, 28 insertions(+), 79 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index ec73639e31..82e0007d1e 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -1,5 +1,3 @@ -import { json } from 'stream/consumers'; - import type { Document } from '../bson'; import type { Collection } from '../collection'; import type { Db } from '../db'; diff --git a/src/utils.ts b/src/utils.ts index 5dfe5b620f..c770ed33a3 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -23,7 +23,6 @@ import { import type { Explain } from './explain'; import type { MongoClient } from './mongo_client'; import type { CommandOperationOptions, OperationParent } from './operations/command'; -import type { IndexDirection, IndexSpecification } from './operations/indexes'; import type { Hint, OperationOptions } from './operations/operation'; import { PromiseProvider } from './promise_provider'; import { ReadConcern } from './read_concern'; @@ -104,67 +103,7 @@ export function normalizeHintField(hint?: Hint): Hint | undefined { return finalHint; } -<<<<<<< HEAD const TO_STRING = (object: unknown) => Object.prototype.toString.call(object); -======= -interface IndexOptions { - name: string; - keys?: string[]; - fieldHash: Document; -} - -/** - * Create an index specifier based on - * @internal - */ -export function parseIndexOptions(indexSpec: IndexSpecification): IndexOptions { - const fieldHash: { [key: string]: IndexDirection } ={}; - const indexes = []; - let keys; - - // Get all the fields accordingly - if ('string' === typeof indexSpec) { - // 'type' - indexes.push(indexSpec + '_' + 1); - fieldHash[indexSpec] = 1; - } else if (Array.isArray(indexSpec)) { - indexSpec.forEach((f: any) => { - if ('string' === typeof f) { - // [{location:'2d'}, 'type'] - indexes.push(f + '_' + 1); - fieldHash[f] = 1; - } else if (Array.isArray(f)) { - // [['location', '2d'],['type', 1]] - indexes.push(f[0] + '_' + (f[1] || 1)); - fieldHash[f[0]] = f[1] || 1; - } else if (isObject(f)) { - // [{location:'2d'}, {type:1}] - keys = Object.keys(f); - keys.forEach(k => { - indexes.push(k + '_' + (f as AnyOptions)[k]); - fieldHash[k] = (f as AnyOptions)[k]; - }); - } else { - // undefined (ignore) - } - }); - } else if (isObject(indexSpec)) { - // {location:'2d', type:1} - keys = Object.keys(indexSpec); - Object.entries(indexSpec).forEach(([key, value]) => { - indexes.push(key + '_' + value); - fieldHash[key] = value; - }); - } - - return { - name: indexes.join('_'), - keys: keys, - fieldHash: fieldHash - }; -} - ->>>>>>> 45d12392f94a9abd3c735bdc515964ea8d48f48d /** * Checks if arg is an Object: * - **NOTE**: the check is based on the `[Symbol.toStringTag]() === 'Object'` diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts index 98e59f4cca..e2b0d34b2b 100644 --- a/test/unit/operations/indexes.test.ts +++ b/test/unit/operations/indexes.test.ts @@ -1,27 +1,32 @@ import { expect } from 'chai'; -import { IndexDirection, makeIndexSpec } from '../../../src/operations/indexes'; +import { + CreateIndexesOptions, + CreateIndexOperation, + IndexDirection +} from '../../../src/operations/indexes'; +import { ns } from '../../../src/utils'; describe('makeIndexSpec()', () => { const testCases = [ { description: 'single string', input: 'sample_index', - mapData: [['sample_index', 1]] + mapData: new Map([['sample_index', 1]]) }, { description: 'single [string, IndexDirection]', input: ['sample_index', -1], - mapData: [['sample_index', -1]] + mapData: new Map([['sample_index', -1]]) }, { description: 'array of strings', input: ['sample_index1', 'sample_index2', 'sample_index3'], - mapData: [ + mapData: new Map([ ['sample_index1', 1], ['sample_index2', 1], ['sample_index3', 1] - ] + ]) }, { description: 'array of [string, IndexDirection]', @@ -30,42 +35,49 @@ describe('makeIndexSpec()', () => { ['sample_index2', 1], ['sample_index3', '2d'] ], - mapData: [ + mapData: new Map([ ['sample_index1', -1], ['sample_index2', 1], ['sample_index3', '2d'] - ] + ]) }, { description: 'single { [key: string]: IndexDirection }', input: { sample_index: -1 }, - mapData: [['sample_index', -1]] + mapData: new Map([['sample_index', -1]]) }, { description: 'array of { [key: string]: IndexDirection }', input: [{ sample_index1: -1 }, { sample_index2: 1 }, { sample_index3: '2d' }], - mapData: [ + mapData: new Map([ ['sample_index1', -1], ['sample_index2', 1], ['sample_index3', '2d'] - ] + ]) }, { name: 'mixed array of [string, [string, IndexDirection], { [key: string]: IndexDirection }]', input: ['sample_index1', ['sample_index2', -1], { sample_index3: '2d' }], - mapData: [ + mapData: new Map([ ['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) { - it(`should parse index options correctly when input is: ${description}`, () => { - const desiredOutput: Map = new Map(mapData); - const realOutput = makeIndexSpec(input, {}); - expect(realOutput.key).to.eql(desiredOutput); + 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); }); } }); From 0df78c016c8a6fd2057da02f935f0d0e50df55a0 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Fri, 8 Jul 2022 17:10:42 -0400 Subject: [PATCH 05/29] Fixed map functionality --- src/operations/indexes.ts | 80 ++++++++++++++++++---------- test/unit/operations/indexes.test.ts | 38 +++++++++---- 2 files changed, 79 insertions(+), 39 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 82e0007d1e..08de9a0faa 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 e2b0d34b2b..aadfcb8dc7 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); + }); }); From af386dc63851a3a0f1bb71ecf0bb91043de175f1 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Mon, 11 Jul 2022 15:31:15 -0400 Subject: [PATCH 06/29] wip --- test/unit/operations/indexes.test.ts | 33 +++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts index aadfcb8dc7..bacde15a53 100644 --- a/test/unit/operations/indexes.test.ts +++ b/test/unit/operations/indexes.test.ts @@ -62,14 +62,41 @@ describe('makeIndexSpec()', () => { name: 'sample_index1_-1_sample_index2_1_sample_index3_2d' }, { - description: 'mixed array of [string, [string, IndexDirection], { [key: string]: IndexDirection }]', - input: ['sample_index1', ['sample_index2', -1], { sample_index3: '2d' }], + description: + 'mixed array of [string, [string, IndexDirection], { [key: string]: IndexDirection }, Map]', + input: [ + 'sample_index1', + ['sample_index2', -1], + { sample_index3: '2d' }, + new Map([['sample_index4', '2dsphere']]) + ], + mapData: new Map([ + ['sample_index1', 1], + ['sample_index2', -1], + ['sample_index3', '2d'], + ['sample_index4', '2dsphere'] + ]), + name: 'sample_index1_1_sample_index2_-1_sample_index3_2d_sample_index4_2dsphere' + }, + { + description: 'array of Map', + input: [ + new Map([['sample_index1', 1]]), + new Map([['sample_index2', -1]]), + new Map([['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' + }, + { + description: 'single Map', + input: new Map([['sample_index', -1]]), + mapData: new Map([['sample_index', -1]]), + name: 'sample_index_-1' } ]; @@ -82,7 +109,7 @@ describe('makeIndexSpec()', () => { expect(realOutput.indexes[0].key).to.deep.equal(mapData); }); - it(`should set name to null if none provided with ${description} input `, () => { + it(`should set name correctly if none provided with ${description} input `, () => { const realOutput = makeIndexOperation(input); expect(realOutput.indexes[0].name).to.equal(name); }); From c6989f2a06da1b75e50cb5116c6c618bf0173b62 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Mon, 11 Jul 2022 17:19:24 -0400 Subject: [PATCH 07/29] lint fix --- test/unit/operations/indexes.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts index bacde15a53..a45183d785 100644 --- a/test/unit/operations/indexes.test.ts +++ b/test/unit/operations/indexes.test.ts @@ -115,7 +115,7 @@ describe('makeIndexSpec()', () => { }); } - it('should keep numerical keys in chronological ordering', () => { + it('should keep numerical keys in chronological ordering when using Map input type', () => { const desiredMapData = new Map([ ['2', -1], ['1', 1] From a1364171dac429036a81437f28e879235824d4a8 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Mon, 11 Jul 2022 17:37:44 -0400 Subject: [PATCH 08/29] Removed all edits from utils.test.js --- src/operations/indexes.ts | 3 --- test/unit/{utils.test.ts => utils.test.js} | 6 +----- 2 files changed, 1 insertion(+), 8 deletions(-) rename test/unit/{utils.test.ts => utils.test.js} (99%) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 08de9a0faa..ef7d2da615 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -56,7 +56,6 @@ function isIndexDirection(x: any): x is IndexDirection { typeof x === 'number' || x === '2d' || x === '2dsphere' || x === 'text' || x === 'geoHaystack' ); } - /** @public */ export type IndexSpecification = OneOrMore< | string @@ -260,8 +259,6 @@ export class CreateIndexesOperation< 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, diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.js similarity index 99% rename from test/unit/utils.test.ts rename to test/unit/utils.test.js index 96eba2b55d..6d68b94624 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.js @@ -1,12 +1,8 @@ +'use strict'; import { expect } from 'chai'; -import * as sinon from 'sinon'; -import { LEGACY_HELLO_COMMAND } from '../../src/constants'; -import { MongoRuntimeError } from '../../src/error'; import { - BufferPool, eachAsync, - isHello, makeInterruptibleAsyncInterval, MongoDBNamespace, parseIndexOptions, From 7eddd4b64122202b36ddc24742642c22dfbfe42d Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Thu, 14 Jul 2022 03:30:59 -0400 Subject: [PATCH 09/29] Added || 1 to tuple case --- src/operations/indexes.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index ef7d2da615..8d71ebd3a9 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -155,7 +155,7 @@ export function makeIndexSpec(indexSpec: IndexSpecification, options: any): Inde if ('string' === typeof spec) { fieldHash.set(spec, 1); } else if (Array.isArray(spec)) { - fieldHash.set(spec[0], spec[1]); + fieldHash.set(spec[0], spec[1] || 1); } else if (spec instanceof Map) { for (const [key, value] of spec) { fieldHash.set(key, value); @@ -166,7 +166,6 @@ export function makeIndexSpec(indexSpec: IndexSpecification, options: any): Inde } } } - return fieldHash; } From 359dd57289af195491d8bfb3c755ee8307d2a707 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Thu, 14 Jul 2022 04:56:44 -0400 Subject: [PATCH 10/29] remove failing test for now (cannot check map equality in JSON) --- src/operations/indexes.ts | 2 +- .../operation/default-write-concern-3.4.json | 54 ------------------- test/unit/operations/indexes.test.ts | 6 +-- 3 files changed, 4 insertions(+), 58 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 8d71ebd3a9..57ede21a00 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -155,7 +155,7 @@ export function makeIndexSpec(indexSpec: IndexSpecification, options: any): Inde if ('string' === typeof spec) { fieldHash.set(spec, 1); } else if (Array.isArray(spec)) { - fieldHash.set(spec[0], spec[1] || 1); + fieldHash.set(spec[0], spec[1] ?? 1); } else if (spec instanceof Map) { for (const [key, value] of spec) { fieldHash.set(key, value); diff --git a/test/spec/read-write-concern/operation/default-write-concern-3.4.json b/test/spec/read-write-concern/operation/default-write-concern-3.4.json index 6519f6f089..6d5c2d6ad0 100644 --- a/test/spec/read-write-concern/operation/default-write-concern-3.4.json +++ b/test/spec/read-write-concern/operation/default-write-concern-3.4.json @@ -116,60 +116,6 @@ } ] }, - { - "description": "CreateIndex and dropIndex omits default write concern", - "operations": [ - { - "object": "collection", - "collectionOptions": { - "writeConcern": {} - }, - "name": "createIndex", - "arguments": { - "keys": { - "x": 1 - } - } - }, - { - "object": "collection", - "collectionOptions": { - "writeConcern": {} - }, - "name": "dropIndex", - "arguments": { - "name": "x_1" - } - } - ], - "expectations": [ - { - "command_started_event": { - "command": { - "createIndexes": "default_write_concern_coll", - "indexes": [ - { - "name": "x_1", - "key": { - "x": 1 - } - } - ], - "writeConcern": null - } - } - }, - { - "command_started_event": { - "command": { - "dropIndexes": "default_write_concern_coll", - "index": "x_1", - "writeConcern": null - } - } - } - ] - }, { "description": "MapReduce omits default write concern", "operations": [ diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts index a45183d785..c1d2945430 100644 --- a/test/unit/operations/indexes.test.ts +++ b/test/unit/operations/indexes.test.ts @@ -47,9 +47,9 @@ describe('makeIndexSpec()', () => { }, { description: 'single { [key: string]: IndexDirection }', - input: { sample_index: -1 }, - mapData: new Map([['sample_index', -1]]), - name: 'sample_index_-1' + input: { x: 1 }, + mapData: new Map([['x', 1]]), + name: 'x_1' }, { description: 'array of { [key: string]: IndexDirection }', From 2d0dde7a58726e0a6efa598516ae4a672fad136a Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Fri, 15 Jul 2022 17:05:17 -0400 Subject: [PATCH 11/29] handled incorrect map stringify singular element case --- src/operations/indexes.ts | 4 +- .../operation/default-write-concern-3.4.json | 54 +++++++++++++++++++ test/tools/spec-runner/index.js | 14 +++++ test/tools/unified-spec-runner/match.ts | 2 + test/tools/unified-spec-runner/operations.ts | 15 +++++- 5 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 57ede21a00..886537e46c 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -137,7 +137,7 @@ 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 makeIndexSpec(indexSpec: IndexSpecification, options: any): IndexDescription { function getFieldHash(indexSpec: IndexSpecification) { const fieldHash: Map = new Map(); @@ -323,7 +323,7 @@ export class CreateIndexOperation extends CreateIndexesOperation { // coll.createIndex({ a: 1 }); // coll.createIndex([['a', 1]]); // createIndexes is always called with an array of index spec objects - super(parent, collectionName, [makeIndexSpec(indexSpec, options)]); + super(parent, collectionName, [makeIndexSpec(indexSpec, options)], options); } override execute( server: Server, diff --git a/test/spec/read-write-concern/operation/default-write-concern-3.4.json b/test/spec/read-write-concern/operation/default-write-concern-3.4.json index 6d5c2d6ad0..6519f6f089 100644 --- a/test/spec/read-write-concern/operation/default-write-concern-3.4.json +++ b/test/spec/read-write-concern/operation/default-write-concern-3.4.json @@ -116,6 +116,60 @@ } ] }, + { + "description": "CreateIndex and dropIndex omits default write concern", + "operations": [ + { + "object": "collection", + "collectionOptions": { + "writeConcern": {} + }, + "name": "createIndex", + "arguments": { + "keys": { + "x": 1 + } + } + }, + { + "object": "collection", + "collectionOptions": { + "writeConcern": {} + }, + "name": "dropIndex", + "arguments": { + "name": "x_1" + } + } + ], + "expectations": [ + { + "command_started_event": { + "command": { + "createIndexes": "default_write_concern_coll", + "indexes": [ + { + "name": "x_1", + "key": { + "x": 1 + } + } + ], + "writeConcern": null + } + } + }, + { + "command_started_event": { + "command": { + "dropIndexes": "default_write_concern_coll", + "index": "x_1", + "writeConcern": null + } + } + } + ] + }, { "description": "MapReduce omits default write concern", "operations": [ diff --git a/test/tools/spec-runner/index.js b/test/tools/spec-runner/index.js index 480485159a..dd9d199cda 100644 --- a/test/tools/spec-runner/index.js +++ b/test/tools/spec-runner/index.js @@ -485,6 +485,13 @@ function validateExpectations(commandEvents, spec, savedSessionData) { actualCommand.sort = { [expectedKey]: actualCommand.sort.get(expectedKey) }; } + if (expectedCommand.createIndexes) { + actualCommand.indexes = actualCommand.indexes.map((dbIndex) => ({ + ...dbIndex, + key: Object.fromEntries(dbIndex.key.entries()) //[expectedKey]: actualCommand.createIndexes.get(expectedKey) + })); + } + expect(actualCommand).withSessionData(savedSessionData).to.matchMongoSpec(expectedCommand); } } @@ -505,6 +512,13 @@ function normalizeCommandShapes(commands) { if (def.command.sort) { output.command.sort = def.command.sort; } + if (def.command.createIndexes) { + for (const [i, dbIndex] of output.command.indexes.entries()) { + dbIndex.key = def.command.indexes[i].key; + } + + //output.command.createIndexes. + } return output; }); } diff --git a/test/tools/unified-spec-runner/match.ts b/test/tools/unified-spec-runner/match.ts index f860d8fbed..68921b2a9b 100644 --- a/test/tools/unified-spec-runner/match.ts +++ b/test/tools/unified-spec-runner/match.ts @@ -166,6 +166,8 @@ export function resultCheck( expect(actual[key]).to.have.all.keys(expectedSortKey); const objFromActual = { [expectedSortKey]: actual[key].get(expectedSortKey) }; resultCheck(objFromActual, value, entities, path, checkExtraKeys); + } else if (key === 'key') { + actual.key = Object.fromEntries(actual.key.entries()); } else { resultCheck(actual[key], value, entities, path, checkExtraKeys); } diff --git a/test/tools/unified-spec-runner/operations.ts b/test/tools/unified-spec-runner/operations.ts index 6062011568..031e7af6bb 100644 --- a/test/tools/unified-spec-runner/operations.ts +++ b/test/tools/unified-spec-runner/operations.ts @@ -76,6 +76,19 @@ operations.set('assertIndexNotExists', async ({ operation, client }) => { const collection = client .db(operation.arguments.databaseName) .collection(operation.arguments.collectionName); + + const listIndexCursor = collection.listIndexes(); + let indexes; + try { + indexes = await listIndexCursor.toArray(); + } catch (error) { + if (error.code === 26 || error.message.includes('ns does not exist')) { + return; + } + } + //.catch(error => (error.code === 26 || error.message.includes('ns does not exist') ? [] : null)); + expect(indexes.map(({ name }) => name)).to.not.include(operation.arguments.indexName); + /* try { expect(await collection.indexExists(operation.arguments.indexName)).to.be.true; } catch (error) { @@ -83,7 +96,7 @@ operations.set('assertIndexNotExists', async ({ operation, client }) => { return; } throw error; - } + } */ }); operations.set('assertDifferentLsidOnLastTwoCommands', async ({ entities, operation }) => { From 453679b7d9d1800aedfe415f5048f8fb5ef2100b Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Fri, 15 Jul 2022 17:12:40 -0400 Subject: [PATCH 12/29] fix --- test/tools/spec-runner/index.js | 5 +++++ test/tools/unified-spec-runner/match.ts | 5 ++++- test/tools/unified-spec-runner/operations.ts | 10 ---------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/test/tools/spec-runner/index.js b/test/tools/spec-runner/index.js index dd9d199cda..c6332dca32 100644 --- a/test/tools/spec-runner/index.js +++ b/test/tools/spec-runner/index.js @@ -486,6 +486,11 @@ function validateExpectations(commandEvents, spec, savedSessionData) { } if (expectedCommand.createIndexes) { + for (const index of actualCommand.indexes) { + expect(index.key).to.have.lengthOf(1); + expect(index.key).to.be.instanceOf(Map); + expect(index.key.size).to.equal(1); + } actualCommand.indexes = actualCommand.indexes.map((dbIndex) => ({ ...dbIndex, key: Object.fromEntries(dbIndex.key.entries()) //[expectedKey]: actualCommand.createIndexes.get(expectedKey) diff --git a/test/tools/unified-spec-runner/match.ts b/test/tools/unified-spec-runner/match.ts index 68921b2a9b..f8d32fdf20 100644 --- a/test/tools/unified-spec-runner/match.ts +++ b/test/tools/unified-spec-runner/match.ts @@ -167,7 +167,10 @@ export function resultCheck( const objFromActual = { [expectedSortKey]: actual[key].get(expectedSortKey) }; resultCheck(objFromActual, value, entities, path, checkExtraKeys); } else if (key === 'key') { - actual.key = Object.fromEntries(actual.key.entries()); + expect(Object.keys(value)).to.have.lengthOf(1); + expect(actual[key]).to.be.instanceOf(Map); + expect(actual[key].size).to.equal(1); + resultCheck(Object.fromEntries(actual.key.entries()), value, entities, path, checkExtraKeys); } else { resultCheck(actual[key], value, entities, path, checkExtraKeys); } diff --git a/test/tools/unified-spec-runner/operations.ts b/test/tools/unified-spec-runner/operations.ts index 031e7af6bb..ff6c62136b 100644 --- a/test/tools/unified-spec-runner/operations.ts +++ b/test/tools/unified-spec-runner/operations.ts @@ -86,17 +86,7 @@ operations.set('assertIndexNotExists', async ({ operation, client }) => { return; } } - //.catch(error => (error.code === 26 || error.message.includes('ns does not exist') ? [] : null)); expect(indexes.map(({ name }) => name)).to.not.include(operation.arguments.indexName); - /* - try { - expect(await collection.indexExists(operation.arguments.indexName)).to.be.true; - } catch (error) { - if (error.code === 26 || error.message.includes('ns does not exist')) { - return; - } - throw error; - } */ }); operations.set('assertDifferentLsidOnLastTwoCommands', async ({ entities, operation }) => { From 91910ee35595a0fff1b4c527ae2492bf8355311b Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Mon, 18 Jul 2022 11:48:52 -0400 Subject: [PATCH 13/29] fixed key === 'key' conditional --- test/tools/spec-runner/index.js | 6 ++---- test/tools/unified-spec-runner/match.ts | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/test/tools/spec-runner/index.js b/test/tools/spec-runner/index.js index c6332dca32..ffadf2be77 100644 --- a/test/tools/spec-runner/index.js +++ b/test/tools/spec-runner/index.js @@ -491,9 +491,9 @@ function validateExpectations(commandEvents, spec, savedSessionData) { expect(index.key).to.be.instanceOf(Map); expect(index.key.size).to.equal(1); } - actualCommand.indexes = actualCommand.indexes.map((dbIndex) => ({ + actualCommand.indexes = actualCommand.indexes.map(dbIndex => ({ ...dbIndex, - key: Object.fromEntries(dbIndex.key.entries()) //[expectedKey]: actualCommand.createIndexes.get(expectedKey) + key: Object.fromEntries(dbIndex.key.entries()) })); } @@ -521,8 +521,6 @@ function normalizeCommandShapes(commands) { for (const [i, dbIndex] of output.command.indexes.entries()) { dbIndex.key = def.command.indexes[i].key; } - - //output.command.createIndexes. } return output; }); diff --git a/test/tools/unified-spec-runner/match.ts b/test/tools/unified-spec-runner/match.ts index f8d32fdf20..45a77854ec 100644 --- a/test/tools/unified-spec-runner/match.ts +++ b/test/tools/unified-spec-runner/match.ts @@ -166,9 +166,8 @@ export function resultCheck( expect(actual[key]).to.have.all.keys(expectedSortKey); const objFromActual = { [expectedSortKey]: actual[key].get(expectedSortKey) }; resultCheck(objFromActual, value, entities, path, checkExtraKeys); - } else if (key === 'key') { + } else if (key === 'key' && key in actual && actual[key] instanceof Map) { expect(Object.keys(value)).to.have.lengthOf(1); - expect(actual[key]).to.be.instanceOf(Map); expect(actual[key].size).to.equal(1); resultCheck(Object.fromEntries(actual.key.entries()), value, entities, path, checkExtraKeys); } else { From 572a8c758a8800635d45dab0758910079acc1fa7 Mon Sep 17 00:00:00 2001 From: aditi-khare-mongoDB Date: Mon, 18 Jul 2022 15:37:01 -0400 Subject: [PATCH 14/29] Fixed build failures hopefully --- test/tools/spec-runner/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/tools/spec-runner/index.js b/test/tools/spec-runner/index.js index ffadf2be77..98153d92cb 100644 --- a/test/tools/spec-runner/index.js +++ b/test/tools/spec-runner/index.js @@ -486,10 +486,10 @@ function validateExpectations(commandEvents, spec, savedSessionData) { } if (expectedCommand.createIndexes) { - for (const index of actualCommand.indexes) { - expect(index.key).to.have.lengthOf(1); - expect(index.key).to.be.instanceOf(Map); - expect(index.key.size).to.equal(1); + for (const [i, dbIndex] of actualCommand.indexes.entries()) { + expect(Object.keys(expectedCommand.indexes[i].key)).to.have.lengthOf(1); + expect(dbIndex.key).to.be.instanceOf(Map); + expect(dbIndex.key.size).to.equal(1); } actualCommand.indexes = actualCommand.indexes.map(dbIndex => ({ ...dbIndex, From 3a017d1ae5636a531f08176b1e72f18a979b9b10 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 18 Jul 2022 18:45:35 -0400 Subject: [PATCH 15/29] rebase fixup --- test/unit/{utils.test.js => utils.test.ts} | 78 ++-------------------- 1 file changed, 5 insertions(+), 73 deletions(-) rename test/unit/{utils.test.js => utils.test.ts} (89%) diff --git a/test/unit/utils.test.js b/test/unit/utils.test.ts similarity index 89% rename from test/unit/utils.test.js rename to test/unit/utils.test.ts index 6d68b94624..f41673d038 100644 --- a/test/unit/utils.test.js +++ b/test/unit/utils.test.ts @@ -1,11 +1,14 @@ -'use strict'; import { expect } from 'chai'; +import * as sinon from 'sinon'; +import { LEGACY_HELLO_COMMAND } from '../../src/constants'; +import { MongoRuntimeError } from '../../src/error'; import { + BufferPool, eachAsync, + isHello, makeInterruptibleAsyncInterval, MongoDBNamespace, - parseIndexOptions, shuffle } from '../../src/utils'; import { createTimerSandbox } from './timer_sandbox'; @@ -582,75 +585,4 @@ describe('driver utils', function () { }); }); }); - - 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); - }); - } - }); }); From e9f6edb7700effba139862ce474ded3587207f3e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 19 Jul 2022 13:09:07 -0400 Subject: [PATCH 16/29] wip --- .../client_side_encryption.prose.test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/integration/client-side-encryption/client_side_encryption.prose.test.js b/test/integration/client-side-encryption/client_side_encryption.prose.test.js index 3f6cdc960c..66f2efdc06 100644 --- a/test/integration/client-side-encryption/client_side_encryption.prose.test.js +++ b/test/integration/client-side-encryption/client_side_encryption.prose.test.js @@ -729,6 +729,21 @@ describe('Client Side Encryption Prose Tests', metadata, function () { }); }); + it('should maintain an ordered sort', async function () { + const coll = this.encryptedColl; + const events = []; + this.clientEncrypted.on('commandStarted', ev => events.push(ev)); + const sort = new Map([ + ['1', 1], + ['0', 1] + ]); + await coll.findOne({}, { sort }); + expect(events).to.have.lengthOf(2); + expect(events[0]).to.have.property('commandName', 'listCollections'); + expect(events[1]).to.have.property('commandName', 'find'); + expect(events[1].command.sort).to.deep.equal(sort); + }); + function pruneEvents(events) { return events.map(event => { // We are pruning out the bunch of repeating As, mostly From 8bf8b113c40906d83ab1d600fedc9206500ce9f5 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 20 Jul 2022 11:47:54 -0400 Subject: [PATCH 17/29] fix: preserve key order in encryption scenarios --- src/cmap/connection.ts | 16 +++++ .../client_side_encryption.prose.test.js | 15 ---- .../client-side-encryption/driver.test.js | 68 +++++++++++++++++++ 3 files changed, 84 insertions(+), 15 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 508a5c3c0f..f304765a41 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -594,11 +594,27 @@ export class CryptoConnection extends Connection { return; } + const sort: Map | null = cmd.find || cmd.findAndModify ? cmd.sort : null; + const indexKeys: Map[] | null = cmd.createIndexes + ? cmd.indexes.map((index: { key: Map }) => index.key) + : null; + autoEncrypter.encrypt(ns.toString(), cmd, options, (err, encrypted) => { if (err || encrypted == null) { callback(err, null); return; } + + // Repair JS Map loss + if ((cmd.find || cmd.findAndModify) && sort != null) { + encrypted.sort = sort; + } + if (cmd.createIndexes && indexKeys != null) { + for (const [offset, index] of indexKeys.entries()) { + encrypted.indexes[offset].key = index; + } + } + super.command(ns, encrypted, options, (err, response) => { if (err || response == null) { callback(err, response); diff --git a/test/integration/client-side-encryption/client_side_encryption.prose.test.js b/test/integration/client-side-encryption/client_side_encryption.prose.test.js index 66f2efdc06..3f6cdc960c 100644 --- a/test/integration/client-side-encryption/client_side_encryption.prose.test.js +++ b/test/integration/client-side-encryption/client_side_encryption.prose.test.js @@ -729,21 +729,6 @@ describe('Client Side Encryption Prose Tests', metadata, function () { }); }); - it('should maintain an ordered sort', async function () { - const coll = this.encryptedColl; - const events = []; - this.clientEncrypted.on('commandStarted', ev => events.push(ev)); - const sort = new Map([ - ['1', 1], - ['0', 1] - ]); - await coll.findOne({}, { sort }); - expect(events).to.have.lengthOf(2); - expect(events[0]).to.have.property('commandName', 'listCollections'); - expect(events[1]).to.have.property('commandName', 'find'); - expect(events[1].command.sort).to.deep.equal(sort); - }); - function pruneEvents(events) { return events.map(event => { // We are pruning out the bunch of repeating As, mostly diff --git a/test/integration/client-side-encryption/driver.test.js b/test/integration/client-side-encryption/driver.test.js index 4d62a70bae..1d6f5b84e3 100644 --- a/test/integration/client-side-encryption/driver.test.js +++ b/test/integration/client-side-encryption/driver.test.js @@ -222,4 +222,72 @@ describe('Client Side Encryption Functional', function () { }); }); }); + + describe('key order aware command properties', () => { + let client; + let collection; + + beforeEach(async function () { + const encryptionOptions = { + monitorCommands: true, + autoEncryption: { + keyVaultNamespace, + kmsProviders: { local: { key: 'A'.repeat(128) } } + } + }; + client = this.configuration.newClient({}, encryptionOptions); + collection = client.db(dataDbName).collection('keyOrder'); + }); + + afterEach(async () => { + if (client) await client.close(); + }); + + describe('find', () => { + it('should maintain ordered sort', async function () { + const events = []; + client.on('commandStarted', ev => events.push(ev)); + const sort = new Map([ + ['1', 1], + ['0', 1] + ]); + await collection.findOne({}, { sort }); + const findEvent = events.find(event => !!event.command.find); + expect(findEvent).to.have.property('commandName', 'find'); + expect(findEvent.command.sort).to.deep.equal(sort); + }); + }); + + describe('findAndModify', () => { + it('should maintain ordered sort', async function () { + const events = []; + client.on('commandStarted', ev => events.push(ev)); + const sort = new Map([ + ['1', 1], + ['0', 1] + ]); + await collection.findOneAndUpdate({}, { $setOnInsert: { a: 1 } }, { sort }); + const findAndModifyEvent = events.find(event => !!event.command.findAndModify); + expect(findAndModifyEvent).to.have.property('commandName', 'findAndModify'); + expect(findAndModifyEvent.command.sort).to.deep.equal(sort); + }); + }); + + describe('createIndexes', () => { + it('should maintain ordered index keys', async function () { + const events = []; + client.on('commandStarted', ev => events.push(ev)); + const indexDescription = new Map([ + ['1', 1], + ['0', 1] + ]); + await collection.createIndex(indexDescription, { name: 'myIndex' }); + const createIndexEvent = events.find(event => !!event.command.createIndexes); + expect(createIndexEvent).to.have.property('commandName', 'createIndexes'); + expect(createIndexEvent.command.indexes).to.have.lengthOf(1); + const index = createIndexEvent.command.indexes[0]; + expect(index.key).to.deep.equal(indexDescription); + }); + }); + }); }); From eb78d8e3fd91a7e262cd927f22c65e3b3abdc5d1 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 20 Jul 2022 13:42:54 -0400 Subject: [PATCH 18/29] fixups --- src/operations/indexes.ts | 91 ++++++++------------ test/tools/unified-spec-runner/match.ts | 14 ++- test/tools/unified-spec-runner/operations.ts | 2 + test/unit/operations/indexes.test.ts | 15 ++++ 4 files changed, 61 insertions(+), 61 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 886537e46c..56162eeff7 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -137,54 +137,39 @@ function isSingleIndexTuple(t: unknown): t is [string, IndexDirection] { return Array.isArray(t) && t.length === 2 && isIndexDirection(t[1]); } -function makeIndexSpec(indexSpec: IndexSpecification, options: any): IndexDescription { - function getFieldHash(indexSpec: IndexSpecification) { - const fieldHash: Map = new Map(); - - let indexSpecArr: IndexSpecification[]; - - // Wrap indexSpec in array if needed - if (!Array.isArray(indexSpec) || isSingleIndexTuple(indexSpec)) { - indexSpecArr = [indexSpec]; - } else { - indexSpecArr = indexSpec; - } - - // 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] ?? 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); - } +function makeIndexSpec( + indexSpec: IndexSpecification, + options?: CreateIndexesOptions +): IndexDescription { + const key: Map = new Map(); + + const indexSpecs = + !Array.isArray(indexSpec) || isSingleIndexTuple(indexSpec) ? [indexSpec] : indexSpec; + + // Iterate through array and handle different types + for (const spec of indexSpecs) { + if (typeof spec === 'string') { + key.set(spec, 1); + } else if (Array.isArray(spec)) { + key.set(spec[0], spec[1] ?? 1); + } else if (spec instanceof Map) { + for (const [property, value] of spec) { + key.set(property, value); + } + } else if (isObject(spec)) { + for (const [property, value] of Object.entries(spec)) { + key.set(property, value); } } - return fieldHash; } - const fieldHash = getFieldHash(indexSpec); - - // Generate the index name - const name = typeof options.name === 'string' ? options.name : null; - // Set up the index - const finalIndexSpec: Document = { name, key: fieldHash }; - - // merge valid index options into the index spec - for (const optionName in options) { - if (VALID_INDEX_OPTIONS.has(optionName)) { - finalIndexSpec[optionName] = options[optionName]; - } - } - - return finalIndexSpec as IndexDescription; + return { + ...Object.fromEntries( + Object.entries({ ...options }).filter(([optionName]) => VALID_INDEX_OPTIONS.has(optionName)) + ), + key + }; } /** @internal */ @@ -235,27 +220,24 @@ export class CreateIndexesOperation< this.collectionName = collectionName; // Ensure we generate the correct name if the parameter is not set - const normalizedIndexes = []; + const namedIndexes = []; 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 + // Ensure the key is a Map to preserve index key ordering + key: userIndex.key instanceof Map ? userIndex.key : new Map(Object.entries(userIndex.key)) }; if (index.name == null) { + // Ensure every index is named const keys = []; - for (const [name, direction] of index.key) { keys.push(`${name}_${direction}`); } - - // Set the name index.name = keys.join('_'); } - normalizedIndexes.push(index); + namedIndexes.push(index); } - this.indexes = normalizedIndexes; + this.indexes = namedIndexes; } override execute( @@ -318,11 +300,6 @@ export class CreateIndexOperation extends CreateIndexesOperation { indexSpec: IndexSpecification, options?: CreateIndexesOptions ) { - // createIndex can be called with a variety of styles: - // coll.createIndex('a'); - // coll.createIndex({ a: 1 }); - // coll.createIndex([['a', 1]]); - // createIndexes is always called with an array of index spec objects super(parent, collectionName, [makeIndexSpec(indexSpec, options)], options); } override execute( diff --git a/test/tools/unified-spec-runner/match.ts b/test/tools/unified-spec-runner/match.ts index 45a77854ec..a085e5f445 100644 --- a/test/tools/unified-spec-runner/match.ts +++ b/test/tools/unified-spec-runner/match.ts @@ -166,10 +166,16 @@ export function resultCheck( expect(actual[key]).to.have.all.keys(expectedSortKey); const objFromActual = { [expectedSortKey]: actual[key].get(expectedSortKey) }; resultCheck(objFromActual, value, entities, path, checkExtraKeys); - } else if (key === 'key' && key in actual && actual[key] instanceof Map) { - expect(Object.keys(value)).to.have.lengthOf(1); - expect(actual[key].size).to.equal(1); - resultCheck(Object.fromEntries(actual.key.entries()), value, entities, path, checkExtraKeys); + } else if (key === 'createIndexes') { + for (const index of actual.indexes) { + expect(index).to.have.property('key').that.is.instanceOf(Map); + expect( + index.key.size, + 'Test input is JSON and cannot correctly test more than 1 key' + ).to.equal(1); + index.key = Object.fromEntries(index.key.entries()); + } + resultCheck(actual[key], value, entities, path, checkExtraKeys); } else { resultCheck(actual[key], value, entities, path, checkExtraKeys); } diff --git a/test/tools/unified-spec-runner/operations.ts b/test/tools/unified-spec-runner/operations.ts index ff6c62136b..7733dcb7c2 100644 --- a/test/tools/unified-spec-runner/operations.ts +++ b/test/tools/unified-spec-runner/operations.ts @@ -85,6 +85,8 @@ operations.set('assertIndexNotExists', async ({ operation, client }) => { if (error.code === 26 || error.message.includes('ns does not exist')) { return; } + // Error will always exist here, this makes the output show what caused an issue with assertIndexNotExists + expect(error).to.not.exist; } expect(indexes.map(({ name }) => name)).to.not.include(operation.arguments.indexName); }); diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts index c1d2945430..97293f0711 100644 --- a/test/unit/operations/indexes.test.ts +++ b/test/unit/operations/indexes.test.ts @@ -125,4 +125,19 @@ describe('makeIndexSpec()', () => { expect(realOutput.indexes[0].key).to.deep.equal(desiredMapData); expect(realOutput.indexes[0].name).to.equal(desiredName); }); + + it('should omit options that are not in the permitted list', () => { + const indexOutput = makeIndexOperation( + { a: 1 }, + // @ts-expect-error: Testing bad options get filtered + { randomOptionThatWillNeverBeAdded: true, storageEngine: { iLoveJavascript: 1 } } + ); + expect(indexOutput.indexes).to.have.lengthOf(1); + expect(indexOutput.indexes[0]).to.have.property('key').that.is.instanceOf(Map); + expect(indexOutput.indexes[0]).to.have.property('name', 'a_1'); + expect(indexOutput.indexes[0]) + .to.have.property('storageEngine') + .that.deep.equals({ iLoveJavascript: 1 }); + expect(indexOutput.indexes[0]).to.not.have.property('randomOptionThatWillNeverBeAdded'); + }); }); From 79babb2e24908bf20d96859b45647d7b7fcae845 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 20 Jul 2022 14:32:01 -0400 Subject: [PATCH 19/29] test: handle Int32s --- .../client-side-encryption/driver.test.js | 23 +++++++++++-------- test/tools/spec-runner/matcher.js | 4 ++++ test/tools/unified-spec-runner/runner.ts | 6 ++--- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/test/integration/client-side-encryption/driver.test.js b/test/integration/client-side-encryption/driver.test.js index 1d6f5b84e3..2097dffa80 100644 --- a/test/integration/client-side-encryption/driver.test.js +++ b/test/integration/client-side-encryption/driver.test.js @@ -7,6 +7,13 @@ const chai = require('chai'); const expect = chai.expect; chai.use(require('chai-subset')); +const metadata = { + requires: { + mongodb: '>=4.2.0', + clientSideEncryption: true + } +}; + describe('Client Side Encryption Functional', function () { const dataDbName = 'db'; const dataCollName = 'coll'; @@ -14,13 +21,6 @@ describe('Client Side Encryption Functional', function () { const keyVaultCollName = 'datakeys'; const keyVaultNamespace = `${keyVaultDbName}.${keyVaultCollName}`; - const metadata = { - requires: { - mongodb: '>=4.2.0', - clientSideEncryption: true - } - }; - it('CSFLE_KMS_PROVIDERS should be valid EJSON', function () { if (process.env.CSFLE_KMS_PROVIDERS) { /** @@ -228,6 +228,9 @@ describe('Client Side Encryption Functional', function () { let collection; beforeEach(async function () { + if (this.configuration.clientSideEncryption == null) { + return; + } const encryptionOptions = { monitorCommands: true, autoEncryption: { @@ -244,7 +247,7 @@ describe('Client Side Encryption Functional', function () { }); describe('find', () => { - it('should maintain ordered sort', async function () { + it('should maintain ordered sort', metadata, async function () { const events = []; client.on('commandStarted', ev => events.push(ev)); const sort = new Map([ @@ -259,7 +262,7 @@ describe('Client Side Encryption Functional', function () { }); describe('findAndModify', () => { - it('should maintain ordered sort', async function () { + it('should maintain ordered sort', metadata, async function () { const events = []; client.on('commandStarted', ev => events.push(ev)); const sort = new Map([ @@ -274,7 +277,7 @@ describe('Client Side Encryption Functional', function () { }); describe('createIndexes', () => { - it('should maintain ordered index keys', async function () { + it('should maintain ordered index keys', metadata, async function () { const events = []; client.on('commandStarted', ev => events.push(ev)); const indexDescription = new Map([ diff --git a/test/tools/spec-runner/matcher.js b/test/tools/spec-runner/matcher.js index 757b049d4a..6ea9b5a0cd 100644 --- a/test/tools/spec-runner/matcher.js +++ b/test/tools/spec-runner/matcher.js @@ -118,6 +118,10 @@ function generateMatchAndDiffSpecialCase(key, expectedObj, actualObj, metadata) function generateMatchAndDiff(expected, actual, metadata) { const typeOfExpected = typeof expected; + if (typeOfExpected === 'object' && expected._bsontype === 'Int32' && typeof actual === 'number') { + return { match: expected.value === actual, expected, actual }; + } + if (typeOfExpected !== typeof actual) { return { match: false, expected, actual }; } diff --git a/test/tools/unified-spec-runner/runner.ts b/test/tools/unified-spec-runner/runner.ts index a5bed8e474..5f37f3467f 100644 --- a/test/tools/unified-spec-runner/runner.ts +++ b/test/tools/unified-spec-runner/runner.ts @@ -40,7 +40,7 @@ async function terminateOpenTransactions(client: MongoClient) { * @param skipFilter - a function that returns null if the test should be run, * or a skip reason if the test should be skipped */ -export async function runUnifiedTest( +async function runUnifiedTest( ctx: Mocha.Context, unifiedSuite: uni.UnifiedSuite, test: uni.Test, @@ -256,8 +256,8 @@ export function runUnifiedSuite( ): void { for (const unifiedSuite of specTests) { context(String(unifiedSuite.description), function () { - for (const test of unifiedSuite.tests) { - it(String(test.description), async function () { + for (const [index, test] of unifiedSuite.tests.entries()) { + it(String(test.description === '' ? `Test ${index}` : test.description), async function () { await runUnifiedTest(this, unifiedSuite, test, skipFilter); }); } From 02c5a1eff0fccd879a7918343cbee02023f78af0 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 21 Jul 2022 14:08:01 -0400 Subject: [PATCH 20/29] clean up fle testing --- .../{driver.test.js => driver.test.ts} | 135 +++++++++--------- test/tools/unified-spec-runner/schema.ts | 2 +- 2 files changed, 68 insertions(+), 69 deletions(-) rename test/integration/client-side-encryption/{driver.test.js => driver.test.ts} (71%) diff --git a/test/integration/client-side-encryption/driver.test.js b/test/integration/client-side-encryption/driver.test.ts similarity index 71% rename from test/integration/client-side-encryption/driver.test.js rename to test/integration/client-side-encryption/driver.test.ts index 2097dffa80..fe0e8ed081 100644 --- a/test/integration/client-side-encryption/driver.test.js +++ b/test/integration/client-side-encryption/driver.test.ts @@ -1,11 +1,17 @@ -'use strict'; +import { EJSON, UUID } from 'bson'; +import { expect } from 'chai'; +import * as crypto from 'crypto'; -const crypto = require('crypto'); -const BSON = require('bson'); -const chai = require('chai'); - -const expect = chai.expect; -chai.use(require('chai-subset')); +import { + Collection, + CommandStartedEvent, + Db, + IndexDirection, + MongoClient, + SortDirection +} from '../../../src'; +import * as BSON from '../../../src/bson'; +import { ClientEncryption } from '../../tools/unified-spec-runner/schema'; const metadata = { requires: { @@ -22,10 +28,12 @@ describe('Client Side Encryption Functional', function () { const keyVaultNamespace = `${keyVaultDbName}.${keyVaultCollName}`; it('CSFLE_KMS_PROVIDERS should be valid EJSON', function () { - if (process.env.CSFLE_KMS_PROVIDERS) { + const CSFLE_KMS_PROVIDERS = process.env.CSFLE_KMS_PROVIDERS; + if (typeof CSFLE_KMS_PROVIDERS === 'string') { /** * The shape of CSFLE_KMS_PROVIDERS is as follows: * + * ```ts * interface CSFLE_kms_providers { * aws: { * accessKeyId: string; @@ -46,8 +54,9 @@ describe('Client Side Encryption Functional', function () { * key: Binary; * } * } + * ``` */ - expect(() => BSON.EJSON.parse(process.env.CSFLE_KMS_PROVIDERS)).to.not.throw(SyntaxError); + expect(() => EJSON.parse(CSFLE_KMS_PROVIDERS)).to.not.throw(SyntaxError); } else { this.skip(); } @@ -56,7 +65,7 @@ describe('Client Side Encryption Functional', function () { describe('Collection', metadata, function () { describe('#bulkWrite()', metadata, function () { context('when encryption errors', function () { - let client; + let client: MongoClient; beforeEach(function () { client = this.configuration.newClient( @@ -74,7 +83,7 @@ describe('Client Side Encryption Functional', function () { fields: [ { path: 'ssn', - keyId: new BSON.UUID('23f786b4-1d39-4c36-ae88-70a663321ec9').toBinary(), + keyId: new UUID('23f786b4-1d39-4c36-ae88-70a663321ec9').toBinary(), bsonType: 'string' } ] @@ -94,6 +103,7 @@ describe('Client Side Encryption Functional', function () { await client .db('test') .collection('coll') + // @ts-expect-error: Incorrectly formatted bulkWrite to test error case .bulkWrite([{ insertOne: { ssn: 'foo' } }]); expect.fail('expected error to be thrown'); } catch (error) { @@ -105,10 +115,12 @@ describe('Client Side Encryption Functional', function () { }); describe('BSON Options', function () { + let client: MongoClient; + beforeEach(function () { - this.client = this.configuration.newClient(); + client = this.configuration.newClient(); - const noop = () => {}; + const noop = () => null; function encryptSchema(keyId, bsonType) { return { encrypt: { @@ -119,13 +131,13 @@ describe('Client Side Encryption Functional', function () { }; } - let encryption; - let dataDb; - let keyVaultDb; + let encryption: ClientEncryption; + let dataDb: Db; + let keyVaultDb: Db; const mongodbClientEncryption = this.configuration.mongodbClientEncryption; const kmsProviders = this.configuration.kmsProviders(crypto.randomBytes(96)); - return this.client + return client .connect() .then(() => { encryption = new mongodbClientEncryption.ClientEncryption(this.client, { @@ -134,8 +146,8 @@ describe('Client Side Encryption Functional', function () { kmsProviders }); }) - .then(() => (dataDb = this.client.db(dataDbName))) - .then(() => (keyVaultDb = this.client.db(keyVaultDbName))) + .then(() => (dataDb = client.db(dataDbName))) + .then(() => (keyVaultDb = client.db(keyVaultDbName))) .then(() => dataDb.dropCollection(dataCollName).catch(noop)) .then(() => keyVaultDb.dropCollection(keyVaultCollName).catch(noop)) .then(() => keyVaultDb.createCollection(keyVaultCollName)) @@ -157,12 +169,7 @@ describe('Client Side Encryption Functional', function () { .then(() => { this.encryptedClient = this.configuration.newClient( {}, - { - autoEncryption: { - keyVaultNamespace, - kmsProviders - } - } + { autoEncryption: { keyVaultNamespace, kmsProviders } } ); return this.encryptedClient.connect(); }); @@ -176,33 +183,20 @@ describe('Client Side Encryption Functional', function () { const testCases = [ {}, - { - promoteValues: true - }, - { - promoteValues: false - }, - { - promoteValues: true, - promoteLongs: false - }, - { - promoteValues: true, - promoteLongs: true - }, - { - bsonRegExp: true - }, - { - ignoreUndefined: true - } + { promoteValues: true }, + { promoteValues: false }, + { promoteValues: true, promoteLongs: false }, + { promoteValues: true, promoteLongs: true }, + { bsonRegExp: true }, + { ignoreUndefined: true } ]; - testCases.forEach(bsonOptions => { + for (const bsonOptions of testCases) { const name = `should respect bson options ${JSON.stringify(bsonOptions)}`; - it(name, metadata, function () { + it(name, metadata, async function () { const data = { + _id: new BSON.ObjectId(), a: 12, b: new BSON.Int32(12), c: new BSON.Long(12), @@ -215,17 +209,20 @@ describe('Client Side Encryption Functional', function () { const expected = BSON.deserialize(BSON.serialize(data, bsonOptions), bsonOptions); const coll = this.encryptedClient.db(dataDbName).collection(dataCollName); - return Promise.resolve() - .then(() => coll.insertOne(data, bsonOptions)) - .then(result => coll.findOne({ _id: result.insertedId }, bsonOptions)) - .then(actual => expect(actual).to.containSubset(expected)); + const result = await coll.insertOne(data, bsonOptions); + const actual = await coll.findOne({ _id: result.insertedId }, bsonOptions); + const gValue = actual.g; + delete actual.g; + + expect(actual).to.deep.equal(expected); + expect(gValue).to.equal(bsonOptions.ignoreUndefined ? data.g : null); }); - }); + } }); describe('key order aware command properties', () => { - let client; - let collection; + let client: MongoClient; + let collection: Collection; beforeEach(async function () { if (this.configuration.clientSideEncryption == null) { @@ -248,48 +245,50 @@ describe('Client Side Encryption Functional', function () { describe('find', () => { it('should maintain ordered sort', metadata, async function () { - const events = []; + const events: CommandStartedEvent[] = []; client.on('commandStarted', ev => events.push(ev)); - const sort = new Map([ + const sort: [string, SortDirection][] = [ ['1', 1], ['0', 1] - ]); + ]; await collection.findOne({}, { sort }); const findEvent = events.find(event => !!event.command.find); expect(findEvent).to.have.property('commandName', 'find'); - expect(findEvent.command.sort).to.deep.equal(sort); + expect(findEvent).to.have.nested.property('command.sort').deep.equal(new Map(sort)); }); }); describe('findAndModify', () => { it('should maintain ordered sort', metadata, async function () { - const events = []; + const events: CommandStartedEvent[] = []; client.on('commandStarted', ev => events.push(ev)); - const sort = new Map([ + const sort: [string, SortDirection][] = [ ['1', 1], ['0', 1] - ]); + ]; await collection.findOneAndUpdate({}, { $setOnInsert: { a: 1 } }, { sort }); const findAndModifyEvent = events.find(event => !!event.command.findAndModify); expect(findAndModifyEvent).to.have.property('commandName', 'findAndModify'); - expect(findAndModifyEvent.command.sort).to.deep.equal(sort); + expect(findAndModifyEvent) + .to.have.nested.property('command.sort') + .deep.equal(new Map(sort)); }); }); describe('createIndexes', () => { it('should maintain ordered index keys', metadata, async function () { - const events = []; + const events: CommandStartedEvent[] = []; client.on('commandStarted', ev => events.push(ev)); - const indexDescription = new Map([ + const indexDescription: [string, IndexDirection][] = [ ['1', 1], ['0', 1] - ]); + ]; await collection.createIndex(indexDescription, { name: 'myIndex' }); const createIndexEvent = events.find(event => !!event.command.createIndexes); expect(createIndexEvent).to.have.property('commandName', 'createIndexes'); - expect(createIndexEvent.command.indexes).to.have.lengthOf(1); - const index = createIndexEvent.command.indexes[0]; - expect(index.key).to.deep.equal(indexDescription); + expect(createIndexEvent).to.have.nested.property('command.indexes').that.has.lengthOf(1); + const index = createIndexEvent?.command.indexes[0]; + expect(index.key).to.deep.equal(new Map(indexDescription)); }); }); }); diff --git a/test/tools/unified-spec-runner/schema.ts b/test/tools/unified-spec-runner/schema.ts index 629753a303..af4033f5bd 100644 --- a/test/tools/unified-spec-runner/schema.ts +++ b/test/tools/unified-spec-runner/schema.ts @@ -299,7 +299,7 @@ export type TestFilter = (test: Test, ctx: TestConfiguration) => string | false; export interface ClientEncryption { // eslint-disable-next-line @typescript-eslint/no-misused-new new (client: MongoClient, options: any): ClientEncryption; - createDataKey(provider, options): Promise; + createDataKey(provider, options?: Document): Promise; rewrapManyDataKey(filter, options): Promise; deleteKey(id): Promise; getKey(id): Promise; From 7b3f2fdd0c1762b4c47e2f08cf96f0107a77f118 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 22 Jul 2022 14:05:16 -0400 Subject: [PATCH 21/29] chore: comments --- src/cmap/connection.ts | 12 +++++++++--- test/tools/spec-runner/index.js | 4 ++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index f304765a41..1647638045 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -594,6 +594,12 @@ export class CryptoConnection extends Connection { return; } + // Save sort or indexKeys based on the command being run + // the encrypt API serializes our JS objects to BSON to pass to the native code layer + // and then deserializes the encrypted result, the protocol level components + // of the command (ex. sort) are then converted to JS objects potentially losing + // import key order information. These fields are never encrypted so we can save the values + // from before the encryption and replace them after encryption has been performed const sort: Map | null = cmd.find || cmd.findAndModify ? cmd.sort : null; const indexKeys: Map[] | null = cmd.createIndexes ? cmd.indexes.map((index: { key: Map }) => index.key) @@ -605,11 +611,11 @@ export class CryptoConnection extends Connection { return; } - // Repair JS Map loss - if ((cmd.find || cmd.findAndModify) && sort != null) { + // Replace the saved values + if (sort != null && (cmd.find || cmd.findAndModify)) { encrypted.sort = sort; } - if (cmd.createIndexes && indexKeys != null) { + if (indexKeys != null && cmd.createIndexes) { for (const [offset, index] of indexKeys.entries()) { encrypted.indexes[offset].key = index; } diff --git a/test/tools/spec-runner/index.js b/test/tools/spec-runner/index.js index 98153d92cb..2ebb66dc67 100644 --- a/test/tools/spec-runner/index.js +++ b/test/tools/spec-runner/index.js @@ -486,6 +486,10 @@ function validateExpectations(commandEvents, spec, savedSessionData) { } if (expectedCommand.createIndexes) { + // TODO(NODE-3235): This is a workaround that works because all indexes 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 for (const [i, dbIndex] of actualCommand.indexes.entries()) { expect(Object.keys(expectedCommand.indexes[i].key)).to.have.lengthOf(1); expect(dbIndex.key).to.be.instanceOf(Map); From 4ee265159fd3080fc2b8dc8bfa46fa92f55db6bb Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 25 Jul 2022 16:51:50 -0400 Subject: [PATCH 22/29] fix: address comments, streamlines naming code --- src/operations/indexes.ts | 39 ++++++------ test/types/community/createIndex.test-d.ts | 13 ++++ test/unit/operations/indexes.test.ts | 70 +++++++++++----------- 3 files changed, 67 insertions(+), 55 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 56162eeff7..ad8c7a29f6 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -51,7 +51,7 @@ const VALID_INDEX_OPTIONS = new Set([ /** @public */ export type IndexDirection = -1 | 1 | '2d' | '2dsphere' | 'text' | 'geoHaystack' | number; -function isIndexDirection(x: any): x is IndexDirection { +function isIndexDirection(x: unknown): x is IndexDirection { return ( typeof x === 'number' || x === '2d' || x === '2dsphere' || x === 'text' || x === 'geoHaystack' ); @@ -172,6 +172,22 @@ function makeIndexSpec( }; } +function fillInIndexNames(indexes: IndexDescription[]) { + const namedIndexes = []; + for (const userIndex of indexes) { + const index: Omit & { key: Map } = { + ...userIndex, + // Ensure the key is a Map to preserve index key ordering + key: userIndex.key instanceof Map ? userIndex.key : new Map(Object.entries(userIndex.key)) + }; + if (index.name == null) { + index.name = Array.from(index.key).flat().join('_'); + } + namedIndexes.push(index); + } + return namedIndexes; +} + /** @internal */ export class IndexesOperation extends AbstractOperation { override options: IndexInformationOptions; @@ -218,26 +234,7 @@ export class CreateIndexesOperation< this.options = options ?? {}; this.collectionName = collectionName; - - // Ensure we generate the correct name if the parameter is not set - const namedIndexes = []; - for (const userIndex of indexes) { - const index: Omit & { key: Map } = { - ...userIndex, - // Ensure the key is a Map to preserve index key ordering - key: userIndex.key instanceof Map ? userIndex.key : new Map(Object.entries(userIndex.key)) - }; - if (index.name == null) { - // Ensure every index is named - const keys = []; - for (const [name, direction] of index.key) { - keys.push(`${name}_${direction}`); - } - index.name = keys.join('_'); - } - namedIndexes.push(index); - } - this.indexes = namedIndexes; + this.indexes = fillInIndexNames(indexes); } override execute( diff --git a/test/types/community/createIndex.test-d.ts b/test/types/community/createIndex.test-d.ts index 5462948ff9..781955f3fe 100644 --- a/test/types/community/createIndex.test-d.ts +++ b/test/types/community/createIndex.test-d.ts @@ -11,3 +11,16 @@ const indexName = collection.createIndex({}, options); expectType>(indexName); expectType(options.partialFilterExpression); + +// One +collection.createIndex('someKey'); +collection.createIndex(['someKey', 1]); +collection.createIndex(new Map([['someKey', 1]])); +collection.createIndex({ a: 1, b: -1 }); +collection.createIndex({ a: '2dsphere', b: -1 }); +// OrMore +collection.createIndex(['someKey']); +collection.createIndex([['someKey', 1]]); +collection.createIndex([new Map([['someKey', 1]])]); +collection.createIndex([{ a: 1, b: -1 }]); +collection.createIndex([{ a: '2dsphere', b: -1 }]); diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts index 97293f0711..629aeac34e 100644 --- a/test/unit/operations/indexes.test.ts +++ b/test/unit/operations/indexes.test.ts @@ -7,7 +7,7 @@ import { } from '../../../src/operations/indexes'; import { ns } from '../../../src/utils'; -describe('makeIndexSpec()', () => { +describe('class CreateIndexOperation', () => { const testCases = [ { description: 'single string', @@ -103,41 +103,43 @@ describe('makeIndexSpec()', () => { const makeIndexOperation = (input, options: CreateIndexesOptions = {}) => new CreateIndexOperation({ s: { namespace: ns('a.b') } }, 'b', input, options); - 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); - }); + describe('#constructor()', () => { + 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 correctly if none provided with ${description} input `, () => { - const realOutput = makeIndexOperation(input); - expect(realOutput.indexes[0].name).to.equal(name); - }); - } + it(`should set name correctly if none provided with ${description} input `, () => { + const realOutput = makeIndexOperation(input); + expect(realOutput.indexes[0].name).to.equal(name); + }); + } - it('should keep numerical keys in chronological ordering when using Map input type', () => { - 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); - }); + it('should keep numerical keys in chronological ordering when using Map input type', () => { + 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); + }); - it('should omit options that are not in the permitted list', () => { - const indexOutput = makeIndexOperation( - { a: 1 }, - // @ts-expect-error: Testing bad options get filtered - { randomOptionThatWillNeverBeAdded: true, storageEngine: { iLoveJavascript: 1 } } - ); - expect(indexOutput.indexes).to.have.lengthOf(1); - expect(indexOutput.indexes[0]).to.have.property('key').that.is.instanceOf(Map); - expect(indexOutput.indexes[0]).to.have.property('name', 'a_1'); - expect(indexOutput.indexes[0]) - .to.have.property('storageEngine') - .that.deep.equals({ iLoveJavascript: 1 }); - expect(indexOutput.indexes[0]).to.not.have.property('randomOptionThatWillNeverBeAdded'); + it('should omit options that are not in the permitted list', () => { + const indexOutput = makeIndexOperation( + { a: 1 }, + // @ts-expect-error: Testing bad options get filtered + { randomOptionThatWillNeverBeAdded: true, storageEngine: { iLoveJavascript: 1 } } + ); + expect(indexOutput.indexes).to.have.lengthOf(1); + expect(indexOutput.indexes[0]).to.have.property('key').that.is.instanceOf(Map); + expect(indexOutput.indexes[0]).to.have.property('name', 'a_1'); + expect(indexOutput.indexes[0]) + .to.have.property('storageEngine') + .that.deep.equals({ iLoveJavascript: 1 }); + expect(indexOutput.indexes[0]).to.not.have.property('randomOptionThatWillNeverBeAdded'); + }); }); }); From 4a1d5092e74d1fcbd7d9d1e97616b5125654e710 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 25 Jul 2022 16:56:51 -0400 Subject: [PATCH 23/29] fix: use map --- src/operations/indexes.ts | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index ad8c7a29f6..047128a0fa 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -173,19 +173,13 @@ function makeIndexSpec( } function fillInIndexNames(indexes: IndexDescription[]) { - const namedIndexes = []; - for (const userIndex of indexes) { - const index: Omit & { key: Map } = { - ...userIndex, - // Ensure the key is a Map to preserve index key ordering - key: userIndex.key instanceof Map ? userIndex.key : new Map(Object.entries(userIndex.key)) - }; - if (index.name == null) { - index.name = Array.from(index.key).flat().join('_'); - } - namedIndexes.push(index); - } - return namedIndexes; + return indexes.map(userIndex => { + // Ensure the key is a Map to preserve index key ordering + const key = + userIndex.key instanceof Map ? userIndex.key : new Map(Object.entries(userIndex.key)); + const name = userIndex.name != null ? userIndex.name : Array.from(key).flat().join('_'); + return { ...userIndex, key, name }; + }); } /** @internal */ From c770dce45c2da3f7b0decdc18e481bee08bd0518 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 25 Jul 2022 17:01:27 -0400 Subject: [PATCH 24/29] fix: wire version below 5 check no longer needed --- src/operations/indexes.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 047128a0fa..ade496c84e 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -241,19 +241,6 @@ export class CreateIndexesOperation< const serverWireVersion = maxWireVersion(server); - for (const index of indexes) { - // Did the user pass in a collation, check if our write server supports it - if (index.collation && serverWireVersion < 5) { - callback( - new MongoCompatibilityError( - `Server ${server.name}, which reports wire version ${serverWireVersion}, ` + - 'does not support collation' - ) - ); - return; - } - } - const cmd: Document = { createIndexes: this.collectionName, indexes }; if (options.commitQuorum != null) { From 4100dbc83043b710f94eb6236314e8cebf7d70c1 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 25 Jul 2022 17:49:53 -0400 Subject: [PATCH 25/29] test: small fixes --- .../client-side-encryption/driver.test.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/integration/client-side-encryption/driver.test.ts b/test/integration/client-side-encryption/driver.test.ts index fe0e8ed081..1665682404 100644 --- a/test/integration/client-side-encryption/driver.test.ts +++ b/test/integration/client-side-encryption/driver.test.ts @@ -116,6 +116,7 @@ describe('Client Side Encryption Functional', function () { describe('BSON Options', function () { let client: MongoClient; + let encryptedClient: MongoClient; beforeEach(function () { client = this.configuration.newClient(); @@ -140,7 +141,7 @@ describe('Client Side Encryption Functional', function () { return client .connect() .then(() => { - encryption = new mongodbClientEncryption.ClientEncryption(this.client, { + encryption = new mongodbClientEncryption.ClientEncryption(client, { bson: BSON, keyVaultNamespace, kmsProviders @@ -167,18 +168,18 @@ describe('Client Side Encryption Functional', function () { }); }) .then(() => { - this.encryptedClient = this.configuration.newClient( + encryptedClient = this.configuration.newClient( {}, { autoEncryption: { keyVaultNamespace, kmsProviders } } ); - return this.encryptedClient.connect(); + return encryptedClient.connect(); }); }); afterEach(function () { return Promise.resolve() - .then(() => this.encryptedClient && this.encryptedClient.close()) - .then(() => this.client.close()); + .then(() => encryptedClient?.close()) + .then(() => client?.close()); }); const testCases = [ @@ -208,11 +209,11 @@ describe('Client Side Encryption Functional', function () { const expected = BSON.deserialize(BSON.serialize(data, bsonOptions), bsonOptions); - const coll = this.encryptedClient.db(dataDbName).collection(dataCollName); + const coll = encryptedClient.db(dataDbName).collection(dataCollName); const result = await coll.insertOne(data, bsonOptions); const actual = await coll.findOne({ _id: result.insertedId }, bsonOptions); - const gValue = actual.g; - delete actual.g; + const gValue = actual?.g; + delete actual?.g; expect(actual).to.deep.equal(expected); expect(gValue).to.equal(bsonOptions.ignoreUndefined ? data.g : null); From c681cba95d399517b9c5ba5aa3c49fc388a34874 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 26 Jul 2022 14:13:52 -0400 Subject: [PATCH 26/29] comments: add test for name, type test for mixed types, match asserts on expected --- src/operations/indexes.ts | 37 +++++++++---------- .../client-side-encryption/driver.test.ts | 7 ++-- test/tools/spec-runner/index.js | 2 +- test/tools/unified-spec-runner/match.ts | 11 ++++-- test/types/community/createIndex.test-d.ts | 6 ++- test/unit/operations/indexes.test.ts | 6 +++ 6 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index ade496c84e..23b1ea3318 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -163,23 +163,7 @@ function makeIndexSpec( } } - // Set up the index - return { - ...Object.fromEntries( - Object.entries({ ...options }).filter(([optionName]) => VALID_INDEX_OPTIONS.has(optionName)) - ), - key - }; -} - -function fillInIndexNames(indexes: IndexDescription[]) { - return indexes.map(userIndex => { - // Ensure the key is a Map to preserve index key ordering - const key = - userIndex.key instanceof Map ? userIndex.key : new Map(Object.entries(userIndex.key)); - const name = userIndex.name != null ? userIndex.name : Array.from(key).flat().join('_'); - return { ...userIndex, key, name }; - }); + return { ...options, key }; } /** @internal */ @@ -216,7 +200,7 @@ export class CreateIndexesOperation< > extends CommandOperation { override options: CreateIndexesOptions; collectionName: string; - indexes: IndexDescription[]; + indexes: ReadonlyArray & { key: Map }>; constructor( parent: OperationParent, @@ -228,7 +212,22 @@ export class CreateIndexesOperation< this.options = options ?? {}; this.collectionName = collectionName; - this.indexes = fillInIndexNames(indexes); + this.indexes = indexes.map(userIndex => { + // Ensure the key is a Map to preserve index key ordering + const key = + userIndex.key instanceof Map ? userIndex.key : new Map(Object.entries(userIndex.key)); + const name = userIndex.name != null ? userIndex.name : Array.from(key).flat().join('_'); + const validIndexOptions = Object.fromEntries( + Object.entries({ ...userIndex }).filter(([optionName]) => + VALID_INDEX_OPTIONS.has(optionName) + ) + ); + return { + ...validIndexOptions, + name, + key + }; + }); } override execute( diff --git a/test/integration/client-side-encryption/driver.test.ts b/test/integration/client-side-encryption/driver.test.ts index 1665682404..fffdc1b226 100644 --- a/test/integration/client-side-encryption/driver.test.ts +++ b/test/integration/client-side-encryption/driver.test.ts @@ -226,9 +226,10 @@ describe('Client Side Encryption Functional', function () { let collection: Collection; beforeEach(async function () { - if (this.configuration.clientSideEncryption == null) { + if (!this.configuration.clientSideEncryption.enabled) { return; } + const encryptionOptions = { monitorCommands: true, autoEncryption: { @@ -248,10 +249,10 @@ describe('Client Side Encryption Functional', function () { it('should maintain ordered sort', metadata, async function () { const events: CommandStartedEvent[] = []; client.on('commandStarted', ev => events.push(ev)); - const sort: [string, SortDirection][] = [ + const sort: ReadonlyArray<[string, SortDirection]> = Object.freeze([ ['1', 1], ['0', 1] - ]; + ]); await collection.findOne({}, { sort }); const findEvent = events.find(event => !!event.command.find); expect(findEvent).to.have.property('commandName', 'find'); diff --git a/test/tools/spec-runner/index.js b/test/tools/spec-runner/index.js index 2ebb66dc67..dc322e269b 100644 --- a/test/tools/spec-runner/index.js +++ b/test/tools/spec-runner/index.js @@ -497,7 +497,7 @@ function validateExpectations(commandEvents, spec, savedSessionData) { } actualCommand.indexes = actualCommand.indexes.map(dbIndex => ({ ...dbIndex, - key: Object.fromEntries(dbIndex.key.entries()) + key: Object.fromEntries(dbIndex.key) })); } diff --git a/test/tools/unified-spec-runner/match.ts b/test/tools/unified-spec-runner/match.ts index a085e5f445..e615d4a421 100644 --- a/test/tools/unified-spec-runner/match.ts +++ b/test/tools/unified-spec-runner/match.ts @@ -167,13 +167,16 @@ export function resultCheck( const objFromActual = { [expectedSortKey]: actual[key].get(expectedSortKey) }; resultCheck(objFromActual, value, entities, path, checkExtraKeys); } else if (key === 'createIndexes') { - for (const index of actual.indexes) { - expect(index).to.have.property('key').that.is.instanceOf(Map); + for (const [i, userIndex] of actual.indexes.entries()) { + expect(expected).to.have.nested.property(`.indexes[${i}].key`).to.be.a('object'); + // @ts-expect-error: Not worth narrowing to a document + expect(Object.keys(expected.indexes[i].key)).to.have.lengthOf(1); + expect(userIndex).to.have.property('key').that.is.instanceOf(Map); expect( - index.key.size, + userIndex.key.size, 'Test input is JSON and cannot correctly test more than 1 key' ).to.equal(1); - index.key = Object.fromEntries(index.key.entries()); + userIndex.key = Object.fromEntries(userIndex.key); } resultCheck(actual[key], value, entities, path, checkExtraKeys); } else { diff --git a/test/types/community/createIndex.test-d.ts b/test/types/community/createIndex.test-d.ts index 781955f3fe..402376f7a4 100644 --- a/test/types/community/createIndex.test-d.ts +++ b/test/types/community/createIndex.test-d.ts @@ -23,4 +23,8 @@ collection.createIndex(['someKey']); collection.createIndex([['someKey', 1]]); collection.createIndex([new Map([['someKey', 1]])]); collection.createIndex([{ a: 1, b: -1 }]); -collection.createIndex([{ a: '2dsphere', b: -1 }]); +collection.createIndex([ + { a: '2dsphere', b: -1 }, + { a: 'geoHaystack', b: 1 } +]); +collection.createIndex(['a', ['b', 1], { a: 'geoHaystack', b: 1 }, new Map([['someKey', 1]])]); diff --git a/test/unit/operations/indexes.test.ts b/test/unit/operations/indexes.test.ts index 629aeac34e..67c82a1c1b 100644 --- a/test/unit/operations/indexes.test.ts +++ b/test/unit/operations/indexes.test.ts @@ -116,6 +116,12 @@ describe('class CreateIndexOperation', () => { }); } + it('should not generate a name if one is provided', () => { + const realOutput = makeIndexOperation({ a: 1, b: 1 }, { name: 'MyIndex' }); + expect(realOutput.indexes).to.be.an('array'); + expect(realOutput.indexes).to.have.nested.property('[0].name', 'MyIndex'); + }); + it('should keep numerical keys in chronological ordering when using Map input type', () => { const desiredMapData = new Map([ ['2', -1], From 4ed0db3c7e7f5f1c24dce429d0af5466afb894c1 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 26 Jul 2022 14:28:04 -0400 Subject: [PATCH 27/29] fix: ts for indexes --- src/operations/indexes.ts | 2 +- test/types/community/createIndex.test-d.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 23b1ea3318..f1dfceeaa7 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -89,7 +89,7 @@ export interface IndexDescription > { collation?: CollationOptions; name?: string; - key: Document | Map; + key: { [key: string]: IndexDirection } | Map; } /** @public */ diff --git a/test/types/community/createIndex.test-d.ts b/test/types/community/createIndex.test-d.ts index 402376f7a4..f1802cb762 100644 --- a/test/types/community/createIndex.test-d.ts +++ b/test/types/community/createIndex.test-d.ts @@ -28,3 +28,6 @@ collection.createIndex([ { a: 'geoHaystack', b: 1 } ]); collection.createIndex(['a', ['b', 1], { a: 'geoHaystack', b: 1 }, new Map([['someKey', 1]])]); + +// @ts-expect-error: CreateIndexes now asserts the object value types as of NODE-3517 +collection.createIndexes([{ key: { a: 34n } }]); From 92b19673b913722ad0b43f5616bd24bcc8e86fb9 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 27 Jul 2022 11:56:44 -0400 Subject: [PATCH 28/29] fix: tests --- .../client-side-encryption/driver.test.ts | 121 +++++++++--------- 1 file changed, 59 insertions(+), 62 deletions(-) diff --git a/test/integration/client-side-encryption/driver.test.ts b/test/integration/client-side-encryption/driver.test.ts index fffdc1b226..2c1f5ee30a 100644 --- a/test/integration/client-side-encryption/driver.test.ts +++ b/test/integration/client-side-encryption/driver.test.ts @@ -118,62 +118,56 @@ describe('Client Side Encryption Functional', function () { let client: MongoClient; let encryptedClient: MongoClient; - beforeEach(function () { + beforeEach(async function () { client = this.configuration.newClient(); - const noop = () => null; - function encryptSchema(keyId, bsonType) { - return { - encrypt: { - bsonType, - algorithm: 'AEAD_AES_256_CBC_HMAC_SHA_512-Random', - keyId: [keyId] - } - }; - } - - let encryption: ClientEncryption; - let dataDb: Db; - let keyVaultDb: Db; + const encryptSchema = (keyId: unknown, bsonType: string) => ({ + encrypt: { + bsonType, + algorithm: 'AEAD_AES_256_CBC_HMAC_SHA_512-Random', + keyId: [keyId] + } + }); const mongodbClientEncryption = this.configuration.mongodbClientEncryption; const kmsProviders = this.configuration.kmsProviders(crypto.randomBytes(96)); - return client - .connect() - .then(() => { - encryption = new mongodbClientEncryption.ClientEncryption(client, { - bson: BSON, - keyVaultNamespace, - kmsProviders - }); - }) - .then(() => (dataDb = client.db(dataDbName))) - .then(() => (keyVaultDb = client.db(keyVaultDbName))) - .then(() => dataDb.dropCollection(dataCollName).catch(noop)) - .then(() => keyVaultDb.dropCollection(keyVaultCollName).catch(noop)) - .then(() => keyVaultDb.createCollection(keyVaultCollName)) - .then(() => encryption.createDataKey('local')) - .then(dataKey => { - const $jsonSchema = { - bsonType: 'object', - properties: { - a: encryptSchema(dataKey, 'int'), - b: encryptSchema(dataKey, 'int'), - c: encryptSchema(dataKey, 'long'), - d: encryptSchema(dataKey, 'double') - } - }; - return dataDb.createCollection(dataCollName, { - validator: { $jsonSchema } - }); - }) - .then(() => { - encryptedClient = this.configuration.newClient( - {}, - { autoEncryption: { keyVaultNamespace, kmsProviders } } - ); - return encryptedClient.connect(); - }); + + await client.connect(); + + const encryption: ClientEncryption = new mongodbClientEncryption.ClientEncryption(client, { + bson: BSON, + keyVaultNamespace, + kmsProviders + }); + + const dataDb = client.db(dataDbName); + const keyVaultDb = client.db(keyVaultDbName); + + await dataDb.dropCollection(dataCollName).catch(() => null); + await keyVaultDb.dropCollection(keyVaultCollName).catch(() => null); + await keyVaultDb.createCollection(keyVaultCollName); + const dataKey = await encryption.createDataKey('local'); + + const $jsonSchema = { + bsonType: 'object', + properties: { + a: encryptSchema(dataKey, 'int'), + b: encryptSchema(dataKey, 'int'), + c: encryptSchema(dataKey, 'long'), + d: encryptSchema(dataKey, 'double') + } + }; + + await dataDb.createCollection(dataCollName, { + validator: { $jsonSchema } + }); + + encryptedClient = this.configuration.newClient( + {}, + { autoEncryption: { keyVaultNamespace, kmsProviders } } + ); + + await encryptedClient.connect(); }); afterEach(function () { @@ -249,10 +243,11 @@ describe('Client Side Encryption Functional', function () { it('should maintain ordered sort', metadata, async function () { const events: CommandStartedEvent[] = []; client.on('commandStarted', ev => events.push(ev)); - const sort: ReadonlyArray<[string, SortDirection]> = Object.freeze([ - ['1', 1], - ['0', 1] + const sort = Object.freeze([ + Object.freeze(['1', 1] as const), + Object.freeze(['0', 1] as const) ]); + // @ts-expect-error: Our findOne API does not accept readonly input await collection.findOne({}, { sort }); const findEvent = events.find(event => !!event.command.find); expect(findEvent).to.have.property('commandName', 'find'); @@ -264,10 +259,11 @@ describe('Client Side Encryption Functional', function () { it('should maintain ordered sort', metadata, async function () { const events: CommandStartedEvent[] = []; client.on('commandStarted', ev => events.push(ev)); - const sort: [string, SortDirection][] = [ - ['1', 1], - ['0', 1] - ]; + const sort = Object.freeze([ + Object.freeze(['1', 1] as const), + Object.freeze(['0', 1] as const) + ]); + // @ts-expect-error: Our findOneAndUpdate API does not accept readonly input await collection.findOneAndUpdate({}, { $setOnInsert: { a: 1 } }, { sort }); const findAndModifyEvent = events.find(event => !!event.command.findAndModify); expect(findAndModifyEvent).to.have.property('commandName', 'findAndModify'); @@ -281,10 +277,11 @@ describe('Client Side Encryption Functional', function () { it('should maintain ordered index keys', metadata, async function () { const events: CommandStartedEvent[] = []; client.on('commandStarted', ev => events.push(ev)); - const indexDescription: [string, IndexDirection][] = [ - ['1', 1], - ['0', 1] - ]; + const indexDescription = Object.freeze([ + Object.freeze(['1', 1] as const), + Object.freeze(['0', 1] as const) + ]); + // @ts-expect-error: Our createIndex API does not accept readonly input await collection.createIndex(indexDescription, { name: 'myIndex' }); const createIndexEvent = events.find(event => !!event.command.createIndexes); expect(createIndexEvent).to.have.property('commandName', 'createIndexes'); From 784c8c1ae1ebaf27b6b9f2dba52839bcd5ecb177 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 27 Jul 2022 12:06:59 -0400 Subject: [PATCH 29/29] fix: lint --- test/integration/client-side-encryption/driver.test.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/integration/client-side-encryption/driver.test.ts b/test/integration/client-side-encryption/driver.test.ts index 2c1f5ee30a..ddb9bb01e8 100644 --- a/test/integration/client-side-encryption/driver.test.ts +++ b/test/integration/client-side-encryption/driver.test.ts @@ -2,14 +2,7 @@ import { EJSON, UUID } from 'bson'; import { expect } from 'chai'; import * as crypto from 'crypto'; -import { - Collection, - CommandStartedEvent, - Db, - IndexDirection, - MongoClient, - SortDirection -} from '../../../src'; +import { Collection, CommandStartedEvent, MongoClient } from '../../../src'; import * as BSON from '../../../src/bson'; import { ClientEncryption } from '../../tools/unified-spec-runner/schema';