From 14363bdc02349515f7da435f3a9696cdaa87106b Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Fri, 9 Jun 2023 16:13:13 -0400 Subject: [PATCH 01/23] test(NODE-5342): modernize explain tests --- test/integration/crud/explain.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/crud/explain.test.ts b/test/integration/crud/explain.test.ts index d00caff025..254781d0a4 100644 --- a/test/integration/crud/explain.test.ts +++ b/test/integration/crud/explain.test.ts @@ -74,6 +74,7 @@ describe('CRUD API explain option', function () { afterEach(async function () { await collection.drop(); await client.close(); + commandsStarted = []; }); for (const explainValue of explain) { From 711c17c621560664fb25576b007d260eadf51fb1 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Tue, 13 Jun 2023 13:19:54 -0400 Subject: [PATCH 02/23] grouped aggregates under context block --- test/integration/crud/explain.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/crud/explain.test.ts b/test/integration/crud/explain.test.ts index 254781d0a4..d00caff025 100644 --- a/test/integration/crud/explain.test.ts +++ b/test/integration/crud/explain.test.ts @@ -74,7 +74,6 @@ describe('CRUD API explain option', function () { afterEach(async function () { await collection.drop(); await client.close(); - commandsStarted = []; }); for (const explainValue of explain) { From f2c449bf6a80098d9df00474ab09fd81185d5d33 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Tue, 13 Jun 2023 13:27:57 -0400 Subject: [PATCH 03/23] fixed formatting --- test/integration/crud/explain.test.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/integration/crud/explain.test.ts b/test/integration/crud/explain.test.ts index d00caff025..6b1c30166e 100644 --- a/test/integration/crud/explain.test.ts +++ b/test/integration/crud/explain.test.ts @@ -1,13 +1,7 @@ import { expect } from 'chai'; import { once } from 'events'; -import { - type Collection, - type CommandStartedEvent, - type Db, - type MongoClient, - MongoServerError -} from '../../mongodb'; +import { type Collection, type Db, type MongoClient, MongoServerError } from '../../mongodb'; const explain = [true, false, 'queryPlanner', 'allPlansExecution', 'executionStats', 'invalid']; From cbd60e65e1eacc61dfc407c548893f243a194ddc Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 14 Jun 2023 13:03:10 -0400 Subject: [PATCH 04/23] added server.commanAsync wrapper to server class --- src/sdam/server.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 62c2fd872b..43f9d1ab6e 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -1,3 +1,5 @@ +import { promisify } from 'util'; + import type { Document } from '../bson'; import { type CommandOptions, Connection, type DestroyOptions } from '../cmap/connection'; import { @@ -116,6 +118,11 @@ export class Server extends TypedEventEmitter { pool: ConnectionPool; serverApi?: ServerApi; hello?: Document; + commandAsync: ( + ns: MongoDBNamespace, + cmd: Document, + options: CommandOptions + ) => Promise; [kMonitor]: Monitor | null; /** @event */ @@ -139,6 +146,15 @@ export class Server extends TypedEventEmitter { constructor(topology: Topology, description: ServerDescription, options: ServerOptions) { super(); + this.commandAsync = promisify( + ( + ns: MongoDBNamespace, + cmd: Document, + options: CommandOptions, + callback: Callback + ) => this.command(ns, cmd, options, callback as any) + ); + this.serverApi = options.serverApi; const poolOptions = { hostAddress: description.hostAddress, ...options }; From 95fcf8e0b500d4d4cbe21d1c0629ced9b3e7cca2 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 14 Jun 2023 15:31:01 -0400 Subject: [PATCH 05/23] created AbstractCallbackOperation class changed all operations to extend AbstractCallbackOperation changed all operations from execute to executeCallback --- src/bulk/common.ts | 10 +++-- src/index.ts | 7 +++- src/operations/add_user.ts | 2 +- src/operations/aggregate.ts | 2 +- src/operations/bulk_write.ts | 6 +-- src/operations/collections.ts | 6 +-- src/operations/command.ts | 4 +- src/operations/count.ts | 2 +- src/operations/count_documents.ts | 4 +- src/operations/create_collection.ts | 6 +-- src/operations/delete.ts | 14 ++++--- src/operations/distinct.ts | 2 +- src/operations/drop.ts | 4 +- src/operations/estimated_document_count.ts | 2 +- src/operations/eval.ts | 2 +- src/operations/execute_operation.ts | 26 ++++++------- src/operations/find.ts | 2 +- src/operations/find_and_modify.ts | 2 +- src/operations/get_more.ts | 11 ++++-- src/operations/indexes.ts | 42 ++++++++++++-------- src/operations/insert.ts | 14 +++---- src/operations/is_capped.ts | 6 +-- src/operations/kill_cursors.ts | 15 ++++++-- src/operations/list_collections.ts | 2 +- src/operations/list_databases.ts | 2 +- src/operations/operation.ts | 45 ++++++++++++++-------- src/operations/options_operation.ts | 6 +-- src/operations/profiling_level.ts | 2 +- src/operations/remove_user.ts | 2 +- src/operations/rename.ts | 4 +- src/operations/run_command.ts | 2 +- src/operations/search_indexes/create.ts | 10 +++-- src/operations/search_indexes/drop.ts | 10 +++-- src/operations/search_indexes/update.ts | 10 +++-- src/operations/set_profiling_level.ts | 2 +- src/operations/stats.ts | 4 +- src/operations/update.ts | 14 +++---- src/operations/validate_collection.ts | 2 +- 38 files changed, 184 insertions(+), 124 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index aed1c7fe2f..1c3b5fa5dc 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -13,7 +13,7 @@ import type { CollationOptions, CommandOperationOptions } from '../operations/co import { DeleteOperation, type DeleteStatement, makeDeleteStatement } from '../operations/delete'; import { executeOperation } from '../operations/execute_operation'; import { InsertOperation } from '../operations/insert'; -import { AbstractOperation, type Hint } from '../operations/operation'; +import { AbstractCallbackOperation, type Hint } from '../operations/operation'; import { makeUpdateStatement, UpdateOperation, type UpdateStatement } from '../operations/update'; import type { Server } from '../sdam/server'; import type { Topology } from '../sdam/topology'; @@ -881,14 +881,18 @@ export interface BulkWriteOptions extends CommandOperationOptions { * We would like this logic to simply live inside the BulkWriteOperation class * @internal */ -class BulkWriteShimOperation extends AbstractOperation { +class BulkWriteShimOperation extends AbstractCallbackOperation { bulkOperation: BulkOperationBase; constructor(bulkOperation: BulkOperationBase, options: BulkWriteOptions) { super(options); this.bulkOperation = bulkOperation; } - execute(server: Server, session: ClientSession | undefined, callback: Callback): void { + executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void { if (this.options.session == null) { // An implicit session could have been created by 'executeOperation' // So if we stick it on finalOptions here, each bulk operation diff --git a/src/index.ts b/src/index.ts index a35c41565d..daf30704a1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -419,7 +419,12 @@ export type { export type { InsertManyResult, InsertOneOptions, InsertOneResult } from './operations/insert'; export type { CollectionInfo, ListCollectionsOptions } from './operations/list_collections'; export type { ListDatabasesOptions, ListDatabasesResult } from './operations/list_databases'; -export type { AbstractOperation, Hint, OperationOptions } from './operations/operation'; +export type { + AbstractCallbackOperation, + AbstractOperation, + Hint, + OperationOptions +} from './operations/operation'; export type { ProfilingLevelOptions } from './operations/profiling_level'; export type { RemoveUserOptions } from './operations/remove_user'; export type { RenameOptions } from './operations/rename'; diff --git a/src/operations/add_user.ts b/src/operations/add_user.ts index 2bcc76d97b..b60381f6d4 100644 --- a/src/operations/add_user.ts +++ b/src/operations/add_user.ts @@ -50,7 +50,7 @@ export class AddUserOperation extends CommandOperation { this.options = options ?? {}; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/aggregate.ts b/src/operations/aggregate.ts index dfe60e4b2a..301f39cfd3 100644 --- a/src/operations/aggregate.ts +++ b/src/operations/aggregate.ts @@ -88,7 +88,7 @@ export class AggregateOperation extends CommandOperation { this.pipeline.push(stage); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index f56fd8c5a1..baeba6facb 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -8,10 +8,10 @@ import type { Collection } from '../collection'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import type { Callback } from '../utils'; -import { AbstractOperation, Aspect, defineAspects } from './operation'; +import { AbstractCallbackOperation, Aspect, defineAspects } from './operation'; /** @internal */ -export class BulkWriteOperation extends AbstractOperation { +export class BulkWriteOperation extends AbstractCallbackOperation { override options: BulkWriteOptions; collection: Collection; operations: AnyBulkWriteOperation[]; @@ -27,7 +27,7 @@ export class BulkWriteOperation extends AbstractOperation { this.operations = operations; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/collections.ts b/src/operations/collections.ts index 151a230ea6..3047e84535 100644 --- a/src/operations/collections.ts +++ b/src/operations/collections.ts @@ -3,14 +3,14 @@ import type { Db } from '../db'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import type { Callback } from '../utils'; -import { AbstractOperation, type OperationOptions } from './operation'; +import { AbstractCallbackOperation, type OperationOptions } from './operation'; export interface CollectionsOptions extends OperationOptions { nameOnly?: boolean; } /** @internal */ -export class CollectionsOperation extends AbstractOperation { +export class CollectionsOperation extends AbstractCallbackOperation { override options: CollectionsOptions; db: Db; @@ -20,7 +20,7 @@ export class CollectionsOperation extends AbstractOperation { this.db = db; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/command.ts b/src/operations/command.ts index 8804762df4..0fb804aeb4 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -15,7 +15,7 @@ import { } from '../utils'; import { WriteConcern, type WriteConcernOptions } from '../write_concern'; import type { ReadConcernLike } from './../read_concern'; -import { AbstractOperation, Aspect, type OperationOptions } from './operation'; +import { AbstractCallbackOperation, Aspect, type OperationOptions } from './operation'; /** @public */ export interface CollationOptions { @@ -68,7 +68,7 @@ export interface OperationParent { } /** @internal */ -export abstract class CommandOperation extends AbstractOperation { +export abstract class CommandOperation extends AbstractCallbackOperation { override options: CommandOperationOptions; readConcern?: ReadConcern; writeConcern?: WriteConcern; diff --git a/src/operations/count.ts b/src/operations/count.ts index 35835711c6..0b21bdd359 100644 --- a/src/operations/count.ts +++ b/src/operations/count.ts @@ -32,7 +32,7 @@ export class CountOperation extends CommandOperation { this.query = filter; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/count_documents.ts b/src/operations/count_documents.ts index 21e08ff23b..7775e7016d 100644 --- a/src/operations/count_documents.ts +++ b/src/operations/count_documents.ts @@ -32,12 +32,12 @@ export class CountDocumentsOperation extends AggregateOperation { super(collection.s.namespace, pipeline, options); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { - super.execute(server, session, (err, result) => { + super.executeCallback(server, session, (err, result) => { if (err || !result) { callback(err); return; diff --git a/src/operations/create_collection.ts b/src/operations/create_collection.ts index da245a79ce..ff28e77dd0 100644 --- a/src/operations/create_collection.ts +++ b/src/operations/create_collection.ts @@ -121,7 +121,7 @@ export class CreateCollectionOperation extends CommandOperation { this.name = name; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -170,9 +170,7 @@ export class CreateCollectionOperation extends CommandOperation { if (encryptedFields) { // Create the required index for queryable encryption support. const createIndexOp = new CreateIndexOperation(db, name, { __safeContent__: 1 }, {}); - await new Promise((resolve, reject) => { - createIndexOp.execute(server, session, err => (err ? reject(err) : resolve())); - }); + await createIndexOp.execute(server, session); } return coll; diff --git a/src/operations/delete.ts b/src/operations/delete.ts index 9920e8c91c..c57658e237 100644 --- a/src/operations/delete.ts +++ b/src/operations/delete.ts @@ -60,7 +60,11 @@ export class DeleteOperation extends CommandOperation { return this.statements.every(op => (op.limit != null ? op.limit > 0 : true)); } - override execute(server: Server, session: ClientSession | undefined, callback: Callback): void { + override executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void { const options = this.options ?? {}; const ordered = typeof options.ordered === 'boolean' ? options.ordered : true; const command: Document = { @@ -97,12 +101,12 @@ export class DeleteOneOperation extends DeleteOperation { super(collection.s.namespace, [makeDeleteStatement(filter, { ...options, limit: 1 })], options); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { - super.execute(server, session, (err, res) => { + super.executeCallback(server, session, (err, res) => { if (err || res == null) return callback(err); if (res.code) return callback(new MongoServerError(res)); if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); @@ -121,12 +125,12 @@ export class DeleteManyOperation extends DeleteOperation { super(collection.s.namespace, [makeDeleteStatement(filter, options)], options); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { - super.execute(server, session, (err, res) => { + super.executeCallback(server, session, (err, res) => { if (err || res == null) return callback(err); if (res.code) return callback(new MongoServerError(res)); if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); diff --git a/src/operations/distinct.ts b/src/operations/distinct.ts index 37564ba862..6468074b15 100644 --- a/src/operations/distinct.ts +++ b/src/operations/distinct.ts @@ -38,7 +38,7 @@ export class DistinctOperation extends CommandOperation { this.query = query; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/drop.ts b/src/operations/drop.ts index e859b394fd..69e2e69826 100644 --- a/src/operations/drop.ts +++ b/src/operations/drop.ts @@ -26,7 +26,7 @@ export class DropCollectionOperation extends CommandOperation { this.name = name; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -102,7 +102,7 @@ export class DropDatabaseOperation extends CommandOperation { super(db, options); this.options = options; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/estimated_document_count.ts b/src/operations/estimated_document_count.ts index f3b3d42596..8a9048bbaa 100644 --- a/src/operations/estimated_document_count.ts +++ b/src/operations/estimated_document_count.ts @@ -27,7 +27,7 @@ export class EstimatedDocumentCountOperation extends CommandOperation { this.collectionName = collection.collectionName; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/eval.ts b/src/operations/eval.ts index 26518d0cf7..1f9b5ebbc4 100644 --- a/src/operations/eval.ts +++ b/src/operations/eval.ts @@ -38,7 +38,7 @@ export class EvalOperation extends CommandOperation { }); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index 0be79af93d..bfaf0bb406 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -25,13 +25,13 @@ import { import type { Topology } from '../sdam/topology'; import type { ClientSession } from '../sessions'; import { type Callback, maybeCallback, supportsRetryableWrites } from '../utils'; -import { AbstractOperation, Aspect } from './operation'; +import { AbstractCallbackOperation, Aspect } from './operation'; const MMAPv1_RETRY_WRITES_ERROR_CODE = MONGODB_ERROR_CODES.IllegalOperation; const MMAPv1_RETRY_WRITES_ERROR_MESSAGE = 'This MongoDB deployment does not support retryable writes. Please add retryWrites=false to your connection string.'; -type ResultTypeFromOperation = TOperation extends AbstractOperation +type ResultTypeFromOperation = TOperation extends AbstractCallbackOperation ? K : never; @@ -61,29 +61,29 @@ export interface ExecutionResult { * @param callback - The command result callback */ export function executeOperation< - T extends AbstractOperation, + T extends AbstractCallbackOperation, TResult = ResultTypeFromOperation >(client: MongoClient, operation: T): Promise; export function executeOperation< - T extends AbstractOperation, + T extends AbstractCallbackOperation, TResult = ResultTypeFromOperation >(client: MongoClient, operation: T, callback: Callback): void; export function executeOperation< - T extends AbstractOperation, + T extends AbstractCallbackOperation, TResult = ResultTypeFromOperation >(client: MongoClient, operation: T, callback?: Callback): Promise | void; export function executeOperation< - T extends AbstractOperation, + T extends AbstractCallbackOperation, TResult = ResultTypeFromOperation >(client: MongoClient, operation: T, callback?: Callback): Promise | void { return maybeCallback(() => executeOperationAsync(client, operation), callback); } async function executeOperationAsync< - T extends AbstractOperation, + T extends AbstractCallbackOperation, TResult = ResultTypeFromOperation >(client: MongoClient, operation: T): Promise { - if (!(operation instanceof AbstractOperation)) { + if (!(operation instanceof AbstractCallbackOperation)) { // TODO(NODE-3483): Extend MongoRuntimeError throw new MongoRuntimeError('This method requires a valid operation instance'); } @@ -152,13 +152,13 @@ async function executeOperationAsync< if (session == null) { // No session also means it is not retryable, early exit - return operation.executeAsync(server, undefined); + return operation.execute(server, undefined); } if (!operation.hasAspect(Aspect.RETRYABLE)) { // non-retryable operation, early exit try { - return await operation.executeAsync(server, session); + return await operation.execute(server, session); } finally { if (session?.owner != null && session.owner === owner) { await session.endSession().catch(() => null); @@ -184,7 +184,7 @@ async function executeOperationAsync< } try { - return await operation.executeAsync(server, session); + return await operation.execute(server, session); } catch (operationError) { if (willRetry && operationError instanceof MongoError) { return await retryOperation(operation, operationError, { @@ -209,7 +209,7 @@ type RetryOptions = { }; async function retryOperation< - T extends AbstractOperation, + T extends AbstractCallbackOperation, TResult = ResultTypeFromOperation >( operation: T, @@ -257,7 +257,7 @@ async function retryOperation< } try { - return await operation.executeAsync(server, session); + return await operation.execute(server, session); } catch (retryError) { if ( retryError instanceof MongoError && diff --git a/src/operations/find.ts b/src/operations/find.ts index 14b8e70ec6..2d2da710e1 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -102,7 +102,7 @@ export class FindOperation extends CommandOperation { this.filter = filter != null && filter._bsontype === 'ObjectId' ? { _id: filter } : filter; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 4b2e5adcba..a9dca92d79 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -179,7 +179,7 @@ class FindAndModifyOperation extends CommandOperation { this.query = query; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/get_more.ts b/src/operations/get_more.ts index 8281bd1201..0169e0a67f 100644 --- a/src/operations/get_more.ts +++ b/src/operations/get_more.ts @@ -3,7 +3,12 @@ import { MongoRuntimeError } from '../error'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import { type Callback, maxWireVersion, type MongoDBNamespace } from '../utils'; -import { AbstractOperation, Aspect, defineAspects, type OperationOptions } from './operation'; +import { + AbstractCallbackOperation, + Aspect, + defineAspects, + type OperationOptions +} from './operation'; /** @internal */ export interface GetMoreOptions extends OperationOptions { @@ -35,7 +40,7 @@ export interface GetMoreCommand { } /** @internal */ -export class GetMoreOperation extends AbstractOperation { +export class GetMoreOperation extends AbstractCallbackOperation { cursorId: Long; override options: GetMoreOptions; @@ -52,7 +57,7 @@ export class GetMoreOperation extends AbstractOperation { * Although there is a server already associated with the get more operation, the signature * for execute passes a server so we will just use that one. */ - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index f6ef2a1dfe..25542c9807 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -14,7 +14,7 @@ import { type OperationParent } from './command'; import { indexInformation, type IndexInformationOptions } from './common_functions'; -import { AbstractOperation, Aspect, defineAspects } from './operation'; +import { AbstractCallbackOperation, Aspect, defineAspects } from './operation'; const VALID_INDEX_OPTIONS = new Set([ 'background', @@ -176,7 +176,7 @@ function makeIndexSpec( } /** @internal */ -export class IndexesOperation extends AbstractOperation { +export class IndexesOperation extends AbstractCallbackOperation { override options: IndexInformationOptions; collection: Collection; @@ -186,7 +186,7 @@ export class IndexesOperation extends AbstractOperation { this.collection = collection; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -239,7 +239,7 @@ export class CreateIndexesOperation< }); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -288,12 +288,12 @@ export class CreateIndexOperation extends CreateIndexesOperation { ) { super(parent, collectionName, [makeIndexSpec(indexSpec, options)], options); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { - super.execute(server, session, (err, indexNames) => { + super.executeCallback(server, session, (err, indexNames) => { if (err || !indexNames) return callback(err); return callback(undefined, indexNames[0]); }); @@ -317,7 +317,11 @@ export class EnsureIndexOperation extends CreateIndexOperation { this.collectionName = collectionName; } - override execute(server: Server, session: ClientSession | undefined, callback: Callback): void { + override executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void { const indexName = this.indexes[0].name; const cursor = this.db.collection(this.collectionName).listIndexes({ session }); cursor.toArray().then( @@ -327,12 +331,12 @@ export class EnsureIndexOperation extends CreateIndexOperation { callback(undefined, indexName); return; } - super.execute(server, session, callback); + super.executeCallback(server, session, callback); }, error => { if (error instanceof MongoError && error.code === MONGODB_ERROR_CODES.NamespaceNotFound) { // ignore "NamespaceNotFound" errors - return super.execute(server, session, callback); + return super.executeCallback(server, session, callback); } return callback(error); } @@ -357,7 +361,7 @@ export class DropIndexOperation extends CommandOperation { this.indexName = indexName; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -373,8 +377,12 @@ export class DropIndexesOperation extends DropIndexOperation { super(collection, '*', options); } - override execute(server: Server, session: ClientSession | undefined, callback: Callback): void { - super.execute(server, session, err => { + override executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void { + super.executeCallback(server, session, err => { if (err) return callback(err, false); callback(undefined, true); }); @@ -407,7 +415,7 @@ export class ListIndexesOperation extends CommandOperation { this.collectionNamespace = collection.s.namespace; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -429,7 +437,7 @@ export class ListIndexesOperation extends CommandOperation { } /** @internal */ -export class IndexExistsOperation extends AbstractOperation { +export class IndexExistsOperation extends AbstractCallbackOperation { override options: IndexInformationOptions; collection: Collection; indexes: string | string[]; @@ -445,7 +453,7 @@ export class IndexExistsOperation extends AbstractOperation { this.indexes = indexes; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -477,7 +485,7 @@ export class IndexExistsOperation extends AbstractOperation { } /** @internal */ -export class IndexInformationOperation extends AbstractOperation { +export class IndexInformationOperation extends AbstractCallbackOperation { override options: IndexInformationOptions; db: Db; name: string; @@ -489,7 +497,7 @@ export class IndexInformationOperation extends AbstractOperation { this.name = name; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/insert.ts b/src/operations/insert.ts index 7ee9afeca1..ed208cad2e 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -10,7 +10,7 @@ import { WriteConcern } from '../write_concern'; import { BulkWriteOperation } from './bulk_write'; import { CommandOperation, type CommandOperationOptions } from './command'; import { prepareDocs } from './common_functions'; -import { AbstractOperation, Aspect, defineAspects } from './operation'; +import { AbstractCallbackOperation, Aspect, defineAspects } from './operation'; /** @internal */ export class InsertOperation extends CommandOperation { @@ -24,7 +24,7 @@ export class InsertOperation extends CommandOperation { this.documents = documents; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -72,12 +72,12 @@ export class InsertOneOperation extends InsertOperation { super(collection.s.namespace, prepareDocs(collection, [doc], options), options); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { - super.execute(server, session, (err, res) => { + super.executeCallback(server, session, (err, res) => { if (err || res == null) return callback(err); if (res.code) return callback(new MongoServerError(res)); if (res.writeErrors) { @@ -104,7 +104,7 @@ export interface InsertManyResult { } /** @internal */ -export class InsertManyOperation extends AbstractOperation { +export class InsertManyOperation extends AbstractCallbackOperation { override options: BulkWriteOptions; collection: Collection; docs: Document[]; @@ -121,7 +121,7 @@ export class InsertManyOperation extends AbstractOperation { this.docs = docs; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -135,7 +135,7 @@ export class InsertManyOperation extends AbstractOperation { options ); - bulkWriteOperation.execute(server, session, (err, res) => { + bulkWriteOperation.executeCallback(server, session, (err, res) => { if (err || res == null) { if (err && err.message === 'Operation must be an object with an operation key') { err = new MongoInvalidArgumentError( diff --git a/src/operations/is_capped.ts b/src/operations/is_capped.ts index 653f1f49db..b5914a719b 100644 --- a/src/operations/is_capped.ts +++ b/src/operations/is_capped.ts @@ -3,10 +3,10 @@ import { MongoAPIError } from '../error'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import type { Callback } from '../utils'; -import { AbstractOperation, type OperationOptions } from './operation'; +import { AbstractCallbackOperation, type OperationOptions } from './operation'; /** @internal */ -export class IsCappedOperation extends AbstractOperation { +export class IsCappedOperation extends AbstractCallbackOperation { override options: OperationOptions; collection: Collection; @@ -16,7 +16,7 @@ export class IsCappedOperation extends AbstractOperation { this.collection = collection; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/kill_cursors.ts b/src/operations/kill_cursors.ts index 0f0b1f4070..2527bf2004 100644 --- a/src/operations/kill_cursors.ts +++ b/src/operations/kill_cursors.ts @@ -3,7 +3,12 @@ import { MongoRuntimeError } from '../error'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import type { Callback, MongoDBNamespace } from '../utils'; -import { AbstractOperation, Aspect, defineAspects, type OperationOptions } from './operation'; +import { + AbstractCallbackOperation, + Aspect, + defineAspects, + type OperationOptions +} from './operation'; /** * https://www.mongodb.com/docs/manual/reference/command/killCursors/ @@ -15,7 +20,7 @@ interface KillCursorsCommand { comment?: unknown; } -export class KillCursorsOperation extends AbstractOperation { +export class KillCursorsOperation extends AbstractCallbackOperation { cursorId: Long; constructor(cursorId: Long, ns: MongoDBNamespace, server: Server, options: OperationOptions) { @@ -25,7 +30,11 @@ export class KillCursorsOperation extends AbstractOperation { this.server = server; } - execute(server: Server, session: ClientSession | undefined, callback: Callback): void { + executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void { if (server !== this.server) { return callback( new MongoRuntimeError('Killcursor must run on the same server operation began on') diff --git a/src/operations/list_collections.ts b/src/operations/list_collections.ts index 858446381e..339380524a 100644 --- a/src/operations/list_collections.ts +++ b/src/operations/list_collections.ts @@ -47,7 +47,7 @@ export class ListCollectionsOperation extends CommandOperation { } } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/list_databases.ts b/src/operations/list_databases.ts index 807616c3cf..9046c2963e 100644 --- a/src/operations/list_databases.ts +++ b/src/operations/list_databases.ts @@ -34,7 +34,7 @@ export class ListDatabasesOperation extends CommandOperation diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 5813bbce9a..8947419050 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -62,18 +62,18 @@ export abstract class AbstractOperation { [kSession]: ClientSession | undefined; - executeAsync: (server: Server, session: ClientSession | undefined) => Promise; + // executeAsync: (server: Server, session: ClientSession | undefined) => Promise; constructor(options: OperationOptions = {}) { - this.executeAsync = promisify( - ( - server: Server, - session: ClientSession | undefined, - callback: (e: Error, r: TResult) => void - ) => { - this.execute(server, session, callback as any); - } - ); + // this.executeAsync = promisify( + // ( + // server: Server, + // session: ClientSession | undefined, + // callback: (e: Error, r: TResult) => void + // ) => { + // this.execute(server, session, callback as any); + // } + //); this.readPreference = this.hasAspect(Aspect.WRITE_OPERATION) ? ReadPreference.primary @@ -89,11 +89,7 @@ export abstract class AbstractOperation { this.trySecondaryWrite = false; } - abstract execute( - server: Server, - session: ClientSession | undefined, - callback: Callback - ): void; + abstract execute(server: Server, session: ClientSession | undefined): Promise; hasAspect(aspect: symbol): boolean { const ctor = this.constructor as OperationConstructor; @@ -121,6 +117,25 @@ export abstract class AbstractOperation { } } +/** @internal */ +export abstract class AbstractCallbackOperation extends AbstractOperation { + constructor(options: OperationOptions = {}) { + super(options); + } + + execute(server: Server, session: ClientSession | undefined): Promise { + const x = promisify((callback: (e: Error, r: TResult) => void) => { + this.executeCallback(server, session, callback as any); + }); + return x(); + } + + protected abstract executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void; +} export function defineAspects( operation: OperationConstructor, aspects: symbol | symbol[] | Set diff --git a/src/operations/options_operation.ts b/src/operations/options_operation.ts index 409836506a..7e70185c78 100644 --- a/src/operations/options_operation.ts +++ b/src/operations/options_operation.ts @@ -4,10 +4,10 @@ import { MongoAPIError } from '../error'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import type { Callback } from '../utils'; -import { AbstractOperation, type OperationOptions } from './operation'; +import { AbstractCallbackOperation, type OperationOptions } from './operation'; /** @internal */ -export class OptionsOperation extends AbstractOperation { +export class OptionsOperation extends AbstractCallbackOperation { override options: OperationOptions; collection: Collection; @@ -17,7 +17,7 @@ export class OptionsOperation extends AbstractOperation { this.collection = collection; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/profiling_level.ts b/src/operations/profiling_level.ts index 8bc240b423..6e4e72eda0 100644 --- a/src/operations/profiling_level.ts +++ b/src/operations/profiling_level.ts @@ -17,7 +17,7 @@ export class ProfilingLevelOperation extends CommandOperation { this.options = options; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/remove_user.ts b/src/operations/remove_user.ts index 53f916ce5d..1da6ebd6cd 100644 --- a/src/operations/remove_user.ts +++ b/src/operations/remove_user.ts @@ -19,7 +19,7 @@ export class RemoveUserOperation extends CommandOperation { this.username = username; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/rename.ts b/src/operations/rename.ts index 11e92e1aad..34da94b5dc 100644 --- a/src/operations/rename.ts +++ b/src/operations/rename.ts @@ -38,14 +38,14 @@ export class RenameOperation extends RunAdminCommandOperation { this.newName = newName; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { const coll = this.collection; - super.execute(server, session, (err, doc) => { + super.executeCallback(server, session, (err, doc) => { if (err) return callback(err); // We have an error if (doc?.errmsg) { diff --git a/src/operations/run_command.ts b/src/operations/run_command.ts index a2b3da2d8b..c11013a370 100644 --- a/src/operations/run_command.ts +++ b/src/operations/run_command.ts @@ -55,7 +55,7 @@ export class RunCommandOperation extends CommandOperation { this.command = command; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/search_indexes/create.ts b/src/operations/search_indexes/create.ts index c7260cedd5..11120e2de0 100644 --- a/src/operations/search_indexes/create.ts +++ b/src/operations/search_indexes/create.ts @@ -4,7 +4,7 @@ import type { Collection } from '../../collection'; import type { Server } from '../../sdam/server'; import type { ClientSession } from '../../sessions'; import type { Callback } from '../../utils'; -import { AbstractOperation } from '../operation'; +import { AbstractCallbackOperation } from '../operation'; /** * @public @@ -18,7 +18,7 @@ export interface SearchIndexDescription { } /** @internal */ -export class CreateSearchIndexesOperation extends AbstractOperation { +export class CreateSearchIndexesOperation extends AbstractCallbackOperation { constructor( private readonly collection: Collection, private readonly descriptions: ReadonlyArray @@ -26,7 +26,11 @@ export class CreateSearchIndexesOperation extends AbstractOperation { super(); } - execute(server: Server, session: ClientSession | undefined, callback: Callback): void { + executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void { const namespace = this.collection.fullNamespace; const command = { createSearchIndexes: namespace.collection, diff --git a/src/operations/search_indexes/drop.ts b/src/operations/search_indexes/drop.ts index 4e3ed88c11..e98f522650 100644 --- a/src/operations/search_indexes/drop.ts +++ b/src/operations/search_indexes/drop.ts @@ -4,15 +4,19 @@ import type { Collection } from '../../collection'; import type { Server } from '../../sdam/server'; import type { ClientSession } from '../../sessions'; import type { Callback } from '../../utils'; -import { AbstractOperation } from '../operation'; +import { AbstractCallbackOperation } from '../operation'; /** @internal */ -export class DropSearchIndexOperation extends AbstractOperation { +export class DropSearchIndexOperation extends AbstractCallbackOperation { constructor(private readonly collection: Collection, private readonly name: string) { super(); } - execute(server: Server, session: ClientSession | undefined, callback: Callback): void { + executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void { const namespace = this.collection.fullNamespace; const command: Document = { diff --git a/src/operations/search_indexes/update.ts b/src/operations/search_indexes/update.ts index 0ed63450c3..de7c0f055e 100644 --- a/src/operations/search_indexes/update.ts +++ b/src/operations/search_indexes/update.ts @@ -4,10 +4,10 @@ import type { Collection } from '../../collection'; import type { Server } from '../../sdam/server'; import type { ClientSession } from '../../sessions'; import type { Callback } from '../../utils'; -import { AbstractOperation } from '../operation'; +import { AbstractCallbackOperation } from '../operation'; /** @internal */ -export class UpdateSearchIndexOperation extends AbstractOperation { +export class UpdateSearchIndexOperation extends AbstractCallbackOperation { constructor( private readonly collection: Collection, private readonly name: string, @@ -16,7 +16,11 @@ export class UpdateSearchIndexOperation extends AbstractOperation { super(); } - execute(server: Server, session: ClientSession | undefined, callback: Callback): void { + executeCallback( + server: Server, + session: ClientSession | undefined, + callback: Callback + ): void { const namespace = this.collection.fullNamespace; const command = { updateSearchIndex: namespace.collection, diff --git a/src/operations/set_profiling_level.ts b/src/operations/set_profiling_level.ts index 4ecbc66626..2d097d2d25 100644 --- a/src/operations/set_profiling_level.ts +++ b/src/operations/set_profiling_level.ts @@ -48,7 +48,7 @@ export class SetProfilingLevelOperation extends CommandOperation this.level = level; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/stats.ts b/src/operations/stats.ts index e37d188971..5ecdea821c 100644 --- a/src/operations/stats.ts +++ b/src/operations/stats.ts @@ -37,7 +37,7 @@ export class CollStatsOperation extends CommandOperation { this.collectionName = collection.collectionName; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -66,7 +66,7 @@ export class DbStatsOperation extends CommandOperation { this.options = options; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback diff --git a/src/operations/update.ts b/src/operations/update.ts index c7b64b6244..2312353044 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -84,7 +84,7 @@ export class UpdateOperation extends CommandOperation { return this.statements.every(op => op.multi == null || op.multi === false); } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback @@ -138,12 +138,12 @@ export class UpdateOneOperation extends UpdateOperation { } } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { - super.execute(server, session, (err, res) => { + super.executeCallback(server, session, (err, res) => { if (err || !res) return callback(err); if (this.explain != null) return callback(undefined, res); if (res.code) return callback(new MongoServerError(res)); @@ -175,12 +175,12 @@ export class UpdateManyOperation extends UpdateOperation { } } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { - super.execute(server, session, (err, res) => { + super.executeCallback(server, session, (err, res) => { if (err || !res) return callback(err); if (this.explain != null) return callback(undefined, res); if (res.code) return callback(new MongoServerError(res)); @@ -231,12 +231,12 @@ export class ReplaceOneOperation extends UpdateOperation { } } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback ): void { - super.execute(server, session, (err, res) => { + super.executeCallback(server, session, (err, res) => { if (err || !res) return callback(err); if (this.explain != null) return callback(undefined, res); if (res.code) return callback(new MongoServerError(res)); diff --git a/src/operations/validate_collection.ts b/src/operations/validate_collection.ts index 7cd1d805ff..c47a0b81b5 100644 --- a/src/operations/validate_collection.ts +++ b/src/operations/validate_collection.ts @@ -34,7 +34,7 @@ export class ValidateCollectionOperation extends CommandOperation { this.collectionName = collectionName; } - override execute( + override executeCallback( server: Server, session: ClientSession | undefined, callback: Callback From b3ba8e6a5d25da08045af398f5d9fbfa15c779b0 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Thu, 15 Jun 2023 15:46:13 -0400 Subject: [PATCH 06/23] tested connection.commandAsync --- test/unit/cmap/connection.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 76e2739932..ae3173412b 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -256,6 +256,22 @@ describe('new Connection()', function () { }); }); + context('when a connection is established', function () { + const inputStream = new InputStream(); + let commandSpy; + let connection; + + beforeEach(function () { + connection = new Connection(inputStream, connectionOptionsDefaults); + commandSpy = sinon.spy(connection, 'command'); + }); + + it('calls the command function through commandAsync', function () { + connection.commandAsync(); + expect(commandSpy).to.have.been.calledOnce; + }); + }); + context('when requestId/responseTo do not match', function () { let callbackSpy; const document = { ok: 1 }; From b9797dee89b045fd42fd09c12dd969cfab0e743f Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Thu, 15 Jun 2023 15:57:57 -0400 Subject: [PATCH 07/23] tested server.commandAsync calls server.command --- test/unit/sdam/server.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/unit/sdam/server.test.ts b/test/unit/sdam/server.test.ts index 56ed004324..1484b4bc39 100644 --- a/test/unit/sdam/server.test.ts +++ b/test/unit/sdam/server.test.ts @@ -64,6 +64,25 @@ describe('Server', () => { {} as any ); }); + + context('when a server is created', function () { + let serverSpy; + let server; + beforeEach(function () { + server = new Server( + topologyWithPlaceholderClient([], {}), + new ServerDescription('a:1'), + {} as any + ); + serverSpy = sinon.spy(server, 'command'); + }); + + it('calls the command function through commandAsync', function () { + server.commandAsync(); + expect(serverSpy).to.have.been.calledOnce; + }); + }); + for (const loadBalanced of [true, false]) { const mode = loadBalanced ? 'loadBalanced' : 'non-loadBalanced'; const contextSuffix = loadBalanced ? ' with connection provided' : ''; From 80ff7b434da97fbcdf94902135affc45e10ce7bc Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Fri, 16 Jun 2023 13:52:02 -0400 Subject: [PATCH 08/23] Minor changes --- src/operations/execute_operation.ts | 4 ++-- src/operations/operation.ts | 14 +------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index bfaf0bb406..7d68a86c76 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -25,13 +25,13 @@ import { import type { Topology } from '../sdam/topology'; import type { ClientSession } from '../sessions'; import { type Callback, maybeCallback, supportsRetryableWrites } from '../utils'; -import { AbstractCallbackOperation, Aspect } from './operation'; +import { AbstractCallbackOperation, type AbstractOperation, Aspect } from './operation'; const MMAPv1_RETRY_WRITES_ERROR_CODE = MONGODB_ERROR_CODES.IllegalOperation; const MMAPv1_RETRY_WRITES_ERROR_MESSAGE = 'This MongoDB deployment does not support retryable writes. Please add retryWrites=false to your connection string.'; -type ResultTypeFromOperation = TOperation extends AbstractCallbackOperation +type ResultTypeFromOperation = TOperation extends AbstractOperation ? K : never; diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 8947419050..f4fb1e97b4 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -62,19 +62,7 @@ export abstract class AbstractOperation { [kSession]: ClientSession | undefined; - // executeAsync: (server: Server, session: ClientSession | undefined) => Promise; - constructor(options: OperationOptions = {}) { - // this.executeAsync = promisify( - // ( - // server: Server, - // session: ClientSession | undefined, - // callback: (e: Error, r: TResult) => void - // ) => { - // this.execute(server, session, callback as any); - // } - //); - this.readPreference = this.hasAspect(Aspect.WRITE_OPERATION) ? ReadPreference.primary : ReadPreference.fromOptions(options) ?? ReadPreference.primary; @@ -118,7 +106,7 @@ export abstract class AbstractOperation { } /** @internal */ -export abstract class AbstractCallbackOperation extends AbstractOperation { +export abstract class AbstractCallbackOperation extends AbstractOperation { constructor(options: OperationOptions = {}) { super(options); } From 430fea5a98e12aa9889afe05e7c08325a79def88 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Tue, 20 Jun 2023 14:24:44 -0400 Subject: [PATCH 09/23] changed default generic changed async tests to await --- src/operations/execute_operation.ts | 4 ++-- src/operations/operation.ts | 7 +++---- test/unit/sdam/server.test.ts | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index 7d68a86c76..bfaf0bb406 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -25,13 +25,13 @@ import { import type { Topology } from '../sdam/topology'; import type { ClientSession } from '../sessions'; import { type Callback, maybeCallback, supportsRetryableWrites } from '../utils'; -import { AbstractCallbackOperation, type AbstractOperation, Aspect } from './operation'; +import { AbstractCallbackOperation, Aspect } from './operation'; const MMAPv1_RETRY_WRITES_ERROR_CODE = MONGODB_ERROR_CODES.IllegalOperation; const MMAPv1_RETRY_WRITES_ERROR_MESSAGE = 'This MongoDB deployment does not support retryable writes. Please add retryWrites=false to your connection string.'; -type ResultTypeFromOperation = TOperation extends AbstractOperation +type ResultTypeFromOperation = TOperation extends AbstractCallbackOperation ? K : never; diff --git a/src/operations/operation.ts b/src/operations/operation.ts index f4fb1e97b4..deb53bd7f4 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -106,16 +106,15 @@ export abstract class AbstractOperation { } /** @internal */ -export abstract class AbstractCallbackOperation extends AbstractOperation { +export abstract class AbstractCallbackOperation extends AbstractOperation { constructor(options: OperationOptions = {}) { super(options); } execute(server: Server, session: ClientSession | undefined): Promise { - const x = promisify((callback: (e: Error, r: TResult) => void) => { + return promisify((callback: (e: Error, r: TResult) => void) => { this.executeCallback(server, session, callback as any); - }); - return x(); + })(); } protected abstract executeCallback( diff --git a/test/unit/sdam/server.test.ts b/test/unit/sdam/server.test.ts index 1484b4bc39..bc33f38656 100644 --- a/test/unit/sdam/server.test.ts +++ b/test/unit/sdam/server.test.ts @@ -77,8 +77,8 @@ describe('Server', () => { serverSpy = sinon.spy(server, 'command'); }); - it('calls the command function through commandAsync', function () { - server.commandAsync(); + it('calls the command function through commandAsync', async function () { + await server.commandAsync(); expect(serverSpy).to.have.been.calledOnce; }); }); From c26267ea74c58926f521f5c99c9253a661b37402 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Tue, 20 Jun 2023 15:53:07 -0400 Subject: [PATCH 10/23] minor edit --- src/operations/execute_operation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index bfaf0bb406..7d68a86c76 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -25,13 +25,13 @@ import { import type { Topology } from '../sdam/topology'; import type { ClientSession } from '../sessions'; import { type Callback, maybeCallback, supportsRetryableWrites } from '../utils'; -import { AbstractCallbackOperation, Aspect } from './operation'; +import { AbstractCallbackOperation, type AbstractOperation, Aspect } from './operation'; const MMAPv1_RETRY_WRITES_ERROR_CODE = MONGODB_ERROR_CODES.IllegalOperation; const MMAPv1_RETRY_WRITES_ERROR_MESSAGE = 'This MongoDB deployment does not support retryable writes. Please add retryWrites=false to your connection string.'; -type ResultTypeFromOperation = TOperation extends AbstractCallbackOperation +type ResultTypeFromOperation = TOperation extends AbstractOperation ? K : never; From 0664585870c5715083b6699159c3f6091ed2c0cf Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Mon, 26 Jun 2023 15:12:39 -0400 Subject: [PATCH 11/23] copied mock server test code --- test/unit/cmap/connection.test.ts | 46 +++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index ae3173412b..cfb6a6bceb 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -1,8 +1,9 @@ import { expect } from 'chai'; import { EventEmitter, once } from 'events'; +import { Server } from 'http'; import { Socket } from 'net'; import * as sinon from 'sinon'; -import { Readable } from 'stream'; +import { Duplex, Readable } from 'stream'; import { setTimeout } from 'timers'; import { promisify } from 'util'; @@ -72,7 +73,7 @@ class FakeSocket extends EventEmitter { } } -class InputStream extends Readable { +class InputStream extends Duplex { writableEnded: boolean; timeout = 0; @@ -87,6 +88,16 @@ class InputStream extends Readable { } } + write( + chunk: any, + encoding?: BufferEncoding | undefined, + cb?: ((error: Error | null | undefined) => void) | undefined + ): boolean; + write(chunk: any, cb?: ((error: Error | null | undefined) => void) | undefined): boolean; + write(chunk: unknown, encoding?: unknown, cb?: unknown): boolean { + this.push; + } + setTimeout(timeout) { this.timeout = timeout; } @@ -186,6 +197,28 @@ describe('new Connection()', function () { }); }); + it.only('calls the command function through commandAsync', async function () { + const inputStream = new InputStream(); + server.setMessageHandler(request => { + const doc = request.document; + if (isHello(doc)) { + request.reply(mock.HELLO); + } + }); + + const options = { + ...connectionOptionsDefaults, + connectionType: Connection, + hostAddress: server.hostAddress() + }; + + const connection: Connection = new Connection(inputStream, options); + const commandSpy = sinon.spy(connection, 'command'); + + await connection.commandAsync(ns('dummy'), { ping: 1 }, {}); + expect(commandSpy).to.have.been.calledOnce; + }); + it('throws a network error with kBeforeHandshake set to true on timeout before handshake', function (done) { server.setMessageHandler(() => { // respond to no requests to trigger timeout event @@ -257,17 +290,18 @@ describe('new Connection()', function () { }); context('when a connection is established', function () { - const inputStream = new InputStream(); + const inputStream = new FakeSocket(); let commandSpy; - let connection; + let connection: Connection; + const document = { ping: 1 }; beforeEach(function () { connection = new Connection(inputStream, connectionOptionsDefaults); commandSpy = sinon.spy(connection, 'command'); }); - it('calls the command function through commandAsync', function () { - connection.commandAsync(); + it('calls the command function through commandAsync', async function () { + await connection.commandAsync(ns('dummy'), document, {}); expect(commandSpy).to.have.been.calledOnce; }); }); From 209bdbb60bc5a032c57397c64750f3f9852b3f3c Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Mon, 26 Jun 2023 16:38:11 -0400 Subject: [PATCH 12/23] created mock server to fix test errors --- test/unit/cmap/connection.test.ts | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index cfb6a6bceb..5ab5b0711d 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -198,25 +198,25 @@ describe('new Connection()', function () { }); it.only('calls the command function through commandAsync', async function () { - const inputStream = new InputStream(); server.setMessageHandler(request => { const doc = request.document; if (isHello(doc)) { request.reply(mock.HELLO); } + request.reply({ ok: 1 }); }); const options = { ...connectionOptionsDefaults, - connectionType: Connection, hostAddress: server.hostAddress() }; - const connection: Connection = new Connection(inputStream, options); + const connectAsync = promisify(connect); + const connection: Connection = await connectAsync(options); const commandSpy = sinon.spy(connection, 'command'); - await connection.commandAsync(ns('dummy'), { ping: 1 }, {}); - expect(commandSpy).to.have.been.calledOnce; + connection.commandAsync(ns('dummy'), { ping: 1 }, {}); + await expect(commandSpy).to.have.been.calledOnce; }); it('throws a network error with kBeforeHandshake set to true on timeout before handshake', function (done) { @@ -289,23 +289,6 @@ describe('new Connection()', function () { }); }); - context('when a connection is established', function () { - const inputStream = new FakeSocket(); - let commandSpy; - let connection: Connection; - const document = { ping: 1 }; - - beforeEach(function () { - connection = new Connection(inputStream, connectionOptionsDefaults); - commandSpy = sinon.spy(connection, 'command'); - }); - - it('calls the command function through commandAsync', async function () { - await connection.commandAsync(ns('dummy'), document, {}); - expect(commandSpy).to.have.been.calledOnce; - }); - }); - context('when requestId/responseTo do not match', function () { let callbackSpy; const document = { ok: 1 }; From 38ebdc3c9926b04e77ca9dcde62bb495cd86986f Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Mon, 26 Jun 2023 16:38:33 -0400 Subject: [PATCH 13/23] removed .only --- test/unit/cmap/connection.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 5ab5b0711d..5543af0631 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -197,7 +197,7 @@ describe('new Connection()', function () { }); }); - it.only('calls the command function through commandAsync', async function () { + it('calls the command function through commandAsync', async function () { server.setMessageHandler(request => { const doc = request.document; if (isHello(doc)) { From 77a5c3c09de8a90f6f39e16c5aea88915d4f2a29 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Tue, 27 Jun 2023 14:16:42 -0400 Subject: [PATCH 14/23] added arguments to commandAsync --- test/unit/sdam/server.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unit/sdam/server.test.ts b/test/unit/sdam/server.test.ts index bc33f38656..6d9633ab77 100644 --- a/test/unit/sdam/server.test.ts +++ b/test/unit/sdam/server.test.ts @@ -9,6 +9,7 @@ import { MongoErrorLabel, MongoNetworkError, MongoNetworkTimeoutError, + ns, ObjectId, Server, ServerDescription, @@ -65,7 +66,7 @@ describe('Server', () => { ); }); - context('when a server is created', function () { + context.only('when a server is created', function () { let serverSpy; let server; beforeEach(function () { @@ -78,7 +79,7 @@ describe('Server', () => { }); it('calls the command function through commandAsync', async function () { - await server.commandAsync(); + await server.commandAsync(ns('dummy'), { ping: 1 }, {}); expect(serverSpy).to.have.been.calledOnce; }); }); From 6d4cad820647c577acfee99342a94db1aca1bd11 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 28 Jun 2023 11:22:13 -0400 Subject: [PATCH 15/23] moved server code to connection test --- test/unit/cmap/connection.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 5543af0631..9c557dcc70 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -211,12 +211,16 @@ describe('new Connection()', function () { hostAddress: server.hostAddress() }; + const serverSpy = sinon.spy(server, 'command'); const connectAsync = promisify(connect); const connection: Connection = await connectAsync(options); const commandSpy = sinon.spy(connection, 'command'); - connection.commandAsync(ns('dummy'), { ping: 1 }, {}); - await expect(commandSpy).to.have.been.calledOnce; + await connection.commandAsync(ns('dummy'), { ping: 1 }, {}); + expect(commandSpy).to.have.been.calledOnce; + + await server.commandAsync(ns('dummy'), { ping: 1 }, {}); + expect(serverSpy).to.have.been.calledOnce; }); it('throws a network error with kBeforeHandshake set to true on timeout before handshake', function (done) { From 9580b071901722b23daf478b3820126cb514f2e2 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 28 Jun 2023 12:06:56 -0400 Subject: [PATCH 16/23] cleaned server test --- test/unit/cmap/connection.test.ts | 4 ++-- test/unit/sdam/server.test.ts | 14 ++------------ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 9c557dcc70..f8293481d9 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -3,7 +3,7 @@ import { EventEmitter, once } from 'events'; import { Server } from 'http'; import { Socket } from 'net'; import * as sinon from 'sinon'; -import { Duplex, Readable } from 'stream'; +import { Readable } from 'stream'; import { setTimeout } from 'timers'; import { promisify } from 'util'; @@ -73,7 +73,7 @@ class FakeSocket extends EventEmitter { } } -class InputStream extends Duplex { +class InputStream extends Readable { writableEnded: boolean; timeout = 0; diff --git a/test/unit/sdam/server.test.ts b/test/unit/sdam/server.test.ts index 6d9633ab77..14f0c2bf13 100644 --- a/test/unit/sdam/server.test.ts +++ b/test/unit/sdam/server.test.ts @@ -66,19 +66,9 @@ describe('Server', () => { ); }); - context.only('when a server is created', function () { - let serverSpy; - let server; - beforeEach(function () { - server = new Server( - topologyWithPlaceholderClient([], {}), - new ServerDescription('a:1'), - {} as any - ); - serverSpy = sinon.spy(server, 'command'); - }); - + context('when a server is created', function () { it('calls the command function through commandAsync', async function () { + const serverSpy = sinon.spy(server, 'command'); await server.commandAsync(ns('dummy'), { ping: 1 }, {}); expect(serverSpy).to.have.been.calledOnce; }); From b1e41dc808627b0760a575b3af5fb301e8fcaa9a Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 28 Jun 2023 12:58:39 -0400 Subject: [PATCH 17/23] added sinon.stub to give mock command reply --- test/unit/cmap/connection.test.ts | 4 ---- test/unit/sdam/server.test.ts | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index f8293481d9..f1ce5a1382 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -211,16 +211,12 @@ describe('new Connection()', function () { hostAddress: server.hostAddress() }; - const serverSpy = sinon.spy(server, 'command'); const connectAsync = promisify(connect); const connection: Connection = await connectAsync(options); const commandSpy = sinon.spy(connection, 'command'); await connection.commandAsync(ns('dummy'), { ping: 1 }, {}); expect(commandSpy).to.have.been.calledOnce; - - await server.commandAsync(ns('dummy'), { ping: 1 }, {}); - expect(serverSpy).to.have.been.calledOnce; }); it('throws a network error with kBeforeHandshake set to true on timeout before handshake', function (done) { diff --git a/test/unit/sdam/server.test.ts b/test/unit/sdam/server.test.ts index 14f0c2bf13..7e55b59ba9 100644 --- a/test/unit/sdam/server.test.ts +++ b/test/unit/sdam/server.test.ts @@ -68,7 +68,7 @@ describe('Server', () => { context('when a server is created', function () { it('calls the command function through commandAsync', async function () { - const serverSpy = sinon.spy(server, 'command'); + const serverSpy = sinon.stub(server, 'command').yieldsRight(undefined, { ok: 1 }); await server.commandAsync(ns('dummy'), { ping: 1 }, {}); expect(serverSpy).to.have.been.calledOnce; }); From 0f2cdb33faf6c142e41420118bb12c5f5316aca8 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 28 Jun 2023 14:37:19 -0400 Subject: [PATCH 18/23] changed execute to executeCallback in tests --- test/unit/operations/find.test.ts | 4 ++-- test/unit/operations/get_more.test.ts | 18 +++++++++--------- test/unit/operations/kill_cursors.test.ts | 19 ++++++++++--------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/test/unit/operations/find.test.ts b/test/unit/operations/find.test.ts index b85f783db8..cd7d950da6 100644 --- a/test/unit/operations/find.test.ts +++ b/test/unit/operations/find.test.ts @@ -43,7 +43,7 @@ describe('FindOperation', function () { it('should build basic find command with filter', async () => { const findOperation = new FindOperation(undefined, namespace, filter); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(findOperation.execute.bind(findOperation))(server, undefined); + await promisify(findOperation.executeCallback.bind(findOperation))(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, { find: namespace.collection, filter @@ -56,7 +56,7 @@ describe('FindOperation', function () { }; const findOperation = new FindOperation(undefined, namespace, {}, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(findOperation.execute.bind(findOperation))(server, undefined); + await promisify(findOperation.executeCallback.bind(findOperation))(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('oplogReplay', options.oplogReplay) diff --git a/test/unit/operations/get_more.test.ts b/test/unit/operations/get_more.test.ts index 8705527b7d..d2d66a99f2 100644 --- a/test/unit/operations/get_more.test.ts +++ b/test/unit/operations/get_more.test.ts @@ -64,7 +64,7 @@ describe('GetMoreOperation', function () { maxTimeMS: 500 }; - await promisify(operation.execute.bind(operation))(server, undefined); + await promisify(operation.executeCallback.bind(operation))(server, undefined); expect(stub.calledOnce).to.be.true; const call = stub.getCall(0); expect(call.args[0]).to.equal(namespace); @@ -93,7 +93,7 @@ describe('GetMoreOperation', function () { expect(error.message).to.equal('Getmore must run on the same server operation began on'); done(); }; - operation.execute(server2, session, callback); + operation.executeCallback(server2, session, callback); }); }); @@ -109,7 +109,7 @@ describe('GetMoreOperation', function () { it('should build basic getMore command with cursorId and collection', async () => { const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, {}); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(getMoreOperation.execute.bind(getMoreOperation))(server, undefined); + await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, { getMore: cursorId, collection: namespace.collection @@ -122,7 +122,7 @@ describe('GetMoreOperation', function () { }; const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(getMoreOperation.execute.bind(getMoreOperation))(server, undefined); + await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('batchSize', options.batchSize) @@ -135,7 +135,7 @@ describe('GetMoreOperation', function () { }; const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(getMoreOperation.execute.bind(getMoreOperation))(server, undefined); + await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('maxTimeMS', options.maxAwaitTimeMS) @@ -193,7 +193,7 @@ describe('GetMoreOperation', function () { }; const operation = new GetMoreOperation(namespace, cursorId, server, optionsWithComment); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(operation.execute.bind(operation))(server, undefined); + await promisify(operation.executeCallback.bind(operation))(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, getMore); }); } @@ -216,7 +216,7 @@ describe('GetMoreOperation', function () { server, options ); - const error = await promisify(getMoreOperation.execute.bind(getMoreOperation))( + const error = await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))( server, undefined ).catch(error => error); @@ -230,7 +230,7 @@ describe('GetMoreOperation', function () { server, options ); - const error = await promisify(getMoreOperation.execute.bind(getMoreOperation))( + const error = await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))( server, undefined ).catch(error => error); @@ -244,7 +244,7 @@ describe('GetMoreOperation', function () { server, options ); - const error = await promisify(getMoreOperation.execute.bind(getMoreOperation))( + const error = await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))( server, undefined ).catch(error => error); diff --git a/test/unit/operations/kill_cursors.test.ts b/test/unit/operations/kill_cursors.test.ts index e820e9d9f8..e736992aa4 100644 --- a/test/unit/operations/kill_cursors.test.ts +++ b/test/unit/operations/kill_cursors.test.ts @@ -65,10 +65,9 @@ describe('class KillCursorsOperation', () => { options ) as any; - const error = await promisify(killCursorsOperation.execute.bind(killCursorsOperation))( - differentServer, - undefined - ).catch(error => error); + const error = await promisify( + killCursorsOperation.executeCallback.bind(killCursorsOperation) + )(differentServer, undefined).catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -81,10 +80,9 @@ describe('class KillCursorsOperation', () => { options ) as any; - const error = await promisify(killCursorsOperation.execute.bind(killCursorsOperation))( - server, - undefined - ).catch(error => error); + const error = await promisify( + killCursorsOperation.executeCallback.bind(killCursorsOperation) + )(server, undefined).catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -97,7 +95,10 @@ describe('class KillCursorsOperation', () => { options ) as any; const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(killCursorsOperation.execute.bind(killCursorsOperation))(server, undefined); + await promisify(killCursorsOperation.executeCallback.bind(killCursorsOperation))( + server, + undefined + ); expect(stub).to.have.been.calledOnceWith(namespace, { killCursors: namespace.collection, cursors: [cursorId] From b558f05f278017a3d78bbc04d07d1568a108bfdf Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 28 Jun 2023 15:08:02 -0400 Subject: [PATCH 19/23] removed unused variables --- test/unit/cmap/connection.test.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index f1ce5a1382..42c4dfae9d 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -1,6 +1,5 @@ import { expect } from 'chai'; import { EventEmitter, once } from 'events'; -import { Server } from 'http'; import { Socket } from 'net'; import * as sinon from 'sinon'; import { Readable } from 'stream'; @@ -88,16 +87,6 @@ class InputStream extends Readable { } } - write( - chunk: any, - encoding?: BufferEncoding | undefined, - cb?: ((error: Error | null | undefined) => void) | undefined - ): boolean; - write(chunk: any, cb?: ((error: Error | null | undefined) => void) | undefined): boolean; - write(chunk: unknown, encoding?: unknown, cb?: unknown): boolean { - this.push; - } - setTimeout(timeout) { this.timeout = timeout; } From 82c14290f725ad3a78edd432fdcf29de052474f6 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 28 Jun 2023 15:27:39 -0400 Subject: [PATCH 20/23] removed promisify --- test/unit/operations/find.test.ts | 5 ++-- test/unit/operations/get_more.test.ts | 32 ++++++++++------------- test/unit/operations/kill_cursors.test.ts | 18 +++++-------- 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/test/unit/operations/find.test.ts b/test/unit/operations/find.test.ts index cd7d950da6..73b1b9ff72 100644 --- a/test/unit/operations/find.test.ts +++ b/test/unit/operations/find.test.ts @@ -1,6 +1,5 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { promisify } from 'util'; import { FindOperation, ns, Server, ServerDescription } from '../../mongodb'; import { topologyWithPlaceholderClient } from '../../tools/utils'; @@ -43,7 +42,7 @@ describe('FindOperation', function () { it('should build basic find command with filter', async () => { const findOperation = new FindOperation(undefined, namespace, filter); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(findOperation.executeCallback.bind(findOperation))(server, undefined); + await findOperation.execute.bind(findOperation)(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, { find: namespace.collection, filter @@ -56,7 +55,7 @@ describe('FindOperation', function () { }; const findOperation = new FindOperation(undefined, namespace, {}, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(findOperation.executeCallback.bind(findOperation))(server, undefined); + await findOperation.execute.bind(findOperation)(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('oplogReplay', options.oplogReplay) diff --git a/test/unit/operations/get_more.test.ts b/test/unit/operations/get_more.test.ts index d2d66a99f2..4ee81c7bd8 100644 --- a/test/unit/operations/get_more.test.ts +++ b/test/unit/operations/get_more.test.ts @@ -1,6 +1,5 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { promisify } from 'util'; import { Aspect, @@ -64,7 +63,7 @@ describe('GetMoreOperation', function () { maxTimeMS: 500 }; - await promisify(operation.executeCallback.bind(operation))(server, undefined); + await operation.execute.bind(operation)(server, undefined); expect(stub.calledOnce).to.be.true; const call = stub.getCall(0); expect(call.args[0]).to.equal(namespace); @@ -109,7 +108,7 @@ describe('GetMoreOperation', function () { it('should build basic getMore command with cursorId and collection', async () => { const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, {}); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))(server, undefined); + await getMoreOperation.execute.bind(getMoreOperation)(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, { getMore: cursorId, collection: namespace.collection @@ -122,7 +121,7 @@ describe('GetMoreOperation', function () { }; const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))(server, undefined); + await getMoreOperation.execute.bind(getMoreOperation)(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('batchSize', options.batchSize) @@ -135,7 +134,7 @@ describe('GetMoreOperation', function () { }; const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))(server, undefined); + await getMoreOperation.execute.bind(getMoreOperation)(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('maxTimeMS', options.maxAwaitTimeMS) @@ -193,7 +192,7 @@ describe('GetMoreOperation', function () { }; const operation = new GetMoreOperation(namespace, cursorId, server, optionsWithComment); const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(operation.executeCallback.bind(operation))(server, undefined); + await operation.execute.bind(operation)(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, getMore); }); } @@ -216,10 +215,9 @@ describe('GetMoreOperation', function () { server, options ); - const error = await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))( - server, - undefined - ).catch(error => error); + const error = await getMoreOperation.execute + .bind(getMoreOperation)(server, undefined) + .catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -230,10 +228,9 @@ describe('GetMoreOperation', function () { server, options ); - const error = await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))( - server, - undefined - ).catch(error => error); + const error = await getMoreOperation.execute + .bind(getMoreOperation)(server, undefined) + .catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -244,10 +241,9 @@ describe('GetMoreOperation', function () { server, options ); - const error = await promisify(getMoreOperation.executeCallback.bind(getMoreOperation))( - server, - undefined - ).catch(error => error); + const error = await getMoreOperation.execute + .bind(getMoreOperation)(server, undefined) + .catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); }); diff --git a/test/unit/operations/kill_cursors.test.ts b/test/unit/operations/kill_cursors.test.ts index e736992aa4..445f7207e7 100644 --- a/test/unit/operations/kill_cursors.test.ts +++ b/test/unit/operations/kill_cursors.test.ts @@ -1,6 +1,5 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { promisify } from 'util'; import { KillCursorsOperation, @@ -65,9 +64,9 @@ describe('class KillCursorsOperation', () => { options ) as any; - const error = await promisify( - killCursorsOperation.executeCallback.bind(killCursorsOperation) - )(differentServer, undefined).catch(error => error); + const error = await killCursorsOperation.execute + .bind(killCursorsOperation)(differentServer, undefined) + .catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -80,9 +79,9 @@ describe('class KillCursorsOperation', () => { options ) as any; - const error = await promisify( - killCursorsOperation.executeCallback.bind(killCursorsOperation) - )(server, undefined).catch(error => error); + const error = await killCursorsOperation.execute + .bind(killCursorsOperation)(server, undefined) + .catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -95,10 +94,7 @@ describe('class KillCursorsOperation', () => { options ) as any; const stub = sinon.stub(server, 'command').yieldsRight(); - await promisify(killCursorsOperation.executeCallback.bind(killCursorsOperation))( - server, - undefined - ); + await killCursorsOperation.execute.bind(killCursorsOperation)(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, { killCursors: namespace.collection, cursors: [cursorId] From 49aeed0d8b4fca0cf9b547f8099b964d6de49a63 Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Wed, 14 Jun 2023 15:31:01 -0400 Subject: [PATCH 21/23] created AbstractCallbackOperation class changed all operations to extend AbstractCallbackOperation changed all operations from execute to executeCallback --- src/operations/execute_operation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index 7d68a86c76..bfaf0bb406 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -25,13 +25,13 @@ import { import type { Topology } from '../sdam/topology'; import type { ClientSession } from '../sessions'; import { type Callback, maybeCallback, supportsRetryableWrites } from '../utils'; -import { AbstractCallbackOperation, type AbstractOperation, Aspect } from './operation'; +import { AbstractCallbackOperation, Aspect } from './operation'; const MMAPv1_RETRY_WRITES_ERROR_CODE = MONGODB_ERROR_CODES.IllegalOperation; const MMAPv1_RETRY_WRITES_ERROR_MESSAGE = 'This MongoDB deployment does not support retryable writes. Please add retryWrites=false to your connection string.'; -type ResultTypeFromOperation = TOperation extends AbstractOperation +type ResultTypeFromOperation = TOperation extends AbstractCallbackOperation ? K : never; From a055b408a3cbbf62aa0d9d9949f3619e409d9b1a Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Thu, 29 Jun 2023 11:18:38 -0400 Subject: [PATCH 22/23] explain --- test/integration/crud/explain.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/integration/crud/explain.test.ts b/test/integration/crud/explain.test.ts index 6b1c30166e..d00caff025 100644 --- a/test/integration/crud/explain.test.ts +++ b/test/integration/crud/explain.test.ts @@ -1,7 +1,13 @@ import { expect } from 'chai'; import { once } from 'events'; -import { type Collection, type Db, type MongoClient, MongoServerError } from '../../mongodb'; +import { + type Collection, + type CommandStartedEvent, + type Db, + type MongoClient, + MongoServerError +} from '../../mongodb'; const explain = [true, false, 'queryPlanner', 'allPlansExecution', 'executionStats', 'invalid']; From 14ab51e71d075137002a5215a688121daff0697f Mon Sep 17 00:00:00 2001 From: Malik Javaid Date: Thu, 29 Jun 2023 13:08:06 -0400 Subject: [PATCH 23/23] removed .bind --- test/unit/operations/find.test.ts | 4 ++-- test/unit/operations/get_more.test.ts | 22 ++++++++-------------- test/unit/operations/kill_cursors.test.ts | 10 ++++------ 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/test/unit/operations/find.test.ts b/test/unit/operations/find.test.ts index 73b1b9ff72..657013de1c 100644 --- a/test/unit/operations/find.test.ts +++ b/test/unit/operations/find.test.ts @@ -42,7 +42,7 @@ describe('FindOperation', function () { it('should build basic find command with filter', async () => { const findOperation = new FindOperation(undefined, namespace, filter); const stub = sinon.stub(server, 'command').yieldsRight(); - await findOperation.execute.bind(findOperation)(server, undefined); + await findOperation.execute(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, { find: namespace.collection, filter @@ -55,7 +55,7 @@ describe('FindOperation', function () { }; const findOperation = new FindOperation(undefined, namespace, {}, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await findOperation.execute.bind(findOperation)(server, undefined); + await findOperation.execute(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('oplogReplay', options.oplogReplay) diff --git a/test/unit/operations/get_more.test.ts b/test/unit/operations/get_more.test.ts index 4ee81c7bd8..3abb798bc0 100644 --- a/test/unit/operations/get_more.test.ts +++ b/test/unit/operations/get_more.test.ts @@ -63,7 +63,7 @@ describe('GetMoreOperation', function () { maxTimeMS: 500 }; - await operation.execute.bind(operation)(server, undefined); + await operation.execute(server, undefined); expect(stub.calledOnce).to.be.true; const call = stub.getCall(0); expect(call.args[0]).to.equal(namespace); @@ -108,7 +108,7 @@ describe('GetMoreOperation', function () { it('should build basic getMore command with cursorId and collection', async () => { const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, {}); const stub = sinon.stub(server, 'command').yieldsRight(); - await getMoreOperation.execute.bind(getMoreOperation)(server, undefined); + await getMoreOperation.execute(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, { getMore: cursorId, collection: namespace.collection @@ -121,7 +121,7 @@ describe('GetMoreOperation', function () { }; const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await getMoreOperation.execute.bind(getMoreOperation)(server, undefined); + await getMoreOperation.execute(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('batchSize', options.batchSize) @@ -134,7 +134,7 @@ describe('GetMoreOperation', function () { }; const getMoreOperation = new GetMoreOperation(namespace, cursorId, server, options); const stub = sinon.stub(server, 'command').yieldsRight(); - await getMoreOperation.execute.bind(getMoreOperation)(server, undefined); + await getMoreOperation.execute(server, undefined); expect(stub).to.have.been.calledOnceWith( namespace, sinon.match.has('maxTimeMS', options.maxAwaitTimeMS) @@ -192,7 +192,7 @@ describe('GetMoreOperation', function () { }; const operation = new GetMoreOperation(namespace, cursorId, server, optionsWithComment); const stub = sinon.stub(server, 'command').yieldsRight(); - await operation.execute.bind(operation)(server, undefined); + await operation.execute(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, getMore); }); } @@ -215,9 +215,7 @@ describe('GetMoreOperation', function () { server, options ); - const error = await getMoreOperation.execute - .bind(getMoreOperation)(server, undefined) - .catch(error => error); + const error = await getMoreOperation.execute(server, undefined).catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -228,9 +226,7 @@ describe('GetMoreOperation', function () { server, options ); - const error = await getMoreOperation.execute - .bind(getMoreOperation)(server, undefined) - .catch(error => error); + const error = await getMoreOperation.execute(server, undefined).catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -241,9 +237,7 @@ describe('GetMoreOperation', function () { server, options ); - const error = await getMoreOperation.execute - .bind(getMoreOperation)(server, undefined) - .catch(error => error); + const error = await getMoreOperation.execute(server, undefined).catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); }); diff --git a/test/unit/operations/kill_cursors.test.ts b/test/unit/operations/kill_cursors.test.ts index 445f7207e7..4468cd33c6 100644 --- a/test/unit/operations/kill_cursors.test.ts +++ b/test/unit/operations/kill_cursors.test.ts @@ -64,8 +64,8 @@ describe('class KillCursorsOperation', () => { options ) as any; - const error = await killCursorsOperation.execute - .bind(killCursorsOperation)(differentServer, undefined) + const error = await killCursorsOperation + .execute(differentServer, undefined) .catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); @@ -79,9 +79,7 @@ describe('class KillCursorsOperation', () => { options ) as any; - const error = await killCursorsOperation.execute - .bind(killCursorsOperation)(server, undefined) - .catch(error => error); + const error = await killCursorsOperation.execute(server, undefined).catch(error => error); expect(error).to.be.instanceOf(MongoRuntimeError); }); @@ -94,7 +92,7 @@ describe('class KillCursorsOperation', () => { options ) as any; const stub = sinon.stub(server, 'command').yieldsRight(); - await killCursorsOperation.execute.bind(killCursorsOperation)(server, undefined); + await killCursorsOperation.execute(server, undefined); expect(stub).to.have.been.calledOnceWith(namespace, { killCursors: namespace.collection, cursors: [cursorId]