From 952fd2fe7ba619a0906133694e4888f57ad8cecc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Ad=C3=A1mek?= <banan23@gmail.com>
Date: Mon, 11 May 2020 22:38:13 +0200
Subject: [PATCH] feat(core): add `having` to `FindOptions`

---
 packages/core/src/EntityManager.ts           | 26 ++++++++++----------
 packages/core/src/drivers/DatabaseDriver.ts  |  4 +--
 packages/core/src/drivers/IDatabaseDriver.ts | 12 +++++----
 packages/knex/src/AbstractSqlDriver.ts       |  6 +++--
 packages/knex/src/query/QueryBuilder.ts      |  2 +-
 packages/mongodb/src/MongoDriver.ts          |  4 +--
 tests/DatabaseDriver.test.ts                 |  4 +--
 tests/EntityManager.mysql.test.ts            |  2 ++
 8 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/packages/core/src/EntityManager.ts b/packages/core/src/EntityManager.ts
index bd9c68efec35..0061e8cd5f6b 100644
--- a/packages/core/src/EntityManager.ts
+++ b/packages/core/src/EntityManager.ts
@@ -67,7 +67,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
   /**
    * Finds all entities matching your `where` query. You can pass additional options via the `options` parameter.
    */
-  async find<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOptions): Promise<T[]>;
+  async find<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOptions<T>): Promise<T[]>;
 
   /**
    * Finds all entities matching your `where` query.
@@ -77,11 +77,11 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
   /**
    * Finds all entities matching your `where` query.
    */
-  async find<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: string[] | boolean | FindOptions, orderBy?: QueryOrderMap, limit?: number, offset?: number): Promise<T[]> {
+  async find<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: string[] | boolean | FindOptions<T>, orderBy?: QueryOrderMap, limit?: number, offset?: number): Promise<T[]> {
     entityName = Utils.className(entityName);
     where = SmartQueryHelper.processWhere(where, entityName, this.metadata.get(entityName));
     this.validator.validateParams(where);
-    const options = Utils.isObject<FindOptions>(populate) ? populate : { populate, orderBy, limit, offset };
+    const options = Utils.isObject<FindOptions<T>>(populate) ? populate : { populate, orderBy, limit, offset };
     options.orderBy = options.orderBy || {};
     const results = await this.driver.find<T>(entityName, where, { ...options, populate: this.preparePopulate(options.populate) }, this.transactionContext);
 
@@ -106,7 +106,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
    * Calls `em.find()` and `em.count()` with the same arguments (where applicable) and returns the results as tuple
    * where first element is the array of entities and the second is the count.
    */
-  async findAndCount<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOptions): Promise<[T[], number]>;
+  async findAndCount<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOptions<T>): Promise<[T[], number]>;
 
   /**
    * Calls `em.find()` and `em.count()` with the same arguments (where applicable) and returns the results as tuple
@@ -118,7 +118,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
    * Calls `em.find()` and `em.count()` with the same arguments (where applicable) and returns the results as tuple
    * where first element is the array of entities and the second is the count.
    */
-  async findAndCount<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: string[] | boolean | FindOptions, orderBy?: QueryOrderMap, limit?: number, offset?: number): Promise<[T[], number]> {
+  async findAndCount<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: string[] | boolean | FindOptions<T>, orderBy?: QueryOrderMap, limit?: number, offset?: number): Promise<[T[], number]> {
     const [entities, count] = await Promise.all([
       this.find(entityName, where, populate as string[], orderBy, limit, offset),
       this.count(entityName, where),
@@ -130,7 +130,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
   /**
    * Finds first entity matching your `where` query.
    */
-  async findOne<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOneOptions): Promise<T | null>;
+  async findOne<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOneOptions<T>): Promise<T | null>;
 
   /**
    * Finds first entity matching your `where` query.
@@ -140,9 +140,9 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
   /**
    * Finds first entity matching your `where` query.
    */
-  async findOne<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: string[] | boolean | FindOneOptions, orderBy?: QueryOrderMap): Promise<T | null> {
+  async findOne<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: string[] | boolean | FindOneOptions<T>, orderBy?: QueryOrderMap): Promise<T | null> {
     entityName = Utils.className(entityName);
-    const options = Utils.isObject<FindOneOptions>(populate) ? populate : { populate, orderBy };
+    const options = Utils.isObject<FindOneOptions<T>>(populate) ? populate : { populate, orderBy };
     const meta = this.metadata.get<T>(entityName);
     this.validator.validateEmptyWhere(where);
     where = SmartQueryHelper.processWhere(where as FilterQuery<T>, entityName, meta);
@@ -172,7 +172,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
    * You can override the factory for creating this method via `options.failHandler` locally
    * or via `Configuration.findOneOrFailHandler` globally.
    */
-  async findOneOrFail<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOneOrFailOptions): Promise<T>;
+  async findOneOrFail<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOneOrFailOptions<T>): Promise<T>;
 
   /**
    * Finds first entity matching your `where` query. If nothing found, it will throw an error.
@@ -186,11 +186,11 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
    * You can override the factory for creating this method via `options.failHandler` locally
    * or via `Configuration.findOneOrFailHandler` globally.
    */
-  async findOneOrFail<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: string[] | boolean | FindOneOrFailOptions, orderBy?: QueryOrderMap): Promise<T> {
+  async findOneOrFail<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, populate?: string[] | boolean | FindOneOrFailOptions<T>, orderBy?: QueryOrderMap): Promise<T> {
     const entity = await this.findOne(entityName, where, populate as string[], orderBy);
 
     if (!entity) {
-      const options = Utils.isObject<FindOneOrFailOptions>(populate) ? populate : {};
+      const options = Utils.isObject<FindOneOrFailOptions<T>>(populate) ? populate : {};
       options.failHandler = options.failHandler || this.config.get('findOneOrFailHandler');
       entityName = Utils.className(entityName);
       throw options.failHandler!(entityName, where);
@@ -574,7 +574,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
     }
   }
 
-  private async lockAndPopulate<T extends AnyEntity<T>>(entityName: string, entity: T, where: FilterQuery<T>, options: FindOneOptions): Promise<T> {
+  private async lockAndPopulate<T extends AnyEntity<T>>(entityName: string, entity: T, where: FilterQuery<T>, options: FindOneOptions<T>): Promise<T> {
     if (options.lockMode === LockMode.OPTIMISTIC) {
       await this.lock(entity, options.lockMode, options.lockVersion);
     }
@@ -590,6 +590,6 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
 
 }
 
-export interface FindOneOrFailOptions extends FindOneOptions {
+export interface FindOneOrFailOptions<T> extends FindOneOptions<T> {
   failHandler?: (entityName: string, where: Dictionary | IPrimaryKey | any) => Error;
 }
diff --git a/packages/core/src/drivers/DatabaseDriver.ts b/packages/core/src/drivers/DatabaseDriver.ts
index e3dff788f230..5abd3da6b48d 100644
--- a/packages/core/src/drivers/DatabaseDriver.ts
+++ b/packages/core/src/drivers/DatabaseDriver.ts
@@ -21,9 +21,9 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD
   protected constructor(protected readonly config: Configuration,
                         protected readonly dependencies: string[]) { }
 
-  abstract async find<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOptions, ctx?: Transaction): Promise<T[]>;
+  abstract async find<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOptions<T>, ctx?: Transaction): Promise<T[]>;
 
-  abstract async findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOneOptions, ctx?: Transaction): Promise<T | null>;
+  abstract async findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOneOptions<T>, ctx?: Transaction): Promise<T | null>;
 
   abstract async nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>, ctx?: Transaction): Promise<QueryResult>;
 
diff --git a/packages/core/src/drivers/IDatabaseDriver.ts b/packages/core/src/drivers/IDatabaseDriver.ts
index c65017a3b493..4fc48a02fa03 100644
--- a/packages/core/src/drivers/IDatabaseDriver.ts
+++ b/packages/core/src/drivers/IDatabaseDriver.ts
@@ -1,4 +1,4 @@
-import { EntityData, EntityMetadata, EntityProperty, AnyEntity, FilterQuery, Primary, Dictionary } from '../typings';
+import { EntityData, EntityMetadata, EntityProperty, AnyEntity, FilterQuery, Primary, Dictionary, QBFilterQuery } from '../typings';
 import { Connection, QueryResult, Transaction } from '../connections';
 import { QueryOrderMap, QueryFlag } from '../enums';
 import { Platform } from '../platforms';
@@ -27,12 +27,12 @@ export interface IDatabaseDriver<C extends Connection = Connection> {
   /**
    * Finds selection of entities
    */
-  find<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOptions, ctx?: Transaction): Promise<T[]>;
+  find<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOptions<T>, ctx?: Transaction): Promise<T[]>;
 
   /**
    * Finds single entity (table row, document)
    */
-  findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOneOptions, ctx?: Transaction): Promise<T | null>;
+  findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOneOptions<T>, ctx?: Transaction): Promise<T | null>;
 
   nativeInsert<T extends AnyEntity<T>>(entityName: string, data: EntityData<T>, ctx?: Transaction): Promise<QueryResult>;
 
@@ -74,7 +74,7 @@ export interface IDatabaseDriver<C extends Connection = Connection> {
 
 }
 
-export interface FindOptions {
+export interface FindOptions<T> {
   populate?: string[] | boolean;
   orderBy?: QueryOrderMap;
   limit?: number;
@@ -84,12 +84,14 @@ export interface FindOptions {
   schema?: string;
   flags?: QueryFlag[];
   groupBy?: string | string[];
+  having?: QBFilterQuery<T>;
 }
 
-export interface FindOneOptions {
+export interface FindOneOptions<T> {
   populate?: string[] | boolean;
   orderBy?: QueryOrderMap;
   groupBy?: string | string[];
+  having?: QBFilterQuery<T>;
   lockMode?: LockMode;
   lockVersion?: number | Date;
   refresh?: boolean;
diff --git a/packages/knex/src/AbstractSqlDriver.ts b/packages/knex/src/AbstractSqlDriver.ts
index 4d1fe63fc0d4..2278e182aa2c 100644
--- a/packages/knex/src/AbstractSqlDriver.ts
+++ b/packages/knex/src/AbstractSqlDriver.ts
@@ -29,7 +29,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
     return new SqlEntityManager(this.config, this, this.metadata, useContext) as unknown as EntityManager<D>;
   }
 
-  async find<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOptions, ctx?: Transaction<KnexTransaction>): Promise<T[]> {
+  async find<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOptions<T>, ctx?: Transaction<KnexTransaction>): Promise<T[]> {
     const meta = this.metadata.get(entityName);
     options = { populate: [], orderBy: {}, ...(options || {}) };
     options.populate = this.autoJoinOneToOneOwner(meta, options.populate as string[]);
@@ -44,6 +44,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
       .where(where as Dictionary)
       .orderBy(options.orderBy!)
       .groupBy(options.groupBy!)
+      .having(options.having!)
       .withSchema(options.schema);
 
     if (options.limit !== undefined) {
@@ -55,7 +56,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
     return this.rethrow(qb.execute('all'));
   }
 
-  async findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOneOptions, ctx?: Transaction<KnexTransaction>): Promise<T | null> {
+  async findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options?: FindOneOptions<T>, ctx?: Transaction<KnexTransaction>): Promise<T | null> {
     options = { populate: [], orderBy: {}, ...(options || {}) };
     const meta = this.metadata.get(entityName);
     options.populate = this.autoJoinOneToOneOwner(meta, options.populate as string[]);
@@ -75,6 +76,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
       .where(where as Dictionary)
       .orderBy(options.orderBy!)
       .groupBy(options.groupBy!)
+      .having(options.having!)
       .limit(1)
       .setLockMode(options.lockMode)
       .withSchema(options.schema);
diff --git a/packages/knex/src/query/QueryBuilder.ts b/packages/knex/src/query/QueryBuilder.ts
index 9637b11f2065..b49276216dec 100644
--- a/packages/knex/src/query/QueryBuilder.ts
+++ b/packages/knex/src/query/QueryBuilder.ts
@@ -157,7 +157,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
     return this;
   }
 
-  having(cond: QBFilterQuery | string, params?: any[]): this {
+  having(cond: QBFilterQuery | string = {}, params?: any[]): this {
     if (Utils.isString(cond)) {
       cond = { [`(${cond})`]: Utils.asArray(params) };
     }
diff --git a/packages/mongodb/src/MongoDriver.ts b/packages/mongodb/src/MongoDriver.ts
index 91cff2e0c29e..44d8dc6a54dd 100644
--- a/packages/mongodb/src/MongoDriver.ts
+++ b/packages/mongodb/src/MongoDriver.ts
@@ -22,14 +22,14 @@ export class MongoDriver extends DatabaseDriver<MongoConnection> {
     return new MongoEntityManager(this.config, this, this.metadata, useContext) as unknown as EntityManager<D>;
   }
 
-  async find<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options: FindOptions, ctx?: Transaction<ClientSession>): Promise<T[]> {
+  async find<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options: FindOptions<T>, ctx?: Transaction<ClientSession>): Promise<T[]> {
     where = this.renameFields(entityName, where);
     const res = await this.rethrow(this.getConnection('read').find<T>(entityName, where, options.orderBy, options.limit, options.offset, options.fields, ctx));
 
     return res.map((r: T) => this.mapResult<T>(r, this.metadata.get(entityName))!);
   }
 
-  async findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options: FindOneOptions = { populate: [], orderBy: {} }, ctx?: Transaction<ClientSession>): Promise<T | null> {
+  async findOne<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, options: FindOneOptions<T> = { populate: [], orderBy: {} }, ctx?: Transaction<ClientSession>): Promise<T | null> {
     if (Utils.isPrimaryKey(where)) {
       where = { _id: new ObjectId(where as string) } as FilterQuery<T>;
     }
diff --git a/tests/DatabaseDriver.test.ts b/tests/DatabaseDriver.test.ts
index 670c04820097..5bf73749da64 100644
--- a/tests/DatabaseDriver.test.ts
+++ b/tests/DatabaseDriver.test.ts
@@ -11,11 +11,11 @@ class Driver extends DatabaseDriver<Connection> {
     return Promise.resolve(0);
   }
 
-  async find<T>(entityName: string, where: FilterQuery<T>, options: FindOptions | undefined, ctx: Transaction | undefined): Promise<T[]> {
+  async find<T>(entityName: string, where: FilterQuery<T>, options: FindOptions<T> | undefined, ctx: Transaction | undefined): Promise<T[]> {
     return Promise.resolve([]);
   }
 
-  async findOne<T>(entityName: string, where: FilterQuery<T>, options: FindOneOptions | undefined, ctx: Transaction | undefined): Promise<T | null> {
+  async findOne<T>(entityName: string, where: FilterQuery<T>, options: FindOneOptions<T> | undefined, ctx: Transaction | undefined): Promise<T | null> {
     return null;
   }
 
diff --git a/tests/EntityManager.mysql.test.ts b/tests/EntityManager.mysql.test.ts
index 42b7e12ecd52..20e267d83d6b 100644
--- a/tests/EntityManager.mysql.test.ts
+++ b/tests/EntityManager.mysql.test.ts
@@ -1986,6 +1986,7 @@ describe('EntityManagerMySql', () => {
       orderBy: { name: QueryOrder.ASC, books: { title: QueryOrder.ASC } },
       limit: 5,
       groupBy: ['id', 'name', 'e1.title'],
+      having: { $or: [{ age: { $gt: 0 } }, { age: { $lte: 0 } }, { age: null }] }, // no-op just for testing purposes
     });
 
     expect(res1).toHaveLength(2);
@@ -1996,6 +1997,7 @@ describe('EntityManagerMySql', () => {
       'left join `address2` as `e2` on `e0`.`id` = `e2`.`author_id` ' +
       'where `e1`.`title` like ? ' +
       'group by `e0`.`id`, `e0`.`name`, `e1`.`title` ' +
+      'having (`e0`.`age` > ? or `e0`.`age` <= ? or `e0`.`age` is null) ' +
       'order by `e0`.`name` asc, `e1`.`title` asc limit ?');
 
     // with paginate flag (and a bit of dark sql magic) we get what we want