mudhorn/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2025-01-27 04:12:38 -08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

refactor(core): Use options object for functions with lots of arguments to make the call site more readable and avoid errors (#11199)

Browse source
This commit is contained in:
Danny Martini 2024-10-10 12:52:10 +02:00 committed by GitHub
parent 9431e3029c
commit 8e6ddfe028
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 93 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

75
packages/core/src/PartialExecutionUtils/__tests__/findStartNodes.test.ts
View file

@ -46,7 +46,7 @@ describe('findStartNodes', () => {
const node = createNodeData({ name: 'Basic Node' });
const graph = new DirectedGraph().addNode(node);
const startNodes = findStartNodes(graph, node, node);
const startNodes = findStartNodes({ graph, trigger: node, destination: node });
expect(startNodes).toHaveLength(1);
expect(startNodes[0]).toEqual(node);
@ -65,7 +65,7 @@ describe('findStartNodes', () => {
// if the trigger has no run data
{
const startNodes = findStartNodes(graph, trigger, destination);
const startNodes = findStartNodes({ graph, trigger, destination });
expect(startNodes).toHaveLength(1);
expect(startNodes[0]).toEqual(trigger);
@ -77,7 +77,7 @@ describe('findStartNodes', () => {
[trigger.name]: [toITaskData([{ data: { value: 1 } }])],
};
const startNodes = findStartNodes(graph, trigger, destination, runData);
const startNodes = findStartNodes({ graph, trigger, destination, runData });
expect(startNodes).toHaveLength(1);
expect(startNodes[0]).toEqual(destination);
@ -112,7 +112,7 @@ describe('findStartNodes', () => {
};
// ACT
const startNodes = findStartNodes(graph, trigger, node, runData);
const startNodes = findStartNodes({ graph, trigger, destination: node, runData });
// ASSERT
expect(startNodes).toHaveLength(1);
@ -153,7 +153,7 @@ describe('findStartNodes', () => {
{
// ACT
const startNodes = findStartNodes(graph, trigger, node4);
const startNodes = findStartNodes({ graph, trigger, destination: node4 });
// ASSERT
expect(startNodes).toHaveLength(1);
@ -172,7 +172,7 @@ describe('findStartNodes', () => {
};
// ACT
const startNodes = findStartNodes(graph, trigger, node4, runData);
const startNodes = findStartNodes({ graph, trigger, destination: node4, runData });
// ASSERT
expect(startNodes).toHaveLength(1);
@ -201,8 +201,13 @@ describe('findStartNodes', () => {
);
// ACT
const startNodes = findStartNodes(graph, trigger, node, {
[trigger.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 0 }])],
const startNodes = findStartNodes({
graph,
trigger,
destination: node,
runData: {
[trigger.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 0 }])],
},
});
// ASSERT
@ -231,8 +236,13 @@ describe('findStartNodes', () => {
);
// ACT
const startNodes = findStartNodes(graph, trigger, node, {
[trigger.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 1 }])],
const startNodes = findStartNodes({
graph,
trigger,
destination: node,
runData: {
[trigger.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 1 }])],
},
});
// ASSERT
@ -261,13 +271,18 @@ describe('findStartNodes', () => {
);
// ACT
const startNodes = findStartNodes(graph, trigger, node, {
[trigger.name]: [
toITaskData([
{ data: { value: 1 }, outputIndex: 0 },
{ data: { value: 1 }, outputIndex: 1 },
]),
],
const startNodes = findStartNodes({
graph,
trigger,
destination: node,
runData: {
[trigger.name]: [
toITaskData([
{ data: { value: 1 }, outputIndex: 0 },
{ data: { value: 1 }, outputIndex: 1 },
]),
],
},
});
// ASSERT
@ -297,10 +312,15 @@ describe('findStartNodes', () => {
);
// ACT
const startNodes = findStartNodes(graph, trigger, node3, {
[trigger.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 0 }])],
[node1.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 0 }])],
[node2.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 0 }])],
const startNodes = findStartNodes({
graph,
trigger,
destination: node3,
runData: {
[trigger.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 0 }])],
[node1.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 0 }])],
[node2.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 0 }])],
},
});
// ASSERT
@ -329,9 +349,14 @@ describe('findStartNodes', () => {
);
// ACT
const startNodes = findStartNodes(graph, node1, node2, {
[trigger.name]: [toITaskData([{ data: { value: 1 } }])],
[node1.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 1 }])],
const startNodes = findStartNodes({
graph,
trigger: node1,
destination: node2,
runData: {
[trigger.name]: [toITaskData([{ data: { value: 1 } }])],
[node1.name]: [toITaskData([{ data: { value: 1 }, outputIndex: 1 }])],
},
});
// ASSERT
@ -364,7 +389,7 @@ describe('findStartNodes', () => {
const pinData: IPinData = {};
// ACT
const startNodes = findStartNodes(graph, trigger, node2, runData, pinData);
const startNodes = findStartNodes({ graph, trigger, destination: node2, runData, pinData });
// ASSERT
expect(startNodes).toHaveLength(1);

22
packages/core/src/PartialExecutionUtils/__tests__/findSubgraph.test.ts
View file

@ -28,7 +28,7 @@ describe('findSubgraph', () => {
.addNodes(trigger, destination)
.addConnections({ from: trigger, to: destination });
const subgraph = findSubgraph(graph, destination, trigger);
const subgraph = findSubgraph({ graph, destination, trigger });
expect(subgraph).toEqual(graph);
});
@ -50,7 +50,7 @@ describe('findSubgraph', () => {
{ from: ifNode, to: noOp, outputIndex: 1 },
);
const subgraph = findSubgraph(graph, noOp, ifNode);
const subgraph = findSubgraph({ graph, destination: noOp, trigger: ifNode });
expect(subgraph).toEqual(graph);
});
@ -70,7 +70,7 @@ describe('findSubgraph', () => {
.addNodes(trigger, destination, node)
.addConnections({ from: trigger, to: destination }, { from: destination, to: node });
const subgraph = findSubgraph(graph, destination, trigger);
const subgraph = findSubgraph({ graph, destination, trigger });
expect(subgraph).toEqual(
new DirectedGraph()
@ -100,7 +100,7 @@ describe('findSubgraph', () => {
.addNodes(trigger, disabled, destination)
.addConnections({ from: trigger, to: disabled }, { from: disabled, to: destination });
const subgraph = findSubgraph(graph, destination, trigger);
const subgraph = findSubgraph({ graph, destination, trigger });
expect(subgraph).toEqual(
new DirectedGraph()
@ -133,7 +133,7 @@ describe('findSubgraph', () => {
);
// ACT
const subgraph = findSubgraph(graph, destination, trigger);
const subgraph = findSubgraph({ graph, destination, trigger });
// ASSERT
expect(subgraph).toEqual(
@ -163,7 +163,7 @@ describe('findSubgraph', () => {
);
// ACT
const subgraph = findSubgraph(graph, node2, trigger);
const subgraph = findSubgraph({ graph, destination: node2, trigger });
// ASSERT
expect(subgraph).toEqual(graph);
@ -187,7 +187,7 @@ describe('findSubgraph', () => {
.addConnections({ from: trigger, to: node1 }, { from: node2, to: node1 });
// ACT
const subgraph = findSubgraph(graph, node1, trigger);
const subgraph = findSubgraph({ graph, destination: node1, trigger });
// ASSERT
expect(subgraph).toEqual(
@ -215,7 +215,7 @@ describe('findSubgraph', () => {
);
// ACT
const subgraph = findSubgraph(graph, destination, trigger);
const subgraph = findSubgraph({ graph, destination, trigger });
// ASSERT
expect(subgraph).toEqual(
@ -248,7 +248,7 @@ describe('findSubgraph', () => {
);
// ACT
const subgraph = findSubgraph(graph, destination, trigger);
const subgraph = findSubgraph({ graph, destination, trigger });
// ASSERT
expect(subgraph).toEqual(graph);
@ -284,7 +284,7 @@ describe('findSubgraph', () => {
);
// ACT
const subgraph = findSubgraph(graph, destination, trigger);
const subgraph = findSubgraph({ graph, destination, trigger });
// ASSERT
expect(subgraph.getConnections()).toHaveLength(0);
@ -320,7 +320,7 @@ describe('findSubgraph', () => {
);
// ACT
const subgraph = findSubgraph(graph, root, trigger);
const subgraph = findSubgraph({ graph, destination: root, trigger });
// ASSERT
expect(subgraph).toEqual(

2
packages/core/src/PartialExecutionUtils/__tests__/recreateNodeExecutionStack.test.ts
View file

@ -32,7 +32,7 @@ describe('recreateNodeExecutionStack', () => {
.addNodes(trigger, node)
.addConnections({ from: trigger, to: node });
const workflow = findSubgraph(graph, node, trigger);
const workflow = findSubgraph({ graph, destination: node, trigger });
const startNodes = [node];
const runData: IRunData = {
[trigger.name]: [toITaskData([{ data: { value: 1 } }])],

20
packages/core/src/PartialExecutionUtils/findStartNodes.ts
View file

@ -131,13 +131,19 @@ function findStartNodesRecursive(
* - stop following the branch, there is no start node on this branch
* 4. Recurse with every direct child that is part of the sub graph
*/
export function findStartNodes(
graph: DirectedGraph,
trigger: INode,
destination: INode,
runData: IRunData = {},
pinData: IPinData = {},
): INode[] {
export function findStartNodes(options: {
graph: DirectedGraph;
trigger: INode;
destination: INode;
runData?: IRunData;
pinData?: IPinData;
}): INode[] {
const graph = options.graph;
const trigger = options.trigger;
const destination = options.destination;
const runData = options.runData ?? {};
const pinData = options.pinData ?? {};
const startNodes = findStartNodesRecursive(
graph,
trigger,

15
packages/core/src/PartialExecutionUtils/findSubgraph.ts
View file

@ -105,14 +105,17 @@ function findSubgraphRecursive(
* dataflow in the graph they are utility nodes, like the AI model used in a
* lang chain node.
*/
export function findSubgraph(
graph: DirectedGraph,
destinationNode: INode,
trigger: INode,
): DirectedGraph {
export function findSubgraph(options: {
graph: DirectedGraph;
destination: INode;
trigger: INode;
}): DirectedGraph {
const graph = options.graph;
const destination = options.destination;
const trigger = options.trigger;
const subgraph = new DirectedGraph();
findSubgraphRecursive(graph, destinationNode, destinationNode, trigger, subgraph, []);
findSubgraphRecursive(graph, destination, destination, trigger, subgraph, []);
// For each node in the subgraph, if it has parent connections of a type that
// is not `Main` in the input graph, add the connections and the nodes

18
packages/core/src/WorkflowExecute.ts
View file

@ -87,7 +87,7 @@ export class WorkflowExecute {
* Executes the given workflow.
*
* @param {Workflow} workflow The workflow to execute
* @param {INode[]} [startNodes] Node to start execution from
* @param {INode[]} [startNode] Node to start execution from
* @param {string} [destinationNode] Node to stop execution at
*/
// IMPORTANT: Do not add "async" to this function, it will then convert the
@ -332,9 +332,9 @@ export class WorkflowExecute {
'a destinationNodeName is required for the new partial execution flow',
);
const destinationNode = workflow.getNode(destinationNodeName);
const destination = workflow.getNode(destinationNodeName);
assert.ok(
destinationNode,
destination,
`Could not find a node with the name ${destinationNodeName} in the workflow.`,
);
@ -348,11 +348,11 @@ export class WorkflowExecute {
// 2. Find the Subgraph
const graph = DirectedGraph.fromWorkflow(workflow);
const subgraph = findSubgraph(graph, destinationNode, trigger);
const subgraph = findSubgraph({ graph, destination, trigger });
const filteredNodes = subgraph.getNodes();
// 3. Find the Start Nodes
const startNodes = findStartNodes(subgraph, trigger, destinationNode, runData);
const startNodes = findStartNodes({ graph: subgraph, trigger, destination, runData });
// 4. Detect Cycles
const cycles = findCycles(workflow);
@ -367,7 +367,7 @@ export class WorkflowExecute {
// 7. Recreate Execution Stack
const { nodeExecutionStack, waitingExecution, waitingExecutionSource } =
recreateNodeExecutionStack(subgraph, startNodes, destinationNode, runData, pinData ?? {});
recreateNodeExecutionStack(subgraph, startNodes, destination, runData, pinData ?? {});
// 8. Execute
this.status = 'running';
@ -1058,7 +1058,7 @@ export class WorkflowExecute {
this.runExecutionData.startData!.runNodeFilter.indexOf(executionNode.name) === -1
) {
// If filter is set and node is not on filter skip it, that avoids the problem that it executes
// leafs that are parallel to a selected destinationNode. Normally it would execute them because
// leaves that are parallel to a selected destinationNode. Normally it would execute them because
// they have the same parent and it executes all child nodes.
continue;
}
@ -1759,7 +1759,7 @@ export class WorkflowExecute {
continue;
}
} else {
// A certain amout of inputs are required (amount of inputs)
// A certain amount of inputs are required (amount of inputs)
if (inputsWithData.length < requiredInputs) {
continue;
}
@ -1817,7 +1817,7 @@ export class WorkflowExecute {
// Node to add did not get found, rather an empty one removed so continue with search
waitingNodes = Object.keys(this.runExecutionData.executionData!.waitingExecution);
// Set counter to start again from the beginning. Set it to -1 as it auto increments
// after run. So only like that will we end up again ot 0.
// after run. So only like that will we end up again at 0.
i = -1;
}
}