From 6390cdd29b314ea761fb0e69b97f96e6f3192084 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 9 Jul 2025 14:05:10 -0400 Subject: [PATCH 01/15] feat(NODE-6472): run findOne without cursor --- src/collection.ts | 25 +++-- src/operations/find.ts | 24 +++- src/operations/find_one_operation.ts | 75 +++++++++++++ test/spec/crud/unified/find.json | 62 +++++++++++ test/spec/crud/unified/find.yml | 28 +++++ test/spec/crud/unified/findOne.json | 158 +++++++++++++++++++++++++++ test/spec/crud/unified/findOne.yml | 75 +++++++++++++ 7 files changed, 437 insertions(+), 10 deletions(-) create mode 100644 src/operations/find_one_operation.ts create mode 100644 test/spec/crud/unified/findOne.json create mode 100644 test/spec/crud/unified/findOne.yml diff --git a/src/collection.ts b/src/collection.ts index d1ae1d7fd92..d06831b898b 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, @@ -52,6 +52,7 @@ import { FindOneAndUpdateOperation, type FindOneAndUpdateOptions } from './operations/find_and_modify'; +import { FindOneOperation } from './operations/find_one_operation'; import { CreateIndexesOperation, type CreateIndexesOptions, @@ -536,7 +537,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 +545,25 @@ 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(); - await cursor.close(); - return res; + //const cursor = this.find(filter, options).limit(-1).batchSize(1); + //const res = await cursor.next(); + //await cursor.close(); + return await executeOperation( + this.client, + new FindOneOperation( + this.s.db, + this.collectionName, + filter, + resolveOptions(this as TODO_NODE_3286, options) + ) + ); } /** diff --git a/src/operations/find.ts b/src/operations/find.ts index 1775ea6e07f..81590ade170 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 { /** @@ -142,7 +152,11 @@ export class FindOperation extends CommandOperation { } } -function makeFindCommand(ns: MongoDBNamespace, filter: Document, options: FindOptions): Document { +export function makeFindCommand( + ns: MongoDBNamespace, + filter: Document, + options: FindOptions +): Document { const findCommand: Document = { find: ns.collection, filter @@ -195,7 +209,13 @@ function makeFindCommand(ns: MongoDBNamespace, filter: Document, options: FindOp findCommand.singleBatch = true; } 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/src/operations/find_one_operation.ts b/src/operations/find_one_operation.ts new file mode 100644 index 00000000000..15b924b2330 --- /dev/null +++ b/src/operations/find_one_operation.ts @@ -0,0 +1,75 @@ +import { type Db } from '..'; +import { type Document, pluckBSONSerializeOptions } from '../bson'; +import { type OnDemandDocumentDeserializeOptions } from '../cmap/wire_protocol/on_demand/document'; +import type { Server } from '../sdam/server'; +import type { ClientSession } from '../sessions'; +import { type TimeoutContext } from '../timeout'; +import { MongoDBNamespace } from '../utils'; +import { CommandOperation } from './command'; +import { type FindOptions, makeFindCommand } from './find'; +import { Aspect, defineAspects } from './operation'; + +/** @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 FindOneOperation extends CommandOperation { + override options: FindOneOptions; + /** @internal */ + private namespace: MongoDBNamespace; + /** @internal */ + private filter: Document; + /** @internal */ + protected deserializationOptions: OnDemandDocumentDeserializeOptions; + + constructor(db: Db, collectionName: string, filter: Document, options: FindOneOptions = {}) { + super(db, options); + this.namespace = new MongoDBNamespace(db.databaseName, collectionName); + this.filter = filter; + this.options = { ...options }; + this.deserializationOptions = { + ...pluckBSONSerializeOptions(options), + validation: { + utf8: options?.enableUtf8Validation === false ? false : true + } + }; + } + + override get commandName() { + return 'find' as const; + } + + override async execute( + server: Server, + session: ClientSession | undefined, + timeoutContext: TimeoutContext + ): Promise { + const command: Document = makeFindCommand(this.namespace, this.filter, this.options); + // 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 + command.limit = 1; + command.singleBatch = true; + if (command.noCursorTimeout != null) { + delete command.noCursorTimeout; + } + if (command.batchSize != null) { + delete command.batchSize; + } + + const response = await super.executeCommand(server, session, command, timeoutContext); + // In this case since we are just running a command, the response is a document with + // a single batch cursor, not an OnDemandDocument. + const document = response.cursor?.firstBatch?.[0] ?? null; + return document; + } +} + +defineAspects(FindOneOperation, [Aspect.READ_OPERATION, Aspect.RETRYABLE]); diff --git a/test/spec/crud/unified/find.json b/test/spec/crud/unified/find.json index 6bf1e4e4453..325cd96c218 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 76676900fad..3a09c4d830f 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 00000000000..826c0f5dfdb --- /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 00000000000..ed74124bf3c --- /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 From bc490fe544c4e72dcb7922944fb7fdfbc17ee7db Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 9 Jul 2025 15:06:00 -0400 Subject: [PATCH 02/15] test: fix crud and explain tests --- src/operations/find_one_operation.ts | 7 ++-- test/integration/crud/crud_api.test.ts | 44 -------------------------- 2 files changed, 4 insertions(+), 47 deletions(-) diff --git a/src/operations/find_one_operation.ts b/src/operations/find_one_operation.ts index 15b924b2330..33cc3588c34 100644 --- a/src/operations/find_one_operation.ts +++ b/src/operations/find_one_operation.ts @@ -66,10 +66,11 @@ export class FindOneOperation extends CommandOperation { const response = await super.executeCommand(server, session, command, timeoutContext); // In this case since we are just running a command, the response is a document with - // a single batch cursor, not an OnDemandDocument. - const document = response.cursor?.firstBatch?.[0] ?? null; + // a single batch cursor, not an OnDemandDocument. If we are explaining, we just + // return the response as is. + const document = this.explain ? response : (response.cursor?.firstBatch?.[0] ?? null); return document; } } -defineAspects(FindOneOperation, [Aspect.READ_OPERATION, Aspect.RETRYABLE]); +defineAspects(FindOneOperation, [Aspect.READ_OPERATION, Aspect.RETRYABLE, Aspect.EXPLAINABLE]); diff --git a/test/integration/crud/crud_api.test.ts b/test/integration/crud/crud_api.test.ts index eceb1ce60fc..6b8bcfbadcb 100644 --- a/test/integration/crud/crud_api.test.ts +++ b/test/integration/crud/crud_api.test.ts @@ -98,50 +98,6 @@ describe('CRUD API', function () { await collection.drop().catch(() => null); await client.close(); }); - - describe('when the operation succeeds', () => { - it('the cursor for findOne is closed', async function () { - const spy = sinon.spy(Collection.prototype, 'find'); - const result = await collection.findOne({}); - expect(result).to.deep.equal({ _id: 1 }); - expect(events.at(0)).to.be.instanceOf(CommandSucceededEvent); - expect(spy.returnValues.at(0)).to.have.property('closed', true); - expect(spy.returnValues.at(0)).to.have.nested.property('session.hasEnded', true); - }); - }); - - describe('when the find operation fails', () => { - beforeEach(async function () { - const failPoint: FailPoint = { - configureFailPoint: 'failCommand', - mode: 'alwaysOn', - data: { - failCommands: ['find'], - // 1 == InternalError, but this value not important to the test - errorCode: 1 - } - }; - await client.db().admin().command(failPoint); - }); - - afterEach(async function () { - const failPoint: FailPoint = { - configureFailPoint: 'failCommand', - mode: 'off', - data: { failCommands: ['find'] } - }; - await client.db().admin().command(failPoint); - }); - - it('the cursor for findOne is closed', async function () { - const spy = sinon.spy(Collection.prototype, 'find'); - const error = await collection.findOne({}).catch(error => error); - expect(error).to.be.instanceOf(MongoServerError); - expect(events.at(0)).to.be.instanceOf(CommandFailedEvent); - expect(spy.returnValues.at(0)).to.have.property('closed', true); - expect(spy.returnValues.at(0)).to.have.nested.property('session.hasEnded', true); - }); - }); }); describe('countDocuments()', () => { From a0c7a84b991d3889a6d7c6c82be4c35238c64aa7 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 9 Jul 2025 16:00:48 -0400 Subject: [PATCH 03/15] fix: raw bson option --- src/cmap/wire_protocol/responses.ts | 6 ++++++ src/operations/find_one_operation.ts | 26 ++++++++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/cmap/wire_protocol/responses.ts b/src/cmap/wire_protocol/responses.ts index d5549aea545..114c37be9cc 100644 --- a/src/cmap/wire_protocol/responses.ts +++ b/src/cmap/wire_protocol/responses.ts @@ -391,3 +391,9 @@ export class ClientBulkWriteCursorResponse extends CursorResponse { return this.get('writeConcernError', BSONType.object, false); } } + +export class ExplainResponse extends MongoDBResponse { + get queryPlanner() { + return this.get('queryPlanner', BSONType.object, false); + } +} diff --git a/src/operations/find_one_operation.ts b/src/operations/find_one_operation.ts index 33cc3588c34..47794aa7f03 100644 --- a/src/operations/find_one_operation.ts +++ b/src/operations/find_one_operation.ts @@ -1,6 +1,7 @@ import { type Db } from '..'; import { type Document, pluckBSONSerializeOptions } from '../bson'; import { type OnDemandDocumentDeserializeOptions } from '../cmap/wire_protocol/on_demand/document'; +import { CursorResponse, ExplainResponse } from '../cmap/wire_protocol/responses'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; import { type TimeoutContext } from '../timeout'; @@ -64,12 +65,25 @@ export class FindOneOperation extends CommandOperation { delete command.batchSize; } - const response = await super.executeCommand(server, session, command, timeoutContext); - // In this case since we are just running a command, the response is a document with - // a single batch cursor, not an OnDemandDocument. If we are explaining, we just - // return the response as is. - const document = this.explain ? response : (response.cursor?.firstBatch?.[0] ?? null); - return document; + if (this.explain) { + const response = await super.executeCommand( + server, + session, + command, + timeoutContext, + ExplainResponse + ); + return response.toObject() as TSchema; + } else { + const response = await super.executeCommand( + server, + session, + command, + timeoutContext, + CursorResponse + ); + return response.shift(this.deserializationOptions); + } } } From b23264ddafa32a342b5219aae69717897ca31a8f Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 9 Jul 2025 18:15:56 -0400 Subject: [PATCH 04/15] fix: handle oid filter case --- src/operations/find_one_operation.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/operations/find_one_operation.ts b/src/operations/find_one_operation.ts index 47794aa7f03..6ca55266b5f 100644 --- a/src/operations/find_one_operation.ts +++ b/src/operations/find_one_operation.ts @@ -33,7 +33,8 @@ export class FindOneOperation extends CommandOperation { constructor(db: Db, collectionName: string, filter: Document, options: FindOneOptions = {}) { super(db, options); this.namespace = new MongoDBNamespace(db.databaseName, collectionName); - this.filter = filter; + // special case passing in an ObjectId as a filter + this.filter = filter != null && filter._bsontype === 'ObjectId' ? { _id: filter } : filter; this.options = { ...options }; this.deserializationOptions = { ...pluckBSONSerializeOptions(options), From 1592f527a6f014aa991f0caa557c315ebe39f80c Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 9 Jul 2025 18:21:10 -0400 Subject: [PATCH 05/15] fix: lint --- src/collection.ts | 1 - src/index.ts | 1 + src/operations/find.ts | 10 ---------- src/operations/{find_one_operation.ts => find_one.ts} | 0 4 files changed, 1 insertion(+), 11 deletions(-) rename src/operations/{find_one_operation.ts => find_one.ts} (100%) diff --git a/src/collection.ts b/src/collection.ts index d06831b898b..8b5174218fe 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -52,7 +52,6 @@ import { FindOneAndUpdateOperation, type FindOneAndUpdateOptions } from './operations/find_and_modify'; -import { FindOneOperation } from './operations/find_one_operation'; import { CreateIndexesOperation, type CreateIndexesOptions, diff --git a/src/index.ts b/src/index.ts index b87a86042a2..2540477a11e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -529,6 +529,7 @@ export type { FindOneAndReplaceOptions, FindOneAndUpdateOptions } from './operations/find_and_modify'; +export type { FindOneOptions } from './operations/find_one'; export type { IndexInformationOptions } from './operations/indexes'; export type { CreateIndexesOptions, diff --git a/src/operations/find.ts b/src/operations/find.ts index 81590ade170..9dc967dda73 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -81,16 +81,6 @@ 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 { /** diff --git a/src/operations/find_one_operation.ts b/src/operations/find_one.ts similarity index 100% rename from src/operations/find_one_operation.ts rename to src/operations/find_one.ts From c2b6e4eb6489009ed194160226bde198d99dedd9 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Mon, 21 Jul 2025 23:52:03 +0200 Subject: [PATCH 06/15] chore: debug retryability --- src/collection.ts | 30 ++++++++++++------- src/operations/execute_operation.ts | 7 ++++- .../retryable_reads.spec.test.ts | 2 +- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 8b5174218fe..832a773d5fa 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -551,18 +551,26 @@ export class Collection { filter: Filter = {}, options: Omit & Abortable = {} ): Promise | null> { - //const cursor = this.find(filter, options).limit(-1).batchSize(1); - //const res = await cursor.next(); + const opts = { ...options }; + opts.singleBatch = true; + if (opts.noCursorTimeout != null) { + delete opts.noCursorTimeout; + } + if (opts.batchSize != null) { + delete opts.batchSize; + } + const cursor = this.find(filter, opts).limit(1); + return await cursor.next(); //await cursor.close(); - return await executeOperation( - this.client, - new FindOneOperation( - this.s.db, - this.collectionName, - filter, - resolveOptions(this as TODO_NODE_3286, options) - ) - ); + //return await executeOperation( + // this.client, + // new FindOneOperation( + // this.s.db, + // this.collectionName, + // filter, + // resolveOptions(this as TODO_NODE_3286, options) + // ) + //); } /** diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index 454f56daaa9..c5cf7ccd4f5 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -232,6 +232,7 @@ async function tryOperation< let previousServer: ServerDescription | undefined; for (let tries = 0; tries < maxTries; tries++) { + console.log('trying', tries, maxTries, willRetry, operation.commandName); if (previousOperationError) { if (hasWriteAspect && previousOperationError.code === MMAPv1_RETRY_WRITES_ERROR_CODE) { throw new MongoServerError({ @@ -248,8 +249,10 @@ async function tryOperation< if (hasWriteAspect && !isRetryableWriteError(previousOperationError)) throw previousOperationError; - if (hasReadAspect && !isRetryableReadError(previousOperationError)) + if (hasReadAspect && !isRetryableReadError(previousOperationError)) { + console.log(previousOperationError, isRetryableReadError(previousOperationError)); throw previousOperationError; + } if ( previousOperationError instanceof MongoNetworkError && @@ -280,8 +283,10 @@ async function tryOperation< if (tries > 0 && operation.hasAspect(Aspect.COMMAND_BATCHING)) { operation.resetBatch(); } + console.log('executing', operation.commandName); return await operation.execute(server, session, timeoutContext); } catch (operationError) { + console.log('operationError', operationError); if (!(operationError instanceof MongoError)) throw operationError; if ( previousOperationError != null && diff --git a/test/integration/retryable-reads/retryable_reads.spec.test.ts b/test/integration/retryable-reads/retryable_reads.spec.test.ts index 19657f01e6d..59ff7e90a2e 100644 --- a/test/integration/retryable-reads/retryable_reads.spec.test.ts +++ b/test/integration/retryable-reads/retryable_reads.spec.test.ts @@ -14,7 +14,7 @@ const UNIMPLEMENTED_APIS = [ const skippedTests = ['collection.listIndexes succeeds after retryable handshake network error']; -describe('Retryable Reads (unified)', function () { +describe.only('Retryable Reads (unified)', function () { runUnifiedSuite(loadSpecTests(path.join('retryable-reads', 'unified')), ({ description }) => { for (const apiName of UNIMPLEMENTED_APIS) { if (description.toLowerCase().includes(apiName.toLowerCase())) { From 92991f4f50bd7d9fea4ce85c6daa99067d3d4db0 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 22 Jul 2025 20:12:47 +0200 Subject: [PATCH 07/15] chore: try with existing cursor logic --- src/collection.ts | 13 +++---------- .../retryable-reads/retryable_reads.spec.test.ts | 2 +- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 832a773d5fa..bc4a7c34fa1 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -552,6 +552,9 @@ export class Collection { options: Omit & Abortable = {} ): Promise | null> { const opts = { ...options }; + // 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 opts.singleBatch = true; if (opts.noCursorTimeout != null) { delete opts.noCursorTimeout; @@ -561,16 +564,6 @@ export class Collection { } const cursor = this.find(filter, opts).limit(1); return await cursor.next(); - //await cursor.close(); - //return await executeOperation( - // this.client, - // new FindOneOperation( - // this.s.db, - // this.collectionName, - // filter, - // resolveOptions(this as TODO_NODE_3286, options) - // ) - //); } /** diff --git a/test/integration/retryable-reads/retryable_reads.spec.test.ts b/test/integration/retryable-reads/retryable_reads.spec.test.ts index 59ff7e90a2e..19657f01e6d 100644 --- a/test/integration/retryable-reads/retryable_reads.spec.test.ts +++ b/test/integration/retryable-reads/retryable_reads.spec.test.ts @@ -14,7 +14,7 @@ const UNIMPLEMENTED_APIS = [ const skippedTests = ['collection.listIndexes succeeds after retryable handshake network error']; -describe.only('Retryable Reads (unified)', function () { +describe('Retryable Reads (unified)', function () { runUnifiedSuite(loadSpecTests(path.join('retryable-reads', 'unified')), ({ description }) => { for (const apiName of UNIMPLEMENTED_APIS) { if (description.toLowerCase().includes(apiName.toLowerCase())) { From 7c20c857aa3c841f103ca9b798265afd2e90c086 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 22 Jul 2025 20:41:09 +0200 Subject: [PATCH 08/15] fix: lint --- src/index.ts | 3 +-- src/operations/execute_operation.ts | 4 ---- src/operations/find.ts | 10 ++++++++++ src/operations/find_one.ts | 12 +----------- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/index.ts b/src/index.ts index 2540477a11e..5c26ae1b706 100644 --- a/src/index.ts +++ b/src/index.ts @@ -523,13 +523,12 @@ 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, FindOneAndUpdateOptions } from './operations/find_and_modify'; -export type { FindOneOptions } from './operations/find_one'; export type { IndexInformationOptions } from './operations/indexes'; export type { CreateIndexesOptions, diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index c5cf7ccd4f5..30d57fdb462 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -232,7 +232,6 @@ async function tryOperation< let previousServer: ServerDescription | undefined; for (let tries = 0; tries < maxTries; tries++) { - console.log('trying', tries, maxTries, willRetry, operation.commandName); if (previousOperationError) { if (hasWriteAspect && previousOperationError.code === MMAPv1_RETRY_WRITES_ERROR_CODE) { throw new MongoServerError({ @@ -250,7 +249,6 @@ async function tryOperation< throw previousOperationError; if (hasReadAspect && !isRetryableReadError(previousOperationError)) { - console.log(previousOperationError, isRetryableReadError(previousOperationError)); throw previousOperationError; } @@ -283,10 +281,8 @@ async function tryOperation< if (tries > 0 && operation.hasAspect(Aspect.COMMAND_BATCHING)) { operation.resetBatch(); } - console.log('executing', operation.commandName); return await operation.execute(server, session, timeoutContext); } catch (operationError) { - console.log('operationError', operationError); if (!(operationError instanceof MongoError)) throw operationError; if ( previousOperationError != null && diff --git a/src/operations/find.ts b/src/operations/find.ts index 9dc967dda73..81590ade170 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 { /** diff --git a/src/operations/find_one.ts b/src/operations/find_one.ts index 6ca55266b5f..4631f747b50 100644 --- a/src/operations/find_one.ts +++ b/src/operations/find_one.ts @@ -7,19 +7,9 @@ import type { ClientSession } from '../sessions'; import { type TimeoutContext } from '../timeout'; import { MongoDBNamespace } from '../utils'; import { CommandOperation } from './command'; -import { type FindOptions, makeFindCommand } from './find'; +import { type FindOneOptions, makeFindCommand } from './find'; import { Aspect, defineAspects } from './operation'; -/** @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 FindOneOperation extends CommandOperation { override options: FindOneOptions; From c7f5f98897617903ff2a47a628fe7c34f0e77ac6 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 22 Jul 2025 21:14:40 +0200 Subject: [PATCH 09/15] chore: remove dead code --- src/cmap/wire_protocol/responses.ts | 6 -- src/operations/find.ts | 6 +- src/operations/find_one.ts | 81 -------------------------- test/integration/crud/crud_api.test.ts | 44 ++++++++++++++ 4 files changed, 45 insertions(+), 92 deletions(-) delete mode 100644 src/operations/find_one.ts diff --git a/src/cmap/wire_protocol/responses.ts b/src/cmap/wire_protocol/responses.ts index 114c37be9cc..d5549aea545 100644 --- a/src/cmap/wire_protocol/responses.ts +++ b/src/cmap/wire_protocol/responses.ts @@ -391,9 +391,3 @@ export class ClientBulkWriteCursorResponse extends CursorResponse { return this.get('writeConcernError', BSONType.object, false); } } - -export class ExplainResponse extends MongoDBResponse { - get queryPlanner() { - return this.get('queryPlanner', BSONType.object, false); - } -} diff --git a/src/operations/find.ts b/src/operations/find.ts index 81590ade170..00987a03d16 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -152,11 +152,7 @@ export class FindOperation extends CommandOperation { } } -export function makeFindCommand( - ns: MongoDBNamespace, - filter: Document, - options: FindOptions -): Document { +function makeFindCommand(ns: MongoDBNamespace, filter: Document, options: FindOptions): Document { const findCommand: Document = { find: ns.collection, filter diff --git a/src/operations/find_one.ts b/src/operations/find_one.ts deleted file mode 100644 index 4631f747b50..00000000000 --- a/src/operations/find_one.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { type Db } from '..'; -import { type Document, pluckBSONSerializeOptions } from '../bson'; -import { type OnDemandDocumentDeserializeOptions } from '../cmap/wire_protocol/on_demand/document'; -import { CursorResponse, ExplainResponse } from '../cmap/wire_protocol/responses'; -import type { Server } from '../sdam/server'; -import type { ClientSession } from '../sessions'; -import { type TimeoutContext } from '../timeout'; -import { MongoDBNamespace } from '../utils'; -import { CommandOperation } from './command'; -import { type FindOneOptions, makeFindCommand } from './find'; -import { Aspect, defineAspects } from './operation'; - -/** @internal */ -export class FindOneOperation extends CommandOperation { - override options: FindOneOptions; - /** @internal */ - private namespace: MongoDBNamespace; - /** @internal */ - private filter: Document; - /** @internal */ - protected deserializationOptions: OnDemandDocumentDeserializeOptions; - - constructor(db: Db, collectionName: string, filter: Document, options: FindOneOptions = {}) { - super(db, options); - this.namespace = new MongoDBNamespace(db.databaseName, collectionName); - // special case passing in an ObjectId as a filter - this.filter = filter != null && filter._bsontype === 'ObjectId' ? { _id: filter } : filter; - this.options = { ...options }; - this.deserializationOptions = { - ...pluckBSONSerializeOptions(options), - validation: { - utf8: options?.enableUtf8Validation === false ? false : true - } - }; - } - - override get commandName() { - return 'find' as const; - } - - override async execute( - server: Server, - session: ClientSession | undefined, - timeoutContext: TimeoutContext - ): Promise { - const command: Document = makeFindCommand(this.namespace, this.filter, this.options); - // 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 - command.limit = 1; - command.singleBatch = true; - if (command.noCursorTimeout != null) { - delete command.noCursorTimeout; - } - if (command.batchSize != null) { - delete command.batchSize; - } - - if (this.explain) { - const response = await super.executeCommand( - server, - session, - command, - timeoutContext, - ExplainResponse - ); - return response.toObject() as TSchema; - } else { - const response = await super.executeCommand( - server, - session, - command, - timeoutContext, - CursorResponse - ); - return response.shift(this.deserializationOptions); - } - } -} - -defineAspects(FindOneOperation, [Aspect.READ_OPERATION, Aspect.RETRYABLE, Aspect.EXPLAINABLE]); diff --git a/test/integration/crud/crud_api.test.ts b/test/integration/crud/crud_api.test.ts index 6b8bcfbadcb..eceb1ce60fc 100644 --- a/test/integration/crud/crud_api.test.ts +++ b/test/integration/crud/crud_api.test.ts @@ -98,6 +98,50 @@ describe('CRUD API', function () { await collection.drop().catch(() => null); await client.close(); }); + + describe('when the operation succeeds', () => { + it('the cursor for findOne is closed', async function () { + const spy = sinon.spy(Collection.prototype, 'find'); + const result = await collection.findOne({}); + expect(result).to.deep.equal({ _id: 1 }); + expect(events.at(0)).to.be.instanceOf(CommandSucceededEvent); + expect(spy.returnValues.at(0)).to.have.property('closed', true); + expect(spy.returnValues.at(0)).to.have.nested.property('session.hasEnded', true); + }); + }); + + describe('when the find operation fails', () => { + beforeEach(async function () { + const failPoint: FailPoint = { + configureFailPoint: 'failCommand', + mode: 'alwaysOn', + data: { + failCommands: ['find'], + // 1 == InternalError, but this value not important to the test + errorCode: 1 + } + }; + await client.db().admin().command(failPoint); + }); + + afterEach(async function () { + const failPoint: FailPoint = { + configureFailPoint: 'failCommand', + mode: 'off', + data: { failCommands: ['find'] } + }; + await client.db().admin().command(failPoint); + }); + + it('the cursor for findOne is closed', async function () { + const spy = sinon.spy(Collection.prototype, 'find'); + const error = await collection.findOne({}).catch(error => error); + expect(error).to.be.instanceOf(MongoServerError); + expect(events.at(0)).to.be.instanceOf(CommandFailedEvent); + expect(spy.returnValues.at(0)).to.have.property('closed', true); + expect(spy.returnValues.at(0)).to.have.nested.property('session.hasEnded', true); + }); + }); }); describe('countDocuments()', () => { From 395d2b1f6c8f1f310f8175181037f28d203b3746 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 24 Jul 2025 11:37:08 +0200 Subject: [PATCH 10/15] Update src/collection.ts Co-authored-by: Bailey Pearson --- src/collection.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index bc4a7c34fa1..ba5f31916d5 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -556,12 +556,8 @@ export class Collection { // noCursorTimeout must be unset as well as batchSize. // See: https://github.com/mongodb/specifications/blob/master/source/crud/crud.md#findone-api-details opts.singleBatch = true; - if (opts.noCursorTimeout != null) { - delete opts.noCursorTimeout; - } - if (opts.batchSize != null) { - delete opts.batchSize; - } + const { batchSize: _batchSize, noCursorTimeout: _noCursorTimeout, ...opts } = options; + opts.singleBatch = true; const cursor = this.find(filter, opts).limit(1); return await cursor.next(); } From 3e28e07fb28df6c451ce0f70a7e94f248db93d52 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 24 Jul 2025 11:37:17 +0200 Subject: [PATCH 11/15] Update src/collection.ts Co-authored-by: Bailey Pearson --- src/collection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/collection.ts b/src/collection.ts index ba5f31916d5..e05e3409857 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -549,7 +549,7 @@ export class Collection { async findOne( filter: Filter = {}, - options: Omit & Abortable = {} + options: Omit & Abortable = {} ): Promise | null> { const opts = { ...options }; // Explicitly set the limit to 1 and singleBatch to true for all commands, per the spec. From 842484c4d064ec4a521496f665c72c2df0adbc64 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 24 Jul 2025 11:41:44 +0200 Subject: [PATCH 12/15] chore: address comments --- src/collection.ts | 2 -- src/operations/find.ts | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index e05e3409857..64e8dfd2dc9 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -551,11 +551,9 @@ export class Collection { filter: Filter = {}, options: Omit & Abortable = {} ): Promise | null> { - const opts = { ...options }; // 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 - opts.singleBatch = true; const { batchSize: _batchSize, noCursorTimeout: _noCursorTimeout, ...opts } = options; opts.singleBatch = true; const cursor = this.find(filter, opts).limit(1); diff --git a/src/operations/find.ts b/src/operations/find.ts index 00987a03d16..90714728ac0 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -202,8 +202,6 @@ function makeFindCommand(ns: MongoDBNamespace, filter: Document, options: FindOp ) { findCommand.limit = -options.batchSize; } - - findCommand.singleBatch = true; } else { if (options.batchSize === options.limit) { // Spec dictates that if these are equal the batchSize should be one more than the From 171f776c28fd0c1847fe476110b436e4c6dc1558 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 25 Jul 2025 19:53:51 +0200 Subject: [PATCH 13/15] chore: comments --- src/collection.ts | 4 +++- src/operations/find.ts | 8 +------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 64e8dfd2dc9..0ad56d04f22 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -557,7 +557,9 @@ export class Collection { const { batchSize: _batchSize, noCursorTimeout: _noCursorTimeout, ...opts } = options; opts.singleBatch = true; const cursor = this.find(filter, opts).limit(1); - return await cursor.next(); + const result = await cursor.next(); + await cursor.close(); + return result; } /** diff --git a/src/operations/find.ts b/src/operations/find.ts index 90714728ac0..9795f583ca8 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -195,13 +195,7 @@ 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.limit = -options.batchSize; } else { if (options.batchSize === options.limit) { // Spec dictates that if these are equal the batchSize should be one more than the From 9474e8f311c7273c5084efadba21711fd5102197 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 31 Jul 2025 12:14:37 +0200 Subject: [PATCH 14/15] chore: comments --- src/operations/find.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/operations/find.ts b/src/operations/find.ts index 9795f583ca8..16283817b16 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -185,12 +185,7 @@ function makeFindCommand(ns: MongoDBNamespace, filter: Document, options: FindOp } if (typeof options.limit === 'number') { - if (options.limit < 0) { - findCommand.limit = -options.limit; - findCommand.singleBatch = true; - } else { - findCommand.limit = options.limit; - } + findCommand.limit = options.limit; } if (typeof options.batchSize === 'number') { From 32925bc937a87eb89e3a2b1c56661e3496d97bdd Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 1 Aug 2025 11:27:05 +0200 Subject: [PATCH 15/15] chore: bring back negative limit check --- src/operations/find.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/operations/find.ts b/src/operations/find.ts index 16283817b16..9795f583ca8 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -185,7 +185,12 @@ function makeFindCommand(ns: MongoDBNamespace, filter: Document, options: FindOp } if (typeof options.limit === 'number') { - findCommand.limit = options.limit; + if (options.limit < 0) { + findCommand.limit = -options.limit; + findCommand.singleBatch = true; + } else { + findCommand.limit = options.limit; + } } if (typeof options.batchSize === 'number') {