From 383273b1b18fea8c1c24a657186c1d0b897c3ab6 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Fri, 23 Dec 2022 11:01:21 +0100 Subject: [PATCH 1/5] spike: Improve workflow list performance --- .../src/workflows/workflows.controller.ee.ts | 2 +- .../src/workflows/workflows.services.ee.ts | 65 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index c32aec1db48f9..cd5fc2aa08e1d 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -208,11 +208,11 @@ EEWorkflowController.get( req.user, req.query.filter, )) as unknown as WorkflowEntity[]; + await EEWorkflows.addCredentialsToWorkflows(workflows, req.user); return Promise.all( workflows.map(async (workflow) => { EEWorkflows.addOwnerAndSharings(workflow); - await EEWorkflows.addCredentialsToWorkflow(workflow, req.user); workflow.nodes = []; return { ...workflow, id: workflow.id.toString() }; }), diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts index f13ddf5456e84..a81db277e4ce4 100644 --- a/packages/cli/src/workflows/workflows.services.ee.ts +++ b/packages/cli/src/workflows/workflows.services.ee.ts @@ -154,6 +154,71 @@ export class EEWorkflowsService extends WorkflowsService { }); } + static async addCredentialsToWorkflows( + workflows: WorkflowWithSharingsAndCredentials[], + currentUser: User, + ): Promise { + // Create 2 maps: one with all the credential ids used by all workflows + // And another to match back workflow <> credentials + const allUsedCredentialIds = new Set(); + const mapsWorkflowsToUsedCredentials: string[][] = []; + workflows.forEach((workflow, idx) => { + workflow.nodes.forEach((node) => { + if (!node.credentials) { + return; + } + Object.keys(node.credentials).forEach((credentialType) => { + if (!mapsWorkflowsToUsedCredentials[idx]) { + mapsWorkflowsToUsedCredentials[idx] = []; + } + const credential = node.credentials?.[credentialType]; + if (!credential?.id) { + return; + } + mapsWorkflowsToUsedCredentials[idx].push(credential.id); + allUsedCredentialIds.add(credential.id); + }); + }); + }); + + const usedWorkflowsCredentials = await EECredentials.getMany({ + where: { + id: In(Array.from(allUsedCredentialIds)), + }, + relations: ['shared', 'shared.user', 'shared.role'], + }); + const userCredentials = await EECredentials.getAll(currentUser, { disableGlobalRole: true }); + const userCredentialIds = userCredentials.map((credential) => credential.id.toString()); + const credentialsMap: Record = {}; + usedWorkflowsCredentials.forEach((credential) => { + credentialsMap[credential.id.toString()] = { + id: credential.id.toString(), + name: credential.name, + type: credential.type, + currentUserHasAccess: userCredentialIds.includes(credential.id.toString()), + 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 }; + } else { + credentialsMap[credential.id.toString()].sharedWith?.push({ + id, + email, + firstName, + lastName, + }); + } + }); + }); + + mapsWorkflowsToUsedCredentials.forEach((usedCredentialIds, idx) => { + workflows[idx].usedCredentials = usedCredentialIds.map((id) => credentialsMap[id]); + }); + } + static validateCredentialPermissionsToUser( workflow: WorkflowEntity, allowedCredentials: ICredentialsDb[], From 2e5ce7604db21adadf6a46c6252539d007b03d99 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Fri, 23 Dec 2022 11:57:58 +0100 Subject: [PATCH 2/5] fix: Correcting override behavior --- packages/cli/src/workflows/workflows.services.ee.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts index a81db277e4ce4..c6f188b0a9ef9 100644 --- a/packages/cli/src/workflows/workflows.services.ee.ts +++ b/packages/cli/src/workflows/workflows.services.ee.ts @@ -89,7 +89,9 @@ export class EEWorkflowsService extends WorkflowsService { static addOwnerAndSharings(workflow: WorkflowWithSharingsAndCredentials): void { workflow.ownedBy = null; workflow.sharedWith = []; - workflow.usedCredentials = []; + if (!workflow.usedCredentials) { + workflow.usedCredentials = []; + } workflow.shared?.forEach(({ user, role }) => { const { id, email, firstName, lastName } = user; @@ -168,13 +170,13 @@ export class EEWorkflowsService extends WorkflowsService { return; } Object.keys(node.credentials).forEach((credentialType) => { - if (!mapsWorkflowsToUsedCredentials[idx]) { - mapsWorkflowsToUsedCredentials[idx] = []; - } const credential = node.credentials?.[credentialType]; if (!credential?.id) { return; } + if (!mapsWorkflowsToUsedCredentials[idx]) { + mapsWorkflowsToUsedCredentials[idx] = []; + } mapsWorkflowsToUsedCredentials[idx].push(credential.id); allUsedCredentialIds.add(credential.id); }); From 6e5f615a21f9e3966c549c8da1c8290fba580952 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Fri, 23 Dec 2022 12:53:41 +0100 Subject: [PATCH 3/5] refactor: Remove unnecessary promise --- .../cli/src/workflows/workflows.controller.ee.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index cd5fc2aa08e1d..72b11b0407a17 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -210,13 +210,11 @@ EEWorkflowController.get( )) as unknown as WorkflowEntity[]; await EEWorkflows.addCredentialsToWorkflows(workflows, req.user); - return Promise.all( - workflows.map(async (workflow) => { - EEWorkflows.addOwnerAndSharings(workflow); - workflow.nodes = []; - return { ...workflow, id: workflow.id.toString() }; - }), - ); + return workflows.map(async (workflow) => { + EEWorkflows.addOwnerAndSharings(workflow); + workflow.nodes = []; + return { ...workflow, id: workflow.id.toString() }; + }); }), ); From cd161b2ce50ec3d41ddc1c1f0ecaac37725a49d7 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: Fri, 23 Dec 2022 13:17:11 +0100 Subject: [PATCH 4/5] remove duplicate code --- .../src/credentials/credentials.service.ts | 11 -------- .../src/workflows/workflows.controller.ee.ts | 23 ++++------------ .../cli/src/workflows/workflows.controller.ts | 26 ++++--------------- .../cli/src/workflows/workflows.services.ts | 22 +++++++--------- 4 files changed, 19 insertions(+), 63 deletions(-) diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index c928e042a28fe..3a0a2f304a86e 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -125,17 +125,6 @@ export class CredentialsService { return Db.collections.SharedCredentials.findOne(options); } - static createCredentialsFromCredentialsEntity( - credential: CredentialsEntity, - encrypt = false, - ): Credentials { - const { id, name, type, nodesAccess, data } = credential; - if (encrypt) { - return new Credentials({ id: null, name }, type, nodesAccess); - } - return new Credentials({ id: id.toString(), name }, type, nodesAccess, data); - } - static async prepareCreateData( data: CredentialRequest.CredentialProperties, ): Promise { diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index 72b11b0407a17..8a1bcc983a6a1 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -108,7 +108,7 @@ EEWorkflowController.get( EEWorkflows.addOwnerAndSharings(workflow); await EEWorkflows.addCredentialsToWorkflow(workflow, req.user); - return { ...workflow, id: workflow.id.toString() }; + return EEWorkflows.entityToResponse(workflow); }), ); @@ -189,12 +189,7 @@ EEWorkflowController.post( await ExternalHooks().run('workflow.afterCreate', [savedWorkflow]); void InternalHooksManager.getInstance().onWorkflowCreated(req.user.id, newWorkflow, false); - const { id, ...rest } = savedWorkflow; - - return { - id: id.toString(), - ...rest, - }; + return EEWorkflows.entityToResponse(savedWorkflow); }), ); @@ -204,16 +199,13 @@ EEWorkflowController.post( EEWorkflowController.get( '/', ResponseHelper.send(async (req: WorkflowRequest.GetAll) => { - const workflows = (await EEWorkflows.getMany( - req.user, - req.query.filter, - )) as unknown as WorkflowEntity[]; + const workflows = await EEWorkflows.getMany(req.user, req.query.filter); await EEWorkflows.addCredentialsToWorkflows(workflows, req.user); return workflows.map(async (workflow) => { EEWorkflows.addOwnerAndSharings(workflow); workflow.nodes = []; - return { ...workflow, id: workflow.id.toString() }; + return EEWorkflows.entityToResponse(workflow); }); }), ); @@ -238,12 +230,7 @@ EEWorkflowController.patch( forceSave, ); - const { id, ...remainder } = updatedWorkflow; - - return { - id: id.toString(), - ...remainder, - }; + return EEWorkflows.entityToResponse(updatedWorkflow); }), ); diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 03ee00b77ee8d..f144da5c61935 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -106,12 +106,7 @@ workflowsController.post( await ExternalHooks().run('workflow.afterCreate', [savedWorkflow]); void InternalHooksManager.getInstance().onWorkflowCreated(req.user.id, newWorkflow, false); - const { id, ...rest } = savedWorkflow; - - return { - id: id.toString(), - ...rest, - }; + return WorkflowsService.entityToResponse(savedWorkflow); }), ); @@ -121,7 +116,8 @@ workflowsController.post( workflowsController.get( '/', ResponseHelper.send(async (req: WorkflowRequest.GetAll) => { - return WorkflowsService.getMany(req.user, req.query.filter); + const workflows = await WorkflowsService.getMany(req.user, req.query.filter); + return workflows.map((workflow) => WorkflowsService.entityToResponse(workflow)); }), ); @@ -222,14 +218,7 @@ workflowsController.get( ); } - const { - workflow: { id, ...rest }, - } = shared; - - return { - id: id.toString(), - ...rest, - }; + return WorkflowsService.entityToResponse(shared.workflow); }), ); @@ -255,12 +244,7 @@ workflowsController.patch( ['owner'], ); - const { id, ...remainder } = updatedWorkflow; - - return { - id: id.toString(), - ...remainder, - }; + return WorkflowsService.entityToResponse(updatedWorkflow); }), ); diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index bd9c3345d4411..81a8d642d434a 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 { IWorkflowDb, IWorkflowExecutionDataProcess, IWorkflowResponse } from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; import { WorkflowRunner } from '@/WorkflowRunner'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; @@ -115,12 +115,17 @@ export class WorkflowsService { return Db.collections.Workflow.findOne(workflow, options); } - // Warning: this function is overriden by EE to disregard role list. + // Warning: this function is overridden by EE to disregard role list. static async getWorkflowIdsForUser(user: User, roles?: string[]): Promise { return getSharedWorkflowIds(user, roles); } - static async getMany(user: User, rawFilter: string) { + 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) { // return early since without shared workflows there can be no hits @@ -185,16 +190,7 @@ export class WorkflowsService { }, }; - const workflows = await Db.collections.Workflow.find(query); - - return workflows.map((workflow) => { - const { id, ...rest } = workflow; - - return { - id: id.toString(), - ...rest, - }; - }); + return Db.collections.Workflow.find(query); } static async update( From 5b2d30b513425e32ab491debddce937260a22c18 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: Fri, 23 Dec 2022 13:35:14 +0100 Subject: [PATCH 5/5] remove the `async` that is breaking the listings page --- packages/cli/src/workflows/workflows.controller.ee.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index 8a1bcc983a6a1..b50e464d0adaf 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -202,7 +202,7 @@ EEWorkflowController.get( const workflows = await EEWorkflows.getMany(req.user, req.query.filter); await EEWorkflows.addCredentialsToWorkflows(workflows, req.user); - return workflows.map(async (workflow) => { + return workflows.map((workflow) => { EEWorkflows.addOwnerAndSharings(workflow); workflow.nodes = []; return EEWorkflows.entityToResponse(workflow);