From 45c18fb09c04749063edc3545c38ad37006c0c49 Mon Sep 17 00:00:00 2001 From: Sandra Zollner Date: Mon, 4 May 2026 09:03:47 +0200 Subject: [PATCH] feat(core): Decouple insights pruning max age from license (#29527) --- ...sights-pruning.service.integration.test.ts | 56 ++++--------------- .../insights-pruning.service.test.ts | 8 --- .../insights.service.integration.test.ts | 29 ++-------- .../insights/insights-pruning.service.ts | 24 ++++---- .../src/modules/insights/insights.config.ts | 8 ++- .../modules/insights/insights.constants.ts | 6 ++ .../src/modules/insights/insights.service.ts | 4 +- 7 files changed, 42 insertions(+), 93 deletions(-) diff --git a/packages/cli/src/modules/insights/__tests__/insights-pruning.service.integration.test.ts b/packages/cli/src/modules/insights/__tests__/insights-pruning.service.integration.test.ts index f062ffcf684..f5b4627e5f4 100644 --- a/packages/cli/src/modules/insights/__tests__/insights-pruning.service.integration.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights-pruning.service.integration.test.ts @@ -1,4 +1,3 @@ -import type { LicenseState } from '@n8n/backend-common'; import { mockLogger, createTeamProject, @@ -41,20 +40,15 @@ describe('InsightsPruningService', () => { let insightsConfig: InsightsConfig; let insightsByPeriodRepository: InsightsByPeriodRepository; let insightsPruningService: InsightsPruningService; - let licenseState: LicenseState; beforeAll(async () => { insightsConfig = Container.get(InsightsConfig); insightsConfig.maxAgeDays = 10; insightsConfig.pruneCheckIntervalHours = 1; insightsByPeriodRepository = Container.get(InsightsByPeriodRepository); - licenseState = mock({ - getInsightsRetentionMaxAge: () => insightsConfig.maxAgeDays, - }); insightsPruningService = new InsightsPruningService( insightsByPeriodRepository, insightsConfig, - licenseState, mockLogger(), ); }); @@ -103,55 +97,25 @@ describe('InsightsPruningService', () => { expect(await insightsByPeriodRepository.count()).toBe(1); }); - test.each<{ config: number; license: number; result: number }>([ - { - config: -1, - license: -1, - result: Number.MAX_SAFE_INTEGER, - }, - { - config: -1, - license: 5, - result: 5, - }, - { - config: 5, - license: -1, - result: 5, - }, - { - config: 5, - license: 10, - result: 5, - }, - { - config: 10, - license: 5, - result: 5, - }, + test.each<{ config: number; result: number }>([ + { config: -1, result: 730 }, + { config: 0, result: 365 }, + { config: 5, result: 5 }, + { config: 365, result: 365 }, + { config: 730, result: 730 }, + { config: 2000, result: 730 }, ])( - 'pruningMaxAgeInDays is minimal age between license and config max age', - async ({ config, license, result }) => { - // ARRANGE - const licenseState = mock({ - getInsightsRetentionMaxAge() { - return license; - }, - }); + 'pruningMaxAgeInDays uses N8N_INSIGHTS_MAX_AGE_DAYS: -1 maps to cap, other values below 1 use default, finite values capped at 730', + async ({ config, result }) => { const insightsPruningService = new InsightsPruningService( insightsByPeriodRepository, mock({ maxAgeDays: config, }), - licenseState, mockLogger(), ); - // ACT - const maxAge = insightsPruningService.pruningMaxAgeInDays; - - // ASSERT - expect(maxAge).toBe(result); + expect(insightsPruningService.pruningMaxAgeInDays).toBe(result); }, ); }); 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 1dd9c16e637..5e912506a2c 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 @@ -1,4 +1,3 @@ -import type { LicenseState } from '@n8n/backend-common'; import { mockLogger } from '@n8n/backend-test-utils'; import { Time } from '@n8n/constants'; import { mock } from 'jest-mock-extended'; @@ -11,20 +10,15 @@ describe('InsightsPruningService', () => { let insightsConfig: InsightsConfig; let insightsByPeriodRepository: InsightsByPeriodRepository; let insightsPruningService: InsightsPruningService; - let licenseState: LicenseState; beforeAll(() => { insightsConfig = new InsightsConfig(); insightsConfig.maxAgeDays = 10; insightsConfig.pruneCheckIntervalHours = 1; insightsByPeriodRepository = mock(); - licenseState = mock({ - getInsightsRetentionMaxAge: () => insightsConfig.maxAgeDays, - }); insightsPruningService = new InsightsPruningService( insightsByPeriodRepository, insightsConfig, - licenseState, mockLogger(), ); }); @@ -51,7 +45,6 @@ describe('InsightsPruningService', () => { const insightsPruningService = new InsightsPruningService( insightsByPeriodRepository, insightsConfig, - licenseState, mockLogger(), ); const pruneSpy = jest.spyOn(insightsPruningService, 'pruneInsights'); @@ -75,7 +68,6 @@ describe('InsightsPruningService', () => { const insightsPruningService = new InsightsPruningService( insightsByPeriodRepository, insightsConfig, - licenseState, mockLogger(), ); diff --git a/packages/cli/src/modules/insights/__tests__/insights.service.integration.test.ts b/packages/cli/src/modules/insights/__tests__/insights.service.integration.test.ts index e94128f5475..8996e8422e2 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.service.integration.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.service.integration.test.ts @@ -85,22 +85,15 @@ describe('InsightsService (Integration)', () => { shutdownSpy.mockRestore(); }); - const setupMocks = ( - instanceType: InstanceType, - isLeader: boolean = false, - isPruningEnabled: boolean = false, - ) => { + const setupMocks = (instanceType: InstanceType, isLeader: boolean = false) => { (instanceSettings as any).instanceType = instanceType; Object.defineProperty(instanceSettings, 'isLeader', { get: jest.fn(() => isLeader), }); - Object.defineProperty(pruningService, 'isPruningEnabled', { - get: jest.fn(() => isPruningEnabled), - }); }; test('starts flushing timer for main instance', async () => { - setupMocks('main', false, false); + setupMocks('main', false); await insightsService.init(); @@ -109,18 +102,8 @@ describe('InsightsService (Integration)', () => { expect(pruningService.startPruningTimer).not.toHaveBeenCalled(); }); - test('starts compaction and flushing timers for main leader instances', async () => { - setupMocks('main', true, false); - - await insightsService.init(); - - expect(initSpy).toHaveBeenCalled(); - expect(compactionService.startCompactionTimer).toHaveBeenCalled(); - expect(pruningService.startPruningTimer).not.toHaveBeenCalled(); - }); - - test('starts compaction, flushing and pruning timers for main leader instance with pruning enabled', async () => { - setupMocks('main', true, true); + test('starts compaction, flushing and pruning timers for main leader instances', async () => { + setupMocks('main', true); await insightsService.init(); @@ -130,7 +113,7 @@ describe('InsightsService (Integration)', () => { }); test('starts only collection flushing timer for webhook instance', async () => { - setupMocks('webhook', false, false); + setupMocks('webhook', false); await insightsService.init(); @@ -140,7 +123,7 @@ describe('InsightsService (Integration)', () => { }); test('do no start any timers for non-main instances', async () => { - setupMocks('worker', false, false); + setupMocks('worker', false); await insightsService.init(); diff --git a/packages/cli/src/modules/insights/insights-pruning.service.ts b/packages/cli/src/modules/insights/insights-pruning.service.ts index ec58935a363..84d1d9ceb2c 100644 --- a/packages/cli/src/modules/insights/insights-pruning.service.ts +++ b/packages/cli/src/modules/insights/insights-pruning.service.ts @@ -1,10 +1,11 @@ -import { LicenseState, Logger } from '@n8n/backend-common'; +import { Logger } from '@n8n/backend-common'; import { Time } from '@n8n/constants'; import { Service } from '@n8n/di'; import { strict } from 'assert'; import { InsightsByPeriodRepository } from './database/repositories/insights-by-period.repository'; import { InsightsConfig } from './insights.config'; +import { INSIGHTS_MAX_AGE_DAYS_CAP, INSIGHTS_MAX_AGE_DAYS_DEFAULT } from './insights.constants'; @Service() export class InsightsPruningService { @@ -17,23 +18,26 @@ export class InsightsPruningService { constructor( private readonly insightsByPeriodRepository: InsightsByPeriodRepository, private readonly config: InsightsConfig, - private readonly licenseState: LicenseState, private readonly logger: Logger, ) { this.logger = this.logger.scoped('insights'); } - get isPruningEnabled() { - return this.licenseState.getInsightsRetentionMaxAge() > -1 || this.config.maxAgeDays > -1; - } - get pruningMaxAgeInDays() { - const toMaxSafeIfUnlimited = (days: number) => (days === -1 ? Number.MAX_SAFE_INTEGER : days); + const configuredMaxAgeDays = this.config.maxAgeDays; + if (typeof configuredMaxAgeDays !== 'number' || !Number.isFinite(configuredMaxAgeDays)) { + return INSIGHTS_MAX_AGE_DAYS_DEFAULT; + } - const licenseMaxAge = toMaxSafeIfUnlimited(this.licenseState.getInsightsRetentionMaxAge()); - const configMaxAge = toMaxSafeIfUnlimited(this.config.maxAgeDays); + if (configuredMaxAgeDays === -1) { + return INSIGHTS_MAX_AGE_DAYS_CAP; + } - return Math.min(licenseMaxAge, configMaxAge); + if (configuredMaxAgeDays < 1) { + return INSIGHTS_MAX_AGE_DAYS_DEFAULT; + } + + return Math.min(configuredMaxAgeDays, INSIGHTS_MAX_AGE_DAYS_CAP); } startPruningTimer() { diff --git a/packages/cli/src/modules/insights/insights.config.ts b/packages/cli/src/modules/insights/insights.config.ts index a28642202e9..0598db09f24 100644 --- a/packages/cli/src/modules/insights/insights.config.ts +++ b/packages/cli/src/modules/insights/insights.config.ts @@ -1,5 +1,7 @@ import { Config, Env } from '@n8n/config'; +import { INSIGHTS_MAX_AGE_DAYS_DEFAULT } from './insights.constants'; + @Config export class InsightsConfig { /** @@ -45,11 +47,11 @@ export class InsightsConfig { flushIntervalSeconds: number = 30; /** - * How old (days) insights data must be to qualify for regular deletion - * Default: -1 (no pruning) + * How old (days) insights data must be to qualify for regular deletion. + * Default: 365. Values are capped at 730 (two years). */ @Env('N8N_INSIGHTS_MAX_AGE_DAYS') - maxAgeDays: number = -1; + maxAgeDays: number = INSIGHTS_MAX_AGE_DAYS_DEFAULT; /** * How often (hours) insights data will be checked for regular deletion. diff --git a/packages/cli/src/modules/insights/insights.constants.ts b/packages/cli/src/modules/insights/insights.constants.ts index ef902ddb5a4..376cb34a884 100644 --- a/packages/cli/src/modules/insights/insights.constants.ts +++ b/packages/cli/src/modules/insights/insights.constants.ts @@ -1,3 +1,9 @@ +/** Default for `N8N_INSIGHTS_MAX_AGE_DAYS`; also used when the configured value is invalid or not a finite number ≥ 1. */ +export const INSIGHTS_MAX_AGE_DAYS_DEFAULT = 365; + +/** Maximum allowed value for `N8N_INSIGHTS_MAX_AGE_DAYS` when pruning (2 years). */ +export const INSIGHTS_MAX_AGE_DAYS_CAP = 730; + export const INSIGHTS_DATE_RANGE_KEYS = [ 'day', 'week', diff --git a/packages/cli/src/modules/insights/insights.service.ts b/packages/cli/src/modules/insights/insights.service.ts index 7ec49190f23..5acd1ee4f3b 100644 --- a/packages/cli/src/modules/insights/insights.service.ts +++ b/packages/cli/src/modules/insights/insights.service.ts @@ -52,9 +52,7 @@ export class InsightsService { @OnLeaderTakeover() startCompactionAndPruningTimers() { this.compactionService.startCompactionTimer(); - if (this.pruningService.isPruningEnabled) { - this.pruningService.startPruningTimer(); - } + this.pruningService.startPruningTimer(); } @OnLeaderStepdown()