Skip to content

Commit 495e86b

Browse files
HanaPearlmanHanaPearlman
authored
refactor: consolidate options inheritance logic (#2614)
A new function, resolveOptions, was introduced to consolidate options resolution logic. It merges in inherited properties from the parent into any provided options. The NO_INHERIT_OPTIONS aspect and the ReadPreference.resolve() function were removed. When appropriate, ReadPreference.fromOptions() now attempts to get a read preference from a session, if one exists on the options. NODE-2870
1 parent f42cfa4 commit 495e86b

File tree

10 files changed

+191
-227
lines changed

10 files changed

+191
-227
lines changed

‎src/collection.ts

Lines changed: 74 additions & 55 deletions
Large diffs are not rendered by default.

‎src/db.ts

Lines changed: 54 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
import { deprecate } from 'util';
2-
import { emitDeprecatedOptionWarning, Callback } from './utils';
2+
import {
3+
emitDeprecatedOptionWarning,
4+
Callback,
5+
resolveOptions,
6+
filterOptions,
7+
deprecateOptions,
8+
MongoDBNamespace,
9+
getTopology
10+
} from './utils';
311
import { loadAdmin } from './dynamic_loaders';
412
import { AggregationCursor, CommandCursor } from './cursor';
513
import { ObjectId, Code, Document, BSONSerializeOptions, resolveBSONOptions } from './bson';
@@ -11,13 +19,6 @@ import * as CONSTANTS from './constants';
1119
import { WriteConcern, WriteConcernOptions } from './write_concern';
1220
import { ReadConcern } from './read_concern';
1321
import { Logger, LoggerOptions } from './logger';
14-
import {
15-
filterOptions,
16-
mergeOptionsAndWriteConcern,
17-
deprecateOptions,
18-
MongoDBNamespace,
19-
getTopology
20-
} from './utils';
2122
import { AggregateOperation, AggregateOptions } from './operations/aggregate';
2223
import { AddUserOperation, AddUserOptions } from './operations/add_user';
2324
import { CollectionsOperation } from './operations/collections';
@@ -254,19 +255,20 @@ export class Db implements OperationParent {
254255
callback?: Callback<Collection>
255256
): Promise<Collection> | void {
256257
if (typeof options === 'function') (callback = options), (options = {});
257-
options = options || {};
258-
options.readConcern = ReadConcern.fromOptions(options) ?? this.readConcern;
259258

260259
return executeOperation(
261260
getTopology(this),
262-
new CreateCollectionOperation(this, name, options),
261+
new CreateCollectionOperation(this, name, resolveOptions(this, options)),
263262
callback
264263
);
265264
}
266265

267266
/**
268267
* Execute a command
269268
*
269+
* @remarks
270+
* This command does not inherit options from the MongoClient.
271+
*
270272
* @param command - The command to run
271273
* @param options - Optional settings for the command
272274
* @param callback - An optional callback, a Promise will be returned if none is provided
@@ -281,11 +283,11 @@ export class Db implements OperationParent {
281283
callback?: Callback<Document>
282284
): Promise<Document> | void {
283285
if (typeof options === 'function') (callback = options), (options = {});
284-
options = options || {};
285286

287+
// Intentionally, we do not inherit options from parent for this operation.
286288
return executeOperation(
287289
getTopology(this),
288-
new RunCommandOperation(this, command, options),
290+
new RunCommandOperation(this, command, options ?? {}),
289291
callback
290292
);
291293
}
@@ -307,8 +309,7 @@ export class Db implements OperationParent {
307309
throw new TypeError('`options` parameter must not be function');
308310
}
309311

310-
options = options || {};
311-
312+
options = resolveOptions(this, options);
312313
const cursor = new AggregationCursor(
313314
getTopology(this),
314315
new AggregateOperation(this, pipeline, options),
@@ -341,21 +342,10 @@ export class Db implements OperationParent {
341342
callback?: Callback<Collection>
342343
): Collection | void {
343344
if (typeof options === 'function') (callback = options), (options = {});
344-
options = Object.assign({}, options);
345-
346-
// If we have not set a collection level readConcern set the db level one
347-
options.readConcern = ReadConcern.fromOptions(options) ?? this.readConcern;
348-
349-
// Merge in all needed options and ensure correct writeConcern merging from db level
350-
const finalOptions = mergeOptionsAndWriteConcern(
351-
options,
352-
this.s.options ?? {},
353-
collectionKeys,
354-
true
355-
) as CollectionOptions;
345+
const finalOptions = resolveOptions(this, options);
356346

357347
// Execute
358-
if (finalOptions == null || !finalOptions.strict) {
348+
if (!finalOptions.strict) {
359349
try {
360350
const collection = new Collection(this, name, finalOptions);
361351
if (callback) callback(undefined, collection);
@@ -414,9 +404,11 @@ export class Db implements OperationParent {
414404
callback?: Callback<Document>
415405
): Promise<Document> | void {
416406
if (typeof options === 'function') (callback = options), (options = {});
417-
options = options || {};
418-
419-
return executeOperation(getTopology(this), new DbStatsOperation(this, options), callback);
407+
return executeOperation(
408+
getTopology(this),
409+
new DbStatsOperation(this, resolveOptions(this, options)),
410+
callback
411+
);
420412
}
421413

422414
/**
@@ -427,7 +419,7 @@ export class Db implements OperationParent {
427419
*/
428420
listCollections(filter?: Document, options?: ListCollectionsOptions): CommandCursor {
429421
filter = filter || {};
430-
options = options || {};
422+
options = resolveOptions(this, options);
431423

432424
return new CommandCursor(
433425
getTopology(this),
@@ -439,6 +431,9 @@ export class Db implements OperationParent {
439431
/**
440432
* Rename a collection.
441433
*
434+
* @remarks
435+
* This operation does not inherit options from the MongoClient.
436+
*
442437
* @param fromCollection - Name of current collection to rename
443438
* @param toCollection - New name of of the collection
444439
* @param options - Optional settings for the command
@@ -468,7 +463,9 @@ export class Db implements OperationParent {
468463
callback?: Callback<Collection>
469464
): Promise<Collection> | void {
470465
if (typeof options === 'function') (callback = options), (options = {});
471-
options = Object.assign({}, options, { readPreference: ReadPreference.PRIMARY });
466+
467+
// Intentionally, we do not inherit options from parent for this operation.
468+
options = { ...options, readPreference: ReadPreference.PRIMARY };
472469

473470
// Add return new collection
474471
options.new_collection = true;
@@ -497,11 +494,10 @@ export class Db implements OperationParent {
497494
callback?: Callback<boolean>
498495
): Promise<boolean> | void {
499496
if (typeof options === 'function') (callback = options), (options = {});
500-
options = options || {};
501497

502498
return executeOperation(
503499
getTopology(this),
504-
new DropCollectionOperation(this, name, options),
500+
new DropCollectionOperation(this, name, resolveOptions(this, options)),
505501
callback
506502
);
507503
}
@@ -521,9 +517,12 @@ export class Db implements OperationParent {
521517
callback?: Callback<boolean>
522518
): Promise<boolean> | void {
523519
if (typeof options === 'function') (callback = options), (options = {});
524-
options = options || {};
525520

526-
return executeOperation(getTopology(this), new DropDatabaseOperation(this, options), callback);
521+
return executeOperation(
522+
getTopology(this),
523+
new DropDatabaseOperation(this, resolveOptions(this, options)),
524+
callback
525+
);
527526
}
528527

529528
/**
@@ -541,14 +540,20 @@ export class Db implements OperationParent {
541540
callback?: Callback<Collection[]>
542541
): Promise<Collection[]> | void {
543542
if (typeof options === 'function') (callback = options), (options = {});
544-
options = options || {};
545543

546-
return executeOperation(getTopology(this), new CollectionsOperation(this, options), callback);
544+
return executeOperation(
545+
getTopology(this),
546+
new CollectionsOperation(this, resolveOptions(this, options)),
547+
callback
548+
);
547549
}
548550

549551
/**
550552
* Runs a command on the database as admin.
551553
*
554+
* @remarks
555+
* This command does not inherit options from the MongoClient.
556+
*
552557
* @param command - The command to run
553558
* @param options - Optional settings for the command
554559
* @param callback - An optional callback, a Promise will be returned if none is provided
@@ -567,11 +572,11 @@ export class Db implements OperationParent {
567572
callback?: Callback<void>
568573
): Promise<void> | void {
569574
if (typeof options === 'function') (callback = options), (options = {});
570-
options = options || {};
571575

576+
// Intentionally, we do not inherit options from parent for this operation.
572577
return executeOperation(
573578
getTopology(this),
574-
new RunAdminCommandOperation(this, command, options),
579+
new RunAdminCommandOperation(this, command, options ?? {}),
575580
callback
576581
);
577582
}
@@ -604,11 +609,10 @@ export class Db implements OperationParent {
604609
callback?: Callback<Document>
605610
): Promise<Document> | void {
606611
if (typeof options === 'function') (callback = options), (options = {});
607-
options = options ? Object.assign({}, options) : {};
608612

609613
return executeOperation(
610614
getTopology(this),
611-
new CreateIndexOperation(this, name, indexSpec, options),
615+
new CreateIndexOperation(this, name, indexSpec, resolveOptions(this, options)),
612616
callback
613617
);
614618
}
@@ -652,10 +656,9 @@ export class Db implements OperationParent {
652656
if (typeof options === 'function') (callback = options), (options = {});
653657
}
654658

655-
options = options || {};
656659
return executeOperation(
657660
getTopology(this),
658-
new AddUserOperation(this, username, password, options),
661+
new AddUserOperation(this, username, password, resolveOptions(this, options)),
659662
callback
660663
);
661664
}
@@ -677,11 +680,10 @@ export class Db implements OperationParent {
677680
callback?: Callback<boolean>
678681
): Promise<boolean> | void {
679682
if (typeof options === 'function') (callback = options), (options = {});
680-
options = options || {};
681683

682684
return executeOperation(
683685
getTopology(this),
684-
new RemoveUserOperation(this, username, options),
686+
new RemoveUserOperation(this, username, resolveOptions(this, options)),
685687
callback
686688
);
687689
}
@@ -710,11 +712,10 @@ export class Db implements OperationParent {
710712
callback?: Callback<ProfilingLevel>
711713
): Promise<ProfilingLevel> | void {
712714
if (typeof options === 'function') (callback = options), (options = {});
713-
options = options || {};
714715

715716
return executeOperation(
716717
getTopology(this),
717-
new SetProfilingLevelOperation(this, level, options),
718+
new SetProfilingLevelOperation(this, level, resolveOptions(this, options)),
718719
callback
719720
);
720721
}
@@ -734,11 +735,10 @@ export class Db implements OperationParent {
734735
callback?: Callback<string>
735736
): Promise<string> | void {
736737
if (typeof options === 'function') (callback = options), (options = {});
737-
options = options || {};
738738

739739
return executeOperation(
740740
getTopology(this),
741-
new ProfilingLevelOperation(this, options),
741+
new ProfilingLevelOperation(this, resolveOptions(this, options)),
742742
callback
743743
);
744744
}
@@ -764,11 +764,10 @@ export class Db implements OperationParent {
764764
callback?: Callback<Document>
765765
): Promise<Document> | void {
766766
if (typeof options === 'function') (callback = options), (options = {});
767-
options = options || {};
768767

769768
return executeOperation(
770769
getTopology(this),
771-
new IndexInformationOperation(this, name, options),
770+
new IndexInformationOperation(this, name, resolveOptions(this, options)),
772771
callback
773772
);
774773
}
@@ -835,11 +834,10 @@ export class Db implements OperationParent {
835834
callback?: Callback<Document>
836835
): Promise<Document> | void {
837836
if (typeof options === 'function') (callback = options), (options = {});
838-
options = options || {};
839837

840838
return executeOperation(
841839
getTopology(this),
842-
new EvalOperation(this, code, parameters, options),
840+
new EvalOperation(this, code, parameters, resolveOptions(this, options)),
843841
callback
844842
);
845843
}
@@ -873,11 +871,10 @@ export class Db implements OperationParent {
873871
callback?: Callback<Document>
874872
): Promise<Document> | void {
875873
if (typeof options === 'function') (callback = options), (options = {});
876-
options = options || {};
877874

878875
return executeOperation(
879876
getTopology(this),
880-
new EnsureIndexOperation(this, name, fieldOrSpec, options),
877+
new EnsureIndexOperation(this, name, fieldOrSpec, resolveOptions(this, options)),
881878
callback
882879
);
883880
}
@@ -905,18 +902,6 @@ export class Db implements OperationParent {
905902
}
906903
}
907904

908-
const collectionKeys = [
909-
'pkFactory',
910-
'readPreference',
911-
'serializeFunctions',
912-
'strict',
913-
'readConcern',
914-
'ignoreUndefined',
915-
'promoteValues',
916-
'promoteBuffers',
917-
'promoteLongs'
918-
];
919-
920905
Db.prototype.createCollection = deprecateOptions(
921906
{
922907
name: 'Db.createCollection',

‎src/operations/command.ts

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,13 @@ export abstract class CommandOperation<
7373
: new MongoDBNamespace('admin', '$cmd');
7474
}
7575

76-
const propertyProvider = this.hasAspect(Aspect.NO_INHERIT_OPTIONS) ? undefined : parent;
7776
this.readPreference = this.hasAspect(Aspect.WRITE_OPERATION)
7877
? ReadPreference.primary
79-
: ReadPreference.resolve(propertyProvider, this.options);
80-
this.readConcern = resolveReadConcern(propertyProvider, this.options);
81-
this.writeConcern = resolveWriteConcern(propertyProvider, this.options);
78+
: ReadPreference.fromOptions(options) ?? ReadPreference.primary;
79+
this.readConcern = ReadConcern.fromOptions(options);
80+
this.writeConcern = WriteConcern.fromOptions(options);
81+
this.bsonOptions = resolveBSONOptions(options);
82+
8283
this.explain = false;
8384
this.fullResponse =
8485
options && typeof options.fullResponse === 'boolean' ? options.fullResponse : false;
@@ -91,9 +92,6 @@ export abstract class CommandOperation<
9192
if (parent && parent.logger) {
9293
this.logger = parent.logger;
9394
}
94-
95-
// Assign BSON serialize options to OperationBase, preferring options over parent options.
96-
this.bsonOptions = resolveBSONOptions(options, parent);
9795
}
9896

9997
abstract execute(server: Server, callback: Callback<TResult>): void;
@@ -149,11 +147,3 @@ export abstract class CommandOperation<
149147
);
150148
}
151149
}
152-
153-
function resolveWriteConcern(parent: OperationParent | undefined, options: any) {
154-
return WriteConcern.fromOptions(options) || parent?.writeConcern;
155-
}
156-
157-
function resolveReadConcern(parent: OperationParent | undefined, options: any) {
158-
return ReadConcern.fromOptions(options) || parent?.readConcern;
159-
}

‎src/operations/create_collection.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,19 @@ const ILLEGAL_COMMAND_FIELDS = new Set([
1616
'j',
1717
'fsync',
1818
'autoIndexId',
19-
'serializeFunctions',
2019
'pkFactory',
2120
'raw',
2221
'readPreference',
2322
'session',
2423
'readConcern',
25-
'writeConcern'
24+
'writeConcern',
25+
'raw',
26+
'fieldsAsRaw',
27+
'promoteLongs',
28+
'promoteValues',
29+
'promoteBuffers',
30+
'serializeFunctions',
31+
'ignoreUndefined'
2632
]);
2733

2834
/** @public */

0 commit comments

Comments
 (0)