diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index ca1a396968..f49fed11b4 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -410,9 +410,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { workflowId: this.workflowData.id, }); - if (this.mode === 'webhook' && config.getEnv('binaryDataManager.mode') !== 'default') { - await restoreBinaryDataId(fullRunData, this.executionId); - } + await restoreBinaryDataId(fullRunData, this.executionId, this.mode); const isManualMode = [this.mode, parentProcessMode].includes('manual'); diff --git a/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts b/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts index c43788c094..15c03e1b4d 100644 --- a/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts +++ b/packages/cli/src/executionLifecycleHooks/restoreBinaryDataId.ts @@ -1,13 +1,17 @@ import Container from 'typedi'; import { BinaryDataService } from 'n8n-core'; -import type { IRun } from 'n8n-workflow'; +import type { IRun, WorkflowExecuteMode } from 'n8n-workflow'; import type { BinaryData } from 'n8n-core'; +import config from '@/config'; /** * Whenever the execution ID is not available to the binary data service at the * time of writing a binary data file, its name is missing the execution ID. * This function restores the ID in the file name and run data reference. * + * This edge case can happen only for a Webhook node that accepts binary data, + * when the binary data manager is set to persist this binary data. + * * ```txt * filesystem-v2:workflows/123/executions/temp/binary_data/69055-83c4-4493-876a-9092c4708b9b -> * filesystem-v2:workflows/123/executions/390/binary_data/69055-83c4-4493-876a-9092c4708b9b @@ -16,7 +20,18 @@ import type { BinaryData } from 'n8n-core'; * s3:workflows/123/executions/390/binary_data/69055-83c4-4493-876a-9092c4708b9b * ``` */ -export async function restoreBinaryDataId(run: IRun, executionId: string) { +export async function restoreBinaryDataId( + run: IRun, + executionId: string, + workflowExecutionMode: WorkflowExecuteMode, +) { + if ( + workflowExecutionMode !== 'webhook' || + config.getEnv('binaryDataManager.mode') === 'default' + ) { + return; + } + const { runData } = run.data.resultData; const promises = Object.keys(runData).map(async (nodeName) => { diff --git a/packages/cli/test/unit/execution-lifecyle/restoreBinaryDataId.test.ts b/packages/cli/test/unit/execution-lifecyle/restoreBinaryDataId.test.ts new file mode 100644 index 0000000000..05d828100c --- /dev/null +++ b/packages/cli/test/unit/execution-lifecyle/restoreBinaryDataId.test.ts @@ -0,0 +1,170 @@ +import { restoreBinaryDataId } from '@/executionLifecycleHooks/restoreBinaryDataId'; +import { BinaryDataService } from 'n8n-core'; +import { mockInstance } from '../../shared/mocking'; + +import type { IRun } from 'n8n-workflow'; +import config from '@/config'; + +function toIRun(item?: object) { + return { + data: { + resultData: { + runData: { + myNode: [ + { + data: { + main: [[item]], + }, + }, + ], + }, + }, + }, + } as unknown as IRun; +} + +function getDataId(run: IRun, kind: 'binary' | 'json') { + // @ts-ignore + return run.data.resultData.runData.myNode[0].data.main[0][0][kind].data.id; +} + +const binaryDataService = mockInstance(BinaryDataService); + +for (const mode of ['filesystem-v2', 's3'] as const) { + describe(`on ${mode} mode`, () => { + beforeAll(() => { + config.set('binaryDataManager.mode', mode); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should restore if binary data ID is missing execution ID', async () => { + const workflowId = '6HYhhKmJch2cYxGj'; + const executionId = 'temp'; + const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6'; + + const incorrectFileId = `workflows/${workflowId}/executions/temp/binary_data/${binaryDataFileUuid}`; + + const run = toIRun({ + binary: { + data: { id: `s3:${incorrectFileId}` }, + }, + }); + + await restoreBinaryDataId(run, executionId, 'webhook'); + + const correctFileId = incorrectFileId.replace('temp', executionId); + const correctBinaryDataId = `s3:${correctFileId}`; + + expect(binaryDataService.rename).toHaveBeenCalledWith(incorrectFileId, correctFileId); + expect(getDataId(run, 'binary')).toBe(correctBinaryDataId); + }); + + it('should do nothing if binary data ID is not missing execution ID', async () => { + const workflowId = '6HYhhKmJch2cYxGj'; + const executionId = '999'; + const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6'; + + const fileId = `workflows/${workflowId}/executions/${executionId}/binary_data/${binaryDataFileUuid}`; + + const binaryDataId = `s3:${fileId}`; + + const run = toIRun({ + binary: { + data: { + id: binaryDataId, + }, + }, + }); + + await restoreBinaryDataId(run, executionId, 'webhook'); + + expect(binaryDataService.rename).not.toHaveBeenCalled(); + expect(getDataId(run, 'binary')).toBe(binaryDataId); + }); + + it('should do nothing if no binary data ID', async () => { + const executionId = '999'; + const dataId = '123'; + + const run = toIRun({ + json: { + data: { id: dataId }, + }, + }); + + await restoreBinaryDataId(run, executionId, 'webhook'); + + expect(binaryDataService.rename).not.toHaveBeenCalled(); + expect(getDataId(run, 'json')).toBe(dataId); + }); + + it('should do nothing on itemless case', async () => { + const executionId = '999'; + + const promise = restoreBinaryDataId(toIRun(), executionId, 'webhook'); + + await expect(promise).resolves.not.toThrow(); + + expect(binaryDataService.rename).not.toHaveBeenCalled(); + }); + + it('should do nothing if data is undefined', async () => { + const executionId = '999'; + + const run = toIRun({ + json: { + data: undefined, + }, + }); + + const promise = restoreBinaryDataId(run, executionId, 'webhook'); + + await expect(promise).resolves.not.toThrow(); + + expect(binaryDataService.rename).not.toHaveBeenCalled(); + }); + + it('should do nothing if workflow execution mode is not `webhook`', async () => { + const executionId = '999'; + + const run = toIRun({ + json: { + data: undefined, + }, + }); + + const promise = restoreBinaryDataId(run, executionId, 'internal'); + + await expect(promise).resolves.not.toThrow(); + + expect(binaryDataService.rename).not.toHaveBeenCalled(); + }); + }); +} + +describe('on default mode', () => { + afterEach(() => { + config.load(config.default); + }); + + it('should do nothing', async () => { + config.set('binaryDataManager.mode', 'default'); + + const executionId = '999'; + + const run = toIRun({ + json: { + data: undefined, + }, + }); + + const promise = restoreBinaryDataId(run, executionId, 'internal'); + + await expect(promise).resolves.not.toThrow(); + + expect(binaryDataService.rename).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/test/unit/execution.lifecycle.test.ts b/packages/cli/test/unit/execution.lifecycle.test.ts deleted file mode 100644 index c20bd07f6f..0000000000 --- a/packages/cli/test/unit/execution.lifecycle.test.ts +++ /dev/null @@ -1,142 +0,0 @@ -import { restoreBinaryDataId } from '@/executionLifecycleHooks/restoreBinaryDataId'; -import { BinaryDataService } from 'n8n-core'; -import { mockInstance } from '../shared/mocking'; - -import type { IRun } from 'n8n-workflow'; -import config from '@/config'; - -function toIRun(item?: object) { - return { - data: { - resultData: { - runData: { - myNode: [ - { - data: { - main: [[item]], - }, - }, - ], - }, - }, - }, - } as unknown as IRun; -} - -function getDataId(run: IRun, kind: 'binary' | 'json') { - // @ts-ignore - return run.data.resultData.runData.myNode[0].data.main[0][0][kind].data.id; -} - -const binaryDataService = mockInstance(BinaryDataService); - -for (const mode of ['filesystem-v2', 's3'] as const) { - describe(`on ${mode} mode`, () => { - describe('restoreBinaryDataId()', () => { - beforeAll(() => { - config.set('binaryDataManager.mode', mode); - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - it('should restore if binary data ID is missing execution ID', async () => { - const workflowId = '6HYhhKmJch2cYxGj'; - const executionId = 'temp'; - const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6'; - - const incorrectFileId = `workflows/${workflowId}/executions/temp/binary_data/${binaryDataFileUuid}`; - - const run = toIRun({ - binary: { - data: { id: `s3:${incorrectFileId}` }, - }, - }); - - await restoreBinaryDataId(run, executionId); - - const correctFileId = incorrectFileId.replace('temp', executionId); - const correctBinaryDataId = `s3:${correctFileId}`; - - expect(binaryDataService.rename).toHaveBeenCalledWith(incorrectFileId, correctFileId); - expect(getDataId(run, 'binary')).toBe(correctBinaryDataId); - }); - - it('should do nothing if binary data ID is not missing execution ID', async () => { - const workflowId = '6HYhhKmJch2cYxGj'; - const executionId = '999'; - const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6'; - - const fileId = `workflows/${workflowId}/executions/${executionId}/binary_data/${binaryDataFileUuid}`; - - const binaryDataId = `s3:${fileId}`; - - const run = toIRun({ - binary: { - data: { - id: binaryDataId, - }, - }, - }); - - await restoreBinaryDataId(run, executionId); - - expect(binaryDataService.rename).not.toHaveBeenCalled(); - expect(getDataId(run, 'binary')).toBe(binaryDataId); - }); - - it('should do nothing if no binary data ID', async () => { - const executionId = '999'; - const dataId = '123'; - - const run = toIRun({ - json: { - data: { id: dataId }, - }, - }); - - await restoreBinaryDataId(run, executionId); - - expect(binaryDataService.rename).not.toHaveBeenCalled(); - expect(getDataId(run, 'json')).toBe(dataId); - }); - - it('should do nothing on itemless case', async () => { - const executionId = '999'; - - const promise = restoreBinaryDataId(toIRun(), executionId); - - await expect(promise).resolves.not.toThrow(); - - expect(binaryDataService.rename).not.toHaveBeenCalled(); - }); - - it('should do nothing if data is undefined', async () => { - const executionId = '999'; - - const run = toIRun({ - json: { - data: undefined, - }, - }); - - const promise = restoreBinaryDataId(run, executionId); - - await expect(promise).resolves.not.toThrow(); - - expect(binaryDataService.rename).not.toHaveBeenCalled(); - }); - }); - }); - - it('should do nothing on itemless case', async () => { - const executionId = '999'; - - const promise = restoreBinaryDataId(toIRun(), executionId); - - await expect(promise).resolves.not.toThrow(); - - expect(binaryDataService.rename).not.toHaveBeenCalled(); - }); -}