Skip to content

Commit

Permalink
fix(NODE-5971): attach v to createIndexes command when version is…
Browse files Browse the repository at this point in the history
… specified (#4043)
  • Loading branch information
baileympearson authored Mar 20, 2024
1 parent abf8bdf commit 1879a04
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 21 deletions.
66 changes: 46 additions & 20 deletions src/operations/indexes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, IndexDirection> {
const key: Map<string, IndexDirection> = new Map();

const indexSpecs =
Expand All @@ -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<ResolvedIndexDescription, 'key'> {
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<IndexDescription, 'key' | 'version'> & {
key: Map<string, IndexDirection>;
v?: IndexDescription['version'];
};

/** @internal */
export class CreateIndexesOperation extends CommandOperation<string[]> {
override options: CreateIndexesOptions;
collectionName: string;
indexes: ReadonlyArray<Omit<IndexDescription, 'key'> & { key: Map<string, IndexDirection> }>;
indexes: ReadonlyArray<ResolvedIndexDescription>;

private constructor(
parent: OperationParent,
Expand All @@ -212,16 +245,12 @@ export class CreateIndexesOperation extends CommandOperation<string[]> {

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,
Expand All @@ -243,14 +272,11 @@ export class CreateIndexesOperation extends CommandOperation<string[]> {
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() {
Expand Down
78 changes: 77 additions & 1 deletion test/integration/index_management.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit 1879a04

Please sign in to comment.