refactor(core): Refactor nodes loading (no-changelog) (#7283)

fixes PAY-605
This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™
2023-10-09 16:09:23 +02:00
committed by GitHub
parent 789e1e7ed4
commit c5ee06cc61
31 changed files with 603 additions and 683 deletions

View File

@@ -8,7 +8,7 @@ import { toReportTitle } from '@/audit/utils';
import { mockInstance } from '../shared/utils/';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { NodeTypes } from '@/NodeTypes';
import { CommunityPackageService } from '@/services/communityPackage.service';
import { CommunityPackagesService } from '@/services/communityPackages.service';
import Container from 'typedi';
import { LoggerProxy } from 'n8n-workflow';
@@ -19,8 +19,8 @@ LoggerProxy.init(getLogger());
const nodesAndCredentials = mockInstance(LoadNodesAndCredentials);
nodesAndCredentials.getCustomDirectories.mockReturnValue([]);
mockInstance(NodeTypes);
const communityPackageService = mockInstance(CommunityPackageService);
Container.set(CommunityPackageService, communityPackageService);
const communityPackagesService = mockInstance(CommunityPackagesService);
Container.set(CommunityPackagesService, communityPackagesService);
beforeAll(async () => {
await testDb.init();
@@ -36,7 +36,7 @@ afterAll(async () => {
});
test('should report risky official nodes', async () => {
communityPackageService.getAllInstalledPackages.mockResolvedValue(MOCK_PACKAGE);
communityPackagesService.getAllInstalledPackages.mockResolvedValue(MOCK_PACKAGE);
const map = [...OFFICIAL_RISKY_NODE_TYPES].reduce<{ [nodeType: string]: string }>((acc, cur) => {
return (acc[cur] = uuid()), acc;
}, {});
@@ -81,7 +81,7 @@ test('should report risky official nodes', async () => {
});
test('should not report non-risky official nodes', async () => {
communityPackageService.getAllInstalledPackages.mockResolvedValue(MOCK_PACKAGE);
communityPackagesService.getAllInstalledPackages.mockResolvedValue(MOCK_PACKAGE);
await saveManualTriggerWorkflow();
const testAudit = await audit(['nodes']);
@@ -96,7 +96,7 @@ test('should not report non-risky official nodes', async () => {
});
test('should report community nodes', async () => {
communityPackageService.getAllInstalledPackages.mockResolvedValue(MOCK_PACKAGE);
communityPackagesService.getAllInstalledPackages.mockResolvedValue(MOCK_PACKAGE);
const testAudit = await audit(['nodes']);

View File

@@ -1,16 +1,18 @@
import * as testDb from '../shared/testDb';
import { mockInstance } from '../shared/utils/';
import * as Config from '@oclif/config';
import { mock } from 'jest-mock-extended';
import { type ILogger, LoggerProxy } from 'n8n-workflow';
import { InternalHooks } from '@/InternalHooks';
import { ImportWorkflowsCommand } from '@/commands/import/workflow';
import * as Config from '@oclif/config';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import * as testDb from '../shared/testDb';
import { mockInstance } from '../shared/utils/';
import { LoggerProxy } from 'n8n-workflow';
import { getLogger } from '@/Logger';
LoggerProxy.init(getLogger());
LoggerProxy.init(mock<ILogger>());
beforeAll(async () => {
mockInstance(InternalHooks);
mockInstance(LoadNodesAndCredentials);
await testDb.init();
});

View File

@@ -1,8 +1,11 @@
import path from 'path';
import type { SuperAgentTest } from 'supertest';
import type { InstalledPackages } from '@db/entities/InstalledPackages';
import type { InstalledNodes } from '@db/entities/InstalledNodes';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { Push } from '@/push';
import { CommunityPackageService } from '@/services/communityPackage.service';
import { CommunityPackagesService } from '@/services/communityPackages.service';
import { COMMUNITY_PACKAGE_VERSION } from './shared/constants';
import * as testDb from './shared/testDb';
@@ -14,15 +17,13 @@ import {
mockPackageName,
} from './shared/utils';
import type { InstalledPackages } from '@db/entities/InstalledPackages';
import type { InstalledNodes } from '@db/entities/InstalledNodes';
import type { SuperAgentTest } from 'supertest';
const communityPackageService = mockInstance(CommunityPackageService);
const mockLoadNodesAndCredentials = mockInstance(LoadNodesAndCredentials);
const communityPackagesService = mockInstance(CommunityPackagesService, {
hasMissingPackages: false,
});
mockInstance(LoadNodesAndCredentials);
mockInstance(Push);
const testServer = setupTestServer({ endpointGroups: ['nodes'] });
const testServer = setupTestServer({ endpointGroups: ['community-packages'] });
const commonUpdatesProps = {
createdAt: new Date(),
@@ -47,12 +48,12 @@ beforeEach(() => {
jest.resetAllMocks();
});
describe('GET /nodes', () => {
describe('GET /community-packages', () => {
test('should respond 200 if no nodes are installed', async () => {
communityPackageService.getAllInstalledPackages.mockResolvedValue([]);
communityPackagesService.getAllInstalledPackages.mockResolvedValue([]);
const {
body: { data },
} = await authAgent.get('/nodes').expect(200);
} = await authAgent.get('/community-packages').expect(200);
expect(data).toHaveLength(0);
});
@@ -61,12 +62,12 @@ describe('GET /nodes', () => {
const pkg = mockPackage();
const node = mockNode(pkg.packageName);
pkg.installedNodes = [node];
communityPackageService.getAllInstalledPackages.mockResolvedValue([pkg]);
communityPackageService.matchPackagesWithUpdates.mockReturnValue([pkg]);
communityPackagesService.getAllInstalledPackages.mockResolvedValue([pkg]);
communityPackagesService.matchPackagesWithUpdates.mockReturnValue([pkg]);
const {
body: { data },
} = await authAgent.get('/nodes').expect(200);
} = await authAgent.get('/community-packages').expect(200);
expect(data).toHaveLength(1);
expect(data[0].installedNodes).toHaveLength(1);
@@ -80,9 +81,9 @@ describe('GET /nodes', () => {
const nodeB = mockNode(pkgB.packageName);
const nodeC = mockNode(pkgB.packageName);
communityPackageService.getAllInstalledPackages.mockResolvedValue([pkgA, pkgB]);
communityPackagesService.getAllInstalledPackages.mockResolvedValue([pkgA, pkgB]);
communityPackageService.matchPackagesWithUpdates.mockReturnValue([
communityPackagesService.matchPackagesWithUpdates.mockReturnValue([
{
...commonUpdatesProps,
packageName: pkgA.packageName,
@@ -97,7 +98,7 @@ describe('GET /nodes', () => {
const {
body: { data },
} = await authAgent.get('/nodes').expect(200);
} = await authAgent.get('/community-packages').expect(200);
expect(data).toHaveLength(2);
@@ -110,26 +111,26 @@ describe('GET /nodes', () => {
});
test('should not check for updates if no packages installed', async () => {
await authAgent.get('/nodes');
await authAgent.get('/community-packages');
expect(communityPackageService.executeNpmCommand).not.toHaveBeenCalled();
expect(communityPackagesService.executeNpmCommand).not.toHaveBeenCalled();
});
test('should check for updates if packages installed', async () => {
communityPackageService.getAllInstalledPackages.mockResolvedValue([mockPackage()]);
communityPackagesService.getAllInstalledPackages.mockResolvedValue([mockPackage()]);
await authAgent.get('/nodes').expect(200);
await authAgent.get('/community-packages').expect(200);
const args = ['npm outdated --json', { doNotHandleError: true }];
expect(communityPackageService.executeNpmCommand).toHaveBeenCalledWith(...args);
expect(communityPackagesService.executeNpmCommand).toHaveBeenCalledWith(...args);
});
test('should report package updates if available', async () => {
const pkg = mockPackage();
communityPackageService.getAllInstalledPackages.mockResolvedValue([pkg]);
communityPackagesService.getAllInstalledPackages.mockResolvedValue([pkg]);
communityPackageService.executeNpmCommand.mockImplementation(() => {
communityPackagesService.executeNpmCommand.mockImplementation(() => {
throw {
code: 1,
stdout: JSON.stringify({
@@ -143,7 +144,7 @@ describe('GET /nodes', () => {
};
});
communityPackageService.matchPackagesWithUpdates.mockReturnValue([
communityPackagesService.matchPackagesWithUpdates.mockReturnValue([
{
packageName: 'test',
installedNodes: [],
@@ -153,7 +154,7 @@ describe('GET /nodes', () => {
const {
body: { data },
} = await authAgent.get('/nodes').expect(200);
} = await authAgent.get('/community-packages').expect(200);
const [returnedPkg] = data;
@@ -162,89 +163,92 @@ describe('GET /nodes', () => {
});
});
describe('POST /nodes', () => {
describe('POST /community-packages', () => {
test('should reject if package name is missing', async () => {
await authAgent.post('/nodes').expect(400);
await authAgent.post('/community-packages').expect(400);
});
test('should reject if package is duplicate', async () => {
communityPackageService.findInstalledPackage.mockResolvedValue(mockPackage());
communityPackageService.isPackageInstalled.mockResolvedValue(true);
communityPackageService.hasPackageLoaded.mockReturnValue(true);
communityPackageService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
communityPackagesService.findInstalledPackage.mockResolvedValue(mockPackage());
communityPackagesService.isPackageInstalled.mockResolvedValue(true);
communityPackagesService.hasPackageLoaded.mockReturnValue(true);
communityPackagesService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
const {
body: { message },
} = await authAgent.post('/nodes').send({ name: mockPackageName() }).expect(400);
} = await authAgent.post('/community-packages').send({ name: mockPackageName() }).expect(400);
expect(message).toContain('already installed');
});
test('should allow installing packages that could not be loaded', async () => {
communityPackageService.findInstalledPackage.mockResolvedValue(mockPackage());
communityPackageService.hasPackageLoaded.mockReturnValue(false);
communityPackageService.checkNpmPackageStatus.mockResolvedValue({ status: 'OK' });
communityPackageService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
mockLoadNodesAndCredentials.installNpmModule.mockResolvedValue(mockPackage());
communityPackagesService.findInstalledPackage.mockResolvedValue(mockPackage());
communityPackagesService.hasPackageLoaded.mockReturnValue(false);
communityPackagesService.checkNpmPackageStatus.mockResolvedValue({ status: 'OK' });
communityPackagesService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
communityPackagesService.installNpmModule.mockResolvedValue(mockPackage());
await authAgent.post('/nodes').send({ name: mockPackageName() }).expect(200);
await authAgent.post('/community-packages').send({ name: mockPackageName() }).expect(200);
expect(communityPackageService.removePackageFromMissingList).toHaveBeenCalled();
expect(communityPackagesService.removePackageFromMissingList).toHaveBeenCalled();
});
test('should not install a banned package', async () => {
communityPackageService.checkNpmPackageStatus.mockResolvedValue({ status: 'Banned' });
communityPackageService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
communityPackagesService.checkNpmPackageStatus.mockResolvedValue({ status: 'Banned' });
communityPackagesService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
const {
body: { message },
} = await authAgent.post('/nodes').send({ name: mockPackageName() }).expect(400);
} = await authAgent.post('/community-packages').send({ name: mockPackageName() }).expect(400);
expect(message).toContain('banned');
});
});
describe('DELETE /nodes', () => {
describe('DELETE /community-packages', () => {
test('should not delete if package name is empty', async () => {
await authAgent.delete('/nodes').expect(400);
await authAgent.delete('/community-packages').expect(400);
});
test('should reject if package is not installed', async () => {
const {
body: { message },
} = await authAgent.delete('/nodes').query({ name: mockPackageName() }).expect(400);
} = await authAgent
.delete('/community-packages')
.query({ name: mockPackageName() })
.expect(400);
expect(message).toContain('not installed');
});
test('should uninstall package', async () => {
communityPackageService.findInstalledPackage.mockResolvedValue(mockPackage());
communityPackagesService.findInstalledPackage.mockResolvedValue(mockPackage());
await authAgent.delete('/nodes').query({ name: mockPackageName() }).expect(200);
await authAgent.delete('/community-packages').query({ name: mockPackageName() }).expect(200);
expect(mockLoadNodesAndCredentials.removeNpmModule).toHaveBeenCalledTimes(1);
expect(communityPackagesService.removeNpmModule).toHaveBeenCalledTimes(1);
});
});
describe('PATCH /nodes', () => {
describe('PATCH /community-packages', () => {
test('should reject if package name is empty', async () => {
await authAgent.patch('/nodes').expect(400);
await authAgent.patch('/community-packages').expect(400);
});
test('should reject if package is not installed', async () => {
const {
body: { message },
} = await authAgent.patch('/nodes').send({ name: mockPackageName() }).expect(400);
} = await authAgent.patch('/community-packages').send({ name: mockPackageName() }).expect(400);
expect(message).toContain('not installed');
});
test('should update a package', async () => {
communityPackageService.findInstalledPackage.mockResolvedValue(mockPackage());
communityPackageService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
communityPackagesService.findInstalledPackage.mockResolvedValue(mockPackage());
communityPackagesService.parseNpmPackageName.mockReturnValue(parsedNpmPackageName);
await authAgent.patch('/nodes').send({ name: mockPackageName() });
await authAgent.patch('/community-packages').send({ name: mockPackageName() });
expect(mockLoadNodesAndCredentials.updateNpmModule).toHaveBeenCalledTimes(1);
expect(communityPackagesService.updateNpmModule).toHaveBeenCalledTimes(1);
});
});

View File

@@ -22,7 +22,7 @@ export type EndpointGroup =
| 'credentials'
| 'workflows'
| 'publicApi'
| 'nodes'
| 'community-packages'
| 'ldap'
| 'saml'
| 'sourceControl'

View File

@@ -25,7 +25,6 @@ import {
LdapController,
MFAController,
MeController,
NodesController,
OwnerController,
PasswordResetController,
TagsController,
@@ -34,12 +33,10 @@ import {
import { rawBodyReader, bodyParser, setupAuthMiddlewares } from '@/middlewares';
import { InternalHooks } from '@/InternalHooks';
import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials';
import { PostHogClient } from '@/posthog';
import { variablesController } from '@/environments/variables/variables.controller';
import { LdapManager } from '@/Ldap/LdapManager.ee';
import { handleLdapInit } from '@/Ldap/helpers';
import { Push } from '@/push';
import { setSamlLoginEnabled } from '@/sso/saml/samlHelpers';
import { SamlController } from '@/sso/saml/routes/saml.controller.ee';
import { EventBusController } from '@/eventbus/eventBus.controller';
@@ -242,17 +239,11 @@ export const setupTestServer = ({
case 'sourceControl':
registerController(app, config, Container.get(SourceControlController));
break;
case 'nodes':
registerController(
app,
config,
new NodesController(
config,
Container.get(LoadNodesAndCredentials),
Container.get(Push),
internalHooks,
),
case 'community-packages':
const { CommunityPackagesController } = await import(
'@/controllers/communityPackages.controller'
);
registerController(app, config, Container.get(CommunityPackagesController));
case 'me':
registerController(
app,