From ff81de3313e8fd612104830b1b541b9dda392bb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Fri, 5 Apr 2024 14:15:49 +0200 Subject: [PATCH] fix(core): Ensure TTL safeguard for test webhooks applies only to multi-main setup (#9062) --- packages/cli/src/services/cache/cache.service.ts | 9 ++++----- .../services/test-webhook-registrations.service.ts | 8 +++++++- .../test-webhook-registrations.service.test.ts | 12 +++++++++++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/services/cache/cache.service.ts b/packages/cli/src/services/cache/cache.service.ts index 09499a4da4df1..226c68438fba8 100644 --- a/packages/cli/src/services/cache/cache.service.ts +++ b/packages/cli/src/services/cache/cache.service.ts @@ -2,7 +2,7 @@ import EventEmitter from 'node:events'; import { Service } from 'typedi'; import { caching } from 'cache-manager'; -import { jsonStringify } from 'n8n-workflow'; +import { ApplicationError, jsonStringify } from 'n8n-workflow'; import config from '@/config'; import { getDefaultRedisClient, getRedisPrefix } from '@/services/redis/RedisServiceHelper'; @@ -137,10 +137,9 @@ export class CacheService extends EventEmitter { if (!key?.length) return; if (this.cache.kind === 'memory') { - setTimeout(async () => { - await this.cache.store.del(key); - }, ttlMs); - return; + throw new ApplicationError('Method `expire` not yet implemented for in-memory cache', { + level: 'warning', + }); } await this.cache.store.expire(key, ttlMs / TIME.SECOND); diff --git a/packages/cli/src/services/test-webhook-registrations.service.ts b/packages/cli/src/services/test-webhook-registrations.service.ts index b1cc8920a1078..e2abae5605f98 100644 --- a/packages/cli/src/services/test-webhook-registrations.service.ts +++ b/packages/cli/src/services/test-webhook-registrations.service.ts @@ -3,6 +3,7 @@ import { CacheService } from '@/services/cache/cache.service'; import type { IWebhookData } from 'n8n-workflow'; import type { IWorkflowDb } from '@/Interfaces'; import { TEST_WEBHOOK_TIMEOUT, TEST_WEBHOOK_TIMEOUT_BUFFER } from '@/constants'; +import { OrchestrationService } from './orchestration.service'; export type TestWebhookRegistration = { pushRef?: string; @@ -13,7 +14,10 @@ export type TestWebhookRegistration = { @Service() export class TestWebhookRegistrationsService { - constructor(private readonly cacheService: CacheService) {} + constructor( + private readonly cacheService: CacheService, + private readonly orchestrationService: OrchestrationService, + ) {} private readonly cacheKey = 'test-webhooks'; @@ -22,6 +26,8 @@ export class TestWebhookRegistrationsService { await this.cacheService.setHash(this.cacheKey, { [hashKey]: registration }); + if (!this.orchestrationService.isMultiMainSetupEnabled) return; + /** * Multi-main setup: In a manual webhook execution, the main process that * handles a webhook might not be the same as the main process that created diff --git a/packages/cli/test/unit/services/test-webhook-registrations.service.test.ts b/packages/cli/test/unit/services/test-webhook-registrations.service.test.ts index 4bd9efac47a31..c93540938c1bd 100644 --- a/packages/cli/test/unit/services/test-webhook-registrations.service.test.ts +++ b/packages/cli/test/unit/services/test-webhook-registrations.service.test.ts @@ -1,11 +1,15 @@ import type { CacheService } from '@/services/cache/cache.service'; +import type { OrchestrationService } from '@/services/orchestration.service'; import type { TestWebhookRegistration } from '@/services/test-webhook-registrations.service'; import { TestWebhookRegistrationsService } from '@/services/test-webhook-registrations.service'; import { mock } from 'jest-mock-extended'; describe('TestWebhookRegistrationsService', () => { const cacheService = mock(); - const registrations = new TestWebhookRegistrationsService(cacheService); + const registrations = new TestWebhookRegistrationsService( + cacheService, + mock({ isMultiMainSetupEnabled: false }), + ); const registration = mock({ webhook: { httpMethod: 'GET', path: 'hello', webhookId: undefined }, @@ -20,6 +24,12 @@ describe('TestWebhookRegistrationsService', () => { expect(cacheService.setHash).toHaveBeenCalledWith(cacheKey, { [webhookKey]: registration }); }); + + test('should skip setting TTL in single-main setup', async () => { + await registrations.register(registration); + + expect(cacheService.expire).not.toHaveBeenCalled(); + }); }); describe('deregister()', () => {