Skip to content

Commit a43c285

Browse files
authored
fix(designer): Changing where expressions are being escaped (#6539)
* Revert "feat(designer): No longer escapes characters in token expressions (#6131)" This reverts commit a3f6e31. * Reapply "feat(designer): No longer escapes characters in token expressions (#6131)" This reverts commit 6b9d422. * Revert "feat(designer): No longer escapes characters in token expressions (#6131)" This reverts commit a3f6e31. * Reapply "feat(designer): No longer escapes characters in token expressions (#6131)" This reverts commit d525665. * fix issue * adding endtoend test * fix test
1 parent 51a9a5a commit a43c285

File tree

10 files changed

+129
-36
lines changed

10 files changed

+129
-36
lines changed
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import test, { expect } from '@playwright/test';
2+
import { GoToMockWorkflow } from '../utils/GoToWorkflow';
3+
4+
test.describe(
5+
'Escape Expression Tokens Tests',
6+
{
7+
tag: '@mock',
8+
},
9+
async () => {
10+
test('Expressions should be able to use escape characters and serialize', async ({ page }) => {
11+
await page.goto('/');
12+
await GoToMockWorkflow(page, 'Panel');
13+
await page.getByLabel('HTTP operation, HTTP connector').click();
14+
await page.getByTitle('Enter request content').getByRole('paragraph').click();
15+
await page.locator('[data-automation-id="msla-token-picker-entrypoint-button-expression"]').click();
16+
const viewLine = page.locator('.view-line').first();
17+
await viewLine.click();
18+
// full expression not typed out because Monaco automatically fills closing brackets and single quotes
19+
await viewLine.pressSequentially("array(split(variables('ArrayVariable'), '\n");
20+
await page.getByRole('tab', { name: 'Dynamic content' }).click();
21+
await page.getByRole('button', { name: 'Add', exact: true }).click();
22+
await page.getByRole('tab', { name: 'Code view' }).click();
23+
await expect(page.getByRole('code')).toContainText(
24+
'{ "type": "Http", "inputs": { "uri": "http://test.com", "method": "GET", "body": "@{variables(\'ArrayVariable\')}@{array(split(variables (\'ArrayVariable\'), \'\\r\\n\'))}" }, "runAfter": { "Filter_array": [ "SUCCEEDED" ] }, "runtimeConfiguration": { "contentTransfer": { "transferMode": "Chunked" } }}'
25+
);
26+
});
27+
28+
test('Expressions should be able to use escaped escape characters and serialize as the same', async ({ page }) => {
29+
await page.goto('/');
30+
await GoToMockWorkflow(page, 'Panel');
31+
await page.getByLabel('HTTP operation, HTTP connector').click();
32+
await page.getByTitle('Enter request content').getByRole('paragraph').click();
33+
await page.locator('[data-automation-id="msla-token-picker-entrypoint-button-expression"]').click();
34+
const viewLine = page.locator('.view-line').first();
35+
await viewLine.click();
36+
// full expression not typed out because Monaco automatically fills closing brackets and single quotes
37+
await viewLine.pressSequentially("array(split(variables('ArrayVariable'), '\\n");
38+
await page.getByRole('tab', { name: 'Dynamic content' }).click();
39+
await page.getByRole('button', { name: 'Add', exact: true }).click();
40+
await page.getByRole('tab', { name: 'Code view' }).click();
41+
await expect(page.getByRole('code')).toContainText(
42+
'{ "type": "Http", "inputs": { "uri": "http://test.com", "method": "GET", "body": "@{variables(\'ArrayVariable\')}@{array(split(variables (\'ArrayVariable\'), \'\\n\'))}" }, "runAfter": { "Filter_array": [ "SUCCEEDED" ] }, "runtimeConfiguration": { "contentTransfer": { "transferMode": "Chunked" } }}'
43+
);
44+
});
45+
}
46+
);
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import test, { expect } from '@playwright/test';
2-
import { GoToMockWorkflow } from './utils/GoToWorkflow';
3-
import { getSerializedWorkflowFromState } from './utils/designerFunctions';
2+
import { GoToMockWorkflow } from '../utils/GoToWorkflow';
3+
import { getSerializedWorkflowFromState } from '../utils/designerFunctions';
44

55
test.describe(
66
'Token Removal Tests',

libs/designer-ui/src/lib/editor/base/plugins/OpenTokenPicker.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { TokenPickerMode } from '../../../tokenpicker';
2-
import { UPDATE_TOKENPICKER_EXPRESSION } from '../../../tokenpicker/plugins/TokenPickerHandler';
2+
import { INITIALIZE_TOKENPICKER_EXPRESSION } from '../../../tokenpicker/plugins/InitializeTokenPickerExpressionHandler';
33
import { TokenType } from '../../models/parameter';
44
import { findChildNode } from '../utils/helper';
55
import { useLexicalComposerContext } from '@lexical/react/LexicalComposerContext';
@@ -24,7 +24,7 @@ export default function OpenTokenPicker({ openTokenPicker }: OpenTokenPickerProp
2424
if (node?.token?.tokenType === TokenType.FX) {
2525
openTokenPicker(TokenPickerMode.EXPRESSION);
2626
setTimeout(() => {
27-
editor.dispatchCommand(UPDATE_TOKENPICKER_EXPRESSION, payload);
27+
editor.dispatchCommand(INITIALIZE_TOKENPICKER_EXPRESSION, payload);
2828
}, 50);
2929
}
3030
return true;

libs/designer-ui/src/lib/tokenpicker/index.tsx

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type { ExpressionEditorEvent } from '../expressioneditor';
66
import { ExpressionEditor } from '../expressioneditor';
77
import { PanelSize } from '../panel/panelUtil';
88
import type { TokenGroup } from '@microsoft/logic-apps-shared';
9-
import TokenPickerHandler from './plugins/TokenPickerHandler';
9+
import TokenPickerHandler from './plugins/InitializeTokenPickerExpressionHandler';
1010
import UpdateTokenNode from './plugins/UpdateTokenNode';
1111
import { TokenPickerFooter } from './tokenpickerfooter';
1212
import { TokenPickerHeader } from './tokenpickerheader';
@@ -22,7 +22,7 @@ import { useIntl } from 'react-intl';
2222
import { Button } from '@fluentui/react-components';
2323
import copilotLogo from './images/copilotLogo.svg';
2424
import { Nl2fExpressionAssistant } from './nl2fExpressionAssistant';
25-
import { isCopilotServiceEnabled } from '@microsoft/logic-apps-shared';
25+
import { escapeString, isCopilotServiceEnabled } from '@microsoft/logic-apps-shared';
2626

2727
export const TokenPickerMode = {
2828
TOKEN: 'token',
@@ -164,16 +164,17 @@ export function TokenPicker({
164164
}
165165
}, [anchorKey, editor, windowDimensions.height]);
166166

167-
const handleUpdateExpressionToken = (s: string, n: NodeKey) => {
168-
setExpression({ value: s, selectionStart: 0, selectionEnd: 0 });
167+
const handleInitializeExpression = (s: string, n: NodeKey) => {
168+
const escapedString = escapeString(s, /*requireSingleQuotesWrap*/ true);
169+
setExpression({ value: escapedString, selectionStart: 0, selectionEnd: 0 });
169170
setSelectedMode(TokenPickerMode.EXPRESSION);
170171
setExpressionToBeUpdated(n);
171172

172173
setTimeout(() => {
173174
expressionEditorRef.current?.setSelection({
174-
startLineNumber: s.length + 1,
175+
startLineNumber: escapedString.length + 1,
175176
startColumn: 1,
176-
endLineNumber: s.length + 1,
177+
endLineNumber: escapedString.length + 1,
177178
endColumn: 1,
178179
});
179180
expressionEditorRef.current?.focus();
@@ -415,7 +416,7 @@ export function TokenPicker({
415416
</div>
416417
</div>
417418
</Callout>
418-
{tokenClickedCallback ? null : <TokenPickerHandler handleUpdateExpressionToken={handleUpdateExpressionToken} />}
419+
{tokenClickedCallback ? null : <TokenPickerHandler handleInitializeExpression={handleInitializeExpression} />}
419420
{tokenClickedCallback ? null : <UpdateTokenNode />}
420421
</>
421422
);

libs/designer-ui/src/lib/tokenpicker/plugins/TokenPickerHandler.tsx renamed to libs/designer-ui/src/lib/tokenpicker/plugins/InitializeTokenPickerExpressionHandler.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,29 +5,29 @@ import type { LexicalCommand, NodeKey } from 'lexical';
55
import { $getRoot, COMMAND_PRIORITY_EDITOR, createCommand } from 'lexical';
66
import { useEffect } from 'react';
77

8-
export const UPDATE_TOKENPICKER_EXPRESSION: LexicalCommand<string> = createCommand();
8+
export const INITIALIZE_TOKENPICKER_EXPRESSION: LexicalCommand<string> = createCommand();
99

1010
interface TokenPickerHandlerProps {
11-
handleUpdateExpressionToken?: (s: string, n: NodeKey) => void;
11+
handleInitializeExpression?: (s: string, n: NodeKey) => void;
1212
}
1313

14-
export default function TokenPickerHandler({ handleUpdateExpressionToken }: TokenPickerHandlerProps): null {
14+
export default function TokenPickerHandler({ handleInitializeExpression }: TokenPickerHandlerProps): null {
1515
const [editor] = useLexicalComposerContext();
1616

1717
useEffect(() => {
1818
return editor.registerCommand<string>(
19-
UPDATE_TOKENPICKER_EXPRESSION,
19+
INITIALIZE_TOKENPICKER_EXPRESSION,
2020
(payload: string) => {
2121
const node = findChildNode($getRoot(), payload, TokenType.FX);
2222
if (node?.token?.tokenType === TokenType.FX) {
23-
handleUpdateExpressionToken?.(node?.value ?? '', payload);
23+
handleInitializeExpression?.(node?.value ?? '', payload);
2424
} else {
2525
editor.focus();
2626
}
2727
return true;
2828
},
2929
COMMAND_PRIORITY_EDITOR
3030
);
31-
}, [editor, handleUpdateExpressionToken]);
31+
}, [editor, handleInitializeExpression]);
3232
return null;
3333
}

libs/designer-ui/src/lib/tokenpicker/tokenpickerfooter.tsx

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
ScannerException,
1919
guid,
2020
isCopilotServiceEnabled,
21+
unescapeString,
2122
} from '@microsoft/logic-apps-shared';
2223
import type { Expression, TokenGroup, Token as TokenGroupToken } from '@microsoft/logic-apps-shared';
2324
import type { LexicalEditor, NodeKey } from 'lexical';
@@ -112,9 +113,12 @@ export function TokenPickerFooter({
112113

113114
const onUpdateOrAddClicked = () => {
114115
let currExpression: Expression | null = null;
116+
const unescapedExpressionValue = unescapeString(expression.value);
115117
try {
116-
currExpression = ExpressionParser.parseExpression(expression.value);
118+
console.log(unescapedExpressionValue);
119+
currExpression = ExpressionParser.parseExpression(unescapedExpressionValue);
117120
} catch (ex) {
121+
console.log(ex);
118122
if (ex instanceof ScannerException && (ex as any).message === ExpressionExceptionCode.MISUSED_DOUBLE_QUOTES) {
119123
// if the expression contains misused double quotes, we'll show a different error message
120124
setExpressionEditorError(invalidExpressionQuotations);
@@ -149,18 +153,18 @@ export function TokenPickerFooter({
149153
icon: FxIcon,
150154
title: getExpressionTokenTitle(currExpression),
151155
key: guid(),
152-
value: expression.value,
156+
value: unescapedExpressionValue,
153157
};
154158

155159
// if the expression is already in the expression editor, we'll update the token node
156160
if (expressionToBeUpdated) {
157161
editor?.dispatchCommand(UPDATE_TOKEN_NODE, {
158-
updatedValue: expression.value,
162+
updatedValue: unescapedExpressionValue,
159163
updatedTitle: token.title,
160164
updatedData: {
161165
id: guid(),
162166
type: ValueSegmentType.TOKEN,
163-
value: expression.value,
167+
value: unescapedExpressionValue,
164168
token,
165169
},
166170
nodeKey: expressionToBeUpdated,
@@ -174,7 +178,7 @@ export function TokenPickerFooter({
174178
title: token.title,
175179
icon: token.icon,
176180
value: token.value,
177-
data: { id: guid(), type: ValueSegmentType.TOKEN, value: expression.value, token },
181+
data: { id: guid(), type: ValueSegmentType.TOKEN, value: unescapedExpressionValue, token },
178182
});
179183
}
180184
}

libs/designer/src/lib/core/utils/parameters/helper.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ import {
117117
isBodySegment,
118118
canStringBeConverted,
119119
splitAtIndex,
120-
unescapeString,
121120
} from '@microsoft/logic-apps-shared';
122121
import type {
123122
AuthProps,
@@ -3791,14 +3790,13 @@ function parameterValueToStringWithoutCasting(value: ValueSegment[], forValidati
37913790
const shouldInterpolateTokens = (value.length > 1 || shouldInterpolateSingleToken) && value.some(isTokenValueSegment);
37923791

37933792
return value
3794-
.map((segment) => {
3795-
const { value: segmentValue } = segment;
3796-
if (isTokenValueSegment(segment)) {
3797-
const token = forValidation ? segmentValue || null : unescapeString(segmentValue);
3798-
return shouldInterpolateTokens ? `@{${token}}` : `@${token}`;
3793+
.map((expression) => {
3794+
let expressionValue = forValidation ? expression.value || null : expression.value;
3795+
if (isTokenValueSegment(expression)) {
3796+
expressionValue = shouldInterpolateTokens ? `@{${expressionValue}}` : `@${expressionValue}`;
37993797
}
38003798

3801-
return forValidation ? segmentValue || null : segmentValue;
3799+
return expressionValue;
38023800
})
38033801
.join('');
38043802
}

libs/designer/src/lib/core/utils/parameters/segment.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
isNullOrUndefined,
1919
startsWith,
2020
UnsupportedException,
21-
escapeString,
2221
} from '@microsoft/logic-apps-shared';
2322

2423
/**
@@ -188,11 +187,10 @@ export class ValueSegmentConvertor {
188187
return dynamicContentTokenSegment;
189188
}
190189
// Note: We need to get the expression value if this is a sub expression resulted from uncasting.
191-
const value = escapeString(
190+
const value =
192191
expression.startPosition === 0
193192
? expression.expression
194-
: expression.expression.substring(expression.startPosition, expression.endPosition)
195-
);
193+
: expression.expression.substring(expression.startPosition, expression.endPosition);
196194
return this._createExpressionTokenValueSegment(value, expression);
197195
}
198196

libs/logic-apps-shared/src/utils/src/lib/helpers/__test__/stringFunctions.spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,46 @@ describe('escapeString', () => {
156156
it('should handle an empty string', () => {
157157
expect(escapeString('')).toEqual('');
158158
});
159+
160+
it('does not escape characters if requireSingleQuotesWrap is true and there are no surrounding single quotes', () => {
161+
const input = 'Test\nTest';
162+
const result = escapeString(input, true);
163+
expect(result).toBe(input); // No change, since it's not surrounded by single quotes
164+
});
165+
166+
it('escapes characters if requireSingleQuotesWrap is true and the string is surrounded by single quotes', () => {
167+
const input = "'Test\nTest'";
168+
const expectedOutput = "'Test\\nTest'";
169+
const result = escapeString(input, true);
170+
expect(result).toBe(expectedOutput); // Should escape \n
171+
});
172+
173+
it('escapes characters even if the string contains multiple lines when requireSingleQuotesWrap is true and surrounded by single quotes', () => {
174+
const input = "'Test\nAnotherLine\nTest'";
175+
const expectedOutput = `'Test
176+
AnotherLine
177+
Test'`;
178+
const result = escapeString(input, true);
179+
expect(result).toBe(expectedOutput);
180+
});
181+
182+
it('does not escape characters if requireSingleQuotesWrap is true and string is not surrounded by single quotes', () => {
183+
const input = 'Test\nTest';
184+
const result = escapeString(input, true);
185+
expect(result).toBe(input); // No change, since it's not surrounded by single quotes
186+
});
187+
188+
it('escapes characters when requireSingleQuotesWrap is false regardless of surrounding quotes', () => {
189+
const input = "'Test\nTest'";
190+
const expectedOutput = "'Test\\nTest'";
191+
const result = escapeString(input, false);
192+
expect(result).toBe(expectedOutput);
193+
});
194+
195+
it('escapes characters when requireSingleQuotesWrap is undefined regardless of surrounding quotes', () => {
196+
const input = "'Test\nTest'";
197+
const expectedOutput = "'Test\\nTest'";
198+
const result = escapeString(input);
199+
expect(result).toBe(expectedOutput);
200+
});
159201
});

libs/logic-apps-shared/src/utils/src/lib/helpers/stringFunctions.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export const canStringBeConverted = (s: string): boolean => {
4343
try {
4444
const parsed = JSON.parse(s);
4545
return Array.isArray(parsed);
46-
} catch (e) {
46+
} catch (_e) {
4747
return false;
4848
}
4949
};
@@ -71,8 +71,12 @@ export const unescapeString = (input: string): string => {
7171
});
7272
};
7373

74-
export const escapeString = (input: string): string => {
75-
return input.replace(/[\n\r\t\v]/g, (char) => {
74+
export const escapeString = (input: string, requireSingleQuotesWrap?: boolean): string => {
75+
if (requireSingleQuotesWrap && !/'.*[\n\r\t\v].*'/.test(input)) {
76+
return input;
77+
}
78+
79+
return input?.replace(/[\n\r\t\v]/g, (char) => {
7680
switch (char) {
7781
case '\n':
7882
return '\\n';

0 commit comments

Comments
 (0)