ModelQuery: fix field disambiguation

package.json

@@ -1,6 +1,6 @@
 {
   "name": "wms-core",
-  "version": "0.21.13-rc.1",
+  "version": "0.21.13-rc.2",
   "description": "Node web application framework and toolbelt.",
   "repository": "https://gitlab.com/ArisuOngaku/wms-core",
   "author": "Alice Gaudon <alice@gaudon.pro>",

src/db/Model.ts

@@ -5,7 +5,7 @@ import ModelComponent from "./ModelComponent";
 import {Type} from "../Utils";
 import ModelFactory from "./ModelFactory";
 import ModelRelation from "./ModelRelation";
-import ModelQuery, {ModelQueryResult} from "./ModelQuery";
+import ModelQuery, {ModelQueryResult, SelectFields, SelectFieldValue} from "./ModelQuery";
 import {Request} from "express";
 
 export default abstract class Model {
@@ -193,7 +193,7 @@ export default abstract class Model {
 
     public async exists(): Promise<boolean> {
         if (this._cached_exists === undefined) {
-            const query = this._factory.select('1');
+            const query = this._factory.select('');
             for (const indexField of this._factory.getPrimaryKeyFields()) {
                 query.where(indexField, this[indexField]);
             }

src/db/ModelFactory.ts

@@ -1,6 +1,6 @@
 import ModelComponent from "./ModelComponent";
 import Model from "./Model";
-import ModelQuery, {ModelQueryResult} from "./ModelQuery";
+import ModelQuery, {ModelQueryResult, SelectFields} from "./ModelQuery";
 import {Request} from "express";
 import {Type} from "../Utils";
 
@@ -43,7 +43,7 @@ export default class ModelFactory<T extends Model> {
         return this.modelType.table;
     }
 
-    public select(...fields: string[]): ModelQuery<T> {
+    public select(...fields: SelectFields): ModelQuery<T> {
         return ModelQuery.select(this, ...fields);
     }
 

src/db/ModelQuery.ts

@@ -7,7 +7,8 @@ import ModelFactory from "./ModelFactory";
 
 
 export default class ModelQuery<M extends Model> implements WhereFieldConsumer<M> {
-    public static select<M extends Model>(factory: ModelFactory<M>, ...fields: string[]): ModelQuery<M> {
+    public static select<M extends Model>(factory: ModelFactory<M>, ...fields: SelectFields): ModelQuery<M> {
+        fields = fields.map(v => v === '' ? new SelectFieldValue('none', 1, true) : v);
         return new ModelQuery(QueryType.SELECT, factory, fields.length > 0 ? fields : ['*']);
     }
 
@@ -30,8 +31,9 @@ export default class ModelQuery<M extends Model> implements WhereFieldConsumer<M
     private readonly type: QueryType;
     private readonly factory: ModelFactory<M>;
     private readonly table: string;
-    private fields: (string | SelectFieldValue | UpdateFieldValue)[];
+    private readonly fields: SelectFields;
     private _leftJoin?: string;
+    private _leftJoinAlias?: string;
     private _leftJoinOn: WhereFieldValue[] = [];
     private _where: (WhereFieldValue | WhereFieldValueGroup)[] = [];
     private _limit?: number;
@@ -42,15 +44,16 @@ export default class ModelQuery<M extends Model> implements WhereFieldConsumer<M
     private _pivot?: string[];
     private _recursiveRelation?: RelationDatabaseProperties;
 
-    private constructor(type: QueryType, factory: ModelFactory<M>, fields?: (string | SelectFieldValue | UpdateFieldValue)[]) {
+    private constructor(type: QueryType, factory: ModelFactory<M>, fields?: SelectFields) {
         this.type = type;
         this.factory = factory;
         this.table = factory.table;
         this.fields = fields || [];
     }
 
-    public leftJoin(table: string): this {
+    public leftJoin(table: string, alias?: string): this {
         this._leftJoin = table;
+        this._leftJoinAlias = alias;
         return this;
     }
 
@@ -119,19 +122,26 @@ export default class ModelQuery<M extends Model> implements WhereFieldConsumer<M
     public toString(final: boolean = false): string {
         let query = '';
 
-        // Prevent wildcard and fields from conflicting
-        if (this._leftJoin) {
-            this.fields = this.fields
-                .map(f => !f.toString().startsWith('(') && f.toString().split('.').length === 1 ? `\`${this.table}\`.${f}` : f);
-        }
-
         if (this._pivot) this.fields.push(...this._pivot);
 
-        let fields = this.fields.join(',');
+        // Prevent wildcard and fields from conflicting
+        let fieldArray = this.fields.map(f => {
+            let field = f.toString();
+
+            if (field.startsWith('(')) return f; // Skip sub-queries
+
+            let parts = field.split('.');
+            if (parts.length === 1) parts = [this.table, field]; // Add table disambiguation
+
+            return parts.map(v => v.startsWith('`') || v === '*' ? v : `\`${v}\``).join('.');
+        });
+
+        let fields = fieldArray.join(',');
 
         let join = '';
         if (this._leftJoin) {
-            join = `LEFT JOIN \`${this._leftJoin}\` ON ${this._leftJoinOn[0]}`;
+            const alias = this._leftJoinAlias ? ` AS \`${this._leftJoinAlias}\`` : '';
+            join = `LEFT JOIN \`${this._leftJoin}\`${alias} ON ${this._leftJoinOn[0]}`;
             for (let i = 1; i < this._leftJoinOn.length; i++) {
                 join += this._leftJoinOn[i].toString(false);
             }
@@ -343,7 +353,7 @@ class FieldValue {
     }
 }
 
-class SelectFieldValue extends FieldValue {
+export class SelectFieldValue extends FieldValue {
     public toString(first: boolean = true): string {
         return `(${this.value instanceof ModelQuery ? this.value : (this.raw ? this.value : '?')}) AS \`${this.field}\``;
     }
@@ -404,3 +414,5 @@ interface WhereFieldConsumer<M extends Model> {
 
     groupWhere(setter: (query: WhereFieldConsumer<M>) => void, operator?: WhereOperator): this;
 }
+
+export type SelectFields = (string | SelectFieldValue | UpdateFieldValue)[];

src/db/ModelRelation.ts

@@ -158,7 +158,7 @@ export class ManyThroughModelRelation<S extends Model, O extends Model> extends
         super(model, foreignFactory, dbProperties);
         this.dbProperties = dbProperties;
         this.constraint(query => query
-            .leftJoin(`${this.dbProperties.pivotTable} as pivot`)
+            .leftJoin(this.dbProperties.pivotTable, 'pivot')
             .on(`pivot.${this.dbProperties.foreignPivotKey}`, `${this.foreignFactory.table}.${this.dbProperties.foreignKey}`)
         );
     }

src/db/Validator.ts

@@ -193,7 +193,7 @@ export default class Validator<T> {
                 if (querySupplier) {
                     query = querySupplier().where(foreignKey, val);
                 } else {
-                    query = (model instanceof Model ? <any>model.constructor : model).select('1').where(foreignKey, val);
+                    query = (model instanceof Model ? <any>model.constructor : model).select('').where(foreignKey, val);
                 }
                 if (model instanceof Model && typeof model.id === 'number') query = query.where('id', model.id, WhereTest.NE);
                 return (await query.execute(c)).results.length === 0;
@@ -206,7 +206,7 @@ export default class Validator<T> {
 
     public exists(modelClass: Function, foreignKey?: string): Validator<T> {
         this.addStep({
-            verifyStep: async (val, thingName, c) => (await (<any>modelClass).select('1').where(foreignKey !== undefined ? foreignKey : thingName, val).execute(c)).results.length >= 1,
+            verifyStep: async (val, thingName, c) => (await (<any>modelClass).select('').where(foreignKey !== undefined ? foreignKey : thingName, val).execute(c)).results.length >= 1,
             throw: () => new UnknownRelationValidationError((<any>modelClass).table, foreignKey),
             isFormat: false,
         });

test/Model.test.ts

@@ -3,6 +3,7 @@ import Model from "../src/db/Model";
 import {MIGRATIONS} from "./_migrations";
 import ModelFactory from "../src/db/ModelFactory";
 import {ValidationBag} from "../src/db/Validator";
+import Logger from "../src/Logger";
 
 class FakeDummyModel extends Model {
     public id?: number = undefined;
@@ -18,6 +19,7 @@ class FakeDummyModel extends Model {
 let factory: ModelFactory<FakeDummyModel>;
 
 beforeAll(async () => {
+    Logger.verbose();
     MysqlConnectionManager.registerMigrations(MIGRATIONS);
     ModelFactory.register(FakeDummyModel);
     await MysqlConnectionManager.prepare();

test/ModelQuery.test.ts

@@ -1,8 +1,32 @@
-import ModelQuery, {WhereOperator} from "../src/db/ModelQuery";
+import ModelQuery, {SelectFieldValue, WhereOperator} from "../src/db/ModelQuery";
 import ModelFactory from "../src/db/ModelFactory";
 import Model from "../src/db/Model";
 
 describe('Test ModelQuery', () => {
+    test('update', () => {
+        const query = ModelQuery.update({table: 'model'} as unknown as ModelFactory<Model>, {
+            'f1': 'v1',
+            'f2': 'v2',
+            'f3': 'v3',
+        }).where('f4', 'v4');
+        expect(query.toString(true)).toBe('UPDATE `model` SET `model`.`f1`=?,`model`.`f2`=?,`model`.`f3`=? WHERE `f4`=?  ');
+        expect(query.variables).toStrictEqual(['v1','v2','v3','v4']);
+    });
+
+    test('function select', () => {
+        const query = ModelQuery.select({table: 'model'} as unknown as ModelFactory<Model>, 'f1', new SelectFieldValue('_count', 'COUNT(*)', true));
+        expect(query.toString(true)).toBe('SELECT `model`.`f1`,(COUNT(*)) AS `_count` FROM `model`    ');
+        expect(query.variables).toStrictEqual([]);
+    });
+
+    test('pivot', () => {
+        const query = ModelQuery.select({table: 'model'} as unknown as ModelFactory<Model>, 'f1');
+        query.pivot('pivot.f2', 'f3');
+
+        expect(query.toString(true)).toBe('SELECT `model`.`f1`,`pivot`.`f2`,`model`.`f3` FROM `model`    ');
+        expect(query.variables).toStrictEqual([]);
+    });
+
     test('groupWhere generates proper query', () => {
         const query = ModelQuery.select({table: 'model'} as unknown as ModelFactory<Model>, '*');
         query.where('f1', 'v1');
@@ -10,8 +34,8 @@ describe('Test ModelQuery', () => {
             .groupWhere(q => q.where('f4', 'v4'), WhereOperator.OR))
             .where('f5', 'v5');
 
-        expect(query.toString(true)).toBe('SELECT * FROM `model`  WHERE `f1`=? AND (`f2`=? AND `f3`=? OR (`f4`=?)) AND `f5`=?  ');
+        expect(query.toString(true)).toBe('SELECT `model`.* FROM `model`  WHERE `f1`=? AND (`f2`=? AND `f3`=? OR (`f4`=?)) AND `f5`=?  ');
-        expect(query.variables.length).toBe(5);
+        expect(query.variables).toStrictEqual(['v1','v2','v3','v4','v5']);
     });
 
     test('recursive queries', () => {
@@ -22,6 +46,6 @@ describe('Test ModelQuery', () => {
         query.sortBy('f2', 'ASC').limit(8);
 
         expect(query.toString(true)).toBe("WITH RECURSIVE cte AS (SELECT `model`.* FROM `model` WHERE `f1`=? UNION SELECT o.* FROM `model` AS o, cte AS c WHERE o.`foreign`=c.`local`) SELECT * FROM cte LEFT JOIN `test` ON `model`.`j1`=`test`.`j2` ORDER BY `f2` ASC LIMIT 8");
-        expect(query.variables.length).toBe(1);
+        expect(query.variables).toStrictEqual(['v1']);
     });
 });