From 963ddeb0cc974df6e6594c2396afe1311c2707a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 14 Oct 2024 16:52:56 +0200 Subject: [PATCH] refactor(core): Remove deprecated properties from orchestration service (#11251) --- packages/cli/src/__tests__/wait-tracker.test.ts | 14 +++++++++++--- packages/cli/src/active-workflow-manager.ts | 5 +++-- packages/cli/src/controllers/debug.controller.ts | 7 +++++-- packages/cli/src/license.ts | 5 +---- packages/cli/src/services/orchestration.service.ts | 14 ++------------ packages/cli/src/services/pruning.service.ts | 3 ++- packages/cli/src/wait-tracker.ts | 5 ++++- .../cli/test/integration/debug.controller.test.ts | 10 ++++++---- 8 files changed, 34 insertions(+), 29 deletions(-) diff --git a/packages/cli/src/__tests__/wait-tracker.test.ts b/packages/cli/src/__tests__/wait-tracker.test.ts index 8f9f31da00..e51cd88ccb 100644 --- a/packages/cli/src/__tests__/wait-tracker.test.ts +++ b/packages/cli/src/__tests__/wait-tracker.test.ts @@ -1,4 +1,5 @@ import { mock } from 'jest-mock-extended'; +import type { InstanceSettings } from 'n8n-core'; import type { ExecutionRepository } from '@/databases/repositories/execution.repository'; import type { IExecutionResponse } from '@/interfaces'; @@ -13,6 +14,7 @@ describe('WaitTracker', () => { const executionRepository = mock(); const multiMainSetup = mock(); const orchestrationService = new OrchestrationService(mock(), mock(), multiMainSetup); + const instanceSettings = mock({ isLeader: true }); const execution = mock({ id: '123', @@ -27,6 +29,7 @@ describe('WaitTracker', () => { mock(), mock(), orchestrationService, + instanceSettings, ); multiMainSetup.on.mockReturnThis(); }); @@ -37,7 +40,6 @@ describe('WaitTracker', () => { describe('init()', () => { it('should query DB for waiting executions if leader', async () => { - jest.spyOn(orchestrationService, 'isLeader', 'get').mockReturnValue(true); executionRepository.getWaitingExecutions.mockResolvedValue([execution]); waitTracker.init(); @@ -120,7 +122,6 @@ describe('WaitTracker', () => { describe('multi-main setup', () => { it('should start tracking if leader', () => { - jest.spyOn(orchestrationService, 'isLeader', 'get').mockReturnValue(true); jest.spyOn(orchestrationService, 'isSingleMainSetup', 'get').mockReturnValue(false); executionRepository.getWaitingExecutions.mockResolvedValue([]); @@ -131,7 +132,14 @@ describe('WaitTracker', () => { }); it('should not start tracking if follower', () => { - jest.spyOn(orchestrationService, 'isLeader', 'get').mockReturnValue(false); + const waitTracker = new WaitTracker( + mockLogger(), + executionRepository, + mock(), + mock(), + orchestrationService, + mock({ isLeader: false }), + ); jest.spyOn(orchestrationService, 'isSingleMainSetup', 'get').mockReturnValue(false); executionRepository.getWaitingExecutions.mockResolvedValue([]); diff --git a/packages/cli/src/active-workflow-manager.ts b/packages/cli/src/active-workflow-manager.ts index 9c5ef15f38..4127909e49 100644 --- a/packages/cli/src/active-workflow-manager.ts +++ b/packages/cli/src/active-workflow-manager.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ -import { ActiveWorkflows, NodeExecuteFunctions } from 'n8n-core'; +import { ActiveWorkflows, InstanceSettings, NodeExecuteFunctions } from 'n8n-core'; import type { ExecutionError, IDeferredPromise, @@ -74,6 +74,7 @@ export class ActiveWorkflowManager { private readonly workflowStaticDataService: WorkflowStaticDataService, private readonly activeWorkflowsService: ActiveWorkflowsService, private readonly workflowExecutionService: WorkflowExecutionService, + private readonly instanceSettings: InstanceSettings, ) {} async init() { @@ -423,7 +424,7 @@ export class ActiveWorkflowManager { if (dbWorkflows.length === 0) return; - if (this.orchestrationService.isLeader) { + if (this.instanceSettings.isLeader) { this.logger.info(' ================================'); this.logger.info(' Start Active Workflows:'); this.logger.info(' ================================'); diff --git a/packages/cli/src/controllers/debug.controller.ts b/packages/cli/src/controllers/debug.controller.ts index 9fd2b067d3..1a2b08d550 100644 --- a/packages/cli/src/controllers/debug.controller.ts +++ b/packages/cli/src/controllers/debug.controller.ts @@ -1,3 +1,5 @@ +import { InstanceSettings } from 'n8n-core'; + import { ActiveWorkflowManager } from '@/active-workflow-manager'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import { Get, RestController } from '@/decorators'; @@ -9,6 +11,7 @@ export class DebugController { private readonly orchestrationService: OrchestrationService, private readonly activeWorkflowManager: ActiveWorkflowManager, private readonly workflowRepository: WorkflowRepository, + private readonly instanceSettings: InstanceSettings, ) {} @Get('/multi-main-setup', { skipAuth: true }) @@ -24,9 +27,9 @@ export class DebugController { const activationErrors = await this.activeWorkflowManager.getAllWorkflowActivationErrors(); return { - instanceId: this.orchestrationService.instanceId, + instanceId: this.instanceSettings.instanceId, leaderKey, - isLeader: this.orchestrationService.isLeader, + isLeader: this.instanceSettings.isLeader, activeWorkflows: { webhooks, // webhook-based active workflows triggersAndPollers, // poller- and trigger-based active workflows diff --git a/packages/cli/src/license.ts b/packages/cli/src/license.ts index c6e8dfa19f..da7ab80313 100644 --- a/packages/cli/src/license.ts +++ b/packages/cli/src/license.ts @@ -143,10 +143,7 @@ export class License { this.orchestrationService.setMultiMainSetupLicensed(isMultiMainLicensed ?? false); - if ( - this.orchestrationService.isMultiMainSetupEnabled && - this.orchestrationService.isFollower - ) { + if (this.orchestrationService.isMultiMainSetupEnabled && this.instanceSettings.isFollower) { this.logger.debug( '[Multi-main setup] Instance is follower, skipping sending of "reload-license" command...', ); diff --git a/packages/cli/src/services/orchestration.service.ts b/packages/cli/src/services/orchestration.service.ts index b8aba46285..a248a144ec 100644 --- a/packages/cli/src/services/orchestration.service.ts +++ b/packages/cli/src/services/orchestration.service.ts @@ -47,16 +47,6 @@ export class OrchestrationService { return config.getEnv('redis.queueModeId'); } - /** @deprecated use InstanceSettings.isLeader */ - get isLeader() { - return this.instanceSettings.isLeader; - } - - /** @deprecated use InstanceSettings.isFollower */ - get isFollower() { - return this.instanceSettings.isFollower; - } - sanityCheck() { return this.isInitialized && config.get('executions.mode') === 'queue'; } @@ -144,7 +134,7 @@ export class OrchestrationService { if (activationMode === 'leadershipChange') return false; - return this.isLeader; // 'update' or 'activate' + return this.instanceSettings.isLeader; // 'update' or 'activate' } /** @@ -154,6 +144,6 @@ export class OrchestrationService { * triggers and pollers in memory, to ensure they are not duplicated. */ shouldAddTriggersAndPollers() { - return this.isLeader; + return this.instanceSettings.isLeader; } } diff --git a/packages/cli/src/services/pruning.service.ts b/packages/cli/src/services/pruning.service.ts index 48d4b0db3b..e9ceab5434 100644 --- a/packages/cli/src/services/pruning.service.ts +++ b/packages/cli/src/services/pruning.service.ts @@ -37,7 +37,8 @@ export class PruningService { * @important Requires `OrchestrationService` to be initialized. */ init() { - const { isLeader, isMultiMainSetupEnabled } = this.orchestrationService; + const { isLeader } = this.instanceSettings; + const { isMultiMainSetupEnabled } = this.orchestrationService; if (isLeader) this.startPruning(); diff --git a/packages/cli/src/wait-tracker.ts b/packages/cli/src/wait-tracker.ts index e999c30401..e7e8b428ed 100644 --- a/packages/cli/src/wait-tracker.ts +++ b/packages/cli/src/wait-tracker.ts @@ -1,3 +1,4 @@ +import { InstanceSettings } from 'n8n-core'; import { ApplicationError, ErrorReporterProxy as ErrorReporter, @@ -28,6 +29,7 @@ export class WaitTracker { private readonly ownershipService: OwnershipService, private readonly workflowRunner: WorkflowRunner, private readonly orchestrationService: OrchestrationService, + private readonly instanceSettings: InstanceSettings, ) { this.logger = this.logger.withScope('executions'); } @@ -40,7 +42,8 @@ export class WaitTracker { * @important Requires `OrchestrationService` to be initialized. */ init() { - const { isLeader, isMultiMainSetupEnabled } = this.orchestrationService; + const { isLeader } = this.instanceSettings; + const { isMultiMainSetupEnabled } = this.orchestrationService; if (isLeader) this.startTracking(); diff --git a/packages/cli/test/integration/debug.controller.test.ts b/packages/cli/test/integration/debug.controller.test.ts index 723edea58a..47695e59aa 100644 --- a/packages/cli/test/integration/debug.controller.test.ts +++ b/packages/cli/test/integration/debug.controller.test.ts @@ -1,9 +1,11 @@ +import { InstanceSettings } from 'n8n-core'; +import Container from 'typedi'; + import { ActiveWorkflowManager } from '@/active-workflow-manager'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import { generateNanoId } from '@/databases/utils/generators'; import { MultiMainSetup } from '@/services/orchestration/main/multi-main-setup.ee'; -import { OrchestrationService } from '@/services/orchestration.service'; import { createOwner } from './shared/db/users'; import { randomName } from './shared/random'; @@ -14,6 +16,8 @@ import { mockInstance } from '../shared/mocking'; describe('DebugController', () => { const workflowRepository = mockInstance(WorkflowRepository); const activeWorkflowManager = mockInstance(ActiveWorkflowManager); + const instanceSettings = Container.get(InstanceSettings); + instanceSettings.markAsLeader(); let testServer = setupTestServer({ endpointGroups: ['debug'] }); let ownerAgent: SuperAgentTest; @@ -30,7 +34,7 @@ describe('DebugController', () => { const webhooks = [{ id: workflowId, name: randomName() }] as WorkflowEntity[]; const triggersAndPollers = [{ id: workflowId, name: randomName() }] as WorkflowEntity[]; const activationErrors = { [workflowId]: 'Failed to activate' }; - const instanceId = 'main-71JdWtq306epIFki'; + const { instanceId } = instanceSettings; const leaderKey = 'some-leader-key'; workflowRepository.findIn.mockResolvedValue(triggersAndPollers); @@ -38,9 +42,7 @@ describe('DebugController', () => { activeWorkflowManager.allActiveInMemory.mockReturnValue([workflowId]); activeWorkflowManager.getAllWorkflowActivationErrors.mockResolvedValue(activationErrors); - jest.spyOn(OrchestrationService.prototype, 'instanceId', 'get').mockReturnValue(instanceId); jest.spyOn(MultiMainSetup.prototype, 'fetchLeaderKey').mockResolvedValue(leaderKey); - jest.spyOn(OrchestrationService.prototype, 'isLeader', 'get').mockReturnValue(true); const response = await ownerAgent.get('/debug/multi-main-setup').expect(200);