move checks to service, fix tests

This commit is contained in:
Charlie Kolb 2025-10-15 13:26:15 +02:00
parent 05dcbe8fbc
commit e0140e7942
No known key found for this signature in database
4 changed files with 54 additions and 33 deletions

View File

@ -1539,7 +1539,7 @@ describe('dataTable', () => {
{
columnName: 'c1',
condition: 'gte',
value: d,
value: `${d.toISOString()}`,
path: 'd',
},
],

View File

@ -102,7 +102,6 @@ function resolvePath(
function getConditionAndParams(
filter: DataTableFilter['filters'][number],
index: number,
columns: DataTableColumn[],
dbType: DataSourceOptions['type'],
tableReference?: string,
): [string, Record<string, unknown>] {
@ -125,11 +124,8 @@ function getConditionAndParams(
}
}
// Find the column type to normalize the value consistently
const columnInfo = columns?.find((col) => col.name === filter.columnName);
const value = columnInfo
? normalizeValueForDatabase(filter.value, columnInfo?.type, dbType, filter.path)
: filter.value;
// For filters, we let TypeORM handle date conversion through parameterized queries.
const value = filter.value;
// Handle operators that map directly to SQL operators
const operators: Record<string, string> = {
@ -393,7 +389,7 @@ export class DataTableRowsRepository {
const query = em.createQueryBuilder().update(table);
// Some DBs (like SQLite) don't allow using table aliases as column prefixes in UPDATE statements
this.applyFilters(query, columns, filter, undefined);
this.applyFilters(query, filter, undefined);
query.set(setData);
if (useReturning && returnData) {
@ -495,7 +491,7 @@ export class DataTableRowsRepository {
// Just delete and return true
const query = em.createQueryBuilder().delete().from(table, 'dataTable');
if (filter) {
this.applyFilters(query, columns, filter, undefined);
this.applyFilters(query, filter, undefined);
}
await query.execute();
@ -508,7 +504,7 @@ export class DataTableRowsRepository {
const selectQuery = em.createQueryBuilder().select('*').from(table, 'dataTable');
if (filter) {
this.applyFilters(selectQuery, columns, filter, 'dataTable');
this.applyFilters(selectQuery, filter, 'dataTable');
}
const rawRows = await selectQuery.getRawMany<DataTableRawRowReturn>();
@ -532,7 +528,7 @@ export class DataTableRowsRepository {
}
if (filter) {
this.applyFilters(deleteQuery, columns, filter, undefined);
this.applyFilters(deleteQuery, filter, undefined);
}
const result = await deleteQuery.execute();
@ -556,7 +552,7 @@ export class DataTableRowsRepository {
const table = toTableName(dataTableId);
const selectColumns = idsOnly ? 'id' : '*';
const selectQuery = em.createQueryBuilder().select(selectColumns).from(table, 'dataTable');
this.applyFilters(selectQuery, columns, filter, 'dataTable');
this.applyFilters(selectQuery, filter, 'dataTable');
const rawRows: DataTableRowsReturn = await selectQuery.getRawMany();
if (idsOnly) {
@ -616,7 +612,6 @@ export class DataTableRowsRepository {
async getManyAndCount(
dataTableId: string,
columns: DataTableColumn[],
dto: ListDataTableContentQueryDto,
trx?: EntityManager,
) {
@ -624,7 +619,7 @@ export class DataTableRowsRepository {
this.dataSource.manager,
trx,
async (em) => {
const [countQuery, query] = this.getManyQuery(dataTableId, columns, dto, em);
const [countQuery, query] = this.getManyQuery(dataTableId, dto, em);
const data: DataTableRowsReturn = await query.select('*').getRawMany();
const countResult = await countQuery.select('COUNT(*) as count').getRawOne<{
count: number | string | null;
@ -675,7 +670,6 @@ export class DataTableRowsRepository {
private getManyQuery(
dataTableId: string,
columns: DataTableColumn[],
dto: ListDataTableContentQueryDto,
em: EntityManager,
): [QueryBuilder, QueryBuilder] {
@ -684,7 +678,7 @@ export class DataTableRowsRepository {
const tableReference = 'dataTable';
query.from(toTableName(dataTableId), tableReference);
if (dto.filter) {
this.applyFilters(query, columns, dto.filter, tableReference);
this.applyFilters(query, dto.filter, tableReference);
}
const countQuery = query.clone().select('COUNT(*)');
this.applySorting(query, dto);
@ -695,7 +689,6 @@ export class DataTableRowsRepository {
private applyFilters<T extends ObjectLiteral>(
query: SelectQueryBuilder<T> | UpdateQueryBuilder<T> | DeleteQueryBuilder<T>,
columns: DataTableColumn[],
filter: DataTableFilter,
tableReference?: string,
): void {
@ -704,7 +697,7 @@ export class DataTableRowsRepository {
const dbType = this.dataSource.options.type;
const conditionsAndParams = filters.map((filter, i) =>
getConditionAndParams(filter, i, columns, dbType, tableReference),
getConditionAndParams(filter, i, dbType, tableReference),
);
if (conditionsAndParams.length === 1) {

View File

@ -26,7 +26,7 @@ import type {
DataTableColumnType,
DataTableRowReturnWithState,
} from 'n8n-workflow';
import { DATA_TABLE_SYSTEM_COLUMN_TYPE_MAP, validateFieldType } from 'n8n-workflow';
import { DATA_TABLE_SYSTEM_COLUMN_TYPE_MAP, UserError, validateFieldType } from 'n8n-workflow';
import { RoleService } from '@/services/role.service';
@ -42,6 +42,12 @@ import { DataTableNotFoundError } from './errors/data-table-not-found.error';
import { DataTableValidationError } from './errors/data-table-validation.error';
import { normalizeRows } from './utils/sql-utils';
type ValidationOptions = Partial<{
includeSystemColumns: boolean;
skipDateTransform: boolean;
validateJsonInput: boolean;
}>;
@Service()
export class DataTableService {
constructor(
@ -160,7 +166,6 @@ export class DataTableService {
: dto;
const result = await this.dataTableRowsRepository.getManyAndCount(
dataTableId,
columns,
transformedDto,
em,
);
@ -194,7 +199,9 @@ export class DataTableService {
const result = await this.dataTableColumnRepository.manager.transaction(async (trx) => {
const columns = await this.dataTableColumnRepository.getColumns(dataTableId, trx);
const transformedRows = this.validateAndTransformRows(rows, columns);
const transformedRows = this.validateAndTransformRows(rows, columns, {
validateJsonInput: true,
});
return await this.dataTableRowsRepository.insertRows(
dataTableId,
@ -303,7 +310,9 @@ export class DataTableService {
throw new DataTableValidationError('Data columns must not be empty');
}
const [transformedData] = this.validateAndTransformRows([data], columns, false);
const [transformedData] = this.validateAndTransformRows([data], columns, {
validateJsonInput: true,
});
const transformedFilter = this.validateAndTransformFilters(filter, columns);
return { data: transformedData, filter: transformedFilter };
@ -432,8 +441,11 @@ export class DataTableService {
private validateAndTransformRows(
rows: DataTableRows,
columns: Array<{ name: string; type: DataTableColumnType }>,
includeSystemColumns = false,
skipDateTransform = false,
{
includeSystemColumns = false,
skipDateTransform = false,
validateJsonInput = false,
}: ValidationOptions = {},
): DataTableRows {
// Include system columns like 'id' if requested
const allColumns = includeSystemColumns
@ -455,12 +467,10 @@ export class DataTableService {
if (!columnNames.has(key)) {
throw new DataTableValidationError(`unknown column name '${key}'`);
}
transformedRow[key] = this.validateAndTransformCell(
row[key],
key,
columnTypeMap,
transformedRow[key] = this.validateAndTransformCell(row[key], key, columnTypeMap, {
skipDateTransform,
);
validateJsonInput,
});
}
return transformedRow;
});
@ -470,7 +480,7 @@ export class DataTableService {
cell: DataTableColumnJsType,
key: string,
columnTypeMap: Map<string, string>,
skipDateTransform = false,
{ skipDateTransform = false, validateJsonInput = false } = {},
): DataTableColumnJsType {
if (cell === null) return null;
@ -480,7 +490,7 @@ export class DataTableService {
const fieldType = columnTypeToFieldType[columnType];
if (!fieldType) return cell;
if (columnType === 'json') return cell;
if (!validateJsonInput && columnType === 'json') return cell;
const validationResult = validateFieldType(key, cell, fieldType, {
strict: false, // Allow type coercion (e.g., string numbers to numbers)
@ -508,6 +518,24 @@ export class DataTableService {
}
}
if (columnType === 'json') {
// json accepts objects or strings containing objects as input
// but expects values with a path for output/get/read operations
if (typeof cell === 'string') {
try {
JSON.parse(cell);
return cell;
} catch (e) {
throw new UserError(`Failed to parse string input '${cell}' as object for json column`);
}
} else if (typeof cell !== 'object') {
throw new UserError(
`Unexpected non-object input '${cell}' of type ${typeof cell} for json column`,
);
}
return JSON.stringify(cell);
}
return validationResult.newValue as DataTableColumnJsType;
}
@ -562,8 +590,7 @@ export class DataTableService {
};
}),
columns,
true,
true,
{ includeSystemColumns: true, skipDateTransform: true },
);
const transformedFilters = filterObject.filters.map((filter, index) => {

View File

@ -10,4 +10,5 @@ export const columnTypeToFieldType: Record<string, keyof FieldTypeMap> = {
// eslint-disable-next-line id-denylist
boolean: 'boolean',
date: 'dateTime',
json: 'object',
};