Skip to content

Commit be7f808

Browse files
durranbaileympearson
durran
and
baileympearson
authored
feat(NODE-6472): findOne and find no longer keep open cursors (#4580)
Co-authored-by: Bailey Pearson <[email protected]>
1 parent 44d0187 commit be7f808

File tree

8 files changed

+356
-19
lines changed

8 files changed

+356
-19
lines changed

‎src/collection.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import {
4343
type EstimatedDocumentCountOptions
4444
} from './operations/estimated_document_count';
4545
import { autoConnect, executeOperation } from './operations/execute_operation';
46-
import type { FindOptions } from './operations/find';
46+
import { type FindOneOptions, type FindOptions } from './operations/find';
4747
import {
4848
FindOneAndDeleteOperation,
4949
type FindOneAndDeleteOptions,
@@ -536,25 +536,30 @@ export class Collection<TSchema extends Document = Document> {
536536
async findOne(filter: Filter<TSchema>): Promise<WithId<TSchema> | null>;
537537
async findOne(
538538
filter: Filter<TSchema>,
539-
options: Omit<FindOptions, 'timeoutMode'> & Abortable
539+
options: Omit<FindOneOptions, 'timeoutMode'> & Abortable
540540
): Promise<WithId<TSchema> | null>;
541541

542542
// allow an override of the schema.
543543
async findOne<T = TSchema>(): Promise<T | null>;
544544
async findOne<T = TSchema>(filter: Filter<TSchema>): Promise<T | null>;
545545
async findOne<T = TSchema>(
546546
filter: Filter<TSchema>,
547-
options?: Omit<FindOptions, 'timeoutMode'> & Abortable
547+
options?: Omit<FindOneOptions, 'timeoutMode'> & Abortable
548548
): Promise<T | null>;
549549

550550
async findOne(
551551
filter: Filter<TSchema> = {},
552-
options: FindOptions & Abortable = {}
552+
options: Omit<FindOneOptions, 'timeoutMode'> & Abortable = {}
553553
): Promise<WithId<TSchema> | null> {
554-
const cursor = this.find(filter, options).limit(-1).batchSize(1);
555-
const res = await cursor.next();
554+
// Explicitly set the limit to 1 and singleBatch to true for all commands, per the spec.
555+
// noCursorTimeout must be unset as well as batchSize.
556+
// See: https://github.com/mongodb/specifications/blob/master/source/crud/crud.md#findone-api-details
557+
const { batchSize: _batchSize, noCursorTimeout: _noCursorTimeout, ...opts } = options;
558+
opts.singleBatch = true;
559+
const cursor = this.find(filter, opts).limit(1);
560+
const result = await cursor.next();
556561
await cursor.close();
557-
return res;
562+
return result;
558563
}
559564

560565
/**

‎src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ export type { DeleteOptions, DeleteResult, DeleteStatement } from './operations/
523523
export type { DistinctOptions } from './operations/distinct';
524524
export type { DropCollectionOptions, DropDatabaseOptions } from './operations/drop';
525525
export type { EstimatedDocumentCountOptions } from './operations/estimated_document_count';
526-
export type { FindOptions } from './operations/find';
526+
export type { FindOneOptions, FindOptions } from './operations/find';
527527
export type {
528528
FindOneAndDeleteOptions,
529529
FindOneAndReplaceOptions,

‎src/operations/execute_operation.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,9 @@ async function tryOperation<
248248
if (hasWriteAspect && !isRetryableWriteError(previousOperationError))
249249
throw previousOperationError;
250250

251-
if (hasReadAspect && !isRetryableReadError(previousOperationError))
251+
if (hasReadAspect && !isRetryableReadError(previousOperationError)) {
252252
throw previousOperationError;
253+
}
253254

254255
if (
255256
previousOperationError instanceof MongoNetworkError &&

‎src/operations/find.ts‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,16 @@ export interface FindOptions<TSchema extends Document = Document>
8181
timeoutMode?: CursorTimeoutMode;
8282
}
8383

84+
/** @public */
85+
export interface FindOneOptions extends FindOptions {
86+
/** @deprecated Will be removed in the next major version. User provided value will be ignored. */
87+
batchSize?: number;
88+
/** @deprecated Will be removed in the next major version. User provided value will be ignored. */
89+
limit?: number;
90+
/** @deprecated Will be removed in the next major version. User provided value will be ignored. */
91+
noCursorTimeout?: boolean;
92+
}
93+
8494
/** @internal */
8595
export class FindOperation extends CommandOperation<CursorResponse> {
8696
/**
@@ -185,17 +195,15 @@ function makeFindCommand(ns: MongoDBNamespace, filter: Document, options: FindOp
185195

186196
if (typeof options.batchSize === 'number') {
187197
if (options.batchSize < 0) {
188-
if (
189-
options.limit &&
190-
options.limit !== 0 &&
191-
Math.abs(options.batchSize) < Math.abs(options.limit)
192-
) {
193-
findCommand.limit = -options.batchSize;
194-
}
195-
196-
findCommand.singleBatch = true;
198+
findCommand.limit = -options.batchSize;
197199
} else {
198-
findCommand.batchSize = options.batchSize;
200+
if (options.batchSize === options.limit) {
201+
// Spec dictates that if these are equal the batchSize should be one more than the
202+
// limit to avoid leaving the cursor open.
203+
findCommand.batchSize = options.batchSize + 1;
204+
} else {
205+
findCommand.batchSize = options.batchSize;
206+
}
199207
}
200208
}
201209

‎test/spec/crud/unified/find.json‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,68 @@
237237
]
238238
}
239239
]
240+
},
241+
{
242+
"description": "Find with batchSize equal to limit",
243+
"operations": [
244+
{
245+
"object": "collection0",
246+
"name": "find",
247+
"arguments": {
248+
"filter": {
249+
"_id": {
250+
"$gt": 1
251+
}
252+
},
253+
"sort": {
254+
"_id": 1
255+
},
256+
"limit": 4,
257+
"batchSize": 4
258+
},
259+
"expectResult": [
260+
{
261+
"_id": 2,
262+
"x": 22
263+
},
264+
{
265+
"_id": 3,
266+
"x": 33
267+
},
268+
{
269+
"_id": 4,
270+
"x": 44
271+
},
272+
{
273+
"_id": 5,
274+
"x": 55
275+
}
276+
]
277+
}
278+
],
279+
"expectEvents": [
280+
{
281+
"client": "client0",
282+
"events": [
283+
{
284+
"commandStartedEvent": {
285+
"command": {
286+
"find": "coll0",
287+
"filter": {
288+
"_id": {
289+
"$gt": 1
290+
}
291+
},
292+
"limit": 4,
293+
"batchSize": 5
294+
},
295+
"commandName": "find",
296+
"databaseName": "find-tests"
297+
}
298+
}
299+
]
300+
}
301+
]
240302
}
241303
]
242304
}

‎test/spec/crud/unified/find.yml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,31 @@ tests:
105105
- { _id: 2, x: 22 }
106106
- { _id: 3, x: 33 }
107107
- { _id: 4, x: 44 }
108+
-
109+
description: 'Find with batchSize equal to limit'
110+
operations:
111+
-
112+
object: *collection0
113+
name: find
114+
arguments:
115+
filter: { _id: { $gt: 1 } }
116+
sort: { _id: 1 }
117+
limit: 4
118+
batchSize: 4
119+
expectResult:
120+
- { _id: 2, x: 22 }
121+
- { _id: 3, x: 33 }
122+
- { _id: 4, x: 44 }
123+
- { _id: 5, x: 55 }
124+
expectEvents:
125+
- client: *client0
126+
events:
127+
- commandStartedEvent:
128+
command:
129+
find: *collection0Name
130+
filter: { _id: { $gt: 1 } }
131+
limit: 4
132+
# Drivers use limit + 1 for batchSize to ensure the server closes the cursor
133+
batchSize: 5
134+
commandName: find
135+
databaseName: *database0Name

‎test/spec/crud/unified/findOne.json‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
{
2+
"description": "findOne",
3+
"schemaVersion": "1.0",
4+
"createEntities": [
5+
{
6+
"client": {
7+
"id": "client0",
8+
"observeEvents": [
9+
"commandStartedEvent"
10+
]
11+
}
12+
},
13+
{
14+
"database": {
15+
"id": "database0",
16+
"client": "client0",
17+
"databaseName": "find-tests"
18+
}
19+
},
20+
{
21+
"collection": {
22+
"id": "collection0",
23+
"database": "database0",
24+
"collectionName": "coll0"
25+
}
26+
}
27+
],
28+
"initialData": [
29+
{
30+
"collectionName": "coll0",
31+
"databaseName": "find-tests",
32+
"documents": [
33+
{
34+
"_id": 1,
35+
"x": 11
36+
},
37+
{
38+
"_id": 2,
39+
"x": 22
40+
},
41+
{
42+
"_id": 3,
43+
"x": 33
44+
},
45+
{
46+
"_id": 4,
47+
"x": 44
48+
},
49+
{
50+
"_id": 5,
51+
"x": 55
52+
},
53+
{
54+
"_id": 6,
55+
"x": 66
56+
}
57+
]
58+
}
59+
],
60+
"tests": [
61+
{
62+
"description": "FindOne with filter",
63+
"operations": [
64+
{
65+
"object": "collection0",
66+
"name": "findOne",
67+
"arguments": {
68+
"filter": {
69+
"_id": 1
70+
}
71+
},
72+
"expectResult": {
73+
"_id": 1,
74+
"x": 11
75+
}
76+
}
77+
],
78+
"expectEvents": [
79+
{
80+
"client": "client0",
81+
"events": [
82+
{
83+
"commandStartedEvent": {
84+
"command": {
85+
"find": "coll0",
86+
"filter": {
87+
"_id": 1
88+
},
89+
"batchSize": {
90+
"$$exists": false
91+
},
92+
"limit": 1,
93+
"singleBatch": true
94+
},
95+
"commandName": "find",
96+
"databaseName": "find-tests"
97+
}
98+
}
99+
]
100+
}
101+
]
102+
},
103+
{
104+
"description": "FindOne with filter, sort, and skip",
105+
"operations": [
106+
{
107+
"object": "collection0",
108+
"name": "findOne",
109+
"arguments": {
110+
"filter": {
111+
"_id": {
112+
"$gt": 2
113+
}
114+
},
115+
"sort": {
116+
"_id": 1
117+
},
118+
"skip": 2
119+
},
120+
"expectResult": {
121+
"_id": 5,
122+
"x": 55
123+
}
124+
}
125+
],
126+
"expectEvents": [
127+
{
128+
"client": "client0",
129+
"events": [
130+
{
131+
"commandStartedEvent": {
132+
"command": {
133+
"find": "coll0",
134+
"filter": {
135+
"_id": {
136+
"$gt": 2
137+
}
138+
},
139+
"sort": {
140+
"_id": 1
141+
},
142+
"skip": 2,
143+
"batchSize": {
144+
"$$exists": false
145+
},
146+
"limit": 1,
147+
"singleBatch": true
148+
},
149+
"commandName": "find",
150+
"databaseName": "find-tests"
151+
}
152+
}
153+
]
154+
}
155+
]
156+
}
157+
]
158+
}

0 commit comments

Comments
 (0)