Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-3058): accept null or undefined anywhere we permit nullish values #2921

Merged
merged 4 commits into from Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 18 additions & 3 deletions .eslintrc.json
Expand Up @@ -27,7 +27,7 @@
"tsdoc/syntax": "warn",

"no-console": "error",
"eqeqeq": ["error", "always", { "null": "ignore" }],
"eqeqeq": ["error", "always", {"null": "ignore"}],
"strict": ["error", "global"],

"@typescript-eslint/no-explicit-any": "off",
Expand All @@ -36,15 +36,30 @@
"error",
{
"selector": "TSEnumDeclaration",
"message": "Don't declare enums"
"message": "Do not declare enums"
},
{
"selector": "BinaryExpression[operator=/[=!]==/] Identifier[name='undefined']",
"message": "Do not strictly check undefined"
},
{
"selector": "BinaryExpression[operator=/[=!]==/] Literal[raw='null']",
"message": "Do not strictly check null"
},
{
"selector": "BinaryExpression[operator=/[=!]==?/] Literal[value='undefined']",
"message": "Do not strictly check typeof undefined (NOTE: currently this rule only detects the usage of 'undefined' string literal so this could be a misfire)"
}
]
},
"overrides": [{
"files": ["*.d.ts"],
"rules": {
"prettier/prettier": "off",
"@typescript-eslint/no-empty-interface": "off"
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-misused-new": "off",
"@typescript-eslint/ban-types": "off",
"@typescript-eslint/no-unused-vars": "off"
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion src/cmap/auth/scram.ts
Expand Up @@ -311,7 +311,7 @@ const hiLengthMap = {
function HI(data: string, salt: Buffer, iterations: number, cryptoMethod: CryptoMethod) {
// omit the work if already generated
const key = [data, salt.toString('base64'), iterations].join('_');
if (_hiCache[key] !== undefined) {
if (_hiCache[key] != null) {
return _hiCache[key];
}

Expand Down
8 changes: 4 additions & 4 deletions src/cmap/command_monitoring_events.ts
Expand Up @@ -212,15 +212,15 @@ function extractCommand(command: WriteProtocolMessageType): Document {
// up-convert legacy find command
result = { find: collectionName(command) };
Object.keys(LEGACY_FIND_QUERY_MAP).forEach(key => {
if (typeof command.query[key] !== 'undefined') {
if (command.query[key] != null) {
result[LEGACY_FIND_QUERY_MAP[key]] = deepCopy(command.query[key]);
}
});
}

Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => {
const legacyKey = key as keyof typeof LEGACY_FIND_OPTIONS_MAP;
if (typeof command[legacyKey] !== 'undefined') {
if (command[legacyKey] != null) {
result[LEGACY_FIND_OPTIONS_MAP[legacyKey]] = deepCopy(command[legacyKey]);
}
});
Expand All @@ -232,7 +232,7 @@ function extractCommand(command: WriteProtocolMessageType): Document {
}
});

if (typeof command.pre32Limit !== 'undefined') {
if (command.pre32Limit != null) {
result.limit = command.pre32Limit;
}

Expand Down Expand Up @@ -286,7 +286,7 @@ function extractReply(command: WriteProtocolMessageType, reply?: Document) {
}

// is this a legacy find command?
if (command.query && typeof command.query.$query !== 'undefined') {
if (command.query && command.query.$query != null) {
return {
ok: 1,
cursor: {
Expand Down
6 changes: 3 additions & 3 deletions src/cmap/connection.ts
Expand Up @@ -343,9 +343,9 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
options: CommandOptions | undefined,
callback: Callback
): void {
if (typeof ns.db === 'undefined' || typeof ns === 'string') {
if (!(ns instanceof MongoDBNamespace)) {
// TODO(NODE-3483): Replace this with a MongoCommandError
throw new MongoDriverError('Namespace cannot be a string');
throw new MongoDriverError('Must provide a MongoDBNamespace instance');
}

const readPreference = getReadPreference(cmd, options);
Expand Down Expand Up @@ -453,7 +453,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {

/** @internal */
query(ns: MongoDBNamespace, cmd: Document, options: QueryOptions, callback: Callback): void {
const isExplain = typeof cmd.$explain !== 'undefined';
const isExplain = cmd.$explain != null;
const readPreference = options.readPreference ?? ReadPreference.primary;
const batchSize = options.batchSize || 0;
const limit = options.limit;
Expand Down
2 changes: 1 addition & 1 deletion src/cmap/stream_description.ts
Expand Up @@ -51,7 +51,7 @@ export class StreamDescription {
receiveResponse(response: Document): void {
this.type = parseServerType(response);
RESPONSE_FIELDS.forEach(field => {
if (typeof response[field] !== 'undefined') {
if (typeof response[field] != null) {
this[field] = response[field];
}

Expand Down
4 changes: 2 additions & 2 deletions src/collection.ts
Expand Up @@ -699,7 +699,7 @@ export class Collection<TSchema extends Document = Document> {
options?: FindOptions<TSchema> | Callback<TSchema | undefined>,
callback?: Callback<TSchema>
): Promise<TSchema | undefined> | void {
if (callback !== undefined && typeof callback !== 'function') {
if (callback != null && typeof callback !== 'function') {
throw new MongoInvalidArgumentError(
'Third parameter to `findOne()` must be a callback or undefined'
);
Expand Down Expand Up @@ -1088,7 +1088,7 @@ export class Collection<TSchema extends Document = Document> {
options?: CountDocumentsOptions | Callback<number>,
callback?: Callback<number>
): Promise<number> | void {
if (typeof filter === 'undefined') {
if (filter == null) {
(filter = {}), (options = {}), (callback = undefined);
} else if (typeof filter === 'function') {
(callback = filter as Callback<number>), (filter = {}), (options = {});
Expand Down
10 changes: 5 additions & 5 deletions src/connection_string.ts
Expand Up @@ -177,7 +177,7 @@ function toRecord(value: string): Record<string, any> {
const keyValuePairs = value.split(',');
for (const keyValue of keyValuePairs) {
const [key, value] = keyValue.split(':');
if (typeof value === 'undefined') {
if (value == null) {
throw new MongoParseError('Cannot have undefined values in key value pairs');
}
try {
Expand Down Expand Up @@ -219,7 +219,7 @@ export function parseOptions(
mongoClient: MongoClient | MongoClientOptions | undefined = undefined,
options: MongoClientOptions = {}
): MongoOptions {
if (typeof mongoClient !== 'undefined' && !(mongoClient instanceof MongoClient)) {
if (mongoClient != null && !(mongoClient instanceof MongoClient)) {
options = mongoClient;
mongoClient = undefined;
}
Expand Down Expand Up @@ -284,7 +284,7 @@ export function parseOptions(
}

const objectOptions = new CaseInsensitiveMap(
Object.entries(options).filter(([, v]) => (v ?? null) !== null)
Object.entries(options).filter(([, v]) => v != null)
);

const allOptions = new CaseInsensitiveMap();
Expand Down Expand Up @@ -418,7 +418,7 @@ function setOption(
mongoOptions[name] = getUint(name, values[0]);
break;
case 'string':
if (values[0] === undefined) {
if (values[0] == null) {
break;
}
mongoOptions[name] = String(values[0]);
Expand Down Expand Up @@ -1051,6 +1051,6 @@ export const OPTIONS = {

export const DEFAULT_OPTIONS = new CaseInsensitiveMap(
Object.entries(OPTIONS)
.filter(([, descriptor]) => typeof descriptor.default !== 'undefined')
.filter(([, descriptor]) => descriptor.default != null)
.map(([k, d]) => [k, d.default])
);
10 changes: 4 additions & 6 deletions src/cursor/abstract_cursor.ts
Expand Up @@ -150,7 +150,7 @@ export abstract class AbstractCursor<
this[kOptions].batchSize = options.batchSize;
}

if (typeof options.comment !== 'undefined') {
if (options.comment != null) {
this[kOptions].comment = options.comment;
}

Expand Down Expand Up @@ -225,9 +225,7 @@ export abstract class AbstractCursor<
return {
next: () =>
this.next().then(value =>
value !== null && value !== undefined
? { value, done: false }
: { value: undefined, done: true }
value != null ? { value, done: false } : { value: undefined, done: true }
)
};
}
Expand Down Expand Up @@ -817,7 +815,7 @@ function makeCursorStream<TSchema extends Document>(cursor: AbstractCursor<TSche
function readNext() {
needToClose = false;
next(cursor, true, (err, result) => {
needToClose = err ? !cursor.closed : result !== null;
needToClose = err ? !cursor.closed : result != null;

if (err) {
// NOTE: This is questionable, but we have a test backing the behavior. It seems the
Expand All @@ -839,7 +837,7 @@ function makeCursorStream<TSchema extends Document>(cursor: AbstractCursor<TSche
return readable.destroy(err);
}

if (result === null) {
if (result == null) {
readable.push(null);
} else if (readable.destroyed) {
cursor.close();
Expand Down
2 changes: 1 addition & 1 deletion src/cursor/aggregation_cursor.ts
Expand Up @@ -93,7 +93,7 @@ export class AggregationCursor<TSchema = Document> extends AbstractCursor<TSchem
callback?: Callback<Document>
): Promise<Document> | void {
if (typeof verbosity === 'function') (callback = verbosity), (verbosity = true);
if (verbosity === undefined) verbosity = true;
if (verbosity == null) verbosity = true;

return executeOperation(
this.topology,
Expand Down
4 changes: 2 additions & 2 deletions src/cursor/find_cursor.ts
Expand Up @@ -52,7 +52,7 @@ export class FindCursor<TSchema = Document> extends AbstractCursor<TSchema> {
this[kFilter] = filter || {};
this[kBuiltOptions] = options;

if (typeof options.sort !== 'undefined') {
if (options.sort != null) {
this[kBuiltOptions].sort = formatSort(options.sort);
}
}
Expand Down Expand Up @@ -155,7 +155,7 @@ export class FindCursor<TSchema = Document> extends AbstractCursor<TSchema> {
callback?: Callback<Document>
): Promise<Document> | void {
if (typeof verbosity === 'function') (callback = verbosity), (verbosity = true);
if (verbosity === undefined) verbosity = true;
if (verbosity == null) verbosity = true;

return executeOperation(
this.topology,
Expand Down
2 changes: 1 addition & 1 deletion src/error.ts
Expand Up @@ -898,7 +898,7 @@ export function isResumableError(error?: MongoError, wireVersion?: number): bool
return true;
}

if (typeof wireVersion !== 'undefined' && wireVersion >= 9) {
if (wireVersion != null && wireVersion >= 9) {
// DRIVERS-1308: For 4.4 drivers running against 4.4 servers, drivers will add a special case to treat the CursorNotFound error code as resumable
if (error && error instanceof MongoError && error.code === 43) {
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/explain.ts
Expand Up @@ -40,7 +40,7 @@ export class Explain {
}

static fromOptions(options?: ExplainOptions): Explain | undefined {
if (options?.explain === undefined) return;
if (options?.explain == null) return;

const explain = options.explain;
if (typeof explain === 'boolean' || typeof explain === 'string') {
Expand Down
4 changes: 2 additions & 2 deletions src/operations/command.ts
Expand Up @@ -97,14 +97,14 @@ export abstract class CommandOperation<T> extends AbstractOperation<T> {

if (this.hasAspect(Aspect.EXPLAINABLE)) {
this.explain = Explain.fromOptions(options);
} else if (options?.explain !== undefined) {
} else if (options?.explain != null) {
throw new MongoInvalidArgumentError(`Option "explain" is not supported on this command`);
}
}

get canRetryWrite(): boolean {
if (this.hasAspect(Aspect.EXPLAINABLE)) {
return this.explain === undefined;
return this.explain == null;
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/operations/count.ts
Expand Up @@ -47,7 +47,7 @@ export class CountOperation extends CommandOperation<number> {
cmd.skip = options.skip;
}

if (typeof options.hint !== 'undefined') {
if (options.hint != null) {
cmd.hint = options.hint;
}

Expand Down
4 changes: 2 additions & 2 deletions src/operations/delete.ts
Expand Up @@ -64,7 +64,7 @@ export class DeleteOperation extends CommandOperation<Document> {
return false;
}

return this.statements.every(op => (typeof op.limit !== 'undefined' ? op.limit > 0 : true));
return this.statements.every(op => (op.limit != null ? op.limit > 0 : true));
}

execute(server: Server, session: ClientSession, callback: Callback): void {
Expand All @@ -80,7 +80,7 @@ export class DeleteOperation extends CommandOperation<Document> {
command.let = options.let;
}

if (options.explain !== undefined && maxWireVersion(server) < 3) {
if (options.explain != null && maxWireVersion(server) < 3) {
return callback
? callback(
new MongoCompatibilityError(`Server ${server.name} does not support explain on delete`)
Expand Down
4 changes: 2 additions & 2 deletions src/operations/find.ts
Expand Up @@ -106,7 +106,7 @@ export class FindOperation extends CommandOperation<Document> {

const serverWireVersion = maxWireVersion(server);
const options = this.options;
if (typeof options.allowDiskUse !== 'undefined' && serverWireVersion < 4) {
if (options.allowDiskUse != null && serverWireVersion < 4) {
callback(
new MongoCompatibilityError('Option "allowDiskUse" is not supported on MongoDB < 3.2')
);
Expand Down Expand Up @@ -338,7 +338,7 @@ function makeLegacyFindCommand(
findCommand.$maxTimeMS = options.maxTimeMS;
}

if (typeof options.explain !== 'undefined') {
if (options.explain != null) {
findCommand.$explain = true;
}

Expand Down
2 changes: 1 addition & 1 deletion src/operations/insert.ts
Expand Up @@ -37,7 +37,7 @@ export class InsertOperation extends CommandOperation<Document> {
command.bypassDocumentValidation = options.bypassDocumentValidation;
}

if (typeof options.comment !== 'undefined') {
if (options.comment != null) {
command.comment = options.comment;
}

Expand Down
6 changes: 3 additions & 3 deletions src/operations/update.ts
Expand Up @@ -159,7 +159,7 @@ export class UpdateOneOperation extends UpdateOperation {
): void {
super.execute(server, session, (err, res) => {
if (err || !res) return callback(err);
if (typeof this.explain !== 'undefined') return callback(undefined, res);
if (this.explain != null) return callback(undefined, res);
if (res.code) return callback(new MongoServerError(res));
if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0]));

Expand Down Expand Up @@ -196,7 +196,7 @@ export class UpdateManyOperation extends UpdateOperation {
): void {
super.execute(server, session, (err, res) => {
if (err || !res) return callback(err);
if (typeof this.explain !== 'undefined') return callback(undefined, res);
if (this.explain != null) return callback(undefined, res);
if (res.code) return callback(new MongoServerError(res));
if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0]));

Expand Down Expand Up @@ -250,7 +250,7 @@ export class ReplaceOneOperation extends UpdateOperation {
): void {
super.execute(server, session, (err, res) => {
if (err || !res) return callback(err);
if (typeof this.explain !== 'undefined') return callback(undefined, res);
if (this.explain != null) return callback(undefined, res);
if (res.code) return callback(new MongoServerError(res));
if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0]));

Expand Down
2 changes: 1 addition & 1 deletion src/read_preference.ts
Expand Up @@ -86,7 +86,7 @@ export class ReadPreference {
if (!ReadPreference.isValid(mode)) {
throw new MongoInvalidArgumentError(`Invalid read preference mode ${JSON.stringify(mode)}`);
}
if (options === undefined && typeof tags === 'object' && !Array.isArray(tags)) {
if (options == null && typeof tags === 'object' && !Array.isArray(tags)) {
options = tags;
tags = undefined;
} else if (tags && !Array.isArray(tags)) {
Expand Down
2 changes: 1 addition & 1 deletion src/sdam/topology_description.ts
Expand Up @@ -113,7 +113,7 @@ export class TopologyDescription {
break;
}

if (this.logicalSessionTimeoutMinutes === undefined) {
if (this.logicalSessionTimeoutMinutes == null) {
// First server with a non null logicalSessionsTimeout
this.logicalSessionTimeoutMinutes = server.logicalSessionTimeoutMinutes;
continue;
Expand Down
4 changes: 2 additions & 2 deletions src/sessions.ts
Expand Up @@ -854,7 +854,7 @@ export function applySession(
Object.assign(command.readConcern, { afterClusterTime: session.operationTime });
} else if (session[kSnapshotEnabled]) {
command.readConcern = command.readConcern || { level: ReadConcernLevel.snapshot };
if (session[kSnapshotTime] !== undefined) {
if (session[kSnapshotTime] != null) {
Object.assign(command.readConcern, { atClusterTime: session[kSnapshotTime] });
}
}
Expand Down Expand Up @@ -897,7 +897,7 @@ export function updateSessionFromResponse(session: ClientSession, document: Docu
session.transaction._recoveryToken = document.recoveryToken;
}

if (session?.[kSnapshotEnabled] && session[kSnapshotTime] === undefined) {
if (session?.[kSnapshotEnabled] && session[kSnapshotTime] == null) {
// find and aggregate commands return atClusterTime on the cursor
// distinct includes it in the response body
const atClusterTime = document.cursor?.atClusterTime || document.atClusterTime;
Expand Down