Skip to content

Commit e12c485

Browse files
emadumemadum
authored
refactor!: CreateIndexOp returns string, CreateIndexesOp returns array (#2666)
`CreateIndexesOperation` was returning the name of the created index instead of the result from the server because of the logic ensuring `CreateIndexOperation` has that behavior. This PR makes `CreateIndexesOperation` return an *array of index names* (breaking change), while `CreateIndexOperation` continues to return a single index name string. This makes the behavior consistent with other official drivers. Users in need of the raw result from the server should use `db.command`. NODE-2602
1 parent 0f2eb49 commit e12c485

File tree

5 files changed

+75
-37
lines changed

5 files changed

+75
-37
lines changed

‎src/bulk/common.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -1221,7 +1221,10 @@ export abstract class BulkOperationBase {
12211221
}
12221222

12231223
/** An internal helper method. Do not invoke directly. Will be going away in the future */
1224-
execute(options?: BulkWriteOptions, callback?: Callback<BulkWriteResult>): Promise<void> | void {
1224+
execute(
1225+
options?: BulkWriteOptions,
1226+
callback?: Callback<BulkWriteResult>
1227+
): Promise<BulkWriteResult> | void {
12251228
if (typeof options === 'function') (callback = options), (options = {});
12261229
options = options ?? {};
12271230

@@ -1303,7 +1306,7 @@ Object.defineProperty(BulkOperationBase.prototype, 'length', {
13031306
function handleEarlyError(
13041307
err?: AnyError,
13051308
callback?: Callback<BulkWriteResult>
1306-
): Promise<void> | void {
1309+
): Promise<BulkWriteResult> | void {
13071310
const Promise = PromiseProvider.get();
13081311
if (typeof callback === 'function') {
13091312
callback(err);

‎src/collection.ts

+14-14
Original file line numberDiff line numberDiff line change
@@ -744,19 +744,19 @@ export class Collection {
744744
* await collection.createIndex(['j', ['k', -1], { l: '2d' }])
745745
* ```
746746
*/
747-
createIndex(indexSpec: IndexSpecification): Promise<Document>;
748-
createIndex(indexSpec: IndexSpecification, callback: Callback<Document>): void;
749-
createIndex(indexSpec: IndexSpecification, options: CreateIndexesOptions): Promise<Document>;
747+
createIndex(indexSpec: IndexSpecification): Promise<string>;
748+
createIndex(indexSpec: IndexSpecification, callback: Callback<string>): void;
749+
createIndex(indexSpec: IndexSpecification, options: CreateIndexesOptions): Promise<string>;
750750
createIndex(
751751
indexSpec: IndexSpecification,
752752
options: CreateIndexesOptions,
753-
callback: Callback<Document>
753+
callback: Callback<string>
754754
): void;
755755
createIndex(
756756
indexSpec: IndexSpecification,
757-
options?: CreateIndexesOptions | Callback<Document>,
758-
callback?: Callback<Document>
759-
): Promise<Document> | void {
757+
options?: CreateIndexesOptions | Callback<string>,
758+
callback?: Callback<string>
759+
): Promise<string> | void {
760760
if (typeof options === 'function') (callback = options), (options = {});
761761

762762
return executeOperation(
@@ -798,19 +798,19 @@ export class Collection {
798798
* ]);
799799
* ```
800800
*/
801-
createIndexes(indexSpecs: IndexDescription[]): Promise<Document>;
802-
createIndexes(indexSpecs: IndexDescription[], callback: Callback<Document>): void;
803-
createIndexes(indexSpecs: IndexDescription[], options: CreateIndexesOptions): Promise<Document>;
801+
createIndexes(indexSpecs: IndexDescription[]): Promise<string[]>;
802+
createIndexes(indexSpecs: IndexDescription[], callback: Callback<string[]>): void;
803+
createIndexes(indexSpecs: IndexDescription[], options: CreateIndexesOptions): Promise<string[]>;
804804
createIndexes(
805805
indexSpecs: IndexDescription[],
806806
options: CreateIndexesOptions,
807-
callback: Callback<Document>
807+
callback: Callback<string[]>
808808
): void;
809809
createIndexes(
810810
indexSpecs: IndexDescription[],
811-
options?: CreateIndexesOptions | Callback<Document>,
812-
callback?: Callback<Document>
813-
): Promise<Document> | void {
811+
options?: CreateIndexesOptions | Callback<string[]>,
812+
callback?: Callback<string[]>
813+
): Promise<string[]> | void {
814814
if (typeof options === 'function') (callback = options), (options = {});
815815
options = options ? Object.assign({}, options) : {};
816816
if (typeof options.maxTimeMS !== 'number') delete options.maxTimeMS;

‎src/db.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -579,25 +579,25 @@ export class Db {
579579
* @param options - Optional settings for the command
580580
* @param callback - An optional callback, a Promise will be returned if none is provided
581581
*/
582-
createIndex(name: string, indexSpec: IndexSpecification): Promise<Document>;
583-
createIndex(name: string, indexSpec: IndexSpecification, callback?: Callback<Document>): void;
582+
createIndex(name: string, indexSpec: IndexSpecification): Promise<string>;
583+
createIndex(name: string, indexSpec: IndexSpecification, callback?: Callback<string>): void;
584584
createIndex(
585585
name: string,
586586
indexSpec: IndexSpecification,
587587
options: CreateIndexesOptions
588-
): Promise<Document>;
588+
): Promise<string>;
589589
createIndex(
590590
name: string,
591591
indexSpec: IndexSpecification,
592592
options: CreateIndexesOptions,
593-
callback: Callback<Document>
593+
callback: Callback<string>
594594
): void;
595595
createIndex(
596596
name: string,
597597
indexSpec: IndexSpecification,
598-
options?: CreateIndexesOptions | Callback<Document>,
599-
callback?: Callback<Document>
600-
): Promise<Document> | void {
598+
options?: CreateIndexesOptions | Callback<string>,
599+
callback?: Callback<string>
600+
): Promise<string> | void {
601601
if (typeof options === 'function') (callback = options), (options = {});
602602

603603
return executeOperation(

‎src/operations/indexes.ts

+14-9
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,11 @@ export class IndexesOperation extends AbstractOperation<Document> {
154154
}
155155

156156
/** @internal */
157-
export class CreateIndexesOperation extends CommandOperation<Document> {
157+
export class CreateIndexesOperation<
158+
T extends string | string[] = string[]
159+
> extends CommandOperation<T> {
158160
options: CreateIndexesOptions;
159161
collectionName: string;
160-
onlyReturnNameOfCreatedIndex?: boolean;
161162
indexes: IndexDescription[];
162163

163164
constructor(
@@ -172,12 +173,9 @@ export class CreateIndexesOperation extends CommandOperation<Document> {
172173
this.collectionName = collectionName;
173174

174175
this.indexes = indexes;
175-
if (indexes.length === 1) {
176-
this.onlyReturnNameOfCreatedIndex = true;
177-
}
178176
}
179177

180-
execute(server: Server, session: ClientSession, callback: Callback<Document>): void {
178+
execute(server: Server, session: ClientSession, callback: Callback<T>): void {
181179
const options = this.options;
182180
const indexes = this.indexes;
183181

@@ -223,19 +221,20 @@ export class CreateIndexesOperation extends CommandOperation<Document> {
223221
// collation is set on each index, it should not be defined at the root
224222
this.options.collation = undefined;
225223

226-
super.executeCommand(server, session, cmd, (err, result) => {
224+
super.executeCommand(server, session, cmd, err => {
227225
if (err) {
228226
callback(err);
229227
return;
230228
}
231229

232-
callback(undefined, this.onlyReturnNameOfCreatedIndex ? indexes[0].name : result);
230+
const indexNames = indexes.map(index => index.name || '');
231+
callback(undefined, indexNames as T);
233232
});
234233
}
235234
}
236235

237236
/** @internal */
238-
export class CreateIndexOperation extends CreateIndexesOperation {
237+
export class CreateIndexOperation extends CreateIndexesOperation<string> {
239238
constructor(
240239
parent: OperationParent,
241240
collectionName: string,
@@ -250,6 +249,12 @@ export class CreateIndexOperation extends CreateIndexesOperation {
250249

251250
super(parent, collectionName, [makeIndexSpec(indexSpec, options)], options);
252251
}
252+
execute(server: Server, session: ClientSession, callback: Callback<string>): void {
253+
super.execute(server, session, (err, indexNames) => {
254+
if (err || !indexNames) return callback(err);
255+
return callback(undefined, indexNames[0]);
256+
});
257+
}
253258
}
254259

255260
/** @internal */

‎test/functional/index.test.js

+35-5
Original file line numberDiff line numberDiff line change
@@ -831,10 +831,8 @@ describe('Indexes', function () {
831831
}
832832
});
833833

834-
it('should correctly execute createIndexes', {
835-
metadata: {
836-
requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl', 'heap', 'wiredtiger'] }
837-
},
834+
it('should correctly execute createIndexes with multiple indexes', {
835+
metadata: { requires: { mongodb: '>=2.6.0', topology: ['single'] } },
838836

839837
test: function (done) {
840838
var configuration = this.configuration;
@@ -845,7 +843,7 @@ describe('Indexes', function () {
845843
[{ key: { a: 1 } }, { key: { b: 1 }, name: 'hello1' }],
846844
function (err, r) {
847845
expect(err).to.not.exist;
848-
test.equal(3, r.numIndexesAfter);
846+
expect(r).to.deep.equal(['a_1', 'hello1']);
849847

850848
db.collection('createIndexes')
851849
.listIndexes()
@@ -868,6 +866,38 @@ describe('Indexes', function () {
868866
}
869867
});
870868

869+
it('should correctly execute createIndexes with one index', {
870+
metadata: { requires: { mongodb: '>=2.6.0', topology: ['single'] } },
871+
872+
test: function (done) {
873+
var configuration = this.configuration;
874+
var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 });
875+
client.connect(function (err, client) {
876+
var db = client.db(configuration.db);
877+
db.collection('createIndexes').createIndexes([{ key: { a: 1 } }], function (err, r) {
878+
expect(err).to.not.exist;
879+
expect(r).to.deep.equal(['a_1']);
880+
881+
db.collection('createIndexes')
882+
.listIndexes()
883+
.toArray(function (err, docs) {
884+
expect(err).to.not.exist;
885+
var keys = {};
886+
887+
for (var i = 0; i < docs.length; i++) {
888+
keys[docs[i].name] = true;
889+
}
890+
891+
test.ok(keys['a_1']);
892+
test.ok(keys['hello1']);
893+
894+
client.close(done);
895+
});
896+
});
897+
});
898+
}
899+
});
900+
871901
it('shouldCorrectlyCreateTextIndex', {
872902
metadata: {
873903
requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] }

0 commit comments

Comments
 (0)