Skip to content

Commit

Permalink
fix(NODE-4609): allow mapping to falsey non-null values in cursors (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
baileympearson authored Oct 24, 2022
1 parent 289300d commit 1bf6ef1
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 8 deletions.
52 changes: 44 additions & 8 deletions src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { promisify } from 'util';
import { BSONSerializeOptions, Document, Long, pluckBSONSerializeOptions } from '../bson';
import {
AnyError,
MongoAPIError,
MongoCursorExhaustedError,
MongoCursorInUseError,
MongoInvalidArgumentError,
Expand Down Expand Up @@ -305,7 +306,18 @@ export abstract class AbstractCursor<
while (true) {
const document = await this.next();

if (document == null) {
// Intentional strict null check, because users can map cursors to falsey values.
// We allow mapping to all values except for null.
// eslint-disable-next-line no-restricted-syntax
if (document === null) {
if (!this.closed) {
const message =
'Cursor returned a `null` document, but the cursor is not exhausted. Mapping documents to `null` is not supported in the cursor transform.';

await cleanupCursorAsync(this, { needsToEmitClosed: true }).catch(() => null);

throw new MongoAPIError(message);
}
break;
}

Expand Down Expand Up @@ -504,6 +516,29 @@ export abstract class AbstractCursor<
* this function's transform.
*
* @remarks
*
* **Note** Cursors use `null` internally to indicate that there are no more documents in the cursor. Providing a mapping
* function that maps values to `null` will result in the cursor closing itself before it has finished iterating
* all documents. This will **not** result in a memory leak, just surprising behavior. For example:
*
* ```typescript
* const cursor = collection.find({});
* cursor.map(() => null);
*
* const documents = await cursor.toArray();
* // documents is always [], regardless of how many documents are in the collection.
* ```
*
* Other falsey values are allowed:
*
* ```typescript
* const cursor = collection.find({});
* cursor.map(() => '');
*
* const documents = await cursor.toArray();
* // documents is now an array of empty strings
* ```
*
* **Note for Typescript Users:** adding a transform changes the return type of the iteration of this cursor,
* it **does not** return a new instance of a cursor. This means when calling map,
* you should always assign the result to a new variable in order to get a correctly typed cursor variable.
Expand Down Expand Up @@ -657,7 +692,7 @@ export abstract class AbstractCursor<
* a significant refactor.
*/
[kInit](callback: Callback<TSchema | null>): void {
this._initialize(this[kSession], (err, state) => {
this._initialize(this[kSession], (error, state) => {
if (state) {
const response = state.response;
this[kServer] = state.server;
Expand Down Expand Up @@ -689,8 +724,12 @@ export abstract class AbstractCursor<
// the cursor is now initialized, even if an error occurred or it is dead
this[kInitialized] = true;

if (err || cursorIsDead(this)) {
return cleanupCursor(this, { error: err }, () => callback(err, nextDocument(this)));
if (error) {
return cleanupCursor(this, { error }, () => callback(error, undefined));
}

if (cursorIsDead(this)) {
return cleanupCursor(this, undefined, () => callback());
}

callback();
Expand Down Expand Up @@ -743,11 +782,8 @@ export function next<T>(

if (cursorId == null) {
// All cursors must operate within a session, one must be made implicitly if not explicitly provided
cursor[kInit]((err, value) => {
cursor[kInit](err => {
if (err) return callback(err);
if (value) {
return callback(undefined, value);
}
return next(cursor, blocking, callback);
});

Expand Down
114 changes: 114 additions & 0 deletions test/integration/node-specific/abstract_cursor.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { expect } from 'chai';
import { inspect } from 'util';

import { Collection, MongoAPIError, MongoClient } from '../../../src';

const falseyValues = [0, 0n, NaN, '', false, undefined];

describe('class AbstractCursor', function () {
let client: MongoClient;

let collection: Collection;
beforeEach(async function () {
client = this.configuration.newClient();

collection = client.db('abstract_cursor_integration').collection('test');

await collection.insertMany(Array.from({ length: 5 }, (_, index) => ({ index })));
});

afterEach(async function () {
await collection.deleteMany({});
await client.close();
});

context('toArray() with custom transforms', function () {
for (const value of falseyValues) {
it(`supports mapping to falsey value '${inspect(value)}'`, async function () {
const cursor = collection.find();
cursor.map(() => value);

const result = await cursor.toArray();

const expected = Array.from({ length: 5 }, () => value);
expect(result).to.deep.equal(expected);
});
}

it('throws when mapping to `null` and cleans up cursor', async function () {
const cursor = collection.find();
cursor.map(() => null);

const error = await cursor.toArray().catch(e => e);

expect(error).be.instanceOf(MongoAPIError);
expect(cursor.closed).to.be.true;
});
});

context('Symbol.asyncIterator() with custom transforms', function () {
for (const value of falseyValues) {
it(`supports mapping to falsey value '${inspect(value)}'`, async function () {
const cursor = collection.find();
cursor.map(() => value);

let count = 0;

for await (const document of cursor) {
expect(document).to.deep.equal(value);
count++;
}

expect(count).to.equal(5);
});
}

it('throws when mapping to `null` and cleans up cursor', async function () {
const cursor = collection.find();
cursor.map(() => null);

try {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
for await (const document of cursor) {
expect.fail('Expected error to be thrown');
}
} catch (error) {
expect(error).to.be.instanceOf(MongoAPIError);
expect(cursor.closed).to.be.true;
}
});
});

context('forEach() with custom transforms', function () {
for (const value of falseyValues) {
it(`supports mapping to falsey value '${inspect(value)}'`, async function () {
const cursor = collection.find();
cursor.map(() => value);

let count = 0;

function transform(value) {
expect(value).to.deep.equal(value);
count++;
}

await cursor.forEach(transform);

expect(count).to.equal(5);
});
}

it('throws when mapping to `null` and cleans up cursor', async function () {
const cursor = collection.find();
cursor.map(() => null);

function iterator() {
expect.fail('Expected no documents from cursor, received at least one.');
}

const error = await cursor.forEach(iterator).catch(e => e);
expect(error).to.be.instanceOf(MongoAPIError);
expect(cursor.closed).to.be.true;
});
});
});

0 comments on commit 1bf6ef1

Please sign in to comment.