From 1b8229161dadfbc8769ef267cb5356476d52334d Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Tue, 19 Jan 2021 08:47:02 +0100 Subject: [PATCH] :bug: Fix issue with nodes not getting executed #1351 --- packages/core/src/WorkflowExecute.ts | 8 +- packages/core/test/Helpers.ts | 438 ++++++++++++++++++++- packages/core/test/WorkflowExecute.test.ts | 416 +++++++++++++++++++ 3 files changed, 856 insertions(+), 6 deletions(-) diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts index 3d4bbce23e..6f342d39a4 100644 --- a/packages/core/src/WorkflowExecute.ts +++ b/packages/core/src/WorkflowExecute.ts @@ -714,7 +714,7 @@ export class WorkflowExecute { if (workflow.connectionsBySourceNode.hasOwnProperty(executionNode.name)) { if (workflow.connectionsBySourceNode[executionNode.name].hasOwnProperty('main')) { let outputIndex: string, connectionData: IConnection; - // Go over all the different + // Iterate over all the outputs // Add the nodes to be executed for (outputIndex in workflow.connectionsBySourceNode[executionNode.name]['main']) { @@ -722,14 +722,14 @@ export class WorkflowExecute { continue; } - // Go through all the different outputs of this connection + // Iterate over all the different connections of this output for (connectionData of workflow.connectionsBySourceNode[executionNode.name]['main'][outputIndex]) { if (!workflow.nodes.hasOwnProperty(connectionData.node)) { return Promise.reject(new Error(`The node "${executionNode.name}" connects to not found node "${connectionData.node}"`)); } - if (nodeSuccessData![outputIndex]) { - // Add the node only if it did execute + if (nodeSuccessData![outputIndex] && (nodeSuccessData![outputIndex].length !== 0 || connectionData.index > 0)) { + // Add the node only if it did execute or if connected to second "optional" input this.addNodeToBeExecuted(workflow, connectionData, parseInt(outputIndex, 10), executionNode.name, nodeSuccessData!, runIndex); } } diff --git a/packages/core/test/Helpers.ts b/packages/core/test/Helpers.ts index b792708332..c52e3ef7f4 100644 --- a/packages/core/test/Helpers.ts +++ b/packages/core/test/Helpers.ts @@ -3,6 +3,7 @@ import { set } from 'lodash'; import { ICredentialDataDecryptedObject, ICredentialsHelper, + IDataObject, IExecuteWorkflowInfo, INodeExecutionData, INodeParameters, @@ -13,6 +14,7 @@ import { ITaskData, IWorkflowBase, IWorkflowExecuteAdditionalData, + NodeParameterValue, WorkflowHooks, } from 'n8n-workflow'; @@ -39,6 +41,335 @@ export class CredentialsHelper extends ICredentialsHelper { class NodeTypesClass implements INodeTypes { nodeTypes: INodeTypeData = { + 'n8n-nodes-base.if': { + sourcePath: '', + type: { + description: { + displayName: 'If', + name: 'if', + group: ['transform'], + version: 1, + description: 'Splits a stream depending on defined compare operations.', + defaults: { + name: 'IF', + color: '#408000', + }, + inputs: ['main'], + outputs: ['main', 'main'], + properties: [ + { + displayName: 'Conditions', + name: 'conditions', + placeholder: 'Add Condition', + type: 'fixedCollection', + typeOptions: { + multipleValues: true, + }, + description: 'The type of values to compare.', + default: {}, + options: [ + { + name: 'boolean', + displayName: 'Boolean', + values: [ + { + displayName: 'Value 1', + name: 'value1', + type: 'boolean', + default: false, + description: 'The value to compare with the second one.', + }, + { + displayName: 'Operation', + name: 'operation', + type: 'options', + options: [ + { + name: 'Equal', + value: 'equal', + }, + { + name: 'Not Equal', + value: 'notEqual', + }, + ], + default: 'equal', + description: 'Operation to decide where the the data should be mapped to.', + }, + { + displayName: 'Value 2', + name: 'value2', + type: 'boolean', + default: false, + description: 'The value to compare with the first one.', + }, + ], + }, + { + name: 'number', + displayName: 'Number', + values: [ + { + displayName: 'Value 1', + name: 'value1', + type: 'number', + default: 0, + description: 'The value to compare with the second one.', + }, + { + displayName: 'Operation', + name: 'operation', + type: 'options', + options: [ + { + name: 'Smaller', + value: 'smaller', + }, + { + name: 'Smaller Equal', + value: 'smallerEqual', + }, + { + name: 'Equal', + value: 'equal', + }, + { + name: 'Not Equal', + value: 'notEqual', + }, + { + name: 'Larger', + value: 'larger', + }, + { + name: 'Larger Equal', + value: 'largerEqual', + }, + { + name: 'Is Empty', + value: 'isEmpty', + }, + ], + default: 'smaller', + description: 'Operation to decide where the the data should be mapped to.', + }, + { + displayName: 'Value 2', + name: 'value2', + type: 'number', + displayOptions: { + hide: { + operation: [ + 'isEmpty', + ], + }, + }, + default: 0, + description: 'The value to compare with the first one.', + }, + ], + }, + { + name: 'string', + displayName: 'String', + values: [ + { + displayName: 'Value 1', + name: 'value1', + type: 'string', + default: '', + description: 'The value to compare with the second one.', + }, + { + displayName: 'Operation', + name: 'operation', + type: 'options', + options: [ + { + name: 'Contains', + value: 'contains', + }, + { + name: 'Ends With', + value: 'endsWith', + }, + { + name: 'Equal', + value: 'equal', + }, + { + name: 'Not Contains', + value: 'notContains', + }, + { + name: 'Not Equal', + value: 'notEqual', + }, + { + name: 'Regex', + value: 'regex', + }, + { + name: 'Starts With', + value: 'startsWith', + }, + { + name: 'Is Empty', + value: 'isEmpty', + }, + ], + default: 'equal', + description: 'Operation to decide where the the data should be mapped to.', + }, + { + displayName: 'Value 2', + name: 'value2', + type: 'string', + displayOptions: { + hide: { + operation: [ + 'isEmpty', + 'regex', + ], + }, + }, + default: '', + description: 'The value to compare with the first one.', + }, + { + displayName: 'Regex', + name: 'value2', + type: 'string', + displayOptions: { + show: { + operation: [ + 'regex', + ], + }, + }, + default: '', + placeholder: '/text/i', + description: 'The regex which has to match.', + }, + ], + }, + ], + }, + { + displayName: 'Combine', + name: 'combineOperation', + type: 'options', + options: [ + { + name: 'ALL', + description: 'Only if all conditions are meet it goes into "true" branch.', + value: 'all', + }, + { + name: 'ANY', + description: 'If any of the conditions is meet it goes into "true" branch.', + value: 'any', + }, + ], + default: 'all', + description: 'If multiple rules got set this settings decides if it is true as soon as ANY condition matches or only if ALL get meet.', + }, + ], + }, + async execute(this: IExecuteFunctions): Promise { + const returnDataTrue: INodeExecutionData[] = []; + const returnDataFalse: INodeExecutionData[] = []; + + const items = this.getInputData(); + + let item: INodeExecutionData; + let combineOperation: string; + + // The compare operations + const compareOperationFunctions: { + [key: string]: (value1: NodeParameterValue, value2: NodeParameterValue) => boolean; + } = { + contains: (value1: NodeParameterValue, value2: NodeParameterValue) => (value1 || '').toString().includes((value2 || '').toString()), + notContains: (value1: NodeParameterValue, value2: NodeParameterValue) => !(value1 || '').toString().includes((value2 || '').toString()), + endsWith: (value1: NodeParameterValue, value2: NodeParameterValue) => (value1 as string).endsWith(value2 as string), + equal: (value1: NodeParameterValue, value2: NodeParameterValue) => value1 === value2, + notEqual: (value1: NodeParameterValue, value2: NodeParameterValue) => value1 !== value2, + larger: (value1: NodeParameterValue, value2: NodeParameterValue) => (value1 || 0) > (value2 || 0), + largerEqual: (value1: NodeParameterValue, value2: NodeParameterValue) => (value1 || 0) >= (value2 || 0), + smaller: (value1: NodeParameterValue, value2: NodeParameterValue) => (value1 || 0) < (value2 || 0), + smallerEqual: (value1: NodeParameterValue, value2: NodeParameterValue) => (value1 || 0) <= (value2 || 0), + startsWith: (value1: NodeParameterValue, value2: NodeParameterValue) => (value1 as string).startsWith(value2 as string), + isEmpty: (value1: NodeParameterValue) => [undefined, null, ''].includes(value1 as string), + regex: (value1: NodeParameterValue, value2: NodeParameterValue) => { + const regexMatch = (value2 || '').toString().match(new RegExp('^/(.*?)/([gimy]*)$')); + + let regex: RegExp; + if (!regexMatch) { + regex = new RegExp((value2 || '').toString()); + } else if (regexMatch.length === 1) { + regex = new RegExp(regexMatch[1]); + } else { + regex = new RegExp(regexMatch[1], regexMatch[2]); + } + + return !!(value1 || '').toString().match(regex); + }, + }; + + // The different dataTypes to check the values in + const dataTypes = [ + 'boolean', + 'number', + 'string', + ]; + + // Itterate over all items to check which ones should be output as via output "true" and + // which ones via output "false" + let dataType: string; + let compareOperationResult: boolean; + itemLoop: + for (let itemIndex = 0; itemIndex < items.length; itemIndex++) { + item = items[itemIndex]; + + let compareData: INodeParameters; + + combineOperation = this.getNodeParameter('combineOperation', itemIndex) as string; + + // Check all the values of the different dataTypes + for (dataType of dataTypes) { + // Check all the values of the current dataType + for (compareData of this.getNodeParameter(`conditions.${dataType}`, itemIndex, []) as INodeParameters[]) { + // Check if the values passes + compareOperationResult = compareOperationFunctions[compareData.operation as string](compareData.value1 as NodeParameterValue, compareData.value2 as NodeParameterValue); + + if (compareOperationResult === true && combineOperation === 'any') { + // If it passes and the operation is "any" we do not have to check any + // other ones as it should pass anyway. So go on with the next item. + returnDataTrue.push(item); + continue itemLoop; + } else if (compareOperationResult === false && combineOperation === 'all') { + // If it fails and the operation is "all" we do not have to check any + // other ones as it should be not pass anyway. So go on with the next item. + returnDataFalse.push(item); + continue itemLoop; + } + } + } + + if (combineOperation === 'all') { + // If the operation is "all" it means the item did match all conditions + // so it passes. + returnDataTrue.push(item); + } else { + // If the operation is "any" it means the the item did not match any condition. + returnDataFalse.push(item); + } + } + + return [returnDataTrue, returnDataFalse]; + }, + }, + }, 'n8n-nodes-base.merge': { sourcePath: '', type: { @@ -168,6 +499,26 @@ class NodeTypesClass implements INodeTypes { description: 'The value to set.', default: {}, options: [ + { + name: 'boolean', + displayName: 'Boolean', + values: [ + { + displayName: 'Name', + name: 'name', + type: 'string', + default: 'propertyName', + description: 'Name of the property to write data to.
Supports dot-notation.
Example: "data.person[0].name"', + }, + { + displayName: 'Value', + name: 'value', + type: 'boolean', + default: false, + description: 'The boolean value to write in the property.', + }, + ], + }, { name: 'number', displayName: 'Number', @@ -188,25 +539,108 @@ class NodeTypesClass implements INodeTypes { }, ], }, + { + name: 'string', + displayName: 'String', + values: [ + { + displayName: 'Name', + name: 'name', + type: 'string', + default: 'propertyName', + description: 'Name of the property to write data to.
Supports dot-notation.
Example: "data.person[0].name"', + }, + { + displayName: 'Value', + name: 'value', + type: 'string', + default: '', + description: 'The string value to write in the property.', + }, + ], + }, + ], + }, + + { + displayName: 'Options', + name: 'options', + type: 'collection', + placeholder: 'Add Option', + default: {}, + options: [ + { + displayName: 'Dot Notation', + name: 'dotNotation', + type: 'boolean', + default: true, + description: `By default does dot-notation get used in property names..
+ This means that "a.b" will set the property "b" underneath "a" so { "a": { "b": value} }.
+ If that is not intended this can be deactivated, it will then set { "a.b": value } instead. + `, + }, ], }, ], }, execute(this: IExecuteFunctions): Promise { + const items = this.getInputData(); + if (items.length === 0) { + items.push({ json: {} }); + } + const returnData: INodeExecutionData[] = []; + let item: INodeExecutionData; + let keepOnlySet: boolean; for (let itemIndex = 0; itemIndex < items.length; itemIndex++) { + keepOnlySet = this.getNodeParameter('keepOnlySet', itemIndex, []) as boolean; item = items[itemIndex]; + const options = this.getNodeParameter('options', itemIndex, {}) as IDataObject; const newItem: INodeExecutionData = { - json: JSON.parse(JSON.stringify(item.json)), + json: {}, }; + if (keepOnlySet !== true) { + if (item.binary !== undefined) { + // Create a shallow copy of the binary data so that the old + // data references which do not get changed still stay behind + // but the incoming data does not get changed. + newItem.binary = {}; + Object.assign(newItem.binary, item.binary); + } + + newItem.json = JSON.parse(JSON.stringify(item.json)); + } + + // Add boolean values + (this.getNodeParameter('values.boolean', itemIndex, []) as INodeParameters[]).forEach((setItem) => { + if (options.dotNotation === false) { + newItem.json[setItem.name as string] = !!setItem.value; + } else { + set(newItem.json, setItem.name as string, !!setItem.value); + } + }); + // Add number values (this.getNodeParameter('values.number', itemIndex, []) as INodeParameters[]).forEach((setItem) => { - set(newItem.json, setItem.name as string, setItem.value); + if (options.dotNotation === false) { + newItem.json[setItem.name as string] = setItem.value; + } else { + set(newItem.json, setItem.name as string, setItem.value); + } + }); + + // Add string values + (this.getNodeParameter('values.string', itemIndex, []) as INodeParameters[]).forEach((setItem) => { + if (options.dotNotation === false) { + newItem.json[setItem.name as string] = setItem.value; + } else { + set(newItem.json, setItem.name as string, setItem.value); + } }); returnData.push(newItem); diff --git a/packages/core/test/WorkflowExecute.test.ts b/packages/core/test/WorkflowExecute.test.ts index 4213cba397..38f25ba2a0 100644 --- a/packages/core/test/WorkflowExecute.test.ts +++ b/packages/core/test/WorkflowExecute.test.ts @@ -570,6 +570,422 @@ describe('WorkflowExecute', () => { }, }, }, + { + description: 'should run workflow also if node has multiple input connections and one is empty', + input: { + // Leave the workflowData in regular JSON to be able to easily + // copy it from/in the UI + workflowData: { + "nodes": [ + { + "parameters": {}, + "name": "Start", + "type": "n8n-nodes-base.start", + "typeVersion": 1, + "position": [ + 250, + 450 + ] + }, + { + "parameters": { + "conditions": { + "boolean": [], + "number": [ + { + "value1": "={{Object.keys($json).length}}", + "operation": "notEqual" + } + ] + } + }, + "name": "IF", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 650, + 350 + ] + }, + { + "parameters": {}, + "name": "Merge1", + "type": "n8n-nodes-base.merge", + "typeVersion": 1, + "position": [ + 1150, + 450 + ] + }, + { + "parameters": { + "values": { + "string": [ + { + "name": "test1", + "value": "a" + } + ] + }, + "options": {} + }, + "name": "Set1", + "type": "n8n-nodes-base.set", + "typeVersion": 1, + "position": [ + 450, + 450 + ] + }, + { + "parameters": { + "values": { + "string": [ + { + "name": "test2", + "value": "b" + } + ] + }, + "options": {} + }, + "name": "Set2", + "type": "n8n-nodes-base.set", + "typeVersion": 1, + "position": [ + 800, + 250 + ] + } + ], + "connections": { + "Start": { + "main": [ + [ + { + "node": "Set1", + "type": "main", + "index": 0 + } + ] + ] + }, + "IF": { + "main": [ + [ + { + "node": "Set2", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "Merge1", + "type": "main", + "index": 0 + } + ] + ] + }, + "Set1": { + "main": [ + [ + { + "node": "IF", + "type": "main", + "index": 0 + }, + { + "node": "Merge1", + "type": "main", + "index": 1 + } + ] + ] + }, + "Set2": { + "main": [ + [ + { + "node": "Merge1", + "type": "main", + "index": 0 + } + ] + ] + } + } + }, + }, + output: { + nodeExecutionOrder: [ + 'Start', + 'Set1', + 'IF', + 'Set2', + 'Merge1', + ], + nodeData: { + Merge1: [ + [ + { + test1: 'a', + test2: 'b', + }, + { + test1: 'a', + }, + ], + ], + }, + }, + }, + { + description: 'should use empty data if second input does not have any data', + input: { + // Leave the workflowData in regular JSON to be able to easily + // copy it from/in the UI + workflowData: { + "nodes": [ + { + "parameters": {}, + "name": "Start", + "type": "n8n-nodes-base.start", + "typeVersion": 1, + "position": [ + 250, + 300 + ] + }, + { + "parameters": {}, + "name": "Merge", + "type": "n8n-nodes-base.merge", + "typeVersion": 1, + "position": [ + 800, + 450 + ] + }, + { + "parameters": {}, + "name": "Merge1", + "type": "n8n-nodes-base.merge", + "typeVersion": 1, + "position": [ + 1000, + 300 + ] + }, + { + "parameters": { + "conditions": { + "boolean": [ + { + "value2": true + } + ], + "string": [ + { + "value1": "={{$json[\"key\"]}}", + "value2": "a" + } + ] + }, + "combineOperation": "any" + }, + "name": "IF", + "type": "n8n-nodes-base.if", + "typeVersion": 1, + "position": [ + 600, + 600 + ], + "alwaysOutputData": false + }, + { + "parameters": { + "values": { + "number": [ + { + "name": "number0" + } + ], + "string": [ + { + "name": "key", + "value": "a" + } + ] + }, + "options": {} + }, + "name": "Set0", + "type": "n8n-nodes-base.set", + "typeVersion": 1, + "position": [ + 450, + 300 + ] + }, + { + "parameters": { + "values": { + "number": [ + { + "name": "number1", + "value": 1 + } + ], + "string": [ + { + "name": "key", + "value": "b" + } + ] + }, + "options": {} + }, + "name": "Set1", + "type": "n8n-nodes-base.set", + "typeVersion": 1, + "position": [ + 450, + 450 + ] + }, + { + "parameters": { + "values": { + "number": [ + { + "name": "number2", + "value": 2 + } + ], + "string": [ + { + "name": "key", + "value": "c" + } + ] + }, + "options": {} + }, + "name": "Set2", + "type": "n8n-nodes-base.set", + "typeVersion": 1, + "position": [ + 450, + 600 + ] + } + ], + "connections": { + "Start": { + "main": [ + [ + { + "node": "Set0", + "type": "main", + "index": 0 + } + ] + ] + }, + "Merge": { + "main": [ + [ + { + "node": "Merge1", + "type": "main", + "index": 1 + } + ] + ] + }, + "IF": { + "main": [ + [ + { + "node": "Merge", + "type": "main", + "index": 1 + } + ] + ] + }, + "Set0": { + "main": [ + [ + { + "node": "Merge1", + "type": "main", + "index": 0 + } + ] + ] + }, + "Set1": { + "main": [ + [ + { + "node": "Merge", + "type": "main", + "index": 0 + } + ] + ] + }, + "Set2": { + "main": [ + [ + { + "node": "IF", + "type": "main", + "index": 0 + } + ] + ] + } + } + }, + }, + output: { + nodeExecutionOrder: [ + 'Start', + 'Set0', + 'Set2', + 'IF', + 'Set1', + 'Merge', + 'Merge1', + ], + nodeData: { + Merge: [ + [ + { + number1: 1, + key: "b", + }, + ], + ], + Merge1: [ + [ + { + number0: 0, + key: "a", + }, + { + number1: 1, + key: "b", + }, + ], + ], + }, + }, + }, ];