fix(core): Optimize getSharedWorkflowIds query (#6314)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
This commit is contained in:
Michael Auerswald 2023-05-26 18:02:55 +02:00 committed by GitHub
parent 7a7b884793
commit 0631f69d98
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 55 additions and 12 deletions

View file

@ -26,6 +26,7 @@ const config = {
collectCoverage: true,
coverageReporters: [process.env.COVERAGE_REPORT === 'true' ? 'text' : 'text-summary'],
collectCoverageFrom: ['src/**/*.ts'],
testTimeout: 10_000,
};
if (process.env.CI === 'true') {

View file

@ -1,3 +1,4 @@
import type { FindOptionsWhere } from 'typeorm';
import { In } from 'typeorm';
import { Container } from 'typedi';
import type {
@ -26,16 +27,16 @@ import type {
} from '@/Interfaces';
import { NodeTypes } from '@/NodeTypes';
import { WorkflowRunner } from '@/WorkflowRunner';
import config from '@/config';
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
import type { User } from '@db/entities/User';
import { RoleRepository } from '@db/repositories';
import { whereClause } from '@/UserManagement/UserManagementHelper';
import omit from 'lodash.omit';
import { PermissionChecker } from './UserManagement/PermissionChecker';
import { isWorkflowIdValid } from './utils';
import { UserService } from './user/user.service';
import type { SharedWorkflow } from './databases/entities/SharedWorkflow';
import type { RoleNames } from './databases/entities/Role';
const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType');
@ -372,16 +373,24 @@ export async function replaceInvalidCredentials(workflow: WorkflowEntity): Promi
* Get the IDs of the workflows that have been shared with the user.
* Returns all IDs if user is global owner (see `whereClause`)
*/
export async function getSharedWorkflowIds(user: User, roles?: string[]): Promise<string[]> {
export async function getSharedWorkflowIds(user: User, roles?: RoleNames[]): Promise<string[]> {
const where: FindOptionsWhere<SharedWorkflow> = {};
if (user.globalRole?.name !== 'owner') {
where.userId = user.id;
}
if (roles?.length) {
const roleIds = await Db.collections.Role.find({
select: ['id'],
where: { name: In(roles), scope: 'workflow' },
}).then((data) => data.map(({ id }) => id));
where.roleId = In(roleIds);
}
const sharedWorkflows = await Db.collections.SharedWorkflow.find({
relations: ['workflow', 'role'],
where: whereClause({ user, entityType: 'workflow', roles }),
where,
select: ['workflowId'],
});
return sharedWorkflows.map(({ workflowId }) => workflowId);
}
/**
* Check if user owns more than 15 workflows or more than 2 workflows with at least 2 nodes.
* If user does, set flag in its settings.

View file

@ -1,16 +1,18 @@
import convict from 'convict';
import dotenv from 'dotenv';
import { tmpdir } from 'os';
import { mkdtempSync, readFileSync } from 'fs';
import { mkdirSync, mkdtempSync, readFileSync } from 'fs';
import { join } from 'path';
import { schema } from './schema';
import { inTest, inE2ETests } from '@/constants';
if (inE2ETests) {
const testsDir = join(tmpdir(), 'n8n-e2e/');
mkdirSync(testsDir, { recursive: true });
// Skip loading config from env variables in end-to-end tests
process.env = {
E2E_TESTS: 'true',
N8N_USER_FOLDER: mkdtempSync(join(tmpdir(), 'n8n-e2e-')),
N8N_USER_FOLDER: mkdtempSync(testsDir),
EXECUTIONS_PROCESS: 'main',
N8N_DIAGNOSTICS_ENABLED: 'false',
N8N_PUBLIC_API_DISABLED: 'true',
@ -19,8 +21,12 @@ if (inE2ETests) {
NODE_FUNCTION_ALLOW_EXTERNAL: 'node-fetch',
};
} else if (inTest) {
const testsDir = join(tmpdir(), 'n8n-tests/');
mkdirSync(testsDir, { recursive: true });
process.env.N8N_LOG_LEVEL = 'silent';
process.env.N8N_ENCRYPTION_KEY = 'test-encryption-key';
process.env.N8N_PUBLIC_API_DISABLED = 'true';
process.env.N8N_PUBLIC_API_SWAGGERUI_DISABLED = 'true';
process.env.N8N_USER_FOLDER = mkdtempSync(testsDir);
} else {
dotenv.config();
}

View file

@ -27,6 +27,7 @@ import { getSharedWorkflowIds } from '@/WorkflowHelpers';
import { isSharingEnabled, whereClause } from '@/UserManagement/UserManagementHelper';
import type { WorkflowForList } from '@/workflows/workflows.types';
import { InternalHooks } from '@/InternalHooks';
import type { RoleNames } from '../databases/entities/Role';
export type IGetWorkflowsQueryFilter = Pick<
FindOptionsWhere<WorkflowEntity>,
@ -111,7 +112,7 @@ export class WorkflowsService {
}
// Warning: this function is overridden by EE to disregard role list.
static async getWorkflowIdsForUser(user: User, roles?: string[]): Promise<string[]> {
static async getWorkflowIdsForUser(user: User, roles?: RoleNames[]): Promise<string[]> {
return getSharedWorkflowIds(user, roles);
}

View file

@ -8,11 +8,12 @@ import type { User } from '@db/entities/User';
import * as utils from './shared/utils';
import * as testDb from './shared/testDb';
import { createWorkflow } from './shared/testDb';
import { createWorkflow, getGlobalMemberRole, getGlobalOwnerRole } from './shared/testDb';
import type { SaveCredentialFunction } from './shared/types';
import { makeWorkflow } from './shared/utils';
import { randomCredentialPayload } from './shared/random';
import { License } from '@/License';
import { getSharedWorkflowIds } from '../../src/WorkflowHelpers';
let owner: User;
let member: User;
@ -850,3 +851,28 @@ describe('PATCH /workflows/:id - validate interim updates', () => {
expect(updateAttemptResponse.body.code).toBe(100);
});
});
describe('getSharedWorkflowIds', () => {
it('should show all workflows to owners', async () => {
owner.globalRole = await getGlobalOwnerRole();
const workflow1 = await createWorkflow({}, member);
const workflow2 = await createWorkflow({}, anotherMember);
const sharedWorkflowIds = await getSharedWorkflowIds(owner);
expect(sharedWorkflowIds).toHaveLength(2);
expect(sharedWorkflowIds).toContain(workflow1.id);
expect(sharedWorkflowIds).toContain(workflow2.id);
});
it('should show shared workflows to users', async () => {
member.globalRole = await getGlobalMemberRole();
const workflow1 = await createWorkflow({}, anotherMember);
const workflow2 = await createWorkflow({}, anotherMember);
const workflow3 = await createWorkflow({}, anotherMember);
await testDb.shareWorkflowWithUsers(workflow1, [member]);
await testDb.shareWorkflowWithUsers(workflow3, [member]);
const sharedWorkflowIds = await getSharedWorkflowIds(member);
expect(sharedWorkflowIds).toHaveLength(2);
expect(sharedWorkflowIds).toContain(workflow1.id);
expect(sharedWorkflowIds).toContain(workflow3.id);
});
});