From 88bce7fd8bb91e4d268d645ed9f0d6ed4557caa2 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Wed, 23 Apr 2025 13:22:08 +0200 Subject: [PATCH] chore: Extract node parsing from renameNode functionality (no-changelog) (#14822) --- .../workflow/src/NodeReferenceParserUtils.ts | 71 ++++++++++ packages/workflow/src/Workflow.ts | 55 +------ .../test/NodeReferenceParserUtils.test.ts | 134 ++++++++++++++++++ 3 files changed, 207 insertions(+), 53 deletions(-) create mode 100644 packages/workflow/src/NodeReferenceParserUtils.ts create mode 100644 packages/workflow/test/NodeReferenceParserUtils.test.ts diff --git a/packages/workflow/src/NodeReferenceParserUtils.ts b/packages/workflow/src/NodeReferenceParserUtils.ts new file mode 100644 index 0000000000..32bd25ee56 --- /dev/null +++ b/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; +} diff --git a/packages/workflow/src/Workflow.ts b/packages/workflow/src/Workflow.ts index 4b711e1fb2..81a4524a4c 100644 --- a/packages/workflow/src/Workflow.ts +++ b/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(arr: T[]): T[] { @@ -333,43 +334,7 @@ export class Workflow { 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 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); - } - } + parameterValue = applyAccessPatterns(parameterValue, currentName, newName); } 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'), '$$$$'); -} diff --git a/packages/workflow/test/NodeReferenceParserUtils.test.ts b/packages/workflow/test/NodeReferenceParserUtils.test.ts new file mode 100644 index 0000000000..e0eb4a6596 --- /dev/null +++ b/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); + }, + ); + }); +});