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

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <[email protected]>

jest.config.js

@@ -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') {

packages/cli/src/WorkflowHelpers.ts

@@ -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.

packages/cli/src/config/index.ts

@@ -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();
 }

packages/cli/src/workflows/workflows.services.ts

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

packages/cli/test/integration/workflows.controller.ee.test.ts

@@ -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);
+	});
+});