fix: findOne(undefined|null|false) returns undefined (#5905)

Co-authored-by: Victor Daev <viktor.d@soft2bet.com>

src/entity-manager/LiteralEntityManager.ts

@@ -472,12 +472,27 @@ export function createLiteralEntityManager<Entity>({ connection, queryRunner }:
             // console.log("results", results);
             // return results;
         },
-
+        /**
+         * @param entityClass
+         * @param {string | string[] | number | number[] | Date | Date[] | ObjectID | ObjectID[] | FindOptions<Entity> | any} [idOrOptionsOrConditions]
+         * @param {FindOptions<Entity>} [maybeOptions]
+         */
         findOne<Entity>(
             entityClass: EntityTarget<Entity>,
-            idOrOptionsOrConditions?: string | string[] | number | number[] | Date | Date[] | ObjectID | ObjectID[] | FindOptions<Entity> | any,
-            maybeOptions?: FindOptions<Entity>
+            ...args: (string | string[] | number | number[] | Date | Date[] | ObjectID | ObjectID[] | FindOptions<Entity> | any)[]
         ): Promise<Entity | undefined> {
+            if (args.length > 2) {
+                throw new Error("Too many arguments.");
+            }
+
+            const idOrOptionsOrConditions = args[0];
+            const maybeOptions = args[1];
+
+            if (args.length >= 1) {
+                if (idOrOptionsOrConditions === undefined || idOrOptionsOrConditions === null || idOrOptionsOrConditions === false) {
+                    return Promise.resolve(undefined);
+                }
+            }
 
             let findOptions: FindOptions<any> | undefined = undefined;
             if (FindOptionsUtils.isFindOptions(idOrOptionsOrConditions)) {

src/entity-manager/LiteralMongoEntityManager.ts

@@ -300,9 +300,27 @@ export function createLiteralMongoEntityManager<Entity>({ connection }: {
             return await cursor.toArray();
         },
 
+        /**
+         * @param entityClassOrName
+         * @param {string | string[] | number | number[] | Date | Date[] | ObjectID | ObjectID[] | FindOptions<Entity> | FindOptionsWhere<Entity>} [optionsOrConditions]
+         * @param {FindOptions<Entity>} [maybeOptions]
+         */
         async findOne<Entity>(entityClassOrName: EntityTarget<Entity>,
-                              optionsOrConditions?: string | string[] | number | number[] | Date | Date[] | ObjectID | ObjectID[] | FindOptions<Entity> | FindOptionsWhere<Entity>,
-                              maybeOptions?: FindOptions<Entity>): Promise<Entity | undefined> {
+                             ...args: (string | string[] | number | number[] | Date | Date[] | ObjectID | ObjectID[] | FindOptions<Entity> | FindOptionsWhere<Entity> | undefined)[]
+                             ): Promise<Entity | undefined> {
+            if (args.length > 2) {
+                throw new Error("Too many arguments.");
+            }
+
+            const optionsOrConditions = args[0];
+            const maybeOptions = args[1];
+
+            if (args.length >= 1) {
+                if (optionsOrConditions === undefined || optionsOrConditions === null || optionsOrConditions === false) {
+                    return Promise.resolve(undefined);
+                }
+            }
+
             const objectIdInstance = PlatformTools.load("mongodb").ObjectID;
             const id = (optionsOrConditions instanceof objectIdInstance) || typeof optionsOrConditions === "string" ? optionsOrConditions : undefined;
             const findOneOptionsOrConditions = (id ? maybeOptions : optionsOrConditions) as any;

src/repository/BaseEntity.ts

@@ -320,9 +320,11 @@ export class BaseEntity {
 
     /**
      * Finds first entity that matches given conditions.
+     * @param {string|number|Date|ObjectID|FindOptions<T>|FindOptionsWhere<T>} [optionsOrConditions]
+     * @param {FindOptions<T>} [maybeOptions]
      */
-    static findOne<T extends BaseEntity>(this: ObjectType<T>, optionsOrConditions?: string|number|Date|ObjectID|FindOptions<T>|FindOptionsWhere<T>, maybeOptions?: FindOptions<T>): Promise<T|undefined> {
-        return (this as any).getRepository().findOne(optionsOrConditions as any, maybeOptions);
+    static findOne<T extends BaseEntity>(this: ObjectType<T>, ...args: (string|number|Date|ObjectID|FindOptions<T>|FindOptionsWhere<T>|undefined)[]): Promise<T|undefined> {
+        return (this as any).getRepository().findOne(...args);
     }
 
     /**

src/repository/LiteralMongoRepository.ts

@@ -70,8 +70,12 @@ export function createLiteralMongoRepository<Entity>({ manager, target, queryRun
             return this.manager.findByIds(this.getMetadata().target, ids, optionsOrConditions);
         },
 
-        findOne(optionsOrConditions?: string | number | Date | ObjectID | FindOptions<Entity> | FindOptionsWhere<Entity>, maybeOptions?: FindOptions<Entity>): Promise<Entity | undefined> {
-            return this.manager.findOne(this.getMetadata().target, optionsOrConditions as any, maybeOptions as any);
+        /**
+         * @param {string | number | Date | ObjectID | FindOptions<Entity> | FindOptionsWhere<Entity>} [optionsOrConditions]
+         * @param {FindOptions<Entity>} [maybeOptions]
+         */
+        findOne(...args: (string | number | Date | ObjectID | FindOptions<Entity> | FindOptionsWhere<Entity> | undefined)[]): Promise<Entity | undefined> {
+            return this.manager.findOne(this.getMetadata().target, ...args);
         },
 
         createCursor<T = any>(query?: ObjectLiteral): Cursor<T> {

src/repository/LiteralRepository.ts

@@ -123,8 +123,12 @@ export function createLiteralRepository<Entity>({ manager, target, queryRunner }
             return this.manager.findByIds(this.getMetadata().target as any, ids, optionsOrConditions as any);
         },
 
-        findOne(optionsOrConditions?: string | number | Date | ObjectID | FindOptions<Entity> | FindOptionsWhere<Entity>, maybeOptions?: FindOptions<Entity>): Promise<Entity | undefined> {
-            return this.manager.findOne(this.getMetadata().target as any, optionsOrConditions as any, maybeOptions);
+        /**
+         * @param {string | number | Date | ObjectID | FindOptions<Entity> | FindOptionsWhere<Entity>} [optionsOrConditions]
+         * @param {FindOptions<Entity>} [maybeOptions]
+         */
+        findOne(...args: (string | number | Date | ObjectID | FindOptions<Entity> | FindOptionsWhere<Entity> | undefined)[]): Promise<Entity | undefined> {
+            return this.manager.findOne(this.getMetadata().target as any, ...args);
         },
 
         findOneOrFail(optionsOrConditions?: string | number | Date | ObjectID | FindOptions<Entity> | FindOptionsWhere<Entity>, maybeOptions?: FindOptions<Entity>): Promise<Entity> {

test/github-issues/2500/entity/Post.mongo.ts

@@ -0,0 +1,16 @@
+import {Entity} from "../../../../src/decorator/entity/Entity";
+import {Column} from "../../../../src/decorator/columns/Column";
+import {BaseEntity} from "../../../../src/repository/BaseEntity";
+import {ObjectID, ObjectIdColumn} from "../../../../src";
+
+@Entity()
+export class MongoPost extends BaseEntity {
+    @ObjectIdColumn()
+    id: ObjectID;
+
+    @Column()
+    title: string;
+
+    @Column()
+    author: string;
+}

test/github-issues/2500/entity/Post.sql.ts

@@ -0,0 +1,16 @@
+import {Entity} from "../../../../src/decorator/entity/Entity";
+import {Column} from "../../../../src/decorator/columns/Column";
+import {BaseEntity} from "../../../../src/repository/BaseEntity";
+import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn";
+
+@Entity()
+export class SqlPost extends BaseEntity {
+    @PrimaryGeneratedColumn()
+    id: number;
+
+    @Column()
+    title: string;
+
+    @Column()
+    author: string;
+}

test/github-issues/2500/issue-2500.ts

@@ -0,0 +1,208 @@
+import "reflect-metadata";
+import {expect} from "chai";
+import {createTestingConnections, closeTestingConnections, reloadTestingDatabases, TestingOptions} from "../../utils/test-utils";
+import {Connection} from "../../../src/connection/Connection";
+import {BaseEntity} from "../../../src/repository/BaseEntity";
+import {SqlPost} from "./entity/Post.sql";
+import {MongoPost} from "./entity/Post.mongo";
+
+const testParams: {dbType: string, connectionsParams: TestingOptions, PostConstructor: typeof SqlPost | typeof MongoPost}[] = [
+    {
+        dbType: "sql",
+        connectionsParams: {
+            entities: [__dirname + "/entity/*.sql{.js,.ts}"],
+            schemaCreate: true,
+            dropSchema: true,
+        },
+        PostConstructor: SqlPost
+    },
+    {
+        dbType: "mongo",
+        connectionsParams: {
+            entities: [__dirname + "/entity/*.mongo{.js,.ts}"],
+            enabledDrivers: ["mongodb"],
+            schemaCreate: true,
+            dropSchema: true,
+        },
+        PostConstructor: MongoPost
+    }
+];
+
+describe("github issues > #2500 .findOne(undefined) returns first item in the database instead of undefined", () => {
+    testParams.forEach(({dbType, connectionsParams, PostConstructor}) => describe(dbType, () => {
+        let connections: Connection[];
+        before(async () => connections = await createTestingConnections(connectionsParams));
+        beforeEach(() => reloadTestingDatabases(connections));
+        after(() => closeTestingConnections(connections));
+
+        const Post = PostConstructor;
+        let firstPost: SqlPost | MongoPost;
+        let secondPost: SqlPost | MongoPost;
+
+        beforeEach(() => Promise.all(connections.map(async connection => {
+            firstPost = new Post();
+            firstPost.title = "How to buy a cat";
+            firstPost.author = "John Doe";
+            await connection.manager.save(firstPost);
+
+            secondPost = new Post();
+            secondPost.title = "How to buy a dog";
+            secondPost.author = "Jane Doe";
+            await connection.manager.save(secondPost);
+        })));
+
+        describe("EntityManager.findOne", () => {
+            it("should find one record when no arguments given", () => Promise.all(connections.map(async connection => {
+                const post = await connection.manager.findOne(Post);
+
+                expect(post).not.to.be.undefined;
+                expect(post!.id).not.to.be.undefined;
+                expect(post!.title).to.be.a("string");
+                expect(post!.author).to.be.a("string");
+            })));
+
+            it("should find one record for given criteria", () => Promise.all(connections.map(async connection => {
+                const post1 = await connection.manager.findOne(Post, firstPost.id);
+                expect(post1).not.to.be.undefined;
+                expect(post1!.id).to.be.eql(firstPost.id);
+                expect(post1!.title).to.be.equal("How to buy a cat");
+                expect(post1!.author).to.be.equal("John Doe");
+
+                const post2 = await connection.manager.findOne(Post, { title: "How to buy a dog" });
+                expect(post2).not.to.be.undefined;
+                expect(post2!.id).to.be.eql(secondPost.id);
+                expect(post2!.title).to.be.equal("How to buy a dog");
+                expect(post2!.author).to.be.equal("Jane Doe");
+
+                const post3 = await connection.manager.findOne(Post, firstPost.id, { where: { title: "How to buy a cat" } });
+                expect(post3).not.to.be.undefined;
+                expect(post3!.id).to.be.eql(firstPost.id);
+                expect(post3!.title).to.be.equal("How to buy a cat");
+                expect(post3!.author).to.be.equal("John Doe");
+            })));
+
+            it("should find no record for wrong criteria", () => Promise.all(connections.map(async connection => {
+                expect(await connection.manager.findOne(Post, { title: "How to buy a pig" })).to.be.undefined;
+                expect(await connection.manager.findOne(Post, firstPost.id, { where: { title: "How to buy a dog" } })).to.be.undefined;
+            })));
+
+            it("should find no record for findOne(undefined)", () => Promise.all(connections.map(async connection => {
+                expect(await connection.manager.findOne(Post, undefined)).to.be.undefined;
+            })));
+
+            it("should throw an error for findOne(null)", () => Promise.all(connections.map(async connection => {
+                expect(await connection.manager.findOne(Post, null as any)).to.be.undefined;
+            })));
+
+            it("should throw an error for findOne(false)", () => Promise.all(connections.map(async connection => {
+                expect(await connection.manager.findOne(Post, false as any)).to.be.undefined;
+            })));
+        });
+
+        describe("Repository.findOne", () => {
+            it("should find one record when no arguments given", () => Promise.all(connections.map(async connection => {
+                const post = await connection.getRepository(Post).findOne();
+
+                expect(post).not.to.be.undefined;
+                expect(post!.id).not.to.be.undefined;
+                expect(post!.title).to.be.a("string");
+                expect(post!.author).to.be.a("string");
+            })));
+
+            it("should find one record for given criteria", () => Promise.all(connections.map(async connection => {
+                const post1 = await connection.getRepository(Post).findOne(firstPost.id);
+                expect(post1).not.to.be.undefined;
+                expect(post1!.id).to.be.eql(firstPost.id);
+                expect(post1!.title).to.be.equal("How to buy a cat");
+                expect(post1!.author).to.be.equal("John Doe");
+
+                const post2 = await connection.getRepository(Post).findOne({ title: "How to buy a dog" });
+                expect(post2).not.to.be.undefined;
+                expect(post2!.id).to.be.eql(secondPost.id);
+                expect(post2!.title).to.be.equal("How to buy a dog");
+                expect(post2!.author).to.be.equal("Jane Doe");
+
+                const post3 = await connection.getRepository(Post).findOne(firstPost.id, { where: { title: "How to buy a cat" } });
+                expect(post3).not.to.be.undefined;
+                expect(post3!.id).to.be.eql(firstPost.id);
+                expect(post3!.title).to.be.equal("How to buy a cat");
+                expect(post3!.author).to.be.equal("John Doe");
+            })));
+
+            it("should find no record for wrong criteria", () => Promise.all(connections.map(async connection => {
+                expect(await connection.getRepository(Post).findOne({ title: "How to buy a pig" })).to.be.undefined;
+                expect(await connection.getRepository(Post).findOne(firstPost.id, { where: { title: "How to buy a dog" } })).to.be.undefined;
+            })));
+
+            it("should find no record for findOne(undefined)", () => Promise.all(connections.map(async connection => {
+                expect(await connection.getRepository(Post).findOne(undefined)).to.be.undefined;
+            })));
+
+            it("should throw an error for findOne(null)", () => Promise.all(connections.map(async connection => {
+                expect(await connection.getRepository(Post).findOne(null as any)).to.be.undefined;
+            })));
+
+            it("should throw an error for findOne(false)", () => Promise.all(connections.map(async connection => {
+                expect(await connection.getRepository(Post).findOne(false as any)).to.be.undefined;
+            })));
+        });
+
+        describe("BaseEntity.findOne", () => {
+            it("should find one record when no arguments given", () => Promise.all(connections.map(async connection => {
+                BaseEntity.useConnection(connection);
+                const post = await Post.findOne();
+
+                expect(post).not.to.be.undefined;
+                expect(post!.id).not.to.be.undefined;
+                expect(post!.title).to.be.a("string");
+                expect(post!.author).to.be.a("string");
+            })));
+
+            it("should find one record for given criteria", () => Promise.all(connections.map(async connection => {
+                BaseEntity.useConnection(connection);
+                const post1 = await Post.findOne(firstPost.id);
+                expect(post1).not.to.be.undefined;
+                expect(post1!.id).to.be.eql(firstPost.id);
+                expect(post1!.title).to.be.equal("How to buy a cat");
+                expect(post1!.author).to.be.equal("John Doe");
+
+                BaseEntity.useConnection(connection);
+                const post2 = await Post.findOne({ title: "How to buy a dog" });
+                expect(post2).not.to.be.undefined;
+                expect(post2!.id).to.be.eql(secondPost.id);
+                expect(post2!.title).to.be.equal("How to buy a dog");
+                expect(post2!.author).to.be.equal("Jane Doe");
+
+                BaseEntity.useConnection(connection);
+                const post3 = await Post.findOne(firstPost.id, { where: { title: "How to buy a cat" } });
+                expect(post3).not.to.be.undefined;
+                expect(post3!.id).to.be.eql(firstPost.id);
+                expect(post3!.title).to.be.equal("How to buy a cat");
+                expect(post3!.author).to.be.equal("John Doe");
+            })));
+
+            it("should find no record for wrong criteria", () => Promise.all(connections.map(async connection => {
+                BaseEntity.useConnection(connection);
+                expect(await Post.findOne({ title: "How to buy a pig" })).to.be.undefined;
+
+                BaseEntity.useConnection(connection);
+                expect(await Post.findOne(firstPost.id, { where: { title: "How to buy a dog" } })).to.be.undefined;
+            })));
+
+            it("should find no record for findOne(undefined)", () => Promise.all(connections.map(async connection => {
+                BaseEntity.useConnection(connection);
+                expect(await Post.findOne(undefined)).to.be.undefined;
+            })));
+
+            it("should throw an error for findOne(null)", () => Promise.all(connections.map(async connection => {
+                BaseEntity.useConnection(connection);
+                expect(await Post.findOne(null as any)).to.be.undefined;
+            })));
+
+            it("should throw an error for findOne(false)", () => Promise.all(connections.map(async connection => {
+                BaseEntity.useConnection(connection);
+                expect(await Post.findOne(false as any)).to.be.undefined;
+            })));
+        });
+    }));
+});