perf(core): Cache roles (#6803)

* refactor: Create `RoleService`

* refactor: Refactor to use service

* refactor: Move `getUserRoleForWorkflow`

* refactor: Clear out old `RoleService`

* refactor: Consolidate utils into service

* refactor: Remove unused methods

* test: Add tests

* refactor: Remove redundant return types

* refactor: Missing utility

* chore: Remove commented out bit

* refactor: Make `Db.collections.Repository` inaccessible

* chore: Cleanup

* feat: Prepopulate cache

* chore: Remove logging

* fix: Account for tests where roles are undefined

* fix: Restore `prettier.prettierPath`

* test: Account for cache enabled and disabled

* fix: Restore `Role` in `Db.collections`

* refactor: Simplify by removing `orFail`

* refactor: Rename for clarity

* refactor: Use `cacheKey` for readability

* refactor: Validate role before creation

* refacator: Remove redundant `cache` prefix

* ci: Lint fix

* test: Fix e2e

packages/cli/test/integration/shared/random.ts

@@ -16,7 +16,7 @@ export function randomApiKey() {
 	return `n8n_api_${randomBytes(20).toString('hex')}`;
 }
 
-const chooseRandomly = <T>(array: T[]) => array[Math.floor(Math.random() * array.length)];
+export const chooseRandomly = <T>(array: T[]) => array[Math.floor(Math.random() * array.length)];
 
 export const randomInteger = (max = 1000) => Math.floor(Math.random() * max);
 

packages/cli/test/integration/shared/testDb.ts

@@ -20,7 +20,6 @@ import type { Role } from '@db/entities/Role';
 import type { TagEntity } from '@db/entities/TagEntity';
 import type { User } from '@db/entities/User';
 import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
-import { RoleRepository } from '@db/repositories';
 import type { ICredentialsDb } from '@/Interfaces';
 
 import { DB_INITIALIZATION_TIMEOUT } from './constants';
@@ -34,6 +33,7 @@ import type {
 } from './types';
 import type { ExecutionData } from '@db/entities/ExecutionData';
 import { generateNanoId } from '@db/utils/generators';
+import { RoleService } from '@/services/role.service';
 import { VariablesService } from '@/environments/variables/variables.service';
 
 export type TestDBType = 'postgres' | 'mysql';
@@ -151,7 +151,7 @@ export async function saveCredential(
 }
 
 export async function shareCredentialWithUsers(credential: CredentialsEntity, users: User[]) {
-	const role = await Container.get(RoleRepository).findCredentialUserRole();
+	const role = await Container.get(RoleService).findCredentialUserRole();
 	const newSharedCredentials = users.map((user) =>
 		Db.collections.SharedCredentials.create({
 			userId: user.id,
@@ -276,23 +276,23 @@ export async function addApiKey(user: User): Promise<User> {
 // ----------------------------------
 
 export async function getGlobalOwnerRole() {
-	return Container.get(RoleRepository).findGlobalOwnerRoleOrFail();
+	return Container.get(RoleService).findGlobalOwnerRole();
 }
 
 export async function getGlobalMemberRole() {
-	return Container.get(RoleRepository).findGlobalMemberRoleOrFail();
+	return Container.get(RoleService).findGlobalMemberRole();
 }
 
 export async function getWorkflowOwnerRole() {
-	return Container.get(RoleRepository).findWorkflowOwnerRoleOrFail();
+	return Container.get(RoleService).findWorkflowOwnerRole();
 }
 
 export async function getWorkflowEditorRole() {
-	return Container.get(RoleRepository).findWorkflowEditorRoleOrFail();
+	return Container.get(RoleService).findWorkflowEditorRole();
 }
 
 export async function getCredentialOwnerRole() {
-	return Container.get(RoleRepository).findCredentialOwnerRoleOrFail();
+	return Container.get(RoleService).findCredentialOwnerRole();
 }
 
 export async function getAllRoles() {

packages/cli/test/integration/shared/utils/testServer.ts

@@ -50,6 +50,7 @@ import { AUTHLESS_ENDPOINTS, PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } f
 import type { EndpointGroup, SetupProps, TestServer } from '../types';
 import { mockInstance } from './mocking';
 import { JwtService } from '@/services/jwt.service';
+import { RoleService } from '@/services/role.service';
 
 /**
  * Plugin to prefix a path segment into a request URL pathname.
@@ -264,6 +265,7 @@ export const setupTestServer = ({
 								activeWorkflowRunner: Container.get(ActiveWorkflowRunner),
 								logger,
 								jwtService,
+								roleService: Container.get(RoleService),
 							}),
 						);
 						break;

packages/cli/test/unit/repositories/role.repository.test.ts

@@ -29,20 +29,6 @@ describe('RoleRepository', () => {
 		});
 	});
 
-	describe('findRoleOrFail', () => {
-		test('should return the role when present', async () => {
-			entityManager.findOneOrFail.mockResolvedValueOnce(createRole('global', 'owner'));
-			const role = await roleRepository.findRoleOrFail('global', 'owner');
-			expect(role?.name).toEqual('owner');
-			expect(role?.scope).toEqual('global');
-		});
-
-		test('should throw otherwise', async () => {
-			entityManager.findOneOrFail.mockRejectedValueOnce(new Error());
-			await expect(async () => roleRepository.findRoleOrFail('global', 'owner')).rejects.toThrow();
-		});
-	});
-
 	const createRole = (scope: RoleScopes, name: RoleNames) =>
 		Object.assign(new Role(), { name, scope, id: `${randomInteger()}` });
 });

packages/cli/test/unit/services/ownership.service.test.ts

@@ -1,11 +1,12 @@
 import { OwnershipService } from '@/services/ownership.service';
-import { RoleRepository, SharedWorkflowRepository, UserRepository } from '@/databases/repositories';
+import { SharedWorkflowRepository, UserRepository } from '@/databases/repositories';
 import { mockInstance } from '../../integration/shared/utils';
 import { Role } from '@/databases/entities/Role';
 import { randomInteger } from '../../integration/shared/random';
 import { SharedWorkflow } from '@/databases/entities/SharedWorkflow';
 import { CacheService } from '@/services/cache.service';
 import { User } from '@/databases/entities/User';
+import { RoleService } from '@/services/role.service';
 
 const wfOwnerRole = () =>
 	Object.assign(new Role(), {
@@ -16,14 +17,14 @@ const wfOwnerRole = () =>
 
 describe('OwnershipService', () => {
 	const cacheService = mockInstance(CacheService);
-	const roleRepository = mockInstance(RoleRepository);
+	const roleService = mockInstance(RoleService);
 	const userRepository = mockInstance(UserRepository);
 	const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository);
 
 	const ownershipService = new OwnershipService(
 		cacheService,
 		userRepository,
-		roleRepository,
+		roleService,
 		sharedWorkflowRepository,
 	);
 
@@ -33,7 +34,7 @@ describe('OwnershipService', () => {
 
 	describe('getWorkflowOwner()', () => {
 		test('should retrieve a workflow owner', async () => {
-			roleRepository.findWorkflowOwnerRole.mockResolvedValueOnce(wfOwnerRole());
+			roleService.findWorkflowOwnerRole.mockResolvedValueOnce(wfOwnerRole());
 
 			const mockOwner = new User();
 			const mockNonOwner = new User();
@@ -52,13 +53,13 @@ describe('OwnershipService', () => {
 		});
 
 		test('should throw if no workflow owner role found', async () => {
-			roleRepository.findWorkflowOwnerRole.mockRejectedValueOnce(new Error());
+			roleService.findWorkflowOwnerRole.mockRejectedValueOnce(new Error());
 
 			await expect(ownershipService.getWorkflowOwnerCached('some-workflow-id')).rejects.toThrow();
 		});
 
 		test('should throw if no workflow owner found', async () => {
-			roleRepository.findWorkflowOwnerRole.mockResolvedValueOnce(wfOwnerRole());
+			roleService.findWorkflowOwnerRole.mockResolvedValueOnce(wfOwnerRole());
 
 			sharedWorkflowRepository.findOneOrFail.mockRejectedValue(new Error());
 

packages/cli/test/unit/services/role.service.test.ts

@@ -1,28 +1,80 @@
 import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
+import type { RoleNames, RoleScopes } from '@db/entities/Role';
 import { Role } from '@db/entities/Role';
-import { SharedWorkflow } from '@db/entities/SharedWorkflow';
-import { RoleService } from '@/role/role.service';
 import { mockInstance } from '../../integration/shared/utils/';
+import { RoleService } from '@/services/role.service';
+import { RoleRepository } from '@/databases/repositories';
+import { CacheService } from '@/services/cache.service';
+import { SharedWorkflow } from '@/databases/entities/SharedWorkflow';
+import { chooseRandomly } from '../../integration/shared/random';
+import config from '@/config';
+
+const ROLE_PROPS: Array<{ name: RoleNames; scope: RoleScopes }> = [
+	{ name: 'owner', scope: 'global' },
+	{ name: 'member', scope: 'global' },
+	{ name: 'owner', scope: 'workflow' },
+	{ name: 'owner', scope: 'credential' },
+	{ name: 'user', scope: 'credential' },
+	{ name: 'editor', scope: 'workflow' },
+];
+
+export const uppercaseInitial = (str: string) => str[0].toUpperCase() + str.slice(1);
 
 describe('RoleService', () => {
 	const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository);
-	const roleService = new RoleService(sharedWorkflowRepository);
+	const roleRepository = mockInstance(RoleRepository);
+	const cacheService = mockInstance(CacheService);
+	const roleService = new RoleService(roleRepository, sharedWorkflowRepository, cacheService);
 
 	const userId = '1';
 	const workflowId = '42';
 
-	describe('getUserRoleForWorkflow', () => {
-		test('should return the role if a shared workflow is found', async () => {
-			const sharedWorkflow = Object.assign(new SharedWorkflow(), { role: new Role() });
-			sharedWorkflowRepository.findOne.mockResolvedValueOnce(sharedWorkflow);
-			const role = await roleService.getUserRoleForWorkflow(userId, workflowId);
-			expect(role).toBe(sharedWorkflow.role);
+	const { name, scope } = chooseRandomly(ROLE_PROPS);
+
+	const display = {
+		name: uppercaseInitial(name),
+		scope: uppercaseInitial(scope),
+	};
+
+	beforeEach(() => {
+		config.load(config.default);
+		jest.clearAllMocks();
+	});
+
+	[true, false].forEach((cacheEnabled) => {
+		const tag = ['cache', cacheEnabled ? 'enabled' : 'disabled'].join(' ');
+
+		describe(`find${display.scope}${display.name}Role() [${tag}]`, () => {
+			test(`should return the ${scope} ${name} role if found`, async () => {
+				config.set('cache.enabled', cacheEnabled);
+
+				const role = roleRepository.create({ name, scope });
+				roleRepository.findRole.mockResolvedValueOnce(role);
+				const returnedRole = await roleRepository.findRole(scope, name);
+
+				expect(returnedRole).toBe(role);
+			});
 		});
 
-		test('should return undefined if no shared workflow is found', async () => {
-			sharedWorkflowRepository.findOne.mockResolvedValueOnce(null);
-			const role = await roleService.getUserRoleForWorkflow(userId, workflowId);
-			expect(role).toBeUndefined();
+		describe(`findRoleByUserAndWorkflow() [${tag}]`, () => {
+			test('should return the role if a shared workflow is found', async () => {
+				config.set('cache.enabled', cacheEnabled);
+
+				const sharedWorkflow = Object.assign(new SharedWorkflow(), { role: new Role() });
+				sharedWorkflowRepository.findOne.mockResolvedValueOnce(sharedWorkflow);
+				const returnedRole = await roleService.findRoleByUserAndWorkflow(userId, workflowId);
+
+				expect(returnedRole).toBe(sharedWorkflow.role);
+			});
+
+			test('should return undefined if no shared workflow is found', async () => {
+				config.set('cache.enabled', cacheEnabled);
+
+				sharedWorkflowRepository.findOne.mockResolvedValueOnce(null);
+				const returnedRole = await roleService.findRoleByUserAndWorkflow(userId, workflowId);
+
+				expect(returnedRole).toBeUndefined();
+			});
 		});
 	});
 });