diff --git a/packages/@n8n_io/eslint-config/base.js b/packages/@n8n_io/eslint-config/base.js index bef8d2c9f8..4c53916545 100644 --- a/packages/@n8n_io/eslint-config/base.js +++ b/packages/@n8n_io/eslint-config/base.js @@ -246,7 +246,7 @@ const config = (module.exports = { format: ['PascalCase'], }, { - selector: ['method', 'function'], + selector: ['method', 'function', 'parameter'], format: ['camelCase'], leadingUnderscore: 'allowSingleOrDouble', }, diff --git a/packages/cli/src/ResponseHelper.ts b/packages/cli/src/ResponseHelper.ts index 98089d56a6..968b637fe2 100644 --- a/packages/cli/src/ResponseHelper.ts +++ b/packages/cli/src/ResponseHelper.ts @@ -162,6 +162,12 @@ export function sendErrorResponse(res: Response, error: Error) { export const isUniqueConstraintError = (error: Error) => ['unique', 'duplicate'].some((s) => error.message.toLowerCase().includes(s)); +export function reportError(error: Error) { + if (!(error instanceof ResponseError) || error.httpStatusCode > 404) { + ErrorReporter.error(error); + } +} + /** * A helper function which does not just allow to return Promises it also makes sure that * all the responses have the same format @@ -181,9 +187,7 @@ export function send( if (!res.headersSent) sendSuccessResponse(res, data, raw); } catch (error) { if (error instanceof Error) { - if (!(error instanceof ResponseError) || error.httpStatusCode > 404) { - ErrorReporter.error(error); - } + reportError(error); if (isUniqueConstraintError(error)) { error.message = 'There is already an entry with this name'; diff --git a/packages/cli/src/databases/dsl/Indices.ts b/packages/cli/src/databases/dsl/Indices.ts index 391a9070bf..d6b868e2aa 100644 --- a/packages/cli/src/databases/dsl/Indices.ts +++ b/packages/cli/src/databases/dsl/Indices.ts @@ -4,11 +4,20 @@ import LazyPromise from 'p-lazy'; abstract class IndexOperation extends LazyPromise { abstract execute(queryRunner: QueryRunner): Promise; + get fullTableName() { + return [this.tablePrefix, this.tableName].join(''); + } + + get fullIndexName() { + return ['IDX', `${this.tablePrefix}${this.tableName}`, ...this.columnNames].join('_'); + } + constructor( - protected name: string, + protected tablePrefix: string, protected tableName: string, - protected prefix: string, + protected columnNames: string[], queryRunner: QueryRunner, + protected customIndexName?: string, ) { super((resolve) => { void this.execute(queryRunner).then(resolve); @@ -18,28 +27,27 @@ abstract class IndexOperation extends LazyPromise { export class CreateIndex extends IndexOperation { constructor( - name: string, + tablePrefix: string, tableName: string, - protected columnNames: string[], + columnNames: string[], protected isUnique: boolean, - prefix: string, queryRunner: QueryRunner, + customIndexName?: string, ) { - super(name, tableName, prefix, queryRunner); + super(tablePrefix, tableName, columnNames, queryRunner, customIndexName); } async execute(queryRunner: QueryRunner) { - const { tableName, name, columnNames, prefix, isUnique } = this; + const { columnNames, isUnique } = this; return queryRunner.createIndex( - `${prefix}${tableName}`, - new TableIndex({ name: `IDX_${prefix}${name}`, columnNames, isUnique }), + this.fullTableName, + new TableIndex({ name: this.customIndexName ?? this.fullIndexName, columnNames, isUnique }), ); } } export class DropIndex extends IndexOperation { async execute(queryRunner: QueryRunner) { - const { tableName, name, prefix } = this; - return queryRunner.dropIndex(`${prefix}${tableName}`, `IDX_${prefix}${name}`); + return queryRunner.dropIndex(this.fullTableName, this.customIndexName ?? this.fullIndexName); } } diff --git a/packages/cli/src/databases/dsl/index.ts b/packages/cli/src/databases/dsl/index.ts index 33f50509ea..445a893a38 100644 --- a/packages/cli/src/databases/dsl/index.ts +++ b/packages/cli/src/databases/dsl/index.ts @@ -7,11 +7,19 @@ export const createSchemaBuilder = (tablePrefix: string, queryRunner: QueryRunne column: (name: string) => new Column(name), /* eslint-disable @typescript-eslint/promise-function-async */ // NOTE: Do not add `async` to these functions, as that messes up the lazy-evaluation of LazyPromise - createTable: (name: string) => new CreateTable(name, tablePrefix, queryRunner), - dropTable: (name: string) => new DropTable(name, tablePrefix, queryRunner), - createIndex: (name: string, tableName: string, columnNames: string[], isUnique = false) => - new CreateIndex(name, tableName, columnNames, isUnique, tablePrefix, queryRunner), - dropIndex: (name: string, tableName: string) => - new DropIndex(name, tableName, tablePrefix, queryRunner), + createTable: (tableName: string) => new CreateTable(tableName, tablePrefix, queryRunner), + + dropTable: (tableName: string) => new DropTable(tableName, tablePrefix, queryRunner), + + createIndex: ( + tableName: string, + columnNames: string[], + isUnique = false, + customIndexName?: string, + ) => new CreateIndex(tablePrefix, tableName, columnNames, isUnique, queryRunner, customIndexName), + + dropIndex: (tableName: string, columnNames: string[], customIndexName?: string) => + new DropIndex(tablePrefix, tableName, columnNames, queryRunner, customIndexName), + /* eslint-enable */ }); diff --git a/packages/cli/src/databases/migrations/common/1691088862123-CreateWorkflowNameIndex.ts b/packages/cli/src/databases/migrations/common/1691088862123-CreateWorkflowNameIndex.ts new file mode 100644 index 0000000000..c38cd6067d --- /dev/null +++ b/packages/cli/src/databases/migrations/common/1691088862123-CreateWorkflowNameIndex.ts @@ -0,0 +1,11 @@ +import type { MigrationContext, ReversibleMigration } from '@db/types'; + +export class CreateWorkflowNameIndex1691088862123 implements ReversibleMigration { + async up({ schemaBuilder: { createIndex } }: MigrationContext) { + await createIndex('workflow_entity', ['name']); + } + + async down({ schemaBuilder: { dropIndex } }: MigrationContext) { + await dropIndex('workflow_entity', ['name']); + } +} diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts index c119af2f70..ca606867ae 100644 --- a/packages/cli/src/databases/migrations/mysqldb/index.ts +++ b/packages/cli/src/databases/migrations/mysqldb/index.ts @@ -43,6 +43,7 @@ import { SeparateExecutionData1690000000030 } from './1690000000030-SeparateExec import { FixExecutionDataType1690000000031 } from './1690000000031-FixExecutionDataType'; import { RemoveSkipOwnerSetup1681134145997 } from './1681134145997-RemoveSkipOwnerSetup'; import { RemoveResetPasswordColumns1690000000030 } from '../common/1690000000030-RemoveResetPasswordColumns'; +import { CreateWorkflowNameIndex1691088862123 } from '../common/1691088862123-CreateWorkflowNameIndex'; export const mysqlMigrations: Migration[] = [ InitialMigration1588157391238, @@ -89,4 +90,5 @@ export const mysqlMigrations: Migration[] = [ FixExecutionDataType1690000000031, RemoveSkipOwnerSetup1681134145997, RemoveResetPasswordColumns1690000000030, + CreateWorkflowNameIndex1691088862123, ]; diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts index 1f5b5e6457..554bb13114 100644 --- a/packages/cli/src/databases/migrations/postgresdb/index.ts +++ b/packages/cli/src/databases/migrations/postgresdb/index.ts @@ -41,6 +41,7 @@ import { SeparateExecutionData1690000000020 } from './1690000000020-SeparateExec import { RemoveSkipOwnerSetup1681134145997 } from './1681134145997-RemoveSkipOwnerSetup'; import { RemoveResetPasswordColumns1690000000030 } from '../common/1690000000030-RemoveResetPasswordColumns'; import { AddMissingPrimaryKeyOnExecutionData1690787606731 } from './1690787606731-AddMissingPrimaryKeyOnExecutionData'; +import { CreateWorkflowNameIndex1691088862123 } from '../common/1691088862123-CreateWorkflowNameIndex'; export const postgresMigrations: Migration[] = [ InitialMigration1587669153312, @@ -85,4 +86,5 @@ export const postgresMigrations: Migration[] = [ RemoveSkipOwnerSetup1681134145997, RemoveResetPasswordColumns1690000000030, AddMissingPrimaryKeyOnExecutionData1690787606731, + CreateWorkflowNameIndex1691088862123, ]; diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index 0b1eaf0781..eeb0cc4995 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -40,6 +40,7 @@ import { SeparateExecutionData1690000000010 } from './1690000000010-SeparateExec import { RemoveSkipOwnerSetup1681134145997 } from './1681134145997-RemoveSkipOwnerSetup'; import { FixMissingIndicesFromStringIdMigration1690000000020 } from './1690000000020-FixMissingIndicesFromStringIdMigration'; import { RemoveResetPasswordColumns1690000000030 } from './1690000000030-RemoveResetPasswordColumns'; +import { CreateWorkflowNameIndex1691088862123 } from '../common/1691088862123-CreateWorkflowNameIndex'; const sqliteMigrations: Migration[] = [ InitialMigration1588102412422, @@ -83,6 +84,7 @@ const sqliteMigrations: Migration[] = [ RemoveSkipOwnerSetup1681134145997, FixMissingIndicesFromStringIdMigration1690000000020, RemoveResetPasswordColumns1690000000030, + CreateWorkflowNameIndex1691088862123, ]; export { sqliteMigrations }; diff --git a/packages/cli/src/middlewares/index.ts b/packages/cli/src/middlewares/index.ts index e578155a0a..c9390709df 100644 --- a/packages/cli/src/middlewares/index.ts +++ b/packages/cli/src/middlewares/index.ts @@ -1,3 +1,4 @@ export * from './auth'; export * from './bodyParser'; export * from './cors'; +export * from './listQuery'; diff --git a/packages/cli/src/middlewares/listQuery/error.ts b/packages/cli/src/middlewares/listQuery/error.ts new file mode 100644 index 0000000000..12deebfab7 --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/error.ts @@ -0,0 +1,21 @@ +import { BadRequestError } from '@/ResponseHelper'; +import { LoggerProxy } from 'n8n-workflow'; +import * as utils from '@/utils'; + +export function handleListQueryError( + paramName: 'filter' | 'select', + paramValue: string, + maybeError: unknown, +) { + const error = utils.toError(maybeError); + + LoggerProxy.error(`Invalid "${paramName}" query parameter`, { + paramName, + paramValue, + error, + }); + + throw new BadRequestError( + `Invalid "${paramName}" query parameter: ${paramValue}. Error: ${error.message}`, + ); +} diff --git a/packages/cli/src/middlewares/listQuery/filter.ts b/packages/cli/src/middlewares/listQuery/filter.ts new file mode 100644 index 0000000000..931719e12b --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/filter.ts @@ -0,0 +1,44 @@ +import { jsonParse } from 'n8n-workflow'; +import { handleListQueryError } from './error'; +import { WorkflowSchema } from './workflow.schema'; +import type { ListQueryRequest } from '@/requests'; +import type { RequestHandler } from 'express'; +import type { Schema } from './schema'; + +function toQueryFilter(rawFilter: string, schema: typeof Schema) { + const parsedFilter = new schema( + jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }), + ); + + return Object.fromEntries( + Object.entries(parsedFilter) + .filter(([_, value]) => value !== undefined) + .map(([key, _]: [keyof Schema, unknown]) => [key, parsedFilter[key]]), + ); +} + +export const filterListQueryMiddleware: RequestHandler = (req: ListQueryRequest, res, next) => { + const { filter: rawFilter } = req.query; + + if (!rawFilter) return next(); + + let schema; + + if (req.baseUrl.endsWith('workflows')) { + schema = WorkflowSchema; + } else { + return next(); + } + + try { + const filter = toQueryFilter(rawFilter, schema); + + if (Object.keys(filter).length === 0) return next(); + + req.listQueryOptions = { ...req.listQueryOptions, filter }; + + next(); + } catch (error) { + handleListQueryError('filter', rawFilter, error); + } +}; diff --git a/packages/cli/src/middlewares/listQuery/index.ts b/packages/cli/src/middlewares/listQuery/index.ts new file mode 100644 index 0000000000..0c8e2c7427 --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/index.ts @@ -0,0 +1,9 @@ +import { filterListQueryMiddleware } from './filter'; +import { selectListQueryMiddleware } from './select'; +import { paginationListQueryMiddleware } from './pagination'; + +export const listQueryMiddleware = [ + filterListQueryMiddleware, + selectListQueryMiddleware, + paginationListQueryMiddleware, +]; diff --git a/packages/cli/src/middlewares/listQuery/pagination.ts b/packages/cli/src/middlewares/listQuery/pagination.ts new file mode 100644 index 0000000000..cfe38e0032 --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/pagination.ts @@ -0,0 +1,27 @@ +import type { ListQueryRequest } from '@/requests'; +import { isIntegerString } from '@/utils'; +import type { RequestHandler } from 'express'; + +function toPaginationOptions(rawTake: string, rawSkip: string) { + const MAX_ITEMS = 50; + + if ([rawTake, rawSkip].some((i) => !isIntegerString(i))) { + throw new Error('Parameter take or skip is not an integer string'); + } + + const [take, skip] = [rawTake, rawSkip].map((o) => parseInt(o, 10)); + + return { skip, take: Math.min(take, MAX_ITEMS) }; +} + +export const paginationListQueryMiddleware: RequestHandler = (req: ListQueryRequest, res, next) => { + const { take: rawTake, skip: rawSkip = '0' } = req.query; + + if (!rawTake) return next(); + + const { take, skip } = toPaginationOptions(rawTake, rawSkip); + + req.listQueryOptions = { ...req.listQueryOptions, take, skip }; + + next(); +}; diff --git a/packages/cli/src/middlewares/listQuery/schema.ts b/packages/cli/src/middlewares/listQuery/schema.ts new file mode 100644 index 0000000000..bfd4e84f5c --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/schema.ts @@ -0,0 +1,7 @@ +export class Schema { + constructor(private data: unknown = {}) {} + + static get fieldNames(): string[] { + return []; + } +} diff --git a/packages/cli/src/middlewares/listQuery/select.ts b/packages/cli/src/middlewares/listQuery/select.ts new file mode 100644 index 0000000000..ff342a3be1 --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/select.ts @@ -0,0 +1,46 @@ +import { handleListQueryError } from './error'; +import { jsonParse } from 'n8n-workflow'; +import { WorkflowSchema } from './workflow.schema'; +import * as utils from '@/utils'; +import type { ListQueryRequest } from '@/requests'; +import type { RequestHandler } from 'express'; +import type { Schema } from '@/middlewares/listQuery/schema'; + +function toQuerySelect(rawSelect: string, schema: typeof Schema) { + const asArr = jsonParse(rawSelect, { errorMessage: 'Failed to parse select JSON' }); + + if (!utils.isStringArray(asArr)) { + throw new Error('Parsed select is not a string array'); + } + + return asArr.reduce>((acc, field) => { + if (!schema.fieldNames.includes(field)) return acc; + return (acc[field] = true), acc; + }, {}); +} + +export const selectListQueryMiddleware: RequestHandler = (req: ListQueryRequest, res, next) => { + const { select: rawSelect } = req.query; + + if (!rawSelect) return next(); + + let schema; + + if (req.baseUrl.endsWith('workflows')) { + schema = WorkflowSchema; + } else { + return next(); + } + + try { + const select = toQuerySelect(rawSelect, schema); + + if (Object.keys(select).length === 0) return next(); + + req.listQueryOptions = { ...req.listQueryOptions, select }; + + next(); + } catch (error) { + handleListQueryError('select', rawSelect, error); + } +}; diff --git a/packages/cli/src/middlewares/listQuery/workflow.schema.ts b/packages/cli/src/middlewares/listQuery/workflow.schema.ts new file mode 100644 index 0000000000..cf0dc1dba7 --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/workflow.schema.ts @@ -0,0 +1,40 @@ +import { Schema } from '@/middlewares/listQuery/schema'; +import { validateSync, IsOptional, IsString, IsBoolean, IsDateString } from 'class-validator'; + +export class WorkflowSchema extends Schema { + constructor(data: unknown = {}) { + super(); + Object.assign(this, data); + + // strip out unknown fields + const result = validateSync(this, { whitelist: true }); + + if (result.length > 0) { + throw new Error('Parsed filter does not fit the schema'); + } + } + + @IsOptional() + @IsString() + id?: string = undefined; + + @IsOptional() + @IsString() + name?: string = undefined; + + @IsOptional() + @IsBoolean() + active?: boolean = undefined; + + @IsOptional() + @IsDateString() + createdAt?: Date = undefined; + + @IsOptional() + @IsDateString() + updatedAt?: Date = undefined; + + static get fieldNames() { + return Object.getOwnPropertyNames(new WorkflowSchema()); + } +} diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index f1c0636557..dfe295cea9 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -100,8 +100,6 @@ export declare namespace WorkflowRequest { type NewName = AuthenticatedRequest<{}, {}, {}, { name?: string }>; - type GetAll = AuthenticatedRequest<{}, {}, {}, { filter: string }>; - type GetAllActive = AuthenticatedRequest; type GetAllActivationErrors = Get; @@ -111,6 +109,28 @@ export declare namespace WorkflowRequest { type Share = AuthenticatedRequest<{ workflowId: string }, {}, { shareWithIds: string[] }>; } +// ---------------------------------- +// list query +// ---------------------------------- + +export type ListQueryRequest = AuthenticatedRequest<{}, {}, {}, ListQueryParams> & { + listQueryOptions?: ListQueryOptions; +}; + +type ListQueryParams = { + filter?: string; + skip?: string; + take?: string; + select?: string; +}; + +export type ListQueryOptions = { + filter?: Record; + select?: Record; + skip?: number; + take?: number; +}; + // ---------------------------------- // /credentials // ---------------------------------- diff --git a/packages/cli/src/services/cache.service.ts b/packages/cli/src/services/cache.service.ts index 411ec55193..641e65050b 100644 --- a/packages/cli/src/services/cache.service.ts +++ b/packages/cli/src/services/cache.service.ts @@ -213,7 +213,6 @@ export class CacheService extends EventEmitter { ([key, value]) => value !== undefined && value !== null && key && key.length > 0, ); if (this.isRedisCache()) { - // eslint-disable-next-line @typescript-eslint/naming-convention nonNullValues.forEach(([_key, value]) => { if (!(this.cache as RedisCache)?.store?.isCacheable(value)) { throw new Error('Value is not cacheable'); diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts index c4a094ef1e..0912b02582 100644 --- a/packages/cli/src/utils.ts +++ b/packages/cli/src/utils.ts @@ -87,3 +87,13 @@ export const webhookNotFoundErrorMessage = ( return `The requested webhook "${webhookPath}" is not registered.`; } }; + +export const toError = (maybeError: unknown) => + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + maybeError instanceof Error ? maybeError : new Error(`${maybeError}`); + +export function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((item) => typeof item === 'string'); +} + +export const isIntegerString = (value: string) => /^\d+$/.test(value); diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index 8332bb63c3..dc5a853883 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -6,7 +6,7 @@ import * as WorkflowHelpers from '@/WorkflowHelpers'; import config from '@/config'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { validateEntity } from '@/GenericHelpers'; -import type { WorkflowRequest } from '@/requests'; +import type { ListQueryRequest, WorkflowRequest } from '@/requests'; import { isSharingEnabled, rightDiff } from '@/UserManagement/UserManagementHelper'; import { EEWorkflowsService as EEWorkflows } from './workflows.services.ee'; import { ExternalHooks } from '@/ExternalHooks'; @@ -20,6 +20,8 @@ import { In } from 'typeorm'; import { Container } from 'typedi'; import { InternalHooks } from '@/InternalHooks'; import { RoleService } from '@/services/role.service'; +import * as utils from '@/utils'; +import { listQueryMiddleware } from '@/middlewares'; import { TagRepository } from '@/databases/repositories'; // eslint-disable-next-line @typescript-eslint/naming-convention @@ -203,17 +205,27 @@ EEWorkflowController.post( */ EEWorkflowController.get( '/', - ResponseHelper.send(async (req: WorkflowRequest.GetAll) => { - const [workflows, workflowOwnerRole] = await Promise.all([ - EEWorkflows.getMany(req.user, req.query.filter), - Container.get(RoleService).findWorkflowOwnerRole(), - ]); + listQueryMiddleware, + async (req: ListQueryRequest, res: express.Response) => { + try { + const [workflows, count] = await EEWorkflows.getMany(req.user, req.listQueryOptions); - return workflows.map((workflow) => { - EEWorkflows.addOwnerId(workflow, workflowOwnerRole); - return workflow; - }); - }), + let data; + + if (req.listQueryOptions?.select) { + data = workflows; + } else { + const role = await Container.get(RoleService).findWorkflowOwnerRole(); + data = workflows.map((w) => EEWorkflows.addOwnerId(w, role)); + } + + res.json({ count, data }); + } catch (maybeError) { + const error = utils.toError(maybeError); + ResponseHelper.reportError(error); + ResponseHelper.sendErrorResponse(res, error); + } + }, ); EEWorkflowController.patch( diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 83b539ad97..ac53c2faed 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -15,7 +15,7 @@ import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import { getLogger } from '@/Logger'; -import type { WorkflowRequest } from '@/requests'; +import type { ListQueryRequest, WorkflowRequest } from '@/requests'; import { isBelowOnboardingThreshold } from '@/WorkflowHelpers'; import { EEWorkflowController } from './workflows.controller.ee'; import { WorkflowsService } from './workflows.services'; @@ -24,6 +24,8 @@ import { In } from 'typeorm'; import { Container } from 'typedi'; import { InternalHooks } from '@/InternalHooks'; import { RoleService } from '@/services/role.service'; +import * as utils from '@/utils'; +import { listQueryMiddleware } from '@/middlewares'; import { TagRepository } from '@/databases/repositories'; export const workflowsController = express.Router(); @@ -116,9 +118,18 @@ workflowsController.post( */ workflowsController.get( '/', - ResponseHelper.send(async (req: WorkflowRequest.GetAll) => { - return WorkflowsService.getMany(req.user, req.query.filter); - }), + listQueryMiddleware, + async (req: ListQueryRequest, res: express.Response) => { + try { + const [data, count] = await WorkflowsService.getMany(req.user, req.listQueryOptions); + + res.json({ count, data }); + } catch (maybeError) { + const error = utils.toError(maybeError); + ResponseHelper.reportError(error); + ResponseHelper.sendErrorResponse(res, error); + } + }, ); /** diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts index ac0e9fc290..f4dfcb7cb9 100644 --- a/packages/cli/src/workflows/workflows.services.ee.ts +++ b/packages/cli/src/workflows/workflows.services.ee.ts @@ -88,11 +88,12 @@ export class EEWorkflowsService extends WorkflowsService { return transaction.save(newSharedWorkflows); } - static addOwnerId(workflow: WorkflowForList, workflowOwnerRole: Role): void { + static addOwnerId(workflow: WorkflowForList, workflowOwnerRole: Role) { const ownerId = workflow.shared?.find(({ roleId }) => String(roleId) === workflowOwnerRole.id) ?.userId; workflow.ownedBy = ownerId ? { id: ownerId } : null; delete workflow.shared; + return workflow; } static addOwnerAndSharings(workflow: WorkflowWithSharingsAndCredentials): void { diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index 24c4cc8554..176f32bd77 100644 --- a/packages/cli/src/workflows/workflows.services.ts +++ b/packages/cli/src/workflows/workflows.services.ts @@ -1,9 +1,8 @@ import { Container } from 'typedi'; -import { validate as jsonSchemaValidate } from 'jsonschema'; -import type { INode, IPinData, JsonObject } from 'n8n-workflow'; -import { NodeApiError, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow'; -import type { FindOptionsSelect, FindOptionsWhere, UpdateResult } from 'typeorm'; -import { In } from 'typeorm'; +import type { INode, IPinData } from 'n8n-workflow'; +import { NodeApiError, LoggerProxy, Workflow } from 'n8n-workflow'; +import type { FindManyOptions, FindOptionsSelect, FindOptionsWhere, UpdateResult } from 'typeorm'; +import { In, Like } from 'typeorm'; import pick from 'lodash/pick'; import { v4 as uuid } from 'uuid'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; @@ -18,7 +17,7 @@ import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import * as TagHelpers from '@/TagHelpers'; -import type { WorkflowRequest } from '@/requests'; +import type { ListQueryOptions, WorkflowRequest } from '@/requests'; import type { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; import { WorkflowRunner } from '@/WorkflowRunner'; @@ -26,25 +25,9 @@ import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData' import { TestWebhooks } from '@/TestWebhooks'; import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { isSharingEnabled, whereClause } from '@/UserManagement/UserManagementHelper'; -import type { WorkflowForList } from '@/workflows/workflows.types'; import { InternalHooks } from '@/InternalHooks'; - -export type IGetWorkflowsQueryFilter = Pick< - FindOptionsWhere, - 'id' | 'name' | 'active' ->; - -const schemaGetWorkflowsQueryFilter = { - $id: '/IGetWorkflowsQueryFilter', - type: 'object', - properties: { - id: { anyOf: [{ type: 'integer' }, { type: 'string' }] }, - name: { type: 'string' }, - active: { type: 'boolean' }, - }, -}; - -const allowedWorkflowsQueryFilterFields = Object.keys(schemaGetWorkflowsQueryFilter.properties); +import type { WorkflowForList } from './workflows.types'; +import { WorkflowRepository } from '@/databases/repositories'; export class WorkflowsService { static async getSharing( @@ -116,71 +99,70 @@ export class WorkflowsService { return getSharedWorkflowIds(user, roles); } - static async getMany(user: User, rawFilter: string): Promise { + static async getMany( + user: User, + options?: ListQueryOptions, + ): Promise<[WorkflowForList[], number]> { const sharedWorkflowIds = await this.getWorkflowIdsForUser(user, ['owner']); if (sharedWorkflowIds.length === 0) { // return early since without shared workflows there can be no hits // (note: getSharedWorkflowIds() returns _all_ workflow ids for global owners) - return []; + return [[], 0]; } - let filter: IGetWorkflowsQueryFilter = {}; - if (rawFilter) { - try { - const filterJson: JsonObject = jsonParse(rawFilter); - if (filterJson) { - Object.keys(filterJson).map((key) => { - if (!allowedWorkflowsQueryFilterFields.includes(key)) delete filterJson[key]; - }); - if (jsonSchemaValidate(filterJson, schemaGetWorkflowsQueryFilter).valid) { - filter = filterJson as IGetWorkflowsQueryFilter; - } - } - } catch (error) { - LoggerProxy.error('Failed to parse filter', { - userId: user.id, - filter, - }); - throw new ResponseHelper.InternalServerError( - 'Parameter "filter" contained invalid JSON string.', - ); - } - } + const filter = options?.filter ?? {}; // safeguard against querying ids not shared with the user const workflowId = filter?.id?.toString(); if (workflowId !== undefined && !sharedWorkflowIds.includes(workflowId)) { LoggerProxy.verbose(`User ${user.id} attempted to query non-shared workflow ${workflowId}`); - return []; + return [[], 0]; } - const select: FindOptionsSelect = { + const DEFAULT_SELECT: FindOptionsSelect = { id: true, name: true, active: true, createdAt: true, updatedAt: true, }; + + const select: FindOptionsSelect = options?.select ?? DEFAULT_SELECT; + const relations: string[] = []; - if (!config.getEnv('workflowTagsDisabled')) { + const isDefaultSelect = options?.select === undefined; + + if (isDefaultSelect && !config.getEnv('workflowTagsDisabled')) { relations.push('tags'); select.tags = { id: true, name: true }; } - if (isSharingEnabled()) { + if (isDefaultSelect && isSharingEnabled()) { relations.push('shared'); select.shared = { userId: true, roleId: true }; select.versionId = true; } filter.id = In(sharedWorkflowIds); - return Db.collections.Workflow.find({ + + if (typeof filter.name === 'string' && filter.name !== '') { + filter.name = Like(`%${filter.name}%`); + } + + const findManyOptions: FindManyOptions = { select, relations, where: filter, - order: { updatedAt: 'DESC' }, - }); + order: { updatedAt: 'ASC' }, + }; + + if (options?.take) { + findManyOptions.skip = options.skip; + findManyOptions.take = options.take; + } + + return Container.get(WorkflowRepository).findAndCount(findManyOptions); } static async update( diff --git a/packages/cli/test/unit/middleware/listQuery.test.ts b/packages/cli/test/unit/middleware/listQuery.test.ts new file mode 100644 index 0000000000..bee36a904d --- /dev/null +++ b/packages/cli/test/unit/middleware/listQuery.test.ts @@ -0,0 +1,119 @@ +import { filterListQueryMiddleware } from '@/middlewares/listQuery/filter'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; +import type { Request, Response, NextFunction } from 'express'; +import type { ListQueryRequest } from '@/requests'; +import { selectListQueryMiddleware } from '@/middlewares/listQuery/select'; +import { paginationListQueryMiddleware } from '@/middlewares/listQuery/pagination'; + +describe('List query middleware', () => { + let mockReq: Partial; + let mockRes: Partial; + let nextFn: NextFunction = jest.fn(); + let args: [Request, Response, NextFunction]; + + beforeEach(() => { + LoggerProxy.init(getLogger()); + mockReq = { baseUrl: '/rest/workflows' }; + args = [mockReq as Request, mockRes as Response, nextFn]; + jest.restoreAllMocks(); + }); + + describe('Query filter', () => { + test('should parse valid filter', () => { + mockReq.query = { filter: '{ "name": "My Workflow" }' }; + filterListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ filter: { name: 'My Workflow' } }); + expect(nextFn).toBeCalledTimes(1); + }); + + test('should ignore invalid filter', () => { + mockReq.query = { filter: '{ "name": "My Workflow", "foo": "bar" }' }; + filterListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ filter: { name: 'My Workflow' } }); + expect(nextFn).toBeCalledTimes(1); + }); + + test('should throw on invalid JSON', () => { + mockReq.query = { filter: '{ "name" : "My Workflow"' }; + const call = () => filterListQueryMiddleware(...args); + + expect(call).toThrowError('Failed to parse filter JSON'); + }); + }); + + describe('Query select', () => { + test('should parse valid select', () => { + mockReq.query = { select: '["name", "id"]' }; + selectListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ select: { name: true, id: true } }); + expect(nextFn).toBeCalledTimes(1); + }); + + test('ignore invalid select', () => { + mockReq.query = { select: '["name", "foo"]' }; + selectListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ select: { name: true } }); + expect(nextFn).toBeCalledTimes(1); + }); + + test('throw on invalid JSON', () => { + mockReq.query = { select: '["name"' }; + const call = () => selectListQueryMiddleware(...args); + + expect(call).toThrowError('Failed to parse select JSON'); + }); + + test('throw on non-string-array JSON for select', () => { + mockReq.query = { select: '"name"' }; + const call = () => selectListQueryMiddleware(...args); + + expect(call).toThrowError('Parsed select is not a string array'); + }); + }); + + describe('Query pagination', () => { + test('should parse valid pagination', () => { + mockReq.query = { skip: '1', take: '2' }; + paginationListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ skip: 1, take: 2 }); + expect(nextFn).toBeCalledTimes(1); + }); + + test('should ignore skip without take', () => { + mockReq.query = { skip: '1' }; + paginationListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toBeUndefined(); + expect(nextFn).toBeCalledTimes(1); + }); + + test('should default skip to 0', () => { + mockReq.query = { take: '2' }; + paginationListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ skip: 0, take: 2 }); + expect(nextFn).toBeCalledTimes(1); + }); + + test('should cap take at 50', () => { + mockReq.query = { take: '51' }; + paginationListQueryMiddleware(...args); + + expect(mockReq.listQueryOptions).toEqual({ skip: 0, take: 50 }); + expect(nextFn).toBeCalledTimes(1); + }); + + test('should throw on non-numeric-integer take', () => { + mockReq.query = { take: '3.2' }; + const call = () => paginationListQueryMiddleware(...args); + + expect(call).toThrowError('Parameter take or skip is not an integer string'); + }); + }); +});