Skip to content

feat(NODE-4522)!: remove callback support #3499

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 37 commits into from
Jan 23, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
17e87cf
test(NODE-4919): import mongodb-legacy in tests
nbbeeken Jan 4, 2023
82ac2b9
move test file, update coverage
nbbeeken Jan 19, 2023
4ac5a2f
test(NODE-4919): import mongodb-legacy in tests
nbbeeken Jan 4, 2023
d71a302
feat: remove callbacks from admin.ts
nbbeeken Dec 20, 2022
b1d5971
feat: remove callbacks from bulk/common.ts
nbbeeken Dec 20, 2022
af01cc7
feat: remove callbacks from change_stream.ts
nbbeeken Dec 20, 2022
28d2093
feat: remove callbacks from collection.ts
nbbeeken Dec 20, 2022
16ee978
feat: remove callbacks from cursors
nbbeeken Dec 20, 2022
5623462
feat: remove callbacks from db.ts
nbbeeken Dec 20, 2022
53ad68e
feat: remove callbacks from mongo_client.ts
nbbeeken Dec 20, 2022
51b0649
feat: remove callbacks from sessions.ts
nbbeeken Dec 20, 2022
bfaf3c9
feat: remove callbacks from gridfs
nbbeeken Dec 20, 2022
217101d
async keyword withTransaction and agg.explain
nbbeeken Jan 19, 2023
ffcd2d8
fle: bump to promise first fle commit
nbbeeken Jan 20, 2023
5f43565
fix: withSession
nbbeeken Jan 20, 2023
c3f2769
rm test file
nbbeeken Jan 20, 2023
4d67771
fix: lint
nbbeeken Jan 20, 2023
7a015e5
fix and skip tests
nbbeeken Jan 20, 2023
84f4f9a
jira ticket
nbbeeken Jan 20, 2023
7c06196
fix: await things
nbbeeken Jan 20, 2023
c1d5738
flip messages to check
nbbeeken Jan 20, 2023
266ec54
use fle alpha
nbbeeken Jan 20, 2023
2a1ee77
round one: fight
nbbeeken Jan 20, 2023
c12c401
comments
nbbeeken Jan 21, 2023
fb12143
consistent default arguments
nbbeeken Jan 21, 2023
e0b9538
fix tests, migration
nbbeeken Jan 21, 2023
43a0dbb
strangely close returns undefined
nbbeeken Jan 21, 2023
1ee4f9a
fix test
nbbeeken Jan 21, 2023
66a8942
Merge branch 'main' into NODE-4522-rm-callbacks
nbbeeken Jan 21, 2023
a11d175
argument defaulting fix
nbbeeken Jan 21, 2023
cad2804
fixup test
nbbeeken Jan 21, 2023
5b7dc71
fix async function strangeness
nbbeeken Jan 22, 2023
b489aca
handle close error
nbbeeken Jan 23, 2023
c0b36a9
migration suggestions
nbbeeken Jan 23, 2023
b91886c
rm todos
nbbeeken Jan 23, 2023
b0fda7d
Merge branch 'main' into NODE-4522-rm-callbacks
nbbeeken Jan 23, 2023
5c54370
Merge branch 'main' into NODE-4522-rm-callbacks
nbbeeken Jan 23, 2023
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
6 changes: 3 additions & 3 deletions .evergreen/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2132,7 +2132,7 @@ tasks:
- func: bootstrap kms servers
- func: run custom csfle tests
vars:
CSFLE_GIT_REF: ff9e095eaf72f9e442761f69080dae159a395d94
CSFLE_GIT_REF: 3b3d0ebd2b180f4cb63adf937ecd985fce7e217b
- name: run-custom-csfle-tests-5.0-master
tags:
- run-custom-dependency-tests
Expand Down Expand Up @@ -2162,7 +2162,7 @@ tasks:
- func: bootstrap kms servers
- func: run custom csfle tests
vars:
CSFLE_GIT_REF: ff9e095eaf72f9e442761f69080dae159a395d94
CSFLE_GIT_REF: 3b3d0ebd2b180f4cb63adf937ecd985fce7e217b
- name: run-custom-csfle-tests-rapid-master
tags:
- run-custom-dependency-tests
Expand Down Expand Up @@ -2192,7 +2192,7 @@ tasks:
- func: bootstrap kms servers
- func: run custom csfle tests
vars:
CSFLE_GIT_REF: ff9e095eaf72f9e442761f69080dae159a395d94
CSFLE_GIT_REF: 3b3d0ebd2b180f4cb63adf937ecd985fce7e217b
- name: run-custom-csfle-tests-latest-master
tags:
- run-custom-dependency-tests
Expand Down
2 changes: 1 addition & 1 deletion .evergreen/generate_evergreen_tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ BUILD_VARIANTS.push({
const oneOffFuncAsTasks = []

for (const version of ['5.0', 'rapid', 'latest']) {
for (const ref of ['ff9e095eaf72f9e442761f69080dae159a395d94', 'master']) {
for (const ref of ['3b3d0ebd2b180f4cb63adf937ecd985fce7e217b', 'master']) {
oneOffFuncAsTasks.push({
name: `run-custom-csfle-tests-${version}-${ref === 'master' ? ref : 'pinned-commit'}`,
tags: ['run-custom-dependency-tests'],
Expand Down
215 changes: 32 additions & 183 deletions src/admin.ts

Large diffs are not rendered by default.

75 changes: 27 additions & 48 deletions src/bulk/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
Callback,
getTopology,
hasAtomicOperators,
maybeCallback,
MongoDBNamespace,
resolveOptions
} from '../utils';
Expand Down Expand Up @@ -1175,56 +1174,36 @@ export abstract class BulkOperationBase {
return batches;
}

execute(options?: BulkWriteOptions): Promise<BulkWriteResult>;
/** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
execute(callback: Callback<BulkWriteResult>): void;
/** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
execute(options: BulkWriteOptions | undefined, callback: Callback<BulkWriteResult>): void;
/** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
execute(
options?: BulkWriteOptions | Callback<BulkWriteResult>,
callback?: Callback<BulkWriteResult>
): Promise<BulkWriteResult> | void;
execute(
options?: BulkWriteOptions | Callback<BulkWriteResult>,
callback?: Callback<BulkWriteResult>
): Promise<BulkWriteResult> | void {
callback =
typeof callback === 'function'
? callback
: typeof options === 'function'
? options
: undefined;
return maybeCallback(async () => {
options = options != null && typeof options !== 'function' ? options : {};

if (this.s.executed) {
throw new MongoBatchReExecutionError();
}
async execute(options?: BulkWriteOptions): Promise<BulkWriteResult> {
options ??= {};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it better to use default arg or coalesce assignment 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personal preference is always default arguments? but either is fine. I just prefer not to have to default ourselves - why not let JS do the work for us 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default arg is cleaner imho, but it honestly doesn't matter as long as we're consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made this consistent in the few places it wasn't


const writeConcern = WriteConcern.fromOptions(options);
if (writeConcern) {
this.s.writeConcern = writeConcern;
}
if (this.s.executed) {
throw new MongoBatchReExecutionError();
}

// If we have current batch
if (this.isOrdered) {
if (this.s.currentBatch) this.s.batches.push(this.s.currentBatch);
} else {
if (this.s.currentInsertBatch) this.s.batches.push(this.s.currentInsertBatch);
if (this.s.currentUpdateBatch) this.s.batches.push(this.s.currentUpdateBatch);
if (this.s.currentRemoveBatch) this.s.batches.push(this.s.currentRemoveBatch);
}
// If we have no operations in the bulk raise an error
if (this.s.batches.length === 0) {
throw new MongoInvalidArgumentError('Invalid BulkOperation, Batch cannot be empty');
}
const writeConcern = WriteConcern.fromOptions(options);
if (writeConcern) {
this.s.writeConcern = writeConcern;
}

// If we have current batch
if (this.isOrdered) {
if (this.s.currentBatch) this.s.batches.push(this.s.currentBatch);
} else {
if (this.s.currentInsertBatch) this.s.batches.push(this.s.currentInsertBatch);
if (this.s.currentUpdateBatch) this.s.batches.push(this.s.currentUpdateBatch);
if (this.s.currentRemoveBatch) this.s.batches.push(this.s.currentRemoveBatch);
}
// If we have no operations in the bulk raise an error
if (this.s.batches.length === 0) {
throw new MongoInvalidArgumentError('Invalid BulkOperation, Batch cannot be empty');
}

this.s.executed = true;
const finalOptions = { ...this.s.options, ...options };
const operation = new BulkWriteShimOperation(this, finalOptions);

this.s.executed = true;
const finalOptions = { ...this.s.options, ...options };
const operation = new BulkWriteShimOperation(this, finalOptions);
return executeOperation(this.s.collection.s.db.s.client, operation);
}, callback);
return executeOperation(this.s.collection.s.db.s.client, operation);
}

/**
Expand Down
138 changes: 58 additions & 80 deletions src/change_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type { AggregateOptions } from './operations/aggregate';
import type { CollationOptions, OperationParent } from './operations/command';
import type { ReadPreference } from './read_preference';
import type { ServerSessionId } from './sessions';
import { Callback, filterOptions, getTopology, maybeCallback, MongoDBNamespace } from './utils';
import { filterOptions, getTopology, MongoDBNamespace } from './utils';

/** @internal */
const kCursorStream = Symbol('cursorStream');
Expand Down Expand Up @@ -637,99 +637,84 @@ export class ChangeStream<
}

/** Check if there is any document still available in the Change Stream */
hasNext(): Promise<boolean>;
/** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
hasNext(callback: Callback<boolean>): void;
hasNext(callback?: Callback): Promise<boolean> | void {
async hasNext(): Promise<boolean> {
this._setIsIterator();
return maybeCallback(async () => {
// Change streams must resume indefinitely while each resume event succeeds.
// This loop continues until either a change event is received or until a resume attempt
// fails.
// eslint-disable-next-line no-constant-condition
while (true) {
// Change streams must resume indefinitely while each resume event succeeds.
// This loop continues until either a change event is received or until a resume attempt
// fails.
// eslint-disable-next-line no-constant-condition
while (true) {
try {
const hasNext = await this.cursor.hasNext();
return hasNext;
} catch (error) {
try {
const hasNext = await this.cursor.hasNext();
return hasNext;
await this._processErrorIteratorMode(error);
} catch (error) {
try {
await this._processErrorIteratorMode(error);
} catch (error) {
try {
await this.close();
} catch {
// We are not concerned with errors from close()
}
throw error;
await this.close();
} catch {
// We are not concerned with errors from close()
}
throw error;
}
}
}, callback);
}
}

/** Get the next available document from the Change Stream. */
next(): Promise<TChange>;
/** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
next(callback: Callback<TChange>): void;
next(callback?: Callback<TChange>): Promise<TChange> | void {
async next(): Promise<TChange> {
this._setIsIterator();
return maybeCallback(async () => {
// Change streams must resume indefinitely while each resume event succeeds.
// This loop continues until either a change event is received or until a resume attempt
// fails.
// eslint-disable-next-line no-constant-condition
while (true) {
// Change streams must resume indefinitely while each resume event succeeds.
// This loop continues until either a change event is received or until a resume attempt
// fails.
// eslint-disable-next-line no-constant-condition
while (true) {
try {
const change = await this.cursor.next();
const processedChange = this._processChange(change ?? null);
return processedChange;
} catch (error) {
try {
const change = await this.cursor.next();
const processedChange = this._processChange(change ?? null);
return processedChange;
await this._processErrorIteratorMode(error);
} catch (error) {
try {
await this._processErrorIteratorMode(error);
} catch (error) {
try {
await this.close();
} catch {
// We are not concerned with errors from close()
}
throw error;
await this.close();
} catch {
// We are not concerned with errors from close()
}
throw error;
}
}
}, callback);
}
}

/**
* Try to get the next available document from the Change Stream's cursor or `null` if an empty batch is returned
*/
tryNext(): Promise<Document | null>;
/** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
tryNext(callback: Callback<Document | null>): void;
tryNext(callback?: Callback<Document | null>): Promise<Document | null> | void {
async tryNext(): Promise<Document | null> {
this._setIsIterator();
return maybeCallback(async () => {
// Change streams must resume indefinitely while each resume event succeeds.
// This loop continues until either a change event is received or until a resume attempt
// fails.
// eslint-disable-next-line no-constant-condition
while (true) {
// Change streams must resume indefinitely while each resume event succeeds.
// This loop continues until either a change event is received or until a resume attempt
// fails.
// eslint-disable-next-line no-constant-condition
while (true) {
try {
const change = await this.cursor.tryNext();
return change ?? null;
} catch (error) {
try {
const change = await this.cursor.tryNext();
return change ?? null;
await this._processErrorIteratorMode(error);
} catch (error) {
try {
await this._processErrorIteratorMode(error);
} catch (error) {
try {
await this.close();
} catch {
// We are not concerned with errors from close()
}
throw error;
await this.close();
} catch {
// We are not concerned with errors from close()
}
throw error;
}
}
}, callback);
}
}

async *[Symbol.asyncIterator](): AsyncGenerator<TChange, void, void> {
Expand Down Expand Up @@ -758,20 +743,15 @@ export class ChangeStream<
}

/** Close the Change Stream */
close(): Promise<void>;
/** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
close(callback: Callback): void;
close(callback?: Callback): Promise<void> | void {
async close(): Promise<void> {
this[kClosed] = true;

return maybeCallback(async () => {
const cursor = this.cursor;
try {
await cursor.close();
} finally {
this._endStream();
}
}, callback);
const cursor = this.cursor;
try {
await cursor.close();
} finally {
this._endStream();
}
}

/**
Expand Down Expand Up @@ -864,9 +844,7 @@ export class ChangeStream<
private _closeEmitterModeWithError(error: AnyError): void {
this.emit(ChangeStream.ERROR, error);

this.close(() => {
// nothing to do
});
this.close().catch(() => null);
}

/** @internal */
Expand Down
Loading