refactor(core): Make Logger a service (no-changelog) (#7494)

2025-12-18 02:21:13 +00:00 · 2023-10-25 16:35:22 +02:00
parent db4e61ba24
commit 05586a900d
131 changed files with 761 additions and 919 deletions
--- a/packages/cli/test/unit/ActiveExecutions.test.ts
+++ b/packages/cli/test/unit/ActiveExecutions.test.ts
@@ -6,6 +6,7 @@ import type { IExecuteResponsePromiseData, IRun } from 'n8n-workflow';
 import { createDeferredPromise } from 'n8n-workflow';
 import type { IWorkflowExecutionDataProcess } from '@/Interfaces';
 import { ExecutionRepository } from '@db/repositories';
+import { mock } from 'jest-mock-extended';

 const FAKE_EXECUTION_ID = '15';
 const FAKE_SECOND_EXECUTION_ID = '20';
@@ -24,7 +25,7 @@ describe('ActiveExecutions', () => {
 	let activeExecutions: ActiveExecutions;

 	beforeEach(() => {
-		activeExecutions = new ActiveExecutions();
+		activeExecutions = new ActiveExecutions(mock());
 	});

 	afterEach(() => {
--- a/packages/cli/test/unit/ActiveWorkflowRunner.test.ts
+++ b/packages/cli/test/unit/ActiveWorkflowRunner.test.ts
@@ -3,7 +3,7 @@ import { mocked } from 'jest-mock';
 import { Container } from 'typedi';

 import type { INode } from 'n8n-workflow';
-import { LoggerProxy, NodeApiError, NodeOperationError, Workflow } from 'n8n-workflow';
+import { NodeApiError, NodeOperationError, Workflow } from 'n8n-workflow';

 import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
 import * as Db from '@/Db';
@@ -11,7 +11,6 @@ import { WorkflowEntity } from '@db/entities/WorkflowEntity';
 import { SharedWorkflow } from '@db/entities/SharedWorkflow';
 import { Role } from '@db/entities/Role';
 import { User } from '@db/entities/User';
-import { getLogger } from '@/Logger';
 import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData';
 import { WorkflowRunner } from '@/WorkflowRunner';
 import { ExternalHooks } from '@/ExternalHooks';
@@ -147,7 +146,6 @@ describe('ActiveWorkflowRunner', () => {
 	const activeWorkflowRunner = Container.get(ActiveWorkflowRunner);

 	beforeAll(async () => {
-		LoggerProxy.init(getLogger());
 		variablesService.getAllCached.mockResolvedValue([]);
 	});

--- a/packages/cli/test/unit/ExternalSecrets/ExternalSecretsManager.test.ts
+++ b/packages/cli/test/unit/ExternalSecrets/ExternalSecretsManager.test.ts
@@ -13,6 +13,7 @@ import {
 	FailedProvider,
 	MockProviders,
 } from '../../shared/ExternalSecrets/utils';
+import { mock } from 'jest-mock-extended';

 describe('External Secrets Manager', () => {
 	const connectedDate = '2023-08-01T12:32:29.000Z';
@@ -48,7 +49,7 @@ describe('External Secrets Manager', () => {
 		});
 		license.isExternalSecretsEnabled.mockReturnValue(true);
 		settingsRepo.getEncryptedSecretsProviderSettings.mockResolvedValue(settings);
-		manager = new ExternalSecretsManager(settingsRepo, license, providersMock, cipher);
+		manager = new ExternalSecretsManager(mock(), settingsRepo, license, providersMock, cipher);
 	});

 	afterEach(() => {
--- a/packages/cli/test/unit/InternalHooks.test.ts
+++ b/packages/cli/test/unit/InternalHooks.test.ts
@@ -1,11 +1,8 @@
 import { Telemetry } from '@/telemetry';
-import { RoleService } from '@/services/role.service';
 import { InternalHooks } from '@/InternalHooks';
-import { NodeTypes } from '@/NodeTypes';
-import { ExecutionRepository } from '@/databases/repositories';
-import { EventsService } from '@/services/events.service';
 import { mockInstance } from '../integration/shared/utils';
 import type { IDiagnosticInfo } from '@/Interfaces';
+import { mock } from 'jest-mock-extended';

 jest.mock('@/telemetry');

@@ -15,13 +12,7 @@ let telemetry: Telemetry;
 describe('InternalHooks', () => {
 	beforeAll(() => {
 		telemetry = mockInstance(Telemetry);
-		internalHooks = new InternalHooks(
-			telemetry,
-			mockInstance(NodeTypes),
-			mockInstance(RoleService),
-			mockInstance(ExecutionRepository),
-			mockInstance(EventsService),
-		);
+		internalHooks = new InternalHooks(telemetry, mock(), mock(), mock(), mock(), mock());
 	});

 	it('Should be defined', () => {
@@ -49,6 +40,7 @@ describe('InternalHooks', () => {
 			saml_enabled: true,
 			licensePlanName,
 			licenseTenantId,
+			binary_data_s3: false,
 		};

 		const parameters = {
--- a/packages/cli/test/unit/License.test.ts
+++ b/packages/cli/test/unit/License.test.ts
@@ -2,6 +2,7 @@ import { LicenseManager } from '@n8n_io/license-sdk';
 import { InstanceSettings } from 'n8n-core';
 import config from '@/config';
 import { License } from '@/License';
+import { Logger } from '@/Logger';
 import { N8N_VERSION } from '@/constants';
 import { mockInstance } from '../integration/shared/utils';

@@ -23,10 +24,11 @@ describe('License', () => {
 	});

 	let license: License;
+	const logger = mockInstance(Logger);
 	const instanceSettings = mockInstance(InstanceSettings, { instanceId: MOCK_INSTANCE_ID });

 	beforeEach(async () => {
-		license = new License(instanceSettings);
+		license = new License(logger, instanceSettings);
 		await license.init();
 	});

@@ -38,7 +40,7 @@ describe('License', () => {
 			renewOnInit: true,
 			deviceFingerprint: expect.any(Function),
 			productIdentifier: `n8n-${N8N_VERSION}`,
-			logger: expect.anything(),
+			logger,
 			loadCertStr: expect.any(Function),
 			saveCertStr: expect.any(Function),
 			onFeatureChange: expect.any(Function),
@@ -49,7 +51,7 @@ describe('License', () => {
 	});

 	test('initializes license manager for worker', async () => {
-		license = new License(instanceSettings);
+		license = new License(logger, instanceSettings);
 		await license.init('worker');
 		expect(LicenseManager).toHaveBeenCalledWith({
 			autoRenewEnabled: false,
@@ -58,7 +60,7 @@ describe('License', () => {
 			renewOnInit: false,
 			deviceFingerprint: expect.any(Function),
 			productIdentifier: `n8n-${N8N_VERSION}`,
-			logger: expect.anything(),
+			logger,
 			loadCertStr: expect.any(Function),
 			saveCertStr: expect.any(Function),
 			onFeatureChange: expect.any(Function),
--- a/packages/cli/test/unit/PermissionChecker.test.ts
+++ b/packages/cli/test/unit/PermissionChecker.test.ts
@@ -1,8 +1,7 @@
 import { v4 as uuid } from 'uuid';
 import { Container } from 'typedi';
-import { mock } from 'jest-mock-extended';
-import type { ILogger, INodeTypes } from 'n8n-workflow';
-import { LoggerProxy, SubworkflowOperationError, Workflow } from 'n8n-workflow';
+import type { INodeTypes } from 'n8n-workflow';
+import { SubworkflowOperationError, Workflow } from 'n8n-workflow';

 import config from '@/config';
 import * as Db from '@/Db';
@@ -26,8 +25,6 @@ import * as testDb from '../integration/shared/testDb';
 import type { SaveCredentialFunction } from '../integration/shared/types';
 import { mockNodeTypesData } from './Helpers';

-LoggerProxy.init(mock<ILogger>());
-
 let mockNodeTypes: INodeTypes;
 let credentialOwnerRole: Role;
 let workflowOwnerRole: Role;
--- a/packages/cli/test/unit/SourceControl.test.ts
+++ b/packages/cli/test/unit/SourceControl.test.ts
@@ -15,8 +15,6 @@ import {
 	SOURCE_CONTROL_SSH_FOLDER,
 	SOURCE_CONTROL_GIT_FOLDER,
 } from '@/environments/sourceControl/constants';
-import { LoggerProxy } from 'n8n-workflow';
-import { getLogger } from '@/Logger';
 import { constants as fsConstants, accessSync } from 'fs';
 import type { SourceControlledFile } from '@/environments/sourceControl/types/sourceControlledFile';
 import type { SourceControlPreferences } from '@/environments/sourceControl/types/sourceControlPreferences';
@@ -152,7 +150,6 @@ const pullResult: SourceControlledFile[] = [
 ];

 beforeAll(async () => {
-	LoggerProxy.init(getLogger());
 	Container.get(License).isSourceControlLicensed = () => true;
 	Container.get(SourceControlPreferencesService).getPreferences = () => ({
 		branchName: 'main',
--- a/packages/cli/test/unit/Telemetry.test.ts
+++ b/packages/cli/test/unit/Telemetry.test.ts
@@ -55,7 +55,7 @@ describe('Telemetry', () => {
 		const postHog = new PostHogClient(instanceSettings);
 		await postHog.init();

-		telemetry = new Telemetry(postHog, mock(), instanceSettings);
+		telemetry = new Telemetry(mock(), postHog, mock(), instanceSettings);
 		(telemetry as any).rudderStack = mockRudderStack;
 	});

--- a/packages/cli/test/unit/WorkflowHelpers.test.ts
+++ b/packages/cli/test/unit/WorkflowHelpers.test.ts
@@ -1,5 +1,5 @@
 import type { INode } from 'n8n-workflow';
-import { LoggerProxy, type Workflow } from 'n8n-workflow';
+import { type Workflow } from 'n8n-workflow';
 import { WorkflowEntity } from '@db/entities/WorkflowEntity';
 import { CredentialsEntity } from '@db/entities/CredentialsEntity';
 import {
@@ -7,8 +7,7 @@ import {
 	getNodesWithInaccessibleCreds,
 	validateWorkflowCredentialUsage,
 } from '@/WorkflowHelpers';
-import { getLogger } from '@/Logger';
-import type { IWorkflowExecutionDataProcess } from '../../src/Interfaces';
+import type { IWorkflowExecutionDataProcess } from '@/Interfaces';

 const FIRST_CREDENTIAL_ID = '1';
 const SECOND_CREDENTIAL_ID = '2';
@@ -18,10 +17,6 @@ const NODE_WITH_NO_CRED = '0133467b-df4a-473d-9295-fdd9d01fa45a';
 const NODE_WITH_ONE_CRED = '4673f869-f2dc-4a33-b053-ca3193bc5226';
 const NODE_WITH_TWO_CRED = '9b4208bd-8f10-4a6a-ad3b-da47a326f7da';

-beforeAll(() => {
-	LoggerProxy.init(getLogger());
-});
-
 describe('WorkflowHelpers', () => {
 	describe('getNodesWithInaccessibleCreds', () => {
 		test('Should return an empty list for a workflow without nodes', () => {
--- a/packages/cli/test/unit/WorkflowRunner.test.ts
+++ b/packages/cli/test/unit/WorkflowRunner.test.ts
@@ -4,10 +4,10 @@ import * as utils from '../integration/shared/utils/';
 import { createWorkflow, createExecution } from '../integration/shared/testDb';
 import { WorkflowRunner } from '@/WorkflowRunner';
 import { WorkflowHooks, type ExecutionError, type IWorkflowExecuteHooks } from 'n8n-workflow';
-import { Push } from '../../src/push';
+import { Push } from '@/push';
 import { mockInstance } from '../integration/shared/utils';
 import Container from 'typedi';
-import config from '../../src/config';
+import config from '@/config';

 let owner: User;
 let runner: WorkflowRunner;
--- a/packages/cli/test/unit/controllers/me.controller.test.ts
+++ b/packages/cli/test/unit/controllers/me.controller.test.ts
@@ -1,22 +1,24 @@
 import type { CookieOptions, Response } from 'express';
+import { Container } from 'typedi';
 import jwt from 'jsonwebtoken';
 import { mock, anyObject, captor } from 'jest-mock-extended';
-import type { ILogger } from 'n8n-workflow';
-import type { IExternalHooksClass, IInternalHooksClass, PublicUser } from '@/Interfaces';
+import type { PublicUser } from '@/Interfaces';
 import type { User } from '@db/entities/User';
 import { MeController } from '@/controllers';
 import { AUTH_COOKIE_NAME } from '@/constants';
 import { BadRequestError } from '@/ResponseHelper';
 import type { AuthenticatedRequest, MeRequest } from '@/requests';
+import { UserService } from '@/services/user.service';
+import { ExternalHooks } from '@/ExternalHooks';
+import { InternalHooks } from '@/InternalHooks';
 import { badPasswords } from '../shared/testData';
-import type { UserService } from '@/services/user.service';
+import { mockInstance } from '../../integration/shared/utils';

 describe('MeController', () => {
-	const logger = mock<ILogger>();
-	const externalHooks = mock<IExternalHooksClass>();
-	const internalHooks = mock<IInternalHooksClass>();
-	const userService = mock<UserService>();
-	const controller = new MeController(logger, externalHooks, internalHooks, userService);
+	const externalHooks = mockInstance(ExternalHooks);
+	const internalHooks = mockInstance(InternalHooks);
+	const userService = mockInstance(UserService);
+	const controller = Container.get(MeController);

 	describe('updateCurrentUser', () => {
 		it('should throw BadRequestError if email is missing in the payload', async () => {
--- a/packages/cli/test/unit/controllers/owner.controller.test.ts
+++ b/packages/cli/test/unit/controllers/owner.controller.test.ts
@@ -1,6 +1,5 @@
 import type { CookieOptions, Response } from 'express';
 import { anyObject, captor, mock } from 'jest-mock-extended';
-import type { ILogger } from 'n8n-workflow';
 import jwt from 'jsonwebtoken';
 import type { IInternalHooksClass } from '@/Interfaces';
 import type { User } from '@db/entities/User';
@@ -16,13 +15,12 @@ import { mockInstance } from '../../integration/shared/utils';

 describe('OwnerController', () => {
 	const config = mock<Config>();
-	const logger = mock<ILogger>();
 	const internalHooks = mock<IInternalHooksClass>();
 	const userService = mockInstance(UserService);
 	const settingsRepository = mock<SettingsRepository>();
 	const controller = new OwnerController(
 		config,
-		logger,
+		mock(),
 		internalHooks,
 		settingsRepository,
 		userService,
--- a/packages/cli/test/unit/middleware/listQuery.test.ts
+++ b/packages/cli/test/unit/middleware/listQuery.test.ts
@@ -1,6 +1,4 @@
 import { filterListQueryMiddleware } from '@/middlewares/listQuery/filter';
-import { LoggerProxy } from 'n8n-workflow';
-import { getLogger } from '@/Logger';
 import { selectListQueryMiddleware } from '@/middlewares/listQuery/select';
 import { paginationListQueryMiddleware } from '@/middlewares/listQuery/pagination';
 import * as ResponseHelper from '@/ResponseHelper';
@@ -18,7 +16,6 @@ describe('List query middleware', () => {
 	beforeEach(() => {
 		jest.restoreAllMocks();

-		LoggerProxy.init(getLogger());
 		mockReq = { baseUrl: '/rest/workflows' } as ListQuery.Request;
 		mockRes = { status: () => ({ json: jest.fn() }) } as unknown as Response;
 		args = [mockReq, mockRes, nextFn];
--- a/packages/cli/test/unit/services/cache.service.test.ts
+++ b/packages/cli/test/unit/services/cache.service.test.ts
@@ -3,8 +3,6 @@ import { CacheService } from '@/services/cache.service';
 import type { MemoryCache } from 'cache-manager';
 import type { RedisCache } from 'cache-manager-ioredis-yet';
 import config from '@/config';
-import { LoggerProxy } from 'n8n-workflow';
-import { getLogger } from '@/Logger';

 const cacheService = Container.get(CacheService);

@@ -36,7 +34,6 @@ const testObject: TestObject = {

 describe('cacheService', () => {
 	beforeAll(async () => {
-		LoggerProxy.init(getLogger());
 		jest.mock('ioredis', () => {
 			const Redis = require('ioredis-mock');
 			if (typeof Redis === 'object') {
--- a/packages/cli/test/unit/services/events.service.test.ts
+++ b/packages/cli/test/unit/services/events.service.test.ts
@@ -1,5 +1,4 @@
-import type { IRun, WorkflowExecuteMode, ILogger } from 'n8n-workflow';
-import { LoggerProxy } from 'n8n-workflow';
+import type { IRun, WorkflowExecuteMode } from 'n8n-workflow';
 import {
 	QueryFailedError,
 	type DataSource,
@@ -36,13 +35,13 @@ describe('EventsService', () => {
 	});
 	Object.assign(entityManager, { connection: dataSource });

-	LoggerProxy.init(mock<ILogger>());
 	config.set('diagnostics.enabled', true);
 	config.set('deployment.type', 'n8n-testing');
 	mocked(ownershipService.getWorkflowOwnerCached).mockResolvedValue(fakeUser);
 	const updateSettingsMock = jest.spyOn(userService, 'updateSettings').mockImplementation();

 	const eventsService = new EventsService(
+		mock(),
 		new WorkflowStatisticsRepository(dataSource),
 		ownershipService,
 	);
--- a/packages/cli/test/unit/services/orchestration.service.test.ts
+++ b/packages/cli/test/unit/services/orchestration.service.test.ts
@@ -1,7 +1,5 @@
 import Container from 'typedi';
 import config from '@/config';
-import { LoggerProxy } from 'n8n-workflow';
-import { getLogger } from '@/Logger';
 import { OrchestrationMainService } from '@/services/orchestration/main/orchestration.main.service';
 import type { RedisServiceWorkerResponseObject } from '@/services/redis/RedisServiceCommands';
 import { eventBus } from '@/eventbus';
@@ -12,6 +10,7 @@ import { handleCommandMessageMain } from '@/services/orchestration/main/handleCo
 import { OrchestrationHandlerMainService } from '@/services/orchestration/main/orchestration.handler.main.service';
 import * as helpers from '@/services/orchestration/helpers';
 import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee';
+import { Logger } from '@/Logger';

 const os = Container.get(OrchestrationMainService);
 const handler = Container.get(OrchestrationHandlerMainService);
@@ -33,10 +32,10 @@ const workerRestartEventbusResponse: RedisServiceWorkerResponseObject = {
 };

 describe('Orchestration Service', () => {
+	const logger = mockInstance(Logger);
 	beforeAll(async () => {
 		mockInstance(RedisService);
 		mockInstance(ExternalSecretsManager);
-		LoggerProxy.init(getLogger());
 		jest.mock('ioredis', () => {
 			const Redis = require('ioredis-mock');
 			if (typeof Redis === 'object') {
@@ -52,7 +51,7 @@ describe('Orchestration Service', () => {
 				return new Redis(args);
 			};
 		});
-		jest.mock('../../../src/services/redis/RedisServicePubSubPublisher', () => {
+		jest.mock('@/services/redis/RedisServicePubSubPublisher', () => {
 			return jest.fn().mockImplementation(() => {
 				return {
 					init: jest.fn(),
@@ -62,7 +61,7 @@ describe('Orchestration Service', () => {
 				};
 			});
 		});
-		jest.mock('../../../src/services/redis/RedisServicePubSubSubscriber', () => {
+		jest.mock('@/services/redis/RedisServicePubSubSubscriber', () => {
 			return jest.fn().mockImplementation(() => {
 				return {
 					subscribeToCommandChannel: jest.fn(),
@@ -75,8 +74,8 @@ describe('Orchestration Service', () => {
 	});

 	afterAll(async () => {
-		jest.mock('../../../src/services/redis/RedisServicePubSubPublisher').restoreAllMocks();
-		jest.mock('../../../src/services/redis/RedisServicePubSubSubscriber').restoreAllMocks();
+		jest.mock('@/services/redis/RedisServicePubSubPublisher').restoreAllMocks();
+		jest.mock('@/services/redis/RedisServicePubSubSubscriber').restoreAllMocks();
 		await os.shutdown();
 	});

@@ -96,7 +95,6 @@ describe('Orchestration Service', () => {
 	});

 	test('should handle command messages from others', async () => {
-		jest.spyOn(LoggerProxy, 'error');
 		const responseFalseId = await handleCommandMessageMain(
 			JSON.stringify({
 				senderId: 'test',
@@ -106,8 +104,7 @@ describe('Orchestration Service', () => {
 		expect(responseFalseId).toBeDefined();
 		expect(responseFalseId!.command).toEqual('reloadLicense');
 		expect(responseFalseId!.senderId).toEqual('test');
-		expect(LoggerProxy.error).toHaveBeenCalled();
-		jest.spyOn(LoggerProxy, 'error').mockRestore();
+		expect(logger.error).toHaveBeenCalled();
 	});

 	test('should reject command messages from iteslf', async () => {
--- a/packages/cli/test/unit/services/redis.service.test.ts
+++ b/packages/cli/test/unit/services/redis.service.test.ts
@@ -1,41 +1,22 @@
 import Container from 'typedi';
+import { Logger } from '@/Logger';
 import config from '@/config';
-import { LoggerProxy } from 'n8n-workflow';
-import { getLogger } from '@/Logger';
 import { RedisService } from '@/services/redis.service';
+import { mockInstance } from '../../integration/shared/utils';

+mockInstance(Logger);
 const redisService = Container.get(RedisService);

 function setDefaultConfig() {
 	config.set('executions.mode', 'queue');
 }

-interface TestObject {
-	test: string;
-	test2: number;
-	test3?: TestObject & { test4: TestObject };
-}
-
-const testObject: TestObject = {
-	test: 'test',
-	test2: 123,
-	test3: {
-		test: 'test3',
-		test2: 123,
-		test4: {
-			test: 'test4',
-			test2: 123,
-		},
-	},
-};
-
 const PUBSUB_CHANNEL = 'testchannel';
 const LIST_CHANNEL = 'testlist';
 const STREAM_CHANNEL = 'teststream';

-describe('cacheService', () => {
+describe('RedisService', () => {
 	beforeAll(async () => {
-		LoggerProxy.init(getLogger());
 		jest.mock('ioredis', () => {
 			const Redis = require('ioredis-mock');
 			if (typeof Redis === 'object') {
--- a/packages/cli/test/unit/utils.test.ts
+++ b/packages/cli/test/unit/utils.test.ts
@@ -1,4 +1,4 @@
-import { webhookNotFoundErrorMessage } from '../../src/utils';
+import { webhookNotFoundErrorMessage } from '@/utils';

 describe('utils test webhookNotFoundErrorMessage ', () => {
 	it('should return a message with path and method', () => {