mirror of
https://github.com/n8n-io/n8n.git
synced 2025-02-21 02:56:40 -08:00
perf(core): Add filtering and pagination to GET /workflows
(#6845)
* Initial setup * Specify max paginated items * Simplify * Add tests * Add more tests * Add migrations * Add top-level property * Add field selection * Cleanup * Rename `total` to `count` * More cleanup * Move query logic into `WorkflowRepository` * Create `AbstractRepository` * Cleanup * Fix name * Remove leftover comments * Replace reference * Add default for `rawSkip` * Remove unneeded typing * Switch to `class-validator` * Simplify * Simplify * Type as optional * Make typing more accurate * Fix lint * Use `getOwnPropertyNames` * Use DSL * Set schema at repo level * Cleanup * Remove comment * Refactor repository methods to middleware * Add middleware tests * Remove old test files * Remove generic experiment * Reuse `reportError` * Remove unused type * Cleanup * Improve wording * Reduce diff * Add missing mw * Use `Container.get` * Adjust lint rule * Reorganize into subdir * Remove unused directive * Remove nodes * Silly mistake * Validate take * refactor(core): Adjust index handling in new migrations DSL (no-changelog) (#6876) * refactor(core): Adjust index handling in new migrations DSL (no-changelog) * Account for custom index name * Also for dropping * Fix `select` issue with `relations` * Tighten validation * Ensure `ownerId` is not added when specifying `select`
This commit is contained in:
parent
f8ad543af5
commit
dceff675ec
|
@ -246,7 +246,7 @@ const config = (module.exports = {
|
|||
format: ['PascalCase'],
|
||||
},
|
||||
{
|
||||
selector: ['method', 'function'],
|
||||
selector: ['method', 'function', 'parameter'],
|
||||
format: ['camelCase'],
|
||||
leadingUnderscore: 'allowSingleOrDouble',
|
||||
},
|
||||
|
|
|
@ -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<T, R extends Request, S extends Response>(
|
|||
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';
|
||||
|
|
|
@ -4,11 +4,20 @@ import LazyPromise from 'p-lazy';
|
|||
abstract class IndexOperation extends LazyPromise<void> {
|
||||
abstract execute(queryRunner: QueryRunner): Promise<void>;
|
||||
|
||||
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<void> {
|
|||
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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 */
|
||||
});
|
||||
|
|
|
@ -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']);
|
||||
}
|
||||
}
|
|
@ -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,
|
||||
];
|
||||
|
|
|
@ -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,
|
||||
];
|
||||
|
|
|
@ -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 };
|
||||
|
|
|
@ -1,3 +1,4 @@
|
|||
export * from './auth';
|
||||
export * from './bodyParser';
|
||||
export * from './cors';
|
||||
export * from './listQuery';
|
||||
|
|
21
packages/cli/src/middlewares/listQuery/error.ts
Normal file
21
packages/cli/src/middlewares/listQuery/error.ts
Normal file
|
@ -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}`,
|
||||
);
|
||||
}
|
44
packages/cli/src/middlewares/listQuery/filter.ts
Normal file
44
packages/cli/src/middlewares/listQuery/filter.ts
Normal file
|
@ -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);
|
||||
}
|
||||
};
|
9
packages/cli/src/middlewares/listQuery/index.ts
Normal file
9
packages/cli/src/middlewares/listQuery/index.ts
Normal file
|
@ -0,0 +1,9 @@
|
|||
import { filterListQueryMiddleware } from './filter';
|
||||
import { selectListQueryMiddleware } from './select';
|
||||
import { paginationListQueryMiddleware } from './pagination';
|
||||
|
||||
export const listQueryMiddleware = [
|
||||
filterListQueryMiddleware,
|
||||
selectListQueryMiddleware,
|
||||
paginationListQueryMiddleware,
|
||||
];
|
27
packages/cli/src/middlewares/listQuery/pagination.ts
Normal file
27
packages/cli/src/middlewares/listQuery/pagination.ts
Normal file
|
@ -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();
|
||||
};
|
7
packages/cli/src/middlewares/listQuery/schema.ts
Normal file
7
packages/cli/src/middlewares/listQuery/schema.ts
Normal file
|
@ -0,0 +1,7 @@
|
|||
export class Schema {
|
||||
constructor(private data: unknown = {}) {}
|
||||
|
||||
static get fieldNames(): string[] {
|
||||
return [];
|
||||
}
|
||||
}
|
46
packages/cli/src/middlewares/listQuery/select.ts
Normal file
46
packages/cli/src/middlewares/listQuery/select.ts
Normal file
|
@ -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<Record<string, true>>((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);
|
||||
}
|
||||
};
|
40
packages/cli/src/middlewares/listQuery/workflow.schema.ts
Normal file
40
packages/cli/src/middlewares/listQuery/workflow.schema.ts
Normal file
|
@ -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());
|
||||
}
|
||||
}
|
|
@ -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<string, unknown>;
|
||||
select?: Record<string, true>;
|
||||
skip?: number;
|
||||
take?: number;
|
||||
};
|
||||
|
||||
// ----------------------------------
|
||||
// /credentials
|
||||
// ----------------------------------
|
||||
|
|
|
@ -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');
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
/**
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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<WorkflowEntity>,
|
||||
'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<WorkflowForList[]> {
|
||||
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<WorkflowEntity> = {
|
||||
const DEFAULT_SELECT: FindOptionsSelect<WorkflowEntity> = {
|
||||
id: true,
|
||||
name: true,
|
||||
active: true,
|
||||
createdAt: true,
|
||||
updatedAt: true,
|
||||
};
|
||||
|
||||
const select: FindOptionsSelect<WorkflowEntity> = 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<WorkflowEntity> = {
|
||||
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(
|
||||
|
|
119
packages/cli/test/unit/middleware/listQuery.test.ts
Normal file
119
packages/cli/test/unit/middleware/listQuery.test.ts
Normal file
|
@ -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<ListQueryRequest>;
|
||||
let mockRes: Partial<Response>;
|
||||
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');
|
||||
});
|
||||
});
|
||||
});
|
Loading…
Reference in a new issue