Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Improve workflows list performance #5021

Merged
merged 6 commits into from
Dec 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions packages/cli/src/credentials/credentials.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<CredentialsEntity> {
Expand Down
37 changes: 11 additions & 26 deletions packages/cli/src/workflows/workflows.controller.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}),
);

Expand Down Expand Up @@ -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);
}),
);

Expand All @@ -204,19 +199,14 @@ EEWorkflowController.post(
EEWorkflowController.get(
'/',
ResponseHelper.send(async (req: WorkflowRequest.GetAll) => {
const workflows = (await EEWorkflows.getMany(
req.user,
req.query.filter,
)) as unknown as WorkflowEntity[];

return Promise.all(
workflows.map(async (workflow) => {
EEWorkflows.addOwnerAndSharings(workflow);
await EEWorkflows.addCredentialsToWorkflow(workflow, req.user);
workflow.nodes = [];
return { ...workflow, id: workflow.id.toString() };
}),
);
const workflows = await EEWorkflows.getMany(req.user, req.query.filter);
await EEWorkflows.addCredentialsToWorkflows(workflows, req.user);

return workflows.map((workflow) => {
EEWorkflows.addOwnerAndSharings(workflow);
workflow.nodes = [];
return EEWorkflows.entityToResponse(workflow);
});
}),
);

Expand All @@ -240,12 +230,7 @@ EEWorkflowController.patch(
forceSave,
);

const { id, ...remainder } = updatedWorkflow;

return {
id: id.toString(),
...remainder,
};
return EEWorkflows.entityToResponse(updatedWorkflow);
}),
);

Expand Down
26 changes: 5 additions & 21 deletions packages/cli/src/workflows/workflows.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}),
);

Expand All @@ -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));
}),
);

Expand Down Expand Up @@ -222,14 +218,7 @@ workflowsController.get(
);
}

const {
workflow: { id, ...rest },
} = shared;

return {
id: id.toString(),
...rest,
};
return WorkflowsService.entityToResponse(shared.workflow);
}),
);

Expand All @@ -255,12 +244,7 @@ workflowsController.patch(
['owner'],
);

const { id, ...remainder } = updatedWorkflow;

return {
id: id.toString(),
...remainder,
};
return WorkflowsService.entityToResponse(updatedWorkflow);
}),
);

Expand Down
69 changes: 68 additions & 1 deletion packages/cli/src/workflows/workflows.services.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -154,6 +156,71 @@ export class EEWorkflowsService extends WorkflowsService {
});
}

static async addCredentialsToWorkflows(
workflows: WorkflowWithSharingsAndCredentials[],
currentUser: User,
): Promise<void> {
// Create 2 maps: one with all the credential ids used by all workflows
// And another to match back workflow <> credentials
const allUsedCredentialIds = new Set<string>();
const mapsWorkflowsToUsedCredentials: string[][] = [];
workflows.forEach((workflow, idx) => {
workflow.nodes.forEach((node) => {
if (!node.credentials) {
return;
}
Object.keys(node.credentials).forEach((credentialType) => {
const credential = node.credentials?.[credentialType];
if (!credential?.id) {
return;
}
if (!mapsWorkflowsToUsedCredentials[idx]) {
mapsWorkflowsToUsedCredentials[idx] = [];
}
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<string, CredentialUsedByWorkflow> = {};
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[],
Expand Down
22 changes: 9 additions & 13 deletions packages/cli/src/workflows/workflows.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<string[]> {
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<WorkflowEntity[]> {
const sharedWorkflowIds = await this.getWorkflowIdsForUser(user, ['owner']);
if (sharedWorkflowIds.length === 0) {
// return early since without shared workflows there can be no hits
Expand Down Expand Up @@ -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(
Expand Down