chore: Extract node parsing from renameNode functionality (no-changelog) (#14822)

packages/workflow/src/NodeReferenceParserUtils.ts

@@ -0,0 +1,71 @@
+export function hasDotNotationBannedChar(nodeName: string) {
+	const DOT_NOTATION_BANNED_CHARS = /^(\d)|[\\ `!@#$%^&*()_+\-=[\]{};':"\\|,.<>?~]/g;
+
+	return DOT_NOTATION_BANNED_CHARS.test(nodeName);
+}
+
+export function backslashEscape(nodeName: string) {
+	const BACKSLASH_ESCAPABLE_CHARS = /[.*+?^${}()|[\]\\]/g;
+
+	return nodeName.replace(BACKSLASH_ESCAPABLE_CHARS, (char) => `\\${char}`);
+}
+
+export function dollarEscape(nodeName: string) {
+	return nodeName.replace(new RegExp('\\$', 'g'), '$$$$');
+}
+
+type AccessPattern = {
+	checkPattern: string;
+	replacePattern: (name: string) => string;
+	customCallback?: (expression: string, newName: string, escapedNewName: string) => string;
+};
+
+const ACCESS_PATTERNS: AccessPattern[] = [
+	{
+		checkPattern: '$(',
+		replacePattern: (s) => String.raw`(\$\(['"])${s}(['"]\))`,
+	},
+	{
+		checkPattern: '$node[',
+		replacePattern: (s) => String.raw`(\$node\[['"])${s}(['"]\])`,
+	},
+	{
+		checkPattern: '$node.',
+		replacePattern: (s) => String.raw`(\$node\.)${s}(\.?)`,
+		customCallback: (expression: string, newName: string, escapedNewName: string) => {
+			if (hasDotNotationBannedChar(newName)) {
+				const regex = new RegExp(`.${backslashEscape(newName)}( |\\.)`, 'g');
+				return expression.replace(regex, `["${escapedNewName}"]$1`);
+			}
+			return expression;
+		},
+	},
+	{
+		checkPattern: '$items(',
+		replacePattern: (s) => String.raw`(\$items\(['"])${s}(['"],|['"]\))`,
+	},
+];
+
+export function applyAccessPatterns(expression: string, previousName: string, newName: string) {
+	// To not run the "expensive" regex stuff when it is not needed
+	// make a simple check first if it really contains the node-name
+	if (!expression.includes(previousName)) return expression;
+
+	// Really contains node-name (even though we do not know yet if really as $node-expression)
+	const escapedOldName = backslashEscape(previousName); // for match
+	const escapedNewName = dollarEscape(newName); // for replacement
+
+	for (const pattern of ACCESS_PATTERNS) {
+		if (expression.includes(pattern.checkPattern)) {
+			expression = expression.replace(
+				new RegExp(pattern.replacePattern(escapedOldName), 'g'),
+				`$1${escapedNewName}$2`,
+			);
+
+			if (pattern.customCallback) {
+				expression = pattern.customCallback(expression, newName, escapedNewName);
+			}
+		}
+	}
+	return expression;
+}

packages/workflow/src/Workflow.ts

@@ -31,6 +31,7 @@ import type {
 } from './Interfaces';
 import { NodeConnectionTypes } from './Interfaces';
 import * as NodeHelpers from './NodeHelpers';
+import { applyAccessPatterns } from './NodeReferenceParserUtils';
 import * as ObservableObject from './ObservableObject';
 
 function dedupe<T>(arr: T[]): T[] {
@@ -333,43 +334,7 @@ export class Workflow {
 				typeof parameterValue === 'string' &&
 				(parameterValue.charAt(0) === '=' || hasRenamableContent)
 			) {
-				// Is expression so has to be rewritten
+				parameterValue = applyAccessPatterns(parameterValue, currentName, newName);
-				// To not run the "expensive" regex stuff when it is not needed
-				// make a simple check first if it really contains the node-name
-				if (parameterValue.includes(currentName)) {
-					// Really contains node-name (even though we do not know yet if really as $node-expression)
-
-					const escapedOldName = backslashEscape(currentName); // for match
-					const escapedNewName = dollarEscape(newName); // for replacement
-
-					const setNewName = (expression: string, oldPattern: string) =>
-						expression.replace(new RegExp(oldPattern, 'g'), `$1${escapedNewName}$2`);
-
-					if (parameterValue.includes('$(')) {
-						const oldPattern = String.raw`(\$\(['"])${escapedOldName}(['"]\))`;
-						parameterValue = setNewName(parameterValue, oldPattern);
-					}
-
-					if (parameterValue.includes('$node[')) {
-						const oldPattern = String.raw`(\$node\[['"])${escapedOldName}(['"]\])`;
-						parameterValue = setNewName(parameterValue, oldPattern);
-					}
-
-					if (parameterValue.includes('$node.')) {
-						const oldPattern = String.raw`(\$node\.)${escapedOldName}(\.?)`;
-						parameterValue = setNewName(parameterValue, oldPattern);
-
-						if (hasDotNotationBannedChar(newName)) {
-							const regex = new RegExp(`.${backslashEscape(newName)}( |\\.)`, 'g');
-							parameterValue = parameterValue.replace(regex, `["${escapedNewName}"]$1`);
-						}
-					}
-
-					if (parameterValue.includes('$items(')) {
-						const oldPattern = String.raw`(\$items\(['"])${escapedOldName}(['"],|['"]\))`;
-						parameterValue = setNewName(parameterValue, oldPattern);
-					}
-				}
 			}
 
 			return parameterValue;
@@ -963,19 +928,3 @@ export class Workflow {
 		return this.__getStartNode(Object.keys(this.nodes));
 	}
 }
-
-function hasDotNotationBannedChar(nodeName: string) {
-	const DOT_NOTATION_BANNED_CHARS = /^(\d)|[\\ `!@#$%^&*()_+\-=[\]{};':"\\|,.<>?~]/g;
-
-	return DOT_NOTATION_BANNED_CHARS.test(nodeName);
-}
-
-function backslashEscape(nodeName: string) {
-	const BACKSLASH_ESCAPABLE_CHARS = /[.*+?^${}()|[\]\\]/g;
-
-	return nodeName.replace(BACKSLASH_ESCAPABLE_CHARS, (char) => `\\${char}`);
-}
-
-function dollarEscape(nodeName: string) {
-	return nodeName.replace(new RegExp('\\$', 'g'), '$$$$');
-}

packages/workflow/test/NodeReferenceParserUtils.test.ts

@@ -0,0 +1,134 @@
+import {
+	hasDotNotationBannedChar,
+	backslashEscape,
+	dollarEscape,
+	applyAccessPatterns,
+} from '../src/NodeReferenceParserUtils';
+
+describe('NodeReferenceParserUtils', () => {
+	describe('hasDotNotationBannedChar', () => {
+		it('should return true for strings with banned characters', () => {
+			expect(hasDotNotationBannedChar('1abc')).toBe(true);
+			expect(hasDotNotationBannedChar('abc!')).toBe(true);
+			expect(hasDotNotationBannedChar('abc@')).toBe(true);
+		});
+
+		it('should return false for strings without banned characters', () => {
+			expect(hasDotNotationBannedChar('abc')).toBe(false);
+			expect(hasDotNotationBannedChar('validName')).toBe(false);
+		});
+	});
+
+	describe('backslashEscape', () => {
+		it('should escape special characters with a backslash', () => {
+			expect(backslashEscape('abc.def')).toBe('abc\\.def');
+			expect(backslashEscape('[abc]')).toBe('\\[abc\\]');
+			expect(backslashEscape('a+b')).toBe('a\\+b');
+		});
+
+		it('should return the same string if no escapable characters are present', () => {
+			expect(backslashEscape('abc')).toBe('abc');
+		});
+	});
+
+	describe('dollarEscape', () => {
+		it('should escape dollar signs with double dollar signs', () => {
+			expect(dollarEscape('$abc')).toBe('$$abc');
+			expect(dollarEscape('abc$')).toBe('abc$$');
+			expect(dollarEscape('$a$b$c')).toBe('$$a$$b$$c');
+		});
+
+		it('should return the same string if no dollar signs are present', () => {
+			expect(dollarEscape('abc')).toBe('abc');
+		});
+	});
+
+	describe('applyAccessPatterns', () => {
+		it.each([
+			{
+				expression: '$node["oldName"].data',
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: '$node["newName"].data',
+			},
+			{
+				expression: '$node.oldName.data',
+				previousName: 'oldName',
+				newName: 'new.Name',
+				expected: '$node["new.Name"].data',
+			},
+			{
+				expression: '$node["someOtherName"].data',
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: '$node["someOtherName"].data',
+			},
+			{
+				expression: '$node["oldName"].data + $node["oldName"].info',
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: '$node["newName"].data + $node["newName"].info',
+			},
+			{
+				expression: '$items("oldName", 0)',
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: '$items("newName", 0)',
+			},
+			{
+				expression: "$items('oldName', 0)",
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: "$items('newName', 0)",
+			},
+			{
+				expression: "$('oldName')",
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: "$('newName')",
+			},
+			{
+				expression: '$("oldName")',
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: '$("newName")',
+			},
+			{
+				expression: '$node["oldName"].data + $items("oldName", 0) + $("oldName")',
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: '$node["newName"].data + $items("newName", 0) + $("newName")',
+			},
+			{
+				expression: '$node["oldName"].data + $items("oldName", 0)',
+				previousName: 'oldName',
+				newName: 'new-Name',
+				expected: '$node["new-Name"].data + $items("new-Name", 0)',
+			},
+			{
+				expression: '$node["old-Name"].data + $items("old-Name", 0)',
+				previousName: 'old-Name',
+				newName: 'newName',
+				expected: '$node["newName"].data + $items("newName", 0)',
+			},
+			{
+				expression: 'someRandomExpression("oldName")',
+				previousName: 'oldName',
+				newName: 'newName',
+				expected: 'someRandomExpression("oldName")',
+			},
+			{
+				expression: '$("old\\"Name")',
+				previousName: 'old\\"Name',
+				newName: 'n\\\'ew\\"Name',
+				expected: '$("n\\\'ew\\"Name")',
+			},
+		])(
+			'should correctly transform expression "$expression" with previousName "$previousName" and newName "$newName"',
+			({ expression, previousName, newName, expected }) => {
+				const result = applyAccessPatterns(expression, previousName, newName);
+				expect(result).toBe(expected);
+			},
+		);
+	});
+});