fix(NODE-3546): revert findOne not found result type to null (#2945)

Author: nbbeeken

src/collection.ts

@@ -1,4 +1,4 @@
-import { DEFAULT_PK_FACTORY, emitWarningOnce, maybePromise, resolveOptions } from './utils';
+import { DEFAULT_PK_FACTORY, emitWarningOnce, resolveOptions } from './utils';
 import { ReadPreference, ReadPreferenceLike } from './read_preference';
 import {
   normalizeHintField,
@@ -99,7 +99,7 @@ import type {
 
 /** @public */
 export interface ModifyResult<TSchema = Document> {
-  value?: TSchema;
+  value: TSchema | null;
   lastErrorObject?: Document;
   ok: 0 | 1;
 }
@@ -676,51 +676,48 @@ export class Collection<TSchema extends Document = Document> {
    * @param options - Optional settings for the command
    * @param callback - An optional callback, a Promise will be returned if none is provided
    */
-  findOne(): Promise<TSchema | undefined>;
-  findOne(callback: Callback<TSchema | undefined>): void;
-  findOne(filter: Filter<TSchema>): Promise<TSchema | undefined>;
-  findOne(filter: Filter<TSchema>, callback: Callback<TSchema | undefined>): void;
-  findOne(filter: Filter<TSchema>, options: FindOptions): Promise<TSchema | undefined>;
-  findOne(
-    filter: Filter<TSchema>,
-    options: FindOptions,
-    callback: Callback<TSchema | undefined>
-  ): void;
+  findOne(): Promise<TSchema | null>;
+  findOne(callback: Callback<TSchema | null>): void;
+  findOne(filter: Filter<TSchema>): Promise<TSchema | null>;
+  findOne(filter: Filter<TSchema>, callback: Callback<TSchema | null>): void;
+  findOne(filter: Filter<TSchema>, options: FindOptions): Promise<TSchema | null>;
+  findOne(filter: Filter<TSchema>, options: FindOptions, callback: Callback<TSchema | null>): void;
 
   // allow an override of the schema.
-  findOne<T = TSchema>(): Promise<T | undefined>;
-  findOne<T = TSchema>(callback: Callback<T | undefined>): void;
-  findOne<T = TSchema>(filter: Filter<TSchema>): Promise<T | undefined>;
-  findOne<T = TSchema>(filter: Filter<TSchema>, options?: FindOptions): Promise<T | undefined>;
+  findOne<T = TSchema>(): Promise<T | null>;
+  findOne<T = TSchema>(callback: Callback<T | null>): void;
+  findOne<T = TSchema>(filter: Filter<TSchema>): Promise<T | null>;
+  findOne<T = TSchema>(filter: Filter<TSchema>, options?: FindOptions): Promise<T | null>;
   findOne<T = TSchema>(
     filter: Filter<TSchema>,
     options?: FindOptions,
-    callback?: Callback<T | undefined>
+    callback?: Callback<T | null>
   ): void;
 
   findOne(
-    filter?: Filter<TSchema> | Callback<TSchema | undefined>,
-    options?: FindOptions | Callback<TSchema | undefined>,
-    callback?: Callback<TSchema>
-  ): Promise<TSchema | undefined> | void {
+    filter?: Filter<TSchema> | Callback<TSchema | null>,
+    options?: FindOptions | Callback<TSchema | null>,
+    callback?: Callback<TSchema | null>
+  ): Promise<TSchema | null> | void {
     if (callback != null && typeof callback !== 'function') {
       throw new MongoInvalidArgumentError(
         'Third parameter to `findOne()` must be a callback or undefined'
       );
     }
 
-    if (typeof filter === 'function')
-      (callback = filter as Callback<Document | undefined>), (filter = {}), (options = {});
-    if (typeof options === 'function') (callback = options), (options = {});
+    if (typeof filter === 'function') {
+      callback = filter as Callback<TSchema | null>;
+      filter = {};
+      options = {};
+    }
+    if (typeof options === 'function') {
+      callback = options;
+      options = {};
+    }
 
     const finalFilter = filter ?? {};
     const finalOptions = options ?? {};
-    return maybePromise(callback, callback =>
-      this.find(finalFilter, finalOptions)
-        .limit(-1)
-        .batchSize(1)
-        .next((error, result) => callback(error, result ?? undefined))
-    );
+    return this.find(finalFilter, finalOptions).limit(-1).batchSize(1).next(callback);
   }
 
   /**

src/cursor/abstract_cursor.ts

@@ -297,6 +297,7 @@ export abstract class AbstractCursor<
   /** Get the next available document from the cursor, returns null if no more documents are available. */
   next(): Promise<TSchema | null>;
   next(callback: Callback<TSchema | null>): void;
+  next(callback?: Callback<TSchema | null>): Promise<TSchema | null> | void;
   next(callback?: Callback<TSchema | null>): Promise<TSchema | null> | void {
     return maybePromise(callback, done => {
       if (this[kId] === Long.ZERO) {

test/functional/crud_api.test.js

@@ -1,7 +1,7 @@
 'use strict';
 const test = require('./shared').assert;
 const { expect } = require('chai');
-const { ReturnDocument } = require('../../src');
+const { ReturnDocument, ObjectId } = require('../../src');
 const setupDatabase = require('./shared').setupDatabase;
 
 // instanceof cannot be use reliably to detect the new models in js due to scoping and new
@@ -14,6 +14,27 @@ describe('CRUD API', function () {
     return setupDatabase(this.configuration);
   });
 
+  it('should correctly execute findOne method using crud api', async function () {
+    const configuration = this.configuration;
+    const client = configuration.newClient();
+    await client.connect();
+    const db = client.db(configuration.db);
+    const collection = db.collection('t');
+
+    await collection.insertOne({ findOneTest: 1 });
+
+    const findOneResult = await collection.findOne({ findOneTest: 1 });
+
+    expect(findOneResult).to.have.property('findOneTest', 1);
+    expect(findOneResult).to.have.property('_id').that.is.instanceOf(ObjectId);
+
+    const findNoneResult = await collection.findOne({ findOneTest: 2 });
+    expect(findNoneResult).to.be.null;
+
+    await collection.drop();
+    await client.close();
+  });
+
   it('should correctly execute find method using crud api', {
     // Add a tag that our runner can trigger on
     // in this case we are setting that node needs to be higher than 0.10.X to run

test/functional/sessions.test.js

@@ -301,8 +301,8 @@ describe('Sessions', function () {
         controlSession = client.startSession();
 
         // set up sessions with two sets of cluster times
-        expect(await collection.findOne({}, { session: controlSession })).to.be.undefined;
-        expect(await collection.findOne({}, { session: testSession })).to.be.undefined;
+        expect(await collection.findOne({}, { session: controlSession })).to.be.null;
+        expect(await collection.findOne({}, { session: testSession })).to.be.null;
         await collection.insertOne({ apple: 'green' });
         expect(await collection.findOne({}, { session: otherSession }))
           .property('apple')

test/types/community/collection/findX.test-d.ts

@@ -87,9 +87,7 @@ cursor.forEach(
   }
 );
 
-expectType<Bag | undefined>(
-  await collectionBag.findOne({ color: 'red' }, { projection: { cost: 1 } })
-);
+expectType<Bag | null>(await collectionBag.findOne({ color: 'red' }, { projection: { cost: 1 } }));
 
 const overrideFind = await collectionBag.findOne<{ cost: number }>(
   { color: 'white' },
@@ -98,7 +96,7 @@ const overrideFind = await collectionBag.findOne<{ cost: number }>(
 expectType<PropExists<typeof overrideFind, 'color'>>(false);
 
 // Overriding findOne, makes the return that exact type
-expectType<{ cost: number } | undefined>(
+expectType<{ cost: number } | null>(
   await collectionBag.findOne<{ cost: number }>({ color: 'red' }, { projection: { cost: 1 } })
 );
 
@@ -121,13 +119,13 @@ interface House {
 
 const car = db.collection<Car>('car');
 
-expectNotType<House | undefined>(await car.findOne({}));
+expectNotType<House | null>(await car.findOne({}));
 
 interface Car {
   make: string;
 }
 
-function printCar(car: Car | undefined) {
+function printCar(car: Car | null) {
   console.log(car ? `A car of ${car.make} make` : 'No car');
 }
 
@@ -232,12 +230,12 @@ interface TypedDb extends Db {
 const typedDb = client.db('test2') as TypedDb;
 
 const person = typedDb.collection('people').findOne({});
-expectType<Promise<Person | undefined>>(person);
+expectType<Promise<Person | null>>(person);
 
 typedDb.collection('people').findOne({}, function (_err, person) {
-  expectType<Person | undefined>(person);
+  expectType<Person | null | undefined>(person); // null is if nothing is found, undefined is when there is an error defined
 });
 
 typedDb.collection('things').findOne({}, function (_err, thing) {
-  expectType<Thing | undefined>(thing);
+  expectType<Thing | null | undefined>(thing);
 });

test/types/union_schema.test-d.ts

@@ -31,9 +31,9 @@ expectAssignable<ShapeInsert>({ height: 4, width: 4 });
 expectAssignable<ShapeInsert>({ radius: 4 });
 
 const c: Collection<Shape> = null as never;
-expectType<Promise<Shape | undefined>>(c.findOne({ height: 4, width: 4 }));
+expectType<Promise<Shape | null>>(c.findOne({ height: 4, width: 4 }));
 // collection API can only respect TSchema given, cannot pick a type inside a union
-expectNotType<Promise<Rectangle | undefined>>(c.findOne({ height: 4, width: 4 }));
+expectNotType<Promise<Rectangle | null>>(c.findOne({ height: 4, width: 4 }));
 
 interface A {
   _id: number;