From 25f890e082adb6d006a9dafeec246e525f8f0f8e Mon Sep 17 00:00:00 2001 From: Alice Gaudon Date: Sun, 6 Sep 2020 10:43:45 +0200 Subject: [PATCH] Remove unnecessary db query to determine whether a model exists in db --- src/db/Model.ts | 84 +++++++++++++++++++++--------------------- src/db/ModelFactory.ts | 4 +- src/db/ModelQuery.ts | 2 +- test/Model.test.ts | 14 +++---- 4 files changed, 51 insertions(+), 53 deletions(-) diff --git a/src/db/Model.ts b/src/db/Model.ts index 4ed6648..df552bb 100644 --- a/src/db/Model.ts +++ b/src/db/Model.ts @@ -21,7 +21,7 @@ export default abstract class Model { } public static create(this: ModelType, data: any): T { - return ModelFactory.get(this).create(data); + return ModelFactory.get(this).create(data, true); } public static select(this: ModelType, ...fields: string[]): ModelQuery { @@ -47,14 +47,15 @@ export default abstract class Model { protected readonly _factory: ModelFactory; private readonly _components: ModelComponent[] = []; private readonly _validators: { [key: string]: Validator } = {}; - private _cached_exists?: boolean = undefined; + private _exists: boolean; [key: string]: any; - public constructor(factory: ModelFactory) { + public constructor(factory: ModelFactory, isNew: boolean) { if (!factory || !(factory instanceof ModelFactory)) throw new Error('Cannot instantiate model directly.'); this._factory = factory; this.init(); + this._exists = !isNew; } protected abstract init(): void; @@ -94,28 +95,29 @@ export default abstract class Model { protected async autoFill(): Promise { } - protected async beforeSave(exists: boolean, connection: Connection): Promise { + protected async beforeSave(connection: Connection): Promise { } protected async afterSave(): Promise { } public async save(connection?: Connection, postHook?: (callback: () => Promise) => void): Promise { + if (connection && !postHook) throw new Error('If connection is provided, postHook must be provided too.'); + await this.autoFill(); await this.validate(false, connection); - const exists = await this.exists(); - let needs_full_update = false; - - if (connection) { - needs_full_update = await this.saveTransaction(connection, exists, needs_full_update); - } else { - needs_full_update = await MysqlConnectionManager.wrapTransaction(async connection => this.saveTransaction(connection, exists, needs_full_update)); - } + const needs_full_update = connection ? + await this.saveTransaction(connection) : + await MysqlConnectionManager.wrapTransaction(async connection => this.saveTransaction(connection)); const callback = async () => { if (needs_full_update) { - this.updateWithData((await this._factory.select().where('id', this.id!).limit(1).execute()).results[0]); + const result = await this._factory.select() + .where('id', this.id!) + .limit(1) + .execute(); + this.updateWithData(result.results[0]); } await this.afterSave(); @@ -128,65 +130,67 @@ export default abstract class Model { } } - private async saveTransaction(connection: Connection, exists: boolean, needs_full_update: boolean): Promise { + private async saveTransaction(connection: Connection): Promise { // Before save - await this.beforeSave(exists, connection); - if (!exists && this.hasOwnProperty('created_at')) { + await this.beforeSave(connection); + if (!this.exists() && this.hasOwnProperty('created_at')) { this.created_at = new Date(); } - if (exists && this.hasOwnProperty('updated_at')) { + if (this.exists() && this.hasOwnProperty('updated_at')) { this.updated_at = new Date(); } const properties = []; const values = []; + let needsFullUpdate = false; - if (exists) { + if (this.exists()) { const data: any = {}; for (const property of this._properties) { const value = this[property]; - if (value !== undefined) { - data[property] = value; - } else { - needs_full_update = true; - } + + if (value === undefined) needsFullUpdate = true; + else data[property] = value; } - let query = this._factory.update(data); + + const query = this._factory.update(data); for (const indexField of this._factory.getPrimaryKeyFields()) { - query = query.where(indexField, this[indexField]); + query.where(indexField, this[indexField]); } await query.execute(connection); } else { const props_holders = []; for (const property of this._properties) { const value = this[property]; - if (value !== undefined) { + + if (value === undefined) { + needsFullUpdate = true; + } else { properties.push(property); props_holders.push('?'); values.push(value); - } else { - needs_full_update = true; } } + const fieldNames = properties.map(f => `\`${f}\``).join(', '); const result = await query(`INSERT INTO ${this.table} (${fieldNames}) VALUES(${props_holders.join(', ')})`, values, connection); if (this.hasOwnProperty('id')) this.id = result.other.insertId; - this._cached_exists = true; + this._exists = true; } - return needs_full_update; + return needsFullUpdate; } public async delete(): Promise { if (!(await this.exists())) throw new Error('This model instance doesn\'t exist in DB.'); - let query = this._factory.delete(); + const query = this._factory.delete(); for (const indexField of this._factory.getPrimaryKeyFields()) { - query = query.where(indexField, this[indexField]); + query.where(indexField, this[indexField]); } await query.execute(); - this._cached_exists = false; + this._exists = false; } public async validate(onlyFormat: boolean = false, connection?: Connection): Promise { @@ -195,16 +199,8 @@ export default abstract class Model { )); } - public async exists(): Promise { - if (this._cached_exists === undefined) { - const query = this._factory.select(''); - for (const indexField of this._factory.getPrimaryKeyFields()) { - query.where(indexField, this[indexField]); - } - this._cached_exists = (await query.limit(1).execute()).results.length > 0; - } - - return this._cached_exists; + public exists(): boolean { + return this._exists; } public equals(model: this): boolean { @@ -230,5 +226,7 @@ export default abstract class Model { export interface ModelType extends Type { table: string; + new(factory: ModelFactory, isNew: boolean): T; + getPrimaryKeyFields(): string[]; } diff --git a/src/db/ModelFactory.ts b/src/db/ModelFactory.ts index f27d1b6..3e44f51 100644 --- a/src/db/ModelFactory.ts +++ b/src/db/ModelFactory.ts @@ -28,8 +28,8 @@ export default class ModelFactory { this.components.push(modelComponentFactory); } - public create(data: any): T { - const model = new this.modelType(this, data); + public create(data: any, isNewModel: boolean): T { + const model = new this.modelType(this, isNewModel); for (const component of this.components) { model.addComponent(new component(model)); } diff --git a/src/db/ModelQuery.ts b/src/db/ModelQuery.ts index ffaea68..e2211a2 100644 --- a/src/db/ModelQuery.ts +++ b/src/db/ModelQuery.ts @@ -240,7 +240,7 @@ export default class ModelQuery implements WhereFieldConsumer { name: 'a_name', date: date, non_existing_property: 'dropped_value', - }); + }, true); expect(model.id).toBeUndefined(); expect(model.name).toBe('a_name'); @@ -67,13 +67,13 @@ describe('Model', () => { const insertInstance: FakeDummyModel | null = factory.create({ name: 'name1', date: date, - }); + }, true); // Insert - expect(await insertInstance.exists()).toBeFalsy(); + expect(insertInstance.exists()).toBeFalsy(); await insertInstance.save(); - expect(await insertInstance.exists()).toBeTruthy(); + expect(insertInstance.exists()).toBeTruthy(); expect(insertInstance.id).toBe(1); // Auto id from insert expect(insertInstance.name).toBe('name1'); @@ -90,14 +90,14 @@ describe('Model', () => { const failingInsertModel = factory.create({ name: 'a', - }); + }, true); await expect(failingInsertModel.save()).rejects.toBeInstanceOf(ValidationBag); }); it('should update properly', async () => { const insertModel = factory.create({ name: 'update', - }); + }, true); await insertModel.save(); const preUpdatedModel = await FakeDummyModel.getById(insertModel.id); @@ -118,7 +118,7 @@ describe('Model', () => { it('should delete properly', async () => { const insertModel = factory.create({ name: 'delete', - }); + }, true); await insertModel.save(); const preDeleteModel = await FakeDummyModel.getById(insertModel.id);