From 7595178740cc7d2c6e5ccc7292b9ef6650ca4c3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 27 Dec 2022 18:11:41 +0100 Subject: [PATCH 01/18] refactor: Use string ids on DB entities instead of transforming between string and number throughout the code --- packages/cli/src/ActiveExecutions.ts | 8 ++- packages/cli/src/ActiveWorkflowRunner.ts | 17 +++--- packages/cli/src/CredentialsHelper.ts | 17 +++--- packages/cli/src/Interfaces.ts | 24 ++++----- packages/cli/src/InternalHooks.ts | 12 ++--- packages/cli/src/PublicApi/types.d.ts | 6 +-- .../credentials/credentials.handler.ts | 4 +- .../credentials/credentials.service.ts | 28 +++------- .../handlers/executions/executions.handler.ts | 4 +- .../handlers/executions/executions.service.ts | 2 +- .../handlers/workflows/workflows.handler.ts | 44 ++++++++-------- .../handlers/workflows/workflows.service.ts | 14 ++--- packages/cli/src/ResponseHelper.ts | 4 +- packages/cli/src/Server.ts | 14 +++-- packages/cli/src/TagHelpers.ts | 5 +- packages/cli/src/TestWebhooks.ts | 4 +- .../src/UserManagement/PermissionChecker.ts | 4 +- .../UserManagement/UserManagementHelper.ts | 4 +- .../cli/src/UserManagement/routes/users.ts | 10 ++-- packages/cli/src/WaitTracker.ts | 6 +-- packages/cli/src/WebhookHelpers.ts | 2 +- .../cli/src/WorkflowExecuteAdditionalData.ts | 39 ++++++-------- packages/cli/src/WorkflowHelpers.ts | 41 ++++++--------- packages/cli/src/WorkflowRunner.ts | 16 +++--- packages/cli/src/WorkflowRunnerProcess.ts | 2 +- packages/cli/src/api/tags.api.ts | 6 +-- packages/cli/src/commands/Interfaces.d.ts | 6 +-- packages/cli/src/commands/execute.ts | 3 +- .../cli/src/commands/export/credentials.ts | 2 +- packages/cli/src/commands/import/workflow.ts | 2 +- .../credentials/credentials.controller.ee.ts | 40 +++++--------- .../src/credentials/credentials.controller.ts | 47 ++++++----------- .../src/credentials/credentials.service.ee.ts | 36 ++++++------- .../src/credentials/credentials.service.ts | 52 ++++++------------- .../databases/entities/CredentialsEntity.ts | 8 +-- .../src/databases/entities/ExecutionEntity.ts | 10 ++-- .../databases/entities/SharedCredentials.ts | 5 +- .../src/databases/entities/SharedWorkflow.ts | 5 +- .../cli/src/databases/entities/TagEntity.ts | 6 +-- .../src/databases/entities/WebhookEntity.ts | 5 +- .../src/databases/entities/WorkflowEntity.ts | 10 ++-- .../databases/entities/WorkflowStatistics.ts | 5 +- ...1630451444017-UpdateWorkflowCredentials.ts | 6 +-- ...1630419189837-UpdateWorkflowCredentials.ts | 6 +-- ...1630330987096-UpdateWorkflowCredentials.ts | 6 +-- .../cli/src/databases/utils/transformers.ts | 5 +- packages/cli/src/events/WorkflowStatistics.ts | 25 ++------- .../executions/executions.controller.ee.ts | 2 +- .../src/executions/executions.controller.ts | 2 +- .../cli/src/executions/executions.service.ts | 17 +++--- packages/cli/src/role/role.service.ts | 5 +- .../src/workflows/workflows.controller.ee.ts | 16 +++--- .../cli/src/workflows/workflows.controller.ts | 9 ++-- .../src/workflows/workflows.services.ee.ts | 33 ++++++------ .../cli/src/workflows/workflows.services.ts | 43 ++++----------- .../test/integration/credentials.ee.test.ts | 2 +- .../cli/test/integration/credentials.test.ts | 11 ++-- .../integration/publicApi/executions.test.ts | 4 +- .../integration/publicApi/workflows.test.ts | 10 ++-- .../cli/test/integration/shared/testDb.ts | 2 +- .../workflows.controller.ee.test.ts | 46 ++++++++-------- packages/cli/test/unit/Events.test.ts | 10 ++-- .../cli/test/unit/PermissionChecker.test.ts | 10 ++-- .../cli/test/unit/WorkflowCredentials.test.ts | 2 +- .../cli/test/unit/WorkflowHelpers.test.ts | 4 +- packages/workflow/src/Interfaces.ts | 6 +-- 66 files changed, 355 insertions(+), 506 deletions(-) diff --git a/packages/cli/src/ActiveExecutions.ts b/packages/cli/src/ActiveExecutions.ts index fa96f4967c0ed..bb7e5a7448b04 100644 --- a/packages/cli/src/ActiveExecutions.ts +++ b/packages/cli/src/ActiveExecutions.ts @@ -55,11 +55,9 @@ export class ActiveExecutions { fullExecutionData.retryOf = executionData.retryOf.toString(); } - if ( - executionData.workflowData.id !== undefined && - WorkflowHelpers.isWorkflowIdValid(executionData.workflowData.id.toString()) - ) { - fullExecutionData.workflowId = executionData.workflowData.id.toString(); + const workflowId = executionData.workflowData.id; + if (workflowId !== undefined && WorkflowHelpers.isWorkflowIdValid(workflowId)) { + fullExecutionData.workflowId = workflowId; } const execution = ResponseHelper.flattenExecutionData(fullExecutionData); diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 901348fb18b8e..2c3ac3d66b7ed 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -113,7 +113,7 @@ export class ActiveWorkflowRunner { workflowId: workflowData.id, }); try { - await this.add(workflowData.id.toString(), 'init', workflowData); + await this.add(workflowData.id, 'init', workflowData); Logger.verbose(`Successfully started workflow "${workflowData.name}"`, { workflowName: workflowData.name, workflowId: workflowData.id, @@ -164,10 +164,7 @@ export class ActiveWorkflowRunner { } const activeWorkflows = await this.getActiveWorkflows(); - activeWorkflowIds = [ - ...activeWorkflowIds, - ...activeWorkflows.map((workflow) => workflow.id.toString()), - ]; + activeWorkflowIds = [...activeWorkflowIds, ...activeWorkflows.map((workflow) => workflow.id)]; // Make sure IDs are unique activeWorkflowIds = Array.from(new Set(activeWorkflowIds)); @@ -279,7 +276,7 @@ export class ActiveWorkflowRunner { const nodeTypes = NodeTypes(); const workflow = new Workflow({ - id: webhook.workflowId.toString(), + id: webhook.workflowId, name: workflowData.name, nodes: workflowData.nodes, connections: workflowData.connections, @@ -712,15 +709,15 @@ export class ActiveWorkflowRunner { `The trigger node "${node.name}" of workflow "${workflowData.name}" failed with the error: "${error.message}". Will try to reactivate.`, { nodeName: node.name, - workflowId: workflowData.id.toString(), + workflowId: workflowData.id, workflowName: workflowData.name, }, ); // Remove the workflow as "active" - await this.activeWorkflows?.remove(workflowData.id.toString()); - this.activationErrors[workflowData.id.toString()] = { + await this.activeWorkflows?.remove(workflowData.id); + this.activationErrors[workflowData.id] = { time: new Date().getTime(), error: { message: error.message, @@ -896,7 +893,7 @@ export class ActiveWorkflowRunner { activationMode: WorkflowActivateMode, workflowData: IWorkflowDb, ): void { - const workflowId = workflowData.id.toString(); + const workflowId = workflowData.id; const workflowName = workflowData.name; const retryFunction = async () => { diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index 1f3824610a39c..01cfebc25f0dd 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -279,7 +279,7 @@ export class CredentialsHelper extends ICredentialsHelper { const credential = userId ? await Db.collections.SharedCredentials.findOneOrFail({ relations: ['credentials'], - where: { credentials: { id: nodeCredential.id, type }, user: { id: userId } }, + where: { credentials: { id: nodeCredential.id, type }, userId }, }).then((shared) => shared.credentials) : await Db.collections.Credentials.findOneOrFail({ id: nodeCredential.id, type }); @@ -290,7 +290,7 @@ export class CredentialsHelper extends ICredentialsHelper { } return new Credentials( - { id: credential.id.toString(), name: credential.name }, + { id: credential.id, name: credential.name }, credential.type, credential.nodesAccess, credential.data, @@ -581,7 +581,7 @@ export class CredentialsHelper extends ICredentialsHelper { position: [0, 0], credentials: { [credentialType]: { - id: credentialsDecrypted.id.toString(), + id: credentialsDecrypted.id, name: credentialsDecrypted.name, }, }, @@ -739,7 +739,7 @@ export class CredentialsHelper extends ICredentialsHelper { * Get a credential if it has been shared with a user. */ export async function getCredentialForUser( - credentialId: string, + credentialsId: string, user: User, ): Promise { const sharedCredential = await Db.collections.SharedCredentials.findOne({ @@ -747,7 +747,7 @@ export async function getCredentialForUser( where: whereClause({ user, entityType: 'credentials', - entityId: credentialId, + entityId: credentialsId, }), }); @@ -760,10 +760,9 @@ export async function getCredentialForUser( * Get a credential without user check */ export async function getCredentialWithoutUser( - credentialId: string, + credentialsId: string, ): Promise { - const credential = await Db.collections.Credentials.findOne(credentialId); - return credential; + return Db.collections.Credentials.findOne(credentialsId); } export function createCredentialsFromCredentialsEntity( @@ -774,5 +773,5 @@ export function createCredentialsFromCredentialsEntity( if (encrypt) { return new Credentials({ id: null, name }, type, nodesAccess); } - return new Credentials({ id: id.toString(), name }, type, nodesAccess, data); + return new Credentials({ id, name }, type, nodesAccess, data); } diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 3ae49bd00d0e4..546edfa1deacc 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -84,7 +84,7 @@ export interface IDatabaseCollections { } export interface IWebhookDb { - workflowId: number | string; + workflowId: string; webhookPath: string; method: string; node: string; @@ -97,14 +97,14 @@ export interface IWebhookDb { // ---------------------------------- export interface ITagDb { - id: number; + id: string; name: string; createdAt: Date; updatedAt: Date; } export interface ITagToImport { - id: string | number; + id: string; name: string; createdAt?: string; updatedAt?: string; @@ -121,12 +121,12 @@ export type ITagWithCountDb = ITagDb & UsageCount; // ---------------------------------- export interface IWorkflowBase extends IWorkflowBaseWorkflow { - id?: number | string; + id?: string; } // Almost identical to editor-ui.Interfaces.ts export interface IWorkflowDb extends IWorkflowBase { - id: number | string; + id: string; tags?: ITagDb[]; } @@ -148,17 +148,13 @@ export interface ICredentialsBase { } export interface ICredentialsDb extends ICredentialsBase, ICredentialsEncrypted { - id: number | string; + id: string; name: string; shared?: SharedCredentials[]; } -export interface ICredentialsResponse extends ICredentialsDb { - id: string; -} - export interface ICredentialsDecryptedDb extends ICredentialsBase, ICredentialsDecrypted { - id: number | string; + id: string; } export interface ICredentialsDecryptedResponse extends ICredentialsDecryptedDb { @@ -169,7 +165,7 @@ export type DatabaseType = 'mariadb' | 'postgresdb' | 'mysqldb' | 'sqlite'; export type SaveExecutionDataType = 'all' | 'none'; export interface IExecutionBase { - id?: number | string; + id?: string; mode: WorkflowExecuteMode; startedAt: Date; stoppedAt?: Date; // empty value means execution is still running @@ -208,7 +204,7 @@ export interface IExecutionFlatted extends IExecutionBase { } export interface IExecutionFlattedDb extends IExecutionBase { - id: number | string; + id: string; data: string; waitTill?: Date | null; workflowData: Omit; @@ -220,7 +216,7 @@ export interface IExecutionFlattedResponse extends IExecutionFlatted { } export interface IExecutionResponseApi { - id: number | string; + id: string; mode: WorkflowExecuteMode; startedAt: Date; stoppedAt?: Date; diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index 626c5e6f7232f..7b31241dda25b 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -114,7 +114,7 @@ export class InternalHooksClass implements IInternalHooksClass { let userRole: 'owner' | 'sharee' | undefined = undefined; if (userId && workflow.id) { - const role = await RoleService.getUserRoleForWorkflow(userId, workflow.id.toString()); + const role = await RoleService.getUserRoleForWorkflow(userId, workflow.id); if (role) { userRole = role.name === 'owner' ? 'owner' : 'sharee'; } @@ -150,7 +150,7 @@ export class InternalHooksClass implements IInternalHooksClass { } const properties: IExecutionTrackProperties = { - workflow_id: workflow.id.toString(), + workflow_id: workflow.id, is_manual: false, version_cli: this.versionCli, success: false, @@ -208,7 +208,7 @@ export class InternalHooksClass implements IInternalHooksClass { let userRole: 'owner' | 'sharee' | undefined = undefined; if (userId) { - const role = await RoleService.getUserRoleForWorkflow(userId, workflow.id.toString()); + const role = await RoleService.getUserRoleForWorkflow(userId, workflow.id); if (role) { userRole = role.name === 'owner' ? 'owner' : 'sharee'; } @@ -216,7 +216,7 @@ export class InternalHooksClass implements IInternalHooksClass { const manualExecEventProperties: ITelemetryTrackProperties = { user_id: userId, - workflow_id: workflow.id.toString(), + workflow_id: workflow.id, status: properties.success ? 'success' : 'failed', error_message: properties.error_message as string, error_node_type: properties.error_node_type, @@ -512,14 +512,14 @@ export class InternalHooksClass implements IInternalHooksClass { */ async onFirstProductionWorkflowSuccess(data: { user_id: string; - workflow_id: string | number; + workflow_id: string; }): Promise { return this.telemetry.track('Workflow first prod success', data, { withPostHog: true }); } async onFirstWorkflowDataLoad(data: { user_id: string; - workflow_id: string | number; + workflow_id: string; node_type: string; node_id: string; credential_type?: string; diff --git a/packages/cli/src/PublicApi/types.d.ts b/packages/cli/src/PublicApi/types.d.ts index 0140f0c4a9c64..4c782bdba384d 100644 --- a/packages/cli/src/PublicApi/types.d.ts +++ b/packages/cli/src/PublicApi/types.d.ts @@ -56,7 +56,7 @@ export declare namespace ExecutionRequest { } >; - type Get = AuthenticatedRequest<{ id: number }, {}, {}, { includeData?: boolean }>; + type Get = AuthenticatedRequest<{ id: string }, {}, {}, { includeData?: boolean }>; type Delete = Get; } @@ -81,9 +81,9 @@ export declare namespace WorkflowRequest { >; type Create = AuthenticatedRequest<{}, {}, WorkflowEntity, {}>; - type Get = AuthenticatedRequest<{ id: number }, {}, {}, {}>; + type Get = AuthenticatedRequest<{ id: string }, {}, {}, {}>; type Delete = Get; - type Update = AuthenticatedRequest<{ id: number }, {}, WorkflowEntity, {}>; + type Update = AuthenticatedRequest<{ id: string }, {}, WorkflowEntity, {}>; type Activate = Get; } diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts index 61561ca449d68..a9a34e403f2e8 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts @@ -39,7 +39,7 @@ export = { const savedCredential = await saveCredential(newCredential, req.user, encryptedData); // LoggerProxy.verbose('New credential created', { - // credentialId: newCredential.id, + // credentialsId: newCredential.id, // ownerId: req.user.id, // }); @@ -77,7 +77,7 @@ export = { } await removeCredential(credential); - credential.id = Number(credentialId); + credential.id = credentialId; return res.json(sanitizeCredentials(credential)); }, diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts index 5d222cf8b142a..24a82ad8419df 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts @@ -1,8 +1,4 @@ -/* eslint-disable no-restricted-syntax */ -/* eslint-disable no-underscore-dangle */ -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ -/* eslint-disable @typescript-eslint/no-unsafe-return */ -import { FindOneOptions } from 'typeorm'; +import type { FindConditions } from 'typeorm'; import { UserSettings, Credentials } from 'n8n-core'; import { IDataObject, INodeProperties, INodePropertyOptions } from 'n8n-workflow'; import * as Db from '@/Db'; @@ -14,29 +10,17 @@ import { ExternalHooks } from '@/ExternalHooks'; import { IDependency, IJsonSchema } from '../../../types'; import { CredentialRequest } from '@/requests'; -export async function getCredentials( - credentialId: number | string, -): Promise { - return Db.collections.Credentials.findOne(credentialId); +export async function getCredentials(credentialsId: string): Promise { + return Db.collections.Credentials.findOne(credentialsId); } export async function getSharedCredentials( userId: string, - credentialId: number | string, + credentialsId: string, relations?: string[], ): Promise { - const options: FindOneOptions = { - where: { - user: { id: userId }, - credentials: { id: credentialId }, - }, - }; - - if (relations) { - options.relations = relations; - } - - return Db.collections.SharedCredentials.findOne(options); + const where: FindConditions = { userId, credentialsId }; + return Db.collections.SharedCredentials.findOne({ where, relations }); } export async function createCredential( diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts index 3ac964d9870db..181eec75a74e5 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts @@ -36,7 +36,7 @@ export = { return res.status(404).json({ message: 'Not Found' }); } - await BinaryDataManager.getInstance().deleteBinaryDataByExecutionId(execution.id.toString()); + await BinaryDataManager.getInstance().deleteBinaryDataByExecutionId(execution.id); await deleteExecution(execution); @@ -110,7 +110,7 @@ export = { const executions = await getExecutions(filters); - const newLastId = !executions.length ? 0 : (executions.slice(-1)[0].id as number); + const newLastId = !executions.length ? 0 : Number(executions.slice(-1)[0].id); filters.lastId = newLastId; diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts index a82fc2f5ddd3c..34159656b7a13 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts @@ -122,7 +122,7 @@ export async function getExecutionsCount(data: { } export async function getExecutionInWorkflows( - id: number, + id: string, workflows: string[], includeData?: boolean, ): Promise { diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts index b6a28fe6cb9ab..af1dcc444d604 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts @@ -1,6 +1,6 @@ import express from 'express'; -import { FindManyOptions, In, ObjectLiteral } from 'typeorm'; +import { FindConditions, FindManyOptions, In } from 'typeorm'; import * as Db from '@/Db'; import * as ActiveWorkflowRunner from '@/ActiveWorkflowRunner'; @@ -60,7 +60,7 @@ export = { async (req: WorkflowRequest.Get, res: express.Response): Promise => { const { id } = req.params; - const sharedWorkflow = await getSharedWorkflow(req.user, id.toString()); + const sharedWorkflow = await getSharedWorkflow(req.user, id); if (!sharedWorkflow) { // user trying to access a workflow he does not own @@ -70,13 +70,13 @@ export = { if (sharedWorkflow.workflow.active) { // deactivate before deleting - await ActiveWorkflowRunner.getInstance().remove(id.toString()); + await ActiveWorkflowRunner.getInstance().remove(id); } await Db.collections.Workflow.delete(id); - void InternalHooksManager.getInstance().onWorkflowDeleted(req.user.id, id.toString(), true); - await ExternalHooks().run('workflow.afterDelete', [id.toString()]); + void InternalHooksManager.getInstance().onWorkflowDeleted(req.user.id, id, true); + await ExternalHooks().run('workflow.afterDelete', [id]); return res.json(sharedWorkflow.workflow); }, @@ -86,7 +86,7 @@ export = { async (req: WorkflowRequest.Get, res: express.Response): Promise => { const { id } = req.params; - const sharedWorkflow = await getSharedWorkflow(req.user, id.toString()); + const sharedWorkflow = await getSharedWorkflow(req.user, id); if (!sharedWorkflow) { // user trying to access a workflow he does not own @@ -111,26 +111,27 @@ export = { let workflows: WorkflowEntity[]; let count: number; - const query: FindManyOptions & { where: ObjectLiteral } = { + const where: FindConditions = { + ...(active !== undefined && { active }), + }; + const query: FindManyOptions = { skip: offset, take: limit, - where: { - ...(active !== undefined && { active }), - }, + where, ...(!config.getEnv('workflowTagsDisabled') && { relations: ['tags'] }), }; if (isInstanceOwner(req.user)) { if (tags) { const workflowIds = await getWorkflowIdsViaTags(parseTagNames(tags)); - Object.assign(query.where, { id: In(workflowIds) }); + Object.assign(where, { id: In(workflowIds) }); } workflows = await getWorkflows(query); count = await getWorkflowsCount(query); } else { - const options: { workflowIds?: number[] } = {}; + const options: { workflowIds?: string[] } = {}; if (tags) { options.workflowIds = await getWorkflowIdsViaTags(parseTagNames(tags)); @@ -147,7 +148,7 @@ export = { const workflowsIds = sharedWorkflows.map((shareWorkflow) => shareWorkflow.workflowId); - Object.assign(query.where, { id: In(workflowsIds) }); + Object.assign(where, { id: In(workflowsIds) }); workflows = await getWorkflows(query); @@ -176,7 +177,7 @@ export = { const updateData = new WorkflowEntity(); Object.assign(updateData, req.body); - const sharedWorkflow = await getSharedWorkflow(req.user, id.toString()); + const sharedWorkflow = await getSharedWorkflow(req.user, id); if (!sharedWorkflow) { // user trying to access a workflow he does not own @@ -196,7 +197,7 @@ export = { if (sharedWorkflow.workflow.active) { // When workflow gets saved always remove it as the triggers could have been // changed and so the changes would not take effect - await workflowRunner.remove(id.toString()); + await workflowRunner.remove(id); } try { @@ -209,7 +210,7 @@ export = { if (sharedWorkflow.workflow.active) { try { - await workflowRunner.add(sharedWorkflow.workflowId.toString(), 'update'); + await workflowRunner.add(sharedWorkflow.workflowId, 'update'); } catch (error) { if (error instanceof Error) { return res.status(400).json({ message: error.message }); @@ -230,7 +231,7 @@ export = { async (req: WorkflowRequest.Activate, res: express.Response): Promise => { const { id } = req.params; - const sharedWorkflow = await getSharedWorkflow(req.user, id.toString()); + const sharedWorkflow = await getSharedWorkflow(req.user, id); if (!sharedWorkflow) { // user trying to access a workflow he does not own @@ -240,10 +241,7 @@ export = { if (!sharedWorkflow.workflow.active) { try { - await ActiveWorkflowRunner.getInstance().add( - sharedWorkflow.workflowId.toString(), - 'activate', - ); + await ActiveWorkflowRunner.getInstance().add(sharedWorkflow.workflowId, 'activate'); } catch (error) { if (error instanceof Error) { return res.status(400).json({ message: error.message }); @@ -267,7 +265,7 @@ export = { async (req: WorkflowRequest.Activate, res: express.Response): Promise => { const { id } = req.params; - const sharedWorkflow = await getSharedWorkflow(req.user, id.toString()); + const sharedWorkflow = await getSharedWorkflow(req.user, id); if (!sharedWorkflow) { // user trying to access a workflow he does not own @@ -278,7 +276,7 @@ export = { const workflowRunner = ActiveWorkflowRunner.getInstance(); if (sharedWorkflow.workflow.active) { - await workflowRunner.remove(sharedWorkflow.workflowId.toString()); + await workflowRunner.remove(sharedWorkflow.workflowId); await setWorkflowAsInactive(sharedWorkflow.workflow); diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts index a831858c1ed0a..dbebeb5766c53 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts @@ -20,7 +20,7 @@ export async function getSharedWorkflowIds(user: User): Promise { where: { user }, }); - return sharedWorkflows.map(({ workflowId }) => workflowId.toString()); + return sharedWorkflows.map(({ workflowId }) => workflowId); } export async function getSharedWorkflow( @@ -30,7 +30,7 @@ export async function getSharedWorkflow( return Db.collections.SharedWorkflow.findOne({ where: { ...(!isInstanceOwner(user) && { user }), - ...(workflowId && { workflow: { id: workflowId } }), + ...(workflowId && { workflowId }), }, relations: [...insertIf(!config.getEnv('workflowTagsDisabled'), ['workflow.tags']), 'workflow'], }); @@ -40,19 +40,19 @@ export async function getSharedWorkflows( user: User, options: { relations?: string[]; - workflowIds?: number[]; + workflowIds?: string[]; }, ): Promise { return Db.collections.SharedWorkflow.find({ where: { ...(!isInstanceOwner(user) && { user }), - ...(options.workflowIds && { workflow: { id: In(options.workflowIds) } }), + ...(options.workflowIds && { workflowId: In(options.workflowIds) }), }, ...(options.relations && { relations: options.relations }), }); } -export async function getWorkflowById(id: number): Promise { +export async function getWorkflowById(id: string): Promise { return Db.collections.Workflow.findOne({ where: { id }, }); @@ -62,7 +62,7 @@ export async function getWorkflowById(id: number): Promise { +export async function getWorkflowIdsViaTags(tags: string[]): Promise { const dbTags = await Db.collections.Tag.find({ where: { name: In(tags) }, relations: ['workflows'], @@ -118,7 +118,7 @@ export async function getWorkflowsCount(options: FindManyOptions } export async function updateWorkflow( - workflowId: number, + workflowId: string, updateData: WorkflowEntity, ): Promise { return Db.collections.Workflow.update(workflowId, updateData); diff --git a/packages/cli/src/ResponseHelper.ts b/packages/cli/src/ResponseHelper.ts index f9a9a611b63ab..3c1899ae894f6 100644 --- a/packages/cli/src/ResponseHelper.ts +++ b/packages/cli/src/ResponseHelper.ts @@ -225,7 +225,7 @@ export function flattenExecutionData(fullExecutionData: IExecutionDb): IExecutio }; if (fullExecutionData.id !== undefined) { - returnData.id = fullExecutionData.id.toString(); + returnData.id = fullExecutionData.id; } if (fullExecutionData.retryOf !== undefined) { @@ -246,7 +246,7 @@ export function flattenExecutionData(fullExecutionData: IExecutionDb): IExecutio */ export function unflattenExecutionData(fullExecutionData: IExecutionFlattedDb): IExecutionResponse { const returnData: IExecutionResponse = { - id: fullExecutionData.id.toString(), + id: fullExecutionData.id, workflowData: fullExecutionData.workflowData as IWorkflowDb, data: parse(fullExecutionData.data), mode: fullExecutionData.mode, diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index eb0728f6b2bfb..59e7c658587fc 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -980,8 +980,7 @@ class App { `/${this.restEndpoint}/active`, ResponseHelper.send(async (req: WorkflowRequest.GetAllActive) => { const activeWorkflows = await this.activeWorkflowRunner.getActiveWorkflows(req.user); - - return activeWorkflows.map(({ id }) => id.toString()); + return activeWorkflows.map(({ id }) => id); }), ); @@ -1361,22 +1360,21 @@ class App { for (const data of executingWorkflows) { if ( (filter.workflowId !== undefined && filter.workflowId !== data.workflowId) || - (data.workflowId !== undefined && - !sharedWorkflowIds.includes(data.workflowId.toString())) + (data.workflowId !== undefined && !sharedWorkflowIds.includes(data.workflowId)) ) { continue; } returnData.push({ - id: data.id.toString(), - workflowId: data.workflowId === undefined ? '' : data.workflowId.toString(), + id: data.id, + workflowId: data.workflowId === undefined ? '' : data.workflowId, mode: data.mode, retryOf: data.retryOf, startedAt: new Date(data.startedAt), }); } - returnData.sort((a, b) => parseInt(b.id, 10) - parseInt(a.id, 10)); + returnData.sort((a, b) => Number(b.id) - Number(a.id)); return returnData; }, @@ -1430,7 +1428,7 @@ class App { const currentJobs = await Queue.getInstance().getJobs(['active', 'waiting']); - const job = currentJobs.find((job) => job.data.executionId.toString() === req.params.id); + const job = currentJobs.find((job) => job.data.executionId === req.params.id); if (!job) { throw new Error(`Could not stop "${req.params.id}" as it is no longer in queue.`); diff --git a/packages/cli/src/TagHelpers.ts b/packages/cli/src/TagHelpers.ts index fef8f8f59e1cd..fc89e5d3b7a00 100644 --- a/packages/cli/src/TagHelpers.ts +++ b/packages/cli/src/TagHelpers.ts @@ -18,7 +18,7 @@ export function sortByRequestOrder( { requestOrder }: { requestOrder: string[] }, ) { const tagMap = tags.reduce>((acc, tag) => { - acc[tag.id.toString()] = tag; + acc[tag.id] = tag; return acc; }, {}); @@ -50,6 +50,7 @@ export async function getTagsWithCountDb(tablePrefix: string): Promise { tagsWithCount.forEach((tag) => { + // NOTE: since this code doesn't use the DB entities, we need to stringify the IDs manually // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call tag.id = tag.id.toString(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access @@ -102,7 +103,7 @@ const findOrCreateTag = async ( // Assume tag is identical if createdAt date is the same to preserve a changed tag name const identicalMatch = tagsEntities.find( (existingTag) => - existingTag.id.toString() === importTag.id.toString() && + existingTag.id === importTag.id && existingTag.createdAt && importTag.createdAt && existingTag.createdAt.getTime() === new Date(importTag.createdAt).getTime(), diff --git a/packages/cli/src/TestWebhooks.ts b/packages/cli/src/TestWebhooks.ts index a146907e37322..38cd5a61b3317 100644 --- a/packages/cli/src/TestWebhooks.ts +++ b/packages/cli/src/TestWebhooks.ts @@ -212,7 +212,7 @@ export class TestWebhooks { // Remove test-webhooks automatically if they do not get called (after 120 seconds) const timeout = setTimeout(() => { - this.cancelTestWebhook(workflowData.id.toString()); + this.cancelTestWebhook(workflowData.id); }, 120000); let key: string; @@ -259,7 +259,7 @@ export class TestWebhooks { for (const webhookKey of Object.keys(this.testWebhookData)) { const webhookData = this.testWebhookData[webhookKey]; - if (webhookData.workflowData.id.toString() !== workflowId) { + if (webhookData.workflowData.id !== workflowId) { // eslint-disable-next-line no-continue continue; } diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts index db473bfe2b295..332994ffe1ff7 100644 --- a/packages/cli/src/UserManagement/PermissionChecker.ts +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -42,7 +42,7 @@ export class PermissionChecker { if (workflow.id && isSharingEnabled()) { const workflowSharings = await Db.collections.SharedWorkflow.find({ relations: ['workflow'], - where: { workflow: { id: Number(workflow.id) } }, + where: { workflowId: workflow.id }, }); workflowUserIds = workflowSharings.map((s) => s.userId); } @@ -58,7 +58,7 @@ export class PermissionChecker { where: credentialsWhere, }); - const accessibleCredIds = credentialSharings.map((s) => s.credentialsId.toString()); + const accessibleCredIds = credentialSharings.map((s) => s.credentialsId); const inaccessibleCredIds = workflowCredIds.filter((id) => !accessibleCredIds.includes(id)); diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index e45e97869c773..45267b0d1b0ed 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -17,11 +17,11 @@ import { getLicense } from '@/License'; import { WhereClause } from '@/Interfaces'; import { RoleService } from '@/role/role.service'; -export async function getWorkflowOwner(workflowId: string | number): Promise { +export async function getWorkflowOwner(workflowId: string): Promise { const workflowOwnerRole = await RoleService.get({ name: 'owner', scope: 'workflow' }); const sharedWorkflow = await Db.collections.SharedWorkflow.findOneOrFail({ - where: { workflow: { id: workflowId }, role: workflowOwnerRole }, + where: { workflowId, role: workflowOwnerRole }, relations: ['user', 'user.globalRole'], }); diff --git a/packages/cli/src/UserManagement/routes/users.ts b/packages/cli/src/UserManagement/routes/users.ts index 803926117186d..0d62f2e05d702 100644 --- a/packages/cli/src/UserManagement/routes/users.ts +++ b/packages/cli/src/UserManagement/routes/users.ts @@ -430,8 +430,8 @@ export function usersNamespace(this: N8nApp): void { where: { user: userToDelete, role: workflowOwnerRole }, }); - const sharedWorkflowIds = sharedWorkflows.map((sharedWorkflow) => - sharedWorkflow.workflowId.toString(), + const sharedWorkflowIds = sharedWorkflows.map( + (sharedWorkflow) => sharedWorkflow.workflowId, ); // Prevents issues with unique key constraints since user being assigned @@ -455,8 +455,8 @@ export function usersNamespace(this: N8nApp): void { where: { user: userToDelete, role: credentialOwnerRole }, }); - const sharedCredentialIds = sharedCredentials.map((sharedCredential) => - sharedCredential.credentialsId.toString(), + const sharedCredentialIds = sharedCredentials.map( + (sharedCredential) => sharedCredential.credentialsId, ); // Prevents issues with unique key constraints since user being assigned @@ -500,7 +500,7 @@ export function usersNamespace(this: N8nApp): void { ownedSharedWorkflows.map(async ({ workflow }) => { if (workflow.active) { // deactivate before deleting - await this.activeWorkflowRunner.remove(workflow.id.toString()); + await this.activeWorkflowRunner.remove(workflow.id); } return workflow; }), diff --git a/packages/cli/src/WaitTracker.ts b/packages/cli/src/WaitTracker.ts index dbacda38beeb7..df86526421dd1 100644 --- a/packages/cli/src/WaitTracker.ts +++ b/packages/cli/src/WaitTracker.ts @@ -77,7 +77,7 @@ export class WaitTrackerClass { return; } - const executionIds = executions.map((execution) => execution.id.toString()).join(', '); + const executionIds = executions.map((execution) => execution.id).join(', '); Logger.debug( `Wait tracker found ${executions.length} executions. Setting timer for IDs: ${executionIds}`, ); @@ -85,7 +85,7 @@ export class WaitTrackerClass { // Add timers for each waiting execution that they get started at the correct time // eslint-disable-next-line no-restricted-syntax for (const execution of executions) { - const executionId = execution.id.toString(); + const executionId = execution.id; if (this.waitingExecutions[executionId] === undefined) { const triggerTime = execution.waitTill!.getTime() - new Date().getTime(); this.waitingExecutions[executionId] = { @@ -161,7 +161,7 @@ export class WaitTrackerClass { if (!fullExecutionData.workflowData.id) { throw new Error('Only saved workflows can be resumed.'); } - const user = await getWorkflowOwner(fullExecutionData.workflowData.id.toString()); + const user = await getWorkflowOwner(fullExecutionData.workflowData.id); const data: IWorkflowExecutionDataProcess = { executionMode: fullExecutionData.mode, diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index d163f93941220..83b445318f945 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -167,7 +167,7 @@ export async function executeWebhook( user = (workflowData as WorkflowEntity).shared[0].user; } else { try { - user = await getWorkflowOwner(workflowData.id.toString()); + user = await getWorkflowOwner(workflowData.id); } catch (error) { throw new ResponseHelper.NotFoundError('Cannot find workflow'); } diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 1732682f65499..e52a02ae6b458 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -107,7 +107,7 @@ export function executeErrorWorkflow( retryOf, }, workflow: { - id: workflowData.id !== undefined ? workflowData.id.toString() : undefined, + id: workflowData.id !== undefined ? workflowData.id : undefined, name: workflowData.name, }, }; @@ -119,7 +119,7 @@ export function executeErrorWorkflow( mode, }, workflow: { - id: workflowData.id !== undefined ? workflowData.id.toString() : undefined, + id: workflowData.id !== undefined ? workflowData.id : undefined, name: workflowData.name, }, }; @@ -134,7 +134,7 @@ export function executeErrorWorkflow( !( mode === 'error' && workflowData.id && - workflowData.settings.errorWorkflow.toString() === workflowData.id.toString() + workflowData.settings.errorWorkflow.toString() === workflowData.id ) ) { Logger.verbose(`Start external error workflow`, { @@ -219,7 +219,7 @@ async function pruneExecutionData(this: WorkflowHooks): Promise { }, timeout * 1000); // Mark binary data for deletion for all executions await BinaryDataManager.getInstance().markDataForDeletionByExecutionIds( - executions.map(({ id }) => id.toString()), + executions.map(({ id }) => id), ); } catch (error) { ErrorReporter.error(error); @@ -505,7 +505,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { try { if ( !isManualMode && - WorkflowHelpers.isWorkflowIdValid(this.workflowData.id as string) && + WorkflowHelpers.isWorkflowIdValid(this.workflowData.id) && newStaticData ) { // Workflow is saved so update in database @@ -593,17 +593,15 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { fullExecutionData.retryOf = this.retryOf.toString(); } - if ( - this.workflowData.id !== undefined && - WorkflowHelpers.isWorkflowIdValid(this.workflowData.id.toString()) - ) { - fullExecutionData.workflowId = this.workflowData.id.toString(); + const workflowId = this.workflowData.id; + if (workflowId !== undefined && WorkflowHelpers.isWorkflowIdValid(workflowId)) { + fullExecutionData.workflowId = workflowId; } // Leave log message before flatten as that operation increased memory usage a lot and the chance of a crash is highest here Logger.debug(`Save execution data to database for execution ID ${this.executionId}`, { executionId: this.executionId, - workflowId: this.workflowData.id, + workflowId, finished: fullExecutionData.finished, stoppedAt: fullExecutionData.stoppedAt, }); @@ -685,7 +683,7 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { newStaticData: IDataObject, ): Promise { try { - if (WorkflowHelpers.isWorkflowIdValid(this.workflowData.id as string) && newStaticData) { + if (WorkflowHelpers.isWorkflowIdValid(this.workflowData.id) && newStaticData) { // Workflow is saved so update in database try { await WorkflowHelpers.saveStaticDataById( @@ -726,11 +724,9 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { fullExecutionData.retryOf = this.retryOf.toString(); } - if ( - this.workflowData.id !== undefined && - WorkflowHelpers.isWorkflowIdValid(this.workflowData.id.toString()) - ) { - fullExecutionData.workflowId = this.workflowData.id.toString(); + const workflowId = this.workflowData.id; + if (workflowId !== undefined && WorkflowHelpers.isWorkflowIdValid(workflowId)) { + fullExecutionData.workflowId = workflowId; } const executionData = ResponseHelper.flattenExecutionData(fullExecutionData); @@ -844,12 +840,7 @@ export async function getWorkflowData( const relations = config.getEnv('workflowTagsDisabled') ? [] : ['tags']; - workflowData = await WorkflowsService.get( - { id: parseInt(workflowInfo.id, 10) }, - { - relations, - }, - ); + workflowData = await WorkflowsService.get({ id: workflowInfo.id }, { relations }); if (workflowData === undefined) { throw new Error(`The workflow with the id "${workflowInfo.id}" does not exist.`); @@ -1004,7 +995,7 @@ async function executeWorkflow( workflowData, }; if (workflowData.id) { - fullExecutionData.workflowId = workflowData.id as string; + fullExecutionData.workflowId = workflowData.id; } const executionData = ResponseHelper.flattenExecutionData(fullExecutionData); diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index 8c39b9ad0f219..53afb2f337fcb 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -71,16 +71,8 @@ export function getDataLastExecutedNodeData(inputData: IRun): ITaskData | undefi * * @param {(string | null | undefined)} id The id to check */ -export function isWorkflowIdValid(id: string | null | undefined | number): boolean { - if (typeof id === 'string') { - id = parseInt(id, 10); - } - - // eslint-disable-next-line no-restricted-globals - if (isNaN(id as number)) { - return false; - } - return true; +export function isWorkflowIdValid(id: string | null | undefined): boolean { + return !(typeof id === 'string' && isNaN(parseInt(id, 10))); } /** @@ -97,7 +89,7 @@ export async function executeErrorWorkflow( // Wrap everything in try/catch to make sure that no errors bubble up and all get caught here try { let workflowData; - if (workflowId.toString() !== workflowErrorData.workflow.id?.toString()) { + if (workflowId !== workflowErrorData.workflow.id) { // To make this code easier to understand, we split it in 2 parts: // 1) Fetch the owner of the errored workflows and then // 2) if now instance owner, then check if the user has access to the @@ -106,13 +98,10 @@ export async function executeErrorWorkflow( const user = await getWorkflowOwner(workflowErrorData.workflow.id!); if (user.globalRole.name === 'owner') { - workflowData = await Db.collections.Workflow.findOne({ id: Number(workflowId) }); + workflowData = await Db.collections.Workflow.findOne({ id: workflowId }); } else { const sharedWorkflowData = await Db.collections.SharedWorkflow.findOne({ - where: { - workflow: { id: workflowId }, - user, - }, + where: { workflowId, user }, relations: ['workflow'], }); if (sharedWorkflowData) { @@ -120,7 +109,7 @@ export async function executeErrorWorkflow( } } } else { - workflowData = await Db.collections.Workflow.findOne({ id: Number(workflowId) }); + workflowData = await Db.collections.Workflow.findOne({ id: workflowId }); } if (workflowData === undefined) { @@ -250,11 +239,11 @@ export async function saveStaticData(workflow: Workflow): Promise { /** * Saves the given static data on workflow * - * @param {(string | number)} workflowId The id of the workflow to save data on + * @param {(string)} workflowId The id of the workflow to save data on * @param {IDataObject} newStaticData The static data to save */ export async function saveStaticDataById( - workflowId: string | number, + workflowId: string, newStaticData: IDataObject, ): Promise { await Db.collections.Workflow.update(workflowId, { @@ -265,10 +254,10 @@ export async function saveStaticDataById( /** * Returns the static data of workflow * - * @param {(string | number)} workflowId The id of the workflow to get static data of + * @param {(string)} workflowId The id of the workflow to get static data of */ // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types -export async function getStaticDataById(workflowId: string | number) { +export async function getStaticDataById(workflowId: string) { const workflowData = await Db.collections.Workflow.findOne(workflowId, { select: ['staticData'], }); @@ -329,7 +318,7 @@ export async function replaceInvalidCredentials(workflow: WorkflowEntity): Promi // if credential name-type combination is unique, use it if (credentials?.length === 1) { credentialsByName[nodeCredentialType][name] = { - id: credentials[0].id.toString(), + id: credentials[0].id, name: credentials[0].name, }; node.credentials[nodeCredentialType] = credentialsByName[nodeCredentialType][name]; @@ -364,7 +353,7 @@ export async function replaceInvalidCredentials(workflow: WorkflowEntity): Promi }); if (credentials) { credentialsById[nodeCredentialType][nodeCredentials.id] = { - id: credentials.id.toString(), + id: credentials.id, name: credentials.name, }; node.credentials[nodeCredentialType] = @@ -380,7 +369,7 @@ export async function replaceInvalidCredentials(workflow: WorkflowEntity): Promi if (credsByName?.length === 1) { // add found credential to cache credentialsById[nodeCredentialType][credsByName[0].id] = { - id: credsByName[0].id.toString(), + id: credsByName[0].id, name: credsByName[0].name, }; node.credentials[nodeCredentialType] = @@ -412,7 +401,7 @@ export async function getSharedWorkflowIds(user: User, roles?: string[]): Promis where: whereClause({ user, entityType: 'workflow', roles }), }); - return sharedWorkflows.map(({ workflowId }) => workflowId.toString()); + return sharedWorkflows.map(({ workflowId }) => workflowId); } /** @@ -538,7 +527,7 @@ export function validateWorkflowCredentialUsage( * - It's a new node which indicates tampering and therefore must fail saving */ - const allowedCredentialIds = credentialsUserHasAccessTo.map((cred) => cred.id.toString()); + const allowedCredentialIds = credentialsUserHasAccessTo.map((cred) => cred.id); const nodesWithCredentialsUserDoesNotHaveAccessTo = getNodesWithInaccessibleCreds( newWorkflowVersion, diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index 0568ab2fbe0fa..0d0a090422315 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -199,10 +199,9 @@ export class WorkflowRunner { restartExecutionId?: string, responsePromise?: IDeferredPromise, ): Promise { - if (loadStaticData === true && data.workflowData.id) { - data.workflowData.staticData = await WorkflowHelpers.getStaticDataById( - data.workflowData.id as string, - ); + const workflowId = data.workflowData.id; + if (loadStaticData === true && workflowId) { + data.workflowData.staticData = await WorkflowHelpers.getStaticDataById(workflowId); } const nodeTypes = NodeTypes(); @@ -221,7 +220,7 @@ export class WorkflowRunner { } const workflow = new Workflow({ - id: data.workflowData.id as string | undefined, + id: workflowId, name: data.workflowData.name, nodes: data.workflowData.nodes, connections: data.workflowData.connections, @@ -598,13 +597,12 @@ export class WorkflowRunner { restartExecutionId?: string, responsePromise?: IDeferredPromise, ): Promise { + const workflowId = data.workflowData.id; let startedAt = new Date(); const subprocess = fork(pathJoin(__dirname, 'WorkflowRunnerProcess.js')); - if (loadStaticData === true && data.workflowData.id) { - data.workflowData.staticData = await WorkflowHelpers.getStaticDataById( - data.workflowData.id as string, - ); + if (loadStaticData === true && workflowId) { + data.workflowData.staticData = await WorkflowHelpers.getStaticDataById(workflowId); } // Register the active execution diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index 69e05af83d995..07a33f062f50f 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -137,7 +137,7 @@ class WorkflowRunnerProcess { } this.workflow = new Workflow({ - id: this.data.workflowData.id as string | undefined, + id: this.data.workflowData.id, name: this.data.workflowData.name, nodes: this.data.workflowData.nodes, connections: this.data.workflowData.connections, diff --git a/packages/cli/src/api/tags.api.ts b/packages/cli/src/api/tags.api.ts index 9d6329b1d6da5..ed01946000aa5 100644 --- a/packages/cli/src/api/tags.api.ts +++ b/packages/cli/src/api/tags.api.ts @@ -63,7 +63,7 @@ tagsController.post( // Updates a tag tagsController.patch( - '/:id', + '/:id(\\d+)', workflowsEnabledMiddleware, ResponseHelper.send(async (req: express.Request): Promise => { const { name } = req.body; @@ -86,7 +86,7 @@ tagsController.patch( ); tagsController.delete( - '/:id', + '/:id(\\d+)', workflowsEnabledMiddleware, ResponseHelper.send(async (req: TagsRequest.Delete): Promise => { if ( @@ -98,7 +98,7 @@ tagsController.delete( 'Only owners can remove tags', ); } - const id = Number(req.params.id); + const id = req.params.id; await externalHooks.run('tag.beforeDelete', [id]); diff --git a/packages/cli/src/commands/Interfaces.d.ts b/packages/cli/src/commands/Interfaces.d.ts index 95c0946dd25bd..96a31ad9f8969 100644 --- a/packages/cli/src/commands/Interfaces.d.ts +++ b/packages/cli/src/commands/Interfaces.d.ts @@ -13,7 +13,7 @@ interface IResult { executions: IExecutionResult[]; } interface IExecutionResult { - workflowId: string | number; + workflowId: string; workflowName: string; executionTime: number; // Given in seconds with decimals for milliseconds finished: boolean; @@ -26,12 +26,12 @@ interface IExecutionResult { } interface IExecutionError { - workflowId: string | number; + workflowId: string; error: string; } interface IWorkflowExecutionProgress { - workflowId: string | number; + workflowId: string; status: ExecutionStatus; } diff --git a/packages/cli/src/commands/execute.ts b/packages/cli/src/commands/execute.ts index 883c04d12bf4d..0ee3c49d171c2 100644 --- a/packages/cli/src/commands/execute.ts +++ b/packages/cli/src/commands/execute.ts @@ -96,8 +96,7 @@ export class Execute extends Command { return; } - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - workflowId = workflowData.id ? workflowData.id.toString() : PLACEHOLDER_EMPTY_WORKFLOW_ID; + workflowId = workflowData.id ?? PLACEHOLDER_EMPTY_WORKFLOW_ID; } // Wait till the database is ready diff --git a/packages/cli/src/commands/export/credentials.ts b/packages/cli/src/commands/export/credentials.ts index 7a785b77c4712..fb2688bbf91b2 100644 --- a/packages/cli/src/commands/export/credentials.ts +++ b/packages/cli/src/commands/export/credentials.ts @@ -128,7 +128,7 @@ export class ExportCredentialsCommand extends Command { for (let i = 0; i < credentials.length; i++) { const { name, type, nodesAccess, data } = credentials[i]; - const id = credentials[i].id as string; + const id = credentials[i].id; const credential = new Credentials({ id, name }, type, nodesAccess, data); const plainData = credential.getData(encryptionKey); (credentials[i] as ICredentialsDecryptedDb).data = plainData; diff --git a/packages/cli/src/commands/import/workflow.ts b/packages/cli/src/commands/import/workflow.ts index 32031947f273f..e38a1dc9d16cd 100644 --- a/packages/cli/src/commands/import/workflow.ts +++ b/packages/cli/src/commands/import/workflow.ts @@ -263,7 +263,7 @@ export class ImportWorkflowsCommand extends Command { ); if (matchingCredentials.length === 1) { - nodeCredentials.id = matchingCredentials[0].id.toString(); + nodeCredentials.id = matchingCredentials[0].id; } // eslint-disable-next-line no-param-reassign diff --git a/packages/cli/src/credentials/credentials.controller.ee.ts b/packages/cli/src/credentials/credentials.controller.ee.ts index e27f30767b6b9..b5b3d77f86b73 100644 --- a/packages/cli/src/credentials/credentials.controller.ee.ts +++ b/packages/cli/src/credentials/credentials.controller.ee.ts @@ -35,13 +35,9 @@ EECredentialsController.get( }); // eslint-disable-next-line @typescript-eslint/unbound-method - return allCredentials - .map((credential: CredentialsEntity & CredentialWithSharings) => - EECredentials.addOwnerAndSharings(credential), - ) - .map( - (credential): CredentialWithSharings => ({ ...credential, id: credential.id.toString() }), - ); + return allCredentials.map((credential: CredentialsEntity & CredentialWithSharings) => + EECredentials.addOwnerAndSharings(credential), + ); } catch (error) { LoggerProxy.error('Request to list credentials failed', error as Error); throw error; @@ -53,16 +49,12 @@ EECredentialsController.get( * GET /credentials/:id */ EECredentialsController.get( - '/:id', + '/:id(\\d+)', (req, res, next) => (req.params.id === 'new' ? next('router') : next()), // skip ee router and use free one for naming ResponseHelper.send(async (req: CredentialRequest.Get) => { const { id: credentialId } = req.params; const includeDecryptedData = req.query.includeData === 'true'; - if (Number.isNaN(Number(credentialId))) { - throw new ResponseHelper.BadRequestError(`Credential ID must be a number.`); - } - let credential = (await EECredentials.get( { id: credentialId }, { relations: ['shared', 'shared.role', 'shared.user'] }, @@ -82,19 +74,12 @@ EECredentialsController.get( credential = EECredentials.addOwnerAndSharings(credential); - // @ts-ignore @TODO_TECH_DEBT: Stringify `id` with entity field transformer - credential.id = credential.id.toString(); - if (!includeDecryptedData || !userSharing || userSharing.role.name !== 'owner') { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { id, data: _, ...rest } = credential; - - // @TODO_TECH_DEBT: Stringify `id` with entity field transformer - return { id: id.toString(), ...rest }; + const { data: _, ...rest } = credential; + return { ...rest }; } - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { id, data: _, ...rest } = credential; + const { data: _, ...rest } = credential; const key = await EECredentials.getEncryptionKey(); const decryptedData = EECredentials.redact( @@ -102,8 +87,7 @@ EECredentialsController.get( credential, ); - // @TODO_TECH_DEBT: Stringify `id` with entity field transformer - return { id: id.toString(), data: decryptedData, ...rest }; + return { data: decryptedData, ...rest }; }), ); @@ -119,9 +103,10 @@ EECredentialsController.post( const encryptionKey = await EECredentials.getEncryptionKey(); - const { ownsCredential } = await EECredentials.isOwned(req.user, credentials.id.toString()); + const credentialsId = credentials.id; + const { ownsCredential } = await EECredentials.isOwned(req.user, credentialsId); - const sharing = await EECredentials.getSharing(req.user, credentials.id); + const sharing = await EECredentials.getSharing(req.user, credentialsId); if (!ownsCredential) { if (!sharing) { throw new ResponseHelper.UnauthorizedError(`Forbidden`); @@ -161,7 +146,6 @@ EECredentialsController.put( } const { ownsCredential, credential } = await EECredentials.isOwned(req.user, credentialId); - if (!ownsCredential || !credential) { throw new ResponseHelper.UnauthorizedError('Forbidden'); } @@ -191,7 +175,7 @@ EECredentialsController.put( void InternalHooksManager.getInstance().onUserSharedCredentials({ credential_type: credential.type, - credential_id: credential.id.toString(), + credential_id: credential.id, user_id_sharer: req.user.id, user_ids_sharees_added: newShareeIds, sharees_removed: amountRemoved, diff --git a/packages/cli/src/credentials/credentials.controller.ts b/packages/cli/src/credentials/credentials.controller.ts index 21a90f26b352b..05e3b9d10e60d 100644 --- a/packages/cli/src/credentials/credentials.controller.ts +++ b/packages/cli/src/credentials/credentials.controller.ts @@ -11,7 +11,7 @@ import { getLogger } from '@/Logger'; import { EECredentialsController } from './credentials.controller.ee'; import { CredentialsService } from './credentials.service'; -import type { ICredentialsResponse } from '@/Interfaces'; +import type { ICredentialsDb } from '@/Interfaces'; import type { CredentialRequest } from '@/requests'; export const credentialsController = express.Router(); @@ -35,14 +35,8 @@ credentialsController.use('/', EECredentialsController); */ credentialsController.get( '/', - ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise => { - const credentials = await CredentialsService.getAll(req.user, { roles: ['owner'] }); - - return credentials.map((credential) => { - // eslint-disable-next-line no-param-reassign - credential.id = credential.id.toString(); - return credential as ICredentialsResponse; - }); + ResponseHelper.send(async (req: CredentialRequest.GetAll): Promise => { + return CredentialsService.getAll(req.user, { roles: ['owner'] }); }), ); @@ -69,15 +63,11 @@ credentialsController.get( * GET /credentials/:id */ credentialsController.get( - '/:id', + '/:id(\\d+)', ResponseHelper.send(async (req: CredentialRequest.Get) => { const { id: credentialId } = req.params; const includeDecryptedData = req.query.includeData === 'true'; - if (Number.isNaN(Number(credentialId))) { - throw new ResponseHelper.BadRequestError(`Credential ID must be a number.`); - } - const sharing = await CredentialsService.getSharing(req.user, credentialId, ['credentials']); if (!sharing) { @@ -88,11 +78,10 @@ credentialsController.get( const { credentials: credential } = sharing; - const { id, data: _, ...rest } = credential; + const { data: _, ...rest } = credential; if (!includeDecryptedData) { - // @TODO_TECH_DEBT: Stringify `id` with entity field transformer - return { id: id.toString(), ...rest }; + return { ...rest }; } const key = await CredentialsService.getEncryptionKey(); @@ -101,8 +90,7 @@ credentialsController.get( credential, ); - // @TODO_TECH_DEBT: Stringify `id` with entity field transformer - return { id: id.toString(), data: decryptedData, ...rest }; + return { data: decryptedData, ...rest }; }), ); @@ -139,15 +127,15 @@ credentialsController.post( const key = await CredentialsService.getEncryptionKey(); const encryptedData = CredentialsService.createEncryptedData(key, null, newCredential); - const { id, ...rest } = await CredentialsService.save(newCredential, encryptedData, req.user); + const credential = await CredentialsService.save(newCredential, encryptedData, req.user); void InternalHooksManager.getInstance().onUserCreatedCredentials({ - credential_type: rest.type, - credential_id: id.toString(), + credential_type: credential.type, + credential_id: credential.id, public_api: false, }); - return { id: id.toString(), ...rest }; + return credential; }), ); @@ -155,8 +143,8 @@ credentialsController.post( * PATCH /credentials/:id */ credentialsController.patch( - '/:id', - ResponseHelper.send(async (req: CredentialRequest.Update): Promise => { + '/:id(\\d+)', + ResponseHelper.send(async (req: CredentialRequest.Update): Promise => { const { id: credentialId } = req.params; const sharing = await CredentialsService.getSharing(req.user, credentialId); @@ -194,14 +182,11 @@ credentialsController.patch( } // Remove the encrypted data as it is not needed in the frontend - const { id, data: _, ...rest } = responseData; + const { data: _, ...rest } = responseData; LoggerProxy.verbose('Credential updated', { credentialId }); - return { - id: id.toString(), - ...rest, - }; + return { ...rest }; }), ); @@ -209,7 +194,7 @@ credentialsController.patch( * DELETE /credentials/:id */ credentialsController.delete( - '/:id', + '/:id(\\d+)', ResponseHelper.send(async (req: CredentialRequest.Delete) => { const { id: credentialId } = req.params; diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index cc2935909d87b..0b0fa166cbf48 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -1,5 +1,5 @@ /* eslint-disable no-param-reassign */ -import { DeleteResult, EntityManager, FindOneOptions, In, Not, ObjectLiteral } from 'typeorm'; +import { DeleteResult, EntityManager, FindConditions, In, Not } from 'typeorm'; import * as Db from '@/Db'; import { RoleService } from '@/role/role.service'; import { CredentialsEntity } from '@db/entities/CredentialsEntity'; @@ -12,9 +12,9 @@ import type { CredentialWithSharings } from './credentials.types'; export class EECredentialsService extends CredentialsService { static async isOwned( user: User, - credentialId: string, + credentialsId: string, ): Promise<{ ownsCredential: boolean; credential?: CredentialsEntity }> { - const sharing = await this.getSharing(user, credentialId, ['credentials', 'role'], { + const sharing = await this.getSharing(user, credentialsId, ['credentials', 'role'], { allowGlobalOwner: false, }); @@ -30,35 +30,31 @@ export class EECredentialsService extends CredentialsService { */ static async getSharing( user: User, - credentialId: number | string, + credentialsId: string, relations: string[] = ['credentials'], { allowGlobalOwner } = { allowGlobalOwner: true }, ): Promise { - const options: FindOneOptions & { where: ObjectLiteral } = { - where: { - credentials: { id: credentialId }, - }, - }; + const where: FindConditions = { credentialsId }; // Omit user from where if the requesting user is the global // owner. This allows the global owner to view and delete // credentials they don't own. if (!allowGlobalOwner || user.globalRole.name !== 'owner') { - options.where.user = { id: user.id }; + where.user = { id: user.id }; } - if (relations?.length) { - options.relations = relations; - } - - return Db.collections.SharedCredentials.findOne(options); + return Db.collections.SharedCredentials.findOne({ + where, + relations, + }); } static async getSharings( transaction: EntityManager, - credentialId: string, + credentialsId: string, ): Promise { - const credential = await transaction.findOne(CredentialsEntity, credentialId, { + const credential = await transaction.findOne(CredentialsEntity, { + where: { id: credentialsId }, relations: ['shared'], }); return credential?.shared ?? []; @@ -66,12 +62,12 @@ export class EECredentialsService extends CredentialsService { static async pruneSharings( transaction: EntityManager, - credentialId: string, + credentialsId: string, userIds: string[], ): Promise { return transaction.delete(SharedCredentials, { - credentials: { id: credentialId }, - user: { id: Not(In(userIds)) }, + credentialsId, + userId: Not(In(userIds)), }); } diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 3a0a2f304a86e..beb450a518e99 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -10,7 +10,7 @@ import { LoggerProxy, NodeHelpers, } from 'n8n-workflow'; -import { FindManyOptions, FindOneOptions, In } from 'typeorm'; +import { FindConditions, FindManyOptions, In } from 'typeorm'; import * as Db from '@/Db'; import * as ResponseHelper from '@/ResponseHelper'; @@ -59,22 +59,13 @@ export class CredentialsService { } // if member, return credentials owned by or shared with member - - const whereConditions: FindManyOptions = { + const userSharings = await Db.collections.SharedCredentials.find({ where: { - user, + userId: user.id, + ...(options?.roles?.length ? { role: { name: In(options.roles) } } : {}), }, - }; - - if (options?.roles?.length) { - whereConditions.where = { - ...whereConditions.where, - role: { name: In(options.roles) }, - } as FindManyOptions; - whereConditions.relations = ['role']; - } - - const userSharings = await Db.collections.SharedCredentials.find(whereConditions); + relations: options?.roles?.length ? ['role'] : [], + }); return Db.collections.Credentials.find({ select: SELECT_FIELDS, @@ -94,35 +85,26 @@ export class CredentialsService { */ static async getSharing( user: User, - credentialId: number | string, + credentialsId: string, relations: string[] = ['credentials'], { allowGlobalOwner } = { allowGlobalOwner: true }, ): Promise { - const options: FindOneOptions = { - where: { - credentials: { id: credentialId }, - }, - }; + const where: FindConditions = { credentialsId }; // Omit user from where if the requesting user is the global // owner. This allows the global owner to view and delete // credentials they don't own. if (!allowGlobalOwner || user.globalRole.name !== 'owner') { - options.where = { - ...options.where, - user: { id: user.id }, + Object.assign(where, { + userId: user.id, role: { name: 'owner' }, - } as FindOneOptions; + }); if (!relations.includes('role')) { relations.push('role'); } } - if (relations?.length) { - options.relations = relations; - } - - return Db.collections.SharedCredentials.findOne(options); + return Db.collections.SharedCredentials.findOne({ where, relations }); } static async prepareCreateData( @@ -132,7 +114,7 @@ export class CredentialsService { const { id, ...rest } = data; // This saves us a merge but requires some type casting. These - // types are compatiable for this case. + // types are compatible for this case. const newCredentials = Db.collections.Credentials.create( rest as ICredentialsDb, ) as CredentialsEntity; @@ -220,17 +202,17 @@ export class CredentialsService { } static async update( - credentialId: string, + credentialsId: string, newCredentialData: ICredentialsDb, ): Promise { await ExternalHooks().run('credentials.update', [newCredentialData]); // Update the credentials in DB - await Db.collections.Credentials.update(credentialId, newCredentialData); + await Db.collections.Credentials.update(credentialsId, newCredentialData); // We sadly get nothing back from "update". Neither if it updated a record // nor the new value. So query now the updated entry. - return Db.collections.Credentials.findOne(credentialId); + return Db.collections.Credentials.findOne(credentialsId); } static async save( @@ -267,7 +249,7 @@ export class CredentialsService { return savedCredential; }); LoggerProxy.verbose('New credential created', { - credentialId: newCredential.id, + credentialsId: newCredential.id, ownerId: user.id, }); return result; diff --git a/packages/cli/src/databases/entities/CredentialsEntity.ts b/packages/cli/src/databases/entities/CredentialsEntity.ts index 70fcde7051779..b02b1fb0a8384 100644 --- a/packages/cli/src/databases/entities/CredentialsEntity.ts +++ b/packages/cli/src/databases/entities/CredentialsEntity.ts @@ -1,14 +1,16 @@ import type { ICredentialNodeAccess } from 'n8n-workflow'; -import { Column, Entity, Index, OneToMany, PrimaryGeneratedColumn } from 'typeorm'; +import { Column, Entity, Generated, Index, OneToMany, PrimaryColumn } from 'typeorm'; import { IsArray, IsObject, IsString, Length } from 'class-validator'; import type { SharedCredentials } from './SharedCredentials'; import { AbstractEntity, jsonColumnType } from './AbstractEntity'; import type { ICredentialsDb } from '@/Interfaces'; +import { idStringifier } from '../utils/transformers'; @Entity() export class CredentialsEntity extends AbstractEntity implements ICredentialsDb { - @PrimaryGeneratedColumn() - id: number; + @Generated() + @PrimaryColumn({ transformer: idStringifier }) + id: string; @Column({ length: 128 }) @IsString({ message: 'Credential `name` must be of type string.' }) diff --git a/packages/cli/src/databases/entities/ExecutionEntity.ts b/packages/cli/src/databases/entities/ExecutionEntity.ts index a3d76e2f66096..60db88beb4a84 100644 --- a/packages/cli/src/databases/entities/ExecutionEntity.ts +++ b/packages/cli/src/databases/entities/ExecutionEntity.ts @@ -1,7 +1,8 @@ import type { WorkflowExecuteMode } from 'n8n-workflow'; -import { Column, Entity, Index, PrimaryGeneratedColumn } from 'typeorm'; +import { Column, Entity, Generated, Index, PrimaryColumn } from 'typeorm'; import { datetimeColumnType, jsonColumnType } from './AbstractEntity'; import type { IExecutionFlattedDb, IWorkflowDb } from '@/Interfaces'; +import { idStringifier } from '../utils/transformers'; @Entity() @Index(['workflowId', 'id']) @@ -10,8 +11,9 @@ import type { IExecutionFlattedDb, IWorkflowDb } from '@/Interfaces'; @Index(['workflowId', 'finished', 'id']) @Index(['workflowId', 'waitTill', 'id']) export class ExecutionEntity implements IExecutionFlattedDb { - @PrimaryGeneratedColumn() - id: number; + @Generated() + @PrimaryColumn({ transformer: idStringifier }) + id: string; @Column('text') data: string; @@ -38,7 +40,7 @@ export class ExecutionEntity implements IExecutionFlattedDb { @Column(jsonColumnType) workflowData: IWorkflowDb; - @Column({ nullable: true }) + @Column({ nullable: true, transformer: idStringifier }) workflowId: string; @Column({ type: datetimeColumnType, nullable: true }) diff --git a/packages/cli/src/databases/entities/SharedCredentials.ts b/packages/cli/src/databases/entities/SharedCredentials.ts index 107d2344f7a0e..c65ca623067b5 100644 --- a/packages/cli/src/databases/entities/SharedCredentials.ts +++ b/packages/cli/src/databases/entities/SharedCredentials.ts @@ -3,6 +3,7 @@ import type { CredentialsEntity } from './CredentialsEntity'; import type { User } from './User'; import type { Role } from './Role'; import { AbstractEntity } from './AbstractEntity'; +import { idStringifier } from '../utils/transformers'; @Entity() export class SharedCredentials extends AbstractEntity { @@ -22,7 +23,7 @@ export class SharedCredentials extends AbstractEntity { }) credentials: CredentialsEntity; - @PrimaryColumn() + @PrimaryColumn({ transformer: idStringifier }) @RelationId((sharedCredential: SharedCredentials) => sharedCredential.credentials) - credentialsId: number; + credentialsId: string; } diff --git a/packages/cli/src/databases/entities/SharedWorkflow.ts b/packages/cli/src/databases/entities/SharedWorkflow.ts index c091381276233..d24b234fc6928 100644 --- a/packages/cli/src/databases/entities/SharedWorkflow.ts +++ b/packages/cli/src/databases/entities/SharedWorkflow.ts @@ -3,6 +3,7 @@ import type { WorkflowEntity } from './WorkflowEntity'; import type { User } from './User'; import type { Role } from './Role'; import { AbstractEntity } from './AbstractEntity'; +import { idStringifier } from '../utils/transformers'; @Entity() export class SharedWorkflow extends AbstractEntity { @@ -22,7 +23,7 @@ export class SharedWorkflow extends AbstractEntity { }) workflow: WorkflowEntity; - @PrimaryColumn() + @PrimaryColumn({ transformer: idStringifier }) @RelationId((sharedWorkflow: SharedWorkflow) => sharedWorkflow.workflow) - workflowId: number; + workflowId: string; } diff --git a/packages/cli/src/databases/entities/TagEntity.ts b/packages/cli/src/databases/entities/TagEntity.ts index 4590bc311a428..f60b09e97f2ad 100644 --- a/packages/cli/src/databases/entities/TagEntity.ts +++ b/packages/cli/src/databases/entities/TagEntity.ts @@ -9,10 +9,8 @@ import { AbstractEntity } from './AbstractEntity'; @Entity() export class TagEntity extends AbstractEntity implements ITagDb { @Generated() - @PrimaryColumn({ - transformer: idStringifier, - }) - id: number; + @PrimaryColumn({ transformer: idStringifier }) + id: string; @Column({ length: 24 }) @Index({ unique: true }) diff --git a/packages/cli/src/databases/entities/WebhookEntity.ts b/packages/cli/src/databases/entities/WebhookEntity.ts index 1850071e113da..c025c0b1055bc 100644 --- a/packages/cli/src/databases/entities/WebhookEntity.ts +++ b/packages/cli/src/databases/entities/WebhookEntity.ts @@ -1,12 +1,13 @@ import { Column, Entity, Index, PrimaryColumn } from 'typeorm'; import { IWebhookDb } from '@/Interfaces'; +import { idStringifier } from '../utils/transformers'; @Entity() @Index(['webhookId', 'method', 'pathLength']) export class WebhookEntity implements IWebhookDb { - @Column() - workflowId: number; + @Column({ transformer: idStringifier }) + workflowId: string; @PrimaryColumn() webhookPath: string; diff --git a/packages/cli/src/databases/entities/WorkflowEntity.ts b/packages/cli/src/databases/entities/WorkflowEntity.ts index 74218a0f43169..0a56abbd7fd53 100644 --- a/packages/cli/src/databases/entities/WorkflowEntity.ts +++ b/packages/cli/src/databases/entities/WorkflowEntity.ts @@ -12,26 +12,28 @@ import type { import { Column, Entity, + Generated, Index, JoinColumn, JoinTable, ManyToMany, OneToMany, - PrimaryGeneratedColumn, + PrimaryColumn, } from 'typeorm'; import config from '@/config'; import type { TagEntity } from './TagEntity'; import type { SharedWorkflow } from './SharedWorkflow'; import type { WorkflowStatistics } from './WorkflowStatistics'; -import { objectRetriever, sqlite } from '../utils/transformers'; +import { idStringifier, objectRetriever, sqlite } from '../utils/transformers'; import { AbstractEntity, jsonColumnType } from './AbstractEntity'; import type { IWorkflowDb } from '@/Interfaces'; @Entity() export class WorkflowEntity extends AbstractEntity implements IWorkflowDb { - @PrimaryGeneratedColumn() - id: number; + @Generated() + @PrimaryColumn({ transformer: idStringifier }) + id: string; // TODO: Add XSS check @Index({ unique: true }) diff --git a/packages/cli/src/databases/entities/WorkflowStatistics.ts b/packages/cli/src/databases/entities/WorkflowStatistics.ts index 8378081604152..b55add2396959 100644 --- a/packages/cli/src/databases/entities/WorkflowStatistics.ts +++ b/packages/cli/src/databases/entities/WorkflowStatistics.ts @@ -1,4 +1,5 @@ import { Column, Entity, RelationId, ManyToOne, PrimaryColumn } from 'typeorm'; +import { idStringifier } from '../utils/transformers'; import { datetimeColumnType } from './AbstractEntity'; import type { WorkflowEntity } from './WorkflowEntity'; @@ -26,7 +27,7 @@ export class WorkflowStatistics { }) workflow: WorkflowEntity; + @PrimaryColumn({ transformer: idStringifier }) @RelationId((workflowStatistics: WorkflowStatistics) => workflowStatistics.workflow) - @PrimaryColumn() - workflowId: number; + workflowId: string; } diff --git a/packages/cli/src/databases/migrations/mysqldb/1630451444017-UpdateWorkflowCredentials.ts b/packages/cli/src/databases/migrations/mysqldb/1630451444017-UpdateWorkflowCredentials.ts index 5ce9629ce43ef..63a102496fd8e 100644 --- a/packages/cli/src/databases/migrations/mysqldb/1630451444017-UpdateWorkflowCredentials.ts +++ b/packages/cli/src/databases/migrations/mysqldb/1630451444017-UpdateWorkflowCredentials.ts @@ -36,7 +36,7 @@ export class UpdateWorkflowCredentials1630451444017 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } @@ -79,7 +79,7 @@ export class UpdateWorkflowCredentials1630451444017 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } @@ -123,7 +123,7 @@ export class UpdateWorkflowCredentials1630451444017 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } diff --git a/packages/cli/src/databases/migrations/postgresdb/1630419189837-UpdateWorkflowCredentials.ts b/packages/cli/src/databases/migrations/postgresdb/1630419189837-UpdateWorkflowCredentials.ts index b5620697b357d..1116d7ceaed3d 100644 --- a/packages/cli/src/databases/migrations/postgresdb/1630419189837-UpdateWorkflowCredentials.ts +++ b/packages/cli/src/databases/migrations/postgresdb/1630419189837-UpdateWorkflowCredentials.ts @@ -42,7 +42,7 @@ export class UpdateWorkflowCredentials1630419189837 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } @@ -85,7 +85,7 @@ export class UpdateWorkflowCredentials1630419189837 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } @@ -131,7 +131,7 @@ export class UpdateWorkflowCredentials1630419189837 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } diff --git a/packages/cli/src/databases/migrations/sqlite/1630330987096-UpdateWorkflowCredentials.ts b/packages/cli/src/databases/migrations/sqlite/1630330987096-UpdateWorkflowCredentials.ts index a423e3714c430..f855575e70e7f 100644 --- a/packages/cli/src/databases/migrations/sqlite/1630330987096-UpdateWorkflowCredentials.ts +++ b/packages/cli/src/databases/migrations/sqlite/1630330987096-UpdateWorkflowCredentials.ts @@ -39,7 +39,7 @@ export class UpdateWorkflowCredentials1630330987096 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } @@ -82,7 +82,7 @@ export class UpdateWorkflowCredentials1630330987096 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } @@ -126,7 +126,7 @@ export class UpdateWorkflowCredentials1630330987096 implements MigrationInterfac // @ts-ignore (credentials) => credentials.name === name && credentials.type === type, ); - node.credentials[type] = { id: matchingCredentials?.id.toString() || null, name }; + node.credentials[type] = { id: matchingCredentials?.id || null, name }; credentialsUpdated = true; } } diff --git a/packages/cli/src/databases/utils/transformers.ts b/packages/cli/src/databases/utils/transformers.ts index 1385369d3d1a6..fefcb3428ef68 100644 --- a/packages/cli/src/databases/utils/transformers.ts +++ b/packages/cli/src/databases/utils/transformers.ts @@ -3,8 +3,9 @@ import { ValueTransformer } from 'typeorm'; import config from '@/config'; export const idStringifier = { - from: (value: number): string | number => (typeof value === 'number' ? value.toString() : value), - to: (value: string): number | string => (typeof value === 'string' ? Number(value) : value), + from: (value: number): string => value?.toString(), + to: (value: string | unknown): number | unknown => + typeof value === 'string' ? Number(value) : value, }; export const lowerCaser = { diff --git a/packages/cli/src/events/WorkflowStatistics.ts b/packages/cli/src/events/WorkflowStatistics.ts index cd2d51335daad..2606553beb625 100644 --- a/packages/cli/src/events/WorkflowStatistics.ts +++ b/packages/cli/src/events/WorkflowStatistics.ts @@ -1,4 +1,4 @@ -import { INode, IRun, IWorkflowBase, LoggerProxy } from 'n8n-workflow'; +import { INode, IRun, IWorkflowBase } from 'n8n-workflow'; import * as Db from '@/Db'; import { InternalHooksManager } from '@/InternalHooksManager'; import { StatisticsNames } from '@/databases/entities/WorkflowStatistics'; @@ -22,14 +22,7 @@ export async function workflowExecutionCompleted( } // Get the workflow id - let workflowId: number; - try { - workflowId = parseInt(workflowData.id as string, 10); - if (isNaN(workflowId)) throw new Error('not a number'); - } catch (error) { - LoggerProxy.error(`Error "${error as string}" when casting workflow ID to a number`); - return; - } + const workflowId = workflowData.id!; // Try insertion and if it fails due to key conflicts then update the existing entry instead try { @@ -62,19 +55,9 @@ export async function workflowExecutionCompleted( } export async function nodeFetchedData(workflowId: string, node: INode): Promise { - // Get the workflow id - let id: number; - try { - id = parseInt(workflowId, 10); - if (isNaN(id)) throw new Error('not a number'); - } catch (error) { - LoggerProxy.error(`Error ${error as string} when casting workflow ID to a number`); - return; - } - // Update only if necessary const response = await Db.collections.Workflow.update( - { id, dataLoaded: false }, + { id: workflowId, dataLoaded: false }, { dataLoaded: true }, ); @@ -85,7 +68,7 @@ export async function nodeFetchedData(workflowId: string, node: INode): Promise< const owner = await getWorkflowOwner(workflowId); let metrics = { user_id: owner.id, - workflow_id: id, + workflow_id: workflowId, node_type: node.type, node_id: node.id, }; diff --git a/packages/cli/src/executions/executions.controller.ee.ts b/packages/cli/src/executions/executions.controller.ee.ts index c455b32286749..8edfe24b0c427 100644 --- a/packages/cli/src/executions/executions.controller.ee.ts +++ b/packages/cli/src/executions/executions.controller.ee.ts @@ -36,7 +36,7 @@ EEExecutionsController.get( * GET /executions/:id */ EEExecutionsController.get( - '/:id', + '/:id(\\d+)', ResponseHelper.send( async ( req: ExecutionRequest.Get, diff --git a/packages/cli/src/executions/executions.controller.ts b/packages/cli/src/executions/executions.controller.ts index af33367611f47..3392405285595 100644 --- a/packages/cli/src/executions/executions.controller.ts +++ b/packages/cli/src/executions/executions.controller.ts @@ -41,7 +41,7 @@ executionsController.get( * GET /executions/:id */ executionsController.get( - '/:id', + '/:id(\\d+)', ResponseHelper.send( async ( req: ExecutionRequest.Get, diff --git a/packages/cli/src/executions/executions.service.ts b/packages/cli/src/executions/executions.service.ts index 5e47eb0b6559d..37f95864d773f 100644 --- a/packages/cli/src/executions/executions.service.ts +++ b/packages/cli/src/executions/executions.service.ts @@ -31,7 +31,7 @@ interface IGetExecutionsQueryFilter { mode?: string; retryOf?: string; retrySuccessId?: string; - workflowId?: number | string; + workflowId?: string; // eslint-disable-next-line @typescript-eslint/no-explicit-any waitTill?: FindOperator | boolean; } @@ -246,7 +246,7 @@ export class ExecutionsService { const formattedExecutions = executions.map((execution) => { return { - id: execution.id.toString(), + id: execution.id, finished: execution.finished, mode: execution.mode, retryOf: execution.retryOf?.toString(), @@ -254,7 +254,7 @@ export class ExecutionsService { waitTill: execution.waitTill as Date | undefined, startedAt: execution.startedAt, stoppedAt: execution.stoppedAt, - workflowId: execution.workflowData?.id?.toString() ?? '', + workflowId: execution.workflowData?.id ?? '', workflowName: execution.workflowData?.name, }; }); @@ -292,13 +292,8 @@ export class ExecutionsService { return ResponseHelper.unflattenExecutionData(execution); } - const { id, ...rest } = execution; - // @ts-ignore - return { - id: id.toString(), - ...rest, - }; + return execution; } static async retryExecution(req: ExecutionRequest.Retry): Promise { @@ -473,7 +468,7 @@ export class ExecutionsService { if (!executions.length) return; - const idsToDelete = executions.map(({ id }) => id.toString()); + const idsToDelete = executions.map(({ id }) => id); await Promise.all( idsToDelete.map(async (id) => binaryDataManager.deleteBinaryDataByExecutionId(id)), @@ -502,7 +497,7 @@ export class ExecutionsService { return; } - const idsToDelete = executions.map(({ id }) => id.toString()); + const idsToDelete = executions.map(({ id }) => id); await Promise.all( idsToDelete.map(async (id) => binaryDataManager.deleteBinaryDataByExecutionId(id)), diff --git a/packages/cli/src/role/role.service.ts b/packages/cli/src/role/role.service.ts index 753e631d9dcd3..274aab0139537 100644 --- a/packages/cli/src/role/role.service.ts +++ b/packages/cli/src/role/role.service.ts @@ -13,10 +13,7 @@ export class RoleService { static async getUserRoleForWorkflow(userId: string, workflowId: string) { const shared = await Db.collections.SharedWorkflow.findOne({ - where: { - workflow: { id: workflowId }, - user: { id: userId }, - }, + where: { workflowId, userId }, relations: ['role'], }); return shared?.role; diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index b50e464d0adaf..37524e103ee2d 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -92,7 +92,7 @@ EEWorkflowController.get( relations.push('tags'); } - const workflow = await EEWorkflows.get({ id: parseInt(workflowId, 10) }, { relations }); + const workflow = await EEWorkflows.get({ id: workflowId }, { relations }); if (!workflow) { throw new ResponseHelper.NotFoundError(`Workflow with ID "${workflowId}" does not exist`); @@ -108,7 +108,7 @@ EEWorkflowController.get( EEWorkflows.addOwnerAndSharings(workflow); await EEWorkflows.addCredentialsToWorkflow(workflow, req.user); - return EEWorkflows.entityToResponse(workflow); + return workflow; }), ); @@ -189,7 +189,7 @@ EEWorkflowController.post( await ExternalHooks().run('workflow.afterCreate', [savedWorkflow]); void InternalHooksManager.getInstance().onWorkflowCreated(req.user.id, newWorkflow, false); - return EEWorkflows.entityToResponse(savedWorkflow); + return savedWorkflow; }), ); @@ -205,7 +205,7 @@ EEWorkflowController.get( return workflows.map((workflow) => { EEWorkflows.addOwnerAndSharings(workflow); workflow.nodes = []; - return EEWorkflows.entityToResponse(workflow); + return workflow; }); }), ); @@ -230,7 +230,7 @@ EEWorkflowController.patch( forceSave, ); - return EEWorkflows.entityToResponse(updatedWorkflow); + return updatedWorkflow; }), ); @@ -244,11 +244,7 @@ EEWorkflowController.post( Object.assign(workflow, req.body.workflowData); if (workflow.id !== undefined) { - const safeWorkflow = await EEWorkflows.preventTampering( - workflow, - workflow.id.toString(), - req.user, - ); + const safeWorkflow = await EEWorkflows.preventTampering(workflow, workflow.id, req.user); req.body.workflowData.nodes = safeWorkflow.nodes; } diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index f144da5c61935..d5027b2f322a8 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -106,7 +106,7 @@ workflowsController.post( await ExternalHooks().run('workflow.afterCreate', [savedWorkflow]); void InternalHooksManager.getInstance().onWorkflowCreated(req.user.id, newWorkflow, false); - return WorkflowsService.entityToResponse(savedWorkflow); + return savedWorkflow; }), ); @@ -116,8 +116,7 @@ workflowsController.post( workflowsController.get( '/', ResponseHelper.send(async (req: WorkflowRequest.GetAll) => { - const workflows = await WorkflowsService.getMany(req.user, req.query.filter); - return workflows.map((workflow) => WorkflowsService.entityToResponse(workflow)); + return WorkflowsService.getMany(req.user, req.query.filter); }), ); @@ -218,7 +217,7 @@ workflowsController.get( ); } - return WorkflowsService.entityToResponse(shared.workflow); + return shared.workflow; }), ); @@ -244,7 +243,7 @@ workflowsController.patch( ['owner'], ); - return WorkflowsService.entityToResponse(updatedWorkflow); + return updatedWorkflow; }), ); diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts index c6f188b0a9ef9..351f8f36e7724 100644 --- a/packages/cli/src/workflows/workflows.services.ee.ts +++ b/packages/cli/src/workflows/workflows.services.ee.ts @@ -42,7 +42,8 @@ export class EEWorkflowsService extends WorkflowsService { transaction: EntityManager, workflowId: string, ): Promise { - const workflow = await transaction.findOne(WorkflowEntity, workflowId, { + const workflow = await transaction.findOne(WorkflowEntity, { + where: { id: workflowId }, relations: ['shared'], }); return workflow?.shared ?? []; @@ -54,7 +55,7 @@ export class EEWorkflowsService extends WorkflowsService { userIds: string[], ): Promise { return transaction.delete(SharedWorkflow, { - workflow: { id: workflowId }, + workflowId, user: { id: Not(In(userIds)) }, }); } @@ -113,7 +114,7 @@ export class EEWorkflowsService extends WorkflowsService { ): Promise { workflow.usedCredentials = []; const userCredentials = await EECredentials.getAll(currentUser, { disableGlobalRole: true }); - const credentialIdsUsedByWorkflow = new Set(); + const credentialIdsUsedByWorkflow = new Set(); workflow.nodes.forEach((node) => { if (!node.credentials) { return; @@ -123,8 +124,7 @@ export class EEWorkflowsService extends WorkflowsService { if (!credential?.id) { return; } - const credentialId = parseInt(credential.id, 10); - credentialIdsUsedByWorkflow.add(credentialId); + credentialIdsUsedByWorkflow.add(credential.id); }); }); const workflowCredentials = await EECredentials.getMany({ @@ -133,11 +133,11 @@ export class EEWorkflowsService extends WorkflowsService { }, relations: ['shared', 'shared.user', 'shared.role'], }); - const userCredentialIds = userCredentials.map((credential) => credential.id.toString()); + const userCredentialIds = userCredentials.map((credential) => credential.id); workflowCredentials.forEach((credential) => { - const credentialId = credential.id.toString(); + const credentialId = credential.id; const workflowCredential: CredentialUsedByWorkflow = { - id: credential.id.toString(), + id: credentialId, name: credential.name, type: credential.type, currentUserHasAccess: userCredentialIds.includes(credentialId), @@ -190,23 +190,24 @@ export class EEWorkflowsService extends WorkflowsService { relations: ['shared', 'shared.user', 'shared.role'], }); const userCredentials = await EECredentials.getAll(currentUser, { disableGlobalRole: true }); - const userCredentialIds = userCredentials.map((credential) => credential.id.toString()); + const userCredentialIds = userCredentials.map((credential) => credential.id); const credentialsMap: Record = {}; usedWorkflowsCredentials.forEach((credential) => { - credentialsMap[credential.id.toString()] = { - id: credential.id.toString(), + const credentialId = credential.id; + credentialsMap[credentialId] = { + id: credentialId, name: credential.name, type: credential.type, - currentUserHasAccess: userCredentialIds.includes(credential.id.toString()), + currentUserHasAccess: userCredentialIds.includes(credentialId), sharedWith: [], ownedBy: null, }; credential.shared?.forEach(({ user, role }) => { const { id, email, firstName, lastName } = user; if (role.name === 'owner') { - credentialsMap[credential.id.toString()].ownedBy = { id, email, firstName, lastName }; + credentialsMap[credentialId].ownedBy = { id, email, firstName, lastName }; } else { - credentialsMap[credential.id.toString()].sharedWith?.push({ + credentialsMap[credentialId].sharedWith?.push({ id, email, firstName, @@ -230,7 +231,7 @@ export class EEWorkflowsService extends WorkflowsService { return; } Object.keys(node.credentials).forEach((credentialType) => { - const credentialId = parseInt(node.credentials?.[credentialType].id ?? '', 10); + const credentialId = node.credentials?.[credentialType].id ?? ''; const matchedCredential = allowedCredentials.find( (credential) => credential.id === credentialId, ); @@ -242,7 +243,7 @@ export class EEWorkflowsService extends WorkflowsService { } static async preventTampering(workflow: WorkflowEntity, workflowId: string, user: User) { - const previousVersion = await EEWorkflowsService.get({ id: parseInt(workflowId, 10) }); + const previousVersion = await EEWorkflowsService.get({ id: workflowId }); if (!previousVersion) { throw new ResponseHelper.NotFoundError('Workflow not found'); diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index 81a8d642d434a..6fd3842a6edda 100644 --- a/packages/cli/src/workflows/workflows.services.ts +++ b/packages/cli/src/workflows/workflows.services.ts @@ -1,6 +1,6 @@ import { validate as jsonSchemaValidate } from 'jsonschema'; import { INode, IPinData, JsonObject, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow'; -import { FindManyOptions, FindOneOptions, In, ObjectLiteral } from 'typeorm'; +import { FindConditions, In } from 'typeorm'; import pick from 'lodash.pick'; import { v4 as uuid } from 'uuid'; import * as ActiveWorkflowRunner from '@/ActiveWorkflowRunner'; @@ -16,7 +16,7 @@ import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import * as TagHelpers from '@/TagHelpers'; import { WorkflowRequest } from '@/requests'; -import { IWorkflowDb, IWorkflowExecutionDataProcess, IWorkflowResponse } from '@/Interfaces'; +import { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; import { WorkflowRunner } from '@/WorkflowRunner'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; @@ -25,7 +25,7 @@ import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { isSharingEnabled, whereClause } from '@/UserManagement/UserManagementHelper'; export interface IGetWorkflowsQueryFilter { - id?: number | string; + id?: string; name?: string; active?: boolean; } @@ -45,28 +45,20 @@ const allowedWorkflowsQueryFilterFields = Object.keys(schemaGetWorkflowsQueryFil export class WorkflowsService { static async getSharing( user: User, - workflowId: number | string, + workflowId: string, relations: string[] = ['workflow'], { allowGlobalOwner } = { allowGlobalOwner: true }, ): Promise { - const options: FindOneOptions & { where: ObjectLiteral } = { - where: { - workflow: { id: workflowId }, - }, - }; + const where: FindConditions = { workflowId }; // Omit user from where if the requesting user is the global // owner. This allows the global owner to view and delete // workflows they don't own. if (!allowGlobalOwner || user.globalRole.name !== 'owner') { - options.where.user = { id: user.id }; + where.user = { id: user.id }; } - if (relations?.length) { - options.relations = relations; - } - - return Db.collections.SharedWorkflow.findOne(options); + return Db.collections.SharedWorkflow.findOne({ where, relations }); } /** @@ -120,11 +112,6 @@ export class WorkflowsService { return getSharedWorkflowIds(user, roles); } - static entityToResponse(entity: WorkflowEntity): IWorkflowResponse { - const { id, ...rest } = entity; - return { ...rest, id: id.toString() }; - } - static async getMany(user: User, rawFilter: string): Promise { const sharedWorkflowIds = await this.getWorkflowIdsForUser(user, ['owner']); if (sharedWorkflowIds.length === 0) { @@ -181,16 +168,14 @@ export class WorkflowsService { relations.push('shared', 'shared.user', 'shared.role'); } - const query: FindManyOptions = { + return Db.collections.Workflow.find({ select: isSharingEnabled() ? [...fields, 'versionId'] : fields, relations, where: { id: In(sharedWorkflowIds), ...filter, }, - }; - - return Db.collections.Workflow.find(query); + }); } static async update( @@ -310,17 +295,11 @@ export class WorkflowsService { } } - const options: FindManyOptions = { - relations: ['tags'], - }; - - if (config.getEnv('workflowTagsDisabled')) { - delete options.relations; - } + const relations = config.getEnv('workflowTagsDisabled') ? [] : ['tags']; // We sadly get nothing back from "update". Neither if it updated a record // nor the new value. So query now the hopefully updated entry. - const updatedWorkflow = await Db.collections.Workflow.findOne(workflowId, options); + const updatedWorkflow = await Db.collections.Workflow.findOne(workflowId, { relations }); if (updatedWorkflow === undefined) { throw new ResponseHelper.BadRequestError( diff --git a/packages/cli/test/integration/credentials.ee.test.ts b/packages/cli/test/integration/credentials.ee.test.ts index 9742c9ee44040..7d98dbc9ec189 100644 --- a/packages/cli/test/integration/credentials.ee.test.ts +++ b/packages/cli/test/integration/credentials.ee.test.ts @@ -351,7 +351,7 @@ test('GET /credentials/:id should return 404 if cred not found', async () => { expect(response.statusCode).toBe(404); const responseAbc = await authAgent(ownerShell).get('/credentials/abc'); - expect(responseAbc.statusCode).toBe(400); + expect(responseAbc.statusCode).toBe(404); // because EE router has precedence, check if forwards this route const responseNew = await authAgent(ownerShell).get('/credentials/new'); diff --git a/packages/cli/test/integration/credentials.test.ts b/packages/cli/test/integration/credentials.test.ts index 3d5c909746b52..df9ca9ea01257 100644 --- a/packages/cli/test/integration/credentials.test.ts +++ b/packages/cli/test/integration/credentials.test.ts @@ -81,7 +81,7 @@ test('GET /credentials should return all creds for owner', async () => { response.body.data.forEach((credential: CredentialsEntity) => { validateMainCredentialData(credential); expect(credential.data).toBeUndefined(); - expect(savedCredentialsIds.includes(Number(credential.id))).toBe(true); + expect(savedCredentialsIds.includes(credential.id)).toBe(true); }); }); @@ -104,7 +104,7 @@ test('GET /credentials should return only own creds for member', async () => { validateMainCredentialData(member1Credential); expect(member1Credential.data).toBeUndefined(); - expect(member1Credential.id).toBe(savedCredential1.id.toString()); + expect(member1Credential.id).toBe(savedCredential1.id); }); test('POST /credentials should create cred', async () => { @@ -573,16 +573,11 @@ test('GET /credentials/:id should fail with missing encryption key', async () => test('GET /credentials/:id should return 404 if cred not found', async () => { const ownerShell = await testDb.createUserShell(globalOwnerRole); - const response = await authAgent(ownerShell).get('/credentials/789'); expect(response.statusCode).toBe(404); -}); - -test('GET /credentials/:id should return 400 if id is not a number', async () => { - const ownerShell = await testDb.createUserShell(globalOwnerRole); const responseAbc = await authAgent(ownerShell).get('/credentials/abc'); - expect(responseAbc.statusCode).toBe(400); + expect(responseAbc.statusCode).toBe(404); }); function validateMainCredentialData(credential: CredentialsEntity) { diff --git a/packages/cli/test/integration/publicApi/executions.test.ts b/packages/cli/test/integration/publicApi/executions.test.ts index eb5bec5b8608a..dbb96e1024157 100644 --- a/packages/cli/test/integration/publicApi/executions.test.ts +++ b/packages/cli/test/integration/publicApi/executions.test.ts @@ -463,7 +463,7 @@ test('GET /executions should retrieve all executions of specific workflow', asyn await testDb.createManyExecutions(2, workflow2, testDb.createSuccessfulExecution); const response = await authOwnerAgent.get(`/executions`).query({ - workflowId: workflow.id.toString(), + workflowId: workflow.id, }); expect(response.statusCode).toBe(200); @@ -490,7 +490,7 @@ test('GET /executions should retrieve all executions of specific workflow', asyn expect(retryOf).toBeNull(); expect(startedAt).not.toBeNull(); expect(stoppedAt).not.toBeNull(); - expect(workflowId).toBe(workflow.id.toString()); + expect(workflowId).toBe(workflow.id); expect(waitTill).toBeNull(); } }); diff --git a/packages/cli/test/integration/publicApi/workflows.test.ts b/packages/cli/test/integration/publicApi/workflows.test.ts index 6736b3c1101dd..536f192ae414b 100644 --- a/packages/cli/test/integration/publicApi/workflows.test.ts +++ b/packages/cli/test/integration/publicApi/workflows.test.ts @@ -195,7 +195,7 @@ test('GET /workflows should return all owned workflows with pagination', async ( } // check that we really received a different result - expect(response.body.data[0].id).toBeLessThan(response2.body.data[0].id); + expect(Number(response.body.data[0].id)).toBeLessThan(Number(response2.body.data[0].id)); }); test('GET /workflows should return all owned workflows filtered by tag', async () => { @@ -690,7 +690,7 @@ test('POST /workflows/:id/activate should set workflow as active', async () => { expect(sharedWorkflow?.workflow.active).toBe(true); // check whether the workflow is on the active workflow runner - expect(await workflowRunner.isActive(workflow.id.toString())).toBe(true); + expect(await workflowRunner.isActive(workflow.id)).toBe(true); }); test('POST /workflows/:id/activate should set non-owned workflow as active when owner', async () => { @@ -744,7 +744,7 @@ test('POST /workflows/:id/activate should set non-owned workflow as active when expect(sharedWorkflow?.workflow.active).toBe(true); // check whether the workflow is on the active workflow runner - expect(await workflowRunner.isActive(workflow.id.toString())).toBe(true); + expect(await workflowRunner.isActive(workflow.id)).toBe(true); }); test('POST /workflows/:id/deactivate should fail due to missing API Key', async () => { @@ -835,7 +835,7 @@ test('POST /workflows/:id/deactivate should deactivate workflow', async () => { // check whether the workflow is deactivated in the database expect(sharedWorkflow?.workflow.active).toBe(false); - expect(await workflowRunner.isActive(workflow.id.toString())).toBe(false); + expect(await workflowRunner.isActive(workflow.id)).toBe(false); }); test('POST /workflows/:id/deactivate should deactivate non-owned workflow when owner', async () => { @@ -888,7 +888,7 @@ test('POST /workflows/:id/deactivate should deactivate non-owned workflow when o expect(sharedWorkflow?.workflow.active).toBe(false); - expect(await workflowRunner.isActive(workflow.id.toString())).toBe(false); + expect(await workflowRunner.isActive(workflow.id)).toBe(false); }); test('POST /workflows should fail due to missing API Key', async () => { diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 28bb3028380cb..108d130b64e24 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -474,7 +474,7 @@ export async function createExecution( finished: finished ?? true, mode: mode ?? 'manual', startedAt: startedAt ?? new Date(), - ...(workflow !== undefined && { workflowData: workflow, workflowId: workflow.id.toString() }), + ...(workflow !== undefined && { workflowData: workflow, workflowId: workflow.id }), stoppedAt: stoppedAt ?? new Date(), waitTill: waitTill ?? null, }); diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows.controller.ee.test.ts index 1aa3b6d9540c3..7b9f088b9c9ff 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -180,7 +180,7 @@ describe('GET /workflows', () => { position: [0, 0], credentials: { actionNetworkApi: { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, }, }, @@ -220,7 +220,7 @@ describe('GET /workflows', () => { const [usedCredential] = fetchedWorkflow.usedCredentials; expect(usedCredential).toMatchObject({ - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, type: savedCredential.type, currentUserHasAccess: true, @@ -313,7 +313,7 @@ describe('GET /workflows/:id', () => { const workflowPayload = makeWorkflow({ withPinData: false, - withCredential: { id: savedCredential.id.toString(), name: savedCredential.name }, + withCredential: { id: savedCredential.id, name: savedCredential.name }, }); const workflow = await createWorkflow(workflowPayload, owner); @@ -322,7 +322,7 @@ describe('GET /workflows/:id', () => { expect(response.statusCode).toBe(200); expect(response.body.data.usedCredentials).toMatchObject([ { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, currentUserHasAccess: true, }, @@ -338,7 +338,7 @@ describe('GET /workflows/:id', () => { const workflowPayload = makeWorkflow({ withPinData: false, - withCredential: { id: savedCredential.id.toString(), name: savedCredential.name }, + withCredential: { id: savedCredential.id, name: savedCredential.name }, }); const workflow = await createWorkflow(workflowPayload, owner); @@ -347,7 +347,7 @@ describe('GET /workflows/:id', () => { expect(response.statusCode).toBe(200); expect(response.body.data.usedCredentials).toMatchObject([ { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, currentUserHasAccess: false, // although owner can see, he does not have access }, @@ -363,7 +363,7 @@ describe('GET /workflows/:id', () => { const workflowPayload = makeWorkflow({ withPinData: false, - withCredential: { id: savedCredential.id.toString(), name: savedCredential.name }, + withCredential: { id: savedCredential.id, name: savedCredential.name }, }); const workflow = await createWorkflow(workflowPayload, member1); await testDb.shareWorkflowWithUsers(workflow, [member2]); @@ -372,7 +372,7 @@ describe('GET /workflows/:id', () => { expect(responseMember1.statusCode).toBe(200); expect(responseMember1.body.data.usedCredentials).toMatchObject([ { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, currentUserHasAccess: true, // one user has access }, @@ -383,7 +383,7 @@ describe('GET /workflows/:id', () => { expect(responseMember2.statusCode).toBe(200); expect(responseMember2.body.data.usedCredentials).toMatchObject([ { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, currentUserHasAccess: false, // the other one doesn't }, @@ -400,7 +400,7 @@ describe('GET /workflows/:id', () => { const workflowPayload = makeWorkflow({ withPinData: false, - withCredential: { id: savedCredential.id.toString(), name: savedCredential.name }, + withCredential: { id: savedCredential.id, name: savedCredential.name }, }); const workflow = await createWorkflow(workflowPayload, member1); await testDb.shareWorkflowWithUsers(workflow, [member2]); @@ -409,7 +409,7 @@ describe('GET /workflows/:id', () => { expect(responseMember1.statusCode).toBe(200); expect(responseMember1.body.data.usedCredentials).toMatchObject([ { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, currentUserHasAccess: true, }, @@ -420,7 +420,7 @@ describe('GET /workflows/:id', () => { expect(responseMember2.statusCode).toBe(200); expect(responseMember2.body.data.usedCredentials).toMatchObject([ { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, currentUserHasAccess: true, }, @@ -446,7 +446,7 @@ describe('POST /workflows', () => { const savedCredential = await saveCredential(randomCredentialPayload(), { user: owner }); const workflow = makeWorkflow({ withPinData: false, - withCredential: { id: savedCredential.id.toString(), name: savedCredential.name }, + withCredential: { id: savedCredential.id, name: savedCredential.name }, }); const response = await authAgent(owner).post('/workflows').send(workflow); @@ -462,7 +462,7 @@ describe('POST /workflows', () => { const savedCredential = await saveCredential(randomCredentialPayload(), { user: owner }); const workflow = makeWorkflow({ withPinData: false, - withCredential: { id: savedCredential.id.toString(), name: savedCredential.name }, + withCredential: { id: savedCredential.id, name: savedCredential.name }, }); const response = await authAgent(member).post('/workflows').send(workflow); @@ -481,7 +481,7 @@ describe('POST /workflows', () => { const savedCredential = await saveCredential(randomCredentialPayload(), { user: member }); const workflow = makeWorkflow({ withPinData: false, - withCredential: { id: savedCredential.id.toString(), name: savedCredential.name }, + withCredential: { id: savedCredential.id, name: savedCredential.name }, }); const response = await authAgent(owner).post('/workflows').send(workflow); @@ -498,7 +498,7 @@ describe('POST /workflows', () => { const workflow = makeWorkflow({ withPinData: false, - withCredential: { id: savedCredential.id.toString(), name: savedCredential.name }, + withCredential: { id: savedCredential.id, name: savedCredential.name }, }); const response = await authAgent(member2).post('/workflows').send(workflow); @@ -525,7 +525,7 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => typeVersion: 1, credentials: { default: { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, }, }, @@ -563,7 +563,7 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => typeVersion: 1, credentials: { default: { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, }, }, @@ -588,7 +588,7 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => typeVersion: 1, credentials: { default: { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, }, }, @@ -619,7 +619,7 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => typeVersion: 1, credentials: { default: { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, }, }, @@ -653,7 +653,7 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => typeVersion: 1, credentials: { default: { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, }, }, @@ -682,7 +682,7 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => typeVersion: 1, credentials: { default: { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, }, }, @@ -721,7 +721,7 @@ describe('PATCH /workflows/:id - validate credential permissions to user', () => typeVersion: 1, credentials: { default: { - id: savedCredential.id.toString(), + id: savedCredential.id, name: savedCredential.name, }, }, diff --git a/packages/cli/test/unit/Events.test.ts b/packages/cli/test/unit/Events.test.ts index 0e2f09a57f2da..641194949d29b 100644 --- a/packages/cli/test/unit/Events.test.ts +++ b/packages/cli/test/unit/Events.test.ts @@ -22,13 +22,13 @@ jest.mock('@/Db', () => { collections: { Workflow: { update: jest.fn(({ id, dataLoaded }, updateArgs) => { - if (id === 1) return { affected: 1 }; + if (id === '1') return { affected: 1 }; return { affected: 0 }; }), }, WorkflowStatistics: { insert: jest.fn(({ count, name, workflowId }) => { - if (workflowId === -1) throw new Error('test error'); + if (workflowId === '-1') throw new Error('test error'); return null; }), update: jest.fn((...args) => {}), @@ -104,7 +104,7 @@ describe('Events', () => { expect(mockedFirstProductionWorkflowSuccess).toBeCalledTimes(1); expect(mockedFirstProductionWorkflowSuccess).toHaveBeenNthCalledWith(1, { user_id: FAKE_USER_ID, - workflow_id: parseInt(workflow.id, 10), + workflow_id: workflow.id, }); }); @@ -180,7 +180,7 @@ describe('Events', () => { expect(mockedFirstWorkflowDataLoad).toBeCalledTimes(1); expect(mockedFirstWorkflowDataLoad).toHaveBeenNthCalledWith(1, { user_id: FAKE_USER_ID, - workflow_id: parseInt(workflowId, 10), + workflow_id: workflowId, node_type: node.type, node_id: node.id, }); @@ -207,7 +207,7 @@ describe('Events', () => { expect(mockedFirstWorkflowDataLoad).toBeCalledTimes(1); expect(mockedFirstWorkflowDataLoad).toHaveBeenNthCalledWith(1, { user_id: FAKE_USER_ID, - workflow_id: parseInt(workflowId, 10), + workflow_id: workflowId, node_type: node.type, node_id: node.id, credential_type: 'testCredentials', diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/unit/PermissionChecker.test.ts index ea9ed29627a04..7a61c6bc2e8af 100644 --- a/packages/cli/test/unit/PermissionChecker.test.ts +++ b/packages/cli/test/unit/PermissionChecker.test.ts @@ -129,7 +129,7 @@ describe('PermissionChecker.check()', () => { position: [0, 0], credentials: { actionNetworkApi: { - id: ownerCred.id.toString(), + id: ownerCred.id, name: ownerCred.name, }, }, @@ -143,7 +143,7 @@ describe('PermissionChecker.check()', () => { position: [0, 0], credentials: { actionNetworkApi: { - id: memberCred.id.toString(), + id: memberCred.id, name: memberCred.name, }, }, @@ -160,7 +160,7 @@ describe('PermissionChecker.check()', () => { const memberCred = await saveCredential(randomCred(), { user: member }); const workflowDetails = { - id: randomPositiveDigit(), + id: `${randomPositiveDigit()}`, name: 'test', active: false, connections: {}, @@ -175,7 +175,7 @@ describe('PermissionChecker.check()', () => { position: [0, 0] as [number, number], credentials: { actionNetworkApi: { - id: memberCred.id.toString(), + id: memberCred.id, name: memberCred.name, }, }, @@ -205,7 +205,7 @@ describe('PermissionChecker.check()', () => { role: workflowOwnerRole, }); - const workflow = new Workflow({ ...workflowDetails, id: workflowDetails.id.toString() }); + const workflow = new Workflow({ ...workflowDetails, id: workflowDetails.id }); expect(PermissionChecker.check(workflow, member.id)).rejects.toThrow(); }); diff --git a/packages/cli/test/unit/WorkflowCredentials.test.ts b/packages/cli/test/unit/WorkflowCredentials.test.ts index 82bb3ee27aa87..27963b282ecc2 100644 --- a/packages/cli/test/unit/WorkflowCredentials.test.ts +++ b/packages/cli/test/unit/WorkflowCredentials.test.ts @@ -8,7 +8,7 @@ async function mockFind({ id, type, }: { - id: string | number; + id: string; type: string; }): Promise { // Simple statement that maps a return value based on the `id` parameter diff --git a/packages/cli/test/unit/WorkflowHelpers.test.ts b/packages/cli/test/unit/WorkflowHelpers.test.ts index fc206d0d40d63..4eb9628555576 100644 --- a/packages/cli/test/unit/WorkflowHelpers.test.ts +++ b/packages/cli/test/unit/WorkflowHelpers.test.ts @@ -146,9 +146,9 @@ describe('WorkflowHelpers', () => { }); }); -function generateCredentialEntity(credentialId: string) { +function generateCredentialEntity(credentialsId: string) { const credentialEntity = new CredentialsEntity(); - credentialEntity.id = parseInt(credentialId, 10); + credentialEntity.id = credentialsId; return credentialEntity; } diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index f91ed4b737f18..0ea0bfdcf097c 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -132,7 +132,7 @@ export interface ICredentialNodeAccess { } export interface ICredentialsDecrypted { - id: string | number; + id: string; name: string; type: string; nodesAccess: ICredentialNodeAccess[]; @@ -142,7 +142,7 @@ export interface ICredentialsDecrypted { } export interface ICredentialsEncrypted { - id?: string | number; + id?: string; name: string; type: string; nodesAccess: ICredentialNodeAccess[]; @@ -1566,7 +1566,7 @@ export interface IWaitingForExecutionSource { } export interface IWorkflowBase { - id?: number | string | any; + id?: string; name: string; active: boolean; createdAt: Date; From b61600dae5c676872c116bf766ce8191bc6ccbaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 18:30:58 +0100 Subject: [PATCH 02/18] update ids in the frontend to be string as well --- packages/editor-ui/src/Interface.ts | 10 +++++----- .../src/components/CredentialEdit/CredentialEdit.vue | 2 +- packages/editor-ui/src/components/ExecutionsList.vue | 2 +- .../src/components/ExecutionsView/ExecutionsView.vue | 2 +- .../src/components/MainHeader/WorkflowDetails.vue | 4 ---- packages/editor-ui/src/mixins/restApi.ts | 4 ++-- packages/workflow/src/Interfaces.ts | 2 +- 7 files changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 65cd0d0985509..6dbf8f874cf22 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -217,8 +217,8 @@ export interface IRestApi { getPastExecutions( filter: object, limit: number, - lastId?: string | number, - firstId?: string | number, + lastId?: string, + firstId?: string, ): Promise; stopCurrentExecution(executionId: string): Promise; makeRestApiRequest(method: string, endpoint: string, data?: any): Promise; @@ -276,7 +276,7 @@ export interface IVariableSelectorOption { // Simple version of n8n-workflow.Workflow export interface IWorkflowData { - id?: string | number; + id?: string; name?: string; active?: boolean; nodes: INode[]; @@ -288,7 +288,7 @@ export interface IWorkflowData { } export interface IWorkflowDataUpdate { - id?: string | number; + id?: string; name?: string; nodes?: INode[]; connections?: IConnections; @@ -391,7 +391,7 @@ export interface ICredentialsDecryptedResponse extends ICredentialsBase, ICreden } export interface IExecutionBase { - id?: number | string; + id?: string; finished: boolean; mode: WorkflowExecuteMode; retryOf?: string; diff --git a/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue b/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue index d838fc68515ae..8b4a1af51e2fe 100644 --- a/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue +++ b/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue @@ -171,7 +171,7 @@ export default mixins(showMessage, nodeHelpers).extend({ required: true, }, activeId: { - type: [String, Number], + type: [String], required: true, }, mode: { diff --git a/packages/editor-ui/src/components/ExecutionsList.vue b/packages/editor-ui/src/components/ExecutionsList.vue index 0d8f043477e62..abe3085fb68de 100644 --- a/packages/editor-ui/src/components/ExecutionsList.vue +++ b/packages/editor-ui/src/components/ExecutionsList.vue @@ -742,7 +742,7 @@ export default mixins(externalHooks, genericHelpers, restApi, showMessage).exten this.isDataLoading = true; const filter = this.workflowFilterPast; - let lastId: string | number | undefined; + let lastId: string | undefined; if (this.finishedExecutions.length !== 0) { const lastItem = this.finishedExecutions.slice(-1)[0]; diff --git a/packages/editor-ui/src/components/ExecutionsView/ExecutionsView.vue b/packages/editor-ui/src/components/ExecutionsView/ExecutionsView.vue index da290eb350f9c..0d764562b1597 100644 --- a/packages/editor-ui/src/components/ExecutionsView/ExecutionsView.vue +++ b/packages/editor-ui/src/components/ExecutionsView/ExecutionsView.vue @@ -219,7 +219,7 @@ export default mixins( } this.loadingMore = true; - let lastId: string | number | undefined; + let lastId: string | undefined; if (this.executions.length !== 0) { const lastItem = this.executions.slice(-1)[0]; lastId = lastItem.id; diff --git a/packages/editor-ui/src/components/MainHeader/WorkflowDetails.vue b/packages/editor-ui/src/components/MainHeader/WorkflowDetails.vue index b3df4ab5ef0bb..80a3a59fbe911 100644 --- a/packages/editor-ui/src/components/MainHeader/WorkflowDetails.vue +++ b/packages/editor-ui/src/components/MainHeader/WorkflowDetails.vue @@ -434,10 +434,6 @@ export default mixins(workflowHelpers, titleChange).extend({ case WORKFLOW_MENU_ACTIONS.DOWNLOAD: { const workflowData = await this.getWorkflowDataToSave(); const { tags, ...data } = workflowData; - if (data.id && typeof data.id === 'string') { - data.id = parseInt(data.id, 10); - } - const exportData: IWorkflowToShare = { ...data, meta: { diff --git a/packages/editor-ui/src/mixins/restApi.ts b/packages/editor-ui/src/mixins/restApi.ts index b9bbbcc643cf4..bf2333f6b5037 100644 --- a/packages/editor-ui/src/mixins/restApi.ts +++ b/packages/editor-ui/src/mixins/restApi.ts @@ -179,8 +179,8 @@ export const restApi = Vue.extend({ getPastExecutions: ( filter: object, limit: number, - lastId?: string | number, - firstId?: string | number, + lastId?: string, + firstId?: string, ): Promise => { let sendData = {}; if (filter) { diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 0ea0bfdcf097c..c44ead4762a16 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -1425,7 +1425,7 @@ export interface IWorkflowDataProxyData { export type IWorkflowDataProxyAdditionalKeys = IDataObject; export interface IWorkflowMetadata { - id?: number | string; + id?: string; name?: string; active: boolean; } From bf484a6e80d484620c5ced037336ef6156f8c776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 18:35:00 +0100 Subject: [PATCH 03/18] credentialsId -> credentialId --- packages/cli/src/CredentialsHelper.ts | 8 +++---- .../credentials/credentials.service.ts | 8 +++---- .../src/credentials/credentials.service.ee.ts | 21 ++++++++++--------- .../src/credentials/credentials.service.ts | 14 ++++++------- .../cli/test/unit/WorkflowHelpers.test.ts | 4 ++-- 5 files changed, 28 insertions(+), 27 deletions(-) diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index 01cfebc25f0dd..d7a8144e31441 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -739,7 +739,7 @@ export class CredentialsHelper extends ICredentialsHelper { * Get a credential if it has been shared with a user. */ export async function getCredentialForUser( - credentialsId: string, + credentialId: string, user: User, ): Promise { const sharedCredential = await Db.collections.SharedCredentials.findOne({ @@ -747,7 +747,7 @@ export async function getCredentialForUser( where: whereClause({ user, entityType: 'credentials', - entityId: credentialsId, + entityId: credentialId, }), }); @@ -760,9 +760,9 @@ export async function getCredentialForUser( * Get a credential without user check */ export async function getCredentialWithoutUser( - credentialsId: string, + credentialId: string, ): Promise { - return Db.collections.Credentials.findOne(credentialsId); + return Db.collections.Credentials.findOne(credentialId); } export function createCredentialsFromCredentialsEntity( diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts index 24a82ad8419df..042a11558a0e7 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts @@ -10,16 +10,16 @@ import { ExternalHooks } from '@/ExternalHooks'; import { IDependency, IJsonSchema } from '../../../types'; import { CredentialRequest } from '@/requests'; -export async function getCredentials(credentialsId: string): Promise { - return Db.collections.Credentials.findOne(credentialsId); +export async function getCredentials(credentialId: string): Promise { + return Db.collections.Credentials.findOne(credentialId); } export async function getSharedCredentials( userId: string, - credentialsId: string, + credentialId: string, relations?: string[], ): Promise { - const where: FindConditions = { userId, credentialsId }; + const where: FindConditions = { userId, credentialsId: credentialId }; return Db.collections.SharedCredentials.findOne({ where, relations }); } diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index 0b0fa166cbf48..cb196a5a8b940 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -12,9 +12,9 @@ import type { CredentialWithSharings } from './credentials.types'; export class EECredentialsService extends CredentialsService { static async isOwned( user: User, - credentialsId: string, + credentialId: string, ): Promise<{ ownsCredential: boolean; credential?: CredentialsEntity }> { - const sharing = await this.getSharing(user, credentialsId, ['credentials', 'role'], { + const sharing = await this.getSharing(user, credentialId, ['credentials', 'role'], { allowGlobalOwner: false, }); @@ -30,11 +30,11 @@ export class EECredentialsService extends CredentialsService { */ static async getSharing( user: User, - credentialsId: string, + credentialId: string, relations: string[] = ['credentials'], { allowGlobalOwner } = { allowGlobalOwner: true }, ): Promise { - const where: FindConditions = { credentialsId }; + const where: FindConditions = { credentialsId: credentialId }; // Omit user from where if the requesting user is the global // owner. This allows the global owner to view and delete @@ -51,10 +51,10 @@ export class EECredentialsService extends CredentialsService { static async getSharings( transaction: EntityManager, - credentialsId: string, + credentialId: string, ): Promise { const credential = await transaction.findOne(CredentialsEntity, { - where: { id: credentialsId }, + where: { id: credentialId }, relations: ['shared'], }); return credential?.shared ?? []; @@ -62,13 +62,14 @@ export class EECredentialsService extends CredentialsService { static async pruneSharings( transaction: EntityManager, - credentialsId: string, + credentialId: string, userIds: string[], ): Promise { - return transaction.delete(SharedCredentials, { - credentialsId, + const conditions: FindConditions = { + credentialsId: credentialId, userId: Not(In(userIds)), - }); + }; + return transaction.delete(SharedCredentials, conditions); } static async share( diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index beb450a518e99..d31be8be31312 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -85,11 +85,11 @@ export class CredentialsService { */ static async getSharing( user: User, - credentialsId: string, + credentialId: string, relations: string[] = ['credentials'], { allowGlobalOwner } = { allowGlobalOwner: true }, ): Promise { - const where: FindConditions = { credentialsId }; + const where: FindConditions = { credentialsId: credentialId }; // Omit user from where if the requesting user is the global // owner. This allows the global owner to view and delete @@ -164,11 +164,11 @@ export class CredentialsService { static createEncryptedData( encryptionKey: string, - credentialsId: string | null, + credentialId: string | null, data: CredentialsEntity, ): ICredentialsDb { const credentials = new Credentials( - { id: credentialsId, name: data.name }, + { id: credentialId, name: data.name }, data.type, data.nodesAccess, ); @@ -202,17 +202,17 @@ export class CredentialsService { } static async update( - credentialsId: string, + credentialId: string, newCredentialData: ICredentialsDb, ): Promise { await ExternalHooks().run('credentials.update', [newCredentialData]); // Update the credentials in DB - await Db.collections.Credentials.update(credentialsId, newCredentialData); + await Db.collections.Credentials.update(credentialId, newCredentialData); // We sadly get nothing back from "update". Neither if it updated a record // nor the new value. So query now the updated entry. - return Db.collections.Credentials.findOne(credentialsId); + return Db.collections.Credentials.findOne(credentialId); } static async save( diff --git a/packages/cli/test/unit/WorkflowHelpers.test.ts b/packages/cli/test/unit/WorkflowHelpers.test.ts index 4eb9628555576..1f00375b63d3f 100644 --- a/packages/cli/test/unit/WorkflowHelpers.test.ts +++ b/packages/cli/test/unit/WorkflowHelpers.test.ts @@ -146,9 +146,9 @@ describe('WorkflowHelpers', () => { }); }); -function generateCredentialEntity(credentialsId: string) { +function generateCredentialEntity(credentialId: string) { const credentialEntity = new CredentialsEntity(); - credentialEntity.id = credentialsId; + credentialEntity.id = credentialId; return credentialEntity; } From df11992e58e97b16f322d53cae071ff23d84ed4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 18:58:58 +0100 Subject: [PATCH 04/18] delete IWebhookDb --- packages/cli/src/ActiveWorkflowRunner.ts | 18 ++++++++---------- packages/cli/src/Interfaces.ts | 12 ++---------- .../src/databases/entities/WebhookEntity.ts | 7 +++---- 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 2c3ac3d66b7ed..60327b7f90710 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -42,7 +42,6 @@ import { IActivationError, IQueuedWorkflowActivations, IResponseCallbackData, - IWebhookDb, IWorkflowDb, IWorkflowExecutionDataProcess, } from '@/Interfaces'; @@ -53,7 +52,8 @@ import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData' import config from '@/config'; import { User } from '@db/entities/User'; -import { WorkflowEntity } from '@db/entities/WorkflowEntity'; +import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; +import type { WebhookEntity } from '@db/entities/WebhookEntity'; import * as ActiveExecutions from '@/ActiveExecutions'; import { createErrorExecution } from '@/GenericHelpers'; import { WORKFLOW_REACTIVATE_INITIAL_TIMEOUT, WORKFLOW_REACTIVATE_MAX_TIMEOUT } from '@/constants'; @@ -202,10 +202,10 @@ export class ActiveWorkflowRunner { path = path.slice(0, -1); } - let webhook = (await Db.collections.Webhook.findOne({ + let webhook = await Db.collections.Webhook.findOne({ webhookPath: path, method: httpMethod, - })) as IWebhookDb; + }); let webhookId: string | undefined; // check if path is dynamic @@ -415,12 +415,12 @@ export class ActiveWorkflowRunner { path = webhookData.path; - const webhook = { + const webhook: WebhookEntity = { workflowId: webhookData.workflowId, webhookPath: path, node: node.name, method: webhookData.httpMethod, - } as IWebhookDb; + }; if (webhook.webhookPath.startsWith('/')) { webhook.webhookPath = webhook.webhookPath.slice(1); @@ -545,11 +545,9 @@ export class ActiveWorkflowRunner { await WorkflowHelpers.saveStaticData(workflow); - const webhook = { + await Db.collections.Webhook.delete({ workflowId: workflowData.id, - } as IWebhookDb; - - await Db.collections.Webhook.delete(webhook); + }); } /** diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 546edfa1deacc..3b803db0d4b43 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -38,6 +38,7 @@ import type { SharedCredentials } from '@db/entities/SharedCredentials'; import type { SharedWorkflow } from '@db/entities/SharedWorkflow'; import type { TagEntity } from '@db/entities/TagEntity'; import type { User } from '@db/entities/User'; +import type { WebhookEntity } from '@db/entities/WebhookEntity'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import type { WorkflowStatistics } from '@db/entities/WorkflowStatistics'; @@ -71,7 +72,7 @@ export interface IDatabaseCollections { Credentials: Repository; Execution: Repository; Workflow: Repository; - Webhook: Repository; + Webhook: Repository; Tag: Repository; Role: Repository; User: Repository; @@ -83,15 +84,6 @@ export interface IDatabaseCollections { WorkflowStatistics: Repository; } -export interface IWebhookDb { - workflowId: string; - webhookPath: string; - method: string; - node: string; - webhookId?: string; - pathLength?: number; -} - // ---------------------------------- // tags // ---------------------------------- diff --git a/packages/cli/src/databases/entities/WebhookEntity.ts b/packages/cli/src/databases/entities/WebhookEntity.ts index c025c0b1055bc..0ef75dc84f991 100644 --- a/packages/cli/src/databases/entities/WebhookEntity.ts +++ b/packages/cli/src/databases/entities/WebhookEntity.ts @@ -1,11 +1,10 @@ import { Column, Entity, Index, PrimaryColumn } from 'typeorm'; -import { IWebhookDb } from '@/Interfaces'; import { idStringifier } from '../utils/transformers'; @Entity() @Index(['webhookId', 'method', 'pathLength']) -export class WebhookEntity implements IWebhookDb { +export class WebhookEntity { @Column({ transformer: idStringifier }) workflowId: string; @@ -19,8 +18,8 @@ export class WebhookEntity implements IWebhookDb { node: string; @Column({ nullable: true }) - webhookId: string; + webhookId?: string; @Column({ nullable: true }) - pathLength: number; + pathLength?: number; } From 7d27dd534baf53949a0e5458c066dff9f52456c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 19:00:51 +0100 Subject: [PATCH 05/18] delete ITagDb --- packages/cli/src/Interfaces.ts | 11 ++--------- packages/cli/src/databases/entities/TagEntity.ts | 3 +-- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 3b803db0d4b43..726683c0f3e4d 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -88,13 +88,6 @@ export interface IDatabaseCollections { // tags // ---------------------------------- -export interface ITagDb { - id: string; - name: string; - createdAt: Date; - updatedAt: Date; -} - export interface ITagToImport { id: string; name: string; @@ -106,7 +99,7 @@ export type UsageCount = { usageCount: number; }; -export type ITagWithCountDb = ITagDb & UsageCount; +export type ITagWithCountDb = TagEntity & UsageCount; // ---------------------------------- // workflows @@ -119,7 +112,7 @@ export interface IWorkflowBase extends IWorkflowBaseWorkflow { // Almost identical to editor-ui.Interfaces.ts export interface IWorkflowDb extends IWorkflowBase { id: string; - tags?: ITagDb[]; + tags?: TagEntity[]; } export interface IWorkflowToImport extends IWorkflowBase { diff --git a/packages/cli/src/databases/entities/TagEntity.ts b/packages/cli/src/databases/entities/TagEntity.ts index f60b09e97f2ad..a2e84a41a0ef2 100644 --- a/packages/cli/src/databases/entities/TagEntity.ts +++ b/packages/cli/src/databases/entities/TagEntity.ts @@ -1,13 +1,12 @@ import { Column, Entity, Generated, Index, ManyToMany, PrimaryColumn } from 'typeorm'; import { IsString, Length } from 'class-validator'; -import type { ITagDb } from '@/Interfaces'; import { idStringifier } from '../utils/transformers'; import type { WorkflowEntity } from './WorkflowEntity'; import { AbstractEntity } from './AbstractEntity'; @Entity() -export class TagEntity extends AbstractEntity implements ITagDb { +export class TagEntity extends AbstractEntity { @Generated() @PrimaryColumn({ transformer: idStringifier }) id: string; From 095bdf7857f0d6607785a7383b5e1e0ca03dc4b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 19:30:35 +0100 Subject: [PATCH 06/18] unify a few of the IWorkflow* types --- packages/cli/src/Interfaces.ts | 14 ++----- packages/cli/src/InternalHooks.ts | 2 +- .../cli/src/WorkflowExecuteAdditionalData.ts | 38 ++++++++----------- packages/cli/src/commands/execute.ts | 4 +- .../cli/src/executions/executions.service.ts | 11 +++++- 5 files changed, 31 insertions(+), 38 deletions(-) diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 726683c0f3e4d..27813ebf0522b 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -14,7 +14,7 @@ import type { ITaskData, ITelemetrySettings, ITelemetryTrackProperties, - IWorkflowBase as IWorkflowBaseWorkflow, + IWorkflowBase, CredentialLoadingDetails, Workflow, WorkflowActivateMode, @@ -105,10 +105,6 @@ export type ITagWithCountDb = TagEntity & UsageCount; // workflows // ---------------------------------- -export interface IWorkflowBase extends IWorkflowBaseWorkflow { - id?: string; -} - // Almost identical to editor-ui.Interfaces.ts export interface IWorkflowDb extends IWorkflowBase { id: string; @@ -138,13 +134,9 @@ export interface ICredentialsDb extends ICredentialsBase, ICredentialsEncrypted shared?: SharedCredentials[]; } -export interface ICredentialsDecryptedDb extends ICredentialsBase, ICredentialsDecrypted { - id: string; -} +export type ICredentialsDecryptedDb = ICredentialsBase & ICredentialsDecrypted; -export interface ICredentialsDecryptedResponse extends ICredentialsDecryptedDb { - id: string; -} +export type ICredentialsDecryptedResponse = ICredentialsDecryptedDb; export type DatabaseType = 'mariadb' | 'postgresdb' | 'mysqldb' | 'sqlite'; export type SaveExecutionDataType = 'all' | 'none'; diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index 7b31241dda25b..4e000dc670c9c 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -5,6 +5,7 @@ import { INodeTypes, IRun, ITelemetryTrackProperties, + IWorkflowBase, TelemetryHelpers, } from 'n8n-workflow'; import { get as pslGet } from 'psl'; @@ -12,7 +13,6 @@ import { IDiagnosticInfo, IInternalHooksClass, ITelemetryUserDeletionData, - IWorkflowBase, IWorkflowDb, IExecutionTrackProperties, } from '@/Interfaces'; diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index e52a02ae6b458..bb6be2abada3c 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -27,13 +27,13 @@ import { IRun, IRunExecutionData, ITaskData, + IWorkflowBase, IWorkflowExecuteAdditionalData, IWorkflowExecuteHooks, IWorkflowHooksOptionalParameters, IWorkflowSettings, ErrorReporterProxy as ErrorReporter, LoggerProxy as Logger, - SubworkflowOperationError, Workflow, WorkflowExecuteMode, WorkflowHooks, @@ -51,7 +51,6 @@ import { IExecutionFlattedDb, IExecutionResponse, IPushDataExecutionFinished, - IWorkflowBase, IWorkflowExecuteProcess, IWorkflowExecutionDataProcess, IWorkflowErrorData, @@ -62,7 +61,7 @@ import * as Push from '@/Push'; import * as ResponseHelper from '@/ResponseHelper'; import * as WebhookHelpers from '@/WebhookHelpers'; import * as WorkflowHelpers from '@/WorkflowHelpers'; -import { getUserById, getWorkflowOwner, whereClause } from '@/UserManagement/UserManagementHelper'; +import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { findSubworkflowStart } from '@/utils'; import { PermissionChecker } from './UserManagement/PermissionChecker'; import { WorkflowsService } from './workflows/workflows.services'; @@ -94,6 +93,7 @@ export function executeErrorWorkflow( if (fullRunData.data.resultData.error !== undefined) { let workflowErrorData: IWorkflowErrorData; + const workflowId = workflowData.id; if (executionId) { // The error did happen in an execution @@ -107,7 +107,7 @@ export function executeErrorWorkflow( retryOf, }, workflow: { - id: workflowData.id !== undefined ? workflowData.id : undefined, + id: workflowId !== undefined ? workflowId : undefined, name: workflowData.name, }, }; @@ -119,7 +119,7 @@ export function executeErrorWorkflow( mode, }, workflow: { - id: workflowData.id !== undefined ? workflowData.id : undefined, + id: workflowId !== undefined ? workflowId : undefined, name: workflowData.name, }, }; @@ -128,30 +128,28 @@ export function executeErrorWorkflow( // Run the error workflow // To avoid an infinite loop do not run the error workflow again if the error-workflow itself failed and it is its own error-workflow. if ( - // eslint-disable-next-line @typescript-eslint/prefer-optional-chain - workflowData.settings !== undefined && - workflowData.settings.errorWorkflow && + workflowData.settings?.errorWorkflow && !( mode === 'error' && - workflowData.id && - workflowData.settings.errorWorkflow.toString() === workflowData.id + workflowId && + workflowData.settings.errorWorkflow.toString() === workflowId ) ) { Logger.verbose(`Start external error workflow`, { executionId, errorWorkflowId: workflowData.settings.errorWorkflow.toString(), - workflowId: workflowData.id, + workflowId, }); // If a specific error workflow is set run only that one // First, do permission checks. - if (!workflowData.id) { + if (!workflowId) { // Manual executions do not trigger error workflows // So this if should never happen. It was added to // make sure there are no possible security gaps return; } - getWorkflowOwner(workflowData.id) + getWorkflowOwner(workflowId) .then((user) => { void WorkflowHelpers.executeErrorWorkflow( workflowData.settings!.errorWorkflow as string, @@ -166,7 +164,7 @@ export function executeErrorWorkflow( { executionId, errorWorkflowId: workflowData.settings!.errorWorkflow!.toString(), - workflowId: workflowData.id, + workflowId, error, workflowErrorData, }, @@ -174,16 +172,12 @@ export function executeErrorWorkflow( }); } else if ( mode !== 'error' && - workflowData.id !== undefined && + workflowId !== undefined && workflowData.nodes.some((node) => node.type === ERROR_TRIGGER_TYPE) ) { - Logger.verbose(`Start internal error workflow`, { executionId, workflowId: workflowData.id }); - void getWorkflowOwner(workflowData.id).then((user) => { - void WorkflowHelpers.executeErrorWorkflow( - workflowData.id!.toString(), - workflowErrorData, - user, - ); + Logger.verbose(`Start internal error workflow`, { executionId, workflowId }); + void getWorkflowOwner(workflowId).then((user) => { + void WorkflowHelpers.executeErrorWorkflow(workflowId, workflowErrorData, user); }); } } diff --git a/packages/cli/src/commands/execute.ts b/packages/cli/src/commands/execute.ts index 0ee3c49d171c2..69ac3fc47482c 100644 --- a/packages/cli/src/commands/execute.ts +++ b/packages/cli/src/commands/execute.ts @@ -4,7 +4,7 @@ import { promises as fs } from 'fs'; import { Command, flags } from '@oclif/command'; import { BinaryDataManager, UserSettings, PLACEHOLDER_EMPTY_WORKFLOW_ID } from 'n8n-core'; -import { LoggerProxy } from 'n8n-workflow'; +import { LoggerProxy, IWorkflowBase } from 'n8n-workflow'; import * as ActiveExecutions from '@/ActiveExecutions'; import { CredentialsOverwrites } from '@/CredentialsOverwrites'; @@ -17,7 +17,7 @@ import { NodeTypes } from '@/NodeTypes'; import { InternalHooksManager } from '@/InternalHooksManager'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import { WorkflowRunner } from '@/WorkflowRunner'; -import { IWorkflowBase, IWorkflowExecutionDataProcess } from '@/Interfaces'; +import { IWorkflowExecutionDataProcess } from '@/Interfaces'; import { getLogger } from '@/Logger'; import config from '@/config'; import { getInstanceOwner } from '@/UserManagement/UserManagementHelper'; diff --git a/packages/cli/src/executions/executions.service.ts b/packages/cli/src/executions/executions.service.ts index 37f95864d773f..5b49b417e79ee 100644 --- a/packages/cli/src/executions/executions.service.ts +++ b/packages/cli/src/executions/executions.service.ts @@ -3,7 +3,15 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ import { validate as jsonSchemaValidate } from 'jsonschema'; import { BinaryDataManager } from 'n8n-core'; -import { deepCopy, IDataObject, LoggerProxy, JsonObject, jsonParse, Workflow } from 'n8n-workflow'; +import { + deepCopy, + IDataObject, + IWorkflowBase, + LoggerProxy, + JsonObject, + jsonParse, + Workflow, +} from 'n8n-workflow'; import { FindOperator, In, IsNull, LessThanOrEqual, Not, Raw } from 'typeorm'; import * as ActiveExecutions from '@/ActiveExecutions'; import config from '@/config'; @@ -12,7 +20,6 @@ import { IExecutionFlattedResponse, IExecutionResponse, IExecutionsListResponse, - IWorkflowBase, IWorkflowExecutionDataProcess, } from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; From c2a71615935d1e1199ad105456c1214e04fd1c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 19:33:07 +0100 Subject: [PATCH 07/18] remove unused line --- .../PublicApi/v1/handlers/credentials/credentials.handler.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts index a9a34e403f2e8..3bc402815ab3a 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.handler.ts @@ -77,8 +77,6 @@ export = { } await removeCredential(credential); - credential.id = credentialId; - return res.json(sanitizeCredentials(credential)); }, ], From 67ad7fb6a8308838edbcb4b0e58d1b0b431e4740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 19:56:02 +0100 Subject: [PATCH 08/18] convert more ids to strings --- packages/cli/src/PublicApi/types.d.ts | 10 +++++----- .../v1/handlers/executions/executions.handler.ts | 4 ++-- .../v1/handlers/executions/executions.service.ts | 14 +++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/PublicApi/types.d.ts b/packages/cli/src/PublicApi/types.d.ts index 4c782bdba384d..6a739bc2d5f7f 100644 --- a/packages/cli/src/PublicApi/types.d.ts +++ b/packages/cli/src/PublicApi/types.d.ts @@ -37,7 +37,7 @@ export type PaginatatedRequest = AuthenticatedRequest< limit?: number; cursor?: string; offset?: number; - lastId?: number; + lastId?: string; } >; export declare namespace ExecutionRequest { @@ -51,8 +51,8 @@ export declare namespace ExecutionRequest { cursor?: string; offset?: number; includeData?: boolean; - workflowId?: number; - lastId?: number; + workflowId?: string; + lastId?: string; } >; @@ -140,11 +140,11 @@ type PaginationBase = { limit: number }; type PaginationOffsetDecoded = PaginationBase & { offset: number }; -type PaginationCursorDecoded = PaginationBase & { lastId: number }; +type PaginationCursorDecoded = PaginationBase & { lastId: string }; type OffsetPagination = PaginationBase & { offset: number; numberOfTotalRecords: number }; -type CursorPagination = PaginationBase & { lastId: number; numberOfNextRecords: number }; +type CursorPagination = PaginationBase & { lastId: string; numberOfNextRecords: number }; export interface IRequired { required?: string[]; not?: { required?: string[] }; diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts index 181eec75a74e5..272bed8ca58ed 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts @@ -97,7 +97,7 @@ export = { // get running workflows so we exclude them from the result const runningExecutionsIds = ActiveExecutions.getInstance() .getActiveExecutions() - .map(({ id }) => Number(id)); + .map(({ id }) => id); const filters = { status, @@ -110,7 +110,7 @@ export = { const executions = await getExecutions(filters); - const newLastId = !executions.length ? 0 : Number(executions.slice(-1)[0].id); + const newLastId = !executions.length ? '0' : executions.slice(-1)[0].id; filters.lastId = newLastId; diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts index 34159656b7a13..d0a35dfd83f1c 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts @@ -60,14 +60,14 @@ function getExecutionSelectableProperties(includeData?: boolean): Array { type WhereClause = Record< string, - string | boolean | FindOperator> + string | boolean | FindOperator> >; let where: WhereClause = {}; @@ -103,10 +103,10 @@ export async function getExecutions(params: { export async function getExecutionsCount(data: { limit: number; - lastId?: number; - workflowIds?: number[]; + lastId?: string; + workflowIds?: string[]; status?: ExecutionStatus; - excludedWorkflowIds?: number[]; + excludedWorkflowIds?: string[]; }): Promise { const executions = await Db.collections.Execution.count({ where: { From 6ea26127a4f723407e6e4ff23afafc13b4f74f8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 20:16:23 +0100 Subject: [PATCH 09/18] only query for the id columns that are needed --- .../handlers/executions/executions.service.ts | 4 +-- .../cli/src/UserManagement/routes/users.ts | 34 +++++++++---------- packages/cli/src/WorkflowHelpers.ts | 10 ++++-- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts index d0a35dfd83f1c..d471897bbd09a 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/executions/executions.service.ts @@ -123,14 +123,14 @@ export async function getExecutionsCount(data: { export async function getExecutionInWorkflows( id: string, - workflows: string[], + workflowIds: string[], includeData?: boolean, ): Promise { const execution = await Db.collections.Execution.findOne({ select: getExecutionSelectableProperties(includeData), where: { id, - workflowId: In(workflows), + workflowId: In(workflowIds), }, }); diff --git a/packages/cli/src/UserManagement/routes/users.ts b/packages/cli/src/UserManagement/routes/users.ts index 0d62f2e05d702..46881f7f25ef7 100644 --- a/packages/cli/src/UserManagement/routes/users.ts +++ b/packages/cli/src/UserManagement/routes/users.ts @@ -426,13 +426,13 @@ export function usersNamespace(this: N8nApp): void { await Db.transaction(async (transactionManager) => { // Get all workflow ids belonging to user to delete - const sharedWorkflows = await transactionManager.getRepository(SharedWorkflow).find({ - where: { user: userToDelete, role: workflowOwnerRole }, - }); - - const sharedWorkflowIds = sharedWorkflows.map( - (sharedWorkflow) => sharedWorkflow.workflowId, - ); + const sharedWorkflowIds = await transactionManager + .getRepository(SharedWorkflow) + .find({ + select: ['workflowId'], + where: { user: userToDelete, role: workflowOwnerRole }, + }) + .then((sharedWorkflows) => sharedWorkflows.map(({ workflowId }) => workflowId)); // Prevents issues with unique key constraints since user being assigned // workflows and credentials might be a sharee @@ -451,21 +451,21 @@ export function usersNamespace(this: N8nApp): void { // Now do the same for creds // Get all workflow ids belonging to user to delete - const sharedCredentials = await transactionManager.getRepository(SharedCredentials).find({ - where: { user: userToDelete, role: credentialOwnerRole }, - }); - - const sharedCredentialIds = sharedCredentials.map( - (sharedCredential) => sharedCredential.credentialsId, - ); + const sharedCredentialIds = await transactionManager + .getRepository(SharedCredentials) + .find({ + select: ['credentialsId'], + where: { user: userToDelete, role: credentialOwnerRole }, + }) + .then((sharedCredentials) => + sharedCredentials.map(({ credentialsId }) => credentialsId), + ); // Prevents issues with unique key constraints since user being assigned // workflows and credentials might be a sharee await transactionManager.delete(SharedCredentials, { user: transferee, - credentials: In( - sharedCredentialIds.map((sharedCredentialId) => ({ id: sharedCredentialId })), - ), + credentialsId: In(sharedCredentialIds), }); // Transfer ownership of owned credentials diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index 53afb2f337fcb..471955db8ed89 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -399,6 +399,7 @@ export async function getSharedWorkflowIds(user: User, roles?: string[]): Promis const sharedWorkflows = await Db.collections.SharedWorkflow.find({ relations: ['workflow', 'role'], where: whereClause({ user, entityType: 'workflow', roles }), + select: ['workflowId'], }); return sharedWorkflows.map(({ workflowId }) => workflowId); @@ -417,9 +418,12 @@ export async function isBelowOnboardingThreshold(user: User): Promise { scope: 'workflow', }); const ownedWorkflowsIds = await Db.collections.SharedWorkflow.find({ - user, - role: workflowOwnerRole, - }).then((ownedWorkflows) => ownedWorkflows.map((wf) => wf.workflowId)); + where: { + user, + role: workflowOwnerRole, + }, + select: ['workflowId'], + }).then((ownedWorkflows) => ownedWorkflows.map(({ workflowId }) => workflowId)); if (ownedWorkflowsIds.length > 15) { belowThreshold = false; From 64e6a99a6ed3a980f579af166fdb44f27c3eb509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 20:18:49 +0100 Subject: [PATCH 10/18] delete unnecessary checks on isWorkflowIdValid --- packages/cli/src/WorkflowExecuteAdditionalData.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index bb6be2abada3c..2558f34394dff 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -107,7 +107,7 @@ export function executeErrorWorkflow( retryOf, }, workflow: { - id: workflowId !== undefined ? workflowId : undefined, + id: workflowId, name: workflowData.name, }, }; @@ -119,7 +119,7 @@ export function executeErrorWorkflow( mode, }, workflow: { - id: workflowId !== undefined ? workflowId : undefined, + id: workflowId, name: workflowData.name, }, }; @@ -588,7 +588,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { } const workflowId = this.workflowData.id; - if (workflowId !== undefined && WorkflowHelpers.isWorkflowIdValid(workflowId)) { + if (WorkflowHelpers.isWorkflowIdValid(workflowId)) { fullExecutionData.workflowId = workflowId; } @@ -719,7 +719,7 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { } const workflowId = this.workflowData.id; - if (workflowId !== undefined && WorkflowHelpers.isWorkflowIdValid(workflowId)) { + if (WorkflowHelpers.isWorkflowIdValid(workflowId)) { fullExecutionData.workflowId = workflowId; } From 326fa0139cc90f4d9b14522f23d1a9762ec73d46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 20:35:14 +0100 Subject: [PATCH 11/18] don't use unknown in idStringifier --- packages/cli/src/databases/utils/transformers.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/databases/utils/transformers.ts b/packages/cli/src/databases/utils/transformers.ts index fefcb3428ef68..df084889565df 100644 --- a/packages/cli/src/databases/utils/transformers.ts +++ b/packages/cli/src/databases/utils/transformers.ts @@ -1,10 +1,12 @@ import { jsonParse } from 'n8n-workflow'; -import { ValueTransformer } from 'typeorm'; +import type { ValueTransformer, FindOperator } from 'typeorm'; import config from '@/config'; export const idStringifier = { - from: (value: number): string => value?.toString(), - to: (value: string | unknown): number | unknown => + from: (value?: number): string | undefined => value?.toString(), + to: ( + value: string | FindOperator | undefined, + ): number | FindOperator | undefined => typeof value === 'string' ? Number(value) : value, }; From 7cb5221f30824008b252ec6f33ad95b06ac664e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 20:35:36 +0100 Subject: [PATCH 12/18] simplify query for sharedWorkflowData --- packages/cli/src/WorkflowHelpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index 471955db8ed89..f754e419e5e88 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -101,7 +101,7 @@ export async function executeErrorWorkflow( workflowData = await Db.collections.Workflow.findOne({ id: workflowId }); } else { const sharedWorkflowData = await Db.collections.SharedWorkflow.findOne({ - where: { workflowId, user }, + where: { workflowId, userId: user.id }, relations: ['workflow'], }); if (sharedWorkflowData) { From b717cfb6dea54c4e0bbd60dcb46e5ed1dec893c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 20:35:46 +0100 Subject: [PATCH 13/18] use type import --- packages/cli/src/events/WorkflowStatistics.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/events/WorkflowStatistics.ts b/packages/cli/src/events/WorkflowStatistics.ts index 2606553beb625..ffa95f8d80bf9 100644 --- a/packages/cli/src/events/WorkflowStatistics.ts +++ b/packages/cli/src/events/WorkflowStatistics.ts @@ -1,4 +1,4 @@ -import { INode, IRun, IWorkflowBase } from 'n8n-workflow'; +import type { INode, IRun, IWorkflowBase } from 'n8n-workflow'; import * as Db from '@/Db'; import { InternalHooksManager } from '@/InternalHooksManager'; import { StatisticsNames } from '@/databases/entities/WorkflowStatistics'; From b4a18297155fbdb55a32c7148456c411f79c99ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 20:42:19 +0100 Subject: [PATCH 14/18] validate rather than assert --- packages/cli/src/events/WorkflowStatistics.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/events/WorkflowStatistics.ts b/packages/cli/src/events/WorkflowStatistics.ts index ffa95f8d80bf9..909940cfa796c 100644 --- a/packages/cli/src/events/WorkflowStatistics.ts +++ b/packages/cli/src/events/WorkflowStatistics.ts @@ -22,7 +22,8 @@ export async function workflowExecutionCompleted( } // Get the workflow id - const workflowId = workflowData.id!; + const workflowId = workflowData.id; + if (workflowId === undefined) return; // Try insertion and if it fails due to key conflicts then update the existing entry instead try { From d820f73fdc191626dbb0e13fca65bba28fbcb61d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 28 Dec 2022 20:51:56 +0100 Subject: [PATCH 15/18] address remaining PR comments --- packages/cli/src/executions/executions.service.ts | 2 +- packages/cli/src/workflows/workflows.services.ee.ts | 9 ++++----- packages/cli/src/workflows/workflows.services.ts | 2 +- packages/cli/test/integration/credentials.test.ts | 2 +- packages/cli/test/unit/PermissionChecker.test.ts | 4 ++-- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/executions/executions.service.ts b/packages/cli/src/executions/executions.service.ts index 5b49b417e79ee..02b5893c14c45 100644 --- a/packages/cli/src/executions/executions.service.ts +++ b/packages/cli/src/executions/executions.service.ts @@ -461,7 +461,7 @@ export class ExecutionsService { }; let query = Db.collections.Execution.createQueryBuilder() - .select() + .select('id') .where({ ...filters, workflowId: In(sharedWorkflowIds), diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts index 351f8f36e7724..4f57b61f0636f 100644 --- a/packages/cli/src/workflows/workflows.services.ee.ts +++ b/packages/cli/src/workflows/workflows.services.ee.ts @@ -56,7 +56,7 @@ export class EEWorkflowsService extends WorkflowsService { ): Promise { return transaction.delete(SharedWorkflow, { workflowId, - user: { id: Not(In(userIds)) }, + userId: Not(In(userIds)), }); } @@ -231,10 +231,9 @@ export class EEWorkflowsService extends WorkflowsService { return; } Object.keys(node.credentials).forEach((credentialType) => { - const credentialId = node.credentials?.[credentialType].id ?? ''; - const matchedCredential = allowedCredentials.find( - (credential) => credential.id === credentialId, - ); + const credentialId = node.credentials?.[credentialType].id; + if (credentialId === undefined) return; + const matchedCredential = allowedCredentials.find(({ id }) => id === credentialId); if (!matchedCredential) { throw new Error('The workflow contains credentials that you do not have access to'); } diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index 6fd3842a6edda..d4dea6123c158 100644 --- a/packages/cli/src/workflows/workflows.services.ts +++ b/packages/cli/src/workflows/workflows.services.ts @@ -16,7 +16,7 @@ import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import * as TagHelpers from '@/TagHelpers'; import { WorkflowRequest } from '@/requests'; -import { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; +import type { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; import { WorkflowRunner } from '@/WorkflowRunner'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; diff --git a/packages/cli/test/integration/credentials.test.ts b/packages/cli/test/integration/credentials.test.ts index df9ca9ea01257..7e19e543eeda5 100644 --- a/packages/cli/test/integration/credentials.test.ts +++ b/packages/cli/test/integration/credentials.test.ts @@ -81,7 +81,7 @@ test('GET /credentials should return all creds for owner', async () => { response.body.data.forEach((credential: CredentialsEntity) => { validateMainCredentialData(credential); expect(credential.data).toBeUndefined(); - expect(savedCredentialsIds.includes(credential.id)).toBe(true); + expect(savedCredentialsIds).toContain(credential.id); }); }); diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/unit/PermissionChecker.test.ts index 7a61c6bc2e8af..e5a22dece9c96 100644 --- a/packages/cli/test/unit/PermissionChecker.test.ts +++ b/packages/cli/test/unit/PermissionChecker.test.ts @@ -160,7 +160,7 @@ describe('PermissionChecker.check()', () => { const memberCred = await saveCredential(randomCred(), { user: member }); const workflowDetails = { - id: `${randomPositiveDigit()}`, + id: randomPositiveDigit().toString(), name: 'test', active: false, connections: {}, @@ -205,7 +205,7 @@ describe('PermissionChecker.check()', () => { role: workflowOwnerRole, }); - const workflow = new Workflow({ ...workflowDetails, id: workflowDetails.id }); + const workflow = new Workflow(workflowDetails); expect(PermissionChecker.check(workflow, member.id)).rejects.toThrow(); }); From 25f0321db4d4a5cecc213617f6050d65f7368865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 29 Dec 2022 18:22:53 +0100 Subject: [PATCH 16/18] address PR comments --- packages/cli/src/Interfaces.ts | 10 +++++----- packages/cli/src/UserManagement/routes/users.ts | 2 +- .../cli/src/credentials/credentials.controller.ee.ts | 6 +++--- packages/cli/src/credentials/credentials.service.ts | 2 +- packages/cli/src/workflows/workflows.services.ts | 2 +- .../src/components/CredentialEdit/CredentialConfig.vue | 2 +- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 27813ebf0522b..915cbfb69060a 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -148,8 +148,8 @@ export interface IExecutionBase { stoppedAt?: Date; // empty value means execution is still running workflowId?: string; // To be able to filter executions easily // finished: boolean; - retryOf?: number | string; // If it is a retry, the id of the execution it is a retry of. - retrySuccessId?: number | string; // If it failed and a retry did succeed. The id of the successful retry. + retryOf?: string; // If it is a retry, the id of the execution it is a retry of. + retrySuccessId?: string; // If it failed and a retry did succeed. The id of the successful retry. } // Data in regular format with references @@ -199,8 +199,8 @@ export interface IExecutionResponseApi { stoppedAt?: Date; workflowId?: string; finished: boolean; - retryOf?: number | string; - retrySuccessId?: number | string; + retryOf?: string; + retrySuccessId?: string; data?: object; waitTill?: Date | null; workflowData: IWorkflowBase; @@ -655,7 +655,7 @@ export interface IWorkflowExecutionDataProcess { executionData?: IRunExecutionData; runData?: IRunData; pinData?: IPinData; - retryOf?: number | string; + retryOf?: string; sessionId?: string; startNodes?: string[]; workflowData: IWorkflowBase; diff --git a/packages/cli/src/UserManagement/routes/users.ts b/packages/cli/src/UserManagement/routes/users.ts index 213911e865242..c35b6c98e7bec 100644 --- a/packages/cli/src/UserManagement/routes/users.ts +++ b/packages/cli/src/UserManagement/routes/users.ts @@ -430,7 +430,7 @@ export function usersNamespace(this: N8nApp): void { .getRepository(SharedWorkflow) .find({ select: ['workflowId'], - where: { user: userToDelete, role: workflowOwnerRole }, + where: { userId: userToDelete.id, role: workflowOwnerRole }, }) .then((sharedWorkflows) => sharedWorkflows.map(({ workflowId }) => workflowId)); diff --git a/packages/cli/src/credentials/credentials.controller.ee.ts b/packages/cli/src/credentials/credentials.controller.ee.ts index 0775c53ac5001..a06f257723250 100644 --- a/packages/cli/src/credentials/credentials.controller.ee.ts +++ b/packages/cli/src/credentials/credentials.controller.ee.ts @@ -103,10 +103,10 @@ EECredentialsController.post( const encryptionKey = await EECredentials.getEncryptionKey(); - const credentialsId = credentials.id; - const { ownsCredential } = await EECredentials.isOwned(req.user, credentialsId); + const credentialId = credentials.id; + const { ownsCredential } = await EECredentials.isOwned(req.user, credentialId); - const sharing = await EECredentials.getSharing(req.user, credentialsId); + const sharing = await EECredentials.getSharing(req.user, credentialId); if (!ownsCredential) { if (!sharing) { throw new ResponseHelper.UnauthorizedError('Forbidden'); diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index d31be8be31312..594e504ae96d5 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -249,7 +249,7 @@ export class CredentialsService { return savedCredential; }); LoggerProxy.verbose('New credential created', { - credentialsId: newCredential.id, + credentialId: newCredential.id, ownerId: user.id, }); return result; diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index 9c7b76bcbdb00..41bbc6d806032 100644 --- a/packages/cli/src/workflows/workflows.services.ts +++ b/packages/cli/src/workflows/workflows.services.ts @@ -55,7 +55,7 @@ export class WorkflowsService { // owner. This allows the global owner to view and delete // workflows they don't own. if (!allowGlobalOwner || user.globalRole.name !== 'owner') { - where.user = { id: user.id }; + where.userId = user.id; } return Db.collections.SharedWorkflow.findOne({ where, relations }); diff --git a/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue b/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue index 5069b1dbba234..e366d92e39c3d 100644 --- a/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue +++ b/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue @@ -153,7 +153,7 @@ export default mixins(restApi).extend({ }, credentialData: {}, credentialId: { - type: [String, Number], + type: String, default: '', }, showValidationWarning: { From 3239bea1043092e8c3c5a5fedae96f42fed1a1b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 2 Jan 2023 14:29:07 +0100 Subject: [PATCH 17/18] update public api specs --- .../v1/handlers/credentials/spec/paths/credentials.id.yml | 2 +- .../v1/handlers/credentials/spec/schemas/credential.yml | 4 ++-- .../v1/handlers/executions/spec/paths/executions.yml | 4 ++-- .../v1/handlers/executions/spec/schemas/execution.yml | 8 ++++---- .../executions/spec/schemas/parameters/executionId.yml | 2 +- .../workflows/spec/schemas/parameters/workflowId.yml | 2 +- .../PublicApi/v1/handlers/workflows/spec/schemas/tag.yml | 2 +- .../v1/handlers/workflows/spec/schemas/workflow.yml | 4 ++-- .../handlers/workflows/spec/schemas/workflowSettings.yml | 2 +- 9 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/spec/paths/credentials.id.yml b/packages/cli/src/PublicApi/v1/handlers/credentials/spec/paths/credentials.id.yml index b80f81ac02bba..7ab682ffabd46 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/spec/paths/credentials.id.yml +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/spec/paths/credentials.id.yml @@ -12,7 +12,7 @@ delete: description: The credential ID that needs to be deleted required: true schema: - type: number + type: string responses: '200': description: Operation successful. diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/spec/schemas/credential.yml b/packages/cli/src/PublicApi/v1/handlers/credentials/spec/schemas/credential.yml index 9c66817475c02..92660184c428a 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/spec/schemas/credential.yml +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/spec/schemas/credential.yml @@ -5,9 +5,9 @@ required: type: object properties: id: - type: number + type: string readOnly: true - example: 42 + example: '42' name: type: string example: Joe's Github Credentials diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/spec/paths/executions.yml b/packages/cli/src/PublicApi/v1/handlers/executions/spec/paths/executions.yml index 0f95e82881f7b..8d26492ec30bf 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/spec/paths/executions.yml +++ b/packages/cli/src/PublicApi/v1/handlers/executions/spec/paths/executions.yml @@ -19,8 +19,8 @@ get: description: Workflow to filter the executions by. required: false schema: - type: number - example: 1000 + type: string + example: '1000' - $ref: '../../../../shared/spec/parameters/limit.yml' - $ref: '../../../../shared/spec/parameters/cursor.yml' responses: diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/spec/schemas/execution.yml b/packages/cli/src/PublicApi/v1/handlers/executions/spec/schemas/execution.yml index 883baa9fa7076..19e5120fcd6c6 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/spec/schemas/execution.yml +++ b/packages/cli/src/PublicApi/v1/handlers/executions/spec/schemas/execution.yml @@ -1,8 +1,8 @@ type: object properties: id: - type: number - example: 1000 + type: string + example: '1000' data: type: object finished: @@ -17,7 +17,7 @@ properties: retrySuccessId: type: string nullable: true - example: 2 + example: '2' startedAt: type: string format: date-time @@ -26,7 +26,7 @@ properties: format: date-time workflowId: type: string - example: 1000 + example: '1000' waitTill: type: string nullable: true diff --git a/packages/cli/src/PublicApi/v1/handlers/executions/spec/schemas/parameters/executionId.yml b/packages/cli/src/PublicApi/v1/handlers/executions/spec/schemas/parameters/executionId.yml index 6ec1bdf094b44..f13da084e4ff9 100644 --- a/packages/cli/src/PublicApi/v1/handlers/executions/spec/schemas/parameters/executionId.yml +++ b/packages/cli/src/PublicApi/v1/handlers/executions/spec/schemas/parameters/executionId.yml @@ -3,4 +3,4 @@ in: path description: The ID of the execution. required: true schema: - type: number + type: string diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/parameters/workflowId.yml b/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/parameters/workflowId.yml index fb802e67a2775..e8389a78b051b 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/parameters/workflowId.yml +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/parameters/workflowId.yml @@ -3,4 +3,4 @@ in: path description: The ID of the workflow. required: true schema: - type: number + type: string diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/tag.yml b/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/tag.yml index ff5b571504dcb..40a831b2aeaee 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/tag.yml +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/tag.yml @@ -2,7 +2,7 @@ type: object properties: id: type: string - example: 12 + example: '12' name: type: string example: Production diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/workflow.yml b/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/workflow.yml index f04070f85b5d7..2582f850e502b 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/workflow.yml +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/workflow.yml @@ -7,9 +7,9 @@ required: - settings properties: id: - type: number + type: string readOnly: true - example: 1 + example: '1' name: type: string example: Workflow 1 diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/workflowSettings.yml b/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/workflowSettings.yml index 78b08b90a6cf3..403b7ac4c50ca 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/workflowSettings.yml +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/spec/schemas/workflowSettings.yml @@ -17,7 +17,7 @@ properties: maxLength: 3600 errorWorkflow: type: string - example: 10 + example: '10' description: The ID of the workflow that contains the error trigger node. timezone: type: string From 3c47d1609137d86adc93a501525ef13846c0977c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 2 Jan 2023 14:37:20 +0100 Subject: [PATCH 18/18] Update executeBatch.ts --- packages/cli/src/commands/executeBatch.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/commands/executeBatch.ts b/packages/cli/src/commands/executeBatch.ts index 5364dd9d16532..6614230e2123f 100644 --- a/packages/cli/src/commands/executeBatch.ts +++ b/packages/cli/src/commands/executeBatch.ts @@ -36,6 +36,8 @@ import { User } from '@db/entities/User'; import { getInstanceOwner } from '@/UserManagement/UserManagementHelper'; import { findCliWorkflowStart } from '@/utils'; +const re = /\d+/; + export class ExecuteBatch extends Command { static description = '\nExecutes multiple workflows once'; @@ -199,8 +201,8 @@ export class ExecuteBatch extends Command { ExecuteBatch.debug = flags.debug; ExecuteBatch.concurrency = flags.concurrency || 1; - const ids: number[] = []; - const skipIds: number[] = []; + const ids: string[] = []; + const skipIds: string[] = []; if (flags.snapshot !== undefined) { if (fs.existsSync(flags.snapshot)) { @@ -241,13 +243,10 @@ export class ExecuteBatch extends Command { if (flags.ids !== undefined) { if (fs.existsSync(flags.ids)) { const contents = fs.readFileSync(flags.ids, { encoding: 'utf-8' }); - ids.push(...contents.split(',').map((id) => parseInt(id.trim(), 10))); + ids.push(...contents.split(',').filter((id) => re.exec(id))); } else { const paramIds = flags.ids.split(','); - const re = /\d+/; - const matchedIds = paramIds - .filter((id) => re.exec(id)) - .map((id) => parseInt(id.trim(), 10)); + const matchedIds = paramIds.filter((id) => re.exec(id)); if (matchedIds.length === 0) { console.log( @@ -263,7 +262,7 @@ export class ExecuteBatch extends Command { if (flags.skipList !== undefined) { if (fs.existsSync(flags.skipList)) { const contents = fs.readFileSync(flags.skipList, { encoding: 'utf-8' }); - skipIds.push(...contents.split(',').map((id) => parseInt(id.trim(), 10))); + skipIds.push(...contents.split(',').filter((id) => re.exec(id))); } else { console.log('Skip list file not found. Exiting.'); return;