Skip to content

Commit

Permalink
fix: lazy boot relationships for circular dependencies to work fine
Browse files Browse the repository at this point in the history
  • Loading branch information
thetutlage committed Oct 2, 2019
1 parent 4e27cdf commit 0e2116d
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 99 deletions.
4 changes: 3 additions & 1 deletion adonis-typings/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ declare module '@ioc:Adonis/Lucid/Model' {
column?: ThroughRelationDecoratorNode,
) => DecoratorFn

export type AvailableRelations = 'hasOne' | 'hasMany' | 'belongsTo'
export type AvailableRelations = 'hasOne' | 'hasMany' | 'belongsTo' | 'manyToMany'

/**
* Callback accepted by the preload method
Expand All @@ -137,6 +137,8 @@ declare module '@ioc:Adonis/Lucid/Model' {
export interface RelationContract {
type: AvailableRelations
serializeAs: string
booted: boolean
boot (): void
relatedModel (): ModelConstructorContract
getQuery (model: ModelContract, client: QueryClientContract): ModelExecuteableQueryBuilder
getEagerQuery (models: ModelContract[], client: QueryClientContract): ModelExecuteableQueryBuilder
Expand Down
2 changes: 1 addition & 1 deletion src/Orm/BaseModel/proxyHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const DEFAULTS: {
hasMany: Object.freeze([]),
belongsTo: null,
// hasOneThrough: null,
// manyToMany: Object.freeze([]),
manyToMany: Object.freeze([]),
// hasManyThrough: Object.freeze([]),
}

Expand Down
1 change: 1 addition & 0 deletions src/Orm/QueryBuilder/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export class ModelQueryBuilder extends Chainable implements ModelQueryBuilderCon
*/
private async _processRelation (models: ModelContract[], name: string) {
const relation = this._preloads[name]
relation.relation.boot()
const query = relation.relation.getEagerQuery(models, this.client)

/**
Expand Down
39 changes: 19 additions & 20 deletions src/Orm/Relations/BelongsTo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,18 @@ export class BelongsTo implements RelationContract {
public relatedModel = this._options.relatedModel!

/**
* Local key to use for constructing the relationship
* Local key to use for constructing the relationship. This is the primary
* key on the related model
*/
public localKey: string

/**
* Adapter local key
* Adapter local key.
*/
public localAdapterKey: string

/**
* Foreign key referenced by the related model
* Foreign key is the on the current model.
*/
public foreignKey: string

Expand All @@ -60,22 +61,21 @@ export class BelongsTo implements RelationContract {
/**
* A flag to know if model keys valid for executing database queries or not
*/
private _isValid: boolean = false
public booted: boolean = false

constructor (
private _relationName: string,
private _options: BaseRelationNode,
private _model: ModelConstructorContract,
) {
this._validateOptions()
this._computeKeys()
this._ensureRelatedModel()
}

/**
* Ensure that related model is defined, otherwise raise an exception, since
* a relationship cannot work with a single model.
*/
private _validateOptions () {
private _ensureRelatedModel () {
if (!this._options.relatedModel) {
throw new Exception(
'Related model reference is required to construct the relationship',
Expand All @@ -88,8 +88,8 @@ export class BelongsTo implements RelationContract {
/**
* Compute keys
*/
private _computeKeys () {
if (this._isValid) {
public boot () {
if (this.booted) {
return
}

Expand All @@ -108,6 +108,7 @@ export class BelongsTo implements RelationContract {
*/
this.localAdapterKey = this.relatedModel().$getColumn(this.localKey)!.castAs
this.foreignAdapterKey = this._model.$getColumn(this.foreignKey)!.castAs
this.booted = true
}

/**
Expand All @@ -134,15 +135,13 @@ export class BelongsTo implements RelationContract {
'E_MISSING_RELATED_FOREIGN_KEY',
)
}

this._isValid = true
}

/**
* Raises exception when value for the foreign key is missing on the model instance. This will
* make the query fail
*/
protected $ensureValue (value: any) {
private _ensureValue (value: any) {
if (value === undefined) {
throw new Exception(
`Cannot preload ${this._relationName}, value of ${this._model.name}.${this.foreignKey} is undefined`,
Expand All @@ -154,14 +153,14 @@ export class BelongsTo implements RelationContract {
}

/**
* Must be implemented by main class
* Returns eager query for a single parent model instance
*/
public getQuery (parent: ModelContract, client: QueryClientContract) {
const value = parent[this.foreignKey]

return this.relatedModel()
.query({ client })
.where(this.localAdapterKey, this.$ensureValue(value))
.where(this.localAdapterKey, this._ensureValue(value))
.limit(1)
}

Expand All @@ -171,7 +170,7 @@ export class BelongsTo implements RelationContract {
*/
public getEagerQuery (parents: ModelContract[], client: QueryClientContract) {
const values = uniq(parents.map((parentInstance) => {
return this.$ensureValue(parentInstance[this.foreignKey])
return this._ensureValue(parentInstance[this.foreignKey])
}))

return this.relatedModel()
Expand All @@ -191,13 +190,13 @@ export class BelongsTo implements RelationContract {
}

/**
* Must be implemented by parent class
* Sets the related instances on the model
*/
public setRelatedMany (models: ModelContract[], related: ModelContract[]) {
models.forEach((model) => {
const relation = related.find((one) => one[this.localKey] === model[this.foreignKey])
public setRelatedMany (parents: ModelContract[], related: ModelContract[]) {
parents.forEach((parent) => {
const relation = related.find((model) => model[this.localKey] === parent[this.foreignKey])
if (relation) {
this.setRelated(model, relation)
this.setRelated(parent, relation)
}
})
}
Expand Down
25 changes: 10 additions & 15 deletions src/Orm/Relations/HasMany.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,21 @@

/// <reference path="../../../adonis-typings/index.ts" />

import {
ModelContract,
BaseRelationNode,
ModelConstructorContract,
} from '@ioc:Adonis/Lucid/Model'

import { QueryClientContract } from '@ioc:Adonis/Lucid/Database'
import { ModelContract, BaseRelationNode, ModelConstructorContract } from '@ioc:Adonis/Lucid/Model'
import { HasOneOrMany } from './HasOneOrMany'

/**
* Exposes the API to construct correct queries and set related
* models for has many relationship
*/
export class HasMany extends HasOneOrMany {
/**
* Relationship type
*/
public type = 'hasMany' as const

constructor (
relationName: string,
options: BaseRelationNode,
model: ModelConstructorContract,
) {
constructor (relationName: string, options: BaseRelationNode, model: ModelConstructorContract) {
super(relationName, options, model)
}

Expand All @@ -46,10 +41,10 @@ export class HasMany extends HasOneOrMany {
/**
* Set many related instances
*/
public setRelatedMany (models: ModelContract[], related: ModelContract[]) {
models.forEach((one) => {
const relation = related.filter((model) => model[this.foreignKey] === one[this.localKey])
this.setRelated(one, relation)
public setRelatedMany (parents: ModelContract[], related: ModelContract[]) {
parents.forEach((parent) => {
const relation = related.filter((model) => model[this.foreignKey] === parent[this.localKey])
this.setRelated(parent, relation)
})
}
}
27 changes: 11 additions & 16 deletions src/Orm/Relations/HasOne.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,22 @@

/// <reference path="../../../adonis-typings/index.ts" />

import {
ModelContract,
BaseRelationNode,
ModelConstructorContract,
} from '@ioc:Adonis/Lucid/Model'

import { QueryClientContract } from '@ioc:Adonis/Lucid/Database'
import { ModelContract, BaseRelationNode, ModelConstructorContract } from '@ioc:Adonis/Lucid/Model'

import { HasOneOrMany } from './HasOneOrMany'

/**
* Exposes the API to construct correct queries and set related
* models for has one relationship
*/
export class HasOne extends HasOneOrMany {
/**
* Relationship type
*/
public type = 'hasOne' as const

constructor (
relationName: string,
options: BaseRelationNode,
model: ModelConstructorContract,
) {
constructor (relationName: string, options: BaseRelationNode, model: ModelConstructorContract) {
super(relationName, options, model)
}

Expand All @@ -48,7 +43,7 @@ export class HasOne extends HasOneOrMany {
/**
* Set many related instances
*/
public setRelatedMany (models: ModelContract[], related: ModelContract[]) {
public setRelatedMany (parents: 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:
Expand All @@ -58,10 +53,10 @@ export class HasOne extends HasOneOrMany {
* - 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)
related.forEach((relation) => {
const parent = parents.find((model) => model[this.localKey] === relation[this.foreignKey])
if (parent) {
this.setRelated(parent, relation)
}
})
}
Expand Down
Loading

0 comments on commit 0e2116d

Please sign in to comment.