refactor: improving relationships API

adonis-typings/model.ts

@@ -88,19 +88,24 @@ declare module '@ioc:Adonis/Lucid/Model' {
 
   export type AvailableRelations = 'hasOne'
 
+  type ModelExecuteableQueryBuilder = ModelQueryBuilderContract<any> & ExcutableQueryBuilderContract<any>
+
+  /**
+   * Callback accepted by the preload method
+   */
+  export type PreloadCallback = (builder: ModelExecuteableQueryBuilder) => void
+
   /**
    * Interface to be implemented by all relationship types
    */
   export interface RelationContract {
     type: AvailableRelations
     serializeAs: string
     relatedModel (): ModelConstructorContract
-    preload (relation: string, callback?: (builder: ModelQueryBuilderContract<any>) => void): this
-    exec (
-      model: ModelContract | ModelContract[],
-      options?: ModelOptions,
-      callback?: (builder: ModelQueryBuilderContract<any>) => void,
-    ): Promise<void>
+    getQuery (model: ModelContract, options?: ModelOptions): ModelExecuteableQueryBuilder
+    getEagerQuery (models: ModelContract[], options?: ModelOptions): ModelExecuteableQueryBuilder
+    setRelated (model: ModelContract, related?: ModelContract | null): void
+    setRelatedMany (models: ModelContract[], related: ModelContract[]): void
   }
 
   /**
@@ -136,7 +141,7 @@ declare module '@ioc:Adonis/Lucid/Model' {
     /**
      * Define relationships to be preloaded
      */
-    preload (relation: string, callback?: (builder: ModelQueryBuilderContract<any>) => void): this
+    preload (relation: string, callback?: PreloadCallback): this
   }
 
   /**

adonis-typings/querybuilder.ts

@@ -363,18 +363,15 @@ declare module '@ioc:Adonis/Lucid/DatabaseQueryBuilder' {
     Record extends Dictionary<any, string>,
   > {
     /**
-     * Accepting a typed column with the alias for the count. Unlike knex
-     * we enforce the alias, otherwise the output highly varies based
-     * upon the driver in use
+     * Accepting a typed column with the alias for the count.
      */
     <K extends keyof Record, Alias extends string>(
       column: OneOrMany<K>,
-      alias: Alias,
+      alias?: Alias,
     ): Builder
 
     /**
-     * Accepting an object for multiple counts in a single query. Again
-     * aliases are enforced for consistency.
+     * Accepting an object for multiple counts in a single query.
      */
     <
       K extends keyof Record,
@@ -389,12 +386,11 @@ declare module '@ioc:Adonis/Lucid/DatabaseQueryBuilder' {
      */
     <Alias extends string>(
       column: OneOrMany<ValueWithSubQueries<string>>,
-      alias: Alias,
+      alias?: Alias,
     ): Builder
 
     /**
-     * Accepting an object for multiple counts in a single query. Again
-     * aliases are enforced for consistency
+     * Accepting an object for multiple counts in a single query.
      */
     <
       Alias extends string,

src/Database/QueryBuilder/Database.ts

@@ -71,7 +71,7 @@ export class DatabaseQueryBuilder extends Chainable implements DatabaseQueryBuil
     }
 
     if (!alias) {
-      throw new Error('Aggregate function needs an alias as 2nd argument')
+      return alias
     }
 
     return { [alias]: this.$transformValue(columns) }

src/Orm/QueryBuilder/index.ts

@@ -15,6 +15,8 @@ import { Exception } from '@poppinss/utils'
 
 import {
   ModelOptions,
+  ModelContract,
+  PreloadCallback,
   RelationContract,
   ModelConstructorContract,
   ModelQueryBuilderContract,
@@ -29,8 +31,7 @@ import { Executable, ExecutableConstructor } from '../../Traits/Executable'
  * updating and deleting records.
  */
 @trait<ExecutableConstructor>(Executable)
-export class ModelQueryBuilder extends Chainable implements ModelQueryBuilderContract<
-  ModelConstructorContract
+export class ModelQueryBuilder extends Chainable implements ModelQueryBuilderContract<ModelConstructorContract
 > {
   /**
    * Sideloaded attributes that will be passed to the model instances
@@ -43,7 +44,8 @@ export class ModelQueryBuilder extends Chainable implements ModelQueryBuilderCon
   private _preloads: {
     [name: string]: {
       relation: RelationContract,
-      callback?: (builder: ModelQueryBuilderContract<any>) => void,
+      callback?: PreloadCallback,
+      children: { relationName: string, callback?: PreloadCallback }[],
     },
   } = {}
 
@@ -61,6 +63,60 @@ export class ModelQueryBuilder extends Chainable implements ModelQueryBuilderCon
     builder.table(model.$table)
   }
 
+  /**
+  * Parses the relation name string for nested relations. Nested relations
+  * can be defined using the dot notation.
+  */
+  private _parseRelationName (relationName: string, callback?: PreloadCallback) {
+    const relations = relationName.split('.')
+    const primary = relations.shift()!
+    const relation = this.model.$getRelation(primary)
+
+    /**
+     * Undefined relationship
+     */
+    if (!relation) {
+      throw new Exception(`${primary} is not defined as a relationship on ${this.model.name} model`)
+    }
+
+    return {
+      primary,
+      relation,
+      children: relations.length ? { relationName: relations.join(''), callback } : null,
+      callback: relations.length ? null : callback,
+    }
+  }
+
+  /**
+   * Process preloaded relationship
+   */
+  private async _processRelation (models: ModelContract[], name: string) {
+    const relation = this._preloads[name]
+    const query = relation.relation.getEagerQuery(models, this.options)
+
+    /**
+     * Pass nested preloads
+     */
+    relation.children.forEach(({ relationName, callback }) => query.preload(relationName, callback))
+
+    /**
+     * Invoke callback when defined
+     */
+    if (typeof (relation.callback) === 'function') {
+      relation.callback(query)
+    }
+
+    /**
+     * Execute query
+     */
+    const result = await query.exec()
+
+    /**
+     * Set relationships on models
+     */
+    relation.relation.setRelatedMany(models, result)
+  }
+
   /**
    * Wraps the query result to model instances. This method is invoked by the
    * Executable trait.
@@ -73,9 +129,9 @@ export class ModelQueryBuilder extends Chainable implements ModelQueryBuilderCon
     )
 
     await Promise.all(Object.keys(this._preloads).map((name) => {
-      const relation = this._preloads[name]
-      return relation.relation.exec(modelInstances, this.options, relation.callback)
+      return this._processRelation(modelInstances, name)
     }))
+
     return modelInstances
   }
 
@@ -106,26 +162,14 @@ export class ModelQueryBuilder extends Chainable implements ModelQueryBuilderCon
   /**
    * Define a relationship to be preloaded
    */
-  public preload (
-    relationName: string,
-    callback?: (builder: ModelQueryBuilderContract<any>) => void,
-  ): this {
-    const relations = relationName.split('.')
-    const primary = relations.shift()!
-    const relation = this.model.$getRelation(primary)
-
-    /**
-     * Undefined relationship
-     */
-    if (!relation) {
-      throw new Exception(`${primary} is not defined as a relationship on ${this.model.name} model`)
-    }
+  public preload (relationName: string, userCallback?: PreloadCallback): this {
+    const { primary, relation, children, callback } = this._parseRelationName(relationName, userCallback)
 
-    const payload = this._preloads[primary] || { relation }
-    if (!relations.length) {
-      payload.callback = callback
+    const payload = this._preloads[primary] || { relation, children: [] }
+    if (children) {
+      payload.children.push(children)
     } else {
-      payload.relation.preload(relations.join('.'), callback)
+      payload.callback = callback!
     }
 
     this._preloads[primary] = payload

src/Orm/Relations/HasOne.ts

@@ -15,7 +15,6 @@ import {
   RelationContract,
   BaseRelationNode,
   ModelConstructorContract,
-  ModelQueryBuilderContract,
 } from '@ioc:Adonis/Lucid/Model'
 
 import { camelCase, snakeCase, uniq } from 'lodash'
@@ -62,11 +61,6 @@ export class HasOne implements RelationContract {
    */
   private _isValid: boolean = false
 
-  /**
-   * Preloads to pass to the query builder
-   */
-  private _preloads: { relationName: string, callback?: any }[] = []
-
   constructor (
     private _relationName: string,
     private _options: Partial<BaseRelationNode>,
@@ -141,17 +135,6 @@ export class HasOne implements RelationContract {
     this._isValid = true
   }
 
-  /**
-   * Sets the related model instances
-   */
-  private _setRelated (model: ModelContract, related?: ModelContract | null) {
-    if (!related) {
-      return
-    }
-
-    model.$setRelated(this._relationName as keyof typeof model, related)
-  }
-
   /**
    * Raises exception when value for the local key is missing on the model instance. This will
    * make the query fail
@@ -168,57 +151,59 @@ export class HasOne implements RelationContract {
   }
 
   /**
-   * Takes preloads that we want to pass to the related query builder
+   * Returns query for the relationship with applied constraints
    */
-  public preload (relationName: string, callback?: (builder: ModelQueryBuilderContract<any>) => void) {
-    this._preloads.push({ relationName, callback })
-    return this
+  public getQuery (parent: ModelContract, options?: ModelOptions) {
+    const value = parent[this.localKey]
+
+    return this.relatedModel()
+      .query(options)
+      .where(this.foreignAdapterKey, this._ensureValue(value))
   }
 
   /**
-   * Execute hasOne and set the relationship on model(s)
+   * Returns query for the relationship with applied constraints for
+   * eagerloading
    */
-  public async exec (
-    parentInstances: ModelContract | ModelContract[],
-    options?: ModelOptions,
-    userCallback?: (builder: ModelQueryBuilderContract<any>) => void,
-  ) {
-    const query = this.relatedModel().query(options)
+  public getEagerQuery (parents: ModelContract[], options?: ModelOptions) {
+    const values = uniq(parents.map((parentInstance) => {
+      return this._ensureValue(parentInstance[this.localKey])
+    }))
 
-    /**
-     * Pass preloads to the query builder
-     */
-    this._preloads.forEach(({ relationName, callback }) => query.preload(relationName, callback))
+    return this.relatedModel()
+      .query(options)
+      .whereIn(this.foreignAdapterKey, values)
+  }
 
-    if (typeof (userCallback) === 'function') {
-      userCallback(query)
+  /**
+   * Sets the related model instance
+   */
+  public setRelated (model: ModelContract, related?: ModelContract | null) {
+    if (!related) {
+      return
     }
 
-    if (Array.isArray(parentInstances)) {
-      const values = uniq(parentInstances.map((parentInstance) => {
-        return this._ensureValue(parentInstance[this.localKey])
-      }))
-      const result = await query.whereIn(this.foreignAdapterKey, values).exec()
-
-      /**
-       * Instead of looping over the model instances, we loop over the related model instances, since
-       * it can improve performance in some case. For example:
-       *
-       * - There are 10 parentInstances and we all of them to have one related instance, in
-       *   this case we run 10 iterations.
-       * - There are 10 parentInstances and 8 of them have related instance, in this case we run 8
-       *   iterations vs 10.
-       */
-      result.forEach((one) => {
-        const related = parentInstances.find((model) => model[this.localKey] === one[this.foreignKey])
-        if (related) {
-          this._setRelated(related, one)
-        }
-      })
-    } else {
-      const value = parentInstances[this.localKey]
-      const result = await query.where(this.foreignAdapterKey, this._ensureValue(value)).first()
-      this._setRelated(parentInstances, result)
-    }
+    model.$setRelated(this._relationName as keyof typeof model, related)
+  }
+
+  /**
+   * Set many related instances
+   */
+  public setRelatedMany (models: ModelContract[], related: ModelContract[]) {
+    /**
+     * Instead of looping over the model instances, we loop over the related model instances, since
+     * it can improve performance in some case. For example:
+     *
+     * - There are 10 parentInstances and we all of them to have one related instance, in
+     *   this case we run 10 iterations.
+     * - There are 10 parentInstances and 8 of them have related instance, in this case we run 8
+     *   iterations vs 10.
+     */
+    related.forEach((one) => {
+      const relation = models.find((model) => model[this.localKey] === one[this.foreignKey])
+      if (relation) {
+        this.setRelated(relation, one)
+      }
+    })
   }
 }

test/model-has-one.spec.ts

@@ -435,8 +435,8 @@ test.group('Model | Has one', (group) => {
     assert.instanceOf(user!.profile!.identity, Identity)
     assert.lengthOf(Object.keys(query['_preloads']), 1)
     assert.property(query['_preloads'], 'profile')
-    assert.lengthOf(query['_preloads'].profile.relation._preloads, 1)
-    assert.equal(query['_preloads'].profile.relation._preloads[0].relationName, 'identity')
+    assert.lengthOf(query['_preloads'].profile.children, 1)
+    assert.equal(query['_preloads'].profile.children[0].relationName, 'identity')
   })
 
   test('pass main query options down the chain', async (assert) => {