Skip to content

refactor(NODE-4906): remove internal callback usage of public APIs #3495

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

Merged
merged 4 commits into from
Dec 22, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,10 @@ function cleanupCursor(

if (session) {
if (session.owner === cursor) {
return session.endSession({ error }, callback);
session.endSession({ error }).finally(() => {
callback();
});
return;
}

if (!session.inTransaction()) {
Expand All @@ -855,10 +858,11 @@ function cleanupCursor(
function completeCleanup() {
if (session) {
if (session.owner === cursor) {
return session.endSession({ error }, () => {
session.endSession({ error }).finally(() => {
cursor.emit(AbstractCursor.CLOSE);
callback();
});
return;
}

if (!session.inTransaction()) {
Expand All @@ -872,11 +876,13 @@ function cleanupCursor(

cursor[kKilled] = true;

return executeOperation(
executeOperation(
cursor[kClient],
new KillCursorsOperation(cursorId, cursorNs, server, { session }),
completeCleanup
);
new KillCursorsOperation(cursorId, cursorNs, server, { session })
).finally(() => {
completeCleanup();
});
return;
}

/** @internal */
Expand Down
7 changes: 6 additions & 1 deletion src/encrypter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable @typescript-eslint/no-var-requires */

import { deserialize, serialize } from './bson';
import { MONGO_CLIENT_EVENTS } from './constants';
import type { AutoEncrypter, AutoEncryptionOptions } from './deps';
Expand Down Expand Up @@ -114,7 +115,11 @@ export class Encrypter {
this.autoEncrypter.teardown(!!force, e => {
const internalClient = this[kInternalClient];
if (internalClient != null && client !== internalClient) {
return internalClient.close(force, callback);
internalClient.close(force).then(
() => callback(),
error => callback(error)
);
return;
}
callback(e);
});
Expand Down
13 changes: 4 additions & 9 deletions src/operations/bulk_write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,10 @@ export class BulkWriteOperation extends AbstractOperation<BulkWriteResult> {
}

// Execute the bulk
bulk.execute({ ...options, session }, (err, r) => {
// We have connection level error
if (!r && err) {
return callback(err);
}

// Return the results
callback(undefined, r);
});
bulk.execute({ ...options, session }).then(
result => callback(undefined, result),
error => callback(error)
);
}
}

Expand Down
36 changes: 19 additions & 17 deletions src/operations/collections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,26 @@ export class CollectionsOperation extends AbstractOperation<Collection[]> {
session: ClientSession | undefined,
callback: Callback<Collection[]>
): void {
const db = this.db;

// Let's get the collection names
db.listCollections(
{},
{ ...this.options, nameOnly: true, readPreference: this.readPreference, session }
).toArray((err, documents) => {
if (err || !documents) return callback(err);
// Filter collections removing any illegal ones
documents = documents.filter(doc => doc.name.indexOf('$') === -1);

// Return the collection objects
callback(
undefined,
documents.map(d => {
return new Collection(db, d.name, db.s.options);
})
this.db
.listCollections(
{},
{ ...this.options, nameOnly: true, readPreference: this.readPreference, session }
)
.toArray()
.then(
documents => {
const collections = [];
for (const { name } of documents) {
if (!name.includes('$')) {
// Filter collections removing any illegal ones
collections.push(new Collection(this.db, name, this.db.s.options));
}
}
// Return the collection objects
callback(undefined, collections);
},
error => callback(error)
);
});
}
}
15 changes: 9 additions & 6 deletions src/operations/common_functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,15 @@ export function indexInformation(
// Get the list of indexes of the specified collection
db.collection(name)
.listIndexes(options)
.toArray((err, indexes) => {
if (err) return callback(err);
if (!Array.isArray(indexes)) return callback(undefined, []);
if (full) return callback(undefined, indexes);
callback(undefined, processResults(indexes));
});
.toArray()
.then(
indexes => {
if (!Array.isArray(indexes)) return callback(undefined, []);
if (full) return callback(undefined, indexes);
callback(undefined, processResults(indexes));
},
error => callback(error)
);
}

export function prepareDocs(
Expand Down
23 changes: 12 additions & 11 deletions src/operations/indexes.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Document } from '../bson';
import type { Collection } from '../collection';
import type { Db } from '../db';
import { MongoCompatibilityError, MONGODB_ERROR_CODES, MongoServerError } from '../error';
import { MongoCompatibilityError, MONGODB_ERROR_CODES, MongoError } from '../error';
import type { OneOrMore } from '../mongo_types';
import { ReadPreference } from '../read_preference';
import type { Server } from '../sdam/server';
Expand Down Expand Up @@ -320,22 +320,23 @@ export class EnsureIndexOperation extends CreateIndexOperation {
override execute(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((err, indexes) => {
/// ignore "NamespaceNotFound" errors
if (err && (err as MongoServerError).code !== MONGODB_ERROR_CODES.NamespaceNotFound) {
return callback(err);
}

if (indexes) {
cursor.toArray().then(
indexes => {
indexes = Array.isArray(indexes) ? indexes : [indexes];
if (indexes.some(index => index.name === indexName)) {
callback(undefined, indexName);
return;
}
super.execute(server, session, callback);
},
error => {
if (error instanceof MongoError && error.code === MONGODB_ERROR_CODES.NamespaceNotFound) {
// ignore "NamespaceNotFound" errors
return super.execute(server, session, callback);
}
return callback(error);
}

super.execute(server, session, callback);
});
);
}
}

Expand Down
20 changes: 11 additions & 9 deletions src/operations/is_capped.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,17 @@ export class IsCappedOperation extends AbstractOperation<boolean> {
{ name: coll.collectionName },
{ ...this.options, nameOnly: false, readPreference: this.readPreference, session }
)
.toArray((err, collections) => {
if (err || !collections) return callback(err);
if (collections.length === 0) {
// TODO(NODE-3485)
return callback(new MongoAPIError(`collection ${coll.namespace} not found`));
}
.toArray()
.then(
collections => {
if (collections.length === 0) {
// TODO(NODE-3485)
return callback(new MongoAPIError(`collection ${coll.namespace} not found`));
}

const collOptions = collections[0].options;
callback(undefined, !!(collOptions && collOptions.capped));
});
callback(undefined, !!collections[0].options?.capped);
},
error => callback(error)
);
}
}
19 changes: 11 additions & 8 deletions src/operations/options_operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,17 @@ export class OptionsOperation extends AbstractOperation<Document> {
{ name: coll.collectionName },
{ ...this.options, nameOnly: false, readPreference: this.readPreference, session }
)
.toArray((err, collections) => {
if (err || !collections) return callback(err);
if (collections.length === 0) {
// TODO(NODE-3485)
return callback(new MongoAPIError(`collection ${coll.namespace} not found`));
}
.toArray()
.then(
collections => {
if (collections.length === 0) {
// TODO(NODE-3485)
return callback(new MongoAPIError(`collection ${coll.namespace} not found`));
}

callback(err, collections[0].options);
});
callback(undefined, collections[0].options);
},
error => callback(error)
);
}
}