Skip to content

Commit

Permalink
fix: model.refresh to use the existing transaction
Browse files Browse the repository at this point in the history
FIXES #650
  • Loading branch information
thetutlage committed Feb 19, 2021
1 parent 36f3717 commit 7a37e08
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 31 deletions.
22 changes: 20 additions & 2 deletions adonis-typings/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -509,9 +509,9 @@ declare module '@ioc:Adonis/Lucid/Model' {
client: QueryClientContract
): ReturnType<QueryClientContract['insertQuery']>
$getQueryFor(
action: 'update' | 'delete',
action: 'update' | 'delete' | 'refresh',
client: QueryClientContract
): ReturnType<QueryClientContract['query']>
): ModelQueryBuilderContract<LucidModel>

/**
* Read/write attributes. Following methods are intentionally loosely typed,
Expand Down Expand Up @@ -540,10 +540,22 @@ declare module '@ioc:Adonis/Lucid/Model' {

fill(value: Partial<ModelAttributes<this>>, allowNonExtraProperties?: boolean): this
merge(value: Partial<ModelAttributes<this>>, allowNonExtraProperties?: boolean): this

/**
* Actions to perform on the instance
*/
save(): Promise<this>
delete(): Promise<void>
refresh(): Promise<this>

/**
* Load relationships onto the instance
*/
load: LucidRowPreload<this>

/**
* Alias for "load"
*/
preload: LucidRowPreload<this>

/**
Expand Down Expand Up @@ -988,6 +1000,12 @@ declare module '@ioc:Adonis/Lucid/Model' {
*/
delete(instance: LucidRow): Promise<void>

/**
* Refresh model instance to reflect new values
* from the database
*/
refresh(instance: LucidRow): Promise<void>

/**
* Perform insert
*/
Expand Down
40 changes: 33 additions & 7 deletions src/Orm/Adapter/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

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

import { Exception } from '@poppinss/utils'
import { DatabaseContract } from '@ioc:Adonis/Lucid/Database'
import { LucidRow, LucidModel, AdapterContract, ModelAdapterOptions } from '@ioc:Adonis/Lucid/Model'

Expand All @@ -19,6 +20,10 @@ import { LucidRow, LucidModel, AdapterContract, ModelAdapterOptions } from '@ioc
export class Adapter implements AdapterContract {
constructor(private db: DatabaseContract) {}

private getPrimaryKeyColumnName(Model: LucidModel) {
return Model.$keys.attributesToColumns.get(Model.primaryKey, Model.primaryKey)
}

/**
* Returns the query client based upon the model instance
*/
Expand Down Expand Up @@ -54,16 +59,13 @@ export class Adapter implements AdapterContract {
* Perform insert query on a given model instance
*/
public async insert(instance: LucidRow, attributes: any) {
const modelConstructor = (instance.constructor as unknown) as LucidModel
const query = instance.$getQueryFor('insert', this.modelClient(instance))

const primaryKeyColumnName = modelConstructor.$keys.attributesToColumns.get(
modelConstructor.primaryKey,
modelConstructor.primaryKey
)
const Model = instance.constructor as LucidModel
const primaryKeyColumnName = this.getPrimaryKeyColumnName(Model)

const result = await query.insert(attributes).reporterData({ model: modelConstructor.name })
if (!modelConstructor.selfAssignPrimaryKey) {
const result = await query.insert(attributes).reporterData({ model: Model.name })
if (!Model.selfAssignPrimaryKey) {
instance.$consumeAdapterResult({ [primaryKeyColumnName]: result[0] })
}
}
Expand All @@ -81,4 +83,28 @@ export class Adapter implements AdapterContract {
public async delete(instance: LucidRow) {
await instance.$getQueryFor('delete', this.modelClient(instance)).del()
}

/**
* Refresh the model instance attributes
*/
public async refresh(instance: LucidRow) {
const Model = instance.constructor as LucidModel
const primaryKeyColumnName = this.getPrimaryKeyColumnName(Model)

const freshModelInstance = await instance
.$getQueryFor('refresh', this.modelClient(instance))
.first()

if (!freshModelInstance) {
throw new Exception(
[
'"Model.refresh" failed. ',
`Unable to lookup "${Model.table}" table where "${primaryKeyColumnName}" = ${instance.$primaryKeyValue}`,
].join('')
)
}

instance.fill(freshModelInstance.$attributes)
instance.$hydrateOriginals()
}
}
28 changes: 6 additions & 22 deletions src/Orm/BaseModel/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1851,7 +1851,10 @@ export class BaseModel implements LucidRow {
* Since the query builder for these actions are not exposed to
* the end user, this method gives a way to compose queries.
*/
public $getQueryFor(action: 'insert' | 'update' | 'delete', client: QueryClientContract): any {
public $getQueryFor(
action: 'insert' | 'update' | 'delete' | 'refresh',
client: QueryClientContract
): any {
const modelConstructor = this.constructor as typeof BaseModel
const primaryKeyColumn = modelConstructor.$keys.attributesToColumns.get(
modelConstructor.primaryKey,
Expand Down Expand Up @@ -1890,12 +1893,6 @@ export class BaseModel implements LucidRow {
*/
public async refresh() {
this.ensureIsntDeleted()
const modelConstructor = this.constructor as typeof BaseModel
const { table } = modelConstructor
const primaryKeyColumn = modelConstructor.$keys.attributesToColumns.get(
modelConstructor.primaryKey,
modelConstructor.primaryKey
)

/**
* Noop when model instance is not persisted
Expand All @@ -1904,22 +1901,9 @@ export class BaseModel implements LucidRow {
return this
}

/**
* This will occur, when some other part of the application removes
* the row
*/
const freshModelInstance = await modelConstructor.find(this.$primaryKeyValue)
if (!freshModelInstance) {
throw new Exception(
[
'"Model.refresh" failed. ',
`Unable to lookup "${table}" table where "${primaryKeyColumn}" = ${this.$primaryKeyValue}`,
].join('')
)
}
const Model = this.constructor as typeof BaseModel
await Model.$adapter.refresh(this)

this.fill(freshModelInstance.$attributes)
this.$hydrateOriginals()
return this
}
}
7 changes: 7 additions & 0 deletions test-helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ export class FakeAdapter implements AdapterContract {
find: null,
delete: null,
findAll: null,
refresh: null,
}

private _invokeHandler(
Expand All @@ -465,6 +466,7 @@ export class FakeAdapter implements AdapterContract {
public on(action: 'insert', handler: (model: LucidRow, attributes: any) => void): void
public on(action: 'update', handler: (model: LucidRow, attributes: any) => void): void
public on(action: 'delete', handler: (model: LucidRow) => void): void
public on(action: 'refresh', handler: (model: LucidRow) => void): void
public on(action: 'find', handler: (model: LucidModel, options?: any) => void): void
public on(action: 'findAll', handler: (model: LucidModel, options?: any) => void): void
public on(
Expand All @@ -483,6 +485,11 @@ export class FakeAdapter implements AdapterContract {
return this._invokeHandler('insert', instance, attributes)
}

public async refresh(instance: LucidRow) {
this.operations.push({ type: 'refresh', instance })
return this._invokeHandler('refresh', instance)
}

public async delete(instance: LucidRow) {
this.operations.push({ type: 'delete', instance })
return this._invokeHandler('delete', instance)
Expand Down
45 changes: 45 additions & 0 deletions test/orm/base-model.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,51 @@ test.group('Base Model | persist', (group) => {
assert.isDefined(user.updatedAt)
})

test('refresh model instance inside a transaction', async (assert) => {
class User extends BaseModel {
@column({ isPrimary: true })
public id: number

@column()
public username: string

@column()
public createdAt: string

@column({ columnName: 'updated_at' })
public updatedAt: string
}

/**
* Create user
*/
const user = new User()
user.username = 'virk'
await user.save()

/**
* Update inside transaction
*/
const trx = await db.transaction()
user.useTransaction(trx)
user.username = 'romain'
await user.save()
assert.equal(user.username, 'romain')

/**
* Refresh inside transaction
*/
await user.refresh()
assert.equal(user.username, 'romain')

/**
* Refresh outside transaction
*/
await trx.rollback()
await user.refresh()
assert.equal(user.username, 'virk')
})

test('raise exception when attempted to refresh deleted row', async (assert) => {
assert.plan(4)

Expand Down

0 comments on commit 7a37e08

Please sign in to comment.