fix(core): Ensure TTL safeguard for test webhooks applies only to multi-main setup (#9062)

This commit is contained in:
Iván Ovejero 2024-04-05 14:15:49 +02:00 committed by GitHub
parent ff77ef4b62
commit ff81de3313
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 22 additions and 7 deletions

View file

@ -2,7 +2,7 @@ import EventEmitter from 'node:events';
import { Service } from 'typedi'; import { Service } from 'typedi';
import { caching } from 'cache-manager'; import { caching } from 'cache-manager';
import { jsonStringify } from 'n8n-workflow'; import { ApplicationError, jsonStringify } from 'n8n-workflow';
import config from '@/config'; import config from '@/config';
import { getDefaultRedisClient, getRedisPrefix } from '@/services/redis/RedisServiceHelper'; import { getDefaultRedisClient, getRedisPrefix } from '@/services/redis/RedisServiceHelper';
@ -137,10 +137,9 @@ export class CacheService extends EventEmitter {
if (!key?.length) return; if (!key?.length) return;
if (this.cache.kind === 'memory') { if (this.cache.kind === 'memory') {
setTimeout(async () => { throw new ApplicationError('Method `expire` not yet implemented for in-memory cache', {
await this.cache.store.del(key); level: 'warning',
}, ttlMs); });
return;
} }
await this.cache.store.expire(key, ttlMs / TIME.SECOND); await this.cache.store.expire(key, ttlMs / TIME.SECOND);

View file

@ -3,6 +3,7 @@ import { CacheService } from '@/services/cache/cache.service';
import type { IWebhookData } from 'n8n-workflow'; import type { IWebhookData } from 'n8n-workflow';
import type { IWorkflowDb } from '@/Interfaces'; import type { IWorkflowDb } from '@/Interfaces';
import { TEST_WEBHOOK_TIMEOUT, TEST_WEBHOOK_TIMEOUT_BUFFER } from '@/constants'; import { TEST_WEBHOOK_TIMEOUT, TEST_WEBHOOK_TIMEOUT_BUFFER } from '@/constants';
import { OrchestrationService } from './orchestration.service';
export type TestWebhookRegistration = { export type TestWebhookRegistration = {
pushRef?: string; pushRef?: string;
@ -13,7 +14,10 @@ export type TestWebhookRegistration = {
@Service() @Service()
export class TestWebhookRegistrationsService { export class TestWebhookRegistrationsService {
constructor(private readonly cacheService: CacheService) {} constructor(
private readonly cacheService: CacheService,
private readonly orchestrationService: OrchestrationService,
) {}
private readonly cacheKey = 'test-webhooks'; private readonly cacheKey = 'test-webhooks';
@ -22,6 +26,8 @@ export class TestWebhookRegistrationsService {
await this.cacheService.setHash(this.cacheKey, { [hashKey]: registration }); 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 * 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 * handles a webhook might not be the same as the main process that created

View file

@ -1,11 +1,15 @@
import type { CacheService } from '@/services/cache/cache.service'; 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 type { TestWebhookRegistration } from '@/services/test-webhook-registrations.service';
import { TestWebhookRegistrationsService } from '@/services/test-webhook-registrations.service'; import { TestWebhookRegistrationsService } from '@/services/test-webhook-registrations.service';
import { mock } from 'jest-mock-extended'; import { mock } from 'jest-mock-extended';
describe('TestWebhookRegistrationsService', () => { describe('TestWebhookRegistrationsService', () => {
const cacheService = mock<CacheService>(); const cacheService = mock<CacheService>();
const registrations = new TestWebhookRegistrationsService(cacheService); const registrations = new TestWebhookRegistrationsService(
cacheService,
mock<OrchestrationService>({ isMultiMainSetupEnabled: false }),
);
const registration = mock<TestWebhookRegistration>({ const registration = mock<TestWebhookRegistration>({
webhook: { httpMethod: 'GET', path: 'hello', webhookId: undefined }, webhook: { httpMethod: 'GET', path: 'hello', webhookId: undefined },
@ -20,6 +24,12 @@ describe('TestWebhookRegistrationsService', () => {
expect(cacheService.setHash).toHaveBeenCalledWith(cacheKey, { [webhookKey]: registration }); 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()', () => { describe('deregister()', () => {