test: add tests to ensure that relationship query builder handles gro…

…upLimit properly
adonisjs · Jun 3, 2020 · 4e5f262 · 4e5f262
1 parent def372c
commit 4e5f262
Show file tree

Hide file tree

Showing 9 changed files with 463 additions and 10 deletions.
diff --git a/adonis-typings/model.ts b/adonis-typings/model.ts
@@ -792,7 +792,7 @@ declare module '@ioc:Adonis/Lucid/Model' {
     firstOrFail<T extends LucidModel> (
       this: T,
       options?: ModelAdapterOptions,
-    ): Promise<null | InstanceType<T>>
+    ): Promise<InstanceType<T>>
 
     /**
      * Find many using an array of primary keys

diff --git a/adonis-typings/relations.ts b/adonis-typings/relations.ts
@@ -626,6 +626,14 @@ declare module '@ioc:Adonis/Lucid/Relations' {
     createMany (
       values: Partial<ModelAttributes<InstanceType<RelatedModel>>>[],
     ): Promise<InstanceType<RelatedModel>[]>
+
+    /**
+     * Return a query builder instance of the relationship
+     */
+    query<Result extends any = InstanceType<RelatedModel>> (): HasManyQueryBuilderContract<
+      RelatedModel,
+      Result
+    >
   }
 
   /**
@@ -728,6 +736,13 @@ declare module '@ioc:Adonis/Lucid/Relations' {
     Relation extends RelationshipsContract,
     RelatedModel extends LucidModel,
   > extends RelationQueryClientContract<Relation, RelatedModel> {
+    /**
+     * Return a query builder instance of the relationship
+     */
+    query<Result extends any = InstanceType<RelatedModel>> (): HasManyThroughQueryBuilderContract<
+      RelatedModel,
+      Result
+    >
   }
 
   /**

diff --git a/src/Orm/Relations/Base/QueryBuilder.ts b/src/Orm/Relations/Base/QueryBuilder.ts
@@ -25,7 +25,13 @@ export abstract class BaseQueryBuilder extends ModelQueryBuilder implements Rela
   /**
    * Eager constraints
    */
-  protected groupConstraints: { limit?: number, orderBy?: string } = {}
+  protected groupConstraints: {
+    limit?: number,
+    orderBy?: {
+      column: string,
+      direction?: 'asc' | 'desc',
+    },
+  } = {}
 
   /**
    * A flag to know, if query builder is instantiated for
@@ -141,7 +147,7 @@ export abstract class BaseQueryBuilder extends ModelQueryBuilder implements Rela
    * Define the group limit
    */
   public groupOrderBy (column: string, direction?: 'asc' | 'desc'): this {
-    this.groupConstraints.orderBy = direction ? `${this.resolveKey(column)} ${direction}` : column
+    this.groupConstraints.orderBy = { column, direction }
     return this
   }
 
@@ -150,6 +156,21 @@ export abstract class BaseQueryBuilder extends ModelQueryBuilder implements Rela
    */
   public toSQL () {
     this.applyConstraints()
+    if (this.isEagerQuery) {
+      return this.groupConstraints.limit ? this.getGroupLimitQuery().toSQL() : super.toSQL()
+    }
+
+    /**
+     * Apply orderBy and limit on the standard query when not
+     * an eagerloading query
+     */
+    if (this.groupConstraints.limit) {
+      this.limit(this.groupConstraints.limit)
+    }
+    if (this.groupConstraints.orderBy) {
+      this.orderBy(this.groupConstraints.orderBy.column, this.groupConstraints.orderBy.direction)
+    }
+
     return super.toSQL()
   }
 
@@ -158,6 +179,21 @@ export abstract class BaseQueryBuilder extends ModelQueryBuilder implements Rela
    */
   public exec () {
     this.applyConstraints()
-    return this.groupConstraints.limit ? this.getGroupLimitQuery().exec() : super.exec()
+    if (this.isEagerQuery) {
+      return this.groupConstraints.limit ? this.getGroupLimitQuery().exec() : super.exec()
+    }
+
+    /**
+     * Apply orderBy and limit on the standard query when not
+     * an eagerloading query
+     */
+    if (this.groupConstraints.limit) {
+      this.limit(this.groupConstraints.limit)
+    }
+    if (this.groupConstraints.orderBy) {
+      this.orderBy(this.groupConstraints.orderBy.column, this.groupConstraints.orderBy.direction)
+    }
+
+    return super.exec()
   }
 }
diff --git a/src/Orm/Relations/HasMany/QueryBuilder.ts b/src/Orm/Relations/HasMany/QueryBuilder.ts
@@ -121,10 +121,14 @@ export class HasManyQueryBuilder extends BaseQueryBuilder implements HasManyQuer
    * Returns the group limit query
    */
   public getGroupLimitQuery () {
+    const { direction, column } = this.groupConstraints.orderBy || {
+      column: this.resolveKey(this.relation.relatedModel().primaryKey),
+      direction: 'desc',
+    }
+
     const rowName = 'ADONIS_GROUP_LIMIT_COUNTER'
-    const primaryColumn = this.resolveKey(this.relation.relatedModel().primaryKey)
     const partitionBy = `PARTITION BY ${this.relation.foreignKeyColumName}`
-    const orderBy = `ORDER BY ${this.groupConstraints.orderBy || `${primaryColumn} DESC`}`
+    const orderBy = `ORDER BY ${column} ${direction}`
 
     /**
      * Select * when no columns are selected

diff --git a/src/Orm/Relations/HasManyThrough/QueryBuilder.ts b/src/Orm/Relations/HasManyThrough/QueryBuilder.ts
@@ -233,10 +233,17 @@ export class HasManyThroughQueryBuilder extends BaseQueryBuilder implements HasM
    * Returns the group limit query
    */
   public getGroupLimitQuery () {
+    console.log(this.relation.relatedModel().primaryKey)
+    console.log(this.relation.foreignKey)
+
+    const { direction, column } = this.groupConstraints.orderBy || {
+      column: this.prefixRelatedTable(this.resolveKey(this.relation.relatedModel().primaryKey)),
+      direction: 'desc',
+    }
+
     const rowName = 'ADONIS_GROUP_LIMIT_COUNTER'
-    const primaryColumn = this.resolveKey(this.relation.relatedModel().primaryKey)
     const partitionBy = `PARTITION BY ${this.prefixThroughTable(this.relation.foreignKeyColumnName)}`
-    const orderBy = `ORDER BY ${this.groupConstraints.orderBy || `${this.prefixRelatedTable(primaryColumn)} DESC`}`
+    const orderBy = `ORDER BY ${column} ${direction}`
 
     /**
      * Select * when no columns are selected

diff --git a/src/Orm/Relations/ManyToMany/QueryBuilder.ts b/src/Orm/Relations/ManyToMany/QueryBuilder.ts
@@ -411,10 +411,14 @@ export class ManyToManyQueryBuilder extends BaseQueryBuilder implements ManyToMa
    * Returns the group limit query
    */
   public getGroupLimitQuery () {
+    const { direction, column } = this.groupConstraints.orderBy || {
+      column: this.prefixRelatedTable(this.resolveKey(this.relation.relatedModel().primaryKey)),
+      direction: 'desc',
+    }
+
     const rowName = 'ADONIS_GROUP_LIMIT_COUNTER'
-    const primaryColumn = this.resolveKey(this.relation.relatedModel().primaryKey)
     const partitionBy = `PARTITION BY ${this.prefixPivotTable(this.relation.pivotForeignKey)}`
-    const orderBy = `ORDER BY ${this.groupConstraints.orderBy || `${this.prefixRelatedTable(primaryColumn)} DESC`}`
+    const orderBy = `ORDER BY ${column} ${direction}`
 
     /**
      * Select * when no columns are selected

diff --git a/test/orm/model-has-many-through.spec.ts b/test/orm/model-has-many-through.spec.ts
@@ -1469,6 +1469,143 @@ if (process.env.DB !== 'mysql_legacy') {
       assert.isDefined(countries[1].posts[1].createdAt)
       assert.equal(countries[1].posts[1].$extras.through_country_id, 2)
     })
+
+    test('apply standard limit when not eagerloading', async (assert) => {
+      class User extends BaseModel {
+        @column({ isPrimary: true })
+        public id: number
+
+        @column()
+        public countryId: number
+      }
+      User.boot()
+
+      class Post extends BaseModel {
+        @column({ isPrimary: true })
+        public id: number
+
+        @column()
+        public userId: number
+
+        @column()
+        public title: string
+      }
+      Post.boot()
+
+      class Country extends BaseModel {
+        @column({ isPrimary: true })
+        public id: number
+
+        @hasManyThrough([() => Post, () => User])
+        public posts: HasManyThrough<typeof Post>
+      }
+      Country.boot()
+
+      await db.insertQuery().table('countries').insert([
+        { name: 'India' },
+      ])
+
+      await db.insertQuery().table('users').insert([
+        { username: 'virk', country_id: 1 },
+        { username: 'nikk', country_id: 1 },
+      ])
+
+      /**
+       * Country 1 posts
+       */
+      await db.insertQuery().table('posts').insert([
+        { title: 'Adonis 101', user_id: 1 },
+        { title: 'Adonis 102', user_id: 1 },
+        { title: 'Adonis 103', user_id: 2 },
+        { title: 'Adonis 104', user_id: 2 },
+        { title: 'Adonis 105', user_id: 1 },
+      ])
+
+      const country = await Country.firstOrFail()
+      const { sql, bindings } = country.related('posts').query().groupLimit(2).toSQL()
+      const { sql: knexSql, bindings: knexBindings } = db.query()
+        .from('posts')
+        .select('posts.*', 'users.country_id as through_country_id')
+        .innerJoin('users', 'users.id', 'posts.user_id')
+        .where('users.country_id', 1)
+        .limit(2)
+        .toSQL()
+
+      assert.equal(sql, knexSql)
+      assert.deepEqual(bindings, knexBindings)
+    })
+
+    test.only('apply standard order by when not eagerloading', async (assert) => {
+      class User extends BaseModel {
+        @column({ isPrimary: true })
+        public id: number
+
+        @column()
+        public countryId: number
+      }
+      User.boot()
+
+      class Post extends BaseModel {
+        @column({ isPrimary: true })
+        public id: number
+
+        @column()
+        public userId: number
+
+        @column()
+        public title: string
+      }
+      Post.boot()
+
+      class Country extends BaseModel {
+        @column({ isPrimary: true })
+        public id: number
+
+        @hasManyThrough([() => Post, () => User])
+        public posts: HasManyThrough<typeof Post>
+      }
+      Country.boot()
+
+      await db.insertQuery().table('countries').insert([
+        { name: 'India' },
+      ])
+
+      await db.insertQuery().table('users').insert([
+        { username: 'virk', country_id: 1 },
+        { username: 'nikk', country_id: 1 },
+      ])
+
+      /**
+       * Country 1 posts
+       */
+      await db.insertQuery().table('posts').insert([
+        { title: 'Adonis 101', user_id: 1 },
+        { title: 'Adonis 102', user_id: 1 },
+        { title: 'Adonis 103', user_id: 2 },
+        { title: 'Adonis 104', user_id: 2 },
+        { title: 'Adonis 105', user_id: 1 },
+      ])
+
+      const country = await Country.firstOrFail()
+      const { sql, bindings } = country
+        .related('posts')
+        .query()
+        .groupLimit(2)
+        .groupOrderBy('users.country_id', 'desc')
+        .toSQL()
+
+      const { sql: knexSql, bindings: knexBindings } = db.query()
+        .from('posts')
+        .select('posts.*', 'users.country_id as through_country_id')
+        .innerJoin('users', 'users.id', 'posts.user_id')
+        .where('users.country_id', 1)
+        .limit(2)
+        .orderBy('users.country_id', 'desc')
+        .toSQL()
+
+      assert.equal(sql, knexSql)
+      assert.deepEqual(bindings, knexBindings)
+    })
   })
 }