From 6496e708da6c4877d5666f3cdd2443d649aa318f Mon Sep 17 00:00:00 2001 From: Harminder virk Date: Sun, 6 Oct 2019 14:48:50 +0530 Subject: [PATCH] refactor: do not have defaults for relationships, let the end user define them --- src/Orm/BaseModel/index.ts | 20 ++++++++------------ src/Orm/BaseModel/proxyHandler.ts | 30 ++++-------------------------- test/orm/base-model.spec.ts | 4 ++-- test/orm/model-belongs-to.spec.ts | 2 +- test/orm/model-has-one.spec.ts | 2 +- 5 files changed, 16 insertions(+), 42 deletions(-) diff --git a/src/Orm/BaseModel/index.ts b/src/Orm/BaseModel/index.ts index c9ae5c04..ae547846 100644 --- a/src/Orm/BaseModel/index.ts +++ b/src/Orm/BaseModel/index.ts @@ -703,19 +703,15 @@ export class BaseModel implements ModelContract { /** * Returns the related model or default value when model is missing */ - public $getRelated (key: string, defaultValue: any): any { - if (this.$hasRelated(key)) { - return this.$preloaded[key as string] - } - - return defaultValue + public $getRelated (key: string): any { + return this.$preloaded[key] } /** * A boolean to know if relationship has been preloaded or not */ public $hasRelated (key: string): boolean { - return this.$preloaded[key as string] !== undefined + return this.$preloaded[key] !== undefined } /** @@ -739,13 +735,13 @@ export class BaseModel implements ModelContract { const manyRelationships = ['hasMany', 'manyToMany', 'hasManyThrough'] if (manyRelationships.includes(relation.type)) { if (!Array.isArray(models)) { - throw new Exception(` - $setRelated accepts an array of related models for ${manyRelationships.join(',')} relationships, - `) + throw new Exception( + `${Model}.${key} must be an array (${manyRelationships.join(',')} relationships)`, + ) } - this.$preloaded[key as string] = models + this.$preloaded[key] = models } else { - this.$preloaded[key as string] = models as unknown as ModelContract + this.$preloaded[key] = models as unknown as ModelContract } } diff --git a/src/Orm/BaseModel/proxyHandler.ts b/src/Orm/BaseModel/proxyHandler.ts index b94d9115..02c6029f 100644 --- a/src/Orm/BaseModel/proxyHandler.ts +++ b/src/Orm/BaseModel/proxyHandler.ts @@ -9,30 +9,7 @@ /// -import { Exception } from '@poppinss/utils' -import { ModelConstructorContract, AvailableRelations } from '@ioc:Adonis/Lucid/Model' - -/** - * Value to return when relationship is not preloaded - * with the model instance. - * - * We are re-using the defaults from a static source to avoid creating empty arrays - * everytime someone access the relationship. However, as a downside, if someone - * decides to mutate the array, that will mutate the source and hence we - * freeze the arrays. - * - * The `Object.freeze` however doesn't stop one from defining values for a specific - * index. - */ -const DEFAULTS: { - [P in AvailableRelations]: any -} = { - hasOne: null, - hasMany: Object.freeze([]), - belongsTo: null, - manyToMany: Object.freeze([]), - hasManyThrough: Object.freeze([]), -} +import { ModelConstructorContract } from '@ioc:Adonis/Lucid/Model' /** * A proxy trap to add support for custom getters and setters @@ -55,7 +32,7 @@ export const proxyHandler = { */ const relation = Model.$getRelation(key) if (relation) { - return target.$getRelated(key, DEFAULTS[relation.type]) + return target.$getRelated(key) } return Reflect.get(target, key, receiver) @@ -79,7 +56,8 @@ export const proxyHandler = { */ const relation = Model.$getRelation(key) if (relation) { - throw new Exception('Cannot set relationships locally', 500, 'E_CANNOT_DEFINE_RELATIONSHIP') + target.$setRelated(key, value) + return true } return Reflect.set(target, key, value, receiver) diff --git a/test/orm/base-model.spec.ts b/test/orm/base-model.spec.ts index b85856f1..c7994435 100644 --- a/test/orm/base-model.spec.ts +++ b/test/orm/base-model.spec.ts @@ -1114,7 +1114,7 @@ test.group('Base Model | relations', (group) => { assert.instanceOf(user.$preloaded.profile, Profile) }) - test('return null when relation is not preloaded', (assert) => { + test('return undefined when relation is not preloaded', (assert) => { class Profile extends BaseModel { @column() public username: string @@ -1136,7 +1136,7 @@ test.group('Base Model | relations', (group) => { id: 1, }) - assert.isNull(user.profile) + assert.isUndefined(user.profile) assert.deepEqual(user.$preloaded, {}) }) diff --git a/test/orm/model-belongs-to.spec.ts b/test/orm/model-belongs-to.spec.ts index 04a16bcb..11f40e80 100644 --- a/test/orm/model-belongs-to.spec.ts +++ b/test/orm/model-belongs-to.spec.ts @@ -476,7 +476,7 @@ test.group('Model | BelongsTo', (group) => { builder.whereNull('username') }).first() - assert.isNull(profile!.user) + assert.isUndefined(profile!.user) }) test('preload nested relations', async (assert) => { diff --git a/test/orm/model-has-one.spec.ts b/test/orm/model-has-one.spec.ts index 7c6c8a99..e7b534c8 100644 --- a/test/orm/model-has-one.spec.ts +++ b/test/orm/model-has-one.spec.ts @@ -446,7 +446,7 @@ test.group('Model | HasOne | preload', (group) => { builder.whereNull('display_name') }).where('username', 'virk').first() - assert.isNull(user!.profile) + assert.isUndefined(user!.profile) }) test('preload nested relations', async (assert) => {