From bb7c68f6bfee83116f4d63e015dc18222c59089a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 19 Jun 2025 14:32:31 +0200 Subject: [PATCH] refactor(core): Centralize module management (#16464) Co-authored-by: Danny Martini --- .../@n8n/api-types/src/frontend-settings.ts | 4 +- packages/cli/src/commands/base-command.ts | 14 +- .../modules/__tests__/module-registry.test.ts | 186 ++++++++++++++++++ .../modules/__tests__/modules.config.test.ts | 61 ++---- .../modules/errors/missing-module.error.ts | 9 + .../modules/errors/module-confusion.error.ts | 11 ++ .../modules/errors/unknown-module.error.ts | 7 + .../insights-collection.service.test.ts | 2 +- .../insights-compaction.service.test.ts | 2 +- .../insights-pruning.service.test.ts | 2 +- .../__tests__/insights.service.test.ts | 2 +- .../entities/__tests__/insights-raw.test.ts | 2 +- .../insights-by-period.repository.test.ts | 2 +- packages/cli/src/modules/module-registry.ts | 77 +++++++- packages/cli/src/modules/modules.config.ts | 48 +---- packages/cli/src/services/frontend.service.ts | 4 +- .../test/integration/shared/test-modules.ts | 13 +- .../integration/shared/utils/test-server.ts | 2 +- .../editor-ui/src/__tests__/defaults.ts | 2 +- .../editor-ui/src/components/MainSidebar.vue | 2 +- .../src/features/insights/insights.store.ts | 2 +- .../editor-ui/src/stores/settings.store.ts | 4 +- .../editor-ui/src/views/SigninView.test.ts | 2 +- .../editor-ui/src/views/SigninView.vue | 2 +- 24 files changed, 328 insertions(+), 134 deletions(-) create mode 100644 packages/cli/src/modules/__tests__/module-registry.test.ts create mode 100644 packages/cli/src/modules/errors/missing-module.error.ts create mode 100644 packages/cli/src/modules/errors/module-confusion.error.ts create mode 100644 packages/cli/src/modules/errors/unknown-module.error.ts diff --git a/packages/@n8n/api-types/src/frontend-settings.ts b/packages/@n8n/api-types/src/frontend-settings.ts index bbb98e5569..dfd0d2c233 100644 --- a/packages/@n8n/api-types/src/frontend-settings.ts +++ b/packages/@n8n/api-types/src/frontend-settings.ts @@ -196,8 +196,8 @@ export interface FrontendSettings { quota: number; }; - /** Backend modules that were loaded during startup based on user configuration and pre-init check. */ - loadedModules: string[]; + /** Backend modules that were initialized during startup. */ + activeModules: string[]; } export type FrontendModuleSettings = { diff --git a/packages/cli/src/commands/base-command.ts b/packages/cli/src/commands/base-command.ts index 834d600a48..4e24757fbf 100644 --- a/packages/cli/src/commands/base-command.ts +++ b/packages/cli/src/commands/base-command.ts @@ -72,19 +72,7 @@ export abstract class BaseCommand extends Command { protected needsTaskRunner = false; protected async loadModules() { - for (const moduleName of this.modulesConfig.modules) { - // add module to the registry for dependency injection - try { - await import(`../modules/${moduleName}/${moduleName}.module`); - } catch { - await import(`../modules/${moduleName}.ee/${moduleName}.module`); - } - - this.modulesConfig.addLoadedModule(moduleName); - this.logger.debug(`Loaded module "${moduleName}"`); - } - - this.moduleRegistry.addEntities(); + await this.moduleRegistry.loadModules(); } async init(): Promise { diff --git a/packages/cli/src/modules/__tests__/module-registry.test.ts b/packages/cli/src/modules/__tests__/module-registry.test.ts new file mode 100644 index 0000000000..5c00316d39 --- /dev/null +++ b/packages/cli/src/modules/__tests__/module-registry.test.ts @@ -0,0 +1,186 @@ +import type { LicenseState } from '@n8n/backend-common'; +import type { ModuleInterface, ModuleMetadata } from '@n8n/decorators'; +import { Container } from '@n8n/di'; +import { mock } from 'jest-mock-extended'; + +import { ModuleConfusionError } from '../errors/module-confusion.error'; +import { ModuleRegistry } from '../module-registry'; +import { MODULE_NAMES } from '../modules.config'; + +beforeEach(() => { + jest.resetAllMocks(); + process.env = {}; + Container.reset(); +}); + +describe('eligibleModules', () => { + it('should consider all default modules eligible', () => { + expect(Container.get(ModuleRegistry).eligibleModules).toEqual(MODULE_NAMES); + }); + + it('should consider a module ineligible if it was disabled via env var', () => { + process.env.N8N_DISABLED_MODULES = 'insights'; + expect(Container.get(ModuleRegistry).eligibleModules).toEqual(['external-secrets']); + }); + + it('should throw `ModuleConfusionError` if a module is both enabled and disabled', () => { + process.env.N8N_ENABLED_MODULES = 'insights'; + process.env.N8N_DISABLED_MODULES = 'insights'; + expect(() => Container.get(ModuleRegistry).eligibleModules).toThrow(ModuleConfusionError); + }); +}); + +describe('loadModules', () => { + it('should load entities defined by modules', async () => { + const FirstEntity = class FirstEntityClass {}; + const SecondEntity = class SecondEntityClass {}; + + const ModuleClass = { + entities: jest.fn().mockReturnValue([FirstEntity, SecondEntity]), + }; + const moduleMetadata = mock({ + getClasses: jest.fn().mockReturnValue([ModuleClass]), + }); + + Container.get = jest.fn().mockReturnValue(ModuleClass); + const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock()); + + await moduleRegistry.loadModules([]); + + expect(moduleRegistry.entities).toEqual([FirstEntity, SecondEntity]); + }); + + it('should load no entities if none are defined by modules', async () => { + const ModuleClass = { entities: jest.fn().mockReturnValue([]) }; + const moduleMetadata = mock({ + getClasses: jest.fn().mockReturnValue([ModuleClass]), + }); + + Container.get = jest.fn().mockReturnValue(ModuleClass); + const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock()); + + await moduleRegistry.loadModules([]); + + expect(moduleRegistry.entities).toEqual([]); + }); +}); + +describe('initModules', () => { + it('should init module if it has no feature flag', async () => { + const ModuleClass = { init: jest.fn() }; + const moduleMetadata = mock({ + getEntries: jest + .fn() + .mockReturnValue([['test-module', { licenseFlag: undefined, class: ModuleClass }]]), + }); + Container.get = jest.fn().mockReturnValue(ModuleClass); + + const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock()); + + await moduleRegistry.initModules(); + + expect(ModuleClass.init).toHaveBeenCalled(); + }); + + it('should init module if it is licensed', async () => { + const ModuleClass = { init: jest.fn() }; + const moduleMetadata = mock({ + getEntries: jest + .fn() + .mockReturnValue([ + ['test-module', { licenseFlag: 'feat:testFeature', class: ModuleClass }], + ]), + }); + const licenseState = mock({ isLicensed: jest.fn().mockReturnValue(true) }); + Container.get = jest.fn().mockReturnValue(ModuleClass); + + const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), licenseState, mock(), mock()); + + await moduleRegistry.initModules(); + + expect(ModuleClass.init).toHaveBeenCalled(); + }); + + it('should skip init for unlicensed module', async () => { + const ModuleClass = { init: jest.fn() }; + const moduleMetadata = mock({ + getEntries: jest + .fn() + .mockReturnValue([ + ['test-module', { licenseFlag: 'feat:testFeature', class: ModuleClass }], + ]), + }); + const licenseState = mock({ isLicensed: jest.fn().mockReturnValue(false) }); + Container.get = jest.fn().mockReturnValue(ModuleClass); + + const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), licenseState, mock(), mock()); + + await moduleRegistry.initModules(); + + expect(ModuleClass.init).not.toHaveBeenCalled(); + }); + + it('should accept module without `init` method', async () => { + const ModuleClass = {}; + const moduleMetadata = mock({ + getEntries: jest + .fn() + .mockReturnValue([['test-module', { licenseFlag: undefined, class: ModuleClass }]]), + }); + + Container.get = jest.fn().mockReturnValue(ModuleClass); + + const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock()); + + await moduleRegistry.initModules(); + + await expect(moduleRegistry.initModules()).resolves.not.toThrow(); + }); + + it('registers settings', async () => { + // ARRANGE + const moduleName = 'test-module'; + const moduleSettings = { foo: 1 }; + const ModuleClass: ModuleInterface = { + init: jest.fn(), + settings: jest.fn().mockReturnValue(moduleSettings), + }; + const moduleMetadata = mock({ + getEntries: jest.fn().mockReturnValue([[moduleName, { class: ModuleClass }]]), + }); + Container.get = jest.fn().mockReturnValue(ModuleClass); + + const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock()); + + // ACT + await moduleRegistry.initModules(); + + // ASSERT + expect(ModuleClass.settings).toHaveBeenCalled(); + expect(moduleRegistry.settings.has(moduleName)).toBe(true); + expect(moduleRegistry.settings.get(moduleName)).toBe(moduleSettings); + }); + + it('activates the module', async () => { + // ARRANGE + const moduleName = 'test-module'; + const moduleSettings = { foo: 1 }; + const ModuleClass: ModuleInterface = { + init: jest.fn(), + settings: jest.fn().mockReturnValue(moduleSettings), + }; + const moduleMetadata = mock({ + getEntries: jest.fn().mockReturnValue([[moduleName, { class: ModuleClass }]]), + }); + Container.get = jest.fn().mockReturnValue(ModuleClass); + + const moduleRegistry = new ModuleRegistry(moduleMetadata, mock(), mock(), mock(), mock()); + + // ACT + await moduleRegistry.initModules(); + + // ASSERT + expect(moduleRegistry.isActive(moduleName as any)).toBe(true); + expect(moduleRegistry.getActiveModules()).toEqual([moduleName]); + }); +}); diff --git a/packages/cli/src/modules/__tests__/modules.config.test.ts b/packages/cli/src/modules/__tests__/modules.config.test.ts index a13aebb63d..ec011fb4aa 100644 --- a/packages/cli/src/modules/__tests__/modules.config.test.ts +++ b/packages/cli/src/modules/__tests__/modules.config.test.ts @@ -1,51 +1,20 @@ import { Container } from '@n8n/di'; -import { UnexpectedError } from 'n8n-workflow'; +import { UnknownModuleError } from '../errors/unknown-module.error'; import { ModulesConfig } from '../modules.config'; -describe('ModulesConfig', () => { - beforeEach(() => { - jest.resetAllMocks(); - process.env = {}; - Container.reset(); - }); - - it('should initialize with insights modules if no environment variable is set', () => { - const config = Container.get(ModulesConfig); - 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']); - }); - - 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']); - }); - - it('should throw UnexpectedError for invalid module names', () => { - process.env.N8N_ENABLED_MODULES = 'invalidModule'; - expect(() => Container.get(ModulesConfig)).toThrow(UnexpectedError); - }); - - it('should throw UnexpectedError if any module is both enabled and disabled', () => { - process.env.N8N_ENABLED_MODULES = 'insights'; - process.env.N8N_DISABLED_MODULES = 'insights'; - const config = Container.get(ModulesConfig); - expect(() => config.modules).toThrow(UnexpectedError); - }); - - it('should throw UnexpectedError if any enabled module name is invalid', () => { - process.env.N8N_ENABLED_MODULES = 'insights,invalidModule'; - expect(() => Container.get(ModulesConfig)).toThrow(UnexpectedError); - }); - - it('should throw UnexpectedError if any disabled module name is invalid', () => { - process.env.N8N_DISABLED_MODULES = 'insights,invalidModule'; - expect(() => Container.get(ModulesConfig)).toThrow(UnexpectedError); - }); +beforeEach(() => { + jest.resetAllMocks(); + process.env = {}; + Container.reset(); +}); + +it('should throw `UnknownModuleError` if any enabled module name is invalid', () => { + process.env.N8N_ENABLED_MODULES = 'insights,invalidModule'; + expect(() => Container.get(ModulesConfig)).toThrowError(UnknownModuleError); +}); + +it('should throw `UnknownModuleError` if any disabled module name is invalid', () => { + process.env.N8N_DISABLED_MODULES = 'insights,invalidModule'; + expect(() => Container.get(ModulesConfig)).toThrowError(UnknownModuleError); }); diff --git a/packages/cli/src/modules/errors/missing-module.error.ts b/packages/cli/src/modules/errors/missing-module.error.ts new file mode 100644 index 0000000000..fad8958be1 --- /dev/null +++ b/packages/cli/src/modules/errors/missing-module.error.ts @@ -0,0 +1,9 @@ +import { UserError } from 'n8n-workflow'; + +export class MissingModuleError extends UserError { + constructor(moduleName: string, errorMsg: string) { + super( + `Failed to load module "${moduleName}": ${errorMsg}. Please review the module's entrypoint file name and the module's directory name.`, + ); + } +} diff --git a/packages/cli/src/modules/errors/module-confusion.error.ts b/packages/cli/src/modules/errors/module-confusion.error.ts new file mode 100644 index 0000000000..9930294e33 --- /dev/null +++ b/packages/cli/src/modules/errors/module-confusion.error.ts @@ -0,0 +1,11 @@ +import { UserError } from 'n8n-workflow'; + +export class ModuleConfusionError extends UserError { + constructor(moduleNames: string[]) { + const modules = moduleNames.length > 1 ? 'modules' : 'a module'; + + super( + `Found ${modules} listed in both \`N8N_ENABLED_MODULES\` and \`N8N_DISABLED_MODULES\`: ${moduleNames.join(', ')}. Please review your environment variables, as a module cannot be both enabled and disabled.`, + ); + } +} diff --git a/packages/cli/src/modules/errors/unknown-module.error.ts b/packages/cli/src/modules/errors/unknown-module.error.ts new file mode 100644 index 0000000000..d80f09face --- /dev/null +++ b/packages/cli/src/modules/errors/unknown-module.error.ts @@ -0,0 +1,7 @@ +import { UnexpectedError } from 'n8n-workflow'; + +export class UnknownModuleError extends UnexpectedError { + constructor(moduleName: string) { + super(`Unknown module "${moduleName}"`, { level: 'fatal' }); + } +} 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 e42eb1c3b0..dee96f3625 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 @@ -27,7 +27,7 @@ import { InsightsCollectionService } from '../insights-collection.service'; import { InsightsConfig } from '../insights.config'; beforeAll(async () => { - await testModules.load(['insights']); + await testModules.loadModules(['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 5d4ec77195..bb68c65681 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 @@ -20,7 +20,7 @@ import { InsightsCompactionService } from '../insights-compaction.service'; import { InsightsConfig } from '../insights.config'; beforeAll(async () => { - await testModules.load(['insights']); + await testModules.loadModules(['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 98dfc0684c..2bb607572d 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 @@ -19,7 +19,7 @@ import { InsightsPruningService } from '../insights-pruning.service'; import { InsightsConfig } from '../insights.config'; beforeAll(async () => { - await testModules.load(['insights']); + await testModules.loadModules(['insights']); await testDb.init(); }); 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 72e5ba5510..ba9b82cd72 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.service.test.ts @@ -31,7 +31,7 @@ import { InsightsConfig } from '../insights.config'; import { InsightsService } from '../insights.service'; beforeAll(async () => { - await testModules.load(['insights']); + await testModules.loadModules(['insights']); await testDb.init(); }); 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 46cfd73a5e..1f19a4c46a 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 @@ -14,7 +14,7 @@ import type { TypeUnit } from '../insights-shared'; let insightsRawRepository: InsightsRawRepository; beforeAll(async () => { - await testModules.load(['insights']); + await testModules.loadModules(['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 8108fb6333..57f5a7d72d 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 @@ -12,7 +12,7 @@ import { InsightsByPeriodRepository } from '../insights-by-period.repository'; describe('InsightsByPeriodRepository', () => { beforeAll(async () => { - await testModules.load(['insights']); + await testModules.loadModules(['insights']); await testDb.init(); }); diff --git a/packages/cli/src/modules/module-registry.ts b/packages/cli/src/modules/module-registry.ts index 3609f3a6d5..e61fceae3e 100644 --- a/packages/cli/src/modules/module-registry.ts +++ b/packages/cli/src/modules/module-registry.ts @@ -13,6 +13,11 @@ import type { Workflow, } from 'n8n-workflow'; +import { MissingModuleError } from './errors/missing-module.error'; +import { ModuleConfusionError } from './errors/module-confusion.error'; +import { ModulesConfig } from './modules.config'; +import type { ModuleName } from './modules.config'; + @Service() export class ModuleRegistry { readonly entities: EntityClass[] = []; @@ -24,8 +29,63 @@ export class ModuleRegistry { private readonly lifecycleMetadata: LifecycleMetadata, private readonly licenseState: LicenseState, private readonly logger: Logger, + private readonly modulesConfig: ModulesConfig, ) {} + private readonly defaultModules: ModuleName[] = ['insights', 'external-secrets']; + + private readonly activeModules: string[] = []; + + get eligibleModules(): ModuleName[] { + const { enabledModules, disabledModules } = this.modulesConfig; + + const doubleListed = enabledModules.filter((m) => disabledModules.includes(m)); + + if (doubleListed.length > 0) throw new ModuleConfusionError(doubleListed); + + const defaultPlusEnabled = [...new Set([...this.defaultModules, ...enabledModules])]; + + return defaultPlusEnabled.filter((m) => !disabledModules.includes(m)); + } + + /** + * Loads [module name].module.ts for each eligible module. + * This only registers the database entities for module and should be done + * before instantiating the datasource. + * + * This will not register routes or do any other kind of module related + * setup. + */ + async loadModules(modules?: ModuleName[]) { + for (const moduleName of modules ?? this.eligibleModules) { + try { + await import(`../modules/${moduleName}/${moduleName}.module`); + } catch { + try { + await import(`../modules/${moduleName}.ee/${moduleName}.module`); + } catch (error) { + throw new MissingModuleError(moduleName, error instanceof Error ? error.message : ''); + } + } + } + + for (const ModuleClass of this.moduleMetadata.getClasses()) { + const entities = Container.get(ModuleClass).entities?.(); + + if (!entities || entities.length === 0) continue; + + this.entities.push(...entities); + } + } + + /** + * Calls `init` on each eligible module. + * + * This will do things like registering routes, setup timers or other module + * specific setup. + * + * `ModuleRegistry.loadModules` must have been called before. + */ async initModules() { for (const [moduleName, moduleEntry] of this.moduleMetadata.getEntries()) { const { licenseFlag, class: ModuleClass } = moduleEntry; @@ -34,6 +94,7 @@ export class ModuleRegistry { this.logger.debug(`Skipped init for unlicensed module "${moduleName}"`); continue; } + await Container.get(ModuleClass).init?.(); const moduleSettings = await Container.get(ModuleClass).settings?.(); @@ -41,17 +102,19 @@ export class ModuleRegistry { if (!moduleSettings) continue; this.settings.set(moduleName, moduleSettings); + + this.logger.debug(`Initialized module "${moduleName}"`); + + this.activeModules.push(moduleName); } } - addEntities() { - for (const ModuleClass of this.moduleMetadata.getClasses()) { - const entities = Container.get(ModuleClass).entities?.(); + isActive(moduleName: ModuleName) { + return this.activeModules.includes(moduleName); + } - if (!entities || entities.length === 0) continue; - - this.entities.push(...entities); - } + getActiveModules() { + return this.activeModules; } registerLifecycleHooks(hooks: ExecutionLifecycleHooks) { diff --git a/packages/cli/src/modules/modules.config.ts b/packages/cli/src/modules/modules.config.ts index 029c588e2f..49f7b343bf 100644 --- a/packages/cli/src/modules/modules.config.ts +++ b/packages/cli/src/modules/modules.config.ts @@ -1,58 +1,28 @@ import { CommaSeparatedStringArray, Config, Env } from '@n8n/config'; -import type { InstanceSettings } from 'n8n-core'; -import { UnexpectedError } from 'n8n-workflow'; -export type ModulePreInitContext = { - instance: InstanceSettings; -}; +import { UnknownModuleError } from './errors/unknown-module.error'; -export type ModulePreInit = { - shouldLoadModule: (ctx: ModulePreInitContext) => boolean; -}; +export const MODULE_NAMES = ['insights', 'external-secrets'] as const; -const moduleNames = ['insights', 'external-secrets'] as const; -export type ModuleName = (typeof moduleNames)[number]; +export type ModuleName = (typeof MODULE_NAMES)[number]; -class Modules extends CommaSeparatedStringArray { +class ModuleArray extends CommaSeparatedStringArray { constructor(str: string) { super(str); for (const moduleName of this) { - if (!moduleNames.includes(moduleName)) { - throw new UnexpectedError(`Unknown module name ${moduleName}`, { level: 'fatal' }); - } + if (!MODULE_NAMES.includes(moduleName)) throw new UnknownModuleError(moduleName); } } } @Config export class ModulesConfig { - /** Comma-separated list of all modules enabled */ + /** Comma-separated list of all enabled modules. */ @Env('N8N_ENABLED_MODULES') - enabledModules: Modules = []; + enabledModules: ModuleArray = []; - /** Comma-separated list of all disabled modules */ + /** Comma-separated list of all disabled modules. */ @Env('N8N_DISABLED_MODULES') - disabledModules: Modules = []; - - // Default modules are always enabled unless explicitly disabled - 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(); - - // Get all modules by merging default and enabled, and filtering out disabled modules - get modules(): ModuleName[] { - if (this.enabledModules.some((module) => this.disabledModules.includes(module))) { - throw new UnexpectedError('Module cannot be both enabled and disabled', { level: 'fatal' }); - } - - const enabledModules = Array.from(new Set(this.defaultModules.concat(this.enabledModules))); - - return enabledModules.filter((module) => !this.disabledModules.includes(module)); - } - - addLoadedModule(module: ModuleName) { - this.loadedModules.add(module); - } + disabledModules: ModuleArray = []; } diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 38be3a8e67..66dff89a23 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -18,7 +18,6 @@ import { getLdapLoginLabel } from '@/ldap.ee/helpers.ee'; import { License } from '@/license'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { ModuleRegistry } from '@/modules/module-registry'; -import { ModulesConfig } from '@/modules/modules.config'; import { isApiEnabled } from '@/public-api'; import { PushConfig } from '@/push/push.config'; import type { CommunityPackagesService } from '@/services/community-packages.service'; @@ -49,7 +48,6 @@ export class FrontendService { private readonly instanceSettings: InstanceSettings, private readonly urlService: UrlService, private readonly securityConfig: SecurityConfig, - private readonly modulesConfig: ModulesConfig, private readonly pushConfig: PushConfig, private readonly binaryDataConfig: BinaryDataConfig, private readonly licenseState: LicenseState, @@ -256,7 +254,7 @@ export class FrontendService { evaluation: { quota: this.licenseState.getMaxWorkflowsWithEvaluations(), }, - loadedModules: this.modulesConfig.modules, + activeModules: this.moduleRegistry.getActiveModules(), }; } diff --git a/packages/cli/test/integration/shared/test-modules.ts b/packages/cli/test/integration/shared/test-modules.ts index 354d7e4847..976bd366ca 100644 --- a/packages/cli/test/integration/shared/test-modules.ts +++ b/packages/cli/test/integration/shared/test-modules.ts @@ -1,15 +1,8 @@ import { Container } from '@n8n/di'; import { ModuleRegistry } from '@/modules/module-registry'; +import type { ModuleName } from '@/modules/modules.config'; -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(); +export async function loadModules(moduleNames: ModuleName[]) { + await Container.get(ModuleRegistry).loadModules(moduleNames); } diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index ea3eee4bbc..78a1e05bee 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -125,7 +125,7 @@ export const setupTestServer = ({ // eslint-disable-next-line complexity beforeAll(async () => { - if (modules) await testModules.load(modules); + if (modules) await testModules.loadModules(modules); await testDb.init(); config.set('userManagement.jwtSecret', 'My JWT secret'); diff --git a/packages/frontend/editor-ui/src/__tests__/defaults.ts b/packages/frontend/editor-ui/src/__tests__/defaults.ts index e8398f4071..7288977f21 100644 --- a/packages/frontend/editor-ui/src/__tests__/defaults.ts +++ b/packages/frontend/editor-ui/src/__tests__/defaults.ts @@ -148,5 +148,5 @@ export const defaultSettings: FrontendSettings = { evaluation: { quota: 0, }, - loadedModules: [], + activeModules: [], }; diff --git a/packages/frontend/editor-ui/src/components/MainSidebar.vue b/packages/frontend/editor-ui/src/components/MainSidebar.vue index 156663bed0..07fc3dda72 100644 --- a/packages/frontend/editor-ui/src/components/MainSidebar.vue +++ b/packages/frontend/editor-ui/src/components/MainSidebar.vue @@ -112,7 +112,7 @@ const mainMenuItems = computed(() => [ position: 'bottom', route: { to: { name: VIEWS.INSIGHTS } }, available: - settingsStore.settings.loadedModules.includes('insights') && + settingsStore.settings.activeModules.includes('insights') && hasPermission(['rbac'], { rbac: { scope: 'insights:list' } }), }, { diff --git a/packages/frontend/editor-ui/src/features/insights/insights.store.ts b/packages/frontend/editor-ui/src/features/insights/insights.store.ts index bc6150164a..3d13b8c680 100644 --- a/packages/frontend/editor-ui/src/features/insights/insights.store.ts +++ b/packages/frontend/editor-ui/src/features/insights/insights.store.ts @@ -19,7 +19,7 @@ export const useInsightsStore = defineStore('insights', () => { ); const isInsightsEnabled = computed(() => - settingsStore.settings.loadedModules.includes('insights'), + settingsStore.settings.activeModules.includes('insights'), ); const isDashboardEnabled = computed( diff --git a/packages/frontend/editor-ui/src/stores/settings.store.ts b/packages/frontend/editor-ui/src/stores/settings.store.ts index 2107160f49..8edae8fbda 100644 --- a/packages/frontend/editor-ui/src/stores/settings.store.ts +++ b/packages/frontend/editor-ui/src/stores/settings.store.ts @@ -185,7 +185,7 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => { const isDevRelease = computed(() => settings.value.releaseChannel === 'dev'); - const loadedModules = computed(() => settings.value.loadedModules); + const activeModules = computed(() => settings.value.activeModules); const setSettings = (newSettings: FrontendSettings) => { settings.value = newSettings; @@ -426,7 +426,7 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => { getSettings, setSettings, initialize, - loadedModules, + activeModules, getModuleSettings, moduleSettings, }; diff --git a/packages/frontend/editor-ui/src/views/SigninView.test.ts b/packages/frontend/editor-ui/src/views/SigninView.test.ts index fada4fea21..f4cbc228f5 100644 --- a/packages/frontend/editor-ui/src/views/SigninView.test.ts +++ b/packages/frontend/editor-ui/src/views/SigninView.test.ts @@ -46,7 +46,7 @@ let telemetry: ReturnType; describe('SigninView', () => { const signInWithValidUser = async () => { settingsStore.isCloudDeployment = false; - settingsStore.loadedModules = []; + settingsStore.activeModules = []; usersStore.loginWithCreds.mockResolvedValueOnce(); const { getByRole, queryByTestId, container } = renderComponent(); diff --git a/packages/frontend/editor-ui/src/views/SigninView.vue b/packages/frontend/editor-ui/src/views/SigninView.vue index 2d037e8fe8..514b584d25 100644 --- a/packages/frontend/editor-ui/src/views/SigninView.vue +++ b/packages/frontend/editor-ui/src/views/SigninView.vue @@ -145,7 +145,7 @@ const login = async (form: LoginRequestDto) => { } await settingsStore.getSettings(); - if (settingsStore.loadedModules.length > 0) { + if (settingsStore.activeModules.length > 0) { await settingsStore.getModuleSettings(); }