From 4e5f2626844b5efa3a330b6000a17fd73533c74e Mon Sep 17 00:00:00 2001 From: Harminder virk Date: Wed, 3 Jun 2020 16:55:34 +0530 Subject: [PATCH] test: add tests to ensure that relationship query builder handles groupLimit properly --- adonis-typings/model.ts | 2 +- adonis-typings/relations.ts | 15 ++ src/Orm/Relations/Base/QueryBuilder.ts | 42 +++++- src/Orm/Relations/HasMany/QueryBuilder.ts | 8 +- .../Relations/HasManyThrough/QueryBuilder.ts | 11 +- src/Orm/Relations/ManyToMany/QueryBuilder.ts | 8 +- test/orm/model-has-many-through.spec.ts | 137 ++++++++++++++++++ test/orm/model-has-many.spec.ts | 137 ++++++++++++++++++ test/orm/model-many-to-many.spec.ts | 113 +++++++++++++++ 9 files changed, 463 insertions(+), 10 deletions(-) diff --git a/adonis-typings/model.ts b/adonis-typings/model.ts index 52ef3229..438d426e 100644 --- a/adonis-typings/model.ts +++ b/adonis-typings/model.ts @@ -792,7 +792,7 @@ declare module '@ioc:Adonis/Lucid/Model' { firstOrFail ( this: T, options?: ModelAdapterOptions, - ): Promise> + ): Promise> /** * Find many using an array of primary keys diff --git a/adonis-typings/relations.ts b/adonis-typings/relations.ts index f897cd68..3745fa5f 100644 --- a/adonis-typings/relations.ts +++ b/adonis-typings/relations.ts @@ -626,6 +626,14 @@ declare module '@ioc:Adonis/Lucid/Relations' { createMany ( values: Partial>>[], ): Promise[]> + + /** + * Return a query builder instance of the relationship + */ + query> (): HasManyQueryBuilderContract< + RelatedModel, + Result + > } /** @@ -728,6 +736,13 @@ declare module '@ioc:Adonis/Lucid/Relations' { Relation extends RelationshipsContract, RelatedModel extends LucidModel, > extends RelationQueryClientContract { + /** + * Return a query builder instance of the relationship + */ + query> (): HasManyThroughQueryBuilderContract< + RelatedModel, + Result + > } /** diff --git a/src/Orm/Relations/Base/QueryBuilder.ts b/src/Orm/Relations/Base/QueryBuilder.ts index bd36c667..526ad96a 100644 --- 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 index 480deb65..56f7c42f 100644 --- 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 index fbd26f52..11b24e88 100644 --- 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 index dd6cbdd8..16b0b870 100644 --- 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 index 3442e382..7ada40c7 100644 --- 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 + } + 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 + } + 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) + }) }) } diff --git a/test/orm/model-has-many.spec.ts b/test/orm/model-has-many.spec.ts index 2cfeb220..302cd491 100644 --- a/test/orm/model-has-many.spec.ts +++ b/test/orm/model-has-many.spec.ts @@ -1461,6 +1461,143 @@ if (process.env.DB !== 'mysql_legacy') { assert.equal(users[1].posts[1].title, 'Lucid 102') assert.exists(users[1].posts[1].createdAt) }) + + test('apply standard limit when not eagerloading', async (assert) => { + class Post extends BaseModel { + @column() + public userId: number + + @column() + public title: string + + @column() + public createdAt: Date + } + + class User extends BaseModel { + @column({ isPrimary: true }) + public id: number + + @hasMany(() => Post) + public posts: HasMany + } + + await db.insertQuery().table('users').insert([{ username: 'virk' }, { username: 'nikk' }]) + const [user0] = await db.query().from('users') + + /** + * User 1 + */ + await db.insertQuery().table('posts').insert([ + { + user_id: user0.id, + title: 'Adonis 101', + created_at: new Date(), + }, + { + user_id: user0.id, + title: 'Adonis 102', + created_at: new Date(), + }, + { + user_id: user0.id, + title: 'Adonis 103', + created_at: new Date(), + }, + { + user_id: user0.id, + title: 'Adonis 104', + created_at: new Date(), + }, + { + user_id: user0.id, + title: 'Adonis 105', + created_at: new Date(), + }, + ]) + + User.boot() + + const user = await User.firstOrFail() + const { sql, bindings } = user.related('posts').query().groupLimit(2).toSQL() + const { sql: knexSql, bindings: knexBindings } = db.query() + .from('posts') + .where('user_id', user.id) + .limit(2) + .toSQL() + + assert.equal(sql, knexSql) + assert.deepEqual(bindings, knexBindings) + }) + + test('apply standard order by when not eagerloading', async (assert) => { + class Post extends BaseModel { + @column() + public userId: number + + @column() + public title: string + + @column() + public createdAt: Date + } + + class User extends BaseModel { + @column({ isPrimary: true }) + public id: number + + @hasMany(() => Post) + public posts: HasMany + } + + await db.insertQuery().table('users').insert([{ username: 'virk' }, { username: 'nikk' }]) + const [user0] = await db.query().from('users') + + /** + * User 1 + */ + await db.insertQuery().table('posts').insert([ + { + user_id: user0.id, + title: 'Adonis 101', + created_at: new Date(), + }, + { + user_id: user0.id, + title: 'Adonis 102', + created_at: new Date(), + }, + { + user_id: user0.id, + title: 'Adonis 103', + created_at: new Date(), + }, + { + user_id: user0.id, + title: 'Adonis 104', + created_at: new Date(), + }, + { + user_id: user0.id, + title: 'Adonis 105', + created_at: new Date(), + }, + ]) + + User.boot() + + const user = await User.firstOrFail() + const { sql, bindings } = user.related('posts').query().groupLimit(2).groupOrderBy('id', 'desc').toSQL() + const { sql: knexSql, bindings: knexBindings } = db.query() + .from('posts') + .where('user_id', user.id) + .limit(2) + .orderBy('id', 'desc') + .toSQL() + + assert.equal(sql, knexSql) + assert.deepEqual(bindings, knexBindings) + }) }) } diff --git a/test/orm/model-many-to-many.spec.ts b/test/orm/model-many-to-many.spec.ts index a6dd88dc..5cde7ec6 100644 --- a/test/orm/model-many-to-many.spec.ts +++ b/test/orm/model-many-to-many.spec.ts @@ -1538,6 +1538,119 @@ if (process.env.DB !== 'mysql_legacy') { assert.equal(users[1].skills[1].$extras.pivot_skill_id, 5) assert.equal(users[1].skills[1].$extras.pivot_proficiency, 'Master') }) + + test('apply standard limit when not eagerloading', async (assert) => { + class Skill extends BaseModel { + @column({ isPrimary: true }) + public id: number + + @column() + public name: string + } + + class User extends BaseModel { + @column({ isPrimary: true }) + public id: number + + @manyToMany(() => Skill) + public skills: ManyToMany + } + + User.boot() + await db.insertQuery().table('users').insert([ + { username: 'virk' }, + { username: 'nikk' }, + ]) + + await db.insertQuery().table('skills').insert([ + { name: 'Programming' }, + { name: 'Dancing' }, + { name: 'Designing' }, + { name: 'Cooking' }, + { name: 'Singing' }, + ]) + + const skillIds = [1, 2, 3, 4, 5] + + /** + * User 1 skills + */ + await db.insertQuery().table('skill_user').insert(skillIds.map((id) => { + return { user_id: 1, skill_id: id } + })) + + const user = await User.query().firstOrFail() + const { sql, bindings } = user.related('skills').query().groupLimit(2).toSQL() + const { sql: knexSql, bindings: knexBindings } = db.query() + .from('skills') + .select('skills.*', 'skill_user.user_id as pivot_user_id', 'skill_user.skill_id as pivot_skill_id') + .innerJoin('skill_user', 'skills.id', 'skill_user.skill_id') + .where('skill_user.user_id', 1) + .limit(2) + .toSQL() + + assert.equal(sql, knexSql) + assert.deepEqual(bindings, knexBindings) + }) + + test('apply standard order by when not eagerloading', async (assert) => { + class Skill extends BaseModel { + @column({ isPrimary: true }) + public id: number + + @column() + public name: string + } + + class User extends BaseModel { + @column({ isPrimary: true }) + public id: number + + @manyToMany(() => Skill) + public skills: ManyToMany + } + + User.boot() + await db.insertQuery().table('users').insert([ + { username: 'virk' }, + { username: 'nikk' }, + ]) + + await db.insertQuery().table('skills').insert([ + { name: 'Programming' }, + { name: 'Dancing' }, + { name: 'Designing' }, + { name: 'Cooking' }, + { name: 'Singing' }, + ]) + + const skillIds = [1, 2, 3, 4, 5] + + /** + * User 1 skills + */ + await db.insertQuery().table('skill_user').insert(skillIds.map((id) => { + return { user_id: 1, skill_id: id } + })) + + const user = await User.query().firstOrFail() + const { sql, bindings } = user.related('skills').query() + .groupLimit(2) + .groupOrderBy('skill_user.id', 'desc') + .toSQL() + + const { sql: knexSql, bindings: knexBindings } = db.query() + .from('skills') + .select('skills.*', 'skill_user.user_id as pivot_user_id', 'skill_user.skill_id as pivot_skill_id') + .innerJoin('skill_user', 'skills.id', 'skill_user.skill_id') + .where('skill_user.user_id', 1) + .limit(2) + .orderBy('skill_user.id', 'desc') + .toSQL() + + assert.equal(sql, knexSql) + assert.deepEqual(bindings, knexBindings) + }) }) }