Skip to content

Commit

Permalink
fix: set belongsTo and hasOne relationship property to null or model …
Browse files Browse the repository at this point in the history
…instance

When the related row is missing, then must set the relationship property
value to explicit null
  • Loading branch information
thetutlage committed May 2, 2021
1 parent df0ec20 commit 3f4973e
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 60 deletions.
6 changes: 3 additions & 3 deletions adonis-typings/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -551,9 +551,9 @@ declare module '@ioc:Adonis/Lucid/Model' {
* advanced use cases
*/
$hasRelated(key: string): boolean
$setRelated(key: string, result: OneOrMany<LucidRow>): void
$pushRelated(key: string, result: OneOrMany<LucidRow>): void
$getRelated(key: string, defaultValue?: any): OneOrMany<LucidRow> | undefined
$setRelated(key: string, result: OneOrMany<LucidRow> | null): void
$pushRelated(key: string, result: OneOrMany<LucidRow> | null): void
$getRelated(key: string, defaultValue?: any): OneOrMany<LucidRow> | undefined | null

/**
* Consume the adapter result and hydrate the model
Expand Down
2 changes: 1 addition & 1 deletion src/Orm/Preloader/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export class Preloader implements PreloaderContract<LucidRow> {
* hasOne and belongsTo will always return an array of a single row (if done right)
*/
if (relation.type === 'hasOne' || relation.type === 'belongsTo') {
relation.setRelated(parent, result[0])
relation.setRelated(parent, result[0] || null)
return
}

Expand Down
14 changes: 3 additions & 11 deletions src/Orm/Relations/BelongsTo/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class BelongsTo implements BelongsToRelationContract<LucidModel, LucidMod
) {}

/**
* Returns a boolean saving related row belongs to the parent
* Returns a boolean telling if the related row belongs to the parent
* row or not.
*/
private isRelatedRow(parent: LucidRow, related: LucidRow) {
Expand Down Expand Up @@ -141,14 +141,10 @@ export class BelongsTo implements BelongsToRelationContract<LucidModel, LucidMod
*/
public setRelated(parent: LucidRow, related: LucidRow | null): void {
ensureRelationIsBooted(this)
if (!related) {
if (related === undefined) {
return
}

if (!this.isRelatedRow(parent, related)) {
throw new Error('malformed setRelated call')
}

parent.$setRelated(this.relationName, related)
}

Expand All @@ -157,14 +153,10 @@ export class BelongsTo implements BelongsToRelationContract<LucidModel, LucidMod
*/
public pushRelated(parent: LucidRow, related: LucidRow | null): void {
ensureRelationIsBooted(this)
if (!related) {
if (related === undefined) {
return
}

if (!this.isRelatedRow(parent, related)) {
throw new Error('malformed pushRelated call')
}

parent.$setRelated(this.relationName, related)
}

Expand Down
20 changes: 0 additions & 20 deletions src/Orm/Relations/HasMany/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,6 @@ export class HasMany implements HasManyRelationContract<LucidModel, LucidModel>
*/
public setRelated(parent: LucidRow, related: LucidRow[]): void {
ensureRelationIsBooted(this)

related.forEach((relatedRow) => {
if (!this.isRelatedRow(parent, relatedRow)) {
throw new Error('malformed setRelated call')
}
})

parent.$setRelated(this.relationName, related)
}

Expand All @@ -155,19 +148,6 @@ export class HasMany implements HasManyRelationContract<LucidModel, LucidModel>
*/
public pushRelated(parent: LucidRow, related: LucidRow | LucidRow[]): void {
ensureRelationIsBooted(this)

if (Array.isArray(related)) {
related.forEach((relatedRow) => {
if (!this.isRelatedRow(parent, relatedRow)) {
throw new Error('malformed pushRelated call')
}
})
} else {
if (!this.isRelatedRow(parent, related)) {
throw new Error('malformed pushRelated call')
}
}

parent.$pushRelated(this.relationName, related)
}

Expand Down
18 changes: 6 additions & 12 deletions src/Orm/Relations/HasOne/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export class HasOne implements HasOneRelationContract<LucidModel, LucidModel> {
*/
public setRelated(parent: LucidRow, related: LucidRow | null): void {
ensureRelationIsBooted(this)
if (!related) {
if (related === undefined) {
return
}

Expand All @@ -135,7 +135,7 @@ export class HasOne implements HasOneRelationContract<LucidModel, LucidModel> {
public pushRelated(parent: LucidRow, related: LucidRow | null): void {
ensureRelationIsBooted(this)

if (!related) {
if (related === undefined) {
return
}

Expand All @@ -149,19 +149,13 @@ export class HasOne implements HasOneRelationContract<LucidModel, LucidModel> {
public setRelatedForMany(parent: LucidRow[], related: LucidRow[]): void {
ensureRelationIsBooted(this)

/**
* The related model will always be equal or less than the parent
* models. So we loop over them to lower down the number of
* iterations.
*/
related.forEach((relatedModel) => {
const match = parent.find((parentModel) => {
parent.forEach((parentModel) => {
const match = related.find((relatedModel) => {
const value = parentModel[this.localKey]
return value !== undefined && value === relatedModel[this.foreignKey]
})
if (match) {
this.setRelated(match, relatedModel)
}

this.setRelated(parentModel, match || null)
})
}

Expand Down
63 changes: 56 additions & 7 deletions test/orm/model-belongs-to.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ test.group('Model | BelongsTo | Set Relations', (group) => {

assert.deepEqual(profile.user, user)
assert.deepEqual(profile1.user, user1)
assert.isUndefined(profile2.user)
assert.isNull(profile2.user)
})
})

Expand Down Expand Up @@ -745,6 +745,30 @@ test.group('Model | BelongsTo | preload', (group) => {
assert.equal(profiles[0].user.id, profiles[0].userId)
})

test('set property value to null when no preload rows were found', async (assert) => {
class User extends BaseModel {
@column({ isPrimary: true })
public id: number
}

class Profile extends BaseModel {
@column()
public userId: number

@belongsTo(() => User)
public user: BelongsTo<typeof User>
}

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

Profile.boot()

const profiles = await Profile.query().preload('user')
assert.lengthOf(profiles, 1)

assert.isNull(profiles[0].user)
})

test('preload relationship for many rows', async (assert) => {
class User extends BaseModel {
@column({ isPrimary: true })
Expand Down Expand Up @@ -817,8 +841,8 @@ test.group('Model | BelongsTo | preload', (group) => {
)

assert.lengthOf(profiles, 2)
assert.isUndefined(profiles[0].user)
assert.isUndefined(profiles[1].user)
assert.isNull(profiles[0].user)
assert.isNull(profiles[1].user)
})

test('cherry pick columns during preload', async (assert) => {
Expand Down Expand Up @@ -1289,7 +1313,32 @@ test.group('Model | BelongsTo | preload', (group) => {
const profiles = await Profile.query().preload('user')
assert.lengthOf(profiles, 1)

assert.isUndefined(profiles[0].user)
assert.isNull(profiles[0].user)
})

test('work fine during lazy load when foreign key is null', async (assert) => {
class User extends BaseModel {
@column({ isPrimary: true })
public id: number
}

class Profile extends BaseModel {
@column()
public userId: number

@belongsTo(() => User)
public user: BelongsTo<typeof User>
}

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

Profile.boot()

const profiles = await Profile.query()
assert.lengthOf(profiles, 1)
await profiles[0].load('user')

assert.isNull(profiles[0].user)
})

test('do not run preload query when parent rows are empty', async (assert) => {
Expand Down Expand Up @@ -1803,7 +1852,7 @@ test.group('Model | BelongsTo | scopes', (group) => {
.first()

const profileWithoutScope = await Profile.query().preload('user').first()
assert.isUndefined(profile?.user)
assert.isNull(profile?.user)
assert.instanceOf(profileWithoutScope?.user, User)
})

Expand Down Expand Up @@ -1899,7 +1948,7 @@ test.group('Model | BelongsTo | onQuery', (group) => {
await db.insertQuery().table('profiles').insert({ display_name: 'Hvirk', user_id: 1 })

const profile = await Profile.query().preload('user').first()
assert.isUndefined(profile?.user)
assert.isNull(profile?.user)
})

test('do not run onQuery hook on subqueries', async (assert) => {
Expand Down Expand Up @@ -1940,7 +1989,7 @@ test.group('Model | BelongsTo | onQuery', (group) => {
})
.first()

assert.isUndefined(profile?.user)
assert.isNull(profile?.user)
})

test('invoke onQuery method on related query builder', async (assert) => {
Expand Down
28 changes: 28 additions & 0 deletions test/orm/model-has-many.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,34 @@ test.group('Model | HasMany | preload', (group) => {
assert.equal(users[1].posts[0].userId, users[1].id)
})

test('set relationship property value to empty array when no related rows have been found', async (assert) => {
class Post extends BaseModel {
@column()
public userId: number
}

class User extends BaseModel {
@column({ isPrimary: true })
public id: number

@hasMany(() => Post)
public posts: HasMany<typeof Post>
}

await db
.insertQuery()
.table('users')
.insert([{ username: 'virk' }, { username: 'nikk' }])

User.boot()

const users = await User.query().preload('posts')
assert.lengthOf(users, 2)

assert.lengthOf(users[0].posts, 0)
assert.lengthOf(users[1].posts, 0)
})

test('preload relationship for many rows', async (assert) => {
class Post extends BaseModel {
@column({ isPrimary: true })
Expand Down
46 changes: 40 additions & 6 deletions test/orm/model-has-one.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ test.group('Model | HasOne | Set Relations', (group) => {
User.$getRelation('profile')!.setRelatedForMany([user, user1, user2], [profile, profile1])
assert.deepEqual(user.profile, profile)
assert.deepEqual(user1.profile, profile1)
assert.isUndefined(user2.profile)
assert.isNull(user2.profile)
})
})

Expand Down Expand Up @@ -796,6 +796,40 @@ test.group('Model | HasOne | preload', (group) => {
assert.equal(users[1].profile.userId, users[1].id)
})

test('set relationship property value to null when no related rows were found', async (assert) => {
class Profile extends BaseModel {
@column({ isPrimary: true })
public id: number

@column()
public userId: number

@column()
public displayName: string
}

class User extends BaseModel {
@column({ isPrimary: true })
public id: number

@hasOne(() => Profile)
public profile: HasOne<typeof Profile>
}

await db
.insertQuery()
.table('users')
.insert([{ username: 'virk' }, { username: 'nikk' }])

User.boot()

const users = await User.query().preload('profile')
assert.lengthOf(users, 2)

assert.isNull(users[0].profile)
assert.isNull(users[1].profile)
})

test('preload nested relations', async (assert) => {
class Identity extends BaseModel {
@column({ isPrimary: true })
Expand Down Expand Up @@ -972,8 +1006,8 @@ test.group('Model | HasOne | preload', (group) => {
)
assert.lengthOf(users, 2)

assert.isUndefined(users[0].profile)
assert.isUndefined(users[1].profile)
assert.isNull(users[0].profile)
assert.isNull(users[1].profile)
})

test('cherry pick columns during preload', async (assert) => {
Expand Down Expand Up @@ -2405,7 +2439,7 @@ test.group('Model | HasOne | scopes', (group) => {
.firstOrFail()
const userWithScopes = await User.query().preload('profile').firstOrFail()

assert.isUndefined(user.profile)
assert.isNull(user.profile)
assert.instanceOf(userWithScopes.profile, Profile)
})

Expand Down Expand Up @@ -2504,7 +2538,7 @@ test.group('Model | HasOne | onQuery', (group) => {
.multiInsert([{ user_id: userId, display_name: 'virk', type: 'github' }])

const user = await User.query().preload('profile').firstOrFail()
assert.isUndefined(user.profile)
assert.isNull(user.profile)
})

test('do not invoke onQuery method on preloading subqueries', async (assert) => {
Expand Down Expand Up @@ -2545,7 +2579,7 @@ test.group('Model | HasOne | onQuery', (group) => {
const user = await User.query()
.preload('profile', (query) => query.where(() => {}))
.firstOrFail()
assert.isUndefined(user.profile)
assert.isNull(user.profile)
})

test('invoke onQuery method on related query builder', async (assert) => {
Expand Down

0 comments on commit 3f4973e

Please sign in to comment.