From 227b6d83574b62944b9786cf28ae685c265c348d Mon Sep 17 00:00:00 2001 From: emadum Date: Mon, 7 Dec 2020 15:04:54 -0500 Subject: [PATCH 1/4] fix: CreateIndexOp returns string, CreateIndexesOp returns array --- src/operations/indexes.ts | 16 +++++++------- test/functional/index.test.js | 40 ++++++++++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 51d4a783c7..01c0e0eb0d 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -153,24 +153,23 @@ export class IndexesOperation extends AbstractOperation { export class CreateIndexesOperation extends CommandOperation { options: CreateIndexesOptions; collectionName: string; - onlyReturnNameOfCreatedIndex?: boolean; + singular?: boolean; indexes: IndexDescription[]; constructor( parent: OperationParent, collectionName: string, indexes: IndexDescription[], - options?: CreateIndexesOptions + options?: CreateIndexesOptions, + singular?: boolean ) { super(parent, options); this.options = options ?? {}; + this.singular = singular ?? false; this.collectionName = collectionName; this.indexes = indexes; - if (indexes.length === 1) { - this.onlyReturnNameOfCreatedIndex = true; - } } execute(server: Server, session: ClientSession, callback: Callback): void { @@ -219,13 +218,14 @@ export class CreateIndexesOperation extends CommandOperation { // collation is set on each index, it should not be defined at the root this.options.collation = undefined; - super.executeCommand(server, session, cmd, (err, result) => { + super.executeCommand(server, session, cmd, err => { if (err) { callback(err); return; } - callback(undefined, this.onlyReturnNameOfCreatedIndex ? indexes[0].name : result); + const names = indexes.map(index => index.name); + callback(undefined, (this.singular ? names[0] : names) as Document); }); } } @@ -244,7 +244,7 @@ export class CreateIndexOperation extends CreateIndexesOperation { // 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)], options, true); } } diff --git a/test/functional/index.test.js b/test/functional/index.test.js index a55dd2786e..10b3a70246 100644 --- a/test/functional/index.test.js +++ b/test/functional/index.test.js @@ -831,10 +831,8 @@ describe('Indexes', function () { } }); - it('should correctly execute createIndexes', { - metadata: { - requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl', 'heap', 'wiredtiger'] } - }, + it('should correctly execute createIndexes with multiple indexes', { + metadata: { requires: { mongodb: '>=2.6.0', topology: ['single'] } }, test: function (done) { var configuration = this.configuration; @@ -845,7 +843,7 @@ describe('Indexes', function () { [{ key: { a: 1 } }, { key: { b: 1 }, name: 'hello1' }], function (err, r) { expect(err).to.not.exist; - test.equal(3, r.numIndexesAfter); + expect(r).to.deep.equal(['a_1', 'hello1']); db.collection('createIndexes') .listIndexes() @@ -868,6 +866,38 @@ describe('Indexes', function () { } }); + it('should correctly execute createIndexes with one index', { + metadata: { requires: { mongodb: '>=2.6.0', topology: ['single'] } }, + + test: function (done) { + var configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + client.connect(function (err, client) { + var db = client.db(configuration.db); + db.collection('createIndexes').createIndexes([{ key: { a: 1 } }], function (err, r) { + expect(err).to.not.exist; + expect(r).to.deep.equal(['a_1']); + + db.collection('createIndexes') + .listIndexes() + .toArray(function (err, docs) { + expect(err).to.not.exist; + var keys = {}; + + for (var i = 0; i < docs.length; i++) { + keys[docs[i].name] = true; + } + + test.ok(keys['a_1']); + test.ok(keys['hello1']); + + client.close(done); + }); + }); + }); + } + }); + it('shouldCorrectlyCreateTextIndex', { metadata: { requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] } From ea6620da4f1882580287ae07ea4ad25e0a5ea86b Mon Sep 17 00:00:00 2001 From: emadum Date: Tue, 8 Dec 2020 16:37:11 -0500 Subject: [PATCH 2/4] fix: update types --- src/collection.ts | 28 ++++++++++++++-------------- src/db.ts | 14 +++++++------- src/operations/indexes.ts | 21 ++++++++++++--------- 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 332dcaf96c..9a15b029b4 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -727,19 +727,19 @@ export class Collection implements OperationParent { * await collection.createIndex(['j', ['k', -1], { l: '2d' }]) * ``` */ - createIndex(indexSpec: IndexSpecification): Promise; - createIndex(indexSpec: IndexSpecification, callback: Callback): void; - createIndex(indexSpec: IndexSpecification, options: CreateIndexesOptions): Promise; + createIndex(indexSpec: IndexSpecification): Promise; + createIndex(indexSpec: IndexSpecification, callback: Callback): void; + createIndex(indexSpec: IndexSpecification, options: CreateIndexesOptions): Promise; createIndex( indexSpec: IndexSpecification, options: CreateIndexesOptions, - callback: Callback + callback: Callback ): void; createIndex( indexSpec: IndexSpecification, - options?: CreateIndexesOptions | Callback, - callback?: Callback - ): Promise | void { + options?: CreateIndexesOptions | Callback, + callback?: Callback + ): Promise | void { if (typeof options === 'function') (callback = options), (options = {}); return executeOperation( @@ -781,19 +781,19 @@ export class Collection implements OperationParent { * ]); * ``` */ - createIndexes(indexSpecs: IndexDescription[]): Promise; - createIndexes(indexSpecs: IndexDescription[], callback: Callback): void; - createIndexes(indexSpecs: IndexDescription[], options: CreateIndexesOptions): Promise; + createIndexes(indexSpecs: IndexDescription[]): Promise; + createIndexes(indexSpecs: IndexDescription[], callback: Callback): void; + createIndexes(indexSpecs: IndexDescription[], options: CreateIndexesOptions): Promise; createIndexes( indexSpecs: IndexDescription[], options: CreateIndexesOptions, - callback: Callback + callback: Callback ): void; createIndexes( indexSpecs: IndexDescription[], - options?: CreateIndexesOptions | Callback, - callback?: Callback - ): Promise | void { + options?: CreateIndexesOptions | Callback, + callback?: Callback + ): Promise | void { if (typeof options === 'function') (callback = options), (options = {}); options = options ? Object.assign({}, options) : {}; if (typeof options.maxTimeMS !== 'number') delete options.maxTimeMS; diff --git a/src/db.ts b/src/db.ts index 3a940445ad..a4d33cddad 100644 --- a/src/db.ts +++ b/src/db.ts @@ -581,25 +581,25 @@ export class Db implements OperationParent { * @param options - Optional settings for the command * @param callback - An optional callback, a Promise will be returned if none is provided */ - createIndex(name: string, indexSpec: IndexSpecification): Promise; - createIndex(name: string, indexSpec: IndexSpecification, callback?: Callback): void; + createIndex(name: string, indexSpec: IndexSpecification): Promise; + createIndex(name: string, indexSpec: IndexSpecification, callback?: Callback): void; createIndex( name: string, indexSpec: IndexSpecification, options: CreateIndexesOptions - ): Promise; + ): Promise; createIndex( name: string, indexSpec: IndexSpecification, options: CreateIndexesOptions, - callback: Callback + callback: Callback ): void; createIndex( name: string, indexSpec: IndexSpecification, - options?: CreateIndexesOptions | Callback, - callback?: Callback - ): Promise | void { + options?: CreateIndexesOptions | Callback, + callback?: Callback + ): Promise | void { if (typeof options === 'function') (callback = options), (options = {}); return executeOperation( diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 01c0e0eb0d..e3f0d3cefe 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -150,29 +150,26 @@ export class IndexesOperation extends AbstractOperation { } /** @internal */ -export class CreateIndexesOperation extends CommandOperation { +export class CreateIndexesOperation extends CommandOperation { options: CreateIndexesOptions; collectionName: string; - singular?: boolean; indexes: IndexDescription[]; constructor( parent: OperationParent, collectionName: string, indexes: IndexDescription[], - options?: CreateIndexesOptions, - singular?: boolean + options?: CreateIndexesOptions ) { super(parent, options); this.options = options ?? {}; - this.singular = singular ?? false; this.collectionName = collectionName; this.indexes = indexes; } - execute(server: Server, session: ClientSession, callback: Callback): void { + execute(server: Server, session: ClientSession, callback: Callback): void { const options = this.options; const indexes = this.indexes; @@ -225,13 +222,13 @@ export class CreateIndexesOperation extends CommandOperation { } const names = indexes.map(index => index.name); - callback(undefined, (this.singular ? names[0] : names) as Document); + callback(undefined, names); }); } } /** @internal */ -export class CreateIndexOperation extends CreateIndexesOperation { +export class CreateIndexOperation extends CreateIndexesOperation { constructor( parent: OperationParent, collectionName: string, @@ -244,7 +241,13 @@ export class CreateIndexOperation extends CreateIndexesOperation { // coll.createIndex([['a', 1]]); // createIndexes is always called with an array of index spec objects - super(parent, collectionName, [makeIndexSpec(indexSpec, options)], options, true); + super(parent, collectionName, [makeIndexSpec(indexSpec, options)], options); + } + execute(server: Server, session: ClientSession, callback: Callback): void { + super.execute(server, session, (err, result) => { + if (err) return callback(err); + return callback(undefined, result[0]); + }); } } From 141646626470846d05d407586b230bcd0d1021fb Mon Sep 17 00:00:00 2001 From: emadum Date: Tue, 8 Dec 2020 17:10:53 -0500 Subject: [PATCH 3/4] cleanup, fix bulk execute typing error --- src/bulk/common.ts | 7 +++++-- src/operations/indexes.ts | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 6a2a9ff41a..06c5fae100 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -1210,7 +1210,10 @@ export abstract class BulkOperationBase { } /** An internal helper method. Do not invoke directly. Will be going away in the future */ - execute(options?: BulkWriteOptions, callback?: Callback): Promise | void { + execute( + options?: BulkWriteOptions, + callback?: Callback + ): Promise | void { if (typeof options === 'function') (callback = options), (options = {}); options = options ?? {}; @@ -1292,7 +1295,7 @@ Object.defineProperty(BulkOperationBase.prototype, 'length', { function handleEarlyError( err?: AnyError, callback?: Callback -): Promise | void { +): Promise | void { const Promise = PromiseProvider.get(); if (typeof callback === 'function') { callback(err); diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index e3f0d3cefe..c38ed3936d 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -221,8 +221,8 @@ export class CreateIndexesOperation extends CommandOperation { return; } - const names = indexes.map(index => index.name); - callback(undefined, names); + const indexNames = indexes.map(index => index.name); + callback(undefined, indexNames); }); } } @@ -244,9 +244,9 @@ export class CreateIndexOperation extends CreateIndexesOperation { super(parent, collectionName, [makeIndexSpec(indexSpec, options)], options); } execute(server: Server, session: ClientSession, callback: Callback): void { - super.execute(server, session, (err, result) => { + super.execute(server, session, (err, indexNames) => { if (err) return callback(err); - return callback(undefined, result[0]); + return callback(undefined, indexNames[0]); }); } } From 65f3dd2e214fd323fbc19bd18a6be4762eb176b1 Mon Sep 17 00:00:00 2001 From: emadum Date: Thu, 10 Dec 2020 17:18:39 -0500 Subject: [PATCH 4/4] remove any type --- src/operations/indexes.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index c38ed3936d..657b45b40c 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -150,7 +150,9 @@ export class IndexesOperation extends AbstractOperation { } /** @internal */ -export class CreateIndexesOperation extends CommandOperation { +export class CreateIndexesOperation< + T extends string | string[] = string[] +> extends CommandOperation { options: CreateIndexesOptions; collectionName: string; indexes: IndexDescription[]; @@ -169,7 +171,7 @@ export class CreateIndexesOperation extends CommandOperation { this.indexes = indexes; } - execute(server: Server, session: ClientSession, callback: Callback): void { + execute(server: Server, session: ClientSession, callback: Callback): void { const options = this.options; const indexes = this.indexes; @@ -221,8 +223,8 @@ export class CreateIndexesOperation extends CommandOperation { return; } - const indexNames = indexes.map(index => index.name); - callback(undefined, indexNames); + const indexNames = indexes.map(index => index.name || ''); + callback(undefined, indexNames as T); }); } } @@ -245,7 +247,7 @@ export class CreateIndexOperation extends CreateIndexesOperation { } execute(server: Server, session: ClientSession, callback: Callback): void { super.execute(server, session, (err, indexNames) => { - if (err) return callback(err); + if (err || !indexNames) return callback(err); return callback(undefined, indexNames[0]); }); }