diff --git a/packages/@n8n/decorators/src/module/__tests__/module.test.ts b/packages/@n8n/decorators/src/module/__tests__/module.test.ts index d54fc1a834..761a4326ec 100644 --- a/packages/@n8n/decorators/src/module/__tests__/module.test.ts +++ b/packages/@n8n/decorators/src/module/__tests__/module.test.ts @@ -1,9 +1,9 @@ import { Container } from '@n8n/di'; -import { N8nModule } from '../module'; +import { BackendModule } from '../module'; import { ModuleMetadata } from '../module-metadata'; -describe('@N8nModule Decorator', () => { +describe('@BackendModule decorator', () => { let moduleMetadata: ModuleMetadata; beforeEach(() => { @@ -14,7 +14,7 @@ describe('@N8nModule Decorator', () => { }); it('should register module in ModuleMetadata', () => { - @N8nModule() + @BackendModule() class TestModule { initialize() {} } @@ -26,17 +26,17 @@ describe('@N8nModule Decorator', () => { }); it('should register multiple modules', () => { - @N8nModule() + @BackendModule() class FirstModule { initialize() {} } - @N8nModule() + @BackendModule() class SecondModule { initialize() {} } - @N8nModule() + @BackendModule() class ThirdModule { initialize() {} } @@ -50,7 +50,7 @@ describe('@N8nModule Decorator', () => { }); it('should work with modules without initialize method', () => { - @N8nModule() + @BackendModule() class TestModule {} const registeredModules = Array.from(moduleMetadata.getModules()); @@ -62,7 +62,7 @@ describe('@N8nModule Decorator', () => { it('should support async initialize method', async () => { const mockInitialize = jest.fn(); - @N8nModule() + @BackendModule() class TestModule { async initialize() { mockInitialize(); @@ -81,10 +81,10 @@ describe('@N8nModule Decorator', () => { describe('ModuleMetadata', () => { it('should allow retrieving and checking registered modules', () => { - @N8nModule() + @BackendModule() class FirstModule {} - @N8nModule() + @BackendModule() class SecondModule {} const registeredModules = Array.from(moduleMetadata.getModules()); @@ -95,7 +95,7 @@ describe('@N8nModule Decorator', () => { }); it('should apply Service decorator', () => { - @N8nModule() + @BackendModule() class TestModule {} expect(Container.has(TestModule)).toBe(true); diff --git a/packages/@n8n/decorators/src/module/index.ts b/packages/@n8n/decorators/src/module/index.ts index 5e1a8aad26..ae9ba9165d 100644 --- a/packages/@n8n/decorators/src/module/index.ts +++ b/packages/@n8n/decorators/src/module/index.ts @@ -1,2 +1,2 @@ -export { BaseN8nModule, N8nModule } from './module'; +export { ModuleInterface, BackendModule, EntityClass } from './module'; export { ModuleMetadata } from './module-metadata'; diff --git a/packages/@n8n/decorators/src/module/module-metadata.ts b/packages/@n8n/decorators/src/module/module-metadata.ts index 891dedfebe..d6bcb7ac8f 100644 --- a/packages/@n8n/decorators/src/module/module-metadata.ts +++ b/packages/@n8n/decorators/src/module/module-metadata.ts @@ -1,12 +1,12 @@ import { Service } from '@n8n/di'; -import type { Module } from './module'; +import type { ModuleClass } from './module'; @Service() export class ModuleMetadata { - private readonly modules: Set = new Set(); + private readonly modules: Set = new Set(); - register(module: Module) { + register(module: ModuleClass) { this.modules.add(module); } diff --git a/packages/@n8n/decorators/src/module/module.ts b/packages/@n8n/decorators/src/module/module.ts index 096d8825ee..cdc1d2a042 100644 --- a/packages/@n8n/decorators/src/module/module.ts +++ b/packages/@n8n/decorators/src/module/module.ts @@ -2,14 +2,30 @@ import { Container, Service, type Constructable } from '@n8n/di'; import { ModuleMetadata } from './module-metadata'; -export interface BaseN8nModule { - initialize?(): void | Promise; +/** + * Structurally similar (not identical) interface to typeorm's `BaseEntity` + * to prevent importing `@n8n/typeorm` into `@n8n/decorators`. + */ +export interface BaseEntity { + hasId(): boolean; + save(options?: unknown): Promise; + remove(options?: unknown): Promise; + softRemove(options?: unknown): Promise; + recover(options?: unknown): Promise; + reload(): Promise; } -export type Module = Constructable; +export type EntityClass = new () => BaseEntity; -export const N8nModule = (): ClassDecorator => (target) => { - Container.get(ModuleMetadata).register(target as unknown as Module); +export interface ModuleInterface { + init?(): void | Promise; + entities?(): EntityClass[]; +} + +export type ModuleClass = Constructable; + +export const BackendModule = (): ClassDecorator => (target) => { + Container.get(ModuleMetadata).register(target as unknown as ModuleClass); // eslint-disable-next-line @typescript-eslint/no-unsafe-return return Service()(target); diff --git a/packages/cli/bin/n8n b/packages/cli/bin/n8n index a20f8b9a34..af3322971f 100755 --- a/packages/cli/bin/n8n +++ b/packages/cli/bin/n8n @@ -63,6 +63,10 @@ if (process.env.NODEJS_PREFER_IPV4 === 'true') { require('net').setDefaultAutoSelectFamily?.(false); (async () => { + // Collect DB entities from modules _before_ `DbConnectionOptions` is instantiated. + const { BaseCommand } = await import('../dist/commands/base-command.js'); + await new BaseCommand([], { root: __dirname }).loadModules(); + const oclif = await import('@oclif/core'); await oclif.execute({ dir: __dirname }); })(); diff --git a/packages/cli/src/commands/base-command.ts b/packages/cli/src/commands/base-command.ts index a7ba2a3a0c..936fb83b17 100644 --- a/packages/cli/src/commands/base-command.ts +++ b/packages/cli/src/commands/base-command.ts @@ -28,7 +28,6 @@ import { ExternalHooks } from '@/external-hooks'; import { License } from '@/license'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { ModuleRegistry } from '@/modules/module-registry'; -import type { ModulePreInit } from '@/modules/modules.config'; import { ModulesConfig } from '@/modules/modules.config'; import { NodeTypes } from '@/node-types'; import { PostHogClient } from '@/posthog'; @@ -39,7 +38,7 @@ import { WorkflowHistoryManager } from '@/workflows/workflow-history.ee/workflow export abstract class BaseCommand extends Command { protected logger = Container.get(Logger); - protected dbConnection = Container.get(DbConnection); + protected dbConnection: DbConnection; protected errorReporter: ErrorReporter; @@ -59,6 +58,8 @@ export abstract class BaseCommand extends Command { protected readonly modulesConfig = Container.get(ModulesConfig); + protected readonly moduleRegistry = Container.get(ModuleRegistry); + /** * How long to wait for graceful shutdown before force killing the process. */ @@ -73,27 +74,18 @@ export abstract class BaseCommand extends Command { protected async loadModules() { for (const moduleName of this.modulesConfig.modules) { - let preInitModule: ModulePreInit | undefined; + // add module to the registry for dependency injection try { - preInitModule = (await import( - `../modules/${moduleName}/${moduleName}.pre-init` - )) as ModulePreInit; - } catch {} - - if ( - !preInitModule || - preInitModule.shouldLoadModule?.({ - instance: this.instanceSettings, - }) - ) { await import(`../modules/${moduleName}/${moduleName}.module`); - - this.modulesConfig.addLoadedModule(moduleName); - this.logger.debug(`Loaded module "${moduleName}"`); + } catch { + await import(`../modules/${moduleName}.ee/${moduleName}.module`); } + + this.modulesConfig.addLoadedModule(moduleName); + this.logger.debug(`Loaded module "${moduleName}"`); } - await Container.get(ModuleRegistry).initializeModules(); + this.moduleRegistry.addEntities(); if (this.instanceSettings.isMultiMain) { Container.get(MultiMainSetup).registerEventHandlers(); @@ -101,6 +93,7 @@ export abstract class BaseCommand extends Command { } async init(): Promise { + this.dbConnection = Container.get(DbConnection); this.errorReporter = Container.get(ErrorReporter); const { backendDsn, environment, deploymentName } = this.globalConfig.sentry; diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 039e5a0643..d4037561eb 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -248,7 +248,7 @@ export class Start extends BaseCommand { await this.generateStaticAssets(); } - await this.loadModules(); + await this.moduleRegistry.initModules(); } async initOrchestration() { diff --git a/packages/cli/src/commands/webhook.ts b/packages/cli/src/commands/webhook.ts index f51b289fd2..92cd196916 100644 --- a/packages/cli/src/commands/webhook.ts +++ b/packages/cli/src/commands/webhook.ts @@ -78,7 +78,7 @@ export class Webhook extends BaseCommand { await this.initExternalHooks(); this.logger.debug('External hooks init complete'); - await this.loadModules(); + await this.moduleRegistry.initModules(); } async run() { diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index 9d662f4842..23a1dfff5d 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -109,7 +109,7 @@ export class Worker extends BaseCommand { }), ); - await this.loadModules(); + await this.moduleRegistry.initModules(); } async initEventBus() { diff --git a/packages/cli/src/databases/__tests__/db-connection-options.test.ts b/packages/cli/src/databases/__tests__/db-connection-options.test.ts index 6e2f9dd0bb..3e14b302ca 100644 --- a/packages/cli/src/databases/__tests__/db-connection-options.test.ts +++ b/packages/cli/src/databases/__tests__/db-connection-options.test.ts @@ -5,6 +5,8 @@ import { sqliteMigrations } from '@n8n/db'; import { mock } from 'jest-mock-extended'; import path from 'path'; +import type { ModuleRegistry } from '@/modules/module-registry'; + import { DbConnectionOptions } from '../db-connection-options'; describe('DbConnectionOptions', () => { @@ -17,7 +19,12 @@ describe('DbConnectionOptions', () => { }); const n8nFolder = '/test/n8n'; const instanceSettingsConfig = mock({ n8nFolder }); - const dbConnectionOptions = new DbConnectionOptions(dbConfig, instanceSettingsConfig); + const moduleRegistry = mock({ entities: [] }); + const dbConnectionOptions = new DbConnectionOptions( + dbConfig, + instanceSettingsConfig, + moduleRegistry, + ); beforeEach(() => jest.resetAllMocks()); diff --git a/packages/cli/src/databases/db-connection-options.ts b/packages/cli/src/databases/db-connection-options.ts index ddce1cb75b..e820ec643e 100644 --- a/packages/cli/src/databases/db-connection-options.ts +++ b/packages/cli/src/databases/db-connection-options.ts @@ -16,15 +16,14 @@ import { UserError } from 'n8n-workflow'; import path from 'path'; import type { TlsOptions } from 'tls'; -import { InsightsByPeriod } from '@/modules/insights/database/entities/insights-by-period'; -import { InsightsMetadata } from '@/modules/insights/database/entities/insights-metadata'; -import { InsightsRaw } from '@/modules/insights/database/entities/insights-raw'; +import { ModuleRegistry } from '@/modules/module-registry'; @Service() export class DbConnectionOptions { constructor( private readonly config: DatabaseConfig, private readonly instanceSettingsConfig: InstanceSettingsConfig, + private readonly moduleRegistry: ModuleRegistry, ) {} getOverrides(dbType: 'postgresdb' | 'mysqldb') { @@ -68,7 +67,7 @@ export class DbConnectionOptions { return { entityPrefix, - entities: [...Object.values(entities), InsightsRaw, InsightsByPeriod, InsightsMetadata], + entities: [...Object.values(entities), ...this.moduleRegistry.entities], subscribers: Object.values(subscribers), migrationsTableName: `${entityPrefix}migrations`, migrationsRun: false, diff --git a/packages/cli/src/modules/__tests__/modules.config.test.ts b/packages/cli/src/modules/__tests__/modules.config.test.ts index 135031dd6c..a13aebb63d 100644 --- a/packages/cli/src/modules/__tests__/modules.config.test.ts +++ b/packages/cli/src/modules/__tests__/modules.config.test.ts @@ -12,19 +12,19 @@ describe('ModulesConfig', () => { it('should initialize with insights modules if no environment variable is set', () => { const config = Container.get(ModulesConfig); - expect(config.modules).toEqual(['insights', 'external-secrets.ee']); + expect(config.modules).toEqual(['insights', 'external-secrets']); }); it('should parse valid module names from environment variable', () => { process.env.N8N_ENABLED_MODULES = 'insights'; const config = Container.get(ModulesConfig); - expect(config.modules).toEqual(['insights', 'external-secrets.ee']); + expect(config.modules).toEqual(['insights', 'external-secrets']); }); it('should disable valid module names from environment variable', () => { process.env.N8N_DISABLED_MODULES = 'insights'; const config = Container.get(ModulesConfig); - expect(config.modules).toEqual(['external-secrets.ee']); + expect(config.modules).toEqual(['external-secrets']); }); it('should throw UnexpectedError for invalid module names', () => { diff --git a/packages/cli/src/modules/external-secrets.ee/external-secrets.ee.module.ts b/packages/cli/src/modules/external-secrets.ee/external-secrets.ee.module.ts deleted file mode 100644 index 55ce55f6a3..0000000000 --- a/packages/cli/src/modules/external-secrets.ee/external-secrets.ee.module.ts +++ /dev/null @@ -1,20 +0,0 @@ -import type { BaseN8nModule } from '@n8n/decorators'; -import { N8nModule } from '@n8n/decorators'; -import { ExternalSecretsProxy } from 'n8n-core'; - -import { ExternalSecretsManager } from './external-secrets-manager.ee'; -import './external-secrets.controller.ee'; - -@N8nModule() -export class ExternalSecretsModule implements BaseN8nModule { - constructor( - private readonly manager: ExternalSecretsManager, - private readonly externalSecretsProxy: ExternalSecretsProxy, - ) {} - - async initialize() { - const { externalSecretsProxy, manager } = this; - await manager.init(); - externalSecretsProxy.setManager(manager); - } -} diff --git a/packages/cli/src/modules/external-secrets.ee/external-secrets.module.ts b/packages/cli/src/modules/external-secrets.ee/external-secrets.module.ts new file mode 100644 index 0000000000..c084b5a212 --- /dev/null +++ b/packages/cli/src/modules/external-secrets.ee/external-secrets.module.ts @@ -0,0 +1,19 @@ +import type { ModuleInterface } from '@n8n/decorators'; +import { BackendModule } from '@n8n/decorators'; +import { Container } from '@n8n/di'; + +@BackendModule() +export class ExternalSecretsModule implements ModuleInterface { + async init() { + await import('./external-secrets.controller.ee'); + + const { ExternalSecretsManager } = await import('./external-secrets-manager.ee'); + const { ExternalSecretsProxy } = await import('n8n-core'); + + const externalSecretsManager = Container.get(ExternalSecretsManager); + const externalSecretsProxy = Container.get(ExternalSecretsProxy); + + await externalSecretsManager.init(); + externalSecretsProxy.setManager(externalSecretsManager); + } +} diff --git a/packages/cli/src/modules/insights/__tests__/insights-collection.service.test.ts b/packages/cli/src/modules/insights/__tests__/insights-collection.service.test.ts index c8811c3f77..21781fceaf 100644 --- a/packages/cli/src/modules/insights/__tests__/insights-collection.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights-collection.service.test.ts @@ -21,12 +21,13 @@ import { mockLogger } from '@test/mocking'; import { createTeamProject } from '@test-integration/db/projects'; import { createWorkflow } from '@test-integration/db/workflows'; import * as testDb from '@test-integration/test-db'; +import * as testModules from '@test-integration/test-modules'; import { InsightsCollectionService } from '../insights-collection.service'; import { InsightsConfig } from '../insights.config'; -// Initialize DB once for all tests beforeAll(async () => { + await testModules.load(['insights']); await testDb.init(); }); diff --git a/packages/cli/src/modules/insights/__tests__/insights-compaction.service.test.ts b/packages/cli/src/modules/insights/__tests__/insights-compaction.service.test.ts index 64853615f2..1ca5363aa5 100644 --- a/packages/cli/src/modules/insights/__tests__/insights-compaction.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights-compaction.service.test.ts @@ -7,6 +7,7 @@ import { mockLogger } from '@test/mocking'; import { createTeamProject } from '@test-integration/db/projects'; import { createWorkflow } from '@test-integration/db/workflows'; import * as testDb from '@test-integration/test-db'; +import * as testModules from '@test-integration/test-modules'; import { createMetadata, @@ -18,8 +19,8 @@ import { InsightsByPeriodRepository } from '../database/repositories/insights-by import { InsightsCompactionService } from '../insights-compaction.service'; import { InsightsConfig } from '../insights.config'; -// Initialize DB once for all tests beforeAll(async () => { + await testModules.load(['insights']); await testDb.init(); }); diff --git a/packages/cli/src/modules/insights/__tests__/insights-pruning.service.test.ts b/packages/cli/src/modules/insights/__tests__/insights-pruning.service.test.ts index 14384b60dd..4ad650fbb9 100644 --- a/packages/cli/src/modules/insights/__tests__/insights-pruning.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights-pruning.service.test.ts @@ -8,6 +8,7 @@ import { mockLogger } from '@test/mocking'; import { createTeamProject } from '@test-integration/db/projects'; import { createWorkflow } from '@test-integration/db/workflows'; import * as testDb from '@test-integration/test-db'; +import * as testModules from '@test-integration/test-modules'; import { createCompactedInsightsEvent, @@ -18,6 +19,7 @@ import { InsightsPruningService } from '../insights-pruning.service'; import { InsightsConfig } from '../insights.config'; beforeAll(async () => { + await testModules.load(['insights']); await testDb.init(); }); diff --git a/packages/cli/src/modules/insights/__tests__/insights.pre-init.test.ts b/packages/cli/src/modules/insights/__tests__/insights.pre-init.test.ts deleted file mode 100644 index 071d74a7da..0000000000 --- a/packages/cli/src/modules/insights/__tests__/insights.pre-init.test.ts +++ /dev/null @@ -1,26 +0,0 @@ -import type { InstanceType } from '@n8n/constants'; -import { mock } from 'jest-mock-extended'; -import type { InstanceSettings } from 'n8n-core'; - -import type { ModulePreInitContext } from '@/modules/modules.config'; - -import { shouldLoadModule } from '../insights.pre-init'; - -describe('InsightsModulePreInit', () => { - it('should return false if instance type is worker', () => { - const ctx: ModulePreInitContext = { - instance: mock({ instanceType: 'worker' }), - }; - expect(shouldLoadModule(ctx)).toBe(false); - }); - - it.each(['main', 'webhook'])( - 'should return true if instance type is "%s"', - (instanceType) => { - const ctx: ModulePreInitContext = { - instance: mock({ instanceType }), - }; - expect(shouldLoadModule(ctx)).toBe(true); - }, - ); -}); diff --git a/packages/cli/src/modules/insights/__tests__/insights.service.test.ts b/packages/cli/src/modules/insights/__tests__/insights.service.test.ts index 5fdaf33fb6..5a6a6bf037 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.service.test.ts @@ -15,6 +15,7 @@ import { mockLogger } from '@test/mocking'; import { createTeamProject } from '@test-integration/db/projects'; import { createWorkflow } from '@test-integration/db/workflows'; import * as testDb from '@test-integration/test-db'; +import * as testModules from '@test-integration/test-modules'; import { createCompactedInsightsEvent, @@ -25,12 +26,13 @@ import type { InsightsRaw } from '../database/entities/insights-raw'; import type { InsightsByPeriodRepository } from '../database/repositories/insights-by-period.repository'; import { InsightsCollectionService } from '../insights-collection.service'; import { InsightsCompactionService } from '../insights-compaction.service'; +import { getAvailableDateRanges } from '../insights-helpers'; import type { InsightsPruningService } from '../insights-pruning.service'; import { InsightsConfig } from '../insights.config'; import { InsightsService } from '../insights.service'; -// Initialize DB once for all tests beforeAll(async () => { + await testModules.load(['insights']); await testDb.init(); }); @@ -581,27 +583,17 @@ describe('getInsightsByTime', () => { }); describe('getAvailableDateRanges', () => { - let insightsService: InsightsService; let licenseMock: jest.Mocked; beforeAll(() => { licenseMock = mock(); - insightsService = new InsightsService( - mock(), - mock(), - mock(), - mock(), - licenseMock, - mock(), - mockLogger(), - ); }); test('returns correct ranges when hourly data is enabled and max history is unlimited', () => { licenseMock.getInsightsMaxHistory.mockReturnValue(-1); licenseMock.isInsightsHourlyDataLicensed.mockReturnValue(true); - const result = insightsService.getAvailableDateRanges(); + const result = getAvailableDateRanges(licenseMock); expect(result).toEqual([ { key: 'day', licensed: true, granularity: 'hour' }, @@ -618,7 +610,7 @@ describe('getAvailableDateRanges', () => { licenseMock.getInsightsMaxHistory.mockReturnValue(365); licenseMock.isInsightsHourlyDataLicensed.mockReturnValue(true); - const result = insightsService.getAvailableDateRanges(); + const result = getAvailableDateRanges(licenseMock); expect(result).toEqual([ { key: 'day', licensed: true, granularity: 'hour' }, @@ -635,7 +627,7 @@ describe('getAvailableDateRanges', () => { licenseMock.getInsightsMaxHistory.mockReturnValue(30); licenseMock.isInsightsHourlyDataLicensed.mockReturnValue(false); - const result = insightsService.getAvailableDateRanges(); + const result = getAvailableDateRanges(licenseMock); expect(result).toEqual([ { key: 'day', licensed: false, granularity: 'hour' }, @@ -652,7 +644,7 @@ describe('getAvailableDateRanges', () => { licenseMock.getInsightsMaxHistory.mockReturnValue(5); licenseMock.isInsightsHourlyDataLicensed.mockReturnValue(false); - const result = insightsService.getAvailableDateRanges(); + const result = getAvailableDateRanges(licenseMock); expect(result).toEqual([ { key: 'day', licensed: false, granularity: 'hour' }, @@ -669,7 +661,7 @@ describe('getAvailableDateRanges', () => { licenseMock.getInsightsMaxHistory.mockReturnValue(90); licenseMock.isInsightsHourlyDataLicensed.mockReturnValue(true); - const result = insightsService.getAvailableDateRanges(); + const result = getAvailableDateRanges(licenseMock); expect(result).toEqual([ { key: 'day', licensed: true, granularity: 'hour' }, diff --git a/packages/cli/src/modules/insights/database/entities/__tests__/insights-by-period.test.ts b/packages/cli/src/modules/insights/database/entities/__tests__/insights-by-period.test.ts index 6d8c43c8cf..b8c3fd4b58 100644 --- a/packages/cli/src/modules/insights/database/entities/__tests__/insights-by-period.test.ts +++ b/packages/cli/src/modules/insights/database/entities/__tests__/insights-by-period.test.ts @@ -1,6 +1,16 @@ +import * as testDb from '@test-integration/test-db'; + import { InsightsByPeriod } from '../insights-by-period'; import type { PeriodUnit, TypeUnit } from '../insights-shared'; +beforeAll(async () => { + await testDb.init(); +}); + +afterAll(async () => { + await testDb.terminate(); +}); + describe('Insights By Period', () => { test.each(['time_saved_min', 'runtime_ms', 'failure', 'success'] satisfies TypeUnit[])( '`%s` can be serialized and deserialized correctly', diff --git a/packages/cli/src/modules/insights/database/entities/__tests__/insights-raw.test.ts b/packages/cli/src/modules/insights/database/entities/__tests__/insights-raw.test.ts index fd29a09415..46cfd73a5e 100644 --- a/packages/cli/src/modules/insights/database/entities/__tests__/insights-raw.test.ts +++ b/packages/cli/src/modules/insights/database/entities/__tests__/insights-raw.test.ts @@ -4,6 +4,7 @@ import { DateTime } from 'luxon'; import { createTeamProject } from '@test-integration/db/projects'; import { createWorkflow } from '@test-integration/db/workflows'; import * as testDb from '@test-integration/test-db'; +import * as testModules from '@test-integration/test-modules'; import { createMetadata, createRawInsightsEvent } from './db-utils'; import { InsightsRawRepository } from '../../repositories/insights-raw.repository'; @@ -13,6 +14,7 @@ import type { TypeUnit } from '../insights-shared'; let insightsRawRepository: InsightsRawRepository; beforeAll(async () => { + await testModules.load(['insights']); await testDb.init(); insightsRawRepository = Container.get(InsightsRawRepository); }); diff --git a/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.test.ts b/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.test.ts index d5f5a183bc..8108fb6333 100644 --- a/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.test.ts +++ b/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.test.ts @@ -5,12 +5,14 @@ import { InsightsConfig } from '@/modules/insights/insights.config'; import { createTeamProject } from '@test-integration/db/projects'; import { createWorkflow } from '@test-integration/db/workflows'; import * as testDb from '@test-integration/test-db'; +import * as testModules from '@test-integration/test-modules'; import { createCompactedInsightsEvent, createMetadata } from '../../entities/__tests__/db-utils'; import { InsightsByPeriodRepository } from '../insights-by-period.repository'; describe('InsightsByPeriodRepository', () => { beforeAll(async () => { + await testModules.load(['insights']); await testDb.init(); }); diff --git a/packages/cli/src/modules/insights/insights-helpers.ts b/packages/cli/src/modules/insights/insights-helpers.ts new file mode 100644 index 0000000000..16a1df91f8 --- /dev/null +++ b/packages/cli/src/modules/insights/insights-helpers.ts @@ -0,0 +1,31 @@ +import { type InsightsDateRange, INSIGHTS_DATE_RANGE_KEYS } from '@n8n/api-types'; +import type { LicenseState } from '@n8n/backend-common'; + +export const keyRangeToDays: Record = { + day: 1, + week: 7, + '2weeks': 14, + month: 30, + quarter: 90, + '6months': 180, + year: 365, +}; + +/** + * Returns the available date ranges with their license authorization and time granularity + * when grouped by time. + */ +export function getAvailableDateRanges(licenseState: LicenseState): InsightsDateRange[] { + const maxHistoryInDays = + licenseState.getInsightsMaxHistory() === -1 + ? Number.MAX_SAFE_INTEGER + : licenseState.getInsightsMaxHistory(); + const isHourlyDateLicensed = licenseState.isInsightsHourlyDataLicensed(); + + return INSIGHTS_DATE_RANGE_KEYS.map((key) => ({ + key, + licensed: + key === 'day' ? (isHourlyDateLicensed ?? false) : maxHistoryInDays >= keyRangeToDays[key], + granularity: key === 'day' ? 'hour' : keyRangeToDays[key] <= 30 ? 'day' : 'week', + })); +} diff --git a/packages/cli/src/modules/insights/insights.module.ts b/packages/cli/src/modules/insights/insights.module.ts index 2b25715f57..4ea7b18cb0 100644 --- a/packages/cli/src/modules/insights/insights.module.ts +++ b/packages/cli/src/modules/insights/insights.module.ts @@ -1,15 +1,31 @@ -import type { BaseN8nModule } from '@n8n/decorators'; -import { N8nModule } from '@n8n/decorators'; - -import { InsightsService } from './insights.service'; - +import type { ModuleInterface } from '@n8n/decorators'; +import { BackendModule } from '@n8n/decorators'; +import { Container } from '@n8n/di'; import './insights.controller'; +import { InstanceSettings } from 'n8n-core'; -@N8nModule() -export class InsightsModule implements BaseN8nModule { - constructor(private readonly insightsService: InsightsService) {} +import { InsightsByPeriod } from './database/entities/insights-by-period'; +import { InsightsMetadata } from './database/entities/insights-metadata'; +import { InsightsRaw } from './database/entities/insights-raw'; - initialize() { - this.insightsService.startTimers(); +@BackendModule() +export class InsightsModule implements ModuleInterface { + async init() { + const { instanceType } = Container.get(InstanceSettings); + + /** + * Only main- and webhook-type instances collect insights because + * only they are informed of finished workflow executions. + */ + if (instanceType === 'worker') return; + + await import('./insights.controller'); + + const { InsightsService } = await import('./insights.service'); + Container.get(InsightsService).startTimers(); + } + + entities() { + return [InsightsByPeriod, InsightsMetadata, InsightsRaw]; } } diff --git a/packages/cli/src/modules/insights/insights.pre-init.ts b/packages/cli/src/modules/insights/insights.pre-init.ts deleted file mode 100644 index a8a6267e7a..0000000000 --- a/packages/cli/src/modules/insights/insights.pre-init.ts +++ /dev/null @@ -1,6 +0,0 @@ -import type { ModulePreInitContext } from '../modules.config'; - -export const shouldLoadModule = (ctx: ModulePreInitContext) => - // Only main and webhook instance(s) should collect insights - // Because main and webhooks instances are the ones informed of all finished workflow executions, whatever the mode - ctx.instance.instanceType === 'main' || ctx.instance.instanceType === 'webhook'; diff --git a/packages/cli/src/modules/insights/insights.service.ts b/packages/cli/src/modules/insights/insights.service.ts index 291444c4c8..8c3535d245 100644 --- a/packages/cli/src/modules/insights/insights.service.ts +++ b/packages/cli/src/modules/insights/insights.service.ts @@ -1,8 +1,4 @@ -import { - type InsightsSummary, - type InsightsDateRange, - INSIGHTS_DATE_RANGE_KEYS, -} from '@n8n/api-types'; +import { type InsightsSummary, type InsightsDateRange } from '@n8n/api-types'; import { LicenseState, Logger } from '@n8n/backend-common'; import { OnLeaderStepdown, OnLeaderTakeover, OnShutdown } from '@n8n/decorators'; import { Service } from '@n8n/di'; @@ -14,18 +10,9 @@ import { NumberToType } from './database/entities/insights-shared'; import { InsightsByPeriodRepository } from './database/repositories/insights-by-period.repository'; import { InsightsCollectionService } from './insights-collection.service'; import { InsightsCompactionService } from './insights-compaction.service'; +import { getAvailableDateRanges, keyRangeToDays } from './insights-helpers'; import { InsightsPruningService } from './insights-pruning.service'; -const keyRangeToDays: Record = { - day: 1, - week: 7, - '2weeks': 14, - month: 30, - quarter: 90, - '6months': 180, - year: 365, -}; - @Service() export class InsightsService { constructor( @@ -40,13 +27,11 @@ export class InsightsService { this.logger = this.logger.scoped('insights'); } + @OnLeaderTakeover() startTimers() { this.collectionService.startFlushingTimer(); - // Start compaction and pruning timers for main leader instance only - if (this.instanceSettings.isLeader) { - this.startCompactionAndPruningTimers(); - } + if (this.instanceSettings.isLeader) this.startCompactionAndPruningTimers(); } @OnLeaderTakeover() @@ -204,31 +189,12 @@ export class InsightsService { }); } - /** - * Returns the available date ranges with their license authorization and time granularity - * when grouped by time. - */ - getAvailableDateRanges(): InsightsDateRange[] { - const maxHistoryInDays = - this.licenseState.getInsightsMaxHistory() === -1 - ? Number.MAX_SAFE_INTEGER - : this.licenseState.getInsightsMaxHistory(); - const isHourlyDateLicensed = this.licenseState.isInsightsHourlyDataLicensed(); - - return INSIGHTS_DATE_RANGE_KEYS.map((key) => ({ - key, - licensed: - key === 'day' ? (isHourlyDateLicensed ?? false) : maxHistoryInDays >= keyRangeToDays[key], - granularity: key === 'day' ? 'hour' : keyRangeToDays[key] <= 30 ? 'day' : 'week', - })); - } - getMaxAgeInDaysAndGranularity( dateRangeKey: InsightsDateRange['key'], ): InsightsDateRange & { maxAgeInDays: number } { - const availableDateRanges = this.getAvailableDateRanges(); - - const dateRange = availableDateRanges.find((range) => range.key === dateRangeKey); + const dateRange = getAvailableDateRanges(this.licenseState).find( + (range) => range.key === dateRangeKey, + ); if (!dateRange) { // Not supposed to happen if we trust the dateRangeKey type throw new UserError('The selected date range is not available'); diff --git a/packages/cli/src/modules/module-registry.ts b/packages/cli/src/modules/module-registry.ts index bd41158c59..3baf546cbf 100644 --- a/packages/cli/src/modules/module-registry.ts +++ b/packages/cli/src/modules/module-registry.ts @@ -1,5 +1,5 @@ -import type { LifecycleContext } from '@n8n/decorators'; import { LifecycleMetadata, ModuleMetadata } from '@n8n/decorators'; +import type { LifecycleContext, EntityClass } from '@n8n/decorators'; import { Container, Service } from '@n8n/di'; import type { ExecutionLifecycleHooks } from 'n8n-core'; import type { @@ -14,14 +14,26 @@ import type { @Service() export class ModuleRegistry { + readonly entities: EntityClass[] = []; + constructor( private readonly moduleMetadata: ModuleMetadata, private readonly lifecycleMetadata: LifecycleMetadata, ) {} - async initializeModules() { + async initModules() { for (const ModuleClass of this.moduleMetadata.getModules()) { - await Container.get(ModuleClass).initialize?.(); + await Container.get(ModuleClass).init?.(); + } + } + + addEntities() { + for (const ModuleClass of this.moduleMetadata.getModules()) { + const entities = Container.get(ModuleClass).entities?.(); + + if (!entities || entities.length === 0) continue; + + this.entities.push(...entities); } } diff --git a/packages/cli/src/modules/modules.config.ts b/packages/cli/src/modules/modules.config.ts index 6d3fa5e3b8..029c588e2f 100644 --- a/packages/cli/src/modules/modules.config.ts +++ b/packages/cli/src/modules/modules.config.ts @@ -10,7 +10,7 @@ export type ModulePreInit = { shouldLoadModule: (ctx: ModulePreInitContext) => boolean; }; -const moduleNames = ['insights', 'external-secrets.ee'] as const; +const moduleNames = ['insights', 'external-secrets'] as const; export type ModuleName = (typeof moduleNames)[number]; class Modules extends CommaSeparatedStringArray { @@ -36,7 +36,7 @@ export class ModulesConfig { disabledModules: Modules = []; // Default modules are always enabled unless explicitly disabled - private readonly defaultModules: ModuleName[] = ['insights', 'external-secrets.ee']; + private readonly defaultModules: ModuleName[] = ['insights', 'external-secrets']; // Loaded modules are the ones that have been loaded so far by the instance readonly loadedModules = new Set(); diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 65658e4488..6903715fe1 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -17,7 +17,7 @@ import { CredentialsOverwrites } from '@/credentials-overwrites'; import { getLdapLoginLabel } from '@/ldap.ee/helpers.ee'; import { License } from '@/license'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; -import { InsightsService } from '@/modules/insights/insights.service'; +import { getAvailableDateRanges as getInsightsAvailableDateRanges } from '@/modules/insights/insights-helpers'; import { ModulesConfig } from '@/modules/modules.config'; import { isApiEnabled } from '@/public-api'; import { PushConfig } from '@/push/push.config'; @@ -52,7 +52,6 @@ export class FrontendService { private readonly modulesConfig: ModulesConfig, private readonly pushConfig: PushConfig, private readonly binaryDataConfig: BinaryDataConfig, - private readonly insightsService: InsightsService, private readonly licenseState: LicenseState, ) { loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes()); @@ -380,7 +379,7 @@ export class FrontendService { enabled: this.modulesConfig.loadedModules.has('insights'), summary: this.licenseState.isInsightsSummaryLicensed(), dashboard: this.licenseState.isInsightsDashboardLicensed(), - dateRanges: this.insightsService.getAvailableDateRanges(), + dateRanges: getInsightsAvailableDateRanges(this.licenseState), }); this.settings.mfa.enabled = config.get('mfa.enabled'); diff --git a/packages/cli/test/integration/external-secrets/external-secrets.api.test.ts b/packages/cli/test/integration/external-secrets/external-secrets.api.test.ts index ef549db13a..aa4eaba7a9 100644 --- a/packages/cli/test/integration/external-secrets/external-secrets.api.test.ts +++ b/packages/cli/test/integration/external-secrets/external-secrets.api.test.ts @@ -35,6 +35,7 @@ mockInstance(ExternalSecretsProviders, mockProvidersInstance); const testServer = setupTestServer({ endpointGroups: ['externalSecrets'], enabledFeatures: ['feat:externalSecrets'], + modules: ['external-secrets'], }); const connectedDate = '2023-08-01T12:32:29.000Z'; diff --git a/packages/cli/test/integration/insights/insights.api.test.ts b/packages/cli/test/integration/insights/insights.api.test.ts index 7c627bf91f..47a2414eb9 100644 --- a/packages/cli/test/integration/insights/insights.api.test.ts +++ b/packages/cli/test/integration/insights/insights.api.test.ts @@ -14,6 +14,7 @@ const testServer = utils.setupTestServer({ endpointGroups: ['insights', 'license', 'auth'], enabledFeatures: ['feat:insights:viewSummary', 'feat:insights:viewDashboard'], quotas: { 'quota:insights:maxHistoryDays': 365 }, + modules: ['insights'], }); beforeAll(async () => { diff --git a/packages/cli/test/integration/shared/test-db.ts b/packages/cli/test/integration/shared/test-db.ts index 4845a475ff..eb7299d9c6 100644 --- a/packages/cli/test/integration/shared/test-db.ts +++ b/packages/cli/test/integration/shared/test-db.ts @@ -7,6 +7,7 @@ import { randomString } from 'n8n-workflow'; import { DbConnection } from '@/databases/db-connection'; import { DbConnectionOptions } from '@/databases/db-connection-options'; +import { ModuleRegistry } from '@/modules/module-registry'; export const testDbPrefix = 'n8n_test_'; @@ -37,6 +38,8 @@ export async function init() { const dbConnection = Container.get(DbConnection); await dbConnection.init(); await dbConnection.migrate(); + + await Container.get(ModuleRegistry).initModules(); } export function isReady() { diff --git a/packages/cli/test/integration/shared/test-modules.ts b/packages/cli/test/integration/shared/test-modules.ts new file mode 100644 index 0000000000..354d7e4847 --- /dev/null +++ b/packages/cli/test/integration/shared/test-modules.ts @@ -0,0 +1,15 @@ +import { Container } from '@n8n/di'; + +import { ModuleRegistry } from '@/modules/module-registry'; + +export async function load(moduleNames: string[]) { + for (const moduleName of moduleNames) { + try { + await import(`../../../src/modules/${moduleName}/${moduleName}.module`); + } catch { + await import(`../../../src/modules/${moduleName}.ee/${moduleName}.module`); + } + } + + Container.get(ModuleRegistry).addEntities(); +} diff --git a/packages/cli/test/integration/shared/types.ts b/packages/cli/test/integration/shared/types.ts index 8eb2d1aa41..5bc3734ffa 100644 --- a/packages/cli/test/integration/shared/types.ts +++ b/packages/cli/test/integration/shared/types.ts @@ -47,10 +47,13 @@ type EndpointGroup = | 'folder' | 'insights'; +type ModuleName = 'insights' | 'external-secrets'; + export interface SetupProps { endpointGroups?: EndpointGroup[]; enabledFeatures?: BooleanLicenseFeature[]; quotas?: Partial<{ [K in NumericLicenseFeature]: number }>; + modules?: ModuleName[]; } export type SuperAgentTest = TestAgent; diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index 6084402216..fa3b93251d 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -17,6 +17,7 @@ import { PostHogClient } from '@/posthog'; import { Push } from '@/push'; import type { APIRequest } from '@/requests'; import { Telemetry } from '@/telemetry'; +import * as testModules from '@test-integration/test-modules'; import { mockInstance, mockLogger } from '../../../shared/mocking'; import { PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants'; @@ -91,6 +92,7 @@ export const setupTestServer = ({ endpointGroups, enabledFeatures, quotas, + modules, }: SetupProps): TestServer => { const app = express(); app.use(rawBodyReader); @@ -120,6 +122,7 @@ export const setupTestServer = ({ // eslint-disable-next-line complexity beforeAll(async () => { + if (modules) await testModules.load(modules); await testDb.init(); config.set('userManagement.jwtSecret', 'My JWT secret'); @@ -289,7 +292,7 @@ export const setupTestServer = ({ await import('@/controllers/folder.controller'); case 'externalSecrets': - await import('@/modules/external-secrets.ee/external-secrets.ee.module'); + await import('@/modules/external-secrets.ee/external-secrets.module'); case 'insights': await import('@/modules/insights/insights.module');