Skip to content

Commit

Permalink
feat: add support for updating/deleting related rows using query builder
Browse files Browse the repository at this point in the history
  • Loading branch information
thetutlage committed Oct 7, 2019
1 parent 632ea1d commit 212b5e9
Show file tree
Hide file tree
Showing 9 changed files with 647 additions and 35 deletions.
24 changes: 23 additions & 1 deletion src/Orm/Relations/Base/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,19 @@ export abstract class BaseRelationQueryBuilder
this.applyConstraints()
}

/**
* Get query sql
*/
public toSQL () {
this.applyConstraints()
return super['toSQL']()
}

/**
* Read value for a key on a model instance, in reference to the
* relationship operations
*/
protected $getRelatedValue (model: ModelContract, key: string, action = 'preload') {
protected $getRelatedValue (model: ModelContract, key: string, action = this.$queryAction()) {
return getValue(model, key, this._baseRelation, action)
}

Expand Down Expand Up @@ -116,6 +124,20 @@ export abstract class BaseRelationQueryBuilder
}
}

/**
* Returns the query action
*/
protected $queryAction (): string {
let action = this.$knexBuilder['_method']
if (action === 'select') {
action = 'preload'
} else if (action === 'del') {
action = 'delete'
}

return action
}

public abstract async save (model: ModelContract, wrapInTransaction?: boolean): Promise<void>
public abstract async saveMany (model: ModelContract[], wrapInTransaction?: boolean): Promise<void>
}
54 changes: 38 additions & 16 deletions src/Orm/Relations/HasManyThrough/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,29 @@ export class HasManyThroughQueryBuilder
})
}

/**
* Applies constraints on the query to limit to the parent row(s).
*/
private _applyParentConstraints (builder: HasManyThroughQueryBuilder) {
const throughTable = this._relation.throughModel().$table

/**
* Constraint for multiple parents
*/
if (Array.isArray(this._parent)) {
const values = unique(this._parent.map((parentInstance) => {
return this.$getRelatedValue(parentInstance, this._relation.localKey)
}))
return builder.whereIn(`${throughTable}.${this._relation.foreignAdapterKey}`, values)
}

/**
* Constraint for one parent
*/
const value = this.$getRelatedValue(this._parent, this._relation.localKey)
return builder.where(`${throughTable}.${this._relation.foreignAdapterKey}`, value)
}

/**
* Applies constraints for `select`, `update` and `delete` queries. The
* inserts are not allowed directly and one must use `save` method
Expand All @@ -55,6 +78,18 @@ export class HasManyThroughQueryBuilder
const throughTable = this._relation.throughModel().$table
const relatedTable = this._relation.relatedModel().$table

/**
* When updating or deleting the through rows, we run a whereIn
* subquery to limit to the parent rows.
*/
if (['delete', 'update'].includes(this.$queryAction())) {
this.whereIn(`${relatedTable}.${this._relation.throughForeignAdapterKey}`, (builder) => {
builder.from(throughTable)
this._applyParentConstraints(builder)
})
return this
}

/**
* Select * from related model and through foreign adapter key
*/
Expand All @@ -72,28 +107,15 @@ export class HasManyThroughQueryBuilder
`${relatedTable}.${this._relation.throughForeignAdapterKey}`,
)

/**
* Constraint for multiple parents
*/
if (Array.isArray(this._parent)) {
const values = unique(this._parent.map((parentInstance) => {
return this.$getRelatedValue(parentInstance, this._relation.localKey)
}))
return this.whereIn(`${throughTable}.${this._relation.foreignAdapterKey}`, values)
}

/**
* Constraint for one parent
*/
const value = this.$getRelatedValue(this._parent, this._relation.localKey)
return this.where(`${throughTable}.${this._relation.foreignAdapterKey}`, value)
this._applyParentConstraints(this)
return this
}

public async save () {
throw new Exception(`Has many through doesn\'t support saving relations`)
}

public async saveMany () {
throw new Exception(`Has many through doesn\'t support saving relations`)
return this.save()
}
}
57 changes: 42 additions & 15 deletions src/Orm/Relations/ManyToMany/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,29 @@ export class ManyToManyQueryBuilder
return this
}

/**
* Adds where or where clause on the query builder based upon the
* number of parent records passed to the query builder.
*/
private _addParentConstraint (builder: ManyToManyQueryBuilder) {
/**
* Constraint for multiple parents
*/
if (Array.isArray(this._parent)) {
const values = unique(this._parent.map((parentInstance) => {
return this.$getRelatedValue(parentInstance, this._relation.localKey)
}))
builder.whereInPivot(this._relation.pivotForeignKey, values)
return
}

/**
* Constraint for one parent
*/
const value = this.$getRelatedValue(this._parent, this._relation.localKey)
builder.wherePivot(this._relation.pivotForeignKey, value)
}

/**
* Applies constraints for `select`, `update` and `delete` queries. The
* inserts are not allowed directly and one must use `save` method
Expand All @@ -221,6 +244,23 @@ export class ManyToManyQueryBuilder

this.$appliedConstraints = true

/**
* We do not allow deleting/updating the related rows via `relationship.delete`.
*
* For example:
* A `user` has many to many `roles`, so issuing a delete query using the
* user instance cannot delete the `roles` from the `roles` table, but
* instead it only deletes the `user roles` from the pivot table.
*
* In short user doesn't own the role directly, it owns a relationship with
* the role and hence it can only remove the relation.
*/
if (['delete', 'update'].includes(this.$queryAction())) {
this.from(this._relation.pivotTable)
this._addParentConstraint(this)
return this
}

/**
* Select * from related model
*/
Expand All @@ -245,21 +285,8 @@ export class ManyToManyQueryBuilder
`${this._relation.pivotTable}.${this._relation.pivotRelatedForeignKey}`,
)

/**
* Constraint for multiple parents
*/
if (Array.isArray(this._parent)) {
const values = unique(this._parent.map((parentInstance) => {
return this.$getRelatedValue(parentInstance, this._relation.localKey)
}))
return this.whereInPivot(this._relation.pivotForeignKey, values)
}

/**
* Constraint for one parent
*/
const value = this.$getRelatedValue(this._parent, this._relation.localKey)
return this.wherePivot(this._relation.pivotForeignKey, value)
this._addParentConstraint(this)
return this
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function getValue (
) {
const value = model[key]

if (value === undefined) {
if (value === undefined || value === null) {
throw new Exception(
`Cannot ${action} ${relation.relationName}, value of ${relation.model.name}.${key} is undefined`,
500,
Expand Down
129 changes: 127 additions & 2 deletions test/orm/model-belongs-to.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ test.group('Model | BelongsTo | fetch related', (group) => {
})
})

test.group('Model | HasOne | persist', (group) => {
test.group('Model | BelongsTo | persist', (group) => {
group.before(async () => {
db = getDb()
BaseModel = getBaseModel(ormAdapter(db))
Expand Down Expand Up @@ -1232,7 +1232,7 @@ test.group('Model | HasOne | persist', (group) => {
})
})

test.group('Model | HasOne | dissociate', (group) => {
test.group('Model | BelongsTo | dissociate', (group) => {
group.before(async () => {
db = getDb()
BaseModel = getBaseModel(ormAdapter(db))
Expand Down Expand Up @@ -1286,3 +1286,128 @@ test.group('Model | HasOne | dissociate', (group) => {
assert.isNull(profile.userId)
})
})

test.group('Model | BelongsTo | bulk operation', (group) => {
group.before(async () => {
db = getDb()
BaseModel = getBaseModel(ormAdapter(db))
await setup()
})

group.after(async () => {
await cleanup()
await db.manager.closeAll()
})

group.afterEach(async () => {
await resetTables()
})

test('generate correct sql for deleting related rows', async (assert) => {
class User extends BaseModel {
@column({ primary: true })
public id: number

@column()
public username: string
}

class Profile extends BaseModel {
@column({ primary: true })
public id: number

@column()
public userId: number

@column()
public displayName: string

@belongsTo(() => User)
public user: User
}

await db.table('profiles').insert({ display_name: 'Hvirk', user_id: 1 })

const profile = await Profile.find(1)
const { sql, bindings } = profile!.related('user').del().toSQL()

const { sql: knexSql, bindings: knexBindings } = db.connection()
.getWriteClient()
.from('users')
.where('id', 1)
.del()
.toSQL()

assert.equal(sql, knexSql)
assert.deepEqual(bindings, knexBindings)
})

test('raise exception when FK is null', async (assert) => {
class User extends BaseModel {
@column({ primary: true })
public id: number

@column()
public username: string
}

class Profile extends BaseModel {
@column({ primary: true })
public id: number

@column()
public userId: number

@column()
public displayName: string

@belongsTo(() => User)
public user: User
}

await db.table('profiles').insert({ display_name: 'Hvirk' })

const profile = await Profile.find(1)
const fn = () => profile!.related('user').del().toSQL()
assert.throw(fn, 'Cannot delete user, value of Profile.userId is undefined')
})

test('generate correct sql for updating related rows', async (assert) => {
class User extends BaseModel {
@column({ primary: true })
public id: number

@column()
public username: string
}

class Profile extends BaseModel {
@column({ primary: true })
public id: number

@column()
public userId: number

@column()
public displayName: string

@belongsTo(() => User)
public user: User
}

await db.table('profiles').insert({ display_name: 'Hvirk', user_id: 1 })

const profile = await Profile.find(1)
const { sql, bindings } = profile!.related('user').update({ username: 'virk' }).toSQL()

const { sql: knexSql, bindings: knexBindings } = db.connection()
.getWriteClient()
.from('users')
.where('id', 1)
.update({ username: 'virk' })
.toSQL()

assert.equal(sql, knexSql)
assert.deepEqual(bindings, knexBindings)
})
})

0 comments on commit 212b5e9

Please sign in to comment.