From 71ee4c863d210d60f7fd5d949ca9c65449917d7f Mon Sep 17 00:00:00 2001 From: Harminder virk Date: Fri, 17 Jan 2020 11:46:00 +0530 Subject: [PATCH] refactor: remove $refs in favor of $columns and rename $columns to $columnDefinitions --- adonis-typings/model.ts | 52 +++++++------- adonis-typings/relations.ts | 29 ++++++-- example/index.ts | 13 ++++ src/Orm/Adapter/index.ts | 4 +- src/Orm/BaseModel/index.ts | 134 +++++++++++++++++++++--------------- test/orm/base-model.spec.ts | 26 +++++-- 6 files changed, 167 insertions(+), 91 deletions(-) create mode 100644 example/index.ts diff --git a/adonis-typings/model.ts b/adonis-typings/model.ts index fab5865f..e85411e0 100644 --- a/adonis-typings/model.ts +++ b/adonis-typings/model.ts @@ -328,17 +328,23 @@ declare module '@ioc:Adonis/Lucid/Model' { /** * A map of defined columns */ - $columns: Map + $columnsDefinitions: Map /** * A map of defined relationships */ - $relations: Map + $relationsDefinitions: Map /** * A map of computed properties */ - $computed: Map + $computedDefinitions: Map + + /** + * Columns is a property to get type information for model + * attributes. This must be declared by the end user + */ + $columns: any /** * The primary key for finding unique referencing to a @@ -372,12 +378,6 @@ declare module '@ioc:Adonis/Lucid/Model' { */ table: string - /** - * Refs are named value pair on model used mainly for autocompleting - * the query constraints - */ - $refs: { [key: string]: string } - /** * Creating model from adapter results */ @@ -443,6 +443,12 @@ declare module '@ioc:Adonis/Lucid/Model' { $resolveCastKey (key: string): string $mapKeysToCastKeys (values: ModelObject): ModelObject + /** + * Resolve keys to their model column properties + */ + $resolveColumnName (key: string): string + $mapsKeysToColumnNames (values: ModelObject): ModelObject + boot (): void /** @@ -468,7 +474,7 @@ declare module '@ioc:Adonis/Lucid/Model' { */ create ( this: T, - values: ModelObject, + values: Partial, options?: ModelAdapterOptions, ): Promise> @@ -477,7 +483,7 @@ declare module '@ioc:Adonis/Lucid/Model' { */ createMany ( this: T, - values: ModelObject[], + values: Partial[], options?: ModelAdapterOptions, ): Promise[]> @@ -514,8 +520,8 @@ declare module '@ioc:Adonis/Lucid/Model' { */ firstOrNew ( this: T, - search: any, - savePayload?: any, + search: Partial, + savePayload?: Partial, options?: ModelAdapterOptions, ): Promise> @@ -524,8 +530,8 @@ declare module '@ioc:Adonis/Lucid/Model' { */ firstOrCreate ( this: T, - search: any, - savePayload?: any, + search: Partial, + savePayload?: Partial, options?: ModelAdapterOptions, ): Promise> @@ -535,8 +541,8 @@ declare module '@ioc:Adonis/Lucid/Model' { */ fetchOrNewUpMany ( this: T, - uniqueKey: string, - payload: ModelObject[], + uniqueKey: keyof T['$columns'], + payload: Partial[], options?: ModelAdapterOptions, mergeAttributes?: boolean, ): Promise[]> @@ -547,8 +553,8 @@ declare module '@ioc:Adonis/Lucid/Model' { */ fetchOrCreateMany ( this: T, - uniqueKey: string, - payload: ModelObject[], + uniqueKey: keyof T['$columns'], + payload: Partial[], options?: ModelAdapterOptions, ): Promise[]> @@ -557,8 +563,8 @@ declare module '@ioc:Adonis/Lucid/Model' { */ updateOrCreate ( this: T, - search: any, - updatePayload: any, + search: Partial, + updatePayload: Partial, options?: ModelAdapterOptions, ): Promise> @@ -567,8 +573,8 @@ declare module '@ioc:Adonis/Lucid/Model' { */ updateOrCreateMany ( this: T, - uniqueKey: string, - payload: ModelObject[], + uniqueKey: keyof T['$columns'], + payload: Partial[], options?: ModelAdapterOptions, ): Promise[]> diff --git a/adonis-typings/relations.ts b/adonis-typings/relations.ts index 4d62e470..e873c71f 100644 --- a/adonis-typings/relations.ts +++ b/adonis-typings/relations.ts @@ -424,9 +424,20 @@ declare module '@ioc:Adonis/Lucid/Relations' { RelatedModel extends ModelConstructorContract > extends RelationBaseQueryClientContract { save (related: InstanceType): Promise - create (values: ModelObject): Promise> - firstOrCreate (search: ModelObject, savePayload?: ModelObject): Promise> - updateOrCreate (search: ModelObject, updatePayload: ModelObject): Promise> + + create ( + values: Partial, + ): Promise> + + firstOrCreate ( + search: Partial, + savePayload?: Partial, + ): Promise> + + updateOrCreate ( + search: ModelObject, + updatePayload: ModelObject, + ): Promise> } /** @@ -439,7 +450,7 @@ declare module '@ioc:Adonis/Lucid/Relations' { RelatedModel extends ModelConstructorContract > extends HasOneClientContract { saveMany (related: InstanceType[]): Promise - createMany (values: ModelObject[]): Promise[]> + createMany (values: Partial[]): Promise[]> } /** @@ -497,12 +508,18 @@ declare module '@ioc:Adonis/Lucid/Relations' { /** * Create related model instance */ - create (values: ModelObject, checkExisting?: boolean): Promise> + create ( + values: Partial, + checkExisting?: boolean, + ): Promise> /** * Create many of related model instances */ - createMany (values: ModelObject[], checkExisting?: boolean): Promise[]> + createMany ( + values: Partial[], + checkExisting?: boolean, + ): Promise[]> /** * Attach new pivot rows diff --git a/example/index.ts b/example/index.ts new file mode 100644 index 00000000..b06f9659 --- /dev/null +++ b/example/index.ts @@ -0,0 +1,13 @@ +import { BaseModel } from '@ioc:Adonis/Lucid/Orm' + +class User extends BaseModel { + public id: string + public username: string + public static $refs: Pick +} + +User.create({ id: '1' }) +User.fetchOrCreateMany('id', [{ id: '1', username: 'virk' }]) +User.create({ id: '1', username: 'virk' }) +// User.create({ id: '1', username: 22 }) +User.create({ id: '1' }) diff --git a/src/Orm/Adapter/index.ts b/src/Orm/Adapter/index.ts index 48740e17..17794e66 100644 --- a/src/Orm/Adapter/index.ts +++ b/src/Orm/Adapter/index.ts @@ -66,7 +66,9 @@ export class Adapter implements AdapterContract { const result = await query.insert(attributes) if (modelConstructor.increments) { - instance.$consumeAdapterResult({ [modelConstructor.$refs[modelConstructor.primaryKey]]: result[0] }) + instance.$consumeAdapterResult({ + [modelConstructor.$resolveColumnName(modelConstructor.primaryKey)]: result[0], + }) } } diff --git a/src/Orm/BaseModel/index.ts b/src/Orm/BaseModel/index.ts index 834d286e..82e7c76e 100644 --- a/src/Orm/BaseModel/index.ts +++ b/src/Orm/BaseModel/index.ts @@ -95,19 +95,19 @@ export class BaseModel implements ModelContract { * the `toJSON` result, else they behave the same way as any other instance * property. */ - public static $computed: Map + public static $computedDefinitions: Map /** * Columns makes it easier to define extra props on the model * and distinguish them with the attributes to be sent * over to the adapter */ - public static $columns: Map + public static $columnsDefinitions: Map /** * Registered relationships for the given model */ - public static $relations: Map + public static $relationsDefinitions: Map /** * Whether or not to rely on database to return the primaryKey @@ -122,11 +122,6 @@ export class BaseModel implements ModelContract { */ public static table: string - /** - * A key-value pair of model properties and their `castAs` keys - */ - public static $refs: { [key: string]: string } - /** * A custom connection to use for queries. The connection defined on * query builder is preferred over the model connection @@ -139,9 +134,19 @@ export class BaseModel implements ModelContract { private static hooks: Hooks /** - * Reverse of `$refs` + * A key-value pair of model properties and their `castAs` keys + */ + private static $attributesToAdapterKeys: { [key: string]: string } + + /** + * Reverse of `$attributesToAdapterKeys` + */ + private static $adapterKeysToAttributes: { [key: string]: string } + + /** + * A type only reference to the columns */ - private static $dbRefs: { [key: string]: string } + public static $columns: any = {} /** * Returns the model query instance for the given model @@ -225,23 +230,23 @@ export class BaseModel implements ModelContract { this.primaryKey = name } - this.$columns.set(name, column) - this.$refs[name] = column.castAs - this.$dbRefs[column.castAs] = name + this.$columnsDefinitions.set(name, column) + this.$attributesToAdapterKeys[name] = column.castAs + this.$adapterKeysToAttributes[column.castAs] = name } /** * Returns a boolean telling if column exists on the model */ public static $hasColumn (name: string): boolean { - return this.$columns.has(name) + return this.$columnsDefinitions.has(name) } /** * Returns the column for a given name */ public static $getColumn (name: string): ColumnOptions | undefined { - return this.$columns.get(name) + return this.$columnsDefinitions.get(name) } /** @@ -251,21 +256,21 @@ export class BaseModel implements ModelContract { const column: ComputedOptions = { serializeAs: options.serializeAs || name, } - this.$computed.set(name, column) + this.$computedDefinitions.set(name, column) } /** * Find if some property is marked as computed */ public static $hasComputed (name: string): boolean { - return this.$computed.has(name) + return this.$computedDefinitions.has(name) } /** * Get computed node */ public static $getComputed (name: string): ComputedOptions | undefined { - return this.$computed.get(name) + return this.$computedDefinitions.get(name) } /** @@ -278,19 +283,19 @@ export class BaseModel implements ModelContract { ) { switch (type) { case 'hasOne': - this.$relations.set(name, new HasOne(name, options, this)) + this.$relationsDefinitions.set(name, new HasOne(name, options, this)) break case 'hasMany': - this.$relations.set(name, new HasMany(name, options, this)) + this.$relationsDefinitions.set(name, new HasMany(name, options, this)) break case 'belongsTo': - this.$relations.set(name, new BelongsTo(name, options, this)) + this.$relationsDefinitions.set(name, new BelongsTo(name, options, this)) break case 'manyToMany': - this.$relations.set(name, new ManyToMany(name, options as ManyToManyRelationOptions, this)) + this.$relationsDefinitions.set(name, new ManyToMany(name, options as ManyToManyRelationOptions, this)) break case 'hasManyThrough': - this.$relations.set(name, new HasManyThrough(name, options as ThroughRelationOptions, this)) + this.$relationsDefinitions.set(name, new HasManyThrough(name, options as ThroughRelationOptions, this)) break default: throw new Error(`${type} is not a supported relation type`) @@ -301,14 +306,14 @@ export class BaseModel implements ModelContract { * Find if some property is marked as a relation or not */ public static $hasRelation (name: any): boolean { - return this.$relations.has(name) + return this.$relationsDefinitions.has(name) } /** * Returns relationship node for a given relation */ public static $getRelation (name: any): RelationshipsContract { - return this.$relations.get(name)! + return this.$relationsDefinitions.get(name)! } /** @@ -316,7 +321,7 @@ export class BaseModel implements ModelContract { * is returned as it is, If property doesn't exists inside refs. */ public static $resolveCastKey (key: string): string { - return this.$refs[key] || key + return this.$attributesToAdapterKeys[key] || key } /** @@ -329,6 +334,24 @@ export class BaseModel implements ModelContract { }, {}) } + /** + * Resolves the module column name for a given property. The original key + * is returned as it is, If property doesn't exists inside dbRefs. + */ + public static $resolveColumnName (key: string): string { + return this.$adapterKeysToAttributes[key] || key + } + + /** + * Maps the object keys to their model column names + */ + public static $mapsKeysToColumnNames (values: ModelObject): ModelObject { + return Object.keys(values).reduce((result, key) => { + result[this.$resolveColumnName(key)] = values[key] + return result + }, {}) + } + /** * Boot the model */ @@ -340,12 +363,13 @@ export class BaseModel implements ModelContract { this.booted = true this.primaryKey = this.primaryKey || 'id' - Object.defineProperty(this, '$refs', { value: {} }) - Object.defineProperty(this, '$dbRefs', { value: {} }) + Object.defineProperty(this, '$attributesToAdapterKeys', { value: {} }) + Object.defineProperty(this, '$adapterKeysToAttributes', { value: {} }) + Object.defineProperty(this, '$columns', { value: {} }) - Object.defineProperty(this, '$columns', { value: new Map() }) - Object.defineProperty(this, '$computed', { value: new Map() }) - Object.defineProperty(this, '$relations', { value: new Map() }) + Object.defineProperty(this, '$columnsDefinitions', { value: new Map() }) + Object.defineProperty(this, '$computedDefinitions', { value: new Map() }) + Object.defineProperty(this, '$relationsDefinitions', { value: new Map() }) Object.defineProperty(this, 'hooks', { value: new Hooks(this.$container.getResolver(undefined, 'modelHooks', 'App/Models/Hooks')), @@ -377,7 +401,7 @@ export class BaseModel implements ModelContract { */ public static async create ( this: T, - values: ModelObject, + values: Partial, options?: ModelAdapterOptions, ): Promise> { const instance = new this() @@ -396,7 +420,7 @@ export class BaseModel implements ModelContract { */ public static async createMany ( this: T, - values: ModelObject[], + values: Partial[], options?: ModelAdapterOptions, ): Promise[]> { const client = this.$adapter.modelConstructorClient(this, options) @@ -449,8 +473,8 @@ export class BaseModel implements ModelContract { */ public static async firstOrNew ( this: T, - search: any, - savePayload?: any, + search: Partial, + savePayload?: Partial, options?: ModelAdapterOptions, ) { const query = this.query(options) @@ -475,8 +499,8 @@ export class BaseModel implements ModelContract { */ public static async firstOrCreate ( this: T, - search: any, - savePayload?: any, + search: Partial, + savePayload?: Partial, options?: ModelAdapterOptions, ) { const row = await this.firstOrNew(search, savePayload, options) @@ -492,8 +516,8 @@ export class BaseModel implements ModelContract { */ public static async updateOrCreate ( this: T, - search: any, - updatedPayload: any, + search: Partial, + updatedPayload: Partial, options?: ModelAdapterOptions, ) { const row = await this.firstOrNew(search, updatedPayload, options) @@ -515,8 +539,8 @@ export class BaseModel implements ModelContract { */ public static async fetchOrNewUpMany ( this: T, - uniqueKey: string, - payload: ModelObject[], + uniqueKey: keyof T['$columns'], + payload: Partial[], options?: ModelAdapterOptions, mergeAttributes: boolean = false, ) { @@ -524,7 +548,7 @@ export class BaseModel implements ModelContract { * Make sure that the unique key is defined as a column * on the current model. */ - const castKey = this.$refs[uniqueKey] + const castKey = this.$getColumn(uniqueKey as string) if (!castKey) { throw new Exception( `"${uniqueKey}" is not defined as a column on the "${this.name}" model`, @@ -535,7 +559,7 @@ export class BaseModel implements ModelContract { * An array of values for the unique key */ const uniqueKeyValues = payload.map((row) => { - return ensureValue(row, uniqueKey, () => { + return ensureValue(row, uniqueKey as string, () => { throw new Exception( `Value for the "${uniqueKey}" is null or undefined inside "fetchOrNewUpMany" payload`, ) @@ -543,7 +567,7 @@ export class BaseModel implements ModelContract { }) const query = this.query(options) - const existingRows = await query.whereIn(castKey, uniqueKeyValues) + const existingRows = await query.whereIn(castKey.castAs, uniqueKeyValues) /** * Return existing or create missing rows in the same order as the original @@ -580,8 +604,8 @@ export class BaseModel implements ModelContract { */ public static async fetchOrCreateMany ( this: T, - uniqueKey: string, - payload: ModelObject[], + uniqueKey: keyof T['$columns'], + payload: Partial[], options?: ModelAdapterOptions, ): Promise[]> { const rows = await this.fetchOrNewUpMany(uniqueKey, payload, options) @@ -616,8 +640,8 @@ export class BaseModel implements ModelContract { */ public static async updateOrCreateMany ( this: T, - uniqueKey: string, - payload: ModelObject[], + uniqueKey: keyof T['$columns'], + payload: Partial[], options?: ModelAdapterOptions, ) { const rows = await this.fetchOrNewUpMany(uniqueKey, payload, options, true) @@ -993,7 +1017,7 @@ export class BaseModel implements ModelContract { */ public $setRelated (key: any, models: ModelContract | ModelContract[]) { const Model = this.constructor as typeof BaseModel - const relation = Model.$relations.get(key as string) + const relation = Model.$relationsDefinitions.get(key as string) /** * Ignore when relation is not defined @@ -1022,7 +1046,7 @@ export class BaseModel implements ModelContract { */ public $pushRelated (key: any, models: ModelContract | ModelContract[]) { const Model = this.constructor as typeof BaseModel - const relation = Model.$relations.get(key as string) + const relation = Model.$relationsDefinitions.get(key as string) /** * Ignore when relation is not defined @@ -1081,7 +1105,7 @@ export class BaseModel implements ModelContract { * need to pull the actual column name for that key and then * set the value. */ - const columnName = Model.$dbRefs[key] + const columnName = Model.$adapterKeysToAttributes[key] if (columnName) { /** * When consuming the adapter result, we must always set the attributes @@ -1095,7 +1119,7 @@ export class BaseModel implements ModelContract { * If key is defined as a relation, then ignore it, since one * must pass a qualified model to `this.$setRelated()` */ - if (Model.$relations.has(key)) { + if (Model.$relationsDefinitions.has(key)) { return } @@ -1137,7 +1161,7 @@ export class BaseModel implements ModelContract { */ if (isObject(values)) { Object.keys(values).forEach((key) => { - if (Model.$refs[key]) { + if (Model.$attributesToAdapterKeys[key]) { this[key] = values[key] return } @@ -1146,7 +1170,7 @@ export class BaseModel implements ModelContract { * If key is defined as a relation, then ignore it, since one * must pass a qualified model to `this.$setRelated()` */ - if (Model.$relations.has(key)) { + if (Model.$relationsDefinitions.has(key)) { return } @@ -1276,7 +1300,7 @@ export class BaseModel implements ModelContract { * Serializing computed properties as last. This gives the option to re-write * keys which are defined as attributes or relations. */ - Model.$computed.forEach((value, key) => { + Model.$computedDefinitions.forEach((value, key) => { const computedValue = this[key] if (computedValue !== undefined && value.serializeAs) { results[value.serializeAs] = computedValue diff --git a/test/orm/base-model.spec.ts b/test/orm/base-model.spec.ts index 32943d6b..2eb534dc 100644 --- a/test/orm/base-model.spec.ts +++ b/test/orm/base-model.spec.ts @@ -97,13 +97,12 @@ test.group('Base model | boot', (group) => { } User.boot() - assert.deepEqual(mapToObj(User.$columns), {}) - assert.deepEqual(mapToObj(User.$relations), {}) - assert.deepEqual(mapToObj(User.$computed), {}) - assert.deepEqual(User.$refs, {}) + assert.deepEqual(mapToObj(User.$columnsDefinitions), {}) + assert.deepEqual(mapToObj(User.$relationsDefinitions), {}) + assert.deepEqual(mapToObj(User.$computedDefinitions), {}) }) - test('compute refs from the added columns', async (assert) => { + test('resolve castAs key for a given attribute', async (assert) => { class User extends BaseModel { public static $increments = false @@ -115,7 +114,22 @@ test.group('Base model | boot', (group) => { } User.boot() - assert.deepEqual(User.$refs, { id: 'id', userName: 'user_name' }) + assert.deepEqual(User.$resolveCastKey('userName'), 'user_name') + }) + + test('resolve column name key for a given adapter key', async (assert) => { + class User extends BaseModel { + public static $increments = false + + @column({ isPrimary: true }) + public id: number + + @column() + public userName: string + } + + User.boot() + assert.deepEqual(User.$resolveColumnName('user_name'), 'userName') }) })