Skip to content

Commit 79a4dc4

Browse files
nbbeekennbbeeken
committed
refactor: 🔥 Remove generic options parameter from OperationBase
Add a builtOptions getter that inherits options from the parent class. NODE-2815
1 parent 1cfe7a6 commit 79a4dc4

File tree

5 files changed

+149
-38
lines changed

5 files changed

+149
-38
lines changed

‎src/operations/command.ts

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Aspect, OperationBase, OperationOptions } from './operation';
22
import { ReadConcern } from '../read_concern';
33
import { WriteConcern, WriteConcernOptions } from '../write_concern';
4-
import { maxWireVersion, MongoDBNamespace, Callback } from '../utils';
4+
import { maxWireVersion, MongoDBNamespace, Callback, deepFreeze } from '../utils';
55
import { ReadPreference, ReadPreferenceLike } from '../read_preference';
66
import { commandSupportsReadConcern } from '../sessions';
77
import { MongoError } from '../error';
@@ -35,9 +35,14 @@ export interface CommandOperationOptions extends OperationOptions, WriteConcernO
3535
noResponse?: boolean;
3636
}
3737

38+
export interface OperationParentPrivate extends BSONSerializeOptions {
39+
options?: BSONSerializeOptions;
40+
namespace: MongoDBNamespace;
41+
}
42+
3843
/** @internal */
3944
export interface OperationParent {
40-
s: { namespace: MongoDBNamespace };
45+
s: OperationParentPrivate;
4146
readConcern?: ReadConcern;
4247
writeConcern?: WriteConcern;
4348
readPreference?: ReadPreference;
@@ -54,10 +59,30 @@ export abstract class CommandOperation<
5459
readPreference: ReadPreference;
5560
readConcern?: ReadConcern;
5661
writeConcern?: WriteConcern;
57-
explain: boolean;
5862
fullResponse?: boolean;
5963
logger?: Logger;
6064

65+
protected collation;
66+
protected maxTimeMS;
67+
protected comment;
68+
protected retryWrites;
69+
protected noResponse;
70+
71+
get builtOptions(): Readonly<CommandOperationOptions> {
72+
return deepFreeze({
73+
...super.builtOptions,
74+
collation: this.collation,
75+
maxTimeMS: this.maxTimeMS,
76+
comment: this.comment,
77+
retryWrites: this.retryWrites,
78+
noResponse: this.noResponse,
79+
fullResponse: this.fullResponse
80+
// Override with proper type
81+
// writeConcern: this.writeConcern,
82+
// readConcern: this.readConcern
83+
});
84+
}
85+
6186
constructor(parent?: OperationParent, options?: T) {
6287
super(options);
6388

@@ -76,17 +101,30 @@ export abstract class CommandOperation<
76101
const propertyProvider = this.hasAspect(Aspect.NO_INHERIT_OPTIONS) ? undefined : parent;
77102
this.readPreference = this.hasAspect(Aspect.WRITE_OPERATION)
78103
? ReadPreference.primary
79-
: ReadPreference.resolve(propertyProvider, this.options);
80-
this.readConcern = resolveReadConcern(propertyProvider, this.options);
81-
this.writeConcern = resolveWriteConcern(propertyProvider, this.options);
104+
: ReadPreference.resolve(propertyProvider, options);
105+
this.readConcern = resolveReadConcern(propertyProvider, options);
106+
this.writeConcern = resolveWriteConcern(propertyProvider, options);
82107
this.explain = false;
83108
this.fullResponse =
84109
options && typeof options.fullResponse === 'boolean' ? options.fullResponse : false;
85110

111+
// if (this.writeConcern && this.writeConcern.w === 0) {
112+
// if (this.session && this.session.explicit) {
113+
// throw new MongoError('Cannot have explicit session with unacknowledged writes');
114+
// }
115+
// return;
116+
// }
117+
86118
// TODO: A lot of our code depends on having the read preference in the options. This should
87119
// go away, but also requires massive test rewrites.
88120
this.options.readPreference = this.readPreference;
89121

122+
this.collation = options?.collation;
123+
this.maxTimeMS = options?.maxTimeMS;
124+
this.comment = options?.comment;
125+
this.retryWrites = options?.retryWrites;
126+
this.noResponse = options?.noResponse;
127+
90128
// TODO(NODE-2056): make logger another "inheritable" property
91129
if (parent && parent.logger) {
92130
this.logger = parent.logger;
@@ -102,15 +140,14 @@ export abstract class CommandOperation<
102140
// TODO: consider making this a non-enumerable property
103141
this.server = server;
104142

105-
const options = { ...this.options, ...this.bsonOptions };
106143
const serverWireVersion = maxWireVersion(server);
107144
const inTransaction = this.session && this.session.inTransaction();
108145

109146
if (this.readConcern && commandSupportsReadConcern(cmd) && !inTransaction) {
110147
Object.assign(cmd, { readConcern: this.readConcern });
111148
}
112149

113-
if (options.collation && serverWireVersion < SUPPORTS_WRITE_CONCERN_AND_COLLATION) {
150+
if (this.builtOptions.collation && serverWireVersion < SUPPORTS_WRITE_CONCERN_AND_COLLATION) {
114151
callback(
115152
new MongoError(
116153
`Server ${server.name}, which reports wire version ${serverWireVersion}, does not support collation`
@@ -124,17 +161,17 @@ export abstract class CommandOperation<
124161
Object.assign(cmd, { writeConcern: this.writeConcern });
125162
}
126163

127-
if (options.collation && typeof options.collation === 'object') {
128-
Object.assign(cmd, { collation: options.collation });
164+
if (this.builtOptions.collation && typeof this.builtOptions.collation === 'object') {
165+
Object.assign(cmd, { collation: this.builtOptions.collation });
129166
}
130167
}
131168

132-
if (typeof options.maxTimeMS === 'number') {
133-
cmd.maxTimeMS = options.maxTimeMS;
169+
if (typeof this.builtOptions.maxTimeMS === 'number') {
170+
cmd.maxTimeMS = this.builtOptions.maxTimeMS;
134171
}
135172

136-
if (typeof options.comment === 'string') {
137-
cmd.comment = options.comment;
173+
if (typeof this.builtOptions.comment === 'string') {
174+
cmd.comment = this.builtOptions.comment;
138175
}
139176

140177
if (this.logger && this.logger.isDebug()) {
@@ -144,7 +181,7 @@ export abstract class CommandOperation<
144181
server.command(
145182
this.ns.toString(),
146183
cmd,
147-
{ fullResult: !!this.fullResponse, ...this.options },
184+
{ fullResult: !!this.fullResponse, ...this.builtOptions },
148185
callback
149186
);
150187
}

‎src/operations/insert.ts

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,54 @@
11
import { MongoError } from '../error';
22
import { defineAspects, Aspect, OperationBase } from './operation';
3-
import { CommandOperation } from './command';
4-
import { applyRetryableWrites, applyWriteConcern, Callback, MongoDBNamespace } from '../utils';
3+
import { CommandOperation, CommandOperationOptions } from './command';
4+
import {
5+
applyRetryableWrites,
6+
applyWriteConcern,
7+
Callback,
8+
deepFreeze,
9+
MongoDBNamespace
10+
} from '../utils';
511
import { prepareDocs } from './common_functions';
612
import type { Server } from '../sdam/server';
713
import type { Collection } from '../collection';
814
import type { WriteCommandOptions } from '../cmap/wire_protocol/write_command';
9-
import type { ObjectId, Document, BSONSerializeOptions } from '../bson';
15+
import type { ObjectId, Document } from '../bson';
1016
import type { Connection } from '../cmap/connection';
1117
import type { BulkWriteOptions } from '../bulk/common';
12-
import type { WriteConcernOptions } from '../write_concern';
1318

1419
/** @internal */
1520
export class InsertOperation extends OperationBase<BulkWriteOptions, Document> {
1621
operations: Document[];
22+
protected bypassDocumentValidation;
23+
protected ordered;
24+
protected forceServerObjectId;
25+
26+
get builtOptions(): BulkWriteOptions {
27+
return deepFreeze({
28+
...super.builtOptions,
29+
bypassDocumentValidation: this.bypassDocumentValidation,
30+
ordered: this.ordered,
31+
forceServerObjectId: this.forceServerObjectId
32+
});
33+
}
1734

1835
constructor(ns: MongoDBNamespace, ops: Document[], options: BulkWriteOptions) {
1936
super(options);
2037
this.ns = ns;
2138
this.operations = ops;
39+
40+
this.bypassDocumentValidation = options.bypassDocumentValidation;
41+
this.ordered = options.ordered ?? !options.keepGoing;
42+
this.forceServerObjectId = options.forceServerObjectId;
2243
}
2344

2445
execute(server: Server, callback: Callback<Document>): void {
25-
server.insert(
26-
this.ns.toString(),
27-
this.operations,
28-
this.options as WriteCommandOptions,
29-
callback
30-
);
46+
server.insert(this.ns.toString(), this.operations, this.builtOptions as any, callback);
3147
}
3248
}
3349

3450
/** @public */
35-
export interface InsertOneOptions extends BSONSerializeOptions, WriteConcernOptions {
51+
export interface InsertOneOptions extends CommandOperationOptions {
3652
/** Allow driver to bypass schema validation in MongoDB 3.2 or higher. */
3753
bypassDocumentValidation?: boolean;
3854
}
@@ -54,26 +70,32 @@ export interface InsertOneResult {
5470
export class InsertOneOperation extends CommandOperation<InsertOneOptions, InsertOneResult> {
5571
collection: Collection;
5672
doc: Document;
73+
protected bypassDocumentValidation;
74+
75+
get builtOptions(): InsertOneOptions {
76+
return deepFreeze({
77+
...super.builtOptions,
78+
bypassDocumentValidation: this.bypassDocumentValidation
79+
});
80+
}
5781

5882
constructor(collection: Collection, doc: Document, options: InsertOneOptions) {
5983
super(collection, options);
6084

6185
this.collection = collection;
6286
this.doc = doc;
87+
this.bypassDocumentValidation = options.bypassDocumentValidation;
6388
}
6489

6590
execute(server: Server, callback: Callback<InsertOneResult>): void {
6691
const coll = this.collection;
6792
const doc = this.doc;
68-
const options = { ...this.options, ...this.bsonOptions };
6993

7094
if (Array.isArray(doc)) {
71-
return callback(
72-
MongoError.create({ message: 'doc parameter must be an object', driver: true })
73-
);
95+
return callback(new MongoError('doc parameter must be an object'));
7496
}
7597

76-
insertDocuments(server, coll, [doc], options, (err, r) => {
98+
insertDocuments(server, coll, [doc], this.builtOptions, (err, r) => {
7799
if (callback == null) return;
78100
if (err && callback) return callback(err);
79101
// Workaround for pre 2.6 servers

‎src/operations/insert_many.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { OperationBase } from './operation';
22
import { BulkWriteOperation } from './bulk_write';
33
import { MongoError } from '../error';
44
import { prepareDocs } from './common_functions';
5-
import type { Callback } from '../utils';
5+
import { Callback, deepFreeze } from '../utils';
66
import type { Collection } from '../collection';
77
import { ObjectId, Document, resolveBSONOptions } from '../bson';
88
import type { BulkWriteResult, BulkWriteOptions } from '../bulk/common';
@@ -25,6 +25,19 @@ export class InsertManyOperation extends OperationBase<BulkWriteOptions, InsertM
2525
collection: Collection;
2626
docs: Document[];
2727

28+
private bypassDocumentValidation;
29+
private ordered;
30+
private forceServerObjectId;
31+
32+
get builtOptions(): BulkWriteOptions {
33+
return deepFreeze({
34+
...super.builtOptions,
35+
bypassDocumentValidation: this.bypassDocumentValidation,
36+
ordered: this.ordered,
37+
forceServerObjectId: this.forceServerObjectId
38+
});
39+
}
40+
2841
constructor(collection: Collection, docs: Document[], options: BulkWriteOptions) {
2942
super(options);
3043

@@ -33,20 +46,26 @@ export class InsertManyOperation extends OperationBase<BulkWriteOptions, InsertM
3346

3447
// Assign BSON serialize options to OperationBase, preferring options over collection options
3548
this.bsonOptions = resolveBSONOptions(options, collection);
49+
this.bypassDocumentValidation = options.bypassDocumentValidation;
50+
this.ordered = options.ordered ?? !options.keepGoing;
51+
this.forceServerObjectId = options.forceServerObjectId;
3652
}
3753

3854
execute(server: Server, callback: Callback<InsertManyResult>): void {
3955
const coll = this.collection;
4056
let docs = this.docs;
41-
const options = { ...this.options, ...this.bsonOptions };
4257

4358
if (!Array.isArray(docs)) {
44-
return callback(
45-
MongoError.create({ message: 'docs parameter must be an array of documents', driver: true })
46-
);
59+
return callback(new MongoError('docs parameter must be an array of documents'));
4760
}
4861

62+
const options = {
63+
...this.builtOptions,
64+
serializeFunctions: this.builtOptions.serializeFunctions || coll.s.bsonOptions.serializeFunctions
65+
};
66+
4967
docs = prepareDocs(coll, docs, options);
68+
// If keep going set unordered
5069

5170
// Generate the bulk write operations
5271
const operations = [{ insertMany: docs }];

‎src/operations/operation.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { ReadPreference } from '../read_preference';
22
import type { ClientSession } from '../sessions';
33
import type { Document, BSONSerializeOptions } from '../bson';
4-
import type { MongoDBNamespace, Callback } from '../utils';
4+
import { MongoDBNamespace, Callback, deepFreeze } from '../utils';
55
import type { Server } from '../sdam/server';
66

77
export const Aspect = {
@@ -22,7 +22,6 @@ export interface OperationConstructor extends Function {
2222
export interface OperationOptions extends BSONSerializeOptions {
2323
/** Specify ClientSession for this command */
2424
session?: ClientSession;
25-
2625
explain?: boolean;
2726
willRetryWrites?: boolean;
2827
}
@@ -44,12 +43,26 @@ export abstract class OperationBase<
4443
readPreference: ReadPreference;
4544
server!: Server;
4645
fullResponse?: boolean;
46+
protected explain;
47+
protected willRetryWrites;
4748

4849
// BSON serialization options
4950
bsonOptions?: BSONSerializeOptions;
5051

52+
get builtOptions(): Readonly<OperationOptions> {
53+
return deepFreeze({
54+
...this.options, // Should go away when options aren't generic params anymore.
55+
...this.bsonOptions,
56+
session: this.session,
57+
explain: this.explain,
58+
willRetryWrites: this.willRetryWrites
59+
});
60+
}
61+
5162
constructor(options: T = {} as T) {
5263
this.options = Object.assign({}, options);
64+
this.explain = options.explain;
65+
this.willRetryWrites = options.willRetryWrites;
5366
this.readPreference = ReadPreference.primary;
5467
}
5568

‎src/utils.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,3 +1108,23 @@ export function hasAtomicOperators(doc: Document | Document[]): boolean {
11081108
const keys = Object.keys(doc);
11091109
return keys.length > 0 && keys[0][0] === '$';
11101110
}
1111+
1112+
export function deepFreeze<O extends Record<string, any>>(object: O): Readonly<O> {
1113+
// Retrieve the property names defined on object
1114+
const propNames = Object.getOwnPropertyNames(object);
1115+
1116+
// Freeze properties before freezing self
1117+
1118+
for (const name of propNames) {
1119+
const value = object[name];
1120+
1121+
if (name === 'session') continue;
1122+
if (ArrayBuffer.isView(value)) continue;
1123+
1124+
if (value && typeof value === 'object' && !Object.isFrozen(value)) {
1125+
deepFreeze(value);
1126+
}
1127+
}
1128+
1129+
return Object.freeze(object);
1130+
}

0 commit comments

Comments
 (0)