Skip to content

Commit

Permalink
fix(core): Ensure TTL safeguard for test webhooks applies only to mul…
Browse files Browse the repository at this point in the history
…ti-main setup (#9062)
  • Loading branch information
ivov committed Apr 5, 2024
1 parent ff77ef4 commit ff81de3
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
9 changes: 4 additions & 5 deletions packages/cli/src/services/cache/cache.service.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
Expand Up @@ -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;
Expand All @@ -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';

Expand All @@ -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
Expand Down
@@ -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<CacheService>();
const registrations = new TestWebhookRegistrationsService(cacheService);
const registrations = new TestWebhookRegistrationsService(
cacheService,
mock<OrchestrationService>({ isMultiMainSetupEnabled: false }),
);

const registration = mock<TestWebhookRegistration>({
webhook: { httpMethod: 'GET', path: 'hello', webhookId: undefined },
Expand All @@ -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()', () => {
Expand Down

0 comments on commit ff81de3

Please sign in to comment.