From e833b1b1a3fbc3dab451f8fcb1bf89616f18e5b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 30 Jan 2025 14:30:52 +0100 Subject: [PATCH] stop passing pushRef and retryOf around --- .../execution-lifecycle-hooks.test.ts | 20 ++-- .../__tests__/save-execution-progress.test.ts | 3 +- .../execution-lifecycle-hooks.ts | 98 +++++++++---------- .../save-execution-progress.ts | 2 - packages/workflow/src/Interfaces.ts | 5 - packages/workflow/src/WorkflowHooks.ts | 18 +--- 6 files changed, 60 insertions(+), 86 deletions(-) diff --git a/packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts b/packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts index f3b862478f..54a3d2bd6a 100644 --- a/packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts +++ b/packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts @@ -201,8 +201,6 @@ describe('Execution Lifecycle Hooks', () => { expect(hooks.mode).toBe('manual'); expect(hooks.executionId).toBe(executionId); expect(hooks.workflowData).toEqual(workflowData); - expect(hooks.pushRef).toEqual('test-push-ref'); - expect(hooks.retryOf).toEqual('test-retry-of'); const { hookFunctions } = hooks; expect(hookFunctions.nodeExecuteBefore).toHaveLength(2); @@ -260,7 +258,7 @@ describe('Execution Lifecycle Hooks', () => { workflowData.settings = { saveExecutionProgress: false }; hooks = createHooks(); - expect(hooks.hookFunctions.nodeExecuteAfter).toHaveLength(3); + expect(hooks.hookFunctions.nodeExecuteAfter).toHaveLength(2); await hooks.executeHookFunctions('nodeExecuteAfter', [ nodeName, @@ -509,10 +507,16 @@ describe('Execution Lifecycle Hooks', () => { describe("when pushRef isn't set", () => { beforeEach(() => { - hooks = getWorkflowHooksMain({ executionMode, workflowData }, executionId); + hooks = getWorkflowHooksMain({ executionMode, workflowData, retryOf }, executionId); }); - it('should not send any push events', async () => { + it('should not setup any push hooks', async () => { + const { hookFunctions } = hooks; + expect(hookFunctions.nodeExecuteBefore).toHaveLength(1); + expect(hookFunctions.nodeExecuteAfter).toHaveLength(1); + expect(hookFunctions.workflowExecuteBefore).toHaveLength(2); + expect(hookFunctions.workflowExecuteAfter).toHaveLength(4); + await hooks.executeHookFunctions('nodeExecuteBefore', [nodeName]); await hooks.executeHookFunctions('nodeExecuteAfter', [ nodeName, @@ -543,8 +547,6 @@ describe('Execution Lifecycle Hooks', () => { expect(hooks.mode).toBe('manual'); expect(hooks.executionId).toBe(executionId); expect(hooks.workflowData).toEqual(workflowData); - expect(hooks.pushRef).toEqual('test-push-ref'); - expect(hooks.retryOf).toEqual('test-retry-of'); const { hookFunctions } = hooks; expect(hookFunctions.nodeExecuteBefore).toHaveLength(0); @@ -621,8 +623,6 @@ describe('Execution Lifecycle Hooks', () => { expect(hooks.mode).toBe('manual'); expect(hooks.executionId).toBe(executionId); expect(hooks.workflowData).toEqual(workflowData); - expect(hooks.pushRef).toEqual('test-push-ref'); - expect(hooks.retryOf).toEqual('test-retry-of'); const { hookFunctions } = hooks; expect(hookFunctions.nodeExecuteBefore).toHaveLength(2); @@ -718,8 +718,6 @@ describe('Execution Lifecycle Hooks', () => { expect(hooks.mode).toBe('manual'); expect(hooks.executionId).toBe(executionId); expect(hooks.workflowData).toEqual(workflowData); - expect(hooks.pushRef).toBeUndefined(); - expect(hooks.retryOf).toBeUndefined(); const { hookFunctions } = hooks; expect(hookFunctions.nodeExecuteBefore).toHaveLength(1); diff --git a/packages/cli/src/execution-lifecycle/__tests__/save-execution-progress.test.ts b/packages/cli/src/execution-lifecycle/__tests__/save-execution-progress.test.ts index 78887a4433..14692ee4ae 100644 --- a/packages/cli/src/execution-lifecycle/__tests__/save-execution-progress.test.ts +++ b/packages/cli/src/execution-lifecycle/__tests__/save-execution-progress.test.ts @@ -16,13 +16,12 @@ afterEach(() => { jest.clearAllMocks(); }); -const commonArgs: [string, string, string, ITaskData, IRunExecutionData, string] = [ +const commonArgs: [string, string, string, ITaskData, IRunExecutionData] = [ 'some-workflow-id', 'some-execution-id', 'My Node', {} as ITaskData, {} as IRunExecutionData, - 'some-session-id', ]; test('should ignore on leftover async call', async () => { diff --git a/packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts b/packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts index 012995e1f8..ac27c679d3 100644 --- a/packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts +++ b/packages/cli/src/execution-lifecycle/execution-lifecycle-hooks.ts @@ -10,7 +10,6 @@ import type { ITaskData, IWorkflowBase, IWorkflowExecuteHooks, - IWorkflowHooksOptionalParameters, WorkflowExecuteMode, IWorkflowExecutionDataProcess, Workflow, @@ -34,6 +33,12 @@ import { } from './shared/shared-hook-functions'; import { type ExecutionSavingSettings, toSaveSettings } from './to-save-settings'; +type HooksSetupParameters = { + saveSettings: ExecutionSavingSettings; + pushRef?: string; + retryOf?: string; +}; + function mergeHookFunctions(...hookFunctions: IWorkflowExecuteHooks[]): IWorkflowExecuteHooks { const result: IWorkflowExecuteHooks = { nodeExecuteBefore: [], @@ -91,19 +96,16 @@ function hookFunctionsNodeEvents(): IWorkflowExecuteHooks { /** * Returns hook functions to push data to Editor-UI */ -function hookFunctionsPush(): IWorkflowExecuteHooks { +function hookFunctionsPush({ pushRef, retryOf }: HooksSetupParameters): IWorkflowExecuteHooks { + if (!pushRef) return {}; const logger = Container.get(Logger); const pushInstance = Container.get(Push); return { nodeExecuteBefore: [ async function (this: WorkflowHooks, nodeName: string): Promise { - const { pushRef, executionId } = this; + const { executionId } = this; // Push data to session which started workflow before each // node which starts rendering - if (pushRef === undefined) { - return; - } - logger.debug(`Executing hook on node "${nodeName}" (hookFunctionsPush)`, { executionId, pushRef, @@ -115,12 +117,8 @@ function hookFunctionsPush(): IWorkflowExecuteHooks { ], nodeExecuteAfter: [ async function (this: WorkflowHooks, nodeName: string, data: ITaskData): Promise { - const { pushRef, executionId } = this; + const { executionId } = this; // Push data to session which started workflow after each rendered node - if (pushRef === undefined) { - return; - } - logger.debug(`Executing hook on node "${nodeName}" (hookFunctionsPush)`, { executionId, pushRef, @@ -135,7 +133,7 @@ function hookFunctionsPush(): IWorkflowExecuteHooks { ], workflowExecuteBefore: [ async function (this: WorkflowHooks, _workflow, data): Promise { - const { pushRef, executionId } = this; + const { executionId } = this; const { id: workflowId, name: workflowName } = this.workflowData; logger.debug('Executing hook (hookFunctionsPush)', { executionId, @@ -143,9 +141,6 @@ function hookFunctionsPush(): IWorkflowExecuteHooks { workflowId, }); // Push data to session which started the workflow - if (pushRef === undefined) { - return; - } pushInstance.send( { type: 'executionStarted', @@ -153,7 +148,7 @@ function hookFunctionsPush(): IWorkflowExecuteHooks { executionId, mode: this.mode, startedAt: new Date(), - retryOf: this.retryOf, + retryOf, workflowId, workflowName, flattedRunData: data?.resultData.runData @@ -167,9 +162,7 @@ function hookFunctionsPush(): IWorkflowExecuteHooks { ], workflowExecuteAfter: [ async function (this: WorkflowHooks, fullRunData: IRun): Promise { - const { pushRef, executionId } = this; - if (pushRef === undefined) return; - + const { executionId } = this; const { id: workflowId } = this.workflowData; logger.debug('Executing hook (hookFunctionsPush)', { executionId, @@ -212,7 +205,7 @@ function hookFunctionsExternalHooks(): IWorkflowExecuteHooks { }; } -function hookFunctionsSaveProgress(saveSettings: ExecutionSavingSettings): IWorkflowExecuteHooks { +function hookFunctionsSaveProgress({ saveSettings }: HooksSetupParameters): IWorkflowExecuteHooks { if (!saveSettings.progress) return {}; return { nodeExecuteAfter: [ @@ -228,7 +221,6 @@ function hookFunctionsSaveProgress(saveSettings: ExecutionSavingSettings): IWork nodeName, data, executionData, - this.pushRef, ); }, ], @@ -249,7 +241,11 @@ function hookFunctionsFinalizeExecutionStatus(): IWorkflowExecuteHooks { /** * Returns hook functions to save workflow execution and call error workflow */ -function hookFunctionsSave(saveSettings: ExecutionSavingSettings): IWorkflowExecuteHooks { +function hookFunctionsSave({ + pushRef, + retryOf, + saveSettings, +}: HooksSetupParameters): IWorkflowExecuteHooks { const logger = Container.get(Logger); const errorReporter = Container.get(ErrorReporter); const executionRepository = Container.get(ExecutionRepository); @@ -314,7 +310,7 @@ function hookFunctionsSave(saveSettings: ExecutionSavingSettings): IWorkflowExec fullRunData, this.mode, this.executionId, - this.retryOf, + retryOf, ); await executionRepository.hardDelete({ @@ -331,12 +327,12 @@ function hookFunctionsSave(saveSettings: ExecutionSavingSettings): IWorkflowExec runData: fullRunData, workflowData: this.workflowData, workflowStatusFinal: fullRunData.status, - retryOf: this.retryOf, + retryOf, }); // When going into the waiting state, store the pushRef in the execution-data if (fullRunData.waitTill && isManualMode) { - fullExecutionData.data.pushRef = this.pushRef; + fullExecutionData.data.pushRef = pushRef; } await updateExistingExecution({ @@ -351,7 +347,7 @@ function hookFunctionsSave(saveSettings: ExecutionSavingSettings): IWorkflowExec fullRunData, this.mode, this.executionId, - this.retryOf, + retryOf, ); } } finally { @@ -375,7 +371,10 @@ function hookFunctionsSave(saveSettings: ExecutionSavingSettings): IWorkflowExec * for running with queues. Manual executions should never run on queues as * they are always executed in the main process. */ -function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { +function hookFunctionsSaveWorker({ + pushRef, + retryOf, +}: HooksSetupParameters): IWorkflowExecuteHooks { const logger = Container.get(Logger); const errorReporter = Container.get(ErrorReporter); const workflowStaticDataService = Container.get(WorkflowStaticDataService); @@ -407,7 +406,7 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { logger.error( // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access `There was a problem saving the workflow with id "${this.workflowData.id}" to save changed staticData: "${e.message}" (workflowExecuteAfter)`, - { pushRef: this.pushRef, workflowId: this.workflowData.id }, + { workflowId: this.workflowData.id }, ); } } @@ -422,7 +421,7 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { fullRunData, this.mode, this.executionId, - this.retryOf, + retryOf, ); } @@ -432,12 +431,12 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { runData: fullRunData, workflowData: this.workflowData, workflowStatusFinal: fullRunData.status, - retryOf: this.retryOf, + retryOf, }); // When going into the waiting state, store the pushRef in the execution-data if (fullRunData.waitTill && isManualMode) { - fullExecutionData.data.pushRef = this.pushRef; + fullExecutionData.data.pushRef = pushRef; } await updateExistingExecution({ @@ -476,8 +475,8 @@ export function getWorkflowHooksIntegrated( hookFunctionsWorkflowEvents(userId), hookFunctionsNodeEvents(), hookFunctionsFinalizeExecutionStatus(), - hookFunctionsSave(saveSettings), - hookFunctionsSaveProgress(saveSettings), + hookFunctionsSave({ saveSettings }), + hookFunctionsSaveProgress({ saveSettings }), hookFunctionsExternalHooks(), ); return new WorkflowHooks(hookFunctions, mode, executionId, workflowData); @@ -490,23 +489,24 @@ export function getWorkflowHooksWorkerExecuter( mode: WorkflowExecuteMode, executionId: string, workflowData: IWorkflowBase, - optionalParameters: IWorkflowHooksOptionalParameters = {}, + { pushRef, retryOf }: Omit = {}, ): WorkflowHooks { const saveSettings = toSaveSettings(workflowData.settings); + const optionalParameters = { pushRef, retryOf, saveSettings }; const toMerge = [ hookFunctionsNodeEvents(), hookFunctionsFinalizeExecutionStatus(), - hookFunctionsSaveWorker(), - hookFunctionsSaveProgress(saveSettings), + hookFunctionsSaveWorker(optionalParameters), + hookFunctionsSaveProgress(optionalParameters), hookFunctionsExternalHooks(), ]; if (mode === 'manual' && Container.get(InstanceSettings).isWorker) { - toMerge.push(hookFunctionsPush()); + toMerge.push(hookFunctionsPush(optionalParameters)); } const hookFunctions = mergeHookFunctions(...toMerge); - return new WorkflowHooks(hookFunctions, mode, executionId, workflowData, optionalParameters); + return new WorkflowHooks(hookFunctions, mode, executionId, workflowData); } /** @@ -516,13 +516,14 @@ export function getWorkflowHooksWorkerMain( mode: WorkflowExecuteMode, executionId: string, workflowData: IWorkflowBase, - optionalParameters: IWorkflowHooksOptionalParameters = {}, + { pushRef, retryOf }: Omit = {}, ): WorkflowHooks { const saveSettings = toSaveSettings(workflowData.settings); + const optionalParameters = { pushRef, retryOf, saveSettings }; const executionRepository = Container.get(ExecutionRepository); const hookFunctions = mergeHookFunctions( hookFunctionsWorkflowEvents(), - hookFunctionsSaveProgress(saveSettings), + hookFunctionsSaveProgress(optionalParameters), hookFunctionsExternalHooks(), hookFunctionsFinalizeExecutionStatus(), { @@ -569,7 +570,7 @@ export function getWorkflowHooksWorkerMain( hookFunctions.nodeExecuteBefore = []; hookFunctions.nodeExecuteAfter = []; - return new WorkflowHooks(hookFunctions, mode, executionId, workflowData, optionalParameters); + return new WorkflowHooks(hookFunctions, mode, executionId, workflowData); } /** @@ -579,18 +580,17 @@ export function getWorkflowHooksMain( data: IWorkflowExecutionDataProcess, executionId: string, ): WorkflowHooks { + const { pushRef, retryOf } = data; const saveSettings = toSaveSettings(data.workflowData.settings); + const optionalParameters = { pushRef, retryOf, saveSettings }; const hookFunctions = mergeHookFunctions( hookFunctionsWorkflowEvents(), hookFunctionsNodeEvents(), hookFunctionsFinalizeExecutionStatus(), - hookFunctionsSave(saveSettings), - hookFunctionsPush(), - hookFunctionsSaveProgress(saveSettings), + hookFunctionsSave(optionalParameters), + hookFunctionsPush(optionalParameters), + hookFunctionsSaveProgress(optionalParameters), hookFunctionsExternalHooks(), ); - return new WorkflowHooks(hookFunctions, data.executionMode, executionId, data.workflowData, { - pushRef: data.pushRef, - retryOf: data.retryOf as string, - }); + return new WorkflowHooks(hookFunctions, data.executionMode, executionId, data.workflowData); } diff --git a/packages/cli/src/execution-lifecycle/save-execution-progress.ts b/packages/cli/src/execution-lifecycle/save-execution-progress.ts index eea94a5420..f70cf28430 100644 --- a/packages/cli/src/execution-lifecycle/save-execution-progress.ts +++ b/packages/cli/src/execution-lifecycle/save-execution-progress.ts @@ -10,7 +10,6 @@ export async function saveExecutionProgress( nodeName: string, data: ITaskData, executionData: IRunExecutionData, - pushRef?: string, ) { const logger = Container.get(Logger); @@ -90,7 +89,6 @@ export async function saveExecutionProgress( { ...error, executionId, - pushRef, workflowId, }, ); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 0d67b07f37..541793a579 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -2409,11 +2409,6 @@ export type WorkflowActivateMode = | 'manual' // unused | 'leadershipChange'; -export interface IWorkflowHooksOptionalParameters { - retryOf?: string; - pushRef?: string; -} - export namespace WorkflowSettings { export type CallerPolicy = 'any' | 'none' | 'workflowsFromAList' | 'workflowsFromSameOwner'; export type SaveDataExecution = 'DEFAULT' | 'all' | 'none'; diff --git a/packages/workflow/src/WorkflowHooks.ts b/packages/workflow/src/WorkflowHooks.ts index feb7a46e20..d7e05b9a1a 100644 --- a/packages/workflow/src/WorkflowHooks.ts +++ b/packages/workflow/src/WorkflowHooks.ts @@ -1,9 +1,4 @@ -import type { - IWorkflowBase, - IWorkflowExecuteHooks, - IWorkflowHooksOptionalParameters, - WorkflowExecuteMode, -} from './Interfaces'; +import type { IWorkflowBase, IWorkflowExecuteHooks, WorkflowExecuteMode } from './Interfaces'; export class WorkflowHooks { mode: WorkflowExecuteMode; @@ -12,10 +7,6 @@ export class WorkflowHooks { executionId: string; - pushRef?: string; - - retryOf?: string; - hookFunctions: IWorkflowExecuteHooks; constructor( @@ -23,18 +14,11 @@ export class WorkflowHooks { mode: WorkflowExecuteMode, executionId: string, workflowData: IWorkflowBase, - optionalParameters?: IWorkflowHooksOptionalParameters, ) { - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - optionalParameters = optionalParameters || {}; - this.hookFunctions = hookFunctions; this.mode = mode; this.executionId = executionId; this.workflowData = workflowData; - this.pushRef = optionalParameters.pushRef; - // retryOf might be `null` from TypeORM - this.retryOf = optionalParameters.retryOf ?? undefined; } // eslint-disable-next-line @typescript-eslint/no-explicit-any