fix(core): Account for nodes with renamable content (#6109)

🐛 Account for nodes with renamable content
This commit is contained in:
Iván Ovejero 2023-05-02 09:37:49 +02:00 committed by GitHub
parent 51f5990559
commit c99f158120
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 86 additions and 16 deletions

View file

@ -1,3 +1,13 @@
/* eslint-disable @typescript-eslint/naming-convention */
export const BINARY_ENCODING = 'base64';
export const WAIT_TIME_UNLIMITED = '3000-01-01T00:00:00.000Z';
/**
* Nodes whose parameter values may refer to other nodes without expressions.
* Their content may need to be updated when the referenced node is renamed.
*/
export const NODES_WITH_RENAMABLE_CONTENT = new Set([
'n8n-nodes-base.code',
'n8n-nodes-base.function',
'n8n-nodes-base.functionItem',
]);

View file

@ -55,6 +55,7 @@ import * as NodeHelpers from './NodeHelpers';
import * as ObservableObject from './ObservableObject';
import { RoutingNode } from './RoutingNode';
import { Expression } from './Expression';
import { NODES_WITH_RENAMABLE_CONTENT } from './Constants';
function dedupe<T>(arr: T[]): T[] {
return [...new Set(arr)];
@ -405,21 +406,18 @@ export class Workflow {
return this.pinData ? this.pinData[nodeName] : undefined;
}
/**
* Renames nodes in expressions
*
* @param {(NodeParameterValue | INodeParameters | NodeParameterValue[] | INodeParameters[])} parameterValue The parameters to check for expressions
* @param {string} currentName The current name of the node
* @param {string} newName The new name
*/
renameNodeInExpressions(
renameNodeInParameterValue(
parameterValue: NodeParameterValueType,
currentName: string,
newName: string,
{ hasRenamableContent } = { hasRenamableContent: false },
): NodeParameterValueType {
if (typeof parameterValue !== 'object') {
// Reached the actual value
if (typeof parameterValue === 'string' && parameterValue.charAt(0) === '=') {
if (
typeof parameterValue === 'string' &&
(parameterValue.charAt(0) === '=' || hasRenamableContent)
) {
// Is expression so has to be rewritten
// To not run the "expensive" regex stuff when it is not needed
// make a simple check first if it really contains the the node-name
@ -467,7 +465,7 @@ export class Workflow {
const returnArray: any[] = [];
for (const currentValue of parameterValue) {
returnArray.push(this.renameNodeInExpressions(currentValue, currentName, newName));
returnArray.push(this.renameNodeInParameterValue(currentValue, currentName, newName));
}
return returnArray;
@ -478,10 +476,11 @@ export class Workflow {
for (const parameterName of Object.keys(parameterValue || {})) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
returnData[parameterName] = this.renameNodeInExpressions(
returnData[parameterName] = this.renameNodeInParameterValue(
parameterValue![parameterName as keyof typeof parameterValue],
currentName,
newName,
{ hasRenamableContent },
);
}
@ -505,11 +504,20 @@ export class Workflow {
// Update the expressions which reference the node
// with its old name
for (const node of Object.values(this.nodes)) {
node.parameters = this.renameNodeInExpressions(
node.parameters = this.renameNodeInParameterValue(
node.parameters,
currentName,
newName,
) as INodeParameters;
if (NODES_WITH_RENAMABLE_CONTENT.has(node.type)) {
node.parameters.jsCode = this.renameNodeInParameterValue(
node.parameters.jsCode,
currentName,
newName,
{ hasRenamableContent: true },
);
}
}
// Change all source connections

View file

@ -20,7 +20,7 @@ interface StubNode {
}
describe('Workflow', () => {
describe('renameNodeInExpressions', () => {
describe('renameNodeInParameterValue for expressions', () => {
const tests = [
{
description: 'do nothing if there is no expression',
@ -257,7 +257,7 @@ describe('Workflow', () => {
for (const testData of tests) {
test(testData.description, () => {
const result = workflow.renameNodeInExpressions(
const result = workflow.renameNodeInParameterValue(
testData.input.parameters,
testData.input.currentName,
testData.input.newName,
@ -267,6 +267,58 @@ describe('Workflow', () => {
}
});
describe('renameNodeInParameterValue for node with renamable content', () => {
const tests = [
{
description: "should work with $('name')",
input: {
currentName: 'Old',
newName: 'New',
parameters: { jsCode: "$('Old').first();" },
},
output: { jsCode: "$('New').first();" },
},
{
description: "should work with $node['name'] and $node.name",
input: {
currentName: 'Old',
newName: 'New',
parameters: { jsCode: "$node['Old'].first(); $node.Old.first();" },
},
output: { jsCode: "$node['New'].first(); $node.New.first();" },
},
{
description: 'should work with $items()',
input: {
currentName: 'Old',
newName: 'New',
parameters: { jsCode: "$items('Old').first();" },
},
output: { jsCode: "$items('New').first();" },
},
];
const workflow = new Workflow({
nodes: [],
connections: {},
active: false,
nodeTypes: Helpers.NodeTypes(),
});
for (const t of tests) {
test(t.description, () => {
expect(
workflow.renameNodeInParameterValue(
t.input.parameters,
t.input.currentName,
t.input.newName,
{ hasRenamableContent: true },
),
).toEqual(t.output);
});
}
});
describe('renameNode', () => {
const tests = [
{
@ -605,9 +657,9 @@ describe('Workflow', () => {
},
},
},
// This does just a basic test if "renameNodeInExpressions" gets used. More complex
// This does just a basic test if "renameNodeInParameterValue" gets used. More complex
// tests with different formats and levels are in the separate tests for the function
// "renameNodeInExpressions"
// "renameNodeInParameterValue"
{
description: 'change name also in expressions which use node-name (dot notation)',
input: {