diff --git a/src/collection.ts b/src/collection.ts index d1ae1d7fd9..0ad56d04f2 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -43,7 +43,7 @@ import { type EstimatedDocumentCountOptions } from './operations/estimated_document_count'; import { autoConnect, executeOperation } from './operations/execute_operation'; -import type { FindOptions } from './operations/find'; +import { type FindOneOptions, type FindOptions } from './operations/find'; import { FindOneAndDeleteOperation, type FindOneAndDeleteOptions, @@ -536,7 +536,7 @@ export class Collection { async findOne(filter: Filter): Promise | null>; async findOne( filter: Filter, - options: Omit & Abortable + options: Omit & Abortable ): Promise | null>; // allow an override of the schema. @@ -544,17 +544,22 @@ export class Collection { async findOne(filter: Filter): Promise; async findOne( filter: Filter, - options?: Omit & Abortable + options?: Omit & Abortable ): Promise; async findOne( filter: Filter = {}, - options: FindOptions & Abortable = {} + options: Omit & Abortable = {} ): Promise | null> { - const cursor = this.find(filter, options).limit(-1).batchSize(1); - const res = await cursor.next(); + // Explicitly set the limit to 1 and singleBatch to true for all commands, per the spec. + // noCursorTimeout must be unset as well as batchSize. + // See: https://github.com/mongodb/specifications/blob/master/source/crud/crud.md#findone-api-details + const { batchSize: _batchSize, noCursorTimeout: _noCursorTimeout, ...opts } = options; + opts.singleBatch = true; + const cursor = this.find(filter, opts).limit(1); + const result = await cursor.next(); await cursor.close(); - return res; + return result; } /** diff --git a/src/index.ts b/src/index.ts index b87a86042a..5c26ae1b70 100644 --- a/src/index.ts +++ b/src/index.ts @@ -523,7 +523,7 @@ export type { DeleteOptions, DeleteResult, DeleteStatement } from './operations/ export type { DistinctOptions } from './operations/distinct'; export type { DropCollectionOptions, DropDatabaseOptions } from './operations/drop'; export type { EstimatedDocumentCountOptions } from './operations/estimated_document_count'; -export type { FindOptions } from './operations/find'; +export type { FindOneOptions, FindOptions } from './operations/find'; export type { FindOneAndDeleteOptions, FindOneAndReplaceOptions, diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index 454f56daaa..30d57fdb46 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -248,8 +248,9 @@ async function tryOperation< if (hasWriteAspect && !isRetryableWriteError(previousOperationError)) throw previousOperationError; - if (hasReadAspect && !isRetryableReadError(previousOperationError)) + if (hasReadAspect && !isRetryableReadError(previousOperationError)) { throw previousOperationError; + } if ( previousOperationError instanceof MongoNetworkError && diff --git a/src/operations/find.ts b/src/operations/find.ts index 1775ea6e07..9795f583ca 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -81,6 +81,16 @@ export interface FindOptions timeoutMode?: CursorTimeoutMode; } +/** @public */ +export interface FindOneOptions extends FindOptions { + /** @deprecated Will be removed in the next major version. User provided value will be ignored. */ + batchSize?: number; + /** @deprecated Will be removed in the next major version. User provided value will be ignored. */ + limit?: number; + /** @deprecated Will be removed in the next major version. User provided value will be ignored. */ + noCursorTimeout?: boolean; +} + /** @internal */ export class FindOperation extends CommandOperation { /** @@ -185,17 +195,15 @@ function makeFindCommand(ns: MongoDBNamespace, filter: Document, options: FindOp if (typeof options.batchSize === 'number') { if (options.batchSize < 0) { - if ( - options.limit && - options.limit !== 0 && - Math.abs(options.batchSize) < Math.abs(options.limit) - ) { - findCommand.limit = -options.batchSize; - } - - findCommand.singleBatch = true; + findCommand.limit = -options.batchSize; } else { - findCommand.batchSize = options.batchSize; + if (options.batchSize === options.limit) { + // Spec dictates that if these are equal the batchSize should be one more than the + // limit to avoid leaving the cursor open. + findCommand.batchSize = options.batchSize + 1; + } else { + findCommand.batchSize = options.batchSize; + } } } diff --git a/test/spec/crud/unified/find.json b/test/spec/crud/unified/find.json index 6bf1e4e445..325cd96c21 100644 --- a/test/spec/crud/unified/find.json +++ b/test/spec/crud/unified/find.json @@ -237,6 +237,68 @@ ] } ] + }, + { + "description": "Find with batchSize equal to limit", + "operations": [ + { + "object": "collection0", + "name": "find", + "arguments": { + "filter": { + "_id": { + "$gt": 1 + } + }, + "sort": { + "_id": 1 + }, + "limit": 4, + "batchSize": 4 + }, + "expectResult": [ + { + "_id": 2, + "x": 22 + }, + { + "_id": 3, + "x": 33 + }, + { + "_id": 4, + "x": 44 + }, + { + "_id": 5, + "x": 55 + } + ] + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "coll0", + "filter": { + "_id": { + "$gt": 1 + } + }, + "limit": 4, + "batchSize": 5 + }, + "commandName": "find", + "databaseName": "find-tests" + } + } + ] + } + ] } ] } diff --git a/test/spec/crud/unified/find.yml b/test/spec/crud/unified/find.yml index 76676900fa..3a09c4d830 100644 --- a/test/spec/crud/unified/find.yml +++ b/test/spec/crud/unified/find.yml @@ -105,3 +105,31 @@ tests: - { _id: 2, x: 22 } - { _id: 3, x: 33 } - { _id: 4, x: 44 } + - + description: 'Find with batchSize equal to limit' + operations: + - + object: *collection0 + name: find + arguments: + filter: { _id: { $gt: 1 } } + sort: { _id: 1 } + limit: 4 + batchSize: 4 + expectResult: + - { _id: 2, x: 22 } + - { _id: 3, x: 33 } + - { _id: 4, x: 44 } + - { _id: 5, x: 55 } + expectEvents: + - client: *client0 + events: + - commandStartedEvent: + command: + find: *collection0Name + filter: { _id: { $gt: 1 } } + limit: 4 + # Drivers use limit + 1 for batchSize to ensure the server closes the cursor + batchSize: 5 + commandName: find + databaseName: *database0Name diff --git a/test/spec/crud/unified/findOne.json b/test/spec/crud/unified/findOne.json new file mode 100644 index 0000000000..826c0f5dfd --- /dev/null +++ b/test/spec/crud/unified/findOne.json @@ -0,0 +1,158 @@ +{ + "description": "findOne", + "schemaVersion": "1.0", + "createEntities": [ + { + "client": { + "id": "client0", + "observeEvents": [ + "commandStartedEvent" + ] + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "find-tests" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "coll0" + } + } + ], + "initialData": [ + { + "collectionName": "coll0", + "databaseName": "find-tests", + "documents": [ + { + "_id": 1, + "x": 11 + }, + { + "_id": 2, + "x": 22 + }, + { + "_id": 3, + "x": 33 + }, + { + "_id": 4, + "x": 44 + }, + { + "_id": 5, + "x": 55 + }, + { + "_id": 6, + "x": 66 + } + ] + } + ], + "tests": [ + { + "description": "FindOne with filter", + "operations": [ + { + "object": "collection0", + "name": "findOne", + "arguments": { + "filter": { + "_id": 1 + } + }, + "expectResult": { + "_id": 1, + "x": 11 + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "coll0", + "filter": { + "_id": 1 + }, + "batchSize": { + "$$exists": false + }, + "limit": 1, + "singleBatch": true + }, + "commandName": "find", + "databaseName": "find-tests" + } + } + ] + } + ] + }, + { + "description": "FindOne with filter, sort, and skip", + "operations": [ + { + "object": "collection0", + "name": "findOne", + "arguments": { + "filter": { + "_id": { + "$gt": 2 + } + }, + "sort": { + "_id": 1 + }, + "skip": 2 + }, + "expectResult": { + "_id": 5, + "x": 55 + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "coll0", + "filter": { + "_id": { + "$gt": 2 + } + }, + "sort": { + "_id": 1 + }, + "skip": 2, + "batchSize": { + "$$exists": false + }, + "limit": 1, + "singleBatch": true + }, + "commandName": "find", + "databaseName": "find-tests" + } + } + ] + } + ] + } + ] +} diff --git a/test/spec/crud/unified/findOne.yml b/test/spec/crud/unified/findOne.yml new file mode 100644 index 0000000000..ed74124bf3 --- /dev/null +++ b/test/spec/crud/unified/findOne.yml @@ -0,0 +1,75 @@ +description: "findOne" + +schemaVersion: "1.0" + +createEntities: + - client: + id: &client0 client0 + observeEvents: [ commandStartedEvent ] + - database: + id: &database0 database0 + client: *client0 + databaseName: &database0Name find-tests + - collection: + id: &collection0 collection0 + database: *database0 + collectionName: &collection0Name coll0 + +initialData: + - collectionName: *collection0Name + databaseName: *database0Name + documents: + - { _id: 1, x: 11 } + - { _id: 2, x: 22 } + - { _id: 3, x: 33 } + - { _id: 4, x: 44 } + - { _id: 5, x: 55 } + - { _id: 6, x: 66 } + +tests: + - + description: 'FindOne with filter' + operations: + - + object: *collection0 + name: findOne + arguments: + filter: { _id: 1 } + expectResult: { _id: 1, x: 11 } + expectEvents: + - client: *client0 + events: + - commandStartedEvent: + command: + find: *collection0Name + filter: { _id: 1 } + batchSize: { $$exists: false } + limit: 1 + singleBatch: true + commandName: find + databaseName: *database0Name + - + description: 'FindOne with filter, sort, and skip' + operations: + - + object: *collection0 + name: findOne + arguments: + filter: { _id: { $gt: 2 } } + sort: { _id: 1 } + skip: 2 + expectResult: { _id: 5, x: 55 } + expectEvents: + - client: *client0 + events: + - commandStartedEvent: + command: + find: *collection0Name + filter: { _id: { $gt: 2 } } + sort: { _id: 1 } + skip: 2 + batchSize: { $$exists: false } + limit: 1 + singleBatch: true + commandName: find + databaseName: *database0Name