refactor(core): Use Dependency Injection for all Controller classes (no-changelog) (#8146)
## Review / Merge checklist - [x] PR title and summary are descriptive
This commit is contained in:
committed by
GitHub
parent
518a99e528
commit
f69ddcd796
@@ -1,22 +1,18 @@
|
||||
import { Container } from 'typedi';
|
||||
import type { SuperAgentTest } from 'supertest';
|
||||
import { SOURCE_CONTROL_API_ROOT } from '@/environments/sourceControl/constants';
|
||||
import * as utils from '../shared/utils/';
|
||||
|
||||
import type { User } from '@db/entities/User';
|
||||
import * as UserManagementHelpers from '@/UserManagement/UserManagementHelper';
|
||||
import Container from 'typedi';
|
||||
import config from '@/config';
|
||||
import { SourceControlPreferencesService } from '@/environments/sourceControl/sourceControlPreferences.service.ee';
|
||||
import { SourceControlService } from '@/environments/sourceControl/sourceControl.service.ee';
|
||||
import type { SourceControlledFile } from '@/environments/sourceControl/types/sourceControlledFile';
|
||||
import { getGlobalMemberRole, getGlobalOwnerRole } from '../shared/db/roles';
|
||||
|
||||
import * as utils from '../shared/utils/';
|
||||
import { getGlobalOwnerRole } from '../shared/db/roles';
|
||||
import { createUser } from '../shared/db/users';
|
||||
|
||||
let authOwnerAgent: SuperAgentTest;
|
||||
let authMemberAgent: SuperAgentTest;
|
||||
let owner: User;
|
||||
let member: User;
|
||||
|
||||
const sharingSpy = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true);
|
||||
|
||||
const testServer = utils.setupTestServer({
|
||||
endpointGroups: ['sourceControl', 'license', 'auth'],
|
||||
@@ -25,11 +21,8 @@ const testServer = utils.setupTestServer({
|
||||
|
||||
beforeAll(async () => {
|
||||
const globalOwnerRole = await getGlobalOwnerRole();
|
||||
const globalMemberRole = await getGlobalMemberRole();
|
||||
owner = await createUser({ globalRole: globalOwnerRole });
|
||||
member = await createUser({ globalRole: globalMemberRole });
|
||||
authOwnerAgent = testServer.authAgentFor(owner);
|
||||
authMemberAgent = testServer.authAgentFor(member);
|
||||
|
||||
Container.get(SourceControlPreferencesService).isSourceControlConnected = () => true;
|
||||
});
|
||||
@@ -37,7 +30,7 @@ beforeAll(async () => {
|
||||
describe('GET /sourceControl/preferences', () => {
|
||||
test('should return Source Control preferences', async () => {
|
||||
await authOwnerAgent
|
||||
.get(`/${SOURCE_CONTROL_API_ROOT}/preferences`)
|
||||
.get('/source-control/preferences')
|
||||
.expect(200)
|
||||
.expect((res) => {
|
||||
return 'repositoryUrl' in res.body && 'branchName' in res.body;
|
||||
@@ -60,7 +53,7 @@ describe('GET /sourceControl/preferences', () => {
|
||||
] as SourceControlledFile[];
|
||||
};
|
||||
await authOwnerAgent
|
||||
.get(`/${SOURCE_CONTROL_API_ROOT}/get-status`)
|
||||
.get('/source-control/get-status')
|
||||
.query({ direction: 'push', preferLocalVersion: 'true', verbose: 'false' })
|
||||
.expect(200)
|
||||
.expect((res) => {
|
||||
@@ -73,7 +66,7 @@ describe('GET /sourceControl/preferences', () => {
|
||||
test('refreshing key pairsshould return new rsa key', async () => {
|
||||
config.set('sourceControl.defaultKeyPairType', 'rsa');
|
||||
await authOwnerAgent
|
||||
.post(`/${SOURCE_CONTROL_API_ROOT}/generate-key-pair`)
|
||||
.post('/source-control/generate-key-pair')
|
||||
.send()
|
||||
.expect(200)
|
||||
.expect((res) => {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { DataSourceOptions as ConnectionOptions, Repository } from 'typeorm';
|
||||
import { DataSource as Connection } from 'typeorm';
|
||||
import { Container } from 'typedi';
|
||||
import type { Class } from 'n8n-core';
|
||||
|
||||
import config from '@/config';
|
||||
import * as Db from '@/Db';
|
||||
@@ -116,7 +117,7 @@ const repositories = [
|
||||
*/
|
||||
export async function truncate(names: Array<(typeof repositories)[number]>) {
|
||||
for (const name of names) {
|
||||
const RepositoryClass: { new (): Repository<any> } = (
|
||||
const RepositoryClass: Class<Repository<object>> = (
|
||||
await import(`@db/repositories/${name.charAt(0).toLowerCase() + name.slice(1)}.repository`)
|
||||
)[`${name}Repository`];
|
||||
await Container.get(RepositoryClass).delete({});
|
||||
|
||||
@@ -14,14 +14,13 @@ import { rawBodyReader, bodyParser, setupAuthMiddlewares } from '@/middlewares';
|
||||
import { PostHogClient } from '@/posthog';
|
||||
import { License } from '@/License';
|
||||
import { Logger } from '@/Logger';
|
||||
import { InternalHooks } from '@/InternalHooks';
|
||||
|
||||
import { mockInstance } from '../../../shared/mocking';
|
||||
import * as testDb from '../../shared/testDb';
|
||||
import { AUTHLESS_ENDPOINTS, PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants';
|
||||
import type { SetupProps, TestServer } from '../types';
|
||||
import { InternalHooks } from '@/InternalHooks';
|
||||
import { LicenseMocker } from '../license';
|
||||
import { PasswordUtility } from '@/services/password.utility';
|
||||
|
||||
/**
|
||||
* Plugin to prefix a path segment into a request URL pathname.
|
||||
@@ -76,7 +75,7 @@ export const setupTestServer = ({
|
||||
app.use(cookieParser());
|
||||
|
||||
// Mock all telemetry and logging
|
||||
const logger = mockInstance(Logger);
|
||||
mockInstance(Logger);
|
||||
mockInstance(InternalHooks);
|
||||
mockInstance(PostHogClient);
|
||||
|
||||
@@ -140,12 +139,12 @@ export const setupTestServer = ({
|
||||
const { VariablesController } = await import(
|
||||
'@/environments/variables/variables.controller.ee'
|
||||
);
|
||||
registerController(app, config, Container.get(VariablesController));
|
||||
registerController(app, VariablesController);
|
||||
break;
|
||||
|
||||
case 'license':
|
||||
const { LicenseController } = await import('@/license/license.controller');
|
||||
registerController(app, config, Container.get(LicenseController));
|
||||
registerController(app, LicenseController);
|
||||
break;
|
||||
|
||||
case 'metrics':
|
||||
@@ -156,166 +155,108 @@ export const setupTestServer = ({
|
||||
case 'eventBus':
|
||||
const { EventBusController } = await import('@/eventbus/eventBus.controller');
|
||||
const { EventBusControllerEE } = await import('@/eventbus/eventBus.controller.ee');
|
||||
registerController(app, config, new EventBusController());
|
||||
registerController(app, config, new EventBusControllerEE());
|
||||
registerController(app, EventBusController);
|
||||
registerController(app, EventBusControllerEE);
|
||||
break;
|
||||
|
||||
case 'auth':
|
||||
const { AuthController } = await import('@/controllers/auth.controller');
|
||||
registerController(app, config, Container.get(AuthController));
|
||||
registerController(app, AuthController);
|
||||
break;
|
||||
|
||||
case 'mfa':
|
||||
const { MFAController } = await import('@/controllers/mfa.controller');
|
||||
registerController(app, config, Container.get(MFAController));
|
||||
registerController(app, MFAController);
|
||||
break;
|
||||
|
||||
case 'ldap':
|
||||
const { LdapManager } = await import('@/Ldap/LdapManager.ee');
|
||||
const { handleLdapInit } = await import('@/Ldap/helpers');
|
||||
const { LdapController } = await import('@/controllers/ldap.controller');
|
||||
testServer.license.enable('feat:ldap');
|
||||
await handleLdapInit();
|
||||
const { service, sync } = LdapManager.getInstance();
|
||||
registerController(
|
||||
app,
|
||||
config,
|
||||
new LdapController(service, sync, Container.get(InternalHooks)),
|
||||
);
|
||||
registerController(app, LdapController);
|
||||
break;
|
||||
|
||||
case 'saml':
|
||||
const { setSamlLoginEnabled } = await import('@/sso/saml/samlHelpers');
|
||||
const { SamlController } = await import('@/sso/saml/routes/saml.controller.ee');
|
||||
await setSamlLoginEnabled(true);
|
||||
registerController(app, config, Container.get(SamlController));
|
||||
registerController(app, SamlController);
|
||||
break;
|
||||
|
||||
case 'sourceControl':
|
||||
const { SourceControlController } = await import(
|
||||
'@/environments/sourceControl/sourceControl.controller.ee'
|
||||
);
|
||||
registerController(app, config, Container.get(SourceControlController));
|
||||
registerController(app, SourceControlController);
|
||||
break;
|
||||
|
||||
case 'community-packages':
|
||||
const { CommunityPackagesController } = await import(
|
||||
'@/controllers/communityPackages.controller'
|
||||
);
|
||||
registerController(app, config, Container.get(CommunityPackagesController));
|
||||
registerController(app, CommunityPackagesController);
|
||||
break;
|
||||
|
||||
case 'me':
|
||||
const { MeController } = await import('@/controllers/me.controller');
|
||||
registerController(app, config, Container.get(MeController));
|
||||
registerController(app, MeController);
|
||||
break;
|
||||
|
||||
case 'passwordReset':
|
||||
const { PasswordResetController } = await import(
|
||||
'@/controllers/passwordReset.controller'
|
||||
);
|
||||
registerController(app, config, Container.get(PasswordResetController));
|
||||
registerController(app, PasswordResetController);
|
||||
break;
|
||||
|
||||
case 'owner':
|
||||
const { UserService } = await import('@/services/user.service');
|
||||
const { SettingsRepository } = await import('@db/repositories/settings.repository');
|
||||
const { OwnerController } = await import('@/controllers/owner.controller');
|
||||
registerController(
|
||||
app,
|
||||
config,
|
||||
new OwnerController(
|
||||
config,
|
||||
logger,
|
||||
Container.get(InternalHooks),
|
||||
Container.get(SettingsRepository),
|
||||
Container.get(UserService),
|
||||
Container.get(PasswordUtility),
|
||||
),
|
||||
);
|
||||
registerController(app, OwnerController);
|
||||
break;
|
||||
|
||||
case 'users':
|
||||
const { SharedCredentialsRepository } = await import(
|
||||
'@db/repositories/sharedCredentials.repository'
|
||||
);
|
||||
const { SharedWorkflowRepository } = await import(
|
||||
'@db/repositories/sharedWorkflow.repository'
|
||||
);
|
||||
const { ActiveWorkflowRunner } = await import('@/ActiveWorkflowRunner');
|
||||
const { UserService: US } = await import('@/services/user.service');
|
||||
const { ExternalHooks: EH } = await import('@/ExternalHooks');
|
||||
const { RoleService: RS } = await import('@/services/role.service');
|
||||
const { UsersController } = await import('@/controllers/users.controller');
|
||||
registerController(
|
||||
app,
|
||||
config,
|
||||
new UsersController(
|
||||
logger,
|
||||
Container.get(EH),
|
||||
Container.get(InternalHooks),
|
||||
Container.get(SharedCredentialsRepository),
|
||||
Container.get(SharedWorkflowRepository),
|
||||
Container.get(ActiveWorkflowRunner),
|
||||
Container.get(RS),
|
||||
Container.get(US),
|
||||
Container.get(License),
|
||||
),
|
||||
);
|
||||
registerController(app, UsersController);
|
||||
break;
|
||||
|
||||
case 'invitations':
|
||||
const { InvitationController } = await import('@/controllers/invitation.controller');
|
||||
const { ExternalHooks: EHS } = await import('@/ExternalHooks');
|
||||
const { UserService: USE } = await import('@/services/user.service');
|
||||
|
||||
registerController(
|
||||
app,
|
||||
config,
|
||||
new InvitationController(
|
||||
config,
|
||||
logger,
|
||||
Container.get(InternalHooks),
|
||||
Container.get(EHS),
|
||||
Container.get(USE),
|
||||
Container.get(License),
|
||||
Container.get(PasswordUtility),
|
||||
),
|
||||
);
|
||||
registerController(app, InvitationController);
|
||||
break;
|
||||
|
||||
case 'tags':
|
||||
const { TagsController } = await import('@/controllers/tags.controller');
|
||||
registerController(app, config, Container.get(TagsController));
|
||||
registerController(app, TagsController);
|
||||
break;
|
||||
|
||||
case 'externalSecrets':
|
||||
const { ExternalSecretsController } = await import(
|
||||
'@/ExternalSecrets/ExternalSecrets.controller.ee'
|
||||
);
|
||||
registerController(app, config, Container.get(ExternalSecretsController));
|
||||
registerController(app, ExternalSecretsController);
|
||||
break;
|
||||
|
||||
case 'workflowHistory':
|
||||
const { WorkflowHistoryController } = await import(
|
||||
'@/workflows/workflowHistory/workflowHistory.controller.ee'
|
||||
);
|
||||
registerController(app, config, Container.get(WorkflowHistoryController));
|
||||
registerController(app, WorkflowHistoryController);
|
||||
break;
|
||||
|
||||
case 'binaryData':
|
||||
const { BinaryDataController } = await import('@/controllers/binaryData.controller');
|
||||
registerController(app, config, Container.get(BinaryDataController));
|
||||
registerController(app, BinaryDataController);
|
||||
break;
|
||||
|
||||
case 'role':
|
||||
const { RoleController } = await import('@/controllers/role.controller');
|
||||
registerController(app, config, Container.get(RoleController));
|
||||
registerController(app, RoleController);
|
||||
break;
|
||||
|
||||
case 'debug':
|
||||
const { DebugController } = await import('@/controllers/debug.controller');
|
||||
registerController(app, config, Container.get(DebugController));
|
||||
registerController(app, DebugController);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,12 +1,13 @@
|
||||
import { Container } from 'typedi';
|
||||
import { mock } from 'jest-mock-extended';
|
||||
import type { DeepPartial } from 'ts-essentials';
|
||||
import type { Class } from 'n8n-core';
|
||||
|
||||
export const mockInstance = <T>(
|
||||
ctor: new (...args: unknown[]) => T,
|
||||
serviceClass: Class<T>,
|
||||
data: DeepPartial<T> | undefined = undefined,
|
||||
) => {
|
||||
const instance = mock<T>(data);
|
||||
Container.set(ctor, instance);
|
||||
Container.set(serviceClass, instance);
|
||||
return instance;
|
||||
};
|
||||
|
||||
@@ -1,10 +1,9 @@
|
||||
import type { CookieOptions, Response } from 'express';
|
||||
import { anyObject, captor, mock } from 'jest-mock-extended';
|
||||
import jwt from 'jsonwebtoken';
|
||||
import type { IInternalHooksClass } from '@/Interfaces';
|
||||
import type { User } from '@db/entities/User';
|
||||
import type { SettingsRepository } from '@db/repositories/settings.repository';
|
||||
import type { Config } from '@/config';
|
||||
import config from '@/config';
|
||||
import type { OwnerRequest } from '@/requests';
|
||||
import { OwnerController } from '@/controllers/owner.controller';
|
||||
import { AUTH_COOKIE_NAME } from '@/constants';
|
||||
@@ -16,32 +15,33 @@ import { badPasswords } from '../shared/testData';
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
import { PasswordUtility } from '@/services/password.utility';
|
||||
import Container from 'typedi';
|
||||
import type { InternalHooks } from '@/InternalHooks';
|
||||
|
||||
describe('OwnerController', () => {
|
||||
const config = mock<Config>();
|
||||
const internalHooks = mock<IInternalHooksClass>();
|
||||
const configGetSpy = jest.spyOn(config, 'getEnv');
|
||||
const internalHooks = mock<InternalHooks>();
|
||||
const userService = mockInstance(UserService);
|
||||
const settingsRepository = mock<SettingsRepository>();
|
||||
mockInstance(License).isWithinUsersLimit.mockReturnValue(true);
|
||||
const controller = new OwnerController(
|
||||
config,
|
||||
mock(),
|
||||
internalHooks,
|
||||
settingsRepository,
|
||||
userService,
|
||||
Container.get(PasswordUtility),
|
||||
mock(),
|
||||
);
|
||||
|
||||
describe('setupOwner', () => {
|
||||
it('should throw a BadRequestError if the instance owner is already setup', async () => {
|
||||
config.getEnv.calledWith('userManagement.isInstanceOwnerSetUp').mockReturnValue(true);
|
||||
configGetSpy.mockReturnValue(true);
|
||||
await expect(controller.setupOwner(mock(), mock())).rejects.toThrowError(
|
||||
new BadRequestError('Instance owner already setup'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw a BadRequestError if the email is invalid', async () => {
|
||||
config.getEnv.calledWith('userManagement.isInstanceOwnerSetUp').mockReturnValue(false);
|
||||
configGetSpy.mockReturnValue(false);
|
||||
const req = mock<OwnerRequest.Post>({ body: { email: 'invalid email' } });
|
||||
await expect(controller.setupOwner(req, mock())).rejects.toThrowError(
|
||||
new BadRequestError('Invalid email address'),
|
||||
@@ -51,7 +51,7 @@ describe('OwnerController', () => {
|
||||
describe('should throw if the password is invalid', () => {
|
||||
Object.entries(badPasswords).forEach(([password, errorMessage]) => {
|
||||
it(password, async () => {
|
||||
config.getEnv.calledWith('userManagement.isInstanceOwnerSetUp').mockReturnValue(false);
|
||||
configGetSpy.mockReturnValue(false);
|
||||
const req = mock<OwnerRequest.Post>({ body: { email: 'valid@email.com', password } });
|
||||
await expect(controller.setupOwner(req, mock())).rejects.toThrowError(
|
||||
new BadRequestError(errorMessage),
|
||||
@@ -61,7 +61,7 @@ describe('OwnerController', () => {
|
||||
});
|
||||
|
||||
it('should throw a BadRequestError if firstName & lastName are missing ', async () => {
|
||||
config.getEnv.calledWith('userManagement.isInstanceOwnerSetUp').mockReturnValue(false);
|
||||
configGetSpy.mockReturnValue(false);
|
||||
const req = mock<OwnerRequest.Post>({
|
||||
body: { email: 'valid@email.com', password: 'NewPassword123', firstName: '', lastName: '' },
|
||||
});
|
||||
@@ -86,7 +86,7 @@ describe('OwnerController', () => {
|
||||
user,
|
||||
});
|
||||
const res = mock<Response>();
|
||||
config.getEnv.calledWith('userManagement.isInstanceOwnerSetUp').mockReturnValue(false);
|
||||
configGetSpy.mockReturnValue(false);
|
||||
userService.save.calledWith(anyObject()).mockResolvedValue(user);
|
||||
jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token');
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { mock } from 'jest-mock-extended';
|
||||
import type { ICredentialTypes } from 'n8n-workflow';
|
||||
import type { Config } from '@/config';
|
||||
import config from '@/config';
|
||||
import type { TranslationRequest } from '@/controllers/translation.controller';
|
||||
import {
|
||||
TranslationController,
|
||||
@@ -9,9 +9,9 @@ import {
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
|
||||
describe('TranslationController', () => {
|
||||
const config = mock<Config>();
|
||||
const configGetSpy = jest.spyOn(config, 'getEnv');
|
||||
const credentialTypes = mock<ICredentialTypes>();
|
||||
const controller = new TranslationController(config, credentialTypes);
|
||||
const controller = new TranslationController(credentialTypes);
|
||||
|
||||
describe('getCredentialTranslation', () => {
|
||||
it('should throw 400 on invalid credential types', async () => {
|
||||
@@ -27,7 +27,7 @@ describe('TranslationController', () => {
|
||||
it('should return translation json on valid credential types', async () => {
|
||||
const credentialType = 'credential-type';
|
||||
const req = mock<TranslationRequest.Credential>({ query: { credentialType } });
|
||||
config.getEnv.calledWith('defaultLocale').mockReturnValue('de');
|
||||
configGetSpy.mockReturnValue('de');
|
||||
credentialTypes.recognizes.calledWith(credentialType).mockReturnValue(true);
|
||||
const response = { translation: 'string' };
|
||||
jest.mock(`${CREDENTIAL_TRANSLATIONS_DIR}/de/credential-type.json`, () => response, {
|
||||
|
||||
Reference in New Issue
Block a user