refactor: Remove generic options parameter from OperationBase

src/operations/add_user.ts

@@ -2,7 +2,7 @@ import * as crypto from 'crypto';
 import { Aspect, defineAspects } from './operation';
 import { CommandOperation, CommandOperationOptions } from './command';
 import { MongoError } from '../error';
-import { Callback, getTopology } from '../utils';
+import { Callback, deepFreeze, getTopology } from '../utils';
 import type { Document } from '../bson';
 import type { Server } from '../sdam/server';
 import type { Db } from '../db';
@@ -22,8 +22,23 @@ export class AddUserOperation extends CommandOperation<AddUserOptions, Document>
   db: Db;
   username: string;
   password?: string;
+  private roles;
+  private customData;
+
+  getOptions(): Readonly<AddUserOptions> {
+    return deepFreeze({
+      ...super.getOptions(),
+      roles: this.roles,
+      customData: this.customData
+    });
+  }
 
-  constructor(db: Db, username: string, password: string | undefined, options?: AddUserOptions) {
+  constructor(
+    db: Db,
+    username: string,
+    password: string | undefined,
+    options: AddUserOptions = {}
+  ) {
     super(db, options);
 
     // Special case where there is no password ($external users)
@@ -35,23 +50,23 @@ export class AddUserOperation extends CommandOperation<AddUserOptions, Document>
     this.db = db;
     this.username = username;
     this.password = password;
+    this.roles = options.roles;
+    this.customData = options.customData;
+
+    // Error out if digestPassword set
+    if (options?.digestPassword != null) {
+      throw new MongoError(
+        'The digestPassword option is not supported via add_user. ' +
+          "Please use db.command('createUser', ...) instead for this option."
+      );
+    }
   }
 
   execute(server: Server, callback: Callback<Document>): void {
     const db = this.db;
     const username = this.username;
     const password = this.password;
-    const options = this.options;
-
-    // Error out if digestPassword set
-    if (options.digestPassword != null) {
-      return callback(
-        new MongoError(
-          'The digestPassword option is not supported via add_user. ' +
-            "Please use db.command('createUser', ...) instead for this option."
-        )
-      );
-    }
+    const options = this.getOptions();
 
     // Get additional values
     let roles: string[] = [];

src/operations/aggregate.ts

@@ -1,7 +1,7 @@
 import { CommandOperation, CommandOperationOptions, OperationParent } from './command';
 import { ReadPreference } from '../read_preference';
 import { MongoError } from '../error';
-import { maxWireVersion } from '../utils';
+import { deepFreeze, maxWireVersion } from '../utils';
 import { Aspect, defineAspects, Hint } from './operation';
 import type { Callback } from '../utils';
 import type { Document } from '../bson';
@@ -40,8 +40,34 @@ export class AggregateOperation<T = Document> extends CommandOperation<Aggregate
   target: string | typeof DB_AGGREGATE_COLLECTION;
   pipeline: Document[];
   hasWriteStage: boolean;
+  protected allowDiskUse;
+  protected batchSize;
+  protected bypassDocumentValidation;
+  protected cursor;
+  protected explain;
+  protected maxTimeMS;
+  protected maxAwaitTimeMS;
+  protected collation;
+  protected hint;
+  protected out;
+
+  getOptions(): Readonly<AggregateOptions> {
+    return deepFreeze({
+      ...super.getOptions(),
+      batchSize: this.batchSize,
+      allowDiskUse: this.allowDiskUse,
+      cursor: this.cursor,
+      bypassDocumentValidation: this.bypassDocumentValidation,
+      maxTimeMS: this.maxTimeMS,
+      explain: this.explain,
+      collation: this.collation,
+      maxAwaitTimeMS: this.maxAwaitTimeMS,
+      out: this.out,
+      hint: this.hint
+    });
+  }
 
-  constructor(parent: OperationParent, pipeline: Document[], options?: AggregateOptions) {
+  constructor(parent: OperationParent, pipeline: Document[], options: AggregateOptions = {}) {
     super(parent, options);
 
     this.target =
@@ -76,6 +102,17 @@ export class AggregateOperation<T = Document> extends CommandOperation<Aggregate
     if (options?.cursor != null && typeof options.cursor !== 'object') {
       throw new MongoError('cursor options must be an object');
     }
+
+    this.batchSize = options.batchSize;
+    this.allowDiskUse = options.allowDiskUse;
+    this.cursor = options.cursor;
+    this.bypassDocumentValidation = options.bypassDocumentValidation;
+    this.maxTimeMS = options.maxTimeMS;
+    this.explain = options.explain;
+    this.collation = options.collation;
+    this.maxAwaitTimeMS = options.maxAwaitTimeMS;
+    this.out = options.out;
+    this.hint = options.hint;
   }
 
   get canRetryRead(): boolean {
@@ -87,7 +124,7 @@ export class AggregateOperation<T = Document> extends CommandOperation<Aggregate
   }
 
   execute(server: Server, callback: Callback<T>): void {
-    const options: AggregateOptions = this.options;
+    const options = this.getOptions();
     const serverWireVersion = maxWireVersion(server);
     const command: Document = { aggregate: this.target, pipeline: this.pipeline };
 

src/operations/bulk_write.ts

@@ -1,4 +1,4 @@
-import { applyRetryableWrites, applyWriteConcern, Callback } from '../utils';
+import { applyRetryableWrites, applyWriteConcern, Callback, deepFreeze } from '../utils';
 import { OperationBase } from './operation';
 import { resolveBSONOptions } from '../bson';
 import { WriteConcern } from '../write_concern';
@@ -15,6 +15,18 @@ import type { Server } from '../sdam/server';
 export class BulkWriteOperation extends OperationBase<BulkWriteOptions, BulkWriteResult> {
   collection: Collection;
   operations: AnyBulkWriteOperation[];
+  protected bypassDocumentValidation;
+  protected ordered;
+  protected forceServerObjectId;
+
+  getOptions(): Readonly<BulkWriteOptions> {
+    return deepFreeze({
+      ...super.getOptions(),
+      bypassDocumentValidation: this.bypassDocumentValidation,
+      ordered: this.ordered,
+      forceServerObjectId: this.forceServerObjectId
+    });
+  }
 
   constructor(
     collection: Collection,
@@ -28,12 +40,15 @@ export class BulkWriteOperation extends OperationBase<BulkWriteOptions, BulkWrit
 
     // Assign BSON serialize options to OperationBase, preferring options over collection options
     this.bsonOptions = resolveBSONOptions(options, collection);
+    this.bypassDocumentValidation = options.bypassDocumentValidation;
+    this.ordered = options.ordered;
+    this.forceServerObjectId = options.forceServerObjectId;
   }
 
   execute(server: Server, callback: Callback<BulkWriteResult>): void {
     const coll = this.collection;
     const operations = this.operations;
-    const options = { ...this.options, ...this.bsonOptions };
+    const options = this.getOptions();
 
     // Create the bulk operation
     const bulk: BulkOperationBase =

src/operations/collections.ts

@@ -1,10 +1,9 @@
 import { OperationBase, OperationOptions } from './operation';
-import { loadCollection } from '../dynamic_loaders';
-import type { Callback } from '../utils';
+import { Callback, deepFreeze } from '../utils';
 import type { Db } from '../db';
 
 // eslint-disable-next-line
-import type { Collection } from '../collection';
+import { Collection } from '../collection';
 import type { Server } from '../sdam/server';
 
 export interface CollectionsOptions extends OperationOptions {
@@ -14,20 +13,26 @@ export interface CollectionsOptions extends OperationOptions {
 /** @internal */
 export class CollectionsOperation extends OperationBase<CollectionsOptions, Collection[]> {
   db: Db;
+  protected nameOnly;
+
+  getOptions(): Readonly<CollectionsOptions> {
+    return deepFreeze({
+      ...super.getOptions(),
+      nameOnly: this.nameOnly
+    });
+  }
 
   constructor(db: Db, options: CollectionsOptions) {
     super(options);
 
     this.db = db;
+    this.nameOnly = options.nameOnly ?? true;
   }
 
   execute(server: Server, callback: Callback<Collection[]>): void {
     const db = this.db;
-    let options: CollectionsOptions = this.options;
-
-    const Collection = loadCollection();
+    const options = this.getOptions();
 
-    options = Object.assign({}, options, { nameOnly: true });
     // Let's get the collection names
     db.listCollections({}, options).toArray((err, documents) => {
       if (err || !documents) return callback(err);

src/operations/command.ts

@@ -1,7 +1,7 @@
 import { Aspect, OperationBase, OperationOptions } from './operation';
 import { ReadConcern } from '../read_concern';
 import { WriteConcern, WriteConcernOptions } from '../write_concern';
-import { maxWireVersion, MongoDBNamespace, Callback } from '../utils';
+import { maxWireVersion, MongoDBNamespace, Callback, deepFreeze } from '../utils';
 import { ReadPreference, ReadPreferenceLike } from '../read_preference';
 import { commandSupportsReadConcern } from '../sessions';
 import { MongoError } from '../error';
@@ -35,9 +35,14 @@ export interface CommandOperationOptions extends OperationOptions, WriteConcernO
   noResponse?: boolean;
 }
 
+export interface OperationParentPrivate extends BSONSerializeOptions {
+  options?: BSONSerializeOptions;
+  namespace: MongoDBNamespace;
+}
+
 /** @internal */
 export interface OperationParent {
-  s: { namespace: MongoDBNamespace };
+  s: OperationParentPrivate;
   readConcern?: ReadConcern;
   writeConcern?: WriteConcern;
   readPreference?: ReadPreference;
@@ -54,10 +59,31 @@ export abstract class CommandOperation<
   readPreference: ReadPreference;
   readConcern?: ReadConcern;
   writeConcern?: WriteConcern;
-  explain: boolean;
   fullResponse?: boolean;
   logger?: Logger;
 
+  protected collation;
+  protected maxTimeMS;
+  protected comment;
+  protected retryWrites;
+  protected noResponse;
+
+  getOptions(): Readonly<CommandOperationOptions> {
+    return deepFreeze({
+      ...super.getOptions(),
+      collation: this.collation,
+      maxTimeMS: this.maxTimeMS,
+      comment: this.comment,
+      retryWrites: this.retryWrites,
+      noResponse: this.noResponse,
+      fullResponse: this.fullResponse,
+      fullResult: !!this.fullResponse // this is the prop that server.command expects, we keep this here so not to refreeze
+      // Override with proper type
+      // writeConcern: this.writeConcern,
+      // readConcern: this.readConcern
+    });
+  }
+
   constructor(parent?: OperationParent, options?: T) {
     super(options);
 
@@ -76,17 +102,30 @@ export abstract class CommandOperation<
     const propertyProvider = this.hasAspect(Aspect.NO_INHERIT_OPTIONS) ? undefined : parent;
     this.readPreference = this.hasAspect(Aspect.WRITE_OPERATION)
       ? ReadPreference.primary
-      : ReadPreference.resolve(propertyProvider, this.options);
-    this.readConcern = resolveReadConcern(propertyProvider, this.options);
-    this.writeConcern = resolveWriteConcern(propertyProvider, this.options);
+      : ReadPreference.resolve(propertyProvider, options);
+    this.readConcern = resolveReadConcern(propertyProvider, options);
+    this.writeConcern = resolveWriteConcern(propertyProvider, options);
     this.explain = false;
     this.fullResponse =
       options && typeof options.fullResponse === 'boolean' ? options.fullResponse : false;
 
+    // if (this.writeConcern && this.writeConcern.w === 0) {
+    //   if (this.session && this.session.explicit) {
+    //     throw new MongoError('Cannot have explicit session with unacknowledged writes');
+    //   }
+    //   return;
+    // }
+
     // TODO: A lot of our code depends on having the read preference in the options. This should
     //       go away, but also requires massive test rewrites.
     this.options.readPreference = this.readPreference;
 
+    this.collation = options?.collation;
+    this.maxTimeMS = options?.maxTimeMS;
+    this.comment = options?.comment;
+    this.retryWrites = options?.retryWrites;
+    this.noResponse = options?.noResponse;
+
     // TODO(NODE-2056): make logger another "inheritable" property
     if (parent && parent.logger) {
       this.logger = parent.logger;
@@ -101,8 +140,8 @@ export abstract class CommandOperation<
   executeCommand(server: Server, cmd: Document, callback: Callback): void {
     // TODO: consider making this a non-enumerable property
     this.server = server;
+    const options = this.getOptions();
 
-    const options = { ...this.options, ...this.bsonOptions };
     const serverWireVersion = maxWireVersion(server);
     const inTransaction = this.session && this.session.inTransaction();
 
@@ -141,12 +180,7 @@ export abstract class CommandOperation<
       this.logger.debug(`executing command ${JSON.stringify(cmd)} against ${this.ns}`);
     }
 
-    server.command(
-      this.ns.toString(),
-      cmd,
-      { fullResult: !!this.fullResponse, ...this.options },
-      callback
-    );
+    server.command(this.ns.toString(), cmd, options, callback);
   }
 }