From ccd2564cd4fac64da6de726de45827776bfa36a2 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: Wed, 6 Nov 2024 10:36:21 +0100 Subject: [PATCH 01/38] refactor(core): Extract execute-single context out of NodeExecutionFunctions (no-changelog) (#11543) --- packages/core/src/NodeExecuteFunctions.ts | 161 ++-------- .../__tests__/execute-single-context.test.ts | 301 ++++++++++++++++++ .../execute-single-context.ts | 212 ++++++++++++ .../core/src/node-execution-context/index.ts | 1 + packages/workflow/src/RoutingNode.ts | 40 +-- 5 files changed, 555 insertions(+), 160 deletions(-) create mode 100644 packages/core/src/node-execution-context/__tests__/execute-single-context.test.ts create mode 100644 packages/core/src/node-execution-context/execute-single-context.ts diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index e50d049f59..383624b569 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -76,7 +76,6 @@ import type { IPollFunctions, IRequestOptions, IRunExecutionData, - ISourceData, ITaskData, ITaskDataConnections, ITriggerFunctions, @@ -166,7 +165,13 @@ import { extractValue } from './ExtractValue'; import { InstanceSettings } from './InstanceSettings'; import type { ExtendedValidationResult, IResponseError } from './Interfaces'; // eslint-disable-next-line import/no-cycle -import { HookContext, PollContext, TriggerContext, WebhookContext } from './node-execution-context'; +import { + ExecuteSingleContext, + HookContext, + PollContext, + TriggerContext, + WebhookContext, +} from './node-execution-context'; import { getSecretsProxy } from './Secrets'; import { SSHClientsManager } from './SSHClientsManager'; @@ -4180,145 +4185,19 @@ export function getExecuteSingleFunctions( mode: WorkflowExecuteMode, abortSignal?: AbortSignal, ): IExecuteSingleFunctions { - return ((workflow, runExecutionData, connectionInputData, inputData, node, itemIndex) => { - return { - ...getCommonWorkflowFunctions(workflow, node, additionalData), - ...executionCancellationFunctions(abortSignal), - continueOnFail: () => continueOnFail(node), - evaluateExpression: (expression: string, evaluateItemIndex: number | undefined) => { - evaluateItemIndex = evaluateItemIndex === undefined ? itemIndex : evaluateItemIndex; - return workflow.expression.resolveSimpleParameterValue( - `=${expression}`, - {}, - runExecutionData, - runIndex, - evaluateItemIndex, - node.name, - connectionInputData, - mode, - getAdditionalKeys(additionalData, mode, runExecutionData), - executeData, - ); - }, - getContext(type: ContextType): IContextObject { - return NodeHelpers.getContext(runExecutionData, type, node); - }, - getCredentials: async (type) => - await getCredentials( - workflow, - node, - type, - additionalData, - mode, - executeData, - runExecutionData, - runIndex, - connectionInputData, - itemIndex, - ), - getInputData: (inputIndex = 0, inputName = 'main') => { - if (!inputData.hasOwnProperty(inputName)) { - // Return empty array because else it would throw error when nothing is connected to input - return { json: {} }; - } - - // TODO: Check if nodeType has input with that index defined - if (inputData[inputName].length < inputIndex) { - throw new ApplicationError('Could not get input index', { - extra: { inputIndex, inputName }, - }); - } - - const allItems = inputData[inputName][inputIndex]; - - if (allItems === null) { - throw new ApplicationError('Input index was not set', { - extra: { inputIndex, inputName }, - }); - } - - if (allItems[itemIndex] === null) { - throw new ApplicationError('Value of input with given index was not set', { - extra: { inputIndex, inputName, itemIndex }, - }); - } - - return allItems[itemIndex]; - }, - getInputSourceData: (inputIndex = 0, inputName = 'main') => { - if (executeData?.source === null) { - // Should never happen as n8n sets it automatically - throw new ApplicationError('Source data is missing'); - } - return executeData.source[inputName][inputIndex] as ISourceData; - }, - getItemIndex: () => itemIndex, - getMode: () => mode, - getExecuteData: () => executeData, - getNodeParameter: ( - parameterName: string, - fallbackValue?: any, - options?: IGetNodeParameterOptions, - ): NodeParameterValueType | object => { - return getNodeParameter( - workflow, - runExecutionData, - runIndex, - connectionInputData, - node, - parameterName, - itemIndex, - mode, - getAdditionalKeys(additionalData, mode, runExecutionData), - executeData, - fallbackValue, - options, - ); - }, - getWorkflowDataProxy: (): IWorkflowDataProxyData => { - const dataProxy = new WorkflowDataProxy( - workflow, - runExecutionData, - runIndex, - itemIndex, - node.name, - connectionInputData, - {}, - mode, - getAdditionalKeys(additionalData, mode, runExecutionData), - executeData, - ); - return dataProxy.getDataProxy(); - }, - helpers: { - createDeferredPromise, - returnJsonArray, - ...getRequestHelperFunctions( - workflow, - node, - additionalData, - runExecutionData, - connectionInputData, - ), - ...getBinaryHelperFunctions(additionalData, workflow.id), - - assertBinaryData: (propertyName, inputIndex = 0) => - assertBinaryData(inputData, node, itemIndex, propertyName, inputIndex), - getBinaryDataBuffer: async (propertyName, inputIndex = 0) => - await getBinaryDataBuffer(inputData, itemIndex, propertyName, inputIndex), - }, - logAiEvent: (eventName: AiEvent, msg: string) => { - return additionalData.logAiEvent(eventName, { - executionId: additionalData.executionId ?? 'unsaved-execution', - nodeName: node.name, - workflowName: workflow.name ?? 'Unnamed workflow', - nodeType: node.type, - workflowId: workflow.id ?? 'unsaved-workflow', - msg, - }); - }, - }; - })(workflow, runExecutionData, connectionInputData, inputData, node, itemIndex); + return new ExecuteSingleContext( + workflow, + node, + additionalData, + mode, + runExecutionData, + runIndex, + connectionInputData, + inputData, + itemIndex, + executeData, + abortSignal, + ); } export function getCredentialTestFunctions(): ICredentialTestFunctions { diff --git a/packages/core/src/node-execution-context/__tests__/execute-single-context.test.ts b/packages/core/src/node-execution-context/__tests__/execute-single-context.test.ts new file mode 100644 index 0000000000..dcd8509c1d --- /dev/null +++ b/packages/core/src/node-execution-context/__tests__/execute-single-context.test.ts @@ -0,0 +1,301 @@ +import { mock } from 'jest-mock-extended'; +import type { + INode, + IWorkflowExecuteAdditionalData, + IRunExecutionData, + INodeExecutionData, + ITaskDataConnections, + IExecuteData, + Workflow, + WorkflowExecuteMode, + ICredentialsHelper, + Expression, + INodeType, + INodeTypes, + OnError, + ContextType, + IContextObject, + ICredentialDataDecryptedObject, + ISourceData, +} from 'n8n-workflow'; +import { ApplicationError, NodeHelpers } from 'n8n-workflow'; + +import { ExecuteSingleContext } from '../execute-single-context'; + +describe('ExecuteSingleContext', () => { + const testCredentialType = 'testCredential'; + const nodeType = mock({ + description: { + credentials: [ + { + name: testCredentialType, + required: true, + }, + ], + properties: [ + { + name: 'testParameter', + required: true, + }, + ], + }, + }); + const nodeTypes = mock(); + const expression = mock(); + const workflow = mock({ expression, nodeTypes }); + const node = mock({ + credentials: { + [testCredentialType]: { + id: 'testCredentialId', + }, + }, + }); + node.parameters = { + testParameter: 'testValue', + }; + const credentialsHelper = mock(); + const additionalData = mock({ credentialsHelper }); + const mode: WorkflowExecuteMode = 'manual'; + const runExecutionData = mock(); + const connectionInputData = mock(); + const inputData: ITaskDataConnections = { main: [[{ json: { test: 'data' } }]] }; + const executeData = mock(); + const runIndex = 0; + const itemIndex = 0; + const abortSignal = mock(); + + const executeSingleContext = new ExecuteSingleContext( + workflow, + node, + additionalData, + mode, + runExecutionData, + runIndex, + connectionInputData, + inputData, + itemIndex, + executeData, + abortSignal, + ); + + beforeEach(() => { + nodeTypes.getByNameAndVersion.mockReturnValue(nodeType); + expression.getParameterValue.mockImplementation((value) => value); + }); + + describe('getExecutionCancelSignal', () => { + it('should return the abort signal', () => { + expect(executeSingleContext.getExecutionCancelSignal()).toBe(abortSignal); + }); + }); + + describe('continueOnFail', () => { + afterEach(() => { + node.onError = undefined; + node.continueOnFail = false; + }); + + it('should return false for nodes by default', () => { + expect(executeSingleContext.continueOnFail()).toEqual(false); + }); + + it('should return true if node has continueOnFail set to true', () => { + node.continueOnFail = true; + expect(executeSingleContext.continueOnFail()).toEqual(true); + }); + + test.each([ + ['continueRegularOutput', true], + ['continueErrorOutput', true], + ['stopWorkflow', false], + ])('if node has onError set to %s, it should return %s', (onError, expected) => { + node.onError = onError as OnError; + expect(executeSingleContext.continueOnFail()).toEqual(expected); + }); + }); + + describe('evaluateExpression', () => { + it('should evaluate the expression correctly', () => { + const expression = '$json.test'; + const expectedResult = 'data'; + const resolveSimpleParameterValueSpy = jest.spyOn( + workflow.expression, + 'resolveSimpleParameterValue', + ); + resolveSimpleParameterValueSpy.mockReturnValue(expectedResult); + + expect(executeSingleContext.evaluateExpression(expression, itemIndex)).toEqual( + expectedResult, + ); + + expect(resolveSimpleParameterValueSpy).toHaveBeenCalledWith( + `=${expression}`, + {}, + runExecutionData, + runIndex, + itemIndex, + node.name, + connectionInputData, + mode, + expect.objectContaining({}), + executeData, + ); + + resolveSimpleParameterValueSpy.mockRestore(); + }); + }); + + describe('getContext', () => { + it('should return the context object', () => { + const contextType: ContextType = 'node'; + const expectedContext = mock(); + const getContextSpy = jest.spyOn(NodeHelpers, 'getContext'); + getContextSpy.mockReturnValue(expectedContext); + + expect(executeSingleContext.getContext(contextType)).toEqual(expectedContext); + + expect(getContextSpy).toHaveBeenCalledWith(runExecutionData, contextType, node); + + getContextSpy.mockRestore(); + }); + }); + + describe('getInputData', () => { + const inputIndex = 0; + const inputName = 'main'; + + afterEach(() => { + inputData[inputName] = [[{ json: { test: 'data' } }]]; + }); + + it('should return the input data correctly', () => { + const expectedData = { json: { test: 'data' } }; + + expect(executeSingleContext.getInputData(inputIndex, inputName)).toEqual(expectedData); + }); + + it('should return an empty object if the input name does not exist', () => { + const inputName = 'nonExistent'; + const expectedData = { json: {} }; + + expect(executeSingleContext.getInputData(inputIndex, inputName)).toEqual(expectedData); + }); + + it('should throw an error if the input index is out of range', () => { + const inputIndex = 1; + + expect(() => executeSingleContext.getInputData(inputIndex, inputName)).toThrow( + ApplicationError, + ); + }); + + it('should throw an error if the input index was not set', () => { + inputData.main[inputIndex] = null; + + expect(() => executeSingleContext.getInputData(inputIndex, inputName)).toThrow( + ApplicationError, + ); + }); + + it('should throw an error if the value of input with given index was not set', () => { + delete inputData.main[inputIndex]![itemIndex]; + + expect(() => executeSingleContext.getInputData(inputIndex, inputName)).toThrow( + ApplicationError, + ); + }); + }); + + describe('getItemIndex', () => { + it('should return the item index correctly', () => { + expect(executeSingleContext.getItemIndex()).toEqual(itemIndex); + }); + }); + + describe('getNodeParameter', () => { + beforeEach(() => { + nodeTypes.getByNameAndVersion.mockReturnValue(nodeType); + expression.getParameterValue.mockImplementation((value) => value); + }); + + it('should return parameter value when it exists', () => { + const parameter = executeSingleContext.getNodeParameter('testParameter'); + + expect(parameter).toBe('testValue'); + }); + + it('should return the fallback value when the parameter does not exist', () => { + const parameter = executeSingleContext.getNodeParameter('otherParameter', 'fallback'); + + expect(parameter).toBe('fallback'); + }); + }); + + describe('getCredentials', () => { + it('should get decrypted credentials', async () => { + nodeTypes.getByNameAndVersion.mockReturnValue(nodeType); + credentialsHelper.getDecrypted.mockResolvedValue({ secret: 'token' }); + + const credentials = + await executeSingleContext.getCredentials( + testCredentialType, + ); + + expect(credentials).toEqual({ secret: 'token' }); + }); + }); + + describe('getExecuteData', () => { + it('should return the execute data correctly', () => { + expect(executeSingleContext.getExecuteData()).toEqual(executeData); + }); + }); + + describe('getWorkflowDataProxy', () => { + it('should return the workflow data proxy correctly', () => { + const workflowDataProxy = executeSingleContext.getWorkflowDataProxy(); + expect(workflowDataProxy.isProxy).toBe(true); + expect(Object.keys(workflowDataProxy.$input)).toEqual([ + 'all', + 'context', + 'first', + 'item', + 'last', + 'params', + ]); + }); + }); + + describe('getInputSourceData', () => { + it('should return the input source data correctly', () => { + const inputSourceData = mock(); + executeData.source = { main: [inputSourceData] }; + + expect(executeSingleContext.getInputSourceData()).toEqual(inputSourceData); + }); + + it('should throw an error if the source data is missing', () => { + executeData.source = null; + + expect(() => executeSingleContext.getInputSourceData()).toThrow(ApplicationError); + }); + }); + + describe('logAiEvent', () => { + it('should log the AI event correctly', () => { + const eventName = 'ai-tool-called'; + const msg = 'test message'; + + executeSingleContext.logAiEvent(eventName, msg); + + expect(additionalData.logAiEvent).toHaveBeenCalledWith(eventName, { + executionId: additionalData.executionId, + nodeName: node.name, + workflowName: workflow.name, + nodeType: node.type, + workflowId: workflow.id, + msg, + }); + }); + }); +}); diff --git a/packages/core/src/node-execution-context/execute-single-context.ts b/packages/core/src/node-execution-context/execute-single-context.ts new file mode 100644 index 0000000000..6d8ef2a083 --- /dev/null +++ b/packages/core/src/node-execution-context/execute-single-context.ts @@ -0,0 +1,212 @@ +import type { + ICredentialDataDecryptedObject, + IGetNodeParameterOptions, + INode, + INodeExecutionData, + IRunExecutionData, + IExecuteSingleFunctions, + IWorkflowExecuteAdditionalData, + Workflow, + WorkflowExecuteMode, + ITaskDataConnections, + IExecuteData, + ContextType, + AiEvent, + ISourceData, +} from 'n8n-workflow'; +import { + ApplicationError, + createDeferredPromise, + NodeHelpers, + WorkflowDataProxy, +} from 'n8n-workflow'; + +// eslint-disable-next-line import/no-cycle +import { + assertBinaryData, + continueOnFail, + getAdditionalKeys, + getBinaryDataBuffer, + getCredentials, + getNodeParameter, + returnJsonArray, +} from '@/NodeExecuteFunctions'; + +import { BinaryHelpers } from './helpers/binary-helpers'; +import { RequestHelpers } from './helpers/request-helpers'; +import { NodeExecutionContext } from './node-execution-context'; + +export class ExecuteSingleContext extends NodeExecutionContext implements IExecuteSingleFunctions { + readonly helpers: IExecuteSingleFunctions['helpers']; + + constructor( + workflow: Workflow, + node: INode, + additionalData: IWorkflowExecuteAdditionalData, + mode: WorkflowExecuteMode, + private readonly runExecutionData: IRunExecutionData, + private readonly runIndex: number, + private readonly connectionInputData: INodeExecutionData[], + private readonly inputData: ITaskDataConnections, + private readonly itemIndex: number, + private readonly executeData: IExecuteData, + private readonly abortSignal?: AbortSignal, + ) { + super(workflow, node, additionalData, mode); + + this.helpers = { + createDeferredPromise, + returnJsonArray, + ...new BinaryHelpers(workflow, additionalData).exported, + ...new RequestHelpers(this, workflow, node, additionalData).exported, + + assertBinaryData: (propertyName, inputIndex = 0) => + assertBinaryData(inputData, node, itemIndex, propertyName, inputIndex), + getBinaryDataBuffer: async (propertyName, inputIndex = 0) => + await getBinaryDataBuffer(inputData, itemIndex, propertyName, inputIndex), + }; + } + + getExecutionCancelSignal() { + return this.abortSignal; + } + + onExecutionCancellation(handler: () => unknown) { + const fn = () => { + this.abortSignal?.removeEventListener('abort', fn); + handler(); + }; + this.abortSignal?.addEventListener('abort', fn); + } + + continueOnFail() { + return continueOnFail(this.node); + } + + evaluateExpression(expression: string, evaluateItemIndex: number | undefined) { + evaluateItemIndex = evaluateItemIndex ?? this.itemIndex; + return this.workflow.expression.resolveSimpleParameterValue( + `=${expression}`, + {}, + this.runExecutionData, + this.runIndex, + evaluateItemIndex, + this.node.name, + this.connectionInputData, + this.mode, + getAdditionalKeys(this.additionalData, this.mode, this.runExecutionData), + this.executeData, + ); + } + + getContext(type: ContextType) { + return NodeHelpers.getContext(this.runExecutionData, type, this.node); + } + + getInputData(inputIndex = 0, inputName = 'main') { + if (!this.inputData.hasOwnProperty(inputName)) { + // Return empty array because else it would throw error when nothing is connected to input + return { json: {} }; + } + + // TODO: Check if nodeType has input with that index defined + if (this.inputData[inputName].length < inputIndex) { + throw new ApplicationError('Could not get input index', { + extra: { inputIndex, inputName }, + }); + } + + const allItems = this.inputData[inputName][inputIndex]; + + if (allItems === null || allItems === undefined) { + throw new ApplicationError('Input index was not set', { + extra: { inputIndex, inputName }, + }); + } + + const data = allItems[this.itemIndex]; + if (data === null || data === undefined) { + throw new ApplicationError('Value of input with given index was not set', { + extra: { inputIndex, inputName, itemIndex: this.itemIndex }, + }); + } + + return data; + } + + getItemIndex() { + return this.itemIndex; + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getNodeParameter(parameterName: string, fallbackValue?: any, options?: IGetNodeParameterOptions) { + return getNodeParameter( + this.workflow, + this.runExecutionData, + this.runIndex, + this.connectionInputData, + this.node, + parameterName, + this.itemIndex, + this.mode, + getAdditionalKeys(this.additionalData, this.mode, this.runExecutionData), + this.executeData, + fallbackValue, + options, + ); + } + + // TODO: extract out in a BaseExecutionContext + async getCredentials(type: string) { + return await getCredentials( + this.workflow, + this.node, + type, + this.additionalData, + this.mode, + this.executeData, + this.runExecutionData, + this.runIndex, + this.connectionInputData, + this.itemIndex, + ); + } + + getExecuteData() { + return this.executeData; + } + + getWorkflowDataProxy() { + return new WorkflowDataProxy( + this.workflow, + this.runExecutionData, + this.runIndex, + this.itemIndex, + this.node.name, + this.connectionInputData, + {}, + this.mode, + getAdditionalKeys(this.additionalData, this.mode, this.runExecutionData), + this.executeData, + ).getDataProxy(); + } + + getInputSourceData(inputIndex = 0, inputName = 'main'): ISourceData { + if (this.executeData?.source === null) { + // Should never happen as n8n sets it automatically + throw new ApplicationError('Source data is missing'); + } + return this.executeData.source[inputName][inputIndex] as ISourceData; + } + + logAiEvent(eventName: AiEvent, msg: string) { + return this.additionalData.logAiEvent(eventName, { + executionId: this.additionalData.executionId ?? 'unsaved-execution', + nodeName: this.node.name, + workflowName: this.workflow.name ?? 'Unnamed workflow', + nodeType: this.node.type, + workflowId: this.workflow.id ?? 'unsaved-workflow', + msg, + }); + } +} diff --git a/packages/core/src/node-execution-context/index.ts b/packages/core/src/node-execution-context/index.ts index a6397c60ce..d5c663b2ab 100644 --- a/packages/core/src/node-execution-context/index.ts +++ b/packages/core/src/node-execution-context/index.ts @@ -1,4 +1,5 @@ // eslint-disable-next-line import/no-cycle +export { ExecuteSingleContext } from './execute-single-context'; export { HookContext } from './hook-context'; export { LoadOptionsContext } from './load-options-context'; export { PollContext } from './poll-context'; diff --git a/packages/workflow/src/RoutingNode.ts b/packages/workflow/src/RoutingNode.ts index 4b3ed7f597..db3b180972 100644 --- a/packages/workflow/src/RoutingNode.ts +++ b/packages/workflow/src/RoutingNode.ts @@ -42,6 +42,7 @@ import type { JsonObject, CloseFunction, INodeCredentialDescription, + IExecutePaginationFunctions, } from './Interfaces'; import * as NodeHelpers from './NodeHelpers'; import { sleep } from './utils'; @@ -623,27 +624,28 @@ export class RoutingNode { ); } - const executePaginationFunctions = { - ...executeSingleFunctions, - makeRoutingRequest: async (requestOptions: DeclarativeRestApiSettings.ResultOptions) => { - return await this.rawRoutingRequest( - executeSingleFunctions, - requestOptions, - credentialType, - credentialsDecrypted, - ).then( - async (data) => - await this.postProcessResponseData( - executeSingleFunctions, - data, - requestData, - itemIndex, - runIndex, - ), - ); - }, + const makeRoutingRequest = async (requestOptions: DeclarativeRestApiSettings.ResultOptions) => { + return await this.rawRoutingRequest( + executeSingleFunctions, + requestOptions, + credentialType, + credentialsDecrypted, + ).then( + async (data) => + await this.postProcessResponseData( + executeSingleFunctions, + data, + requestData, + itemIndex, + runIndex, + ), + ); }; + const executePaginationFunctions = Object.create(executeSingleFunctions, { + makeRoutingRequest: { value: makeRoutingRequest }, + }) as IExecutePaginationFunctions; + if (requestData.paginate && requestOperations?.pagination) { // Has pagination From a26c0e2c3c7da87bfaba9737a967aa0070810d85 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Wed, 6 Nov 2024 11:01:45 +0100 Subject: [PATCH 02/38] fix(editor): Fix selected credential being overwritten in NDV (#11496) --- cypress/e2e/2-credentials.cy.ts | 50 ++++++++++++ .../src/components/NodeCredentials.vue | 77 +++++++++---------- .../composables/useCanvasOperations.test.ts | 30 -------- .../src/composables/useCanvasOperations.ts | 55 ------------- 4 files changed, 87 insertions(+), 125 deletions(-) diff --git a/cypress/e2e/2-credentials.cy.ts b/cypress/e2e/2-credentials.cy.ts index 8ce3bc4080..71c3083856 100644 --- a/cypress/e2e/2-credentials.cy.ts +++ b/cypress/e2e/2-credentials.cy.ts @@ -26,6 +26,22 @@ const nodeDetailsView = new NDV(); const NEW_CREDENTIAL_NAME = 'Something else'; const NEW_CREDENTIAL_NAME2 = 'Something else entirely'; +function createNotionCredential() { + workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME); + workflowPage.actions.openNode(NOTION_NODE_NAME); + workflowPage.getters.nodeCredentialsSelect().click(); + getVisibleSelect().find('li').last().click(); + credentialsModal.actions.fillCredentialsForm(); + cy.get('body').type('{esc}'); + workflowPage.actions.deleteNode(NOTION_NODE_NAME); +} + +function deleteSelectedCredential() { + workflowPage.getters.nodeCredentialsEditButton().click(); + credentialsModal.getters.deleteButton().click(); + cy.get('.el-message-box').find('button').contains('Yes').click(); +} + describe('Credentials', () => { beforeEach(() => { cy.visit(credentialsPage.url); @@ -229,6 +245,40 @@ describe('Credentials', () => { .should('have.value', NEW_CREDENTIAL_NAME); }); + it('should set a default credential when adding nodes', () => { + workflowPage.actions.visit(); + + createNotionCredential(); + + workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true); + workflowPage.getters + .nodeCredentialsSelect() + .find('input') + .should('have.value', NEW_NOTION_ACCOUNT_NAME); + + deleteSelectedCredential(); + }); + + it('should set a default credential when editing a node', () => { + workflowPage.actions.visit(); + + createNotionCredential(); + + workflowPage.actions.addNodeToCanvas(HTTP_REQUEST_NODE_NAME, true, true); + nodeDetailsView.getters.parameterInput('authentication').click(); + getVisibleSelect().find('li').contains('Predefined').click(); + + nodeDetailsView.getters.parameterInput('nodeCredentialType').click(); + getVisibleSelect().find('li').contains('Notion API').click(); + + workflowPage.getters + .nodeCredentialsSelect() + .find('input') + .should('have.value', NEW_NOTION_ACCOUNT_NAME); + + deleteSelectedCredential(); + }); + it('should setup generic authentication for HTTP node', () => { workflowPage.actions.visit(); workflowPage.actions.addNodeToCanvas(SCHEDULE_TRIGGER_NODE_NAME); diff --git a/packages/editor-ui/src/components/NodeCredentials.vue b/packages/editor-ui/src/components/NodeCredentials.vue index 34657284c3..a718b91757 100644 --- a/packages/editor-ui/src/components/NodeCredentials.vue +++ b/packages/editor-ui/src/components/NodeCredentials.vue @@ -37,6 +37,7 @@ import { N8nText, N8nTooltip, } from 'n8n-design-system'; +import { isEmpty } from '@/utils/typesUtils'; interface CredentialDropdownOption extends ICredentialsResponse { typeDisplayName: string; @@ -87,9 +88,9 @@ const credentialTypesNode = computed(() => ); const credentialTypesNodeDescriptionDisplayed = computed(() => - credentialTypesNodeDescription.value.filter((credentialTypeDescription) => - displayCredentials(credentialTypeDescription), - ), + credentialTypesNodeDescription.value + .filter((credentialTypeDescription) => displayCredentials(credentialTypeDescription)) + .map((type) => ({ type, options: getCredentialOptions(getAllRelatedCredentialTypes(type)) })), ); const credentialTypesNodeDescription = computed(() => { if (typeof props.overrideCredType !== 'string') return []; @@ -149,6 +150,27 @@ watch( { immediate: true, deep: true }, ); +// Select most recent credential by default +watch( + credentialTypesNodeDescriptionDisplayed, + (types) => { + if (types.length === 0 || !isEmpty(selected.value)) return; + + const allOptions = types.map((type) => type.options).flat(); + + if (allOptions.length === 0) return; + + const mostRecentCredential = allOptions.reduce( + (mostRecent, current) => + mostRecent && mostRecent.updatedAt > current.updatedAt ? mostRecent : current, + allOptions[0], + ); + + onCredentialSelected(mostRecentCredential.type, mostRecentCredential.id); + }, + { immediate: true }, +); + onMounted(() => { credentialsStore.$onAction(({ name, after, args }) => { const listeningForActions = ['createNewCredential', 'updateCredential', 'deleteCredential']; @@ -481,12 +503,9 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s v-if="credentialTypesNodeDescriptionDisplayed.length" :class="['node-credentials', $style.container]" > -
+
-
+
@@ -567,10 +567,7 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s
@@ -578,7 +575,7 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s icon="pen" class="clickable" :title="$locale.baseText('nodeCredentials.updateCredential')" - @click="editCredential(credentialTypeDescription.name)" + @click="editCredential(type.name)" />
diff --git a/packages/editor-ui/src/composables/useCanvasOperations.test.ts b/packages/editor-ui/src/composables/useCanvasOperations.test.ts index 9b6a819238..b3c9c06b15 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.test.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.test.ts @@ -191,36 +191,6 @@ describe('useCanvasOperations', () => { expect(result.position).toEqual([20, 20]); }); - it('should create node with default credentials when only one credential is available', () => { - const credentialsStore = useCredentialsStore(); - const credential = mock({ id: '1', name: 'cred', type: 'cred' }); - const nodeTypeName = 'type'; - const nodeTypeDescription = mockNodeTypeDescription({ - name: nodeTypeName, - credentials: [{ name: credential.name }], - }); - - credentialsStore.state.credentials = { - [credential.id]: credential, - }; - - // @ts-expect-error Known pinia issue when spying on store getters - vi.spyOn(credentialsStore, 'getUsableCredentialByType', 'get').mockReturnValue(() => [ - credential, - ]); - - const { addNode } = useCanvasOperations({ router }); - const result = addNode( - { - type: nodeTypeName, - typeVersion: 1, - }, - nodeTypeDescription, - ); - - expect(result.credentials).toEqual({ [credential.name]: { id: '1', name: credential.name } }); - }); - it('should not assign credentials when multiple credentials are available', () => { const credentialsStore = useCredentialsStore(); const credentialA = mock({ id: '1', name: 'credA', type: 'cred' }); diff --git a/packages/editor-ui/src/composables/useCanvasOperations.ts b/packages/editor-ui/src/composables/useCanvasOperations.ts index 2d811a539e..a27c4f99db 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.ts @@ -777,7 +777,6 @@ export function useCanvasOperations({ router }: { router: ReturnType credentialsStore.getUsableCredentialByType(type.name)) - .flat(); - - if (credentialPerType?.length === 1) { - const defaultCredential = credentialPerType[0]; - - const selectedCredentials = credentialsStore.getCredentialById(defaultCredential.id); - const selected = { id: selectedCredentials.id, name: selectedCredentials.name }; - const credentials = { - [defaultCredential.type]: selected, - }; - - if (nodeTypeDescription.credentials) { - const authentication = nodeTypeDescription.credentials.find( - (type) => type.name === defaultCredential.type, - ); - - const authDisplayOptionsHide = authentication?.displayOptions?.hide; - const authDisplayOptionsShow = authentication?.displayOptions?.show; - - if (!authDisplayOptionsHide) { - if (!authDisplayOptionsShow) { - node.credentials = credentials; - } else if ( - Object.keys(authDisplayOptionsShow).length === 1 && - authDisplayOptionsShow.authentication - ) { - // ignore complex case when there's multiple dependencies - node.credentials = credentials; - - let parameters: { [key: string]: string } = {}; - for (const displayOption of Object.keys(authDisplayOptionsShow)) { - if (node.parameters && !node.parameters[displayOption]) { - parameters = {}; - node.credentials = undefined; - break; - } - const optionValue = authDisplayOptionsShow[displayOption]?.[0]; - if (optionValue && typeof optionValue === 'string') { - parameters[displayOption] = optionValue; - } - node.parameters = { - ...node.parameters, - ...parameters, - }; - } - } - } - } - } - } - function resolveNodePosition( node: Omit & { position?: INodeUi['position'] }, nodeTypeDescription: INodeTypeDescription, From 7254359855b89769613cd5cc24dbb4f45a7cc76f Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Wed, 6 Nov 2024 12:14:23 +0200 Subject: [PATCH 03/38] fix(editor): Keep workflow pristine after load on new canvas (#11579) --- .../composables/useCanvasOperations.test.ts | 115 +++++++++++++++++- .../src/composables/useCanvasOperations.ts | 52 +++++--- 2 files changed, 147 insertions(+), 20 deletions(-) diff --git a/packages/editor-ui/src/composables/useCanvasOperations.test.ts b/packages/editor-ui/src/composables/useCanvasOperations.test.ts index b3c9c06b15..3a90f05ff7 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.test.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.test.ts @@ -7,7 +7,7 @@ import type { } from 'n8n-workflow'; import { NodeConnectionType, NodeHelpers } from 'n8n-workflow'; import { useCanvasOperations } from '@/composables/useCanvasOperations'; -import type { CanvasNode } from '@/types'; +import type { CanvasConnection, CanvasNode } from '@/types'; import { CanvasConnectionMode } from '@/types'; import type { ICredentialsResponse, INodeUi, IWorkflowDb } from '@/Interface'; import { RemoveNodeCommand } from '@/models/history'; @@ -618,6 +618,48 @@ describe('useCanvasOperations', () => { const added = await addNodes(nodes, {}); expect(added.length).toBe(2); }); + + it('should mark UI state as dirty', async () => { + const workflowsStore = mockedStore(useWorkflowsStore); + const uiStore = mockedStore(useUIStore); + const nodeTypesStore = useNodeTypesStore(); + const nodeTypeName = 'type'; + const nodes = [mockNode({ name: 'Node 1', type: nodeTypeName, position: [30, 40] })]; + + workflowsStore.getCurrentWorkflow.mockReturnValue( + createTestWorkflowObject(workflowsStore.workflow), + ); + + nodeTypesStore.nodeTypes = { + [nodeTypeName]: { 1: mockNodeTypeDescription({ name: nodeTypeName }) }, + }; + + const { addNodes } = useCanvasOperations({ router }); + await addNodes(nodes, { keepPristine: false }); + + expect(uiStore.stateIsDirty).toEqual(true); + }); + + it('should not mark UI state as dirty if keepPristine is true', async () => { + const workflowsStore = mockedStore(useWorkflowsStore); + const uiStore = mockedStore(useUIStore); + const nodeTypesStore = useNodeTypesStore(); + const nodeTypeName = 'type'; + const nodes = [mockNode({ name: 'Node 1', type: nodeTypeName, position: [30, 40] })]; + + workflowsStore.getCurrentWorkflow.mockReturnValue( + createTestWorkflowObject(workflowsStore.workflow), + ); + + nodeTypesStore.nodeTypes = { + [nodeTypeName]: { 1: mockNodeTypeDescription({ name: nodeTypeName }) }, + }; + + const { addNodes } = useCanvasOperations({ router }); + await addNodes(nodes, { keepPristine: true }); + + expect(uiStore.stateIsDirty).toEqual(false); + }); }); describe('revertAddNode', () => { @@ -1013,6 +1055,26 @@ describe('useCanvasOperations', () => { ], }); }); + + it('should set UI state as dirty', async () => { + const uiStore = mockedStore(useUIStore); + const connections: CanvasConnection[] = []; + + const { addConnections } = useCanvasOperations({ router }); + await addConnections(connections, { keepPristine: false }); + + expect(uiStore.stateIsDirty).toBe(true); + }); + + it('should not set UI state as dirty if keepPristine is true', async () => { + const uiStore = mockedStore(useUIStore); + const connections: CanvasConnection[] = []; + + const { addConnections } = useCanvasOperations({ router }); + await addConnections(connections, { keepPristine: true }); + + expect(uiStore.stateIsDirty).toBe(false); + }); }); describe('createConnection', () => { @@ -1102,6 +1164,57 @@ describe('useCanvasOperations', () => { }); expect(uiStore.stateIsDirty).toBe(true); }); + + it('should not set UI state as dirty if keepPristine is true', () => { + const workflowsStore = mockedStore(useWorkflowsStore); + const uiStore = mockedStore(useUIStore); + const nodeTypesStore = mockedStore(useNodeTypesStore); + + const nodeTypeDescription = mockNodeTypeDescription({ + name: SET_NODE_TYPE, + inputs: [NodeConnectionType.Main], + outputs: [NodeConnectionType.Main], + }); + + const nodeA = createTestNode({ + id: 'a', + type: nodeTypeDescription.name, + name: 'Node A', + }); + + const nodeB = createTestNode({ + id: 'b', + type: nodeTypeDescription.name, + name: 'Node B', + }); + + const connection: Connection = { + source: nodeA.id, + sourceHandle: `outputs/${NodeConnectionType.Main}/0`, + target: nodeB.id, + targetHandle: `inputs/${NodeConnectionType.Main}/0`, + }; + + nodeTypesStore.nodeTypes = { + node: { 1: nodeTypeDescription }, + }; + + workflowsStore.workflow.nodes = [nodeA, nodeB]; + workflowsStore.getNodeById.mockReturnValueOnce(nodeA).mockReturnValueOnce(nodeB); + nodeTypesStore.getNodeType = vi.fn().mockReturnValue(nodeTypeDescription); + + const workflowObject = createTestWorkflowObject(workflowsStore.workflow); + workflowsStore.getCurrentWorkflow.mockReturnValue(workflowObject); + + const { createConnection, editableWorkflowObject } = useCanvasOperations({ router }); + + editableWorkflowObject.value.nodes[nodeA.name] = nodeA; + editableWorkflowObject.value.nodes[nodeB.name] = nodeB; + + createConnection(connection, { keepPristine: true }); + + expect(uiStore.stateIsDirty).toBe(false); + }); }); describe('revertCreateConnection', () => { diff --git a/packages/editor-ui/src/composables/useCanvasOperations.ts b/packages/editor-ui/src/composables/useCanvasOperations.ts index a27c4f99db..4b78da01b2 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.ts @@ -105,14 +105,23 @@ type AddNodeDataWithTypeVersion = AddNodeData & { typeVersion: INodeUi['typeVersion']; }; -type AddNodeOptions = { +type AddNodesBaseOptions = { dragAndDrop?: boolean; - openNDV?: boolean; trackHistory?: boolean; - isAutoAdd?: boolean; + keepPristine?: boolean; telemetry?: boolean; }; +type AddNodesOptions = AddNodesBaseOptions & { + position?: XYPosition; + trackBulk?: boolean; +}; + +type AddNodeOptions = AddNodesBaseOptions & { + openNDV?: boolean; + isAutoAdd?: boolean; +}; + export function useCanvasOperations({ router }: { router: ReturnType }) { const rootStore = useRootStore(); const workflowsStore = useWorkflowsStore(); @@ -479,17 +488,7 @@ export function useCanvasOperations({ router }: { router: ReturnType { workflowsStore.setNodePristine(nodeData.name, true); + if (!options.keepPristine) { + uiStore.stateIsDirty = true; + } + nodeHelpers.matchCredentials(nodeData); nodeHelpers.updateNodeParameterIssues(nodeData); nodeHelpers.updateNodeCredentialIssues(nodeData); @@ -1073,7 +1076,10 @@ export function useCanvasOperations({ router }: { router: ReturnType Date: Wed, 6 Nov 2024 12:12:49 +0000 Subject: [PATCH 04/38] fix: Rapid7 InsightVM copy tweaks and docs url (no-changelog) (#11585) --- .../credentials/Rapid7InsightVmApi.credentials.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/nodes-base/credentials/Rapid7InsightVmApi.credentials.ts b/packages/nodes-base/credentials/Rapid7InsightVmApi.credentials.ts index d46675298b..8fbcf1b39f 100644 --- a/packages/nodes-base/credentials/Rapid7InsightVmApi.credentials.ts +++ b/packages/nodes-base/credentials/Rapid7InsightVmApi.credentials.ts @@ -8,9 +8,9 @@ import type { export class Rapid7InsightVmApi implements ICredentialType { name = 'rapid7InsightVmApi'; - displayName = 'Rapid7 InsightVm API'; + displayName = 'Rapid7 InsightVM API'; - documentationUrl = 'Rapid7 InsightVm'; + documentationUrl = 'rapid7insightvm'; icon = { light: 'file:icons/Rapid7InsightVm.svg', @@ -18,7 +18,7 @@ export class Rapid7InsightVmApi implements ICredentialType { } as const; httpRequestNode = { - name: 'Rapid7 Insight Vm', + name: 'Rapid7 InsightVM', docsUrl: 'https://docs.rapid7.com/', apiBaseUrlPlaceholder: 'https://insight.rapid7.com/', }; From befa26f89aebf7e3591ccb8176e1b8f5e7937133 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 6 Nov 2024 13:16:23 +0100 Subject: [PATCH 05/38] refactor(core): Minor improvements to pruning service (#11578) Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> --- packages/cli/src/constants.ts | 3 + .../cli/src/services/orchestration.service.ts | 2 +- .../pruning/__tests__/pruning.service.test.ts | 77 +++++----- .../src/services/pruning/pruning.service.ts | 135 +++++++----------- .../test/integration/pruning.service.test.ts | 36 ++--- 5 files changed, 111 insertions(+), 142 deletions(-) diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 04512e8be9..be26616fb6 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -127,6 +127,9 @@ export const TIME = { * Eventually this will superseed `TIME` above */ export const Time = { + milliseconds: { + toMinutes: 1 / (60 * 1000), + }, seconds: { toMilliseconds: 1000, }, diff --git a/packages/cli/src/services/orchestration.service.ts b/packages/cli/src/services/orchestration.service.ts index 225badbf18..19da88e412 100644 --- a/packages/cli/src/services/orchestration.service.ts +++ b/packages/cli/src/services/orchestration.service.ts @@ -20,7 +20,7 @@ export class OrchestrationService { private subscriber: Subscriber; - protected isInitialized = false; + isInitialized = false; private isMultiMainSetupLicensed = false; diff --git a/packages/cli/src/services/pruning/__tests__/pruning.service.test.ts b/packages/cli/src/services/pruning/__tests__/pruning.service.test.ts index 5ea26ad540..64cecd1c06 100644 --- a/packages/cli/src/services/pruning/__tests__/pruning.service.test.ts +++ b/packages/cli/src/services/pruning/__tests__/pruning.service.test.ts @@ -1,4 +1,4 @@ -import type { GlobalConfig } from '@n8n/config'; +import type { PruningConfig } from '@n8n/config'; import { mock } from 'jest-mock-extended'; import type { InstanceSettings } from 'n8n-core'; @@ -8,9 +8,13 @@ import { mockLogger } from '@test/mocking'; import { PruningService } from '../pruning.service'; +jest.mock('@/db', () => ({ + connectionState: { migrated: true }, +})); + describe('PruningService', () => { describe('init', () => { - it('should start pruning if leader', () => { + it('should start pruning on main instance that is the leader', () => { const pruningService = new PruningService( mockLogger(), mock({ isLeader: true }), @@ -29,7 +33,7 @@ describe('PruningService', () => { expect(startPruningSpy).toHaveBeenCalled(); }); - it('should not start pruning if follower', () => { + it('should not start pruning on main instance that is a follower', () => { const pruningService = new PruningService( mockLogger(), mock({ isLeader: false }), @@ -48,7 +52,7 @@ describe('PruningService', () => { expect(startPruningSpy).not.toHaveBeenCalled(); }); - it('should register leadership events if multi-main setup is enabled', () => { + it('should register leadership events if main on multi-main setup', () => { const pruningService = new PruningService( mockLogger(), mock({ isLeader: true }), @@ -88,13 +92,10 @@ describe('PruningService', () => { isMultiMainSetupEnabled: true, multiMainSetup: mock(), }), - mock({ pruning: { isEnabled: true } }), + mock({ isEnabled: true }), ); - // @ts-expect-error Private method - const isEnabled = pruningService.isEnabled(); - - expect(isEnabled).toBe(true); + expect(pruningService.isEnabled).toBe(true); }); it('should return `false` based on config if leader main', () => { @@ -107,16 +108,13 @@ describe('PruningService', () => { isMultiMainSetupEnabled: true, multiMainSetup: mock(), }), - mock({ pruning: { isEnabled: false } }), + mock({ isEnabled: false }), ); - // @ts-expect-error Private method - const isEnabled = pruningService.isEnabled(); - - expect(isEnabled).toBe(false); + expect(pruningService.isEnabled).toBe(false); }); - it('should return `false` if non-main even if enabled', () => { + it('should return `false` if non-main even if config is enabled', () => { const pruningService = new PruningService( mockLogger(), mock({ isLeader: false, instanceType: 'worker' }), @@ -126,16 +124,13 @@ describe('PruningService', () => { isMultiMainSetupEnabled: true, multiMainSetup: mock(), }), - mock({ pruning: { isEnabled: true } }), + mock({ isEnabled: true }), ); - // @ts-expect-error Private method - const isEnabled = pruningService.isEnabled(); - - expect(isEnabled).toBe(false); + expect(pruningService.isEnabled).toBe(false); }); - it('should return `false` if follower main even if enabled', () => { + it('should return `false` if follower main even if config is enabled', () => { const pruningService = new PruningService( mockLogger(), mock({ isLeader: false, isFollower: true, instanceType: 'main' }), @@ -145,13 +140,10 @@ describe('PruningService', () => { isMultiMainSetupEnabled: true, multiMainSetup: mock(), }), - mock({ pruning: { isEnabled: true }, multiMainSetup: { enabled: true } }), + mock({ isEnabled: true }), ); - // @ts-expect-error Private method - const isEnabled = pruningService.isEnabled(); - - expect(isEnabled).toBe(false); + expect(pruningService.isEnabled).toBe(false); }); }); @@ -166,22 +158,25 @@ describe('PruningService', () => { isMultiMainSetupEnabled: true, multiMainSetup: mock(), }), - mock({ pruning: { isEnabled: false } }), + mock({ isEnabled: false }), + ); + + const scheduleRollingSoftDeletionsSpy = jest.spyOn( + pruningService, + // @ts-expect-error Private method + 'scheduleRollingSoftDeletions', ); // @ts-expect-error Private method - const setSoftDeletionInterval = jest.spyOn(pruningService, 'setSoftDeletionInterval'); - - // @ts-expect-error Private method - const scheduleHardDeletion = jest.spyOn(pruningService, 'scheduleHardDeletion'); + const scheduleNextHardDeletionSpy = jest.spyOn(pruningService, 'scheduleNextHardDeletion'); pruningService.startPruning(); - expect(setSoftDeletionInterval).not.toHaveBeenCalled(); - expect(scheduleHardDeletion).not.toHaveBeenCalled(); + expect(scheduleRollingSoftDeletionsSpy).not.toHaveBeenCalled(); + expect(scheduleNextHardDeletionSpy).not.toHaveBeenCalled(); }); - it('should start pruning if service is enabled', () => { + it('should start pruning if service is enabled and DB is migrated', () => { const pruningService = new PruningService( mockLogger(), mock({ isLeader: true, instanceType: 'main' }), @@ -191,23 +186,23 @@ describe('PruningService', () => { isMultiMainSetupEnabled: true, multiMainSetup: mock(), }), - mock({ pruning: { isEnabled: true } }), + mock({ isEnabled: true }), ); - const setSoftDeletionInterval = jest + const scheduleRollingSoftDeletionsSpy = jest // @ts-expect-error Private method - .spyOn(pruningService, 'setSoftDeletionInterval') + .spyOn(pruningService, 'scheduleRollingSoftDeletions') .mockImplementation(); - const scheduleHardDeletion = jest + const scheduleNextHardDeletionSpy = jest // @ts-expect-error Private method - .spyOn(pruningService, 'scheduleHardDeletion') + .spyOn(pruningService, 'scheduleNextHardDeletion') .mockImplementation(); pruningService.startPruning(); - expect(setSoftDeletionInterval).toHaveBeenCalled(); - expect(scheduleHardDeletion).toHaveBeenCalled(); + expect(scheduleRollingSoftDeletionsSpy).toHaveBeenCalled(); + expect(scheduleNextHardDeletionSpy).toHaveBeenCalled(); }); }); }); diff --git a/packages/cli/src/services/pruning/pruning.service.ts b/packages/cli/src/services/pruning/pruning.service.ts index 7314015091..34be37cf21 100644 --- a/packages/cli/src/services/pruning/pruning.service.ts +++ b/packages/cli/src/services/pruning/pruning.service.ts @@ -1,27 +1,37 @@ -import { GlobalConfig } from '@n8n/config'; +import { PruningConfig } from '@n8n/config'; import { BinaryDataService, InstanceSettings } from 'n8n-core'; -import { jsonStringify } from 'n8n-workflow'; +import { ensureError } from 'n8n-workflow'; +import { strict } from 'node:assert'; import { Service } from 'typedi'; -import { TIME } from '@/constants'; +import { Time } from '@/constants'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; +import { connectionState as dbConnectionState } from '@/db'; import { OnShutdown } from '@/decorators/on-shutdown'; import { Logger } from '@/logging/logger.service'; import { OrchestrationService } from '../orchestration.service'; +/** + * Responsible for pruning executions from the database and their associated binary data + * from the filesystem, on a rolling basis. By default we soft-delete execution rows + * every cycle and hard-delete them and their binary data every 4th cycle. + */ @Service() export class PruningService { - private hardDeletionBatchSize = 100; + /** Timer for soft-deleting executions on a rolling basis. */ + private softDeletionInterval: NodeJS.Timer | undefined; - private rates: Record = { - softDeletion: this.globalConfig.pruning.softDeleteInterval * TIME.MINUTE, - hardDeletion: this.globalConfig.pruning.hardDeleteInterval * TIME.MINUTE, + /** Timeout for next hard-deletion of soft-deleted executions. */ + private hardDeletionTimeout: NodeJS.Timeout | undefined; + + private readonly rates = { + softDeletion: this.pruningConfig.softDeleteInterval * Time.minutes.toMilliseconds, + hardDeletion: this.pruningConfig.hardDeleteInterval * Time.minutes.toMilliseconds, }; - public softDeletionInterval: NodeJS.Timer | undefined; - - public hardDeletionTimeout: NodeJS.Timeout | undefined; + /** Max number of executions to hard-delete in a cycle. */ + private readonly batchSize = 100; private isShuttingDown = false; @@ -31,103 +41,68 @@ export class PruningService { private readonly executionRepository: ExecutionRepository, private readonly binaryDataService: BinaryDataService, private readonly orchestrationService: OrchestrationService, - private readonly globalConfig: GlobalConfig, + private readonly pruningConfig: PruningConfig, ) { this.logger = this.logger.scoped('pruning'); } - /** - * @important Requires `OrchestrationService` to be initialized. - */ init() { - const { isLeader } = this.instanceSettings; - const { isMultiMainSetupEnabled } = this.orchestrationService; + strict(this.instanceSettings.instanceRole !== 'unset', 'Instance role is not set'); - if (isLeader) this.startPruning(); + if (this.instanceSettings.isLeader) this.startPruning(); - if (isMultiMainSetupEnabled) { + if (this.orchestrationService.isMultiMainSetupEnabled) { this.orchestrationService.multiMainSetup.on('leader-takeover', () => this.startPruning()); this.orchestrationService.multiMainSetup.on('leader-stepdown', () => this.stopPruning()); } } - private isEnabled() { - const { instanceType, isFollower } = this.instanceSettings; - if (!this.globalConfig.pruning.isEnabled || instanceType !== 'main') { - return false; - } - - if (this.globalConfig.multiMainSetup.enabled && instanceType === 'main' && isFollower) { - return false; - } - - return true; + get isEnabled() { + return ( + this.pruningConfig.isEnabled && + this.instanceSettings.instanceType === 'main' && + this.instanceSettings.isLeader + ); } - /** - * @important Call this method only after DB migrations have completed. - */ startPruning() { - if (!this.isEnabled()) return; + if (!this.isEnabled || !dbConnectionState.migrated || this.isShuttingDown) return; - if (this.isShuttingDown) { - this.logger.warn('Cannot start pruning while shutting down'); - return; - } - - this.logger.debug('Starting soft-deletion and hard-deletion timers'); - - this.setSoftDeletionInterval(); - this.scheduleHardDeletion(); + this.scheduleRollingSoftDeletions(); + this.scheduleNextHardDeletion(); } stopPruning() { - if (!this.isEnabled()) return; - - this.logger.debug('Removing soft-deletion and hard-deletion timers'); + if (!this.isEnabled) return; clearInterval(this.softDeletionInterval); clearTimeout(this.hardDeletionTimeout); } - private setSoftDeletionInterval(rateMs = this.rates.softDeletion) { - const when = [rateMs / TIME.MINUTE, 'min'].join(' '); - + private scheduleRollingSoftDeletions(rateMs = this.rates.softDeletion) { this.softDeletionInterval = setInterval( - async () => await this.softDeleteOnPruningCycle(), + async () => await this.softDelete(), this.rates.softDeletion, ); - this.logger.debug(`Soft-deletion scheduled every ${when}`); + this.logger.debug(`Soft-deletion every ${rateMs * Time.milliseconds.toMinutes} minutes`); } - private scheduleHardDeletion(rateMs = this.rates.hardDeletion) { - const when = [rateMs / TIME.MINUTE, 'min'].join(' '); - + private scheduleNextHardDeletion(rateMs = this.rates.hardDeletion) { this.hardDeletionTimeout = setTimeout(() => { - this.hardDeleteOnPruningCycle() - .then((rate) => this.scheduleHardDeletion(rate)) + this.hardDelete() + .then((rate) => this.scheduleNextHardDeletion(rate)) .catch((error) => { - this.scheduleHardDeletion(1 * TIME.SECOND); - - const errorMessage = - error instanceof Error - ? error.message - : jsonStringify(error, { replaceCircularRefs: true }); - - this.logger.error('Failed to hard-delete executions', { errorMessage }); + this.scheduleNextHardDeletion(1_000); + this.logger.error('Failed to hard-delete executions', { error: ensureError(error) }); }); }, rateMs); - this.logger.debug(`Hard-deletion scheduled for next ${when}`); + this.logger.debug(`Hard-deletion in next ${rateMs * Time.milliseconds.toMinutes} minutes`); } - /** - * Mark executions as deleted based on age and count, in a pruning cycle. - */ - async softDeleteOnPruningCycle() { - this.logger.debug('Starting soft-deletion of executions'); - + /** Soft-delete executions based on max age and/or max count. */ + async softDelete() { const result = await this.executionRepository.softDeletePrunableExecutions(); if (result.affected === 0) { @@ -145,10 +120,11 @@ export class PruningService { } /** - * Permanently remove all soft-deleted executions and their binary data, in a pruning cycle. - * @return Delay in ms after which the next cycle should be started + * Delete all soft-deleted executions and their binary data. + * + * @returns Delay in milliseconds until next hard-deletion */ - private async hardDeleteOnPruningCycle() { + private async hardDelete(): Promise { const ids = await this.executionRepository.findSoftDeletedExecutions(); const executionIds = ids.map((o) => o.executionId); @@ -160,8 +136,6 @@ export class PruningService { } try { - this.logger.debug('Starting hard-deletion of executions', { executionIds }); - await this.binaryDataService.deleteMany(ids); await this.executionRepository.deleteByIds(executionIds); @@ -170,16 +144,13 @@ export class PruningService { } catch (error) { this.logger.error('Failed to hard-delete executions', { executionIds, - error: error instanceof Error ? error.message : `${error}`, + error: ensureError(error), }); } - /** - * For next batch, speed up hard-deletion cycle in high-volume case - * to prevent high concurrency from causing duplicate deletions. - */ - const isHighVolume = executionIds.length >= this.hardDeletionBatchSize; + // if high volume, speed up next hard-deletion + if (executionIds.length >= this.batchSize) return 1 * Time.seconds.toMilliseconds; - return isHighVolume ? 1 * TIME.SECOND : this.rates.hardDeletion; + return this.rates.hardDeletion; } } diff --git a/packages/cli/test/integration/pruning.service.test.ts b/packages/cli/test/integration/pruning.service.test.ts index 76ee107903..4ea8455b94 100644 --- a/packages/cli/test/integration/pruning.service.test.ts +++ b/packages/cli/test/integration/pruning.service.test.ts @@ -1,4 +1,4 @@ -import { GlobalConfig } from '@n8n/config'; +import { PruningConfig } from '@n8n/config'; import { mock } from 'jest-mock-extended'; import { BinaryDataService, InstanceSettings } from 'n8n-core'; import type { ExecutionStatus } from 'n8n-workflow'; @@ -27,19 +27,19 @@ describe('softDeleteOnPruningCycle()', () => { const now = new Date(); const yesterday = new Date(Date.now() - TIME.DAY); let workflow: WorkflowEntity; - let globalConfig: GlobalConfig; + let pruningConfig: PruningConfig; beforeAll(async () => { await testDb.init(); - globalConfig = Container.get(GlobalConfig); + pruningConfig = Container.get(PruningConfig); pruningService = new PruningService( mockLogger(), instanceSettings, Container.get(ExecutionRepository), mockInstance(BinaryDataService), mock(), - globalConfig, + pruningConfig, ); workflow = await createWorkflow(); @@ -62,8 +62,8 @@ describe('softDeleteOnPruningCycle()', () => { describe('when EXECUTIONS_DATA_PRUNE_MAX_COUNT is set', () => { beforeAll(() => { - globalConfig.pruning.maxAge = 336; - globalConfig.pruning.maxCount = 1; + pruningConfig.maxAge = 336; + pruningConfig.maxCount = 1; }); test('should mark as deleted based on EXECUTIONS_DATA_PRUNE_MAX_COUNT', async () => { @@ -73,7 +73,7 @@ describe('softDeleteOnPruningCycle()', () => { await createSuccessfulExecution(workflow), ]; - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -92,7 +92,7 @@ describe('softDeleteOnPruningCycle()', () => { await createSuccessfulExecution(workflow), ]; - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -113,7 +113,7 @@ describe('softDeleteOnPruningCycle()', () => { await createSuccessfulExecution(workflow), ]; - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -132,7 +132,7 @@ describe('softDeleteOnPruningCycle()', () => { await createSuccessfulExecution(workflow), ]; - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -150,7 +150,7 @@ describe('softDeleteOnPruningCycle()', () => { await annotateExecution(executions[0].id, { vote: 'up' }, [workflow.id]); - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -163,8 +163,8 @@ describe('softDeleteOnPruningCycle()', () => { describe('when EXECUTIONS_DATA_MAX_AGE is set', () => { beforeAll(() => { - globalConfig.pruning.maxAge = 1; - globalConfig.pruning.maxCount = 0; + pruningConfig.maxAge = 1; + pruningConfig.maxCount = 0; }); test('should mark as deleted based on EXECUTIONS_DATA_MAX_AGE', async () => { @@ -179,7 +179,7 @@ describe('softDeleteOnPruningCycle()', () => { ), ]; - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -203,7 +203,7 @@ describe('softDeleteOnPruningCycle()', () => { await createSuccessfulExecution(workflow), ]; - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -221,7 +221,7 @@ describe('softDeleteOnPruningCycle()', () => { ])('should prune %s executions', async (status, attributes) => { const execution = await createExecution({ status, ...attributes }, workflow); - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -239,7 +239,7 @@ describe('softDeleteOnPruningCycle()', () => { await createSuccessfulExecution(workflow), ]; - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ @@ -266,7 +266,7 @@ describe('softDeleteOnPruningCycle()', () => { await annotateExecution(executions[0].id, { vote: 'up' }, [workflow.id]); - await pruningService.softDeleteOnPruningCycle(); + await pruningService.softDelete(); const result = await findAllExecutions(); expect(result).toEqual([ From ca7502082188e32ac4be9260ebebb356d3e53d96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 6 Nov 2024 13:39:31 +0100 Subject: [PATCH 06/38] perf(core): Load node types on demand on runners (no-changelog) (#11559) --- .../src/__tests__/node-types.test.ts | 31 ++++++++ .../src/js-task-runner/js-task-runner.ts | 24 ++++++ .../@n8n/task-runner/src/message-types.ts | 55 +++++++++++++- packages/@n8n/task-runner/src/node-types.ts | 28 +++++++ packages/@n8n/task-runner/src/runner-types.ts | 3 + packages/@n8n/task-runner/src/task-runner.ts | 54 ++++++++++++-- packages/cli/src/node-types.ts | 19 +++++ .../src/runners/__tests__/task-broker.test.ts | 73 +++++++++++++++++-- packages/cli/src/runners/runner-ws-server.ts | 2 +- .../cli/src/runners/task-broker.service.ts | 70 +++++++++++------- .../task-managers/local-task-manager.ts | 9 ++- .../src/runners/task-managers/task-manager.ts | 26 ++++++- .../cli/src/runners/task-runner-module.ts | 2 +- 13 files changed, 345 insertions(+), 51 deletions(-) diff --git a/packages/@n8n/task-runner/src/__tests__/node-types.test.ts b/packages/@n8n/task-runner/src/__tests__/node-types.test.ts index c102c80df3..c535bb0147 100644 --- a/packages/@n8n/task-runner/src/__tests__/node-types.test.ts +++ b/packages/@n8n/task-runner/src/__tests__/node-types.test.ts @@ -63,4 +63,35 @@ describe('TaskRunnerNodeTypes', () => { }); }); }); + + describe('addNodeTypeDescriptions', () => { + it('should add new node types', () => { + const nodeTypes = new TaskRunnerNodeTypes(TYPES); + + const nodeTypeDescriptions = [ + { name: 'new-type', version: 1 }, + { name: 'new-type', version: 2 }, + ] as INodeTypeDescription[]; + + nodeTypes.addNodeTypeDescriptions(nodeTypeDescriptions); + + expect(nodeTypes.getByNameAndVersion('new-type', 1)).toEqual({ + description: { name: 'new-type', version: 1 }, + }); + expect(nodeTypes.getByNameAndVersion('new-type', 2)).toEqual({ + description: { name: 'new-type', version: 2 }, + }); + }); + }); + + describe('onlyUnknown', () => { + it('should return only unknown node types', () => { + const nodeTypes = new TaskRunnerNodeTypes(TYPES); + + const candidate = { name: 'unknown', version: 1 }; + + expect(nodeTypes.onlyUnknown([candidate])).toEqual([candidate]); + expect(nodeTypes.onlyUnknown([SINGLE_VERSIONED])).toEqual([]); + }); + }); }); diff --git a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts index e0bebe0521..7e79bba73b 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/js-task-runner.ts @@ -17,6 +17,7 @@ import type { IRunExecutionData, WorkflowExecuteMode, EnvProviderState, + INodeTypeDescription, } from 'n8n-workflow'; import * as a from 'node:assert'; import { runInNewContext, type Context } from 'node:vm'; @@ -119,6 +120,29 @@ export class JsTaskRunner extends TaskRunner { neededBuiltIns.toDataRequestParams(), ); + /** + * We request node types only when we know a task needs all nodes, because + * needing all nodes means that the task relies on paired item functionality, + * which is the same requirement for needing node types. + */ + if (neededBuiltIns.needsAllNodes) { + const uniqueNodeTypes = new Map( + data.workflow.nodes.map((node) => [ + `${node.type}|${node.typeVersion}`, + { name: node.type, version: node.typeVersion }, + ]), + ); + + const unknownNodeTypes = this.nodeTypes.onlyUnknown([...uniqueNodeTypes.values()]); + + const nodeTypes = await this.requestNodeTypes( + task.taskId, + unknownNodeTypes, + ); + + this.nodeTypes.addNodeTypeDescriptions(nodeTypes); + } + const workflowParams = data.workflow; const workflow = new Workflow({ ...workflowParams, diff --git a/packages/@n8n/task-runner/src/message-types.ts b/packages/@n8n/task-runner/src/message-types.ts index 95c5f5b72f..b5f17f965e 100644 --- a/packages/@n8n/task-runner/src/message-types.ts +++ b/packages/@n8n/task-runner/src/message-types.ts @@ -1,6 +1,11 @@ import type { INodeTypeBaseDescription } from 'n8n-workflow'; -import type { RPC_ALLOW_LIST, TaskDataRequestParams, TaskResultData } from './runner-types'; +import type { + NeededNodeType, + RPC_ALLOW_LIST, + TaskDataRequestParams, + TaskResultData, +} from './runner-types'; export namespace BrokerMessage { export namespace ToRunner { @@ -47,6 +52,8 @@ export namespace BrokerMessage { export interface NodeTypes { type: 'broker:nodetypes'; + taskId: string; + requestId: string; nodeTypes: INodeTypeBaseDescription[]; } @@ -87,6 +94,13 @@ export namespace BrokerMessage { requestParams: TaskDataRequestParams; } + export interface NodeTypesRequest { + type: 'broker:nodetypesrequest'; + taskId: string; + requestId: string; + requestParams: NeededNodeType[]; + } + export interface RPC { type: 'broker:rpc'; callId: string; @@ -95,7 +109,7 @@ export namespace BrokerMessage { params: unknown[]; } - export type All = TaskReady | TaskDone | TaskError | TaskDataRequest | RPC; + export type All = TaskReady | TaskDone | TaskError | TaskDataRequest | NodeTypesRequest | RPC; } } @@ -120,6 +134,13 @@ export namespace RequesterMessage { data: unknown; } + export interface NodeTypesResponse { + type: 'requester:nodetypesresponse'; + taskId: string; + requestId: string; + nodeTypes: INodeTypeBaseDescription[]; + } + export interface RPCResponse { type: 'requester:rpcresponse'; taskId: string; @@ -134,7 +155,13 @@ export namespace RequesterMessage { taskType: string; } - export type All = TaskSettings | TaskCancel | RPCResponse | TaskDataResponse | TaskRequest; + export type All = + | TaskSettings + | TaskCancel + | RPCResponse + | TaskDataResponse + | NodeTypesResponse + | TaskRequest; } } @@ -183,6 +210,25 @@ export namespace RunnerMessage { requestParams: TaskDataRequestParams; } + export interface NodeTypesRequest { + type: 'runner:nodetypesrequest'; + taskId: string; + requestId: string; + + /** + * Which node types should be included in the runner's node types request. + * + * Node types are needed only when the script relies on paired item functionality. + * If so, we need only the node types not already cached in the runner. + * + * TODO: In future we can trim this down to only node types in the paired item chain, + * rather than assuming we need all node types in the workflow. + * + * @example [{ name: 'n8n-nodes-base.httpRequest', version: 1 }] + */ + requestParams: NeededNodeType[]; + } + export interface RPC { type: 'runner:rpc'; callId: string; @@ -199,6 +245,7 @@ export namespace RunnerMessage { | TaskRejected | TaskOffer | RPC - | TaskDataRequest; + | TaskDataRequest + | NodeTypesRequest; } } diff --git a/packages/@n8n/task-runner/src/node-types.ts b/packages/@n8n/task-runner/src/node-types.ts index 046321bff9..8f910134b5 100644 --- a/packages/@n8n/task-runner/src/node-types.ts +++ b/packages/@n8n/task-runner/src/node-types.ts @@ -7,6 +7,8 @@ import { type IVersionedNodeType, } from 'n8n-workflow'; +import type { NeededNodeType } from './runner-types'; + type VersionedTypes = Map; export const DEFAULT_NODETYPE_VERSION = 1; @@ -61,4 +63,30 @@ export class TaskRunnerNodeTypes implements INodeTypes { getKnownTypes(): IDataObject { throw new ApplicationError('Unimplemented `getKnownTypes`', { level: 'error' }); } + + addNodeTypeDescriptions(nodeTypeDescriptions: INodeTypeDescription[]) { + const newNodeTypes = this.parseNodeTypes(nodeTypeDescriptions); + + for (const [name, newVersions] of newNodeTypes.entries()) { + if (!this.nodeTypesByVersion.has(name)) { + this.nodeTypesByVersion.set(name, newVersions); + } else { + const existingVersions = this.nodeTypesByVersion.get(name)!; + for (const [version, nodeType] of newVersions.entries()) { + existingVersions.set(version, nodeType); + } + } + } + } + + /** Filter out node type versions that are already registered. */ + onlyUnknown(nodeTypes: NeededNodeType[]) { + return nodeTypes.filter(({ name, version }) => { + const existingVersions = this.nodeTypesByVersion.get(name); + + if (!existingVersions) return true; + + return !existingVersions.has(version); + }); + } } diff --git a/packages/@n8n/task-runner/src/runner-types.ts b/packages/@n8n/task-runner/src/runner-types.ts index c55b50bf4c..836c42ed49 100644 --- a/packages/@n8n/task-runner/src/runner-types.ts +++ b/packages/@n8n/task-runner/src/runner-types.ts @@ -112,3 +112,6 @@ export const RPC_ALLOW_LIST = [ 'helpers.httpRequest', 'logNodeOutput', ] as const; + +/** Node types needed for the runner to execute a task. */ +export type NeededNodeType = { name: string; version: number }; diff --git a/packages/@n8n/task-runner/src/task-runner.ts b/packages/@n8n/task-runner/src/task-runner.ts index 81de93e6b0..4bd8661daa 100644 --- a/packages/@n8n/task-runner/src/task-runner.ts +++ b/packages/@n8n/task-runner/src/task-runner.ts @@ -1,4 +1,4 @@ -import { ApplicationError, type INodeTypeDescription } from 'n8n-workflow'; +import { ApplicationError } from 'n8n-workflow'; import { nanoid } from 'nanoid'; import { type MessageEvent, WebSocket } from 'ws'; @@ -25,6 +25,12 @@ interface DataRequest { reject: (error: unknown) => void; } +interface NodeTypesRequest { + requestId: string; + resolve: (data: unknown) => void; + reject: (error: unknown) => void; +} + interface RPCCall { callId: string; resolve: (data: unknown) => void; @@ -58,6 +64,8 @@ export abstract class TaskRunner { dataRequests: Map = new Map(); + nodeTypesRequests: Map = new Map(); + rpcCalls: Map = new Map(); nodeTypes: TaskRunnerNodeTypes = new TaskRunnerNodeTypes([]); @@ -168,15 +176,11 @@ export abstract class TaskRunner { this.handleRpcResponse(message.callId, message.status, message.data); break; case 'broker:nodetypes': - this.setNodeTypes(message.nodeTypes as unknown as INodeTypeDescription[]); + this.processNodeTypesResponse(message.requestId, message.nodeTypes); break; } } - setNodeTypes(nodeTypes: INodeTypeDescription[]) { - this.nodeTypes = new TaskRunnerNodeTypes(nodeTypes); - } - processDataResponse(requestId: string, data: unknown) { const request = this.dataRequests.get(requestId); if (!request) { @@ -187,6 +191,16 @@ export abstract class TaskRunner { request.resolve(data); } + processNodeTypesResponse(requestId: string, nodeTypes: unknown) { + const request = this.nodeTypesRequests.get(requestId); + + if (!request) return; + + // Deleting of the request is handled in `requestNodeTypes`, using a + // `finally` wrapped around the return + request.resolve(nodeTypes); + } + hasOpenTasks() { return Object.values(this.runningTasks).length < this.maxConcurrency; } @@ -282,6 +296,34 @@ export abstract class TaskRunner { throw new ApplicationError('Unimplemented'); } + async requestNodeTypes( + taskId: Task['taskId'], + requestParams: RunnerMessage.ToBroker.NodeTypesRequest['requestParams'], + ) { + const requestId = nanoid(); + + const nodeTypesPromise = new Promise((resolve, reject) => { + this.nodeTypesRequests.set(requestId, { + requestId, + resolve: resolve as (data: unknown) => void, + reject, + }); + }); + + this.send({ + type: 'runner:nodetypesrequest', + taskId, + requestId, + requestParams, + }); + + try { + return await nodeTypesPromise; + } finally { + this.nodeTypesRequests.delete(requestId); + } + } + async requestData( taskId: Task['taskId'], requestParams: RunnerMessage.ToBroker.TaskDataRequest['requestParams'], diff --git a/packages/cli/src/node-types.ts b/packages/cli/src/node-types.ts index 26b1b61e36..553aedd620 100644 --- a/packages/cli/src/node-types.ts +++ b/packages/cli/src/node-types.ts @@ -1,3 +1,4 @@ +import type { NeededNodeType } from '@n8n/task-runner'; import type { Dirent } from 'fs'; import { readdir } from 'fs/promises'; import { loadClassInIsolation } from 'n8n-core'; @@ -149,4 +150,22 @@ export class NodeTypes implements INodeTypes { dirent.name.toLowerCase().startsWith('v') ); } + + getNodeTypeDescriptions(nodeTypes: NeededNodeType[]): INodeTypeDescription[] { + return nodeTypes.map(({ name: nodeTypeName, version: nodeTypeVersion }) => { + const nodeType = this.getNode(nodeTypeName); + + if (!nodeType) throw new ApplicationError(`Unknown node type: ${nodeTypeName}`); + + const { description } = NodeHelpers.getVersionedNodeType(nodeType.type, nodeTypeVersion); + + const descriptionCopy = { ...description }; + + descriptionCopy.name = descriptionCopy.name.startsWith('n8n-nodes') + ? descriptionCopy.name + : `n8n-nodes-base.${descriptionCopy.name}`; // nodes-base nodes are unprefixed + + return descriptionCopy; + }); + } } diff --git a/packages/cli/src/runners/__tests__/task-broker.test.ts b/packages/cli/src/runners/__tests__/task-broker.test.ts index 715c5d8eb8..614d04c3b5 100644 --- a/packages/cli/src/runners/__tests__/task-broker.test.ts +++ b/packages/cli/src/runners/__tests__/task-broker.test.ts @@ -1,5 +1,6 @@ import type { RunnerMessage, TaskResultData } from '@n8n/task-runner'; import { mock } from 'jest-mock-extended'; +import type { INodeTypeBaseDescription } from 'n8n-workflow'; import { TaskRejectError } from '../errors'; import { TaskBroker } from '../task-broker.service'; @@ -11,7 +12,7 @@ describe('TaskBroker', () => { let taskBroker: TaskBroker; beforeEach(() => { - taskBroker = new TaskBroker(mock(), mock()); + taskBroker = new TaskBroker(mock()); jest.restoreAllMocks(); }); @@ -76,13 +77,6 @@ describe('TaskBroker', () => { const messageCallback = jest.fn(); taskBroker.registerRunner(runner, messageCallback); - - expect(messageCallback).toBeCalledWith({ - type: 'broker:nodetypes', - // We're mocking the node types service, so this will - // be undefined. - nodeType: undefined, - }); }); }); @@ -560,5 +554,68 @@ describe('TaskBroker', () => { params: rpcParams, }); }); + + it('should handle `runner:nodetypesrequest` message', async () => { + const runnerId = 'runner1'; + const taskId = 'task1'; + const requesterId = 'requester1'; + const requestId = 'request1'; + const requestParams = [ + { + name: 'n8n-nodes-base.someNode', + version: 1, + }, + ]; + + const message: RunnerMessage.ToBroker.NodeTypesRequest = { + type: 'runner:nodetypesrequest', + taskId, + requestId, + requestParams, + }; + + const requesterMessageCallback = jest.fn(); + + taskBroker.registerRunner(mock({ id: runnerId }), jest.fn()); + taskBroker.setTasks({ + [taskId]: { id: taskId, runnerId, requesterId, taskType: 'test' }, + }); + taskBroker.registerRequester(requesterId, requesterMessageCallback); + + await taskBroker.onRunnerMessage(runnerId, message); + + expect(requesterMessageCallback).toHaveBeenCalledWith({ + type: 'broker:nodetypesrequest', + taskId, + requestId, + requestParams, + }); + }); + }); + + describe('onRequesterMessage', () => { + it('should handle `requester:nodetypesresponse` message', async () => { + const runnerId = 'runner1'; + const taskId = 'task1'; + const requesterId = 'requester1'; + const requestId = 'request1'; + const nodeTypes = [mock(), mock()]; + + const runnerMessageCallback = jest.fn(); + + taskBroker.registerRunner(mock({ id: runnerId }), runnerMessageCallback); + taskBroker.setTasks({ + [taskId]: { id: taskId, runnerId, requesterId, taskType: 'test' }, + }); + + await taskBroker.handleRequesterNodeTypesResponse(taskId, requestId, nodeTypes); + + expect(runnerMessageCallback).toHaveBeenCalledWith({ + type: 'broker:nodetypes', + taskId, + requestId, + nodeTypes, + }); + }); }); }); diff --git a/packages/cli/src/runners/runner-ws-server.ts b/packages/cli/src/runners/runner-ws-server.ts index baaac82bf3..c691462558 100644 --- a/packages/cli/src/runners/runner-ws-server.ts +++ b/packages/cli/src/runners/runner-ws-server.ts @@ -70,7 +70,7 @@ export class TaskRunnerWsServer { this.sendMessage.bind(this, id) as MessageCallback, ); - this.logger.info(`Runner "${message.name}"(${id}) has been registered`); + this.logger.info(`Runner "${message.name}" (${id}) has been registered`); return; } diff --git a/packages/cli/src/runners/task-broker.service.ts b/packages/cli/src/runners/task-broker.service.ts index 7d00cf232e..daa5b48c07 100644 --- a/packages/cli/src/runners/task-broker.service.ts +++ b/packages/cli/src/runners/task-broker.service.ts @@ -8,7 +8,6 @@ import { ApplicationError } from 'n8n-workflow'; import { nanoid } from 'nanoid'; import { Service } from 'typedi'; -import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { Logger } from '@/logging/logger.service'; import { TaskRejectError } from './errors'; @@ -79,19 +78,7 @@ export class TaskBroker { private pendingTaskRequests: TaskRequest[] = []; - constructor( - private readonly logger: Logger, - private readonly loadNodesAndCredentials: LoadNodesAndCredentials, - ) { - this.loadNodesAndCredentials.addPostProcessor(this.updateNodeTypes); - } - - updateNodeTypes = async () => { - await this.messageAllRunners({ - type: 'broker:nodetypes', - nodeTypes: this.loadNodesAndCredentials.types.nodes, - }); - }; + constructor(private readonly logger: Logger) {} expireTasks() { const now = process.hrtime.bigint(); @@ -105,10 +92,6 @@ export class TaskBroker { registerRunner(runner: TaskRunner, messageCallback: MessageCallback) { this.knownRunners.set(runner.id, { runner, messageCallback }); void this.knownRunners.get(runner.id)!.messageCallback({ type: 'broker:runnerregistered' }); - void this.knownRunners.get(runner.id)!.messageCallback({ - type: 'broker:nodetypes', - nodeTypes: this.loadNodesAndCredentials.types.nodes, - }); } deregisterRunner(runnerId: string, error: Error) { @@ -145,14 +128,6 @@ export class TaskBroker { await this.knownRunners.get(runnerId)?.messageCallback(message); } - private async messageAllRunners(message: BrokerMessage.ToRunner.All) { - await Promise.allSettled( - [...this.knownRunners.values()].map(async (runner) => { - await runner.messageCallback(message); - }), - ); - } - private async messageRequester(requesterId: string, message: BrokerMessage.ToRequester.All) { await this.requesters.get(requesterId)?.(message); } @@ -187,7 +162,9 @@ export class TaskBroker { case 'runner:taskdatarequest': await this.handleDataRequest(message.taskId, message.requestId, message.requestParams); break; - + case 'runner:nodetypesrequest': + await this.handleNodeTypesRequest(message.taskId, message.requestId, message.requestParams); + break; case 'runner:rpc': await this.handleRpcRequest(message.taskId, message.callId, message.name, message.params); break; @@ -249,6 +226,23 @@ export class TaskBroker { }); } + async handleNodeTypesRequest( + taskId: Task['id'], + requestId: RunnerMessage.ToBroker.NodeTypesRequest['requestId'], + requestParams: RunnerMessage.ToBroker.NodeTypesRequest['requestParams'], + ) { + const task = this.tasks.get(taskId); + if (!task) { + return; + } + await this.messageRequester(task.requesterId, { + type: 'broker:nodetypesrequest', + taskId, + requestId, + requestParams, + }); + } + async handleResponse( taskId: Task['id'], requestId: RunnerMessage.ToBroker.TaskDataRequest['requestId'], @@ -284,6 +278,13 @@ export class TaskBroker { case 'requester:taskdataresponse': await this.handleRequesterDataResponse(message.taskId, message.requestId, message.data); break; + case 'requester:nodetypesresponse': + await this.handleRequesterNodeTypesResponse( + message.taskId, + message.requestId, + message.nodeTypes, + ); + break; case 'requester:rpcresponse': await this.handleRequesterRpcResponse( message.taskId, @@ -322,6 +323,21 @@ export class TaskBroker { }); } + async handleRequesterNodeTypesResponse( + taskId: Task['id'], + requestId: RequesterMessage.ToBroker.NodeTypesResponse['requestId'], + nodeTypes: RequesterMessage.ToBroker.NodeTypesResponse['nodeTypes'], + ) { + const runner = await this.getRunnerOrFailTask(taskId); + + await this.messageRunner(runner.id, { + type: 'broker:nodetypes', + taskId, + requestId, + nodeTypes, + }); + } + handleRequesterAccept( taskId: Task['id'], settings: RequesterMessage.ToBroker.TaskSettings['settings'], diff --git a/packages/cli/src/runners/task-managers/local-task-manager.ts b/packages/cli/src/runners/task-managers/local-task-manager.ts index 623b9c912e..7d898aaebe 100644 --- a/packages/cli/src/runners/task-managers/local-task-manager.ts +++ b/packages/cli/src/runners/task-managers/local-task-manager.ts @@ -1,17 +1,20 @@ import type { RequesterMessage } from '@n8n/task-runner'; -import Container from 'typedi'; +import Container, { Service } from 'typedi'; + +import { NodeTypes } from '@/node-types'; import { TaskManager } from './task-manager'; import type { RequesterMessageCallback } from '../task-broker.service'; import { TaskBroker } from '../task-broker.service'; +@Service() export class LocalTaskManager extends TaskManager { taskBroker: TaskBroker; id: string = 'single-main'; - constructor() { - super(); + constructor(nodeTypes: NodeTypes) { + super(nodeTypes); this.registerRequester(); } diff --git a/packages/cli/src/runners/task-managers/task-manager.ts b/packages/cli/src/runners/task-managers/task-manager.ts index 617008a450..21d4e8eb9d 100644 --- a/packages/cli/src/runners/task-managers/task-manager.ts +++ b/packages/cli/src/runners/task-managers/task-manager.ts @@ -17,6 +17,9 @@ import type { } from 'n8n-workflow'; import { createResultOk, createResultError } from 'n8n-workflow'; import { nanoid } from 'nanoid'; +import { Service } from 'typedi'; + +import { NodeTypes } from '@/node-types'; import { DataRequestResponseBuilder } from './data-request-response-builder'; @@ -43,7 +46,8 @@ interface ExecuteFunctionObject { [name: string]: ((...args: unknown[]) => unknown) | ExecuteFunctionObject; } -export class TaskManager { +@Service() +export abstract class TaskManager { requestAcceptRejects: Map = new Map(); taskAcceptRejects: Map = new Map(); @@ -52,6 +56,8 @@ export class TaskManager { tasks: Map = new Map(); + constructor(private readonly nodeTypes: NodeTypes) {} + async startTask( additionalData: IWorkflowExecuteAdditionalData, taskType: string, @@ -173,6 +179,9 @@ export class TaskManager { case 'broker:taskdatarequest': this.sendTaskData(message.taskId, message.requestId, message.requestParams); break; + case 'broker:nodetypesrequest': + this.sendNodeTypes(message.taskId, message.requestId, message.requestParams); + break; case 'broker:rpc': void this.handleRpc(message.taskId, message.callId, message.name, message.params); break; @@ -239,6 +248,21 @@ export class TaskManager { }); } + sendNodeTypes( + taskId: string, + requestId: string, + neededNodeTypes: BrokerMessage.ToRequester.NodeTypesRequest['requestParams'], + ) { + const nodeTypes = this.nodeTypes.getNodeTypeDescriptions(neededNodeTypes); + + this.sendMessage({ + type: 'requester:nodetypesresponse', + taskId, + requestId, + nodeTypes, + }); + } + async handleRpc( taskId: string, callId: string, diff --git a/packages/cli/src/runners/task-runner-module.ts b/packages/cli/src/runners/task-runner-module.ts index 13521c599b..f1383f1fba 100644 --- a/packages/cli/src/runners/task-runner-module.ts +++ b/packages/cli/src/runners/task-runner-module.ts @@ -54,7 +54,7 @@ export class TaskRunnerModule { private async loadTaskManager() { const { TaskManager } = await import('@/runners/task-managers/task-manager'); const { LocalTaskManager } = await import('@/runners/task-managers/local-task-manager'); - this.taskManager = new LocalTaskManager(); + this.taskManager = Container.get(LocalTaskManager); Container.set(TaskManager, this.taskManager); } From a6070afdda29631fd36e5213f52bf815268bcda4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 6 Nov 2024 13:50:41 +0100 Subject: [PATCH 07/38] fix(core): Include `projectId` in range query middleware (#11590) --- .../parse-range-query.middleware.test.ts | 16 ++++++++++++++++ packages/cli/src/executions/execution.service.ts | 1 + 2 files changed, 17 insertions(+) diff --git a/packages/cli/src/executions/__tests__/parse-range-query.middleware.test.ts b/packages/cli/src/executions/__tests__/parse-range-query.middleware.test.ts index 8b3395b226..3c8fa93053 100644 --- a/packages/cli/src/executions/__tests__/parse-range-query.middleware.test.ts +++ b/packages/cli/src/executions/__tests__/parse-range-query.middleware.test.ts @@ -108,6 +108,22 @@ describe('`parseRangeQuery` middleware', () => { expect(nextFn).toBeCalledTimes(1); }); + test('should parse `projectId` field', () => { + const req = mock({ + query: { + filter: '{ "projectId": "123" }', + limit: undefined, + firstId: undefined, + lastId: undefined, + }, + }); + + parseRangeQuery(req, res, nextFn); + + expect(req.rangeQuery.projectId).toBe('123'); + expect(nextFn).toBeCalledTimes(1); + }); + test('should delete invalid fields', () => { const req = mock({ query: { diff --git a/packages/cli/src/executions/execution.service.ts b/packages/cli/src/executions/execution.service.ts index 5f4ec0c535..60dadfdc1b 100644 --- a/packages/cli/src/executions/execution.service.ts +++ b/packages/cli/src/executions/execution.service.ts @@ -66,6 +66,7 @@ export const schemaGetExecutionsQueryFilter = { startedBefore: { type: 'date-time' }, annotationTags: { type: 'array', items: { type: 'string' } }, vote: { type: 'string' }, + projectId: { type: 'string' }, }, $defs: { metadata: { From 7a2be77f384a32ede3acad8fe24fb89227c058bf Mon Sep 17 00:00:00 2001 From: Ria Scholz <123465523+riascho@users.noreply.github.com> Date: Wed, 6 Nov 2024 18:58:26 +0530 Subject: [PATCH 08/38] feat(Gmail Trigger Node): Add filter option to include drafts (#11441) --- .../nodes/Google/Gmail/GmailTrigger.node.ts | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/packages/nodes-base/nodes/Google/Gmail/GmailTrigger.node.ts b/packages/nodes-base/nodes/Google/Gmail/GmailTrigger.node.ts index c0bacf2085..9fcac0e41d 100644 --- a/packages/nodes-base/nodes/Google/Gmail/GmailTrigger.node.ts +++ b/packages/nodes-base/nodes/Google/Gmail/GmailTrigger.node.ts @@ -24,7 +24,7 @@ export class GmailTrigger implements INodeType { name: 'gmailTrigger', icon: 'file:gmail.svg', group: ['trigger'], - version: [1, 1.1], + version: [1, 1.1, 1.2], description: 'Fetches emails from Gmail and starts the workflow on specified polling intervals.', subtitle: '={{"Gmail Trigger"}}', @@ -106,6 +106,13 @@ export class GmailTrigger implements INodeType { default: false, description: 'Whether to include messages from SPAM and TRASH in the results', }, + { + displayName: 'Include Drafts', + name: 'includeDrafts', + type: 'boolean', + default: false, + description: 'Whether to include email drafts in the results', + }, { displayName: 'Label Names or IDs', name: 'labelIds', @@ -284,6 +291,15 @@ export class GmailTrigger implements INodeType { qs.format = 'raw'; } + let includeDrafts; + if (node.typeVersion > 1.1) { + includeDrafts = (qs.includeDrafts as boolean) ?? false; + } else { + includeDrafts = (qs.includeDrafts as boolean) ?? true; + } + delete qs.includeDrafts; + const withoutDrafts = []; + for (let i = 0; i < responseData.length; i++) { responseData[i] = await googleApiRequest.call( this, @@ -292,8 +308,12 @@ export class GmailTrigger implements INodeType { {}, qs, ); - - if (!simple) { + if (!includeDrafts) { + if (responseData[i].labelIds.includes('DRAFT')) { + continue; + } + } + if (!simple && responseData?.length) { const dataPropertyNameDownload = (options.dataPropertyAttachmentsPrefixName as string) || 'attachment_'; @@ -303,9 +323,14 @@ export class GmailTrigger implements INodeType { dataPropertyNameDownload, ); } + withoutDrafts.push(responseData[i]); } - if (simple) { + if (!includeDrafts) { + responseData = withoutDrafts; + } + + if (simple && responseData?.length) { responseData = this.helpers.returnJsonArray( await simplifyOutput.call(this, responseData as IDataObject[]), ); @@ -324,17 +349,14 @@ export class GmailTrigger implements INodeType { }, ); } - if (!responseData?.length) { nodeStaticData.lastTimeChecked = endDate; return null; } const emailsWithInvalidDate = new Set(); - const getEmailDateAsSeconds = (email: IDataObject): number => { let date; - if (email.internalDate) { date = +(email.internalDate as string) / 1000; } else if (email.date) { From 5c69ba2c44ceb2b3a28c0cf218db44ab16d7f07d Mon Sep 17 00:00:00 2001 From: Eugene Date: Wed, 6 Nov 2024 14:35:45 +0100 Subject: [PATCH 09/38] chore: Create table and typeorm entity for test definitions (no-changelog) (#11505) Co-authored-by: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> --- packages/cli/src/databases/dsl/table.ts | 4 +- packages/cli/src/databases/entities/index.ts | 2 + .../src/databases/entities/test-definition.ts | 61 +++++++++++++++++++ ...1730386903556-CreateTestDefinitionTable.ts | 37 +++++++++++ .../src/databases/migrations/mysqldb/index.ts | 2 + .../databases/migrations/postgresdb/index.ts | 2 + .../src/databases/migrations/sqlite/index.ts | 2 + packages/cli/src/generic-helpers.ts | 2 + 8 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 packages/cli/src/databases/entities/test-definition.ts create mode 100644 packages/cli/src/databases/migrations/common/1730386903556-CreateTestDefinitionTable.ts diff --git a/packages/cli/src/databases/dsl/table.ts b/packages/cli/src/databases/dsl/table.ts index 78ed98272c..f598b674d7 100644 --- a/packages/cli/src/databases/dsl/table.ts +++ b/packages/cli/src/databases/dsl/table.ts @@ -50,8 +50,8 @@ export class CreateTable extends TableOperation { ref: { tableName: string; columnName: string; - onDelete?: 'CASCADE'; - onUpdate?: 'CASCADE'; + onDelete?: 'RESTRICT' | 'CASCADE' | 'NO ACTION' | 'SET NULL'; + onUpdate?: 'RESTRICT' | 'CASCADE' | 'NO ACTION' | 'SET NULL'; name?: string; }, ) { diff --git a/packages/cli/src/databases/entities/index.ts b/packages/cli/src/databases/entities/index.ts index 39f67b3252..b73b348d8a 100644 --- a/packages/cli/src/databases/entities/index.ts +++ b/packages/cli/src/databases/entities/index.ts @@ -20,6 +20,7 @@ import { Settings } from './settings'; import { SharedCredentials } from './shared-credentials'; import { SharedWorkflow } from './shared-workflow'; import { TagEntity } from './tag-entity'; +import { TestDefinition } from './test-definition'; import { User } from './user'; import { Variables } from './variables'; import { WebhookEntity } from './webhook-entity'; @@ -58,4 +59,5 @@ export const entities = { ProjectRelation, ApiKey, ProcessedData, + TestDefinition, }; diff --git a/packages/cli/src/databases/entities/test-definition.ts b/packages/cli/src/databases/entities/test-definition.ts new file mode 100644 index 0000000000..5395bd0c4c --- /dev/null +++ b/packages/cli/src/databases/entities/test-definition.ts @@ -0,0 +1,61 @@ +import { + Column, + Entity, + Generated, + Index, + ManyToOne, + OneToOne, + PrimaryColumn, + RelationId, +} from '@n8n/typeorm'; +import { Length } from 'class-validator'; + +import { AnnotationTagEntity } from '@/databases/entities/annotation-tag-entity.ee'; +import { WorkflowEntity } from '@/databases/entities/workflow-entity'; + +import { WithTimestamps } from './abstract-entity'; + +/** + * Entity representing a Test Definition + * It combines: + * - the workflow under test + * - the workflow used to evaluate the results of test execution + * - the filter used to select test cases from previous executions of the workflow under test - annotation tag + */ +@Entity() +@Index(['workflow']) +@Index(['evaluationWorkflow']) +export class TestDefinition extends WithTimestamps { + @Generated() + @PrimaryColumn() + id: number; + + @Column({ length: 255 }) + @Length(1, 255, { message: 'Test name must be $constraint1 to $constraint2 characters long.' }) + name: string; + + /** + * Relation to the workflow under test + */ + @ManyToOne('WorkflowEntity', 'tests') + workflow: WorkflowEntity; + + @RelationId((test: TestDefinition) => test.workflow) + workflowId: string; + + /** + * Relation to the workflow used to evaluate the results of test execution + */ + @ManyToOne('WorkflowEntity', 'evaluationTests') + evaluationWorkflow: WorkflowEntity; + + @RelationId((test: TestDefinition) => test.evaluationWorkflow) + evaluationWorkflowId: string; + + /** + * Relation to the annotation tag associated with the test + * This tag will be used to select the test cases to run from previous executions + */ + @OneToOne('AnnotationTagEntity', 'test') + annotationTag: AnnotationTagEntity; +} diff --git a/packages/cli/src/databases/migrations/common/1730386903556-CreateTestDefinitionTable.ts b/packages/cli/src/databases/migrations/common/1730386903556-CreateTestDefinitionTable.ts new file mode 100644 index 0000000000..d71353ee73 --- /dev/null +++ b/packages/cli/src/databases/migrations/common/1730386903556-CreateTestDefinitionTable.ts @@ -0,0 +1,37 @@ +import type { MigrationContext, ReversibleMigration } from '@/databases/types'; + +const testEntityTableName = 'test_definition'; + +export class CreateTestDefinitionTable1730386903556 implements ReversibleMigration { + async up({ schemaBuilder: { createTable, column } }: MigrationContext) { + await createTable(testEntityTableName) + .withColumns( + column('id').int.notNull.primary.autoGenerate, + column('name').varchar(255).notNull, + column('workflowId').varchar(36).notNull, + column('evaluationWorkflowId').varchar(36), + column('annotationTagId').varchar(16), + ) + .withIndexOn('workflowId') + .withIndexOn('evaluationWorkflowId') + .withForeignKey('workflowId', { + tableName: 'workflow_entity', + columnName: 'id', + onDelete: 'CASCADE', + }) + .withForeignKey('evaluationWorkflowId', { + tableName: 'workflow_entity', + columnName: 'id', + onDelete: 'SET NULL', + }) + .withForeignKey('annotationTagId', { + tableName: 'annotation_tag_entity', + columnName: 'id', + onDelete: 'SET NULL', + }).withTimestamps; + } + + async down({ schemaBuilder: { dropTable } }: MigrationContext) { + await dropTable(testEntityTableName); + } +} diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts index ff40fd9dc0..ebe7cf76c0 100644 --- a/packages/cli/src/databases/migrations/mysqldb/index.ts +++ b/packages/cli/src/databases/migrations/mysqldb/index.ts @@ -68,6 +68,7 @@ import { CreateProcessedDataTable1726606152711 } from '../common/1726606152711-C import { SeparateExecutionCreationFromStart1727427440136 } from '../common/1727427440136-SeparateExecutionCreationFromStart'; import { AddMissingPrimaryKeyOnAnnotationTagMapping1728659839644 } from '../common/1728659839644-AddMissingPrimaryKeyOnAnnotationTagMapping'; import { UpdateProcessedDataValueColumnToText1729607673464 } from '../common/1729607673464-UpdateProcessedDataValueColumnToText'; +import { CreateTestDefinitionTable1730386903556 } from '../common/1730386903556-CreateTestDefinitionTable'; export const mysqlMigrations: Migration[] = [ InitialMigration1588157391238, @@ -138,4 +139,5 @@ export const mysqlMigrations: Migration[] = [ CreateProcessedDataTable1726606152711, AddMissingPrimaryKeyOnAnnotationTagMapping1728659839644, UpdateProcessedDataValueColumnToText1729607673464, + CreateTestDefinitionTable1730386903556, ]; diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts index f3ac7e0474..731ddc2680 100644 --- a/packages/cli/src/databases/migrations/postgresdb/index.ts +++ b/packages/cli/src/databases/migrations/postgresdb/index.ts @@ -68,6 +68,7 @@ import { CreateProcessedDataTable1726606152711 } from '../common/1726606152711-C import { SeparateExecutionCreationFromStart1727427440136 } from '../common/1727427440136-SeparateExecutionCreationFromStart'; import { AddMissingPrimaryKeyOnAnnotationTagMapping1728659839644 } from '../common/1728659839644-AddMissingPrimaryKeyOnAnnotationTagMapping'; import { UpdateProcessedDataValueColumnToText1729607673464 } from '../common/1729607673464-UpdateProcessedDataValueColumnToText'; +import { CreateTestDefinitionTable1730386903556 } from '../common/1730386903556-CreateTestDefinitionTable'; export const postgresMigrations: Migration[] = [ InitialMigration1587669153312, @@ -138,4 +139,5 @@ export const postgresMigrations: Migration[] = [ CreateProcessedDataTable1726606152711, AddMissingPrimaryKeyOnAnnotationTagMapping1728659839644, UpdateProcessedDataValueColumnToText1729607673464, + CreateTestDefinitionTable1730386903556, ]; diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts index e53b5f43bd..8004894ccf 100644 --- a/packages/cli/src/databases/migrations/sqlite/index.ts +++ b/packages/cli/src/databases/migrations/sqlite/index.ts @@ -65,6 +65,7 @@ import { CreateAnnotationTables1724753530828 } from '../common/1724753530828-Cre import { CreateProcessedDataTable1726606152711 } from '../common/1726606152711-CreateProcessedDataTable'; import { SeparateExecutionCreationFromStart1727427440136 } from '../common/1727427440136-SeparateExecutionCreationFromStart'; import { UpdateProcessedDataValueColumnToText1729607673464 } from '../common/1729607673464-UpdateProcessedDataValueColumnToText'; +import { CreateTestDefinitionTable1730386903556 } from '../common/1730386903556-CreateTestDefinitionTable'; const sqliteMigrations: Migration[] = [ InitialMigration1588102412422, @@ -132,6 +133,7 @@ const sqliteMigrations: Migration[] = [ CreateProcessedDataTable1726606152711, AddMissingPrimaryKeyOnAnnotationTagMapping1728659839644, UpdateProcessedDataValueColumnToText1729607673464, + CreateTestDefinitionTable1730386903556, ]; export { sqliteMigrations }; diff --git a/packages/cli/src/generic-helpers.ts b/packages/cli/src/generic-helpers.ts index e5978bb34a..378619a4e9 100644 --- a/packages/cli/src/generic-helpers.ts +++ b/packages/cli/src/generic-helpers.ts @@ -3,6 +3,7 @@ import { validate } from 'class-validator'; import type { AnnotationTagEntity } from '@/databases/entities/annotation-tag-entity.ee'; import type { CredentialsEntity } from '@/databases/entities/credentials-entity'; import type { TagEntity } from '@/databases/entities/tag-entity'; +import type { TestDefinition } from '@/databases/entities/test-definition'; import type { User } from '@/databases/entities/user'; import type { WorkflowEntity } from '@/databases/entities/workflow-entity'; @@ -12,6 +13,7 @@ import { BadRequestError } from './errors/response-errors/bad-request.error'; export async function validateEntity( entity: | WorkflowEntity + | TestDefinition | CredentialsEntity | TagEntity | AnnotationTagEntity From 7326487ab35f3078a5d4da8f0221c7ed7d67c926 Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Wed, 6 Nov 2024 14:48:41 +0100 Subject: [PATCH 10/38] refactor(editor): Migrate WorkflowSettings to setup script and composition API (no-changelog) (#11580) --- .../src/components/WorkflowSettings.vue | 885 ++++++++---------- 1 file changed, 412 insertions(+), 473 deletions(-) diff --git a/packages/editor-ui/src/components/WorkflowSettings.vue b/packages/editor-ui/src/components/WorkflowSettings.vue index 1ecac538a6..c552e8327c 100644 --- a/packages/editor-ui/src/components/WorkflowSettings.vue +++ b/packages/editor-ui/src/components/WorkflowSettings.vue @@ -1,13 +1,10 @@ - @@ -836,7 +775,7 @@ export default defineComponent({ :disabled="readOnlyEnv || !workflowPermissions.update" :model-value="timeoutHMS.hours" :min="0" - @update:model-value="(value: string) => setTimeout('hours', value)" + @update:model-value="(value: string) => setTheTimeout('hours', value)" > @@ -847,7 +786,7 @@ export default defineComponent({ :model-value="timeoutHMS.minutes" :min="0" :max="60" - @update:model-value="(value: string) => setTimeout('minutes', value)" + @update:model-value="(value: string) => setTheTimeout('minutes', value)" > @@ -858,7 +797,7 @@ export default defineComponent({ :model-value="timeoutHMS.seconds" :min="0" :max="60" - @update:model-value="(value: string) => setTimeout('seconds', value)" + @update:model-value="(value: string) => setTheTimeout('seconds', value)" > From 80224727848150f5c3da131a8e8ac6eef5960da5 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Wed, 6 Nov 2024 16:09:10 +0200 Subject: [PATCH 11/38] feat: Enable turbosnap on chromatic workflow (no-changelog) (#11595) --- .github/workflows/chromatic.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/chromatic.yml b/.github/workflows/chromatic.yml index 7c5682076a..feb39f1f4f 100644 --- a/.github/workflows/chromatic.yml +++ b/.github/workflows/chromatic.yml @@ -65,6 +65,7 @@ jobs: continue-on-error: true with: workingDir: packages/design-system + onlyChanged: true projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }} exitZeroOnChanges: false From 93fae5d8a7eaeb1aaa123313e88fb4315949ffb4 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Wed, 6 Nov 2024 16:16:47 +0200 Subject: [PATCH 12/38] fix: Fix loading workflows with `null` connection value (no-changelog) (#11592) --- .../editor-ui/src/utils/canvasUtilsV2.test.ts | 92 ++++++++++- packages/editor-ui/src/utils/canvasUtilsV2.ts | 4 +- packages/workflow/src/Workflow.ts | 3 +- packages/workflow/test/Workflow.test.ts | 151 ++++++++++++++++++ 4 files changed, 239 insertions(+), 11 deletions(-) diff --git a/packages/editor-ui/src/utils/canvasUtilsV2.test.ts b/packages/editor-ui/src/utils/canvasUtilsV2.test.ts index 25ede479f2..088a219738 100644 --- a/packages/editor-ui/src/utils/canvasUtilsV2.test.ts +++ b/packages/editor-ui/src/utils/canvasUtilsV2.test.ts @@ -7,7 +7,8 @@ import { parseCanvasConnectionHandleString, checkOverlap, } from '@/utils/canvasUtilsV2'; -import { type IConnections, type INodeTypeDescription, NodeConnectionType } from 'n8n-workflow'; +import { NodeConnectionType } from 'n8n-workflow'; +import type { IConnections, INodeTypeDescription, IConnection } from 'n8n-workflow'; import type { CanvasConnection } from '@/types'; import { CanvasConnectionMode } from '@/types'; import type { INodeUi } from '@/Interface'; @@ -22,7 +23,7 @@ describe('mapLegacyConnectionsToCanvasConnections', () => { it('should map legacy connections to canvas connections', () => { const legacyConnections: IConnections = { 'Node A': { - main: [[{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }]], + [NodeConnectionType.Main]: [[{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }]], }, }; const nodes: INodeUi[] = [ @@ -93,7 +94,7 @@ describe('mapLegacyConnectionsToCanvasConnections', () => { it('should return empty array when no matching nodes found', () => { const legacyConnections: IConnections = { 'Node A': { - main: [[{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }]], + [NodeConnectionType.Main]: [[{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }]], }, }; const nodes: INodeUi[] = []; @@ -138,7 +139,7 @@ describe('mapLegacyConnectionsToCanvasConnections', () => { it('should map multiple connections between the same nodes', () => { const legacyConnections: IConnections = { 'Node A': { - main: [ + [NodeConnectionType.Main]: [ [{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }], [{ node: 'Node B', type: NodeConnectionType.Main, index: 1 }], ], @@ -249,7 +250,7 @@ describe('mapLegacyConnectionsToCanvasConnections', () => { it('should map multiple connections from one node to different nodes', () => { const legacyConnections: IConnections = { 'Node A': { - main: [ + [NodeConnectionType.Main]: [ [{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }], [{ node: 'Node C', type: NodeConnectionType.Main, index: 0 }], ], @@ -368,13 +369,13 @@ describe('mapLegacyConnectionsToCanvasConnections', () => { it('should map complex node setup with mixed inputs and outputs', () => { const legacyConnections: IConnections = { 'Node A': { - main: [[{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }]], + [NodeConnectionType.Main]: [[{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }]], [NodeConnectionType.AiMemory]: [ [{ node: 'Node C', type: NodeConnectionType.AiMemory, index: 1 }], ], }, 'Node B': { - main: [[{ node: 'Node C', type: NodeConnectionType.Main, index: 0 }]], + [NodeConnectionType.Main]: [[{ node: 'Node C', type: NodeConnectionType.Main, index: 0 }]], }, }; const nodes: INodeUi[] = [ @@ -527,7 +528,7 @@ describe('mapLegacyConnectionsToCanvasConnections', () => { it('should handle edge cases with invalid data gracefully', () => { const legacyConnections: IConnections = { 'Node A': { - main: [ + [NodeConnectionType.Main]: [ [{ node: 'Nonexistent Node', type: NodeConnectionType.Main, index: 0 }], [{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }], ], @@ -599,6 +600,81 @@ describe('mapLegacyConnectionsToCanvasConnections', () => { }, ]); }); + + // @issue https://linear.app/n8n/issue/N8N-7880/cannot-load-some-templates + it('should handle null connections gracefully', () => { + const legacyConnections: IConnections = { + 'Node A': { + [NodeConnectionType.Main]: [ + null as unknown as IConnection[], + [{ node: 'Node B', type: NodeConnectionType.Main, index: 0 }], + ], + }, + }; + const nodes: INodeUi[] = [ + { + id: '1', + name: 'Node A', + type: 'n8n-nodes-base.node', + typeVersion: 1, + position: [100, 100], + parameters: {}, + }, + { + id: '2', + name: 'Node B', + type: 'n8n-nodes-base.node', + typeVersion: 1, + position: [200, 200], + parameters: {}, + }, + ]; + + const result: CanvasConnection[] = mapLegacyConnectionsToCanvasConnections( + legacyConnections, + nodes, + ); + + const source = nodes[0].id; + const sourceHandle = createCanvasConnectionHandleString({ + mode: CanvasConnectionMode.Output, + type: NodeConnectionType.Main, + index: 1, + }); + const target = nodes[1].id; + const targetHandle = createCanvasConnectionHandleString({ + mode: CanvasConnectionMode.Input, + type: NodeConnectionType.Main, + index: 0, + }); + const id = createCanvasConnectionId({ + source, + target, + sourceHandle, + targetHandle, + }); + + expect(result).toEqual([ + { + id, + source, + target, + sourceHandle, + targetHandle, + data: { + fromNodeName: nodes[0].name, + source: { + index: 1, + type: NodeConnectionType.Main, + }, + target: { + index: 0, + type: NodeConnectionType.Main, + }, + }, + }, + ]); + }); }); describe('parseCanvasConnectionHandleString', () => { diff --git a/packages/editor-ui/src/utils/canvasUtilsV2.ts b/packages/editor-ui/src/utils/canvasUtilsV2.ts index c4a49d8c94..c86e4a7f2c 100644 --- a/packages/editor-ui/src/utils/canvasUtilsV2.ts +++ b/packages/editor-ui/src/utils/canvasUtilsV2.ts @@ -20,8 +20,8 @@ export function mapLegacyConnectionsToCanvasConnections( fromConnectionTypes.forEach((fromConnectionType) => { const fromPorts = legacyConnections[fromNodeName][fromConnectionType]; - fromPorts.forEach((toPorts, fromIndex) => { - toPorts.forEach((toPort) => { + fromPorts?.forEach((toPorts, fromIndex) => { + toPorts?.forEach((toPort) => { const toId = nodes.find((node) => node.name === toPort.node)?.id ?? ''; const toConnectionType = toPort.type as NodeConnectionType; const toIndex = toPort.index; diff --git a/packages/workflow/src/Workflow.ts b/packages/workflow/src/Workflow.ts index c60b949449..ea569e5886 100644 --- a/packages/workflow/src/Workflow.ts +++ b/packages/workflow/src/Workflow.ts @@ -180,7 +180,8 @@ export class Workflow { if (!connections[sourceNode][type].hasOwnProperty(inputIndex)) { continue; } - for (connectionInfo of connections[sourceNode][type][inputIndex]) { + + for (connectionInfo of connections[sourceNode][type][inputIndex] ?? []) { if (!returnConnection.hasOwnProperty(connectionInfo.node)) { returnConnection[connectionInfo.node] = {}; } diff --git a/packages/workflow/test/Workflow.test.ts b/packages/workflow/test/Workflow.test.ts index bdd5fec8b9..ded592c9a6 100644 --- a/packages/workflow/test/Workflow.test.ts +++ b/packages/workflow/test/Workflow.test.ts @@ -1,6 +1,7 @@ import { mock } from 'jest-mock-extended'; import { NodeConnectionType } from '@/Interfaces'; +import type { IConnection } from '@/Interfaces'; import type { IBinaryKeyData, IConnections, @@ -2084,4 +2085,154 @@ describe('Workflow', () => { expect(triggerResponse.closeFunction).toHaveBeenCalled(); }); }); + + describe('__getConnectionsByDestination', () => { + it('should return empty object when there are no connections', () => { + const workflow = new Workflow({ + nodes: [], + connections: {}, + active: false, + nodeTypes: mock(), + }); + + const result = workflow.__getConnectionsByDestination({}); + + expect(result).toEqual({}); + }); + + it('should return connections by destination node', () => { + const connections: IConnections = { + Node1: { + [NodeConnectionType.Main]: [ + [ + { node: 'Node2', type: NodeConnectionType.Main, index: 0 }, + { node: 'Node3', type: NodeConnectionType.Main, index: 1 }, + ], + ], + }, + }; + const workflow = new Workflow({ + nodes: [], + connections, + active: false, + nodeTypes: mock(), + }); + const result = workflow.__getConnectionsByDestination(connections); + expect(result).toEqual({ + Node2: { + [NodeConnectionType.Main]: [[{ node: 'Node1', type: NodeConnectionType.Main, index: 0 }]], + }, + Node3: { + [NodeConnectionType.Main]: [ + [], + [{ node: 'Node1', type: NodeConnectionType.Main, index: 0 }], + ], + }, + }); + }); + + it('should handle multiple connection types', () => { + const connections: IConnections = { + Node1: { + [NodeConnectionType.Main]: [[{ node: 'Node2', type: NodeConnectionType.Main, index: 0 }]], + [NodeConnectionType.AiAgent]: [ + [{ node: 'Node3', type: NodeConnectionType.AiAgent, index: 0 }], + ], + }, + }; + + const workflow = new Workflow({ + nodes: [], + connections, + active: false, + nodeTypes: mock(), + }); + + const result = workflow.__getConnectionsByDestination(connections); + expect(result).toEqual({ + Node2: { + [NodeConnectionType.Main]: [[{ node: 'Node1', type: NodeConnectionType.Main, index: 0 }]], + }, + Node3: { + [NodeConnectionType.AiAgent]: [ + [{ node: 'Node1', type: NodeConnectionType.AiAgent, index: 0 }], + ], + }, + }); + }); + + it('should handle nodes with no connections', () => { + const connections: IConnections = { + Node1: { + [NodeConnectionType.Main]: [[]], + }, + }; + + const workflow = new Workflow({ + nodes: [], + connections, + active: false, + nodeTypes: mock(), + }); + + const result = workflow.__getConnectionsByDestination(connections); + expect(result).toEqual({}); + }); + + // @issue https://linear.app/n8n/issue/N8N-7880/cannot-load-some-templates + it('should handle nodes with null connections', () => { + const connections: IConnections = { + Node1: { + [NodeConnectionType.Main]: [ + null as unknown as IConnection[], + [{ node: 'Node2', type: NodeConnectionType.Main, index: 0 }], + ], + }, + }; + + const workflow = new Workflow({ + nodes: [], + connections, + active: false, + nodeTypes: mock(), + }); + + const result = workflow.__getConnectionsByDestination(connections); + expect(result).toEqual({ + Node2: { + [NodeConnectionType.Main]: [[{ node: 'Node1', type: NodeConnectionType.Main, index: 1 }]], + }, + }); + }); + + it('should handle nodes with multiple input connections', () => { + const connections: IConnections = { + Node1: { + [NodeConnectionType.Main]: [[{ node: 'Node2', type: NodeConnectionType.Main, index: 0 }]], + }, + Node3: { + [NodeConnectionType.Main]: [[{ node: 'Node2', type: NodeConnectionType.Main, index: 0 }]], + }, + }; + + const workflow = new Workflow({ + nodes: [], + connections, + active: false, + nodeTypes: mock(), + }); + + const result = workflow.__getConnectionsByDestination(connections); + expect(result).toEqual({ + Node2: { + [NodeConnectionType.Main]: [ + [ + { node: 'Node1', type: NodeConnectionType.Main, index: 0 }, + { node: 'Node3', type: NodeConnectionType.Main, index: 0 }, + ], + ], + }, + }); + }); + }); }); From 499c54b29a52baa44314596c9f1f3098d08e194d Mon Sep 17 00:00:00 2001 From: Shireen Missi <94372015+ShireenMissi@users.noreply.github.com> Date: Wed, 6 Nov 2024 14:40:49 +0000 Subject: [PATCH 13/38] refactor(editor): Migrate FixedCollectionParameter to composition API (#11555) Co-authored-by: Elias Meire --- .../src/components/ExpressionEditModal.vue | 1 + .../FixedCollectionParameter.test.ts | 110 +++++ .../components/FixedCollectionParameter.vue | 460 +++++++++--------- .../src/components/MultipleParameter.vue | 18 +- .../src/components/ParameterInput.vue | 26 +- .../src/components/ParameterInputFull.vue | 5 +- .../src/components/ParameterInputList.vue | 25 +- 7 files changed, 386 insertions(+), 259 deletions(-) create mode 100644 packages/editor-ui/src/components/FixedCollectionParameter.test.ts diff --git a/packages/editor-ui/src/components/ExpressionEditModal.vue b/packages/editor-ui/src/components/ExpressionEditModal.vue index 934876f75e..08e3848c43 100644 --- a/packages/editor-ui/src/components/ExpressionEditModal.vue +++ b/packages/editor-ui/src/components/ExpressionEditModal.vue @@ -22,6 +22,7 @@ import DraggableTarget from './DraggableTarget.vue'; import { dropInExpressionEditor } from '@/plugins/codemirror/dragAndDrop'; import { APP_MODALS_ELEMENT_ID } from '@/constants'; +import { N8nInput, N8nText } from 'n8n-design-system'; type Props = { parameter: INodeProperties; diff --git a/packages/editor-ui/src/components/FixedCollectionParameter.test.ts b/packages/editor-ui/src/components/FixedCollectionParameter.test.ts new file mode 100644 index 0000000000..9438a5a78a --- /dev/null +++ b/packages/editor-ui/src/components/FixedCollectionParameter.test.ts @@ -0,0 +1,110 @@ +import { createComponentRenderer } from '@/__tests__/render'; +import { cleanupAppModals, createAppModals, SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils'; +import FixedCollectionParameter, { type Props } from '@/components/FixedCollectionParameter.vue'; +import { STORES } from '@/constants'; +import { createTestingPinia } from '@pinia/testing'; +import userEvent from '@testing-library/user-event'; +import { setActivePinia } from 'pinia'; +describe('FixedCollectionParameter.vue', () => { + const pinia = createTestingPinia({ + initialState: { + [STORES.SETTINGS]: { + settings: SETTINGS_STORE_DEFAULT_STATE.settings, + }, + }, + }); + setActivePinia(pinia); + + const props: Props = { + parameter: { + displayName: 'Routing Rules', + name: 'rules', + placeholder: 'Add Routing Rule', + type: 'fixedCollection', + typeOptions: { + multipleValues: true, + sortable: true, + }, + default: '', + options: [ + { + name: 'values', + displayName: 'Values', + values: [ + { + displayName: 'Output Name', + name: 'outputKey', + type: 'string', + default: 'Default Output Name', + }, + ], + }, + ], + }, + path: 'parameters.rules', + nodeValues: { + parameters: { + rules: { values: [{ outputKey: 'Test Output Name' }] }, + }, + }, + values: { + values: [{ outputKey: 'Test Output Name' }], + }, + isReadOnly: false, + }; + const renderComponent = createComponentRenderer(FixedCollectionParameter, { props }); + + beforeEach(() => { + createAppModals(); + }); + + afterEach(() => { + cleanupAppModals(); + }); + + it('renders the component', () => { + const { getByTestId } = renderComponent(); + expect(getByTestId('fixed-collection-rules')).toBeInTheDocument(); + expect(getByTestId('fixed-collection-add')).toBeInTheDocument(); + expect(getByTestId('fixed-collection-delete')).toBeInTheDocument(); + expect(getByTestId('parameter-item')).toBeInTheDocument(); + }); + + it('computes placeholder text correctly', () => { + const { getByTestId } = renderComponent(); + expect(getByTestId('fixed-collection-add')).toHaveTextContent('Add Routing Rule'); + }); + + it('emits valueChanged event on option creation', async () => { + const { getByTestId, emitted } = renderComponent(); + await userEvent.click(getByTestId('fixed-collection-add')); + expect(emitted('valueChanged')).toEqual([ + [ + { + name: 'parameters.rules.values', + value: [{ outputKey: 'Test Output Name' }, { outputKey: 'Default Output Name' }], + }, + ], + ]); + }); + + it('emits valueChanged event on option deletion', async () => { + const { getByTestId, emitted } = renderComponent({ + props: { + ...props, + values: { + values: [{ outputKey: 'Test' }], + }, + }, + }); + await userEvent.click(getByTestId('fixed-collection-delete')); + expect(emitted('valueChanged')).toEqual([ + [ + { + name: 'parameters.rules.values', + value: undefined, + }, + ], + ]); + }); +}); diff --git a/packages/editor-ui/src/components/FixedCollectionParameter.vue b/packages/editor-ui/src/components/FixedCollectionParameter.vue index 98c5ef5971..de7359dfdd 100644 --- a/packages/editor-ui/src/components/FixedCollectionParameter.vue +++ b/packages/editor-ui/src/components/FixedCollectionParameter.vue @@ -1,226 +1,236 @@ -