fix(core): Fix test webhook deregistration (#8247)

This commit is contained in:
Iván Ovejero 2024-01-09 16:02:32 +01:00 committed by GitHub
parent 90404a4b88
commit 5032bf0e34
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 72 additions and 42 deletions

View file

@ -24,6 +24,7 @@ import { WebhookNotFoundError } from '@/errors/response-errors/webhook-not-found
import * as NodeExecuteFunctions from 'n8n-core';
import { removeTrailingSlash } from './utils';
import { TestWebhookRegistrationsService } from '@/services/test-webhook-registrations.service';
import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData';
@Service()
export class TestWebhooks implements IWebhookManager {
@ -185,6 +186,7 @@ export class TestWebhooks implements IWebhookManager {
* For every webhook call to listen for, also activate the webhook.
*/
async needsWebhook(
userId: string,
workflowEntity: IWorkflowDb,
additionalData: IWorkflowExecuteAdditionalData,
sessionId?: string,
@ -219,21 +221,23 @@ export class TestWebhooks implements IWebhookManager {
webhook.isTest = true;
/**
* Remove additional data from webhook because:
*
* - It is not needed for the test webhook to be executed.
* - It contains circular refs that cannot be cached.
* Additional data cannot be cached because of circular refs.
* Hence store the `userId` and recreate additional data when needed.
*/
const { workflowExecuteAdditionalData: _, ...rest } = webhook;
const { workflowExecuteAdditionalData: _, ...cacheableWebhook } = webhook;
cacheableWebhook.userId = userId;
try {
await workflow.createWebhookIfNotExists(webhook, NodeExecuteFunctions, 'manual', 'manual');
cacheableWebhook.staticData = workflow.staticData;
await this.registrations.register({
sessionId,
workflowEntity,
destinationNode,
webhook: rest as IWebhookData,
webhook: cacheableWebhook as IWebhookData,
});
this.timeouts[key] = timeout;
@ -341,6 +345,14 @@ export class TestWebhooks implements IWebhookManager {
if (!webhooks) return; // nothing to deactivate
for (const webhook of webhooks) {
const { userId, staticData } = webhook;
if (userId) {
webhook.workflowExecuteAdditionalData = await WorkflowExecuteAdditionalData.getBase(userId);
}
if (staticData) workflow.staticData = staticData;
await workflow.deleteWebhook(webhook, NodeExecuteFunctions, 'internal', 'update');
await this.registrations.deregister(webhook);
@ -350,7 +362,7 @@ export class TestWebhooks implements IWebhookManager {
/**
* Convert a `WorkflowEntity` from `typeorm` to a `Workflow` from `n8n-workflow`.
*/
private toWorkflow(workflowEntity: IWorkflowDb) {
toWorkflow(workflowEntity: IWorkflowDb) {
return new Workflow({
id: workflowEntity.id,
name: workflowEntity.name,
@ -358,7 +370,14 @@ export class TestWebhooks implements IWebhookManager {
connections: workflowEntity.connections,
active: false,
nodeTypes: this.nodeTypes,
/**
* `staticData` in the original workflow entity has production webhook IDs.
* Since we are creating here a temporary workflow only for a test webhook,
* `staticData` from the original workflow entity should not be transferred.
*/
staticData: undefined,
settings: workflowEntity.settings,
});
}

View file

@ -317,6 +317,7 @@ export class WorkflowService {
const additionalData = await WorkflowExecuteAdditionalData.getBase(user.id);
const needsWebhook = await this.testWebhooks.needsWebhook(
user.id,
workflowData,
additionalData,
sessionId,

View file

@ -5,53 +5,49 @@ import { v4 as uuid } from 'uuid';
import { generateNanoId } from '@/databases/utils/generators';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import * as WebhookHelpers from '@/WebhookHelpers';
import type * as express from 'express';
import type { IWorkflowDb, WebhookRequest } from '@/Interfaces';
import type {
IWebhookData,
IWorkflowExecuteAdditionalData,
Workflow,
WorkflowActivateMode,
WorkflowExecuteMode,
} from 'n8n-workflow';
import type { IWebhookData, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import type {
TestWebhookRegistrationsService,
TestWebhookRegistration,
} from '@/services/test-webhook-registrations.service';
describe('TestWebhooks', () => {
const registrations = mock<TestWebhookRegistrationsService>();
const testWebhooks = new TestWebhooks(mock(), mock(), registrations);
import * as AdditionalData from '@/WorkflowExecuteAdditionalData';
jest.mock('@/WorkflowExecuteAdditionalData');
const mockedAdditionalData = AdditionalData as jest.Mocked<typeof AdditionalData>;
const workflowEntity = mock<IWorkflowDb>({ id: generateNanoId(), nodes: [] });
const httpMethod = 'GET';
const path = uuid();
const userId = '04ab4baf-85df-478f-917b-d303934a97de';
const webhook = mock<IWebhookData>({
httpMethod,
path,
workflowId: workflowEntity.id,
userId,
});
const registrations = mock<TestWebhookRegistrationsService>();
let testWebhooks: TestWebhooks;
describe('TestWebhooks', () => {
beforeAll(() => {
testWebhooks = new TestWebhooks(mock(), mock(), registrations);
jest.useFakeTimers();
});
const httpMethod = 'GET';
const path = uuid();
const workflowId = generateNanoId();
const webhook = mock<IWebhookData>({
httpMethod,
path,
workflowId,
});
describe('needsWebhook()', () => {
type NeedsWebhookArgs = [
IWorkflowDb,
IWorkflowExecuteAdditionalData,
WorkflowExecuteMode,
WorkflowActivateMode,
];
const workflow = mock<Workflow>({ id: workflowId });
const args: NeedsWebhookArgs = [
mock<IWorkflowDb>({ id: workflowId, nodes: [] }),
const args: Parameters<typeof testWebhooks.needsWebhook> = [
userId,
workflowEntity,
mock<IWorkflowExecuteAdditionalData>(),
'manual',
'manual',
];
test('if webhook is needed, should return true and activate webhook', async () => {
@ -103,17 +99,29 @@ describe('TestWebhooks', () => {
const registration = mock<TestWebhookRegistration>({
sessionId: 'some-session-id',
workflowEntity: mock<IWorkflowDb>({}),
workflowEntity,
});
await registrations.register(registration);
const promise = testWebhooks.executeWebhook(
mock<WebhookRequest>({ params: { path } }),
mock(),
mock<express.Response>(),
);
await expect(promise).rejects.toThrowError(NotFoundError);
});
});
describe('deactivateWebhooks()', () => {
test('should add additional data to workflow', async () => {
registrations.getAllRegistrations.mockResolvedValue([{ workflowEntity, webhook }]);
const workflow = testWebhooks.toWorkflow(workflowEntity);
await testWebhooks.deactivateWebhooks(workflow);
expect(mockedAdditionalData.getBase).toHaveBeenCalledWith(userId);
});
});
});

View file

@ -1674,6 +1674,8 @@ export interface IWebhookData {
workflowExecuteAdditionalData: IWorkflowExecuteAdditionalData;
webhookId?: string;
isTest?: boolean;
userId?: string;
staticData?: Workflow['staticData'];
}
export interface IWebhookDescription {