From 1879a04b9053e8c77c6957e415886c534fe9992b Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 20 Mar 2024 14:56:20 -0600 Subject: [PATCH] fix(NODE-5971): attach `v` to createIndexes command when `version` is specified (#4043) --- src/operations/indexes.ts | 66 +++++++++++++------ test/integration/index_management.test.ts | 78 ++++++++++++++++++++++- 2 files changed, 123 insertions(+), 21 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index a1c2f12bfc..1259bff31b 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -167,10 +167,11 @@ function isSingleIndexTuple(t: unknown): t is [string, IndexDirection] { return Array.isArray(t) && t.length === 2 && isIndexDirection(t[1]); } -function makeIndexSpec( - indexSpec: IndexSpecification, - options?: CreateIndexesOptions -): IndexDescription { +/** + * Converts an `IndexSpecification`, which can be specified in multiple formats, into a + * valid `key` for the createIndexes command. + */ +function constructIndexDescriptionMap(indexSpec: IndexSpecification): Map { const key: Map = new Map(); const indexSpecs = @@ -193,14 +194,46 @@ function makeIndexSpec( } } - return { ...options, key }; + return key; } +/** + * Receives an index description and returns a modified index description which has had invalid options removed + * from the description and has mapped the `version` option to the `v` option. + */ +function resolveIndexDescription( + description: IndexDescription +): Omit { + const validProvidedOptions = Object.entries(description).filter(([optionName]) => + VALID_INDEX_OPTIONS.has(optionName) + ); + + return Object.fromEntries( + // we support the `version` option, but the `createIndexes` command expects it to be the `v` + validProvidedOptions.map(([name, value]) => (name === 'version' ? ['v', value] : [name, value])) + ); +} + +/** + * @internal + * + * Internally, the driver represents index description keys with `Map`s to preserve key ordering. + * We don't require users to specify maps, so we transform user provided descriptions into + * "resolved" by converting the `key` into a JS `Map`, if it isn't already a map. + * + * Additionally, we support the `version` option, but the `createIndexes` command uses the field `v` + * to specify the index version so we map the value of `version` to `v`, if provided. + */ +type ResolvedIndexDescription = Omit & { + key: Map; + v?: IndexDescription['version']; +}; + /** @internal */ export class CreateIndexesOperation extends CommandOperation { override options: CreateIndexesOptions; collectionName: string; - indexes: ReadonlyArray & { key: Map }>; + indexes: ReadonlyArray; private constructor( parent: OperationParent, @@ -212,16 +245,12 @@ export class CreateIndexesOperation extends CommandOperation { this.options = options ?? {}; this.collectionName = collectionName; - this.indexes = indexes.map(userIndex => { + this.indexes = indexes.map((userIndex: IndexDescription): ResolvedIndexDescription => { // 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) - ) - ); + const name = userIndex.name ?? Array.from(key).flat().join('_'); + const validIndexOptions = resolveIndexDescription(userIndex); return { ...validIndexOptions, name, @@ -243,14 +272,11 @@ export class CreateIndexesOperation extends CommandOperation { parent: OperationParent, collectionName: string, indexSpec: IndexSpecification, - options?: CreateIndexesOptions + options: CreateIndexesOptions = {} ): CreateIndexesOperation { - return new CreateIndexesOperation( - parent, - collectionName, - [makeIndexSpec(indexSpec, options)], - options - ); + const key = constructIndexDescriptionMap(indexSpec); + const description: IndexDescription = { ...options, key }; + return new CreateIndexesOperation(parent, collectionName, [description], options); } override get commandName() { diff --git a/test/integration/index_management.test.ts b/test/integration/index_management.test.ts index fab28b5890..5fdb9799d9 100644 --- a/test/integration/index_management.test.ts +++ b/test/integration/index_management.test.ts @@ -8,7 +8,7 @@ import { type MongoClient, MongoServerError } from '../mongodb'; -import { assert as test, setupDatabase } from './shared'; +import { assert as test, filterForCommands, setupDatabase } from './shared'; describe('Indexes', function () { let client: MongoClient; @@ -160,6 +160,82 @@ describe('Indexes', function () { }); }); + describe('Collection.createIndex()', function () { + const started: CommandStartedEvent[] = []; + beforeEach(() => { + started.length = 0; + client.on('commandStarted', filterForCommands('createIndexes', started)); + }); + + context('when version is not specified as an option', function () { + it('does not attach `v` to the command', async () => { + await collection.createIndex({ age: 1 }); + const { command } = started[0]; + expect(command).to.exist; + expect(command.indexes[0]).not.to.have.property('v'); + }); + }); + + context('when version is specified as an option', function () { + it('attaches `v` to the command with the value of `version`', async () => { + await collection.createIndex({ age: 1 }, { version: 1 }); + const { command } = started[0]; + expect(command).to.exist; + expect(command.indexes[0]).to.have.property('v', 1); + }); + }); + }); + + describe('Collection.createIndexes()', function () { + const started: CommandStartedEvent[] = []; + beforeEach(() => { + started.length = 0; + client.on('commandStarted', filterForCommands('createIndexes', started)); + }); + + context('when version is not specified as an option', function () { + it('does not attach `v` to the command', async () => { + await collection.createIndexes([{ key: { age: 1 } }]); + const { command } = started[0]; + expect(command).to.exist; + expect(command.indexes[0]).not.to.have.property('v'); + }); + }); + + context('when version is specified as an option', function () { + it('does not attach `v` to the command', async () => { + await collection.createIndexes([{ key: { age: 1 } }], { version: 1 }); + const { command } = started[0]; + expect(command).to.exist; + expect(command.indexes[0]).not.to.have.property('v', 1); + }); + }); + + context('when version is provided in the index description and the options', function () { + it('the value in the description takes precedence', async () => { + await collection.createIndexes([{ key: { age: 1 }, version: 1 }], { version: 0 }); + const { command } = started[0]; + expect(command).to.exist; + expect(command.indexes[0]).to.have.property('v', 1); + }); + }); + + context( + 'when version is provided in some of the index descriptions and the options', + function () { + it('does not specify a version from the `version` provided in the options', async () => { + await collection.createIndexes([{ key: { age: 1 }, version: 1 }, { key: { date: 1 } }], { + version: 0 + }); + const { command } = started[0]; + expect(command).to.exist; + expect(command.indexes[0]).to.have.property('v', 1); + expect(command.indexes[1]).not.to.have.property('v'); + }); + } + ); + }); + describe('Collection.indexExists()', function () { beforeEach(() => collection.createIndex({ age: 1 })); afterEach(() => collection.dropIndexes());