mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
fix(core): Re-register the n8n Trigger on every publish through the publication service (#38153)
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
a234034ca1
commit
cbda01aa6d
|
|
@ -85,4 +85,44 @@ describe('computeTriggerDiff', () => {
|
|||
test('returns empty diff for two empty trigger sets', () => {
|
||||
expect(computeTriggerDiff([], [])).toEqual({ toAdd: new Set(), toRemove: new Set() });
|
||||
});
|
||||
|
||||
describe('triggers that observe the publication itself', () => {
|
||||
const n8nTrigger = makeNode('n8n', {
|
||||
type: 'n8n-nodes-base.n8nTrigger',
|
||||
parameters: { events: ['update'] },
|
||||
});
|
||||
|
||||
test('re-registers an unchanged n8n Trigger when the published version changed', () => {
|
||||
const diff = computeTriggerDiff([n8nTrigger], [{ ...n8nTrigger }], { versionChanged: true });
|
||||
|
||||
expect(diff).toEqual({ toAdd: new Set(['n8n']), toRemove: new Set(['n8n']) });
|
||||
});
|
||||
|
||||
test('leaves an unchanged n8n Trigger running when the published version is the same', () => {
|
||||
const diff = computeTriggerDiff([n8nTrigger], [{ ...n8nTrigger }], { versionChanged: false });
|
||||
|
||||
expect(diff).toEqual({ toAdd: new Set(), toRemove: new Set() });
|
||||
});
|
||||
|
||||
test('re-registers the deprecated Workflow Trigger the same way', () => {
|
||||
const workflowTrigger = makeNode('legacy', {
|
||||
type: 'n8n-nodes-base.workflowTrigger',
|
||||
parameters: { events: ['update'] },
|
||||
});
|
||||
|
||||
const diff = computeTriggerDiff([workflowTrigger], [{ ...workflowTrigger }], {
|
||||
versionChanged: true,
|
||||
});
|
||||
|
||||
expect(diff).toEqual({ toAdd: new Set(['legacy']), toRemove: new Set(['legacy']) });
|
||||
});
|
||||
|
||||
test('does not force other unchanged trigger types when the published version changed', () => {
|
||||
const schedule = makeNode('a');
|
||||
|
||||
const diff = computeTriggerDiff([schedule], [{ ...schedule }], { versionChanged: true });
|
||||
|
||||
expect(diff).toEqual({ toAdd: new Set(), toRemove: new Set() });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -757,6 +757,51 @@ describe('WorkflowPublicationApplier', () => {
|
|||
expect(callOrder).toEqual(['remove', 'invalidate', 'advance', 'refresh', 'add']);
|
||||
});
|
||||
|
||||
describe('n8n Trigger', () => {
|
||||
// Emits "Published Workflow Updated" from `trigger()` itself, so it only fires
|
||||
// when re-registered: the diff must re-apply it on every version change.
|
||||
const n8nTrigger = triggerNode('n8n', {
|
||||
type: 'n8n-nodes-base.n8nTrigger',
|
||||
parameters: { events: ['update'] },
|
||||
});
|
||||
|
||||
test('re-applies an unchanged n8n Trigger when the published version changes', async () => {
|
||||
setTriggerSets([n8nTrigger], [{ ...n8nTrigger }]);
|
||||
workflowTriggerActivator.activate.mockResolvedValue({ activated: ['n8n'], failures: [] });
|
||||
|
||||
const result = await applier.apply(makeRecord(), abort);
|
||||
|
||||
expect(result.type).toBe('completed');
|
||||
expect(workflowTriggerActivator.deactivate).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: 'wf-1' }),
|
||||
oldVersion,
|
||||
new Set(['n8n']),
|
||||
abort,
|
||||
);
|
||||
expect(workflowTriggerActivator.activate).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: 'wf-1' }),
|
||||
newVersion,
|
||||
new Set(['n8n']),
|
||||
'update',
|
||||
abort,
|
||||
);
|
||||
});
|
||||
|
||||
test('leaves an unchanged n8n Trigger running when the record targets the already-published version', async () => {
|
||||
setTriggerSets([n8nTrigger], [{ ...n8nTrigger }]);
|
||||
workflowRepository.findOneBy.mockResolvedValue(makeWorkflow({ activeVersionId: 'v-2' }));
|
||||
workflowPublishedVersionRepository.findOne.mockResolvedValue(
|
||||
makePublishedVersion(newVersion),
|
||||
);
|
||||
|
||||
const result = await applier.apply(makeRecord(), abort);
|
||||
|
||||
expect(result.type).toBe('completed');
|
||||
expect(workflowTriggerActivator.deactivate).not.toHaveBeenCalled();
|
||||
expect(workflowTriggerActivator.activate).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
test('completes carrying external teardown failures from removed triggers, still advancing', async () => {
|
||||
setTriggerSets([triggerNode('a'), triggerNode('b')], [triggerNode('a')]);
|
||||
const failure = { nodeName: 'b', error: new Error('remote unreachable') };
|
||||
|
|
|
|||
|
|
@ -12,6 +12,20 @@ function registrationEqual(base: INode | undefined, target: INode | undefined):
|
|||
return isEqual(pick(base, registrationProps), pick(target, registrationProps));
|
||||
}
|
||||
|
||||
// Before the workflow publication service, we re-registered every trigger on
|
||||
// publication. With the new flow, we only re-register triggers that have been
|
||||
// modified. However, some triggers relied on the old behaviour, so we force it
|
||||
// for those triggers.
|
||||
const ALWAYS_REREGISTER_TRIGGER_TYPES: ReadonlySet<string> = new Set([
|
||||
'n8n-nodes-base.n8nTrigger',
|
||||
'n8n-nodes-base.workflowTrigger',
|
||||
]);
|
||||
|
||||
export interface TriggerDiffOptions {
|
||||
/** Whether the publish moves to a version other than the one currently running. */
|
||||
versionChanged?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* The trigger nodes that need to be deregistered (`toRemove`) and registered
|
||||
* (`toAdd`) to move a workflow's active triggers from `oldTriggerNodes` to
|
||||
|
|
@ -32,13 +46,14 @@ export interface TriggerDiff {
|
|||
export function computeTriggerDiff(
|
||||
oldTriggerNodes: INode[],
|
||||
newTriggerNodes: INode[],
|
||||
{ versionChanged = false }: TriggerDiffOptions = {},
|
||||
): TriggerDiff {
|
||||
const diff = compareWorkflowsNodes(oldTriggerNodes, newTriggerNodes, registrationEqual);
|
||||
|
||||
const toAdd: Set<INode['id']> = new Set();
|
||||
const toRemove: Set<INode['id']> = new Set();
|
||||
|
||||
for (const [nodeId, { status }] of diff) {
|
||||
for (const [nodeId, { status, node }] of diff) {
|
||||
switch (status) {
|
||||
case NodeDiffStatus.Added:
|
||||
toAdd.add(nodeId);
|
||||
|
|
@ -51,6 +66,10 @@ export function computeTriggerDiff(
|
|||
toAdd.add(nodeId);
|
||||
break;
|
||||
case NodeDiffStatus.Eq:
|
||||
if (versionChanged && ALWAYS_REREGISTER_TRIGGER_TYPES.has(node.type)) {
|
||||
toRemove.add(nodeId);
|
||||
toAdd.add(nodeId);
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -149,7 +149,9 @@ export class WorkflowPublicationApplier {
|
|||
const desiredTriggerNodes = this.workflowTriggerActivator.getEnabledTriggerNodes(newVersion);
|
||||
const triggerKinds = this.workflowTriggerActivator.getTriggerKinds(desiredTriggerNodes);
|
||||
|
||||
const { toAdd, toRemove } = computeTriggerDiff(oldTriggerNodes, desiredTriggerNodes);
|
||||
const { toAdd, toRemove } = computeTriggerDiff(oldTriggerNodes, desiredTriggerNodes, {
|
||||
versionChanged: oldVersion !== null && oldVersion.versionId !== newVersion.versionId,
|
||||
});
|
||||
|
||||
this.logger.debug(
|
||||
`Calculated trigger diff for workflow publication: ${toAdd.size} to add, ${toRemove.size} to remove`,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,21 @@ import { nanoid } from 'nanoid';
|
|||
|
||||
import { test, expect } from '../../../fixtures/base';
|
||||
|
||||
// The node emits from `trigger()` at registration time, so its events depend on
|
||||
// how the publication path (re)registers triggers. Run against the publication
|
||||
// service, which becomes the default.
|
||||
test.use({
|
||||
capability: {
|
||||
env: {
|
||||
TEST_ISOLATION: 'n8n-trigger-publication-service',
|
||||
N8N_USE_WORKFLOW_PUBLICATION_SERVICE: 'true',
|
||||
// Activation is applied asynchronously by the publication outbox
|
||||
// consumer, so poll frequently to keep the test fast.
|
||||
N8N_WORKFLOW_PUBLICATION_OUTBOX_POLL_INTERVAL_MS: '250',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
type TriggerEventType = 'activate' | 'update';
|
||||
|
||||
const makeN8nTriggerWorkflow = (events: TriggerEventType[]) => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user