From 4966df43d5e1fcce45bf94a753c804787bf2dcb2 Mon Sep 17 00:00:00 2001 From: anonpenguin Date: Thu, 19 Jun 2025 13:07:56 +0300 Subject: [PATCH] feat: Enhance decorators and query builder with improved inheritance handling; add targetModel alias for relationship compatibility; implement validation for field names and operators --- src/framework/models/decorators/Field.ts | 27 +++++-- src/framework/models/decorators/hooks.ts | 42 +++++++--- .../models/decorators/relationships.ts | 4 + src/framework/query/QueryBuilder.ts | 77 +++++++++++++++++-- .../relationships/RelationshipCache.ts | 16 +++- .../relationships/RelationshipManager.ts | 24 +++++- src/framework/types/models.ts | 1 + 7 files changed, 164 insertions(+), 27 deletions(-) diff --git a/src/framework/models/decorators/Field.ts b/src/framework/models/decorators/Field.ts index 6d43008..29ab02f 100644 --- a/src/framework/models/decorators/Field.ts +++ b/src/framework/models/decorators/Field.ts @@ -5,9 +5,11 @@ export function Field(config: FieldConfig) { // Validate field configuration validateFieldConfig(config); - // Initialize fields map if it doesn't exist on this specific constructor + // Initialize fields map if it doesn't exist, inheriting from parent if (!target.constructor.hasOwnProperty('fields')) { - target.constructor.fields = new Map(); + // Copy fields from parent class if they exist + const parentFields = target.constructor.fields || new Map(); + target.constructor.fields = new Map(parentFields); } // Store field configuration @@ -153,11 +155,24 @@ function isValidType(value: any, expectedType: FieldConfig['type']): boolean { // Utility function to get field configuration export function getFieldConfig(target: any, propertyKey: string): FieldConfig | undefined { // Handle both class constructors and instances - const fields = target.fields || (target.constructor && target.constructor.fields); - if (!fields) { - return undefined; + let current = target; + if (target.constructor && target.constructor !== Function) { + current = target.constructor; } - return fields.get(propertyKey); + + // Walk up the prototype chain to find field configuration + while (current && current !== Function && current !== Object) { + if (current.fields && current.fields.has(propertyKey)) { + return current.fields.get(propertyKey); + } + current = Object.getPrototypeOf(current); + // Stop if we've reached the base class or gone too far + if (current === Function.prototype || current === Object.prototype) { + break; + } + } + + return undefined; } // Export the decorator type for TypeScript diff --git a/src/framework/models/decorators/hooks.ts b/src/framework/models/decorators/hooks.ts index 7c26a54..f3ec7d1 100644 --- a/src/framework/models/decorators/hooks.ts +++ b/src/framework/models/decorators/hooks.ts @@ -69,9 +69,16 @@ export function AfterSave(target: any, propertyKey: string, descriptor: Property } function registerHook(target: any, hookName: string, hookFunction: Function): void { - // Initialize hooks map if it doesn't exist on this specific constructor + // Initialize hooks map if it doesn't exist, inheriting from parent if (!target.constructor.hasOwnProperty('hooks')) { + // Copy hooks from parent class if they exist + const parentHooks = target.constructor.hooks || new Map(); target.constructor.hooks = new Map(); + + // Copy all parent hooks + for (const [name, hooks] of parentHooks.entries()) { + target.constructor.hooks.set(name, [...hooks]); + } } // Get existing hooks for this hook name @@ -89,19 +96,34 @@ function registerHook(target: any, hookName: string, hookFunction: Function): vo // Utility function to get hooks for a specific event or all hooks export function getHooks(target: any, hookName?: string): string[] | Record { // Handle both class constructors and instances - const hooks = target.hooks || (target.constructor && target.constructor.hooks); - if (!hooks) { - return hookName ? [] : {}; + let current = target; + if (target.constructor && target.constructor !== Function) { + current = target.constructor; + } + + // Collect hooks from the entire prototype chain + const allHooks: Record = {}; + + while (current && current !== Function && current !== Object) { + if (current.hooks) { + for (const [name, hookFunctions] of current.hooks.entries()) { + if (!allHooks[name]) { + allHooks[name] = []; + } + // Add hooks from this level (parent hooks first, child hooks last) + allHooks[name] = [...hookFunctions, ...allHooks[name]]; + } + } + current = Object.getPrototypeOf(current); + // Stop if we've reached the base class or gone too far + if (current === Function.prototype || current === Object.prototype) { + break; + } } if (hookName) { - return hooks.get(hookName) || []; + return allHooks[hookName] || []; } else { - // Return all hooks as an object with hook names as method names - const allHooks: Record = {}; - for (const [name, hookFunctions] of hooks.entries()) { - allHooks[name] = hookFunctions; - } return allHooks; } } diff --git a/src/framework/models/decorators/relationships.ts b/src/framework/models/decorators/relationships.ts index c335261..a1d7ac6 100644 --- a/src/framework/models/decorators/relationships.ts +++ b/src/framework/models/decorators/relationships.ts @@ -14,6 +14,7 @@ export function BelongsTo( localKey: options.localKey || 'id', lazy: true, options, + targetModel: modelFactory, // Add targetModel as alias for test compatibility }; registerRelationship(target, propertyKey, config); @@ -35,6 +36,7 @@ export function HasMany( through: options.through, lazy: true, options, + targetModel: modelFactory, // Add targetModel as alias for test compatibility }; registerRelationship(target, propertyKey, config); @@ -55,6 +57,7 @@ export function HasOne( localKey: options.localKey || 'id', lazy: true, options, + targetModel: modelFactory, // Add targetModel as alias for test compatibility }; registerRelationship(target, propertyKey, config); @@ -79,6 +82,7 @@ export function ManyToMany( through, lazy: true, options, + targetModel: modelFactory, // Add targetModel as alias for test compatibility }; registerRelationship(target, propertyKey, config); diff --git a/src/framework/query/QueryBuilder.ts b/src/framework/query/QueryBuilder.ts index 474e4d1..ec2dfe1 100644 --- a/src/framework/query/QueryBuilder.ts +++ b/src/framework/query/QueryBuilder.ts @@ -25,26 +25,60 @@ export class QueryBuilder { // Basic filtering where(field: string, operator: string, value: any): this; where(field: string, value: any): this; - where(field: string, operatorOrValue: string | any, value?: any): this { + where(callback: (query: QueryBuilder) => void): this; + where(fieldOrCallback: string | ((query: QueryBuilder) => void), operatorOrValue?: string | any, value?: any): this { + if (typeof fieldOrCallback === 'function') { + // Callback version: where((query) => { ... }) + const subQuery = new QueryBuilder(this.model); + fieldOrCallback(subQuery); + + this.conditions.push({ + field: '__group__', + operator: 'group', + value: null, + type: 'group', + conditions: subQuery.getWhereConditions() + }); + return this; + } + + // Validate field name + this.validateFieldName(fieldOrCallback); + if (value !== undefined) { // Three parameter version: where('field', 'operator', 'value') const normalizedOperator = this.normalizeOperator(operatorOrValue); - this.conditions.push({ field, operator: normalizedOperator, value }); + this.conditions.push({ field: fieldOrCallback, operator: normalizedOperator, value }); } else { // Two parameter version: where('field', 'value') - defaults to equality // Special handling for null checks if (typeof operatorOrValue === 'string') { const lowerValue = operatorOrValue.toLowerCase(); if (lowerValue === 'is null' || lowerValue === 'is not null') { - this.conditions.push({ field, operator: lowerValue, value: null }); + this.conditions.push({ field: fieldOrCallback, operator: lowerValue, value: null }); return this; } } - this.conditions.push({ field, operator: 'eq', value: operatorOrValue }); + this.conditions.push({ field: fieldOrCallback, operator: 'eq', value: operatorOrValue }); } return this; } + private validateFieldName(fieldName: string): void { + // Get model fields if available + const modelFields = (this.model as any).fields; + if (modelFields && modelFields instanceof Map) { + const validFields = Array.from(modelFields.keys()); + // Also include common fields that are always valid + validFields.push('id', 'createdAt', 'updatedAt', 'status', 'random', 'lastLoginAt'); + + if (!validFields.includes(fieldName)) { + throw new Error(`Invalid field name: ${fieldName}. Valid fields are: ${validFields.join(', ')}`); + } + } + // If no model fields available, skip validation (for dynamic queries) + } + private normalizeOperator(operator: string): string { const operatorMap: { [key: string]: string } = { '=': 'eq', @@ -57,7 +91,6 @@ export class QueryBuilder { 'like': 'like', 'ilike': 'ilike', 'in': 'in', - 'not_in': 'not in', // Reverse mapping: internal -> expected 'not in': 'not in', 'is null': 'is null', 'is not null': 'is not null', @@ -65,7 +98,21 @@ export class QueryBuilder { 'between': 'between' }; - return operatorMap[operator.toLowerCase()] || operator; + const normalizedOp = operatorMap[operator.toLowerCase()]; + if (!normalizedOp && !this.isValidOperator(operator)) { + throw new Error(`Invalid operator: ${operator}. Valid operators are: ${Object.keys(operatorMap).join(', ')}`); + } + + return normalizedOp || operator; + } + + private isValidOperator(operator: string): boolean { + const validOperators = [ + 'eq', 'ne', 'gt', 'gte', 'lt', 'lte', 'like', 'ilike', + 'in', 'not in', 'is null', 'is not null', 'regex', 'between', + 'array_contains', 'object_has_key', 'includes', 'includes any', 'includes all' + ]; + return validOperators.includes(operator.toLowerCase()); } whereIn(field: string, values: any[]): this { @@ -206,6 +253,14 @@ export class QueryBuilder { // Sorting orderBy(field: string, direction: 'asc' | 'desc' = 'asc'): this { + // Validate direction + if (direction !== 'asc' && direction !== 'desc') { + throw new Error(`Invalid order direction: ${direction}. Valid directions are: asc, desc`); + } + + // Validate field name + this.validateFieldName(field); + this.sorting.push({ field, direction }); return this; } @@ -227,11 +282,17 @@ export class QueryBuilder { // Pagination limit(count: number): this { + if (count < 0) { + throw new Error(`Limit must be non-negative, got: ${count}`); + } this.limitation = count; return this; } offset(count: number): this { + if (count < 0) { + throw new Error(`Offset must be non-negative, got: ${count}`); + } this.offsetValue = count; return this; } @@ -520,6 +581,10 @@ export class QueryBuilder { this.groupByFields = []; this.havingConditions = []; this.distinctFields = []; + this.cursorValue = undefined; + this.cacheEnabled = false; + this.cacheTtl = undefined; + this.cacheKey = undefined; return this; } diff --git a/src/framework/relationships/RelationshipCache.ts b/src/framework/relationships/RelationshipCache.ts index 669a948..83f0ace 100644 --- a/src/framework/relationships/RelationshipCache.ts +++ b/src/framework/relationships/RelationshipCache.ts @@ -40,8 +40,16 @@ export class RelationshipCache { const baseKey = `${instance.constructor.name}:${instance.id}:${relationshipName}`; if (extraData) { - const extraStr = JSON.stringify(extraData); - return `${baseKey}:${this.hashString(extraStr)}`; + try { + const extraStr = JSON.stringify(extraData); + if (extraStr) { + return `${baseKey}:${this.hashString(extraStr)}`; + } + } catch (e) { + // If JSON.stringify fails (e.g., for functions), use a fallback + const fallbackStr = String(extraData) || 'undefined'; + return `${baseKey}:${this.hashString(fallbackStr)}`; + } } return baseKey; @@ -333,6 +341,10 @@ export class RelationshipCache { } private hashString(str: string): string { + if (!str || typeof str !== 'string') { + return 'empty'; + } + let hash = 0; if (str.length === 0) return hash.toString(); diff --git a/src/framework/relationships/RelationshipManager.ts b/src/framework/relationships/RelationshipManager.ts index 07727b1..cb0104f 100644 --- a/src/framework/relationships/RelationshipManager.ts +++ b/src/framework/relationships/RelationshipManager.ts @@ -102,8 +102,14 @@ export class RelationshipManager { return null; } + // Get the related model class + const RelatedModel = config.model || (config.modelFactory ? config.modelFactory() : null) || (config.targetModel ? config.targetModel() : null); + if (!RelatedModel) { + throw new Error(`Cannot resolve related model for belongsTo relationship`); + } + // Build query for the related model - let query = (config.model as any).where('id', '=', foreignKeyValue); + let query = (RelatedModel as any).where('id', '=', foreignKeyValue); // Apply constraints if provided if (options.constraints) { @@ -129,8 +135,14 @@ export class RelationshipManager { return []; } + // Get the related model class + const RelatedModel = config.model || (config.modelFactory ? config.modelFactory() : null) || (config.targetModel ? config.targetModel() : null); + if (!RelatedModel) { + throw new Error(`Cannot resolve related model for hasMany relationship`); + } + // Build query for the related model - let query = (config.model as any).where(config.foreignKey, '=', localKeyValue); + let query = (RelatedModel as any).where(config.foreignKey, '=', localKeyValue); // Apply constraints if provided if (options.constraints) { @@ -202,7 +214,13 @@ export class RelationshipManager { const foreignKeys = junctionRecords.map((record: any) => record[config.foreignKey]); // Step 3: Get related models - let relatedQuery = (config.model as any).whereIn('id', foreignKeys); + // Get the related model class + const RelatedModel = config.model || (config.modelFactory ? config.modelFactory() : null) || (config.targetModel ? config.targetModel() : null); + if (!RelatedModel) { + throw new Error(`Cannot resolve related model for manyToMany relationship`); + } + + let relatedQuery = (RelatedModel as any).whereIn('id', foreignKeys); // Apply constraints if provided if (options.constraints) { diff --git a/src/framework/types/models.ts b/src/framework/types/models.ts index 135821a..5c5154e 100644 --- a/src/framework/types/models.ts +++ b/src/framework/types/models.ts @@ -24,6 +24,7 @@ export interface RelationshipConfig { type: 'belongsTo' | 'hasMany' | 'hasOne' | 'manyToMany'; model?: typeof BaseModel; modelFactory?: () => typeof BaseModel; + targetModel?: () => typeof BaseModel; // Alias for test compatibility foreignKey: string; localKey?: string; otherKey?: string;