mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-30 18:01:23 +08:00
feat: Validate nodes before activating (#22916)
This commit is contained in:
@@ -2,14 +2,7 @@ import { mockLogger } from '@n8n/backend-test-utils';
|
||||
import type { WorkflowEntity, WorkflowHistory, WorkflowRepository } from '@n8n/db';
|
||||
import { mock } from 'jest-mock-extended';
|
||||
import type { InstanceSettings } from 'n8n-core';
|
||||
import type {
|
||||
WorkflowParameters,
|
||||
INode,
|
||||
INodeType,
|
||||
INodeTypeDescription,
|
||||
WorkflowActivateMode,
|
||||
} from 'n8n-workflow';
|
||||
import { Workflow } from 'n8n-workflow';
|
||||
import type { WorkflowActivateMode } from 'n8n-workflow';
|
||||
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import type { NodeTypes } from '@/node-types';
|
||||
@@ -43,49 +36,6 @@ describe('ActiveWorkflowManager', () => {
|
||||
);
|
||||
});
|
||||
|
||||
describe('checkIfWorkflowCanBeActivated', () => {
|
||||
const disabledNode = mock<INode>({ type: 'triggerNode', disabled: true });
|
||||
const unknownNode = mock<INode>({ type: 'unknownNode' });
|
||||
const noTriggersNode = mock<INode>({ type: 'noTriggersNode' });
|
||||
const pollNode = mock<INode>({ type: 'pollNode' });
|
||||
const triggerNode = mock<INode>({ type: 'triggerNode' });
|
||||
const webhookNode = mock<INode>({ type: 'webhookNode' });
|
||||
|
||||
nodeTypes.getByNameAndVersion.mockImplementation((type) => {
|
||||
// TODO: getByNameAndVersion signature needs to be updated to allow returning undefined
|
||||
if (type === 'unknownNode') return undefined as unknown as INodeType;
|
||||
const partial: Partial<INodeType> = {
|
||||
poll: undefined,
|
||||
trigger: undefined,
|
||||
webhook: undefined,
|
||||
description: mock<INodeTypeDescription>({
|
||||
properties: [],
|
||||
}),
|
||||
};
|
||||
if (type === 'pollNode') partial.poll = jest.fn();
|
||||
if (type === 'triggerNode') partial.trigger = jest.fn();
|
||||
if (type === 'webhookNode') partial.webhook = jest.fn();
|
||||
return mock(partial);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['should skip disabled nodes', disabledNode, [], false],
|
||||
['should skip nodes marked as ignored', triggerNode, ['triggerNode'], false],
|
||||
['should skip unknown nodes', unknownNode, [], false],
|
||||
['should skip nodes with no trigger method', noTriggersNode, [], false],
|
||||
['should activate if poll method exists', pollNode, [], true],
|
||||
['should activate if trigger method exists', triggerNode, [], true],
|
||||
['should activate if webhook method exists', webhookNode, [], true],
|
||||
])('%s', async (_, node, ignoredNodes, expected) => {
|
||||
const workflow = new Workflow(mock<WorkflowParameters>({ nodeTypes, nodes: [node] }));
|
||||
const canBeActivated = activeWorkflowManager.checkIfWorkflowCanBeActivated(
|
||||
workflow,
|
||||
ignoredNodes,
|
||||
);
|
||||
expect(canBeActivated).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldAddWebhooks', () => {
|
||||
describe('if leader', () => {
|
||||
beforeAll(() => {
|
||||
@@ -130,7 +80,6 @@ describe('ActiveWorkflowManager', () => {
|
||||
test.each<[WorkflowActivateMode]>([['init'], ['leadershipChange']])(
|
||||
'should skip inactive workflow in `%s` activation mode',
|
||||
async (mode) => {
|
||||
const checkSpy = jest.spyOn(activeWorkflowManager, 'checkIfWorkflowCanBeActivated');
|
||||
const addWebhooksSpy = jest.spyOn(activeWorkflowManager, 'addWebhooks');
|
||||
const addTriggersAndPollersSpy = jest.spyOn(
|
||||
activeWorkflowManager,
|
||||
@@ -142,7 +91,6 @@ describe('ActiveWorkflowManager', () => {
|
||||
|
||||
const added = await activeWorkflowManager.add('some-id', mode);
|
||||
|
||||
expect(checkSpy).not.toHaveBeenCalled();
|
||||
expect(addWebhooksSpy).not.toHaveBeenCalled();
|
||||
expect(addTriggersAndPollersSpy).not.toHaveBeenCalled();
|
||||
expect(added).toEqual({ triggersAndPollers: false, webhooks: false });
|
||||
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
UnexpectedError,
|
||||
ensureError,
|
||||
createRunExecutionData,
|
||||
validateWorkflowHasTriggerLikeNode,
|
||||
} from 'n8n-workflow';
|
||||
import { strict } from 'node:assert';
|
||||
|
||||
@@ -626,9 +627,13 @@ export class ActiveWorkflowManager {
|
||||
settings: dbWorkflow.settings,
|
||||
});
|
||||
|
||||
const canBeActivated = this.checkIfWorkflowCanBeActivated(workflow, STARTING_NODES);
|
||||
const validation = validateWorkflowHasTriggerLikeNode(
|
||||
workflow.nodes,
|
||||
this.nodeTypes,
|
||||
STARTING_NODES,
|
||||
);
|
||||
|
||||
if (!canBeActivated) {
|
||||
if (!validation.isValid) {
|
||||
throw new WorkflowActivationError(
|
||||
`Workflow ${formatWorkflow(dbWorkflow)} has no node to start the workflow - at least one trigger, poller or webhook node is required`,
|
||||
{ level: 'warning' },
|
||||
@@ -741,48 +746,6 @@ export class ActiveWorkflowManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A workflow can only be activated if it has a node which has either triggers
|
||||
* or webhooks defined.
|
||||
*
|
||||
* @param {string[]} [ignoreNodeTypes] Node-types to ignore in the check
|
||||
*/
|
||||
checkIfWorkflowCanBeActivated(workflow: Workflow, ignoreNodeTypes?: string[]): boolean {
|
||||
let node: INode;
|
||||
let nodeType: INodeType | undefined;
|
||||
|
||||
for (const nodeName of Object.keys(workflow.nodes)) {
|
||||
node = workflow.nodes[nodeName];
|
||||
|
||||
if (node.disabled === true) {
|
||||
// Deactivated nodes can not trigger a run so ignore
|
||||
continue;
|
||||
}
|
||||
|
||||
if (ignoreNodeTypes !== undefined && ignoreNodeTypes.includes(node.type)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
|
||||
|
||||
if (nodeType === undefined) {
|
||||
// Type is not known so check is not possible
|
||||
continue;
|
||||
}
|
||||
|
||||
if (
|
||||
nodeType.poll !== undefined ||
|
||||
nodeType.trigger !== undefined ||
|
||||
nodeType.webhook !== undefined
|
||||
) {
|
||||
// Is a trigger node. So workflow can be activated.
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Count all triggers in the workflow, excluding Manual Trigger and other n8n-internal triggers.
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
import { BadRequestError } from './bad-request.error';
|
||||
|
||||
/**
|
||||
* Error thrown when a workflow fails validation before activation.
|
||||
*/
|
||||
export class WorkflowValidationError extends BadRequestError {
|
||||
readonly meta = { validationError: true as const };
|
||||
|
||||
constructor(message: string) {
|
||||
super(message);
|
||||
this.name = 'WorkflowValidationError';
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,29 @@
|
||||
import { Service } from '@n8n/di';
|
||||
import { validateWorkflowHasTriggerLikeNode } from 'n8n-workflow';
|
||||
import type { INodes } from 'n8n-workflow';
|
||||
|
||||
import { STARTING_NODES } from '@/constants';
|
||||
import type { NodeTypes } from '@/node-types';
|
||||
|
||||
export interface WorkflowValidationResult {
|
||||
isValid: boolean;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
@Service()
|
||||
export class WorkflowValidationService {
|
||||
validateForActivation(nodes: INodes, nodeTypes: NodeTypes): WorkflowValidationResult {
|
||||
const triggerValidation = validateWorkflowHasTriggerLikeNode(nodes, nodeTypes, STARTING_NODES);
|
||||
|
||||
if (!triggerValidation.isValid) {
|
||||
return {
|
||||
isValid: false,
|
||||
error:
|
||||
triggerValidation.error ??
|
||||
'Workflow cannot be activated because it has no trigger node. At least one trigger, webhook, or polling node is required.',
|
||||
};
|
||||
}
|
||||
|
||||
return { isValid: true };
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@ import type {
|
||||
ListQueryDb,
|
||||
WorkflowFolderUnionFull,
|
||||
WorkflowHistoryUpdate,
|
||||
WorkflowHistory,
|
||||
} from '@n8n/db';
|
||||
import {
|
||||
SharedWorkflow,
|
||||
@@ -26,6 +27,7 @@ import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPar
|
||||
import isEqual from 'lodash/isEqual';
|
||||
import pick from 'lodash/pick';
|
||||
import { FileLocation, BinaryDataService } from 'n8n-core';
|
||||
import type { INode, INodes } from 'n8n-workflow';
|
||||
import { NodeApiError, PROJECT_ROOT, assert, calculateWorkflowChecksum } from 'n8n-workflow';
|
||||
import { v4 as uuid } from 'uuid';
|
||||
|
||||
@@ -37,10 +39,12 @@ import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import { FolderNotFoundError } from '@/errors/folder-not-found.error';
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
import { NotFoundError } from '@/errors/response-errors/not-found.error';
|
||||
import { WorkflowValidationError } from '@/errors/response-errors/workflow-validation.error';
|
||||
import { WorkflowHistoryVersionNotFoundError } from '@/errors/workflow-history-version-not-found.error';
|
||||
import { EventService } from '@/events/event.service';
|
||||
import { ExternalHooks } from '@/external-hooks';
|
||||
import { validateEntity } from '@/generic-helpers';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
import type { ListQuery } from '@/requests';
|
||||
import { hasSharing } from '@/requests';
|
||||
import { OwnershipService } from '@/services/ownership.service';
|
||||
@@ -49,6 +53,8 @@ import { RoleService } from '@/services/role.service';
|
||||
import { TagService } from '@/services/tag.service';
|
||||
import * as WorkflowHelpers from '@/workflow-helpers';
|
||||
|
||||
import { WorkflowValidationService } from './workflow-validation.service';
|
||||
|
||||
@Service()
|
||||
export class WorkflowService {
|
||||
constructor(
|
||||
@@ -71,6 +77,8 @@ export class WorkflowService {
|
||||
private readonly folderRepository: FolderRepository,
|
||||
private readonly workflowFinderService: WorkflowFinderService,
|
||||
private readonly workflowPublishHistoryRepository: WorkflowPublishHistoryRepository,
|
||||
private readonly workflowValidationService: WorkflowValidationService,
|
||||
private readonly nodeTypes: NodeTypes,
|
||||
) {}
|
||||
|
||||
async getMany(
|
||||
@@ -519,13 +527,19 @@ export class WorkflowService {
|
||||
);
|
||||
}
|
||||
|
||||
const versionToActivate = options?.versionId ?? workflow.versionId;
|
||||
const versionIdToActivate = options?.versionId ?? workflow.versionId;
|
||||
const wasActive = workflow.activeVersionId !== null;
|
||||
|
||||
let versionToActivate: WorkflowHistory;
|
||||
try {
|
||||
await this.workflowHistoryService.getVersion(user, workflow.id, versionToActivate, {
|
||||
includePublishHistory: false,
|
||||
});
|
||||
versionToActivate = await this.workflowHistoryService.getVersion(
|
||||
user,
|
||||
workflow.id,
|
||||
versionIdToActivate,
|
||||
{
|
||||
includePublishHistory: false,
|
||||
},
|
||||
);
|
||||
} catch (error) {
|
||||
if (error instanceof WorkflowHistoryVersionNotFoundError) {
|
||||
throw new NotFoundError('Version not found');
|
||||
@@ -537,6 +551,8 @@ export class WorkflowService {
|
||||
await this._detectConflicts(workflow, options.expectedChecksum);
|
||||
}
|
||||
|
||||
this._validateNodes(workflowId, versionToActivate.nodes);
|
||||
|
||||
if (wasActive) {
|
||||
await this.activeWorkflowManager.remove(workflowId);
|
||||
}
|
||||
@@ -544,7 +560,7 @@ export class WorkflowService {
|
||||
const activationMode = wasActive ? 'update' : 'activate';
|
||||
|
||||
await this.workflowRepository.update(workflowId, {
|
||||
activeVersionId: versionToActivate,
|
||||
activeVersionId: versionIdToActivate,
|
||||
active: true,
|
||||
// workflow content did not change, so we keep updatedAt as is
|
||||
updatedAt: workflow.updatedAt,
|
||||
@@ -578,7 +594,11 @@ export class WorkflowService {
|
||||
const updateFields: WorkflowHistoryUpdate = {};
|
||||
if (options.name !== undefined) updateFields.name = options.name;
|
||||
if (options.description !== undefined) updateFields.description = options.description;
|
||||
await this.workflowHistoryService.updateVersion(versionToActivate, workflowId, updateFields);
|
||||
await this.workflowHistoryService.updateVersion(
|
||||
versionIdToActivate,
|
||||
workflowId,
|
||||
updateFields,
|
||||
);
|
||||
}
|
||||
|
||||
// Fetch workflow again with workflowPublishHistory after activation to include the new entry
|
||||
@@ -890,4 +910,23 @@ export class WorkflowService {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
_validateNodes(workflowId: string, nodes: INode[]) {
|
||||
const nodesToValidate = nodes.reduce<INodes>((acc, node) => {
|
||||
acc[node.name] = node;
|
||||
return acc;
|
||||
}, {});
|
||||
const validation = this.workflowValidationService.validateForActivation(
|
||||
nodesToValidate,
|
||||
this.nodeTypes,
|
||||
);
|
||||
|
||||
if (!validation.isValid) {
|
||||
this.logger.warn('Workflow activation failed validation', {
|
||||
workflowId,
|
||||
error: validation.error,
|
||||
});
|
||||
throw new WorkflowValidationError(validation.error ?? 'Workflow validation failed');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ import { InstanceSettings, ExternalSecretsProxy } from 'n8n-core';
|
||||
import { FormTrigger } from 'n8n-nodes-base/nodes/Form/FormTrigger.node';
|
||||
import { ScheduleTrigger } from 'n8n-nodes-base/nodes/Schedule/ScheduleTrigger.node';
|
||||
import { NodeApiError, Workflow } from 'n8n-workflow';
|
||||
import type * as N8nWorkflow from 'n8n-workflow';
|
||||
import type {
|
||||
IWebhookData,
|
||||
IWorkflowBase,
|
||||
@@ -19,9 +20,6 @@ import type {
|
||||
INodeTypeData,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
import { createOwner } from './shared/db/users';
|
||||
import * as utils from './shared/utils/';
|
||||
|
||||
import { ActiveExecutions } from '@/active-executions';
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import { ExecutionService } from '@/executions/execution.service';
|
||||
@@ -34,6 +32,9 @@ import { WebhookService } from '@/webhooks/webhook.service';
|
||||
import * as AdditionalData from '@/workflow-execute-additional-data';
|
||||
import { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
import { createOwner } from './shared/db/users';
|
||||
import * as utils from './shared/utils/';
|
||||
|
||||
mockInstance(ActiveExecutions);
|
||||
mockInstance(Push);
|
||||
mockInstance(ExternalSecretsProxy);
|
||||
@@ -54,6 +55,23 @@ let createActiveWorkflow: (
|
||||
let createInactiveWorkflow: () => Promise<IWorkflowBase>;
|
||||
let owner: User;
|
||||
|
||||
jest.mock('n8n-workflow', () => {
|
||||
const actual = jest.requireActual<typeof N8nWorkflow>('n8n-workflow');
|
||||
return {
|
||||
...actual,
|
||||
validateWorkflowHasTriggerLikeNode: jest.fn(
|
||||
(...args: Parameters<typeof actual.validateWorkflowHasTriggerLikeNode>) =>
|
||||
actual.validateWorkflowHasTriggerLikeNode(...args),
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
const { validateWorkflowHasTriggerLikeNode } = jest.requireMock<typeof N8nWorkflow>('n8n-workflow');
|
||||
const validateWorkflowHasTriggerLikeNodeSpy =
|
||||
validateWorkflowHasTriggerLikeNode as jest.MockedFunction<
|
||||
typeof N8nWorkflow.validateWorkflowHasTriggerLikeNode
|
||||
>;
|
||||
|
||||
beforeAll(async () => {
|
||||
await testDb.init();
|
||||
|
||||
@@ -113,13 +131,9 @@ describe('init()', () => {
|
||||
it('should check that workflow can be activated', async () => {
|
||||
await Promise.all([createActiveWorkflow(), createActiveWorkflow()]);
|
||||
|
||||
const checkSpy = jest
|
||||
.spyOn(activeWorkflowManager, 'checkIfWorkflowCanBeActivated')
|
||||
.mockReturnValue(true);
|
||||
|
||||
await activeWorkflowManager.init();
|
||||
|
||||
expect(checkSpy).toHaveBeenCalledTimes(2);
|
||||
expect(validateWorkflowHasTriggerLikeNodeSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -342,7 +356,6 @@ describe('addWebhooks()', () => {
|
||||
const [node] = dbWorkflow.nodes;
|
||||
|
||||
jest.spyOn(Workflow.prototype, 'getNode').mockReturnValue(node);
|
||||
jest.spyOn(activeWorkflowManager, 'checkIfWorkflowCanBeActivated').mockReturnValue(true);
|
||||
webhookService.createWebhookIfNotExists.mockResolvedValue(undefined);
|
||||
|
||||
await activeWorkflowManager.addWebhooks(workflow, additionalData, 'trigger', 'init');
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
import { createWorkflowWithHistory, testDb, mockInstance } from '@n8n/backend-test-utils';
|
||||
import {
|
||||
createWorkflowWithHistory,
|
||||
testDb,
|
||||
mockInstance,
|
||||
createActiveWorkflow,
|
||||
} from '@n8n/backend-test-utils';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import {
|
||||
SharedWorkflowRepository,
|
||||
@@ -12,19 +17,24 @@ import { v4 as uuid } from 'uuid';
|
||||
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
import { Telemetry } from '@/telemetry';
|
||||
import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
|
||||
import { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service';
|
||||
import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
|
||||
import { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
import { createOwner } from '../shared/db/users';
|
||||
import { createWorkflowHistoryItem } from '../shared/db/workflow-history';
|
||||
|
||||
let globalConfig: GlobalConfig;
|
||||
let workflowRepository: WorkflowRepository;
|
||||
let workflowService: WorkflowService;
|
||||
let workflowPublishHistoryRepository: WorkflowPublishHistoryRepository;
|
||||
let workflowHistoryService: WorkflowHistoryService;
|
||||
const activeWorkflowManager = mockInstance(ActiveWorkflowManager);
|
||||
const workflowHistoryService = mockInstance(WorkflowHistoryService);
|
||||
const workflowValidationService = mockInstance(WorkflowValidationService);
|
||||
const nodeTypes = mockInstance(NodeTypes);
|
||||
mockInstance(MessageEventBus);
|
||||
mockInstance(Telemetry);
|
||||
|
||||
@@ -32,11 +42,13 @@ beforeAll(async () => {
|
||||
await testDb.init();
|
||||
|
||||
globalConfig = Container.get(GlobalConfig);
|
||||
workflowRepository = Container.get(WorkflowRepository);
|
||||
workflowPublishHistoryRepository = Container.get(WorkflowPublishHistoryRepository);
|
||||
workflowHistoryService = Container.get(WorkflowHistoryService);
|
||||
workflowService = new WorkflowService(
|
||||
mock(),
|
||||
Container.get(SharedWorkflowRepository),
|
||||
Container.get(WorkflowRepository),
|
||||
workflowRepository,
|
||||
mock(),
|
||||
mock(),
|
||||
mock(),
|
||||
@@ -53,9 +65,15 @@ beforeAll(async () => {
|
||||
mock(),
|
||||
Container.get(WorkflowFinderService),
|
||||
workflowPublishHistoryRepository,
|
||||
workflowValidationService,
|
||||
nodeTypes,
|
||||
);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await testDb.truncate(['WorkflowEntity', 'WorkflowHistory', 'WorkflowPublishHistory']);
|
||||
jest.restoreAllMocks();
|
||||
@@ -190,4 +208,37 @@ describe('activateWorkflow()', () => {
|
||||
userId: owner.id,
|
||||
});
|
||||
});
|
||||
|
||||
test('should not activate workflow if validation fails and keep old active version', async () => {
|
||||
const owner = await createOwner();
|
||||
const workflow = await createActiveWorkflow({}, owner);
|
||||
|
||||
const oldActiveVersionId = workflow.activeVersionId;
|
||||
|
||||
const addRecordSpy = jest.spyOn(workflowPublishHistoryRepository, 'addRecord');
|
||||
|
||||
// Create a new version to try to activate
|
||||
const newVersionId = uuid();
|
||||
await createWorkflowHistoryItem(workflow.id, { versionId: newVersionId });
|
||||
|
||||
// Mock validation to fail
|
||||
workflowValidationService.validateForActivation.mockReturnValue({
|
||||
isValid: false,
|
||||
error: 'Workflow cannot be activated because it has no trigger node.',
|
||||
});
|
||||
|
||||
await expect(
|
||||
workflowService.activateWorkflow(owner, workflow.id, {
|
||||
versionId: newVersionId,
|
||||
}),
|
||||
).rejects.toThrow('Workflow cannot be activated because it has no trigger node.');
|
||||
|
||||
// Verify no publish history was added
|
||||
expect(addRecordSpy).not.toBeCalled();
|
||||
|
||||
// Verify the workflow still has the old active version
|
||||
const workflowAfter = await workflowRepository.findOne({ where: { id: workflow.id } });
|
||||
expect(workflowAfter?.activeVersionId).toBe(oldActiveVersionId);
|
||||
expect(workflowAfter?.active).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
} from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import type { Scope } from '@n8n/permissions';
|
||||
import { WorkflowValidationService } from '@/workflows/workflow-validation.service';
|
||||
import { createFolder } from '@test-integration/db/folders';
|
||||
import { DateTime } from 'luxon';
|
||||
import { PROJECT_ROOT, type INode, type IPinData, type IWorkflowBase } from 'n8n-workflow';
|
||||
@@ -67,6 +68,7 @@ const testServer = utils.setupTestServer({
|
||||
const { objectContaining, arrayContaining, any } = expect;
|
||||
|
||||
const activeWorkflowManagerLike = mockInstance(ActiveWorkflowManager);
|
||||
const workflowValidationService = mockInstance(WorkflowValidationService);
|
||||
|
||||
let projectRepository: ProjectRepository;
|
||||
let workflowRepository: WorkflowRepository;
|
||||
@@ -98,6 +100,8 @@ beforeEach(async () => {
|
||||
authMemberAgent = testServer.authAgentFor(member);
|
||||
anotherMember = await createMember();
|
||||
|
||||
workflowValidationService.validateForActivation.mockReturnValue({ isValid: true });
|
||||
|
||||
folderListMissingRole = await createCustomRoleWithScopeSlugs(['workflow:read', 'workflow:list'], {
|
||||
roleType: 'project',
|
||||
displayName: 'Workflow Read-Only',
|
||||
@@ -3113,14 +3117,14 @@ describe('POST /workflows/:workflowId/activate', () => {
|
||||
|
||||
const emitSpy = jest.spyOn(eventService, 'emit');
|
||||
|
||||
activeWorkflowManagerLike.add.mockRejectedValueOnce(new Error('Validation failed'));
|
||||
activeWorkflowManagerLike.add.mockRejectedValueOnce(new Error('Activation failed'));
|
||||
|
||||
const response = await authOwnerAgent
|
||||
.post(`/workflows/${workflow.id}/activate`)
|
||||
.send({ versionId: newVersionId });
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(response.body.message).toBe('Validation failed');
|
||||
expect(response.body.message).toBe('Activation failed');
|
||||
|
||||
const updatedWorkflow = await workflowRepository.findOne({
|
||||
where: { id: workflow.id },
|
||||
|
||||
@@ -214,7 +214,10 @@ export function useWorkflowActivate() {
|
||||
interpolate: { newStateName: 'published' },
|
||||
}) + ':',
|
||||
);
|
||||
workflowsStore.setWorkflowInactive(workflowId);
|
||||
// Only update workflow state to inactive if this is not a validation error
|
||||
if (!error.meta?.validationError) {
|
||||
workflowsStore.setWorkflowInactive(workflowId);
|
||||
}
|
||||
return false;
|
||||
} finally {
|
||||
updatingWorkflowActivation.value = false;
|
||||
|
||||
@@ -27,6 +27,7 @@ export * from './workflow';
|
||||
export * from './workflow-checksum';
|
||||
export * from './workflow-data-proxy';
|
||||
export * from './workflow-data-proxy-env-provider';
|
||||
export * from './workflow-validation';
|
||||
export * from './versioned-node-type';
|
||||
export * from './type-validation';
|
||||
export * from './result';
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
import type { INode, INodes, INodeType } from './interfaces';
|
||||
|
||||
export interface INodeTypesGetter {
|
||||
getByNameAndVersion(nodeType: string, version?: number): INodeType | undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that a workflow has at least one trigger-like node (trigger, webhook, or polling node).
|
||||
* A workflow can only be activated if it has a node that can start the workflow execution.
|
||||
*
|
||||
* @param nodes - The workflow nodes to validate
|
||||
* @param nodeTypes - Node types getter to retrieve node definitions
|
||||
* @param ignoreNodeTypes - Optional array of node types to ignore (e.g., manual trigger, start node)
|
||||
* @returns Object with isValid flag and error message if invalid
|
||||
*/
|
||||
export function validateWorkflowHasTriggerLikeNode(
|
||||
nodes: INodes,
|
||||
nodeTypes: INodeTypesGetter,
|
||||
ignoreNodeTypes?: string[],
|
||||
): { isValid: boolean; error?: string } {
|
||||
let node: INode;
|
||||
let nodeType: INodeType | undefined;
|
||||
|
||||
for (const nodeName of Object.keys(nodes)) {
|
||||
node = nodes[nodeName];
|
||||
|
||||
// Skip disabled nodes - they cannot trigger a run
|
||||
if (node.disabled === true) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Skip ignored node types (e.g., manual trigger, start node)
|
||||
if (ignoreNodeTypes?.includes(node.type)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
nodeType = nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
|
||||
|
||||
if (nodeType === undefined) {
|
||||
// Type is not known, skip validation
|
||||
continue;
|
||||
}
|
||||
|
||||
if (
|
||||
nodeType.poll !== undefined ||
|
||||
nodeType.trigger !== undefined ||
|
||||
nodeType.webhook !== undefined
|
||||
) {
|
||||
// Is a trigger node. So workflow can be activated.
|
||||
return { isValid: true };
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
isValid: false,
|
||||
error:
|
||||
'Workflow cannot be activated because it has no trigger node. At least one trigger, webhook, or polling node is required.',
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,71 @@
|
||||
import { vi } from 'vitest';
|
||||
|
||||
import type { INode, INodes, INodeType, INodeTypeDescription } from '../src/interfaces';
|
||||
import type { INodeTypesGetter } from '../src/workflow-validation';
|
||||
import { validateWorkflowHasTriggerLikeNode } from '../src/workflow-validation';
|
||||
|
||||
describe('validateWorkflowHasTriggerLikeNode', () => {
|
||||
const disabledNode = { type: 'triggerNode', disabled: true } as INode;
|
||||
const unknownNode = { type: 'unknownNode' } as INode;
|
||||
const noTriggersNode = { type: 'noTriggersNode' } as INode;
|
||||
const pollNode = { type: 'pollNode' } as INode;
|
||||
const triggerNode = { type: 'triggerNode' } as INode;
|
||||
const webhookNode = { type: 'webhookNode' } as INode;
|
||||
|
||||
const nodeTypes: INodeTypesGetter = {
|
||||
getByNameAndVersion: vi.fn(),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(nodeTypes.getByNameAndVersion).mockImplementation((type): INodeType | undefined => {
|
||||
if (type === 'unknownNode') return undefined;
|
||||
|
||||
const nodeType: Partial<INodeType> = {
|
||||
poll: undefined,
|
||||
trigger: undefined,
|
||||
webhook: undefined,
|
||||
description: {} as INodeTypeDescription,
|
||||
};
|
||||
|
||||
if (type === 'pollNode') nodeType.poll = vi.fn();
|
||||
if (type === 'triggerNode') nodeType.trigger = vi.fn();
|
||||
if (type === 'webhookNode') nodeType.webhook = vi.fn();
|
||||
|
||||
return nodeType as INodeType;
|
||||
});
|
||||
});
|
||||
|
||||
test.each([
|
||||
['should skip disabled nodes', { disabledNode }, [], false],
|
||||
['should skip nodes marked as ignored', { triggerNode }, ['triggerNode'], false],
|
||||
['should skip unknown nodes', { unknownNode }, [], false],
|
||||
['should skip nodes with no trigger method', { noTriggersNode }, [], false],
|
||||
['should activate if poll method exists', { pollNode }, [], true],
|
||||
['should activate if trigger method exists', { triggerNode }, [], true],
|
||||
['should activate if webhook method exists', { webhookNode }, [], true],
|
||||
[
|
||||
'should ignore multiple node types',
|
||||
{ triggerNode, webhookNode, pollNode },
|
||||
['triggerNode', 'webhookNode', 'pollNode'],
|
||||
false,
|
||||
],
|
||||
])('%s', (_, nodes: INodes, ignoredNodes: string[], expectedValid: boolean) => {
|
||||
const result = validateWorkflowHasTriggerLikeNode(nodes, nodeTypes, ignoredNodes);
|
||||
|
||||
expect(result.isValid).toBe(expectedValid);
|
||||
if (!expectedValid) {
|
||||
expect(result.error).toBeDefined();
|
||||
expect(result.error).toContain('no trigger node');
|
||||
}
|
||||
});
|
||||
|
||||
test('should return error message when no trigger nodes found', () => {
|
||||
const nodes: INodes = { noTriggersNode };
|
||||
const result = validateWorkflowHasTriggerLikeNode(nodes, nodeTypes);
|
||||
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toBe(
|
||||
'Workflow cannot be activated because it has no trigger node. At least one trigger, webhook, or polling node is required.',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user