Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-5971): attach v to createIndexes command when version is specified #4043

Merged
merged 4 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 45 additions & 19 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]) =>
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
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('_');
W-A-James marked this conversation as resolved.
Show resolved Hide resolved
const validIndexOptions = Object.fromEntries(
Object.entries({ ...userIndex }).filter(([optionName]) =>
VALID_INDEX_OPTIONS.has(optionName)
)
);
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
Loading